Support Sorbet typed: false files for completion requests (#1245)

* enable completion on typed: false files Co-authored-by: Joshua Scharf <[email protected]> * Add tests for completion.rb Co-authored-by: Joshua Scharf <[email protected]> * use prism to look for magic comments * rename method and clean up tests * use asser/refute_predicate * Add a test for other comments / strings * Remove completion on constants --------- Co-authored-by: Joshua Scharf <[email protected]>
Shopify · Dec 18, 2023 · 8d80bc6 · 8d80bc6
1 parent 2233c04
commit 8d80bc6
Show file tree

Hide file tree

Showing 5 changed files with 133 additions and 4 deletions.
diff --git a/lib/ruby_lsp/document.rb b/lib/ruby_lsp/document.rb
@@ -173,6 +173,18 @@ def locate(node, char_position, node_types: [])
       [closest, parent, nesting.map { |n| n.constant_path.location.slice }]
     end
 
+    sig { returns(T::Boolean) }
+    def sorbet_sigil_is_true_or_higher
+      parse_result.magic_comments.any? do |comment|
+        comment.key == "typed" && ["true", "strict", "strong"].include?(comment.value)
+      end
+    end
+
+    sig { returns(T::Boolean) }
+    def typechecker_enabled?
+      DependencyDetector.instance.typechecker && sorbet_sigil_is_true_or_higher
+    end
+
     class Scanner
       extend T::Sig
 

diff --git a/lib/ruby_lsp/executor.rb b/lib/ruby_lsp/executor.rb
@@ -555,7 +555,7 @@ def completion(uri, position)
       return unless target
 
       dispatcher = Prism::Dispatcher.new
-      listener = Requests::Completion.new(@index, nesting, dispatcher)
+      listener = Requests::Completion.new(@index, nesting, dispatcher, document.typechecker_enabled?)
       dispatcher.dispatch_once(target)
       listener.response
     end

diff --git a/lib/ruby_lsp/requests/completion.rb b/lib/ruby_lsp/requests/completion.rb
@@ -36,13 +36,15 @@ class Completion < Listener
           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)
         super(dispatcher)
         @_response = T.let([], ResponseType)
         @index = index
         @nesting = nesting
+        @typechecker_enabled = typechecker_enabled
 
         dispatcher.register(
           self,
@@ -125,7 +127,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)
 
         name = node.message

diff --git a/test/requests/completion_test.rb b/test/requests/completion_test.rb
@@ -9,7 +9,6 @@ def setup
     @uri = URI("file:///fake.rb")
     @store = RubyLsp::Store.new
     @executor = RubyLsp::Executor.new(@store, @message_queue)
-    stub_no_typechecker
   end
 
   def teardown
@@ -175,6 +174,7 @@ def test_completion_is_not_triggered_if_argument_is_not_a_string
   end
 
   def test_completion_for_constants
+    stub_no_typechecker
     document = RubyLsp::RubyDocument.new(source: +<<~RUBY, version: 1, uri: @uri)
       class Foo
       end
@@ -196,6 +196,7 @@ class Foo
   end
 
   def test_completion_for_constant_paths
+    stub_no_typechecker
     document = RubyLsp::RubyDocument.new(source: +<<~RUBY, version: 1, uri: @uri)
       class Bar
       end
@@ -235,6 +236,7 @@ module Foo
   end
 
   def test_completion_conflicting_constants
+    stub_no_typechecker
     document = RubyLsp::RubyDocument.new(source: +<<~RUBY, version: 1, uri: @uri)
       module Foo
         class Qux; end
@@ -264,6 +266,7 @@ class Qux; end
   end
 
   def test_completion_for_top_level_constants_inside_nesting
+    stub_no_typechecker
     document = RubyLsp::RubyDocument.new(source: +<<~RUBY, version: 1, uri: @uri)
       class Bar
       end
@@ -292,6 +295,7 @@ module Foo
   end
 
   def test_completion_private_constants_inside_the_same_namespace
+    stub_no_typechecker
     document = RubyLsp::RubyDocument.new(source: +<<~RUBY, version: 1, uri: @uri)
       class A
         CONST = 1
@@ -542,6 +546,58 @@ def qux
     assert_equal(["bar", "bar="], result.map { |completion| completion.text_edit.new_text })
   end
 
