diff --git a/bin/commands/generate.mjs b/bin/commands/generate.mjs index 6911b7df..92b68b68 100644 --- a/bin/commands/generate.mjs +++ b/bin/commands/generate.mjs @@ -1,6 +1,5 @@ import { cpus } from 'node:os'; import { resolve } from 'node:path'; -import process from 'node:process'; import { coerce } from 'semver'; @@ -12,7 +11,8 @@ import createGenerator from '../../src/generators.mjs'; import { publicGenerators } from '../../src/generators/index.mjs'; import createNodeReleases from '../../src/releases.mjs'; import { loadAndParse } from '../utils.mjs'; -import { runLint } from './lint.mjs'; +import createLinter from '../../src/linter/index.mjs'; +import { getEnabledRules } from '../../src/linter/utils/rules.mjs'; const availableGenerators = Object.keys(publicGenerators); @@ -123,9 +123,14 @@ export default { * @returns {Promise} */ async action(opts) { - const docs = await loadAndParse(opts.input, opts.ignore); + const rules = getEnabledRules(opts.disableRule); + const linter = opts.skipLint ? undefined : createLinter(rules); - if (!opts.skipLint && !runLint(docs)) { + const docs = await loadAndParse(opts.input, opts.ignore, linter); + + linter?.report(); + + if (linter?.hasError()) { console.error('Lint failed; aborting generation.'); process.exit(1); } diff --git a/bin/commands/lint.mjs b/bin/commands/lint.mjs index a0abafe2..5c0ae5f2 100644 --- a/bin/commands/lint.mjs +++ b/bin/commands/lint.mjs @@ -4,6 +4,7 @@ import createLinter from '../../src/linter/index.mjs'; import reporters from '../../src/linter/reporters/index.mjs'; import rules from '../../src/linter/rules/index.mjs'; import { loadAndParse } from '../utils.mjs'; +import { getEnabledRules } from '../../src/linter/utils/rules.mjs'; const availableRules = Object.keys(rules); const availableReporters = Object.keys(reporters); @@ -17,22 +18,6 @@ const availableReporters = Object.keys(reporters); * @property {keyof reporters} reporter - Reporter for linter output. */ -/** - * Run the linter on parsed documentation. - * @param {ApiDocMetadataEntry[]} docs - Parsed documentation objects. - * @param {LinterOptions} options - Linter configuration options. - * @returns {boolean} - True if no errors, false otherwise. - */ -export function runLint( - docs, - { disableRule = [], dryRun = false, reporter = 'console' } = {} -) { - const linter = createLinter(dryRun, disableRule); - linter.lintAll(docs); - linter.report(reporter); - return !linter.hasError(); -} - /** * @type {import('../utils.mjs').Command} */ @@ -95,9 +80,14 @@ export default { */ async action(opts) { try { - const docs = await loadAndParse(opts.input, opts.ignore); - const success = runLint(docs, opts); - process.exitCode = success ? 0 : 1; + const rules = getEnabledRules(opts.disableRule); + const linter = createLinter(rules, opts.dryRun); + + await loadAndParse(opts.input, opts.ignore, linter); + + linter.report(); + + process.exitCode = +linter.hasError(); } catch (error) { console.error('Error running the linter:', error); process.exitCode = 1; diff --git a/bin/utils.mjs b/bin/utils.mjs index b32a0929..13eec8dc 100644 --- a/bin/utils.mjs +++ b/bin/utils.mjs @@ -9,7 +9,7 @@ import createMarkdownParser from '../src/parsers/markdown.mjs'; */ export const lazy = factory => { let instance; - return () => (instance ??= factory()); + return args => (instance ??= factory(args)); }; // Instantiate loader and parser once to reuse, @@ -23,11 +23,12 @@ const parser = lazy(createMarkdownParser); * Load and parse markdown API docs. * @param {string[]} input - Glob patterns for input files. * @param {string[]} [ignore] - Glob patterns to ignore. + * @param {import('../src/linter/types').Linter} [linter] - Linter instance * @returns {Promise} - Parsed documentation objects. */ -export async function loadAndParse(input, ignore) { +export async function loadAndParse(input, ignore, linter) { const files = await loader().loadFiles(input, ignore); - return parser().parseApiDocs(files); + return parser(linter).parseApiDocs(files); } /** diff --git a/package-lock.json b/package-lock.json index 5aee468d..5d3cebd7 100644 --- a/package-lock.json +++ b/package-lock.json @@ -28,7 +28,9 @@ "shiki": "^3.2.1", "unified": "^11.0.5", "unist-builder": "^4.0.0", + "unist-util-find": "^3.0.0", "unist-util-find-after": "^5.0.0", + "unist-util-find-before": "^4.0.1", "unist-util-position": "^5.0.0", "unist-util-remove": "^4.0.0", "unist-util-select": "^5.1.0", @@ -2073,6 +2075,12 @@ "url": "https://github.com/sponsors/sindresorhus" } }, + "node_modules/lodash.iteratee": { + "version": "4.7.0", + "resolved": "https://registry.npmjs.org/lodash.iteratee/-/lodash.iteratee-4.7.0.tgz", + "integrity": "sha512-yv3cSQZmfpbIKo4Yo45B1taEvxjNvcpF1CEOc0Y6dEyvhPIfEJE3twDwPgWTPQubcSgXyBwBKG6wpQvWMDOf6Q==", + "license": "MIT" + }, "node_modules/lodash.merge": { "version": "4.6.2", "resolved": "https://registry.npmjs.org/lodash.merge/-/lodash.merge-4.6.2.tgz", @@ -4021,6 +4029,21 @@ "url": "https://opencollective.com/unified" } }, + "node_modules/unist-util-find": { + "version": "3.0.0", + "resolved": "https://registry.npmjs.org/unist-util-find/-/unist-util-find-3.0.0.tgz", + "integrity": "sha512-T7ZqS7immLjYyC4FCp2hDo3ksZ1v+qcbb+e5+iWxc2jONgHOLXPCpms1L8VV4hVxCXgWTxmBHDztuEZFVwC+Gg==", + "license": "MIT", + "dependencies": { + "@types/unist": "^3.0.0", + "lodash.iteratee": "^4.0.0", + "unist-util-visit": "^5.0.0" + }, + "funding": { + "type": "opencollective", + "url": "https://opencollective.com/unified" + } + }, "node_modules/unist-util-find-after": { "version": "5.0.0", "resolved": "https://registry.npmjs.org/unist-util-find-after/-/unist-util-find-after-5.0.0.tgz", @@ -4035,6 +4058,20 @@ "url": "https://opencollective.com/unified" } }, + "node_modules/unist-util-find-before": { + "version": "4.0.1", + "resolved": "https://registry.npmjs.org/unist-util-find-before/-/unist-util-find-before-4.0.1.tgz", + "integrity": "sha512-hfpCNqPbCOseOjHU8oBkRXM8gDQ++Ua3dN7sDYz7VJ+1alt+yd/I+ECZDhv1aqpJ1x47AHbzP/xA0jWf4omAIw==", + "license": "MIT", + "dependencies": { + "@types/unist": "^3.0.0", + "unist-util-is": "^6.0.0" + }, + "funding": { + "type": "opencollective", + "url": "https://opencollective.com/unified" + } + }, "node_modules/unist-util-is": { "version": "6.0.0", "resolved": "https://registry.npmjs.org/unist-util-is/-/unist-util-is-6.0.0.tgz", diff --git a/package.json b/package.json index 734ac400..befe5f90 100644 --- a/package.json +++ b/package.json @@ -56,7 +56,9 @@ "shiki": "^3.2.1", "unified": "^11.0.5", "unist-builder": "^4.0.0", + "unist-util-find": "^3.0.0", "unist-util-find-after": "^5.0.0", + "unist-util-find-before": "^4.0.1", "unist-util-position": "^5.0.0", "unist-util-remove": "^4.0.0", "unist-util-select": "^5.1.0", diff --git a/src/generators/legacy-json/utils/buildSection.mjs b/src/generators/legacy-json/utils/buildSection.mjs index 51dea85d..e73f7c1a 100644 --- a/src/generators/legacy-json/utils/buildSection.mjs +++ b/src/generators/legacy-json/utils/buildSection.mjs @@ -3,14 +3,7 @@ import { getRemarkRehype } from '../../../utils/remark.mjs'; import { transformNodesToString } from '../../../utils/unist.mjs'; import { parseList } from './parseList.mjs'; import { SECTION_TYPE_PLURALS, UNPROMOTED_KEYS } from '../constants.mjs'; - -/** - * Converts a value to an array. - * @template T - * @param {T | T[]} val - The value to convert. - * @returns {T[]} The value as an array. - */ -const enforceArray = val => (Array.isArray(val) ? val : [val]); +import { enforceArray } from '../../../utils/array.mjs'; /** * diff --git a/src/linter/constants.mjs b/src/linter/constants.mjs index 85f2589a..934b32d2 100644 --- a/src/linter/constants.mjs +++ b/src/linter/constants.mjs @@ -1,5 +1,9 @@ 'use strict'; +export const INTRDOCUED_IN_REGEX = //; + +export const LLM_DESCRIPTION_REGEX = //; + export const LINT_MESSAGES = { missingIntroducedIn: "Missing 'introduced_in' field in the API doc entry", missingChangeVersion: 'Missing version field in the API doc entry', diff --git a/src/linter/context.mjs b/src/linter/context.mjs new file mode 100644 index 00000000..00a07d8d --- /dev/null +++ b/src/linter/context.mjs @@ -0,0 +1,56 @@ +'use strict'; + +/** + * Creates a linting context for a given file and AST tree. + * + * @param {import('vfile').VFile} file + * @param {import('mdast').Root} tree + * @returns {import('./types').LintContext} + */ +const createContext = (file, tree) => { + /** + * Lint issues reported during validation. + * + * @type {import('./types').LintIssue[]} + */ + const issues = []; + + /** + * Reports a lint issue. + * + * @param {import('./types').IssueDescriptor} descriptor + * @returns {void} + */ + const report = ({ level, message, position }) => { + /** + * @type {import('./types').LintIssueLocation} + */ + const location = { + path: file.path, + position, + }; + + issues.push({ + level, + message, + location, + }); + }; + + /** + * Gets all reported issues. + * + * @returns {import('./types').LintIssue[]} + */ + const getIssues = () => { + return issues; + }; + + return { + tree, + report, + getIssues, + }; +}; + +export default createContext; diff --git a/src/linter/engine.mjs b/src/linter/engine.mjs deleted file mode 100644 index 9ae164c7..00000000 --- a/src/linter/engine.mjs +++ /dev/null @@ -1,30 +0,0 @@ -'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 an array of ApiDocMetadataEntry entries against all defined rules - * - * @param {ApiDocMetadataEntry[]} entries - * @returns {import('./types').LintIssue[]} - */ - const lintAll = entries => { - const issues = []; - - for (const rule of rules) { - issues.push(...rule(entries)); - } - - return issues; - }; - - return { - lintAll, - }; -}; - -export default createLinterEngine; diff --git a/src/linter/index.mjs b/src/linter/index.mjs index 451d6ee3..1f94aa0b 100644 --- a/src/linter/index.mjs +++ b/src/linter/index.mjs @@ -1,52 +1,48 @@ 'use strict'; -import createLinterEngine from './engine.mjs'; +import createContext from './context.mjs'; import reporters from './reporters/index.mjs'; -import rules from './rules/index.mjs'; /** - * Creates a linter instance to validate ApiDocMetadataEntry entries + * Creates a linter instance to validate API documentation ASTs against a + * defined set of rules. * - * @param {boolean} dryRun Whether to run the engine in dry-run mode - * @param {string[]} disabledRules List of disabled rules names + * @param {import('./types').LintRule[]} rules - Lint rules to apply + * @param {boolean} [dryRun] - If true, the linter runs without reporting + * @returns {import('./types').Linter} */ -const createLinter = (dryRun, disabledRules) => { +const createLinter = (rules, dryRun = false) => { /** - * Retrieves all enabled rules - * - * @returns {import('./types').LintRule[]} - */ - const getEnabledRules = () => { - return Object.entries(rules) - .filter(([ruleName]) => !disabledRules.includes(ruleName)) - .map(([, rule]) => rule); - }; - - const engine = createLinterEngine(getEnabledRules(disabledRules)); - - /** - * Lint issues found during validations + * Lint issues collected during validations. * * @type {Array} */ const issues = []; /** - * Lints all entries using the linter engine + * Lints a API doc and collects issues. * - * @param entries + * @param {import('vfile').VFile} file + * @param {import('mdast').Root} tree + * @returns {void} */ - const lintAll = entries => { - issues.push(...engine.lintAll(entries)); + const lint = (file, tree) => { + const context = createContext(file, tree); + + for (const rule of rules) { + rule(context); + } + + issues.push(...context.getIssues()); }; /** - * Reports found issues using the specified reporter + * Reports collected issues using the specified reporter. * - * @param {keyof typeof reporters} reporterName Reporter name + * @param {keyof typeof reporters} [reporterName] Reporter name * @returns {void} */ - const report = reporterName => { + const report = (reporterName = 'console') => { if (dryRun) { return; } @@ -59,7 +55,7 @@ const createLinter = (dryRun, disabledRules) => { }; /** - * Checks if any error-level issues were found during linting + * Checks if any error-level issues were collected. * * @returns {boolean} */ @@ -68,7 +64,8 @@ const createLinter = (dryRun, disabledRules) => { }; return { - lintAll, + issues, + lint, report, hasError, }; diff --git a/src/linter/rules/duplicate-stability-nodes.mjs b/src/linter/rules/duplicate-stability-nodes.mjs index 0f7a164a..eea43613 100644 --- a/src/linter/rules/duplicate-stability-nodes.mjs +++ b/src/linter/rules/duplicate-stability-nodes.mjs @@ -1,38 +1,61 @@ +'use strict'; + +import createQueries from '../../utils/queries/index.mjs'; import { LINT_MESSAGES } from '../constants.mjs'; +import { visit } from 'unist-util-visit'; /** * Checks if there are multiple stability nodes within a chain. * - * @param {ApiDocMetadataEntry[]} entries - * @returns {Array} + * @param {import('../types.d.ts').LintContext} context + * @returns {void} */ -export const duplicateStabilityNodes = entries => { - const issues = []; +export const duplicateStabilityNodes = context => { let currentDepth = 0; let currentStability = -1; + let currentHeaderDepth = 0; - 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; + visit(context.tree, node => { + // Track the current header depth + if (node.type === 'heading') { + currentHeaderDepth = node.depth; } - } - return issues; + // Process blockquotes to find stability nodes + if (node.type === 'blockquote') { + if (node.children && node.children.length > 0) { + const paragraph = node.children[0]; + if ( + paragraph.type === 'paragraph' && + paragraph.children && + paragraph.children.length > 0 + ) { + const text = paragraph.children[0]; + if (text.type === 'text') { + const match = text.value.match( + createQueries.QUERIES.stabilityIndex + ); + if (match) { + const stability = parseFloat(match[1]); + + if ( + currentHeaderDepth > currentDepth && + stability >= 0 && + stability === currentStability + ) { + context.report({ + level: 'warn', + message: LINT_MESSAGES.duplicateStabilityNode, + position: node.position, + }); + } else { + currentDepth = currentHeaderDepth; + currentStability = stability; + } + } + } + } + } + } + }); }; diff --git a/src/linter/rules/invalid-change-version.mjs b/src/linter/rules/invalid-change-version.mjs index 0e69a5c0..edd28584 100644 --- a/src/linter/rules/invalid-change-version.mjs +++ b/src/linter/rules/invalid-change-version.mjs @@ -1,6 +1,15 @@ -import { LINT_MESSAGES } from '../constants.mjs'; import { valid, parse } from 'semver'; import { env } from 'node:process'; +import { visit } from 'unist-util-visit'; +import yaml from 'yaml'; + +import createQueries from '../../utils/queries/index.mjs'; +import { + extractYamlContent, + normalizeYamlSyntax, +} from '../../utils/parser/index.mjs'; +import { LINT_MESSAGES } from '../constants.mjs'; +import { enforceArray } from '../../utils/array.mjs'; const NODE_RELEASED_VERSIONS = env.NODE_RELEASED_VERSIONS?.split(','); @@ -50,20 +59,40 @@ const isInvalid = NODE_RELEASED_VERSIONS /** * Identifies invalid change versions from metadata entries. * - * @param {ApiDocMetadataEntry[]} entries - Metadata entries to check. - * @returns {import('../types').LintIssue[]} List of detected lint issues. + * @param {import('../types.d.ts').LintContext} context + * @returns {void} */ -export const invalidChangeVersion = entries => - entries.flatMap(({ changes, api_doc_source, yaml_position }) => - changes.flatMap(({ version }) => - (Array.isArray(version) ? version : [version]) +export const invalidChangeVersion = context => { + visit(context.tree, createQueries.UNIST.isYamlNode, node => { + const yamlContent = extractYamlContent(node); + + const normalizedYaml = normalizeYamlSyntax(yamlContent); + + // TODO: Use YAML AST to provide better issues positions + /** + * @type {ApiDocRawMetadataEntry} + */ + const { changes } = yaml.parse(normalizedYaml); + + if (!changes) { + return; + } + + changes.forEach(({ version }) => + enforceArray(version) .filter(isInvalid) - .map(version => ({ - level: 'error', - message: version - ? LINT_MESSAGES.invalidChangeVersion.replace('{{version}}', version) - : LINT_MESSAGES.missingChangeVersion, - location: { path: api_doc_source, position: yaml_position }, - })) - ) - ); + .forEach(version => + context.report({ + level: 'error', + message: version + ? LINT_MESSAGES.invalidChangeVersion.replace( + '{{version}}', + version + ) + : LINT_MESSAGES.missingChangeVersion, + position: node.position, + }) + ) + ); + }); +}; diff --git a/src/linter/rules/missing-introduced-in.mjs b/src/linter/rules/missing-introduced-in.mjs index 785bc6b1..1353e39e 100644 --- a/src/linter/rules/missing-introduced-in.mjs +++ b/src/linter/rules/missing-introduced-in.mjs @@ -1,26 +1,26 @@ -import { LINT_MESSAGES } from '../constants.mjs'; +'use strict'; + +import { INTRDOCUED_IN_REGEX, LINT_MESSAGES } from '../constants.mjs'; +import { findTopLevelEntry } from '../utils/find.mjs'; /** - * Checks if `introduced_in` field is missing + * Checks if `introduced_in` field is missing in the top-level entry. * - * @param {ApiDocMetadataEntry[]} entries - * @returns {Array} + * @param {import('../types.d.ts').LintContext} context + * @returns {void} */ -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; +export const missingIntroducedIn = context => { + const introducedIn = findTopLevelEntry( + context.tree, + node => node.type === 'html' && INTRDOCUED_IN_REGEX.test(node.value) + ); - issues.push({ - level: 'info', - message: LINT_MESSAGES.missingIntroducedIn, - location: { - path: entry.api_doc_source, - }, - }); + if (introducedIn) { + return; } - return issues; + return context.report({ + level: 'info', + message: LINT_MESSAGES.missingIntroducedIn, + }); }; diff --git a/src/linter/rules/missing-llm-description.mjs b/src/linter/rules/missing-llm-description.mjs index 18d5b1d5..ec3bb1c4 100644 --- a/src/linter/rules/missing-llm-description.mjs +++ b/src/linter/rules/missing-llm-description.mjs @@ -1,47 +1,36 @@ -import { LINT_MESSAGES } from '../constants.mjs'; +import { LINT_MESSAGES, LLM_DESCRIPTION_REGEX } from '../constants.mjs'; +import { findTopLevelEntry } from '../utils/find.mjs'; /** * Checks if a top-level entry is missing a llm_description field or a paragraph * node. * - * @param {ApiDocMetadataEntry[]} entries - * @returns {Array} + * @param {import('../types.d.ts').LintContext} context + * @returns {void} */ -export const missingLlmDescription = entries => { - return entries - .filter(entry => { - // Only process top-level headings - if (entry.heading.depth !== 1) { - return false; - } +export const missingLlmDescription = context => { + const llmDescription = findTopLevelEntry( + context.tree, + node => node.type === 'html' && LLM_DESCRIPTION_REGEX.test(node.value) + ); - // Skip entries that have an llm_description property - if (entry.llm_description !== undefined) { - return false; - } + if (llmDescription) { + return; + } - const hasParagraph = entry.content.children.some( - node => node.type === 'paragraph' - ); + // Check if there is a paragraph node in the top-level entry that can be used + // as fallback for llm_description + const paragraph = findTopLevelEntry( + context.tree, + node => node.type === 'paragraph' + ); - // Skip entries that contain a paragraph that can be used as a fallback. - if (hasParagraph) { - return false; - } + if (paragraph) { + return; + } - return true; - }) - .map(entry => mapToMissingEntryWarning(entry)); + context.report({ + level: 'warn', + message: LINT_MESSAGES.missingLlmDescription, + }); }; - -/** - * Maps a entry to a warning for missing llm description. - * - * @param {ApiDocMetadataEntry} entry - * @returns {import('../types.d.ts').LintIssue} - */ -const mapToMissingEntryWarning = entry => ({ - level: 'warn', - message: LINT_MESSAGES.missingLlmDescription, - location: { path: entry.api_doc_source }, -}); diff --git a/src/linter/tests/engine.test.mjs b/src/linter/tests/engine.test.mjs deleted file mode 100644 index ac33921f..00000000 --- a/src/linter/tests/engine.test.mjs +++ /dev/null @@ -1,44 +0,0 @@ -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.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]]); - }); - - 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.lintAll([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.lintAll([assertEntry]); - - assert.deepEqual(issues, []); - }); -}); diff --git a/src/linter/tests/fixtures/descriptors.mjs b/src/linter/tests/fixtures/descriptors.mjs new file mode 100644 index 00000000..c67a3c75 --- /dev/null +++ b/src/linter/tests/fixtures/descriptors.mjs @@ -0,0 +1,26 @@ +/** + * @type {import('../../types').IssueDescriptor} + */ +export const infoDescriptor = { + level: 'info', + message: 'This is a INFO issue', + position: { start: { line: 1, column: 1 }, end: { line: 1, column: 2 } }, +}; + +/** + * @type {import('../../types').IssueDescriptor} + */ +export const warnDescriptor = { + level: 'warn', + message: 'This is a WARN issue', + position: { start: { line: 1, column: 1 }, end: { line: 1, column: 2 } }, +}; + +/** + * @type {import('../../types').IssueDescriptor} + */ +export const errorDescriptor = { + level: 'error', + message: 'This is a ERROR issue', + position: { start: { line: 1, column: 1 }, end: { line: 1, column: 2 } }, +}; diff --git a/src/linter/tests/fixtures/entries.mjs b/src/linter/tests/fixtures/entries.mjs deleted file mode 100644 index 1319c18e..00000000 --- a/src/linter/tests/fixtures/entries.mjs +++ /dev/null @@ -1,80 +0,0 @@ -/** - * @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')`.", - }, - { - version: 'REPLACEME', - 'pr-url': 'https://github.com/nodejs/node/pull/12345', - description: 'This is a test entry.', - }, - ], - 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', - }, - }, - stability: { - type: 'root', - children: [], - }, - 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/invalidChangeVersion-environment.mjs b/src/linter/tests/fixtures/invalidChangeVersion-environment.mjs index cbb4d0ca..fb7f70be 100644 --- a/src/linter/tests/fixtures/invalidChangeVersion-environment.mjs +++ b/src/linter/tests/fixtures/invalidChangeVersion-environment.mjs @@ -1,15 +1,38 @@ -import { invalidChangeVersion } from '../../rules/invalid-change-version.mjs'; import { deepEqual } from 'node:assert'; -import { assertEntry } from './entries.mjs'; - -const issues = invalidChangeVersion([ - { - ...assertEntry, - changes: [ - ...assertEntry.changes, - { version: ['SOME_OTHER_RELEASED_VERSION', 'v0.1.2'] }, +import { mock } from 'node:test'; + +import dedent from 'dedent'; + +import { invalidChangeVersion } from '../../rules/invalid-change-version.mjs'; + +const yamlContent = dedent` +`; + +const context = { + tree: { + type: 'root', + children: [ + { + type: 'html', + value: yamlContent, + position: { + start: { line: 1, column: 1, offset: 1 }, + end: { line: 1, column: 1, offset: 1 }, + }, + }, ], }, -]); + report: mock.fn(), + getIssues: mock.fn(), +}; + +invalidChangeVersion(context); -deepEqual(issues, []); +deepEqual(context.report.mock.callCount(), 0); diff --git a/src/linter/tests/fixtures/issues.mjs b/src/linter/tests/fixtures/issues.mjs index 1546e8bc..d3c6f94d 100644 --- a/src/linter/tests/fixtures/issues.mjs +++ b/src/linter/tests/fixtures/issues.mjs @@ -1,5 +1,3 @@ -// @ts-check - /** * @type {import('../../types').LintIssue} */ @@ -7,6 +5,7 @@ export const infoIssue = { level: 'info', location: { path: 'doc/api/test.md', + position: { start: { line: 1, column: 1 }, end: { line: 1, column: 2 } }, }, message: 'This is a INFO issue', }; diff --git a/src/linter/tests/fixtures/tree.mjs b/src/linter/tests/fixtures/tree.mjs new file mode 100644 index 00000000..d1fce2ca --- /dev/null +++ b/src/linter/tests/fixtures/tree.mjs @@ -0,0 +1,4 @@ +export const emptyTree = { + type: 'root', + children: [], +}; diff --git a/src/linter/tests/index.test.mjs b/src/linter/tests/index.test.mjs new file mode 100644 index 00000000..de4c9768 --- /dev/null +++ b/src/linter/tests/index.test.mjs @@ -0,0 +1,67 @@ +import { describe, mock, it } from 'node:test'; +import assert from 'node:assert/strict'; +import { VFile } from 'vfile'; + +import createLinter from '../index.mjs'; +import { errorIssue, infoIssue, warnIssue } from './fixtures/issues.mjs'; +import createContext from '../context.mjs'; +import { + errorDescriptor, + infoDescriptor, + warnDescriptor, +} from './fixtures/descriptors.mjs'; +import { emptyTree } from './fixtures/tree.mjs'; + +describe('createLinter', () => { + it('should call each rule with context', t => { + const context = { + tree: emptyTree, + report: () => {}, + getIssues: () => [], + }; + + t.mock.fn(createContext).mock.mockImplementation(() => context); + + const rule1 = mock.fn(() => []); + const rule2 = mock.fn(() => []); + + const linter = createLinter([rule1, rule2]); + + linter.lint(new VFile({ path: 'doc/api/test.md' }), emptyTree); + + assert.strictEqual(rule1.mock.callCount(), 1); + assert.strictEqual(rule2.mock.callCount(), 1); + + const rule1Context = rule1.mock.calls[0].arguments[0]; + const rule2Context = rule2.mock.calls[0].arguments[0]; + + assert.strictEqual(rule1Context.tree, emptyTree); + assert.strictEqual(rule2Context.tree, emptyTree); + }); + + it('should return the aggregated issues from all rules', () => { + const rule1 = mock.fn(ctx => { + ctx.report(infoDescriptor); + ctx.report(warnDescriptor); + }); + + const rule2 = mock.fn(ctx => ctx.report(errorDescriptor)); + + const linter = createLinter([rule1, rule2]); + + linter.lint(new VFile({ path: 'doc/api/test.md' }), emptyTree); + + assert.equal(linter.issues.length, 3); + assert.deepEqual(linter.issues, [infoIssue, warnIssue, errorIssue]); + }); + + it('should return an empty array when no issues are found', () => { + const rule = () => []; + + const linter = createLinter([rule]); + + linter.lint(new VFile({ path: 'doc/api/test.md' }), emptyTree); + + assert.deepEqual(linter.issues, []); + }); +}); diff --git a/src/linter/tests/reporters/console.test.mjs b/src/linter/tests/reporters/console.test.mjs index a14d4172..faa04235 100644 --- a/src/linter/tests/reporters/console.test.mjs +++ b/src/linter/tests/reporters/console.test.mjs @@ -7,7 +7,7 @@ const testCases = [ { issue: infoIssue, method: 'info', - expected: '\x1B[90mThis is a INFO issue at doc/api/test.md\x1B[39m', + expected: '\x1B[90mThis is a INFO issue at doc/api/test.md (1:1)\x1B[39m', }, { issue: warnIssue, diff --git a/src/linter/tests/reporters/github.test.mjs b/src/linter/tests/reporters/github.test.mjs index 94c92712..5de47858 100644 --- a/src/linter/tests/reporters/github.test.mjs +++ b/src/linter/tests/reporters/github.test.mjs @@ -18,7 +18,7 @@ describe('github', () => { ); assert.deepStrictEqual(callsArgs, [ - '::notice file=doc/api/test.md::This is a INFO issue', + '::notice file=doc/api/test.md,line=1,endLine=1::This is a INFO issue', '::warning file=doc/api/test.md,line=1,endLine=1::This is a WARN issue', '::error file=doc/api/test.md,line=1,endLine=1::This is a ERROR issue', ]); diff --git a/src/linter/tests/rules/duplicate-stability-nodes.test.mjs b/src/linter/tests/rules/duplicate-stability-nodes.test.mjs index 7c3c700d..dc7ebe90 100644 --- a/src/linter/tests/rules/duplicate-stability-nodes.test.mjs +++ b/src/linter/tests/rules/duplicate-stability-nodes.test.mjs @@ -1,156 +1,233 @@ -import { describe, it } from 'node:test'; -import { deepStrictEqual } from 'assert'; +import { describe, it, mock } from 'node:test'; +import { deepStrictEqual, strictEqual } from 'node:assert'; import { duplicateStabilityNodes } from '../../rules/duplicate-stability-nodes.mjs'; import { LINT_MESSAGES } from '../../constants.mjs'; // Mock data structure for creating test entries -const createEntry = ( +const createStabilityNode = (value, line = 1, column = 1) => ({ + type: 'blockquote', + children: [ + { + type: 'paragraph', + children: [ + { + type: 'text', + value: `Stability: ${value}`, + }, + ], + }, + ], + position: { + start: { line, column }, + end: { line, column: column + 20 }, + }, +}); + +const createHeadingNode = (depth, line = 1, column = 1) => ({ + type: 'heading', depth, - stabilityIndex, - source = 'file.yaml', - position = { line: 1, column: 1 } -) => ({ - heading: { data: { depth } }, - stability: { - children: [{ data: { index: stabilityIndex }, children: [{ position }] }], + children: [ + { + type: 'text', + value: `Heading ${depth}`, + }, + ], + position: { + start: { line, column }, + end: { line, column: column + 10 }, + }, +}); + +const createContext = (nodes, path = 'file.md') => ({ + tree: { + type: 'root', + children: nodes, }, - api_doc_source: source, + path, + report: mock.fn(), }); describe('duplicateStabilityNodes', () => { - it('returns empty array when there are no entries', () => { - deepStrictEqual(duplicateStabilityNodes([]), []); + it('should not report when there are no stability nodes', () => { + const context = createContext([ + createHeadingNode(1, 1), + createHeadingNode(2, 2), + ]); + duplicateStabilityNodes(context); + strictEqual(context.report.mock.callCount(), 0); }); - 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('should not report when there are no duplicate stability nodes', () => { + const context = createContext([ + createHeadingNode(1, 1), + createStabilityNode(0, 2), + createHeadingNode(2, 3), + createStabilityNode(1, 4), + createHeadingNode(3, 5), + createStabilityNode(2, 6), + ]); + duplicateStabilityNodes(context); + strictEqual(context.report.mock.callCount(), 0); }); it('detects duplicate stability nodes within a chain', () => { - const entries = [ - createEntry(1, 0), - createEntry(2, 0), // Duplicate stability node - ]; + const duplicateNode = createStabilityNode(0, 4); + const context = createContext([ + createHeadingNode(1, 1), + createStabilityNode(0, 2), + createHeadingNode(2, 3), + duplicateNode, // Duplicate stability node + ]); + + duplicateStabilityNodes(context); + + strictEqual(context.report.mock.callCount(), 1); + + const call = context.report.mock.calls[0]; - deepStrictEqual(duplicateStabilityNodes(entries), [ + deepStrictEqual(call.arguments, [ { level: 'warn', message: LINT_MESSAGES.duplicateStabilityNode, - location: { - path: 'file.yaml', - position: { line: 1, column: 1 }, - }, + position: duplicateNode.position, }, ]); }); 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), [ + const duplicateNode1 = createStabilityNode(0, 4); + const duplicateNode2 = createStabilityNode(1, 8); + const context = createContext([ + createHeadingNode(1, 1), + createStabilityNode(0, 2), + createHeadingNode(2, 3), + duplicateNode1, // This should trigger an issue + createHeadingNode(1, 5), + createStabilityNode(1, 6), + createHeadingNode(2, 7), + duplicateNode2, // This should trigger another issue + ]); + + duplicateStabilityNodes(context); + + strictEqual(context.report.mock.callCount(), 2); + + const calls = context.report.mock.calls.flatMap(call => call.arguments); + + deepStrictEqual(calls, [ { level: 'warn', message: LINT_MESSAGES.duplicateStabilityNode, - location: { - path: 'file.yaml', - position: { line: 1, column: 1 }, - }, + position: duplicateNode1.position, }, { level: 'warn', message: LINT_MESSAGES.duplicateStabilityNode, - location: { - path: 'file.yaml', - position: { line: 1, column: 1 }, - }, + position: duplicateNode2.position, }, ]); }); 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), [ + const duplicateNode = createStabilityNode(0, 6); + const context = createContext([ + createHeadingNode(1, 1), { - level: 'warn', - message: LINT_MESSAGES.duplicateStabilityNode, - location: { - path: 'file.yaml', - position: { line: 1, column: 1 }, - }, + type: 'blockquote', + children: [ + { + type: 'paragraph', + children: [{ type: 'text', value: 'Not a stability node' }], + }, + ], + position: { start: { line: 2 }, end: { line: 2 } }, }, + createHeadingNode(3, 3), + createStabilityNode(0, 4), + createHeadingNode(4, 5), + duplicateNode, // This should trigger an issue ]); - }); - 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), []); - }); + duplicateStabilityNodes(context); - 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), [ + strictEqual(context.report.mock.callCount(), 1); + + const call = context.report.mock.calls[0]; + + deepStrictEqual(call.arguments, [ { level: 'warn', message: LINT_MESSAGES.duplicateStabilityNode, - location: { - path: 'file.yaml', - position: { line: 1, column: 1 }, - }, + position: duplicateNode.position, }, ]); }); 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), [ + const duplicateNode1 = createStabilityNode(1, 6); + const duplicateNode2 = createStabilityNode(2, 10); + const context = createContext([ + createHeadingNode(1, 1), + createStabilityNode(0, 2), + createHeadingNode(2, 3), + createStabilityNode(1, 4), + createHeadingNode(3, 5), + duplicateNode1, // This should trigger an issue + createHeadingNode(2, 7), + createStabilityNode(2, 8), + createHeadingNode(3, 9), + duplicateNode2, // This should trigger another issue + ]); + + duplicateStabilityNodes(context); + + strictEqual(context.report.mock.callCount(), 2); + + const calls = context.report.mock.calls.flatMap(call => call.arguments); + + deepStrictEqual(calls, [ { level: 'warn', message: LINT_MESSAGES.duplicateStabilityNode, - location: { - path: 'file.yaml', - position: { line: 1, column: 1 }, - }, + position: duplicateNode1.position, }, { level: 'warn', message: LINT_MESSAGES.duplicateStabilityNode, - location: { - path: 'file.yaml', - position: { line: 1, column: 1 }, - }, + position: duplicateNode2.position, + }, + ]); + }); + + it('handles malformed blockquotes gracefully', () => { + const context = createContext([ + createHeadingNode(1, 1), + { + type: 'blockquote', + children: [], // Empty children array + position: { start: { line: 2 }, end: { line: 2 } }, + }, + createHeadingNode(2, 3), + { + type: 'blockquote', + children: [{ type: 'thematicBreak' }], // No paragraph + position: { start: { line: 4 }, end: { line: 4 } }, + }, + createHeadingNode(3, 5), + { + type: 'blockquote', + children: [ + { + type: 'paragraph', + children: [], // No text node + }, + ], + position: { start: { line: 6 }, end: { line: 6 } }, }, ]); + + duplicateStabilityNodes(context); + + strictEqual(context.report.mock.callCount(), 0); }); }); diff --git a/src/linter/tests/rules/invalid-change-version.test.mjs b/src/linter/tests/rules/invalid-change-version.test.mjs index 2b7fe370..c51ad99e 100644 --- a/src/linter/tests/rules/invalid-change-version.test.mjs +++ b/src/linter/tests/rules/invalid-change-version.test.mjs @@ -1,37 +1,85 @@ -import { describe, it } from 'node:test'; +import { describe, it, mock } from 'node:test'; import { invalidChangeVersion } from '../../rules/invalid-change-version.mjs'; -import { deepEqual, strictEqual } from 'node:assert'; -import { assertEntry } from '../fixtures/entries.mjs'; +import { deepStrictEqual, strictEqual } from 'node:assert'; +import dedent from 'dedent'; import { spawnSync } from 'node:child_process'; -import { fileURLToPath } from 'node:url'; import { execPath } from 'node:process'; +import { fileURLToPath } from 'node:url'; describe('invalidChangeVersion', () => { - it('should return an empty array if all change versions are non-empty', () => { - const issues = invalidChangeVersion([assertEntry]); + it('should not report if all change versions are non-empty', () => { + const yamlContent = dedent` +`; + + const context = { + tree: { + type: 'root', + children: [ + { + type: 'html', + value: yamlContent, + }, + ], + }, + report: mock.fn(), + getIssues: mock.fn(), + }; + + invalidChangeVersion(context); - deepEqual(issues, []); + strictEqual(context.report.mock.callCount(), 0); }); - it('should return an issue if a change version is missing', () => { - const issues = invalidChangeVersion([ - { - ...assertEntry, - changes: [...assertEntry.changes, { version: undefined }], + it('should report an issue if a change version is missing', () => { + const yamlContent = dedent` +`; + + const context = { + tree: { + type: 'root', + children: [ + { + type: 'html', + value: yamlContent, + position: { + start: { line: 1, column: 1, offset: 1 }, + end: { line: 1, column: 1, offset: 1 }, + }, + }, + ], }, - ]); + report: mock.fn(), + getIssues: mock.fn(), + }; + + invalidChangeVersion(context); + + strictEqual(context.report.mock.callCount(), 1); + + const call = context.report.mock.calls[0]; - deepEqual(issues, [ + deepStrictEqual(call.arguments, [ { level: 'error', - location: { - path: 'doc/api/assert.md', - position: { - end: { column: 35, line: 7, offset: 137 }, - start: { column: 1, line: 7, offset: 103 }, - }, - }, message: 'Missing version field in the API doc entry', + position: { + start: { line: 1, column: 1, offset: 1 }, + end: { line: 1, column: 1, offset: 1 }, + }, }, ]); }); @@ -65,58 +113,120 @@ describe('invalidChangeVersion', () => { strictEqual(result.error, undefined); }); - it('should return an empty array if all change versions are valid', () => { - deepEqual(invalidChangeVersion([assertEntry]), []); + it('should not report if all change versions are valid', () => { + const yamlContent = dedent` +`; + + const context = { + tree: { + type: 'root', + children: [ + { + type: 'html', + value: yamlContent, + }, + ], + }, + report: mock.fn(), + getIssues: mock.fn(), + }; + + invalidChangeVersion(context); + + strictEqual(context.report.mock.callCount(), 0); }); - it('should return an issue if a change version is invalid', () => { - const issues = invalidChangeVersion([ - { - ...assertEntry, - changes: [ - ...assertEntry.changes, - { version: ['v13.9.0', 'INVALID_VERSION'] }, + it('should report an issue if a change version is invalid', () => { + const yamlContent = dedent` +`; + + const context = { + tree: { + type: 'root', + children: [ + { + type: 'html', + value: yamlContent, + position: { + start: { column: 1, line: 7, offset: 103 }, + end: { column: 35, line: 7, offset: 137 }, + }, + }, ], }, - ]); + report: mock.fn(), + getIssues: mock.fn(), + }; - deepEqual(issues, [ + invalidChangeVersion(context); + strictEqual(context.report.mock.callCount(), 1); + const call = context.report.mock.calls[0]; + deepStrictEqual(call.arguments, [ { level: 'error', - location: { - path: 'doc/api/assert.md', - position: { - start: { column: 1, line: 7, offset: 103 }, - end: { column: 35, line: 7, offset: 137 }, - }, - }, message: 'Invalid version number: INVALID_VERSION', + position: { + start: { column: 1, line: 7, offset: 103 }, + end: { column: 35, line: 7, offset: 137 }, + }, }, ]); }); - it('should return an issue if a change version contains a REPLACEME and a version', () => { - const issues = invalidChangeVersion([ - { - ...assertEntry, - changes: [ - ...assertEntry.changes, - { version: ['v24.0.0', 'REPLACEME'] }, + it('should report an issue if a change version contains a REPLACEME and a version', () => { + const yamlContent = dedent` +`; + + const context = { + tree: { + type: 'root', + children: [ + { + type: 'html', + value: yamlContent, + position: { + start: { column: 1, line: 7, offset: 103 }, + end: { column: 35, line: 7, offset: 137 }, + }, + }, ], }, - ]); + report: mock.fn(), + getIssues: mock.fn(), + }; - deepEqual(issues, [ + invalidChangeVersion(context); + strictEqual(context.report.mock.callCount(), 1); + const call = context.report.mock.calls[0]; + deepStrictEqual(call.arguments, [ { level: 'error', - location: { - path: 'doc/api/assert.md', - position: { - start: { column: 1, line: 7, offset: 103 }, - end: { column: 35, line: 7, offset: 137 }, - }, - }, message: 'Invalid version number: REPLACEME', + position: { + start: { column: 1, line: 7, offset: 103 }, + end: { column: 35, line: 7, offset: 137 }, + }, }, ]); }); diff --git a/src/linter/tests/rules/missing-introduced-in.test.mjs b/src/linter/tests/rules/missing-introduced-in.test.mjs index 4671a8b2..443d2553 100644 --- a/src/linter/tests/rules/missing-introduced-in.test.mjs +++ b/src/linter/tests/rules/missing-introduced-in.test.mjs @@ -1,40 +1,81 @@ -import { describe, it } from 'node:test'; +import { deepStrictEqual, strictEqual } from 'node:assert'; +import { describe, it, mock } 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]); + it('should not report if the introduced_in field is not missing', () => { + const context = { + tree: { + type: 'root', + children: [ + { + type: 'html', + value: '', + }, + ], + }, + report: mock.fn(), + getIssues: mock.fn(), + }; - deepEqual(issues, []); + missingIntroducedIn(context); + + strictEqual(context.report.mock.callCount(), 0); }); - it('should return an empty array if the heading depth is not equal to 1', () => { - const issues = missingIntroducedIn([ - { - ...assertEntry, - heading: { ...assertEntry.heading, depth: 2 }, + it('should report an issue if the introduced_in field is missing in the first entry', () => { + const context = { + tree: { + type: 'root', + children: [ + { + type: 'heading', + depth: 2, + }, + { + type: 'html', + value: '', + }, + ], }, - ]); + report: mock.fn(), + getIssues: mock.fn(), + }; - deepEqual(issues, []); - }); + missingIntroducedIn(context); + + strictEqual(context.report.mock.callCount(), 1); + + const call = context.report.mock.calls[0]; - it('should return an issue if the introduced_in property is missing', () => { - const issues = missingIntroducedIn([ + deepStrictEqual(call.arguments, [ { - ...assertEntry, - introduced_in: undefined, + level: 'info', + message: "Missing 'introduced_in' field in the API doc entry", }, ]); + }); + + it('should report an issue if the introduced_in property is missing', () => { + const context = { + tree: { + type: 'root', + children: [], + }, + report: mock.fn(), + getIssues: mock.fn(), + }; + + missingIntroducedIn(context); + + strictEqual(context.report.mock.callCount(), 1); + + const call = context.report.mock.calls[0]; - deepEqual(issues, [ + deepStrictEqual(call.arguments, [ { 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 index cd236513..2c328e17 100644 --- a/src/linter/types.d.ts +++ b/src/linter/types.d.ts @@ -1,4 +1,7 @@ +import { Root } from 'mdast'; import { Position } from 'unist'; +import reporters from './reporters/index.mjs'; +import { VFile } from 'vfile'; export type IssueLevel = 'info' | 'warn' | 'error'; @@ -13,6 +16,25 @@ export interface LintIssue { location: LintIssueLocation; } -type LintRule = (input: ApiDocMetadataEntry[]) => LintIssue[]; +type LintRule = (context: LintContext) => void; -export type Reporter = (msg: LintIssue) => void; +export type Reporter = (message: LintIssue) => void; + +export interface Linter { + readonly issues: LintIssue[]; + lint: (file: VFile, tree: Root) => void; + report: (reporterName: keyof typeof reporters) => void; + hasError: () => boolean; +} + +export interface IssueDescriptor { + level: IssueLevel; + message: string; + position?: Position; +} + +export interface LintContext { + readonly tree: Root; + report(descriptor: IssueDescriptor): void; + getIssues(): LintIssue[]; +} diff --git a/src/linter/utils/find.mjs b/src/linter/utils/find.mjs new file mode 100644 index 00000000..3da03e53 --- /dev/null +++ b/src/linter/utils/find.mjs @@ -0,0 +1,20 @@ +import { find } from 'unist-util-find'; +import { findBefore } from 'unist-util-find-before'; + +/** + * Finds the first node that matches the condition before the first h2 heading, + * this area is considered the top-level section of the tree + * + * @param {import('mdast').Node} node + * @param {import('unist-util-find').TestFn} condition + */ +export const findTopLevelEntry = (node, condition) => { + const h2 = find(node, { type: 'heading', depth: 2 }); + + // If there isn't h2, search the entire tree + if (!h2) { + return find(node, condition); + } + + return findBefore(node, h2, condition); +}; diff --git a/src/linter/utils/rules.mjs b/src/linter/utils/rules.mjs new file mode 100644 index 00000000..22479f75 --- /dev/null +++ b/src/linter/utils/rules.mjs @@ -0,0 +1,15 @@ +'use strict'; + +import rules from '../rules/index.mjs'; + +/** + * Gets all enabled rules + * + * @param {string[]} [disabledRules] - List of disabled rule names + * @returns {import('../types').LintRule[]} - List of enabled rules + */ +export const getEnabledRules = (disabledRules = []) => { + return Object.entries(rules) + .filter(([ruleName]) => !disabledRules.includes(ruleName)) + .map(([, rule]) => rule); +}; diff --git a/src/parsers/markdown.mjs b/src/parsers/markdown.mjs index 3617baca..e0e7f229 100644 --- a/src/parsers/markdown.mjs +++ b/src/parsers/markdown.mjs @@ -15,9 +15,9 @@ import { createNodeSlugger } from '../utils/slugger/index.mjs'; /** * Creates an API doc parser for a given Markdown API doc file * - * @param {import('./linter/index.mjs').Linter | undefined} linter + * @param {import('../linter/types').Linter} [linter] */ -const createParser = () => { +const createParser = linter => { // Creates an instance of the Remark processor with GFM support // which is used for stringifying the AST tree back to Markdown const remarkProcessor = getRemark(); @@ -63,6 +63,8 @@ const createParser = () => { // Parses the API doc into an AST tree using `unified` and `remark` const apiDocTree = remarkProcessor.parse(resolvedApiDoc); + linter?.lint(resolvedApiDoc, apiDocTree); + // Get all Markdown Footnote definitions from the tree const markdownDefinitions = selectAll('definition', apiDocTree); diff --git a/src/utils/array.mjs b/src/utils/array.mjs new file mode 100644 index 00000000..706c6df8 --- /dev/null +++ b/src/utils/array.mjs @@ -0,0 +1,7 @@ +/** + * Converts a value to an array. + * @template T + * @param {T | T[]} val - The value to convert. + * @returns {T[]} The value as an array. + */ +export const enforceArray = val => (Array.isArray(val) ? val : [val]); diff --git a/src/utils/parser/index.mjs b/src/utils/parser/index.mjs index dedfa4b3..5a6fc485 100644 --- a/src/utils/parser/index.mjs +++ b/src/utils/parser/index.mjs @@ -11,6 +11,38 @@ import { DOC_TYPES_MAPPING_OTHER, DOC_TYPES_MAPPING_PRIMITIVES, } from './constants.mjs'; +import createQueries from '../queries/index.mjs'; + +/** + * Extracts raw YAML content from a node + * + * @param {import('mdast').Html} node A HTML node containing the YAML content + * @returns {string} The extracted raw YAML content + */ +export const extractYamlContent = node => { + return node.value.replace( + createQueries.QUERIES.yamlInnerContent, + // Either capture a YAML multinline block, or a simple single-line YAML block + (_, simple, yaml) => simple || yaml + ); +}; + +/** + * Normalizes YAML syntax by fixing some non-cool formatted properties of the + * docs schema + * + * @param {string} yamlContent The raw YAML content to normalize + * @returns {string} The normalized YAML content + */ +export const normalizeYamlSyntax = yamlContent => { + return yamlContent + .replace('introduced_in=', 'introduced_in: ') + .replace('source_link=', 'source_link: ') + .replace('type=', 'type: ') + .replace('name=', 'name: ') + .replace('llm_description=', 'llm_description: ') + .replace(/^[\r\n]+|[\r\n]+$/g, ''); // Remove initial and final line breaks +}; /** * This method replaces plain text Types within the Markdown content into Markdown links @@ -90,18 +122,12 @@ export const transformTypeToReferenceLink = type => { * @returns {ApiDocRawMetadataEntry} The parsed YAML metadata */ export const parseYAMLIntoMetadata = yamlString => { - const replacedContent = yamlString - // special validations for some non-cool formatted properties of the docs schema - .replace('introduced_in=', 'introduced_in: ') - .replace('source_link=', 'source_link: ') - .replace('type=', 'type: ') - .replace('name=', 'name: ') - .replace('llm_description=', 'llm_description: '); + const normalizedYaml = normalizeYamlSyntax(yamlString); // Ensures that the parsed YAML is an object, because even if it is not // i.e. a plain string or an array, it will simply not result into anything /** @type {ApiDocRawMetadataEntry | string} */ - let parsedYaml = yaml.parse(replacedContent); + let parsedYaml = yaml.parse(normalizedYaml); // Ensure that only Objects get parsed on Object.keys(), since some `