[GitHub] [daffodil] tuxji commented on a change in pull request #635: Fix unique qname check in unordered sequences

[GitBox <gi...@apache.org>]

tuxji commented on a change in pull request #635:
URL: https://github.com/apache/daffodil/pull/635#discussion_r709489638



##########
File path: daffodil-core/src/main/scala/org/apache/daffodil/dsom/SequenceGroup.scala
##########
@@ -175,47 +175,36 @@ abstract class SequenceGroupTermBase(
       groupMembers.filter { m => m.isInstanceOf[LocalElementDecl] || m.isInstanceOf[ElementRef]
       }.map(_.asInstanceOf[ElementBase])
 
-    val invalidChildren = validChildren.filter(e => {
+    val invalidChild = validChildren.find(e => {
       if (e.minOccurs == 0 | !e.isScalar) {
         e.occursCountKind match {
           case OccursCountKind.Parsed => false
           case _ => true
         }
       } else false
     })
-    val hasInvalidChildren = invalidChildren.length > 0
-    if (hasInvalidChildren)
-      this.SDE("Members of an unordered sequence (%s) that are optional or array elements must have dfdl:occursCountKind='parsed'." +
-        "\nThe offending members: %s.", this.path, invalidChildren.mkString(","))
+    if (invalidChild.isDefined) {
+      invalidChild.get.SDE("Member of an unordered sequence that is an optional or array element must have dfdl:occursCountKind='parsed'")
+    }
   }
 
   private lazy val checkMembersAreAllElementOrElementRef: Unit = {
-    val invalidChildren = groupMembers.filterNot(child =>
-      child.isInstanceOf[LocalElementDecl] || child.isInstanceOf[ElementRef])
-    val hasInvalidChildren = invalidChildren.length > 0
-    if (hasInvalidChildren)
-      this.SDE("Members of an unordered sequence (%s) must be Element or ElementRef." +
-        "\nThe offending members: %s.", this.path, invalidChildren.mkString(","))
+    val invalidChild = groupMembers.find(!_.isInstanceOf[ElementBase])
+    if (invalidChild.isDefined) {
+      invalidChild.get.SDE("Member of an unordered sequence must be an element declaration or element reference")
+    }
   }
 
   private lazy val checkMembersHaveUniqueNamesInNamespaces: Unit = {
-    val childrenGroupedByNamespace =
-      groupMembers.filter(m => m.isInstanceOf[ElementBase]).map(_.asInstanceOf[ElementBase]).groupBy(_.targetNamespace)
-
-    childrenGroupedByNamespace.foreach {
-      case (ns, children) => {
-        // At this point we're looking at the individual namespace buckets
-        val childrenGroupedByName = children.groupBy(child => child.name)
-        childrenGroupedByName.foreach {
-          case (name, children) =>
-            // Now we're looking at the individual name buckets within the
-            // individual namespace bucket.
-            if (children.length > 1)
-              this.SDE(
-                "Two or more members of the unordered sequence (%s) have the same name and the same namespace." +
-                  "\nNamespace: %s\tName: %s.",
-                this.path, ns, name)
-        }
+    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) {
+        children.head.SDE("Two or more memebers of an unordered sequence have the same name and the same namespace")

Review comment:
       Typo: memebers -> members




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@daffodil.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org