Add metadata documents to the MAS workspace upfront

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

@@ -1022,7 +1022,7 @@ await RunTestAsync(async path =>
             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);
         });
     }
 
@@ -1057,7 +1057,7 @@ await RunTestAsync(async path =>
 
             // 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/IMetadataAsSourceFileProvider.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/MetadataAsSourceFileService.cs

@@ -165,47 +165,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).FirstOrDefault();
+        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/MetadataAsSourceGeneratedFileInfo.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/PdbSourceDocument/PdbSourceDocumentMetadataAsSourceFileProvider.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();
     }