Skip to content

Commit 6e8d2f6

Browse files
committed
fix(language_server): ignore JS plugins (#14379)
Previously we produced an error when JS plugins are defined in config but no `ExternalLinter` is present. That's the desired behavior in `oxlint` CLI, because it happens when user is running Oxlint on an unsupported platform (32-bit, or big-endian). But it's *not* what we want to happen in language server. Language server does not yet support JS plugins, but user may want to use them in their project (via `oxlint` CLI), and language server should not fail with a "JS plugins are not supported" error on any file in such a project. This PR prevents that by adding an `is_enabled` field to `ExternalPluginStore`. By default, `is_enabled = true`, but language server sets it to `false`. When `false`, JS plugins are silently ignored, rather than producing an error. Note: A better solution would be to pass around `Option<ExternalPluginStore>` instead of `ExternalPluginStore` having an `is_enabled` flag. But JS plugin support will be added to the language server pretty soon, at which point we'll need to revert this change. `is_enabled` flag approach will produce less churn when we do that.
1 parent 791c72a commit 6e8d2f6

File tree

13 files changed

+172
-15
lines changed

13 files changed

+172
-15
lines changed
Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
{
2+
"jsPlugins": ["./plugin.js"],
3+
"rules": {
4+
"js-plugin/no-debugger": "error",
5+
"no-unused-vars": "off"
6+
}
7+
}
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
debugger;
2+
3+
let x;
Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
const plugin = {
2+
meta: {
3+
name: 'js-plugin',
4+
},
5+
rules: {
6+
'no-debugger': {
7+
create(context) {
8+
return {
9+
DebuggerStatement(debuggerStatement) {
10+
context.report({
11+
message: 'Unexpected Debugger Statement',
12+
node: debuggerStatement,
13+
});
14+
},
15+
};
16+
},
17+
},
18+
},
19+
};
20+
21+
export default plugin;

crates/oxc_language_server/src/linter/server_linter.rs

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -109,7 +109,7 @@ impl ServerLinter {
109109

110110
let base_patterns = oxlintrc.ignore_patterns.clone();
111111

112-
let mut external_plugin_store = ExternalPluginStore::default();
112+
let mut external_plugin_store = ExternalPluginStore::new(false);
113113
let config_builder =
114114
ConfigStoreBuilder::from_oxlintrc(false, oxlintrc, None, &mut external_plugin_store)
115115
.unwrap_or_default();
@@ -211,7 +211,7 @@ impl ServerLinter {
211211
};
212212
// Collect ignore patterns and their root
213213
nested_ignore_patterns.push((oxlintrc.ignore_patterns.clone(), dir_path.to_path_buf()));
214-
let mut external_plugin_store = ExternalPluginStore::default();
214+
let mut external_plugin_store = ExternalPluginStore::new(false);
215215
let Ok(config_store_builder) = ConfigStoreBuilder::from_oxlintrc(
216216
false,
217217
oxlintrc,
@@ -669,4 +669,13 @@ mod test {
669669
);
670670
tester.test_and_snapshot_single_file("no-floating-promises/index.ts");
671671
}
672+
673+
#[test]
674+
fn test_ignore_js_plugins() {
675+
let tester = Tester::new(
676+
"fixtures/linter/js_plugins",
677+
Some(LintOptions { run: Run::OnSave, ..Default::default() }),
678+
);
679+
tester.test_and_snapshot_single_file("index.js");
680+
}
672681
}
Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,69 @@
1+
---
2+
source: crates/oxc_language_server/src/tester.rs
3+
---
4+
##########
5+
file: fixtures/linter/js_plugins/index.js
6+
----------
7+
8+
code: "eslint(no-debugger)"
9+
code_description.href: "https://oxc.rs/docs/guide/usage/linter/rules/eslint/no-debugger.html"
10+
message: "`debugger` statement is not allowed\nhelp: Remove the debugger statement"
11+
range: Range { start: Position { line: 0, character: 0 }, end: Position { line: 0, character: 9 } }
12+
related_information[0].message: ""
13+
related_information[0].location.uri: "file://<variable>/fixtures/linter/js_plugins/index.js"
14+
related_information[0].location.range: Range { start: Position { line: 0, character: 0 }, end: Position { line: 0, character: 9 } }
15+
severity: Some(Warning)
16+
source: Some("oxc")
17+
tags: None
18+
fixed: Multiple(
19+
[
20+
FixedContent {
21+
message: Some(
22+
"Remove the debugger statement",
23+
),
24+
code: "",
25+
range: Range {
26+
start: Position {
27+
line: 0,
28+
character: 0,
29+
},
30+
end: Position {
31+
line: 0,
32+
character: 9,
33+
},
34+
},
35+
},
36+
FixedContent {
37+
message: Some(
38+
"Disable no-debugger for this line",
39+
),
40+
code: "// oxlint-disable-next-line no-debugger\n",
41+
range: Range {
42+
start: Position {
43+
line: 0,
44+
character: 0,
45+
},
46+
end: Position {
47+
line: 0,
48+
character: 0,
49+
},
50+
},
51+
},
52+
FixedContent {
53+
message: Some(
54+
"Disable no-debugger for this file",
55+
),
56+
code: "// oxlint-disable no-debugger\n",
57+
range: Range {
58+
start: Position {
59+
line: 0,
60+
character: 0,
61+
},
62+
end: Position {
63+
line: 0,
64+
character: 0,
65+
},
66+
},
67+
},
68+
],
69+
)

crates/oxc_linter/src/config/config_builder.rs

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -159,7 +159,10 @@ impl ConfigStoreBuilder {
159159
}
160160
}
161161

