diff --git a/bids/types/json.js b/bids/types/json.js index c82d1583..3aad6193 100644 --- a/bids/types/json.js +++ b/bids/types/json.js @@ -1,8 +1,13 @@ +import isPlainObject from 'lodash/isPlainObject' + import { sidecarValueHasHed } from '../utils' import { parseHedString } from '../../parser/main' import ParsedHedString from '../../parser/parsedHedString' import { BidsFile } from './basic' import BidsHedSidecarValidator from '../validator/bidsHedSidecarValidator' +import { generateIssue, IssueError } from '../../common/issues/issues' + +const ILLEGAL_SIDECAR_KEYS = new Set(['hed', 'n/a']) /** * A BIDS JSON file. @@ -74,9 +79,14 @@ export class BidsSidecar extends BidsJsonFile { _filterHedStrings() { const sidecarHedTags = Object.entries(this.jsonData) .map(([sidecarKey, sidecarValue]) => { + const trimmedSidecarKey = sidecarKey.trim() + if (ILLEGAL_SIDECAR_KEYS.has(trimmedSidecarKey.toLowerCase())) { + throw new IssueError(generateIssue('illegalSidecarHedKey', {})) + } if (sidecarValueHasHed(sidecarValue)) { - return [sidecarKey.trim(), new BidsSidecarKey(sidecarKey.trim(), sidecarValue.HED)] + return [trimmedSidecarKey, new BidsSidecarKey(trimmedSidecarKey, sidecarValue.HED, this)] } else { + this._verifyKeyHasNoDeepHed(sidecarKey, sidecarValue) return null } }) @@ -84,6 +94,26 @@ export class BidsSidecar extends BidsJsonFile { this.sidecarKeys = new Map(sidecarHedTags) } + /** + * Verify that a column has no deeply nested "HED" keys. + * + * @param {string} key An object key. + * @param {*} value An object value. + * @throws {IssueError} If an invalid "HED" key is found. + * @private + */ + _verifyKeyHasNoDeepHed(key, value) { + if (key.toUpperCase() === 'HED') { + throw new IssueError(generateIssue('illegalSidecarHedDeepKey', {})) + } + if (!isPlainObject(value)) { + return + } + for (const [subkey, subvalue] of Object.entries(value)) { + this._verifyKeyHasNoDeepHed(subkey, subvalue) + } + } + _categorizeHedStrings() { this.hedValueStrings = [] this.hedCategoricalStrings = [] @@ -176,7 +206,9 @@ export class BidsSidecar extends BidsJsonFile { this.columnSpliceMapping.set(sidecarKey, keyReferences) } } else { - throw new Error('Unexpected type found in sidecar parsedHedData map.') + throw new IssueError( + generateIssue('internalConsistencyError', { message: 'Unexpected type found in sidecar parsedHedData map.' }), + ) } } } @@ -224,19 +256,26 @@ export class BidsSidecarKey { * @type {ParsedHedString} */ parsedValueString + /** + * Weak reference to the sidecar. + * @type {WeakRef} + */ + sidecar /** * Constructor. * * @param {string} key The name of this key. * @param {string|Object} data The data for this key. + * @param {BidsSidecar} sidecar The parent sidecar. */ - constructor(key, data) { + constructor(key, data, sidecar) { this.name = key + this.sidecar = new WeakRef(sidecar) if (typeof data === 'string') { this.valueString = data - } else if (data !== Object(data)) { - throw new Error('Non-object passed as categorical data.') + } else if (!isPlainObject(data)) { + throw new IssueError(generateIssue('illegalSidecarHedType', { key: key, file: sidecar.file.relativePath })) } else { this.categoryMap = data } @@ -266,6 +305,14 @@ export class BidsSidecarKey { const issues = [] this.parsedCategoryMap = new Map() for (const [value, string] of Object.entries(this.categoryMap)) { + const trimmedValue = value.trim() + if (ILLEGAL_SIDECAR_KEYS.has(trimmedValue.toLowerCase())) { + throw new IssueError(generateIssue('illegalSidecarHedCategoricalValue', {})) + } else if (typeof string !== 'string') { + throw new IssueError( + generateIssue('illegalSidecarHedType', { key: value, file: this.sidecar.deref()?.file?.relativePath }), + ) + } const [parsedString, parsingIssues] = parseHedString(string, hedSchemas) this.parsedCategoryMap.set(value, parsedString) issues.push(...Object.values(parsingIssues).flat()) diff --git a/bids/types/tsv.js b/bids/types/tsv.js index f70dce53..73ca5da7 100644 --- a/bids/types/tsv.js +++ b/bids/types/tsv.js @@ -5,6 +5,7 @@ import { convertParsedTSVData, parseTSV } from '../tsvParser' import { BidsSidecar } from './json' import ParsedHedString from '../../parser/parsedHedString' import BidsHedTsvValidator from '../validator/bidsHedTsvValidator' +import { generateIssue, IssueError } from '../../common/issues/issues' /** * A BIDS TSV file. @@ -57,11 +58,11 @@ export class BidsTsvFile extends BidsFile { } else if (isPlainObject(tsvData)) { this.parsedTsv = convertParsedTSVData(tsvData) } else { - throw new Error('parsedTsv has an invalid type') + throw new IssueError(generateIssue('internalError', { message: 'parsedTsv has an invalid type' })) } this.potentialSidecars = potentialSidecars - this.mergedSidecar = new BidsSidecar(name, mergedDictionary, null) + this.mergedSidecar = new BidsSidecar(name, mergedDictionary, this.file) this.sidecarHedData = this.mergedSidecar.hedData this._parseHedColumn() } @@ -195,7 +196,9 @@ export class BidsTsvRow extends ParsedHedString { get onset() { const value = Number(this.rowCells.get('onset')) if (Number.isNaN(value)) { - throw new Error('Attempting to access the onset of a TSV row without one.') + throw new IssueError( + generateIssue('internalError', { message: 'Attempting to access the onset of a TSV row without one.' }), + ) } return value } diff --git a/bids/validator/bidsHedSidecarValidator.js b/bids/validator/bidsHedSidecarValidator.js index 29b7364b..c78669f6 100644 --- a/bids/validator/bidsHedSidecarValidator.js +++ b/bids/validator/bidsHedSidecarValidator.js @@ -126,6 +126,14 @@ export class BidsHedSidecarValidator { ), ) } + if (!this.sidecar.parsedHedData.has(referredKey) && referredKey !== 'HED') { + issues.push( + BidsHedIssue.fromHedIssue( + generateIssue('undefinedCurlyBraces', { column: referredKey }), + this.sidecar.file, + ), + ) + } } } diff --git a/common/issues/data.js b/common/issues/data.js index a7877d1e..2ef06151 100644 --- a/common/issues/data.js +++ b/common/issues/data.js @@ -313,6 +313,21 @@ export default { level: 'error', message: stringTemplate`The HED data for sidecar key "${'key'}" of file "${'file'}" is not either a key-value dictionary or a string.`, }, + illegalSidecarHedKey: { + hedCode: 'SIDECAR_INVALID', + level: 'error', + message: stringTemplate`The string 'HED' or 'n/a' was illegally used as a sidecar key.`, + }, + illegalSidecarHedCategoricalValue: { + hedCode: 'SIDECAR_INVALID', + level: 'error', + message: stringTemplate`The string 'HED' or 'n/a' was illegally used as a sidecar categorical value.`, + }, + illegalSidecarHedDeepKey: { + hedCode: 'SIDECAR_INVALID', + level: 'error', + message: stringTemplate`The key 'HED' was illegally used within a non-HED sidecar column.`, + }, // Generic errors genericError: { hedCode: 'GENERIC_ERROR', diff --git a/spec_tests/jsonTests.spec.js b/spec_tests/jsonTests.spec.js index 78f5257f..277484aa 100644 --- a/spec_tests/jsonTests.spec.js +++ b/spec_tests/jsonTests.spec.js @@ -8,6 +8,7 @@ import { buildSchemas } from '../validator/schema/init' import { SchemaSpec, SchemasSpec } from '../common/schema/types' import path from 'path' import { BidsSidecar, BidsTsvFile } from '../bids' +import { generateIssue, IssueError } from '../common/issues/issues' const fs = require('fs') const displayLog = process.env.DISPLAY_LOG === 'true' @@ -162,10 +163,20 @@ describe('HED validation using JSON tests', () => { const status = expectError ? 'Expect fail' : 'Expect pass' const header = `\n[${eCode} ${eName}](${status})\tCOMBO\t"${side}"\n"${events}"` const mergedSide = getMergedSidecar(side, defs) - const bidsSide = new BidsSidecar(`sidecar`, mergedSide, null) - const bidsTsv = new BidsTsvFile(`events`, events, null, [side], mergedSide) - const sidecarIssues = bidsSide.validate(schema) - const eventsIssues = bidsTsv.validate(schema) + let sidecarIssues = [] + try { + const bidsSide = new BidsSidecar(`sidecar`, mergedSide, { relativePath: 'combo test sidecar' }) + sidecarIssues = bidsSide.validate(schema) + } catch (e) { + sidecarIssues = [convertIssue(e)] + } + let eventsIssues = [] + try { + const bidsTsv = new BidsTsvFile(`events`, events, { relativePath: 'combo test tsv' }, [side], mergedSide) + eventsIssues = bidsTsv.validate(schema) + } catch (e) { + eventsIssues = [convertIssue(e)] + } const allIssues = [...sidecarIssues, ...eventsIssues] assertErrors(eCode, altCodes, expectError, iLog, header, allIssues) } @@ -173,8 +184,13 @@ describe('HED validation using JSON tests', () => { const eventsValidator = function (eCode, altCodes, eName, events, schema, defs, expectError, iLog) { const status = expectError ? 'Expect fail' : 'Expect pass' const header = `\n[${eCode} ${eName}](${status})\tEvents:\n"${events}"` - const bidsTsv = new BidsTsvFile(`events`, events, null, [], defs) - const eventsIssues = bidsTsv.validate(schema) + let eventsIssues = [] + try { + const bidsTsv = new BidsTsvFile(`events`, events, { relativePath: 'events test' }, [], defs) + eventsIssues = bidsTsv.validate(schema) + } catch (e) { + eventsIssues = [convertIssue(e)] + } assertErrors(eCode, altCodes, expectError, iLog, header, eventsIssues) } @@ -182,8 +198,13 @@ describe('HED validation using JSON tests', () => { const status = expectError ? 'Expect fail' : 'Expect pass' const header = `\n[${eCode} ${eName}](${status})\tSIDECAR "${side}"` const side1 = getMergedSidecar(side, defs) - const bidsSide = new BidsSidecar(`sidecar`, side1, null) - const sidecarIssues = bidsSide.validate(schema) + let sidecarIssues = [] + try { + const bidsSide = new BidsSidecar(`sidecar`, side1, { relativePath: 'sidecar test' }) + sidecarIssues = bidsSide.validate(schema) + } catch (e) { + sidecarIssues = [convertIssue(e)] + } assertErrors(eCode, altCodes, expectError, iLog, header, sidecarIssues) } @@ -191,11 +212,30 @@ describe('HED validation using JSON tests', () => { const status = expectError ? 'Expect fail' : 'Expect pass' const header = `\n[${eCode} ${eName}](${status})\tSTRING: "${str}"` const hTsv = `HED\n${str}\n` - const bidsTsv = new BidsTsvFile(`events`, hTsv, null, [], defs) - const stringIssues = bidsTsv.validate(schema) + let stringIssues = [] + try { + const bidsTsv = new BidsTsvFile(`events`, hTsv, { relativePath: 'string test tsv' }, [], defs) + stringIssues = bidsTsv.validate(schema) + } catch (e) { + stringIssues = [convertIssue(e)] + } assertErrors(eCode, altCodes, expectError, iLog, header, stringIssues) } + /** + * Convert an Error into an Issue. + * + * @param {Error} issueError A thrown error. + * @returns {Issue} A HED issue. + */ + const convertIssue = function (issueError) { + if (issueError instanceof IssueError) { + return issueError.issue + } else { + return generateIssue('internalError', { message: issueError.message }) + } + } + beforeAll(async () => { hedSchema = schemaMap.get(schema) defs = { definitions: { HED: { defList: definitions.join(',') } } } diff --git a/tests/bids.spec.js b/tests/bids.spec.js index cbaaec20..19e2cd6d 100644 --- a/tests/bids.spec.js +++ b/tests/bids.spec.js @@ -550,6 +550,18 @@ describe('BIDS datasets', () => { }), standaloneSidecars[1].file, ), + BidsHedIssue.fromHedIssue( + generateIssue('undefinedCurlyBraces', { + column: 'event_code', + }), + standaloneSidecars[1].file, + ), + BidsHedIssue.fromHedIssue( + generateIssue('undefinedCurlyBraces', { + column: 'response_time', + }), + standaloneSidecars[1].file, + ), BidsHedIssue.fromHedIssue( generateIssue('recursiveCurlyBracesWithKey', { column: 'response_time', @@ -626,14 +638,12 @@ describe('BIDS datasets', () => { column: 'response_time', }), combinedDatasets[0].file, - { tsvLine: 3 }, ), BidsHedIssue.fromHedIssue( generateIssue('undefinedCurlyBraces', { column: 'response_time', }), combinedDatasets[1].file, - { tsvLine: 3 }, ), BidsHedIssue.fromHedIssue( generateIssue('duplicateTag', {