Skip to content

Commit f9bd440

Browse files
dibarbetdibarbet
authored
Merge pull request #67625 from dibarbet/include_syntactic_always
Always include syntactic classifications in semantic response
2 parents 8afa157 + 79c516e commit f9bd440

File tree

4 files changed

+24
-103
lines changed

4 files changed

+24
-103
lines changed

‎src/Features/LanguageServer/Protocol/Handler/SemanticTokens/SemanticTokensHelpers.cs‎

Lines changed: 14 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,6 @@ internal static async Task<int[]> ComputeSemanticTokensDataAsync(
7575
Dictionary<string, int> tokenTypesToIndex,
7676
LSP.Range? range,
7777
ClassificationOptions options,
78-
bool includeSyntacticClassifications,
7978
CancellationToken cancellationToken)
8079
{
8180
var root = await document.GetRequiredSyntaxRootAsync(cancellationToken).ConfigureAwait(false);
@@ -86,7 +85,7 @@ internal static async Task<int[]> ComputeSemanticTokensDataAsync(
8685
var textSpan = range is null ? root.FullSpan : ProtocolConversions.RangeToTextSpan(range, text);
8786

8887
var classifiedSpans = await GetClassifiedSpansForDocumentAsync(
89-
document, textSpan, options, includeSyntacticClassifications, cancellationToken).ConfigureAwait(false);
88+
document, textSpan, options, cancellationToken).ConfigureAwait(false);
9089

9190
// Multi-line tokens are not supported by VS (tracked by https://devdiv.visualstudio.com/DevDiv/_workitems/edit/1265495).
9291
// Roslyn's classifier however can return multi-line classified spans, so we must break these up into single-line spans.
@@ -101,41 +100,24 @@ private static async Task<ClassifiedSpan[]> GetClassifiedSpansForDocumentAsync(
101100
Document document,
102101
TextSpan textSpan,
103102
ClassificationOptions options,
104-
bool includeSyntacticClassifications,
105103
CancellationToken cancellationToken)
106104
{
107105
var classificationService = document.GetRequiredLanguageService<IClassificationService>();
108106
using var _ = ArrayBuilder<ClassifiedSpan>.GetInstance(out var classifiedSpans);
109107

110-
// Case 1 - Generated Razor documents:
111-
// In Razor, the C# syntax classifier does not run on the client. This means we need to return both
112-
// syntactic and semantic classifications.
113-
// Case 2 - C# and VB documents:
114-
// In C#/VB, the syntax classifier runs on the client. This means we only need to return semantic
115-
// classifications.
116-
//
117-
// Ideally, Razor will eventually run the classifier on their end so we can get rid of this special
118-
// casing: https://github.com/dotnet/razor-tooling/issues/5850
119-
if (includeSyntacticClassifications)
120-
{
121-
// `includeAdditiveSpans` will add token modifiers such as 'static', which we want to include in LSP.
122-
var spans = await ClassifierHelper.GetClassifiedSpansAsync(
123-
document, textSpan, options, includeAdditiveSpans: true, cancellationToken).ConfigureAwait(false);
124-
125-
// The spans returned to us may include some empty spans, which we don't care about. We also don't care
126-
// about the 'text' classification. It's added for everything between real classifications (including
127-
// whitespace), and just means 'don't classify this'. No need for us to actually include that in
128-
// semantic tokens as it just wastes space in the result.
129-
var nonEmptySpans = spans.Where(s => !s.TextSpan.IsEmpty && s.ClassificationType != ClassificationTypeNames.Text);
130-
classifiedSpans.AddRange(nonEmptySpans);
131-
}
132-
else
133-
{
134-
await classificationService.AddSemanticClassificationsAsync(
135-
document, textSpan, options, classifiedSpans, cancellationToken).ConfigureAwait(false);
136-
await classificationService.AddEmbeddedLanguageClassificationsAsync(
137-
document, textSpan, options, classifiedSpans, cancellationToken).ConfigureAwait(false);
138-
}
108+
// We always return both syntactic and semantic classifications. If there is a syntactic classifier running on the client
109+
// then the semantic token classifications will override them.
110+
111+
// `includeAdditiveSpans` will add token modifiers such as 'static', which we want to include in LSP.
112+
var spans = await ClassifierHelper.GetClassifiedSpansAsync(
113+
document, textSpan, options, includeAdditiveSpans: true, cancellationToken).ConfigureAwait(false);
114+
115+
// The spans returned to us may include some empty spans, which we don't care about. We also don't care
116+
// about the 'text' classification. It's added for everything between real classifications (including
117+
// whitespace), and just means 'don't classify this'. No need for us to actually include that in
118+
// semantic tokens as it just wastes space in the result.
119+
var nonEmptySpans = spans.Where(s => !s.TextSpan.IsEmpty && s.ClassificationType != ClassificationTypeNames.Text);
120+
classifiedSpans.AddRange(nonEmptySpans);
139121

140122
// Classified spans are not guaranteed to be returned in a certain order so we sort them to be safe.
141123
classifiedSpans.Sort(ClassifiedSpanComparer.Instance);

‎src/Features/LanguageServer/Protocol/Handler/SemanticTokens/SemanticTokensRangeHandler.cs‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,6 @@ public TextDocumentIdentifier GetTextDocumentIdentifier(LSP.SemanticTokensRangeP
6060
SemanticTokensHelpers.TokenTypeToIndex,
6161
request.Range,
6262
options,
63-
includeSyntacticClassifications: contextDocument.IsRazorDocument(),
6463
cancellationToken).ConfigureAwait(false);
6564

6665
// The above call to get semantic tokens may be inaccurate (because we use frozen partial semantics). Kick

‎src/Features/LanguageServer/ProtocolUnitTests/SemanticTokens/SemanticTokensRangeTests.cs‎

Lines changed: 8 additions & 67 deletions
Original file line numberDiff line numberDiff line change
@@ -38,32 +38,6 @@ static class C { }
3838
var range = new LSP.Range { Start = new Position(0, 0), End = new Position(2, 0) };
3939
var results = await RunGetSemanticTokensRangeAsync(testLspServer, testLspServer.GetLocations("caret").First(), range);
4040

41-
// Everything is colorized syntactically, so we shouldn't be returning any semantic results.
42-
var expectedResults = new LSP.SemanticTokens
43-
{
44-
Data = Array.Empty<int>()
45-
};
46-
47-
Assert.Equal(expectedResults.Data, results.Data);
48-
}
49-
50-
[Theory, CombinatorialData]
51-
public async Task TestGetSemanticTokensRange_FullDoc_RazorAsync(bool mutatingLspWorkspace)
52-
{
53-
// Razor docs should be returning semantic + syntactic reuslts.
54-
var markup =
55-
@"{|caret:|}// Comment
56-
static class C { }
57-
";
58-
await using var testLspServer = await CreateTestLspServerAsync(markup, mutatingLspWorkspace);
59-
60-
var document = testLspServer.GetCurrentSolution().Projects.First().Documents.First();
61-
var range = new LSP.Range { Start = new Position(0, 0), End = new Position(2, 0) };
62-
var options = ClassificationOptions.Default;
63-
64-
var results = await SemanticTokensHelpers.ComputeSemanticTokensDataAsync(
65-
document, SemanticTokensHelpers.TokenTypeToIndex, range, options, includeSyntacticClassifications: true, CancellationToken.None);
66-
6741
var expectedResults = new LSP.SemanticTokens
6842
{
6943
Data = new int[]
@@ -78,12 +52,12 @@ static class C { }
7852
},
7953
};
8054

81-
await VerifyBasicInvariantsAndNoMultiLineTokens(testLspServer, results).ConfigureAwait(false);
82-
AssertEx.Equal(ConvertToReadableFormat(expectedResults.Data), ConvertToReadableFormat(results));
55+
await VerifyBasicInvariantsAndNoMultiLineTokens(testLspServer, results.Data).ConfigureAwait(false);
56+
AssertEx.Equal(ConvertToReadableFormat(expectedResults.Data), ConvertToReadableFormat(results.Data));
8357
}
8458

