Make it so validation happens after filtering and normalization

src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/Formatting/IFormattingValidationPass.cs

@@ -0,0 +1,14 @@
+// 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.Text;
+
+namespace Microsoft.CodeAnalysis.Razor.Formatting;
+
+internal interface IFormattingValidationPass
+{
+    Task<bool> IsValidAsync(FormattingContext formattingContext, ImmutableArray<TextChange> changes, CancellationToken cancellationToken);
+}

src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/Formatting/Passes/FormattingContentValidationPass.cs

@@ -6,19 +6,20 @@
 using System.Linq;
 using System.Threading;
 using System.Threading.Tasks;
+using Microsoft.AspNetCore.Razor.Threading;
 using Microsoft.CodeAnalysis.Razor.Logging;
 using Microsoft.CodeAnalysis.Text;
 
 namespace Microsoft.CodeAnalysis.Razor.Formatting;
 
-internal sealed class FormattingContentValidationPass(ILoggerFactory loggerFactory) : IFormattingPass
+internal sealed class FormattingContentValidationPass(ILoggerFactory loggerFactory) : IFormattingValidationPass
 {
     private readonly ILogger _logger = loggerFactory.GetOrCreateLogger<FormattingContentValidationPass>();
 
     // Internal for testing.
     internal bool DebugAssertsEnabled { get; set; } = true;
 
-    public Task<ImmutableArray<TextChange>> ExecuteAsync(FormattingContext context, ImmutableArray<TextChange> changes, CancellationToken cancellationToken)
+    public Task<bool> IsValidAsync(FormattingContext context, ImmutableArray<TextChange> changes, CancellationToken cancellationToken)
     {
         var text = context.SourceText;
         var changedText = text.WithChanges(changes);
@@ -47,9 +48,9 @@ public Task<ImmutableArray<TextChange>> ExecuteAsync(FormattingContext context,
                 Debug.Fail("A formatting result was rejected because it was going to change non-whitespace content in the document.");
             }
 
-            return Task.FromResult<ImmutableArray<TextChange>>([]);
+            return SpecializedTasks.False;
         }
 
-        return Task.FromResult(changes);
+        return SpecializedTasks.True;
     }
 }

src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/Formatting/Passes/FormattingDiagnosticValidationPass.cs

@@ -14,14 +14,14 @@
 
 namespace Microsoft.CodeAnalysis.Razor.Formatting;
 
-internal sealed class FormattingDiagnosticValidationPass(ILoggerFactory loggerFactory) : IFormattingPass
+internal sealed class FormattingDiagnosticValidationPass(ILoggerFactory loggerFactory) : IFormattingValidationPass
 {
     private readonly ILogger _logger = loggerFactory.GetOrCreateLogger<FormattingDiagnosticValidationPass>();
 
     // Internal for testing.
     internal bool DebugAssertsEnabled { get; set; } = true;
 
-    public async Task<ImmutableArray<TextChange>> ExecuteAsync(FormattingContext context, ImmutableArray<TextChange> changes, CancellationToken cancellationToken)
+    public async Task<bool> IsValidAsync(FormattingContext context, ImmutableArray<TextChange> changes, CancellationToken cancellationToken)
     {
         var originalDiagnostics = context.CodeDocument.GetSyntaxTree().Diagnostics;
 
@@ -55,10 +55,10 @@ public async Task<ImmutableArray<TextChange>> ExecuteAsync(FormattingContext con
                 Debug.Fail("A formatting result was rejected because the formatted text produced different diagnostics compared to the original text.");
             }
 
-            return [];
+            return false;
         }
 
-        return changes;
+        return true;
     }
 
     private class LocationIgnoringDiagnosticComparer : IEqualityComparer<RazorDiagnostic>

src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/Formatting/RazorFormattingService.cs

@@ -30,7 +30,7 @@ internal class RazorFormattingService : IRazorFormattingService
     private static readonly FrozenSet<string> s_htmlTriggerCharacterSet = FrozenSet.ToFrozenSet(["\n", "{", "}", ";"], StringComparer.Ordinal);
 
     private readonly ImmutableArray<IFormattingPass> _documentFormattingPasses;
-    private readonly ImmutableArray<IFormattingPass> _validationPasses;
+    private readonly ImmutableArray<IFormattingValidationPass> _validationPasses;
     private readonly CSharpOnTypeFormattingPass _csharpOnTypeFormattingPass;
     private readonly HtmlOnTypeFormattingPass _htmlOnTypeFormattingPass;
     private readonly LanguageServerFeatureOptions _languageServerFeatureOptions;
@@ -55,13 +55,11 @@ public RazorFormattingService(
                 new New.HtmlFormattingPass(loggerFactory),
                 new RazorFormattingPass(languageServerFeatureOptions, loggerFactory),
                 new New.CSharpFormattingPass(hostServicesProvider, loggerFactory),
-                .. _validationPasses
             ]
             : [
                 new HtmlFormattingPass(loggerFactory),
                 new RazorFormattingPass(languageServerFeatureOptions, loggerFactory),
                 new CSharpFormattingPass(documentMappingService, hostServicesProvider, loggerFactory),
-                .. _validationPasses
             ];
     }
 
@@ -121,6 +119,16 @@ public async Task<ImmutableArray<TextChange>> GetDocumentFormattingChangesAsync(
             : result.Where(e => linePositionSpan.LineOverlapsWith(sourceText.GetLinePositionSpan(e.Span))).ToImmutableArray();
 
         var normalizedChanges = NormalizeLineEndings(originalText, filteredChanges);
+
+        foreach (var validationPass in _validationPasses)
+        {
+            var isValid = await validationPass.IsValidAsync(context, normalizedChanges, cancellationToken).ConfigureAwait(false);
+            if (!isValid)
+            {
+                return [];
+            }
+        }
+
         return originalText.MinimizeTextChanges(normalizedChanges);
     }
 
@@ -139,9 +147,10 @@ public async Task<ImmutableArray<TextChange>> GetCSharpOnTypeFormattingChangesAs
                 options,
                 hostDocumentIndex,
                 triggerCharacter,
-                [_csharpOnTypeFormattingPass, .. _validationPasses],
+                _csharpOnTypeFormattingPass,
                 collapseChanges: false,
                 automaticallyAddUsings: false,
+                validate: true,
                 cancellationToken: cancellationToken).ConfigureAwait(false);
     }
 
@@ -159,9 +168,10 @@ public async Task<ImmutableArray<TextChange>> GetHtmlOnTypeFormattingChangesAsyn
                 options,
                 hostDocumentIndex,
                 triggerCharacter,
-                [_htmlOnTypeFormattingPass, .. _validationPasses],
+                _htmlOnTypeFormattingPass,
                 collapseChanges: false,
                 automaticallyAddUsings: false,
+                validate: true,
                 cancellationToken: cancellationToken).ConfigureAwait(false);
     }
 
@@ -178,9 +188,10 @@ public async Task<ImmutableArray<TextChange>> GetHtmlOnTypeFormattingChangesAsyn
             options,
             hostDocumentIndex: 0,
             triggerCharacter: '\0',
-            [_csharpOnTypeFormattingPass, .. _validationPasses],
+            _csharpOnTypeFormattingPass,
             collapseChanges: false,
             automaticallyAddUsings: false,
+            validate: true,
             cancellationToken: cancellationToken).ConfigureAwait(false);
         return razorChanges.SingleOrDefault();
     }
@@ -198,9 +209,10 @@ public async Task<ImmutableArray<TextChange>> GetHtmlOnTypeFormattingChangesAsyn
             options,
             hostDocumentIndex: 0,
             triggerCharacter: '\0',
-            [_csharpOnTypeFormattingPass],
+            _csharpOnTypeFormattingPass,
             collapseChanges: true,
             automaticallyAddUsings: true,
+            validate: false,
             cancellationToken: cancellationToken).ConfigureAwait(false);
         return razorChanges.SingleOrDefault();
     }
@@ -220,9 +232,10 @@ public async Task<ImmutableArray<TextChange>> GetHtmlOnTypeFormattingChangesAsyn
             options,
             hostDocumentIndex: 0,
             triggerCharacter: '\0',
-            [_csharpOnTypeFormattingPass],
+            _csharpOnTypeFormattingPass,
             collapseChanges: true,
             automaticallyAddUsings: false,
+            validate: false,
             cancellationToken: cancellationToken).ConfigureAwait(false);
 
         razorChanges = UnwrapCSharpSnippets(razorChanges);
@@ -249,9 +262,10 @@ private async Task<ImmutableArray<TextChange>> ApplyFormattedChangesAsync(
         RazorFormattingOptions options,
         int hostDocumentIndex,
         char triggerCharacter,
-        ImmutableArray<IFormattingPass> formattingPasses,
+        IFormattingPass formattingPass,
         bool collapseChanges,
         bool automaticallyAddUsings,
+        bool validate,
         CancellationToken cancellationToken)
     {
         // If we only received a single edit, let's always return a single edit back.
@@ -265,17 +279,23 @@ private async Task<ImmutableArray<TextChange>> ApplyFormattedChangesAsync(
             automaticallyAddUsings: automaticallyAddUsings,
             hostDocumentIndex,
             triggerCharacter);
-        var result = generatedDocumentChanges;
-
-        foreach (var pass in formattingPasses)
-        {
-            cancellationToken.ThrowIfCancellationRequested();
-            result = await pass.ExecuteAsync(context, result, cancellationToken).ConfigureAwait(false);
-        }
 
+        var result = await formattingPass.ExecuteAsync(context, generatedDocumentChanges, cancellationToken).ConfigureAwait(false);
         var originalText = context.SourceText;
         var razorChanges = originalText.MinimizeTextChanges(result);
 
+        if (validate)
+        {
+            foreach (var validationPass in _validationPasses)
+            {
+                var isValid = await validationPass.IsValidAsync(context, razorChanges, cancellationToken).ConfigureAwait(false);
+                if (!isValid)
+                {
+                    return [];
+                }
+            }
+        }
+
         if (collapseChanges)
         {
             var collapsedEdit = MergeChanges(razorChanges, originalText);
@@ -355,9 +375,17 @@ private static ImmutableArray<TextChange> ReplaceInChanges(ImmutableArray<TextCh
         return changes.DrainToImmutable();
     }
 
-    internal static class TestAccessor
+    internal TestAccessor GetTestAccessor() => new(this);
+
+    internal class TestAccessor(RazorFormattingService service)
     {
         public static FrozenSet<string> GetCSharpTriggerCharacterSet() => s_csharpTriggerCharacterSet;
         public static FrozenSet<string> GetHtmlTriggerCharacterSet() => s_htmlTriggerCharacterSet;
+
+        public void SetDebugAssertsEnabled(bool debugAssertsEnabled)
+        {
+            var contentValidationPass = service._validationPasses.OfType<FormattingContentValidationPass>().Single();
+            contentValidationPass.DebugAssertsEnabled = debugAssertsEnabled;
+        }
     }
 }

src/Razor/test/Microsoft.AspNetCore.Razor.LanguageServer.Test/Formatting_NetFx/DocumentFormattingTest.cs

@@ -6082,4 +6082,37 @@ await RunFormattingTestAsync(
             input: code,
             expected: code);
     }
+
+    [FormattingTestFact(SkipOldFormattingEngine = true)]
+    [WorkItem("https://github.com/dotnet/razor/issues/11777")]
+    public Task RangeFormat_AfterProperty()
+        => RunFormattingTestAsync(
+            input: """
+                @code
+                {
+                    public string S
+                    {
+                        get => _s;
+                        set
+                        {
+                            _s = value;
+                        }
+                    } [|private string _s = "";|]
+                }
+                """,
+            expected: """
+                @code
+                {
+                    public string S
+                    {
+                        get => _s;
+                        set
+                        {
+                            _s = value;
+                        }
+                    } private string _s = "";
+                }
+                """,
+            debugAssertsEnabled: false
+            );
 }

src/Razor/test/Microsoft.AspNetCore.Razor.LanguageServer.Test/Formatting_NetFx/DocumentRangeFormattingEndpointTest.cs

@@ -2,8 +2,6 @@
 // Licensed under the MIT license. See License.txt in the project root for license information.
 
 using System;
-using System.Text;
-using System.Text.Json;
 using System.Threading.Tasks;
 using Microsoft.AspNetCore.Razor.Language;
 using Xunit;

src/Razor/test/Microsoft.AspNetCore.Razor.LanguageServer.Test/Formatting_NetFx/FormattingContentValidationPassTest.cs

