Skip to content

Commit

Permalink
feat: warn by default for unused disable directives (#17879)
Browse files Browse the repository at this point in the history 
* feat: warn by default for unused disable directives

* change location of default

* fix tests

* update docs

* add tests

* revert configuration-files-new.md

* tweak test name

* revert rule tests
  • Loading branch information
@bmish
bmish committed Dec 28, 2023
1 parent 02a8baf commit 24ce927
Show file tree
Hide file tree
Showing 7 changed files with 107 additions and 20 deletions.
2 changes: 1 addition & 1 deletion docs/src/use/command-line-interface.md
View file
Expand Up @@ -615,7 +615,7 @@ Same as [`--report-unused-disable-directives`](#--report-unused-disable-directiv
1. `warn` (or `1`)
1. `error` (or `2`)
* **Multiple Arguments**: No
* **Default Value**: By default, `linterOptions.reportUnusedDisableDirectives` configuration setting is used.
* **Default Value**: By default, `linterOptions.reportUnusedDisableDirectives` configuration setting is used (which defaults to `"warn"`).

##### `--report-unused-disable-directives-severity` example

Expand Down
4 changes: 3 additions & 1 deletion docs/src/use/configure/configuration-files.md
View file
Expand Up @@ -79,7 +79,7 @@ Each configuration object contains all of the information ESLint needs to execut
* `parserOptions` - An object specifying additional options that are passed directly to the `parse()` or `parseForESLint()` method on the parser. The available options are parser-dependent.
* `linterOptions` - An object containing settings related to the linting process.
* `noInlineConfig` - A Boolean value indicating if inline configuration is allowed.
* `reportUnusedDisableDirectives` - A severity string indicating if and how unused disable and enable directives should be tracked and reported. For legacy compatibility, `true` is equivalent to `"warn"` and `false` is equivalent to `"off"`. (default: `"off"`).
* `reportUnusedDisableDirectives` - A severity string indicating if and how unused disable and enable directives should be tracked and reported. For legacy compatibility, `true` is equivalent to `"warn"` and `false` is equivalent to `"off"`. (default: `"warn"`).
* `processor` - Either an object containing `preprocess()` and `postprocess()` methods or a string indicating the name of a processor inside of a plugin (i.e., `"pluginName/processorName"`).
* `plugins` - An object containing a name-value mapping of plugin names to plugin objects. When `files` is specified, these plugins are only available to the matching files.
* `rules` - An object containing the configured rules. When `files` or `ignores` are specified, these rule configurations are only available to the matching files.
Expand Down Expand Up @@ -251,6 +251,8 @@ export default [
];
```

This setting defaults to `"warn"`.

You can override this setting using the [`--report-unused-disable-directives`](../command-line-interface#--report-unused-disable-directives) or the [`--report-unused-disable-directives-severity`](../command-line-interface#--report-unused-disable-directives-severity) command line options.

For legacy compatibility, `true` is equivalent to `"warn"` and `false` is equivalent to `"off"`.
Expand Down
4 changes: 3 additions & 1 deletion docs/src/use/configure/rules.md
View file
Expand Up @@ -352,10 +352,12 @@ To report unused `eslint-disable` comments, use the `reportUnusedDisableDirectiv
export default [
{
linterOptions: {
reportUnusedDisableDirectives: true
reportUnusedDisableDirectives: "error"
}
}
];
```

This setting defaults to `"warn"`.

This setting is similar to [`--report-unused-disable-directives`](../command-line-interface#--report-unused-disable-directives) and [`--report-unused-disable-directives-severity`](../command-line-interface#--report-unused-disable-directives-severity) CLI options.
3 changes: 3 additions & 0 deletions lib/config/default-config.js
View file
Expand Up @@ -42,6 +42,9 @@ exports.defaultConfig = [
ecmaVersion: "latest",
parser: require("espree"),
parserOptions: {}
},
linterOptions: {
reportUnusedDisableDirectives: 1
}
},

Expand Down
18 changes: 18 additions & 0 deletions tests/lib/cli.js
View file
Expand Up @@ -1545,6 +1545,24 @@ describe("cli", () => {
assert.deepStrictEqual(log.error.firstCall.args, ["The --report-unused-disable-directives option and the --report-unused-disable-directives-severity option cannot be used together."], "has the right text to log.error");
assert.strictEqual(exitCode, 2, "exit code should be 2");
});

it("warns by default in flat config only", async () => {
const exitCode = await cli.execute(`${useFlatConfig ? "--no-config-lookup" : "--no-eslintrc"} --rule "'no-console': 'error'"`,
"foo(); // eslint-disable-line no-console",
useFlatConfig);

if (useFlatConfig) {
assert.strictEqual(log.error.callCount, 0, "log.error should not be called");
assert.strictEqual(log.info.callCount, 1, "log.info is called once");
assert.ok(log.info.firstCall.args[0].includes("Unused eslint-disable directive (no problems were reported from 'no-console')"), "has correct message about unused directives");
assert.ok(log.info.firstCall.args[0].includes("0 errors and 1 warning"), "has correct error and warning count");
assert.strictEqual(exitCode, 0, "exit code should be 0");
} else {
assert.strictEqual(log.error.callCount, 0, "log.error should not be called");
assert.strictEqual(log.info.callCount, 0, "log.info should not be called");
assert.strictEqual(exitCode, 0, "exit code should be 0");
}
});
});
});

Expand Down
33 changes: 33 additions & 0 deletions tests/lib/config/flat-config-array.js
View file
Expand Up @@ -190,6 +190,9 @@ describe("FlatConfigArray", () => {
parser: `espree@${espree.version}`,
parserOptions: {}
},
linterOptions: {
reportUnusedDisableDirectives: 1
},
processor: void 0
};
const actual = config.toJSON();
Expand Down Expand Up @@ -222,6 +225,9 @@ describe("FlatConfigArray", () => {
parser: `espree@${espree.version}`,
parserOptions: {}
},
linterOptions: {
reportUnusedDisableDirectives: 1
},
processor: void 0
};
const actual = config.toJSON();
Expand Down Expand Up @@ -256,6 +262,9 @@ describe("FlatConfigArray", () => {
parser: `espree@${espree.version}`,
parserOptions: {}
},
linterOptions: {
reportUnusedDisableDirectives: 1
},
processor: void 0
};
const actual = config.toJSON();
Expand Down Expand Up @@ -353,6 +362,9 @@ describe("FlatConfigArray", () => {
parserOptions: {},
sourceType: "module"
},
linterOptions: {
reportUnusedDisableDirectives: 1
},
plugins: ["@"],
processor: void 0
});
Expand Down Expand Up @@ -384,6 +396,9 @@ describe("FlatConfigArray", () => {
parserOptions: {},
sourceType: "module"
},
linterOptions: {
reportUnusedDisableDirectives: 1
},
plugins: ["@"],
processor: void 0
});
Expand Down Expand Up @@ -415,6 +430,9 @@ describe("FlatConfigArray", () => {
parserOptions: {},
sourceType: "module"
},
linterOptions: {
reportUnusedDisableDirectives: 1
},
plugins: ["@"],
processor: void 0
});
Expand Down Expand Up @@ -444,6 +462,9 @@ describe("FlatConfigArray", () => {
parserOptions: {},
sourceType: "module"
},
linterOptions: {
reportUnusedDisableDirectives: 1
},
plugins: ["@"],
processor: void 0
});
Expand Down Expand Up @@ -513,6 +534,9 @@ describe("FlatConfigArray", () => {
parserOptions: {},
sourceType: "module"
},
linterOptions: {
reportUnusedDisableDirectives: 1
},
plugins: ["@"],
processor: "custom-processor"
});
Expand Down Expand Up @@ -540,6 +564,9 @@ describe("FlatConfigArray", () => {
parserOptions: {},
sourceType: "module"
},
linterOptions: {
reportUnusedDisableDirectives: 1
},
plugins: ["@"],
processor: "custom-processor"
});
Expand Down Expand Up @@ -571,6 +598,9 @@ describe("FlatConfigArray", () => {
parserOptions: {},
sourceType: "module"
},
linterOptions: {
reportUnusedDisableDirectives: 1
},
plugins: ["@"],
processor: "custom-processor@1.2.3"
});
Expand Down Expand Up @@ -600,6 +630,9 @@ describe("FlatConfigArray", () => {
parserOptions: {},
sourceType: "module"
},
linterOptions: {
reportUnusedDisableDirectives: 1
},
plugins: ["@"],
processor: "custom-processor@1.2.3"
});
Expand Down
63 changes: 46 additions & 17 deletions tests/lib/linter/linter.js
View file
Expand Up @@ -10901,7 +10901,8 @@ describe("Linter with FlatConfigArray", () => {
},
rules: {
"test/checker": "error"
}
},
linterOptions: { reportUnusedDisableDirectives: 0 }
};

const problems = linter.verify(code, config);
Expand Down Expand Up @@ -11036,7 +11037,7 @@ describe("Linter with FlatConfigArray", () => {
"/*eslint-enable*/"
].join("\n");

const config = { rules: { "no-alert": 1 } };
const config = { rules: { "no-alert": 1 }, linterOptions: { reportUnusedDisableDirectives: 0 } };

const messages = linter.verify(code, config, filename);
const suppressedMessages = linter.getSuppressedMessages();
Expand Down Expand Up @@ -11298,7 +11299,7 @@ var a = "test2";
"console.log(\"foo\");",
"/* eslint-enable quotes */"
].join("\n");
const config = { rules: { quotes: 2 } };
const config = { rules: { quotes: 2 }, linterOptions: { reportUnusedDisableDirectives: 0 } };

const messages = linter.verify(code, config, filename);
const suppressedMessages = linter.getSuppressedMessages();
Expand Down Expand Up @@ -11343,7 +11344,7 @@ var a = "test2";
"/*eslint-enable no-console */",
"alert('test');"
].join("\n");
const config = { rules: { "no-alert": 1, "no-console": 1 } };
const config = { rules: { "no-alert": 1, "no-console": 1 }, linterOptions: { reportUnusedDisableDirectives: 0 } };

const messages = linter.verify(code, config, filename);
const suppressedMessages = linter.getSuppressedMessages();
Expand Down Expand Up @@ -11407,7 +11408,7 @@ var a = "test2";

"/*eslint-enable*/"
].join("\n");
const config = { rules: { "no-alert": 1, "no-console": 1 } };
const config = { rules: { "no-alert": 1, "no-console": 1 }, linterOptions: { reportUnusedDisableDirectives: 0 } };

const messages = linter.verify(code, config, filename);
const suppressedMessages = linter.getSuppressedMessages();
Expand Down Expand Up @@ -11446,7 +11447,7 @@ var a = "test2";
"console.log('test');", // here
"/*eslint-enable no-console */"
].join("\n");
const config = { rules: { "no-alert": 1, "no-console": 1 } };
const config = { rules: { "no-alert": 1, "no-console": 1 }, linterOptions: { reportUnusedDisableDirectives: 0 } };

const messages = linter.verify(code, config, filename);
const suppressedMessages = linter.getSuppressedMessages();
Expand Down Expand Up @@ -11485,7 +11486,7 @@ var a = "test2";
"console.log('test');", // here
"/*eslint-enable no-console */"
].join("\n");
const config = { rules: { "no-alert": "warn", "no-console": "warn" } };
const config = { rules: { "no-alert": "warn", "no-console": "warn" }, linterOptions: { reportUnusedDisableDirectives: 0 } };

const messages = linter.verify(code, config, filename);
const suppressedMessages = linter.getSuppressedMessages();
Expand Down Expand Up @@ -11514,7 +11515,7 @@ var a = "test2";
"var bar;",
"/* eslint-enable no-unused-vars */" // here
].join("\n");
const config = { rules: { "no-unused-vars": 2 } };
const config = { rules: { "no-unused-vars": 2 }, linterOptions: { reportUnusedDisableDirectives: 0 } };

const messages = linter.verify(code, config, filename);
const suppressedMessages = linter.getSuppressedMessages();
Expand Down Expand Up @@ -11914,7 +11915,8 @@ var a = "test2";
const config = {
rules: {
"no-alert": 1
}
},
linterOptions: { reportUnusedDisableDirectives: 0 }
};
const messages = linter.verify(code, config, filename);
const suppressedMessages = linter.getSuppressedMessages();
Expand Down Expand Up @@ -11978,7 +11980,8 @@ var a = "test2";
rules: {
"no-alert": 1,
"no-console": 1
}
},
linterOptions: { reportUnusedDisableDirectives: 0 }
};
const messages = linter.verify(code, config, filename);
const suppressedMessages = linter.getSuppressedMessages();
Expand All @@ -12001,7 +12004,8 @@ var a = "test2";
rules: {
"no-alert": 1,
"no-console": 1
}
},
linterOptions: { reportUnusedDisableDirectives: 0 }
};
const messages = linter.verify(code, config, filename);

Expand Down Expand Up @@ -12406,7 +12410,7 @@ var a = "test2";
/*eslint-enable no-redeclare -- no-unused-vars */
var aaa = {}
var aaa = {}
`, { rules: { "no-redeclare": "error", "no-unused-vars": "error" } });
`, { rules: { "no-redeclare": "error", "no-unused-vars": "error" }, linterOptions: { reportUnusedDisableDirectives: 0 } });
const suppressedMessages = linter.getSuppressedMessages();

// Do include `no-redeclare` but not `no-unused-vars`
Expand Down Expand Up @@ -12446,7 +12450,7 @@ var a = "test2";
const messages = linter.verify(`
var aaa = {} //eslint-disable-line no-redeclare -- no-unused-vars
var aaa = {} //eslint-disable-line no-redeclare -- no-unused-vars
`, { rules: { "no-redeclare": "error", "no-unused-vars": "error" } });
`, { rules: { "no-redeclare": "error", "no-unused-vars": "error" }, linterOptions: { reportUnusedDisableDirectives: 0 } });
const suppressedMessages = linter.getSuppressedMessages();