+  def test_with_typed_false
+    document = RubyLsp::RubyDocument.new(source: +<<~RUBY, version: 1, uri: @uri)
+      # typed: false
+      class Foo
+        def complete_me
+        end
+
+        def you
+          comp
+        end
+      end
+    RUBY
+
+    end_position = { line: 6, character: 8 }
+    @store.set(uri: @uri, source: document.source, version: 1)
+
+    index = @executor.instance_variable_get(:@index)
+    index.index_single(RubyIndexer::IndexablePath.new(nil, @uri.to_standardized_path), document.source)
+
+    result = run_request(
+      method: "textDocument/completion",
+      params: { textDocument: { uri: @uri.to_s }, position: end_position },
+    )
+    assert_equal(["complete_me"], result.map(&:label))
+  end
+
+  def test_with_typed_true
+    document = RubyLsp::RubyDocument.new(source: +<<~RUBY, version: 1, uri: @uri)
+      # typed: true
+      class Foo
+        def complete_me
+        end
+
+        def you
+          comp
+        end
+      end
+    RUBY
+
+    end_position = { line: 6, character: 8 }
+    @store.set(uri: @uri, source: document.source, version: 1)
+
+    index = @executor.instance_variable_get(:@index)
+    index.index_single(RubyIndexer::IndexablePath.new(nil, @uri.to_standardized_path), document.source)
+
+    result = run_request(
+      method: "textDocument/completion",
+      params: { textDocument: { uri: @uri.to_s }, position: end_position },
+    )
+    assert_empty(result)
+  end
+
   private
 
   def run_request(method:, params: {})

diff --git a/test/ruby_document_test.rb b/test/ruby_document_test.rb
@@ -536,6 +536,65 @@ def test_cache_set_and_get
     assert_equal(value, document.cache_get("textDocument/semanticHighlighting"))
   end
 
+  def test_no_sigil
+    document = RubyLsp::RubyDocument.new(
+      source: +"# frozen_string_literal: true",
+      version: 1,
+      uri: URI("file:///foo/bar.rb"),
+    )
+    refute_predicate(document, :sorbet_sigil_is_true_or_higher)
+  end
+
+  def test_sigil_ignore
+    document = RubyLsp::RubyDocument.new(source: +"# typed: ignored", version: 1, uri: URI("file:///foo/bar.rb"))
+    refute_predicate(document, :sorbet_sigil_is_true_or_higher)
+  end
+
+  def test_sigil_false
+    document = RubyLsp::RubyDocument.new(source: +"# typed: false", version: 1, uri: URI("file:///foo/bar.rb"))
+    refute_predicate(document, :sorbet_sigil_is_true_or_higher)
+  end
+
+  def test_sigil_true
+    document = RubyLsp::RubyDocument.new(source: +"# typed: true", version: 1, uri: URI("file:///foo/bar.rb"))
+    assert_predicate(document, :sorbet_sigil_is_true_or_higher)
+  end
+
+  def test_sigil_strict
+    document = RubyLsp::RubyDocument.new(source: +"# typed: strict", version: 1, uri: URI("file:///foo/bar.rb"))
+    assert_predicate(document, :sorbet_sigil_is_true_or_higher)
+  end
+
+  def test_sigil_strong
+    document = RubyLsp::RubyDocument.new(source: +"# typed: strong", version: 1, uri: URI("file:///foo/bar.rb"))
+    assert_predicate(document, :sorbet_sigil_is_true_or_higher)
+  end
+
+  def test_sorbet_sigil_only_in_magic_comment
+    document = RubyLsp::RubyDocument.new(source: +<<~RUBY, version: 1, uri: URI("file:///foo/bar.rb"))
+      # typed: false
+
+      def foo
+        some_string = "# typed: true"
+      end
+
+      # Shouldn't be tricked by the following comment:
+      # ```
+      # # typed: strict
+      #
+      # def main; end
+      # ```
+      def bar; end
+
+      def baz
+        <<-CODE
+          # typed: strong
+        CODE
+      end
+    RUBY
+    refute_predicate(document, :sorbet_sigil_is_true_or_higher)
+  end
+
   private
 
   def assert_error_edit(actual, error_range)