diff --git a/bids/types/json.js b/bids/types/json.js index 560e3f9c..5ee71c38 100644 --- a/bids/types/json.js +++ b/bids/types/json.js @@ -293,7 +293,7 @@ export class BidsSidecarKey { } _parseValueString(hedSchemas) { - const [parsedString, parsingIssues] = parseHedString(this.valueString, hedSchemas) + const [parsedString, parsingIssues] = parseHedString(this.valueString, hedSchemas, false) const flatIssues = Object.values(parsingIssues).flat() this.parsedValueString = parsedString return flatIssues diff --git a/bids/validator/tsvValidator.js b/bids/validator/tsvValidator.js index 826d8d93..65e840ea 100644 --- a/bids/validator/tsvValidator.js +++ b/bids/validator/tsvValidator.js @@ -111,7 +111,7 @@ export class BidsHedTsvValidator { definitionsAllowed: 'no', } - const [parsedString, parsingIssues] = parseHedString(hedString, this.hedSchemas) + const [parsedString, parsingIssues] = parseHedString(hedString, this.hedSchemas, true) issues.push( ...BidsHedIssue.fromHedIssues(Object.values(parsingIssues).flat(), this.tsvFile.file, { tsvLine: rowIndex }), ) diff --git a/common/issues/data.js b/common/issues/data.js index 82feb739..ae17c817 100644 --- a/common/issues/data.js +++ b/common/issues/data.js @@ -73,6 +73,11 @@ export default { level: 'error', message: stringTemplate`Descendant tag required - "${'tag'}".`, }, + valueRequired: { + hedCode: 'TAG_REQUIRES_CHILD', + level: 'error', + message: stringTemplate`Tag "${'tag'}" requires a value.`, + }, childForbidden: { hedCode: 'TAG_INVALID', level: 'error', @@ -124,11 +129,16 @@ export default { level: 'error', message: stringTemplate`Illegal nested definition in tag group for definition "${'definition'}".`, }, - missingDefinition: { + missingDefinitionForDef: { hedCode: 'DEF_INVALID', level: 'error', message: stringTemplate`Def tag found for definition name "${'definition'}" does not correspond to an existing definition.`, }, + missingDefinitionForDefExpand: { + hedCode: 'DEF_EXPAND_INVALID', + level: 'error', + message: stringTemplate`Def-expand tag found for definition name "${'definition'}" does not correspond to an existing definition.`, + }, duplicateDefinition: { hedCode: 'DEFINITION_INVALID', level: 'error', diff --git a/data/json/specialTags.json b/data/json/specialTags.json index b52afabe..2f8c8412 100644 --- a/data/json/specialTags.json +++ b/data/json/specialTags.json @@ -1,7 +1,7 @@ { "Def": { "name": "Def", - "allowExtension": false, + "noExtension": true, "allowValue": true, "allowTwoLevelValue": true, "requireValue": true, @@ -19,7 +19,7 @@ }, "Def-expand": { "name": "Def-expand", - "allowExtension": false, + "noExtension": true, "allowValue": true, "allowTwoLevelValue": true, "requireValue": true, @@ -37,7 +37,7 @@ }, "Definition": { "name": "Definition", - "allowExtension": false, + "noExtension": true, "allowValue": true, "allowTwoLevelValue": true, "requireValue": true, @@ -55,7 +55,7 @@ }, "Delay": { "name": "Delay", - "allowExtension": false, + "noExtension": true, "allowValue": true, "allowTwoLevelValue": false, "requireValue": true, @@ -73,7 +73,7 @@ }, "Duration": { "name": "Duration", - "allowExtension": false, + "noExtension": true, "allowValue": true, "allowTwoLevelValue": false, "requireValue": true, @@ -91,7 +91,7 @@ }, "Event-context": { "name": "Event-context", - "allowExtension": false, + "noExtension": true, "allowValue": false, "allowTwoLevelValue": false, "requireValue": false, @@ -109,7 +109,7 @@ }, "Inset": { "name": "Inset", - "allowExtension": false, + "noExtension": true, "allowValue": false, "allowTwoLevelValue": false, "requireValue": false, @@ -127,7 +127,7 @@ }, "Offset": { "name": "Offset", - "allowExtension": false, + "noExtension": true, "allowValue": false, "allowTwoLevelValue": false, "requireValue": false, @@ -145,7 +145,7 @@ }, "Onset": { "name": "Onset", - "allowExtension": false, + "noExtension": true, "allowValue": false, "allowTwoLevelValue": false, "requireValue": false, diff --git a/parser/parsedHedGroup.js b/parser/parsedHedGroup.js index 7311ffac..f5092af0 100644 --- a/parser/parsedHedGroup.js +++ b/parser/parsedHedGroup.js @@ -5,7 +5,6 @@ import { getTagName } from '../utils/hedStrings' import ParsedHedSubstring from './parsedHedSubstring' import ParsedHedTag from './parsedHedTag' import ParsedHedColumnSplice from './parsedHedColumnSplice' -import { SpecialChecker } from './special' /** * A parsed HED tag group. @@ -272,28 +271,29 @@ export default class ParsedHedGroup extends ParsedHedSubstring { }) } - get specialTagList() { + /* get specialTagList() { return this._memoize('specialTagList', () => { const special = new SpecialChecker() return this.allTags.filter((obj) => special.specialNames.includes(obj.schemaTag.name)) }) - } + }*/ - get hasForbiddenSubgroupTags() { + /* get hasForbiddenSubgroupTags() { return this._memoize('hasForbiddenSubgroupTags', () => { return this.allTags.some((obj) => new SpecialChecker().hasForbiddenSubgroupTags.includes(obj.schemaTag.name)) }) - } - /** + }*/ + + /* /!** * A list of all column splices at all levels * * @returns {ParsedHedColumnSplice[]} The "name" portion of the canonical tag. - */ + *!/ get allColumnSplices() { return this._memoize('allColumnSplices', () => { return Array.from(this.columnSpliceIterator()) }) - } + }*/ /** * Determine the name of this group's definition. diff --git a/parser/parsedHedString.js b/parser/parsedHedString.js index dc2f0c2b..5c756eb1 100644 --- a/parser/parsedHedString.js +++ b/parser/parsedHedString.js @@ -52,12 +52,6 @@ export class ParsedHedString { */ context - /** - * A list of the special tags that are in the string - * @type {ParsedHedTag[]} - */ - specialTags - /** * Constructor. * @param {string} hedString The original HED string. diff --git a/parser/parsedHedTag.js b/parser/parsedHedTag.js index 68d72628..4ca4260a 100644 --- a/parser/parsedHedTag.js +++ b/parser/parsedHedTag.js @@ -1,13 +1,11 @@ import { IssueError } from '../common/issues/issues' -import { getTagLevels } from '../utils/hedStrings' import ParsedHedSubstring from './parsedHedSubstring' import { SchemaValueTag } from '../schema/entries' import TagConverter from './tagConverter' +import { SpecialChecker } from './special' const allowedRegEx = /^[^{}\,]*$/ -//TODO This is temporary until special tag handling is available. -const threeLevelTags = ['Definition', 'Def', 'Def-expand'] /** * A parsed HED tag. */ @@ -60,9 +58,9 @@ export default class ParsedHedTag extends ParsedHedSubstring { _value /** - * If definition + * If definition this is the second value if * - * @type {Array} + * @type {string} * @private */ _splitValue @@ -121,22 +119,30 @@ export default class ParsedHedTag extends ParsedHedSubstring { } /** - * Handle the remainder portion + * Handle the remainder portion for value tag (converter handles others) * + * @param (SchemaTag) schemaTag - the part of the tag that is in the schema + * @param {string} remainder - the leftover part * @throws {IssueError} If parsing the remainder section fails. */ _handleRemainder(schemaTag, remainder) { - if (remainder === '' || !(schemaTag instanceof SchemaValueTag)) { - this._extension = remainder + if (!(schemaTag instanceof SchemaValueTag)) { return } - if (threeLevelTags.includes(this.schemaTag.name)) { - this._handleSpecial(remainder) - return + // Check that there is a value if required + const special = SpecialChecker.getInstance() + if ( + (schemaTag.hasAttributeName('requireChild') || special.requireValueTags.includes(schemaTag.name)) && + remainder === '' + ) { + IssueError.generateAndThrow('valueRequired', { tag: this.originalTag }) } - this._splitValue = null + // Check if this could have a two-level value + const [value, rest] = this._getSplitValue(remainder, special) + this._splitValue = rest - const [actualUnit, actualUnitString, actualValueString] = this._separateUnits(schemaTag, remainder) + // Resolve the units and check + const [actualUnit, actualUnitString, actualValueString] = this._separateUnits(schemaTag, value) this._units = actualUnit this._value = actualValueString @@ -162,18 +168,18 @@ export default class ParsedHedTag extends ParsedHedSubstring { return [actualUnit, actualUnitString, actualValueString] } - // TODO: Fix this /** - * Handle special -- handles special three-level tags + * Handle special three-level tags + * @param {string} remainder - the remainder of the tag string after schema tag + * @param {SpecialChecker} special - the special checker for checking the special tag properties */ - _handleSpecial(remainder) { - const splitValue = remainder.split('/', 2) - const entryManager = this.schema.entries.valueClasses - if (entryManager.getEntry('nameClass').validateValue(splitValue[0])) { - this._splitValue = splitValue - } else { - IssueError.generateAndThrow('invalidValue', { tag: this.originalTag }) + _getSplitValue(remainder, special) { + if (!special.allowTwoLevelValueTags.includes(this.schemaTag.name)) { + return [remainder, null] } + const split = remainder.split('/', 2) + const rest = split.length > 1 ? split[1] : null + return [split[0], rest] } /** @@ -247,27 +253,29 @@ export default class ParsedHedTag extends ParsedHedSubstring { } } - /** + /* + /!** * The trailing portion of {@link canonicalTag}. * * @returns {string} The "name" portion of the canonical tag. - */ + *!/ get canonicalTagName() { return this._memoize('canonicalTagName', () => { return ParsedHedTag.getTagName(this.canonicalTag) }) - } + }*/ - /** + /* + /!** * The trailing portion of {@link formattedTag}. * * @returns {string} The "name" portion of the formatted tag. - */ + *!/ get formattedTagName() { return this._memoize('formattedTagName', () => { return ParsedHedTag.getTagName(this.formattedTag) }) - } + }*/ /** * The trailing portion of {@link originalTag}. @@ -317,16 +325,17 @@ export default class ParsedHedTag extends ParsedHedSubstring { }) } - /** + /* + /!** * The parent portion of {@link originalTag}. * * @returns {string} The "parent" portion of the original tag. - */ + *!/ get parentOriginalTag() { return this._memoize('parentOriginalTag', () => { return ParsedHedTag.getParentTag(this.originalTag) }) - } + }*/ /** * Iterate through a tag's ancestor tag strings. @@ -363,11 +372,12 @@ export default class ParsedHedTag extends ParsedHedSubstring { return false } - /** + /* + /!** * Check if any level of this HED tag allows extensions. * * @returns {boolean} Whether any level of this HED tag allows extensions. - */ + *!/ get allowsExtensions() { return this._memoize('allowsExtensions', () => { if (this.originalTagName === '#') { @@ -381,7 +391,7 @@ export default class ParsedHedTag extends ParsedHedSubstring { this.schema?.tagHasAttribute(tagSubstring, extensionAllowedAttribute), ) }) - } + }*/ /** * Determine if this HED tag is equivalent to another HED tag. diff --git a/parser/parser.js b/parser/parser.js index f0eb54d3..6c468b77 100644 --- a/parser/parser.js +++ b/parser/parser.js @@ -48,7 +48,7 @@ class HedStringParser { return [null, parsingIssues] } const parsedString = new ParsedHedString(this.hedString, parsedTags) - const checkIssues = new SpecialChecker().checkHedString(parsedString, fullCheck) + const checkIssues = SpecialChecker.getInstance().checkHedString(parsedString, fullCheck) mergeParsingIssues(parsingIssues, { syntaxIssues: checkIssues }) if (checkIssues.length > 0) { return [null, parsingIssues] diff --git a/parser/special.js b/parser/special.js index ea192f9b..bd345726 100644 --- a/parser/special.js +++ b/parser/special.js @@ -1,108 +1,81 @@ import specialTags from '../data/json/specialTags.json' -import ParsedHedGroup from './parsedHedGroup' -import ParsedHedTag from './parsedHedTag' -import ParsedHedColumnSplice from './parsedHedColumnSplice' import { generateIssue } from '../common/issues/issues' -import { TagSpec } from './tokenizer' -//import fs from 'fs' - -//const readFileSync = fs.readFileSync -//const specialJson = readFileSync('../data/json/specialTags.json', 'utf8') -const specialMap = new Map(Object.entries(specialTags)) export class SpecialChecker { - /** - * Map of properties for special tags. - * @type {Object} - */ - specialMap - specialNames - specialGroupTags - specialTopGroupTags - exclusiveTags - noSpliceInGroup - hasForbiddenSubgroupTags - - /** - * Array of the special tags in this HED string - * @type {ParsedHedTag []} - */ - specialTags + static instance = null + static specialMap = new Map(Object.entries(specialTags)) constructor() { if (SpecialChecker.instance) { - // eslint-disable-next-line no-constructor-return - return SpecialChecker.instance + throw new Error('Use SpecialChecker.getInstance() to get an instance of this class.') } - this.specialMap = specialMap - this.specialNames = Array.from(specialMap.keys()) - this.specialGroupTags = Array.from(this.specialMap.values()) - .filter((value) => value.tagGroup === true) - .map((value) => value.name) - this.specialTopGroupTags = Array.from(this.specialMap.values()) - .filter((value) => value.topLevelTagGroup === true) - .map((value) => value.name) - this.exclusiveTags = Array.from(this.specialMap.values()) - .filter((value) => value.exclusive === true) - .map((value) => value.name) - this.noSpliceInGroup = Array.from(this.specialMap.values()) - .filter((value) => value.noSpliceInGroup === true) - .map((value) => value.name) - this.hasForbiddenSubgroupTags = Array.from(this.specialMap.values()) - .filter((value) => value.forbiddenSubgroupTags.length > 0) - .map((value) => value.name) - // eslint-disable-next-line no-constructor-return - return this + + this._initializeSpecialTags() } - /** - * Do the checks that can be done a parse time to provide an early out. - * @param {ParsedHedString} hedString to be checked for syntactical violations - * @param {boolean} fullCheck if true may assume that all splices have been resolved in the string - * @returns {Issues[]|[]} - */ - checkHedString(hedString, fullCheck) { - let issues = this.spliceCheck(hedString, fullCheck) - if (issues.length > 0) { - return issues - } - issues = this.checkTagGroupLevels(hedString, fullCheck) - if (issues.length > 0) { - return issues + // Static method to control access to the singleton instance + static getInstance() { + if (!SpecialChecker.instance) { + SpecialChecker.instance = new SpecialChecker() } + return SpecialChecker.instance + } - issues = this.checkUnique(hedString) // Check if more than one tag with the unique attribute - if (issues.length > 0) { - return issues - } + _initializeSpecialTags() { + this.specialNames = [...SpecialChecker.specialMap.keys()] + this.requireValueTags = SpecialChecker._getSpecialTagsByProperty('requireValue') + this.noExtensionTags = SpecialChecker._getSpecialTagsByProperty('noExtension') + this.allowTwoLevelValueTags = SpecialChecker._getSpecialTagsByProperty('allowTwoLevelValue') + this.specialGroupTags = SpecialChecker._getSpecialTagsByProperty('tagGroup') + this.specialTopGroupTags = SpecialChecker._getSpecialTagsByProperty('topLevelTagGroup') + this.exclusiveTags = SpecialChecker._getSpecialTagsByProperty('exclusive') + this.noSpliceInGroup = SpecialChecker._getSpecialTagsByProperty('noSpliceInGroup') + this.hasForbiddenSubgroupTags = new Set( + [...SpecialChecker.specialMap.values()] + .filter((value) => value.forbiddenSubgroupTags.length > 0) + .map((value) => value.name), + ) + } - // Now get down to checking special tags - if (!fullCheck && !hedString.tags.some((tag) => this.specialNames.includes(tag.schemaTag.name))) { - return [] - } + static _getSpecialTagsByProperty(property) { + return [...SpecialChecker.specialMap.values()] + .filter((value) => value[property] === true) + .map((value) => value.name) + } - issues = this.checkExclusive(hedString) - if (issues.length > 0) { - return issues - } - issues = this.checkSpecialTopGroups(hedString, fullCheck) - if (issues.length > 0) { - return issues - } + /** + * Perform syntactical checks on the provided HED string to detect violations. + * + * @param {ParsedHedString} hedString - The HED string to be checked. + * @param {boolean} fullCheck - If true, assumes that all splices have been resolved. + * @returns {Issue[]} An array of issues if violations are found otherwise, an empty array. + */ + checkHedString(hedString, fullCheck) { + const checks = [ + () => this.spliceCheck(hedString, fullCheck), + () => this.checkTagGroupLevels(hedString, fullCheck), + () => this.checkUnique(hedString), + () => this.checkExclusive(hedString), + () => this.checkSpecialTopGroups(hedString), + () => this.checkNoSpliceInGroupTags(hedString), + () => this.checkForbiddenGroups(hedString), + ] - issues = this.checkNoSpliceInGroupTags(hedString) - if (issues.length > 0) { - return issues + for (const check of checks) { + const issues = check() + if (issues.length > 0) { + return issues + } } - - return this.checkForbiddenGroups(hedString.tagGroups) + return [] } /** * Check whether column splices are allowed - * @param {ParsedHedString} - hed string to be checked for splice conflicts - * @param {boolean} fullCheck - if true, then column splices should have been resolved - * @returns {Issue[]|[]} - issues with splices that can be detected + * + * @param {ParsedHedString} hedString - The HED string to check for splice conflicts. + * @param {boolean} fullCheck - If true, then column splices should have been resolved. + * @returns {Issue[]} An array of `Issue` objects if there are violations; otherwise, an empty array. */ spliceCheck(hedString, fullCheck) { if (hedString.columnSplices.length === 0) { @@ -111,7 +84,7 @@ export class SpecialChecker { } // If doing a full-check, column splices should be resolved - if (fullCheck || hedString.tags.some((obj) => this.exclusiveTags.includes(obj.schemaTag._name))) { + if (fullCheck || hedString.tags.some((tag) => this.exclusiveTags.includes(tag.schemaTag._name))) { return [generateIssue('curlyBracesNotAllowed', { string: hedString.hedString })] } return [] @@ -120,27 +93,30 @@ export class SpecialChecker { /** * Check whether tags are not in groups -- or top-level groups as required * - * @param {ParsedHedString} hedString -- parsed object to be checked for special tag syntax - * @param {boolean} fullCheck -- if true, can assume that no column splices are around. - * @returns {issues[]} -- the issues that occurred. + * @param {ParsedHedString} hedString - The HED string to be checked for special tag syntax. + * @param {boolean} fullCheck - If true, can assume that no column splices are around. + * @returns {Issue[]} An array of `Issue` objects if there are violations; otherwise, an empty array. */ checkTagGroupLevels(hedString, fullCheck) { const issues = [] const topGroupTags = hedString.topLevelGroupTags.flat() hedString.tags.forEach((tag) => { - // Iterating over all tags in the string + // Check for top-level violations because tag is deep if (this.hasTopLevelTagGroupAttribute(tag)) { - // Check for top-level violations because tag is deep + //Tag is in a top-level tag group if (topGroupTags.includes(tag)) { - //Tag is in a top-level tag group return - } else if (!hedString.topLevelTags.includes(tag) || (fullCheck && hedString.topLevelTags.includes(tag))) { + } + + // Check the top-level tag requirements + if (!hedString.topLevelTags.includes(tag) || (fullCheck && hedString.topLevelTags.includes(tag))) { issues.push( generateIssue('invalidTopLevelTagGroupTag', { tag: tag.originalTag, string: hedString.hedString }), ) return } } + // In final form --- if not in a group (not just a top group) but has the group tag attribute if (fullCheck && hedString.topLevelTags.includes(tag) && this.hasGroupAttribute(tag)) { issues.push(generateIssue('missingTagGroup', { tag: tag.originalTag, string: hedString.hedString })) @@ -151,13 +127,14 @@ export class SpecialChecker { /** * Check the exclusive property (so far only for Definitions) - only groups of same kind allowed in string - * @param hedString {parsedHedString} - the object to be checked - * @returns {Issue[]|[]} + * + * @param hedString {ParsedHedString} - the HED string to be checked. + * @returns {Issue[]} An array of `Issue` objects if there are violations; otherwise, an empty array. * * Notes: Can only be in a top group and with other top groups of the same kind */ checkExclusive(hedString) { - const exclusiveTags = hedString.tags.filter((obj) => this.exclusiveTags.includes(obj.schemaTag._name)) + const exclusiveTags = hedString.tags.filter((tag) => this.exclusiveTags.includes(tag.schemaTag._name)) if (exclusiveTags.length === 0) { return [] } @@ -173,7 +150,7 @@ export class SpecialChecker { } // Make sure that all the objects in exclusiveTags have same schema tag name - not an issue currently - const badList = exclusiveTags.filter((obj) => obj.schemaTag._name != exclusiveTags[0].schemaTag._name) + const badList = exclusiveTags.filter((tag) => tag.schemaTag._name !== exclusiveTags[0].schemaTag._name) if (badList.length > 0) { return [generateIssue('illegalExclusiveContext', { tag: badList[0].originalTag, string: hedString.hedString })] } @@ -181,104 +158,143 @@ export class SpecialChecker { } /** - * Check the group conditions of the special tags. The top-level has already been verified - * @param {ParsedHedString }hedString - * @returns {Issues[]|[]} + * Check the group conditions of the special tags. The top-level has already been verified. + * + * @param {ParsedHedString} hedString - The HED string to check for group conflicts. + * @returns {Issue[]} An array of `Issue` objects if there are violations; otherwise, an empty array. * * Notes: These include the number of groups and top tag compatibility in the group */ checkSpecialTopGroups(hedString) { const issues = [] for (const group of hedString.tagGroups) { - const nextIssues = this.checkSpecialTopGroup(group) + const nextIssues = this._checkSpecialTopGroup(group) issues.push(...nextIssues) } return issues } /** - * Check special group requirements for special top-level tags - * @param {ParsedHedGroup} group to be checked for requirements - * @returns {Issue[]|[]} + * Check special group requirements for top-level tags in a parsed HED group. + * + * This method verifies whether the given group complies with specific requirements + * for top-level tags, focusing on groups with special properties such as + * 'Def' or 'Def-expand'. It ensures that group combinations are valid and that + * restrictions on subgroup relationships are respected. + * + * @param {ParsedHedGroup} group - The parsed HED group containing tags to be validated. + * @returns {Issue[]} An array of `Issue` objects if violations are found; otherwise, an empty array. * * Note: This is a top-group check only */ - checkSpecialTopGroup(group) { - const specialTags = group.topTags.filter((obj) => this.specialTopGroupTags.includes(obj.schemaTag.name)) + _checkSpecialTopGroup(group) { + const specialTags = group.topTags.filter((tag) => this.specialTopGroupTags.includes(tag.schemaTag.name)) + + // If there are no special tags, there are no issues to check if (specialTags.length === 0) { return [] - } else if (specialTags.length >= group.topTags.length + 1) { + } + + // Ensure that groups with special tags can only contain other special tags or a Def tag. + if (specialTags.length > group.topTags.length) { // ASSUME that groups with special tags can only have other tags which are special or a Def //TODO fix when map is removed from specialTags - return [ - generateIssue('invalidTagGroup', { tags: this.getTagListString(group.topTags), string: group.originalTag }), - ] - } else if (group.isDefExpandGroup && group.topTags.length === 1 && group.topGroups.length <= 1) { - // This group is a valid Def-expand group, and we don't have to check it further + return [generateIssue('invalidTagGroup', { tagGroup: group.originalTag })] + } + + // Validate Def-expand groups: must have only one tag and limited subgroups. + if (group.isDefExpandGroup && group.topTags.length === 1 && group.topGroups.length <= 1) { return [] - } else if (group.isDefExpandGroup || (group.hasDefExpandChildren && group.defExpandChildren.length > 1)) { - // It is an invalid Def-expand group or too many Def-expand subgroups - return [ - generateIssue('invalidTagGroup', { tags: this.getTagListString(group.topTags), string: group.originalTag }), - ] - } else if (group.hasDefExpandChildren && group.topTags.some((obj) => obj.schemaTag.name === 'Def')) { - // It has both a Def tag and a Def-expand group --- which currently are not allowed - return [ - generateIssue('invalidTagGroup', { tags: this.getTagListString(group.topTags), string: group.originalTag }), - ] } - return this._checkSpecialTopGroup(group, specialTags) + + // Check if group is an invalid Def-expand group or has too many Def-expand subgroups. + if (group.isDefExpandGroup || (group.hasDefExpandChildren && group.defExpandChildren.length > 1)) { + return [generateIssue('invalidTagGroup', { tagGroup: group.originalTag })] + } + + // Ensure a group does not contain both a Def tag and a Def-expand group, which is disallowed. + if (group.hasDefExpandChildren && group.topTags.some((tag) => tag.schemaTag.name === 'Def')) { + return [generateIssue('invalidTagGroup', { tagGroup: group.originalTag })] + } + + // Delegate to check special tags in the group for further validation. + return this._checkSpecialTagsInGroup(group, specialTags) } /** - * Check the details after the guard conditions have been handled - * @param group - * @returns {[]|[Issue]} + * Check the compatibility of special tags within a group after the initial guard conditions have been handled. + * + * This function verifies whether special tags in the provided group meet the required constraints. + * Specifically, it checks if tags are allowed based on group properties, if there are any duplicate names, + * and if certain required tags or group conditions are missing. + * + * @param {ParsedHedGroup} - The HED group object containing tags to be validated. + * @param {ParsedHedTag[]} specialTags - Tags within the group that have special properties requiring validation. + * @returns {Issue[]} An array of `Issue` objects if there are violations; otherwise, an empty array. * @private */ - _checkSpecialTopGroup(group, specialTags) { - const hasDef = group.topTags.some((obj) => obj.schemaTag.name === 'Def') || group.hasDefExpandChildren - for (const sTag of specialTags) { - const sTagReqs = this.specialMap.get(sTag.schemaTag.name) - const otherTags = group.topTags.filter((obj) => obj !== sTag) - const notAllowed = otherTags.filter((obj) => !sTagReqs.otherAllowedTags?.includes(obj.schemaTag.name)) - if (notAllowed.length > 0 || this.hasDuplicateNames(otherTags)) { - // Make sure that there are not any duplicates in the allowed tags - return [ - generateIssue('invalidTagGroup', { tags: this.getTagListString(group.topTags), string: group.originalTag }), - ] - } + _checkSpecialTagsInGroup(group, specialTags) { + const hasDef = group.topTags.some((tag) => tag.schemaTag.name === 'Def') || group.hasDefExpandChildren + + for (const specialTag of specialTags) { + const specialRequirements = SpecialChecker.specialMap.get(specialTag.schemaTag.name) + const otherTags = group.topTags.filter((tag) => tag !== specialTag) + + // Check for disallowed tags in the group + const disallowedTags = this._getDisallowedTags(otherTags, specialRequirements) - if (!hasDef && sTagReqs.defTagRequired && group.topColumnSplices.length === 0) { - // Make sure that the tag has a required definition grouped with it - return [generateIssue('temporalWithoutDefinition', { tag: sTag.schemaTag.name, tagGroup: group.originalTag })] + // Make sure that there are not any duplicates in the allowed tags + if (disallowedTags.length > 0 || this.hasDuplicateNames(otherTags)) { + return [generateIssue('invalidTagGroup', { tagGroup: group.originalTag })] } - const defExpandCount = sTagReqs.defTagRequired && group.hasDefExpandChildren ? 1 : 0 - const maxReq = (sTagReqs.maxNonDefSubgroups ?? Infinity) + defExpandCount - if (group.topGroups.length > maxReq) { + // Check if required definition tag is missing + if (!hasDef && specialRequirements.defTagRequired && group.topColumnSplices.length === 0) { return [ - generateIssue('invalidTagGroup', { tags: this.getTagListString(group.topTags), string: group.originalTag }), + generateIssue('temporalWithoutDefinition', { + tag: specialTag.schemaTag.name, + tagGroup: group.originalTag, + }), ] } - if (group.topColumnSplices.length === 0 && group.topGroups.length < sTagReqs.minNonDefSubgroups) { - // If no column splices, the minimum number is known - return [ - generateIssue('invalidTagGroup', { tags: this.getTagListString(group.topTags), string: group.originalTag }), - ] + + // Check for the maximum number of subgroups allowed + const defExpandCount = specialRequirements.defTagRequired && group.hasDefExpandChildren ? 1 : 0 + const maxRequired = (specialRequirements.maxNonDefSubgroups ?? Infinity) + defExpandCount + if (group.topGroups.length > maxRequired) { + return [generateIssue('invalidTagGroup', { tagGroup: group.originalTag })] + } + + // Check if no column splices so the minimum number of groups can be verified + if (group.topColumnSplices.length === 0 && group.topGroups.length < specialRequirements.minNonDefSubgroups) { + return [generateIssue('invalidTagGroup', { tagGroup: group.originalTag })] } } return [] } /** - * This checks if there are conflicting subgroup tags. - * @param {ParsedHedGroup} topGroups - the top groups in the HedString being checkec - * @returns {Issues[]} + * Get tags that are not allowed in the current group. + * + * @param {ParsedHedTag[]} tags - The HED tags to be evaluated. + * @param {Object} specialTagRequirements - The requirements for the special tag. + * @returns {ParsedHedTag[]} An array of tags that are not allowed. + * @private + */ + _getDisallowedTags(tags, specialTagRequirements) { + return tags.filter((tag) => !specialTagRequirements.otherAllowedTags?.includes(tag.schemaTag.name)) + } + + /** + * Check if there are conflicting subgroup tags. + * + * @param {ParsedHedString} hedString - the HED string to be checked. + * @returns {Issue[]} An array of `Issue` objects if there are violations; otherwise, an empty array. */ - checkForbiddenGroups(topGroups) { + checkForbiddenGroups(hedString) { const issues = [] - for (const group of topGroups) { - if (group.hasForbiddenSubgroupTags) { + for (const group of hedString.tagGroups) { + // Only check the group if there are tags with forbidden subgroup tags + if (group.allTags.some((tag) => this.hasForbiddenSubgroupTags.has(tag.schemaTag.name))) { issues.push(...this.checkForbiddenGroup(group)) } } @@ -286,35 +302,34 @@ export class SpecialChecker { } /** - * Check a group completely for forbidden tag conflicts such as a Def in a Definition group - * @param {ParsedHedGroup} group check for - * @returns {Issue[]| []} + * Check a group completely for forbidden tag conflicts such as a Def in a Definition group. + * + * @param {ParsedHedGroup} group - HED group to check for forbidden group conflicts. + * @returns {Issue[]} An array of `Issue` objects if there are violations; otherwise, an empty array. * * Note: Returns in a given group as soon as it finds a conflict */ checkForbiddenGroup(group) { for (const subGroup of group.subParsedGroupIterator()) { - // Iterator includes this group - const forbiddenTags = subGroup.topTags.filter((obj) => - this.hasForbiddenSubgroupTags.includes(obj.schemaTag._name), - ) - // Handle the top level tags in the subgroup -- there can only be one tag if there is a forbidden tag. - if (forbiddenTags.length > 1 || (forbiddenTags.length === 1 && subGroup.topTags.length > 1)) { - return [ - generateIssue('invalidGroupTags', { - tags: this.getTagListString(subGroup.topTags), - string: subGroup.originalTag, - }), - ] + // if this group does not have top tags with forbidden subgroups -- must go deeper + const forbiddenTags = subGroup.topTags.filter((tag) => this.hasForbiddenSubgroupTags.has(tag.schemaTag._name)) + if (forbiddenTags.length === 0) { + continue } - if (forbiddenTags.length === 1) { - // We found one forbidden tag at the top level in this subgroup - const forbiddenName = forbiddenTags[0].schemaTag._name - const forbiddenSubgroupTags = this.specialMap.get(forbiddenName).forbiddenSubgroupTags - const forbidden = subGroup.allSubgroupTags.filter((obj) => forbiddenSubgroupTags.includes(obj.schemaTag._name)) - if (forbidden.length > 0) { + + // Check the tags in + for (const tag of forbiddenTags) { + const otherTags = subGroup.allTags.filter((otherTag) => otherTag !== tag) + const badTags = otherTags.filter((otherTag) => + SpecialChecker.specialMap.get(tag.schemaTag.name)?.forbiddenSubgroupTags.includes(otherTag.schemaTag.name), + ) + + if (badTags?.length > 0) { return [ - generateIssue('invalidGroupTags', { tags: this.getTagListString(forbidden), string: subGroup.originalTag }), + generateIssue('invalidGroupTags', { + tags: this.getTagListString(badTags), + string: subGroup.originalTag, + }), ] } } @@ -323,21 +338,22 @@ export class SpecialChecker { } /** - * This checks for special tags that have no splice in group that no forbidden tags are in their subgroup + * Check for special tags that have no splice in group that no forbidden tags are in their subgroup. * - * @param {ParsedHedString} hedString to be checked - * @returns {Issues[]} + * @param {ParsedHedString} hedString - the HED string to be checked. + * @returns {Issue[]} An array of `Issue` objects if there are violations; otherwise, an empty array. * * Notes: Currently these are Definition and Def-expand */ /** - * Check that no splice in group tags are not at the top level - * @param hedString - * @returns {Issue[]|[]} + * Check that no splice in group tags are not at the top level. + * + * @param {ParsedHedString} hedString - the HED string to be checked for splices at the top level. + * @returns {Issue[]} An array of `Issue` objects if there are violations; otherwise, an empty array. */ checkNoSpliceInGroupTags(hedString) { - const spliceTags = hedString.topLevelTags.filter((obj) => this.noSpliceInGroup.includes(obj.schemaTag._name)) + const spliceTags = hedString.topLevelTags.filter((tag) => this.noSpliceInGroup.includes(tag.schemaTag._name)) if (spliceTags.length > 0) { return [generateIssue('missingTagGroup', { tag: spliceTags[0].originalTag, string: hedString.hedString })] } @@ -345,9 +361,10 @@ export class SpecialChecker { } /** - * Check for tags with the unique attribute - * @param {HedString} hedString - * @returns {Issue[]|*[]} + * Check for tags with the unique attribute. + * + * @param {ParsedHedString} hedString - The HED string to be checked for tags with the unique attribute. + * @returns {Issue[]} An array of `Issue` objects if there are violations; otherwise, an empty array. */ checkUnique(hedString) { const uniqueTags = hedString.tags.filter((tag) => tag.hasAttribute('unique')) @@ -363,47 +380,78 @@ export class SpecialChecker { return [] } - hasTopLevelTagGroupAttribute(tag) { - return ( - tag.hasAttribute('topLevelTagGroup') || - (this.specialMap.has(tag.schemaTag.name) && this.specialMap.get(tag.schemaTag.name).topLevelTagGroup) - ) - } - /** - * Returns the value of a special attribute if special tag, otherwise undefined - * @param {ParsedHedTag} tag - * @param {string} attributeName - * @returns value of the property or undefined + * Return the value of a special attribute if special tag, otherwise undefined. + * + * @param {ParsedHedTag} tag - The HED tag to be checked for the attribute + * @param {string} attributeName - The name of the special attribute to check for. + * + * @returns value of the property or undefined. */ getSpecialAttributeForTag(tag, attributeName) { return this.specialMap.get(tag.schemaTag.name)?.[attributeName] } - hasGroupAttribute(tag) { - return ( - tag.hasAttribute('tagGroup') || - (this.specialMap.has(tag.schemaTag.name) && this.specialMap.get(tag.schemaTag.name).tagGroup) - ) - } - + /** + * Return true if a list of tags has any duplicate names. + * + * @param {list} - A list of ParsedHedTag objects to be checked. + * @returns {boolean} If true, indicates that there tags with duplicate names in the list. + * + */ hasDuplicateNames(list) { const seen = new Set() for (const obj of list) { if (seen.has(obj.schemaTag.name)) { - return true // Duplicate found + return true } seen.add(obj.schemaTag.name) } - return false // No duplicates + return false } /** - * Return a string of original tag names for error messages - * @param {ParsedHedTag} tagList - * @returns {string} comma separated list of original tag names for tags in tagList + * Return a string of original tag names for error messages. + * @param {ParsedHedTag} tagList - The HED tags whose string representations should be put in a comma-separated list. + * @returns {string} A comma separated list of original tag names for tags in tagList. */ getTagListString(tagList) { return tagList.map((tag) => tag.toString()).join(', ') } + + _hasExclusiveTags(hedString) { + return hedString.tags.some((tag) => this.exclusiveTags.includes(tag.schemaTag._name)) + } + + /** + * Indicate whether a tag should be a top-level tag. + * + * @param {ParsedHedTag} tag - HED tag to check for top-level requirements. + * @returns {boolean} If true, the tag is required to be at the top level. + * + * Note: This check both the special requirements and the 'topLevelTagGroup' attribute in the schema. + * + */ + hasTopLevelTagGroupAttribute(tag) { + return ( + tag.hasAttribute('topLevelTagGroup') || + (SpecialChecker.specialMap.has(tag.schemaTag.name) && + SpecialChecker.specialMap.get(tag.schemaTag.name).topLevelTagGroup) + ) + } + + /** + * Return a boolean indicating whether a tag is required to be in a tag group. + * + * @param {ParsedHedTag} tag - The HED tag to be checked. + * @returns {boolean} If true, this indicates that tag must be in a tag group. + * + * Note: This checks both special and schema tag requirements. + */ + hasGroupAttribute(tag) { + return ( + tag.hasAttribute('tagGroup') || + (SpecialChecker.specialMap.has(tag.schemaTag.name) && SpecialChecker.specialMap.get(tag.schemaTag.name).tagGroup) + ) + } } diff --git a/parser/splitter.js b/parser/splitter.js index 268567bf..c2f514bb 100644 --- a/parser/splitter.js +++ b/parser/splitter.js @@ -5,7 +5,7 @@ import { recursiveMap } from '../utils/array' import { mergeParsingIssues } from '../utils/hedData' import { HedStringTokenizer, ColumnSpliceSpec, TagSpec } from './tokenizer' import { generateIssue, IssueError } from '../common/issues/issues' -import { Schemas } from '../schema/containers' +import { SpecialChecker } from './special' export default class HedStringSplitter { /** @@ -40,6 +40,7 @@ export default class HedStringSplitter { this.hedSchemas = hedSchemas this.conversionIssues = [] this.syntaxIssues = [] + this.special = SpecialChecker.getInstance() } /** diff --git a/parser/tagConverter.js b/parser/tagConverter.js index c38a6a56..d56fa6d9 100644 --- a/parser/tagConverter.js +++ b/parser/tagConverter.js @@ -1,5 +1,6 @@ import { IssueError } from '../common/issues/issues' import { getTagSlashIndices } from '../utils/hedStrings' +import { SpecialChecker } from './special' /** * Converter from a tag specification to a schema-based tag object. @@ -61,6 +62,7 @@ export default class TagConverter { this.tagLevels = this.tagString.split('/') this.tagSlashes = getTagSlashIndices(this.tagString) this.remainder = undefined + this.special = SpecialChecker.getInstance() } /** @@ -73,7 +75,7 @@ export default class TagConverter { let parentTag = undefined for (let tagLevelIndex = 0; tagLevelIndex < this.tagLevels.length; tagLevelIndex++) { if (parentTag?.valueTag) { - // Its a value tag + // It is a value tag this._setSchemaTag(parentTag.valueTag, tagLevelIndex) return [this.schemaTag, this.remainder] } @@ -96,9 +98,11 @@ export default class TagConverter { // Top level tags can't be extensions IssueError.generateAndThrow('invalidTag', { tag: this.tagString }) } - if (parentTag !== undefined && !parentTag.hasAttributeName('extensionAllowed')) { + if ( + parentTag !== undefined && + (!parentTag.hasAttributeName('extensionAllowed') || this.special.noExtensionTags.includes(parentTag.name)) + ) { IssueError.generateAndThrow('invalidExtension', { - // The parent doesn't allow extension tag: this.tagLevels[tagLevelIndex], parentTag: this.tagLevels.slice(0, tagLevelIndex).join('/'), }) @@ -125,7 +129,7 @@ export default class TagConverter { if (child !== undefined) { // A schema tag showed up after a non-schema tag IssueError.generateAndThrow('invalidParentNode', { - tag: this.tagLevels[index], + tag: child.name, parentTag: this.tagLevels.slice(0, index).join('/'), }) } @@ -133,6 +137,23 @@ export default class TagConverter { } } + /* /!** + * Handle the case where it does not allow an extension or if it requires a child and doesn't have one. + * @param {int} tagLevelIndex index of the tag + * @throws {IssueError} If the tag has an extension that is not allowed. + *!/ + _checkExtensionRequirements(tagLevelIndex) { + // Check allow extension or requires a child + const schemaTag = this.getSchemaTag(tagLevelIndex - 1) + const remainder = this.tagLevels.slice(tagLevelIndex).join('/') + if (this.special.noExtension.includes(schemaTag.name) && remainder !== '') { + IssueError.generateAndThrow('invalidExtension',{tag: remainder, parentTag: schemaTag.name}) + } + if (remainder === '' && schemaTag.hasAttributeName('requireChild')) ( + IssueError.generateAndThrow('childRequired', {tag:schemaTag.name}) + ) + }*/ + _getSchemaTag(tagLevelIndex) { const tagLevel = this.tagLevels[tagLevelIndex].toLowerCase() return this.tagMapping.getEntry(tagLevel) diff --git a/parser/tempRegex.js b/parser/tempRegex.js deleted file mode 100644 index 4937ea31..00000000 --- a/parser/tempRegex.js +++ /dev/null @@ -1,25 +0,0 @@ -import regexData from '../data/json/class_regex.json' - -// Function to get the RegExp -export function getRegExp(name) { - if (!regexData.class_chars[name]) { - throw new Error(`Invalid class name: ${name}`) - } - - const charNames = regexData.class_chars[name] - if (charNames.length === 0) { - throw new Error(`No character definitions for class: ${name}`) - } - - // Join the individual character regex patterns - const pattern = charNames - .map((charName) => { - if (!regexData.char_regex[charName]) { - throw new Error(`Invalid character name: ${charName}`) - } - return regexData.char_regex[charName] - }) - .join('|') - - return new RegExp(`^(?:${pattern})+$`) -} diff --git a/parser/tokenizer.js b/parser/tokenizer.js index dc99d492..dcdfb4fb 100644 --- a/parser/tokenizer.js +++ b/parser/tokenizer.js @@ -341,7 +341,7 @@ export class HedStringTokenizer { } pushTag(i) { - if (this.state.currentToken.trim().length == 0) { + if (this.state.currentToken.trim().length === 0) { this.pushIssue('emptyTagFound', i) } else if (this.checkForBadPlaceholderIssues(i)) { this.pushInvalidTag('invalidPlaceholder', i, this.state.currentToken) @@ -365,15 +365,11 @@ export class HedStringTokenizer { // No placeholders to worry about for this tag return false } - if ( + return ( tokenSplit.length > 2 || !tokenSplit[0].endsWith(CHARACTERS.SLASH) || // A placeholder must be after a slash (tokenSplit[1].trim().length > 0 && tokenSplit[1][0] !== CHARACTERS.BLANK) - ) { - // If units, blank after placeholder - return true - } - return false + ) } closeGroup(i) { diff --git a/schema/entries.js b/schema/entries.js index 8f03bc7b..5d82a98c 100644 --- a/schema/entries.js +++ b/schema/entries.js @@ -819,7 +819,7 @@ export class SchemaTag extends SchemaEntryWithAttributes { * @type {SchemaUnitClass[]} */ get unitClasses() { - return this._unitClasses.slice() + return this._unitClasses.slice() // The slice prevents modification } /** diff --git a/schema/parser.js b/schema/parser.js index 14d32ec3..a5993a2e 100644 --- a/schema/parser.js +++ b/schema/parser.js @@ -24,7 +24,7 @@ import { } from './entries' import { IssueError } from '../common/issues/issues' -const specialTags = require('../data/json/specialTags.json') +//const specialTags = require('../data/json/specialTags.json') import classRegex from '../data/json/class_regex.json' diff --git a/tests/converter.spec.js b/tests/converter.spec.js index 895c41e3..88f7fb36 100644 --- a/tests/converter.spec.js +++ b/tests/converter.spec.js @@ -199,7 +199,8 @@ describe('HED string conversion', () => { return validator(testStrings, expectedResults, expectedIssues) }) - it.skip('should validate whether a node actually allows extensions', () => { + //TODO: Remove --- these cases are handled by tagParserTests + it.skip('(REMODE)should validate whether a node actually allows extensions', () => { const testStrings = { validTakesValue: 'Property/Agent-property/Agent-trait/Age/15', cascadeExtension: 'Property/Agent-property/Agent-state/Agent-emotional-state/Awed/Cascade Extension', @@ -287,7 +288,7 @@ describe('HED string conversion', () => { const validator = function (testStrings, expectedResults, expectedIssues) { return validatorBase(testStrings, expectedResults, expectedIssues, converter.convertHedStringToLong) } - //TODO: now part of tag parsing + //TODO: Remove as it is now part of tag parsing it.skip('(REMOVE)should convert basic HED tags to long form', () => { const testStrings = { singleLevel: 'Event', @@ -313,7 +314,8 @@ describe('HED string conversion', () => { return validator(testStrings, expectedResults, expectedIssues) }) - it.skip('should convert HED tags with values to long form', () => { + //TODO: Remove as it is now part of tag parsing + it.skip('(REMOVE)should convert HED tags with values to long form', () => { const testStrings = { uniqueValue: 'Environmental-sound/Unique Value', multiLevel: 'Environmental-sound/Long Unique Value With/Slash Marks', @@ -332,7 +334,8 @@ describe('HED string conversion', () => { return validator(testStrings, expectedResults, expectedIssues) }) - it.skip('should convert HED tags with extensions to long form', () => { + //TODO: Remove as it is now part of tag parsing + it.skip('(REMOVE)should convert HED tags with extensions to long form', () => { const testStrings = { singleLevel: 'Object/extended lvl1', multiLevel: 'Object/extended lvl1/Extension2', @@ -351,7 +354,8 @@ describe('HED string conversion', () => { return validator(testStrings, expectedResults, expectedIssues) }) - it.skip('should raise an issue if an "extension" is already a valid node', () => { + //TODO: Remove as it is now part of tag parsing + it.skip('(REMOVE)should raise an issue if an "extension" is already a valid node', () => { const testStrings = { validThenInvalid: 'Object/valid extension followed by invalid/Event', singleLevel: 'Object/Visual-presentation', @@ -391,7 +395,8 @@ describe('HED string conversion', () => { return validator(testStrings, expectedResults, expectedIssues) }) - it.skip('should raise an issue if an invalid node is found', () => { + //TODO: Remove as it is now part of tag parsing + it.skip('(REMOVE) should raise an issue if an invalid node is found', () => { const testStrings = { single: 'InvalidEvent', invalidChild: 'InvalidEvent/InvalidExtension', @@ -410,7 +415,8 @@ describe('HED string conversion', () => { return validator(testStrings, expectedResults, expectedIssues) }) - it.skip('should validate whether a node actually allows extensions', () => { + //TODO: Remove as it is now part of tag parsing + it.skip('(REMOVE)should validate whether a node actually allows extensions', () => { const testStrings = { validTakesValue: 'Age/15', cascadeExtension: 'Awed/Cascade Extension', @@ -429,7 +435,8 @@ describe('HED string conversion', () => { return validator(testStrings, expectedResults, expectedIssues) }) - it.skip('should handle leading and trailing spaces correctly', () => { + //TODO: Remove as it is now part of tag parsing + it.skip('(REMOVE)should handle leading and trailing spaces correctly', () => { const testStrings = { leadingSpace: ' Environmental-sound/Unique Value', trailingSpace: 'Environmental-sound/Unique Value ', @@ -492,8 +499,8 @@ describe('HED string conversion', () => { return validator(testStrings, expectedResults, expectedIssues) }) - // TODO: Revisit - it.skip('(REVISIT) should properly handle node names in value-taking strings', () => { + // TODO: Remove as this is handled by tag parser + it.skip('(REMOVE) should properly handle node names in value-taking strings', () => { const testStrings = { valueTaking: 'Label/Red', nonValueTaking: 'Train/Car', @@ -548,7 +555,8 @@ describe('HED string conversion', () => { return validatorBase(testStrings, expectedResults, expectedIssues, converter.convertHedStringToShort) } - it('should properly convert HED strings to short form', () => { + //TODO: Remove as this is covered in string parser. + it.skip('(REMOVE) should properly convert HED strings to short form', () => { const testStrings = { singleLevel: 'Event', multiLevel: 'Event/Sensory-event', diff --git a/tests/event.spec.js b/tests/event.spec.js index 2e4baa3b..a2515f82 100644 --- a/tests/event.spec.js +++ b/tests/event.spec.js @@ -718,8 +718,8 @@ describe('HED string and event validation', () => { greenTriangleDef: [], trainDefExpand: [], yellowCubeDef: [], - invalidDef: [generateIssue('missingDefinition', { definition: 'InvalidDefinition' })], - invalidDefExpand: [generateIssue('missingDefinition', { definition: 'InvalidDefExpand' })], + invalidDef: [generateIssue('missingDefinitionForDef', { definition: 'InvalidDefinition' })], + invalidDefExpand: [generateIssue('missingDefinitionForDefExpand', { definition: 'InvalidDefExpand' })], } return validatorSemanticWithDefinitions(testStrings, testDefinitions, expectedIssues, (validator, tag) => { validator.checkForMissingDefinitions(tag, 'Def') @@ -778,8 +778,8 @@ describe('HED string and event validation', () => { ) } - // TODO: Equivalent tests now in bidsTests -- but only work for sidecar - it('should have syntactically valid definitions', () => { + // TODO: (REMOVE) Equivalent tests now in stringParser + it.skip('should have syntactically valid definitions', () => { const testStrings = { nonDefinition: 'Car', nonDefinitionGroup: '(Train/Maglev, Age/15, RGB-red/0.5)', diff --git a/tests/stringParserTests.spec.js b/tests/stringParserTests.spec.js index 6aaf393b..7d6b9774 100644 --- a/tests/stringParserTests.spec.js +++ b/tests/stringParserTests.spec.js @@ -9,12 +9,10 @@ import { generateIssue } from '../common/issues/issues' import { parseHedString, parseHedStrings } from '../parser/parser' import { parseTestData } from './testData/stringParserTests.data' import { shouldRun, getHedString } from './testUtilities' -import { SpecialChecker } from '../parser/special' -import { TagSpec } from '../parser/tokenizer' -import ParsedHedTag from '../parser/parsedHedTag' + const skipMap = new Map() const runAll = true -const runMap = new Map([['special-tag-group-tests', ['onset-with-def-expand']]]) +const runMap = new Map([['special-tag-group-tests', ['definition-with-deep-defs-inside']]]) describe('Null schema objects should cause parsing to bail', () => { it('Should not proceed if no schema and valid string', () => { @@ -67,45 +65,6 @@ describe('Parse HED string tests', () => { afterAll(() => {}) - describe('testExperiment', () => { - /*it('Should give experiment', () => { - const thisSchema = schemaMap.get('8.3.0') - // const w = new TagSpec('Speed/5 mph', 0, 10, '') - // const g = new ParsedHedTag(w, thisSchema, 'Speed/5 mph') - // console.log(g) - // const x =[g] - // // Check top-level-tag-group-tags - // const y = x.includes(g) - // let p = g.unitClasses - // console.log(p) - //const z = x.includes(g) - assert.isDefined(thisSchema, `should be defined`) - const stringIn = 'Item, Sensory-event, (Red, Blue, {help}, (Definition/Blech, (Green, Black))), (Orange, ((Definition/Blech1, (White))))' - //const stringIn = 'Item, ((Def-expand/Apple, (Purple)), (((Def-expand/Banana, (Orange)), Item)), Sensory-event), Red' - //const stringIn = 'Item/Object, (Length/5 m, (Green)), (Green, Object), (Sensory-event, Green), Red' - const [parsedString, issues] = parseHedString(stringIn, thisSchema) - console.log(issues) -*/ - }) - - // it('Should give experiment 2', () => { - // const thisSchema = schemaMap.get('8.3.0') - // assert.isDefined(thisSchema, `should be defined`) - // const [sParsed, errors, warnings] = getHedString('Red, Blue', thisSchema, true - // //const stringIn = 'Item, Sensory-event, (Red, Blue, {help}, (Definition/Blech, (Green, Black))), (Orange, ((Definition/Blech1, (White))))' - // //const stringIn = '((Def-expand/Apple, (Purple)), (((Def-expand/Banana, (Orange)), Item)), Sensory-event)' - // // const stringIn = 'Item, Sensory-event, (Red, Blue, {help}, (Definition/Blech, (Green, Black))), (Definition/Banana)' - // // const [parsedString, issues] = parseHedString(stringIn, thisSchema) - // // console.log(parsedString) - // // const special = new SpecialChecker(parsedString) - // // const tagList = special.getSpecialTags(parsedString) - // // const group = special.getTagGroup(tagList[1]) - // //const [parsedString, errorIssues, warningIssues] = getHedString(stringIn, thisSchema) - // console.log(sParsed) - // - // }) - // }) - describe.each(parseTestData)('$name : $description', ({ name, tests }) => { const testConvert = function (test) { const status = test.errors.length === 0 ? 'Expect pass' : 'Expect fail' diff --git a/tests/tagParserTests.spec.js b/tests/tagParserTests.spec.js index 668d6847..00b4b96d 100644 --- a/tests/tagParserTests.spec.js +++ b/tests/tagParserTests.spec.js @@ -16,7 +16,7 @@ import { BidsSidecar, BidsTsvFile } from '../bids' // Ability to select individual tests to run const skipMap = new Map() const runAll = true -const runMap = new Map([['valid-tags', ['valid-numeric-scientific-value']]]) +const runMap = new Map([['valid-tags', ['valid-tag-with-extension']]]) describe('TagSpec converter tests using JSON tests', () => { const schemaMap = new Map([ @@ -37,80 +37,6 @@ describe('TagSpec converter tests using JSON tests', () => { afterAll(() => {}) - /* describe('BIDS experiments', () => { - /!*it('should be able to convert', () => { - const thisSchema = schemaMap.get('8.3.0') - assert.isDefined(thisSchema, 'yes') - const hTsv = `HED\nRed\n` - let stringIssues = [] - try { - const bidsTsv = new BidsTsvFile(`events`, hTsv, { relativePath: 'string test tsv' }, [], {}) - stringIssues = bidsTsv.validate(thisSchema) - console.log(stringIssues) - } catch (e) { - console.log(stringIssues) - } - })*!/ - - /!* it('should be able to convert', () => { - const thisSchema = schemaMap.get('8.3.0') - assert.isDefined(thisSchema, 'yes') - //const definitions = ["(Definition/Acc/#, (Acceleration/#, Red))", "(Definition/MyColor, (Label/Pie))"] - const definitions = ["(Definition/MyColor, Red)"] - const defs = { definitions: { HED: { defList: definitions.join(',') } } } - const bidsSide = new BidsSidecar(`sidecar`, defs, { relativePath: 'sidecar test' }) - const sidecarIssues = bidsSide.validate(thisSchema) - console.log(sidecarIssues) - })*!/ - - }) -*/ - // TODO: Remove after refactoring of validation complete - describe.skip('TagConverter experiments', () => { - it('should be able to convert', () => { - const thisSchema = schemaMap.get('8.3.0') - assert.isDefined(thisSchema, 'yes') - - // const spec = new TagSpec('Length/5 m', 0, 10, '') - // const pTag = new ParsedHedTag(spec, thisSchema, 'Length/5 m') - - const spec = new TagSpec('Def/Apple/5', 0, 11, '') - const pTag = new ParsedHedTag(spec, thisSchema, 'Def/Apple/5') - assert.instanceOf(pTag, ParsedHedTag) - const valueAttributeNames = pTag._schemaTag.valueAttributeNames - const valueClassNames = valueAttributeNames.get('valueClass', []) - console.log(pTag) - assert.instanceOf(pTag, ParsedHedTag) - console.log(valueAttributeNames) - console.log(valueClassNames) - const valueClasses = pTag.schema.entries.valueClasses - console.log(valueClasses) - const vClass = valueClasses.hasEntry('numericClass') - console.log(vClass) - const tClass = valueClasses.getEntry('numericClass') - console.log(tClass) - const okay = tClass.validateValue('3') - console.log(okay) - const notOkay = tClass.validateValue('ab') - console.log(notOkay) - // const spec = new TagSpec('Length/5 m', 0, 10, '') - // const myCon = new TagConverter(spec, thisSchema) - // const [tag, remainder] = myCon.convert(); - // assert.instanceOf(tag, SchemaTag, 'A schema tag comes back') - // //assert.instanceOf(remainder, String, 'A string comes back') - // const unitClasses = tag.unitClasses - // let actualUnit = null - // let actualValue = null - // for (let i = 0; i < unitClasses.length; i++) { - // [actualUnit, actualValue] = unitClasses[i].extractUnits(remainder) - // if (actualUnit !== null || actualValue !== null) { - // break - // } - // } - // console.log(`actualUnit = ${actualUnit?.name} actualValue = ${actualValue}`) - }) - }) - describe.each(parsedHedTagTests)('$name : $description', ({ name, tests }) => { const hedTagTest = function (test) { const status = test.error !== null ? 'Expect fail' : 'Expect pass' diff --git a/tests/testData/bidsTests.data.js b/tests/testData/bidsTests.data.js index 203e754f..f76eb3a8 100644 --- a/tests/testData/bidsTests.data.js +++ b/tests/testData/bidsTests.data.js @@ -5,6 +5,7 @@ export const bidsTestData = [ { name: 'valid-bids-datasets-with-limited-hed', description: 'HED or data is missing in various places', + definitions: ['(Definition/Acc/#, (Acceleration/# m-per-s^2, Red))', '(Definition/MyColor, (Label/Pie))'], tests: [ { testname: 'no-hed-at-all-but-both-tsv-json-non-empty', @@ -49,6 +50,7 @@ export const bidsTestData = [ { name: 'invalid-syntax', description: 'Syntax errors in various places', + definitions: ['(Definition/Acc/#, (Acceleration/# m-per-s^2, Red))', '(Definition/MyColor, (Label/Pie))'], tests: [ { testname: 'mismatched-parentheses-in-tsv', @@ -85,6 +87,7 @@ export const bidsTestData = [ { name: 'invalid-tag-tests', description: 'JSON is valid but tsv is invalid', + definitions: ['(Definition/Acc/#, (Acceleration/# m-per-s^2, Red))', '(Definition/MyColor, (Label/Pie))'], tests: [ { testname: 'invalid-bad-tag-in-tsv', @@ -179,14 +182,14 @@ export const bidsTestData = [ eventsString: 'onset\tduration\tevent_code\tHED\n' + '7\t4\tface\tRed,{blue}', sidecarErrors: [], tsvErrors: [ - BidsHedIssue.fromHedIssue(generateIssue('curlyBracesInHedColumn', { column: '{blue}', tsvLine: 2 }), { + BidsHedIssue.fromHedIssue(generateIssue('curlyBracesNotAllowed', { string: 'Red,{blue}', tsvLine: 2 }), { path: 'valid-sidecar-tsv-curly-brace.tsv', relativePath: 'valid-sidecar-tsv-curly-brace.tsv', }), ], comboErrors: [ BidsHedIssue.fromHedIssue( - generateIssue('curlyBracesInHedColumn', { column: '{blue}' }), + generateIssue('curlyBracesNotAllowed', { string: 'Red,{blue}' }), { path: 'valid-sidecar-tsv-curly-brace.tsv', relativePath: 'valid-sidecar-tsv-curly-brace.tsv' }, { tsvLine: 2 }, ), @@ -197,6 +200,7 @@ export const bidsTestData = [ { name: 'duplicate-tag-tests', description: 'Duplicate tags can appear in isolation or in combination', + definitions: ['(Definition/Acc/#, (Acceleration/# m-per-s^2, Red))', '(Definition/MyColor, (Label/Pie))'], tests: [ { testname: 'valid-no-duplicate-tsv', @@ -558,6 +562,7 @@ export const bidsTestData = [ { name: 'curly-brace-tests', description: 'Curly braces tested in various places', + definitions: ['(Definition/Acc/#, (Acceleration/# m-per-s^2, Red))', '(Definition/MyColor, (Label/Pie))'], tests: [ { testname: 'valid-curly-brace-in-sidecar-with-value-splice', @@ -720,14 +725,14 @@ export const bidsTestData = [ eventsString: 'onset\tduration\tHED\n' + '19\t6\t{event_code}\n', sidecarErrors: [], tsvErrors: [ - BidsHedIssue.fromHedIssue(generateIssue('curlyBracesInHedColumn', { column: '{event_code}', tsvLine: 2 }), { + BidsHedIssue.fromHedIssue(generateIssue('curlyBracesNotAllowed', { string: '{event_code}', tsvLine: 2 }), { path: 'invalid-curly-brace-in-HED-tsv-column.tsv', relativePath: 'invalid-curly-brace-in-HED-tsv-column.tsv', }), ], comboErrors: [ BidsHedIssue.fromHedIssue( - generateIssue('curlyBracesInHedColumn', { column: '{event_code}' }), + generateIssue('curlyBracesNotAllowed', { string: '{event_code}' }), { path: 'invalid-curly-brace-in-HED-tsv-column.tsv', relativePath: 'invalid-curly-brace-in-HED-tsv-column.tsv', @@ -866,6 +871,7 @@ export const bidsTestData = [ { name: 'placeholder-tests', description: 'Various placeholder tests', + definitions: ['(Definition/Acc/#, (Acceleration/# m-per-s^2, Red))', '(Definition/MyColor, (Label/Pie))'], tests: [ { testname: 'valid-placeholder-used-in-tsv', @@ -963,6 +969,7 @@ export const bidsTestData = [ { name: 'unit-tests', description: 'Various unit tests (limited for now)', + definitions: ['(Definition/Acc/#, (Acceleration/# m-per-s^2, Red))', '(Definition/MyColor, (Label/Pie))'], tests: [ { testname: 'valid-units-on-a-placeholder', @@ -1012,6 +1019,7 @@ export const bidsTestData = [ { name: 'definition-tests', description: 'Various definition tests (limited for now)', + definitions: ['(Definition/Acc/#, (Acceleration/# m-per-s^2, Red))', '(Definition/MyColor, (Label/Pie))'], tests: [ { testname: 'valid-definition-no-placeholder', @@ -1103,7 +1111,7 @@ export const bidsTestData = [ tsvErrors: [], comboErrors: [ BidsHedIssue.fromHedIssue( - generateIssue('missingDefinition', { definition: 'MissingDef' }), + generateIssue('missingDefinitionForDef', { definition: 'MissingDef' }), { path: 'invalid-missing-definition-for-def.tsv', relativePath: 'invalid-missing-definition-for-def.tsv', @@ -1171,8 +1179,7 @@ export const bidsTestData = [ sidecarErrors: [ BidsHedIssue.fromHedIssue( generateIssue('invalidTagGroup', { - tags: 'Definition/Apple, Definition/Banana', - string: '(Definition/Apple, Definition/Banana, (Blue))', + tagGroup: '(Definition/Apple, Definition/Banana, (Blue))', }), { path: 'invalid-multiple-definition-tags.json', @@ -1184,8 +1191,7 @@ export const bidsTestData = [ comboErrors: [ BidsHedIssue.fromHedIssue( generateIssue('invalidTagGroup', { - tags: 'Definition/Apple, Definition/Banana', - string: '(Definition/Apple, Definition/Banana, (Blue))', + tagGroup: '(Definition/Apple, Definition/Banana, (Blue))', }), { path: 'invalid-multiple-definition-tags.tsv', @@ -1212,8 +1218,7 @@ export const bidsTestData = [ sidecarErrors: [ BidsHedIssue.fromHedIssue( generateIssue('invalidTagGroup', { - tags: 'Definition/ExtraGroupDef', - string: '(Definition/ExtraGroupDef, (Red), (Blue))', + tagGroup: '(Definition/ExtraGroupDef, (Red), (Blue))', }), { path: 'invalid-definition-with-extra-groups.json', @@ -1225,8 +1230,7 @@ export const bidsTestData = [ comboErrors: [ BidsHedIssue.fromHedIssue( generateIssue('invalidTagGroup', { - tags: 'Definition/ExtraGroupDef', - string: '(Definition/ExtraGroupDef, (Red), (Blue))', + tagGroup: '(Definition/ExtraGroupDef, (Red), (Blue))', }), { path: 'invalid-definition-with-extra-groups.tsv', @@ -1253,8 +1257,7 @@ export const bidsTestData = [ sidecarErrors: [ BidsHedIssue.fromHedIssue( generateIssue('invalidTagGroup', { - string: '(Definition/ExtraSiblingDef, Red, (Blue))', - tags: 'Definition/ExtraSiblingDef, Red', + tagGroup: '(Definition/ExtraSiblingDef, Red, (Blue))', }), { path: 'invalid-definition-with-extra-sibling.json', @@ -1266,8 +1269,7 @@ export const bidsTestData = [ comboErrors: [ BidsHedIssue.fromHedIssue( generateIssue('invalidTagGroup', { - string: '(Definition/ExtraSiblingDef, Red, (Blue))', - tags: 'Definition/ExtraSiblingDef, Red', + tagGroup: '(Definition/ExtraSiblingDef, Red, (Blue))', }), { path: 'invalid-definition-with-extra-sibling.tsv', @@ -1425,4 +1427,92 @@ export const bidsTestData = [ }, ], }, + { + name: 'delay-tests', + description: 'Tests with delay', + definitions: ['(Definition/Acc/#, (Acceleration/# m-per-s^2, Red))', '(Definition/MyColor, (Label/Pie))'], + tests: [ + { + testname: 'nested-delay', + explanation: 'A delay tag with nesting', + schemaVersion: '8.3.0', + sidecar: { + event_code: { + HED: { + face: '((Delay/5.0 s, Onset, Def/MyColor), Red)', + }, + }, + }, + eventsString: 'onset\tduration\tHED\n4.5\t0\t((Delay/5.0 s, Onset, Def/MyColor), Red)\n', + sidecarErrors: [ + BidsHedIssue.fromHedIssue( + generateIssue('invalidTopLevelTagGroupTag', { + string: '((Delay/5.0 s, Onset, Def/MyColor), Red)', + tag: 'Delay/5.0 s', + }), + { + path: 'nested-delay.json', + relativePath: 'nested-delay.json', + }, + ), + BidsHedIssue.fromHedIssue( + generateIssue('invalidTopLevelTagGroupTag', { + string: '((Delay/5.0 s, Onset, Def/MyColor), Red)', + tag: 'Onset', + }), + { + path: 'nested-delay.json', + relativePath: 'nested-delay.json', + }, + ), + ], + tsvErrors: [ + BidsHedIssue.fromHedIssue( + generateIssue('invalidTopLevelTagGroupTag', { + string: '((Delay/5.0 s, Onset, Def/MyColor), Red)', + tag: 'Delay/5.0 s', + }), + { + path: 'nested-delay.tsv', + relativePath: 'nested-delay.tsv', + }, + { tsvLine: '2' }, + ), + BidsHedIssue.fromHedIssue( + generateIssue('invalidTopLevelTagGroupTag', { + string: '((Delay/5.0 s, Onset, Def/MyColor), Red)', + tag: 'Onset', + }), + { + path: 'nested-delay.tsv', + relativePath: 'nested-delay.tsv', + }, + { tsvLine: '2' }, + ), + ], + comboErrors: [ + BidsHedIssue.fromHedIssue( + generateIssue('invalidTopLevelTagGroupTag', { + string: '((Delay/5.0 s, Onset, Def/MyColor), Red)', + tag: 'Delay/5.0 s', + }), + { + path: 'nested-delay.tsv', + relativePath: 'nested-delay.tsv', + }, + ), + BidsHedIssue.fromHedIssue( + generateIssue('invalidTopLevelTagGroupTag', { + string: '((Delay/5.0 s, Onset, Def/MyColor), Red)', + tag: 'Onset', + }), + { + path: 'nested-delay.tsv', + relativePath: 'nested-delay.tsv', + }, + ), + ], + }, + ], + }, ] diff --git a/tests/testData/stringParserTests.data.js b/tests/testData/stringParserTests.data.js index 7816fec8..a71d6b86 100644 --- a/tests/testData/stringParserTests.data.js +++ b/tests/testData/stringParserTests.data.js @@ -471,8 +471,8 @@ export const parseTestData = [ warnings: [], }, { - testname: 'definition-at-top-level-but-slices', - explanation: '"Definition/Green1, (Green)" is not in group (not allowed to be in a slice)', + testname: 'definition-at-top-level-but-splices', + explanation: '"Definition/Green1, (Green)" is not in group (not allowed to be in a splice)', schemaVersion: '8.3.0', stringIn: 'Definition/Green1, (Green)', stringLong: null, @@ -486,6 +486,73 @@ export const parseTestData = [ ], warnings: [], }, + { + testname: 'definition-with-extra-tags', + explanation: '"(Definition/IllegalSibling, Train, (Circle))" should not have an extra tag in definition', + schemaVersion: '8.3.0', + stringIn: '(Definition/IllegalSibling, Train, (Circle))', + stringLong: null, + stringShort: null, + fullCheck: false, + errors: [ + generateIssue('invalidTagGroup', { + tagGroup: '(Definition/IllegalSibling, Train, (Circle))', + }), + ], + warnings: [], + }, + + { + testname: 'definition-with-deep-defs-inside', + explanation: + '"(Definition/DefNested, (Def/Nested, (Red, Blue, (Def/Blech)), Triangle))" cannot have Def in definition', + schemaVersion: '8.3.0', + stringIn: '(Definition/DefNested, (Def/Nested, (Red, Blue, (Def/Blech)), Triangle))', + stringLong: null, + stringShort: null, + fullCheck: false, + errors: [ + generateIssue('invalidGroupTags', { + string: '(Definition/DefNested, (Def/Nested, (Red, Blue, (Def/Blech)), Triangle))', + tags: 'Def/Nested, Def/Blech', + }), + ], + warnings: [], + }, + + { + testname: 'definition-with-nested-definition', + explanation: + '"(Definition/NestedDefinition, (Touchscreen, (Definition/InnerDefinition, (Square))))" should not have an extra tag in definition', + schemaVersion: '8.3.0', + stringIn: '(Definition/NestedDefinition, (Touchscreen, (Definition/InnerDefinition, (Square))))', + stringLong: null, + stringShort: null, + fullCheck: false, + errors: [ + generateIssue('invalidTopLevelTagGroupTag', { + string: '(Definition/NestedDefinition, (Touchscreen, (Definition/InnerDefinition, (Square))))', + tag: 'Definition/InnerDefinition', + }), + ], + warnings: [], + }, + { + testname: 'definition-with-multiple-groups', + explanation: + '"(Definition/MultipleTagGroupDefinition, (Touchscreen), (Square))" should only have 1 inner group', + schemaVersion: '8.3.0', + stringIn: '(Definition/MultipleTagGroupDefinition, (Touchscreen), (Square))', + stringLong: null, + stringShort: null, + fullCheck: false, + errors: [ + generateIssue('invalidTagGroup', { + tagGroup: '(Definition/MultipleTagGroupDefinition, (Touchscreen), (Square))', + }), + ], + warnings: [], + }, { testname: 'definition-group-with-multiple-definition-tags', explanation: '"(Definition/Apple, Definition/Banana, (Blue))" has two definition tags in the same group.', @@ -496,8 +563,7 @@ export const parseTestData = [ fullCheck: false, errors: [ generateIssue('invalidTagGroup', { - tags: 'Definition/Apple, Definition/Banana', - string: '(Definition/Apple, Definition/Banana, (Blue))', + tagGroup: '(Definition/Apple, Definition/Banana, (Blue))', }), ], warnings: [], @@ -540,6 +606,23 @@ export const parseTestData = [ errors: [], warnings: [], }, + { + testname: 'def-expand-with-inner-def-expand', + explanation: + '"Item, (Event, Object, (Item, (Def-expand/Blech, (Agent-action, Item))))" has a Def-expand inside a Def-expand', + schemaVersion: '8.3.0', + stringIn: 'Item, (Event, Object, (Item, (Def-expand/Blech, (Agent-action, (Def-expand/Temp), Item))))', + stringLong: null, + stringShort: null, + fullCheck: false, + errors: [ + generateIssue('invalidGroupTags', { + tags: 'Def-expand/Temp', + string: '(Def-expand/Blech, (Agent-action, (Def-expand/Temp), Item))', + }), + ], + warnings: [], + }, { testname: 'event-context-in-subgroup', explanation: '"(Red, (Event-context, (Blue)))" has Event-context not in a top-level-tag group', @@ -620,7 +703,7 @@ export const parseTestData = [ stringLong: null, stringShort: null, fullCheck: false, - errors: [generateIssue('invalidTagGroup', { tags: 'Offset, Item', string: '(Offset, Item)' })], + errors: [generateIssue('invalidTagGroup', { tagGroup: '(Offset, Item)' })], warnings: [], }, { @@ -659,6 +742,114 @@ export const parseTestData = [ errors: [], warnings: [], }, + { + testname: 'onset-delay-no-def', + explanation: '"(Onset, Delay/5 s)" does not have a Def for Onset.', + schemaVersion: '8.3.0', + stringIn: '(Onset, Delay/5 s)', + stringLong: null, + stringShort: null, + fullCheck: false, + errors: [generateIssue('temporalWithoutDefinition', { tagGroup: '(Onset, Delay/5 s)', tag: 'Onset' })], + warnings: [], + }, + { + testname: 'onset-delay-with-def', + explanation: '"(Onset, Delay/5 s, Def/myDef)" has an Onset, Delay and Def.', + schemaVersion: '8.3.0', + stringIn: '(Onset, Delay/5 s, Def/myDef)', + stringLong: + '(Property/Data-property/Data-marker/Temporal-marker/Onset, Property/Data-property/Data-value/Spatiotemporal-value/Temporal-value/Delay/5 s, Property/Organizational-property/Def/myDef)', + stringShort: '(Onset, Delay/5 s, Def/myDef)', + fullCheck: false, + errors: [], + warnings: [], + }, + { + testname: 'inset-with delay-no-def', + explanation: '"(Inset, Delay/5 s)" does not have a Def for Inset.', + schemaVersion: '8.3.0', + stringIn: '(Inset, Delay/5 s)', + stringLong: null, + stringShort: null, + fullCheck: false, + errors: [generateIssue('temporalWithoutDefinition', { tagGroup: '(Inset, Delay/5 s)', tag: 'Inset' })], + warnings: [], + }, + { + testname: 'inset-delay-with-def', + explanation: '"(Inset, Delay/5 s, Def/myDef)" has an Inset, Delay and Def.', + schemaVersion: '8.3.0', + stringIn: '(Inset, Delay/5 s, Def/myDef)', + stringLong: + '(Property/Data-property/Data-marker/Temporal-marker/Inset, Property/Data-property/Data-value/Spatiotemporal-value/Temporal-value/Delay/5 s, Property/Organizational-property/Def/myDef)', + stringShort: '(Inset, Delay/5 s, Def/myDef)', + fullCheck: false, + errors: [], + warnings: [], + }, + { + testname: 'inset-with-def-and-group', + explanation: '"(Inset, Def/myDef, (Def/Blech, Item))" has a def and a grpi[.', + schemaVersion: '8.3.0', + stringIn: '(Inset, Def/myDef, (Def/Blech, Item))', + stringLong: + '(Property/Data-property/Data-marker/Temporal-marker/Inset, Property/Organizational-property/Def/myDef, (Property/Organizational-property/Def/Blech, Item))', + stringShort: '(Inset, Def/myDef, (Def/Blech, Item))', + fullCheck: false, + errors: [], + warnings: [], + }, + { + testname: 'inset-with-def-expand-and-group', + explanation: '"(Inset, (Def-expand/myDef), (Def/Blech, Item))" has a Def-expand and a group.', + schemaVersion: '8.3.0', + stringIn: '(Inset, (Def-expand/myDef), (Def/Blech, Item))', + stringLong: + '(Property/Data-property/Data-marker/Temporal-marker/Inset, (Property/Organizational-property/Def-expand/myDef), (Property/Organizational-property/Def/Blech, Item))', + stringShort: '(Inset, (Def-expand/myDef), (Def/Blech, Item))', + fullCheck: false, + errors: [], + warnings: [], + }, + { + testname: 'inset-with-bad-def-expand-and-group', + explanation: + '"(Inset, (Def-expand/myDef, (Item,(Def/Temp))), (Def/Blech, Item))" has a bad Def-expand and a group.', + schemaVersion: '8.3.0', + stringIn: '(Inset, (Def-expand/myDef, (Item,(Def/Temp))), (Def/Blech, Item))', + stringLong: null, + stringShort: null, + fullCheck: false, + errors: [ + generateIssue('invalidGroupTags', { string: '(Def-expand/myDef, (Item,(Def/Temp)))', tags: 'Def/Temp' }), + ], + warnings: [], + }, + { + testname: 'Offset-with delay-no-def', + explanation: '"(Offset, Delay/5 s)" does not have a Def for Inset.', + schemaVersion: '8.3.0', + stringIn: '(Offset, Delay/5 s)', + stringLong: null, + stringShort: null, + fullCheck: false, + errors: [generateIssue('temporalWithoutDefinition', { tagGroup: '(Offset, Delay/5 s)', tag: 'Offset' })], + warnings: [], + }, + { + testname: 'offset-delay-with-def', + explanation: '"(Offset, Delay/5 s, Def/myDef)" has an Offset, Delay and Def.', + schemaVersion: '8.3.0', + stringIn: '(Offset, Delay/5 s, Def/myDef)', + stringLong: + '(Property/Data-property/Data-marker/Temporal-marker/Offset, Property/Data-property/Data-value/Spatiotemporal-value/Temporal-value/Delay/5 s, Property/Organizational-property/Def/myDef)', + stringShort: '(Offset, Delay/5 s, Def/myDef)', + fullCheck: false, + errors: [], + warnings: [], + }, + { testname: 'onset-with-def-expand', explanation: '"(Onset, (Def-expand/MyColor, (Label/Pie)), (Red))" is okay.', @@ -671,6 +862,18 @@ export const parseTestData = [ errors: [], warnings: [], }, + { + testname: 'def-expand-with-value', + explanation: '"(Def-expand/Acc/4.5, (Acceleration/4.5, Red))" is okay.', + schemaVersion: '8.3.0', + stringIn: '(Def-expand/Acc/4.5, (Acceleration/4.5, Red))', + stringLong: + '(Property/Organizational-property/Def-expand/Acc/4.5, (Property/Data-property/Data-value/Spatiotemporal-value/Rate-of-change/Acceleration/4.5, Property/Sensory-property/Sensory-attribute/Visual-attribute/Color/CSS-color/Red-color/Red))', + stringShort: '(Def-expand/Acc/4.5, (Acceleration/4.5, Red))', + fullCheck: false, + errors: [], + warnings: [], + }, { testname: 'def-expand-not-in-group-fullcheck', explanation: '"Def-expand/Acc/4.5, (Acceleration/4.5, Red)" needs a group on full-check.', @@ -716,17 +919,16 @@ export const parseTestData = [ warnings: [], }, { - testname: 'offset-group-has-other-tag', - explanation: '"((Def-expand/MyColor, (Label/Pie)), Offset, Blue)" should not have an extra tag.', + testname: 'inset-group-has-other-tag', + explanation: '"((Def-expand/MyColor, (Label/Pie)), Inset, Blue)" should not have an extra tag.', schemaVersion: '8.3.0', - stringIn: '((Def-expand/MyColor, (Label/Pie)), Offset, Blue)', + stringIn: '((Def-expand/MyColor, (Label/Pie)), Inset, Blue)', stringLong: null, stringShort: null, fullCheck: false, errors: [ generateIssue('invalidTagGroup', { - tags: 'Offset, Blue', - string: '((Def-expand/MyColor, (Label/Pie)), Offset, Blue)', + tagGroup: '((Def-expand/MyColor, (Label/Pie)), Inset, Blue)', }), ], warnings: [], diff --git a/tests/testData/tagParserTests.data.js b/tests/testData/tagParserTests.data.js index 79c3a0b4..b5850eed 100644 --- a/tests/testData/tagParserTests.data.js +++ b/tests/testData/tagParserTests.data.js @@ -33,6 +33,19 @@ export const parsedHedTagTests = [ takesValue: false, error: null, }, + { + testname: 'valid-tag-with-extension-and-blanks', + explanation: '" Item/Blech " has surrounding blanks.', + schemaVersion: '8.3.0', + fullString: ' Item/Blech ', + tagSpec: new TagSpec('Item/Blech', 1, 11, ''), + tagLong: 'Item/Blech', + tagShort: 'Item/Blech', + formattedTag: 'item/blech', + canonicalTag: 'Item/Blech', + takesValue: false, + error: null, + }, { testname: 'valid-Two-level-tag', explanation: '" Item/object " is two-level and mixed case.', @@ -59,6 +72,104 @@ export const parsedHedTagTests = [ takesValue: false, error: null, }, + { + testname: 'valid-long-tag-with-value', + explanation: '"Property/Agent-property/Agent-trait/Age/15" is a valid long form tag with value.', + schemaVersion: '8.3.0', + fullString: 'Property/Agent-property/Agent-trait/Age/15"', + tagSpec: new TagSpec('Property/Agent-property/Agent-trait/Age/15', 1, 43, ''), + tagLong: 'Property/Agent-property/Agent-trait/Age/15', + tagShort: 'Age/15', + formattedTag: 'property/agent-property/agent-trait/age/15', + canonicalTag: 'Property/Agent-property/Agent-trait/Age/15', + takesValue: true, + error: null, + }, + { + testname: 'valid-long-tag-with-cascade-extension', + explanation: + '"Property/Agent-property/Agent-state/Agent-emotional-state/Awed/Cascade-Extension" is two-level and mixed case.', + schemaVersion: '8.3.0', + fullString: 'Property/Agent-property/Agent-state/Agent-emotional-state/Awed/Cascade-Extension', + tagSpec: new TagSpec( + 'Property/Agent-property/Agent-state/Agent-emotional-state/Awed/Cascade-Extension', + 1, + 43, + '', + ), + tagLong: 'Property/Agent-property/Agent-state/Agent-emotional-state/Awed/Cascade-Extension', + tagShort: 'Awed/Cascade-Extension', + formattedTag: 'property/agent-property/agent-state/agent-emotional-state/awed/cascade-extension', + canonicalTag: 'Property/Agent-property/Agent-state/Agent-emotional-state/Awed/Cascade-Extension', + takesValue: false, + error: null, + }, + { + testname: 'valid-short-tag-with-cascade-extension', + explanation: + '"Environmental-sound/Long-Unique-Value-With/Slash-Marks" is a valid short tag with cascade extension.', + schemaVersion: '8.3.0', + fullString: 'Environmental-sound/Long-Unique-Value-With/Slash-Marks', + tagSpec: new TagSpec('Environmental-sound/Long-Unique-Value-With/Slash-Marks', 1, 43, ''), + tagLong: 'Item/Sound/Environmental-sound/Long-Unique-Value-With/Slash-Marks', + tagShort: 'Environmental-sound/Long-Unique-Value-With/Slash-Marks', + formattedTag: 'item/sound/environmental-sound/long-unique-value-with/slash-marks', + canonicalTag: 'Item/Sound/Environmental-sound/Long-Unique-Value-With/Slash-Marks', + takesValue: false, + error: null, + }, + { + testname: 'valid-tag-with-two-level-value', + explanation: '"Definition/BlueSquare/Blech" is a two-level-value.', + schemaVersion: '8.3.0', + fullString: 'Definition/BlueSquare/Blech', + tagSpec: new TagSpec('Definition/BlueSquare/Blech', 1, 27, ''), + tagLong: 'Property/Organizational-property/Definition/BlueSquare/Blech', + tagShort: 'Definition/BlueSquare/Blech', + formattedTag: 'property/organizational-property/definition/bluesquare/blech', + canonicalTag: 'Property/Organizational-property/Definition/BlueSquare/Blech', + takesValue: true, + error: null, + }, + { + testname: 'valid-tag-with-two-level-value-placeholder', + explanation: '"Definition/BlueSquare/#" is a two-level value with a placeholder.', + schemaVersion: '8.3.0', + fullString: 'Definition/BlueSquare/#', + tagSpec: new TagSpec('Definition/BlueSquare/#', 1, 22, ''), + tagLong: 'Property/Organizational-property/Definition/BlueSquare/#', + tagShort: 'Definition/BlueSquare/#', + formattedTag: 'property/organizational-property/definition/bluesquare/#', + canonicalTag: 'Property/Organizational-property/Definition/BlueSquare/#', + takesValue: true, + error: null, + }, + { + testname: 'valid-tag-with-two-level-value-node', + explanation: '"Definition/Blue/Red" uses node names for values.', + schemaVersion: '8.3.0', + fullString: 'Definition/Blue/Red', + tagSpec: new TagSpec('Definition/Blue/Red', 1, 22, ''), + tagLong: 'Property/Organizational-property/Definition/Blue/Red', + tagShort: 'Definition/Blue/Red', + formattedTag: 'property/organizational-property/definition/blue/red', + canonicalTag: 'Property/Organizational-property/Definition/Blue/Red', + takesValue: true, + error: null, + }, + { + testname: 'valid-tag-with-one-level-value-node', + explanation: '"Definition/Blue" uses node names for values.', + schemaVersion: '8.3.0', + fullString: 'Definition/Blue', + tagSpec: new TagSpec('Definition/Blue', 1, 18, ''), + tagLong: 'Property/Organizational-property/Definition/Blue', + tagShort: 'Definition/Blue', + formattedTag: 'property/organizational-property/definition/blue', + canonicalTag: 'Property/Organizational-property/Definition/Blue', + takesValue: true, + error: null, + }, { testname: 'valid-tag-with-value-no-units', explanation: '" Age/5 " has a value but no units.', @@ -145,8 +256,8 @@ export const parsedHedTagTests = [ error: generateIssue('invalidTag', { tag: 'Blech' }), }, { - testname: 'invalid-tag-requires-child', - explanation: '"Duration" should have a child.', + testname: 'invalid-tag-requires-value', + explanation: '"Duration" should have a value.', schemaVersion: '8.3.0', fullString: 'Duration', tagSpec: new TagSpec('Duration', 0, 8, ''), @@ -158,19 +269,70 @@ export const parsedHedTagTests = [ error: generateIssue('childRequired', { tag: 'Duration' }), }, { - //TODO: Special tag Event-context is unique and doesn't allow extension although parent does testname: 'invalid-tag-does-not-allow-extension', - explanation: '"Sensory-event/Blech" should not have a child no recursive-extension allowed.', + explanation: '"Sensory-event/Blech" should not have a child.', schemaVersion: '8.3.0', - fullString: 'Duration', + fullString: 'Sensory-event/Blech', tagSpec: new TagSpec('Sensory-event/Blech', 0, 19, ''), tagLong: undefined, tagShort: undefined, formattedTag: undefined, canonicalTag: undefined, - takesValue: true, + takesValue: false, error: generateIssue('invalidExtension', { parentTag: 'Sensory-event', tag: 'Blech' }), }, + { + testname: 'invalid-tag-does-not-allow-cascade-extension', + explanation: '"Event/Agent-action/Good/Time" should not allow cascade extension.', + schemaVersion: '8.3.0', + fullString: 'Event/Agent-action/Good/Time', + tagSpec: new TagSpec('Event/Agent-action/Good/Time', 0, 29, ''), + tagLong: undefined, + tagShort: undefined, + formattedTag: undefined, + canonicalTag: undefined, + takesValue: false, + error: generateIssue('invalidExtension', { parentTag: 'Event/Agent-action', tag: 'Good' }), + }, + { + testname: 'invalid-no-extension-tag-in-extension-allowed', + explanation: '"Onset/Blech" should not have an extension.', + schemaVersion: '8.3.0', + fullString: 'Onset/Blech', + tagSpec: new TagSpec('Onset/Blech', 0, 11, ''), + tagLong: undefined, + tagShort: undefined, + formattedTag: undefined, + canonicalTag: undefined, + takesValue: true, + error: generateIssue('invalidExtension', { parentTag: 'Onset', tag: 'Blech' }), + }, + { + testname: 'invalid-duplicate-tag-in-path', + explanation: '"Item/Object/Geometric-object/Blech/Object" should not reused Object.', + schemaVersion: '8.3.0', + fullString: 'Item/Object/Geometric-object/Blech/Object', + tagSpec: new TagSpec('Item/Object/Geometric-object/Blech/Object', 0, 41, ''), + tagLong: undefined, + tagShort: undefined, + formattedTag: undefined, + canonicalTag: undefined, + takesValue: true, + error: generateIssue('invalidParentNode', { parentTag: 'Item/Object/Geometric-object/Blech', tag: 'Object' }), + }, + { + testname: 'invalid-top-node', + explanation: '"InvalidEvent" is not a valid node.', + schemaVersion: '8.3.0', + fullString: 'InvalidEvent', + tagSpec: new TagSpec('InvalidEvent', 0, 41, ''), + tagLong: undefined, + tagShort: undefined, + formattedTag: undefined, + canonicalTag: undefined, + takesValue: true, + error: generateIssue('invalidTag', { tag: 'InvalidEvent' }), + }, { testname: 'invalid-tag-with-blank-in-extension', explanation: '" Object/blec h " has a blank in the tag extension', diff --git a/validator/event/validator.js b/validator/event/validator.js index 2ac230f5..cc83e17b 100644 --- a/validator/event/validator.js +++ b/validator/event/validator.js @@ -145,7 +145,7 @@ export default class HedValidator { */ // eslint-disable-next-line no-unused-vars validateHedTagGroup(parsedTagGroup) { - this.checkDefinitionGroupSyntax(parsedTagGroup) + //this.checkDefinitionGroupSyntax(parsedTagGroup) this.checkTemporalSyntax(parsedTagGroup) } @@ -218,19 +218,6 @@ export default class HedValidator { }) } - // TODO: Doesn't seem to be working correctly - /** - * Check for multiple instances of a unique tag. - */ - checkForMultipleUniqueTags(tagList) { - const actualTagList = tagList.filter((tagOrGroup) => tagOrGroup instanceof ParsedHedTag) - this._checkForTagAttribute(uniqueType, (uniqueTagPrefix) => { - if (actualTagList.filter((tag) => tag.formattedTag.startsWith(uniqueTagPrefix)).length > 1) { - this.pushIssue('multipleUniqueTags', { tag: uniqueTagPrefix, string: '' }) - } - }) - } - /** * Validation check based on a tag attribute. * @@ -456,7 +443,8 @@ export default class HedValidator { } const defName = ParsedHedGroup.findDefinitionName(tag.canonicalTag, defShortTag) if (!this.definitions.has(defName)) { - this.pushIssue('missingDefinition', { definition: defName }) + const code = defShortTag === 'Def' ? 'missingDefinitionForDef' : 'missingDefinitionForDefExpand' + this.pushIssue(code, { definition: defName }) } }