From 452bc49b372a407935640ee3e837a46d5283e26a Mon Sep 17 00:00:00 2001 From: Vinicius Stock Date: Tue, 10 Oct 2023 11:26:58 -0400 Subject: [PATCH] Use shortest possible constant name for filtering completions --- lib/ruby_lsp/requests/completion.rb | 38 +++++++++++++++++------ test/requests/completion_test.rb | 48 ++++++++++++++++++++++++----- 2 files changed, 70 insertions(+), 16 deletions(-) diff --git a/lib/ruby_lsp/requests/completion.rb b/lib/ruby_lsp/requests/completion.rb index e1e45da0a..bce57d643 100644 --- a/lib/ruby_lsp/requests/completion.rb +++ b/lib/ruby_lsp/requests/completion.rb @@ -59,7 +59,13 @@ def on_constant_read(node) candidates = @index.prefix_search(name, @nesting) candidates.each do |entries| complete_name = T.must(entries.first).name - @_response << build_entry_completion(complete_name, node, entries, top_level?(complete_name, candidates)) + @_response << build_entry_completion( + complete_name, + name, + node, + entries, + top_level?(complete_name, candidates), + ) end end @@ -99,6 +105,7 @@ def on_constant_path(node) @_response << build_entry_completion( full_name, + name, node, entries, top_level_reference || top_level?(T.must(entries.first).name, candidates), @@ -122,13 +129,14 @@ def build_completion(label, node) sig do params( - name: String, + real_name: String, + incomplete_name: String, node: Prism::Node, entries: T::Array[RubyIndexer::Index::Entry], top_level: T::Boolean, ).returns(Interface::CompletionItem) end - def build_entry_completion(name, node, entries, top_level) + def build_entry_completion(real_name, incomplete_name, node, entries, top_level) first_entry = T.must(entries.first) kind = case first_entry when RubyIndexer::Index::Entry::Class @@ -141,14 +149,18 @@ def build_entry_completion(name, node, entries, top_level) Constant::CompletionItemKind::REFERENCE end - insertion_text = name.dup + insertion_text = real_name.dup + filter_text = real_name.dup # If we have two entries with the same name inside the current namespace and the user selects the top level # option, we have to ensure it's prefixed with `::` or else we're completing the wrong constant. For example: # If we have the index with ["Foo::Bar", "Bar"], and we're providing suggestions for `B` inside a `Foo` module, # then selecting the `Foo::Bar` option needs to complete to `Bar` and selecting the top level `Bar` option needs # to complete to `::Bar`. - insertion_text.prepend("::") if top_level + if top_level + insertion_text.prepend("::") + filter_text.prepend("::") + end # If the user is searching for a constant inside the current namespace, then we prefer completing the short name # of that constant. E.g.: @@ -159,14 +171,22 @@ def build_entry_completion(name, node, entries, top_level) # # Foo::B # --> completion inserts `Bar` instead of `Foo::Bar` # end - @nesting.each { |namespace| insertion_text.delete_prefix!("#{namespace}::") } + @nesting.each do |namespace| + prefix = "#{namespace}::" + insertion_text.delete_prefix!(prefix) + + # If the user is typing a fully qualified name `Foo::Bar::Baz`, then we should not use the short name (e.g.: + # `Baz`) as filtering. So we only shorten the filter text if the user is not including the namespaces in their + # typing + filter_text.delete_prefix!(prefix) unless incomplete_name.start_with?(prefix) + end # When using a top level constant reference (e.g.: `::Bar`), the editor includes the `::` as part of the filter. # For these top level references, we need to include the `::` as part of the filter text or else it won't match # the right entries in the index Interface::CompletionItem.new( - label: name, - filter_text: top_level ? "::#{name}" : name, + label: real_name, + filter_text: filter_text, text_edit: Interface::TextEdit.new( range: range_from_node(node), new_text: insertion_text, @@ -175,7 +195,7 @@ def build_entry_completion(name, node, entries, top_level) label_details: Interface::CompletionItemLabelDetails.new( description: entries.map(&:file_name).join(","), ), - documentation: markdown_from_index_entries(name, entries), + documentation: markdown_from_index_entries(real_name, entries), ) end diff --git a/test/requests/completion_test.rb b/test/requests/completion_test.rb index c16990289..e8cfa416c 100644 --- a/test/requests/completion_test.rb +++ b/test/requests/completion_test.rb @@ -245,7 +245,7 @@ module Foo params: { textDocument: { uri: @uri.to_s }, position: end_position }, ) assert_equal(["Foo::Bar", "Bar"], result.map(&:label)) - assert_equal(["Foo::Bar", "::Bar"], result.map(&:filter_text)) + assert_equal(["Bar", "::Bar"], result.map(&:filter_text)) assert_equal(["Bar", "::Bar"], result.map { |completion| completion.text_edit.new_text }) end_position = { line: 10, character: 6 } @@ -333,7 +333,7 @@ class A end def test_completion_for_aliased_constants - RubyLsp::DependencyDetector.const_set(:HAS_TYPECHECKER, false) + stub_no_typechecker document = RubyLsp::Document.new(source: +<<~RUBY, version: 1, uri: @uri) module A module B @@ -362,12 +362,10 @@ module Other assert_equal(["ALIAS_NAME::B::CONST"], result.map(&:label)) assert_equal(["ALIAS_NAME::B::CONST"], result.map(&:filter_text)) assert_equal(["ALIAS_NAME::B::CONST"], result.map { |completion| completion.text_edit.new_text }) - ensure - RubyLsp::DependencyDetector.const_set(:HAS_TYPECHECKER, true) end def test_completion_for_aliased_complex_constants - RubyLsp::DependencyDetector.const_set(:HAS_TYPECHECKER, false) + stub_no_typechecker document = RubyLsp::Document.new(source: +<<~RUBY, version: 1, uri: @uri) module A module B @@ -397,8 +395,44 @@ module Other assert_equal(["FINAL_ALIAS::ALIAS_NAME::B::CONST"], result.map(&:label)) assert_equal(["FINAL_ALIAS::ALIAS_NAME::B::CONST"], result.map(&:filter_text)) assert_equal(["FINAL_ALIAS::ALIAS_NAME::B::CONST"], result.map { |completion| completion.text_edit.new_text }) - ensure - RubyLsp::DependencyDetector.const_set(:HAS_TYPECHECKER, true) + end + + def test_completion_uses_shortest_possible_name_for_filter_text + stub_no_typechecker + document = RubyLsp::Document.new(source: +<<~RUBY, version: 1, uri: @uri) + module A + module B + class Foo + end + + F + A::B::F + end + end + RUBY + + @store.set(uri: @uri, source: document.source, version: 1) + + index = @executor.instance_variable_get(:@index) + index.index_single(RubyIndexer::IndexablePath.new(nil, @uri.to_standardized_path), document.source) + + result = run_request( + method: "textDocument/completion", + params: { textDocument: { uri: @uri.to_s }, position: { line: 5, character: 5 } }, + ) + + assert_equal(["A::B::Foo"], result.map(&:label)) + assert_equal(["Foo"], result.map(&:filter_text)) + assert_equal(["Foo"], result.map { |completion| completion.text_edit.new_text }) + + result = run_request( + method: "textDocument/completion", + params: { textDocument: { uri: @uri.to_s }, position: { line: 6, character: 11 } }, + ) + + assert_equal(["A::B::Foo"], result.map(&:label)) + assert_equal(["A::B::Foo"], result.map(&:filter_text)) + assert_equal(["Foo"], result.map { |completion| completion.text_edit.new_text }) end private