From 1d2213deb69c5901c1950bbe648aa819e7e742ed Mon Sep 17 00:00:00 2001 From: Josh Goldberg Date: Thu, 5 Aug 2021 16:09:32 -0400 Subject: [PATCH] Breaking: Fixable disable directives (fixes #11815) (#14617) * New: Fixable disable directives * Part of the way there: not ready yet * Generally fixed up, and just started on unit tests * lodash.flatMap * Merge branch 'master' * Progress... * Fixed the remaining pre-existing unit tests * Fixed up some tests * unprocessedDirective.parentComment * Avoided rescan by passing commentToken in parentComment * A lil edge case * Fix directive grouping the other direction * Incorrect state directive reference * Simplified parentComment / commentToken range * Use Map in lib/linter/apply-disable-directives.js Co-authored-by: Milos Djermanovic * Used suggestion for individual directives removal * Boolean in lib/cli-engine/cli-engine.js Co-authored-by: Milos Djermanovic * Docs fixups * Added tests * Tests and fix for multiple rule lists * Remove unused sourceCode from tests * Added high-level test in linter.js * Apply suggestions from code review Co-authored-by: Brandon Mills * Apply suggestions from code review Co-authored-by: Milos Djermanovic * Added require * Wrong order... * Also mention in command-line-interface.md * Respect disableFixes (not yet tested) * Fixed up existing tests * Add unit test for options.disableFixes * Apply suggestions from code review Co-authored-by: Milos Djermanovic Co-authored-by: Milos Djermanovic Co-authored-by: Brandon Mills --- docs/developer-guide/nodejs-api.md | 2 +- docs/user-guide/command-line-interface.md | 5 +- lib/cli-engine/cli-engine.js | 26 +- lib/eslint/eslint.js | 4 +- lib/linter/apply-disable-directives.js | 159 ++- lib/linter/linter.js | 13 +- lib/options.js | 4 +- tests/lib/cli-engine/cli-engine.js | 8 +- tests/lib/eslint/eslint.js | 10 +- tests/lib/linter/apply-disable-directives.js | 1079 ++++++++++++++++-- tests/lib/linter/linter.js | 50 + 11 files changed, 1210 insertions(+), 150 deletions(-) diff --git a/docs/developer-guide/nodejs-api.md b/docs/developer-guide/nodejs-api.md index d21ee7b7a04..593a1f71bb4 100644 --- a/docs/developer-guide/nodejs-api.md +++ b/docs/developer-guide/nodejs-api.md @@ -148,7 +148,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/docs/user-guide/command-line-interface.md b/docs/user-guide/command-line-interface.md index e86611f6765..34c03beb292 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 @@ -240,11 +240,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: diff --git a/lib/cli-engine/cli-engine.js b/lib/cli-engine/cli-engine.js index 105de3e55cc..3ba0dfe0d9a 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 @@ -331,6 +331,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 Boolean(rule && rule.meta && fixTypes.has(rule.meta.type)); +} + /** * Collect used deprecated rules. * @param {ConfigArray[]} usedConfigArrays The config arrays which were used. @@ -623,12 +640,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 056e04b5945..62229be8068 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 0ba69ca9cc4..de81c24447f 100644 --- a/lib/linter/apply-disable-directives.js +++ b/lib/linter/apply-disable-directives.js @@ -5,6 +5,8 @@ "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 @@ -16,6 +18,123 @@ function compareLocations(itemA, itemB) { return itemA.line - itemB.line || itemA.column - itemB.column; } +/** + * 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 groupByParentComment(directives) { + const groups = new Map(); + + for (const directive of directives) { + const { unprocessedDirective: { parentComment } } = directive; + + if (groups.has(parentComment)) { + groups.get(parentComment).push(directive); + } else { + groups.set(parentComment, [directive]); + } + } + + return [...groups.values()]; +} + +/** + * 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 {{ 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; + 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 { ruleId } = directive; + 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}'`, + fix: { + range: [ + listStart + ruleOffset + (ruleText.startsWith(",") && ruleText.endsWith(",") ? 1 : 0), + listStart + ruleEndOffset + ], + text: "" + }, + position: directive.unprocessedDirective + }; + }); +} + +/** + * 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 {{ description, fix, position }} Details for later creation of an output Problem. + */ +function createCommentRemoval(directives, commentToken) { + const { range } = commentToken; + const ruleIds = directives.filter(directive => directive.ruleId).map(directive => `'${directive.ruleId}'`); + + return { + description: ruleIds.length <= 2 + ? ruleIds.join(" or ") + : `${ruleIds.slice(0, ruleIds.length - 1).join(", ")}, or ${ruleIds[ruleIds.length - 1]}`, + fix: { + range, + text: " " + }, + position: directives[0].unprocessedDirective + }; +} + +/** + * 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. + * @returns {{ description, fix, position }[]} Details for later creation of output Problems. + */ +function processUnusedDisableDirectives(allDirectives) { + const directiveGroups = groupByParentComment(allDirectives); + + return flatMap( + directiveGroups, + directives => { + const { parentComment } = directives[0].unprocessedDirective; + const remainingRuleIds = new Set(parentComment.ruleIds); + + for (const directive of directives) { + remainingRuleIds.delete(directive.ruleId); + } + + return remainingRuleIds.size + ? createIndividualDirectivesRemoval(directives, parentComment.commentToken) + : [createCommentRemoval(directives, parentComment.commentToken)]; + } + ); +} + /** * 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 @@ -82,17 +201,22 @@ function applyDirectives(options) { } } - const unusedDisableDirectives = options.directives - .filter(directive => directive.type === "disable" && !usedDisableDirectives.has(directive)) - .map(directive => ({ + const unusedDisableDirectivesToReport = options.directives + .filter(directive => directive.type === "disable" && !usedDisableDirectives.has(directive)); + + const processed = processUnusedDisableDirectives(unusedDisableDirectivesToReport); + + const unusedDisableDirectives = processed + .map(({ description, fix, position }) => ({ 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 + nodeType: null, + ...options.disableFixes ? {} : { fix } })); return { problems, unusedDisableDirectives }; @@ -113,29 +237,16 @@ 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 })) .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": @@ -162,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 6e3b13e33c1..76a3a2062fe 100644 --- a/lib/linter/linter.js +++ b/lib/linter/linter.js @@ -242,14 +242,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 = { @@ -257,13 +257,15 @@ function createDisableDirectives(options) { directiveProblems: [] // problems in directives }; + 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({ 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; @@ -340,7 +342,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); @@ -1216,6 +1218,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), diff --git a/lib/options.js b/lib/options.js index e3661ec81d5..2dd186de3e8 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 @@ -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/tests/lib/cli-engine/cli-engine.js b/tests/lib/cli-engine/cli-engine.js index 996778822f2..b7a82ff2209 100644 --- a/tests/lib/cli-engine/cli-engine.js +++ b/tests/lib/cli-engine/cli-engine.js @@ -5098,6 +5098,10 @@ describe("CLIEngine", () => { message: "Unused eslint-disable directive (no problems were reported).", line: 1, column: 1, + fix: { + range: [0, 20], + text: " " + }, severity: 2, nodeType: null } @@ -5105,7 +5109,7 @@ describe("CLIEngine", () => { errorCount: 1, warningCount: 0, fatalErrorCount: 0, - fixableErrorCount: 0, + fixableErrorCount: 1, fixableWarningCount: 0, source: "/* eslint-disable */" } @@ -5113,7 +5117,7 @@ describe("CLIEngine", () => { errorCount: 1, warningCount: 0, fatalErrorCount: 0, - fixableErrorCount: 0, + fixableErrorCount: 1, fixableWarningCount: 0, usedDeprecatedRules: [] } diff --git a/tests/lib/eslint/eslint.js b/tests/lib/eslint/eslint.js index 02b518570b8..0a5b5e1a9a4 100644 --- a/tests/lib/eslint/eslint.js +++ b/tests/lib/eslint/eslint.js @@ -199,7 +199,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.", @@ -443,7 +443,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 () => { @@ -5009,6 +5009,10 @@ describe("ESLint", () => { message: "Unused eslint-disable directive (no problems were reported).", line: 1, column: 1, + fix: { + range: [0, 20], + text: " " + }, severity: 2, nodeType: null } @@ -5016,7 +5020,7 @@ describe("ESLint", () => { errorCount: 1, warningCount: 0, fatalErrorCount: 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..c0aaf8519c0 100644 --- a/tests/lib/linter/apply-disable-directives.js +++ b/tests/lib/linter/apply-disable-directives.js @@ -8,12 +8,26 @@ const assert = require("chai").assert; const applyDisableDirectives = require("../../../lib/linter/apply-disable-directives"); +/** + * Creates a ParentComment for a given range. + * @param {[number, number]} range total range of 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, value, ruleIds = []) { + return { + commentToken: { range, value }, + 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([0, 7]), type: "disable", line: 1, column: 8, ruleId: null }], problems: [{ line: 1, column: 7, ruleId: "foo" }] }), [{ ruleId: "foo", line: 1, column: 7 }] @@ -23,7 +37,7 @@ 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 }], + directives: [{ parentComment: createParentComment([21, 27]), type: "disable", line: 2, column: 1, ruleId: null }], problems: [{ line: 1, column: 10, ruleId: "foo" }] }), [{ ruleId: "foo", line: 1, column: 10 }] @@ -85,7 +99,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: 8, ruleId: "foo" }], + directives: [{ + parentComment: createParentComment([26, 29]), + type: "disable", + line: 1, + column: 1, + ruleId: "foo" + }], problems: [{ line: 2, column: 3, ruleId: "not-foo" }] }), [{ line: 2, column: 3, ruleId: "not-foo" }] @@ -95,7 +115,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([7, 31]), + type: "disable", + line: 1, + column: 8, + ruleId: "foo" + }], problems: [{ line: 1, column: 7, ruleId: "foo" }] }), [{ line: 1, column: 7, ruleId: "foo" }] @@ -108,12 +134,24 @@ describe("apply-disable-directives", () => { assert.deepStrictEqual( applyDisableDirectives({ directives: [ - { type: "disable", line: 1, column: 1, ruleId: null }, - { type: "enable", line: 1, column: 5, ruleId: null } + { + parentComment: createParentComment([0, 26]), + type: "disable", + line: 1, + column: 1, + ruleId: null + }, + { + parentComment: createParentComment([27, 45]), + type: "enable", + line: 1, + column: 26, + ruleId: null + } ], - problems: [{ line: 1, column: 7, ruleId: "foo" }] + problems: [{ line: 1, column: 27, ruleId: "foo" }] }), - [{ line: 1, column: 7, ruleId: "foo" }] + [{ line: 1, column: 27, ruleId: "foo" }] ); }); @@ -121,12 +159,24 @@ describe("apply-disable-directives", () => { assert.deepStrictEqual( applyDisableDirectives({ directives: [ - { type: "disable", line: 1, column: 1, ruleId: null }, - { type: "enable", line: 1, column: 5, ruleId: null } + { + parentComment: createParentComment([0, 25]), + type: "disable", + line: 1, + column: 1, + ruleId: null + }, + { + parentComment: createParentComment([26, 40]), + type: "enable", + line: 1, + column: 26, + ruleId: null + } ], - problems: [{ line: 1, column: 5, ruleId: "foo" }] + problems: [{ line: 1, column: 26, ruleId: "foo" }] }), - [{ line: 1, column: 5, ruleId: "foo" }] + [{ line: 1, column: 26, ruleId: "foo" }] ); }); @@ -135,7 +185,7 @@ 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" }] }), @@ -147,9 +197,27 @@ describe("apply-disable-directives", () => { assert.deepStrictEqual( applyDisableDirectives({ directives: [ - { type: "disable", line: 1, column: 1, ruleId: null }, - { type: "enable", line: 1, column: 5, ruleId: "foo" }, - { type: "disable", line: 2, column: 1, ruleId: "foo" } + { + parentComment: createParentComment([0, 20]), + type: "disable", + line: 1, + column: 1, + ruleId: null + }, + { + parentComment: createParentComment([26, 44]), + type: "enable", + line: 1, + column: 26, + ruleId: "foo" + }, + { + parentComment: createParentComment([45, 63]), + type: "disable", + line: 2, + column: 1, + ruleId: "foo" + } ], problems: [{ line: 3, column: 3, ruleId: "foo" }] }), @@ -161,9 +229,27 @@ describe("apply-disable-directives", () => { assert.deepStrictEqual( applyDisableDirectives({ directives: [ - { type: "disable", line: 1, column: 1, ruleId: null }, - { type: "enable", line: 1, column: 5, ruleId: "foo" }, - { type: "disable", line: 2, column: 1, ruleId: null } + { + parentComment: createParentComment([0, 20]), + type: "disable", + line: 1, + column: 1, + ruleId: null + }, + { + parentComment: createParentComment([21, 44]), + type: "enable", + line: 1, + column: 26, + ruleId: "foo" + }, + { + parentComment: createParentComment([45, 63]), + type: "disable", + line: 2, + column: 1, + ruleId: null + } ], problems: [{ line: 3, column: 3, ruleId: "foo" }] }), @@ -175,8 +261,20 @@ describe("apply-disable-directives", () => { assert.deepStrictEqual( applyDisableDirectives({ directives: [ - { type: "disable", line: 1, column: 1, ruleId: "foo" }, - { type: "enable", line: 1, column: 5, ruleId: null } + { + parentComment: createParentComment([0, 20]), + type: "disable", + line: 1, + column: 1, + ruleId: "foo" + }, + { + parentComment: createParentComment([25, 44]), + type: "enable", + line: 1, + column: 26, + ruleId: null + } ], problems: [{ line: 1, column: 3, ruleId: "not-foo" }] }), @@ -190,8 +288,20 @@ describe("apply-disable-directives", () => { assert.deepStrictEqual( applyDisableDirectives({ directives: [ - { type: "disable", line: 1, column: 4, ruleId: null }, - { type: "enable", line: 2, column: 1, ruleId: "foo" } + { + parentComment: createParentComment([0, 20]), + type: "disable", + line: 1, + column: 1, + ruleId: null + }, + { + parentComment: createParentComment([21, 44]), + type: "enable", + line: 2, + column: 1, + ruleId: "foo" + } ], problems: [{ line: 2, column: 4, ruleId: "foo" }] }), @@ -203,8 +313,20 @@ describe("apply-disable-directives", () => { assert.deepStrictEqual( applyDisableDirectives({ directives: [ - { type: "disable", line: 1, column: 4, ruleId: null }, - { type: "enable", line: 2, column: 1, ruleId: "foo" } + { + parentComment: createParentComment([0, 20]), + type: "disable", + line: 1, + column: 1, + ruleId: null + }, + { + parentComment: createParentComment([21, 44]), + type: "enable", + line: 2, + column: 1, + ruleId: "foo" + } ], problems: [{ line: 2, column: 1, ruleId: "foo" }] }), @@ -216,7 +338,7 @@ 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" }] @@ -230,22 +352,22 @@ 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" } ] }), [ - { 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" } ] ); }); @@ -255,7 +377,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([6, 27]), + type: "disable-line", + line: 2, + column: 1, + ruleId: null + }], problems: [{ line: 1, column: 5, ruleId: "foo" }] }), [{ line: 1, column: 5, ruleId: "foo" }] @@ -265,7 +393,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: 5, ruleId: null }], + directives: [{ + parentComment: createParentComment([7, 28]), + type: "disable-line", + line: 1, + column: 8, + ruleId: null + }], problems: [{ line: 1, column: 1, ruleId: "foo" }] }), [] @@ -275,7 +409,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: 5, ruleId: null }], + directives: [{ + parentComment: createParentComment([7, 28]), + type: "disable-line", + line: 1, + column: 8, + ruleId: null + }], problems: [{ line: 1, column: 10, ruleId: "foo" }] }), [] @@ -285,7 +425,13 @@ describe("apply-disable-directives", () => { it("keeps problems on a following line", () => { assert.deepStrictEqual( applyDisableDirectives({ - directives: [{ type: "disable-line", line: 1, column: 4 }], + directives: [{ + parentComment: createParentComment([7, 34]), + type: "disable-line", + line: 1, + column: 8, + ruleId: "foo" + }], problems: [{ line: 2, column: 1, ruleId: "foo" }] }), [{ line: 2, column: 1, ruleId: "foo" }] @@ -297,7 +443,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: 4, ruleId: "foo" }], + directives: [{ + parentComment: createParentComment([7, 34]), + type: "disable-line", + line: 1, + column: 8, + ruleId: "foo" + }], problems: [{ line: 1, column: 2, ruleId: "foo" }] }), [] @@ -307,7 +459,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: 4, ruleId: "foo" }], + directives: [{ parentComment: createParentComment([0, 27]), type: "disable-line", line: 1, column: 1, ruleId: "foo" }], problems: [{ line: 1, column: 2, ruleId: "not-foo" }] }), [{ line: 1, column: 2, ruleId: "not-foo" }] @@ -318,8 +470,20 @@ describe("apply-disable-directives", () => { assert.deepStrictEqual( applyDisableDirectives({ directives: [ - { type: "disable", line: 1, column: 1, ruleId: null }, - { type: "disable-line", line: 1, column: 3, ruleId: "foo" } + { + parentComment: createParentComment([0, 21]), + type: "disable", + line: 1, + column: 1, + ruleId: null + }, + { + parentComment: createParentComment([24, 28]), + type: "disable-line", + line: 1, + column: 22, + ruleId: "foo" + } ], problems: [{ line: 1, column: 5, ruleId: "not-foo" }] }), @@ -331,12 +495,48 @@ 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" } + { + parentComment: createParentComment([7, 34]), + type: "disable-line", + line: 1, + column: 8, + ruleId: "foo" + }, + { + parentComment: createParentComment([38, 73]), + type: "disable-line", + line: 2, + column: 8, + ruleId: "foo" + }, + { + parentComment: createParentComment([76, 111]), + type: "disable-line", + line: 3, + column: 8, + ruleId: "foo" + }, + { + parentComment: createParentComment([114, 149]), + type: "disable-line", + line: 4, + column: 8, + ruleId: "foo" + }, + { + parentComment: createParentComment([152, 187]), + type: "disable-line", + line: 5, + column: 8, + ruleId: "foo" + }, + { + parentComment: createParentComment([190, 225]), + type: "disable-line", + line: 6, + column: 8, + ruleId: "foo" + } ], problems: [{ line: 2, column: 1, ruleId: "foo" }] }), @@ -349,7 +549,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([0, 31]), + type: "disable-next-line", + line: 1, + column: 1, + ruleId: null + }], problems: [{ line: 2, column: 3, ruleId: "foo" }] }), [] @@ -359,7 +565,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([0, 31]), + type: "disable-next-line", + line: 1, + column: 1, + ruleId: null + }], problems: [{ line: 1, column: 3, ruleId: "foo" }] }), [{ line: 1, column: 3, ruleId: "foo" }] @@ -369,7 +581,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([0, 31]), + type: "disable-next-line", + line: 1, + column: 1, + ruleId: null + }], problems: [{ line: 3, column: 3, ruleId: "foo" }] }), [{ line: 3, column: 3, ruleId: "foo" }] @@ -404,7 +622,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([0, 31]), + type: "disable-next-line", + line: 1, + column: 1, + ruleId: "foo" + }], problems: [{ line: 2, column: 1, ruleId: "not-foo" }] }), [{ line: 2, column: 1, ruleId: "not-foo" }] @@ -429,7 +653,42 @@ describe("apply-disable-directives", () => { it("Adds a problem for /* eslint-disable */", () => { assert.deepStrictEqual( applyDisableDirectives({ - directives: [{ type: "disable", line: 1, column: 5 }], + directives: [{ + parentComment: createParentComment([0, 20]), + type: "disable", + line: 1, + column: 1 + }], + problems: [], + reportUnusedDisableDirectives: "error" + }), + [ + { + ruleId: null, + message: "Unused eslint-disable directive (no problems were reported).", + line: 1, + column: 1, + fix: { + range: [0, 20], + text: " " + }, + severity: 2, + nodeType: null + } + ] + ); + }); + + 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" }), @@ -438,7 +697,7 @@ describe("apply-disable-directives", () => { ruleId: null, message: "Unused eslint-disable directive (no problems were reported).", line: 1, - column: 5, + column: 1, severity: 2, nodeType: null } @@ -449,7 +708,7 @@ 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" }), @@ -460,7 +719,13 @@ 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: [{ + parentComment: createParentComment([0, 21]), + type: "disable", + line: 1, + column: 1, + ruleId: "foo" + }], problems: [], reportUnusedDisableDirectives: "error" }), @@ -469,7 +734,11 @@ describe("apply-disable-directives", () => { ruleId: null, message: "Unused eslint-disable directive (no problems were reported from 'foo').", line: 1, - column: 5, + column: 1, + fix: { + range: [0, 21], + text: " " + }, severity: 2, nodeType: null } @@ -480,7 +749,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: 5, ruleId: "foo" }], + directives: [{ + parentComment: createParentComment([0, 24]), + type: "disable", + line: 1, + column: 1, + ruleId: "foo" + }], problems: [{ line: 1, column: 20, ruleId: "not-foo" }], reportUnusedDisableDirectives: "error" }), @@ -489,7 +764,11 @@ describe("apply-disable-directives", () => { 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,8 +785,20 @@ describe("apply-disable-directives", () => { assert.deepStrictEqual( applyDisableDirectives({ directives: [ - { type: "disable", line: 1, column: 5, ruleId: null }, - { type: "enable", line: 1, column: 6, ruleId: "foo" } + { + parentComment: createParentComment([0, 21]), + type: "disable", + line: 1, + column: 8, + ruleId: null + }, + { + parentComment: createParentComment([0, 21]), + type: "enable", + line: 1, + column: 24, + ruleId: "foo" + } ], problems: [{ line: 1, column: 2, ruleId: "foo" }], reportUnusedDisableDirectives: "error" @@ -521,8 +812,12 @@ describe("apply-disable-directives", () => { { ruleId: null, message: "Unused eslint-disable directive (no problems were reported).", + fix: { + range: [0, 21], + text: " " + }, line: 1, - column: 5, + column: 8, severity: 2, nodeType: null } @@ -534,8 +829,20 @@ describe("apply-disable-directives", () => { assert.deepStrictEqual( applyDisableDirectives({ directives: [ - { type: "disable", line: 1, column: 5, ruleId: null }, - { type: "enable", line: 1, column: 6, ruleId: null } + { + parentComment: createParentComment([0, 20]), + type: "disable", + line: 1, + column: 1, + ruleId: null + }, + { + parentComment: createParentComment([21, 41]), + type: "enable", + line: 1, + column: 12, + ruleId: null + } ], problems: [], reportUnusedDisableDirectives: "error" @@ -545,7 +852,11 @@ describe("apply-disable-directives", () => { 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 } @@ -557,8 +868,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([0, 21]), + type: "disable", + line: 1, + column: 1, + ruleId: null + }, + { + parentComment: createParentComment([21, 42]), + type: "disable", + line: 2, + column: 1, + ruleId: null + } ], problems: [], reportUnusedDisableDirectives: "error" @@ -569,6 +892,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 +904,10 @@ describe("apply-disable-directives", () => { message: "Unused eslint-disable directive (no problems were reported).", line: 2, column: 1, + fix: { + range: [21, 42], + text: " " + }, severity: 2, nodeType: null } @@ -588,8 +919,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([0, 21]), + type: "disable", + line: 1, + column: 1, + ruleId: null + }, + { + parentComment: createParentComment([22, 45]), + type: "disable", + line: 2, + column: 1, + ruleId: null + } ], problems: [{ line: 3, column: 1, ruleId: "foo" }], reportUnusedDisableDirectives: "error" @@ -600,6 +943,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 } @@ -611,8 +958,20 @@ 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([0, 21]), + type: "disable", + line: 1, + column: 1, + ruleId: "foo" + }, + { + parentComment: createParentComment([22, 45]), + type: "disable", + line: 2, + column: 1, + ruleId: null + } ], problems: [{ line: 3, column: 1, ruleId: "foo" }], reportUnusedDisableDirectives: "error" @@ -623,6 +982,10 @@ describe("apply-disable-directives", () => { message: "Unused eslint-disable directive (no problems were reported from 'foo').", line: 1, column: 1, + fix: { + range: [0, 21], + text: " " + }, severity: 2, nodeType: null } @@ -633,7 +996,7 @@ 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" }), @@ -645,8 +1008,20 @@ 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([0, 21]), + type: "disable", + line: 1, + column: 1, + ruleId: null + }, + { + parentComment: createParentComment([22, 45]), + type: "disable", + line: 2, + column: 1, + ruleId: "foo" + } ], problems: [{ line: 3, column: 1, ruleId: "foo" }], reportUnusedDisableDirectives: "error" @@ -657,6 +1032,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 } @@ -668,8 +1047,20 @@ 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([0, 20]), + type: "disable", + line: 1, + column: 1, + ruleId: null + }, + { + parentComment: createParentComment([21, 45]), + type: "disable", + line: 2, + column: 1, + ruleId: "foo" + } ], problems: [{ line: 3, column: 1, ruleId: "bar" }], reportUnusedDisableDirectives: "error" @@ -680,6 +1071,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,10 +1086,22 @@ describe("apply-disable-directives", () => { assert.deepStrictEqual( applyDisableDirectives({ directives: [ - { type: "disable", line: 1, column: 5, ruleId: "foo" }, - { type: "enable", line: 1, column: 8, ruleId: "foo" } + { + parentComment: createParentComment([0, 20]), + type: "disable", + line: 1, + column: 1, + ruleId: "foo" + }, + { + parentComment: createParentComment([25, 46]), + type: "enable", + line: 1, + column: 26, + ruleId: "foo" + } ], - problems: [{ line: 1, column: 10, ruleId: "foo" }], + problems: [{ line: 1, column: 30, ruleId: "foo" }], reportUnusedDisableDirectives: "error" }), [ @@ -702,14 +1109,18 @@ describe("apply-disable-directives", () => { 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,10 +1130,22 @@ describe("apply-disable-directives", () => { assert.deepStrictEqual( applyDisableDirectives({ directives: [ - { type: "disable", line: 1, column: 5, ruleId: "foo" }, - { type: "enable", line: 1, column: 8, ruleId: null } + { + parentComment: createParentComment([0, 24]), + type: "disable", + line: 1, + column: 1, + ruleId: "foo" + }, + { + parentComment: createParentComment([25, 49]), + type: "enable", + line: 1, + column: 26, + ruleId: null + } ], - problems: [{ line: 1, column: 10, ruleId: "foo" }], + problems: [{ line: 1, column: 30, ruleId: "foo" }], reportUnusedDisableDirectives: "error" }), [ @@ -730,14 +1153,18 @@ describe("apply-disable-directives", () => { 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 } ] ); @@ -747,9 +1174,27 @@ 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([0, 21]), + type: "disable", + line: 1, + column: 1, + ruleId: null + }, + { + parentComment: createParentComment([22, 45]), + type: "disable", + line: 2, + column: 1, + ruleId: "foo" + }, + { + parentComment: createParentComment([46, 69]), + type: "enable", + line: 3, + column: 1, + ruleId: "foo" + } ], problems: [{ line: 4, column: 1, ruleId: "foo" }], reportUnusedDisableDirectives: "error" @@ -760,6 +1205,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 +1217,11 @@ describe("apply-disable-directives", () => { message: "Unused eslint-disable directive (no problems were reported from 'foo').", line: 2, column: 1, + fix: { + range: [22, 45], + text: " " + }, severity: 2, - nodeType: null }, { @@ -784,7 +1236,13 @@ 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: [{ + parentComment: createParentComment([0, 22]), + type: "disable-line", + line: 1, + column: 1, + ruleId: null + }], problems: [], reportUnusedDisableDirectives: "error" }), @@ -793,7 +1251,11 @@ describe("apply-disable-directives", () => { 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,7 +1267,7 @@ 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" }), @@ -816,7 +1278,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: 5, ruleId: null }], + directives: [{ + parentComment: createParentComment([0, 27]), + type: "disable-next-line", + line: 1, + column: 1, + ruleId: null + }], problems: [], reportUnusedDisableDirectives: "error" }), @@ -825,7 +1293,11 @@ describe("apply-disable-directives", () => { 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,7 +1308,7 @@ 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" }), @@ -848,8 +1320,8 @@ describe("apply-disable-directives", () => { assert.deepStrictEqual( applyDisableDirectives({ directives: [ - { type: "disable", line: 1, column: 1, ruleId: null }, - { type: "disable-line", line: 1, column: 5, 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" @@ -860,6 +1332,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 +1343,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,7 +1358,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: 5, ruleId: null }], + directives: [{ parentComment: createParentComment([0, 27]), type: "disable-next-line", line: 1, column: 1, ruleId: null }], problems: [], reportUnusedDisableDirectives: "off" }), @@ -886,4 +1366,397 @@ 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 + } + ] + ); + }); + + 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 + } + ] + ); + }); + }); }); diff --git a/tests/lib/linter/linter.js b/tests/lib/linter/linter.js index 25533506e6b..0da677d943f 100644 --- a/tests/lib/linter/linter.js +++ b/tests/lib/linter/linter.js @@ -3302,6 +3302,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 } @@ -3318,6 +3322,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 } @@ -3334,6 +3342,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 } @@ -3350,6 +3362,44 @@ 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 + } + ] + ); + }); + + 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 }