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

Provide constant type differentiation in semantic highlighting request #1324

Closed
wants to merge 5 commits into from
Closed

Provide constant type differentiation in semantic highlighting request #1324

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
60c0549
Add index as a parameter to the SemanticHighlighting request
mateusdeap Oct 27, 2023
6b92772
Track nesting
mateusdeap Oct 27, 2023
cb206a7
Listen for class and module leave events
mateusdeap Oct 27, 2023
87439be
Add `:constant` symbol to `TOKEN_TYPES`
mateusdeap Jan 25, 2024
d79ec2b
Push appropriate token types depending on entry type
mateusdeap Jan 25, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
47 changes: 43 additions & 4 deletions lib/ruby_lsp/listeners/semantic_highlighting.rb
View file
Open in desktop
Expand Up @@ -34,6 +34,7 @@ class SemanticHighlighting < Listener
regexp: 20,
operator: 21,
decorator: 22,
constant: 23
Copy link
Member

Choose a reason for hiding this comment

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

This token type doesn't exist in the specification. We can't add it here because editors won't know what to do with that type.

We have two options here. We can use static variable as the token type for constants or we can propose the addition of constant as a token type to the specification.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Interesting. I was completely unaware of this.

Particularly, I'll just use the static variable, but do you think there is value in adding the constant token type to the spec? I'm assuming this might be an issue that the spec is built thinking of wording that doesn't exactly match Ruby concepts. So maybe static variable would be the equivalent of a Ruby constant. Despite the glaring contradiction in naming.

}.freeze,
T::Hash[Symbol, Integer],
)
Expand Down Expand Up @@ -92,26 +93,36 @@ def initialize(location:, length:, type:, modifier:)
sig { override.returns(ResponseType) }
attr_reader :_response

sig { params(dispatcher: Prism::Dispatcher, range: T.nilable(T::Range[Integer])).void }
def initialize(dispatcher, range: nil)
sig {
params(
dispatcher: Prism::Dispatcher,
index: RubyIndexer::Index,
range: T.nilable(T::Range[Integer])
).void
}
def initialize(dispatcher, index, range: nil)
super(dispatcher)

@_response = T.let([], ResponseType)
@range = range
@special_methods = T.let(nil, T.nilable(T::Array[String]))
@current_scope = T.let(ParameterScope.new, ParameterScope)
@index = index
@nesting = T.let([], T::Array[String])
@inside_regex_capture = T.let(false, T::Boolean)

