Skip to content

Add metadata documents to the MAS workspace upfront #78886

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Add metadata documents to the MAS workspace upfront #78886

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a7e6b19
Add metadata documents to the MAS workspace upfront
dibarbet Jun 6, 2025
91702f6
Use singleordefault as there should only be one documentId per path
dibarbet Jun 11, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 0 additions & 8 deletions src/EditorFeatures/CSharpTest/PdbSourceDocument/AbstractPdbSourceDocumentTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
11 changes: 0 additions & 11 deletions src/EditorFeatures/CSharpTest/PdbSourceDocument/NullResultMetadataAsSourceFileProvider.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
4 changes: 2 additions & 2 deletions src/EditorFeatures/CSharpTest/PdbSourceDocument/PdbSourceDocumentTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this still be returning false? I'd assume we don't say we removed something if we didn't add it in the first place? It seems like this is missing a different return for the "if not open" case.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is deleted in the followup (open doesn't always make sense here)
#79023

});
}

Expand Down Expand Up @@ -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));
});
}
}
343 changes: 176 additions & 167 deletions src/Features/Core/Portable/MetadataAsSource/DecompilationMetadataAsSourceFileProvider.cs
View file
Open in desktop

Large diffs are not rendered by default.

12 changes: 0 additions & 12 deletions src/Features/Core/Portable/MetadataAsSource/IMetadataAsSourceFileProvider.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
63 changes: 36 additions & 27 deletions src/Features/Core/Portable/MetadataAsSource/MetadataAsSourceFileService.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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).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)
Expand Down
93 changes: 10 additions & 83 deletions src/Features/Core/Portable/MetadataAsSource/MetadataAsSourceGeneratedFileInfo.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
{
Expand All @@ -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);
}
}
69 changes: 6 additions & 63 deletions ...Features/Core/Portable/PdbSourceDocument/PdbSourceDocumentMetadataAsSourceFileProvider.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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,
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -383,50 +371,6 @@ public bool ShouldCollapseOnOpen(MetadataAsSourceWorkspace workspace, string fil
return _fileToDocumentInfoMap.TryGetValue(filePath, out _) && blockStructureOptions.CollapseMetadataImplementationsWhenFirstOpened;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm beginning to wonder why _fileToDocumentInfoMap needs to exist at all -- couldn't that now just be centrally held?

}

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 &&
Expand Down Expand Up @@ -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();
}
Expand Down
Loading
Loading