From 8d80bc65e69f8da5fd798b35a10244cb142a98b7 Mon Sep 17 00:00:00 2001 From: Sean Graves Date: Mon, 18 Dec 2023 16:52:38 -0500 Subject: [PATCH] Support Sorbet `typed: false` files for completion requests (#1245) * enable completion on typed: false files Co-authored-by: Joshua Scharf * Add tests for completion.rb Co-authored-by: Joshua Scharf * 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 --- lib/ruby_lsp/document.rb | 12 ++++++ lib/ruby_lsp/executor.rb | 2 +- lib/ruby_lsp/requests/completion.rb | 6 ++- test/requests/completion_test.rb | 58 +++++++++++++++++++++++++++- test/ruby_document_test.rb | 59 +++++++++++++++++++++++++++++ 5 files changed, 133 insertions(+), 4 deletions(-) diff --git a/lib/ruby_lsp/document.rb b/lib/ruby_lsp/document.rb index 9434e31c9..399c90475 100644 --- 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 index 1de878af3..ac112c9aa 100644 --- 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 index 5a3144be7..7acd50896 100644 --- 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 index 65db81927..8e1badd39 100644 --- 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 index b8bbc8967..ab9a8b14f 100644 --- 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)