Fix handling of extensions and update tests

.github/copilot-instructions.md

@@ -40,16 +40,18 @@ Source code is plain JavaScript (`.js`) in `src/`. TypeScript type declarations
   - `tagConverter.js`, `eventManager.js`
 
 - `schema/` — HED schema loading, parsing, and merging
-  - `loader.js`, `parser.js`, `schemaMerger.js`
-  - `config.js`, `containers.js`, `entries.js`, `specs.js`, `init.js`
+  - `loader.js`, `init.js`, `config.js`, `containers.js`, `specs.js`, `xmlCollection.js`, `xmlType.js`,
+    `characterSets.js`
+  - `entries/` - one schema entry class per file, as hed-typescript `src/schema/entries/`
+  - `parser/` - one entry-type parser per file (`schemaParser.js` runs them), as hed-typescript `src/schema/parser/`
 
 - `issues/` — Issue codes and formatting
   - `issues.js`, `data.js`
 
 - `data/` — Bundled JSON configs and XML schema files
 
 - `utils/` — General utilities
-  - `array.js`, `string.js`, `files.js`, `paths.js`, `hedStrings.js`, `memoizer.js`, `xml.js`
+  - `array.js`, `string.js`, `files.js`, `paths.js`, `hedStrings.js`, `xml.js`
 
 ## Common commands
 

CHANGELOG.md

@@ -38,9 +38,44 @@ testSpecs`.
   `addIssueParameters` replaces `updateIssueParameters`, which stays as an
   alias; `BidsHedIssue` gains `toString` and `addIssueParameters`; internal
   errors are thrown as `IssueError`.
+- The schema layer follows hed-typescript's layout (realignment phase 3):
+  `src/schema/entries.js` and `src/schema/parser.js` are split into one file
+  per class under `src/schema/entries/` and `src/schema/parser/`; `Schema`,
+  `Schemas` and `SchemaXmlCollection` are renamed `HedSchema`, `HedSchemas` and
+  `HedSchemaXMLCollection`, and the old names stay as aliases through 4.x. A
+  schema attribute holds one set of properties and a `recursive` flag; every
+  property a schema defines is registered (8.3.0 and later gain 13
+  domain/range properties); `getSingleAttributeValue` and `getAttributeValues`
+  replace `getAttributeValue`. Removed with the old property model:
+  `SchemaProperty.isCategoryProperty`, `isTypeProperty`, `isRoleProperty`,
+  `SchemaAttribute.categoryProperty`, `typeProperty`, `roleProperties`,
+  `propertyNames`, and `getAttributeValue`. This is not a major version change
+  because the schema entry classes were never exported by the package: they
+  are reachable only inside the object `buildSchemasFromVersion` returns, no
+  known consumer uses the removed members, and the schema class declarations
+  in `types/index.d.ts` are to be withdrawn from the public declarations before
+  this release. Those declarations now match the runtime (members that never
+  existed, such as `getValue` and `SchemaTag.isUnitClass`, are gone, and
+  `SchemaUnitClass.units` is a `Map`). A schema that uses an undefined
+  attribute or gives `rooted` two values reports `invalidSchema`
+  (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).
 
 ### Fixed
 
+- A schema file that is well-formed XML but has no `HED` root or no `schema`
+  element is reported as `localSchemaLoadFailed` (SCHEMA_LOAD_FAILED) instead of
+  throwing a bare `TypeError`.
+- Tag extensions follow specification 3.2.5 rules 5 and 6 (hed-tests
+  `placeholder-invalid-after-extension`, `tag-extension-invalid-no-letter`): a
+  `#` after an extension term or in place of one (`Item/Blech/#`, `Item/#`) is
+  PLACEHOLDER_INVALID in a sidecar as in an events file (it was
+  TAG_EXTENSION_INVALID), while `Onset/#` stays TAG_EXTENSION_INVALID; and an
+  extension term must contain a letter, so `Item/Blech/3` and `Item/1-2` are
+  TAG_EXTENSION_INVALID for every schema version. A letter is A-Z or a-z, or any
+  Unicode letter from standard schema 8.3.0. The spec tests now use hed-tests
+  54f137e.
 - esbuild 0.28.1 and later work with `esbuild-plugins-node-modules-polyfill`:
   `legacy-peer-deps` is enabled in `.npmrc`.
 - Improved error reporting for duplicate tags.

