From 88d5f277ee0c01fede056d4571c6be7297cfe521 Mon Sep 17 00:00:00 2001 From: Alexander Jones Date: Fri, 4 Oct 2024 08:53:37 -0500 Subject: [PATCH 1/2] Throw errors from ParsedHedTag constructors instead of using field Also fix testcases to remove now-deleted duplicate messages and bad outputs. --- parser/parsedHedTag.js | 35 ++++++++++---------------- parser/splitHedString.js | 31 +++++++++++++++-------- tests/bids.spec.js | 2 -- tests/dataset.spec.js | 8 ------ tests/stringParser.spec.js | 6 ++--- utils/hedData.js | 16 ++++++------ validator/hed2/parser/parsedHed2Tag.js | 1 - 7 files changed, 45 insertions(+), 54 deletions(-) diff --git a/parser/parsedHedTag.js b/parser/parsedHedTag.js index cf6ac97d..420b0f72 100644 --- 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 index d0d96db0..1887fdc4 100644 --- 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 index 19e2cd6d..af427877 100644 --- 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 index eb860d69..8963264f 100644 --- 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 index def20bd1..0e3aca2f 100644 --- 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 index 13835693..31509713 100644 --- 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 index 7f259929..f9991318 100644 --- 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 } From 43f20500879884805d2596ecc7d1604e4024801e Mon Sep 17 00:00:00 2001 From: Alexander Jones Date: Mon, 7 Oct 2024 16:44:10 -0500 Subject: [PATCH 2/2] Use new IssueError static method to throw IssueErrors --- parser/parsedHedTag.js | 22 ++++++++++------------ 1 file changed, 10 insertions(+), 12 deletions(-) diff --git a/parser/parsedHedTag.js b/parser/parsedHedTag.js index 420b0f72..6542f74e 100644 --- a/parser/parsedHedTag.js +++ b/parser/parsedHedTag.js @@ -317,7 +317,9 @@ export class ParsedHed3Tag extends ParsedHedTag { _convertTag(hedSchemas, hedString, tagSpec) { const hed3ValidCharacters = /^[^{}[\]()~,\0\t]+$/ if (!hed3ValidCharacters.test(this.originalTag)) { - throw new Error('The parser failed to properly remove an illegal or special character.') + IssueError.generateAndThrow('internalConsistencyError', { + message: 'The parser failed to properly remove an illegal or special character.', + }) } const schemaName = tagSpec.library @@ -325,18 +327,14 @@ export class ParsedHed3Tag extends ParsedHedTag { if (this.schema === undefined) { this.canonicalTag = this.originalTag if (schemaName !== '') { - throw new IssueError( - generateIssue('unmatchedLibrarySchema', { - tag: this.originalTag, - library: schemaName, - }), - ) + IssueError.generateAndThrow('unmatchedLibrarySchema', { + tag: this.originalTag, + library: schemaName, + }) } else { - throw new IssueError( - generateIssue('unmatchedBaseSchema', { - tag: this.originalTag, - }), - ) + IssueError.generateAndThrow('unmatchedBaseSchema', { + tag: this.originalTag, + }) } }