Handle multibyte characters in RubyDocument (#2669)

NotFounds · web-flow · commit eca961380e26 · 2024-10-04T15:18:05.000-04:00
diff --git a/lib/ruby_lsp/erb_document.rb b/lib/ruby_lsp/erb_document.rb
@@ -50,7 +50,12 @@ def language_id
       ).returns(NodeContext)
     end
     def locate_node(position, node_types: [])
-      RubyDocument.locate(@parse_result.value, create_scanner.find_char_position(position), node_types: node_types)
+      RubyDocument.locate(
+        @parse_result.value,
+        create_scanner.find_char_position(position),
+        node_types: node_types,
+        encoding: @encoding,
+      )
     end
 
     sig { params(char_position: Integer).returns(T.nilable(T::Boolean)) }
diff --git a/lib/ruby_lsp/requests/code_action_resolve.rb b/lib/ruby_lsp/requests/code_action_resolve.rb
@@ -23,10 +23,11 @@ class Error < ::T::Enum
         end
       end
 
-      sig { params(document: RubyDocument, code_action: T::Hash[Symbol, T.untyped]).void }
-      def initialize(document, code_action)
+      sig { params(document: RubyDocument, global_state: GlobalState, code_action: T::Hash[Symbol, T.untyped]).void }
+      def initialize(document, global_state, code_action)
         super()
         @document = document
+        @global_state = global_state
         @code_action = code_action
       end
 
@@ -191,7 +192,12 @@ def refactor_method
         extracted_source = T.must(@document.source[start_index...end_index])
 
         # Find the closest method declaration node, so that we place the refactor in a valid position
-        node_context = RubyDocument.locate(@document.parse_result.value, start_index, node_types: [Prism::DefNode])
+        node_context = RubyDocument.locate(
+          @document.parse_result.value,
+          start_index,
+          node_types: [Prism::DefNode],
+          encoding: @global_state.encoding,
+        )
         closest_node = node_context.node
         return Error::InvalidTargetRange unless closest_node
 
diff --git a/lib/ruby_lsp/requests/completion.rb b/lib/ruby_lsp/requests/completion.rb
@@ -57,6 +57,7 @@ def initialize(document, global_state, params, sorbet_level, dispatcher)
             Prism::InstanceVariableTargetNode,
             Prism::InstanceVariableWriteNode,
           ],
+          encoding: global_state.encoding,
         )
         @response_builder = T.let(
           ResponseBuilders::CollectionResponseBuilder[Interface::CompletionItem].new,
diff --git a/lib/ruby_lsp/requests/definition.rb b/lib/ruby_lsp/requests/definition.rb
@@ -57,6 +57,7 @@ def initialize(document, global_state, position, dispatcher, sorbet_level)
             Prism::SuperNode,
             Prism::ForwardingSuperNode,
           ],
+          encoding: global_state.encoding,
         )
 
         target = node_context.node
diff --git a/lib/ruby_lsp/requests/document_highlight.rb b/lib/ruby_lsp/requests/document_highlight.rb
@@ -28,7 +28,7 @@ def initialize(global_state, document, position, dispatcher)
         char_position = document.create_scanner.find_char_position(position)
         delegate_request_if_needed!(global_state, document, char_position)
 
-        node_context = RubyDocument.locate(document.parse_result.value, char_position)
+        node_context = RubyDocument.locate(document.parse_result.value, char_position, encoding: global_state.encoding)
 
         @response_builder = T.let(
           ResponseBuilders::CollectionResponseBuilder[Interface::DocumentHighlight].new,
diff --git a/lib/ruby_lsp/requests/hover.rb b/lib/ruby_lsp/requests/hover.rb
@@ -41,6 +41,7 @@ def initialize(document, global_state, position, dispatcher, sorbet_level)
           document.parse_result.value,
           char_position,
           node_types: Listeners::Hover::ALLOWED_TARGETS,
+          encoding: global_state.encoding,
         )
         target = node_context.node
         parent = node_context.parent
diff --git a/lib/ruby_lsp/requests/references.rb b/lib/ruby_lsp/requests/references.rb
@@ -42,6 +42,7 @@ def perform
             Prism::CallNode,
             Prism::DefNode,
           ],
+          encoding: @global_state.encoding,
         )
         target = node_context.node
         parent = node_context.parent
diff --git a/lib/ruby_lsp/requests/rename.rb b/lib/ruby_lsp/requests/rename.rb
@@ -37,6 +37,7 @@ def perform
           @document.parse_result.value,
           char_position,
           node_types: [Prism::ConstantReadNode, Prism::ConstantPathNode, Prism::ConstantPathTargetNode],
+          encoding: @global_state.encoding,
         )
         target = node_context.node
         parent = node_context.parent
diff --git a/lib/ruby_lsp/requests/signature_help.rb b/lib/ruby_lsp/requests/signature_help.rb
@@ -39,7 +39,12 @@ def initialize(document, global_state, position, context, dispatcher, sorbet_lev
         char_position = document.create_scanner.find_char_position(position)
         delegate_request_if_needed!(global_state, document, char_position)
 
-        node_context = RubyDocument.locate(document.parse_result.value, char_position, node_types: [Prism::CallNode])
+        node_context = RubyDocument.locate(
+          document.parse_result.value,
+          char_position,
+          node_types: [Prism::CallNode],
+          encoding: global_state.encoding,
+        )
 
         target = adjust_for_nested_target(node_context.node, node_context.parent, position)
 
diff --git a/lib/ruby_lsp/ruby_document.rb b/lib/ruby_lsp/ruby_document.rb
@@ -26,9 +26,10 @@ class << self
           node: Prism::Node,
           char_position: Integer,
           node_types: T::Array[T.class_of(Prism::Node)],
+          encoding: Encoding,
         ).returns(NodeContext)
       end
-      def locate(node, char_position, node_types: [])
+      def locate(node, char_position, node_types: [], encoding: Encoding::UTF_8)
         queue = T.let(node.child_nodes.compact, T::Array[T.nilable(Prism::Node)])
         closest = node
         parent = T.let(nil, T.nilable(Prism::Node))
@@ -61,16 +62,21 @@ def locate(node, char_position, node_types: [])
 
           # Skip if the current node doesn't cover the desired position
           loc = candidate.location
-          next unless (loc.start_offset...loc.end_offset).cover?(char_position)
+          loc_start_offset = loc.start_code_units_offset(encoding)
+          loc_end_offset = loc.end_code_units_offset(encoding)
+          next unless (loc_start_offset...loc_end_offset).cover?(char_position)
 
           # If the node's start character is already past the position, then we should've found the closest node
           # already
-          break if char_position < loc.start_offset
+          break if char_position < loc_start_offset
 
           # If the candidate starts after the end of the previous nesting level, then we've exited that nesting level
           # and need to pop the stack
           previous_level = nesting_nodes.last
-          nesting_nodes.pop if previous_level && loc.start_offset > previous_level.location.end_offset
+          if previous_level &&
+              (loc_start_offset > previous_level.location.end_code_units_offset(encoding))
+            nesting_nodes.pop
+          end
 
           # Keep track of the nesting where we found the target. This is used to determine the fully qualified name of
           # the target when it is a constant
@@ -83,8 +89,10 @@ def locate(node, char_position, node_types: [])
           if candidate.is_a?(Prism::CallNode)
             arg_loc = candidate.arguments&.location
             blk_loc = candidate.block&.location
-            if (arg_loc && (arg_loc.start_offset...arg_loc.end_offset).cover?(char_position)) ||
-                (blk_loc && (blk_loc.start_offset...blk_loc.end_offset).cover?(char_position))
+            if (arg_loc && (arg_loc.start_code_units_offset(encoding)...
+                            arg_loc.end_code_units_offset(encoding)).cover?(char_position)) ||
+                (blk_loc && (blk_loc.start_code_units_offset(encoding)...
+                            blk_loc.end_code_units_offset(encoding)).cover?(char_position))
               call_node = candidate
             end
           end
@@ -94,7 +102,9 @@ def locate(node, char_position, node_types: [])
 
           # If the current node is narrower than or equal to the previous closest node, then it is more precise
           closest_loc = closest.location
-          if loc.end_offset - loc.start_offset <= closest_loc.end_offset - closest_loc.start_offset
+          closest_node_start_offset = closest_loc.start_code_units_offset(encoding)
+          closest_node_end_offset = closest_loc.end_code_units_offset(encoding)
+          if loc_end_offset - loc_start_offset <= closest_node_end_offset - closest_node_start_offset
             parent = closest
             closest = candidate
           end
@@ -201,7 +211,12 @@ def locate_first_within_range(range, node_types: [])
       ).returns(NodeContext)
     end
     def locate_node(position, node_types: [])
-      RubyDocument.locate(@parse_result.value, create_scanner.find_char_position(position), node_types: node_types)
+      RubyDocument.locate(
+        @parse_result.value,
+        create_scanner.find_char_position(position),
+        node_types: node_types,
+        encoding: @encoding,
+      )
     end
   end
 end
diff --git a/lib/ruby_lsp/server.rb b/lib/ruby_lsp/server.rb
@@ -772,7 +772,7 @@ def code_action_resolve(message)
         return
       end
 
-      result = Requests::CodeActionResolve.new(document, params).perform
+      result = Requests::CodeActionResolve.new(document, @global_state, params).perform
 
       case result
       when Requests::CodeActionResolve::Error::EmptySelection
diff --git a/test/requests/code_action_resolve_expectations_test.rb b/test/requests/code_action_resolve_expectations_test.rb
@@ -7,11 +7,15 @@
 class CodeActionResolveExpectationsTest < ExpectationsTestRunner
   expectations_tests RubyLsp::Requests::CodeActionResolve, "code_action_resolve"
 
+  def setup
+    @global_state = RubyLsp::GlobalState.new
+  end
+
   def run_expectations(source)
     params = @__params&.any? ? @__params : default_args
     document = RubyLsp::RubyDocument.new(source: source, version: 1, uri: URI("file:///fake.rb"))
 
-    RubyLsp::Requests::CodeActionResolve.new(document, params).perform
+    RubyLsp::Requests::CodeActionResolve.new(document, @global_state, params).perform
   end
 
   def assert_expectations(source, expected)
diff --git a/test/requests/definition_expectations_test.rb b/test/requests/definition_expectations_test.rb
@@ -136,6 +136,31 @@ class Baz
     end
   end
 
+  def test_multibyte_character_precision
+    source = <<~RUBY
+      module Fほげ
+        module Bar
+          class Baz
+          end
+        end
+      end
+
+      Fほげ::Bar::Baz
+    RUBY
+
+    with_server(source, stub_no_typechecker: true) do |server, uri|
+      # Foo
+      server.process_message(
+        id: 1,
+        method: "textDocument/definition",
+        params: { textDocument: { uri: uri }, position: { line: 7, character: 0 } },
+      )
+      range = server.pop_response.response[0].attributes[:targetRange].attributes
+      range_hash = { start: range[:start].to_hash, end: range[:end].to_hash }
+      assert_equal({ start: { line: 0, character: 0 }, end: { line: 5, character: 3 } }, range_hash)
+    end
+  end
+
   def test_jumping_to_default_require_of_a_gem
     with_server("require \"bundler\"") do |server, uri|
       index = server.global_state.index
diff --git a/test/ruby_document_test.rb b/test/ruby_document_test.rb
@@ -558,6 +558,26 @@ def baz
     assert_equal(["Foo", "Bar"], node_context.nesting)
   end
 
+  def test_locate_returns_correct_nesting_when_contains_multibyte_characters
+    document = RubyLsp::RubyDocument.new(source: <<~RUBY, version: 1, uri: URI("file:///foo/bar.rb"))
+      module A動物
+        class Bねこ
+          def C鳴く
+            "にゃー"
+          end
+        end
+      end
+    RUBY
+
+    node_context = document.locate_node(
+      { line: 2, character: 8 },
+      node_types: [Prism::DefNode],
+    )
+    found = node_context.node
+    assert_equal(:C鳴く, T.cast(found, Prism::DefNode).name)
+    assert_equal(["A動物", "Bねこ"], node_context.nesting)
+  end
+
   def test_reparsing_without_new_edits_does_nothing
     text = "def foo; end"
 

Original file line number	Diff line number	Diff line change
`@@ -57,6 +57,7 @@ def initialize(document, global_state, params, sorbet_level, dispatcher)`
`57`	`57`	`Prism::InstanceVariableTargetNode,`
`58`	`58`	`Prism::InstanceVariableWriteNode,`
`59`	`59`	`],`
	`60`	`+ encoding: global_state.encoding,`
`60`	`61`	`)`
`61`	`62`	`@response_builder = T.let(`
`62`	`63`	`ResponseBuilders::CollectionResponseBuilder[Interface::CompletionItem].new,`
Original file line number	Diff line number	Diff line change
`@@ -41,6 +41,7 @@ def initialize(document, global_state, position, dispatcher, sorbet_level)`
`41`	`41`	`document.parse_result.value,`
`42`	`42`	`char_position,`
`43`	`43`	`node_types: Listeners::Hover::ALLOWED_TARGETS,`
	`44`	`+ encoding: global_state.encoding,`
`44`	`45`	`)`
`45`	`46`	`target = node_context.node`
`46`	`47`	`parent = node_context.parent`
Original file line number	Diff line number	Diff line change
`@@ -42,6 +42,7 @@ def perform`
`42`	`42`	`Prism::CallNode,`
`43`	`43`	`Prism::DefNode,`
`44`	`44`	`],`
	`45`	`+ encoding: @global_state.encoding,`
`45`	`46`	`)`
`46`	`47`	`target = node_context.node`
`47`	`48`	`parent = node_context.parent`
Original file line number	Diff line number	Diff line change
`@@ -37,6 +37,7 @@ def perform`
`37`	`37`	`@document.parse_result.value,`
`38`	`38`	`char_position,`
`39`	`39`	`node_types: [Prism::ConstantReadNode, Prism::ConstantPathNode, Prism::ConstantPathTargetNode],`
	`40`	`+ encoding: @global_state.encoding,`
`40`	`41`	`)`
`41`	`42`	`target = node_context.node`
`42`	`43`	`parent = node_context.parent`