Merge branch 'main' into andyw8/use-index-to-detect-test-library

Gemfile.lock

@@ -1,7 +1,7 @@
 PATH
   remote: .
   specs:
-    ruby-lsp (0.23.2)
+    ruby-lsp (0.23.5)
       language_server-protocol (~> 3.17.0)
       prism (>= 1.2, < 2.0)
       rbs (>= 3, < 4)

VERSION

@@ -1 +1 @@
-0.23.2
+0.23.5

exe/ruby-lsp-launcher

@@ -45,6 +45,12 @@ rescue Errno::ECHILD
   # In theory, the child process can finish before we even get to the wait call, but that is not an error
 end
 
+error_path = File.join(".ruby-lsp", "install_error")
+
+install_error = if File.exist?(error_path)
+  Marshal.load(File.read(error_path))
+end
+
 begin
   bundle_env_path = File.join(".ruby-lsp", "bundle_env")
   # We can't require `bundler/setup` because that file prematurely exits the process if setup fails. However, we can't
@@ -67,21 +73,21 @@ begin
 rescue StandardError => e
   # If installing gems failed for any reason, we don't want to exit the process prematurely. We can still provide most
   # features in a degraded mode. We simply save the error so that we can report to the user that certain gems might be
-  # missing, but we respect the LSP life cycle
-  setup_error = e
-  $stderr.puts("Failed to set up composed Bundle\n#{e.full_message}")
+  # missing, but we respect the LSP life cycle.
+  #
+  # If an install error occurred and one of the gems is not installed, Bundler.setup is guaranteed to fail with
+  # `Bundler::GemNotFound`. We don't set `setup_error` here because that would essentially report the same problem twice
+  # to telemetry, which is not useful
+  unless install_error && e.is_a?(Bundler::GemNotFound)
+    setup_error = e
+    $stderr.puts("Failed to set up composed Bundle\n#{e.full_message}")
+  end
 
   # If Bundler.setup fails, we need to restore the original $LOAD_PATH so that we can still require the Ruby LSP server
   # in degraded mode
   $LOAD_PATH.unshift(File.expand_path("../lib", __dir__))
 end
 
-error_path = File.join(".ruby-lsp", "install_error")
-
-install_error = if File.exist?(error_path)
-  Marshal.load(File.read(error_path))
-end
-
 # Now that the bundle is set up, we can begin actually launching the server. Note that `Bundler.setup` will have already
 # configured the load path using the version of the Ruby LSP present in the composed bundle. Do not push any Ruby LSP
 # paths into the load path manually or we may end up requiring the wrong version of the gem

lib/ruby_indexer/lib/ruby_indexer/declaration_listener.rb

@@ -64,7 +64,6 @@ def initialize(index, dispatcher, parse_result, uri, collect_comments: false)
         :on_constant_path_or_write_node_enter,
         :on_constant_path_operator_write_node_enter,
         :on_constant_path_and_write_node_enter,
-        :on_constant_or_write_node_enter,
         :on_constant_write_node_enter,
         :on_constant_or_write_node_enter,
         :on_constant_and_write_node_enter,
@@ -288,7 +287,6 @@ def on_call_node_enter(node)
       when :module_function
         handle_module_function(node)
       when :private_class_method
-        @visibility_stack.push(VisibilityScope.new(visibility: Entry::Visibility::PRIVATE))
         handle_private_class_method(node)
       end
 
@@ -976,39 +974,42 @@ def handle_module_function(node)
 
     sig { params(node: Prism::CallNode).void }
     def handle_private_class_method(node)
-      node.arguments&.arguments&.each do |argument|
-        string_or_symbol_nodes = case argument
-        when Prism::StringNode, Prism::SymbolNode
-          [argument]
-        when Prism::ArrayNode
-          argument.elements
-        else
-          []
-        end
+      arguments = node.arguments&.arguments
+      return unless arguments
 
-        unless string_or_symbol_nodes.empty?
-          # pop the visibility off since there isn't a method definition following `private_class_method`
-          @visibility_stack.pop
-        end
+      # If we're passing a method definition directly to `private_class_method`, push a new private scope. That will be
+      # applied when the indexer finds the method definition and then popped on `call_node_leave`
+      if arguments.first.is_a?(Prism::DefNode)
+        @visibility_stack.push(VisibilityScope.new(visibility: Entry::Visibility::PRIVATE))
+        return
+      end
 
-        string_or_symbol_nodes.each do |string_or_symbol_node|
-          method_name = case string_or_symbol_node
-          when Prism::StringNode
-            string_or_symbol_node.content
-          when Prism::SymbolNode
-            string_or_symbol_node.value
-          end
-          next unless method_name
+      owner_name = @owner_stack.last&.name
+      return unless owner_name
 
-          owner_name = @owner_stack.last&.name
-          next unless owner_name
+      # private_class_method accepts strings, symbols or arrays of strings and symbols as arguments. Here we build a
+      # single list of all of the method names that have to be made private
+      arrays, others = T.cast(
+        arguments.partition { |argument| argument.is_a?(Prism::ArrayNode) },
+        [T::Array[Prism::ArrayNode], T::Array[Prism::Node]],
+      )
+      arrays.each { |array| others.concat(array.elements) }
 
-          entries = @index.resolve_method(method_name, @index.existing_or_new_singleton_class(owner_name).name)
-          next unless entries
+      names = others.filter_map do |argument|
+        case argument
+        when Prism::StringNode
+          argument.unescaped
+        when Prism::SymbolNode
+          argument.value
+        end
+      end
 
-          entries.each do |entry|
-            entry.visibility = Entry::Visibility::PRIVATE
-          end
+      names.each do |name|
+        entries = @index.resolve_method(name, @index.existing_or_new_singleton_class(owner_name).name)
+        next unless entries
+
+        entries.each do |entry|
+          entry.visibility = Entry::Visibility::PRIVATE
         end
       end
     end

lib/ruby_indexer/test/method_test.rb

@@ -909,6 +909,21 @@ def baz; end
       assert_equal("Foo", T.must(entry.owner).name)
     end
 
+    def test_making_several_class_methods_private
+      index(<<~RUBY)
+        class Foo
+          def self.bar; end
+          def self.baz; end
+          def self.qux; end
+
+          private_class_method :bar, :baz, :qux
+
+          def initialize
+          end
+        end
+      RUBY
+    end
+
     private
 
     sig { params(entry: Entry::Method, call_string: String).void }

lib/ruby_lsp/base_server.rb

@@ -90,8 +90,7 @@ def start
         # The following requests need to be executed in the main thread directly to avoid concurrency issues. Everything
         # else is pushed into the incoming queue
         case method
-        when "initialize", "initialized", "textDocument/didOpen", "textDocument/didClose", "textDocument/didChange",
-             "$/cancelRequest"
+        when "initialize", "initialized", "textDocument/didOpen", "textDocument/didClose", "textDocument/didChange"
           process_message(message)
         when "shutdown"
           @global_state.synchronize do
@@ -101,11 +100,7 @@ def start
             @writer.write(Result.new(id: message[:id], response: nil).to_hash)
           end
         when "exit"
-          @global_state.synchronize do
-            status = @incoming_queue.closed? ? 0 : 1
-            send_log_message("Shutdown complete with status #{status}")
-            exit(status)
-          end
+          @global_state.synchronize { exit(@incoming_queue.closed? ? 0 : 1) }
         else
           @incoming_queue << message
         end
@@ -120,8 +115,8 @@ def run_shutdown
       @outgoing_queue.close
       @cancelled_requests.clear
 
-      @worker.join
-      @outgoing_dispatcher.join
+      @worker.terminate
+      @outgoing_dispatcher.terminate
       @store.clear
     end
 

lib/ruby_lsp/document.rb

@@ -40,6 +40,9 @@ class LocationNotFoundError < StandardError; end
     sig { returns(Encoding) }
     attr_reader :encoding
 
+    sig { returns(T.nilable(Edit)) }
+    attr_reader :last_edit
+
     sig { returns(T.any(Interface::SemanticTokens, Object)) }
     attr_accessor :semantic_tokens
 
@@ -54,6 +57,7 @@ def initialize(source:, version:, uri:, global_state:)
       @uri = T.let(uri, URI::Generic)
       @needs_parsing = T.let(true, T::Boolean)
       @parse_result = T.let(T.unsafe(nil), ParseResultType)
+      @last_edit = T.let(nil, T.nilable(Edit))
       parse!
     end
 
@@ -93,20 +97,31 @@ def cache_get(request_name)
 
     sig { params(edits: T::Array[T::Hash[Symbol, T.untyped]], version: Integer).void }
     def push_edits(edits, version:)
-      @global_state.synchronize do
-        edits.each do |edit|
-          range = edit[:range]
-          scanner = create_scanner
+      edits.each do |edit|
+        range = edit[:range]
+        scanner = create_scanner
 
-          start_position = scanner.find_char_position(range[:start])
-          end_position = scanner.find_char_position(range[:end])
+        start_position = scanner.find_char_position(range[:start])
+        end_position = scanner.find_char_position(range[:end])
 
-          @source[start_position...end_position] = edit[:text]
-        end
+        @source[start_position...end_position] = edit[:text]
+      end
+
+      @version = version
+      @needs_parsing = true
+      @cache.clear
+
+      last_edit = edits.last
+      return unless last_edit
+
+      last_edit_range = last_edit[:range]
 
-        @version = version
-        @needs_parsing = true
-        @cache.clear
+      @last_edit = if last_edit_range[:start] == last_edit_range[:end]
+        Insert.new(last_edit_range)
+      elsif last_edit[:text].empty?
+        Delete.new(last_edit_range)
+      else
+        Replace.new(last_edit_range)
       end
     end
 
@@ -144,6 +159,25 @@ def create_scanner
       Scanner.new(@source, @encoding)
     end
 
+    class Edit
+      extend T::Sig
+      extend T::Helpers
+
+      abstract!
+
+      sig { returns(T::Hash[Symbol, T.untyped]) }
+      attr_reader :range
+
+      sig { params(range: T::Hash[Symbol, T.untyped]).void }
+      def initialize(range)
+        @range = range
+      end
+    end
+
+    class Insert < Edit; end
+    class Replace < Edit; end
+    class Delete < Edit; end
+
     class Scanner
       extend T::Sig
 

lib/ruby_lsp/listeners/definition.rb

@@ -330,7 +330,8 @@ def handle_method_definition(message, receiver_type, inherited_only: false)
 
         methods.each do |target_method|
           uri = target_method.uri
-          next if sorbet_level_true_or_higher?(@sorbet_level) && not_in_dependencies?(T.must(uri.full_path))
+          full_path = uri.full_path
+          next if sorbet_level_true_or_higher?(@sorbet_level) && (!full_path || not_in_dependencies?(full_path))
 
           @response_builder << Interface::LocationLink.new(
             target_uri: uri.to_s,
@@ -403,7 +404,11 @@ def find_in_index(value)
           # additional behavior on top of jumping to RBIs. The only sigil where Sorbet cannot handle constants is typed
           # ignore
           uri = entry.uri
-          next if @sorbet_level != RubyDocument::SorbetLevel::Ignore && not_in_dependencies?(T.must(uri.full_path))
+          full_path = uri.full_path
+
+          if @sorbet_level != RubyDocument::SorbetLevel::Ignore && (!full_path || not_in_dependencies?(full_path))
+            next
+          end
 
           @response_builder << Interface::LocationLink.new(
             target_uri: uri.to_s,

lib/ruby_lsp/requests/rename.rb

@@ -116,8 +116,8 @@ def collect_file_renames(fully_qualified_name, document_changes)
         T.must(@global_state.index[fully_qualified_name]).each do |entry|
           # Do not rename files that are not part of the workspace
           uri = entry.uri
-          file_path = T.must(uri.full_path)
-          next unless file_path.start_with?(@global_state.workspace_path)
+          file_path = uri.full_path
+          next unless file_path&.start_with?(@global_state.workspace_path)
 
           case entry
           when RubyIndexer::Entry::Class, RubyIndexer::Entry::Module, RubyIndexer::Entry::Constant,

lib/ruby_lsp/requests/workspace_symbol.rb

@@ -22,10 +22,10 @@ def initialize(global_state, query)
       def perform
         @index.fuzzy_search(@query).filter_map do |entry|
           uri = entry.uri
-          file_path = T.must(uri.full_path)
+          file_path = uri.full_path
 
           # We only show symbols declared in the workspace
-          in_dependencies = !not_in_dependencies?(file_path)
+          in_dependencies = file_path && !not_in_dependencies?(file_path)
           next if in_dependencies
 
           # We should never show private symbols when searching the entire workspace