From 336f3926e1ec2213599e67dc7a69d0b1d938205a Mon Sep 17 00:00:00 2001 From: Josh Goldberg Date: Sun, 23 May 2021 05:39:09 -0400 Subject: [PATCH 01/31] New: Fixable disable directives --- docs/developer-guide/nodejs-api.md | 2 +- lib/cli-engine/cli-engine.js | 26 +- lib/eslint/eslint.js | 4 +- lib/linter/apply-disable-directives.js | 83 +++- lib/linter/linter.js | 3 +- lib/options.js | 2 +- lib/shared/types.js | 2 +- tests/lib/cli-engine/cli-engine.js | 8 +- tests/lib/eslint/eslint.js | 10 +- tests/lib/linter/apply-disable-directives.js | 395 +++++++++++++------ tests/lib/linter/linter.js | 16 + 11 files changed, 407 insertions(+), 144 deletions(-) diff --git a/docs/developer-guide/nodejs-api.md b/docs/developer-guide/nodejs-api.md index f1e21584e32..28311312461 100644 --- a/docs/developer-guide/nodejs-api.md +++ b/docs/developer-guide/nodejs-api.md @@ -147,7 +147,7 @@ The `ESLint` constructor takes an `options` object. If you omit the `options` ob * `options.fix` (`boolean | (message: LintMessage) => boolean`)
Default is `false`. If `true` is present, the [`eslint.lintFiles()`][eslint-lintfiles] and [`eslint.lintText()`][eslint-linttext] methods work in autofix mode. If a predicate function is present, the methods pass each lint message to the function, then use only the lint messages for which the function returned `true`. -* `options.fixTypes` (`("problem" | "suggestion" | "layout")[] | null`)
+* `options.fixTypes` (`("directive" | "problem" | "suggestion" | "layout")[] | null`)
Default is `null`. The types of the rules that the [`eslint.lintFiles()`][eslint-lintfiles] and [`eslint.lintText()`][eslint-linttext] methods use for autofix. ##### Cache-related diff --git a/lib/cli-engine/cli-engine.js b/lib/cli-engine/cli-engine.js index ca298f9c356..c6d84d2c6f8 100644 --- a/lib/cli-engine/cli-engine.js +++ b/lib/cli-engine/cli-engine.js @@ -41,7 +41,7 @@ const hash = require("./hash"); const LintResultCache = require("./lint-result-cache"); const debug = require("debug")("eslint:cli-engine"); -const validFixTypes = new Set(["problem", "suggestion", "layout"]); +const validFixTypes = new Set(["directive", "problem", "suggestion", "layout"]); //------------------------------------------------------------------------------ // Typedefs @@ -325,6 +325,23 @@ function getRule(ruleId, configArrays) { return builtInRules.get(ruleId) || null; } +/** + * Checks whether a message's rule type should be fixed. + * @param {LintMessage} message The message to check. + * @param {ConfigArray[]} lastConfigArrays The list of config arrays that the last `executeOnFiles` or `executeOnText` used. + * @param {string[]} fixTypes An array of fix types to check. + * @returns {boolean} Whether the message should be fixed. + */ +function shouldMessageBeFixed(message, lastConfigArrays, fixTypes) { + if (!message.ruleId) { + return fixTypes.has("directive"); + } + + const rule = message.ruleId && getRule(message.ruleId, lastConfigArrays); + + return rule && rule.meta && fixTypes.has(rule.meta.type); +} + /** * Collect used deprecated rules. * @param {ConfigArray[]} usedConfigArrays The config arrays which were used. @@ -617,12 +634,7 @@ class CLIEngine { const originalFix = (typeof options.fix === "function") ? options.fix : () => true; - options.fix = message => { - const rule = message.ruleId && getRule(message.ruleId, lastConfigArrays); - const matches = rule && rule.meta && fixTypes.has(rule.meta.type); - - return matches && originalFix(message); - }; + options.fix = message => shouldMessageBeFixed(message, lastConfigArrays, fixTypes) && originalFix(message); } } diff --git a/lib/eslint/eslint.js b/lib/eslint/eslint.js index ae2d2100861..5e66546d157 100644 --- a/lib/eslint/eslint.js +++ b/lib/eslint/eslint.js @@ -125,7 +125,7 @@ function isArrayOfNonEmptyString(x) { * @returns {boolean} `true` if `x` is valid fix type. */ function isFixType(x) { - return x === "problem" || x === "suggestion" || x === "layout"; + return x === "directive" || x === "problem" || x === "suggestion" || x === "layout"; } /** @@ -237,7 +237,7 @@ function processOptions({ errors.push("'fix' must be a boolean or a function."); } if (fixTypes !== null && !isFixTypeArray(fixTypes)) { - errors.push("'fixTypes' must be an array of any of \"problem\", \"suggestion\", and \"layout\"."); + errors.push("'fixTypes' must be an array of any of \"directive\", \"problem\", \"suggestion\", and \"layout\"."); } if (typeof globInputPaths !== "boolean") { errors.push("'globInputPaths' must be a boolean."); diff --git a/lib/linter/apply-disable-directives.js b/lib/linter/apply-disable-directives.js index 41d6934abba..774184762ee 100644 --- a/lib/linter/apply-disable-directives.js +++ b/lib/linter/apply-disable-directives.js @@ -18,6 +18,76 @@ function compareLocations(itemA, itemB) { return itemA.line - itemB.line || itemA.column - itemB.column; } +/** + * Returns whether a comment is alone on its line. + * @param {Comment} comment The comment node which will be deleted. + * @param {string} line Contents of the comment's line. + * @returns {boolean} Whether the comment is alone on its line. + */ +function commentIsAloneOnLine(comment, line) { + return comment.type === "Block" ? line.length === comment.value.length + "/**/".length : line.startsWith("//"); +} + +/** + * Finds where to remove text for an unused directive. + * @param {Directive} directive Unused directive to be removed. + * @param {SourceCode} sourceCode A SourceCode object for the given text + * @returns {Range} Removal range for the unused directive. + * @see https://github.com/eslint/rfcs/pull/78 + */ +function getDirectiveRemovalRange(directive, sourceCode) { + const lineIndex = directive.unprocessedDirective.line - 1; + const lineStart = sourceCode.lineStartIndices[lineIndex]; + const commentStart = lineStart + directive.unprocessedDirective.column - 1; + const comment = sourceCode.getTokenByRangeStart(commentStart, { includeComments: true }); + const line = sourceCode.lines[lineIndex]; + + // If the directive's rule isn't the only one in the comment, only remove that rule + if (comment.value.includes(",")) { + const ruleStart = comment.value.indexOf(directive.ruleId); + const ruleEnd = ruleStart + directive.ruleId.length; + + if (comment.value.trimRight().endsWith(directive.ruleId)) { + for (const prefix of [", ", ","]) { + if (comment.value.slice(ruleStart - prefix.length, ruleStart) === prefix) { + return [commentStart + 2 + ruleStart - prefix.length, commentStart + ruleEnd + 2]; + } + } + } else { + for (const suffix of [", ", ","]) { + if (comment.value.slice(ruleEnd, ruleEnd + suffix.length) === suffix) { + return [commentStart + 2 + ruleStart, commentStart + 2 + ruleEnd + suffix.length]; + } + } + } + + return [commentStart + 2 + ruleStart, commentStart + 2 + ruleEnd]; + } + + // If the comment is alone on its line, remove the entire line + if (commentIsAloneOnLine(comment, line)) { + return [ + lineStart, + lineIndex === sourceCode.lines.length - 1 + ? lineStart + line.length + : sourceCode.lineStartIndices[lineIndex + 1] + ]; + } + + // If the comment has space between it and whatever else is on its line, collapse the space + if (commentStart !== 0 && ["\n", " "].includes(sourceCode.text[commentStart - 1])) { + if (comment.range[1] === sourceCode.text.length) { + return [comment.range[0] - 1, comment.range[1]]; + } + + if (sourceCode.text[comment.range[1]] === " ") { + return [comment.range[0], comment.range[1] + 1]; + } + } + + return comment.range; +} + /** * This is the same as the exported function, except that it * doesn't handle disable-line and disable-next-line directives, and it always reports unused @@ -87,6 +157,10 @@ function applyDirectives(options) { const unusedDisableDirectives = options.directives .filter(directive => directive.type === "disable" && !usedDisableDirectives.has(directive)) .map(directive => ({ + fix: { + range: getDirectiveRemovalRange(directive, options.sourceCode), + text: "" + }, ruleId: null, message: directive.ruleId ? `Unused eslint-disable directive (no problems were reported from '${directive.ruleId}').` @@ -115,10 +189,11 @@ function applyDirectives(options) { * @param {{ruleId: (string|null), line: number, column: number}[]} options.problems * A list of problems reported by rules, sorted by increasing location in the file, with one-based columns. * @param {"off" | "warn" | "error"} options.reportUnusedDisableDirectives If `"warn"` or `"error"`, adds additional problems for unused directives + * @param {SourceCode} options.sourceCode A SourceCode object for the given text * @returns {{ruleId: (string|null), line: number, column: number}[]} * A list of reported problems that were not disabled by the directive comments. */ -module.exports = ({ directives, problems, reportUnusedDisableDirectives = "off" }) => { +module.exports = ({ directives, problems, reportUnusedDisableDirectives = "off", sourceCode }) => { const blockDirectives = directives .filter(directive => directive.type === "disable" || directive.type === "enable") .map(directive => Object.assign({}, directive, { unprocessedDirective: directive })) @@ -150,12 +225,14 @@ module.exports = ({ directives, problems, reportUnusedDisableDirectives = "off" const blockDirectivesResult = applyDirectives({ problems, directives: blockDirectives, - reportUnusedDisableDirectives + reportUnusedDisableDirectives, + sourceCode }); const lineDirectivesResult = applyDirectives({ problems: blockDirectivesResult.problems, directives: lineDirectives, - reportUnusedDisableDirectives + reportUnusedDisableDirectives, + sourceCode }); return reportUnusedDisableDirectives !== "off" diff --git a/lib/linter/linter.js b/lib/linter/linter.js index 576816b5b7b..58e2d692a30 100644 --- a/lib/linter/linter.js +++ b/lib/linter/linter.js @@ -1204,7 +1204,8 @@ class Linter { problems: lintingProblems .concat(commentDirectives.problems) .sort((problemA, problemB) => problemA.line - problemB.line || problemA.column - problemB.column), - reportUnusedDisableDirectives: options.reportUnusedDisableDirectives + reportUnusedDisableDirectives: options.reportUnusedDisableDirectives, + sourceCode }); } diff --git a/lib/options.js b/lib/options.js index 92c140bd3c8..f3b419874dc 100644 --- a/lib/options.js +++ b/lib/options.js @@ -32,7 +32,7 @@ const optionator = require("optionator"); * @property {string[]} [ext] Specify JavaScript file extensions * @property {boolean} fix Automatically fix problems * @property {boolean} fixDryRun Automatically fix problems without saving the changes to the file system - * @property {("problem" | "suggestion" | "layout")[]} [fixType] Specify the types of fixes to apply (problem, suggestion, layout) + * @property {("directive" | "problem" | "suggestion" | "layout")[]} [fixType] Specify the types of fixes to apply (directive, problem, suggestion, layout) * @property {string} format Use a specific output format * @property {string[]} [global] Define global variables * @property {boolean} [help] Show help diff --git a/lib/shared/types.js b/lib/shared/types.js index c3b76e42d5f..d3bf184d030 100644 --- a/lib/shared/types.js +++ b/lib/shared/types.js @@ -125,7 +125,7 @@ module.exports = {}; * @property {Record} [messages] The messages the rule reports. * @property {string[]} [replacedBy] The IDs of the alternative rules. * @property {Array|Object} schema The option schema of the rule. - * @property {"problem"|"suggestion"|"layout"} type The rule type. + * @property {"directive"|"problem"|"suggestion"|"layout"} type The rule type. */ /** diff --git a/tests/lib/cli-engine/cli-engine.js b/tests/lib/cli-engine/cli-engine.js index 59243b0b7dc..f3ffdeccb68 100644 --- a/tests/lib/cli-engine/cli-engine.js +++ b/tests/lib/cli-engine/cli-engine.js @@ -5071,20 +5071,24 @@ describe("CLIEngine", () => { message: "Unused eslint-disable directive (no problems were reported).", line: 1, column: 1, + fix: { + range: [0, 20], + text: "" + }, severity: 2, nodeType: null } ], errorCount: 1, warningCount: 0, - fixableErrorCount: 0, + fixableErrorCount: 1, fixableWarningCount: 0, source: "/* eslint-disable */" } ], errorCount: 1, warningCount: 0, - fixableErrorCount: 0, + fixableErrorCount: 1, fixableWarningCount: 0, usedDeprecatedRules: [] } diff --git a/tests/lib/eslint/eslint.js b/tests/lib/eslint/eslint.js index c7783a6c059..75cf6fa0493 100644 --- a/tests/lib/eslint/eslint.js +++ b/tests/lib/eslint/eslint.js @@ -198,7 +198,7 @@ describe("ESLint", () => { "- 'errorOnUnmatchedPattern' must be a boolean.", "- 'extensions' must be an array of non-empty strings or null.", "- 'fix' must be a boolean or a function.", - "- 'fixTypes' must be an array of any of \"problem\", \"suggestion\", and \"layout\".", + "- 'fixTypes' must be an array of any of \"directive\", \"problem\", \"suggestion\", and \"layout\".", "- 'globInputPaths' must be a boolean.", "- 'ignore' must be a boolean.", "- 'ignorePath' must be a non-empty string or null.", @@ -441,7 +441,7 @@ describe("ESLint", () => { fix: true, fixTypes: ["layou"] }); - }, /'fixTypes' must be an array of any of "problem", "suggestion", and "layout"\./iu); + }, /'fixTypes' must be an array of any of "directive", "problem", "suggestion", and "layout"\./iu); }); it("should not fix any rules when fixTypes is used without fix", async () => { @@ -4925,13 +4925,17 @@ describe("ESLint", () => { message: "Unused eslint-disable directive (no problems were reported).", line: 1, column: 1, + fix: { + range: [0, 20], + text: "" + }, severity: 2, nodeType: null } ], errorCount: 1, warningCount: 0, - fixableErrorCount: 0, + fixableErrorCount: 1, fixableWarningCount: 0, source: "/* eslint-disable */", usedDeprecatedRules: [] diff --git a/tests/lib/linter/apply-disable-directives.js b/tests/lib/linter/apply-disable-directives.js index 84dddcba4e6..52798a86ca3 100644 --- a/tests/lib/linter/apply-disable-directives.js +++ b/tests/lib/linter/apply-disable-directives.js @@ -6,7 +6,26 @@ "use strict"; const assert = require("chai").assert; +const espree = require("espree"); const applyDisableDirectives = require("../../../lib/linter/apply-disable-directives"); +const { SourceCode } = require("../../../lib/source-code"); + +const DEFAULT_CONFIG = { + ecmaVersion: 6, + comment: true, + tokens: true, + range: true, + loc: true +}; + +/** + * Create a SourceCode instance from the given code. + * @param {string} text The text of the code. + * @returns {SourceCode} The SourceCode. + */ +function createSourceCode(text) { + return new SourceCode(text, espree.parse(text, DEFAULT_CONFIG)); +} describe("apply-disable-directives", () => { describe("/* eslint-disable */ comments without rules", () => { @@ -14,7 +33,8 @@ describe("apply-disable-directives", () => { assert.deepStrictEqual( applyDisableDirectives({ directives: [{ type: "disable", line: 1, column: 8, ruleId: null }], - problems: [{ line: 1, column: 7, ruleId: "foo" }] + problems: [{ line: 1, column: 7, ruleId: "foo" }], + sourceCode: createSourceCode("first; /* eslint-disable */") }), [{ ruleId: "foo", line: 1, column: 7 }] ); @@ -23,8 +43,9 @@ describe("apply-disable-directives", () => { it("keeps problems on a previous line before the comment", () => { assert.deepStrictEqual( applyDisableDirectives({ - directives: [{ type: "disable", line: 2, column: 8, ruleId: null }], - problems: [{ line: 1, column: 10, ruleId: "foo" }] + directives: [{ type: "disable", line: 2, column: 1, ruleId: null }], + problems: [{ line: 1, column: 10, ruleId: "foo" }], + sourceCode: createSourceCode("\n/* eslint-disable*/") }), [{ ruleId: "foo", line: 1, column: 10 }] ); @@ -34,7 +55,8 @@ describe("apply-disable-directives", () => { assert.deepStrictEqual( applyDisableDirectives({ directives: [{ type: "disable", line: 1, column: 8, ruleId: null }], - problems: [{ line: 1, column: 8, ruleId: null }] + problems: [{ line: 1, column: 8, ruleId: null }], + sourceCode: createSourceCode("first; /* eslint-disable foo */") }), [] ); @@ -54,7 +76,8 @@ describe("apply-disable-directives", () => { assert.deepStrictEqual( applyDisableDirectives({ directives: [{ type: "disable", line: 1, column: 8, ruleId: null }], - problems: [{ line: 2, column: 3, ruleId: "foo" }] + problems: [{ line: 2, column: 3, ruleId: "foo" }], + sourceCode: createSourceCode("first; /* eslint-disable foo */") }), [] ); @@ -66,7 +89,8 @@ describe("apply-disable-directives", () => { assert.deepStrictEqual( applyDisableDirectives({ directives: [{ type: "disable", line: 1, column: 8, ruleId: "foo" }], - problems: [{ line: 2, column: 3, ruleId: "foo" }] + problems: [{ line: 2, column: 3, ruleId: "foo" }], + sourceCode: createSourceCode("first; /* eslint-disable foo */") }), [] ); @@ -76,7 +100,8 @@ describe("apply-disable-directives", () => { assert.deepStrictEqual( applyDisableDirectives({ directives: [{ type: "disable", line: 1, column: 8, ruleId: "foo" }], - problems: [{ line: 1, column: 8, ruleId: "foo" }] + problems: [{ line: 1, column: 8, ruleId: "foo" }], + sourceCode: createSourceCode("first; /* eslint-disable foo */") }), [] ); @@ -85,8 +110,9 @@ describe("apply-disable-directives", () => { it("keeps problems after the comment that have a different ruleId", () => { assert.deepStrictEqual( applyDisableDirectives({ - directives: [{ type: "disable", line: 1, column: 8, ruleId: "foo" }], - problems: [{ line: 2, column: 3, ruleId: "not-foo" }] + directives: [{ type: "disable", line: 1, column: 1, ruleId: "foo" }], + problems: [{ line: 2, column: 3, ruleId: "not-foo" }], + sourceCode: createSourceCode("/* eslint-disable foo */") }), [{ line: 2, column: 3, ruleId: "not-foo" }] ); @@ -96,7 +122,8 @@ describe("apply-disable-directives", () => { assert.deepStrictEqual( applyDisableDirectives({ directives: [{ type: "disable", line: 1, column: 8, ruleId: "foo" }], - problems: [{ line: 1, column: 7, ruleId: "foo" }] + problems: [{ line: 1, column: 7, ruleId: "foo" }], + sourceCode: createSourceCode("first; /* eslint-disable foo */") }), [{ line: 1, column: 7, ruleId: "foo" }] ); @@ -109,11 +136,12 @@ describe("apply-disable-directives", () => { applyDisableDirectives({ directives: [ { type: "disable", line: 1, column: 1, ruleId: null }, - { type: "enable", line: 1, column: 5, ruleId: null } + { type: "enable", line: 1, column: 26, ruleId: null } ], - problems: [{ line: 1, column: 7, ruleId: "foo" }] + problems: [{ line: 1, column: 27, ruleId: "foo" }], + sourceCode: createSourceCode("/* eslint-disable */ /* eslint-enable */") }), - [{ line: 1, column: 7, ruleId: "foo" }] + [{ line: 1, column: 27, ruleId: "foo" }] ); }); @@ -122,11 +150,12 @@ describe("apply-disable-directives", () => { applyDisableDirectives({ directives: [ { type: "disable", line: 1, column: 1, ruleId: null }, - { type: "enable", line: 1, column: 5, ruleId: null } + { type: "enable", line: 1, column: 26, ruleId: null } ], - problems: [{ line: 1, column: 5, ruleId: "foo" }] + problems: [{ line: 1, column: 26, ruleId: "foo" }], + sourceCode: createSourceCode("/* eslint-disable */ /* eslint-enable */") }), - [{ line: 1, column: 5, ruleId: "foo" }] + [{ line: 1, column: 26, ruleId: "foo" }] ); }); @@ -135,9 +164,10 @@ describe("apply-disable-directives", () => { applyDisableDirectives({ directives: [ { type: "disable", line: 1, column: 1, ruleId: null }, - { type: "enable", line: 1, column: 5, ruleId: null } + { type: "enable", line: 1, column: 26, ruleId: null } ], - problems: [{ line: 1, column: 3, ruleId: "foo" }] + problems: [{ line: 1, column: 3, ruleId: "foo" }], + sourceCode: createSourceCode("/* eslint-disable */ /* eslint-enable */") }), [] ); @@ -148,10 +178,11 @@ describe("apply-disable-directives", () => { applyDisableDirectives({ directives: [ { type: "disable", line: 1, column: 1, ruleId: null }, - { type: "enable", line: 1, column: 5, ruleId: "foo" }, + { type: "enable", line: 1, column: 26, ruleId: "foo" }, { type: "disable", line: 2, column: 1, ruleId: "foo" } ], - problems: [{ line: 3, column: 3, ruleId: "foo" }] + problems: [{ line: 3, column: 3, ruleId: "foo" }], + sourceCode: createSourceCode("/* eslint-disable */ /* eslint-enable foo */\n/* eslint-disable foo */") }), [] ); @@ -162,10 +193,11 @@ describe("apply-disable-directives", () => { applyDisableDirectives({ directives: [ { type: "disable", line: 1, column: 1, ruleId: null }, - { type: "enable", line: 1, column: 5, ruleId: "foo" }, + { type: "enable", line: 1, column: 26, ruleId: "foo" }, { type: "disable", line: 2, column: 1, ruleId: null } ], - problems: [{ line: 3, column: 3, ruleId: "foo" }] + problems: [{ line: 3, column: 3, ruleId: "foo" }], + sourceCode: createSourceCode("/* eslint-disable */ /* eslint-enable foo *//* eslint-disable */") }), [] ); @@ -176,9 +208,10 @@ describe("apply-disable-directives", () => { applyDisableDirectives({ directives: [ { type: "disable", line: 1, column: 1, ruleId: "foo" }, - { type: "enable", line: 1, column: 5, ruleId: null } + { type: "enable", line: 1, column: 26, ruleId: null } ], - problems: [{ line: 1, column: 3, ruleId: "not-foo" }] + problems: [{ line: 1, column: 3, ruleId: "not-foo" }], + sourceCode: createSourceCode("/* eslint-disable foo */ /* eslint-enable */") }), [{ line: 1, column: 3, ruleId: "not-foo" }] ); @@ -190,10 +223,11 @@ describe("apply-disable-directives", () => { assert.deepStrictEqual( applyDisableDirectives({ directives: [ - { type: "disable", line: 1, column: 4, ruleId: null }, + { type: "disable", line: 1, column: 1, ruleId: null }, { type: "enable", line: 2, column: 1, ruleId: "foo" } ], - problems: [{ line: 2, column: 4, ruleId: "foo" }] + problems: [{ line: 2, column: 4, ruleId: "foo" }], + sourceCode: createSourceCode("/* eslint-disable */\n/* eslint-enable foo */") }), [{ line: 2, column: 4, ruleId: "foo" }] ); @@ -203,10 +237,11 @@ describe("apply-disable-directives", () => { assert.deepStrictEqual( applyDisableDirectives({ directives: [ - { type: "disable", line: 1, column: 4, ruleId: null }, + { type: "disable", line: 1, column: 1, ruleId: null }, { type: "enable", line: 2, column: 1, ruleId: "foo" } ], - problems: [{ line: 2, column: 1, ruleId: "foo" }] + problems: [{ line: 2, column: 1, ruleId: "foo" }], + sourceCode: createSourceCode("/* eslint-disable */\n/* eslint-enable foo */") }), [{ line: 2, column: 1, ruleId: "foo" }] ); @@ -216,10 +251,11 @@ describe("apply-disable-directives", () => { assert.deepStrictEqual( applyDisableDirectives({ directives: [ - { type: "disable", line: 1, column: 4, ruleId: null }, + { type: "disable", line: 1, column: 1, ruleId: null }, { type: "enable", line: 2, column: 1, ruleId: "foo" } ], - problems: [{ line: 2, column: 4, ruleId: "not-foo" }] + problems: [{ line: 2, column: 4, ruleId: "not-foo" }], + sourceCode: createSourceCode("/* eslint-disable */\n/* eslint-enable foo */") }), [] ); @@ -230,22 +266,23 @@ describe("apply-disable-directives", () => { applyDisableDirectives({ directives: [ { type: "disable", line: 1, column: 1, ruleId: null }, - { type: "enable", line: 1, column: 3, ruleId: "foo" }, - { type: "enable", line: 1, column: 5, ruleId: "bar" } + { type: "enable", line: 1, column: 22, ruleId: "foo" }, + { type: "enable", line: 1, column: 46, ruleId: "bar" } ], problems: [ - { line: 1, column: 2, ruleId: "foo" }, - { line: 1, column: 2, ruleId: "bar" }, - { line: 1, column: 4, ruleId: "foo" }, - { line: 1, column: 4, ruleId: "bar" }, - { line: 1, column: 6, ruleId: "foo" }, - { line: 1, column: 6, ruleId: "bar" } - ] + { line: 1, column: 10, ruleId: "foo" }, + { line: 1, column: 10, ruleId: "bar" }, + { line: 1, column: 30, ruleId: "foo" }, + { line: 1, column: 30, ruleId: "bar" }, + { line: 1, column: 50, ruleId: "foo" }, + { line: 1, column: 50, ruleId: "bar" } + ], + sourceCode: createSourceCode("/* eslint-disable */ /* eslint-enable foo */ /* eslint-enable bar */") }), [ - { line: 1, column: 4, ruleId: "foo" }, - { line: 1, column: 6, ruleId: "foo" }, - { line: 1, column: 6, ruleId: "bar" } + { line: 1, column: 30, ruleId: "foo" }, + { line: 1, column: 50, ruleId: "foo" }, + { line: 1, column: 50, ruleId: "bar" } ] ); }); @@ -256,7 +293,8 @@ describe("apply-disable-directives", () => { assert.deepStrictEqual( applyDisableDirectives({ directives: [{ type: "disable-line", line: 2, column: 1, ruleId: null }], - problems: [{ line: 1, column: 5, ruleId: "foo" }] + problems: [{ line: 1, column: 5, ruleId: "foo" }], + sourceCode: createSourceCode("first;\n// eslint-disable-line") }), [{ line: 1, column: 5, ruleId: "foo" }] ); @@ -265,8 +303,9 @@ describe("apply-disable-directives", () => { it("filters problems before the comment on the same line", () => { assert.deepStrictEqual( applyDisableDirectives({ - directives: [{ type: "disable-line", line: 1, column: 5, ruleId: null }], - problems: [{ line: 1, column: 1, ruleId: "foo" }] + directives: [{ type: "disable-line", line: 1, column: 8, ruleId: null }], + problems: [{ line: 1, column: 1, ruleId: "foo" }], + sourceCode: createSourceCode("first; // eslint-disable-line") }), [] ); @@ -275,8 +314,9 @@ describe("apply-disable-directives", () => { it("filters problems after the comment on the same line", () => { assert.deepStrictEqual( applyDisableDirectives({ - directives: [{ type: "disable-line", line: 1, column: 5, ruleId: null }], - problems: [{ line: 1, column: 10, ruleId: "foo" }] + directives: [{ type: "disable-line", line: 1, column: 8, ruleId: null }], + problems: [{ line: 1, column: 10, ruleId: "foo" }], + sourceCode: createSourceCode("first; // eslint-disable-line") }), [] ); @@ -285,8 +325,9 @@ describe("apply-disable-directives", () => { it("keeps problems on a following line", () => { assert.deepStrictEqual( applyDisableDirectives({ - directives: [{ type: "disable-line", line: 1, column: 4 }], - problems: [{ line: 2, column: 1, ruleId: "foo" }] + directives: [{ type: "disable-line", line: 1, column: 8 }], + problems: [{ line: 2, column: 1, ruleId: "foo" }], + sourceCode: createSourceCode("first; // eslint-disable-line foo") }), [{ line: 2, column: 1, ruleId: "foo" }] ); @@ -297,8 +338,9 @@ describe("apply-disable-directives", () => { it("filters problems on the current line that match the ruleId", () => { assert.deepStrictEqual( applyDisableDirectives({ - directives: [{ type: "disable-line", line: 1, column: 4, ruleId: "foo" }], - problems: [{ line: 1, column: 2, ruleId: "foo" }] + directives: [{ type: "disable-line", line: 1, column: 8, ruleId: "foo" }], + problems: [{ line: 1, column: 2, ruleId: "foo" }], + sourceCode: createSourceCode("first; // eslint-disable-line foo") }), [] ); @@ -307,8 +349,9 @@ describe("apply-disable-directives", () => { it("keeps problems on the current line that do not match the ruleId", () => { assert.deepStrictEqual( applyDisableDirectives({ - directives: [{ type: "disable-line", line: 1, column: 4, ruleId: "foo" }], - problems: [{ line: 1, column: 2, ruleId: "not-foo" }] + directives: [{ type: "disable-line", line: 1, column: 1, ruleId: "foo" }], + problems: [{ line: 1, column: 2, ruleId: "not-foo" }], + sourceCode: createSourceCode("// eslint-disable-line foo") }), [{ line: 1, column: 2, ruleId: "not-foo" }] ); @@ -319,9 +362,10 @@ describe("apply-disable-directives", () => { applyDisableDirectives({ directives: [ { type: "disable", line: 1, column: 1, ruleId: null }, - { type: "disable-line", line: 1, column: 3, ruleId: "foo" } + { type: "disable-line", line: 1, column: 22, ruleId: "foo" } ], - problems: [{ line: 1, column: 5, ruleId: "not-foo" }] + problems: [{ line: 1, column: 5, ruleId: "not-foo" }], + sourceCode: createSourceCode("/* eslint-disable */ // eslint-disable-line foo") }), [] ); @@ -331,14 +375,17 @@ describe("apply-disable-directives", () => { assert.deepStrictEqual( applyDisableDirectives({ directives: [ - { type: "disable-line", line: 1, column: 5, ruleId: "foo" }, - { type: "disable-line", line: 2, column: 5, ruleId: "foo" }, - { type: "disable-line", line: 3, column: 5, ruleId: "foo" }, - { type: "disable-line", line: 4, column: 5, ruleId: "foo" }, - { type: "disable-line", line: 5, column: 5, ruleId: "foo" }, - { type: "disable-line", line: 6, column: 5, ruleId: "foo" } + { type: "disable-line", line: 1, column: 8, ruleId: "foo" }, + { type: "disable-line", line: 2, column: 8, ruleId: "foo" }, + { type: "disable-line", line: 3, column: 8, ruleId: "foo" }, + { type: "disable-line", line: 4, column: 8, ruleId: "foo" }, + { type: "disable-line", line: 5, column: 8, ruleId: "foo" }, + { type: "disable-line", line: 6, column: 8, ruleId: "foo" } ], - problems: [{ line: 2, column: 1, ruleId: "foo" }] + problems: [{ line: 2, column: 1, ruleId: "foo" }], + sourceCode: createSourceCode( + new Array(6).fill(0).map(() => "first; // eslint-disable-line foo").join("\n") + ) }), [] ); @@ -350,7 +397,8 @@ describe("apply-disable-directives", () => { assert.deepStrictEqual( applyDisableDirectives({ directives: [{ type: "disable-next-line", line: 1, column: 1, ruleId: null }], - problems: [{ line: 2, column: 3, ruleId: "foo" }] + problems: [{ line: 2, column: 3, ruleId: "foo" }], + sourceCode: createSourceCode("// eslint-disable-next-line") }), [] ); @@ -360,7 +408,8 @@ describe("apply-disable-directives", () => { assert.deepStrictEqual( applyDisableDirectives({ directives: [{ type: "disable-next-line", line: 1, column: 1, ruleId: null }], - problems: [{ line: 1, column: 3, ruleId: "foo" }] + problems: [{ line: 1, column: 3, ruleId: "foo" }], + sourceCode: createSourceCode("// eslint-disable-next-line") }), [{ line: 1, column: 3, ruleId: "foo" }] ); @@ -370,7 +419,8 @@ describe("apply-disable-directives", () => { assert.deepStrictEqual( applyDisableDirectives({ directives: [{ type: "disable-next-line", line: 1, column: 1, ruleId: null }], - problems: [{ line: 3, column: 3, ruleId: "foo" }] + problems: [{ line: 3, column: 3, ruleId: "foo" }], + sourceCode: createSourceCode("// eslint-disable-next-line") }), [{ line: 3, column: 3, ruleId: "foo" }] ); @@ -383,7 +433,8 @@ describe("apply-disable-directives", () => { { type: "disable-next-line", line: 1, column: 1, ruleId: null }, { type: "enable", line: 1, column: 5, ruleId: null } ], - problems: [{ line: 2, column: 2, ruleId: "foo" }] + problems: [{ line: 2, column: 2, ruleId: "foo" }], + sourceCode: createSourceCode("// eslint-disable-next-line\n/* eslint-enable */") }), [] ); @@ -395,7 +446,8 @@ describe("apply-disable-directives", () => { assert.deepStrictEqual( applyDisableDirectives({ directives: [{ type: "disable-next-line", line: 1, column: 1, ruleId: "foo" }], - problems: [{ line: 2, column: 1, ruleId: "foo" }] + problems: [{ line: 2, column: 1, ruleId: "foo" }], + sourceCode: createSourceCode("// eslint-disable-next-line foo") }), [] ); @@ -405,7 +457,8 @@ describe("apply-disable-directives", () => { assert.deepStrictEqual( applyDisableDirectives({ directives: [{ type: "disable-next-line", line: 1, column: 1, ruleId: "foo" }], - problems: [{ line: 2, column: 1, ruleId: "not-foo" }] + problems: [{ line: 2, column: 1, ruleId: "not-foo" }], + sourceCode: createSourceCode("// eslint-disable-next-line foo\n/* eslint-enable foo */") }), [{ line: 2, column: 1, ruleId: "not-foo" }] ); @@ -429,16 +482,21 @@ describe("apply-disable-directives", () => { it("Adds a problem for /* eslint-disable */", () => { assert.deepStrictEqual( applyDisableDirectives({ - directives: [{ type: "disable", line: 1, column: 5 }], + directives: [{ type: "disable", line: 1, column: 1 }], problems: [], - reportUnusedDisableDirectives: "error" + reportUnusedDisableDirectives: "error", + sourceCode: createSourceCode("/* eslint-disable */") }), [ { ruleId: null, message: "Unused eslint-disable directive (no problems were reported).", line: 1, - column: 5, + column: 1, + fix: { + range: [0, 20], + text: "" + }, severity: 2, nodeType: null } @@ -449,9 +507,10 @@ describe("apply-disable-directives", () => { it("Does not add a problem for /* eslint-disable */ /* (problem) */", () => { assert.deepStrictEqual( applyDisableDirectives({ - directives: [{ type: "disable", line: 1, column: 5, ruleId: null }], + directives: [{ type: "disable", line: 1, column: 1, ruleId: null }], problems: [{ line: 2, column: 1, ruleId: "foo" }], - reportUnusedDisableDirectives: "error" + reportUnusedDisableDirectives: "error", + sourceCode: createSourceCode("/* eslint-disable */") }), [] ); @@ -460,16 +519,21 @@ describe("apply-disable-directives", () => { it("Adds a problem for /* eslint-disable foo */", () => { assert.deepStrictEqual( applyDisableDirectives({ - directives: [{ type: "disable", line: 1, column: 5, ruleId: "foo" }], + directives: [{ type: "disable", line: 1, column: 1, ruleId: "foo" }], problems: [], - reportUnusedDisableDirectives: "error" + reportUnusedDisableDirectives: "error", + sourceCode: createSourceCode("/* eslint-disable foo */") }), [ { ruleId: null, message: "Unused eslint-disable directive (no problems were reported from 'foo').", line: 1, - column: 5, + column: 1, + fix: { + range: [0, 24], + text: "" + }, severity: 2, nodeType: null } @@ -480,16 +544,21 @@ describe("apply-disable-directives", () => { it("Adds a problem for /* eslint-disable foo */ /* (problem from another rule) */", () => { assert.deepStrictEqual( applyDisableDirectives({ - directives: [{ type: "disable", line: 1, column: 5, ruleId: "foo" }], + directives: [{ type: "disable", line: 1, column: 1, ruleId: "foo" }], problems: [{ line: 1, column: 20, ruleId: "not-foo" }], - reportUnusedDisableDirectives: "error" + reportUnusedDisableDirectives: "error", + sourceCode: createSourceCode("/* eslint-disable foo */") }), [ { ruleId: null, message: "Unused eslint-disable directive (no problems were reported from 'foo').", line: 1, - column: 5, + column: 1, + fix: { + range: [0, 24], + text: "" + }, severity: 2, nodeType: null }, @@ -506,11 +575,12 @@ describe("apply-disable-directives", () => { assert.deepStrictEqual( applyDisableDirectives({ directives: [ - { type: "disable", line: 1, column: 5, ruleId: null }, - { type: "enable", line: 1, column: 6, ruleId: "foo" } + { type: "disable", line: 1, column: 8, ruleId: null }, + { type: "enable", line: 1, column: 24, ruleId: "foo" } ], problems: [{ line: 1, column: 2, ruleId: "foo" }], - reportUnusedDisableDirectives: "error" + reportUnusedDisableDirectives: "error", + sourceCode: createSourceCode("first; /* eslint-disable */ /* eslint-enable foo */") }), [ { @@ -521,8 +591,12 @@ describe("apply-disable-directives", () => { { ruleId: null, message: "Unused eslint-disable directive (no problems were reported).", + fix: { + range: [7, 28], + text: "" + }, line: 1, - column: 5, + column: 8, severity: 2, nodeType: null } @@ -534,18 +608,23 @@ describe("apply-disable-directives", () => { assert.deepStrictEqual( applyDisableDirectives({ directives: [ - { type: "disable", line: 1, column: 5, ruleId: null }, - { type: "enable", line: 1, column: 6, ruleId: null } + { type: "disable", line: 1, column: 1, ruleId: null }, + { type: "enable", line: 1, column: 12, ruleId: null } ], problems: [], - reportUnusedDisableDirectives: "error" + reportUnusedDisableDirectives: "error", + sourceCode: createSourceCode("/* eslint-disable */ /* eslint-enable */") }), [ { ruleId: null, message: "Unused eslint-disable directive (no problems were reported).", line: 1, - column: 5, + column: 1, + fix: { + range: [0, 20], + text: "" + }, severity: 2, nodeType: null } @@ -561,7 +640,8 @@ describe("apply-disable-directives", () => { { type: "disable", line: 2, column: 1, ruleId: null } ], problems: [], - reportUnusedDisableDirectives: "error" + reportUnusedDisableDirectives: "error", + sourceCode: createSourceCode("/* eslint-disable */\n/* eslint-disable */") }), [ { @@ -569,6 +649,10 @@ describe("apply-disable-directives", () => { message: "Unused eslint-disable directive (no problems were reported).", line: 1, column: 1, + fix: { + range: [0, 21], + text: "" + }, severity: 2, nodeType: null }, @@ -577,6 +661,10 @@ describe("apply-disable-directives", () => { message: "Unused eslint-disable directive (no problems were reported).", line: 2, column: 1, + fix: { + range: [21, 41], + text: "" + }, severity: 2, nodeType: null } @@ -592,7 +680,8 @@ describe("apply-disable-directives", () => { { type: "disable", line: 2, column: 1, ruleId: null } ], problems: [{ line: 3, column: 1, ruleId: "foo" }], - reportUnusedDisableDirectives: "error" + reportUnusedDisableDirectives: "error", + sourceCode: createSourceCode("/* eslint-disable */\n/* eslint-disable */") }), [ { @@ -600,6 +689,10 @@ describe("apply-disable-directives", () => { message: "Unused eslint-disable directive (no problems were reported).", line: 1, column: 1, + fix: { + range: [0, 21], + text: "" + }, severity: 2, nodeType: null } @@ -615,7 +708,8 @@ describe("apply-disable-directives", () => { { type: "disable", line: 2, column: 1, ruleId: null } ], problems: [{ line: 3, column: 1, ruleId: "foo" }], - reportUnusedDisableDirectives: "error" + reportUnusedDisableDirectives: "error", + sourceCode: createSourceCode("/* eslint-disable foo */\n/* eslint-disable */") }), [ { @@ -623,6 +717,10 @@ describe("apply-disable-directives", () => { message: "Unused eslint-disable directive (no problems were reported from 'foo').", line: 1, column: 1, + fix: { + range: [0, 25], + text: "" + }, severity: 2, nodeType: null } @@ -633,9 +731,10 @@ describe("apply-disable-directives", () => { it("Does not add a problem for /* eslint-disable foo */ /* (problem from foo) */", () => { assert.deepStrictEqual( applyDisableDirectives({ - directives: [{ type: "disable", line: 1, column: 5, ruleId: "foo" }], + directives: [{ type: "disable", line: 1, column: 1, ruleId: "foo" }], problems: [{ line: 1, column: 6, ruleId: "foo" }], - reportUnusedDisableDirectives: "error" + reportUnusedDisableDirectives: "error", + sourceCode: createSourceCode("/* eslint-disable foo */") }), [] ); @@ -649,7 +748,8 @@ describe("apply-disable-directives", () => { { type: "disable", line: 2, column: 1, ruleId: "foo" } ], problems: [{ line: 3, column: 1, ruleId: "foo" }], - reportUnusedDisableDirectives: "error" + reportUnusedDisableDirectives: "error", + sourceCode: createSourceCode("/* eslint-disable */\n/* eslint-disable foo */") }), [ { @@ -657,6 +757,10 @@ describe("apply-disable-directives", () => { message: "Unused eslint-disable directive (no problems were reported).", line: 1, column: 1, + fix: { + range: [0, 21], + text: "" + }, severity: 2, nodeType: null } @@ -672,7 +776,8 @@ describe("apply-disable-directives", () => { { type: "disable", line: 2, column: 1, ruleId: "foo" } ], problems: [{ line: 3, column: 1, ruleId: "bar" }], - reportUnusedDisableDirectives: "error" + reportUnusedDisableDirectives: "error", + sourceCode: createSourceCode("/* eslint-disable */\n/* eslint-disable foo */") }), [ { @@ -680,6 +785,10 @@ describe("apply-disable-directives", () => { message: "Unused eslint-disable directive (no problems were reported from 'foo').", line: 2, column: 1, + fix: { + range: [21, 45], + text: "" + }, severity: 2, nodeType: null } @@ -691,25 +800,30 @@ describe("apply-disable-directives", () => { assert.deepStrictEqual( applyDisableDirectives({ directives: [ - { type: "disable", line: 1, column: 5, ruleId: "foo" }, - { type: "enable", line: 1, column: 8, ruleId: "foo" } + { type: "disable", line: 1, column: 1, ruleId: "foo" }, + { type: "enable", line: 1, column: 26, ruleId: "foo" } ], - problems: [{ line: 1, column: 10, ruleId: "foo" }], - reportUnusedDisableDirectives: "error" + problems: [{ line: 1, column: 30, ruleId: "foo" }], + reportUnusedDisableDirectives: "error", + sourceCode: createSourceCode("/* eslint-disable */ /* eslint-enable foo */") }), [ { ruleId: null, message: "Unused eslint-disable directive (no problems were reported from 'foo').", line: 1, - column: 5, + column: 1, + fix: { + range: [0, 20], + text: "" + }, severity: 2, nodeType: null }, { ruleId: "foo", line: 1, - column: 10 + column: 30 } ] ); @@ -719,25 +833,30 @@ describe("apply-disable-directives", () => { assert.deepStrictEqual( applyDisableDirectives({ directives: [ - { type: "disable", line: 1, column: 5, ruleId: "foo" }, - { type: "enable", line: 1, column: 8, ruleId: null } + { type: "disable", line: 1, column: 1, ruleId: "foo" }, + { type: "enable", line: 1, column: 26, ruleId: null } ], - problems: [{ line: 1, column: 10, ruleId: "foo" }], - reportUnusedDisableDirectives: "error" + problems: [{ line: 1, column: 30, ruleId: "foo" }], + reportUnusedDisableDirectives: "error", + sourceCode: createSourceCode("/* eslint-disable foo */ /* eslint-enable */") }), [ { ruleId: null, message: "Unused eslint-disable directive (no problems were reported from 'foo').", line: 1, - column: 5, + column: 1, + fix: { + range: [0, 24], + text: "" + }, severity: 2, nodeType: null }, { ruleId: "foo", line: 1, - column: 10 + column: 30 } ] ); @@ -752,7 +871,8 @@ describe("apply-disable-directives", () => { { type: "enable", line: 3, column: 1, ruleId: "foo" } ], problems: [{ line: 4, column: 1, ruleId: "foo" }], - reportUnusedDisableDirectives: "error" + reportUnusedDisableDirectives: "error", + sourceCode: createSourceCode("/* eslint-disable */\n/* eslint-disable foo */\n/* eslint-enable foo */") }), [ { @@ -760,6 +880,10 @@ describe("apply-disable-directives", () => { message: "Unused eslint-disable directive (no problems were reported).", line: 1, column: 1, + fix: { + range: [0, 21], + text: "" + }, severity: 2, nodeType: null }, @@ -768,8 +892,11 @@ describe("apply-disable-directives", () => { message: "Unused eslint-disable directive (no problems were reported from 'foo').", line: 2, column: 1, + fix: { + range: [21, 46], + text: "" + }, severity: 2, - nodeType: null }, { @@ -784,16 +911,21 @@ describe("apply-disable-directives", () => { it("Adds a problem for // eslint-disable-line", () => { assert.deepStrictEqual( applyDisableDirectives({ - directives: [{ type: "disable-line", line: 1, column: 5, ruleId: null }], + directives: [{ type: "disable-line", line: 1, column: 1, ruleId: null }], problems: [], - reportUnusedDisableDirectives: "error" + reportUnusedDisableDirectives: "error", + sourceCode: createSourceCode("// eslint-disable-line") }), [ { ruleId: null, message: "Unused eslint-disable directive (no problems were reported).", line: 1, - column: 5, + column: 1, + fix: { + range: [0, 22], + text: "" + }, severity: 2, nodeType: null } @@ -805,9 +937,10 @@ describe("apply-disable-directives", () => { it("Does not add a problem for // eslint-disable-line (problem)", () => { assert.deepStrictEqual( applyDisableDirectives({ - directives: [{ type: "disable-line", line: 1, column: 5, ruleId: null }], + directives: [{ type: "disable-line", line: 1, column: 1, ruleId: null }], problems: [{ line: 1, column: 10, ruleId: "foo" }], - reportUnusedDisableDirectives: "error" + reportUnusedDisableDirectives: "error", + sourceCode: createSourceCode("/* eslint-disable line */") }), [] ); @@ -816,16 +949,21 @@ describe("apply-disable-directives", () => { it("Adds a problem for // eslint-disable-next-line", () => { assert.deepStrictEqual( applyDisableDirectives({ - directives: [{ type: "disable-next-line", line: 1, column: 5, ruleId: null }], + directives: [{ type: "disable-next-line", line: 1, column: 1, ruleId: null }], problems: [], - reportUnusedDisableDirectives: "error" + reportUnusedDisableDirectives: "error", + sourceCode: createSourceCode("// eslint-disable-next-line") }), [ { ruleId: null, message: "Unused eslint-disable directive (no problems were reported).", line: 1, - column: 5, + column: 1, + fix: { + range: [0, 27], + text: "" + }, severity: 2, nodeType: null } @@ -836,9 +974,10 @@ describe("apply-disable-directives", () => { it("Does not add a problem for // eslint-disable-next-line \\n (problem)", () => { assert.deepStrictEqual( applyDisableDirectives({ - directives: [{ type: "disable-next-line", line: 1, column: 5, ruleId: null }], + directives: [{ type: "disable-next-line", line: 1, column: 1, ruleId: null }], problems: [{ line: 2, column: 10, ruleId: "foo" }], - reportUnusedDisableDirectives: "error" + reportUnusedDisableDirectives: "error", + sourceCode: createSourceCode("// eslint-disable foo-next-line") }), [] ); @@ -849,10 +988,11 @@ describe("apply-disable-directives", () => { applyDisableDirectives({ directives: [ { type: "disable", line: 1, column: 1, ruleId: null }, - { type: "disable-line", line: 1, column: 5, ruleId: null } + { type: "disable-line", line: 1, column: 22, ruleId: null } ], problems: [], - reportUnusedDisableDirectives: "error" + reportUnusedDisableDirectives: "error", + sourceCode: createSourceCode("/* eslint-disable */ // eslint-disable-line") }), [ { @@ -860,6 +1000,10 @@ describe("apply-disable-directives", () => { message: "Unused eslint-disable directive (no problems were reported).", line: 1, column: 1, + fix: { + range: [0, 20], + text: "" + }, severity: 2, nodeType: null }, @@ -867,7 +1011,11 @@ describe("apply-disable-directives", () => { ruleId: null, message: "Unused eslint-disable directive (no problems were reported).", line: 1, - column: 5, + column: 22, + fix: { + range: [20, 43], + text: "" + }, severity: 2, nodeType: null } @@ -878,9 +1026,10 @@ describe("apply-disable-directives", () => { it("Does not add problems when reportUnusedDisableDirectives: \"off\" is used", () => { assert.deepStrictEqual( applyDisableDirectives({ - directives: [{ type: "disable-next-line", line: 1, column: 5, ruleId: null }], + directives: [{ type: "disable-next-line", line: 1, column: 1, ruleId: null }], problems: [], - reportUnusedDisableDirectives: "off" + reportUnusedDisableDirectives: "off", + sourceCode: createSourceCode("// eslint-disable-next-line") }), [] ); diff --git a/tests/lib/linter/linter.js b/tests/lib/linter/linter.js index 976bd765755..9a62d15c773 100644 --- a/tests/lib/linter/linter.js +++ b/tests/lib/linter/linter.js @@ -3266,6 +3266,10 @@ var a = "test2"; message: "Unused eslint-disable directive (no problems were reported).", line: 1, column: 1, + fix: { + range: [0, 20], + text: "" + }, severity: 2, nodeType: null } @@ -3282,6 +3286,10 @@ var a = "test2"; message: "Unused eslint-disable directive (no problems were reported).", line: 1, column: 1, + fix: { + range: [0, 20], + text: "" + }, severity: 2, nodeType: null } @@ -3298,6 +3306,10 @@ var a = "test2"; message: "Unused eslint-disable directive (no problems were reported).", line: 1, column: 1, + fix: { + range: [0, 20], + text: "" + }, severity: 1, nodeType: null } @@ -3314,6 +3326,10 @@ var a = "test2"; message: "Unused eslint-disable directive (no problems were reported).", line: 1, column: 1, + fix: { + range: [0, 20], + text: "" + }, severity: 1, nodeType: null } From d0148c87acbf17ba3c3d394cb9ebb09e20fd4ca3 Mon Sep 17 00:00:00 2001 From: Josh Goldberg Date: Mon, 21 Jun 2021 22:03:58 -0400 Subject: [PATCH 02/31] Part of the way there: not ready yet --- lib/linter/apply-disable-directives.js | 25 +++++++++++++++++++------ 1 file changed, 19 insertions(+), 6 deletions(-) diff --git a/lib/linter/apply-disable-directives.js b/lib/linter/apply-disable-directives.js index 774184762ee..389710258b3 100644 --- a/lib/linter/apply-disable-directives.js +++ b/lib/linter/apply-disable-directives.js @@ -22,10 +22,17 @@ function compareLocations(itemA, itemB) { * Returns whether a comment is alone on its line. * @param {Comment} comment The comment node which will be deleted. * @param {string} line Contents of the comment's line. + * @param {number} lineStart Character position the line starts at. * @returns {boolean} Whether the comment is alone on its line. */ -function commentIsAloneOnLine(comment, line) { - return comment.type === "Block" ? line.length === comment.value.length + "/**/".length : line.startsWith("//"); +function commentIsAloneOnLine(comment, line, lineStart) { + if (comment.range[0] !== lineStart) { + return false; + } + + return comment.type === "Block" + ? line.length === comment.value.length + "/**/".length + : true; } /** @@ -42,9 +49,15 @@ function getDirectiveRemovalRange(directive, sourceCode) { const comment = sourceCode.getTokenByRangeStart(commentStart, { includeComments: true }); const line = sourceCode.lines[lineIndex]; + const directiveContents = comment.value.split("--")[0].trimRight(); + const ruleIdsArea = directiveContents + .trim() + .slice(`eslint-${directive.unprocessedDirective.type}`.length) + .replace(/(,\W)*$/gu, ""); + // If the directive's rule isn't the only one in the comment, only remove that rule - if (comment.value.includes(",")) { - const ruleStart = comment.value.indexOf(directive.ruleId); + if (ruleIdsArea.includes(",")) { + const ruleStart = directiveContents.indexOf(directive.ruleId); const ruleEnd = ruleStart + directive.ruleId.length; if (comment.value.trimRight().endsWith(directive.ruleId)) { @@ -56,7 +69,7 @@ function getDirectiveRemovalRange(directive, sourceCode) { } else { for (const suffix of [", ", ","]) { if (comment.value.slice(ruleEnd, ruleEnd + suffix.length) === suffix) { - return [commentStart + 2 + ruleStart, commentStart + 2 + ruleEnd + suffix.length]; + return [commentStart + 2 + ruleStart, commentStart + ruleEnd + suffix.length]; } } } @@ -65,7 +78,7 @@ function getDirectiveRemovalRange(directive, sourceCode) { } // If the comment is alone on its line, remove the entire line - if (commentIsAloneOnLine(comment, line)) { + if (commentIsAloneOnLine(comment, line, sourceCode.lineStartIndices[lineIndex])) { return [ lineStart, lineIndex === sourceCode.lines.length - 1 From bead8e8c023b1dfbbd6922ddcfedafde7888140a Mon Sep 17 00:00:00 2001 From: Josh Goldberg Date: Tue, 22 Jun 2021 00:36:13 -0400 Subject: [PATCH 03/31] Generally fixed up, and just started on unit tests --- lib/linter/apply-disable-directives.js | 187 +++++++++++-------- lib/linter/linter.js | 4 +- tests/lib/cli-engine/cli-engine.js | 2 +- tests/lib/eslint/eslint.js | 2 +- tests/lib/linter/apply-disable-directives.js | 29 ++- 5 files changed, 141 insertions(+), 83 deletions(-) diff --git a/lib/linter/apply-disable-directives.js b/lib/linter/apply-disable-directives.js index 389710258b3..9b50ccd73b5 100644 --- a/lib/linter/apply-disable-directives.js +++ b/lib/linter/apply-disable-directives.js @@ -19,86 +19,118 @@ function compareLocations(itemA, itemB) { } /** - * Returns whether a comment is alone on its line. - * @param {Comment} comment The comment node which will be deleted. - * @param {string} line Contents of the comment's line. - * @param {number} lineStart Character position the line starts at. - * @returns {boolean} Whether the comment is alone on its line. + * Groups a set of directives into sub-arrays by their parent comment. + * @param {Directive[]} directives Unused directives to be removed. + * @returns {Directive[][]} Directives grouped by their parent comment. */ -function commentIsAloneOnLine(comment, line, lineStart) { - if (comment.range[0] !== lineStart) { - return false; +function groupByLine(directives) { + const groups = []; + + if (!directives.length) { + return groups; } - return comment.type === "Block" - ? line.length === comment.value.length + "/**/".length - : true; -} + let state = null; -/** - * Finds where to remove text for an unused directive. - * @param {Directive} directive Unused directive to be removed. - * @param {SourceCode} sourceCode A SourceCode object for the given text - * @returns {Range} Removal range for the unused directive. - * @see https://github.com/eslint/rfcs/pull/78 - */ -function getDirectiveRemovalRange(directive, sourceCode) { - const lineIndex = directive.unprocessedDirective.line - 1; - const lineStart = sourceCode.lineStartIndices[lineIndex]; - const commentStart = lineStart + directive.unprocessedDirective.column - 1; - const comment = sourceCode.getTokenByRangeStart(commentStart, { includeComments: true }); - const line = sourceCode.lines[lineIndex]; - - const directiveContents = comment.value.split("--")[0].trimRight(); - const ruleIdsArea = directiveContents - .trim() - .slice(`eslint-${directive.unprocessedDirective.type}`.length) - .replace(/(,\W)*$/gu, ""); - - // If the directive's rule isn't the only one in the comment, only remove that rule - if (ruleIdsArea.includes(",")) { - const ruleStart = directiveContents.indexOf(directive.ruleId); - const ruleEnd = ruleStart + directive.ruleId.length; - - if (comment.value.trimRight().endsWith(directive.ruleId)) { - for (const prefix of [", ", ","]) { - if (comment.value.slice(ruleStart - prefix.length, ruleStart) === prefix) { - return [commentStart + 2 + ruleStart - prefix.length, commentStart + ruleEnd + 2]; - } - } + for (const directive of directives) { + if (!state) { + state = { + group: [directive], + line: directive.line + }; + } else if (directive.line === state.line) { + state.group.push(directive); } else { - for (const suffix of [", ", ","]) { - if (comment.value.slice(ruleEnd, ruleEnd + suffix.length) === suffix) { - return [commentStart + 2 + ruleStart, commentStart + ruleEnd + suffix.length]; - } - } + groups.push(state.group); + state = null; } - - return [commentStart + 2 + ruleStart, commentStart + 2 + ruleEnd]; } - // If the comment is alone on its line, remove the entire line - if (commentIsAloneOnLine(comment, line, sourceCode.lineStartIndices[lineIndex])) { - return [ - lineStart, - lineIndex === sourceCode.lines.length - 1 - ? lineStart + line.length - : sourceCode.lineStartIndices[lineIndex + 1] - ]; + if (state && state.group.length) { + groups.push(state.group); } - // If the comment has space between it and whatever else is on its line, collapse the space - if (commentStart !== 0 && ["\n", " "].includes(sourceCode.text[commentStart - 1])) { - if (comment.range[1] === sourceCode.text.length) { - return [comment.range[0] - 1, comment.range[1]]; - } + return groups; +} + +/** + * Creates removal details for a set of directives within the same comment. + * @param {Directive[]} directives Unused directives to be removed. + * @param {ParentComment} parentComment Location and rule IDs for the comment to delete. + * @param {SourceCode} sourceCode A SourceCode object for the given text. + * @returns {{ fix, position, ruleIds }[]} Details for later creation of output Problems. + */ +function createIndividualDirectivesRemoval(directives, parentComment, sourceCode) { + const commentStart = sourceCode.lineStartIndices[parentComment.loc.start.line - 1] + parentComment.loc.start.column; + const commentToken = sourceCode.getTokenByRangeStart(commentStart, { includeComments: true }); + const commentContents = commentToken.value.split("--")[0].trimRight(); + let lastRuleEnd = 0; + + return directives.map(directive => { + const ruleStart = commentContents.indexOf(directive.ruleId, lastRuleEnd); + const ruleEnd = lastRuleEnd = ruleStart + directive.ruleId.length; + + return { + description: directive.ruleId, + fix: { + range: [ + commentStart + 2 + ruleStart, + commentStart + 2 + ruleEnd + (commentContents[ruleEnd] === "," ? 1 : 0) + ], + text: "" + }, + position: directive.unprocessedDirective + }; + }); +} + +/** + * Creates a description of deleting an entire unused disable comment. + * @param {Directive[]} directives Unused directives to be removed. + * @param {ParentComment} parentComment Location and rule IDs for the comment to delete. + * @param {SourceCode} sourceCode A SourceCode object for the given text. + * @returns {{ fix, position, ruleIds }} Details for later creation of an output Problem. + */ +function createCommentRemoval(directives, parentComment, sourceCode) { + const range = [ + sourceCode.lineStartIndices[parentComment.loc.start.line - 1] + parentComment.loc.start.column, + sourceCode.lineStartIndices[parentComment.loc.end.line - 1] + parentComment.loc.end.column + ]; + const ruleIds = directives.filter(directive => directive.ruleId).map(directive => `'${directive.ruleId}'`); - if (sourceCode.text[comment.range[1]] === " ") { - return [comment.range[0], comment.range[1] + 1]; + return { + description: ruleIds.length <= 2 + ? ruleIds.join(" or ") + : `${ruleIds.slice(0, ruleIds.length - 2).join(", ")}, or ${ruleIds[ruleIds.length - 1]}`, + fix: { + range, + text: " " + }, + position: directives[0].unprocessedDirective + }; +} + +/** + * Parses details from directives to create output Problems. + * @param {Directive[]} allDirectives Unused directives to be removed. + * @param {SourceCode} sourceCode A SourceCode object for the given text. + * @returns {{ fix, position, ruleIds }} Details for later creation of output Problems. + */ +function processUnusedDisableDirectives(allDirectives, sourceCode) { + const directiveGroups = groupByLine(allDirectives); + + return directiveGroups.flatMap(directives => { + const { parentComment } = directives[0].unprocessedDirective; + const remainingRuleIds = new Set(parentComment.ruleIds); + + for (const directive of directives) { + remainingRuleIds.delete(directive.ruleId); } - } - return comment.range; + return remainingRuleIds.size + ? createIndividualDirectivesRemoval(directives, parentComment, sourceCode) + : [createCommentRemoval(directives, parentComment, sourceCode)]; + }); } /** @@ -167,19 +199,18 @@ function applyDirectives(options) { } } - const unusedDisableDirectives = options.directives - .filter(directive => directive.type === "disable" && !usedDisableDirectives.has(directive)) - .map(directive => ({ - fix: { - range: getDirectiveRemovalRange(directive, options.sourceCode), - text: "" - }, + const unusedDisableDirectivesToReport = options.directives + .filter(directive => directive.type === "disable" && !usedDisableDirectives.has(directive)); + + const unusedDisableDirectives = processUnusedDisableDirectives(unusedDisableDirectivesToReport, options.sourceCode) + .map(({ description, fix, position }) => ({ + fix, ruleId: null, - message: directive.ruleId - ? `Unused eslint-disable directive (no problems were reported from '${directive.ruleId}').` + message: description + ? `Unused eslint-disable directive (no problems were reported from ${description}).` : "Unused eslint-disable directive (no problems were reported).", - line: directive.unprocessedDirective.line, - column: directive.unprocessedDirective.column, + line: position.line, + column: position.column, severity: options.reportUnusedDisableDirectives === "warn" ? 1 : 2, nodeType: null })); diff --git a/lib/linter/linter.js b/lib/linter/linter.js index 58e2d692a30..08f980c091d 100644 --- a/lib/linter/linter.js +++ b/lib/linter/linter.js @@ -255,11 +255,13 @@ function createDisableDirectives(options) { directiveProblems: [] // problems in directives }; + const parentComment = { loc, ruleIds }; + for (const ruleId of directiveRules) { // push to directives, if the rule is defined(including null, e.g. /*eslint enable*/) if (ruleId === null || ruleMapper(ruleId) !== null) { - result.directives.push({ type, line: loc.start.line, column: loc.start.column + 1, ruleId }); + result.directives.push({ parentComment, type, line: loc.start.line, column: loc.start.column + 1, ruleId }); } else { result.directiveProblems.push(createLintingProblem({ ruleId, loc })); } diff --git a/tests/lib/cli-engine/cli-engine.js b/tests/lib/cli-engine/cli-engine.js index f3ffdeccb68..c1a9c23e042 100644 --- a/tests/lib/cli-engine/cli-engine.js +++ b/tests/lib/cli-engine/cli-engine.js @@ -5073,7 +5073,7 @@ describe("CLIEngine", () => { column: 1, fix: { range: [0, 20], - text: "" + text: " " }, severity: 2, nodeType: null diff --git a/tests/lib/eslint/eslint.js b/tests/lib/eslint/eslint.js index 75cf6fa0493..f767f5ed490 100644 --- a/tests/lib/eslint/eslint.js +++ b/tests/lib/eslint/eslint.js @@ -4927,7 +4927,7 @@ describe("ESLint", () => { column: 1, fix: { range: [0, 20], - text: "" + text: " " }, severity: 2, nodeType: null diff --git a/tests/lib/linter/apply-disable-directives.js b/tests/lib/linter/apply-disable-directives.js index 52798a86ca3..bb27c51e18a 100644 --- a/tests/lib/linter/apply-disable-directives.js +++ b/tests/lib/linter/apply-disable-directives.js @@ -27,12 +27,37 @@ function createSourceCode(text) { return new SourceCode(text, espree.parse(text, DEFAULT_CONFIG)); } +/** + * Creates a ParentComment for a given range. + * @param {number} startLine loc.start.line value + * @param {number} startColumn loc.start.column value + * @param {number} endLine loc.end.line value + * @param {number} endColumn loc.end.column value + * @param {string[]} ruleIds Rule IDs reported in the comment + * @returns {ParentComment} Test-ready ParentComment object. + */ +function createParentComment(startLine, startColumn, endLine, endColumn, ruleIds = []) { + return { + loc: { + start: { + line: startLine, + column: startColumn + }, + end: { + line: endLine, + column: endColumn + } + }, + ruleIds + }; +} + describe("apply-disable-directives", () => { describe("/* eslint-disable */ comments without rules", () => { it("keeps problems before the comment on the same line", () => { assert.deepStrictEqual( applyDisableDirectives({ - directives: [{ type: "disable", line: 1, column: 8, ruleId: null }], + directives: [{ parentComment: createParentComment(1, 7, 1, 27), type: "disable", line: 1, column: 8, ruleId: null }], problems: [{ line: 1, column: 7, ruleId: "foo" }], sourceCode: createSourceCode("first; /* eslint-disable */") }), @@ -43,7 +68,7 @@ describe("apply-disable-directives", () => { it("keeps problems on a previous line before the comment", () => { assert.deepStrictEqual( applyDisableDirectives({ - directives: [{ type: "disable", line: 2, column: 1, ruleId: null }], + directives: [{ parentComment: createParentComment(2, 1, 2, 20), type: "disable", line: 2, column: 1, ruleId: null }], problems: [{ line: 1, column: 10, ruleId: "foo" }], sourceCode: createSourceCode("\n/* eslint-disable*/") }), From bd16fc85e503d856bfda5be5c2c10f00abcffe7d Mon Sep 17 00:00:00 2001 From: Josh Goldberg Date: Tue, 22 Jun 2021 00:42:47 -0400 Subject: [PATCH 04/31] lodash.flatMap --- lib/linter/apply-disable-directives.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/linter/apply-disable-directives.js b/lib/linter/apply-disable-directives.js index 9b50ccd73b5..399c1ec8368 100644 --- a/lib/linter/apply-disable-directives.js +++ b/lib/linter/apply-disable-directives.js @@ -119,7 +119,7 @@ function createCommentRemoval(directives, parentComment, sourceCode) { function processUnusedDisableDirectives(allDirectives, sourceCode) { const directiveGroups = groupByLine(allDirectives); - return directiveGroups.flatMap(directives => { + return lodash.flatMap(directiveGroups).map(directives => { const { parentComment } = directives[0].unprocessedDirective; const remainingRuleIds = new Set(parentComment.ruleIds); From 47723eeadb448caf71bae68737e00a2b5323c99f Mon Sep 17 00:00:00 2001 From: Josh Goldberg Date: Tue, 22 Jun 2021 09:17:44 -0400 Subject: [PATCH 05/31] Merge branch 'master' --- lib/linter/apply-disable-directives.js | 51 ++++++++++++++------------ 1 file changed, 27 insertions(+), 24 deletions(-) diff --git a/lib/linter/apply-disable-directives.js b/lib/linter/apply-disable-directives.js index 3a18e612c56..9044085257d 100644 --- a/lib/linter/apply-disable-directives.js +++ b/lib/linter/apply-disable-directives.js @@ -108,6 +108,20 @@ function createCommentRemoval(directives, parentComment, sourceCode) { }; } +/** + * Returns a new array formed by applying a given callback function to each element of the array, and then flattening the result by one level. + * TODO(stephenwade): Replace this with array.flatMap when we drop support for Node v10 + * @param {any[]} array The array to process + * @param {Function} fn The function to use + * @returns {any[]} The result array + */ +function flatMap(array, fn) { + const mapped = array.map(fn); + const flattened = [].concat(...mapped); + + return flattened; +} + /** * Parses details from directives to create output Problems. * @param {Directive[]} allDirectives Unused directives to be removed. @@ -117,18 +131,21 @@ function createCommentRemoval(directives, parentComment, sourceCode) { function processUnusedDisableDirectives(allDirectives, sourceCode) { const directiveGroups = groupByLine(allDirectives); - return lodash.flatMap(directiveGroups).map(directives => { - const { parentComment } = directives[0].unprocessedDirective; - const remainingRuleIds = new Set(parentComment.ruleIds); + return flatMap( + directiveGroups, + directives => { + const { parentComment } = directives[0].unprocessedDirective; + const remainingRuleIds = new Set(parentComment.ruleIds); - for (const directive of directives) { - remainingRuleIds.delete(directive.ruleId); - } + for (const directive of directives) { + remainingRuleIds.delete(directive.ruleId); + } - return remainingRuleIds.size - ? createIndividualDirectivesRemoval(directives, parentComment, sourceCode) - : [createCommentRemoval(directives, parentComment, sourceCode)]; - }); + return remainingRuleIds.size + ? createIndividualDirectivesRemoval(directives, parentComment, sourceCode) + : [createCommentRemoval(directives, parentComment, sourceCode)]; + } + ); } /** @@ -241,20 +258,6 @@ module.exports = ({ directives, problems, reportUnusedDisableDirectives = "off", .map(directive => Object.assign({}, directive, { unprocessedDirective: directive })) .sort(compareLocations); - /** - * Returns a new array formed by applying a given callback function to each element of the array, and then flattening the result by one level. - * TODO(stephenwade): Replace this with array.flatMap when we drop support for Node v10 - * @param {any[]} array The array to process - * @param {Function} fn The function to use - * @returns {any[]} The result array - */ - function flatMap(array, fn) { - const mapped = array.map(fn); - const flattened = [].concat(...mapped); - - return flattened; - } - const lineDirectives = flatMap(directives, directive => { switch (directive.type) { case "disable": From 34ce0a7523ce35913c27fc152d902a7aa0dafdd9 Mon Sep 17 00:00:00 2001 From: Josh Goldberg Date: Fri, 25 Jun 2021 09:56:39 -0400 Subject: [PATCH 06/31] Progress... --- lib/linter/apply-disable-directives.js | 30 +++--- tests/lib/linter/apply-disable-directives.js | 100 ++++++++++++++----- 2 files changed, 94 insertions(+), 36 deletions(-) diff --git a/lib/linter/apply-disable-directives.js b/lib/linter/apply-disable-directives.js index 9044085257d..90438fd2940 100644 --- a/lib/linter/apply-disable-directives.js +++ b/lib/linter/apply-disable-directives.js @@ -21,7 +21,7 @@ function compareLocations(itemA, itemB) { * @param {Directive[]} directives Unused directives to be removed. * @returns {Directive[][]} Directives grouped by their parent comment. */ -function groupByLine(directives) { +function groupByParentComment(directives) { const groups = []; if (!directives.length) { @@ -31,17 +31,19 @@ function groupByLine(directives) { let state = null; for (const directive of directives) { - if (!state) { - state = { - group: [directive], - line: directive.line - }; - } else if (directive.line === state.line) { - state.group.push(directive); - } else { - groups.push(state.group); - state = null; + if (state) { + if (directive.parentComment === state.parentComment) { + state.group.push(directive); + } else { + groups.push(state.group); + state = null; + } } + + state = { + group: [directive], + parentComment: directive.parentComment + }; } if (state && state.group.length) { @@ -129,7 +131,7 @@ function flatMap(array, fn) { * @returns {{ fix, position, ruleIds }} Details for later creation of output Problems. */ function processUnusedDisableDirectives(allDirectives, sourceCode) { - const directiveGroups = groupByLine(allDirectives); + const directiveGroups = groupByParentComment(allDirectives); return flatMap( directiveGroups, @@ -217,7 +219,9 @@ function applyDirectives(options) { const unusedDisableDirectivesToReport = options.directives .filter(directive => directive.type === "disable" && !usedDisableDirectives.has(directive)); - const unusedDisableDirectives = processUnusedDisableDirectives(unusedDisableDirectivesToReport, options.sourceCode) + const processed = processUnusedDisableDirectives(unusedDisableDirectivesToReport, options.sourceCode); + + const unusedDisableDirectives = processed .map(({ description, fix, position }) => ({ fix, ruleId: null, diff --git a/tests/lib/linter/apply-disable-directives.js b/tests/lib/linter/apply-disable-directives.js index bb27c51e18a..0622daa208d 100644 --- a/tests/lib/linter/apply-disable-directives.js +++ b/tests/lib/linter/apply-disable-directives.js @@ -825,8 +825,20 @@ describe("apply-disable-directives", () => { assert.deepStrictEqual( applyDisableDirectives({ directives: [ - { type: "disable", line: 1, column: 1, ruleId: "foo" }, - { type: "enable", line: 1, column: 26, ruleId: "foo" } + { + parentComment: createParentComment(1, 0, 1, 20), + type: "disable", + line: 1, + column: 1, + ruleId: "foo" + }, + { + parentComment: createParentComment(1, 26, 1, 46), + type: "enable", + line: 1, + column: 26, + ruleId: "foo" + } ], problems: [{ line: 1, column: 30, ruleId: "foo" }], reportUnusedDisableDirectives: "error", @@ -840,7 +852,7 @@ describe("apply-disable-directives", () => { column: 1, fix: { range: [0, 20], - text: "" + text: " " }, severity: 2, nodeType: null @@ -858,12 +870,24 @@ describe("apply-disable-directives", () => { assert.deepStrictEqual( applyDisableDirectives({ directives: [ - { type: "disable", line: 1, column: 1, ruleId: "foo" }, - { type: "enable", line: 1, column: 26, ruleId: null } + { + parentComment: createParentComment(1, 0, 1, 24), + type: "disable", + line: 1, + column: 1, + ruleId: "foo" + }, + { + parentComment: createParentComment(1, 0, 25, 49), + type: "enable", + line: 1, + column: 26, + ruleId: null + } ], problems: [{ line: 1, column: 30, ruleId: "foo" }], reportUnusedDisableDirectives: "error", - sourceCode: createSourceCode("/* eslint-disable foo */ /* eslint-enable */") + sourceCode: createSourceCode("/* eslint-disable foo */ /* eslint-enable */ /* (problem from foo) */") }), [ { @@ -873,7 +897,7 @@ describe("apply-disable-directives", () => { column: 1, fix: { range: [0, 24], - text: "" + text: " " }, severity: 2, nodeType: null @@ -891,13 +915,31 @@ describe("apply-disable-directives", () => { assert.deepStrictEqual( applyDisableDirectives({ directives: [ - { type: "disable", line: 1, column: 1, ruleId: null }, - { type: "disable", line: 2, column: 1, ruleId: "foo" }, - { type: "enable", line: 3, column: 1, ruleId: "foo" } + { + parentComment: createParentComment(1, 0, 1, 21), + type: "disable", + line: 1, + column: 1, + ruleId: null + }, + { + parentComment: createParentComment(2, 0, 2, 24), + type: "disable", + line: 2, + column: 1, + ruleId: "foo" + }, + { + parentComment: createParentComment(3, 0, 3, 23), + type: "enable", + line: 3, + column: 1, + ruleId: "foo" + } ], problems: [{ line: 4, column: 1, ruleId: "foo" }], reportUnusedDisableDirectives: "error", - sourceCode: createSourceCode("/* eslint-disable */\n/* eslint-disable foo */\n/* eslint-enable foo */") + sourceCode: createSourceCode("/* eslint-disable */\n/* eslint-disable foo */\n/* eslint-enable foo */ /* (problem from foo) */") }), [ { @@ -907,7 +949,7 @@ describe("apply-disable-directives", () => { column: 1, fix: { range: [0, 21], - text: "" + text: " " }, severity: 2, nodeType: null @@ -918,8 +960,8 @@ describe("apply-disable-directives", () => { line: 2, column: 1, fix: { - range: [21, 46], - text: "" + range: [21, 45], + text: " " }, severity: 2, nodeType: null @@ -936,7 +978,13 @@ describe("apply-disable-directives", () => { it("Adds a problem for // eslint-disable-line", () => { assert.deepStrictEqual( applyDisableDirectives({ - directives: [{ type: "disable-line", line: 1, column: 1, ruleId: null }], + directives: [{ + parentComment: createParentComment(1, 0, 1, 22), + type: "disable-line", + line: 1, + column: 1, + ruleId: null + }], problems: [], reportUnusedDisableDirectives: "error", sourceCode: createSourceCode("// eslint-disable-line") @@ -949,7 +997,7 @@ describe("apply-disable-directives", () => { column: 1, fix: { range: [0, 22], - text: "" + text: " " }, severity: 2, nodeType: null @@ -974,7 +1022,13 @@ describe("apply-disable-directives", () => { it("Adds a problem for // eslint-disable-next-line", () => { assert.deepStrictEqual( applyDisableDirectives({ - directives: [{ type: "disable-next-line", line: 1, column: 1, ruleId: null }], + directives: [{ + parentComment: createParentComment(1, 0, 1, 27), + type: "disable-next-line", + line: 1, + column: 1, + ruleId: null + }], problems: [], reportUnusedDisableDirectives: "error", sourceCode: createSourceCode("// eslint-disable-next-line") @@ -987,7 +1041,7 @@ describe("apply-disable-directives", () => { column: 1, fix: { range: [0, 27], - text: "" + text: " " }, severity: 2, nodeType: null @@ -1012,8 +1066,8 @@ describe("apply-disable-directives", () => { assert.deepStrictEqual( applyDisableDirectives({ directives: [ - { type: "disable", line: 1, column: 1, ruleId: null }, - { type: "disable-line", line: 1, column: 22, ruleId: null } + { parentComment: createParentComment(1, 0, 1, 20), type: "disable", line: 1, column: 1, ruleId: null }, + { parentComment: createParentComment(1, 20, 1, 43), type: "disable-line", line: 1, column: 22, ruleId: null } ], problems: [], reportUnusedDisableDirectives: "error", @@ -1027,7 +1081,7 @@ describe("apply-disable-directives", () => { column: 1, fix: { range: [0, 20], - text: "" + text: " " }, severity: 2, nodeType: null @@ -1039,7 +1093,7 @@ describe("apply-disable-directives", () => { column: 22, fix: { range: [20, 43], - text: "" + text: " " }, severity: 2, nodeType: null @@ -1051,7 +1105,7 @@ describe("apply-disable-directives", () => { it("Does not add problems when reportUnusedDisableDirectives: \"off\" is used", () => { assert.deepStrictEqual( applyDisableDirectives({ - directives: [{ type: "disable-next-line", line: 1, column: 1, ruleId: null }], + directives: [{ parentComment: createParentComment(1, 1, 1, 27), type: "disable-next-line", line: 1, column: 1, ruleId: null }], problems: [], reportUnusedDisableDirectives: "off", sourceCode: createSourceCode("// eslint-disable-next-line") From ad043ff60d6687cec5fc14786462b63665cca52f Mon Sep 17 00:00:00 2001 From: Josh Goldberg Date: Fri, 25 Jun 2021 20:37:27 -0400 Subject: [PATCH 07/31] Fixed the remaining pre-existing unit tests --- tests/lib/linter/apply-disable-directives.js | 455 ++++++++++++++++--- 1 file changed, 383 insertions(+), 72 deletions(-) diff --git a/tests/lib/linter/apply-disable-directives.js b/tests/lib/linter/apply-disable-directives.js index 0622daa208d..c3cb70a23c8 100644 --- a/tests/lib/linter/apply-disable-directives.js +++ b/tests/lib/linter/apply-disable-directives.js @@ -135,7 +135,13 @@ describe("apply-disable-directives", () => { it("keeps problems after the comment that have a different ruleId", () => { assert.deepStrictEqual( applyDisableDirectives({ - directives: [{ type: "disable", line: 1, column: 1, ruleId: "foo" }], + directives: [{ + parentComment: createParentComment(1, 0, 1, 25), + type: "disable", + line: 1, + column: 1, + ruleId: "foo" + }], problems: [{ line: 2, column: 3, ruleId: "not-foo" }], sourceCode: createSourceCode("/* eslint-disable foo */") }), @@ -146,7 +152,13 @@ describe("apply-disable-directives", () => { it("keeps problems before the comment that have the same ruleId", () => { assert.deepStrictEqual( applyDisableDirectives({ - directives: [{ type: "disable", line: 1, column: 8, ruleId: "foo" }], + directives: [{ + parentComment: createParentComment(1, 8, 1, 30), + type: "disable", + line: 1, + column: 8, + ruleId: "foo" + }], problems: [{ line: 1, column: 7, ruleId: "foo" }], sourceCode: createSourceCode("first; /* eslint-disable foo */") }), @@ -160,8 +172,20 @@ describe("apply-disable-directives", () => { assert.deepStrictEqual( applyDisableDirectives({ directives: [ - { type: "disable", line: 1, column: 1, ruleId: null }, - { type: "enable", line: 1, column: 26, ruleId: null } + { + parentComment: createParentComment(1, 0, 1, 21), + type: "disable", + line: 1, + column: 1, + ruleId: null + }, + { + parentComment: createParentComment(1, 26, 1, 45), + type: "enable", + line: 1, + column: 26, + ruleId: null + } ], problems: [{ line: 1, column: 27, ruleId: "foo" }], sourceCode: createSourceCode("/* eslint-disable */ /* eslint-enable */") @@ -174,8 +198,20 @@ describe("apply-disable-directives", () => { assert.deepStrictEqual( applyDisableDirectives({ directives: [ - { type: "disable", line: 1, column: 1, ruleId: null }, - { type: "enable", line: 1, column: 26, ruleId: null } + { + parentComment: createParentComment(1, 0, 1, 26), + type: "disable", + line: 1, + column: 1, + ruleId: null + }, + { + parentComment: createParentComment(1, 26, 1, 41), + type: "enable", + line: 1, + column: 26, + ruleId: null + } ], problems: [{ line: 1, column: 26, ruleId: "foo" }], sourceCode: createSourceCode("/* eslint-disable */ /* eslint-enable */") @@ -202,9 +238,27 @@ describe("apply-disable-directives", () => { assert.deepStrictEqual( applyDisableDirectives({ directives: [ - { type: "disable", line: 1, column: 1, ruleId: null }, - { type: "enable", line: 1, column: 26, ruleId: "foo" }, - { type: "disable", line: 2, column: 1, ruleId: "foo" } + { + parentComment: createParentComment(1, 0, 1, 21), + type: "disable", + line: 1, + column: 1, + ruleId: null + }, + { + parentComment: createParentComment(1, 26, 1, 45), + type: "enable", + line: 1, + column: 26, + ruleId: "foo" + }, + { + parentComment: createParentComment(2, 0, 2, 21), + type: "disable", + line: 2, + column: 1, + ruleId: "foo" + } ], problems: [{ line: 3, column: 3, ruleId: "foo" }], sourceCode: createSourceCode("/* eslint-disable */ /* eslint-enable foo */\n/* eslint-disable foo */") @@ -217,9 +271,27 @@ describe("apply-disable-directives", () => { assert.deepStrictEqual( applyDisableDirectives({ directives: [ - { type: "disable", line: 1, column: 1, ruleId: null }, - { type: "enable", line: 1, column: 26, ruleId: "foo" }, - { type: "disable", line: 2, column: 1, ruleId: null } + { + parentComment: createParentComment(1, 0, 1, 21), + type: "disable", + line: 1, + column: 1, + ruleId: null + }, + { + parentComment: createParentComment(1, 26, 1, 45), + type: "enable", + line: 1, + column: 26, + ruleId: "foo" + }, + { + parentComment: createParentComment(2, 0, 2, 21), + type: "disable", + line: 2, + column: 1, + ruleId: null + } ], problems: [{ line: 3, column: 3, ruleId: "foo" }], sourceCode: createSourceCode("/* eslint-disable */ /* eslint-enable foo *//* eslint-disable */") @@ -232,8 +304,20 @@ describe("apply-disable-directives", () => { assert.deepStrictEqual( applyDisableDirectives({ directives: [ - { type: "disable", line: 1, column: 1, ruleId: "foo" }, - { type: "enable", line: 1, column: 26, ruleId: null } + { + parentComment: createParentComment(1, 0, 1, 21), + type: "disable", + line: 1, + column: 1, + ruleId: "foo" + }, + { + parentComment: createParentComment(1, 26, 1, 45), + type: "enable", + line: 1, + column: 26, + ruleId: null + } ], problems: [{ line: 1, column: 3, ruleId: "not-foo" }], sourceCode: createSourceCode("/* eslint-disable foo */ /* eslint-enable */") @@ -248,8 +332,20 @@ describe("apply-disable-directives", () => { assert.deepStrictEqual( applyDisableDirectives({ directives: [ - { type: "disable", line: 1, column: 1, ruleId: null }, - { type: "enable", line: 2, column: 1, ruleId: "foo" } + { + parentComment: createParentComment(1, 0, 1, 21), + type: "disable", + line: 1, + column: 1, + ruleId: null + }, + { + parentComment: createParentComment(2, 0, 2, 25), + type: "enable", + line: 2, + column: 1, + ruleId: "foo" + } ], problems: [{ line: 2, column: 4, ruleId: "foo" }], sourceCode: createSourceCode("/* eslint-disable */\n/* eslint-enable foo */") @@ -262,8 +358,20 @@ describe("apply-disable-directives", () => { assert.deepStrictEqual( applyDisableDirectives({ directives: [ - { type: "disable", line: 1, column: 1, ruleId: null }, - { type: "enable", line: 2, column: 1, ruleId: "foo" } + { + parentComment: createParentComment(1, 0, 1, 21), + type: "disable", + line: 1, + column: 1, + ruleId: null + }, + { + parentComment: createParentComment(2, 0, 2, 25), + type: "enable", + line: 2, + column: 1, + ruleId: "foo" + } ], problems: [{ line: 2, column: 1, ruleId: "foo" }], sourceCode: createSourceCode("/* eslint-disable */\n/* eslint-enable foo */") @@ -317,7 +425,13 @@ describe("apply-disable-directives", () => { it("keeps problems on a previous line", () => { assert.deepStrictEqual( applyDisableDirectives({ - directives: [{ type: "disable-line", line: 2, column: 1, ruleId: null }], + directives: [{ + parentComment: createParentComment(2, 0, 1, 21), + type: "disable-line", + line: 2, + column: 1, + ruleId: null + }], problems: [{ line: 1, column: 5, ruleId: "foo" }], sourceCode: createSourceCode("first;\n// eslint-disable-line") }), @@ -328,7 +442,13 @@ describe("apply-disable-directives", () => { it("filters problems before the comment on the same line", () => { assert.deepStrictEqual( applyDisableDirectives({ - directives: [{ type: "disable-line", line: 1, column: 8, ruleId: null }], + directives: [{ + parentComment: createParentComment(1, 8, 1, 29), + type: "disable-line", + line: 1, + column: 8, + ruleId: null + }], problems: [{ line: 1, column: 1, ruleId: "foo" }], sourceCode: createSourceCode("first; // eslint-disable-line") }), @@ -339,7 +459,13 @@ describe("apply-disable-directives", () => { it("filters problems after the comment on the same line", () => { assert.deepStrictEqual( applyDisableDirectives({ - directives: [{ type: "disable-line", line: 1, column: 8, ruleId: null }], + directives: [{ + parentComment: createParentComment(1, 8, 1, 29), + type: "disable-line", + line: 1, + column: 8, + ruleId: null + }], problems: [{ line: 1, column: 10, ruleId: "foo" }], sourceCode: createSourceCode("first; // eslint-disable-line") }), @@ -350,7 +476,13 @@ describe("apply-disable-directives", () => { it("keeps problems on a following line", () => { assert.deepStrictEqual( applyDisableDirectives({ - directives: [{ type: "disable-line", line: 1, column: 8 }], + directives: [{ + parentComment: createParentComment(1, 8, 1, 35), + type: "disable-line", + line: 1, + column: 8, + ruleId: "foo" + }], problems: [{ line: 2, column: 1, ruleId: "foo" }], sourceCode: createSourceCode("first; // eslint-disable-line foo") }), @@ -363,7 +495,13 @@ describe("apply-disable-directives", () => { it("filters problems on the current line that match the ruleId", () => { assert.deepStrictEqual( applyDisableDirectives({ - directives: [{ type: "disable-line", line: 1, column: 8, ruleId: "foo" }], + directives: [{ + parentComment: createParentComment(1, 8, 1, 35), + type: "disable-line", + line: 1, + column: 8, + ruleId: "foo" + }], problems: [{ line: 1, column: 2, ruleId: "foo" }], sourceCode: createSourceCode("first; // eslint-disable-line foo") }), @@ -374,7 +512,7 @@ describe("apply-disable-directives", () => { it("keeps problems on the current line that do not match the ruleId", () => { assert.deepStrictEqual( applyDisableDirectives({ - directives: [{ type: "disable-line", line: 1, column: 1, ruleId: "foo" }], + directives: [{ parentComment: createParentComment(1, 0, 1, 27), type: "disable-line", line: 1, column: 1, ruleId: "foo" }], problems: [{ line: 1, column: 2, ruleId: "not-foo" }], sourceCode: createSourceCode("// eslint-disable-line foo") }), @@ -386,8 +524,20 @@ describe("apply-disable-directives", () => { assert.deepStrictEqual( applyDisableDirectives({ directives: [ - { type: "disable", line: 1, column: 1, ruleId: null }, - { type: "disable-line", line: 1, column: 22, ruleId: "foo" } + { + parentComment: createParentComment(1, 0, 1, 21), + type: "disable", + line: 1, + column: 1, + ruleId: null + }, + { + parentComment: createParentComment(1, 25, 1, 49), + type: "disable-line", + line: 1, + column: 22, + ruleId: "foo" + } ], problems: [{ line: 1, column: 5, ruleId: "not-foo" }], sourceCode: createSourceCode("/* eslint-disable */ // eslint-disable-line foo") @@ -400,12 +550,48 @@ describe("apply-disable-directives", () => { assert.deepStrictEqual( applyDisableDirectives({ directives: [ - { type: "disable-line", line: 1, column: 8, ruleId: "foo" }, - { type: "disable-line", line: 2, column: 8, ruleId: "foo" }, - { type: "disable-line", line: 3, column: 8, ruleId: "foo" }, - { type: "disable-line", line: 4, column: 8, ruleId: "foo" }, - { type: "disable-line", line: 5, column: 8, ruleId: "foo" }, - { type: "disable-line", line: 6, column: 8, ruleId: "foo" } + { + parentComment: createParentComment(1, 8, 1, 35), + type: "disable-line", + line: 1, + column: 8, + ruleId: "foo" + }, + { + parentComment: createParentComment(2, 8, 2, 35), + type: "disable-line", + line: 2, + column: 8, + ruleId: "foo" + }, + { + parentComment: createParentComment(3, 8, 3, 35), + type: "disable-line", + line: 3, + column: 8, + ruleId: "foo" + }, + { + parentComment: createParentComment(4, 8, 4, 35), + type: "disable-line", + line: 4, + column: 8, + ruleId: "foo" + }, + { + parentComment: createParentComment(5, 8, 5, 35), + type: "disable-line", + line: 5, + column: 8, + ruleId: "foo" + }, + { + parentComment: createParentComment(6, 8, 6, 35), + type: "disable-line", + line: 6, + column: 8, + ruleId: "foo" + } ], problems: [{ line: 2, column: 1, ruleId: "foo" }], sourceCode: createSourceCode( @@ -421,7 +607,13 @@ describe("apply-disable-directives", () => { it("filters problems on the next line", () => { assert.deepStrictEqual( applyDisableDirectives({ - directives: [{ type: "disable-next-line", line: 1, column: 1, ruleId: null }], + directives: [{ + parentComment: createParentComment(1, 0, 1, 31), + type: "disable-next-line", + line: 1, + column: 1, + ruleId: null + }], problems: [{ line: 2, column: 3, ruleId: "foo" }], sourceCode: createSourceCode("// eslint-disable-next-line") }), @@ -432,7 +624,13 @@ describe("apply-disable-directives", () => { it("keeps problems on the same line", () => { assert.deepStrictEqual( applyDisableDirectives({ - directives: [{ type: "disable-next-line", line: 1, column: 1, ruleId: null }], + directives: [{ + parentComment: createParentComment(1, 0, 1, 31), + type: "disable-next-line", + line: 1, + column: 1, + ruleId: null + }], problems: [{ line: 1, column: 3, ruleId: "foo" }], sourceCode: createSourceCode("// eslint-disable-next-line") }), @@ -443,7 +641,13 @@ describe("apply-disable-directives", () => { it("keeps problems after the next line", () => { assert.deepStrictEqual( applyDisableDirectives({ - directives: [{ type: "disable-next-line", line: 1, column: 1, ruleId: null }], + directives: [{ + parentComment: createParentComment(1, 0, 1, 31), + type: "disable-next-line", + line: 1, + column: 1, + ruleId: null + }], problems: [{ line: 3, column: 3, ruleId: "foo" }], sourceCode: createSourceCode("// eslint-disable-next-line") }), @@ -481,7 +685,13 @@ describe("apply-disable-directives", () => { it("keeps problems on the next line that do not match the ruleId", () => { assert.deepStrictEqual( applyDisableDirectives({ - directives: [{ type: "disable-next-line", line: 1, column: 1, ruleId: "foo" }], + directives: [{ + parentComment: createParentComment(1, 0, 1, 31), + type: "disable-next-line", + line: 1, + column: 1, + ruleId: "foo" + }], problems: [{ line: 2, column: 1, ruleId: "not-foo" }], sourceCode: createSourceCode("// eslint-disable-next-line foo\n/* eslint-enable foo */") }), @@ -507,7 +717,12 @@ describe("apply-disable-directives", () => { it("Adds a problem for /* eslint-disable */", () => { assert.deepStrictEqual( applyDisableDirectives({ - directives: [{ type: "disable", line: 1, column: 1 }], + directives: [{ + parentComment: createParentComment(1, 0, 1, 20), + type: "disable", + line: 1, + column: 1 + }], problems: [], reportUnusedDisableDirectives: "error", sourceCode: createSourceCode("/* eslint-disable */") @@ -520,7 +735,7 @@ describe("apply-disable-directives", () => { column: 1, fix: { range: [0, 20], - text: "" + text: " " }, severity: 2, nodeType: null @@ -544,7 +759,13 @@ describe("apply-disable-directives", () => { it("Adds a problem for /* eslint-disable foo */", () => { assert.deepStrictEqual( applyDisableDirectives({ - directives: [{ type: "disable", line: 1, column: 1, ruleId: "foo" }], + directives: [{ + parentComment: createParentComment(1, 0, 1, 21), + type: "disable", + line: 1, + column: 1, + ruleId: "foo" + }], problems: [], reportUnusedDisableDirectives: "error", sourceCode: createSourceCode("/* eslint-disable foo */") @@ -556,8 +777,8 @@ describe("apply-disable-directives", () => { line: 1, column: 1, fix: { - range: [0, 24], - text: "" + range: [0, 21], + text: " " }, severity: 2, nodeType: null @@ -569,7 +790,13 @@ describe("apply-disable-directives", () => { it("Adds a problem for /* eslint-disable foo */ /* (problem from another rule) */", () => { assert.deepStrictEqual( applyDisableDirectives({ - directives: [{ type: "disable", line: 1, column: 1, ruleId: "foo" }], + directives: [{ + parentComment: createParentComment(1, 0, 1, 24), + type: "disable", + line: 1, + column: 1, + ruleId: "foo" + }], problems: [{ line: 1, column: 20, ruleId: "not-foo" }], reportUnusedDisableDirectives: "error", sourceCode: createSourceCode("/* eslint-disable foo */") @@ -582,7 +809,7 @@ describe("apply-disable-directives", () => { column: 1, fix: { range: [0, 24], - text: "" + text: " " }, severity: 2, nodeType: null @@ -600,8 +827,20 @@ describe("apply-disable-directives", () => { assert.deepStrictEqual( applyDisableDirectives({ directives: [ - { type: "disable", line: 1, column: 8, ruleId: null }, - { type: "enable", line: 1, column: 24, ruleId: "foo" } + { + parentComment: createParentComment(1, 0, 1, 21), + type: "disable", + line: 1, + column: 8, + ruleId: null + }, + { + parentComment: createParentComment(1, 0, 1, 21), + type: "enable", + line: 1, + column: 24, + ruleId: "foo" + } ], problems: [{ line: 1, column: 2, ruleId: "foo" }], reportUnusedDisableDirectives: "error", @@ -617,8 +856,8 @@ describe("apply-disable-directives", () => { ruleId: null, message: "Unused eslint-disable directive (no problems were reported).", fix: { - range: [7, 28], - text: "" + range: [0, 21], + text: " " }, line: 1, column: 8, @@ -633,8 +872,20 @@ describe("apply-disable-directives", () => { assert.deepStrictEqual( applyDisableDirectives({ directives: [ - { type: "disable", line: 1, column: 1, ruleId: null }, - { type: "enable", line: 1, column: 12, ruleId: null } + { + parentComment: createParentComment(1, 0, 1, 20), + type: "disable", + line: 1, + column: 1, + ruleId: null + }, + { + parentComment: createParentComment(1, 22, 1, 42), + type: "enable", + line: 1, + column: 12, + ruleId: null + } ], problems: [], reportUnusedDisableDirectives: "error", @@ -648,7 +899,7 @@ describe("apply-disable-directives", () => { column: 1, fix: { range: [0, 20], - text: "" + text: " " }, severity: 2, nodeType: null @@ -661,8 +912,20 @@ describe("apply-disable-directives", () => { assert.deepStrictEqual( applyDisableDirectives({ directives: [ - { type: "disable", line: 1, column: 1, ruleId: null }, - { type: "disable", line: 2, column: 1, ruleId: null } + { + parentComment: createParentComment(1, 0, 1, 21), + type: "disable", + line: 1, + column: 1, + ruleId: null + }, + { + parentComment: createParentComment(2, 0, 2, 21), + type: "disable", + line: 2, + column: 1, + ruleId: null + } ], problems: [], reportUnusedDisableDirectives: "error", @@ -676,7 +939,7 @@ describe("apply-disable-directives", () => { column: 1, fix: { range: [0, 21], - text: "" + text: " " }, severity: 2, nodeType: null @@ -687,8 +950,8 @@ describe("apply-disable-directives", () => { line: 2, column: 1, fix: { - range: [21, 41], - text: "" + range: [21, 42], + text: " " }, severity: 2, nodeType: null @@ -701,12 +964,24 @@ describe("apply-disable-directives", () => { assert.deepStrictEqual( applyDisableDirectives({ directives: [ - { type: "disable", line: 1, column: 1, ruleId: null }, - { type: "disable", line: 2, column: 1, ruleId: null } + { + parentComment: createParentComment(1, 0, 1, 21), + type: "disable", + line: 1, + column: 1, + ruleId: null + }, + { + parentComment: createParentComment(2, 0, 2, 21), + type: "disable", + line: 2, + column: 1, + ruleId: null + } ], problems: [{ line: 3, column: 1, ruleId: "foo" }], reportUnusedDisableDirectives: "error", - sourceCode: createSourceCode("/* eslint-disable */\n/* eslint-disable */") + sourceCode: createSourceCode("/* eslint-disable */\n/* eslint-disable */ /* (problem) */") }), [ { @@ -716,7 +991,7 @@ describe("apply-disable-directives", () => { column: 1, fix: { range: [0, 21], - text: "" + text: " " }, severity: 2, nodeType: null @@ -729,12 +1004,24 @@ describe("apply-disable-directives", () => { assert.deepStrictEqual( applyDisableDirectives({ directives: [ - { type: "disable", line: 1, column: 1, ruleId: "foo" }, - { type: "disable", line: 2, column: 1, ruleId: null } + { + parentComment: createParentComment(1, 0, 1, 21), + type: "disable", + line: 1, + column: 1, + ruleId: "foo" + }, + { + parentComment: createParentComment(2, 0, 2, 21), + type: "disable", + line: 2, + column: 1, + ruleId: null + } ], problems: [{ line: 3, column: 1, ruleId: "foo" }], reportUnusedDisableDirectives: "error", - sourceCode: createSourceCode("/* eslint-disable foo */\n/* eslint-disable */") + sourceCode: createSourceCode("/* eslint-disable foo */\n/* eslint-disable */ /* (problem from foo) */") }), [ { @@ -743,8 +1030,8 @@ describe("apply-disable-directives", () => { line: 1, column: 1, fix: { - range: [0, 25], - text: "" + range: [0, 21], + text: " " }, severity: 2, nodeType: null @@ -769,12 +1056,24 @@ describe("apply-disable-directives", () => { assert.deepStrictEqual( applyDisableDirectives({ directives: [ - { type: "disable", line: 1, column: 1, ruleId: null }, - { type: "disable", line: 2, column: 1, ruleId: "foo" } + { + parentComment: createParentComment(1, 0, 1, 21), + type: "disable", + line: 1, + column: 1, + ruleId: null + }, + { + parentComment: createParentComment(2, 0, 2, 24), + type: "disable", + line: 2, + column: 1, + ruleId: "foo" + } ], problems: [{ line: 3, column: 1, ruleId: "foo" }], reportUnusedDisableDirectives: "error", - sourceCode: createSourceCode("/* eslint-disable */\n/* eslint-disable foo */") + sourceCode: createSourceCode("/* eslint-disable */\n/* eslint-disable foo */ /* (problem from foo) */") }), [ { @@ -784,7 +1083,7 @@ describe("apply-disable-directives", () => { column: 1, fix: { range: [0, 21], - text: "" + text: " " }, severity: 2, nodeType: null @@ -797,12 +1096,24 @@ describe("apply-disable-directives", () => { assert.deepStrictEqual( applyDisableDirectives({ directives: [ - { type: "disable", line: 1, column: 1, ruleId: null }, - { type: "disable", line: 2, column: 1, ruleId: "foo" } + { + parentComment: createParentComment(1, 0, 1, 20), + type: "disable", + line: 1, + column: 1, + ruleId: null + }, + { + parentComment: createParentComment(2, 0, 2, 24), + type: "disable", + line: 2, + column: 1, + ruleId: "foo" + } ], problems: [{ line: 3, column: 1, ruleId: "bar" }], reportUnusedDisableDirectives: "error", - sourceCode: createSourceCode("/* eslint-disable */\n/* eslint-disable foo */") + sourceCode: createSourceCode("/* eslint-disable */\n/* eslint-disable foo */ /* (problem from another rule) */") }), [ { @@ -812,7 +1123,7 @@ describe("apply-disable-directives", () => { column: 1, fix: { range: [21, 45], - text: "" + text: " " }, severity: 2, nodeType: null From 0922789fcde24c6d13efac1be64ef88d80359104 Mon Sep 17 00:00:00 2001 From: Josh Goldberg Date: Tue, 29 Jun 2021 20:27:36 -0400 Subject: [PATCH 08/31] Fixed up some tests --- tests/lib/linter/linter.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/lib/linter/linter.js b/tests/lib/linter/linter.js index a29165777df..97c9a4b699b 100644 --- a/tests/lib/linter/linter.js +++ b/tests/lib/linter/linter.js @@ -3301,7 +3301,7 @@ var a = "test2"; column: 1, fix: { range: [0, 20], - text: "" + text: " " }, severity: 2, nodeType: null @@ -3321,7 +3321,7 @@ var a = "test2"; column: 1, fix: { range: [0, 20], - text: "" + text: " " }, severity: 2, nodeType: null @@ -3341,7 +3341,7 @@ var a = "test2"; column: 1, fix: { range: [0, 20], - text: "" + text: " " }, severity: 1, nodeType: null @@ -3361,7 +3361,7 @@ var a = "test2"; column: 1, fix: { range: [0, 20], - text: "" + text: " " }, severity: 1, nodeType: null From 187d39636cf38867369f7d70aa168578e83b3282 Mon Sep 17 00:00:00 2001 From: Josh Goldberg Date: Tue, 29 Jun 2021 20:34:45 -0400 Subject: [PATCH 09/31] unprocessedDirective.parentComment --- lib/linter/apply-disable-directives.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/linter/apply-disable-directives.js b/lib/linter/apply-disable-directives.js index 90438fd2940..ec039e38298 100644 --- a/lib/linter/apply-disable-directives.js +++ b/lib/linter/apply-disable-directives.js @@ -32,8 +32,8 @@ function groupByParentComment(directives) { for (const directive of directives) { if (state) { - if (directive.parentComment === state.parentComment) { - state.group.push(directive); + if (directive.unprocessedDirective.parentComment === state.parentComment) { + state.group.push(directive.unprocessedDirective); } else { groups.push(state.group); state = null; From 89d15c82e78841da2af1b03233903f6f3f739c4e Mon Sep 17 00:00:00 2001 From: Josh Goldberg Date: Tue, 29 Jun 2021 20:44:04 -0400 Subject: [PATCH 10/31] Avoided rescan by passing commentToken in parentComment --- lib/linter/apply-disable-directives.js | 19 +++++++++---------- lib/linter/linter.js | 12 ++++++------ tests/lib/linter/apply-disable-directives.js | 18 ++++++++++-------- 3 files changed, 25 insertions(+), 24 deletions(-) diff --git a/lib/linter/apply-disable-directives.js b/lib/linter/apply-disable-directives.js index ec039e38298..e6f60d6c376 100644 --- a/lib/linter/apply-disable-directives.js +++ b/lib/linter/apply-disable-directives.js @@ -56,13 +56,12 @@ function groupByParentComment(directives) { /** * Creates removal details for a set of directives within the same comment. * @param {Directive[]} directives Unused directives to be removed. - * @param {ParentComment} parentComment Location and rule IDs for the comment to delete. + * @param {Token} commentToken The backing Comment token. * @param {SourceCode} sourceCode A SourceCode object for the given text. * @returns {{ fix, position, ruleIds }[]} Details for later creation of output Problems. */ -function createIndividualDirectivesRemoval(directives, parentComment, sourceCode) { - const commentStart = sourceCode.lineStartIndices[parentComment.loc.start.line - 1] + parentComment.loc.start.column; - const commentToken = sourceCode.getTokenByRangeStart(commentStart, { includeComments: true }); +function createIndividualDirectivesRemoval(directives, commentToken, sourceCode) { + const commentStart = sourceCode.lineStartIndices[commentToken.loc.start.line - 1] + commentToken.loc.start.column; const commentContents = commentToken.value.split("--")[0].trimRight(); let lastRuleEnd = 0; @@ -87,14 +86,14 @@ function createIndividualDirectivesRemoval(directives, parentComment, sourceCode /** * Creates a description of deleting an entire unused disable comment. * @param {Directive[]} directives Unused directives to be removed. - * @param {ParentComment} parentComment Location and rule IDs for the comment to delete. + * @param {Token} commentToken The backing Comment token. * @param {SourceCode} sourceCode A SourceCode object for the given text. * @returns {{ fix, position, ruleIds }} Details for later creation of an output Problem. */ -function createCommentRemoval(directives, parentComment, sourceCode) { +function createCommentRemoval(directives, commentToken, sourceCode) { const range = [ - sourceCode.lineStartIndices[parentComment.loc.start.line - 1] + parentComment.loc.start.column, - sourceCode.lineStartIndices[parentComment.loc.end.line - 1] + parentComment.loc.end.column + sourceCode.lineStartIndices[commentToken.loc.start.line - 1] + commentToken.loc.start.column, + sourceCode.lineStartIndices[commentToken.loc.end.line - 1] + commentToken.loc.end.column ]; const ruleIds = directives.filter(directive => directive.ruleId).map(directive => `'${directive.ruleId}'`); @@ -144,8 +143,8 @@ function processUnusedDisableDirectives(allDirectives, sourceCode) { } return remainingRuleIds.size - ? createIndividualDirectivesRemoval(directives, parentComment, sourceCode) - : [createCommentRemoval(directives, parentComment, sourceCode)]; + ? createIndividualDirectivesRemoval(directives, parentComment.commentToken, sourceCode) + : [createCommentRemoval(directives, parentComment.commentToken, sourceCode)]; } ); } diff --git a/lib/linter/linter.js b/lib/linter/linter.js index debcdb03534..ce13ca32bcb 100644 --- a/lib/linter/linter.js +++ b/lib/linter/linter.js @@ -240,14 +240,14 @@ function createLintingProblem(options) { * Creates a collection of disable directives from a comment * @param {Object} options to create disable directives * @param {("disable"|"enable"|"disable-line"|"disable-next-line")} options.type The type of directive comment - * @param {{line: number, column: number}} options.loc The 0-based location of the comment token + * @param {token} options.commentToken The Comment token * @param {string} options.value The value after the directive in the comment * comment specified no specific rules, so it applies to all rules (e.g. `eslint-disable`) * @param {function(string): {create: Function}} options.ruleMapper A map from rule IDs to defined rules * @returns {Object} Directives and problems from the comment */ function createDisableDirectives(options) { - const { type, loc, value, ruleMapper } = options; + const { commentToken, type, value, ruleMapper } = options; const ruleIds = Object.keys(commentParser.parseListConfig(value)); const directiveRules = ruleIds.length ? ruleIds : [null]; const result = { @@ -255,15 +255,15 @@ function createDisableDirectives(options) { directiveProblems: [] // problems in directives }; - const parentComment = { loc, ruleIds }; + const parentComment = { commentToken, ruleIds }; for (const ruleId of directiveRules) { // push to directives, if the rule is defined(including null, e.g. /*eslint enable*/) if (ruleId === null || ruleMapper(ruleId) !== null) { - result.directives.push({ parentComment, type, line: loc.start.line, column: loc.start.column + 1, ruleId }); + result.directives.push({ parentComment, type, line: commentToken.loc.start.line, column: commentToken.loc.start.column + 1, ruleId }); } else { - result.directiveProblems.push(createLintingProblem({ ruleId, loc })); + result.directiveProblems.push(createLintingProblem({ ruleId, loc: commentToken.loc })); } } return result; @@ -344,7 +344,7 @@ function getDirectiveComments(filename, ast, ruleMapper, warnInlineConfig) { case "eslint-disable-next-line": case "eslint-disable-line": { const directiveType = directiveText.slice("eslint-".length); - const options = { type: directiveType, loc: comment.loc, value: directiveValue, ruleMapper }; + const options = { commentToken: comment, type: directiveType, value: directiveValue, ruleMapper }; const { directives, directiveProblems } = createDisableDirectives(options); disableDirectives.push(...directives); diff --git a/tests/lib/linter/apply-disable-directives.js b/tests/lib/linter/apply-disable-directives.js index c3cb70a23c8..0336a37096d 100644 --- a/tests/lib/linter/apply-disable-directives.js +++ b/tests/lib/linter/apply-disable-directives.js @@ -38,14 +38,16 @@ function createSourceCode(text) { */ function createParentComment(startLine, startColumn, endLine, endColumn, ruleIds = []) { return { - loc: { - start: { - line: startLine, - column: startColumn - }, - end: { - line: endLine, - column: endColumn + commentToken: { + loc: { + start: { + line: startLine, + column: startColumn + }, + end: { + line: endLine, + column: endColumn + } } }, ruleIds From 14129040f40df6e94d2bd1b460ce89b536e298b0 Mon Sep 17 00:00:00 2001 From: Josh Goldberg Date: Tue, 29 Jun 2021 20:50:40 -0400 Subject: [PATCH 11/31] A lil edge case --- lib/linter/apply-disable-directives.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/linter/apply-disable-directives.js b/lib/linter/apply-disable-directives.js index e6f60d6c376..97a3becfac1 100644 --- a/lib/linter/apply-disable-directives.js +++ b/lib/linter/apply-disable-directives.js @@ -41,7 +41,7 @@ function groupByParentComment(directives) { } state = { - group: [directive], + group: [directive.unprocessedDirective], parentComment: directive.parentComment }; } From 4f6905207b2e63d5bbd11dc190cd31c6bd42b9a4 Mon Sep 17 00:00:00 2001 From: Josh Goldberg Date: Tue, 29 Jun 2021 20:55:26 -0400 Subject: [PATCH 12/31] Fix directive grouping the other direction --- lib/linter/apply-disable-directives.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/linter/apply-disable-directives.js b/lib/linter/apply-disable-directives.js index 97a3becfac1..1cc15a2849b 100644 --- a/lib/linter/apply-disable-directives.js +++ b/lib/linter/apply-disable-directives.js @@ -32,8 +32,8 @@ function groupByParentComment(directives) { for (const directive of directives) { if (state) { - if (directive.unprocessedDirective.parentComment === state.parentComment) { - state.group.push(directive.unprocessedDirective); + if (directive.unprocessedDirective.parentComment === state.unprocessedDirective.parentComment) { + state.group.push(directive); } else { groups.push(state.group); state = null; @@ -41,7 +41,7 @@ function groupByParentComment(directives) { } state = { - group: [directive.unprocessedDirective], + group: [directive], parentComment: directive.parentComment }; } From 03bfb8ee46a441fed7cf3260f3e6da63b780340a Mon Sep 17 00:00:00 2001 From: Josh Goldberg Date: Tue, 29 Jun 2021 21:00:28 -0400 Subject: [PATCH 13/31] Incorrect state directive reference --- lib/linter/apply-disable-directives.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/linter/apply-disable-directives.js b/lib/linter/apply-disable-directives.js index 1cc15a2849b..c73fcbc15bb 100644 --- a/lib/linter/apply-disable-directives.js +++ b/lib/linter/apply-disable-directives.js @@ -32,7 +32,7 @@ function groupByParentComment(directives) { for (const directive of directives) { if (state) { - if (directive.unprocessedDirective.parentComment === state.unprocessedDirective.parentComment) { + if (directive.unprocessedDirective.parentComment === state.parentComment) { state.group.push(directive); } else { groups.push(state.group); From f60e0be1c8cc847008e32e4adf53726fdf6c7825 Mon Sep 17 00:00:00 2001 From: Josh Goldberg Date: Wed, 14 Jul 2021 14:11:20 -0400 Subject: [PATCH 14/31] Simplified parentComment / commentToken range --- lib/linter/apply-disable-directives.js | 35 ++--- lib/linter/linter.js | 3 +- tests/lib/linter/apply-disable-directives.js | 156 +++++++++---------- 3 files changed, 85 insertions(+), 109 deletions(-) diff --git a/lib/linter/apply-disable-directives.js b/lib/linter/apply-disable-directives.js index c73fcbc15bb..7a995cb1456 100644 --- a/lib/linter/apply-disable-directives.js +++ b/lib/linter/apply-disable-directives.js @@ -57,12 +57,11 @@ function groupByParentComment(directives) { * Creates removal details for a set of directives within the same comment. * @param {Directive[]} directives Unused directives to be removed. * @param {Token} commentToken The backing Comment token. - * @param {SourceCode} sourceCode A SourceCode object for the given text. * @returns {{ fix, position, ruleIds }[]} Details for later creation of output Problems. */ -function createIndividualDirectivesRemoval(directives, commentToken, sourceCode) { - const commentStart = sourceCode.lineStartIndices[commentToken.loc.start.line - 1] + commentToken.loc.start.column; +function createIndividualDirectivesRemoval(directives, commentToken) { const commentContents = commentToken.value.split("--")[0].trimRight(); + const fixStart = commentToken.range[0] + 2; let lastRuleEnd = 0; return directives.map(directive => { @@ -73,8 +72,8 @@ function createIndividualDirectivesRemoval(directives, commentToken, sourceCode) description: directive.ruleId, fix: { range: [ - commentStart + 2 + ruleStart, - commentStart + 2 + ruleEnd + (commentContents[ruleEnd] === "," ? 1 : 0) + fixStart + ruleStart, + fixStart + ruleEnd + (commentContents[ruleEnd] === "," ? 1 : 0) ], text: "" }, @@ -87,14 +86,10 @@ function createIndividualDirectivesRemoval(directives, commentToken, sourceCode) * Creates a description of deleting an entire unused disable comment. * @param {Directive[]} directives Unused directives to be removed. * @param {Token} commentToken The backing Comment token. - * @param {SourceCode} sourceCode A SourceCode object for the given text. * @returns {{ fix, position, ruleIds }} Details for later creation of an output Problem. */ -function createCommentRemoval(directives, commentToken, sourceCode) { - const range = [ - sourceCode.lineStartIndices[commentToken.loc.start.line - 1] + commentToken.loc.start.column, - sourceCode.lineStartIndices[commentToken.loc.end.line - 1] + commentToken.loc.end.column - ]; +function createCommentRemoval(directives, commentToken) { + const { range } = commentToken; const ruleIds = directives.filter(directive => directive.ruleId).map(directive => `'${directive.ruleId}'`); return { @@ -126,10 +121,9 @@ function flatMap(array, fn) { /** * Parses details from directives to create output Problems. * @param {Directive[]} allDirectives Unused directives to be removed. - * @param {SourceCode} sourceCode A SourceCode object for the given text. * @returns {{ fix, position, ruleIds }} Details for later creation of output Problems. */ -function processUnusedDisableDirectives(allDirectives, sourceCode) { +function processUnusedDisableDirectives(allDirectives) { const directiveGroups = groupByParentComment(allDirectives); return flatMap( @@ -143,8 +137,8 @@ function processUnusedDisableDirectives(allDirectives, sourceCode) { } return remainingRuleIds.size - ? createIndividualDirectivesRemoval(directives, parentComment.commentToken, sourceCode) - : [createCommentRemoval(directives, parentComment.commentToken, sourceCode)]; + ? createIndividualDirectivesRemoval(directives, parentComment.commentToken) + : [createCommentRemoval(directives, parentComment.commentToken)]; } ); } @@ -218,7 +212,7 @@ function applyDirectives(options) { const unusedDisableDirectivesToReport = options.directives .filter(directive => directive.type === "disable" && !usedDisableDirectives.has(directive)); - const processed = processUnusedDisableDirectives(unusedDisableDirectivesToReport, options.sourceCode); + const processed = processUnusedDisableDirectives(unusedDisableDirectivesToReport); const unusedDisableDirectives = processed .map(({ description, fix, position }) => ({ @@ -251,11 +245,10 @@ function applyDirectives(options) { * @param {{ruleId: (string|null), line: number, column: number}[]} options.problems * A list of problems reported by rules, sorted by increasing location in the file, with one-based columns. * @param {"off" | "warn" | "error"} options.reportUnusedDisableDirectives If `"warn"` or `"error"`, adds additional problems for unused directives - * @param {SourceCode} options.sourceCode A SourceCode object for the given text * @returns {{ruleId: (string|null), line: number, column: number}[]} * A list of reported problems that were not disabled by the directive comments. */ -module.exports = ({ directives, problems, reportUnusedDisableDirectives = "off", sourceCode }) => { +module.exports = ({ directives, problems, reportUnusedDisableDirectives = "off" }) => { const blockDirectives = directives .filter(directive => directive.type === "disable" || directive.type === "enable") .map(directive => Object.assign({}, directive, { unprocessedDirective: directive })) @@ -287,14 +280,12 @@ module.exports = ({ directives, problems, reportUnusedDisableDirectives = "off", const blockDirectivesResult = applyDirectives({ problems, directives: blockDirectives, - reportUnusedDisableDirectives, - sourceCode + reportUnusedDisableDirectives }); const lineDirectivesResult = applyDirectives({ problems: blockDirectivesResult.problems, directives: lineDirectives, - reportUnusedDisableDirectives, - sourceCode + reportUnusedDisableDirectives }); return reportUnusedDisableDirectives !== "off" diff --git a/lib/linter/linter.js b/lib/linter/linter.js index ce13ca32bcb..9b41f0677c3 100644 --- a/lib/linter/linter.js +++ b/lib/linter/linter.js @@ -1209,8 +1209,7 @@ class Linter { problems: lintingProblems .concat(commentDirectives.problems) .sort((problemA, problemB) => problemA.line - problemB.line || problemA.column - problemB.column), - reportUnusedDisableDirectives: options.reportUnusedDisableDirectives, - sourceCode + reportUnusedDisableDirectives: options.reportUnusedDisableDirectives }); } diff --git a/tests/lib/linter/apply-disable-directives.js b/tests/lib/linter/apply-disable-directives.js index 0336a37096d..d1d55e3613e 100644 --- a/tests/lib/linter/apply-disable-directives.js +++ b/tests/lib/linter/apply-disable-directives.js @@ -29,27 +29,13 @@ function createSourceCode(text) { /** * Creates a ParentComment for a given range. - * @param {number} startLine loc.start.line value - * @param {number} startColumn loc.start.column value - * @param {number} endLine loc.end.line value - * @param {number} endColumn loc.end.column value + * @param {[number, number]} range total range of the comment * @param {string[]} ruleIds Rule IDs reported in the comment * @returns {ParentComment} Test-ready ParentComment object. */ -function createParentComment(startLine, startColumn, endLine, endColumn, ruleIds = []) { +function createParentComment(range, ruleIds = []) { return { - commentToken: { - loc: { - start: { - line: startLine, - column: startColumn - }, - end: { - line: endLine, - column: endColumn - } - } - }, + commentToken: { range }, ruleIds }; } @@ -59,7 +45,7 @@ describe("apply-disable-directives", () => { it("keeps problems before the comment on the same line", () => { assert.deepStrictEqual( applyDisableDirectives({ - directives: [{ parentComment: createParentComment(1, 7, 1, 27), type: "disable", line: 1, column: 8, ruleId: null }], + directives: [{ parentComment: createParentComment([0, 7]), type: "disable", line: 1, column: 8, ruleId: null }], problems: [{ line: 1, column: 7, ruleId: "foo" }], sourceCode: createSourceCode("first; /* eslint-disable */") }), @@ -70,7 +56,7 @@ describe("apply-disable-directives", () => { it("keeps problems on a previous line before the comment", () => { assert.deepStrictEqual( applyDisableDirectives({ - directives: [{ parentComment: createParentComment(2, 1, 2, 20), type: "disable", line: 2, column: 1, ruleId: null }], + directives: [{ parentComment: createParentComment([21, 27]), type: "disable", line: 2, column: 1, ruleId: null }], problems: [{ line: 1, column: 10, ruleId: "foo" }], sourceCode: createSourceCode("\n/* eslint-disable*/") }), @@ -138,7 +124,7 @@ describe("apply-disable-directives", () => { assert.deepStrictEqual( applyDisableDirectives({ directives: [{ - parentComment: createParentComment(1, 0, 1, 25), + parentComment: createParentComment([26, 29]), type: "disable", line: 1, column: 1, @@ -155,7 +141,7 @@ describe("apply-disable-directives", () => { assert.deepStrictEqual( applyDisableDirectives({ directives: [{ - parentComment: createParentComment(1, 8, 1, 30), + parentComment: createParentComment([7, 31]), type: "disable", line: 1, column: 8, @@ -175,14 +161,14 @@ describe("apply-disable-directives", () => { applyDisableDirectives({ directives: [ { - parentComment: createParentComment(1, 0, 1, 21), + parentComment: createParentComment([0, 26]), type: "disable", line: 1, column: 1, ruleId: null }, { - parentComment: createParentComment(1, 26, 1, 45), + parentComment: createParentComment([27, 45]), type: "enable", line: 1, column: 26, @@ -201,14 +187,14 @@ describe("apply-disable-directives", () => { applyDisableDirectives({ directives: [ { - parentComment: createParentComment(1, 0, 1, 26), + parentComment: createParentComment([0, 25]), type: "disable", line: 1, column: 1, ruleId: null }, { - parentComment: createParentComment(1, 26, 1, 41), + parentComment: createParentComment([26, 40]), type: "enable", line: 1, column: 26, @@ -241,21 +227,21 @@ describe("apply-disable-directives", () => { applyDisableDirectives({ directives: [ { - parentComment: createParentComment(1, 0, 1, 21), + parentComment: createParentComment([0, 20]), type: "disable", line: 1, column: 1, ruleId: null }, { - parentComment: createParentComment(1, 26, 1, 45), + parentComment: createParentComment([26, 44]), type: "enable", line: 1, column: 26, ruleId: "foo" }, { - parentComment: createParentComment(2, 0, 2, 21), + parentComment: createParentComment([45, 63]), type: "disable", line: 2, column: 1, @@ -274,21 +260,21 @@ describe("apply-disable-directives", () => { applyDisableDirectives({ directives: [ { - parentComment: createParentComment(1, 0, 1, 21), + parentComment: createParentComment([0, 20]), type: "disable", line: 1, column: 1, ruleId: null }, { - parentComment: createParentComment(1, 26, 1, 45), + parentComment: createParentComment([21, 44]), type: "enable", line: 1, column: 26, ruleId: "foo" }, { - parentComment: createParentComment(2, 0, 2, 21), + parentComment: createParentComment([45, 63]), type: "disable", line: 2, column: 1, @@ -307,14 +293,14 @@ describe("apply-disable-directives", () => { applyDisableDirectives({ directives: [ { - parentComment: createParentComment(1, 0, 1, 21), + parentComment: createParentComment([0, 20]), type: "disable", line: 1, column: 1, ruleId: "foo" }, { - parentComment: createParentComment(1, 26, 1, 45), + parentComment: createParentComment([25, 44]), type: "enable", line: 1, column: 26, @@ -335,14 +321,14 @@ describe("apply-disable-directives", () => { applyDisableDirectives({ directives: [ { - parentComment: createParentComment(1, 0, 1, 21), + parentComment: createParentComment([0, 20]), type: "disable", line: 1, column: 1, ruleId: null }, { - parentComment: createParentComment(2, 0, 2, 25), + parentComment: createParentComment([21, 44]), type: "enable", line: 2, column: 1, @@ -361,14 +347,14 @@ describe("apply-disable-directives", () => { applyDisableDirectives({ directives: [ { - parentComment: createParentComment(1, 0, 1, 21), + parentComment: createParentComment([0, 20]), type: "disable", line: 1, column: 1, ruleId: null }, { - parentComment: createParentComment(2, 0, 2, 25), + parentComment: createParentComment([21, 44]), type: "enable", line: 2, column: 1, @@ -428,7 +414,7 @@ describe("apply-disable-directives", () => { assert.deepStrictEqual( applyDisableDirectives({ directives: [{ - parentComment: createParentComment(2, 0, 1, 21), + parentComment: createParentComment([6, 27]), type: "disable-line", line: 2, column: 1, @@ -445,7 +431,7 @@ describe("apply-disable-directives", () => { assert.deepStrictEqual( applyDisableDirectives({ directives: [{ - parentComment: createParentComment(1, 8, 1, 29), + parentComment: createParentComment([7, 28]), type: "disable-line", line: 1, column: 8, @@ -462,7 +448,7 @@ describe("apply-disable-directives", () => { assert.deepStrictEqual( applyDisableDirectives({ directives: [{ - parentComment: createParentComment(1, 8, 1, 29), + parentComment: createParentComment([7, 28]), type: "disable-line", line: 1, column: 8, @@ -479,7 +465,7 @@ describe("apply-disable-directives", () => { assert.deepStrictEqual( applyDisableDirectives({ directives: [{ - parentComment: createParentComment(1, 8, 1, 35), + parentComment: createParentComment([7, 34]), type: "disable-line", line: 1, column: 8, @@ -498,7 +484,7 @@ describe("apply-disable-directives", () => { assert.deepStrictEqual( applyDisableDirectives({ directives: [{ - parentComment: createParentComment(1, 8, 1, 35), + parentComment: createParentComment([7, 34]), type: "disable-line", line: 1, column: 8, @@ -514,7 +500,7 @@ describe("apply-disable-directives", () => { it("keeps problems on the current line that do not match the ruleId", () => { assert.deepStrictEqual( applyDisableDirectives({ - directives: [{ parentComment: createParentComment(1, 0, 1, 27), type: "disable-line", line: 1, column: 1, ruleId: "foo" }], + directives: [{ parentComment: createParentComment([0, 27]), type: "disable-line", line: 1, column: 1, ruleId: "foo" }], problems: [{ line: 1, column: 2, ruleId: "not-foo" }], sourceCode: createSourceCode("// eslint-disable-line foo") }), @@ -527,14 +513,14 @@ describe("apply-disable-directives", () => { applyDisableDirectives({ directives: [ { - parentComment: createParentComment(1, 0, 1, 21), + parentComment: createParentComment([0, 21]), type: "disable", line: 1, column: 1, ruleId: null }, { - parentComment: createParentComment(1, 25, 1, 49), + parentComment: createParentComment([24, 28]), type: "disable-line", line: 1, column: 22, @@ -553,42 +539,42 @@ describe("apply-disable-directives", () => { applyDisableDirectives({ directives: [ { - parentComment: createParentComment(1, 8, 1, 35), + parentComment: createParentComment([7, 34]), type: "disable-line", line: 1, column: 8, ruleId: "foo" }, { - parentComment: createParentComment(2, 8, 2, 35), + parentComment: createParentComment([38, 73]), type: "disable-line", line: 2, column: 8, ruleId: "foo" }, { - parentComment: createParentComment(3, 8, 3, 35), + parentComment: createParentComment([76, 111]), type: "disable-line", line: 3, column: 8, ruleId: "foo" }, { - parentComment: createParentComment(4, 8, 4, 35), + parentComment: createParentComment([114, 149]), type: "disable-line", line: 4, column: 8, ruleId: "foo" }, { - parentComment: createParentComment(5, 8, 5, 35), + parentComment: createParentComment([152, 187]), type: "disable-line", line: 5, column: 8, ruleId: "foo" }, { - parentComment: createParentComment(6, 8, 6, 35), + parentComment: createParentComment([190, 225]), type: "disable-line", line: 6, column: 8, @@ -610,7 +596,7 @@ describe("apply-disable-directives", () => { assert.deepStrictEqual( applyDisableDirectives({ directives: [{ - parentComment: createParentComment(1, 0, 1, 31), + parentComment: createParentComment([0, 31]), type: "disable-next-line", line: 1, column: 1, @@ -627,7 +613,7 @@ describe("apply-disable-directives", () => { assert.deepStrictEqual( applyDisableDirectives({ directives: [{ - parentComment: createParentComment(1, 0, 1, 31), + parentComment: createParentComment([0, 31]), type: "disable-next-line", line: 1, column: 1, @@ -644,7 +630,7 @@ describe("apply-disable-directives", () => { assert.deepStrictEqual( applyDisableDirectives({ directives: [{ - parentComment: createParentComment(1, 0, 1, 31), + parentComment: createParentComment([0, 31]), type: "disable-next-line", line: 1, column: 1, @@ -688,7 +674,7 @@ describe("apply-disable-directives", () => { assert.deepStrictEqual( applyDisableDirectives({ directives: [{ - parentComment: createParentComment(1, 0, 1, 31), + parentComment: createParentComment([0, 31]), type: "disable-next-line", line: 1, column: 1, @@ -720,7 +706,7 @@ describe("apply-disable-directives", () => { assert.deepStrictEqual( applyDisableDirectives({ directives: [{ - parentComment: createParentComment(1, 0, 1, 20), + parentComment: createParentComment([0, 20]), type: "disable", line: 1, column: 1 @@ -762,7 +748,7 @@ describe("apply-disable-directives", () => { assert.deepStrictEqual( applyDisableDirectives({ directives: [{ - parentComment: createParentComment(1, 0, 1, 21), + parentComment: createParentComment([0, 21]), type: "disable", line: 1, column: 1, @@ -793,7 +779,7 @@ describe("apply-disable-directives", () => { assert.deepStrictEqual( applyDisableDirectives({ directives: [{ - parentComment: createParentComment(1, 0, 1, 24), + parentComment: createParentComment([0, 24]), type: "disable", line: 1, column: 1, @@ -830,14 +816,14 @@ describe("apply-disable-directives", () => { applyDisableDirectives({ directives: [ { - parentComment: createParentComment(1, 0, 1, 21), + parentComment: createParentComment([0, 21]), type: "disable", line: 1, column: 8, ruleId: null }, { - parentComment: createParentComment(1, 0, 1, 21), + parentComment: createParentComment([0, 21]), type: "enable", line: 1, column: 24, @@ -875,14 +861,14 @@ describe("apply-disable-directives", () => { applyDisableDirectives({ directives: [ { - parentComment: createParentComment(1, 0, 1, 20), + parentComment: createParentComment([0, 20]), type: "disable", line: 1, column: 1, ruleId: null }, { - parentComment: createParentComment(1, 22, 1, 42), + parentComment: createParentComment([21, 41]), type: "enable", line: 1, column: 12, @@ -915,14 +901,14 @@ describe("apply-disable-directives", () => { applyDisableDirectives({ directives: [ { - parentComment: createParentComment(1, 0, 1, 21), + parentComment: createParentComment([0, 21]), type: "disable", line: 1, column: 1, ruleId: null }, { - parentComment: createParentComment(2, 0, 2, 21), + parentComment: createParentComment([21, 42]), type: "disable", line: 2, column: 1, @@ -967,14 +953,14 @@ describe("apply-disable-directives", () => { applyDisableDirectives({ directives: [ { - parentComment: createParentComment(1, 0, 1, 21), + parentComment: createParentComment([0, 21]), type: "disable", line: 1, column: 1, ruleId: null }, { - parentComment: createParentComment(2, 0, 2, 21), + parentComment: createParentComment([22, 45]), type: "disable", line: 2, column: 1, @@ -1007,14 +993,14 @@ describe("apply-disable-directives", () => { applyDisableDirectives({ directives: [ { - parentComment: createParentComment(1, 0, 1, 21), + parentComment: createParentComment([0, 21]), type: "disable", line: 1, column: 1, ruleId: "foo" }, { - parentComment: createParentComment(2, 0, 2, 21), + parentComment: createParentComment([22, 45]), type: "disable", line: 2, column: 1, @@ -1059,14 +1045,14 @@ describe("apply-disable-directives", () => { applyDisableDirectives({ directives: [ { - parentComment: createParentComment(1, 0, 1, 21), + parentComment: createParentComment([0, 21]), type: "disable", line: 1, column: 1, ruleId: null }, { - parentComment: createParentComment(2, 0, 2, 24), + parentComment: createParentComment([22, 45]), type: "disable", line: 2, column: 1, @@ -1099,14 +1085,14 @@ describe("apply-disable-directives", () => { applyDisableDirectives({ directives: [ { - parentComment: createParentComment(1, 0, 1, 20), + parentComment: createParentComment([0, 20]), type: "disable", line: 1, column: 1, ruleId: null }, { - parentComment: createParentComment(2, 0, 2, 24), + parentComment: createParentComment([21, 45]), type: "disable", line: 2, column: 1, @@ -1139,14 +1125,14 @@ describe("apply-disable-directives", () => { applyDisableDirectives({ directives: [ { - parentComment: createParentComment(1, 0, 1, 20), + parentComment: createParentComment([0, 20]), type: "disable", line: 1, column: 1, ruleId: "foo" }, { - parentComment: createParentComment(1, 26, 1, 46), + parentComment: createParentComment([25, 46]), type: "enable", line: 1, column: 26, @@ -1184,14 +1170,14 @@ describe("apply-disable-directives", () => { applyDisableDirectives({ directives: [ { - parentComment: createParentComment(1, 0, 1, 24), + parentComment: createParentComment([0, 24]), type: "disable", line: 1, column: 1, ruleId: "foo" }, { - parentComment: createParentComment(1, 0, 25, 49), + parentComment: createParentComment([25, 49]), type: "enable", line: 1, column: 26, @@ -1229,21 +1215,21 @@ describe("apply-disable-directives", () => { applyDisableDirectives({ directives: [ { - parentComment: createParentComment(1, 0, 1, 21), + parentComment: createParentComment([0, 21]), type: "disable", line: 1, column: 1, ruleId: null }, { - parentComment: createParentComment(2, 0, 2, 24), + parentComment: createParentComment([22, 45]), type: "disable", line: 2, column: 1, ruleId: "foo" }, { - parentComment: createParentComment(3, 0, 3, 23), + parentComment: createParentComment([46, 69]), type: "enable", line: 3, column: 1, @@ -1273,7 +1259,7 @@ describe("apply-disable-directives", () => { line: 2, column: 1, fix: { - range: [21, 45], + range: [22, 45], text: " " }, severity: 2, @@ -1292,7 +1278,7 @@ describe("apply-disable-directives", () => { assert.deepStrictEqual( applyDisableDirectives({ directives: [{ - parentComment: createParentComment(1, 0, 1, 22), + parentComment: createParentComment([0, 22]), type: "disable-line", line: 1, column: 1, @@ -1336,7 +1322,7 @@ describe("apply-disable-directives", () => { assert.deepStrictEqual( applyDisableDirectives({ directives: [{ - parentComment: createParentComment(1, 0, 1, 27), + parentComment: createParentComment([0, 27]), type: "disable-next-line", line: 1, column: 1, @@ -1379,8 +1365,8 @@ describe("apply-disable-directives", () => { assert.deepStrictEqual( applyDisableDirectives({ directives: [ - { parentComment: createParentComment(1, 0, 1, 20), type: "disable", line: 1, column: 1, ruleId: null }, - { parentComment: createParentComment(1, 20, 1, 43), type: "disable-line", line: 1, column: 22, ruleId: null } + { parentComment: createParentComment([0, 20]), type: "disable", line: 1, column: 1, ruleId: null }, + { parentComment: createParentComment([20, 43]), type: "disable-line", line: 1, column: 22, ruleId: null } ], problems: [], reportUnusedDisableDirectives: "error", @@ -1418,7 +1404,7 @@ describe("apply-disable-directives", () => { it("Does not add problems when reportUnusedDisableDirectives: \"off\" is used", () => { assert.deepStrictEqual( applyDisableDirectives({ - directives: [{ parentComment: createParentComment(1, 1, 1, 27), type: "disable-next-line", line: 1, column: 1, ruleId: null }], + directives: [{ parentComment: createParentComment([0, 27]), type: "disable-next-line", line: 1, column: 1, ruleId: null }], problems: [], reportUnusedDisableDirectives: "off", sourceCode: createSourceCode("// eslint-disable-next-line") From e834d3d49b376d3d025cc4a776fa279cc426e4fb Mon Sep 17 00:00:00 2001 From: Josh Goldberg Date: Wed, 14 Jul 2021 14:13:10 -0400 Subject: [PATCH 15/31] Use Map in lib/linter/apply-disable-directives.js Co-authored-by: Milos Djermanovic --- lib/linter/apply-disable-directives.js | 32 +++++++------------------- 1 file changed, 8 insertions(+), 24 deletions(-) diff --git a/lib/linter/apply-disable-directives.js b/lib/linter/apply-disable-directives.js index 7a995cb1456..1d9db8461f7 100644 --- a/lib/linter/apply-disable-directives.js +++ b/lib/linter/apply-disable-directives.js @@ -22,35 +22,19 @@ function compareLocations(itemA, itemB) { * @returns {Directive[][]} Directives grouped by their parent comment. */ function groupByParentComment(directives) { - const groups = []; - - if (!directives.length) { - return groups; - } - - let state = null; + const groups = new Map(); for (const directive of directives) { - if (state) { - if (directive.unprocessedDirective.parentComment === state.parentComment) { - state.group.push(directive); - } else { - groups.push(state.group); - state = null; - } - } + const { unprocessedDirective: { parentComment } } = directive; - state = { - group: [directive], - parentComment: directive.parentComment - }; - } - - if (state && state.group.length) { - groups.push(state.group); + if (groups.has(parentComment)) { + groups.get(parentComment).push(directive); + } else { + groups.set(parentComment, [directive]); + } } - return groups; + return [...groups.values()]; } /** From df8267e9a1d8bcdce3a991c4afcc89653df64528 Mon Sep 17 00:00:00 2001 From: Josh Goldberg Date: Wed, 14 Jul 2021 14:15:56 -0400 Subject: [PATCH 16/31] Used suggestion for individual directives removal --- lib/linter/apply-disable-directives.js | 23 +++++++++++++++-------- 1 file changed, 15 insertions(+), 8 deletions(-) diff --git a/lib/linter/apply-disable-directives.js b/lib/linter/apply-disable-directives.js index 1d9db8461f7..637cf418229 100644 --- a/lib/linter/apply-disable-directives.js +++ b/lib/linter/apply-disable-directives.js @@ -44,20 +44,26 @@ function groupByParentComment(directives) { * @returns {{ fix, position, ruleIds }[]} Details for later creation of output Problems. */ function createIndividualDirectivesRemoval(directives, commentToken) { - const commentContents = commentToken.value.split("--")[0].trimRight(); - const fixStart = commentToken.range[0] + 2; - let lastRuleEnd = 0; + const listOffset = /^\s*\S+\s+/u.exec(commentToken.value)[0].length; + const listText = commentToken.value + .slice(listOffset) // remove eslint-* + .split(/\s-{2,}\s/u)[0] // remove -- directive comment + .trimRight(); + const listStart = commentToken.range[0] + 2 + listOffset; return directives.map(directive => { - const ruleStart = commentContents.indexOf(directive.ruleId, lastRuleEnd); - const ruleEnd = lastRuleEnd = ruleStart + directive.ruleId.length; + const { ruleId } = directive; + const match = new RegExp(String.raw`(?:^|,)\s*${ruleId}\s*(?:$|,)`, "u").exec(listText); + const ruleOffset = match.index; + const ruleEndOffset = ruleOffset + match[0].length; + const ruleText = listText.slice(ruleOffset, ruleEndOffset); return { - description: directive.ruleId, + description: ruleId, fix: { range: [ - fixStart + ruleStart, - fixStart + ruleEnd + (commentContents[ruleEnd] === "," ? 1 : 0) + listStart + ruleOffset + (ruleText.startsWith(",") && ruleText.endsWith(",") ? 1 : 0), + listStart + ruleEndOffset ], text: "" }, @@ -66,6 +72,7 @@ function createIndividualDirectivesRemoval(directives, commentToken) { }); } + /** * Creates a description of deleting an entire unused disable comment. * @param {Directive[]} directives Unused directives to be removed. From 40fd95485ced79ca7c691517cfed38bb2db1ffdb Mon Sep 17 00:00:00 2001 From: Josh Goldberg Date: Wed, 14 Jul 2021 14:17:16 -0400 Subject: [PATCH 17/31] Boolean in lib/cli-engine/cli-engine.js Co-authored-by: Milos Djermanovic --- lib/cli-engine/cli-engine.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/cli-engine/cli-engine.js b/lib/cli-engine/cli-engine.js index c6d84d2c6f8..dbb7a3ea9d6 100644 --- a/lib/cli-engine/cli-engine.js +++ b/lib/cli-engine/cli-engine.js @@ -339,7 +339,7 @@ function shouldMessageBeFixed(message, lastConfigArrays, fixTypes) { const rule = message.ruleId && getRule(message.ruleId, lastConfigArrays); - return rule && rule.meta && fixTypes.has(rule.meta.type); + return Boolean(rule && rule.meta && fixTypes.has(rule.meta.type)); } /** From 8808b4f5c2e572f443e216dd7eee57caab25d8c0 Mon Sep 17 00:00:00 2001 From: Josh Goldberg Date: Wed, 14 Jul 2021 14:19:20 -0400 Subject: [PATCH 18/31] Docs fixups --- docs/user-guide/command-line-interface.md | 2 +- lib/options.js | 2 +- lib/shared/types.js | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/docs/user-guide/command-line-interface.md b/docs/user-guide/command-line-interface.md index 99fd61eb950..0d217fb41a7 100644 --- a/docs/user-guide/command-line-interface.md +++ b/docs/user-guide/command-line-interface.md @@ -47,7 +47,7 @@ Specifying rules and plugins: Fixing problems: --fix Automatically fix problems --fix-dry-run Automatically fix problems without saving the changes to the file system - --fix-type Array Specify the types of fixes to apply (problem, suggestion, layout) + --fix-type Array Specify the types of fixes to apply (directive, problem, suggestion, layout) Ignoring files: --ignore-path path::String Specify path of ignore file diff --git a/lib/options.js b/lib/options.js index f3b419874dc..2fa03a2317d 100644 --- a/lib/options.js +++ b/lib/options.js @@ -151,7 +151,7 @@ module.exports = optionator({ { option: "fix-type", type: "Array", - description: "Specify the types of fixes to apply (problem, suggestion, layout)" + description: "Specify the types of fixes to apply (directive, problem, suggestion, layout)" }, { heading: "Ignoring files" diff --git a/lib/shared/types.js b/lib/shared/types.js index d3bf184d030..c3b76e42d5f 100644 --- a/lib/shared/types.js +++ b/lib/shared/types.js @@ -125,7 +125,7 @@ module.exports = {}; * @property {Record} [messages] The messages the rule reports. * @property {string[]} [replacedBy] The IDs of the alternative rules. * @property {Array|Object} schema The option schema of the rule. - * @property {"directive"|"problem"|"suggestion"|"layout"} type The rule type. + * @property {"problem"|"suggestion"|"layout"} type The rule type. */ /** From 195d76b2c26e3d6aa174786ab11cbd2a3aebd7cd Mon Sep 17 00:00:00 2001 From: Josh Goldberg Date: Wed, 14 Jul 2021 15:02:18 -0400 Subject: [PATCH 19/31] Added tests --- tests/lib/linter/apply-disable-directives.js | 311 ++++++++++++++++++- 1 file changed, 308 insertions(+), 3 deletions(-) diff --git a/tests/lib/linter/apply-disable-directives.js b/tests/lib/linter/apply-disable-directives.js index d1d55e3613e..7dd028ed518 100644 --- a/tests/lib/linter/apply-disable-directives.js +++ b/tests/lib/linter/apply-disable-directives.js @@ -30,12 +30,13 @@ function createSourceCode(text) { /** * Creates a ParentComment for a given range. * @param {[number, number]} range total range of the comment - * @param {string[]} ruleIds Rule IDs reported in the comment + * @param {string} value String value of the comment + * @param {string[]} ruleIds Rule IDs reported in the value * @returns {ParentComment} Test-ready ParentComment object. */ -function createParentComment(range, ruleIds = []) { +function createParentComment(range, value, ruleIds = []) { return { - commentToken: { range }, + commentToken: { range, value }, ruleIds }; } @@ -1413,4 +1414,308 @@ describe("apply-disable-directives", () => { ); }); }); + + describe("unused rules within directives", () => { + it("Adds a problem for /* eslint-disable used, unused */", () => { + const parentComment = createParentComment([0, 32], " eslint-disable used, unused ", ["used", "unused"]); + + assert.deepStrictEqual( + applyDisableDirectives({ + directives: [ + { + parentComment, + ruleId: "used", + type: "disable", + line: 1, + column: 18 + }, + { + parentComment, + ruleId: "unused", + type: "disable", + line: 1, + column: 22 + } + ], + problems: [{ line: 2, column: 1, ruleId: "used" }], + reportUnusedDisableDirectives: "error" + }), + [ + { + ruleId: null, + message: "Unused eslint-disable directive (no problems were reported from unused).", + line: 1, + column: 22, + fix: { + range: [22, 30], + text: "" + }, + severity: 2, + nodeType: null + } + ] + ); + }); + it("Adds a problem for /* eslint-disable used , unused , -- unused and used are ok */", () => { + const parentComment = createParentComment([0, 62], " eslint-disable used , unused , -- unused and used are ok ", ["used", "unused"]); + + assert.deepStrictEqual( + applyDisableDirectives({ + directives: [ + { + parentComment, + ruleId: "used", + type: "disable", + line: 1, + column: 18 + }, + { + parentComment, + ruleId: "unused", + type: "disable", + line: 1, + column: 24 + } + ], + problems: [{ line: 2, column: 1, ruleId: "used" }], + reportUnusedDisableDirectives: "error" + }), + [ + { + ruleId: null, + message: "Unused eslint-disable directive (no problems were reported from unused).", + line: 1, + column: 24, + fix: { + range: [24, 33], + text: "" + }, + severity: 2, + nodeType: null + } + ] + ); + }); + + it("Adds a problem for /* eslint-disable unused, used */", () => { + const parentComment = createParentComment([0, 32], " eslint-disable unused, used ", ["unused", "used"]); + + assert.deepStrictEqual( + applyDisableDirectives({ + directives: [ + { + parentComment, + ruleId: "unused", + type: "disable", + line: 1, + column: 18 + }, + { + parentComment, + ruleId: "used", + type: "disable", + line: 1, + column: 25 + } + ], + problems: [{ line: 2, column: 1, ruleId: "used" }], + reportUnusedDisableDirectives: "error" + }), + [ + { + ruleId: null, + message: "Unused eslint-disable directive (no problems were reported from unused).", + line: 1, + column: 18, + fix: { + range: [18, 25], + text: "" + }, + severity: 2, + nodeType: null + } + ] + ); + }); + + it("Adds a problem for /* eslint-disable unused,, ,, used */", () => { + const parentComment = createParentComment([0, 37], " eslint-disable unused,, ,, used ", ["unused", "used"]); + + assert.deepStrictEqual( + applyDisableDirectives({ + directives: [ + { + parentComment, + ruleId: "unused", + type: "disable", + line: 1, + column: 18 + }, + { + parentComment, + ruleId: "used", + type: "disable", + line: 1, + column: 29 + } + ], + problems: [{ line: 2, column: 1, ruleId: "used" }], + reportUnusedDisableDirectives: "error" + }), + [ + { + ruleId: null, + message: "Unused eslint-disable directive (no problems were reported from unused).", + line: 1, + column: 18, + fix: { + range: [18, 25], + text: "" + }, + severity: 2, + nodeType: null + } + ] + ); + }); + + it("Adds a problem for /* eslint-disable unused-1, unused-2, used */", () => { + const parentComment = createParentComment([0, 45], " eslint-disable unused-1, unused-2, used ", ["unused-1", "unused-2", "used"]); + + assert.deepStrictEqual( + applyDisableDirectives({ + directives: [ + { + parentComment, + ruleId: "unused-1", + type: "disable", + line: 1, + column: 18 + }, + { + parentComment, + ruleId: "unused-2", + type: "disable", + line: 1, + column: 28 + }, + { + parentComment, + ruleId: "used", + type: "disable", + line: 1, + column: 38 + } + ], + problems: [{ line: 2, column: 1, ruleId: "used" }], + reportUnusedDisableDirectives: "error" + }), + [ + { + ruleId: null, + message: "Unused eslint-disable directive (no problems were reported from unused-1).", + line: 1, + column: 18, + fix: { + range: [18, 27], + text: "" + }, + severity: 2, + nodeType: null + }, + { + ruleId: null, + message: "Unused eslint-disable directive (no problems were reported from unused-2).", + line: 1, + column: 28, + fix: { + range: [27, 37], + text: "" + }, + severity: 2, + nodeType: null + } + ] + ); + }); + + it("Adds a problem for /* eslint-disable unused-1, unused-2, used, unused-3 */", () => { + const parentComment = createParentComment([0, 55], " eslint-disable unused-1, unused-2, used, unused-3 ", ["unused-1", "unused-2", "used", "unused-3"]); + + assert.deepStrictEqual( + applyDisableDirectives({ + directives: [ + { + parentComment, + ruleId: "unused-1", + type: "disable", + line: 1, + column: 18 + }, + { + parentComment, + ruleId: "unused-2", + type: "disable", + line: 1, + column: 28 + }, + { + parentComment, + ruleId: "used", + type: "disable", + line: 1, + column: 38 + }, + { + parentComment, + ruleId: "unused-3", + type: "disable", + line: 1, + column: 43 + } + ], + problems: [{ line: 2, column: 1, ruleId: "used" }], + reportUnusedDisableDirectives: "error" + }), + [ + { + ruleId: null, + message: "Unused eslint-disable directive (no problems were reported from unused-1).", + line: 1, + column: 18, + fix: { + range: [18, 27], + text: "" + }, + severity: 2, + nodeType: null + }, + { + ruleId: null, + message: "Unused eslint-disable directive (no problems were reported from unused-2).", + line: 1, + column: 28, + fix: { + range: [27, 37], + text: "" + }, + severity: 2, + nodeType: null + }, + { + ruleId: null, + message: "Unused eslint-disable directive (no problems were reported from unused-3).", + line: 1, + column: 43, + fix: { + range: [42, 52], + text: "" + }, + severity: 2, + nodeType: null + } + ] + ); + }); + }); }); From eb4ddecb544f178e1d3e7ff99fd0b28789afc02f Mon Sep 17 00:00:00 2001 From: Josh Goldberg Date: Wed, 14 Jul 2021 15:12:08 -0400 Subject: [PATCH 20/31] Tests and fix for multiple rule lists --- lib/linter/apply-disable-directives.js | 3 +- tests/lib/linter/apply-disable-directives.js | 89 ++++++++++++++++++++ 2 files changed, 90 insertions(+), 2 deletions(-) diff --git a/lib/linter/apply-disable-directives.js b/lib/linter/apply-disable-directives.js index 637cf418229..421f1c11e88 100644 --- a/lib/linter/apply-disable-directives.js +++ b/lib/linter/apply-disable-directives.js @@ -72,7 +72,6 @@ function createIndividualDirectivesRemoval(directives, commentToken) { }); } - /** * Creates a description of deleting an entire unused disable comment. * @param {Directive[]} directives Unused directives to be removed. @@ -86,7 +85,7 @@ function createCommentRemoval(directives, commentToken) { return { description: ruleIds.length <= 2 ? ruleIds.join(" or ") - : `${ruleIds.slice(0, ruleIds.length - 2).join(", ")}, or ${ruleIds[ruleIds.length - 1]}`, + : `${ruleIds.slice(0, ruleIds.length - 1).join(", ")}, or ${ruleIds[ruleIds.length - 1]}`, fix: { range, text: " " diff --git a/tests/lib/linter/apply-disable-directives.js b/tests/lib/linter/apply-disable-directives.js index 7dd028ed518..2b66d112aa9 100644 --- a/tests/lib/linter/apply-disable-directives.js +++ b/tests/lib/linter/apply-disable-directives.js @@ -1717,5 +1717,94 @@ describe("apply-disable-directives", () => { ] ); }); + + it("Adds a problem for /* eslint-disable unused-1, unused-2 */", () => { + const parentComment = createParentComment([0, 39], " eslint-disable unused-1, unused-2 ", ["unused-1", "unused-2"]); + + assert.deepStrictEqual( + applyDisableDirectives({ + directives: [ + { + parentComment, + ruleId: "unused-1", + type: "disable", + line: 1, + column: 18 + }, + { + parentComment, + ruleId: "unused-2", + type: "disable", + line: 1, + column: 28 + } + ], + problems: [], + reportUnusedDisableDirectives: "error" + }), + [ + { + ruleId: null, + message: "Unused eslint-disable directive (no problems were reported from 'unused-1' or 'unused-2').", + line: 1, + column: 18, + fix: { + range: [0, 39], + text: " " + }, + severity: 2, + nodeType: null + } + ] + ); + }); + + it("Adds a problem for /* eslint-disable unused-1, unused-2, unused-3 */", () => { + const parentComment = createParentComment([0, 49], " eslint-disable unused-1, unused-2, unused-3 ", ["unused-1", "unused-2", "unused-3"]); + + assert.deepStrictEqual( + applyDisableDirectives({ + directives: [ + { + parentComment, + ruleId: "unused-1", + type: "disable", + line: 1, + column: 18 + }, + { + parentComment, + ruleId: "unused-2", + type: "disable", + line: 1, + column: 28 + }, + { + parentComment, + ruleId: "unused-3", + type: "disable", + line: 1, + column: 38 + } + ], + problems: [], + reportUnusedDisableDirectives: "error" + }), + [ + { + ruleId: null, + message: "Unused eslint-disable directive (no problems were reported from 'unused-1', 'unused-2', or 'unused-3').", + line: 1, + column: 18, + fix: { + range: [0, 49], + text: " " + }, + severity: 2, + nodeType: null + } + ] + ); + }); }); }); From 4d98acb4bb0cedc6d74541d0948cc50dc3723c0a Mon Sep 17 00:00:00 2001 From: Josh Goldberg Date: Wed, 14 Jul 2021 15:20:13 -0400 Subject: [PATCH 21/31] Remove unused sourceCode from tests --- tests/lib/linter/apply-disable-directives.js | 180 ++++++------------- 1 file changed, 53 insertions(+), 127 deletions(-) diff --git a/tests/lib/linter/apply-disable-directives.js b/tests/lib/linter/apply-disable-directives.js index 2b66d112aa9..32be4409814 100644 --- a/tests/lib/linter/apply-disable-directives.js +++ b/tests/lib/linter/apply-disable-directives.js @@ -6,26 +6,7 @@ "use strict"; const assert = require("chai").assert; -const espree = require("espree"); const applyDisableDirectives = require("../../../lib/linter/apply-disable-directives"); -const { SourceCode } = require("../../../lib/source-code"); - -const DEFAULT_CONFIG = { - ecmaVersion: 6, - comment: true, - tokens: true, - range: true, - loc: true -}; - -/** - * Create a SourceCode instance from the given code. - * @param {string} text The text of the code. - * @returns {SourceCode} The SourceCode. - */ -function createSourceCode(text) { - return new SourceCode(text, espree.parse(text, DEFAULT_CONFIG)); -} /** * Creates a ParentComment for a given range. @@ -47,8 +28,7 @@ describe("apply-disable-directives", () => { assert.deepStrictEqual( applyDisableDirectives({ directives: [{ parentComment: createParentComment([0, 7]), type: "disable", line: 1, column: 8, ruleId: null }], - problems: [{ line: 1, column: 7, ruleId: "foo" }], - sourceCode: createSourceCode("first; /* eslint-disable */") + problems: [{ line: 1, column: 7, ruleId: "foo" }] }), [{ ruleId: "foo", line: 1, column: 7 }] ); @@ -58,8 +38,7 @@ describe("apply-disable-directives", () => { assert.deepStrictEqual( applyDisableDirectives({ directives: [{ parentComment: createParentComment([21, 27]), type: "disable", line: 2, column: 1, ruleId: null }], - problems: [{ line: 1, column: 10, ruleId: "foo" }], - sourceCode: createSourceCode("\n/* eslint-disable*/") + problems: [{ line: 1, column: 10, ruleId: "foo" }] }), [{ ruleId: "foo", line: 1, column: 10 }] ); @@ -69,8 +48,7 @@ describe("apply-disable-directives", () => { assert.deepStrictEqual( applyDisableDirectives({ directives: [{ type: "disable", line: 1, column: 8, ruleId: null }], - problems: [{ line: 1, column: 8, ruleId: null }], - sourceCode: createSourceCode("first; /* eslint-disable foo */") + problems: [{ line: 1, column: 8, ruleId: null }] }), [] ); @@ -90,8 +68,7 @@ describe("apply-disable-directives", () => { assert.deepStrictEqual( applyDisableDirectives({ directives: [{ type: "disable", line: 1, column: 8, ruleId: null }], - problems: [{ line: 2, column: 3, ruleId: "foo" }], - sourceCode: createSourceCode("first; /* eslint-disable foo */") + problems: [{ line: 2, column: 3, ruleId: "foo" }] }), [] ); @@ -103,8 +80,7 @@ describe("apply-disable-directives", () => { assert.deepStrictEqual( applyDisableDirectives({ directives: [{ type: "disable", line: 1, column: 8, ruleId: "foo" }], - problems: [{ line: 2, column: 3, ruleId: "foo" }], - sourceCode: createSourceCode("first; /* eslint-disable foo */") + problems: [{ line: 2, column: 3, ruleId: "foo" }] }), [] ); @@ -114,8 +90,7 @@ describe("apply-disable-directives", () => { assert.deepStrictEqual( applyDisableDirectives({ directives: [{ type: "disable", line: 1, column: 8, ruleId: "foo" }], - problems: [{ line: 1, column: 8, ruleId: "foo" }], - sourceCode: createSourceCode("first; /* eslint-disable foo */") + problems: [{ line: 1, column: 8, ruleId: "foo" }] }), [] ); @@ -131,8 +106,7 @@ describe("apply-disable-directives", () => { column: 1, ruleId: "foo" }], - problems: [{ line: 2, column: 3, ruleId: "not-foo" }], - sourceCode: createSourceCode("/* eslint-disable foo */") + problems: [{ line: 2, column: 3, ruleId: "not-foo" }] }), [{ line: 2, column: 3, ruleId: "not-foo" }] ); @@ -148,8 +122,7 @@ describe("apply-disable-directives", () => { column: 8, ruleId: "foo" }], - problems: [{ line: 1, column: 7, ruleId: "foo" }], - sourceCode: createSourceCode("first; /* eslint-disable foo */") + problems: [{ line: 1, column: 7, ruleId: "foo" }] }), [{ line: 1, column: 7, ruleId: "foo" }] ); @@ -176,8 +149,7 @@ describe("apply-disable-directives", () => { ruleId: null } ], - problems: [{ line: 1, column: 27, ruleId: "foo" }], - sourceCode: createSourceCode("/* eslint-disable */ /* eslint-enable */") + problems: [{ line: 1, column: 27, ruleId: "foo" }] }), [{ line: 1, column: 27, ruleId: "foo" }] ); @@ -202,8 +174,7 @@ describe("apply-disable-directives", () => { ruleId: null } ], - problems: [{ line: 1, column: 26, ruleId: "foo" }], - sourceCode: createSourceCode("/* eslint-disable */ /* eslint-enable */") + problems: [{ line: 1, column: 26, ruleId: "foo" }] }), [{ line: 1, column: 26, ruleId: "foo" }] ); @@ -216,8 +187,7 @@ describe("apply-disable-directives", () => { { type: "disable", line: 1, column: 1, ruleId: null }, { type: "enable", line: 1, column: 26, ruleId: null } ], - problems: [{ line: 1, column: 3, ruleId: "foo" }], - sourceCode: createSourceCode("/* eslint-disable */ /* eslint-enable */") + problems: [{ line: 1, column: 3, ruleId: "foo" }] }), [] ); @@ -249,8 +219,7 @@ describe("apply-disable-directives", () => { ruleId: "foo" } ], - problems: [{ line: 3, column: 3, ruleId: "foo" }], - sourceCode: createSourceCode("/* eslint-disable */ /* eslint-enable foo */\n/* eslint-disable foo */") + problems: [{ line: 3, column: 3, ruleId: "foo" }] }), [] ); @@ -282,8 +251,7 @@ describe("apply-disable-directives", () => { ruleId: null } ], - problems: [{ line: 3, column: 3, ruleId: "foo" }], - sourceCode: createSourceCode("/* eslint-disable */ /* eslint-enable foo *//* eslint-disable */") + problems: [{ line: 3, column: 3, ruleId: "foo" }] }), [] ); @@ -308,8 +276,7 @@ describe("apply-disable-directives", () => { ruleId: null } ], - problems: [{ line: 1, column: 3, ruleId: "not-foo" }], - sourceCode: createSourceCode("/* eslint-disable foo */ /* eslint-enable */") + problems: [{ line: 1, column: 3, ruleId: "not-foo" }] }), [{ line: 1, column: 3, ruleId: "not-foo" }] ); @@ -336,8 +303,7 @@ describe("apply-disable-directives", () => { ruleId: "foo" } ], - problems: [{ line: 2, column: 4, ruleId: "foo" }], - sourceCode: createSourceCode("/* eslint-disable */\n/* eslint-enable foo */") + problems: [{ line: 2, column: 4, ruleId: "foo" }] }), [{ line: 2, column: 4, ruleId: "foo" }] ); @@ -362,8 +328,7 @@ describe("apply-disable-directives", () => { ruleId: "foo" } ], - problems: [{ line: 2, column: 1, ruleId: "foo" }], - sourceCode: createSourceCode("/* eslint-disable */\n/* eslint-enable foo */") + problems: [{ line: 2, column: 1, ruleId: "foo" }] }), [{ line: 2, column: 1, ruleId: "foo" }] ); @@ -376,8 +341,7 @@ describe("apply-disable-directives", () => { { type: "disable", line: 1, column: 1, ruleId: null }, { type: "enable", line: 2, column: 1, ruleId: "foo" } ], - problems: [{ line: 2, column: 4, ruleId: "not-foo" }], - sourceCode: createSourceCode("/* eslint-disable */\n/* eslint-enable foo */") + problems: [{ line: 2, column: 4, ruleId: "not-foo" }] }), [] ); @@ -398,8 +362,7 @@ describe("apply-disable-directives", () => { { line: 1, column: 30, ruleId: "bar" }, { line: 1, column: 50, ruleId: "foo" }, { line: 1, column: 50, ruleId: "bar" } - ], - sourceCode: createSourceCode("/* eslint-disable */ /* eslint-enable foo */ /* eslint-enable bar */") + ] }), [ { line: 1, column: 30, ruleId: "foo" }, @@ -421,8 +384,7 @@ describe("apply-disable-directives", () => { column: 1, ruleId: null }], - problems: [{ line: 1, column: 5, ruleId: "foo" }], - sourceCode: createSourceCode("first;\n// eslint-disable-line") + problems: [{ line: 1, column: 5, ruleId: "foo" }] }), [{ line: 1, column: 5, ruleId: "foo" }] ); @@ -438,8 +400,7 @@ describe("apply-disable-directives", () => { column: 8, ruleId: null }], - problems: [{ line: 1, column: 1, ruleId: "foo" }], - sourceCode: createSourceCode("first; // eslint-disable-line") + problems: [{ line: 1, column: 1, ruleId: "foo" }] }), [] ); @@ -455,8 +416,7 @@ describe("apply-disable-directives", () => { column: 8, ruleId: null }], - problems: [{ line: 1, column: 10, ruleId: "foo" }], - sourceCode: createSourceCode("first; // eslint-disable-line") + problems: [{ line: 1, column: 10, ruleId: "foo" }] }), [] ); @@ -472,8 +432,7 @@ describe("apply-disable-directives", () => { column: 8, ruleId: "foo" }], - problems: [{ line: 2, column: 1, ruleId: "foo" }], - sourceCode: createSourceCode("first; // eslint-disable-line foo") + problems: [{ line: 2, column: 1, ruleId: "foo" }] }), [{ line: 2, column: 1, ruleId: "foo" }] ); @@ -491,8 +450,7 @@ describe("apply-disable-directives", () => { column: 8, ruleId: "foo" }], - problems: [{ line: 1, column: 2, ruleId: "foo" }], - sourceCode: createSourceCode("first; // eslint-disable-line foo") + problems: [{ line: 1, column: 2, ruleId: "foo" }] }), [] ); @@ -502,8 +460,7 @@ describe("apply-disable-directives", () => { assert.deepStrictEqual( applyDisableDirectives({ directives: [{ parentComment: createParentComment([0, 27]), type: "disable-line", line: 1, column: 1, ruleId: "foo" }], - problems: [{ line: 1, column: 2, ruleId: "not-foo" }], - sourceCode: createSourceCode("// eslint-disable-line foo") + problems: [{ line: 1, column: 2, ruleId: "not-foo" }] }), [{ line: 1, column: 2, ruleId: "not-foo" }] ); @@ -528,8 +485,7 @@ describe("apply-disable-directives", () => { ruleId: "foo" } ], - problems: [{ line: 1, column: 5, ruleId: "not-foo" }], - sourceCode: createSourceCode("/* eslint-disable */ // eslint-disable-line foo") + problems: [{ line: 1, column: 5, ruleId: "not-foo" }] }), [] ); @@ -582,10 +538,7 @@ describe("apply-disable-directives", () => { ruleId: "foo" } ], - problems: [{ line: 2, column: 1, ruleId: "foo" }], - sourceCode: createSourceCode( - new Array(6).fill(0).map(() => "first; // eslint-disable-line foo").join("\n") - ) + problems: [{ line: 2, column: 1, ruleId: "foo" }] }), [] ); @@ -603,8 +556,7 @@ describe("apply-disable-directives", () => { column: 1, ruleId: null }], - problems: [{ line: 2, column: 3, ruleId: "foo" }], - sourceCode: createSourceCode("// eslint-disable-next-line") + problems: [{ line: 2, column: 3, ruleId: "foo" }] }), [] ); @@ -620,8 +572,7 @@ describe("apply-disable-directives", () => { column: 1, ruleId: null }], - problems: [{ line: 1, column: 3, ruleId: "foo" }], - sourceCode: createSourceCode("// eslint-disable-next-line") + problems: [{ line: 1, column: 3, ruleId: "foo" }] }), [{ line: 1, column: 3, ruleId: "foo" }] ); @@ -637,8 +588,7 @@ describe("apply-disable-directives", () => { column: 1, ruleId: null }], - problems: [{ line: 3, column: 3, ruleId: "foo" }], - sourceCode: createSourceCode("// eslint-disable-next-line") + problems: [{ line: 3, column: 3, ruleId: "foo" }] }), [{ line: 3, column: 3, ruleId: "foo" }] ); @@ -651,8 +601,7 @@ describe("apply-disable-directives", () => { { type: "disable-next-line", line: 1, column: 1, ruleId: null }, { type: "enable", line: 1, column: 5, ruleId: null } ], - problems: [{ line: 2, column: 2, ruleId: "foo" }], - sourceCode: createSourceCode("// eslint-disable-next-line\n/* eslint-enable */") + problems: [{ line: 2, column: 2, ruleId: "foo" }] }), [] ); @@ -664,8 +613,7 @@ describe("apply-disable-directives", () => { assert.deepStrictEqual( applyDisableDirectives({ directives: [{ type: "disable-next-line", line: 1, column: 1, ruleId: "foo" }], - problems: [{ line: 2, column: 1, ruleId: "foo" }], - sourceCode: createSourceCode("// eslint-disable-next-line foo") + problems: [{ line: 2, column: 1, ruleId: "foo" }] }), [] ); @@ -681,8 +629,7 @@ describe("apply-disable-directives", () => { column: 1, ruleId: "foo" }], - problems: [{ line: 2, column: 1, ruleId: "not-foo" }], - sourceCode: createSourceCode("// eslint-disable-next-line foo\n/* eslint-enable foo */") + problems: [{ line: 2, column: 1, ruleId: "not-foo" }] }), [{ line: 2, column: 1, ruleId: "not-foo" }] ); @@ -713,8 +660,7 @@ describe("apply-disable-directives", () => { column: 1 }], problems: [], - reportUnusedDisableDirectives: "error", - sourceCode: createSourceCode("/* eslint-disable */") + reportUnusedDisableDirectives: "error" }), [ { @@ -738,8 +684,7 @@ describe("apply-disable-directives", () => { applyDisableDirectives({ directives: [{ type: "disable", line: 1, column: 1, ruleId: null }], problems: [{ line: 2, column: 1, ruleId: "foo" }], - reportUnusedDisableDirectives: "error", - sourceCode: createSourceCode("/* eslint-disable */") + reportUnusedDisableDirectives: "error" }), [] ); @@ -756,8 +701,7 @@ describe("apply-disable-directives", () => { ruleId: "foo" }], problems: [], - reportUnusedDisableDirectives: "error", - sourceCode: createSourceCode("/* eslint-disable foo */") + reportUnusedDisableDirectives: "error" }), [ { @@ -787,8 +731,7 @@ describe("apply-disable-directives", () => { ruleId: "foo" }], problems: [{ line: 1, column: 20, ruleId: "not-foo" }], - reportUnusedDisableDirectives: "error", - sourceCode: createSourceCode("/* eslint-disable foo */") + reportUnusedDisableDirectives: "error" }), [ { @@ -832,8 +775,7 @@ describe("apply-disable-directives", () => { } ], problems: [{ line: 1, column: 2, ruleId: "foo" }], - reportUnusedDisableDirectives: "error", - sourceCode: createSourceCode("first; /* eslint-disable */ /* eslint-enable foo */") + reportUnusedDisableDirectives: "error" }), [ { @@ -877,8 +819,7 @@ describe("apply-disable-directives", () => { } ], problems: [], - reportUnusedDisableDirectives: "error", - sourceCode: createSourceCode("/* eslint-disable */ /* eslint-enable */") + reportUnusedDisableDirectives: "error" }), [ { @@ -917,8 +858,7 @@ describe("apply-disable-directives", () => { } ], problems: [], - reportUnusedDisableDirectives: "error", - sourceCode: createSourceCode("/* eslint-disable */\n/* eslint-disable */") + reportUnusedDisableDirectives: "error" }), [ { @@ -969,8 +909,7 @@ describe("apply-disable-directives", () => { } ], problems: [{ line: 3, column: 1, ruleId: "foo" }], - reportUnusedDisableDirectives: "error", - sourceCode: createSourceCode("/* eslint-disable */\n/* eslint-disable */ /* (problem) */") + reportUnusedDisableDirectives: "error" }), [ { @@ -1009,8 +948,7 @@ describe("apply-disable-directives", () => { } ], problems: [{ line: 3, column: 1, ruleId: "foo" }], - reportUnusedDisableDirectives: "error", - sourceCode: createSourceCode("/* eslint-disable foo */\n/* eslint-disable */ /* (problem from foo) */") + reportUnusedDisableDirectives: "error" }), [ { @@ -1034,8 +972,7 @@ describe("apply-disable-directives", () => { applyDisableDirectives({ directives: [{ type: "disable", line: 1, column: 1, ruleId: "foo" }], problems: [{ line: 1, column: 6, ruleId: "foo" }], - reportUnusedDisableDirectives: "error", - sourceCode: createSourceCode("/* eslint-disable foo */") + reportUnusedDisableDirectives: "error" }), [] ); @@ -1061,8 +998,7 @@ describe("apply-disable-directives", () => { } ], problems: [{ line: 3, column: 1, ruleId: "foo" }], - reportUnusedDisableDirectives: "error", - sourceCode: createSourceCode("/* eslint-disable */\n/* eslint-disable foo */ /* (problem from foo) */") + reportUnusedDisableDirectives: "error" }), [ { @@ -1101,8 +1037,7 @@ describe("apply-disable-directives", () => { } ], problems: [{ line: 3, column: 1, ruleId: "bar" }], - reportUnusedDisableDirectives: "error", - sourceCode: createSourceCode("/* eslint-disable */\n/* eslint-disable foo */ /* (problem from another rule) */") + reportUnusedDisableDirectives: "error" }), [ { @@ -1141,8 +1076,7 @@ describe("apply-disable-directives", () => { } ], problems: [{ line: 1, column: 30, ruleId: "foo" }], - reportUnusedDisableDirectives: "error", - sourceCode: createSourceCode("/* eslint-disable */ /* eslint-enable foo */") + reportUnusedDisableDirectives: "error" }), [ { @@ -1186,8 +1120,7 @@ describe("apply-disable-directives", () => { } ], problems: [{ line: 1, column: 30, ruleId: "foo" }], - reportUnusedDisableDirectives: "error", - sourceCode: createSourceCode("/* eslint-disable foo */ /* eslint-enable */ /* (problem from foo) */") + reportUnusedDisableDirectives: "error" }), [ { @@ -1238,8 +1171,7 @@ describe("apply-disable-directives", () => { } ], problems: [{ line: 4, column: 1, ruleId: "foo" }], - reportUnusedDisableDirectives: "error", - sourceCode: createSourceCode("/* eslint-disable */\n/* eslint-disable foo */\n/* eslint-enable foo */ /* (problem from foo) */") + reportUnusedDisableDirectives: "error" }), [ { @@ -1286,8 +1218,7 @@ describe("apply-disable-directives", () => { ruleId: null }], problems: [], - reportUnusedDisableDirectives: "error", - sourceCode: createSourceCode("// eslint-disable-line") + reportUnusedDisableDirectives: "error" }), [ { @@ -1312,8 +1243,7 @@ describe("apply-disable-directives", () => { applyDisableDirectives({ directives: [{ type: "disable-line", line: 1, column: 1, ruleId: null }], problems: [{ line: 1, column: 10, ruleId: "foo" }], - reportUnusedDisableDirectives: "error", - sourceCode: createSourceCode("/* eslint-disable line */") + reportUnusedDisableDirectives: "error" }), [] ); @@ -1330,8 +1260,7 @@ describe("apply-disable-directives", () => { ruleId: null }], problems: [], - reportUnusedDisableDirectives: "error", - sourceCode: createSourceCode("// eslint-disable-next-line") + reportUnusedDisableDirectives: "error" }), [ { @@ -1355,8 +1284,7 @@ describe("apply-disable-directives", () => { applyDisableDirectives({ directives: [{ type: "disable-next-line", line: 1, column: 1, ruleId: null }], problems: [{ line: 2, column: 10, ruleId: "foo" }], - reportUnusedDisableDirectives: "error", - sourceCode: createSourceCode("// eslint-disable foo-next-line") + reportUnusedDisableDirectives: "error" }), [] ); @@ -1370,8 +1298,7 @@ describe("apply-disable-directives", () => { { parentComment: createParentComment([20, 43]), type: "disable-line", line: 1, column: 22, ruleId: null } ], problems: [], - reportUnusedDisableDirectives: "error", - sourceCode: createSourceCode("/* eslint-disable */ // eslint-disable-line") + reportUnusedDisableDirectives: "error" }), [ { @@ -1407,8 +1334,7 @@ describe("apply-disable-directives", () => { applyDisableDirectives({ directives: [{ parentComment: createParentComment([0, 27]), type: "disable-next-line", line: 1, column: 1, ruleId: null }], problems: [], - reportUnusedDisableDirectives: "off", - sourceCode: createSourceCode("// eslint-disable-next-line") + reportUnusedDisableDirectives: "off" }), [] ); From 597949fab7728ee9217fe5f2e62dbf800fae4d88 Mon Sep 17 00:00:00 2001 From: Josh Goldberg Date: Wed, 14 Jul 2021 15:32:47 -0400 Subject: [PATCH 22/31] Added high-level test in linter.js --- tests/lib/linter/linter.js | 34 ++++++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/tests/lib/linter/linter.js b/tests/lib/linter/linter.js index 97c9a4b699b..c59764f7916 100644 --- a/tests/lib/linter/linter.js +++ b/tests/lib/linter/linter.js @@ -3369,6 +3369,40 @@ var a = "test2"; ] ); }); + + it("reports problems for partially unused eslint-disable comments (in config)", () => { + const code = "alert('test'); // eslint-disable-line no-alert, no-redeclare"; + const config = { + reportUnusedDisableDirectives: true, + rules: { + "no-alert": 1, + "no-redeclare": 1 + } + }; + + const messages = linter.verify(code, config, { + filename, + allowInlineConfig: true + }); + + assert.deepStrictEqual( + messages, + [ + { + ruleId: null, + message: "Unused eslint-disable directive (no problems were reported from no-redeclare).", + line: 1, + column: 16, + fix: { + range: [46, 60], + text: "" + }, + severity: 1, + nodeType: null + } + ] + ); + }); }); describe("when evaluating code with comments to change config when allowInlineConfig is disabled", () => { From 66e016c52442ead449259e0a16caaa8c6f2d4f03 Mon Sep 17 00:00:00 2001 From: Josh Goldberg Date: Sat, 17 Jul 2021 10:48:47 -0400 Subject: [PATCH 23/31] Apply suggestions from code review Co-authored-by: Brandon Mills --- lib/linter/apply-disable-directives.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/linter/apply-disable-directives.js b/lib/linter/apply-disable-directives.js index 421f1c11e88..076d0ef1e1c 100644 --- a/lib/linter/apply-disable-directives.js +++ b/lib/linter/apply-disable-directives.js @@ -41,7 +41,7 @@ function groupByParentComment(directives) { * Creates removal details for a set of directives within the same comment. * @param {Directive[]} directives Unused directives to be removed. * @param {Token} commentToken The backing Comment token. - * @returns {{ fix, position, ruleIds }[]} Details for later creation of output Problems. + * @returns {{ description, fix, position }[]} Details for later creation of output Problems. */ function createIndividualDirectivesRemoval(directives, commentToken) { const listOffset = /^\s*\S+\s+/u.exec(commentToken.value)[0].length; @@ -76,7 +76,7 @@ function createIndividualDirectivesRemoval(directives, commentToken) { * Creates a description of deleting an entire unused disable comment. * @param {Directive[]} directives Unused directives to be removed. * @param {Token} commentToken The backing Comment token. - * @returns {{ fix, position, ruleIds }} Details for later creation of an output Problem. + * @returns {{ description, fix, position }} Details for later creation of an output Problem. */ function createCommentRemoval(directives, commentToken) { const { range } = commentToken; @@ -111,7 +111,7 @@ function flatMap(array, fn) { /** * Parses details from directives to create output Problems. * @param {Directive[]} allDirectives Unused directives to be removed. - * @returns {{ fix, position, ruleIds }} Details for later creation of output Problems. + * @returns {{ description, fix, position }[]} Details for later creation of output Problems. */ function processUnusedDisableDirectives(allDirectives) { const directiveGroups = groupByParentComment(allDirectives); From a3228b6a5d5f6bc08537f02fbc202a559dc2d3fb Mon Sep 17 00:00:00 2001 From: Josh Goldberg Date: Thu, 29 Jul 2021 21:13:13 -0400 Subject: [PATCH 24/31] Apply suggestions from code review Co-authored-by: Milos Djermanovic --- lib/linter/apply-disable-directives.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/linter/apply-disable-directives.js b/lib/linter/apply-disable-directives.js index 076d0ef1e1c..bd2502d4f73 100644 --- a/lib/linter/apply-disable-directives.js +++ b/lib/linter/apply-disable-directives.js @@ -53,13 +53,13 @@ function createIndividualDirectivesRemoval(directives, commentToken) { return directives.map(directive => { const { ruleId } = directive; - const match = new RegExp(String.raw`(?:^|,)\s*${ruleId}\s*(?:$|,)`, "u").exec(listText); + const match = new RegExp(String.raw`(?:^|,)\s*${escapeRegExp(ruleId)}\s*(?:$|,)`, "u").exec(listText); const ruleOffset = match.index; const ruleEndOffset = ruleOffset + match[0].length; const ruleText = listText.slice(ruleOffset, ruleEndOffset); return { - description: ruleId, + description: `'${ruleId}'`, fix: { range: [ listStart + ruleOffset + (ruleText.startsWith(",") && ruleText.endsWith(",") ? 1 : 0), From 04595417c03a6e6b0fd20c60308170acdb9e1189 Mon Sep 17 00:00:00 2001 From: Josh Goldberg Date: Thu, 29 Jul 2021 21:14:55 -0400 Subject: [PATCH 25/31] Added require --- lib/linter/apply-disable-directives.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lib/linter/apply-disable-directives.js b/lib/linter/apply-disable-directives.js index bd2502d4f73..425a0316a37 100644 --- a/lib/linter/apply-disable-directives.js +++ b/lib/linter/apply-disable-directives.js @@ -1,3 +1,5 @@ +const escapeRegExp = require("escape-string-regexp"); + /** * @fileoverview A module that filters reported problems based on `eslint-disable` and `eslint-enable` comments * @author Teddy Katz From b5401716d02dff37f5c69ec11e833c3ddc1024e0 Mon Sep 17 00:00:00 2001 From: Josh Goldberg Date: Thu, 29 Jul 2021 21:25:00 -0400 Subject: [PATCH 26/31] Wrong order... --- lib/linter/apply-disable-directives.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/linter/apply-disable-directives.js b/lib/linter/apply-disable-directives.js index 425a0316a37..872fb8da0aa 100644 --- a/lib/linter/apply-disable-directives.js +++ b/lib/linter/apply-disable-directives.js @@ -1,5 +1,3 @@ -const escapeRegExp = require("escape-string-regexp"); - /** * @fileoverview A module that filters reported problems based on `eslint-disable` and `eslint-enable` comments * @author Teddy Katz @@ -7,6 +5,8 @@ const escapeRegExp = require("escape-string-regexp"); "use strict"; +const escapeRegExp = require("escape-string-regexp"); + /** * Compares the locations of two objects in a source file * @param {{line: number, column: number}} itemA The first object From c7606315f9cd6a6c730d58c04c3ecc1af38752b8 Mon Sep 17 00:00:00 2001 From: Josh Goldberg Date: Thu, 29 Jul 2021 21:35:08 -0400 Subject: [PATCH 27/31] Also mention in command-line-interface.md --- docs/user-guide/command-line-interface.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/docs/user-guide/command-line-interface.md b/docs/user-guide/command-line-interface.md index 0d217fb41a7..17b72e6f5d1 100644 --- a/docs/user-guide/command-line-interface.md +++ b/docs/user-guide/command-line-interface.md @@ -239,11 +239,12 @@ This flag can be useful for integrations (e.g. editor plugins) which need to aut #### `--fix-type` -This option allows you to specify the type of fixes to apply when using either `--fix` or `--fix-dry-run`. The three types of fixes are: +This option allows you to specify the type of fixes to apply when using either `--fix` or `--fix-dry-run`. The four types of fixes are: 1. `problem` - fix potential errors in the code 1. `suggestion` - apply fixes to the code that improve it 1. `layout` - apply fixes that do not change the program structure (AST) +1. `directive` - apply fixes to inline directives such as `// eslint-disable` You can specify one or more fix type on the command line. Here are some examples: From a638521408222549c0d8db89295a28aa0e63c556 Mon Sep 17 00:00:00 2001 From: Josh Goldberg Date: Fri, 30 Jul 2021 19:22:18 -0400 Subject: [PATCH 28/31] Respect disableFixes (not yet tested) --- lib/linter/apply-disable-directives.js | 9 ++++++--- lib/linter/linter.js | 1 + 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/lib/linter/apply-disable-directives.js b/lib/linter/apply-disable-directives.js index 872fb8da0aa..1e24cbb8b67 100644 --- a/lib/linter/apply-disable-directives.js +++ b/lib/linter/apply-disable-directives.js @@ -208,7 +208,6 @@ function applyDirectives(options) { const unusedDisableDirectives = processed .map(({ description, fix, position }) => ({ - fix, ruleId: null, message: description ? `Unused eslint-disable directive (no problems were reported from ${description}).` @@ -216,7 +215,8 @@ function applyDirectives(options) { line: position.line, column: position.column, severity: options.reportUnusedDisableDirectives === "warn" ? 1 : 2, - nodeType: null + nodeType: null, + fix: options.disableFixes ? null : fix })); return { problems, unusedDisableDirectives }; @@ -237,10 +237,11 @@ function applyDirectives(options) { * @param {{ruleId: (string|null), line: number, column: number}[]} options.problems * A list of problems reported by rules, sorted by increasing location in the file, with one-based columns. * @param {"off" | "warn" | "error"} options.reportUnusedDisableDirectives If `"warn"` or `"error"`, adds additional problems for unused directives + * @param {boolean} options.disableFixes If true, it doesn't make `fix` properties. * @returns {{ruleId: (string|null), line: number, column: number}[]} * A list of reported problems that were not disabled by the directive comments. */ -module.exports = ({ directives, problems, reportUnusedDisableDirectives = "off" }) => { +module.exports = ({ directives, disableFixes, problems, reportUnusedDisableDirectives = "off" }) => { const blockDirectives = directives .filter(directive => directive.type === "disable" || directive.type === "enable") .map(directive => Object.assign({}, directive, { unprocessedDirective: directive })) @@ -272,11 +273,13 @@ module.exports = ({ directives, problems, reportUnusedDisableDirectives = "off" const blockDirectivesResult = applyDirectives({ problems, directives: blockDirectives, + disableFixes, reportUnusedDisableDirectives }); const lineDirectivesResult = applyDirectives({ problems: blockDirectivesResult.problems, directives: lineDirectives, + disableFixes, reportUnusedDisableDirectives }); diff --git a/lib/linter/linter.js b/lib/linter/linter.js index 9b41f0677c3..bb5d8329bc1 100644 --- a/lib/linter/linter.js +++ b/lib/linter/linter.js @@ -1206,6 +1206,7 @@ class Linter { return applyDisableDirectives({ directives: commentDirectives.disableDirectives, + disableFixes: options.disableFixes, problems: lintingProblems .concat(commentDirectives.problems) .sort((problemA, problemB) => problemA.line - problemB.line || problemA.column - problemB.column), From 7e3ae491157002b0539b840b1628a80121a10d51 Mon Sep 17 00:00:00 2001 From: Josh Goldberg Date: Fri, 30 Jul 2021 19:29:25 -0400 Subject: [PATCH 29/31] Fixed up existing tests --- tests/lib/linter/apply-disable-directives.js | 18 +++++++++--------- tests/lib/linter/linter.js | 2 +- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/tests/lib/linter/apply-disable-directives.js b/tests/lib/linter/apply-disable-directives.js index 32be4409814..3c5d349459a 100644 --- a/tests/lib/linter/apply-disable-directives.js +++ b/tests/lib/linter/apply-disable-directives.js @@ -1369,7 +1369,7 @@ describe("apply-disable-directives", () => { [ { ruleId: null, - message: "Unused eslint-disable directive (no problems were reported from unused).", + message: "Unused eslint-disable directive (no problems were reported from 'unused').", line: 1, column: 22, fix: { @@ -1409,7 +1409,7 @@ describe("apply-disable-directives", () => { [ { ruleId: null, - message: "Unused eslint-disable directive (no problems were reported from unused).", + message: "Unused eslint-disable directive (no problems were reported from 'unused').", line: 1, column: 24, fix: { @@ -1450,7 +1450,7 @@ describe("apply-disable-directives", () => { [ { ruleId: null, - message: "Unused eslint-disable directive (no problems were reported from unused).", + message: "Unused eslint-disable directive (no problems were reported from 'unused').", line: 1, column: 18, fix: { @@ -1491,7 +1491,7 @@ describe("apply-disable-directives", () => { [ { ruleId: null, - message: "Unused eslint-disable directive (no problems were reported from unused).", + message: "Unused eslint-disable directive (no problems were reported from 'unused').", line: 1, column: 18, fix: { @@ -1539,7 +1539,7 @@ describe("apply-disable-directives", () => { [ { ruleId: null, - message: "Unused eslint-disable directive (no problems were reported from unused-1).", + message: "Unused eslint-disable directive (no problems were reported from 'unused-1').", line: 1, column: 18, fix: { @@ -1551,7 +1551,7 @@ describe("apply-disable-directives", () => { }, { ruleId: null, - message: "Unused eslint-disable directive (no problems were reported from unused-2).", + message: "Unused eslint-disable directive (no problems were reported from 'unused-2').", line: 1, column: 28, fix: { @@ -1606,7 +1606,7 @@ describe("apply-disable-directives", () => { [ { ruleId: null, - message: "Unused eslint-disable directive (no problems were reported from unused-1).", + message: "Unused eslint-disable directive (no problems were reported from 'unused-1').", line: 1, column: 18, fix: { @@ -1618,7 +1618,7 @@ describe("apply-disable-directives", () => { }, { ruleId: null, - message: "Unused eslint-disable directive (no problems were reported from unused-2).", + message: "Unused eslint-disable directive (no problems were reported from 'unused-2').", line: 1, column: 28, fix: { @@ -1630,7 +1630,7 @@ describe("apply-disable-directives", () => { }, { ruleId: null, - message: "Unused eslint-disable directive (no problems were reported from unused-3).", + message: "Unused eslint-disable directive (no problems were reported from 'unused-3').", line: 1, column: 43, fix: { diff --git a/tests/lib/linter/linter.js b/tests/lib/linter/linter.js index c59764f7916..4ec8f3f0321 100644 --- a/tests/lib/linter/linter.js +++ b/tests/lib/linter/linter.js @@ -3390,7 +3390,7 @@ var a = "test2"; [ { ruleId: null, - message: "Unused eslint-disable directive (no problems were reported from no-redeclare).", + message: "Unused eslint-disable directive (no problems were reported from 'no-redeclare').", line: 1, column: 16, fix: { From 6edce5e3d69247a9df0590560bab233c09945196 Mon Sep 17 00:00:00 2001 From: Josh Goldberg Date: Fri, 30 Jul 2021 19:35:51 -0400 Subject: [PATCH 30/31] Add unit test for options.disableFixes --- tests/lib/linter/apply-disable-directives.js | 27 ++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/tests/lib/linter/apply-disable-directives.js b/tests/lib/linter/apply-disable-directives.js index 3c5d349459a..c1635756a91 100644 --- a/tests/lib/linter/apply-disable-directives.js +++ b/tests/lib/linter/apply-disable-directives.js @@ -679,6 +679,33 @@ describe("apply-disable-directives", () => { ); }); + it("Does not fix a problem for /* eslint-disable */ when disableFixes is enabled", () => { + assert.deepStrictEqual( + applyDisableDirectives({ + directives: [{ + parentComment: createParentComment([0, 20]), + type: "disable", + line: 1, + column: 1 + }], + disableFixes: true, + problems: [], + reportUnusedDisableDirectives: "error" + }), + [ + { + ruleId: null, + message: "Unused eslint-disable directive (no problems were reported).", + line: 1, + column: 1, + fix: null, + severity: 2, + nodeType: null + } + ] + ); + }); + it("Does not add a problem for /* eslint-disable */ /* (problem) */", () => { assert.deepStrictEqual( applyDisableDirectives({ From 804d1010f10c607e68966157d78c2621dab0c6e2 Mon Sep 17 00:00:00 2001 From: Josh Goldberg Date: Sat, 31 Jul 2021 18:40:55 -0400 Subject: [PATCH 31/31] Apply suggestions from code review Co-authored-by: Milos Djermanovic --- lib/linter/apply-disable-directives.js | 2 +- tests/lib/linter/apply-disable-directives.js | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/lib/linter/apply-disable-directives.js b/lib/linter/apply-disable-directives.js index 1e24cbb8b67..de81c24447f 100644 --- a/lib/linter/apply-disable-directives.js +++ b/lib/linter/apply-disable-directives.js @@ -216,7 +216,7 @@ function applyDirectives(options) { column: position.column, severity: options.reportUnusedDisableDirectives === "warn" ? 1 : 2, nodeType: null, - fix: options.disableFixes ? null : fix + ...options.disableFixes ? {} : { fix } })); return { problems, unusedDisableDirectives }; diff --git a/tests/lib/linter/apply-disable-directives.js b/tests/lib/linter/apply-disable-directives.js index c1635756a91..c0aaf8519c0 100644 --- a/tests/lib/linter/apply-disable-directives.js +++ b/tests/lib/linter/apply-disable-directives.js @@ -698,7 +698,6 @@ describe("apply-disable-directives", () => { message: "Unused eslint-disable directive (no problems were reported).", line: 1, column: 1, - fix: null, severity: 2, nodeType: null }