Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 39 additions & 9 deletions spec_tests/jsonTests.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,7 @@ const runMap = new Map([['TAG_EXPRESSION_REPEATED', ['tags-duplicated-across-mul
//const runOnly = new Set(["eventsPass"])
const runOnly = new Set()
const skippedErrors = {
TAG_EXTENDED: 'Warning not being checked',
SIDECAR_KEY_MISSING: 'Warning not being checked',
ELEMENT_DEPRECATED: 'Warning not being checked',
}
const readFileSync = fs.readFileSync
const test_file_name = 'javascriptTests.json'
Expand Down Expand Up @@ -137,22 +135,54 @@ describe('HED validation using JSON tests', () => {
const failedCombos = comboListToStrings(tests.combo_tests.fails)
const passedCombos = comboListToStrings(tests.combo_tests.passes)

const assertErrors = function (expectedErrors, issues, header) {
// Get the set of actual issues that were encountered.
/**
* Separates the error codes and warning codes from the issues
* @param issues
* @returns {[Set<string>,Set<string]}
*/
const extractErrorCodes = function (issues) {
const errors = new Set()
const warnings = new Set()
for (const issue of issues) {
let code
let level
if (issue instanceof BidsHedIssue) {
errors.add(issue.hedIssue.hedCode)
code = issue.hedIssue.hedCode
level = issue.hedIssue.level
} else {
errors.add(issue.hedCode)
code = issue.hedCode
level = issue.level
}
if (level === 'error') {
errors.add(code)
} else if (level === 'warning') {
warnings.add(code)
}
}
return [errors, warnings]
}

const assertErrors = function (expectedErrors, issues, header) {
// Get the set of actual issues that were encountered.
const [errors, warnings] = extractErrorCodes(issues)
if (warning) {
assert.isTrue(errors.size === 0, `${header} expected no errors but received [${[...errors].join(', ')}]`)
let warningIntersection = [...expectedErrors].some((element) => warnings.has(element))
if (expectedErrors.size === 0 && warnings.size === 0) {
warningIntersection = true
}
assert.isTrue(
warningIntersection,
`${header} expected one of warnings[${[...expectedErrors].join(', ')}] but received [${[...warnings].join(', ')}]`,
)
return
}
let hasIntersection = [...expectedErrors].some((element) => errors.has(element))
let errorIntersection = [...expectedErrors].some((element) => errors.has(element))
if (expectedErrors.size === 0 && errors.size === 0) {
hasIntersection = true
errorIntersection = true
}
assert.isTrue(
hasIntersection,
errorIntersection,
`${header} expected one of errors[${[...expectedErrors].join(', ')}] but received [${[...errors].join(', ')}]`,
)
}
Expand Down
4 changes: 2 additions & 2 deletions src/bids/types/json.js
Original file line number Diff line number Diff line change
Expand Up @@ -363,9 +363,9 @@ export class BidsSidecarKey {
* @private
*/
_parseValueString(hedSchemas) {
const [parsedString, errors, warnings] = parseHedString(this.valueString, hedSchemas, false, true)
const [parsedString, errorIssues, warningIssues] = parseHedString(this.valueString, hedSchemas, false, true)
this.parsedValueString = parsedString
return [errors, warnings]
return [errorIssues, warningIssues]
}

/**
Expand Down
2 changes: 1 addition & 1 deletion src/bids/validator/tsvValidator.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import { EventManager } from '../../parser/eventManager'
*/
export class BidsHedTsvValidator extends BidsValidator {
/**
* The BIDS TSV file being validated.
* The singleton instance of the checker for reserved requirements.
* @type {ReservedChecker}
*/
reserved
Expand Down
15 changes: 10 additions & 5 deletions src/issues/data.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,11 +27,21 @@ export default {
level: 'error',
message: stringTemplate`Comma missing after - "${'tag'}".`,
},
deprecatedTag: {
hedCode: 'ELEMENT_DEPRECATED',
level: 'warning',
message: stringTemplate`Tags "${'tags'} in "${'string'} are deprecated. Please see tag description for instructions on replacement.".`,
},
duplicateTag: {
hedCode: 'TAG_EXPRESSION_REPEATED',
level: 'error',
message: stringTemplate`Duplicate tags - "${'tags'} in "${'string'}".`,
},
extendedTag: {
hedCode: 'TAG_EXTENDED',
level: 'warning',
message: stringTemplate`Tag extensions found for ${'tags'} in "${'string'}".`,
},
invalidCharacter: {
hedCode: 'CHARACTER_INVALID',
level: 'error',
Expand Down Expand Up @@ -98,11 +108,6 @@ export default {
level: 'error',
message: stringTemplate`Invalid placeholder value for tag "${'tag'}".`,
},
extension: {
hedCode: 'TAG_EXTENDED',
level: 'warning',
message: stringTemplate`Tag extension found - "${'tag'}".`,
},
invalidPlaceholderContext: {
hedCode: 'PLACEHOLDER_INVALID',
level: 'error',
Expand Down
32 changes: 16 additions & 16 deletions src/parser/definitionManager.js
Original file line number Diff line number Diff line change
Expand Up @@ -54,27 +54,27 @@ export class Definition {
* @param {ParsedHedTag} tag - The parsed HEd tag whose details should be checked.
* @param {Schemas} hedSchema - The HED schemas used to validate against.
* @param {boolean} placeholderAllowed - If true then placeholder is allowed in the def tag.
* @returns {Array} - Returns [string, Issue[]] containing the evaluated normalized definition string and any issues in the evaluation,
* @returns {Array} - Returns [string, Issue[], Issue[]] containing the evaluated normalized definition string and any issues in the evaluation,
*/
evaluateDefinition(tag, hedSchema, placeholderAllowed) {
// Check that the level of the value of tag agrees with the definition
if (!!this.defTag._splitValue !== !!tag._splitValue) {
const errorType = tag.schemaTag.name === 'Def' ? 'missingDefinitionForDef' : 'missingDefinitionForDefExpand'
return [null, [generateIssue(errorType, { definition: tag._value })]]
return [null, [generateIssue(errorType, { definition: tag._value })], []]
}
// Check that the evaluated definition contents okay (if two-level value)
if (!this.defContents) {
return ['', []]
return ['', [], []]
}
if (!this.defTag._splitValue || (placeholderAllowed && tag._splitValue === '#')) {
return [this.defContents.normalized, []]
return [this.defContents.normalized, [], []]
}
const evalString = this.defContents.originalTag.replace('#', tag._splitValue)
const [normalizedValue, issues] = parseHedString(evalString, hedSchema, false, false)
if (issues.length > 0) {
return [null, issues]
const [normalizedValue, errorIssues, warningIssues] = parseHedString(evalString, hedSchema, false, false)
if (errorIssues.length > 0) {
return [null, errorIssues, warningIssues]
}
return [normalizedValue.normalized, []]
return [normalizedValue.normalized, [], []]
}

/**
Expand Down Expand Up @@ -106,30 +106,30 @@ export class Definition {
*
* @param {string} hedString - A list of string definitions.
* @param {Schemas} hedSchemas - The HED schemas to use in creation.
* @returns {Array} - Returns [Definition, Issue[]] with the definition and any issues.
* @returns {Array} - Returns [Definition, Issue[], Issue[]] with the definition and any issues.
*/
static createDefinition(hedString, hedSchemas) {
const [parsedString, issues] = parseHedString(hedString, hedSchemas, true, true)
if (issues.length > 0) {
return [null, issues]
const [parsedString, errorIssues, warningIssues] = parseHedString(hedString, hedSchemas, true, true)
if (errorIssues.length > 0) {
return [null, errorIssues, warningIssues]
}
if (parsedString.topLevelTags.length !== 0 || parsedString.tagGroups.length > 1) {
return [null, [generateIssue('invalidDefinition', { definition: hedString })]]
return [null, [generateIssue('invalidDefinition', { definition: hedString }), warningIssues]]
}
return Definition.createDefinitionFromGroup(parsedString.tagGroups[0])
}

/**
* Create a definition from a ParsedHedGroup.
* @param {ParsedHedGroup} group - The group to create a definition from.
* @returns {Array} - Returns [Definition, Issue[]] with the definition and any issues. (The definition will be null if issues.)
* @returns {Array} - Returns [Definition, Issue[], Issue[]] with the definition and any issues. (The definition will be null if issues.)
*/
static createDefinitionFromGroup(group) {
const def = new Definition(group)
if (def._checkDefinitionPlaceholderCount()) {
return [def, []]
return [def, [], []]
}
return [null, [generateIssue('invalidPlaceholderInDefinition', { definition: def.defGroup.originalTag })]]
return [null, [generateIssue('invalidPlaceholderInDefinition', { definition: def.defGroup.originalTag })], []]
}
}

Expand Down
24 changes: 19 additions & 5 deletions src/parser/parsedHedTag.js
Original file line number Diff line number Diff line change
Expand Up @@ -126,10 +126,7 @@ export default class ParsedHedTag extends ParsedHedSubstring {
}
// Check that there is a value if required
const reserved = ReservedChecker.getInstance()
if (
(schemaTag.hasAttributeName('requireChild') || reserved.requireValueTags.has(schemaTag.name)) &&
remainder === ''
) {
if ((schemaTag.hasAttribute('requireChild') || reserved.requireValueTags.has(schemaTag.name)) && remainder === '') {
IssueError.generateAndThrow('valueRequired', { tag: this.originalTag })
}
// Check if this could have a two-level value
Expand Down Expand Up @@ -234,7 +231,8 @@ export default class ParsedHedTag extends ParsedHedSubstring {
* @returns {boolean} Whether this tag has the named attribute.
*/
hasAttribute(attribute) {
return this.schema?.tagHasAttribute(this.formattedTag, attribute)
return this.schemaTag.hasAttribute(attribute)
//return this.schema?.tagHasAttribute(this.formattedTag, attribute)
}

/**
Expand Down Expand Up @@ -262,6 +260,22 @@ export default class ParsedHedTag extends ParsedHedSubstring {
}
}

/**
* Indicates whether the tag is deprecated
* @returns {boolean}
*/
get isDeprecated() {
return this.schemaTag.hasAttribute('deprecatedFrom')
}

/**
* Indicates whether the tag is deprecated
* @returns {boolean}
*/
get isExtended() {
return !this.takesValueTag && this._remainder !== ''
}

/**
* Get the schema tag object for this tag's value-taking form.
*
Expand Down
31 changes: 30 additions & 1 deletion src/parser/parser.js
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,36 @@ class HedStringParser {
if (checkIssues.length > 0) {
return [null, checkIssues, []]
}
return [parsedString, [], []]

// Warnings are only checked when there are no fatal errors
return [parsedString, [], this._getWarnings(parsedString)]
}

/**
* Get warnings applicable for a parsed HED string.
* @param {ParsedHedString} parsedString - HED string object to check for warnings.
* @returns {Issue[]} - Warnings for the parsed HED string
* @private
*/
_getWarnings(parsedString) {
const warnings = []
// Check for deprecated
const deprecatedTags = parsedString.tags.filter((tag) => tag.isDeprecated === true)
if (deprecatedTags.length > 0) {
const deprecated = deprecatedTags.map((tag) => tag.toString())
warnings.push(
generateIssue('deprecatedTag', { tags: '[' + deprecated.join(', ') + ']', string: parsedString.hedString }),
)
}
// Check for tag extensions
const extendedTags = parsedString.tags.filter((tag) => tag.isExtended === true)
if (extendedTags.length > 0) {
const extended = extendedTags.map((tag) => tag.toString())
warnings.push(
generateIssue('extendedTag', { tags: '[' + extended.join(', ') + ']', string: parsedString.hedString }),
)
}
return warnings
}

/**
Expand Down
4 changes: 2 additions & 2 deletions src/parser/reservedChecker.js
Original file line number Diff line number Diff line change
Expand Up @@ -278,7 +278,7 @@ export class ReservedChecker {
*/
static hasTopLevelTagGroupAttribute(tag) {
return (
tag.schemaTag.hasAttributeName('topLevelTagGroup') ||
tag.schemaTag.hasAttribute('topLevelTagGroup') ||
(ReservedChecker.reservedMap.has(tag.schemaTag.name) &&
ReservedChecker.reservedMap.get(tag.schemaTag.name).topLevelTagGroup)
)
Expand All @@ -293,6 +293,6 @@ export class ReservedChecker {
* Note: This checks both reserved and schema tag requirements.
*/
static hasGroupAttribute(tag) {
return tag.schemaTag.hasAttributeName('tagGroup')
return tag.schemaTag.hasAttribute('tagGroup')
}
}
4 changes: 2 additions & 2 deletions src/parser/tagConverter.js
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ export default class TagConverter {
}
if (
parentTag !== undefined &&
(!parentTag.hasAttributeName('extensionAllowed') || this.special.noExtensionTags.has(parentTag.name))
(!parentTag.hasAttribute('extensionAllowed') || this.special.noExtensionTags.has(parentTag.name))
) {
IssueError.generateAndThrow('invalidExtension', {
tag: this.tagLevels[tagLevelIndex],
Expand Down Expand Up @@ -154,7 +154,7 @@ export default class TagConverter {
}
this.schemaTag = schemaTag
this.remainder = this.tagLevels.slice(remainderStartLevelIndex).join('/')
if (this.schemaTag?.hasAttributeName('requireChild') && !this.remainder) {
if (this.schemaTag?.hasAttribute('requireChild') && !this.remainder) {
IssueError.generateAndThrow('childRequired', { tag: this.tagString })
}
}
Expand Down
Loading
Loading