make completions work for quoted remote calls

`Mymod."4quo\"ted"()` is valid elixir

CHANGELOG.md

@@ -1,12 +1,13 @@
 ### Unreleased
 
-### v0.18.1: 27 December 2023
+### v0.18.1: 28 December 2023
 
 #### Improvements
 
 - Variables defined in `ex_unit` `test`, `setup` and `setup_all` context are now returned by completions provider. Navigation to variable definition and references now also works correctly
 - Suggest spec code lens now emits specs for all arity variants when function has default arguments. Previously only the one with all parameters was emitted
 - Missing required OTP `:crypto` module is now detected on startup
+- Completions provider is now properly returning quoted remote calls. Previously accepting a suggestion would insert invalid code
 
 #### Fixes
 

apps/language_server/lib/language_server/providers/completion.ex

@@ -695,7 +695,13 @@ defmodule ElixirLS.LanguageServer.Providers.Completion do
     insert_text =
       case name do
         name when name in ["size", "unit"] ->
-          function_snippet(name, ["integer"], 1, options |> Keyword.merge(with_parens?: true))
+          function_snippet(
+            name,
+            ["integer"],
+            1,
+            "Kernel",
+            options |> Keyword.merge(with_parens?: true)
+          )
 
         other ->
           other
@@ -895,7 +901,7 @@ defmodule ElixirLS.LanguageServer.Providers.Completion do
 
   defp def_snippet(def_str, name, args, arity, opts) do
     if Keyword.get(opts, :snippets_supported, false) do
-      "#{def_str}#{function_snippet(name, args, arity, opts)} do\n\t$0\nend"
+      "#{def_str}#{function_snippet(name, args, arity, "Kernel", opts)} do\n\t$0\nend"
     else
       "#{def_str}#{name}"
     end
@@ -972,7 +978,8 @@ defmodule ElixirLS.LanguageServer.Providers.Completion do
     nil
   end
 
-  def function_snippet(name, args, arity, opts) do
+  def function_snippet(name, args, arity, origin, opts) do
+    name = sanitize_function_name(name, origin)
     snippets_supported? = Keyword.get(opts, :snippets_supported, false)
     trigger_signature? = Keyword.get(opts, :trigger_signature?, false)
     capture_before? = Keyword.get(opts, :capture_before?, false)
@@ -1173,36 +1180,32 @@ defmodule ElixirLS.LanguageServer.Providers.Completion do
 
     trigger_signature? = signature_help_supported? && ((arity == 1 && !pipe_before?) || arity > 1)
 
-    {label, insert_text} =
+    insert_text =
       cond do
         match?("~" <> _, name) ->
           "~" <> sigil_name = name
-          {name, sigil_name}
+          sigil_name
 
         use_name_only?(origin, name) or String.starts_with?(text_after_cursor, "(") ->
-          {name, name}
+          sanitize_function_name(name, origin)
 
         true ->
-          label = name
-
-          insert_text =
-            function_snippet(
-              name,
-              args_list,
-              arity,
-              Keyword.merge(
-                options,
-                pipe_before?: pipe_before?,
-                capture_before?: capture_before?,
-                trigger_signature?: trigger_signature?,
-                locals_without_parens: locals_without_parens,
-                text_after_cursor: text_after_cursor,
-                with_parens?: with_parens?,
-                snippet: info[:snippet]
-              )
+          function_snippet(
+            name,
+            args_list,
+            arity,
+            origin,
+            Keyword.merge(
+              options,
+              pipe_before?: pipe_before?,
+              capture_before?: capture_before?,
+              trigger_signature?: trigger_signature?,
+              locals_without_parens: locals_without_parens,
+              text_after_cursor: text_after_cursor,
+              with_parens?: with_parens?,
+              snippet: info[:snippet]
             )
-
-          {label, insert_text}
+          )
       end
 
     footer = SourceFile.format_spec(spec, line_length: 30)
@@ -1215,20 +1218,24 @@ defmodule ElixirLS.LanguageServer.Providers.Completion do
         }
       end
 
-    %__MODULE__{
-      label: label,
-      kind: :function,
-      detail: to_string(type),
-      label_details: %{
-        "detail" => "(#{Enum.join(args_list, ", ")})",
-        "description" => "#{origin}.#{name}/#{arity}"
-      },
-      documentation: summary <> footer,
-      insert_text: insert_text,
-      priority: 17,
-      tags: metadata_to_tags(metadata),
-      command: command
-    }
+    label = sanitize_function_name(name, origin)
+
+    if label == name or remote_calls? do
+      %__MODULE__{
+        label: label,
+        kind: :function,
+        detail: to_string(type),
+        label_details: %{
+          "detail" => "(#{Enum.join(args_list, ", ")})",
+          "description" => "#{origin}.#{label}/#{arity}"
+        },
+        documentation: summary <> footer,
+        insert_text: insert_text,
+        priority: 17,
+        tags: metadata_to_tags(metadata),
+        command: command
+      }
+    end
   end
 
   defp use_name_only?(module_name, function_name) do
@@ -1369,4 +1376,21 @@ defmodule ElixirLS.LanguageServer.Providers.Completion do
     |> MapSet.member?({String.to_atom(name), arity})
     |> Kernel.not()
   end
+
+  defp sanitize_function_name(name, origin) when origin in ["Kernel", "Kernel.SpecialForms"],
+    do: name
+
+  defp sanitize_function_name(name, origin) do
+    if not Regex.match?(~r/^([_\p{Ll}\p{Lo}][\p{L}\p{N}_]*[?!]?)$/u, name) do
+      # not an allowed identifier - quote
+      escaped =
+        name
+        |> String.replace("\\", "\\\\")
+        |> String.replace("\"", "\\\"")
+
+      "\"" <> escaped <> "\""
+    else
+      name
+    end
+  end
 end

apps/language_server/test/providers/completion_test.exs

@@ -1442,6 +1442,34 @@ defmodule ElixirLS.LanguageServer.Providers.CompletionTest do
                "insertText" => "defprotocol $1 do\n\t$0\nend"
              } = first
     end
+
+    test "will suggest remote quoted calls" do
+      text = """
+      alias ElixirLS.LanguageServer.Fixtures.ExampleQuotedDefs, as: Quoted
+      Quoted.
+      #      ^
+      """
+
+      {line, char} = {1, 7}
+
+      TestUtils.assert_has_cursor_char(text, line, char)
+      {line, char} = SourceFile.lsp_position_to_elixir(text, {line, char})
+      parser_context = ParserContextBuilder.from_string(text, {line, char})
+
+      assert {:ok, %{"items" => items}} =
+               Completion.completion(
+                 parser_context,
+                 line,
+                 char,
+                 @supports
+               )
+
+      assert item = Enum.find(items, fn item -> item["label"] == "\"0abc\\\"asd\"" end)
+      assert item["insertText"] == "\"0abc\\\"asd\"($1)$0"
+
+      assert item["labelDetails"]["description"] ==
+               "ElixirLS.LanguageServer.Fixtures.ExampleQuotedDefs.\"0abc\\\"asd\"/2"
+    end
   end
 
   describe "generic suggestions" do
@@ -1501,7 +1529,13 @@ defmodule ElixirLS.LanguageServer.Providers.CompletionTest do
       ]
 
       assert "do_sth()" ==
-               Completion.function_snippet("do_sth", ["My.record(x: x0, y: y0)"], 1, opts)
+               Completion.function_snippet(
+                 "do_sth",
+                 ["My.record(x: x0, y: y0)"],
+                 1,
+                 "Kernel",
+                 opts
+               )
     end
   end
 

apps/language_server/test/support/fixtures/example_quoted_defs.ex

@@ -0,0 +1,9 @@
+defmodule ElixirLS.LanguageServer.Fixtures.ExampleQuotedDefs do
+  @doc """
+  quoted def
+  """
+  @spec unquote(:"0abc\"asd")(any, integer) :: :ok
+  def unquote(:"0abc\"asd")(example, arg) do
+    :ok
+  end
+end