Skip to content

Fix route document highlights across partial-class files#66770

Draft
Copilot wants to merge 3 commits into
mainfrom
copilot/fix-document-highlights-issue
Draft

Fix route document highlights across partial-class files#66770
Copilot wants to merge 3 commits into
mainfrom
copilot/fix-document-highlights-issue

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented May 20, 2026

Fix route document highlights across partial-class files

  • You've read the Contributor Guide and Code of Conduct.
  • You've included unit or integration tests for your change, where applicable.
  • You've included inline docs for your change, where applicable.
  • There's an open issue for the PR that you are making. If you'd like to propose a new feature or change, please open an issue to discuss the change or find an existing issue.

Prevent cross-tree semantic lookup in route highlighter

Description

Document highlights could throw Syntax node is not within syntax tree when a Minimal API route mapping and its handler method are in different files of the same partial class. This change makes symbol resolution tree-correct and adds a regression test for that shape.

  • Analyzer fix (RoutePatternHighlighter)

    • Resolve identifier symbols with the semantic model for the current method syntax tree, not always the route file’s semantic model.
    • Keeps same behavior for same-tree cases; switches to Compilation.GetSemanticModel(...) for cross-file declarations.
  • Regression coverage

    • Added a focused test for MapPost(..., LoadAsync) where Map... and LoadAsync are in separate partial-class files.
    • Verifies highlights are produced without exception.
var methodSemanticModel = methodSyntax.SyntaxTree == semanticModel.SyntaxTree
    ? semanticModel
    : semanticModel.Compilation.GetSemanticModel(methodSyntax.SyntaxTree);

Copilot AI changed the title [WIP] Fix document highlights internal error in VS2026 Fix route document highlights across partial-class files May 20, 2026
Copilot AI requested a review from danroth27 May 20, 2026 23:18
@github-actions github-actions Bot added the needs-area-label Used by the dotnet-issue-labeler to label those issues which couldn't be triaged automatically label May 20, 2026
AspNetCoreDocumentHighlights cannot carry a Document reference, so every
highlight span the route highlighter emits must belong to the active
document. Spans from other syntax trees would be mis-mapped into the
active document and highlight unrelated text.

- Skip parameter/method DeclaringSyntaxReferences whose SyntaxTree
  differs from semanticModel.SyntaxTree. This obsoletes the per-tree
  semantic model branching introduced earlier in this PR.
- Tighten the cross-file regression test to assert exactly the route
  literal span is highlighted (no stray spans from the handler file).
- Rename the test to describe the desired behavior.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@danroth27
Copy link
Copy Markdown
Member

Pushed a follow-up commit (14bb9c2) that addresses the latent correctness issue and tightens the test.

Why a follow-up was needed

AspNetCoreDocumentHighlights cannot carry a Document reference (see Roslyn source), so every span returned by RoutePatternHighlighter.GetDocumentHighlights must belong to the active document (the tree that owns semanticModel). The previous diff fixed the GetSymbolInfo crash but still emitted spans for:

  • parameterSyntax.Identifier.Span from the parameter declaration in the handler file
  • reference.Identifier.Span from identifier references inside the handler method body

Both spans are TextSpans in the handler file's tree. Reporting them as highlights in the route file would mis-highlight whatever text happens to sit at those offsets in the route file (or be out of range). The crash got masked behind the bogus highlights.

Change

In HighlightSymbol, skip DeclaringSyntaxReferences whose SyntaxTree != semanticModel.SyntaxTree in both loops. This also obsoletes the methodSemanticModel per-tree branching introduced earlier in this PR — once we filter to same-tree only, the original semanticModel is always correct.

Trade-off: the parameter highlight in the other file is not surfaced. That's a feature gap rather than a bug — the API simply has no way to express cross-document highlights from an embedded language highlighter. The route literal span is still highlighted, and there is no crash. A complete cross-file highlighting experience would require extending Roslyn's embedded-language highlighter contract; out of scope here.

Test

  • Renamed …_DoesNotThrow…_HighlightsOnlyRouteLiteral to describe the desired behavior.
  • Asserts the result is exactly {routeSpan} rather than Assert.Contains(... routeSpan). The previous assertion would have passed even if a stray (incorrect) span from the handler file leaked into the result. The stricter assertion would have caught the latent bug.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs-area-label Used by the dotnet-issue-labeler to label those issues which couldn't be triaged automatically

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Document highlights: Syntax node is not within syntax tree

3 participants