Revert "Check for Unused Properties on Annotated Schema Components"

This reverts commit f9854f4.

This commit caused a regression on some schemas, creating many "unused
property" false positive warnings. The change isn't critical and is
likely a difficult change to complete and verify for the next release.
Revert it for now, in the next dev cycle we can cherry-pick the change
to restore and fix it.

DAFFODIL-2798

daffodil-core/src/main/scala/org/apache/daffodil/core/dsom/AnnotatedSchemaComponent.scala

@@ -291,7 +291,6 @@ final class SharedFactory[SharedType] {
 trait AnnotatedSchemaComponent
   extends SchemaComponent
   with AnnotatedMixin
-  with ResolvesLocalProperties
   with OverlapCheckMixin {
 
   protected override def initialize(): Unit = {
@@ -378,45 +377,6 @@ trait AnnotatedSchemaComponent
     val res = schemaDocument.formatAnnotation.formatChain
     res
   }
-
-  /**
-   * Used to look for DFDL properties on Annotated Schema Components that
-   * have not been accessed and record it as a warning. This function uses the
-   * property cache state to determine which properties have been accessed, so
-   * this function must only be called after all property accesses are complete
-   * (e.g. schema compilation has finished) and after propagateUsedProperties
-   * has been called on all AnnotatedSchemaComponents, to ensure there are no false
-   * positives/negatives.
-   *
-   * Note: This is not a recursive walk. It identifies and issues warnings about
-   * unaccessed properties on just one AnnotatedSchemaComponent
-   */
-  final lazy val checkUnusedProperties: Unit = {
-    // Get the properties defined on this component and what it refers to
-    val localProps = formatAnnotation.justThisOneProperties
-    val refProps = optReferredToComponent
-      .map { _.formatAnnotation.justThisOneProperties }
-      .getOrElse(Map.empty)
-
-    val usedProperties = propCache
-
-    localProps.foreach { case (prop, (value, _)) =>
-      if (!usedProperties.contains(prop)) {
-        SDW(WarnID.IgnoreDFDLProperty, "DFDL property was ignored: %s=\"%s\"", prop, value)
-      }
-    }
-
-    refProps.foreach { case (prop, (value, _)) =>
-      if (!usedProperties.contains(prop)) {
-        optReferredToComponent.get.SDW(
-          WarnID.IgnoreDFDLProperty,
-          "DFDL property was ignored: %s=\"%s\"",
-          prop,
-          value
-        )
-      }
-    }
-  }
 }
 
 /**

daffodil-core/src/main/scala/org/apache/daffodil/core/dsom/RestrictionUnion.scala

@@ -278,7 +278,7 @@ final class Union private (val xmlArg: Node, simpleTypeDef: SimpleTypeDefBase)
   private lazy val immediateTypeXMLs = xml \ "simpleType"
   private lazy val immediateTypes: Seq[SimpleTypeDefBase] = immediateTypeXMLs.map { node =>
     {
-      LocalSimpleTypeDef(node, this)
+      LocalSimpleTypeDef(node, schemaDocument)
     }
   }
 

daffodil-core/src/main/scala/org/apache/daffodil/core/dsom/Root.scala

@@ -82,22 +82,6 @@ final class Root private (
     m.toMap
   }
 
-  /**
-   * For any given global simple type, tells us what other simple types contain
-   * references to it
-   *
-   * This is intended to be used when we're propagating used properties to ensure
-   * properties are properly propagates along the simple type base chain
-   */
-  lazy val typeBaseChainMap: Map[GlobalComponent, Seq[(String, Seq[RefSpec])]] = {
-    val refEntries: Seq[(GlobalComponent, Seq[RefSpec])] =
-      typeBaseChains.groupBy { _.to }.toSeq
-    val m: Seq[(GlobalComponent, Seq[(String, Seq[RefSpec])])] = refEntries.map {
-      case (to, seq) => (to, seq.groupBy { _.from.shortSchemaComponentDesignator }.toSeq)
-    }
-    m.toMap
-  }
-
   final def elementRefsTo(decl: GlobalElementDecl): Seq[ElementRef] =
     refsTo(decl).asInstanceOf[Seq[ElementRef]]
 
@@ -157,17 +141,6 @@ final class Root private (
     }.flatten
   }
 
