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.

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

Jump to bottom

Fix race condition in LSP FindAllReferences when linked files were involved. #74566

Merged
merged 5 commits into from
Jul 26, 2024
Merged

Fix race condition in LSP FindAllReferences when linked files were involved. #74566

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
e76f120
Add LSP far many references tests
dibarbet Jul 25, 2024
bce3670
Add linked file FAR test
dibarbet Jul 26, 2024
96b5350
Fix early exist in loop
CyrusNajmabadi Jul 26, 2024
ceece59
Inline'
CyrusNajmabadi Jul 26, 2024
88c618c
Update tests'
CyrusNajmabadi Jul 26, 2024
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
4 changes: 2 additions & 2 deletions src/LanguageServer/Protocol/Handler/References/FindUsagesLSPContext.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -138,15 +138,15 @@ public override async ValueTask OnReferencesFoundAsync(IAsyncEnumerable<SourceRe
// Each reference should be associated with a definition. If this somehow isn't the
// case, we bail out early.
if (!_definitionToId.TryGetValue(reference.Definition, out var definitionId))
return;
continue;
CyrusNajmabadi marked this conversation as resolved.
Show resolved Hide resolved

var documentSpan = reference.SourceSpan;
var document = documentSpan.Document;

// If this is reference to the same physical location we've already reported, just
// filter this out. it will clutter the UI to show the same places.
if (!_referenceLocations.Add((document.FilePath, reference.SourceSpan.SourceSpan)))
return;
continue;

// If the definition hasn't been reported yet, add it to our list of references to report.
if (_definitionsWithoutReference.TryGetValue(definitionId, out var definition))
Expand Down
155 changes: 135 additions & 20 deletions src/LanguageServer/ProtocolUnitTests/References/FindAllReferencesHandlerFeaturesTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,9 @@
using LSP = Roslyn.LanguageServer.Protocol;

namespace Microsoft.CodeAnalysis.LanguageServer.UnitTests.References;
public class FindAllReferencesHandlerFeaturesTests : AbstractLanguageServerProtocolTests
public sealed class FindAllReferencesHandlerFeaturesTests(ITestOutputHelper? testOutputHelper)
: AbstractLanguageServerProtocolTests(testOutputHelper)
{
public FindAllReferencesHandlerFeaturesTests(ITestOutputHelper? testOutputHelper) : base(testOutputHelper)
{
}

protected override TestComposition Composition => LspTestCompositions.LanguageServerProtocol
.AddParts(typeof(TestDocumentTrackingService))
.AddParts(typeof(TestWorkspaceRegistrationService));
Expand All @@ -26,25 +23,143 @@ public FindAllReferencesHandlerFeaturesTests(ITestOutputHelper? testOutputHelper
public async Task TestFindAllReferencesAsync_DoesNotUseVSTypes(bool mutatingLspWorkspace)
{
var markup =
@"class A
{
public int {|reference:someInt|} = 1;
void M()
{
var i = {|reference:someInt|} + 1;
"""
class A
{
public int {|reference:someInt|} = 1;
void M()
{
var i = {|reference:someInt|} + 1;
}
}
class B
{
int someInt = A.{|reference:someInt|} + 1;
void M2()
{
var j = someInt + A.{|caret:|}{|reference:someInt|};
}
}
""";
await using var testLspServer = await CreateTestLspServerAsync(markup, mutatingLspWorkspace, new LSP.ClientCapabilities());

var results = await FindAllReferencesHandlerTests.RunFindAllReferencesNonVSAsync(testLspServer, testLspServer.GetLocations("caret").First());
AssertLocationsEqual(testLspServer.GetLocations("reference"), results.Select(result => result));
}
}
class B
{
int someInt = A.{|reference:someInt|} + 1;
void M2()

[Theory, CombinatorialData]
public async Task TestFindAllReferencesAsync_LargeNumberOfReferences(bool mutatingLspWorkspace)
{
var j = someInt + A.{|caret:|}{|reference:someInt|};
}
}";
var markup =
"""
using System.Threading.Tasks
class A
{
private {|caret:Task|} someTask = Task.CompletedTask;
}
""";
await using var testLspServer = await CreateTestLspServerAsync(markup, mutatingLspWorkspace, new LSP.ClientCapabilities());

for (var i = 0; i < 100; i++)
{
var source = $$"""
using System.Threading.Tasks
class SomeClass{{i}}
{
private Task someTask;
}
""";

var testDocument = new EditorTestHostDocument(text: source, displayName: @$"C:\SomeFile{i}.cs", exportProvider: testLspServer.TestWorkspace.ExportProvider, filePath: @$"C:\SomeFile{i}.cs");
testLspServer.TestWorkspace.AddTestProject(new EditorTestHostProject(testLspServer.TestWorkspace, documents: new[] { testDocument }));
}

await WaitForWorkspaceOperationsAsync(testLspServer.TestWorkspace);

var results = await FindAllReferencesHandlerTests.RunFindAllReferencesNonVSAsync(testLspServer, testLspServer.GetLocations("caret").First());
AssertLocationsEqual(testLspServer.GetLocations("reference"), results.Select(result => result));
Assert.Equal(103, results.Length);
}

[Theory, CombinatorialData]
public async Task TestFindAllReferencesAsync_LinkedFile(bool mutatingLspWorkspace, [CombinatorialRange(0, 10)] int iteration)
{
_ = iteration;
var markup =
"""
using System.Threading.Tasks
class A
{
private void SomeMethod()
{
Do({|caret:Task|}.CompletedTask);
Do(Task.CompletedTask);
Do(Task.CompletedTask);
Do(Task.CompletedTask);
Do(Task.CompletedTask);
Do(Task.CompletedTask);
Do(Task.CompletedTask);
Do(Task.CompletedTask);
Do(Task.CompletedTask);
Do(Task.CompletedTask);
Do(Task.CompletedTask);
Do(Task.CompletedTask);
Do(Task.CompletedTask);
Do(Task.CompletedTask);
Do(Task.CompletedTask);
Do(Task.CompletedTask);
Do(Task.CompletedTask);
Do(Task.CompletedTask);
Do(Task.CompletedTask);
Do(Task.CompletedTask);
Do(Task.CompletedTask);
Do(Task.CompletedTask);
Do(Task.CompletedTask);
Do(Task.CompletedTask);
Do(Task.CompletedTask);
Do(Task.CompletedTask);
Do(Task.CompletedTask);
Do(Task.CompletedTask);
Do(Task.CompletedTask);
Do(Task.CompletedTask);
Do(Task.CompletedTask);
Do(Task.CompletedTask);
Do(Task.CompletedTask);
Do(Task.CompletedTask);
Do(Task.CompletedTask);
Do(Task.CompletedTask);
Do(Task.CompletedTask);
Do(Task.CompletedTask);
Do(Task.CompletedTask);
Do(Task.CompletedTask);
Do(Task.CompletedTask);
Do(Task.CompletedTask);
Do(Task.CompletedTask);
Do(Task.CompletedTask);
Do(Task.CompletedTask);
}
}
""";

var workspaceXml =
$"""
<Workspace>
<Project Language="C#" CommonReferences="true" AssemblyName="CSProj1">
<Document FilePath="C:\C.cs">{markup}</Document>
</Project>
<Project Language="C#" CommonReferences="true" AssemblyName="CSProj2">
<Document IsLinkFile="true" LinkFilePath="C:\C.cs" LinkAssemblyName="CSProj1"></Document>
</Project>
</Workspace>
""";

await using var testLspServer = await CreateXmlTestLspServerAsync(workspaceXml, mutatingLspWorkspace, initializationOptions: new InitializationOptions
{
ClientCapabilities = new LSP.ClientCapabilities()
});

await WaitForWorkspaceOperationsAsync(testLspServer.TestWorkspace);

var results = await FindAllReferencesHandlerTests.RunFindAllReferencesNonVSAsync(testLspServer, testLspServer.GetLocations("caret").First());
Assert.Equal(46, results.Length);
}
}
Loading