From 9ef56fcf1f72efe9e2087984e86c45497fabfbc5 Mon Sep 17 00:00:00 2001 From: Vinicius Stock Date: Wed, 17 Apr 2024 13:41:53 -0400 Subject: [PATCH] Split constant and method entries --- .../lib/ruby_indexer/collector.rb | 2 +- lib/ruby_indexer/lib/ruby_indexer/index.rb | 106 +++++++++++++----- .../test/classes_and_modules_test.rb | 44 ++++---- lib/ruby_indexer/test/constant_test.rb | 60 +++++----- lib/ruby_indexer/test/index_test.rb | 40 +++---- lib/ruby_indexer/test/method_test.rb | 32 +++--- lib/ruby_indexer/test/test_case.rb | 10 +- lib/ruby_lsp/listeners/completion.rb | 14 +-- lib/ruby_lsp/listeners/definition.rb | 4 +- lib/ruby_lsp/listeners/hover.rb | 2 +- lib/ruby_lsp/requests/completion_resolve.rb | 9 +- test/requests/completion_resolve_test.rb | 3 +- test/server_test.rb | 2 +- 13 files changed, 196 insertions(+), 132 deletions(-) diff --git a/lib/ruby_indexer/lib/ruby_indexer/collector.rb b/lib/ruby_indexer/lib/ruby_indexer/collector.rb index dab6b359d..ad004482c 100644 --- a/lib/ruby_indexer/lib/ruby_indexer/collector.rb +++ b/lib/ruby_indexer/lib/ruby_indexer/collector.rb @@ -206,7 +206,7 @@ def handle_private_constant(node) # The private_constant method does not resolve the constant name. It always points to a constant that needs to # exist in the current namespace - entries = @index[fully_qualify_name(name)] + entries = @index.get_constant(fully_qualify_name(name)) entries&.each { |entry| entry.visibility = :private } end diff --git a/lib/ruby_indexer/lib/ruby_indexer/index.rb b/lib/ruby_indexer/lib/ruby_indexer/index.rb index 7228be0d9..2568c5d40 100644 --- a/lib/ruby_indexer/lib/ruby_indexer/index.rb +++ b/lib/ruby_indexer/lib/ruby_indexer/index.rb @@ -12,15 +12,25 @@ class UnresolvableAliasError < StandardError; end sig { void } def initialize - # Holds all entries in the index using the following format: + # Holds all constant entries in the index using the following format: # { # "Foo" => [#, #], # "Foo::Bar" => [#], # } - @entries = T.let({}, T::Hash[String, T::Array[Entry]]) + @constant_entries = T.let({}, T::Hash[String, T::Array[Entry]]) # Holds all entries in the index using a prefix tree for searching based on prefixes to provide autocompletion - @entries_tree = T.let(PrefixTree[T::Array[Entry]].new, PrefixTree[T::Array[Entry]]) + @constant_entries_tree = T.let(PrefixTree[T::Array[Entry]].new, PrefixTree[T::Array[Entry]]) + + # Holds all method entries in the index using the following format: + # { + # "method_name" => [#, #], + # "foo" => [#], + # } + @method_entries = T.let({}, T::Hash[String, T::Array[Entry]]) + + # Holds all entries in the index using a prefix tree for searching based on prefixes to provide autocompletion + @method_entries_tree = T.let(PrefixTree[T::Array[Entry]].new, PrefixTree[T::Array[Entry]]) # Holds references to where entries where discovered so that we can easily delete them # { @@ -38,8 +48,16 @@ def delete(indexable) # For each constant discovered in `path`, delete the associated entry from the index. If there are no entries # left, delete the constant from the index. @files_to_entries[indexable.full_path]&.each do |entry| + if entry.is_a?(Entry::Member) + entry_set = @method_entries + entry_tree = @method_entries_tree + else + entry_set = @constant_entries + entry_tree = @constant_entries_tree + end + name = entry.name - entries = @entries[name] + entries = entry_set[name] next unless entries # Delete the specific entry from the list for this name @@ -48,10 +66,10 @@ def delete(indexable) # If all entries were deleted, then remove the name from the hash and from the prefix tree. Otherwise, update # the prefix tree with the current entries if entries.empty? - @entries.delete(name) - @entries_tree.delete(name) + entry_set.delete(name) + entry_tree.delete(name) else - @entries_tree.insert(name, entries) + entry_tree.insert(name, entries) end end @@ -64,15 +82,28 @@ def delete(indexable) sig { params(entry: Entry).void } def <<(entry) name = entry.name - - (@entries[name] ||= []) << entry (@files_to_entries[entry.file_path] ||= []) << entry - @entries_tree.insert(name, T.must(@entries[name])) + + if entry.is_a?(Entry::Member) + entry_set = @method_entries + entry_tree = @method_entries_tree + else + entry_set = @constant_entries + entry_tree = @constant_entries_tree + end + + (entry_set[name] ||= []) << entry + entry_tree.insert(name, T.must(entry_set[name])) end sig { params(fully_qualified_name: String).returns(T.nilable(T::Array[Entry])) } - def [](fully_qualified_name) - @entries[fully_qualified_name.delete_prefix("::")] + def get_constant(fully_qualified_name) + @constant_entries[fully_qualified_name.delete_prefix("::")] + end + + sig { params(name: String).returns(T.nilable(T::Array[Entry])) } + def get_method(name) + @method_entries[name] end sig { params(query: String).returns(T::Array[IndexablePath]) } @@ -94,9 +125,27 @@ def search_require_paths(query) # ] # ``` sig { params(query: String, nesting: T.nilable(T::Array[String])).returns(T::Array[T::Array[Entry]]) } - def prefix_search(query, nesting = nil) + def prefix_search_constants(query, nesting = nil) + unless nesting + results = @constant_entries_tree.search(query) + results.uniq! + return results + end + + results = nesting.length.downto(0).flat_map do |i| + prefix = T.must(nesting[0...i]).join("::") + namespaced_query = prefix.empty? ? query : "#{prefix}::#{query}" + @constant_entries_tree.search(namespaced_query) + end + + results.uniq! + results + end + + sig { params(query: String, nesting: T.nilable(T::Array[String])).returns(T::Array[T::Array[Entry]]) } + def prefix_search_methods(query, nesting = nil) unless nesting - results = @entries_tree.search(query) + results = @method_entries_tree.search(query) results.uniq! return results end @@ -104,7 +153,7 @@ def prefix_search(query, nesting = nil) results = nesting.length.downto(0).flat_map do |i| prefix = T.must(nesting[0...i]).join("::") namespaced_query = prefix.empty? ? query : "#{prefix}::#{query}" - @entries_tree.search(namespaced_query) + @method_entries_tree.search(namespaced_query) end results.uniq! @@ -114,11 +163,14 @@ def prefix_search(query, nesting = nil) # Fuzzy searches index entries based on Jaro-Winkler similarity. If no query is provided, all entries are returned sig { params(query: T.nilable(String)).returns(T::Array[Entry]) } def fuzzy_search(query) - return @entries.flat_map { |_name, entries| entries } unless query + unless query + constants = @constant_entries.flat_map { |_name, entries| entries } + return constants + @method_entries.flat_map { |_name, entries| entries } + end normalized_query = query.gsub("::", "").downcase - results = @entries.filter_map do |name, entries| + results = @constant_entries.merge(@method_entries).filter_map do |name, entries| similarity = DidYouMean::JaroWinkler.distance(name.gsub("::", "").downcase, normalized_query) [entries, -similarity] if similarity > ENTRY_SIMILARITY_THRESHOLD end @@ -132,10 +184,10 @@ def fuzzy_search(query) # 2. Foo::Baz # 3. Baz sig { params(name: String, nesting: T::Array[String]).returns(T.nilable(T::Array[Entry])) } - def resolve(name, nesting) + def resolve_constant(name, nesting) if name.start_with?("::") name = name.delete_prefix("::") - results = @entries[name] || @entries[follow_aliased_namespace(name)] + results = @constant_entries[name] || @constant_entries[follow_aliased_namespace(name)] return results&.map { |e| e.is_a?(Entry::UnresolvedAlias) ? resolve_alias(e) : e } end @@ -150,7 +202,7 @@ def resolve(name, nesting) # the LSP itself we alias `RubyLsp::Interface` to `LanguageServer::Protocol::Interface`, which means doing # `RubyLsp::Interface::Location` is allowed. For these cases, we need some way to realize that the # `RubyLsp::Interface` part is an alias, that has to be resolved - entries = @entries[full_name] || @entries[follow_aliased_namespace(full_name)] + entries = @constant_entries[full_name] || @constant_entries[follow_aliased_namespace(full_name)] return entries.map { |e| e.is_a?(Entry::UnresolvedAlias) ? resolve_alias(e) : e } if entries end @@ -208,14 +260,14 @@ def index_single(indexable_path, source = nil) # aliases, so we have to invoke `follow_aliased_namespace` again to check until we only return a real name sig { params(name: String).returns(String) } def follow_aliased_namespace(name) - return name if @entries[name] + return name if @constant_entries[name] parts = name.split("::") real_parts = [] (parts.length - 1).downto(0).each do |i| current_name = T.must(parts[0..i]).join("::") - entry = @entries[current_name]&.first + entry = @constant_entries[current_name]&.first case entry when Entry::Alias @@ -242,8 +294,8 @@ def follow_aliased_namespace(name) # Returns `nil` if the method does not exist on that receiver sig { params(method_name: String, receiver_name: String).returns(T.nilable(T::Array[Entry::Member])) } def resolve_method(method_name, receiver_name) - method_entries = self[method_name] - owner_entries = self[receiver_name] + method_entries = get_method(method_name) + owner_entries = get_constant(receiver_name) return unless owner_entries && method_entries owner_name = T.must(owner_entries.first).name @@ -261,18 +313,18 @@ def resolve_method(method_name, receiver_name) # that doesn't exist, then we return the same UnresolvedAlias sig { params(entry: Entry::UnresolvedAlias).returns(T.any(Entry::Alias, Entry::UnresolvedAlias)) } def resolve_alias(entry) - target = resolve(entry.target, entry.nesting) + target = resolve_constant(entry.target, entry.nesting) return entry unless target target_name = T.must(target.first).name resolved_alias = Entry::Alias.new(target_name, entry) # Replace the UnresolvedAlias by a resolved one so that we don't have to do this again later - original_entries = T.must(@entries[entry.name]) + original_entries = T.must(@constant_entries[entry.name]) original_entries.delete(entry) original_entries << resolved_alias - @entries_tree.insert(entry.name, original_entries) + @constant_entries_tree.insert(entry.name, original_entries) resolved_alias end diff --git a/lib/ruby_indexer/test/classes_and_modules_test.rb b/lib/ruby_indexer/test/classes_and_modules_test.rb index a33d18578..14d5bd38e 100644 --- a/lib/ruby_indexer/test/classes_and_modules_test.rb +++ b/lib/ruby_indexer/test/classes_and_modules_test.rb @@ -161,10 +161,10 @@ class Foo class Bar; end RUBY - foo_entry = @index["Foo"].first + foo_entry = @index.get_constant("Foo").first assert_equal("This is a Foo comment\nThis is another Foo comment", foo_entry.comments.join("\n")) - bar_entry = @index["Bar"].first + bar_entry = @index.get_constant("Bar").first assert_equal("This Bar comment has 1 line padding", bar_entry.comments.join("\n")) end @@ -174,7 +174,7 @@ def test_skips_comments_containing_invalid_encodings class Foo end RUBY - assert(@index["Foo"].first) + assert(@index.get_constant("Foo").first) end def test_comments_can_be_attached_to_a_namespaced_class @@ -187,10 +187,10 @@ class Bar; end end RUBY - foo_entry = @index["Foo"].first + foo_entry = @index.get_constant("Foo").first assert_equal("This is a Foo comment\nThis is another Foo comment", foo_entry.comments.join("\n")) - bar_entry = @index["Foo::Bar"].first + bar_entry = @index.get_constant("Foo::Bar").first assert_equal("This is a Bar comment", bar_entry.comments.join("\n")) end @@ -203,10 +203,10 @@ class Foo; end class Foo; end RUBY - first_foo_entry = @index["Foo"][0] + first_foo_entry = @index.get_constant("Foo")[0] assert_equal("This is a Foo comment", first_foo_entry.comments.join("\n")) - second_foo_entry = @index["Foo"][1] + second_foo_entry = @index.get_constant("Foo")[1] assert_equal("This is another Foo comment", second_foo_entry.comments.join("\n")) end @@ -219,10 +219,10 @@ class Foo; end class Bar; end RUBY - first_foo_entry = @index["Foo"][0] + first_foo_entry = @index.get_constant("Foo")[0] assert_equal("This is a Foo comment", first_foo_entry.comments.join("\n")) - second_foo_entry = @index["Bar"][0] + second_foo_entry = @index.get_constant("Bar")[0] assert_equal("This is a Bar comment", second_foo_entry.comments.join("\n")) end @@ -239,13 +239,13 @@ class D; end end RUBY - b_const = @index["A::B"].first + b_const = @index.get_constant("A::B").first assert_equal(:private, b_const.visibility) - c_const = @index["A::C"].first + c_const = @index.get_constant("A::C").first assert_equal(:private, c_const.visibility) - d_const = @index["A::D"].first + d_const = @index.get_constant("A::D").first assert_equal(:public, d_const.visibility) end @@ -269,16 +269,16 @@ class FinalThing < Something::Baz end RUBY - foo = T.must(@index["Foo"].first) + foo = T.must(@index.get_constant("Foo").first) assert_equal("Bar", foo.parent_class) - baz = T.must(@index["Baz"].first) + baz = T.must(@index.get_constant("Baz").first) assert_nil(baz.parent_class) - qux = T.must(@index["Something::Qux"].first) + qux = T.must(@index.get_constant("Something::Qux").first) assert_equal("::Baz", qux.parent_class) - final_thing = T.must(@index["FinalThing"].first) + final_thing = T.must(@index.get_constant("FinalThing").first) assert_equal("Something::Baz", final_thing.parent_class) end @@ -318,13 +318,13 @@ class ConstantPathReferences end RUBY - foo = T.must(@index["Foo"][0]) + foo = T.must(@index.get_constant("Foo")[0]) assert_equal(["A1", "A2", "A3", "A4", "A5", "A6"], foo.included_modules) - qux = T.must(@index["Foo::Qux"][0]) + qux = T.must(@index.get_constant("Foo::Qux")[0]) assert_equal(["Corge", "Corge", "Baz"], qux.included_modules) - constant_path_references = T.must(@index["ConstantPathReferences"][0]) + constant_path_references = T.must(@index.get_constant("ConstantPathReferences")[0]) assert_equal(["Foo::Bar", "Foo::Bar2"], constant_path_references.included_modules) end @@ -364,13 +364,13 @@ class ConstantPathReferences end RUBY - foo = T.must(@index["Foo"][0]) + foo = T.must(@index.get_constant("Foo")[0]) assert_equal(["A1", "A2", "A3", "A4", "A5", "A6"], foo.prepended_modules) - qux = T.must(@index["Foo::Qux"][0]) + qux = T.must(@index.get_constant("Foo::Qux")[0]) assert_equal(["Corge", "Corge", "Baz"], qux.prepended_modules) - constant_path_references = T.must(@index["ConstantPathReferences"][0]) + constant_path_references = T.must(@index.get_constant("ConstantPathReferences")[0]) assert_equal(["Foo::Bar", "Foo::Bar2"], constant_path_references.prepended_modules) end end diff --git a/lib/ruby_indexer/test/constant_test.rb b/lib/ruby_indexer/test/constant_test.rb index 16aec45df..08912b887 100644 --- a/lib/ruby_indexer/test/constant_test.rb +++ b/lib/ruby_indexer/test/constant_test.rb @@ -83,16 +83,16 @@ class A A::BAZ = 1 RUBY - foo_comment = @index["FOO"].first.comments.join("\n") + foo_comment = @index.get_constant("FOO").first.comments.join("\n") assert_equal("FOO comment", foo_comment) - a_foo_comment = @index["A::FOO"].first.comments.join("\n") + a_foo_comment = @index.get_constant("A::FOO").first.comments.join("\n") assert_equal("A::FOO comment", a_foo_comment) - bar_comment = @index["BAR"].first.comments.join("\n") + bar_comment = @index.get_constant("BAR").first.comments.join("\n") assert_equal("::BAR comment", bar_comment) - a_baz_comment = @index["A::BAZ"].first.comments.join("\n") + a_baz_comment = @index.get_constant("A::BAZ").first.comments.join("\n") assert_equal("A::BAZ comment", a_baz_comment) end @@ -118,13 +118,13 @@ class A end RUBY - b_const = @index["A::B"].first + b_const = @index.get_constant("A::B").first assert_equal(:private, b_const.visibility) - c_const = @index["A::C"].first + c_const = @index.get_constant("A::C").first assert_equal(:private, c_const.visibility) - d_const = @index["A::D"].first + d_const = @index.get_constant("A::D").first assert_equal(:public, d_const.visibility) end @@ -151,13 +151,13 @@ module B end RUBY - a_const = @index["A::B::CONST_A"].first + a_const = @index.get_constant("A::B::CONST_A").first assert_equal(:private, a_const.visibility) - b_const = @index["A::B::CONST_B"].first + b_const = @index.get_constant("A::B::CONST_B").first assert_equal(:private, b_const.visibility) - c_const = @index["A::B::CONST_C"].first + c_const = @index.get_constant("A::B::CONST_C").first assert_equal(:private, c_const.visibility) end @@ -175,10 +175,10 @@ module B A::B.private_constant(:CONST_B) RUBY - a_const = @index["A::B::CONST_A"].first + a_const = @index.get_constant("A::B::CONST_A").first assert_equal(:private, a_const.visibility) - b_const = @index["A::B::CONST_B"].first + b_const = @index.get_constant("A::B::CONST_B").first assert_equal(:private, b_const.visibility) end @@ -196,12 +196,12 @@ module C SECOND = A::FIRST RUBY - unresolve_entry = @index["A::FIRST"].first + unresolve_entry = @index.get_constant("A::FIRST").first assert_instance_of(Entry::UnresolvedAlias, unresolve_entry) assert_equal(["A"], unresolve_entry.nesting) assert_equal("B::C", unresolve_entry.target) - resolved_entry = @index.resolve("A::FIRST", []).first + resolved_entry = @index.resolve_constant("A::FIRST", []).first assert_instance_of(Entry::Alias, resolved_entry) assert_equal("A::B::C", resolved_entry.target) end @@ -222,25 +222,25 @@ module Other end RUBY - unresolve_entry = @index["A::ALIAS"].first + unresolve_entry = @index.get_constant("A::ALIAS").first assert_instance_of(Entry::UnresolvedAlias, unresolve_entry) assert_equal(["A"], unresolve_entry.nesting) assert_equal("B", unresolve_entry.target) - resolved_entry = @index.resolve("ALIAS", ["A"]).first + resolved_entry = @index.resolve_constant("ALIAS", ["A"]).first assert_instance_of(Entry::Alias, resolved_entry) assert_equal("A::B", resolved_entry.target) - resolved_entry = @index.resolve("ALIAS::C", ["A"]).first + resolved_entry = @index.resolve_constant("ALIAS::C", ["A"]).first assert_instance_of(Entry::Module, resolved_entry) assert_equal("A::B::C", resolved_entry.name) - unresolve_entry = @index["Other::ONE_MORE"].first + unresolve_entry = @index.get_constant("Other::ONE_MORE").first assert_instance_of(Entry::UnresolvedAlias, unresolve_entry) assert_equal(["Other"], unresolve_entry.nesting) assert_equal("A::ALIAS", unresolve_entry.target) - resolved_entry = @index.resolve("Other::ONE_MORE::C", []).first + resolved_entry = @index.resolve_constant("Other::ONE_MORE::C", []).first assert_instance_of(Entry::Module, resolved_entry) end @@ -255,55 +255,55 @@ module A RUBY # B and C - unresolve_entry = @index["A::B"].first + unresolve_entry = @index.get_constant("A::B").first assert_instance_of(Entry::UnresolvedAlias, unresolve_entry) assert_equal(["A"], unresolve_entry.nesting) assert_equal("C", unresolve_entry.target) - resolved_entry = @index.resolve("A::B", []).first + resolved_entry = @index.resolve_constant("A::B", []).first assert_instance_of(Entry::Alias, resolved_entry) assert_equal("A::C", resolved_entry.target) - constant = @index["A::C"].first + constant = @index.get_constant("A::C").first assert_instance_of(Entry::Constant, constant) # D and E - unresolve_entry = @index["A::D"].first + unresolve_entry = @index.get_constant("A::D").first assert_instance_of(Entry::UnresolvedAlias, unresolve_entry) assert_equal(["A"], unresolve_entry.nesting) assert_equal("E", unresolve_entry.target) - resolved_entry = @index.resolve("A::D", []).first + resolved_entry = @index.resolve_constant("A::D", []).first assert_instance_of(Entry::Alias, resolved_entry) assert_equal("A::E", resolved_entry.target) # F and G::H - unresolve_entry = @index["A::F"].first + unresolve_entry = @index.get_constant("A::F").first assert_instance_of(Entry::UnresolvedAlias, unresolve_entry) assert_equal(["A"], unresolve_entry.nesting) assert_equal("G::H", unresolve_entry.target) - resolved_entry = @index.resolve("A::F", []).first + resolved_entry = @index.resolve_constant("A::F", []).first assert_instance_of(Entry::Alias, resolved_entry) assert_equal("A::G::H", resolved_entry.target) # I::J, K::L and M - unresolve_entry = @index["A::I::J"].first + unresolve_entry = @index.get_constant("A::I::J").first assert_instance_of(Entry::UnresolvedAlias, unresolve_entry) assert_equal(["A"], unresolve_entry.nesting) assert_equal("K::L", unresolve_entry.target) - resolved_entry = @index.resolve("A::I::J", []).first + resolved_entry = @index.resolve_constant("A::I::J", []).first assert_instance_of(Entry::Alias, resolved_entry) assert_equal("A::K::L", resolved_entry.target) # When we are resolving A::I::J, we invoke `resolve("K::L", ["A"])`, which recursively resolves A::K::L too. # Therefore, both A::I::J and A::K::L point to A::M by the end of the previous resolve invocation - resolved_entry = @index["A::K::L"].first + resolved_entry = @index.get_constant("A::K::L").first assert_instance_of(Entry::Alias, resolved_entry) assert_equal("A::M", resolved_entry.target) - constant = @index["A::M"].first + constant = @index.get_constant("A::M").first assert_instance_of(Entry::Constant, constant) end diff --git a/lib/ruby_indexer/test/index_test.rb b/lib/ruby_indexer/test/index_test.rb index 6f759471a..1dac110bd 100644 --- a/lib/ruby_indexer/test/index_test.rb +++ b/lib/ruby_indexer/test/index_test.rb @@ -15,11 +15,11 @@ class Foo end RUBY - entries = @index["Foo"] + entries = @index.get_constant("Foo") assert_equal(2, entries.length) @index.delete(IndexablePath.new(nil, "/fake/path/other_foo.rb")) - entries = @index["Foo"] + entries = @index.get_constant("Foo") assert_equal(1, entries.length) end @@ -29,11 +29,11 @@ class Foo end RUBY - entries = @index["Foo"] + entries = @index.get_constant("Foo") assert_equal(1, entries.length) @index.delete(IndexablePath.new(nil, "/fake/path/foo.rb")) - entries = @index["Foo"] + entries = @index.get_constant("Foo") assert_nil(entries) end @@ -52,23 +52,23 @@ class Something end RUBY - entries = @index.resolve("Something", ["Foo", "Baz"]) + entries = @index.resolve_constant("Something", ["Foo", "Baz"]) refute_empty(entries) assert_equal("Foo::Baz::Something", entries.first.name) - entries = @index.resolve("Bar", ["Foo"]) + entries = @index.resolve_constant("Bar", ["Foo"]) refute_empty(entries) assert_equal("Foo::Bar", entries.first.name) - entries = @index.resolve("Bar", ["Foo", "Baz"]) + entries = @index.resolve_constant("Bar", ["Foo", "Baz"]) refute_empty(entries) assert_equal("Foo::Bar", entries.first.name) - entries = @index.resolve("Foo::Bar", ["Foo", "Baz"]) + entries = @index.resolve_constant("Foo::Bar", ["Foo", "Baz"]) refute_empty(entries) assert_equal("Foo::Bar", entries.first.name) - assert_nil(@index.resolve("DoesNotExist", ["Foo"])) + assert_nil(@index.resolve_constant("DoesNotExist", ["Foo"])) end def test_accessing_with_colon_colon_prefix @@ -86,7 +86,7 @@ class Something end RUBY - entries = @index["::Foo::Baz::Something"] + entries = @index.get_constant("::Foo::Baz::Something") refute_empty(entries) assert_equal("Foo::Baz::Something", entries.first.name) end @@ -108,7 +108,7 @@ class Something result = @index.fuzzy_search("Bar") assert_equal(1, result.length) - assert_equal(@index["Bar"].first, result.first) + assert_equal(@index.get_constant("Bar").first, result.first) result = @index.fuzzy_search("foobarsomeking") assert_equal(5, result.length) @@ -152,10 +152,10 @@ class Foo::Baz end RUBY - results = @index.prefix_search("Foo", []).map { |entries| entries.map(&:name) } + results = @index.prefix_search_constants("Foo", []).map { |entries| entries.map(&:name) } assert_equal([["Foo::Bar", "Foo::Bar"], ["Foo::Baz"]], results) - results = @index.prefix_search("Ba", ["Foo"]).map { |entries| entries.map(&:name) } + results = @index.prefix_search_constants("Ba", ["Foo"]).map { |entries| entries.map(&:name) } assert_equal([["Foo::Bar", "Foo::Bar"], ["Foo::Baz"]], results) end @@ -168,12 +168,12 @@ class Bar; end end RUBY - entries = @index.resolve("::Foo::Bar", []) + entries = @index.resolve_constant("::Foo::Bar", []) refute_nil(entries) assert_equal("Foo::Bar", entries.first.name) - entries = @index.resolve("::Bar", ["Foo"]) + entries = @index.resolve_constant("::Bar", ["Foo"]) refute_nil(entries) assert_equal("Bar", entries.first.name) @@ -188,7 +188,7 @@ class Float < self end RUBY - entry = @index.resolve("INFINITY", ["Foo", "Float"]).first + entry = @index.resolve_constant("INFINITY", ["Foo", "Float"]).first refute_nil(entry) assert_instance_of(Entry::UnresolvedAlias, entry) @@ -291,7 +291,7 @@ def baz; end end RUBY - entries = @index.prefix_search("ba") + entries = @index.prefix_search_methods("ba") refute_empty(entries) entry = T.must(entries.first).first @@ -306,12 +306,14 @@ def test_indexing_prism_fixtures_succeeds @index.index_single(indexable_path) end - refute_empty(@index.instance_variable_get(:@entries)) + refute_empty(@index.instance_variable_get(:@constant_entries)) + refute_empty(@index.instance_variable_get(:@method_entries)) end def test_index_single_does_not_fail_for_non_existing_file @index.index_single(IndexablePath.new(nil, "/fake/path/foo.rb")) - assert_empty(@index.instance_variable_get(:@entries)) + assert_empty(@index.instance_variable_get(:@constant_entries)) + assert_empty(@index.instance_variable_get(:@method_entries)) end end end diff --git a/lib/ruby_indexer/test/method_test.rb b/lib/ruby_indexer/test/method_test.rb index 3e5854667..d174e2a5a 100644 --- a/lib/ruby_indexer/test/method_test.rb +++ b/lib/ruby_indexer/test/method_test.rb @@ -47,7 +47,7 @@ def bar(a) RUBY assert_entry("bar", Entry::InstanceMethod, "/fake/path/foo.rb:1-2:2-5") - entry = T.must(@index["bar"].first) + entry = T.must(@index.get_method("bar").first) assert_equal(1, entry.parameters.length) parameter = entry.parameters.first assert_equal(:a, parameter.name) @@ -63,7 +63,7 @@ def bar((a, (b, ))) RUBY assert_entry("bar", Entry::InstanceMethod, "/fake/path/foo.rb:1-2:2-5") - entry = T.must(@index["bar"].first) + entry = T.must(@index.get_method("bar").first) assert_equal(1, entry.parameters.length) parameter = entry.parameters.first assert_equal(:"(a, (b, ))", parameter.name) @@ -79,7 +79,7 @@ def bar(a = 123) RUBY assert_entry("bar", Entry::InstanceMethod, "/fake/path/foo.rb:1-2:2-5") - entry = T.must(@index["bar"].first) + entry = T.must(@index.get_method("bar").first) assert_equal(1, entry.parameters.length) parameter = entry.parameters.first assert_equal(:a, parameter.name) @@ -95,7 +95,7 @@ def bar(a:, b: 123) RUBY assert_entry("bar", Entry::InstanceMethod, "/fake/path/foo.rb:1-2:2-5") - entry = T.must(@index["bar"].first) + entry = T.must(@index.get_method("bar").first) assert_equal(2, entry.parameters.length) a, b = entry.parameters @@ -115,7 +115,7 @@ def bar(*a, **b) RUBY assert_entry("bar", Entry::InstanceMethod, "/fake/path/foo.rb:1-2:2-5") - entry = T.must(@index["bar"].first) + entry = T.must(@index.get_method("bar").first) assert_equal(2, entry.parameters.length) a, b = entry.parameters @@ -140,7 +140,7 @@ def qux(*a, (b, c)) RUBY assert_entry("bar", Entry::InstanceMethod, "/fake/path/foo.rb:1-2:2-5") - entry = T.must(@index["bar"].first) + entry = T.must(@index.get_method("bar").first) assert_equal(2, entry.parameters.length) a, b = entry.parameters @@ -150,7 +150,7 @@ def qux(*a, (b, c)) assert_equal(:b, b.name) assert_instance_of(Entry::RequiredParameter, b) - entry = T.must(@index["baz"].first) + entry = T.must(@index.get_method("baz").first) assert_equal(2, entry.parameters.length) a, b = entry.parameters @@ -160,7 +160,7 @@ def qux(*a, (b, c)) assert_equal(:b, b.name) assert_instance_of(Entry::RequiredParameter, b) - entry = T.must(@index["qux"].first) + entry = T.must(@index.get_method("qux").first) assert_equal(2, entry.parameters.length) _a, second = entry.parameters @@ -177,7 +177,7 @@ def bar((a, *b)) RUBY assert_entry("bar", Entry::InstanceMethod, "/fake/path/foo.rb:1-2:2-5") - entry = T.must(@index["bar"].first) + entry = T.must(@index.get_method("bar").first) assert_equal(1, entry.parameters.length) param = entry.parameters.first @@ -196,12 +196,12 @@ def baz(&) end RUBY - entry = T.must(@index["bar"].first) + entry = T.must(@index.get_method("bar").first) param = entry.parameters.first assert_equal(:block, param.name) assert_instance_of(Entry::BlockParameter, param) - entry = T.must(@index["baz"].first) + entry = T.must(@index.get_method("baz").first) assert_equal(1, entry.parameters.length) param = entry.parameters.first @@ -218,7 +218,7 @@ def bar(*, **) RUBY assert_entry("bar", Entry::InstanceMethod, "/fake/path/foo.rb:1-2:2-5") - entry = T.must(@index["bar"].first) + entry = T.must(@index.get_method("bar").first) assert_equal(2, entry.parameters.length) first, second = entry.parameters @@ -238,7 +238,7 @@ def bar(**nil) RUBY assert_entry("bar", Entry::InstanceMethod, "/fake/path/foo.rb:1-2:2-5") - entry = T.must(@index["bar"].first) + entry = T.must(@index.get_method("bar").first) assert_empty(entry.parameters) end @@ -250,7 +250,7 @@ def bar end RUBY - entry = T.must(@index["bar"].first) + entry = T.must(@index.get_method("bar").first) owner_name = T.must(entry.owner).name assert_equal("Foo", owner_name) @@ -267,9 +267,9 @@ class Foo RUBY assert_entry("bar", Entry::Accessor, "/fake/path/foo.rb:2-15:2-18") - assert_equal("Hello there", @index["bar"].first.comments.join("\n")) + assert_equal("Hello there", @index.get_method("bar").first.comments.join("\n")) assert_entry("other", Entry::Accessor, "/fake/path/foo.rb:2-21:2-26") - assert_equal("Hello there", @index["other"].first.comments.join("\n")) + assert_equal("Hello there", @index.get_method("other").first.comments.join("\n")) assert_entry("baz=", Entry::Accessor, "/fake/path/foo.rb:3-15:3-18") assert_entry("qux", Entry::Accessor, "/fake/path/foo.rb:4-17:4-20") assert_entry("qux=", Entry::Accessor, "/fake/path/foo.rb:4-17:4-20") diff --git a/lib/ruby_indexer/test/test_case.rb b/lib/ruby_indexer/test/test_case.rb index cf666cb4e..1a7d2eb40 100644 --- a/lib/ruby_indexer/test/test_case.rb +++ b/lib/ruby_indexer/test/test_case.rb @@ -16,7 +16,7 @@ def index(source) end def assert_entry(expected_name, type, expected_location) - entries = @index[expected_name] + entries = @index.get_constant(expected_name) || @index.get_method(expected_name) refute_empty(entries, "Expected #{expected_name} to be indexed") entry = entries.first @@ -31,16 +31,18 @@ def assert_entry(expected_name, type, expected_location) end def refute_entry(expected_name) - entries = @index[expected_name] + entries = @index.get_constant(expected_name) || @index.get_method(expected_name) assert_nil(entries, "Expected #{expected_name} to not be indexed") end def assert_no_entries - assert_empty(@index.instance_variable_get(:@entries), "Expected nothing to be indexed") + assert_empty(@index.instance_variable_get(:@constant_entries), "Expected nothing to be indexed") + assert_empty(@index.instance_variable_get(:@method_entries), "Expected nothing to be indexed") end def assert_no_entry(entry) - refute(@index.instance_variable_get(:@entries).key?(entry), "Expected '#{entry}' to not be indexed") + refute(@index.instance_variable_get(:@constant_entries).key?(entry), "Expected '#{entry}' to not be indexed") + refute(@index.instance_variable_get(:@method_entries).key?(entry), "Expected '#{entry}' to not be indexed") end end end diff --git a/lib/ruby_lsp/listeners/completion.rb b/lib/ruby_lsp/listeners/completion.rb index e7aff969a..b836e0cc3 100644 --- a/lib/ruby_lsp/listeners/completion.rb +++ b/lib/ruby_lsp/listeners/completion.rb @@ -41,7 +41,7 @@ def on_constant_read_node_enter(node) name = constant_name(node) return if name.nil? - candidates = @index.prefix_search(name, @nesting) + candidates = @index.prefix_search_constants(name, @nesting) candidates.each do |entries| complete_name = T.must(entries.first).name @response_builder << build_entry_completion( @@ -73,12 +73,12 @@ def on_constant_path_node_enter(node) # order to find which possible constants match the desired search *namespace, incomplete_name = name.split("::") aliased_namespace = T.must(namespace).join("::") - namespace_entries = @index.resolve(aliased_namespace, @nesting) + namespace_entries = @index.resolve_constant(aliased_namespace, @nesting) return unless namespace_entries real_namespace = @index.follow_aliased_namespace(T.must(namespace_entries.first).name) - candidates = @index.prefix_search( + candidates = @index.prefix_search_constants( "#{real_namespace}::#{incomplete_name}", top_level_reference ? [] : @nesting, ) @@ -166,12 +166,12 @@ def complete_require_relative(node) sig { params(node: Prism::CallNode, name: String).void } def complete_self_receiver_method(node, name) - receiver_entries = @index[@nesting.join("::")] + receiver_entries = @index.get_constant(@nesting.join("::")) return unless receiver_entries receiver = T.must(receiver_entries.first) - @index.prefix_search(name).each do |entries| + @index.prefix_search_methods(name).each do |entries| entry = entries.find { |e| e.is_a?(RubyIndexer::Entry::Member) && e.owner&.name == receiver.name } next unless entry @@ -269,7 +269,7 @@ def build_entry_completion(real_name, incomplete_name, node, entries, top_level) # If a different entry exists for the shortened name, then there's a conflict and we should not shorten it conflict_name = "#{@nesting.join("::")}::#{shortened_name}" - break if real_name != conflict_name && @index[conflict_name] + break if real_name != conflict_name && @index.get_constant(conflict_name) insertion_text = shortened_name @@ -314,7 +314,7 @@ def top_level?(entry_name) full_name = prefix.empty? ? entry_name : "#{prefix}::#{entry_name}" next if full_name == entry_name - return true if @index[full_name] + return true if @index.get_constant(full_name) end false diff --git a/lib/ruby_lsp/listeners/definition.rb b/lib/ruby_lsp/listeners/definition.rb index fefc3f020..153d6aaf6 100644 --- a/lib/ruby_lsp/listeners/definition.rb +++ b/lib/ruby_lsp/listeners/definition.rb @@ -74,7 +74,7 @@ def handle_method_definition(node) else # If the method doesn't have a receiver, then we provide a few candidates to jump to # But we don't want to provide too many candidates, as it can be overwhelming - @index[message]&.take(MAX_NUMBER_OF_DEFINITION_CANDIDATES_WITHOUT_RECEIVER) + @index.get_method(message)&.take(MAX_NUMBER_OF_DEFINITION_CANDIDATES_WITHOUT_RECEIVER) end return unless methods @@ -138,7 +138,7 @@ def handle_require_definition(node) sig { params(value: String).void } def find_in_index(value) - entries = @index.resolve(value, @nesting) + entries = @index.resolve_constant(value, @nesting) return unless entries # We should only allow jumping to the definition of private constants if the constant is defined in the same diff --git a/lib/ruby_lsp/listeners/hover.rb b/lib/ruby_lsp/listeners/hover.rb index 7b5b102ce..64149f858 100644 --- a/lib/ruby_lsp/listeners/hover.rb +++ b/lib/ruby_lsp/listeners/hover.rb @@ -105,7 +105,7 @@ def on_call_node_enter(node) sig { params(name: String, location: Prism::Location).void } def generate_hover(name, location) - entries = @index.resolve(name, @nesting) + entries = @index.resolve_constant(name, @nesting) return unless entries # We should only show hover for private constants if the constant is defined in the same namespace as the diff --git a/lib/ruby_lsp/requests/completion_resolve.rb b/lib/ruby_lsp/requests/completion_resolve.rb index ec5fb3a06..6b3780236 100644 --- a/lib/ruby_lsp/requests/completion_resolve.rb +++ b/lib/ruby_lsp/requests/completion_resolve.rb @@ -39,7 +39,14 @@ def initialize(global_state, item) sig { override.returns(Interface::CompletionItem) } def perform label = @item[:label] - entries = @index[label] || [] + entries = case @item[:kind] + when Constant::CompletionItemKind::CLASS, Constant::CompletionItemKind::MODULE, + Constant::CompletionItemKind::CONSTANT + @index.get_constant(label) || [] + else + @index.get_method(label) || [] + end + Interface::CompletionItem.new( label: label, label_details: Interface::CompletionItemLabelDetails.new( diff --git a/test/requests/completion_resolve_test.rb b/test/requests/completion_resolve_test.rb index 547215d45..6fded88f3 100644 --- a/test/requests/completion_resolve_test.rb +++ b/test/requests/completion_resolve_test.rb @@ -19,6 +19,7 @@ class Foo with_server(source, stub_no_typechecker: true) do |server, _uri| server.process_message(id: 1, method: "completionItem/resolve", params: { label: "Foo", + kind: Constant::CompletionItemKind::CLASS, }) result = server.pop_response.response @@ -30,7 +31,7 @@ class Foo ), documentation: Interface::MarkupContent.new( kind: "markdown", - value: markdown_from_index_entries("Foo", T.must(server.global_state.index["Foo"])), + value: markdown_from_index_entries("Foo", T.must(server.global_state.index.get_constant("Foo"))), ), ) assert_match(/This is a class that does things/, result.documentation.value) diff --git a/test/server_test.rb b/test/server_test.rb index e6c39bf63..e8689c715 100644 --- a/test/server_test.rb +++ b/test/server_test.rb @@ -167,7 +167,7 @@ def test_initialized_populates_index assert_equal("$/progress", @server.pop_response.method) index = @server.global_state.index - refute_empty(index.instance_variable_get(:@entries)) + refute_empty(index.instance_variable_get(:@constant_entries)) end end