diff --git a/.github/workflows/pr.yml b/.github/workflows/pr.yml index 1f2f2c4a..ccab0879 100644 --- a/.github/workflows/pr.yml +++ b/.github/workflows/pr.yml @@ -9,6 +9,9 @@ on: permissions: contents: read +env: + FORCE_COLOR: 1 + jobs: build: runs-on: ubuntu-latest diff --git a/bin/cli.mjs b/bin/cli.mjs index ba6f8eaa..1d0620d8 100755 --- a/bin/cli.mjs +++ b/bin/cli.mjs @@ -1,7 +1,7 @@ #!/usr/bin/env node import { resolve } from 'node:path'; -import { argv } from 'node:process'; +import { argv, exit } from 'node:process'; import { Command, Option } from 'commander'; @@ -12,6 +12,9 @@ import generators from '../src/generators/index.mjs'; import createMarkdownLoader from '../src/loaders/markdown.mjs'; import createMarkdownParser from '../src/parsers/markdown.mjs'; import createNodeReleases from '../src/releases.mjs'; +import createLinter from '../src/linter/index.mjs'; +import reporters from '../src/linter/reporters/index.mjs'; +import rules from '../src/linter/rules/index.mjs'; const availableGenerators = Object.keys(generators); @@ -50,6 +53,19 @@ program 'Set the processing target modes' ).choices(availableGenerators) ) + .addOption( + new Option('--disable-rule [rule...]', 'Disable a specific linter rule') + .choices(Object.keys(rules)) + .default([]) + ) + .addOption( + new Option('--lint-dry-run', 'Run linter in dry-run mode').default(false) + ) + .addOption( + new Option('-r, --reporter [reporter]', 'Specify the linter reporter') + .choices(Object.keys(reporters)) + .default('console') + ) .parse(argv); /** @@ -60,13 +76,27 @@ program * @property {string} output Specifies the directory where output files will be saved. * @property {Target[]} target Specifies the generator target mode. * @property {string} version Specifies the target Node.js version. - * @property {string} changelog Specifies the path to the Node.js CHANGELOG.md file + * @property {string} changelog Specifies the path to the Node.js CHANGELOG.md file. + * @property {string[]} disableRule Specifies the linter rules to disable. + * @property {boolean} lintDryRun Specifies whether the linter should run in dry-run mode. + * @property {keyof reporters} reporter Specifies the linter reporter. * * @name ProgramOptions * @type {Options} * @description The return type for values sent to the program from the CLI. */ -const { input, output, target = [], version, changelog } = program.opts(); +const { + input, + output, + target = [], + version, + changelog, + disableRule, + lintDryRun, + reporter, +} = program.opts(); + +const linter = createLinter(lintDryRun, disableRule); const { loadFiles } = createMarkdownLoader(); const { parseApiDocs } = createMarkdownParser(); @@ -80,6 +110,8 @@ const { runGenerators } = createGenerator(parsedApiDocs); // Retrieves Node.js release metadata from a given Node.js version and CHANGELOG.md file const { getAllMajors } = createNodeReleases(changelog); +linter.lintAll(parsedApiDocs); + await runGenerators({ // A list of target modes for the API docs parser generators: target, @@ -92,3 +124,7 @@ await runGenerators({ // A list of all Node.js major versions with LTS status releases: await getAllMajors(), }); + +linter.report(reporter); + +exit(Number(linter.hasError())); diff --git a/package-lock.json b/package-lock.json index 777215e9..8e0f7735 100644 --- a/package-lock.json +++ b/package-lock.json @@ -6,6 +6,7 @@ "": { "name": "@node-core/api-docs-tooling", "dependencies": { + "@actions/core": "^1.11.1", "acorn": "^8.14.0", "commander": "^13.1.0", "dedent": "^1.5.3", @@ -48,6 +49,41 @@ "prettier": "3.4.2" } }, + "node_modules/@actions/core": { + "version": "1.11.1", + "resolved": "https://registry.npmjs.org/@actions/core/-/core-1.11.1.tgz", + "integrity": "sha512-hXJCSrkwfA46Vd9Z3q4cpEpHB1rL5NG04+/rbqW9d3+CSvtB1tYe8UTpAlixa1vj0m/ULglfEK2UKxMGxCxv5A==", + "license": "MIT", + "dependencies": { + "@actions/exec": "^1.1.1", + "@actions/http-client": "^2.0.1" + } + }, + "node_modules/@actions/exec": { + "version": "1.1.1", + "resolved": "https://registry.npmjs.org/@actions/exec/-/exec-1.1.1.tgz", + "integrity": "sha512-+sCcHHbVdk93a0XT19ECtO/gIXoxvdsgQLzb2fE2/5sIZmWQuluYyjPQtrtTHdU1YzTZ7bAPN4sITq2xi1679w==", + "license": "MIT", + "dependencies": { + "@actions/io": "^1.0.1" + } + }, + "node_modules/@actions/http-client": { + "version": "2.2.3", + "resolved": "https://registry.npmjs.org/@actions/http-client/-/http-client-2.2.3.tgz", + "integrity": "sha512-mx8hyJi/hjFvbPokCg4uRd4ZX78t+YyRPtnKWwIl+RzNaVuFpQHfmlGVfsKEJN8LwTCvL+DfVgAM04XaHkm6bA==", + "license": "MIT", + "dependencies": { + "tunnel": "^0.0.6", + "undici": "^5.25.4" + } + }, + "node_modules/@actions/io": { + "version": "1.1.3", + "resolved": "https://registry.npmjs.org/@actions/io/-/io-1.1.3.tgz", + "integrity": "sha512-wi9JjgKLYS7U/z8PPbco+PvTb/nRWjeoFlJ1Qer83k/3C5PHQi28hiVdeE2kHXmIL99mQFawx8qt/JPjZilJ8Q==", + "license": "MIT" + }, "node_modules/@es-joy/jsdoccomment": { "version": "0.49.0", "resolved": "https://registry.npmjs.org/@es-joy/jsdoccomment/-/jsdoccomment-0.49.0.tgz", @@ -201,6 +237,15 @@ "node": "^18.18.0 || ^20.9.0 || >=21.1.0" } }, + "node_modules/@fastify/busboy": { + "version": "2.1.1", + "resolved": "https://registry.npmjs.org/@fastify/busboy/-/busboy-2.1.1.tgz", + "integrity": "sha512-vBZP4NlzfOlerQTnba4aqZoMhE/a9HY7HRqoOPaETQcSQuWEIyZMHGfVu6w9wGtGK5fED5qRs2DteVCjOH60sA==", + "license": "MIT", + "engines": { + "node": ">=14" + } + }, "node_modules/@humanfs/core": { "version": "0.19.1", "resolved": "https://registry.npmjs.org/@humanfs/core/-/core-0.19.1.tgz", @@ -3925,6 +3970,15 @@ "integrity": "sha512-gLXCKdN1/j47AiHiOkJN69hJmcbGTHI0ImLmbYLHykhgeN0jVGola9yVjFgzCUklsZQMW55o+dW7IXv3RCXDzA==", "license": "0BSD" }, + "node_modules/tunnel": { + "version": "0.0.6", + "resolved": "https://registry.npmjs.org/tunnel/-/tunnel-0.0.6.tgz", + "integrity": "sha512-1h/Lnq9yajKY2PEbBadPXj3VxsDDu844OnaAo52UVmIzIvwwtBPIuNvkjuzBlTWpfJyUbG3ez0KSBibQkj4ojg==", + "license": "MIT", + "engines": { + "node": ">=0.6.11 <=0.7.0 || >=0.7.3" + } + }, "node_modules/type-check": { "version": "0.4.0", "resolved": "https://registry.npmjs.org/type-check/-/type-check-0.4.0.tgz", @@ -3938,6 +3992,18 @@ "node": ">= 0.8.0" } }, + "node_modules/undici": { + "version": "5.28.5", + "resolved": "https://registry.npmjs.org/undici/-/undici-5.28.5.tgz", + "integrity": "sha512-zICwjrDrcrUE0pyyJc1I2QzBkLM8FINsgOrt6WjA+BgajVq9Nxu2PbFFXUrAggLfDXlZGZBVZYw7WNV5KiBiBA==", + "license": "MIT", + "dependencies": { + "@fastify/busboy": "^2.0.0" + }, + "engines": { + "node": ">=14.0" + } + }, "node_modules/undici-types": { "version": "6.20.0", "resolved": "https://registry.npmjs.org/undici-types/-/undici-types-6.20.0.tgz", diff --git a/package.json b/package.json index e335033d..d2c933a4 100644 --- a/package.json +++ b/package.json @@ -30,6 +30,7 @@ }, "dependencies": { "acorn": "^8.14.0", + "@actions/core": "^1.11.1", "commander": "^13.1.0", "estree-util-visit": "^2.0.0", "dedent": "^1.5.3", diff --git a/src/constants.mjs b/src/constants.mjs index 803bd2db..473cf35b 100644 --- a/src/constants.mjs +++ b/src/constants.mjs @@ -125,17 +125,45 @@ export const DOC_SLUG_ENVIRONMENT = 'environment-variables-1'; // JavaScript globals types within the MDN JavaScript docs // @see DOC_MDN_BASE_URL_JS_GLOBALS export const DOC_TYPES_MAPPING_GLOBALS = { - ...Object.fromEntries([ - 'AggregateError', 'Array', 'ArrayBuffer', 'DataView', 'Date', 'Error', - 'EvalError', 'Function', 'Map', 'NaN', 'Object', 'Promise', 'Proxy', 'RangeError', - 'ReferenceError', 'RegExp', 'Set', 'SharedArrayBuffer', 'SyntaxError', 'Symbol', - 'TypeError', 'URIError', 'WeakMap', 'WeakSet', - - 'TypedArray', - 'Float32Array', 'Float64Array', - 'Int8Array', 'Int16Array', 'Int32Array', - 'Uint8Array', 'Uint8ClampedArray', 'Uint16Array', 'Uint32Array', - ].map(e => [e, e])), + ...Object.fromEntries( + [ + 'AggregateError', + 'Array', + 'ArrayBuffer', + 'DataView', + 'Date', + 'Error', + 'EvalError', + 'Function', + 'Map', + 'NaN', + 'Object', + 'Promise', + 'Proxy', + 'RangeError', + 'ReferenceError', + 'RegExp', + 'Set', + 'SharedArrayBuffer', + 'SyntaxError', + 'Symbol', + 'TypeError', + 'URIError', + 'WeakMap', + 'WeakSet', + + 'TypedArray', + 'Float32Array', + 'Float64Array', + 'Int8Array', + 'Int16Array', + 'Int32Array', + 'Uint8Array', + 'Uint8ClampedArray', + 'Uint16Array', + 'Uint32Array', + ].map(e => [e, e]) + ), bigint: 'BigInt', 'WebAssembly.Instance': 'WebAssembly/Instance', }; @@ -392,3 +420,9 @@ export const DOC_TYPES_MAPPING_OTHER = { Response: `${DOC_MDN_BASE_URL}/API/Response`, Request: `${DOC_MDN_BASE_URL}/API/Request`, }; + +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}}', +}; diff --git a/src/linter/engine.mjs b/src/linter/engine.mjs new file mode 100644 index 00000000..41d8f3a2 --- /dev/null +++ b/src/linter/engine.mjs @@ -0,0 +1,51 @@ +'use strict'; + +/** + * Creates a linter engine instance to validate ApiDocMetadataEntry entries + * + * @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 + * + * @param {ApiDocMetadataEntry[]} entries + * @returns {import('./types').LintIssue[]} + */ + const lintAll = entries => { + const issues = []; + + for (const entry of entries) { + issues.push(...lint(entry)); + } + + return issues; + }; + + return { + lint, + lintAll, + }; +}; + +export default createLinterEngine; diff --git a/src/linter/index.mjs b/src/linter/index.mjs new file mode 100644 index 00000000..689c6a63 --- /dev/null +++ b/src/linter/index.mjs @@ -0,0 +1,77 @@ +'use strict'; + +import createLinterEngine from './engine.mjs'; +import reporters from './reporters/index.mjs'; +import rules from './rules/index.mjs'; + +/** + * Creates a linter instance to validate ApiDocMetadataEntry entries + * + * @param {boolean} dryRun Whether to run the engine in dry-run mode + * @param {string[]} disabledRules List of disabled rules names + */ +const createLinter = (dryRun, disabledRules) => { + const engine = createLinterEngine(getEnabledRules(disabledRules)); + + /** + * Lint issues found during validations + * + * @type {Array} + */ + const issues = []; + + /** + * Lints all entries using the linter engine + * + * @param entries + */ + const lintAll = entries => { + issues.push(...engine.lintAll(entries)); + }; + + /** + * Reports found issues using the specified reporter + * + * @param {keyof typeof reporters} reporterName Reporter name + * @returns {void} + */ + const report = reporterName => { + if (dryRun) { + return; + } + + const reporter = reporters[reporterName]; + + for (const issue of issues) { + reporter(issue); + } + }; + + /** + * Checks if any error-level issues were found during linting + * + * @returns {boolean} + */ + const hasError = () => { + return issues.some(issue => issue.level === 'error'); + }; + + /** + * Retrieves all enabled rules + * + * @returns {import('./types').LintRule[]} + */ + const getEnabledRules = () => { + return Object.entries(rules) + .filter(([ruleName]) => !disabledRules.includes(ruleName)) + .map(([, rule]) => rule); + }; + + return { + lintAll, + report, + hasError, + }; +}; + +export default createLinter; diff --git a/src/linter/reporters/console.mjs b/src/linter/reporters/console.mjs new file mode 100644 index 00000000..58975931 --- /dev/null +++ b/src/linter/reporters/console.mjs @@ -0,0 +1,30 @@ +'use strict'; + +import { styleText } from 'node:util'; + +/** + * @type {Record} + */ +const levelToColorMap = { + info: 'gray', + warn: 'yellow', + error: 'red', +}; + +/** + * Console reporter + * + * @type {import('../types.d.ts').Reporter} + */ +export default issue => { + const position = issue.location.position + ? ` (${issue.location.position.start.line}:${issue.location.position.end.line})` + : ''; + + process.stdout.write( + styleText( + levelToColorMap[issue.level], + `${issue.message} at ${issue.location.path}${position}` + ) + '\n' + ); +}; diff --git a/src/linter/reporters/github.mjs b/src/linter/reporters/github.mjs new file mode 100644 index 00000000..114a0550 --- /dev/null +++ b/src/linter/reporters/github.mjs @@ -0,0 +1,24 @@ +'use strict'; + +import * as core from '@actions/core'; + +const actions = { + warn: core.warning, + error: core.error, + info: core.notice, +}; + +/** + * GitHub actions reporter + * + * @type {import('../types.d.ts').Reporter} + */ +export default issue => { + const logFn = actions[issue.level] || core.notice; + + logFn(issue.message, { + file: issue.location.path, + startLine: issue.location.position?.start.line, + endLine: issue.location.position?.end.line, + }); +}; diff --git a/src/linter/reporters/index.mjs b/src/linter/reporters/index.mjs new file mode 100644 index 00000000..a49d9b6b --- /dev/null +++ b/src/linter/reporters/index.mjs @@ -0,0 +1,9 @@ +'use strict'; + +import console from './console.mjs'; +import github from './github.mjs'; + +export default { + console: console, + github: github, +}; diff --git a/src/linter/rules/index.mjs b/src/linter/rules/index.mjs new file mode 100644 index 00000000..b780eca9 --- /dev/null +++ b/src/linter/rules/index.mjs @@ -0,0 +1,14 @@ +'use strict'; + +import { invalidChangeVersion } from './invalid-change-version.mjs'; +import { missingChangeVersion } from './missing-change-version.mjs'; +import { missingIntroducedIn } from './missing-introduced-in.mjs'; + +/** + * @type {Record} + */ +export default { + '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 new file mode 100644 index 00000000..779901aa --- /dev/null +++ b/src/linter/rules/invalid-change-version.mjs @@ -0,0 +1,33 @@ +import { LINT_MESSAGES } from '../../constants.mjs'; +import { valid } from 'semver'; + +/** + * Checks if any change version is invalid + * + * @param {ApiDocMetadataEntry} entry + * @returns {Array} + */ +export const invalidChangeVersion = entry => { + if (entry.changes.length === 0) { + return []; + } + + 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 + ); + + return invalidVersions.map(version => ({ + level: 'warn', + message: LINT_MESSAGES.invalidChangeVersion.replace('{{version}}', version), + location: { + path: entry.api_doc_source, + position: entry.yaml_position, + }, + })); +}; diff --git a/src/linter/rules/missing-change-version.mjs b/src/linter/rules/missing-change-version.mjs new file mode 100644 index 00000000..67b42a19 --- /dev/null +++ b/src/linter/rules/missing-change-version.mjs @@ -0,0 +1,22 @@ +/** + * Checks if any change version is missing + * + * @param {ApiDocMetadataEntry} entry + * @returns {Array} + */ +export const missingChangeVersion = entry => { + if (entry.changes.length === 0) { + return []; + } + + return entry.changes + .filter(change => !change.version) + .map(() => ({ + level: 'warn', + message: 'Missing change version', + location: { + path: entry.api_doc_source, + position: entry.yaml_position, + }, + })); +}; diff --git a/src/linter/rules/missing-introduced-in.mjs b/src/linter/rules/missing-introduced-in.mjs new file mode 100644 index 00000000..80224c9d --- /dev/null +++ b/src/linter/rules/missing-introduced-in.mjs @@ -0,0 +1,24 @@ +import { LINT_MESSAGES } from '../../constants.mjs'; + +/** + * Checks if `introduced_in` field is missing + * + * @param {ApiDocMetadataEntry} entry + * @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 []; + } + + return [ + { + level: 'info', + message: LINT_MESSAGES.missingIntroducedIn, + location: { + path: entry.api_doc_source, + }, + }, + ]; +}; diff --git a/src/linter/tests/engine.test.mjs b/src/linter/tests/engine.test.mjs new file mode 100644 index 00000000..cbbd1c5a --- /dev/null +++ b/src/linter/tests/engine.test.mjs @@ -0,0 +1,44 @@ +import { describe, mock, it } from 'node:test'; +import assert from 'node:assert/strict'; +import createLinterEngine from '../engine.mjs'; +import { assertEntry } from './fixtures/entries.mjs'; +import { errorIssue, infoIssue, warnIssue } from './fixtures/issues.mjs'; + +describe('createLinterEngine', () => { + it('should call each rule with the provided entry', () => { + const rule1 = mock.fn(() => []); + const rule2 = mock.fn(() => []); + + const engine = createLinterEngine([rule1, rule2]); + + engine.lint(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]); + }); + + it('should return the aggregated issues from all rules', () => { + const rule1 = mock.fn(() => [infoIssue, warnIssue]); + const rule2 = mock.fn(() => [errorIssue]); + + const engine = createLinterEngine([rule1, rule2]); + + const issues = engine.lint(assertEntry); + + assert.equal(issues.length, 3); + assert.deepEqual(issues, [infoIssue, warnIssue, errorIssue]); + }); + + it('should return an empty array when no issues are found', () => { + const rule = () => []; + + const engine = createLinterEngine([rule]); + + const issues = engine.lint(assertEntry); + + assert.deepEqual(issues, []); + }); +}); diff --git a/src/linter/tests/fixtures/entries.mjs b/src/linter/tests/fixtures/entries.mjs new file mode 100644 index 00000000..89f717c9 --- /dev/null +++ b/src/linter/tests/fixtures/entries.mjs @@ -0,0 +1,84 @@ +/** + * Noop function. + * + * @returns {any} + */ +const noop = () => {}; + +/** + * @type {ApiDocMetadataEntry} + */ +export const assertEntry = { + api: 'assert', + slug: 'assert', + source_link: 'lib/assert.js', + api_doc_source: 'doc/api/assert.md', + added_in: undefined, + deprecated_in: undefined, + removed_in: undefined, + n_api_version: undefined, + changes: [ + { + version: 'v9.9.0', + 'pr-url': 'https://github.com/nodejs/node/pull/17615', + description: 'Added error diffs to the strict assertion mode.', + }, + { + version: 'v9.9.0', + 'pr-url': 'https://github.com/nodejs/node/pull/17002', + description: 'Added strict assertion mode to the assert module.', + }, + { + version: ['v13.9.0', 'v12.16.2'], + 'pr-url': 'https://github.com/nodejs/node/pull/31635', + description: + 'Changed "strict mode" to "strict assertion mode" and "legacy mode" to "legacy assertion mode" to avoid confusion with the more usual meaning of "strict mode".', + }, + { + version: 'v15.0.0', + 'pr-url': 'https://github.com/nodejs/node/pull/34001', + description: "Exposed as `require('node:assert/strict')`.", + }, + ], + heading: { + type: 'heading', + depth: 1, + children: [ + { + type: 'text', + value: 'Assert', + position: { + start: { line: 1, column: 3, offset: 2 }, + end: { line: 1, column: 9, offset: 8 }, + }, + }, + ], + position: { + start: { line: 1, column: 1, offset: 0 }, + end: { line: 1, column: 9, offset: 8 }, + }, + data: { + text: 'Assert', + name: 'Assert', + depth: 1, + slug: 'assert', + type: 'property', + }, + toJSON: noop, + }, + stability: { + type: 'root', + children: [], + toJSON: noop, + }, + content: { + type: 'root', + children: [], + }, + tags: [], + introduced_in: 'v0.1.21', + yaml_position: { + start: { line: 7, column: 1, offset: 103 }, + end: { line: 7, column: 35, offset: 137 }, + }, +}; diff --git a/src/linter/tests/fixtures/issues.mjs b/src/linter/tests/fixtures/issues.mjs new file mode 100644 index 00000000..1546e8bc --- /dev/null +++ b/src/linter/tests/fixtures/issues.mjs @@ -0,0 +1,36 @@ +// @ts-check + +/** + * @type {import('../../types').LintIssue} + */ +export const infoIssue = { + level: 'info', + location: { + path: 'doc/api/test.md', + }, + message: 'This is a INFO issue', +}; + +/** + * @type {import('../../types').LintIssue} + */ +export const warnIssue = { + level: 'warn', + location: { + path: 'doc/api/test.md', + position: { start: { line: 1, column: 1 }, end: { line: 1, column: 2 } }, + }, + message: 'This is a WARN issue', +}; + +/** + * @type {import('../../types').LintIssue} + */ +export const errorIssue = { + level: 'error', + location: { + path: 'doc/api/test.md', + position: { start: { line: 1, column: 1 }, end: { line: 1, column: 2 } }, + }, + message: 'This is a ERROR issue', +}; diff --git a/src/linter/tests/reporters/console.test.mjs b/src/linter/tests/reporters/console.test.mjs new file mode 100644 index 00000000..4960c9df --- /dev/null +++ b/src/linter/tests/reporters/console.test.mjs @@ -0,0 +1,26 @@ +import { describe, it } from 'node:test'; +import console from '../../reporters/console.mjs'; +import assert from 'node:assert'; +import { errorIssue, infoIssue, warnIssue } from '../fixtures/issues.mjs'; + +describe('console', () => { + it('should write to stdout with the correct colors based on the issue level', t => { + t.mock.method(process.stdout, 'write'); + + console(infoIssue); + console(warnIssue); + console(errorIssue); + + assert.strictEqual(process.stdout.write.mock.callCount(), 3); + + const callsArgs = process.stdout.write.mock.calls.map( + call => call.arguments[0] + ); + + assert.deepStrictEqual(callsArgs, [ + '\x1B[90mThis is a INFO issue at doc/api/test.md\x1B[39m\n', + '\x1B[33mThis is a WARN issue at doc/api/test.md (1:1)\x1B[39m\n', + '\x1B[31mThis is a ERROR issue at doc/api/test.md (1:1)\x1B[39m\n', + ]); + }); +}); diff --git a/src/linter/tests/reporters/gihub.test.mjs b/src/linter/tests/reporters/gihub.test.mjs new file mode 100644 index 00000000..09e3e4b3 --- /dev/null +++ b/src/linter/tests/reporters/gihub.test.mjs @@ -0,0 +1,26 @@ +import { describe, it } from 'node:test'; +import github from '../../reporters/github.mjs'; +import assert from 'node:assert'; +import { errorIssue, infoIssue, warnIssue } from '../fixtures/issues.mjs'; + +describe('github', () => { + it('should write to stdout with the correct fn based on the issue level', t => { + t.mock.method(process.stdout, 'write'); + + github(infoIssue); + github(warnIssue); + github(errorIssue); + + assert.strictEqual(process.stdout.write.mock.callCount(), 3); + + const callsArgs = process.stdout.write.mock.calls.map( + call => call.arguments[0] + ); + + assert.deepStrictEqual(callsArgs, [ + '::notice file=doc/api/test.md::This is a INFO issue\n', + '::warning file=doc/api/test.md,line=1,endLine=1::This is a WARN issue\n', + '::error file=doc/api/test.md,line=1,endLine=1::This is a ERROR issue\n', + ]); + }); +}); diff --git a/src/linter/tests/rules/invalid-change-version.test.mjs b/src/linter/tests/rules/invalid-change-version.test.mjs new file mode 100644 index 00000000..6f445194 --- /dev/null +++ b/src/linter/tests/rules/invalid-change-version.test.mjs @@ -0,0 +1,41 @@ +import { describe, it } from 'node:test'; +import { invalidChangeVersion } from '../../rules/invalid-change-version.mjs'; +import { deepEqual } from 'node:assert'; +import { assertEntry } from '../fixtures/entries.mjs'; + +describe('invalidChangeVersion', () => { + it('should return an empty array if all change versions are valid', () => { + 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'] }], + }); + + deepEqual(issues, [ + { + level: 'warn', + location: { + path: 'doc/api/assert.md', + position: { + end: { + column: 35, + line: 7, + offset: 137, + }, + start: { + column: 1, + line: 7, + offset: 103, + }, + }, + }, + message: 'Invalid version number: REPLACEME', + }, + ]); + }); +}); diff --git a/src/linter/tests/rules/missing-change-version.test.mjs b/src/linter/tests/rules/missing-change-version.test.mjs new file mode 100644 index 00000000..bafab7be --- /dev/null +++ b/src/linter/tests/rules/missing-change-version.test.mjs @@ -0,0 +1,33 @@ +import { describe, it } from 'node:test'; +import { deepEqual } from 'node:assert'; +import { missingChangeVersion } from '../../rules/missing-change-version.mjs'; +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); + + deepEqual(issues, []); + }); + + it('should return an issue if a change version is missing', () => { + const issues = missingChangeVersion({ + ...assertEntry, + changes: [...assertEntry.changes, { version: undefined }], + }); + + deepEqual(issues, [ + { + level: 'warn', + location: { + path: 'doc/api/assert.md', + position: { + end: { column: 35, line: 7, offset: 137 }, + start: { column: 1, line: 7, offset: 103 }, + }, + }, + message: 'Missing change version', + }, + ]); + }); +}); diff --git a/src/linter/tests/rules/missing-introduced-in.test.mjs b/src/linter/tests/rules/missing-introduced-in.test.mjs new file mode 100644 index 00000000..f6ca0fe4 --- /dev/null +++ b/src/linter/tests/rules/missing-introduced-in.test.mjs @@ -0,0 +1,38 @@ +import { describe, it } from 'node:test'; +import { missingIntroducedIn } from '../../rules/missing-introduced-in.mjs'; +import { deepEqual } from 'assert'; +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); + + 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 }, + }); + + deepEqual(issues, []); + }); + + it('should return an issue if the introduced_in property is missing', () => { + const issues = missingIntroducedIn({ + ...assertEntry, + introduced_in: undefined, + }); + + deepEqual(issues, [ + { + level: 'info', + location: { + path: 'doc/api/assert.md', + }, + message: "Missing 'introduced_in' field in the API doc entry", + }, + ]); + }); +}); diff --git a/src/linter/types.d.ts b/src/linter/types.d.ts new file mode 100644 index 00000000..719b1fd8 --- /dev/null +++ b/src/linter/types.d.ts @@ -0,0 +1,18 @@ +import { Position } from 'unist'; + +export type IssueLevel = 'info' | 'warn' | 'error'; + +export interface LintIssueLocation { + path: string; // The absolute path to the file + position?: Position; +} + +export interface LintIssue { + level: IssueLevel; + message: string; + location: LintIssueLocation; +} + +type LintRule = (input: ApiDocMetadataEntry) => LintIssue[]; + +export type Reporter = (msg: LintIssue) => void; diff --git a/src/metadata.mjs b/src/metadata.mjs index 579061f1..8c30a80f 100644 --- a/src/metadata.mjs +++ b/src/metadata.mjs @@ -45,6 +45,7 @@ const createMetadata = slugger => { const internalMetadata = { heading: createTree('root', { data: {} }), stability: createTree('root', []), + yaml_position: {}, properties: { type: undefined, changes: [], tags: [] }, }; @@ -92,6 +93,14 @@ const createMetadata = slugger => { internalMetadata.properties[key] = value; }); }, + /** + * Set the YAML position of the current Metadata entry + * + * @param {import('unist').Position} yaml_position + */ + setYamlPosition: yaml_position => { + internalMetadata.yaml_position = yaml_position; + }, /** * Generates a new Navigation entry and pushes them to the internal collection * of Navigation entries, and returns a MetadataEntry which is then used by the parser @@ -159,6 +168,7 @@ const createMetadata = slugger => { content: section, tags, introduced_in, + yaml_position: internalMetadata.yaml_position, }; }, }; diff --git a/src/parsers/markdown.mjs b/src/parsers/markdown.mjs index 65707e38..800a51b6 100644 --- a/src/parsers/markdown.mjs +++ b/src/parsers/markdown.mjs @@ -14,6 +14,8 @@ import { createNodeSlugger } from '../utils/slugger.mjs'; /** * Creates an API doc parser for a given Markdown API doc file + * + * @param {import('./linter/index.mjs').Linter | undefined} linter */ const createParser = () => { // Creates an instance of the Remark processor with GFM support @@ -134,9 +136,11 @@ const createParser = () => { // Visits all HTML nodes from the current subtree and if there's any that matches // our YAML metadata structure, it transforms into YAML metadata // and then apply the YAML Metadata to the current Metadata entry - visit(subTree, createQueries.UNIST.isYamlNode, node => - addYAMLMetadata(node, apiEntryMetadata) - ); + visit(subTree, createQueries.UNIST.isYamlNode, node => { + // TODO: Is there always only one YAML node? + apiEntryMetadata.setYamlPosition(node.position); + addYAMLMetadata(node, apiEntryMetadata); + }); // Visits all Text nodes from the current subtree and if there's any that matches // any API doc type reference and then updates the type reference to be a Markdown link diff --git a/src/test/metadata.test.mjs b/src/test/metadata.test.mjs index 96c20819..a36a99e9 100644 --- a/src/test/metadata.test.mjs +++ b/src/test/metadata.test.mjs @@ -79,6 +79,7 @@ describe('createMetadata', () => { stability: { type: 'root', children: [stability] }, tags: [], updates: [], + yaml_position: {}, }; const actual = metadata.create(apiDoc, section); delete actual.stability.toJSON; diff --git a/src/types.d.ts b/src/types.d.ts index ebae255e..e0aec952 100644 --- a/src/types.d.ts +++ b/src/types.d.ts @@ -1,7 +1,7 @@ import type { Heading, Root } from '@types/mdast'; import type { Program } from 'acorn'; import type { SemVer } from 'semver'; -import type { Data, Node, Parent } from 'unist'; +import type { Data, Node, Parent, Position } from 'unist'; // String serialization of the AST tree // @see https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/JSON/stringify#tojson_behavior @@ -99,6 +99,8 @@ declare global { // Extra YAML section entries that are stringd and serve // to provide additional metadata about the API doc entry tags: Array; + // The postion of the YAML of the API doc entry + yaml_position: Position; } export interface JsProgram extends Program {