Skip to content

Commit 7bc8741

Browse files
JosmithrJosmithr
authored
fix(eslint-plugin-fluid): Fix false-positives in no-markdown-links-in-jsdoc (#25930)
Also bumps package version for publishing. [AB#51719](https://dev.azure.com/fluidframework/235294da-091d-4c29-84fc-cdfc3d90890b/_workitems/edit/51719)
1 parent 22b549e commit 7bc8741

File tree

5 files changed

+183
-36
lines changed

5 files changed

+183
-36
lines changed

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

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,12 @@
11
# @fluidframework/eslint-plugin-fluid Changelog
22

3+
## [0.4.1](https://github.com/microsoft/FluidFramework/releases/tag/eslint-plugin-fluid_v0.4.1)
4+
5+
Fixes false positives in the following rules:
6+
7+
- `@fluid-internal/fluid/no-hyphen-after-jsdoc-tag`
8+
- `@fluid-internal/fluid/no-markdown-links-in-jsdoc`
9+
310
## [0.4.0](https://github.com/microsoft/FluidFramework/releases/tag/eslint-plugin-fluid_v0.4.0)
411

512
Updates plugin to be compatible with both ESLint 8 and ESLint 9.

‎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.4.0",
3+
"version": "0.4.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-markdown-links-in-jsdoc.js‎

Lines changed: 140 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,81 @@
33
* Licensed under the MIT License.
44
*/
55

6-
// eslint-plugin-no-markdown-comments/lib/rules/no-markdown-links-in-comments.js
7-
module.exports = {
6+
//@ts-check
7+
/**
8+
* @typedef {import("eslint").Rule.RuleModule} RuleModule
9+
* @typedef {import('@microsoft/tsdoc').DocNode} DocNode
10+
* @typedef {import('@microsoft/tsdoc').DocPlainText} DocPlainText
11+
*
12+
* @typedef {{
13+
* linkText: string;
14+
* linkTarget: string;
15+
* startIndex: number;
16+
* endIndex: number;
17+
* }} MarkdownLinkInfo
18+
*/
19+
20+
const { fail } = require("node:assert");
21+
const { DocNodeKind, TSDocParser } = require("@microsoft/tsdoc");
22+
23+
const parser = new TSDocParser();
24+
25+
/**
26+
* Finds instances of Markdown-syntax links within the provided plain text.
27+
* @param {DocPlainText} plainTextNode - The plain text node to check.
28+
* @returns {MarkdownLinkInfo[]} The list of found Markdown links.
29+
*/
30+
function findMarkdownLinksInPlainText(plainTextNode) {
31+
const textRange =
32+
plainTextNode.textExcerpt?.getContainingTextRange() ??
33+
fail("Expected textExcerpt to be defined.");
34+
// RegEx explanation:
35+
// \[ - Match the opening square bracket
36+
// ([^\]]*) - Capture group 1: Match zero or more characters that are not a closing square bracket (the link text)
37+
// \] - Match the closing square bracket
38+
// \( - Match the opening parenthesis
39+
// ([^)]*) - Capture group 2: Match zero or more characters that are not a closing parenthesis (the link target)
40+
// \) - Match the closing parenthesis
41+
const matches = plainTextNode.text.matchAll(/\[([^\]]*)\]\(([^)]*)\)/g);
42+
return Array.from(matches, (match) => ({
43+
linkText: match[1],
44+
linkTarget: match[2],
45+
startIndex: textRange.pos + match.index,
46+
endIndex: textRange.pos + match.index + match[0].length,
47+
}));
48+
}
49+
50+
/**
51+
* Finds instances of Markdown-syntax links within the provided comment body.
52+
* @param { DocNode } commentBodyNode - The doc node representing the body of the comment.
53+
* @returns {MarkdownLinkInfo[]} The list of found Markdown links.
54+
*/
55+
function findMarkdownLinks(commentBodyNode) {
56+
// Walk down all children to find all plain text nodes.
57+
// Search those nodes for Markdown links.
58+
// We only search plain text because we want to ignore link syntax that may appear in other
59+
// contexts like code spans / code blocks where they would not be interpreted as links, and
60+
// where they may exist to serve as examples, etc.
61+
if (commentBodyNode.kind === DocNodeKind.PlainText) {
62+
return findMarkdownLinksInPlainText(/** @type {DocPlainText} */ (commentBodyNode));
63+
}
64+
65+
const childNodes = commentBodyNode.getChildNodes();
66+
67+
const links = [];
68+
for (const childNode of childNodes) {
69+
links.push(...findMarkdownLinks(childNode));
70+
}
71+
return links;
72+
}
73+
74+
/**
75+
* Eslint rule to disallow Markdown link syntax in JSDoc/TSDoc comments.
76+
* `{@link}` syntax should be used instead.
77+
*
78+
* @type {RuleModule}
79+
*/
80+
const rule = {
881
meta: {
982
type: "problem",
1083
docs: {
@@ -32,33 +105,74 @@ module.exports = {
32105
.filter((comment) => comment.type === "Block" && comment.value.startsWith("*"));
33106

34107
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-
38-
const matches = comment.value.matchAll(/\[([^\]]+)\]\(([^)]+)\)/g);
39-
for (const match of matches) {
40-
const [fullMatch, text, url] = match;
41-
42-
const startIndex = commentStartIndex + match.index;
43-
const endIndex = startIndex + fullMatch.length;
44-
45-
context.report({
46-
loc: {
47-
start: sourceCode.getLocFromIndex(startIndex),
48-
end: sourceCode.getLocFromIndex(endIndex),
49-
},
50-
messageId: "markdownLink",
51-
fix(fixer) {
52-
const trimmedText = text?.trim();
53-
const tsdocLink = trimmedText
54-
? `{@link ${url} | ${trimmedText}}`
55-
: `{@link ${url}}`;
56-
return fixer.replaceTextRange([startIndex, endIndex], tsdocLink);
57-
},
58-
});
108+
if (comment.range === undefined) {
109+
continue;
110+
}
111+
112+
const commentStartIndex = comment.range[0];
113+
114+
// TSDoc parser requires the surrounding "/**" and "*/", but eslint strips those off in `comment.value`.
115+
const parserContext = parser.parseString(`/**${comment.value}*/`);
116+
const parsedComment = parserContext.docComment;
117+
118+
const blocksToCheck = [
119+
...parsedComment.customBlocks,
120+
...parsedComment.seeBlocks,
121+
];
122+
if (parsedComment.remarksBlock) {
123+
blocksToCheck.push(parsedComment.remarksBlock);
124+
}
125+
if (parsedComment.privateRemarks) {
126+
blocksToCheck.push(parsedComment.privateRemarks);
127+
}
128+
if (parsedComment.deprecatedBlock) {
129+
blocksToCheck.push(parsedComment.deprecatedBlock);
130+
}
131+
if (parsedComment.returnsBlock) {
132+
blocksToCheck.push(parsedComment.returnsBlock);
133+
}
134+
135+
/**
136+
* Checks the provided comment block for Markdown-syntax links and report eslint errors for them.
137+
* @param {DocNode} node - The comment block to check.
138+
* @returns {void}
139+
*/
140+
function checkCommentBlock(node) {
141+
const links = findMarkdownLinks(node);
142+
for (const link of links) {
143+
const startIndex = commentStartIndex + link.startIndex - 1;
144+
const endIndex = commentStartIndex + link.endIndex - 1;
145+
146+
context.report({
147+
loc: {
148+
start: sourceCode.getLocFromIndex(startIndex),
149+
end: sourceCode.getLocFromIndex(endIndex),
150+
},
151+
messageId: "markdownLink",
152+
fix(fixer) {
153+
const trimmedText = link.linkText.trim();
154+
const tsdocLink = trimmedText.length > 0
155+
? `{@link ${link.linkTarget} | ${trimmedText}}`
156+
: `{@link ${link.linkTarget}}`;
157+
return fixer.replaceTextRange(
158+
[startIndex, endIndex],
159+
tsdocLink,
160+
);
161+
},
162+
});
163+
}
164+
}
165+
166+
// Note: the TSDoc format makes it difficult to extract the range information for the block content specifically.
167+
// Instead, we just report the range for the tag itself.
168+
checkCommentBlock(parsedComment.summarySection);
169+
for (const block of blocksToCheck) {
170+
checkCommentBlock(block.content);
59171
}
60172
}
61173
},
62174
};
63175
},
64176
};
177+
178+
module.exports = rule;

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

