Improve autocomplete and signature help (elixir-editors#273)

* Improve autocomple and signagture help

* Addressing feedback

* Fix callback completion test

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

@@ -10,7 +10,17 @@ defmodule ElixirLS.LanguageServer.Providers.Completion do
   alias ElixirLS.LanguageServer.SourceFile
 
   @enforce_keys [:label, :kind, :insert_text, :priority, :tags]
-  defstruct [:label, :kind, :detail, :documentation, :insert_text, :filter_text, :priority, :tags]
+  defstruct [
+    :label,
+    :kind,
+    :detail,
+    :documentation,
+    :insert_text,
+    :filter_text,
+    :priority,
+    :tags,
+    :command
+  ]
 
   @module_attr_snippets [
     {"doc", "doc \"\"\"\n$0\n\"\"\"", "Documents a function"},
@@ -130,7 +140,8 @@ defmodule ElixirLS.LanguageServer.Providers.Completion do
       def_before: def_before,
       pipe_before?: Regex.match?(Regex.recompile!(~r/\|>\s*#{prefix}$/), text_before_cursor),
       capture_before?: Regex.match?(Regex.recompile!(~r/&#{prefix}$/), text_before_cursor),
-      scope: scope
+      scope: scope,
+      module: env.module
     }
 
     items =
@@ -278,7 +289,8 @@ defmodule ElixirLS.LanguageServer.Providers.Completion do
           end
         end
 
-      insert_text = def_snippet(def_str, name, args, arity, options)
+      opts = Keyword.put(options, :with_parens?, true)
+      insert_text = def_snippet(def_str, name, args, arity, opts)
       label = "#{def_str}#{function_label(name, args, arity)}"
 
       filter_text =
@@ -434,12 +446,8 @@ defmodule ElixirLS.LanguageServer.Providers.Completion do
     nil
   end
 
-  defp function_label(name, args, arity) do
-    if args && args != "" do
-      Enum.join([to_string(name), "(", args, ")"])
-    else
-      Enum.join([to_string(name), "/", arity])
-    end
+  defp function_label(name, _args, arity) do
+    Enum.join([to_string(name), "/", arity])
   end
 
   defp def_snippet(def_str, name, args, arity, opts) do
@@ -458,6 +466,18 @@ defmodule ElixirLS.LanguageServer.Providers.Completion do
       not Keyword.get(opts, :snippets_supported, false) ->
         name
 
+      Keyword.get(opts, :trigger_signature?, false) ->
+        text_after_cursor = Keyword.get(opts, :text_after_cursor, "")
+
+        # Don't add the closing parenthesis to the snippet if the cursor is
+        # immediately before a valid argument (this usually happens when we
+        # want to wrap an existing variable or literal, e.g. using IO.inspect)
+        if Regex.match?(~r/^[a-zA-Z0-9_:"'%<\[\{]/, text_after_cursor) do
+          "#{name}("
+        else
+          "#{name}($1)$0"
+        end
+
       true ->
         args_list =
           if args && args != "" do
@@ -478,7 +498,14 @@ defmodule ElixirLS.LanguageServer.Providers.Completion do
           |> Enum.with_index()
           |> Enum.map(fn {arg, i} -> "${#{i + 1}:#{arg}}" end)
 
-        Enum.join([name, "(", Enum.join(tabstops, ", "), ")"])
+        {before_args, after_args} =
+          if Keyword.get(opts, :with_parens?, false) do
+            {"(", ")"}
+          else
+            {" ", ""}
+          end
+
+        Enum.join([name, before_args, Enum.join(tabstops, ", "), after_args])
     end
   end
 
@@ -527,9 +554,12 @@ defmodule ElixirLS.LanguageServer.Providers.Completion do
     |> String.replace("$", "\\$")
     |> String.replace("}", "\\}")
     |> String.split(",")
+    |> Enum.reject(&is_default_argument?/1)
     |> Enum.map(&String.trim/1)
   end
 
+  defp is_default_argument?(s), do: String.contains?(s, "\\\\")
+
   defp module_attr_snippets(%{prefix: prefix, scope: :module, def_before: nil}) do
     for {name, snippet, docs} <- @module_attr_snippets,
         label = "@" <> name,
@@ -575,6 +605,7 @@ defmodule ElixirLS.LanguageServer.Providers.Completion do
   defp function_completion(info, context, options) do
     %{
       type: type,
+      visibility: visibility,
       args: args,
       name: name,
       summary: summary,
@@ -590,9 +621,19 @@ defmodule ElixirLS.LanguageServer.Providers.Completion do
     %{
       pipe_before?: pipe_before?,
       capture_before?: capture_before?,
-      text_after_cursor: text_after_cursor
+      text_after_cursor: text_after_cursor,
+      module: module
     } = context
 
+    locals_without_parens = Keyword.get(options, :locals_without_parens)
+    with_parens? = function_name_with_parens?(name, arity, locals_without_parens)
+
+    trigger_signature? =
+      Keyword.get(options, :signature_help_supported, false) &&
+        Keyword.get(options, :snippets_supported, false) &&
+        arity > 0 &&
+        with_parens?
+
     {label, insert_text} =
       cond do
         match?("sigil_" <> _, name) ->
@@ -614,33 +655,48 @@ defmodule ElixirLS.LanguageServer.Providers.Completion do
               Keyword.merge(
                 options,
                 pipe_before?: pipe_before?,
-                capture_before?: capture_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?
               )
             )
 
           {label, insert_text}
       end
 
-    detail =
-      cond do
-        spec && spec != "" ->
-          spec
+    detail_header =
+      if inspect(module) == origin do
+        "#{visibility} #{type}"
+      else
+        "#{origin} #{type}"
+      end
 
-        String.starts_with?(type, ["private", "public"]) ->
-          String.replace(type, "_", " ")
+    footer =
+      if String.starts_with?(type, ["private", "public"]) do
+        String.replace(type, "_", " ")
+      else
+        SourceFile.format_spec(spec, line_length: 30)
+      end
 
-        true ->
-          "(#{origin}) #{type}"
+    command =
+      if trigger_signature? do
+        %{
+          "title" => "Trigger Parameter Hint",
+          "command" => "editor.action.triggerParameterHints"
+        }
       end
 
     %__MODULE__{
       label: label,
       kind: :function,
-      detail: detail,
-      documentation: summary,
+      detail: detail_header <> "\n\n" <> Enum.join([to_string(name), "(", args, ")"]),
+      documentation: summary <> footer,
       insert_text: insert_text,
       priority: 7,
-      tags: metadata_to_tags(metadata)
+      tags: metadata_to_tags(metadata),
+      command: command
     }
   end
 
@@ -678,6 +734,7 @@ defmodule ElixirLS.LanguageServer.Providers.Completion do
       "filterText" => item.filter_text,
       "sortText" => String.pad_leading(to_string(idx), 8, "0"),
       "insertText" => item.insert_text,
+      "command" => item.command,
       "insertTextFormat" =>
         if Keyword.get(options, :snippets_supported, false) do
           insert_text_format(:snippet)
@@ -724,4 +781,10 @@ defmodule ElixirLS.LanguageServer.Providers.Completion do
       _ -> [:deprecated]
     end
   end
+
+  defp function_name_with_parens?(name, arity, locals_without_parens) do
+    (locals_without_parens || MapSet.new())
+    |> MapSet.member?({String.to_atom(name), arity})
+    |> Kernel.not()
+  end
 end

apps/language_server/lib/language_server/providers/execute_command.ex

@@ -47,7 +47,8 @@ defmodule ElixirLS.LanguageServer.Providers.ExecuteCommand do
     formatted =
       try do
         target_line_length =
-          Mix.Tasks.Format.formatter_opts_for_file(SourceFile.path_from_uri(uri))
+          uri
+          |> SourceFile.formatter_opts()
           |> Keyword.get(:line_length, 98)
 
         target_line_length = target_line_length - String.length(indentation)

apps/language_server/lib/language_server/providers/formatting.ex

@@ -8,8 +8,7 @@ defmodule ElixirLS.LanguageServer.Providers.Formatting do
 
   def format(source_file, uri, project_dir) do
     if can_format?(uri, project_dir) do
-      file = SourceFile.path_from_uri(uri) |> Path.relative_to(project_dir)
-      opts = formatter_opts(file)
+      opts = SourceFile.formatter_opts(uri)
       formatted = IO.iodata_to_binary([Code.format_string!(source_file.text, opts), ?\n])
 
       response =
@@ -41,10 +40,6 @@ defmodule ElixirLS.LanguageServer.Providers.Formatting do
       String.starts_with?(Path.absname(file_path), cwd)
   end
 
-  defp formatter_opts(for_file) do
-    Mix.Tasks.Format.formatter_opts_for_file(for_file)
-  end
-
   defp myers_diff_to_text_edits(myers_diff, starting_pos \\ {0, 0}) do
     myers_diff_to_text_edits(myers_diff, starting_pos, [])
   end

apps/language_server/lib/language_server/providers/signature_help.ex

@@ -1,4 +1,6 @@
 defmodule ElixirLS.LanguageServer.Providers.SignatureHelp do
+  alias ElixirLS.LanguageServer.SourceFile
+
   def signature(source_file, line, character) do
     response =
       case ElixirSense.signature(source_file.text, line + 1, character + 1) do
@@ -23,9 +25,19 @@ defmodule ElixirLS.LanguageServer.Providers.SignatureHelp do
     response = %{"label" => label, "parameters" => params_info}
 
     case {spec, documentation} do
-      {"", ""} -> response
-      {"", _} -> Map.put(response, "documentation", documentation)
-      {_, _} -> Map.put(response, "documentation", "#{spec}\n#{documentation}")
+      {"", ""} ->
+        response
+
+      {"", _} ->
+        put_documentation(response, documentation)
+
+      {_, _} ->
+        spec_str = SourceFile.format_spec(spec, line_length: 42)
+        put_documentation(response, "#{documentation}\n#{spec_str}")
     end
   end
+
+  defp put_documentation(response, documentation) do
+    Map.put(response, "documentation", %{"kind" => "markdown", "value" => documentation})
+  end
 end

apps/language_server/lib/language_server/server.ex

@@ -471,11 +471,22 @@ defmodule ElixirLS.LanguageServer.Server do
         %{"valueSet" => value_set} -> value_set
       end
 
+    signature_help_supported =
+      !!get_in(state.client_capabilities, ["textDocument", "signatureHelp"])
+
+    locals_without_parens =
+      uri
+      |> SourceFile.formatter_opts()
+      |> Keyword.get(:locals_without_parens, [])
+      |> MapSet.new()
+
     fun = fn ->
       Completion.completion(state.source_files[uri].text, line, character,
         snippets_supported: snippets_supported,
         deprecated_supported: deprecated_supported,
-        tags_supported: tags_supported
+        tags_supported: tags_supported,
+        signature_help_supported: signature_help_supported,
+        locals_without_parens: locals_without_parens
       )
     end
 

apps/language_server/lib/language_server/source_file.ex

@@ -181,4 +181,55 @@ defmodule ElixirLS.LanguageServer.SourceFile do
       _other -> {function, arity}
     end
   end
+
+  @spec format_spec(String.t(), keyword()) :: String.t()
+  def format_spec(spec, _opts) when spec in [nil, ""] do
+    ""
+  end
+
+  def format_spec(spec, opts) do
+    line_length = Keyword.fetch!(opts, :line_length)
+
+    spec_str =
+      case format_code(spec, line_length: line_length) do
+        {:ok, code} ->
+          code
+          |> to_string()
+          |> lines()
+          |> remove_indentation(String.length("@spec "))
+          |> Enum.join("\n")
+
+        {:error, _} ->
+          spec
+      end
+
+    """
+
+    ```
+    #{spec_str}
+    ```
+    """
+  end
+
+  @spec formatter_opts(String.t()) :: keyword()
+  def formatter_opts(uri) do
+    uri
+    |> path_from_uri()
+    |> Mix.Tasks.Format.formatter_opts_for_file()
+  end
+
+  defp format_code(code, opts) do
+    try do
+      {:ok, Code.format_string!(code, opts)}
+    rescue
+      e ->
+        {:error, e}
+    end
+  end
+
+  defp remove_indentation([line | rest], length) do
+    [line | Enum.map(rest, &String.slice(&1, length..-1))]
+  end
+
+  defp remove_indentation(lines, _), do: lines
 end