Options IDs are stored in their own arrays. This was laid out inrefactor(linter/plugins): prepare for rule options #16158. The original approach was to storeruleId-optionId pairs as tuples in an array - several heap allocated objects and excessive pointer indirection.
The rust code populates the options by calling a new JS callback namedsetupConfigs(). I expect we would use this callback for more plugin related data, including settings which are currently implemented in a hacky way.
Investigate a refactor to avoid requiring a mutable reference toExternalPluginStore everywhere.

Smaller Changes

Removed the extra fixture ("working version"). There were no meaningful changes compared to the first fixture. Probably slop.

CopilotAI review requested due to automatic review settings

November 27, 2025 20:58

lilnasy requested review fromSysix,camc314 andoverlookmotel ascode owners

November 27, 2025 20:58

lilnasy marked this pull request as draft

November 27, 2025 20:58

github-actionsbot added A-linter

Area - Linter

A-cli

Area - CLI

A-editor

Area - Editor and Language Server

A-linter-plugins

Area - Linter JS plugins

C-enhancementCategory - New feature or request labels

Nov 27, 2025

Copilotstarted reviewing on behalf oflilnasy

November 27, 2025 20:58

View session

Copilot finished reviewing on behalf oflilnasy

November 27, 2025 21:00

CopilotAI reviewed

Nov 27, 2025

View reviewed changes

Copy link

Contributor

CopilotAI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Pull request overview

This PR implements rule options support for external JavaScript plugins in the linter. It introduces anExternalOptionsId type to track and pass rule-specific configuration options from Rust to JavaScript plugins, allowing JS plugin rules to receive configuration parameters similar to ESLint rule options.

Key changes:

AddedExternalOptionsId type and options storage toExternalPluginStore with index 0 reserved for "no options"
Updated external rule signatures from(ExternalRuleId, AllowWarnDeny) to(ExternalRuleId, ExternalOptionsId, AllowWarnDeny) throughout the codebase
Implemented options serialization/deserialization flow between Rust and JavaScript usingOnceLock for thread-safe caching

Reviewed changes

Copilot reviewed 23 out of 24 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
`crates/oxc_linter/src/external_plugin_store.rs`	Added`ExternalOptionsId` type and options storage with index 0 reserved for empty/null options
`crates/oxc_linter/src/external_linter.rs`	Updated callback signature to include`options_ids` parameter
`crates/oxc_linter/src/lib.rs`	Updated external rule handling to pass options IDs to JS side
`crates/oxc_linter/src/config/rules.rs`	Modified`override_rules` to track options alongside severity; added comprehensive tests
`crates/oxc_linter/src/config/config_store.rs`	Updated rule storage and override logic to handle options IDs; added override precedence test
`crates/oxc_linter/src/config/config_builder.rs`	Updated builder to accept mutable reference to external plugin store for options registration
`crates/oxc_linter/src/config/mod.rs`	Updated test to use mutable external plugin store
`crates/oxc_linter/src/tester.rs`	Updated to pass mutable reference to external plugin store
`crates/oxc_language_server/src/linter/server_linter.rs`	Updated config building calls to use mutable external plugin store
`tasks/benchmark/benches/linter.rs`	Updated benchmark to use mutable external plugin store
`napi/playground/src/lib.rs`	Updated playground to properly share external plugin store between config building and linter creation
`apps/oxlint/src/run.rs`	Added`OnceLock`-based caching for serialized options JSON and export function for JS
`apps/oxlint/src/lint.rs`	Added serialization of external options after config building
`apps/oxlint/src/js_plugins/external_linter.rs`	Updated wrapper to forward options IDs to JS callback
`apps/oxlint/src-js/plugins/options.ts`	Added options initialization logic and storage
`apps/oxlint/src-js/plugins/load.ts`	Duplicated options initialization logic (should be removed)
`apps/oxlint/src-js/plugins/lint.ts`	Added lazy options initialization and options ID handling; duplicate`ruleIndex` assignment
`apps/oxlint/src-js/cli.ts`	Updated wrapper to forward options IDs parameter
`apps/oxlint/src-js/bindings.js`	Exported`getExternalRuleOptions` function
`apps/oxlint/src-js/bindings.d.ts`	Added TypeScript declaration for`getExternalRuleOptions`
`apps/oxlint/test/fixtures/custom_plugin_with_options/*`	Added integration test fixture for external plugin with options

💡Add Copilot custom instructions for smarter, more guided reviews.Learn how to get started.

apps/oxlint/src-js/plugins/load.ts OutdatedShow resolvedHide resolved

apps/oxlint/src-js/plugins/lint.ts OutdatedShow resolvedHide resolved

