Skip to content

Commit

Permalink
Introduce guessed receiver types (#2210)
Browse files Browse the repository at this point in the history
* Introduce guessed types

* Use guessed types in hover

* Use guessed types in signature help

* Use guessed types in completion

* Address PR feedback
  • Loading branch information
vinistock authored Jul 23, 2024
1 parent c05b9cc commit e66513d
Show file tree
Hide file tree
Showing 17 changed files with 360 additions and 44 deletions.
82 changes: 82 additions & 0 deletions DESIGN_AND_ROADMAP.md
Original file line number Diff line number Diff line change
Expand Up @@ -218,3 +218,85 @@ Interested in contributing? Check out the issues tagged with [help-wanted] or [g
[Explore speeding up indexing by caching the index for gems]: https://github.com/Shopify/ruby-lsp/issues/1009
[Add range formatting support for formatters that do support it]: https://github.com/Shopify/ruby-lsp/issues/203
[Add ERB support]: https://github.com/Shopify/ruby-lsp/issues/1055

## Guessed types

Guessed types is an experimental feature where the Ruby LSP attempts to identify the type of a receiver based on its
identifier name. For example:

```ruby
# The receiver identifier here is `user` and so the Ruby LSP will assign to it the `User` type if that class exists
user.name

# Similarly, the receiver identifier here is `post` and so the LSP searches for the `Post` class
@post.like!
```

> [!IMPORTANT] The goal of this experiment is to understand if we can get better accuracy for the code that you already
> have. The hypothesis is that a reasonable amount of code already uses patterns like the ones in the example and, in
> those cases, we can achieve nicer results.
>
> However, identifiers are not the ideal medium for proper type annotations. It would not be possible to express anything
> complex, such as unions, intersections or generics. Additionally, it is very trivial to fool the type guessing by simply
> naming a variable with a type name that doesn't match its actual type.
```ruby
pathname = something_that_returns_an_integer
# This will show methods available in `Pathname`, despite the variable being an Integer
pathname.a
```

We do not recommend renaming methods, instance variables or local variables for the sole purpose of getting better
accuracy - readibility should always come first. For example:

```ruby
# It would not be a good idea to name every string "string" for the sake of getting better accuracy.
# Using descriptive names will outweight the benefits of the more accurate editor experience

# don't
string = something.other_thing

# do
title = something.other_thing
name = foo
```

That said, this feature can also be used for quickly exploring methods available in classes. Simply type the lower case
name of the class and completion can show the methods available.

```ruby
# Any class name as an identifier
pathname.a
integer.a
file.a
```

To guess types, the Ruby LSP will first try to resolve a constant based on the receiver identifier and current nesting.
If that does not identify any valid types, then it will fallback to matching based on the first match for the
unqualified type name. For example:

```ruby
module Admin
class User
end

# Will match to `Admin::User` because the `user` reference is inside the `Admin` namespace
user.a
end

module Blog
class User
end

# Will match to `Blog::User` because the `user` reference is inside the `Blog` namespace
user.a
end

# Will match to the first class that has the unqualified name of `User`. This may return `Admin::User` or `Blog::User`
# randomly
user.a
```

This is an experimental feature and can only be accessed if `initializationOptions.experimentalFeaturesEnabled` is
`true` (the `"rubyLsp.enableExperimentalFeatures": true` setting for VS Code users). If you have feedback about this
experiment, please let us know in a GitHub issue.
28 changes: 28 additions & 0 deletions lib/ruby_indexer/lib/ruby_indexer/index.rb
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,34 @@ def search_require_paths(query)
@require_paths_tree.search(query)
end

# Searches for a constant based on an unqualified name and returns the first possible match regardless of whether
# there are more possible matching entries
sig do
params(
name: String,
).returns(T.nilable(T::Array[T.any(
Entry::Namespace,
Entry::Alias,
Entry::UnresolvedAlias,
Entry::Constant,
)]))
end
def first_unqualified_const(name)
_name, entries = @entries.find do |const_name, _entries|
const_name.end_with?(name)
end

T.cast(
entries,
T.nilable(T::Array[T.any(
Entry::Namespace,
Entry::Alias,
Entry::UnresolvedAlias,
Entry::Constant,
)]),
)
end

# Searches entries in the index based on an exact prefix, intended for providing autocomplete. All possible matches
# to the prefix are returned. The return is an array of arrays, where each entry is the array of entries for a given
# name match. For example:
Expand Down
15 changes: 15 additions & 0 deletions lib/ruby_indexer/test/index_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -1542,6 +1542,21 @@ class Foo
assert_empty(@index.method_completion_candidates("bar", "Foo"))
end

def test_first_unqualified_const
index(<<~RUBY)
module Foo
class Bar; end
end
module Baz
class Bar; end
end
RUBY

entry = T.must(@index.first_unqualified_const("Bar")&.first)
assert_equal("Foo::Bar", entry.name)
end

def test_completion_does_not_duplicate_overridden_methods
index(<<~RUBY)
class Foo
Expand Down
3 changes: 2 additions & 1 deletion lib/ruby_lsp/global_state.rb
Original file line number Diff line number Diff line change
Expand Up @@ -36,10 +36,10 @@ def initialize
@test_library = T.let("minitest", String)
@has_type_checker = T.let(true, T::Boolean)
@index = T.let(RubyIndexer::Index.new, RubyIndexer::Index)
@type_inferrer = T.let(TypeInferrer.new(@index), TypeInferrer)
@supported_formatters = T.let({}, T::Hash[String, Requests::Support::Formatter])
@supports_watching_files = T.let(false, T::Boolean)
@experimental_features = T.let(false, T::Boolean)
@type_inferrer = T.let(TypeInferrer.new(@index, @experimental_features), TypeInferrer)
end

sig { params(identifier: String, instance: Requests::Support::Formatter).void }
Expand Down Expand Up @@ -90,6 +90,7 @@ def apply_options(options)
end

@experimental_features = options.dig(:initializationOptions, :experimentalFeaturesEnabled) || false
@type_inferrer.experimental_features = @experimental_features
end

sig { returns(String) }
Expand Down
10 changes: 7 additions & 3 deletions lib/ruby_lsp/listeners/completion.rb
Original file line number Diff line number Diff line change
Expand Up @@ -262,7 +262,7 @@ def handle_instance_variable_completion(name, location)
type = @type_inferrer.infer_receiver_type(@node_context)
return unless type

@index.instance_variable_completion_candidates(name, type).each do |entry|
@index.instance_variable_completion_candidates(name, type.name).each do |entry|
variable_name = entry.name

label_details = Interface::CompletionItemLabelDetails.new(
Expand Down Expand Up @@ -366,8 +366,11 @@ def complete_methods(node, name)

return unless range

@index.method_completion_candidates(method_name, type).each do |entry|
guessed_type = type.name

@index.method_completion_candidates(method_name, type.name).each do |entry|
entry_name = entry.name
owner_name = entry.owner&.name

label_details = Interface::CompletionItemLabelDetails.new(
description: entry.file_name,
Expand All @@ -380,7 +383,8 @@ def complete_methods(node, name)
text_edit: Interface::TextEdit.new(range: range, new_text: entry_name),
kind: Constant::CompletionItemKind::METHOD,
data: {
owner_name: entry.owner&.name,
owner_name: owner_name,
guessed_type: guessed_type,
},
)
end
Expand Down
8 changes: 4 additions & 4 deletions lib/ruby_lsp/listeners/definition.rb
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ def on_call_node_enter(node)

# Until we can properly infer the receiver type in erb files (maybe with ruby-lsp-rails),
# treating method calls' type as `nil` will allow users to get some completion support first
if @language_id == Document::LanguageId::ERB && inferrer_receiver_type == "Object"
if @language_id == Document::LanguageId::ERB && inferrer_receiver_type&.name == "Object"
inferrer_receiver_type = nil
end

Expand Down Expand Up @@ -176,7 +176,7 @@ def handle_instance_variable_definition(name)
type = @type_inferrer.infer_receiver_type(@node_context)
return unless type

entries = @index.resolve_instance_variable(name, type)
entries = @index.resolve_instance_variable(name, type.name)
return unless entries

entries.each do |entry|
Expand All @@ -194,10 +194,10 @@ def handle_instance_variable_definition(name)
# If by any chance we haven't indexed the owner, then there's no way to find the right declaration
end

sig { params(message: String, receiver_type: T.nilable(String), inherited_only: T::Boolean).void }
sig { params(message: String, receiver_type: T.nilable(TypeInferrer::Type), inherited_only: T::Boolean).void }
def handle_method_definition(message, receiver_type, inherited_only: false)
methods = if receiver_type
@index.resolve_method(message, receiver_type, inherited_only: inherited_only)
@index.resolve_method(message, receiver_type.name, inherited_only: inherited_only)
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
Expand Down
9 changes: 7 additions & 2 deletions lib/ruby_lsp/listeners/hover.rb
Original file line number Diff line number Diff line change
Expand Up @@ -171,11 +171,16 @@ def handle_method_hover(message, inherited_only: false)
type = @type_inferrer.infer_receiver_type(@node_context)
return unless type

methods = @index.resolve_method(message, type, inherited_only: inherited_only)
methods = @index.resolve_method(message, type.name, inherited_only: inherited_only)
return unless methods

title = "#{message}#{T.must(methods.first).decorated_parameters}"

if type.is_a?(TypeInferrer::GuessedType)
title << "\n\nGuessed receiver: #{type.name}"
@response_builder.push("[Learn more about guessed types](#{GUESSED_TYPES_URL})\n", category: :links)
end

categorized_markdown_from_index_entries(title, methods).each do |category, content|
@response_builder.push(content, category: category)
end
Expand All @@ -190,7 +195,7 @@ def handle_instance_variable_hover(name)
type = @type_inferrer.infer_receiver_type(@node_context)
return unless type

entries = @index.resolve_instance_variable(name, type)
entries = @index.resolve_instance_variable(name, type.name)
return unless entries

categorized_markdown_from_index_entries(name, entries).each do |category, content|
Expand Down
11 changes: 9 additions & 2 deletions lib/ruby_lsp/listeners/signature_help.rb
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ def on_call_node_enter(node)
type = @type_inferrer.infer_receiver_type(@node_context)
return unless type

methods = @index.resolve_method(message, type)
methods = @index.resolve_method(message, type.name)
return unless methods

target_method = methods.first
Expand All @@ -61,14 +61,21 @@ def on_call_node_enter(node)
active_parameter += 1
end

title = +""

extra_links = if type.is_a?(TypeInferrer::GuessedType)
title << "\n\nGuessed receiver: #{type.name}"
"[Learn more about guessed types](#{GUESSED_TYPES_URL})"
end

signature_help = Interface::SignatureHelp.new(
signatures: [
Interface::SignatureInformation.new(
label: label,
parameters: parameters.map { |param| Interface::ParameterInformation.new(label: param.name) },
documentation: Interface::MarkupContent.new(
kind: "markdown",
value: markdown_from_index_entries("", methods),
value: markdown_from_index_entries(title, methods, extra_links: extra_links),
),
),
],
Expand Down
13 changes: 10 additions & 3 deletions lib/ruby_lsp/requests/completion_resolve.rb
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ def perform
#
# For example, forgetting to return the `insertText` included in the original item will make the editor use the
# `label` for the text edit instead
label = @item[:label]
label = @item[:label].dup
entries = @index[label] || []

owner_name = @item.dig(:data, :owner_name)
Expand All @@ -62,12 +62,19 @@ def perform
first_entry = T.must(entries.first)

if first_entry.is_a?(RubyIndexer::Entry::Member)
label = "#{label}#{first_entry.decorated_parameters}"
label = +"#{label}#{first_entry.decorated_parameters}"
end

guessed_type = @item.dig(:data, :guessed_type)

extra_links = if guessed_type
label << "\n\nGuessed receiver: #{guessed_type}"
"[Learn more about guessed types](#{GUESSED_TYPES_URL})"
end

@item[:documentation] = Interface::MarkupContent.new(
kind: "markdown",
value: markdown_from_index_entries(label, entries, MAX_DOCUMENTATION_ENTRIES),
value: markdown_from_index_entries(label, entries, MAX_DOCUMENTATION_ENTRIES, extra_links: extra_links),
)

@item
Expand Down
8 changes: 6 additions & 2 deletions lib/ruby_lsp/requests/support/common.rb
Original file line number Diff line number Diff line change
Expand Up @@ -130,13 +130,17 @@ def categorized_markdown_from_index_entries(title, entries, max_entries = nil)
title: String,
entries: T.any(T::Array[RubyIndexer::Entry], RubyIndexer::Entry),
max_entries: T.nilable(Integer),
extra_links: T.nilable(String),
).returns(String)
end
def markdown_from_index_entries(title, entries, max_entries = nil)
def markdown_from_index_entries(title, entries, max_entries = nil, extra_links: nil)
categorized_markdown = categorized_markdown_from_index_entries(title, entries, max_entries)

markdown = +(categorized_markdown[:title] || "")
markdown << "\n\n#{extra_links}" if extra_links

<<~MARKDOWN.chomp
#{categorized_markdown[:title]}
#{markdown}
#{categorized_markdown[:links]}
Expand Down
2 changes: 1 addition & 1 deletion lib/ruby_lsp/test_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ def with_server(source = nil, uri = Kernel.URI("file:///fake.rb"), stub_no_typec
&block)
server = RubyLsp::Server.new(test_mode: true)
server.global_state.stubs(:has_type_checker).returns(false) if stub_no_typechecker
server.global_state.apply_options({})
server.global_state.apply_options({ initializationOptions: { experimentalFeaturesEnabled: true } })
language_id = uri.to_s.end_with?(".erb") ? "erb" : "ruby"

if source
Expand Down
Loading

0 comments on commit e66513d

Please sign in to comment.