Skip to content

feat(linter): introduce raw ast rules #239

New issue

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

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

Already on GitHub? Sign in to your account

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 1 addition & 4 deletions bin/cli.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ const {
const linter = createLinter(lintDryRun, disableRule);

const { loadFiles } = createMarkdownLoader();
const { parseApiDocs } = createMarkdownParser();
const { parseApiDocs } = createMarkdownParser(linter);

const apiDocFiles = await loadFiles(input, ignore);

Expand All @@ -124,9 +124,6 @@ const { runGenerators } = createGenerator(parsedApiDocs);
// Retrieves Node.js release metadata from a given Node.js version and CHANGELOG.md file
const { getAllMajors } = createNodeReleases(changelog);

// Runs the Linter on the parsed API docs
linter.lintAll(parsedApiDocs);

if (target) {
await runGenerators({
// A list of target modes for the API docs parser
Expand Down
13 changes: 7 additions & 6 deletions src/linter/engine.mjs
Original file line number Diff line number Diff line change
@@ -1,29 +1,30 @@
'use strict';

/**
* Creates a linter engine instance to validate ApiDocMetadataEntry entries
* Creates a linter engine instance to validate mdast trees.
*
* @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
* Validates an array of mdast trees against all defined rules
*
* @param {ApiDocMetadataEntry[]} entries
* @param {import('vfile').VFile} file
* @param {import('mdast').Root[]} tree
* @returns {import('./types').LintIssue[]}
*/
const lintAll = entries => {
const lint = (file, tree) => {
const issues = [];

for (const rule of rules) {
issues.push(...rule(entries));
issues.push(...rule(file, tree));
}

return issues;
};

return {
lintAll,
lint,
};
};

Expand Down
13 changes: 8 additions & 5 deletions src/linter/index.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,11 @@ 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 mdast trees
*
* @param {boolean} dryRun Whether to run the engine in dry-run mode
* @param {string[]} disabledRules List of disabled rules names
* @returns {import('./types').Linter}
*/
const createLinter = (dryRun, disabledRules) => {
/**
Expand All @@ -34,10 +35,12 @@ const createLinter = (dryRun, disabledRules) => {
/**
* Lints all entries using the linter engine
*
* @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) => {
issues.push(...engine.lint(file, tree));
};

/**
Expand Down Expand Up @@ -68,7 +71,7 @@ const createLinter = (dryRun, disabledRules) => {
};

return {
lintAll,
lint,
report,
hasError,
};
Expand Down
8 changes: 4 additions & 4 deletions src/linter/rules/index.mjs
Original file line number Diff line number Diff line change
@@ -1,16 +1,16 @@
'use strict';

import { duplicateStabilityNodes } from './duplicate-stability-nodes.mjs';
// import { duplicateStabilityNodes } from './duplicate-stability-nodes.mjs';
import { invalidChangeVersion } from './invalid-change-version.mjs';
import { missingChangeVersion } from './missing-change-version.mjs';
// import { missingChangeVersion } from './missing-change-version.mjs';
import { missingIntroducedIn } from './missing-introduced-in.mjs';

/**
* @type {Record<string, import('../types').LintRule>}
*/
export default {
'duplicate-stability-nodes': duplicateStabilityNodes,
// 'duplicate-stability-nodes': duplicateStabilityNodes,
'invalid-change-version': invalidChangeVersion,
'missing-change-version': missingChangeVersion,
// 'missing-change-version': missingChangeVersion,
'missing-introduced-in': missingIntroducedIn,
};
162 changes: 136 additions & 26 deletions src/linter/rules/invalid-change-version.mjs
Original file line number Diff line number Diff line change
@@ -1,42 +1,152 @@
import { visit } from 'unist-util-visit';
import createQueries from '../../utils/queries/index.mjs';
import {
isSeq,
parseDocument,
isMap,
isPair,
isScalar,
LineCounter,
} from 'yaml';
import {
extractYamlContent,
normalizeYamlSyntax,
} from '../../utils/parser/index.mjs';
import { LINT_MESSAGES } from '../constants.mjs';
import { valid } from 'semver';

/**
* Checks if any change version is invalid
*
* @param {ApiDocMetadataEntry[]} entries
* @param {{value: string, location: import('../types').LintIssueLocation}[]} versions
* @returns {Array<import('../types').LintIssue>}
*/
export const invalidChangeVersion = entries => {
const getInvalidVersions = versions => {
const issues = [];

for (const entry of entries) {
if (entry.changes.length === 0) continue;

const allVersions = entry.changes
.filter(change => change.version)
.flatMap(change =>
Array.isArray(change.version) ? change.version : [change.version]
);

const invalidVersions = allVersions.filter(
version => valid(version) === null
);

issues.push(
...invalidVersions.map(version => ({
level: 'warn',
message: LINT_MESSAGES.invalidChangeVersion.replace(
'{{version}}',
version
),
const invalidVersions = versions.filter(
version => valid(version.value) === null
);

issues.push(
...invalidVersions.map(version => ({
level: 'warn',
message: LINT_MESSAGES.invalidChangeVersion.replace(
'{{version}}',
version.value
),
location: version.location ?? undefined,
}))
);

return issues;
};

/**
* Normalizes a version
*
* @param {import('vfile').VFile} file
* @param {Pair<unknown, unknown>} node
* @param {LineCounter} lineCounter
* @param {number} startLine
*/
const extractNodeVersions = (file, node, lineCounter, startLine) => {
if (isSeq(node.value)) {
return node.value.items.map(item => ({
value: item.value,
location: {
path: `doc/api/${file.basename}`,
position: {
start: {
line: lineCounter.linePos(item.range[0]).line + startLine,
},
end: {
line: lineCounter.linePos(item.range[1]).line + startLine,
},
},
},
}));
}

if (isScalar(node.value)) {
const offset = node.value.range[0];

return [
{
value: node.value.value,
location: {
path: entry.api_doc_source,
position: entry.yaml_position,
path: `doc/api/${file.basename}`,
position: {
start: {
line: lineCounter.linePos(offset).line + startLine,
},
end: {
line: lineCounter.linePos(node.value.range[1]).line + startLine,
},
},
},
}))
);
},
];
}

throw new Error('Change item must be a seq or scalar');
};

/**
* Checks if any change version is invalid
*
* @param {import('vfile').VFile} file
* @param {import('mdast').Root} tree
* @returns {Array<import('../types').LintIssue>}
*/
export const invalidChangeVersion = (file, tree) => {
const issues = [];

visit(tree, createQueries.UNIST.isYamlNode, node => {
const lineCounter = new LineCounter();

const normalizedYaml = normalizeYamlSyntax(extractYamlContent(node));
const doc = parseDocument(normalizedYaml, {
lineCounter,
});

const changes = doc.get('changes');

if (!changes) {
return;
}

if (!isSeq(changes)) {
throw new Error('Changes must be an seq');
}

changes.items.forEach(changeNode => {
if (isMap(changeNode) === false) {
throw new Error('Change item must be a map');
}

changeNode.items.forEach(changeItem => {
if (!isPair(changeItem)) {
throw new Error('Change item must be a pair');
}

if (changeItem.key.value !== 'version') {
return;
}

return issues.push(
...getInvalidVersions(
extractNodeVersions(
file,
changeItem,
lineCounter,
node.position.start.line
)
)
);
});
});
});

return issues;
};
32 changes: 18 additions & 14 deletions src/linter/rules/missing-introduced-in.mjs
Original file line number Diff line number Diff line change
@@ -1,26 +1,30 @@
import { LINT_MESSAGES } from '../constants.mjs';

/**
* Checks if `introduced_in` field is missing
* Checks if `introduced_in` node is missing
*
* @param {ApiDocMetadataEntry[]} entries
* @param {import('vfile').VFile} file
* @param {import('mdast').Root} tree
* @returns {Array<import('../types.d.ts').LintIssue>}
*/
export const missingIntroducedIn = entries => {
const issues = [];
export const missingIntroducedIn = (file, tree) => {
const regex = /<!--introduced_in=.*-->/;

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;
const introduced_in = tree.children.find(
node => node.type === 'html' && regex.test(node.value)
);

issues.push({
level: 'info',
message: LINT_MESSAGES.missingIntroducedIn,
location: {
path: entry.api_doc_source,
if (!introduced_in) {
return [
{
level: 'info',
message: LINT_MESSAGES.missingIntroducedIn,
location: {
path: file.path,
},
},
});
];
}

return issues;
return [];
};
10 changes: 9 additions & 1 deletion src/linter/types.d.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,12 @@
import { Root } from 'mdast';
import { Position } from 'unist';
import { VFile } from 'vfile';

export interface Linter {
lint: (tree: Root) => void;
report: (reporterName: keyof typeof reporters) => void;
hasError: () => boolean;
}

export type IssueLevel = 'info' | 'warn' | 'error';

Expand All @@ -13,6 +21,6 @@ export interface LintIssue {
location: LintIssueLocation;
}

type LintRule = (input: ApiDocMetadataEntry[]) => LintIssue[];
type LintRule = (file: VFile, tree: Root[]) => LintIssue[];

export type Reporter = (msg: LintIssue) => void;
8 changes: 5 additions & 3 deletions src/parsers/markdown.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -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);

Expand Down Expand Up @@ -137,8 +139,8 @@ const createParser = () => {
// 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 => {
// TODO: Is there always only one YAML node?
apiEntryMetadata.setYamlPosition(node.position);

addYAMLMetadata(node, apiEntryMetadata);
});

Expand Down
Loading