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

Fix C# background color bug when using tab indentation #9257

Merged
merged 3 commits into from
Sep 9, 2023
Merged

Fix C# background color bug when using tab indentation #9257

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
ac33b67
Add failing test
davidwengier Sep 8, 2023
2dc21d4
Fix the bug, and update test baselines
davidwengier Sep 8, 2023
4155bcf
Logging tweaks
davidwengier Sep 8, 2023
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
6 changes: 5 additions & 1 deletion ...osoft.AspNetCore.Razor.LanguageServer/Semantic/Services/RazorSemanticTokensInfoService.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,10 @@ public RazorSemanticTokensInfoService(
{
csharpSemanticRanges = await GetCSharpSemanticRangesAsync(codeDocument, textDocumentIdentifier, range, razorSemanticTokensLegend, documentContext.Version, correlationId, cancellationToken).ConfigureAwait(false);
}
catch (OperationCanceledException)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Noticed in the logs we were reporting this as an issue, when its totally normal, and in fact it should probably happen a little more than it does now!

{
return null;
}
catch (Exception ex)
{
_logger.LogError(ex, "Error thrown while retrieving CSharp semantic range.");
Expand Down Expand Up @@ -230,7 +234,7 @@ private static bool ContainsOnlySpacesOrTabs(SourceText razorSource, int startIn
var end = startIndex + count;
for (var i = startIndex; i < end; i++)
{
if (razorSource[i] is not ' ' or '\t')
if (razorSource[i] is not (' ' or '\t'))
Copy link
Contributor

Choose a reason for hiding this comment

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

This is subtle, nice catch!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Would have been nicer to catch it when I wrote the code in the first place. Not the first time I've made this mistake either 🤦🏼‍♂️

{
return false;
}
Expand Down
3 changes: 3 additions & 0 deletions src/Razor/src/Microsoft.VisualStudio.LanguageServerClient.Razor/Endpoints/SemanticTokens.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
using Microsoft.AspNetCore.Razor.LanguageServer.Semantic;
using Microsoft.AspNetCore.Razor.LanguageServer.Semantic.Models;
using Microsoft.AspNetCore.Razor.PooledObjects;
using Microsoft.Extensions.Logging;
using Microsoft.VisualStudio.LanguageServer.ContainedLanguage;
using Microsoft.VisualStudio.LanguageServer.Protocol;
using StreamJsonRpc;
Expand Down Expand Up @@ -85,10 +86,12 @@ internal partial class RazorCustomMessageTarget
var nonEmptyResults = results.Select(r => r?.Response?.Data).WithoutNull().ToArray();
if (nonEmptyResults.Length != semanticTokensParams.Ranges.Length)
{
_logger?.LogDebug("Made {count} semantic tokens requests to Roslyn but only got {nonEmpty} results back", semanticTokensParams.Ranges.Length, nonEmptyResults.Length);
// Weren't able to re-invoke C# semantic tokens but we have to indicate it's due to out of sync by providing the old version
return new ProvideSemanticTokensResponse(tokens: null, hostDocumentSyncVersion: csharpDoc.HostDocumentSyncVersion ?? -1);
}

_logger?.LogDebug("Made {count} semantic tokens requests to Roslyn", semanticTokensParams.Ranges.Length);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Snuck this one in too, so we can see what is going on with our new feature flag.

return new ProvideSemanticTokensResponse(nonEmptyResults, semanticTokensParams.RequiredHostDocumentVersion);
}
}
28 changes: 26 additions & 2 deletions ...osoft.AspNetCore.Razor.LanguageServer.Test/Semantic/RazorSemanticTokensInfoServiceTest.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,11 @@
using System.Collections.Generic;
using System.Threading;
using System.Threading.Tasks;
using Microsoft.AspNetCore.Razor.Language;
using Microsoft.AspNetCore.Razor.LanguageServer.Common;
using Microsoft.AspNetCore.Razor.LanguageServer.Extensions;
using Microsoft.AspNetCore.Razor.Test.Common;
using Microsoft.CodeAnalysis.Razor.Workspaces;
using Microsoft.CodeAnalysis.Razor.Workspaces.Extensions;
using Microsoft.CodeAnalysis.Text;
using Microsoft.CommonLanguageServerProtocol.Framework;
using Microsoft.Extensions.Options;
using Microsoft.VisualStudio.LanguageServer.Protocol;
Expand Down Expand Up @@ -803,6 +801,32 @@ public void M()
Assert.NotEmpty(csharpTokens.Tokens);
}

[Fact]
public async Task GetSemanticTokens_CSharp_Tabs_Static_WithBackground()
{
var documentText = """
@using System
@code
{
private static bool
_isStatic;

public void M()
{
if (_isStatic)
{
}
}
}
""";

var razorRange = GetRange(documentText);
var csharpTokens = await GetCSharpSemanticTokensResponseAsync(documentText, razorRange, isRazorFile: true);
await AssertSemanticTokensAsync(documentText, isRazorFile: true, razorRange, csharpTokens: csharpTokens, withCSharpBackground: true);
Assert.NotNull(csharpTokens.Tokens);
Assert.NotEmpty(csharpTokens.Tokens);
}

[Fact]
public async Task GetSemanticTokens_CSharp_WithBackground()
{
Expand Down
40 changes: 40 additions & 0 deletions ...ticTokensInfoServiceTest/GetSemanticTokens_CSharp_Tabs_Static_WithBackground.semantic.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
//line,characterPos,length,tokenType,modifier,text
0 0 1 razorTransition 0 [@]
0 1 5 keyword 4 [using]
0 5 1 markupTextLiteral 4 [ ]
0 1 6 namespace name 4 [System]
1 0 1 razorTransition 0 [@]
0 1 4 razorDirective 0 [code]
1 0 1 razorTransition 0 [{]
1 0 1 markupTextLiteral 4 [ ]
0 1 7 keyword 4 [private]
0 7 1 markupTextLiteral 4 [ ]
0 1 6 keyword 4 [static]
0 6 1 markupTextLiteral 4 [ ]
0 1 4 keyword 4 [bool]
1 0 2 markupTextLiteral 4 [ ]
0 2 9 field name 5 [_isStatic]
0 9 1 punctuation 4 [;]
2 0 1 markupTextLiteral 4 [ ]
0 1 6 keyword 4 [public]
0 6 1 markupTextLiteral 4 [ ]
0 1 4 keyword 4 [void]
0 4 1 markupTextLiteral 4 [ ]
0 1 1 method name 4 [M]
0 1 1 punctuation 4 [(]
0 1 1 punctuation 4 [)]
1 0 1 markupTextLiteral 4 [ ]
0 1 1 punctuation 4 [{]
1 0 2 markupTextLiteral 4 [ ]
0 2 2 keyword - control 4 [if]
0 2 1 markupTextLiteral 4 [ ]
0 1 1 punctuation 4 [(]
0 1 9 field name 5 [_isStatic]
0 9 1 punctuation 4 [)]
1 0 2 markupTextLiteral 4 [ ]
0 2 1 punctuation 4 [{]
1 0 2 markupTextLiteral 4 [ ]
0 2 1 punctuation 4 [}]
1 0 1 markupTextLiteral 4 [ ]
0 1 1 punctuation 4 [}]
1 0 1 razorTransition 0 [}]
40 changes: 40 additions & 0 deletions ...ticTokensInfoServiceTest/GetSemanticTokens_CSharp_Tabs_Static_WithBackground.semantic.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
//line,characterPos,length,tokenType,modifier,text
0 0 1 razorTransition 0 [@]
0 1 5 keyword 4 [using]
0 5 1 markupTextLiteral 4 [ ]
0 1 6 namespace name 4 [System]
1 0 1 razorTransition 0 [@]
0 1 4 razorDirective 0 [code]
1 0 1 razorTransition 0 [{]
1 0 1 markupTextLiteral 4 [ ]
0 1 7 keyword 4 [private]
0 7 1 markupTextLiteral 4 [ ]
0 1 6 keyword 4 [static]
0 6 1 markupTextLiteral 4 [ ]
0 1 4 keyword 4 [bool]
1 0 2 markupTextLiteral 4 [ ]
0 2 9 field name 5 [_isStatic]
0 9 1 punctuation 4 [;]
2 0 1 markupTextLiteral 4 [ ]
0 1 6 keyword 4 [public]
0 6 1 markupTextLiteral 4 [ ]
0 1 4 keyword 4 [void]
0 4 1 markupTextLiteral 4 [ ]
0 1 1 method name 4 [M]
0 1 1 punctuation 4 [(]
0 1 1 punctuation 4 [)]
1 0 1 markupTextLiteral 4 [ ]
0 1 1 punctuation 4 [{]
1 0 2 markupTextLiteral 4 [ ]
0 2 2 keyword - control 4 [if]
0 2 1 markupTextLiteral 4 [ ]
0 1 1 punctuation 4 [(]
0 1 9 field name 5 [_isStatic]
0 9 1 punctuation 4 [)]
1 0 2 markupTextLiteral 4 [ ]
0 2 1 punctuation 4 [{]
1 0 2 markupTextLiteral 4 [ ]
0 2 1 punctuation 4 [}]
1 0 1 markupTextLiteral 4 [ ]
0 1 1 punctuation 4 [}]
1 0 1 razorTransition 0 [}]