From 17deaa485b2320c24f6bddd0e6b02a2282123ca9 Mon Sep 17 00:00:00 2001 From: Lukasz Samson Date: Thu, 28 Dec 2023 00:09:51 +0100 Subject: [PATCH] make completions work for quoted remote calls `Mymod."4quo\"ted"()` is valid elixir --- CHANGELOG.md | 3 +- .../language_server/providers/completion.ex | 102 +++++++++++------- .../test/providers/completion_test.exs | 36 ++++++- .../support/fixtures/example_quoted_defs.ex | 9 ++ 4 files changed, 109 insertions(+), 41 deletions(-) create mode 100644 apps/language_server/test/support/fixtures/example_quoted_defs.ex diff --git a/CHANGELOG.md b/CHANGELOG.md index 16764ffe9..55dfde8e8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,12 +1,13 @@ ### Unreleased -### v0.18.1: 27 December 2023 +### v0.18.1: 28 December 2023 #### Improvements - Variables defined in `ex_unit` `test`, `setup` and `setup_all` context are now returned by completions provider. Navigation to variable definition and references now also works correctly - Suggest spec code lens now emits specs for all arity variants when function has default arguments. Previously only the one with all parameters was emitted - Missing required OTP `:crypto` module is now detected on startup +- Completions provider is now properly returning quoted remote calls. Previously accepting a suggestion would insert invalid code #### Fixes diff --git a/apps/language_server/lib/language_server/providers/completion.ex b/apps/language_server/lib/language_server/providers/completion.ex index 2fe91c552..6df2e7bef 100644 --- a/apps/language_server/lib/language_server/providers/completion.ex +++ b/apps/language_server/lib/language_server/providers/completion.ex @@ -695,7 +695,13 @@ defmodule ElixirLS.LanguageServer.Providers.Completion do insert_text = case name do name when name in ["size", "unit"] -> - function_snippet(name, ["integer"], 1, options |> Keyword.merge(with_parens?: true)) + function_snippet( + name, + ["integer"], + 1, + "Kernel", + options |> Keyword.merge(with_parens?: true) + ) other -> other @@ -895,7 +901,7 @@ defmodule ElixirLS.LanguageServer.Providers.Completion do defp def_snippet(def_str, name, args, arity, opts) do if Keyword.get(opts, :snippets_supported, false) do - "#{def_str}#{function_snippet(name, args, arity, opts)} do\n\t$0\nend" + "#{def_str}#{function_snippet(name, args, arity, "Kernel", opts)} do\n\t$0\nend" else "#{def_str}#{name}" end @@ -972,7 +978,8 @@ defmodule ElixirLS.LanguageServer.Providers.Completion do nil end - def function_snippet(name, args, arity, opts) do + def function_snippet(name, args, arity, origin, opts) do + name = sanitize_function_name(name, origin) snippets_supported? = Keyword.get(opts, :snippets_supported, false) trigger_signature? = Keyword.get(opts, :trigger_signature?, false) capture_before? = Keyword.get(opts, :capture_before?, false) @@ -1173,36 +1180,32 @@ defmodule ElixirLS.LanguageServer.Providers.Completion do trigger_signature? = signature_help_supported? && ((arity == 1 && !pipe_before?) || arity > 1) - {label, insert_text} = + insert_text = cond do match?("~" <> _, name) -> "~" <> sigil_name = name - {name, sigil_name} + sigil_name use_name_only?(origin, name) or String.starts_with?(text_after_cursor, "(") -> - {name, name} + sanitize_function_name(name, origin) true -> - label = name - - insert_text = - function_snippet( - name, - args_list, - arity, - Keyword.merge( - options, - pipe_before?: pipe_before?, - capture_before?: capture_before?, - trigger_signature?: trigger_signature?, - locals_without_parens: locals_without_parens, - text_after_cursor: text_after_cursor, - with_parens?: with_parens?, - snippet: info[:snippet] - ) + function_snippet( + name, + args_list, + arity, + origin, + Keyword.merge( + options, + pipe_before?: pipe_before?, + capture_before?: capture_before?, + trigger_signature?: trigger_signature?, + locals_without_parens: locals_without_parens, + text_after_cursor: text_after_cursor, + with_parens?: with_parens?, + snippet: info[:snippet] ) - - {label, insert_text} + ) end footer = SourceFile.format_spec(spec, line_length: 30) @@ -1215,20 +1218,24 @@ defmodule ElixirLS.LanguageServer.Providers.Completion do } end - %__MODULE__{ - label: label, - kind: :function, - detail: to_string(type), - label_details: %{ - "detail" => "(#{Enum.join(args_list, ", ")})", - "description" => "#{origin}.#{name}/#{arity}" - }, - documentation: summary <> footer, - insert_text: insert_text, - priority: 17, - tags: metadata_to_tags(metadata), - command: command - } + label = sanitize_function_name(name, origin) + + if label == name or remote_calls? do + %__MODULE__{ + label: label, + kind: :function, + detail: to_string(type), + label_details: %{ + "detail" => "(#{Enum.join(args_list, ", ")})", + "description" => "#{origin}.#{label}/#{arity}" + }, + documentation: summary <> footer, + insert_text: insert_text, + priority: 17, + tags: metadata_to_tags(metadata), + command: command + } + end end defp use_name_only?(module_name, function_name) do @@ -1369,4 +1376,21 @@ defmodule ElixirLS.LanguageServer.Providers.Completion do |> MapSet.member?({String.to_atom(name), arity}) |> Kernel.not() end + + defp sanitize_function_name(name, origin) when origin in ["Kernel", "Kernel.SpecialForms"], + do: name + + defp sanitize_function_name(name, origin) do + if not Regex.match?(~r/^([_\p{Ll}\p{Lo}][\p{L}\p{N}_]*[?!]?)$/u, name) do + # not an allowed identifier - quote + escaped = + name + |> String.replace("\\", "\\\\") + |> String.replace("\"", "\\\"") + + "\"" <> escaped <> "\"" + else + name + end + end end diff --git a/apps/language_server/test/providers/completion_test.exs b/apps/language_server/test/providers/completion_test.exs index 66e7db4d9..d5ad19aea 100644 --- a/apps/language_server/test/providers/completion_test.exs +++ b/apps/language_server/test/providers/completion_test.exs @@ -1442,6 +1442,34 @@ defmodule ElixirLS.LanguageServer.Providers.CompletionTest do "insertText" => "defprotocol $1 do\n\t$0\nend" } = first end + + test "will suggest remote quoted calls" do + text = """ + alias ElixirLS.LanguageServer.Fixtures.ExampleQuotedDefs, as: Quoted + Quoted. + # ^ + """ + + {line, char} = {1, 7} + + TestUtils.assert_has_cursor_char(text, line, char) + {line, char} = SourceFile.lsp_position_to_elixir(text, {line, char}) + parser_context = ParserContextBuilder.from_string(text, {line, char}) + + assert {:ok, %{"items" => items}} = + Completion.completion( + parser_context, + line, + char, + @supports + ) + + assert item = Enum.find(items, fn item -> item["label"] == "\"0abc\\\"asd\"" end) + assert item["insertText"] == "\"0abc\\\"asd\"($1)$0" + + assert item["labelDetails"]["description"] == + "ElixirLS.LanguageServer.Fixtures.ExampleQuotedDefs.\"0abc\\\"asd\"/2" + end end describe "generic suggestions" do @@ -1501,7 +1529,13 @@ defmodule ElixirLS.LanguageServer.Providers.CompletionTest do ] assert "do_sth()" == - Completion.function_snippet("do_sth", ["My.record(x: x0, y: y0)"], 1, opts) + Completion.function_snippet( + "do_sth", + ["My.record(x: x0, y: y0)"], + 1, + "Kernel", + opts + ) end end diff --git a/apps/language_server/test/support/fixtures/example_quoted_defs.ex b/apps/language_server/test/support/fixtures/example_quoted_defs.ex new file mode 100644 index 000000000..756e797a0 --- /dev/null +++ b/apps/language_server/test/support/fixtures/example_quoted_defs.ex @@ -0,0 +1,9 @@ +defmodule ElixirLS.LanguageServer.Fixtures.ExampleQuotedDefs do + @doc """ + quoted def + """ + @spec unquote(:"0abc\"asd")(any, integer) :: :ok + def unquote(:"0abc\"asd")(example, arg) do + :ok + end +end