package.json

@@ -45,7 +45,6 @@
   },
   "homepage": "https://github.com/hed-standard/hed-javascript",
   "dependencies": {
-    "core-js-pure": "^3.50.0",
     "fast-xml-parser": "^5.11.1",
     "lodash": "^4.18.1",
     "pluralize": "^8.0.0",

src/issues/data.js

@@ -487,6 +487,11 @@ const issueData = {
     level: 'error',
     message: issueMessageTemplate`Lazy partnered schemas are incompatible because they share the entry "${'entryName'}". These schemas require different prefixes.`,
   },
+  invalidSchema: {
+    hedCode: 'SCHEMA_LOAD_FAILED',
+    level: 'error',
+    message: issueMessageTemplate`The HED schema is invalid - "${'error'}".`,
+  },
   deprecatedStandardSchemaVersion: {
     hedCode: 'VERSION_DEPRECATED',
     level: 'error',

src/parser/parsedHedTag.js

@@ -8,7 +8,7 @@ import { ReservedChecker } from './reservedChecker'
 import TagConverter from './tagConverter'
 import { IssueError } from '../issues/issues'
 import { characterSets, codePoint } from '../schema/characterSets'
-import { SchemaValueTag } from '../schema/entries'
+import SchemaValueTag from '../schema/entries/valueTag'
 
 // The characters of a value whose placeholder declares no valueClass (character_sets.json; hed-python does the same).
 const UNCLASSED_VALUE_SETS = ['value-text']

src/parser/tagConverter.js

@@ -8,8 +8,8 @@ import { IssueError } from '../issues/issues'
 import { getTagSlashIndices } from '../utils/hedStrings'
 
 /**
- * @typedef {import('../schema/entries.js').SchemaTag} SchemaTag
- * @typedef {import('../schema/containers.js').Schemas} Schemas
+ * @typedef {import('../schema/entries/tag.js').default} SchemaTag
+ * @typedef {import('../schema/containers.js').HedSchemas} Schemas
  * @typedef {import('./tokenizer.js').TagSpec} TagSpec
  */
 
@@ -141,15 +141,14 @@ export default class TagConverter {
           msg: 'Tag extensions must have a parent in the HED schema.',
         })
       }
-      if (
-        parentTag !== undefined &&
-        (!parentTag.hasAttribute('extensionAllowed') || this.special.noExtensionTags.has(parentTag.name))
-      ) {
-        IssueError.generateAndThrow('invalidExtension', {
-          tag: this.tagLevels[tagLevelIndex],
-          parentTag: this.tagLevels.slice(0, tagLevelIndex).join('/'),
-          msg: `The tag "${this.tagLevels[tagLevelIndex]}" is an extension, but the parent tag "${parentTag.name}" does not allow extensions.`,
-        })
+      // A reserved tag that takes no value is not extended, even by "#" (TAG_EXTENSION_INVALID c); any other "#"
+      // here stands where its parent has no placeholder child (PLACEHOLDER_INVALID d).
+      if (parentTag !== undefined && this.special.noExtensionTags.has(parentTag.name)) {
+        this._throwExtensionNotAllowed(parentTag, tagLevelIndex)
+      }
+      this._checkPlaceholder(tagLevelIndex)
+      if (parentTag !== undefined && !parentTag.hasAttribute('extensionAllowed')) {
+        this._throwExtensionNotAllowed(parentTag, tagLevelIndex)
       }
       this._checkExtensions(tagLevelIndex)
       return childTag
@@ -166,11 +165,27 @@ export default class TagConverter {
     return childTag
   }
 
+  /**
+   * Report an extension of a tag that does not allow one.
+   *
+   * @param {SchemaTag} parentTag - The schema tag being extended.
+   * @param {number} tagLevelIndex - The index of the extension level.
+   * @throws {IssueError} Always.
+   * @private
+   */
+  _throwExtensionNotAllowed(parentTag, tagLevelIndex) {
+    IssueError.generateAndThrow('invalidExtension', {
+      tag: this.tagLevels[tagLevelIndex],
+      parentTag: this.tagLevels.slice(0, tagLevelIndex).join('/'),
+      msg: `The tag "${this.tagLevels[tagLevelIndex]}" is an extension, but the parent tag "${parentTag.name}" does not allow extensions.`,
+    })
+  }
+
   /**
    * Check the levels of an extension.
    *
    * @param {number} tagLevelIndex - The index of the first extension level.
-   * @throws {IssueError} If a level is a schema tag or not in the name class.
+   * @throws {IssueError} If a level is a schema tag, a placeholder, not in the name class, or has no letter.
    * @private
    */
   _checkExtensions(tagLevelIndex) {
@@ -186,10 +201,27 @@ export default class TagConverter {
           msg: `The tag "${child.name}" is a schema tag, but it appears after an extension tag "${this.tagLevels[tagLevelIndex]}".`,
         })
       }
