Skip to content

Commit 6d6185e

Browse files
respencer-nclclaude
respencer-ncl
and
claude
committed
Enhance ValidationPass with bug fixes and new validations
Fix three bugs: SagaStep checked doStatements twice instead of undoStatements for revert validation, SagaStep shape check compared getClass of identical types (always true), and validateState called checkMetadata twice. Add validation for previously unvalidated definitions: Schema (kind vs structure compatibility, data/link/index ref resolution) and Relationship (processor ref resolution). Add semantic validations: streamlet shape vs inlet/outlet count, handler requirements for streamlets/adaptors/repositories, adaptor empty handler warning, projector repository ref resolution, epic/usecase user story user ref resolution, and function input/output type expression validation. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 3efc2f0 commit 6d6185e

4 files changed

Lines changed: 170 additions & 6 deletions

File tree

‎passes/input/check/everything/everything.check‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -112,6 +112,24 @@ passes/input/check/everything/everything.riddl(50:7->37):
112112
passes/input/check/everything/everything.riddl(50:7->37):
113113
Function 'whenUnderTheInfluence' is unused:
114114
function whenUnderTheInfluence is {
115+
passes/input/check/everything/everything.riddl(51:20->21):
116+
Field identifier 'n' is too short. The minimum length is 3:
117+
requires { n: Nothing }
118+
passes/input/check/everything/everything.riddl(51:20->21):
119+
Metadata in Field 'n' should not be empty:
120+
requires { n: Nothing }
121+
passes/input/check/everything/everything.riddl(51:20->21):
122+
Field 'n' should have a description:
123+
requires { n: Nothing }
124+
passes/input/check/everything/everything.riddl(52:19->20):
125+
Field identifier 'b' is too short. The minimum length is 3:
126+
returns { b: Boolean }
127+
passes/input/check/everything/everything.riddl(52:19->20):
128+
Metadata in Field 'b' should not be empty:
129+
returns { b: Boolean }
130+
passes/input/check/everything/everything.riddl(52:19->20):
131+
Field 'b' should have a description:
132+
returns { b: Boolean }
115133
passes/input/check/everything/everything.riddl(66:5->26):
116134
Metadata in Entity 'SomeOtherThing' should not be empty:
117135
entity SomeOtherThing is {
@@ -127,6 +145,9 @@ passes/input/check/everything/everything.riddl(67:7->20):
127145
passes/input/check/everything/everything.riddl(67:7->20):
128146
Sink 'trashBin' should have a description:
129147
sink trashBin is { inlet SOT_In is SomeOtherThing.ItHappened }
148+
passes/input/check/everything/everything.riddl(67:7->20):
149+
Sink 'trashBin' should have a handler:
150+
sink trashBin is { inlet SOT_In is SomeOtherThing.ItHappened }
130151
passes/input/check/everything/everything.riddl(67:26->38):
131152
Inlet 'SOT_In' is not connected:
132153
sink trashBin is { inlet SOT_In is SomeOtherThing.ItHappened }
@@ -136,8 +157,14 @@ passes/input/check/everything/everything.riddl(69:7->28):
136157
passes/input/check/everything/everything.riddl(69:7->28):
137158
State 'otherThingState' should have a description:
138159
state otherThingState of type Everything.StateType
160+
passes/input/check/everything/everything.riddl(7:5->18):
161+
Source 'Source' should have a handler:
162+
source Source is { outlet Commands is DoAThing } with { described by "Data Source" }
139163
passes/input/check/everything/everything.riddl(7:24->39):
140164
Outlet 'Commands' is overloaded with 2 kinds:
141165
Outlet 'Commands' at passes/input/check/everything/everything.riddl(7:24->52),
142166
Inlet 'Commands' at passes/input/check/everything/everything.riddl(8:20->47):
143167
source Source is { outlet Commands is DoAThing } with { described by "Data Source" }
168+
passes/input/check/everything/everything.riddl(8:5->14):
169+
Sink 'Sink' should have a handler:
170+
sink Sink is { inlet Commands is DoAThing } with { explained as "Data Sink" }

‎passes/input/check/saga/saga.check‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,9 @@ passes/input/check/saga/saga.riddl(13:7->20):
2222
passes/input/check/saga/saga.riddl(13:7->20):
2323
Sink 'trashCan' should have a description:
2424
sink trashCan is { inlet in is command Something }
25+
passes/input/check/saga/saga.riddl(13:7->20):
26+
Sink 'trashCan' should have a handler:
27+
sink trashCan is { inlet in is command Something }
2528
passes/input/check/saga/saga.riddl(4:5->23):
2629
Command 'UndoSomething' is unused:
2730
type UndoSomething = command { ??? }
@@ -34,6 +37,18 @@ passes/input/check/saga/saga.riddl(8:5->30):
3437
passes/input/check/saga/saga.riddl(5:5->26):
3538
Function 'AnotherThing' in Context 'ignore2' should have statements:
3639
function AnotherThing {
40+
passes/input/check/saga/saga.riddl(6:18->19):
41+
Field identifier 'a' is too short. The minimum length is 3:
42+
requires { a: Integer with { described by "a" } } returns { b: Integer with { described by "b"} }
43+
passes/input/check/saga/saga.riddl(6:67->68):
44+
Field identifier 'b' is too short. The minimum length is 3:
45+
requires { a: Integer with { described by "a" } } returns { b: Integer with { described by "b"} }
3746
passes/input/check/saga/saga.riddl(8:5->30):
3847
Function 'UndoAnotherThing' in Context 'ignore2' should have statements:
3948
function UndoAnotherThing {
49+
passes/input/check/saga/saga.riddl(9:18->19):
50+
Field identifier 'c' is too short. The minimum length is 3:
51+
requires { c: Integer with { described by "c" } } returns { d: Integer with { described by "d"} }
52+
passes/input/check/saga/saga.riddl(9:67->68):
53+
Field identifier 'd' is too short. The minimum length is 3:
54+
requires { c: Integer with { described by "c" } } returns { d: Integer with { described by "d"} }

‎passes/input/check/streaming/streaming.check‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
passes/input/check/streaming/streaming.riddl(4:3->28):
2+
Source 'GetWeatherForecast' should have a handler:
3+
source GetWeatherForecast is {
4+
passes/input/check/streaming/streaming.riddl(10:3->29):
5+
Flow 'GetCurrentTemperature' should have a handler:
6+
flow GetCurrentTemperature is {
7+
passes/input/check/streaming/streaming.riddl(17:3->23):
8+
Sink 'AttenuateSensor' should have a handler:
9+
sink AttenuateSensor is {

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

Lines changed: 119 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -151,6 +151,10 @@ case class ValidationPass(
151151
validateInclude(include)
152152
case bi: BASTImport =>
153153
validateBASTImport(bi, parentsAsSeq)
154+
case s: Schema =>
155+
validateSchema(s, parentsAsSeq)
156+
case r: Relationship =>
157+
validateRelationship(r, parentsAsSeq)
154158
case _: MatchCase => () // Validated through MatchStatement
155159
case _: Definition => () // abstract type
156160
case _: NonDefinitionValues => () // We only validate definitions
@@ -465,7 +469,6 @@ case class ValidationPass(
465469
s.loc
466470
)
467471
}
468-
checkMetadata(s)
469472
}
470473

471474
private def validateFunction(
@@ -480,6 +483,12 @@ case class ValidationPass(
480483
MissingWarning,
481484
f.errorLoc
482485
)
486+
f.input.foreach { agg =>
487+
checkTypeExpression(agg, f, parents)
488+
}
489+
f.output.foreach { agg =>
490+
checkTypeExpression(agg, f, parents)
491+
}
483492
checkMetadata(f)
484493
}
485494

@@ -503,6 +512,62 @@ case class ValidationPass(
503512
check(bi.path.s.endsWith(".bast"), s"BAST load path '${bi.path.s}' should end with .bast", Messages.Warning, bi.loc)
504513
}
505514

515+
private def validateSchema(
516+
schema: Schema,
517+
parents: Parents
518+
): Unit = {
519+
checkIdentifierLength(schema)
520+
checkMetadata(schema)
521+
checkNonEmpty(
522+
schema.data.toSeq,
523+
"data definitions",
524+
schema,
525+
schema.errorLoc,
526+
MissingWarning,
527+
required = true
528+
)
529+
schema.schemaKind match {
530+
case RepositorySchemaKind.Flat | RepositorySchemaKind.Document |
531+
RepositorySchemaKind.Columnar | RepositorySchemaKind.Vector =>
532+
if schema.links.nonEmpty then
533+
messages.addWarning(
534+
schema.errorLoc,
535+
s"${schema.identify} is ${schema.schemaKind} and should not define links"
536+
)
537+
case RepositorySchemaKind.Graphical =>
538+
if schema.links.isEmpty && schema.data.nonEmpty then
539+
messages.addWarning(
540+
schema.errorLoc,
541+
s"${schema.identify} is graphical but has no links (edges)"
542+
)
543+
case _ => ()
544+
}
545+
if schema.schemaKind == RepositorySchemaKind.Vector && schema.data.size > 1 then
546+
messages.addWarning(
547+
schema.errorLoc,
548+
s"${schema.identify} is a vector schema but defines ${schema.data.size} data nodes; typically only one is expected"
549+
)
550+
schema.data.values.foreach { typeRef =>
551+
checkRef[Type](typeRef, parents)
552+
}
553+
schema.links.values.foreach { case (from, to) =>
554+
checkRef[Field](from, parents)
555+
checkRef[Field](to, parents)
556+
}
557+
schema.indices.foreach { fieldRef =>
558+
checkRef[Field](fieldRef, parents)
559+
}
560+
}
561+
562+
private def validateRelationship(
563+
relationship: Relationship,
564+
parents: Parents
565+
): Unit = {
566+
checkIdentifierLength(relationship)
567+
checkRef[Processor[?]](relationship.withProcessor, parents)
568+
checkMetadata(relationship)
569+
}
570+
506571
private def validateEntity(
507572
entity: Entity,
508573
parents: Parents
@@ -573,6 +638,9 @@ case class ValidationPass(
573638
Messages.Error,
574639
projector.errorLoc
575640
)
641+
projector.repositories.foreach { repoRef =>
642+
checkRef[Repository](repoRef, parents)
643+
}
576644
checkMetadata(projector)
577645
}
578646

@@ -590,6 +658,11 @@ case class ValidationPass(
590658
MissingWarning,
591659
required = false
592660
)
661+
if repository.handlers.isEmpty && repository.nonEmpty then
662+
messages.addMissing(
663+
repository.errorLoc,
664+
s"${repository.identify} should have at least one handler"
665+
)
593666
}
594667

595668
private def validateAdaptor(
@@ -607,6 +680,16 @@ case class ValidationPass(
607680
messages.addError(adaptor.errorLoc, message)
608681
}
609682
}
683+
if adaptor.handlers.isEmpty && adaptor.nonEmpty then
684+
messages.addMissing(
685+
adaptor.errorLoc,
686+
s"${adaptor.identify} should have at least one handler"
687+
)
688+
else if adaptor.handlers.nonEmpty && adaptor.handlers.forall(_.clauses.isEmpty) then
689+
messages.addMissing(
690+
adaptor.errorLoc,
691+
s"${adaptor.identify} has only empty handlers"
692+
)
610693
checkMetadata(adaptor)
611694
case None | Some(_) =>
612695
messages.addError(adaptor.errorLoc, "Adaptor not contained within Context")
@@ -619,6 +702,33 @@ case class ValidationPass(
619702
): Unit = {
620703
addStreamlet(streamlet)
621704
checkContainer(parents, streamlet)
705+
if streamlet.nonEmpty then
706+
val numInlets = streamlet.inlets.size
707+
val numOutlets = streamlet.outlets.size
708+
streamlet.shape match {
709+
case _: Source =>
710+
check(numInlets == 0, s"${streamlet.identify} is a source but has $numInlets inlets; sources must have none", Messages.Error, streamlet.errorLoc)
711+
check(numOutlets >= 1, s"${streamlet.identify} is a source but has no outlets; sources must have at least one", Messages.Error, streamlet.errorLoc)
712+
case _: Sink =>
713+
check(numInlets >= 1, s"${streamlet.identify} is a sink but has no inlets; sinks must have at least one", Messages.Error, streamlet.errorLoc)
714+
check(numOutlets == 0, s"${streamlet.identify} is a sink but has $numOutlets outlets; sinks must have none", Messages.Error, streamlet.errorLoc)
715+
case _: Flow =>
716+
check(numInlets >= 1, s"${streamlet.identify} is a flow but has no inlets; flows must have at least one", Messages.Error, streamlet.errorLoc)
717+
check(numOutlets >= 1, s"${streamlet.identify} is a flow but has no outlets; flows must have at least one", Messages.Error, streamlet.errorLoc)
718+
case _: Merge =>
719+
check(numInlets >= 2, s"${streamlet.identify} is a merge but has $numInlets inlets; merges must have at least two", Messages.Error, streamlet.errorLoc)
720+
check(numOutlets >= 1, s"${streamlet.identify} is a merge but has no outlets; merges must have at least one", Messages.Error, streamlet.errorLoc)
721+
case _: Split =>
722+
check(numInlets >= 1, s"${streamlet.identify} is a split but has no inlets; splits must have at least one", Messages.Error, streamlet.errorLoc)
723+
check(numOutlets >= 2, s"${streamlet.identify} is a split but has $numOutlets outlets; splits must have at least two", Messages.Error, streamlet.errorLoc)
724+
case _: Router =>
725+
check(numInlets >= 2, s"${streamlet.identify} is a router but has $numInlets inlets; routers must have at least two", Messages.Error, streamlet.errorLoc)
726+
check(numOutlets >= 2, s"${streamlet.identify} is a router but has $numOutlets outlets; routers must have at least two", Messages.Error, streamlet.errorLoc)
727+
case _: Void => ()
728+
}
729+
end if
730+
if streamlet.handlers.isEmpty && streamlet.nonEmpty then
731+
messages.addMissing(streamlet.errorLoc, s"${streamlet.identify} should have a handler")
622732
checkMetadata(streamlet)
623733
}
624734

@@ -662,10 +772,10 @@ case class ValidationPass(
662772
): Unit = {
663773
checkDefinition(parents, s)
664774
checkNonEmpty(s.doStatements.toSeq, "Do Statements", s, MissingWarning)
665-
checkNonEmpty(s.doStatements.toSeq, "Revert Statements", s, MissingWarning)
775+
checkNonEmpty(s.undoStatements.toSeq, "Revert Statements", s, MissingWarning)
666776
check(
667-
s.doStatements.getClass == s.undoStatements.getClass,
668-
"The primary action and revert action must be the same shape",
777+
s.doStatements.nonEmpty == s.undoStatements.nonEmpty,
778+
"A saga step with do statements must also have revert statements, and vice versa",
669779
Messages.Error,
670780
s.errorLoc
671781
)
@@ -685,9 +795,10 @@ case class ValidationPass(
685795
parents: Parents
686796
): Unit = {
687797
checkContainer(parents, epic)
688-
if epic.userStory.isEmpty then {
798+
if epic.userStory.isEmpty then
689799
messages.addMissing(epic.errorLoc, s"${epic.identify} is missing a user story")
690-
}
800+
else
801+
checkRef[User](epic.userStory.user, parents)
691802
checkMetadata(epic)
692803
}
693804

@@ -750,6 +861,8 @@ case class ValidationPass(
750861
parents: Parents
751862
): Unit = {
752863
checkDefinition(parents, uc)
864+
if uc.userStory.nonEmpty then
865+
checkRef[User](uc.userStory.user, parents)
753866
if uc.contents.nonEmpty then {
754867
uc.contents.foreach {
755868
case seq: SequentialInteractions =>

0 commit comments

Comments
 (0)