Skip to content

Commit

Permalink
Merge pull request #66805 from CyrusNajmabadi/farCrash
Browse files Browse the repository at this point in the history
Fix crash in FAR helpers when only part of a linked-document set has been updated
  • Loading branch information
CyrusNajmabadi authored Feb 13, 2023
2 parents 43eb511 + b804e45 commit c768df7
Show file tree
Hide file tree
Showing 2 changed files with 82 additions and 0 deletions.
73 changes: 73 additions & 0 deletions src/EditorFeatures/Test2/FindReferences/FindReferencesTests.vb
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
Imports System.Collections.Immutable
Imports System.Threading
Imports Microsoft.CodeAnalysis
Imports Microsoft.CodeAnalysis.CSharp.Syntax
Imports Microsoft.CodeAnalysis.Editor.UnitTests.Workspaces
Imports Microsoft.CodeAnalysis.FindSymbols
Imports Microsoft.CodeAnalysis.FindUsages
Expand All @@ -13,6 +14,7 @@ Imports Microsoft.CodeAnalysis.Options
Imports Microsoft.CodeAnalysis.PooledObjects
Imports Microsoft.CodeAnalysis.Remote.Testing
Imports Microsoft.CodeAnalysis.Text
Imports Microsoft.CodeAnalysis.VisualBasic.Completion.KeywordRecommenders.PreprocessorDirectives
Imports Roslyn.Utilities
Imports Xunit.Abstractions

Expand Down Expand Up @@ -513,5 +515,76 @@ Namespace Microsoft.CodeAnalysis.Editor.UnitTests.FindReferences
Private Shared Function GetFilePathAndProjectLabel(hostDocument As TestHostDocument) As String
Return $"{hostDocument.Project.Name}: {hostDocument.FilePath}"
End Function

<Fact>
Public Async Function LinkedFilesWhereContentHasChangedInOneLink() As Task
Using workspace = TestWorkspace.Create("
<Workspace>
<Project Language='C#' CommonReferences='true' AssemblyName='LinkedProj1' Name='CSProj.1'>
<Document FilePath='C.cs'>
partial class C
{
int i;

public int P { get { return i; } }

public C()
{
this.i = 0;
}
}
</Document>
</Project>
<Project Language='C#' CommonReferences='true' AssemblyName='LinkedProj2' Name='CSProj.2'>
<Document IsLinkFile='true' LinkProjectName='CSProj.1' LinkFilePath='C.cs'/>
</Project>
</Workspace>")

Dim solution = workspace.CurrentSolution
Dim document1 = solution.Projects.Single(Function(p) p.Name = "CSProj.1").Documents.Single()
Dim text1 = Await document1.GetTextAsync()

Dim linkedDocuments = document1.GetLinkedDocumentIds()
Assert.Equal(1, linkedDocuments.Length)

Dim document2 = solution.GetDocument(linkedDocuments.Single())
Assert.NotSame(document1, document2)

' ensure we normally have two linked symbols when the files are the same.
Await LinkedFileTestHelper(solution, expectedLinkedSymbolCount:=2)

' now change the linked file and run again.
solution = solution.WithDocumentText(document2.Id, SourceText.From(""))
Await LinkedFileTestHelper(solution, expectedLinkedSymbolCount:=1)

' changing the contents back to the original should return us to two symbols
solution = solution.WithDocumentText(document2.Id, text1)
Await LinkedFileTestHelper(solution, expectedLinkedSymbolCount:=2)

' changing `int i` to `int j` should give us 1 symbol. the text lengths are the same, but the symbols
' have changed.
solution = solution.WithDocumentText(document2.Id, SourceText.From(text1.ToString().Replace("int i", "int j")))
Await LinkedFileTestHelper(solution, expectedLinkedSymbolCount:=1)
End Using
End Function

Private Shared Async Function LinkedFileTestHelper(solution As Solution, expectedLinkedSymbolCount As Integer) As Task
Dim document1 = solution.Projects.Single(Function(p) p.Name = "CSProj.1").Documents.Single()

Dim linkedDocuments = document1.GetLinkedDocumentIds()
Assert.Equal(1, linkedDocuments.Length)

Dim document2 = solution.GetDocument(linkedDocuments.Single())
Assert.NotSame(document1, document2)

Dim semanticModel1 = Await document1.GetSemanticModelAsync()
Dim root1 = Await semanticModel1.SyntaxTree.GetRootAsync()
Dim declarator1 = root1.DescendantNodes().OfType(Of VariableDeclaratorSyntax).First()
Dim symbol1 = semanticModel1.GetDeclaredSymbol(declarator1)
Assert.NotNull(symbol1)

Dim linkedSymbols = Await SymbolFinder.FindLinkedSymbolsAsync(symbol1, solution, cancellationToken:=Nothing)
Assert.Equal(expectedLinkedSymbolCount, linkedSymbols.Length)
End Function
End Class
End Namespace
9 changes: 9 additions & 0 deletions src/Workspaces/Core/Portable/FindSymbols/SymbolFinder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -259,6 +259,7 @@ internal static async Task<ImmutableArray<ISymbol>> FindLinkedSymbolsAsync(
foreach (var location in symbol.DeclaringSyntaxReferences)
{
var originalDocument = solution.GetDocument(location.SyntaxTree);
var originalRoot = await location.SyntaxTree.GetRootAsync(cancellationToken).ConfigureAwait(false);

// GetDocument will return null for locations in #load'ed trees. TODO: Remove this check and add logic
// to fetch the #load'ed tree's Document once https://github.com/dotnet/roslyn/issues/5260 is fixed.
Expand All @@ -271,7 +272,15 @@ internal static async Task<ImmutableArray<ISymbol>> FindLinkedSymbolsAsync(
foreach (var linkedDocumentId in originalDocument.GetLinkedDocumentIds())
{
var linkedDocument = solution.GetRequiredDocument(linkedDocumentId);

// It's possible for us to have a solution snapshot where only part of a linked set of documents has
// been updated. As such, the other linked docs may have different contents/sizes than the original
// doc we started with. Skip those files as there's no sensible way to say that we have linked
// symbols here when the contents are not the same.
var linkedSyntaxRoot = await linkedDocument.GetRequiredSyntaxRootAsync(cancellationToken).ConfigureAwait(false);
if (originalRoot.FullSpan != linkedSyntaxRoot.FullSpan)
continue;

var linkedNode = linkedSyntaxRoot.FindNode(location.Span, getInnermostNodeForTie: true);

var semanticModel = await linkedDocument.GetRequiredSemanticModelAsync(cancellationToken).ConfigureAwait(false);
Expand Down

0 comments on commit c768df7

Please # to comment.