From 5393842d806ccf49dc43973602315f37cb44327a Mon Sep 17 00:00:00 2001 From: Reid Spencer Date: Mon, 2 Oct 2023 22:53:12 +0200 Subject: [PATCH] Allow groups to be the target of a from/to epic step (#449) Also: * Don't allow type definitions in inputs, outputs or groups * adjust syntax so body of input or output don't require { and } * Fix test cases for new input and output syntax * Add a test case to the ReportedIssues test suite to prevent regression Signed-off-by: reidspencer --- .../test/input/regressions/match-error.riddl | 10 ++-- .../com/reactific/riddl/language/AST.scala | 60 +++++++------------ .../language/parsing/ApplicationParser.scala | 47 +++++++++------ .../language/parsing/ReferenceParser.scala | 13 ++-- .../passes/validate/ApplicationTest.scala | 10 ++-- .../riddl/passes/validate/EpicTest.scala | 37 +++--------- testkit/src/test/input/issues/445.riddl | 21 +++++++ .../riddl/testkit/ReportedIssuesTest.scala | 8 +++ 8 files changed, 105 insertions(+), 101 deletions(-) create mode 100644 testkit/src/test/input/issues/445.riddl diff --git a/hugo/src/test/input/regressions/match-error.riddl b/hugo/src/test/input/regressions/match-error.riddl index caac022fd..b5037d563 100644 --- a/hugo/src/test/input/regressions/match-error.riddl +++ b/hugo/src/test/input/regressions/match-error.riddl @@ -15,12 +15,10 @@ domain PersonalBanking is { result Title is { value: String } command Name is { value: String } group HomePage is { - output AccountDetailsPage is { - presents result Title - } described as "Show a blank page with a title" - input Two is { - acquires command Name - } described as "Show a blank page with a title, collect a Name" + output AccountDetailsPage presents result Title + described as "Show a blank page with a title" + input Two acquires command Name + described as "Show a blank page with a title, collect a Name" } } described as "A very simple app for testing" } briefly "The user interface for the Personal Banking Application" diff --git a/language/src/main/scala/com/reactific/riddl/language/AST.scala b/language/src/main/scala/com/reactific/riddl/language/AST.scala index 6a20595f2..581d38177 100644 --- a/language/src/main/scala/com/reactific/riddl/language/AST.scala +++ b/language/src/main/scala/com/reactific/riddl/language/AST.scala @@ -806,7 +806,7 @@ object AST { // extends ast.AbstractDefinitions with ast.Definitions with ast.Op super.isAssignmentCompatible(other) || { other match case _: Strng => true - case _ => false + case _ => false } } } @@ -1522,20 +1522,19 @@ object AST { // extends ast.AbstractDefinitions with ast.Definitions with ast.Op */ sealed trait AdaptorDefinition extends Definition - /** Base trait of any definition that is in the content of an Application - */ + /** Base trait of any definition that is in the content of an Application */ sealed trait ApplicationDefinition extends Definition - /** Base trait of any definition that is in the content of a context - */ + /** Base trait of any definition that is in the content of an Application's group */ + sealed trait GroupDefinition extends Definition + + /** Base trait of any definition that is in the content of a context */ sealed trait ContextDefinition extends Definition - /** Base trait of any definition that is in the content of a domain - */ + /** Base trait of any definition that is in the content of a domain */ sealed trait DomainDefinition extends Definition - /** Base trait of any definition that is in the content of an entity. - */ + /** Base trait of any definition that is in the content of an entity. */ sealed trait EntityDefinition extends Definition /** Base trait of definitions that are part of a Handler Definition */ @@ -3305,19 +3304,14 @@ object AST { // extends ast.AbstractDefinitions with ast.Definitions with ast.Op def format: String = s"${Keywords.epic} ${pathId.format}" } - /** Sealed trait for all UI elements that derive from it - */ - sealed trait UIElement extends ApplicationDefinition - - /** A group of UIElement that can be treated as a whole. For example, a form, a button group, etc. + /** A group of GroupDefinition that can be treated as a whole. For example, + * a form, a button group, etc. * @param loc * The location of the group * @param id * The unique identifier of the group - * @param types - * Type definitions to define types shared by more than one UIElement * @param elements - * The list of UIElements + * The list of GroupDefinition * @param brief * A brief description of the group * @param description @@ -3326,19 +3320,17 @@ object AST { // extends ast.AbstractDefinitions with ast.Definitions with ast.Op case class Group( loc: At, id: Identifier, - types: Seq[Type] = Seq.empty[Type], - elements: Seq[UIElement] = Seq.empty[UIElement], + alias: String, + elements: Seq[GroupDefinition] = Seq.empty[GroupDefinition], brief: Option[LiteralString] = None, description: Option[Description] = None - ) extends UIElement { + ) extends ApplicationDefinition with GroupDefinition { override def kind: String = "Group" - override lazy val contents: Seq[ApplicationDefinition] = { - types ++ elements - } + override lazy val contents: Seq[GroupDefinition] = { elements } /** Format the node to a string */ - override def format: String = "" + override def format: String = "group $id" } /** A Reference to a Group @@ -3357,8 +3349,6 @@ object AST { // extends ast.AbstractDefinitions with ast.Definitions with ast.Op * Location of the view in the source * @param id * unique identifier oof the view - * @param types - * any type definitions the view needs * @param putOut * A result reference for the data too be presented * @param brief @@ -3369,17 +3359,15 @@ object AST { // extends ast.AbstractDefinitions with ast.Definitions with ast.Op case class Output( loc: At, id: Identifier, - types: Seq[Type], + alias: String, putOut: MessageRef, brief: Option[LiteralString] = None, description: Option[Description] = None - ) extends UIElement { + ) extends LeafDefinition with GroupDefinition { override def kind: String = "Output" - override lazy val contents: Seq[ApplicationDefinition] = types - /** Format the node to a string */ - override def format: String = "" + override def format: String = s"${if id.isEmpty then "inoutputput" else id.format} presents ${putOut.format}" } /** A reference to an View using a path identifier @@ -3399,8 +3387,6 @@ object AST { // extends ast.AbstractDefinitions with ast.Definitions with ast.Op * Location of the Give * @param id * Name of the give - * @param types - * type definitions needed for the Give * @param putIn * a Type reference of the type given by the user * @param brief @@ -3411,17 +3397,15 @@ object AST { // extends ast.AbstractDefinitions with ast.Definitions with ast.Op case class Input( loc: At, id: Identifier, - types: Seq[Type], + alias: String, putIn: MessageRef, brief: Option[LiteralString] = None, description: Option[Description] = None - ) extends UIElement { + ) extends LeafDefinition with GroupDefinition { override def kind: String = "Input" - override lazy val contents: Seq[Definition] = types - /** Format the node to a string */ - override def format: String = "" + override def format: String = s"${if id.isEmpty then "input" else id.format} acquires ${putIn.format}" } /** A reference to an Input using a path identifier diff --git a/language/src/main/scala/com/reactific/riddl/language/parsing/ApplicationParser.scala b/language/src/main/scala/com/reactific/riddl/language/parsing/ApplicationParser.scala index 47ac743f9..827d66e80 100644 --- a/language/src/main/scala/com/reactific/riddl/language/parsing/ApplicationParser.scala +++ b/language/src/main/scala/com/reactific/riddl/language/parsing/ApplicationParser.scala @@ -25,53 +25,62 @@ private[parsing] trait ApplicationParser { } } - private def groupAliases[u: P]: P[Unit] = { + private def groupAliases[u: P]: P[String] = { P( - StringIn(Keywords.group, "page", "pange", "dialog", "popup", "frame", "column", "window", "section", "tab") + StringIn(Keywords.group, "page", "pane", "dialog", "popup", "frame", "column", "window", "section", "tab").! ) } + private def groupDefinitions[u:P]: P[Seq[GroupDefinition]] = { + P { + undefined(Seq.empty[GroupDefinition]) | (group | appOutput | appInput).rep(0) + } + } + private def group[u: P]: P[Group] = { P( - location ~ groupAliases ~/ identifier ~ is ~ open ~ types ~ - (group | appOutput | appInput).rep(0) ~ close ~ briefly ~ description - ).map { case (loc, id, types, elements, brief, description) => - Group(loc, id, types, elements, brief, description) + location ~ groupAliases ~ identifier ~/ is ~ open ~ + groupDefinitions ~ + close ~ briefly ~ description + ).map { case (loc, alias, id, elements, brief, description) => + Group(loc, id, alias, elements, brief, description) } } - private def outputAliases[u: P]: P[Unit] = { + private def outputAliases[u: P]: P[String] = { P( StringIn( Keywords.output, "document", "list", - "table" - ) + "table", + "graph", + "animation", + "picture" + ).! ) } private def appOutput[u: P]: P[Output] = { P( - location ~ outputAliases ~/ identifier ~ is ~ open ~ types ~ - Keywords.presents ~/ messageRef ~ close ~ briefly ~ description - ).map { case (loc, id, types, result, brief, description) => - Output(loc, id, types, result, brief, description) + location ~ outputAliases ~/ identifier ~ Keywords.presents ~/ + messageRef ~ briefly ~ description + ).map { case (loc, alias, id, putOut, brief, description) => + Output(loc, id, alias, putOut, brief, description) } } - private def inputAliases[u: P]: P[Unit] = { + private def inputAliases[u: P]: P[String] = { P( - StringIn(Keywords.input, "form", "textblock", "button", "picklist") + StringIn(Keywords.input, "form", "textblock", "button", "picklist").! ) } private def appInput[u: P]: P[Input] = { P( - location ~ inputAliases ~/ identifier ~ is ~ open ~ types ~ - Keywords.acquires ~ messageRef ~ close ~ briefly ~ description - ).map { case (loc, id, types, yields, brief, description) => - Input(loc, id, types, yields, brief, description) + location ~ inputAliases ~/ identifier ~ is ~ Keywords.acquires ~ messageRef ~ briefly ~ description + ).map { case (loc, alias, id, putIn, brief, description) => + Input(loc, id, alias, putIn, brief, description) } } diff --git a/language/src/main/scala/com/reactific/riddl/language/parsing/ReferenceParser.scala b/language/src/main/scala/com/reactific/riddl/language/parsing/ReferenceParser.scala index 44d07e543..1ce068dbc 100644 --- a/language/src/main/scala/com/reactific/riddl/language/parsing/ReferenceParser.scala +++ b/language/src/main/scala/com/reactific/riddl/language/parsing/ReferenceParser.scala @@ -165,6 +165,11 @@ private[parsing] trait ReferenceParser extends CommonParser { .map(tpl => (InputRef.apply _).tupled(tpl)) } + def groupRef[u:P]: P[GroupRef] = { + P(location ~ Keywords.group ~ pathIdentifier) + .map(tpl => (GroupRef.apply _).tupled(tpl)) + } + def authorRefs[u: P]: P[Seq[AuthorRef]] = { P(location ~ by ~ Keywords.author ~ pathIdentifier) .map(tpl => (AuthorRef.apply _).tupled(tpl)) @@ -174,16 +179,16 @@ private[parsing] trait ReferenceParser extends CommonParser { def processorRef[u: P]: P[ProcessorRef[Processor[?, ?]]] = { P( adaptorRef | applicationRef | contextRef | entityRef | projectorRef | - repositoryRef | streamletRef + repositoryRef | streamletRef ) } - + def arbitraryInteractionRef[u:P]: P[Reference[Definition]] = { - P( processorRef | sagaRef | inputRef | outputRef ) + P( processorRef | sagaRef | inputRef | outputRef | groupRef ) } def anyInteractionRef[u: P]: P[Reference[Definition]] = { - arbitraryInteractionRef | userRef + arbitraryInteractionRef | userRef } } diff --git a/passes/src/test/scala/com/reactific/riddl/passes/validate/ApplicationTest.scala b/passes/src/test/scala/com/reactific/riddl/passes/validate/ApplicationTest.scala index 38c86d0c8..4c533ad74 100644 --- a/passes/src/test/scala/com/reactific/riddl/passes/validate/ApplicationTest.scala +++ b/passes/src/test/scala/com/reactific/riddl/passes/validate/ApplicationTest.scala @@ -21,12 +21,10 @@ class ApplicationTest extends ValidatingTest { | result Title { content: String } | command Name { content: String } | group Together is { - | output One is { - | presents result Title - | } described as "Show a blank page with title" - | input Two is { - | acquires command Name - | } described as "yield a Name" + | output One presents result Title + | described as "Show a blank page with title" + | input Two acquires command Name + | described as "yield a Name" | } described as "Show a title, collect a Name" | } described as "A very simple app just for testing" |} described as "Just a parsing convenience" diff --git a/passes/src/test/scala/com/reactific/riddl/passes/validate/EpicTest.scala b/passes/src/test/scala/com/reactific/riddl/passes/validate/EpicTest.scala index 569dc435d..eadcf86f5 100644 --- a/passes/src/test/scala/com/reactific/riddl/passes/validate/EpicTest.scala +++ b/passes/src/test/scala/com/reactific/riddl/passes/validate/EpicTest.scala @@ -77,14 +77,8 @@ class EpicTest extends ValidatingTest { | |application Improving_app is { | group OrganizationPage is { - | input accept is { - | acquires command - | ImprovingApp.OrganizationContext.CreateOrganization - | } - | output show is { - | presents result - | ImprovingApp.OrganizationContext.OrganizationInfo - | } + | input accept acquires command ImprovingApp.OrganizationContext.CreateOrganization + | output show presents result ImprovingApp.OrganizationContext.OrganizationInfo | } |} | @@ -119,12 +113,11 @@ class EpicTest extends ValidatingTest { parseAndValidateDomain(rpi, shouldFailOnErrors = false) { case (domain: Domain, _: RiddlParserInput, msgs: Messages.Messages) => val errors = msgs.justErrors - info ( errors.format ) + info(errors.format) if errors.isEmpty then domain mustNot be(empty) domain.epics mustNot be(empty) - else - fail("Shouldn't be any errors") + else fail(errors.format) } } "handle parallel group" in { @@ -152,14 +145,8 @@ class EpicTest extends ValidatingTest { | |application Improving_app is { | group OrganizationPage is { - | input accept is { - | acquires command - | ImprovingApp.OrganizationContext.CreateOrganization - | } - | output show is { - | presents result - | ImprovingApp.OrganizationContext.OrganizationInfo - | } + | input accept acquires command ImprovingApp.OrganizationContext.CreateOrganization + | output show presents result ImprovingApp.OrganizationContext.OrganizationInfo | } |} | @@ -227,14 +214,8 @@ class EpicTest extends ValidatingTest { | |application Improving_app is { | group OrganizationPage is { - | input accept is { - | acquires command - | ImprovingApp.OrganizationContext.CreateOrganization - | } - | output show is { - | presents result - | ImprovingApp.OrganizationContext.OrganizationInfo - | } + | input accept acquires command ImprovingApp.OrganizationContext.CreateOrganization + | output show presents result ImprovingApp.OrganizationContext.OrganizationInfo | } |} | @@ -270,7 +251,7 @@ class EpicTest extends ValidatingTest { else domain mustNot be(empty) domain.epics mustNot be(empty) - if msgs.nonEmpty then { } + if msgs.nonEmpty then {} msgs.hasErrors mustBe false succeed } diff --git a/testkit/src/test/input/issues/445.riddl b/testkit/src/test/input/issues/445.riddl new file mode 100644 index 000000000..88e471c98 --- /dev/null +++ b/testkit/src/test/input/issues/445.riddl @@ -0,0 +1,21 @@ +domain example is { + user Admin is "fickle" + record Activate { name: String } + application App is { + group NewTenantPage { + group TenantInfoPage { + input NextButton acquires record Activate + } + group InfoList { + ??? + } + } + } + epic allow_groups_to_receive { + case UserCreatesANewCompany { + user Admin wants to "create a new tenant" so that "the tenant can do stuff" + step from user Admin "presses" to application App, + step from input App.NewTenantPage.TenantInfoPage.NextButton "routes user" to group App.NewTenantPage.InfoList + } + } +} diff --git a/testkit/src/test/scala/com/reactific/riddl/testkit/ReportedIssuesTest.scala b/testkit/src/test/scala/com/reactific/riddl/testkit/ReportedIssuesTest.scala index 500c57344..d4f39ab21 100644 --- a/testkit/src/test/scala/com/reactific/riddl/testkit/ReportedIssuesTest.scala +++ b/testkit/src/test/scala/com/reactific/riddl/testkit/ReportedIssuesTest.scala @@ -88,5 +88,13 @@ class ReportedIssuesTest extends ValidatingTest { succeed } } + "445" in { + checkOne("445.riddl") { + case Left(messages) => + fail(messages.format) + case Right(result) => + succeed + } + } } }