Skip to content

fix(lsp): scan injected languages for diagnostics#2528

Merged
HerringtonDarkholme merged 1 commit into
mainfrom
fix-lsp-injection
Mar 11, 2026
Merged

fix(lsp): scan injected languages for diagnostics#2528
HerringtonDarkholme merged 1 commit into
mainfrom
fix-lsp-injection

Conversation

@HerringtonDarkholme

@HerringtonDarkholme HerringtonDarkholme commented Mar 10, 2026

Copy link
Copy Markdown
Member

Summary by CodeRabbit

  • New Features

    • Diagnostics now scan multiple injected languages in a document and apply language-specific rules per injected context.
  • Refactor

    • Public language interface updated to require parse-from-string capability, aligning implementations with the new trait requirement (public API update).
  • Tests

    • Added test coverage for injected-language diagnostics and code-action fix application.

@coderabbitai

coderabbitai Bot commented Mar 10, 2026

Copy link
Copy Markdown
Contributor
📝 Walkthrough

Walkthrough

The LSP diagnostic flow now performs per-injected-language scans instead of a single global pass and requires FromStr on LSPLang. Diagnostics and fixes are aggregated across injected-language contexts; tests added to verify TypeScript-in-HTML diagnostics and code actions.

Changes

Cohort / File(s) Summary
LSP Trait & Diagnostic Scanning
crates/lsp/src/lib.rs
Adds FromStr bound to LSPLang trait and its impl; refactors diagnostics to gather injected languages, fetch language-specific rules, run per-injected-language scans, and aggregate diagnostics and fixes.
Injected Language Test
crates/lsp/tests/basic.rs
Adds async test test_injected_language_diagnostics that opens HTML with injected TypeScript, waits for diagnostics, requests and applies a code action, and asserts the applied fix (alert → console.log).

Sequence Diagram

sequenceDiagram
    participant Client as LSP Client
    participant Server as LSP Server
    participant Gatherer as Injection Gatherer
    participant Rules as Rule Engine
    participant Scanner as Per-Lang Scanner
    participant Aggregator as Result Aggregator

    Client->>Server: Request diagnostics for document (HTML with TS)
    Server->>Gatherer: Collect injected-language contexts
    Gatherer-->>Server: [TypeScript, ...]
    loop for each injected language
        Server->>Rules: Get rules for language
        Rules-->>Server: Language-specific rule refs
        Server->>Scanner: Run scan on injected subtree
        Scanner-->>Server: (rule, matches) pairs
        Server->>Aggregator: Add diagnostics & fixes
    end
    Aggregator-->>Server: Aggregated diagnostics & code actions
    Server-->>Client: Publish diagnostics & respond to code action requests
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~22 minutes

Possibly related issues

Possibly related PRs

Poem

