Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Introduce guessed receiver types #2210

Merged
merged 5 commits into from
Jul 23, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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,
vinistock marked this conversation as resolved.
Show resolved Hide resolved
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)
vinistock marked this conversation as resolved.
Show resolved Hide resolved
@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
Loading