crates/biome_cli/tests/commands/check.rs (1)

3489-3531: Missing result assertion.

The other two new tests assert result.is_ok(), but this one silently drops the result status. Since the plugin emits a warning-level diagnostic and no --write is passed, it would be good to assert the expected outcome (likely is_err) for clarity and to catch regressions.

Proposed fix

     let (fs, result) = run_cli_with_server_workspace(
         fs,
         &mut console,
         Args::from(["check", file_path.as_str()].as_slice()),
     );
 
+    assert!(result.is_err(), "run_cli returned {result:?}");
     assert_file_contents(&fs, file_path, "console.log(\"hello\");\n");

crates/biome_plugin_loader/src/analyzer_grit_plugin.rs (2)

74-99: Consider extracting the duplicated closure into a helper.

The three match arms perform identical work (grab range + text + send-node), differing only in the downcast type. This is a pre-existing pattern, but the new fields make the duplication more visible.

Not blocking — just a future readability win.

---

140-146: All rewrite effects are converted to actions — even if the diagnostic has no span.

If a Grit pattern produces a rewrite effect but register_diagnostic was never called (or was called without a span), the action still gets created with the root-level source_range. This seems intentional for the current design, but worth noting that the action's source_range always covers the entire root node, not the matched sub-expression.

crates/biome_analyze/src/analyzer_plugin.rs (1)

127-153: Each diagnostic signal receives a clone of all actions — O(N×M) duplication.

When a plugin produces N diagnostics and M rewrite effects, every signal gets all M actions cloned onto it. For 3 console.log matches this means 3 signals × 3 actions = 9 AnalyzerActions emitted, even though only 3 are meaningful.

The fix-all loop and no-op guard likely handle this correctly, but:

1. It's wasteful (N full Vec clones).
2. It can confuse action counts in diagnostics/UX.

A future improvement could pair each diagnostic with its corresponding action(s) — e.g. by correlating the diagnostic span with the rewrite's source range.

Not blocking this PR, but worth tracking.

crates/biome_analyze/src/signals.rs (1)

188-196: Good doc comment on text_edit.

One small note: the unwrap_or_default() at the end of the or_else chain (lines 229 and 243) means a completely empty (TextRange::default(), TextEdit::default()) is produced if neither path yields a result. This is a safe fallback but could mask bugs where an action was expected to carry edits. Probably fine for now — just flagging.

crates/biome_grit_patterns/src/linearization.rs (1)

84-91: Overlapping or out-of-order replacements silently produce garbled output.

If two replacements overlap (i.e. *start < cursor after processing the previous replacement), the gap copy is silently skipped but the replacement text is still appended, leading to corrupted output. Consider adding a diagnostic or assertion when *start < cursor.

Defensive guard

     for (start, end, replacement) in &replacements {
+        if *start < cursor {
+            // Overlapping replacements — skip or error
+            continue;
+        }
         // Copy the gap from cursor to this replacement.
         if *start > cursor {
             result.push_str(&source[cursor..*start]);
         }

crates/biome_grit_patterns/src/grit_resolved_pattern.rs (1)

593-671: linearized_text() — clean delegation across all variants.

One small observation: the List variant joins with "," (no space) on line 636, while the Map variant joins with ", " (with space) on line 653. The text() method (line 772) also uses "," for lists and ", " for maps, so this is at least internally consistent. Just flagging in case the comma-only separator for lists was unintentional.

crates/biome_grit_patterns/src/linearization.rs (2)

84-91: Defensive guard for overlapping replacements.

If two replacements ever overlap (i.e. *start < cursor), the current loop silently stitches garbled text. get_top_level_effects should prevent this by contract, but a debug_assert! or skip would be cheap insurance against subtle bugs during future refactors.

Suggested defensive check

     for (start, end, replacement) in &replacements {
-        // Copy the gap from cursor to this replacement.
-        if *start > cursor {
+        // Skip overlapping replacements (should not happen with top-level effects).
+        if *start < cursor {
+            debug_assert!(false, "overlapping replacements detected: start={start}, cursor={cursor}");
+            continue;
+        }
+        if *start > cursor {
             result.push_str(&source[cursor..*start]);
         }

---

11-16: Consider extracting args into a struct to avoid the too_many_arguments suppression.

Seven parameters is a lot to thread around. A small context struct (e.g. LinearizationContext { language, effects, files, memo, logs }) would tidy call sites and remove the need for #[expect(clippy::too_many_arguments)]. No rush — just something to keep in mind if the signature grows further.