🐰
I hop through nested code today,
Finding rules where scripts may play.
TypeScript tucked inside HTML’s nest,
Scanned with care and fixed the rest.
Hooray for diagnostics that know the way!

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 75.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'fix(lsp): scan injected languages for diagnostics' directly describes the main change—enabling language-injection-aware diagnostics scanning. It is clear, specific, and accurately represents the primary modification to the LSP diagnostic flow.

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

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix-lsp-injection

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.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@crates/lsp/src/lib.rs`:
- Around line 291-297: The code currently returns early when
rule_refs.is_empty(), which skips calling
CombinedScan::set_unused_suppression_rule and loses unused-suppression hints for
languages with zero rules; instead ensure the unused-suppression rule is always
set: create the unused_suppression_rule via
CombinedScan::unused_config(Severity::Hint, injected.lang().clone()) and call
CombinedScan::new(rule_refs) (or create an empty CombinedScan) and then call
scan.set_unused_suppression_rule(&unused_suppression_rule) before any early
continue or skip logic so that set_unused_suppression_rule runs even when
rule_refs.is_empty().

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 65bd0178-1fd1-45cf-8875-6a03f4c8eb01

📥 Commits

Reviewing files that changed from the base of the PR and between 1c1e989 and a119b4d.

📒 Files selected for processing (2)
  • crates/lsp/src/lib.rs
  • crates/lsp/tests/basic.rs

Comment thread crates/lsp/src/lib.rs

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

♻️ Duplicate comments (1)
crates/lsp/src/lib.rs (1)

290-297: ⚠️ Potential issue | 🟡 Minor

Don’t skip unused-suppression scanning when a language has no rules.

The continue on Line 291 still bypasses set_unused_suppression_rule, so host or injected regions with zero configured rules lose the unused-suppression hint entirely. CombinedScan::new(rule_refs) needs to run even for an empty rule set here.

Suggested fix
       let rule_refs = rules.get_rule_from_lang(&path, injected.lang().clone());
-      if rule_refs.is_empty() {
-        continue;
-      }
       let unused_suppression_rule =
         CombinedScan::unused_config(Severity::Hint, injected.lang().clone());
       let mut scan = CombinedScan::new(rule_refs);
       scan.set_unused_suppression_rule(&unused_suppression_rule);
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@crates/lsp/src/lib.rs` around lines 290 - 297, The current early return skips
creating a CombinedScan and setting the unused-suppression hint when
rules.get_rule_from_lang(&path, injected.lang().clone()) yields an empty
rule_refs; remove the continue and always construct CombinedScan::new(rule_refs)
(even when empty), then call
scan.set_unused_suppression_rule(&unused_suppression_rule) using
CombinedScan::unused_config(Severity::Hint, injected.lang().clone()); this
ensures the unused-suppression hint is applied for host or injected regions with
zero configured rules while preserving existing logic that processes non-empty
rule sets.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Duplicate comments:
In `@crates/lsp/src/lib.rs`:
- Around line 290-297: The current early return skips creating a CombinedScan
and setting the unused-suppression hint when rules.get_rule_from_lang(&path,
injected.lang().clone()) yields an empty rule_refs; remove the continue and
always construct CombinedScan::new(rule_refs) (even when empty), then call
scan.set_unused_suppression_rule(&unused_suppression_rule) using
CombinedScan::unused_config(Severity::Hint, injected.lang().clone()); this
ensures the unused-suppression hint is applied for host or injected regions with
zero configured rules while preserving existing logic that processes non-empty
rule sets.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: a48e2562-6841-4d7f-a44c-3b84af7c6e8c

📥 Commits

Reviewing files that changed from the base of the PR and between a119b4d and 8e6230f.

📒 Files selected for processing (2)
  • crates/lsp/src/lib.rs
  • crates/lsp/tests/basic.rs
🚧 Files skipped from review as they are similar to previous changes (1)
  • crates/lsp/tests/basic.rs

@codecov

codecov Bot commented Mar 10, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 84.48%. Comparing base (3830aa6) to head (8e6230f).
⚠️ Report is 3 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #2528   +/-   ##
=======================================
  Coverage   84.47%   84.48%           
=======================================
  Files         113      113           
  Lines       18685    18686    +1     
=======================================
+ Hits        15784    15786    +2     
+ Misses       2901     2900    -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@HerringtonDarkholme HerringtonDarkholme added this pull request to the merge queue Mar 11, 2026
Merged via the queue into main with commit 6795104 Mar 11, 2026
6 checks passed
@HerringtonDarkholme HerringtonDarkholme deleted the fix-lsp-injection branch March 11, 2026 03:25
nicopauss pushed a commit to Intersec/lib-common that referenced this pull request Apr 1, 2026
##### [\`v0.42.0\`](https://github.com/ast-grep/ast-grep/blob/HEAD/CHANGELOG.md#0420)

