From a53b2fea019c2511eb476be05294289f3328e3d3 Mon Sep 17 00:00:00 2001 From: rogancodes Date: Sat, 30 Nov 2024 19:56:36 +0530 Subject: [PATCH 01/11] Index class variables --- .../lib/ruby_indexer/declaration_listener.rb | 58 ++++++++++ lib/ruby_indexer/lib/ruby_indexer/entry.rb | 20 ++++ lib/ruby_indexer/test/class_variables_test.rb | 108 ++++++++++++++++++ 3 files changed, 186 insertions(+) create mode 100644 lib/ruby_indexer/test/class_variables_test.rb diff --git a/lib/ruby_indexer/lib/ruby_indexer/declaration_listener.rb b/lib/ruby_indexer/lib/ruby_indexer/declaration_listener.rb index 0295e4cf2..e11a0e780 100644 --- a/lib/ruby_indexer/lib/ruby_indexer/declaration_listener.rb +++ b/lib/ruby_indexer/lib/ruby_indexer/declaration_listener.rb @@ -80,6 +80,11 @@ def initialize(index, dispatcher, parse_result, uri, collect_comments: false) :on_instance_variable_or_write_node_enter, :on_instance_variable_target_node_enter, :on_alias_method_node_enter, + :on_class_variable_and_write_node_enter, + :on_class_variable_operator_write_node_enter, + :on_class_variable_or_write_node_enter, + :on_class_variable_target_node_enter, + :on_class_variable_write_node_enter, ) end @@ -434,6 +439,31 @@ def on_alias_method_node_enter(node) ) end + sig { params(node: Prism::ClassVariableAndWriteNode).void } + def on_class_variable_and_write_node_enter(node) + handle_class_variable(node, node.name_loc) + end + + sig { params(node: Prism::ClassVariableOperatorWriteNode).void } + def on_class_variable_operator_write_node_enter(node) + handle_class_variable(node, node.name_loc) + end + + sig { params(node: Prism::ClassVariableOrWriteNode).void } + def on_class_variable_or_write_node_enter(node) + handle_class_variable(node, node.name_loc) + end + + sig { params(node: Prism::ClassVariableTargetNode).void } + def on_class_variable_target_node_enter(node) + handle_class_variable(node, node.location) + end + + sig { params(node: Prism::ClassVariableWriteNode).void } + def on_class_variable_write_node_enter(node) + handle_class_variable(node, node.name_loc) + end + sig do params( name: String, @@ -552,6 +582,34 @@ def handle_global_variable(node, loc) )) end + sig do + params( + node: T.any( + Prism::ClassVariableAndWriteNode, + Prism::ClassVariableOperatorWriteNode, + Prism::ClassVariableOrWriteNode, + Prism::ClassVariableTargetNode, + Prism::ClassVariableWriteNode, + ), + loc: Prism::Location, + ).void + end + def handle_class_variable(node, loc) + name = node.name.to_s + return if name == "@@" + + comments = collect_comments(node) + owner = @owner_stack.last + + @index.add(Entry::ClassVariable.new( + name, + @uri, + Location.from_prism_location(loc, @code_units_cache), + comments, + owner, + )) + end + sig do params( node: T.any( diff --git a/lib/ruby_indexer/lib/ruby_indexer/entry.rb b/lib/ruby_indexer/lib/ruby_indexer/entry.rb index 327f211f7..a9ab2df12 100644 --- a/lib/ruby_indexer/lib/ruby_indexer/entry.rb +++ b/lib/ruby_indexer/lib/ruby_indexer/entry.rb @@ -486,6 +486,26 @@ def initialize(target, unresolved_alias) # Represents a global variable e.g.: $DEBUG class GlobalVariable < Entry; end + # Represents a class variable e.g.: @@a = 1 + class ClassVariable < Entry + sig { returns(T.nilable(Entry::Namespace)) } + attr_reader :owner + + sig do + params( + name: String, + uri: URI::Generic, + location: Location, + comments: T.nilable(String), + owner: T.nilable(Entry::Namespace), + ).void + end + def initialize(name, uri, location, comments, owner) + super(name, uri, location, comments) + @owner = owner + end + end + # Represents an instance variable e.g.: @a = 1 class InstanceVariable < Entry sig { returns(T.nilable(Entry::Namespace)) } diff --git a/lib/ruby_indexer/test/class_variables_test.rb b/lib/ruby_indexer/test/class_variables_test.rb new file mode 100644 index 000000000..e41d7658f --- /dev/null +++ b/lib/ruby_indexer/test/class_variables_test.rb @@ -0,0 +1,108 @@ +# typed: true +# frozen_string_literal: true + +require_relative "test_case" + +module RubyIndexer + class ClassVariableTest < TestCase + def test_class_variable_and_write + index(<<~RUBY) + class Foo + @@bar &&= 1 + end + RUBY + + assert_entry("@@bar", Entry::ClassVariable, "/fake/path/foo.rb:1-2:1-7") + + entry = T.must(@index["@@bar"]&.first) + owner = T.must(entry.owner) + assert_instance_of(Entry::Class, owner) + assert_equal("Foo", owner.name) + end + + def test_class_variable_operator_write + index(<<~RUBY) + class Foo + @@bar += 1 + end + RUBY + + assert_entry("@@bar", Entry::ClassVariable, "/fake/path/foo.rb:1-2:1-7") + end + + def test_class_variable_or_write + index(<<~RUBY) + class Foo + @@bar ||= 1 + end + RUBY + + assert_entry("@@bar", Entry::ClassVariable, "/fake/path/foo.rb:1-2:1-7") + end + + def test_class_variable_target_node + index(<<~RUBY) + class Foo + @@foo, @@bar = 1 + end + RUBY + + assert_entry("@@foo", Entry::ClassVariable, "/fake/path/foo.rb:1-2:1-7") + assert_entry("@@bar", Entry::ClassVariable, "/fake/path/foo.rb:1-9:1-14") + + entry = T.must(@index["@@foo"]&.first) + owner = T.must(entry.owner) + assert_instance_of(Entry::Class, owner) + assert_equal("Foo", owner.name) + + entry = T.must(@index["@@bar"]&.first) + owner = T.must(entry.owner) + assert_instance_of(Entry::Class, owner) + assert_equal("Foo", owner.name) + end + + def test_class_variable_write + index(<<~RUBY) + class Foo + @@bar = 1 + end + RUBY + + assert_entry("@@bar", Entry::ClassVariable, "/fake/path/foo.rb:1-2:1-7") + end + + def test_empty_name_class_variables + index(<<~RUBY) + module Foo + @@ = 1 + end + RUBY + + refute_entry("@@") + end + + def test_top_level_class_variables + index(<<~RUBY) + @foo = 123 + RUBY + + entry = T.must(@index["@foo"]&.first) + assert_nil(entry.owner) + end + + def test_class_variables_inside_self_method + index(<<~RUBY) + class Foo + def self.bar + @@bar = 123 + end + end + RUBY + + entry = T.must(@index["@@bar"]&.first) + owner = T.must(entry.owner) + assert_instance_of(Entry::SingletonClass, owner) + assert_equal("Foo::", owner.name) + end + end +end From d7b3b310471b676b5ad65fe2763830ac88c7ecb6 Mon Sep 17 00:00:00 2001 From: rogancodes Date: Sun, 1 Dec 2024 10:34:05 +0530 Subject: [PATCH 02/11] support for class varaibles go to definition --- lib/ruby_lsp/listeners/definition.rb | 55 +++++++++++++++++++ lib/ruby_lsp/requests/definition.rb | 6 ++ test/requests/definition_expectations_test.rb | 30 ++++++++++ 3 files changed, 91 insertions(+) diff --git a/lib/ruby_lsp/listeners/definition.rb b/lib/ruby_lsp/listeners/definition.rb index 9c37b6cf9..daf939f58 100644 --- a/lib/ruby_lsp/listeners/definition.rb +++ b/lib/ruby_lsp/listeners/definition.rb @@ -55,6 +55,12 @@ def initialize(response_builder, global_state, language_id, uri, node_context, d :on_symbol_node_enter, :on_super_node_enter, :on_forwarding_super_node_enter, + :on_class_variable_and_write_node_enter, + :on_class_variable_operator_write_node_enter, + :on_class_variable_or_write_node_enter, + :on_class_variable_read_node_enter, + :on_class_variable_target_node_enter, + :on_class_variable_write_node_enter, ) end @@ -196,6 +202,36 @@ def on_forwarding_super_node_enter(node) handle_super_node_definition end + sig { params(node: Prism::ClassVariableAndWriteNode).void } + def on_class_variable_and_write_node_enter(node) + handle_class_variable_definition(node.name.to_s) + end + + sig { params(node: Prism::ClassVariableOperatorWriteNode).void } + def on_class_variable_operator_write_node_enter(node) + handle_class_variable_definition(node.name.to_s) + end + + sig { params(node: Prism::ClassVariableOrWriteNode).void } + def on_class_variable_or_write_node_enter(node) + handle_class_variable_definition(node.name.to_s) + end + + sig { params(node: Prism::ClassVariableTargetNode).void } + def on_class_variable_target_node_enter(node) + handle_class_variable_definition(node.name.to_s) + end + + sig { params(node: Prism::ClassVariableReadNode).void } + def on_class_variable_read_node_enter(node) + handle_class_variable_definition(node.name.to_s) + end + + sig { params(node: Prism::ClassVariableWriteNode).void } + def on_class_variable_write_node_enter(node) + handle_class_variable_definition(node.name.to_s) + end + private sig { void } @@ -232,6 +268,25 @@ def handle_global_variable_definition(name) end end + sig { params(name: String).void } + def handle_class_variable_definition(name) + entries = @index[name] + + return unless entries + + entries.each do |entry| + location = entry.location + + @response_builder << Interface::Location.new( + uri: entry.uri.to_s, + range: Interface::Range.new( + start: Interface::Position.new(line: location.start_line - 1, character: location.start_column), + end: Interface::Position.new(line: location.end_line - 1, character: location.end_column), + ), + ) + end + end + sig { params(name: String).void } def handle_instance_variable_definition(name) # Sorbet enforces that all instance variables be declared on typed strict or higher, which means it will be able diff --git a/lib/ruby_lsp/requests/definition.rb b/lib/ruby_lsp/requests/definition.rb index e4817e3db..35513f814 100644 --- a/lib/ruby_lsp/requests/definition.rb +++ b/lib/ruby_lsp/requests/definition.rb @@ -56,6 +56,12 @@ def initialize(document, global_state, position, dispatcher, sorbet_level) Prism::StringNode, Prism::SuperNode, Prism::ForwardingSuperNode, + Prism::ClassVariableAndWriteNode, + Prism::ClassVariableOperatorWriteNode, + Prism::ClassVariableOrWriteNode, + Prism::ClassVariableReadNode, + Prism::ClassVariableTargetNode, + Prism::ClassVariableWriteNode, ], code_units_cache: document.code_units_cache, ) diff --git a/test/requests/definition_expectations_test.rb b/test/requests/definition_expectations_test.rb index a66867fc6..65b43dc27 100644 --- a/test/requests/definition_expectations_test.rb +++ b/test/requests/definition_expectations_test.rb @@ -744,6 +744,36 @@ class Foo end end + def test_definition_for_class_variables + source = <<~RUBY + class Foo + def foo + @@a ||= 1 + end + + def bar + @@a += 5 + end + + def baz + @@a + end + end + RUBY + + with_server(source) do |server, uri| + server.process_message( + id: 1, + method: "textDocument/definition", + params: { textDocument: { uri: uri }, position: { character: 5, line: 10 } }, + ) + response = server.pop_response.response + assert_equal(2, response.size) + assert_equal(2, response[0].range.start.line) + assert_equal(6, response[0].range.start.line) + end + end + def test_definition_for_instance_variables source = <<~RUBY class Foo From 37ba38ed46c9fe4c6d32fd7017661b947818e17b Mon Sep 17 00:00:00 2001 From: rogancodes Date: Sun, 1 Dec 2024 11:57:44 +0530 Subject: [PATCH 03/11] Fixed failing class variable definition test --- test/requests/definition_expectations_test.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/requests/definition_expectations_test.rb b/test/requests/definition_expectations_test.rb index 65b43dc27..5a7a8c0a1 100644 --- a/test/requests/definition_expectations_test.rb +++ b/test/requests/definition_expectations_test.rb @@ -770,7 +770,7 @@ def baz response = server.pop_response.response assert_equal(2, response.size) assert_equal(2, response[0].range.start.line) - assert_equal(6, response[0].range.start.line) + assert_equal(6, response[1].range.start.line) end end From 823ff6e85082f0419ab1b01c009e679642619157 Mon Sep 17 00:00:00 2001 From: rogancodes Date: Sun, 1 Dec 2024 12:05:16 +0530 Subject: [PATCH 04/11] support for class variables hover --- lib/ruby_lsp/listeners/hover.rb | 52 ++++++++++++++++++++++++ test/requests/hover_expectations_test.rb | 32 +++++++++++++++ 2 files changed, 84 insertions(+) diff --git a/lib/ruby_lsp/listeners/hover.rb b/lib/ruby_lsp/listeners/hover.rb index 5d1220f57..48dafae6e 100644 --- a/lib/ruby_lsp/listeners/hover.rb +++ b/lib/ruby_lsp/listeners/hover.rb @@ -31,6 +31,12 @@ class Hover Prism::SuperNode, Prism::ForwardingSuperNode, Prism::YieldNode, + Prism::ClassVariableAndWriteNode, + Prism::ClassVariableOperatorWriteNode, + Prism::ClassVariableOrWriteNode, + Prism::ClassVariableReadNode, + Prism::ClassVariableTargetNode, + Prism::ClassVariableWriteNode, ], T::Array[T.class_of(Prism::Node)], ) @@ -85,6 +91,12 @@ def initialize(response_builder, global_state, uri, node_context, dispatcher, so :on_string_node_enter, :on_interpolated_string_node_enter, :on_yield_node_enter, + :on_class_variable_and_write_node_enter, + :on_class_variable_operator_write_node_enter, + :on_class_variable_or_write_node_enter, + :on_class_variable_read_node_enter, + :on_class_variable_target_node_enter, + :on_class_variable_write_node_enter, ) end @@ -215,6 +227,36 @@ def on_yield_node_enter(node) handle_keyword_documentation(node.keyword) end + sig { params(node: Prism::ClassVariableAndWriteNode).void } + def on_class_variable_and_write_node_enter(node) + handle_class_variable_hover(node.name.to_s) + end + + sig { params(node: Prism::ClassVariableOperatorWriteNode).void } + def on_class_variable_operator_write_node_enter(node) + handle_class_variable_hover(node.name.to_s) + end + + sig { params(node: Prism::ClassVariableOrWriteNode).void } + def on_class_variable_or_write_node_enter(node) + handle_class_variable_hover(node.name.to_s) + end + + sig { params(node: Prism::ClassVariableTargetNode).void } + def on_class_variable_target_node_enter(node) + handle_class_variable_hover(node.name.to_s) + end + + sig { params(node: Prism::ClassVariableReadNode).void } + def on_class_variable_read_node_enter(node) + handle_class_variable_hover(node.name.to_s) + end + + sig { params(node: Prism::ClassVariableWriteNode).void } + def on_class_variable_write_node_enter(node) + handle_class_variable_hover(node.name.to_s) + end + private sig { params(node: T.any(Prism::InterpolatedStringNode, Prism::StringNode)).void } @@ -317,6 +359,16 @@ def handle_global_variable_hover(name) end end + sig { params(name: String).void } + def handle_class_variable_hover(name) + entries = @index[name] + return unless entries + + categorized_markdown_from_index_entries(name, entries).each do |category, content| + @response_builder.push(content, category: category) + end + end + sig { params(name: String, location: Prism::Location).void } def generate_hover(name, location) entries = @index.resolve(name, @node_context.nesting) diff --git a/test/requests/hover_expectations_test.rb b/test/requests/hover_expectations_test.rb index b43ae8709..f3ea3949f 100644 --- a/test/requests/hover_expectations_test.rb +++ b/test/requests/hover_expectations_test.rb @@ -430,6 +430,38 @@ def baz end end + def test_hovering_for_class_variables + source = <<~RUBY + class Foo + def foo + # or write node + @@a ||= 1 + end + + def bar + # operator write node + @@a += 5 + end + + def baz + @@a + end + end + RUBY + + with_server(source) do |server, uri| + server.process_message( + id: 1, + method: "textDocument/hover", + params: { textDocument: { uri: uri }, position: { character: 6, line: 12 } }, + ) + + contents = server.pop_response.response.contents.value + assert_match("or write node", contents) + assert_match("operator write node", contents) + end + end + def test_hovering_over_inherited_methods source = <<~RUBY module Foo From c6583c0f5ee016b09ffc84e32418b2bcc8bdb288 Mon Sep 17 00:00:00 2001 From: rogancodes Date: Sun, 1 Dec 2024 17:46:33 +0530 Subject: [PATCH 05/11] support for class variables completion --- lib/ruby_lsp/listeners/completion.rb | 59 ++++++++++++++++++++++++++++ lib/ruby_lsp/requests/completion.rb | 6 +++ test/requests/completion_test.rb | 59 ++++++++++++++++++++++++++++ 3 files changed, 124 insertions(+) diff --git a/lib/ruby_lsp/listeners/completion.rb b/lib/ruby_lsp/listeners/completion.rb index da976619d..242c2d95d 100644 --- a/lib/ruby_lsp/listeners/completion.rb +++ b/lib/ruby_lsp/listeners/completion.rb @@ -97,6 +97,12 @@ def initialize( # rubocop:disable Metrics/ParameterLists :on_instance_variable_operator_write_node_enter, :on_instance_variable_or_write_node_enter, :on_instance_variable_target_node_enter, + :on_class_variable_and_write_node_enter, + :on_class_variable_operator_write_node_enter, + :on_class_variable_or_write_node_enter, + :on_class_variable_read_node_enter, + :on_class_variable_target_node_enter, + :on_class_variable_write_node_enter, ) end @@ -246,6 +252,36 @@ def on_instance_variable_target_node_enter(node) handle_instance_variable_completion(node.name.to_s, node.location) end + sig { params(node: Prism::ClassVariableAndWriteNode).void } + def on_class_variable_and_write_node_enter(node) + handle_class_variable_completion(node.name.to_s, node.name_loc) + end + + sig { params(node: Prism::ClassVariableOperatorWriteNode).void } + def on_class_variable_operator_write_node_enter(node) + handle_class_variable_completion(node.name.to_s, node.name_loc) + end + + sig { params(node: Prism::ClassVariableOrWriteNode).void } + def on_class_variable_or_write_node_enter(node) + handle_class_variable_completion(node.name.to_s, node.name_loc) + end + + sig { params(node: Prism::ClassVariableTargetNode).void } + def on_class_variable_target_node_enter(node) + handle_class_variable_completion(node.name.to_s, node.location) + end + + sig { params(node: Prism::ClassVariableReadNode).void } + def on_class_variable_read_node_enter(node) + handle_class_variable_completion(node.name.to_s, node.location) + end + + sig { params(node: Prism::ClassVariableWriteNode).void } + def on_class_variable_write_node_enter(node) + handle_class_variable_completion(node.name.to_s, node.name_loc) + end + private sig { params(name: String, range: Interface::Range).void } @@ -326,6 +362,29 @@ def handle_global_variable_completion(name, location) end end + sig { params(name: String, location: Prism::Location).void } + def handle_class_variable_completion(name, location) + candidates = @index.prefix_search(name) + + return if candidates.none? + + range = range_from_location(location) + + candidates.flatten.uniq(&:name).each do |entry| + entry_name = entry.name + + @response_builder << Interface::CompletionItem.new( + label: entry_name, + filter_text: entry_name, + label_details: Interface::CompletionItemLabelDetails.new( + description: entry.file_name, + ), + text_edit: Interface::TextEdit.new(range: range, new_text: entry_name), + kind: Constant::CompletionItemKind::FIELD, + ) + end + end + sig { params(name: String, location: Prism::Location).void } def handle_instance_variable_completion(name, location) # Sorbet enforces that all instance variables be declared on typed strict or higher, which means it will be able diff --git a/lib/ruby_lsp/requests/completion.rb b/lib/ruby_lsp/requests/completion.rb index 5ed77c0df..68258da91 100644 --- a/lib/ruby_lsp/requests/completion.rb +++ b/lib/ruby_lsp/requests/completion.rb @@ -62,6 +62,12 @@ def initialize(document, global_state, params, sorbet_level, dispatcher) Prism::InstanceVariableOrWriteNode, Prism::InstanceVariableTargetNode, Prism::InstanceVariableWriteNode, + Prism::ClassVariableAndWriteNode, + Prism::ClassVariableOperatorWriteNode, + Prism::ClassVariableOrWriteNode, + Prism::ClassVariableReadNode, + Prism::ClassVariableTargetNode, + Prism::ClassVariableWriteNode, ], code_units_cache: document.code_units_cache, ) diff --git a/test/requests/completion_test.rb b/test/requests/completion_test.rb index 8dff7f507..7efcb03b7 100644 --- a/test/requests/completion_test.rb +++ b/test/requests/completion_test.rb @@ -1087,6 +1087,65 @@ def test_completion_for_global_variables_show_only_uniq_entries end end + def test_completion_for_class_variables + source = <<~RUBY + class Foo + def set_variables + @@foo = 1 + @@foobar = 2 + end + + def bar + @ + end + + def baz + @@ = 123 + end + end + RUBY + + with_server(source, stub_no_typechecker: true) do |server, uri| + server.process_message(id: 1, method: "textDocument/completion", params: { + textDocument: { uri: uri }, + position: { line: 7, character: 5 }, + }) + + result = server.pop_response.response + assert_equal(["@@foo", "@@foobar"], result.map(&:label)) + assert_equal(["fake.rb", "fake.rb"], result.map { _1.label_details.description }) + end + end + + def test_completion_for_class_variables_show_only_uniq_entries + source = <<~RUBY + class Foo + def set_variables + @@foo = 1 + @@foo = 2 + end + + def bar + @ + end + + def baz + @@ = 123 + end + end + RUBY + + with_server(source, stub_no_typechecker: true) do |server, uri| + server.process_message(id: 1, method: "textDocument/completion", params: { + textDocument: { uri: uri }, + position: { line: 7, character: 5 }, + }) + + result = server.pop_response.response + assert_equal(["@@foo"], result.map(&:label)) + end + end + def test_completion_for_instance_variables source = +<<~RUBY class Foo From fd97ea1a0529f49399e457821813209c502b631b Mon Sep 17 00:00:00 2001 From: rogancodes Date: Sun, 1 Dec 2024 19:50:56 +0530 Subject: [PATCH 06/11] provide hover, completion, go to definition for class variable based on receiver similiar to instance variable --- lib/ruby_indexer/lib/ruby_indexer/index.rb | 21 ++++++++++ lib/ruby_lsp/listeners/completion.rb | 28 +++++++++----- lib/ruby_lsp/listeners/definition.rb | 6 ++- lib/ruby_lsp/listeners/hover.rb | 7 +++- lib/ruby_lsp/type_inferrer.rb | 4 +- test/requests/completion_test.rb | 34 +++++++++++++++++ test/requests/definition_expectations_test.rb | 36 ++++++++++++++++++ test/requests/hover_expectations_test.rb | 38 +++++++++++++++++++ 8 files changed, 161 insertions(+), 13 deletions(-) diff --git a/lib/ruby_indexer/lib/ruby_indexer/index.rb b/lib/ruby_indexer/lib/ruby_indexer/index.rb index 3c60163e3..d7225625f 100644 --- a/lib/ruby_indexer/lib/ruby_indexer/index.rb +++ b/lib/ruby_indexer/lib/ruby_indexer/index.rb @@ -584,6 +584,17 @@ def resolve_instance_variable(variable_name, owner_name) entries.select { |e| ancestors.include?(e.owner&.name) } end + sig { params(variable_name: String, owner_name: String).returns(T.nilable(T::Array[Entry::ClassVariable])) } + def resolve_class_variable(variable_name, owner_name) + entries = T.cast(self[variable_name], T.nilable(T::Array[Entry::ClassVariable])) + return unless entries + + ancestors = linearized_ancestors_of(owner_name) + return if ancestors.empty? + + entries.select { |e| ancestors.include?(e.owner&.name) } + end + # Returns a list of possible candidates for completion of instance variables for a given owner name. The name must # include the `@` prefix sig { params(name: String, owner_name: String).returns(T::Array[Entry::InstanceVariable]) } @@ -596,6 +607,16 @@ def instance_variable_completion_candidates(name, owner_name) variables end + sig { params(name: String, owner_name: String).returns(T::Array[Entry::ClassVariable]) } + def class_variable_completion_candidates(name, owner_name) + entries = T.cast(prefix_search(name).flatten, T::Array[Entry::ClassVariable]) + ancestors = linearized_ancestors_of(owner_name) + + variables = entries.select { |e| ancestors.any?(e.owner&.name) } + variables.uniq!(&:name) + variables + end + # Synchronizes a change made to the given URI. This method will ensure that new declarations are indexed, removed # declarations removed and that the ancestor linearization cache is cleared if necessary sig { params(uri: URI::Generic, source: String).void } diff --git a/lib/ruby_lsp/listeners/completion.rb b/lib/ruby_lsp/listeners/completion.rb index 242c2d95d..d78dbb3da 100644 --- a/lib/ruby_lsp/listeners/completion.rb +++ b/lib/ruby_lsp/listeners/completion.rb @@ -364,25 +364,33 @@ def handle_global_variable_completion(name, location) sig { params(name: String, location: Prism::Location).void } def handle_class_variable_completion(name, location) - candidates = @index.prefix_search(name) - - return if candidates.none? + type = @type_inferrer.infer_receiver_type(@node_context) + return unless type range = range_from_location(location) - candidates.flatten.uniq(&:name).each do |entry| - entry_name = entry.name + @index.class_variable_completion_candidates(name, type.name).each do |entry| + variable_name = entry.name + + label_details = Interface::CompletionItemLabelDetails.new( + description: entry.file_name, + ) @response_builder << Interface::CompletionItem.new( - label: entry_name, - filter_text: entry_name, - label_details: Interface::CompletionItemLabelDetails.new( - description: entry.file_name, + label: variable_name, + label_details: label_details, + text_edit: Interface::TextEdit.new( + range: range, + new_text: variable_name, ), - text_edit: Interface::TextEdit.new(range: range, new_text: entry_name), kind: Constant::CompletionItemKind::FIELD, + data: { + owner_name: entry.owner&.name, + }, ) end + rescue RubyIndexer::Index::NonExistingNamespaceError + # If by any chance we haven't indexed the owner, then there's no way to find the right declaration end sig { params(name: String, location: Prism::Location).void } diff --git a/lib/ruby_lsp/listeners/definition.rb b/lib/ruby_lsp/listeners/definition.rb index daf939f58..b3a3d20c6 100644 --- a/lib/ruby_lsp/listeners/definition.rb +++ b/lib/ruby_lsp/listeners/definition.rb @@ -270,8 +270,10 @@ def handle_global_variable_definition(name) sig { params(name: String).void } def handle_class_variable_definition(name) - entries = @index[name] + type = @type_inferrer.infer_receiver_type(@node_context) + return unless type + entries = @index.resolve_class_variable(name, type.name) return unless entries entries.each do |entry| @@ -285,6 +287,8 @@ def handle_class_variable_definition(name) ), ) end + rescue RubyIndexer::Index::NonExistingNamespaceError + # If by any chance we haven't indexed the owner, then there's no way to find the right declaration end sig { params(name: String).void } diff --git a/lib/ruby_lsp/listeners/hover.rb b/lib/ruby_lsp/listeners/hover.rb index 48dafae6e..dcf523a15 100644 --- a/lib/ruby_lsp/listeners/hover.rb +++ b/lib/ruby_lsp/listeners/hover.rb @@ -361,12 +361,17 @@ def handle_global_variable_hover(name) sig { params(name: String).void } def handle_class_variable_hover(name) - entries = @index[name] + type = @type_inferrer.infer_receiver_type(@node_context) + return unless type + + entries = @index.resolve_class_variable(name, type.name) return unless entries categorized_markdown_from_index_entries(name, entries).each do |category, content| @response_builder.push(content, category: category) end + rescue RubyIndexer::Index::NonExistingNamespaceError + # If by any chance we haven't indexed the owner, then there's no way to find the right declaration end sig { params(name: String, location: Prism::Location).void } diff --git a/lib/ruby_lsp/type_inferrer.rb b/lib/ruby_lsp/type_inferrer.rb index cd1af202c..628cd5a17 100644 --- a/lib/ruby_lsp/type_inferrer.rb +++ b/lib/ruby_lsp/type_inferrer.rb @@ -21,7 +21,9 @@ def infer_receiver_type(node_context) infer_receiver_for_call_node(node, node_context) when Prism::InstanceVariableReadNode, Prism::InstanceVariableAndWriteNode, Prism::InstanceVariableWriteNode, Prism::InstanceVariableOperatorWriteNode, Prism::InstanceVariableOrWriteNode, Prism::InstanceVariableTargetNode, - Prism::SuperNode, Prism::ForwardingSuperNode + Prism::SuperNode, Prism::ForwardingSuperNode, Prism::ClassVariableAndWriteNode, Prism::ClassVariableWriteNode, + Prism::ClassVariableOperatorWriteNode, Prism::ClassVariableOrWriteNode, Prism::ClassVariableReadNode, + Prism::ClassVariableTargetNode self_receiver_handling(node_context) end end diff --git a/test/requests/completion_test.rb b/test/requests/completion_test.rb index 7efcb03b7..62a0e43d1 100644 --- a/test/requests/completion_test.rb +++ b/test/requests/completion_test.rb @@ -1117,6 +1117,40 @@ def baz end end + def test_completion_for_inherited_class_variables + source = <<~RUBY + module Foo + def set_variable + @@bar = 9 + end + end + + class Parent + def set_variable + @@baz = 5 + end + end + + class Child < Parent + include Foo + + def do_something + @ + end + end + RUBY + + with_server(source, stub_no_typechecker: true) do |server, uri| + server.process_message(id: 1, method: "textDocument/completion", params: { + textDocument: { uri: uri }, + position: { line: 16, character: 5 }, + }) + + result = server.pop_response.response + assert_equal(["@@bar", "@@baz"], result.map(&:label)) + end + end + def test_completion_for_class_variables_show_only_uniq_entries source = <<~RUBY class Foo diff --git a/test/requests/definition_expectations_test.rb b/test/requests/definition_expectations_test.rb index 5a7a8c0a1..ff125d66d 100644 --- a/test/requests/definition_expectations_test.rb +++ b/test/requests/definition_expectations_test.rb @@ -774,6 +774,42 @@ def baz end end + def test_definition_for_inherited_class_variables + source = <<~RUBY + module Foo + def set_variable + @@bar = 1 + end + end + + class Parent + def set_variable + @@bar = 5 + end + end + + class Child < Parent + include Foo + + def do_something + @@bar + end + end + RUBY + + with_server(source) do |server, uri| + server.process_message( + id: 1, + method: "textDocument/definition", + params: { textDocument: { uri: uri }, position: { character: 4, line: 16 } }, + ) + response = server.pop_response.response + + assert_equal(2, response[0].range.start.line) + assert_equal(8, response[1].range.start.line) + end + end + def test_definition_for_instance_variables source = <<~RUBY class Foo diff --git a/test/requests/hover_expectations_test.rb b/test/requests/hover_expectations_test.rb index f3ea3949f..d8086f72e 100644 --- a/test/requests/hover_expectations_test.rb +++ b/test/requests/hover_expectations_test.rb @@ -462,6 +462,44 @@ def baz end end + def test_hovering_for_inherited_class_variables + source = <<~RUBY + module Foo + def set_variable + # Foo + @@bar = 1 + end + end + + class Parent + def set_variable + # Parent + @@bar = 5 + end + end + + class Child < Parent + include Foo + + def do_something + @@bar + end + end + RUBY + + with_server(source) do |server, uri| + server.process_message( + id: 1, + method: "textDocument/hover", + params: { textDocument: { uri: uri }, position: { character: 4, line: 18 } }, + ) + + contents = server.pop_response.response.contents.value + assert_match("Foo", contents) + assert_match("Parent", contents) + end + end + def test_hovering_over_inherited_methods source = <<~RUBY module Foo From f48298af9b2dcda1687bcb9eac6172103f86c2a3 Mon Sep 17 00:00:00 2001 From: rogancodes Date: Tue, 17 Dec 2024 13:37:41 +0530 Subject: [PATCH 07/11] Set class variable owner to attached context in singleton scope during indexing --- .../lib/ruby_indexer/declaration_listener.rb | 6 +++ lib/ruby_indexer/test/class_variables_test.rb | 42 ++++++++++++++++--- 2 files changed, 43 insertions(+), 5 deletions(-) diff --git a/lib/ruby_indexer/lib/ruby_indexer/declaration_listener.rb b/lib/ruby_indexer/lib/ruby_indexer/declaration_listener.rb index e11a0e780..8e1a1c9bd 100644 --- a/lib/ruby_indexer/lib/ruby_indexer/declaration_listener.rb +++ b/lib/ruby_indexer/lib/ruby_indexer/declaration_listener.rb @@ -599,8 +599,14 @@ def handle_class_variable(node, loc) return if name == "@@" comments = collect_comments(node) + owner = @owner_stack.last + # set the class variable's owner to the attached context when defined within a singleton scope. + if owner.is_a?(Entry::SingletonClass) + owner = @owner_stack.reverse.find { |entry| !entry.name.include?("", owner.name) + assert_instance_of(Entry::Class, owner) + assert_equal("Foo", owner.name) + end + + def test_class_variable_inside_singleton_class + index(<<~RUBY) + class Foo + class << self + @@bar = 123 + end + end + RUBY + + entry = T.must(@index["@@bar"]&.first) + owner = T.must(entry.owner) + assert_instance_of(Entry::Class, owner) + assert_equal("Foo", owner.name) + end + + def test_class_variable_in_singleton_class_method + index(<<~RUBY) + class Foo + class << self + def self.bar + @@bar = 123 + end + end + end + RUBY + + entry = T.must(@index["@@bar"]&.first) + owner = T.must(entry.owner) + assert_instance_of(Entry::Class, owner) + assert_equal("Foo", owner.name) end end end From 80a759542c25b328015feac97869665000db8917 Mon Sep 17 00:00:00 2001 From: rogancodes Date: Tue, 17 Dec 2024 16:28:53 +0530 Subject: [PATCH 08/11] added support for type inference of class variables --- lib/ruby_lsp/type_inferrer.rb | 26 ++++++++++++-- test/type_inferrer_test.rb | 68 +++++++++++++++++++++++++++++++++++ 2 files changed, 91 insertions(+), 3 deletions(-) diff --git a/lib/ruby_lsp/type_inferrer.rb b/lib/ruby_lsp/type_inferrer.rb index 628cd5a17..85732c9af 100644 --- a/lib/ruby_lsp/type_inferrer.rb +++ b/lib/ruby_lsp/type_inferrer.rb @@ -21,10 +21,11 @@ def infer_receiver_type(node_context) infer_receiver_for_call_node(node, node_context) when Prism::InstanceVariableReadNode, Prism::InstanceVariableAndWriteNode, Prism::InstanceVariableWriteNode, Prism::InstanceVariableOperatorWriteNode, Prism::InstanceVariableOrWriteNode, Prism::InstanceVariableTargetNode, - Prism::SuperNode, Prism::ForwardingSuperNode, Prism::ClassVariableAndWriteNode, Prism::ClassVariableWriteNode, - Prism::ClassVariableOperatorWriteNode, Prism::ClassVariableOrWriteNode, Prism::ClassVariableReadNode, - Prism::ClassVariableTargetNode + Prism::SuperNode, Prism::ForwardingSuperNode self_receiver_handling(node_context) + when Prism::ClassVariableAndWriteNode, Prism::ClassVariableWriteNode, Prism::ClassVariableOperatorWriteNode, + Prism::ClassVariableOrWriteNode, Prism::ClassVariableReadNode,Prism::ClassVariableTargetNode + infer_receiver_for_class_variables(node_context) end end @@ -145,6 +146,25 @@ def constant_name(node) nil end + sig { params(node_context: NodeContext).returns(T.nilable(Type)) } + def infer_receiver_for_class_variables(node_context) + nesting_parts = node_context.nesting + + return Type.new("Object") if nesting_parts.empty? + + nesting_parts.reverse_each do |part| + break unless part.include?("", @type_inferrer.infer_receiver_type(node_context).name) end + def test_infer_receiver_type_class_variables_in_class_body + node_context = index_and_locate(<<~RUBY, { line: 1, character: 2 }) + class Foo + @@hello1 + end + RUBY + + assert_equal("Foo", @type_inferrer.infer_receiver_type(node_context).name) + end + + def test_infer_receiver_type_class_variables_in_singleton_method + node_context = index_and_locate(<<~RUBY, { line: 2, character: 4 }) + class Foo + def self.bar + @@hello1 + end + end + RUBY + + assert_equal("Foo", @type_inferrer.infer_receiver_type(node_context).name) + end + + def test_infer_receiver_type_class_variables_in_singleton_block_body + node_context = index_and_locate(<<~RUBY, { line: 2, character: 4 }) + class Foo + class << self + @@hello1 + end + end + RUBY + + assert_equal("Foo", @type_inferrer.infer_receiver_type(node_context).name) + end + + def test_infer_receiver_type_class_variables_in_singleton_block_method + node_context = index_and_locate(<<~RUBY, { line: 3, character: 6 }) + class Foo + class << self + def bar + @@hello1 + end + end + end + RUBY + + assert_equal("Foo", @type_inferrer.infer_receiver_type(node_context).name) + end + + def test_infer_receiver_type_class_variables_in_instance_method + node_context = index_and_locate(<<~RUBY, { line: 2, character: 4 }) + class Foo + def bar + @@hello1 + end + end + RUBY + + assert_equal("Foo", @type_inferrer.infer_receiver_type(node_context).name) + end + + def test_infer_top_level_class_variables + node_context = index_and_locate(<<~RUBY, { line: 0, character: 0 }) + @@foo + RUBY + + assert_equal("Object", @type_inferrer.infer_receiver_type(node_context).name) + end + private def index_and_locate(source, position) From 28f2c681ac6d93a4aff41e514bf0a443333e5c7a Mon Sep 17 00:00:00 2001 From: rogancodes Date: Tue, 17 Dec 2024 17:27:04 +0530 Subject: [PATCH 09/11] added test cases for hover, definition, and completion of class variables in different contexts --- lib/ruby_lsp/type_inferrer.rb | 2 +- test/requests/completion_test.rb | 38 ++++++++++++++++ test/requests/definition_expectations_test.rb | 39 ++++++++++++++++ test/requests/hover_expectations_test.rb | 44 +++++++++++++++++++ 4 files changed, 122 insertions(+), 1 deletion(-) diff --git a/lib/ruby_lsp/type_inferrer.rb b/lib/ruby_lsp/type_inferrer.rb index 85732c9af..d8f045b3f 100644 --- a/lib/ruby_lsp/type_inferrer.rb +++ b/lib/ruby_lsp/type_inferrer.rb @@ -24,7 +24,7 @@ def infer_receiver_type(node_context) Prism::SuperNode, Prism::ForwardingSuperNode self_receiver_handling(node_context) when Prism::ClassVariableAndWriteNode, Prism::ClassVariableWriteNode, Prism::ClassVariableOperatorWriteNode, - Prism::ClassVariableOrWriteNode, Prism::ClassVariableReadNode,Prism::ClassVariableTargetNode + Prism::ClassVariableOrWriteNode, Prism::ClassVariableReadNode, Prism::ClassVariableTargetNode infer_receiver_for_class_variables(node_context) end end diff --git a/test/requests/completion_test.rb b/test/requests/completion_test.rb index 62a0e43d1..5eeda13dd 100644 --- a/test/requests/completion_test.rb +++ b/test/requests/completion_test.rb @@ -1180,6 +1180,44 @@ def baz end end + def test_completion_for_class_variables_in_different_context + source = <<~RUBY + class Foo + @@a = 1 + + class << self + @@b = 2 + + def foo + @@c = 3 + end + end + + def bar + @ + end + + def baz + @@ = 4 + end + + def self.foobar + @@d = 5 + end + end + RUBY + + with_server(source, stub_no_typechecker: true) do |server, uri| + server.process_message(id: 1, method: "textDocument/completion", params: { + textDocument: { uri: uri }, + position: { line: 12, character: 5 }, + }) + + result = server.pop_response.response + assert_equal(["@@a", "@@b", "@@c", "@@d"], result.map(&:label)) + end + end + def test_completion_for_instance_variables source = +<<~RUBY class Foo diff --git a/test/requests/definition_expectations_test.rb b/test/requests/definition_expectations_test.rb index ff125d66d..eb703e89c 100644 --- a/test/requests/definition_expectations_test.rb +++ b/test/requests/definition_expectations_test.rb @@ -810,6 +810,45 @@ def do_something end end + def test_definition_for_class_variables_in_different_context + source = <<~RUBY + class Foo + @@a = 1 + + class << self + @@a = 2 + + def foo + @@a = 3 + end + end + + def bar + @@a = 4 + end + + def self.baz + @@a = 5 + end + end + RUBY + + with_server(source) do |server, uri| + server.process_message( + id: 1, + method: "textDocument/definition", + params: { textDocument: { uri: uri }, position: { character: 4, line: 1 } }, + ) + response = server.pop_response.response + + assert_equal(1, response[0].range.start.line) + assert_equal(4, response[1].range.start.line) + assert_equal(7, response[2].range.start.line) + assert_equal(12, response[3].range.start.line) + assert_equal(16, response[4].range.start.line) + end + end + def test_definition_for_instance_variables source = <<~RUBY class Foo diff --git a/test/requests/hover_expectations_test.rb b/test/requests/hover_expectations_test.rb index d8086f72e..cadd6892a 100644 --- a/test/requests/hover_expectations_test.rb +++ b/test/requests/hover_expectations_test.rb @@ -500,6 +500,50 @@ def do_something end end + def test_hovering_for_class_variables_in_different_context + source = <<~RUBY + class Foo + # comment 1 + @@a = 1 + + class << self + # comment 2 + @@a = 2 + + def foo + # comment 3 + @@a = 3 + end + end + + def bar + # comment 4 + @@a = 4 + end + + def self.baz + # comment 5 + @@a = 5 + end + end + RUBY + + with_server(source) do |server, uri| + server.process_message( + id: 1, + method: "textDocument/hover", + params: { textDocument: { uri: uri }, position: { character: 4, line: 2 } }, + ) + + contents = server.pop_response.response.contents.value + assert_match("comment 1", contents) + assert_match("comment 2", contents) + assert_match("comment 3", contents) + assert_match("comment 4", contents) + assert_match("comment 5", contents) + end + end + def test_hovering_over_inherited_methods source = <<~RUBY module Foo From 358972db53c126bb67e672dea50d0d83c6fc3bae Mon Sep 17 00:00:00 2001 From: rogancodes Date: Tue, 17 Dec 2024 17:36:42 +0530 Subject: [PATCH 10/11] changes as per review comment --- lib/ruby_indexer/lib/ruby_indexer/index.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/ruby_indexer/lib/ruby_indexer/index.rb b/lib/ruby_indexer/lib/ruby_indexer/index.rb index d7225625f..fbfa578c8 100644 --- a/lib/ruby_indexer/lib/ruby_indexer/index.rb +++ b/lib/ruby_indexer/lib/ruby_indexer/index.rb @@ -586,8 +586,8 @@ def resolve_instance_variable(variable_name, owner_name) sig { params(variable_name: String, owner_name: String).returns(T.nilable(T::Array[Entry::ClassVariable])) } def resolve_class_variable(variable_name, owner_name) - entries = T.cast(self[variable_name], T.nilable(T::Array[Entry::ClassVariable])) - return unless entries + entries = self[variable_name]&.grep(Entry::ClassVariable) + return unless entries&.any? ancestors = linearized_ancestors_of(owner_name) return if ancestors.empty? From 6afa177d948637a63a981bc16e7edf2711c3d519 Mon Sep 17 00:00:00 2001 From: rogancodes Date: Wed, 18 Dec 2024 08:27:38 +0530 Subject: [PATCH 11/11] added a comment on declaration listener and renamed a completion test definition --- lib/ruby_indexer/lib/ruby_indexer/declaration_listener.rb | 2 ++ test/requests/completion_test.rb | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/lib/ruby_indexer/lib/ruby_indexer/declaration_listener.rb b/lib/ruby_indexer/lib/ruby_indexer/declaration_listener.rb index 8e1a1c9bd..8f63c1b48 100644 --- a/lib/ruby_indexer/lib/ruby_indexer/declaration_listener.rb +++ b/lib/ruby_indexer/lib/ruby_indexer/declaration_listener.rb @@ -596,6 +596,8 @@ def handle_global_variable(node, loc) end def handle_class_variable(node, loc) name = node.name.to_s + # Ignore incomplete class variable names, which aren't valid Ruby syntax. + # This could occur if the code is in an incomplete or temporary state. return if name == "@@" comments = collect_comments(node) diff --git a/test/requests/completion_test.rb b/test/requests/completion_test.rb index 5eeda13dd..90e391e08 100644 --- a/test/requests/completion_test.rb +++ b/test/requests/completion_test.rb @@ -1151,7 +1151,7 @@ def do_something end end - def test_completion_for_class_variables_show_only_uniq_entries + def test_completion_for_class_variables_show_only_unique_entries source = <<~RUBY class Foo def set_variables