Lines changed: 23 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -24,20 +24,34 @@ describe(`Do not allow Markdown links in JSDoc/TSDoc comments (eslint ${eslintVe
2424

2525
it("Should report errors for Markdown links in block comments", async function () {
2626
const result = await lintFile("test.ts");
27-
assert.strictEqual(result.errorCount, 1);
27+
assert.strictEqual(result.errorCount, 2);
2828

29-
const error = result.messages[0];
29+
const error1 = result.messages[0];
3030
assert.strictEqual(
31-
error.message,
31+
error1.message,
3232
"Markdown link syntax (`[text](url)`) is not allowed in JSDoc/TSDoc comments. Use `{@link url|text}` syntax instead.",
3333
);
34-
assert.strictEqual(error.line, 10);
35-
assert.strictEqual(error.column, 51); // 1-based, inclusive
36-
assert.strictEqual(error.endColumn, 75); // 1-based, exclusive
34+
assert.strictEqual(error1.line, 10);
35+
assert.strictEqual(error1.column, 51); // 1-based, inclusive
36+
assert.strictEqual(error1.endColumn, 75); // 1-based, exclusive
3737

3838
// Test auto-fix
39-
assert.notEqual(error.fix, undefined);
40-
assert.deepEqual(error.fix.range, [259, 283]); // 0-based global character index in the file. The start is inclusive, and the end is exclusive.
41-
assert.deepEqual(error.fix.text, "{@link https://bing.com | bing}");
39+
assert.notEqual(error1.fix, undefined);
40+
assert.deepEqual(error1.fix.range, [259, 283]); // 0-based global character index in the file. The start is inclusive, and the end is exclusive.
41+
assert.deepEqual(error1.fix.text, "{@link https://bing.com | bing}");
42+
43+
const error2 = result.messages[1];
44+
assert.strictEqual(
45+
error2.message,
46+
"Markdown link syntax (`[text](url)`) is not allowed in JSDoc/TSDoc comments. Use `{@link url|text}` syntax instead.",
47+
);
48+
assert.strictEqual(error2.line, 11);
49+
assert.strictEqual(error2.column, 23); // 1-based, inclusive
50+
assert.strictEqual(error2.endColumn, 27); // 1-based, exclusive
51+
52+
// Test auto-fix
53+
assert.notEqual(error2.fix, undefined);
54+
assert.deepEqual(error2.fix.range, [307, 311]); // 0-based global character index in the file. The start is inclusive, and the end is exclusive.
55+
assert.deepEqual(error2.fix.text, "{@link }");
4256
});
4357
});

‎common/build/eslint-plugin-fluid/src/test/test-cases/no-markdown-links-in-jsdoc/test.ts‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
// TSDoc comment with a Markdown link should be flagged.
99
/**
1010
* TSDoc comment with link using Markdown syntax: [bing](https://bing.com).
11+
* And an empty link: []().
1112
*/
1213
const tsdocCommentWithMarkdownLink = "invalid";
1314

@@ -37,4 +38,15 @@ const blockCommentWithMarkdownLink = "valid";
3738
// Line comment with link using Markdown syntax: [bing](https://bing.com).
3839
const lineCommentWithMarkdownLink = "valid";
3940

41+
// Link syntax in code blocks should not be flagged
42+
/**
43+
* `[bing](https://bing.com)`
44+
*
45+
* @example
46+
* ```
47+
* [bing](https://bing.com)
48+
* ```
49+
*/
50+
const linkInCodeBlocks = "valid";
51+
4052
// #endregion

0 commit comments

Comments
 (0)