dispatcher.register(
self,
:on_call_node_enter,
:on_class_node_enter,
:on_class_node_leave,
:on_def_node_enter,
:on_def_node_leave,
:on_block_node_enter,
:on_block_node_leave,
:on_self_node_enter,
:on_module_node_enter,
:on_module_node_leave,
:on_local_variable_write_node_enter,
:on_local_variable_read_node_enter,
:on_block_parameter_node_enter,
Expand Down Expand Up @@ -171,9 +182,25 @@ def on_match_write_node_leave(node)

sig { params(node: Prism::ConstantReadNode).void }
def on_constant_read_node_enter(node)
return unless visible?(node, @range)
entries = @index.resolve(node.name, @nesting)
# if entries is nil, we didn't find the declaration for it
# it might be defined with meta-programming or using C code
unless entries
add_token(node.location, :constant)
end

add_token(node.location, :namespace)
# Otherwise, we can check the type of entry to determine the
# type of constant
first_entry = T.must(entries.first)

case first_entry
when RubyIndexer::Entry::Class
add_token(node.location, :class, [:declaration])
when RubyIndexer::Entry::Module
add_token(node.location, :namespace, [:declaration])
when RubyIndexer::Entry::Constant
add_token(node.location, :constant, [:declaration])
Comment on lines +198 to +202
Copy link
Member

Choose a reason for hiding this comment

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

Constant reads are references to constants, not declarations. So you need to remove the modifiers on all of these add_token calls.

end
end

sig { params(node: Prism::ConstantWriteNode).void }
Expand Down Expand Up @@ -366,19 +393,31 @@ def on_local_variable_target_node_enter(node)
def on_class_node_enter(node)
return unless visible?(node, @range)

@nesting.push(node.name)
add_token(node.constant_path.location, :class, [:declaration])

superclass = node.superclass
add_token(superclass.location, :class) if superclass
end

sig { params(node: Prism::ClassNode).void }
def on_class_node_leave(node)
@nesting.pop
end

sig { params(node: Prism::ModuleNode).void }
def on_module_node_enter(node)
return unless visible?(node, @range)

@nesting.push(node.name)
add_token(node.constant_path.location, :namespace, [:declaration])
end

sig { params(node: Prism::ClassNode).void }
def on_module_node_leave(node)
@nesting.pop
end

private

sig { params(location: Prism::Location, type: Symbol, modifiers: T::Array[Symbol]).void }
Expand Down
12 changes: 9 additions & 3 deletions lib/ruby_lsp/requests/semantic_highlighting.rb
View file
Open in desktop
Expand Up @@ -43,10 +43,16 @@ def provider
end
end

sig { params(dispatcher: Prism::Dispatcher, range: T.nilable(T::Range[Integer])).void }
def initialize(dispatcher, range: nil)
sig {
params(
dispatcher: Prism::Dispatcher,
index: RubyIndexer::Index,
range: T.nilable(T::Range[Integer])
).void
}
def initialize(dispatcher, index, range: nil)
super()
@listener = T.let(Listeners::SemanticHighlighting.new(dispatcher, range: range), Listener[ResponseType])
@listener = T.let(Listeners::SemanticHighlighting.new(dispatcher, index, range: range), Listener[ResponseType])
end

sig { override.returns(ResponseType) }
Expand Down
2 changes: 1 addition & 1 deletion test/fixtures/prism
Open in desktop
Copy link
Member

Choose a reason for hiding this comment

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

Let's not commit these changes.

Submodule prism updated 1022 files
22 changes: 21 additions & 1 deletion test/requests/semantic_highlighting_expectations_test.rb
View file
Open in desktop
Expand Up @@ -11,14 +11,34 @@ def run_expectations(source)
document = RubyLsp::RubyDocument.new(source: source, version: 1, uri: URI("file:///fake.rb"))
range = @__params&.any? ? @__params.first : nil

store = RubyLsp::Store.new
store.set(uri: URI("file:///folder/fake.rb"), source: source, version: 1)
executor = RubyLsp::Executor.new(store, message_queue)
index = executor.instance_variable_get(:@index)
Dir.glob(TEST_RUBY_LSP_FIXTURES).each do |path|
index.index_single(
RubyIndexer::IndexablePath.new(
"#{Dir.pwd}/lib",
File.expand_path(
"../../#{path}",
__dir__,
),
),
)
end
Comment on lines +18 to +28
Copy link
Member

Choose a reason for hiding this comment

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

Instead of indexing every fixture here, we should use the source we receive in this method instead. Something like

index.index_single(
  RubyIndexer::IndexablePath.new(
    "fake",
    "fake/path.rb",
  ),
  source
)

Otherwise, we'd be indexing every fixture file in the codebase every time we're running the test for a single fixture.


if range
start_line = range.dig(:start, :line)
end_line = range.dig(:end, :line)
processed_range = start_line..end_line
end

dispatcher = Prism::Dispatcher.new
listener = RubyLsp::Requests::SemanticHighlighting.new(dispatcher, range: processed_range)
listener = RubyLsp::Requests::SemanticHighlighting.new(
dispatcher,
executor.instance_variable_get(:@index),
range: processed_range
)

dispatcher.dispatch(document.tree)
RubyLsp::Requests::Support::SemanticTokenEncoder.new.encode(listener.perform)
Expand Down