Skip to content

Commit 28e76ec

Browse files
committed
fix(oxlint): resolving JS plugin failing when extends is used (#14556)
1 parent f5c83a9 commit 28e76ec

File tree

14 files changed

+176
-25
lines changed

14 files changed

+176
-25
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: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,13 @@ interface TestOptions {
1515
snapshotName?: string;
1616
// Function to get extra data to include in the snapshot
1717
getExtraSnapshotData?: (dirPath: string) => Promise<{ [key: string]: string }>;
18+
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+
*/
24+
overrideFiles?: string;
1825
}
1926

2027
/**
@@ -29,7 +36,7 @@ async function testFixture(fixtureName: string, options?: TestOptions): Promise<
2936
// Use current NodeJS executable, rather than `node`, to avoid problems with a Node version manager
3037
// installed on system resulting in using wrong NodeJS version
3138
command: process.execPath,
32-
args: [CLI_PATH, ...args, 'files'],
39+
args: [CLI_PATH, ...args, options?.overrideFiles ?? 'files'],
3340
fixtureName,
3441
snapshotName: options?.snapshotName ?? 'output',
3542
getExtraSnapshotData: options?.getExtraSnapshotData,
@@ -73,6 +80,12 @@ describe('oxlint CLI', () => {
7380
await testFixture('basic_custom_plugin_many_files');
7481
});
7582

83+
it('should load a custom plugin correctly when extending in a nested config', async () => {
84+
await testFixture('custom_plugin_nested_config', {
85+
overrideFiles: '.',
86+
});
87+
});
88+
7689
it('should load a custom plugin when configured in overrides', async () => {
7790
await testFixture('custom_plugin_via_overrides');
7891
});
Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
{
2+
"jsPlugins": ["./plugin.ts"],
3+
"rules": {
4+
"basic-custom-plugin/no-debugger": "error"
5+
},
6+
"categories": { "correctness": "off" }
7+
}
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
{
2+
"extends": ["../.oxlintrc.json"]
3+
}
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
debugger;
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+
```
Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
import type { Plugin } from '../../../dist/index.js';
2+
3+
const plugin: Plugin = {
4+
meta: {
5+
name: 'basic-custom-plugin',
6+
},
7+
rules: {
8+
'no-debugger': {
9+
create(context) {
10+
return {
11+
DebuggerStatement(debuggerStatement) {
12+
context.report({
13+
message: 'Unexpected Debugger Statement',
14+
node: debuggerStatement,
15+
});
16+
},
17+
};
18+
},
19+
},
20+
},
21+
};
22+
23+
export default plugin;

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: 39 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,32 @@ impl GlobSet {
139149
}
140150
}
141151

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

0 commit comments

Comments
 (0)