From a782f9e8e3fd31678e13c85ea6b6ce3bea40a9cd Mon Sep 17 00:00:00 2001 From: David Wengier Date: Wed, 11 Sep 2024 16:50:50 +1000 Subject: [PATCH] Create basic service and endpoints to gather diagnostics and pass them over to OOP Wanted to validate my Roslyn code would work first, before moving our services around, in case the order of this work seems backwards to usual :) --- eng/targets/Services.props | 1 + .../RazorLanguageServer.cs | 4 +- .../Remote/IRemoteDiagnosticsService.cs | 20 ++ .../Remote/RazorServices.cs | 1 + .../Diagnostics/RemoteDiagnosticsService.cs | 43 ++++ .../CohostDocumentPullDiagnosticsEndpoint.cs | 205 ++++++++++++++++++ 6 files changed, 273 insertions(+), 1 deletion(-) create mode 100644 src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/Remote/IRemoteDiagnosticsService.cs create mode 100644 src/Razor/src/Microsoft.CodeAnalysis.Remote.Razor/Diagnostics/RemoteDiagnosticsService.cs create mode 100644 src/Razor/src/Microsoft.VisualStudio.LanguageServices.Razor/LanguageClient/Cohost/CohostDocumentPullDiagnosticsEndpoint.cs diff --git a/eng/targets/Services.props b/eng/targets/Services.props index fc2acc27df..0501df0e11 100644 --- a/eng/targets/Services.props +++ b/eng/targets/Services.props @@ -32,5 +32,6 @@ + diff --git a/src/Razor/src/Microsoft.AspNetCore.Razor.LanguageServer/RazorLanguageServer.cs b/src/Razor/src/Microsoft.AspNetCore.Razor.LanguageServer/RazorLanguageServer.cs index be86fe0140..70d4fcd135 100644 --- a/src/Razor/src/Microsoft.AspNetCore.Razor.LanguageServer/RazorLanguageServer.cs +++ b/src/Razor/src/Microsoft.AspNetCore.Razor.LanguageServer/RazorLanguageServer.cs @@ -131,7 +131,6 @@ internal partial class RazorLanguageServer : SystemTextJsonLanguageServer(); services.AddSingleton(); diff --git a/src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/Remote/IRemoteDiagnosticsService.cs b/src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/Remote/IRemoteDiagnosticsService.cs new file mode 100644 index 0000000000..65d6cc876c --- /dev/null +++ b/src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/Remote/IRemoteDiagnosticsService.cs @@ -0,0 +1,20 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the MIT license. See License.txt in the project root for license information. + +using System.Collections.Immutable; +using System.Threading; +using System.Threading.Tasks; +using Microsoft.CodeAnalysis.ExternalAccess.Razor; +using RoslynLspDiagnostic = Roslyn.LanguageServer.Protocol.Diagnostic; + +namespace Microsoft.CodeAnalysis.Razor.Remote; + +internal interface IRemoteDiagnosticsService : IRemoteJsonService +{ + ValueTask> GetDiagnosticsAsync( + JsonSerializableRazorPinnedSolutionInfoWrapper solutionInfo, + JsonSerializableDocumentId documentId, + ImmutableArray csharpDiagnostics, + ImmutableArray htmlDiagnostics, + CancellationToken cancellationToken); +} diff --git a/src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/Remote/RazorServices.cs b/src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/Remote/RazorServices.cs index 4ede429257..2e4257cdb3 100644 --- a/src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/Remote/RazorServices.cs +++ b/src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/Remote/RazorServices.cs @@ -35,6 +35,7 @@ internal static class RazorServices (typeof(IRemoteDocumentSymbolService), null), (typeof(IRemoteRenameService), null), (typeof(IRemoteGoToImplementationService), null), + (typeof(IRemoteDiagnosticsService), null), ]; private const string ComponentName = "Razor"; diff --git a/src/Razor/src/Microsoft.CodeAnalysis.Remote.Razor/Diagnostics/RemoteDiagnosticsService.cs b/src/Razor/src/Microsoft.CodeAnalysis.Remote.Razor/Diagnostics/RemoteDiagnosticsService.cs new file mode 100644 index 0000000000..cb95569cd2 --- /dev/null +++ b/src/Razor/src/Microsoft.CodeAnalysis.Remote.Razor/Diagnostics/RemoteDiagnosticsService.cs @@ -0,0 +1,43 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the MIT license. See License.txt in the project root for license information. + +using System.Collections.Immutable; +using System.Threading; +using System.Threading.Tasks; +using Microsoft.CodeAnalysis.ExternalAccess.Razor; +using Microsoft.CodeAnalysis.Razor.Remote; +using Microsoft.CodeAnalysis.Remote.Razor.ProjectSystem; +using LspDiagnostic = Roslyn.LanguageServer.Protocol.Diagnostic; + +namespace Microsoft.CodeAnalysis.Remote.Razor; + +internal sealed class RemoteDiagnosticsService(in ServiceArgs args) : RazorDocumentServiceBase(in args), IRemoteDiagnosticsService +{ + internal sealed class Factory : FactoryBase + { + protected override IRemoteDiagnosticsService CreateService(in ServiceArgs args) + => new RemoteDiagnosticsService(in args); + } + + public ValueTask> GetDiagnosticsAsync( + JsonSerializableRazorPinnedSolutionInfoWrapper solutionInfo, + JsonSerializableDocumentId documentId, + ImmutableArray csharpDiagnostics, + ImmutableArray htmlDiagnostics, + CancellationToken cancellationToken) + => RunServiceAsync( + solutionInfo, + documentId, + context => GetDiagnosticsAsync(context, csharpDiagnostics, htmlDiagnostics, cancellationToken), + cancellationToken); + + private async ValueTask> GetDiagnosticsAsync( + RemoteDocumentContext context, + ImmutableArray csharpDiagnostics, + ImmutableArray htmlDiagnostics, + CancellationToken cancellationToken) + { + // TODO: More work! + return htmlDiagnostics; + } +} diff --git a/src/Razor/src/Microsoft.VisualStudio.LanguageServices.Razor/LanguageClient/Cohost/CohostDocumentPullDiagnosticsEndpoint.cs b/src/Razor/src/Microsoft.VisualStudio.LanguageServices.Razor/LanguageClient/Cohost/CohostDocumentPullDiagnosticsEndpoint.cs new file mode 100644 index 0000000000..37493d1873 --- /dev/null +++ b/src/Razor/src/Microsoft.VisualStudio.LanguageServices.Razor/LanguageClient/Cohost/CohostDocumentPullDiagnosticsEndpoint.cs @@ -0,0 +1,205 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the MIT license. See License.txt in the project root for license information. + +using System; +using System.Collections.Generic; +using System.Collections.Immutable; +using System.Composition; +using System.Linq; +using System.Text.Json; +using System.Threading; +using System.Threading.Tasks; +using Microsoft.AspNetCore.Razor; +using Microsoft.AspNetCore.Razor.PooledObjects; +using Microsoft.AspNetCore.Razor.Threading; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.ExternalAccess.Razor; +using Microsoft.CodeAnalysis.ExternalAccess.Razor.Cohost; +using Microsoft.CodeAnalysis.Razor.Logging; +using Microsoft.CodeAnalysis.Razor.ProjectSystem; +using Microsoft.CodeAnalysis.Razor.Remote; +using Microsoft.CodeAnalysis.Razor.Workspaces; +using Microsoft.VisualStudio.LanguageServer.ContainedLanguage; +using Microsoft.VisualStudio.LanguageServer.Protocol; +using ExternalHandlers = Microsoft.CodeAnalysis.ExternalAccess.Razor.Cohost.Handlers; +using RoslynLspDiagnostic = Roslyn.LanguageServer.Protocol.Diagnostic; +using RoslynVSInternalDiagnosticReport = Roslyn.LanguageServer.Protocol.VSInternalDiagnosticReport; + +namespace Microsoft.VisualStudio.Razor.LanguageClient.Cohost; + +#pragma warning disable RS0030 // Do not use banned APIs +[Shared] +[CohostEndpoint(VSInternalMethods.DocumentPullDiagnosticName)] +[Export(typeof(IDynamicRegistrationProvider))] +[ExportCohostStatelessLspService(typeof(CohostDocumentPullDiagnosticsEndpoint))] +[method: ImportingConstructor] +#pragma warning restore RS0030 // Do not use banned APIs +internal class CohostDocumentPullDiagnosticsEndpoint( + IRemoteServiceInvoker remoteServiceInvoker, + IHtmlDocumentSynchronizer htmlDocumentSynchronizer, + LSPRequestInvoker requestInvoker, + IFilePathService filePathService, + ILoggerFactory loggerFactory) + : AbstractRazorCohostDocumentRequestHandler, IDynamicRegistrationProvider +{ + private readonly IRemoteServiceInvoker _remoteServiceInvoker = remoteServiceInvoker; + private readonly IHtmlDocumentSynchronizer _htmlDocumentSynchronizer = htmlDocumentSynchronizer; + private readonly LSPRequestInvoker _requestInvoker = requestInvoker; + private readonly IFilePathService _filePathService = filePathService; + private readonly ILogger _logger = loggerFactory.GetOrCreateLogger(); + + protected override bool MutatesSolutionState => false; + + protected override bool RequiresLSPSolution => true; + + public Registration? GetRegistration(VSInternalClientCapabilities clientCapabilities, DocumentFilter[] filter, RazorCohostRequestContext requestContext) + { + // TODO: if (clientCapabilities.TextDocument?.Diagnostic?.DynamicRegistration is true) + { + return new Registration() + { + Method = VSInternalMethods.DocumentPullDiagnosticName, + RegisterOptions = new VSInternalDiagnosticRegistrationOptions() + { + DocumentSelector = filter, + DiagnosticKinds = [VSInternalDiagnosticKind.Syntax] + } + }; + } + + // return null; + } + + protected override RazorTextDocumentIdentifier? GetRazorTextDocumentIdentifier(VSInternalDocumentDiagnosticsParams request) + => request.TextDocument?.ToRazorTextDocumentIdentifier(); + + protected override Task HandleRequestAsync(VSInternalDocumentDiagnosticsParams request, RazorCohostRequestContext context, CancellationToken cancellationToken) + => HandleRequestAsync(context.TextDocument.AssumeNotNull(), cancellationToken); + + private async Task HandleRequestAsync(TextDocument razorDocument, CancellationToken cancellationToken) + { + // Diagnostics is a little different, because Roslyn is not designed to run diagnostics in OOP. Their system will transition to OOP + // as it needs, but we have to start here in devenv. This is not as big a problem as it sounds, specifically for diagnostics, because + // we only need to tell Roslyn the document we need diagnostics for. If we had to map positions or ranges etc. it would be worse + // because we'd have to transition to our OOP to find out that info, then back here to get the diagnostics, then back to OOP to process. + _logger.LogDebug($"Getting diagnostics for {razorDocument.FilePath}"); + + var csharpTask = GetCSharpDiagnosticsAsync(razorDocument, cancellationToken); + var htmlTask = GetHtmlDiagnosticsAsync(razorDocument, cancellationToken); + + try + { + await Task.WhenAll(htmlTask, csharpTask).ConfigureAwait(false); + } + catch (Exception e) + { + if (e is not OperationCanceledException) + { + _logger.LogError(e, $"Exception thrown in PullDiagnostic delegation"); + } + // Return null if any of the tasks getting diagnostics results in an error + return null; + } + + var csharpDiagnostics = await csharpTask.ConfigureAwait(false); + var htmlDiagnostics = await htmlTask.ConfigureAwait(false); + + _logger.LogDebug($"Calling OOP with the {csharpDiagnostics.Length} C# and {htmlDiagnostics.Length} Html diagnostics"); + var diagnostics = await _remoteServiceInvoker.TryInvokeAsync>( + razorDocument.Project.Solution, + (service, solutionInfo, cancellationToken) => service.GetDiagnosticsAsync(solutionInfo, razorDocument.Id, csharpDiagnostics, htmlDiagnostics, cancellationToken), + cancellationToken).ConfigureAwait(false); + + if (diagnostics.IsDefaultOrEmpty) + { + return null; + } + + return + [ + new() + { + Diagnostics = diagnostics.ToArray(), + ResultId = Guid.NewGuid().ToString() + } + ]; + } + + private Task> GetCSharpDiagnosticsAsync(TextDocument razorDocument, CancellationToken cancellationToken) + { + // TODO: This code will not work when the source generator is hooked up. + // How do we get the source generated C# document without OOP? Can we reverse engineer a file path? + var projectKey = razorDocument.Project.ToProjectKey(); + var csharpFilePath = _filePathService.GetRazorCSharpFilePath(projectKey, razorDocument.FilePath.AssumeNotNull()); + // We put the project Id in the generated document path, so there can only be one document + if (razorDocument.Project.Solution.GetDocumentIdsWithFilePath(csharpFilePath) is not [{ } generatedDocumentId] || + razorDocument.Project.GetDocument(generatedDocumentId) is not { } generatedDocument) + { + return SpecializedTasks.EmptyImmutableArray(); + } + + _logger.LogDebug($"Getting C# diagnostics for {generatedDocument.FilePath}"); + return ExternalHandlers.Diagnostics.GetDocumentDiagnosticsAsync(generatedDocument, supportsVisualStudioExtensions: true, cancellationToken); + } + + private async Task> GetHtmlDiagnosticsAsync(TextDocument razorDocument, CancellationToken cancellationToken) + { + var htmlDocument = await _htmlDocumentSynchronizer.TryGetSynchronizedHtmlDocumentAsync(razorDocument, cancellationToken).ConfigureAwait(false); + if (htmlDocument is null) + { + return []; + } + + var diagnosticsParams = new VSInternalDocumentDiagnosticsParams + { + TextDocument = new TextDocumentIdentifier { Uri = htmlDocument.Uri } + }; + + _logger.LogDebug($"Getting Html diagnostics for {htmlDocument.Uri}"); + + var result = await _requestInvoker.ReinvokeRequestOnServerAsync( + htmlDocument.Buffer, + VSInternalMethods.DocumentPullDiagnosticName, + RazorLSPConstants.HtmlLanguageServerName, + diagnosticsParams, + cancellationToken).ConfigureAwait(false); + + if (result?.Response is null) + { + return []; + } + + // This is, to say the least, not ideal. In future we're going to normalize on to Roslyn LSP types, and this can go. + var options = new JsonSerializerOptions(); + foreach (var converter in RazorServiceDescriptorsWrapper.GetLspConverters()) + { + options.Converters.Add(converter); + } + + var hmlDiagnostics = JsonSerializer.Deserialize(JsonSerializer.SerializeToDocument(result.Response), options); + if (hmlDiagnostics is not { } convertedHtmlDiagnostics) + { + return []; + } + + using var allDiagnostics = new PooledArrayBuilder(); + foreach (var report in convertedHtmlDiagnostics) + { + if (report.Diagnostics is not null) + { + allDiagnostics.AddRange(report.Diagnostics); + } + } + + return allDiagnostics.ToImmutable(); + } + + internal TestAccessor GetTestAccessor() => new(this); + + internal readonly struct TestAccessor(CohostDocumentPullDiagnosticsEndpoint instance) + { + public Task HandleRequestAsync(TextDocument razorDocument, CancellationToken cancellationToken) + => instance.HandleRequestAsync(razorDocument, cancellationToken); + } +} +