// Do include `no-unused-vars` but not `no-redeclare`
Expand Down Expand Up @@ -12486,7 +12490,7 @@ var a = "test2";
const messages = linter.verify(`
var aaa = {} /*eslint-disable-line no-redeclare -- no-unused-vars */
var aaa = {} /*eslint-disable-line no-redeclare -- no-unused-vars */
`, { rules: { "no-redeclare": "error", "no-unused-vars": "error" } });
`, { rules: { "no-redeclare": "error", "no-unused-vars": "error" }, linterOptions: { reportUnusedDisableDirectives: 0 } });
const suppressedMessages = linter.getSuppressedMessages();

// Do include `no-unused-vars` but not `no-redeclare`
Expand Down Expand Up @@ -12528,7 +12532,7 @@ var a = "test2";
var aaa = {}
//eslint-disable-next-line no-redeclare -- no-unused-vars
var aaa = {}
`, { rules: { "no-redeclare": "error", "no-unused-vars": "error" } });
`, { rules: { "no-redeclare": "error", "no-unused-vars": "error" }, linterOptions: { reportUnusedDisableDirectives: 0 } });
const suppressedMessages = linter.getSuppressedMessages();

// Do include `no-unused-vars` but not `no-redeclare`
Expand Down Expand Up @@ -12570,7 +12574,7 @@ var a = "test2";
var aaa = {}
/*eslint-disable-next-line no-redeclare -- no-unused-vars */
var aaa = {}
`, { rules: { "no-redeclare": "error", "no-unused-vars": "error" } });
`, { rules: { "no-redeclare": "error", "no-unused-vars": "error" }, linterOptions: { reportUnusedDisableDirectives: 0 } });
const suppressedMessages = linter.getSuppressedMessages();

// Do include `no-unused-vars` but not `no-redeclare`
Expand Down Expand Up @@ -13712,7 +13716,7 @@ var a = "test2";
assert.strictEqual(suppressedMessages[1].suppressions.length, 1);
});

it("reports problems for unused eslint-disable comments (warn)", () => {
it("reports problems for unused eslint-disable comments (warn, explicitly set)", () => {
const messages = linter.verify("/* eslint-disable */", {}, { reportUnusedDisableDirectives: "warn" });
const suppressedMessages = linter.getSuppressedMessages();

Expand All @@ -13737,6 +13741,31 @@ var a = "test2";
assert.strictEqual(suppressedMessages.length, 0);
});

it("reports problems for unused eslint-disable comments (warn by default)", () => {
const messages = linter.verify("/* eslint-disable */", {});
const suppressedMessages = linter.getSuppressedMessages();

assert.deepStrictEqual(
messages,
[
{
ruleId: null,
message: "Unused eslint-disable directive (no problems were reported).",
line: 1,
column: 1,
fix: {
range: [0, 20],
text: " "
},
severity: 1,
nodeType: null
}
]
);

assert.strictEqual(suppressedMessages.length, 0);
});

describe("autofix", () => {
const alwaysReportsRule = {
create(context) {
Expand Down

0 comments on commit 24ce927

Please sign in to comment.