Skip to content

Commit 6c30cc9

Browse files
camc314lilnasy
andcommitted
fix(oxlintrc): resolve relative plugin specifiers before merging extended configs (#14562)
- Fixes #14478 - 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. - 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: Arsh <emmagoldmanvevo+github@pm.me>
1 parent 7412c17 commit 6c30cc9

File tree

10 files changed

+134
-24
lines changed

10 files changed

+134
-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": {

apps/oxlint/test/e2e.test.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,11 @@ interface TestOptions {
1616
// Function to get extra data to include in the snapshot
1717
getExtraSnapshotData?: (dirPath: string) => Promise<{ [key: string]: string }>;
1818

19+
/**
20+
* Override the `files` directory within the fixture to lint.
21+
* This is useful when the fixture has a different structure, e.g. when testing nested configs.
22+
* Defaults to `files`.
23+
*/
1924
overrideFiles?: string;
2025
}
2126

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: 15 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -147,15 +147,16 @@ 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<(&PathBuf, &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.extend(base_external_plugins.iter().map(|(k, v)| (k, v.as_str())));
154154
}
155155

156156
for r#override in &oxlintrc.overrides {
157157
if let Some(override_external_plugins) = &r#override.external_plugins {
158-
external_plugins.extend(override_external_plugins.iter().cloned());
158+
external_plugins
159+
.extend(override_external_plugins.iter().map(|(k, v)| (k, v.as_str())));
159160
}
160161
}
161162

@@ -165,8 +166,10 @@ impl ConfigStoreBuilder {
165166
if !external_plugins.is_empty() && external_plugin_store.is_enabled() {
166167
let Some(external_linter) = external_linter else {
167168
#[expect(clippy::missing_panics_doc, reason = "infallible")]
168-
let plugin_specifier = external_plugins.iter().next().unwrap().clone();
169-
return Err(ConfigBuilderError::NoExternalLinterConfigured { plugin_specifier });
169+
let (_, original_specifier) = external_plugins.iter().next().unwrap();
170+
return Err(ConfigBuilderError::NoExternalLinterConfigured {
171+
plugin_specifier: (*original_specifier).to_string(),
172+
});
170173
};
171174

172175
let resolver = Resolver::new(ResolveOptions {
@@ -175,19 +178,17 @@ impl ConfigStoreBuilder {
175178
..Default::default()
176179
});
177180

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 {
181+
for (config_path, specifier) in &external_plugins {
182182
Self::load_external_plugin(
183-
oxlintrc_dir,
184-
plugin_specifier,
183+
config_path,
184+
specifier,
185185
external_linter,
186186
&resolver,
187187
external_plugin_store,
188188
)?;
189189
}
190190
}
191+
191192
let plugins = oxlintrc.plugins.unwrap_or_default();
192193

193194
let rules =
@@ -505,7 +506,7 @@ impl ConfigStoreBuilder {
505506
}
506507

507508
fn load_external_plugin(
508-
oxlintrc_dir_path: &Path,
509+
resolve_dir: &Path,
509510
plugin_specifier: &str,
510511
external_linter: &ExternalLinter,
511512
resolver: &Resolver,
@@ -521,7 +522,8 @@ impl ConfigStoreBuilder {
521522
);
522523
}
523524

524-
let resolved = resolver.resolve(oxlintrc_dir_path, plugin_specifier).map_err(|e| {
525+
// Resolve the specifier relative to the config directory
526+
let resolved = resolver.resolve(resolve_dir, plugin_specifier).map_err(|e| {
525527
ConfigBuilderError::PluginLoadFailed {
526528
plugin_specifier: plugin_specifier.to_string(),
527529
error: e.to_string(),

crates/oxc_linter/src/config/overrides.rs

Lines changed: 40 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,12 @@
11
use std::{
22
borrow::Cow,
33
ops::{Deref, DerefMut},
4+
path::PathBuf,
45
};
56

67
use rustc_hash::FxHashSet;
78
use schemars::{JsonSchema, r#gen, schema::Schema};
8-
use serde::{Deserialize, Deserializer, Serialize};
9+
use serde::{Deserialize, Deserializer, Serialize, Serializer};
910

1011
use crate::{LintPlugins, OxlintEnv, OxlintGlobals, config::OxlintRules};
1112

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

103113
#[serde(default)]
104114
pub rules: OxlintRules,
@@ -139,6 +149,33 @@ impl GlobSet {
139149
}
140150
}
141151

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

crates/oxc_linter/src/config/oxlintrc.rs

Lines changed: 54 additions & 3 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<(PathBuf, String)>>,
7380
pub categories: OxlintCategories,
7481
/// Example
7582
///
@@ -152,6 +159,24 @@ impl Oxlintrc {
152159

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

162+
#[expect(clippy::missing_panics_doc)]
163+
let config_dir = config.path.parent().unwrap();
164+
if let Some(external_plugins) = &mut config.external_plugins {
165+
*external_plugins = std::mem::take(external_plugins)
166+
.into_iter()
167+
.map(|(_, specifier)| (config_dir.to_path_buf(), specifier))
168+
.collect();
169+
}
170+
171+
for override_config in config.overrides.iter_mut() {
172+
if let Some(external_plugins) = &mut override_config.external_plugins {
173+
*external_plugins = std::mem::take(external_plugins)
174+
.into_iter()
175+
.map(|(_, specifier)| (config_dir.to_path_buf(), specifier))
176+
.collect();
177+
}
178+
}
179+
155180
Ok(config)
156181
}
157182

@@ -232,6 +257,32 @@ fn is_json_ext(ext: &str) -> bool {
232257
ext == "json" || ext == "jsonc"
233258
}
234259

260+
fn deserialize_external_plugins<'de, D>(
261+
deserializer: D,
262+
) -> Result<Option<FxHashSet<(PathBuf, String)>>, D::Error>
263+
where
264+
D: Deserializer<'de>,
265+
{
266+
let opt_set: Option<FxHashSet<String>> = Option::deserialize(deserializer)?;
267+
Ok(opt_set
268+
.map(|set| set.into_iter().map(|specifier| (PathBuf::default(), specifier)).collect()))
269+
}
270+
271+
#[expect(clippy::ref_option)]
272+
fn serialize_external_plugins<S>(
273+
plugins: &Option<FxHashSet<(PathBuf, String)>>,
274+
serializer: S,
275+
) -> Result<S::Ok, S::Error>
276+
where
277+
S: Serializer,
278+
{
279+
// Serialize as an array of original specifiers (the values in the map)
280+
match plugins {
281+
Some(set) => serializer.collect_seq(set.iter().map(|(_, specifier)| specifier)),
282+
None => serializer.serialize_none(),
283+
}
284+
}
285+
235286
#[cfg(test)]
236287
mod test {
237288
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"

tasks/website/src/linter/snapshots/schema_markdown.snap

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -197,7 +197,6 @@ Globs to ignore during linting. These are resolved from the configuration file p
197197

198198
type: `string[]`
199199

200-
default: `null`
201200

202201
JS plugins.
203202

0 commit comments

Comments
 (0)