From 6e8d2f63180664552a522a2397698563d75b60c4 Mon Sep 17 00:00:00 2001 From: overlookmotel <557937+overlookmotel@users.noreply.github.com> Date: Tue, 7 Oct 2025 10:19:21 +0000 Subject: [PATCH] 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` 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. --- .../fixtures/linter/js_plugins/.oxlintrc.json | 7 ++ .../fixtures/linter/js_plugins/index.js | 3 + .../fixtures/linter/js_plugins/plugin.js | 21 ++++++ .../src/linter/server_linter.rs | 13 +++- .../fixtures_linter_js_plugins@index.js.snap | 69 +++++++++++++++++++ .../oxc_linter/src/config/config_builder.rs | 7 +- crates/oxc_linter/src/config/overrides.rs | 3 + crates/oxc_linter/src/config/oxlintrc.rs | 3 + crates/oxc_linter/src/config/rules.rs | 21 ++++-- .../oxc_linter/src/external_plugin_store.rs | 26 ++++++- .../oxc_linter/src/snapshots/schema_json.snap | 4 +- npm/oxlint/configuration_schema.json | 4 +- .../src/linter/snapshots/schema_markdown.snap | 6 ++ 13 files changed, 172 insertions(+), 15 deletions(-) create mode 100644 crates/oxc_language_server/fixtures/linter/js_plugins/.oxlintrc.json create mode 100644 crates/oxc_language_server/fixtures/linter/js_plugins/index.js create mode 100644 crates/oxc_language_server/fixtures/linter/js_plugins/plugin.js create mode 100644 crates/oxc_language_server/src/snapshots/fixtures_linter_js_plugins@index.js.snap diff --git a/crates/oxc_language_server/fixtures/linter/js_plugins/.oxlintrc.json b/crates/oxc_language_server/fixtures/linter/js_plugins/.oxlintrc.json new file mode 100644 index 0000000000000..96a5c3cc2e7c9 --- /dev/null +++ b/crates/oxc_language_server/fixtures/linter/js_plugins/.oxlintrc.json @@ -0,0 +1,7 @@ +{ + "jsPlugins": ["./plugin.js"], + "rules": { + "js-plugin/no-debugger": "error", + "no-unused-vars": "off" + } +} diff --git a/crates/oxc_language_server/fixtures/linter/js_plugins/index.js b/crates/oxc_language_server/fixtures/linter/js_plugins/index.js new file mode 100644 index 0000000000000..e4a0f949219a9 --- /dev/null +++ b/crates/oxc_language_server/fixtures/linter/js_plugins/index.js @@ -0,0 +1,3 @@ +debugger; + +let x; diff --git a/crates/oxc_language_server/fixtures/linter/js_plugins/plugin.js b/crates/oxc_language_server/fixtures/linter/js_plugins/plugin.js new file mode 100644 index 0000000000000..7cb9c046dbed4 --- /dev/null +++ b/crates/oxc_language_server/fixtures/linter/js_plugins/plugin.js @@ -0,0 +1,21 @@ +const plugin = { + meta: { + name: 'js-plugin', + }, + rules: { + 'no-debugger': { + create(context) { + return { + DebuggerStatement(debuggerStatement) { + context.report({ + message: 'Unexpected Debugger Statement', + node: debuggerStatement, + }); + }, + }; + }, + }, + }, +}; + +export default plugin; diff --git a/crates/oxc_language_server/src/linter/server_linter.rs b/crates/oxc_language_server/src/linter/server_linter.rs index df695d3637c12..902b7387b230f 100644 --- a/crates/oxc_language_server/src/linter/server_linter.rs +++ b/crates/oxc_language_server/src/linter/server_linter.rs @@ -109,7 +109,7 @@ impl ServerLinter { let base_patterns = oxlintrc.ignore_patterns.clone(); - let mut external_plugin_store = ExternalPluginStore::default(); + let mut external_plugin_store = ExternalPluginStore::new(false); let config_builder = ConfigStoreBuilder::from_oxlintrc(false, oxlintrc, None, &mut external_plugin_store) .unwrap_or_default(); @@ -211,7 +211,7 @@ impl ServerLinter { }; // Collect ignore patterns and their root nested_ignore_patterns.push((oxlintrc.ignore_patterns.clone(), dir_path.to_path_buf())); - let mut external_plugin_store = ExternalPluginStore::default(); + let mut external_plugin_store = ExternalPluginStore::new(false); let Ok(config_store_builder) = ConfigStoreBuilder::from_oxlintrc( false, oxlintrc, @@ -669,4 +669,13 @@ mod test { ); tester.test_and_snapshot_single_file("no-floating-promises/index.ts"); } + + #[test] + fn test_ignore_js_plugins() { + let tester = Tester::new( + "fixtures/linter/js_plugins", + Some(LintOptions { run: Run::OnSave, ..Default::default() }), + ); + tester.test_and_snapshot_single_file("index.js"); + } } diff --git a/crates/oxc_language_server/src/snapshots/fixtures_linter_js_plugins@index.js.snap b/crates/oxc_language_server/src/snapshots/fixtures_linter_js_plugins@index.js.snap new file mode 100644 index 0000000000000..1b0b7188a6974 --- /dev/null +++ b/crates/oxc_language_server/src/snapshots/fixtures_linter_js_plugins@index.js.snap @@ -0,0 +1,69 @@ +--- +source: crates/oxc_language_server/src/tester.rs +--- +########## +file: fixtures/linter/js_plugins/index.js +---------- + +code: "eslint(no-debugger)" +code_description.href: "https://oxc.rs/docs/guide/usage/linter/rules/eslint/no-debugger.html" +message: "`debugger` statement is not allowed\nhelp: Remove the debugger statement" +range: Range { start: Position { line: 0, character: 0 }, end: Position { line: 0, character: 9 } } +related_information[0].message: "" +related_information[0].location.uri: "file:///fixtures/linter/js_plugins/index.js" +related_information[0].location.range: Range { start: Position { line: 0, character: 0 }, end: Position { line: 0, character: 9 } } +severity: Some(Warning) +source: Some("oxc") +tags: None +fixed: Multiple( + [ + FixedContent { + message: Some( + "Remove the debugger statement", + ), + code: "", + range: Range { + start: Position { + line: 0, + character: 0, + }, + end: Position { + line: 0, + character: 9, + }, + }, + }, + FixedContent { + message: Some( + "Disable no-debugger for this line", + ), + code: "// oxlint-disable-next-line no-debugger\n", + range: Range { + start: Position { + line: 0, + character: 0, + }, + end: Position { + line: 0, + character: 0, + }, + }, + }, + FixedContent { + message: Some( + "Disable no-debugger for this file", + ), + code: "// oxlint-disable no-debugger\n", + range: Range { + start: Position { + line: 0, + character: 0, + }, + end: Position { + line: 0, + character: 0, + }, + }, + }, + ], +) diff --git a/crates/oxc_linter/src/config/config_builder.rs b/crates/oxc_linter/src/config/config_builder.rs index 044c0738d5063..6d81043e89de1 100644 --- a/crates/oxc_linter/src/config/config_builder.rs +++ b/crates/oxc_linter/src/config/config_builder.rs @@ -159,7 +159,10 @@ impl ConfigStoreBuilder { } } - if !external_plugins.is_empty() { + // If external plugins are not enabled (language server), then skip loading JS plugins. + // This is so that a project can use JS plugins via `oxlint` CLI, and language server + // will just silently ignore them - rather than crashing. + if !external_plugins.is_empty() && external_plugin_store.is_enabled() { let Some(external_linter) = external_linter else { #[expect(clippy::missing_panics_doc, reason = "infallible")] let plugin_specifier = external_plugins.iter().next().unwrap().clone(); @@ -613,7 +616,7 @@ impl Display for ConfigBuilderError { ConfigBuilderError::NoExternalLinterConfigured { plugin_specifier } => { write!( f, - "`jsPlugins` config contains '{plugin_specifier}'. JS plugins are not supported in the language server, or on 32-bit or big-endian platforms at present.", + "`jsPlugins` config contains '{plugin_specifier}'. JS plugins are not supported on 32-bit or big-endian platforms at present.", )?; Ok(()) } diff --git a/crates/oxc_linter/src/config/overrides.rs b/crates/oxc_linter/src/config/overrides.rs index f91b26d206eb7..9873c7eab3a52 100644 --- a/crates/oxc_linter/src/config/overrides.rs +++ b/crates/oxc_linter/src/config/overrides.rs @@ -94,6 +94,9 @@ pub struct OxlintOverride { pub plugins: Option, /// JS plugins for this override. + /// + /// Note: JS plugins are experimental and not subject to semver. + /// They are not supported in language server at present. #[serde(rename = "jsPlugins")] pub external_plugins: Option>, diff --git a/crates/oxc_linter/src/config/oxlintrc.rs b/crates/oxc_linter/src/config/oxlintrc.rs index a33d5ee784ce4..d5e3bd3170d85 100644 --- a/crates/oxc_linter/src/config/oxlintrc.rs +++ b/crates/oxc_linter/src/config/oxlintrc.rs @@ -65,6 +65,9 @@ use super::{ pub struct Oxlintrc { pub plugins: Option, /// JS plugins. + /// + /// Note: JS plugins are experimental and not subject to semver. + /// They are not supported in language server at present. #[serde(rename = "jsPlugins")] pub external_plugins: Option>, pub categories: OxlintCategories, diff --git a/crates/oxc_linter/src/config/rules.rs b/crates/oxc_linter/src/config/rules.rs index a909ed73704dd..3cf38589e0688 100644 --- a/crates/oxc_linter/src/config/rules.rs +++ b/crates/oxc_linter/src/config/rules.rs @@ -97,12 +97,21 @@ impl OxlintRules { rules_to_replace.push((rule.read_json(config), severity)); } } else { - let external_rule_id = - external_plugin_store.lookup_rule_id(plugin_name, rule_name)?; - external_rules_for_override - .entry(external_rule_id) - .and_modify(|sev| *sev = severity) - .or_insert(severity); + // If JS plugins are disabled (language server), assume plugin name refers to a JS plugin, + // and that rule name is valid for that plugin. + // But language server doesn't support JS plugins, so ignore the rule. + // + // This unfortunately means we can't catch genuinely invalid plugin names in language server + // (e.g. typos like `unicon/filename-case`). But we can't avoid this as the name of a JS plugin + // can only be known by loading it, which language server can't do at present. + if external_plugin_store.is_enabled() { + let external_rule_id = + external_plugin_store.lookup_rule_id(plugin_name, rule_name)?; + external_rules_for_override + .entry(external_rule_id) + .and_modify(|sev| *sev = severity) + .or_insert(severity); + } } } } diff --git a/crates/oxc_linter/src/external_plugin_store.rs b/crates/oxc_linter/src/external_plugin_store.rs index 9b95decfef7da..266d080644c2f 100644 --- a/crates/oxc_linter/src/external_plugin_store.rs +++ b/crates/oxc_linter/src/external_plugin_store.rs @@ -12,16 +12,40 @@ define_index_type! { pub struct ExternalRuleId = u32; } -#[derive(Debug, Default)] +#[derive(Debug)] pub struct ExternalPluginStore { registered_plugin_paths: FxHashSet, plugins: IndexVec, plugin_names: FxHashMap, rules: IndexVec, + + // `true` for `oxlint`, `false` for language server + is_enabled: bool, +} + +impl Default for ExternalPluginStore { + fn default() -> Self { + Self::new(true) + } } impl ExternalPluginStore { + pub fn new(is_enabled: bool) -> Self { + Self { + registered_plugin_paths: FxHashSet::default(), + plugins: IndexVec::default(), + plugin_names: FxHashMap::default(), + rules: IndexVec::default(), + is_enabled, + } + } + + /// Returns `true` if external plugins are enabled. + pub fn is_enabled(&self) -> bool { + self.is_enabled + } + /// Returns `true` if no external plugins have been loaded. pub fn is_empty(&self) -> bool { self.plugins.is_empty() diff --git a/crates/oxc_linter/src/snapshots/schema_json.snap b/crates/oxc_linter/src/snapshots/schema_json.snap index 511ec873a7e59..dcd7408bae586 100644 --- a/crates/oxc_linter/src/snapshots/schema_json.snap +++ b/crates/oxc_linter/src/snapshots/schema_json.snap @@ -52,7 +52,7 @@ expression: json } }, "jsPlugins": { - "description": "JS plugins.", + "description": "JS plugins.\n\nNote: JS plugins are experimental and not subject to semver.\nThey are not supported in language server at present.", "default": null, "type": [ "array", @@ -444,7 +444,7 @@ expression: json ] }, "jsPlugins": { - "description": "JS plugins for this override.", + "description": "JS plugins for this override.\n\nNote: JS plugins are experimental and not subject to semver.\nThey are not supported in language server at present.", "type": [ "array", "null" diff --git a/npm/oxlint/configuration_schema.json b/npm/oxlint/configuration_schema.json index a848ee2e491c2..a564c89e07154 100644 --- a/npm/oxlint/configuration_schema.json +++ b/npm/oxlint/configuration_schema.json @@ -48,7 +48,7 @@ } }, "jsPlugins": { - "description": "JS plugins.", + "description": "JS plugins.\n\nNote: JS plugins are experimental and not subject to semver.\nThey are not supported in language server at present.", "default": null, "type": [ "array", @@ -440,7 +440,7 @@ ] }, "jsPlugins": { - "description": "JS plugins for this override.", + "description": "JS plugins for this override.\n\nNote: JS plugins are experimental and not subject to semver.\nThey are not supported in language server at present.", "type": [ "array", "null" diff --git a/tasks/website/src/linter/snapshots/schema_markdown.snap b/tasks/website/src/linter/snapshots/schema_markdown.snap index 40c0e37f523c7..2f73534efd056 100644 --- a/tasks/website/src/linter/snapshots/schema_markdown.snap +++ b/tasks/website/src/linter/snapshots/schema_markdown.snap @@ -201,6 +201,9 @@ default: `null` JS plugins. +Note: JS plugins are experimental and not subject to semver. +They are not supported in language server at present. + ## overrides @@ -233,6 +236,9 @@ type: `string[]` JS plugins for this override. +Note: JS plugins are experimental and not subject to semver. +They are not supported in language server at present. + #### overrides[n].rules