feat(linter): Add support for options for jsPlugins by wagenet · Pull Request #15822 · oxc-project/oxc

wagenet · 2025-11-18T00:31:50Z

graphite-app · 2025-11-18T00:31:56Z

How to use the Graphite Merge Queue

Add either label to this PR to merge it via the merge queue:

0-merge - adds this PR to the back of the merge queue
hotfix - for urgent hot fixes, skip the queue and merge this PR next

You must have a Graphite account in order to use the merge queue. Sign up using this link.

_{An organization admin has enabled the Graphite Merge Queue in this repository.} _{Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue.}

codspeed-hq · 2025-11-18T00:38:57Z

CodSpeed Performance Report

Merging #15822 will not alter performance

_{Comparing wagenet:js-plugin-options (6bb6c2b) with main (4fe3aac)}

Summary

✅ 42 untouched
⏩ 3 skipped¹

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. ↩

Copilot

Pull Request Overview

This PR adds support for passing options to JavaScript plugin rules, enabling external linting rules to receive configuration parameters from .oxlintrc.json files.

Key changes:

Introduces ExternalOptionsId to track rule options alongside rule IDs
Modifies the external linter interface to pass (ruleId, optionsId) tuples instead of just rule IDs
Adds serialization of options from Rust to JavaScript via a global JSON string

Reviewed Changes

Copilot reviewed 29 out of 30 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
crates/oxc_linter/src/external_plugin_store.rs	Adds `ExternalOptionsId` type and options storage/serialization methods
crates/oxc_linter/src/config/rules.rs	Updates rule override logic to store and pass options IDs
crates/oxc_linter/src/config/config_store.rs	Updates configuration structures to include options IDs with external rules
crates/oxc_linter/src/config/config_builder.rs	Modifies builder to handle options IDs and mutable plugin store
crates/oxc_linter/src/lib.rs	Updates external rule execution to pass options IDs
crates/oxc_linter/src/external_linter.rs	Changes callback signature to accept rule/options ID pairs
apps/oxlint/src/run.rs	Adds global storage and NAPI functions for options JSON
apps/oxlint/src/lint.rs	Serializes options after config building
apps/oxlint/src-js/plugins/load.ts	Adds options storage and initialization functions
apps/oxlint/src-js/plugins/lint.ts	Updates linting logic to fetch and apply options per rule
Test files	Adds comprehensive test coverage for options functionality

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

crates/oxc_linter/src/config/config_store.rs

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

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

apps/oxlint/src/run.rs

Sysix · 2025-11-18T11:54:08Z

crates/oxc_language_server/src/linter/server_linter_js_plugins_test.rs

