Skip to content
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.

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

Do not share trees across files shared in different language projects #75606

Merged
merged 3 commits into from
Oct 23, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -181,6 +181,10 @@ static bool TryReuseSiblingRoot(
// is the same.
bool CanReuseSiblingRoot(bool forceEvenIfTreesWouldDiffer)
{
// We can never reuse trees across languages.
if (siblingRoot.Language != languageServices.Language)
return false;

// If we're forcing reuse of a sibling tree, then this always succeeds.
if (forceEvenIfTreesWouldDiffer)
return true;
Expand Down
4 changes: 2 additions & 2 deletions src/Workspaces/Core/Portable/Workspace/Solution/Solution.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1538,8 +1538,8 @@ internal async Task<Solution> WithMergedLinkedFileChangesAsync(
return (await session.MergeDiffsAsync(mergeConflictHandler, cancellationToken).ConfigureAwait(false)).MergedSolution;
}

internal ImmutableArray<DocumentId> GetRelatedDocumentIds(DocumentId documentId)
=> this.SolutionState.GetRelatedDocumentIds(documentId);
internal ImmutableArray<DocumentId> GetRelatedDocumentIds(DocumentId documentId, bool includeDifferentLanguages = false)
=> this.SolutionState.GetRelatedDocumentIds(documentId, includeDifferentLanguages);

/// <summary>
/// Returns one of any of the related documents of <paramref name="documentId"/>. Importantly, this will never
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -977,8 +977,8 @@ public SolutionCompilationState WithDocumentContentsFrom(
arg: forceEvenIfTreesWouldDiffer,
static (oldDocumentState, documentState, forceEvenIfTreesWouldDiffer) =>
oldDocumentState.TextAndVersionSource == documentState.TextAndVersionSource && oldDocumentState.TreeSource == documentState.TreeSource
? oldDocumentState
: oldDocumentState.UpdateTextAndTreeContents(documentState.TextAndVersionSource, documentState.TreeSource, forceEvenIfTreesWouldDiffer));
? oldDocumentState
: oldDocumentState.UpdateTextAndTreeContents(documentState.TextAndVersionSource, documentState.TreeSource, forceEvenIfTreesWouldDiffer));
}

/// <inheritdoc cref="SolutionState.WithDocumentSourceCodeKind"/>
Expand Down Expand Up @@ -1561,7 +1561,12 @@ public SolutionCompilationState WithFrozenPartialCompilationIncludingSpecificDoc
// WithDocumentContentsFrom with the current document state no-ops immediately, returning back the same
// compilation state instance. So in the case where there are no linked documents, there is no cost here. And
// there is no additional cost processing the initiating document in this loop.
var allDocumentIds = this.SolutionState.GetRelatedDocumentIds(documentId);
//
// Note: when getting related document ids, we want to include those from different languages. That way we
// ensure a consistent state where all the files (even those shared across languages) agree on their contents.
const bool includeDifferentLanguages = true;

var allDocumentIds = this.SolutionState.GetRelatedDocumentIds(documentId, includeDifferentLanguages);
var allDocumentIdsWithCurrentDocumentState = allDocumentIds.SelectAsArray(static (docId, currentDocumentState) => (docId, currentDocumentState), currentDocumentState);
currentCompilationState = currentCompilationState.WithDocumentContentsFrom(allDocumentIdsWithCurrentDocumentState, forceEvenIfTreesWouldDiffer: true);

Expand All @@ -1573,7 +1578,7 @@ static SolutionCompilationState WithFrozenPartialCompilationIncludingSpecificDoc
{
try
{
var allDocumentIds = @this.SolutionState.GetRelatedDocumentIds(documentId);
var allDocumentIds = @this.SolutionState.GetRelatedDocumentIds(documentId, includeDifferentLanguages);
using var _ = ArrayBuilder<DocumentState>.GetInstance(allDocumentIds.Length, out var documentStates);

// We grab all the contents of linked files as well to ensure that our snapshot is correct wrt to the
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1304,7 +1304,7 @@ public SolutionState WithAnalyzerReferences(IReadOnlyList<AnalyzerReference> ana
return null;
}

public ImmutableArray<DocumentId> GetRelatedDocumentIds(DocumentId documentId)
public ImmutableArray<DocumentId> GetRelatedDocumentIds(DocumentId documentId, bool includeDifferentLanguages)
{
var projectState = this.GetProjectState(documentId.ProjectId);
if (projectState == null)
Expand All @@ -1331,7 +1331,9 @@ public ImmutableArray<DocumentId> GetRelatedDocumentIds(DocumentId documentId)
return documentIds.WhereAsArray(
static (documentId, args) =>
{
var projectState = args.solution.GetProjectState(documentId.ProjectId);
var (@this, language, includeDifferentLanguages) = args;

var projectState = @this.GetProjectState(documentId.ProjectId);
if (projectState == null)
{
// this document no longer exist
Expand All @@ -1341,13 +1343,13 @@ public ImmutableArray<DocumentId> GetRelatedDocumentIds(DocumentId documentId)
return false;
}

if (projectState.ProjectInfo.Language != args.Language)
if (!includeDifferentLanguages && projectState.ProjectInfo.Language != language)
return false;

// GetDocumentIdsWithFilePath may return DocumentIds for other types of documents (like additional files), so filter to normal documents
return projectState.DocumentStates.Contains(documentId);
},
(solution: this, projectState.Language));
(solution: this, projectState.Language, includeDifferentLanguages));
}

/// <summary>
Expand Down
6 changes: 4 additions & 2 deletions src/Workspaces/Core/Portable/Workspace/Workspace.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1254,8 +1254,10 @@ private void OnAnyDocumentTextChanged<TArg>(
{
updatedDocumentIds.Add(documentId);

// Now go update the linked docs to have the same doc contents.
var linkedDocumentIds = oldSolution.GetRelatedDocumentIds(documentId);
// Now go update the linked docs to have the same doc contents. Note: We want to do this even across
// languags. If two projects are actually referring to the same file and that file changes, we need
// them all to agree on the contents to leave us in a consistent state.
var linkedDocumentIds = oldSolution.GetRelatedDocumentIds(documentId, includeDifferentLanguages: true);
if (linkedDocumentIds.Length > 0)
{
// Have the linked documents point *into* the same instance data that the initial document
Expand Down
88 changes: 88 additions & 0 deletions src/Workspaces/CoreTest/SolutionTests/SolutionTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -823,6 +823,94 @@ public class Goo { }
Assert.True(root1.IsIncrementallyIdenticalTo(root2));
}

[Theory, CombinatorialData]
public async Task WithDocumentText_LinkedFiles_DifferentLanguage(
PreservationMode mode,
TextUpdateType updateType)
{
var parseOptions1 = CSharpParseOptions.Default;
var parseOptions2 = VisualBasicParseOptions.Default;
var projectId1 = ProjectId.CreateNewId();
var projectId2 = ProjectId.CreateNewId();

using var workspace = CreateWorkspace();

var docContents = "";

// Validate strange case were we have linked files to the same file, but with different languages.
Assert.True(workspace.TryApplyChanges(workspace.CurrentSolution
.AddProject(projectId1, "proj1", "proj1.dll", LanguageNames.CSharp).WithProjectParseOptions(projectId1, parseOptions1)
.AddDocument(DocumentId.CreateNewId(projectId1), "goo.cs", SourceText.From(docContents, Encoding.UTF8, SourceHashAlgorithms.Default), filePath: "goo.cs")
.AddProject(projectId2, "proj2", "proj2.dll", LanguageNames.VisualBasic).WithProjectParseOptions(projectId2, parseOptions2)
.AddDocument(DocumentId.CreateNewId(projectId2), "goo.cs", SourceText.From(docContents, Encoding.UTF8, SourceHashAlgorithms.Default), filePath: "goo.cs")));

var solution = workspace.CurrentSolution;

var documentId1 = solution.Projects.First().DocumentIds.Single();
var documentId2 = solution.Projects.Last().DocumentIds.Single();

var document1 = solution.GetRequiredDocument(documentId1);
var document2 = solution.GetRequiredDocument(documentId2);

var text1 = await document1.GetTextAsync();
var text2 = await document2.GetTextAsync();
var version1 = await document1.GetTextVersionAsync();
var version2 = await document2.GetTextVersionAsync();
var root1 = await document1.GetRequiredSyntaxRootAsync(CancellationToken.None);
var root2 = await document2.GetRequiredSyntaxRootAsync(CancellationToken.None);

Assert.Equal(text1.ToString(), text2.ToString());
Assert.Equal(version1, version2);
CyrusNajmabadi marked this conversation as resolved.
Show resolved Hide resolved

// These are different languages, so we should get entirely different tree structures.
Assert.NotEqual(root1.GetType(), root2.GetType());

var text = SourceText.From(" ", encoding: null, SourceHashAlgorithm.Sha1);
var textAndVersion = TextAndVersion.Create(text, VersionStamp.Create());
solution = UpdateSolution(mode, updateType, solution, documentId1, text, textAndVersion);

// because we only forked one doc, the text/versions should be different in this interim solution.

document1 = solution.GetRequiredDocument(documentId1);
document2 = solution.GetRequiredDocument(documentId2);

text1 = await document1.GetTextAsync();
text2 = await document2.GetTextAsync();
version1 = await document1.GetTextVersionAsync();
version2 = await document2.GetTextVersionAsync();
root1 = await document1.GetRequiredSyntaxRootAsync(CancellationToken.None);
root2 = await document2.GetRequiredSyntaxRootAsync(CancellationToken.None);

Assert.NotEqual(text1.ToString(), text2.ToString());

// The versions will not match as we won't share the underlying text-and-tree instances between languages.
Assert.NotEqual(version1, version2);

// These are different languages, so we should get entirely different tree structures.
Assert.NotEqual(root1.GetType(), root2.GetType());

// Now apply the change to the workspace. This should bring the linked document in sync with the one we changed.
// But not cause them to share trees.
workspace.TryApplyChanges(solution);
solution = workspace.CurrentSolution;

document1 = solution.GetRequiredDocument(documentId1);
document2 = solution.GetRequiredDocument(documentId2);

text1 = await document1.GetTextAsync();
text2 = await document2.GetTextAsync();
version1 = await document1.GetTextVersionAsync();
version2 = await document2.GetTextVersionAsync();
root1 = await document1.GetRequiredSyntaxRootAsync(CancellationToken.None);
root2 = await document2.GetRequiredSyntaxRootAsync(CancellationToken.None);

Assert.Equal(text1.ToString(), text2.ToString());
Assert.Equal(version1, version2);

// These are different languages, so we should get entirely different tree structures.
Assert.NotEqual(root1.GetType(), root2.GetType());
}

[Fact]
public void WithAdditionalDocumentText_SourceText()
{
Expand Down
Loading