diff --git a/CHANGELOG.md b/CHANGELOG.md index 0981892b..739b5d1f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -61,6 +61,13 @@ testSpecs`. (SCHEMA_LOAD_FAILED) instead of an internal error. `core-js-pure` is no longer a dependency, so building a schema in a browser needs the native `Set` methods (2024 browsers and later). +- A `unitClass` or `valueClass` name the schema does not define is dropped + when the schema is built, as in hed-typescript, instead of being kept as an + undefined entry in the tag's class list. +- Value validation reads a tag's value classes from its value-taking schema + tag (`SchemaTag.valueClasses`) instead of the raw `valueClass` attribute, + which is no longer kept among the tag's value attributes (as `unitClass` + already was not). ### Fixed @@ -153,7 +160,9 @@ testSpecs`. - in HED 8.0.0, whose time units have no `conversionFactor`, a timeline `Delay` or `Duration` in `s`, `second`, `seconds` or no unit converts as is; `ms` or `minute` is still TEMPORAL_TAG_ERROR. + The dead `ReservedChecker.checkNonTopGroups` is removed (hed-tests e2753fb). + - A column name repeated in the header of a TSV file is TABULAR_INVALID, one error per repeated name, and the file is not validated further (specification 3.2.10; #896; hed-tests @@ -213,6 +222,7 @@ testSpecs`. `tests/otherTests/unmergedSchemas.spec.js` compares both forms of every partnered hed-tests library (#676). + - Value characters follow the specification's `character_sets.json`, copied verbatim to `src/data/json/characterSets.json` (the new `character_sets.yml` workflow checks the copy); `classRegex.json` is gone. @@ -239,6 +249,9 @@ testSpecs`. in its I/O error message; the file name was blank. Definition and placeholder issues found in a sidecar now carry its file path, as other sidecar issues do. +- Comparing two declarations of a schema entry joined attribute values with + `|`, so `["a|b", "c"]` matched `["a", "b|c"]`; values are now compared item + by item. ### Dependencies diff --git a/src/issues/data.js b/src/issues/data.js index 5a25b186..cf31001b 100644 --- a/src/issues/data.js +++ b/src/issues/data.js @@ -30,12 +30,12 @@ const issueData = { deprecatedTag: { hedCode: 'ELEMENT_DEPRECATED', level: 'warning', - message: issueMessageTemplate`Tags "${'tags'} in "${'string'} are deprecated. Please see tag description for instructions on replacement.".`, + message: issueMessageTemplate`Tags "${'tags'}" in "${'string'}" are deprecated. Please see tag description for instructions on replacement.`, }, duplicateTag: { hedCode: 'TAG_EXPRESSION_REPEATED', level: 'error', - message: issueMessageTemplate`Duplicate tags - "${'tags'} in "${'string'}".`, + message: issueMessageTemplate`Duplicate tags - "${'tags'}" in "${'string'}".`, }, extendedTag: { hedCode: 'TAG_EXTENDED', @@ -172,17 +172,17 @@ const issueData = { invalidDefinitionGroupStructure: { hedCode: 'DEFINITION_INVALID', level: 'error', - message: issueMessageTemplate`The Definition or Def-expand tag "${'tag'} in "${'tagGroup'}" has multiple top tags or more than one top group.`, + message: issueMessageTemplate`The Definition or Def-expand tag "${'tag'}" in "${'tagGroup'}" has multiple top tags or more than one top group.`, }, invalidDefExpandGroupStructure: { hedCode: 'DEF_EXPAND_INVALID', level: 'error', - message: issueMessageTemplate`The Def-expand tag "${'tag'} in "${'tagGroup'}" has multiple top tags or more than one top group.`, + message: issueMessageTemplate`The Def-expand tag "${'tag'}" in "${'tagGroup'}" has multiple top tags or more than one top group.`, }, invalidDefinitionForbidden: { hedCode: 'DEFINITION_INVALID', level: 'error', - message: issueMessageTemplate`The Definition or Def-expand tag "${'tag'} in "${'tagGroup'}" has other definition-related tags in subgroups.`, + message: issueMessageTemplate`The Definition or Def-expand tag "${'tag'}" in "${'tagGroup'}" has other definition-related tags in subgroups.`, }, defExpandContentsInvalid: { hedCode: 'DEF_EXPAND_INVALID', @@ -197,7 +197,7 @@ const issueData = { conflictingDefinitions: { hedCode: 'DEFINITION_INVALID', level: 'error', - message: issueMessageTemplate`Definition "${'definition1'}" and "${'definition2'}' conflict.`, + message: issueMessageTemplate`Definition "${'definition1'}" and "${'definition2'}" conflict.`, }, duplicateDefinitionNames: { hedCode: 'DEFINITION_INVALID', @@ -327,7 +327,7 @@ const issueData = { invalidNumberOfSubgroups: { hedCode: 'TAG_GROUP_ERROR', level: 'error', - message: issueMessageTemplate`The tag "${'tag'} is in a "${'string'} with too many or too few subgroups.`, + message: issueMessageTemplate`The tag "${'tag'}" is in a "${'string'}" with too many or too few subgroups.`, }, invalidTopLevelTag: { hedCode: 'TAG_GROUP_ERROR', diff --git a/src/parser/parsedHedTag.js b/src/parser/parsedHedTag.js index 9f522809..8439c334 100644 --- a/src/parser/parsedHedTag.js +++ b/src/parser/parsedHedTag.js @@ -254,7 +254,7 @@ export default class ParsedHedTag extends ParsedHedSubstring { return null } if (this._units === null) { - const acceptsAnyUnits = this.takesValueTag.unitClasses.some((unitClass) => unitClass?.name === 'anyUnits') + const acceptsAnyUnits = this.takesValueTag.unitClasses.some((unitClass) => unitClass.name === 'anyUnits') return acceptsAnyUnits ? null : value } const factor = this._unit?.conversionFactor(this._units) ?? null @@ -510,29 +510,28 @@ export default class ParsedHedTag extends ParsedHedSubstring { // Placeholders work return null } - const valueClassNames = this._schemaTag.valueAttributeNames?.get('valueClass') - if (!valueClassNames) { + const valueClasses = this.takesValueTag.valueClasses + if (valueClasses.length === 0) { const characters = characterSets.problemCharacters(value, UNCLASSED_VALUE_SETS) if (characters.length === 0) { return null } return { characters: characters, subject: 'a value with no value class', names: UNCLASSED_VALUE_SETS } } - const entryManager = this.schema.entries.valueClasses let characterProblem = null - for (const valueClassName of valueClassNames) { - const valueClass = entryManager.getEntry(valueClassName) + for (const valueClass of valueClasses) { const problem = valueClass.valueProblem(value) if (problem === null) { return null } if (problem.characters && characterProblem === null) { - characterProblem = { characters: problem.characters, subject: valueClassName, names: valueClass.allowedNames } + characterProblem = { characters: problem.characters, subject: valueClass.name, names: valueClass.allowedNames } } } if (characterProblem !== null) { return characterProblem } + const valueClassNames = valueClasses.map((valueClass) => valueClass.name) return { msg: `Tag "${this.schemaTag.name}" has value classes [${valueClassNames.join(', ')}] but its value "${value}" is not in any of them.`, } diff --git a/src/schema/entries/schemaEntryWithAttributes.js b/src/schema/entries/schemaEntryWithAttributes.js index 4824983c..213050ce 100644 --- a/src/schema/entries/schemaEntryWithAttributes.js +++ b/src/schema/entries/schemaEntryWithAttributes.js @@ -84,12 +84,29 @@ export default class SchemaEntryWithAttributes extends SchemaEntry { if (ours.length !== theirs.size) { return false } - const sorted = (values) => - [...values] - .map(String) - .sort((a, b) => a.localeCompare(b)) - .join('|') - return ours.every(([name, values]) => theirs.has(name) && sorted(theirs.get(name)) === sorted(values)) + return ours.every( + ([name, values]) => theirs.has(name) && SchemaEntryWithAttributes.sameValues(theirs.get(name), values), + ) + } + + /** + * Determine whether two lists of attribute values hold the same values in any order. + * + * The values are compared as strings, item by item after sorting, so a value containing "|" is not confused with + * two values. + * + * @param {Array} first The first list of values. + * @param {Array} second The second list of values. + * @returns {boolean} Whether the two lists hold the same values. + */ + static sameValues(first, second) { + if (first.length !== second.length) { + return false + } + const sorted = (values) => [...values].map(String).sort((a, b) => a.localeCompare(b)) + const sortedFirst = sorted(first) + const sortedSecond = sorted(second) + return sortedFirst.every((value, index) => value === sortedSecond[index]) } /** diff --git a/src/schema/entries/valueTag.js b/src/schema/entries/valueTag.js index ccd8f77e..5ac077be 100644 --- a/src/schema/entries/valueTag.js +++ b/src/schema/entries/valueTag.js @@ -80,14 +80,10 @@ export default class SchemaValueTag extends SchemaTag { if (!SchemaEntryWithAttributes.prototype.equivalent.call(this.parent, other.parent)) { return false } - // A class without an entry (anyUnits before HED 8.5.0) is compared as ''. - const names = (entries) => - entries - .map((entry) => entry?.name ?? '') - .sort((a, b) => a.localeCompare(b)) - .join('|') + const names = (entries) => entries.map((entry) => entry.name) return ( - names(this.unitClasses) === names(other.unitClasses) && names(this.valueClasses) === names(other.valueClasses) + SchemaEntryWithAttributes.sameValues(names(this.unitClasses), names(other.unitClasses)) && + SchemaEntryWithAttributes.sameValues(names(this.valueClasses), names(other.valueClasses)) ) } } diff --git a/src/schema/parser/tag.js b/src/schema/parser/tag.js index 0ffa6439..185b6d91 100644 --- a/src/schema/parser/tag.js +++ b/src/schema/parser/tag.js @@ -67,12 +67,7 @@ export default class TagParser extends SchemaEntryWithAttributesParser { this.processRootedElements(parentMap, valueAttributeDefinitions) const tagUnitClassDefinitions = this.processTagClasses(valueAttributeDefinitions, 'unitClass', this.unitClasses) - const tagValueClassDefinitions = this.processTagClasses( - valueAttributeDefinitions, - 'valueClass', - this.valueClasses, - false, - ) + const tagValueClassDefinitions = this.processTagClasses(valueAttributeDefinitions, 'valueClass', this.valueClasses) this.processRecursiveAttributes(parentMap, booleanAttributeDefinitions) this.createSchemaTags( @@ -224,10 +219,11 @@ export default class TagParser extends SchemaEntryWithAttributesParser { if (classNames === undefined) { continue } - // A class name without an entry (anyUnits before HED 8.5.0) is kept as undefined. + // A class name the schema does not define is dropped, as in hed-typescript; the specification requires a + // defined class or anyUnits, and the validator assumes a valid schema. tagClassDefinitions.set( tagName, - classNames.map((className) => classes.getEntry(className)), + classNames.map((className) => classes.getEntry(className)).filter((entry) => entry !== undefined), ) if (removeAttribute) { valueAttributes.delete(attribute) diff --git a/tests/otherTests/schemaParser.spec.js b/tests/otherTests/schemaParser.spec.js index 13d0d0ff..107668ad 100644 --- a/tests/otherTests/schemaParser.spec.js +++ b/tests/otherTests/schemaParser.spec.js @@ -111,6 +111,14 @@ describe('Schema layer (realignment phase 3)', () => { assert.isFalse(first.equivalent(other)) }) + it('compares attribute values item by item, so a value containing "|" is not two values', () => { + const tags = new SchemaAttribute('relatedTag', undefined, new Set(), false) + const entry = (values) => new SchemaUnitModifier('k', undefined, new Set(), new Map([[tags, values]])) + assert.isTrue(entry(['a|b', 'c']).equivalent(entry(['c', 'a|b']))) + assert.isFalse(entry(['a|b', 'c']).equivalent(entry(['a', 'b|c']))) + assert.isFalse(entry(['a', 'b']).equivalent(entry(['a', 'b', 'b']))) + }) + it('gives a single attribute value and throws when there are several', () => { const factor = new SchemaAttribute('conversionFactor', undefined, new Set(), false) const modifier = new SchemaUnitModifier('k', undefined, new Set(), new Map([[factor, ['1000']]]))