Throw errors from ParsedHedTag constructors instead of using field

Also fix testcases to remove now-deleted duplicate messages and bad outputs.
hed-standard · Oct 4, 2024 · 7bd7b3e · 7bd7b3e
1 parent 670d7bf
commit 7bd7b3e
Show file tree

Hide file tree

Showing 7 changed files with 45 additions and 54 deletions.
diff --git a/parser/parsedHedTag.js b/parser/parsedHedTag.js
@@ -1,4 +1,4 @@
-import { generateIssue } from '../common/issues/issues'
+import { generateIssue, IssueError } from '../common/issues/issues'
 import { Schema } from '../common/schema/types'
 import { getTagLevels, replaceTagNameWithPound } from '../utils/hedStrings'
 import ParsedHedSubstring from './parsedHedSubstring'
@@ -19,11 +19,6 @@ export class ParsedHedTag extends ParsedHedSubstring {
    * @type {string}
    */
   canonicalTag
-  /**
-   * Any issues encountered during tag conversion.
-   * @type {Issue[]}
-   */
-  conversionIssues
   /**
    * The HED schema this tag belongs to.
    * @type {Schema}
@@ -35,12 +30,12 @@ export class ParsedHedTag extends ParsedHedSubstring {
    *
    * @param {string} originalTag The original HED tag.
    * @param {number[]} originalBounds The bounds of the HED tag in the original HED string.
+   * @throws {IssueError} If tag conversion or parsing fails.
    */
   constructor(originalTag, originalBounds) {
     super(originalTag, originalBounds)
 
     this.canonicalTag = this.originalTag
-    this.conversionIssues = []
 
     this.formattedTag = this._formatTag()
   }
@@ -301,6 +296,7 @@ export class ParsedHed3Tag extends ParsedHedTag {
    * @param {TagSpec} tagSpec The token for this tag.
    * @param {Schemas} hedSchemas The collection of HED schemas.
    * @param {string} hedString The original HED string.
+   * @throws {IssueError} If tag conversion or parsing fails.
    */
   constructor(tagSpec, hedSchemas, hedString) {
     super(tagSpec.tag, tagSpec.bounds)
@@ -316,6 +312,7 @@ export class ParsedHed3Tag extends ParsedHedTag {
    * @param {Schemas} hedSchemas The collection of HED schemas.
    * @param {string} hedString The original HED string.
    * @param {TagSpec} tagSpec The token for this tag.
+   * @throws {IssueError} If tag conversion or parsing fails.
    */
   _convertTag(hedSchemas, hedString, tagSpec) {
     const hed3ValidCharacters = /^[^{}[\]()~,\0\t]+$/
@@ -326,33 +323,27 @@ export class ParsedHed3Tag extends ParsedHedTag {
     const schemaName = tagSpec.library
     this.schema = hedSchemas.getSchema(schemaName)
     if (this.schema === undefined) {
+      this.canonicalTag = this.originalTag
       if (schemaName !== '') {
-        this.conversionIssues = [
+        throw new IssueError(
           generateIssue('unmatchedLibrarySchema', {
             tag: this.originalTag,
             library: schemaName,
           }),
-        ]
+        )
       } else {
-        this.conversionIssues = [
+        throw new IssueError(
           generateIssue('unmatchedBaseSchema', {
             tag: this.originalTag,
           }),
-        ]
+        )
       }
-      this.canonicalTag = this.originalTag
-      return
     }
 
-    try {
-      const [schemaTag, remainder] = new TagConverter(tagSpec, hedSchemas).convert()
-      this._schemaTag = schemaTag
-      this._remainder = remainder
-      this.canonicalTag = this._schemaTag.longExtend(remainder)
-      this.conversionIssues = []
-    } catch (error) {
-      this.conversionIssues = [error.issue]
-    }
+    const [schemaTag, remainder] = new TagConverter(tagSpec, hedSchemas).convert()
+    this._schemaTag = schemaTag
+    this._remainder = remainder
+    this.canonicalTag = this._schemaTag.longExtend(remainder)
   }
 
   /**

diff --git a/parser/splitHedString.js b/parser/splitHedString.js
@@ -6,6 +6,7 @@ import { recursiveMap } from '../utils/array'
 import { mergeParsingIssues } from '../utils/hedData'
 import { ParsedHed2Tag } from '../validator/hed2/parser/parsedHed2Tag'
 import { HedStringTokenizer, ColumnSpliceSpec, TagSpec } from './tokenizer'
+import { generateIssue, IssueError } from '../common/issues/issues'
 
 const generationToClass = [
   (originalTag, hedString, originalBounds, hedSchemas, schemaName, tagSpec) =>
@@ -34,16 +35,24 @@ const createParsedTags = function (hedString, hedSchemas, tagSpecs, groupSpecs)
 
   const createParsedTag = (tagSpec) => {
     if (tagSpec instanceof TagSpec) {
-      const parsedTag = ParsedHedTagConstructor(
-        tagSpec.tag,
-        hedString,
-        tagSpec.bounds,
-        hedSchemas,
-        tagSpec.library,
-        tagSpec,
-      )
-      conversionIssues.push(...parsedTag.conversionIssues)
-      return parsedTag
+      try {
+        const parsedTag = ParsedHedTagConstructor(
+          tagSpec.tag,
+          hedString,
+          tagSpec.bounds,
+          hedSchemas,
+          tagSpec.library,
+          tagSpec,
+        )
+        return parsedTag
+      } catch (issueError) {
+        if (issueError instanceof IssueError) {
+          conversionIssues.push(issueError.issue)
+        } else if (issueError instanceof Error) {
+          conversionIssues.push(generateIssue('internalError', { message: issueError.message }))
+        }
+        return null
+      }
     } else if (tagSpec instanceof ColumnSpliceSpec) {
       return new ParsedHedColumnSplice(tagSpec.columnName, tagSpec.bounds)
     }
@@ -58,7 +67,7 @@ const createParsedTags = function (hedString, hedSchemas, tagSpecs, groupSpecs)
           new ParsedHedGroup(createParsedGroups(tag, groupSpec.children), hedSchemas, hedString, groupSpec.bounds),
         )
         index++
-      } else {
+      } else if (tag !== null) {
         tagGroups.push(tag)
       }
     }

diff --git a/tests/bids.spec.js b/tests/bids.spec.js
@@ -155,7 +155,6 @@ describe('BIDS datasets', () => {
         tag: 'Speed/300 miles',
         unitClassUnits: legalSpeedUnits.sort().join(','),
       })
-      const converterMaglevError = generateIssue('invalidTag', { tag: 'Maglev' })
       const maglevError = generateIssue('invalidTag', { tag: 'Maglev' })
       const maglevWarning = generateIssue('extension', { tag: 'Train/Maglev' })
       const expectedIssues = {
@@ -164,7 +163,6 @@ describe('BIDS datasets', () => {
           BidsHedIssue.fromHedIssue(cloneDeep(speedIssue), badDatasets[0].file, { tsvLine: 2 }),
           BidsHedIssue.fromHedIssue(cloneDeep(maglevWarning), badDatasets[1].file, { tsvLine: 2 }),
           BidsHedIssue.fromHedIssue(cloneDeep(speedIssue), badDatasets[2].file, { tsvLine: 3 }),
-          BidsHedIssue.fromHedIssue(converterMaglevError, badDatasets[3].file, { tsvLine: 2 }),
           BidsHedIssue.fromHedIssue(cloneDeep(maglevError), badDatasets[3].file, { tsvLine: 2 }),
           BidsHedIssue.fromHedIssue(cloneDeep(speedIssue), badDatasets[3].file, { tsvLine: 3 }),
           BidsHedIssue.fromHedIssue(cloneDeep(maglevWarning), badDatasets[4].file, { tsvLine: 2 }),

diff --git a/tests/dataset.spec.js b/tests/dataset.spec.js
@@ -61,10 +61,6 @@ describe('HED dataset validation', () => {
             tag: testDatasets.multipleInvalid[0],
             unitClassUnits: legalTimeUnits.sort().join(','),
           }),
-          // TODO: Duplication temporary
-          generateValidationIssue('invalidTag', {
-            tag: testDatasets.multipleInvalid[1],
-          }),
           generateValidationIssue('invalidTag', { tag: testDatasets.multipleInvalid[1] }),
         ],
       }
@@ -114,10 +110,6 @@ describe('HED dataset validation', () => {
             tag: testDatasets.multipleInvalid[0],
             unitClassUnits: legalTimeUnits.sort().join(','),
           }),
-          // TODO: Duplication temporary
-          generateValidationIssue('invalidTag', {
-            tag: testDatasets.multipleInvalid[1],
-          }),
           generateValidationIssue('invalidTag', { tag: testDatasets.multipleInvalid[1] }),
         ],
       }

diff --git a/tests/stringParser.spec.js b/tests/stringParser.spec.js
@@ -341,14 +341,14 @@ describe('HED string parsing', () => {
           'Property/Sensory-property/Sensory-attribute/Visual-attribute/Color/RGB-color/RGB-red/0.5',
           'Item/Object/Man-made-object/Vehicle/Car',
         ],
-        invalidTag: ['InvalidTag'],
-        invalidParentNode: ['Car/Train/Maglev'],
+        invalidTag: [],
+        invalidParentNode: [],
       }
       const expectedIssues = {
         simple: {},
         groupAndTag: {},
         invalidTag: {
-          conversion: [generateIssue('invalidTag', { tag: expectedResults.invalidTag[0] })],
+          conversion: [generateIssue('invalidTag', { tag: testStrings.invalidTag })],
         },
         invalidParentNode: {
           conversion: [

diff --git a/utils/hedData.js b/utils/hedData.js
@@ -35,13 +35,15 @@ export const mergeParsingIssues = function (previousIssues, currentIssues) {
 export const getParsedParentTags = function (hedSchemas, shortTag) {
   const parentTags = new Map()
   for (const [schemaNickname, schema] of hedSchemas.schemas) {
-    const parentTag = new ParsedHed3Tag(
-      new TagSpec(shortTag, 0, shortTag.length - 1, schemaNickname),
-      hedSchemas,
-      shortTag,
-    )
-    parentTags.set(schema, parentTag)
-    parentTag.conversionIssues = parentTag.conversionIssues.filter((issue) => issue.internalCode !== 'invalidTag')
+    try {
+      const parentTag = new ParsedHed3Tag(
+        new TagSpec(shortTag, 0, shortTag.length - 1, schemaNickname),
+        hedSchemas,
+        shortTag,
+      )
+      parentTags.set(schema, parentTag)
+      // eslint-disable-next-line no-empty
+    } catch (e) {}
   }
   return parentTags
 }
diff --git a/validator/hed2/parser/parsedHed2Tag.js b/validator/hed2/parser/parsedHed2Tag.js
@@ -30,7 +30,6 @@ export class ParsedHed2Tag extends ParsedHedTag {
   // eslint-disable-next-line no-unused-vars
   _convertTag(hedString, hedSchemas, schemaName) {
     this.canonicalTag = this.originalTag
-    this.conversionIssues = []
     this.schema = hedSchemas.standardSchema
   }