Add metadata documents to the MAS workspace upfront (#78886)

Precursor to changes to allow LSP to use virtual files for MAS. This
modifies the MAS providers to put the generated document into the
workspace upfront, instead of waiting for someone to attempt to open it.
This does mean that we may keep files in the workspace for longer, but
generally we were keeping the project and file info around anyway in
order to service requests when the document is re-opened. We can remove
those separate stores and just use the workspace as the document holder.

Additionally, this simplifies the current LSP MAS implementation (now
the MAS workspace is just a normal workspace which we pull documents
from), and will simplify the virtual file implementation (virtual files
will be stored as documents in the workspace that LSP can query as it
normally does). Will be working on that in a followup

Resolves #78421

Clean RPS -
https://dev.azure.com/devdiv/DevDiv/_git/VS/pullrequest/644402

Author: dibarbet

src/EditorFeatures/CSharpTest/PdbSourceDocument/AbstractPdbSourceDocumentTests.cs

@@ -156,15 +156,7 @@ protected static async Task GenerateFileAndVerifyAsync(
 
             var pdbService = (PdbSourceDocumentMetadataAsSourceFileProvider)workspace.ExportProvider.GetExportedValues<IMetadataAsSourceFileProvider>().Single(s => s is PdbSourceDocumentMetadataAsSourceFileProvider);
 
-            // Add the document to the workspace.  We provide an empty static source text as the API requires it to open the document.
-            // We're not really trying to verify that the source text the editor hands to us is the right encoding - just that the document we added has the right encoding.
-            var result = pdbService.TryAddDocumentToWorkspace((MetadataAsSourceWorkspace)masWorkspace!, file.FilePath, new StaticSourceTextContainer(SourceText.From(string.Empty)), out _);
-            Assert.True(result);
-
-            // Immediately close the document so that we get the source text provided by the workspace (instead of the empty one we passed).
             var info = pdbService.GetTestAccessor().Documents[file.FilePath];
-            masWorkspace!.OnDocumentClosed(info.DocumentId, new WorkspaceFileTextLoader(workspace.Services.SolutionServices, file.FilePath, info.Encoding));
-
             var document = masWorkspace!.CurrentSolution.GetRequiredDocument(info.DocumentId);
 
             // Mapping the project from the generated document should map back to the original project

src/EditorFeatures/CSharpTest/PdbSourceDocument/NullResultMetadataAsSourceFileProvider.cs

@@ -47,17 +47,6 @@ public void CleanupGeneratedFiles(MetadataAsSourceWorkspace workspace)
         return null;
     }
 
-    public bool TryAddDocumentToWorkspace(MetadataAsSourceWorkspace workspace, string filePath, Text.SourceTextContainer sourceTextContainer, [NotNullWhen(true)] out DocumentId? documentId)
-    {
-        documentId = null!;
-        return true;
-    }
-
-    public bool TryRemoveDocumentFromWorkspace(MetadataAsSourceWorkspace workspace, string filePath)
-    {
-        return true;
-    }
-
     public bool ShouldCollapseOnOpen(MetadataAsSourceWorkspace workspace, string filePath, BlockStructureOptions options)
     {
         return true;

src/EditorFeatures/CSharpTest/PdbSourceDocument/PdbSourceDocumentTests.cs

@@ -911,7 +911,7 @@ public class C
             var file = await service.GetGeneratedFileAsync(project.Solution.Workspace, project, symbol, signaturesOnly: false, options: MetadataAsSourceOptions.Default, cancellationToken: CancellationToken.None);
 
             var result = service.TryRemoveDocumentFromWorkspace(file.FilePath);
-            Assert.False(result);
+            Assert.True(result);
         });
 
     [Fact, WorkItem("https://github.com/dotnet/vscode-csharp/issues/7514")]
@@ -942,6 +942,6 @@ public class C
 
             // Opening should still throw (should never be called as we should be able to find the previously
             // opened document in the MAS workspace).
-            Assert.Throws<System.InvalidOperationException>(() => service.TryAddDocumentToWorkspace(fileTwo.FilePath, new StaticSourceTextContainer(SourceText.From(string.Empty)), out var documentIdTwo));
+            Assert.Throws<System.ArgumentException>(() => service.TryAddDocumentToWorkspace(fileTwo.FilePath, new StaticSourceTextContainer(SourceText.From(string.Empty)), out var documentIdTwo));
         });
 }

