You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@daffodil.apache.org by mbeckerle <gi...@git.apache.org> on 2017/11/09 00:13:35 UTC

[GitHub] incubator-daffodil pull request #5: Property Resolution - part of fixing sch...

GitHub user mbeckerle opened a pull request:

    https://github.com/apache/incubator-daffodil/pull/5

    Property Resolution - part of fixing schema compilation

    Refactors property resolving onto elementrefs and grouprefs so
    as to eliminate use of backrefs when accessing properties.
    
    Reorganized DSOM for model groups and references to them.
    
    Converted OOLAGHost, SchemaComponent etc. down to LocalElementBase
    into traits so that I can mix them the way I need to so that
    Global elements aren't terms, etc.
    
    Group references are allowed to carry annotations like dfdl:assert. It
    is global group definitions that cannot. (They can't have annotations
    period. Annotations go on the model group within the global group
    definition)
    
    SequenceBase is a ModelGroup is a Term, and is
    the base of Sequence and SequenceGroupRef.
    
    SequenceGroupDef is not a SequenceBase. It is GroupDefLike in that
    it has children. It is not a Term, because the SequenceGroupRef that
    refers to it is the Term.
    
    Groups work exactly analogously to Elements now. For Elements, the Terms
    are
    LocalElementDecl and ElementRef. A GlobalElementDecl is *not* a Term.

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/mbeckerle/incubator-daffodil daffodil-1855-prop-resolve

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/incubator-daffodil/pull/5.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #5
    
----
commit 89f640dab66bd68546f43bbd828eb58451f8f987
Author: mbeckerle <mb...@tresys.com>
Date:   2017-10-23T21:50:21Z

    Property Resolution - part of fixing schema compilation
    
    Refactors property resolving onto elementrefs and grouprefs so
    as to eliminate use of backrefs when accessing properties.
    
    Reorganized DSOM for model groups and references to them.
    
    Converted OOLAGHost, SchemaComponent etc. down to LocalElementBase
    into traits so that I can mix them the way I need to so that
    Global elements aren't terms, etc.
    
    Group references are allowed to carry annotations like dfdl:assert. It
    is global group definitions that cannot. (They can't have annotations
    period. Annotations go on the model group within the global group
    definition)
    
    SequenceBase is a ModelGroup is a Term, and is
    the base of Sequence and SequenceGroupRef.
    
    SequenceGroupDef is not a SequenceBase. It is GroupDefLike in that
    it has children. It is not a Term, because the SequenceGroupRef that
    refers to it is the Term.
    
    Groups work exactly analogously to Elements now. For Elements, the Terms
    are
    LocalElementDecl and ElementRef. A GlobalElementDecl is *not* a Term.

----


---

[GitHub] incubator-daffodil pull request #5: Property Resolution - part of fixing sch...

Posted by mbeckerle <gi...@git.apache.org>.
Github user mbeckerle commented on a diff in the pull request:

    https://github.com/apache/incubator-daffodil/pull/5#discussion_r150681797
  
    --- Diff: daffodil-core/src/main/scala/edu/illinois/ncsa/daffodil/dsom/LocalElementDecl.scala ---
    @@ -34,27 +34,24 @@ package edu.illinois.ncsa.daffodil.dsom
     
     import scala.xml.Node
     
    -final class LocalElementDeclFactory(override val xml: Node, sd: SchemaDocument)
    -  extends SchemaComponentFactory(xml, sd)
    +final class LocalElementDeclFactory(xmlArg: Node, sd: SchemaDocument)
    --- End diff --
    
    There is no LocalComplexTypeDefFactory, but the others removed..


---

[GitHub] incubator-daffodil pull request #5: Property Resolution - part of fixing sch...

