Do not show counterintuitive placement for testKind=expression

The counterintuitive placement warning should only be displayed for asserts/disciminators that have testKind=expression. DAFFODIL-2912
apache · Aug 23, 2024 · d26c634 · d26c634
1 parent 67eef7e
commit d26c634
Show file tree

Hide file tree

Showing 3 changed files with 105 additions and 19 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
@@ -33,6 +33,7 @@ import org.apache.daffodil.lib.schema.annotation.props.SeparatorSuppressionPolic
 import org.apache.daffodil.lib.schema.annotation.props.gen.OccursCountKind
 import org.apache.daffodil.lib.schema.annotation.props.gen.SeparatorPosition
 import org.apache.daffodil.lib.schema.annotation.props.gen.SequenceKind
+import org.apache.daffodil.lib.schema.annotation.props.gen.TestKind
 import org.apache.daffodil.lib.xml.RefQName
 import org.apache.daffodil.lib.xml.XMLUtils
 import org.apache.daffodil.runtime1.layers.LayerRuntimeData
@@ -164,9 +165,12 @@ abstract class SequenceGroupTermBase(xml: Node, lexicalParent: SchemaComponent,
   protected final lazy val checkIfNonEmptyAndDiscrimsOrAsserts: Unit = {
     val msg = "Counterintuitive placement detected. Wrap the discriminator or assert " +
       "in an empty sequence to evaluate before the contents."
-    if (groupMembers.nonEmpty && discriminatorStatements.nonEmpty)
+    // Only show warning if the testKind=expression, see DFDL spec Section 9.5 Evaluation Order for Statement Annotations
+    if (
+      groupMembers.nonEmpty && discriminatorStatements.exists(_.testKind == TestKind.Expression)
+    )
       SDW(WarnID.DiscouragedDiscriminatorPlacement, msg)
-    if (groupMembers.nonEmpty && assertStatements.nonEmpty)
+    if (groupMembers.nonEmpty && assertStatements.exists(_.testKind == TestKind.Expression))
       SDW(WarnID.DiscouragedAssertPlacement, msg)
   }
 

diff --git a/...l-test/src/test/resources/org/apache/daffodil/section07/discriminators/discriminator.tdml b/...l-test/src/test/resources/org/apache/daffodil/section07/discriminators/discriminator.tdml
@@ -32,7 +32,7 @@
     <xs:include schemaLocation="/org/apache/daffodil/xsd/DFDLGeneralFormat.dfdl.xsd"/>
     <dfdl:format ref="ex:GeneralFormat" />
 
-    <xs:element name="root-discrim" dfdl:lengthKind="implicit" >
+    <xs:element name="root-discrim-pattern" dfdl:lengthKind="implicit" >
       <xs:complexType>
         <xs:sequence dfdl:separator="," dfdl:separatorPosition="infix">
           <xs:annotation>
@@ -48,14 +48,46 @@
       </xs:complexType>
     </xs:element>
 
-    <xs:element name="root-assert" dfdl:lengthKind="implicit" >
+    <xs:element name="root-discrim-expression" dfdl:lengthKind="implicit" >
+      <xs:complexType>
+        <xs:sequence dfdl:separator="," dfdl:separatorPosition="infix">
+          <xs:annotation>
+            <xs:appinfo source="http://www.ogf.org/dfdl/">
+              <dfdl:discriminator testKind="expression"
+                                  test="{ fn:true() }"
+                                  message="discriminator failed" />
+            </xs:appinfo>
+          </xs:annotation>
+          <xs:element ref="ex:description" dfdl:lengthKind="delimited"/>
+          <xs:element ref="ex:quantity" dfdl:lengthKind="delimited"/>
+        </xs:sequence>
+      </xs:complexType>
+    </xs:element>
+
+    <xs:element name="root-assert-pattern" dfdl:lengthKind="implicit" >
       <xs:complexType>
         <xs:sequence dfdl:separator="," dfdl:separatorPosition="infix">
           <xs:annotation>
             <xs:appinfo source="http://www.ogf.org/dfdl/">
               <dfdl:assert testKind="pattern"
-                                  testPattern="\p{L}{3}"
-                                  message="assert failed for pattern '\p{L}{3}'" />
+                           testPattern="\p{L}{3}"
+                           message="assert failed for pattern '\p{L}{3}'" />
+            </xs:appinfo>
+          </xs:annotation>
+          <xs:element ref="ex:description" dfdl:lengthKind="delimited"/>
+          <xs:element ref="ex:quantity" dfdl:lengthKind="delimited"/>
+        </xs:sequence>
+      </xs:complexType>
+    </xs:element>
+
+    <xs:element name="root-assert-expression" dfdl:lengthKind="implicit" >
+      <xs:complexType>
+        <xs:sequence dfdl:separator="," dfdl:separatorPosition="infix">
+          <xs:annotation>
+            <xs:appinfo source="http://www.ogf.org/dfdl/">
+              <dfdl:assert testKind="expression"
+                           test="{ fn:true() }"
+                           message="assert failed" />
             </xs:appinfo>
           </xs:annotation>
           <xs:element ref="ex:description" dfdl:lengthKind="delimited"/>
@@ -69,21 +101,41 @@
   </tdml:defineSchema>
 
   <!--
-  Test name: discrimPlacementSDW
-  Schema: discrimPlacement, root root-discrim
+  Test name: discrimPlacementPatternSDW
+  Schema: discrimPlacement, root root-discrim-pattern
+  Purpose: This test demonstrates that no SDW occurs for testKind=pattern
+  -->
+
+  <tdml:parserTestCase name="discrimPlacementPatternSDW"
+                       root="root-discrim-pattern" model="discrimAssertPlacement"
+                       description="This test demonstrates that no SDW occurs for testKind=pattern">
+    <tdml:document>Hat,2</tdml:document>
+    <tdml:infoset>
+      <tdml:dfdlInfoset>
+        <root-discrim-pattern>
+          <description>Hat</description>
+          <quantity>2</quantity>
+        </root-discrim-pattern>
+      </tdml:dfdlInfoset>
+    </tdml:infoset>
+  </tdml:parserTestCase>
+
+  <!--
+  Test name: discrimPlacementExpressionSDW
+  Schema: discrimPlacement, root root-discrim-expression
   Purpose: This test demonstrates the SDW resulting from discouraged discriminator placement
   -->
 
-  <tdml:parserTestCase name="discrimPlacementSDW"
-                       root="root-discrim" model="discrimAssertPlacement"
+  <tdml:parserTestCase name="discrimPlacementExpressionSDW"
+                       root="root-discrim-expression" model="discrimAssertPlacement"
                        description="This test demonstrates the SDW resulting from discouraged discriminator placement">
     <tdml:document>Hat,2</tdml:document>
     <tdml:infoset>
       <tdml:dfdlInfoset>
-        <root-discrim>
+        <root-discrim-expression>
           <description>Hat</description>
           <quantity>2</quantity>
-        </root-discrim>
+        </root-discrim-expression>
       </tdml:dfdlInfoset>
     </tdml:infoset>
     <tdml:warnings>
@@ -93,21 +145,41 @@
   </tdml:parserTestCase>
 
   <!--
-  Test name: assertPlacementSDW
+  Test name: assertPlacementPatternSDW
+  Schema: discrimAssertPlacement, root root-assert-pattern
+  Purpose: This test demonstrates that no SDW occurs for testKind=pattern
+  -->
+
+  <tdml:parserTestCase name="assertPlacementPatternSDW"
+                       root="root-assert-pattern" model="discrimAssertPlacement"
+                       description="This test demonstrates no SDW occurs for testKind=pattern">
+    <tdml:document>Hat,2</tdml:document>
+    <tdml:infoset>
+      <tdml:dfdlInfoset>
+        <root-assert-pattern>
+          <description>Hat</description>
+          <quantity>2</quantity>
+        </root-assert-pattern>
+      </tdml:dfdlInfoset>
+    </tdml:infoset>
+  </tdml:parserTestCase>
+
+  <!--
+  Test name: assertPlacementExpressionSDW
   Schema: discrimAssertPlacement, root root-assert
   Purpose: This test demonstrates the SDW resulting from discouraged assert placement
   -->
 
-  <tdml:parserTestCase name="assertPlacementSDW"
-                       root="root-assert" model="discrimAssertPlacement"
+  <tdml:parserTestCase name="assertPlacementExpressionSDW"
+                       root="root-assert-expression" model="discrimAssertPlacement"
                        description="This test demonstrates the SDW resulting from discouraged assert placement">
     <tdml:document>Hat,2</tdml:document>
     <tdml:infoset>
       <tdml:dfdlInfoset>
-        <root-assert>
+        <root-assert-expression>
           <description>Hat</description>
           <quantity>2</quantity>
-        </root-assert>
+        </root-assert-expression>
       </tdml:dfdlInfoset>
     </tdml:infoset>
     <tdml:warnings>

diff --git a/...test/src/test/scala/org/apache/daffodil/section07/discriminators/TestDiscriminators.scala b/...test/src/test/scala/org/apache/daffodil/section07/discriminators/TestDiscriminators.scala
@@ -111,8 +111,18 @@ class TestDiscriminators {
     runner2.runOneTest("multipleDiscriminators5")
   }
 
-  @Test def test_discrimPlacementSDW(): Unit = { runner.runOneTest("discrimPlacementSDW") }
-  @Test def test_assertPlacementSDW(): Unit = { runner.runOneTest("assertPlacementSDW") }
+  @Test def test_discrimPlacementExpressionSDW(): Unit = {
+    runner.runOneTest("discrimPlacementExpressionSDW")
+  }
+  @Test def test_discrimPlacementPatternSDW(): Unit = {
+    runner.runOneTest("discrimPlacementPatternSDW")
+  }
+  @Test def test_assertPlacementExpressionSDW(): Unit = {
+    runner.runOneTest("assertPlacementExpressionSDW")
+  }
+  @Test def test_assertPlacementPatternSDW(): Unit = {
+    runner.runOneTest("assertPlacementPatternSDW")
+  }
 
   @Test def test_nameDOB_test1(): Unit = { runner3.runOneTest("nameDOB_test1") }
   @Test def test_nameDOB_test_bad_date_first_row(): Unit = {