stevedlawrence commented on code in PR #1321:
URL: https://github.com/apache/daffodil/pull/1321#discussion_r1777747773
##########
daffodil-core/src/main/scala/org/apache/daffodil/core/dsom/SequenceGroup.scala:
##########
@@ -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]] = {
Review Comment:
Might suggest a different name for this since it isn't really checking
anything, it's just grouping things.
##########
daffodil-core/src/main/scala/org/apache/daffodil/core/dsom/SequenceGroup.scala:
##########
@@ -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
Review Comment:
I wonder if this needs a different grouping than with the unorderd sequences?
The goal of this check is to warn in case the schema is used with an infoset
that don't support duplicate fields, and those kinds of infosets probably also
don't support namespaces. So we probably want to warn even if they have
different namespaces, which means we need to group by name only and not by
QName.
Also, we kindof already have a similar check with
`WarnID.NamespaceDifferencesOnly`. I wonder if that check should be removed in
place of this one, since this new check is really just a more general version
of that check? The existing one is about same-name elements next to each other,
this one is about same-name elements that are siblings. Anything that this new
check warns about will also warn for the existing check.
##########
daffodil-core/src/main/scala/org/apache/daffodil/core/dsom/SequenceGroup.scala:
##########
@@ -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,
Review Comment:
Maybe this wants to be WarnID.NamespaceDifferencesOnly?
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
