Improve completion (#747)

* Add keywords to completion

* Fix signature request caching for call without parentheses

* Don't insert parentheses for def* macros

* Don't trigger missing runtime message when auto completion is enabled

* Don't insert parentheses for keyword macros

* Improve completion of env macros

* Apply review comments

* Update locals without parentheses

* Apply suggestions from code review

Co-authored-by: José Valim <jose.valim@dashbit.co>

* Format

Co-authored-by: José Valim <jose.valim@dashbit.co>

assets/js/cell/live_editor.js

@@ -250,6 +250,7 @@ class LiveEditor {
 
       return this.__asyncIntellisenseRequest("completion", {
         hint: lineUntilCursor,
+        editor_auto_completion: settings.editor_auto_completion,
       })
         .then((response) => {
           const suggestions = completionItemsToSuggestions(
@@ -311,8 +312,11 @@ class LiveEditor {
       const lineUntilCursor = lines[lineIdx].slice(0, position.column - 1);
       const codeUntilCursor = [...prevLines, lineUntilCursor].join("\n");
 
+      const codeUntilLastStop = codeUntilCursor
         // Remove trailing characters that don't affect the signature
-      const codeUntilLastStop = codeUntilCursor.replace(/[^(),]*?$/, "");
+        .replace(/[^(),\s]*?$/, "")
+        // Remove whitespace before delimiter
+        .replace(/([(),])\s*$/, "$1");
 
       // Cache subsequent requests for the same prefix, so that we don't
       // make unnecessary requests
@@ -464,6 +468,8 @@ function parseItemKind(kind) {
       return monaco.languages.CompletionItemKind.Variable;
     case "field":
       return monaco.languages.CompletionItemKind.Field;
+    case "keyword":
+      return monaco.languages.CompletionItemKind.Keyword;
     default:
       return null;
   }

lib/livebook/intellisense.ex

@@ -114,13 +114,14 @@ defmodule Livebook.Intellisense do
     IdentifierMatcher.completion_identifiers(hint, binding, env)
     |> Enum.filter(&include_in_completion?/1)
     |> Enum.map(&format_completion_item/1)
+    |> Enum.concat(extra_completion_items(hint))
     |> Enum.sort_by(&completion_item_priority/1)
   end
 
   defp include_in_completion?({:module, _module, _display_name, :hidden}), do: false
 
   defp include_in_completion?(
-         {:function, _module, _name, _arity, _display_name, :hidden, _signatures, _specs}
+         {:function, _module, _name, _arity, _type, _display_name, :hidden, _signatures, _specs}
        ),
        do: false
 
@@ -168,7 +169,7 @@ defmodule Livebook.Intellisense do
   end
 
   defp format_completion_item(
-         {:function, module, name, arity, display_name, documentation, signatures, specs}
+         {:function, module, name, arity, type, display_name, documentation, signatures, specs}
        ),
        do: %{
          label: "#{display_name}/#{arity}",
@@ -181,11 +182,24 @@ defmodule Livebook.Intellisense do
            ]),
          insert_text:
            cond do
-             String.starts_with?(display_name, "~") -> display_name
-             Macro.operator?(name, arity) -> display_name
+             type == :macro and keyword_macro?(name) ->
+               "#{display_name} "
+
+             type == :macro and env_macro?(name) ->
+               display_name
+
+             String.starts_with?(display_name, "~") ->
+               display_name
+
+             Macro.operator?(name, arity) ->
+               display_name
+
+             arity == 0 ->
+               "#{display_name}()"
+
+             true ->
                # A snippet with cursor in parentheses
-             arity == 0 -> "#{display_name}()"
-             true -> "#{display_name}($0)"
+               "#{display_name}($0)"
            end
        }
 
@@ -207,7 +221,87 @@ defmodule Livebook.Intellisense do
       insert_text: name
     }
 
-  @ordered_kinds [:field, :variable, :module, :struct, :interface, :function, :type]
+  defp keyword_macro?(name) do
+    def? = name |> Atom.to_string() |> String.starts_with?("def")
+
+    def? or
+      name in [
+        # Special forms
+        :alias,
+        :case,
+        :cond,
+        :for,
+        :fn,
+        :import,
+        :quote,
+        :receive,
+        :require,
+        :try,
+        :with,
+
+        # Kernel
+        :destructure,
+        :raise,
+        :reraise,
+        :if,
+        :unless,
+        :use
+      ]
+  end
+
+  defp env_macro?(name) do
+    name in [:__ENV__, :__MODULE__, :__DIR__, :__STACKTRACE__, :__CALLER__]
+  end
+
+  defp extra_completion_items(hint) do
+    items = [
+      %{
+        label: "do",
+        kind: :keyword,
+        detail: "do-end block",
+        documentation: nil,
+        insert_text: "do\n  $0\nend"
+      },
+      %{
+        label: "true",
+        kind: :keyword,
+        detail: "boolean",
+        documentation: nil,
+        insert_text: "true"
+      },
+      %{
+        label: "false",
+        kind: :keyword,
+        detail: "boolean",
+        documentation: nil,
+        insert_text: "false"
+      },
+      %{
+        label: "nil",
+        kind: :keyword,
+        detail: "special atom",
+        documentation: nil,
+        insert_text: "nil"
+      },
+      %{
+        label: "when",
+        kind: :keyword,
+        detail: "guard operator",
+        documentation: nil,
+        insert_text: "when"
+      }
+    ]
+
+    last_word = hint |> String.split(~r/\s/) |> List.last()
+
+    if last_word == "" do
+      []
+    else
+      Enum.filter(items, &String.starts_with?(&1.label, last_word))
+    end
+  end
+
+  @ordered_kinds [:keyword, :field, :variable, :module, :struct, :interface, :function, :type]
 
   defp completion_item_priority(%{kind: :struct, detail: "exception"} = completion_item) do
     {length(@ordered_kinds), completion_item.label}
@@ -263,7 +357,7 @@ defmodule Livebook.Intellisense do
   end
 
   defp format_details_item(
-         {:function, module, name, _arity, _display_name, documentation, signatures, specs}
+         {:function, module, name, _arity, _type, _display_name, documentation, signatures, specs}
        ) do
     join_with_divider([
       format_signatures(signatures, module) |> code(),

lib/livebook/intellisense/identifier_matcher.ex

@@ -22,14 +22,15 @@ defmodule Livebook.Intellisense.IdentifierMatcher do
           {:variable, name(), value()}
           | {:map_field, name(), value()}
           | {:module, module(), display_name(), Docs.documentation()}
-          | {:function, module(), name(), arity(), display_name(), Docs.documentation(),
-             list(Docs.signature()), list(Docs.spec())}
+          | {:function, module(), name(), arity(), function_type(), display_name(),
+             Docs.documentation(), list(Docs.signature()), list(Docs.spec())}
           | {:type, module(), name(), arity(), Docs.documentation()}
           | {:module_attribute, name(), Docs.documentation()}
 
   @type name :: atom()
   @type display_name :: String.t()
   @type value :: term()
+  @type function_type :: :function | :macro
 
   @exact_matcher &Kernel.==/2
   @prefix_matcher &String.starts_with?/2
@@ -241,10 +242,13 @@ defmodule Livebook.Intellisense.IdentifierMatcher do
   end
 
   defp match_sigil(hint, ctx) do
-    for {:function, module, name, arity, "sigil_" <> sigil_name, documentation, signatures, specs} <-
+    for {:function, module, name, arity, type, "sigil_" <> sigil_name, documentation, signatures,
+         specs} <-
           match_local("sigil_", %{ctx | matcher: @prefix_matcher}),
         ctx.matcher.(sigil_name, hint),
-        do: {:function, module, name, arity, "~" <> sigil_name, documentation, signatures, specs}
+        do:
+          {:function, module, name, arity, type, "~" <> sigil_name, documentation, signatures,
+           specs}
   end
 
   defp match_erlang_module(hint, ctx) do
@@ -376,24 +380,26 @@ defmodule Livebook.Intellisense.IdentifierMatcher do
       funs = funs || exports(mod)
 
       matching_funs =
-        Enum.filter(funs, fn {name, _arity} ->
+        Enum.filter(funs, fn {name, _arity, _type} ->
           name = Atom.to_string(name)
           ctx.matcher.(name, hint)
         end)
 
       doc_items =
-        Livebook.Intellisense.Docs.lookup_module_members(mod, matching_funs,
+        Livebook.Intellisense.Docs.lookup_module_members(
+          mod,
+          Enum.map(matching_funs, &Tuple.delete_at(&1, 2)),
           kinds: [:function, :macro]
         )
 
-      Enum.map(matching_funs, fn {name, arity} ->
+      Enum.map(matching_funs, fn {name, arity, type} ->
         doc_item =
           Enum.find(doc_items, %{documentation: nil, signatures: [], specs: []}, fn doc_item ->
             doc_item.name == name && doc_item.arity == arity
           end)
 
-        {:function, mod, name, arity, Atom.to_string(name), doc_item && doc_item.documentation,
-         doc_item.signatures, doc_item.specs}
+        {:function, mod, name, arity, type, Atom.to_string(name),
+         doc_item && doc_item.documentation, doc_item.signatures, doc_item.specs}
       end)
     else
       []
@@ -402,12 +408,19 @@ defmodule Livebook.Intellisense.IdentifierMatcher do
 
   defp exports(mod) do
     if Code.ensure_loaded?(mod) and function_exported?(mod, :__info__, 1) do
-      mod.__info__(:macros) ++ (mod.__info__(:functions) -- [__info__: 1])
+      macros = mod.__info__(:macros)
+      functions = mod.__info__(:functions) -- [__info__: 1]
+      append_funs_type(macros, :macro) ++ append_funs_type(functions, :function)
     else
-      mod.module_info(:exports) -- [module_info: 0, module_info: 1]
+      functions = mod.module_info(:exports) -- [module_info: 0, module_info: 1]
+      append_funs_type(functions, :function)
     end
   end
 
+  defp append_funs_type(funs, type) do
+    Enum.map(funs, &Tuple.append(&1, type))
+  end
+
   defp match_module_type(mod, hint, ctx) do
     types = get_module_types(mod)
 
@@ -444,7 +457,14 @@ defmodule Livebook.Intellisense.IdentifierMatcher do
   defp ensure_loaded?(Elixir), do: false
   defp ensure_loaded?(mod), do: Code.ensure_loaded?(mod)
 
-  defp imports_from_env(env), do: env.functions ++ env.macros
+  defp imports_from_env(env) do
+    Enum.map(env.functions, fn {mod, funs} ->
+      {mod, append_funs_type(funs, :function)}
+    end) ++
+      Enum.map(env.macros, fn {mod, funs} ->
+        {mod, append_funs_type(funs, :macro)}
+      end)
+  end
 
   defp split_at_last_occurrence(string, pattern) do
     case :binary.matches(string, pattern) do

lib/livebook/runtime.ex

@@ -72,7 +72,7 @@ defprotocol Livebook.Runtime do
         }
 
   @type completion_item_kind ::
-          :function | :module | :struct | :interface | :type | :variable | :field
+          :function | :module | :struct | :interface | :type | :variable | :field | :keyword
 
   @typedoc """
   Looks up more details about an identifier found in `column` in `line`.

lib/livebook_web/live/session_live.ex

@@ -784,14 +784,14 @@ defmodule LivebookWeb.SessionLive do
         {:reply, %{"ref" => inspect(ref)}, socket}
       else
         info =
-          case params["type"] do
-            "completion" ->
+          cond do
+            params["type"] == "completion" and not params["editor_auto_completion"] ->
               "You need to start a runtime (or evaluate a cell) for code completion"
 
-            "format" ->
+            params["type"] == "format" ->
               "You need to start a runtime (or evaluate a cell) to enable code formatting"
 
-            _ ->
+            true ->
               nil
           end
 

test/livebook/intellisense_test.exs

@@ -738,7 +738,7 @@ defmodule Livebook.IntellisenseTest do
                  kind: :function,
                  detail: "Kernel.SpecialForms.quote(opts, block)",
                  documentation: "Gets the representation of any expression.",
-                 insert_text: "quote($0)"
+                 insert_text: "quote "
                }
              ] = Intellisense.get_completion_items("quot", binding, env)
     end
@@ -1110,6 +1110,54 @@ defmodule Livebook.IntellisenseTest do
       {binding, env} = eval(do: nil)
       assert [] = Intellisense.get_completion_items("@attr.value", binding, env)
     end
+
+    test "includes language keywords" do
+      {binding, env} = eval(do: nil)
+
+      assert [
+               %{
+                 label: "do",
+                 kind: :keyword,
+                 detail: "do-end block",
+                 documentation: nil,
+                 insert_text: "do\n  $0\nend"
+               }
+               | _
+             ] = Intellisense.get_completion_items("do", binding, env)
+    end
+
+    test "includes space instead of parentheses for def* macros" do
+      {binding, env} = eval(do: nil)
+
+      assert [
+               %{
+                 label: "defmodule/2",
+                 insert_text: "defmodule "
+               }
+             ] = Intellisense.get_completion_items("defmodu", binding, env)
+    end
+
+    test "includes space instead of parentheses for keyword macros" do
+      {binding, env} = eval(do: nil)
+
+      assert [
+               %{
+                 label: "import/2",
+                 insert_text: "import "
+               }
+             ] = Intellisense.get_completion_items("impor", binding, env)
+    end
+
+    test "includes doesn't include space nor parentheses for macros like __ENV__" do
+      {binding, env} = eval(do: nil)
+
+      assert [
+               %{
+                 label: "__ENV__/0",
+                 insert_text: "__ENV__"
+               }
+             ] = Intellisense.get_completion_items("__EN", binding, env)
+    end
   end
 
   describe "get_details/3" do

test/livebook_web/live/session_live_test.exs

@@ -348,7 +348,8 @@ defmodule LivebookWeb.SessionLiveTest do
       |> render_hook("intellisense_request", %{
         "cell_id" => cell_id,
         "type" => "completion",
-        "hint" => "System.ver"
+        "hint" => "System.ver",
+        "editor_auto_completion" => false
       })
 
       assert_reply view, %{"ref" => nil}
@@ -369,7 +370,8 @@ defmodule LivebookWeb.SessionLiveTest do
       |> render_hook("intellisense_request", %{
         "cell_id" => cell_id,
         "type" => "completion",
-        "hint" => "System.ver"
+        "hint" => "System.ver",
+        "editor_auto_completion" => false
       })
 
       assert_reply view, %{"ref" => ref}