@@ -30,14 +30,13 @@ public async Task Execute_NonDestructiveEdit_Allowed()
             """;
         var context = CreateFormattingContext(source);
         var edits = ImmutableArray.Create(new TextChange(source.Span, "    "));
-        var input = edits;
         var pass = GetPass();
 
         // Act
-        var result = await pass.ExecuteAsync(context, edits, DisposalToken);
+        var result = await pass.IsValidAsync(context, edits, DisposalToken);
 
         // Assert
-        Assert.Equal(input, result);
+        Assert.True(result);
     }
 
     [Fact]
@@ -55,10 +54,10 @@ public async Task Execute_DestructiveEdit_Rejected()
         var pass = GetPass();
 
         // Act
-        var result = await pass.ExecuteAsync(context, input, DisposalToken);
+        var result = await pass.IsValidAsync(context, input, DisposalToken);
 
         // Assert
-        Assert.Empty(result);
+        Assert.False(result);
     }
 
     private FormattingContentValidationPass GetPass()

src/Razor/test/Microsoft.AspNetCore.Razor.LanguageServer.Test/Formatting_NetFx/FormattingDiagnosticValidationPassTest.cs

@@ -28,14 +28,13 @@ public async Task ExecuteAsync_NonDestructiveEdit_Allowed()
             """;
         var context = CreateFormattingContext(source);
         var edits = ImmutableArray.Create(new TextChange(source.Span, "    "));
-        var input = edits;
         var pass = GetPass();
 
         // Act
-        var result = await pass.ExecuteAsync(context, input, DisposalToken);
+        var result = await pass.IsValidAsync(context, edits, DisposalToken);
 
         // Assert
-        Assert.Equal(input, result);
+        Assert.True(result);
     }
 
     [Fact]
@@ -53,10 +52,10 @@ public class Foo { }
         var pass = GetPass();
 
         // Act
-        var result = await pass.ExecuteAsync(context, [badEdit], DisposalToken);
+        var result = await pass.IsValidAsync(context, [badEdit], DisposalToken);
 
         // Assert
-        Assert.Empty(result);
+        Assert.False(result);
     }
 
     private FormattingDiagnosticValidationPass GetPass()

src/Razor/test/Microsoft.AspNetCore.Razor.LanguageServer.Test/Formatting_NetFx/FormattingTestBase.cs

@@ -64,13 +64,14 @@ private protected async Task RunFormattingTestAsync(
         ImmutableArray<TagHelperDescriptor> tagHelpers = default,
         bool allowDiagnostics = false,
         bool codeBlockBraceOnNextLine = false,
-        bool inGlobalNamespace = false)
+        bool inGlobalNamespace = false,
+        bool debugAssertsEnabled = true)
     {
         (input, expected) = ProcessFormattingContext(input, expected);
 
         var razorLSPOptions = RazorLSPOptions.Default with { CodeBlockBraceOnNextLine = codeBlockBraceOnNextLine };
 
-        await RunFormattingTestInternalAsync(input, expected, tabSize, insertSpaces, fileKind, tagHelpers, allowDiagnostics, razorLSPOptions, inGlobalNamespace);
+        await RunFormattingTestInternalAsync(input, expected, tabSize, insertSpaces, fileKind, tagHelpers, allowDiagnostics, razorLSPOptions, inGlobalNamespace, debugAssertsEnabled);
     }
 
     private async Task RunFormattingTestInternalAsync(
@@ -82,7 +83,8 @@ private async Task RunFormattingTestInternalAsync(
         ImmutableArray<TagHelperDescriptor> tagHelpers,
         bool allowDiagnostics,
         RazorLSPOptions? razorLSPOptions,
-        bool inGlobalNamespace)
+        bool inGlobalNamespace,
+        bool debugAssertsEnabled)
     {
         // Arrange
         var fileKindValue = fileKind ?? RazorFileKind.Component;
@@ -109,7 +111,7 @@ private async Task RunFormattingTestInternalAsync(
 
         var languageServerFeatureOptions = new TestLanguageServerFeatureOptions(useNewFormattingEngine: _context.UseNewFormattingEngine);
 
-        var formattingService = await TestRazorFormattingService.CreateWithFullSupportAsync(LoggerFactory, codeDocument, razorLSPOptions, languageServerFeatureOptions);
+        var formattingService = await TestRazorFormattingService.CreateWithFullSupportAsync(LoggerFactory, codeDocument, razorLSPOptions, languageServerFeatureOptions, debugAssertsEnabled);
         var documentContext = new DocumentContext(uri, documentSnapshot, projectContext: null);
 
         var client = new FormattingLanguageServerClient(_htmlFormattingService, LoggerFactory);