Merge pull request #237 from VisLab/revised-validator

Added handling of duplicates across onset rows

bids/types/basic.js

@@ -22,6 +22,12 @@ export class BidsFile {
    */
   _validatorClass
 
+  /**
+   *
+   * @param {string} name - The name of the file -- used for messages.
+   * @param {object} file - The representation of the file for error messages.
+   * @param validatorClass
+   */
   constructor(name, file, validatorClass) {
     this.name = name
     this.file = file
@@ -31,17 +37,17 @@ export class BidsFile {
   /**
    * Determine whether this file has any HED data.
    *
-   * @returns {boolean}
+   * @returns {boolean} - True if this file has HED data.
    */
   hasHedData() {
     return false
   }
 
   /**
-   * Validate this validator's tsv file
+   * Validate this validator's tsv file.
    *
-   * @param {Schemas} schemas
-   * @returns {BidsIssue[]} Any issues found during validation of this TSV file.
+   * @param {Schemas} schemas - The HED schemas used to validate this file.
+   * @returns {BidsIssue[]} - Any issues found during validation of this TSV file.
    */
   validate(schemas) {
     if (!this.hasHedData()) {

bids/types/dataset.js

@@ -5,7 +5,7 @@ export class BidsDataset {
    */
   eventData
   /**
-   * The dataset's sidecar data.
+   * The dataset's bidsFile data.
    * @type {BidsSidecar[]}
    */
   sidecarData

bids/types/json.js

@@ -28,17 +28,17 @@ export class BidsJsonFile extends BidsFile {
 
 export class BidsSidecar extends BidsJsonFile {
   /**
-   * The extracted keys for this sidecar.
+   * The extracted keys for this bidsFile.
    * @type {Map<string, BidsSidecarKey>}
    */
   sidecarKeys
   /**
-   * The extracted HED data for this sidecar.
+   * The extracted HED data for this bidsFile.
    * @type {Map<string, string|Object<string, string>>}
    */
   hedData
   /**
-   * The parsed HED data for this sidecar.
+   * The parsed HED data for this bidsFile.
    * @type {Map<string, ParsedHedString|Map<string, ParsedHedString>>}
    */
   parsedHedData
@@ -72,7 +72,7 @@ export class BidsSidecar extends BidsJsonFile {
   /**
    * Constructor.
    *
-   * @param {string} name The name of the sidecar file.
+   * @param {string} name The name of the bidsFile file.
    * @param {Object} sidecarData The raw JSON data.
    * @param {Object} file The file object representing this file.
    * @param {DefinitionManager } defManager - The external definitions to use
@@ -99,7 +99,7 @@ export class BidsSidecar extends BidsJsonFile {
   }
 
   /**
-   * Create the sidecar key map from the JSON.
+   * Create the bidsFile key map from the JSON.
    * @private
    */
   _filterHedStrings() {
@@ -173,7 +173,7 @@ export class BidsSidecar extends BidsJsonFile {
   }
 
   /**
-   * Parse this sidecar's HED strings within the sidecar structure.
+   * Parse this bidsFile's HED strings within the bidsFile structure.
    *
    * The parsed strings are placed into {@link parsedHedData}.
    *
@@ -197,7 +197,7 @@ export class BidsSidecar extends BidsJsonFile {
   }
 
   /**
-   * Generate a mapping of an individual BIDS sidecar's curly brace references.
+   * Generate a mapping of an individual BIDS bidsFile's curly brace references.
    *
    * @private
    */
@@ -212,7 +212,7 @@ export class BidsSidecar extends BidsJsonFile {
         this._parseCategorySplice(sidecarKey, hedData)
       } else if (hedData) {
         IssueError.generateAndThrow('internalConsistencyError', {
-          message: 'Unexpected type found in sidecar parsedHedData map.',
+          message: 'Unexpected type found in bidsFile parsedHedData map.',
         })
       }
     }
@@ -289,7 +289,7 @@ export class BidsSidecarKey {
    */
   parsedValueString
   /**
-   * Weak reference to the sidecar.
+   * Weak reference to the bidsFile.
    * @type {BidsSidecar}
    */
   sidecar
@@ -301,7 +301,7 @@ export class BidsSidecarKey {
    *
    * @param {string} key The name of this key.
    * @param {string|Object<string, string>} data The data for this key.
-   * @param {BidsSidecar} sidecar The parent sidecar.
+   * @param {BidsSidecar} sidecar The parent bidsFile.
    */
   constructor(key, data, sidecar) {
     this.name = key
@@ -331,7 +331,7 @@ export class BidsSidecarKey {
   }
 
   /**
-   * Parse the value string in a sidecar
+   * Parse the value string in a bidsFile
    * @param {Schemas} hedSchemas - The HED schemas to use.
    * @param {boolean} fullCheck - If true, then assume in final form and not up for potential splice.
    * @returns {Issue[]}

bids/types/tsv.js

@@ -26,7 +26,7 @@ export class BidsTsvFile extends BidsFile {
    */
   potentialSidecars
   /**
-   * The pseudo-sidecar object representing the merged sidecar data.
+   * The pseudo-bidsFile object representing the merged bidsFile data.
    * @type {BidsSidecar}
    */
   mergedSidecar
@@ -40,7 +40,7 @@ export class BidsTsvFile extends BidsFile {
    * @param {{headers: string[], rows: string[][]}|Map<string, string[]>|string} tsvData This file's TSV data.
    * @param {object} file The file object representing this file.
    * @param {string[]} potentialSidecars The list of potential JSON sidecars.
-   * @param {object} mergedDictionary The merged sidecar data.
+   * @param {object} mergedDictionary The merged bidsFile data.
    * @param {DefinitionManager} defManager
    */
   constructor(name, tsvData, file, potentialSidecars = [], mergedDictionary = {}, defManager) {
@@ -147,6 +147,15 @@ export class BidsTsvElement {
     const onsetString = this.onset ? ` with onset=${this.onset.toString()}` : ''
     return this.hedString + ` in TSV file "${this.fileName}" at line(s) ${this.tsvLine}` + onsetString
   }
+
+  /**
+   * Create a string list of the
+   * @param BidsTsvElements[] elements - A list of elements to construct line numbers from.
+   * @returns {string} - A string with the list of line numbers for error messages.
+   */
+  static getTsvLines(elements) {
+    return elements.map((element) => element.tsvLine).join(',')
+  }
 }
 
 /**

bids/utils.js

@@ -1,8 +1,8 @@
 /**
- * Determine whether a sidecar value has HED data.
+ * Determine whether a bidsFile value has HED data.
  *
- * @param {object} sidecarValue A BIDS sidecar value.
- * @returns {boolean} Whether the sidecar value has HED data.
+ * @param {object} sidecarValue A BIDS bidsFile value.
+ * @returns {boolean} Whether the bidsFile value has HED data.
  */
 export const sidecarValueHasHed = function (sidecarValue) {
   return sidecarValue !== null && typeof sidecarValue === 'object' && sidecarValue.HED !== undefined

bids/validator/sidecarValidator.js

@@ -1,155 +1,154 @@
 import { BidsHedIssue } from '../types/issues'
 import ParsedHedString from '../../parser/parsedHedString'
-// IMPORTANT: This import cannot be shortened to '../../validator', as this creates a circular dependency until v4.0.0.
-//import { validateHedString } from '../../validator/event/init'
 import { generateIssue, IssueError } from '../../common/issues/issues'
 import { getCharacterCount } from '../../utils/string.js'
+import { BidsValidator } from './validator'
+
 /**
  * Validator for HED data in BIDS JSON sidecars.
  */
-export class BidsHedSidecarValidator {
-  /**
-   * The BIDS sidecar being validated.
-   * @type {BidsSidecar}
-   */
-  sidecar
-  /**
-   * The HED schema collection being validated against.
-   * @type {Schemas}
-   */
-  hedSchemas
-  /**
-   * The issues found during validation.
-   * @type {BidsIssue[]}
-   */
-  issues
-
+export class BidsHedSidecarValidator extends BidsValidator {
   /**
-   * Constructor.
+   * Constructor for the BidsHedSidecarValidator.
    *
-   * @param {BidsSidecar} sidecar The BIDS sidecar being validated.
-   * @param {Schemas} hedSchemas
+   * @param {BidsSidecar} sidecar - The BIDS bidsFile being validated.
+   * @param {Schemas} hedSchemas - The schemas used for the sidecar validation.
    */
   constructor(sidecar, hedSchemas) {
-    this.sidecar = sidecar
-    this.hedSchemas = hedSchemas
-    this.issues = []
+    super(sidecar, hedSchemas)
   }
 
   /**
-   * Validate a BIDS JSON sidecar file. This method returns the complete issue list for convenience.
+   * Validate a BIDS JSON bidsFile file. This method returns the complete issue list for convenience.
    *
-   * @returns {BidsIssue[]} Any issues found during validation of this sidecar file.
+   * @returns {BidsIssue[]} - Any issues found during validation of this bidsFile file.
    */
   validate() {
     // Allow schema to be set a validation time -- this is checked by the superclass of BIDS file
     const sidecarParsingIssues = BidsHedIssue.fromHedIssues(
-      this.sidecar.parseHedStrings(this.hedSchemas),
-      this.sidecar.file,
+      this.bidsFile.parseHedStrings(this.hedSchemas),
+      this.bidsFile.file,
     )
     this.issues.push(...sidecarParsingIssues)
     if (sidecarParsingIssues.length > 0) {
       return this.issues
     }
-    this.issues.push(...this._validateStrings(), ...this.validateCurlyBraces())
+    this.issues.push(...this._validateStrings(), ...this._validateCurlyBraces())
     return this.issues
   }
 
   /**
-   * Validate this sidecar's HED strings.
+   * Validate this bidsFile's HED strings.
    *
    * @returns {BidsIssue[]} All issues found.
    */
   _validateStrings() {
     const issues = []
 
-    for (const [sidecarKeyName, hedData] of this.sidecar.parsedHedData) {
+    for (const [sidecarKeyName, hedData] of this.bidsFile.parsedHedData) {
       if (hedData instanceof ParsedHedString) {
-        // Value options have HED as string
+        // Value options have HED as string.
         issues.push(...this._checkDetails(sidecarKeyName, hedData, true))
       } else if (hedData instanceof Map) {
-        // Categorical options have HED as a Map
+        // Categorical options have HED as a Map.
         for (const valueString of hedData.values()) {
-          const placeholdersAllowed = this.sidecar.sidecarKeys.get(sidecarKeyName).hasDefinitions
+          const placeholdersAllowed = this.bidsFile.sidecarKeys.get(sidecarKeyName).hasDefinitions
           issues.push(...this._checkDetails(sidecarKeyName, valueString, placeholdersAllowed))
         }
       } else {
         IssueError.generateAndThrow('internalConsistencyError', {
-          message: 'Unexpected type found in sidecar parsedHedData map.',
+          message: 'Unexpected type found in bidsFile parsedHedData map.',
         })
       }
     }
     return issues
   }
 
+  /**
+   * Check definitions and placeholders for a string associated with a sidecar key.
+   *
+   * @param {string} sidecarKeyName - The name of the sidecar key associated with string to be checked.
+   * @param {ParsedHedString} hedString - The parsed string to be checked.
+   * @returns {BidsHedIssue[]} - Issues associated with the check.
+   * @private
+   */
   _checkDetails(sidecarKeyName, hedString) {
     const issues = this._checkDefs(sidecarKeyName, hedString, true)
     issues.push(...this._checkPlaceholders(sidecarKeyName, hedString))
     return issues
   }
 
-  _checkDefs(sidecarKeyName, sidecarString, placeholdersAllowed) {
-    let issues = this.sidecar.definitions.validateDefs(sidecarString, this.hedSchemas, placeholdersAllowed)
+  /**
+   * Validate the Def and Def-expand usage against the sidecar definitions.
+   *
+   * @param {string} sidecarKeyName - Name of the sidecar key for this HED string
+   * @param {ParsedHedString} hedString - The parsed HED string object associated with this key.
+   * @param {boolean} placeholdersAllowed - If true, placeholders are allowed here.
+   * @returns {BidsHedIssue[]} - Issues encountered such as missing definitions or improper Def-expand values.
+   * @private
+   */
+  _checkDefs(sidecarKeyName, hedString, placeholdersAllowed) {
+    let issues = this.bidsFile.definitions.validateDefs(hedString, this.hedSchemas, placeholdersAllowed)
     if (issues.length > 0) {
-      return BidsHedIssue.fromHedIssues(issues, this.sidecar.file, { sidecarKeyName: sidecarKeyName })
+      return BidsHedIssue.fromHedIssues(issues, this.bidsFile.file, { sidecarKeyName: sidecarKeyName })
     }
-    issues = this.sidecar.definitions.validateDefExpands(sidecarString, this.hedSchemas, placeholdersAllowed)
-    return BidsHedIssue.fromHedIssues(issues, this.sidecar.file, { sidecarKeyName: sidecarKeyName })
+    issues = this.bidsFile.definitions.validateDefExpands(hedString, this.hedSchemas, placeholdersAllowed)
+    return BidsHedIssue.fromHedIssues(issues, this.bidsFile.file, { sidecarKeyName: sidecarKeyName })
   }
 
   _checkPlaceholders(sidecarKeyName, hedString) {
     const numberPlaceholders = getCharacterCount(hedString.hedString, '#')
-    const sidecarKey = this.sidecar.sidecarKeys.get(sidecarKeyName)
+    const sidecarKey = this.bidsFile.sidecarKeys.get(sidecarKeyName)
     if (!sidecarKey.valueString && !sidecarKey.hasDefinitions && numberPlaceholders > 0) {
       return [
         BidsHedIssue.fromHedIssue(
           generateIssue('invalidSidecarPlaceholder', { column: sidecarKeyName, string: hedString.hedString }),
-          this.sidecar.file,
+          this.bidsFile.file,
         ),
       ]
     } else if (sidecarKey.valueString && numberPlaceholders === 0) {
       return [
         BidsHedIssue.fromHedIssue(
           generateIssue('missingPlaceholder', { column: sidecarKeyName, string: hedString.hedString }),
-          this.sidecar.file,
+          this.bidsFile.file,
         ),
       ]
     }
     if (sidecarKey.valueString && numberPlaceholders > 1) {
       return [
         BidsHedIssue.fromHedIssue(
           generateIssue('invalidSidecarPlaceholder', { column: sidecarKeyName, string: hedString.hedString }),
-          this.sidecar.file,
+          this.bidsFile.file,
         ),
       ]
     }
     return []
   }
 
   /**
-   * Validate this sidecar's curly braces.
+   * Validate this bidsFile's curly braces -- checking recursion and missing columns.
    *
    * @returns {BidsIssue[]} All issues found.
    */
-  validateCurlyBraces() {
+  _validateCurlyBraces() {
     const issues = []
-    const references = this.sidecar.columnSpliceMapping
+    const references = this.bidsFile.columnSpliceMapping
 
     for (const [key, referredKeys] of references) {
       for (const referredKey of referredKeys) {
         if (references.has(referredKey)) {
           issues.push(
             BidsHedIssue.fromHedIssue(
               generateIssue('recursiveCurlyBracesWithKey', { column: referredKey, referrer: key }),
-              this.sidecar.file,
+              this.bidsFile.file,
             ),
           )
         }
-        if (!this.sidecar.parsedHedData.has(referredKey) && referredKey !== 'HED') {
+        if (!this.bidsFile.parsedHedData.has(referredKey) && referredKey !== 'HED') {
           issues.push(
             BidsHedIssue.fromHedIssue(
               generateIssue('undefinedCurlyBraces', { column: referredKey }),
-              this.sidecar.file,
+              this.bidsFile.file,
             ),
           )
         }