From df63ec734154c4f8617e6d88faf90e9360b77f28 Mon Sep 17 00:00:00 2001
From: olabusayoT <50379531+olabusayoT@users.noreply.github.com>
Date: Tue, 1 Oct 2024 10:41:50 -0400
Subject: [PATCH] fixup! Generate warning when node indexed like array

- remove regex check
- update text definition to include predicate if it exists
- generate error when node indexed like array for . and ..
- add tests for UpStepExpression with predicate (..[1])
- add tests to show existing error for NamedStep
- remove warning
- remove irrelevant comments

DAFFODIL-2773
---
 .../daffodil/core/dpath/Expression.scala      | 28 ++++---
 .../core/dsom/CompiledExpression.scala        | 11 ---
 .../org/apache/daffodil/xsd/dafext.xsd        |  1 -
 .../dfdl_expressions/expressions3.tdml        | 81 ++++++++++++++-----
 .../TestDFDLExpressions3.scala                |  2 +
 5 files changed, 82 insertions(+), 41 deletions(-)

diff --git a/daffodil-core/src/main/scala/org/apache/daffodil/core/dpath/Expression.scala b/daffodil-core/src/main/scala/org/apache/daffodil/core/dpath/Expression.scala
index 35c1bacad2..a7a4966629 100644
--- a/daffodil-core/src/main/scala/org/apache/daffodil/core/dpath/Expression.scala
+++ b/daffodil-core/src/main/scala/org/apache/daffodil/core/dpath/Expression.scala
@@ -773,6 +773,15 @@ case class RelativePathExpression(stepsRaw: List[StepExpression], isEvaluatedAbo
 sealed abstract class StepExpression(val step: String, val pred: Option[PredicateExpression])
   extends Expression {
 
+  def checkIfNodeIndexedLikeArray(): Unit = {
+    if (pred.isDefined) {
+      SDE(
+        "Expression contains indexing on a node element: %s.",
+        this.wholeExpressionText
+      )
+    }
+  }
+
   def relPathErr() = {
     // This path expression cannot be compiled because we went past the root. This normally
     // should be an SDE with a RelativePathPastRootError. However, if we don't have any element
@@ -1012,14 +1021,14 @@ sealed abstract class DownStepExpression(s: String, predArg: Option[PredicateExp
   }
 }
 
-// TODO: Is ".[i]" ever a valid expression in DFDL?
-// Perhaps. Doesn't work currently though. See DAFFODIL-2182
-
 sealed abstract class SelfStepExpression(s: String, predArg: Option[PredicateExpression])
   extends DownStepExpression(s, predArg) {
 
-  override lazy val compiledDPath = new CompiledDPath(SelfMove)
-  override def text = "."
+  override lazy val compiledDPath = {
+    checkIfNodeIndexedLikeArray()
+    new CompiledDPath(SelfMove)
+  }
+  override def text = "." + predArg.map(_.text).getOrElse("")
 
   protected def stepElementDefs: Seq[DPathElementCompileInfo] = {
     if (this.isFirstStep) {
@@ -1056,7 +1065,7 @@ case class Self2(s: String, predArg: Option[PredicateExpression])
 sealed abstract class UpStepExpression(s: String, predArg: Option[PredicateExpression])
   extends StepExpression(s, predArg) {
 
-  override def text = ".."
+  override def text = ".." + predArg.map(_.text).getOrElse("")
 
   final override lazy val compiledDPath = {
     val areAllArrays = isLastStep && stepElements.forall {
@@ -1065,6 +1074,7 @@ sealed abstract class UpStepExpression(s: String, predArg: Option[PredicateExpre
     if (areAllArrays) {
       new CompiledDPath(UpMoveArray)
     } else {
+      checkIfNodeIndexedLikeArray
       new CompiledDPath(UpMove)
     }
   }
@@ -1146,10 +1156,8 @@ case class NamedStep(s: String, predArg: Option[PredicateExpression])
         new DownArray(nqn)
       }
     } else {
-      //
-      // Note: DFDL spec allows a[exp] if a is not an array, but it's a processing
-      // error if exp doesn't evaluate to 1.
-      // TODO: Implement this.
+      // a[exp] is only supported on arrays , because Daffodil no longer treats
+      // optional elements as arrays
       if (pred.isDefined)
         subsetError(
           "Indexing is only allowed on arrays. Offending path step: '%s%s'.",
diff --git a/daffodil-core/src/main/scala/org/apache/daffodil/core/dsom/CompiledExpression.scala b/daffodil-core/src/main/scala/org/apache/daffodil/core/dsom/CompiledExpression.scala
index 07ff59f7f4..3dee43b6ae 100644
--- a/daffodil-core/src/main/scala/org/apache/daffodil/core/dsom/CompiledExpression.scala
+++ b/daffodil-core/src/main/scala/org/apache/daffodil/core/dsom/CompiledExpression.scala
@@ -22,7 +22,6 @@ import java.lang.{ Long => JLong }
 import scala.xml.NamespaceBinding
 
 import org.apache.daffodil.core.dpath._
-import org.apache.daffodil.lib.api.WarnID
 import org.apache.daffodil.lib.exceptions.Assert
 import org.apache.daffodil.lib.schema.annotation.props.Found
 import org.apache.daffodil.lib.util.DPathUtil
@@ -236,16 +235,6 @@ class ExpressionCompiler[T <: AnyRef] extends ExpressionCompilerBase[T] {
       compileInfoWhereExpressionWasLocated.SDE(msg, exprOrLiteral)
     }
 
-    if (expr.contains(".[") && expr.matches(".*\\.\\[.*].*")) {
-      // checks that only exprs containing the starting part of the problematic expression
-      // gets the regex check checking that it contains '.[...anything...]'
-      host.SDW(
-        WarnID.NodeIndexedLikeArray,
-        "Expression contains indexing on a node element: %s.",
-        expr
-      )
-    }
-
     val compiler = new DFDLPathExpressionParser[T](
       qn,
       nodeInfoKind,
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 2af61fd340..3d89c56617 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
@@ -721,7 +721,6 @@
           <xs:enumeration value="layerCompileWarning" />
           <xs:enumeration value="multipleChoiceBranches" />
           <xs:enumeration value="namespaceDifferencesOnly" />
-          <xs:enumeration value="nodeIndexedLikeArray" />
           <xs:enumeration value="noEmptyDefault" />
           <xs:enumeration value="nonExpressionPropertyValueLooksLikeExpression" />
           <xs:enumeration value="patternEncodingSlashW" />
diff --git a/daffodil-test/src/test/resources/org/apache/daffodil/section23/dfdl_expressions/expressions3.tdml b/daffodil-test/src/test/resources/org/apache/daffodil/section23/dfdl_expressions/expressions3.tdml
index 192ca3f9c8..ce8b0663f9 100644
--- a/daffodil-test/src/test/resources/org/apache/daffodil/section23/dfdl_expressions/expressions3.tdml
+++ b/daffodil-test/src/test/resources/org/apache/daffodil/section23/dfdl_expressions/expressions3.tdml
@@ -507,38 +507,81 @@
         </xs:sequence>
       </xs:complexType>
     </xs:element>
+
+    <xs:element name="ry">
+      <xs:complexType>
+        <xs:sequence>
+          <xs:element name="e1" type="xs:string"/>
+          <xs:sequence>
+            <xs:annotation>
+              <xs:appinfo source="http://www.ogf.org/dfdl/">
+                <dfdl:assert>{ e1[1] eq '42' }</dfdl:assert>
+              </xs:appinfo>
+            </xs:annotation>
+          </xs:sequence>
+        </xs:sequence>
+      </xs:complexType>
+    </xs:element>
+
+    <xs:element name="rz">
+      <xs:complexType>
+        <xs:sequence>
+          <xs:element name="e1" type="xs:string">
+            <xs:annotation>
+              <xs:appinfo source="http://www.ogf.org/dfdl/">
+                <dfdl:assert>{ fn:exists(..[1]) }</dfdl:assert>
+              </xs:appinfo>
+            </xs:annotation>
+          </xs:element>
+        </xs:sequence>
+      </xs:complexType>
+    </xs:element>
             
  </tdml:defineSchema>
 
   <tdml:parserTestCase name="test_array_self_expr1" model="arraySchema" root="r" ignoreUnexpectedWarnings="false">
     <tdml:document>1|2|3</tdml:document>
     <tdml:errors>
-      <tdml:error>Parse Error</tdml:error>
-      <tdml:error>Assertion expression failed</tdml:error>
+      <tdml:error>Schema Definition Error</tdml:error>
+      <tdml:error>Expression contains</tdml:error>
+      <tdml:error>indexing</tdml:error>
+      <tdml:error>node element</tdml:error>
+      <tdml:error>.[1]</tdml:error>
     </tdml:errors>
-    <tdml:warnings>
-      <tdml:warning>Schema Definition Warning</tdml:warning>
-      <tdml:warning>Expression contains</tdml:warning>
-      <tdml:warning>indexing</tdml:warning>
-      <tdml:warning>node element</tdml:warning>
-      <tdml:warning>.[1]</tdml:warning>
-    </tdml:warnings>
   </tdml:parserTestCase>
   
   <tdml:parserTestCase name="test_array_self_expr2" model="arraySchema" root="rx" ignoreUnexpectedWarnings="false">
     <tdml:document>1</tdml:document>
     <tdml:errors>
-      <tdml:error>Parse Error</tdml:error>
-      <tdml:error>Assertion failed</tdml:error>
-      <tdml:error>42</tdml:error>
+      <tdml:error>Schema Definition Error</tdml:error>
+      <tdml:error>Expression contains</tdml:error>
+      <tdml:error>indexing</tdml:error>
+      <tdml:error>node element</tdml:error>
+      <tdml:error>.[1]</tdml:error>
+    </tdml:errors>
+  </tdml:parserTestCase>
+
+  <tdml:parserTestCase name="test_array_path_expr1" model="arraySchema" root="ry" ignoreUnexpectedWarnings="false">
+    <tdml:document>1</tdml:document>
+    <tdml:errors>
+      <tdml:error>Schema Definition Error</tdml:error>
+      <tdml:error>Subset</tdml:error>
+      <tdml:error>Indexing</tdml:error>
+      <tdml:error>only allowed</tdml:error>
+      <tdml:error>arrays</tdml:error>
+      <tdml:error>e1[1]</tdml:error>
+    </tdml:errors>
+  </tdml:parserTestCase>
+
+  <tdml:parserTestCase name="test_array_path_expr2" model="arraySchema" root="rz" ignoreUnexpectedWarnings="false">
+    <tdml:document>1</tdml:document>
+    <tdml:errors>
+      <tdml:error>Schema Definition Error</tdml:error>
+      <tdml:error>Expression contains</tdml:error>
+      <tdml:error>indexing</tdml:error>
+      <tdml:error>node element</tdml:error>
+      <tdml:error>..[1]</tdml:error>
     </tdml:errors>
-    <tdml:warnings>
-      <tdml:warning>Schema Definition Warning</tdml:warning>
-      <tdml:warning>Expression contains</tdml:warning>
-      <tdml:warning>indexing</tdml:warning>
-      <tdml:warning>node element</tdml:warning>
-      <tdml:warning>.[1]</tdml:warning>
-    </tdml:warnings>
   </tdml:parserTestCase>
 
   <tdml:parserTestCase name="setVariable_neg_01"
diff --git a/daffodil-test/src/test/scala/org/apache/daffodil/section23/dfdl_expressions/TestDFDLExpressions3.scala b/daffodil-test/src/test/scala/org/apache/daffodil/section23/dfdl_expressions/TestDFDLExpressions3.scala
index 502ab109fc..8f2bdcebc3 100644
--- a/daffodil-test/src/test/scala/org/apache/daffodil/section23/dfdl_expressions/TestDFDLExpressions3.scala
+++ b/daffodil-test/src/test/scala/org/apache/daffodil/section23/dfdl_expressions/TestDFDLExpressions3.scala
@@ -48,6 +48,8 @@ class TestDFDLExpressions3 {
 
   @Test def test_array_self_expr1(): Unit = { runner.runOneTest("test_array_self_expr1") }
   @Test def test_array_self_expr2(): Unit = { runner.runOneTest("test_array_self_expr2") }
+  @Test def test_array_path_expr1(): Unit = { runner.runOneTest("test_array_path_expr1") }
+  @Test def test_array_path_expr2(): Unit = { runner.runOneTest("test_array_path_expr2") }
 
   @Test def test_setVariable_neg_01(): Unit = { runner.runOneTest("setVariable_neg_01") }