src/Features/Core/Portable/MetadataAsSource/DecompilationMetadataAsSourceFileProvider.cs

@@ -32,18 +32,6 @@ internal interface IMetadataAsSourceFileProvider
     /// </summary>
     void CleanupGeneratedFiles(MetadataAsSourceWorkspace workspace);
 
-    /// <summary>
-    /// Called when the file returned from <see cref="GetGeneratedFileAsync"/> needs to be added to the workspace,
-    /// to be opened.  Will be called on the main thread of the workspace host.
-    /// </summary>
-    bool TryAddDocumentToWorkspace(MetadataAsSourceWorkspace workspace, string filePath, SourceTextContainer sourceTextContainer, [NotNullWhen(true)] out DocumentId? documentId);
-
-    /// <summary>
-    /// Called when the file is being closed, and so needs to be removed from the workspace.  Will be called on the
-    /// main thread of the workspace host.
-    /// </summary>
-    bool TryRemoveDocumentFromWorkspace(MetadataAsSourceWorkspace workspace, string filePath);
-
     /// <summary>
     /// Called to determine if the file should be collapsed by default when opened for the first time.  Will be
     /// called on the main thread of the workspace host.

src/Features/Core/Portable/MetadataAsSource/IMetadataAsSourceFileProvider.cs

@@ -167,47 +167,56 @@ private static void AssertIsMainThread(MetadataAsSourceWorkspace workspace)
 
     public bool TryAddDocumentToWorkspace(string filePath, SourceTextContainer sourceTextContainer, [NotNullWhen(true)] out DocumentId? documentId)
     {
-        // If we haven't even created a MetadataAsSource workspace yet, then this file definitely cannot be added to
-        // it. This happens when the MiscWorkspace calls in to just see if it can attach this document to the
-        // MetadataAsSource instead of itself.
         var workspace = _workspace;
-        if (workspace != null)
+        if (workspace is null)
         {
-            foreach (var provider in _providers.Value)
-            {
-                if (!provider.IsValueCreated)
-                    continue;
+            // If we haven't even created a MetadataAsSource workspace yet, then this file definitely cannot be added to
+            // it. This happens when the MiscWorkspace calls in to just see if it can attach this document to the
+            // MetadataAsSource instead of itself.
+            documentId = null;
+            return false;
+        }
 
-                if (provider.Value.TryAddDocumentToWorkspace(workspace, filePath, sourceTextContainer, out documentId))
-                {
-                    return true;
-                }
-            }
+        // There are no linked files in the MetadataAsSource workspace, so we can just use the first document id
+        documentId = workspace.CurrentSolution.GetDocumentIdsWithFilePath(filePath).SingleOrDefault();
+        if (documentId is null)
+        {
+            return false;
         }
 
-        documentId = null;
-        return false;
+        workspace.OnDocumentOpened(documentId, sourceTextContainer);
+        return true;
     }
 
     public bool TryRemoveDocumentFromWorkspace(string filePath)
     {
-        // If we haven't even created a MetadataAsSource workspace yet, then this file definitely cannot be removed
-        // from it. This happens when the MiscWorkspace is hearing about a doc closing, and calls into the
-        // MetadataAsSource system to see if it owns the file and should handle that event.
         var workspace = _workspace;
-        if (workspace != null)
+        if (workspace is null)
         {
-            foreach (var provider in _providers.Value)
-            {
-                if (!provider.IsValueCreated)
-                    continue;
+            // If we haven't even created a MetadataAsSource workspace yet, then this file definitely cannot be removed
+            // from it. This happens when the MiscWorkspace is hearing about a doc closing, and calls into the
+            // MetadataAsSource system to see if it owns the file and should handle that event.
+            return false;
+        }
 
-                if (provider.Value.TryRemoveDocumentFromWorkspace(workspace, filePath))
-                    return true;
-            }
+        // There are no linked files in the MetadataAsSource workspace, so we can just use the first document id
+        var documentId = workspace.CurrentSolution.GetDocumentIdsWithFilePath(filePath).FirstOrDefault();
+        if (documentId is null)
+        {
+            return false;
         }
 
-        return false;
+        // In LSP, while calls to TryAddDocumentToWorkspace and TryRemoveDocumentFromWorkspace are handled
+        // serially, it is possible that TryRemoveDocumentFromWorkspace called without TryAddDocumentToWorkspace first.
+        // This can happen if the document is immediately closed after opening - only feature requests that force us
+        // to materialize a solution will trigger TryAddDocumentToWorkspace, if none are made it is never called.
+        // However TryRemoveDocumentFromWorkspace is always called on close.
+        if (workspace.GetOpenDocumentIds().Contains(documentId))
+        {
+            workspace.OnDocumentClosed(documentId, new WorkspaceFileTextLoader(workspace.Services.SolutionServices, filePath, defaultEncoding: null));
+        }
+
+        return true;
     }
 
     public bool ShouldCollapseOnOpen(string? filePath, BlockStructureOptions blockStructureOptions)

