From 134f3208c3538cf861225b53814409fadd854de8 Mon Sep 17 00:00:00 2001 From: Dave Lucia Date: Mon, 14 Aug 2023 09:32:24 -0400 Subject: [PATCH 1/3] Improve UndefinedFunctionError for unqualified module This PR builds off of #12839, handling the case where a developer forgets to alias a module, resulting in UndefinedFunctionError. Imagine we have the following modules in our Elixir program. ```elixir def MyAppWeb.Context.Event do def foo, do: :bar end ``` If the developer attempts to reference this module, but forgets to type the alias, they will now get module suggestions like. ```elixir ** (UndefinedFunctionError) function Event.foo/0 is undefined (module Event is not available). Did you mean: * MyAppWeb.Context.Event.foo/0 Event.foo() iex:2: (file) ``` --- lib/elixir/lib/exception.ex | 30 ++++++++++++++++------ lib/elixir/test/elixir/exception_test.exs | 31 +++++++++++++++++++++++ 2 files changed, 53 insertions(+), 8 deletions(-) diff --git a/lib/elixir/lib/exception.ex b/lib/elixir/lib/exception.ex index 9ab6352bc81..5caadd3d9f4 100644 --- a/lib/elixir/lib/exception.ex +++ b/lib/elixir/lib/exception.ex @@ -1345,17 +1345,25 @@ defmodule UndefinedFunctionError do defp hint(module, function, arity, _loaded?) do downcased_module = downcase_module_name(module) - candidate = - Enum.find(:code.all_available(), fn {name, _, _} -> - downcase_module_name(name) == downcased_module + candidates = + Enum.filter(:code.all_available(), fn {name, _, _} = candidate -> + if downcase_module_name(name) == downcased_module or + String.ends_with?(to_string(name), strip_elixir_prefix(module)) do + with {:module, module} <- load_module(candidate), + true <- function_exported?(module, function, arity) do + true + else + _ -> false + end + else + false + end end) - with {_, _, _} <- candidate, - {:module, module} <- load_module(candidate), - true <- function_exported?(module, function, arity) do - ". Did you mean:\n\n * #{Exception.format_mfa(module, function, arity)}\n" + if candidates != [] do + ". Did you mean:\n#{Enum.map(candidates, fn {module, _, _} -> "\n * #{Exception.format_mfa(String.to_atom(to_string(module)), function, arity)}" end)}\n" else - _ -> "" + "" end end @@ -1365,6 +1373,12 @@ defmodule UndefinedFunctionError do |> Code.ensure_loaded() end + defp strip_elixir_prefix(module) do + module + |> to_string() + |> String.replace_leading("Elixir.", "") + end + defp downcase_module_name(module) do module |> to_string() diff --git a/lib/elixir/test/elixir/exception_test.exs b/lib/elixir/test/elixir/exception_test.exs index 364ba95f249..6084499cf58 100644 --- a/lib/elixir/test/elixir/exception_test.exs +++ b/lib/elixir/test/elixir/exception_test.exs @@ -596,6 +596,25 @@ defmodule ExceptionTest do end test "annotates undefined function error with module suggestions" do + import PathHelpers + + modules = [ + Namespace.A.One, + Namespace.A.Two, + Namespace.A.Three, + Namespace.B.One, + Namespace.B.Two, + Namespace.B.Three + ] + + for module <- modules do + write_beam( + defmodule module do + def foo, do: :bar + end + ) + end + assert blame_message(ENUM, & &1.map(&1, 1)) == """ function ENUM.map/2 is undefined (module ENUM is not available). Did you mean: @@ -604,6 +623,18 @@ defmodule ExceptionTest do assert blame_message(ENUM, & &1.not_a_function(&1, 1)) == "function ENUM.not_a_function/2 is undefined (module ENUM is not available)" + + assert blame_message(One, & &1.foo()) == """ + function One.foo/0 is undefined (module One is not available). Did you mean: + + * Namespace.A.One.foo/0 + * Namespace.B.One.foo/0 + """ + + for module <- modules do + :code.delete(module) + :code.purge(module) + end end test "annotates undefined function clause error with macro hints" do From 26c6ea38244d584e73987d0bfd5973d4183847fa Mon Sep 17 00:00:00 2001 From: Dave Lucia Date: Mon, 14 Aug 2023 20:00:46 -0400 Subject: [PATCH 2/3] clean up code --- lib/elixir/lib/exception.ex | 37 +++++++++++++++++-------------------- 1 file changed, 17 insertions(+), 20 deletions(-) diff --git a/lib/elixir/lib/exception.ex b/lib/elixir/lib/exception.ex index 5caadd3d9f4..cd6b6e3c29f 100644 --- a/lib/elixir/lib/exception.ex +++ b/lib/elixir/lib/exception.ex @@ -1342,26 +1342,29 @@ defmodule UndefinedFunctionError do hint_for_loaded_module(module, function, arity, nil) end + @max_suggestions 10 defp hint(module, function, arity, _loaded?) do downcased_module = downcase_module_name(module) + stripped_module = module |> Atom.to_string() |> String.replace_leading("Elixir.", "") candidates = - Enum.filter(:code.all_available(), fn {name, _, _} = candidate -> - if downcase_module_name(name) == downcased_module or - String.ends_with?(to_string(name), strip_elixir_prefix(module)) do - with {:module, module} <- load_module(candidate), - true <- function_exported?(module, function, arity) do - true - else - _ -> false - end - else - false - end - end) + for {name, _, _} = candidate <- :code.all_available(), + downcase_module_name(name) == downcased_module or + String.ends_with?(List.to_string(name), stripped_module), + {:module, module} <- [load_module(candidate)], + function_exported?(module, function, arity), + do: module if candidates != [] do - ". Did you mean:\n#{Enum.map(candidates, fn {module, _, _} -> "\n * #{Exception.format_mfa(String.to_atom(to_string(module)), function, arity)}" end)}\n" + suggestions = + candidates + |> Enum.take(@max_suggestions) + |> Enum.sort(:asc) + |> Enum.map(fn module -> + ["\n * ", Exception.format_mfa(module, function, arity)] + end) + + IO.iodata_to_binary([". Did you mean:\n", suggestions, "\n"]) else "" end @@ -1373,12 +1376,6 @@ defmodule UndefinedFunctionError do |> Code.ensure_loaded() end - defp strip_elixir_prefix(module) do - module - |> to_string() - |> String.replace_leading("Elixir.", "") - end - defp downcase_module_name(module) do module |> to_string() From 1ab148fd22b02b3181466fd9b3e9a8daf88543bc Mon Sep 17 00:00:00 2001 From: Dave Lucia Date: Mon, 14 Aug 2023 20:27:24 -0400 Subject: [PATCH 3/3] 5 max suggestions --- lib/elixir/lib/exception.ex | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/elixir/lib/exception.ex b/lib/elixir/lib/exception.ex index cd6b6e3c29f..3ca79087d21 100644 --- a/lib/elixir/lib/exception.ex +++ b/lib/elixir/lib/exception.ex @@ -1342,7 +1342,7 @@ defmodule UndefinedFunctionError do hint_for_loaded_module(module, function, arity, nil) end - @max_suggestions 10 + @max_suggestions 5 defp hint(module, function, arity, _loaded?) do downcased_module = downcase_module_name(module) stripped_module = module |> Atom.to_string() |> String.replace_leading("Elixir.", "")