Improve completions

apps/common/lib/lexical/ast/env.ex

@@ -132,31 +132,26 @@ defmodule Lexical.Ast.Env do
     end
   end
 
-  defp do_in_context?(env, :struct_arguments) do
+  defp do_in_context?(env, :struct_fields) do
+    env.document
+    |> Ast.cursor_path(env.position)
+    |> Enum.any?(&match?({:%, _, _}, &1))
+  end
+
+  defp do_in_context?(env, :struct_field_key) do
     cursor_path = Ast.cursor_path(env.document, env.position)
 
-    Enum.any?(cursor_path, fn
-      # struct leading by current module: `%__MODULE__.Struct{|}`
-      # or leading by a module alias: `%Alias.Struct{|}`
-      # or just a struct: `%Struct{|}`
-      {:%, _, [{:__aliases__, _, _aliases} | _]} -> true
-      # current module struct: `%__MODULE__{|}`
-      {:%, _, [{:__MODULE__, _, _} | _]} -> true
-      _ -> false
-    end)
+    match?(
+      # in the key position, the cursor will always be followed by the
+      # map node because, in any other case, there will minimally be a
+      # 2-element key-value tuple containing the cursor
+      [{:__cursor__, _, _}, {:%{}, _, _}, {:%, _, _} | _],
+      cursor_path
+    )
   end
 
   defp do_in_context?(env, :struct_field_value) do
-    if do_in_context?(env, :struct_arguments) do
-      env
-      |> prefix_tokens(2)
-      |> Enum.any?(fn
-        {:kw_identifier, _, _} -> true
-        _ -> false
-      end)
-    else
-      false
-    end
+    do_in_context?(env, :struct_fields) and not do_in_context?(env, :struct_field_key)
   end
 
   defp do_in_context?(env, :pipe) do

apps/common/lib/lexical/ast/environment.ex

@@ -6,7 +6,15 @@ defmodule Lexical.Ast.Environment do
   @type lexer_token :: {atom, token_value}
   @type token_count :: pos_integer | :all
 
-  @type context_type :: :pipe | :alias | :struct_reference | :function_capture | :bitstring
+  @type context_type ::
+          :pipe
+          | :alias
+          | :struct_reference
+          | :struct_fields
+          | :struct_field_key
+          | :struct_field_value
+          | :function_capture
+          | :bitstring
 
   @callback in_context?(t, context_type) :: boolean
 

apps/common/test/lexical/ast/env_test.exs

@@ -221,7 +221,7 @@ defmodule Lexical.Ast.EnvTest do
     end
   end
 
