diff --git a/apps/oxlint/src-js/bindings.d.ts b/apps/oxlint/src-js/bindings.d.ts index 93921c1f0a557..f540a05ba066d 100644 --- a/apps/oxlint/src-js/bindings.d.ts +++ b/apps/oxlint/src-js/bindings.d.ts @@ -1,8 +1,14 @@ /* auto-generated by NAPI-RS */ /* eslint-disable */ +/** + * JS callable function to retrieve the serialized external rule options. + * Returns a JSON string of options arrays. Called once from JS after creating the external linter. + */ +export declare function getExternalRuleOptions(): string | null + /** JS callback to lint a file. */ export type JsLintFileCb = - ((arg0: string, arg1: number, arg2: Uint8Array | undefined | null, arg3: Array, arg4: string) => string) + ((arg0: string, arg1: number, arg2: Uint8Array | undefined | null, arg3: Array<[number, number]>) => string) /** JS callback to load a JS plugin. */ export type JsLoadPluginCb = diff --git a/apps/oxlint/src-js/bindings.js b/apps/oxlint/src-js/bindings.js index 9006b18c4f29c..a067191f6e6be 100644 --- a/apps/oxlint/src-js/bindings.js +++ b/apps/oxlint/src-js/bindings.js @@ -575,5 +575,6 @@ if (!nativeBinding) { throw new Error(`Failed to load native binding`) } -const { lint } = nativeBinding +const { getExternalRuleOptions, lint } = nativeBinding +export { getExternalRuleOptions } export { lint } diff --git a/apps/oxlint/src-js/cli.ts b/apps/oxlint/src-js/cli.ts index cb2d6fbad95a5..53e948cc73bd3 100644 --- a/apps/oxlint/src-js/cli.ts +++ b/apps/oxlint/src-js/cli.ts @@ -30,7 +30,7 @@ function loadPluginWrapper(path: string, packageName: string | null): Promise = Object.freeze({}); * @param filePath - Absolute path of file being linted * @param bufferId - ID of buffer containing file data * @param buffer - Buffer containing file data, or `null` if buffer with this ID was previously sent to JS - * @param ruleIds - IDs of rules to run on this file + * @param ruleData - Array of [ruleId, optionsId] pairs for rules to run on this file * @param settingsJSON - Settings for file, as JSON - * @returns Diagnostics or error serialized to JSON string + * @returns JSON result */ export function lintFile( filePath: string, bufferId: number, buffer: Uint8Array | null, - ruleIds: number[], + ruleData: Array<[number, number]>, settingsJSON: string, ): string { try { - lintFileImpl(filePath, bufferId, buffer, ruleIds, settingsJSON); + lintFileImpl(filePath, bufferId, buffer, ruleData, settingsJSON); return JSON.stringify({ Success: diagnostics }); } catch (err) { return JSON.stringify({ Failure: getErrorMessage(err) }); @@ -65,8 +66,8 @@ export function lintFile( * @param filePath - Absolute path of file being linted * @param bufferId - ID of buffer containing file data * @param buffer - Buffer containing file data, or `null` if buffer with this ID was previously sent to JS - * @param ruleIds - IDs of rules to run on this file - * @param settingsJSON - Stringified settings for this file + * @param ruleData - Array of [ruleId, optionsId] pairs for rules to run on this file + * @param settingsJSON - Settings for file, as JSON * @returns Diagnostics to send back to Rust * @throws {Error} If any parameters are invalid * @throws {*} If any rule throws @@ -75,7 +76,7 @@ function lintFileImpl( filePath: string, bufferId: number, buffer: Uint8Array | null, - ruleIds: number[], + ruleData: Array<[number, number]>, settingsJSON: string, ) { // If new buffer, add it to `buffers` array. Otherwise, get existing buffer from array. @@ -101,8 +102,8 @@ function lintFileImpl( if (typeof filePath !== 'string' || filePath.length === 0) { throw new Error('expected filePath to be a non-zero length string'); } - if (!Array.isArray(ruleIds) || ruleIds.length === 0) { - throw new Error('Expected `ruleIds` to be a non-zero len array'); + if (!Array.isArray(ruleData) || ruleData.length === 0) { + throw new Error('Expected `ruleData` to be a non-zero len array'); } // Pass file path to context module, so `Context`s know what file is being linted @@ -126,19 +127,27 @@ function lintFileImpl( // Get visitors for this file from all rules initCompiledVisitor(); - for (let i = 0, len = ruleIds.length; i < len; i++) { - const ruleId = ruleIds[i], - ruleDetails = registeredRules[ruleId]; + // Initialize external rule options if not already initialized + if (!areOptionsInitialized()) { + const optionsJson = getExternalRuleOptions(); + if (optionsJson !== null && optionsJson.length > 0) { + setOptions(optionsJson); + } + } - // Set `ruleIndex` for rule. It's used when sending diagnostics back to Rust. + for (let i = 0; i < ruleData.length; i++) { + const [ruleId, optionsId] = ruleData[i]; + const ruleDetails = registeredRules[ruleId]; + const options = allOptions[optionsId]; ruleDetails.ruleIndex = i; + ruleDetails.options = options === undefined || !Array.isArray(options) ? [] : (options as Options); - const { rule, context } = ruleDetails; + const { rule } = ruleDetails; let { visitor } = ruleDetails; if (visitor === null) { // Rule defined with `create` method - visitor = rule.create(context); + visitor = rule.create(ruleDetails.context); } else { // Rule defined with `createOnce` method const { beforeHook, afterHook } = ruleDetails; diff --git a/apps/oxlint/src-js/plugins/load.ts b/apps/oxlint/src-js/plugins/load.ts index eb01df3dfbb02..b9926c92a2c31 100644 --- a/apps/oxlint/src-js/plugins/load.ts +++ b/apps/oxlint/src-js/plugins/load.ts @@ -9,6 +9,43 @@ import type { JsonValue } from './json.ts'; import type { RuleMeta } from './rule_meta.ts'; import type { AfterHook, BeforeHook, Visitor, VisitorWithHooks } from './types.ts'; +// All external rule options serialized from Rust. +// Indexed by ExternalOptionsId (numeric) sent alongside ruleId for each file. +// Element 0 is always an empty array (reserved for rules with no options). +export let allOptions: unknown[][] = []; + +// Track if options have been initialized to avoid re-initialization +let optionsInitialized = false; + +/** + * Set all external rule options. + * Called once from Rust after config building, before any linting occurs. + * @param optionsJson - JSON string of outer array of per-options arrays. + */ +export function setOptions(optionsJson: string): void { + try { + const parsed = JSON.parse(optionsJson); + if (!Array.isArray(parsed)) throw new TypeError('Expected optionsJson to decode to an array'); + // Basic shape validation: each element must be an array (options tuple array) + for (let i = 0; i < parsed.length; i++) { + const el = parsed[i]; + if (!Array.isArray(el)) throw new TypeError('Each options entry must be an array'); + } + // Always update options - they may be set multiple times during the process + allOptions = parsed; + optionsInitialized = true; + } catch (err) { + // Re-throw with clearer message for Rust side logging. + throw new Error( + 'Failed to parse external rule options JSON: ' + (err instanceof Error ? err.message : String(err)), + ); + } +} + +export function areOptionsInitialized(): boolean { + return optionsInitialized; +} + const ObjectKeys = Object.keys; /** diff --git a/apps/oxlint/src/js_plugins/external_linter.rs b/apps/oxlint/src/js_plugins/external_linter.rs index 670552ee3ad81..f1f7de952e356 100644 --- a/apps/oxlint/src/js_plugins/external_linter.rs +++ b/apps/oxlint/src/js_plugins/external_linter.rs @@ -71,7 +71,7 @@ pub enum LintFileReturnValue { fn wrap_lint_file(cb: JsLintFileCb) -> ExternalLinterLintFileCb { Box::new( move |file_path: String, - rule_ids: Vec, + rule_data: Vec<(u32, u32)>, settings_json: String, allocator: &Allocator| { let (tx, rx) = channel(); @@ -87,7 +87,7 @@ fn wrap_lint_file(cb: JsLintFileCb) -> ExternalLinterLintFileCb { // Send data to JS let status = cb.call_with_return_value( - FnArgs::from((file_path, buffer_id, buffer, rule_ids, settings_json)), + FnArgs::from((file_path, buffer_id, buffer, rule_data, settings_json)), ThreadsafeFunctionCallMode::NonBlocking, move |result, _env| { let _ = match &result { diff --git a/apps/oxlint/src/lint.rs b/apps/oxlint/src/lint.rs index 4c36af07d6929..ff8abd00ea6e1 100644 --- a/apps/oxlint/src/lint.rs +++ b/apps/oxlint/src/lint.rs @@ -277,7 +277,7 @@ impl CliRunner { || nested_configs.values().any(|config| config.plugins().has_import()); let mut options = LintServiceOptions::new(self.cwd).with_cross_module(use_cross_module); - let lint_config = match config_builder.build(&external_plugin_store) { + let lint_config = match config_builder.build(&mut external_plugin_store) { Ok(config) => config, Err(e) => { print_and_flush_stdout( @@ -329,6 +329,13 @@ impl CliRunner { return CliRunResult::None; } + // After building config, serialize external rule options for JS side. + #[cfg(all(feature = "napi", target_pointer_width = "64", target_endian = "little"))] + { + let store = config_store.external_plugin_store(); + crate::set_external_options_json(store); + } + let files_to_lint = paths .into_iter() .filter(|path| !ignore_matcher.should_ignore(Path::new(path))) diff --git a/apps/oxlint/src/run.rs b/apps/oxlint/src/run.rs index f77b974545128..b4bdb3de8ca79 100644 --- a/apps/oxlint/src/run.rs +++ b/apps/oxlint/src/run.rs @@ -16,6 +16,30 @@ use crate::{ result::CliRunResult, }; +#[cfg(all(feature = "napi", target_pointer_width = "64", target_endian = "little"))] +use oxc_linter::ExternalPluginStore; + +#[cfg(all(feature = "napi", target_pointer_width = "64", target_endian = "little"))] +use std::sync::OnceLock; + +#[cfg(all(feature = "napi", target_pointer_width = "64", target_endian = "little"))] +static EXTERNAL_OPTIONS_JSON: OnceLock = OnceLock::new(); + +/// Set serialized external rule options JSON after building configs. +/// Called from Rust side (internal) before any linting, then consumed on first call to `lint`. +#[cfg(all(feature = "napi", target_pointer_width = "64", target_endian = "little"))] +pub fn set_external_options_json(plugin_store: &ExternalPluginStore) { + let _ = EXTERNAL_OPTIONS_JSON.set(plugin_store.serialize_all_options()); +} + +/// JS callable function to retrieve the serialized external rule options. +/// Returns a JSON string of options arrays. Called once from JS after creating the external linter. +#[cfg(all(feature = "napi", target_pointer_width = "64", target_endian = "little"))] +#[napi] +pub fn get_external_rule_options() -> Option { + EXTERNAL_OPTIONS_JSON.get().cloned() +} + /// JS callback to load a JS plugin. #[napi] pub type JsLoadPluginCb = ThreadsafeFunction< @@ -39,13 +63,13 @@ pub type JsLintFileCb = ThreadsafeFunction< String, // Absolute path of file to lint u32, // Buffer ID Option, // Buffer (optional) - Vec, // Array of rule IDs - String, // Stringified settings effective for the file + Vec<(u32, u32)>, // Array of [ruleId, optionsId] pairs + String, // Settings JSON )>, // Return value String, // `Vec`, serialized to JSON // Arguments (repeated) - FnArgs<(String, u32, Option, Vec, String)>, + FnArgs<(String, u32, Option, Vec<(u32, u32)>, String)>, // Error status Status, // CalleeHandled diff --git a/apps/oxlint/test/e2e.test.ts b/apps/oxlint/test/e2e.test.ts index be4c98e0627f4..45e0173444dc5 100644 --- a/apps/oxlint/test/e2e.test.ts +++ b/apps/oxlint/test/e2e.test.ts @@ -96,6 +96,14 @@ describe('oxlint CLI', () => { await testFixture('custom_plugin_via_overrides'); }); + it('should pass options to custom plugin rules', async () => { + await testFixture('custom_plugin_with_options'); + }); + + it('should pass options to custom plugin rules (working version)', async () => { + await testFixture('custom_plugin_options_working'); + }); + it('should report an error if a custom plugin is missing', async () => { await testFixture('missing_custom_plugin'); }); diff --git a/apps/oxlint/test/fixtures/custom_plugin_options_working/.oxlintrc.json b/apps/oxlint/test/fixtures/custom_plugin_options_working/.oxlintrc.json new file mode 100644 index 0000000000000..63eedac28585e --- /dev/null +++ b/apps/oxlint/test/fixtures/custom_plugin_options_working/.oxlintrc.json @@ -0,0 +1,6 @@ +{ + "jsPlugins": ["./plugin.js"], + "rules": { + "test-plugin-options/check-options": ["error", true, { "expected": "production" }] + } +} diff --git a/apps/oxlint/test/fixtures/custom_plugin_options_working/files/index.js b/apps/oxlint/test/fixtures/custom_plugin_options_working/files/index.js new file mode 100644 index 0000000000000..64a70539f6362 --- /dev/null +++ b/apps/oxlint/test/fixtures/custom_plugin_options_working/files/index.js @@ -0,0 +1,4 @@ +// Test file with debugger statement +debugger; + +console.log('Hello, world!'); diff --git a/apps/oxlint/test/fixtures/custom_plugin_options_working/output.snap.md b/apps/oxlint/test/fixtures/custom_plugin_options_working/output.snap.md new file mode 100644 index 0000000000000..a65d598e1326a --- /dev/null +++ b/apps/oxlint/test/fixtures/custom_plugin_options_working/output.snap.md @@ -0,0 +1,31 @@ +# Exit code +1 + +# stdout +``` + ! eslint(no-debugger): `debugger` statement is not allowed + ,-[files/index.js:2:1] + 1 | // Test file with debugger statement + 2 | debugger; + : ^^^^^^^^^ + 3 | + `---- + help: Remove the debugger statement + + x test-plugin-options(check-options): Expected value to be production, got disabled + ,-[files/index.js:2:1] + 1 | // Test file with debugger statement + 2 | debugger; + : ^^^^^^^^^ + 3 | + `---- + +Found 1 warning and 1 error. +Finished in Xms on 1 file using X threads. +``` + +# stderr +``` +WARNING: JS plugins are experimental and not subject to semver. +Breaking changes are possible while JS plugins support is under development. +``` diff --git a/apps/oxlint/test/fixtures/custom_plugin_options_working/plugin.js b/apps/oxlint/test/fixtures/custom_plugin_options_working/plugin.js new file mode 100644 index 0000000000000..a34415c0c7273 --- /dev/null +++ b/apps/oxlint/test/fixtures/custom_plugin_options_working/plugin.js @@ -0,0 +1,57 @@ +export default { + meta: { + name: 'test-plugin-options', + }, + rules: { + 'check-options': { + meta: { + messages: { + wrongValue: 'Expected value to be {{expected}}, got {{actual}}', + noOptions: 'No options provided', + }, + }, + createOnce(context) { + // Don't access context.options here - it's not available yet! + // Options are only available in visitor methods after setupContextForFile is called. + + return { + before() { + // Options are now available since setupContextForFile was called + const options = context.options; + + // Check if options were passed correctly + if (!options || options.length === 0) { + context.report({ + messageId: 'noOptions', + loc: { start: { line: 1, column: 0 }, end: { line: 1, column: 1 } }, + }); + return false; + } + return true; + }, + + DebuggerStatement(node) { + // Options are available in visitor methods + const options = context.options; + + // First option is a boolean + const shouldReport = options[0]; + // Second option is an object + const config = options[1] || {}; + + if (shouldReport) { + context.report({ + messageId: 'wrongValue', + data: { + expected: String(config.expected || 'enabled'), + actual: 'disabled', + }, + node, + }); + } + }, + }; + }, + }, + }, +}; diff --git a/apps/oxlint/test/fixtures/custom_plugin_with_options/.oxlintrc.json b/apps/oxlint/test/fixtures/custom_plugin_with_options/.oxlintrc.json new file mode 100644 index 0000000000000..21088deb76521 --- /dev/null +++ b/apps/oxlint/test/fixtures/custom_plugin_with_options/.oxlintrc.json @@ -0,0 +1,7 @@ +{ + "jsPlugins": ["./plugin.ts"], + "rules": { + "test-plugin-options/check-options": ["error", true, { "expected": "production" }] + } +} + diff --git a/apps/oxlint/test/fixtures/custom_plugin_with_options/files/index.js b/apps/oxlint/test/fixtures/custom_plugin_with_options/files/index.js new file mode 100644 index 0000000000000..64a70539f6362 --- /dev/null +++ b/apps/oxlint/test/fixtures/custom_plugin_with_options/files/index.js @@ -0,0 +1,4 @@ +// Test file with debugger statement +debugger; + +console.log('Hello, world!'); diff --git a/apps/oxlint/test/fixtures/custom_plugin_with_options/output.snap.md b/apps/oxlint/test/fixtures/custom_plugin_with_options/output.snap.md new file mode 100644 index 0000000000000..a65d598e1326a --- /dev/null +++ b/apps/oxlint/test/fixtures/custom_plugin_with_options/output.snap.md @@ -0,0 +1,31 @@ +# Exit code +1 + +# stdout +``` + ! eslint(no-debugger): `debugger` statement is not allowed + ,-[files/index.js:2:1] + 1 | // Test file with debugger statement + 2 | debugger; + : ^^^^^^^^^ + 3 | + `---- + help: Remove the debugger statement + + x test-plugin-options(check-options): Expected value to be production, got disabled + ,-[files/index.js:2:1] + 1 | // Test file with debugger statement + 2 | debugger; + : ^^^^^^^^^ + 3 | + `---- + +Found 1 warning and 1 error. +Finished in Xms on 1 file using X threads. +``` + +# stderr +``` +WARNING: JS plugins are experimental and not subject to semver. +Breaking changes are possible while JS plugins support is under development. +``` diff --git a/apps/oxlint/test/fixtures/custom_plugin_with_options/plugin.ts b/apps/oxlint/test/fixtures/custom_plugin_with_options/plugin.ts new file mode 100644 index 0000000000000..281de0880e972 --- /dev/null +++ b/apps/oxlint/test/fixtures/custom_plugin_with_options/plugin.ts @@ -0,0 +1,59 @@ +import { definePlugin } from 'oxlint'; + +export default definePlugin({ + meta: { + name: 'test-plugin-options', + }, + rules: { + 'check-options': { + meta: { + messages: { + wrongValue: 'Expected value to be {{expected}}, got {{actual}}', + noOptions: 'No options provided', + }, + }, + createOnce(context) { + // Don't access context.options here - it's not available yet! + // Options are only available in visitor methods after setupContextForFile is called. + + return { + before() { + // Options are now available since setupContextForFile was called + const options = context.options; + + // Check if options were passed correctly + if (!options || options.length === 0) { + context.report({ + messageId: 'noOptions', + loc: { start: { line: 1, column: 0 }, end: { line: 1, column: 1 } }, + }); + return false; + } + return true; + }, + + DebuggerStatement(node) { + // Options are available in visitor methods + const options = context.options; + + // First option is a boolean + const shouldReport = options[0]; + // Second option is an object + const config = (options[1] || {}) as { expected?: string }; + + if (shouldReport) { + context.report({ + messageId: 'wrongValue', + data: { + expected: String(config.expected || 'enabled'), + actual: 'disabled', + }, + node, + }); + } + }, + }; + }, + }, + }, +}); diff --git a/crates/oxc_language_server/src/linter/mod.rs b/crates/oxc_language_server/src/linter/mod.rs index c8c05c4ea6587..e54bcbff90918 100644 --- a/crates/oxc_language_server/src/linter/mod.rs +++ b/crates/oxc_language_server/src/linter/mod.rs @@ -6,6 +6,8 @@ mod isolated_lint_handler; mod options; mod server_linter; #[cfg(test)] +mod server_linter_js_plugins_test; +#[cfg(test)] mod tester; pub use server_linter::ServerLinterBuilder; diff --git a/crates/oxc_language_server/src/linter/server_linter.rs b/crates/oxc_language_server/src/linter/server_linter.rs index b1b0ea6703401..74d61931ef689 100644 --- a/crates/oxc_language_server/src/linter/server_linter.rs +++ b/crates/oxc_language_server/src/linter/server_linter.rs @@ -88,9 +88,9 @@ impl ServerLinterBuilder { && nested_configs.pin().values().any(|config| config.plugins().has_import())); extended_paths.extend(config_builder.extended_paths.clone()); - let base_config = config_builder.build(&external_plugin_store).unwrap_or_else(|err| { + let base_config = config_builder.build(&mut external_plugin_store).unwrap_or_else(|err| { warn!("Failed to build config: {err}"); - ConfigStoreBuilder::empty().build(&external_plugin_store).unwrap() + ConfigStoreBuilder::empty().build(&mut external_plugin_store).unwrap() }); let lint_options = LintOptions { @@ -195,10 +195,11 @@ impl ServerLinterBuilder { continue; }; extended_paths.extend(config_store_builder.extended_paths.clone()); - let config = config_store_builder.build(&external_plugin_store).unwrap_or_else(|err| { - warn!("Failed to build nested config for {}: {:?}", dir_path.display(), err); - ConfigStoreBuilder::empty().build(&external_plugin_store).unwrap() - }); + let config = + config_store_builder.build(&mut external_plugin_store).unwrap_or_else(|err| { + warn!("Failed to build nested config for {}: {:?}", dir_path.display(), err); + ConfigStoreBuilder::empty().build(&mut external_plugin_store).unwrap() + }); nested_configs.pin().insert(dir_path.to_path_buf(), config); } diff --git a/crates/oxc_language_server/src/linter/server_linter_js_plugins_test.rs b/crates/oxc_language_server/src/linter/server_linter_js_plugins_test.rs new file mode 100644 index 0000000000000..7e219f881079d --- /dev/null +++ b/crates/oxc_language_server/src/linter/server_linter_js_plugins_test.rs @@ -0,0 +1,38 @@ +#[cfg(test)] +mod test { + use std::fs; + use std::path::PathBuf; + use std::time::{SystemTime, UNIX_EPOCH}; + + use serde_json::json; + use tower_lsp_server::{UriExt, lsp_types::Uri}; + + use super::super::server_linter::ServerLinterBuilder; + + #[test] + fn test_new_with_js_plugins_in_config_does_not_crash() { + // Create a unique temp directory + let mut dir = std::env::temp_dir(); + let uniq = SystemTime::now().duration_since(UNIX_EPOCH).unwrap().as_millis(); + dir.push(format!("oxc_ls_test_{uniq}")); + fs::create_dir_all(&dir).unwrap(); + + // Write a config containing jsPlugins and a rule belonging to an external plugin + let config_path = dir.join(".oxlintrc.json"); + let config = json!({ + "jsPlugins": ["custom-plugin"], + "rules": { + "custom/some-rule": ["warn", {"flag": true}] + } + }); + fs::write(&config_path, serde_json::to_vec(&config).unwrap()).unwrap(); + + // Initialize language server linter over the directory; should not panic/crash + let uri = Uri::from_file_path(PathBuf::from(&dir)).unwrap(); + let _server = ServerLinterBuilder::build(&uri, json!({})); + + // Cleanup best-effort + let _ = fs::remove_file(config_path); + let _ = fs::remove_dir(dir); + } +} diff --git a/crates/oxc_linter/src/config/config_builder.rs b/crates/oxc_linter/src/config/config_builder.rs index ece65cba25884..eaac79e60e3af 100644 --- a/crates/oxc_linter/src/config/config_builder.rs +++ b/crates/oxc_linter/src/config/config_builder.rs @@ -16,7 +16,7 @@ use crate::{ ESLintRule, OxlintOverrides, OxlintRules, overrides::OxlintOverride, plugins::LintPlugins, }, external_linter::ExternalLinter, - external_plugin_store::{ExternalRuleId, ExternalRuleLookupError}, + external_plugin_store::{ExternalOptionsId, ExternalRuleId, ExternalRuleLookupError}, rules::RULES, }; @@ -29,7 +29,7 @@ use super::{ #[must_use = "You dropped your builder without building a Linter! Did you mean to call .build()?"] pub struct ConfigStoreBuilder { pub(super) rules: FxHashMap, - pub(super) external_rules: FxHashMap, + pub(super) external_rules: FxHashMap, config: LintConfig, categories: OxlintCategories, overrides: OxlintOverrides, @@ -385,7 +385,7 @@ impl ConfigStoreBuilder { /// Returns [`ConfigBuilderError::UnknownRules`] if there are rules that could not be matched. pub fn build( mut self, - external_plugin_store: &ExternalPluginStore, + external_plugin_store: &mut ExternalPluginStore, ) -> Result { // When a plugin gets disabled before build(), rules for that plugin aren't removed until // with_filters() gets called. If the user never calls it, those now-undesired rules need @@ -412,8 +412,14 @@ impl ConfigStoreBuilder { .collect(); rules.sort_unstable_by_key(|(r, _)| r.id()); - let mut external_rules: Vec<_> = self.external_rules.into_iter().collect(); - external_rules.sort_unstable_by_key(|(r, _)| *r); + // Convert HashMap entries (ExternalRuleId -> (ExternalOptionsId, AllowWarnDeny)) + // into Vec<(ExternalRuleId, ExternalOptionsId, AllowWarnDeny)> and sort by rule id. + let mut external_rules: Vec<_> = self + .external_rules + .into_iter() + .map(|(rule_id, (options_id, severity))| (rule_id, options_id, severity)) + .collect(); + external_rules.sort_unstable_by_key(|(r, _, _)| *r); Ok(Config::new(rules, external_rules, self.categories, self.config, resolved_overrides)) } @@ -421,15 +427,19 @@ impl ConfigStoreBuilder { fn resolve_overrides( &self, overrides: OxlintOverrides, - external_plugin_store: &ExternalPluginStore, + external_plugin_store: &mut ExternalPluginStore, ) -> Result { let resolved = overrides .into_iter() .map(|override_config| { let mut builtin_rules = Vec::new(); - let mut external_rules = Vec::new(); + let mut external_rules: Vec<(ExternalRuleId, ExternalOptionsId, AllowWarnDeny)> = + Vec::new(); let mut rules_map = FxHashMap::default(); - let mut external_rules_map = FxHashMap::default(); + let mut external_rules_map: FxHashMap< + ExternalRuleId, + (ExternalOptionsId, AllowWarnDeny), + > = FxHashMap::default(); let all_rules = self.get_all_rules_for_plugins(override_config.plugins); @@ -443,7 +453,11 @@ impl ConfigStoreBuilder { // Convert to vectors builtin_rules.extend(rules_map.into_iter()); - external_rules.extend(external_rules_map.into_iter()); + external_rules.extend( + external_rules_map + .into_iter() + .map(|(rule_id, (options_id, severity))| (rule_id, options_id, severity)), + ); Ok(ResolvedOxlintOverride { files: override_config.files, @@ -813,10 +827,10 @@ mod test { let mut desired_plugins = LintPlugins::default(); desired_plugins.set(LintPlugins::TYPESCRIPT, false); - let external_plugin_store = ExternalPluginStore::default(); + let mut external_plugin_store = ExternalPluginStore::default(); let linter = ConfigStoreBuilder::default() .with_builtin_plugins(desired_plugins) - .build(&external_plugin_store) + .build(&mut external_plugin_store) .unwrap(); for (rule, _) in linter.base.rules.iter() { let name = rule.name(); @@ -1261,7 +1275,7 @@ mod test { ) .unwrap(); - let config = builder.build(&external_plugin_store).unwrap(); + let config = builder.build(&mut external_plugin_store).unwrap(); // Apply overrides for a foo.test.ts file (matches both overrides) let resolved = config.apply_overrides(Path::new("foo.test.ts")); @@ -1287,7 +1301,7 @@ mod test { &mut external_plugin_store, ) .unwrap() - .build(&external_plugin_store) + .build(&mut external_plugin_store) .unwrap() } @@ -1300,7 +1314,7 @@ mod test { &mut external_plugin_store, ) .unwrap() - .build(&external_plugin_store) + .build(&mut external_plugin_store) .unwrap() } } diff --git a/crates/oxc_linter/src/config/config_store.rs b/crates/oxc_linter/src/config/config_store.rs index d174ac237eca6..c4e36d750ad7d 100644 --- a/crates/oxc_linter/src/config/config_store.rs +++ b/crates/oxc_linter/src/config/config_store.rs @@ -7,7 +7,7 @@ use rustc_hash::FxHashMap; use crate::{ AllowWarnDeny, - external_plugin_store::{ExternalPluginStore, ExternalRuleId}, + external_plugin_store::{ExternalOptionsId, ExternalPluginStore, ExternalRuleId}, rules::{RULES, RuleEnum}, }; @@ -23,7 +23,7 @@ pub struct ResolvedLinterState { pub rules: Arc<[(RuleEnum, AllowWarnDeny)]>, pub config: Arc, - pub external_rules: Arc<[(ExternalRuleId, AllowWarnDeny)]>, + pub external_rules: Arc<[(ExternalRuleId, ExternalOptionsId, AllowWarnDeny)]>, } impl Clone for ResolvedLinterState { @@ -65,7 +65,7 @@ pub struct ResolvedOxlintOverride { #[derive(Debug, Clone)] pub struct ResolvedOxlintOverrideRules { pub(crate) builtin_rules: Vec<(RuleEnum, AllowWarnDeny)>, - pub(crate) external_rules: Vec<(ExternalRuleId, AllowWarnDeny)>, + pub(crate) external_rules: Vec<(ExternalRuleId, ExternalOptionsId, AllowWarnDeny)>, } #[derive(Debug, Clone)] @@ -90,7 +90,7 @@ pub struct Config { impl Config { pub fn new( rules: Vec<(RuleEnum, AllowWarnDeny)>, - mut external_rules: Vec<(ExternalRuleId, AllowWarnDeny)>, + mut external_rules: Vec<(ExternalRuleId, ExternalOptionsId, AllowWarnDeny)>, categories: OxlintCategories, config: LintConfig, overrides: ResolvedOxlintOverrides, @@ -107,7 +107,7 @@ impl Config { ), config: Arc::new(config), external_rules: Arc::from({ - external_rules.retain(|(_, sev)| sev.is_warn_deny()); + external_rules.retain(|(_, _, sev)| sev.is_warn_deny()); external_rules.into_boxed_slice() }), }, @@ -184,8 +184,13 @@ impl Config { .cloned() .collect::>(); - let mut external_rules = - self.base.external_rules.iter().copied().collect::>(); + // Build a hashmap of existing external rules keyed by rule id with value (options_id, severity) + let mut external_rules: FxHashMap = + self.base + .external_rules + .iter() + .map(|(rule_id, options_id, severity)| (*rule_id, (*options_id, *severity))) + .collect(); // Track which plugins have already had their category rules applied. // Start with the root plugins since they already have categories applied in base_rules. @@ -229,8 +234,8 @@ impl Config { } } - for (external_rule_id, severity) in &override_config.rules.external_rules { - external_rules.insert(*external_rule_id, *severity); + for (external_rule_id, options_id, severity) in &override_config.rules.external_rules { + external_rules.insert(*external_rule_id, (*options_id, *severity)); } if let Some(override_env) = &override_config.env { @@ -263,7 +268,8 @@ impl Config { let external_rules = external_rules .into_iter() - .filter(|(_, severity)| severity.is_warn_deny()) + .filter(|(_, (.., severity))| severity.is_warn_deny()) + .map(|(rule_id, (options_id, severity))| (rule_id, options_id, severity)) .collect::>(); ResolvedLinterState { @@ -320,6 +326,11 @@ impl ConfigStore { self.base.base.config.plugins } + /// Access the external plugin store. Used by oxlint CLI to serialize external rule options. + pub fn external_plugin_store(&self) -> &ExternalPluginStore { + &self.external_plugin_store + } + pub(crate) fn get_related_config(&self, path: &Path) -> &Config { if self.nested_configs.is_empty() { &self.base @@ -366,7 +377,7 @@ mod test { use super::{ConfigStore, ResolvedOxlintOverrides}; use crate::{ - AllowWarnDeny, ExternalPluginStore, LintPlugins, RuleCategory, RuleEnum, + AllowWarnDeny, ExternalOptionsId, ExternalPluginStore, LintPlugins, RuleCategory, RuleEnum, config::{ LintConfig, OxlintEnv, OxlintGlobals, OxlintSettings, categories::OxlintCategories, @@ -717,7 +728,7 @@ mod test { let store = ConfigStore::new( Config::new( vec![], - vec![(rule_id, AllowWarnDeny::Deny)], + vec![(rule_id, ExternalOptionsId::from_usize(0), AllowWarnDeny::Deny)], OxlintCategories::default(), LintConfig::default(), overrides, @@ -1062,4 +1073,53 @@ mod test { assert_eq!(store_with_nested_configs.number_of_rules(false), None); assert_eq!(store_with_nested_configs.number_of_rules(true), None); } + + #[test] + fn test_external_rule_options_override_precedence() { + // Prepare external plugin store with a custom plugin and rule + let mut store = ExternalPluginStore::new(true); + store.register_plugin( + "path/to/custom".to_string(), + "custom".to_string(), + 0, + vec!["my-rule".to_string()], + ); + + // Base config has external rule with options A, severity warn + let base_external_rule_id = store.lookup_rule_id("custom", "my-rule").unwrap(); + let base_options_id = store.add_options(serde_json::json!([{ "opt": "A" }])); + + let base = Config::new( + vec![], + vec![(base_external_rule_id, base_options_id, AllowWarnDeny::Warn)], + OxlintCategories::default(), + LintConfig::default(), + ResolvedOxlintOverrides::new(vec![ResolvedOxlintOverride { + files: GlobSet::new(vec!["*.js"]), + env: None, + globals: None, + plugins: None, + // Override redefines the same rule with options B and severity error + rules: ResolvedOxlintOverrideRules { + builtin_rules: vec![], + external_rules: vec![( + base_external_rule_id, + store.add_options(serde_json::json!([{ "opt": "B" }])), + AllowWarnDeny::Deny, + )], + }, + }]), + ); + + let config_store = ConfigStore::new(base, FxHashMap::default(), store); + let resolved = config_store.resolve(&PathBuf::from_str("/root/a.js").unwrap()); + + // Should prefer override (options B, severity error) + assert_eq!(resolved.external_rules.len(), 1); + let (rid, opts_id, sev) = resolved.external_rules[0]; + assert_eq!(rid, base_external_rule_id); + assert!(sev.is_warn_deny()); + // opts_id should not equal the base options id + assert_ne!(opts_id, base_options_id); + } } diff --git a/crates/oxc_linter/src/config/mod.rs b/crates/oxc_linter/src/config/mod.rs index 84c5ce2902bbc..4dda1aa486788 100644 --- a/crates/oxc_linter/src/config/mod.rs +++ b/crates/oxc_linter/src/config/mod.rs @@ -114,14 +114,14 @@ mod test { let config = Oxlintrc::from_file(&fixture_path).unwrap(); let mut set = FxHashMap::default(); let mut external_rules_for_override = FxHashMap::default(); - let external_linter_store = ExternalPluginStore::default(); + let mut external_linter_store = ExternalPluginStore::default(); config .rules .override_rules( &mut set, &mut external_rules_for_override, &RULES, - &external_linter_store, + &mut external_linter_store, ) .unwrap(); diff --git a/crates/oxc_linter/src/config/rules.rs b/crates/oxc_linter/src/config/rules.rs index d42bb08e3c7bc..5e1ffff7fcc79 100644 --- a/crates/oxc_linter/src/config/rules.rs +++ b/crates/oxc_linter/src/config/rules.rs @@ -13,7 +13,7 @@ use oxc_diagnostics::{Error, OxcDiagnostic}; use crate::{ AllowWarnDeny, ExternalPluginStore, LintPlugins, - external_plugin_store::{ExternalRuleId, ExternalRuleLookupError}, + external_plugin_store::{ExternalOptionsId, ExternalRuleId, ExternalRuleLookupError}, rules::{RULES, RuleEnum}, utils::{is_eslint_rule_adapted_to_typescript, is_jest_rule_adapted_to_vitest}, }; @@ -63,9 +63,12 @@ impl OxlintRules { pub(crate) fn override_rules( &self, rules_for_override: &mut RuleSet, - external_rules_for_override: &mut FxHashMap, + external_rules_for_override: &mut FxHashMap< + ExternalRuleId, + (ExternalOptionsId, AllowWarnDeny), + >, all_rules: &[RuleEnum], - external_plugin_store: &ExternalPluginStore, + external_plugin_store: &mut ExternalPluginStore, ) -> Result<(), ExternalRuleLookupError> { let mut rules_to_replace = vec![]; @@ -106,10 +109,22 @@ impl OxlintRules { if external_plugin_store.is_enabled() { let external_rule_id = external_plugin_store.lookup_rule_id(plugin_name, rule_name)?; + + // Add options to store and get options ID + let options_id = if let Some(config) = &rule_config.config { + external_plugin_store.add_options(config.clone()) + } else { + // No options - use reserved index 0 + ExternalOptionsId::from_usize(0) + }; + external_rules_for_override .entry(external_rule_id) - .and_modify(|sev| *sev = severity) - .or_insert(severity); + .and_modify(|(opts_id, sev)| { + *opts_id = options_id; + *sev = severity; + }) + .or_insert((options_id, severity)); } } } @@ -329,6 +344,7 @@ mod test { use serde::Deserialize; use serde_json::{Value, json}; + use crate::external_plugin_store::ExternalOptionsId; use crate::{ AllowWarnDeny, ExternalPluginStore, rules::{RULES, RuleEnum}, @@ -381,9 +397,14 @@ mod test { fn r#override(rules: &mut RuleSet, rules_rc: &Value) { let rules_config = OxlintRules::deserialize(rules_rc).unwrap(); let mut external_rules_for_override = FxHashMap::default(); - let external_linter_store = ExternalPluginStore::default(); + let mut external_linter_store = ExternalPluginStore::default(); rules_config - .override_rules(rules, &mut external_rules_for_override, &RULES, &external_linter_store) + .override_rules( + rules, + &mut external_rules_for_override, + &RULES, + &mut external_linter_store, + ) .unwrap(); } @@ -517,4 +538,73 @@ mod test { assert_eq!(r2.plugin_name, "unicorn"); assert!(r2.severity.is_warn_deny()); } + + #[test] + fn test_external_rule_options_are_recorded() { + // Register a fake external plugin and rule + let mut store = ExternalPluginStore::new(true); + store.register_plugin( + "path/to/custom-plugin".to_string(), + "custom".to_string(), + 0, + vec!["my-rule".to_string()], + ); + + // Configure rule with options array (non-empty) and ensure options id != 0 + let rules = OxlintRules::deserialize(&json!({ + "custom/my-rule": ["warn", {"foo": 1}] + })) + .unwrap(); + + let mut builtin_rules = RuleSet::default(); + let mut external_rules = FxHashMap::default(); + rules.override_rules(&mut builtin_rules, &mut external_rules, &[], &mut store).unwrap(); + + assert_eq!(builtin_rules.len(), 0); + assert_eq!(external_rules.len(), 1); + let (_rule_id, (options_id, severity)) = + external_rules.iter().next().map(|(k, v)| (*k, *v)).unwrap(); + assert_ne!( + options_id, + ExternalOptionsId::from_usize(0), + "non-empty options should allocate a new id" + ); + assert!(severity.is_warn_deny()); + + // Now configure with no options which should map to reserved index 0 + let rules_no_opts = OxlintRules::deserialize(&json!({ + "custom/my-rule": "error" + })) + .unwrap(); + let mut builtin_rules2 = RuleSet::default(); + let mut external_rules2 = FxHashMap::default(); + rules_no_opts + .override_rules(&mut builtin_rules2, &mut external_rules2, &[], &mut store) + .unwrap(); + let (_rid2, (options_id2, severity2)) = + external_rules2.iter().next().map(|(k, v)| (*k, *v)).unwrap(); + assert_eq!( + options_id2, + ExternalOptionsId::from_usize(0), + "no options should use reserved id 0" + ); + assert!(severity2.is_warn_deny()); + + // Test that null config values also map to reserved index 0 + // This tests the case where config might be explicitly null (though unlikely in practice) + let null_options_id = store.add_options(serde_json::Value::Null); + assert_eq!( + null_options_id, + ExternalOptionsId::from_usize(0), + "null options should use reserved id 0" + ); + + // Test that empty array also maps to reserved index 0 + let empty_array_id = store.add_options(serde_json::json!([])); + assert_eq!( + empty_array_id, + ExternalOptionsId::from_usize(0), + "empty array options should use reserved id 0" + ); + } } diff --git a/crates/oxc_linter/src/external_linter.rs b/crates/oxc_linter/src/external_linter.rs index c62d04e7ccdaf..1e8c779d7dd49 100644 --- a/crates/oxc_linter/src/external_linter.rs +++ b/crates/oxc_linter/src/external_linter.rs @@ -14,7 +14,7 @@ pub type ExternalLinterLoadPluginCb = Box< >; pub type ExternalLinterLintFileCb = Box< - dyn Fn(String, Vec, String, &Allocator) -> Result, String> + dyn Fn(String, Vec<(u32, u32)>, String, &Allocator) -> Result, String> + Sync + Send, >; diff --git a/crates/oxc_linter/src/external_plugin_store.rs b/crates/oxc_linter/src/external_plugin_store.rs index 266d080644c2f..0cbf385ad1c51 100644 --- a/crates/oxc_linter/src/external_plugin_store.rs +++ b/crates/oxc_linter/src/external_plugin_store.rs @@ -12,6 +12,10 @@ define_index_type! { pub struct ExternalRuleId = u32; } +define_index_type! { + pub struct ExternalOptionsId = u32; +} + #[derive(Debug)] pub struct ExternalPluginStore { registered_plugin_paths: FxHashSet, @@ -19,6 +23,7 @@ pub struct ExternalPluginStore { plugins: IndexVec, plugin_names: FxHashMap, rules: IndexVec, + options: IndexVec, // `true` for `oxlint`, `false` for language server is_enabled: bool, @@ -32,11 +37,16 @@ impl Default for ExternalPluginStore { impl ExternalPluginStore { pub fn new(is_enabled: bool) -> Self { + let mut options = IndexVec::default(); + // Index 0 is reserved for "no options" (empty array) + options.push(serde_json::json!([])); + Self { registered_plugin_paths: FxHashSet::default(), plugins: IndexVec::default(), plugin_names: FxHashMap::default(), rules: IndexVec::default(), + options, is_enabled, } } @@ -116,6 +126,25 @@ impl ExternalPluginStore { let plugin = &self.plugins[external_rule.plugin_id]; (&plugin.name, &external_rule.name) } + + /// Add options to the store and return its ID. + /// Returns index 0 for empty arrays or null values (no options). + pub fn add_options(&mut self, options: serde_json::Value) -> ExternalOptionsId { + // If it's null or an empty array, return reserved index 0 + if options.is_null() || options.as_array().is_some_and(Vec::is_empty) { + return ExternalOptionsId::from_usize(0); + } + + self.options.push(options) + } + + /// Serialize all options to JSON string. + /// + /// # Panics + /// Panics if serialization fails. + pub fn serialize_all_options(&self) -> String { + serde_json::to_string(&self.options).expect("Failed to serialize options") + } } #[derive(Debug, Clone, PartialEq, Eq)] diff --git a/crates/oxc_linter/src/lib.rs b/crates/oxc_linter/src/lib.rs index 3bd5efe74b653..f8755e711ee35 100644 --- a/crates/oxc_linter/src/lib.rs +++ b/crates/oxc_linter/src/lib.rs @@ -63,7 +63,7 @@ pub use crate::{ ExternalLinter, ExternalLinterLintFileCb, ExternalLinterLoadPluginCb, JsFix, LintFileResult, PluginLoadResult, }, - external_plugin_store::{ExternalPluginStore, ExternalRuleId}, + external_plugin_store::{ExternalOptionsId, ExternalPluginStore, ExternalRuleId}, fixer::{Fix, FixKind, Message, PossibleFixes}, frameworks::FrameworkFlags, lint_runner::{DirectivesStore, LintRunner, LintRunnerBuilder}, @@ -384,7 +384,7 @@ impl Linter { fn run_external_rules<'a>( &self, - external_rules: &[(ExternalRuleId, AllowWarnDeny)], + external_rules: &[(ExternalRuleId, ExternalOptionsId, AllowWarnDeny)], path: &Path, ctx_host: &mut Rc>, allocator: &'a Allocator, @@ -461,9 +461,13 @@ impl Linter { None => "{}".to_string(), }; + // Pass AST and rule IDs + options IDs to JS let result = (external_linter.lint_file)( path.to_str().unwrap().to_string(), - external_rules.iter().map(|(rule_id, _)| rule_id.raw()).collect(), + external_rules + .iter() + .map(|(rule_id, options_id, _)| (rule_id.raw(), options_id.raw())) + .collect(), settings_json, allocator, ); @@ -477,7 +481,7 @@ impl Linter { let mut span = Span::new(diagnostic.start, diagnostic.end); span_converter.convert_span_back(&mut span); - let (external_rule_id, severity) = + let (external_rule_id, _options_id, severity) = external_rules[diagnostic.rule_index as usize]; let (plugin_name, rule_name) = self.config.resolve_plugin_rule_names(external_rule_id); diff --git a/crates/oxc_linter/src/tester.rs b/crates/oxc_linter/src/tester.rs index d7fe212318b31..f31a09fcc3979 100644 --- a/crates/oxc_linter/src/tester.rs +++ b/crates/oxc_linter/src/tester.rs @@ -525,7 +525,7 @@ impl Tester { }), ) .with_rule(rule, AllowWarnDeny::Warn) - .build(&external_plugin_store) + .build(&mut external_plugin_store) .unwrap(), FxHashMap::default(), external_plugin_store, diff --git a/napi/playground/src/lib.rs b/napi/playground/src/lib.rs index 21400414182a7..f0a09d77c5527 100644 --- a/napi/playground/src/lib.rs +++ b/napi/playground/src/lib.rs @@ -375,7 +375,7 @@ impl Oxc { ) { // Only lint if there are no syntax errors if run_options.lint && self.diagnostics.is_empty() { - let external_plugin_store = ExternalPluginStore::default(); + let mut external_plugin_store = ExternalPluginStore::default(); let semantic_ret = SemanticBuilder::new().with_cfg(true).build(program); let semantic = semantic_ret.semantic; let lint_config = if linter_options.config.is_some() { @@ -386,12 +386,12 @@ impl Oxc { false, oxlintrc, None, - &mut ExternalPluginStore::default(), + &mut external_plugin_store, ) .unwrap_or_default(); - config_builder.build(&external_plugin_store) + config_builder.build(&mut external_plugin_store) } else { - ConfigStoreBuilder::default().build(&external_plugin_store) + ConfigStoreBuilder::default().build(&mut external_plugin_store) }; let lint_config = lint_config.unwrap(); let linter_ret = Linter::new( diff --git a/tasks/benchmark/benches/linter.rs b/tasks/benchmark/benches/linter.rs index bcea7bdde3af1..e4bcdb5298836 100644 --- a/tasks/benchmark/benches/linter.rs +++ b/tasks/benchmark/benches/linter.rs @@ -38,8 +38,9 @@ fn bench_linter(criterion: &mut Criterion) { let semantic = semantic_ret.semantic; let module_record = Arc::new(ModuleRecord::new(path, &parser_ret.module_record, &semantic)); - let external_plugin_store = ExternalPluginStore::default(); - let lint_config = ConfigStoreBuilder::all().build(&external_plugin_store).unwrap(); + let mut external_plugin_store = ExternalPluginStore::default(); + let lint_config = + ConfigStoreBuilder::all().build(&mut external_plugin_store).unwrap(); let linter = Linter::new( LintOptions::default(), ConfigStore::new(lint_config, FxHashMap::default(), external_plugin_store),