From 02c101d03c0b5a81379b3905e7baa6e685c0fe99 Mon Sep 17 00:00:00 2001 From: Gustavo Aguiar Date: Mon, 27 Nov 2023 17:42:06 -0300 Subject: [PATCH] Improve Phoenix integration (#281) * add phoenix scope utils * refactor source to be more readable * add phoenix plugin * phoenix goto def * improve scopes module readability * remove redundant `is_list/1` guard * define `suggestions/4` only on elixir versions >= 1.14.0 * require elixir >=1.14 to run tests * fixup! require elixir >=1.14 to run tests * add @moduledoc false * expand variables and module attributes on scope * fixup! expand variables and module attributes on scope * fixup! expand variables and module attributes on scope --------- Co-authored-by: Gustavo Aguiar --- lib/elixir_sense/core/source.ex | 60 ++++++-- lib/elixir_sense/plugins/phoenix.ex | 105 ++++++++++++++ lib/elixir_sense/plugins/phoenix/scope.ex | 110 +++++++++++++++ lib/elixir_sense/providers/definition.ex | 30 ++-- test/elixir_sense/definition_test.exs | 21 +++ .../plugins/phoenix/scope_test.exs | 116 ++++++++++++++++ test/elixir_sense/plugins/phoenix_test.exs | 130 ++++++++++++++++++ .../plugins/phoenix/page_controller.ex | 10 ++ test/support/plugins/phoenix/router.ex | 4 + 9 files changed, 563 insertions(+), 23 deletions(-) create mode 100644 lib/elixir_sense/plugins/phoenix.ex create mode 100644 lib/elixir_sense/plugins/phoenix/scope.ex create mode 100644 test/elixir_sense/plugins/phoenix/scope_test.exs create mode 100644 test/elixir_sense/plugins/phoenix_test.exs create mode 100644 test/support/plugins/phoenix/page_controller.ex create mode 100644 test/support/plugins/phoenix/router.ex diff --git a/lib/elixir_sense/core/source.ex b/lib/elixir_sense/core/source.ex index 1e45487c..a793b714 100644 --- a/lib/elixir_sense/core/source.ex +++ b/lib/elixir_sense/core/source.ex @@ -400,25 +400,26 @@ defmodule ElixirSense.Core.Source do # TODO refactor to use Macro.path on elixir 1.14 with {:ok, ast} <- NormalizedCode.Fragment.container_cursor_to_quoted(prefix, columns: true), - {_, {:ok, call, npar, meta, options, cursor_at_option, option}} <- - Macro.prewalk(ast, nil, &find_call_pre/2), - {{m, elixir_prefix}, f} when f not in @excluded_funs <- get_mod_fun(call, binding_env) do + {_, {:ok, call_info}} <- Macro.prewalk(ast, nil, &find_call_pre/2), + {{m, elixir_prefix}, f} when f not in @excluded_funs <- + get_mod_fun(call_info.call, binding_env) do %{ candidate: {m, f}, elixir_prefix: elixir_prefix, - npar: npar, - pos: {{meta[:line], meta[:column]}, {meta[:line], nil}}, - cursor_at_option: cursor_at_option, - options_so_far: options, - option: option + params: call_info.params, + npar: call_info.npar, + pos: {{call_info.meta[:line], call_info.meta[:column]}, {call_info.meta[:line], nil}}, + cursor_at_option: call_info.cursor_at_option, + options_so_far: call_info.options, + option: call_info.option } else _ -> nil end end - def find_call_pre(ast, {:ok, call, npar, meta, options, cursor_at_option, option}), - do: {ast, {:ok, call, npar, meta, options, cursor_at_option, option}} + def find_call_pre(ast, {:ok, call_info}), + do: {ast, {:ok, call_info}} # transform `a |> b(c)` calls into `b(a, c)` def find_call_pre({:|>, _, [params_1, {call, meta, params_rest}]}, state) do @@ -440,20 +441,45 @@ defmodule ElixirSense.Core.Source do defp find_cursor_in_params(params, call, meta) do case Enum.reverse(params) do [{:__cursor__, _, []} | rest] -> - {:ok, call, length(rest), meta, [], :maybe, nil} + {:ok, + %{ + call: call, + params: Enum.reverse(rest), + npar: length(rest), + meta: meta, + options: [], + cursor_at_option: :maybe, + option: nil + }} [keyword_list | rest] when is_list(keyword_list) -> case Enum.reverse(keyword_list) do [{:__cursor__, _, []} | kl_rest] -> if Keyword.keyword?(kl_rest) do - {:ok, call, length(rest), meta, Enum.reverse(kl_rest) |> Enum.map(&elem(&1, 0)), - true, nil} + {:ok, + %{ + call: call, + params: Enum.reverse(rest), + npar: length(rest), + meta: meta, + options: Enum.reverse(kl_rest) |> Enum.map(&elem(&1, 0)), + cursor_at_option: true, + option: nil + }} end [{atom, {:__cursor__, _, []}} | kl_rest] when is_atom(atom) -> if Keyword.keyword?(kl_rest) do - {:ok, call, length(rest), meta, Enum.reverse(kl_rest) |> Enum.map(&elem(&1, 0)), - false, atom} + {:ok, + %{ + call: call, + params: Enum.reverse(rest), + npar: length(rest), + meta: meta, + options: Enum.reverse(kl_rest) |> Enum.map(&elem(&1, 0)), + cursor_at_option: false, + option: atom + }} end _ -> @@ -504,6 +530,10 @@ defmodule ElixirSense.Core.Source do def get_mod_fun([atom, fun], _binding_env) when is_atom(atom), do: {{atom, false}, fun} def get_mod_fun(_, _binding_env), do: nil + def get_mod([{:__aliases__, _, list} | _rest], binding_env) do + get_mod(list, binding_env) + end + def get_mod([{:__MODULE__, _, nil} | rest], binding_env) do if binding_env.current_module not in [nil, Elixir] do mod = diff --git a/lib/elixir_sense/plugins/phoenix.ex b/lib/elixir_sense/plugins/phoenix.ex new file mode 100644 index 00000000..75864eea --- /dev/null +++ b/lib/elixir_sense/plugins/phoenix.ex @@ -0,0 +1,105 @@ +defmodule ElixirSense.Plugins.Phoenix do + @moduledoc false + + @behaviour ElixirSense.Plugin + + use ElixirSense.Providers.Suggestion.GenericReducer + + alias ElixirSense.Core.Source + alias ElixirSense.Core.Binding + alias ElixirSense.Core.Introspection + alias ElixirSense.Core.ModuleStore + alias ElixirSense.Plugins.Phoenix.Scope + alias ElixirSense.Plugins.Util + alias ElixirSense.Providers.Suggestion.Matcher + + @phoenix_route_funcs ~w( + get put patch trace + delete head options + forward connect post + )a + + @impl true + def setup(context) do + ModuleStore.ensure_compiled(context, Phoenix.Router) + end + + if Version.match?(System.version(), ">= 1.14.0") do + @impl true + def suggestions(hint, {Phoenix.Router, func, 1, _info}, _list, opts) + when func in @phoenix_route_funcs do + binding = Binding.from_env(opts.env, opts.buffer_metadata) + {_, scope_alias} = Scope.within_scope(opts.cursor_context.text_before, binding) + + case find_controllers(opts.module_store, opts.env, hint, scope_alias) do + [] -> :ignore + controllers -> {:override, controllers} + end + end + + def suggestions( + hint, + {Phoenix.Router, func, 2, %{params: [_path, module]}}, + _list, + opts + ) + when func in @phoenix_route_funcs do + binding_env = Binding.from_env(opts.env, opts.buffer_metadata) + {_, scope_alias} = Scope.within_scope(opts.cursor_context.text_before) + {module, _} = Source.get_mod([module], binding_env) + + module = Module.safe_concat(scope_alias, module) + + suggestions = + for {export, {2, :function}} when export not in ~w(action call)a <- + Introspection.get_exports(module), + name = inspect(export), + Matcher.match?(name, hint) do + %{ + type: :generic, + kind: :function, + label: name, + insert_text: Util.trim_leading_for_insertion(hint, name), + detail: "Phoenix action" + } + end + + {:override, suggestions} + end + end + + @impl true + def suggestions(_hint, _func_call, _list, _opts) do + :ignore + end + + defp find_controllers(module_store, env, hint, scope_alias) do + [prefix | _] = + env.module + |> inspect() + |> String.split(".") + + for module <- module_store.list, + mod_str = inspect(module), + Util.match_module?(mod_str, prefix), + mod_str =~ "Controller", + Util.match_module?(mod_str, hint) do + {doc, _} = Introspection.get_module_docs_summary(module) + + %{ + type: :generic, + kind: :class, + label: mod_str, + insert_text: skip_scope_alias(scope_alias, mod_str), + detail: "Phoenix controller", + documentation: doc + } + end + |> Enum.sort_by(& &1.label) + end + + defp skip_scope_alias(nil, insert_text), do: insert_text + + defp skip_scope_alias(scope_alias, insert_text), + do: String.replace_prefix(insert_text, "#{inspect(scope_alias)}.", "") +end diff --git a/lib/elixir_sense/plugins/phoenix/scope.ex b/lib/elixir_sense/plugins/phoenix/scope.ex new file mode 100644 index 00000000..9a4f918a --- /dev/null +++ b/lib/elixir_sense/plugins/phoenix/scope.ex @@ -0,0 +1,110 @@ +defmodule ElixirSense.Plugins.Phoenix.Scope do + @moduledoc false + + alias ElixirSense.Core.Source + alias ElixirSense.Core.Binding + + import Module, only: [safe_concat: 2, safe_concat: 1] + + def within_scope(buffer, binding_env \\ %Binding{}) do + {:ok, ast} = Code.Fragment.container_cursor_to_quoted(buffer) + + with {true, scopes_ast} <- get_scopes(ast), + scopes_ast = Enum.reverse(scopes_ast), + scope_alias <- get_scope_alias(scopes_ast, binding_env) do + {true, scope_alias} + end + end + + defp get_scopes(ast) do + path = Macro.path(ast, &match?({:__cursor__, _, _}, &1)) + + scopes = + path + |> Enum.filter(&match?({:scope, _, _}, &1)) + |> Enum.map(fn {:scope, meta, params} -> + params = Enum.reject(params, &match?([{:do, _} | _], &1)) + {:scope, meta, params} + end) + + case scopes do + [] -> {false, nil} + scopes -> {true, scopes} + end + end + + defp get_scope_alias(scopes_ast, binding_env, module \\ nil) + + # is this possible? scope do ... end + defp get_scope_alias([{:scope, _, []}], _binding_env, module), do: module + + # scope "/" do ... end + defp get_scope_alias([{:scope, _, [scope_params]}], _binding_env, module) + when not is_list(scope_params), + do: module + + # scope path: "/", alias: ExampleWeb do ... end + defp get_scope_alias([{:scope, _, [scope_params]}], binding_env, module) do + scope_alias = Keyword.get(scope_params, :alias) + scope_alias = get_mod(scope_alias, binding_env) + safe_concat(module, scope_alias) + end + + # scope "/", alias: ExampleWeb do ... end + defp get_scope_alias( + [{:scope, _, [_scope_path, scope_params]}], + binding_env, + module + ) + when is_list(scope_params) do + scope_alias = Keyword.get(scope_params, :alias) + scope_alias = get_mod(scope_alias, binding_env) + safe_concat(module, scope_alias) + end + + # scope "/", ExampleWeb do ... end + defp get_scope_alias( + [{:scope, _, [_scope_path, scope_alias]}], + binding_env, + module + ) do + scope_alias = get_mod(scope_alias, binding_env) + safe_concat(module, scope_alias) + end + + # scope "/", ExampleWeb, host: "api." do ... end + defp get_scope_alias( + [{:scope, _, [_scope_path, scope_alias, _scope_params]}], + binding_env, + module + ) do + scope_alias = get_mod(scope_alias, binding_env) + safe_concat(module, scope_alias) + end + + # recurse + defp get_scope_alias([head | tail], binding_env, module) do + scope_alias = get_scope_alias([head], binding_env, module) + safe_concat([module, scope_alias, get_scope_alias(tail, binding_env)]) + end + + defp get_mod({:__aliases__, _, [scope_alias]}, binding_env) do + get_mod(scope_alias, binding_env) + end + + defp get_mod({name, _, nil}, binding_env) when is_atom(name) do + case Binding.expand(binding_env, {:variable, name}) do + {:atom, atom} -> + atom + + _ -> + nil + end + end + + defp get_mod(scope_alias, binding_env) do + with {mod, _} <- Source.get_mod([scope_alias], binding_env) do + mod + end + end +end diff --git a/lib/elixir_sense/providers/definition.ex b/lib/elixir_sense/providers/definition.ex index c5a192d8..753de77e 100644 --- a/lib/elixir_sense/providers/definition.ex +++ b/lib/elixir_sense/providers/definition.ex @@ -13,8 +13,10 @@ defmodule ElixirSense.Providers.Definition do alias ElixirSense.Core.State.ModFunInfo alias ElixirSense.Core.State.TypeInfo alias ElixirSense.Core.State.VarInfo + alias ElixirSense.Core.Source alias ElixirSense.Core.SurroundContext alias ElixirSense.Location + alias ElixirSense.Plugins.Phoenix.Scope @doc """ Finds out where a module, function, macro or variable was defined. @@ -174,14 +176,7 @@ defmodule ElixirSense.Providers.Definition do scope: scope } = env - m = - case module do - {:atom, a} -> - a - - _ -> - nil - end + m = get_module(module, context, env, metadata) case {m, function} |> Introspection.actual_mod_fun( @@ -249,4 +244,23 @@ defmodule ElixirSense.Providers.Definition do end end end + + defp get_module(module, %{end: {line, col}}, env, metadata) do + with {true, module} <- get_phoenix_module(module, env) do + text_before = Source.text_before(metadata.source, line, col) + + case Scope.within_scope(text_before) do + {false, _} -> module + {true, scope_alias} -> Module.safe_concat(scope_alias, module) + end + end + end + + defp get_phoenix_module(module, env) do + case {Phoenix.Router in env.requires, module} do + {true, {:atom, module}} -> {true, module} + {false, {:atom, module}} -> module + _ -> nil + end + end end diff --git a/test/elixir_sense/definition_test.exs b/test/elixir_sense/definition_test.exs index 386ac3b6..e0df9f42 100644 --- a/test/elixir_sense/definition_test.exs +++ b/test/elixir_sense/definition_test.exs @@ -14,6 +14,27 @@ defmodule ElixirSense.Providers.DefinitionTest do assert nil == ElixirSense.definition("__MODULE__", 1, 1) end + @tag requires_elixir_1_14: true + test "find module definition inside Phoenix's scope" do + _define_existing_atom = ExampleWeb + + buffer = """ + defmodule ExampleWeb.Router do + import Phoenix.Router + + scope "/", ExampleWeb do + get "/", PageController, :home + end + end + """ + + %Location{type: :module, file: file, line: line, column: column} = + ElixirSense.definition(buffer, 5, 15) + + assert file =~ "elixir_sense/test/support/plugins/phoenix/page_controller.ex" + assert read_line(file, {line, column}) =~ "ExampleWeb.PageController" + end + test "find definition of aliased modules in `use`" do buffer = """ defmodule MyModule do diff --git a/test/elixir_sense/plugins/phoenix/scope_test.exs b/test/elixir_sense/plugins/phoenix/scope_test.exs new file mode 100644 index 00000000..88ca7c51 --- /dev/null +++ b/test/elixir_sense/plugins/phoenix/scope_test.exs @@ -0,0 +1,116 @@ +defmodule ElixirSense.Plugins.Phoenix.ScopeTest do + use ExUnit.Case + alias ElixirSense.Core.Binding + alias ElixirSense.Plugins.Phoenix.Scope + + @moduletag requires_elixir_1_14: true + describe "within_scope/1" do + test "returns true and nil alias" do + buffer = """ + scope "/" do + get "/", + """ + + assert {true, nil} = Scope.within_scope(buffer) + end + + test "returns true and alias when passing alias as option" do + buffer = """ + scope "/", alias: ExampleWeb do + get "/", + """ + + assert {true, ExampleWeb} = Scope.within_scope(buffer) + end + + test "returns true and alias when passing alias as second parameter" do + buffer = """ + scope "/", ExampleWeb do + get "/", + """ + + assert {true, ExampleWeb} = Scope.within_scope(buffer) + end + + test "returns true and alias when nested within other scopes" do + _define_existing_atom = ExampleWeb.Admin + _define_existing_atom = Admin + + buffer = """ + scope "/", ExampleWeb do + scope "/admin", Admin do + get "/", + """ + + assert {true, ExampleWeb.Admin} = Scope.within_scope(buffer) + end + + test "can expand module attributes" do + buffer = """ + defmodule ExampleWeb.Router do + import Phoenix.Router + @web_prefix ExampleWweb + + scope "/", @web_prefix do + get "/", + """ + + binding = %Binding{ + structs: %{}, + variables: [], + attributes: [ + %ElixirSense.Core.State.AttributeInfo{ + name: :web_prefix, + positions: [{4, 5}], + type: {:atom, ExampleWeb} + } + ], + current_module: ExampleWeb.Router, + imports: [{Kernel, []}, {Phoenix.Router, []}], + specs: %{}, + types: %{}, + mods_funs: %{} + } + + assert {true, ExampleWeb} = Scope.within_scope(buffer, binding) + end + + test "can expand variables" do + buffer = """ + defmodule ExampleWeb.Router do + import Phoenix.Router + web_prefix = ExampleWweb + + scope "/", web_prefix do + get "/", + """ + + binding = %Binding{ + structs: %{}, + variables: [ + %ElixirSense.Core.State.VarInfo{ + name: :web_prefix, + positions: [{5, 5}], + scope_id: 2, + is_definition: true, + type: {:atom, ExampleWeb} + } + ], + attributes: [], + current_module: ExampleWeb.Router, + imports: [{Kernel, []}, {Phoenix.Router, []}], + specs: %{}, + types: %{}, + mods_funs: %{} + } + + assert {true, ExampleWeb} = Scope.within_scope(buffer, binding) + end + + test "returns false" do + buffer = "get \"\\\" ," + + assert {false, nil} = Scope.within_scope(buffer) + end + end +end diff --git a/test/elixir_sense/plugins/phoenix_test.exs b/test/elixir_sense/plugins/phoenix_test.exs new file mode 100644 index 00000000..b019b573 --- /dev/null +++ b/test/elixir_sense/plugins/phoenix_test.exs @@ -0,0 +1,130 @@ +defmodule ElixirSense.Plugins.PhoenixTest do + use ExUnit.Case + import TestHelper + + @moduletag requires_elixir_1_14: true + describe "suggestions/4" do + test "overrides with controllers for phoenix_route_funcs, when in the second parameter" do + buffer = """ + defmodule ExampleWeb.Router do + import Phoenix.Router + + get "/", P + # ^ + end + """ + + [cursor] = cursors(buffer) + + result = suggestions(buffer, cursor) + + assert [ + %{ + type: :generic, + kind: :class, + label: "ExampleWeb.PageController", + insert_text: "ExampleWeb.PageController", + detail: "Phoenix controller" + } + ] = result + end + + test "do not prepend alias defined within Phoenix `scope` functions" do + _define_existing_atom = ExampleWeb + + buffer = """ + defmodule ExampleWeb.Router do + import Phoenix.Router + + scope "/", ExampleWeb do + get "/", P + # ^ + end + end + """ + + [cursor] = cursors(buffer) + + result = suggestions(buffer, cursor) + + assert [ + %{ + type: :generic, + kind: :class, + label: "ExampleWeb.PageController", + insert_text: "PageController", + detail: "Phoenix controller" + } + ] = result + end + + test "overrides with action suggestions for phoenix_route_funcs, when in the third parameter" do + buffer = """ + defmodule ExampleWeb.Router do + import Phoenix.Router + + get "/", ExampleWeb.PageController, : + # ^ + end + """ + + [cursor] = cursors(buffer) + + result = suggestions(buffer, cursor) + + assert [ + %{ + detail: "Phoenix action", + insert_text: "home", + kind: :function, + label: ":home", + type: :generic + } + ] = result + end + + test "overrides with action suggestions even when inside scope" do + buffer = """ + defmodule ExampleWeb.Router do + import Phoenix.Router + + scope "/", ExampleWeb do + get "/", PageController, : + # ^ + end + end + """ + + [cursor] = cursors(buffer) + + result = suggestions(buffer, cursor) + + assert [ + %{ + detail: "Phoenix action", + insert_text: "home", + kind: :function, + label: ":home", + type: :generic + } + ] = result + end + + test "ignores for non-phoenix_route_funcs" do + buffer = """ + defmodule ExampleWeb.Router do + import Phoenix.Router + + something_else "/", P + # ^ + end + """ + + [cursor] = cursors(buffer) + + result = suggestions(buffer, cursor) + + refute Enum.find(result, &(&1[:detail] == "Phoenix controller")) + end + end +end diff --git a/test/support/plugins/phoenix/page_controller.ex b/test/support/plugins/phoenix/page_controller.ex new file mode 100644 index 00000000..3c70e506 --- /dev/null +++ b/test/support/plugins/phoenix/page_controller.ex @@ -0,0 +1,10 @@ +defmodule ExampleWeb.PageController do + def call(_conn, _params) do + end + + def action(_conn, _params) do + end + + def home(_conn, _params) do + end +end diff --git a/test/support/plugins/phoenix/router.ex b/test/support/plugins/phoenix/router.ex new file mode 100644 index 00000000..fa0d3064 --- /dev/null +++ b/test/support/plugins/phoenix/router.ex @@ -0,0 +1,4 @@ +defmodule Phoenix.Router do + def get(_route, _plug, _plut_opts, _opts \\ []) do + end +end