fix(lsp): apply diagnostic offset for plugin diagnostics in Vue/Astro/Svelte files#9012
Conversation
…/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 detectedLatest commit: 94933c7 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 |
WalkthroughReplaces Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
|
Clearly AI. Restore our template and adhere to it. Please disclose the use of AI |
|
@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? |
|
AI contributions are fine as long as they are disclosed in the PR description (this is in our PR template). |
|
Sounds good @dyc3, PR description updated to follow the template + AI disclosure at the top. |
|
also, please add a changeset |
There was a problem hiding this comment.
I would like to see a test for the LSP too
|
@dyc3 @ematipico addressed your concerns (I think):
|
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 inhandlers/analysis.rs) and pass it todiagnostic_to_lspinstead ofNone. Guarded bysupports_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
DiagnosticSignalwith a newPluginSignal<L>for plugin diagnostics.DiagnosticSignalconvertsRuleDiagnostic→Error→DiagnosticKind::Raw, which is skipped byadd_diagnostic_offset.PluginSignalpreservesDiagnosticKind::RuleviaAnalyzerDiagnostic::from(RuleDiagnostic).This doesn't change current behavior since
pull_diagnosticspassesdiagnostic_offset: Nonefor 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 ensuresadd_diagnostic_offsetwould work correctly if the offset strategy changes in the future.Test Plan
Added
check_plugin_diagnostic_offset_in_vue_fileCLI snapshot test: a GritQL plugin targetingfoobindings, 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.