-  describe "in_context?(env, :struct_arguments)" do
+  describe "in_context?(env, :struct_fields)" do
     def wrap_with_module(text) do
       """
       defmodule MyModule do
@@ -247,42 +247,42 @@ defmodule Lexical.Ast.EnvTest do
 
     test "is true if the cursor is directly after the opening curly" do
       env = "%User{|}" |> wrap_with_module() |> new_env()
-      assert in_context?(env, :struct_arguments)
+      assert in_context?(env, :struct_fields)
     end
 
     test "is true when the struct is in the function variable" do
       env = "%User{|}" |> wrap_with_function() |> wrap_with_module() |> new_env()
-      assert in_context?(env, :struct_arguments)
+      assert in_context?(env, :struct_fields)
     end
 
     test "is true when the struct is in the function arguments" do
       env = "%User{|}" |> wrap_with_function_arguments() |> wrap_with_module() |> new_env()
-      assert in_context?(env, :struct_arguments)
+      assert in_context?(env, :struct_fields)
     end
 
     test "is true if the cursor is after the field name" do
       env = "%User{name: |}" |> wrap_with_module() |> new_env()
-      assert in_context?(env, :struct_arguments)
+      assert in_context?(env, :struct_fields)
     end
 
     test "is true if the cursor is after the field value" do
       env = "%User{name: \"John\"|}" |> wrap_with_module() |> new_env()
-      assert in_context?(env, :struct_arguments)
+      assert in_context?(env, :struct_fields)
     end
 
     test "is true if the cursor starts in the middle of the struct" do
       env = "%User{name: \"John\", |}" |> wrap_with_module() |> new_env()
-      assert in_context?(env, :struct_arguments)
+      assert in_context?(env, :struct_fields)
     end
 
     test "is false if the cursor is after the closing curly" do
       env = "%User{}|" |> wrap_with_module() |> new_env()
-      refute in_context?(env, :struct_arguments)
+      refute in_context?(env, :struct_fields)
     end
 
     test "is true if the cursor is in current module arguments" do
       env = "%__MODULE__{|}" |> wrap_with_function() |> wrap_with_module() |> new_env()
-      assert in_context?(env, :struct_arguments)
+      assert in_context?(env, :struct_fields)
     end
 
     test "is true if the struct alias spans multiple lines" do
@@ -293,17 +293,22 @@ defmodule Lexical.Ast.EnvTest do
         }
       ]
       env = new_env(source)
-      assert in_context?(env, :struct_arguments)
+      assert in_context?(env, :struct_fields)
     end
 
     test "is true even if the value of a struct key is a tuple" do
       env = new_env("%User{favorite_numbers: {3}|")
-      assert in_context?(env, :struct_arguments)
+      assert in_context?(env, :struct_fields)
     end
 
     test "is true even if the cursor is at a nested struct" do
       env = new_env("%User{address: %Address{}|")
-      assert in_context?(env, :struct_arguments)
+      assert in_context?(env, :struct_fields)
+    end
+
+    test "is false if the cursor is in a map" do
+      env = "%{|field: value}" |> wrap_with_module() |> new_env()
+      refute in_context?(env, :struct_fields)
     end
   end
 
@@ -346,6 +351,55 @@ defmodule Lexical.Ast.EnvTest do
     end
   end
 
+  describe "in_context?(env, :struct_field_key)" do
+    test "is true if the cursor is after the struct opening" do
+      env = new_env("%User{|}")
+      assert in_context?(env, :struct_field_key)
+    end
+
+    test "is true if a key is partially typed" do
+      env = new_env("%User{fo|}")
+      assert in_context?(env, :struct_field_key)
+    end
+
+    test "is true if after a comma" do
+      env = new_env("%User{foo: 1, |}")
+      assert in_context?(env, :struct_field_key)
+    end
+
+    test "is true if after a comma on another line" do
+      source = ~q[
+        %User{
+          foo: 1,
+          |
+        }
+      ]
+
+      env = new_env(source)
+      assert in_context?(env, :struct_field_key)
+    end
+
+    test "is false in static keywords" do
+      env = "[fo|]" |> wrap_with_module() |> new_env()
+      refute in_context?(env, :struct_field_key)
+    end
+
+    test "is false in static keywords nested in a struct" do
+      env = "%User{foo: [fo|]}" |> wrap_with_module() |> new_env()
+      refute in_context?(env, :struct_field_key)
+    end
+
+    test "is false in map field key position" do
+      env = "%{|}" |> wrap_with_module() |> new_env()
+      refute in_context?(env, :struct_field_key)
+    end
+
+    test "is false in map field key position nested in a struct" do
+      env = "%User{foo: %{|}}" |> wrap_with_module() |> new_env()
+      refute in_context?(env, :struct_field_key)
+    end
+  end
+
   describe "in_context?(env, :struct_reference)" do
     test "is true if the reference starts on the beginning of the line" do
       env = new_env("%User|")

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

@@ -4,14 +4,14 @@ defmodule Lexical.Server.CodeIntelligence.Completion do
   alias Lexical.Completion.Translatable
   alias Lexical.Document
   alias Lexical.Document.Position
-  alias Lexical.Math
   alias Lexical.Project
   alias Lexical.Protocol.Types.Completion
   alias Lexical.Protocol.Types.InsertTextFormat
   alias Lexical.RemoteControl
   alias Lexical.RemoteControl.Completion.Candidate
   alias Lexical.RemoteControl.Modules.Predicate
   alias Lexical.Server.CodeIntelligence.Completion.Builder
+  alias Lexical.Server.Configuration
   alias Lexical.Server.Project.Intelligence
   alias Mix.Tasks.Namespace
 
@@ -38,34 +38,30 @@ defmodule Lexical.Server.CodeIntelligence.Completion do
     case Env.new(project, document, position) do
       {:ok, env} ->
         completions = completions(project, env, context)
-        Logger.warning("Emitting completions: #{inspect(completions)}")
-        completion_list(completions)
+        Logger.info("Emitting completions: #{inspect(completions)}")
+        maybe_to_completion_list(completions)
 
       {:error, _} = error ->
         Logger.error("Failed to build completion env #{inspect(error)}")
-        completion_list()
+        maybe_to_completion_list()
     end
   end
 
   defp completions(%Project{} = project, %Env{} = env, %Completion.Context{} = context) do
     prefix_tokens = Env.prefix_tokens(env, 1)
 
     cond do
-      prefix_tokens == [] ->
+      prefix_tokens == [] or not should_emit_completions?(env) ->
         []
 
-      match?([{:operator, :do, _}], prefix_tokens) and Env.empty?(env.suffix) ->
+      should_emit_do_end_snippet?(env) ->
         do_end_snippet = "do\n  $0\nend"
 
         env
         |> Builder.snippet(do_end_snippet, label: "do/end block")
         |> List.wrap()
 
-      Enum.empty?(prefix_tokens) or not context_will_give_meaningful_completions?(env) ->
-        []
-
-      Env.in_context?(env, :struct_arguments) and not Env.in_context?(env, :struct_field_value) and
-          not prefix_is_trigger?(env) ->
+      Env.in_context?(env, :struct_field_key) ->
         project
         |> RemoteControl.Api.complete_struct_fields(env.document, env.position)
         |> Enum.map(&Translatable.translate(&1, Builder, env))
@@ -79,16 +75,57 @@ defmodule Lexical.Server.CodeIntelligence.Completion do
     end
   end
 
-  defp prefix_is_trigger?(env) do
-    case Env.prefix_tokens(env, 1) do
-      [{_, token, _}] ->
-        to_string(token) in trigger_characters()
+  defp should_emit_completions?(%Env{} = env) do
+    always_emit_completions?() or has_meaningful_completions?(env)
+  end
+
+  defp always_emit_completions? do
+    # If VS Code receives an empty completion list, it will never issue
+    # a new request, even if `is_incomplete: true` is specified.
+    # https://github.com/lexical-lsp/lexical/issues/400
+    Configuration.get().client_name == "Visual Studio Code"
+  end
 
-      _ ->
+  defp has_meaningful_completions?(%Env{} = env) do
+    case Code.Fragment.cursor_context(env.prefix) do
+      :none ->
         false
+
+      {:unquoted_atom, name} ->
+        length(name) > 1
+
+      {:local_or_var, name} ->
+        local_length = length(name)
+        surround_begin = max(1, env.position.character - local_length - 1)
+
+        local_length > 1 or has_surround_context?(env.prefix, 1, surround_begin)
+
+      _ ->
+        true
     end
   end
 
+  defp has_surround_context?(fragment, line, column)
+       when is_binary(fragment) and line >= 1 and column >= 1 do
+    Code.Fragment.surround_context(fragment, {line, column}) != :none
+  end
+
+  # We emit a do/end snippet if the prefix token is the do operator and
+  # there is a space before the token preceding it on the same line. This
+  # handles situations like `@do|` where a do/end snippet would be invalid.
+  defp should_emit_do_end_snippet?(%Env{} = env) do
+    prefix_tokens = Env.prefix_tokens(env, 2)
+
+    valid_prefix? =
+      match?(
+        [{:operator, :do, {line, do_col}}, {_, _, {line, preceding_col}}]
+        when do_col - preceding_col > 1,
+        prefix_tokens
+      )
+
+    valid_prefix? and Env.empty?(env.suffix)
+  end
+
   defp to_completion_items(
          local_completions,
          %Project{} = project,
@@ -101,36 +138,15 @@ defmodule Lexical.Server.CodeIntelligence.Completion do
         displayable?(project, result),
         applies_to_context?(project, result, context),
         applies_to_env?(env, result),
-        %Completion.Item{} = item <- List.wrap(Translatable.translate(result, Builder, env)) do
+        %Completion.Item{} = item <- to_completion_item(result, env) do
       item
     end
   end
 
-  defp context_will_give_meaningful_completions?(%Env{} = env) do
-    case Code.Fragment.cursor_context(env.prefix) do
-      {:local_or_var, name} ->
-        local_length = length(name)
-
-        surround_begin =
-          Math.clamp(env.position.character - local_length - 1, 1, env.position.character)
-
-        case Code.Fragment.surround_context(env.prefix, {1, surround_begin}) do
-          :none ->
-            local_length > 1
-
-          _other ->
-            true
-        end
-
-      :none ->
-        false
-
-      {:unquoted_atom, name} ->
-        length(name) > 1
-
-      _ ->
-        true
-    end
+  defp to_completion_item(candidate, env) do
+    candidate
+    |> Translatable.translate(Builder, env)
+    |> List.wrap()
   end
 
   defp displayable?(%Project{} = project, result) do
@@ -229,7 +245,11 @@ defmodule Lexical.Server.CodeIntelligence.Completion do
     true
   end
 
-  defp completion_list(items \\ []) do
-    Completion.List.new(items: items, is_incomplete: true)
+  defp maybe_to_completion_list(items \\ [])
+
+  defp maybe_to_completion_list([]) do
+    Completion.List.new(items: [], is_incomplete: true)
   end
+
+  defp maybe_to_completion_list(items), do: items
 end

apps/server/lib/lexical/server/code_intelligence/completion/builder.ex

@@ -54,6 +54,7 @@ defmodule Lexical.Server.CodeIntelligence.Completion.Builder do
 
   @impl Builder
   def text_edit_snippet(%Env{} = env, text, {start_char, end_char}, options \\ []) do
+    snippet = String.trim_trailing(text, "\n")
     line_number = env.position.line
 
     range =
@@ -62,7 +63,7 @@ defmodule Lexical.Server.CodeIntelligence.Completion.Builder do
         Position.new(env.document, line_number, end_char)
       )
 
-    edits = Document.Changes.new(env.document, Edit.new(text, range))
+    edits = Document.Changes.new(env.document, Edit.new(snippet, range))
 
     options
     |> Keyword.put(:text_edit, edits)