Fix FormattingContext disposal (#10887)

From a conversation on Teams. `FormattingContext` is disposable because
it owns a workspace, but it also gets non-destructively mutated so it's
unclear as to whether the disposal was working as intended, and unclear
to consumers what needed to happen. Upon review of the code, this PR:

* Moves workspace ownership to the caller of the formatting code, so its
disposal is clear
* Makes `AdhocWorkspaceFactory` shared code, because it did the same
thing in OOP and LSP server
* Adds `IHostServicesProvider` to OOP, because thats the thing that was
actually different between OOP and LSP server
* Random cleanup of some related things

Author: davidwengier

src/Razor/src/Microsoft.AspNetCore.Razor.LanguageServer/AdhocWorkspaceFactory.cs

@@ -5,6 +5,7 @@
 using Microsoft.AspNetCore.Razor.LanguageServer.Hosting;
 using Microsoft.CodeAnalysis;
 using Microsoft.CodeAnalysis.Host;
+using Microsoft.CodeAnalysis.Razor.Workspaces;
 
 namespace Microsoft.AspNetCore.Razor.LanguageServer;
 

src/Razor/src/Microsoft.AspNetCore.Razor.LanguageServer/DefaultHostServicesProvider.cs

@@ -29,7 +29,6 @@ internal sealed class InlineCompletionEndpoint(
     IDocumentMappingService documentMappingService,
     IClientConnection clientConnection,
     IFormattingCodeDocumentProvider formattingCodeDocumentProvider,
-    IAdhocWorkspaceFactory adhocWorkspaceFactory,
     RazorLSPOptionsMonitor optionsMonitor,
     ILoggerFactory loggerFactory)
     : IRazorRequestHandler<VSInternalInlineCompletionRequest, VSInternalInlineCompletionList?>, ICapabilitiesProvider
@@ -39,10 +38,9 @@ internal sealed class InlineCompletionEndpoint(
         "if", "indexer", "interface", "invoke", "iterator", "iterindex", "lock", "mbox", "namespace", "#if", "#region", "prop",
         "propfull", "propg", "sim", "struct", "svm", "switch", "try", "tryf", "unchecked", "unsafe", "using", "while");
 
-    private readonly IDocumentMappingService _documentMappingService = documentMappingService ?? throw new ArgumentNullException(nameof(documentMappingService));
-    private readonly IClientConnection _clientConnection = clientConnection ?? throw new ArgumentNullException(nameof(clientConnection));
+    private readonly IDocumentMappingService _documentMappingService = documentMappingService;
+    private readonly IClientConnection _clientConnection = clientConnection;
     private readonly IFormattingCodeDocumentProvider _formattingCodeDocumentProvider = formattingCodeDocumentProvider;
-    private readonly IAdhocWorkspaceFactory _adhocWorkspaceFactory = adhocWorkspaceFactory ?? throw new ArgumentNullException(nameof(adhocWorkspaceFactory));
     private readonly RazorLSPOptionsMonitor _optionsMonitor = optionsMonitor;
     private readonly ILogger _logger = loggerFactory.GetOrCreateLogger<InlineCompletionEndpoint>();
 
@@ -63,10 +61,7 @@ public TextDocumentIdentifier GetTextDocumentIdentifier(VSInternalInlineCompleti
 
     public async Task<VSInternalInlineCompletionList?> HandleRequestAsync(VSInternalInlineCompletionRequest request, RazorRequestContext requestContext, CancellationToken cancellationToken)
     {
-        if (request is null)
-        {
-            throw new ArgumentNullException(nameof(request));
-        }
+        ArgHelper.ThrowIfNull(request);
 
         _logger.LogInformation($"Starting request for {request.TextDocument.Uri} at {request.Position}.");
 
@@ -118,7 +113,6 @@ public TextDocumentIdentifier GetTextDocumentIdentifier(VSInternalInlineCompleti
         using var items = new PooledArrayBuilder<VSInternalInlineCompletionItem>(list.Items.Length);
         foreach (var item in list.Items)
         {
-            var containsSnippet = item.TextFormat == InsertTextFormat.Snippet;
             var range = item.Range ?? projectedPosition.ToZeroWidthRange();
 
             if (!_documentMappingService.TryMapToHostDocumentRange(codeDocument.GetCSharpDocument(), range, out var rangeInRazorDoc))
@@ -128,13 +122,11 @@ public TextDocumentIdentifier GetTextDocumentIdentifier(VSInternalInlineCompleti
             }
 
             var options = RazorFormattingOptions.From(request.Options, _optionsMonitor.CurrentValue.CodeBlockBraceOnNextLine);
-            using var formattingContext = FormattingContext.Create(
-                request.TextDocument.Uri,
+            var formattingContext = FormattingContext.Create(
                 documentContext.Snapshot,
                 codeDocument,
                 options,
-                _formattingCodeDocumentProvider,
-                _adhocWorkspaceFactory);
+                _formattingCodeDocumentProvider);
             if (!TryGetSnippetWithAdjustedIndentation(formattingContext, item.Text, hostDocumentIndex, out var newSnippetText))
             {
                 continue;

src/Razor/src/Microsoft.AspNetCore.Razor.LanguageServer/InlineCompletion/InlineCompletionEndPoint.cs

@@ -119,9 +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<ILspLogger>(Logger);
 
-        services.AddSingleton<IAdhocWorkspaceFactory, AdhocWorkspaceFactory>();
-        services.AddSingleton<IWorkspaceProvider, LspWorkspaceProvider>();
-
         services.AddSingleton<IFormattingCodeDocumentProvider, LspFormattingCodeDocumentProvider>();
 
         var featureOptions = _featureOptions ?? new DefaultLanguageServerFeatureOptions();

src/Razor/src/Microsoft.AspNetCore.Razor.LanguageServer/LspWorkspaceProvider.cs

@@ -12,6 +12,7 @@
 using Microsoft.CodeAnalysis.CSharp;
 using Microsoft.CodeAnalysis.CSharp.Syntax;
 using Microsoft.CodeAnalysis.ExternalAccess.Razor;
+using Microsoft.CodeAnalysis.Host;
 using Microsoft.CodeAnalysis.Razor.DocumentMapping;
 using Microsoft.CodeAnalysis.Text;
 using Microsoft.VisualStudio.LanguageServer.Protocol;
@@ -24,28 +25,29 @@ internal sealed class CSharpFormatter(IDocumentMappingService documentMappingSer
 
     private readonly IDocumentMappingService _documentMappingService = documentMappingService;
 
-    public async Task<ImmutableArray<TextChange>> FormatAsync(FormattingContext context, LinePositionSpan spanToFormat, CancellationToken cancellationToken)
+    public async Task<ImmutableArray<TextChange>> FormatAsync(HostWorkspaceServices hostWorkspaceServices, Document csharpDocument, FormattingContext context, LinePositionSpan spanToFormat, CancellationToken cancellationToken)
     {
         if (!_documentMappingService.TryMapToGeneratedDocumentRange(context.CodeDocument.GetCSharpDocument(), spanToFormat, out var projectedSpan))
         {
             return [];
         }
 
-        var edits = await GetFormattingEditsAsync(context, projectedSpan, cancellationToken).ConfigureAwait(false);
+        var edits = await GetFormattingEditsAsync(hostWorkspaceServices, csharpDocument, projectedSpan, context.Options.ToIndentationOptions(), cancellationToken).ConfigureAwait(false);
         var mappedEdits = MapEditsToHostDocument(context.CodeDocument, edits);
         return mappedEdits;
     }
 
     public static async Task<IReadOnlyDictionary<int, int>> GetCSharpIndentationAsync(
         FormattingContext context,
         HashSet<int> projectedDocumentLocations,
+        HostWorkspaceServices hostWorkspaceServices,
         CancellationToken cancellationToken)
     {
         // Sorting ensures we count the marker offsets correctly.
         // We also want to ensure there are no duplicates to avoid duplicate markers.
         var filteredLocations = projectedDocumentLocations.OrderAsArray();
 
-        var indentations = await GetCSharpIndentationCoreAsync(context, filteredLocations, cancellationToken).ConfigureAwait(false);
+        var indentations = await GetCSharpIndentationCoreAsync(context, filteredLocations, hostWorkspaceServices, cancellationToken).ConfigureAwait(false);
         return indentations;
     }
 
@@ -56,18 +58,18 @@ private ImmutableArray<TextChange> MapEditsToHostDocument(RazorCodeDocument code
         return actualEdits.ToImmutableArray();
     }
 
-    private static async Task<ImmutableArray<TextChange>> GetFormattingEditsAsync(FormattingContext context, LinePositionSpan projectedSpan, CancellationToken cancellationToken)
+    private static async Task<ImmutableArray<TextChange>> GetFormattingEditsAsync(HostWorkspaceServices hostWorkspaceServices, Document csharpDocument, LinePositionSpan projectedSpan, RazorIndentationOptions indentationOptions, CancellationToken cancellationToken)
     {
-        var csharpSourceText = context.CodeDocument.GetCSharpSourceText();
+        var root = await csharpDocument.GetSyntaxRootAsync(cancellationToken).ConfigureAwait(false);
+        var csharpSourceText = await csharpDocument.GetTextAsync(cancellationToken).ConfigureAwait(false);
         var spanToFormat = csharpSourceText.GetTextSpan(projectedSpan);
-        var root = await context.CSharpWorkspaceDocument.GetSyntaxRootAsync(cancellationToken).ConfigureAwait(false);
         Assumes.NotNull(root);
 
-        var changes = RazorCSharpFormattingInteractionService.GetFormattedTextChanges(context.CSharpWorkspace.Services, root, spanToFormat, context.Options.ToIndentationOptions(), cancellationToken);
+        var changes = RazorCSharpFormattingInteractionService.GetFormattedTextChanges(hostWorkspaceServices, root, spanToFormat, indentationOptions, cancellationToken);
         return changes.ToImmutableArray();
     }
 
-    private static async Task<Dictionary<int, int>> GetCSharpIndentationCoreAsync(FormattingContext context, ImmutableArray<int> projectedDocumentLocations, CancellationToken cancellationToken)
+    private static async Task<Dictionary<int, int>> GetCSharpIndentationCoreAsync(FormattingContext context, ImmutableArray<int> projectedDocumentLocations, HostWorkspaceServices hostWorkspaceServices, CancellationToken cancellationToken)
     {
         // No point calling the C# formatting if we won't be interested in any of its work anyway
         if (projectedDocumentLocations.Length == 0)
@@ -83,7 +85,7 @@ private static async Task<Dictionary<int, int>> GetCSharpIndentationCoreAsync(Fo
 
         // At this point, we have added all the necessary markers and attached annotations.
         // Let's invoke the C# formatter and hope for the best.
-        var formattedRoot = RazorCSharpFormattingInteractionService.Format(context.CSharpWorkspace.Services, root, context.Options.ToIndentationOptions(), cancellationToken);
+        var formattedRoot = RazorCSharpFormattingInteractionService.Format(hostWorkspaceServices, root, context.Options.ToIndentationOptions(), cancellationToken);
         var formattedText = formattedRoot.GetText();
 
         var desiredIndentationMap = new Dictionary<int, int>();

src/Razor/src/Microsoft.AspNetCore.Razor.LanguageServer/RazorLanguageServer.cs

@@ -12,27 +12,20 @@
 using Microsoft.AspNetCore.Razor.Language.Syntax;
 using Microsoft.CodeAnalysis;
 using Microsoft.CodeAnalysis.Razor.ProjectSystem;
-using Microsoft.CodeAnalysis.Razor.Workspaces;
 using Microsoft.CodeAnalysis.Text;
 using Microsoft.VisualStudio.LanguageServer.Protocol;
 
 namespace Microsoft.CodeAnalysis.Razor.Formatting;
 
-internal sealed class FormattingContext : IDisposable
+internal sealed class FormattingContext
 {
-    private readonly IAdhocWorkspaceFactory _workspaceFactory;
     private readonly IFormattingCodeDocumentProvider _codeDocumentProvider;
 
-    private Document? _csharpWorkspaceDocument;
-    private AdhocWorkspace? _csharpWorkspace;
-
     private IReadOnlyList<FormattingSpan>? _formattingSpans;
     private IReadOnlyDictionary<int, IndentationContext>? _indentations;
 
     private FormattingContext(
         IFormattingCodeDocumentProvider codeDocumentProvider,
-        IAdhocWorkspaceFactory workspaceFactory,
-        Uri uri,
         IDocumentSnapshot originalSnapshot,
         RazorCodeDocument codeDocument,
         RazorFormattingOptions options,
@@ -41,8 +34,6 @@ private FormattingContext(
         char triggerCharacter)
     {
         _codeDocumentProvider = codeDocumentProvider;
-        _workspaceFactory = workspaceFactory;
-        Uri = uri;
         OriginalSnapshot = originalSnapshot;
         CodeDocument = codeDocument;
         Options = options;
@@ -53,7 +44,6 @@ private FormattingContext(
 
     public static bool SkipValidateComponents { get; set; }
 
-    public Uri Uri { get; }
     public IDocumentSnapshot OriginalSnapshot { get; }
     public RazorCodeDocument CodeDocument { get; }
     public RazorFormattingOptions Options { get; }
@@ -67,24 +57,6 @@ private FormattingContext(
 
     public string NewLineString => Environment.NewLine;
 
-    public Document CSharpWorkspaceDocument
-    {
-        get
-        {
-            if (_csharpWorkspaceDocument is null)
-            {
-                var workspace = CSharpWorkspace;
-                var project = workspace.AddProject("TestProject", LanguageNames.CSharp);
-                var csharpSourceText = CodeDocument.GetCSharpSourceText();
-                _csharpWorkspaceDocument = workspace.AddDocument(project.Id, "TestDocument", csharpSourceText);
-            }
-
-            return _csharpWorkspaceDocument;
-        }
-    }
-
-    public AdhocWorkspace CSharpWorkspace => _csharpWorkspace ??= _workspaceFactory.Create();
-
     /// <summary>A Dictionary of int (line number) to IndentationContext.</summary>
     /// <remarks>
     /// Don't use this to discover the indentation level you should have, use
@@ -252,15 +224,6 @@ public bool TryGetFormattingSpan(int absoluteIndex, [NotNullWhen(true)] out Form
         return false;
     }
 
-    public void Dispose()
-    {
-        _csharpWorkspace?.Dispose();
-        if (_csharpWorkspaceDocument != null)
-        {
-            _csharpWorkspaceDocument = null;
-        }
-    }
-
     public async Task<FormattingContext> WithTextAsync(SourceText changedText)
     {
         var changedSnapshot = OriginalSnapshot.WithText(changedText);
@@ -271,8 +234,6 @@ public async Task<FormattingContext> WithTextAsync(SourceText changedText)
 
         var newContext = new FormattingContext(
             _codeDocumentProvider,
-            _workspaceFactory,
-            Uri,
             OriginalSnapshot,
             codeDocument,
             Options,
@@ -302,20 +263,16 @@ private static void DEBUG_ValidateComponents(RazorCodeDocument oldCodeDocument,
     }
 
     public static FormattingContext CreateForOnTypeFormatting(
-        Uri uri,
         IDocumentSnapshot originalSnapshot,
         RazorCodeDocument codeDocument,
         RazorFormattingOptions options,
         IFormattingCodeDocumentProvider codeDocumentProvider,
-        IAdhocWorkspaceFactory workspaceFactory,
         bool automaticallyAddUsings,
         int hostDocumentIndex,
         char triggerCharacter)
     {
         return new FormattingContext(
             codeDocumentProvider,
-            workspaceFactory,
-            uri,
             originalSnapshot,
             codeDocument,
             options,
@@ -325,17 +282,13 @@ public static FormattingContext CreateForOnTypeFormatting(
     }
 
     public static FormattingContext Create(
-        Uri uri,
         IDocumentSnapshot originalSnapshot,
         RazorCodeDocument codeDocument,
         RazorFormattingOptions options,
-        IFormattingCodeDocumentProvider codeDocumentProvider,
-        IAdhocWorkspaceFactory workspaceFactory)
+        IFormattingCodeDocumentProvider codeDocumentProvider)
     {
         return new FormattingContext(
             codeDocumentProvider,
-            workspaceFactory,
-            uri,
             originalSnapshot,
             codeDocument,
             options,