Skip to content

Commit 587c8aa

Browse files
JoeRobichJoeRobich
authored
Do not return metadata names for document symbols (#78077)
For EditorFeatures NavigationBar and non-hierarchy LSP DocumentSymbols we do not use the SymbolItem.Name and instead return SymbolItem.Text which is the symbol formatted as member name with parameters. The hierarchy DocumentSymbols did return the SymbolItem.Name which could contain values such as `.ctor`. The NavBarItemService is updated to construct SymbolItems with the symbol formatted as a simple member name instead. Resolves #78072
2 parents 4e5e39b + 60ca3b2 commit 587c8aa

File tree

2 files changed

+84
-49
lines changed

2 files changed

+84
-49
lines changed

‎src/Features/CSharp/Portable/NavigationBar/CSharpNavigationBarItemService.cs‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,10 @@ internal sealed class CSharpNavigationBarItemService() : AbstractNavigationBarIt
2828
private static readonly SymbolDisplayFormat s_typeFormat =
2929
SymbolDisplayFormat.CSharpErrorMessageFormat.AddGenericsOptions(SymbolDisplayGenericsOptions.IncludeVariance);
3030

31-
private static readonly SymbolDisplayFormat s_memberFormat =
31+
private static readonly SymbolDisplayFormat s_memberNameFormat =
32+
new(typeQualificationStyle: SymbolDisplayTypeQualificationStyle.NameOnly);
33+
34+
private static readonly SymbolDisplayFormat s_memberDetailsFormat =
3235
new(genericsOptions: SymbolDisplayGenericsOptions.IncludeTypeParameters,
3336
memberOptions: SymbolDisplayMemberOptions.IncludeParameters |
3437
SymbolDisplayMemberOptions.IncludeExplicitInterface,
@@ -186,8 +189,8 @@ private static bool IsAccessor(ISymbol member)
186189
return null;
187190

188191
return new SymbolItem(
189-
member.Name,
190-
member.ToDisplayString(s_memberFormat),
192+
member.ToDisplayString(s_memberNameFormat),
193+
member.ToDisplayString(s_memberDetailsFormat),
191194
member.GetGlyph(),
192195
member.IsObsolete(),
193196
location.Value);

‎src/LanguageServer/ProtocolUnitTests/Symbols/DocumentSymbolsTests.cs‎

Lines changed: 78 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,6 @@
22
// The .NET Foundation licenses this file to you under the MIT license.
33
// See the LICENSE file in the project root for more information.
44

5-
#nullable disable
6-
75
using System.Linq;
86
using System.Threading;
97
using System.Threading.Tasks;
@@ -24,12 +22,22 @@ public DocumentSymbolsTests(ITestOutputHelper testOutputHelper) : base(testOutpu
2422
public async Task TestGetDocumentSymbolsAsync(bool mutatingLspWorkspace)
2523
{
2624
var markup =
27-
@"{|class:class {|classSelection:A|}
28-
{
29-
{|method:void {|methodSelection:M|}()
30-
{
31-
}|}
32-
}|}";
25+
"""
26+
namespace Test;
27+
28+
{|class:class {|classSelection:A|}
29+
{
30+
{|constructor:public {|constructorSelection:A|}()
31+
{
32+
}|}
33+
34+
{|method:void {|methodSelection:M|}()
35+
{
36+
}|}
37+
38+
{|operator:static A {|operatorSelection:operator +|}(A a1, A a2) => a1;|}
39+
}|}
40+
""";
3341
var clientCapabilities = new LSP.ClientCapabilities()
3442
{
3543
TextDocument = new LSP.TextDocumentClientCapabilities()
@@ -41,13 +49,15 @@ public async Task TestGetDocumentSymbolsAsync(bool mutatingLspWorkspace)
4149
}
4250
};
4351
await using var testLspServer = await CreateTestLspServerAsync(markup, mutatingLspWorkspace, clientCapabilities);
44-
var expected = new LSP.DocumentSymbol[]
45-
{
46-
CreateDocumentSymbol(LSP.SymbolKind.Class, "A", "A", testLspServer.GetLocations("class").Single(), testLspServer.GetLocations("classSelection").Single())
47-
};
48-
CreateDocumentSymbol(LSP.SymbolKind.Method, "M", "M()", testLspServer.GetLocations("method").Single(), testLspServer.GetLocations("methodSelection").Single(), expected.First());
52+
var classSymbol = CreateDocumentSymbol(LSP.SymbolKind.Class, "A", "Test.A", testLspServer.GetLocations("class").Single(), testLspServer.GetLocations("classSelection").Single());
53+
var constructorSymbol = CreateDocumentSymbol(LSP.SymbolKind.Method, "A", "A()", testLspServer.GetLocations("constructor").Single(), testLspServer.GetLocations("constructorSelection").Single(), classSymbol);
54+
var methodSymbol = CreateDocumentSymbol(LSP.SymbolKind.Method, "M", "M()", testLspServer.GetLocations("method").Single(), testLspServer.GetLocations("methodSelection").Single(), classSymbol);
55+
var operatorSymbol = CreateDocumentSymbol(LSP.SymbolKind.Operator, "operator +", "operator +(A a1, A a2)", testLspServer.GetLocations("operator").Single(), testLspServer.GetLocations("operatorSelection").Single(), classSymbol);
56+
57+
LSP.DocumentSymbol[] expected = [classSymbol];
4958

5059
var results = await RunGetDocumentSymbolsAsync<LSP.DocumentSymbol[]>(testLspServer);
60+
Assert.NotNull(results);
5161
Assert.Equal(expected.Length, results.Length);
5262
for (var i = 0; i < results.Length; i++)
5363
{
@@ -59,18 +69,29 @@ public async Task TestGetDocumentSymbolsAsync(bool mutatingLspWorkspace)
5969
public async Task TestGetDocumentSymbolsAsync_WithoutHierarchicalSupport(bool mutatingLspWorkspace)
6070
{
6171
var markup =
62-
@"class {|class:A|}
63-
{
64-
void {|method:M|}()
65-
{
66-
}
67-
}";
72+
"""
73+
namespace Test;
74+
75+
class {|class:A|}
76+
{
77+
public {|constructor:A|}()
78+
{
79+
}
80+
81+
void {|method:M|}()
82+
{
83+
}
84+
85+
static A operator {|operator:+|}(A a1, A a2) => a1;
86+
}
87+
""";
6888
await using var testLspServer = await CreateTestLspServerAsync(markup, mutatingLspWorkspace, CapabilitiesWithVSExtensions);
69-
var expected = new LSP.SymbolInformation[]
70-
{
71-
CreateSymbolInformation(LSP.SymbolKind.Class, "A", testLspServer.GetLocations("class").Single(), Glyph.ClassInternal),
72-
CreateSymbolInformation(LSP.SymbolKind.Method, "M()", testLspServer.GetLocations("method").Single(), Glyph.MethodPrivate, "A")
73-
};
89+
LSP.SymbolInformation[] expected = [
90+
CreateSymbolInformation(LSP.SymbolKind.Class, "Test.A", testLspServer.GetLocations("class").Single(), Glyph.ClassInternal),
91+
CreateSymbolInformation(LSP.SymbolKind.Method, "A()", testLspServer.GetLocations("constructor").Single(), Glyph.MethodPublic, "Test.A"),
92+
CreateSymbolInformation(LSP.SymbolKind.Method, "M()", testLspServer.GetLocations("method").Single(), Glyph.MethodPrivate, "Test.A"),
93+
CreateSymbolInformation(LSP.SymbolKind.Operator, "operator +(A a1, A a2)", testLspServer.GetLocations("operator").Single(), Glyph.Operator, "Test.A"),
94+
];
7495

7596
var results = await RunGetDocumentSymbolsAsync<LSP.SymbolInformation[]>(testLspServer);
7697
AssertJsonEquals(expected, results);
@@ -82,29 +103,34 @@ public async Task TestGetDocumentSymbolsAsync_WithoutHierarchicalSupport(bool mu
82103
public async Task TestGetDocumentSymbolsAsync__WithLocals(bool mutatingLspWorkspace)
83104
{
84105
var markup =
85-
@"class A
86-
{
87-
void Method()
88-
{
89-
int i = 1;
90-
}
91-
}";
106+
"""
107+
class A
108+
{
109+
void Method()
110+
{
111+
int i = 1;
112+
}
113+
}
114+
""";
92115
await using var testLspServer = await CreateTestLspServerAsync(markup, mutatingLspWorkspace);
93116
var results = await RunGetDocumentSymbolsAsync<LSP.SymbolInformation[]>(testLspServer).ConfigureAwait(false);
117+
Assert.NotNull(results);
94118
Assert.Equal(3, results.Length);
95119
}
96120

97121
[Theory, CombinatorialData]
98122
public async Task TestGetDocumentSymbolsAsync_EmptyName(bool mutatingLspWorkspace)
99123
{
100124
var markup =
101-
@"namepsace NamespaceA
102-
{
103-
public class
104-
";
125+
"""
126+
namepsace NamespaceA
127+
{
128+
public class
129+
""";
105130

106131
await using var testLspServer = await CreateTestLspServerAsync(markup, mutatingLspWorkspace);
107132
var results = await RunGetDocumentSymbolsAsync<LSP.SymbolInformation[]>(testLspServer).ConfigureAwait(false);
133+
Assert.NotNull(results);
108134
#pragma warning disable CS0618 // Type or member is obsolete
109135
Assert.Equal(".", results.First().Name);
110136
#pragma warning restore CS0618
@@ -114,10 +140,11 @@ public class
114140
public async Task TestGetDocumentSymbolsAsync_EmptyNameWithHierarchicalSupport(bool mutatingLspWorkspace)
115141
{
116142
var markup =
117-
@"namepsace NamespaceA
118-
{
119-
public class
120-
";
143+
"""
144+
namepsace NamespaceA
145+
{
146+
public class
147+
""";
121148
var clientCapabilities = new LSP.ClientCapabilities()
122149
{
123150
TextDocument = new LSP.TextDocumentClientCapabilities()
@@ -132,8 +159,8 @@ public class
132159
await using var testLspServer = await CreateTestLspServerAsync(markup, mutatingLspWorkspace, clientCapabilities);
133160

134161
var results = await RunGetDocumentSymbolsAsync<LSP.SymbolInformation[]>(testLspServer).ConfigureAwait(false);
162+
Assert.NotNull(results);
135163
#pragma warning disable CS0618 // Type or member is obsolete
136-
137164
Assert.Equal(".", results.First().Name);
138165
#pragma warning restore CS0618
139166
}
@@ -144,10 +171,11 @@ public async Task TestGetDocumentSymbolsAsync__NoSymbols(bool mutatingLspWorkspa
144171
await using var testLspServer = await CreateTestLspServerAsync(string.Empty, mutatingLspWorkspace);
145172

146173
var results = await RunGetDocumentSymbolsAsync<LSP.SymbolInformation[]>(testLspServer);
174+
Assert.NotNull(results);
147175
Assert.Empty(results);
148176
}
149177

150-
private static async Task<TReturn> RunGetDocumentSymbolsAsync<TReturn>(TestLspServer testLspServer)
178+
private static async Task<TReturn?> RunGetDocumentSymbolsAsync<TReturn>(TestLspServer testLspServer)
151179
{
152180
var document = testLspServer.GetCurrentSolution().Projects.First().Documents.First();
153181
var request = new LSP.DocumentSymbolParams
@@ -163,16 +191,20 @@ private static void AssertDocumentSymbolEquals(LSP.DocumentSymbol expected, LSP.
163191
{
164192
Assert.Equal(expected.Kind, actual.Kind);
165193
Assert.Equal(expected.Name, actual.Name);
194+
Assert.Equal(expected.Detail, actual.Detail);
166195
Assert.Equal(expected.Range, actual.Range);
167-
Assert.Equal(expected.Children.Length, actual.Children.Length);
168-
for (var i = 0; i < actual.Children.Length; i++)
196+
Assert.Equal(expected.Children?.Length, actual.Children?.Length);
197+
if (expected.Children is not null)
169198
{
170-
AssertDocumentSymbolEquals(expected.Children[i], actual.Children[i]);
199+
for (var i = 0; i < actual.Children!.Length; i++)
200+
{
201+
AssertDocumentSymbolEquals(expected.Children[i], actual.Children[i]);
202+
}
171203
}
172204
}
173205

174206
private static LSP.DocumentSymbol CreateDocumentSymbol(LSP.SymbolKind kind, string name, string detail,
175-
LSP.Location location, LSP.Location selection, LSP.DocumentSymbol parent = null)
207+
LSP.Location location, LSP.Location selection, LSP.DocumentSymbol? parent = null)
176208
{
177209
var documentSymbol = new LSP.DocumentSymbol()
178210
{
@@ -189,7 +221,7 @@ private static LSP.DocumentSymbol CreateDocumentSymbol(LSP.SymbolKind kind, stri
189221

190222
if (parent != null)
191223
{
192-
var children = parent.Children.ToList();
224+
var children = parent.Children?.ToList() ?? [];
193225
children.Add(documentSymbol);
194226
parent.Children = [.. children];
195227
}

0 commit comments

Comments
 (0)