Improved struct completion (#181)

Struct completion under elixir sense was extremely inconsistent from
context to context and even file to file. This was extremely
frustrating, as structs might or might not complete depending,
seemingly on the time of day.

On the other hand, module completion was pretty consistent, so to fix
the issue, prior to sending a document for completion, if we're in a
struct reference, we strip it out to get the original module reference
back. This means we'll only get module completions from elixir_sense,
but we already check for struct references and if we detect that we're
in one, turn module completions into struct completions.

I've played around with this and it does seem to fix the problems I
was having prior.

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

@@ -61,8 +61,10 @@ defmodule Lexical.Server.CodeIntelligence.Completion do
         Completion.List.new(items: [], is_incomplete: true)
 
       true ->
+        {document, position} = Env.strip_struct_reference(env)
+
         project
-        |> RemoteControl.Api.complete(env.document, env.position)
+        |> RemoteControl.Api.complete(document, position)
         |> to_completion_items(project, env, context)
     end
   end
@@ -142,7 +144,7 @@ defmodule Lexical.Server.CodeIntelligence.Completion do
         true
 
       struct_reference? and struct_module == Result.Module ->
-        Intelligence.defines_struct?(env.project, result.full_name, to: :grandchild)
+        Intelligence.defines_struct?(env.project, result.full_name, to: :child)
 
       struct_reference? and match?(%Result.Macro{name: "__MODULE__"}, result) ->
         true

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

@@ -10,6 +10,8 @@ defmodule Lexical.Server.CodeIntelligence.Completion.Env do
   alias Lexical.Protocol.Types.Completion
   alias Lexical.Server.CodeIntelligence.Completion.Env
 
+  import Document.Line
+
   defstruct [
     :project,
     :document,
@@ -262,8 +264,60 @@ defmodule Lexical.Server.CodeIntelligence.Completion.Env do
     boost(text, 0)
   end
 
+  # end builder behaviour
+
+  @spec strip_struct_reference(t()) :: {Document.t(), Position.t()}
+  def strip_struct_reference(%__MODULE__{} = env) do
+    if in_context?(env, :struct_reference) do
+      do_strip_struct_reference(env)
+    else
+      {env.document, env.position}
+    end
+  end
+
   # private
 
+  defp do_strip_struct_reference(%__MODULE__{} = env) do
+    completion_length =
+      case Code.Fragment.cursor_context(env.prefix) do
+        {:struct, {:dot, {:alias, struct_name}, []}} ->
+          # add one because of the trailing period
+          length(struct_name) + 1
+
+        {:struct, {:local_or_var, local_name}} ->
+          length(local_name)
+
+        {:struct, struct_name} ->
+          length(struct_name)
+
+        {:local_or_var, local_name} ->
+          length(local_name)
+      end
+
+    column = env.position.character
+    percent_position = column - (completion_length + 1)
+
+    new_line_start = String.slice(env.line, 0, percent_position - 1)
+    new_line_end = String.slice(env.line, percent_position..-1)
+    new_line = [new_line_start, new_line_end]
+    new_position = Position.new(env.position.line, env.position.character - 1)
+    line_to_replace = env.position.line
+
+    new_document =
+      env.document.lines
+      |> Enum.with_index(1)
+      |> Enum.reduce([], fn
+        {line(ending: ending), ^line_to_replace}, acc ->
+          [acc, new_line, ending]
+
+        {line(text: line_text, ending: ending), _}, acc ->
+          [acc, line_text, ending]
+      end)
+      |> IO.iodata_to_binary()
+
+    {new_document, new_position}
+  end
+
   defp in_directive?(%__MODULE__{} = env, context_name) do
     env
     |> prefix_token_stream()

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

@@ -26,15 +26,15 @@ defmodule Lexical.Server.CodeIntelligence.Completion.Translations.ModuleOrBehavi
 
     cond do
       struct_reference? and defines_struct? ->
-        Translations.Struct.completion(env, builder, module.name)
+        Translations.Struct.completion(env, builder, module.name, module.full_name)
 
       struct_reference? and
           immediate_descentent_defines_struct?(env.project, module.full_name) ->
         env.project
         |> immediate_descendent_struct_modules(module.full_name)
         |> Enum.map(fn child_module_name ->
           local_name = local_module_name(module.full_name, child_module_name)
-          Translations.Struct.completion(env, builder, local_name)
+          Translations.Struct.completion(env, builder, local_name, child_module_name)
         end)
 
       true ->

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

@@ -5,24 +5,26 @@ defmodule Lexical.Server.CodeIntelligence.Completion.Translations.Struct do
   alias Lexical.Server.CodeIntelligence.Completion.Translations
 
   use Translatable.Impl, for: Result.Struct
+  require Logger
 
   def translate(%Result.Struct{} = struct, builder, %Env{} = env) do
     if Env.in_context?(env, :struct_reference) do
-      completion(env, builder, struct.name)
+      completion(env, builder, struct.name, struct.full_name)
     else
       Translations.ModuleOrBehaviour.completion(
         env,
         builder,
-        struct.name
+        struct.name,
+        struct.full_name
       )
     end
   end
 
-  def completion(%Env{} = env, builder, struct_name) do
+  def completion(%Env{} = env, builder, struct_name, full_name) do
     builder_opts = [
       kind: :struct,
-      detail: "#{struct_name} (Struct)",
-      label: "%#{struct_name}"
+      detail: "#{full_name}",
+      label: "#{struct_name}"
     ]
 
     range = edit_range(env)
@@ -37,46 +39,14 @@ defmodule Lexical.Server.CodeIntelligence.Completion.Translations.Struct do
     builder.text_edit_snippet(env, insert_text, range, builder_opts)
   end
 
-  def add_curlies?(%Env{} = env) do
+  defp add_curlies?(%Env{} = env) do
     if Env.in_context?(env, :struct_reference) do
       not String.contains?(env.suffix, "{")
     else
       false
     end
   end
 
-  def add_percent?(%Env{} = env) do
-    if Env.in_context?(env, :struct_reference) do
-      # A leading percent is added only if the struct reference is to a top-level struct.
-      # If it's for a child struct (e.g. %Types.Range) then adding a percent at "Range"
-      # will be syntactically invalid and get us `%Types.%Range{}`
-
-      struct_module_name =
-        case Code.Fragment.cursor_context(env.prefix) do
-          {:struct, {:dot, {:alias, module_name}, []}} ->
-            '#{module_name}.'
-
-          {:struct, module_name} ->
-            module_name
-
-          {:dot, {:alias, module_name}, _} ->
-            module_name
-
-          _ ->
-            ''
-        end
-
-      contains_period? =
-        struct_module_name
-        |> List.to_string()
-        |> String.contains?(".")
-
-      not contains_period?
-    else
-      false
-    end
-  end
-
   defp edit_range(%Env{} = env) do
     prefix_end = env.position.character
 

apps/server/test/lexical/server/code_intelligence/completion/env_test.exs

@@ -1,18 +1,21 @@
 defmodule Lexical.Server.CodeIntelligence.Completion.EnvTest do
   alias Lexical.Document
   alias Lexical.Server.CodeIntelligence.Completion
+  alias Lexical.Test.CodeSigil
   alias Lexical.Test.CursorSupport
   alias Lexical.Test.Fixtures
 
-  use ExUnit.Case
+  use ExUnit.Case, async: true
+
+  import CodeSigil
   import Completion.Env
   import CursorSupport
   import Fixtures
 
   def new_env(text) do
     project = project()
     {line, column} = cursor_position(text)
-    stripped_text = context_before_cursor(text)
+    stripped_text = strip_cursor(text)
     document = Document.new("file://foo.ex", stripped_text, 0)
 
     position = Document.Position.new(line, column)
@@ -473,12 +476,12 @@ defmodule Lexical.Server.CodeIntelligence.Completion.EnvTest do
 
     test "should be true if this is a multiple alias on multiple lines" do
       env =
-        """
+        ~q[
         alias Foo.{
           Bar,
           Baz|
         }
-        """
+        ]t
         |> new_env()
 
       assert in_context?(env, :alias)
@@ -499,35 +502,35 @@ defmodule Lexical.Server.CodeIntelligence.Completion.EnvTest do
 
     test "should be false if this is after a multiple alias on multiple lines" do
       env =
-        """
+        ~q[
         alias Foo.{
           Bar,
           Baz
         }|
-        """
+        ]t
         |> new_env()
 
       refute in_context?(env, :alias)
     end
 
     test "should be false if this is after a multiple alias on multiple lines (second form)" do
       env =
