Improve LSP consistency

position should trim to the end of the line

Author: lukaszsamson

apps/elixir_ls_debugger/lib/debugger/utils.ex

@@ -34,9 +34,6 @@ defmodule ElixirLS.Debugger.Utils do
 
     byte_size = byte_size(utf16_line)
 
-    # if character index is over the length of the string assume we pad it with spaces (1 byte in utf8)
-    diff = div(max(dap_character * 2 - byte_size, 0), 2)
-
     utf8_character =
       utf16_line
       |> (&binary_part(
@@ -47,6 +44,6 @@ defmodule ElixirLS.Debugger.Utils do
       |> characters_to_binary!(:utf16, :utf8)
       |> String.length()
 
-    utf8_character + diff
+    utf8_character
   end
 end

apps/elixir_ls_debugger/test/utils_test.exs

@@ -39,6 +39,10 @@ defmodule ElixirLS.Debugger.UtilsTest do
       assert 0 == Utils.dap_character_to_elixir("", 0)
     end
 
+    test "dap_character_to_elixir empty after end" do
+      assert 0 == Utils.dap_character_to_elixir("", 1)
+    end
+
     test "dap_character_to_elixir first char" do
       assert 0 == Utils.dap_character_to_elixir("abcde", 0)
     end
@@ -47,6 +51,14 @@ defmodule ElixirLS.Debugger.UtilsTest do
       assert 1 == Utils.dap_character_to_elixir("abcde", 1)
     end
 
+    test "dap_character_to_elixir before line start" do
+      assert 0 == Utils.dap_character_to_elixir("abcde", -1)
+    end
+
+    test "dap_character_to_elixir after line end" do
+      assert 5 == Utils.dap_character_to_elixir("abcde", 15)
+    end
+
     test "dap_character_to_elixir utf8" do
       assert 1 == Utils.dap_character_to_elixir("🏳️‍🌈abcde", 6)
     end

apps/language_server/lib/language_server/source_file.ex

@@ -283,9 +283,6 @@ defmodule ElixirLS.LanguageServer.SourceFile do
 
     byte_size = byte_size(utf16_line)
 
-    # if character index is over the length of the string assume we pad it with spaces (1 byte in utf8)
-    diff = div(max(lsp_character * 2 - byte_size, 0), 2)
-
     utf8_character =
       utf16_line
       |> (&binary_part(
@@ -296,19 +293,30 @@ defmodule ElixirLS.LanguageServer.SourceFile do
       |> characters_to_binary!(:utf16, :utf8)
       |> String.length()
 
-    utf8_character + 1 + diff
+    utf8_character + 1
   end
 
+  def lsp_position_to_elixir(_urf8_text, {lsp_line, _lsp_character}) when lsp_line < 0,
+    do: {1, 1}
+
   def lsp_position_to_elixir(_urf8_text, {lsp_line, lsp_character}) when lsp_character <= 0,
     do: {max(lsp_line + 1, 1), 1}
 
   def lsp_position_to_elixir(urf8_text, {lsp_line, lsp_character}) do
-    utf8_character =
-      lines(urf8_text)
-      |> Enum.at(max(lsp_line, 0))
-      |> lsp_character_to_elixir(lsp_character)
-
-    {lsp_line + 1, utf8_character}
+    source_file_lines = lines(urf8_text)
+    total_lines = length(source_file_lines)
+
+    if lsp_line > total_lines - 1 do
+      # sanitize to position after last char in last line
+      {total_lines, String.length(source_file_lines |> Enum.at(total_lines - 1)) + 1}
+    else
+      utf8_character =
+        source_file_lines
+        |> Enum.at(lsp_line)
+        |> lsp_character_to_elixir(lsp_character)
+
+      {lsp_line + 1, utf8_character}
+    end
   end
 
   def elixir_character_to_lsp(_utf8_line, elixir_character) when elixir_character <= 1, do: 0

apps/language_server/test/providers/completion_test.exs

@@ -476,7 +476,7 @@ defmodule ElixirLS.LanguageServer.Providers.CompletionTest do
         defstruct [some: nil, other: 1]
 
         def dummy_function(var = %MyModule{}) do
-          %{var |
+          %{var | 
           #       ^
         end
       end
@@ -496,7 +496,7 @@ defmodule ElixirLS.LanguageServer.Providers.CompletionTest do
       text = """
       defmodule MyModule do
         def dummy_function(var = %{some: nil, other: 1}) do
-          %{var |
+          %{var | 
           #       ^
         end
       end

apps/language_server/test/source_file_test.exs

@@ -655,6 +655,18 @@ defmodule ElixirLS.LanguageServer.SourceFileTest do
       assert {1, 2} == SourceFile.lsp_position_to_elixir("abcde", {0, 1})
     end
 
+    # This is not specified in LSP but some clients fail to synchronize text properly
+    test "lsp_position_to_elixir single line before line start" do
+      assert {1, 1} == SourceFile.lsp_position_to_elixir("abcde", {0, -1})
+    end
+
+    # LSP spec 3.17 https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#position
+    # position character If the character value is greater than the line length it defaults back to the line length
+    test "lsp_position_to_elixir single line after line end" do
+      assert {1, 6} == SourceFile.lsp_position_to_elixir("abcde", {0, 15})
+      assert {1, 1} == SourceFile.lsp_position_to_elixir("", {0, 15})
+    end
+
     test "lsp_position_to_elixir single line utf8" do
       assert {1, 2} == SourceFile.lsp_position_to_elixir("🏳️‍🌈abcde", {0, 6})
     end
@@ -663,6 +675,16 @@ defmodule ElixirLS.LanguageServer.SourceFileTest do
       assert {2, 2} == SourceFile.lsp_position_to_elixir("abcde\n1234", {1, 1})
     end
 
+    # This is not specified in LSP but some clients fail to synchronize text properly
+    test "lsp_position_to_elixir multi line before first line" do
+      assert {1, 1} == SourceFile.lsp_position_to_elixir("abcde\n1234", {-1, 2})
+    end
+
+    # This is not specified in LSP but some clients fail to synchronize text properly
+    test "lsp_position_to_elixir multi line after last line" do
+      assert {2, 5} == SourceFile.lsp_position_to_elixir("abcde\n1234", {8, 2})
+    end
+
     test "elixir_position_to_lsp empty" do
       assert {0, 0} == SourceFile.elixir_position_to_lsp("", {1, 1})
     end