Skip to content

Commit 51738db

Browse files
reid-spencerclaude
andcommitted
Fix entity Id type check for correct scope placement
Replace single "no Id type" check with three scope-aware checks: - (a) Id defined inside entity body: warn to move to context - (b) Id not defined at all: warn as before - (c) Id defined at domain level: warn scope is too broad, use adaptors for inter-context invocations The correct placement is in the containing context so other entities within the same context can reference it. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 326e9be commit 51738db

2 files changed

Lines changed: 110 additions & 8 deletions

File tree

passes/jvm-native/src/test/scala/com/ossuminc/riddl/passes/validate/CompletenessTest.scala

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -464,6 +464,62 @@ class CompletenessTest extends AbstractValidatingTest {
464464
}
465465
}
466466

467+
"warn when entity Id type is defined inside entity body" in { (td: TestData) =>
468+
val input = RiddlParserInput(
469+
"""domain D is {
470+
| context C is {
471+
| type Evt is event { data: String }
472+
| entity E is {
473+
| type EId is Id(E)
474+
| record Fields is { data: String }
475+
| state Main of E.Fields
476+
| handler H is {
477+
| on init { set field E.Fields.data to "x" }
478+
| on event D.C.Evt {
479+
| set field E.Fields.data to "updated"
480+
| }
481+
| }
482+
| }
483+
| }
484+
|}
485+
|""".stripMargin,
486+
td
487+
)
488+
parseAndValidate(input.data, "test", shouldFailOnErrors = false) {
489+
(_, _, msgs) =>
490+
val cw = completenessWarnings(msgs)
491+
cw.exists(_.message.contains("move it to the containing context")) mustBe true
492+
}
493+
}
494+
495+
"warn when entity Id type is defined at domain level" in { (td: TestData) =>
496+
val input = RiddlParserInput(
497+
"""domain D is {
498+
| type EId is Id(C.E)
499+
| context C is {
500+
| type Evt is event { data: String }
501+
| entity E is {
502+
| record Fields is { data: String }
503+
| state Main of E.Fields
504+
| handler H is {
505+
| on init { set field E.Fields.data to "x" }
506+
| on event D.C.Evt {
507+
| set field E.Fields.data to "updated"
508+
| }
509+
| }
510+
| }
511+
| }
512+
|}
513+
|""".stripMargin,
514+
td
515+
)
516+
parseAndValidate(input.data, "test", shouldFailOnErrors = false) {
517+
(_, _, msgs) =>
518+
val cw = completenessWarnings(msgs)
519+
cw.exists(_.message.contains("outside the containing context")) mustBe true
520+
}
521+
}
522+
467523
"warn when event-sourced entity command handler does not emit event" in { (td: TestData) =>
468524
val input = RiddlParserInput(
469525
"""domain D is {

passes/shared/src/main/scala/com/ossuminc/riddl/passes/validate/ValidationPass.scala

Lines changed: 54 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1005,21 +1005,67 @@ case class ValidationPass(
10051005
s"${entity.identify} in ${context.identify} has no outlet streamlet to publish events on"
10061006
)
10071007
}
1008-
// Completeness: entity should define an Id type for its identity
1008+
// Completeness: entity Id type placement checks
10091009
if entity.nonEmpty then {
10101010
val parentContext = parents.collectFirst { case c: Context => c }
1011-
val entityTypes = entity.types ++ parentContext.toSeq.flatMap(_.types)
1012-
val hasIdType = entityTypes.exists { t =>
1013-
t.typEx match {
1014-
case _: UniqueId => true
1015-
case _ => false
1016-
}
1011+
1012+
// Search all known types via symbols table for Id types referencing this entity
1013+
def isIdForEntity(t: Type): Boolean = t.typEx match {
1014+
case uid: UniqueId =>
1015+
uid.entityPath.value.lastOption.contains(entity.id.value)
1016+
case _ => false
10171017
}
1018-
if !hasIdType then {
1018+
1019+
// Find all Id types for this entity and determine their scope
1020+
val allIdTypes = symbols.parentage.keys.collect {
1021+
case t: Type if isIdForEntity(t) => t
1022+
}.toSeq
1023+
1024+
if allIdTypes.isEmpty then {
1025+
// (b) Not defined at all
10191026
messages.addCompleteness(
10201027
entity.errorLoc,
10211028
s"${entity.identify} does not define an Id type for its identity"
10221029
)
1030+
} else {
1031+
allIdTypes.foreach { idType =>
1032+
val idParents = symbols.parentsOf(idType)
1033+
val directParent = idParents.headOption
1034+
directParent match {
1035+
case Some(e: Entity) if e == entity =>
1036+
// (a) Id defined inside entity — too narrow
1037+
messages.addCompleteness(
1038+
idType.errorLoc,
1039+
s"${idType.identify} is defined inside ${entity.identify}; " +
1040+
"move it to the containing context so other entities can reference it"
1041+
)
1042+
case Some(c: Context) if parentContext.contains(c) =>
1043+
// Correct placement — no warning
1044+
case Some(_: Include[?]) =>
1045+
// Id is in an include — check if the include's context matches
1046+
val idContext = symbols.contextOf(idType)
1047+
if idContext == parentContext then {
1048+
// Correct — in an included file within the same context
1049+
} else {
1050+
// (c) Outside the containing context
1051+
messages.addCompleteness(
1052+
idType.errorLoc,
1053+
s"${idType.identify} for ${entity.identify} is defined outside the containing context; " +
1054+
"constrain it to the context scope and use adaptors for inter-context invocations"
1055+
)
1056+
}
1057+
case _ =>
1058+
// (c) Defined at domain level or elsewhere — too broad
1059+
val idContext = symbols.contextOf(idType)
1060+
if idContext != parentContext then {
1061+
messages.addCompleteness(
1062+
idType.errorLoc,
1063+
s"${idType.identify} for ${entity.identify} is defined outside the containing context; " +
1064+
"constrain it to the context scope and use adaptors for inter-context invocations"
1065+
)
1066+
}
1067+
}
1068+
}
10231069
}
10241070
}
10251071
// Completeness: event-sourced entity must emit events on every command

0 commit comments

Comments
 (0)