diff --git a/tools/src/main/js/linter/checks/index.js b/tools/src/main/js/linter/checks/index.js index dc12726c..922f85e6 100644 --- a/tools/src/main/js/linter/checks/index.js +++ b/tools/src/main/js/linter/checks/index.js @@ -39,6 +39,7 @@ export * from './schema-id-pattern.check.js'; export * from './schema-id-filepath.check.js'; export * from './schema-comment.check.js'; export * from './schema-draft.check.js'; +export * from './ref-best-practice.check.js'; export * from './model-property-order.check.js'; export * from './model-structure.check.js'; export * from './formatting-indent.check.js'; diff --git a/tools/src/main/js/linter/checks/ref-best-practice.check.js b/tools/src/main/js/linter/checks/ref-best-practice.check.js new file mode 100644 index 00000000..f040e89f --- /dev/null +++ b/tools/src/main/js/linter/checks/ref-best-practice.check.js @@ -0,0 +1,153 @@ +/** + * CycloneDX Schema Linter - $ref Best Practice Check + * + * Validates that `$ref` usage follows JSON Schema best practice: + * - `$ref` value is a string + * - no absolute `$ref` paths + * - same-file `$ref` starts with "#" + * - only documentational siblings are allowed alongside `$ref` + * + * @license Apache-2.0 + */ + +import { resolve } from 'path'; +import { pathToFileURL } from 'url'; + +import { LintCheck, registerCheck, Severity, traverseSchema } from '../index.js'; + +/** + * Keys allowed as siblings of `$ref`. + */ +const REF_ALLOWED_SIBLINGS = Object.freeze(new Set([ + '$ref', // the $ref itself + '$comment', 'title', 'description', 'examples', // documentational + /* do NOT add any non-documentationals + instead, use: + { "allOf": { "$ref": ... }, "$id" ..., "$anchor": ... } + { "allOf": { "$ref": ... }, "default" ... } + instead of additionalProperties -- { "allOf": { "$ref": ... }, "unevaluatedItems" ... } + */ +])); + +/** + * Keys whose values aren't schemas - their entire subtrees are pruned + */ +const SKIP_KEYS = Object.freeze(new Set([ + 'enum', 'const', 'default', // values + 'examples', 'meta:enum', // documentational +])); + + +/** + * Build a predicate that tells whether a `$ref` file-part resolves to the current schema itself, + * mimicking JSON Schema's reference resolution: + * the base URI is the root schema's `$id` if present, otherwise the retrieval URI (the file's location). + * + * @param {Readonly<*>} schema - the root schema + * @param {string|null} filePath - OS-native path the schema was read from, if any + * @return {(function(string): boolean)|null} predicate, or null if no base URI is determinable + */ +function makeSameFileTest(schema, filePath) { + // per JSON Schema spec: `$id` establishes the base URI ... + const id = schema?.['$id']; + const base = (typeof id === 'string' && URL.canParse(id)) + ? new URL(id) + : (filePath === null + ? null + : pathToFileURL(resolve(filePath))); + if (base === null) { + // no base known - skip the same-file rule + return null; + } + return filePart => { + try { + return new URL(filePart, base).href === base.href; + } catch (err) { + // maybe throw the error?? + return false; + } + }; +} + +/** + * Check that validates `$ref` best practice. + */ +class RefBestPracticeCheck extends LintCheck { + constructor() { + super( + 'ref-best-practice', + '$ref Best Practice', + 'Validates that $ref usage follows JSON Schema best practice.', + Severity.ERROR + ); + } + + async run(schema, rawContent, config = {}, filePath = null) { + const issues = []; + + const isSameFile = makeSameFileTest(schema, filePath); + + traverseSchema(schema, (node, path, key) => { + if (typeof key === 'string' && SKIP_KEYS.has(key)) { + // don't descend into keys whose values aren't schemas + return false; + } + + if (node === null || typeof node !== 'object' || Array.isArray(node)) return; + if (!('$ref' in node)) return; + + const ref = node['$ref']; + + if (typeof ref !== 'string') { + issues.push(this.createIssue( + 'Unexpected type of $ref.', + `${path}.$ref`, + { actual: typeof ref, expected: 'string' } + )); + return; + } + + if (ref.startsWith('/')) { + issues.push(this.createIssue( + 'Absolute $ref is not allowed.', + `${path}.$ref`, + { actual: ref, expected: 'a relative reference' } + )); + } else if (isSameFile !== null) { + const hashPos = ref.indexOf('#'); + const filePart = hashPos === -1 + ? ref + : ref.slice(0, hashPos); + if (filePart !== '' && isSameFile(filePart)) { + const fragment = hashPos === -1 + ? '#' + : ref.slice(hashPos); + issues.push(this.createIssue( + 'Same-file $ref must start with "#".', + `${path}.$ref`, + { actual: ref, expected: fragment } + )); + } + } + + for (const siblingKey of Object.keys(node)) { + if (!REF_ALLOWED_SIBLINGS.has(siblingKey)) { + issues.push(this.createIssue( + 'Unexpected key along with $ref. Wrap the $ref in an "allOf" instead.', + `${path}.${siblingKey}`, + { actual: 'present', expected: 'absent' } + )); + } + } + }); + + return issues; + } +} + +// Create and register the check +const check = new RefBestPracticeCheck(); +registerCheck(check); + +export { RefBestPracticeCheck }; +export default check; diff --git a/tools/src/main/js/linter/index.js b/tools/src/main/js/linter/index.js index a56ccf9b..a51f0bb2 100644 --- a/tools/src/main/js/linter/index.js +++ b/tools/src/main/js/linter/index.js @@ -376,15 +376,40 @@ export class SchemaLinter { } /** - * Utility to traverse a JSON schema and call a visitor function - * @param {object} schema - The schema to traverse - * @param {function} visitor - Function called for each node: (node, path, key, parent) - * @param {string} [path] - Current path (used internally) - * @param {string} [key] - Current key (used internally) - * @param {object} [parent] - Parent node (used internally) + * Visitor invoked for each node during schema traversal. + * + * All arguments are to be treated as read-only: + * visitors must NOT modify the schema, nodes, or parents — traversal is + * for inspection only. Mutating during traversal leads to undefined behaviour. + * + * @callback SchemaVisitor + * @param {Readonly<*>} node - The current value (object, array, or primitive). Do not modify. + * @param {string} path - JSON-path-like location, e.g. `$.properties.foo[0]` + * @param {string|number|null} key - The property name or array index of `node` in its parent, `null` at the root + * @param {Readonly|null} parent - The parent object/array, `null` at the root. Do not modify. + * @returns {boolean|void} Return `false` to skip traversal of this node's children; any other value continues. + */ + +/** + * Utility to traverse a JSON schema depth-first and call a visitor function for each node. + * + * The visitor is invoked before descending into a node's children. + * If the visitor returns `false`, the node's children are not traversed + * (the subtree is pruned); any other return value (including `undefined`) + * continues the traversal. + * + * The traversed schema is treated as immutable: visitors must not mutate it. + * + * @param {Readonly<*>} schema - The schema (or sub-schema/value) to traverse. Not modified. + * @param {SchemaVisitor} visitor - Function called for each node with `(node, path, key, parent)` + * @param {string} [path='$'] - Current path (used internally) + * @param {string|number|null} [key=null] - Current key (used internally) + * @param {object|null} [parent=null] - Parent node (used internally) */ export function traverseSchema(schema, visitor, path = '$', key = null, parent = null) { - visitor(schema, path, key, parent); + if (visitor(schema, path, key, parent) === false) { + return; // visitor pruned this subtree + } if (typeof schema !== 'object' || schema === null) { return; diff --git a/tools/src/test/js/schema-v2/json-schema-semantic-tests.js b/tools/src/test/js/schema-v2/json-schema-semantic-tests.js index 92bd924c..ab5ec62c 100644 --- a/tools/src/test/js/schema-v2/json-schema-semantic-tests.js +++ b/tools/src/test/js/schema-v2/json-schema-semantic-tests.js @@ -267,75 +267,6 @@ function _printError(actual, expected, msg, schemaFile, schemaPath) { // region tests -const _REF_ALLOWED_SIBLINGS = Object.freeze(new Set([ - '$ref', // the $ref itself - '$comment', 'title', 'description', 'examples', // documentational - /* do NOT add any non-documentationals - instead, use: - { "allOf": { "$ref": ... }, "$id" ..., "$anchor": ... } - { "allOf": { "$ref": ... }, "default" ... } - instead of additionalProperties -- { "allOf": { "$ref": ... }, "unevaluatedItems" ... } - */ -])) - -/** - * `$ref` must follow JSON schema best-practice. - * @param {*} schema - * @param {string} schemaFile - * @return {number} number of errors found - */ -function testRefBestPractice(schema, schemaFile) { - let errCnt = 0 - for (const [path, node] of _findRefs(schema)) { - const ref = node['$ref'] - - if (typeof ref !== 'string') { - ++errCnt - _printError( - ref, 'a string', - 'unexpected type of $ref', - schemaFile, `${path}.$ref`) - continue - } - - if (ref.startsWith('/')) { - ++errCnt - _printError( - ref, 'a string', - 'absolute $ref', - schemaFile, `${path}.$ref`) - } else { - const hashPos = ref.indexOf('#') - const filePart = hashPos === -1 - ? ref - : ref.slice(0, hashPos) - if (filePart !== '') { - const resolved = join(dirname(schemaFile), filePart) - if (resolved === schemaFile) { - ++errCnt - const fragment = hashPos === -1 - ? '#' - : ref.slice(hashPos) - _printError( - ref, fragment, - 'same-file $ref must start with "#"', - schemaFile, path) - } - } - } - - const otherKeys = new Set(Object.keys(node)) - for (const key of otherKeys.difference(_REF_ALLOWED_SIBLINGS)) { - ++errCnt - _printError( - 'present', 'absent', - 'unexpected key along with $ref', - schemaFile, `${path}.${key}`) - } - } - return errCnt -} - /** * `bom-ref` properties must `$ref` refType — and nothing else may. * @param {*} schema @@ -500,7 +431,6 @@ function testDefaultValues(schema, schemaFile) { /** @type {Readonly>} */ const tests = Object.freeze({ - '$ref best practice': testRefBestPractice, 'refType usage (`bom-ref` <-> refType)': testRefTypeUsage, 'additionalProperties is `false`': testAdditionalProperties, 'enum value in range': testEnumValues,