Skip to content

Commit b30ffe3

Browse files
reid-spencerclaude
andcommitted
[2.3] Work the resolution-position slice: size it, clear one with a test
The slice named this morning, sized: 80 sites, concentrated in ValidationPass (35), UseCaseWitnessPass (9), JsonifierPass (7) and AST.scala (7). First one examined is `ResolutionPass.valueScopeField`'s `aggFields`, and it was a strong suspect on precedent: it reads a Type's fields and answers `Seq.empty` for anything that is not an AggregateTypeExpression, so it cannot see through an alias — exactly the defect `isAddressFieldFor` had, fixed in ccd278c because aliasing is riddl-models' house style. It is NOT a defect. An aliased state record resolves exactly as a direct one, because `valueScopeField` is not the only route: the A55 ValueRef walk reaches the field anyway. So the empty is the "nothing to do here" kind the rule permits. `AliasedValueScopeTest` keeps both forms green, which is worth more than the verdict: if a future change ever makes `valueScopeField` the sole route, this reddens instead of riddl-models reddening. The method note is the point. Reasoning from "this shape was a defect over there" got the site onto the list and was still wrong about it — only running it settled the question. Same lesson as [2.6]'s unreachable arm, from the other direction. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
1 parent adfe54e commit b30ffe3

2 files changed

Lines changed: 137 additions & 2 deletions

File tree

BACKLOG.md

Lines changed: 22 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -857,8 +857,28 @@ that needs a ruling before either can be fixed.
857857
**That is the whole output-producing slice.** `RiddlAPI.scala:156` carries its
858858
own justification (Comment/Include have no identifier).
859859

