refactor(linter/plugins): prepare for rule options#16158
refactor(linter/plugins): prepare for rule options#16158graphite-app[bot] merged 1 commit intomainfrom
Conversation
How to use the Graphite Merge QueueAdd either label to this PR to merge it via the merge queue:
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. This stack of pull requests is managed by Graphite. Learn more about stacking. |
Merge activity
|
There was a problem hiding this comment.
Pull request overview
This PR refactors the options handling system for linter rules in preparation for supporting rule options. The main change is to centralize options management through a new options.ts module with an allOptions array that stores all rule options configurations.
Key Changes:
- Creates a new centralized options module (
options.ts) with anallOptionsarray for storing rule configurations - Changes
RuleDetails.optionsfrom always beingDEFAULT_OPTIONSto initiallynull, then set before linting each file - Threads
optionsIdsparameter through the linting pipeline to support per-rule options lookup
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| apps/oxlint/src-js/plugins/options.ts | New module centralizing options type and storage with allOptions array and DEFAULT_OPTIONS_ID constant |
| apps/oxlint/src-js/plugins/load.ts | Moves Options type to new module and initializes RuleDetails.options to null instead of DEFAULT_OPTIONS |
| apps/oxlint/src-js/plugins/lint.ts | Adds optionsIds parameter and assigns options from allOptions array before linting each file |
| apps/oxlint/src-js/plugins/context.ts | Updates imports and adds null assertion for options in context getter |
| apps/oxlint/src-js/index.ts | Updates exports to reference Options type from new module |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Preparatory work for supporting rule options. Store options for all rules in an `allOptions` array. Currently this array only contains a single value for empty options, but in future we'll populate it with options extracted from configs, sent over from Rust side. Am adding this now, before full implementation (#14825), as it's required for rule tester.
388e098 to
fa3b070
Compare
- 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>
Preparatory work for supporting rule options. Store options for all rules in an `allOptions` array. Currently this array only contains a single value for empty options, but in future we'll populate it with options extracted from configs, sent over from Rust side. Am adding this now, before full implementation (oxc-project#14825), as it's required for rule tester.
- 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>

Preparatory work for supporting rule options.
Store options for all rules in an
allOptionsarray. Currently this array only contains a single value for empty options, but in future we'll populate it with options extracted from configs, sent over from Rust side.Am adding this now, before full implementation (#14825), as it's required for rule tester.