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

Jump to bottom

[LSP] Remove isFinalized from semantic tokens logic #60322

Closed
wants to merge 2 commits into from
Closed

[LSP] Remove isFinalized from semantic tokens logic #60322

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f5d94e7
Remove isFinalized logic
allisonchou Mar 22, 2022
5ad0d5b
Remove unnecessary using
allisonchou Mar 22, 2022
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
26 changes: 0 additions & 26 deletions src/Features/LanguageServer/Protocol/Handler/SemanticTokens/RoslynSemanticTokens.cs
View file
Open in desktop

This file was deleted.

14 changes: 8 additions & 6 deletions src/Features/LanguageServer/Protocol/Handler/SemanticTokens/SemanticTokensHelpers.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ static SemanticTokensHelpers()
/// <summary>
/// Returns the semantic tokens data for a given document with an optional range.
/// </summary>
internal static async Task<(int[], bool isFinalized)> ComputeSemanticTokensDataAsync(
internal static async Task<int[]> ComputeSemanticTokensDataAsync(
Document document,
Dictionary<string, int> tokenTypesToIndex,
LSP.Range? range,
Expand All @@ -147,10 +147,12 @@ static SemanticTokensHelpers()

// If the full compilation is not yet available, we'll try getting a partial one. It may contain inaccurate
// results but will speed up how quickly we can respond to the client's request.
var frozenDocument = document.WithFrozenPartialSemantics(cancellationToken);
var semanticModel = await frozenDocument.GetRequiredSemanticModelAsync(cancellationToken).ConfigureAwait(false);
var isFinalized = document.Project.TryGetCompilation(out var compilation) && compilation == semanticModel.Compilation;
Copy link
Member

@dibarbet dibarbet Mar 22, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

is this because of the issue where the compilation out of proc is not necessarily finalized when the compilation in proc is? Or is checking for the compilation here just not correct?

Is this not an issue for regular C# tagger too @CyrusNajmabadi ? It looks like we use the compilation available event there as well, which I presume is coming from the in-proc version - https://sourceroslyn.io/#Microsoft.CodeAnalysis.EditorFeatures/Classification/Semantic/AbstractSemanticOrEmbeddedClassificationViewTaggerProvider.cs,65

document = frozenDocument;
// Razor cannot use frozen partial semantics as there is currently no way to know whether we have full semantics
// available, which is an impediment to Razor caching.
if (!document.IsRazorDocument())
{
document = document.WithFrozenPartialSemantics(cancellationToken);
}

var classifiedSpans = await GetClassifiedSpansForDocumentAsync(
document, textSpan, options, includeSyntacticClassifications, cancellationToken).ConfigureAwait(false);
Expand All @@ -161,7 +163,7 @@ static SemanticTokensHelpers()

// 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, tokenTypesToIndex), isFinalized);
return ComputeTokens(text.Lines, updatedClassifiedSpans, tokenTypesToIndex);
}

private static async Task<ClassifiedSpan[]> GetClassifiedSpansForDocumentAsync(
Expand Down
4 changes: 2 additions & 2 deletions src/Features/LanguageServer/Protocol/Handler/SemanticTokens/SemanticTokensRangeHandler.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,15 +54,15 @@ public SemanticTokensRangeHandler(IGlobalOptionService globalOptions)
// partial token results. In addition, a range request is only ever called with a whole
// document request, so caching range results is unnecessary since the whole document
// handler will cache the results anyway.
var (tokensData, isFinalized) = await SemanticTokensHelpers.ComputeSemanticTokensDataAsync(
var tokensData = await SemanticTokensHelpers.ComputeSemanticTokensDataAsync(
context.Document,
SemanticTokensHelpers.TokenTypeToIndex,
request.Range,
options,
includeSyntacticClassifications: context.Document.IsRazorDocument(),
cancellationToken).ConfigureAwait(false);

return new RoslynSemanticTokens { Data = tokensData, IsFinalized = isFinalized };
return new LSP.SemanticTokens { Data = tokensData };
}
}
}
3 changes: 1 addition & 2 deletions src/Features/LanguageServer/ProtocolUnitTests/SemanticTokens/AbstractSemanticTokensTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
using System.Threading;
using System.Threading.Tasks;
using Microsoft.CodeAnalysis.Editor.UnitTests.Workspaces;
using Microsoft.CodeAnalysis.LanguageServer.Handler.SemanticTokens;
using Microsoft.CodeAnalysis.Text;
using Roslyn.Test.Utilities;
using Roslyn.Utilities;
Expand All @@ -21,7 +20,7 @@ public abstract class AbstractSemanticTokensTests : AbstractLanguageServerProtoc
{
private protected static async Task<LSP.SemanticTokens> RunGetSemanticTokensRangeAsync(TestLspServer testLspServer, LSP.Location caret, LSP.Range range)
{
var result = await testLspServer.ExecuteRequestAsync<LSP.SemanticTokensRangeParams, RoslynSemanticTokens>(LSP.Methods.TextDocumentSemanticTokensRangeName,
var result = await testLspServer.ExecuteRequestAsync<LSP.SemanticTokensRangeParams, LSP.SemanticTokens>(LSP.Methods.TextDocumentSemanticTokensRangeName,
CreateSemanticTokensRangeParams(caret, range), CancellationToken.None);
Contract.ThrowIfNull(result);
return result;
Expand Down
10 changes: 5 additions & 5 deletions src/Features/LanguageServer/ProtocolUnitTests/SemanticTokens/SemanticTokensRangeTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ static class C { }
var range = new LSP.Range { Start = new Position(0, 0), End = new Position(2, 0) };
var options = ClassificationOptions.Default;

var (results, _) = await SemanticTokensHelpers.ComputeSemanticTokensDataAsync(
var results = await SemanticTokensHelpers.ComputeSemanticTokensDataAsync(
document, SemanticTokensHelpers.TokenTypeToIndex, range, options, includeSyntacticClassifications: true, CancellationToken.None);

var expectedResults = new LSP.SemanticTokens
Expand Down Expand Up @@ -87,7 +87,7 @@ static class C { }
var document = testLspServer.GetCurrentSolution().Projects.First().Documents.First();
var range = new LSP.Range { Start = new Position(1, 0), End = new Position(2, 0) };
var options = ClassificationOptions.Default;
var (results, _) = await SemanticTokensHelpers.ComputeSemanticTokensDataAsync(
var results = await SemanticTokensHelpers.ComputeSemanticTokensDataAsync(
document, SemanticTokensHelpers.TokenTypeToIndex, range, options, includeSyntacticClassifications: true, CancellationToken.None);

var expectedResults = new LSP.SemanticTokens
Expand Down Expand Up @@ -122,7 +122,7 @@ public async Task TestGetSemanticTokensRange_MultiLineComment_RazorAsync()
var document = testLspServer.GetCurrentSolution().Projects.First().Documents.First();
var range = new LSP.Range { Start = new Position(0, 0), End = new Position(4, 0) };
var options = ClassificationOptions.Default;
var (results, _) = await SemanticTokensHelpers.ComputeSemanticTokensDataAsync(
var results = await SemanticTokensHelpers.ComputeSemanticTokensDataAsync(
document, SemanticTokensHelpers.TokenTypeToIndex, range, options, includeSyntacticClassifications: true, CancellationToken.None);

var expectedResults = new LSP.SemanticTokens
Expand Down Expand Up @@ -197,7 +197,7 @@ void M()
var document = testLspServer.GetCurrentSolution().Projects.First().Documents.First();
var range = new LSP.Range { Start = new Position(0, 0), End = new Position(9, 0) };
var options = ClassificationOptions.Default;
var (results, _) = await SemanticTokensHelpers.ComputeSemanticTokensDataAsync(
var results = await SemanticTokensHelpers.ComputeSemanticTokensDataAsync(
document, SemanticTokensHelpers.TokenTypeToIndex, range, options, includeSyntacticClassifications: true, CancellationToken.None);

var expectedResults = new LSP.SemanticTokens
Expand Down Expand Up @@ -250,7 +250,7 @@ void M()
var document = testLspServer.GetCurrentSolution().Projects.First().Documents.First();
var range = new LSP.Range { Start = new Position(0, 0), End = new Position(9, 0) };
var options = ClassificationOptions.Default;
var (results, _) = await SemanticTokensHelpers.ComputeSemanticTokensDataAsync(
var results = await SemanticTokensHelpers.ComputeSemanticTokensDataAsync(
document, SemanticTokensHelpers.TokenTypeToIndex, range, options, includeSyntacticClassifications: true, CancellationToken.None);

var expectedResults = new LSP.SemanticTokens
Expand Down