Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve UX when completing struct #190

Merged
Merged
Show file tree
Hide file tree
Changes from 9 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions apps/remote_control/lib/lexical/remote_control/api.ex
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ defmodule Lexical.RemoteControl.Api do
alias Lexical.RemoteControl.Build
alias Lexical.RemoteControl.CodeIntelligence
alias Lexical.RemoteControl.CodeMod
alias Lexical.RemoteControl.Completion.Struct

require Logger

Expand Down Expand Up @@ -47,6 +48,10 @@ defmodule Lexical.RemoteControl.Api do
])
end

def struct_doc(%Project{} = project, struct_module) do
scohen marked this conversation as resolved.
Show resolved Hide resolved
RemoteControl.call(project, Struct, :doc, [struct_module])
scohen marked this conversation as resolved.
Show resolved Hide resolved
end

def definition(%Project{} = project, %Document{} = document, %Position{} = position) do
RemoteControl.call(project, CodeIntelligence.Definition, :definition, [
document,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,80 @@
defmodule Lexical.RemoteControl.Completion.Struct do
scottming marked this conversation as resolved.
Show resolved Hide resolved
require Logger

def doc(full_name) when is_binary(full_name) do
struct = Module.concat(Elixir, full_name)

case fetch_struct_type(struct) do
{:ok, t} ->
default = default_key_values(struct)

t
|> Macro.to_string()
|> trim_parent(full_name)
|> maybe_put_default(default)
|> replace_default_symbol()

_ ->
"#{inspect(struct.__struct__(), pretty: true, width: 40)}"
|> trim_parent(full_name)
end
end

defp fetch_struct_type(struct) do
with {:ok, types} <- Code.Typespec.fetch_types(struct),
{:type, t} <- Enum.find(types, &match?({:type, {:t, _, _}}, &1)) do
scohen marked this conversation as resolved.
Show resolved Hide resolved
{:ok, Code.Typespec.type_to_quoted(t)}
end
end

defp maybe_put_default(doc, []) do
doc
|> Sourceror.parse_string!()
|> to_pretty_string()
end

defp maybe_put_default(doc, default_key_values) do
doc
|> Sourceror.parse_string!()
|> Macro.postwalk(fn
{{:__block__, _, [key_name]} = key, value} ->
if key_name in Keyword.keys(default_key_values) do
value_string = Sourceror.to_string(value, locals_without_parens: [])

new_value =
"#{value_string} || #{inspect(default_key_values[key_name])}"
|> Sourceror.parse_string!()

{key, new_value}
else
{key, value}
end

quoted ->
quoted
end)
|> to_pretty_string()
end

defp default_key_values(struct) do
for {key, value} <- Map.from_struct(struct.__struct__), not is_nil(value) do
{key, value}
end
end

defp to_pretty_string(quoted) do
Sourceror.to_string(quoted, locals_without_parens: [], line_length: 40)
end

defp trim_parent(doc, full_name) do
parent_module = full_name |> String.split(".") |> Enum.slice(0..-2) |> Enum.join(".")

doc
|> String.replace("%#{parent_module}.", "%")
|> String.replace(" #{parent_module}.", " ")
end

defp replace_default_symbol(doc) do
String.replace(doc, "||", "\\\\")
end
end
17 changes: 17 additions & 0 deletions apps/remote_control/test/fixtures/project/lib/structs.ex
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,23 @@ defmodule Project.Structs do
}
end

defmodule Order do
@type t :: %__MODULE__{
id: integer(),
lines: [Line.t()]
}
defstruct [:id, :lines]

defmodule Line do
@type t :: %__MODULE__{
id: integer(),
product_id: integer(),
quantity: integer()
}
defstruct [:id, :product_id, :quantity]
end
end

defmodule NotAStruct do
end
end
51 changes: 18 additions & 33 deletions apps/server/lib/lexical/server/code_intelligence/completion/env.ex
Original file line number Diff line number Diff line change
Expand Up @@ -113,13 +113,24 @@ defmodule Lexical.Server.CodeIntelligence.Completion.Env do

@impl Environment
def in_context?(%__MODULE__{} = env, :pipe) do
with {:ok, line, context} <- surround_context(env),
{:ok, {:operator, '|>'}} <- previous_surround_context(line, context) do
true
else
_ ->
false
end
env
|> prefix_tokens(:all)
|> Enum.reduce_while(false, fn
{:identifier, _}, _ ->
{:cont, false}

{:operator, :.}, _ ->
{:cont, false}

{:alias, _}, _ ->
{:cont, false}

{:arrow_op, nil}, _ ->
{:halt, true}

_x, _acc ->
{:halt, false}
end)
end

@impl Environment
Expand Down Expand Up @@ -340,32 +351,6 @@ defmodule Lexical.Server.CodeIntelligence.Completion.Env do
end
end

defp surround_context(%__MODULE__{} = env) do
with {:ok, line} <- Document.fetch_text_at(env.document, env.position.line),
%{context: _} = context <-
Code.Fragment.surround_context(line, {1, env.zero_based_character}) do
{:ok, line, context}
end
end

defp previous_surround_context(line, %{begin: {1, column}}) do
previous_surround_context(line, column)
end

defp previous_surround_context(_line, 1) do
:error
end

defp previous_surround_context(line, character) when is_integer(character) do
case Code.Fragment.surround_context(line, {1, character - 1}) do
:none ->
previous_surround_context(line, character - 1)

%{context: context} ->
{:ok, context}
end
end

# credo:disable-for-next-line Credo.Check.Refactor.CyclomaticComplexity
defp normalize_token(token) do
case token do
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,25 @@ defmodule Lexical.Server.CodeIntelligence.Completion.Translations.Callable do

@callables [Result.Function, Result.Macro]

def completion(%callable_module{argument_names: []} = callable, %Env{} = env)
when callable_module in @callables do
if not Env.in_context?(env, :pipe) do
do_completion(callable, env)
end
end

def completion(%callable_module{} = callable, %Env{} = env)
when callable_module in @callables do
do_completion(callable, env)
end

defp do_completion(callable, %Env{} = env) do
callable = reduce_args_when_pipe(callable, env)
add_args? = not String.contains?(env.suffix, "(")

insert_text =
if add_args? do
callable_snippet(callable, env)
callable_snippet(callable)
else
callable.name
end
Expand Down Expand Up @@ -41,7 +53,7 @@ defmodule Lexical.Server.CodeIntelligence.Completion.Translations.Callable do
)

call_capture =
Env.snippet(env, callable_snippet(callable, env),
Env.snippet(env, callable_snippet(callable),
detail: "(Capture with arguments)",
kind: :function,
label: label(callable),
Expand All @@ -51,16 +63,9 @@ defmodule Lexical.Server.CodeIntelligence.Completion.Translations.Callable do
[complete_capture, call_capture]
end

defp callable_snippet(%_{} = callable, %Env{} = env) do
argument_names =
if Env.in_context?(env, :pipe) do
tl(callable.argument_names)
else
callable.argument_names
end

defp callable_snippet(%_{} = callable) do
argument_templates =
argument_names
callable.argument_names
|> Enum.with_index(1)
|> Enum.map_join(", ", fn {name, index} ->
"${#{index}:#{name}}"
Expand All @@ -82,4 +87,12 @@ defmodule Lexical.Server.CodeIntelligence.Completion.Translations.Callable do
defp name_and_arity(%_{name: name, arity: arity}) do
"#{name}/#{arity}"
end

defp reduce_args_when_pipe(%{argument_names: argument_names} = callable, %Env{} = env) do
if Env.in_context?(env, :pipe) do
%{callable | argument_names: tl(argument_names)}
else
callable
end
end
end
Original file line number Diff line number Diff line change
Expand Up @@ -24,15 +24,28 @@ defmodule Lexical.Server.CodeIntelligence.Completion.Translations.ModuleOrBehavi

defines_struct? = Intelligence.defines_struct?(env.project, module.full_name)

immediate_descentent_structs =
immediate_descendent_struct_modules(env.project, module.full_name)

defines_struct_in_descentents? =
scottming marked this conversation as resolved.
Show resolved Hide resolved
immediate_descentent_defines_struct?(env.project, module.full_name) and
length(immediate_descentent_structs) > 1

cond do
struct_reference? and defines_struct_in_descentents? and defines_struct? ->
more = length(immediate_descentent_structs) - 1

[
Translations.Struct.completion(env, builder, module.name, module.full_name, more),
Translations.Struct.completion(env, builder, module.name, module.full_name)
]

struct_reference? and defines_struct? ->
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 ->
Enum.map(immediate_descentent_structs, fn child_module_name ->
local_name = local_module_name(module.full_name, child_module_name)
Translations.Struct.completion(env, builder, local_name, child_module_name)
end)
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
defmodule Lexical.Server.CodeIntelligence.Completion.Translations.Struct do
alias Lexical.Protocol.Types.Markup.Content
alias Lexical.RemoteControl.Api
alias Lexical.RemoteControl.Completion.Result
alias Lexical.Server.CodeIntelligence.Completion.Env
alias Lexical.Server.CodeIntelligence.Completion.Translatable
Expand All @@ -20,11 +22,25 @@ defmodule Lexical.Server.CodeIntelligence.Completion.Translations.Struct do
end
end

def completion(%Env{} = env, builder, module_name, full_name, more) when is_integer(more) do
builder_opts = [
kind: :module,
label: "#{module_name}...(#{more} more structs)",
detail: "#{full_name}."
]

insert_text = "#{module_name}."
range = edit_range(env)

builder.text_edit_snippet(env, insert_text, range, builder_opts)
end

def completion(%Env{} = env, builder, struct_name, full_name) do
builder_opts = [
kind: :struct,
detail: "#{full_name}",
label: "#{struct_name}"
label: "#{struct_name}",
documentation: documentation(env.project, full_name)
]

range = edit_range(env)
Expand All @@ -39,6 +55,20 @@ defmodule Lexical.Server.CodeIntelligence.Completion.Translations.Struct do
builder.text_edit_snippet(env, insert_text, range, builder_opts)
end

defp documentation(project, full_name) do
project |> Api.struct_doc(full_name) |> to_markup()
end

defp to_markup(doc) do
value = """
```elixir
#{doc}
```
"""

Content.new(kind: :markdown, value: value)
end

defp add_curlies?(%Env{} = env) do
if Env.in_context?(env, :struct_reference) do
not String.contains?(env.suffix, "{")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -417,6 +417,11 @@ defmodule Lexical.Server.CodeIntelligence.Completion.EnvTest do
assert in_context?(env, :pipe)
end

test "is true if the pipe is in a remote function call" do
env = new_env("[] |> Enum.|")
assert in_context?(env, :pipe)
end

test "is false if the pipe is in a function call and the cursor is outside it" do
env = new_env("foo( a |> b |> c)|")
refute in_context?(env, :pipe)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,13 +12,30 @@ defmodule Lexical.Server.CodeIntelligence.Completion.Translations.FunctionTest d
assert [:deprecated] = completion.tags
end

test "suggest arity 0 functions if not in a pipeline", %{project: project} do
{:ok, completion} =
project
|> complete("Application.loaded_app|")
|> fetch_completion(kind: :function)

assert completion.insert_text == "loaded_applications()"
end

test "do not suggest arity 0 functions if in a pipeline", %{project: project} do
assert {:error, :not_found} =
project
|> complete("|> Application.loaded_app|")
|> fetch_completion(kind: :function)
end

test "arity 1 omits arguments if in a pipeline", %{project: project} do
{:ok, [completion, _]} =
project
|> complete("|> Enum.dedu|")
|> fetch_completion(kind: :function)

assert completion.insert_text == "dedup()"
assert completion.label == "dedup()"
end

test "arity > 1 omits the first argument if in a pipeline", %{project: project} do
Expand Down
Loading