+    use super::super::server_linter::ServerLinterBuilder;
+
+    #[test]
+    fn test_new_with_js_plugins_in_config_does_not_crash() {


This is not really testing anything, only if the ServerLinterBuilder::build did not panic.
jsPlugins for the language server is not implemented, but a milestone goal: #14826.

Can you remove this file?

overlookmotel

Thanks very much for working on this. This'll be a big ✅ off our list of requirements for JS plugins getting to alpha status.

Comments below are mostly small things.

My broader, more substantive points are:

Push not pull

Currently there's a rather convoluted method of getting options from Rust to JS. The options get stored in a static, then every time JS lints a file it asks "do I have the options yet?", and if not then JS calls back into Rust to get the options back out of that static.

I think this could be simplified considerably by using a "push" model instead of "pull".

After all configs have been parsed, if there are any JS plugins registered, send the options JSON over to JS. Done.

In practice, this means getting rid of getExternalRuleOptions, and instead passing a 3rd callback into Rust at the start (cli.ts):

- const success = await lint(args, loadPluginWrapper, lintFileWrapper);
+ const success = await lint(args, loadPluginWrapper, setupWrapper, lintFileWrapper);

Rust would call setup and pass the options JSON before it starts linting any files.

`ConfigStoreBuilder::build`

Much of the diff in this PR is just due to ConfigStoreBuilder::build now taking a &mut ExternalPluginStore instead of &ExternalPluginStore.

I have a feeling that this shouldn't be required. But I find the config logic so labyrinthine that I can't put my finger on what'd need to change to enable that (or whether my intuition is incorrect).

So let's leave that one for now, address the other stuff first, and come back to it.

CI failure

The snapshot differences are due to an annoyance with character encoding. If you run cd apps/oxlint; pnpm run build-test; (build-test not build), that should fix them - and also fix bindings.d.ts.

overlookmotel · 2025-11-18T14:06:48Z

apps/oxlint/src-js/plugins/load.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[][] = [];


Suggested change

export let allOptions: unknown[][] = [];

export let allOptions: Options[] = [];

Then can get rid of an as Options in lint.ts.

overlookmotel · 2025-11-18T14:14:17Z

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


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


Replace with:

Suggested change

const [ruleId, optionsId] = ruleData[i];

const ruleId = ruleData[0],

optionsId = ruleData[1];

I know the original is more idiomatic, but in apps/oxlint we prioritize perf over readability! Array destructuring is much more expensive for the JS engine.

overlookmotel · 2025-11-18T14:17:22Z

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

+    const options = allOptions[optionsId];
    ruleDetails.ruleIndex = i;
+    ruleDetails.options = options === undefined || !Array.isArray(options) ? [] : (options as Options);


We should be able to compress this to:

ruleDetails.ruleIndex = i; ruleDetails.options = allOptions[optionsId];

We need to make sure allOptions[0] is always an empty array. Rust already passes 0 as optionsId when there are no options for the rule.

Also, we can get rid of DEFAULT_OPTIONS in load.ts, and initialize ruleDetails.options as null when the RuleDetails is originally created. ruleDetails.options is now set before running the rule on each file, so this default is always overwritten.

overlookmotel · 2025-11-18T14:31:46Z

apps/oxlint/test/fixtures/custom_plugin_options_working/plugin.js

+          noOptions: 'No options provided',
+        },
+      },
+      createOnce(context) {


Please use create (not createOnce) in tests, for consistency.

This will also simplify the implementation - can do the check for options in body of create method, instead of in before.

overlookmotel · 2025-11-18T14:33:25Z

apps/oxlint/test/fixtures/custom_plugin_options_working/plugin.js

+        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,
+              });
+            }
+          },
+        };


Can greatly simplify all this. Just unconditionally report JSON.stringify(context.options). Then eyeball the snapshot to make sure it looks right.

crates/oxc_linter/src/config/config_store.rs

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

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

apps/oxlint/src/run.rs

overlookmotel · 2025-11-18T15:17:47Z

crates/oxc_linter/src/external_plugin_store.rs

+        let mut options = IndexVec::default();
+        // Index 0 is reserved for "no options" (empty array)
+        options.push(serde_json::json!([]));


Nit:

Suggested change

let mut options = IndexVec::default();

// Index 0 is reserved for "no options" (empty array)

options.push(serde_json::json!([]));

// Index 0 is reserved for "no options" (empty array)

let options = index_vec![serde_json::json!([])];

(import index_vec macro from oxc_index crate)

overlookmotel · 2025-11-24T00:25:15Z

Hi @wagenet. Not intended as hassling you, just a request for information: Do you think you're going to have any time to address the comments above? If not, that's totally fine, just it'd be useful to know so I can set aside time to finish this up, if you need to hand it over. I'm keen to get it merged ideally within the next week.

overlookmotel · 2025-11-27T00:03:42Z