162-
if !external_plugins.is_empty() {
162+
// If external plugins are not enabled (language server), then skip loading JS plugins.
163+
// This is so that a project can use JS plugins via `oxlint` CLI, and language server
164+
// will just silently ignore them - rather than crashing.
165+
if !external_plugins.is_empty() && external_plugin_store.is_enabled() {
163166
let Some(external_linter) = external_linter else {
164167
#[expect(clippy::missing_panics_doc, reason = "infallible")]
165168
let plugin_specifier = external_plugins.iter().next().unwrap().clone();
@@ -613,7 +616,7 @@ impl Display for ConfigBuilderError {
613616
ConfigBuilderError::NoExternalLinterConfigured { plugin_specifier } => {
614617
write!(
615618
f,
616-
"`jsPlugins` config contains '{plugin_specifier}'. JS plugins are not supported in the language server, or on 32-bit or big-endian platforms at present.",
619+
"`jsPlugins` config contains '{plugin_specifier}'. JS plugins are not supported on 32-bit or big-endian platforms at present.",
617620
)?;
618621
Ok(())
619622
}

crates/oxc_linter/src/config/overrides.rs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,9 @@ pub struct OxlintOverride {
9494
pub plugins: Option<LintPlugins>,
9595

9696
/// JS plugins for this override.
97+
///
98+
/// Note: JS plugins are experimental and not subject to semver.
99+
/// They are not supported in language server at present.
97100
#[serde(rename = "jsPlugins")]
98101
pub external_plugins: Option<FxHashSet<String>>,
99102

crates/oxc_linter/src/config/oxlintrc.rs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,9 @@ use super::{
6565
pub struct Oxlintrc {
6666
pub plugins: Option<LintPlugins>,
6767
/// JS plugins.
68+
///
69+
/// Note: JS plugins are experimental and not subject to semver.
70+
/// They are not supported in language server at present.
6871
#[serde(rename = "jsPlugins")]
6972
pub external_plugins: Option<FxHashSet<String>>,
7073
pub categories: OxlintCategories,

crates/oxc_linter/src/config/rules.rs

Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -97,12 +97,21 @@ impl OxlintRules {
9797
rules_to_replace.push((rule.read_json(config), severity));
9898
}
9999
} else {
100-
let external_rule_id =
101-
external_plugin_store.lookup_rule_id(plugin_name, rule_name)?;
102-
external_rules_for_override
103-
.entry(external_rule_id)
104-
.and_modify(|sev| *sev = severity)
105-
.or_insert(severity);
100+
// If JS plugins are disabled (language server), assume plugin name refers to a JS plugin,
101+
// and that rule name is valid for that plugin.
102+
// But language server doesn't support JS plugins, so ignore the rule.
103+
//
104+
// This unfortunately means we can't catch genuinely invalid plugin names in language server
105+
// (e.g. typos like `unicon/filename-case`). But we can't avoid this as the name of a JS plugin
106+
// can only be known by loading it, which language server can't do at present.
107+
if external_plugin_store.is_enabled() {
108+
let external_rule_id =
109+
external_plugin_store.lookup_rule_id(plugin_name, rule_name)?;
110+
external_rules_for_override
111+
.entry(external_rule_id)
112+
.and_modify(|sev| *sev = severity)
113+
.or_insert(severity);
114+
}
106115
}
107116
}
108117
}

crates/oxc_linter/src/external_plugin_store.rs

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,16 +12,40 @@ define_index_type! {
1212
pub struct ExternalRuleId = u32;
1313
}
1414

15-
#[derive(Debug, Default)]
15+
#[derive(Debug)]
1616
pub struct ExternalPluginStore {
1717
registered_plugin_paths: FxHashSet<String>,
1818

1919
plugins: IndexVec<ExternalPluginId, ExternalPlugin>,
2020
plugin_names: FxHashMap<String, ExternalPluginId>,
2121
rules: IndexVec<ExternalRuleId, ExternalRule>,
22+
23+
// `true` for `oxlint`, `false` for language server
24+
is_enabled: bool,
25+
}
26+
27+
impl Default for ExternalPluginStore {
28+
fn default() -> Self {
29+
Self::new(true)
30+
}
2231
}
2332

2433
impl ExternalPluginStore {
34+
pub fn new(is_enabled: bool) -> Self {
35+
Self {
36+
registered_plugin_paths: FxHashSet::default(),
37+
plugins: IndexVec::default(),
38+
plugin_names: FxHashMap::default(),
39+
rules: IndexVec::default(),
40+
is_enabled,
41+
}
42+
}
43+
44+
/// Returns `true` if external plugins are enabled.
45+
pub fn is_enabled(&self) -> bool {
46+
self.is_enabled
47+
}
48+
2549
/// Returns `true` if no external plugins have been loaded.
2650
pub fn is_empty(&self) -> bool {
2751
self.plugins.is_empty()

0 commit comments

Comments
 (0)