Skip to content

Commit 2a8c2a8

Browse files
authored
fix(eslint-plugin-fluid): Indexing fixes (#25586)
Fixed indexing issues in the following rules, which would cause incorrect notification ranges and could cause malformed code fixes: - `@fluid-internal/fluid/no-file-path-links-in-jsdoc` - `@fluid-internal/fluid/no-hyphen-after-jsdoc-tag` - `@fluid-internal/fluid/no-markdown-links-in-jsdoc`
1 parent 517869c commit 2a8c2a8

File tree

9 files changed

+79
-29
lines changed

9 files changed

+79
-29
lines changed

common/build/eslint-plugin-fluid/CHANGELOG.md

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,14 @@
11
# @fluidframework/eslint-plugin-fluid Changelog
22

3-
## [0.3.0](https://github.com/microsoft/FluidFramework/releases/tag/eslint-plugin-fluid_v0.2.0)
3+
## [0.3.1](https://github.com/microsoft/FluidFramework/releases/tag/eslint-plugin-fluid_v0.3.1)
4+
5+
Fixes indexing issues in the following rules, which would cause incorrect notification ranges and could cause malformed code fixes:
6+
7+
- `@fluid-internal/fluid/no-file-path-links-in-jsdoc`
8+
- `@fluid-internal/fluid/no-hyphen-after-jsdoc-tag`
9+
- `@fluid-internal/fluid/no-markdown-links-in-jsdoc`
10+
11+
## [0.3.0](https://github.com/microsoft/FluidFramework/releases/tag/eslint-plugin-fluid_v0.3.0)
412

513
New rules added:
614

common/build/eslint-plugin-fluid/package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{
22
"name": "@fluid-internal/eslint-plugin-fluid",
3-
"version": "0.3.0",
3+
"version": "0.3.1",
44
"description": "Custom ESLint rules for the Fluid Framework",
55
"homepage": "https://fluidframework.com",
66
"repository": {

common/build/eslint-plugin-fluid/src/rules/no-file-path-links-in-jsdoc.js

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,11 +36,14 @@ module.exports = {
3636
.filter((comment) => comment.type === "Block" && comment.value.startsWith("*"));
3737

3838
for (const comment of comments) {
39+
// +2 for the leading "/*", which is omitted by `comment.value`, but included in `comment.range`.
40+
const commentStartIndex = comment.range[0] + 2;
41+
3942
// JSDoc/TSDoc link syntax: {@link target|text} or {@link target}
4043
// Find links where the `target` component is a file path (starts with `/`, `./`, or `../`)
4144
const matches = comment.value.matchAll(/{@link\s+(\/|\.\/|\.\.\/).*}/g);
4245
for (const match of matches) {
43-
const startIndex = comment.range[0] + match.index;
46+
const startIndex = commentStartIndex + match.index;
4447
const endIndex = startIndex + match[0].length;
4548

4649
context.report({

common/build/eslint-plugin-fluid/src/rules/no-hyphen-after-jsdoc-tag.js

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -32,12 +32,15 @@ module.exports = {
3232
.filter((comment) => comment.type === "Block" && comment.value.startsWith("*"));
3333

3434
for (const comment of comments) {
35+
// +2 for the leading "/*", which is omitted by `comment.value`, but included in `comment.range`.
36+
const commentStartIndex = comment.range[0] + 2;
37+
3538
// Find any JSDoc/TSDoc tags followed by a hyphen
36-
const matches = comment.value.matchAll(/(@[a-zA-Z0-9]+)\s*?-(.*)/g);
39+
const matches = comment.value.matchAll(/(@[a-zA-Z0-9]+)\s*?-\s*?(.*)/g);
3740
for (const match of matches) {
3841
const [fullMatch, tag, body] = match;
3942

40-
const startIndex = comment.range[0] + match.index;
43+
const startIndex = commentStartIndex + match.index;
4144
const endIndex = startIndex + fullMatch.length;
4245

4346
context.report({

common/build/eslint-plugin-fluid/src/rules/no-markdown-links-in-jsdoc.js

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,10 +32,14 @@ module.exports = {
3232
.filter((comment) => comment.type === "Block" && comment.value.startsWith("*"));
3333

3434
for (const comment of comments) {
35+
// +2 for the leading "/*", which is omitted by `comment.value`, but included in `comment.range`.
36+
const commentStartIndex = comment.range[0] + 2;
37+
3538
const matches = comment.value.matchAll(/\[([^\]]+)\]\(([^)]+)\)/g);
3639
for (const match of matches) {
3740
const [fullMatch, text, url] = match;
38-
const startIndex = comment.range[0] + match.index;
41+
42+
const startIndex = commentStartIndex + match.index;
3943
const endIndex = startIndex + fullMatch.length;
4044

4145
context.report({

common/build/eslint-plugin-fluid/src/test/enforce-no-file-path-links-in-jsdoc/enforce-no-file-path-links-in-jsdoc.test.js

Lines changed: 20 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -36,19 +36,31 @@ describe("Do not allow file path links in JSDoc/TSDoc comments", function () {
3636
assert.strictEqual(result.errorCount, 4);
3737

3838
// Error 1
39-
assert.strictEqual(result.messages[0].message, expectedErrorMessage);
40-
assert.strictEqual(result.messages[0].line, 10);
39+
const error1 = result.messages[0];
40+
assert.strictEqual(error1.message, expectedErrorMessage);
41+
assert.strictEqual(error1.line, 10);
42+
assert.strictEqual(error1.column, 56); // 1-based, inclusive
43+
assert.strictEqual(error1.endColumn, 84); // 1-based, exclusive
4144

4245
// Error 2
43-
assert.strictEqual(result.messages[1].message, expectedErrorMessage);
44-
assert.strictEqual(result.messages[1].line, 11);
46+
const error2 = result.messages[1];
47+
assert.strictEqual(error2.message, expectedErrorMessage);
48+
assert.strictEqual(error2.line, 11);
49+
assert.strictEqual(error2.column, 17); // 1-based, inclusive
50+
assert.strictEqual(error2.endColumn, 41); // 1-based, exclusive
4551

4652
// Error 3
47-
assert.strictEqual(result.messages[2].message, expectedErrorMessage);
48-
assert.strictEqual(result.messages[2].line, 16);
53+
const error3 = result.messages[2];
54+
assert.strictEqual(error3.message, expectedErrorMessage);
55+
assert.strictEqual(error3.line, 16);
56+
assert.strictEqual(error3.column, 57); // 1-based, inclusive
57+
assert.strictEqual(error3.endColumn, 84); // 1-based, exclusive
4958

5059
// Error 4
51-
assert.strictEqual(result.messages[3].message, expectedErrorMessage);
52-
assert.strictEqual(result.messages[3].line, 17);
60+
const error4 = result.messages[3];
61+
assert.strictEqual(error4.message, expectedErrorMessage);
62+
assert.strictEqual(error4.line, 17);
63+
assert.strictEqual(error4.column, 17); // 1-based, inclusive
64+
assert.strictEqual(error4.endColumn, 40); // 1-based, exclusive
5365
});
5466
});

common/build/eslint-plugin-fluid/src/test/enforce-no-hyphen-after-jsdoc-tag/enforce-no-hyphen-after-jsdoc-tag.test.js

Lines changed: 24 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -41,21 +41,36 @@ describe("Do not allow `-` following JSDoc/TSDoc tags", function () {
4141
assert.strictEqual(result.errorCount, 3);
4242

4343
// Error 1
44-
assert.strictEqual(result.messages[0].message, expectedErrorMessage);
45-
assert.strictEqual(result.messages[0].line, 8);
46-
assert.strictEqual(result.messages[0].fix?.text, "@remarks Here are some remarks.");
44+
const error1 = result.messages[0];
45+
assert.strictEqual(error1.message, expectedErrorMessage);
46+
assert.strictEqual(error1.line, 8);
47+
assert.strictEqual(error1.column, 4); // 1-based, inclusive
48+
assert.strictEqual(error1.endColumn, 37); // 1-based, exclusive
49+
assert(error1.fix !== undefined);
50+
assert.strictEqual(error1.fix.text, "@remarks Here are some remarks.");
51+
assert.deepEqual(error1.fix.range, [226, 259]); // 0-based global character index in the file. The start is inclusive, and the end is exclusive.
4752

4853
// Error 2
49-
assert.strictEqual(result.messages[1].message, expectedErrorMessage);
50-
assert.strictEqual(result.messages[1].line, 9);
54+
const error2 = result.messages[1];
55+
assert.strictEqual(error2.message, expectedErrorMessage);
56+
assert.strictEqual(error2.line, 9);
57+
assert.strictEqual(error2.column, 4); // 1-based, inclusive
58+
assert.strictEqual(error2.endColumn, 65); // 1-based, exclusive
59+
assert(error2.fix !== undefined);
5160
assert.strictEqual(
52-
result.messages[1].fix?.text,
61+
error2.fix.text,
5362
"@deprecated This function is deprecated, use something else.",
5463
);
64+
assert.deepEqual(error2.fix.range, [263, 324]); // 0-based global character index in the file. The start is inclusive, and the end is exclusive.
5565

5666
// Error 3
57-
assert.strictEqual(result.messages[2].message, expectedErrorMessage);
58-
assert.strictEqual(result.messages[2].line, 10);
59-
assert.strictEqual(result.messages[2].fix?.text, "@returns The concatenated string.");
67+
const error3 = result.messages[2];
68+
assert.strictEqual(error3.message, expectedErrorMessage);
69+
assert.strictEqual(error3.line, 10);
70+
assert.strictEqual(error3.column, 4); // 1-based, inclusive
71+
assert.strictEqual(error3.endColumn, 39); // 1-based, exclusive
72+
assert(error3.fix !== undefined);
73+
assert.strictEqual(error3.fix.text, "@returns The concatenated string.");
74+
assert.deepEqual(error3.fix.range, [328, 363]); // 0-based global character index in the file. The start is inclusive, and the end is exclusive.
6075
});
6176
});

common/build/eslint-plugin-fluid/src/test/enforce-no-markdown-links-in-jsdoc/enforce-no-markdown-links-in-jsdoc.test.js

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -31,14 +31,19 @@ describe("Do not allow Markdown links in JSDoc/TSDoc comments", function () {
3131
it("Should report errors for Markdown links in block comments", async function () {
3232
const result = await lintFile("test.ts");
3333
assert.strictEqual(result.errorCount, 1);
34+
35+
const error = result.messages[0];
3436
assert.strictEqual(
35-
result.messages[0].message,
37+
error.message,
3638
"Markdown link syntax (`[text](url)`) is not allowed in JSDoc/TSDoc comments. Use `{@link url|text}` syntax instead.",
3739
);
38-
assert.strictEqual(result.messages[0].line, 10);
40+
assert.strictEqual(error.line, 10);
41+
assert.strictEqual(error.column, 51); // 1-based, inclusive
42+
assert.strictEqual(error.endColumn, 75); // 1-based, exclusive
3943

4044
// Test auto-fix
41-
assert.notEqual(result.messages[0].fix, undefined);
42-
assert.deepEqual(result.messages[0].fix.text, "{@link https://bing.com | bing}");
45+
assert.notEqual(error.fix, undefined);
46+
assert.deepEqual(error.fix.range, [259, 283]); // 0-based global character index in the file. The start is inclusive, and the end is exclusive.
47+
assert.deepEqual(error.fix.text, "{@link https://bing.com | bing}");
4348
});
4449
});

common/build/eslint-plugin-fluid/src/test/example/no-hyphen-after-jsdoc-tag/test.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
*/
55

66
/**
7-
* I am a test function with pretty standard docs, but all of my tags have hyphens after them ☹️.
7+
* I am a test function with pretty standard docs, but all of my tags have hyphens after them :(
88
* @remarks - Here are some remarks.
99
* @deprecated- This function is deprecated, use something else.
1010
* @returns - The concatenated string.
@@ -14,7 +14,7 @@ function invalid<T>(param1: string, param2: T): string {
1414
}
1515

1616
/**
17-
* I am a test function with pretty standard docs, and none of my tags have hyphens after them 🙂.
17+
* I am a test function with pretty standard docs, and none of my tags have hyphens after them :)
1818
* @remarks Here are some remarks.
1919
* @deprecated This function is deprecated, use something else.
2020
* @returns The concatenated string.

0 commit comments

Comments
 (0)