8559
[Theory, CombinatorialData]
86-
public async Task TestGetSemanticTokensRange_PartialDoc_RazorAsync(bool mutatingLspWorkspace)
60+
public async Task TestGetSemanticTokensRange_PartialDocAsync(bool mutatingLspWorkspace)
8761
{
8862
// Razor docs should be returning semantic + syntactic reuslts.
8963
var markup =
@@ -96,7 +70,7 @@ static class C { }
9670
var range = new LSP.Range { Start = new Position(1, 0), End = new Position(2, 0) };
9771
var options = ClassificationOptions.Default;
9872
var results = await SemanticTokensHelpers.ComputeSemanticTokensDataAsync(
99-
document, SemanticTokensHelpers.TokenTypeToIndex, range, options, includeSyntacticClassifications: true, CancellationToken.None);
73+
document, SemanticTokensHelpers.TokenTypeToIndex, range, options, CancellationToken.None);
10074

10175
var expectedResults = new LSP.SemanticTokens
10276
{
@@ -131,7 +105,7 @@ public async Task TestGetSemanticTokensRange_MultiLineComment_IncludeSyntacticCl
131105
var range = new LSP.Range { Start = new Position(0, 0), End = new Position(4, 0) };
132106
var options = ClassificationOptions.Default;
133107
var results = await SemanticTokensHelpers.ComputeSemanticTokensDataAsync(
134-
document, SemanticTokensHelpers.TokenTypeToIndex, range, options, includeSyntacticClassifications: true, CancellationToken.None);
108+
document, SemanticTokensHelpers.TokenTypeToIndex, range, options, CancellationToken.None);
135109

