Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add duplicate stability linters #215

Merged
merged 8 commits into from
Mar 25, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/linter/constants.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -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',
};
27 changes: 3 additions & 24 deletions src/linter/engine.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -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
*
Expand All @@ -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,
};
};
Expand Down
38 changes: 38 additions & 0 deletions src/linter/rules/duplicate-stability-nodes.mjs
Original file line number Diff line number Diff line change
@@ -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<import('../types').LintIssue>}
*/
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;
};
2 changes: 2 additions & 0 deletions src/linter/rules/index.mjs
Original file line number Diff line number Diff line change
@@ -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';
Expand All @@ -8,6 +9,7 @@ import { missingIntroducedIn } from './missing-introduced-in.mjs';
* @type {Record<string, import('../types').LintRule>}
*/
export default {
'duplicate-stability-nodes': duplicateStabilityNodes,
'invalid-change-version': invalidChangeVersion,
'missing-change-version': missingChangeVersion,
'missing-introduced-in': missingIntroducedIn,
Expand Down
49 changes: 29 additions & 20 deletions src/linter/rules/invalid-change-version.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -4,30 +4,39 @@ import { valid } from 'semver';
/**
* Checks if any change version is invalid
*
* @param {ApiDocMetadataEntry} entry
* @param {ApiDocMetadataEntry[]} entries
* @returns {Array<import('../types').LintIssue>}
*/
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;
};
34 changes: 20 additions & 14 deletions src/linter/rules/missing-change-version.mjs
Original file line number Diff line number Diff line change
@@ -1,22 +1,28 @@
/**
* Checks if any change version is missing
*
* @param {ApiDocMetadataEntry} entry
* @param {ApiDocMetadataEntry[]} entries
* @returns {Array<import('../types').LintIssue>}
*/
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;
};
22 changes: 12 additions & 10 deletions src/linter/rules/missing-introduced-in.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -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<import('../types.d.ts').LintIssue>}
*/
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;
};
10 changes: 5 additions & 5 deletions src/linter/tests/engine.test.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
Expand All @@ -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]);
Expand All @@ -37,7 +37,7 @@ describe('createLinterEngine', () => {

const engine = createLinterEngine([rule]);

const issues = engine.lint(assertEntry);
const issues = engine.lintAll([assertEntry]);

assert.deepEqual(issues, []);
});
Expand Down
Loading
Loading