Conversation
🦋 Changeset detectedLatest commit: 2770365 The changes in this PR will be included in the next version bump. This PR includes changesets to release 13 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
ematipico
force-pushed
the
fix/incorrect-options
branch
from
48fbc58 to
67c19ce
Compare
github-actions
bot
added
A-Project
Contributor
Parser conformance results onjs/262
jsx/babel
markdown/commonmark
symbols/microsoft
ts/babel
ts/microsoft
|
Contributor
WalkthroughAdds a test-only feature and generated test helper to validate that lint rule Options types match their canonical configuration types: a Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
📝 Coding Plan
Comment Tip You can validate your CodeRabbit configuration file in your editor.If your editor has YAML language server, you can enable auto-completion and validation by adding |
Merging this PR will not alter performance
Comparing 
Footnotes
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@crates/biome_service/src/configuration.rs`:
- Around line 1256-1283: The current check only reports mismatched type IDs for
entries present in config_side; extend it to also fail when keys are missing by
comparing key sets between config_side (from config_side_rule_options_types())
and visitor.types: compute the set of (group, rule) keys from config_side and
from visitor.types, detect keys present in one but not the other, and add
descriptive messages to mismatches for "missing in config_side" and "missing in
visitor.types" (or vice versa); keep the existing mismatches.push behavior and
the final panic that joins mismatches so the harness fails when entries are
omitted as well as when types differ.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 3bcc2d6c-48bf-435f-8975-4a59e53d1d32
⛔ Files ignored due to path filters (1)
crates/biome_configuration/src/generated/linter_options_check.rsis excluded by!**/generated/**,!**/generated/**and included by**
📒 Files selected for processing (2)
crates/biome_service/src/configuration.rsxtask/codegen/src/generate_configuration.rs
siketyan
approved these changes
Mar 12, 2026
ematipico
force-pushed
the
fix/incorrect-options
branch
from
106d5f6 to
e870d48
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@xtask/codegen/src/generate_configuration.rs`:
- Around line 850-892: The generated test currently reconstructs TypeIds from
naming conventions (Case::Snake + to_capitalized) in
config_side_rule_options_types(), which can miss mismatches introduced by
biome_configuration_macros::lint_group_structs!() and push_to_analyzer_rules;
instead, change the test to derive the asserted TypeIds from the actual
generated config structs or by exercising the real config → AnalyzerRules
wiring: import the concrete generated group structs produced by
biome_configuration_macros::lint_group_structs!() (and the equivalent for
assists), construct/inspect the AnalyzerRules/path used by
push_to_analyzer_rules (or call the same helper used by configuration wiring),
and call TypeId::of::<ActualGeneratedOptionsType>() or inspect the resulting
AnalyzerRules mapping to build result.push entries so the test fails when the
config-layer imports the wrong options type rather than when only the naming
convention differs.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: bbc4f035-22f5-4f0e-9ae8-3977c82f6310
⛔ Files ignored due to path filters (1)
crates/biome_configuration/src/generated/linter_options_check.rsis excluded by!**/generated/**,!**/generated/**and included by**
📒 Files selected for processing (1)
xtask/codegen/src/generate_configuration.rs
Comment on lines
+850
to
+892
| // Generate the options type-check test file for lint rules | ||
| if kind == RuleCategory::Lint { | ||
| let mut push_statements = String::new(); | ||
| for rg in &rule_group_names { | ||
| let snake_name = Case::Snake.convert(rg.rule_name); | ||
| let options_type_name = format!("{}Options", to_capitalized(rg.rule_name)); | ||
| push_statements.push_str(&format!( | ||
| "result.push((\"{group}\", \"{rule}\", TypeId::of::<biome_rule_options::{module}::{options}>()));\n", | ||
| group = rg.group_name, | ||
| rule = rg.rule_name, | ||
| module = snake_name, | ||
| options = options_type_name, | ||
| )); | ||
| } | ||
|
|
||
| let options_check = format!( | ||
| "\ | ||
| #![expect(clippy::vec_init_then_push)] | ||
| use std::any::TypeId; | ||
|
|
||
| /// Returns a list of `(group, rule, config_side_type_id)` for every lint rule. | ||
| /// | ||
| /// The `TypeId` is derived from the rule name convention: | ||
| /// `biome_rule_options::{{snake_case_name}}::{{PascalCaseName}}Options` | ||
| /// | ||
| /// This is the type that the configuration layer uses when constructing | ||
| /// [`RuleOptions`](biome_analyze::options::RuleOptions). If the rule's | ||
| /// `type Options` doesn't match, the `debug_assert_eq!` inside | ||
| /// `RuleOptions::value()` will fire at runtime. | ||
| pub fn config_side_rule_options_types() -> Vec<(&'static str, &'static str, TypeId)> {{ | ||
| let mut result = Vec::new(); | ||
| {push_statements} result | ||
| }} | ||
| " | ||
| ); | ||
|
|
||
| let options_check_file = push_directory.join("linter_options_check.rs"); | ||
| update( | ||
| &options_check_file, | ||
| &xtask_glue::reformat(options_check)?, | ||
| mode, | ||
| )?; | ||
| } |
Contributor
There was a problem hiding this comment.
This guardrail checks the convention, not the path that actually panicked.
The emitted TypeId is rebuilt from Case::Snake + to_capitalized(rule_name), whilst the real config-side wiring still comes from biome_configuration_macros::lint_group_structs!() and then flows through push_to_analyzer_rules. If that generated wiring imports the wrong options type again, this helper can stay green because it never reads the concrete type used by the configuration layer. Please derive the assertion from the generated config structs themselves, or exercise the config → AnalyzerRules path directly; otherwise we’re testing the map, not the road. The same gap also exists for assists.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@xtask/codegen/src/generate_configuration.rs` around lines 850 - 892, The
generated test currently reconstructs TypeIds from naming conventions
(Case::Snake + to_capitalized) in config_side_rule_options_types(), which can
miss mismatches introduced by biome_configuration_macros::lint_group_structs!()
and push_to_analyzer_rules; instead, change the test to derive the asserted
TypeIds from the actual generated config structs or by exercising the real
config → AnalyzerRules wiring: import the concrete generated group structs
produced by biome_configuration_macros::lint_group_structs!() (and the
equivalent for assists), construct/inspect the AnalyzerRules/path used by
push_to_analyzer_rules (or call the same helper used by configuration wiring),
and call TypeId::of::<ActualGeneratedOptionsType>() or inspect the resulting
AnalyzerRules mapping to build result.push entries so the test fails when the
config-layer imports the wrong options type rather than when only the naming
convention differs.
dyc3
approved these changes
Mar 12, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Closes #9314
Test Plan
It's a bug we couldn't catch because we didn't have a test for it. So I know added some codegen to create it, so it fails in CI. The test was vibecoded and it works.
Docs