Skip to content

Commit 492c2da

Browse files
araujoguiaraujogui
authored
feat: improve changes issues positions (#313)
* feat: improve changes issues positions * fix: some fixes * refactor: some improvements * refactor: code review * test: create skippable test cases * fix: dedent alignment * test: remove wrong node type
1 parent f26013f commit 492c2da

File tree

7 files changed

+454
-80
lines changed

7 files changed

+454
-80
lines changed

‎src/linter/constants.mjs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ export const LLM_DESCRIPTION_REGEX = /<!--\s?llm_description=.*-->/;
66

77
export const LINT_MESSAGES = {
88
missingIntroducedIn: "Missing 'introduced_in' field in the API doc entry",
9+
invalidChangeProperty: 'Invalid change property type',
910
missingChangeVersion: 'Missing version field in the API doc entry',
1011
invalidChangeVersion: 'Invalid version number: {{version}}',
1112
duplicateStabilityNode: 'Duplicate stability node',

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

Lines changed: 196 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -12,14 +12,14 @@ import { createContext } from './utils.mjs';
1212
describe('invalidChangeVersion', () => {
1313
it('should not report if all change versions are non-empty', () => {
1414
const yamlContent = dedent`
15-
<!-- YAML
16-
changes:
17-
- version:
18-
- v15.7.0
19-
- v14.18.0
20-
- version: v6.4.0
21-
- version: v5.0.0
22-
-->`;
15+
<!-- YAML
16+
changes:
17+
- version:
18+
- v15.7.0
19+
- v14.18.0
20+
- version: v6.4.0
21+
- version: v5.0.0
22+
-->`;
2323

2424
const context = createContext([
2525
{
@@ -35,14 +35,11 @@ changes:
3535

3636
it('should report an issue if a change version is missing', () => {
3737
const yamlContent = dedent`
38-
<!-- YAML
39-
changes:
40-
- version:
41-
- v15.7.0
42-
- v14.18.0
43-
- version: v6.4.0
44-
- version:
45-
-->`;
38+
<!-- YAML
39+
changes:
40+
- version:
41+
- pr-url: https://github.com/nodejs/node/pull/1
42+
-->`;
4643

4744
const context = createContext([
4845
{
@@ -57,17 +54,27 @@ changes:
5754

5855
invalidChangeVersion(context);
5956

60-
strictEqual(context.report.mock.callCount(), 1);
57+
strictEqual(context.report.mock.callCount(), 2);
6158

62-
const call = context.report.mock.calls[0];
59+
const callArguments = context.report.mock.calls.flatMap(
60+
call => call.arguments
61+
);
6362

64-
deepStrictEqual(call.arguments, [
63+
deepStrictEqual(callArguments, [
6564
{
6665
level: 'error',
6766
message: 'Missing version field in the API doc entry',
6867
position: {
69-
start: { line: 1, column: 1, offset: 1 },
70-
end: { line: 1, column: 1, offset: 1 },
68+
start: { line: 3 },
69+
end: { line: 3 },
70+
},
71+
},
72+
{
73+
level: 'error',
74+
message: 'Missing version field in the API doc entry',
75+
position: {
76+
start: { line: 4 },
77+
end: { line: 4 },
7178
},
7279
},
7380
]);
@@ -104,14 +111,14 @@ changes:
104111

105112
it('should not report if all change versions are valid', () => {
106113
const yamlContent = dedent`
107-
<!-- YAML
108-
changes:
109-
- version:
110-
- v15.7.0
111-
- v14.18.0
112-
- version: v6.4.0
113-
- version: v5.0.0
114-
-->`;
114+
<!-- YAML
115+
changes:
116+
- version:
117+
- v15.7.0
118+
- v14.18.0
119+
- version: v6.4.0
120+
- version: v5.0.0
121+
-->`;
115122

116123
const context = createContext([
117124
{
@@ -127,14 +134,14 @@ changes:
127134

128135
it('should report an issue if a change version is invalid', () => {
129136
const yamlContent = dedent`
130-
<!-- YAML
131-
changes:
132-
- version:
133-
- v13.9.0
134-
- INVALID_VERSION
135-
- version: v6.4.0
136-
- version: v5.0.0
137-
-->`;
137+
<!-- YAML
138+
changes:
139+
- version:
140+
- v13.9.0
141+
- INVALID_VERSION
142+
- version: v6.4.0
143+
- version: v5.0.0
144+
-->`;
138145

139146
const context = createContext([
140147
{
@@ -155,23 +162,23 @@ changes:
155162
level: 'error',
156163
message: 'Invalid version number: INVALID_VERSION',
157164
position: {
158-
start: { column: 1, line: 7, offset: 103 },
159-
end: { column: 35, line: 7, offset: 137 },
165+
start: { line: 11 },
166+
end: { line: 11 },
160167
},
161168
},
162169
]);
163170
});
164171

165172
it('should report an issue if a change version contains a REPLACEME and a version', () => {
166173
const yamlContent = dedent`
167-
<!-- YAML
168-
changes:
169-
- version:
170-
- v24.0.0
171-
- REPLACEME
172-
- version: v6.4.0
173-
- version: v5.0.0
174-
-->`;
174+
<!-- YAML
175+
changes:
176+
- version:
177+
- v24.0.0
178+
- REPLACEME
179+
- version: v6.4.0
180+
- version: v5.0.0
181+
-->`;
175182

176183
const context = createContext([
177184
{
@@ -192,10 +199,150 @@ changes:
192199
level: 'error',
193200
message: 'Invalid version number: REPLACEME',
194201
position: {
195-
start: { column: 1, line: 7, offset: 103 },
196-
end: { column: 35, line: 7, offset: 137 },
202+
start: { line: 11 },
203+
end: { line: 11 },
204+
},
205+
},
206+
]);
207+
});
208+
209+
it('should report an issue if changes is not a sequence', () => {
210+
const yamlContent = dedent`
211+
<!-- YAML
212+
changes:
213+
abc:
214+
def:
215+
-->`;
216+
217+
const context = {
218+
tree: {
219+
type: 'root',
220+
children: [
221+
{
222+
type: 'html',
223+
value: yamlContent,
224+
position: {
225+
start: { column: 1, line: 7, offset: 103 },
226+
end: { column: 35, line: 7, offset: 137 },
227+
},
228+
},
229+
],
230+
},
231+
report: mock.fn(),
232+
getIssues: mock.fn(),
233+
};
234+
235+
invalidChangeVersion(context);
236+
strictEqual(context.report.mock.callCount(), 1);
237+
const call = context.report.mock.calls[0];
238+
deepStrictEqual(call.arguments, [
239+
{
240+
level: 'error',
241+
message: 'Invalid change property type',
242+
position: {
243+
start: { line: 8 },
244+
end: { line: 8 },
197245
},
198246
},
199247
]);
200248
});
249+
250+
it('should report an issue if version is not a mapping', () => {
251+
const yamlContent = dedent`
252+
<!-- YAML
253+
changes:
254+
version:
255+
- abc
256+
- def
257+
-->`;
258+
259+
const context = {
260+
tree: {
261+
type: 'root',
262+
children: [
263+
{
264+
type: 'html',
265+
value: yamlContent,
266+
position: {
267+
start: { column: 1, line: 7, offset: 103 },
268+
end: { column: 35, line: 7, offset: 137 },
269+
},
270+
},
271+
],
272+
},
273+
report: mock.fn(),
274+
getIssues: mock.fn(),
275+
};
276+
277+
invalidChangeVersion(context);
278+
strictEqual(context.report.mock.callCount(), 1);
279+
const call = context.report.mock.calls[0];
280+
deepStrictEqual(call.arguments, [
281+
{
282+
level: 'error',
283+
message: 'Invalid change property type',
284+
position: {
285+
start: { line: 8 },
286+
end: { line: 8 },
287+
},
288+
},
289+
]);
290+
});
291+
292+
it("should skip validations if yaml root node isn't a mapping", () => {
293+
const yamlContent = dedent`
294+
<!-- YAML
295+
- abc
296+
- def
297+
-->`;
298+
299+
const context = {
300+
tree: {
301+
type: 'root',
302+
children: [
303+
{
304+
type: 'html',
305+
value: yamlContent,
306+
position: {
307+
start: { column: 1, line: 7, offset: 103 },
308+
end: { column: 35, line: 7, offset: 137 },
309+
},
310+
},
311+
],
312+
},
313+
report: mock.fn(),
314+
getIssues: mock.fn(),
315+
};
316+
317+
invalidChangeVersion(context);
318+
strictEqual(context.report.mock.callCount(), 0);
319+
});
320+
321+
it('should skip validations if changes node is missing', () => {
322+
const yamlContent = dedent`
323+
<!-- YAML
324+
added: v0.1.91
325+
-->`;
326+
327+
const context = {
328+
tree: {
329+
type: 'root',
330+
children: [
331+
{
332+
type: 'html',
333+
value: yamlContent,
334+
position: {
335+
start: { column: 1, line: 7, offset: 103 },
336+
end: { column: 35, line: 7, offset: 137 },
337+
},
338+
},
339+
],
340+
},
341+
report: mock.fn(),
342+
getIssues: mock.fn(),
343+
};
344+
345+
invalidChangeVersion(context);
346+
strictEqual(context.report.mock.callCount(), 0);
347+
});
201348
});

0 commit comments

Comments
 (0)