Hi @wagenet. Have not heard from you in a few days (no problem, I imagine you're busy). @lilnasy has volunteered to take over the PR and finish it off. Hope that's Ok with you.

@lilnasy I'm not familiar with the language server, so I don't know how to evaluate the changes there, or @Sysix's comment above. But if you'd be able to address my feedback above, that'd be amazing.

- Implements #14825 - Rebased #15822. Refactored. Pruned the slop. Tests passing. ### Big changes from the original approach - [x] Options IDs are stored in their own arrays. This was laid out in #16158. The original approach was to store `ruleId`-`optionId` pairs as tuples in an array - several heap allocated objects and excessive pointer indirection. - [x] The rust code populates the options by calling a new JS callback named `setupConfigs()`. 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 to `ExternalPluginStore` everywhere. <details> <summary> #### Smaller Changes </summary> - Removed the extra fixture ("working version"). There were no meaningful changes compared to the first fixture. Probably slop. </details> --------- Co-authored-by: Peter Wagenet <peter@wagenet.us> Co-authored-by: overlookmotel <theoverlookmotel@gmail.com>

overlookmotel · 2025-12-01T12:29:18Z

#16215 built on this and completed it.

- Implements oxc-project#14825 - Rebased oxc-project#15822. Refactored. Pruned the slop. Tests passing. ### Big changes from the original approach - [x] Options IDs are stored in their own arrays. This was laid out in oxc-project#16158. The original approach was to store `ruleId`-`optionId` pairs as tuples in an array - several heap allocated objects and excessive pointer indirection. - [x] The rust code populates the options by calling a new JS callback named `setupConfigs()`. 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 to `ExternalPluginStore` everywhere. <details> <summary> #### Smaller Changes </summary> - Removed the extra fixture ("working version"). There were no meaningful changes compared to the first fixture. Probably slop. </details> --------- Co-authored-by: Peter Wagenet <peter@wagenet.us> Co-authored-by: overlookmotel <theoverlookmotel@gmail.com>

github-actions bot added A-linter Area - Linter A-cli Area - CLI A-editor Area - Editor and Language Server A-linter-plugins Area - Linter JS plugins labels Nov 18, 2025

wagenet force-pushed the js-plugin-options branch 4 times, most recently from f483727 to c96b043 Compare November 18, 2025 04:12

Add support for options for jsPlugins

6bb6c2b

wagenet changed the title ~~Add support for options for jsPlugins~~ feat(linter) Add support for options for jsPlugins Nov 18, 2025

wagenet force-pushed the js-plugin-options branch from c96b043 to 6bb6c2b Compare November 18, 2025 04:31

wagenet marked this pull request as ready for review November 18, 2025 04:31

wagenet requested review from Sysix and camc314 as code owners November 18, 2025 04:31

Copilot AI review requested due to automatic review settings November 18, 2025 04:31

wagenet changed the title ~~feat(linter) Add support for options for jsPlugins~~ feat(linter): Add support for options for jsPlugins Nov 18, 2025

Copilot AI reviewed Nov 18, 2025

View reviewed changes

crates/oxc_linter/src/config/config_store.rs Show resolved Hide resolved

apps/oxlint/src-js/plugins/load.ts Show resolved Hide resolved

apps/oxlint/src-js/plugins/lint.ts Show resolved Hide resolved

apps/oxlint/src/run.rs Show resolved Hide resolved

github-actions bot added the C-enhancement Category - New feature or request label Nov 18, 2025

Sysix reviewed Nov 18, 2025

View reviewed changes

overlookmotel reviewed Nov 18, 2025

View reviewed changes

overlookmotel marked this pull request as draft November 27, 2025 00:05

lilnasy mentioned this pull request Nov 27, 2025

feat(linter/plugins): rule options #16215

Merged

3 tasks

overlookmotel closed this Dec 1, 2025

	export let allOptions: unknown[][] = [];
	export let allOptions: Options[] = [];

	const [ruleId, optionsId] = ruleData[i];
	const ruleId = ruleData[0],
	optionsId = ruleData[1];

Uh oh!

Comments

Conversation

wagenet commented Nov 18, 2025

Uh oh!

graphite-app bot commented Nov 18, 2025

How to use the Graphite Merge Queue

Uh oh!

codspeed-hq bot commented Nov 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

CodSpeed Performance Report

Merging #15822 will not alter performance

Summary

Footnotes

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Sysix Nov 18, 2025

Choose a reason for hiding this comment

Uh oh!

overlookmotel left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Push not pull

ConfigStoreBuilder::build

CI failure

Uh oh!

overlookmotel Nov 18, 2025

Choose a reason for hiding this comment

Uh oh!

overlookmotel Nov 18, 2025

Choose a reason for hiding this comment

Uh oh!

overlookmotel Nov 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

overlookmotel Nov 18, 2025

Choose a reason for hiding this comment

Uh oh!

overlookmotel Nov 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

overlookmotel Nov 18, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

overlookmotel Nov 18, 2025

Choose a reason for hiding this comment

Uh oh!

overlookmotel commented Nov 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

overlookmotel commented Nov 27, 2025

Uh oh!

overlookmotel commented Dec 1, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

codspeed-hq bot commented Nov 18, 2025 •

edited

Loading

overlookmotel left a comment •

edited

Loading

`ConfigStoreBuilder::build`

overlookmotel Nov 18, 2025 •

edited

Loading

overlookmotel Nov 18, 2025 •

edited

Loading

overlookmotel commented Nov 24, 2025 •

edited

Loading