From e8babf444394865bfbf3e4d0fbf58f6839694268 Mon Sep 17 00:00:00 2001 From: "Nicholas C. Zakas" Date: Fri, 13 Sep 2024 15:19:27 -0400 Subject: [PATCH 1/7] feat: Add disable comment support --- README.md | 15 +- package.json | 2 +- src/language/markdown-source-code.js | 185 +++++++++++++++++++- src/rules/no-html.js | 56 +++--- tests/language/markdown-source-code.test.js | 107 ++++++++++- tests/plugin.test.js | 84 +++++++++ tests/rules/no-html.test.js | 42 +++++ 7 files changed, 467 insertions(+), 24 deletions(-) diff --git a/README.md b/README.md index 8484ee3e..a3b09585 100644 --- a/README.md +++ b/README.md @@ -17,7 +17,7 @@ Lint JS, JSX, TypeScript, and more inside Markdown. ### Installing -Install the plugin alongside ESLint v8 or greater: +Install the plugin alongside ESLint v9 or greater: ```sh npm install --save-dev eslint @eslint/markdown @@ -80,6 +80,19 @@ export default [ ]; ``` +You can individually disable rules in Markdown using HTML comments, such as: + +```markdown + +Hello world! + + +Goodbye world! + + +[Object] +``` + ### Languages | **Language Name** | **Description** | diff --git a/package.json b/package.json index 6e664473..a12c71aa 100644 --- a/package.json +++ b/package.json @@ -65,7 +65,7 @@ "c8": "^10.1.2", "chai": "^5.1.1", "dedent": "^1.5.3", - "eslint": "^9.8.0", + "eslint": "^9.10.0", "eslint-config-eslint": "^11.0.0", "globals": "^15.1.0", "lint-staged": "^15.2.9", diff --git a/src/language/markdown-source-code.js b/src/language/markdown-source-code.js index d57f6f93..cc8049a6 100644 --- a/src/language/markdown-source-code.js +++ b/src/language/markdown-source-code.js @@ -7,7 +7,13 @@ // Imports //----------------------------------------------------------------------------- -import { VisitNodeStep, TextSourceCodeBase } from "@eslint/plugin-kit"; +import { + VisitNodeStep, + TextSourceCodeBase, + ConfigCommentParser, + Directive, +} from "@eslint/plugin-kit"; +import { findOffsets } from "../util.js"; //----------------------------------------------------------------------------- // Types @@ -15,6 +21,7 @@ import { VisitNodeStep, TextSourceCodeBase } from "@eslint/plugin-kit"; /** @typedef {import("mdast").Root} RootNode */ /** @typedef {import("mdast").Node} MarkdownNode */ +/** @typedef {import("mdast").Html} HTMLNode */ /** @typedef {import("@eslint/core").Language} Language */ /** @typedef {import("@eslint/core").File} File */ /** @typedef {import("@eslint/core").TraversalStep} TraversalStep */ @@ -23,6 +30,104 @@ import { VisitNodeStep, TextSourceCodeBase } from "@eslint/plugin-kit"; /** @typedef {import("@eslint/core").ParseResult} ParseResult */ /** @typedef {import("@eslint/core").SourceLocation} SourceLocation */ /** @typedef {import("@eslint/core").SourceRange} SourceRange */ +/** @typedef {import("@eslint/core").FileProblem} FileProblem */ +/** @typedef {import("@eslint/core").DirectiveType} DirectiveType */ + +//----------------------------------------------------------------------------- +// Helpers +//----------------------------------------------------------------------------- + +const commentParser = new ConfigCommentParser(); +const configCommentStart = + //gu; + +/** + * Represents an inline config comment in the source code. + */ +class InlineConfigComment { + /** + * The comment text. + * @type {string} + */ + value; + + /** + * The position of the comment in the source code. + * @type {SourceLocation} + */ + position; + + /** + * Creates a new instance. + * @param {Object} options The options for the instance. + * @param {string} options.value The comment text. + * @param {SourceLocation} options.position The position of the comment in the source code. + */ + constructor({ value, position }) { + this.value = value.trim(); + this.position = position; + } +} + +/** + * Extracts inline configuration comments from an HTML node. + * @param {HTMLNode} node The HTML node to extract comments from. + * @returns {Array} The inline configuration comments found in the node. + */ +function extractInlineConfigCommentsFromHTML(node) { + if (!configCommentStart.test(node.value)) { + return []; + } + const comments = []; + + let match; + + while ((match = htmlComment.exec(node.value))) { + if (configCommentStart.test(match[0])) { + const comment = match[0]; + + // calculate location of the comment inside the node + const start = { + ...node.position.start, + }; + + const end = { + ...node.position.start, + }; + + const { + lineOffset: startLineOffset, + columnOffset: startColumnOffset, + } = findOffsets(node.value, match.index); + + start.line += startLineOffset; + start.column += startColumnOffset; + start.offset += match.index; + + const commentLineCount = comment.split("\n").length - 1; + + end.line = start.line + commentLineCount; + end.column = + commentLineCount === 0 + ? start.column + comment.length + : comment.length - comment.lastIndexOf("\n") - 1; + end.offset = start.offset + comment.length; + + comments.push( + new InlineConfigComment({ + value: match[1], + position: { + start, + end, + }, + }), + ); + } + } + + return comments; +} //----------------------------------------------------------------------------- // Exports @@ -44,6 +149,18 @@ export class MarkdownSourceCode extends TextSourceCodeBase { */ #parents = new WeakMap(); + /** + * Collection of HTML nodes. Used to find directive comments. + * @type {Array} + */ + #htmlNodes = []; + + /** + * Collection of inline configuration comments. + * @type {Array} + */ + #inlineConfigComments = []; + /** * The AST of the source code. * @type {RootNode} @@ -59,6 +176,9 @@ export class MarkdownSourceCode extends TextSourceCodeBase { constructor({ text, ast }) { super({ ast, text }); this.ast = ast; + + // need to traverse the source code to get the inline config nodes + this.traverse(); } /** @@ -70,6 +190,64 @@ export class MarkdownSourceCode extends TextSourceCodeBase { return this.#parents.get(node); } + /** + * Returns an array of all inline configuration nodes found in the + * source code. + * @returns {Array} An array of all inline configuration nodes. + */ + getInlineConfigNodes() { + if (this.#inlineConfigComments.length === 0) { + this.#inlineConfigComments = this.#htmlNodes.flatMap( + extractInlineConfigCommentsFromHTML, + ); + } + + return this.#inlineConfigComments; + } + + /** + * Returns an all directive nodes that enable or disable rules along with any problems + * encountered while parsing the directives. + * @returns {{problems:Array,directives:Array}} Information + * that ESLint needs to further process the directives. + */ + getDisableDirectives() { + const problems = []; + const directives = []; + + this.getInlineConfigNodes().forEach(comment => { + // Step 1: Parse the directive + const { + label, + value, + justification: justificationPart, + } = commentParser.parseDirective(comment.value); + + // Step 2: Extract the directive value and create the Directive object + switch (label) { + case "eslint-disable": + case "eslint-enable": + case "eslint-disable-next-line": + case "eslint-disable-line": { + const directiveType = label.slice("eslint-".length); + + directives.push( + new Directive({ + type: /** @type {DirectiveType} */ (directiveType), + node: comment, + value, + justification: justificationPart, + }), + ); + } + + // no default + } + }); + + return { problems, directives }; + } + /** * Traverse the source code and return the steps that were taken. * @returns {Iterable} The steps that were taken while traversing the source code. @@ -96,6 +274,11 @@ export class MarkdownSourceCode extends TextSourceCodeBase { }), ); + // save HTML nodes + if (node.type === "html") { + this.#htmlNodes.push(node); + } + // then visit the children if (node.children) { node.children.forEach(child => { diff --git a/src/rules/no-html.js b/src/rules/no-html.js index 9a858da8..450a2a95 100644 --- a/src/rules/no-html.js +++ b/src/rules/no-html.js @@ -3,12 +3,24 @@ * @author Nicholas C. Zakas */ +//----------------------------------------------------------------------------- +// Imports +//----------------------------------------------------------------------------- + +import { findOffsets } from "../util.js"; + //----------------------------------------------------------------------------- // Type Definitions //----------------------------------------------------------------------------- /** @typedef {import("eslint").Rule.RuleModule} RuleModule */ +//----------------------------------------------------------------------------- +// Helpers +//----------------------------------------------------------------------------- + +const htmlTagPattern = /<([a-z0-9]+(?:-[a-z0-9]+)*)/giu; + //----------------------------------------------------------------------------- // Rule Definition //----------------------------------------------------------------------------- @@ -48,28 +60,32 @@ export default { return { html(node) { - // don't care about closing tags - if (node.value.startsWith(" + +This is a paragraph with an inline config comment. + +Something something`; const ast = fromMarkdown(markdownText); @@ -78,6 +86,77 @@ describe("MarkdownSourceCode", () => { }); }); + describe("getInlineConfigNodes()", () => { + it("should return the inline config nodes", () => { + const nodes = sourceCode.getInlineConfigNodes(); + assert.strictEqual(nodes.length, 4); + + /* eslint-disable no-restricted-properties -- Needed to avoid extra asserts. */ + + assert.deepEqual(nodes[0], { + value: "eslint-disable-next-line no-console", + position: { + start: { line: 13, column: 1, offset: 140 }, + end: { line: 13, column: 45, offset: 184 }, + }, + }); + + assert.deepEqual(nodes[1], { + value: "eslint-disable-line no-console", + position: { + start: { line: 15, column: 52, offset: 237 }, + end: { line: 15, column: 91, offset: 276 }, + }, + }); + + assert.deepEqual(nodes[2], { + value: "eslint-enable no-console -- ok to use console here", + position: { + start: { line: 17, column: 1, offset: 278 }, + end: { line: 19, column: 3, offset: 337 }, + }, + }); + + assert.deepEqual(nodes[3], { + value: "eslint-disable semi", + position: { + start: { line: 19, column: 23, offset: 356 }, + end: { line: 19, column: 51, offset: 384 }, + }, + }); + + /* eslint-enable no-restricted-properties -- Needed to avoid extra asserts. */ + }); + }); + + describe("getDisableDirectives()", () => { + it("should return the disable directives", () => { + const { problems, directives } = sourceCode.getDisableDirectives(); + + assert.strictEqual(problems.length, 0); + assert.strictEqual(directives.length, 4); + + assert.strictEqual(directives[0].type, "disable-next-line"); + assert.strictEqual(directives[0].value, "no-console"); + assert.strictEqual(directives[0].justification, ""); + + assert.strictEqual(directives[1].type, "disable-line"); + assert.strictEqual(directives[1].value, "no-console"); + assert.strictEqual(directives[1].justification, ""); + + assert.strictEqual(directives[2].type, "enable"); + assert.strictEqual(directives[2].value, "no-console"); + assert.strictEqual( + directives[2].justification, + "ok to use console here", + ); + + assert.strictEqual(directives[3].type, "disable"); + assert.strictEqual(directives[3].value, "semi"); + assert.strictEqual(directives[3].justification, ""); + }); + }); + describe("traverse()", () => { it("should traverse the AST", () => { const steps = sourceCode.traverse(); @@ -113,6 +192,32 @@ describe("MarkdownSourceCode", () => { [1, "text", " paragraph."], [2, "text", " paragraph."], [2, "paragraph", void 0], + [1, "html", ""], + [2, "html", ""], + [1, "paragraph", void 0], + [ + 1, + "text", + "This is a paragraph with an inline config comment. ", + ], + [ + 2, + "text", + "This is a paragraph with an inline config comment. ", + ], + [1, "html", ""], + [2, "html", ""], + [2, "paragraph", void 0], + [ + 1, + "html", + "Something something", + ], + [ + 2, + "html", + "Something something", + ], [2, "root", void 0], ]); }); diff --git a/tests/plugin.test.js b/tests/plugin.test.js index 4848a52b..ad6d0897 100644 --- a/tests/plugin.test.js +++ b/tests/plugin.test.js @@ -2263,4 +2263,88 @@ describe("FlatESLint", () => { }); }); }); + + describe("Configuration Comments", () => { + const config = { + files: ["*.md"], + plugins: { + markdown: plugin, + }, + language: "markdown/commonmark", + rules: { + "markdown/no-html": "error", + }, + }; + + let eslint; + + beforeEach(() => { + eslint = new ESLint({ + overrideConfigFile: true, + overrideConfig: config, + }); + }); + + it("should report html without any configuration comments present", async () => { + const code = "Hello world"; + const results = await eslint.lintText(code, { + filePath: "test.md", + }); + + assert.strictEqual(results.length, 1); + assert.strictEqual(results[0].messages.length, 1); + assert.strictEqual( + results[0].messages[0].message, + 'HTML element "b" is not allowed.', + ); + }); + + it("should report html when a disable configuration comment is present and followed by an enable configuration comment", async () => { + const code = + "Hello worldGoodbye"; + const results = await eslint.lintText(code, { + filePath: "test.md", + }); + + assert.strictEqual(results.length, 1); + assert.strictEqual(results[0].messages.length, 1); + assert.strictEqual( + results[0].messages[0].message, + 'HTML element "i" is not allowed.', + ); + }); + + it("should not report html when a disable configuration comment is present", async () => { + const code = + "\nHello world"; + const results = await eslint.lintText(code, { + filePath: "test.md", + }); + + assert.strictEqual(results.length, 1); + assert.strictEqual(results[0].messages.length, 0); + }); + + it("should not report html when a disable-line configuration comment is present", async () => { + const code = + "Hello world"; + const results = await eslint.lintText(code, { + filePath: "test.md", + }); + + assert.strictEqual(results.length, 1); + assert.strictEqual(results[0].messages.length, 0); + }); + + it("should not report html when a disable-next-line configuration comment is present", async () => { + const code = + "\nHello world"; + const results = await eslint.lintText(code, { + filePath: "test.md", + }); + + assert.strictEqual(results.length, 1); + assert.strictEqual(results[0].messages.length, 0); + }); + }); }); diff --git a/tests/rules/no-html.test.js b/tests/rules/no-html.test.js index 4ed85a9b..8f396c80 100644 --- a/tests/rules/no-html.test.js +++ b/tests/rules/no-html.test.js @@ -72,6 +72,48 @@ ruleTester.run("no-html", rule, { }, ], }, + { + code: "Hello world!Goodbye world!", + options: [{ allowed: ["em"] }], + errors: [ + { + messageId: "disallowedElement", + line: 1, + column: 1, + endLine: 1, + endColumn: 4, + data: { + name: "b", + }, + }, + { + messageId: "disallowedElement", + line: 1, + column: 20, + endLine: 1, + endColumn: 23, + data: { + name: "i", + }, + }, + ], + }, + { + code: "Hello world!", + options: [{ allowed: ["em"] }], + errors: [ + { + messageId: "disallowedElement", + line: 1, + column: 12, + endLine: 1, + endColumn: 15, + data: { + name: "b", + }, + }, + ], + }, { code: "Hello world!", options: [{ allowed: ["em"] }], From b0bc353e901ef65ee70b79ed8ce75a9d0860d584 Mon Sep 17 00:00:00 2001 From: "Nicholas C. Zakas" Date: Tue, 17 Sep 2024 12:17:05 -0400 Subject: [PATCH 2/7] Check for multiline eslint-disable-line comments --- src/language/markdown-source-code.js | 17 ++++++++++- tests/language/markdown-source-code.test.js | 31 +++++++++++++++++++-- 2 files changed, 44 insertions(+), 4 deletions(-) diff --git a/src/language/markdown-source-code.js b/src/language/markdown-source-code.js index cc8049a6..94b68b6e 100644 --- a/src/language/markdown-source-code.js +++ b/src/language/markdown-source-code.js @@ -223,7 +223,22 @@ export class MarkdownSourceCode extends TextSourceCodeBase { justification: justificationPart, } = commentParser.parseDirective(comment.value); - // Step 2: Extract the directive value and create the Directive object + // Step 2: Validate the directive does not span multiple lines + if ( + label === "eslint-disable-line" && + comment.position.start.line !== comment.position.end.line + ) { + const message = `${label} comment should not span multiple lines.`; + + problems.push({ + ruleId: null, + message, + loc: comment.position, + }); + return; + } + + // Step 3: Extract the directive value and create the Directive object switch (label) { case "eslint-disable": case "eslint-enable": diff --git a/tests/language/markdown-source-code.test.js b/tests/language/markdown-source-code.test.js index 620288cc..8b89d00b 100644 --- a/tests/language/markdown-source-code.test.js +++ b/tests/language/markdown-source-code.test.js @@ -33,7 +33,11 @@ This is a paragraph with an inline config comment. Something something`; +-->Something something + +`; const ast = fromMarkdown(markdownText); @@ -89,7 +93,7 @@ describe("MarkdownSourceCode", () => { describe("getInlineConfigNodes()", () => { it("should return the inline config nodes", () => { const nodes = sourceCode.getInlineConfigNodes(); - assert.strictEqual(nodes.length, 4); + assert.strictEqual(nodes.length, 5); /* eslint-disable no-restricted-properties -- Needed to avoid extra asserts. */ @@ -125,6 +129,14 @@ describe("MarkdownSourceCode", () => { }, }); + assert.deepEqual(nodes[4], { + value: "eslint-disable-line no-console", + position: { + start: { line: 21, column: 1, offset: 386 }, + end: { line: 23, column: 4, offset: 427 }, + }, + }); + /* eslint-enable no-restricted-properties -- Needed to avoid extra asserts. */ }); }); @@ -133,7 +145,18 @@ describe("MarkdownSourceCode", () => { it("should return the disable directives", () => { const { problems, directives } = sourceCode.getDisableDirectives(); - assert.strictEqual(problems.length, 0); + assert.strictEqual(problems.length, 1); + + assert.strictEqual(problems[0].ruleId, null); + assert.strictEqual( + problems[0].message, + "eslint-disable-line comment should not span multiple lines.", + ); + assert.deepStrictEqual(problems[0].loc, { + start: { line: 21, column: 1, offset: 386 }, + end: { line: 23, column: 4, offset: 427 }, + }); + assert.strictEqual(directives.length, 4); assert.strictEqual(directives[0].type, "disable-next-line"); @@ -218,6 +241,8 @@ describe("MarkdownSourceCode", () => { "html", "Something something", ], + [1, "html", ""], + [2, "html", ""], [2, "root", void 0], ]); }); From d97eddd61c6f6b184210c68edbe23fb063accf14 Mon Sep 17 00:00:00 2001 From: "Nicholas C. Zakas" Date: Thu, 26 Sep 2024 14:18:24 -0400 Subject: [PATCH 3/7] Clean up regex --- src/language/markdown-source-code.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/language/markdown-source-code.js b/src/language/markdown-source-code.js index 94b68b6e..558c2781 100644 --- a/src/language/markdown-source-code.js +++ b/src/language/markdown-source-code.js @@ -40,7 +40,7 @@ import { findOffsets } from "../util.js"; const commentParser = new ConfigCommentParser(); const configCommentStart = //gu; +const htmlComment = //gsu; /** * Represents an inline config comment in the source code. @@ -116,7 +116,7 @@ function extractInlineConfigCommentsFromHTML(node) { comments.push( new InlineConfigComment({ - value: match[1], + value: match[1].trim(), position: { start, end, From 4c767f90f62294f382134d1503a5465a8bc7d534 Mon Sep 17 00:00:00 2001 From: "Nicholas C. Zakas" Date: Fri, 27 Sep 2024 10:31:15 -0400 Subject: [PATCH 4/7] Update README.md Co-authored-by: Milos Djermanovic --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index a3b09585..040e00e1 100644 --- a/README.md +++ b/README.md @@ -90,7 +90,7 @@ You can individually disable rules in Markdown using HTML comments, such as: Goodbye world! -[Object] +[Object] ``` ### Languages From 2021755e295f33b6777269153aedadf00bad95f0 Mon Sep 17 00:00:00 2001 From: "Nicholas C. Zakas" Date: Fri, 27 Sep 2024 10:31:35 -0400 Subject: [PATCH 5/7] Update src/language/markdown-source-code.js Co-authored-by: Milos Djermanovic --- src/language/markdown-source-code.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/language/markdown-source-code.js b/src/language/markdown-source-code.js index 558c2781..7ff11a8e 100644 --- a/src/language/markdown-source-code.js +++ b/src/language/markdown-source-code.js @@ -39,7 +39,7 @@ import { findOffsets } from "../util.js"; const commentParser = new ConfigCommentParser(); const configCommentStart = - /)/u; const htmlComment = //gsu; /** From c3fe8e56e7b92e17af07eda549655d90473722a5 Mon Sep 17 00:00:00 2001 From: "Nicholas C. Zakas" Date: Fri, 27 Sep 2024 10:34:16 -0400 Subject: [PATCH 6/7] Don't initialize inlineConfigComments with array --- src/language/markdown-source-code.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/language/markdown-source-code.js b/src/language/markdown-source-code.js index 7ff11a8e..978fe96a 100644 --- a/src/language/markdown-source-code.js +++ b/src/language/markdown-source-code.js @@ -159,7 +159,7 @@ export class MarkdownSourceCode extends TextSourceCodeBase { * Collection of inline configuration comments. * @type {Array} */ - #inlineConfigComments = []; + #inlineConfigComments; /** * The AST of the source code. @@ -196,7 +196,7 @@ export class MarkdownSourceCode extends TextSourceCodeBase { * @returns {Array} An array of all inline configuration nodes. */ getInlineConfigNodes() { - if (this.#inlineConfigComments.length === 0) { + if (!this.#inlineConfigComments) { this.#inlineConfigComments = this.#htmlNodes.flatMap( extractInlineConfigCommentsFromHTML, ); From faa17b45f02e8772eaeb97276ebe1a140b2a0846 Mon Sep 17 00:00:00 2001 From: "Nicholas C. Zakas" Date: Fri, 27 Sep 2024 10:38:21 -0400 Subject: [PATCH 7/7] Fix column calculation for inline config nodes --- src/language/markdown-source-code.js | 2 +- tests/language/markdown-source-code.test.js | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/language/markdown-source-code.js b/src/language/markdown-source-code.js index 978fe96a..87322d70 100644 --- a/src/language/markdown-source-code.js +++ b/src/language/markdown-source-code.js @@ -111,7 +111,7 @@ function extractInlineConfigCommentsFromHTML(node) { end.column = commentLineCount === 0 ? start.column + comment.length - : comment.length - comment.lastIndexOf("\n") - 1; + : comment.length - comment.lastIndexOf("\n"); end.offset = start.offset + comment.length; comments.push( diff --git a/tests/language/markdown-source-code.test.js b/tests/language/markdown-source-code.test.js index 8b89d00b..e06fdf7f 100644 --- a/tests/language/markdown-source-code.test.js +++ b/tests/language/markdown-source-code.test.js @@ -117,7 +117,7 @@ describe("MarkdownSourceCode", () => { value: "eslint-enable no-console -- ok to use console here", position: { start: { line: 17, column: 1, offset: 278 }, - end: { line: 19, column: 3, offset: 337 }, + end: { line: 19, column: 4, offset: 337 }, }, }); @@ -133,7 +133,7 @@ describe("MarkdownSourceCode", () => { value: "eslint-disable-line no-console", position: { start: { line: 21, column: 1, offset: 386 }, - end: { line: 23, column: 4, offset: 427 }, + end: { line: 23, column: 5, offset: 427 }, }, }); @@ -154,7 +154,7 @@ describe("MarkdownSourceCode", () => { ); assert.deepStrictEqual(problems[0].loc, { start: { line: 21, column: 1, offset: 386 }, - end: { line: 23, column: 4, offset: 427 }, + end: { line: 23, column: 5, offset: 427 }, }); assert.strictEqual(directives.length, 4);