fix: detect inifinite loops caused by conflicting rule fixes #7732
Conversation
|
WalkthroughAdds a new public Suggested labels
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
crates/biome_service/src/file_handlers/javascript.rs (1)
875-1006: Loop counter trips during perfectly normal fix_all runs
loopsincrements once per applied fix, and we bail out after 51 passes. A file with, say, 60 ordinary autofixes (no conflicts) will now hit this guard and returnConflictingRuleFixesError, breakingfix_all. We need to detect “we are cycling” rather than “we’ve done lots of work”. Please gate the diagnostic on observing a repeated fix (e.g. by tracking(range, rule)pairs or a tree fingerprint) instead of a hard iteration cap. Happy to brainstorm if helpful.
🧹 Nitpick comments (1)
crates/biome_service/src/file_handlers/javascript.rs (1)
991-1004: Make conflicting-rule list deterministic
HashSet::into_iter()gives us an arbitrary order, so the diagnostic message will reshuffle between runs. Sorting (or usingBTreeSet) before formatting keeps the output stable.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
crates/biome_analyze/src/diagnostics.rs(3 hunks)crates/biome_service/src/file_handlers/javascript.rs(3 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
crates/biome_*/**
📄 CodeRabbit inference engine (CLAUDE.md)
Place core crates under /crates/biome_*/
Files:
crates/biome_service/src/file_handlers/javascript.rscrates/biome_analyze/src/diagnostics.rs
**/*.{rs,toml}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Before committing, format Rust and TOML files (e.g., via
just f/just format)
Files:
crates/biome_service/src/file_handlers/javascript.rscrates/biome_analyze/src/diagnostics.rs
**/*.rs
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Document rules, assists, and options via inline rustdoc in Rust source
Files:
crates/biome_service/src/file_handlers/javascript.rscrates/biome_analyze/src/diagnostics.rs
🧠 Learnings (1)
📚 Learning: 2025-10-02T12:56:59.418Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_diagnostics/CONTRIBUTING.md:0-0
Timestamp: 2025-10-02T12:56:59.418Z
Learning: Applies to crates/biome_diagnostics/**/*.rs : When deriving Diagnostic for an enum, ensure each variant contains a type that is itself a diagnostic
Applied to files:
crates/biome_analyze/src/diagnostics.rs
🧬 Code graph analysis (1)
crates/biome_analyze/src/diagnostics.rs (1)
packages/@biomejs/backend-jsonrpc/src/workspace.ts (1)
Category(8813-9235)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: Test (depot-ubuntu-24.04-arm-16)
- GitHub Check: Documentation
- GitHub Check: Test (depot-windows-2022-16)
- GitHub Check: Check Dependencies
- GitHub Check: Test Node.js API
- GitHub Check: autofix
dyc3
force-pushed
the
dyc3/mitigate-infinite-loops
branch
from
22e7793 to
7a4b1d2
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
crates/biome_service/src/file_handlers/javascript.rs (1)
874-874: Solid infinite loop detection!The implementation correctly detects pathological fix cycles by tracking iterations and collecting the likely culprits. The logic flow is sound: the counter increments only after successful fix application, and the loop naturally exits before incrementing if no fixes remain.
The collection of the last 10 fixes and HashSet deduplication is a pragmatic approach to identifying conflicting rules.
Optionally, consider extracting
50as a named constant (e.g.,MAX_FIX_ITERATIONS) to make the threshold more discoverable and easier to adjust if needed:+const MAX_FIX_ITERATIONS: usize = 50; + pub(crate) fn fix_all(params: FixAllParams) -> Result<FixFileResult, WorkspaceError> { // ... loop { // ... loops += 1; - if loops > 50 { + if loops > MAX_FIX_ITERATIONS {Also applies to: 983-1006
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
crates/biome_analyze/src/diagnostics.rs(3 hunks)crates/biome_service/src/file_handlers/javascript.rs(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- crates/biome_analyze/src/diagnostics.rs
🧰 Additional context used
📓 Path-based instructions (3)
crates/biome_*/**
📄 CodeRabbit inference engine (CLAUDE.md)
Place core crates under /crates/biome_*/
Files:
crates/biome_service/src/file_handlers/javascript.rs
**/*.{rs,toml}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Before committing, format Rust and TOML files (e.g., via
just f/just format)
Files:
crates/biome_service/src/file_handlers/javascript.rs
**/*.rs
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Document rules, assists, and options via inline rustdoc in Rust source
Files:
crates/biome_service/src/file_handlers/javascript.rs
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
- GitHub Check: Test (depot-windows-2022-16)
- GitHub Check: Test Node.js API
- GitHub Check: Test (depot-ubuntu-24.04-arm-16)
- GitHub Check: Documentation
- GitHub Check: Lint project (depot-windows-2022)
- GitHub Check: Check Dependencies
- GitHub Check: autofix
🔇 Additional comments (1)
crates/biome_service/src/file_handlers/javascript.rs (1)
59-59: LGTM!The HashSet import is necessary for deduplicating conflicting rules in the infinite loop detection logic below.
dyc3
force-pushed
the
dyc3/mitigate-infinite-loops
branch
from
7a4b1d2 to
74e8eb8
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
crates/biome_service/src/file_handlers/javascript.rs (1)
988-990: Maybe soften the comment toneThe detection logic is lovely, but the comment’s swear jar moment might age poorly in the repo. Fancy rephrasing it without the profanity so future readers don’t get a jump-scare?
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
crates/biome_analyze/src/diagnostics.rs(3 hunks)crates/biome_service/src/file_handlers/javascript.rs(3 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
crates/biome_*/**
📄 CodeRabbit inference engine (CLAUDE.md)
Place core crates under /crates/biome_*/
Files:
crates/biome_service/src/file_handlers/javascript.rscrates/biome_analyze/src/diagnostics.rs
**/*.{rs,toml}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Before committing, format Rust and TOML files (e.g., via
just f/just format)
Files:
crates/biome_service/src/file_handlers/javascript.rscrates/biome_analyze/src/diagnostics.rs
**/*.rs
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Document rules, assists, and options via inline rustdoc in Rust source
Files:
crates/biome_service/src/file_handlers/javascript.rscrates/biome_analyze/src/diagnostics.rs
🧬 Code graph analysis (1)
crates/biome_analyze/src/diagnostics.rs (1)
packages/@biomejs/backend-jsonrpc/src/workspace.ts (1)
Category(8813-9235)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: Check Dependencies
- GitHub Check: Test (depot-ubuntu-24.04-arm-16)
- GitHub Check: Test (depot-windows-2022-16)
- GitHub Check: Documentation
- GitHub Check: Test Node.js API
- GitHub Check: autofix
| fmt.write_markup(markup! { | ||
| "An internal error occurred when analyzing this file." | ||
| })?; | ||
| fmt.write_markup(markup! { | ||
| {self} | ||
| })?; | ||
| fmt.write_markup(markup! { | ||
| "This is likely a bug in Biome, not an error in your code. Please consider filing an issue on "<Hyperlink href="https://github.com/biomejs/biome/issues/new/choose">"GitHub"</Hyperlink>" with a reproduction of this error." | ||
| })?; |
There was a problem hiding this comment.
Add paragraph breaks to the diagnostic message
Right now each write_markup call runs back-to-back, so the rendered text comes out as one long sentence (…file.The…This…). A couple of explicit separators will fix the readability.
fmt.write_markup(markup! {
"An internal error occurred when analyzing this file."
})?;
+ fmt.write_markup(markup! {
+ "\n\n"
+ })?;
fmt.write_markup(markup! {
{self}
})?;
+ fmt.write_markup(markup! {
+ "\n\n"
+ })?;
fmt.write_markup(markup! {
"This is likely a bug in Biome, not an error in your code. Please consider filing an issue on "<Hyperlink href="https://github.com/biomejs/biome/issues/new/choose">"GitHub"</Hyperlink>" with a reproduction of this error."
})?;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| fmt.write_markup(markup! { | |
| "An internal error occurred when analyzing this file." | |
| })?; | |
| fmt.write_markup(markup! { | |
| {self} | |
| })?; | |
| fmt.write_markup(markup! { | |
| "This is likely a bug in Biome, not an error in your code. Please consider filing an issue on "<Hyperlink href="https://github.com/biomejs/biome/issues/new/choose">"GitHub"</Hyperlink>" with a reproduction of this error." | |
| })?; | |
| fmt.write_markup(markup! { | |
| "An internal error occurred when analyzing this file." | |
| })?; | |
| fmt.write_markup(markup! { | |
| "\n\n" | |
| })?; | |
| fmt.write_markup(markup! { | |
| {self} | |
| })?; | |
| fmt.write_markup(markup! { | |
| "\n\n" | |
| })?; | |
| fmt.write_markup(markup! { | |
| "This is likely a bug in Biome, not an error in your code. Please consider filing an issue on "<Hyperlink href="https://github.com/biomejs/biome/issues/new/choose">"GitHub"</Hyperlink>" with a reproduction of this error." | |
| })?; |
🤖 Prompt for AI Agents
In crates/biome_analyze/src/diagnostics.rs around lines 268 to 276, the three
consecutive fmt.write_markup calls produce concatenated text with no spacing;
add explicit paragraph breaks between the messages (for example insert an empty
paragraph or newline markup between the first and second and between the second
and third write_markup calls) so the rendered diagnostic shows separate
sentences/paragraphs rather than a single run-on line.
74e8eb8 to
7039241
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
crates/biome_analyze/src/diagnostics.rs(3 hunks)crates/biome_service/src/file_handlers/javascript.rs(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- crates/biome_analyze/src/diagnostics.rs
🧰 Additional context used
📓 Path-based instructions (3)
crates/biome_*/**
📄 CodeRabbit inference engine (CLAUDE.md)
Place core crates under /crates/biome_*/
Files:
crates/biome_service/src/file_handlers/javascript.rs
**/*.{rs,toml}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Before committing, format Rust and TOML files (e.g., via
just f/just format)
Files:
crates/biome_service/src/file_handlers/javascript.rs
**/*.rs
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Document rules, assists, and options via inline rustdoc in Rust source
Files:
crates/biome_service/src/file_handlers/javascript.rs
🧬 Code graph analysis (1)
crates/biome_service/src/file_handlers/javascript.rs (1)
crates/biome_analyze/src/signals.rs (8)
new(50-57)new(174-185)new(281-292)new(331-349)actions(23-23)actions(87-93)actions(180-182)actions(383-459)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (9)
- GitHub Check: Validate PR title
- GitHub Check: autofix
- GitHub Check: Test (depot-windows-2022-16)
- GitHub Check: Test (depot-ubuntu-24.04-arm-16)
- GitHub Check: Lint project (depot-windows-2022)
- GitHub Check: Documentation
- GitHub Check: Lint project (depot-ubuntu-24.04-arm-16)
- GitHub Check: Check Dependencies
- GitHub Check: Test Node.js API
dyc3
force-pushed
the
dyc3/mitigate-infinite-loops
branch
from
7039241 to
c0779e7
Compare
|
I think I've come up with a better heuristic. Instead of a hard limit on loops, we keep track of "growth streaks" instead. |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
crates/biome_service/src/file_handlers/javascript.rs (3)
874-884: Heuristic is good; add a cycle fallback to catch “ping‑pong” loopsStable-size oscillations won’t trip growth/accel streaks. Consider a lightweight cycle detector (e.g. repeat of the same (rule, range.start, range.end) sequence K times) or a generous hard ceiling with telemetry before bailing with the same error.
Can you confirm if you’ve observed ping‑pong loops in #7711 reproduction? If yes, I can sketch a minimal range‑based cycle check.
969-986: Guard growth on empty baselineWhen
prev_len == 0, every change counts as “growth” and will build a streak from an empty file. Gate the check onprev_len > 0.Apply this small tweak:
- if curr_len.saturating_mul(RATIO_Q) >= prev_len.saturating_mul(RATIO_GROWTH) { + if prev_len > 0 + && curr_len.saturating_mul(RATIO_Q) >= prev_len.saturating_mul(RATIO_GROWTH) + {
875-876: Tiny tidy‑up
initial_lenis only used to seedprev_len. Inline it to reduce locals.- let initial_len: u32 = tree.syntax().text_range_with_trivia().len().into(); - let mut prev_len = initial_len; + let mut prev_len: u32 = tree.syntax().text_range_with_trivia().len().into();
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
crates/biome_analyze/src/diagnostics.rs(3 hunks)crates/biome_service/src/file_handlers/javascript.rs(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- crates/biome_analyze/src/diagnostics.rs
🧰 Additional context used
📓 Path-based instructions (3)
crates/biome_*/**
📄 CodeRabbit inference engine (CLAUDE.md)
Place core crates under /crates/biome_*/
Files:
crates/biome_service/src/file_handlers/javascript.rs
**/*.{rs,toml}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Before committing, format Rust and TOML files (e.g., via
just f/just format)
Files:
crates/biome_service/src/file_handlers/javascript.rs
**/*.rs
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Document rules, assists, and options via inline rustdoc in Rust source
Files:
crates/biome_service/src/file_handlers/javascript.rs
🧬 Code graph analysis (1)
crates/biome_service/src/file_handlers/javascript.rs (1)
crates/biome_analyze/src/signals.rs (8)
new(50-57)new(174-185)new(281-292)new(331-349)actions(23-23)actions(87-93)actions(180-182)actions(383-459)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: Lint project (depot-windows-2022)
- GitHub Check: Check Dependencies
- GitHub Check: Test (depot-windows-2022-16)
- GitHub Check: Test (depot-ubuntu-24.04-arm-16)
- GitHub Check: autofix
- GitHub Check: Test Node.js API
🔇 Additional comments (1)
crates/biome_service/src/file_handlers/javascript.rs (1)
59-59: Import looks fine
HashSetis used below for deduping recent rules. No concerns.
dyc3
force-pushed
the
dyc3/mitigate-infinite-loops
branch
from
c0779e7 to
e639e70
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (2)
crates/biome_service/src/file_handlers/javascript.rs (1)
962-984: Own the rule IDs and make output deterministic (and tweak comment)
- Storing
&Cowthen cloning yields&Cow, notCow— mismatches the error’s expected type.- Sort for stable diagnostics.
- Fix typo/wording.
- // Check for runaway edit growth + // Check for runaway edit growth let curr_len: u32 = tree.syntax().text_range_with_trivia().len().into(); if !growth_guard.check(curr_len) { - // In order to provide a useful diagnostic, we want to flag the rules that caused the conflict. - // We can do this by inspecting the last few fixes that were applied. - // We limit it to the last 10 fixes. If there is a chain of conclicting fixes longer than that, something is **really** fucked up. + // Provide a useful diagnostic by flagging rules implicated in the conflict. + // Inspect the last few fixes that were applied (limit to the last 10). - let mut seen_rules = HashSet::new(); + let mut seen_rules: HashSet<(Cow<'static, str>, Cow<'static, str>)> = + HashSet::new(); for action in actions.iter().rev().take(10) { if let Some((group, rule)) = action.rule_name.as_ref() { - seen_rules.insert((group, rule)); + seen_rules.insert(((*group).clone(), (*rule).clone())); } } - return Err(WorkspaceError::RuleError( - RuleError::ConflictingRuleFixesError { - rules: seen_rules - .into_iter() - .map(|(g, r)| (g.clone(), r.clone())) - .collect(), - }, - )); + let mut rules: Vec<_> = seen_rules.into_iter().collect(); + rules.sort_unstable(); + return Err(WorkspaceError::RuleError( + RuleError::ConflictingRuleFixesError { rules }, + )); }Also consider extracting a small helper to DRY this across handlers.
crates/biome_analyze/src/diagnostics.rs (1)
249-279: Improve message readability: add paragraph breaksThe three write_markup calls currently run together. Insert explicit separators between them.
Apply:
fmt.write_markup(markup! { "An internal error occurred when analyzing this file." })?; + fmt.write_markup(markup! { "\n\n" })?; fmt.write_markup(markup! { {self} })?; + fmt.write_markup(markup! { "\n\n" })?; fmt.write_markup(markup! { "This is likely a bug in Biome, not an error in your code. Please consider filing an issue on "<Hyperlink href="https://github.com/biomejs/biome/issues/new/choose">"GitHub"</Hyperlink>" with a reproduction of this error." })?;
🧹 Nitpick comments (5)
crates/biome_service/src/utils/mod.rs (1)
1-1: Scope the module to the crateMake the module
pub(crate)to avoid exposing an otherwise crate‑private utility in the public API surface.-pub mod growth_guard; +pub(crate) mod growth_guard;crates/biome_service/src/file_handlers/json.rs (2)
46-46: Make diagnostics deterministic: prefer BTreeSet over HashSetStable ordering avoids flaky messages/tests and eases triage.
Apply:
-use std::collections::HashSet; +use std::collections::BTreeSet;
719-741: Deterministic rule list + tidy comment
- Use BTreeSet (or sort) so listed rules are consistently ordered.
- Fix typo and tone in the comment (“conclicting” → “conflicting”; avoid profanity).
Apply:
- let mut seen_rules = HashSet::new(); + let mut seen_rules = BTreeSet::new(); for action in actions.iter().rev().take(10) { if let Some((group, rule)) = action.rule_name.as_ref() { seen_rules.insert((group, rule)); } } return Err(WorkspaceError::RuleError( RuleError::ConflictingRuleFixesError { rules: seen_rules .into_iter() .map(|(g, r)| (g.clone(), r.clone())) .collect(), }, ));And reword the preceding comment:
- // We limit it to the last 10 fixes. If there is a chain of conclicting fixes longer than that, something is **really** fucked up. + // We limit it to the last 10 fixes. If there is a chain of conflicting fixes longer than that, something is severely broken.crates/biome_analyze/src/diagnostics.rs (2)
299-301: Consistent capitalisationMatch console Display and other branches.
Apply:
- return std::write!(fmt, "conflicting rule fixes detected"); + return std::write!(fmt, "Conflicting rule fixes detected.");
303-311: Optional: sort rule names for stable outputHelps avoid flaky snapshots and eases scanning.
Example change:
- let rules_list = rules - .iter() - .map(|(group, rule)| format!("'{group}/{rule}'")) - .collect::<Vec<_>>() - .join(", "); + let mut items = rules + .iter() + .map(|(group, rule)| format!("'{group}/{rule}'")) + .collect::<Vec<_>>(); + items.sort_unstable(); + let rules_list = items.join(", ");
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
crates/biome_analyze/src/diagnostics.rs(3 hunks)crates/biome_service/src/file_handlers/css.rs(4 hunks)crates/biome_service/src/file_handlers/graphql.rs(4 hunks)crates/biome_service/src/file_handlers/javascript.rs(4 hunks)crates/biome_service/src/file_handlers/json.rs(4 hunks)crates/biome_service/src/lib.rs(1 hunks)crates/biome_service/src/utils/growth_guard.rs(1 hunks)crates/biome_service/src/utils/mod.rs(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- crates/biome_service/src/lib.rs
🧰 Additional context used
📓 Path-based instructions (3)
crates/biome_*/**
📄 CodeRabbit inference engine (CLAUDE.md)
Place core crates under /crates/biome_*/
Files:
crates/biome_service/src/utils/mod.rscrates/biome_service/src/file_handlers/json.rscrates/biome_analyze/src/diagnostics.rscrates/biome_service/src/utils/growth_guard.rscrates/biome_service/src/file_handlers/css.rscrates/biome_service/src/file_handlers/javascript.rscrates/biome_service/src/file_handlers/graphql.rs
**/*.{rs,toml}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Before committing, format Rust and TOML files (e.g., via
just f/just format)
Files:
crates/biome_service/src/utils/mod.rscrates/biome_service/src/file_handlers/json.rscrates/biome_analyze/src/diagnostics.rscrates/biome_service/src/utils/growth_guard.rscrates/biome_service/src/file_handlers/css.rscrates/biome_service/src/file_handlers/javascript.rscrates/biome_service/src/file_handlers/graphql.rs
**/*.rs
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Document rules, assists, and options via inline rustdoc in Rust source
Files:
crates/biome_service/src/utils/mod.rscrates/biome_service/src/file_handlers/json.rscrates/biome_analyze/src/diagnostics.rscrates/biome_service/src/utils/growth_guard.rscrates/biome_service/src/file_handlers/css.rscrates/biome_service/src/file_handlers/javascript.rscrates/biome_service/src/file_handlers/graphql.rs
🧬 Code graph analysis (4)
crates/biome_service/src/file_handlers/json.rs (1)
crates/biome_service/src/utils/growth_guard.rs (1)
new(19-26)
crates/biome_analyze/src/diagnostics.rs (1)
packages/@biomejs/backend-jsonrpc/src/workspace.ts (1)
Category(8813-9235)
crates/biome_service/src/file_handlers/css.rs (1)
crates/biome_service/src/utils/growth_guard.rs (1)
new(19-26)
crates/biome_service/src/file_handlers/graphql.rs (1)
crates/biome_service/src/utils/growth_guard.rs (1)
new(19-26)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
- GitHub Check: Check Dependencies
- GitHub Check: Documentation
- GitHub Check: Test (depot-ubuntu-24.04-arm-16)
- GitHub Check: Test (depot-windows-2022-16)
- GitHub Check: Lint project (depot-ubuntu-24.04-arm-16)
- GitHub Check: Lint project (depot-windows-2022)
- GitHub Check: Test Node.js API
- GitHub Check: autofix
🔇 Additional comments (7)
crates/biome_service/src/file_handlers/graphql.rs (1)
549-550: Good call: initialising GrowthGuard with current text lengthThis is the right baseline for relative growth checks. LGTM.
crates/biome_service/src/file_handlers/css.rs (1)
639-640: Good baseline for GrowthGuardInitialising from the current syntax length is correct. LGTM.
crates/biome_service/src/file_handlers/javascript.rs (1)
875-877: Nice: growth guard replaces brittle loop capThis is a more robust heuristic for fix‑all. LGTM.
crates/biome_service/src/file_handlers/json.rs (2)
15-15: Good call: wire in GrowthGuardImport is correct and matches the usage below. No concerns.
649-649: Initialising GrowthGuard with current length looks rightThe u32 conversion from TextSize is safe. Proceed.
crates/biome_analyze/src/diagnostics.rs (2)
243-247: New error variant looks solidShape and docs are clear and future‑proof. Nice.
334-346: Console Display branch mirrors std::fmt—LGTMReads well; only follow the optional sorting if you do it above for parity.
| // Check for runaway edit growth | ||
| let curr_len: u32 = tree.syntax().text_range_with_trivia().len().into(); | ||
| if !growth_guard.check(curr_len) { | ||
| // In order to provide a useful diagnostic, we want to flag the rules that caused the conflict. | ||
| // We can do this by inspecting the last few fixes that were applied. | ||
| // We limit it to the last 10 fixes. If there is a chain of conclicting fixes longer than that, something is **really** fucked up. | ||
|
|
||
| let mut seen_rules = HashSet::new(); | ||
| for action in actions.iter().rev().take(10) { | ||
| if let Some((group, rule)) = action.rule_name.as_ref() { | ||
| seen_rules.insert((group, rule)); | ||
| } | ||
| } | ||
|
|
||
| return Err(WorkspaceError::RuleError( | ||
| RuleError::ConflictingRuleFixesError { | ||
| rules: seen_rules | ||
| .into_iter() | ||
| .map(|(g, r)| (g.clone(), r.clone())) | ||
| .collect(), | ||
| }, | ||
| )); | ||
| } |
There was a problem hiding this comment.
Own the Cows and sort; fix comment wording
Same issues as in GraphQL:
- Store owned
Cows (cloning refs yields refs). - Sort for stability.
- Clean up wording/typo.
- // Check for runaway edit growth
+ // Check for runaway edit growth
let curr_len: u32 = tree.syntax().text_range_with_trivia().len().into();
if !growth_guard.check(curr_len) {
- // In order to provide a useful diagnostic, we want to flag the rules that caused the conflict.
- // We can do this by inspecting the last few fixes that were applied.
- // We limit it to the last 10 fixes. If there is a chain of conclicting fixes longer than that, something is **really** fucked up.
+ // Provide a useful diagnostic by flagging rules implicated in the conflict.
+ // Inspect the last few fixes that were applied (limit to the last 10).
- let mut seen_rules = HashSet::new();
+ let mut seen_rules: HashSet<(Cow<'static, str>, Cow<'static, str>)> =
+ HashSet::new();
for action in actions.iter().rev().take(10) {
if let Some((group, rule)) = action.rule_name.as_ref() {
- seen_rules.insert((group, rule));
+ seen_rules.insert(((*group).clone(), (*rule).clone()));
}
}
- return Err(WorkspaceError::RuleError(
- RuleError::ConflictingRuleFixesError {
- rules: seen_rules
- .into_iter()
- .map(|(g, r)| (g.clone(), r.clone()))
- .collect(),
- },
- ));
+ let mut rules: Vec<_> = seen_rules.into_iter().collect();
+ rules.sort_unstable();
+ return Err(WorkspaceError::RuleError(
+ RuleError::ConflictingRuleFixesError { rules },
+ ));
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Check for runaway edit growth | |
| let curr_len: u32 = tree.syntax().text_range_with_trivia().len().into(); | |
| if !growth_guard.check(curr_len) { | |
| // In order to provide a useful diagnostic, we want to flag the rules that caused the conflict. | |
| // We can do this by inspecting the last few fixes that were applied. | |
| // We limit it to the last 10 fixes. If there is a chain of conclicting fixes longer than that, something is **really** fucked up. | |
| let mut seen_rules = HashSet::new(); | |
| for action in actions.iter().rev().take(10) { | |
| if let Some((group, rule)) = action.rule_name.as_ref() { | |
| seen_rules.insert((group, rule)); | |
| } | |
| } | |
| return Err(WorkspaceError::RuleError( | |
| RuleError::ConflictingRuleFixesError { | |
| rules: seen_rules | |
| .into_iter() | |
| .map(|(g, r)| (g.clone(), r.clone())) | |
| .collect(), | |
| }, | |
| )); | |
| } | |
| // Check for runaway edit growth | |
| let curr_len: u32 = tree.syntax().text_range_with_trivia().len().into(); | |
| if !growth_guard.check(curr_len) { | |
| // Provide a useful diagnostic by flagging rules implicated in the conflict. | |
| // Inspect the last few fixes that were applied (limit to the last 10). | |
| let mut seen_rules: HashSet<(Cow<'static, str>, Cow<'static, str>)> = | |
| HashSet::new(); | |
| for action in actions.iter().rev().take(10) { | |
| if let Some((group, rule)) = action.rule_name.as_ref() { | |
| seen_rules.insert(((*group).clone(), (*rule).clone())); | |
| } | |
| } | |
| let mut rules: Vec<_> = seen_rules.into_iter().collect(); | |
| rules.sort_unstable(); | |
| return Err(WorkspaceError::RuleError( | |
| RuleError::ConflictingRuleFixesError { rules }, | |
| )); | |
| } |
🤖 Prompt for AI Agents
In crates/biome_service/src/file_handlers/css.rs around lines 715 to 737, the
code currently collects rule identifiers into a HashSet of borrowed Cows and
returns them in an arbitrary order with an awkward comment; change the HashSet
to store owned values (e.g. HashSet<(String, String)> or HashSet<(Cow<'static,
str>, Cow<'static, str>)> by cloning the group and rule into the set), then when
building the Vec for the RuleError convert the owned pairs into the final
(String,String) tuples, sort that Vec deterministically (e.g. sort_by on tuple)
before returning it for stability, and update the comment text to fix the
wording/typo (remove profanity and clarify “longer than that indicates a serious
issue”).
| // Check for runaway edit growth | ||
| let curr_len: u32 = tree.syntax().text_range_with_trivia().len().into(); | ||
| if !growth_guard.check(curr_len) { | ||
| // In order to provide a useful diagnostic, we want to flag the rules that caused the conflict. | ||
| // We can do this by inspecting the last few fixes that were applied. | ||
| // We limit it to the last 10 fixes. If there is a chain of conclicting fixes longer than that, something is **really** fucked up. | ||
|
|
||
| let mut seen_rules = HashSet::new(); | ||
| for action in actions.iter().rev().take(10) { | ||
| if let Some((group, rule)) = action.rule_name.as_ref() { | ||
| seen_rules.insert((group, rule)); | ||
| } | ||
| } | ||
|
|
||
| return Err(WorkspaceError::RuleError( | ||
| RuleError::ConflictingRuleFixesError { | ||
| rules: seen_rules | ||
| .into_iter() | ||
| .map(|(g, r)| (g.clone(), r.clone())) | ||
| .collect(), | ||
| }, | ||
| )); | ||
| } |
There was a problem hiding this comment.
Own the rule IDs and sort for deterministic, valid diagnostics (also fix comment text)
- The set currently stores
&Cow; cloning returns&Cow, notCow, yielding the wrong element type forConflictingRuleFixesError.rules. - HashSet iteration order is non‑deterministic; sort for stable output.
- Minor: fix “conclicting” and avoid profanity in comments.
- // Check for runaway edit growth
+ // Check for runaway edit growth
let curr_len: u32 = tree.syntax().text_range_with_trivia().len().into();
if !growth_guard.check(curr_len) {
- // In order to provide a useful diagnostic, we want to flag the rules that caused the conflict.
- // We can do this by inspecting the last few fixes that were applied.
- // We limit it to the last 10 fixes. If there is a chain of conclicting fixes longer than that, something is **really** fucked up.
+ // Provide a useful diagnostic by flagging rules implicated in the conflict.
+ // Inspect the last few fixes that were applied (limit to the last 10).
- let mut seen_rules = HashSet::new();
+ let mut seen_rules: HashSet<(Cow<'static, str>, Cow<'static, str>)> =
+ HashSet::new();
for action in actions.iter().rev().take(10) {
if let Some((group, rule)) = action.rule_name.as_ref() {
- seen_rules.insert((group, rule));
+ // Own the values (cheap when Borrowed)
+ seen_rules.insert(((*group).clone(), (*rule).clone()));
}
}
- return Err(WorkspaceError::RuleError(
- RuleError::ConflictingRuleFixesError {
- rules: seen_rules
- .into_iter()
- .map(|(g, r)| (g.clone(), r.clone()))
- .collect(),
- },
- ));
+ // Deterministic output
+ let mut rules: Vec<_> = seen_rules.into_iter().collect();
+ rules.sort_unstable();
+ return Err(WorkspaceError::RuleError(
+ RuleError::ConflictingRuleFixesError { rules },
+ ));
}Also consider extracting this “collect last N rule pairs and build error” into a small helper to avoid duplication across handlers.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Check for runaway edit growth | |
| let curr_len: u32 = tree.syntax().text_range_with_trivia().len().into(); | |
| if !growth_guard.check(curr_len) { | |
| // In order to provide a useful diagnostic, we want to flag the rules that caused the conflict. | |
| // We can do this by inspecting the last few fixes that were applied. | |
| // We limit it to the last 10 fixes. If there is a chain of conclicting fixes longer than that, something is **really** fucked up. | |
| let mut seen_rules = HashSet::new(); | |
| for action in actions.iter().rev().take(10) { | |
| if let Some((group, rule)) = action.rule_name.as_ref() { | |
| seen_rules.insert((group, rule)); | |
| } | |
| } | |
| return Err(WorkspaceError::RuleError( | |
| RuleError::ConflictingRuleFixesError { | |
| rules: seen_rules | |
| .into_iter() | |
| .map(|(g, r)| (g.clone(), r.clone())) | |
| .collect(), | |
| }, | |
| )); | |
| } | |
| // Check for runaway edit growth | |
| let curr_len: u32 = tree.syntax().text_range_with_trivia().len().into(); | |
| if !growth_guard.check(curr_len) { | |
| // Provide a useful diagnostic by flagging rules implicated in the conflict. | |
| // Inspect the last few fixes that were applied (limit to the last 10). | |
| let mut seen_rules: HashSet<(Cow<'static, str>, Cow<'static, str>)> = | |
| HashSet::new(); | |
| for action in actions.iter().rev().take(10) { | |
| if let Some((group, rule)) = action.rule_name.as_ref() { | |
| // Own the values (cheap when Borrowed) | |
| seen_rules.insert(((*group).clone(), (*rule).clone())); | |
| } | |
| } | |
| // Deterministic output | |
| let mut rules: Vec<_> = seen_rules.into_iter().collect(); | |
| rules.sort_unstable(); | |
| return Err(WorkspaceError::RuleError( | |
| RuleError::ConflictingRuleFixesError { rules }, | |
| )); | |
| } |
🤖 Prompt for AI Agents
crates/biome_service/src/file_handlers/graphql.rs around lines 619-641: the code
currently inserts references to rule IDs into a HashSet and then collects them
directly, which yields the wrong element types and non‑deterministic ordering;
change the HashSet to store owned rule IDs (clone the Cow values when inserting,
e.g. insert((g.clone(), r.clone()))), then convert the set into a Vec, sort it
deterministically (e.g. sort_by on group then rule) before constructing
ConflictingRuleFixesError.rules so the diagnostic is stable and has the correct
owned types; also fix the comment text typos (“conclicting” → “conflicting”,
remove profanity) and consider extracting the “collect last N rule pairs and
build error” logic into a small helper function to avoid duplication across
handlers.
| let diff = new.saturating_sub(self.previous); | ||
|
|
||
| // Check for multiplicative growth | ||
| if new * RATIO_Q >= self.previous * RATIO_GROWTH { | ||
| self.growth_streak += 1; | ||
| } else { | ||
| self.growth_streak = 0; | ||
| } | ||
|
|
||
| // Check for delta acceleration | ||
| if diff * RATIO_Q >= self.previous_diff * RATIO_ACCEL && self.previous_diff > 0 { | ||
| self.accel_streak += 1; |
There was a problem hiding this comment.
Avoid u32 multiplication overflow in growth checks
new * RATIO_Q and diff * RATIO_Q can overflow u32 (debug panics; release wraps), skewing detection on large files. Use widened arithmetic.
- // Check for multiplicative growth
- if new * RATIO_Q >= self.previous * RATIO_GROWTH {
+ // Check for multiplicative growth (avoid u32 overflow)
+ if (new as u64) * (RATIO_Q as u64) >= (self.previous as u64) * (RATIO_GROWTH as u64) {
self.growth_streak += 1;
} else {
self.growth_streak = 0;
}
// Check for delta acceleration
- if diff * RATIO_Q >= self.previous_diff * RATIO_ACCEL && self.previous_diff > 0 {
+ if (diff as u64) * (RATIO_Q as u64)
+ >= (self.previous_diff as u64) * (RATIO_ACCEL as u64)
+ && self.previous_diff > 0
+ {
self.accel_streak += 1;
} else {
self.accel_streak = 0;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| let diff = new.saturating_sub(self.previous); | |
| // Check for multiplicative growth | |
| if new * RATIO_Q >= self.previous * RATIO_GROWTH { | |
| self.growth_streak += 1; | |
| } else { | |
| self.growth_streak = 0; | |
| } | |
| // Check for delta acceleration | |
| if diff * RATIO_Q >= self.previous_diff * RATIO_ACCEL && self.previous_diff > 0 { | |
| self.accel_streak += 1; | |
| let diff = new.saturating_sub(self.previous); | |
| // Check for multiplicative growth (avoid u32 overflow) | |
| if (new as u64) * (RATIO_Q as u64) >= (self.previous as u64) * (RATIO_GROWTH as u64) { | |
| self.growth_streak += 1; | |
| } else { | |
| self.growth_streak = 0; | |
| } | |
| // Check for delta acceleration | |
| if (diff as u64) * (RATIO_Q as u64) | |
| >= (self.previous_diff as u64) * (RATIO_ACCEL as u64) | |
| && self.previous_diff > 0 | |
| { | |
| self.accel_streak += 1; | |
| } else { | |
| self.accel_streak = 0; | |
| } |
🤖 Prompt for AI Agents
In crates/biome_service/src/utils/growth_guard.rs around lines 41 to 52, the
multiplications new * RATIO_Q and diff * RATIO_Q can overflow u32; change the
arithmetic to widened integer types (e.g., cast new, diff, previous,
previous_diff and the RATIO_* constants to u64) before multiplying and comparing
so all intermediate products are computed in u64, then perform the comparisons
in u64 and only assign back to u32 fields if necessary; alternatively use
checked_mul/checked_ge to handle overflow explicitly, but the simplest fix is to
cast operands to u64 for the multiplications and comparisons and preserve the
existing logic.
dyc3
force-pushed
the
dyc3/mitigate-infinite-loops
branch
from
e639e70 to
aaa7cfb
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (3)
crates/biome_service/src/file_handlers/css.rs (1)
715-737: Own the Cows and sort; fix comment wording.The current code has several issues already flagged in a previous review:
- Line 725 inserts borrowed
&Cowreferences, then lines 731-734 clone them, yielding borrowed Cows instead of owned ones.- HashSet iteration is non-deterministic, producing unstable output.
- Line 720 contains unprofessional language.
Apply this diff:
- // Check for runaway edit growth + // Check for runaway edit growth let curr_len: u32 = tree.syntax().text_range_with_trivia().len().into(); if !growth_guard.check(curr_len) { - // In order to provide a useful diagnostic, we want to flag the rules that caused the conflict. - // We can do this by inspecting the last few fixes that were applied. - // We limit it to the last 10 fixes. If there is a chain of conclicting fixes longer than that, something is **really** fucked up. + // Provide a useful diagnostic by flagging rules implicated in the conflict. + // Inspect the last few fixes that were applied (limit to the last 10). - let mut seen_rules = HashSet::new(); + let mut seen_rules: HashSet<(Cow<'static, str>, Cow<'static, str>)> = + HashSet::new(); for action in actions.iter().rev().take(10) { if let Some((group, rule)) = action.rule_name.as_ref() { - seen_rules.insert((group, rule)); + seen_rules.insert(((*group).clone(), (*rule).clone())); } } - return Err(WorkspaceError::RuleError( - RuleError::ConflictingRuleFixesError { - rules: seen_rules - .into_iter() - .map(|(g, r)| (g.clone(), r.clone())) - .collect(), - }, - )); + let mut rules: Vec<_> = seen_rules.into_iter().collect(); + rules.sort_unstable(); + return Err(WorkspaceError::RuleError( + RuleError::ConflictingRuleFixesError { rules }, + )); }crates/biome_service/src/file_handlers/javascript.rs (1)
962-984: Own the Cows and sort; fix comment wording.The current code has the same issues as in css.rs:
- Line 972 inserts borrowed
&Cowreferences, then lines 978-981 clone them, yielding borrowed Cows instead of owned ones.- HashSet iteration is non-deterministic.
- Line 967 contains unprofessional language.
Apply the same diff as in css.rs:
- // Check for runaway edit growth + // Check for runaway edit growth let curr_len: u32 = tree.syntax().text_range_with_trivia().len().into(); if !growth_guard.check(curr_len) { - // In order to provide a useful diagnostic, we want to flag the rules that caused the conflict. - // We can do this by inspecting the last few fixes that were applied. - // We limit it to the last 10 fixes. If there is a chain of conclicting fixes longer than that, something is **really** fucked up. + // Provide a useful diagnostic by flagging rules implicated in the conflict. + // Inspect the last few fixes that were applied (limit to the last 10). - let mut seen_rules = HashSet::new(); + let mut seen_rules: HashSet<(Cow<'static, str>, Cow<'static, str>)> = + HashSet::new(); for action in actions.iter().rev().take(10) { if let Some((group, rule)) = action.rule_name.as_ref() { - seen_rules.insert((group, rule)); + seen_rules.insert(((*group).clone(), (*rule).clone())); } } - return Err(WorkspaceError::RuleError( - RuleError::ConflictingRuleFixesError { - rules: seen_rules - .into_iter() - .map(|(g, r)| (g.clone(), r.clone())) - .collect(), - }, - )); + let mut rules: Vec<_> = seen_rules.into_iter().collect(); + rules.sort_unstable(); + return Err(WorkspaceError::RuleError( + RuleError::ConflictingRuleFixesError { rules }, + )); }Note: As ematipico suggested, consider extracting this detection logic into a shared helper function to eliminate duplication across file handlers.
crates/biome_analyze/src/diagnostics.rs (1)
249-279: Add paragraph breaks between messages.Lines 268-276 concatenate three messages without spacing, producing run-on text (
…file.The…This…).Apply this diff:
fmt.write_markup(markup! { "An internal error occurred when analyzing this file." })?; + fmt.write_markup(markup! { + "\n\n" + })?; fmt.write_markup(markup! { {self} })?; + fmt.write_markup(markup! { + "\n\n" + })?; fmt.write_markup(markup! { "This is likely a bug in Biome, not an error in your code. Please consider filing an issue on "<Hyperlink href="https://github.com/biomejs/biome/issues/new/choose">"GitHub"</Hyperlink>" with a reproduction of this error." })?;
🧹 Nitpick comments (1)
crates/biome_analyze/src/diagnostics.rs (1)
281-314: Capitalisation inconsistency across error types.Lines 289 and 295 capitalise differently ("The rule" vs "A code action"), whilst line 300 uses lowercase ("conflicting") but line 335 uses uppercase ("Conflicting"). Consider harmonising the style.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
crates/biome_analyze/src/diagnostics.rs(3 hunks)crates/biome_service/src/file_handlers/css.rs(4 hunks)crates/biome_service/src/file_handlers/graphql.rs(4 hunks)crates/biome_service/src/file_handlers/javascript.rs(4 hunks)crates/biome_service/src/file_handlers/json.rs(4 hunks)crates/biome_service/src/lib.rs(1 hunks)crates/biome_service/src/utils/growth_guard.rs(1 hunks)crates/biome_service/src/utils/mod.rs(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- crates/biome_service/src/file_handlers/graphql.rs
- crates/biome_service/src/file_handlers/json.rs
🧰 Additional context used
📓 Path-based instructions (3)
crates/biome_*/**
📄 CodeRabbit inference engine (CLAUDE.md)
Place core crates under /crates/biome_*/
Files:
crates/biome_service/src/utils/mod.rscrates/biome_analyze/src/diagnostics.rscrates/biome_service/src/file_handlers/css.rscrates/biome_service/src/file_handlers/javascript.rscrates/biome_service/src/lib.rscrates/biome_service/src/utils/growth_guard.rs
**/*.{rs,toml}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Before committing, format Rust and TOML files (e.g., via
just f/just format)
Files:
crates/biome_service/src/utils/mod.rscrates/biome_analyze/src/diagnostics.rscrates/biome_service/src/file_handlers/css.rscrates/biome_service/src/file_handlers/javascript.rscrates/biome_service/src/lib.rscrates/biome_service/src/utils/growth_guard.rs
**/*.rs
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Document rules, assists, and options via inline rustdoc in Rust source
Files:
crates/biome_service/src/utils/mod.rscrates/biome_analyze/src/diagnostics.rscrates/biome_service/src/file_handlers/css.rscrates/biome_service/src/file_handlers/javascript.rscrates/biome_service/src/lib.rscrates/biome_service/src/utils/growth_guard.rs
🧬 Code graph analysis (3)
crates/biome_analyze/src/diagnostics.rs (1)
packages/@biomejs/backend-jsonrpc/src/workspace.ts (1)
Category(8813-9235)
crates/biome_service/src/file_handlers/css.rs (1)
crates/biome_service/src/utils/growth_guard.rs (1)
new(19-26)
crates/biome_service/src/file_handlers/javascript.rs (1)
crates/biome_analyze/src/signals.rs (8)
new(50-57)new(174-185)new(281-292)new(331-349)actions(23-23)actions(87-93)actions(180-182)actions(383-459)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (9)
- GitHub Check: Lint project (depot-ubuntu-24.04-arm-16)
- GitHub Check: Documentation
- GitHub Check: Lint project (depot-windows-2022)
- GitHub Check: Test (depot-ubuntu-24.04-arm-16)
- GitHub Check: Test (depot-windows-2022-16)
- GitHub Check: Check Dependencies
- GitHub Check: End-to-end tests
- GitHub Check: Test Node.js API
- GitHub Check: autofix
🔇 Additional comments (12)
crates/biome_service/src/lib.rs (1)
15-15: LGTM!Clean internal module declaration to expose the new utilities.
crates/biome_service/src/utils/mod.rs (1)
1-1: LGTM!Properly exposes the growth_guard module.
crates/biome_service/src/file_handlers/css.rs (3)
16-16: LGTM!Appropriate import for the new GrowthGuard utility.
46-46: LGTM!HashSet import needed for collecting implicated rules.
639-639: LGTM!GrowthGuard initialised correctly with the current syntax length.
crates/biome_service/src/file_handlers/javascript.rs (3)
12-12: LGTM!Appropriate import for the GrowthGuard utility.
60-60: LGTM!HashSet import needed for collecting implicated rules.
875-876: LGTM!Clear comment and correct GrowthGuard initialisation.
crates/biome_analyze/src/diagnostics.rs (1)
243-246: LGTM!New error variant properly declared for conflicting rule fixes.
crates/biome_service/src/utils/growth_guard.rs (3)
1-16: LGTM!Constants and struct declaration are clear and well-documented.
19-26: LGTM!Constructor correctly initialises all fields.
31-69: Growth detection logic looks sound.The saturating arithmetic at lines 44 and 51 properly addresses the previous overflow concern. The streak tracking and limit enforcement correctly implement the runaway detection heuristic.
2 participants
Summary
I recently encountered an infinite loop when applying fixes, and I have a hunch it's related to recent reports of biome using too many resources. Regardless, we want to detect this anyway to make it easier for users to give us good bug reports when this happens.
related to (but does not solve) #7711
Test Plan
Reproduce #7711, and see that the diagnostic gets emitted.
Docs