Most PR feedback

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.

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();

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)]

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<TextEdit[]> ApplyFormattedEditsAsync(

src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/Remote/IRemoteFormattingService.cs

@@ -12,10 +12,36 @@ namespace Microsoft.CodeAnalysis.Razor.Remote;
 
 internal interface IRemoteFormattingService
 {
-    ValueTask<ImmutableArray<TextChange>> GetDocumentFormattingEditsAsync(RazorPinnedSolutionInfoWrapper solutionInfo, DocumentId documentId, ImmutableArray<TextChange> htmlChanges, RazorFormattingOptions options, CancellationToken cancellationToken);
-    ValueTask<ImmutableArray<TextChange>> GetRangeFormattingEditsAsync(RazorPinnedSolutionInfoWrapper solutionInfo, DocumentId documentId, ImmutableArray<TextChange> htmlChanges, LinePositionSpan linePositionSpan, RazorFormattingOptions options, CancellationToken cancellationToken);
-    ValueTask<ImmutableArray<TextChange>> GetOnTypeFormattingEditsAsync(RazorPinnedSolutionInfoWrapper solutionInfo, DocumentId documentId, ImmutableArray<TextChange> htmlChanges, LinePosition linePosition, string triggerCharacter, RazorFormattingOptions options, CancellationToken cancellationToken);
-    ValueTask<TriggerKind> GetOnTypeFormattingTriggerKindAsync(RazorPinnedSolutionInfoWrapper solutionInfo, DocumentId documentId, LinePosition linePosition, string triggerCharacter, CancellationToken cancellationToken);
+    ValueTask<ImmutableArray<TextChange>> GetDocumentFormattingEditsAsync(
+        RazorPinnedSolutionInfoWrapper solutionInfo,
+        DocumentId documentId,
+        ImmutableArray<TextChange> htmlChanges,
+        RazorFormattingOptions options,
+        CancellationToken cancellationToken);
+
+    ValueTask<ImmutableArray<TextChange>> GetRangeFormattingEditsAsync(
+        RazorPinnedSolutionInfoWrapper solutionInfo,
+        DocumentId documentId,
+        ImmutableArray<TextChange> htmlChanges,
+        LinePositionSpan linePositionSpan,
+        RazorFormattingOptions options,
+        CancellationToken cancellationToken);
+
+    ValueTask<ImmutableArray<TextChange>> GetOnTypeFormattingEditsAsync(
+        RazorPinnedSolutionInfoWrapper solutionInfo,
+        DocumentId documentId,
+        ImmutableArray<TextChange> htmlChanges,
+        LinePosition linePosition,
+        string triggerCharacter,
+        RazorFormattingOptions options,
+        CancellationToken cancellationToken);
+
+    ValueTask<TriggerKind> GetOnTypeFormattingTriggerKindAsync(
+        RazorPinnedSolutionInfoWrapper solutionInfo,
+        DocumentId documentId,
+        LinePosition linePosition,
+        string triggerCharacter,
+        CancellationToken cancellationToken);
 
     internal enum TriggerKind
     {

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,7 +54,7 @@ internal sealed class RemoteFormattingService(in ServiceArgs args) : RazorDocume
 
         if (edits is null)
         {
-            return ImmutableArray<TextChange>.Empty;
+            return [];
         }
 
         return edits.SelectAsArray(sourceText.GetTextChange);
@@ -86,7 +87,7 @@ internal sealed class RemoteFormattingService(in ServiceArgs args) : RazorDocume
 
         if (edits is null)
         {
-            return ImmutableArray<TextChange>.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<ImmutableArray<TextChange>>();
         }
 
         return result.SelectAsArray(sourceText.GetTextChange);
@@ -154,7 +154,7 @@ internal sealed class RemoteFormattingService(in ServiceArgs args) : RazorDocume
     private async ValueTask<Response> 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;
     }
 }

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)
 {
 }

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<TextEdit[]?> 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<TextEdit[]?> GetHtmlFormattingEditsAsync(DocumentFormattingParams request, TextDocument razorDocument, CancellationToken cancellationToken)
+    private async Task<TextEdit[]?> TryGetHtmlFormattingEditsAsync(DocumentFormattingParams request, TextDocument razorDocument, CancellationToken cancellationToken)
     {
         var htmlDocument = await _htmlDocumentSynchronizer.TryGetSynchronizedHtmlDocumentAsync(razorDocument, cancellationToken).ConfigureAwait(false);
         if (htmlDocument is null)

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;

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;

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;
         }
     }