From 81c6fdc842691958d6ec659c81d6b0a31a8892bb Mon Sep 17 00:00:00 2001 From: Kay Robbins <1189050+VisLab@users.noreply.github.com> Date: Tue, 12 Nov 2024 15:29:40 -0600 Subject: [PATCH 1/4] Added tests for values and unit variations to tagParserTests --- tests/event.spec.js | 18 +++---- tests/tagParserTests.spec.js | 2 +- tests/testData/tagParserTests.data.js | 75 +++++++++++++++++++++++++-- 3 files changed, 80 insertions(+), 15 deletions(-) diff --git a/tests/event.spec.js b/tests/event.spec.js index b1699984..2c9f1ada 100644 --- a/tests/event.spec.js +++ b/tests/event.spec.js @@ -617,8 +617,8 @@ describe('HED string and event validation', () => { ) }) - // TODO: Wait to generate tests until detection moved to stringParser. - it('should have a proper unit when required', () => { + // TODO: REMOVE as these tests have been moved to tagParserTests + it.skip('(REMOVE) now in tagParserTests - should have a proper unit when required', () => { const testStrings = { correctUnit: 'Time-value/3 ms', correctUnitScientific: 'Time-value/3.5e1 ms', @@ -727,25 +727,25 @@ describe('HED string and event validation', () => { }) }) - // TODO: The requireChild seems to be hardcoded somewhere as stringParser doesn't require it. - it.skip('(INCORRECT REWRITE) should have a child when required', () => { + // TODO: This has been fixed + it('should have a child when required', () => { const testStrings = { noRequiredChild: 'Red', - hasRequiredChild: 'Label/Blah', - missingChild: 'Label', - longMissingChild: 'Property/Informational-property/Label', + hasRequiredChild: '(Duration/5)', + missingChild: '(Duration)', + longMissingChild: '(Property/Data-property/Data-value/Spatiotemporal-value/Temporal-value/Duration)', } const expectedIssues = { noRequiredChild: [], hasRequiredChild: [], missingChild: [ generateIssue('childRequired', { - tag: testStrings.missingChild, + tag: 'Duration', }), ], longMissingChild: [ generateIssue('childRequired', { - tag: testStrings.longMissingChild, + tag: 'Property/Data-property/Data-value/Spatiotemporal-value/Temporal-value/Duration', }), ], } diff --git a/tests/tagParserTests.spec.js b/tests/tagParserTests.spec.js index ab73ee4a..1f67a8ff 100644 --- a/tests/tagParserTests.spec.js +++ b/tests/tagParserTests.spec.js @@ -36,7 +36,7 @@ describe('TagSpec converter tests using JSON tests', () => { afterAll(() => {}) - describe('TagConverter tests', () => { + describe.skip('TagConverter tests', () => { it('should be able to convert', () => { const thisSchema = schemaMap.get('8.3.0') assert.isDefined(thisSchema, 'yes') diff --git a/tests/testData/tagParserTests.data.js b/tests/testData/tagParserTests.data.js index 3dbd4297..a2064337 100644 --- a/tests/testData/tagParserTests.data.js +++ b/tests/testData/tagParserTests.data.js @@ -198,7 +198,7 @@ export const parsedHedTagTests = [ error: generateIssue('invalidParentNode', { parentTag: 'object/Junk/baloney', tag: 'Red' }), }, { - testname: 'invalid-tag-bad-units', + testname: 'invalid-tag-bad-unit-class', explanation: '"Length/2 s" has wrong unit class.', schemaVersion: '8.3.0', fullString: 'Length/2 s', @@ -211,17 +211,82 @@ export const parsedHedTagTests = [ error: generateIssue('unitClassInvalidUnit', { tag: 'Length/2 s' }), }, { - testname: 'invalid-tag-bad-units', + testname: 'invalid-tag-bad-unit-plural', + explanation: '"Frequency/3 hertzs" is not the plural of hertz.', + schemaVersion: '8.3.0', + fullString: 'Frequency/3 hertzs', + tagSpec: new TagSpec('Frequency/3 hertzs', 0, 18, ''), + tagLong: undefined, + tagShort: undefined, + formattedTag: undefined, + canonicalTag: undefined, + takesValue: false, + error: generateIssue('unitClassInvalidUnit', { tag: 'Frequency/3 hertzs' }), + }, + { + testname: 'invalid-tag-incorrect-unit-symbol-caps', + explanation: '"Frequency/3 hz" has unit symbol not correctly capitalized.', + schemaVersion: '8.3.0', + fullString: 'Frequency/3 hz', + tagSpec: new TagSpec('Frequency/3 hz', 0, 14, ''), + tagLong: undefined, + tagShort: undefined, + formattedTag: undefined, + canonicalTag: undefined, + takesValue: false, + error: generateIssue('unitClassInvalidUnit', { tag: 'Frequency/3 hz' }), + }, + { + testname: 'invalid-tag-incorrect-unit-modifier caps', + explanation: '"Frequency/3 KHz" has unit modifier not correctly capitalized.', + schemaVersion: '8.3.0', + fullString: 'Frequency/3 KHz', + tagSpec: new TagSpec('Frequency/3 KHz', 0, 15, ''), + tagLong: undefined, + tagShort: undefined, + formattedTag: undefined, + canonicalTag: undefined, + takesValue: false, + error: generateIssue('unitClassInvalidUnit', { tag: 'Frequency/3 KHz' }), + }, + { + testname: 'invalid-tag-non-SI-unit-modified', + explanation: '"Time-value/1 millihour" has a non-SI unit with unit modifier.', + schemaVersion: '8.3.0', + fullString: 'Time-value/1 millihour', + tagSpec: new TagSpec('Time-value/1 millihour', 0, 22, ''), + tagLong: undefined, + tagShort: undefined, + formattedTag: undefined, + canonicalTag: undefined, + takesValue: false, + error: generateIssue('unitClassInvalidUnit', { tag: 'Time-value/1 millihour' }), + }, + { + testname: 'invalid-tag-bad-unit-symbol-modifier', + explanation: '"Speed/100 Mkph" has a bad unit symbol.', + schemaVersion: '8.3.0', + fullString: 'Speed/100 Mkph', + tagSpec: new TagSpec('Speed/100 Mkph', 0, 14, ''), + tagLong: undefined, + tagShort: undefined, + formattedTag: undefined, + canonicalTag: undefined, + takesValue: false, + error: generateIssue('unitClassInvalidUnit', { tag: 'Speed/100 Mkph' }), + }, + { + testname: 'invalid-tag-bad-units-double-blank', explanation: '"Length/5 m" has a double blank.', schemaVersion: '8.3.0', - fullString: 'Length/2 s', - tagSpec: new TagSpec('Length/2 s', 0, 10, ''), + fullString: 'Length/5 m', + tagSpec: new TagSpec('Length/5 m', 0, 11, ''), tagLong: undefined, tagShort: undefined, formattedTag: undefined, canonicalTag: undefined, takesValue: false, - error: generateIssue('unitClassInvalidUnit', { tag: 'Length/2 s' }), + error: generateIssue('unitClassInvalidUnit', { tag: 'Length/5 m' }), }, { testname: 'invalid-tag-bad-unit-capitalization', From d3f8d48def2912611dff752df32a931a8b0e81c2 Mon Sep 17 00:00:00 2001 From: Kay Robbins <1189050+VisLab@users.noreply.github.com> Date: Wed, 13 Nov 2024 09:10:39 -0600 Subject: [PATCH 2/4] parseHedString now returns immediately with error if no schema --- parser/parser.js | 8 ++++- tests/bidsTests.spec.js | 2 +- tests/schemaBuildTests.spec.js | 2 +- tests/schemaSpecTests.spec.js | 2 +- tests/stringParserTests.spec.js | 44 ++++++++++++++++++++---- tests/tagParserTests.spec.js | 5 +-- tests/testData/stringParserTests.data.js | 27 +++++++++++---- tests/tokenizerTests.spec.js | 2 +- validator/event/init.js | 3 +- 9 files changed, 74 insertions(+), 21 deletions(-) diff --git a/parser/parser.js b/parser/parser.js index 51b15f47..87b33f10 100644 --- a/parser/parser.js +++ b/parser/parser.js @@ -1,6 +1,7 @@ import { mergeParsingIssues } from '../utils/hedData' import ParsedHedString from './parsedHedString' import HedStringSplitter from './splitter' +import { generateIssue } from '../common/issues/issues' /** * A parser for HED strings. @@ -37,6 +38,9 @@ class HedStringParser { if (this.hedString instanceof ParsedHedString) { return [this.hedString, {}] } + if (!this.hedSchemas) { + return [null, { syntaxIssues: [generateIssue('missingSchemaSpecification', {})] }] + } const [parsedTags, parsingIssues] = new HedStringSplitter(this.hedString, this.hedSchemas).splitHedString() if (parsedTags === null) { @@ -55,9 +59,11 @@ class HedStringParser { * @returns {[ParsedHedString[], Object]} The parsed HED strings and any issues found. */ static parseHedStrings(hedStrings, hedSchemas) { + if (!hedSchemas) { + return [null, { syntaxIssues: [generateIssue('missingSchemaSpecification', {})] }] + } const parsedStrings = [] const cumulativeIssues = {} - for (const hedString of hedStrings) { const [parsedString, currentIssues] = new HedStringParser(hedString, hedSchemas).parseHedString() parsedStrings.push(parsedString) diff --git a/tests/bidsTests.spec.js b/tests/bidsTests.spec.js index 885698ed..839f4df8 100644 --- a/tests/bidsTests.spec.js +++ b/tests/bidsTests.spec.js @@ -84,7 +84,7 @@ describe('BIDS validation', () => { if (shouldRun(name, test.testname, runAll, runMap, skipMap)) { validate(test) } else { - console.log(`----Skipping ${name}: ${test.testname}`) + console.log(`----Skipping bidsTest ${name}: ${test.testname}`) } }) } diff --git a/tests/schemaBuildTests.spec.js b/tests/schemaBuildTests.spec.js index 9bc49daa..eb383533 100644 --- a/tests/schemaBuildTests.spec.js +++ b/tests/schemaBuildTests.spec.js @@ -60,7 +60,7 @@ describe('Schema build validation', () => { if (shouldRun(name, test.testname, runAll, runMap, skipMap)) { await testSchema(test) } else { - console.log(`----Skipping ${name}: ${test.testname}`) + console.log(`----Skipping schemaBuildTest ${name}: ${test.testname}`) } }) } diff --git a/tests/schemaSpecTests.spec.js b/tests/schemaSpecTests.spec.js index e35d77a6..4933e536 100644 --- a/tests/schemaSpecTests.spec.js +++ b/tests/schemaSpecTests.spec.js @@ -52,7 +52,7 @@ describe('Schema validation', () => { if (shouldRun(name, test.testname, runAll, runMap, skipMap)) { validateSpec(test) } else { - console.log(`----Skipping ${name}: ${test.testname}`) + console.log(`----Skipping schemaSpecTest ${name}: ${test.testname}`) } }) } diff --git a/tests/stringParserTests.spec.js b/tests/stringParserTests.spec.js index 4e9db736..1eece08b 100644 --- a/tests/stringParserTests.spec.js +++ b/tests/stringParserTests.spec.js @@ -5,16 +5,46 @@ import path from 'path' import { buildSchemas } from '../schema/init' import { SchemaSpec, SchemasSpec } from '../schema/specs' - +import { generateIssue } from '../common/issues/issues' +import { parseHedString, parseHedStrings } from '../parser/parser' import { parseTestData } from './testData/stringParserTests.data' import { shouldRun, getHedString } from './testUtilities' -// Ability to select individual tests to run -const skipMap = new Map([ - ['placeholders-in-various-places', ['string-with-placeholder-not-allowed', 'place-holder-on-extension']], -]) +const skipMap = new Map() const runAll = true -const runMap = new Map() +const runMap = new Map([['placeholders-in-various-places', ['string-with-placeholder-not-allowed']]]) + +describe('Null schema objects should cause parsing to bail', () => { + it('Should not proceed if no schema and valid string', () => { + const stringIn = 'Item, Red' + const [parsedString, errorIssues, warningIssues] = getHedString(stringIn, null) + 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) + assert.isNull(directParsed, `Parsed HED string of ${stringIn} is null for invalid string`) + assert.deepStrictEqual(directIssues.syntaxIssues, expectedIssues) + }) + + it('Should not proceed if no schema and invalid string', () => { + const stringIn = 'Item/#, Red' + const [parsedString, errorIssues, warningIssues] = getHedString(stringIn, null) + 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) + assert.isNull(directParsed, `Parsed HED string of ${stringIn} is null for invalid string`) + assert.deepStrictEqual(directIssues.syntaxIssues, expectedIssues) + }) + + it('Should not proceed if no schema and valid array of strings', () => { + const arrayIn = new Array('Item, Red', 'Blue') + const expectedIssues = [generateIssue('missingSchemaSpecification', {})] + const [directParsed, directIssues] = parseHedStrings(arrayIn, null) + assert.isNull(directParsed, `Parsed HED string of ${arrayIn} is null for invalid string`) + assert.deepStrictEqual(directIssues.syntaxIssues, expectedIssues) + }) +}) describe('Parse HED string tests', () => { const schemaMap = new Map([ @@ -87,7 +117,7 @@ describe('Parse HED string tests', () => { if (shouldRun(name, test.testname, runAll, runMap, skipMap)) { testConvert(test) } else { - console.log(`----Skipping ${name}: ${test.testname}`) + console.log(`----Skipping stringParserTest ${name}: ${test.testname}`) } }) } diff --git a/tests/tagParserTests.spec.js b/tests/tagParserTests.spec.js index 1f67a8ff..d01e5a98 100644 --- a/tests/tagParserTests.spec.js +++ b/tests/tagParserTests.spec.js @@ -36,7 +36,8 @@ describe('TagSpec converter tests using JSON tests', () => { afterAll(() => {}) - describe.skip('TagConverter tests', () => { + // TODO: Remove after refactoring of validation complete + describe.skip('TagConverter experiments', () => { it('should be able to convert', () => { const thisSchema = schemaMap.get('8.3.0') assert.isDefined(thisSchema, 'yes') @@ -120,7 +121,7 @@ describe('TagSpec converter tests using JSON tests', () => { if (shouldRun(name, test.testname, runAll, runMap, skipMap)) { hedTagTest(test) } else { - console.log(`----Skipping ${name}: ${test.testname}`) + console.log(`----Skipping tagParserTest ${name}: ${test.testname}`) } }) } diff --git a/tests/testData/stringParserTests.data.js b/tests/testData/stringParserTests.data.js index 35a3ec58..8953d8b0 100644 --- a/tests/testData/stringParserTests.data.js +++ b/tests/testData/stringParserTests.data.js @@ -1,6 +1,5 @@ import { generateIssue } from '../../common/issues/issues' -export const convertSchemaVersionsToPreload = ['8.3.0'] export const parseTestData = [ { name: 'valid-tags', @@ -273,7 +272,7 @@ export const parseTestData = [ stringIn: 'Object/#, Red', stringLong: null, stringShort: null, - errors: [generateIssue('childRequired', { tag: 'Duration' })], + errors: [generateIssue('invalidExtension', { tag: '#', parentTag: 'Object' })], warnings: [], }, { @@ -283,7 +282,7 @@ export const parseTestData = [ stringIn: 'Object/Thingie/#, Red', stringLong: null, stringShort: null, - errors: [generateIssue('childRequired', { tag: 'Duration' })], + errors: [generateIssue('invalidExtension', { parentTag: 'Object/Thingie', tag: '#' })], warnings: [], }, { @@ -318,9 +317,9 @@ export const parseTestData = [ testname: 'single-valid-tag-with-value', explanation: '"Age/5" has a valid value with no units', schemaVersion: '8.3.0', - stringIn: 'Age', - stringLong: 'Property/Agent-property/Agent-trait/Age', - stringShort: 'Age', + stringIn: 'Age/5', + stringLong: 'Property/Agent-property/Agent-trait/Age/5', + stringShort: 'Age/5', errors: [], warnings: [], }, @@ -347,4 +346,20 @@ export const parseTestData = [ }, ], }, + { + name: 'multiple-tags', + description: 'Multiple tags', + tests: [ + { + testname: 'multiple-valid-tags', + explanation: '"Age, Perform/Operate" is a valid HED string', + schemaVersion: '8.3.0', + stringIn: 'Age, Perform/Operate', + stringLong: 'Property/Agent-property/Agent-trait/Age, Action/Perform/Operate', + stringShort: 'Age, Operate', + errors: [], + warnings: [], + }, + ], + }, ] diff --git a/tests/tokenizerTests.spec.js b/tests/tokenizerTests.spec.js index 7d6b2fd8..f1b26304 100644 --- a/tests/tokenizerTests.spec.js +++ b/tests/tokenizerTests.spec.js @@ -37,7 +37,7 @@ describe('Tokenizer validation using JSON tests', () => { if (shouldRun(name, test.testname, runAll, runMap, skipMap)) { stringTokenizer(test) } else { - console.log(`----Skipping ${name}: ${test.testname}`) + console.log(`----Skipping tokenizerTest ${name}: ${test.testname}`) } }) } diff --git a/validator/event/init.js b/validator/event/init.js index a53adaab..e9441c86 100644 --- a/validator/event/init.js +++ b/validator/event/init.js @@ -20,8 +20,9 @@ const initiallyValidateHedString = function (hedString, hedSchemas, options, def hedSchemas = new Schemas(null) } let parsedString, parsingIssues - // Skip parsing if we're passed an already-parsed string. + if (hedString instanceof ParsedHedString) { + // Skip parsing if we're passed an already-parsed string. parsedString = hedString parsingIssues = { syntax: [], delimiter: [] } } else { From 48ed2f07f1f01788d065da9dae27db92a340f4f0 Mon Sep 17 00:00:00 2001 From: Kay Robbins <1189050+VisLab@users.noreply.github.com> Date: Wed, 13 Nov 2024 09:54:43 -0600 Subject: [PATCH 3/4] Removed checking of units and values from validator --- .codespellrc | 2 +- parser/parsedHedTag.js | 17 +- parser/tagConverter.js | 3 +- spec_tests/jsonTests.spec.js | 2 +- tests/event.spec.js | 10 +- tests/event2G.spec.js | 531 ----------------------------------- validator/event/validator.js | 125 +-------- 7 files changed, 16 insertions(+), 674 deletions(-) delete mode 100644 tests/event2G.spec.js diff --git a/.codespellrc b/.codespellrc index 84a3e08e..b8dcffec 100644 --- a/.codespellrc +++ b/.codespellrc @@ -1,3 +1,3 @@ [codespell] skip = datasets,.git,*.pdf,*.svg,deprecated,*.xml,*.mediawiki,*.omn,datasets -ignore-words-list = covert,hed,recuse,afterAll +ignore-words-list = covert,hed,recuse,afterAll,hertzs diff --git a/parser/parsedHedTag.js b/parser/parsedHedTag.js index 5f6e795b..06883adc 100644 --- a/parser/parsedHedTag.js +++ b/parser/parsedHedTag.js @@ -1,11 +1,9 @@ import { IssueError } from '../common/issues/issues' -import { getParentTag, getTagLevels, getTagName } from '../utils/hedStrings' +import { getTagLevels } from '../utils/hedStrings' import ParsedHedSubstring from './parsedHedSubstring' import { SchemaValueTag } from '../schema/entries' import TagConverter from './tagConverter' -import { getRegExp } from './tempRegex' -import RegexClass from '../schema/regExps' const allowedRegEx = /^[^{}\,]*$/ //TODO This is temporary until special tag handling is available. @@ -168,19 +166,6 @@ export default class ParsedHedTag extends ParsedHedSubstring { } } - /** - * Handle potential extensions - * - * @throws {IssueError} If parsing the remainder section fails. - */ - _handleExtension() { - this._extension = this._remainder - const testReg = getRegExp('nameClass') - if (!testReg.test(this._extension)) { - IssueError.generateAndThrow('invalidExtension', { tag: this.originalTag }) - } - } - /** * Nicely format this tag. * diff --git a/parser/tagConverter.js b/parser/tagConverter.js index 3515dc8b..c38a6a56 100644 --- a/parser/tagConverter.js +++ b/parser/tagConverter.js @@ -1,7 +1,6 @@ import { IssueError } from '../common/issues/issues' import { getTagSlashIndices } from '../utils/hedStrings' -import { SchemaValueTag } from '../schema/entries' -import { getRegExp } from './tempRegex' + /** * Converter from a tag specification to a schema-based tag object. */ diff --git a/spec_tests/jsonTests.spec.js b/spec_tests/jsonTests.spec.js index d6a1012d..2f9f5ace 100644 --- a/spec_tests/jsonTests.spec.js +++ b/spec_tests/jsonTests.spec.js @@ -99,7 +99,7 @@ describe('HED validation using JSON tests', () => { describe.each(testInfo)( '$error_code $name : $description', - ({ error_code, alt_codes, name, schema, warning, definitions, tests }) => { + ({ error_code, alt_codes, name, schema, definitions, tests }) => { let hedSchema let defs let expectedErrors diff --git a/tests/event.spec.js b/tests/event.spec.js index 2c9f1ada..ff937213 100644 --- a/tests/event.spec.js +++ b/tests/event.spec.js @@ -611,13 +611,13 @@ describe('HED string and event validation', () => { testStrings, expectedIssues, (validator, tag, previousTag) => { - validator.checkIfTagIsValid(tag, previousTag) + validator.checkIfTagIsValid(tag) }, { checkForWarnings: true }, ) }) - // TODO: REMOVE as these tests have been moved to tagParserTests + /*// TODO: REMOVE as these tests have been moved to tagParserTests it.skip('(REMOVE) now in tagParserTests - should have a proper unit when required', () => { const testStrings = { correctUnit: 'Time-value/3 ms', @@ -636,8 +636,8 @@ describe('HED string and event validation', () => { incorrectNonSIUnitSymbolModifier: 'Speed/100 Mkph', notRequiredNumber: 'RGB-red/0.5', notRequiredScientific: 'RGB-red/5e-1', - /*properTime: 'Clockface/08:30', - invalidTime: 'Clockface/54:54',*/ + /!*properTime: 'Clockface/08:30', + invalidTime: 'Clockface/54:54',*!/ } const expectedIssues = { correctUnit: [], @@ -698,7 +698,7 @@ describe('HED string and event validation', () => { }, { checkForWarnings: true }, ) - }) + })*/ // TODO: BIDS sidecar validation does not detect missing definitions (under definition-tests in bidsTests) it('should not contain undefined definitions', () => { diff --git a/tests/event2G.spec.js b/tests/event2G.spec.js deleted file mode 100644 index 3642d196..00000000 --- a/tests/event2G.spec.js +++ /dev/null @@ -1,531 +0,0 @@ -import chai from 'chai' -const assert = chai.assert -import { beforeAll, describe, it } from '@jest/globals' - -import * as hed from '../validator/event' -import { buildSchemas } from '../schema/init' -import { parseHedString } from '../parser/parser' -import ParsedHedTag from '../parser/parsedHedTag' -import { generateIssue } from '../common/issues/issues' -import { SchemaSpec, SchemasSpec } from '../schema/specs' -import { Schemas } from '../schema/containers' - -describe.skip('HED string and event validation', () => { - /** - * Validation base function. - * - * @param {Schemas} hedSchemas The HED schema collection used for testing. - * @param {typeof HedValidator} ValidatorClass A subclass of {@link HedValidator} to use for validation. - * @param {Object} testStrings A mapping of test strings. - * @param {Object} expectedIssues The expected issues for each test string. - * @param {function(HedValidator): void} testFunction A test-specific function that executes the required validation check. - * @param {Object?} testOptions Any needed custom options for the validator. - */ - const validatorBase = function ( - hedSchemas, - ValidatorClass, - testStrings, - expectedIssues, - testFunction, - testOptions = {}, - ) { - for (const [testStringKey, testString] of Object.entries(testStrings)) { - assert.property(expectedIssues, testStringKey, testStringKey + ' is not in expectedIssues') - const [parsedTestString, parsingIssues] = parseHedString(testString, hedSchemas) - const validator = new ValidatorClass(parsedTestString, hedSchemas, testOptions) - const flattenedParsingIssues = Object.values(parsingIssues).flat() - if (flattenedParsingIssues.length === 0) { - testFunction(validator) - } - const issues = [].concat(flattenedParsingIssues, validator.issues) - assert.sameDeepMembers(issues, expectedIssues[testStringKey], testString) - } - } - - describe.skip('HED-2G validation', () => { - describe('Later HED-2G schemas', () => { - const hedSchemaFile = 'tests/data/HED7.1.1.xml' - let hedSchemas - - beforeAll(async () => { - const spec1 = new SchemaSpec('', '7.1.1', '', hedSchemaFile) - const specs = new SchemasSpec().addSchemaSpec(spec1) - hedSchemas = await buildSchemas(specs) - }) - - /** - * HED 2 semantic validation base function. - * - * This base function uses the HED 2-specific {@link Hed2Validator} validator class. - * - * @param {Object} testStrings A mapping of test strings. - * @param {Object} expectedIssues The expected issues for each test string. - * @param {function(HedValidator): void} testFunction A test-specific function that executes the required validation check. - * @param {Object?} testOptions Any needed custom options for the validator. - */ - const validatorSemanticBase = function (testStrings, expectedIssues, testFunction, testOptions = {}) { - validatorBase(hedSchemas, Hed2Validator, testStrings, expectedIssues, testFunction, testOptions) - } - - describe('Full HED Strings', () => { - const validatorSemantic = validatorSemanticBase - - // TODO: Rewrite as HED 3 test - it.skip('should not validate strings with extensions that are valid node names', () => { - const testStrings = { - // Event/Duration/20 cm is an obviously invalid tag that should not be caught due to the first error. - red: 'Attribute/Red, Event/Duration/20 cm', - redAndBlue: 'Attribute/Red, Attribute/Blue, Event/Duration/20 cm', - } - const expectedIssues = { - red: [ - generateIssue('invalidParentNode', { - tag: 'Red', - parentTag: 'Attribute/Visual/Color/Red', - }), - ], - redAndBlue: [ - generateIssue('invalidParentNode', { - tag: 'Red', - parentTag: 'Attribute/Visual/Color/Red', - }), - generateIssue('invalidParentNode', { - tag: 'Blue', - parentTag: 'Attribute/Visual/Color/Blue', - }), - ], - } - // This is a no-op function since this is checked during string parsing. - return validatorSemantic( - testStrings, - expectedIssues, - // eslint-disable-next-line no-unused-vars - (validator) => {}, - ) - }) - }) - - describe('Individual HED Tags', () => { - /** - * HED 2 individual tag semantic validation base function. - * - * @param {Object} testStrings A mapping of test strings. - * @param {Object} expectedIssues The expected issues for each test string. - * @param {function(HedValidator, ParsedHedTag, ParsedHedTag): void} testFunction A test-specific function that executes the required validation check. - * @param {Object?} testOptions Any needed custom options for the validator. - */ - const validatorSemantic = function (testStrings, expectedIssues, testFunction, testOptions) { - return validatorSemanticBase( - testStrings, - expectedIssues, - (validator) => { - let previousTag = new ParsedHedTag('', '', [0, 0], validator.hedSchemas) - for (const tag of validator.parsedString.tags) { - testFunction(validator, tag, previousTag) - previousTag = tag - } - }, - testOptions, - ) - } - //TODO: Rewrite for HED-3 - it('should exist in the schema or be an allowed extension', () => { - const testStrings = { - takesValue: 'Event/Duration/3 ms', - full: 'Attribute/Object side/Left', - extensionAllowed: 'Item/Object/Person/Driver', - leafExtension: 'Event/Category/Initial context/Something', - nonExtensionAllowed: 'Event/Nonsense', - illegalComma: 'Event/Label/This is a label,This/Is/A/Tag', - placeholder: 'Item/Object/#', - } - const expectedIssues = { - takesValue: [], - full: [], - extensionAllowed: [generateIssue('extension', { tag: testStrings.extensionAllowed })], - leafExtension: [generateIssue('invalidTag', { tag: testStrings.leafExtension })], - nonExtensionAllowed: [ - generateIssue('invalidTag', { - tag: testStrings.nonExtensionAllowed, - }), - ], - illegalComma: [ - generateIssue('extraCommaOrInvalid', { - previousTag: 'Event/Label/This is a label', - tag: 'This/Is/A/Tag', - }), - ], - placeholder: [ - generateIssue('invalidTag', { - tag: testStrings.placeholder, - }), - ], - } - return validatorSemantic( - testStrings, - expectedIssues, - (validator, tag, previousTag) => { - //validator.checkIfTagIsValid(tag, previousTag) - return true - }, - { checkForWarnings: true }, - ) - }) - - it('should have a child when required', () => { - const testStrings = { - hasChild: 'Event/Category/Experimental stimulus', - missingChild: 'Event/Category', - } - const expectedIssues = { - hasChild: [], - missingChild: [generateIssue('childRequired', { tag: testStrings.missingChild })], - } - return validatorSemantic( - testStrings, - expectedIssues, - // eslint-disable-next-line no-unused-vars - (validator, tag, previousTag) => { - validator.checkIfTagRequiresChild(tag) - }, - { checkForWarnings: true }, - ) - }) - - it('should have a proper unit when required', () => { - const testStrings = { - correctUnit: 'Event/Duration/3 ms', - correctUnitScientific: 'Event/Duration/3.5e1 ms', - correctSingularUnit: 'Event/Duration/1 millisecond', - correctPluralUnit: 'Event/Duration/3 milliseconds', - correctNoPluralUnit: 'Attribute/Temporal rate/3 hertz', - correctPrefixUnit: 'Participant/Effect/Cognitive/Reward/$19.69', - correctNonSymbolCapitalizedUnit: 'Event/Duration/3 MilliSeconds', - correctSymbolCapitalizedUnit: 'Attribute/Temporal rate/3 kHz', - missingRequiredUnit: 'Event/Duration/3', - incorrectUnit: 'Event/Duration/3 cm', - incorrectNonNumericValue: 'Event/Duration/A ms', - incorrectPluralUnit: 'Attribute/Temporal rate/3 hertzs', - incorrectSymbolCapitalizedUnit: 'Attribute/Temporal rate/3 hz', - incorrectSymbolCapitalizedUnitModifier: 'Attribute/Temporal rate/3 KHz', - incorrectNonSIUnitModifier: 'Event/Duration/1 millihour', - incorrectNonSIUnitSymbolModifier: 'Attribute/Path/Velocity/100 Mkph', - notRequiredNumber: 'Attribute/Visual/Color/Red/0.5', - notRequiredScientific: 'Attribute/Visual/Color/Red/5e-1', - properTime: 'Item/2D shape/Clock face/08:30', - invalidTime: 'Item/2D shape/Clock face/54:54', - } - const legalTimeUnits = ['s', 'second', 'day', 'minute', 'hour'] - const legalFrequencyUnits = ['Hz', 'hertz'] - const legalSpeedUnits = ['m-per-s', 'kph', 'mph'] - const expectedIssues = { - correctUnit: [], - correctUnitScientific: [], - correctSingularUnit: [], - correctPluralUnit: [], - correctNoPluralUnit: [], - correctPrefixUnit: [], - correctNonSymbolCapitalizedUnit: [], - correctSymbolCapitalizedUnit: [], - missingRequiredUnit: [ - generateIssue('unitClassDefaultUsed', { - defaultUnit: 's', - tag: testStrings.missingRequiredUnit, - }), - ], - incorrectUnit: [ - generateIssue('unitClassInvalidUnit', { - tag: testStrings.incorrectUnit, - unitClassUnits: legalTimeUnits.sort().join(','), - }), - ], - incorrectNonNumericValue: [ - generateIssue('invalidValue', { - tag: testStrings.incorrectNonNumericValue, - }), - ], - incorrectPluralUnit: [ - generateIssue('unitClassInvalidUnit', { - tag: testStrings.incorrectPluralUnit, - unitClassUnits: legalFrequencyUnits.sort().join(','), - }), - ], - incorrectSymbolCapitalizedUnit: [ - generateIssue('unitClassInvalidUnit', { - tag: testStrings.incorrectSymbolCapitalizedUnit, - unitClassUnits: legalFrequencyUnits.sort().join(','), - }), - ], - incorrectSymbolCapitalizedUnitModifier: [ - generateIssue('unitClassInvalidUnit', { - tag: testStrings.incorrectSymbolCapitalizedUnitModifier, - unitClassUnits: legalFrequencyUnits.sort().join(','), - }), - ], - incorrectNonSIUnitModifier: [ - generateIssue('unitClassInvalidUnit', { - tag: testStrings.incorrectNonSIUnitModifier, - unitClassUnits: legalTimeUnits.sort().join(','), - }), - ], - incorrectNonSIUnitSymbolModifier: [ - generateIssue('unitClassInvalidUnit', { - tag: testStrings.incorrectNonSIUnitSymbolModifier, - unitClassUnits: legalSpeedUnits.sort().join(','), - }), - ], - notRequiredNumber: [], - notRequiredScientific: [], - properTime: [], - invalidTime: [ - generateIssue('invalidValue', { - tag: testStrings.invalidTime, - }), - ], - } - return validatorSemantic( - testStrings, - expectedIssues, - // eslint-disable-next-line no-unused-vars - (validator, tag, previousTag) => { - validator.checkIfTagUnitClassUnitsAreValid(tag) - }, - { checkForWarnings: true }, - ) - }) - }) - - //TODO: Replace with HED-3 - describe('HED Tag Levels', () => { - /** - * HED 2 Tag level semantic validation base function. - * - * @param {Object} testStrings A mapping of test strings. - * @param {Object} expectedIssues The expected issues for each test string. - * @param {function(HedValidator, ParsedHedSubstring[]): void} testFunction A test-specific function that executes the required validation check. - * @param {Object?} testOptions Any needed custom options for the validator. - */ - const validatorSemantic = function (testStrings, expectedIssues, testFunction, testOptions = {}) { - return validatorSemanticBase( - testStrings, - expectedIssues, - (validator) => { - for (const tagGroup of validator.parsedString.tagGroups) { - for (const subGroup of tagGroup.subGroupArrayIterator()) { - testFunction(validator, subGroup) - } - } - testFunction(validator, validator.parsedString.parseTree) - }, - testOptions, - ) - } - - it('should not have multiple copies of a unique tag', () => { - const testStrings = { - legal: - 'Event/Description/Rail vehicles,Item/Object/Vehicle/Train,(Item/Object/Vehicle/Train,Event/Category/Experimental stimulus)', - multipleDesc: - 'Event/Description/Rail vehicles,Event/Description/Locomotive-pulled or multiple units,Item/Object/Vehicle/Train,(Item/Object/Vehicle/Train,Event/Category/Experimental stimulus)', - } - const expectedIssues = { - legal: [], - multipleDesc: [generateIssue('multipleUniqueTags', { tag: 'event/description' })], - } - return validatorSemantic(testStrings, expectedIssues, (validator, tagLevel) => { - validator.checkForMultipleUniqueTags(tagLevel) - }) - }) - }) - - describe('Top-level Tags', () => { - const validatorSemantic = validatorSemanticBase - - it('should include all required tags', () => { - const testStrings = { - complete: - 'Event/Label/Bus,Event/Category/Experimental stimulus,Event/Description/Shown a picture of a bus,Item/Object/Vehicle/Bus', - missingLabel: - 'Event/Category/Experimental stimulus,Event/Description/Shown a picture of a bus,Item/Object/Vehicle/Bus', - missingCategory: 'Event/Label/Bus,Event/Description/Shown a picture of a bus,Item/Object/Vehicle/Bus', - missingDescription: 'Event/Label/Bus,Event/Category/Experimental stimulus,Item/Object/Vehicle/Bus', - missingAllRequired: 'Item/Object/Vehicle/Bus', - } - const expectedIssues = { - complete: [], - missingLabel: [ - generateIssue('requiredPrefixMissing', { - tagPrefix: 'event/label', - }), - ], - missingCategory: [ - generateIssue('requiredPrefixMissing', { - tagPrefix: 'event/category', - }), - ], - missingDescription: [ - generateIssue('requiredPrefixMissing', { - tagPrefix: 'event/description', - }), - ], - missingAllRequired: [ - generateIssue('requiredPrefixMissing', { - tagPrefix: 'event/label', - }), - generateIssue('requiredPrefixMissing', { - tagPrefix: 'event/category', - }), - generateIssue('requiredPrefixMissing', { - tagPrefix: 'event/description', - }), - ], - } - return validatorSemantic( - testStrings, - expectedIssues, - (validator) => { - validator.checkForRequiredTags() - }, - { checkForWarnings: true }, - ) - }) - }) - }) - - describe('Pre-v7.1.0 HED schemas', () => { - const hedSchemaFile = 'tests/data/HED7.0.4.xml' - let hedSchemas - - beforeAll(async () => { - const spec2 = new SchemaSpec('', '7.0.4', '', hedSchemaFile) - const specs = new SchemasSpec().addSchemaSpec(spec2) - hedSchemas = await buildSchemas(specs) - }) - - /** - * HED 2 semantic validation base function. - * - * This base function uses the HED 2-specific {@link Hed2Validator} validator class. - * - * @param {Object} testStrings A mapping of test strings. - * @param {Object} expectedIssues The expected issues for each test string. - * @param {function(HedValidator): void} testFunction A test-specific function that executes the required validation check. - * @param {Object?} testOptions Any needed custom options for the validator. - */ - const validatorSemanticBase = function (testStrings, expectedIssues, testFunction, testOptions = {}) { - validatorBase(hedSchemas, Hed2Validator, testStrings, expectedIssues, testFunction, testOptions) - } - - describe('Individual HED Tags', () => { - /** - * HED 2 individual tag semantic validation base function. - * - * @param {Object} testStrings A mapping of test strings. - * @param {Object} expectedIssues The expected issues for each test string. - * @param {function(HedValidator, ParsedHedTag, ParsedHedTag): void} testFunction A test-specific function that executes the required validation check. - * @param {Object?} testOptions Any needed custom options for the validator. - */ - const validatorSemantic = function (testStrings, expectedIssues, testFunction, testOptions) { - return validatorSemanticBase( - testStrings, - expectedIssues, - (validator) => { - let previousTag = new ParsedHedTag('', '', [0, 0], validator.hedSchemas) - for (const tag of validator.parsedString.tags) { - testFunction(validator, tag, previousTag) - previousTag = tag - } - }, - testOptions, - ) - } - - it('should have a proper unit when required', () => { - const testStrings = { - correctUnit: 'Event/Duration/3 ms', - correctUnitWord: 'Event/Duration/3 milliseconds', - correctUnitScientific: 'Event/Duration/3.5e1 ms', - missingRequiredUnit: 'Event/Duration/3', - incorrectUnit: 'Event/Duration/3 cm', - incorrectNonNumericValue: 'Event/Duration/A ms', - incorrectUnitWord: 'Event/Duration/3 nanoseconds', - incorrectModifier: 'Event/Duration/3 ns', - notRequiredNumber: 'Attribute/Visual/Color/Red/0.5', - notRequiredScientific: 'Attribute/Visual/Color/Red/5e-1', - properTime: 'Item/2D shape/Clock face/08:30', - invalidTime: 'Item/2D shape/Clock face/54:54', - } - const legalTimeUnits = [ - 's', - 'second', - 'seconds', - 'centiseconds', - 'centisecond', - 'cs', - 'hour:min', - 'day', - 'days', - 'ms', - 'milliseconds', - 'millisecond', - 'minute', - 'minutes', - 'hour', - 'hours', - ] - const expectedIssues = { - correctUnit: [], - correctUnitWord: [], - correctUnitScientific: [], - missingRequiredUnit: [ - generateIssue('unitClassDefaultUsed', { - defaultUnit: 's', - tag: testStrings.missingRequiredUnit, - }), - ], - incorrectUnit: [ - generateIssue('unitClassInvalidUnit', { - tag: testStrings.incorrectUnit, - unitClassUnits: legalTimeUnits.sort().join(','), - }), - ], - incorrectNonNumericValue: [ - generateIssue('invalidValue', { - tag: testStrings.incorrectNonNumericValue, - }), - ], - incorrectUnitWord: [ - generateIssue('unitClassInvalidUnit', { - tag: testStrings.incorrectUnitWord, - unitClassUnits: legalTimeUnits.sort().join(','), - }), - ], - incorrectModifier: [ - generateIssue('unitClassInvalidUnit', { - tag: testStrings.incorrectModifier, - unitClassUnits: legalTimeUnits.sort().join(','), - }), - ], - notRequiredNumber: [], - notRequiredScientific: [], - properTime: [], - invalidTime: [ - generateIssue('invalidValue', { - tag: testStrings.invalidTime, - }), - ], - } - return validatorSemantic( - testStrings, - expectedIssues, - // eslint-disable-next-line no-unused-vars - (validator, tag, previousTag) => { - validator.checkIfTagUnitClassUnitsAreValid(tag) - }, - { checkForWarnings: true }, - ) - }) - }) - }) - }) -}) diff --git a/validator/event/validator.js b/validator/event/validator.js index 016e8853..82f49267 100644 --- a/validator/event/validator.js +++ b/validator/event/validator.js @@ -1,7 +1,7 @@ import differenceWith from 'lodash/differenceWith' import isEqual from 'lodash/isEqual' -import { IssueError, generateIssue, Issue } from '../../common/issues/issues' +import { IssueError, generateIssue } from '../../common/issues/issues' import ParsedHedGroup from '../../parser/parsedHedGroup' import ParsedHedTag from '../../parser/parsedHedTag' import ParsedHedColumnSplice from '../../parser/parsedHedColumnSplice' @@ -12,7 +12,6 @@ import { getCharacterCount, isNumber } from '../../utils/string' const tagGroupType = 'tagGroup' const topLevelTagGroupType = 'topLevelTagGroup' -const NAME_CLASS_REGEX = /^[\w\-\u0080-\uFFFF]+$/ const uniqueType = 'unique' const requiredType = 'required' const specialTags = require('../../data/json/specialTags.json') @@ -87,21 +86,19 @@ export default class HedValidator { * Validate the individual HED tags in a parsed HED string object. */ validateIndividualHedTags() { - let previousTag = null for (const tag of this.parsedString.tags) { - this.validateIndividualHedTag(tag, previousTag) - previousTag = tag + this.validateIndividualHedTag(tag) } } /** * Validate an individual HED tag. */ - validateIndividualHedTag(tag, previousTag) { + validateIndividualHedTag(tag) { //this.checkIfTagIsValid(tag, previousTag) - this.checkIfTagUnitClassUnitsAreValid(tag) + //this.checkIfTagUnitClassUnitsAreValid(tag) if (!this.options.isEventLevel) { - this.checkValueTagSyntax(tag) + //this.checkValueTagSyntax(tag) } if (this.definitions !== null) { this.checkForMissingDefinitions(tag, 'Def') @@ -128,9 +125,7 @@ export default class HedValidator { * Validate a HED tag level. */ validateHedTagLevel(tagList) { - if (this.hedSchemas.generation > 0) { - this.checkForMultipleUniqueTags(tagList) - } + this.checkForMultipleUniqueTags(tagList) this.checkForDuplicateTags(tagList) } @@ -253,7 +248,7 @@ export default class HedValidator { /** * Check if an individual HED tag is in the schema or is an allowed extension. */ - checkIfTagIsValid(tag, previousTag) { + checkIfTagIsValid(tag) { if (tag.existsInSchema || tag.takesValue) { return } @@ -261,61 +256,6 @@ export default class HedValidator { // This is an allowed extension. this.pushIssue('extension', { tag: tag }) } - // - // if (this.options.expectValuePlaceholderString && getCharacterCount(tag.formattedTag, '#') === 1) { - // const valueTag = replaceTagNameWithPound(tag.formattedTag) - // if (getCharacterCount(valueTag, '#') === 1) { - // // Ending placeholder was replaced with itself. - // this.pushIssue('invalidPlaceholder', { - // tag: tag, - // }) - // } /* else { - // Handled in checkPlaceholderTagSyntax(). - // } */ - // return - // } - - // const isExtensionAllowedTag = tag.allowsExtensions - // if (!isExtensionAllowedTag && previousTag?.takesValue) { - // // This tag isn't an allowed extension, but the previous tag takes a value. - // // This is likely caused by an extraneous comma. - // this.pushIssue('extraCommaOrInvalid', { - // tag: tag, - // previousTag: previousTag, - // }) - // } else if (!isExtensionAllowedTag) { - // // This is not a valid tag. - // this.pushIssue('invalidTag', { tag: tag }) - // } else if (!NAME_CLASS_REGEX.test(tag._remainder)) { - // this.pushIssue('invalidTag', { tag: tag }) - // } else if (!this.options.isEventLevel && this.options.checkForWarnings) { - // // This is an allowed extension. - // this.pushIssue('extension', { tag: tag }) - // } - } - - /** - * Check that the unit is valid for the tag's unit class. - * - * @param {ParsedHedTag} tag A HED tag. - */ - checkIfTagUnitClassUnitsAreValid(tag) { - if (!tag.takesValue || !tag.hasUnitClass || tag._remainder) { - return - } - const [foundUnit, validUnit, value] = this.validateUnits(tag) - if (!validUnit) { - const tagUnitClassUnits = Array.from(tag.validUnits).map((unit) => unit.name) - this.pushIssue('unitClassInvalidUnit', { - tag: tag, - unitClassUnits: tagUnitClassUnits.sort().join(','), - }) - } else { - const validValue = this.validateValue(value, true) - if (!validValue) { - this.pushIssue('invalidValue', { tag: tag }) - } - } } /** @@ -428,23 +368,6 @@ export default class HedValidator { } } - /** - * Check the syntax of tag values. - * - * @param {ParsedHedTag} tag A HED tag. - */ - checkValueTagSyntax(tag) { - if (tag.takesValue && !tag.hasUnitClass) { - const isValidValue = this.validateValue( - tag.formattedTagName, - tag.takesValueTag.hasAttributeName('isNumeric'), // Always false - ) - if (!isValidValue) { - this.pushIssue('invalidValue', { tag: tag }) - } - } - } - // TODO: This can be simplified. /** * Validate a unit and strip it from the value. @@ -500,26 +423,6 @@ export default class HedValidator { return [!noUnitFound, false, originalTagUnitValue] } - /** - * Determine if a stripped value is valid. - * - * @param {string} value The stripped value. - * @param {boolean} isNumeric Whether the tag is numeric. - * @returns {boolean} Whether the stripped value is valid. - * @todo This function is a placeholder until support for value classes is implemented. - */ - validateValue(value, isNumeric) { - if (value === '#') { - return true - } - // TODO: Replace with full value class-based implementation. - if (isNumeric) { - return isNumber(value) - } - // TODO: Placeholder. - return true - } - /** * Check full-string Definition syntax. */ @@ -736,18 +639,4 @@ export default class HedValidator { } this.issues.push(generateIssue(internalCode, parameters)) } - - /** - * Generate a new issue object and push it to the end of the issues array. - * - * @param {string} internalCode The internal error code. - * @param {Object} parameters The error string parameters. - */ - pushIssue(internalCode, parameters) { - const tsvLine = this.parsedString.tsvLine ?? this.parsedString.tsvLines - if (tsvLine) { - parameters.tsvLine = tsvLine - } - this.issues.push(generateIssue(internalCode, parameters)) - } } From 928f626ec8aeacf0bf428f68e0329a7d85de4b5f Mon Sep 17 00:00:00 2001 From: Kay Robbins <1189050+VisLab@users.noreply.github.com> Date: Wed, 13 Nov 2024 11:06:34 -0600 Subject: [PATCH 4/4] Eliminated various redundancies in validator section --- tests/event.spec.js | 12 ++-- validator/event/init.js | 20 +----- validator/event/validator.js | 132 +++++++---------------------------- 3 files changed, 33 insertions(+), 131 deletions(-) diff --git a/tests/event.spec.js b/tests/event.spec.js index ff937213..bfb70c49 100644 --- a/tests/event.spec.js +++ b/tests/event.spec.js @@ -571,7 +571,7 @@ describe('HED string and event validation', () => { } // TODO: Already covered in stringParserTests -- units still to move down. - it.skip('(REMOVE) should exist in the schema or be an allowed extension', () => { + /*it.skip('(REMOVE) should exist in the schema or be an allowed extension', () => { const testStrings = { takesValue: 'Time-value/3 ms', full: 'Left-side-of', @@ -594,12 +594,12 @@ describe('HED string and event validation', () => { ], illegalComma: [ generateIssue('invalidTag', { tag: 'This/Is/A/Tag' }), - /* Intentionally not thrown (validation ends at parsing stage) + /!* Intentionally not thrown (validation ends at parsing stage) generateIssue('extraCommaOrInvalid', { previousTag: 'Label/This_is_a_label', tag: 'This/Is/A/Tag', }), - */ + *!/ ], placeholder: [ generateIssue('invalidTag', { @@ -615,7 +615,7 @@ describe('HED string and event validation', () => { }, { checkForWarnings: true }, ) - }) + })*/ /*// TODO: REMOVE as these tests have been moved to tagParserTests it.skip('(REMOVE) now in tagParserTests - should have a proper unit when required', () => { @@ -968,7 +968,7 @@ describe('HED string and event validation', () => { ], } return validatorSemantic(testStrings, expectedIssues, (validator) => { - validator.checkForInvalidTopLevelTags() + validator.validateTopLevelTags() }) }) }) @@ -1032,7 +1032,7 @@ describe('HED string and event validation', () => { ], } return validatorSemantic(testStrings, expectedIssues, (validator) => { - validator.checkForInvalidTopLevelTagGroupTags() + validator.validateTopLevelTagGroups() }) }) }) diff --git a/validator/event/init.js b/validator/event/init.js index e9441c86..479e71b0 100644 --- a/validator/event/init.js +++ b/validator/event/init.js @@ -1,9 +1,6 @@ import { parseHedString } from '../../parser/parser' -import ParsedHedString from '../../parser/parsedHedString' - import HedValidator from './validator' import { Issue } from '../../common/issues/issues' -import { Schemas } from '../../schema/containers' /** * Perform initial validation on a HED string and parse it so further validation can be performed. @@ -15,23 +12,9 @@ import { Schemas } from '../../schema/containers' * @returns {[ParsedHedString, Issue[], HedValidator]} The parsed HED string, the actual HED schema collection to use, any issues found, and whether to perform semantic validation. */ const initiallyValidateHedString = function (hedString, hedSchemas, options, definitions = null) { - const doSemanticValidation = hedSchemas instanceof Schemas - if (!doSemanticValidation) { - hedSchemas = new Schemas(null) - } - let parsedString, parsingIssues - - if (hedString instanceof ParsedHedString) { - // Skip parsing if we're passed an already-parsed string. - parsedString = hedString - parsingIssues = { syntax: [], delimiter: [] } - } else { - ;[parsedString, parsingIssues] = parseHedString(hedString, hedSchemas) - } + const [parsedString, parsingIssues] = parseHedString(hedString, hedSchemas) if (parsedString === null) { return [null, [].concat(...Object.values(parsingIssues)), null] - } else if (parsingIssues.syntax.length > 0) { - hedSchemas = new Schemas(null) } const hedValidator = new HedValidator(parsedString, hedSchemas, definitions, options) const allParsingIssues = [].concat(...Object.values(parsingIssues)) @@ -48,6 +31,7 @@ const initiallyValidateHedString = function (hedString, hedSchemas, options, def * @returns {[boolean, Issue[]]} Whether the HED string is valid and any issues found. * @deprecated */ + export const validateHedString = function (hedString, hedSchemas, ...args) { let settings const settingsArg = args[0] diff --git a/validator/event/validator.js b/validator/event/validator.js index 82f49267..b5c65809 100644 --- a/validator/event/validator.js +++ b/validator/event/validator.js @@ -153,17 +153,32 @@ export default class HedValidator { * Validate the top-level HED tags in a parsed HED string. */ validateTopLevelTags() { - if (this.options.checkForWarnings) { - this.checkForRequiredTags() + for (const topLevelTag of this.parsedString.topLevelTags) { + if ( + !hedStringIsAGroup(topLevelTag.formattedTag) && + (topLevelTag.hasAttribute(tagGroupType) || topLevelTag.parentHasAttribute(tagGroupType)) + ) { + this.pushIssue('invalidTopLevelTag', { + tag: topLevelTag, + }) + } } - this.checkForInvalidTopLevelTags() } /** * Validate the top-level HED tag groups in a parsed HED string. */ validateTopLevelTagGroups() { - this.checkForInvalidTopLevelTagGroupTags() + for (const tag of this.parsedString.tags) { + if (!tag.hasAttribute(topLevelTagGroupType) && !tag.parentHasAttribute(topLevelTagGroupType)) { + continue + } + if (!this.parsedString.topLevelTagGroups.some((topLevelTagGroup) => topLevelTagGroup.includes(tag))) { + this.pushIssue('invalidTopLevelTagGroupTag', { + tag: tag, + }) + } + } } /** @@ -202,6 +217,7 @@ export default class HedValidator { }) } + // TODO: Doesn't seem to be working correctly /** * Check for multiple instances of a unique tag. */ @@ -216,19 +232,6 @@ export default class HedValidator { }) } - /** - * Check that all required tags are present. - */ - checkForRequiredTags() { - this._checkForTagAttribute(requiredType, (requiredTagPrefix) => { - if (!this.parsedString.topLevelTags.some((tag) => tag.formattedTag.startsWith(requiredTagPrefix))) { - this.pushIssue('requiredPrefixMissing', { - tagPrefix: requiredTagPrefix, - }) - } - }) - } - /** * Validation check based on a tag attribute. * @@ -245,9 +248,10 @@ export default class HedValidator { } } - /** + // TODO: Checking for extensions is being removed temporarily -- well have to add it back eventually. + /* /!** * Check if an individual HED tag is in the schema or is an allowed extension. - */ + *!/ checkIfTagIsValid(tag) { if (tag.existsInSchema || tag.takesValue) { return @@ -256,7 +260,7 @@ export default class HedValidator { // This is an allowed extension. this.pushIssue('extension', { tag: tag }) } - } + }*/ /** * Check basic placeholder tag syntax. @@ -264,6 +268,7 @@ export default class HedValidator { * @param {ParsedHedTag} tag A HED tag. */ checkPlaceholderTagSyntax(tag) { + // TODO: Refactor or eliminate after column splicing completed const placeholderCount = getCharacterCount(tag.formattedTag, '#') if (placeholderCount === 1) { const valueTag = replaceTagNameWithPound(tag.formattedTag) @@ -368,61 +373,6 @@ export default class HedValidator { } } - // TODO: This can be simplified. - /** - * Validate a unit and strip it from the value. - * - * @param {ParsedHedTag} tag A HED tag. - * @returns {[boolean, boolean, string]} Whether a unit was found, whether it was valid, and the stripped value. - */ - static validateUnits(tag) { - const originalTagUnitValue = tag.originalTagName - const tagUnitClassUnits = tag.validUnits - const validUnits = tag.schema.entries.allUnits - const unitStrings = Array.from(validUnits.keys()) - unitStrings.sort((first, second) => { - return second.length - first.length - }) - let actualUnit = getTagName(originalTagUnitValue, ' ') - let noUnitFound = false - if (actualUnit === originalTagUnitValue) { - actualUnit = '' - noUnitFound = true - } - let foundUnit, foundWrongCaseUnit, strippedValue - for (const unitName of unitStrings) { - const unit = validUnits.get(unitName) - const isPrefixUnit = unit.isPrefixUnit - const isUnitSymbol = unit.isUnitSymbol - for (const derivativeUnit of unit.derivativeUnits()) { - if (isPrefixUnit && originalTagUnitValue.startsWith(derivativeUnit)) { - foundUnit = true - noUnitFound = false - strippedValue = originalTagUnitValue.substring(derivativeUnit.length).trim() - } - if (actualUnit === derivativeUnit) { - foundUnit = true - strippedValue = getParentTag(originalTagUnitValue, ' ') - } else if (actualUnit.toLowerCase() === derivativeUnit.toLowerCase()) { - if (isUnitSymbol) { - foundWrongCaseUnit = true - } else { - foundUnit = true - } - strippedValue = getParentTag(originalTagUnitValue, ' ') - } - if (foundUnit) { - const unitIsValid = tagUnitClassUnits.has(unit) - return [true, unitIsValid, strippedValue] - } - } - if (foundWrongCaseUnit) { - return [true, false, strippedValue] - } - } - return [!noUnitFound, false, originalTagUnitValue] - } - /** * Check full-string Definition syntax. */ @@ -594,38 +544,6 @@ export default class HedValidator { } } - /** - * Check for invalid top-level tags. - */ - checkForInvalidTopLevelTags() { - for (const topLevelTag of this.parsedString.topLevelTags) { - if ( - !hedStringIsAGroup(topLevelTag.formattedTag) && - (topLevelTag.hasAttribute(tagGroupType) || topLevelTag.parentHasAttribute(tagGroupType)) - ) { - this.pushIssue('invalidTopLevelTag', { - tag: topLevelTag, - }) - } - } - } - - /** - * Check for tags marked with the topLevelTagGroup attribute that are not in top-level tag groups. - */ - checkForInvalidTopLevelTagGroupTags() { - for (const tag of this.parsedString.tags) { - if (!tag.hasAttribute(topLevelTagGroupType) && !tag.parentHasAttribute(topLevelTagGroupType)) { - continue - } - if (!this.parsedString.topLevelTagGroups.some((topLevelTagGroup) => topLevelTagGroup.includes(tag))) { - this.pushIssue('invalidTopLevelTagGroupTag', { - tag: tag, - }) - } - } - } - /** * Generate a new issue object and push it to the end of the issues array. *