Skip to content

Commit 34782ff

Browse files
lilnasycamc314
andcommitted
fix(oxlintrc): resolve relative plugin specifiers before merging extended configs (#14562)
- Fixes #14478 ## What's going wrong - Relative specifiers need to joined with their config's directory path. - When extended configs are merged. we drop the "other" config's path. - In this "extends" case, the plugins from the "other" config are resolved relative to the "self" config's path. That shouldn't happen. ## Fix - All relative specifiers are resolved _before_ the merge. - Since full platform-specific (correct) paths to the plugin are logged when loading fails, this won't work well with the snapshot tests. <img width="898" height="249" alt="image" src="https://github.com/user-attachments/assets/7e4d0fa1-c0c1-4dc5-8135-dbf663900902" /> I am vacillating on this solution. Maybe external_plugin should be an enum. --------- Co-authored-by: Cameron Clark <cameron.clark@hey.com>
1 parent 7412c17 commit 34782ff

File tree

8 files changed

+142
-24
lines changed

8 files changed

+142
-24
lines changed

apps/oxlint/src/snapshots/_-c fixtures__print_config__ban_rules__eslintrc.json -A all -D eqeqeq --print-config@oxlint.snap

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@ working directory:
1111
"typescript",
1212
"oxc"
1313
],
14-
"jsPlugins": null,
1514
"categories": {},
1615
"rules": {
1716
"eqeqeq": [

apps/oxlint/src/snapshots/fixtures_-A all --print-config@oxlint.snap

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@ working directory: fixtures
1111
"typescript",
1212
"oxc"
1313
],
14-
"jsPlugins": null,
1514
"categories": {},
1615
"rules": {},
1716
"settings": {
Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
# Exit code
2+
1
3+
4+
# stdout
5+
```
6+
x basic-custom-plugin(no-debugger): Unexpected Debugger Statement
7+
,-[nested/index.ts:1:1]
8+
1 | debugger;
9+
: ^^^^^^^^^
10+
`----
11+
12+
Found 0 warnings and 1 error.
13+
Finished in Xms on 2 files using X threads.
14+
```
15+
16+
# stderr
17+
```
18+
WARNING: JS plugins are experimental and not subject to semver.
19+
Breaking changes are possible while JS plugins support is under development.
20+
```

crates/oxc_linter/src/config/config_builder.rs

Lines changed: 18 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -147,15 +147,18 @@ impl ConfigStoreBuilder {
147147
let (oxlintrc, extended_paths) = resolve_oxlintrc_config(oxlintrc)?;
148148

149149
// Collect external plugins from both base config and overrides
150-
let mut external_plugins = FxHashSet::default();
150+
let mut external_plugins: FxHashSet<(Option<&Path>, &str)> = FxHashSet::default();
151151

152152
if let Some(base_external_plugins) = &oxlintrc.external_plugins {
153-
external_plugins.extend(base_external_plugins.iter().cloned());
153+
external_plugins
154+
.extend(base_external_plugins.iter().map(|(k, v)| (k.as_deref(), v.as_str())));
154155
}
155156

156157
for r#override in &oxlintrc.overrides {
157158
if let Some(override_external_plugins) = &r#override.external_plugins {
158-
external_plugins.extend(override_external_plugins.iter().cloned());
159+
external_plugins.extend(
160+
override_external_plugins.iter().map(|(k, v)| (k.as_deref(), v.as_str())),
161+
);
159162
}
160163
}
161164

@@ -165,8 +168,10 @@ impl ConfigStoreBuilder {
165168
if !external_plugins.is_empty() && external_plugin_store.is_enabled() {
166169
let Some(external_linter) = external_linter else {
167170
#[expect(clippy::missing_panics_doc, reason = "infallible")]
168-
let plugin_specifier = external_plugins.iter().next().unwrap().clone();
169-
return Err(ConfigBuilderError::NoExternalLinterConfigured { plugin_specifier });
171+
let (_, original_specifier) = external_plugins.iter().next().unwrap();
172+
return Err(ConfigBuilderError::NoExternalLinterConfigured {
173+
plugin_specifier: (*original_specifier).to_string(),
174+
});
170175
};
171176

172177
let resolver = Resolver::new(ResolveOptions {
@@ -175,19 +180,17 @@ impl ConfigStoreBuilder {
175180
..Default::default()
176181
});
177182

178-
#[expect(clippy::missing_panics_doc, reason = "oxlintrc.path is always a file path")]
179-
let oxlintrc_dir = oxlintrc.path.parent().unwrap();
180-
181-
for plugin_specifier in &external_plugins {
183+
for (config_path, specifier) in &external_plugins {
182184
Self::load_external_plugin(
183-
oxlintrc_dir,
184-
plugin_specifier,
185+
*config_path,
186+
specifier,
185187
external_linter,
186188
&resolver,
187189
external_plugin_store,
188190
)?;
189191
}
190192
}
193+
191194
let plugins = oxlintrc.plugins.unwrap_or_default();
192195

193196
let rules =
@@ -505,7 +508,7 @@ impl ConfigStoreBuilder {
505508
}
506509

507510
fn load_external_plugin(
508-
oxlintrc_dir_path: &Path,
511+
config_dir: Option<&Path>,
509512
plugin_specifier: &str,
510513
external_linter: &ExternalLinter,
511514
resolver: &Resolver,
@@ -521,7 +524,9 @@ impl ConfigStoreBuilder {
521524
);
522525
}
523526

524-
let resolved = resolver.resolve(oxlintrc_dir_path, plugin_specifier).map_err(|e| {
527+
// Resolve the specifier relative to the config directory
528+
let resolve_dir = config_dir.unwrap_or_else(|| Path::new("."));
529+
let resolved = resolver.resolve(resolve_dir, plugin_specifier).map_err(|e| {
525530
ConfigBuilderError::PluginLoadFailed {
526531
plugin_specifier: plugin_specifier.to_string(),
527532
error: e.to_string(),

crates/oxc_linter/src/config/overrides.rs

Lines changed: 45 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ use std::{
55

66
use rustc_hash::FxHashSet;
77
use schemars::{JsonSchema, r#gen, schema::Schema};
8-
use serde::{Deserialize, Deserializer, Serialize};
8+
use serde::{Deserialize, Deserializer, Serialize, Serializer};
99

1010
use crate::{LintPlugins, OxlintEnv, OxlintGlobals, config::OxlintRules};
1111

@@ -97,8 +97,20 @@ pub struct OxlintOverride {
9797
///
9898
/// Note: JS plugins are experimental and not subject to semver.
9999
/// They are not supported in language server at present.
100-
#[serde(rename = "jsPlugins")]
101-
pub external_plugins: Option<FxHashSet<String>>,
100+
#[serde(
101+
rename = "jsPlugins",
102+
deserialize_with = "deserialize_external_plugins_override",
103+
serialize_with = "serialize_external_plugins_override",
104+
default,
105+
skip_serializing_if = "Option::is_none"
106+
)]
107+
#[schemars(with = "Option<FxHashSet<String>>")]
108+
pub external_plugins: Option<
109+
FxHashSet<(
110+
Option<std::path::PathBuf>, /* config file directory */
111+
String, /* plugin specifier */
112+
)>,
113+
>,
102114

103115
#[serde(default)]
104116
pub rules: OxlintRules,
@@ -139,6 +151,36 @@ impl GlobSet {
139151
}
140152
}
141153

154+
#[expect(clippy::type_complexity)]
155+
fn deserialize_external_plugins_override<'de, D>(
156+
deserializer: D,
157+
) -> Result<Option<FxHashSet<(Option<std::path::PathBuf>, String)>>, D::Error>
158+
where
159+
D: Deserializer<'de>,
160+
{
161+
let opt_set: Option<FxHashSet<String>> = Option::deserialize(deserializer)?;
162+
Ok(opt_set.map(|set| set.into_iter().map(|specifier| (None, specifier)).collect()))
163+
}
164+
165+
#[expect(clippy::ptr_arg)]
166+
fn serialize_external_plugins_override<S>(
167+
plugins: &Option<FxHashSet<(Option<std::path::PathBuf>, String)>>,
168+
serializer: S,
169+
) -> Result<S::Ok, S::Error>
170+
where
171+
S: Serializer,
172+
{
173+
// Serialize as an array of original specifiers (the values in the map)
174+
match plugins {
175+
Some(set) => {
176+
let values: FxHashSet<String> =
177+
set.iter().map(|(_, specifier)| specifier).cloned().collect();
178+
values.serialize(serializer)
179+
}
180+
None => serializer.serialize_none(),
181+
}
182+
}
183+
142184
#[cfg(test)]
143185
mod test {
144186
use crate::config::{globals::GlobalValue, plugins::LintPlugins};

crates/oxc_linter/src/config/oxlintrc.rs

Lines changed: 59 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ use std::{
55

66
use rustc_hash::{FxHashMap, FxHashSet};
77
use schemars::JsonSchema;
8-
use serde::{Deserialize, Serialize};
8+
use serde::{Deserialize, Deserializer, Serialize, Serializer};
99

1010
use oxc_diagnostics::OxcDiagnostic;
1111

@@ -68,8 +68,15 @@ pub struct Oxlintrc {
6868
///
6969
/// Note: JS plugins are experimental and not subject to semver.
7070
/// They are not supported in language server at present.
71-
#[serde(rename = "jsPlugins")]
72-
pub external_plugins: Option<FxHashSet<String>>,
71+
#[serde(
72+
rename = "jsPlugins",
73+
deserialize_with = "deserialize_external_plugins",
74+
serialize_with = "serialize_external_plugins",
75+
default,
76+
skip_serializing_if = "Option::is_none"
77+
)]
78+
#[schemars(with = "Option<FxHashSet<String>>")]
79+
pub external_plugins: Option<FxHashSet<(Option<PathBuf>, String)>>,
7380
pub categories: OxlintCategories,
7481
/// Example
7582
///
@@ -152,6 +159,23 @@ impl Oxlintrc {
152159

153160
config.path = path.to_path_buf();
154161

162+
if let Some(config_dir) = config.path.parent() {
163+
if let Some(external_plugins) = &mut config.external_plugins {
164+
*external_plugins = std::mem::take(external_plugins)
165+
.into_iter()
166+
.map(|(_, specifier)| (Some(config_dir.to_path_buf()), specifier))
167+
.collect();
168+
}
169+
170+
for override_config in config.overrides.iter_mut() {
171+
if let Some(external_plugins) = &mut override_config.external_plugins {
172+
*external_plugins = std::mem::take(external_plugins)
173+
.into_iter()
174+
.map(|(_, specifier)| (Some(config_dir.to_path_buf()), specifier))
175+
.collect();
176+
}
177+
}
178+
}
155179
Ok(config)
156180
}
157181

@@ -205,7 +229,9 @@ impl Oxlintrc {
205229

206230
let external_plugins = match (&self.external_plugins, &other.external_plugins) {
207231
(Some(self_external), Some(other_external)) => {
208-
Some(self_external.iter().chain(other_external).cloned().collect())
232+
let mut merged = other_external.clone();
233+
merged.extend(self_external.clone());
234+
Some(merged)
209235
}
210236
(Some(self_external), None) => Some(self_external.clone()),
211237
(None, Some(other_external)) => Some(other_external.clone()),
@@ -232,6 +258,35 @@ fn is_json_ext(ext: &str) -> bool {
232258
ext == "json" || ext == "jsonc"
233259
}
234260

261+
#[expect(clippy::type_complexity)]
262+
fn deserialize_external_plugins<'de, D>(
263+
deserializer: D,
264+
) -> Result<Option<FxHashSet<(Option<PathBuf>, String)>>, D::Error>
265+
where
266+
D: Deserializer<'de>,
267+
{
268+
let opt_set: Option<FxHashSet<String>> = Option::deserialize(deserializer)?;
269+
Ok(opt_set.map(|set| set.into_iter().map(|specifier| (None, specifier)).collect()))
270+
}
271+
272+
#[expect(clippy::ptr_arg)]
273+
fn serialize_external_plugins<S>(
274+
plugins: &Option<FxHashSet<(Option<PathBuf>, String)>>,
275+
serializer: S,
276+
) -> Result<S::Ok, S::Error>
277+
where
278+
S: Serializer,
279+
{
280+
// Serialize as an array of original specifiers (the values in the map)
281+
match plugins {
282+
Some(set) => {
283+
let values: FxHashSet<String> = set.iter().map(|(_, k)| k).cloned().collect();
284+
values.serialize(serializer)
285+
}
286+
None => serializer.serialize_none(),
287+
}
288+
}
289+
235290
#[cfg(test)]
236291
mod test {
237292
use serde_json::json;

crates/oxc_linter/src/snapshots/schema_json.snap

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,6 @@ expression: json
5353
},
5454
"jsPlugins": {
5555
"description": "JS plugins.\n\nNote: JS plugins are experimental and not subject to semver.\nThey are not supported in language server at present.",
56-
"default": null,
5756
"type": [
5857
"array",
5958
"null"

npm/oxlint/configuration_schema.json

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,6 @@
4949
},
5050
"jsPlugins": {
5151
"description": "JS plugins.\n\nNote: JS plugins are experimental and not subject to semver.\nThey are not supported in language server at present.",
52-
"default": null,
5352
"type": [
5453
"array",
5554
"null"

0 commit comments

Comments
 (0)