-        """
+        ~q[
         alias Foo.{ Bar,
           Baz
         }|
-        """
+        ]t
         |> new_env()
 
       refute in_context?(env, :alias)
     end
 
     test "should be false if this is after a multiple alias on multiple lines (third form)" do
       env =
-        """
+        ~q[
         alias Foo.{ Bar, Baz
         }|
-        """
+        ]t
         |> new_env()
 
       refute in_context?(env, :alias)
@@ -540,10 +543,10 @@ defmodule Lexical.Server.CodeIntelligence.Completion.EnvTest do
 
     test "is false if the alias is on another line" do
       env =
-        """
+        ~q[
         alias Something.Else
         Macro.|
-        """
+        ]t
         |> new_env()
 
       refute in_context?(env, :alias)
@@ -579,4 +582,106 @@ defmodule Lexical.Server.CodeIntelligence.Completion.EnvTest do
       refute in_context?(env, :alias)
     end
   end
+
+  describe "strip_struct_reference/1" do
+    test "with a reference followed by  __" do
+      {doc, _position} =
+        "%__"
+        |> new_env()
+        |> strip_struct_reference()
+
+      assert doc == "__"
+    end
+
+    test "with a reference followed by a module name" do
+      {doc, _position} =
+        "%Module"
+        |> new_env()
+        |> strip_struct_reference()
+
+      assert doc == "Module"
+    end
+
+    test "with a reference followed by a module and a dot" do
+      {doc, _position} =
+        "%Module."
+        |> new_env()
+        |> strip_struct_reference()
+
+      assert doc == "Module."
+    end
+
+    test "with a reference followed by a nested module" do
+      {doc, _position} =
+        "%Module.Sub"
+        |> new_env()
+        |> strip_struct_reference()
+
+      assert doc == "Module.Sub"
+    end
+
+    test "with a reference followed by an alias" do
+      code = ~q[
+        alias Something.Else
+        %El|
+      ]t
+
+      {doc, _position} =
+        code
+        |> new_env()
+        |> strip_struct_reference()
+
+      assert doc == "alias Something.Else\nEl"
+    end
+
+    test "on a line with two references, replacing the first" do
+      {doc, _position} =
+        "%First{} = %Se"
+        |> new_env()
+        |> strip_struct_reference()
+
+      assert doc == "%First{} = Se"
+    end
+
+    test "on a line with two references, replacing the second" do
+      {doc, _position} =
+        "%Fir| = %Second{}"
+        |> new_env()
+        |> strip_struct_reference()
+
+      assert doc == "Fir = %Second{}"
+    end
+
+    test "with a plain module" do
+      env = new_env("Module")
+      {doc, _position} = strip_struct_reference(env)
+
+      assert doc == env.document
+    end
+
+    test "with a plain module strip_struct_reference a dot" do
+      env = new_env("Module.")
+      {doc, _position} = strip_struct_reference(env)
+
+      assert doc == env.document
+    end
+
+    test "leaves leading spaces in place" do
+      {doc, _position} =
+        "     %Some"
+        |> new_env()
+        |> strip_struct_reference()
+
+      assert doc == "     Some"
+    end
+
+    test "works in a function definition" do
+      {doc, _position} =
+        "def my_function(%Lo|)"
+        |> new_env()
+        |> strip_struct_reference()
+
+      assert doc == "def my_function(Lo)"
+    end
+  end
 end