Move HED 3 requireChild testing to TagConverter and refactor that class

hed-standard · Oct 7, 2024 · 9d82533 · 9d82533
1 parent cea6719
commit 9d82533
Show file tree

Hide file tree

Showing 4 changed files with 69 additions and 47 deletions.
diff --git a/parser/converter.js b/parser/converter.js
@@ -1,9 +1,9 @@
-import { generateIssue, IssueError } from '../common/issues/issues'
+import { IssueError } from '../common/issues/issues'
 import { getTagSlashIndices } from '../utils/hedStrings'
 import { SchemaValueTag } from '../validator/schema/types'
 
 /**
- * Converter from a tas specification to a schema-based tag object.
+ * Converter from a tag specification to a schema-based tag object.
  */
 export default class TagConverter {
   /**
@@ -68,30 +68,8 @@ export default class TagConverter {
    * @returns {[SchemaTag, string]} The schema's corresponding tag object and the remainder of the tag string.
    */
   convert() {
-    const firstLevel = this._checkFirstLevel()
-    if (firstLevel) {
-      return [firstLevel, '']
-    }
-
-    return this._checkLowerLevels()
-  }
-
-  _checkFirstLevel() {
-    const firstLevel = this.tagLevels[0].toLowerCase().trimStart()
-    const schemaTag = this.tagMapping.getEntry(firstLevel)
-    if (!schemaTag || firstLevel === '' || firstLevel !== firstLevel.trim()) {
-      IssueError.generateAndThrow('invalidTag', { tag: this.tagString })
-    }
-    if (this.tagLevels.length === 1) {
-      return schemaTag
-    } else {
-      return undefined
-    }
-  }
-
-  _checkLowerLevels() {
-    let parentTag = this._getSchemaTag(0)
-    for (let i = 1; i < this.tagLevels.length; i++) {
+    let parentTag = undefined
+    for (let i = 0; i < this.tagLevels.length; i++) {
       if (parentTag?.valueTag) {
         this._setSchemaTag(parentTag.valueTag, i)
         break
@@ -113,13 +91,16 @@ export default class TagConverter {
         message: 'Child tag is a value tag which should have been handled earlier.',
       })
     }
+    if (childTag === undefined && i === 0) {
+      IssueError.generateAndThrow('invalidTag', { tag: this.tagString })
+    }
     if (childTag === undefined && parentTag && !parentTag.hasAttributeName('extensionAllowed')) {
       IssueError.generateAndThrow('invalidExtension', {
         tag: this.tagLevels[i],
         parentTag: parentTag.longName,
       })
     }
-    if (childTag !== undefined && (childTag.parent === undefined || childTag.parent !== parentTag)) {
+    if (childTag !== undefined && parentTag && (childTag.parent === undefined || childTag.parent !== parentTag)) {
       IssueError.generateAndThrow('invalidParentNode', {
         tag: this.tagLevels[i],
         parentTag: childTag.longName,
@@ -128,18 +109,25 @@ export default class TagConverter {
     return childTag
   }
 
-  _getSchemaTag(i) {
-    const tagLevel = this.tagLevels[i].toLowerCase()
+  _getSchemaTag(i, trimLeft = false) {
+    let tagLevel = this.tagLevels[i].toLowerCase()
+    if (trimLeft) {
+      tagLevel = tagLevel.trimLeft()
+    }
     if (tagLevel === '' || tagLevel !== tagLevel.trim()) {
       IssueError.generateAndThrow('invalidTag', { tag: this.tagString })
     }
     return this.tagMapping.getEntry(tagLevel)
   }
 
   _setSchemaTag(schemaTag, i) {
-    if (this.schemaTag === undefined) {
-      this.schemaTag = schemaTag
-      this.remainder = this.tagLevels.slice(i).join('/')
+    if (this.schemaTag !== undefined) {
+      return
+    }
+    this.schemaTag = schemaTag
+    this.remainder = this.tagLevels.slice(i).join('/')
+    if (this.schemaTag?.hasAttributeName('requireChild') && !this.remainder) {
+      IssueError.generateAndThrow('childRequired', { tag: this.tagString })
     }
   }
 }
diff --git a/tests/event.spec.js b/tests/event.spec.js
@@ -1235,6 +1235,32 @@ describe('HED string and event validation', () => {
           validator.checkForMissingDefinitions(tag, 'Def-expand')
         })
       })
+
+      it('should have a child when required', () => {
+        const testStrings = {
+          noRequiredChild: 'Red',
+          hasRequiredChild: 'Label/Blah',
+          missingChild: 'Label',
+          longMissingChild: 'Property/Informational-property/Label',
+        }
+        const expectedIssues = {
+          noRequiredChild: [],
+          hasRequiredChild: [],
+          missingChild: [
+            generateIssue('childRequired', {
+              tag: testStrings.missingChild,
+            }),
+          ],
+          longMissingChild: [
+            generateIssue('childRequired', {
+              tag: testStrings.longMissingChild,
+            }),
+          ],
+        }
+        return validatorSemantic(testStrings, expectedIssues, (validator) => {
+          validator.validateEventLevel()
+        })
+      })
     })
 
     describe('HED Tag Groups', () => {

diff --git a/validator/event/validator.js b/validator/event/validator.js
@@ -5,7 +5,6 @@ import { Schemas } from '../../common/schema/types'
 const NAME_CLASS_REGEX = /^[\w\-\u0080-\uFFFF]+$/
 const uniqueType = 'unique'
 const requiredType = 'required'
-const requireChildType = 'requireChild'
 const specialTags = require('./specialTags.json')
 
 // Validation tests
@@ -84,7 +83,6 @@ export class HedValidator {
     if (this.hedSchemas.generation > 0) {
       this.checkIfTagIsValid(tag, previousTag)
       this.checkIfTagUnitClassUnitsAreValid(tag)
-      this.checkIfTagRequiresChild(tag)
       if (!this.options.isEventLevel) {
         this.checkValueTagSyntax(tag)
       }
@@ -209,19 +207,6 @@ export class HedValidator {
   // eslint-disable-next-line no-unused-vars
   _checkForTagAttribute(attribute, fn) {}
 
-  /**
-   * Check if a tag is missing a required child.
-   *
-   * @param {ParsedHedTag} tag The HED tag to be checked.
-   */
-  checkIfTagRequiresChild(tag) {
-    const invalid = tag.hasAttribute(requireChildType)
-    if (invalid) {
-      // If this tag has the "requireChild" attribute, then by virtue of even being in the dataset it is missing a required child.
-      this.pushIssue('childRequired', { tag: tag })
-    }
-  }
-
   /**
    * Check that the unit is valid for the tag's unit class.
    *

diff --git a/validator/hed2/event/hed2Validator.js b/validator/hed2/event/hed2Validator.js
@@ -6,6 +6,8 @@ const clockTimeUnitClass = 'clockTime'
 const dateTimeUnitClass = 'dateTime'
 const timeUnitClass = 'time'
 
+const requireChildType = 'requireChild'
+
 /**
  * Hed2Validator class
  */
@@ -14,6 +16,14 @@ export class Hed2Validator extends HedValidator {
     super(parsedString, hedSchemas, options)
   }
 
+  /**
+   * Validate an individual HED tag.
+   */
+  validateIndividualHedTag(tag, previousTag) {
+    super.validateIndividualHedTag(tag, previousTag)
+    this.checkIfTagRequiresChild(tag)
+  }
+
   _checkForTagAttribute(attribute, fn) {
     const tags = this.hedSchemas.baseSchema.attributes.tagAttributes[attribute]
     for (const tag of Object.keys(tags)) {
@@ -119,4 +129,17 @@ export class Hed2Validator extends HedValidator {
     const hed2ValidValueCharacters = /^[-a-zA-Z0-9.$%^+_; :]+$/
     return hed2ValidValueCharacters.test(value)
   }
+
+  /**
+   * Check if a tag is missing a required child.
+   *
+   * @param {ParsedHed2Tag} tag The HED tag to be checked.
+   */
+  checkIfTagRequiresChild(tag) {
+    const invalid = tag.hasAttribute(requireChildType)
+    if (invalid) {
+      // If this tag has the "requireChild" attribute, then by virtue of even being in the dataset it is missing a required child.
+      this.pushIssue('childRequired', { tag: tag })
+    }
+  }
 }