Skip to content

Commit

Permalink
Address PR feedback
Browse files Browse the repository at this point in the history
  • Loading branch information
vinistock committed Jul 23, 2024
1 parent b961833 commit e597de1
Show file tree
Hide file tree
Showing 11 changed files with 28 additions and 27 deletions.
16 changes: 8 additions & 8 deletions DESIGN_AND_ROADMAP.md
Original file line number Diff line number Diff line change
Expand Up @@ -221,7 +221,7 @@ Interested in contributing? Check out the issues tagged with [help-wanted] or [g

## Guessed types

Guessed types is an experimental features where the Ruby LSP attempts to identify the type of a receiver based on its
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
Expand All @@ -232,13 +232,13 @@ user.name
@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.
> [!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
Expand Down
5 changes: 3 additions & 2 deletions lib/ruby_lsp/listeners/completion.rb
Original file line number Diff line number Diff line change
Expand Up @@ -366,10 +366,11 @@ def complete_methods(node, name)

return unless range

guessed_type = type.is_a?(TypeInferrer::GuessedType)
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 @@ -382,7 +383,7 @@ 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,
},
)
Expand Down
2 changes: 1 addition & 1 deletion 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
5 changes: 2 additions & 3 deletions lib/ruby_lsp/listeners/hover.rb
Original file line number Diff line number Diff line change
Expand Up @@ -177,9 +177,8 @@ def handle_method_hover(message, inherited_only: false)
title = "#{message}#{T.must(methods.first).decorated_parameters}"

if type.is_a?(TypeInferrer::GuessedType)
title << " | guessed receiver: #{type.name}"
link = "https://github.com/Shopify/ruby-lsp/blob/main/DESIGN_AND_ROADMAP.md#guessed-types"
@response_builder.push("[Learn more about guessed types](#{link})\n", category: :links)
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|
Expand Down
5 changes: 2 additions & 3 deletions lib/ruby_lsp/listeners/signature_help.rb
Original file line number Diff line number Diff line change
Expand Up @@ -64,9 +64,8 @@ def on_call_node_enter(node)
title = +""

extra_links = if type.is_a?(TypeInferrer::GuessedType)
title << "guessed receiver: #{type.name}"
link = "https://github.com/Shopify/ruby-lsp/blob/main/DESIGN_AND_ROADMAP.md#guessed-types"
"[Learn more about guessed types](#{link})"
title << "\n\nGuessed receiver: #{type.name}"
"[Learn more about guessed types](#{GUESSED_TYPES_URL})"
end

signature_help = Interface::SignatureHelp.new(
Expand Down
9 changes: 5 additions & 4 deletions lib/ruby_lsp/requests/completion_resolve.rb
Original file line number Diff line number Diff line change
Expand Up @@ -65,10 +65,11 @@ def perform
label = +"#{label}#{first_entry.decorated_parameters}"
end

extra_links = if @item.dig(:data, :guessed_type)
label << " | guessed receiver: #{owner_name}"
link = "https://github.com/Shopify/ruby-lsp/blob/main/DESIGN_AND_ROADMAP.md#guessed-types"
"[Learn more about guessed types](#{link})"
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(
Expand Down
1 change: 1 addition & 0 deletions lib/ruby_lsp/utils.rb
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ module RubyLsp
end,
String,
)
GUESSED_TYPES_URL = "https://github.com/Shopify/ruby-lsp/blob/main/DESIGN_AND_ROADMAP.md#guessed-types"

# A notification to be sent to the client
class Message
Expand Down
4 changes: 2 additions & 2 deletions test/requests/completion_resolve_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -122,13 +122,13 @@ def foo(a, b, c)
existing_item = {
label: "foo",
kind: RubyLsp::Constant::CompletionItemKind::METHOD,
data: { owner_name: "User", guessed_type: true },
data: { owner_name: "User", guessed_type: "User" },
}

server.process_message(id: 1, method: "completionItem/resolve", params: existing_item)

result = server.pop_response.response
assert_match("guessed receiver: User", result[:documentation].value)
assert_match("Guessed receiver: User", result[:documentation].value)
assert_match("Learn more about guessed types", result[:documentation].value)
end
end
Expand Down
2 changes: 1 addition & 1 deletion test/requests/hover_expectations_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -707,7 +707,7 @@ def name; end
)

contents = server.pop_response.response.contents.value
assert_match("guessed receiver: User", contents)
assert_match("Guessed receiver: User", contents)
assert_match("Learn more about guessed types", contents)
end
end
Expand Down
2 changes: 1 addition & 1 deletion test/requests/signature_help_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -392,7 +392,7 @@ def subscribe!(news_letter)
signature = result.signatures.first

assert_equal("subscribe!(news_letter)", signature.label)
assert_match("guessed receiver: User", signature.documentation.value)
assert_match("Guessed receiver: User", signature.documentation.value)
end
end
end
4 changes: 2 additions & 2 deletions test/type_inferrer_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -234,7 +234,7 @@ def initialize
end
RUBY

assert_equal("Foo", @type_inferrer.infer_receiver_type(node_context))
assert_equal("Foo", @type_inferrer.infer_receiver_type(node_context).name)
end

def test_infer_super_receiver
Expand All @@ -246,7 +246,7 @@ def initialize(a, b, c)
end
RUBY

assert_equal("Foo", @type_inferrer.infer_receiver_type(node_context))
assert_equal("Foo", @type_inferrer.infer_receiver_type(node_context).name)
end

private
Expand Down

0 comments on commit e597de1

Please sign in to comment.