[Sonic] Support multiple C# generated documents in diagnostics - #83989
Conversation
I think it's 50/50 whether these helpers make it to the main branch, but for now it's really convenient to be able to obsolete all of the old methods, and be able to see which code has been modified to be explicit, and which are just using the old methods. I realise, in hindsight, that it was my own comment on a previous PR that caused things to be in this state 🤦♂️
…e end mapped result
There was a problem hiding this comment.
Oops, didn't mean to commit this. This is part of getting code actions to work with Sonic, which is what I decided to start with, but turns out code actions depends on diagnostics, so I changed direction a little.
There was a problem hiding this comment.
Pull request overview
This PR updates Razor cohosting diagnostics plumbing to handle two source-generated C# documents (implementation + declaration) and adjusts diagnostics translation/deduplication accordingly, re-enabling several previously-skipped tests and adding coverage for new split-document scenarios.
Changes:
- Plumb separate impl/decl C# diagnostics/task-items through
IRemoteDiagnosticsServiceand cohost endpoints. - Update diagnostic translation to filter/deduplicate unused-using diagnostics across impl/decl documents and map each document independently.
- Re-enable and expand diagnostics/organize-usings test coverage for split generated documents (including no-code-block and implicit-expression cases).
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| src/Razor/src/Razor/test/Microsoft.VisualStudioCode.RazorExtension.UnitTests/Endpoints/CohostDocumentPullDiagnosticsTest.cs | Re-enables VS Code pull-diagnostics test now that unused-using dedup is implemented. |
| src/Razor/src/Razor/test/Microsoft.VisualStudio.LanguageServices.Razor.UnitTests/Cohost/OrganizeUsingsCommandTest.cs | Re-enables Organize Usings command tests and adds a no-code-block scenario. |
| src/Razor/src/Razor/test/Microsoft.VisualStudio.LanguageServices.Razor.UnitTests/Cohost/CohostDocumentPullDiagnosticsTest.cs | Re-enables VS cohost diagnostics tests and extends TODO/task-list coverage across impl/decl. |
| src/Razor/src/Razor/test/Microsoft.CodeAnalysis.Razor.CohostingShared.UnitTests/Endpoints/CohostDocumentPullDiagnosticsTest.cs | Re-enables shared diagnostics tests and adds implicit-expression + no-code-block cases. |
| src/Razor/src/Razor/src/Microsoft.VisualStudio.LanguageServices.Razor/OrganizeUsingsCommand.cs | Updates organize-usings flow to fetch and forward impl+decl diagnostics to remote filtering/hydration. |
| src/Razor/src/Razor/src/Microsoft.VisualStudio.LanguageServices.Razor/LanguageClient/Cohost/CohostDocumentPullDiagnosticsEndpoint.cs | Splits C# task-list item collection across impl/decl generated docs and forwards both to OOP. |
| src/Razor/src/Razor/src/Microsoft.CodeAnalysis.Remote.Razor/Diagnostics/RemoteDiagnosticsService.cs | Updates remote diagnostics/task-list entrypoints to accept impl+decl inputs and delegate to new translation APIs. |
| src/Razor/src/Razor/src/Microsoft.CodeAnalysis.Remote.Razor/Diagnostics/RazorTranslateDiagnosticsService.cs | Adds C# impl/decl translation and unused-using cross-document filtering; separates HTML translation. |
| src/Razor/src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/Remote/IRemoteDiagnosticsService.cs | Extends remote interface signatures to carry impl+decl diagnostics/task-items. |
| src/Razor/src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/Extensions/ProjectExtensions.cs | Adds helper to locate both impl+decl source-generated Razor documents by hint name. |
| src/Razor/src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/DocumentMapping/DocumentPositionInfo.cs | Extends position info with an inDeclDocument flag for split-document scenarios. |
| src/Razor/src/Razor/src/Microsoft.CodeAnalysis.Razor.CohostingShared/Diagnostics/CohostDocumentPullDiagnosticsEndpointBase.cs | Updates shared cohost endpoint base to gather impl+decl C# diagnostics and forward both to OOP. |
| src/Razor/src/Compiler/Microsoft.CodeAnalysis.Razor.Compiler/src/Language/RazorCodeDocument.cs | Adds GetCSharpDocument/GetRequiredCSharpDocument helpers to access impl vs decl documents. |
Comments suppressed due to low confidence (1)
src/Razor/src/Razor/src/Microsoft.CodeAnalysis.Razor.CohostingShared/Diagnostics/CohostDocumentPullDiagnosticsEndpointBase.cs:139
- Same typo in
GetHtmlDiagnosticsAsync:correletionIdshould becorrelationIdfor consistency and readability.
private async Task<LspDiagnostic[]> GetHtmlDiagnosticsAsync(TextDocument razorDocument, Guid correletionId, CancellationToken cancellationToken)
{
var diagnosticsParams = CreateHtmlParams(razorDocument.CreateSystemUri());
var result = await _requestInvoker.MakeHtmlLspRequestAsync<TRequest, TResponse>(
razorDocument,
LspMethodName,
diagnosticsParams,
TelemetryThresholds.DiagnosticsSubLSPTelemetryThreshold,
correletionId,
cancellationToken).ConfigureAwait(false);
Every consumer immedately converted to an array anyway
| var projects = RazorDiagnosticHelper.GetProjectInformation(documentSnapshot); | ||
| using var mappedDiagnostics = new PooledArrayBuilder<LspDiagnostic>(); | ||
| var codeDocument = await documentSnapshot.GetGeneratedOutputAsync(cancellationToken).ConfigureAwait(false); | ||
| var implDocument = codeDocument.GetRequiredCSharpDocument(declarationDocument: false); |
There was a problem hiding this comment.
I don't disagree, and I definitely wouldn't rule out going back to that shape before integrating this feature branch, but I don't think we can reasonably do this work in multiple PRs without having some way to tell if a particular call has been checked/updated.
If you have any ideas, I'm all ears.
| internal readonly record struct DocumentPositionInfo( | ||
| [property: JsonPropertyName("languageKind")] RazorLanguageKind LanguageKind, | ||
| [property: JsonPropertyName("position")] Position Position, | ||
| [property: JsonPropertyName("hostDocumentIndex")] int HostDocumentIndex); | ||
| [property: JsonPropertyName("hostDocumentIndex")] int HostDocumentIndex, | ||
| [property: JsonPropertyName("inDeclDocument")] bool InDeclDocument = false); |
| foreach (var otherDiagnostic in diagnostics) | ||
| { | ||
| if (otherDiagnostic.Code is { Value: RemoveUnnecessaryImportsConstants.IDE0005_gen } && | ||
| IsSameDiagnosticSpan(diagnostic, otherDiagnostic)) | ||
| { |
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
First cab off the rank for tooling support for Project Sonic.
I'm not 100% on the APIs here but given its going to a feature branch I think it's better to get some things working and see what the most useful API is as it emerges.
Microsoft Reviewers: Open in CodeFlow