-
-
Notifications
You must be signed in to change notification settings - Fork 720
fix(lsp): unsafe fixes when pulling code actions #7249
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
🦋 Changeset detectedLatest commit: 5ee54d0 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 |
WalkthroughRemoves gating and filtering in the LSP analysis handler that previously skipped quick fixes marked as MaybeIncorrect, ensuring such fixes are now returned. Updates server tests to include and validate an unsafe quick fix (noDoubleEquals) alongside suppression actions. Adds a changeset note documenting a patch release for @biomejs/biome and references fix for #748. No public API changes. Assessment against linked issues
Suggested labels
Suggested reviewers
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
Status, Documentation and Community
|
There was a problem hiding this 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 (3)
.changeset/fancy-plants-exist.md (1)
5-5
: Polish the wording for clarity.Tiny grammar/word-order tweak; keeps the meaning intact and reads more naturally in release notes.
-Fixed [#748](https://github.com/biomejs/biome-vscode/issues/748), where Biome Language Server didn't show the unsafe fixes when requesting the quick fixes. Now all LSP editors will show also opt-in, unsafe fixes. +Fixed [#748](https://github.com/biomejs/biome-vscode/issues/748), where the Biome Language Server didn't show unsafe fixes when requesting quick fixes. Now all LSP editors also show opt‑in unsafe fixes.crates/biome_lsp/src/server.tests.rs (2)
1554-1560
: Tidy the iterator: avoid the extra closure.Minor readability/idiomatic Rust improvement; no behavioural change.
- only: Some( - DEFAULT_CODE_ACTION_CAPABILITIES - .iter() - .map(|s| CodeActionKind::from(*s)) - .collect::<Vec<_>>(), - ), + only: Some( + DEFAULT_CODE_ACTION_CAPABILITIES + .iter() + .copied() + .map(CodeActionKind::from) + .collect(), + ),
1647-1661
: Consistent naming: “top_level” vs “toplevel”.Nitpick on naming consistency with other tests/vars in this file.
- let expected_toplevel_suppression_action = CodeActionOrCommand::CodeAction(CodeAction { + let expected_top_level_suppression_action = CodeActionOrCommand::CodeAction(CodeAction { @@ - expected_toplevel_suppression_action, + expected_top_level_suppression_action,
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
.changeset/fancy-plants-exist.md
(1 hunks)crates/biome_lsp/src/handlers/analysis.rs
(1 hunks)crates/biome_lsp/src/server.tests.rs
(3 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
.changeset/*.md
📄 CodeRabbit Inference Engine (CONTRIBUTING.md)
.changeset/*.md
: Create changesets withjust new-changeset
; store them in.changeset/
with correct frontmatter (package keys and change type).
In changeset descriptions, follow content conventions: user-facing changes only; past tense for what you did; present tense for current behavior; link issues for fixes; link rules/assists; include representative code blocks; end every sentence with a period.
When adding headers in a changeset, only use #### or ##### levels.
Files:
.changeset/fancy-plants-exist.md
**/*.{rs,toml}
📄 CodeRabbit Inference Engine (CONTRIBUTING.md)
Format Rust and TOML files before committing (use
just f
/just format
).
Files:
crates/biome_lsp/src/handlers/analysis.rs
crates/biome_lsp/src/server.tests.rs
crates/biome_*/**
📄 CodeRabbit Inference Engine (CLAUDE.md)
Place core crates under /crates/biome_*/
Files:
crates/biome_lsp/src/handlers/analysis.rs
crates/biome_lsp/src/server.tests.rs
crates/biome_service/../biome_lsp/src/server.tests.rs
📄 CodeRabbit Inference Engine (crates/biome_service/CONTRIBUTING.md)
Keep end-to-end LSP tests in biome_lsp’s server.tests.rs
Files:
crates/biome_lsp/src/server.tests.rs
🧠 Learnings (9)
📚 Learning: 2025-08-17T08:56:30.822Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-08-17T08:56:30.822Z
Learning: Applies to crates/biome_analyze/crates/**/lib/src/**/nursery/**/*.rs : If a rule provides a code action, add `fix_kind` in `declare_lint_rule!` and use `ctx.action_category(ctx.category(), ctx.group())` and `ctx.metadata().applicability()` when constructing actions
Applied to files:
crates/biome_lsp/src/handlers/analysis.rs
📚 Learning: 2025-08-11T11:46:05.836Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_diagnostics/CONTRIBUTING.md:0-0
Timestamp: 2025-08-11T11:46:05.836Z
Learning: Applies to crates/biome_diagnostics/**/*.rs : Use helper advice types from biome_diagnostics::v2 (CodeFrameAdvice, CommandAdvice, DiffAdvice, LogAdvice) when suitable
Applied to files:
crates/biome_lsp/src/handlers/analysis.rs
📚 Learning: 2025-08-17T08:56:30.822Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-08-17T08:56:30.822Z
Learning: Applies to crates/biome_analyze/crates/biome_js_analyze/tests/quick_test.rs : Use `biome_js_analyze/tests/quick_test.rs` for quick, ad-hoc testing; un-ignore the test and adjust the rule filter as needed
Applied to files:
crates/biome_lsp/src/handlers/analysis.rs
crates/biome_lsp/src/server.tests.rs
📚 Learning: 2025-08-17T08:56:30.822Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-08-17T08:56:30.822Z
Learning: Applies to crates/biome_analyze/crates/**/lib/src/**/nursery/**/*.rs : Use the local `rule_category!()` macro in diagnostics for the rule’s category, not string-parsed categories
Applied to files:
crates/biome_lsp/src/handlers/analysis.rs
📚 Learning: 2025-08-17T08:56:30.822Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-08-17T08:56:30.822Z
Learning: Applies to crates/biome_analyze/crates/biome_js_analyze/lib/src/lint/nursery/*.rs : For new JavaScript lint rules generated by `just new-js-lintrule`, implement the rule in the generated file under `biome_js_analyze/lib/src/lint/nursery/`
Applied to files:
crates/biome_lsp/src/handlers/analysis.rs
📚 Learning: 2025-08-17T08:56:30.822Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-08-17T08:56:30.822Z
Learning: Applies to crates/biome_analyze/crates/**/lib/src/**/nursery/**/*.rs : Use `domains` in `declare_lint_rule!` when applicable; recommended rules with domains enable only when the domain is active
Applied to files:
crates/biome_lsp/src/handlers/analysis.rs
📚 Learning: 2025-08-11T11:46:05.836Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_diagnostics/CONTRIBUTING.md:0-0
Timestamp: 2025-08-11T11:46:05.836Z
Learning: Applies to crates/biome_diagnostics/**/*.rs : Specify category and severity using #[diagnostic(...)] on the type or derive them from fields
Applied to files:
crates/biome_lsp/src/handlers/analysis.rs
📚 Learning: 2025-08-17T08:56:30.822Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-08-17T08:56:30.822Z
Learning: Applies to crates/biome_analyze/crates/biome_rule_options/lib/**/*.rs : Define per-rule options in the `biome_rule_options` crate under `lib/`, with serde- and schemars-compatible derives and `#[serde(rename_all = "camelCase", deny_unknown_fields, default)]`
Applied to files:
crates/biome_lsp/src/handlers/analysis.rs
📚 Learning: 2025-08-11T11:53:15.299Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_service/CONTRIBUTING.md:0-0
Timestamp: 2025-08-11T11:53:15.299Z
Learning: Applies to crates/biome_service/../biome_lsp/src/server.tests.rs : Keep end-to-end LSP tests in biome_lsp’s server.tests.rs
Applied to files:
crates/biome_lsp/src/server.tests.rs
🪛 LanguageTool
.changeset/fancy-plants-exist.md
[uncategorized] ~5-~5: A different word order might sound more natural.
Context: ...e quick fixes. Now all LSP editors will show also opt-in, unsafe fixes.
(AI_HYDRA_LEO_WORD_ORDER)
⏰ 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: Documentation
- GitHub Check: autofix
- GitHub Check: End-to-end tests
- GitHub Check: Test (depot-windows-2022-16)
- GitHub Check: Lint project (depot-ubuntu-24.04-arm-16)
- GitHub Check: Test (depot-ubuntu-24.04-arm-16)
- GitHub Check: Lint project (depot-windows-2022)
- GitHub Check: Check Dependencies
- GitHub Check: Test Node.js API
🔇 Additional comments (4)
crates/biome_lsp/src/handlers/analysis.rs (1)
11-11
: Import cleanup looks right.Dropping the unused Applicability import in favour of Error aligns with the removal of applicability-based gating. The subsequent debug print of action.suggestion.applicability remains valid via the workspace types.
crates/biome_lsp/src/server.tests.rs (3)
1498-1711
: Great regression test to ensure unsafe quick fixes are surfaced.This validates the exact scenario the issue reported. Insertion at Position { line: 0, character: 7 } correctly transforms "==" to "===". Nice.
1662-1695
: Edits for unsafe action are spot on.The change set inserts the extra "=" at the correct offset and uses the expected kind/title. Attaching the diagnostic keeps it visible under “Quick Fixes”.
1650-1650
: Clone to preserve ownership is correct.Cloning unsafe_fixable here avoids moving it earlier and keeps the later use valid.
This should also close biomejs/biome-vscode#751 right? |
Yes. Added it |
Hello. Any idea what's the minimum version of Biome and/or the VS Code extension that will have this? Thanks a lot. |
@Meligy it will be released in Biome 2.2.1. |
Co-authored-by: dyc3 <1808807+dyc3@users.noreply.github.com>
Co-authored-by: dyc3 <1808807+dyc3@users.noreply.github.com>
Co-authored-by: dyc3 <1808807+dyc3@users.noreply.github.com>
Summary
Closes biomejs/biome-vscode#748
Closes biomejs/biome-vscode#751
Now that we officially deprecated the use of
quickfix.biome
, we shouldn't use it anymore internally. The code we had was preventing the editors from opting for unsafe fixes.This PR fixes it.
Test Plan
Updated existing test. Manually tested against the reproduction.
Docs