-  final lazy val typeBaseChains: Seq[RefSpec] = {
-    allComponents.collect {
-      case std: SimpleTypeDefBase => {
-        val optStd = std.optReferredToComponent.collect { case stdb: SimpleTypeDefBase =>
-          stdb
-        }
-        optStd.map { stdb => RefSpec(std, stdb, 1) }.toSeq
-      }
-    }.flatten
-  }
-
   lazy val allERefs = allComponents
     .filter {
       case er: ElementRef => true

daffodil-core/src/main/scala/org/apache/daffodil/core/dsom/SchemaSet.scala

@@ -18,7 +18,6 @@
 package org.apache.daffodil.core.dsom
 
 import java.io.File
-import scala.collection.mutable
 import scala.xml.Node
 
 import org.apache.daffodil.core.compiler.RootSpec
@@ -554,127 +553,8 @@ final class SchemaSet private (
     .flatMap(_.defineVariables)
     .union(predefinedVars)
 
-  private val doneSimpleTypeDefs: mutable.Set[SimpleTypeDefBase] = mutable.Set()
-
-  /**
-   * Used to propagate used properties through all element/group references, element declarations
-   * and simple types. This is necessary because we can have elements being referenced
-   * by multiple element refs, where the first element gets its propCache populated with
-   * the properties it has used, and the second element doesn't because it shares the parsers
-   * of the first element.
-   *
-   * With this function, we copy used properties from element refs to elements, group refs to groups,
-   * from elements to their simple types, from enclosing elements to their local simple types,
-   * and from simple types to their base types. This function is recursive when processing
-   * simple type definitions that reference simple type defs whose base is a primitive type.
-   *
-   * Note: this is clobbering the property caches, making them subsequently unusable
-   * for their initial property lookup uses. Hence, this must be done in a pass that
-   * happens only after all other schema compilation involving properties is complete.
-   */
-  def propagateUsedPropertiesForThis(asc: AnnotatedSchemaComponent): Unit = {
-    asc match {
-      case ggd: GlobalGroupDef => {
-        val groupRefs = root.refMap.getOrElse(ggd, Seq.empty)
-        groupRefs.foreach { case (_, grs) =>
-          grs.foreach { rs =>
-            val gr = rs.from.asInstanceOf[AnnotatedSchemaComponent]
-            gr.propCache.foreach { kv =>
-              if (ggd.formatAnnotation.justThisOneProperties.contains(kv._1)) {
-                ggd.propCache.put(kv._1, kv._2)
-              }
-            }
-          }
-        }
-      }
-      case ged: GlobalElementDecl => {
-        val elementRefs = root.refMap.getOrElse(ged, Seq.empty)
-        elementRefs.foreach { case (_, ers) =>
-          ers.foreach { rs =>
-            val er = rs.from.asInstanceOf[AnnotatedSchemaComponent]
-            er.propCache.foreach { kv =>
-              if (ged.formatAnnotation.justThisOneProperties.contains(kv._1)) {
-                ged.propCache.put(kv._1, kv._2)
-              }
-            }
-          }
-        }
-      }
-      case gstd: GlobalSimpleTypeDef if gstd.bases.nonEmpty => {
-        val elementsOfType = root.refMap.getOrElse(gstd, Seq.empty)
-        elementsOfType.foreach { case (_, eles) =>
-          eles.foreach { rs =>
-            val er = rs.from.asInstanceOf[AnnotatedSchemaComponent]
-            er.propCache.foreach { kv =>
-              if (gstd.formatAnnotation.justThisOneProperties.contains(kv._1)) {
-                gstd.propCache.put(kv._1, kv._2)
-              }
-            }
-          }
-        }
-        doneSimpleTypeDefs.add(gstd)
-      }
-      case lstd: LocalSimpleTypeDef => {
-        val enclElements = lstd.enclosingElements
-        enclElements.foreach { ee =>
-          ee.propCache.foreach { kv =>
-            if (lstd.formatAnnotation.justThisOneProperties.contains(kv._1)) {
-              lstd.propCache.put(kv._1, kv._2)
-            }
-          }
-        }
-        doneSimpleTypeDefs.add(lstd)
-      }
-      case gstd: GlobalSimpleTypeDef if gstd.bases.isEmpty => {
-        val othersReferencingThis =
-          root.typeBaseChainMap
-            .getOrElse(gstd, Seq.empty) ++ root.refMap.getOrElse(gstd, Seq.empty)
-        othersReferencingThis.foreach { case (_, refs) =>
-          refs.collect {
-            case rs if rs.from.isInstanceOf[SimpleTypeDefBase] =>
-              val std = rs.from.asInstanceOf[SimpleTypeDefBase]
-              if (!doneSimpleTypeDefs.contains(std)) {
-                propagateUsedPropertiesForThis(std)
-              }
-              std.propCache.foreach(kv => gstd.propCache.put(kv._1, kv._2))
-            case rs =>
-              val er = rs.from.asInstanceOf[AnnotatedSchemaComponent]
-              er.propCache.foreach { kv =>
-                if (gstd.formatAnnotation.justThisOneProperties.contains(kv._1)) {
-                  gstd.propCache.put(kv._1, kv._2)
-                }
-              }
-          }
-        }
-        doneSimpleTypeDefs.add(gstd)
-      }
-      case _ => // do nothing
-    }
-  }
-
-  /**
-   * Propagate used properties through AnnotatedSchemaComponent.
-   * This must be done after compilation is complete to ensure all properties
-   * that are used are accounted for.
-   * This is part of the algorithm for identifying properties that are present, but unused.
-   *
-   * Note: This algorithm repurposes propCaches which are no longer usable
-   * for property lookups after this is done.
-   */
-  private lazy val propagateUsedProperties = {
-    root.allComponents.collect { case asc: AnnotatedSchemaComponent =>
-      propagateUsedPropertiesForThis(asc)
-    }
-  }
-
-  /**
-   * check unused properties on annotated schema component, must be done
-   * after compilation is completed and used properties are propagated
-   */
-  private lazy val checkUnusedProperties = LV('checkUnusedProperties) {
-    root.allComponents.collect { case asc: AnnotatedSchemaComponent =>
-      asc.checkUnusedProperties
-    }
+  private lazy val checkUnusedProperties = LV('hasUnusedProperties) {
+    root.checkUnusedProperties
   }.value
 
   /**
@@ -705,15 +585,6 @@ final class SchemaSet private (
       // take into account.
       val hasErrors = super.isError
       if (!hasErrors) {
-        // must be called after compilation is done
-        // this is a central part of the algorithm by which we identify properties that are present,
-        // but not used (see checkUnusedProperties).
-        // The overall algorithm is expressed in multiple places.
-        // This part handles the propagation of properties from referencer to referenced object
-        // when property combining occurs.
-        propagateUsedProperties
-        // The rest of the algorithm is then just traversing every AnnotatedSchemaComponent
-        // to see what properties are present on them that do not appear in the propCache.
         // must be last, after all compilation is done.
         // only check this if there are no errors.
         checkUnusedProperties
@@ -830,9 +701,8 @@ class TransitiveClosureSchemaComponents private () extends TransitiveClosure[Sch
         st.bases ++
           st.optRestriction ++
           st.optUnion
-      case r: Restriction => r.optUnion.toSeq ++ r.enumerations
+      case r: Restriction => r.optUnion.toSeq
       case u: Union => u.unionMemberTypes
-      case e: EnumerationDef => Nil
       case c: ComplexTypeBase => Seq(c.modelGroup)
       case gd: GlobalGroupDef => gd.groupMembers
       case stmt: DFDLStatement => Nil

daffodil-core/src/main/scala/org/apache/daffodil/core/dsom/SimpleTypes.scala

@@ -305,5 +305,4 @@ final class EnumerationDef(xml: Node, parentType: SimpleTypeDefBase)
   protected def isMyFormatAnnotation(a: DFDLAnnotation): Boolean =
     Assert.invariantFailed("Should not be called")
 
-  override val diagnosticDebugNameImpl = "enumeration"
 }

daffodil-core/src/main/scala/org/apache/daffodil/core/dsom/Term.scala

@@ -22,6 +22,7 @@ import java.util.UUID
 
 import org.apache.daffodil.core.dsom.walker.TermView
 import org.apache.daffodil.core.grammar.TermGrammarMixin
+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.schema.annotation.props.NotFound
@@ -104,6 +105,43 @@ trait Term
    */
   final def tci = dpathCompileInfo
 
+  /**
+   * Used to recursively go through Terms and look for DFDL properties that
+   * have not been accessed and record it as a warning. This function uses the
+   * property cache state to determine which properties have been access, so
+   * this function must only be called after all property accesses are complete
+   * (e.g. schema compilation has finished) to ensure there are no false
+   * positives.
+   */
+  final lazy val checkUnusedProperties: Unit = {
+    // Get the properties defined on this term and what it refers to
+    val localProps = formatAnnotation.justThisOneProperties
+    val refProps = optReferredToComponent
+      .map { _.formatAnnotation.justThisOneProperties }
+      .getOrElse(Map.empty)
+
+    val usedProperties = propCache
+
+    localProps.foreach { case (prop, (value, _)) =>
+      if (!usedProperties.contains(prop)) {
+        SDW(WarnID.IgnoreDFDLProperty, "DFDL property was ignored: %s=\"%s\"", prop, value)
+      }
+    }
+
+    refProps.foreach { case (prop, (value, _)) =>
+      if (!usedProperties.contains(prop)) {
+        optReferredToComponent.get.SDW(
+          WarnID.IgnoreDFDLProperty,
+          "DFDL property was ignored: %s=\"%s\"",
+          prop,
+          value
+        )
+      }
+    }
+
+    termChildren.foreach { _.checkUnusedProperties }
+  }
+
   def position: Int
 
   def optIgnoreCase: Option[YesNo] = {

daffodil-lib/src/main/scala/org/apache/daffodil/lib/exceptions/SchemaFileLocatable.scala

@@ -136,14 +136,11 @@ class XercesSchemaFileLocation(
 
   // we have to override equals and hashCode because the OOlag error checks for duplicates in its error list
   override def equals(obj: Any): Boolean = {
-    // sometimes what OOlag error/warning is comparing against isn't a XercesSchemaFileLocation, but
-    // is a schemaFileLocation instead, since XercesSchemaFileLocation is a SchemaFileLocation as well,
-    // we just use that for comparison
-    val sflObj = obj.asInstanceOf[SchemaFileLocation]
-    sflObj.lineNumber.getOrElse("") == this.lineNumber.getOrElse("") &&
-    sflObj.columnNumber.getOrElse("") == this.columnNumber.getOrElse("") &&
-    sflObj.diagnosticFile == this.diagnosticFile &&
-    sflObj.diagnosticDebugName == this.diagnosticDebugName
+    val xsflObj = obj.asInstanceOf[XercesSchemaFileLocation]
+    xsflObj.xercesError.getLineNumber == this.xercesError.getLineNumber &&
+    xsflObj.xercesError.getColumnNumber == this.xercesError.getColumnNumber &&
+    xsflObj.xercesError.getSystemId == this.xercesError.getSystemId &&
+    xsflObj.schemaFileLocation == this.schemaFileLocation
   }
 
   override def hashCode: Int = {

daffodil-lib/src/main/scala/org/apache/daffodil/lib/schema/annotation/props/PropertyScoping.scala

@@ -152,13 +152,6 @@ trait FindPropertyMixin extends PropTypes {
    */
   protected def lookupProperty(pname: String): PropertyLookupResult
 
-  /**
-   * the PropCache plays two roles in different phases of the compilation.
-   * First for property lookups, second for determining which properties are present, but unused.
-   *
-   * These two uses are incompatible as the second does propagation of used properties using propCache.
-   * The first use must therefore be over. Then the caches are repurposed for the second algorithm.
-   */
   val propCache = new scala.collection.mutable.LinkedHashMap[String, PropertyLookupResult]
 
   /**