diff --git a/apps/oxlint/src-js/bindings.d.ts b/apps/oxlint/src-js/bindings.d.ts index f0a161bcdd837..0259264e048a1 100644 --- a/apps/oxlint/src-js/bindings.d.ts +++ b/apps/oxlint/src-js/bindings.d.ts @@ -10,23 +10,28 @@ export declare function getBufferOffset(buffer: Uint8Array): number /** 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, arg4: Array, arg5: string) => string) /** JS callback to load a JS plugin. */ export type JsLoadPluginCb = ((arg0: string, arg1?: string | undefined | null) => Promise) +/** JS callback to setup configs. */ +export type JsSetupConfigsCb = + ((arg: string) => void) + /** * NAPI entry point. * * JS side passes in: * 1. `args`: Command line arguments (process.argv.slice(2)) * 2. `load_plugin`: Load a JS plugin from a file path. - * 3. `lint_file`: Lint a file. + * 3. `setup_configs`: Setup configuration options. + * 4. `lint_file`: Lint a file. * * Returns `true` if linting succeeded without errors, `false` otherwise. */ -export declare function lint(args: Array, loadPlugin: JsLoadPluginCb, lintFile: JsLintFileCb): Promise +export declare function lint(args: Array, loadPlugin: JsLoadPluginCb, setupConfigs: JsSetupConfigsCb, lintFile: JsLintFileCb): Promise /** * Parse AST into provided `Uint8Array` buffer, synchronously. diff --git a/apps/oxlint/src-js/cli.ts b/apps/oxlint/src-js/cli.ts index 34450e0f6c0c9..299bb910f69f9 100644 --- a/apps/oxlint/src-js/cli.ts +++ b/apps/oxlint/src-js/cli.ts @@ -5,6 +5,7 @@ import { debugAssertIsNonNull } from "./utils/asserts.js"; // Using `typeof wrapper` here makes TS check that the function signatures of `loadPlugin` and `loadPluginWrapper` // are identical. Ditto `lintFile` and `lintFileWrapper`. let loadPlugin: typeof loadPluginWrapper | null = null; +let setupConfigs: typeof setupConfigsWrapper | null = null; let lintFile: typeof lintFileWrapper | null = null; /** @@ -21,7 +22,7 @@ function loadPluginWrapper(path: string, packageName: string | null): Promise { - ({ loadPlugin, lintFile } = mod); + ({ loadPlugin, lintFile, setupConfigs } = mod); return loadPlugin(path, packageName); }); } @@ -29,6 +30,18 @@ function loadPluginWrapper(path: string, packageName: string | null): Promise = Object.freeze({}); * @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 optionsIds - IDs of options to use for rules on this file, in same order as `ruleIds` * @param settingsJSON - Settings for file, as JSON * @returns Diagnostics or error serialized to JSON string */ @@ -56,11 +57,9 @@ export function lintFile( bufferId: number, buffer: Uint8Array | null, ruleIds: number[], + optionsIds: number[], settingsJSON: string, ): string { - // TODO: Get `optionsIds` from Rust side - const optionsIds = ruleIds.map((_) => DEFAULT_OPTIONS_ID); - try { lintFileImpl(filePath, bufferId, buffer, ruleIds, optionsIds, settingsJSON); @@ -85,7 +84,7 @@ export function lintFile( * @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 optionsIds - IDs of options to use for rules on this file + * @param optionsIds - IDs of options to use for rules on this file, in same order as `ruleIds` * @param settingsJSON - Stringified settings for this file * @throws {Error} If any parameters are invalid * @throws {*} If any rule throws @@ -164,6 +163,9 @@ export function lintFileImpl( // Set `options` for rule const optionsId = optionsIds[i]; debugAssert(optionsId < allOptions.length, "Options ID out of bounds"); + + // If the rule has no user-provided options, use the plugin-provided default + // options (which falls back to `DEFAULT_OPTIONS`) ruleDetails.options = optionsId === DEFAULT_OPTIONS_ID ? ruleDetails.defaultOptions : allOptions[optionsId]; diff --git a/apps/oxlint/src-js/plugins/options.ts b/apps/oxlint/src-js/plugins/options.ts index fdb70c038a14b..98827cdcd339f 100644 --- a/apps/oxlint/src-js/plugins/options.ts +++ b/apps/oxlint/src-js/plugins/options.ts @@ -22,8 +22,10 @@ export type Options = JsonValue[]; // Default rule options export const DEFAULT_OPTIONS: Readonly = Object.freeze([]); -// All rule options -export const allOptions: Readonly[] = [DEFAULT_OPTIONS]; +// All rule options. +// Indexed by options ID sent alongside rule ID for each file. +// Element 0 is always the default options (empty array). +export let allOptions: Readonly[] = [DEFAULT_OPTIONS]; // Index into `allOptions` for default options export const DEFAULT_OPTIONS_ID = 0; @@ -128,3 +130,27 @@ function mergeValues(configValue: JsonValue, defaultValue: JsonValue): JsonValue return freeze(merged); } + +/** + * Set all external rule options. + * Called once from Rust after config building, before any linting occurs. + * @param optionsJSON - Array of all rule options across all configurations, serialized as JSON + */ +export function setOptions(optionsJson: string): void { + allOptions = JSON.parse(optionsJson); + + // Validate that `allOptions` is an array of arrays + if (DEBUG) { + if (!isArray(allOptions)) { + throw new TypeError(`Expected optionsJson to decode to an array, got ${typeof allOptions}`); + } + for (const options of allOptions) { + if (!isArray(options)) { + throw new TypeError(`Each options entry must be an array, got ${typeof options}`); + } + } + } + + // `allOptions`' type is `Readonly`, but the array is mutable until after this call + deepFreezeArray(allOptions as JsonValue[]); +} diff --git a/apps/oxlint/src/js_plugins/external_linter.rs b/apps/oxlint/src/js_plugins/external_linter.rs index 59e6bd5c9f24d..cca811da4bf22 100644 --- a/apps/oxlint/src/js_plugins/external_linter.rs +++ b/apps/oxlint/src/js_plugins/external_linter.rs @@ -9,24 +9,26 @@ use serde::Deserialize; use oxc_allocator::{Allocator, free_fixed_size_allocator}; use oxc_linter::{ - ExternalLinter, ExternalLinterLintFileCb, ExternalLinterLoadPluginCb, LintFileResult, - PluginLoadResult, + ExternalLinter, ExternalLinterLintFileCb, ExternalLinterLoadPluginCb, + ExternalLinterSetupConfigsCb, LintFileResult, PluginLoadResult, }; use crate::{ generated::raw_transfer_constants::{BLOCK_ALIGN, BUFFER_SIZE}, - run::{JsLintFileCb, JsLoadPluginCb}, + run::{JsLintFileCb, JsLoadPluginCb, JsSetupConfigsCb}, }; /// Wrap JS callbacks as normal Rust functions, and create [`ExternalLinter`]. pub fn create_external_linter( load_plugin: JsLoadPluginCb, + setup_configs: JsSetupConfigsCb, lint_file: JsLintFileCb, ) -> ExternalLinter { let rust_load_plugin = wrap_load_plugin(load_plugin); + let rust_setup_configs = wrap_setup_configs(setup_configs); let rust_lint_file = wrap_lint_file(lint_file); - ExternalLinter::new(rust_load_plugin, rust_lint_file) + ExternalLinter::new(rust_load_plugin, rust_setup_configs, rust_lint_file) } /// Wrap `loadPlugin` JS callback as a normal Rust function. @@ -59,6 +61,38 @@ pub enum LintFileReturnValue { Failure(String), } +/// Wrap `setupConfigs` JS callback as a normal Rust function. +/// +/// The JS-side `setupConfigs` function is synchronous, but it's wrapped in a `ThreadsafeFunction`, +/// so cannot be called synchronously. Use an `mpsc::channel` to wait for the result from JS side, +/// and block current thread until `setupConfigs` completes execution. +fn wrap_setup_configs(cb: JsSetupConfigsCb) -> ExternalLinterSetupConfigsCb { + Box::new(move |options_json: String| { + let (tx, rx) = channel(); + + // Send data to JS + let status = cb.call_with_return_value( + options_json, + ThreadsafeFunctionCallMode::NonBlocking, + move |result, _env| { + let _ = match &result { + Ok(()) => tx.send(Ok(())), + Err(e) => tx.send(Err(e.to_string())), + }; + result + }, + ); + + assert!(status == Status::Ok, "Failed to schedule setupConfigs callback: {status:?}"); + + match rx.recv() { + Ok(Ok(())) => Ok(()), + Ok(Err(err)) => Err(err), + Err(err) => panic!("setupConfigs callback did not respond: {err}"), + } + }) +} + /// Wrap `lintFile` JS callback as a normal Rust function. /// /// The returned function creates a `Uint8Array` referencing the memory of the given `Allocator`, @@ -72,6 +106,7 @@ fn wrap_lint_file(cb: JsLintFileCb) -> ExternalLinterLintFileCb { Box::new( move |file_path: String, rule_ids: Vec, + options_ids: Vec, settings_json: String, allocator: &Allocator| { let (tx, rx) = channel(); @@ -87,7 +122,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_ids, options_ids, 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 07e13aa61e1ee..09a183d5eca1a 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,18 @@ impl CliRunner { return CliRunResult::None; } + // Send JS plugins config to JS side + if let Some(external_linter) = &external_linter { + let res = config_store.external_plugin_store().setup_configs(external_linter); + if let Err(err) = res { + print_and_flush_stdout( + stdout, + &format!("Failed to setup external plugin options: {err}\n"), + ); + return CliRunResult::InvalidOptionConfig; + } + } + 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 ff572e0f478d9..1fe8a2a858017 100644 --- a/apps/oxlint/src/run.rs +++ b/apps/oxlint/src/run.rs @@ -40,12 +40,28 @@ pub type JsLintFileCb = ThreadsafeFunction< u32, // Buffer ID Option, // Buffer (optional) Vec, // Array of rule IDs + Vec, // Array of options IDs String, // Stringified settings effective for the file )>, // Return value String, // `Vec`, serialized to JSON // Arguments (repeated) - FnArgs<(String, u32, Option, Vec, String)>, + FnArgs<(String, u32, Option, Vec, Vec, String)>, + // Error status + Status, + // CalleeHandled + false, +>; + +/// JS callback to setup configs. +#[napi] +pub type JsSetupConfigsCb = ThreadsafeFunction< + // Arguments + String, // Options array, as JSON string + // Return value + (), // `void` + // Arguments (repeated) + String, // Error status Status, // CalleeHandled @@ -57,20 +73,27 @@ pub type JsLintFileCb = ThreadsafeFunction< /// JS side passes in: /// 1. `args`: Command line arguments (process.argv.slice(2)) /// 2. `load_plugin`: Load a JS plugin from a file path. -/// 3. `lint_file`: Lint a file. +/// 3. `setup_configs`: Setup configuration options. +/// 4. `lint_file`: Lint a file. /// /// Returns `true` if linting succeeded without errors, `false` otherwise. #[expect(clippy::allow_attributes)] #[allow(clippy::trailing_empty_array, clippy::unused_async)] // https://github.com/napi-rs/napi-rs/issues/2758 #[napi] -pub async fn lint(args: Vec, load_plugin: JsLoadPluginCb, lint_file: JsLintFileCb) -> bool { - lint_impl(args, load_plugin, lint_file).await.report() == ExitCode::SUCCESS +pub async fn lint( + args: Vec, + load_plugin: JsLoadPluginCb, + setup_configs: JsSetupConfigsCb, + lint_file: JsLintFileCb, +) -> bool { + lint_impl(args, load_plugin, setup_configs, lint_file).await.report() == ExitCode::SUCCESS } /// Run the linter. async fn lint_impl( args: Vec, load_plugin: JsLoadPluginCb, + setup_configs: JsSetupConfigsCb, lint_file: JsLintFileCb, ) -> CliRunResult { // Convert String args to OsString for compatibility with bpaf @@ -104,10 +127,11 @@ async fn lint_impl( // JS plugins are only supported on 64-bit little-endian platforms at present #[cfg(all(target_pointer_width = "64", target_endian = "little"))] - let external_linter = Some(crate::js_plugins::create_external_linter(load_plugin, lint_file)); + let external_linter = + Some(crate::js_plugins::create_external_linter(load_plugin, setup_configs, lint_file)); #[cfg(not(all(target_pointer_width = "64", target_endian = "little")))] let external_linter = { - let (_, _) = (load_plugin, lint_file); + let (_, _, _) = (load_plugin, setup_configs, lint_file); None }; diff --git a/apps/oxlint/test/fixtures/options/.oxlintrc.json b/apps/oxlint/test/fixtures/options/.oxlintrc.json index dfbce6c0cf2b1..6fedc5250e103 100644 --- a/apps/oxlint/test/fixtures/options/.oxlintrc.json +++ b/apps/oxlint/test/fixtures/options/.oxlintrc.json @@ -4,7 +4,18 @@ "correctness": "off" }, "rules": { - "options-plugin/options": "error", + "options-plugin/no-options": "error", + "options-plugin/options": [ + "error", + false, + { + "array": [ + {"deep": true, "str": "hello"}, + 456 + ], + "not": null + } + ], "options-plugin/default-options": "error" } } diff --git a/apps/oxlint/test/fixtures/options/output.snap.md b/apps/oxlint/test/fixtures/options/output.snap.md index 1986f2f332bac..c47ffc3bd89ec 100644 --- a/apps/oxlint/test/fixtures/options/output.snap.md +++ b/apps/oxlint/test/fixtures/options/output.snap.md @@ -34,7 +34,7 @@ : ^ `---- - x options-plugin(options): + x options-plugin(no-options): | options: [] | isDeepFrozen: true ,-[files/index.js:1:1] @@ -42,7 +42,27 @@ : ^ `---- -Found 0 warnings and 2 errors. + x options-plugin(options): + | options: [ + | false, + | { + | "array": [ + | { + | "deep": true, + | "str": "hello" + | }, + | 456 + | ], + | "not": null + | } + | ] + | isDeepFrozen: true + ,-[files/index.js:1:1] + 1 | debugger; + : ^ + `---- + +Found 0 warnings and 3 errors. Finished in Xms on 1 file using X threads. ``` diff --git a/apps/oxlint/test/fixtures/options/plugin.ts b/apps/oxlint/test/fixtures/options/plugin.ts index f7688f0ac88f6..7f83cae63fb39 100644 --- a/apps/oxlint/test/fixtures/options/plugin.ts +++ b/apps/oxlint/test/fixtures/options/plugin.ts @@ -15,6 +15,17 @@ const plugin: Plugin = { name: "options-plugin", }, rules: { + "no-options": { + create(context) { + context.report({ + message: + `\noptions: ${JSON.stringify(context.options, null, 2)}\n` + + `isDeepFrozen: ${isDeepFrozen(context.options)}`, + node: SPAN, + }); + return {}; + }, + }, options: { create(context) { context.report({ diff --git a/crates/oxc_language_server/src/linter/server_linter.rs b/crates/oxc_language_server/src/linter/server_linter.rs index 6f0628bc3a02f..822a455b9b3da 100644 --- a/crates/oxc_language_server/src/linter/server_linter.rs +++ b/crates/oxc_language_server/src/linter/server_linter.rs @@ -90,9 +90,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 { @@ -249,10 +249,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_linter/src/config/config_builder.rs b/crates/oxc_linter/src/config/config_builder.rs index 88200ebacbcfc..b7765d2babdb4 100644 --- a/crates/oxc_linter/src/config/config_builder.rs +++ b/crates/oxc_linter/src/config/config_builder.rs @@ -17,7 +17,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, }; @@ -30,7 +30,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, @@ -386,7 +386,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 @@ -413,8 +413,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)) } @@ -422,7 +428,7 @@ impl ConfigStoreBuilder { fn resolve_overrides( &self, overrides: OxlintOverrides, - external_plugin_store: &ExternalPluginStore, + external_plugin_store: &mut ExternalPluginStore, ) -> Result { let resolved = overrides .into_iter() @@ -444,7 +450,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, @@ -814,10 +824,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(); @@ -1262,7 +1272,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")); @@ -1288,7 +1298,7 @@ mod test { &mut external_plugin_store, ) .unwrap() - .build(&external_plugin_store) + .build(&mut external_plugin_store) .unwrap() } @@ -1301,7 +1311,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 0b94ad4944221..912c623af0689 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)]>, } #[derive(Debug, Default, Clone)] @@ -55,7 +55,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)] @@ -80,7 +80,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, @@ -97,7 +97,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() }), }, @@ -174,8 +174,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 = 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. @@ -219,8 +224,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 { @@ -253,7 +258,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 { @@ -345,6 +351,10 @@ impl ConfigStore { ) -> (/* plugin name */ &str, /* rule name */ &str) { self.external_plugin_store.resolve_plugin_rule_names(external_rule_id) } + + pub fn external_plugin_store(&self) -> &ExternalPluginStore { + &self.external_plugin_store + } } #[cfg(test)] @@ -356,7 +366,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, @@ -707,7 +717,7 @@ mod test { let store = ConfigStore::new( Config::new( vec![], - vec![(rule_id, AllowWarnDeny::Deny)], + vec![(rule_id, ExternalOptionsId::NONE, AllowWarnDeny::Deny)], OxlintCategories::default(), LintConfig::default(), overrides, @@ -1052,4 +1062,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( + PathBuf::from("path/to/custom"), + "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 (rule_id, options_id, severity) = resolved.external_rules[0]; + assert_eq!(rule_id, base_external_rule_id); + assert_eq!(severity, AllowWarnDeny::Deny); + // `options_id` should not equal the base options ID + assert_ne!(options_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..35eceb10ba678 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::NONE + }; + 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)); } } } @@ -325,12 +340,15 @@ impl ESLintRule { #[cfg(test)] #[expect(clippy::default_trait_access)] mod test { + use std::path::PathBuf; + use rustc_hash::FxHashMap; use serde::Deserialize; use serde_json::{Value, json}; use crate::{ AllowWarnDeny, ExternalPluginStore, + external_plugin_store::ExternalOptionsId, rules::{RULES, RuleEnum}, }; @@ -381,9 +399,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 +540,50 @@ 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( + PathBuf::from("path/to/custom-plugin"), + "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, &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().unwrap(); + assert_ne!( + options_id, + ExternalOptionsId::NONE, + "non-empty options should allocate a new id" + ); + assert_eq!(severity, AllowWarnDeny::Warn); + + // 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_rules = RuleSet::default(); + let mut external_rules = FxHashMap::default(); + rules_no_opts + .override_rules(&mut builtin_rules, &mut external_rules, &RULES, &mut store) + .unwrap(); + let (_rule_id, &(options_id, severity)) = external_rules.iter().next().unwrap(); + assert_eq!(options_id, ExternalOptionsId::NONE, "no options should use reserved id 0"); + assert_eq!(severity, AllowWarnDeny::Deny); + } } diff --git a/crates/oxc_linter/src/external_linter.rs b/crates/oxc_linter/src/external_linter.rs index d9f3a213444fe..5d2c4b31ef9b2 100644 --- a/crates/oxc_linter/src/external_linter.rs +++ b/crates/oxc_linter/src/external_linter.rs @@ -10,8 +10,10 @@ pub type ExternalLinterLoadPluginCb = Box< + Sync, >; +pub type ExternalLinterSetupConfigsCb = Box Result<(), String> + Send + Sync>; + pub type ExternalLinterLintFileCb = Box< - dyn Fn(String, Vec, String, &Allocator) -> Result, String> + dyn Fn(String, Vec, Vec, String, &Allocator) -> Result, String> + Sync + Send, >; @@ -46,15 +48,17 @@ pub struct JsFix { pub struct ExternalLinter { pub(crate) load_plugin: ExternalLinterLoadPluginCb, + pub(crate) setup_configs: ExternalLinterSetupConfigsCb, pub(crate) lint_file: ExternalLinterLintFileCb, } impl ExternalLinter { pub fn new( load_plugin: ExternalLinterLoadPluginCb, + setup_configs: ExternalLinterSetupConfigsCb, lint_file: ExternalLinterLintFileCb, ) -> Self { - Self { load_plugin, lint_file } + Self { load_plugin, setup_configs, lint_file } } } diff --git a/crates/oxc_linter/src/external_plugin_store.rs b/crates/oxc_linter/src/external_plugin_store.rs index 6b57bb0792136..921626cf701e3 100644 --- a/crates/oxc_linter/src/external_plugin_store.rs +++ b/crates/oxc_linter/src/external_plugin_store.rs @@ -5,7 +5,9 @@ use std::{ use rustc_hash::{FxHashMap, FxHashSet}; -use oxc_index::{IndexVec, define_index_type}; +use oxc_index::{IndexVec, define_index_type, index_vec}; + +use crate::ExternalLinter; define_index_type! { pub struct ExternalPluginId = u32; @@ -15,6 +17,16 @@ define_index_type! { pub struct ExternalRuleId = u32; } +define_index_type! { + pub struct ExternalOptionsId = u32; +} + +impl ExternalOptionsId { + /// The value `0`. + /// Used as the ID when a rule does not have options. + pub const NONE: Self = Self::from_usize(0); +} + #[derive(Debug)] pub struct ExternalPluginStore { registered_plugin_paths: FxHashSet, @@ -22,6 +34,7 @@ pub struct ExternalPluginStore { plugins: IndexVec, plugin_names: FxHashMap, rules: IndexVec, + options: IndexVec, // `true` for `oxlint`, `false` for language server is_enabled: bool, @@ -35,11 +48,14 @@ impl Default for ExternalPluginStore { impl ExternalPluginStore { pub fn new(is_enabled: bool) -> Self { + let options = index_vec![serde_json::json!([])]; + Self { registered_plugin_paths: FxHashSet::default(), plugins: IndexVec::default(), plugin_names: FxHashMap::default(), rules: IndexVec::default(), + options, is_enabled, } } @@ -119,6 +135,33 @@ impl ExternalPluginStore { let plugin = &self.plugins[external_rule.plugin_id]; (&plugin.name, &external_rule.name) } + + /// Add options to the store and return its [`ExternalOptionsId`]. + /// + /// `options` must be a `serde_json::Value::Array`. + /// + /// # Panics + /// Panics in debug build if `options` is not an array or is an empty array. + pub fn add_options(&mut self, options: serde_json::Value) -> ExternalOptionsId { + debug_assert!(options.is_array(), "`options` should be an array"); + debug_assert!( + !options.as_array().unwrap().is_empty(), + "`options` should never be an empty `Vec`" + ); + self.options.push(options) + } + + /// Send options to JS side. + /// + /// # Errors + /// Returns an error if serialization of rule options fails. + pub fn setup_configs(&self, external_linter: &ExternalLinter) -> Result<(), String> { + let json = serde_json::to_string(&self.options); + match json { + Ok(options_json) => (external_linter.setup_configs)(options_json), + Err(err) => Err(format!("Failed to serialize external plugin options: {err}")), + } + } } #[derive(Debug, Clone, PartialEq, Eq)] diff --git a/crates/oxc_linter/src/lib.rs b/crates/oxc_linter/src/lib.rs index de5dc48e458ff..613d29a2f2d76 100644 --- a/crates/oxc_linter/src/lib.rs +++ b/crates/oxc_linter/src/lib.rs @@ -60,10 +60,10 @@ pub use crate::{ }, context::{ContextSubHost, LintContext}, external_linter::{ - ExternalLinter, ExternalLinterLintFileCb, ExternalLinterLoadPluginCb, JsFix, - LintFileResult, PluginLoadResult, + ExternalLinter, ExternalLinterLintFileCb, ExternalLinterLoadPluginCb, + ExternalLinterSetupConfigsCb, 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,11 @@ impl Linter { None => "{}".to_string(), }; + // Pass AST and rule IDs + options IDs to JS let result = (external_linter.lint_file)( path.to_string_lossy().into_owned(), - external_rules.iter().map(|(rule_id, _)| rule_id.raw()).collect(), + external_rules.iter().map(|(rule_id, _, _)| rule_id.raw()).collect(), + external_rules.iter().map(|(_, options_id, _)| options_id.raw()).collect(), settings_json, allocator, ); @@ -477,7 +479,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 759fc521c01ed..4a65830a32baf 100644 --- a/crates/oxc_linter/src/tester.rs +++ b/crates/oxc_linter/src/tester.rs @@ -505,7 +505,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 d5e1e56b98467..56b028355f085 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),