Utf 8 encoding issue (#3583)

* Fixing a bug where UTF-8 encodings resulted in incorrect char offset
 calculations.

* Revert "Fail requests that are searching for a non existing position (#2938)"

This reverts commit 0d421a6.

* Calculate character length based on encountered bytes

---------
Co-authored-by: Vinicius Stock <[email protected]>

Author: ChallaHalla

lib/ruby_lsp/document.rb

@@ -7,8 +7,6 @@ module RubyLsp
   class Document
     extend T::Generic
 
-    class LocationNotFoundError < StandardError; end
-
     # This maximum number of characters for providing expensive features, like semantic highlighting and diagnostics.
     # This is the same number used by the TypeScript extension in VS Code
     MAXIMUM_CHARACTERS_FOR_EXPENSIVE_FEATURES = 100_000
@@ -176,7 +174,7 @@ class Scanner
       def initialize(source, encoding)
         @current_line = 0 #: Integer
         @pos = 0 #: Integer
-        @source = source.codepoints #: Array[Integer]
+        @bytes_or_codepoints = encoding == Encoding::UTF_8 ? source.bytes : source.codepoints #: Array[Integer]
         @encoding = encoding
       end
 
@@ -185,23 +183,40 @@ def initialize(source, encoding)
       def find_char_position(position)
         # Find the character index for the beginning of the requested line
         until @current_line == position[:line]
-          until LINE_BREAK == @source[@pos]
-            @pos += 1
+          @pos += 1 until LINE_BREAK == @bytes_or_codepoints[@pos]
+          @pos += 1
+          @current_line += 1
+        end
 
-            if @pos >= @source.length
-              # Pack the code points back into the original string to provide context in the error message
-              raise LocationNotFoundError, "Requested position: #{position}\nSource:\n\n#{@source.pack("U*")}"
+        # For UTF-8, the code unit length is the same as bytes, but we want to return the character index
+        requested_position = if @encoding == Encoding::UTF_8
+          character_offset = 0
+          i = @pos
+
+          # Each group of bytes is a character. We advance based on the number of bytes to count how many full
+          # characters we have in the requested offset
+          while i < @pos + position[:character] && i < @bytes_or_codepoints.length
+            byte = @bytes_or_codepoints[i] #: as !nil
+            i += if byte < 0x80 # 1-byte character
+              1
+            elsif byte < 0xE0 # 2-byte character
+              2
+            elsif byte < 0xF0 # 3-byte character
+              3
+            else # 4-byte character
+              4
             end
+
+            character_offset += 1
           end
 
-          @pos += 1
-          @current_line += 1
+          @pos + character_offset
+        else
+          @pos + position[:character]
         end
 
         # The final position is the beginning of the line plus the requested column. If the encoding is UTF-16, we also
         # need to adjust for surrogate pairs
-        requested_position = @pos + position[:character]
-
         if @encoding == Encoding::UTF_16LE
           requested_position -= utf_16_character_position_correction(@pos, requested_position)
         end
@@ -216,7 +231,7 @@ def utf_16_character_position_correction(current_position, requested_position)
         utf16_unicode_correction = 0
 
         until current_position == requested_position
-          codepoint = @source[current_position]
+          codepoint = @bytes_or_codepoints[current_position]
           utf16_unicode_correction += 1 if codepoint && codepoint > SURROGATE_PAIR_START
 
           current_position += 1

lib/ruby_lsp/server.rb

@@ -121,24 +121,12 @@ def process_message(message)
         # If a document is deleted before we are able to process all of its enqueued requests, we will try to read it
         # from disk and it raise this error. This is expected, so we don't include the `data` attribute to avoid
         # reporting these to our telemetry
-        case e
-        when Store::NonExistingDocumentError
+        if e.is_a?(Store::NonExistingDocumentError)
           send_message(Error.new(
             id: message[:id],
             code: Constant::ErrorCodes::INVALID_PARAMS,
             message: e.full_message,
           ))
-        when Document::LocationNotFoundError
-          send_message(Error.new(
-            id: message[:id],
-            code: Constant::ErrorCodes::REQUEST_FAILED,
-            message: <<~MESSAGE,
-              Request #{message[:method]} failed to find the target position.
-              The file might have been modified while the server was in the middle of searching for the target.
-              If you experience this regularly, please report any findings and extra information on
-              https://github.com/Shopify/ruby-lsp/issues/2446
-            MESSAGE
-          ))
         else
           send_message(Error.new(
             id: message[:id],

test/ruby_document_test.rb

@@ -59,6 +59,48 @@ def foo
     RUBY
   end
 
+  def test_multibyte_character_offsets_are_bytes_in_utf8
+    document = RubyLsp::RubyDocument.new(source: +<<~RUBY, version: 1, uri: @uri, global_state: @global_state)
+      bá
+    RUBY
+
+    document.push_edits(
+      [{ range: { start: { line: 0, character: 3 }, end: { line: 0, character: 3 } }, text: "r" }], version: 2
+    )
+
+    assert_equal(<<~RUBY, document.source)
+      bár
+    RUBY
+  end
+
+  def test_multibyte_character_offsets_for_3_byte_character
+    document = RubyLsp::RubyDocument.new(source: +<<~RUBY, version: 1, uri: @uri, global_state: @global_state)
+      bあ
+    RUBY
+
+    document.push_edits(
+      [{ range: { start: { line: 0, character: 4 }, end: { line: 0, character: 4 } }, text: "r" }], version: 2
+    )
+
+    assert_equal(<<~RUBY, document.source)
+      bあr
+    RUBY
+  end
+
+  def test_multibyte_character_offsets_for_4_byte_character
+    document = RubyLsp::RubyDocument.new(source: +<<~RUBY, version: 1, uri: @uri, global_state: @global_state)
+      b🙂
+    RUBY
+
+    document.push_edits(
+      [{ range: { start: { line: 0, character: 5 }, end: { line: 0, character: 5 } }, text: "r" }], version: 2
+    )
+
+    assert_equal(<<~RUBY, document.source)
+      b🙂r
+    RUBY
+  end
+
   def test_deletion_full_node
     document = RubyLsp::RubyDocument.new(source: +<<~RUBY, version: 1, uri: @uri, global_state: @global_state)
       def foo
@@ -275,55 +317,6 @@ def test_multi_cursor_edit
     RUBY
   end
 
-  def test_pushing_edits_to_document_with_unicode
-    document = RubyLsp::RubyDocument.new(source: +<<~RUBY, version: 1, uri: @uri, global_state: @global_state)
-      chars = ["億"]
-    RUBY
-
-    # Write puts 'a' in incremental edits
-    document.push_edits(
-      [{ range: { start: { line: 0, character: 13 }, end: { line: 0, character: 13 } }, text: "\n" }],
-      version: 2,
-    )
-    document.push_edits(
-      [{ range: { start: { line: 1, character: 0 }, end: { line: 1, character: 0 } }, text: "p" }],
-      version: 3,
-    )
-    document.push_edits(
-      [{ range: { start: { line: 1, character: 1 }, end: { line: 1, character: 1 } }, text: "u" }],
-      version: 4,
-    )
-    document.push_edits(
-      [{ range: { start: { line: 1, character: 2 }, end: { line: 1, character: 2 } }, text: "t" }],
-      version: 5,
-    )
-    document.push_edits(
-      [{ range: { start: { line: 1, character: 3 }, end: { line: 1, character: 3 } }, text: "s" }],
-      version: 6,
-    )
-    document.push_edits(
-      [{ range: { start: { line: 1, character: 4 }, end: { line: 1, character: 4 } }, text: " " }],
-      version: 7,
-    )
-    document.push_edits(
-      [{ range: { start: { line: 1, character: 5 }, end: { line: 1, character: 5 } }, text: "'" }],
-      version: 8,
-    )
-    document.push_edits(
-      [{ range: { start: { line: 1, character: 6 }, end: { line: 1, character: 6 } }, text: "a" }],
-      version: 9,
-    )
-    document.push_edits(
-      [{ range: { start: { line: 1, character: 7 }, end: { line: 1, character: 7 } }, text: "'" }],
-      version: 10,
-    )
-
-    assert_equal(<<~RUBY, document.source)
-      chars = ["億"]
-      puts 'a'
-    RUBY
-  end
-
   def test_document_handle_4_byte_unicode_characters
     source = +<<~RUBY
       class Foo
@@ -760,29 +753,6 @@ class Foo
     assert_nil(document.cache_get("textDocument/codeLens"))
   end
 
-  def test_locating_a_non_existing_location_raises
-    document = RubyLsp::RubyDocument.new(source: <<~RUBY.chomp, version: 1, uri: @uri, global_state: @global_state)
-      class Foo
-      end
-    RUBY
-
-    # Exactly at the last character doesn't raise
-    document.locate_node({ line: 1, character: 2 })
-
-    # Anything beyond does
-    error = assert_raises(RubyLsp::Document::LocationNotFoundError) do
-      document.locate_node({ line: 3, character: 2 })
-    end
-
-    assert_match(/Requested position: {(:)?line[\s:=>]+3, (:)?character[\s:=>]+2}/, error.message)
-    assert_match(<<~MESSAGE.chomp, error.message)
-      Source:
-
-      class Foo
-      end
-    MESSAGE
-  end
-
   def test_document_tracks_latest_edit_context
     document = RubyLsp::RubyDocument.new(source: +<<~RUBY, version: 1, uri: @uri, global_state: @global_state)
       class Foo

test/server_test.rb

@@ -893,36 +893,6 @@ def handle_window_show_message_response(title)
     end
   end
 
-  def test_requests_to_a_non_existing_position_return_error
-    uri = URI("file:///foo.rb")
-
-    @server.process_message({
-      method: "textDocument/didOpen",
-      params: {
-        textDocument: {
-          uri: uri,
-          text: "class Foo\nend",
-          version: 1,
-          languageId: "ruby",
-        },
-      },
-    })
-
-    @server.process_message({
-      id: 1,
-      method: "textDocument/completion",
-      params: {
-        textDocument: {
-          uri: uri,
-        },
-        position: { line: 10, character: 0 },
-      },
-    })
-
-    error = find_message(RubyLsp::Error)
-    assert_match("Request textDocument/completion failed to find the target position.", error.message)
-  end
-
   def test_cancelling_requests_returns_nil
     uri = URI("file:///foo.rb")