diff --git a/common/build/eslint-plugin-fluid/package.json b/common/build/eslint-plugin-fluid/package.json index d92a7a731165..a5ffcf5e961b 100644 --- a/common/build/eslint-plugin-fluid/package.json +++ b/common/build/eslint-plugin-fluid/package.json @@ -34,6 +34,7 @@ "devDependencies": { "@fluid-tools/markdown-magic": "file:../../../tools/markdown-magic", "@fluidframework/build-common": "^2.0.3", + "@types/eslint": "^9.6.1", "cross-env": "^7.0.3", "eslint": "~9.37.0", "eslint8": "npm:eslint@~8.57.1", diff --git a/common/build/eslint-plugin-fluid/pnpm-lock.yaml b/common/build/eslint-plugin-fluid/pnpm-lock.yaml index b0bef9191f36..0aacae2c6167 100644 --- a/common/build/eslint-plugin-fluid/pnpm-lock.yaml +++ b/common/build/eslint-plugin-fluid/pnpm-lock.yaml @@ -27,6 +27,9 @@ importers: '@fluidframework/build-common': specifier: ^2.0.3 version: 2.0.3 + '@types/eslint': + specifier: ^9.6.1 + version: 9.6.1 cross-env: specifier: ^7.0.3 version: 7.0.3 @@ -195,6 +198,9 @@ packages: '@types/concat-stream@1.6.1': resolution: {integrity: sha512-eHE4cQPoj6ngxBZMvVf6Hw7Mh4jMW4U9lpGmS5GBPB9RYxlFg+CHaVN7ErNY4W9XfLIEn20b4VDYaIrbq0q4uA==} + '@types/eslint@9.6.1': + resolution: {integrity: sha512-FXx2pKgId/WyYo2jXw63kk7/+TY7u7AziEJxJAnSFzHlqTAS3Ync6SvgYAN/k4/PQpnnVuzoMuVnByKK2qp0ag==} + '@types/estree@1.0.8': resolution: {integrity: sha512-dWHzHa2WqEXI/O1E9OjrocMTKJl2mSrEolh1Iomrv6U+JuNwaHXsXx9bLu5gG7BUWFIN0skIQJQ/L1rIex4X6w==} @@ -1860,6 +1866,11 @@ snapshots: dependencies: '@types/node': 20.12.12 + '@types/eslint@9.6.1': + dependencies: + '@types/estree': 1.0.8 + '@types/json-schema': 7.0.15 + '@types/estree@1.0.8': {} '@types/form-data@0.0.33': diff --git a/common/build/eslint-plugin-fluid/src/rules/no-hyphen-after-jsdoc-tag.js b/common/build/eslint-plugin-fluid/src/rules/no-hyphen-after-jsdoc-tag.js index 5967b072ca9c..d0e3cc70de83 100644 --- a/common/build/eslint-plugin-fluid/src/rules/no-hyphen-after-jsdoc-tag.js +++ b/common/build/eslint-plugin-fluid/src/rules/no-hyphen-after-jsdoc-tag.js @@ -3,22 +3,61 @@ * Licensed under the MIT License. */ +//@ts-check +/** + * @typedef {import("eslint").Rule.RuleModule} RuleModule + * @typedef {import('@microsoft/tsdoc').DocNode} DocNode + * @typedef {import('@microsoft/tsdoc').DocPlainText} DocPlainText + */ + +const { DocNodeKind, TSDocParser } = require("@microsoft/tsdoc"); + +const parser = new TSDocParser(); + +/** + * Checks if a comment text starts with a hyphen. + * @param {DocPlainText} plainTextNode - The plain text node to check. + */ +function doesTextNodeStartWithHyphen(plainTextNode) { + return plainTextNode.text.trimStart().startsWith("-"); +} + +/** + * Checks if a comment body starts with a hyphen. + * @param { DocNode } commentBodyNode - The doc node representing the body of the comment. + */ +function doesCommentBodyStartWithHyphen(commentBodyNode) { + // Walk down first node of the tree until we find a leaf. + // If it's plain text, and starts with a hyphen, return true. + // Otherwise, return false. + if (commentBodyNode.kind === DocNodeKind.PlainText) { + return doesTextNodeStartWithHyphen(/** @type {DocPlainText} */ (commentBodyNode)); + } + + const childNodes = commentBodyNode.getChildNodes(); + if (childNodes.length === 0) { + return false; + } + + return doesCommentBodyStartWithHyphen(childNodes[0]); +} + /** * JSDoc/TSDoc tags do not require a hyphen after them. + * @type {RuleModule} */ -module.exports = { +const rule = { meta: { type: "problem", docs: { - description: "Disallow hyphen character immediately following JSDoc tag", + description: "Disallow hyphen character immediately following JSDoc/TSDoc block tag", category: "Best Practices", recommended: false, }, messages: { hyphenAfterTag: - "JSDoc/TSDoc block tags should not be followed by a hyphen character ('-').", + "JSDoc/TSDoc block tags must not be followed by a hyphen character (`-`).", }, - fixable: "code", schema: [], }, @@ -32,33 +71,62 @@ module.exports = { .filter((comment) => comment.type === "Block" && comment.value.startsWith("*")); for (const comment of comments) { - // +2 for the leading "/*", which is omitted by `comment.value`, but included in `comment.range`. - const commentStartIndex = comment.range[0] + 2; - - // Find any JSDoc/TSDoc tags followed by a hyphen - const matches = comment.value.matchAll(/(@[a-zA-Z0-9]+)\s*?-\s*?(.*)/g); - for (const match of matches) { - const [fullMatch, tag, body] = match; - - const startIndex = commentStartIndex + match.index; - const endIndex = startIndex + fullMatch.length; - - context.report({ - loc: { - start: sourceCode.getLocFromIndex(startIndex), - end: sourceCode.getLocFromIndex(endIndex), - }, - messageId: "hyphenAfterTag", - fix(fixer) { - return fixer.replaceTextRange( - [startIndex, endIndex], - `${tag} ${body.trimStart()}`, - ); - }, - }); + if (comment.range === undefined) { + continue; + } + + const commentStartIndex = comment.range[0]; + + // TSDoc parser requires the surrounding "/**" and "*/", but eslint strips those off in `comment.value`. + const parserContext = parser.parseString(`/**${comment.value}*/`); + const parsedComment = parserContext.docComment; + + const blocksToCheck = [ + ...parsedComment.customBlocks, + ...parsedComment.seeBlocks, + ]; + if (parsedComment.remarksBlock) { + blocksToCheck.push(parsedComment.remarksBlock); + } + if (parsedComment.privateRemarks) { + blocksToCheck.push(parsedComment.privateRemarks); + } + if (parsedComment.deprecatedBlock) { + blocksToCheck.push(parsedComment.deprecatedBlock); + } + if (parsedComment.returnsBlock) { + blocksToCheck.push(parsedComment.returnsBlock); + } + + // Note: the TSDoc format makes it difficult to extract the range information for the block content specifically. + // Instead, we just report the range for the tag itself. + for (const block of blocksToCheck) { + if (doesCommentBodyStartWithHyphen(block.content)) { + const tagTextRange = block.blockTag + .getTokenSequence() + .getContainingTextRange(); + const tagTextRangeStart = tagTextRange.pos - 1; // Include the `@` + const tagTextRangeEnd = tagTextRange.end; + const startIndex = sourceCode.getLocFromIndex( + commentStartIndex + tagTextRangeStart, + ); + const endIndex = sourceCode.getLocFromIndex( + commentStartIndex + tagTextRangeEnd, + ); + + context.report({ + loc: { + start: startIndex, + end: endIndex, + }, + messageId: "hyphenAfterTag", + }); + } } } }, }; }, }; + +module.exports = rule; diff --git a/common/build/eslint-plugin-fluid/src/test/no-hyphen-after-jsdoc-tag.test.js b/common/build/eslint-plugin-fluid/src/test/no-hyphen-after-jsdoc-tag.test.js index 79fbca2b5756..8a96dc906904 100644 --- a/common/build/eslint-plugin-fluid/src/test/no-hyphen-after-jsdoc-tag.test.js +++ b/common/build/eslint-plugin-fluid/src/test/no-hyphen-after-jsdoc-tag.test.js @@ -28,7 +28,7 @@ describe(`Do not allow \`-\` following JSDoc/TSDoc tags (eslint ${eslintVersion} } const expectedErrorMessage = - "JSDoc/TSDoc block tags should not be followed by a hyphen character ('-')."; + "JSDoc/TSDoc block tags must not be followed by a hyphen character (`-`)."; it("Should report errors JSDoc/TSDoc tags followed by a hyphen", async function () { const result = await lintFile("test.ts"); @@ -39,32 +39,20 @@ describe(`Do not allow \`-\` following JSDoc/TSDoc tags (eslint ${eslintVersion} assert.strictEqual(error1.message, expectedErrorMessage); assert.strictEqual(error1.line, 8); assert.strictEqual(error1.column, 4); // 1-based, inclusive - assert.strictEqual(error1.endColumn, 37); // 1-based, exclusive - assert(error1.fix !== undefined); - assert.strictEqual(error1.fix.text, "@remarks Here are some remarks."); - assert.deepEqual(error1.fix.range, [226, 259]); // 0-based global character index in the file. The start is inclusive, and the end is exclusive. + assert.strictEqual(error1.endColumn, 13); // 1-based, exclusive // Error 2 const error2 = result.messages[1]; assert.strictEqual(error2.message, expectedErrorMessage); assert.strictEqual(error2.line, 9); assert.strictEqual(error2.column, 4); // 1-based, inclusive - assert.strictEqual(error2.endColumn, 65); // 1-based, exclusive - assert(error2.fix !== undefined); - assert.strictEqual( - error2.fix.text, - "@deprecated This function is deprecated, use something else.", - ); - assert.deepEqual(error2.fix.range, [263, 324]); // 0-based global character index in the file. The start is inclusive, and the end is exclusive. + assert.strictEqual(error2.endColumn, 16); // 1-based, exclusive // Error 3 const error3 = result.messages[2]; assert.strictEqual(error3.message, expectedErrorMessage); assert.strictEqual(error3.line, 10); assert.strictEqual(error3.column, 4); // 1-based, inclusive - assert.strictEqual(error3.endColumn, 39); // 1-based, exclusive - assert(error3.fix !== undefined); - assert.strictEqual(error3.fix.text, "@returns The concatenated string."); - assert.deepEqual(error3.fix.range, [328, 363]); // 0-based global character index in the file. The start is inclusive, and the end is exclusive. + assert.strictEqual(error3.endColumn, 13); // 1-based, exclusive }); }); diff --git a/common/build/eslint-plugin-fluid/src/test/test-cases/no-hyphen-after-jsdoc-tag/test.ts b/common/build/eslint-plugin-fluid/src/test/test-cases/no-hyphen-after-jsdoc-tag/test.ts index e29f6f458522..5cc2e1991490 100644 --- a/common/build/eslint-plugin-fluid/src/test/test-cases/no-hyphen-after-jsdoc-tag/test.ts +++ b/common/build/eslint-plugin-fluid/src/test/test-cases/no-hyphen-after-jsdoc-tag/test.ts @@ -6,7 +6,7 @@ /** * I am a test function with pretty standard docs, but all of my tags have hyphens after them :( * @remarks - Here are some remarks. - * @deprecated- This function is deprecated, use something else. + * @deprecated - This function is deprecated, use something else. * @returns - The concatenated string. */ function invalid(param1: string, param2: T): string { @@ -15,11 +15,19 @@ function invalid(param1: string, param2: T): string { /** * I am a test function with pretty standard docs, and none of my tags have hyphens after them :) + * \@tag - Escaped tags should be valid since the @ is escaped. + * `@tag - in a code block` - This should also be valid. + * I also have a {@link @foo/bar | link} - This should not trigger the rule. * @remarks Here are some remarks. * @deprecated This function is deprecated, use something else. * @returns The concatenated string. * @param param1 - I am a param comment. Since my hyphen follows the param name, this is valid. * @typeParam T - I am a type param comment. I am also valid. + * @example + * An example that should be valid: + * ``` + * @foo - Bar (this should be ignored by the rule, since it occurs within a fenced code block and therefore cannot be an actual tag) + * ``` */ function valid(param1: string, param2: T): string { return `${param1} - ${param2}`;