From 8695b4d74e27fa2df35e7359b0306b089290de4a Mon Sep 17 00:00:00 2001 From: Andrew Hall Date: Thu, 1 May 2025 16:50:39 -0700 Subject: [PATCH 1/5] Make it so validation happens after filtering and normalization --- .../Formatting/IFormattingValidationPass.cs | 14 +++++ .../Passes/FormattingContentValidationPass.cs | 9 +-- .../FormattingDiagnosticValidationPass.cs | 8 +-- .../Formatting/RazorFormattingService.cs | 59 ++++++++++++++++--- .../DocumentFormattingTest.cs | 32 ++++++++++ .../FormattingContentValidationPassTest.cs | 9 ++- .../FormattingDiagnosticValidationPassTest.cs | 9 ++- .../Formatting_NetFx/FormattingTestBase.cs | 10 ++-- .../Formatting_NetFx/OnTypeFormattingTest.cs | 31 ++++++++++ .../TestRazorFormattingService.cs | 14 ++++- .../Cohost/FormattingTestBase.cs | 8 ++- 11 files changed, 170 insertions(+), 33 deletions(-) create mode 100644 src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/Formatting/IFormattingValidationPass.cs diff --git a/src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/Formatting/IFormattingValidationPass.cs b/src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/Formatting/IFormattingValidationPass.cs new file mode 100644 index 00000000000..bf686936d8e --- /dev/null +++ b/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 IsValidAsync(FormattingContext formattingContext, ImmutableArray changes, CancellationToken cancellationToken); +} diff --git a/src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/Formatting/Passes/FormattingContentValidationPass.cs b/src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/Formatting/Passes/FormattingContentValidationPass.cs index 6e7c9470052..cddcc42b1d6 100644 --- a/src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/Formatting/Passes/FormattingContentValidationPass.cs +++ b/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(); // Internal for testing. internal bool DebugAssertsEnabled { get; set; } = true; - public Task> ExecuteAsync(FormattingContext context, ImmutableArray changes, CancellationToken cancellationToken) + public Task IsValidAsync(FormattingContext context, ImmutableArray changes, CancellationToken cancellationToken) { var text = context.SourceText; var changedText = text.WithChanges(changes); @@ -47,9 +48,9 @@ public Task> 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>([]); + return SpecializedTasks.False; } - return Task.FromResult(changes); + return SpecializedTasks.True; } } diff --git a/src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/Formatting/Passes/FormattingDiagnosticValidationPass.cs b/src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/Formatting/Passes/FormattingDiagnosticValidationPass.cs index 63344d0c1f1..7a80926b25b 100644 --- a/src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/Formatting/Passes/FormattingDiagnosticValidationPass.cs +++ b/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(); // Internal for testing. internal bool DebugAssertsEnabled { get; set; } = true; - public async Task> ExecuteAsync(FormattingContext context, ImmutableArray changes, CancellationToken cancellationToken) + public async Task IsValidAsync(FormattingContext context, ImmutableArray changes, CancellationToken cancellationToken) { var originalDiagnostics = context.CodeDocument.GetSyntaxTree().Diagnostics; @@ -55,10 +55,10 @@ public async Task> 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 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 38166958a2a..174e3fac637 100644 --- a/src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/Formatting/RazorFormattingService.cs +++ b/src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/Formatting/RazorFormattingService.cs @@ -30,7 +30,7 @@ internal class RazorFormattingService : IRazorFormattingService private static readonly FrozenSet s_htmlTriggerCharacterSet = FrozenSet.ToFrozenSet(["\n", "{", "}", ";"], StringComparer.Ordinal); private readonly ImmutableArray _documentFormattingPasses; - private readonly ImmutableArray _validationPasses; + private readonly ImmutableArray _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> 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> GetCSharpOnTypeFormattingChangesAs options, hostDocumentIndex, triggerCharacter, - [_csharpOnTypeFormattingPass, .. _validationPasses], + [_csharpOnTypeFormattingPass], collapseChanges: false, automaticallyAddUsings: false, + validate: true, cancellationToken: cancellationToken).ConfigureAwait(false); } @@ -159,9 +168,10 @@ public async Task> GetHtmlOnTypeFormattingChangesAsyn options, hostDocumentIndex, triggerCharacter, - [_htmlOnTypeFormattingPass, .. _validationPasses], + [_htmlOnTypeFormattingPass], collapseChanges: false, automaticallyAddUsings: false, + validate: true, cancellationToken: cancellationToken).ConfigureAwait(false); } @@ -178,9 +188,10 @@ public async Task> GetHtmlOnTypeFormattingChangesAsyn options, hostDocumentIndex: 0, triggerCharacter: '\0', - [_csharpOnTypeFormattingPass, .. _validationPasses], + [_csharpOnTypeFormattingPass], collapseChanges: false, automaticallyAddUsings: false, + validate: true, cancellationToken: cancellationToken).ConfigureAwait(false); return razorChanges.SingleOrDefault(); } @@ -201,6 +212,7 @@ public async Task> GetHtmlOnTypeFormattingChangesAsyn [_csharpOnTypeFormattingPass], collapseChanges: true, automaticallyAddUsings: true, + validate: false, cancellationToken: cancellationToken).ConfigureAwait(false); return razorChanges.SingleOrDefault(); } @@ -223,6 +235,7 @@ public async Task> GetHtmlOnTypeFormattingChangesAsyn [_csharpOnTypeFormattingPass], collapseChanges: true, automaticallyAddUsings: false, + validate: false, cancellationToken: cancellationToken).ConfigureAwait(false); razorChanges = UnwrapCSharpSnippets(razorChanges); @@ -252,6 +265,7 @@ private async Task> ApplyFormattedChangesAsync( ImmutableArray formattingPasses, bool collapseChanges, bool automaticallyAddUsings, + bool validate, CancellationToken cancellationToken) { // If we only received a single edit, let's always return a single edit back. @@ -285,7 +299,33 @@ private async Task> ApplyFormattedChangesAsync( return []; } - return [collapsedEdit]; + ImmutableArray collapsedEdits = [collapsedEdit]; + + if (validate) + { + foreach (var validationPass in _validationPasses) + { + var isValid = await validationPass.IsValidAsync(context, collapsedEdits, cancellationToken).ConfigureAwait(false); + if (!isValid) + { + return []; + } + } + } + + return collapsedEdits; + } + + if (validate) + { + foreach (var validationPass in _validationPasses) + { + var isValid = await validationPass.IsValidAsync(context, razorChanges, cancellationToken).ConfigureAwait(false); + if (!isValid) + { + return []; + } + } } return razorChanges; @@ -355,9 +395,12 @@ private static ImmutableArray ReplaceInChanges(ImmutableArray new(this); + + internal class TestAccessor(RazorFormattingService service) { public static FrozenSet GetCSharpTriggerCharacterSet() => s_csharpTriggerCharacterSet; public static FrozenSet GetHtmlTriggerCharacterSet() => s_htmlTriggerCharacterSet; + public ImmutableArray FormattingValidationPasses => service._validationPasses; } } diff --git a/src/Razor/test/Microsoft.AspNetCore.Razor.LanguageServer.Test/Formatting_NetFx/DocumentFormattingTest.cs b/src/Razor/test/Microsoft.AspNetCore.Razor.LanguageServer.Test/Formatting_NetFx/DocumentFormattingTest.cs index d69d65bfecd..219715a0a53 100644 --- a/src/Razor/test/Microsoft.AspNetCore.Razor.LanguageServer.Test/Formatting_NetFx/DocumentFormattingTest.cs +++ b/src/Razor/test/Microsoft.AspNetCore.Razor.LanguageServer.Test/Formatting_NetFx/DocumentFormattingTest.cs @@ -6082,4 +6082,36 @@ await RunFormattingTestAsync( input: code, expected: code); } + + [FormattingTestFact] + 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 + ); } diff --git a/src/Razor/test/Microsoft.AspNetCore.Razor.LanguageServer.Test/Formatting_NetFx/FormattingContentValidationPassTest.cs b/src/Razor/test/Microsoft.AspNetCore.Razor.LanguageServer.Test/Formatting_NetFx/FormattingContentValidationPassTest.cs index d9599dea581..0ccbab4e66e 100644 --- a/src/Razor/test/Microsoft.AspNetCore.Razor.LanguageServer.Test/Formatting_NetFx/FormattingContentValidationPassTest.cs +++ b/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() diff --git a/src/Razor/test/Microsoft.AspNetCore.Razor.LanguageServer.Test/Formatting_NetFx/FormattingDiagnosticValidationPassTest.cs b/src/Razor/test/Microsoft.AspNetCore.Razor.LanguageServer.Test/Formatting_NetFx/FormattingDiagnosticValidationPassTest.cs index 3082ce22031..9934082139a 100644 --- a/src/Razor/test/Microsoft.AspNetCore.Razor.LanguageServer.Test/Formatting_NetFx/FormattingDiagnosticValidationPassTest.cs +++ b/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() diff --git a/src/Razor/test/Microsoft.AspNetCore.Razor.LanguageServer.Test/Formatting_NetFx/FormattingTestBase.cs b/src/Razor/test/Microsoft.AspNetCore.Razor.LanguageServer.Test/Formatting_NetFx/FormattingTestBase.cs index efa57db14e4..ea35ac51588 100644 --- a/src/Razor/test/Microsoft.AspNetCore.Razor.LanguageServer.Test/Formatting_NetFx/FormattingTestBase.cs +++ b/src/Razor/test/Microsoft.AspNetCore.Razor.LanguageServer.Test/Formatting_NetFx/FormattingTestBase.cs @@ -64,13 +64,14 @@ private protected async Task RunFormattingTestAsync( ImmutableArray 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 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); diff --git a/src/Razor/test/Microsoft.AspNetCore.Razor.LanguageServer.Test/Formatting_NetFx/OnTypeFormattingTest.cs b/src/Razor/test/Microsoft.AspNetCore.Razor.LanguageServer.Test/Formatting_NetFx/OnTypeFormattingTest.cs index 5580eafd50f..9d671214a64 100644 --- a/src/Razor/test/Microsoft.AspNetCore.Razor.LanguageServer.Test/Formatting_NetFx/OnTypeFormattingTest.cs +++ b/src/Razor/test/Microsoft.AspNetCore.Razor.LanguageServer.Test/Formatting_NetFx/OnTypeFormattingTest.cs @@ -1163,4 +1163,35 @@ public class Foo { } triggerCharacter: '}', expectedChangedLines: 1); } + + [FormattingTestFact] + public Task Paste_AfterProperty() + => RunOnTypeFormattingTestAsync( + 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 = ""; + } + """, + triggerCharacter: ';', + expectedChangedLines: 2); } diff --git a/src/Razor/test/Microsoft.AspNetCore.Razor.LanguageServer.Test/Formatting_NetFx/TestRazorFormattingService.cs b/src/Razor/test/Microsoft.AspNetCore.Razor.LanguageServer.Test/Formatting_NetFx/TestRazorFormattingService.cs index cefbea2bd90..dce24e905eb 100644 --- a/src/Razor/test/Microsoft.AspNetCore.Razor.LanguageServer.Test/Formatting_NetFx/TestRazorFormattingService.cs +++ b/src/Razor/test/Microsoft.AspNetCore.Razor.LanguageServer.Test/Formatting_NetFx/TestRazorFormattingService.cs @@ -20,7 +20,8 @@ public static async Task CreateWithFullSupportAsync( ILoggerFactory loggerFactory, RazorCodeDocument? codeDocument = null, RazorLSPOptions? razorLSPOptions = null, - LanguageServerFeatureOptions? languageServerFeatureOptions = null) + LanguageServerFeatureOptions? languageServerFeatureOptions = null, + bool debugAssertsEnabled = false) { codeDocument ??= TestRazorCodeDocument.CreateEmpty(); @@ -43,6 +44,15 @@ public static async Task CreateWithFullSupportAsync( var hostServicesProvider = new DefaultHostServicesProvider(); - return new RazorFormattingService(mappingService, hostServicesProvider, languageServerFeatureOptions, loggerFactory); + var service = new RazorFormattingService(mappingService, hostServicesProvider, languageServerFeatureOptions, loggerFactory); + foreach (var validation in service.GetTestAccessor().FormattingValidationPasses) + { + if (validation is FormattingContentValidationPass contentValidationPass) + { + contentValidationPass.DebugAssertsEnabled = debugAssertsEnabled; + } + } + + return service; } } diff --git a/src/Razor/test/Microsoft.VisualStudio.LanguageServices.Razor.Test/Cohost/FormattingTestBase.cs b/src/Razor/test/Microsoft.VisualStudio.LanguageServices.Razor.Test/Cohost/FormattingTestBase.cs index ec7076811e5..411af1b0ee6 100644 --- a/src/Razor/test/Microsoft.VisualStudio.LanguageServices.Razor.Test/Cohost/FormattingTestBase.cs +++ b/src/Razor/test/Microsoft.VisualStudio.LanguageServices.Razor.Test/Cohost/FormattingTestBase.cs @@ -42,7 +42,8 @@ private protected async Task RunFormattingTestAsync( bool codeBlockBraceOnNextLine = false, bool insertSpaces = true, int tabSize = 4, - bool allowDiagnostics = false) + bool allowDiagnostics = false, + bool debugAssertsEnabled = true) { (input, expected) = ProcessFormattingContext(input, expected); @@ -59,6 +60,11 @@ private protected async Task RunFormattingTestAsync( //Assert.False(csharpDocument.Diagnostics.Any(), "Error creating document:" + Environment.NewLine + string.Join(Environment.NewLine, csharpDocument.Diagnostics)); } + if (debugAssertsEnabled) + { + // Cohosting tests do not use this value + } + var generatedHtml = await RemoteServiceInvoker.TryInvokeAsync(document.Project.Solution, (service, solutionInfo, ct) => service.GetHtmlDocumentTextAsync(solutionInfo, document.Id, ct), DisposalToken).ConfigureAwait(false); From dc71bf8de079b499b878083248ad085aa6cb0b1e Mon Sep 17 00:00:00 2001 From: Andrew Hall Date: Thu, 1 May 2025 16:53:12 -0700 Subject: [PATCH 2/5] Undo OnType test, it was a range format --- .../Formatting_NetFx/OnTypeFormattingTest.cs | 31 ------------------- 1 file changed, 31 deletions(-) diff --git a/src/Razor/test/Microsoft.AspNetCore.Razor.LanguageServer.Test/Formatting_NetFx/OnTypeFormattingTest.cs b/src/Razor/test/Microsoft.AspNetCore.Razor.LanguageServer.Test/Formatting_NetFx/OnTypeFormattingTest.cs index 9d671214a64..5580eafd50f 100644 --- a/src/Razor/test/Microsoft.AspNetCore.Razor.LanguageServer.Test/Formatting_NetFx/OnTypeFormattingTest.cs +++ b/src/Razor/test/Microsoft.AspNetCore.Razor.LanguageServer.Test/Formatting_NetFx/OnTypeFormattingTest.cs @@ -1163,35 +1163,4 @@ public class Foo { } triggerCharacter: '}', expectedChangedLines: 1); } - - [FormattingTestFact] - public Task Paste_AfterProperty() - => RunOnTypeFormattingTestAsync( - 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 = ""; - } - """, - triggerCharacter: ';', - expectedChangedLines: 2); } From 2343c1cab2822f75b04c52f1a303f51084107255 Mon Sep 17 00:00:00 2001 From: Andrew Hall Date: Thu, 1 May 2025 17:22:56 -0700 Subject: [PATCH 3/5] PR feedback --- .../Formatting/RazorFormattingService.cs | 65 +++++++------------ .../DocumentRangeFormattingEndpointTest.cs | 2 - .../TestRazorFormattingService.cs | 9 +-- 3 files changed, 26 insertions(+), 50 deletions(-) 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 174e3fac637..7b90987cf62 100644 --- a/src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/Formatting/RazorFormattingService.cs +++ b/src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/Formatting/RazorFormattingService.cs @@ -147,7 +147,7 @@ public async Task> GetCSharpOnTypeFormattingChangesAs options, hostDocumentIndex, triggerCharacter, - [_csharpOnTypeFormattingPass], + _csharpOnTypeFormattingPass, collapseChanges: false, automaticallyAddUsings: false, validate: true, @@ -168,7 +168,7 @@ public async Task> GetHtmlOnTypeFormattingChangesAsyn options, hostDocumentIndex, triggerCharacter, - [_htmlOnTypeFormattingPass], + _htmlOnTypeFormattingPass, collapseChanges: false, automaticallyAddUsings: false, validate: true, @@ -188,7 +188,7 @@ public async Task> GetHtmlOnTypeFormattingChangesAsyn options, hostDocumentIndex: 0, triggerCharacter: '\0', - [_csharpOnTypeFormattingPass], + _csharpOnTypeFormattingPass, collapseChanges: false, automaticallyAddUsings: false, validate: true, @@ -209,7 +209,7 @@ public async Task> GetHtmlOnTypeFormattingChangesAsyn options, hostDocumentIndex: 0, triggerCharacter: '\0', - [_csharpOnTypeFormattingPass], + _csharpOnTypeFormattingPass, collapseChanges: true, automaticallyAddUsings: true, validate: false, @@ -232,7 +232,7 @@ public async Task> GetHtmlOnTypeFormattingChangesAsyn options, hostDocumentIndex: 0, triggerCharacter: '\0', - [_csharpOnTypeFormattingPass], + _csharpOnTypeFormattingPass, collapseChanges: true, automaticallyAddUsings: false, validate: false, @@ -262,7 +262,7 @@ private async Task> ApplyFormattedChangesAsync( RazorFormattingOptions options, int hostDocumentIndex, char triggerCharacter, - ImmutableArray formattingPasses, + IFormattingPass formattingPass, bool collapseChanges, bool automaticallyAddUsings, bool validate, @@ -279,43 +279,11 @@ private async Task> 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 (collapseChanges) - { - var collapsedEdit = MergeChanges(razorChanges, originalText); - if (collapsedEdit.NewText is null or { Length: 0 } && - collapsedEdit.Span.IsEmpty) - { - return []; - } - - ImmutableArray collapsedEdits = [collapsedEdit]; - - if (validate) - { - foreach (var validationPass in _validationPasses) - { - var isValid = await validationPass.IsValidAsync(context, collapsedEdits, cancellationToken).ConfigureAwait(false); - if (!isValid) - { - return []; - } - } - } - - return collapsedEdits; - } - if (validate) { foreach (var validationPass in _validationPasses) @@ -328,6 +296,18 @@ private async Task> ApplyFormattedChangesAsync( } } + if (collapseChanges) + { + var collapsedEdit = MergeChanges(razorChanges, originalText); + if (collapsedEdit.NewText is null or { Length: 0 } && + collapsedEdit.Span.IsEmpty) + { + return []; + } + + return [collapsedEdit]; + } + return razorChanges; } @@ -401,6 +381,11 @@ internal class TestAccessor(RazorFormattingService service) { public static FrozenSet GetCSharpTriggerCharacterSet() => s_csharpTriggerCharacterSet; public static FrozenSet GetHtmlTriggerCharacterSet() => s_htmlTriggerCharacterSet; - public ImmutableArray FormattingValidationPasses => service._validationPasses; + + public void SetDebugAssertsEnabled(bool debugAssertsEnabled) + { + var contentValidationPass = service._validationPasses.OfType().Single(); + contentValidationPass.DebugAssertsEnabled = debugAssertsEnabled; + } } } diff --git a/src/Razor/test/Microsoft.AspNetCore.Razor.LanguageServer.Test/Formatting_NetFx/DocumentRangeFormattingEndpointTest.cs b/src/Razor/test/Microsoft.AspNetCore.Razor.LanguageServer.Test/Formatting_NetFx/DocumentRangeFormattingEndpointTest.cs index bdf3f31ec25..1f0cba0db9f 100644 --- a/src/Razor/test/Microsoft.AspNetCore.Razor.LanguageServer.Test/Formatting_NetFx/DocumentRangeFormattingEndpointTest.cs +++ b/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; diff --git a/src/Razor/test/Microsoft.AspNetCore.Razor.LanguageServer.Test/Formatting_NetFx/TestRazorFormattingService.cs b/src/Razor/test/Microsoft.AspNetCore.Razor.LanguageServer.Test/Formatting_NetFx/TestRazorFormattingService.cs index dce24e905eb..a82ba6fb61c 100644 --- a/src/Razor/test/Microsoft.AspNetCore.Razor.LanguageServer.Test/Formatting_NetFx/TestRazorFormattingService.cs +++ b/src/Razor/test/Microsoft.AspNetCore.Razor.LanguageServer.Test/Formatting_NetFx/TestRazorFormattingService.cs @@ -45,14 +45,7 @@ public static async Task CreateWithFullSupportAsync( var hostServicesProvider = new DefaultHostServicesProvider(); var service = new RazorFormattingService(mappingService, hostServicesProvider, languageServerFeatureOptions, loggerFactory); - foreach (var validation in service.GetTestAccessor().FormattingValidationPasses) - { - if (validation is FormattingContentValidationPass contentValidationPass) - { - contentValidationPass.DebugAssertsEnabled = debugAssertsEnabled; - } - } - + service.GetTestAccessor().SetDebugAssertsEnabled(debugAssertsEnabled); return service; } } From d6eaed4dcc5c174bd7b69393682ce17881b55eb7 Mon Sep 17 00:00:00 2001 From: Andrew Hall Date: Mon, 5 May 2025 12:24:13 -0700 Subject: [PATCH 4/5] Add work item and skip on old formatting engine --- .../Formatting_NetFx/DocumentFormattingTest.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/Razor/test/Microsoft.AspNetCore.Razor.LanguageServer.Test/Formatting_NetFx/DocumentFormattingTest.cs b/src/Razor/test/Microsoft.AspNetCore.Razor.LanguageServer.Test/Formatting_NetFx/DocumentFormattingTest.cs index 219715a0a53..654b721f584 100644 --- a/src/Razor/test/Microsoft.AspNetCore.Razor.LanguageServer.Test/Formatting_NetFx/DocumentFormattingTest.cs +++ b/src/Razor/test/Microsoft.AspNetCore.Razor.LanguageServer.Test/Formatting_NetFx/DocumentFormattingTest.cs @@ -6083,7 +6083,8 @@ await RunFormattingTestAsync( expected: code); } - [FormattingTestFact] + [FormattingTestFact(SkipOldFormattingEngine = true)] + [WorkItem("https://github.com/dotnet/razor/issues/11777")] public Task RangeFormat_AfterProperty() => RunFormattingTestAsync( input: """ From 986476449c721b970767e3ff0e13478d44aac24f Mon Sep 17 00:00:00 2001 From: Andrew Hall Date: Tue, 6 May 2025 13:32:56 -0700 Subject: [PATCH 5/5] Use debugAssertsEnabled in cohost test --- .../Cohost/FormattingTestBase.cs | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/Razor/test/Microsoft.VisualStudio.LanguageServices.Razor.Test/Cohost/FormattingTestBase.cs b/src/Razor/test/Microsoft.VisualStudio.LanguageServices.Razor.Test/Cohost/FormattingTestBase.cs index f6579f056d9..6a2f3aea0dc 100644 --- a/src/Razor/test/Microsoft.VisualStudio.LanguageServices.Razor.Test/Cohost/FormattingTestBase.cs +++ b/src/Razor/test/Microsoft.VisualStudio.LanguageServices.Razor.Test/Cohost/FormattingTestBase.cs @@ -61,10 +61,8 @@ private protected async Task RunFormattingTestAsync( //Assert.False(csharpDocument.Diagnostics.Any(), "Error creating document:" + Environment.NewLine + string.Join(Environment.NewLine, csharpDocument.Diagnostics)); } - if (debugAssertsEnabled) - { - // Cohosting tests do not use this value - } + var formattingService = (RazorFormattingService)OOPExportProvider.GetExportedValue(); + formattingService.GetTestAccessor().SetDebugAssertsEnabled(debugAssertsEnabled); var generatedHtml = await RemoteServiceInvoker.TryInvokeAsync(document.Project.Solution, (service, solutionInfo, ct) => service.GetHtmlDocumentTextAsync(solutionInfo, document.Id, ct),