apps/oxlint/src/run.rs Outdated

Comment on lines 35 to 40

		/// 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]
		pubfnget_external_rule_options() ->Option<String>{
		EXTERNAL_OPTIONS_JSON.get().cloned()

Copy link

CopilotAINov 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

The documentation comment states "Returns a JSON string of options arrays" but the function returnsOption<String>, which can beNone. The comment should clarify whenNone is returned (e.g., "Returns a JSON string of options arrays, orNone if options haven't been set yet").

Copilot uses AI. Check for mistakes.

Copy link

codspeed-hqbot commentedNov 27, 2025•
edited
Loading

CodSpeed Performance Report

Merging#16215 willnot alter performance

_{Comparinglilnasy:js-plugin-options (42eee5d) withmain (505ceb1)}

Summary

✅ 42 untouched
⏩ 3 skipped¹

Footnotes

3 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase,click here and archive them to remove them from the performance reports.↩

lilnasy force-pushed thejs-plugin-options branch 2 times, most recently froma7a3ca1 to9c2d8a4Compare

November 27, 2025 21:11

Copy link

Member

overlookmotel commentedNov 27, 2025

Just a note: As I mentioned on Discord, I don't think we should worry about merging options with default options in this first PR - can leave that to a follow-on. But FYI I've implemented the merging logic in#16217 because I needed it for the rule tester.

lilnasy force-pushed thejs-plugin-options branch 2 times, most recently fromd62762c to3c46139Compare

November 28, 2025 13:01

Copy link

Member

overlookmotel commentedNov 28, 2025•
edited
Loading

Refactor to avoid requiring a mutable reference toExternalPluginStore everywhere.

I know it was me that said this should be possible, but I could very well be wrong. If it's not possible, don't worry about it.

wagenetand others added4 commits

November 29, 2025 00:38

Add support for options for jsPlugins

4c19782

lint issues

a3f1ba5

deleteset_external_options_json()

839d6af

deletegetExternalRuleOptions()

26513bb

lilnasy force-pushed thejs-plugin-options branch from3c46139 toae52e34Compare

November 28, 2025 19:08

lilnasy commented

Nov 28, 2025

View reviewed changes

apps/oxlint/src/lint.rs

Comment on lines 294 to 304

		// TODO: refactor this elsewhere.
		// This code is in the oxlint app, not in oxc_linter crate
		ifletSome(ref external_linter) = external_linter{
		ifletErr(err) = external_plugin_store.setup_configs(external_linter){
		print_and_flush_stdout(
		stdout,
		&format!("Failed to setup external plugin options: {err}\n"),
		);
		returnCliRunResult::InvalidOptionConfig;
		}
		}

Copy link

ContributorAuthor

lilnasyNov 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

I could use help here. I exposedsetup_configs() for theoxlint crate by putting it onExternalPluginStore only because it's the first place I found. I could do with a few pointers on separation of concerns regarding these structs.

Copy link

Member

overlookmotelNov 29, 2025•
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Yeah, the types are a bit of a convoluted mess here. I'm tied up today. Will take a look tomorrow.

lilnasy force-pushed thejs-plugin-options branch fromae52e34 to3e53d55Compare

November 28, 2025 19:20

introducesetupConfigs()

1b4cd57

lilnasy force-pushed thejs-plugin-options branch fromdfb4439 to1b4cd57Compare

November 28, 2025 19:21

createOnce() ->create()

a20747d

lilnasy added3 commits

November 29, 2025 01:33

refactor: initoptions usingindex_vec!

9701596

deep freeze the options array parsing

f56f662

comment: explainruleDetails.options assignment

ad6d9fd

lilnasy marked this pull request as ready for review

November 28, 2025 20:25

lilnasy requested a review fromCopilot

November 28, 2025 20:25

Copilotstarted reviewing on behalf oflilnasy

November 28, 2025 20:26

View session

Copilot finished reviewing on behalf oflilnasy

November 28, 2025 20:28

CopilotAI reviewed

Nov 28, 2025

View reviewed changes

Copy link

Contributor

CopilotAI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Pull request overview

Copilot reviewed 23 out of 24 changed files in this pull request and generated 2 comments.

💡Add Copilot custom instructions for smarter, more guided reviews.Learn how to get started.

apps/oxlint/src/js_plugins/external_linter.rs OutdatedShow resolvedHide resolved

apps/oxlint/src-js/plugins/config.ts

Comment on lines +4 to +14

		* Populates Rust-resolved configuration options on the JS side.
		* Called from Rust side after all configuration options have been resolved.
		*
		* Note: the name `setupConfigs` is currently incorrect, as we only populate rule options.
		* The intention is for this function to transfer all configurations in a multi-config workspace.
		* The configuration relevant to each file would then be resolved on the JS side.
		*
		*@param optionsJSON - JSON serialization of an array containing all rule options across all configurations.
		*@returns "ok" on success, or error message on failure
		*/
		exportfunctionsetupConfigs(optionsJSON:string):string{

Copy link

CopilotAINov 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

[nitpick] The comment states "Note: the namesetupConfigs is currently incorrect, as we only populate rule options." While this is acknowledged as a TODO, consider renaming tosetupOptions orsetupRuleOptions to accurately reflect the current functionality. This would make the code clearer until multi-config workspace support is implemented.

Suggested change

	*PopulatesRust-resolvedconfigurationoptionsontheJSside.
	*CalledfromRustsideafterallconfigurationoptionshavebeenresolved.
	*
	*Note:thename`setupConfigs`iscurrentlyincorrect,asweonlypopulateruleoptions.
	*Theintentionisforthisfunctiontotransferallconfigurationsinamulti-configworkspace.
	*TheconfigurationrelevanttoeachfilewouldthenberesolvedontheJSside.
	*
	* @paramoptionsJSON-JSONserializationofanarraycontainingallruleoptionsacrossallconfigurations.
	* @returns"ok"onsuccess,orerrormessageonfailure
	*/
	exportfunctionsetupConfigs(optionsJSON:string):string{
	*PopulatesRust-resolvedruleoptionsontheJSside.
	*CalledfromRustsideafterallruleoptionshavebeenresolved.
	*
	*Theintentionisforthisfunctiontoeventuallytransferallconfigurationsinamulti-configworkspace.
	*TheconfigurationrelevanttoeachfilewouldthenberesolvedontheJSside.
	*
	* @paramoptionsJSON-JSONserializationofanarraycontainingallruleoptionsacrossallconfigurations.
	* @returns"ok"onsuccess,orerrormessageonfailure
	*/
	exportfunctionsetupOptions(optionsJSON:string):string{

Copilot uses AI. Check for mistakes.

lilnasy added3 commits

November 29, 2025 02:10

update copy-pasted comment

88f5f37

simplify test

4e03bc9

remove unused method

42eee5d

lilnasy commented

Nov 28, 2025

View reviewed changes

apps/oxlint/src-js/plugins/lint.ts

Comment on lines +158 to 162


		// 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];

Copy link

ContributorAuthor

lilnasyNov 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

@overlookmotel You mentioned this could be simplified toallOptions[optionsId]. How would plugin-provided default options get used in that case?

apps/oxlint/src-js/plugins/options.ts

		constel=parsed[i];
		if(!isArray(el))thrownewTypeError("Each options entry must be an array",{cause:el});
		}
		}

Copy link

ContributorAuthor

lilnasyNov 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

This is a debug check because I this validation is done probably done on the rust side as well. I need to confirm this.

Labels

A-cli

Area - CLI

A-editor

Area - Editor and Language Server

A-linter

Area - Linter

A-linter-plugins

Area - Linter JS plugins

C-enhancement

Category - New feature or request

Movatterモバイル変換

Uh oh!

feat(linter/plugins): rule options#16215

Are you sure you want to change the base?

feat(linter/plugins): rule options#16215

Conversation

lilnasy commentedNov 27, 2025• editedLoading Uh oh!There was an error while loading.Please reload this page.

Uh oh!

Big changes from the original approach

Smaller Changes

Uh oh!

CopilotAI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

CopilotAINov 27, 2025

Choose a reason for hiding this comment

Uh oh!

codspeed-hqbot commentedNov 27, 2025• editedLoading Uh oh!There was an error while loading.Please reload this page.

Uh oh!

CodSpeed Performance Report

Merging#16215 willnot alter performance

Summary

Footnotes

Uh oh!

overlookmotel commentedNov 27, 2025

Uh oh!

overlookmotel commentedNov 28, 2025• editedLoading Uh oh!There was an error while loading.Please reload this page.

Uh oh!

Uh oh!

lilnasyNov 28, 2025

Choose a reason for hiding this comment

Uh oh!

overlookmotelNov 29, 2025• editedLoading Uh oh!There was an error while loading.Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

CopilotAI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

CopilotAINov 28, 2025

Choose a reason for hiding this comment

Uh oh!

lilnasyNov 28, 2025

Choose a reason for hiding this comment

Uh oh!

lilnasyNov 28, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

lilnasy commentedNov 27, 2025•
edited
Loading

codspeed-hqbot commentedNov 27, 2025•
edited
Loading

overlookmotel commentedNov 28, 2025•
edited
Loading

overlookmotelNov 29, 2025•
edited
Loading