Posted by mbeckerle <gi...@git.apache.org>.
Github user mbeckerle commented on a diff in the pull request:

    https://github.com/apache/incubator-daffodil/pull/5#discussion_r149840160
  
    --- Diff: daffodil-core/src/main/scala/edu/illinois/ncsa/daffodil/grammar/ElementReferenceGrammarMixin.scala ---
    @@ -32,10 +32,8 @@
     
     package edu.illinois.ncsa.daffodil.grammar
     
    -import edu.illinois.ncsa.daffodil.dsom.ElementRef
    +import edu.illinois.ncsa.daffodil.dsom.AbstractElementRef
     
    -trait ElementReferenceGrammarMixin { self: ElementRef =>
    -
    -  final override lazy val termContentBody = self.referencedElement.termContentBody
    +trait ElementReferenceGrammarMixin { self: AbstractElementRef =>
    --- End diff --
    
    Delete this whole file and remove references to the trait.


---

[GitHub] incubator-daffodil pull request #5: Property Resolution - part of fixing sch...

Posted by mbeckerle <gi...@git.apache.org>.
Github user mbeckerle commented on a diff in the pull request:

    https://github.com/apache/incubator-daffodil/pull/5#discussion_r149843822
  
    --- Diff: daffodil-core/src/main/scala/edu/illinois/ncsa/daffodil/dsom/RealTermMixin.scala ---
    @@ -70,10 +70,10 @@ trait PropertyReferencedElementInfosMixin {
     }
     
     /**
    - * A RealTerm is an element or a sequence or a choice. Group references
    - * are excluded as they go away and really have no realization.
    + * A RealTerm is a LocalElementDecl or ElementRef, or
    + * a Sequence or SequenceGroupRef
    + * or a Choice or ChoiceGroupRef
      *
    - * Things that apply to real terms, but not group references, go in this trait.
      */
     trait RealTermMixin { self: Term =>
    --- End diff --
    
    RealTerm can be collapsed into Term now. All Term are RealTerm, and nothing else is, because ElemetnRef and GroupRef are both Terms now.


---

[GitHub] incubator-daffodil pull request #5: Property Resolution - part of fixing sch...

Posted by stevedlawrence <gi...@git.apache.org>.
Github user stevedlawrence commented on a diff in the pull request:

    https://github.com/apache/incubator-daffodil/pull/5#discussion_r150866270
  
    --- Diff: daffodil-core/src/main/scala/edu/illinois/ncsa/daffodil/dsom/Term.scala ---
    @@ -473,14 +446,20 @@ abstract class Term(xmlArg: Node, parentArg: SchemaComponent, val position: Int)
        */
       lazy val childrenInHiddenGroupNotDefaultableOrOVC: Seq[ElementBase] = {
         // this should only be called on hidden elements
    -    Assert.invariant(this.isHidden)
    +    val isH = isHidden
    +    Assert.invariant(isH)
     
         val res = this match {
    -      case s: Sequence => {
    -        s.groupMembersNoRefs.flatMap { _.childrenInHiddenGroupNotDefaultableOrOVC }
    +      case s: SequenceBase => {
    +        s.groupMembers.flatMap { member =>
    +          if (!member.isHidden)
    +            println("Not hidden")
    --- End diff --
    
    Remove println debug.


---

[GitHub] incubator-daffodil pull request #5: Property Resolution - part of fixing sch...

Posted by stevedlawrence <gi...@git.apache.org>.
Github user stevedlawrence commented on a diff in the pull request:

    https://github.com/apache/incubator-daffodil/pull/5#discussion_r150859793
  
    --- Diff: daffodil-core/src/main/scala/edu/illinois/ncsa/daffodil/dsom/Nesting.scala ---
    @@ -62,11 +62,12 @@ trait NestingMixin {
        */
       final lazy val enclosingComponent: Option[SchemaComponent] = {
         val optECD = enclosingComponentDef
    -    optECD match {
    -      case Some(eref: ElementRef) => eref.enclosingComponent
    -      case Some(gref: GroupRef) => gref.enclosingComponent
    +    val res = optECD match {
    +//      case Some(eref: ElementRef) => eref.enclosingComponent
    +//      case Some(gref: GroupRef) => gref.enclosingComponent
    --- End diff --
    
    This match/case is essentially a no-op now.


---

[GitHub] incubator-daffodil pull request #5: Property Resolution - part of fixing sch...

Posted by mbeckerle <gi...@git.apache.org>.
Github user mbeckerle commented on a diff in the pull request:

    https://github.com/apache/incubator-daffodil/pull/5#discussion_r150652871
  
    --- Diff: daffodil-core/src/main/scala/edu/illinois/ncsa/daffodil/dsom/GroupBase.scala ---
    @@ -32,28 +32,26 @@
     
     package edu.illinois.ncsa.daffodil.dsom
     
    -import scala.xml.Node
    -import scala.xml._
     import edu.illinois.ncsa.daffodil.schema.annotation.props.AlignmentType
     import edu.illinois.ncsa.daffodil.schema.annotation.props.gen.AlignmentUnits
     import java.lang.{ Integer => JInt }
     
    -abstract class GroupBase(xmlArg: Node, parentArg: SchemaComponent, position: Int)
    -  extends Term(xmlArg, parentArg, position) {
    +trait GroupBase
    --- End diff --
    
    GroupBase class elminated, but postponing the renames ModelGroup and ElementBase


---

[GitHub] incubator-daffodil pull request #5: Property Resolution - part of fixing sch...

Posted by mbeckerle <gi...@git.apache.org>.
Github user mbeckerle commented on a diff in the pull request:

    https://github.com/apache/incubator-daffodil/pull/5#discussion_r149839418
  
    --- Diff: daffodil-core/src/main/scala/edu/illinois/ncsa/daffodil/dsom/GroupDef.scala ---
    @@ -34,70 +34,137 @@ package edu.illinois.ncsa.daffodil.dsom
     
     import scala.xml.Node
     import edu.illinois.ncsa.daffodil.exceptions.Assert
    -import edu.illinois.ncsa.daffodil.xml.QName
    +import scala.xml.Text
    +import scala.xml.Comment
     
    -class GlobalGroupDefFactory(xmlArg: Node, schemaDocumentArg: SchemaDocument)
    -  extends SchemaComponentFactory(xmlArg, schemaDocumentArg)
    +final class GlobalGroupDefFactory(defXML: Node, schemaDocument: SchemaDocument)
    +  extends SchemaComponentFactory(defXML, schemaDocument)
       with GlobalNonElementComponentMixin {
     
    -  private lazy val trimmedXml = scala.xml.Utility.trim(xmlArg)
    -
    -  def forGroupRef(gref: GroupRef, position: Int) = {
    +  def forGroupRef(refXML: Node, refLexicalParent: SchemaComponent, position: Int,
    +    isHidden: Boolean): (GroupRef, GlobalGroupDef) = {
    +    val trimmedXml = scala.xml.Utility.trim(defXML)
         trimmedXml match {
           case <group>{ contents @ _* }</group> => {
    -        val ggd = contents.collect {
    -          case <sequence>{ _* }</sequence> => new GlobalSequenceGroupDef(xml, schemaDocument, gref, position)
    -          case <choice>{ _* }</choice> => new GlobalChoiceGroupDef(xml, schemaDocument, gref, position)
    +        val list = contents.collect {
    +          case groupXML @ <sequence>{ _* }</sequence> => {
    +            lazy val gref: SequenceGroupRef = new SequenceGroupRef(gdef, refXML, refLexicalParent, position, isHidden)
    +            lazy val gdef: GlobalSequenceGroupDef = new GlobalSequenceGroupDef(defXML, groupXML, schemaDocument, gref)
    +            gref.groupDef
    +            gdef.groupRef
    +            (gref, gdef)
    +          }
    +          case groupXML @ <choice>{ _* }</choice> =>
    +            lazy val gref: ChoiceGroupRef = new ChoiceGroupRef(gdef, refXML, refLexicalParent, position, isHidden)
    +            lazy val gdef: GlobalChoiceGroupDef = new GlobalChoiceGroupDef(defXML, groupXML, schemaDocument, gref)
    +            gref.groupDef
    +            gdef.groupRef
    +            (gref, gdef)
             }
    -        Assert.invariant(ggd.length == 1)
    -        ggd(0)
    +        val res = list(0)
    +        res
           }
           case _ => Assert.invariantFailed("not a group")
         }
       }
    -
    -  override lazy val namedQName = QName.createGlobal(name, targetNamespace, xml.scope)
    -
     }
     
    -sealed abstract class GlobalGroupDef(xmlArg: Node, schemaDocumentArg: SchemaDocument, val groupRef: GroupRef, position: Int)
    -  extends SchemaComponent(xmlArg, schemaDocumentArg)
    -  with GlobalNonElementComponentMixin 
    -  with NestingTraversesToReferenceMixin {
    -
    -  requiredEvaluations(modelGroup)
    -  requiredEvaluations(validateChoiceBranchKey)
    -
    -  final lazy val referringComponent = {
    -    val res = Some(groupRef)
    -    res
    -  }
    +/**
    + * Many things can be contained either by a model group or a
    + * group definition (aka global sequence group definition or global choice
    + * group definition).
    + *
    + * These things are GroupLike.
    --- End diff --
    
    Improve scaladoc, and clarify trait names. Perhaps we should have LocalSequenceDef and GlobalSequenceDef, LocalChoiceDef and GlobalChoiceDef, by analogy to LocalElementDecl and GlobalElementDecl. 


---

[GitHub] incubator-daffodil pull request #5: Property Resolution - part of fixing sch...

Posted by stevedlawrence <gi...@git.apache.org>.
Github user stevedlawrence commented on a diff in the pull request:

    https://github.com/apache/incubator-daffodil/pull/5#discussion_r151133758
  
    --- Diff: daffodil-core/src/main/scala/edu/illinois/ncsa/daffodil/grammar/package.scala ---
    @@ -0,0 +1,104 @@
    +package edu.illinois.ncsa.daffodil
    +
    +/**
    + * Grammar Mixins for Schema Compilation
    + *
    + * DFDL is defined using a data syntax grammar. Daffodil compiles a DFDL schema
    + * into runtime data structures using a technique that tries to be faithful to
    + * this notion of a data syntax grammar. The exact data syntax grammar used
    + * in the DFDL specification is not really suitable to base an implementation on,
    + * but where possible we have named the grammar production rules, and terminals of
    + * the grammar consistently with the DFDL specification.
    + *
    + * The grammar rules are members of the DSOM traits/classes that use a
    + * grammar notation, organized as grammar rules the applicability of which
    + * is controlled by boolean test guard expressions.
    + *
    + * In addition to the grammar rules, these mixins contain the methods that analzye
    + * the DFDL schema with its DFDL annotations, to determine whether the grammar
    + * rule is applicable. Most optimizations in the Daffodil schema compiler are
    + * implemented by way of a grammar rule or [[Terminal]] which is included or
    + * excluded (aka guarded) by an optimization.
    + *
    + * The grammar rule mixins all inherit from the [[Gram]] trait which provides
    + * operators for expressing the rules. The concrete classes are the
    + * terminals of the grammar (instances of [[Terminal]]) and these
    + * are either primitives or combinators. The primitves and combinators are
    + * defined in the [[edu.illinois.ncsa.daffodil.grammar.primitives]] package.
    + *
    + * This all works very much like Scala's `scala.util.parsing.combinator` library,
    + * which is described in the [[http://www.artima.com/pins1ed/ Programming in Scala]] book
    + * in the [[http://www.artima.com/pins1ed/combinator-parsing.html chapter on Combinator Parsing]].
    + * However, Daffodil's grammar adds the notion of rich predicate guards controlling
    + * the rules, and of course the result of the grammar is an entirely different
    + * data structure. But but the way rules are expressed and use of operators
    + * like "`~`" and "`||`" to create a little grammar composition language are similar.
    + *
    + * It is best to illustrate how the grammar works by an example drawn from model groups.
    + * This example no longer matches the code of the actual implementation, but
    + * illustrates the ideas behind the grammar:
    + * @example
    + * {{{
    + * trait ModelGroupMixin extends ... {
    + *
    + * lazy val modelGroup = groupLeftFraming ~ groupContent ~ groupRightFraming
    + *
    + * lazy val groupLeftFraming = LeadingSkipRegion(this) ~ AlignmentFill(this)
    + *
    + * lazy val groupRightFraming = TrailingSkipRegion(this)
    + * }
    + * }}}
    + * Non-terminals of the data syntax grammar are ordinary lazy val members named
    + * beginning with lower case letter. Terminals of the grammar are classes
    + * and so are named beginning with an upper case letter.
    + *
    + * A terminal like `LeadingSkipRegion` will optimize itself out by evaluating a
    + * guard predicate test. Basically, if the DFDL schema has a `dfdl:leadingSkip`
    + * property of '0', then the `LeadingSkipRegion` of the data syntax grammar is zero
    + * width, so this terminal defines itself so that the `isEmpty` method returns
    + * `true`. That enables the "`~`" operator to see that it has an empty grammar term
    + * on it's left, hence, the "`~`" operator can reduce to just the right grammar term.
    + *
    + * The right grammar term, `AlignmentFill`, requires a much more sophisticated
    + * analysis of the schema and properties, but ultimately could also decide that
    + * it is not needed, and so `isEmpty`. Then the "`~`" operator will see that both sides are
    + * empty and it itself is then empty. That allows the modelGroup rule to recognize that
    + * for this DFDL schema there is no `groupLeftFraming`.
    + *
    + * The net result of this is all the grammar regions that are not applicable disappear
    + * from the grammar. What is left is a nest of combinators and primitives suitable
    + * for this specific DFDL schema's runtime to be generated.
    + *
    + * Lazy evaluation and by-name argument passing insure that this occurs without
    + * evaluating any irrelevant grammar rules.
    + *
    + * At the end, the schema compiler generates a parser or unparser instance
    + * by "asking" the now-optimized grammar data structure for a parser or unparser.
    + * This means really by invoking the corresponding
    + * method (`parser()` or `unparser()`) on the grammar object.
    + * Recursively each combinator constructs an instance of
    + * a class that implements
    + * `edu.illinois.ncsa.daffodil.processors.parsers.Parser` (or
    + * `fer `edu.illinois.ncsa.daffodil.processors.unparsers.Unparser`)
    --- End diff --
    
    `fer typo?


---

[GitHub] incubator-daffodil pull request #5: Property Resolution - part of fixing sch...

Posted by mbeckerle <gi...@git.apache.org>.
Github user mbeckerle commented on a diff in the pull request:

    https://github.com/apache/incubator-daffodil/pull/5#discussion_r149840612
  
    --- Diff: daffodil-core/src/main/scala/edu/illinois/ncsa/daffodil/dsom/ModelGroup.scala ---
    @@ -80,17 +96,56 @@ sealed abstract class ModelGroupFactory private () {
     
     object ModelGroupFactory extends ModelGroupFactory()
     
    +sealed abstract class TermFactory private () {
    --- End diff --
    
    scaladoc


---

[GitHub] incubator-daffodil pull request #5: Property Resolution - part of fixing sch...

Posted by mbeckerle <gi...@git.apache.org>.
Github user mbeckerle commented on a diff in the pull request:

    https://github.com/apache/incubator-daffodil/pull/5#discussion_r151015264
  
    --- Diff: daffodil-core/src/main/scala/edu/illinois/ncsa/daffodil/compiler/Compiler.scala ---
    @@ -67,6 +67,7 @@ import edu.illinois.ncsa.daffodil.processors.parsers.NotParsableParser
     import edu.illinois.ncsa.daffodil.processors.unparsers.NotUnparsableUnparser
    --- End diff --
    
    Starting a review


---

[GitHub] incubator-daffodil pull request #5: Property Resolution - part of fixing sch...

Posted by mbeckerle <gi...@git.apache.org>.
Github user mbeckerle commented on a diff in the pull request:

    https://github.com/apache/incubator-daffodil/pull/5#discussion_r149837035
  
    --- Diff: daffodil-core/src/main/scala/edu/illinois/ncsa/daffodil/dsom/AnnotatedSchemaComponent.scala ---
    @@ -41,23 +41,122 @@ import edu.illinois.ncsa.daffodil.equality._
     import edu.illinois.ncsa.daffodil.schema.annotation.props.PropertyLookupResult
     import edu.illinois.ncsa.daffodil.schema.annotation.props.NotFound
     import edu.illinois.ncsa.daffodil.schema.annotation.props.Found
    +import edu.illinois.ncsa.daffodil.schema.annotation.props.FindPropertyMixin
    +
    +/**
    + * Only objects from which we generate processors (parsers/unparsers)
    + * can lookup property values.
    + *
    + * This avoids the possibility of a property being resolved incorrectly by
    + * not looking at the complete chain of schema components contributing to the
    + * property resolution.
    + *
    + * The only objects that should resolve properties are
    + * ElementRef, Root, LocalElementDecl, Sequence, Choice, SequenceRef, ChoiceRef
    + *
    + * These are all the "real" terms. Everything else is just contributing
    + * properties to the mix, but they are not points where properties are
    + * used to generate processors.
    + */
    +trait ResolvesProperties
    +  extends FindPropertyMixin { self: AnnotatedSchemaComponent =>
    +
    +  def term: Term
    +
    +  private def findNonDefaultProperty(pname: String): PropertyLookupResult = {
    +    val result = findDefaultOrNonDefaultProperty(pname, nonDefaultPropertySources)
    +    result match {
    +      case f: Found => f
    +      case NotFound(nd, d, _) =>
    +        Assert.invariant(d.isEmpty)
    +    }
    +    result
    +  }
    +
    +  private def findDefaultProperty(pname: String): PropertyLookupResult = {
    +    val result = findDefaultOrNonDefaultProperty(pname, defaultPropertySources)
    +    val fixup = result match {
    +      case Found(value, loc, pname, _) =>
    +        // found as a default property.
    +        // supply constructor's last arg is boolean indicating it's a default property
    +        Found(value, loc, pname, true)
    +      case NotFound(nd, d, pn) =>
    +        Assert.invariant(d.isEmpty)
    +        NotFound(Seq(), nd, pn) // we want the places we searched shown as default locations searched
    +    }
    +    fixup
    +  }
    +
    +  override def findPropertyOption(pname: String): PropertyLookupResult = {
    +    ExecutionMode.requireCompilerMode
    +    // first try in regular properties
    +    val regularResult = resolver.findNonDefaultProperty(pname)
    +    regularResult match {
    +      case f: Found => f
    +      case NotFound(nonDefaultLocsTried1, defaultLocsTried1, _) => {
    +        Assert.invariant(defaultLocsTried1.isEmpty)
    +        val defaultResult = resolver.findDefaultProperty(pname)
    +        defaultResult match {
    +          case f: Found => f
    +          case NotFound(nonDefaultLocsTried2, defaultLocsTried2, _) => {
    +            Assert.invariant(nonDefaultLocsTried2.isEmpty)
    +            // did not find it at all. Return a NotFound with all the places we
    +            // looked non-default and default.
    +            val nonDefaultPlaces = nonDefaultLocsTried1
    +            val defaultPlaces = defaultLocsTried2
    +            NotFound(nonDefaultPlaces, defaultPlaces, pname)
    +          }
    +        }
    +      }
    +    }
    +  }
    +}
    +
    +abstract class AnnotatedSchemaComponentImpl( final override val xml: Node,
    +  final override val parent: SchemaComponent)
    +  extends AnnotatedSchemaComponent
     
     /**
      * Shared characteristics of any annotated schema component.
      * Not all components can carry DFDL annotations.
      */
    +trait AnnotatedSchemaComponent
    +  extends SchemaComponent
    +  with AnnotatedMixin
    +  with OverlapCheckMixin {
     
    -abstract class AnnotatedSchemaComponent(xml: Node, sc: SchemaComponent)
    -  extends SchemaComponent(xml, sc)
    -  with AnnotatedMixin {
    +  final lazy val term: Term = this match {
    +    case gr: GroupRef => gr.asModelGroup
    +    //    case mg: ModelGroup => mg.parent match {
    +    //      case ggd: GlobalGroupDef => {
    +    //        // this is the model group that is the definition of
    +    //        // a global group, so our term is the group reference referring to this.
    +    //        ggd.groupRef.asModelGroup
    +    //      }
    +    //      case _ => mg
    +    //    }
    +    case t: Term => t
    +    case ged: GlobalElementDecl => ged.elementRef
    +    case ty: SimpleTypeDefBase => ty.elementBase
    +    case ty: ComplexTypeBase => ty.elementBase
    +    case ggd: GlobalGroupDef => ggd.groupRef.asModelGroup
    +    case sd: SchemaDocument =>
    +      Assert.usageError("not to be called for schema documents")
    +  }
    +
    +  final lazy val resolver: ResolvesProperties = {
    +    val res = this match {
    +      case sd: SchemaDocument => sd
    +      case _ => term
    +    }
    +    res
    +  }
     
       requiredEvaluations(annotationObjs)
       requiredEvaluations(shortFormPropertiesCorrect)
       requiredEvaluations(nonDefaultPropertySources)
       requiredEvaluations(defaultPropertySources)
     
    -  def term: Term
    --- End diff --
    
    Add scaladoc. What does term do if the annotated schema component is say, a variable declaration?


---

[GitHub] incubator-daffodil pull request #5: Property Resolution - part of fixing sch...

Posted by mbeckerle <gi...@git.apache.org>.
Github user mbeckerle commented on a diff in the pull request:

    https://github.com/apache/incubator-daffodil/pull/5#discussion_r149841779
  
    --- Diff: daffodil-core/src/main/scala/edu/illinois/ncsa/daffodil/dsom/Term.scala ---
    @@ -48,13 +46,17 @@ import edu.illinois.ncsa.daffodil.schema.annotation.props.gen.NilKind
     /////////////////////////////////////////////////////////////////
     
     // A term is content of a group
    --- End diff --
    
    scaladoc. Term, and what is a term and what isn't a term, is key to all of DSOM.
    
    ElementRef and LocalElementDecl are the two concrete kinds of element Term. GlobalElementDecl is not a Term. 
    
    This was done so that property resolving and statement aggregation can only be done on Term, which then forces one to only obtain properties from Term.
    
    Similarly for groups...
    
    SequenceGroupRef and Sequence are Term, GlobalSequenceGroupDef is not. Similarly for choices.
    Perhaps add dsom package doc explaining this?


---

[GitHub] incubator-daffodil pull request #5: Property Resolution - part of fixing sch...

Posted by VTGuy <gi...@git.apache.org>.
Github user VTGuy commented on a diff in the pull request:

    https://github.com/apache/incubator-daffodil/pull/5#discussion_r151206630
  
    --- Diff: daffodil-core/src/main/scala/edu/illinois/ncsa/daffodil/dsom/ElementBase.scala ---
    @@ -1176,4 +1162,35 @@ trait ElementBase
         }
       }
     
    +  private def gcd(a: Int, b: Int): Int = if (b == 0) a else gcd(b, a % b)
    +
    +  /**
    --- End diff --
    
    Thank you for these comments.


---

[GitHub] incubator-daffodil pull request #5: Property Resolution - part of fixing sch...

Posted by stevedlawrence <gi...@git.apache.org>.
Github user stevedlawrence commented on a diff in the pull request:

    https://github.com/apache/incubator-daffodil/pull/5#discussion_r150861124
  
    --- Diff: daffodil-core/src/main/scala/edu/illinois/ncsa/daffodil/dsom/RealTermMixin.scala ---
    @@ -113,17 +113,19 @@ trait RealTermMixin { self: Term =>
                 case true => oSeq.possibleNextTerms
                 case false => {
     
    -              val members = oSeq.groupMembersNoRefs
    +              val members = oSeq.groupMembers
     
    -              val nextMember =
    -                members.dropWhile(m => m != thisTermNoRefs).filterNot(m => m == thisTermNoRefs).headOption
    +              val selfAndAfter = members.dropWhile(m => m ne this)
    +              val after = selfAndAfter.filterNot(_ eq this)
    --- End diff --
    
    Can this be change to this?
    ```
    val after = selfAndAfter.drop(1)
    ```
    ``this`` shoudl also be the first element in ``selfAndAfter``, and ``this`` shouldn't appear anywhere else in ``selfAndAfter``. Just a little bit more efficient?


---

[GitHub] incubator-daffodil pull request #5: Property Resolution - part of fixing sch...

Posted by mbeckerle <gi...@git.apache.org>.
Github user mbeckerle commented on a diff in the pull request:

    https://github.com/apache/incubator-daffodil/pull/5#discussion_r149845131
  
    --- Diff: daffodil-lib/src/main/scala/edu/illinois/ncsa/daffodil/oolag/OOLAG.scala ---
    @@ -131,18 +131,30 @@ object OOLAG extends Logging {
        * already. This insures that the value exists for 'foo', or any errors/warnings
        * to be determined by its calculation have been recorded.
        */
    -  abstract class OOLAGHost private (oolagContextArg: OOLAGHost, nArgs: Args)
    +  abstract class OOLAGHostImpl private (
    +    final override val oolagContextArg: OOLAGHost,
    +    final override val nArgs: Args)
    +    extends OOLAGHost {
    +
    +    def this(oolagContext: OOLAGHost) = this(oolagContext, OneArg)
    +    def this() = this(null, ZeroArgs)
    +  }
    +
    +  trait OOLAGHost
         extends Logging with WithDiagnostics
         with NamedMixinBase {
     
    +    protected def oolagContextArg: OOLAGHost
    --- End diff --
    
    rename here also. 


---

[GitHub] incubator-daffodil pull request #5: Property Resolution - part of fixing sch...

Posted by stevedlawrence <gi...@git.apache.org>.
Github user stevedlawrence commented on a diff in the pull request:

    https://github.com/apache/incubator-daffodil/pull/5#discussion_r150864544
  
    --- Diff: daffodil-core/src/main/scala/edu/illinois/ncsa/daffodil/dsom/SimpleTypes.scala ---
    @@ -141,25 +150,18 @@ abstract class SimpleTypeDefBase(xmlArg: Node, override val parent: SchemaCompon
         res
       }
     
    +  /**
    +   * Exclusive of self.
    +   */
       final lazy val bases: Seq[SimpleTypeDefBase] =
         if (restrictions.isEmpty) Nil
         else restrictions.tail.map { _.simpleType }
     
    -  private lazy val sTypeNonDefault: Seq[ChainPropProvider] = bases.reverse.map { _.nonDefaultFormatChain }
    -  private lazy val sTypeDefault: Seq[ChainPropProvider] = bases.reverse.map { _.defaultFormatChain }
    -
    -  // want a QueueSet i.e., fifo order if iterated, but duplicates
    -  // kept out of the set. Will simulate by calling distinct.
    -  def nonDefaultPropertySources = LV('nonDefaultPropertySources) {
    -    val seq = (this.nonDefaultFormatChain +: sTypeNonDefault).distinct
    -    checkNonOverlap(seq)
    -    seq
    -  }.value
    +  override final def optReferredToComponent = optRestriction.flatMap { _.optBaseDef }
     
    -  def defaultPropertySources = LV('defaultPropertySources) {
    -    val seq = (this.defaultFormatChain +: sTypeDefault).distinct
    -    seq
    -  }.value
    +  // Keep while Debugging - why are these reverse calls here?? TBD
    +  //  private lazy val sTypeNonDefault: Seq[ChainPropProvider] = bases.reverse.map { _.nonDefaultFormatChain }
    +  //  private lazy val sTypeDefault: Seq[ChainPropProvider] = bases.reverse.map { _.defaultFormatChain }
    --- End diff --
    
    Was this determined? I assume reverse is no longer needed?


---

[GitHub] incubator-daffodil pull request #5: Property Resolution - part of fixing sch...

Posted by mbeckerle <gi...@git.apache.org>.
Github user mbeckerle commented on a diff in the pull request:

    https://github.com/apache/incubator-daffodil/pull/5#discussion_r149838673
  
    --- Diff: daffodil-core/src/main/scala/edu/illinois/ncsa/daffodil/dsom/GroupBase.scala ---
    @@ -64,22 +62,22 @@ abstract class GroupBase(xmlArg: Node, parentArg: SchemaComponent, position: Int
       final lazy val enclosingComponentModelGroup = enclosingComponent.collect { case mg: ModelGroup => mg }
       final lazy val sequencePeers = enclosingComponentModelGroup.map { _.sequenceChildren }
       final lazy val choicePeers = enclosingComponentModelGroup.map { _.choiceChildren }
    -  final lazy val groupRefPeers = enclosingComponentModelGroup.map { _.groupRefChildren }
    +  // final lazy val groupRefPeers = enclosingComponentModelGroup.map { _.groupRefChildren }
     
       protected def myPeers: Option[Seq[GroupBase]]
     
       def group: ModelGroup
     
    -  final lazy val immediateGroup: Option[ModelGroup] = {
    -    val res: Option[ModelGroup] = this.group match {
    -      case (s: Sequence) => Some(s)
    -      case (c: Choice) => Some(c)
    -      case _ => None
    -    }
    -    res
    -  }
    +  //  final lazy val immediateGroup: Option[ModelGroup] = {
    --- End diff --
    
    delete commented code.


---

[GitHub] incubator-daffodil pull request #5: Property Resolution - part of fixing sch...

Posted by stevedlawrence <gi...@git.apache.org>.
Github user stevedlawrence commented on a diff in the pull request:

    https://github.com/apache/incubator-daffodil/pull/5#discussion_r150855476
  
    --- Diff: daffodil-core/src/main/scala/edu/illinois/ncsa/daffodil/dsom/GlobalElementDecl.scala ---
    @@ -43,52 +41,47 @@ import edu.illinois.ncsa.daffodil.grammar._
      */
     final class GlobalElementDecl(
       val factory: GlobalElementDeclFactory,
    -  override val elementRef: Option[ElementRef])
    -  extends LocalElementBase(factory.xml, factory.schemaDocument, 0)
    +  elementRefArg: => AbstractElementRef)
    +  extends AnnotatedSchemaComponent
       with GlobalElementComponentMixin
       with ElementDeclMixin
    -  with GlobalElementDeclGrammarMixin
    -  with NestingTraversesToReferenceMixin
    -  /*
    +  with DFDLStatementMixin
    +  // with ElementDeclMixin
    +  with NestingTraversesToReferenceMixin /*
        * global elements combined with element references referring to them can
        * be multiple occurring (aka arrays) hence, we have to have things
        * that take root and referenced situation into account.
    -   */
    -  with RequiredOptionalMixin {
    -
    -  requiredEvaluations(validateChoiceBranchKey)
    -
    -  override lazy val maxOccurs = elementRef match {
    -    case Some(er) => er.maxOccurs
    -    case None => 1
    -  }
    +   */ // with RequiredOptionalMixin
    +   {
     
    -  override lazy val minOccurs = elementRef match {
    -    case Some(er) => er.minOccurs
    -    case None => 1
    -  }
    -
    -  lazy val isRoot = elementRef == None
    -
    -  override lazy val isHidden = if (isRoot) false else elementRef.get.isHidden
    +  override val xml = factory.xml
    +  override def parent = factory.schemaDocument
     
    -  // final override protected def enclosingComponentDef = elementRef.flatMap { _.enclosingComponent }
    +  lazy val elementRef = elementRefArg
    +  override lazy val dpathCompileInfo = elementRef.dpathElementCompileInfo
     
    -  override lazy val referringComponent: Option[SchemaComponent] = elementRef
    +  requiredEvaluations(validateChoiceBranchKey)
     
    -  // GlobalElementDecls need to have access to elementRef's local properties.
    +  //  override protected def computeElementRuntimeData(): ElementRuntimeData = {
    +  //    optElementRef match {
    +  //      case None => super.computeElementRuntimeData
    +  //      case Some(er) => er.elementRuntimeData
    +  //    }
    +  //  }
     
    -  // We inherit the requirement for these attributes from Term. It all gets
    -  // too complicated in DSOM if you try to make GlobalElementDecl share with the other
    -  // element structures but not be a Term.
    -  //
    -  // But a GlobalElementDecl isn't really a Term except in a degenerate sense
    -  // that the root element is sort of a Term.
    +  //  override lazy val maxOccurs = optElementRef match {
    +  //    case Some(er) => er.maxOccurs
    +  //    case None => 1
    +  //  }
       //
    -  // In other words, we shouldn't be treating this as a term.
    +  //  override lazy val minOccurs = optElementRef match {
    +  //    case Some(er) => er.minOccurs
    +  //    case None => 1
    +  //  }
       //
    +  // final override protected def enclosingComponentDef = optElementRef.flatMap { _.enclosingComponent }
    --- End diff --
    
    Should these comments be removed? They look like they're just old.


---

[GitHub] incubator-daffodil pull request #5: Property Resolution - part of fixing sch...

Posted by mbeckerle <gi...@git.apache.org>.
Github user mbeckerle commented on a diff in the pull request:

    https://github.com/apache/incubator-daffodil/pull/5#discussion_r149846369
  
    --- Diff: daffodil-japi/src/test/java/edu/illinois/ncsa/daffodil/example/TestJavaAPI.java ---
    @@ -58,803 +58,806 @@
     import edu.illinois.ncsa.daffodil.japi.ProcessorFactory;
     import edu.illinois.ncsa.daffodil.japi.UnparseResult;
     import edu.illinois.ncsa.daffodil.japi.ValidationMode;
    +import edu.illinois.ncsa.daffodil.japi.infoset.JDOMInfosetInputter;
    +import edu.illinois.ncsa.daffodil.japi.infoset.JDOMInfosetOutputter;
     import edu.illinois.ncsa.daffodil.japi.logger.ConsoleLogWriter;
     import edu.illinois.ncsa.daffodil.japi.logger.LogLevel;
    -import edu.illinois.ncsa.daffodil.japi.infoset.JDOMInfosetOutputter;
    -import edu.illinois.ncsa.daffodil.japi.infoset.JDOMInfosetInputter;
     
     public class TestJavaAPI {
     
    -	public java.io.File getResource(String resPath) {
    -		try {
    -			return new java.io.File(this.getClass().getResource(resPath).toURI());
    -		} catch (Exception e) {
    -			return null;
    -		}
    -	}
    -
    -	@Test
    -	public void testJavaAPI1() throws IOException {
    -		LogWriterForJAPITest lw = new LogWriterForJAPITest();
    -		DebuggerRunnerForJAPITest debugger = new DebuggerRunnerForJAPITest();
    -
    -		Daffodil.setLogWriter(lw);
    -		Daffodil.setLoggingLevel(LogLevel.Debug);
    -
    -		edu.illinois.ncsa.daffodil.japi.Compiler c = Daffodil.compiler();
    -		c.setValidateDFDLSchemas(false);
    -		java.io.File schemaFile = getResource("/test/japi/mySchema1.dfdl.xsd");
    -		ProcessorFactory pf = c.compileFile(schemaFile);
    -		DataProcessor dp = pf.onPath("/");
    -		dp.setDebugger(debugger);
    -		dp.setDebugging(true);
    -		java.io.File file = getResource("/test/japi/myData.dat");
    -		java.io.FileInputStream fis = new java.io.FileInputStream(file);
    -		java.nio.channels.ReadableByteChannel rbc = java.nio.channels.Channels.newChannel(fis);
    -		JDOMInfosetOutputter outputter = new JDOMInfosetOutputter();
    -		ParseResult res = dp.parse(rbc, outputter, 2 << 3);
    -		boolean err = res.isError();
    -		assertFalse(err);
    -		assertTrue(res.location().isAtEnd());
    -		assertEquals(0, lw.errors.size());
    -		assertEquals(0, lw.warnings.size());
    -		assertTrue(lw.others.size() > 0);
    -		assertTrue(debugger.lines.size() > 0);
    -		assertTrue(debugger.lines.contains("----------------------------------------------------------------- 1\n"));
    -		assertTrue(debugger.getCommand().equals("trace"));
    -
    -		java.io.ByteArrayOutputStream bos = new java.io.ByteArrayOutputStream();
    -		java.nio.channels.WritableByteChannel wbc = java.nio.channels.Channels.newChannel(bos);
    -		JDOMInfosetInputter inputter = new JDOMInfosetInputter(outputter.getResult());
    -		UnparseResult res2 = dp.unparse(inputter, wbc);
    -		err = res2.isError();
    -		assertFalse(err);
    -		assertEquals("42", bos.toString());
    -
    -		// reset the global logging and debugger state
    -		Daffodil.setLogWriter(new ConsoleLogWriter());
    -		Daffodil.setLoggingLevel(LogLevel.Info);
    -	}
    -
    -	// This is a duplicate of test testJavaAPI1 that serializes the parser
    -	// before executing the test.
    -	@Test
    -	public void testJavaAPI1_A() throws Exception {
    -		LogWriterForJAPITest lw = new LogWriterForJAPITest();
    -		DebuggerRunnerForJAPITest debugger = new DebuggerRunnerForJAPITest();
    -
    -		Daffodil.setLogWriter(lw);
    -		Daffodil.setLoggingLevel(LogLevel.Debug);
    -
    -		edu.illinois.ncsa.daffodil.japi.Compiler c = Daffodil.compiler();
    -		c.setValidateDFDLSchemas(false);
    -		java.io.File schemaFile = getResource("/test/japi/mySchema1.dfdl.xsd");
    -		ProcessorFactory pf = c.compileFile(schemaFile);
    -		DataProcessor dp = pf.onPath("/");
    -
    -		// Serialize the parser to memory, then deserialize for parsing.
    -		ByteArrayOutputStream os = new ByteArrayOutputStream();
    -		WritableByteChannel output = Channels.newChannel(os);
    -		dp.save(output);
    -
    -		ByteArrayInputStream is = new ByteArrayInputStream(os.toByteArray());
    -		ReadableByteChannel input = Channels.newChannel(is);
    -		edu.illinois.ncsa.daffodil.japi.Compiler compiler = Daffodil.compiler();
    -		DataProcessor parser = compiler.reload(input);
    -		parser.setDebugger(debugger);
    -		parser.setDebugging(true);
    -
    -		java.io.File file = getResource("/test/japi/myData.dat");
    -		java.io.FileInputStream fis = new java.io.FileInputStream(file);
    -		java.nio.channels.ReadableByteChannel rbc = java.nio.channels.Channels.newChannel(fis);
    -		JDOMInfosetOutputter outputter = new JDOMInfosetOutputter();
    -		ParseResult res = parser.parse(rbc, outputter, 2 << 3);
    -		boolean err = res.isError();
    -		assertFalse(err);
    -		assertTrue(res.location().isAtEnd());
    -		assertEquals(0, lw.errors.size());
    -		assertEquals(0, lw.warnings.size());
    -		assertTrue(lw.others.size() > 0);
    -		assertTrue(debugger.lines.size() > 0);
    -		assertTrue(debugger.lines.contains("----------------------------------------------------------------- 1\n"));
    -		assertTrue(debugger.getCommand().equals("trace"));
    -
    -		java.io.ByteArrayOutputStream bos = new java.io.ByteArrayOutputStream();
    -		java.nio.channels.WritableByteChannel wbc = java.nio.channels.Channels.newChannel(bos);
    -		JDOMInfosetInputter inputter = new JDOMInfosetInputter(outputter.getResult());
    -		UnparseResult res2 = dp.unparse(inputter, wbc);
    -		err = res2.isError();
    -		assertFalse(err);
    -		assertEquals("42", bos.toString());
    -
    -		// reset the global logging and debugger state
    -		Daffodil.setLogWriter(new ConsoleLogWriter());
    -		Daffodil.setLoggingLevel(LogLevel.Info);
    -	}
    -
    -	// This is a duplicate of test testJavaAPI1 that serializes the parser
    -	// before executing the test.
    -	@Test
    -	public void testJavaAPI1_A_FullFails() throws Exception {
    -		LogWriterForJAPITest lw = new LogWriterForJAPITest();
    -		DebuggerRunnerForJAPITest debugger = new DebuggerRunnerForJAPITest();
    -
    -		Daffodil.setLogWriter(lw);
    -		Daffodil.setLoggingLevel(LogLevel.Debug);
    -
    -		edu.illinois.ncsa.daffodil.japi.Compiler c = Daffodil.compiler();
    -		c.setValidateDFDLSchemas(false);
    -		java.io.File schemaFile = getResource("/test/japi/mySchema1.dfdl.xsd");
    -		ProcessorFactory pf = c.compileFile(schemaFile);
    -		DataProcessor dp = pf.onPath("/");
    -		dp.setDebugger(debugger);
    -		dp.setDebugging(true);
    -
    -		// Serialize the parser to memory, then deserialize for parsing.
    -		ByteArrayOutputStream os = new ByteArrayOutputStream();
    -		WritableByteChannel output = Channels.newChannel(os);
    -		dp.save(output);
    -
    -		ByteArrayInputStream is = new ByteArrayInputStream(os.toByteArray());
    -		ReadableByteChannel input = Channels.newChannel(is);
    -		edu.illinois.ncsa.daffodil.japi.Compiler compiler = Daffodil.compiler();
    -		DataProcessor parser = compiler.reload(input);
    -
    -		try {
    -			parser.setValidationMode(ValidationMode.Full);
    -			fail();
    -		} catch (InvalidUsageException e) {
    -			assertEquals("'Full' validation not allowed when using a restored parser.", e.getMessage());
    -		}
    -
    -		// reset the global logging and debugger state
    -		Daffodil.setLogWriter(new ConsoleLogWriter());
    -		Daffodil.setLoggingLevel(LogLevel.Info);
    -	}
    -
    -	@Test
    -	public void testJavaAPI2() throws IOException {
    -		LogWriterForJAPITest lw = new LogWriterForJAPITest();
    -
    -		Daffodil.setLogWriter(lw);
    -		Daffodil.setLoggingLevel(LogLevel.Info);
    -
    -		edu.illinois.ncsa.daffodil.japi.Compiler c = Daffodil.compiler();
    -		c.setValidateDFDLSchemas(false);
    -		java.io.File schemaFile = getResource("/test/japi/mySchema1.dfdl.xsd");
    -		ProcessorFactory pf = c.compileFile(schemaFile);
    -		DataProcessor dp = pf.onPath("/");
    -		java.io.File file = getResource("/test/japi/myDataBroken.dat");
    -		java.io.FileInputStream fis = new java.io.FileInputStream(file);
    -		java.nio.channels.ReadableByteChannel rbc = java.nio.channels.Channels.newChannel(fis);
    -		JDOMInfosetOutputter outputter = new JDOMInfosetOutputter();
    -		ParseResult res = dp.parse(rbc, outputter);
    -
    -		// TODO: NEED a java friendly way to get the status of the outputter. Scala enums don't work well
    -		//assertTrue(outputter.getStatus() != Status.DONE); // This is a hack so that Java doesn't have to know about Nope/Maybe, need to figure out better api that is Java compatible
    -		assertTrue(res.isError());
    -		java.util.List<Diagnostic> diags = res.getDiagnostics();
    -		assertEquals(1, diags.size());
    -		Diagnostic d = diags.get(0);
    -		// System.err.println(d.getMessage());
    -		assertTrue(d.getMessage().contains("int"));
    -		assertTrue(d.getMessage().contains("Not an int"));
    -		assertTrue(d.getDataLocations().toString().contains("10"));
    -		java.util.List<LocationInSchemaFile> locs = d.getLocationsInSchemaFiles();
    -		assertEquals(1, locs.size());
    -		LocationInSchemaFile loc = locs.get(0);
    -		assertTrue(loc.toString().contains("mySchema2.dfdl.xsd"));
    -
    -		assertEquals(0, lw.errors.size());
    -		assertEquals(0, lw.warnings.size());
    -		// assertTrue(lw.infos.size() > 0);
    -		assertEquals(0, lw.others.size());
    -
    -		// reset the global logging state
    -		Daffodil.setLogWriter(new ConsoleLogWriter());
    -		Daffodil.setLoggingLevel(LogLevel.Info);
    -	}
    -
    -	/**
    -	 * Verify that we can detect when the parse did not consume all the data.
    -	 * 
    -	 * @throws IOException
    -	 */
    -	@Test
    -	public void testJavaAPI3() throws IOException {
    -		edu.illinois.ncsa.daffodil.japi.Compiler c = Daffodil.compiler();
    -		c.setValidateDFDLSchemas(false);
    -		java.io.File schemaFile = getResource("/test/japi/mySchema3.dfdl.xsd");
    -		ProcessorFactory pf = c.compileFile(schemaFile);
    -		pf.setDistinguishedRootNode("e3", null);
    -		DataProcessor dp = pf.onPath("/");
    -		java.io.File file = getResource("/test/japi/myData16.dat");
    -		java.io.FileInputStream fis = new java.io.FileInputStream(file);
    -		java.nio.channels.ReadableByteChannel rbc = java.nio.channels.Channels.newChannel(fis);
    -		JDOMInfosetOutputter outputter = new JDOMInfosetOutputter();
    -		ParseResult res = dp.parse(rbc, outputter, 16 << 3);
    -		boolean err = res.isError();
    -		assertFalse(err);
    -		assertFalse(res.location().isAtEnd());
    -		assertEquals(2, res.location().bytePos1b());
    -		assertEquals(9, res.location().bitPos1b());
    -
    -		java.io.ByteArrayOutputStream bos = new java.io.ByteArrayOutputStream();
    -		java.nio.channels.WritableByteChannel wbc = java.nio.channels.Channels.newChannel(bos);
    -		JDOMInfosetInputter inputter = new JDOMInfosetInputter(outputter.getResult());
    -		UnparseResult res2 = dp.unparse(inputter, wbc);
    -		err = res2.isError();
    -		assertFalse(err);
    -		assertEquals("9", bos.toString());
    -	}
    -
    -	// This is a duplicate of test testJavaAPI3 that serializes the parser
    -	// before executing the test.
    -	@Test
    -	public void testJavaAPI3_A() throws Exception {
    -		edu.illinois.ncsa.daffodil.japi.Compiler c = Daffodil.compiler();
    -		c.setValidateDFDLSchemas(false);
    -		java.io.File schemaFile = getResource("/test/japi/mySchema3.dfdl.xsd");
    -		ProcessorFactory pf = c.compileFile(schemaFile);
    -		pf.setDistinguishedRootNode("e3", null);
    -		DataProcessor dp = pf.onPath("/");
    -
    -		// Serialize the parser to memory, then deserialize for parsing.
    -		ByteArrayOutputStream os = new ByteArrayOutputStream();
    -		WritableByteChannel output = Channels.newChannel(os);
    -		dp.save(output);
    -
    -		ByteArrayInputStream is = new ByteArrayInputStream(os.toByteArray());
    -		ReadableByteChannel input = Channels.newChannel(is);
    -		edu.illinois.ncsa.daffodil.japi.Compiler compiler = Daffodil.compiler();
    -		DataProcessor parser = compiler.reload(input);
    -
    -		java.io.File file = getResource("/test/japi/myData16.dat");
    -		java.io.FileInputStream fis = new java.io.FileInputStream(file);
    -		java.nio.channels.ReadableByteChannel rbc = java.nio.channels.Channels.newChannel(fis);
    -		JDOMInfosetOutputter outputter = new JDOMInfosetOutputter();
    -		ParseResult res = parser.parse(rbc, outputter, 16 << 3);
    -		boolean err = res.isError();
    -		assertFalse(err);
    -		assertFalse(res.location().isAtEnd());
    -		assertEquals(2, res.location().bytePos1b());
    -		assertEquals(9, res.location().bitPos1b());
    -
    -		java.io.ByteArrayOutputStream bos = new java.io.ByteArrayOutputStream();
    -		java.nio.channels.WritableByteChannel wbc = java.nio.channels.Channels.newChannel(bos);
    -		JDOMInfosetInputter inputter = new JDOMInfosetInputter(outputter.getResult());
    -		UnparseResult res2 = dp.unparse(inputter, wbc);
    -		err = res2.isError();
    -		assertFalse(err);
    -		assertEquals("9", bos.toString());
    -	}
    -
    -	@Test
    -	public void testJavaAPI4b() throws IOException {
    -		edu.illinois.ncsa.daffodil.japi.Compiler c = Daffodil.compiler();
    -		c.setValidateDFDLSchemas(false);
    -		File schemaFileName = getResource("/test/japi/mySchema3.dfdl.xsd");
    -		c.setDistinguishedRootNode("e4", null);
    -		ProcessorFactory pf = c.compileFile(schemaFileName);
    -		DataProcessor dp = pf.onPath("/");
    -		java.io.File file = getResource("/test/japi/myData2.dat");
    -		java.io.FileInputStream fis = new java.io.FileInputStream(file);
    -		java.nio.channels.ReadableByteChannel rbc = java.nio.channels.Channels.newChannel(fis);
    -		JDOMInfosetOutputter outputter = new JDOMInfosetOutputter();
    -		ParseResult res = dp.parse(rbc, outputter, 64 << 3);
    -		boolean err = res.isError();
    -		assertFalse(err);
    -		assertFalse(res.location().isAtEnd());
    -		assertEquals(5, res.location().bytePos1b());
    -		assertEquals(33, res.location().bitPos1b());
    -
    -		java.io.ByteArrayOutputStream bos = new java.io.ByteArrayOutputStream();
    -		java.nio.channels.WritableByteChannel wbc = java.nio.channels.Channels.newChannel(bos);
    -		JDOMInfosetInputter inputter = new JDOMInfosetInputter(outputter.getResult());
    -		UnparseResult res2 = dp.unparse(inputter, wbc);
    -		err = res2.isError();
    -		assertFalse(err);
    -		assertEquals("data", bos.toString());
    -	}
    -
    -	@Test
    -	public void testJavaAPI5() throws IOException {
    -		edu.illinois.ncsa.daffodil.japi.Compiler c = Daffodil.compiler();
    -		c.setValidateDFDLSchemas(false);
    -		File schemaFileName = getResource("/test/japi/mySchema3.dfdl.xsd");
    -		c.setDistinguishedRootNode("e4", null); // e4 is a 4-byte long string
    -												// element
    -		ProcessorFactory pf = c.compileFile(schemaFileName);
    -		DataProcessor dp = pf.onPath("/");
    -		java.io.File file = getResource("/test/japi/myData3.dat"); // contains 5
    -																	// bytes
    -		java.io.FileInputStream fis = new java.io.FileInputStream(file);
    -		java.nio.channels.ReadableByteChannel rbc = java.nio.channels.Channels.newChannel(fis);
    -		JDOMInfosetOutputter outputter = new JDOMInfosetOutputter();
    -		ParseResult res = dp.parse(rbc, outputter, 4 << 3);
    -		boolean err = res.isError();
    -		assertFalse(err);
    -		assertTrue("Assertion failed: End of data not reached.", res.location().isAtEnd());
    -		assertEquals(5, res.location().bytePos1b());
    -		assertEquals(33, res.location().bitPos1b());
    -
    -		java.io.ByteArrayOutputStream bos = new java.io.ByteArrayOutputStream();
    -		java.nio.channels.WritableByteChannel wbc = java.nio.channels.Channels.newChannel(bos);
    -		JDOMInfosetInputter inputter = new JDOMInfosetInputter(outputter.getResult());
    -		UnparseResult res2 = dp.unparse(inputter, wbc);
    -		err = res2.isError();
    -		assertFalse(err);
    -		assertEquals("data", bos.toString());
    -	}
    -
    -	/***
    -	 * Verify that the compiler throws a FileNotFound exception when fed a list
    -	 * of schema files that do not exist.
    -	 * 
    -	 * @throws IOException
    -	 */
    -	@Test
    -	public void testJavaAPI6() throws IOException {
    -		LogWriterForJAPITest lw = new LogWriterForJAPITest();
    -
    -		Daffodil.setLogWriter(lw);
    -		Daffodil.setLoggingLevel(LogLevel.Debug);
    -
    -		edu.illinois.ncsa.daffodil.japi.Compiler c = Daffodil.compiler();
    -		c.setValidateDFDLSchemas(false);
    -		java.io.File schemaFile = new java.io.File("/test/japi/notHere1.dfdl.xsd");
    -		ProcessorFactory pf = c.compileFile(schemaFile);
    -		assertTrue(pf.isError());
    -		List<Diagnostic> diags = pf.getDiagnostics();
    -		boolean found1 = false;
    -		for (Diagnostic d : diags) {
    -			if (d.getMessage().contains("notHere1")) {
    -				found1 = true;
    -			}
    -		}
    -		assertTrue(found1);
    -
    -		// reset the global logging state
    -		Daffodil.setLogWriter(new ConsoleLogWriter());
    -		Daffodil.setLoggingLevel(LogLevel.Info);
    -	}
    -
    -	/**
    -	 * Tests a user submitted case where the XML appears to be serializing odd
    -	 * xml entities into the output.
    -	 * 
    -	 * @throws IOException
    -	 */
    -	@Test
    -	public void testJavaAPI7() throws IOException {
    -		// TODO: This is due to the fact that we are doing several conversions
    -		// back and forth between Scala.xml.Node and JDOM. And the conversions
    -		// both use XMLOutputter to format the result (which escapes the
    -		// entities).
    -		LogWriterForJAPITest lw = new LogWriterForJAPITest();
    -
    -		Daffodil.setLogWriter(lw);
    -		Daffodil.setLoggingLevel(LogLevel.Debug);
    -
    -		edu.illinois.ncsa.daffodil.japi.Compiler c = Daffodil.compiler();
    -		c.setValidateDFDLSchemas(false);
    -		java.io.File schemaFile = getResource("/test/japi/TopLevel.xsd");
    -		c.setDistinguishedRootNode("TopLevel", null);
    -		ProcessorFactory pf = c.compileFile(schemaFile);
    -		DataProcessor dp = pf.onPath("/");
    -		java.io.File file = getResource("/test/japi/01very_simple.txt");
    -		java.io.FileInputStream fis = new java.io.FileInputStream(file);
    -		java.nio.channels.ReadableByteChannel rbc = java.nio.channels.Channels.newChannel(fis);
    -		JDOMInfosetOutputter outputter = new JDOMInfosetOutputter();
    -		ParseResult res = dp.parse(rbc, outputter);
    -		boolean err = res.isError();
    -		assertFalse(err);
    -		assertTrue(res.location().isAtEnd());
    -
    -		java.io.ByteArrayOutputStream bos = new java.io.ByteArrayOutputStream();
    -		java.nio.channels.WritableByteChannel wbc = java.nio.channels.Channels.newChannel(bos);
    -		JDOMInfosetInputter inputter = new JDOMInfosetInputter(outputter.getResult());
    -		UnparseResult res2 = dp.unparse(inputter, wbc);
    -		err = res2.isError();
    -		assertFalse(err);
    -		assertTrue(bos.toString().contains("Return-Path: <bo...@smith.com>"));
    -
    -		// reset the global logging state
    -		Daffodil.setLogWriter(new ConsoleLogWriter());
    -		Daffodil.setLoggingLevel(LogLevel.Info);
    -	}
    -
    -	/**
    -	 * This test is nearly identical to testJavaAPI7. The only difference is
    -	 * that this test uses double newline as a terminator for the first element
    -	 * in the sequence rather than double newline as a separator for the
    -	 * sequence
    -	 * 
    -	 * @throws IOException
    -	 */
    -	@Test
    -	public void testJavaAPI8() throws IOException {
    -		LogWriterForJAPITest lw = new LogWriterForJAPITest();
    -
    -		Daffodil.setLogWriter(lw);
    -		Daffodil.setLoggingLevel(LogLevel.Debug);
    -
    -		edu.illinois.ncsa.daffodil.japi.Compiler c = Daffodil.compiler();
    -		c.setValidateDFDLSchemas(false);
    -		java.io.File schemaFile = getResource("/test/japi/TopLevel.xsd");
    -		c.setDistinguishedRootNode("TopLevel2", null);
    -		ProcessorFactory pf = c.compileFile(schemaFile);
    -		DataProcessor dp = pf.onPath("/");
    -		java.io.File file = getResource("/test/japi/01very_simple.txt");
    -		java.io.FileInputStream fis = new java.io.FileInputStream(file);
    -		java.nio.channels.ReadableByteChannel rbc = java.nio.channels.Channels.newChannel(fis);
    -		JDOMInfosetOutputter outputter = new JDOMInfosetOutputter();
    -		ParseResult res = dp.parse(rbc, outputter);
    -		boolean err = res.isError();
    -		assertFalse(err);
    -		assertTrue(res.location().isAtEnd());
    -
    -		java.io.ByteArrayOutputStream bos = new java.io.ByteArrayOutputStream();
    -		java.nio.channels.WritableByteChannel wbc = java.nio.channels.Channels.newChannel(bos);
    -		JDOMInfosetInputter inputter = new JDOMInfosetInputter(outputter.getResult());
    -		UnparseResult res2 = dp.unparse(inputter, wbc);
    -		err = res2.isError();
    -		assertFalse(err);
    -		assertTrue(bos.toString().contains("Return-Path: <bo...@smith.com>"));
    -
    -
    -		// reset the global logging state
    -		Daffodil.setLogWriter(new ConsoleLogWriter());
    -		Daffodil.setLoggingLevel(LogLevel.Info);
    -	}
    -
    -	/**
    -	 * Verify that calling result() on the ParseResult multiple times does not
    -	 * error.
    -	 */
    -	@Test
    -	public void testJavaAPI9() throws IOException {
    -		LogWriterForJAPITest lw = new LogWriterForJAPITest();
    -
    -		Daffodil.setLogWriter(lw);
    -		Daffodil.setLoggingLevel(LogLevel.Debug);
    -
    -		edu.illinois.ncsa.daffodil.japi.Compiler c = Daffodil.compiler();
    -		c.setValidateDFDLSchemas(false);
    -		java.io.File schemaFile = getResource("/test/japi/TopLevel.xsd");
    -		c.setDistinguishedRootNode("TopLevel2", null);
    -		ProcessorFactory pf = c.compileFile(schemaFile);
    -		DataProcessor dp = pf.onPath("/");
    -		java.io.File file = getResource("/test/japi/01very_simple.txt");
    -		java.io.FileInputStream fis = new java.io.FileInputStream(file);
    -		java.nio.channels.ReadableByteChannel rbc = java.nio.channels.Channels.newChannel(fis);
    -		JDOMInfosetOutputter outputter = new JDOMInfosetOutputter();
    -		ParseResult res = dp.parse(rbc, outputter);
    -		boolean err = res.isError();
    -		assertFalse(err);
    -		assertTrue(res.location().isAtEnd());
    -
    -		org.jdom2.Document doc1 = outputter.getResult();
    -
    -		java.io.ByteArrayOutputStream bos1 = new java.io.ByteArrayOutputStream();
    -		java.nio.channels.WritableByteChannel wbc1 = java.nio.channels.Channels.newChannel(bos1);
    -		JDOMInfosetInputter inputter1 = new JDOMInfosetInputter(doc1);
    -		UnparseResult res2 = dp.unparse(inputter1, wbc1);
    -		err = res2.isError();
    -		assertFalse(err);
    -		assertTrue(bos1.toString().contains("Return-Path: <bo...@smith.com>"));
    -
    -		org.jdom2.Document doc2 = outputter.getResult();
    -
    -		java.io.ByteArrayOutputStream bos2 = new java.io.ByteArrayOutputStream();
    -		java.nio.channels.WritableByteChannel wbc2 = java.nio.channels.Channels.newChannel(bos2);
    -		JDOMInfosetInputter inputter2 = new JDOMInfosetInputter(doc2);
    -		UnparseResult res3 = dp.unparse(inputter2, wbc2);
    -		err = res3.isError();
    -		assertFalse(err);
    -		assertTrue(bos2.toString().contains("Return-Path: <bo...@smith.com>"));
    -
    -		// reset the global logging state
    -		Daffodil.setLogWriter(new ConsoleLogWriter());
    -		Daffodil.setLoggingLevel(LogLevel.Info);
    -	}
    -
    -	/**
    -	 * Verify that hidden elements do not appear in the resulting infoset
    -	 */
    -	@Test
    -	public void testJavaAPI10() throws IOException {
    -
    -		edu.illinois.ncsa.daffodil.japi.Compiler c = Daffodil.compiler();
    -		c.setValidateDFDLSchemas(false);
    -		java.io.File schemaFile = getResource("/test/japi/mySchema4.dfdl.xsd");
    -		ProcessorFactory pf = c.compileFile(schemaFile);
    -		DataProcessor dp = pf.onPath("/");
    -		java.io.File file = getResource("/test/japi/myData4.dat");
    -		java.io.FileInputStream fis = new java.io.FileInputStream(file);
    -		java.nio.channels.ReadableByteChannel rbc = java.nio.channels.Channels.newChannel(fis);
    -		JDOMInfosetOutputter outputter = new JDOMInfosetOutputter();
    -		ParseResult res = dp.parse(rbc, outputter);
    -		boolean err = res.isError();
    -		assertFalse(err);
    -		org.jdom2.Document doc = outputter.getResult();
    -		org.jdom2.output.XMLOutputter xo = new org.jdom2.output.XMLOutputter();
    -		xo.setFormat(Format.getPrettyFormat());
    -		org.jdom2.Element rootNode = doc.getRootElement();
    -		org.jdom2.Element hidden = rootNode.getChild("hiddenElement", rootNode.getNamespace());
    -		assertTrue(null == hidden);
    -		assertTrue(res.location().isAtEnd());
    -	}
    -
    -	/**
    -	 * Verify that nested elements do not appear as duplicates
    -	 */
    -	@Test
    -	public void testJavaAPI11() throws IOException {
    -
    -		edu.illinois.ncsa.daffodil.japi.Compiler c = Daffodil.compiler();
    -		c.setValidateDFDLSchemas(false);
    -		java.io.File schemaFile = getResource("/test/japi/mySchema5.dfdl.xsd");
    -		ProcessorFactory pf = c.compileFile(schemaFile);
    -		DataProcessor dp = pf.onPath("/");
    -		java.io.File file = getResource("/test/japi/myData5.dat");
    -		java.io.FileInputStream fis = new java.io.FileInputStream(file);
    -		java.nio.channels.ReadableByteChannel rbc = java.nio.channels.Channels.newChannel(fis);
    -		JDOMInfosetOutputter outputter = new JDOMInfosetOutputter();
    -		ParseResult res = dp.parse(rbc, outputter);
    -		boolean err = res.isError();
    -		assertFalse(err);
    -		org.jdom2.Document doc = outputter.getResult();
    -		org.jdom2.output.XMLOutputter xo = new org.jdom2.output.XMLOutputter();
    -		xo.setFormat(Format.getPrettyFormat());
    -		// xo.output(doc, System.out);
    -		org.jdom2.Element rootNode = doc.getRootElement();
    -		org.jdom2.Element elementGroup = rootNode.getChild("elementGroup", null); // local
    -																					// element
    -																					// names
    -																					// are
    -																					// unqualified
    -		assertTrue(null != elementGroup);
    -		org.jdom2.Element groupE2 = elementGroup.getChild("e2", null);
    -		assertTrue(null != groupE2);
    -		org.jdom2.Element groupE3 = elementGroup.getChild("e3", null);
    -		assertTrue(null != groupE3);
    -		org.jdom2.Element rootE2 = rootNode.getChild("e2", null);
    -		assertTrue(null == rootE2);
    -		org.jdom2.Element rootE3 = rootNode.getChild("e3", null);
    -		assertTrue(null == rootE3);
    -		assertTrue(res.location().isAtEnd());
    -	}
    -
    -	@Test
    -	public void testJavaAPI12() throws IOException {
    -		LogWriterForJAPITest2 lw2 = new LogWriterForJAPITest2();
    -		DebuggerRunnerForJAPITest debugger = new DebuggerRunnerForJAPITest();
    -
    -		Daffodil.setLogWriter(lw2);
    -		Daffodil.setLoggingLevel(LogLevel.Debug);
    -
    -		edu.illinois.ncsa.daffodil.japi.Compiler c = Daffodil.compiler();
    -		c.setValidateDFDLSchemas(false);
    -
    -		java.io.File schemaFile = getResource("/test/japi/mySchema1.dfdl.xsd");
    -		ProcessorFactory pf = c.compileFile(schemaFile);
    -		DataProcessor dp = pf.onPath("/");
    -		dp.setDebugger(debugger);
    -		dp.setDebugging(true);
    -
    -		java.io.File file = getResource("/test/japi/myData.dat");
    -		java.io.FileInputStream fis = new java.io.FileInputStream(file);
    -		java.nio.channels.ReadableByteChannel rbc = java.nio.channels.Channels.newChannel(fis);
    -		JDOMInfosetOutputter outputter = new JDOMInfosetOutputter();
    -		ParseResult res = dp.parse(rbc, outputter, 2 << 3);
    -		boolean err = res.isError();
    -		assertFalse(err);
    -		assertTrue(res.location().isAtEnd());
    -
    -		assertEquals(0, lw2.errors.size());
    -		assertEquals(0, lw2.warnings.size());
    -		assertTrue(lw2.others.size() > 0);
    -		assertTrue(debugger.lines.size() > 0);
    -		assertTrue(debugger.lines.contains("----------------------------------------------------------------- 1\n"));
    -
    -		// reset the global logging and debugger state
    -		Daffodil.setLogWriter(new ConsoleLogWriter());
    -		Daffodil.setLoggingLevel(LogLevel.Info);
    -	}
    -
    -	@Test
    -	public void testJavaAPI13() throws IOException {
    -		// Demonstrates here that we can set external variables
    -		// after compilation but before parsing via Compiler.
    -		LogWriterForJAPITest lw = new LogWriterForJAPITest();
    -		DebuggerRunnerForJAPITest debugger = new DebuggerRunnerForJAPITest();
    -
    -		Daffodil.setLogWriter(lw);
    -		Daffodil.setLoggingLevel(LogLevel.Debug);
    -
    -		edu.illinois.ncsa.daffodil.japi.Compiler c = Daffodil.compiler();
    -		c.setValidateDFDLSchemas(false);
    -		java.io.File extVarsFile = getResource("/test/japi/external_vars_1.xml");
    -		java.io.File schemaFile = getResource("/test/japi/mySchemaWithVars.dfdl.xsd");
    -		c.setExternalDFDLVariables(extVarsFile);
    -		ProcessorFactory pf = c.compileFile(schemaFile);
    -
    -		DataProcessor dp = pf.onPath("/");
    -		dp.setDebugger(debugger);
    -		dp.setDebugging(true);
    -
    -		java.io.File file = getResource("/test/japi/myData.dat");
    -		java.io.FileInputStream fis = new java.io.FileInputStream(file);
    -		java.nio.channels.ReadableByteChannel rbc = java.nio.channels.Channels.newChannel(fis);
    -		JDOMInfosetOutputter outputter = new JDOMInfosetOutputter();
    -		ParseResult res = dp.parse(rbc, outputter, 2 << 3);
    -		boolean err = res.isError();
    -		assertFalse(err);
    -		org.jdom2.Document doc = outputter.getResult();
    -		org.jdom2.output.XMLOutputter xo = new org.jdom2.output.XMLOutputter();
    -		xo.setFormat(Format.getPrettyFormat());
    -		String docString = xo.outputString(doc);
    -		boolean containsVar1 = docString.contains("var1Value");
    -		boolean containsVar1Value = docString.contains("externallySet");
    -		assertTrue(containsVar1);
    -		assertTrue(containsVar1Value);
    -
    -		// reset the global logging and debugger state
    -		Daffodil.setLogWriter(new ConsoleLogWriter());
    -		Daffodil.setLoggingLevel(LogLevel.Info);
    -	}
    -
    -	@Test
    -	public void testJavaAPI14() throws IOException {
    -		// Demonstrates here that we can set external variables
    -		// after compilation but before parsing via DataProcessor.
    -		LogWriterForJAPITest lw = new LogWriterForJAPITest();
    -		DebuggerRunnerForJAPITest debugger = new DebuggerRunnerForJAPITest();
    -
    -		Daffodil.setLogWriter(lw);
    -		Daffodil.setLoggingLevel(LogLevel.Debug);
    -
    -		edu.illinois.ncsa.daffodil.japi.Compiler c = Daffodil.compiler();
    -		c.setValidateDFDLSchemas(false);
    -		java.io.File extVarFile = getResource("/test/japi/external_vars_1.xml");
    -		java.io.File schemaFile = getResource("/test/japi/mySchemaWithVars.dfdl.xsd");
    -		ProcessorFactory pf = c.compileFile(schemaFile);
    -		DataProcessor dp = pf.onPath("/");
    -		dp.setDebugger(debugger);
    -		dp.setDebugging(true);
    -		dp.setExternalVariables(extVarFile);
    -
    -		java.io.File file = getResource("/test/japi/myData.dat");
    -		java.io.FileInputStream fis = new java.io.FileInputStream(file);
    -		java.nio.channels.ReadableByteChannel rbc = java.nio.channels.Channels.newChannel(fis);
    -		JDOMInfosetOutputter outputter = new JDOMInfosetOutputter();
    -		ParseResult res = dp.parse(rbc, outputter, 2 << 3);
    -		boolean err = res.isError();
    -		assertFalse(err);
    -		org.jdom2.Document doc = outputter.getResult();
    -		org.jdom2.output.XMLOutputter xo = new org.jdom2.output.XMLOutputter();
    -		xo.setFormat(Format.getPrettyFormat());
    -		String docString = xo.outputString(doc);
    -		boolean containsVar1 = docString.contains("var1Value");
    -		boolean containsVar1Value = docString.contains("externallySet");
    -		assertTrue(containsVar1);
    -		assertTrue(containsVar1Value);
    -		assertTrue(res.location().isAtEnd());
    -
    -		assertEquals(0, lw.errors.size());
    -		assertEquals(0, lw.warnings.size());
    -		assertTrue(lw.others.size() > 0);
    -		assertTrue(debugger.lines.size() > 0);
    -		assertTrue(debugger.lines.contains("----------------------------------------------------------------- 1\n"));
    -
    -		// reset the global logging and debugger state
    -		Daffodil.setLogWriter(new ConsoleLogWriter());
    -		Daffodil.setLoggingLevel(LogLevel.Info);
    -	}
    -	
    -	@Test
    -	public void testJavaAPI15() throws IOException {
    -		LogWriterForJAPITest lw = new LogWriterForJAPITest();
    -
    -		Daffodil.setLogWriter(lw);
    -		Daffodil.setLoggingLevel(LogLevel.Info);
    -
    -		edu.illinois.ncsa.daffodil.japi.Compiler c = Daffodil.compiler();
    -		c.setValidateDFDLSchemas(false);
    -		java.io.File schemaFile = getResource("/test/japi/mySchema1.dfdl.xsd");
    -		ProcessorFactory pf = c.compileFile(schemaFile);
    -		DataProcessor dp = pf.onPath("/");
    -		java.io.File file = getResource("/test/japi/myInfosetBroken.xml");
    -		org.jdom2.input.SAXBuilder builder = new org.jdom2.input.SAXBuilder();
    -
    -		org.jdom2.Document doc = null;
    -		try {
    -			doc = builder.build(file);
    -		} catch (Exception e) {
    -			fail();
    -		}
    -
    -		java.io.ByteArrayOutputStream bos = new java.io.ByteArrayOutputStream();
    -		java.nio.channels.WritableByteChannel wbc = java.nio.channels.Channels.newChannel(bos);
    -		JDOMInfosetInputter inputter = new JDOMInfosetInputter(doc);
    -		UnparseResult res = dp.unparse(inputter, wbc);
    -		boolean err = res.isError();
    -		assertTrue(err);
    -
    -		java.util.List<Diagnostic> diags = res.getDiagnostics();
    -		assertEquals(1, diags.size());
    -		Diagnostic d = diags.get(0);
    -		assertTrue(d.getMessage().contains("wrong"));
    -		assertTrue(d.getMessage().contains("e1"));
    -
    -		// reset the global logging state
    -		Daffodil.setLogWriter(new ConsoleLogWriter());
    -		Daffodil.setLoggingLevel(LogLevel.Info);
    -	}
    -
    -
    -	@Test
    -	public void testJavaAPI16() throws IOException, InvalidUsageException {
    -		edu.illinois.ncsa.daffodil.japi.Compiler c = Daffodil.compiler();
    -		c.setValidateDFDLSchemas(false);
    -		java.io.File schemaFile = getResource("/test/japi/mySchema1.dfdl.xsd");
    -		ProcessorFactory pf = c.compileFile(schemaFile);
    -		DataProcessor dp = pf.onPath("/");
    -		dp.setValidationMode(ValidationMode.Limited);
    -		java.io.File file = getResource("/test/japi/myData.dat");
    -		java.io.FileInputStream fis = new java.io.FileInputStream(file);
    -		java.nio.channels.ReadableByteChannel rbc = java.nio.channels.Channels.newChannel(fis);
    -		JDOMInfosetOutputter outputter = new JDOMInfosetOutputter();
    -		ParseResult res = dp.parse(rbc, outputter, 2 << 3);
    -		assertTrue(res.isError());
    -		assertFalse(res.isProcessingError());
    -		assertTrue(res.isValidationError());
    -		assertTrue(res.location().isAtEnd());
    -
    -		java.util.List<Diagnostic> diags = res.getDiagnostics();
    -		assertEquals(1, diags.size());
    -		Diagnostic d = diags.get(0);
    -		assertTrue(d.getMessage().contains("maxInclusive"));
    -		assertTrue(d.getMessage().contains("e2"));
    -		assertTrue(d.getMessage().contains("20"));
    -	}
    -
    -	@Test
    -	public void testJavaAPI17() throws IOException, InvalidUsageException {
    -		edu.illinois.ncsa.daffodil.japi.Compiler c = Daffodil.compiler();
    -		c.setValidateDFDLSchemas(false);
    -		java.io.File schemaFile = getResource("/test/japi/mySchema1.dfdl.xsd");
    -		ProcessorFactory pf = c.compileFile(schemaFile);
    -		DataProcessor dp = pf.onPath("/");
    -		dp.setValidationMode(ValidationMode.Full);
    -		java.io.File file = getResource("/test/japi/myData.dat");
    -		java.io.FileInputStream fis = new java.io.FileInputStream(file);
    -		java.nio.channels.ReadableByteChannel rbc = java.nio.channels.Channels.newChannel(fis);
    -		JDOMInfosetOutputter outputter = new JDOMInfosetOutputter();
    -		ParseResult res = dp.parse(rbc, outputter, 2 << 3);
    -		assertTrue(res.isError());
    -		assertFalse(res.isProcessingError());
    -		assertTrue(res.isValidationError());
    -		assertTrue(res.location().isAtEnd());
    -
    -		java.util.List<Diagnostic> diags = res.getDiagnostics();
    -		assertEquals(3, diags.size());
    -		Diagnostic d0 = diags.get(0);
    -		Diagnostic d1 = diags.get(1);
    -		Diagnostic d2 = diags.get(2);
    -
    -		assertTrue(d0.getMessage().contains("42"));
    -		assertTrue(d0.getMessage().contains("e2"));
    -		assertTrue(d0.getMessage().contains("not valid"));
    -
    -		assertTrue(d1.getMessage().contains("42"));
    -		assertTrue(d1.getMessage().contains("maxInclusive"));
    -		assertTrue(d1.getMessage().contains("20"));
    -
    -		assertTrue(d2.getMessage().contains("maxInclusive"));
    -		assertTrue(d2.getMessage().contains("e2"));
    -		assertTrue(d2.getMessage().contains("20"));
    -	}
    +    public java.io.File getResource(String resPath) {
    +        try {
    +            return new java.io.File(this.getClass().getResource(resPath).toURI());
    +        } catch (Exception e) {
    +            return null;
    +        }
    +    }
    +
    +    @Test
    +    public void testJavaAPI1() throws IOException {
    +        LogWriterForJAPITest lw = new LogWriterForJAPITest();
    +        DebuggerRunnerForJAPITest debugger = new DebuggerRunnerForJAPITest();
    +
    +        Daffodil.setLogWriter(lw);
    +        Daffodil.setLoggingLevel(LogLevel.Debug);
    +
    +        edu.illinois.ncsa.daffodil.japi.Compiler c = Daffodil.compiler();
    +        c.setValidateDFDLSchemas(false);
    +        java.io.File schemaFile = getResource("/test/japi/mySchema1.dfdl.xsd");
    +        ProcessorFactory pf = c.compileFile(schemaFile);
    +        DataProcessor dp = pf.onPath("/");
    +        dp.setDebugger(debugger);
    +        dp.setDebugging(true);
    +        java.io.File file = getResource("/test/japi/myData.dat");
    +        java.io.FileInputStream fis = new java.io.FileInputStream(file);
    +        java.nio.channels.ReadableByteChannel rbc = java.nio.channels.Channels.newChannel(fis);
    +        JDOMInfosetOutputter outputter = new JDOMInfosetOutputter();
    +        ParseResult res = dp.parse(rbc, outputter, 2 << 3);
    +        boolean err = res.isError();
    +        assertFalse(err);
    +        assertTrue(res.location().isAtEnd());
    +        assertEquals(0, lw.errors.size());
    +        assertEquals(0, lw.warnings.size());
    +        assertTrue(lw.others.size() > 0);
    +        assertTrue(debugger.lines.size() > 0);
    +        assertTrue(debugger.lines.contains("----------------------------------------------------------------- 1\n"));
    +        assertTrue(debugger.getCommand().equals("trace"));
    +
    +        java.io.ByteArrayOutputStream bos = new java.io.ByteArrayOutputStream();
    +        java.nio.channels.WritableByteChannel wbc = java.nio.channels.Channels.newChannel(bos);
    +        JDOMInfosetInputter inputter = new JDOMInfosetInputter(outputter.getResult());
    +        UnparseResult res2 = dp.unparse(inputter, wbc);
    +        err = res2.isError();
    +        assertFalse(err);
    +        assertEquals("42", bos.toString());
    +
    +        // reset the global logging and debugger state
    +        Daffodil.setLogWriter(new ConsoleLogWriter());
    +        Daffodil.setLoggingLevel(LogLevel.Info);
    +    }
    +
    +    // This is a duplicate of test testJavaAPI1 that serializes the parser
    +    // before executing the test.
    +    @Test
    +    public void testJavaAPI1_A() throws Exception {
    +        LogWriterForJAPITest lw = new LogWriterForJAPITest();
    +        DebuggerRunnerForJAPITest debugger = new DebuggerRunnerForJAPITest();
    +
    +        Daffodil.setLogWriter(lw);
    +        Daffodil.setLoggingLevel(LogLevel.Debug);
    +
    +        edu.illinois.ncsa.daffodil.japi.Compiler c = Daffodil.compiler();
    +        c.setValidateDFDLSchemas(false);
    +        java.io.File schemaFile = getResource("/test/japi/mySchema1.dfdl.xsd");
    +        ProcessorFactory pf = c.compileFile(schemaFile);
    +        DataProcessor dp = pf.onPath("/");
    +
    +        // Serialize the parser to memory, then deserialize for parsing.
    +        ByteArrayOutputStream os = new ByteArrayOutputStream();
    +        WritableByteChannel output = Channels.newChannel(os);
    +        dp.save(output);
    +
    +        ByteArrayInputStream is = new ByteArrayInputStream(os.toByteArray());
    +        ReadableByteChannel input = Channels.newChannel(is);
    +        edu.illinois.ncsa.daffodil.japi.Compiler compiler = Daffodil.compiler();
    +        DataProcessor parser = compiler.reload(input);
    +        parser.setDebugger(debugger);
    +        parser.setDebugging(true);
    +
    +        java.io.File file = getResource("/test/japi/myData.dat");
    +        java.io.FileInputStream fis = new java.io.FileInputStream(file);
    +        java.nio.channels.ReadableByteChannel rbc = java.nio.channels.Channels.newChannel(fis);
    +        JDOMInfosetOutputter outputter = new JDOMInfosetOutputter();
    +        ParseResult res = parser.parse(rbc, outputter, 2 << 3);
    +        boolean err = res.isError();
    +        assertFalse(err);
    +        assertTrue(res.location().isAtEnd());
    +        assertEquals(0, lw.errors.size());
    +        assertEquals(0, lw.warnings.size());
    +        assertTrue(lw.others.size() > 0);
    +        assertTrue(debugger.lines.size() > 0);
    +        assertTrue(debugger.lines.contains("----------------------------------------------------------------- 1\n"));
    +        assertTrue(debugger.getCommand().equals("trace"));
    +
    +        java.io.ByteArrayOutputStream bos = new java.io.ByteArrayOutputStream();
    +        java.nio.channels.WritableByteChannel wbc = java.nio.channels.Channels.newChannel(bos);
    +        JDOMInfosetInputter inputter = new JDOMInfosetInputter(outputter.getResult());
    +        UnparseResult res2 = dp.unparse(inputter, wbc);
    +        err = res2.isError();
    +        assertFalse(err);
    +        assertEquals("42", bos.toString());
    +
    +        // reset the global logging and debugger state
    +        Daffodil.setLogWriter(new ConsoleLogWriter());
    +        Daffodil.setLoggingLevel(LogLevel.Info);
    +    }
    +
    +    // This is a duplicate of test testJavaAPI1 that serializes the parser
    +    // before executing the test.
    +    @Test
    +    public void testJavaAPI1_A_FullFails() throws Exception {
    +        LogWriterForJAPITest lw = new LogWriterForJAPITest();
    +        DebuggerRunnerForJAPITest debugger = new DebuggerRunnerForJAPITest();
    +
    +        Daffodil.setLogWriter(lw);
    +        Daffodil.setLoggingLevel(LogLevel.Debug);
    +
    +        edu.illinois.ncsa.daffodil.japi.Compiler c = Daffodil.compiler();
    +        c.setValidateDFDLSchemas(false);
    +        java.io.File schemaFile = getResource("/test/japi/mySchema1.dfdl.xsd");
    +        ProcessorFactory pf = c.compileFile(schemaFile);
    +        DataProcessor dp = pf.onPath("/");
    +        dp.setDebugger(debugger);
    +        dp.setDebugging(true);
    +
    +        // Serialize the parser to memory, then deserialize for parsing.
    +        ByteArrayOutputStream os = new ByteArrayOutputStream();
    +        WritableByteChannel output = Channels.newChannel(os);
    +        dp.save(output);
    +
    +        ByteArrayInputStream is = new ByteArrayInputStream(os.toByteArray());
    +        ReadableByteChannel input = Channels.newChannel(is);
    +        edu.illinois.ncsa.daffodil.japi.Compiler compiler = Daffodil.compiler();
    +        DataProcessor parser = compiler.reload(input);
    +
    +        try {
    +            parser.setValidationMode(ValidationMode.Full);
    +            fail();
    +        } catch (InvalidUsageException e) {
    +            assertEquals("'Full' validation not allowed when using a restored parser.", e.getMessage());
    +        }
    +
    +        // reset the global logging and debugger state
    +        Daffodil.setLogWriter(new ConsoleLogWriter());
    +        Daffodil.setLoggingLevel(LogLevel.Info);
    +    }
    +
    +    @Test
    +    public void testJavaAPI2() throws IOException {
    +        LogWriterForJAPITest lw = new LogWriterForJAPITest();
    +
    +        Daffodil.setLogWriter(lw);
    +        Daffodil.setLoggingLevel(LogLevel.Info);
    +
    +        edu.illinois.ncsa.daffodil.japi.Compiler c = Daffodil.compiler();
    +        c.setValidateDFDLSchemas(false);
    +        java.io.File schemaFile = getResource("/test/japi/mySchema1.dfdl.xsd");
    +        ProcessorFactory pf = c.compileFile(schemaFile);
    +        DataProcessor dp = pf.onPath("/");
    +        java.io.File file = getResource("/test/japi/myDataBroken.dat");
    +        java.io.FileInputStream fis = new java.io.FileInputStream(file);
    +        java.nio.channels.ReadableByteChannel rbc = java.nio.channels.Channels.newChannel(fis);
    +        JDOMInfosetOutputter outputter = new JDOMInfosetOutputter();
    +        ParseResult res = dp.parse(rbc, outputter);
    +
    +        // TODO: NEED a java friendly way to get the status of the outputter.
    +        // Scala enums don't work well
    +        // assertTrue(outputter.getStatus() != Status.DONE); // This is a hack
    +        // so that Java doesn't have to know about Nope/Maybe, need to figure
    +        // out better api that is Java compatible
    +        assertTrue(res.isError());
    +        java.util.List<Diagnostic> diags = res.getDiagnostics();
    +        assertEquals(1, diags.size());
    +        Diagnostic d = diags.get(0);
    +        // System.err.println(d.getMessage());
    +        assertTrue(d.getMessage().contains("int"));
    +        assertTrue(d.getMessage().contains("Not an int"));
    +        assertTrue(d.getDataLocations().toString().contains("10"));
    +        java.util.List<LocationInSchemaFile> locs = d.getLocationsInSchemaFiles();
    +        assertEquals(1, locs.size());
    +        LocationInSchemaFile loc = locs.get(0);
    +        assertTrue(loc.toString().contains("mySchema1.dfdl.xsd"));
    --- End diff --
    
    This is the actual change.  Was "mySchema2.dfdl.xsd" which is arguably better.
    
    A side effect of the change to resolve everything from Term, is that we're always pointing at a term when we issue an SDE, and never pointing at a global element decl, or type definition. This is not correct. Somehow we're not getting the right object to issue the SDE. When we resolve properties, the Found object also includes the location where that property was found - seems we aren't utilizing that information to get a more specific object to use to issue the SDE.


---


[GitHub] incubator-daffodil pull request #5: Property Resolution - part of fixing sch...

Posted by mbeckerle <gi...@git.apache.org>.
Github user mbeckerle commented on a diff in the pull request:

    https://github.com/apache/incubator-daffodil/pull/5#discussion_r149837661
  
    --- Diff: daffodil-core/src/main/scala/edu/illinois/ncsa/daffodil/dsom/AnnotatedSchemaComponent.scala ---
    @@ -41,23 +41,122 @@ import edu.illinois.ncsa.daffodil.equality._
     import edu.illinois.ncsa.daffodil.schema.annotation.props.PropertyLookupResult
     import edu.illinois.ncsa.daffodil.schema.annotation.props.NotFound
     import edu.illinois.ncsa.daffodil.schema.annotation.props.Found
    +import edu.illinois.ncsa.daffodil.schema.annotation.props.FindPropertyMixin
    +
    +/**
    + * Only objects from which we generate processors (parsers/unparsers)
    + * can lookup property values.
    + *
    + * This avoids the possibility of a property being resolved incorrectly by
    + * not looking at the complete chain of schema components contributing to the
    + * property resolution.
    + *
    + * The only objects that should resolve properties are
    + * ElementRef, Root, LocalElementDecl, Sequence, Choice, SequenceRef, ChoiceRef
    + *
    + * These are all the "real" terms. Everything else is just contributing
    + * properties to the mix, but they are not points where properties are
    + * used to generate processors.
    + */
    +trait ResolvesProperties
    +  extends FindPropertyMixin { self: AnnotatedSchemaComponent =>
    +
    +  def term: Term
    +
    +  private def findNonDefaultProperty(pname: String): PropertyLookupResult = {
    +    val result = findDefaultOrNonDefaultProperty(pname, nonDefaultPropertySources)
    +    result match {
    +      case f: Found => f
    +      case NotFound(nd, d, _) =>
    +        Assert.invariant(d.isEmpty)
    +    }
    +    result
    +  }
    +
    +  private def findDefaultProperty(pname: String): PropertyLookupResult = {
    +    val result = findDefaultOrNonDefaultProperty(pname, defaultPropertySources)
    +    val fixup = result match {
    +      case Found(value, loc, pname, _) =>
    +        // found as a default property.
    +        // supply constructor's last arg is boolean indicating it's a default property
    +        Found(value, loc, pname, true)
    +      case NotFound(nd, d, pn) =>
    +        Assert.invariant(d.isEmpty)
    +        NotFound(Seq(), nd, pn) // we want the places we searched shown as default locations searched
    +    }
    +    fixup
    +  }
    +
    +  override def findPropertyOption(pname: String): PropertyLookupResult = {
    +    ExecutionMode.requireCompilerMode
    +    // first try in regular properties
    +    val regularResult = resolver.findNonDefaultProperty(pname)
    +    regularResult match {
    +      case f: Found => f
    +      case NotFound(nonDefaultLocsTried1, defaultLocsTried1, _) => {
    +        Assert.invariant(defaultLocsTried1.isEmpty)
    +        val defaultResult = resolver.findDefaultProperty(pname)
    +        defaultResult match {
    +          case f: Found => f
    +          case NotFound(nonDefaultLocsTried2, defaultLocsTried2, _) => {
    +            Assert.invariant(nonDefaultLocsTried2.isEmpty)
    +            // did not find it at all. Return a NotFound with all the places we
    +            // looked non-default and default.
    +            val nonDefaultPlaces = nonDefaultLocsTried1
    +            val defaultPlaces = defaultLocsTried2
    +            NotFound(nonDefaultPlaces, defaultPlaces, pname)
    +          }
    +        }
    +      }
    +    }
    +  }
    +}
    +
    +abstract class AnnotatedSchemaComponentImpl( final override val xml: Node,
    +  final override val parent: SchemaComponent)
    +  extends AnnotatedSchemaComponent
     
     /**
      * Shared characteristics of any annotated schema component.
      * Not all components can carry DFDL annotations.
      */
    +trait AnnotatedSchemaComponent
    +  extends SchemaComponent
    +  with AnnotatedMixin
    +  with OverlapCheckMixin {
     
    -abstract class AnnotatedSchemaComponent(xml: Node, sc: SchemaComponent)
    -  extends SchemaComponent(xml, sc)
    -  with AnnotatedMixin {
    +  final lazy val term: Term = this match {
    +    case gr: GroupRef => gr.asModelGroup
    +    //    case mg: ModelGroup => mg.parent match {
    +    //      case ggd: GlobalGroupDef => {
    +    //        // this is the model group that is the definition of
    +    //        // a global group, so our term is the group reference referring to this.
    +    //        ggd.groupRef.asModelGroup
    +    //      }
    +    //      case _ => mg
    +    //    }
    +    case t: Term => t
    +    case ged: GlobalElementDecl => ged.elementRef
    +    case ty: SimpleTypeDefBase => ty.elementBase
    +    case ty: ComplexTypeBase => ty.elementBase
    +    case ggd: GlobalGroupDef => ggd.groupRef.asModelGroup
    +    case sd: SchemaDocument =>
    +      Assert.usageError("not to be called for schema documents")
    +  }
    +
    +  final lazy val resolver: ResolvesProperties = {
    +    val res = this match {
    +      case sd: SchemaDocument => sd
    +      case _ => term
    +    }
    +    res
    +  }
     
       requiredEvaluations(annotationObjs)
       requiredEvaluations(shortFormPropertiesCorrect)
       requiredEvaluations(nonDefaultPropertySources)
       requiredEvaluations(defaultPropertySources)
     
    -  def term: Term
    --- End diff --
    
    Actually , not well formed question - a variable declarations are not Annotated Schema Components, because they themselves are DFDL annotations. 


---

[GitHub] incubator-daffodil pull request #5: Property Resolution - part of fixing sch...

Posted by mbeckerle <gi...@git.apache.org>.
Github user mbeckerle commented on a diff in the pull request:

    https://github.com/apache/incubator-daffodil/pull/5#discussion_r149843031
  
    --- Diff: daffodil-core/src/main/scala/edu/illinois/ncsa/daffodil/dsom/SequenceGroup.scala ---
    @@ -296,6 +259,65 @@ class Sequence(xmlArg: Node, parent: SchemaComponent, position: Int)
     
     }
     
    +trait SequenceLikeMixin
    --- End diff --
    
    scaladoc. Also consider renaming this mixin to be more specific. I think this is SequenceDefMixin, and it captures that the component carries a DFDLSequence annotation (not a DFDLGroup annotation), and contains XML elements inside it.


---

[GitHub] incubator-daffodil pull request #5: Property Resolution - part of fixing sch...

Posted by mbeckerle <gi...@git.apache.org>.
Github user mbeckerle commented on a diff in the pull request:

    https://github.com/apache/incubator-daffodil/pull/5#discussion_r150685971
  
    --- Diff: daffodil-core/src/main/scala/edu/illinois/ncsa/daffodil/dsom/ModelGroup.scala ---
    @@ -115,118 +170,47 @@ abstract class ModelGroup(xmlArg: Node, parentArg: SchemaComponent, position: In
     
       def modelGroupRuntimeData: ModelGroupRuntimeData
     
    -  final lazy val gRefNonDefault: Option[ChainPropProvider] = groupRef.map { _.nonDefaultFormatChain }
    -  final lazy val gRefDefault: Option[ChainPropProvider] = groupRef.map { _.defaultFormatChain }
    -
    -  final def nonDefaultPropertySources = LV('nonDefaultPropertySources) {
    -    val seq = (gRefNonDefault.toSeq ++ Seq(this.nonDefaultFormatChain)).distinct
    -    checkNonOverlap(seq)
    -    seq
    -  }.value
    -
    -  final def defaultPropertySources = LV('defaultPropertySources) {
    -    val seq = (gRefDefault.toSeq ++ Seq(this.defaultFormatChain)).distinct
    -    seq
    -  }.value
    +  protected final lazy val prettyBaseName = xml.label
     
    -  protected final lazy val prettyBaseName = xmlArg.label
    +  def xmlChildren: Seq[Node]
     
    -  protected def xmlChildren: Seq[Node]
    -
    -  private def goodXmlChildren = LV('goodXMLChildren) { xmlChildren.flatMap { removeNonInteresting(_) } }.value
    -  private lazy val positions = List.range(1, goodXmlChildren.length + 1) // range is exclusive on 2nd arg. So +1.
    -  private lazy val pairs = goodXmlChildren zip positions
    -
    -  final lazy val sequenceChildren = groupMembers.collect { case s: Sequence => s }
    -  final lazy val choiceChildren = groupMembers.collect { case s: Choice => s }
    +  final lazy val sequenceChildren = groupMembers.collect { case s: SequenceBase => s }
    +  final lazy val choiceChildren = groupMembers.collect { case s: ChoiceBase => s }
       final lazy val groupRefChildren = groupMembers.collect { case s: GroupRef => s }
     
    -  final def group = this
    -
    -  final lazy val groupMembers = {
    -    pairs.flatMap {
    -      case (n, i) =>
    -        termFactory(n, this, i)
    -    }
    -  }
    +  def group = this
     
       final override lazy val termChildren = groupMembers
    --- End diff --
    
    Not just a renaming. termChildren generalizes for Elements and ModelGroups.


---

[GitHub] incubator-daffodil pull request #5: Property Resolution - part of fixing sch...

Posted by mbeckerle <gi...@git.apache.org>.
Github user mbeckerle commented on a diff in the pull request:

    https://github.com/apache/incubator-daffodil/pull/5#discussion_r150876868
  
    --- Diff: daffodil-lib/src/main/scala/edu/illinois/ncsa/daffodil/schema/annotation/props/PropertyScoping.scala ---
    @@ -209,8 +209,10 @@ trait FindPropertyMixin extends PropTypes {
        * For unit testing convenience
        */
       final def verifyPropValue(key: String, value: String): Boolean = {
    -    findPropertyOption(key) match {
    -      case Found(`value`, _, _, _) => true
    +    val prop = findPropertyOption(key)
    +    val valueLC = value.toLowerCase
    +    prop match {
    +      case Found(v, _, _, _) if (v.toLowerCase == valueLC) => true
    --- End diff --
    
    Removed case insensitivity. Only one test had to be changed to UTF-8 from "utf-8". 
    
    Moving this is problematic, as tests depend on this being available on DSOM objects so as to examine specific object for properties. Might be convenient to have when debugging as well. So I'll leave it on FindPropertyMixin for now. 



---

[GitHub] incubator-daffodil pull request #5: Property Resolution - part of fixing sch...

Posted by mbeckerle <gi...@git.apache.org>.
Github user mbeckerle commented on a diff in the pull request:

    https://github.com/apache/incubator-daffodil/pull/5#discussion_r150691098
  
    --- Diff: daffodil-core/src/main/scala/edu/illinois/ncsa/daffodil/dsom/Term.scala ---
    @@ -48,13 +46,17 @@ import edu.illinois.ncsa.daffodil.schema.annotation.props.gen.NilKind
     /////////////////////////////////////////////////////////////////
     
     // A term is content of a group
    -abstract class Term(xmlArg: Node, parentArg: SchemaComponent, val position: Int)
    -  extends AnnotatedSchemaComponent(xmlArg, parentArg)
    +trait Term
    +  extends AnnotatedSchemaComponent
    +  with ResolvesProperties
    --- End diff --
    
    yes.


---

[GitHub] incubator-daffodil pull request #5: Property Resolution - part of fixing sch...

Posted by stevedlawrence <gi...@git.apache.org>.
Github user stevedlawrence commented on a diff in the pull request:

    https://github.com/apache/incubator-daffodil/pull/5#discussion_r150594216
  
    --- Diff: daffodil-core/src/main/scala/edu/illinois/ncsa/daffodil/dsom/ChoiceGroup.scala ---
    @@ -257,3 +275,13 @@ final class Choice(xmlArg: Node, parent: SchemaComponent, position: Int)
           optIgnoreCase)
       }
     }
    +
    +final class Choice(xmlArg: Node, parent: SchemaComponent, position: Int)
    +  extends ChoiceBase(xmlArg, parent, position)
    +  with ChoiceLikeMixin
    --- End diff --
    
    If ChoiceBase should mixin ChoiceLikeMixin, this isn't necessary. But maybe the above is wrong?


---

[GitHub] incubator-daffodil pull request #5: Property Resolution - part of fixing sch...

Posted by mbeckerle <gi...@git.apache.org>.
Github user mbeckerle commented on a diff in the pull request:

    https://github.com/apache/incubator-daffodil/pull/5#discussion_r151118087
  
    --- Diff: daffodil-core/src/main/scala/edu/illinois/ncsa/daffodil/dsom/GroupBase.scala ---
    @@ -1,92 +1,44 @@
    -/* Copyright (c) 2012-2015 Tresys Technology, LLC. All rights reserved.
    --- End diff --
    
    GroupBase was merged into ModelGroup


---

[GitHub] incubator-daffodil pull request #5: Property Resolution - part of fixing sch...

Posted by mbeckerle <gi...@git.apache.org>.
Github user mbeckerle commented on a diff in the pull request:

    https://github.com/apache/incubator-daffodil/pull/5#discussion_r149841226
  
    --- Diff: daffodil-core/src/main/scala/edu/illinois/ncsa/daffodil/dsom/ModelGroup.scala ---
    @@ -115,118 +170,47 @@ abstract class ModelGroup(xmlArg: Node, parentArg: SchemaComponent, position: In
     
       def modelGroupRuntimeData: ModelGroupRuntimeData
     
    -  final lazy val gRefNonDefault: Option[ChainPropProvider] = groupRef.map { _.nonDefaultFormatChain }
    -  final lazy val gRefDefault: Option[ChainPropProvider] = groupRef.map { _.defaultFormatChain }
    -
    -  final def nonDefaultPropertySources = LV('nonDefaultPropertySources) {
    -    val seq = (gRefNonDefault.toSeq ++ Seq(this.nonDefaultFormatChain)).distinct
    -    checkNonOverlap(seq)
    -    seq
    -  }.value
    -
    -  final def defaultPropertySources = LV('defaultPropertySources) {
    -    val seq = (gRefDefault.toSeq ++ Seq(this.defaultFormatChain)).distinct
    -    seq
    -  }.value
    +  protected final lazy val prettyBaseName = xml.label
     
    -  protected final lazy val prettyBaseName = xmlArg.label
    +  def xmlChildren: Seq[Node]
     
    -  protected def xmlChildren: Seq[Node]
    -
    -  private def goodXmlChildren = LV('goodXMLChildren) { xmlChildren.flatMap { removeNonInteresting(_) } }.value
    -  private lazy val positions = List.range(1, goodXmlChildren.length + 1) // range is exclusive on 2nd arg. So +1.
    -  private lazy val pairs = goodXmlChildren zip positions
    -
    -  final lazy val sequenceChildren = groupMembers.collect { case s: Sequence => s }
    -  final lazy val choiceChildren = groupMembers.collect { case s: Choice => s }
    +  final lazy val sequenceChildren = groupMembers.collect { case s: SequenceBase => s }
    +  final lazy val choiceChildren = groupMembers.collect { case s: ChoiceBase => s }
       final lazy val groupRefChildren = groupMembers.collect { case s: GroupRef => s }
     
    -  final def group = this
    -
    -  final lazy val groupMembers = {
    -    pairs.flatMap {
    -      case (n, i) =>
    -        termFactory(n, this, i)
    -    }
    -  }
    +  def group = this
     
       final override lazy val termChildren = groupMembers
     
    -  final lazy val groupMembersNoRefs = groupMembers.map {
    -    case eRef: ElementRef => eRef.referencedElement
    -    case gb: GroupBase => gb.group
    -    case x => x
    -  }
    -
    -  /**
    -   * Factory for Terms
    -   *
    -   * Because of the context where this is used, this returns a list. Nil for non-terms, non-Nil for
    -   * an actual term. There should be only one non-Nil.
    -   *
    -   * This could be static code in an object. It doesn't reference any of the state of the ModelGroup,
    -   * it's here so that type-specific overrides are possible in Sequence or Choice
    -   */
    -  private def termFactory(child: Node, parent: ModelGroup, position: Int) = {
    -    val childList: List[Term] = child match {
    -      case <element>{ _* }</element> => {
    -        val refProp = child.attribute("ref").map { _.text }
    -        // must get an unprefixed attribute name, i.e. ref='foo:bar', and not
    -        // be tripped up by dfdl:ref="fmt:fooey" which is a format reference.
    -        refProp match {
    -          case None => List(schemaSet.LocalElementDeclFactory(child, schemaDocument).forModelGroup(parent, position))
    -          case Some(_) => List(new ElementRef(child, parent, position))
    -        }
    -      }
    -      case <annotation>{ _* }</annotation> => Nil
    -      case textNode: Text => Nil
    -      case _ => ModelGroupFactory(child, parent, position)
    -    }
    -    childList
    -  }
    -
    -  /**
    -   * XML is full of uninteresting text nodes. We just want the element children, not all children.
    -   */
    -  private def removeNonInteresting(child: Node) = {
    -    val childList: List[Node] = child match {
    -      case _: Text => Nil
    -      case _: Comment => Nil
    -      case <annotation>{ _* }</annotation> => Nil
    -      case _ => List(child)
    -    }
    -    childList
    -  }
    -
       /**
        * Combine our statements with those of the group ref that is referencing us (if there is one)
        */
    -  final lazy val statements: Seq[DFDLStatement] = localStatements ++ groupRef.map { _.statements }.getOrElse(Nil)
    -  final lazy val newVariableInstanceStatements: Seq[DFDLNewVariableInstance] =
    -    localNewVariableInstanceStatements ++ groupRef.map { _.newVariableInstanceStatements }.getOrElse(Nil)
    -  final lazy val (discriminatorStatements, assertStatements) = checkDiscriminatorsAssertsDisjoint(combinedDiscrims, combinedAsserts)
    -  private lazy val combinedAsserts: Seq[DFDLAssert] = localAssertStatements ++ groupRef.map { _.assertStatements }.getOrElse(Nil)
    -  private lazy val combinedDiscrims: Seq[DFDLDiscriminator] = localDiscriminatorStatements ++ groupRef.map { _.discriminatorStatements }.getOrElse(Nil)
    -
    -  final lazy val setVariableStatements: Seq[DFDLSetVariable] = {
    -    val combinedSvs = localSetVariableStatements ++ groupRef.map { _.setVariableStatements }.getOrElse(Nil)
    -    checkDistinctVariableNames(combinedSvs)
    -  }
    -
    -  final lazy val groupRef = parent match {
    -    case ggd: GlobalGroupDef => Some(ggd.groupRef)
    -    case _ => None
    -  }
    +  //  final lazy val statements: Seq[DFDLStatement] = localStatements ++ groupRef.map { _.statements }.getOrElse(Nil)
    --- End diff --
    
    remove commented code.


---

[GitHub] incubator-daffodil pull request #5: Property Resolution - part of fixing sch...

Posted by stevedlawrence <gi...@git.apache.org>.
Github user stevedlawrence commented on a diff in the pull request:

    https://github.com/apache/incubator-daffodil/pull/5#discussion_r151126177
  
    --- Diff: daffodil-core/src/main/scala/edu/illinois/ncsa/daffodil/dsom/GroupBase.scala ---
    @@ -1,90 +1,44 @@
    -/* Copyright (c) 2012-2015 Tresys Technology, LLC. All rights reserved.
    - *
    - * Developed by: Tresys Technology, LLC
    - *               http://www.tresys.com
    - *
    - * Permission is hereby granted, free of charge, to any person obtaining a copy of
    - * this software and associated documentation files (the "Software"), to deal with
    - * the Software without restriction, including without limitation the rights to
    - * use, copy, modify, merge, publish, distribute, sublicense, and/or sell copies
    - * of the Software, and to permit persons to whom the Software is furnished to do
    - * so, subject to the following conditions:
    - *
    - *  1. Redistributions of source code must retain the above copyright notice,
    - *     this list of conditions and the following disclaimers.
    - *
    - *  2. Redistributions in binary form must reproduce the above copyright
    - *     notice, this list of conditions and the following disclaimers in the
    - *     documentation and/or other materials provided with the distribution.
    - *
    - *  3. Neither the names of Tresys Technology, nor the names of its contributors
    - *     may be used to endorse or promote products derived from this Software
    - *     without specific prior written permission.
    - *
    - * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
    - * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
    - * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
    - * CONTRIBUTORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
    - * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
    - * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS WITH THE
    - * SOFTWARE.
    - */
    -
    -package edu.illinois.ncsa.daffodil.dsom
    -
    -import edu.illinois.ncsa.daffodil.schema.annotation.props.AlignmentType
    -import edu.illinois.ncsa.daffodil.schema.annotation.props.gen.AlignmentUnits
    -import java.lang.{ Integer => JInt }
    -
    -trait GroupBase
    -  extends Term {
    -
    -  final override def isScalar = true
    -  final override def isOptional = false
    -  final override def isRequired = true
    -  final override def isArray = false
    -
    -  private def prettyIndex = LV('prettyIndex) {
    -    myPeers.map { peers =>
    -      {
    -        if (peers.length == 1) "" // no index expression if we are the only one
    -        else "[" + (peers.indexOf(this) + 1) + "]" // 1-based indexing in XML/XSD
    -      }
    -    }.getOrElse("")
    -  }.value
    -
    -  override lazy val diagnosticDebugName = prettyBaseName + prettyIndex
    -  protected def prettyBaseName: String
    -
    -  /**
    -   * This is only the immediately enclosing model group. It doesn't walk outward.
    -   */
    -  final lazy val enclosingComponentModelGroup = enclosingComponent.collect { case mg: ModelGroup => mg }
    -  final lazy val sequencePeers = enclosingComponentModelGroup.map { _.sequenceChildren }
    -  final lazy val choicePeers = enclosingComponentModelGroup.map { _.choiceChildren }
    -  // final lazy val groupRefPeers = enclosingComponentModelGroup.map { _.groupRefChildren }
    -
    -  protected def myPeers: Option[Seq[GroupBase]]
    -
    -  def group: ModelGroup
    -
    -  //  final lazy val immediateGroup: Option[ModelGroup] = {
    -  //    val res: Option[ModelGroup] = this.group match {
    -  //      case (s: Sequence) => Some(s)
    -  //      case (c: Choice) => Some(c)
    -  //      case _ => None
    -  //    }
    -  //    res
    -  //  }
    -
    -  override lazy val alignmentValueInBits: JInt = {
    -    this.alignment match {
    -      case AlignmentType.Implicit => 1
    -      case align: JInt => this.alignmentUnits match {
    -        case AlignmentUnits.Bits => align
    -        case AlignmentUnits.Bytes => 8 * align
    -      }
    -    }
    -  }
    -
    -}
    +///* Copyright (c) 2012-2015 Tresys Technology, LLC. All rights reserved.
    +// *
    +// * Developed by: Tresys Technology, LLC
    +// *               http://www.tresys.com
    +// *
    +// * Permission is hereby granted, free of charge, to any person obtaining a copy of
    +// * this software and associated documentation files (the "Software"), to deal with
    +// * the Software without restriction, including without limitation the rights to
    +// * use, copy, modify, merge, publish, distribute, sublicense, and/or sell copies
    +// * of the Software, and to permit persons to whom the Software is furnished to do
    +// * so, subject to the following conditions:
    +// *
    +// *  1. Redistributions of source code must retain the above copyright notice,
    +// *     this list of conditions and the following disclaimers.
    +// *
    +// *  2. Redistributions in binary form must reproduce the above copyright
    +// *     notice, this list of conditions and the following disclaimers in the
    +// *     documentation and/or other materials provided with the distribution.
    +// *
    +// *  3. Neither the names of Tresys Technology, nor the names of its contributors
    +// *     may be used to endorse or promote products derived from this Software
    +// *     without specific prior written permission.
    +// *
    +// * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
    +// * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
    +// * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
    +// * CONTRIBUTORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
    +// * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
    +// * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS WITH THE
    +// * SOFTWARE.
    +// */
    +//
    +//package edu.illinois.ncsa.daffodil.dsom
    +//
    +//import edu.illinois.ncsa.daffodil.schema.annotation.props.AlignmentType
    +//import edu.illinois.ncsa.daffodil.schema.annotation.props.gen.AlignmentUnits
    +//import java.lang.{ Integer => JInt }
    +//
    +//trait GroupBase
    +//  extends Term {
    +//
    +//
    +//
    +//}
    --- End diff --
    
    Delete file?


---

[GitHub] incubator-daffodil pull request #5: Property Resolution - part of fixing sch...

Posted by stevedlawrence <gi...@git.apache.org>.
Github user stevedlawrence commented on a diff in the pull request:

    https://github.com/apache/incubator-daffodil/pull/5#discussion_r150862794
  
    --- Diff: daffodil-core/src/main/scala/edu/illinois/ncsa/daffodil/dsom/SchemaComponent.scala ---
    @@ -149,23 +157,45 @@ abstract class SchemaComponent(xmlArg: Node, val parent: SchemaComponent)
         val ee = et match {
           case None => None
           case Some(eb: ElementBase) => Some(eb)
    -      case Some(sc: SchemaComponent) => sc.enclosingElement
    +      case Some(sc: SchemaComponent) => {
    +        val scee = sc.enclosingElement
    +        scee
    +      }
         }
         ee
       }.value
     
    -  final lazy val rootElement: Option[GlobalElementDecl] = {
    +  final lazy val rootElementRef = rootElementDecl.map { _.elementRef }
    +
    +  final lazy val rootElementDecl: Option[GlobalElementDecl] = {
         enclosingElement match {
    -      case Some(e) => e.rootElement
    -      case None => {
    -        if (this.isInstanceOf[GlobalElementDecl])
    -          Some(this.asInstanceOf[GlobalElementDecl])
    -        else
    -          None
    +      case Some(e) => e.rootElementDecl
    +      case None => this match {
    +        case ged: GlobalElementDecl => Some(ged)
    +        case root: Root => root.optReferredToComponent
    +        case _ => Assert.invariantFailed("No global element decl")
           }
         }
       }
     
    +  //  final lazy val enclosingTerm: Option[Term] = {
    +  //    val optec = enclosingComponent
    +  //    val res = optec match {
    +  //      case None => None
    +  //      // case Some(ge: GlobalElementDecl) => ge.optElementRef.flatMap { _.enclosingTerm }
    +  //      case Some(ggd: GlobalGroupDef) => {
    +  //        val ggdet = ggd.groupRef.asModelGroup
    +  //        Some(ggdet)
    +  //      }
    +  //      case Some(t: Term) => Some(t)
    +  //      case Some(ged: GlobalElementDecl) => Some(ged.elementRef)
    +  //      case Some(ec) => {
    +  //        val ecet = ec.enclosingTerm
    +  //        ecet
    +  //      }
    +  //    }
    +  //    res
    +  //  }
    --- End diff --
    
    Remove comment


---

[GitHub] incubator-daffodil pull request #5: Property Resolution - part of fixing sch...

Posted by mbeckerle <gi...@git.apache.org>.
Github user mbeckerle commented on a diff in the pull request:

    https://github.com/apache/incubator-daffodil/pull/5#discussion_r149840319
  
    --- Diff: daffodil-core/src/main/scala/edu/illinois/ncsa/daffodil/dsom/InitiatedTerminatedMixin.scala ---
    @@ -75,7 +76,7 @@ trait InitiatedTerminatedMixin
        * present, or there is an expression. (Such expressions are not allowed to evaluate to "" - you
        * can't turn off a delimiter by providing "" at runtime. Minimum length is 1 for these at runtime.
        * <p>
    -   * Override in Sequence to also check for separator.
    +   * Override in SequenceBase to also check for separator.
    --- End diff --
    
    Verify if this comment is still relevant.


---

[GitHub] incubator-daffodil pull request #5: Property Resolution - part of fixing sch...

Posted by stevedlawrence <gi...@git.apache.org>.
Github user stevedlawrence commented on a diff in the pull request:

    https://github.com/apache/incubator-daffodil/pull/5#discussion_r150594489
  
    --- Diff: daffodil-core/src/main/scala/edu/illinois/ncsa/daffodil/dsom/ComplexTypes.scala ---
    @@ -1,7 +1,7 @@
     /* Copyright (c) 2012-2015 Tresys Technology, LLC. All rights reserved.
      *
      * Developed by: Tresys Technology, LLC
    - *               http://www.tresys.com
    + *               http://woverride val ww.tresys.com
    --- End diff --
    
    Accidental typo


---

[GitHub] incubator-daffodil pull request #5: Property Resolution - part of fixing sch...

Posted by mbeckerle <gi...@git.apache.org>.
Github user mbeckerle commented on a diff in the pull request:

    https://github.com/apache/incubator-daffodil/pull/5#discussion_r149840038
  
    --- Diff: daffodil-core/src/main/scala/edu/illinois/ncsa/daffodil/dsom/GroupBase.scala ---
    @@ -64,22 +62,22 @@ abstract class GroupBase(xmlArg: Node, parentArg: SchemaComponent, position: Int
       final lazy val enclosingComponentModelGroup = enclosingComponent.collect { case mg: ModelGroup => mg }
       final lazy val sequencePeers = enclosingComponentModelGroup.map { _.sequenceChildren }
       final lazy val choicePeers = enclosingComponentModelGroup.map { _.choiceChildren }
    -  final lazy val groupRefPeers = enclosingComponentModelGroup.map { _.groupRefChildren }
    +  // final lazy val groupRefPeers = enclosingComponentModelGroup.map { _.groupRefChildren }
    --- End diff --
    
    delete commented code.


---

[GitHub] incubator-daffodil pull request #5: Property Resolution - part of fixing sch...

Posted by mbeckerle <gi...@git.apache.org>.
Github user mbeckerle commented on a diff in the pull request:

    https://github.com/apache/incubator-daffodil/pull/5#discussion_r149840662
  
    --- Diff: daffodil-core/src/main/scala/edu/illinois/ncsa/daffodil/dsom/ModelGroup.scala ---
    @@ -56,15 +55,32 @@ sealed abstract class ModelGroupFactory private () {
        * flatmap it to get a collection of model groups. Nil for non-model groups, non-Nil for the model group
        * object. There should be only one non-Nil.
        */
    -  def apply(child: Node, parent: SchemaComponent, position: Int) = {
    -    val childList: List[GroupBase] = child match {
    -      case <sequence>{ _* }</sequence> => List(new Sequence(child, parent, position))
    +  def apply(child: Node, parent: SchemaComponent, position: Int, isHidden: Boolean): List[ModelGroup] = {
    --- End diff --
    
    scaladoc (probably for the object, not just for this method.)


---

[GitHub] incubator-daffodil pull request #5: Property Resolution - part of fixing sch...

Posted by mbeckerle <gi...@git.apache.org>.
Github user mbeckerle commented on a diff in the pull request:

    https://github.com/apache/incubator-daffodil/pull/5#discussion_r149837234
  
    --- Diff: daffodil-core/src/main/scala/edu/illinois/ncsa/daffodil/dsom/ChoiceGroup.scala ---
    @@ -78,29 +77,48 @@ import edu.illinois.ncsa.daffodil.api.WarnID
      *
      */
     
    -final class Choice(xmlArg: Node, parent: SchemaComponent, position: Int)
    -  extends ModelGroup(xmlArg, parent, position)
    -  with Choice_AnnotationMixin
    -  with RawDelimitedRuntimeValuedPropertiesMixin // initiator and terminator (not separator)
    -  with ChoiceGrammarMixin {
    +trait ChoiceLikeMixin
    --- End diff --
    
    Scaladoc, and/or change trait name to be more descriptive. What aspect of Choices does ChoiceLike capture?


---

[GitHub] incubator-daffodil pull request #5: Property Resolution - part of fixing sch...

Posted by stevedlawrence <gi...@git.apache.org>.
Github user stevedlawrence commented on a diff in the pull request:

    https://github.com/apache/incubator-daffodil/pull/5#discussion_r150869739
  
    --- Diff: daffodil-lib/src/main/scala/edu/illinois/ncsa/daffodil/schema/annotation/props/PropertyScoping.scala ---
    @@ -209,8 +209,10 @@ trait FindPropertyMixin extends PropTypes {
        * For unit testing convenience
        */
       final def verifyPropValue(key: String, value: String): Boolean = {
    -    findPropertyOption(key) match {
    -      case Found(`value`, _, _, _) => true
    +    val prop = findPropertyOption(key)
    +    val valueLC = value.toLowerCase
    +    prop match {
    +      case Found(v, _, _, _) if (v.toLowerCase == valueLC) => true
    --- End diff --
    
    I was about to make a comment about using String.equalsIgnoreCase since that might be a little faster in some cases. But then realized this is only used in unit tests. Should this just be moved to a unit test file? Also seems that we would want to be strict about checking case during unit tests? Case matters with property values, right?


---

[GitHub] incubator-daffodil pull request #5: Property Resolution - part of fixing sch...

Posted by mbeckerle <gi...@git.apache.org>.
Github user mbeckerle commented on a diff in the pull request:

    https://github.com/apache/incubator-daffodil/pull/5#discussion_r149845852
  
    --- Diff: daffodil-japi/src/test/java/edu/illinois/ncsa/daffodil/example/TestJavaAPI.java ---
    @@ -58,803 +58,806 @@
     import edu.illinois.ncsa.daffodil.japi.ProcessorFactory;
     import edu.illinois.ncsa.daffodil.japi.UnparseResult;
     import edu.illinois.ncsa.daffodil.japi.ValidationMode;
    +import edu.illinois.ncsa.daffodil.japi.infoset.JDOMInfosetInputter;
    +import edu.illinois.ncsa.daffodil.japi.infoset.JDOMInfosetOutputter;
     import edu.illinois.ncsa.daffodil.japi.logger.ConsoleLogWriter;
     import edu.illinois.ncsa.daffodil.japi.logger.LogLevel;
    -import edu.illinois.ncsa.daffodil.japi.infoset.JDOMInfosetOutputter;
    -import edu.illinois.ncsa.daffodil.japi.infoset.JDOMInfosetInputter;
     
     public class TestJavaAPI {
     
    -	public java.io.File getResource(String resPath) {
    --- End diff --
    
    Looks like all whitespace changes. There is one line of change below. But probably eclipse auto-save settings messed with the whitespace here, so need to be checked, and if possible revised so that it's not going to reindent our work from the way it was already indented.


---

[GitHub] incubator-daffodil pull request #5: Property Resolution - part of fixing sch...

Posted by VTGuy <gi...@git.apache.org>.
Github user VTGuy commented on a diff in the pull request:

    https://github.com/apache/incubator-daffodil/pull/5#discussion_r151152802
  
    --- Diff: daffodil-core/src/main/scala/edu/illinois/ncsa/daffodil/dsom/DFDLStatementMixin.scala ---
    @@ -38,16 +38,83 @@ import edu.illinois.ncsa.daffodil.exceptions.ThrowsSDE
     import edu.illinois.ncsa.daffodil.schema.annotation.props.gen.TestKind
     import edu.illinois.ncsa.daffodil.grammar.primitives.AssertBase
     
    +trait ResolvesDFDLStatementMixin
    +  extends ThrowsSDE
    +  with ProvidesDFDLStatementMixin { self: Term =>
    +
    +  requiredEvaluations(statements)
    +
    +  final lazy val statements: Seq[DFDLStatement] =
    +    optReferencedStatementSource.toSeq.flatMap { _.resolvedStatements } ++
    +      localStatements
    +
    +  private def getParserExprReferencedElements(s: DFDLStatement,
    +    f: ContentValueReferencedElementInfoMixin => Set[DPathElementCompileInfo]) = {
    +    s match {
    +
    +      case a: DFDLAssertionBase if (a.testKind eq TestKind.Expression) => {
    +        a.gram match {
    +          case ab: AssertBase => f(ab.expr)
    +          case _ => ReferencedElementInfos.None
    +        }
    +      }
    +
    --- End diff --
    
    Nit picking here but extra space is not needed between these lines.


---

[GitHub] incubator-daffodil pull request #5: Property Resolution - part of fixing sch...

Posted by mbeckerle <gi...@git.apache.org>.
Github user mbeckerle commented on a diff in the pull request:

    https://github.com/apache/incubator-daffodil/pull/5#discussion_r149846957
  
    --- Diff: daffodil-test/src/test/resources/edu/illinois/ncsa/daffodil/section06/namespaces/namespaces.tdml ---
    @@ -1549,7 +1549,12 @@
           <tdml:error>MinInclusive(16) must be less than or equal to MaxInclusive(10)</tdml:error>
           <tdml:error>shishi</tdml:error>
           <tdml:error>Location line</tdml:error>
    -      <tdml:error>section06/namespaces/shi</tdml:error>
    +      <!-- 
    --- End diff --
    
    See comment in TestJavaAPI.java - this change is undesirable. If we can't avoid it we can live with it, but really we don't want to lose precision on the error locations just because we're resolving the properties differently now.


---

[GitHub] incubator-daffodil pull request #5: Property Resolution - part of fixing sch...

Posted by mbeckerle <gi...@git.apache.org>.
Github user mbeckerle commented on a diff in the pull request:

    https://github.com/apache/incubator-daffodil/pull/5#discussion_r149838279
  
    --- Diff: daffodil-core/src/main/scala/edu/illinois/ncsa/daffodil/dsom/ElementBase.scala ---
    @@ -252,26 +258,35 @@ abstract class ElementBase(xmlArg: Node, parent: SchemaComponent, position: Int)
       lazy val optMinOccurs: Option[Int] = None
       lazy val optMaxOccurs: Option[Int] = None
     
    -  def elementRef: Option[ElementRef]
    -
       final override lazy val dpathCompileInfo = dpathElementCompileInfo
     
    +  /**
    +   * This is the compile info for this element. Since this might be an
    +   * element ref, we optionally carry the compile info for the referenced
    +   * element in that case.
    +   */
       lazy val dpathElementCompileInfo: DPathElementCompileInfo = {
    +    val ee = enclosingElement
         val eci = new DPathElementCompileInfo(
    -      enclosingElement.map { _.dpathElementCompileInfo },
    +      ee.map {
    +        _.dpathElementCompileInfo
    +      },
           variableMap,
    +      elementChildrenCompileInfo,
           namespaces,
           slashPath,
           name,
           isArray,
           namedQName,
           optPrimType,
           schemaFileLocation,
    -      elementChildrenCompileInfo,
    -      tunable)
    +      tunable,
    +      maybeElementRefReferencedElementCompileInfo)
         eci
       }
     
    +  protected def maybeElementRefReferencedElementCompileInfo: Maybe[DPathElementCompileInfo] = Maybe.Nope
    --- End diff --
    
    check if this is used. It might not be.


---

[GitHub] incubator-daffodil pull request #5: Property Resolution - part of fixing sch...

Posted by mbeckerle <gi...@git.apache.org>.
Github user mbeckerle commented on a diff in the pull request:

    https://github.com/apache/incubator-daffodil/pull/5#discussion_r149844341
  
    --- Diff: daffodil-core/src/main/scala/edu/illinois/ncsa/daffodil/dsom/Term.scala ---
    @@ -48,13 +46,17 @@ import edu.illinois.ncsa.daffodil.schema.annotation.props.gen.NilKind
     /////////////////////////////////////////////////////////////////
     
     // A term is content of a group
    -abstract class Term(xmlArg: Node, parentArg: SchemaComponent, val position: Int)
    -  extends AnnotatedSchemaComponent(xmlArg, parentArg)
    +trait Term
    +  extends AnnotatedSchemaComponent
    +  with ResolvesProperties
    --- End diff --
    
    Is there a ProvidesProperties to complement this ResolvesProperties? 


---

[GitHub] incubator-daffodil issue #5: Property Resolution - part of fixing schema com...

Posted by VTGuy <gi...@git.apache.org>.
Github user VTGuy commented on the issue:

    https://github.com/apache/incubator-daffodil/pull/5
  
    +1


---

[GitHub] incubator-daffodil pull request #5: Property Resolution - part of fixing sch...

Posted by mbeckerle <gi...@git.apache.org>.
Github user mbeckerle commented on a diff in the pull request:

    https://github.com/apache/incubator-daffodil/pull/5#discussion_r150649102
  
    --- Diff: daffodil-core/src/main/scala/edu/illinois/ncsa/daffodil/dsom/ElementRef.scala ---
    @@ -106,43 +108,27 @@ final class ElementRef(xmlArg: Node, parent: ModelGroup, position: Int)
         res
       }.value
     
    -  override lazy val runtimeData = referencedElement.runtimeData
    -  override lazy val termRuntimeData = referencedElement.termRuntimeData
    -  override def erd = referencedElement.erd
    -  override lazy val elementRuntimeData: ElementRuntimeData = LV('elementRuntimeData) {
    -    referencedElement.elementRuntimeData
    -  }.value
    -  override lazy val dpathElementCompileInfo = referencedElement.dpathElementCompileInfo
    -
    -  // These will just delegate to the referenced element declaration
    -  def isNillable = referencedElement.isNillable
    -  def isSimpleType = referencedElement.isSimpleType
    -  def isComplexType = referencedElement.isComplexType
    -
    -  def isDefaultable: Boolean = referencedElement.isDefaultable
    -  def defaultValueAsString = referencedElement.defaultValueAsString
    +  //  // These will just delegate to the referenced element declaration
    --- End diff --
    
    delete


---

[GitHub] incubator-daffodil pull request #5: Property Resolution - part of fixing sch...

Posted by mbeckerle <gi...@git.apache.org>.
Github user mbeckerle commented on a diff in the pull request:

    https://github.com/apache/incubator-daffodil/pull/5#discussion_r149844012
  
    --- Diff: daffodil-core/src/main/scala/edu/illinois/ncsa/daffodil/dsom/Root.scala ---
    @@ -0,0 +1,20 @@
    +package edu.illinois.ncsa.daffodil.dsom
    +
    +import edu.illinois.ncsa.daffodil.grammar.RootGrammarMixin
    +import edu.illinois.ncsa.daffodil.xml.NamedQName
    +
    +final class Root(parentArg: SchemaDocument,
    --- End diff --
    
    Scaladoc. The Root is a specialized kind of ElementRef that doesn't live within a model group. 


---

[GitHub] incubator-daffodil pull request #5: Property Resolution - part of fixing sch...

Posted by stevedlawrence <gi...@git.apache.org>.
Github user stevedlawrence commented on a diff in the pull request:

    https://github.com/apache/incubator-daffodil/pull/5#discussion_r151132760
  
    --- Diff: daffodil-core/src/main/scala/edu/illinois/ncsa/daffodil/dsom/package.scala ---
    @@ -0,0 +1,100 @@
    +package edu.illinois.ncsa.daffodil
    +
    +/**
    + * DSOM - DFDL Schema Object Model
    + *
    + * ==Overview==
    + * DSOM is the abstract syntax "tree" of a DFDL schema. It is not actually a tree, it is
    + * a graph, as there are back-pointers, and shared objects.
    + *
    + * A schema is made up of [[SchemaComponent]] objects. A [[SchemaSet]] is a collection of
    + * [[Schema]]. A schema is a collection of [[SchemaDocument]] that have a common
    + * namespace. The [[SchemaSet]] is the
    + * ultimate root of all the objects in a compilation unit. The [[Term]] class.
    + *
    + * Many [[SchemaComponent]] carry DFDL annotations; hence, [[AnnotatedSchemaComponent]]
    + * is a key base trait.
    + *
    + * ==UML Class Diagram==
    + *
    + * See the
    + * [[https://cwiki.apache.org/confluence/display/DAFFODIL/DFDL+Schema+Object+Model+%28DSOM%29+with+UML
    + * Daffodil Wiki]]
    + * for class diagrams.
    + *
    + * ==Terminology==
    + *
    + * Parsing - in this description we are talking about the Daffodil Schema Compiler.
    + * So when we refer to "parsing" the XML, we are referring
    + * to the recursive descent walk of the DFDL schema, with that schema represented as Scala's
    + * `scala.xml.Node` objects. Strictly speaking, the string text in files of the DFDL
    + * schema's XML is already parsed into Scala's `scala.xml.Node` objects, but
    + * it is the walk through that structure constructing the DSOM tree/graph that we
    + * refer to as "parsing" the DFDL schema.
    + *
    + * ==Principles of Operation==
    + *
    + * === Constructing the DSOM Graph===
    + *
    + * The DSOM object graph must be constructed by looking at only the XML without
    + * examining any DFDL annotations. The DSOM structure is required in order to
    + * implement DFDL's scoping rules for finding annotations including both
    + * properties (like dfdl:byteOrder) and statements (like dfdl:assert); hence,
    + * one must have the DSOM graph before one can begin
    + * accessing DFDL annotations or you end up in cycles/stack-overflows.
    + *
    + * This requires a careful consideration of class/trait members and methods that
    + * are used when constructing the DSOM graph, and those used after the DSOM
    + * graph has been created, in order to compile it into the runtime data structures.
    + *
    + * There are a few exceptions to the above. The dfdl:hiddenGroupRef attribute is one such. It
    + * must be local to the [[Sequence]] object, and has implications for the parsing
    + * of the XML as it implies there should be no children of that xs:sequence.
    + * Since it is not scoped, the DSOM graph is not needed in order to access it.
    + * Only the local [[Sequence]] object and it's [[DFDLSequence]] annotation object.
    + * The [[AnnotatedSchemaComponent]] trait provides methods for this local-only
    + * property lookup.
    + *
    + * The DSOM object graph is also needed in order to issue good diagnostic
    + * messages from the compiler; hence, Daffodil validates the DFDL schema before
    + * parsing it into the DSOM graph. Careful consideration must be given if a
    + * SchemaDefinitionError (SDE) is issued while constructing the DSOM graph.
    + *
    + * If you run into stack-overflows while the DSOM graph is being constructed, the
    + * above is a common cause of them, as the SDE diagnostic messaging uses DSOM graph
    + * information to construct context information about the error for inclusion
    + * in the messages. If the DSOM graph is still being constructed at that time, then
    + * this can be circular.
    + *
    + * ===Using the DSOM Graph===
    + *
    + * DSOM supports Daffodil schema compilation by way of the
    + * `OOLAG` pattern which
    + * is an object oriented way of using the
    + * [[https://en.wikipedia.org/wiki/Attribute_grammar attribute grammars]] compiler technique.
    + *
    + * Many attributes (in the attribute grammar sense, nothing to do with XML attributes)
    + * are simply Scala lazy val definitions, but some are declared as OOLAG
    + * attributes (using `edu.illinois.ncsa.daffodil.oolag.OOLAG.OOLAGHost.LV`) which
    + * provides for gathering of multiple diagnostic messages (`SchemaDefinitionError`)
    + * before abandoning compilation.
    + *
    + * DFDL schema compilation largely occurs by evaluating lazy val members of DSOM
    + * objects. These include the members of the grammar traits
    + * (@see [[edu.illinois.ncsa.daffodil.grammar]] package), which are mixed
    + * in to the appropriate DSOM traits/classes.
    + *
    + * ===FAQ===
    + * Q: Why invent this? Why not use XSOM, or the Apache XML Schema library?
    + *
    + * A:We had trouble with other XML-schema libraries for
    + * lack of adequate support for annotations, non-native attributes, and schema
    + * documents as first class objects. So DSOM is specific to Daffodil Basically these
    + * libraries are more about implementing XML Schema and validation, and not so
    + * much about a complex language built on the annotations of the schema. DSOM
    + * is really mostly about the annotations.
    + *
    + */
    +package object dsom {
    +  // This package object is just for scaladoc
    +}
    --- End diff --
    
    These are very helpful!  We'll definitely want to continue adding package.scala files to help new developers onboard.


---

[GitHub] incubator-daffodil pull request #5: Property Resolution - part of fixing sch...

Posted by mbeckerle <gi...@git.apache.org>.
Github user mbeckerle commented on a diff in the pull request:

    https://github.com/apache/incubator-daffodil/pull/5#discussion_r149846600
  
    --- Diff: daffodil-sapi/src/test/scala/edu/illinois/ncsa/daffodil/example/TestScalaAPI.scala ---
    @@ -196,7 +196,7 @@ class TestScalaAPI {
         val locs = d.getLocationsInSchemaFiles
         assertEquals(1, locs.size)
         val loc = locs(0)
    -    assertTrue(loc.toString().contains("mySchema2.dfdl.xsd"))
    +    assertTrue(loc.toString().contains("mySchema1.dfdl.xsd")) // reports the element ref, not element decl.
    --- End diff --
    
    See comment in TestJavaAPI.java. This change should be unnecessary.


---

[GitHub] incubator-daffodil pull request #5: Property Resolution - part of fixing sch...

Posted by mbeckerle <gi...@git.apache.org>.
Github user mbeckerle commented on a diff in the pull request:

    https://github.com/apache/incubator-daffodil/pull/5#discussion_r149839947
  
    --- Diff: daffodil-core/src/main/scala/edu/illinois/ncsa/daffodil/dsom/GroupBase.scala ---
    @@ -32,28 +32,26 @@
     
     package edu.illinois.ncsa.daffodil.dsom
     
    -import scala.xml.Node
    -import scala.xml._
     import edu.illinois.ncsa.daffodil.schema.annotation.props.AlignmentType
     import edu.illinois.ncsa.daffodil.schema.annotation.props.gen.AlignmentUnits
     import java.lang.{ Integer => JInt }
     
    -abstract class GroupBase(xmlArg: Node, parentArg: SchemaComponent, position: Int)
    -  extends Term(xmlArg, parentArg, position) {
    +trait GroupBase
    --- End diff --
    
    GroupBase and ModelGroup can be collapsed together now. The right name for the merged trait/class is ModelGroupTerm. Similarly the right name for ElementBase is probably ElementTerm.


---

[GitHub] incubator-daffodil pull request #5: Property Resolution - part of fixing sch...

Posted by mbeckerle <gi...@git.apache.org>.
Github user mbeckerle commented on a diff in the pull request:

    https://github.com/apache/incubator-daffodil/pull/5#discussion_r149836777
  
    --- Diff: daffodil-core/src/main/scala/edu/illinois/ncsa/daffodil/dsom/AnnotatedSchemaComponent.scala ---
    @@ -41,23 +41,122 @@ import edu.illinois.ncsa.daffodil.equality._
     import edu.illinois.ncsa.daffodil.schema.annotation.props.PropertyLookupResult
     import edu.illinois.ncsa.daffodil.schema.annotation.props.NotFound
     import edu.illinois.ncsa.daffodil.schema.annotation.props.Found
    +import edu.illinois.ncsa.daffodil.schema.annotation.props.FindPropertyMixin
    +
    +/**
    + * Only objects from which we generate processors (parsers/unparsers)
    + * can lookup property values.
    + *
    + * This avoids the possibility of a property being resolved incorrectly by
    + * not looking at the complete chain of schema components contributing to the
    + * property resolution.
    + *
    + * The only objects that should resolve properties are
    + * ElementRef, Root, LocalElementDecl, Sequence, Choice, SequenceRef, ChoiceRef
    + *
    + * These are all the "real" terms. Everything else is just contributing
    + * properties to the mix, but they are not points where properties are
    + * used to generate processors.
    + */
    +trait ResolvesProperties
    +  extends FindPropertyMixin { self: AnnotatedSchemaComponent =>
    +
    +  def term: Term
    +
    +  private def findNonDefaultProperty(pname: String): PropertyLookupResult = {
    --- End diff --
    
    This is not new code, but was moved into this trait.


---

[GitHub] incubator-daffodil issue #5: Property Resolution - part of fixing schema com...

Posted by mbeckerle <gi...@git.apache.org>.
Github user mbeckerle commented on the issue:

    https://github.com/apache/incubator-daffodil/pull/5
  
    VMF works - it was failing for lack of a -Xss3M JVM parameter. We need 3M stacks for these larger schemas.


---

[GitHub] incubator-daffodil pull request #5: Property Resolution - part of fixing sch...

Posted by mbeckerle <gi...@git.apache.org>.
Github user mbeckerle commented on a diff in the pull request:

    https://github.com/apache/incubator-daffodil/pull/5#discussion_r150873946
  
    --- Diff: daffodil-runtime1/src/main/scala/edu/illinois/ncsa/daffodil/infoset/InfosetImpl.scala ---
    @@ -213,7 +213,7 @@ case class InfosetValueLengthUnknownException(lengthState: LengthState, override
     final class FakeDINode extends DISimple(null) {
       private def die = throw new InfosetNoInfosetException(Nope)
     
    -  override def parent = die
    +  override val parent = die
    --- End diff --
    
    That is the only explanation. I've fixed the val by changing to def anyway. 


---

[GitHub] incubator-daffodil pull request #5: Property Resolution - part of fixing sch...

Posted by mbeckerle <gi...@git.apache.org>.
Github user mbeckerle commented on a diff in the pull request:

    https://github.com/apache/incubator-daffodil/pull/5#discussion_r149841148
  
    --- Diff: daffodil-core/src/main/scala/edu/illinois/ncsa/daffodil/dsom/ModelGroup.scala ---
    @@ -115,118 +170,47 @@ abstract class ModelGroup(xmlArg: Node, parentArg: SchemaComponent, position: In
     
       def modelGroupRuntimeData: ModelGroupRuntimeData
     
    -  final lazy val gRefNonDefault: Option[ChainPropProvider] = groupRef.map { _.nonDefaultFormatChain }
    -  final lazy val gRefDefault: Option[ChainPropProvider] = groupRef.map { _.defaultFormatChain }
    -
    -  final def nonDefaultPropertySources = LV('nonDefaultPropertySources) {
    -    val seq = (gRefNonDefault.toSeq ++ Seq(this.nonDefaultFormatChain)).distinct
    -    checkNonOverlap(seq)
    -    seq
    -  }.value
    -
    -  final def defaultPropertySources = LV('defaultPropertySources) {
    -    val seq = (gRefDefault.toSeq ++ Seq(this.defaultFormatChain)).distinct
    -    seq
    -  }.value
    +  protected final lazy val prettyBaseName = xml.label
     
    -  protected final lazy val prettyBaseName = xmlArg.label
    +  def xmlChildren: Seq[Node]
     
    -  protected def xmlChildren: Seq[Node]
    -
    -  private def goodXmlChildren = LV('goodXMLChildren) { xmlChildren.flatMap { removeNonInteresting(_) } }.value
    -  private lazy val positions = List.range(1, goodXmlChildren.length + 1) // range is exclusive on 2nd arg. So +1.
    -  private lazy val pairs = goodXmlChildren zip positions
    -
    -  final lazy val sequenceChildren = groupMembers.collect { case s: Sequence => s }
    -  final lazy val choiceChildren = groupMembers.collect { case s: Choice => s }
    +  final lazy val sequenceChildren = groupMembers.collect { case s: SequenceBase => s }
    +  final lazy val choiceChildren = groupMembers.collect { case s: ChoiceBase => s }
       final lazy val groupRefChildren = groupMembers.collect { case s: GroupRef => s }
     
    -  final def group = this
    -
    -  final lazy val groupMembers = {
    -    pairs.flatMap {
    -      case (n, i) =>
    -        termFactory(n, this, i)
    -    }
    -  }
    +  def group = this
     
       final override lazy val termChildren = groupMembers
    --- End diff --
    
    Try to get rid of these renamings. They just introduce additional names we have to understand. In this case getting rid of termChildren is the right thing. All the groupMembers are always Term, so there can't be any other kind of children. 


---

[GitHub] incubator-daffodil pull request #5: Property Resolution - part of fixing sch...

Posted by mbeckerle <gi...@git.apache.org>.
Github user mbeckerle commented on a diff in the pull request:

    https://github.com/apache/incubator-daffodil/pull/5#discussion_r149843246
  
    --- Diff: daffodil-core/src/main/scala/edu/illinois/ncsa/daffodil/dsom/DFDLStatementMixin.scala ---
    @@ -49,12 +49,13 @@ trait DFDLStatementMixin extends ThrowsSDE { self: AnnotatedSchemaComponent =>
     
    --- End diff --
    
    Consider split and rename: ProvidesDFDLStatementsMixin, and ResolvesDFDLStatementsMixin. Terms would mix in the latter, local and global defining forms would mix in the former. 


---

[GitHub] incubator-daffodil pull request #5: Property Resolution - part of fixing sch...

Posted by mbeckerle <gi...@git.apache.org>.
Github user mbeckerle commented on a diff in the pull request:

    https://github.com/apache/incubator-daffodil/pull/5#discussion_r150628084
  
    --- Diff: daffodil-core/src/main/scala/edu/illinois/ncsa/daffodil/dsom/ChoiceGroup.scala ---
    @@ -78,29 +77,48 @@ import edu.illinois.ncsa.daffodil.api.WarnID
      *
      */
     
    -final class Choice(xmlArg: Node, parent: SchemaComponent, position: Int)
    -  extends ModelGroup(xmlArg, parent, position)
    -  with Choice_AnnotationMixin
    -  with RawDelimitedRuntimeValuedPropertiesMixin // initiator and terminator (not separator)
    -  with ChoiceGrammarMixin {
    +trait ChoiceLikeMixin
    +  extends AnnotatedSchemaComponent
    +  with DFDLStatementMixin {
     
    -  requiredEvaluations(branchesAreNonOptional)
    -  requiredEvaluations(branchesAreNotIVCElements)
    -  requiredEvaluations(modelGroupRuntimeData.preSerialization)
    +  protected def isMyFormatAnnotation(a: DFDLAnnotation) = a.isInstanceOf[DFDLChoice]
     
    -  protected final override lazy val myPeers = choicePeers
    -
    -  protected final override def annotationFactory(node: Node): Option[DFDLAnnotation] = {
    +  protected override def annotationFactory(node: Node): Option[DFDLAnnotation] = {
         node match {
           case <dfdl:choice>{ contents @ _* }</dfdl:choice> => Some(new DFDLChoice(node, this))
           case _ => annotationFactoryForDFDLStatement(node, this)
         }
       }
     
    -  protected final def emptyFormatFactory = new DFDLChoice(newDFDLAnnotationXML("choice"), this)
    -  protected final def isMyFormatAnnotation(a: DFDLAnnotation) = a.isInstanceOf[DFDLChoice]
    +  protected def emptyFormatFactory: DFDLFormatAnnotation = new DFDLChoice(newDFDLAnnotationXML("choice"), this)
    +
    +  lazy val xmlChildren = xml match {
    +    case <choice>{ c @ _* }</choice> => c
    +    case <group>{ _* }</group> => {
    +      val ch = this match {
    +        case cgr: ChoiceGroupRef => cgr.groupDef.xml \\ "choice"
    +        case cgd: GlobalChoiceGroupDef => cgd.xml \\ "choice"
    --- End diff --
    
    Changed to "\". Once I fixed the inheritance to get ChoiceLikeMixin out of ChoiceBase, this now gets a "fruitless type test warning". So removed the first case. 


---

[GitHub] incubator-daffodil pull request #5: Property Resolution - part of fixing sch...

Posted by mbeckerle <gi...@git.apache.org>.
Github user mbeckerle commented on a diff in the pull request:

    https://github.com/apache/incubator-daffodil/pull/5#discussion_r151015228
  
    --- Diff: daffodil-core/src/main/scala/edu/illinois/ncsa/daffodil/dsom/RealTermMixin.scala ---
    @@ -69,226 +65,3 @@ trait PropertyReferencedElementInfosMixin {
     
     }
     
    -/**
    --- End diff --
    
    RealTerm merged into Term


---

[GitHub] incubator-daffodil pull request #5: Property Resolution - part of fixing sch...

Posted by mbeckerle <gi...@git.apache.org>.
Github user mbeckerle commented on a diff in the pull request:

    https://github.com/apache/incubator-daffodil/pull/5#discussion_r149845012
  
    --- Diff: daffodil-lib/src/main/scala/edu/illinois/ncsa/daffodil/oolag/OOLAG.scala ---
    @@ -131,18 +131,30 @@ object OOLAG extends Logging {
        * already. This insures that the value exists for 'foo', or any errors/warnings
        * to be determined by its calculation have been recorded.
        */
    -  abstract class OOLAGHost private (oolagContextArg: OOLAGHost, nArgs: Args)
    +  abstract class OOLAGHostImpl private (
    --- End diff --
    
    Also sclaadoc should explain this dual track thing where we have OOLAGHost trait, and OOLAGHostImpl which is a class.
    
    We needed this to enable richer combinations of mixins. 


---

[GitHub] incubator-daffodil pull request #5: Property Resolution - part of fixing sch...

Posted by stevedlawrence <gi...@git.apache.org>.
Github user stevedlawrence commented on a diff in the pull request:

    https://github.com/apache/incubator-daffodil/pull/5#discussion_r150870368
  
    --- Diff: daffodil-runtime1/src/main/scala/edu/illinois/ncsa/daffodil/infoset/InfosetImpl.scala ---
    @@ -213,7 +213,7 @@ case class InfosetValueLengthUnknownException(lengthState: LengthState, override
     final class FakeDINode extends DISimple(null) {
       private def die = throw new InfosetNoInfosetException(Nope)
     
    -  override def parent = die
    +  override val parent = die
    --- End diff --
    
    Maybe FakeDINode is just never allocated? Looks like it's only allocated when currentNode is called in DStateForConstantFolding. Maybe we just don't have a test for that?


---

[GitHub] incubator-daffodil pull request #5: Property Resolution - part of fixing sch...

Posted by mbeckerle <gi...@git.apache.org>.
Github user mbeckerle commented on a diff in the pull request:

    https://github.com/apache/incubator-daffodil/pull/5#discussion_r149847177
  
    --- Diff: daffodil-test/src/test/resources/edu/illinois/ncsa/daffodil/section14/sequence_groups/SequenceGroupDelimiters.tdml ---
    @@ -357,7 +357,8 @@
     
       <tdml:parserTestCase name="groupRefInheritProps"
         root="list" model="groupRefInheritProps.xsd"
    -    description="separator defined on a group reference - DFDL-14-008R">
    +    description="separator defined on a group reference - DFDL-14-008R"
    +    roundTrip="false">
    --- End diff --
    
    Does this need to be roundTrip false still? Revert and test. 


---

[GitHub] incubator-daffodil pull request #5: Property Resolution - part of fixing sch...

Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:

    https://github.com/apache/incubator-daffodil/pull/5


---

[GitHub] incubator-daffodil pull request #5: Property Resolution - part of fixing sch...

Posted by stevedlawrence <gi...@git.apache.org>.
Github user stevedlawrence commented on a diff in the pull request:

    https://github.com/apache/incubator-daffodil/pull/5#discussion_r151135172
  
    --- Diff: daffodil-test/src/test/scala-new/edu/illinois/ncsa/daffodil/section07/external_variables/TestExternalVariablesNew.scala ---
    @@ -53,4 +53,12 @@ class TestExternalVariablesNew {
         runner.runOneTest("read_config_from_file")
       }
     
    +  //
    +  // Use to reproduce CLI test test_2360_CLI_Parsing_SimpleParse_stdOut_extVars2
    +  // TODO: When Daffodil-1846 is fixed this test could be enabled.
    +  // @Test
    +  def test_testNoRootUnnecessaryBinding(): Unit = {
    +    runner.trace.runOneTest("testNoRootUnnecessaryBinding")
    +  }
    +
    --- End diff --
    
    Rather than commenting out @Test, can this be put in scala-debug? Makes it easier to run the test when someone goes to fix it. Also, makes it easier to just run all debug tests and see if tests started working due and we just forgot to move a test to scala. 


---

[GitHub] incubator-daffodil issue #5: Property Resolution - part of fixing schema com...

Posted by stevedlawrence <gi...@git.apache.org>.
Github user stevedlawrence commented on the issue:

    https://github.com/apache/incubator-daffodil/pull/5
  
    +1


---

[GitHub] incubator-daffodil pull request #5: Property Resolution - part of fixing sch...

Posted by VTGuy <gi...@git.apache.org>.
Github user VTGuy commented on a diff in the pull request:

    https://github.com/apache/incubator-daffodil/pull/5#discussion_r151153142
  
    --- Diff: daffodil-core/src/main/scala/edu/illinois/ncsa/daffodil/dsom/DFDLStatementMixin.scala ---
    @@ -38,16 +38,83 @@ import edu.illinois.ncsa.daffodil.exceptions.ThrowsSDE
     import edu.illinois.ncsa.daffodil.schema.annotation.props.gen.TestKind
     import edu.illinois.ncsa.daffodil.grammar.primitives.AssertBase
     
    +trait ResolvesDFDLStatementMixin
    +  extends ThrowsSDE
    +  with ProvidesDFDLStatementMixin { self: Term =>
    +
    +  requiredEvaluations(statements)
    +
    +  final lazy val statements: Seq[DFDLStatement] =
    +    optReferencedStatementSource.toSeq.flatMap { _.resolvedStatements } ++
    +      localStatements
    +
    +  private def getParserExprReferencedElements(s: DFDLStatement,
    +    f: ContentValueReferencedElementInfoMixin => Set[DPathElementCompileInfo]) = {
    +    s match {
    +
    +      case a: DFDLAssertionBase if (a.testKind eq TestKind.Expression) => {
    +        a.gram match {
    +          case ab: AssertBase => f(ab.expr)
    +          case _ => ReferencedElementInfos.None
    +        }
    +      }
    +
    +      case _ => getUnparserExprReferencedElements(s, f)
    +    }
    +  }
    +
    +  private def getUnparserExprReferencedElements(s: DFDLStatement,
    +    f: ContentValueReferencedElementInfoMixin => Set[DPathElementCompileInfo]) = {
    +    s match {
    +      case sv: DFDLSetVariable => {
    +        val mdv = sv.defv.maybeDefaultValueExpr
    +        if (mdv.isDefined)
    +          f(mdv.get)
    +        else
    +          ReferencedElementInfos.None
    +      }
    +      case nv: DFDLNewVariableInstance => {
    +        // nv.defaultValueExpr.contentReferencedElementInfos
    +        s.notYetImplemented("dfdl:newVariableInstance")
    +      }
    +      case _ => ReferencedElementInfos.None
    +    }
    +  }
    +
    +  final protected def creis(rei: ContentValueReferencedElementInfoMixin) = rei.contentReferencedElementInfos
    +  final protected def vreis(rei: ContentValueReferencedElementInfoMixin) = rei.valueReferencedElementInfos
    +
    +  private def statementReferencedElementInfos(f: DFDLStatement => Set[DPathElementCompileInfo]) = {
    +
    --- End diff --
    
    Same here regarding extra space.


---

[GitHub] incubator-daffodil pull request #5: Property Resolution - part of fixing sch...

Posted by stevedlawrence <gi...@git.apache.org>.
Github user stevedlawrence commented on a diff in the pull request:

    https://github.com/apache/incubator-daffodil/pull/5#discussion_r151129750
  
    --- Diff: daffodil-core/src/main/scala/edu/illinois/ncsa/daffodil/dsom/Root.scala ---
    @@ -2,19 +2,31 @@ package edu.illinois.ncsa.daffodil.dsom
     
     import edu.illinois.ncsa.daffodil.grammar.RootGrammarMixin
     import edu.illinois.ncsa.daffodil.xml.NamedQName
    +import edu.illinois.ncsa.daffodil.xml.XMLUtils
    +import scala.xml.Node
     
    -final class Root(parentArg: SchemaDocument,
    +/**
    + * Root is a special kind of ElementRef that has no enclosing group.
    + *
    + * This is the entity that is compiled by the schema compiler.
    + */
    +final class Root(defXML: Node, parentArg: SchemaDocument,
       namedQNameArg: NamedQName,
       globalElementDecl: => GlobalElementDecl)
    -  extends AbstractElementRef(
    -    <root/>, // % Attribute(None, "ref", Text(refQName.toQNameString), Null), // have to have ref attribute.
    -    parentArg, 1)
    +  extends AbstractElementRef(null, parentArg, 1)
       with RootGrammarMixin {
     
    +  final override lazy val xml = {
    +    val elem = XMLUtils.getXSDElement(defXML.scope)
    +    val attrs = 
    +    <dummy ref={ refQName.toQNameString }/>.attributes
    +    val res = elem % attrs
    --- End diff --
    
    Seems an odd way to add an attribute. Can you do something like
    ```
    val res = elem % Attribute("", "ref", refQName.toQNameString, scala.xml.Null)
    ```
    Not sure if Null is correct for the ``next``  parameter, but something like that is a little more clear to me.


---

[GitHub] incubator-daffodil pull request #5: Property Resolution - part of fixing sch...

Posted by stevedlawrence <gi...@git.apache.org>.
Github user stevedlawrence commented on a diff in the pull request:

    https://github.com/apache/incubator-daffodil/pull/5#discussion_r151131415
  
    --- Diff: daffodil-core/src/main/scala/edu/illinois/ncsa/daffodil/dsom/package.scala ---
    @@ -0,0 +1,100 @@
    +package edu.illinois.ncsa.daffodil
    +
    +/**
    + * DSOM - DFDL Schema Object Model
    + *
    + * ==Overview==
    + * DSOM is the abstract syntax "tree" of a DFDL schema. It is not actually a tree, it is
    + * a graph, as there are back-pointers, and shared objects.
    + *
    + * A schema is made up of [[SchemaComponent]] objects. A [[SchemaSet]] is a collection of
    + * [[Schema]]. A schema is a collection of [[SchemaDocument]] that have a common
    + * namespace. The [[SchemaSet]] is the
    + * ultimate root of all the objects in a compilation unit. The [[Term]] class.
    --- End diff --
    
    Sentence fragment "The Term class". Did you mean to add more?


---

[GitHub] incubator-daffodil pull request #5: Property Resolution - part of fixing sch...

Posted by mbeckerle <gi...@git.apache.org>.
Github user mbeckerle commented on a diff in the pull request:

    https://github.com/apache/incubator-daffodil/pull/5#discussion_r149844708
  
    --- Diff: daffodil-core/src/main/scala/edu/illinois/ncsa/daffodil/dsom/TermEncodingMixin.scala ---
    @@ -114,9 +114,6 @@ trait TermEncodingMixin extends KnownEncodingMixin { self: Term =>
             mg.hasNoSkipRegions &&
               hasTextAlignment
           }
    -      case gr: GroupRef => {
    --- End diff --
    
    Add scaladoc to this trait to explain what its role is. Maybe rename to something more specific. 


---

[GitHub] incubator-daffodil pull request #5: Property Resolution - part of fixing sch...

Posted by mbeckerle <gi...@git.apache.org>.
Github user mbeckerle commented on a diff in the pull request:

    https://github.com/apache/incubator-daffodil/pull/5#discussion_r149845076
  
    --- Diff: daffodil-lib/src/main/scala/edu/illinois/ncsa/daffodil/oolag/OOLAG.scala ---
    @@ -131,18 +131,30 @@ object OOLAG extends Logging {
        * already. This insures that the value exists for 'foo', or any errors/warnings
        * to be determined by its calculation have been recorded.
        */
    -  abstract class OOLAGHost private (oolagContextArg: OOLAGHost, nArgs: Args)
    +  abstract class OOLAGHostImpl private (
    +    final override val oolagContextArg: OOLAGHost,
    --- End diff --
    
    Rename. Shouldn't be named with "Arg" suffix if it's public.


---

[GitHub] incubator-daffodil pull request #5: Property Resolution - part of fixing sch...

Posted by mbeckerle <gi...@git.apache.org>.
Github user mbeckerle commented on a diff in the pull request:

    https://github.com/apache/incubator-daffodil/pull/5#discussion_r150624809
  
    --- Diff: daffodil-core/src/main/scala/edu/illinois/ncsa/daffodil/dsom/ChoiceGroup.scala ---
    @@ -257,3 +275,13 @@ final class Choice(xmlArg: Node, parent: SchemaComponent, position: Int)
           optIgnoreCase)
       }
     }
    +
    +final class Choice(xmlArg: Node, parent: SchemaComponent, position: Int)
    +  extends ChoiceBase(xmlArg, parent, position)
    +  with ChoiceLikeMixin
    --- End diff --
    
    ChoiceBase should not mixin ChoiceLikeMixin, as ChoiceLikeMixin is for things that *define* choice groups, and ChoiceBase is base for terms that act like choice groups at runtime. 
    ChoiceBase is base of ChoiceRefs as well as Choice. ChoiceRefs don't define a choice group.  
    
    This is why I think I should rename ChoiceLikeMixin to ChoiceDefMixin. 


---

[GitHub] incubator-daffodil pull request #5: Property Resolution - part of fixing sch...

Posted by stevedlawrence <gi...@git.apache.org>.
Github user stevedlawrence commented on a diff in the pull request:

    https://github.com/apache/incubator-daffodil/pull/5#discussion_r150864925
  
    --- Diff: daffodil-core/src/main/scala/edu/illinois/ncsa/daffodil/dsom/Term.scala ---
    @@ -115,14 +115,14 @@ abstract class Term(xmlArg: Node, parentArg: SchemaComponent, val position: Int)
     
       def elementChildren: Seq[ElementBase]
     
    -  override lazy val dpathCompileInfo =
    -    new DPathCompileInfo(
    -      enclosingComponent.map { _.dpathCompileInfo },
    -      variableMap,
    -      namespaces,
    -      path,
    -      schemaFileLocation,
    -      tunable)
    +  //  override lazy val dpathCompileInfo =
    +  //    new DPathCompileInfo(
    +  //      enclosingComponent.map { _.dpathCompileInfo },
    +  //      variableMap,
    +  //      namespaces,
    +  //      path,
    +  //      schemaFileLocation,
    +  //      tunable)
    --- End diff --
    
    Remove.


---

[GitHub] incubator-daffodil pull request #5: Property Resolution - part of fixing sch...

Posted by mbeckerle <gi...@git.apache.org>.
Github user mbeckerle commented on a diff in the pull request:

    https://github.com/apache/incubator-daffodil/pull/5#discussion_r149840571
  
    --- Diff: daffodil-core/src/main/scala/edu/illinois/ncsa/daffodil/dsom/ModelGroup.scala ---
    @@ -80,17 +96,56 @@ sealed abstract class ModelGroupFactory private () {
     
     object ModelGroupFactory extends ModelGroupFactory()
     
    +sealed abstract class TermFactory private () {
    +  /**
    +   * Factory for Terms
    +   *
    +   * Because of the context where this is used, this returns a list. Nil for non-terms, non-Nil for
    +   * an actual term. There should be only one non-Nil.
    +   *
    +   * This could be static code in an object. It doesn't reference any of the state of the ModelGroup,
    +   * it's here so that type-specific overrides are possible in Sequence or Choice
    +   */
    +  def apply(child: Node, parent: GroupDefLike, position: Int) = {
    +    val childList: List[Term] = child match {
    +      case <element>{ _* }</element> => {
    +        val refProp = child.attribute("ref").map { _.text }
    +        // must get an unprefixed attribute name, i.e. ref='foo:bar', and not
    +        // be tripped up by dfdl:ref="fmt:fooey" which is a format reference.
    +        refProp match {
    +          case None => {
    +            val factory = parent.schemaSet.LocalElementDeclFactory(
    +              child,
    +              parent.schemaDocument)
    +            val eDecl = factory.forModelGroup(parent, position)
    +            List(eDecl)
    +          }
    +          case Some(_) => List(new ElementRef(child, parent, position))
    +        }
    +      }
    +      case <annotation>{ _* }</annotation> => Nil
    +      case textNode: Text => Nil
    +      case _ => ModelGroupFactory(child, parent, position, false)
    +    }
    +    childList
    +  }
    +}
    +
    +object TermFactory extends TermFactory()
    --- End diff --
    
    collapse to just the object. class is unnecessary.


---

[GitHub] incubator-daffodil pull request #5: Property Resolution - part of fixing sch...

Posted by stevedlawrence <gi...@git.apache.org>.
Github user stevedlawrence commented on a diff in the pull request:

    https://github.com/apache/incubator-daffodil/pull/5#discussion_r150858048
  
    --- Diff: daffodil-core/src/main/scala/edu/illinois/ncsa/daffodil/dsom/GroupDef.scala ---
    @@ -34,70 +34,137 @@ package edu.illinois.ncsa.daffodil.dsom
     
     import scala.xml.Node
     import edu.illinois.ncsa.daffodil.exceptions.Assert
    -import edu.illinois.ncsa.daffodil.xml.QName
    +import scala.xml.Text
    +import scala.xml.Comment
     
    -class GlobalGroupDefFactory(xmlArg: Node, schemaDocumentArg: SchemaDocument)
    -  extends SchemaComponentFactory(xmlArg, schemaDocumentArg)
    +final class GlobalGroupDefFactory(defXML: Node, schemaDocument: SchemaDocument)
    +  extends SchemaComponentFactory(defXML, schemaDocument)
       with GlobalNonElementComponentMixin {
     
    -  private lazy val trimmedXml = scala.xml.Utility.trim(xmlArg)
    -
    -  def forGroupRef(gref: GroupRef, position: Int) = {
    +  def forGroupRef(refXML: Node, refLexicalParent: SchemaComponent, position: Int,
    +    isHidden: Boolean): (GroupRef, GlobalGroupDef) = {
    +    val trimmedXml = scala.xml.Utility.trim(defXML)
         trimmedXml match {
           case <group>{ contents @ _* }</group> => {
    -        val ggd = contents.collect {
    -          case <sequence>{ _* }</sequence> => new GlobalSequenceGroupDef(xml, schemaDocument, gref, position)
    -          case <choice>{ _* }</choice> => new GlobalChoiceGroupDef(xml, schemaDocument, gref, position)
    +        val list = contents.collect {
    +          case groupXML @ <sequence>{ _* }</sequence> => {
    +            lazy val gref: SequenceGroupRef = new SequenceGroupRef(gdef, refXML, refLexicalParent, position, isHidden)
    +            lazy val gdef: GlobalSequenceGroupDef = new GlobalSequenceGroupDef(defXML, groupXML, schemaDocument, gref)
    +            gref.groupDef
    +            gdef.groupRef
    +            (gref, gdef)
    +          }
    +          case groupXML @ <choice>{ _* }</choice> =>
    +            lazy val gref: ChoiceGroupRef = new ChoiceGroupRef(gdef, refXML, refLexicalParent, position, isHidden)
    +            lazy val gdef: GlobalChoiceGroupDef = new GlobalChoiceGroupDef(defXML, groupXML, schemaDocument, gref)
    +            gref.groupDef
    +            gdef.groupRef
    +            (gref, gdef)
             }
    -        Assert.invariant(ggd.length == 1)
    -        ggd(0)
    +        val res = list(0)
    +        res
           }
           case _ => Assert.invariantFailed("not a group")
         }
       }
    -
    -  override lazy val namedQName = QName.createGlobal(name, targetNamespace, xml.scope)
    -
     }
     
    -sealed abstract class GlobalGroupDef(xmlArg: Node, schemaDocumentArg: SchemaDocument, val groupRef: GroupRef, position: Int)
    -  extends SchemaComponent(xmlArg, schemaDocumentArg)
    -  with GlobalNonElementComponentMixin 
    -  with NestingTraversesToReferenceMixin {
    -
    -  requiredEvaluations(modelGroup)
    -  requiredEvaluations(validateChoiceBranchKey)
    -
    -  final lazy val referringComponent = {
    -    val res = Some(groupRef)
    -    res
    -  }
    +/**
    + * Many things can be contained either by a model group or a
    + * group definition (aka global sequence group definition or global choice
    + * group definition).
    + *
    + * These things are GroupLike.
    + */
    +trait GroupDefLike
    +  extends AnnotatedSchemaComponent {
     
    -  // final override protected def enclosingComponentDef = groupRef.enclosingComponent
    +  def xmlChildren: Seq[Node]
     
       //
    -  // Note: Dealing with XML can be fragile. It's easy to forget some of these children
    -  // might be annotations and Text nodes. Even if you trim the text nodes out, there are
    -  // places where annotations can be.
    -  //
    -  final lazy val <group>{ xmlChildren @ _* }</group> = xml
    -  //
       // So we have to flatMap, so that we can tolerate annotation objects (like documentation objects).
       // and our ModelGroup factory has to return Nil for annotations and Text nodes.
       //
    -  final lazy val Seq(modelGroup: ModelGroup) = xmlChildren.flatMap { ModelGroupFactory(_, this, position) }
    +  final lazy val groupMembers: Seq[Term] = LV('groupMembers) {
    +    val goodXmlChildren = LV('goodXMLChildren) { xmlChildren.flatMap { removeNonInteresting(_) } }.value
    +    val positions = List.range(1, goodXmlChildren.length + 1) // range is exclusive on 2nd arg. So +1.
    +    val pairs = goodXmlChildren zip positions
    +    pairs.flatMap {
    +      case (n, i) =>
    +        TermFactory(n, this, i)
    +    }
    +  }.value
    +
    +  /**
    +   * XML is full of uninteresting text nodes. We just want the element children, not all children.
    +   */
    +  private def removeNonInteresting(child: Node) = {
    +    val childList: List[Node] = child match {
    +      case _: Text => Nil
    +      case _: Comment => Nil
    +      case <annotation>{ _* }</annotation> => Nil
    +      case _ => List(child)
    +    }
    +    childList
    +  }
    --- End diff --
    
    I think this will end up creating a copy of the XML, but with text/comment/etc. removed? For very large schemas, this could add some potential overhead. Not something worth worrying about without profiling, but seems like a lot of copying going on that might affect schema compilation.


---

[GitHub] incubator-daffodil pull request #5: Property Resolution - part of fixing sch...

Posted by VTGuy <gi...@git.apache.org>.
Github user VTGuy commented on a diff in the pull request:

    https://github.com/apache/incubator-daffodil/pull/5#discussion_r151152926
  
    --- Diff: daffodil-core/src/main/scala/edu/illinois/ncsa/daffodil/dsom/DFDLStatementMixin.scala ---
    @@ -38,16 +38,83 @@ import edu.illinois.ncsa.daffodil.exceptions.ThrowsSDE
     import edu.illinois.ncsa.daffodil.schema.annotation.props.gen.TestKind
     import edu.illinois.ncsa.daffodil.grammar.primitives.AssertBase
     
    +trait ResolvesDFDLStatementMixin
    +  extends ThrowsSDE
    +  with ProvidesDFDLStatementMixin { self: Term =>
    +
    +  requiredEvaluations(statements)
    +
    +  final lazy val statements: Seq[DFDLStatement] =
    +    optReferencedStatementSource.toSeq.flatMap { _.resolvedStatements } ++
    +      localStatements
    +
    +  private def getParserExprReferencedElements(s: DFDLStatement,
    +    f: ContentValueReferencedElementInfoMixin => Set[DPathElementCompileInfo]) = {
    +    s match {
    +
    --- End diff --
    
    Nit picking here, but extra space is not needed between these lines.


---

[GitHub] incubator-daffodil pull request #5: Property Resolution - part of fixing sch...

Posted by mbeckerle <gi...@git.apache.org>.
Github user mbeckerle commented on a diff in the pull request:

    https://github.com/apache/incubator-daffodil/pull/5#discussion_r150654648
  
    --- Diff: daffodil-core/src/main/scala/edu/illinois/ncsa/daffodil/dsom/GroupDef.scala ---
    @@ -34,70 +34,137 @@ package edu.illinois.ncsa.daffodil.dsom
     
     import scala.xml.Node
     import edu.illinois.ncsa.daffodil.exceptions.Assert
    -import edu.illinois.ncsa.daffodil.xml.QName
    +import scala.xml.Text
    +import scala.xml.Comment
     
    -class GlobalGroupDefFactory(xmlArg: Node, schemaDocumentArg: SchemaDocument)
    -  extends SchemaComponentFactory(xmlArg, schemaDocumentArg)
    +final class GlobalGroupDefFactory(defXML: Node, schemaDocument: SchemaDocument)
    +  extends SchemaComponentFactory(defXML, schemaDocument)
       with GlobalNonElementComponentMixin {
     
    -  private lazy val trimmedXml = scala.xml.Utility.trim(xmlArg)
    -
    -  def forGroupRef(gref: GroupRef, position: Int) = {
    +  def forGroupRef(refXML: Node, refLexicalParent: SchemaComponent, position: Int,
    +    isHidden: Boolean): (GroupRef, GlobalGroupDef) = {
    +    val trimmedXml = scala.xml.Utility.trim(defXML)
         trimmedXml match {
           case <group>{ contents @ _* }</group> => {
    -        val ggd = contents.collect {
    -          case <sequence>{ _* }</sequence> => new GlobalSequenceGroupDef(xml, schemaDocument, gref, position)
    -          case <choice>{ _* }</choice> => new GlobalChoiceGroupDef(xml, schemaDocument, gref, position)
    +        val list = contents.collect {
    +          case groupXML @ <sequence>{ _* }</sequence> => {
    +            lazy val gref: SequenceGroupRef = new SequenceGroupRef(gdef, refXML, refLexicalParent, position, isHidden)
    +            lazy val gdef: GlobalSequenceGroupDef = new GlobalSequenceGroupDef(defXML, groupXML, schemaDocument, gref)
    +            gref.groupDef
    +            gdef.groupRef
    +            (gref, gdef)
    +          }
    +          case groupXML @ <choice>{ _* }</choice> =>
    +            lazy val gref: ChoiceGroupRef = new ChoiceGroupRef(gdef, refXML, refLexicalParent, position, isHidden)
    +            lazy val gdef: GlobalChoiceGroupDef = new GlobalChoiceGroupDef(defXML, groupXML, schemaDocument, gref)
    +            gref.groupDef
    +            gdef.groupRef
    +            (gref, gdef)
             }
    -        Assert.invariant(ggd.length == 1)
    -        ggd(0)
    +        val res = list(0)
    +        res
           }
           case _ => Assert.invariantFailed("not a group")
         }
       }
    -
    -  override lazy val namedQName = QName.createGlobal(name, targetNamespace, xml.scope)
    -
     }
     
    -sealed abstract class GlobalGroupDef(xmlArg: Node, schemaDocumentArg: SchemaDocument, val groupRef: GroupRef, position: Int)
    -  extends SchemaComponent(xmlArg, schemaDocumentArg)
    -  with GlobalNonElementComponentMixin 
    -  with NestingTraversesToReferenceMixin {
    -
    -  requiredEvaluations(modelGroup)
    -  requiredEvaluations(validateChoiceBranchKey)
    -
    -  final lazy val referringComponent = {
    -    val res = Some(groupRef)
    -    res
    -  }
    +/**
    + * Many things can be contained either by a model group or a
    + * group definition (aka global sequence group definition or global choice
    + * group definition).
    + *
    + * These things are GroupLike.
    --- End diff --
    
    Not doing this rename. Changing SequenceBase to SequenceTermBase, and ChoiceBase to ChoiceTermBase instead.


---

[GitHub] incubator-daffodil pull request #5: Property Resolution - part of fixing sch...

Posted by stevedlawrence <gi...@git.apache.org>.
Github user stevedlawrence commented on a diff in the pull request:

    https://github.com/apache/incubator-daffodil/pull/5#discussion_r150593947
  
    --- Diff: daffodil-core/src/main/scala/edu/illinois/ncsa/daffodil/dsom/ChoiceGroup.scala ---
    @@ -78,29 +77,48 @@ import edu.illinois.ncsa.daffodil.api.WarnID
      *
      */
     
    -final class Choice(xmlArg: Node, parent: SchemaComponent, position: Int)
    -  extends ModelGroup(xmlArg, parent, position)
    -  with Choice_AnnotationMixin
    -  with RawDelimitedRuntimeValuedPropertiesMixin // initiator and terminator (not separator)
    -  with ChoiceGrammarMixin {
    +trait ChoiceLikeMixin
    +  extends AnnotatedSchemaComponent
    +  with DFDLStatementMixin {
     
    -  requiredEvaluations(branchesAreNonOptional)
    -  requiredEvaluations(branchesAreNotIVCElements)
    -  requiredEvaluations(modelGroupRuntimeData.preSerialization)
    +  protected def isMyFormatAnnotation(a: DFDLAnnotation) = a.isInstanceOf[DFDLChoice]
     
    -  protected final override lazy val myPeers = choicePeers
    -
    -  protected final override def annotationFactory(node: Node): Option[DFDLAnnotation] = {
    +  protected override def annotationFactory(node: Node): Option[DFDLAnnotation] = {
         node match {
           case <dfdl:choice>{ contents @ _* }</dfdl:choice> => Some(new DFDLChoice(node, this))
           case _ => annotationFactoryForDFDLStatement(node, this)
         }
       }
     
    -  protected final def emptyFormatFactory = new DFDLChoice(newDFDLAnnotationXML("choice"), this)
    -  protected final def isMyFormatAnnotation(a: DFDLAnnotation) = a.isInstanceOf[DFDLChoice]
    +  protected def emptyFormatFactory: DFDLFormatAnnotation = new DFDLChoice(newDFDLAnnotationXML("choice"), this)
    +
    +  lazy val xmlChildren = xml match {
    +    case <choice>{ c @ _* }</choice> => c
    +    case <group>{ _* }</group> => {
    +      val ch = this match {
    +        case cgr: ChoiceGroupRef => cgr.groupDef.xml \\ "choice"
    +        case cgd: GlobalChoiceGroupDef => cgd.xml \\ "choice"
    +      }
    +      val <choice>{ c @ _* }</choice> = ch(0)
    +      c
    +    }
    +  }
    +}
    +
    +abstract class ChoiceBase( final override val xml: Node,
    +  final override val parent: SchemaComponent,
    +  final override val position: Int)
    +  extends ModelGroup
    +  with ChoiceLikeMixin
    --- End diff --
    
    The UML diagram doesn't show ChoiceBase mixing in ChoiceLikeMixin, is this correct?


---

[GitHub] incubator-daffodil pull request #5: Property Resolution - part of fixing sch...

Posted by stevedlawrence <gi...@git.apache.org>.
Github user stevedlawrence commented on a diff in the pull request:

    https://github.com/apache/incubator-daffodil/pull/5#discussion_r150855789
  
    --- Diff: daffodil-core/src/main/scala/edu/illinois/ncsa/daffodil/dsom/GlobalElementDeclFactory.scala ---
    @@ -50,10 +50,16 @@ class GlobalElementDeclFactory(xmlArg: Node, schemaDocumentArg: SchemaDocument)
       with GlobalElementComponentMixin {
     
       def forRoot() = asRoot // cache. Not a new one every time.
    -  lazy val asRoot = new GlobalElementDecl(this, None)
    +  lazy val asRoot = {
    +    lazy val ged = new GlobalElementDecl(this, root)
    +    lazy val root: Root = new Root(schemaDocument, namedQName, ged)
    +    root
    +  }
     
    -  def forElementRef(eRef: ElementRef) = {
    -    new GlobalElementDecl(this, Some(eRef))
    +  def forElementRef(eRef: AbstractElementRef) = {
    +    if (this.namedQName.local == "NS_06")
    +      println("stop here")
    --- End diff --
    
    Looks like debug statements? Can be removed.


---

[GitHub] incubator-daffodil pull request #5: Property Resolution - part of fixing sch...

Posted by mbeckerle <gi...@git.apache.org>.
Github user mbeckerle commented on a diff in the pull request:

    https://github.com/apache/incubator-daffodil/pull/5#discussion_r149840944
  
    --- Diff: daffodil-core/src/main/scala/edu/illinois/ncsa/daffodil/dsom/ModelGroup.scala ---
    @@ -80,17 +96,56 @@ sealed abstract class ModelGroupFactory private () {
     
     object ModelGroupFactory extends ModelGroupFactory()
     
    +sealed abstract class TermFactory private () {
    +  /**
    +   * Factory for Terms
    +   *
    +   * Because of the context where this is used, this returns a list. Nil for non-terms, non-Nil for
    +   * an actual term. There should be only one non-Nil.
    +   *
    +   * This could be static code in an object. It doesn't reference any of the state of the ModelGroup,
    +   * it's here so that type-specific overrides are possible in Sequence or Choice
    +   */
    +  def apply(child: Node, parent: GroupDefLike, position: Int) = {
    +    val childList: List[Term] = child match {
    +      case <element>{ _* }</element> => {
    +        val refProp = child.attribute("ref").map { _.text }
    +        // must get an unprefixed attribute name, i.e. ref='foo:bar', and not
    +        // be tripped up by dfdl:ref="fmt:fooey" which is a format reference.
    +        refProp match {
    +          case None => {
    +            val factory = parent.schemaSet.LocalElementDeclFactory(
    +              child,
    +              parent.schemaDocument)
    +            val eDecl = factory.forModelGroup(parent, position)
    +            List(eDecl)
    +          }
    +          case Some(_) => List(new ElementRef(child, parent, position))
    +        }
    +      }
    +      case <annotation>{ _* }</annotation> => Nil
    +      case textNode: Text => Nil
    +      case _ => ModelGroupFactory(child, parent, position, false)
    +    }
    +    childList
    +  }
    +}
    +
    +object TermFactory extends TermFactory()
    +
     /**
      * Base class for all model groups, which are term containers.
      */
    --- End diff --
    
    Expand scaladoc. What are the ultimate kinds of ModelGroups?  SequenceGroupRef, ChoiceGroupRef, Sequence, and Choice. (Those may be renamed tho.)


---

[GitHub] incubator-daffodil pull request #5: Property Resolution - part of fixing sch...

Posted by mbeckerle <gi...@git.apache.org>.
Github user mbeckerle commented on a diff in the pull request:

    https://github.com/apache/incubator-daffodil/pull/5#discussion_r150624101
  
    --- Diff: daffodil-core/src/main/scala/edu/illinois/ncsa/daffodil/dsom/ComplexTypes.scala ---
    @@ -1,7 +1,7 @@
     /* Copyright (c) 2012-2015 Tresys Technology, LLC. All rights reserved.
      *
      * Developed by: Tresys Technology, LLC
    - *               http://www.tresys.com
    + *               http://woverride val ww.tresys.com
    --- End diff --
    
    Found 6 or 7 of these. Must have been a query replace error.


---

[GitHub] incubator-daffodil pull request #5: Property Resolution - part of fixing sch...

Posted by mbeckerle <gi...@git.apache.org>.
Github user mbeckerle commented on a diff in the pull request:

    https://github.com/apache/incubator-daffodil/pull/5#discussion_r149838765
  
    --- Diff: daffodil-core/src/main/scala/edu/illinois/ncsa/daffodil/dsom/GroupDef.scala ---
    @@ -1,7 +1,7 @@
     /* Copyright (c) 2012-2015 Tresys Technology, LLC. All rights reserved.
      *
      * Developed by: Tresys Technology, LLC
    - *               http://www.tresys.com
    + *               http://woverride val ww.tresys.com
    --- End diff --
    
    Something got clobbered in some automated change here.


---

[GitHub] incubator-daffodil pull request #5: Property Resolution - part of fixing sch...

Posted by mbeckerle <gi...@git.apache.org>.
Github user mbeckerle commented on a diff in the pull request:

    https://github.com/apache/incubator-daffodil/pull/5#discussion_r149840520
  
    --- Diff: daffodil-core/src/main/scala/edu/illinois/ncsa/daffodil/dsom/ModelGroup.scala ---
    @@ -80,17 +96,56 @@ sealed abstract class ModelGroupFactory private () {
     
     object ModelGroupFactory extends ModelGroupFactory()
    --- End diff --
    
    collapse this to just the object. class is not needed.


---

[GitHub] incubator-daffodil pull request #5: Property Resolution - part of fixing sch...

Posted by mbeckerle <gi...@git.apache.org>.
Github user mbeckerle commented on a diff in the pull request:

    https://github.com/apache/incubator-daffodil/pull/5#discussion_r149842123
  
    --- Diff: daffodil-core/src/main/scala/edu/illinois/ncsa/daffodil/dsom/GroupDef.scala ---
    @@ -34,70 +34,137 @@ package edu.illinois.ncsa.daffodil.dsom
     
     import scala.xml.Node
     import edu.illinois.ncsa.daffodil.exceptions.Assert
    -import edu.illinois.ncsa.daffodil.xml.QName
    +import scala.xml.Text
    +import scala.xml.Comment
     
    -class GlobalGroupDefFactory(xmlArg: Node, schemaDocumentArg: SchemaDocument)
    -  extends SchemaComponentFactory(xmlArg, schemaDocumentArg)
    +final class GlobalGroupDefFactory(defXML: Node, schemaDocument: SchemaDocument)
    +  extends SchemaComponentFactory(defXML, schemaDocument)
       with GlobalNonElementComponentMixin {
     
    -  private lazy val trimmedXml = scala.xml.Utility.trim(xmlArg)
    -
    -  def forGroupRef(gref: GroupRef, position: Int) = {
    +  def forGroupRef(refXML: Node, refLexicalParent: SchemaComponent, position: Int,
    +    isHidden: Boolean): (GroupRef, GlobalGroupDef) = {
    +    val trimmedXml = scala.xml.Utility.trim(defXML)
         trimmedXml match {
           case <group>{ contents @ _* }</group> => {
    -        val ggd = contents.collect {
    -          case <sequence>{ _* }</sequence> => new GlobalSequenceGroupDef(xml, schemaDocument, gref, position)
    -          case <choice>{ _* }</choice> => new GlobalChoiceGroupDef(xml, schemaDocument, gref, position)
    +        val list = contents.collect {
    +          case groupXML @ <sequence>{ _* }</sequence> => {
    +            lazy val gref: SequenceGroupRef = new SequenceGroupRef(gdef, refXML, refLexicalParent, position, isHidden)
    +            lazy val gdef: GlobalSequenceGroupDef = new GlobalSequenceGroupDef(defXML, groupXML, schemaDocument, gref)
    +            gref.groupDef
    +            gdef.groupRef
    +            (gref, gdef)
    +          }
    +          case groupXML @ <choice>{ _* }</choice> =>
    +            lazy val gref: ChoiceGroupRef = new ChoiceGroupRef(gdef, refXML, refLexicalParent, position, isHidden)
    +            lazy val gdef: GlobalChoiceGroupDef = new GlobalChoiceGroupDef(defXML, groupXML, schemaDocument, gref)
    +            gref.groupDef
    +            gdef.groupRef
    +            (gref, gdef)
             }
    -        Assert.invariant(ggd.length == 1)
    -        ggd(0)
    +        val res = list(0)
    +        res
           }
           case _ => Assert.invariantFailed("not a group")
         }
       }
    -
    -  override lazy val namedQName = QName.createGlobal(name, targetNamespace, xml.scope)
    -
     }
     
    -sealed abstract class GlobalGroupDef(xmlArg: Node, schemaDocumentArg: SchemaDocument, val groupRef: GroupRef, position: Int)
    -  extends SchemaComponent(xmlArg, schemaDocumentArg)
    -  with GlobalNonElementComponentMixin 
    -  with NestingTraversesToReferenceMixin {
    -
    -  requiredEvaluations(modelGroup)
    -  requiredEvaluations(validateChoiceBranchKey)
    -
    -  final lazy val referringComponent = {
    -    val res = Some(groupRef)
    -    res
    -  }
    +/**
    + * Many things can be contained either by a model group or a
    + * group definition (aka global sequence group definition or global choice
    + * group definition).
    + *
    + * These things are GroupLike.
    + */
    +trait GroupDefLike
    +  extends AnnotatedSchemaComponent {
     
    -  // final override protected def enclosingComponentDef = groupRef.enclosingComponent
    +  def xmlChildren: Seq[Node]
     
       //
    -  // Note: Dealing with XML can be fragile. It's easy to forget some of these children
    -  // might be annotations and Text nodes. Even if you trim the text nodes out, there are
    -  // places where annotations can be.
    -  //
    -  final lazy val <group>{ xmlChildren @ _* }</group> = xml
    -  //
       // So we have to flatMap, so that we can tolerate annotation objects (like documentation objects).
       // and our ModelGroup factory has to return Nil for annotations and Text nodes.
       //
    -  final lazy val Seq(modelGroup: ModelGroup) = xmlChildren.flatMap { ModelGroupFactory(_, this, position) }
    +  final lazy val groupMembers: Seq[Term] = LV('groupMembers) {
    +    val goodXmlChildren = LV('goodXMLChildren) { xmlChildren.flatMap { removeNonInteresting(_) } }.value
    +    val positions = List.range(1, goodXmlChildren.length + 1) // range is exclusive on 2nd arg. So +1.
    +    val pairs = goodXmlChildren zip positions
    +    pairs.flatMap {
    +      case (n, i) =>
    +        TermFactory(n, this, i)
    +    }
    +  }.value
    +
    +  /**
    +   * XML is full of uninteresting text nodes. We just want the element children, not all children.
    +   */
    +  private def removeNonInteresting(child: Node) = {
    +    val childList: List[Node] = child match {
    +      case _: Text => Nil
    +      case _: Comment => Nil
    +      case <annotation>{ _* }</annotation> => Nil
    +      case _ => List(child)
    +    }
    +    childList
    +  }
    +}
    +
    +/**
    + * Global Group Defs are not actual carriers of annotations syntactically,
    + * but the model groups they contain carry annotations that must be combined
    + * with those from the referring group ref; hence they must be AnnotatedSchemaComponents
    + *
    + * However logic in that base trait takes this behavor of group definitions
    --- End diff --
    
    What is this referring to in AnnotatedSchemaComponent? Be specific... check if the code is still in that class/trait. It may have moved.


---

[GitHub] incubator-daffodil pull request #5: Property Resolution - part of fixing sch...

Posted by mbeckerle <gi...@git.apache.org>.
Github user mbeckerle commented on a diff in the pull request:

    https://github.com/apache/incubator-daffodil/pull/5#discussion_r149847101
  
    --- Diff: daffodil-test/src/test/resources/edu/illinois/ncsa/daffodil/section07/assertions/assert.tdml ---
    @@ -639,11 +639,12 @@
     
       <tdml:parserTestCase name="assertOnGroupRef" root="Item"
         model="s1" description="assert on a group reference - DFDL-7-050R">
    -
         <tdml:document><![CDATA[Shirts,30]]></tdml:document>
         <tdml:errors>
    -      <tdml:error>Assert</tdml:error>
    -      <tdml:error>pattern</tdml:error>
    +      <tdml:error>Parse Error</tdml:error>
    +      <tdml:error>Assertion</tdml:error>
    --- End diff --
    
    Just say "Assert" , as we really don't care if the word "assertion" is used in the message vs say, dfdl:assert. 


---

[GitHub] incubator-daffodil pull request #5: Property Resolution - part of fixing sch...

Posted by mbeckerle <gi...@git.apache.org>.
Github user mbeckerle commented on a diff in the pull request:

    https://github.com/apache/incubator-daffodil/pull/5#discussion_r149836309
  
    --- Diff: daffodil-runtime1/src/main/scala/edu/illinois/ncsa/daffodil/infoset/InfosetImpl.scala ---
    @@ -213,7 +213,7 @@ case class InfosetValueLengthUnknownException(lengthState: LengthState, override
     final class FakeDINode extends DISimple(null) {
       private def die = throw new InfosetNoInfosetException(Nope)
     
    -  override def parent = die
    +  override val parent = die
    --- End diff --
    
    No idea how val parent = die works unless this val is overridden in all cases. Should be def. 


---

[GitHub] incubator-daffodil pull request #5: Property Resolution - part of fixing sch...

Posted by mbeckerle <gi...@git.apache.org>.
Github user mbeckerle commented on a diff in the pull request:

    https://github.com/apache/incubator-daffodil/pull/5#discussion_r149843503
  
    --- Diff: daffodil-core/src/main/scala/edu/illinois/ncsa/daffodil/dsom/PropProviders.scala ---
    @@ -120,6 +120,7 @@ trait LeafPropProvider
     final class ChainPropProvider(leafProvidersArg: Seq[LeafPropProvider], forAnnotation: String)
       extends Logging with PropTypes {
     
    +  override def toString() = Misc.getNameFromClass(this) + "(" + forAnnotation + ")"
       /**
        * for debug/test only
        */
    --- End diff --
    
    Consider removing things that are for debug/test only - see if there are actually tests using them.


---

[GitHub] incubator-daffodil pull request #5: Property Resolution - part of fixing sch...

Posted by mbeckerle <gi...@git.apache.org>.
Github user mbeckerle commented on a diff in the pull request:

    https://github.com/apache/incubator-daffodil/pull/5#discussion_r149837895
  
    --- Diff: daffodil-core/src/main/scala/edu/illinois/ncsa/daffodil/dsom/DFDLStatement.scala ---
    @@ -46,4 +46,5 @@ abstract class DFDLStatement(node: Node,
       requiredEvaluations(gram)
     
       def gram: Gram
    +
    --- End diff --
    
    Revert file. Just a whitespace change.


---

[GitHub] incubator-daffodil pull request #5: Property Resolution - part of fixing sch...

Posted by stevedlawrence <gi...@git.apache.org>.
Github user stevedlawrence commented on a diff in the pull request:

    https://github.com/apache/incubator-daffodil/pull/5#discussion_r151126478
  
    --- Diff: daffodil-core/src/main/scala/edu/illinois/ncsa/daffodil/dsom/LocalElementBase.scala ---
    @@ -29,50 +29,13 @@
      * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS WITH THE
      * SOFTWARE.
      */
    -
    -package edu.illinois.ncsa.daffodil.dsom
    -
    -trait LocalElementBase
    -  extends ElementBase
    -  with LocalElementMixin {
    -
    -  requiredEvaluations(checkForAlignmentAmbiguity)
    -
    -  private def gcd(a: Int, b: Int): Int = if (b == 0) a else gcd(b, a % b)
    -  //  private def lcm(a: Int, b: Int): Int = math.abs(a * b) / gcd(a, b)
    -  //  private def isXAMultipleOfY(x: Int, y: Int): Boolean = (x % y) == 0
    -
    -  //  private def isAlignmentCompatible(current: Int, next: Int): Boolean = {
    -  //    isXAMultipleOfY(current, next)
    -  //  }
    -
    -  /**
    -   * Changed to a warning - DFDL WG decided to make this check optional, but it
    -   * is still useful as a warning.
    -   *
    -   * Turns out that MIL STD 2045 header format needs to pad out to a byte boundary
    -   * at the end of the structure. An optional, non-byte aligned field precedes
    -   * the end of the structure; hence, putting a zero-length byte-aligned field
    -   * at the end was crashing into this error. I couldn't think of a work-around,
    -   * so changed this into a warning.
    -   *
    -   * The old requirement was:
    -   *   To avoid ambiguity when parsing, optional elements and variable-occurrence arrays
    -   *   where the minimum number of occurrences is zero cannot have alignment properties
    -   *   different from the items that follow them. It is a schema definition error otherwise.
    -   *
    -   * Part of the required evaluations for LocalElementBase.
    -   */
    -  final def checkForAlignmentAmbiguity: Unit = {
    -    if (isOptional) {
    -      this.possibleNextTerms.filterNot(m => m == this).foreach { that =>
    -        val isSame = this.alignmentValueInBits == that.alignmentValueInBits
    -        if (!isSame) {
    -          this.SDW("%s is an optional element or a variable-occurrence array and its alignment (%s) is not the same as %s's alignment (%s).",
    -            this.toString, this.alignmentValueInBits, that.toString, that.alignmentValueInBits)
    -        }
    -      }
    -    }
    -  }
    -
    -}
    +//
    +//package edu.illinois.ncsa.daffodil.dsom
    +//
    +//trait LocalElementBase
    +//  extends ElementBase
    +//  with LocalElementMixin {
    +//
    +//
    +//
    +//}
    --- End diff --
    
    Delete file?


---

[GitHub] incubator-daffodil pull request #5: Property Resolution - part of fixing sch...

Posted by mbeckerle <gi...@git.apache.org>.
Github user mbeckerle commented on a diff in the pull request:

    https://github.com/apache/incubator-daffodil/pull/5#discussion_r150626971
  
    --- Diff: daffodil-core/src/main/scala/edu/illinois/ncsa/daffodil/dsom/ChoiceGroup.scala ---
    @@ -78,29 +77,48 @@ import edu.illinois.ncsa.daffodil.api.WarnID
      *
      */
     
    -final class Choice(xmlArg: Node, parent: SchemaComponent, position: Int)
    -  extends ModelGroup(xmlArg, parent, position)
    -  with Choice_AnnotationMixin
    -  with RawDelimitedRuntimeValuedPropertiesMixin // initiator and terminator (not separator)
    -  with ChoiceGrammarMixin {
    +trait ChoiceLikeMixin
    +  extends AnnotatedSchemaComponent
    +  with DFDLStatementMixin {
     
    -  requiredEvaluations(branchesAreNonOptional)
    -  requiredEvaluations(branchesAreNotIVCElements)
    -  requiredEvaluations(modelGroupRuntimeData.preSerialization)
    +  protected def isMyFormatAnnotation(a: DFDLAnnotation) = a.isInstanceOf[DFDLChoice]
     
    -  protected final override lazy val myPeers = choicePeers
    -
    -  protected final override def annotationFactory(node: Node): Option[DFDLAnnotation] = {
    +  protected override def annotationFactory(node: Node): Option[DFDLAnnotation] = {
         node match {
           case <dfdl:choice>{ contents @ _* }</dfdl:choice> => Some(new DFDLChoice(node, this))
           case _ => annotationFactoryForDFDLStatement(node, this)
         }
       }
     
    -  protected final def emptyFormatFactory = new DFDLChoice(newDFDLAnnotationXML("choice"), this)
    -  protected final def isMyFormatAnnotation(a: DFDLAnnotation) = a.isInstanceOf[DFDLChoice]
    +  protected def emptyFormatFactory: DFDLFormatAnnotation = new DFDLChoice(newDFDLAnnotationXML("choice"), this)
    +
    +  lazy val xmlChildren = xml match {
    +    case <choice>{ c @ _* }</choice> => c
    +    case <group>{ _* }</group> => {
    +      val ch = this match {
    +        case cgr: ChoiceGroupRef => cgr.groupDef.xml \\ "choice"
    +        case cgd: GlobalChoiceGroupDef => cgd.xml \\ "choice"
    +      }
    +      val <choice>{ c @ _* }</choice> = ch(0)
    +      c
    +    }
    +  }
    +}
    +
    +abstract class ChoiceBase( final override val xml: Node,
    +  final override val parent: SchemaComponent,
    +  final override val position: Int)
    +  extends ModelGroup
    +  with ChoiceLikeMixin
    --- End diff --
    
    Should not be mixed into ChoiceBase. Will change code. 


---

[GitHub] incubator-daffodil pull request #5: Property Resolution - part of fixing sch...

Posted by stevedlawrence <gi...@git.apache.org>.
Github user stevedlawrence commented on a diff in the pull request:

    https://github.com/apache/incubator-daffodil/pull/5#discussion_r150593735
  
    --- Diff: daffodil-core/src/main/scala/edu/illinois/ncsa/daffodil/dsom/ChoiceGroup.scala ---
    @@ -78,29 +77,48 @@ import edu.illinois.ncsa.daffodil.api.WarnID
      *
      */
     
    -final class Choice(xmlArg: Node, parent: SchemaComponent, position: Int)
    -  extends ModelGroup(xmlArg, parent, position)
    -  with Choice_AnnotationMixin
    -  with RawDelimitedRuntimeValuedPropertiesMixin // initiator and terminator (not separator)
    -  with ChoiceGrammarMixin {
    +trait ChoiceLikeMixin
    +  extends AnnotatedSchemaComponent
    +  with DFDLStatementMixin {
     
    -  requiredEvaluations(branchesAreNonOptional)
    -  requiredEvaluations(branchesAreNotIVCElements)
    -  requiredEvaluations(modelGroupRuntimeData.preSerialization)
    +  protected def isMyFormatAnnotation(a: DFDLAnnotation) = a.isInstanceOf[DFDLChoice]
     
    -  protected final override lazy val myPeers = choicePeers
    -
    -  protected final override def annotationFactory(node: Node): Option[DFDLAnnotation] = {
    +  protected override def annotationFactory(node: Node): Option[DFDLAnnotation] = {
         node match {
           case <dfdl:choice>{ contents @ _* }</dfdl:choice> => Some(new DFDLChoice(node, this))
           case _ => annotationFactoryForDFDLStatement(node, this)
         }
       }
     
    -  protected final def emptyFormatFactory = new DFDLChoice(newDFDLAnnotationXML("choice"), this)
    -  protected final def isMyFormatAnnotation(a: DFDLAnnotation) = a.isInstanceOf[DFDLChoice]
    +  protected def emptyFormatFactory: DFDLFormatAnnotation = new DFDLChoice(newDFDLAnnotationXML("choice"), this)
    +
    +  lazy val xmlChildren = xml match {
    +    case <choice>{ c @ _* }</choice> => c
    +    case <group>{ _* }</group> => {
    +      val ch = this match {
    +        case cgr: ChoiceGroupRef => cgr.groupDef.xml \\ "choice"
    +        case cgd: GlobalChoiceGroupDef => cgd.xml \\ "choice"
    --- End diff --
    
    Should these XML path steps use "\" instead if "\\"? Also, ChoiceGroupRef overrides xmlChildren, so the first branch of this match/case will never get hit.


---

[GitHub] incubator-daffodil pull request #5: Property Resolution - part of fixing sch...

Posted by mbeckerle <gi...@git.apache.org>.
Github user mbeckerle commented on a diff in the pull request:

    https://github.com/apache/incubator-daffodil/pull/5#discussion_r149849367
  
    --- Diff: daffodil-core/src/main/scala/edu/illinois/ncsa/daffodil/dsom/LocalElementDecl.scala ---
    @@ -34,27 +34,24 @@ package edu.illinois.ncsa.daffodil.dsom
     
     import scala.xml.Node
     
    -final class LocalElementDeclFactory(override val xml: Node, sd: SchemaDocument)
    -  extends SchemaComponentFactory(xml, sd)
    +final class LocalElementDeclFactory(xmlArg: Node, sd: SchemaDocument)
    --- End diff --
    
    Is this factory really needed? I think it is just confusing.


---

[GitHub] incubator-daffodil pull request #5: Property Resolution - part of fixing sch...

Posted by stevedlawrence <gi...@git.apache.org>.
Github user stevedlawrence commented on a diff in the pull request:

    https://github.com/apache/incubator-daffodil/pull/5#discussion_r151125229
  
    --- Diff: daffodil-core/src/main/scala/edu/illinois/ncsa/daffodil/dsom/DFDLStatementMixin.scala ---
    @@ -89,36 +156,34 @@ trait DFDLStatementMixin extends ThrowsSDE { self: AnnotatedSchemaComponent =>
        * reference.
        */
     
    -  final protected def optReferencedStatementSource: Option[DFDLStatementMixin] =
    -    self.optReferredToComponent.asInstanceOf[Option[DFDLStatementMixin]]
    +  final def optReferencedStatementSource: Option[ProvidesDFDLStatementMixin] =
    +    self.optReferredToComponent.asInstanceOf[Option[ProvidesDFDLStatementMixin]]
     
    -  final lazy val statements: Seq[DFDLStatement] =
    -    optReferencedStatementSource.toSeq.flatMap { _.statements } ++ localStatements
    +  final lazy val resolvedStatements: Seq[DFDLStatement] =
    +    optReferencedStatementSource.toSeq.flatMap { _.resolvedStatements } ++ localStatements
     
       final lazy val newVariableInstanceStatements: Seq[DFDLNewVariableInstance] =
         optReferencedStatementSource.toSeq.flatMap { _.newVariableInstanceStatements } ++
           localNewVariableInstanceStatements
     
    -  final lazy val notNewVariableInstanceStatements = setVariableStatements ++ discriminatorStatements ++ assertStatements
    -
       final lazy val (discriminatorStatements, assertStatements) =
         checkDiscriminatorsAssertsDisjoint(combinedDiscrims, combinedAsserts)
     
       private lazy val combinedAsserts: Seq[DFDLAssert] =
    -    optReferencedStatementSource.toSeq.flatMap { _.assertStatements } ++ localAssertStatements
    +    optReferencedStatementSource.toSeq.flatMap { _.assertStatements } ++ localassertStatements
     
       private lazy val combinedDiscrims: Seq[DFDLDiscriminator] =
    -    optReferencedStatementSource.toSeq.flatMap { _.discriminatorStatements } ++ localDiscriminatorStatements
    +    optReferencedStatementSource.toSeq.flatMap { _.discriminatorStatements } ++ localdiscriminatorStatements
    --- End diff --
    
    Did you mean to change the name of this? The camelcase before seems more readable to me.


---

[GitHub] incubator-daffodil pull request #5: Property Resolution - part of fixing sch...

Posted by mbeckerle <gi...@git.apache.org>.
Github user mbeckerle commented on a diff in the pull request:

    https://github.com/apache/incubator-daffodil/pull/5#discussion_r150872917
  
    --- Diff: daffodil-core/src/main/scala/edu/illinois/ncsa/daffodil/dsom/GroupDef.scala ---
    @@ -34,70 +34,137 @@ package edu.illinois.ncsa.daffodil.dsom
     
     import scala.xml.Node
     import edu.illinois.ncsa.daffodil.exceptions.Assert
    -import edu.illinois.ncsa.daffodil.xml.QName
    +import scala.xml.Text
    +import scala.xml.Comment
     
    -class GlobalGroupDefFactory(xmlArg: Node, schemaDocumentArg: SchemaDocument)
    -  extends SchemaComponentFactory(xmlArg, schemaDocumentArg)
    +final class GlobalGroupDefFactory(defXML: Node, schemaDocument: SchemaDocument)
    +  extends SchemaComponentFactory(defXML, schemaDocument)
       with GlobalNonElementComponentMixin {
     
    -  private lazy val trimmedXml = scala.xml.Utility.trim(xmlArg)
    -
    -  def forGroupRef(gref: GroupRef, position: Int) = {
    +  def forGroupRef(refXML: Node, refLexicalParent: SchemaComponent, position: Int,
    +    isHidden: Boolean): (GroupRef, GlobalGroupDef) = {
    +    val trimmedXml = scala.xml.Utility.trim(defXML)
         trimmedXml match {
           case <group>{ contents @ _* }</group> => {
    -        val ggd = contents.collect {
    -          case <sequence>{ _* }</sequence> => new GlobalSequenceGroupDef(xml, schemaDocument, gref, position)
    -          case <choice>{ _* }</choice> => new GlobalChoiceGroupDef(xml, schemaDocument, gref, position)
    +        val list = contents.collect {
    +          case groupXML @ <sequence>{ _* }</sequence> => {
    +            lazy val gref: SequenceGroupRef = new SequenceGroupRef(gdef, refXML, refLexicalParent, position, isHidden)
    +            lazy val gdef: GlobalSequenceGroupDef = new GlobalSequenceGroupDef(defXML, groupXML, schemaDocument, gref)
    +            gref.groupDef
    +            gdef.groupRef
    +            (gref, gdef)
    +          }
    +          case groupXML @ <choice>{ _* }</choice> =>
    +            lazy val gref: ChoiceGroupRef = new ChoiceGroupRef(gdef, refXML, refLexicalParent, position, isHidden)
    +            lazy val gdef: GlobalChoiceGroupDef = new GlobalChoiceGroupDef(defXML, groupXML, schemaDocument, gref)
    +            gref.groupDef
    +            gdef.groupRef
    +            (gref, gdef)
             }
    -        Assert.invariant(ggd.length == 1)
    -        ggd(0)
    +        val res = list(0)
    +        res
           }
           case _ => Assert.invariantFailed("not a group")
         }
       }
    -
    -  override lazy val namedQName = QName.createGlobal(name, targetNamespace, xml.scope)
    -
     }
     
    -sealed abstract class GlobalGroupDef(xmlArg: Node, schemaDocumentArg: SchemaDocument, val groupRef: GroupRef, position: Int)
    -  extends SchemaComponent(xmlArg, schemaDocumentArg)
    -  with GlobalNonElementComponentMixin 
    -  with NestingTraversesToReferenceMixin {
    -
    -  requiredEvaluations(modelGroup)
    -  requiredEvaluations(validateChoiceBranchKey)
    -
    -  final lazy val referringComponent = {
    -    val res = Some(groupRef)
    -    res
    -  }
    +/**
    + * Many things can be contained either by a model group or a
    + * group definition (aka global sequence group definition or global choice
    + * group definition).
    + *
    + * These things are GroupLike.
    + */
    +trait GroupDefLike
    +  extends AnnotatedSchemaComponent {
     
    -  // final override protected def enclosingComponentDef = groupRef.enclosingComponent
    +  def xmlChildren: Seq[Node]
     
       //
    -  // Note: Dealing with XML can be fragile. It's easy to forget some of these children
    -  // might be annotations and Text nodes. Even if you trim the text nodes out, there are
    -  // places where annotations can be.
    -  //
    -  final lazy val <group>{ xmlChildren @ _* }</group> = xml
    -  //
       // So we have to flatMap, so that we can tolerate annotation objects (like documentation objects).
       // and our ModelGroup factory has to return Nil for annotations and Text nodes.
       //
    -  final lazy val Seq(modelGroup: ModelGroup) = xmlChildren.flatMap { ModelGroupFactory(_, this, position) }
    +  final lazy val groupMembers: Seq[Term] = LV('groupMembers) {
    +    val goodXmlChildren = LV('goodXMLChildren) { xmlChildren.flatMap { removeNonInteresting(_) } }.value
    +    val positions = List.range(1, goodXmlChildren.length + 1) // range is exclusive on 2nd arg. So +1.
    +    val pairs = goodXmlChildren zip positions
    +    pairs.flatMap {
    +      case (n, i) =>
    +        TermFactory(n, this, i)
    +    }
    +  }.value
    +
    +  /**
    +   * XML is full of uninteresting text nodes. We just want the element children, not all children.
    +   */
    +  private def removeNonInteresting(child: Node) = {
    +    val childList: List[Node] = child match {
    +      case _: Text => Nil
    +      case _: Comment => Nil
    +      case <annotation>{ _* }</annotation> => Nil
    +      case _ => List(child)
    +    }
    +    childList
    +  }
    --- End diff --
    
    I agree that we should profile it. 
    
    The alternative coding that wouldn't copy has to cope with the variety of XML nodes subsequently where children are processed. The copying here may be worth it to reduce the complexity. Keeping in mind everywhere one traverses XML  children that comments, whitespace text nodes, PIs, etc. can appear is very error prone. Basically you can't use XML pattern matching, because you'll get all these additional nodes. You'd have to use an XPath expression (scala formulation thereof) to pull the nodes you want, only.
    
    One thing we could do is walk the whole schema exactly once removing all these irrelevant nodes. I just think that would make debugging very tedious, because the XML would all be giant single lines.   


---

[GitHub] incubator-daffodil pull request #5: Property Resolution - part of fixing sch...

Posted by mbeckerle <gi...@git.apache.org>.
Github user mbeckerle commented on a diff in the pull request:

    https://github.com/apache/incubator-daffodil/pull/5#discussion_r149842210
  
    --- Diff: daffodil-core/src/main/scala/edu/illinois/ncsa/daffodil/dsom/GroupDef.scala ---
    @@ -34,70 +34,137 @@ package edu.illinois.ncsa.daffodil.dsom
     
     import scala.xml.Node
     import edu.illinois.ncsa.daffodil.exceptions.Assert
    -import edu.illinois.ncsa.daffodil.xml.QName
    +import scala.xml.Text
    +import scala.xml.Comment
     
    -class GlobalGroupDefFactory(xmlArg: Node, schemaDocumentArg: SchemaDocument)
    -  extends SchemaComponentFactory(xmlArg, schemaDocumentArg)
    +final class GlobalGroupDefFactory(defXML: Node, schemaDocument: SchemaDocument)
    +  extends SchemaComponentFactory(defXML, schemaDocument)
       with GlobalNonElementComponentMixin {
     
    -  private lazy val trimmedXml = scala.xml.Utility.trim(xmlArg)
    -
    -  def forGroupRef(gref: GroupRef, position: Int) = {
    +  def forGroupRef(refXML: Node, refLexicalParent: SchemaComponent, position: Int,
    +    isHidden: Boolean): (GroupRef, GlobalGroupDef) = {
    +    val trimmedXml = scala.xml.Utility.trim(defXML)
         trimmedXml match {
           case <group>{ contents @ _* }</group> => {
    -        val ggd = contents.collect {
    -          case <sequence>{ _* }</sequence> => new GlobalSequenceGroupDef(xml, schemaDocument, gref, position)
    -          case <choice>{ _* }</choice> => new GlobalChoiceGroupDef(xml, schemaDocument, gref, position)
    +        val list = contents.collect {
    +          case groupXML @ <sequence>{ _* }</sequence> => {
    +            lazy val gref: SequenceGroupRef = new SequenceGroupRef(gdef, refXML, refLexicalParent, position, isHidden)
    +            lazy val gdef: GlobalSequenceGroupDef = new GlobalSequenceGroupDef(defXML, groupXML, schemaDocument, gref)
    +            gref.groupDef
    +            gdef.groupRef
    +            (gref, gdef)
    +          }
    +          case groupXML @ <choice>{ _* }</choice> =>
    +            lazy val gref: ChoiceGroupRef = new ChoiceGroupRef(gdef, refXML, refLexicalParent, position, isHidden)
    +            lazy val gdef: GlobalChoiceGroupDef = new GlobalChoiceGroupDef(defXML, groupXML, schemaDocument, gref)
    +            gref.groupDef
    +            gdef.groupRef
    +            (gref, gdef)
             }
    -        Assert.invariant(ggd.length == 1)
    -        ggd(0)
    +        val res = list(0)
    +        res
           }
           case _ => Assert.invariantFailed("not a group")
         }
       }
    -
    -  override lazy val namedQName = QName.createGlobal(name, targetNamespace, xml.scope)
    -
     }
     
    -sealed abstract class GlobalGroupDef(xmlArg: Node, schemaDocumentArg: SchemaDocument, val groupRef: GroupRef, position: Int)
    -  extends SchemaComponent(xmlArg, schemaDocumentArg)
    -  with GlobalNonElementComponentMixin 
    -  with NestingTraversesToReferenceMixin {
    -
    -  requiredEvaluations(modelGroup)
    -  requiredEvaluations(validateChoiceBranchKey)
    -
    -  final lazy val referringComponent = {
    -    val res = Some(groupRef)
    -    res
    -  }
    +/**
    + * Many things can be contained either by a model group or a
    + * group definition (aka global sequence group definition or global choice
    + * group definition).
    + *
    + * These things are GroupLike.
    + */
    +trait GroupDefLike
    +  extends AnnotatedSchemaComponent {
     
    -  // final override protected def enclosingComponentDef = groupRef.enclosingComponent
    +  def xmlChildren: Seq[Node]
     
       //
    -  // Note: Dealing with XML can be fragile. It's easy to forget some of these children
    -  // might be annotations and Text nodes. Even if you trim the text nodes out, there are
    -  // places where annotations can be.
    -  //
    -  final lazy val <group>{ xmlChildren @ _* }</group> = xml
    -  //
       // So we have to flatMap, so that we can tolerate annotation objects (like documentation objects).
       // and our ModelGroup factory has to return Nil for annotations and Text nodes.
       //
    -  final lazy val Seq(modelGroup: ModelGroup) = xmlChildren.flatMap { ModelGroupFactory(_, this, position) }
    +  final lazy val groupMembers: Seq[Term] = LV('groupMembers) {
    +    val goodXmlChildren = LV('goodXMLChildren) { xmlChildren.flatMap { removeNonInteresting(_) } }.value
    +    val positions = List.range(1, goodXmlChildren.length + 1) // range is exclusive on 2nd arg. So +1.
    +    val pairs = goodXmlChildren zip positions
    +    pairs.flatMap {
    +      case (n, i) =>
    +        TermFactory(n, this, i)
    +    }
    +  }.value
    +
    +  /**
    +   * XML is full of uninteresting text nodes. We just want the element children, not all children.
    +   */
    +  private def removeNonInteresting(child: Node) = {
    +    val childList: List[Node] = child match {
    +      case _: Text => Nil
    +      case _: Comment => Nil
    +      case <annotation>{ _* }</annotation> => Nil
    +      case _ => List(child)
    +    }
    +    childList
    +  }
    +}
    +
    +/**
    + * Global Group Defs are not actual carriers of annotations syntactically,
    + * but the model groups they contain carry annotations that must be combined
    + * with those from the referring group ref; hence they must be AnnotatedSchemaComponents
    + *
    + * However logic in that base trait takes this behavor of group definitions
    + * into account.
    + */
    +sealed abstract class GlobalGroupDef(
    +  defXML: Node,
    +  groupXML: Node,
    +  schemaDocumentArg: SchemaDocument,
    +  grefArg: => GroupRef)
    +  extends AnnotatedSchemaComponentImpl(groupXML, schemaDocumentArg)
    +  with GroupDefLike
    +  with GlobalNonElementComponentMixin
    +  with NestingTraversesToReferenceMixin
    +  with DFDLStatementMixin {
    +
    +  requiredEvaluations(validateChoiceBranchKey)
     
       def validateChoiceBranchKey(): Unit = {
         // Ensure the model group of a global group def do not define choiceBranchKey.
    -    val found = modelGroup.findPropertyOptionThisComponentOnly("choiceBranchKey")
    +    val found = findPropertyOptionThisComponentOnly("choiceBranchKey")
         if (found.isDefined) {
           SDE("dfdl:choiceBranchKey cannot be specified on the choice/sequence child of a global group definition")
         }
       }
    +
    +  final override lazy val name = defXML.attribute("name").map { _.text }.getOrElse(
    +    Assert.invariantFailed("Global group def without name attribute."))
    +
    +  lazy val groupRef =
    +    grefArg
    +
    +  //  // delegate to the annotated component.
    --- End diff --
    
    delete commented code here and below


---

[GitHub] incubator-daffodil pull request #5: Property Resolution - part of fixing sch...

Posted by stevedlawrence <gi...@git.apache.org>.
Github user stevedlawrence commented on a diff in the pull request:

    https://github.com/apache/incubator-daffodil/pull/5#discussion_r150862972
  
    --- Diff: daffodil-core/src/main/scala/edu/illinois/ncsa/daffodil/dsom/SchemaComponentFactory.scala ---
    @@ -33,29 +33,40 @@
     package edu.illinois.ncsa.daffodil.dsom
     
     import edu.illinois.ncsa.daffodil.exceptions.SchemaFileLocatable
    -import edu.illinois.ncsa.daffodil.xml.GetAttributesMixin
     import edu.illinois.ncsa.daffodil.xml.XMLUtils
     import edu.illinois.ncsa.daffodil.exceptions.ThrowsSDE
     import scala.xml.NamespaceBinding
     import edu.illinois.ncsa.daffodil.xml.NS
     import edu.illinois.ncsa.daffodil.oolag.OOLAG.OOLAGHost
    +import scala.xml.Node
     
     /**
      * Anything that can be computed without reference to the point of use
      * or point of reference can be computed here on these factory objects.
      */
    -class SchemaComponentFactory(override val xml: scala.xml.Node,
    -  override val schemaDocument: SchemaDocument)
    -  extends OOLAGHost(schemaDocument)
    -  with SchemaFileLocatableImpl
    -  with CommonContextMixin
    -  with GetAttributesMixin
    -  with ImplementsThrowsSDE
    +abstract class SchemaComponentFactory( final override val xml: Node,
    +  final override val schemaDocument: SchemaDocument)
    +  extends SchemaComponent
       with NestingLexicalMixin {
    +  //  extends OOLAGHost(schemaDocument)
    +  //  with SchemaFileLocatableImpl
    +  //  with CommonContextMixin
    +  //  with GetAttributesMixin
    +  //  with ImplementsThrowsSDE
    +  //  with NestingLexicalMixin {
    +
    --- End diff --
    
    Remove comments


---