+      this._checkPlaceholder(index)
       this._checkNameClass(index)
     }
   }
 
+  /**
+   * Check that a level of an extension is not a "#" placeholder.
+   *
+   * A "#" after an extension term (Item/Blech/#) or in place of one (Item/#) has no placeholder child to stand for,
+   * so it is PLACEHOLDER_INVALID in every context (specification 3.2.5 rule 5).
+   *
+   * @param {number} index - The index of the level.
+   * @throws {IssueError} If the level is a placeholder.
+   * @private
+   */
+  _checkPlaceholder(index) {
+    if (this.tagLevels[index] === '#') {
+      IssueError.generateAndThrow('invalidPlaceholder', { tag: this.tagString })
+    }
+  }
+
   /**
    * Get the schema tag for a level of the tag string.
    *
@@ -229,20 +261,34 @@ export default class TagConverter {
   }
 
   /**
-   * Check that a level of the tag string is in the name class.
+   * Check that a level of the tag string is in the name class and contains a letter.
+   *
+   * A letter is A-Z or a-z, or any Unicode letter when the schema's name class allows non-ASCII characters
+   * (standard schema 8.3.0 and later). A term without one (3, 1-2, an Arabic-Indic digit) reads as a value
+   * (specification 3.2.5 rule 6).
    *
    * @param {number} index - The index of the level.
-   * @throws {IssueError} If the level is not in the name class.
+   * @throws {IssueError} If the level is not in the name class or has no letter.
    * @private
    */
   _checkNameClass(index) {
     // Check whether the tagLevel is a valid name class
+    const tagLevel = this.tagLevels[index]
     const valueClasses = this.hedSchemas.getSchema(this.tagSpec.library)?.entries.valueClasses
-    if (!valueClasses?.definitions?.get('nameClass')?.validateValue(this.tagLevels[index])) {
+    const nameClass = valueClasses?.getEntry('nameClass')
+    if (!nameClass?.validateValue(tagLevel)) {
+      IssueError.generateAndThrow('invalidExtension', {
+        tag: tagLevel,
+        parentTag: this.tagLevels.slice(0, index).join('/'),
+        msg: `The tag extension "${tagLevel}" is not in the HED name class, so it cannot be used as a tag extension.`,
+      })
+    }
+    const letter = nameClass.allowedNames.includes('nonascii') ? /\p{L}/u : /[A-Za-z]/
+    if (!letter.test(tagLevel)) {
       IssueError.generateAndThrow('invalidExtension', {
-        tag: this.tagLevels[index],
+        tag: tagLevel,
         parentTag: this.tagLevels.slice(0, index).join('/'),
-        msg: `The tag extension "${this.tagLevels[index]}" is not in the HED name class, so it cannot be used as a tag extension.`,
+        msg: `The tag extension "${tagLevel}" contains no letter, so it reads as a value and cannot be used as a tag extension.`,
       })
     }
   }