Skip to content

Fix embedded language classification inside multi-line string #78588

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

Merged
merged 1 commit into from
May 15, 2025
Merged

Fix embedded language classification inside multi-line string #78588

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
25 changes: 6 additions & 19 deletions src/LanguageServer/Protocol/Handler/SemanticTokens/SemanticTokensHelpers.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,13 +97,15 @@ public static async Task<int[]> ComputeSemanticTokensDataAsync(
await GetClassifiedSpansForDocumentAsync(
classifiedSpans, document, textSpans, options, cancellationToken).ConfigureAwait(false);

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

// Multi-line tokens are not supported by VS (tracked by https://devdiv.visualstudio.com/DevDiv/_workitems/edit/1265495).
// Roslyn's classifier however can return multi-line classified spans, so we must break these up into single-line spans.
ConvertMultiLineToSingleLineSpans(text, classifiedSpans, updatedClassifiedSpans);

// Classified spans are not guaranteed to be returned in a certain order and
// converting multi-line spans to single line spans can change put spans in the wrong order.
// Sort them before we compute the tokens to ensure we return them to the client in the correct order.
updatedClassifiedSpans.Sort(ClassifiedSpanComparer.Instance);

// TO-DO: We should implement support for streaming if LSP adds support for it:
// https://devdiv.visualstudio.com/DevDiv/_workitems/edit/1276300
return ComputeTokens(text.Lines, updatedClassifiedSpans, supportsVisualStudioExtensions, tokenTypesToIndex);
Expand Down Expand Up @@ -146,7 +148,7 @@ private static void ConvertMultiLineToSingleLineSpans(SourceText text, Segmented
if (startLine != endLine)
{
ConvertToSingleLineSpan(
text, classifiedSpans, updatedClassifiedSpans, ref spanIndex, span.ClassificationType,
text, updatedClassifiedSpans, span.ClassificationType,
startLine, startOffset, endLine, endOffSet);
}
else
Expand All @@ -158,9 +160,7 @@ private static void ConvertMultiLineToSingleLineSpans(SourceText text, Segmented

static void ConvertToSingleLineSpan(
SourceText text,
SegmentedList<ClassifiedSpan> originalClassifiedSpans,
SegmentedList<ClassifiedSpan> updatedClassifiedSpans,
ref int spanIndex,
string classificationType,
int startLine,
int startOffset,
Expand Down Expand Up @@ -202,19 +202,6 @@ static void ConvertToSingleLineSpan(
var updatedClassifiedSpan = new ClassifiedSpan(textSpan, classificationType);
updatedClassifiedSpans.Add(updatedClassifiedSpan);
}

// Since spans are expected to be ordered, when breaking up a multi-line span, we may have to insert
// other spans in-between. For example, we may encounter this case when breaking up a multi-line verbatim
// string literal containing escape characters:
// var x = @"one ""
// two";
// The check below ensures we correctly return the spans in the correct order, i.e. 'one', '""', 'two'.
while (spanIndex + 1 < originalClassifiedSpans.Count &&
textSpan.Contains(originalClassifiedSpans[spanIndex + 1].TextSpan))
Copy link
Member Author

@dibarbet dibarbet May 15, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this was buggy when there was a multi-line classification that had another multi-line classification inside of it. For example, in

// lang=c#-test
const string Code = @"
class C {
    //
}";

the entire (multi-line) string is classified as roslyn-test code, but there are also verbatim string classifications for all the whitespace in between, which also spans multiple lines (e.g from after the { to before the //).

As we create new spans line by line for roslyn-test classification, this code tries to insert classifications that are on the same line into the set, before it proceeds to the next line (to keep everything ordered). But that only happens for single line spans. If there is a multi-line span inside the one we're breaking up, this exits and does not add it until after the entire original multi-line is added, causing the spans to be out of order.

Instead of having this complexity, we can just break up the multi-line spans and sort the set afterwards.

{
updatedClassifiedSpans.Add(originalClassifiedSpans[spanIndex + 1]);
spanIndex++;
}
}
}
}
Expand Down
10 changes: 5 additions & 5 deletions src/LanguageServer/ProtocolUnitTests/SemanticTokens/AbstractSemanticTokensTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,10 +35,10 @@ private protected static IReadOnlyDictionary<string, int> GetTokenTypeToIndex(Te
return result;
}

private protected static async Task<LSP.SemanticTokens> RunGetSemanticTokensRangeAsync(TestLspServer testLspServer, LSP.Location caret, LSP.Range range)
private protected static async Task<LSP.SemanticTokens> RunGetSemanticTokensRangeAsync(TestLspServer testLspServer, LSP.Location location)
{
var result = await testLspServer.ExecuteRequestAsync<LSP.SemanticTokensRangeParams, LSP.SemanticTokens>(LSP.Methods.TextDocumentSemanticTokensRangeName,
CreateSemanticTokensRangeParams(caret, range), CancellationToken.None);
CreateSemanticTokensRangeParams(location), CancellationToken.None);
Contract.ThrowIfNull(result);
return result;
}
Expand All @@ -57,11 +57,11 @@ private static LSP.SemanticTokensFullParams CreateSemanticTokensFullParams(LSP.L
TextDocument = new LSP.TextDocumentIdentifier { DocumentUri = caret.DocumentUri }
};

private static LSP.SemanticTokensRangeParams CreateSemanticTokensRangeParams(LSP.Location caret, LSP.Range range)
private static LSP.SemanticTokensRangeParams CreateSemanticTokensRangeParams(LSP.Location location)
=> new LSP.SemanticTokensRangeParams
{
TextDocument = new LSP.TextDocumentIdentifier { DocumentUri = caret.DocumentUri },
Range = range
TextDocument = new LSP.TextDocumentIdentifier { DocumentUri = location.DocumentUri },
Range = location.Range
};

private static SemanticTokensRangesParams CreateSemanticTokensRangesParams(LSP.Location caret, Range[] ranges)
Expand Down
92 changes: 88 additions & 4 deletions src/LanguageServer/ProtocolUnitTests/SemanticTokens/SemanticTokensRangeTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,15 +27,14 @@ public async Task TestGetSemanticTokensRange_FullDocAsync(bool mutatingLspWorksp
{
var markup =
"""
{|caret:|}// Comment
static class C { }
{|range:// Comment
static class C { }|}

""";
await using var testLspServer = await CreateTestLspServerAsync(
markup, mutatingLspWorkspace, GetCapabilities(isVS));

var range = new LSP.Range { Start = new Position(0, 0), End = new Position(2, 0) };
var results = await RunGetSemanticTokensRangeAsync(testLspServer, testLspServer.GetLocations("caret").First(), range);
var results = await RunGetSemanticTokensRangeAsync(testLspServer, testLspServer.GetLocations("range").First());

var expectedResults = new LSP.SemanticTokens();
var tokenTypeToIndex = GetTokenTypeToIndex(testLspServer);
Expand Down Expand Up @@ -605,4 +604,89 @@ public void TestGetSemanticTokensRange_AssertCustomTokenTypes(bool isVS)
Assert.True(schema.AllTokenTypes.Contains(tokenName));
}
}

[Theory, CombinatorialData]
[WorkItem("https://github.com/dotnet/roslyn/issues/74809")]
public async Task TestGetSemanticTokensRange_EmbeddedClassificationVerbatimString(bool mutatingLspWorkspace, bool isVS)
{
var markup =
"""
public class C
{
public void M()
{
{|range:// lang=c#-test
const string Code = @"
class C {
//
}";|}
}
}
""";
await using var testLspServer = await CreateTestLspServerAsync(
markup, mutatingLspWorkspace, GetCapabilities(isVS));

var location = testLspServer.GetLocations("range").Single();
var results = await RunGetSemanticTokensRangeAsync(testLspServer, location);

var expectedResults = new LSP.SemanticTokens();
var tokenTypeToIndex = GetTokenTypeToIndex(testLspServer);
if (isVS)
{
expectedResults.Data =
[
// Line | Char | Len | Token type | Modifier
4, 8, 15, tokenTypeToIndex[SemanticTokenTypes.Comment], 0,
1, 8, 5, tokenTypeToIndex[SemanticTokenTypes.Keyword], 0,
0, 6, 6, tokenTypeToIndex[SemanticTokenTypes.Keyword], 0,
0, 7, 4, tokenTypeToIndex[ClassificationTypeNames.ConstantName], 1,
0, 5, 1, tokenTypeToIndex[SemanticTokenTypes.Operator], 0,
0, 2, 2, tokenTypeToIndex[ClassificationTypeNames.VerbatimStringLiteral],0,
1, 0, 17, tokenTypeToIndex[SemanticTokenTypes.Namespace], 0,
0, 8, 5, tokenTypeToIndex[SemanticTokenTypes.Keyword], 0,
0, 5, 1, tokenTypeToIndex[ClassificationTypeNames.VerbatimStringLiteral],0,
0, 1, 1, tokenTypeToIndex[ClassificationTypeNames.ClassName], 0,
0, 1, 1, tokenTypeToIndex[ClassificationTypeNames.VerbatimStringLiteral],0,
0, 1, 1, tokenTypeToIndex[ClassificationTypeNames.Punctuation], 0,
1, 0, 12, tokenTypeToIndex[ClassificationTypeNames.VerbatimStringLiteral],0,
0, 0, 14, tokenTypeToIndex[SemanticTokenTypes.Namespace], 0,
0, 12, 2, tokenTypeToIndex[SemanticTokenTypes.Comment], 0,
1, 0, 8, tokenTypeToIndex[ClassificationTypeNames.VerbatimStringLiteral],0,
0, 0, 9, tokenTypeToIndex[SemanticTokenTypes.Namespace], 0,
0, 8, 1, tokenTypeToIndex[ClassificationTypeNames.Punctuation], 0,
0, 1, 1, tokenTypeToIndex[ClassificationTypeNames.VerbatimStringLiteral],0,
0, 1, 1, tokenTypeToIndex[ClassificationTypeNames.Punctuation], 0,
];
}
else
{
expectedResults.Data =
[
// Line | Char | Len | Token type | Modifier
4, 8, 15, tokenTypeToIndex[SemanticTokenTypes.Comment], 0,
1, 8, 5, tokenTypeToIndex[SemanticTokenTypes.Keyword], 0,
0, 6, 6, tokenTypeToIndex[SemanticTokenTypes.Keyword], 0,
0, 7, 4, tokenTypeToIndex[CustomLspSemanticTokenNames.ConstantName], 1,
0, 5, 1, tokenTypeToIndex[SemanticTokenTypes.Operator], 0,
0, 2, 2, tokenTypeToIndex[CustomLspSemanticTokenNames.StringVerbatim],0,
1, 0, 17, tokenTypeToIndex[SemanticTokenTypes.Namespace], 0,
0, 8, 5, tokenTypeToIndex[SemanticTokenTypes.Keyword], 0,
0, 5, 1, tokenTypeToIndex[CustomLspSemanticTokenNames.StringVerbatim],0,
0, 1, 1, tokenTypeToIndex[SemanticTokenTypes.Class], 0,
0, 1, 1, tokenTypeToIndex[CustomLspSemanticTokenNames.StringVerbatim],0,
0, 1, 1, tokenTypeToIndex[ClassificationTypeNames.Punctuation], 0,
1, 0, 12, tokenTypeToIndex[CustomLspSemanticTokenNames.StringVerbatim],0,
0, 0, 14, tokenTypeToIndex[SemanticTokenTypes.Namespace], 0,
0, 12, 2, tokenTypeToIndex[SemanticTokenTypes.Comment], 0,
1, 0, 8, tokenTypeToIndex[CustomLspSemanticTokenNames.StringVerbatim],0,
0, 0, 9, tokenTypeToIndex[SemanticTokenTypes.Namespace], 0,
0, 8, 1, tokenTypeToIndex[ClassificationTypeNames.Punctuation], 0,
0, 1, 1, tokenTypeToIndex[CustomLspSemanticTokenNames.StringVerbatim],0,
0, 1, 1, tokenTypeToIndex[ClassificationTypeNames.Punctuation], 0,
];
}

await VerifyBasicInvariantsAndNoMultiLineTokens(testLspServer, results.Data).ConfigureAwait(false);
AssertEx.Equal(ConvertToReadableFormat(testLspServer.ClientCapabilities, expectedResults.Data), ConvertToReadableFormat(testLspServer.ClientCapabilities, results.Data));
}
}
Loading