From b007b41ac126c8d14f2cb85e7e24afabfbad27bd Mon Sep 17 00:00:00 2001
From: olabusayoT <50379531+olabusayoT@users.noreply.github.com>
Date: Fri, 27 Sep 2024 14:59:12 -0400
Subject: [PATCH] fixup! Warn about Multiple Child Elements with Same Name

- rename/refactor groupedMembersWithSameName. allow to accept boolean flag to include namespace in grouping or not
- add test for elems with different namespaces but same name; ensure warning triggered

DAFFODIL-2736
---
 .../daffodil/core/dsom/SequenceGroup.scala    | 33 ++++----
 .../sequence_groups/SequenceGroup.tdml        | 75 ++++++++++++++++++-
 .../sequence_groups/TestSequenceGroups.scala  |  3 +
 3 files changed, 94 insertions(+), 17 deletions(-)

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 b1213d7416..10918bf517 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
@@ -217,37 +217,42 @@ abstract class SequenceGroupTermBase(xml: Node, lexicalParent: SchemaComponent,
   }
 
   private lazy val checkUnorderedSequenceMembersHaveUniqueNamesInNamespaces: Unit = {
-    val nonUniqueNameChildren =
-      checkMembersHaveUniqueNamesInNamespaces.filter(_._1 == true).values
-    nonUniqueNameChildren.foreach { children =>
+    groupedMembersWithSameName().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 checkMembersHaveUniqueNamesInNamespaces: Map[Boolean, Seq[Term]] = {
-    val childrenGroupedByQName = groupMembers
+  private def groupedMembersWithSameName(
+    includeNamespace: Boolean = true
+  ): Map[Boolean, Seq[Term]] = {
+    val childrenGroupedByName = 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
+        if (includeNamespace) {
+          gm.asInstanceOf[ElementBase].namedQName
+        } else {
+          gm.asInstanceOf[ElementBase].name
+        }
       }
-    childrenGroupedByQName.map { case (qname, children) =>
-      (children.length > 1, children)
-    }
+    val groupedMembersSameName = childrenGroupedByName
+      .map { case (_, children) =>
+        (children.length > 1, children)
+      }
+      .filter(_._1 == true)
+    groupedMembersSameName
   }
 
   private lazy val checkIfMultipleChildrenWithSameName: Unit = {
-    val nonUniqueNameChildren =
-      checkMembersHaveUniqueNamesInNamespaces.filter(_._1 == true).values
-    nonUniqueNameChildren.foreach { children =>
+    groupedMembersWithSameName(includeNamespace = false).values.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
+        "Two or more members of the sequence have the same name: %s",
+        children.map(c => c.asInstanceOf[ElementBase].namedQName).mkString(", ")
       )
     }
   }
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 2770b64f8f..d26eb14ddb 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,9 +1536,10 @@
     </tdml:infoset>
   </tdml:parserTestCase>
 
-  <tdml:defineSchema name="multipleElemSameName" elementFormDefault="unqualified">
+  <tdml:defineSchema name="multipleElemSameName" xmlns:u="USMTF" elementFormDefault="unqualified">
 
     <xs:include schemaLocation="/org/apache/daffodil/xsd/DFDLGeneralFormat.dfdl.xsd"/>
+    <xs:import namespace="USMTF" schemaLocation="/org/apache/daffodil/section14/sequence_groups/sequenceWithComplexType.dfdl.xsd"/>
     <dfdl:format ref="ex:GeneralFormat" lengthKind="explicit" occursCountKind="implicit"/>
 
     <xs:element name="multipleElemSameName" dfdl:lengthKind="implicit">
@@ -1552,10 +1553,31 @@
       </xs:complexType>
     </xs:element>
 
+    <xs:element name="multipleElemSameNameDifferentNamespaces" dfdl:lengthKind="implicit">
+      <xs:complexType>
+        <xs:sequence>
+          <xs:element name="A" type="xs:string" dfdl:length="1" />
+          <xs:element name="e1" dfdl:terminator="/" dfdl:lengthKind="delimited">
+            <xs:complexType>
+              <xs:sequence dfdl:initiator="-" dfdl:separator=";" dfdl:separatorPosition="prefix">
+                <xs:element name="e2" type="u:ct1" minOccurs="1" maxOccurs="unbounded" dfdl:lengthKind="delimited" />
+              </xs:sequence>
+            </xs:complexType>
+          </xs:element>
+          <xs:element name="B" type="xs:string" dfdl:length="1" />
+          <xs:element ref="u:e1" dfdl:lengthKind="delimited"/>
+        </xs:sequence>
+      </xs:complexType>
+    </xs:element>
+
   </tdml:defineSchema>
 
 
-  <tdml:parserTestCase name="multipleElemSameName" root="multipleElemSameName" model="multipleElemSameName" description="TDML runner verifies warnings and infoset.">
+  <tdml:parserTestCase name="multipleElemSameName"
+                       root="multipleElemSameName"
+                       model="multipleElemSameName"
+                       ignoreUnexpectedWarnings="false"
+                       description="TDML runner verifies warnings and infoset.">
 
     <tdml:document><![CDATA[1234]]></tdml:document>
 
@@ -1572,10 +1594,57 @@
 
     <tdml:warnings>
       <tdml:warning>Schema Definition Warning</tdml:warning>
-      <tdml:warning>same name and namespace</tdml:warning>
+      <tdml:warning>same name</tdml:warning>
       <tdml:warning>AmbigElt</tdml:warning>
     </tdml:warnings>
   </tdml:parserTestCase>
 
+  <tdml:parserTestCase name="multipleElemSameNameDifferentNamespaces"
+                       root="multipleElemSameNameDifferentNamespaces"
+                       model="multipleElemSameName"
+                       ignoreUnexpectedWarnings="false"
+                       description="TDML runner verifies warnings and infoset.">
+
+    <tdml:document><![CDATA[1-;2;3;4/5-;6;7;8/]]></tdml:document>
+
+    <tdml:infoset>
+      <tdml:dfdlInfoset>
+        <multipleElemSameNameDifferentNamespaces>
+          <A>1</A>
+          <e1>
+            <e2>
+              <String1>2</String1>
+            </e2>
+            <e2>
+              <String1>3</String1>
+            </e2>
+            <e2>
+              <String1>4</String1>
+            </e2>
+          </e1>
+          <B>5</B>
+          <e1>
+            <e2>
+              <String1>6</String1>
+            </e2>
+            <e2>
+              <String1>7</String1>
+            </e2>
+            <e2>
+              <String1>8</String1>
+            </e2>
+          </e1>
+        </multipleElemSameNameDifferentNamespaces>
+      </tdml:dfdlInfoset>
+    </tdml:infoset>
+
+    <tdml:warnings>
+      <tdml:warning>Schema Definition Warning</tdml:warning>
+      <tdml:warning>same name</tdml:warning>
+      <tdml:warning>{}e1</tdml:warning>
+      <tdml:warning>u:e1</tdml:warning>
+    </tdml:warnings>
+  </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 70db3a2d1e..432c4b5dd5 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
@@ -149,4 +149,7 @@ class TestSequenceGroups {
   @Test def test_multipleElemSameName() = {
     runner_02.runOneTest("multipleElemSameName")
   }
+  @Test def test_multipleElemSameNameDifferentNamespaces() = {
+    runner_02.runOneTest("multipleElemSameNameDifferentNamespaces")
+  }
 }