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

Minimize semantic tokens returned for constants and method calls #2479

Merged
merged 3 commits into from
Aug 23, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
79 changes: 11 additions & 68 deletions lib/ruby_lsp/listeners/semantic_highlighting.rb
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ class SemanticHighlighting
Kernel.methods(false),
Bundler::Dsl.instance_methods(false),
Module.private_instance_methods(false),
].flatten.map(&:to_s),
].flatten.map(&:to_s).freeze,
T::Array[String],
)

Expand Down Expand Up @@ -50,13 +50,6 @@ def initialize(dispatcher, response_builder)
:on_optional_parameter_node_enter,
:on_required_parameter_node_enter,
:on_rest_parameter_node_enter,
:on_constant_read_node_enter,
:on_constant_write_node_enter,
:on_constant_and_write_node_enter,
:on_constant_operator_write_node_enter,
:on_constant_or_write_node_enter,
:on_constant_target_node_enter,
:on_constant_path_node_enter,
:on_local_variable_and_write_node_enter,
:on_local_variable_operator_write_node_enter,
:on_local_variable_or_write_node_enter,
Expand All @@ -82,8 +75,14 @@ def on_call_node_enter(node)
return if message == "=~"
return if special_method?(message)

type = Requests::Support::Sorbet.annotation?(node) ? :type : :method
@response_builder.add_token(T.must(node.message_loc), type)
if Requests::Support::Sorbet.annotation?(node)
@response_builder.add_token(T.must(node.message_loc), :type)
elsif !node.receiver && !node.opening_loc
# If the node has a receiver, then the syntax is not ambiguous and semantic highlighting is not necessary to
# determine that the token is a method call. The only ambiguous case is method calls with implicit self
# receiver and no parenthesis, which may be confused with local variables
@response_builder.add_token(T.must(node.message_loc), :method)
end
end

sig { params(node: Prism::MatchWriteNode).void }
Expand All @@ -101,42 +100,9 @@ def on_match_write_node_leave(node)
@inside_regex_capture = true if node.call.message == "=~"
end

sig { params(node: Prism::ConstantReadNode).void }
def on_constant_read_node_enter(node)
return if @inside_implicit_node

@response_builder.add_token(node.location, :namespace)
end

sig { params(node: Prism::ConstantWriteNode).void }
def on_constant_write_node_enter(node)
@response_builder.add_token(node.name_loc, :namespace)
end

sig { params(node: Prism::ConstantAndWriteNode).void }
def on_constant_and_write_node_enter(node)
@response_builder.add_token(node.name_loc, :namespace)
end

sig { params(node: Prism::ConstantOperatorWriteNode).void }
def on_constant_operator_write_node_enter(node)
@response_builder.add_token(node.name_loc, :namespace)
end

sig { params(node: Prism::ConstantOrWriteNode).void }
def on_constant_or_write_node_enter(node)
@response_builder.add_token(node.name_loc, :namespace)
end

sig { params(node: Prism::ConstantTargetNode).void }
def on_constant_target_node_enter(node)
@response_builder.add_token(node.location, :namespace)
end

sig { params(node: Prism::DefNode).void }
def on_def_node_enter(node)
@current_scope = ParameterScope.new(@current_scope)
@response_builder.add_token(node.name_loc, :method, [:declaration])
end

sig { params(node: Prism::DefNode).void }
Expand Down Expand Up @@ -168,49 +134,33 @@ def on_block_parameter_node_enter(node)
sig { params(node: Prism::RequiredKeywordParameterNode).void }
def on_required_keyword_parameter_node_enter(node)
@current_scope << node.name

location = node.name_loc
@response_builder.add_token(location.copy(length: location.length - 1), :parameter)
end

sig { params(node: Prism::OptionalKeywordParameterNode).void }
def on_optional_keyword_parameter_node_enter(node)
@current_scope << node.name

location = node.name_loc
@response_builder.add_token(location.copy(length: location.length - 1), :parameter)
end

sig { params(node: Prism::KeywordRestParameterNode).void }
def on_keyword_rest_parameter_node_enter(node)
name = node.name

if name
@current_scope << name.to_sym
@response_builder.add_token(T.must(node.name_loc), :parameter)
end
@current_scope << name.to_sym if name
end

sig { params(node: Prism::OptionalParameterNode).void }
def on_optional_parameter_node_enter(node)
@current_scope << node.name
@response_builder.add_token(node.name_loc, :parameter)
end

sig { params(node: Prism::RequiredParameterNode).void }
def on_required_parameter_node_enter(node)
@current_scope << node.name
@response_builder.add_token(node.location, :parameter)
end

sig { params(node: Prism::RestParameterNode).void }
def on_rest_parameter_node_enter(node)
name = node.name

if name
@current_scope << name.to_sym
@response_builder.add_token(T.must(node.name_loc), :parameter)
end
@current_scope << name.to_sym if name
end

sig { params(node: Prism::SelfNode).void }
Expand Down Expand Up @@ -332,13 +282,6 @@ def on_implicit_node_leave(node)
@inside_implicit_node = false
end

sig { params(node: Prism::ConstantPathNode).void }
def on_constant_path_node_enter(node)
return if @inside_implicit_node

@response_builder.add_token(node.name_loc, :namespace)
end

private

# Textmate provides highlighting for a subset of these special Ruby-specific methods. We want to utilize that
Expand Down
10 changes: 10 additions & 0 deletions lib/ruby_lsp/requests/semantic_highlighting.rb
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,16 @@ module Requests
# some_invocation # --> semantic highlighting: method invocation
# var # --> semantic highlighting: local variable
# end
#
# # Strategy
#
# To maximize editor performance, the Ruby LSP will return the minimum number of semantic tokens, since applying
# them is an expensive operation for the client. This means that the server only returns tokens for ambiguous pieces
# of syntax, such as method invocations with no receivers or parenthesis (which can be confused with local
# variables).
#
# Offloading as much handling as possible to Text Mate grammars or equivalent will guarantee responsiveness in the
# editor and allow for a much smoother experience.
# ```
class SemanticHighlighting < Request
extend T::Sig
Expand Down
7 changes: 0 additions & 7 deletions test/expectations/semantic_highlighting/aref_field.exp.json
Original file line number Diff line number Diff line change
@@ -1,12 +1,5 @@
{
"result": [
{
"delta_line": 0,
"delta_start_char": 4,
"length": 9,
"token_type": 13,
"token_modifiers": 1
},
{
"delta_line": 1,
"delta_start_char": 2,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,12 +1,5 @@
{
"result": [
{
"delta_line": 0,
"delta_start_char": 4,
"length": 9,
"token_type": 13,
"token_modifiers": 1
},
{
"delta_line": 1,
"delta_start_char": 2,
Expand Down
17 changes: 1 addition & 16 deletions test/expectations/semantic_highlighting/block_locals.exp.json
Original file line number Diff line number Diff line change
@@ -1,23 +1,8 @@
{
"params": [],
"result": [
{
"delta_line": 0,
"delta_start_char": 3,
"length": 4,
"token_type": 13,
"token_modifiers": 0
},
{
"delta_line": 0,
"delta_start_char": 9,
"length": 1,
"token_type": 7,
"token_modifiers": 0
},
{
"delta_line": 0,
"delta_start_char": 3,
"delta_start_char": 15,
"length": 11,
"token_type": 8,
"token_modifiers": 0
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
@@ -1,12 +1,5 @@
{
"result": [
{
"delta_line": 0,
"delta_start_char": 4,
"length": 11,
"token_type": 13,
"token_modifiers": 1
},
{
"delta_line": 1,
"delta_start_char": 2,
Expand All @@ -20,13 +13,6 @@
"length": 3,
"token_type": 8,
"token_modifiers": 0
},
{
"delta_line": 0,
"delta_start_char": 4,
"length": 6,
"token_type": 13,
"token_modifiers": 0
}
]
}
Original file line number Diff line number Diff line change
@@ -1,12 +1,5 @@
{
"result": [
{
"delta_line": 0,
"delta_start_char": 4,
"length": 11,
"token_type": 13,
"token_modifiers": 1
},
{
"delta_line": 1,
"delta_start_char": 2,
Expand All @@ -30,14 +23,7 @@
},
{
"delta_line": 0,
"delta_start_char": 4,
"length": 6,
"token_type": 13,
"token_modifiers": 0
},
{
"delta_line": 0,
"delta_start_char": 7,
"delta_start_char": 11,
"length": 3,
"token_type": 8,
"token_modifiers": 0
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,26 +7,12 @@
"token_type": 2,
"token_modifiers": 1
},
{
"delta_line": 0,
"delta_start_char": 0,
"length": 3,
"token_type": 0,
"token_modifiers": 0
},
{
"delta_line": 0,
"delta_start_char": 6,
"length": 10,
"token_type": 2,
"token_modifiers": 0
},
{
"delta_line": 0,
"delta_start_char": 0,
"length": 10,
"token_type": 0,
"token_modifiers": 0
}
]
}
Original file line number Diff line number Diff line change
Expand Up @@ -16,14 +16,7 @@
},
{
"delta_line": 0,
"delta_start_char": 7,
"length": 10,
"token_type": 13,
"token_modifiers": 0
},
{
"delta_line": 0,
"delta_start_char": 11,
"delta_start_char": 18,
"length": 3,
"token_type": 8,
"token_modifiers": 0
Expand Down
Loading
Loading