Remove keywords from completions (#259)

* Remove keywords from completions They were being returned regardless of context which made them very noisy. Related: * elixir-lsp/elixir_sense#99 * #251 * Add changelog entry * Return isIncomplete = true for completions * Fix tests and add a couple.
elixir-lsp · May 31, 2020 · b726c7e · b726c7e
1 parent b79f80d
commit b726c7e
Show file tree

Hide file tree

Showing 4 changed files with 39 additions and 35 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -4,6 +4,9 @@ Improvements:
 - Vendor Jason library to prevent conflicts with user's code (thanks [Jason Axelson](https://github.com/axelson)) [#253](https://github.com/elixir-lsp/elixir-ls/pull/253)
 - Switch to new supervisor format (thanks [Jason Axelson](https://github.com/axelson)) [#260](https://github.com/elixir-lsp/elixir-ls/pull/260)
 
+Changes:
+- No longer always return a static list of keywords for completion (thanks [Jason Axelson](https://github.com/axelson)) [#259](https://github.com/elixir-lsp/elixir-ls/pull/259)
+
 Bug Fixes:
 - Formatting was returning invalid floating point number (thanks [Thanabodee Charoenpiriyakij](https://github.com/wingyplus)) [#250](https://github.com/elixir-lsp/elixir-ls/pull/250)
 

diff --git a/apps/language_server/lib/language_server/providers/completion.ex b/apps/language_server/lib/language_server/providers/completion.ex
@@ -75,19 +75,6 @@ defmodule ElixirLS.LanguageServer.Providers.Completion do
                    "ExUnit.Assertions"
                  ])
 
-  @keywords %{
-    "end" => "end",
-    "do" => "do\n\t$0\nend",
-    "true" => "true",
-    "false" => "false",
-    "nil" => "nil",
-    "when" => "when",
-    "else" => "else\n\t$0",
-    "rescue" => "rescue\n\t$0",
-    "catch" => "catch\n\t$0",
-    "after" => "after\n\t$0"
-  }
-
   def trigger_characters do
     # VS Code's 24x7 autocompletion triggers automatically on alphanumeric characters. We add these
     # for "SomeModule." calls and @module_attrs
@@ -148,7 +135,6 @@ defmodule ElixirLS.LanguageServer.Providers.Completion do
       ElixirSense.suggestions(text, line + 1, character + 1)
       |> Enum.map(&from_completion_item(&1, context, options))
       |> Enum.concat(module_attr_snippets(context))
-      |> Enum.concat(keyword_completions(context))
 
     items_json =
       items
@@ -157,11 +143,22 @@ defmodule ElixirLS.LanguageServer.Providers.Completion do
       |> sort_items()
       |> items_to_json(options)
 
-    {:ok, %{"isIncomplete" => false, "items" => items_json}}
+    {:ok, %{"isIncomplete" => is_incomplete(items_json), "items" => items_json}}
   end
 
   ## Helpers
 
+  defp is_incomplete(items) do
+    if Enum.empty?(items) do
+      false
+    else
+      # By returning isIncomplete = true we tell the client that it should
+      # always fetch more results, this lets us control the ordering of
+      # completions accurately
+      true
+    end
+  end
+
   defp from_completion_item(
          %{type: :attribute, name: name},
          %{
@@ -585,23 +582,6 @@ defmodule ElixirLS.LanguageServer.Providers.Completion do
 
   defp module_attr_snippets(_), do: []
 
-  # These aren't really useful, to be honest, and it interferes with the auto-indentation
-  # for "else", but better to show them even if there's no good reason to use them
-  defp keyword_completions(%{prefix: prefix}) do
-    @keywords
-    |> Enum.filter(fn {keyword, _} -> String.starts_with?(keyword, prefix) end)
-    |> Enum.map(fn {keyword, snippet} ->
-      %__MODULE__{
-        label: keyword,
-        kind: :keyword,
-        detail: "keyword",
-        insert_text: snippet,
-        tags: [],
-        priority: 1
-      }
-    end)
-  end
-
   defp function_completion(info, context, options) do
     %{
       type: type,

diff --git a/apps/language_server/test/providers/completion_test.exs b/apps/language_server/test/providers/completion_test.exs
@@ -339,13 +339,34 @@ defmodule ElixirLS.LanguageServer.Providers.CompletionTest do
 
       {line, char} = {4, 14}
       TestUtils.assert_has_cursor_char(text, line, char)
-      {:ok, %{"items" => items}} = Completion.completion(text, line, char, @supports)
+      {:ok, result} = Completion.completion(text, line, char, @supports)
+
+      assert result["isIncomplete"] == true
+      items = result["items"]
 
       assert ["__struct__", "other", "some"] ==
                items |> Enum.filter(&(&1["kind"] == 5)) |> Enum.map(& &1["label"]) |> Enum.sort()
 
       assert (items |> hd)["detail"] == "MyModule struct field"
     end
+
+    test "isIncomplete is false when there are no results" do
+      text = """
+      defmodule MyModule do
+        defstruct [some: nil, other: 1]
+
+        def dummy_function() do
+          #                    ^
+      end
+      """
+
+      {line, char} = {3, 25}
+      TestUtils.assert_has_cursor_char(text, line, char)
+
+      {:ok, result} = Completion.completion(text, line, char, @supports)
+      assert result["isIncomplete"] == false
+      assert result["items"] == []
+    end
   end
 
   describe "function completion" do

diff --git a/apps/language_server/test/server_test.exs b/apps/language_server/test/server_test.exs
@@ -98,7 +98,7 @@ defmodule ElixirLS.LanguageServer.ServerTest do
     resp = assert_receive(%{"id" => 1}, 1000)
 
     assert response(1, %{
-             "isIncomplete" => false,
+             "isIncomplete" => true,
              "items" => [
                %{
                  "detail" => "behaviour",
@@ -423,7 +423,7 @@ defmodule ElixirLS.LanguageServer.ServerTest do
         resp = assert_receive(%{"id" => 3}, 5000)
 
         assert response(3, %{
-                 "isIncomplete" => false,
+                 "isIncomplete" => true,
                  "items" => [
                    %{
                      "detail" => "module",