Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Throw errors from ParsedHedTag constructors instead of using field #200

Merged
merged 2 commits into from
Oct 7, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
49 changes: 19 additions & 30 deletions parser/parsedHedTag.js
Original file line number Diff line number Diff line change
@@ -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'
Expand All @@ -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}
Expand All @@ -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()
}
Expand Down Expand Up @@ -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)
Expand All @@ -316,43 +312,36 @@ 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]+$/
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
this.schema = hedSchemas.getSchema(schemaName)
if (this.schema === undefined) {
this.canonicalTag = this.originalTag
if (schemaName !== '') {
this.conversionIssues = [
generateIssue('unmatchedLibrarySchema', {
tag: this.originalTag,
library: schemaName,
}),
]
IssueError.generateAndThrow('unmatchedLibrarySchema', {
tag: this.originalTag,
library: schemaName,
})
} else {
this.conversionIssues = [
generateIssue('unmatchedBaseSchema', {
tag: this.originalTag,
}),
]
IssueError.generateAndThrow('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)
}

/**
Expand Down
31 changes: 20 additions & 11 deletions parser/splitHedString.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) =>
Expand Down Expand Up @@ -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)
}
Expand All @@ -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)
}
}
Expand Down
2 changes: 0 additions & 2 deletions tests/bids.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 = {
Expand All @@ -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 }),
Expand Down
8 changes: 0 additions & 8 deletions tests/dataset.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -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] }),
],
}
Expand Down Expand Up @@ -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] }),
],
}
Expand Down
6 changes: 3 additions & 3 deletions tests/stringParser.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -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: [
Expand Down
16 changes: 9 additions & 7 deletions utils/hedData.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
1 change: 0 additions & 1 deletion validator/hed2/parser/parsedHed2Tag.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down
Loading