Skip to content

Commit 348f46c

Browse files
christianvueringscamc314
christianvuerings
and
camc314
authored
feat(linter): add respectEslintDisableDirectives option (#21384)
## Summary - add a root-only `options.respectEslintDisableDirectives` config option - make directive parsing and `reportUnusedDisableDirectives` honor that switch - preserve current behavior by default with `true` - regenerate the schema / generated TS config types and add CLI + unit coverage ## Proposal Use a boolean option to control whether oxlint recognizes ESLint-style disable directives: ```json { "options": { "respectEslintDisableDirectives": false } } ``` Behavior: - unset or `true`: oxlint recognizes both `oxlint-*` and `eslint-*` - `false`: oxlint only recognizes `oxlint-*` This also applies to `reportUnusedDisableDirectives`. ## Notes - unused-directive diagnostics now report the actual matched prefix (`oxlint-disable` vs `eslint-disable`) instead of always saying `eslint-disable` - the option is root-only, consistent with the other global `options.*` switches in the config loader closes #20447 fixes #20375 --------- Co-authored-by: Cameron Clark <[email protected]>
1 parent 0aa13eb commit 348f46c

19 files changed

Lines changed: 569 additions & 65 deletions

File tree

‎apps/oxlint/fixtures/cli/report_unused_directives_oxlint_only/.oxlintrc.json‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
{
2+
"options": {
3+
"respectEslintDisableDirectives": false
4+
},
5+
"categories": {
6+
"correctness": "off"
7+
},
8+
"rules": {
9+
"no-console": "warn",
10+
"no-debugger": "warn"
11+
}
12+
}

‎apps/oxlint/fixtures/cli/report_unused_directives_oxlint_only/test.js‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
// eslint-disable-next-line no-console
2+
console.log("eslint prefix is ignored");
3+
4+
// oxlint-disable-next-line no-console
5+
console.log("oxlint prefix still suppresses");
6+
7+
// oxlint-disable-next-line no-debugger
8+
console.log("oxlint unused");
9+
10+
// oxlint-disable-next-line no-console
11+
debugger;
12+
13+
// eslint-enable
14+
// oxlint-enable

‎apps/oxlint/fixtures/cli/report_unused_directives_oxlint_only/test.vue‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
<script>
2+
3+
// eslint-disable-next-line no-console
4+
console.log("eslint prefix is ignored");
5+
6+
// oxlint-disable-next-line no-console
7+
console.log("oxlint prefix still suppresses");
8+
9+
// oxlint-disable-next-line no-debugger
10+
console.log("oxlint unused");
11+
12+
// oxlint-disable-next-line no-console
13+
debugger;
14+
15+
// eslint-enable
16+
// oxlint-enable
17+
18+
</script>

‎apps/oxlint/src-js/package/config.generated.ts‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -448,6 +448,14 @@ export interface OxlintOptions {
448448
* Only supported in the root configuration file.
449449
*/
450450
reportUnusedDisableDirectives?: AllowWarnDeny;
451+
/**
452+
* Whether oxlint should respect `eslint-disable*` and `eslint-enable*`
453+
* directives in addition to its native `oxlint-*` directives.
454+
*
455+
* Defaults to `true`.
456+
* Only supported in the root configuration file.
457+
*/
458+
respectEslintDisableDirectives?: boolean;
451459
/**
452460
* Enable rules that require type information.
453461
*

‎apps/oxlint/src/config_loader.rs‎

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -435,6 +435,12 @@ impl<'a> ConfigLoader<'a> {
435435
));
436436
continue;
437437
}
438+
if options.respect_eslint_disable_directives.is_some() {
439+
errors.push(ConfigLoadError::Diagnostic(
440+
nested_respect_eslint_disable_directives_not_supported(&path),
441+
));
442+
continue;
443+
}
438444
}
439445

440446
let builder = match ConfigStoreBuilder::from_oxlintrc(
@@ -728,6 +734,14 @@ fn nested_report_unused_disable_directives_not_supported(path: &Path) -> OxcDiag
728734
.with_help("Move `options.reportUnusedDisableDirectives` to the root configuration file.")
729735
}
730736

737+
fn nested_respect_eslint_disable_directives_not_supported(path: &Path) -> OxcDiagnostic {
738+
OxcDiagnostic::error(format!(
739+
"The `options.respectEslintDisableDirectives` option is only supported in the root config, but it was found in {}.",
740+
path.display()
741+
))
742+
.with_help("Move `options.respectEslintDisableDirectives` to the root configuration file.")
743+
}
744+
731745
#[cfg(test)]
732746
mod test {
733747
use std::path::PathBuf;
@@ -903,6 +917,27 @@ mod test {
903917
assert!(matches!(errors[0], ConfigLoadError::Diagnostic(_)));
904918
}
905919

920+
#[test]
921+
fn test_nested_json_config_rejects_respect_eslint_disable_directives() {
922+
let root_dir = tempfile::tempdir().unwrap();
923+
let nested_path = root_dir.path().join("nested/.oxlintrc.json");
924+
std::fs::create_dir_all(nested_path.parent().unwrap()).unwrap();
925+
std::fs::write(
926+
&nested_path,
927+
r#"{ "options": { "respectEslintDisableDirectives": false } }"#,
928+
)
929+
.unwrap();
930+
931+
let mut external_plugin_store = ExternalPluginStore::new(false);
932+
let mut loader = ConfigLoader::new(None, &mut external_plugin_store, &[], None);
933+
let (_configs, errors) = loader.load_discovered_with_root_dir(
934+
root_dir.path(),
935+
[DiscoveredConfigFile::Json(nested_path)],
936+
);
937+
assert_eq!(errors.len(), 1);
938+
assert!(matches!(errors[0], ConfigLoadError::Diagnostic(_)));
939+
}
940+
906941
#[test]
907942
fn test_nested_json_config_allows_type_aware_from_extends() {
908943
let root_dir = tempfile::tempdir().unwrap();

‎apps/oxlint/src/lint.rs‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1209,6 +1209,15 @@ mod test {
12091209
.test_and_snapshot(args);
12101210
}
12111211

1212+
#[test]
1213+
fn test_report_unused_directives_with_oxlint_prefix_only() {
1214+
let args = &["-c", ".oxlintrc.json", "--report-unused-disable-directives"];
1215+
1216+
Tester::new()
1217+
.with_cwd("fixtures/cli/report_unused_directives_oxlint_only".into())
1218+
.test_and_snapshot(args);
1219+
}
1220+
12121221
#[test]
12131222
fn test_nested_config() {
12141223
let args = &[];

‎apps/oxlint/src/lsp/error_with_position.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -305,7 +305,7 @@ pub fn create_unused_directives_report(
305305

306306
// Report unused enable comments
307307
let unused_enable = directives.unused_enable_comments();
308-
for (rule_name, span) in unused_enable {
308+
for (_directive_prefix, rule_name, span) in unused_enable {
309309
let message = if let Some(rule_name) = rule_name {
310310
format!(
311311
"Unused oxlint-enable directive (no matching oxlint-disable directives were found for {rule_name})."

‎apps/oxlint/src/snapshots/fixtures__cli__report_unused_directives_oxlint_only_-c .oxlintrc.json [email protected]‎

Lines changed: 114 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,114 @@
1+
---
2+
source: apps/oxlint/src/tester.rs
3+
---
4+
##########
5+
arguments: -c .oxlintrc.json --report-unused-disable-directives
6+
working directory: fixtures/cli/report_unused_directives_oxlint_only
7+
----------
8+
9+
! eslint(no-console): Unexpected console statement.
10+
,-[test.js:2:1]
11+
1 | // eslint-disable-next-line no-console
12+
2 | console.log("eslint prefix is ignored");
13+
: ^^^^^^^^^^^
14+
3 |
15+
`----
16+
help: Delete this console statement.
17+
18+
! Unused eslint-disable directive (no problems were reported).
19+
,-[test.js:7:1]
20+
6 |
21+
7 | // oxlint-disable-next-line no-debugger
22+
: ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
23+
8 | console.log("oxlint unused");
24+
`----
25+
26+
! eslint(no-console): Unexpected console statement.
27+
,-[test.js:8:1]
28+
7 | // oxlint-disable-next-line no-debugger
29+
8 | console.log("oxlint unused");
30+
: ^^^^^^^^^^^
31+
9 |
32+
`----
33+
help: Delete this console statement.
34+
35+
! Unused eslint-disable directive (no problems were reported).
36+
,-[test.js:10:1]
37+
9 |
38+
10 | // oxlint-disable-next-line no-console
39+
: ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
40+
11 | debugger;
41+
`----
42+
43+
! eslint(no-debugger): `debugger` statement is not allowed
44+
,-[test.js:11:1]
45+
10 | // oxlint-disable-next-line no-console
46+
11 | debugger;
47+
: ^^^^^^^^^
48+
12 |
49+
`----
50+
help: Remove the debugger statement
51+
52+
! Unused eslint-enable directive (no matching eslint-disable directives were found).
53+
,-[test.js:14:3]
54+
13 | // eslint-enable
55+
14 | // oxlint-enable
56+
: ^^^^^^^^^^^^^^
57+
`----
58+
59+
! eslint(no-console): Unexpected console statement.
60+
,-[test.vue:4:1]
61+
3 | // eslint-disable-next-line no-console
62+
4 | console.log("eslint prefix is ignored");
63+
: ^^^^^^^^^^^
64+
5 |
65+
`----
66+
help: Delete this console statement.
67+
68+
! Unused eslint-disable directive (no problems were reported).
69+
,-[test.vue:9:1]
70+
8 |
71+
9 | // oxlint-disable-next-line no-debugger
72+
: ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
73+
10 | console.log("oxlint unused");
74+
`----
75+
76+
! eslint(no-console): Unexpected console statement.
77+
,-[test.vue:10:1]
78+
9 | // oxlint-disable-next-line no-debugger
79+
10 | console.log("oxlint unused");
80+
: ^^^^^^^^^^^
81+
11 |
82+
`----
83+
help: Delete this console statement.
84+
85+
! Unused eslint-disable directive (no problems were reported).
86+
,-[test.vue:12:1]
87+
11 |
88+
12 | // oxlint-disable-next-line no-console
89+
: ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
90+
13 | debugger;
91+
`----
92+
93+
! eslint(no-debugger): `debugger` statement is not allowed
94+
,-[test.vue:13:1]
95+
12 | // oxlint-disable-next-line no-console
96+
13 | debugger;
97+
: ^^^^^^^^^
98+
14 |
99+
`----
100+
help: Remove the debugger statement
101+
102+
! Unused eslint-enable directive (no matching eslint-disable directives were found).
103+
,-[test.vue:16:3]
104+
15 | // eslint-enable
105+
16 | // oxlint-enable
106+
: ^^^^^^^^^^^^^^
107+
17 |
108+
`----
109+
110+
Found 12 warnings and 0 errors.
111+
Finished in <variable>ms on 2 files with 2 rules using 1 threads.
112+
----------
113+
CLI result: LintSucceeded
114+
----------

‎crates/oxc_linter/fixtures/extends_config/options/respect_eslint_disable_directives_false.json‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
{
2+
"options": {
3+
"respectEslintDisableDirectives": false
4+
}
5+
}

‎crates/oxc_linter/src/config/config_builder.rs‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1454,6 +1454,15 @@ mod test {
14541454
Some(AllowWarnDeny::Deny)
14551455
);
14561456

1457+
let config =
1458+
config_store_from_str(r#"{ "options": {"respectEslintDisableDirectives": false } }"#);
1459+
assert_eq!(config.base.config.options.respect_eslint_disable_directives, Some(false));
1460+
1461+
let config = config_store_from_str(
1462+
r#"{ "extends": ["fixtures/extends_config/options/respect_eslint_disable_directives_false.json"] }"#,
1463+
);
1464+
assert_eq!(config.base.config.options.respect_eslint_disable_directives, Some(false));
1465+
14571466
let config = config_store_from_str(
14581467
r#"{ "extends": ["fixtures/extends_config/options/deny_warnings_true.json"] }"#,
14591468
);

0 commit comments

Comments
 (0)