Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

feat: warn by default for unused disable directives #17879

Merged
merged 11 commits into from Dec 28, 2023
Merged

feat: warn by default for unused disable directives #17879

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
bdfc123
feat: warn by default for unused disable directives
bmish Dec 14, 2023
ae4e075
Merge branch 'main' into rfc-100-part2
bmish Dec 21, 2023
0aebc8a
Merge branch 'main' into rfc-100-part2
bmish Dec 23, 2023
b3f6e9b
change location of default
bmish Dec 27, 2023
e3ce7dc
fix tests
bmish Dec 27, 2023
7023416
Merge branch 'main' into rfc-100-part2
bmish Dec 27, 2023
ec2da97
update docs
bmish Dec 27, 2023
ab13ef5
add tests
bmish Dec 27, 2023
e4ff4f4
revert configuration-files-new.md
bmish Dec 27, 2023
2ed2f87
tweak test name
bmish Dec 27, 2023
07352ef
revert rule tests
bmish Dec 27, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion docs/src/use/command-line-interface.md
View file
Open in desktop
Expand Up @@ -611,7 +611,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
Open in desktop
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
6 changes: 4 additions & 2 deletions docs/src/use/configure/rules.md
View file
Open in desktop
Expand Up @@ -339,7 +339,7 @@ export default [
rules: {
"no-unused-expressions": "error"
}
}
}
];
```

Expand All @@ -354,10 +354,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
Open in desktop
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
Open in desktop
Expand Up @@ -1523,6 +1523,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
Open in desktop
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
Open in desktop
Expand Up @@ -10590,7 +10590,8 @@ describe("Linter with FlatConfigArray", () => {
},
rules: {
"test/checker": "error"
}
},
linterOptions: { reportUnusedDisableDirectives: 0 }
};

const problems = linter.verify(code, config);
Expand Down Expand Up @@ -10725,7 +10726,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 @@ -10987,7 +10988,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 @@ -11032,7 +11033,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 @@ -11096,7 +11097,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 @@ -11135,7 +11136,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 @@ -11174,7 +11175,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 @@ -11203,7 +11204,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 @@ -11603,7 +11604,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 @@ -11667,7 +11669,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 @@ -11690,7 +11693,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 @@ -12095,7 +12099,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 @@ -12135,7 +12139,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 @@ -12175,7 +12179,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 @@ -12217,7 +12221,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 @@ -12259,7 +12263,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 @@ -13401,7 +13405,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 @@ -13426,6 +13430,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