Refactor dependency detector (#1042)

Refactor DependencyDetector

lib/ruby_lsp/executor.rb

@@ -13,7 +13,6 @@ def initialize(store, message_queue)
       # Requests that mutate the store must be run sequentially! Parallel requests only receive a temporary copy of the
       # store
       @store = store
-      @test_library = T.let(DependencyDetector.detected_test_library, String)
       @message_queue = message_queue
       @index = T.let(RubyIndexer::Index.new, RubyIndexer::Index)
     end
@@ -97,7 +96,7 @@ def run(request)
         folding_range = Requests::FoldingRanges.new(document.parse_result.comments, emitter, @message_queue)
         document_symbol = Requests::DocumentSymbol.new(emitter, @message_queue)
         document_link = Requests::DocumentLink.new(uri, document.comments, emitter, @message_queue)
-        code_lens = Requests::CodeLens.new(uri, @test_library, emitter, @message_queue)
+        code_lens = Requests::CodeLens.new(uri, emitter, @message_queue)
 
         semantic_highlighting = Requests::SemanticHighlighting.new(emitter, @message_queue)
         emitter.visit(document.tree)
@@ -254,7 +253,13 @@ def definition(uri, position)
       target = parent if target.is_a?(YARP::ConstantReadNode) && parent.is_a?(YARP::ConstantPathNode)
 
       emitter = EventEmitter.new
-      base_listener = Requests::Definition.new(uri, nesting, @index, emitter, @message_queue)
+      base_listener = Requests::Definition.new(
+        uri,
+        nesting,
+        @index,
+        emitter,
+        @message_queue,
+      )
       emitter.emit_for_target(target)
       base_listener.response
     end
@@ -498,7 +503,12 @@ def completion(uri, position)
       return unless target
 
       emitter = EventEmitter.new
-      listener = Requests::Completion.new(@index, nesting, emitter, @message_queue)
+      listener = Requests::Completion.new(
+        @index,
+        nesting,
+        emitter,
+        @message_queue,
+      )
       emitter.emit_for_target(target)
       listener.response
     end
@@ -550,7 +560,7 @@ def initialize_request(options)
       @store.supports_progress = options.dig(:capabilities, :window, :workDoneProgress) || true
       formatter = options.dig(:initializationOptions, :formatter) || "auto"
       @store.formatter = if formatter == "auto"
-        DependencyDetector.detected_formatter
+        DependencyDetector.instance.detected_formatter
       else
         formatter
       end

lib/ruby_lsp/requests/code_lens.rb

@@ -31,10 +31,9 @@ class CodeLens < ExtensibleListener
       sig { override.returns(ResponseType) }
       attr_reader :_response
 
-      sig { params(uri: URI::Generic, test_library: String, emitter: EventEmitter, message_queue: Thread::Queue).void }
-      def initialize(uri, test_library, emitter, message_queue)
+      sig { params(uri: URI::Generic, emitter: EventEmitter, message_queue: Thread::Queue).void }
+      def initialize(uri, emitter, message_queue)
         @uri = T.let(uri, URI::Generic)
-        @test_library = T.let(test_library, String)
         @_response = T.let([], ResponseType)
         @path = T.let(uri.to_standardized_path, T.nilable(String))
         # visibility_stack is a stack of [current_visibility, previous_visibility]
@@ -145,7 +144,7 @@ def merge_response!(other)
       sig { params(node: YARP::Node, name: String, command: String, kind: Symbol).void }
       def add_test_code_lens(node, name:, command:, kind:)
         # don't add code lenses if the test library is not supported or unknown
-        return unless SUPPORTED_TEST_LIBRARIES.include?(@test_library) && @path
+        return unless SUPPORTED_TEST_LIBRARIES.include?(DependencyDetector.instance.detected_test_library) && @path
 
         arguments = [
           @path,
@@ -198,7 +197,7 @@ def resolve_gem_remote(gem_name)
       def generate_test_command(class_name:, method_name: nil)
         command = BASE_COMMAND + T.must(@path)
 
-        case @test_library
+        case DependencyDetector.instance.detected_test_library
         when "minitest"
           command += if method_name
             " --name " + "/#{Shellwords.escape(class_name + "#" + method_name)}/"

lib/ruby_lsp/requests/completion.rb

@@ -53,7 +53,7 @@ def on_string(node)
       # Handle completion on regular constant references (e.g. `Bar`)
       sig { params(node: YARP::ConstantReadNode).void }
       def on_constant_read(node)
-        return if DependencyDetector::HAS_TYPECHECKER
+        return if DependencyDetector.instance.typechecker
 
         name = node.slice
         candidates = @index.prefix_search(name, @nesting)
@@ -65,7 +65,7 @@ def on_constant_read(node)
       # Handle completion on namespaced constant references (e.g. `Foo::Bar`)
       sig { params(node: YARP::ConstantPathNode).void }
       def on_constant_path(node)
-        return if DependencyDetector::HAS_TYPECHECKER
+        return if DependencyDetector.instance.typechecker
 
         name = node.slice
 

lib/ruby_lsp/requests/definition.rb

@@ -45,7 +45,6 @@ def initialize(uri, nesting, index, emitter, message_queue)
 
         emitter.register(self, :on_call, :on_constant_read, :on_constant_path)
       end
-
       sig { override.params(addon: Addon).returns(T.nilable(RubyLsp::Listener[ResponseType])) }
       def initialize_external_listener(addon)
         addon.create_definition_listener(@uri, @nesting, @index, @emitter, @message_queue)
@@ -145,7 +144,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
-          if DependencyDetector::HAS_TYPECHECKER && bundle_path && !file_path.start_with?(bundle_path) &&
+          if DependencyDetector.instance.typechecker && bundle_path && !file_path.start_with?(bundle_path) &&
               !file_path.start_with?(RbConfig::CONFIG["rubylibdir"])
 
             next

lib/ruby_lsp/requests/hover.rb

@@ -41,8 +41,8 @@ class Hover < ExtensibleListener
         ).void
       end
       def initialize(index, nesting, emitter, message_queue)
-        @nesting = nesting
         @index = index
+        @nesting = nesting
         @_response = T.let(nil, ResponseType)
 
         super(emitter, message_queue)
@@ -71,21 +71,21 @@ def merge_response!(other)
 
       sig { params(node: YARP::ConstantReadNode).void }
       def on_constant_read(node)
-        return if DependencyDetector::HAS_TYPECHECKER
+        return if DependencyDetector.instance.typechecker
 
         generate_hover(node.slice, node.location)
       end
 
       sig { params(node: YARP::ConstantWriteNode).void }
       def on_constant_write(node)
-        return if DependencyDetector::HAS_TYPECHECKER
+        return if DependencyDetector.instance.typechecker
 
         generate_hover(node.name.to_s, node.name_loc)
       end
 
       sig { params(node: YARP::ConstantPathNode).void }
       def on_constant_path(node)
-        return if DependencyDetector::HAS_TYPECHECKER
+        return if DependencyDetector.instance.typechecker
 
         generate_hover(node.slice, node.location)
       end

lib/ruby_lsp/requests/support/dependency_detector.rb

@@ -1,55 +1,80 @@
 # typed: strict
 # frozen_string_literal: true
 
+require "singleton"
+
 module RubyLsp
-  module DependencyDetector
-    class << self
-      extend T::Sig
-
-      sig { returns(String) }
-      def detected_formatter
-        # NOTE: Intentionally no $ at end, since we want to match rubocop-shopify, etc.
-        if direct_dependency?(/^rubocop/)
-          "rubocop"
-        elsif direct_dependency?(/^syntax_tree$/)
-          "syntax_tree"
-        else
-          "none"
-        end
-      end
+  class DependencyDetector
+    include Singleton
+    extend T::Sig
 
-      sig { returns(String) }
-      def detected_test_library
-        # A Rails app may have a dependency on minitest, but we would instead want to use the Rails test runner provided
-        # by ruby-lsp-rails.
-        if direct_dependency?(/^rails$/)
-          "rails"
-        # NOTE: Intentionally ends with $ to avoid mis-matching minitest-reporters, etc. in a Rails app.
-        elsif direct_dependency?(/^minitest$/)
-          "minitest"
-        elsif direct_dependency?(/^test-unit/)
-          "test-unit"
-        elsif direct_dependency?(/^rspec/)
-          "rspec"
-        else
-          "unknown"
-        end
-      end
+    sig { returns(String) }
+    attr_reader :detected_formatter
+
+    sig { returns(String) }
+    attr_reader :detected_test_library
+
+    sig { returns(T::Boolean) }
+    attr_reader :typechecker
+
+    sig { void }
+    def initialize
+      @detected_formatter = T.let(detect_formatter, String)
+      @detected_test_library = T.let(detect_test_library, String)
+      @typechecker = T.let(detect_typechecker, T::Boolean)
+    end
 
-      sig { returns(T::Boolean) }
-      def typechecker?
-        direct_dependency?(/^sorbet/) || direct_dependency?(/^sorbet-static-and-runtime/)
+    sig { returns(String) }
+    def detect_formatter
+      # NOTE: Intentionally no $ at end, since we want to match rubocop-shopify, etc.
+      if direct_dependency?(/^rubocop/)
+        "rubocop"
+      elsif direct_dependency?(/^syntax_tree$/)
+        "syntax_tree"
+      else
+        "none"
       end
+    end
 
-      sig { params(gem_pattern: Regexp).returns(T::Boolean) }
-      def direct_dependency?(gem_pattern)
-        Bundler.with_original_env { Bundler.default_gemfile } &&
-          Bundler.locked_gems.dependencies.keys.grep(gem_pattern).any?
-      rescue Bundler::GemfileNotFound
-        false
+    sig { returns(String) }
+    def detect_test_library
+      # A Rails app may have a dependency on minitest, but we would instead want to use the Rails test runner provided
+      # by ruby-lsp-rails.
+      if direct_dependency?(/^rails$/)
+        "rails"
+      # NOTE: Intentionally ends with $ to avoid mis-matching minitest-reporters, etc. in a Rails app.
+      elsif direct_dependency?(/^minitest$/)
+        "minitest"
+      elsif direct_dependency?(/^test-unit/)
+        "test-unit"
+      elsif direct_dependency?(/^rspec/)
+        "rspec"
+      else
+        "unknown"
       end
     end
 
-    HAS_TYPECHECKER = T.let(typechecker?, T::Boolean)
+    sig { params(gem_pattern: Regexp).returns(T::Boolean) }
+    def direct_dependency?(gem_pattern)
+      dependency_keys.grep(gem_pattern).any?
+    end
+
+    sig { returns(T::Boolean) }
+    def detect_typechecker
+      direct_dependency?(/^sorbet/) || direct_dependency?(/^sorbet-static-and-runtime/)
+    end
+
+    sig { returns(T::Array[String]) }
+    def dependency_keys
+      @dependency_keys ||= T.let(
+        begin
+          Bundler.with_original_env { Bundler.default_gemfile }
+          Bundler.locked_gems.dependencies.keys
+        rescue Bundler::GemfileNotFound
+          []
+        end,
+        T.nilable(T::Array[String]),
+      )
+    end
   end
 end

lib/ruby_lsp/requests/workspace_symbol.rb

@@ -39,7 +39,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
-          if DependencyDetector::HAS_TYPECHECKER && bundle_path && !file_path.start_with?(bundle_path) &&
+          if DependencyDetector.instance.typechecker && bundle_path && !file_path.start_with?(bundle_path) &&
               !file_path.start_with?(RbConfig::CONFIG["rubylibdir"])
 
             next

sorbet/rbi/shims/singleton.rbi

@@ -0,0 +1,6 @@
+# typed: true
+
+module Singleton
+  sig { params(klass: Class).void }
+  def self.__init__(klass); end
+end

test/executor_test.rb

@@ -247,6 +247,7 @@ def unload_rubocop_runner
   end
 
   def stub_dependencies(rubocop:, syntax_tree:)
+    Singleton.__init__(RubyLsp::DependencyDetector)
     dependencies = {}
     dependencies["syntax_tree"] = "..." if syntax_tree
     dependencies["rubocop"] = "..." if rubocop

test/expectations/expectations_test_runner.rb

@@ -126,7 +126,7 @@ def ruby_requirement_magic_comment_version(fixture_path)
   private
 
   def test_addon(addon_creation_method, source:)
-    RubyLsp::DependencyDetector.const_set(:HAS_TYPECHECKER, false)
+    stub_no_typechecker
     message_queue = Thread::Queue.new
 
     send(addon_creation_method)
@@ -144,7 +144,6 @@ def test_addon(addon_creation_method, source:)
     yield(executor)
   ensure
     RubyLsp::Addon.addons.clear
-    RubyLsp::DependencyDetector.const_set(:HAS_TYPECHECKER, true)
     T.must(message_queue).close
   end
 

test/requests/code_lens_expectations_test.rb

@@ -12,12 +12,14 @@ def run_expectations(source)
     document = RubyLsp::Document.new(source: source, version: 1, uri: uri)
 
     emitter = RubyLsp::EventEmitter.new
-    listener = RubyLsp::Requests::CodeLens.new(uri, "minitest", emitter, @message_queue)
+    stub_test_library("minitest")
+    listener = RubyLsp::Requests::CodeLens.new(uri, emitter, @message_queue)
     emitter.visit(document.tree)
     listener.response
   end
 
   def test_command_generation_for_test_unit
+    stub_test_library("test-unit")
     source = <<~RUBY
       class FooTest < Test::Unit::TestCase
         def test_bar; end
@@ -28,7 +30,7 @@ def test_bar; end
     document = RubyLsp::Document.new(source: source, version: 1, uri: uri)
 
     emitter = RubyLsp::EventEmitter.new
-    listener = RubyLsp::Requests::CodeLens.new(uri, "test-unit", emitter, @message_queue)
+    listener = RubyLsp::Requests::CodeLens.new(uri, emitter, @message_queue)
     emitter.visit(document.tree)
     response = listener.response
 
@@ -54,7 +56,8 @@ def test_bar; end
     document = RubyLsp::Document.new(source: source, version: 1, uri: uri)
 
     emitter = RubyLsp::EventEmitter.new
-    listener = RubyLsp::Requests::CodeLens.new(uri, "unknown", emitter, @message_queue)
+    stub_test_library("unknown")
+    listener = RubyLsp::Requests::CodeLens.new(uri, emitter, @message_queue)
     emitter.visit(document.tree)
     response = listener.response
 
@@ -72,7 +75,8 @@ def test_bar; end
     document = RubyLsp::Document.new(source: source, version: 1, uri: uri)
 
     emitter = RubyLsp::EventEmitter.new
-    listener = RubyLsp::Requests::CodeLens.new(uri, "rspec", emitter, @message_queue)
+    stub_test_library("rspec")
+    listener = RubyLsp::Requests::CodeLens.new(uri, emitter, @message_queue)
     emitter.visit(document.tree)
     response = listener.response
 
@@ -90,7 +94,8 @@ def test_bar; end
     document = RubyLsp::Document.new(source: source, version: 1, uri: uri)
 
     emitter = RubyLsp::EventEmitter.new
-    listener = RubyLsp::Requests::CodeLens.new(uri, "minitest", emitter, @message_queue)
+    stub_test_library("minitest")
+    listener = RubyLsp::Requests::CodeLens.new(uri, emitter, @message_queue)
     emitter.visit(document.tree)
     response = listener.response
 
@@ -154,4 +159,9 @@ def on_class(node)
       end
     end
   end
+
+  def stub_test_library(name)
+    Singleton.__init__(RubyLsp::DependencyDetector)
+    RubyLsp::DependencyDetector.instance.stubs(:detected_test_library).returns(name)
+  end
 end