Add support for hover and go to definition when typechecker enabled b…

…ut typed: false (#1280) Add support for hover and go to definiton when typechecker enabled but typed: false Co-authored-by: Sean Graves <[email protected]>
Shopify · Jan 8, 2024 · 24147e8 · 24147e8
1 parent 436e3a7
commit 24147e8
Show file tree

Hide file tree

Showing 7 changed files with 29 additions and 19 deletions.
diff --git a/lib/ruby_lsp/executor.rb b/lib/ruby_lsp/executor.rb
@@ -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
@@ -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)

diff --git a/lib/ruby_lsp/requests/definition.rb b/lib/ruby_lsp/requests/definition.rb
@@ -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)
 
@@ -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,
@@ -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,

diff --git a/lib/ruby_lsp/requests/hover.rb b/lib/ruby_lsp/requests/hover.rb
@@ -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(
@@ -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

diff --git a/lib/ruby_lsp/requests/support/common.rb b/lib/ruby_lsp/requests/support/common.rb
@@ -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
 

diff --git a/lib/ruby_lsp/requests/workspace_symbol.rb b/lib/ruby_lsp/requests/workspace_symbol.rb
@@ -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

diff --git a/test/requests/definition_expectations_test.rb b/test/requests/definition_expectations_test.rb
@@ -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
@@ -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])
@@ -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
@@ -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])
@@ -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
@@ -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)

diff --git a/test/requests/hover_expectations_test.rb b/test/requests/hover_expectations_test.rb
@@ -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
@@ -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)
@@ -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
@@ -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)