From 73880165ba6b111e9cf9c2c6fab4dfb26f52df58 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

- update code to reuse logic for unordered sequences check
- add multipleChildElementsWithSameName warning
- add test to check we get UPA error with choices
- add tests to exercise new warning

DAFFODIL-2736
---
 .../daffodil/core/dsom/SequenceGroup.scala    | 45 +++++++++----
 .../org/apache/daffodil/xsd/dafext.xsd        |  1 +
 .../sequence_groups/SequenceGroup.tdml        | 42 ++++++++++++
 .../section15/choice_groups/choice2736.tdml   | 66 +++++++++++++++++++
 .../sequence_groups/TestSequenceGroups.scala  |  5 ++
 .../section15/choice_groups/TestChoice2.scala |  6 ++
 6 files changed, 153 insertions(+), 12 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/SequenceGroup.scala b/daffodil-core/src/main/scala/org/apache/daffodil/core/dsom/SequenceGroup.scala
index 5defef394c..b1213d7416 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,19 +216,39 @@ 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
+  private lazy val checkUnorderedSequenceMembersHaveUniqueNamesInNamespaces: Unit = {
+    val nonUniqueNameChildren =
+      checkMembersHaveUniqueNamesInNamespaces.filter(_._1 == true).values
+    nonUniqueNameChildren.foreach { children =>
+      children.head.SDE(
+        "Two or more members of an unordered sequence have the same name and the same namespace"
+      )
     }
-    childrenGroupedByQName.foreach { case (qname, children) =>
-      if (children.length > 1) {
-        children.head.SDE(
-          "Two or more members of an unordered sequence have the same name and the same namespace"
-        )
+  }
+
+  private lazy val checkMembersHaveUniqueNamesInNamespaces: Map[Boolean, Seq[Term]] = {
+    val childrenGroupedByQName = groupMembers
+      .filter { m => m.isInstanceOf[LocalElementDecl] || m.isInstanceOf[ElementRef] }
+      .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.map { case (qname, children) =>
+      (children.length > 1, children)
+    }
+  }
+
+  private lazy val checkIfMultipleChildrenWithSameName: Unit = {
+    val nonUniqueNameChildren =
+      checkMembersHaveUniqueNamesInNamespaces.filter(_._1 == true).values
+    nonUniqueNameChildren.foreach { children =>
+      children.head.SDW(
+        WarnID.MultipleChildElementsWithSameName,
+        "Two or more members of the sequence have the same name and namespace: %s",
+        children.head.asInstanceOf[ElementBase].namedQName
+      )
     }
   }
 
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 3d89c56617..24b90871ed 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
@@ -720,6 +720,7 @@
           <xs:enumeration value="invalidAnnotationPoint" />
           <xs:enumeration value="layerCompileWarning" />
           <xs:enumeration value="multipleChoiceBranches" />
+          <xs:enumeration value="multipleChildElementsWithSameName" />
           <xs:enumeration value="namespaceDifferencesOnly" />
           <xs:enumeration value="noEmptyDefault" />
           <xs:enumeration value="nonExpressionPropertyValueLooksLikeExpression" />
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 b4b8d7552c..2770b64f8f 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
@@ -1536,4 +1536,46 @@
     </tdml:infoset>
   </tdml:parserTestCase>
 
+  <tdml:defineSchema name="multipleElemSameName" elementFormDefault="unqualified">
+
+    <xs:include schemaLocation="/org/apache/daffodil/xsd/DFDLGeneralFormat.dfdl.xsd"/>
+    <dfdl:format ref="ex:GeneralFormat" lengthKind="explicit" occursCountKind="implicit"/>
+
+    <xs:element name="multipleElemSameName" dfdl:lengthKind="implicit">
+      <xs:complexType>
+        <xs:sequence>
+          <xs:element name="A" type="xs:string" dfdl:length="1" />
+          <xs:element name="AmbigElt" type="xs:string" dfdl:length="1" />
+          <xs:element name="B" type="xs:string" dfdl:length="1" />
+          <xs:element name="AmbigElt" type="xs:string" dfdl:length="1" />
+        </xs:sequence>
+      </xs:complexType>
+    </xs:element>
+
+  </tdml:defineSchema>
+
+
+  <tdml:parserTestCase name="multipleElemSameName" root="multipleElemSameName" model="multipleElemSameName" description="TDML runner verifies warnings and infoset.">
+
+    <tdml:document><![CDATA[1234]]></tdml:document>
+
+    <tdml:infoset>
+      <tdml:dfdlInfoset>
+        <multipleElemSameName>
+          <A>1</A>
+          <AmbigElt>2</AmbigElt>
+          <B>3</B>
+          <AmbigElt>4</AmbigElt>
+        </multipleElemSameName>
+      </tdml:dfdlInfoset>
+    </tdml:infoset>
+
+    <tdml:warnings>
+      <tdml:warning>Schema Definition Warning</tdml:warning>
+      <tdml:warning>same name and namespace</tdml:warning>
+      <tdml:warning>AmbigElt</tdml:warning>
+    </tdml:warnings>
+  </tdml:parserTestCase>
+
+
 </tdml:testSuite>
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 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+  Licensed to the Apache Software Foundation (ASF) under one or more
+  contributor license agreements.  See the NOTICE file distributed with
+  this work for additional information regarding copyright ownership.
+  The ASF licenses this file to You under the Apache License, Version 2.0
+  (the "License"); you may not use this file except in compliance with
+  the License.  You may obtain a copy of the License at
+
+      http://www.apache.org/licenses/LICENSE-2.0
+
+  Unless required by applicable law or agreed to in writing, software
+  distributed under the License is distributed on an "AS IS" BASIS,
+  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+  See the License for the specific language governing permissions and
+  limitations under the License.
+-->
+
+<tdml:testSuite 
+  suiteName="choice1765" 
+  description="Tests for choice construct. Bug DFDL-1765." 
+  xmlns:tdml="http://www.ibm.com/xmlns/dfdl/testData"
+  xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" 
+  xmlns:dfdl="http://www.ogf.org/dfdl/dfdl-1.0/" 
+  xmlns:xs="http://www.w3.org/2001/XMLSchema"
+  xmlns:ex="http://example.com" 
+  xmlns:fn="http://www.w3.org/2005/xpath-functions">
+
+  <tdml:defineSchema name="ambiguousChoice" elementFormDefault="unqualified">
+
+    <xs:include schemaLocation="/org/apache/daffodil/xsd/DFDLGeneralFormat.dfdl.xsd"/>
+    <dfdl:format ref="ex:GeneralFormat" lengthKind="explicit" occursCountKind="implicit"/>
+
+
+    <xs:element name="ambiguousChoice" dfdl:lengthKind="implicit">
+      <xs:complexType>
+        <xs:sequence>
+          <xs:element name="key" type="xs:string" dfdl:length="1"/>
+          <xs:choice dfdl:choiceDispatchKey="{key}">
+            <xs:element name="A" type="xs:string" dfdl:length="1" dfdl:choiceBranchKey="A" />
+            <xs:element name="AmbigElt" type="xs:string" dfdl:length="1" dfdl:choiceBranchKey="B" />
+            <xs:element name="B" type="xs:string" dfdl:length="1" dfdl:choiceBranchKey="C" />
+            <xs:element name="AmbigElt" type="xs:string" dfdl:length="1" dfdl:choiceBranchKey="D" />
+          </xs:choice>
+        </xs:sequence>
+      </xs:complexType>
+    </xs:element>
+
+
+  </tdml:defineSchema>
+
+  <tdml:parserTestCase name="choiceAmbiguousUPA" root="ambiguousChoice" model="ambiguousChoice"
+                       description="verify UPA error in choice"
+                       ignoreUnexpectedWarnings="false">
+
+    <tdml:document><![CDATA[B2]]></tdml:document>
+
+    <tdml:errors>
+      <tdml:error>Schema Definition Error</tdml:error>
+      <tdml:error>AmbigElt</tdml:error>
+      <tdml:error>Unique Particle Attribution</tdml:error>
+    </tdml:errors>
+  </tdml:parserTestCase>
+  
+
+</tdml:testSuite>
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 819d0fc2ee..70db3a2d1e 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
@@ -144,4 +144,9 @@ 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")
+  }
 }
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")
+  }
 }