[Sonic] Move a few more Razor features to be sonic compatible - #84069
Conversation
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Prefer the declaration generated document when present, fall back to the implementation document, and synthesize the render method symbol when Roslyn does not return it. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Enable the split-document document symbol tests and add component and legacy coverage for empty code scenarios. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Route C# selection range requests to the generated declaration or implementation document for each position, then map results back to the original request order. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Enable split-document selection range tests and add matching legacy coverage. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR updates several Razor cohosting LSP features to work correctly with declaration/implementation split generated C# documents by introducing a document-type marker (IsDeclarationDocument) and routing feature logic to the appropriate generated document (decl vs impl). It also expands cohosting endpoint test coverage to validate behavior for both component and legacy Razor file kinds.
Changes:
- Add
RazorCSharpDocument.IsDeclarationDocumentand plumb it through decl C# document generation to enable decl/impl-aware feature behavior. - Update Remote Razor services (Semantic Tokens, CodeLens, Document Symbols, Selection Range) to query and/or combine results across the correct generated document halves.
- Extend cohosting tests (CodeLens, Document Symbols, Selection Range) to cover legacy (
.cshtml) scenarios and additional document-shape cases.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/Razor/src/Razor/test/Microsoft.VisualStudioCode.RazorExtension.UnitTests/Endpoints/CohostCodeLensEndpointTest.cs | Adds legacy CodeLens test cases and parameterizes test document kind. |
| src/Razor/src/Razor/test/Microsoft.CodeAnalysis.Razor.CohostingShared.UnitTests/Endpoints/CohostSelectionRangeEndpointTest.cs | Enables previously skipped tests and adds legacy selection range coverage with file-kind routing. |
| src/Razor/src/Razor/test/Microsoft.CodeAnalysis.Razor.CohostingShared.UnitTests/Endpoints/CohostDocumentSymbolEndpointTest.cs | Enables tests, adds additional cases, and expands legacy document symbol coverage. |
| src/Razor/src/Razor/src/Microsoft.CodeAnalysis.Remote.Razor/SemanticTokens/RazorSemanticTokensInfoService.cs | Makes semantic tokens decl/impl aware and inlines C# semantic token retrieval via Roslyn helpers + telemetry. |
| src/Razor/src/Razor/src/Microsoft.CodeAnalysis.Remote.Razor/SemanticTokens/ICSharpSemanticTokensProvider.cs | Removes the C# semantic token provider interface (no longer needed after inlining). |
| src/Razor/src/Razor/src/Microsoft.CodeAnalysis.Remote.Razor/SemanticTokens/CSharpSemanticTokensProvider.cs | Removes the provider implementation (replaced by direct helper usage). |
| src/Razor/src/Razor/src/Microsoft.CodeAnalysis.Remote.Razor/SelectionRanges/RemoteSelectionRangeService.cs | Splits selection range requests by decl vs impl generated document and remaps results back to original order. |
| src/Razor/src/Razor/src/Microsoft.CodeAnalysis.Remote.Razor/DocumentSymbols/RemoteDocumentSymbolService.cs | Chooses the decl generated document when available for document symbol computation. |
| src/Razor/src/Razor/src/Microsoft.CodeAnalysis.Remote.Razor/DocumentSymbols/DocumentSymbolService.cs | Synthesizes render-method symbols when missing and improves symbol remapping for decl/impl scenarios. |
| src/Razor/src/Razor/src/Microsoft.CodeAnalysis.Remote.Razor/CodeLens/RemoteCodeLensService.cs | Routes CodeLens to the appropriate generated document and restores original resolve data before resolving. |
| src/Razor/src/Compiler/Microsoft.CodeAnalysis.Razor.Compiler/src/Language/RazorCSharpDocumentWriter.cs | Plumbs isDeclarationDocument into generated RazorCSharpDocument creation. |
| src/Razor/src/Compiler/Microsoft.CodeAnalysis.Razor.Compiler/src/Language/RazorCSharpDocument.cs | Adds IsDeclarationDocument to RazorCSharpDocument and constructor plumbing. |
| src/Razor/src/Compiler/Microsoft.CodeAnalysis.Razor.Compiler/src/Language/DefaultRazorDeclCSharpLoweringPhase.cs | Marks decl-half generated C# documents as declaration documents. |
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
| ImmutableArray<SourceMapping> sourceMappings = default, | ||
| ImmutableArray<LinePragma> linePragmas = default) | ||
| ImmutableArray<LinePragma> linePragmas = default, | ||
| bool isDeclarationDocument = false) |
There was a problem hiding this comment.
I think if anything, we could probably simplify this a lot more. The SourceText property used to be the only way to get the C# text, but with cohosting we can get it from the Roslyn solution snapshot. The diagnostics property is weird in sonic, since currently both documents duplicate all diagnostics, so I could see those moving up to RazorCodeDocument. I'm pretty sure linePragmas is only used for tests. Which means a RazorCSharpDocument could become a simple record that just stores source mappings and whether its a declaration document. In fact, I could see an argument for source mappings to store isDeclarationDocument, and have them just be a property on RazorCodeDocument and make this unnecessary altogether. Though I've no idea what impact that would have on the compiler, I'm not as familiar with that codebase.
I definitely still want to improve the architecture here, along with the DocumentContext and DocumentSnapshot stuff, but the longer the sonic feature branch lives, the more stressed I become about conflicts.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Go to definition is a little more complex because I also moved things from Workspaces to Remote as part of it. ###### Microsoft Reviewers: [Open in CodeFlow](https://microsoft.github.io/open-pr/?codeflow=https://github.com/dotnet/roslyn/pull/84102)
Similar to edit service, except even bigger, and definitely big enough for its own PR. Hopefully the last of the "big" services to move that other things will depend on. Commit-at-a-time is probably the easiest, even though some of them are pretty small.
This is a reasonable chunk of work on its own, and is pivotal to a few other things, so putting it up on its own. We can't move `IRazorEditService` itself yet, sadly, because formatting needs to use it. Formatting is probably the next PR though. Commit at a time will probably be easiest. ###### Microsoft Reviewers: [Open in CodeFlow](https://microsoft.github.io/open-pr/?codeflow=https://github.com/dotnet/roslyn/pull/84086)
Another few smallish changes, best reviewed commit at a time I guess. ###### Microsoft Reviewers: [Open in CodeFlow](https://microsoft.github.io/open-pr/?codeflow=https://github.com/dotnet/roslyn/pull/84075)
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 161 out of 161 changed files in this pull request and generated 3 comments.
Comments suppressed due to low confidence (1)
src/Razor/src/Razor/src/Microsoft.CodeAnalysis.Remote.Razor/Formatting/Passes/CSharpFormattingPass.cs:3
- The file appears to start with an empty line (likely due to a stray BOM), which pushes the license header down to line 2. This looks accidental and makes the header inconsistent with other files. Please remove the leading blank line so the license header is the first line.
| // This calls Roslyn's Go To Definition handler directly to verify that Roslyn can call Razor's span mapping service. | ||
| // A real Go To Definition request from a C# file doesn't go through Razor's remote service. Since both the impl and | ||
| // decl generated documents contain the component symbol, Roslyn maps both results back to the same Razor location here. | ||
| // TODO: Check if we need to de-dupe on the Roslyn side, after span mapping, or if Roslyn/VS/VS Code takes care of it already. | ||
| Assert.Equal(2, definition.Length); |
Did a few "small"ish ones in one go today, and also one simple compiler change to make things easier. We now have a property on RazorCSharpDocument so things can tell what type of document they're given, rather than also keeping booleans everywhere. That also meant I could simplify semantic tokens a bunch. Also did CodeLens, Document Symbols and Selection Range. They're all reasonably simple, in fact selection range is the only one that actually cares about both document types.
Commit-at-a-time probably makes sense, given there are 3 features in one PR here. Let me know if you'd rather I split them up.
Microsoft Reviewers: Open in CodeFlow