From 7d8be8e16767e182beb31388d70c73eab1006abf Mon Sep 17 00:00:00 2001 From: David Wengier Date: Thu, 5 Sep 2024 12:12:55 +1000 Subject: [PATCH] Most PR feedback --- .../RazorCSharpFormattingBenchmark.cs | 2 +- .../CSharp/DefaultCSharpCodeActionResolver.cs | 2 +- .../Formatting/RazorFormattingOptions.cs | 2 -- .../Formatting/RazorFormattingService.cs | 15 ++++---- .../Remote/IRemoteFormattingService.cs | 34 +++++++++++++++--- .../Formatting/RemoteFormattingService.cs | 35 ++++++++++--------- .../RemoteRazorFormattingService.cs | 2 +- .../CohostDocumentFormattingEndpoint.cs | 12 +++---- .../Cohost/CohostOnTypeFormattingEndpoint.cs | 8 ++--- .../Cohost/CohostRangeFormattingEndpoint.cs | 8 ++--- .../FormattingLanguageServerTestBase.cs | 2 +- 11 files changed, 73 insertions(+), 49 deletions(-) diff --git a/src/Razor/benchmarks/Microsoft.AspNetCore.Razor.Microbenchmarks/LanguageServer/RazorCSharpFormattingBenchmark.cs b/src/Razor/benchmarks/Microsoft.AspNetCore.Razor.Microbenchmarks/LanguageServer/RazorCSharpFormattingBenchmark.cs index 12056563e0..88d6e3b633 100644 --- a/src/Razor/benchmarks/Microsoft.AspNetCore.Razor.Microbenchmarks/LanguageServer/RazorCSharpFormattingBenchmark.cs +++ b/src/Razor/benchmarks/Microsoft.AspNetCore.Razor.Microbenchmarks/LanguageServer/RazorCSharpFormattingBenchmark.cs @@ -112,7 +112,7 @@ public class RazorCSharpFormattingBenchmark : RazorLanguageServerBenchmarkBase { var documentContext = new DocumentContext(DocumentUri, DocumentSnapshot, projectContext: null); - var edits = await RazorFormattingService.GetDocumentFormattingEditsAsync(documentContext, htmlEdits: [], range: null, RazorFormattingOptions.Default, CancellationToken.None); + var edits = await RazorFormattingService.GetDocumentFormattingEditsAsync(documentContext, htmlEdits: [], range: null, new RazorFormattingOptions(), CancellationToken.None); #if DEBUG // For debugging purposes only. diff --git a/src/Razor/src/Microsoft.AspNetCore.Razor.LanguageServer/CodeActions/CSharp/DefaultCSharpCodeActionResolver.cs b/src/Razor/src/Microsoft.AspNetCore.Razor.LanguageServer/CodeActions/CSharp/DefaultCSharpCodeActionResolver.cs index 5e0636c681..7457c02597 100644 --- a/src/Razor/src/Microsoft.AspNetCore.Razor.LanguageServer/CodeActions/CSharp/DefaultCSharpCodeActionResolver.cs +++ b/src/Razor/src/Microsoft.AspNetCore.Razor.LanguageServer/CodeActions/CSharp/DefaultCSharpCodeActionResolver.cs @@ -68,7 +68,7 @@ internal sealed class DefaultCSharpCodeActionResolver( var formattedEdit = await _razorFormattingService.GetCSharpCodeActionEditAsync( documentContext, csharpTextEdits, - RazorFormattingOptions.Default, + new RazorFormattingOptions(), cancellationToken).ConfigureAwait(false); cancellationToken.ThrowIfCancellationRequested(); diff --git a/src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/Formatting/RazorFormattingOptions.cs b/src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/Formatting/RazorFormattingOptions.cs index a1ab2f6a3e..fdacc443ec 100644 --- a/src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/Formatting/RazorFormattingOptions.cs +++ b/src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/Formatting/RazorFormattingOptions.cs @@ -10,8 +10,6 @@ namespace Microsoft.CodeAnalysis.Razor.Formatting; [DataContract] internal readonly record struct RazorFormattingOptions { - public static readonly RazorFormattingOptions Default = new(); - [DataMember(Order = 0)] public bool InsertSpaces { get; init; } = true; [DataMember(Order = 1)] diff --git a/src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/Formatting/RazorFormattingService.cs b/src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/Formatting/RazorFormattingService.cs index c5f28c682c..87bf5d68bf 100644 --- a/src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/Formatting/RazorFormattingService.cs +++ b/src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/Formatting/RazorFormattingService.cs @@ -199,16 +199,13 @@ internal class RazorFormattingService : IRazorFormattingService public bool TryGetOnTypeFormattingTriggerKind(RazorCodeDocument codeDocument, int hostDocumentIndex, string triggerCharacter, out RazorLanguageKind triggerCharacterKind) { triggerCharacterKind = _documentMappingService.GetLanguageKind(codeDocument, hostDocumentIndex, rightAssociative: false); - if (triggerCharacterKind is RazorLanguageKind.CSharp) - { - return s_csharpTriggerCharacterSet.Contains(triggerCharacter); - } - else if (triggerCharacterKind is RazorLanguageKind.Html) - { - return s_htmlTriggerCharacterSet.Contains(triggerCharacter); - } - return false; + return triggerCharacterKind switch + { + RazorLanguageKind.CSharp => s_csharpTriggerCharacterSet.Contains(triggerCharacter), + RazorLanguageKind.Html => s_htmlTriggerCharacterSet.Contains(triggerCharacter), + _ => false, + }; } private async Task ApplyFormattedEditsAsync( diff --git a/src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/Remote/IRemoteFormattingService.cs b/src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/Remote/IRemoteFormattingService.cs index 8a1b7aa379..3ec3ae4cb9 100644 --- a/src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/Remote/IRemoteFormattingService.cs +++ b/src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/Remote/IRemoteFormattingService.cs @@ -12,10 +12,36 @@ namespace Microsoft.CodeAnalysis.Razor.Remote; internal interface IRemoteFormattingService { - ValueTask> GetDocumentFormattingEditsAsync(RazorPinnedSolutionInfoWrapper solutionInfo, DocumentId documentId, ImmutableArray htmlChanges, RazorFormattingOptions options, CancellationToken cancellationToken); - ValueTask> GetRangeFormattingEditsAsync(RazorPinnedSolutionInfoWrapper solutionInfo, DocumentId documentId, ImmutableArray htmlChanges, LinePositionSpan linePositionSpan, RazorFormattingOptions options, CancellationToken cancellationToken); - ValueTask> GetOnTypeFormattingEditsAsync(RazorPinnedSolutionInfoWrapper solutionInfo, DocumentId documentId, ImmutableArray htmlChanges, LinePosition linePosition, string triggerCharacter, RazorFormattingOptions options, CancellationToken cancellationToken); - ValueTask GetOnTypeFormattingTriggerKindAsync(RazorPinnedSolutionInfoWrapper solutionInfo, DocumentId documentId, LinePosition linePosition, string triggerCharacter, CancellationToken cancellationToken); + ValueTask> GetDocumentFormattingEditsAsync( + RazorPinnedSolutionInfoWrapper solutionInfo, + DocumentId documentId, + ImmutableArray htmlChanges, + RazorFormattingOptions options, + CancellationToken cancellationToken); + + ValueTask> GetRangeFormattingEditsAsync( + RazorPinnedSolutionInfoWrapper solutionInfo, + DocumentId documentId, + ImmutableArray htmlChanges, + LinePositionSpan linePositionSpan, + RazorFormattingOptions options, + CancellationToken cancellationToken); + + ValueTask> GetOnTypeFormattingEditsAsync( + RazorPinnedSolutionInfoWrapper solutionInfo, + DocumentId documentId, + ImmutableArray htmlChanges, + LinePosition linePosition, + string triggerCharacter, + RazorFormattingOptions options, + CancellationToken cancellationToken); + + ValueTask GetOnTypeFormattingTriggerKindAsync( + RazorPinnedSolutionInfoWrapper solutionInfo, + DocumentId documentId, + LinePosition linePosition, + string triggerCharacter, + CancellationToken cancellationToken); internal enum TriggerKind { diff --git a/src/Razor/src/Microsoft.CodeAnalysis.Remote.Razor/Formatting/RemoteFormattingService.cs b/src/Razor/src/Microsoft.CodeAnalysis.Remote.Razor/Formatting/RemoteFormattingService.cs index e0dc5d35ac..e3e334604c 100644 --- a/src/Razor/src/Microsoft.CodeAnalysis.Remote.Razor/Formatting/RemoteFormattingService.cs +++ b/src/Razor/src/Microsoft.CodeAnalysis.Remote.Razor/Formatting/RemoteFormattingService.cs @@ -3,6 +3,7 @@ using System.Collections.Generic; using System.Collections.Immutable; +using System.Diagnostics; using System.Linq; using System.Threading; using System.Threading.Tasks; @@ -53,24 +54,24 @@ internal sealed class RemoteFormattingService(in ServiceArgs args) : RazorDocume if (edits is null) { - return ImmutableArray.Empty; + return []; } return edits.SelectAsArray(sourceText.GetTextChange); } public ValueTask> GetRangeFormattingEditsAsync( - RazorPinnedSolutionInfoWrapper solutionInfo, - DocumentId documentId, - ImmutableArray htmlChanges, - LinePositionSpan linePositionSpan, - RazorFormattingOptions options, - CancellationToken cancellationToken) - => RunServiceAsync( - solutionInfo, - documentId, - context => GetRangeFormattingEditsAsync(context, htmlChanges, linePositionSpan, options, cancellationToken), - cancellationToken); + RazorPinnedSolutionInfoWrapper solutionInfo, + DocumentId documentId, + ImmutableArray htmlChanges, + LinePositionSpan linePositionSpan, + RazorFormattingOptions options, + CancellationToken cancellationToken) + => RunServiceAsync( + solutionInfo, + documentId, + context => GetRangeFormattingEditsAsync(context, htmlChanges, linePositionSpan, options, cancellationToken), + cancellationToken); private async ValueTask> GetRangeFormattingEditsAsync( RemoteDocumentContext context, @@ -86,7 +87,7 @@ internal sealed class RemoteFormattingService(in ServiceArgs args) : RazorDocume if (edits is null) { - return ImmutableArray.Empty; + return []; } return edits.SelectAsArray(sourceText.GetTextChange); @@ -132,8 +133,7 @@ internal sealed class RemoteFormattingService(in ServiceArgs args) : RazorDocume } else { - Assumed.Unreachable(); - return []; + return Assumed.Unreachable>(); } return result.SelectAsArray(sourceText.GetTextChange); @@ -154,7 +154,7 @@ internal sealed class RemoteFormattingService(in ServiceArgs args) : RazorDocume private async ValueTask IsValidOnTypeFormattingTriggerAsync(RemoteDocumentContext context, LinePosition linePosition, string triggerCharacter, CancellationToken cancellationToken) { var codeDocument = await context.GetCodeDocumentAsync(cancellationToken).ConfigureAwait(false); - var sourceText = await context.GetSourceTextAsync(cancellationToken).ConfigureAwait(false); + var sourceText = codeDocument.Source.Text; if (!sourceText.TryGetAbsoluteIndex(linePosition, out var hostDocumentIndex)) { return Response.Invalid; @@ -170,6 +170,9 @@ internal sealed class RemoteFormattingService(in ServiceArgs args) : RazorDocume return Response.ValidHtml; } + // TryGetOnTypeFormattingTriggerKind only returns true for C# or Html + Debug.Assert(triggerCharacterKind is RazorLanguageKind.CSharp); + return Response.ValidCSharp; } } diff --git a/src/Razor/src/Microsoft.CodeAnalysis.Remote.Razor/Formatting/RemoteRazorFormattingService.cs b/src/Razor/src/Microsoft.CodeAnalysis.Remote.Razor/Formatting/RemoteRazorFormattingService.cs index 7d9344fe3d..6974dabd01 100644 --- a/src/Razor/src/Microsoft.CodeAnalysis.Remote.Razor/Formatting/RemoteRazorFormattingService.cs +++ b/src/Razor/src/Microsoft.CodeAnalysis.Remote.Razor/Formatting/RemoteRazorFormattingService.cs @@ -11,7 +11,7 @@ namespace Microsoft.CodeAnalysis.Remote.Razor.Formatting; [Export(typeof(IRazorFormattingService)), Shared] [method: ImportingConstructor] -internal class RemoteRazorFormattingService(IFormattingCodeDocumentProvider codeDocumentProvider, IDocumentMappingService documentMappingService, IAdhocWorkspaceFactory adhocWorkspaceFactory, ILoggerFactory loggerFactory) +internal sealed class RemoteRazorFormattingService(IFormattingCodeDocumentProvider codeDocumentProvider, IDocumentMappingService documentMappingService, IAdhocWorkspaceFactory adhocWorkspaceFactory, ILoggerFactory loggerFactory) : RazorFormattingService(codeDocumentProvider, documentMappingService, adhocWorkspaceFactory, loggerFactory) { } diff --git a/src/Razor/src/Microsoft.VisualStudio.LanguageServices.Razor/LanguageClient/Cohost/CohostDocumentFormattingEndpoint.cs b/src/Razor/src/Microsoft.VisualStudio.LanguageServices.Razor/LanguageClient/Cohost/CohostDocumentFormattingEndpoint.cs index 8589644a08..9fa42babe9 100644 --- a/src/Razor/src/Microsoft.VisualStudio.LanguageServices.Razor/LanguageClient/Cohost/CohostDocumentFormattingEndpoint.cs +++ b/src/Razor/src/Microsoft.VisualStudio.LanguageServices.Razor/LanguageClient/Cohost/CohostDocumentFormattingEndpoint.cs @@ -27,7 +27,7 @@ namespace Microsoft.VisualStudio.Razor.LanguageClient.Cohost; [ExportCohostStatelessLspService(typeof(CohostDocumentFormattingEndpoint))] [method: ImportingConstructor] #pragma warning restore RS0030 // Do not use banned APIs -internal class CohostDocumentFormattingEndpoint( +internal sealed class CohostDocumentFormattingEndpoint( IRemoteServiceInvoker remoteServiceInvoker, IHtmlDocumentSynchronizer htmlDocumentSynchronizer, LSPRequestInvoker requestInvoker, @@ -71,7 +71,7 @@ internal class CohostDocumentFormattingEndpoint( private async Task HandleRequestAsync(DocumentFormattingParams request, TextDocument razorDocument, CancellationToken cancellationToken) { _logger.LogDebug($"Getting Html formatting changes for {razorDocument.FilePath}"); - var htmlResult = await GetHtmlFormattingEditsAsync(request, razorDocument, cancellationToken).ConfigureAwait(false); + var htmlResult = await TryGetHtmlFormattingEditsAsync(request, razorDocument, cancellationToken).ConfigureAwait(false); if (htmlResult is not { } htmlEdits) { @@ -91,17 +91,17 @@ internal class CohostDocumentFormattingEndpoint( (service, solutionInfo, cancellationToken) => service.GetDocumentFormattingEditsAsync(solutionInfo, razorDocument.Id, htmlChanges, options, cancellationToken), cancellationToken).ConfigureAwait(false); - if (remoteResult is [_, ..] allChanges) + if (remoteResult.Length > 0) { - _logger.LogDebug($"Got a total of {allChanges.Length} ranges back from OOP"); + _logger.LogDebug($"Got a total of {remoteResult.Length} ranges back from OOP"); - return allChanges.Select(sourceText.GetTextEdit).ToArray(); + return remoteResult.Select(sourceText.GetTextEdit).ToArray(); } return null; } - private async Task GetHtmlFormattingEditsAsync(DocumentFormattingParams request, TextDocument razorDocument, CancellationToken cancellationToken) + private async Task TryGetHtmlFormattingEditsAsync(DocumentFormattingParams request, TextDocument razorDocument, CancellationToken cancellationToken) { var htmlDocument = await _htmlDocumentSynchronizer.TryGetSynchronizedHtmlDocumentAsync(razorDocument, cancellationToken).ConfigureAwait(false); if (htmlDocument is null) diff --git a/src/Razor/src/Microsoft.VisualStudio.LanguageServices.Razor/LanguageClient/Cohost/CohostOnTypeFormattingEndpoint.cs b/src/Razor/src/Microsoft.VisualStudio.LanguageServices.Razor/LanguageClient/Cohost/CohostOnTypeFormattingEndpoint.cs index e5cd437628..11a2a6f207 100644 --- a/src/Razor/src/Microsoft.VisualStudio.LanguageServices.Razor/LanguageClient/Cohost/CohostOnTypeFormattingEndpoint.cs +++ b/src/Razor/src/Microsoft.VisualStudio.LanguageServices.Razor/LanguageClient/Cohost/CohostOnTypeFormattingEndpoint.cs @@ -27,7 +27,7 @@ namespace Microsoft.VisualStudio.Razor.LanguageClient.Cohost; [ExportCohostStatelessLspService(typeof(CohostOnTypeFormattingEndpoint))] [method: ImportingConstructor] #pragma warning restore RS0030 // Do not use banned APIs -internal class CohostOnTypeFormattingEndpoint( +internal sealed class CohostOnTypeFormattingEndpoint( IRemoteServiceInvoker remoteServiceInvoker, IHtmlDocumentSynchronizer htmlDocumentSynchronizer, LSPRequestInvoker requestInvoker, @@ -127,11 +127,11 @@ internal class CohostOnTypeFormattingEndpoint( (service, solutionInfo, cancellationToken) => service.GetOnTypeFormattingEditsAsync(solutionInfo, razorDocument.Id, htmlChanges, request.Position.ToLinePosition(), request.Character, options, cancellationToken), cancellationToken).ConfigureAwait(false); - if (remoteResult is [_, ..] allChanges) + if (remoteResult.Length > 0) { - _logger.LogDebug($"Got a total of {allChanges.Length} ranges back from OOP"); + _logger.LogDebug($"Got a total of {remoteResult.Length} ranges back from OOP"); - return allChanges.Select(sourceText.GetTextEdit).ToArray(); + return remoteResult.Select(sourceText.GetTextEdit).ToArray(); } return null; diff --git a/src/Razor/src/Microsoft.VisualStudio.LanguageServices.Razor/LanguageClient/Cohost/CohostRangeFormattingEndpoint.cs b/src/Razor/src/Microsoft.VisualStudio.LanguageServices.Razor/LanguageClient/Cohost/CohostRangeFormattingEndpoint.cs index 9be30b7eab..cc27035b56 100644 --- a/src/Razor/src/Microsoft.VisualStudio.LanguageServices.Razor/LanguageClient/Cohost/CohostRangeFormattingEndpoint.cs +++ b/src/Razor/src/Microsoft.VisualStudio.LanguageServices.Razor/LanguageClient/Cohost/CohostRangeFormattingEndpoint.cs @@ -27,7 +27,7 @@ namespace Microsoft.VisualStudio.Razor.LanguageClient.Cohost; [ExportCohostStatelessLspService(typeof(CohostRangeFormattingEndpoint))] [method: ImportingConstructor] #pragma warning restore RS0030 // Do not use banned APIs -internal class CohostRangeFormattingEndpoint( +internal sealed class CohostRangeFormattingEndpoint( IRemoteServiceInvoker remoteServiceInvoker, IHtmlDocumentSynchronizer htmlDocumentSynchronizer, LSPRequestInvoker requestInvoker, @@ -91,11 +91,11 @@ internal class CohostRangeFormattingEndpoint( (service, solutionInfo, cancellationToken) => service.GetRangeFormattingEditsAsync(solutionInfo, razorDocument.Id, htmlChanges, request.Range.ToLinePositionSpan(), options, cancellationToken), cancellationToken).ConfigureAwait(false); - if (remoteResult is [_, ..] allChanges) + if (remoteResult.Length > 0) { - _logger.LogDebug($"Got a total of {allChanges.Length} ranges back from OOP"); + _logger.LogDebug($"Got a total of {remoteResult.Length} ranges back from OOP"); - return allChanges.Select(sourceText.GetTextEdit).ToArray(); + return remoteResult.Select(sourceText.GetTextEdit).ToArray(); } return null; diff --git a/src/Razor/test/Microsoft.AspNetCore.Razor.LanguageServer.Test/Formatting_NetFx/FormattingLanguageServerTestBase.cs b/src/Razor/test/Microsoft.AspNetCore.Razor.LanguageServer.Test/Formatting_NetFx/FormattingLanguageServerTestBase.cs index d14e475aa5..85dfee530a 100644 --- a/src/Razor/test/Microsoft.AspNetCore.Razor.LanguageServer.Test/Formatting_NetFx/FormattingLanguageServerTestBase.cs +++ b/src/Razor/test/Microsoft.AspNetCore.Razor.LanguageServer.Test/Formatting_NetFx/FormattingLanguageServerTestBase.cs @@ -69,7 +69,7 @@ public abstract class FormattingLanguageServerTestBase(ITestOutputHelper testOut public bool TryGetOnTypeFormattingTriggerKind(RazorCodeDocument codeDocument, int hostDocumentIndex, string triggerCharacter, out RazorLanguageKind triggerCharacterKind) { - triggerCharacterKind = languageKind ?? RazorLanguageKind.CSharp; + triggerCharacterKind = languageKind.GetValueOrDefault(); return languageKind is not null; } }