Skip to content

Commit

Permalink
Eliminated various redundancies in validator section
Browse files Browse the repository at this point in the history
  • Loading branch information
VisLab committed Nov 13, 2024
1 parent 48ed2f0 commit 928f626
Show file tree
Hide file tree
Showing 3 changed files with 33 additions and 131 deletions.
12 changes: 6 additions & 6 deletions tests/event.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -571,7 +571,7 @@ describe('HED string and event validation', () => {
}

// TODO: Already covered in stringParserTests -- units still to move down.
it.skip('(REMOVE) should exist in the schema or be an allowed extension', () => {
/*it.skip('(REMOVE) should exist in the schema or be an allowed extension', () => {
const testStrings = {
takesValue: 'Time-value/3 ms',
full: 'Left-side-of',
Expand All @@ -594,12 +594,12 @@ describe('HED string and event validation', () => {
],
illegalComma: [
generateIssue('invalidTag', { tag: 'This/Is/A/Tag' }),
/* Intentionally not thrown (validation ends at parsing stage)
/!* Intentionally not thrown (validation ends at parsing stage)
generateIssue('extraCommaOrInvalid', {
previousTag: 'Label/This_is_a_label',
tag: 'This/Is/A/Tag',
}),
*/
*!/
],
placeholder: [
generateIssue('invalidTag', {
Expand All @@ -615,7 +615,7 @@ describe('HED string and event validation', () => {
},
{ checkForWarnings: true },
)
})
})*/

/*// TODO: REMOVE as these tests have been moved to tagParserTests
it.skip('(REMOVE) now in tagParserTests - should have a proper unit when required', () => {
Expand Down Expand Up @@ -968,7 +968,7 @@ describe('HED string and event validation', () => {
],
}
return validatorSemantic(testStrings, expectedIssues, (validator) => {
validator.checkForInvalidTopLevelTags()
validator.validateTopLevelTags()
})
})
})
Expand Down Expand Up @@ -1032,7 +1032,7 @@ describe('HED string and event validation', () => {
],
}
return validatorSemantic(testStrings, expectedIssues, (validator) => {
validator.checkForInvalidTopLevelTagGroupTags()
validator.validateTopLevelTagGroups()
})
})
})
Expand Down
20 changes: 2 additions & 18 deletions validator/event/init.js
Original file line number Diff line number Diff line change
@@ -1,9 +1,6 @@
import { parseHedString } from '../../parser/parser'
import ParsedHedString from '../../parser/parsedHedString'

import HedValidator from './validator'
import { Issue } from '../../common/issues/issues'
import { Schemas } from '../../schema/containers'

/**
* Perform initial validation on a HED string and parse it so further validation can be performed.
Expand All @@ -15,23 +12,9 @@ import { Schemas } from '../../schema/containers'
* @returns {[ParsedHedString, Issue[], HedValidator]} The parsed HED string, the actual HED schema collection to use, any issues found, and whether to perform semantic validation.
*/
const initiallyValidateHedString = function (hedString, hedSchemas, options, definitions = null) {
const doSemanticValidation = hedSchemas instanceof Schemas
if (!doSemanticValidation) {
hedSchemas = new Schemas(null)
}
let parsedString, parsingIssues

if (hedString instanceof ParsedHedString) {
// Skip parsing if we're passed an already-parsed string.
parsedString = hedString
parsingIssues = { syntax: [], delimiter: [] }
} else {
;[parsedString, parsingIssues] = parseHedString(hedString, hedSchemas)
}
const [parsedString, parsingIssues] = parseHedString(hedString, hedSchemas)
if (parsedString === null) {
return [null, [].concat(...Object.values(parsingIssues)), null]
} else if (parsingIssues.syntax.length > 0) {
hedSchemas = new Schemas(null)
}
const hedValidator = new HedValidator(parsedString, hedSchemas, definitions, options)
const allParsingIssues = [].concat(...Object.values(parsingIssues))
Expand All @@ -48,6 +31,7 @@ const initiallyValidateHedString = function (hedString, hedSchemas, options, def
* @returns {[boolean, Issue[]]} Whether the HED string is valid and any issues found.
* @deprecated
*/

export const validateHedString = function (hedString, hedSchemas, ...args) {
let settings
const settingsArg = args[0]
Expand Down
132 changes: 25 additions & 107 deletions validator/event/validator.js
Original file line number Diff line number Diff line change
Expand Up @@ -153,17 +153,32 @@ export default class HedValidator {
* Validate the top-level HED tags in a parsed HED string.
*/
validateTopLevelTags() {
if (this.options.checkForWarnings) {
this.checkForRequiredTags()
for (const topLevelTag of this.parsedString.topLevelTags) {
if (
!hedStringIsAGroup(topLevelTag.formattedTag) &&
(topLevelTag.hasAttribute(tagGroupType) || topLevelTag.parentHasAttribute(tagGroupType))
) {
this.pushIssue('invalidTopLevelTag', {
tag: topLevelTag,
})
}
}
this.checkForInvalidTopLevelTags()
}

/**
* Validate the top-level HED tag groups in a parsed HED string.
*/
validateTopLevelTagGroups() {
this.checkForInvalidTopLevelTagGroupTags()
for (const tag of this.parsedString.tags) {
if (!tag.hasAttribute(topLevelTagGroupType) && !tag.parentHasAttribute(topLevelTagGroupType)) {
continue
}
if (!this.parsedString.topLevelTagGroups.some((topLevelTagGroup) => topLevelTagGroup.includes(tag))) {
this.pushIssue('invalidTopLevelTagGroupTag', {
tag: tag,
})
}
}
}

/**
Expand Down Expand Up @@ -202,6 +217,7 @@ export default class HedValidator {
})
}

// TODO: Doesn't seem to be working correctly
/**
* Check for multiple instances of a unique tag.
*/
Expand All @@ -216,19 +232,6 @@ export default class HedValidator {
})
}

/**
* Check that all required tags are present.
*/
checkForRequiredTags() {
this._checkForTagAttribute(requiredType, (requiredTagPrefix) => {
if (!this.parsedString.topLevelTags.some((tag) => tag.formattedTag.startsWith(requiredTagPrefix))) {
this.pushIssue('requiredPrefixMissing', {
tagPrefix: requiredTagPrefix,
})
}
})
}

/**
* Validation check based on a tag attribute.
*
Expand All @@ -245,9 +248,10 @@ export default class HedValidator {
}
}

/**
// TODO: Checking for extensions is being removed temporarily -- well have to add it back eventually.
/* /!**
* Check if an individual HED tag is in the schema or is an allowed extension.
*/
*!/
checkIfTagIsValid(tag) {
if (tag.existsInSchema || tag.takesValue) {
return
Expand All @@ -256,14 +260,15 @@ export default class HedValidator {
// This is an allowed extension.
this.pushIssue('extension', { tag: tag })
}
}
}*/

/**
* Check basic placeholder tag syntax.
*
* @param {ParsedHedTag} tag A HED tag.
*/
checkPlaceholderTagSyntax(tag) {
// TODO: Refactor or eliminate after column splicing completed
const placeholderCount = getCharacterCount(tag.formattedTag, '#')
if (placeholderCount === 1) {
const valueTag = replaceTagNameWithPound(tag.formattedTag)
Expand Down Expand Up @@ -368,61 +373,6 @@ export default class HedValidator {
}
}

// TODO: This can be simplified.
/**
* Validate a unit and strip it from the value.
*
* @param {ParsedHedTag} tag A HED tag.
* @returns {[boolean, boolean, string]} Whether a unit was found, whether it was valid, and the stripped value.
*/
static validateUnits(tag) {
const originalTagUnitValue = tag.originalTagName
const tagUnitClassUnits = tag.validUnits
const validUnits = tag.schema.entries.allUnits
const unitStrings = Array.from(validUnits.keys())
unitStrings.sort((first, second) => {
return second.length - first.length
})
let actualUnit = getTagName(originalTagUnitValue, ' ')
let noUnitFound = false
if (actualUnit === originalTagUnitValue) {
actualUnit = ''
noUnitFound = true
}
let foundUnit, foundWrongCaseUnit, strippedValue
for (const unitName of unitStrings) {
const unit = validUnits.get(unitName)
const isPrefixUnit = unit.isPrefixUnit
const isUnitSymbol = unit.isUnitSymbol
for (const derivativeUnit of unit.derivativeUnits()) {
if (isPrefixUnit && originalTagUnitValue.startsWith(derivativeUnit)) {
foundUnit = true
noUnitFound = false
strippedValue = originalTagUnitValue.substring(derivativeUnit.length).trim()
}
if (actualUnit === derivativeUnit) {
foundUnit = true
strippedValue = getParentTag(originalTagUnitValue, ' ')
} else if (actualUnit.toLowerCase() === derivativeUnit.toLowerCase()) {
if (isUnitSymbol) {
foundWrongCaseUnit = true
} else {
foundUnit = true
}
strippedValue = getParentTag(originalTagUnitValue, ' ')
}
if (foundUnit) {
const unitIsValid = tagUnitClassUnits.has(unit)
return [true, unitIsValid, strippedValue]
}
}
if (foundWrongCaseUnit) {
return [true, false, strippedValue]
}
}
return [!noUnitFound, false, originalTagUnitValue]
}

/**
* Check full-string Definition syntax.
*/
Expand Down Expand Up @@ -594,38 +544,6 @@ export default class HedValidator {
}
}

/**
* Check for invalid top-level tags.
*/
checkForInvalidTopLevelTags() {
for (const topLevelTag of this.parsedString.topLevelTags) {
if (
!hedStringIsAGroup(topLevelTag.formattedTag) &&
(topLevelTag.hasAttribute(tagGroupType) || topLevelTag.parentHasAttribute(tagGroupType))
) {
this.pushIssue('invalidTopLevelTag', {
tag: topLevelTag,
})
}
}
}

/**
* Check for tags marked with the topLevelTagGroup attribute that are not in top-level tag groups.
*/
checkForInvalidTopLevelTagGroupTags() {
for (const tag of this.parsedString.tags) {
if (!tag.hasAttribute(topLevelTagGroupType) && !tag.parentHasAttribute(topLevelTagGroupType)) {
continue
}
if (!this.parsedString.topLevelTagGroups.some((topLevelTagGroup) => topLevelTagGroup.includes(tag))) {
this.pushIssue('invalidTopLevelTagGroupTag', {
tag: tag,
})
}
}
}

/**
* Generate a new issue object and push it to the end of the issues array.
*
Expand Down

0 comments on commit 928f626

Please sign in to comment.