From fd941cb5467f30cea8417bda63574c6bfb90a47c Mon Sep 17 00:00:00 2001 From: olabusayoT <50379531+olabusayoT@users.noreply.github.com> Date: Thu, 26 Sep 2024 12:04:59 -0400 Subject: [PATCH] Warn about Multiple Child Elements with Same Name - currently when we have multiple child elements in a sequence with the same name, daffodil allows it, while choice branches return a UPA error and unordered sequences return an SDE. We want to generate a warning when this happens in an ordered sequence so we refactor code to warn if the local names are the same. This has the effect of warning whether namespaces are supported or unsupported - we group using elementChildren instead of groupMembers since we want to consider all children since regardless of the model groups, their immediate children end up as siblings in the infoset and can cause potential ambiguities for some infoset types - add test to check we get UPA error with choices - add tests to exercise new warning (same/different namespaces, but same local name) DAFFODIL-2736 --- .../daffodil/core/dsom/ModelGroup.scala | 16 +++ .../daffodil/core/dsom/SequenceGroup.scala | 37 +++-- .../org/apache/daffodil/xsd/dafext.xsd | 1 + .../sequence_groups/SequenceGroup.tdml | 130 ++++++++++++++++++ .../section15/choice_groups/choice2736.tdml | 66 +++++++++ .../sequence_groups/TestSequenceGroups.scala | 8 ++ .../section15/choice_groups/TestChoice2.scala | 6 + 7 files changed, 253 insertions(+), 11 deletions(-) create mode 100644 daffodil-test/src/test/resources/org/apache/daffodil/section15/choice_groups/choice2736.tdml diff --git a/daffodil-core/src/main/scala/org/apache/daffodil/core/dsom/ModelGroup.scala b/daffodil-core/src/main/scala/org/apache/daffodil/core/dsom/ModelGroup.scala index 2e4db8b3f4..b97efb1e5c 100644 --- a/daffodil-core/src/main/scala/org/apache/daffodil/core/dsom/ModelGroup.scala +++ b/daffodil-core/src/main/scala/org/apache/daffodil/core/dsom/ModelGroup.scala @@ -341,6 +341,22 @@ abstract class ModelGroup protected (index: Int) } } + final lazy val elementChildrenInNonHiddenContext: Seq[ElementBase] = + LV('elementChildrenInNonHiddenContext) { + val ebs = groupMembers + .filter { + case gr: GroupRef => !gr.isHidden + case _ => true + } + .flatMap { gm => + gm match { + case eb: ElementBase => Seq(eb) + case gb: ModelGroup => gb.elementChildren + } + } + ebs + }.value + final lazy val elementChildren: Seq[ElementBase] = LV('elementChildren) { val gms = groupMembers val echls = gms.flatMap { gm => diff --git a/daffodil-core/src/main/scala/org/apache/daffodil/core/dsom/SequenceGroup.scala b/daffodil-core/src/main/scala/org/apache/daffodil/core/dsom/SequenceGroup.scala index 5defef394c..68978d5bf6 100644 --- a/daffodil-core/src/main/scala/org/apache/daffodil/core/dsom/SequenceGroup.scala +++ b/daffodil-core/src/main/scala/org/apache/daffodil/core/dsom/SequenceGroup.scala @@ -103,6 +103,7 @@ abstract class SequenceGroupTermBase(xml: Node, lexicalParent: SchemaComponent, requiredEvaluationsIfActivated(checkIfValidUnorderedSequence) requiredEvaluationsIfActivated(checkIfNonEmptyAndDiscrimsOrAsserts) + requiredEvaluationsIfActivated(checkIfMultipleChildrenWithSameName) protected def apparentXMLChildren: Seq[Node] @@ -181,7 +182,7 @@ abstract class SequenceGroupTermBase(xml: Node, lexicalParent: SchemaComponent, if (!isOrdered) { checkMembersAreAllElementOrElementRef checkMembersHaveValidOccursCountKind - checkMembersHaveUniqueNamesInNamespaces + checkUnorderedSequenceMembersHaveUniqueNamesInNamespaces } } @@ -215,20 +216,34 @@ abstract class SequenceGroupTermBase(xml: Node, lexicalParent: SchemaComponent, } } - private lazy val checkMembersHaveUniqueNamesInNamespaces: Unit = { - val childrenGroupedByQName = groupMembers.groupBy { gm => - // previous checks should ensure that all group members are either local - // elements or element references - Assert.invariant(gm.isInstanceOf[ElementBase]) - gm.asInstanceOf[ElementBase].namedQName - } - childrenGroupedByQName.foreach { case (qname, children) => - if (children.length > 1) { + private lazy val checkUnorderedSequenceMembersHaveUniqueNamesInNamespaces: Unit = { + // previous checks should ensure that all element children for unordered sequences are either local + // elements or element references + elementChildrenInNonHiddenContext + .groupBy(_.namedQName) + .filter(_._2.length > 1) + .values + .foreach { children => children.head.SDE( "Two or more members of an unordered sequence have the same name and the same namespace" ) } - } + } + + private lazy val checkIfMultipleChildrenWithSameName: Unit = { + elementChildrenInNonHiddenContext + .groupBy(_.name) + .filter(_._2.length > 1) + .values + .foreach { children => + children.head.SDW( + WarnID.MultipleChildElementsWithSameName, + "Two or more members of the sequence have the same name. " + + "Since not all infosets support namespaces or siblings with the same name, " + + "this could cause issues. QNames are: %s", + children.map(c => c.namedQName).mkString(", ") + ) + } } final lazy val isOrdered: Boolean = this.sequenceKind match { diff --git a/daffodil-propgen/src/main/resources/org/apache/daffodil/xsd/dafext.xsd b/daffodil-propgen/src/main/resources/org/apache/daffodil/xsd/dafext.xsd index 69ea3fdbb5..befb240b68 100644 --- a/daffodil-propgen/src/main/resources/org/apache/daffodil/xsd/dafext.xsd +++ b/daffodil-propgen/src/main/resources/org/apache/daffodil/xsd/dafext.xsd @@ -736,6 +736,7 @@ + diff --git a/daffodil-test/src/test/resources/org/apache/daffodil/section14/sequence_groups/SequenceGroup.tdml b/daffodil-test/src/test/resources/org/apache/daffodil/section14/sequence_groups/SequenceGroup.tdml index 7707d14547..1b809da886 100644 --- a/daffodil-test/src/test/resources/org/apache/daffodil/section14/sequence_groups/SequenceGroup.tdml +++ b/daffodil-test/src/test/resources/org/apache/daffodil/section14/sequence_groups/SequenceGroup.tdml @@ -1237,6 +1237,7 @@ + @@ -1353,6 +1354,12 @@ + + Two or more members + same name + ex:inty + + + + Two or more members + same name + ex:inty + + + + Two or more members + same name + ex:inty + + @@ -1651,4 +1670,115 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + 1 + 2 + 3 + 4 + + + + + + Schema Definition Warning + same name + AmbigElt + + + + + + + + + + + 1 + + + 2 + + + 3 + + + 4 + + + 5 + + + 6 + + + 7 + + + 8 + + + + + + + + Schema Definition Warning + same name + {}e1 + u:e1 + + + + diff --git a/daffodil-test/src/test/resources/org/apache/daffodil/section15/choice_groups/choice2736.tdml b/daffodil-test/src/test/resources/org/apache/daffodil/section15/choice_groups/choice2736.tdml new file mode 100644 index 0000000000..a3e90a6a80 --- /dev/null +++ b/daffodil-test/src/test/resources/org/apache/daffodil/section15/choice_groups/choice2736.tdml @@ -0,0 +1,66 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + Schema Definition Error + AmbigElt + Unique Particle Attribution + + + + + diff --git a/daffodil-test/src/test/scala/org/apache/daffodil/section14/sequence_groups/TestSequenceGroups.scala b/daffodil-test/src/test/scala/org/apache/daffodil/section14/sequence_groups/TestSequenceGroups.scala index 873eb6e7b9..56d47d8056 100644 --- a/daffodil-test/src/test/scala/org/apache/daffodil/section14/sequence_groups/TestSequenceGroups.scala +++ b/daffodil-test/src/test/scala/org/apache/daffodil/section14/sequence_groups/TestSequenceGroups.scala @@ -147,4 +147,12 @@ class TestSequenceGroups { // @Test def test_delimiterScanning_03() { runner_01.runOneTest("delimiterScanning_03") } @Test def test_hiddenGroupIVC(): Unit = { runner_02.runOneTest("hiddenGroupIVC") } + + // DAFFODIL-2736 + @Test def test_multipleElemSameName() = { + runner_02.runOneTest("multipleElemSameName") + } + @Test def test_multipleElemSameNameDifferentNamespaces() = { + runner_02.runOneTest("multipleElemSameNameDifferentNamespaces") + } } diff --git a/daffodil-test/src/test/scala/org/apache/daffodil/section15/choice_groups/TestChoice2.scala b/daffodil-test/src/test/scala/org/apache/daffodil/section15/choice_groups/TestChoice2.scala index 647ddec52a..3cd9025302 100644 --- a/daffodil-test/src/test/scala/org/apache/daffodil/section15/choice_groups/TestChoice2.scala +++ b/daffodil-test/src/test/scala/org/apache/daffodil/section15/choice_groups/TestChoice2.scala @@ -28,11 +28,13 @@ object TestChoice2 { val runner = Runner(testDir, "choice1765.tdml") val runner1773 = Runner(testDir, "choice1773.tdml") val runner2162 = Runner(testDir, "choice2162.tdml") + val runner2736 = Runner(testDir, "choice2736.tdml") @AfterClass def shutDown(): Unit = { runner.reset runner1773.reset runner2162.reset + runner2736.reset } } @@ -63,4 +65,8 @@ class TestChoice2 { runner2162.runOneTest("choiceArrayDirectDispatch1") } + // DAFFODIL-2736 + @Test def test_choiceAmbiguousUPA() = { + runner2736.runOneTest("choiceAmbiguousUPA") + } }