Skip to content

fix: incorrect logic for CSS fix all#7699

Merged
ematipico merged 1 commit intonextfrom
fix/test-snippets
Oct 6, 2025
Merged

fix: incorrect logic for CSS fix all#7699
ematipico merged 1 commit intonextfrom
fix/test-snippets

Conversation

@ematipico
Copy link
Member

Summary

My last PR introduced a regression. This PR fixes it

Test Plan

The previous failing test should_apply_fixes_to_embedded_languages should pass

Docs

@changeset-bot
Copy link

changeset-bot bot commented Oct 6, 2025

⚠️ No Changeset found

Latest commit: f34ddb7

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@github-actions github-actions bot added the A-Project Area: project label Oct 6, 2025
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Oct 6, 2025

Walkthrough

The fix_all logic in crates/biome_service/src/file_handlers/css.rs was adjusted so that FixFileMode::SafeAndUnsafeFixes now skips suppression actions, matching the behaviour of SafeFixes. Suppression actions are ignored in both modes, with no changes to error counting or mutation logic. No public or exported declarations were modified.

Suggested labels

L-CSS

Suggested reviewers

  • siketyan

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Title Check ✅ Passed The title clearly summarises the core change by indicating a fix to the CSS fix-all logic and directly reflects the modifications made in the CSS handler.
Description Check ✅ Passed The description relates directly to the changeset by explaining the regression introduced by the prior PR and outlining the test plan for the fix, so it is on-topic and sufficient for review.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix/test-snippets

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (1)
crates/biome_service/src/file_handlers/css.rs (1)

715-718: Fix correctly addresses the regression.

The logic now properly skips suppression actions in SafeAndUnsafeFixes mode, matching the behaviour in SafeFixes. This resolves the regression as intended.

However, the identical suppression check now appears in both branches (lines 703–705 and 715–718). Consider extracting this into a helper or restructuring the match arms to reduce duplication:

 for action in signal.actions() {
+    // Suppression actions should not be part of safe or unsafe fixes
+    if !matches!(params.fix_file_mode, FixFileMode::ApplySuppressions)
+        && action.is_suppression()
+    {
+        continue;
+    }
+
     match params.fix_file_mode {
         FixFileMode::SafeFixes => {
-            // suppression actions should not be part of safe fixes
-            if action.is_suppression() {
-                continue;
-            }
             if action.applicability == Applicability::MaybeIncorrect {
                 skipped_suggested_fixes += 1;
             }
             if action.applicability == Applicability::Always {
                 errors = errors.saturating_sub(1);
                 return ControlFlow::Break(action);
             }
         }
         FixFileMode::SafeAndUnsafeFixes => {
-            // suppression actions should not be part of safe fixes
-            if action.is_suppression() {
-                continue;
-            }
             if matches!(
                 action.applicability,
                 Applicability::Always | Applicability::MaybeIncorrect
             ) {
                 errors = errors.saturating_sub(1);
                 return ControlFlow::Break(action);
             }
         }
         FixFileMode::ApplySuppressions => {
             if action.is_suppression() {
                 return ControlFlow::Break(action);
             }
         }
     }
 }

Minor: The comment at line 715 mentions "safe fixes" but sits in the SafeAndUnsafeFixes branch—consider "safe or unsafe fixes" for precision.

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 461c478 and f34ddb7.

📒 Files selected for processing (1)
  • crates/biome_service/src/file_handlers/css.rs (1 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/css.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/css.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/css.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: Documentation
  • GitHub Check: Test (depot-ubuntu-24.04-arm-16)
  • GitHub Check: Check Dependencies
  • GitHub Check: Lint project (depot-windows-2022)
  • GitHub Check: Lint project (depot-ubuntu-24.04-arm-16)
  • GitHub Check: autofix

@ematipico ematipico merged commit 6bd1e3b into next Oct 6, 2025
9 of 11 checks passed
@ematipico ematipico deleted the fix/test-snippets branch October 6, 2025 09:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-Project Area: project

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant