-
Notifications
You must be signed in to change notification settings - Fork 4k
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
Send back icons for FAR via LSP #47882
Changes from 19 commits
fe82ec6
8b9f26d
d66e4e5
e30456b
3abf5c2
c703f1a
0b60e89
c5312b1
a805ec6
1213208
711e42d
b206219
9f10b94
20bf6f1
263c416
eab8465
e380fe6
51bd904
0f09454
9b05079
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -4,9 +4,13 @@ | |
|
||
#nullable disable | ||
|
||
using System; | ||
using System.Collections.Generic; | ||
using System.Linq; | ||
using System.Threading; | ||
using System.Threading.Tasks; | ||
using Microsoft.CodeAnalysis.Editor.Shared.Extensions; | ||
using Microsoft.VisualStudio.Text.Adornments; | ||
using Roslyn.Test.Utilities; | ||
using Xunit; | ||
using LSP = Microsoft.VisualStudio.LanguageServer.Protocol; | ||
|
@@ -45,7 +49,44 @@ void M2() | |
Assert.Equal("M", results[1].ContainingMember); | ||
Assert.Equal("M2", results[3].ContainingMember); | ||
|
||
AssertValidDefinitionProperties(results, 0); | ||
AssertValidDefinitionProperties(results, 0, Glyph.FieldPublic); | ||
} | ||
|
||
[WpfFact(Skip = "https://github.com/dotnet/roslyn/issues/43063")] | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. does this need to be skipped still? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I can't see the build results in the linked issue to see why they failed, but with the handler only supporting streaming I can't see how they ever passed, so was planning on unskipping when adding non-streaming support. |
||
public async Task TestFindAllReferencesAsync_Class() | ||
{ | ||
var markup = | ||
@"class {|reference:A|} | ||
{ | ||
public int someInt = 1; | ||
void M() | ||
{ | ||
var i = someInt + 1; | ||
} | ||
} | ||
class B | ||
{ | ||
int someInt = {|reference:A|}.someInt + 1; | ||
void M2() | ||
{ | ||
var j = someInt + {|caret:|}{|reference:A|}.someInt; | ||
} | ||
}"; | ||
using var workspace = CreateTestWorkspace(markup, out var locations); | ||
|
||
var results = await RunFindAllReferencesAsync(workspace.CurrentSolution, locations["caret"].First()); | ||
AssertLocationsEqual(locations["reference"], results.Select(result => result.Location)); | ||
|
||
var textElement = results[0].Text as ClassifiedTextElement; | ||
Assert.NotNull(textElement); | ||
var actualText = string.Concat(textElement.Runs.Select(r => r.Text)); | ||
|
||
Assert.Equal("class A", actualText); | ||
Assert.Equal("B", results[1].ContainingType); | ||
Assert.Equal("B", results[2].ContainingType); | ||
Assert.Equal("M2", results[2].ContainingMember); | ||
|
||
AssertValidDefinitionProperties(results, 0, Glyph.ClassInternal); | ||
} | ||
|
||
[WpfFact(Skip = "https://github.com/dotnet/roslyn/issues/43063")] | ||
|
@@ -80,7 +121,7 @@ void M2() | |
Assert.Equal("M", results[1].ContainingMember); | ||
Assert.Equal("M2", results[3].ContainingMember); | ||
|
||
AssertValidDefinitionProperties(results, 0); | ||
AssertValidDefinitionProperties(results, 0, Glyph.FieldPublic); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we want some test for something other than FieldPublic? |
||
} | ||
|
||
[WpfFact] | ||
|
@@ -116,12 +157,13 @@ void M() | |
Assert.NotNull(results[0].Location.Uri); | ||
} | ||
|
||
private static LSP.ReferenceParams CreateReferenceParams(LSP.Location caret) => | ||
private static LSP.ReferenceParams CreateReferenceParams(LSP.Location caret, IProgress<object> progress) => | ||
new LSP.ReferenceParams() | ||
{ | ||
TextDocument = CreateTextDocumentIdentifier(caret.Uri), | ||
Position = caret.Range.Start, | ||
Context = new LSP.ReferenceContext(), | ||
PartialResultToken = progress | ||
}; | ||
|
||
private static async Task<LSP.VSReferenceItem[]> RunFindAllReferencesAsync(Solution solution, LSP.Location caret) | ||
|
@@ -131,17 +173,23 @@ private static LSP.ReferenceParams CreateReferenceParams(LSP.Location caret) => | |
SupportsVisualStudioExtensions = true | ||
}; | ||
|
||
var progress = new ProgressCollector<LSP.VSReferenceItem>(); | ||
|
||
var queue = CreateRequestQueue(solution); | ||
return await GetLanguageServer(solution).ExecuteRequestAsync<LSP.ReferenceParams, LSP.VSReferenceItem[]>(queue, LSP.Methods.TextDocumentReferencesName, | ||
CreateReferenceParams(caret), vsClientCapabilities, null, CancellationToken.None); | ||
await GetLanguageServer(solution).ExecuteRequestAsync<LSP.ReferenceParams, LSP.VSReferenceItem[]>(queue, LSP.Methods.TextDocumentReferencesName, | ||
CreateReferenceParams(caret, progress), vsClientCapabilities, null, CancellationToken.None); | ||
|
||
return progress.GetItems(); | ||
} | ||
|
||
private static void AssertValidDefinitionProperties(LSP.ReferenceItem[] referenceItems, int definitionIndex) | ||
private static void AssertValidDefinitionProperties(LSP.VSReferenceItem[] referenceItems, int definitionIndex, Glyph definitionGlyph) | ||
{ | ||
var definition = referenceItems[definitionIndex]; | ||
var definitionId = definition.DefinitionId; | ||
Assert.NotNull(definition.DefinitionText); | ||
|
||
Assert.Equal(definitionGlyph.GetImageId(), definition.DefinitionIcon.ImageId); | ||
|
||
for (var i = 0; i < referenceItems.Length; i++) | ||
{ | ||
if (i == definitionIndex) | ||
|
@@ -150,9 +198,22 @@ private static void AssertValidDefinitionProperties(LSP.ReferenceItem[] referenc | |
} | ||
|
||
Assert.Null(referenceItems[i].DefinitionText); | ||
Assert.Equal(0, referenceItems[i].DefinitionIcon.ImageId.Id); | ||
Assert.Equal(definitionId, referenceItems[i].DefinitionId); | ||
Assert.NotEqual(definitionId, referenceItems[i].Id); | ||
} | ||
} | ||
|
||
private sealed class ProgressCollector<T> : IProgress<object> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Making a note here to switch this as well in #48350 |
||
{ | ||
private readonly List<T> _items = new List<T>(); | ||
|
||
public T[] GetItems() => _items.ToArray(); | ||
|
||
public void Report(object value) | ||
{ | ||
_items.AddRange((T[])value); | ||
} | ||
} | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Once pull diagnostics goes in we should switch this to use the progress implementation there.
Filed #48350
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I already logged #48150 (and #48151) for this :P
I started working on, but need to make changes to the BufferedProgress to make it thread safe, so yes, was waiting for pull diagnostics to be in first.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Woops, closed mine as dupe!