From 9b7ed904a715b06b5abc3babaa2435e1eda21aef Mon Sep 17 00:00:00 2001 From: David Wengier Date: Wed, 30 Oct 2024 17:06:58 +1100 Subject: [PATCH 1/5] Create failing test Sadly, to get this far I had to opt cohosting into FUSE, which will have untold consequences --- .../RemoteClientInitializationOptions.cs | 3 ++ .../RemoteLanguageServerFeatureOptions.cs | 2 +- .../ProjectSystem/RemoteDocumentSnapshot.cs | 28 +++++++++++----- .../ProjectSystem/RemoteSnapshotManager.cs | 3 +- .../Remote/RemoteServiceInvoker.cs | 1 + .../Cohost/CohostEndpointTestBase.cs | 1 + .../Cohost/CohostOnAutoInsertEndpointTest.cs | 33 ++++++++++++++++--- 7 files changed, 55 insertions(+), 16 deletions(-) diff --git a/src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/Remote/RemoteClientInitializationOptions.cs b/src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/Remote/RemoteClientInitializationOptions.cs index 58e0cb07b55..0ca48ba1a20 100644 --- a/src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/Remote/RemoteClientInitializationOptions.cs +++ b/src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/Remote/RemoteClientInitializationOptions.cs @@ -25,4 +25,7 @@ internal struct RemoteClientInitializationOptions [DataMember(Order = 5)] internal required bool ReturnCodeActionAndRenamePathsWithPrefixedSlash; + + [DataMember(Order = 6)] + internal required bool ForceRuntimeCodeGeneration; } diff --git a/src/Razor/src/Microsoft.CodeAnalysis.Remote.Razor/Initialization/RemoteLanguageServerFeatureOptions.cs b/src/Razor/src/Microsoft.CodeAnalysis.Remote.Razor/Initialization/RemoteLanguageServerFeatureOptions.cs index 8d95d417774..ed6b2ddb602 100644 --- a/src/Razor/src/Microsoft.CodeAnalysis.Remote.Razor/Initialization/RemoteLanguageServerFeatureOptions.cs +++ b/src/Razor/src/Microsoft.CodeAnalysis.Remote.Razor/Initialization/RemoteLanguageServerFeatureOptions.cs @@ -41,5 +41,5 @@ internal class RemoteLanguageServerFeatureOptions : LanguageServerFeatureOptions public override bool DisableRazorLanguageServer => throw new InvalidOperationException("This option has not been synced to OOP."); - public override bool ForceRuntimeCodeGeneration => throw new InvalidOperationException("This option has not been synced to OOP."); + public override bool ForceRuntimeCodeGeneration => _options.ForceRuntimeCodeGeneration; } diff --git a/src/Razor/src/Microsoft.CodeAnalysis.Remote.Razor/ProjectSystem/RemoteDocumentSnapshot.cs b/src/Razor/src/Microsoft.CodeAnalysis.Remote.Razor/ProjectSystem/RemoteDocumentSnapshot.cs index 4c566a4da5c..8231e30a12f 100644 --- a/src/Razor/src/Microsoft.CodeAnalysis.Remote.Razor/ProjectSystem/RemoteDocumentSnapshot.cs +++ b/src/Razor/src/Microsoft.CodeAnalysis.Remote.Razor/ProjectSystem/RemoteDocumentSnapshot.cs @@ -66,14 +66,29 @@ public ValueTask GetGeneratedOutputAsync( bool forceDesignTimeGeneratedOutput, CancellationToken cancellationToken) { + // We don't cache if we're forcing, as that would break everything else + if (forceDesignTimeGeneratedOutput) + { + return new ValueTask(GetRazorCodeDocumentAsync(forceRuntimeCodeGeneration: false, cancellationToken)); + } + + var forceRuntimeCodeGeneration = ProjectSnapshot.SolutionSnapshot.SnapshotManager.LanguageServerFeatureOptions.ForceRuntimeCodeGeneration; + // TODO: We don't need to worry about locking if we get called from the didOpen/didChange LSP requests, as CLaSP // takes care of that for us, and blocks requests until those are complete. If that doesn't end up happening, // then a locking mechanism here would prevent concurrent compilations. return TryGetGeneratedOutput(out var codeDocument) ? new(codeDocument) - : new(GetGeneratedOutputCoreAsync(cancellationToken)); + : new(GetGeneratedOutputCoreAsync(forceRuntimeCodeGeneration, cancellationToken)); + + async Task GetGeneratedOutputCoreAsync(bool forceRuntimeCodeGeneration, CancellationToken cancellationToken) + { + codeDocument = await GetRazorCodeDocumentAsync(forceRuntimeCodeGeneration, cancellationToken).ConfigureAwait(false); - async Task GetGeneratedOutputCoreAsync(CancellationToken cancellationToken) + return _codeDocument ??= InterlockedOperations.Initialize(ref _codeDocument, codeDocument); + } + + async Task GetRazorCodeDocumentAsync(bool forceRuntimeCodeGeneration, CancellationToken cancellationToken) { // The non-cohosted DocumentSnapshot implementation uses DocumentState to get the generated output, and we could do that too // but most of that code is optimized around caching pre-computed results when things change that don't affect the compilation. @@ -87,14 +102,9 @@ async Task GetGeneratedOutputCoreAsync(CancellationToken canc var tagHelpers = await ProjectSnapshot.GetTagHelpersAsync(cancellationToken).ConfigureAwait(false); var imports = await DocumentState.GetImportsAsync(this, projectEngine, cancellationToken).ConfigureAwait(false); - // TODO: Get the configuration for forceRuntimeCodeGeneration - // var forceRuntimeCodeGeneration = _projectSnapshot.Configuration.LanguageServerFlags?.ForceRuntimeCodeGeneration ?? false; - - codeDocument = await DocumentState - .GenerateCodeDocumentAsync(this, projectEngine, imports, tagHelpers, forceRuntimeCodeGeneration: false, cancellationToken) + return await DocumentState + .GenerateCodeDocumentAsync(this, projectEngine, imports, tagHelpers, forceRuntimeCodeGeneration: forceRuntimeCodeGeneration, cancellationToken) .ConfigureAwait(false); - - return _codeDocument ??= InterlockedOperations.Initialize(ref _codeDocument, codeDocument); } } diff --git a/src/Razor/src/Microsoft.CodeAnalysis.Remote.Razor/ProjectSystem/RemoteSnapshotManager.cs b/src/Razor/src/Microsoft.CodeAnalysis.Remote.Razor/ProjectSystem/RemoteSnapshotManager.cs index 3af835eb8a0..41ceec49ab6 100644 --- a/src/Razor/src/Microsoft.CodeAnalysis.Remote.Razor/ProjectSystem/RemoteSnapshotManager.cs +++ b/src/Razor/src/Microsoft.CodeAnalysis.Remote.Razor/ProjectSystem/RemoteSnapshotManager.cs @@ -11,10 +11,11 @@ namespace Microsoft.CodeAnalysis.Remote.Razor.ProjectSystem; [Shared] [Export(typeof(RemoteSnapshotManager))] [method: ImportingConstructor] -internal sealed class RemoteSnapshotManager(IFilePathService filePathService, ITelemetryReporter telemetryReporter) +internal sealed class RemoteSnapshotManager(LanguageServerFeatureOptions languageServerFeatureOptions, IFilePathService filePathService, ITelemetryReporter telemetryReporter) { private static readonly ConditionalWeakTable s_solutionToSnapshotMap = new(); + public LanguageServerFeatureOptions LanguageServerFeatureOptions { get; } = languageServerFeatureOptions; public IFilePathService FilePathService { get; } = filePathService; public ITelemetryReporter TelemetryReporter { get; } = telemetryReporter; diff --git a/src/Razor/src/Microsoft.VisualStudio.LanguageServices.Razor/Remote/RemoteServiceInvoker.cs b/src/Razor/src/Microsoft.VisualStudio.LanguageServices.Razor/Remote/RemoteServiceInvoker.cs index a085fcc21a0..1e2e54bcc46 100644 --- a/src/Razor/src/Microsoft.VisualStudio.LanguageServices.Razor/Remote/RemoteServiceInvoker.cs +++ b/src/Razor/src/Microsoft.VisualStudio.LanguageServices.Razor/Remote/RemoteServiceInvoker.cs @@ -136,6 +136,7 @@ private async Task InitializeRemoteClientAsync(RazorRemoteHostClient remoteClien HtmlVirtualDocumentSuffix = _languageServerFeatureOptions.HtmlVirtualDocumentSuffix, IncludeProjectKeyInGeneratedFilePath = _languageServerFeatureOptions.IncludeProjectKeyInGeneratedFilePath, ReturnCodeActionAndRenamePathsWithPrefixedSlash = _languageServerFeatureOptions.ReturnCodeActionAndRenamePathsWithPrefixedSlash, + ForceRuntimeCodeGeneration = _languageServerFeatureOptions.ForceRuntimeCodeGeneration }; _logger.LogDebug($"First OOP call, so initializing OOP service."); diff --git a/src/Razor/test/Microsoft.VisualStudio.LanguageServices.Razor.Test/Cohost/CohostEndpointTestBase.cs b/src/Razor/test/Microsoft.VisualStudio.LanguageServices.Razor.Test/Cohost/CohostEndpointTestBase.cs index fac6c663afa..1a49037fb60 100644 --- a/src/Razor/test/Microsoft.VisualStudio.LanguageServices.Razor.Test/Cohost/CohostEndpointTestBase.cs +++ b/src/Razor/test/Microsoft.VisualStudio.LanguageServices.Razor.Test/Cohost/CohostEndpointTestBase.cs @@ -60,6 +60,7 @@ protected override async Task InitializeAsync() UsePreciseSemanticTokenRanges = false, UseRazorCohostServer = true, ReturnCodeActionAndRenamePathsWithPrefixedSlash = false, + ForceRuntimeCodeGeneration = false }; UpdateClientInitializationOptions(c => c); diff --git a/src/Razor/test/Microsoft.VisualStudio.LanguageServices.Razor.Test/Cohost/CohostOnAutoInsertEndpointTest.cs b/src/Razor/test/Microsoft.VisualStudio.LanguageServices.Razor.Test/Cohost/CohostOnAutoInsertEndpointTest.cs index 74d5036d454..3919a1298df 100644 --- a/src/Razor/test/Microsoft.VisualStudio.LanguageServices.Razor.Test/Cohost/CohostOnAutoInsertEndpointTest.cs +++ b/src/Razor/test/Microsoft.VisualStudio.LanguageServices.Razor.Test/Cohost/CohostOnAutoInsertEndpointTest.cs @@ -120,17 +120,36 @@ void TestMethod() {} formatOnType: false); } - [Fact] - public async Task CSharp_OnEnter() + [Theory] + [CombinatorialData] + public async Task CSharp_OnEnter(bool fuse) { await VerifyOnAutoInsertAsync( input: """ + Hello +
+ Hello +

Hello

+

Hello

+
+ + Hello + @code { void TestMethod() { $$} } """, output: """ + Hello +
+ Hello +

Hello

+

Hello

+
+ + Hello + @code { void TestMethod() { @@ -138,7 +157,8 @@ void TestMethod() } } """, - triggerCharacter: "\n"); + triggerCharacter: "\n", + fuse: fuse); } [Fact] @@ -194,8 +214,11 @@ private async Task VerifyOnAutoInsertAsync( bool insertSpaces = true, int tabSize = 4, bool formatOnType = true, - bool autoClosingTags = true) - { + bool autoClosingTags = true, + bool fuse = false) + { + UpdateClientInitializationOptions(opt => opt with { ForceRuntimeCodeGeneration = fuse }); + var document = await CreateProjectAndRazorDocumentAsync(input.Text); var sourceText = await document.GetTextAsync(DisposalToken); From f959a9a4dabf44f6fd5083da4c25a45b2181d2ae Mon Sep 17 00:00:00 2001 From: David Wengier Date: Wed, 30 Oct 2024 14:27:33 +1100 Subject: [PATCH 2/5] Make sure we get the right code document and pass it in Previously this was using a boolean flag that I thought was the correct pivot point, but I was wrong, and it made things very confusing. This is much clearer, and more importantly, correct. Essentially if we are trying to format and we get passed a C# edit, then we can't force design time or it would be a different document than the original edit came from, if FUSE is on. --- .../Formatting/RazorFormattingService.cs | 98 +++++++++++-------- 1 file changed, 59 insertions(+), 39 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 326ef4093a8..eb03574c68e 100644 --- a/src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/Formatting/RazorFormattingService.cs +++ b/src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/Formatting/RazorFormattingService.cs @@ -119,56 +119,78 @@ public async Task> GetDocumentFormattingChangesAsync( return originalText.MinimizeTextChanges(normalizedChanges); } - public Task> GetCSharpOnTypeFormattingChangesAsync(DocumentContext documentContext, RazorFormattingOptions options, int hostDocumentIndex, char triggerCharacter, CancellationToken cancellationToken) - => ApplyFormattedChangesAsync( - documentContext, - generatedDocumentChanges: [], - options, - hostDocumentIndex, - triggerCharacter, - [_csharpOnTypeFormattingPass, .. _validationPasses], - collapseChanges: false, - isCodeActionFormattingRequest: false, - cancellationToken: cancellationToken); + public async Task> GetCSharpOnTypeFormattingChangesAsync(DocumentContext documentContext, RazorFormattingOptions options, int hostDocumentIndex, char triggerCharacter, CancellationToken cancellationToken) + { + var documentSnapshot = documentContext.Snapshot; + var codeDocument = await documentContext.Snapshot.GetGeneratedOutputAsync(forceDesignTimeGeneratedOutput: true, cancellationToken).ConfigureAwait(false); + + return await ApplyFormattedChangesAsync( + documentSnapshot, + codeDocument, + generatedDocumentChanges: [], + options, + hostDocumentIndex, + triggerCharacter, + [_csharpOnTypeFormattingPass, .. _validationPasses], + collapseChanges: false, + automaticallyAddUsings: false, + cancellationToken: cancellationToken).ConfigureAwait(false); + } - public Task> GetHtmlOnTypeFormattingChangesAsync(DocumentContext documentContext, ImmutableArray htmlChanges, RazorFormattingOptions options, int hostDocumentIndex, char triggerCharacter, CancellationToken cancellationToken) - => ApplyFormattedChangesAsync( - documentContext, - htmlChanges, - options, - hostDocumentIndex, - triggerCharacter, - [_htmlOnTypeFormattingPass, .. _validationPasses], - collapseChanges: false, - isCodeActionFormattingRequest: false, - cancellationToken: cancellationToken); + public async Task> GetHtmlOnTypeFormattingChangesAsync(DocumentContext documentContext, ImmutableArray htmlChanges, RazorFormattingOptions options, int hostDocumentIndex, char triggerCharacter, CancellationToken cancellationToken) + { + var documentSnapshot = documentContext.Snapshot; + var codeDocument = await documentContext.Snapshot.GetGeneratedOutputAsync(forceDesignTimeGeneratedOutput: true, cancellationToken).ConfigureAwait(false); + + return await ApplyFormattedChangesAsync( + documentSnapshot, + codeDocument, + htmlChanges, + options, + hostDocumentIndex, + triggerCharacter, + [_htmlOnTypeFormattingPass, .. _validationPasses], + collapseChanges: false, + automaticallyAddUsings: false, + cancellationToken: cancellationToken).ConfigureAwait(false); + } public async Task TryGetSingleCSharpEditAsync(DocumentContext documentContext, TextChange csharpEdit, RazorFormattingOptions options, CancellationToken cancellationToken) { + var documentSnapshot = documentContext.Snapshot; + // Since we've been provided with an edit from the C# generated doc, forcing design time would make things not line up + var codeDocument = await documentContext.Snapshot.GetGeneratedOutputAsync(forceDesignTimeGeneratedOutput: false, cancellationToken).ConfigureAwait(false); + var razorChanges = await ApplyFormattedChangesAsync( - documentContext, + documentSnapshot, + codeDocument, [csharpEdit], options, hostDocumentIndex: 0, triggerCharacter: '\0', [_csharpOnTypeFormattingPass, .. _validationPasses], collapseChanges: false, - isCodeActionFormattingRequest: false, + automaticallyAddUsings: false, cancellationToken: cancellationToken).ConfigureAwait(false); return razorChanges.SingleOrDefault(); } public async Task TryGetCSharpCodeActionEditAsync(DocumentContext documentContext, ImmutableArray csharpChanges, RazorFormattingOptions options, CancellationToken cancellationToken) { + var documentSnapshot = documentContext.Snapshot; + // Since we've been provided with edits from the C# generated doc, forcing design time would make things not line up + var codeDocument = await documentContext.Snapshot.GetGeneratedOutputAsync(forceDesignTimeGeneratedOutput: false, cancellationToken).ConfigureAwait(false); + var razorChanges = await ApplyFormattedChangesAsync( - documentContext, + documentSnapshot, + codeDocument, csharpChanges, options, hostDocumentIndex: 0, triggerCharacter: '\0', [_csharpOnTypeFormattingPass], collapseChanges: true, - isCodeActionFormattingRequest: true, + automaticallyAddUsings: true, cancellationToken: cancellationToken).ConfigureAwait(false); return razorChanges.SingleOrDefault(); } @@ -177,15 +199,20 @@ public Task> GetHtmlOnTypeFormattingChangesAsync(Docu { csharpChanges = WrapCSharpSnippets(csharpChanges); + var documentSnapshot = documentContext.Snapshot; + // Since we've been provided with edits from the C# generated doc, forcing design time would make things not line up + var codeDocument = await documentContext.Snapshot.GetGeneratedOutputAsync(forceDesignTimeGeneratedOutput: false, cancellationToken).ConfigureAwait(false); + var razorChanges = await ApplyFormattedChangesAsync( - documentContext, + documentSnapshot, + codeDocument, csharpChanges, options, hostDocumentIndex: 0, triggerCharacter: '\0', [_csharpOnTypeFormattingPass], collapseChanges: true, - isCodeActionFormattingRequest: false, + automaticallyAddUsings: false, cancellationToken: cancellationToken).ConfigureAwait(false); razorChanges = UnwrapCSharpSnippets(razorChanges); @@ -206,34 +233,27 @@ public bool TryGetOnTypeFormattingTriggerKind(RazorCodeDocument codeDocument, in } private async Task> ApplyFormattedChangesAsync( - DocumentContext documentContext, + IDocumentSnapshot documentSnapshot, + RazorCodeDocument codeDocument, ImmutableArray generatedDocumentChanges, RazorFormattingOptions options, int hostDocumentIndex, char triggerCharacter, ImmutableArray formattingPasses, bool collapseChanges, - bool isCodeActionFormattingRequest, + bool automaticallyAddUsings, CancellationToken cancellationToken) { // If we only received a single edit, let's always return a single edit back. // Otherwise, merge only if explicitly asked. collapseChanges |= generatedDocumentChanges.Length == 1; - var documentSnapshot = documentContext.Snapshot; - - // Code actions were computed on the regular document, which with FUSE could be a runtime document. We have to make - // sure for code actions specifically we are formatting that same document, or TextChange spans may not line up - var codeDocument = isCodeActionFormattingRequest - ? await documentSnapshot.GetGeneratedOutputAsync(cancellationToken).ConfigureAwait(false) - : await _codeDocumentProvider.GetCodeDocumentAsync(documentSnapshot, cancellationToken).ConfigureAwait(false); - var context = FormattingContext.CreateForOnTypeFormatting( documentSnapshot, codeDocument, options, _codeDocumentProvider, - automaticallyAddUsings: isCodeActionFormattingRequest, + automaticallyAddUsings: automaticallyAddUsings, hostDocumentIndex, triggerCharacter); var result = generatedDocumentChanges; From 03a0e2285e7127b67d593c6e0d94fae4cf2c0884 Mon Sep 17 00:00:00 2001 From: David Wengier Date: Wed, 30 Oct 2024 14:28:38 +1100 Subject: [PATCH 3/5] Fix guard clause, and failure result This was returning the passed in changes, which were for a C# document, when the formatting engine should be producing changes for the Razor document, so that was wrong. Additionally, the guard itself was wrong --- .../Formatting/Passes/CSharpOnTypeFormattingPass.cs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/Formatting/Passes/CSharpOnTypeFormattingPass.cs b/src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/Formatting/Passes/CSharpOnTypeFormattingPass.cs index 58ba8c123b9..bcb24af4b20 100644 --- a/src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/Formatting/Passes/CSharpOnTypeFormattingPass.cs +++ b/src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/Formatting/Passes/CSharpOnTypeFormattingPass.cs @@ -45,7 +45,7 @@ protected async override Task> ExecuteCoreAsync(Forma if (!DocumentMappingService.TryMapToGeneratedDocumentPosition(codeDocument.GetCSharpDocument(), context.HostDocumentIndex, out _, out var projectedIndex)) { _logger.LogWarning($"Failed to map to projected position for document {context.OriginalSnapshot.FilePath}."); - return changes; + return []; } // Ask C# for formatting changes. @@ -64,7 +64,7 @@ protected async override Task> ExecuteCoreAsync(Forma if (formattingChanges.IsEmpty) { _logger.LogInformation($"Received no results."); - return changes; + return []; } changes = formattingChanges; @@ -80,10 +80,10 @@ protected async override Task> ExecuteCoreAsync(Forma var startPos = edit.Span.Start; var endPos = edit.Span.End; var count = csharpText.Length; - if (startPos >= count || endPos >= count) + if (startPos > count || endPos > count) { _logger.LogWarning($"Got a bad edit that couldn't be applied. Edit is {startPos}-{endPos} but there are only {count} characters in C#."); - return changes; + return []; } } From 9f6058542806376436aff7a95de502e171a89d06 Mon Sep 17 00:00:00 2001 From: David Wengier Date: Wed, 30 Oct 2024 16:24:26 +1100 Subject: [PATCH 4/5] Remove IFormattingCodeDocumentProvider Since this service was written, IDocumentSnapshot has been updated such that now, all implementations just passed true for a parameter that is part of the existing API --- .../LspFormattingCodeDocumentProvider.cs | 19 ----------------- .../InlineCompletionEndPoint.cs | 7 +------ .../RazorLanguageServer.cs | 2 -- .../Formatting/FormattingContext.cs | 19 ++++------------- .../IFormattingCodeDocumentProvider.cs | 14 ------------- .../Formatting/RazorFormattingService.cs | 13 ++---------- .../RemoteFormattingCodeDocumentProvider.cs | 21 ------------------- .../RemoteRazorFormattingService.cs | 4 ++-- .../FormattingContentValidationPassTest.cs | 3 +-- .../FormattingDiagnosticValidationPassTest.cs | 3 +-- .../TestRazorFormattingService.cs | 3 +-- 11 files changed, 12 insertions(+), 96 deletions(-) delete mode 100644 src/Razor/src/Microsoft.AspNetCore.Razor.LanguageServer/Formatting/LspFormattingCodeDocumentProvider.cs delete mode 100644 src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/Formatting/IFormattingCodeDocumentProvider.cs delete mode 100644 src/Razor/src/Microsoft.CodeAnalysis.Remote.Razor/Formatting/RemoteFormattingCodeDocumentProvider.cs diff --git a/src/Razor/src/Microsoft.AspNetCore.Razor.LanguageServer/Formatting/LspFormattingCodeDocumentProvider.cs b/src/Razor/src/Microsoft.AspNetCore.Razor.LanguageServer/Formatting/LspFormattingCodeDocumentProvider.cs deleted file mode 100644 index e04d70fff32..00000000000 --- a/src/Razor/src/Microsoft.AspNetCore.Razor.LanguageServer/Formatting/LspFormattingCodeDocumentProvider.cs +++ /dev/null @@ -1,19 +0,0 @@ -// Copyright (c) .NET Foundation. All rights reserved. -// Licensed under the MIT license. See License.txt in the project root for license information. - -using System.Threading; -using System.Threading.Tasks; -using Microsoft.AspNetCore.Razor.Language; -using Microsoft.CodeAnalysis.Razor.Formatting; -using Microsoft.CodeAnalysis.Razor.ProjectSystem; - -namespace Microsoft.AspNetCore.Razor.LanguageServer.Formatting; - -internal sealed class LspFormattingCodeDocumentProvider : IFormattingCodeDocumentProvider -{ - public ValueTask GetCodeDocumentAsync(IDocumentSnapshot snapshot, CancellationToken cancellationToken) - { - // Formatting always uses design time - return snapshot.GetGeneratedOutputAsync(forceDesignTimeGeneratedOutput: true, cancellationToken); - } -} diff --git a/src/Razor/src/Microsoft.AspNetCore.Razor.LanguageServer/InlineCompletion/InlineCompletionEndPoint.cs b/src/Razor/src/Microsoft.AspNetCore.Razor.LanguageServer/InlineCompletion/InlineCompletionEndPoint.cs index 03ff37b121a..c9abfb35c97 100644 --- a/src/Razor/src/Microsoft.AspNetCore.Razor.LanguageServer/InlineCompletion/InlineCompletionEndPoint.cs +++ b/src/Razor/src/Microsoft.AspNetCore.Razor.LanguageServer/InlineCompletion/InlineCompletionEndPoint.cs @@ -1,7 +1,6 @@ // 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.Diagnostics.CodeAnalysis; @@ -18,7 +17,6 @@ using Microsoft.CodeAnalysis.Razor.Logging; using Microsoft.CodeAnalysis.Razor.Protocol; using Microsoft.CodeAnalysis.Razor.Protocol.Completion; -using Microsoft.CodeAnalysis.Razor.Workspaces; using Microsoft.CodeAnalysis.Text; using Microsoft.VisualStudio.LanguageServer.Protocol; @@ -28,7 +26,6 @@ namespace Microsoft.AspNetCore.Razor.LanguageServer.InlineCompletion; internal sealed class InlineCompletionEndpoint( IDocumentMappingService documentMappingService, IClientConnection clientConnection, - IFormattingCodeDocumentProvider formattingCodeDocumentProvider, RazorLSPOptionsMonitor optionsMonitor, ILoggerFactory loggerFactory) : IRazorRequestHandler, ICapabilitiesProvider @@ -40,7 +37,6 @@ internal sealed class InlineCompletionEndpoint( private readonly IDocumentMappingService _documentMappingService = documentMappingService; private readonly IClientConnection _clientConnection = clientConnection; - private readonly IFormattingCodeDocumentProvider _formattingCodeDocumentProvider = formattingCodeDocumentProvider; private readonly RazorLSPOptionsMonitor _optionsMonitor = optionsMonitor; private readonly ILogger _logger = loggerFactory.GetOrCreateLogger(); @@ -125,8 +121,7 @@ public TextDocumentIdentifier GetTextDocumentIdentifier(VSInternalInlineCompleti var formattingContext = FormattingContext.Create( documentContext.Snapshot, codeDocument, - options, - _formattingCodeDocumentProvider); + options); if (!TryGetSnippetWithAdjustedIndentation(formattingContext, item.Text, hostDocumentIndex, out var newSnippetText)) { continue; diff --git a/src/Razor/src/Microsoft.AspNetCore.Razor.LanguageServer/RazorLanguageServer.cs b/src/Razor/src/Microsoft.AspNetCore.Razor.LanguageServer/RazorLanguageServer.cs index 780d31c5e56..a646391abc0 100644 --- a/src/Razor/src/Microsoft.AspNetCore.Razor.LanguageServer/RazorLanguageServer.cs +++ b/src/Razor/src/Microsoft.AspNetCore.Razor.LanguageServer/RazorLanguageServer.cs @@ -119,8 +119,6 @@ protected override ILspServices ConstructLspServices() // Add the logger as a service in case anything in CLaSP pulls it out to do logging services.AddSingleton(Logger); - services.AddSingleton(); - var featureOptions = _featureOptions ?? new DefaultLanguageServerFeatureOptions(); services.AddSingleton(featureOptions); diff --git a/src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/Formatting/FormattingContext.cs b/src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/Formatting/FormattingContext.cs index 52bb5579988..1366be999f9 100644 --- a/src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/Formatting/FormattingContext.cs +++ b/src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/Formatting/FormattingContext.cs @@ -20,13 +20,10 @@ namespace Microsoft.CodeAnalysis.Razor.Formatting; internal sealed class FormattingContext { - private readonly IFormattingCodeDocumentProvider _codeDocumentProvider; - private IReadOnlyList? _formattingSpans; private IReadOnlyDictionary? _indentations; private FormattingContext( - IFormattingCodeDocumentProvider codeDocumentProvider, IDocumentSnapshot originalSnapshot, RazorCodeDocument codeDocument, RazorFormattingOptions options, @@ -34,7 +31,6 @@ private FormattingContext( int hostDocumentIndex, char triggerCharacter) { - _codeDocumentProvider = codeDocumentProvider; OriginalSnapshot = originalSnapshot; CodeDocument = codeDocument; Options = options; @@ -229,14 +225,12 @@ public async Task WithTextAsync(SourceText changedText, Cance { var changedSnapshot = OriginalSnapshot.WithText(changedText); - var codeDocument = await _codeDocumentProvider - .GetCodeDocumentAsync(changedSnapshot, cancellationToken) - .ConfigureAwait(false); + // Formatting always uses design time document + var codeDocument = await changedSnapshot.GetGeneratedOutputAsync(forceDesignTimeGeneratedOutput: true, cancellationToken).ConfigureAwait(false); DEBUG_ValidateComponents(CodeDocument, codeDocument); var newContext = new FormattingContext( - _codeDocumentProvider, OriginalSnapshot, codeDocument, Options, @@ -269,13 +263,11 @@ public static FormattingContext CreateForOnTypeFormatting( IDocumentSnapshot originalSnapshot, RazorCodeDocument codeDocument, RazorFormattingOptions options, - IFormattingCodeDocumentProvider codeDocumentProvider, bool automaticallyAddUsings, int hostDocumentIndex, char triggerCharacter) { return new FormattingContext( - codeDocumentProvider, originalSnapshot, codeDocument, options, @@ -287,17 +279,14 @@ public static FormattingContext CreateForOnTypeFormatting( public static FormattingContext Create( IDocumentSnapshot originalSnapshot, RazorCodeDocument codeDocument, - RazorFormattingOptions options, - IFormattingCodeDocumentProvider codeDocumentProvider) + RazorFormattingOptions options) { return new FormattingContext( - codeDocumentProvider, originalSnapshot, codeDocument, options, automaticallyAddUsings: false, hostDocumentIndex: 0, - triggerCharacter: '\0' - ); + triggerCharacter: '\0'); } } diff --git a/src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/Formatting/IFormattingCodeDocumentProvider.cs b/src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/Formatting/IFormattingCodeDocumentProvider.cs deleted file mode 100644 index 34ceec8cb23..00000000000 --- a/src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/Formatting/IFormattingCodeDocumentProvider.cs +++ /dev/null @@ -1,14 +0,0 @@ -// Copyright (c) .NET Foundation. All rights reserved. -// Licensed under the MIT license. See License.txt in the project root for license information. - -using System.Threading; -using System.Threading.Tasks; -using Microsoft.AspNetCore.Razor.Language; -using Microsoft.CodeAnalysis.Razor.ProjectSystem; - -namespace Microsoft.CodeAnalysis.Razor.Formatting; - -internal interface IFormattingCodeDocumentProvider -{ - ValueTask GetCodeDocumentAsync(IDocumentSnapshot snapshot, CancellationToken cancellationToken); -} 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 eb03574c68e..585299916ce 100644 --- a/src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/Formatting/RazorFormattingService.cs +++ b/src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/Formatting/RazorFormattingService.cs @@ -29,21 +29,16 @@ internal class RazorFormattingService : IRazorFormattingService private static readonly FrozenSet s_csharpTriggerCharacterSet = FrozenSet.ToFrozenSet(["}", ";"], StringComparer.Ordinal); private static readonly FrozenSet s_htmlTriggerCharacterSet = FrozenSet.ToFrozenSet(["\n", "{", "}", ";"], StringComparer.Ordinal); - private readonly IFormattingCodeDocumentProvider _codeDocumentProvider; - private readonly ImmutableArray _documentFormattingPasses; private readonly ImmutableArray _validationPasses; private readonly CSharpOnTypeFormattingPass _csharpOnTypeFormattingPass; private readonly HtmlOnTypeFormattingPass _htmlOnTypeFormattingPass; public RazorFormattingService( - IFormattingCodeDocumentProvider codeDocumentProvider, IDocumentMappingService documentMappingService, IHostServicesProvider hostServicesProvider, ILoggerFactory loggerFactory) { - _codeDocumentProvider = codeDocumentProvider; - _htmlOnTypeFormattingPass = new HtmlOnTypeFormattingPass(loggerFactory); _csharpOnTypeFormattingPass = new CSharpOnTypeFormattingPass(documentMappingService, hostServicesProvider, loggerFactory); _validationPasses = @@ -67,9 +62,7 @@ public async Task> GetDocumentFormattingChangesAsync( RazorFormattingOptions options, CancellationToken cancellationToken) { - var codeDocument = await _codeDocumentProvider - .GetCodeDocumentAsync(documentContext.Snapshot, cancellationToken) - .ConfigureAwait(false); + var codeDocument = await documentContext.Snapshot.GetGeneratedOutputAsync(forceDesignTimeGeneratedOutput: true, cancellationToken).ConfigureAwait(false); // Range formatting happens on every paste, and if there are Razor diagnostics in the file // that can make some very bad results. eg, given: @@ -100,8 +93,7 @@ public async Task> GetDocumentFormattingChangesAsync( var context = FormattingContext.Create( documentSnapshot, codeDocument, - options, - _codeDocumentProvider); + options); var originalText = context.SourceText; var result = htmlChanges; @@ -252,7 +244,6 @@ private async Task> ApplyFormattedChangesAsync( documentSnapshot, codeDocument, options, - _codeDocumentProvider, automaticallyAddUsings: automaticallyAddUsings, hostDocumentIndex, triggerCharacter); diff --git a/src/Razor/src/Microsoft.CodeAnalysis.Remote.Razor/Formatting/RemoteFormattingCodeDocumentProvider.cs b/src/Razor/src/Microsoft.CodeAnalysis.Remote.Razor/Formatting/RemoteFormattingCodeDocumentProvider.cs deleted file mode 100644 index 914e05b6139..00000000000 --- a/src/Razor/src/Microsoft.CodeAnalysis.Remote.Razor/Formatting/RemoteFormattingCodeDocumentProvider.cs +++ /dev/null @@ -1,21 +0,0 @@ -// Copyright (c) .NET Foundation. All rights reserved. -// Licensed under the MIT license. See License.txt in the project root for license information. - -using System.Composition; -using System.Threading; -using System.Threading.Tasks; -using Microsoft.AspNetCore.Razor.Language; -using Microsoft.CodeAnalysis.Razor.Formatting; -using Microsoft.CodeAnalysis.Razor.ProjectSystem; - -namespace Microsoft.CodeAnalysis.Remote.Razor.Formatting; - -[Export(typeof(IFormattingCodeDocumentProvider)), Shared] -internal sealed class RemoteFormattingCodeDocumentProvider : IFormattingCodeDocumentProvider -{ - public ValueTask GetCodeDocumentAsync(IDocumentSnapshot snapshot, CancellationToken cancellationToken) - { - // Formatting always uses design time - return snapshot.GetGeneratedOutputAsync(forceDesignTimeGeneratedOutput: true, cancellationToken); - } -} 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 7de42441ecc..25793db1569 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 sealed class RemoteRazorFormattingService(IFormattingCodeDocumentProvider codeDocumentProvider, IDocumentMappingService documentMappingService, IHostServicesProvider hostServicesProvider, ILoggerFactory loggerFactory) - : RazorFormattingService(codeDocumentProvider, documentMappingService, hostServicesProvider, loggerFactory) +internal sealed class RemoteRazorFormattingService(IDocumentMappingService documentMappingService, IHostServicesProvider hostServicesProvider, ILoggerFactory loggerFactory) + : RazorFormattingService(documentMappingService, hostServicesProvider, loggerFactory) { } 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 ed626bc07bd..ee58d38dcd0 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 @@ -88,8 +88,7 @@ private static FormattingContext CreateFormattingContext(TestCode input, int tab var context = FormattingContext.Create( documentSnapshot, codeDocument, - options, - new LspFormattingCodeDocumentProvider()); + options); return context; } 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 e7a66fad443..736d19ac580 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 @@ -86,8 +86,7 @@ private static FormattingContext CreateFormattingContext(TestCode input, int tab var context = FormattingContext.Create( documentSnapshot, codeDocument, - options, - new LspFormattingCodeDocumentProvider()); + options); return context; } 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 217024e2dca..65b33247361 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 @@ -38,9 +38,8 @@ public static async Task CreateWithFullSupportAsync( await optionsMonitor.UpdateAsync(CancellationToken.None); } - var formattingCodeDocumentProvider = new LspFormattingCodeDocumentProvider(); var hostServicesProvider = new DefaultHostServicesProvider(); - return new RazorFormattingService(formattingCodeDocumentProvider, mappingService, hostServicesProvider, loggerFactory); + return new RazorFormattingService(mappingService, hostServicesProvider, loggerFactory); } } From 5838cd55c96cbcb88c14c34f7f3bdbfca363cb29 Mon Sep 17 00:00:00 2001 From: David Wengier Date: Wed, 30 Oct 2024 17:17:57 +1100 Subject: [PATCH 5/5] More FUSE test coverage --- .../CohostDocumentFormattingEndpointTest.cs | 12 ++++++--- .../Cohost/CohostOnAutoInsertEndpointTest.cs | 25 ++++++++++++------- .../CohostOnTypeFormattingEndpointTest.cs | 20 +++++++++------ 3 files changed, 37 insertions(+), 20 deletions(-) diff --git a/src/Razor/test/Microsoft.VisualStudio.LanguageServices.Razor.Test/Cohost/CohostDocumentFormattingEndpointTest.cs b/src/Razor/test/Microsoft.VisualStudio.LanguageServices.Razor.Test/Cohost/CohostDocumentFormattingEndpointTest.cs index 7370b711fc1..a1bfb08d661 100644 --- a/src/Razor/test/Microsoft.VisualStudio.LanguageServices.Razor.Test/Cohost/CohostDocumentFormattingEndpointTest.cs +++ b/src/Razor/test/Microsoft.VisualStudio.LanguageServices.Razor.Test/Cohost/CohostDocumentFormattingEndpointTest.cs @@ -26,8 +26,9 @@ public class CohostDocumentFormattingEndpointTest(HtmlFormattingFixture htmlForm // and provide regression prevention. The tests here are not exhaustive, but they validate the the cohost endpoints // call into the formatting engine at least, and handles C#, Html and Razor formatting changes correctly. - [Fact] - public Task Formatting() + [Theory] + [CombinatorialData] + public Task Formatting(bool fuse) => VerifyDocumentFormattingAsync( input: """ @preservewhitespace true @@ -104,10 +105,13 @@ private void M(string thisIsMyString) } } - """); + """, + fuse: fuse); - private async Task VerifyDocumentFormattingAsync(string input, string expected) + private async Task VerifyDocumentFormattingAsync(string input, string expected, bool fuse) { + UpdateClientInitializationOptions(opt => opt with { ForceRuntimeCodeGeneration = fuse }); + var document = await CreateProjectAndRazorDocumentAsync(input); var inputText = await document.GetTextAsync(DisposalToken); diff --git a/src/Razor/test/Microsoft.VisualStudio.LanguageServices.Razor.Test/Cohost/CohostOnAutoInsertEndpointTest.cs b/src/Razor/test/Microsoft.VisualStudio.LanguageServices.Razor.Test/Cohost/CohostOnAutoInsertEndpointTest.cs index 3919a1298df..587a3aaa6f5 100644 --- a/src/Razor/test/Microsoft.VisualStudio.LanguageServices.Razor.Test/Cohost/CohostOnAutoInsertEndpointTest.cs +++ b/src/Razor/test/Microsoft.VisualStudio.LanguageServices.Razor.Test/Cohost/CohostOnAutoInsertEndpointTest.cs @@ -10,6 +10,7 @@ using Microsoft.CodeAnalysis.Text; using Microsoft.VisualStudio.LanguageServer.Protocol; using Microsoft.VisualStudio.LanguageServices.Razor.LanguageClient.Cohost; +using Microsoft.VisualStudio.LicenseManagement.Interop; using Microsoft.VisualStudio.Razor.Settings; using Roslyn.Test.Utilities; using Xunit; @@ -84,8 +85,9 @@ The end. delegatedResponseText: "\"$0\""); } - [Fact] - public async Task CSharp_OnForwardSlash() + [Theory] + [CombinatorialData] + public async Task CSharp_OnForwardSlash(bool fuse) { await VerifyOnAutoInsertAsync( input: """ @@ -102,7 +104,8 @@ void TestMethod() {} void TestMethod() {} } """, - triggerCharacter: "/"); + triggerCharacter: "/", + fuse: fuse); } [Fact] @@ -161,8 +164,9 @@ void TestMethod() fuse: fuse); } - [Fact] - public async Task CSharp_OnEnter_TwoSpaceIndent() + [Theory] + [CombinatorialData] + public async Task CSharp_OnEnter_TwoSpaceIndent(bool fuse) { await VerifyOnAutoInsertAsync( input: """ @@ -180,11 +184,13 @@ void TestMethod() } """, triggerCharacter: "\n", - tabSize: 2); + tabSize: 2, + fuse: fuse); } - [Fact] - public async Task CSharp_OnEnter_UseTabs() + [Theory] + [CombinatorialData] + public async Task CSharp_OnEnter_UseTabs(bool fuse) { const char tab = '\t'; await VerifyOnAutoInsertAsync( @@ -203,7 +209,8 @@ void TestMethod() { } """, triggerCharacter: "\n", - insertSpaces: false); + insertSpaces: false, + fuse: fuse); } private async Task VerifyOnAutoInsertAsync( diff --git a/src/Razor/test/Microsoft.VisualStudio.LanguageServices.Razor.Test/Cohost/CohostOnTypeFormattingEndpointTest.cs b/src/Razor/test/Microsoft.VisualStudio.LanguageServices.Razor.Test/Cohost/CohostOnTypeFormattingEndpointTest.cs index 1919956dad4..305704c6412 100644 --- a/src/Razor/test/Microsoft.VisualStudio.LanguageServices.Razor.Test/Cohost/CohostOnTypeFormattingEndpointTest.cs +++ b/src/Razor/test/Microsoft.VisualStudio.LanguageServices.Razor.Test/Cohost/CohostOnTypeFormattingEndpointTest.cs @@ -56,8 +56,9 @@ await VerifyOnTypeFormattingAsync( triggerCharacter: '\n'); } - [Fact] - public async Task CSharp() + [Theory] + [CombinatorialData] + public async Task CSharp(bool fuse) { await VerifyOnTypeFormattingAsync( input: """ @@ -70,11 +71,13 @@ await VerifyOnTypeFormattingAsync( if (true) { } } """, - triggerCharacter: '}'); + triggerCharacter: '}', + fuse: fuse); } - [Fact] - public async Task FormatsSimpleHtmlTag_OnType() + [Theory] + [CombinatorialData] + public async Task FormatsSimpleHtmlTag_OnType(bool fuse) { await VerifyOnTypeFormattingAsync( input: """ @@ -98,11 +101,14 @@ await VerifyOnTypeFormattingAsync( """, triggerCharacter: ';', - html: true); + html: true, + fuse: fuse); } - private async Task VerifyOnTypeFormattingAsync(TestCode input, string expected, char triggerCharacter, bool html = false) + private async Task VerifyOnTypeFormattingAsync(TestCode input, string expected, char triggerCharacter, bool html = false, bool fuse = false) { + UpdateClientInitializationOptions(opt => opt with { ForceRuntimeCodeGeneration = fuse }); + var document = await CreateProjectAndRazorDocumentAsync(input.Text); var inputText = await document.GetTextAsync(DisposalToken); var position = inputText.GetPosition(input.Position);