diff --git a/src/linter/constants.mjs b/src/linter/constants.mjs index b70ba65..95d29b5 100644 --- a/src/linter/constants.mjs +++ b/src/linter/constants.mjs @@ -4,4 +4,5 @@ export const LINT_MESSAGES = { missingIntroducedIn: "Missing 'introduced_in' field in the API doc entry", missingChangeVersion: 'Missing version field in the API doc entry', invalidChangeVersion: 'Invalid version number: {{version}}', + duplicateStabilityNode: 'Duplicate stability node', }; diff --git a/src/linter/engine.mjs b/src/linter/engine.mjs index 41d8f3a..9ae164c 100644 --- a/src/linter/engine.mjs +++ b/src/linter/engine.mjs @@ -3,29 +3,9 @@ /** * Creates a linter engine instance to validate ApiDocMetadataEntry entries * - * @param {import('./types').LintRule} rules Lint rules to validate the entries against + * @param {import('./types').LintRule[]} rules Lint rules to validate the entries against */ const createLinterEngine = rules => { - /** - * Validates a ApiDocMetadataEntry entry against all defined rules - * - * @param {ApiDocMetadataEntry} entry - * @returns {import('./types').LintIssue[]} - */ - const lint = entry => { - const issues = []; - - for (const rule of rules) { - const ruleIssues = rule(entry); - - if (ruleIssues.length > 0) { - issues.push(...ruleIssues); - } - } - - return issues; - }; - /** * Validates an array of ApiDocMetadataEntry entries against all defined rules * @@ -35,15 +15,14 @@ const createLinterEngine = rules => { const lintAll = entries => { const issues = []; - for (const entry of entries) { - issues.push(...lint(entry)); + for (const rule of rules) { + issues.push(...rule(entries)); } return issues; }; return { - lint, lintAll, }; }; diff --git a/src/linter/rules/duplicate-stability-nodes.mjs b/src/linter/rules/duplicate-stability-nodes.mjs new file mode 100644 index 0000000..0f7a164 --- /dev/null +++ b/src/linter/rules/duplicate-stability-nodes.mjs @@ -0,0 +1,38 @@ +import { LINT_MESSAGES } from '../constants.mjs'; + +/** + * Checks if there are multiple stability nodes within a chain. + * + * @param {ApiDocMetadataEntry[]} entries + * @returns {Array} + */ +export const duplicateStabilityNodes = entries => { + const issues = []; + let currentDepth = 0; + let currentStability = -1; + + for (const entry of entries) { + const { depth } = entry.heading.data; + const entryStability = entry.stability.children[0]?.data.index ?? -1; + + if ( + depth > currentDepth && + entryStability >= 0 && + entryStability === currentStability + ) { + issues.push({ + level: 'warn', + message: LINT_MESSAGES.duplicateStabilityNode, + location: { + path: entry.api_doc_source, + position: entry.stability.children[0].children[0].position, + }, + }); + } else { + currentDepth = depth; + currentStability = entryStability; + } + } + + return issues; +}; diff --git a/src/linter/rules/index.mjs b/src/linter/rules/index.mjs index b780eca..eb0a6b0 100644 --- a/src/linter/rules/index.mjs +++ b/src/linter/rules/index.mjs @@ -1,5 +1,6 @@ 'use strict'; +import { duplicateStabilityNodes } from './duplicate-stability-nodes.mjs'; import { invalidChangeVersion } from './invalid-change-version.mjs'; import { missingChangeVersion } from './missing-change-version.mjs'; import { missingIntroducedIn } from './missing-introduced-in.mjs'; @@ -8,6 +9,7 @@ import { missingIntroducedIn } from './missing-introduced-in.mjs'; * @type {Record} */ export default { + 'duplicate-stability-nodes': duplicateStabilityNodes, 'invalid-change-version': invalidChangeVersion, 'missing-change-version': missingChangeVersion, 'missing-introduced-in': missingIntroducedIn, diff --git a/src/linter/rules/invalid-change-version.mjs b/src/linter/rules/invalid-change-version.mjs index 927f35f..465e28a 100644 --- a/src/linter/rules/invalid-change-version.mjs +++ b/src/linter/rules/invalid-change-version.mjs @@ -4,30 +4,39 @@ import { valid } from 'semver'; /** * Checks if any change version is invalid * - * @param {ApiDocMetadataEntry} entry + * @param {ApiDocMetadataEntry[]} entries * @returns {Array} */ -export const invalidChangeVersion = entry => { - if (entry.changes.length === 0) { - return []; - } +export const invalidChangeVersion = entries => { + const issues = []; + + for (const entry of entries) { + if (entry.changes.length === 0) continue; + + const allVersions = entry.changes + .filter(change => change.version) + .flatMap(change => + Array.isArray(change.version) ? change.version : [change.version] + ); - const allVersions = entry.changes - .filter(change => change.version) - .flatMap(change => - Array.isArray(change.version) ? change.version : [change.version] + const invalidVersions = allVersions.filter( + version => valid(version) === null ); - const invalidVersions = allVersions.filter( - version => valid(version) === null - ); + issues.push( + ...invalidVersions.map(version => ({ + level: 'warn', + message: LINT_MESSAGES.invalidChangeVersion.replace( + '{{version}}', + version + ), + location: { + path: entry.api_doc_source, + position: entry.yaml_position, + }, + })) + ); + } - return invalidVersions.map(version => ({ - level: 'warn', - message: LINT_MESSAGES.invalidChangeVersion.replace('{{version}}', version), - location: { - path: entry.api_doc_source, - position: entry.yaml_position, - }, - })); + return issues; }; diff --git a/src/linter/rules/missing-change-version.mjs b/src/linter/rules/missing-change-version.mjs index 67b42a1..e540ee0 100644 --- a/src/linter/rules/missing-change-version.mjs +++ b/src/linter/rules/missing-change-version.mjs @@ -1,22 +1,28 @@ /** * Checks if any change version is missing * - * @param {ApiDocMetadataEntry} entry + * @param {ApiDocMetadataEntry[]} entries * @returns {Array} */ -export const missingChangeVersion = entry => { - if (entry.changes.length === 0) { - return []; +export const missingChangeVersion = entries => { + const issues = []; + + for (const entry of entries) { + if (entry.changes.length === 0) continue; + + issues.push( + ...entry.changes + .filter(change => !change.version) + .map(() => ({ + level: 'warn', + message: 'Missing change version', + location: { + path: entry.api_doc_source, + position: entry.yaml_position, + }, + })) + ); } - return entry.changes - .filter(change => !change.version) - .map(() => ({ - level: 'warn', - message: 'Missing change version', - location: { - path: entry.api_doc_source, - position: entry.yaml_position, - }, - })); + return issues; }; diff --git a/src/linter/rules/missing-introduced-in.mjs b/src/linter/rules/missing-introduced-in.mjs index 8dc7989..785bc6b 100644 --- a/src/linter/rules/missing-introduced-in.mjs +++ b/src/linter/rules/missing-introduced-in.mjs @@ -3,22 +3,24 @@ import { LINT_MESSAGES } from '../constants.mjs'; /** * Checks if `introduced_in` field is missing * - * @param {ApiDocMetadataEntry} entry + * @param {ApiDocMetadataEntry[]} entries * @returns {Array} */ -export const missingIntroducedIn = entry => { - // Early return if not a top-level heading or if introduced_in exists - if (entry.heading.depth !== 1 || entry.introduced_in) { - return []; - } +export const missingIntroducedIn = entries => { + const issues = []; + + for (const entry of entries) { + // Early continue if not a top-level heading or if introduced_in exists + if (entry.heading.depth !== 1 || entry.introduced_in) continue; - return [ - { + issues.push({ level: 'info', message: LINT_MESSAGES.missingIntroducedIn, location: { path: entry.api_doc_source, }, - }, - ]; + }); + } + + return issues; }; diff --git a/src/linter/tests/engine.test.mjs b/src/linter/tests/engine.test.mjs index cbbd1c5..ac33921 100644 --- a/src/linter/tests/engine.test.mjs +++ b/src/linter/tests/engine.test.mjs @@ -11,13 +11,13 @@ describe('createLinterEngine', () => { const engine = createLinterEngine([rule1, rule2]); - engine.lint(assertEntry); + engine.lintAll([assertEntry]); assert.strictEqual(rule1.mock.callCount(), 1); assert.strictEqual(rule2.mock.callCount(), 1); - assert.deepEqual(rule1.mock.calls[0].arguments, [assertEntry]); - assert.deepEqual(rule2.mock.calls[0].arguments, [assertEntry]); + assert.deepEqual(rule1.mock.calls[0].arguments, [[assertEntry]]); + assert.deepEqual(rule2.mock.calls[0].arguments, [[assertEntry]]); }); it('should return the aggregated issues from all rules', () => { @@ -26,7 +26,7 @@ describe('createLinterEngine', () => { const engine = createLinterEngine([rule1, rule2]); - const issues = engine.lint(assertEntry); + const issues = engine.lintAll([assertEntry]); assert.equal(issues.length, 3); assert.deepEqual(issues, [infoIssue, warnIssue, errorIssue]); @@ -37,7 +37,7 @@ describe('createLinterEngine', () => { const engine = createLinterEngine([rule]); - const issues = engine.lint(assertEntry); + const issues = engine.lintAll([assertEntry]); assert.deepEqual(issues, []); }); diff --git a/src/linter/tests/rules/duplicate-stability-nodes.test.mjs b/src/linter/tests/rules/duplicate-stability-nodes.test.mjs new file mode 100644 index 0000000..7c3c700 --- /dev/null +++ b/src/linter/tests/rules/duplicate-stability-nodes.test.mjs @@ -0,0 +1,156 @@ +import { describe, it } from 'node:test'; +import { deepStrictEqual } from 'assert'; +import { duplicateStabilityNodes } from '../../rules/duplicate-stability-nodes.mjs'; +import { LINT_MESSAGES } from '../../constants.mjs'; + +// Mock data structure for creating test entries +const createEntry = ( + depth, + stabilityIndex, + source = 'file.yaml', + position = { line: 1, column: 1 } +) => ({ + heading: { data: { depth } }, + stability: { + children: [{ data: { index: stabilityIndex }, children: [{ position }] }], + }, + api_doc_source: source, +}); + +describe('duplicateStabilityNodes', () => { + it('returns empty array when there are no entries', () => { + deepStrictEqual(duplicateStabilityNodes([]), []); + }); + + it('returns empty array when there are no duplicate stability nodes', () => { + const entries = [createEntry(1, 0), createEntry(2, 1), createEntry(3, 2)]; + deepStrictEqual(duplicateStabilityNodes(entries), []); + }); + + it('detects duplicate stability nodes within a chain', () => { + const entries = [ + createEntry(1, 0), + createEntry(2, 0), // Duplicate stability node + ]; + + deepStrictEqual(duplicateStabilityNodes(entries), [ + { + level: 'warn', + message: LINT_MESSAGES.duplicateStabilityNode, + location: { + path: 'file.yaml', + position: { line: 1, column: 1 }, + }, + }, + ]); + }); + + it('resets stability tracking when depth decreases', () => { + const entries = [ + createEntry(1, 0), + createEntry(2, 0), // This should trigger an issue + createEntry(1, 1), + createEntry(2, 1), // This should trigger another issue + ]; + + deepStrictEqual(duplicateStabilityNodes(entries), [ + { + level: 'warn', + message: LINT_MESSAGES.duplicateStabilityNode, + location: { + path: 'file.yaml', + position: { line: 1, column: 1 }, + }, + }, + { + level: 'warn', + message: LINT_MESSAGES.duplicateStabilityNode, + location: { + path: 'file.yaml', + position: { line: 1, column: 1 }, + }, + }, + ]); + }); + + it('handles missing stability nodes gracefully', () => { + const entries = [ + createEntry(1, -1), + createEntry(2, -1), + createEntry(3, 0), + createEntry(4, 0), // This should trigger an issue + ]; + + deepStrictEqual(duplicateStabilityNodes(entries), [ + { + level: 'warn', + message: LINT_MESSAGES.duplicateStabilityNode, + location: { + path: 'file.yaml', + position: { line: 1, column: 1 }, + }, + }, + ]); + }); + + it('handles entries with no stability property gracefully', () => { + const entries = [ + { + heading: { data: { depth: 1 } }, + stability: { children: [] }, + api_doc_source: 'file.yaml', + yaml_position: { line: 2, column: 5 }, + }, + createEntry(2, 0), + ]; + deepStrictEqual(duplicateStabilityNodes(entries), []); + }); + + it('handles entries with undefined stability index', () => { + const entries = [ + createEntry(1, undefined), + createEntry(2, undefined), + createEntry(3, 1), + createEntry(4, 1), // This should trigger an issue + ]; + deepStrictEqual(duplicateStabilityNodes(entries), [ + { + level: 'warn', + message: LINT_MESSAGES.duplicateStabilityNode, + location: { + path: 'file.yaml', + position: { line: 1, column: 1 }, + }, + }, + ]); + }); + + it('handles mixed depths and stability nodes correctly', () => { + const entries = [ + createEntry(1, 0), + createEntry(2, 1), + createEntry(3, 1), // This should trigger an issue + createEntry(2, 2), + createEntry(3, 2), // This should trigger another issue + ]; + + deepStrictEqual(duplicateStabilityNodes(entries), [ + { + level: 'warn', + message: LINT_MESSAGES.duplicateStabilityNode, + location: { + path: 'file.yaml', + position: { line: 1, column: 1 }, + }, + }, + { + level: 'warn', + message: LINT_MESSAGES.duplicateStabilityNode, + location: { + path: 'file.yaml', + position: { line: 1, column: 1 }, + }, + }, + ]); + }); +}); diff --git a/src/linter/tests/rules/invalid-change-version.test.mjs b/src/linter/tests/rules/invalid-change-version.test.mjs index 6f44519..561e055 100644 --- a/src/linter/tests/rules/invalid-change-version.test.mjs +++ b/src/linter/tests/rules/invalid-change-version.test.mjs @@ -5,16 +5,21 @@ import { assertEntry } from '../fixtures/entries.mjs'; describe('invalidChangeVersion', () => { it('should return an empty array if all change versions are valid', () => { - const issues = invalidChangeVersion(assertEntry); + const issues = invalidChangeVersion([assertEntry]); deepEqual(issues, []); }); it('should return an issue if a change version is invalid', () => { - const issues = invalidChangeVersion({ - ...assertEntry, - changes: [...assertEntry.changes, { version: ['v13.9.0', 'REPLACEME'] }], - }); + const issues = invalidChangeVersion([ + { + ...assertEntry, + changes: [ + ...assertEntry.changes, + { version: ['v13.9.0', 'REPLACEME'] }, + ], + }, + ]); deepEqual(issues, [ { diff --git a/src/linter/tests/rules/missing-change-version.test.mjs b/src/linter/tests/rules/missing-change-version.test.mjs index bafab7b..9ca6ce2 100644 --- a/src/linter/tests/rules/missing-change-version.test.mjs +++ b/src/linter/tests/rules/missing-change-version.test.mjs @@ -5,16 +5,18 @@ import { assertEntry } from '../fixtures/entries.mjs'; describe('missingChangeVersion', () => { it('should return an empty array if all change versions are non-empty', () => { - const issues = missingChangeVersion(assertEntry); + const issues = missingChangeVersion([assertEntry]); deepEqual(issues, []); }); it('should return an issue if a change version is missing', () => { - const issues = missingChangeVersion({ - ...assertEntry, - changes: [...assertEntry.changes, { version: undefined }], - }); + const issues = missingChangeVersion([ + { + ...assertEntry, + changes: [...assertEntry.changes, { version: undefined }], + }, + ]); deepEqual(issues, [ { diff --git a/src/linter/tests/rules/missing-introduced-in.test.mjs b/src/linter/tests/rules/missing-introduced-in.test.mjs index f6ca0fe..4671a8b 100644 --- a/src/linter/tests/rules/missing-introduced-in.test.mjs +++ b/src/linter/tests/rules/missing-introduced-in.test.mjs @@ -5,25 +5,29 @@ import { assertEntry } from '../fixtures/entries.mjs'; describe('missingIntroducedIn', () => { it('should return an empty array if the introduced_in field is not missing', () => { - const issues = missingIntroducedIn(assertEntry); + const issues = missingIntroducedIn([assertEntry]); deepEqual(issues, []); }); it('should return an empty array if the heading depth is not equal to 1', () => { - const issues = missingIntroducedIn({ - ...assertEntry, - heading: { ...assertEntry.heading, depth: 2 }, - }); + const issues = missingIntroducedIn([ + { + ...assertEntry, + heading: { ...assertEntry.heading, depth: 2 }, + }, + ]); deepEqual(issues, []); }); it('should return an issue if the introduced_in property is missing', () => { - const issues = missingIntroducedIn({ - ...assertEntry, - introduced_in: undefined, - }); + const issues = missingIntroducedIn([ + { + ...assertEntry, + introduced_in: undefined, + }, + ]); deepEqual(issues, [ { diff --git a/src/linter/types.d.ts b/src/linter/types.d.ts index 719b1fd..cd23651 100644 --- a/src/linter/types.d.ts +++ b/src/linter/types.d.ts @@ -13,6 +13,6 @@ export interface LintIssue { location: LintIssueLocation; } -type LintRule = (input: ApiDocMetadataEntry) => LintIssue[]; +type LintRule = (input: ApiDocMetadataEntry[]) => LintIssue[]; export type Reporter = (msg: LintIssue) => void;