From 976d7ed1558a1fe2f0a3dfb0c42d8e737a2776f3 Mon Sep 17 00:00:00 2001 From: Kay Robbins <1189050+VisLab@users.noreply.github.com> Date: Wed, 19 Feb 2025 17:04:01 -0600 Subject: [PATCH 1/2] Now handles deprecated tags at the string parsing level --- spec_tests/javascriptTests.json | 81 ++++++++++++++++++++++++ src/bids/types/json.js | 4 +- src/bids/validator/tsvValidator.js | 2 +- src/issues/data.js | 5 ++ src/parser/definitionManager.js | 32 +++++----- src/parser/parsedHedTag.js | 16 +++-- src/parser/parser.js | 13 ++++ src/parser/reservedChecker.js | 4 +- src/parser/tagConverter.js | 4 +- src/schema/entries.js | 22 ++++--- tests/definitionManagerTests.spec.js | 23 +++++-- tests/stringParserTests.spec.js | 18 ++++-- tests/tagParserTests.spec.js | 2 +- tests/testData/stringParserTests.data.js | 13 ++++ tests/testData/tagParserTests.data.js | 2 +- tests/testUtilities.js | 13 ++-- 16 files changed, 194 insertions(+), 60 deletions(-) diff --git a/spec_tests/javascriptTests.json b/spec_tests/javascriptTests.json index 9bf20fe1..abdff7fc 100644 --- a/spec_tests/javascriptTests.json +++ b/spec_tests/javascriptTests.json @@ -3396,6 +3396,87 @@ } } }, + { + "error_code": "TAG_DEPRECATED", + "alt_codes": [], + "name": "tag-deprecated", + "description": "A HED tag has been deprecated.", + "warning": true, + "schema": "8.3.0", + "definitions": ["(Definition/Acc/#, (Acceleration/# m-per-s^2, Red))", "(Definition/MyColor, (Label/Pie))"], + "tests": { + "string_tests": { + "fails": ["Gentalia", "Clock-face"], + "passes": ["(Red, Blue), Green"] + }, + "sidecar_tests": { + "fails": [ + { + "event_code": { + "HED": { + "square": "Gentalia, Clock-face" + } + } + } + ], + "passes": [ + { + "event_code": { + "HED": { + "square": "Genitalia" + } + } + } + ] + }, + "event_tests": { + "fails": [ + [ + ["onset", "duration", "HED"], + [5.5, 0, "Gentalia"] + ] + ], + "passes": [ + [ + ["onset", "duration", "HED"], + [4.5, 0, "Genitalia"] + ] + ] + }, + "combo_tests": { + "fails": [ + { + "sidecar": { + "event_code": { + "HED": { + "square": "Gentalia" + } + } + }, + "events": [ + ["onset", "duration", "event_code", "HED"], + [4.5, 0, "square", "Red, Def/MyColor, Clock-face"] + ] + } + ], + "passes": [ + { + "sidecar": { + "event_code": { + "HED": { + "face": "Genitalia" + } + } + }, + "events": [ + ["onset", "duration", "event_code", "HED"], + [4.5, 0, "face", "Blue"] + ] + } + ] + } + } + }, { "error_code": "TAG_EMPTY", "alt_codes": [], diff --git a/src/bids/types/json.js b/src/bids/types/json.js index c61084d5..a4dcf68c 100644 --- a/src/bids/types/json.js +++ b/src/bids/types/json.js @@ -363,9 +363,9 @@ export class BidsSidecarKey { * @private */ _parseValueString(hedSchemas) { - const [parsedString, errors, warnings] = parseHedString(this.valueString, hedSchemas, false, true) + const [parsedString, errorIssues, warningIssues] = parseHedString(this.valueString, hedSchemas, false, true) this.parsedValueString = parsedString - return [errors, warnings] + return [errorIssues, warningIssues] } /** diff --git a/src/bids/validator/tsvValidator.js b/src/bids/validator/tsvValidator.js index 4b0fba80..4a32380a 100644 --- a/src/bids/validator/tsvValidator.js +++ b/src/bids/validator/tsvValidator.js @@ -13,7 +13,7 @@ import { EventManager } from '../../parser/eventManager' */ export class BidsHedTsvValidator extends BidsValidator { /** - * The BIDS TSV file being validated. + * The singleton instance of the checker for reserved requirements. * @type {ReservedChecker} */ reserved diff --git a/src/issues/data.js b/src/issues/data.js index 0ef05641..452b176e 100644 --- a/src/issues/data.js +++ b/src/issues/data.js @@ -27,6 +27,11 @@ export default { level: 'error', message: stringTemplate`Comma missing after - "${'tag'}".`, }, + deprecatedTag: { + hedCode: 'TAG_DEPRECATED', + level: 'warning', + message: stringTemplate`Tags "${'tags'} in "${'string'} are deprecated. Please see tag description for instructions on replacement.".`, + }, duplicateTag: { hedCode: 'TAG_EXPRESSION_REPEATED', level: 'error', diff --git a/src/parser/definitionManager.js b/src/parser/definitionManager.js index 290b1dd6..b5a0ed50 100644 --- a/src/parser/definitionManager.js +++ b/src/parser/definitionManager.js @@ -54,27 +54,27 @@ export class Definition { * @param {ParsedHedTag} tag - The parsed HEd tag whose details should be checked. * @param {Schemas} hedSchema - The HED schemas used to validate against. * @param {boolean} placeholderAllowed - If true then placeholder is allowed in the def tag. - * @returns {Array} - Returns [string, Issue[]] containing the evaluated normalized definition string and any issues in the evaluation, + * @returns {Array} - Returns [string, Issue[], Issue[]] containing the evaluated normalized definition string and any issues in the evaluation, */ evaluateDefinition(tag, hedSchema, placeholderAllowed) { // Check that the level of the value of tag agrees with the definition if (!!this.defTag._splitValue !== !!tag._splitValue) { const errorType = tag.schemaTag.name === 'Def' ? 'missingDefinitionForDef' : 'missingDefinitionForDefExpand' - return [null, [generateIssue(errorType, { definition: tag._value })]] + return [null, [generateIssue(errorType, { definition: tag._value })], []] } // Check that the evaluated definition contents okay (if two-level value) if (!this.defContents) { - return ['', []] + return ['', [], []] } if (!this.defTag._splitValue || (placeholderAllowed && tag._splitValue === '#')) { - return [this.defContents.normalized, []] + return [this.defContents.normalized, [], []] } const evalString = this.defContents.originalTag.replace('#', tag._splitValue) - const [normalizedValue, issues] = parseHedString(evalString, hedSchema, false, false) - if (issues.length > 0) { - return [null, issues] + const [normalizedValue, errorIssues, warningIssues] = parseHedString(evalString, hedSchema, false, false) + if (errorIssues.length > 0) { + return [null, errorIssues, warningIssues] } - return [normalizedValue.normalized, []] + return [normalizedValue.normalized, [], []] } /** @@ -106,15 +106,15 @@ export class Definition { * * @param {string} hedString - A list of string definitions. * @param {Schemas} hedSchemas - The HED schemas to use in creation. - * @returns {Array} - Returns [Definition, Issue[]] with the definition and any issues. + * @returns {Array} - Returns [Definition, Issue[], Issue[]] with the definition and any issues. */ static createDefinition(hedString, hedSchemas) { - const [parsedString, issues] = parseHedString(hedString, hedSchemas, true, true) - if (issues.length > 0) { - return [null, issues] + const [parsedString, errorIssues, warningIssues] = parseHedString(hedString, hedSchemas, true, true) + if (errorIssues.length > 0) { + return [null, errorIssues, warningIssues] } if (parsedString.topLevelTags.length !== 0 || parsedString.tagGroups.length > 1) { - return [null, [generateIssue('invalidDefinition', { definition: hedString })]] + return [null, [generateIssue('invalidDefinition', { definition: hedString }), warningIssues]] } return Definition.createDefinitionFromGroup(parsedString.tagGroups[0]) } @@ -122,14 +122,14 @@ export class Definition { /** * Create a definition from a ParsedHedGroup. * @param {ParsedHedGroup} group - The group to create a definition from. - * @returns {Array} - Returns [Definition, Issue[]] with the definition and any issues. (The definition will be null if issues.) + * @returns {Array} - Returns [Definition, Issue[], Issue[]] with the definition and any issues. (The definition will be null if issues.) */ static createDefinitionFromGroup(group) { const def = new Definition(group) if (def._checkDefinitionPlaceholderCount()) { - return [def, []] + return [def, [], []] } - return [null, [generateIssue('invalidPlaceholderInDefinition', { definition: def.defGroup.originalTag })]] + return [null, [generateIssue('invalidPlaceholderInDefinition', { definition: def.defGroup.originalTag })], []] } } diff --git a/src/parser/parsedHedTag.js b/src/parser/parsedHedTag.js index e09ed89f..3865eb52 100644 --- a/src/parser/parsedHedTag.js +++ b/src/parser/parsedHedTag.js @@ -126,10 +126,7 @@ export default class ParsedHedTag extends ParsedHedSubstring { } // Check that there is a value if required const reserved = ReservedChecker.getInstance() - if ( - (schemaTag.hasAttributeName('requireChild') || reserved.requireValueTags.has(schemaTag.name)) && - remainder === '' - ) { + if ((schemaTag.hasAttribute('requireChild') || reserved.requireValueTags.has(schemaTag.name)) && remainder === '') { IssueError.generateAndThrow('valueRequired', { tag: this.originalTag }) } // Check if this could have a two-level value @@ -234,7 +231,8 @@ export default class ParsedHedTag extends ParsedHedSubstring { * @returns {boolean} Whether this tag has the named attribute. */ hasAttribute(attribute) { - return this.schema?.tagHasAttribute(this.formattedTag, attribute) + return this.schemaTag.hasAttribute(attribute) + //return this.schema?.tagHasAttribute(this.formattedTag, attribute) } /** @@ -262,6 +260,14 @@ export default class ParsedHedTag extends ParsedHedSubstring { } } + /** + * Indicates whether the tag is deprecated + * @returns {boolean} + */ + get isDeprecated() { + return this.schemaTag.hasAttribute('deprecatedFrom') + } + /** * Get the schema tag object for this tag's value-taking form. * diff --git a/src/parser/parser.js b/src/parser/parser.js index aca64589..abf17483 100644 --- a/src/parser/parser.js +++ b/src/parser/parser.js @@ -93,6 +93,19 @@ class HedStringParser { if (checkIssues.length > 0) { return [null, checkIssues, []] } + + // Check for deprecated + const deprecatedTags = parsedString.tags.filter((tag) => tag.isDeprecated === true) + if (deprecatedTags.length > 0) { + const deprecated = deprecatedTags.map((tag) => tag.toString()) + return [ + parsedString, + [], + [generateIssue('deprecatedTag', { tags: '[' + deprecated.join('],[') + ']', string: parsedString.hedString })], + ] + } + // Check for extension + return [parsedString, [], []] } diff --git a/src/parser/reservedChecker.js b/src/parser/reservedChecker.js index b0a4f635..ba4593b9 100644 --- a/src/parser/reservedChecker.js +++ b/src/parser/reservedChecker.js @@ -278,7 +278,7 @@ export class ReservedChecker { */ static hasTopLevelTagGroupAttribute(tag) { return ( - tag.schemaTag.hasAttributeName('topLevelTagGroup') || + tag.schemaTag.hasAttribute('topLevelTagGroup') || (ReservedChecker.reservedMap.has(tag.schemaTag.name) && ReservedChecker.reservedMap.get(tag.schemaTag.name).topLevelTagGroup) ) @@ -293,6 +293,6 @@ export class ReservedChecker { * Note: This checks both reserved and schema tag requirements. */ static hasGroupAttribute(tag) { - return tag.schemaTag.hasAttributeName('tagGroup') + return tag.schemaTag.hasAttribute('tagGroup') } } diff --git a/src/parser/tagConverter.js b/src/parser/tagConverter.js index 3ca9260d..6a36e8cc 100644 --- a/src/parser/tagConverter.js +++ b/src/parser/tagConverter.js @@ -106,7 +106,7 @@ export default class TagConverter { } if ( parentTag !== undefined && - (!parentTag.hasAttributeName('extensionAllowed') || this.special.noExtensionTags.has(parentTag.name)) + (!parentTag.hasAttribute('extensionAllowed') || this.special.noExtensionTags.has(parentTag.name)) ) { IssueError.generateAndThrow('invalidExtension', { tag: this.tagLevels[tagLevelIndex], @@ -154,7 +154,7 @@ export default class TagConverter { } this.schemaTag = schemaTag this.remainder = this.tagLevels.slice(remainderStartLevelIndex).join('/') - if (this.schemaTag?.hasAttributeName('requireChild') && !this.remainder) { + if (this.schemaTag?.hasAttribute('requireChild') && !this.remainder) { IssueError.generateAndThrow('childRequired', { tag: this.tagString }) } } diff --git a/src/schema/entries.js b/src/schema/entries.js index 22a7f818..e4cf539d 100644 --- a/src/schema/entries.js +++ b/src/schema/entries.js @@ -99,7 +99,7 @@ export class SchemaEntries extends Memoizer { if (!this.tags.hasLongNameEntry(tag)) { return false } - return this.tags.getLongNameEntry(tag).hasAttributeName(tagAttribute) + return this.tags.getLongNameEntry(tag).hasAttribute(tagAttribute) } } @@ -182,7 +182,7 @@ export class SchemaEntryManager extends Memoizer { getEntriesWithBooleanAttribute(booleanAttributeName) { return this._memoize(booleanAttributeName, () => { return this.filter(([, v]) => { - return v.hasAttributeName(booleanAttributeName) + return v.hasAttribute(booleanAttributeName) }) }) } @@ -436,22 +436,25 @@ export class SchemaAttribute extends SchemaEntry { /** * SchemaEntryWithAttributes class */ -class SchemaEntryWithAttributes extends SchemaEntry { +export class SchemaEntryWithAttributes extends SchemaEntry { /** * The set of boolean attributes this schema entry has. * @type {Set} */ booleanAttributes + /** * The collection of value attributes this schema entry has. * @type {Map} */ valueAttributes + /** * The set of boolean attribute names this schema entry has. * @type {Set} */ booleanAttributeNames + /** * The collection of value attribute names this schema entry has. * @type {Map} @@ -487,7 +490,7 @@ class SchemaEntryWithAttributes extends SchemaEntry { * @returns {boolean} Whether this schema entry has this attribute. */ hasAttribute(attribute) { - return this.booleanAttributes.has(attribute) + return this.booleanAttributeNames.has(attribute) || this.valueAttributeNames.has(attribute) } /** @@ -595,15 +598,15 @@ export class SchemaUnit extends SchemaEntryWithAttributes { } get isPrefixUnit() { - return this.hasAttributeName('unitPrefix') + return this.hasAttribute('unitPrefix') } get isSIUnit() { - return this.hasAttributeName('SIUnit') + return this.hasAttribute('SIUnit') } get isUnitSymbol() { - return this.hasAttributeName('unitSymbol') + return this.hasAttribute('unitSymbol') } /** @@ -722,11 +725,11 @@ export class SchemaUnitModifier extends SchemaEntryWithAttributes { } get isSIUnitModifier() { - return this.hasAttributeName('SIUnitModifier') + return this.hasAttribute('SIUnitModifier') } get isSIUnitSymbolModifier() { - return this.hasAttributeName('SIUnitSymbolModifier') + return this.hasAttribute('SIUnitSymbolModifier') } } @@ -784,6 +787,7 @@ export class SchemaTag extends SchemaEntryWithAttributes { * @private */ #parent + /** * This tag's unit classes. * @type {SchemaUnitClass[]} diff --git a/tests/definitionManagerTests.spec.js b/tests/definitionManagerTests.spec.js index 64e48a22..8b42f5b2 100644 --- a/tests/definitionManagerTests.spec.js +++ b/tests/definitionManagerTests.spec.js @@ -61,20 +61,29 @@ describe('DefinitionManager tests', () => { } thisDefManager.addDefinitions(defsToAdd) } - const [parsedHed, issues] = parseHedString(test.stringIn, thisSchema, false, test.placeholderAllowed) - if (parsedHed === null && issues.length > 0) { + const [parsedHed, errorIssues, warningIssues] = parseHedString( + test.stringIn, + thisSchema, + false, + test.placeholderAllowed, + ) + if (parsedHed === null && errorIssues.length > 0) { assert.deepStrictEqual( - issues, + errorIssues, test.errors, - `${header}: expected ${issues} errors but received ${test.errors}\n`, + `${header}: expected ${errorIssues} errors but received ${test.errors}\n`, ) } if (parsedHed === null) { return } - issues.push(...thisDefManager.validateDefs(parsedHed, thisSchema, test.placeholderAllowed)) - issues.push(...thisDefManager.validateDefExpands(parsedHed, thisSchema, test.placeholderAllowed)) - assert.deepStrictEqual(issues, test.errors, `${header}: expected ${issues} errors but received ${test.errors}\n`) + errorIssues.push(...thisDefManager.validateDefs(parsedHed, thisSchema, test.placeholderAllowed)) + errorIssues.push(...thisDefManager.validateDefExpands(parsedHed, thisSchema, test.placeholderAllowed)) + assert.deepStrictEqual( + errorIssues, + test.errors, + `${header}: expected ${errorIssues} errors but received ${test.errors}\n`, + ) } test.each(tests)('$testname: $explanation ', (test) => { diff --git a/tests/stringParserTests.spec.js b/tests/stringParserTests.spec.js index a67badce..b680fbe5 100644 --- a/tests/stringParserTests.spec.js +++ b/tests/stringParserTests.spec.js @@ -12,7 +12,8 @@ import { shouldRun, getHedString } from './testUtilities' const skipMap = new Map() const runAll = true -const runMap = new Map([['valid-value-tags', ['date-time-value-tag']]]) +//const runMap = new Map([['valid-tags', ['single-tag-extension']]]) +const runMap = new Map([['valid-tags', ['deprecated-tag']]]) describe('Null schema objects should cause parsing to bail', () => { it('Should not proceed if no schema and valid string', () => { @@ -21,10 +22,12 @@ describe('Null schema objects should cause parsing to bail', () => { assert.isNull(parsedString, `Parsed HED string of ${stringIn} is null although string is valid`) const expectedIssues = [generateIssue('missingSchemaSpecification', {})] assert.deepStrictEqual(errorIssues, expectedIssues, `A SCHEMA_LOAD_FAILED issue should be generated`) - const [directParsed, directIssues] = parseHedString(stringIn, null, true, true) - assert.isNull(directParsed, `Parsed HED string of ${stringIn} is null for invalid string`) - assert.deepStrictEqual(directIssues, expectedIssues) assert.equal(warningIssues.length, 0, `Null schema produces errors, not warnings`) + + const [directParsed, errorsDirect, warningsDirect] = parseHedString(stringIn, null, true, true) + assert.isNull(directParsed, `Parsed HED string of ${stringIn} is null for invalid string`) + assert.deepStrictEqual(errorsDirect, expectedIssues) + assert.equal(warningsDirect.length, 0, `Null schema produces errors, not warnings`) }) it('Should not proceed if no schema and invalid string', () => { @@ -33,10 +36,11 @@ describe('Null schema objects should cause parsing to bail', () => { assert.isNull(parsedString, `Parsed HED string of ${stringIn} is null for invalid string`) const expectedIssues = [generateIssue('missingSchemaSpecification', {})] assert.deepStrictEqual(errorIssues, expectedIssues, `A SCHEMA_LOAD_FAILED issue should be generated`) - const [directParsed, directIssues] = parseHedString(stringIn, null, true, true) - assert.isNull(directParsed, `Parsed HED string of ${stringIn} is null for invalid string`) - assert.deepStrictEqual(directIssues, expectedIssues) assert.equal(warningIssues.length, 0, `Null schema produces errors, not warnings`) + const [directParsed, errorsDirect, warningsDirect] = parseHedString(stringIn, null, true, true) + assert.isNull(directParsed, `Parsed HED string of ${stringIn} is null for invalid string`) + assert.deepStrictEqual(errorsDirect, expectedIssues) + assert.equal(warningsDirect.length, 0, `Null schema produces errors, not warnings`) }) it('Should not proceed if no schema and valid array of strings', () => { diff --git a/tests/tagParserTests.spec.js b/tests/tagParserTests.spec.js index 0ebd6baa..8a429415 100644 --- a/tests/tagParserTests.spec.js +++ b/tests/tagParserTests.spec.js @@ -13,7 +13,7 @@ import { SchemaValueTag } from '../src/schema/entries' // Ability to select individual tests to run const skipMap = new Map() const runAll = true -const runMap = new Map([['name-space-tests', ['valid-date-time']]]) +const runMap = new Map([['valid-tags', ['valid-short-tag-with-cascade-extension']]]) describe('TagSpec converter tests using JSON tests', () => { const schemaMap = new Map([['8.3.0', undefined]]) diff --git a/tests/testData/stringParserTests.data.js b/tests/testData/stringParserTests.data.js index e37a9f1a..050ebbf4 100644 --- a/tests/testData/stringParserTests.data.js +++ b/tests/testData/stringParserTests.data.js @@ -102,6 +102,19 @@ export const parseTestData = [ errors: [], warnings: [], }, + { + testname: 'deprecated-tag', + explanation: '"Deprecated tag should give a warning"', + schemaVersion: '8.3.0', + stringIn: 'Agent-action, Gentalia', + stringLong: + 'Event/Agent-action, Item/Biological-item/Anatomical-item/Body-part/Torso-part/Pelvis-part/Gentalia', + stringShort: 'Agent-action, Gentalia', + placeholdersAllowed: false, + definitionsAllowed: false, + errors: [], + warnings: [generateIssue('deprecatedTag', { string: 'Agent-action, Gentalia', tags: '[Gentalia]' })], + }, ], }, { diff --git a/tests/testData/tagParserTests.data.js b/tests/testData/tagParserTests.data.js index 241686e8..cc82e204 100644 --- a/tests/testData/tagParserTests.data.js +++ b/tests/testData/tagParserTests.data.js @@ -4,7 +4,7 @@ import { TagSpec } from '../../src/parser/tokenizer' export const parsedHedTagTests = [ { name: 'valid-tags', - description: 'Valid tags with extensions', + description: 'Valid tags with different types of forms', warning: false, tests: [ { diff --git a/tests/testUtilities.js b/tests/testUtilities.js index 9287e7b4..32870ca6 100644 --- a/tests/testUtilities.js +++ b/tests/testUtilities.js @@ -30,13 +30,12 @@ export function extractHedCodes(issues) { // Parse the HED string export function getHedString(hedString, hedSchemas, definitionsAllowed, placeholdersAllowed) { - const [parsedString, issues] = parseHedString(hedString, hedSchemas, definitionsAllowed, placeholdersAllowed) - let errorIssues = [] - let warningIssues = [] - if (issues.length !== 0) { - errorIssues = issues.filter((obj) => obj.level === 'error') - warningIssues = issues.filter((obj) => obj.level !== 'error') - } + const [parsedString, errorIssues, warningIssues] = parseHedString( + hedString, + hedSchemas, + definitionsAllowed, + placeholdersAllowed, + ) if (errorIssues.length > 0) { return [null, errorIssues, warningIssues] } else { From c3926586d4ddc05b0727d035773fc52a4406f068 Mon Sep 17 00:00:00 2001 From: Kay Robbins <1189050+VisLab@users.noreply.github.com> Date: Thu, 20 Feb 2025 08:42:36 -0600 Subject: [PATCH 2/2] Now handles tag extension warnings --- spec_tests/javascriptTests.json | 81 ------------------------ spec_tests/jsonTests.spec.js | 48 +++++++++++--- src/issues/data.js | 12 ++-- src/parser/parsedHedTag.js | 8 +++ src/parser/parser.js | 32 +++++++--- tests/testData/stringParserTests.data.js | 58 +++++++++++++++-- 6 files changed, 130 insertions(+), 109 deletions(-) diff --git a/spec_tests/javascriptTests.json b/spec_tests/javascriptTests.json index abdff7fc..9bf20fe1 100644 --- a/spec_tests/javascriptTests.json +++ b/spec_tests/javascriptTests.json @@ -3396,87 +3396,6 @@ } } }, - { - "error_code": "TAG_DEPRECATED", - "alt_codes": [], - "name": "tag-deprecated", - "description": "A HED tag has been deprecated.", - "warning": true, - "schema": "8.3.0", - "definitions": ["(Definition/Acc/#, (Acceleration/# m-per-s^2, Red))", "(Definition/MyColor, (Label/Pie))"], - "tests": { - "string_tests": { - "fails": ["Gentalia", "Clock-face"], - "passes": ["(Red, Blue), Green"] - }, - "sidecar_tests": { - "fails": [ - { - "event_code": { - "HED": { - "square": "Gentalia, Clock-face" - } - } - } - ], - "passes": [ - { - "event_code": { - "HED": { - "square": "Genitalia" - } - } - } - ] - }, - "event_tests": { - "fails": [ - [ - ["onset", "duration", "HED"], - [5.5, 0, "Gentalia"] - ] - ], - "passes": [ - [ - ["onset", "duration", "HED"], - [4.5, 0, "Genitalia"] - ] - ] - }, - "combo_tests": { - "fails": [ - { - "sidecar": { - "event_code": { - "HED": { - "square": "Gentalia" - } - } - }, - "events": [ - ["onset", "duration", "event_code", "HED"], - [4.5, 0, "square", "Red, Def/MyColor, Clock-face"] - ] - } - ], - "passes": [ - { - "sidecar": { - "event_code": { - "HED": { - "face": "Genitalia" - } - } - }, - "events": [ - ["onset", "duration", "event_code", "HED"], - [4.5, 0, "face", "Blue"] - ] - } - ] - } - } - }, { "error_code": "TAG_EMPTY", "alt_codes": [], diff --git a/spec_tests/jsonTests.spec.js b/spec_tests/jsonTests.spec.js index 89c03bd6..2a46b681 100644 --- a/spec_tests/jsonTests.spec.js +++ b/spec_tests/jsonTests.spec.js @@ -22,9 +22,7 @@ const runMap = new Map([['TAG_EXPRESSION_REPEATED', ['tags-duplicated-across-mul //const runOnly = new Set(["eventsPass"]) const runOnly = new Set() const skippedErrors = { - TAG_EXTENDED: 'Warning not being checked', SIDECAR_KEY_MISSING: 'Warning not being checked', - ELEMENT_DEPRECATED: 'Warning not being checked', } const readFileSync = fs.readFileSync const test_file_name = 'javascriptTests.json' @@ -137,22 +135,54 @@ describe('HED validation using JSON tests', () => { const failedCombos = comboListToStrings(tests.combo_tests.fails) const passedCombos = comboListToStrings(tests.combo_tests.passes) - const assertErrors = function (expectedErrors, issues, header) { - // Get the set of actual issues that were encountered. + /** + * Separates the error codes and warning codes from the issues + * @param issues + * @returns {[Set,Set warnings.has(element)) + if (expectedErrors.size === 0 && warnings.size === 0) { + warningIntersection = true } + assert.isTrue( + warningIntersection, + `${header} expected one of warnings[${[...expectedErrors].join(', ')}] but received [${[...warnings].join(', ')}]`, + ) + return } - let hasIntersection = [...expectedErrors].some((element) => errors.has(element)) + let errorIntersection = [...expectedErrors].some((element) => errors.has(element)) if (expectedErrors.size === 0 && errors.size === 0) { - hasIntersection = true + errorIntersection = true } assert.isTrue( - hasIntersection, + errorIntersection, `${header} expected one of errors[${[...expectedErrors].join(', ')}] but received [${[...errors].join(', ')}]`, ) } diff --git a/src/issues/data.js b/src/issues/data.js index 452b176e..0f1ca98a 100644 --- a/src/issues/data.js +++ b/src/issues/data.js @@ -28,7 +28,7 @@ export default { message: stringTemplate`Comma missing after - "${'tag'}".`, }, deprecatedTag: { - hedCode: 'TAG_DEPRECATED', + hedCode: 'ELEMENT_DEPRECATED', level: 'warning', message: stringTemplate`Tags "${'tags'} in "${'string'} are deprecated. Please see tag description for instructions on replacement.".`, }, @@ -37,6 +37,11 @@ export default { level: 'error', message: stringTemplate`Duplicate tags - "${'tags'} in "${'string'}".`, }, + extendedTag: { + hedCode: 'TAG_EXTENDED', + level: 'warning', + message: stringTemplate`Tag extensions found for ${'tags'} in "${'string'}".`, + }, invalidCharacter: { hedCode: 'CHARACTER_INVALID', level: 'error', @@ -103,11 +108,6 @@ export default { level: 'error', message: stringTemplate`Invalid placeholder value for tag "${'tag'}".`, }, - extension: { - hedCode: 'TAG_EXTENDED', - level: 'warning', - message: stringTemplate`Tag extension found - "${'tag'}".`, - }, invalidPlaceholderContext: { hedCode: 'PLACEHOLDER_INVALID', level: 'error', diff --git a/src/parser/parsedHedTag.js b/src/parser/parsedHedTag.js index 3865eb52..6d963de9 100644 --- a/src/parser/parsedHedTag.js +++ b/src/parser/parsedHedTag.js @@ -268,6 +268,14 @@ export default class ParsedHedTag extends ParsedHedSubstring { return this.schemaTag.hasAttribute('deprecatedFrom') } + /** + * Indicates whether the tag is deprecated + * @returns {boolean} + */ + get isExtended() { + return !this.takesValueTag && this._remainder !== '' + } + /** * Get the schema tag object for this tag's value-taking form. * diff --git a/src/parser/parser.js b/src/parser/parser.js index abf17483..ec88cfd5 100644 --- a/src/parser/parser.js +++ b/src/parser/parser.js @@ -94,19 +94,35 @@ class HedStringParser { return [null, checkIssues, []] } + // Warnings are only checked when there are no fatal errors + return [parsedString, [], this._getWarnings(parsedString)] + } + + /** + * Get warnings applicable for a parsed HED string. + * @param {ParsedHedString} parsedString - HED string object to check for warnings. + * @returns {Issue[]} - Warnings for the parsed HED string + * @private + */ + _getWarnings(parsedString) { + const warnings = [] // Check for deprecated const deprecatedTags = parsedString.tags.filter((tag) => tag.isDeprecated === true) if (deprecatedTags.length > 0) { const deprecated = deprecatedTags.map((tag) => tag.toString()) - return [ - parsedString, - [], - [generateIssue('deprecatedTag', { tags: '[' + deprecated.join('],[') + ']', string: parsedString.hedString })], - ] + warnings.push( + generateIssue('deprecatedTag', { tags: '[' + deprecated.join(', ') + ']', string: parsedString.hedString }), + ) } - // Check for extension - - return [parsedString, [], []] + // Check for tag extensions + const extendedTags = parsedString.tags.filter((tag) => tag.isExtended === true) + if (extendedTags.length > 0) { + const extended = extendedTags.map((tag) => tag.toString()) + warnings.push( + generateIssue('extendedTag', { tags: '[' + extended.join(', ') + ']', string: parsedString.hedString }), + ) + } + return warnings } /** diff --git a/tests/testData/stringParserTests.data.js b/tests/testData/stringParserTests.data.js index 050ebbf4..aacca3c9 100644 --- a/tests/testData/stringParserTests.data.js +++ b/tests/testData/stringParserTests.data.js @@ -75,7 +75,26 @@ export const parseTestData = [ placeholdersAllowed: false, definitionsAllowed: false, errors: [], - warnings: [], + warnings: [ + generateIssue('extendedTag', { + string: 'Item/Sound/Environmental-sound/Unique-value', + tags: '[Item/Sound/Environmental-sound/Unique-value]', + }), + ], + }, + { + testname: 'multiple-extended-tags', + explanation: '"Item/Junk1, Item/Junk2" has two extended tags"', + schemaVersion: '8.3.0', + stringIn: 'Item/Junk1, Item/Junk2', + stringLong: 'Item/Junk1, Item/Junk2', + stringShort: 'Item/Junk1, Item/Junk2', + placeholdersAllowed: false, + definitionsAllowed: false, + errors: [], + warnings: [ + generateIssue('extendedTag', { string: 'Item/Junk1, Item/Junk2', tags: '[Item/Junk1, Item/Junk2]' }), + ], }, { testname: 'multi-level-tag-extension', @@ -84,11 +103,15 @@ export const parseTestData = [ stringIn: 'Item/Sound/Environmental-sound/Unique-value/Junk', stringLong: 'Item/Sound/Environmental-sound/Unique-value/Junk', stringShort: 'Environmental-sound/Unique-value/Junk', - operation: 'toShort', placeholdersAllowed: false, definitionsAllowed: false, errors: [], - warnings: [], + warnings: [ + generateIssue('extendedTag', { + string: 'Item/Sound/Environmental-sound/Unique-value/Junk', + tags: '[Item/Sound/Environmental-sound/Unique-value/Junk]', + }), + ], }, { testname: 'multi-level-tag-extension-for-partial-path-to-short', @@ -100,7 +123,12 @@ export const parseTestData = [ placeholdersAllowed: false, definitionsAllowed: false, errors: [], - warnings: [], + warnings: [ + generateIssue('extendedTag', { + string: 'Sound/Environmental-sound/Unique-value/Junk', + tags: '[Sound/Environmental-sound/Unique-value/Junk]', + }), + ], }, { testname: 'deprecated-tag', @@ -115,6 +143,24 @@ export const parseTestData = [ errors: [], warnings: [generateIssue('deprecatedTag', { string: 'Agent-action, Gentalia', tags: '[Gentalia]' })], }, + { + testname: 'multiple-deprecated-tags', + explanation: '"Multiple deprecated tags should give 1 warning"', + schemaVersion: '8.3.0', + stringIn: 'Agent-action, Gentalia, (Item, Gentalia)', + stringLong: + 'Event/Agent-action, Item/Biological-item/Anatomical-item/Body-part/Torso-part/Pelvis-part/Gentalia, (Item, Item/Biological-item/Anatomical-item/Body-part/Torso-part/Pelvis-part/Gentalia)', + stringShort: 'Agent-action, Gentalia, (Item, Gentalia)', + placeholdersAllowed: false, + definitionsAllowed: false, + errors: [], + warnings: [ + generateIssue('deprecatedTag', { + string: 'Agent-action, Gentalia, (Item, Gentalia)', + tags: '[Gentalia, Gentalia]', + }), + ], + }, ], }, { @@ -186,7 +232,9 @@ export const parseTestData = [ placeholdersAllowed: false, definitionsAllowed: false, errors: [], - warnings: [], + warnings: [ + generateIssue('extendedTag', { string: '(Train/Maglev,Age/15,RGB-red/0.5),Operate', tags: '[Train/Maglev]' }), + ], }, { testname: 'value with units',