src/Features/Core/Portable/MetadataAsSource/MetadataAsSourceFileService.cs

@@ -3,27 +3,23 @@
 // See the LICENSE file in the project root for more information.
 
 using System;
-using System.Collections.Immutable;
 using System.IO;
 using System.Text;
-using Microsoft.CodeAnalysis.Host;
 using Microsoft.CodeAnalysis.Text;
 
 namespace Microsoft.CodeAnalysis.MetadataAsSource;
 
 internal sealed class MetadataAsSourceGeneratedFileInfo
 {
-    public readonly ProjectId SourceProjectId;
-    public readonly Workspace Workspace;
+    public string TemporaryFilePath { get; }
+    public Workspace Workspace { get; }
+    public ProjectId SourceProjectId { get; }
+    public bool SignaturesOnly { get; }
+    public string LanguageName { get; }
+    public string Extension { get; }
 
-    public readonly AssemblyIdentity AssemblyIdentity;
-    public readonly string LanguageName;
-    public readonly bool SignaturesOnly;
-    public readonly ImmutableArray<MetadataReference> References;
-
-    public readonly string TemporaryFilePath;
-
-    private readonly ParseOptions? _parseOptions;
+    public static Encoding Encoding => Encoding.UTF8;
+    public static SourceHashAlgorithm ChecksumAlgorithm => SourceHashAlgorithms.Default;
 
     public MetadataAsSourceGeneratedFileInfo(string rootPath, Workspace sourceWorkspace, Project sourceProject, INamedTypeSymbol topLevelNamedType, bool signaturesOnly)
     {
@@ -32,78 +28,9 @@ public MetadataAsSourceGeneratedFileInfo(string rootPath, Workspace sourceWorksp
         this.LanguageName = signaturesOnly ? sourceProject.Language : LanguageNames.CSharp;
         this.SignaturesOnly = signaturesOnly;
 
-        _parseOptions = sourceProject.Language == LanguageName
-            ? sourceProject.ParseOptions
-            : sourceProject.Solution.Services.GetLanguageServices(LanguageName).GetRequiredService<ISyntaxTreeFactoryService>().GetDefaultParseOptionsWithLatestLanguageVersion();
-
-        this.References = [.. sourceProject.MetadataReferences];
-        this.AssemblyIdentity = topLevelNamedType.ContainingAssembly.Identity;
-
-        var extension = LanguageName == LanguageNames.CSharp ? ".cs" : ".vb";
+        this.Extension = LanguageName == LanguageNames.CSharp ? ".cs" : ".vb";
 
         var directoryName = Guid.NewGuid().ToString("N");
-        this.TemporaryFilePath = Path.Combine(rootPath, directoryName, topLevelNamedType.Name + extension);
-    }
-
-    public static Encoding Encoding => Encoding.UTF8;
-    public static SourceHashAlgorithm ChecksumAlgorithm => SourceHashAlgorithms.Default;
-
-    /// <summary>
-    /// Creates a ProjectInfo to represent the fake project created for metadata as source documents.
-    /// </summary>
-    /// <param name="services">Solution services.</param>
-    /// <param name="loadFileFromDisk">Whether the source file already exists on disk and should be included. If
-    /// this is a false, a document is still created, but it's not backed by the file system and thus we won't
-    /// try to load it.</param>
-    public (ProjectInfo, DocumentId) GetProjectInfoAndDocumentId(SolutionServices services, bool loadFileFromDisk)
-    {
-        var projectId = ProjectId.CreateNewId();
-
-        // Just say it's always a DLL since we probably won't have a Main method
-        var compilationOptions = services.GetRequiredLanguageService<ICompilationFactoryService>(LanguageName).GetDefaultCompilationOptions().WithOutputKind(OutputKind.DynamicallyLinkedLibrary);
-
-        var extension = LanguageName == LanguageNames.CSharp ? ".cs" : ".vb";
-
-        // We need to include the version information of the assembly so InternalsVisibleTo and stuff works
-        var assemblyInfoDocumentId = DocumentId.CreateNewId(projectId);
-        var assemblyInfoFileName = "AssemblyInfo" + extension;
-        var assemblyInfoString = LanguageName == LanguageNames.CSharp
-            ? string.Format(@"[assembly: System.Reflection.AssemblyVersion(""{0}"")]", AssemblyIdentity.Version)
-            : string.Format(@"<Assembly: System.Reflection.AssemblyVersion(""{0}"")>", AssemblyIdentity.Version);
-
-        var assemblyInfoSourceText = SourceText.From(assemblyInfoString, Encoding, ChecksumAlgorithm);
-
-        var assemblyInfoDocument = DocumentInfo.Create(
-            assemblyInfoDocumentId,
-            assemblyInfoFileName,
-            loader: TextLoader.From(assemblyInfoSourceText.Container, VersionStamp.Default),
-            filePath: null,
-            isGenerated: true)
-            .WithDesignTimeOnly(true);
-
-        var generatedDocumentId = DocumentId.CreateNewId(projectId);
-        var generatedDocument = DocumentInfo.Create(
-            generatedDocumentId,
-            Path.GetFileName(TemporaryFilePath),
-            loader: loadFileFromDisk ? new WorkspaceFileTextLoader(services, TemporaryFilePath, Encoding) : null,
-            filePath: TemporaryFilePath,
-            isGenerated: true)
-            .WithDesignTimeOnly(true);
-
-        var projectInfo = ProjectInfo.Create(
-            new ProjectInfo.ProjectAttributes(
-                id: projectId,
-                version: VersionStamp.Default,
-                name: AssemblyIdentity.Name,
-                assemblyName: AssemblyIdentity.Name,
-                language: LanguageName,
-                compilationOutputInfo: default,
-                checksumAlgorithm: ChecksumAlgorithm),
-            compilationOptions: compilationOptions,
-            parseOptions: _parseOptions,
-            documents: [assemblyInfoDocument, generatedDocument],
-            metadataReferences: References);
-
-        return (projectInfo, generatedDocumentId);
+        this.TemporaryFilePath = Path.Combine(rootPath, directoryName, topLevelNamedType.Name + Extension);
     }
 }

src/Features/Core/Portable/MetadataAsSource/MetadataAsSourceGeneratedFileInfo.cs

@@ -44,11 +44,6 @@ internal sealed class PdbSourceDocumentMetadataAsSourceFileProvider(
     private readonly IImplementationAssemblyLookupService _implementationAssemblyLookupService = implementationAssemblyLookupService;
     private readonly IPdbSourceDocumentLogger? _logger = logger;
 
-    /// <summary>
-    /// Lock to guard access to workspace updates when opening / closing documents.
-    /// </summary>
-    private readonly object _gate = new();
-
     /// <summary>
     /// Accessed only in <see cref="GetGeneratedFileAsync"/> and <see cref="CleanupGeneratedFiles"/>, both of which
     /// are called under a lock in <see cref="MetadataAsSourceFileService"/>.  So this is safe as a plain
@@ -70,11 +65,6 @@ internal sealed class PdbSourceDocumentMetadataAsSourceFileProvider(
     /// </summary>
     private readonly ConcurrentDictionary<string, SourceDocumentInfo> _fileToDocumentInfoMap = new(StringComparer.OrdinalIgnoreCase);
 
-    /// <summary>
-    /// Only accessed and mutated in serial calls either from the UI thread or LSP queue.
-    /// </summary>
-    private readonly HashSet<DocumentId> _openedDocumentIds = [];
-
     public async Task<MetadataAsSourceFile?> GetGeneratedFileAsync(
         MetadataAsSourceWorkspace metadataWorkspace,
         Workspace sourceWorkspace,
@@ -262,24 +252,22 @@ internal sealed class PdbSourceDocumentMetadataAsSourceFileProvider(
 
         var symbolId = SymbolKey.Create(symbol, cancellationToken);
 
-        // Get a view of the solution with the document added, but do not actually update the workspace.
-        // TryAddDocumentToWorkspace is responsible for actually updating the solution with the new document(s).
-        // We just need a view with the document added so we can find the right location in the generated source.
-        var pendingSolution = metadataWorkspace.CurrentSolution;
+        var navigateProject = metadataWorkspace.CurrentSolution.GetRequiredProject(projectId);
         var documentInfos = CreateDocumentInfos(sourceFileInfos, encoding, projectId, sourceWorkspace, sourceProject);
         if (documentInfos.Length > 0)
         {
             foreach (var documentInfo in documentInfos)
             {
                 // The document might have already been added by a previous go to definition call.
-                if (!pendingSolution.ContainsDocument(documentInfo.Id))
+                if (!metadataWorkspace.CurrentSolution.ContainsDocument(documentInfo.Id))
                 {
-                    pendingSolution = pendingSolution.AddDocument(documentInfo);
+                    metadataWorkspace.OnDocumentAdded(documentInfo);
                 }
             }
-        }
 
-        var navigateProject = pendingSolution.GetRequiredProject(projectId);
+            // Get a new view of the project with the documents added.
+            navigateProject = metadataWorkspace.CurrentSolution.GetRequiredProject(projectId);
+        }
 
         // If MetadataAsSourceHelpers.GetLocationInGeneratedSourceAsync can't find the actual document to navigate to, it will fall back
         // to the document passed in, which we just use the first document for.
@@ -383,50 +371,6 @@ public bool ShouldCollapseOnOpen(MetadataAsSourceWorkspace workspace, string fil
         return _fileToDocumentInfoMap.TryGetValue(filePath, out _) && blockStructureOptions.CollapseMetadataImplementationsWhenFirstOpened;
     }
 
-    public bool TryAddDocumentToWorkspace(MetadataAsSourceWorkspace workspace, string filePath, SourceTextContainer sourceTextContainer, [NotNullWhen(true)] out DocumentId? documentId)
-    {
-        lock (_gate)
-        {
-            if (_fileToDocumentInfoMap.TryGetValue(filePath, out var info))
-            {
-                Contract.ThrowIfTrue(_openedDocumentIds.Contains(info.DocumentId));
-
-                workspace.OnDocumentAdded(info.DocumentInfo);
-                workspace.OnDocumentOpened(info.DocumentId, sourceTextContainer);
-                documentId = info.DocumentId;
-                _openedDocumentIds.Add(documentId);
-                return true;
-            }
-
-            documentId = null;
-            return false;
-        }
-    }
-
-    public bool TryRemoveDocumentFromWorkspace(MetadataAsSourceWorkspace workspace, string filePath)
-    {
-        lock (_gate)
-        {
-            if (_fileToDocumentInfoMap.TryGetValue(filePath, out var info))
-            {
-                // In LSP, while calls to TryAddDocumentToWorkspace and TryRemoveDocumentFromWorkspace are handled
-                // serially, it is possible that TryRemoveDocumentFromWorkspace called without TryAddDocumentToWorkspace first.
-                // This can happen if the document is immediately closed after opening - only feature requests that force us
-                // to materialize a solution will trigger TryAddDocumentToWorkspace, if none are made it is never called.
-                // However TryRemoveDocumentFromWorkspace is always called on close.
-                if (_openedDocumentIds.Contains(info.DocumentId))
-                {
-                    workspace.OnDocumentClosed(info.DocumentId, new WorkspaceFileTextLoader(workspace.Services.SolutionServices, filePath, info.Encoding));
-                    workspace.OnDocumentRemoved(info.DocumentId);
-                    _openedDocumentIds.Remove(info.DocumentId);
-                    return true;
-                }
-            }
-
-            return false;
-        }
-    }
-
     public Project? MapDocument(Document document)
     {
         if (document.FilePath is not null &&
@@ -462,7 +406,6 @@ public void CleanupGeneratedFiles(MetadataAsSourceWorkspace workspace)
 
         // The MetadataAsSourceFileService will clean up the entire temp folder so no need to do anything here
         _fileToDocumentInfoMap.Clear();
-        _openedDocumentIds.Clear();
         _sourceLinkEnabledProjects.Clear();
         _implementationAssemblyLookupService.Clear();
     }