From c317c35986901eee2c3771b008dfa3be2d627a56 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Guilherme=20Ara=C3=BAjo?= Date: Wed, 11 Jun 2025 12:56:55 -0300 Subject: [PATCH 1/7] feat: improve changes issues positions --- src/linter/constants.mjs | 1 + .../__tests__/invalid-change-version.test.mjs | 119 ++++++++++++++++-- src/linter/rules/invalid-change-version.mjs | 106 ++++++++++++---- src/linter/types.d.ts | 6 +- src/linter/utils/__tests__/yaml.mjs | 99 +++++++++++++++ src/linter/utils/yaml.mjs | 77 ++++++++++++ src/utils/parser/index.mjs | 2 +- 7 files changed, 370 insertions(+), 40 deletions(-) create mode 100644 src/linter/utils/__tests__/yaml.mjs create mode 100644 src/linter/utils/yaml.mjs diff --git a/src/linter/constants.mjs b/src/linter/constants.mjs index 934b32d2..20bc27c7 100644 --- a/src/linter/constants.mjs +++ b/src/linter/constants.mjs @@ -6,6 +6,7 @@ export const LLM_DESCRIPTION_REGEX = //; export const LINT_MESSAGES = { missingIntroducedIn: "Missing 'introduced_in' field in the API doc entry", + invalidChangeProperty: 'Invalid change property type: {{type}}', missingChangeVersion: 'Missing version field in the API doc entry', invalidChangeVersion: 'Invalid version number: {{version}}', duplicateStabilityNode: 'Duplicate stability node', diff --git a/src/linter/rules/__tests__/invalid-change-version.test.mjs b/src/linter/rules/__tests__/invalid-change-version.test.mjs index bdae5c40..df9c2b35 100644 --- a/src/linter/rules/__tests__/invalid-change-version.test.mjs +++ b/src/linter/rules/__tests__/invalid-change-version.test.mjs @@ -44,10 +44,7 @@ changes: `; const context = { @@ -70,17 +67,30 @@ changes: invalidChangeVersion(context); - strictEqual(context.report.mock.callCount(), 1); + strictEqual(context.report.mock.callCount(), 2); - const call = context.report.mock.calls[0]; + const first_call = context.report.mock.calls[0]; - deepStrictEqual(call.arguments, [ + deepStrictEqual(first_call.arguments, [ + { + level: 'error', + message: 'Missing version field in the API doc entry', + position: { + start: { line: 4 }, + end: { line: 4 }, + }, + }, + ]); + + const second_call = context.report.mock.calls[1]; + + deepStrictEqual(second_call.arguments, [ { level: 'error', message: 'Missing version field in the API doc entry', position: { - start: { line: 1, column: 1, offset: 1 }, - end: { line: 1, column: 1, offset: 1 }, + start: { line: 3 }, + end: { line: 3 }, }, }, ]); @@ -182,8 +192,8 @@ changes: level: 'error', message: 'Invalid version number: INVALID_VERSION', position: { - start: { column: 1, line: 7, offset: 103 }, - end: { column: 35, line: 7, offset: 137 }, + start: { line: 11 }, + end: { line: 11 }, }, }, ]); @@ -226,8 +236,91 @@ changes: level: 'error', message: 'Invalid version number: REPLACEME', position: { - start: { column: 1, line: 7, offset: 103 }, - end: { column: 35, line: 7, offset: 137 }, + start: { line: 11 }, + end: { line: 11 }, + }, + }, + ]); + }); + + it('should report an issue if changes is not a sequence', () => { + 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(), + }; + + invalidChangeVersion(context); + strictEqual(context.report.mock.callCount(), 1); + const call = context.report.mock.calls[0]; + deepStrictEqual(call.arguments, [ + { + level: 'error', + message: 'Invalid change property type: PLAIN', + position: { + start: { line: 8 }, + end: { line: 8 }, + }, + }, + ]); + }); + + it('should report an issue if version is not a mapping', () => { + 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(), + }; + + invalidChangeVersion(context); + strictEqual(context.report.mock.callCount(), 1); + const call = context.report.mock.calls[0]; + deepStrictEqual(call.arguments, [ + { + level: 'error', + message: 'Invalid change property type: PLAIN', + position: { + start: { line: 8 }, + end: { line: 8 }, }, }, ]); diff --git a/src/linter/rules/invalid-change-version.mjs b/src/linter/rules/invalid-change-version.mjs index d5f980c7..1a04cc55 100644 --- a/src/linter/rules/invalid-change-version.mjs +++ b/src/linter/rules/invalid-change-version.mjs @@ -2,15 +2,19 @@ import { env } from 'node:process'; import { valid, parse } from 'semver'; import { visit } from 'unist-util-visit'; -import yaml from 'yaml'; +import { isMap, isSeq, LineCounter, parseDocument } from 'yaml'; -import { enforceArray } from '../../utils/array.mjs'; import { extractYamlContent, normalizeYamlSyntax, } from '../../utils/parser/index.mjs'; import createQueries from '../../utils/queries/index.mjs'; import { LINT_MESSAGES } from '../constants.mjs'; +import { + createYamlIssueReporter, + findPropertyByName, + normalizeNode, +} from '../utils/yaml.mjs'; const NODE_RELEASED_VERSIONS = env.NODE_RELEASED_VERSIONS?.split(','); @@ -42,7 +46,7 @@ const isIgnoredVersion = version => { /** * Determines if a given version is invalid. * - * @param {string} version - The version to check. + * @param {Scalar} version - The version to check. * @param {unknown} _ - Unused parameter. * @param {{ length: number }} context - Array containing the length property. * @returns {boolean} True if the version is invalid, otherwise false. @@ -50,12 +54,42 @@ const isIgnoredVersion = version => { const isInvalid = NODE_RELEASED_VERSIONS ? (version, _, { length }) => !( - isValidReplaceMe(version, length) || - isIgnoredVersion(version) || - NODE_RELEASED_VERSIONS.includes(version.replace(/^v/, '')) + isValidReplaceMe(version.value, length) || + isIgnoredVersion(version.value) || + NODE_RELEASED_VERSIONS.includes(version.value.replace(/^v/, '')) ) : (version, _, { length }) => - !(isValidReplaceMe(version, length) || valid(version)); + !(isValidReplaceMe(version.value, length) || valid(version.value)); + +/** + * + * @param {object} root0 + * @param {import('../types.d.ts').LintContext} root0.context + * @param {import('yaml').Node} root0.node + * @param {(message: string, node: import('yaml').Node) => import('../types.d.ts').IssueDescriptor} root0.report + */ +export const extractVersions = ({ context, node, report }) => { + if (!isMap(node)) { + context.report( + report( + LINT_MESSAGES.invalidChangeProperty.replace('{{type}}', node.NODE_TYPE), + node + ) + ); + + return null; + } + + const versionNode = findPropertyByName(node, 'version'); + + if (!versionNode) { + context.report(report(LINT_MESSAGES.missingChangeVersion, node)); + + return null; + } + + return normalizeNode(versionNode.value); +}; /** * Identifies invalid change versions from metadata entries. @@ -69,30 +103,52 @@ export const invalidChangeVersion = context => { const normalizedYaml = normalizeYamlSyntax(yamlContent); - // TODO: Use YAML AST to provide better issues positions - /** - * @type {ApiDocRawMetadataEntry} - */ - const { changes } = yaml.parse(normalizedYaml); + const lineCounter = new LineCounter(); + const document = parseDocument(normalizedYaml, { lineCounter }); - if (!changes) { + const report = createYamlIssueReporter(node, lineCounter); + + // Skip if yaml isn't a mapping + if (!isMap(document.contents)) { return; } - changes.forEach(({ version }) => - enforceArray(version) + const changesNode = findPropertyByName(document.contents, 'changes'); + + // Skip if changes node is not present + if (!changesNode) { + return; + } + + // Validate changes node is a sequence + if (!isSeq(changesNode.value)) { + return context.report( + report( + LINT_MESSAGES.invalidChangeProperty.replace( + '{{type}}', + changesNode.value.NODE_TYPE + ), + changesNode.key + ) + ); + } + + changesNode.value.items.forEach(node => + extractVersions({ context, node, report }) + .filter(Boolean) // Filter already reported empt items .filter(isInvalid) .forEach(version => - context.report({ - level: 'error', - message: version - ? LINT_MESSAGES.invalidChangeVersion.replace( - '{{version}}', - version - ) - : LINT_MESSAGES.missingChangeVersion, - position: node.position, - }) + context.report( + report( + version?.value + ? LINT_MESSAGES.invalidChangeVersion.replace( + '{{version}}', + version.value + ) + : LINT_MESSAGES.missingChangeVersion, + version + ) + ) ) ); }); diff --git a/src/linter/types.d.ts b/src/linter/types.d.ts index 2c328e17..41e64cf9 100644 --- a/src/linter/types.d.ts +++ b/src/linter/types.d.ts @@ -1,10 +1,14 @@ import { Root } from 'mdast'; -import { Position } from 'unist'; import reporters from './reporters/index.mjs'; import { VFile } from 'vfile'; export type IssueLevel = 'info' | 'warn' | 'error'; +export interface Position { + start: { line: number }; + end: { line: number }; +} + export interface LintIssueLocation { path: string; // The absolute path to the file position?: Position; diff --git a/src/linter/utils/__tests__/yaml.mjs b/src/linter/utils/__tests__/yaml.mjs new file mode 100644 index 00000000..ec715053 --- /dev/null +++ b/src/linter/utils/__tests__/yaml.mjs @@ -0,0 +1,99 @@ +import { deepStrictEqual, strictEqual, throws } from 'node:assert'; +import { describe, it } from 'node:test'; + +import { Scalar, Pair, YAMLSeq, YAMLMap } from 'yaml'; + +import { findPropertyByName, normalizeNode } from '../yaml.mjs'; + +describe('yaml', () => { + describe('findPropertyByName', () => { + it('should find a property by name when it exists', () => { + const mockMap = { + items: [ + new Pair(new Scalar('propertyA'), new Scalar('valueA')), + new Pair(new Scalar('propertyB'), new Scalar('valueB')), + ], + }; + + const result = findPropertyByName(mockMap, 'propertyA'); + + strictEqual(result.key.value, 'propertyA'); + strictEqual(result.value.value, 'valueA'); + }); + + it('should return undefined when property does not exist', () => { + const mockMap = { + items: [ + new Pair(new Scalar('propertyA'), new Scalar('valueA')), + new Pair(new Scalar('propertyB'), new Scalar('valueB')), + ], + }; + + const result = findPropertyByName(mockMap, 'nonExistent'); + strictEqual(result, undefined); + }); + }); + + describe('normalizeNode', () => { + it('should normalize a scalar node', () => { + const scalar = new Scalar('test-value'); + scalar.range = [0, 10, 10]; + + const result = normalizeNode(scalar); + + deepStrictEqual(result, [ + { + value: 'test-value', + range: [0, 10, 10], + }, + ]); + }); + + it('should normalize a sequence with scalar items', () => { + const item1 = new Scalar('first'); + item1.range = [0, 5, 5]; + + const item2 = new Scalar('second'); + item2.range = [6, 12, 12]; + + const sequence = new YAMLSeq(); + sequence.items = [item1, item2]; + + const result = normalizeNode(sequence); + + deepStrictEqual(result, [ + { value: 'first', range: [0, 5, 5] }, + { value: 'second', range: [6, 12, 12] }, + ]); + }); + + it('should normalize nested sequences', () => { + const innerItem = new Scalar('nested'); + innerItem.range = [0, 6, 6]; + + const innerSeq = new YAMLSeq(); + innerSeq.items = [innerItem]; + + const outerItem = new Scalar('outer'); + outerItem.range = [6, 12, 12]; + + const outerSeq = new YAMLSeq(); + outerSeq.items = [outerItem, innerSeq]; + + const result = normalizeNode(outerSeq); + + deepStrictEqual(result, [ + { value: 'outer', range: [6, 12, 12] }, + { value: 'nested', range: [0, 6, 6] }, + ]); + }); + + it('should throw error for map nodes', () => { + const mapNode = new YAMLMap(); + + throws(() => normalizeNode(mapNode), { + message: 'Unexpected node type: map', + }); + }); + }); +}); diff --git a/src/linter/utils/yaml.mjs b/src/linter/utils/yaml.mjs new file mode 100644 index 00000000..cdd460e5 --- /dev/null +++ b/src/linter/utils/yaml.mjs @@ -0,0 +1,77 @@ +// @ts-check + +'use strict'; + +import { isScalar, isSeq, isNode } from 'yaml'; + +/** + * Searches for a property by name in a map node. + * + * @param {import('yaml').YAMLMap} node - The map node to search in. + * @param {string} propertyName - The property name to search for. + * @returns {import('yaml').Pair | undefined} + */ +export const findPropertyByName = (node, propertyName) => { + return node.items.find(pair => { + if (!isScalar(pair.key)) { + return; + } + + return pair.key.value == propertyName; + }); +}; + +/** + * Normalizes a YAML node values into an array. + * + * @param {import('yaml').Node} node + * @returns {import('yaml').Scalar[]} + */ +export const normalizeNode = node => { + if (isScalar(node)) { + return [node]; + } + + if (isSeq(node)) { + // @ts-ignore + return node.items.flatMap(item => normalizeNode(item)); + } + + throw new Error(`Unexpected node type: map`); +}; + +/** + * Creates a factory function for generating error descriptors with proper line positioning + * for YAML nodes within markdown documents. + * + * @param {import('mdast').Node} yamlNode + * @param {import('yaml').LineCounter} lineCounter + */ +export const createYamlIssueReporter = (yamlNode, lineCounter) => { + const initialLine = yamlNode.position?.start.line ?? 0; + + /** + * @param {string} message + * @param {import('yaml').Node} node + * @returns {import('../types').IssueDescriptor} + */ + return (message, node) => { + const absoluteLine = + isNode(node) && node.range + ? initialLine + lineCounter.linePos(node.range[0]).line + : initialLine; + + return { + level: 'error', + message, + position: { + start: { + line: absoluteLine, + }, + end: { + line: absoluteLine, + }, + }, + }; + }; +}; diff --git a/src/utils/parser/index.mjs b/src/utils/parser/index.mjs index 5a6fc485..b47cf7af 100644 --- a/src/utils/parser/index.mjs +++ b/src/utils/parser/index.mjs @@ -16,7 +16,7 @@ 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 + * @param {import('mdast').Node} node A HTML node containing the YAML content * @returns {string} The extracted raw YAML content */ export const extractYamlContent = node => { From 41eb2cef0cdbe87e843fc0ab5e62bc16de8e4339 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Guilherme=20Ara=C3=BAjo?= Date: Wed, 11 Jun 2025 13:23:16 -0300 Subject: [PATCH 2/7] fix: some fixes --- .../__tests__/invalid-change-version.test.mjs | 8 +++---- src/linter/rules/invalid-change-version.mjs | 24 ++++++++++++------- 2 files changed, 19 insertions(+), 13 deletions(-) diff --git a/src/linter/rules/__tests__/invalid-change-version.test.mjs b/src/linter/rules/__tests__/invalid-change-version.test.mjs index df9c2b35..35a86e69 100644 --- a/src/linter/rules/__tests__/invalid-change-version.test.mjs +++ b/src/linter/rules/__tests__/invalid-change-version.test.mjs @@ -76,8 +76,8 @@ changes: level: 'error', message: 'Missing version field in the API doc entry', position: { - start: { line: 4 }, - end: { line: 4 }, + start: { line: 3 }, + end: { line: 3 }, }, }, ]); @@ -89,8 +89,8 @@ changes: level: 'error', message: 'Missing version field in the API doc entry', position: { - start: { line: 3 }, - end: { line: 3 }, + start: { line: 4 }, + end: { line: 4 }, }, }, ]); diff --git a/src/linter/rules/invalid-change-version.mjs b/src/linter/rules/invalid-change-version.mjs index 1a04cc55..22f6664e 100644 --- a/src/linter/rules/invalid-change-version.mjs +++ b/src/linter/rules/invalid-change-version.mjs @@ -72,12 +72,12 @@ export const extractVersions = ({ context, node, report }) => { if (!isMap(node)) { context.report( report( - LINT_MESSAGES.invalidChangeProperty.replace('{{type}}', node.NODE_TYPE), + LINT_MESSAGES.invalidChangeProperty.replace('{{type}}', node.type), node ) ); - return null; + return; } const versionNode = findPropertyByName(node, 'version'); @@ -85,7 +85,7 @@ export const extractVersions = ({ context, node, report }) => { if (!versionNode) { context.report(report(LINT_MESSAGES.missingChangeVersion, node)); - return null; + return; } return normalizeNode(versionNode.value); @@ -126,16 +126,22 @@ export const invalidChangeVersion = context => { report( LINT_MESSAGES.invalidChangeProperty.replace( '{{type}}', - changesNode.value.NODE_TYPE + changesNode.value.type ), changesNode.key ) ); } - changesNode.value.items.forEach(node => - extractVersions({ context, node, report }) - .filter(Boolean) // Filter already reported empt items + changesNode.value.items.forEach(node => { + const versions = extractVersions({ context, node, report }); + + if (!versions) { + return; + } + + versions + .filter(Boolean) // Filter already reported empt items, .filter(isInvalid) .forEach(version => context.report( @@ -149,7 +155,7 @@ export const invalidChangeVersion = context => { version ) ) - ) - ); + ); + }); }); }; From 7ce79ef95e0e599e08dead77dc55448031e278d4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Guilherme=20Ara=C3=BAjo?= Date: Wed, 11 Jun 2025 13:27:08 -0300 Subject: [PATCH 3/7] refactor: some improvements --- src/linter/rules/invalid-change-version.mjs | 13 ++++--------- src/linter/utils/yaml.mjs | 2 +- 2 files changed, 5 insertions(+), 10 deletions(-) diff --git a/src/linter/rules/invalid-change-version.mjs b/src/linter/rules/invalid-change-version.mjs index 22f6664e..9ec11fbc 100644 --- a/src/linter/rules/invalid-change-version.mjs +++ b/src/linter/rules/invalid-change-version.mjs @@ -62,6 +62,7 @@ const isInvalid = NODE_RELEASED_VERSIONS !(isValidReplaceMe(version.value, length) || valid(version.value)); /** + * Validates and extracts versions of a change node * * @param {object} root0 * @param {import('../types.d.ts').LintContext} root0.context @@ -77,7 +78,7 @@ export const extractVersions = ({ context, node, report }) => { ) ); - return; + return []; } const versionNode = findPropertyByName(node, 'version'); @@ -85,7 +86,7 @@ export const extractVersions = ({ context, node, report }) => { if (!versionNode) { context.report(report(LINT_MESSAGES.missingChangeVersion, node)); - return; + return []; } return normalizeNode(versionNode.value); @@ -134,13 +135,7 @@ export const invalidChangeVersion = context => { } changesNode.value.items.forEach(node => { - const versions = extractVersions({ context, node, report }); - - if (!versions) { - return; - } - - versions + extractVersions({ context, node, report }) .filter(Boolean) // Filter already reported empt items, .filter(isInvalid) .forEach(version => diff --git a/src/linter/utils/yaml.mjs b/src/linter/utils/yaml.mjs index cdd460e5..63df8ba8 100644 --- a/src/linter/utils/yaml.mjs +++ b/src/linter/utils/yaml.mjs @@ -17,7 +17,7 @@ export const findPropertyByName = (node, propertyName) => { return; } - return pair.key.value == propertyName; + return pair.key.value === propertyName; }); }; From e98d69f7bc7cfebec783dadcb9340ffded97eb4e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Guilherme=20Ara=C3=BAjo?= Date: Wed, 11 Jun 2025 18:00:39 -0300 Subject: [PATCH 4/7] refactor: code review --- .../__tests__/invalid-change-version.test.mjs | 11 +++---- src/linter/rules/invalid-change-version.mjs | 30 +++++++++---------- 2 files changed, 19 insertions(+), 22 deletions(-) diff --git a/src/linter/rules/__tests__/invalid-change-version.test.mjs b/src/linter/rules/__tests__/invalid-change-version.test.mjs index 35a86e69..3665d475 100644 --- a/src/linter/rules/__tests__/invalid-change-version.test.mjs +++ b/src/linter/rules/__tests__/invalid-change-version.test.mjs @@ -69,9 +69,11 @@ changes: strictEqual(context.report.mock.callCount(), 2); - const first_call = context.report.mock.calls[0]; + const callArguments = context.report.mock.calls.flatMap( + call => call.arguments + ); - deepStrictEqual(first_call.arguments, [ + deepStrictEqual(callArguments, [ { level: 'error', message: 'Missing version field in the API doc entry', @@ -80,11 +82,6 @@ changes: end: { line: 3 }, }, }, - ]); - - const second_call = context.report.mock.calls[1]; - - deepStrictEqual(second_call.arguments, [ { level: 'error', message: 'Missing version field in the API doc entry', diff --git a/src/linter/rules/invalid-change-version.mjs b/src/linter/rules/invalid-change-version.mjs index 9ec11fbc..926b3dcd 100644 --- a/src/linter/rules/invalid-change-version.mjs +++ b/src/linter/rules/invalid-change-version.mjs @@ -46,20 +46,20 @@ const isIgnoredVersion = version => { /** * Determines if a given version is invalid. * - * @param {Scalar} version - The version to check. + * @param {import('yaml').Scalar} version - The version to check. * @param {unknown} _ - Unused parameter. * @param {{ length: number }} context - Array containing the length property. * @returns {boolean} True if the version is invalid, otherwise false. */ const isInvalid = NODE_RELEASED_VERSIONS - ? (version, _, { length }) => + ? ({ value }, _, { length }) => !( - isValidReplaceMe(version.value, length) || - isIgnoredVersion(version.value) || - NODE_RELEASED_VERSIONS.includes(version.value.replace(/^v/, '')) + isValidReplaceMe(value, length) || + isIgnoredVersion(value) || + NODE_RELEASED_VERSIONS.includes(value.replace(/^v/, '')) ) - : (version, _, { length }) => - !(isValidReplaceMe(version.value, length) || valid(version.value)); + : ({ value }, _, { length }) => + !(isValidReplaceMe(value, length) || valid(value)); /** * Validates and extracts versions of a change node @@ -67,12 +67,12 @@ const isInvalid = NODE_RELEASED_VERSIONS * @param {object} root0 * @param {import('../types.d.ts').LintContext} root0.context * @param {import('yaml').Node} root0.node - * @param {(message: string, node: import('yaml').Node) => import('../types.d.ts').IssueDescriptor} root0.report + * @param {(message: string, node: import('yaml').Node) => import('../types.d.ts').IssueDescriptor} root0.createYamlIssue */ -export const extractVersions = ({ context, node, report }) => { +export const extractVersions = ({ context, node, createYamlIssue }) => { if (!isMap(node)) { context.report( - report( + createYamlIssue( LINT_MESSAGES.invalidChangeProperty.replace('{{type}}', node.type), node ) @@ -84,7 +84,7 @@ export const extractVersions = ({ context, node, report }) => { const versionNode = findPropertyByName(node, 'version'); if (!versionNode) { - context.report(report(LINT_MESSAGES.missingChangeVersion, node)); + context.report(createYamlIssue(LINT_MESSAGES.missingChangeVersion, node)); return []; } @@ -107,7 +107,7 @@ export const invalidChangeVersion = context => { const lineCounter = new LineCounter(); const document = parseDocument(normalizedYaml, { lineCounter }); - const report = createYamlIssueReporter(node, lineCounter); + const createYamlIssue = createYamlIssueReporter(node, lineCounter); // Skip if yaml isn't a mapping if (!isMap(document.contents)) { @@ -124,7 +124,7 @@ export const invalidChangeVersion = context => { // Validate changes node is a sequence if (!isSeq(changesNode.value)) { return context.report( - report( + createYamlIssue( LINT_MESSAGES.invalidChangeProperty.replace( '{{type}}', changesNode.value.type @@ -135,12 +135,12 @@ export const invalidChangeVersion = context => { } changesNode.value.items.forEach(node => { - extractVersions({ context, node, report }) + extractVersions({ context, node, createYamlIssue }) .filter(Boolean) // Filter already reported empt items, .filter(isInvalid) .forEach(version => context.report( - report( + createYamlIssue( version?.value ? LINT_MESSAGES.invalidChangeVersion.replace( '{{version}}', From a1299392748b1ed536f10e5255ab1b6b9d6495f6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Guilherme=20Ara=C3=BAjo?= Date: Wed, 11 Jun 2025 18:04:44 -0300 Subject: [PATCH 5/7] test: create skippable test cases --- .../__tests__/invalid-change-version.test.mjs | 57 +++++++++++++++++++ 1 file changed, 57 insertions(+) diff --git a/src/linter/rules/__tests__/invalid-change-version.test.mjs b/src/linter/rules/__tests__/invalid-change-version.test.mjs index 3665d475..eea9598e 100644 --- a/src/linter/rules/__tests__/invalid-change-version.test.mjs +++ b/src/linter/rules/__tests__/invalid-change-version.test.mjs @@ -322,4 +322,61 @@ changes: }, ]); }); + + it("should skip validations if yaml root node isn't a mapping", () => { + 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(), + }; + + invalidChangeVersion(context); + strictEqual(context.report.mock.callCount(), 0); + }); + + it('should skip validations if changes node is missing', () => { + 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(), + }; + + invalidChangeVersion(context); + strictEqual(context.report.mock.callCount(), 0); + }); }); From cda6a804147a737ba63b16bf6feb98116aa9f596 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Guilherme=20Ara=C3=BAjo?= Date: Sat, 14 Jun 2025 16:30:08 -0300 Subject: [PATCH 6/7] fix: dedent alignment --- .../__tests__/invalid-change-version.test.mjs | 110 +++++++++--------- 1 file changed, 55 insertions(+), 55 deletions(-) diff --git a/src/linter/rules/__tests__/invalid-change-version.test.mjs b/src/linter/rules/__tests__/invalid-change-version.test.mjs index eea9598e..420da3f7 100644 --- a/src/linter/rules/__tests__/invalid-change-version.test.mjs +++ b/src/linter/rules/__tests__/invalid-change-version.test.mjs @@ -11,14 +11,14 @@ import { invalidChangeVersion } from '../invalid-change-version.mjs'; describe('invalidChangeVersion', () => { it('should not report if all change versions are non-empty', () => { const yamlContent = dedent` -`; + `; const context = { tree: { @@ -41,11 +41,11 @@ changes: it('should report an issue if a change version is missing', () => { const yamlContent = dedent` -`; + `; const context = { tree: { @@ -124,14 +124,14 @@ changes: it('should not report if all change versions are valid', () => { const yamlContent = dedent` -`; + `; const context = { tree: { @@ -154,14 +154,14 @@ changes: it('should report an issue if a change version is invalid', () => { const yamlContent = dedent` -`; + `; const context = { tree: { @@ -198,14 +198,14 @@ changes: it('should report an issue if a change version contains a REPLACEME and a version', () => { const yamlContent = dedent` -`; + `; const context = { tree: { @@ -242,11 +242,11 @@ changes: it('should report an issue if changes is not a sequence', () => { const yamlContent = dedent` -`; + `; const context = { tree: { @@ -283,12 +283,12 @@ changes: it('should report an issue if version is not a mapping', () => { const yamlContent = dedent` -`; + `; const context = { tree: { @@ -325,10 +325,10 @@ changes: it("should skip validations if yaml root node isn't a mapping", () => { const yamlContent = dedent` -`; + `; const context = { tree: { @@ -354,9 +354,9 @@ changes: it('should skip validations if changes node is missing', () => { const yamlContent = dedent` -`; + `; const context = { tree: { From fcb134177cc72cb9652ac2cacf3d7e8faeda7ee1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Guilherme=20Ara=C3=BAjo?= Date: Sat, 14 Jun 2025 16:31:53 -0300 Subject: [PATCH 7/7] test: remove wrong node type --- src/linter/constants.mjs | 2 +- .../__tests__/invalid-change-version.test.mjs | 4 ++-- src/linter/rules/invalid-change-version.mjs | 15 ++------------- 3 files changed, 5 insertions(+), 16 deletions(-) diff --git a/src/linter/constants.mjs b/src/linter/constants.mjs index 20bc27c7..ab0edd2b 100644 --- a/src/linter/constants.mjs +++ b/src/linter/constants.mjs @@ -6,7 +6,7 @@ export const LLM_DESCRIPTION_REGEX = //; export const LINT_MESSAGES = { missingIntroducedIn: "Missing 'introduced_in' field in the API doc entry", - invalidChangeProperty: 'Invalid change property type: {{type}}', + invalidChangeProperty: 'Invalid change property type', missingChangeVersion: 'Missing version field in the API doc entry', invalidChangeVersion: 'Invalid version number: {{version}}', duplicateStabilityNode: 'Duplicate stability node', diff --git a/src/linter/rules/__tests__/invalid-change-version.test.mjs b/src/linter/rules/__tests__/invalid-change-version.test.mjs index 420da3f7..5df682f6 100644 --- a/src/linter/rules/__tests__/invalid-change-version.test.mjs +++ b/src/linter/rules/__tests__/invalid-change-version.test.mjs @@ -272,7 +272,7 @@ describe('invalidChangeVersion', () => { deepStrictEqual(call.arguments, [ { level: 'error', - message: 'Invalid change property type: PLAIN', + message: 'Invalid change property type', position: { start: { line: 8 }, end: { line: 8 }, @@ -314,7 +314,7 @@ describe('invalidChangeVersion', () => { deepStrictEqual(call.arguments, [ { level: 'error', - message: 'Invalid change property type: PLAIN', + message: 'Invalid change property type', position: { start: { line: 8 }, end: { line: 8 }, diff --git a/src/linter/rules/invalid-change-version.mjs b/src/linter/rules/invalid-change-version.mjs index 926b3dcd..47a54f5c 100644 --- a/src/linter/rules/invalid-change-version.mjs +++ b/src/linter/rules/invalid-change-version.mjs @@ -71,12 +71,7 @@ const isInvalid = NODE_RELEASED_VERSIONS */ export const extractVersions = ({ context, node, createYamlIssue }) => { if (!isMap(node)) { - context.report( - createYamlIssue( - LINT_MESSAGES.invalidChangeProperty.replace('{{type}}', node.type), - node - ) - ); + context.report(createYamlIssue(LINT_MESSAGES.invalidChangeProperty, node)); return []; } @@ -124,13 +119,7 @@ export const invalidChangeVersion = context => { // Validate changes node is a sequence if (!isSeq(changesNode.value)) { return context.report( - createYamlIssue( - LINT_MESSAGES.invalidChangeProperty.replace( - '{{type}}', - changesNode.value.type - ), - changesNode.key - ) + createYamlIssue(LINT_MESSAGES.invalidChangeProperty, changesNode.key) ); }