860-
**The remaining ~185 are unexamined.** Next slice, by the same "cheap to size"
861-
logic: `case _ => None` and `case _ => Seq.empty` in RESOLUTION positions,
860+
**AUDIT PROGRESS 2026-08-18 — the resolution-position slice, sized and started.**
861+
`grep -rn -E "case _ *=> *(None|Seq\.empty|Nil)"` over `passes`/`language`/
862+
`riddlLib` returns **80 sites**, concentrated in `ValidationPass` (35),
863+
`UseCaseWitnessPass` (9), `JsonifierPass` (7) and `AST.scala` (7).
864+
**EXAMINED AND CLEARED, with a test rather than an opinion:**
865+
`ResolutionPass.valueScopeField`'s `aggFields` (`:724`) — it reads a Type's
866+
fields and answers `Seq.empty` for anything that is not an
867+
`AggregateTypeExpression`, which does NOT see through an alias. That made it a
868+
strong suspect, since `isAddressFieldFor` had exactly this defect and was taught
869+
to follow alias chains in `ccd278c00` because aliasing is riddl-models' house
870+
style. **It is not a defect**: an aliased state record resolves exactly as a
871+
direct one, because `valueScopeField` is not the only route — the A55 `ValueRef`
872+
walk reaches the field anyway. `AliasedValueScopeTest` pins both forms, so a
873+
future change that made `valueScopeField` the sole route reddens here instead of
874+
reddening riddl-models.
875+
**Method note worth keeping: the suspicion was well-founded and still wrong.**
876+
Reasoning from "this shape was a defect over there" got the site onto the list;
877+
only running it settled the question. That is the same lesson as [2.6]'s
878+
unreachable arm, in the opposite direction.
879+
880+
**The remaining ~78 are unexamined.** Continue with the same
881+
slice: `case _ => None` and `case _ => Seq.empty` in RESOLUTION positions,
862882
where an empty answer is read downstream as "no such thing" — the shape that
863883
produced both of this week's found defects (`DependencyAnalysisPass.typeDeps`
864884
empty forever, `MessageFlowPass` dropping let-local edges). Neither of those
Lines changed: 115 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,115 @@
1+
/*
2+
* Copyright 2019-2026 Ossum Inc.
3+
*
4+
* SPDX-License-Identifier: Apache-2.0
5+
*/
6+
7+
package com.ossuminc.riddl.passes.resolve
8+
9+
import com.ossuminc.riddl.language.Messages
10+
import com.ossuminc.riddl.language.parsing.RiddlParserInput
11+
import com.ossuminc.riddl.passes.validate.AbstractValidatingTest
12+
import com.ossuminc.riddl.utils.pc
13+
14+
import org.scalatest.TestData
15+
16+
/** [2.3]: `ResolutionPass.valueScopeField`'s `aggFields` reads a Type's fields with
17+
* `case ate: AggregateTypeExpression => ate.fields; case _ => Seq.empty` — an empty answer in a
18+
* RESOLUTION position, which downstream reads as "no such field".
19+
*
20+
* The question this pins is whether that empty is reachable through an ALIAS. `isAddressFieldFor`
21+
* had exactly this defect and was taught to follow alias chains in `ccd278c00`, on the grounds
22+
* that aliasing is riddl-models' documented house style — so the shape was a strong suspect.
23+
*
24+
* **VERDICT 2026-08-18: NOT a defect, and this suite is the evidence.** An aliased state record
25+
* resolves exactly as a direct one does, because `valueScopeField` is not the only route — the
26+
* A55 `ValueRef` walk reaches the field anyway. So the `case _ => Seq.empty` is the "nothing to do
27+
* here" kind the no-silent-fall-through rule explicitly permits, not the "I do not know what this
28+
* is" kind.
29+
*
30+
* Kept rather than deleted: it costs nothing and it pins the ONE thing that would make the empty
31+
* bite — a future change that made `valueScopeField` the sole route would turn these green cases
32+
* red instead of turning riddl-models red.
33+
*/
34+
class AliasedValueScopeTest extends AbstractValidatingTest {
35+
36+
private def errorsFor(src: String, td: TestData): Messages.Messages =
37+
var captured: Messages.Messages = Messages.empty
38+
parseAndValidateInput(RiddlParserInput(src, td), shouldFailOnErrors = false) {
39+
case (_, _, msgs) => captured = msgs; succeed
40+
}
41+
captured
42+
43+
"a bare value reference to a field of an ALIASED handled message" should {
44+
"resolve" in { (td: TestData) =>
45+
val src =
46+
"""domain D is {
47+
| context C is {
48+
| command Direct is { amount: Real }
49+
| entity E is {
50+
| handler H is {
51+
| on command D.C.Direct is {
52+
| let a = amount
53+
| do "use it"
54+
| }
55+
| }
56+
| }
57+
| }
58+
|}
59+
|""".stripMargin
60+
val errs = errorsFor(src, td).justErrors
61+
withClue(s"CONTROL (no alias) messages:\n${errorsFor(src, td).format}\n") {
62+
errs mustBe empty
63+
}
64+
}
65+
}
66+
67+
"a bare value reference to a field of an ALIASED entity state record" should {
68+
"resolve, exactly as it does when the state names the record directly" in { (td: TestData) =>
69+
val direct =
70+
"""domain D is {
71+
| context C is {
72+
| command Touch is { ??? }
73+
| record OrderData is { total: Real }
74+
| entity E is {
75+
| state main of record D.C.OrderData
76+
| handler H is {
77+
| on command D.C.Touch is {
78+
| let t = total
79+
| do "use it"
80+
| }
81+
| }
82+
| }
83+
| }
84+
|}
85+
|""".stripMargin
86+
val aliased =
87+
"""domain D is {
88+
| context C is {
89+
| command Touch is { ??? }
90+
| record OrderData is { total: Real }
91+
| type OrderState is D.C.OrderData
92+
| entity E is {
93+
| state main of record D.C.OrderState
94+
| handler H is {
95+
| on command D.C.Touch is {
96+
| let t = total
97+
| do "use it"
98+
| }
99+
| }
100+
| }
101+
| }
102+
|}
103+
|""".stripMargin
104+
105+
val directErrs = errorsFor(direct, td).justErrors
106+
withClue(s"DIRECT form messages:\n${errorsFor(direct, td).format}\n") {
107+
directErrs mustBe empty
108+
}
109+
val aliasErrs = errorsFor(aliased, td).justErrors
110+
withClue(s"ALIASED form messages:\n${errorsFor(aliased, td).format}\n") {
111+
aliasErrs mustBe empty
112+
}
113+
}
114+
}
115+
}

0 commit comments

Comments
 (0)