- chore(deps): update dependency dprint to v0.53.0 [`#2547`](ast-grep/ast-grep#2547)
- chore(deps): update rust crate tree-sitter to v0.26.7 [`#2541`](ast-grep/ast-grep#2541)
- chore(deps): update dependency web-tree-sitter to v0.26.7 [`#2540`](ast-grep/ast-grep#2540)
- chore(deps): update dependency [@types/node](https://github.com/types/node) to v24.12.0 [`#2518`](ast-grep/ast-grep#2518)
- fix(deps): update rust crate tree-sitter-lua to 0.5.0 [`#2506`](ast-grep/ast-grep#2506)
- chore(deps): update rust crate clap to v4.6.0 [`#2538`](ast-grep/ast-grep#2538)
- fix(deps): update rust crate tree-sitter-scala to 0.25.0 [`#2536`](ast-grep/ast-grep#2536)
- chore(deps): update dependency oxlint to v1.55.0 [`#2533`](ast-grep/ast-grep#2533)
- chore(deps): update rust crate clap\_complete to v4.6.0 [`#2539`](ast-grep/ast-grep#2539)
- chore(deps): update rust crate bit-set to v0.9.1 [`#2537`](ast-grep/ast-grep#2537)
- chore(deps): update rust crate bit-set to 0.9.0 [`#2527`](ast-grep/ast-grep#2527)
- chore(deps): update rust crate assert\_cmd to v2.2.0 [`#2529`](ast-grep/ast-grep#2529)
- chore(deps): update rust crate tempfile to v3.27.0 [`#2531`](ast-grep/ast-grep#2531)
- fix(lsp): scan injected languages for diagnostics [`#2528`](ast-grep/ast-grep#2528)
- chore(deps): update dependency [@ast-grep/napi](https://github.com/ast-grep/napi) to v0.41.1 [`#2526`](ast-grep/ast-grep#2526)
- chore(deps): update dependency oxlint to v1.52.0 [`#2524`](ast-grep/ast-grep#2524)
- feat: support nth-child esquery [`#2546`](ast-grep/ast-grep#2546)
- feat: support :is selector [`#2545`](ast-grep/ast-grep#2545)
- feat: support :not selector [`#2544`](ast-grep/ast-grep#2544)
- feat: support :has rule [`#2543`](ast-grep/ast-grep#2543)
- fix(lsp): scan injected languages for diagnostics ([#2528](ast-grep/ast-grep#2528)) [`#2522`](ast-grep/ast-grep#2522)
- feat: add parameterized util [`3d90372`](ast-grep/ast-grep@3d90372)
- refactor: limit parameterized utils to globals [`2d69a34`](ast-grep/ast-grep@2d69a34)
- refactor: move parameterized\_util [`c77e38d`](ast-grep/ast-grep@c77e38d)
##### [\`v0.41.1\`](https://github.com/ast-grep/ast-grep/blob/HEAD/CHANGELOG.md#0411)

> 10 March 2026

- fix: lsp on change encounter deadlock [`#2511`](ast-grep/ast-grep#2511)
- chore(deps): update dependency oxlint to v1.51.0 [`#2512`](ast-grep/ast-grep#2512)
- chore(deps): update rust crate tempfile to v3.26.0 [`#2497`](ast-grep/ast-grep#2497)
- chore(deps): update rust crate inquire to v0.9.4 [`#2498`](ast-grep/ast-grep#2498)
- chore(deps): update dependency [@types/node](https://github.com/types/node) to v24.11.0 [`#2502`](ast-grep/ast-grep#2502)
- chore(deps): update dependency dprint to v0.52.0 [`#2499`](ast-grep/ast-grep#2499)
- fix: override severity on `rule` & `inline-rules` flags [`#2505`](ast-grep/ast-grep#2505)
- chore(deps): update github artifact actions [`#2507`](ast-grep/ast-grep#2507)
- chore(deps): update rust crate tree-sitter to v0.26.6 [`#2501`](ast-grep/ast-grep#2501)
- chore(deps): update dependency web-tree-sitter to v0.26.6 [`#2500`](ast-grep/ast-grep#2500)
- chore(deps): update dependency ava to v7 [`#2508`](ast-grep/ast-grep#2508)
- chore(deps): update dependency oxlint to v1.50.0 [`#2495`](ast-grep/ast-grep#2495)
- chore(deps): update dependency [@ast-grep/napi](https://github.com/ast-grep/napi) to v0.41.0 [`#2494`](ast-grep/ast-grep#2494)
- fix: bump ls-types version [`#2525`](ast-grep/ast-grep#2525)
- refactor: change versioned ast work [`a86b2ab`](ast-grep/ast-grep@a86b2ab)
- fix: bump wasm-bindgen [`d23e334`](ast-grep/ast-grep@d23e334)
- fix: fix wasm package [`0b92e94`](ast-grep/ast-grep@0b92e94)

Renovate-Branch: renovate/2024.6-ast-grep-cli-0.x
Change-Id: I91bcfa93167c7af0833afeb42df774621bad595c
Priv-Id: ef06dc14816a639036bb167212251f765ce38ad8
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant