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

Add support for hover and go to definition when typechecker enabled but typed: false #1280

Merged
merged 1 commit into from
Jan 8, 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
4 changes: 2 additions & 2 deletions lib/ruby_lsp/executor.rb
Original file line number Diff line number Diff line change
Expand Up @@ -301,7 +301,7 @@ def definition(uri, position)
target = parent if target.is_a?(Prism::ConstantReadNode) && parent.is_a?(Prism::ConstantPathNode)

dispatcher = Prism::Dispatcher.new
base_listener = Requests::Definition.new(uri, nesting, @index, dispatcher)
base_listener = Requests::Definition.new(uri, nesting, @index, dispatcher, document.typechecker_enabled?)
dispatcher.dispatch_once(target)
base_listener.response
end
Expand All @@ -327,7 +327,7 @@ def hover(uri, position)

# Instantiate all listeners
dispatcher = Prism::Dispatcher.new
hover = Requests::Hover.new(@index, nesting, dispatcher)
hover = Requests::Hover.new(@index, nesting, dispatcher, document.typechecker_enabled?)

# Emit events for all listeners
dispatcher.dispatch_once(target)
Expand Down
8 changes: 5 additions & 3 deletions lib/ruby_lsp/requests/definition.rb
Original file line number Diff line number Diff line change
Expand Up @@ -37,13 +37,15 @@ class Definition < ExtensibleListener
nesting: T::Array[String],
index: RubyIndexer::Index,
dispatcher: Prism::Dispatcher,
typechecker_enabled: T::Boolean,
).void
end
def initialize(uri, nesting, index, dispatcher)
def initialize(uri, nesting, index, dispatcher, typechecker_enabled)
@uri = uri
@nesting = nesting
@index = index
@_response = T.let(nil, ResponseType)
@typechecker_enabled = typechecker_enabled

super(dispatcher)

Expand Down Expand Up @@ -111,7 +113,7 @@ def handle_method_definition(node)

location = target_method.location
file_path = target_method.file_path
return if defined_in_gem?(file_path)
return if @typechecker_enabled && defined_in_gem?(file_path)

@_response = Interface::Location.new(
uri: URI::Generic.from_path(path: file_path).to_s,
Expand Down Expand Up @@ -180,7 +182,7 @@ def find_in_index(value)
# additional behavior on top of jumping to RBIs. Sorbet can already handle go to definition for all constants
# in the project, even if the files are typed false
file_path = entry.file_path
next if defined_in_gem?(file_path)
next if DependencyDetector.instance.typechecker && defined_in_gem?(file_path)

Interface::Location.new(
uri: URI::Generic.from_path(path: file_path).to_s,
Expand Down
6 changes: 4 additions & 2 deletions lib/ruby_lsp/requests/hover.rb
Original file line number Diff line number Diff line change
Expand Up @@ -46,12 +46,14 @@ def provider
index: RubyIndexer::Index,
nesting: T::Array[String],
dispatcher: Prism::Dispatcher,
typechecker_enabled: T::Boolean,
).void
end
def initialize(index, nesting, dispatcher)
def initialize(index, nesting, dispatcher, typechecker_enabled)
@index = index
@nesting = nesting
@_response = T.let(nil, ResponseType)
@typechecker_enabled = typechecker_enabled

super(dispatcher)
dispatcher.register(
Expand Down Expand Up @@ -106,7 +108,7 @@ def on_constant_path_node_enter(node)

sig { params(node: Prism::CallNode).void }
def on_call_node_enter(node)
return if DependencyDetector.instance.typechecker
return if @typechecker_enabled
return unless self_receiver?(node)

message = node.message
Expand Down
3 changes: 2 additions & 1 deletion lib/ruby_lsp/requests/support/common.rb
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,8 @@ def create_code_lens(node, title:, command_name:, arguments:, data:)

sig { params(file_path: String).returns(T.nilable(T::Boolean)) }
def defined_in_gem?(file_path)
DependencyDetector.instance.typechecker && BUNDLE_PATH && !file_path.start_with?(T.must(BUNDLE_PATH)) &&
BUNDLE_PATH &&
!file_path.start_with?(T.must(BUNDLE_PATH)) &&
!file_path.start_with?(RbConfig::CONFIG["rubylibdir"])
end

Expand Down
2 changes: 1 addition & 1 deletion lib/ruby_lsp/requests/workspace_symbol.rb
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ def run
# If the project is using Sorbet, we let Sorbet handle symbols defined inside the project itself and RBIs, but
# we still return entries defined in gems to allow developers to jump directly to the source
file_path = entry.file_path
next if defined_in_gem?(file_path)
next if DependencyDetector.instance.typechecker && defined_in_gem?(file_path)

# We should never show private symbols when searching the entire workspace
next if entry.visibility == :private
Expand Down
15 changes: 9 additions & 6 deletions test/requests/definition_expectations_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -229,6 +229,8 @@ def test_jumping_to_method_definitions_when_declaration_exists

uri = URI("file:///folder/fake.rb")
source = <<~RUBY
# typed: false

class A
def bar
foo
Expand All @@ -246,10 +248,9 @@ def foo; end
RubyIndexer::IndexablePath.new(nil, T.must(uri.to_standardized_path)), source
)

stub_no_typechecker
response = executor.execute({
method: "textDocument/definition",
params: { textDocument: { uri: "file:///folder/fake.rb" }, position: { character: 4, line: 2 } },
params: { textDocument: { uri: "file:///folder/fake.rb" }, position: { character: 4, line: 4 } },
}).response

assert_equal(uri.to_s, response.attributes[:uri])
Expand All @@ -263,6 +264,8 @@ def test_jumping_to_method_method_calls_on_explicit_self

uri = URI("file:///folder/fake.rb")
source = <<~RUBY
# typed: false

class A
def bar
self.foo
Expand All @@ -280,10 +283,9 @@ def foo; end
RubyIndexer::IndexablePath.new(nil, T.must(uri.to_standardized_path)), source
)

stub_no_typechecker
response = executor.execute({
method: "textDocument/definition",
params: { textDocument: { uri: "file:///folder/fake.rb" }, position: { character: 9, line: 2 } },
params: { textDocument: { uri: "file:///folder/fake.rb" }, position: { character: 9, line: 4 } },
}).response

assert_equal(uri.to_s, response.attributes[:uri])
Expand All @@ -297,6 +299,8 @@ def test_jumping_to_method_definitions_when_declaration_does_not_exist

uri = URI("file:///folder/fake.rb")
source = <<~RUBY
# typed: false

class A
def bar
foo
Expand All @@ -312,10 +316,9 @@ def bar
RubyIndexer::IndexablePath.new(nil, T.must(uri.to_standardized_path)), source
)

stub_no_typechecker
response = executor.execute({
method: "textDocument/definition",
params: { textDocument: { uri: "file:///folder/fake.rb" }, position: { character: 4, line: 2 } },
params: { textDocument: { uri: "file:///folder/fake.rb" }, position: { character: 4, line: 4 } },
}).response

assert_nil(response)
Expand Down
10 changes: 6 additions & 4 deletions test/requests/hover_expectations_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,8 @@ def test_hovering_methods_invoked_on_implicit_self

uri = URI("file:///fake.rb")
source = <<~RUBY
# typed: false

class A
# Hello from `foo`
def foo; end
Expand All @@ -82,10 +84,9 @@ def bar
index = executor.instance_variable_get(:@index)
index.index_single(RubyIndexer::IndexablePath.new(nil, T.must(uri.to_standardized_path)), source)

stub_no_typechecker
response = executor.execute({
method: "textDocument/hover",
params: { textDocument: { uri: uri }, position: { character: 4, line: 5 } },
params: { textDocument: { uri: uri }, position: { character: 4, line: 7 } },
}).response

assert_match("Hello from `foo`", response.contents.value)
Expand All @@ -99,6 +100,8 @@ def test_hovering_methods_invoked_on_explicit_self

uri = URI("file:///fake.rb")
source = <<~RUBY
# typed: false

class A
# Hello from `foo`
def foo; end
Expand All @@ -114,10 +117,9 @@ def bar
index = executor.instance_variable_get(:@index)
index.index_single(RubyIndexer::IndexablePath.new(nil, T.must(uri.to_standardized_path)), source)

stub_no_typechecker
response = executor.execute({
method: "textDocument/hover",
params: { textDocument: { uri: uri }, position: { character: 9, line: 5 } },
params: { textDocument: { uri: uri }, position: { character: 9, line: 7 } },
}).response

assert_match("Hello from `foo`", response.contents.value)
Expand Down
Loading