136110
var expectedResults = new LSP.SemanticTokens
137111
{
@@ -152,39 +126,6 @@ public async Task TestGetSemanticTokensRange_MultiLineComment_IncludeSyntacticCl
152126
AssertEx.Equal(ConvertToReadableFormat(expectedResults.Data), ConvertToReadableFormat(results));
153127
}
154128

155-
[Theory, CombinatorialData]
156-
public async Task TestGetSemanticTokensRange_StringLiteralAsync(bool mutatingLspWorkspace)
157-
{
158-
var markup =
159-
@"{|caret:|}class C
160-
{
161-
void M()
162-
{
163-
var x = @""one
164-
two """"
165-
three"";
166-
}
167-
}
168-
";
169-
170-
await using var testLspServer = await CreateTestLspServerAsync(markup, mutatingLspWorkspace);
171-
var range = new LSP.Range { Start = new Position(0, 0), End = new Position(9, 0) };
172-
var results = await RunGetSemanticTokensRangeAsync(testLspServer, testLspServer.GetLocations("caret").First(), range);
173-
174-
var expectedResults = new LSP.SemanticTokens
175-
{
176-
Data = new int[]
177-
{
178-
// Line | Char | Len | Token type | Modifier
179-
4, 8, 3, SemanticTokensHelpers.TokenTypeToIndex[ClassificationTypeNames.Keyword], 0, // 'var'
180-
1, 4, 2, SemanticTokensHelpers.TokenTypeToIndex[ClassificationTypeNames.StringEscapeCharacter], 0, // '""'
181-
},
182-
};
183-
184-
await VerifyBasicInvariantsAndNoMultiLineTokens(testLspServer, results.Data!).ConfigureAwait(false);
185-
AssertEx.Equal(ConvertToReadableFormat(expectedResults.Data), ConvertToReadableFormat(results.Data));
186-
}
187-
188129
[Theory, CombinatorialData]
189130
public async Task TestGetSemanticTokensRange_StringLiteral_IncludeSyntacticClassificationsAsync(bool mutatingLspWorkspace)
190131
{
@@ -206,7 +147,7 @@ void M()
206147
var range = new LSP.Range { Start = new Position(0, 0), End = new Position(9, 0) };
207148
var options = ClassificationOptions.Default;
208149
var results = await SemanticTokensHelpers.ComputeSemanticTokensDataAsync(
209-
document, SemanticTokensHelpers.TokenTypeToIndex, range, options, includeSyntacticClassifications: true, CancellationToken.None);
150+
document, SemanticTokensHelpers.TokenTypeToIndex, range, options, CancellationToken.None);
210151

211152
var expectedResults = new LSP.SemanticTokens
212153
{
@@ -259,7 +200,7 @@ void M()
259200
var range = new LSP.Range { Start = new Position(0, 0), End = new Position(9, 0) };
260201
var options = ClassificationOptions.Default;
261202
var results = await SemanticTokensHelpers.ComputeSemanticTokensDataAsync(
262-
document, SemanticTokensHelpers.TokenTypeToIndex, range, options, includeSyntacticClassifications: true, CancellationToken.None);
203+
document, SemanticTokensHelpers.TokenTypeToIndex, range, options, CancellationToken.None);
263204

264205
var expectedResults = new LSP.SemanticTokens
265206
{
@@ -326,7 +267,7 @@ void M()
326267
var document = testLspServer.GetCurrentSolution().Projects.First().Documents.First();
327268
var options = ClassificationOptions.Default;
328269
var results = await SemanticTokensHelpers.ComputeSemanticTokensDataAsync(
329-
document, SemanticTokensHelpers.TokenTypeToIndex, range: null, options, includeSyntacticClassifications: true, CancellationToken.None);
270+
document, SemanticTokensHelpers.TokenTypeToIndex, range: null, options: options, cancellationToken: CancellationToken.None);
330271

331272
var expectedResults = new LSP.SemanticTokens
332273
{

‎src/Features/Lsif/Generator/Generator.cs‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -445,9 +445,8 @@ private static async Task GenerateSemanticTokensAsync(
445445
document,
446446
SemanticTokensHelpers.TokenTypeToIndex,
447447
range: null,
448-
Classification.ClassificationOptions.Default,
449-
includeSyntacticClassifications: true,
450-
CancellationToken.None);
448+
options: Classification.ClassificationOptions.Default,
449+
cancellationToken: CancellationToken.None);
451450

452451
var semanticTokensResult = new SemanticTokensResult(new SemanticTokens { Data = data }, idFactory);
453452
var semanticTokensEdge = Edge.Create(Methods.TextDocumentSemanticTokensFullName, documentVertex.GetId(), semanticTokensResult.GetId(), idFactory);

0 commit comments

Comments
 (0)