Skip to content

Commit 1901729

Browse files
avivkelleravivkeller
committed
fixes
1 parent 6cefedd commit 1901729

12 files changed

+109
-128
lines changed

‎bin/cli.mjs

+1-3
Original file line numberDiff line numberDiff line change
@@ -64,9 +64,7 @@ program
6464
)
6565
.addOption(
6666
new Option('--disable-rule [rule...]', 'Disable a specific linter rule')
67-
.choices(
68-
Object.keys(multiEntryRules).concat(Object.keys(singleEntryRules))
69-
)
67+
.choices(Object.keys(rules))
7068
.default([])
7169
)
7270
.addOption(

‎src/linter/engine.mjs

+3-31
Original file line numberDiff line numberDiff line change
@@ -3,32 +3,9 @@
33
/**
44
* Creates a linter engine instance to validate ApiDocMetadataEntry entries
55
*
6-
* @param {{
7-
* multiEntryRules: import('./types').MultipleEntriesLintRules[]
8-
* singleEntryRules: import('./types').SingleEntryLintRule[]
9-
* }} rules Lint rules to validate the entries against
6+
* @param {import('./types').LintRule[]} rules Lint rules to validate the entries against
107
*/
11-
const createLinterEngine = ({ multiEntryRules, singleEntryRules }) => {
12-
/**
13-
* Validates a ApiDocMetadataEntry entry against all defined rules
14-
*
15-
* @param {ApiDocMetadataEntry} entry
16-
* @returns {import('./types').LintIssue[]}
17-
*/
18-
const lint = entry => {
19-
const issues = [];
20-
21-
for (const rule of singleEntryRules) {
22-
const ruleIssues = rule(entry);
23-
24-
if (ruleIssues.length > 0) {
25-
issues.push(...ruleIssues);
26-
}
27-
}
28-
29-
return issues;
30-
};
31-
8+
const createLinterEngine = rules => {
329
/**
3310
* Validates an array of ApiDocMetadataEntry entries against all defined rules
3411
*
@@ -38,19 +15,14 @@ const createLinterEngine = ({ multiEntryRules, singleEntryRules }) => {
3815
const lintAll = entries => {
3916
const issues = [];
4017

41-
for (const rule of multiEntryRules) {
18+
for (const rule of rules) {
4219
issues.push(...rule(entries));
4320
}
4421

45-
for (const entry of entries) {
46-
issues.push(...lint(entry));
47-
}
48-
4922
return issues;
5023
};
5124

5225
return {
53-
lint,
5426
lintAll,
5527
};
5628
};

‎src/linter/index.mjs

+4-7
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22

33
import createLinterEngine from './engine.mjs';
44
import reporters from './reporters/index.mjs';
5-
import { multiEntryRules, singleEntryRules } from './rules/index.mjs';
5+
import rules from './rules/index.mjs';
66

77
/**
88
* Creates a linter instance to validate ApiDocMetadataEntry entries
@@ -13,19 +13,16 @@ import { multiEntryRules, singleEntryRules } from './rules/index.mjs';
1313
const createLinter = (dryRun, disabledRules) => {
1414
/**
1515
* Retrieves all enabled rules
16-
* @param {Record<string, import('./types').LintRule>} rules
16+
*
1717
* @returns {import('./types').LintRule[]}
1818
*/
19-
const getEnabledRules = rules => {
19+
const getEnabledRules = () => {
2020
return Object.entries(rules)
2121
.filter(([ruleName]) => !disabledRules.includes(ruleName))
2222
.map(([, rule]) => rule);
2323
};
2424

25-
const engine = createLinterEngine({
26-
multiEntryRules: getEnabledRules(multiEntryRules),
27-
singleEntryRules: getEnabledRules(singleEntryRules),
28-
});
25+
const engine = createLinterEngine(getEnabledRules(disabledRules));
2926

3027
/**
3128
* Lint issues found during validations

‎src/linter/rules/index.mjs

+3-9
Original file line numberDiff line numberDiff line change
@@ -6,17 +6,11 @@ import { missingChangeVersion } from './missing-change-version.mjs';
66
import { missingIntroducedIn } from './missing-introduced-in.mjs';
77

88
/**
9-
* @type {Record<string, import('../types').SingleEntryLintRule>}
9+
* @type {Record<string, import('../types').LintRule>}
1010
*/
11-
export const singleEntryRules = {
11+
export default {
12+
'duplicate-stability-nodes': duplicateStabilityNodes,
1213
'invalid-change-version': invalidChangeVersion,
1314
'missing-change-version': missingChangeVersion,
1415
'missing-introduced-in': missingIntroducedIn,
1516
};
16-
17-
/**
18-
* @type {Record<string, import('../types').MultipleEntriesLintRule>}
19-
*/
20-
export const multiEntryRules = {
21-
'duplicate-stability-nodes': duplicateStabilityNodes,
22-
};

‎src/linter/rules/invalid-change-version.mjs

+29-20
Original file line numberDiff line numberDiff line change
@@ -4,30 +4,39 @@ import { valid } from 'semver';
44
/**
55
* Checks if any change version is invalid
66
*
7-
* @param {ApiDocMetadataEntry} entry
7+
* @param {ApiDocMetadataEntry[]} entries
88
* @returns {Array<import('../types').LintIssue>}
99
*/
10-
export const invalidChangeVersion = entry => {
11-
if (entry.changes.length === 0) {
12-
return [];
13-
}
10+
export const invalidChangeVersion = entries => {
11+
const issues = [];
12+
13+
for (const entry of entries) {
14+
if (entry.changes.length === 0) continue;
15+
16+
const allVersions = entry.changes
17+
.filter(change => change.version)
18+
.flatMap(change =>
19+
Array.isArray(change.version) ? change.version : [change.version]
20+
);
1421

15-
const allVersions = entry.changes
16-
.filter(change => change.version)
17-
.flatMap(change =>
18-
Array.isArray(change.version) ? change.version : [change.version]
22+
const invalidVersions = allVersions.filter(
23+
version => valid(version) === null
1924
);
2025

21-
const invalidVersions = allVersions.filter(
22-
version => valid(version) === null
23-
);
26+
issues.push(
27+
...invalidVersions.map(version => ({
28+
level: 'warn',
29+
message: LINT_MESSAGES.invalidChangeVersion.replace(
30+
'{{version}}',
31+
version
32+
),
33+
location: {
34+
path: entry.api_doc_source,
35+
position: entry.yaml_position,
36+
},
37+
}))
38+
);
39+
}
2440

25-
return invalidVersions.map(version => ({
26-
level: 'warn',
27-
message: LINT_MESSAGES.invalidChangeVersion.replace('{{version}}', version),
28-
location: {
29-
path: entry.api_doc_source,
30-
position: entry.yaml_position,
31-
},
32-
}));
41+
return issues;
3342
};

‎src/linter/rules/missing-change-version.mjs

+20-14
Original file line numberDiff line numberDiff line change
@@ -1,22 +1,28 @@
11
/**
22
* Checks if any change version is missing
33
*
4-
* @param {ApiDocMetadataEntry} entry
4+
* @param {ApiDocMetadataEntry[]} entries
55
* @returns {Array<import('../types').LintIssue>}
66
*/
7-
export const missingChangeVersion = entry => {
8-
if (entry.changes.length === 0) {
9-
return [];
7+
export const missingChangeVersion = entries => {
8+
const issues = [];
9+
10+
for (const entry of entries) {
11+
if (entry.changes.length === 0) continue;
12+
13+
issues.push(
14+
...entry.changes
15+
.filter(change => !change.version)
16+
.map(() => ({
17+
level: 'warn',
18+
message: 'Missing change version',
19+
location: {
20+
path: entry.api_doc_source,
21+
position: entry.yaml_position,
22+
},
23+
}))
24+
);
1025
}
1126

12-
return entry.changes
13-
.filter(change => !change.version)
14-
.map(() => ({
15-
level: 'warn',
16-
message: 'Missing change version',
17-
location: {
18-
path: entry.api_doc_source,
19-
position: entry.yaml_position,
20-
},
21-
}));
27+
return issues;
2228
};

‎src/linter/rules/missing-introduced-in.mjs

+12-10
Original file line numberDiff line numberDiff line change
@@ -3,22 +3,24 @@ import { LINT_MESSAGES } from '../constants.mjs';
33
/**
44
* Checks if `introduced_in` field is missing
55
*
6-
* @param {ApiDocMetadataEntry} entry
6+
* @param {ApiDocMetadataEntry[]} entries
77
* @returns {Array<import('../types.d.ts').LintIssue>}
88
*/
9-
export const missingIntroducedIn = entry => {
10-
// Early return if not a top-level heading or if introduced_in exists
11-
if (entry.heading.depth !== 1 || entry.introduced_in) {
12-
return [];
13-
}
9+
export const missingIntroducedIn = entries => {
10+
const issues = [];
11+
12+
for (const entry of entries) {
13+
// Early continue if not a top-level heading or if introduced_in exists
14+
if (entry.heading.depth !== 1 || entry.introduced_in) continue;
1415

15-
return [
16-
{
16+
issues.push({
1717
level: 'info',
1818
message: LINT_MESSAGES.missingIntroducedIn,
1919
location: {
2020
path: entry.api_doc_source,
2121
},
22-
},
23-
];
22+
});
23+
}
24+
25+
return issues;
2426
};

‎src/linter/tests/engine.test.mjs

+6-12
Original file line numberDiff line numberDiff line change
@@ -9,41 +9,35 @@ describe('createLinterEngine', () => {
99
const rule1 = mock.fn(() => []);
1010
const rule2 = mock.fn(() => []);
1111

12-
const engine = createLinterEngine({
13-
singleEntryRules: [rule1],
14-
multiEntryRules: [rule2],
15-
});
12+
const engine = createLinterEngine([rule1, rule2]);
1613

1714
engine.lintAll([assertEntry]);
1815

1916
assert.strictEqual(rule1.mock.callCount(), 1);
2017
assert.strictEqual(rule2.mock.callCount(), 1);
2118

22-
assert.deepEqual(rule1.mock.calls[0].arguments, [assertEntry]);
19+
assert.deepEqual(rule1.mock.calls[0].arguments, [[assertEntry]]);
2320
assert.deepEqual(rule2.mock.calls[0].arguments, [[assertEntry]]);
2421
});
2522

2623
it('should return the aggregated issues from all rules', () => {
2724
const rule1 = mock.fn(() => [infoIssue, warnIssue]);
2825
const rule2 = mock.fn(() => [errorIssue]);
2926

30-
const engine = createLinterEngine({
31-
singleEntryRules: [rule1],
32-
multiEntryRules: [rule2],
33-
});
27+
const engine = createLinterEngine([rule1, rule2]);
3428

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

3731
assert.equal(issues.length, 3);
38-
assert.deepEqual(issues, [errorIssue, infoIssue, warnIssue]);
32+
assert.deepEqual(issues, [infoIssue, warnIssue, errorIssue]);
3933
});
4034

4135
it('should return an empty array when no issues are found', () => {
4236
const rule = () => [];
4337

44-
const engine = createLinterEngine({ singleEntryRules: [rule] });
38+
const engine = createLinterEngine([rule]);
4539

46-
const issues = engine.lint(assertEntry);
40+
const issues = engine.lintAll([assertEntry]);
4741

4842
assert.deepEqual(issues, []);
4943
});

‎src/linter/tests/rules/invalid-change-version.test.mjs

+10-5
Original file line numberDiff line numberDiff line change
@@ -5,16 +5,21 @@ import { assertEntry } from '../fixtures/entries.mjs';
55

66
describe('invalidChangeVersion', () => {
77
it('should return an empty array if all change versions are valid', () => {
8-
const issues = invalidChangeVersion(assertEntry);
8+
const issues = invalidChangeVersion([assertEntry]);
99

1010
deepEqual(issues, []);
1111
});
1212

1313
it('should return an issue if a change version is invalid', () => {
14-
const issues = invalidChangeVersion({
15-
...assertEntry,
16-
changes: [...assertEntry.changes, { version: ['v13.9.0', 'REPLACEME'] }],
17-
});
14+
const issues = invalidChangeVersion([
15+
{
16+
...assertEntry,
17+
changes: [
18+
...assertEntry.changes,
19+
{ version: ['v13.9.0', 'REPLACEME'] },
20+
],
21+
},
22+
]);
1823

1924
deepEqual(issues, [
2025
{

‎src/linter/tests/rules/missing-change-version.test.mjs

+7-5
Original file line numberDiff line numberDiff line change
@@ -5,16 +5,18 @@ import { assertEntry } from '../fixtures/entries.mjs';
55

66
describe('missingChangeVersion', () => {
77
it('should return an empty array if all change versions are non-empty', () => {
8-
const issues = missingChangeVersion(assertEntry);
8+
const issues = missingChangeVersion([assertEntry]);
99

1010
deepEqual(issues, []);
1111
});
1212

1313
it('should return an issue if a change version is missing', () => {
14-
const issues = missingChangeVersion({
15-
...assertEntry,
16-
changes: [...assertEntry.changes, { version: undefined }],
17-
});
14+
const issues = missingChangeVersion([
15+
{
16+
...assertEntry,
17+
changes: [...assertEntry.changes, { version: undefined }],
18+
},
19+
]);
1820

1921
deepEqual(issues, [
2022
{

‎src/linter/tests/rules/missing-introduced-in.test.mjs

+13-9
Original file line numberDiff line numberDiff line change
@@ -5,25 +5,29 @@ import { assertEntry } from '../fixtures/entries.mjs';
55

66
describe('missingIntroducedIn', () => {
77
it('should return an empty array if the introduced_in field is not missing', () => {
8-
const issues = missingIntroducedIn(assertEntry);
8+
const issues = missingIntroducedIn([assertEntry]);
99

1010
deepEqual(issues, []);
1111
});
1212

1313
it('should return an empty array if the heading depth is not equal to 1', () => {
14-
const issues = missingIntroducedIn({
15-
...assertEntry,
16-
heading: { ...assertEntry.heading, depth: 2 },
17-
});
14+
const issues = missingIntroducedIn([
15+
{
16+
...assertEntry,
17+
heading: { ...assertEntry.heading, depth: 2 },
18+
},
19+
]);
1820

1921
deepEqual(issues, []);
2022
});
2123

2224
it('should return an issue if the introduced_in property is missing', () => {
23-
const issues = missingIntroducedIn({
24-
...assertEntry,
25-
introduced_in: undefined,
26-
});
25+
const issues = missingIntroducedIn([
26+
{
27+
...assertEntry,
28+
introduced_in: undefined,
29+
},
30+
]);
2731

2832
deepEqual(issues, [
2933
{

0 commit comments

Comments
 (0)