linter: add check ref-best-practice

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';

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;

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<object>|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;

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<Record<string, function(*, string): number>>} */
 const tests = Object.freeze({
-    '$ref best practice': testRefBestPractice,
     'refType usage (`bom-ref` <-> refType)': testRefTypeUsage,
     'additionalProperties is `false`': testAdditionalProperties,
     'enum value in range': testEnumValues,