Skip to content

fix(lsp): apply diagnostic offset for plugin diagnostics in Vue/Astro/Svelte files#9012

Merged
ematipico merged 4 commits intobiomejs:mainfrom
kiroushi:fix/plugin-diagnostic-offset-vue-sfc
Feb 12, 2026
Merged

fix(lsp): apply diagnostic offset for plugin diagnostics in Vue/Astro/Svelte files#9012
ematipico merged 4 commits intobiomejs:mainfrom
kiroushi:fix/plugin-diagnostic-offset-vue-sfc

Conversation

@kiroushi
Copy link
Copy Markdown
Contributor

@kiroushi kiroushi commented Feb 9, 2026

AI disclosure: This PR was developed with AI assistance (Claude) for the Rust implementation. The changes were reviewed manually.

Summary

Plugin diagnostics (GritQL) in Vue/Astro/Svelte SFC files report incorrect positions in the LSP — the byte offsets are relative to the extracted <script> block but are sent to the editor without adding the script block's offset within the file, causing diagnostics to appear in the <template> section instead of the <script> section.

LSP fix (biome_lsp)

In update_diagnostics_for_document, compute the script block offset for Vue/Astro/Svelte files (matching the existing pattern in handlers/analysis.rs) and pass it to diagnostic_to_lsp instead of None. Guarded by supports_full_html_support() since full HTML mode produces file-relative positions that don't need adjustment.

This is the fix that changes behavior and resolves the bug.

Analyzer correctness (biome_analyze)

Replace DiagnosticSignal with a new PluginSignal<L> for plugin diagnostics. DiagnosticSignal converts RuleDiagnosticErrorDiagnosticKind::Raw, which is skipped by add_diagnostic_offset. PluginSignal preserves DiagnosticKind::Rule via AnalyzerDiagnostic::from(RuleDiagnostic).

This doesn't change current behavior since pull_diagnostics passes diagnostic_offset: None for Vue files — the offset is applied post-hoc by the CLI and LSP layers. However, it's the correct semantic representation (plugin diagnostics are rule diagnostics) and ensures add_diagnostic_offset would work correctly if the offset strategy changes in the future.

Test Plan

Added check_plugin_diagnostic_offset_in_vue_file CLI snapshot test: a GritQL plugin targeting foo bindings, run against a Vue file with a multi-line <template> preceding the <script>. The snapshot confirms both the lint and plugin diagnostics point to the correct line in <script>.

Docs

No user-facing changes, this is a bugfix for existing plugin diagnostic behavior in SFC files.

…/Svelte files

Plugin diagnostics in Vue, Astro, and Svelte SFC files reported incorrect
positions in the LSP. The diagnostic spans pointed into the <template>
section instead of the <script> section because the embedded language
byte offset was not applied to the diagnostics.

Root cause: Session::update_diagnostics_for_document called
diagnostic_to_lsp with offset: None for all files, including
Vue/Astro/Svelte SFCs where the JavaScript content is extracted from
the <script> block with a byte offset from the start of the file.

Fixes:

1. LSP session (session.rs): Compute the script block byte offset for
   Vue/Astro/Svelte files and pass it to diagnostic_to_lsp, matching
   the existing pattern in handlers/analysis.rs and handlers/formatting.rs.

2. Analyzer plugin signal (signals.rs): Introduce PluginSignal<L> that
   preserves DiagnosticKind::Rule instead of converting through
   DiagnosticSignal which produces DiagnosticKind::Raw. This ensures
   add_diagnostic_offset correctly adjusts plugin diagnostic spans in
   the embedded HTML content path.
@changeset-bot
Copy link
Copy Markdown

changeset-bot Bot commented Feb 9, 2026

🦋 Changeset detected

Latest commit: 94933c7

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 13 packages
Name Type
@biomejs/biome Patch
@biomejs/cli-win32-x64 Patch
@biomejs/cli-win32-arm64 Patch
@biomejs/cli-darwin-x64 Patch
@biomejs/cli-darwin-arm64 Patch
@biomejs/cli-linux-x64 Patch
@biomejs/cli-linux-arm64 Patch
@biomejs/cli-linux-x64-musl Patch
@biomejs/cli-linux-arm64-musl Patch
@biomejs/wasm-web Patch
@biomejs/wasm-bundler Patch
@biomejs/wasm-nodejs Patch
@biomejs/backend-jsonrpc Patch

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

@github-actions github-actions Bot added A-CLI Area: CLI A-Linter Area: linter A-LSP Area: language server protocol labels Feb 9, 2026
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Feb 9, 2026

Walkthrough

Replaces DiagnosticSignal with a new PluginSignal<L> that owns a RuleDiagnostic and implements AnalyzerSignal<L> (returns a diagnostic; actions and transformations empty). Updates analyzer plugin wiring to construct PluginSignal by value. Exports PluginSignal from the crate. Adds tests and changes in CLI/LSP session code to compute and apply start offsets for Vue/Astro/Svelte files so plugin diagnostics map into the full document (script/frontmatter offsets). Includes a changelog entry for the fix.

Suggested labels

A-Diagnostic, L-JavaScript, L-HTML

Suggested reviewers

  • dyc3
  • ematipico
  • arendjr
🚥 Pre-merge checks | ✅ 2
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately reflects the main change: fixing LSP diagnostic offsets for plugin diagnostics in Vue/Astro/Svelte files.
Description check ✅ Passed The description is comprehensive and directly related to the changeset, explaining the bug, the fix, analyser correctness improvements, tests added, and AI disclosure.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

No actionable comments were generated in the recent review. 🎉


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

@ematipico
Copy link
Copy Markdown
Member

Clearly AI. Restore our template and adhere to it. Please disclose the use of AI

@kiroushi
Copy link
Copy Markdown
Contributor Author

@ematipico That's correct, I used CC to help with the implementation as a opportunistic fix to an internal issue. I'll update the PR to follow the template.

That said, I did review the changes and the fix itself looks correct to me, though I'm not a Rust expert. Are AI contributions acceptable here as long as they're properly disclosed and reviewed, or would you prefer I close this?

@dyc3
Copy link
Copy Markdown
Contributor

dyc3 commented Feb 10, 2026

AI contributions are fine as long as they are disclosed in the PR description (this is in our PR template).

@kiroushi
Copy link
Copy Markdown
Contributor Author

Sounds good @dyc3, PR description updated to follow the template + AI disclosure at the top.

Comment thread crates/biome_analyze/src/signals.rs Outdated
Comment thread crates/biome_cli/tests/commands/check.rs Outdated
@dyc3
Copy link
Copy Markdown
Contributor

dyc3 commented Feb 10, 2026

also, please add a changeset

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I would like to see a test for the LSP too

@kiroushi
Copy link
Copy Markdown
Contributor Author

@dyc3 @ematipico addressed your concerns (I think):

  • Removed L: Language bound from PluginSignal struct.
  • Changed the CLI test to lint.
  • Added the changeset.
  • Added a LSP test in server.tests.rs for Vue files. Maybe an Astro/Svelte test too would be good here?

Comment thread crates/biome_analyze/src/signals.rs
@kiroushi kiroushi requested a review from ematipico February 11, 2026 13:53
Copy link
Copy Markdown
Member

@ematipico ematipico left a comment

Choose a reason for hiding this comment

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

Thank you!

@ematipico ematipico merged commit 18cdd45 into biomejs:main Feb 12, 2026
14 checks passed
@github-actions github-actions Bot mentioned this pull request Feb 12, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-CLI Area: CLI A-Linter Area: linter A-LSP Area: language server protocol

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants