Improve completions with default arguments

A couple of updates ago, elixir sense started returning the defaults
of arguments along with the names of the arguments. This make our
completions much worse, as the arity of the completion didn't match
the arity of the function selected *and* the defaults would get pasted
into the completion. This meant you had to remove the default from the
completion, which was beyond annoying.

These changes strip out defaults and ensure that the arity and
arguments match the completion result generated by elixir sense.

apps/remote_control/lib/lexical/remote_control/completion/result.ex

@@ -1,12 +1,20 @@
 defmodule Lexical.RemoteControl.Completion.Result do
+  alias Lexical.RemoteControl.Completion.Result.ArgumentNames
+
   defmodule Function do
     @moduledoc false
     defstruct [:argument_names, :arity, :name, :origin, :type, :visibility, :spec, :metadata]
 
     def new(%{} = elixir_sense_map) do
+      arg_names =
+        case ArgumentNames.from_elixir_sense_map(elixir_sense_map) do
+          :error -> []
+          names -> names
+        end
+
       __MODULE__
       |> struct(elixir_sense_map)
-      |> Map.put(:argument_names, Map.get(elixir_sense_map, :args_list))
+      |> Map.put(:argument_names, arg_names)
     end
   end
 
@@ -15,9 +23,15 @@ defmodule Lexical.RemoteControl.Completion.Result do
     defstruct [:argument_names, :arity, :metadata, :name, :origin, :spec, :summary, :type]
 
     def new(%{} = elixir_sense_map) do
+      arg_names =
+        case ArgumentNames.from_elixir_sense_map(elixir_sense_map) do
+          :error -> []
+          names -> names
+        end
+
       __MODULE__
       |> struct(elixir_sense_map)
-      |> Map.put(:argument_names, Map.get(elixir_sense_map, :args_list))
+      |> Map.put(:argument_names, arg_names)
     end
   end
 
@@ -26,9 +40,15 @@ defmodule Lexical.RemoteControl.Completion.Result do
     defstruct [:argument_names, :arity, :name, :origin, :type, :visibility, :spec, :metadata]
 
     def new(%{} = elixir_sense_map) do
+      arg_names =
+        case ArgumentNames.from_elixir_sense_map(elixir_sense_map) do
+          :error -> []
+          names -> names
+        end
+
       __MODULE__
       |> struct(elixir_sense_map)
-      |> Map.put(:argument_names, Map.get(elixir_sense_map, :args_list))
+      |> Map.put(:argument_names, arg_names)
     end
   end
 

apps/remote_control/lib/lexical/remote_control/completion/result/argument_names.ex

@@ -0,0 +1,80 @@
+defmodule Lexical.RemoteControl.Completion.Result.ArgumentNames do
+  @moduledoc """
+  Elixir sense, for whatever reason returns all the argument names when asked to do a completion on a function.
+  This means that the arity of the function might differ from the argument names returned. Furthermore, the
+  argument names have the defaults in them for some reason, which is completely unhelpful for completions.
+
+  The functions below match the arity of the argument names to the given arity, and strip out the defaults,
+  rendering the results useful for completion.
+  """
+  @type argument_names :: [String.t()]
+  @default_specifier "\\\\"
+
+  @spec from_elixir_sense_map(map()) :: :error | argument_names()
+  def from_elixir_sense_map(%{args_list: argument_names, arity: arity}) do
+    from_elixir_sense(argument_names, arity)
+  end
+
+  def from_elixir_sense_map(%{}) do
+    :error
+  end
+
+  @spec from_elixir_sense([String.t()], non_neg_integer) :: :error | argument_names()
+  def from_elixir_sense(argument_names, arity) when is_list(argument_names) do
+    {names, required_count} = preprocess(argument_names)
+
+    cond do
+      length(argument_names) < arity ->
+        :error
+
+      arity < required_count ->
+        :error
+
+      true ->
+        generate_argument_list(names, required_count, arity)
+    end
+  end
+
+  defp generate_argument_list(names, required_count, arity) do
+    optional_count = arity - required_count
+
+    {arg_list, _} =
+      Enum.reduce(names, {[], optional_count}, fn
+        {:required, arg_name}, {arg_list, optional_count} ->
+          {[arg_name | arg_list], optional_count}
+
+        {:optional, _}, {_arg_list, 0} = acc ->
+          acc
+
+        {:optional, arg_name}, {arg_list, optional_count} ->
+          {[arg_name | arg_list], optional_count - 1}
+      end)
+
+    Enum.reverse(arg_list)
+  end
+
+  defp split_on_default(argument) do
+    argument
+    |> String.split(@default_specifier)
+    |> Enum.map(&String.trim/1)
+  end
+
+  @spec preprocess(argument_names()) :: {[String.t()], non_neg_integer()}
+  defp preprocess(argument_names) do
+    {names, required_count} =
+      Enum.reduce(argument_names, {[], 0}, fn argument, {names, required_count} ->
+        {name, increment} =
+          case split_on_default(argument) do
+            [argument_name] ->
+              {{:required, argument_name}, 1}
+
+            [argument_name, _default] ->
+              {{:optional, argument_name}, 0}
+          end
+
+        {[name | names], required_count + increment}
+      end)
+
+    {Enum.reverse(names), required_count}
+  end
+end

apps/remote_control/test/fixtures/project/lib/default_args.ex

@@ -0,0 +1,21 @@
+defmodule Project.DefaultArgs do
+  def first_arg(x \\ 3, y) do
+    x + y
+  end
+
+  def middle_arg(a, b \\ 3, c) do
+    a + b + c
+  end
+
+  def last_arg(x, y \\ 3) do
+    x + y
+  end
+
+  def options(a, opts \\ []) do
+    a + Keyword.get(opts, :b, 0)
+  end
+
+  def struct_arg(a, b \\ %Project.Structs.User{}) do
+    Map.put(b, :username, a)
+  end
+end

apps/remote_control/test/lexical/remote_control/completion/result/argument_names_test.exs

@@ -0,0 +1,56 @@
+defmodule Lexical.RemoteControl.Completion.Result.ArgumentNamesTest do
+  alias Lexical.RemoteControl.Completion.Result.ArgumentNames
+
+  use ExUnit.Case
+  import ArgumentNames
+
+  describe "parsing elixir sense argument names" do
+    test "handles normal arguments" do
+      assert ~w(first second third) == from_elixir_sense(~w(first second third), 3)
+    end
+
+    test "handles default arguments in the first position" do
+      args = ["first \\\\ 3", "second", "third"]
+
+      assert ~w(second third) == from_elixir_sense(args, 2)
+      assert ~w(first second third) == from_elixir_sense(args, 3)
+    end
+
+    test "handles default arguments in the middle" do
+      args = ["first", "second \\\\", "third"]
+
+      assert ~w(first third) = from_elixir_sense(args, 2)
+      assert ~w(first second third) = from_elixir_sense(args, 3)
+    end
+
+    test "handles default arguments at the last position" do
+      args = ["first", "second", "third \\\\ 3"]
+
+      assert ~w(first second) = from_elixir_sense(args, 2)
+      assert ~w(first second third) = from_elixir_sense(args, 3)
+    end
+
+    test "handles multiple default arguments" do
+      args = ["first", "second \\\\ 1", "third", "fourth \\\\ 2", "fifth \\\\ 3", "sixth"]
+
+      assert ~w(first third sixth) = from_elixir_sense(args, 3)
+      assert ~w(first second third sixth) = from_elixir_sense(args, 4)
+      assert ~w(first second third fourth sixth) = from_elixir_sense(args, 5)
+      assert ~w(first second third fourth fifth sixth) = from_elixir_sense(args, 6)
+    end
+
+    test "handles struct defaults" do
+      args = ["first", "second \\\\ %Struct{}", "third"]
+
+      assert ~w(first third) = from_elixir_sense(args, 2)
+      assert ~w(first second third) = from_elixir_sense(args, 3)
+    end
+
+    test "handles incorrect arity" do
+      args = ~w(first second third)
+      assert :error == from_elixir_sense(args, 1)
+      assert :error == from_elixir_sense(args, 2)
+      assert :error == from_elixir_sense(args, 4)
+    end
+  end
+end

apps/server/lib/lexical/server/code_intelligence/completion/translations/callable.ex

@@ -61,10 +61,9 @@ defmodule Lexical.Server.CodeIntelligence.Completion.Translations.Callable do
 
     argument_templates =
       argument_names
-      |> Enum.with_index()
+      |> Enum.with_index(1)
       |> Enum.map_join(", ", fn {name, index} ->
-        escaped_name = String.replace(name, "\\", "\\\\")
-        "${#{index + 1}:#{escaped_name}}"
+        "${#{index}:#{name}}"
       end)
 
     "#{callable.name}(#{argument_templates})"

apps/server/test/lexical/server/code_intelligence/completion/translations/function_test.exs

@@ -139,6 +139,58 @@ defmodule Lexical.Server.CodeIntelligence.Completion.Translations.FunctionTest d
     end
   end
 
+  describe "handling default arguments" do
+    test "works with a first arg", %{project: project} do
+      {:ok, [arity_1, arity_2]} =
+        project
+        |> complete("Project.DefaultArgs.first|")
+        |> fetch_completion(kind: :function)
+
+      assert arity_1.insert_text == "first_arg(${1:y})"
+      assert arity_2.insert_text == "first_arg(${1:x}, ${2:y})"
+    end
+
+    test "works with the middle arg", %{project: project} do
+      {:ok, [arity_1, arity_2]} =
+        project
+        |> complete("Project.DefaultArgs.middle|")
+        |> fetch_completion(kind: :function)
+
+      assert arity_1.insert_text == "middle_arg(${1:a}, ${2:c})"
+      assert arity_2.insert_text == "middle_arg(${1:a}, ${2:b}, ${3:c})"
+    end
+
+    test "works with the last arg", %{project: project} do
+      {:ok, [arity_1, arity_2]} =
+        project
+        |> complete("Project.DefaultArgs.last|")
+        |> fetch_completion(kind: :function)
+
+      assert arity_1.insert_text == "last_arg(${1:x})"
+      assert arity_2.insert_text == "last_arg(${1:x}, ${2:y})"
+    end
+
+    test "works with options", %{project: project} do
+      {:ok, [arity_1, arity_2]} =
+        project
+        |> complete("Project.DefaultArgs.opt|")
+        |> fetch_completion(kind: :function)
+
+      assert arity_1.insert_text == "options(${1:a})"
+      assert arity_2.insert_text == "options(${1:a}, ${2:opts})"
+    end
+
+    test "works with struct defaults", %{project: project} do
+      {:ok, [arity_1, arity_2]} =
+        project
+        |> complete("Project.DefaultArgs.struct|")
+        |> fetch_completion(kind: :function)
+
+      assert arity_1.insert_text == "struct_arg(${1:a})"
+      assert arity_2.insert_text == "struct_arg(${1:a}, ${2:b})"
+    end
+  end
+
   describe "sort_text" do
     test "dunder functions have the dunder removed in their sort_text", %{project: project} do
       assert {:ok, completion} =