Conversation
🦋 Changeset detectedLatest commit: d5103e7 The changes in this PR will be included in the next version bump. This PR includes changesets to release 14 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 |
Merging this PR will degrade performance by 72.63%
Performance Changes
Comparing |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds a Suggested reviewers
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
crates/biome_configuration_macros/src/group_struct.rs (1)
255-305:⚠️ Potential issue | 🟠 Major
preset: "all"is ignored for configured groups.Both
collect_preset_rules()branches only handle the recommended path. Sosuspicious: { preset: "all" }enables nothing, and a top-levelpreset: "all"also stops applying tosuspiciousas soon as that group has any nested config. The effective preset needs to flow throughpreset_as_filters(...)here, not just the recommended special-case.Also applies to: 391-412
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/biome_configuration_macros/src/group_struct.rs` around lines 255 - 305, collect_preset_rules currently only applies the "recommended" preset path and ignores PresetConfig::All for configured groups; update collect_preset_rules (and the similar branch at the other location) to compute the effective preset for the group (e.g. prefer self.preset if Some(...) else parent_preset) and, when that effective preset is PresetConfig::All or FromAnalyzer(...), extend enabled_rules with Self::preset_as_filters(effective_preset) in addition to the existing recommended_rules_as_filters() handling; use the existing helpers is_preset_recommended()/is_recommended_unset() only for the recommended-special-case so the All path flows through preset_as_filters instead of being skipped.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.changeset/huge-walls-float.md:
- Around line 7-10: The "all" preset description currently overpromises by
implying it enables every Biome rule while the implementation still keeps
nursery rules opt-in; update the text for the `"all"` preset (and the other
occurrences referenced around lines 24-30) to explicitly state that `"all"`
enables all non-nursery rules or "all stable rules" and that
nursery/experimental rules remain opt-in, e.g., replace "it enables all Biome
rules" with language like "it enables all stable/non-nursery Biome rules;
nursery (experimental) rules remain opt-in" so readers get the correct
expectation.
In `@crates/biome_analyze/src/rule.rs`:
- Around line 50-51: The new pub field rule_presets: &'static [RulePreset] on
RuleMetadata is never populated by the existing fluent builder used by
declare_*_rule!, so add a builder setter method to RuleMetadata (e.g., a
rule_presets(...) fluent method) that accepts &'static [RulePreset] and sets the
internal field; update the RuleMetadata::default/constructor to initialize
rule_presets to &[] and ensure any places that build metadata via the fluent API
(the declare_*_rule! macro usages) can call .rule_presets(...) to populate it so
the field is no longer left empty by default.
In `@crates/biome_cli/tests/cases/preset.rs`:
- Around line 358-398: The test group_preset_all_with_rule_off only triggers
noDebugger (which is explicitly "off"), so it can pass even if the "suspicious"
preset handling is broken; update the test by adding a second non-recommended
rule under "suspicious" in the config JSON (in the Utf8Path "biome.json"
inserted in the fs) and make test.js include code that triggers that rule (in
addition to DEBUGGER_BEFORE) so the run_cli call will produce a diagnostic when
the "preset": "all" behavior is working; keep the existing noDebugger:"off"
entry and assert_cli_snapshot as before.
In `@crates/biome_configuration/src/analyzer/linter/mod.rs`:
- Line 55: Remove the stray dbg!(&self) call that prints the parsed linter
config; locate the dbg! invocation in the module (mod.rs) inside the
Analyzer/Linter implementation (the dbg!(&self) line) and delete it so the
linter no longer writes user config to stderr and muddies CLI or
machine-readable output.
In `@crates/biome_migrate/src/analyzers.rs`:
- Around line 49-50: The Recommended analyzer is being registered
unconditionally via registry.record_rule::<Recommended>() which causes it to
match any JSON member named "recommended"; remove or comment out that
registration here (registry.record_rule::<Recommended>()) and instead only
register Recommended after its matcher is made path-scoped in
crates/biome_migrate/src/analyzers/recommended.rs (or add a TODO noting to
register once path-scoped). This prevents the analyzer from rewriting unrelated
keys outside linter.rules until Recommended's matcher is updated to be
path-aware.
In `@crates/biome_migrate/src/analyzers/recommended.rs`:
- Around line 69-84: The fixer currently misapplies trivia: when constructing
the json_member for "preset" use name.trailing_trivia().pieces() for the
string's trailing trivia instead of name.leading_trivia(), and fix the colon
token trivia by assigning its leading trivia to the token's leading_trivia and
its trailing trivia to the token's trailing_trivia (do not call
with_leading_trivia_pieces twice). Update the json_string_literal("preset") call
in the json_member_name to use
.with_trailing_trivia_pieces(name.trailing_trivia().pieces()), and set the
token(T![:]) to use node.colon_token().ok()?.leading_trivia().pieces() for
leading and node.colon_token().ok()?.trailing_trivia().pieces() for trailing (or
capture colon_token() once into a local to avoid duplicate calls) so inline
comments/spacing around the member are preserved.
In `@crates/biome_service/src/settings.rs`:
- Around line 634-646: The logic in recommended_enabled() is reading raw fields
and using or(Some(...)) which causes an empty rules object to resolve
incorrectly; instead derive the flag from the effective preset resolution used
by Rules (use the preset() or effective_preset() accessor on rules to get the
resolved preset) and check that preset.is_recommended(); update
recommended_enabled() to use rules.as_ref().map(|rules|
rules.preset().is_recommended()).unwrap_or(true) (and make the analogous change
for the other related method at lines 649-654 so both use the effective preset
accessor rather than or(Some(...))).
---
Outside diff comments:
In `@crates/biome_configuration_macros/src/group_struct.rs`:
- Around line 255-305: collect_preset_rules currently only applies the
"recommended" preset path and ignores PresetConfig::All for configured groups;
update collect_preset_rules (and the similar branch at the other location) to
compute the effective preset for the group (e.g. prefer self.preset if Some(...)
else parent_preset) and, when that effective preset is PresetConfig::All or
FromAnalyzer(...), extend enabled_rules with
Self::preset_as_filters(effective_preset) in addition to the existing
recommended_rules_as_filters() handling; use the existing helpers
is_preset_recommended()/is_recommended_unset() only for the
recommended-special-case so the All path flows through preset_as_filters instead
of being skipped.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 3cee5f18-ada8-451d-8d26-df92e062fbc7
⛔ Files ignored due to path filters (28)
crates/biome_cli/tests/snapshots/main_cases_migrate_v2/should_migrate_aws_config.snapis excluded by!**/*.snapand included by**crates/biome_cli/tests/snapshots/main_cases_migrate_v2/should_migrate_nested_config.snapis excluded by!**/*.snapand included by**crates/biome_cli/tests/snapshots/main_cases_migrate_v2/should_successfully_migrate_ariakit.snapis excluded by!**/*.snapand included by**crates/biome_cli/tests/snapshots/main_cases_migrate_v2/should_successfully_migrate_knip.snapis excluded by!**/*.snapand included by**crates/biome_cli/tests/snapshots/main_cases_migrate_v2/should_successfully_migrate_sentry.snapis excluded by!**/*.snapand included by**crates/biome_cli/tests/snapshots/main_cases_preset/group_preset_all_with_rule_off.snapis excluded by!**/*.snapand included by**crates/biome_cli/tests/snapshots/main_cases_preset/group_preset_none_disables_group_rules.snapis excluded by!**/*.snapand included by**crates/biome_cli/tests/snapshots/main_cases_preset/group_preset_none_with_rule_enabled.snapis excluded by!**/*.snapand included by**crates/biome_cli/tests/snapshots/main_cases_preset/group_preset_recommended_enables_group_rules.snapis excluded by!**/*.snapand included by**crates/biome_cli/tests/snapshots/main_cases_preset/nursery_preset_all_does_not_enable_rules.snapis excluded by!**/*.snapand included by**crates/biome_cli/tests/snapshots/main_cases_preset/nursery_rule_enabled_individually.snapis excluded by!**/*.snapand included by**crates/biome_cli/tests/snapshots/main_cases_preset/preset_all_does_not_enable_nursery_rules.snapis excluded by!**/*.snapand included by**crates/biome_cli/tests/snapshots/main_cases_preset/preset_all_enables_all_rules.snapis excluded by!**/*.snapand included by**crates/biome_cli/tests/snapshots/main_cases_preset/preset_none_disables_all_rules.snapis excluded by!**/*.snapand included by**crates/biome_cli/tests/snapshots/main_cases_preset/preset_recommended_does_not_enable_nursery_rules.snapis excluded by!**/*.snapand included by**crates/biome_cli/tests/snapshots/main_cases_preset/preset_recommended_enables_recommended_rules.snapis excluded by!**/*.snapand included by**crates/biome_cli/tests/snapshots/main_cases_preset/preset_recommended_same_as_recommended_true.snapis excluded by!**/*.snapand included by**crates/biome_cli/tests/snapshots/main_cases_preset/recommended_true_does_not_enable_nursery_rules.snapis excluded by!**/*.snapand included by**crates/biome_cli/tests/snapshots/main_commands_init/creates_config_file.snapis excluded by!**/*.snapand included by**crates/biome_cli/tests/snapshots/main_commands_init/creates_config_file_when_biome_installed_via_package_manager.snapis excluded by!**/*.snapand included by**crates/biome_cli/tests/snapshots/main_commands_init/creates_config_jsonc_file.snapis excluded by!**/*.snapand included by**crates/biome_cli/tests/snapshots/main_commands_init/enables_vcs_and_ignore_dist.snapis excluded by!**/*.snapand included by**crates/biome_cli/tests/snapshots/main_commands_lint/linter_shows_the_default_severity_of_rule_on.snapis excluded by!**/*.snapand included by**crates/biome_cli/tests/snapshots/main_configuration/incorrect_rule_name.snapis excluded by!**/*.snapand included by**crates/biome_configuration/src/analyzer/linter/rules.rsis excluded by!**/rules.rsand included by**crates/biome_configuration/tests/invalid/preset.json.snapis excluded by!**/*.snapand included by**packages/@biomejs/backend-jsonrpc/src/workspace.tsis excluded by!**/backend-jsonrpc/src/workspace.tsand included by**packages/@biomejs/biome/configuration_schema.jsonis excluded by!**/configuration_schema.jsonand included by**
📒 Files selected for processing (16)
.changeset/huge-walls-float.mdcrates/biome_analyze/src/lib.rscrates/biome_analyze/src/rule.rscrates/biome_cli/tests/cases/mod.rscrates/biome_cli/tests/cases/preset.rscrates/biome_configuration/src/analyzer/assist/actions.rscrates/biome_configuration/src/analyzer/linter/mod.rscrates/biome_configuration/src/analyzer/mod.rscrates/biome_configuration/src/analyzer/presets.rscrates/biome_configuration/src/lib.rscrates/biome_configuration/tests/invalid/preset.jsoncrates/biome_configuration_macros/src/group_struct.rscrates/biome_migrate/src/analyzers.rscrates/biome_migrate/src/analyzers/recommended.rscrates/biome_service/src/settings.rsxtask/codegen/src/generate_configuration.rs
| let new_value = json_string_value( | ||
| json_string_literal(new_value) | ||
| .with_leading_trivia_pieces(value.leading_trivia().pieces()) | ||
| .with_trailing_trivia_pieces(value.trailing_trivia().pieces()), | ||
| ); | ||
| let member = json_member( | ||
| json_member_name( | ||
| json_string_literal("preset") | ||
| .with_leading_trivia_pieces(name.leading_trivia().pieces()) | ||
| .with_trailing_trivia_pieces(name.leading_trivia().pieces()), | ||
| ) | ||
| .into(), | ||
| token(T![:]) | ||
| .with_leading_trivia_pieces(node.colon_token().ok()?.leading_trivia().pieces()) | ||
| .with_leading_trivia_pieces(node.colon_token().ok()?.trailing_trivia().pieces()), | ||
| AnyJsonValue::JsonStringValue(new_value.clone()), |
There was a problem hiding this comment.
The fixer drops trivia around "recommended":.
Line 78 uses name.leading_trivia() as trailing trivia, and Line 83 assigns the colon’s leading trivia twice. Inline comments or spacing around the member will be lost or moved by the migration.
Small fix
let member = json_member(
json_member_name(
json_string_literal("preset")
.with_leading_trivia_pieces(name.leading_trivia().pieces())
- .with_trailing_trivia_pieces(name.leading_trivia().pieces()),
+ .with_trailing_trivia_pieces(name.trailing_trivia().pieces()),
)
.into(),
token(T![:])
.with_leading_trivia_pieces(node.colon_token().ok()?.leading_trivia().pieces())
- .with_leading_trivia_pieces(node.colon_token().ok()?.trailing_trivia().pieces()),
+ .with_trailing_trivia_pieces(node.colon_token().ok()?.trailing_trivia().pieces()),
AnyJsonValue::JsonStringValue(new_value.clone()),
);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@crates/biome_migrate/src/analyzers/recommended.rs` around lines 69 - 84, The
fixer currently misapplies trivia: when constructing the json_member for
"preset" use name.trailing_trivia().pieces() for the string's trailing trivia
instead of name.leading_trivia(), and fix the colon token trivia by assigning
its leading trivia to the token's leading_trivia and its trailing trivia to the
token's trailing_trivia (do not call with_leading_trivia_pieces twice). Update
the json_string_literal("preset") call in the json_member_name to use
.with_trailing_trivia_pieces(name.trailing_trivia().pieces()), and set the
token(T![:]) to use node.colon_token().ok()?.leading_trivia().pieces() for
leading and node.colon_token().ok()?.trailing_trivia().pieces() for trailing (or
capture colon_token() once into a local to avoid duplicate calls) so inline
comments/spacing around the member are preserved.
dyc3
left a comment
There was a problem hiding this comment.
Maybe for a follow up PR, but I do think there should be some way for users to opt in to all the nursery rules. Maybe an all-with-nursery preset.
How will this interact with domains? should domains have the preset field as well?
|
I am slightly concerned by the
I guess this should run as today:
Domains have already a preset concept. We could extend it to support a {
"linter": {
"domains": {
"project": "all"
}
}
}Regarding the unit tests, it could be nice to have some tests for combining group and root {
"linter": {
"rules": {
"preset": "none",
"suspicious": {
"preset": "all",
}
}
}
}In the past we had some bugs there. |
While I understand the sentiment, nursery rules are opt-in by design, where users opt-in singularly. Nursery rules must stay a convenience for us maintainers, and less for users. Still, that's out of scope of this PR. |
Users have already answered this question in the relative issue. They enable all rules and remove what they need. |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
crates/biome_cli/src/commands/rage.rs (1)
239-243:⚠️ Potential issue | 🟡 MinorUse a neutral label for non-error diagnostics.
Line 241 always prints
"Error", which is now misleading when a diagnostic is a warning/hint/info.Suggested tweak
- {KeyValuePair::new("Error", markup!{ + {KeyValuePair::new("Diagnostic", markup!{ {format!{"{}", PrintDescription(&diagnostic)}} })}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/biome_cli/src/commands/rage.rs` around lines 239 - 243, The code currently hardcodes the label "Error" when rendering each diagnostic; update the KeyValuePair creation to use a neutral or level-aware label instead — e.g., replace KeyValuePair::new("Error", ...) with something like KeyValuePair::new(diagnostic.level().to_string().as_str() or "Diagnostic", ...) so warnings/hints/info are labeled correctly; ensure you still pass the same PrintDescription(&diagnostic) value for the message.
🧹 Nitpick comments (1)
crates/biome_configuration/src/analyzer/presets.rs (1)
93-99: Consider a more informative error message.When
RulePreset::from_strfails, the diagnostic shows "Invalid rule preset." which is less helpful than the serde error that lists valid values. You could construct a richer message here to match the serde path.That said, this is minor and won't block anything.
💡 Optional: Improve error message
match result { Ok(preset) => return Some(PresetConfig::FromAnalyzer(preset)), Err(err) => { - ctx.report(DeserializationDiagnostic::new(err).with_range(range)); + ctx.report( + DeserializationDiagnostic::new(err) + .with_range(range) + .note_with_list("Accepted values:", &["recommended", "all", "none"]), + ); } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/biome_configuration/src/analyzer/presets.rs` around lines 93 - 99, The diagnostic created when RulePreset::from_str(&value) returns Err should include the serde/parsing error text so users see the valid values; modify the Err branch (where ctx.report is called) to build a richer message that embeds err.to_string() (or formats "Invalid rule preset: {err}") into the DeserializationDiagnostic before calling ctx.report, keeping the existing .with_range(range) and returning PresetConfig::FromAnalyzer on Ok(preset).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@crates/biome_cli/src/commands/rage.rs`:
- Around line 239-243: The code currently hardcodes the label "Error" when
rendering each diagnostic; update the KeyValuePair creation to use a neutral or
level-aware label instead — e.g., replace KeyValuePair::new("Error", ...) with
something like KeyValuePair::new(diagnostic.level().to_string().as_str() or
"Diagnostic", ...) so warnings/hints/info are labeled correctly; ensure you
still pass the same PrintDescription(&diagnostic) value for the message.
---
Nitpick comments:
In `@crates/biome_configuration/src/analyzer/presets.rs`:
- Around line 93-99: The diagnostic created when RulePreset::from_str(&value)
returns Err should include the serde/parsing error text so users see the valid
values; modify the Err branch (where ctx.report is called) to build a richer
message that embeds err.to_string() (or formats "Invalid rule preset: {err}")
into the DeserializationDiagnostic before calling ctx.report, keeping the
existing .with_range(range) and returning PresetConfig::FromAnalyzer on
Ok(preset).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: e82c57e7-e1d6-47e6-9876-79300a6201bb
⛔ Files ignored due to path filters (14)
Cargo.lockis excluded by!**/*.lockand included by**crates/biome_cli/tests/snapshots/main_cases_overrides_linter/does_override_recommended.snapis excluded by!**/*.snapand included by**crates/biome_cli/tests/snapshots/main_cases_preset/customising_one_group_still_enables_recommended.snapis excluded by!**/*.snapand included by**crates/biome_cli/tests/snapshots/main_cases_preset/empty_rules_object_still_enables_recommended.snapis excluded by!**/*.snapand included by**crates/biome_cli/tests/snapshots/main_cases_preset/group_preset_all_with_rule_off.snapis excluded by!**/*.snapand included by**crates/biome_cli/tests/snapshots/main_cases_preset/preset_none_with_group_preset_all.snapis excluded by!**/*.snapand included by**crates/biome_cli/tests/snapshots/main_cases_rules_via_dependencies/doesnt_enable_rules_when_recommended_is_false.snapis excluded by!**/*.snapand included by**crates/biome_cli/tests/snapshots/main_commands_lint/should_not_choke_on_recursive_function_call.snapis excluded by!**/*.snapand included by**crates/biome_cli/tests/snapshots/main_commands_rage/with_configuration.snapis excluded by!**/*.snapand included by**crates/biome_cli/tests/snapshots/main_commands_rage/with_formatter_configuration.snapis excluded by!**/*.snapand included by**crates/biome_cli/tests/snapshots/main_commands_rage/with_jsonc_configuration.snapis excluded by!**/*.snapand included by**crates/biome_cli/tests/snapshots/main_commands_rage/with_linter_configuration.snapis excluded by!**/*.snapand included by**crates/biome_cli/tests/snapshots/main_commands_rage/with_malformed_configuration.snapis excluded by!**/*.snapand included by**crates/biome_configuration/src/analyzer/linter/rules.rsis excluded by!**/rules.rsand included by**
📒 Files selected for processing (9)
crates/biome_analyze/src/rule.rscrates/biome_cli/src/commands/rage.rscrates/biome_cli/tests/cases/preset.rscrates/biome_configuration/Cargo.tomlcrates/biome_configuration/src/analyzer/linter/mod.rscrates/biome_configuration/src/analyzer/presets.rscrates/biome_configuration_macros/src/group_struct.rscrates/biome_deserialize/src/diagnostics.rsxtask/codegen/src/generate_configuration.rs
✅ Files skipped from review due to trivial changes (1)
- crates/biome_configuration/Cargo.toml
🚧 Files skipped from review as they are similar to previous changes (2)
- crates/biome_analyze/src/rule.rs
- crates/biome_configuration/src/analyzer/linter/mod.rs
There was a problem hiding this comment.
🧹 Nitpick comments (1)
crates/biome_configuration/src/analyzer/presets.rs (1)
34-49: Unify parsing logic to avoid serde/visitor drift.You currently parse presets in two places with slightly different logic/messages. A tiny shared parser would keep behaviour consistent and future-proof this when
RulePresetgrows. One parser to rule them all.♻️ Proposed refactor
impl PresetConfig { + fn parse_str(value: &str) -> Result<Self, &'static str> { + match value { + "all" => Ok(Self::All), + "none" => Ok(Self::None), + _ => RulePreset::from_str(value).map(Self::FromAnalyzer), + } + } + pub const fn is_all(&self) -> bool { matches!(self, Self::All) } @@ impl<'de> Deserialize<'de> for PresetConfig { @@ - let s = <String as Deserialize>::deserialize(deserializer)?; - match s.as_str() { - "recommended" => Ok(Self::FromAnalyzer(RulePreset::Recommended)), - "all" => Ok(Self::All), - "none" => Ok(Self::None), - _ => Err(serde::de::Error::custom(format!( - "Invalid preset value: \"{s}\". Expected \"recommended\", \"all\", or \"none\"" - ))), - } + let s = <String as Deserialize>::deserialize(deserializer)?; + Self::parse_str(&s).map_err(|_| { + serde::de::Error::custom(format!( + "Invalid preset value: \"{s}\". Expected \"recommended\", \"all\", or \"none\"" + )) + }) } } @@ fn visit_str( @@ - if value == "none" { - return Some(PresetConfig::None); - } else if value == "all" { - return Some(PresetConfig::All); - } else { - let result = RulePreset::from_str(&value); - match result { - Ok(preset) => return Some(PresetConfig::FromAnalyzer(preset)), - Err(err) => { - ctx.report(DeserializationDiagnostic::new(err).with_range(range)); - } - } - } - - None + match PresetConfig::parse_str(&value) { + Ok(preset) => Some(preset), + Err(err) => { + ctx.report(DeserializationDiagnostic::new(err).with_range(range)); + None + } + } } }Also applies to: 71-107
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/biome_configuration/src/analyzer/presets.rs` around lines 34 - 49, The PresetConfig deserialize impl duplicates parsing logic already used elsewhere; create a single parser (e.g., implement std::str::FromStr for RulePreset or add a private fn parse_rule_preset(s: &str) -> Result<RulePreset, String>) and use it from PresetConfig::deserialize and the other parsing site (the code referenced around lines 71-107) so messages and accepted values stay consistent; update Deserialize for PresetConfig to call that shared parser and map its error into serde::de::Error::custom.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@crates/biome_configuration/src/analyzer/presets.rs`:
- Around line 34-49: The PresetConfig deserialize impl duplicates parsing logic
already used elsewhere; create a single parser (e.g., implement
std::str::FromStr for RulePreset or add a private fn parse_rule_preset(s: &str)
-> Result<RulePreset, String>) and use it from PresetConfig::deserialize and the
other parsing site (the code referenced around lines 71-107) so messages and
accepted values stay consistent; update Deserialize for PresetConfig to call
that shared parser and map its error into serde::de::Error::custom.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: a4473e67-02ba-4e6c-9943-8c358e568abb
📒 Files selected for processing (3)
crates/biome_cli/tests/snapshots/main_cases_preset/customising_one_group_still_enables_recommended.snap.newcrates/biome_cli/tests/snapshots/main_cases_preset/empty_rules_object_still_enables_recommended.snap.newcrates/biome_configuration/src/analyzer/presets.rs
✅ Files skipped from review due to trivial changes (2)
- crates/biome_cli/tests/snapshots/main_cases_preset/customising_one_group_still_enables_recommended.snap.new
- crates/biome_cli/tests/snapshots/main_cases_preset/empty_rules_object_still_enables_recommended.snap.new
25f4e1b to
b959845
Compare
|
Sorry for the delay. I don't feel like my concerns about how this interacts with domains is completely addressed.
|
That's already the case with the
No, it would increase complexity a lot on our part, and in time of writing, it would provide little gains to users.
It depends on the preset (time of writing). The all preset enables all rules regardless. The recommended preset works the same as the existing flag. So to give you a more generic answer, I think presets don't have predefined rules, but they have the rules we decide and document. For example, wild idea, if in the future we add a |
Ok, that makes sense to me 👍 |
a68879e to
b8955a5
Compare
b8955a5 to
d5103e7
Compare
What do you mean. Setting preset to |
I think things aren't that complex. With |
Summary
Closes #5764
Adds a new
presetconfiguration, next to therecommendedconfiguration. As for now, thepresetyields the same semantics ofrecommended: trueandrecommended: false, plus, it adds anallpreset, which brings back the oldall: truefield, but it has different semantics: nursery rules aren't enabled regardless of its value.I added a migration to move to the new field, as
recommendedwill fade.The reason why
presetis a string and not a list is that:AI was used only to create tests and to help me understand the macros.
Test Plan
Added a new series of tests. Made sure
Docs
I will open a new PR