Skip to content

test(linter): add tests for missing rule config#23140

Merged
graphite-app[bot] merged 1 commit into
mainfrom
06-08-test_linter_add_tests_for_missing_rule_config_
Jun 9, 2026
Merged

test(linter): add tests for missing rule config#23140
graphite-app[bot] merged 1 commit into
mainfrom
06-08-test_linter_add_tests_for_missing_rule_config_

Conversation

@Sysix

@Sysix Sysix commented Jun 8, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

follow up from #23139

Added tests when a test-case has an option but the rule.has_config(), making sure this code path will never hit for rules which should have a config:

oxc/crates/oxc_linter/src/config/rules.rs

Lines 158 to 165 in 01b74c4

if !rule_config.config.is_empty() && !rule.has_config() {
errors.push(OverrideRulesError::RuleConfiguration {
rule_name: rule_config.full_name().into_owned(),
message: "This rule does not accept configuration options."
.to_string(),
});
continue;
}

On the way I found some rules where the test-cases shown us options for rules, which we do not support atm.

@Sysix Sysix mentioned this pull request Jun 8, 2026
Merged
@Sysix Graphite App

Sysix commented Jun 8, 2026 •
edited
Loading

Copy link
Copy Markdown
Member Author

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 changes, fast-track this PR to the front of 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.

@github-actions github-actions Bot added the A-linter Area - Linter label Jun 8, 2026
@codspeed-hq

codspeed-hq Bot commented Jun 8, 2026 •
edited
Loading

Copy link
Copy Markdown

Merging this PR will not alter performance

✅ 5 untouched benchmarks
⏩ 66 skipped benchmarks1

Comparing 06-08-test_linter_add_tests_for_missing_rule_config_ (aac7be9) with 06-08-fix_linter_allow_options_for_eslint_capitalized-comments_ (e6c5b80)

Open in CodSpeed

Footnotes

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

@Sysix Sysix force-pushed the 06-08-test_linter_add_tests_for_missing_rule_config_ branch 2 times, most recently from f95e376 to 97e0414 Compare June 8, 2026 20:46
@Sysix Sysix marked this pull request as ready for review June 8, 2026 20:51
@Sysix Sysix requested a review from camc314 as a code owner June 8, 2026 20:51
@camc314 camc314 added the 0-merge Merge with Graphite Merge Queue label Jun 9, 2026
@camc314 Graphite App

camc314 commented Jun 9, 2026 •
edited by graphite-app Bot
Loading

Copy link
Copy Markdown
Contributor

Merge activity

@graphite-app graphite-app Bot changed the base branch from 06-08-fix_linter_allow_options_for_eslint_capitalized-comments_ to graphite-base/23140 June 9, 2026 08:20
@graphite-app graphite-app Bot changed the base branch from graphite-base/23140 to main June 9, 2026 08:26
@graphite-app graphite-app Bot requested a review from overlookmotel as a code owner June 9, 2026 08:26
@github-actions github-actions Bot added the A-cli Area - CLI label Jun 9, 2026
@Sysix
test(linter): add tests for missing rule config (#23140)
e78e75b 
follow up from #23139

Added tests when a test-case has an option but the `rule.has_config()`, making sure this code path will never hit for rules which should have a config:
https://github.com/oxc-project/oxc/blob/01b74c4e84696950add4b813b0dfa2600c459f09/crates/oxc_linter/src/config/rules.rs#L158-L165

On the way I found some rules where the test-cases shown us options for rules, which we do not support atm.
@graphite-app graphite-app Bot force-pushed the 06-08-test_linter_add_tests_for_missing_rule_config_ branch from aac7be9 to e78e75b Compare June 9, 2026 08:27
@graphite-app graphite-app Bot merged commit e78e75b into main Jun 9, 2026
29 checks passed
@graphite-app graphite-app Bot removed the 0-merge Merge with Graphite Merge Queue label Jun 9, 2026
@graphite-app graphite-app Bot deleted the 06-08-test_linter_add_tests_for_missing_rule_config_ branch June 9, 2026 08:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@camc314 camc314 camc314 approved these changes

@overlookmotel overlookmotel Awaiting requested review from overlookmotel

Assignees

@camc314 camc314

Labels

A-cli Area - CLI A-linter Area - Linter

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@Sysix @camc314