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.

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

Added Handler for Semantic Tokenization #1328

Merged
merged 64 commits into from
Jul 30, 2020
Merged
Show file tree
Hide file tree
Changes from 21 commits
Commits
Show all changes
64 commits
Select commit Hold shift + click to select a range
2790655
added basic semantic token support
Jul 16, 2020
04458e4
removed unnecessary imports
Jul 16, 2020
2e46016
removed unnecessary field
Jul 16, 2020
b3077cc
minor refactoring changes
Jul 17, 2020
f68e4e3
minor refactoring changes
Jul 17, 2020
6f28dc5
change tokenize to non async
Jul 17, 2020
d960ac7
rename handler
Jul 17, 2020
2853580
refactoring + copyright
Jul 17, 2020
a55108c
renamed handler file
Jul 17, 2020
75b6386
Delete log20200713.txt
justinytchen Jul 17, 2020
01bde39
moved/refactored handler
Jul 17, 2020
ea0fab2
added e2e tets
Jul 17, 2020
ccf1028
Merge branch 'semantic-token' of https://github.com/justinytchen/Powe…
Jul 17, 2020
170fb6b
updated test
Jul 20, 2020
4764d2e
remove pragma
Jul 20, 2020
5572a16
removed extra spacing
Jul 21, 2020
f8411cc
added testing for converting from PS token to semantic tokens
Jul 21, 2020
4b7db57
refactored the functions related to converting between tokens
Jul 21, 2020
a20241b
refactored ConvertSemanticToken
Jul 21, 2020
bebc507
fixed tests
Jul 22, 2020
5550780
added more test cases
Jul 22, 2020
3fb9abe
fixed spacing
Jul 23, 2020
73bda8d
added enum test
Jul 23, 2020
4703804
fixed spacing issues
Jul 23, 2020
c013b1d
fixed spacing, added note about token array representation
Jul 23, 2020
5ab13c4
changed name to PsesSemanticTokensHandler
Jul 23, 2020
62566a8
reformatted fields
Jul 24, 2020
bafff92
renamed file
Jul 24, 2020
5a631dd
used Assert.Collection instead of Assert.Single
Jul 24, 2020
7df4e67
modified yml file to fix build
Jul 27, 2020
b2e43b1
undo changes in yml file
Jul 28, 2020
4a6955f
addressed issues in PR
Jul 30, 2020
f4562c0
added basic semantic token support
Jul 16, 2020
6264c0b
removed unnecessary imports
Jul 16, 2020
ae5a498
removed unnecessary field
Jul 16, 2020
9ae83aa
minor refactoring changes
Jul 17, 2020
b4d558c
minor refactoring changes
Jul 17, 2020
874db6e
change tokenize to non async
Jul 17, 2020
bec8bd6
rename handler
Jul 17, 2020
9848c71
refactoring + copyright
Jul 17, 2020
855790c
renamed handler file
Jul 17, 2020
514012c
moved/refactored handler
Jul 17, 2020
5294cf4
added e2e tets
Jul 17, 2020
9aed66b
Delete log20200713.txt
justinytchen Jul 17, 2020
b4024e7
updated test
Jul 20, 2020
4908752
remove pragma
Jul 20, 2020
2e4f098
removed extra spacing
Jul 21, 2020
8f5db93
added testing for converting from PS token to semantic tokens
Jul 21, 2020
42714b5
refactored the functions related to converting between tokens
Jul 21, 2020
1657fdf
refactored ConvertSemanticToken
Jul 21, 2020
ebae357
fixed tests
Jul 22, 2020
f109d71
added more test cases
Jul 22, 2020
c9b858d
fixed spacing
Jul 23, 2020
d8f0a36
added enum test
Jul 23, 2020
3fba85f
fixed spacing issues
Jul 23, 2020
8592540
fixed spacing, added note about token array representation
Jul 23, 2020
d4a86ac
changed name to PsesSemanticTokensHandler
Jul 23, 2020
e86c5b8
reformatted fields
Jul 24, 2020
6c21195
renamed file
Jul 24, 2020
09d632c
used Assert.Collection instead of Assert.Single
Jul 24, 2020
7c4b3b7
addressed issues in PR
Jul 30, 2020
99c1c0b
Merge branch 'semantic-token' of https://github.com/justinytchen/Powe…
Jul 30, 2020
725e8eb
remove unused using
Jul 30, 2020
1aedd88
Delete Untitled-1.json
justinytchen Jul 30, 2020
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
1 change: 1 addition & 0 deletions src/PowerShellEditorServices/Server/PsesLanguageServer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,7 @@ public async Task StartAsync()
.WithHandler<GetCommandHandler>()
.WithHandler<ShowHelpHandler>()
.WithHandler<ExpandAliasHandler>()
.WithHandler<PsesSemanticTokens>()
.OnInitialize(
async (languageServer, request, cancellationToken) =>
{
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,160 @@
//
// Copyright (c) Microsoft. All rights reserved.
// Licensed under the MIT license. See LICENSE file in the project root for full license information.
//

using System;
using System.Collections.Generic;
using System.Management.Automation.Language;
using System.Threading;
using System.Threading.Tasks;
using Microsoft.Extensions.Logging;
using Microsoft.PowerShell.EditorServices.Services;
using Microsoft.PowerShell.EditorServices.Services.TextDocument;
using Microsoft.PowerShell.EditorServices.Utility;
using OmniSharp.Extensions.LanguageServer.Protocol;
using OmniSharp.Extensions.LanguageServer.Protocol.Document.Proposals;
using OmniSharp.Extensions.LanguageServer.Protocol.Models;
using OmniSharp.Extensions.LanguageServer.Protocol.Models.Proposals;

namespace Microsoft.PowerShell.EditorServices.Handlers
{
internal class PsesSemanticTokens : SemanticTokensHandler
Copy link
Contributor

@rjmholt rjmholt Jul 21, 2020

Choose a reason for hiding this comment

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

Suggested change
internal class PsesSemanticTokens : SemanticTokensHandler
internal class PsesSemanticTokensHandler : SemanticTokensHandler

Copy link
Member

Choose a reason for hiding this comment

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

Tokens*

Copy link
Contributor

Choose a reason for hiding this comment

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

(Fixed in my suggestion)

{
private readonly ILogger _logger;
private readonly WorkspaceService _workspaceService;
private static readonly SemanticTokensRegistrationOptions s_registrationOptions = new SemanticTokensRegistrationOptions() {
DocumentSelector = LspUtils.PowerShellDocumentSelector,
Legend = new SemanticTokensLegend(),
DocumentProvider = new Supports<SemanticTokensDocumentProviderOptions>(isSupported: true,
new SemanticTokensDocumentProviderOptions {
Edits = true
}),
RangeProvider = true
};

Copy link
Contributor

@rjmholt rjmholt Jul 21, 2020

Choose a reason for hiding this comment

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

Suggested change
private readonly ILogger _logger;
private readonly WorkspaceService _workspaceService;
private static readonly SemanticTokensRegistrationOptions s_registrationOptions = new SemanticTokensRegistrationOptions() {
DocumentSelector = LspUtils.PowerShellDocumentSelector,
Legend = new SemanticTokensLegend(),
DocumentProvider = new Supports<SemanticTokensDocumentProviderOptions>(isSupported: true,
new SemanticTokensDocumentProviderOptions {
Edits = true
}),
RangeProvider = true
};
private static readonly SemanticTokensRegistrationOptions s_registrationOptions = new SemanticTokensRegistrationOptions
{
DocumentSelector = LspUtils.PowerShellDocumentSelector,
Legend = new SemanticTokensLegend(),
DocumentProvider = new Supports<SemanticTokensDocumentProviderOptions>(
isSupported: true,
new SemanticTokensDocumentProviderOptions
{
Edits = true
}),
RangeProvider = true
};
private readonly ILogger _logger;
private readonly WorkspaceService _workspaceService;

Copy link
Member

Choose a reason for hiding this comment

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

this but with { on new lines and remove redundant () on SemanticTokensRegistrationOptions.

Copy link
Contributor

Choose a reason for hiding this comment

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

Fixed above

public PsesSemanticTokens(ILogger<PsesSemanticTokens> logger, WorkspaceService workspaceService) : base(s_registrationOptions)
{
_logger = logger;
_workspaceService = workspaceService;
}

protected override Task Tokenize(SemanticTokensBuilder builder, ITextDocumentIdentifierParams identifier,
CancellationToken cancellationToken)
{
ScriptFile file = _workspaceService.GetFile(DocumentUri.GetFileSystemPath(identifier));
foreach (var token in file.ScriptTokens){
rjmholt marked this conversation as resolved.
Show resolved Hide resolved
PushToken(token, builder);
}
return Task.CompletedTask;
}

private static void PushToken(Token token, SemanticTokensBuilder builder)
{
foreach(SemanticToken sToken in ConvertToSemanticTokens(token))
{
builder.Push(
sToken.Line,
sToken.Index,
length: sToken.Text.Length,
sToken.Type,
tokenModifiers: sToken.TokenModifiers);
}
}

internal static IEnumerable<SemanticToken> ConvertToSemanticTokens(Token token)
{
if (token is StringExpandableToken stringExpandableToken)
{
// Try parsing tokens within the string
if (stringExpandableToken.NestedTokens != null)
{
foreach (Token t in stringExpandableToken.NestedTokens)
{
foreach (SemanticToken subToken in ConvertToSemanticTokens(t))
yield return subToken;
}
yield break;
}
}

SemanticTokenType mappedType = MapSemanticTokenType(token);
rjmholt marked this conversation as resolved.
Show resolved Hide resolved
if (mappedType == null){
rjmholt marked this conversation as resolved.
Show resolved Hide resolved
yield break;
}

//Tokens line and col numbers indexed starting from 1, expecting indexing from 0
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
//Tokens line and col numbers indexed starting from 1, expecting indexing from 0
// Tokens line and col numbers indexed starting from 1, expecting indexing from 0

int line = token.Extent.StartLineNumber - 1;
int index = token.Extent.StartColumnNumber - 1;
SemanticToken sToken = new SemanticToken(token.Text, mappedType,
line, index, Array.Empty<string>());
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
SemanticToken sToken = new SemanticToken(token.Text, mappedType,
line, index, Array.Empty<string>());
SemanticToken sToken = new SemanticToken(
token.Text,
mappedType,
line,
index,
Array.Empty<string>());

yield return sToken;
}

private static SemanticTokenType MapSemanticTokenType(Token token)
{
// First check token flags
if ((token.TokenFlags & TokenFlags.Keyword) != 0)
{
return SemanticTokenType.Keyword;
}

if ((token.TokenFlags & TokenFlags.CommandName) != 0)
{
return SemanticTokenType.Function;
}

if (token.Kind != TokenKind.Generic && (token.TokenFlags &
(TokenFlags.BinaryOperator | TokenFlags.UnaryOperator | TokenFlags.AssignmentOperator)) != 0)
{
return SemanticTokenType.Operator;
}

if ((token.TokenFlags & TokenFlags.TypeName) != 0)
{
return SemanticTokenType.Type;
}

if ((token.TokenFlags & TokenFlags.MemberName) != 0)
{
return SemanticTokenType.Member;
}

// Only check token kind after checking flags
switch (token.Kind)
{
case TokenKind.Comment:
return SemanticTokenType.Comment;

case TokenKind.Parameter:
case TokenKind.Generic when token is StringLiteralToken slt && slt.Text.StartsWith("--"):
return SemanticTokenType.Parameter;

case TokenKind.Variable:
case TokenKind.SplattedVariable:
return SemanticTokenType.Variable;

case TokenKind.StringExpandable:
case TokenKind.StringLiteral:
case TokenKind.HereStringExpandable:
case TokenKind.HereStringLiteral:
return SemanticTokenType.String;

case TokenKind.Number:
return SemanticTokenType.Number;

case TokenKind.Generic:
return SemanticTokenType.Function;
}

return null;
}

protected override Task<SemanticTokensDocument> GetSemanticTokensDocument(
ITextDocumentIdentifierParams @params,
CancellationToken cancellationToken)
{
return Task.FromResult(new SemanticTokensDocument(GetRegistrationOptions().Legend));
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
using System.Collections.Generic;
using OmniSharp.Extensions.LanguageServer.Protocol.Models.Proposals;

namespace Microsoft.PowerShell.EditorServices.Services.TextDocument
{
class SemanticToken
Copy link
Member

Choose a reason for hiding this comment

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

Isn't there an Omnisharp type for this?

Choose a reason for hiding this comment

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

No, never made one because it adds extra allocations.

Choose a reason for hiding this comment

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

I can add one if you'd like

Choose a reason for hiding this comment

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

I do have NormalizedToken that I used for unit testing purposes. Emits everything as an easy to compare string.

https://github.com/OmniSharp/csharp-language-server-protocol/blob/master/test/Lsp.Tests/SemanticTokensDocumentTests.cs#L280-L353

rjmholt marked this conversation as resolved.
Show resolved Hide resolved
{
public SemanticToken(string text, SemanticTokenType type, int line, int index, IEnumerable<string> tokenModifiers)
TylerLeonhardt marked this conversation as resolved.
Show resolved Hide resolved
{
Line = line;
Text = text;
Index = index;
Type = type;
TokenModifiers = tokenModifiers;
}
public string Text {get; set;}
public int Line {get; set;}
public int Index {get; set;}
public SemanticTokenType Type {get; set;}
public IEnumerable<string> TokenModifiers {get; set;}
rjmholt marked this conversation as resolved.
Show resolved Hide resolved
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,12 @@
using OmniSharp.Extensions.LanguageServer.Protocol.Document;
using OmniSharp.Extensions.LanguageServer.Protocol.Models;
using OmniSharp.Extensions.LanguageServer.Protocol.Workspace;
using OmniSharp.Extensions.LanguageServer.Protocol.Models.Proposals;
using Xunit;
using Xunit.Abstractions;
using Range = OmniSharp.Extensions.LanguageServer.Protocol.Models.Range;
using OmniSharp.Extensions.LanguageServer.Protocol.Document.Proposals;
using Microsoft.PowerShell.EditorServices.Utility;
TylerLeonhardt marked this conversation as resolved.
Show resolved Hide resolved

namespace PowerShellEditorServices.Test.E2E
{
Expand Down Expand Up @@ -1136,5 +1139,35 @@ await PsesLanguageClient

Assert.Equal("Get-ChildItem", expandAliasResult.Text);
}

[Fact]
public async Task CanSendSemanticTokenRequest()
{
string scriptContent = "function";
string scriptPath = NewTestFile(scriptContent);

SemanticTokens result =
await PsesLanguageClient
.SendRequest<SemanticTokensParams>(
"textDocument/semanticTokens",
new SemanticTokensParams
{
TextDocument = new TextDocumentIdentifier
{
Uri = new Uri(scriptPath)
}
})
.Returning<SemanticTokens>(CancellationToken.None);

// More information about how this data is generated can be found at
// https://github.com/microsoft/vscode-extension-samples/blob/5ae1f7787122812dcc84e37427ca90af5ee09f14/semantic-tokens-sample/vscode.proposed.d.ts#L71
var expectedArr = new int[5]
{
//line, index, token length, token type, token modifiers
rjmholt marked this conversation as resolved.
Show resolved Hide resolved
0, 0, scriptContent.Length, 2, 0 //token 1 is function
rjmholt marked this conversation as resolved.
Show resolved Hide resolved
};

Assert.Equal(expectedArr, result.Data.ToArray());
}
}
}
156 changes: 156 additions & 0 deletions test/PowerShellEditorServices.Test/Language/SemanticTokenTest.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,156 @@
//
// Copyright (c) Microsoft. All rights reserved.
// Licensed under the MIT license. See LICENSE file in the project root for full license information.
//

using System;
using System.Collections.Generic;
using System.IO;
using System.Management.Automation.Language;
using System.Threading.Tasks;
using Microsoft.PowerShell.EditorServices.Services.TextDocument;
using Microsoft.PowerShell.EditorServices.Handlers;
using OmniSharp.Extensions.LanguageServer.Protocol;
using OmniSharp.Extensions.LanguageServer.Protocol.Document.Proposals;
using OmniSharp.Extensions.LanguageServer.Protocol.Models.Proposals;
using Xunit;

namespace Microsoft.PowerShell.EditorServices.Test.Language
Copy link
Contributor

Choose a reason for hiding this comment

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

The tests here look good. One extra thing I would do is include tests for the less conspicuous tokens like (, ), {, } -- we have to classify and highlight those too, unless perhaps there's some other mechanism at work there?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Those tokens aren't mapped to SemanticTokenTypes and are handled by EditorSyntax

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh, I didn't realise that. That seems a bit strange, and maybe like we're doing more work than we should... We should maybe discuss at some point

Copy link
Contributor Author

Choose a reason for hiding this comment

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

{
public class SemanticTokenTest
{
[Fact]
public async Task TokenizesFunctionElements()
{
string text = @"
function Get-Sum {
param( [int]$a, [int]$b )
return $a + $b
}
";
ScriptFile scriptFile = new ScriptFile(
// Use any absolute path. Even if it doesn't exist.
DocumentUri.FromFileSystemPath(Path.Combine(Path.GetTempPath(), "TestFile.ps1")),
text,
Version.Parse("5.0"));

foreach(Token t in scriptFile.ScriptTokens)
{
List<SemanticToken> mappedTokens = new List<SemanticToken>(PsesSemanticTokens.ConvertToSemanticTokens(t));
switch(t.Text)
{
case "function":
case "param":
case "return":
Assert.Collection(mappedTokens, sToken => Assert.Equal(SemanticTokenType.Keyword, mappedTokens[0].Type));
diego-alvarez-by marked this conversation as resolved.
Show resolved Hide resolved
break;
case "Get-Sum":
Assert.Collection(mappedTokens, sToken => Assert.Equal(SemanticTokenType.Function, mappedTokens[0].Type));
break;
case "$a":
case "$b":
Assert.Collection(mappedTokens, sToken => Assert.Equal(SemanticTokenType.Variable, mappedTokens[0].Type));
break;
case "[int]":
Assert.Collection(mappedTokens, sToken => Assert.Equal(SemanticTokenType.Type, mappedTokens[0].Type));
break;
case "+":
Assert.Collection(mappedTokens, sToken => Assert.Equal(SemanticTokenType.Operator, mappedTokens[0].Type));
break;
}
}
}

[Fact]
public async Task TokenizesStringExpansion()
{
string text = "Write-Host \"$(Test-Property Get-Whatever) $(Get-Whatever)\"";
ScriptFile scriptFile = new ScriptFile(
// Use any absolute path. Even if it doesn't exist.
DocumentUri.FromFileSystemPath(Path.Combine(Path.GetTempPath(), "TestFile.ps1")),
text,
Version.Parse("5.0"));

Token commandToken = scriptFile.ScriptTokens[0];
List<SemanticToken> mappedTokens = new List<SemanticToken>(PsesSemanticTokens.ConvertToSemanticTokens(commandToken));
Assert.Single(mappedTokens);
Assert.Equal(SemanticTokenType.Function, mappedTokens[0].Type);

Token stringExpandableToken = scriptFile.ScriptTokens[1];
mappedTokens = new List<SemanticToken>(PsesSemanticTokens.ConvertToSemanticTokens(stringExpandableToken));
Assert.Collection(mappedTokens,
sToken => Assert.Equal(SemanticTokenType.Function, sToken.Type),
sToken => Assert.Equal(SemanticTokenType.Function, sToken.Type),
sToken => Assert.Equal(SemanticTokenType.Function, sToken.Type)
);
}

[Fact]
public async Task RecognizesTokensWithAsterisk()
{
string text = @"
function Get-A*A {
}
Get-A*A
";
ScriptFile scriptFile = new ScriptFile(
// Use any absolute path. Even if it doesn't exist.
DocumentUri.FromFileSystemPath(Path.Combine(Path.GetTempPath(), "TestFile.ps1")),
text,
Version.Parse("5.0"));

foreach(Token t in scriptFile.ScriptTokens)
{
List<SemanticToken> mappedTokens = new List<SemanticToken>(PsesSemanticTokens.ConvertToSemanticTokens(t));
switch(t.Text)
{
case "function":
Assert.Collection(mappedTokens, sToken => Assert.Equal(SemanticTokenType.Keyword, mappedTokens[0].Type));
break;
case "Get-A*A":
Assert.Collection(mappedTokens, sToken => Assert.Equal(SemanticTokenType.Function, mappedTokens[0].Type));
break;
TylerLeonhardt marked this conversation as resolved.
Show resolved Hide resolved
}
}
}

[Fact]
public async Task RecognizesArrayMemberInExpandableString()
{
string text = "\"$(@($Array).Count) OtherText\"";
ScriptFile scriptFile = new ScriptFile(
// Use any absolute path. Even if it doesn't exist.
DocumentUri.FromFileSystemPath(Path.Combine(Path.GetTempPath(), "TestFile.ps1")),
text,
Version.Parse("5.0"));

foreach(Token t in scriptFile.ScriptTokens)
{
List<SemanticToken> mappedTokens = new List<SemanticToken>(PsesSemanticTokens.ConvertToSemanticTokens(t));
switch(t.Text)
{
case "$Array":
Assert.Collection(mappedTokens, sToken => Assert.Equal(SemanticTokenType.Variable, mappedTokens[0].Type));
break;
case "Count":
Assert.Collection(mappedTokens, sToken => Assert.Equal(SemanticTokenType.Member, mappedTokens[0].Type));
break;
}
}
}

[Fact]
public async Task RecognizesCurlyQuotedString()
{
string text = "“^[-'a-z]*”";
ScriptFile scriptFile = new ScriptFile(
// Use any absolute path. Even if it doesn't exist.
DocumentUri.FromFileSystemPath(Path.Combine(Path.GetTempPath(), "TestFile.ps1")),
text,
Version.Parse("5.0"));

List<SemanticToken> mappedTokens = new List<SemanticToken>(PsesSemanticTokens.ConvertToSemanticTokens(scriptFile.ScriptTokens[0]));
Assert.Collection(mappedTokens, sToken => Assert.Equal(SemanticTokenType.String, mappedTokens[0].Type));
TylerLeonhardt marked this conversation as resolved.
Show resolved Hide resolved
}
}
}