Minor fixes for alignment

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
 

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',

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.`,
     }

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])
   }
 
   /**

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))
     )
   }
 }

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)

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']]]))