Skip to content

Comments

refactor(linter/plugins): RuleTester use setOptions to set up options#16929

Merged
graphite-app[bot] merged 1 commit intomainfrom
12-15-refactor_linter_plugins_ruletester_use_setoptions_to_set_up_options
Dec 16, 2025
Merged

refactor(linter/plugins): RuleTester use setOptions to set up options#16929
graphite-app[bot] merged 1 commit intomainfrom
12-15-refactor_linter_plugins_ruletester_use_setoptions_to_set_up_options

Conversation

@overlookmotel
Copy link
Member

@overlookmotel overlookmotel commented Dec 16, 2025 •
edited
Loading

RuleTester call setOptions to set up rule options, same as linter does. This makes sure the rule tester's behavior matches actual linter exactly.

It also prepares the way for applying defaults from rule.meta.schema (#16930).

@github-actions github-actions bot added A-linter Area - Linter A-cli Area - CLI A-linter-plugins Area - Linter JS plugins C-cleanup Category - technical debt or refactoring. Solution not expected to change behavior labels Dec 16, 2025
This was referenced Dec 16, 2025
Merged
Merged
@overlookmotel Graphite App
Copy link
Member Author

overlookmotel commented Dec 16, 2025 •
edited
Loading

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.

This stack of pull requests is managed by Graphite. Learn more about stacking.

@overlookmotel overlookmotel mentioned this pull request Dec 16, 2025
Merged
@overlookmotel overlookmotel marked this pull request as ready for review December 16, 2025 03:37
Copilot AI review requested due to automatic review settings December 16, 2025 03:37
@overlookmotel overlookmotel self-assigned this Dec 16, 2025
Copilot AI reviewed Dec 16, 2025
View reviewed changes
Copy link
Contributor

Copilot AI 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 refactors the RuleTester implementation to use setOptions() for setting up rule options, making it follow the same code path as the actual linter. Previously, the RuleTester directly called mergeOptions() and manipulated the allOptions array, which diverged from how the Rust linter processes options. This refactor improves consistency and fixes a bug where a test case with empty options object [{}] wasn't being handled correctly.

Key changes:

  • RuleTester now builds a JSON string in the same format as Rust does and calls setOptions() instead of directly manipulating options
  • Removed initAllOptions() function that was only used by RuleTester
  • Made mergeOptions() a private function since it's now only called internally by setOptions()

Reviewed changes

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

File Description
apps/oxlint/src-js/plugins/options.ts Removed initAllOptions() function and made mergeOptions() private since it's now only used internally by setOptions()
apps/oxlint/src-js/package/rule_tester.ts Added setupOptions() function that mimics Rust behavior by building JSON string and calling setOptions(), updated imports, and improved cleanup logic to handle null allOptions
apps/oxlint/conformance/snapshot.md Test results updated showing no-useless-computed-key rule now fully passing (217 vs 216 fully passing rules, 28671 vs 28670 passing tests)

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

@overlookmotel overlookmotel added the 0-merge Merge with Graphite Merge Queue label Dec 16, 2025
@overlookmotel Graphite App
Copy link
Member Author

overlookmotel commented Dec 16, 2025 •
edited by graphite-app bot
Loading

Merge activity

@overlookmotel
refactor(linter/plugins): RuleTester use setOptions to set up opt…
0c799e2 
…ions (#16929)

`RuleTester` call `setOptions` to set up rule options, same as linter does. This makes sure the rule tester's behavior matches actual linter exactly.

It also prepares the way for applying defaults from `rule.meta.schema` (#16930).
@graphite-app graphite-app bot force-pushed the 12-16-fix_linter_plugins_correctly_handle_object_with___proto___keys_in_options_merging branch from 69d41c5 to b845871 Compare December 16, 2025 03:41
@graphite-app graphite-app bot force-pushed the 12-15-refactor_linter_plugins_ruletester_use_setoptions_to_set_up_options branch from 0772a71 to 0c799e2 Compare December 16, 2025 03:42
Base automatically changed from 12-16-fix_linter_plugins_correctly_handle_object_with___proto___keys_in_options_merging to main December 16, 2025 03:47
@graphite-app graphite-app bot removed the 0-merge Merge with Graphite Merge Queue label Dec 16, 2025
@graphite-app graphite-app bot merged commit 0c799e2 into main Dec 16, 2025
19 checks passed
@graphite-app graphite-app bot deleted the 12-15-refactor_linter_plugins_ruletester_use_setoptions_to_set_up_options branch December 16, 2025 03:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot Copilot left review comments

@camc314 camc314 Awaiting requested review from camc314 camc314 is a code owner

Assignees

@overlookmotel overlookmotel

Labels

A-cli Area - CLI A-linter Area - Linter A-linter-plugins Area - Linter JS plugins C-cleanup Category - technical debt or refactoring. Solution not expected to change behavior

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@overlookmotel