From eda7a604a883faa21a0925f9849d000e73760601 Mon Sep 17 00:00:00 2001 From: Eksperimental Date: Sun, 5 Oct 2025 17:03:43 -0500 Subject: [PATCH 1/3] Fix misleading warning when implementing a callback of undefined behaviour MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit There was a bug in the warning when we try to implement a callback of a behaviour that does not exist. Example: ```elixir defmodule BehaviourNotDefined do @behaviour FooBehaviour @impl FooBehaviour def foo(), do: :foo end ``` Will give us two warnings: ``` warning: @behaviour FooBehaviour does not exist (in module BehaviourNotDefined) │ 17 │ defmodule BehaviourNotDefined do │ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ │ └─ lib/behaviour_not_defined.ex:17: BehaviourNotDefined (module) warning: got "@impl FooBehaviour" for function foo/0 but this behaviour does not specify such callback. There are no known callbacks, please specify the proper @behaviour and make sure it defines callbacks │ 21 │ def foo(), do: :foo │ ~~~~~~~~~~~~~~~~~~~ │ └─ lib/behaviour_not_defined.ex:21: BehaviourNotDefined (module) ``` The first warning is correct. The behaviour does not exist. The second one is misleadingly telling us that the behaviour does not specify such callback, when the behaviour doesn't even exist. The internal atom for this error was `:behaviour_not_defined` but what was being checked was actually that the callback was not defined. So `:behaviour_not_defined` has been renamed as `:callback_not_defined`, while keeping the original message. And the missing detection for `:behaviour_not_defined` has been implemented. --- lib/elixir/lib/module/behaviour.ex | 30 ++++++++++++++++++++- lib/elixir/test/elixir/kernel/impl_test.exs | 5 ++-- 2 files changed, 32 insertions(+), 3 deletions(-) diff --git a/lib/elixir/lib/module/behaviour.ex b/lib/elixir/lib/module/behaviour.ex index f436596b756..c883123895b 100644 --- a/lib/elixir/lib/module/behaviour.ex +++ b/lib/elixir/lib/module/behaviour.ex @@ -181,8 +181,11 @@ defmodule Module.Behaviour do behaviour not in behaviours -> {:error, {:behaviour_not_declared, behaviour}} - true -> + not behaviour_defined?(callbacks, behaviour) -> {:error, {:behaviour_not_defined, behaviour, callbacks}} + + true -> + {:error, {:callback_not_defined, behaviour, callbacks}} end end @@ -215,6 +218,18 @@ defmodule Module.Behaviour do end end + # Determines whether there is at least one callback defined for the given behaviour. + # If not, that means that the behaviour has not been defined. + defp behaviour_defined?(callbacks, behaviour) do + callbacks + |> Map.values() + |> List.flatten() + |> Enum.any?(fn + {_kind, ^behaviour, _optional?} -> true + {_kind, _behaviour, _optional?} -> false + end) + end + defp warn_missing_impls(%{callbacks: callbacks} = context, _impl_contexts, _defs) when map_size(callbacks) == 0 do context @@ -391,6 +406,19 @@ defmodule Module.Behaviour do end defp format_warning({:behaviour_not_defined, callback, kind, behaviour, callbacks}) do + [ + "got \"@impl ", + inspect(behaviour), + "\" for ", + format_definition(kind, callback), + " but behaviour ", + inspect(behaviour), + " does not exist", + known_callbacks(callbacks) + ] + end + + defp format_warning({:callback_not_defined, callback, kind, behaviour, callbacks}) do [ "got \"@impl ", inspect(behaviour), diff --git a/lib/elixir/test/elixir/kernel/impl_test.exs b/lib/elixir/test/elixir/kernel/impl_test.exs index a998b6d8a96..e3fbb21187c 100644 --- a/lib/elixir/test/elixir/kernel/impl_test.exs +++ b/lib/elixir/test/elixir/kernel/impl_test.exs @@ -122,7 +122,7 @@ defmodule Kernel.ImplTest do end end - test "warns for undefined value" do + test "warns for undefined behaviour" do assert capture_err(fn -> Code.eval_string(""" defmodule Kernel.ImplTest.ImplAttributes do @@ -133,7 +133,8 @@ defmodule Kernel.ImplTest do end """) end) =~ - "got \"@impl :abc\" for function foo/0 but this behaviour does not specify such callback. There are no known callbacks" + "got \"@impl :abc\" for function foo/0 but behaviour :abc does not exist. " <> + "There are no known callbacks, please specify the proper @behaviour and make sure it defines callbacks" end test "warns for callbacks without impl and @impl has been set before" do From 1ed6eed6b4f94ab29721e00dd06653ec9bfa919c Mon Sep 17 00:00:00 2001 From: Eksperimental Date: Sun, 5 Oct 2025 20:23:41 -0500 Subject: [PATCH 2/3] Implement suggestions by @sabiwara --- lib/elixir/lib/module/behaviour.ex | 38 ++++++++++++++++----- lib/elixir/test/elixir/kernel/impl_test.exs | 33 ++++++++++++------ 2 files changed, 53 insertions(+), 18 deletions(-) diff --git a/lib/elixir/lib/module/behaviour.ex b/lib/elixir/lib/module/behaviour.ex index c883123895b..15e52d2f6ec 100644 --- a/lib/elixir/lib/module/behaviour.ex +++ b/lib/elixir/lib/module/behaviour.ex @@ -181,8 +181,11 @@ defmodule Module.Behaviour do behaviour not in behaviours -> {:error, {:behaviour_not_declared, behaviour}} + not Code.ensure_loaded?(behaviour) -> + {:error, {:module_not_defined, behaviour}} + not behaviour_defined?(callbacks, behaviour) -> - {:error, {:behaviour_not_defined, behaviour, callbacks}} + {:error, {:behaviour_not_defined, behaviour}} true -> {:error, {:callback_not_defined, behaviour, callbacks}} @@ -405,26 +408,45 @@ defmodule Module.Behaviour do ] end - defp format_warning({:behaviour_not_defined, callback, kind, behaviour, callbacks}) do + defp format_warning({:module_not_defined, callback, kind, behaviour}) do + behaviour_string = inspect(behaviour) + [ "got \"@impl ", - inspect(behaviour), + behaviour_string, + "\" for ", + format_definition(kind, callback), + " but module ", + behaviour_string, + " does not exist" + ] + end + + defp format_warning({:behaviour_not_defined, callback, kind, behaviour}) do + behaviour_string = inspect(behaviour) + + [ + "got \"@impl ", + behaviour_string, "\" for ", format_definition(kind, callback), " but behaviour ", - inspect(behaviour), - " does not exist", - known_callbacks(callbacks) + behaviour_string, + " does not exist" ] end defp format_warning({:callback_not_defined, callback, kind, behaviour, callbacks}) do + behaviour_string = inspect(behaviour) + [ "got \"@impl ", - inspect(behaviour), + behaviour_string, "\" for ", format_definition(kind, callback), - " but this behaviour does not specify such callback", + " but ", + behaviour_string, + " does not specify such callback", known_callbacks(callbacks) ] end diff --git a/lib/elixir/test/elixir/kernel/impl_test.exs b/lib/elixir/test/elixir/kernel/impl_test.exs index e3fbb21187c..13af85015d8 100644 --- a/lib/elixir/test/elixir/kernel/impl_test.exs +++ b/lib/elixir/test/elixir/kernel/impl_test.exs @@ -122,19 +122,32 @@ defmodule Kernel.ImplTest do end end - test "warns for undefined behaviour" do + test "warns about undefined module" do assert capture_err(fn -> Code.eval_string(""" defmodule Kernel.ImplTest.ImplAttributes do - @behaviour :abc + @behaviour Abc - @impl :abc + @impl Abc def foo(), do: :ok end """) end) =~ - "got \"@impl :abc\" for function foo/0 but behaviour :abc does not exist. " <> - "There are no known callbacks, please specify the proper @behaviour and make sure it defines callbacks" + "got \"@impl Abc\" for function foo/0 but module Abc does not exist" + end + + test "warns about undefined behaviour" do + assert capture_err(fn -> + Code.eval_string(""" + defmodule Kernel.ImplTest.ImplAttributes do + @behaviour Enum + + @impl Enum + def foo(), do: :ok + end + """) + end) =~ + "got \"@impl Enum\" for function foo/0 but behaviour Enum does not exist" end test "warns for callbacks without impl and @impl has been set before" do @@ -316,7 +329,7 @@ defmodule Kernel.ImplTest do end """) end) =~ - "got \"@impl Kernel.ImplTest.Behaviour\" for function bar/0 but this behaviour does not specify such callback" + "got \"@impl Kernel.ImplTest.Behaviour\" for function bar/0 but Kernel.ImplTest.Behaviour does not specify such callback" end test "warns for @impl module with macro callback name not in behaviour" do @@ -329,7 +342,7 @@ defmodule Kernel.ImplTest do end """) end) =~ - "got \"@impl Kernel.ImplTest.MacroBehaviour\" for macro foo/0 but this behaviour does not specify such callback" + "got \"@impl Kernel.ImplTest.MacroBehaviour\" for macro foo/0 but Kernel.ImplTest.MacroBehaviour does not specify such callback" end test "warns for @impl module with macro callback kind not in behaviour" do @@ -342,7 +355,7 @@ defmodule Kernel.ImplTest do end """) end) =~ - "got \"@impl Kernel.ImplTest.MacroBehaviour\" for function foo/0 but this behaviour does not specify such callback" + "got \"@impl Kernel.ImplTest.MacroBehaviour\" for function foo/0 but Kernel.ImplTest.MacroBehaviour does not specify such callback" end test "warns for @impl module and callback belongs to another known module" do @@ -356,7 +369,7 @@ defmodule Kernel.ImplTest do end """) end) =~ - "got \"@impl Kernel.ImplTest.Behaviour\" for function bar/0 but this behaviour does not specify such callback" + "got \"@impl Kernel.ImplTest.Behaviour\" for function bar/0 but Kernel.ImplTest.Behaviour does not specify such callback" end test "warns for @impl module and callback belongs to another unknown module" do @@ -510,7 +523,7 @@ defmodule Kernel.ImplTest do end """) end) =~ - "got \"@impl Kernel.ImplTest.MacroBehaviour\" for function foo/0 but this behaviour does not specify such callback" + "got \"@impl Kernel.ImplTest.MacroBehaviour\" for function foo/0 but Kernel.ImplTest.MacroBehaviour does not specify such callback" end test "warns only for non-generated functions in non-generated @impl" do From 1bf78417146ff7a89ee68f9a21931aa01aefa62a Mon Sep 17 00:00:00 2001 From: Eksperimental Date: Mon, 6 Oct 2025 07:25:08 -0500 Subject: [PATCH 3/3] Implement suggestions by @josevalim --- lib/elixir/lib/module/behaviour.ex | 34 ++----------- lib/elixir/test/elixir/kernel/impl_test.exs | 56 +++++++++++++-------- 2 files changed, 38 insertions(+), 52 deletions(-) diff --git a/lib/elixir/lib/module/behaviour.ex b/lib/elixir/lib/module/behaviour.ex index 15e52d2f6ec..42e76baf22a 100644 --- a/lib/elixir/lib/module/behaviour.ex +++ b/lib/elixir/lib/module/behaviour.ex @@ -182,10 +182,12 @@ defmodule Module.Behaviour do {:error, {:behaviour_not_declared, behaviour}} not Code.ensure_loaded?(behaviour) -> - {:error, {:module_not_defined, behaviour}} + # Module does not exist, but we have already warned about it. + {:ok, []} not behaviour_defined?(callbacks, behaviour) -> - {:error, {:behaviour_not_defined, behaviour}} + # Module does not define behaviour, but we have already warned about it. + {:ok, []} true -> {:error, {:callback_not_defined, behaviour, callbacks}} @@ -408,34 +410,6 @@ defmodule Module.Behaviour do ] end - defp format_warning({:module_not_defined, callback, kind, behaviour}) do - behaviour_string = inspect(behaviour) - - [ - "got \"@impl ", - behaviour_string, - "\" for ", - format_definition(kind, callback), - " but module ", - behaviour_string, - " does not exist" - ] - end - - defp format_warning({:behaviour_not_defined, callback, kind, behaviour}) do - behaviour_string = inspect(behaviour) - - [ - "got \"@impl ", - behaviour_string, - "\" for ", - format_definition(kind, callback), - " but behaviour ", - behaviour_string, - " does not exist" - ] - end - defp format_warning({:callback_not_defined, callback, kind, behaviour, callbacks}) do behaviour_string = inspect(behaviour) diff --git a/lib/elixir/test/elixir/kernel/impl_test.exs b/lib/elixir/test/elixir/kernel/impl_test.exs index 13af85015d8..617f1a77eb5 100644 --- a/lib/elixir/test/elixir/kernel/impl_test.exs +++ b/lib/elixir/test/elixir/kernel/impl_test.exs @@ -122,32 +122,44 @@ defmodule Kernel.ImplTest do end end - test "warns about undefined module" do - assert capture_err(fn -> - Code.eval_string(""" - defmodule Kernel.ImplTest.ImplAttributes do - @behaviour Abc + test "warns about undefined module, but does not warn at @impl line" do + capture_err = + capture_err(fn -> + Code.eval_string(""" + defmodule Kernel.ImplTest.ImplAttributes do + @behaviour Abc - @impl Abc - def foo(), do: :ok - end - """) - end) =~ - "got \"@impl Abc\" for function foo/0 but module Abc does not exist" + @impl Abc + def foo(), do: :ok + end + """) + end) + + assert capture_err =~ + "@behaviour Abc does not exist (in module Kernel.ImplTest.ImplAttributes)" + + refute capture_err =~ + "got \"@impl Abc\"" end - test "warns about undefined behaviour" do - assert capture_err(fn -> - Code.eval_string(""" - defmodule Kernel.ImplTest.ImplAttributes do - @behaviour Enum + test "warns about undefined behaviour, but does not warn at @impl line" do + capture_err = + capture_err(fn -> + Code.eval_string(""" + defmodule Kernel.ImplTest.ImplAttributes do + @behaviour Enum - @impl Enum - def foo(), do: :ok - end - """) - end) =~ - "got \"@impl Enum\" for function foo/0 but behaviour Enum does not exist" + @impl Enum + def foo(), do: :ok + end + """) + end) + + assert capture_err =~ + "module Enum is not a behaviour (in module Kernel.ImplTest.ImplAttributes)" + + refute capture_err =~ + "got \"@impl Abc\"" end test "warns for callbacks without impl and @impl has been set before" do