Add autocomplete for classes, modules and constants #957

vinistock · 2023-08-30T19:49:44Z

Motivation

Closes #61

Add completion for classes, modules and constants.

Implementation

I'll explain the changes per file

Executor: when a completion request is triggered, we receive the position for the character immediately after the letter the user just typed. So I started checking if the previous character (the one just inserted) is an uppercase character and then we go down the path of completion for classes, modules and constants. Otherwise, we do the previous path completion. Also started passing the nesting to the completion request and adapted our capabilities
Completion: started handling const and const_path_ref to provide completion for both namespaced and regular constants. These events use the prefix_search added in Add entries prefix tree #955 and then build the completions. I also extracted the logic to build markdown content from hover into Common, so that we can display the exact same documentation on both hover and completion (the documentation shows up on the side when selecting a completion)

Other than that, I just renamed the request to be more generic, using Completion instead of PathCompletion.

Automated Tests

Added tests for both const and const path refs.

Manual Tests

Open a codebase that does not use Sorbet (e.g.: rails/rails)
Configure the Ruby LSP to use this branch "rubyLsp.branch": "vs/add_autocomplete"
Type some uppercase letter somewhere
Verify you get completion options
Verify you can see the documentation properly when switching options (notice that in VS Code the documentation might be hidden. You need to click the little right arrow on top of a completion item to show docs)

github-actions · 2023-08-30T19:58:49Z

Benchmark results in seconds (slowest at top)

          textDocument/diagnostic average: 0.042066 std_dev: 0.010297
          textDocument/definition average: 0.00485 std_dev: 0.002825
      textDocument/selectionRange average: 0.004171 std_dev: 0.000585
   textDocument/documentHighlight average: 0.002627 std_dev: 0.000196
        textDocument/documentLink average: 0.002564 std_dev: 0.000205
 textDocument/semanticTokens/full average: 0.002548 std_dev: 0.000222
            textDocument/codeLens average: 0.002537 std_dev: 0.000192
      textDocument/documentSymbol average: 0.002511 std_dev: 0.000152
        textDocument/foldingRange average: 0.002507 std_dev: 0.000218
textDocument/semanticTokens/range average: 0.001688 std_dev: 0.000124
               codeAction/resolve average: 0.00148 std_dev: 0.00013
           textDocument/inlayHint average: 0.00143 std_dev: 0.00012
               textDocument/hover average: 0.001416 std_dev: 0.000103
          textDocument/completion average: 0.001364 std_dev: 7.8e-05
          textDocument/formatting average: 0.000874 std_dev: 0.000296
    textDocument/onTypeFormatting average: 0.000873 std_dev: 8.3e-05
          textDocument/codeAction average: 0.000844 std_dev: 0.0001


================================================================================
Comparison with main branch:

 textDocument/semanticTokens/full unchanged
textDocument/semanticTokens/range unchanged
      textDocument/documentSymbol unchanged
        textDocument/foldingRange unchanged
          textDocument/formatting unchanged
          textDocument/diagnostic unchanged
        textDocument/documentLink unchanged
           textDocument/inlayHint unchanged
      textDocument/selectionRange unchanged
   textDocument/documentHighlight unchanged
               textDocument/hover unchanged
          textDocument/codeAction unchanged
    textDocument/onTypeFormatting unchanged
               codeAction/resolve unchanged
          textDocument/completion faster by 99.331 %
            textDocument/codeLens unchanged
          textDocument/definition unchanged


================================================================================
Missing benchmarks:

RubyLsp::Requests::ShowSyntaxTree

andyw8 · 2023-08-30T20:22:15Z

lib/ruby_lsp/executor.rb

+      # When the user types in the first letter of a constant name, we actually receive the position of the next
+      # immediate character. We check to see if the character is uppercase and then remove the offset to try to locate
+      # the node, as it could not be a constant
+      target_node_types = if ("A".."Z").cover?(document.source[char_position - 1])


What about constants beginning with letters from other alphabets, e.g. Á? 😁

Or just _FOO

_FOO would just be an odd-looking local variable:

irb(main):001:0> _FOO = 1 => 1 irb(main):002:0> defined? _FOO => "local-variable"

How do we get all upper case characters including the ones with accents in Ruby? I never seen this done before. And we need to return the list of trigger characters to the editor.

Maybe with /[[:upper:]]/, if that's consistent with what Ruby considers for constants:

irb(main):003> "Á".match?(/[[:upper:]]/) => true

(if you can find the lower and upper bounds, you can use that for the input)

With a little help from ChatGPT 😁

# Define the range of Unicode characters unicode_range = 0..0x10FFFF # Initialize an array to store the uppercase characters uppercase_chars = [] # Iterate over the unicode range unicode_range.each do |i| char = [i].pack('U*') if char =~ /[[:upper:]]/ uppercase_chars << char end rescue # to avoid 'invalid byte sequence in UTF-8' for some values nil end.compact # Output the uppercase characters puts uppercase_chars

(1951 results)

I created #990 to explore this in more detail. For now, let's ship without unicode support.

It is usually best to ask Ruby to compile a snippet with the given input and see if Ruby treats it as a constant:

pry> RubyVM::InstructionSequence.compile("Ã = 42", nil, nil, 0, false).to_a.dig(-1, -2, 0) == :setconstant => true pry> RubyVM::InstructionSequence.compile("_FOO = 42", nil, nil, 0, false).to_a.dig(-1, -2, 0) == :setconstant => false pry> RubyVM::InstructionSequence.compile("FOO = 42", nil, nil, 0, false).to_a.dig(-1, -2, 0) == :setconstant => true pry> RubyVM::InstructionSequence.compile("Ҷ = 42", nil, nil, 0, false).to_a.dig(-1, -2, 0) == :setconstant => true

lib/ruby_lsp/requests/support/common.rb

Morriar · 2023-08-31T17:55:11Z

lib/ruby_lsp/executor.rb

+      # When the user types in the first letter of a constant name, we actually receive the position of the next
+      # immediate character. We check to see if the character is uppercase and then remove the offset to try to locate
+      # the node, as it could not be a constant
+      target_node_types = if ("A".."Z").cover?(document.source[char_position - 1])


Or just _FOO

lib/ruby_lsp/executor.rb

test/requests/completion_test.rb

Morriar · 2023-08-31T18:01:33Z

lib/ruby_lsp/requests/completion.rb

+          Constant::CompletionItemKind::REFERENCE
+        end
+
+        insertion_text = first_entry.name.dup


Is this dup necessary? Is name mutable?

We're mutating it on the lines below. If we don't dup it, we end up mutating the index entry itself.

We were actually bit by this in the past

a = first_entry.name # => Foo::Bar a.delete_prefix!("Foo::") first_entry.name # => Bar

The example config as show would result in one config key being overwritten by the other so I thought it would be better to simplify the example to show one instance of the `rubyLsp.bundleGemfile` key and adding a note about relative and absolute path support.

vinistock added the enhancement New feature or request label Aug 30, 2023

vinistock added this to the 2023-Q3 milestone Aug 30, 2023

vinistock self-assigned this Aug 30, 2023

vinistock requested a review from a team as a code owner August 30, 2023 19:49

vinistock requested review from andyw8 and st0012 August 30, 2023 19:49

andyw8 reviewed Aug 30, 2023

View reviewed changes

lib/ruby_lsp/requests/support/common.rb Outdated Show resolved Hide resolved

andyw8 reviewed Aug 30, 2023

View reviewed changes

lib/ruby_lsp/requests/support/common.rb Outdated Show resolved Hide resolved

Morriar reviewed Aug 31, 2023

View reviewed changes

vinistock force-pushed the vs/add_entries_prefix_tree branch from 0647342 to 357a126 Compare September 1, 2023 17:59

Base automatically changed from vs/add_entries_prefix_tree to main September 1, 2023 18:29

vinistock force-pushed the vs/add_autocomplete branch from 934956b to 4605f6e Compare September 1, 2023 19:44

vinistock requested review from Morriar and andyw8 September 1, 2023 19:47

This was referenced Sep 5, 2023

Index modules and classes starting with unicode characters #973

Closed

Handle unicode uppercase characters in autocomplete #990

Closed

Add autocomplete for classes, modules and constants

d6dbb33

vinistock force-pushed the vs/add_autocomplete branch from 4605f6e to d6dbb33 Compare September 8, 2023 18:30

andyw8 approved these changes Sep 8, 2023

View reviewed changes

Morriar approved these changes Sep 8, 2023

View reviewed changes

vinistock merged commit 721dbda into main Sep 8, 2023
25 of 29 checks passed

vinistock deleted the vs/add_autocomplete branch September 8, 2023 18:44

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add autocomplete for classes, modules and constants #957

Add autocomplete for classes, modules and constants #957

vinistock commented Aug 30, 2023

github-actions bot commented Aug 30, 2023

andyw8 Aug 30, 2023

Morriar Aug 31, 2023

andyw8 Aug 31, 2023

Morriar Aug 31, 2023

vinistock Sep 1, 2023

andyw8 Sep 1, 2023

andyw8 Sep 1, 2023

andyw8 Sep 1, 2023

vinistock Sep 8, 2023

paracycle Sep 8, 2023

Morriar Aug 31, 2023

Morriar Aug 31, 2023

vinistock Sep 1, 2023

Add autocomplete for classes, modules and constants #957

Add autocomplete for classes, modules and constants #957

Conversation

vinistock commented Aug 30, 2023

Motivation

Implementation

Automated Tests

Manual Tests

github-actions bot commented Aug 30, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment