From 444eab641aca0a568f66c68f5f6a9dfda290faea Mon Sep 17 00:00:00 2001 From: Dave Lucia Date: Sat, 5 Aug 2023 20:16:24 -0400 Subject: [PATCH 1/7] Improve UndefinedFunctionError for mispelled module This is a proof of concept to see if we can improve error messages when a module name is mispelled or otherwise incorrect. This can happen when a user misspells a module, or does not fully qualify it. Ideally, the elixir compiler can provide suggestions for what they meant. If we were to merge a version of this PR, we would get error messages like ```elixir iex(1)> Enu.map([1, 2, 3], & &1 + 1) ** (UndefinedFunctionError) function Enu.map/2 is undefined (module Enu is not available). Did you mean: * Enum.map/2 Enu.map([1, 2, 3], #Function<42.125776118/1 in :erl_eval.expr/6>) iex:1: (file) ``` ```elixir iex(1)> defmodule Namespace.SomeModule do ...(1)> def foo, do: :bar ...(1)> end {:module, Namespace.SomeModule, <<70, 79, 82, 49, 0, 0, 5, 36, 66, 69, 65, 77, 65, 116, 85, 56, 0, 0, 0, 186, 0, 0, 0, 18, 27, 69, 108, 105, 120, 105, 114, 46, 78, 97, 109, 101, 115, 112, 97, 99, 101, 46, 83, 111, 109, 101, 77, ...>>, {:foo, 0}} iex(2)> SomeModule.foo() ** (UndefinedFunctionError) function SomeModule.foo/0 is undefined (module SomeModule is not available). Did you mean: * Namespace.SomeModule.foo/0 SomeModule.foo() iex:2: (file) ``` I saw two experienced developers who are new two Elixir get stuck on this issue on the same day, not including a mention on Twitter! [Twitter Conversation](https://twitter.com/davydog187/status/1687628454240428034?s=20) - [ ] Better heuristic that is accurate for most usecases - [ ] Consider other edge cases and measure results (forgetten alias?) - [ ] Possibly turn off outside of dev? - [ ] Remove `to_atom/1` - [ ] Write more tests * https://groups.google.com/g/elixir-lang-core/c/-IHneszMheI/m/cCCxHB9PBwAJ * https://github.com/elixir-lang/elixir/pull/5665 --- lib/elixir/lib/exception.ex | 17 +++++++++++++++-- lib/elixir/test/elixir/exception_test.exs | 8 ++++++++ 2 files changed, 23 insertions(+), 2 deletions(-) diff --git a/lib/elixir/lib/exception.ex b/lib/elixir/lib/exception.ex index 76d412b0801..c863d000b50 100644 --- a/lib/elixir/lib/exception.ex +++ b/lib/elixir/lib/exception.ex @@ -1342,8 +1342,21 @@ defmodule UndefinedFunctionError do hint_for_loaded_module(module, function, arity, nil) end - defp hint(_module, _function, _arity, _loaded?) do - "" + defp hint(module, function, arity, _loaded?) do + {module, _distance} = + Enum.map(:code.all_available(), fn {name, _, _} -> + {name, String.jaro_distance(to_string(module), to_string(name))} + end) + |> Enum.sort_by(&elem(&1, 1), :desc) + |> Enum.take(3) + |> Enum.find(fn {module, _distance} -> + module + |> to_string() + |> String.to_atom() + |> function_exported?(function, arity) + end) + + ". Did you mean:\n\n * #{Exception.format_mfa(String.to_existing_atom(to_string(module)), function, arity)}\n" end @doc false diff --git a/lib/elixir/test/elixir/exception_test.exs b/lib/elixir/test/elixir/exception_test.exs index 030b68cce02..9f2a73399eb 100644 --- a/lib/elixir/test/elixir/exception_test.exs +++ b/lib/elixir/test/elixir/exception_test.exs @@ -595,6 +595,14 @@ defmodule ExceptionTest do assert message =~ "* set_cookie/2" end + test "annotates undefined function error with module suggestions" do + assert blame_message(Enu, & &1.map(&1, 1)) == """ + function Enu.map/2 is undefined (module Enu is not available). Did you mean: + + * Enum.map/2 + """ + end + test "annotates undefined function clause error with macro hints" do assert blame_message(Integer, & &1.is_odd(1)) == "function Integer.is_odd/1 is undefined or private. However, there is " <> From e5e149940d41c953594c7830d2d7ab83e431e792 Mon Sep 17 00:00:00 2001 From: Dave Lucia Date: Wed, 9 Aug 2023 09:36:38 -0400 Subject: [PATCH 2/7] only handle miscased edge case --- lib/elixir/lib/exception.ex | 42 ++++++++++++++++------- lib/elixir/test/elixir/exception_test.exs | 7 ++-- 2 files changed, 35 insertions(+), 14 deletions(-) diff --git a/lib/elixir/lib/exception.ex b/lib/elixir/lib/exception.ex index c863d000b50..6c3fc27bc4c 100644 --- a/lib/elixir/lib/exception.ex +++ b/lib/elixir/lib/exception.ex @@ -1343,20 +1343,38 @@ defmodule UndefinedFunctionError do end defp hint(module, function, arity, _loaded?) do - {module, _distance} = - Enum.map(:code.all_available(), fn {name, _, _} -> - {name, String.jaro_distance(to_string(module), to_string(name))} - end) - |> Enum.sort_by(&elem(&1, 1), :desc) - |> Enum.take(3) - |> Enum.find(fn {module, _distance} -> - module - |> to_string() - |> String.to_atom() - |> function_exported?(function, arity) + normalized_module = normalized_module_name(module) + + candidate = + :code.all_available() + |> Enum.find(fn {name, _, _} -> + normalized_module_name(name) == normalized_module end) - ". Did you mean:\n\n * #{Exception.format_mfa(String.to_existing_atom(to_string(module)), function, arity)}\n" + with {_, _, _} <- candidate, + {:module, module} <- load_module_filename(candidate), + true <- function_exported?(module, function, arity) do + ". Did you mean:\n\n * #{Exception.format_mfa(module, function, arity)}\n" + else + _ -> "" + end + end + + defp load_module_filename({name, path, loaded?}) do + if loaded? do + {:module, name |> to_string() |> String.to_existing_atom()} + else + path + |> Path.rootname(".beam") + |> String.to_charlist() + |> :code.load_abs() + end + end + + defp normalized_module_name(module) do + module + |> to_string() + |> String.downcase() end @doc false diff --git a/lib/elixir/test/elixir/exception_test.exs b/lib/elixir/test/elixir/exception_test.exs index 9f2a73399eb..364ba95f249 100644 --- a/lib/elixir/test/elixir/exception_test.exs +++ b/lib/elixir/test/elixir/exception_test.exs @@ -596,11 +596,14 @@ defmodule ExceptionTest do end test "annotates undefined function error with module suggestions" do - assert blame_message(Enu, & &1.map(&1, 1)) == """ - function Enu.map/2 is undefined (module Enu is not available). Did you mean: + assert blame_message(ENUM, & &1.map(&1, 1)) == """ + function ENUM.map/2 is undefined (module ENUM is not available). Did you mean: * Enum.map/2 """ + + assert blame_message(ENUM, & &1.not_a_function(&1, 1)) == + "function ENUM.not_a_function/2 is undefined (module ENUM is not available)" end test "annotates undefined function clause error with macro hints" do From af85b5c726d36fc5aa261de4a166a8a018512613 Mon Sep 17 00:00:00 2001 From: Dave Lucia Date: Wed, 9 Aug 2023 10:09:58 -0400 Subject: [PATCH 3/7] downcase, simplifly loading --- lib/elixir/lib/exception.ex | 20 ++++++++------------ 1 file changed, 8 insertions(+), 12 deletions(-) diff --git a/lib/elixir/lib/exception.ex b/lib/elixir/lib/exception.ex index 6c3fc27bc4c..a16e723f393 100644 --- a/lib/elixir/lib/exception.ex +++ b/lib/elixir/lib/exception.ex @@ -1343,12 +1343,12 @@ defmodule UndefinedFunctionError do end defp hint(module, function, arity, _loaded?) do - normalized_module = normalized_module_name(module) + downcased_module = downcase_module_name(module) candidate = :code.all_available() |> Enum.find(fn {name, _, _} -> - normalized_module_name(name) == normalized_module + downcase_module_name(name) == downcased_module end) with {_, _, _} <- candidate, @@ -1360,18 +1360,14 @@ defmodule UndefinedFunctionError do end end - defp load_module_filename({name, path, loaded?}) do - if loaded? do - {:module, name |> to_string() |> String.to_existing_atom()} - else - path - |> Path.rootname(".beam") - |> String.to_charlist() - |> :code.load_abs() - end + defp load_module_filename({name, _path, _loaded?}) do + name + |> to_string() + |> String.to_atom() + |> Code.ensure_loaded() end - defp normalized_module_name(module) do + defp downcase_module_name(module) do module |> to_string() |> String.downcase() From 3dd9c6041aa8dc438b75a0d010318e7b63e36a29 Mon Sep 17 00:00:00 2001 From: Dave Lucia Date: Wed, 9 Aug 2023 10:10:49 -0400 Subject: [PATCH 4/7] ascii --- 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 a16e723f393..a6b090e313c 100644 --- a/lib/elixir/lib/exception.ex +++ b/lib/elixir/lib/exception.ex @@ -1370,7 +1370,7 @@ defmodule UndefinedFunctionError do defp downcase_module_name(module) do module |> to_string() - |> String.downcase() + |> String.downcase(:ascii) end @doc false From 235801be0ce53106eb48fd0d1a73b610132850d8 Mon Sep 17 00:00:00 2001 From: Dave Lucia Date: Wed, 9 Aug 2023 10:12:28 -0400 Subject: [PATCH 5/7] load_module_filename -> load_module --- lib/elixir/lib/exception.ex | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/elixir/lib/exception.ex b/lib/elixir/lib/exception.ex index a6b090e313c..f88990c821c 100644 --- a/lib/elixir/lib/exception.ex +++ b/lib/elixir/lib/exception.ex @@ -1352,7 +1352,7 @@ defmodule UndefinedFunctionError do end) with {_, _, _} <- candidate, - {:module, module} <- load_module_filename(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" else @@ -1360,7 +1360,7 @@ defmodule UndefinedFunctionError do end end - defp load_module_filename({name, _path, _loaded?}) do + defp load_module({name, _path, _loaded?}) do name |> to_string() |> String.to_atom() From 0fe444fdf51703419e5d866319748bb4990cdbf0 Mon Sep 17 00:00:00 2001 From: Dave Lucia Date: Wed, 9 Aug 2023 10:18:58 -0400 Subject: [PATCH 6/7] optimize atom creation --- lib/elixir/lib/exception.ex | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/lib/elixir/lib/exception.ex b/lib/elixir/lib/exception.ex index f88990c821c..f8cdddfa5a4 100644 --- a/lib/elixir/lib/exception.ex +++ b/lib/elixir/lib/exception.ex @@ -1362,8 +1362,7 @@ defmodule UndefinedFunctionError do defp load_module({name, _path, _loaded?}) do name - |> to_string() - |> String.to_atom() + |> List.to_atom() |> Code.ensure_loaded() end From 7ea6e81b0585daab4e73efa139a96bae468deb39 Mon Sep 17 00:00:00 2001 From: Andrea Leopardi Date: Wed, 9 Aug 2023 16:51:30 +0200 Subject: [PATCH 7/7] Update lib/elixir/lib/exception.ex --- lib/elixir/lib/exception.ex | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/lib/elixir/lib/exception.ex b/lib/elixir/lib/exception.ex index f8cdddfa5a4..9ab6352bc81 100644 --- a/lib/elixir/lib/exception.ex +++ b/lib/elixir/lib/exception.ex @@ -1346,8 +1346,7 @@ defmodule UndefinedFunctionError do downcased_module = downcase_module_name(module) candidate = - :code.all_available() - |> Enum.find(fn {name, _, _} -> + Enum.find(:code.all_available(), fn {name, _, _} -> downcase_module_name(name) == downcased_module end)