-
Notifications
You must be signed in to change notification settings - Fork 3.5k
Fix misleading warning when implementing a callback of undefined behaviour #14814
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix misleading warning when implementing a callback of undefined behaviour #14814
Conversation
…viour 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.
6e40b9e
to
eda7a60
Compare
I think the - 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
+ got "@impl FooBehaviour" for function foo/0 but FooBehaviour does not specify such callback. There are no known callbacks, please specify the proper @behaviour and make sure it defines callbacks |
I think that your suggestion is an improvement to the error message, but it does not solve the issue here. There are times where you will get a list of defined callbacks, and the issue will still remain, for example: defmodule BehaviourNotDefined2 do
@behaviour GenServer
@behaviour FooBehaviour
@impl GenServer
def init(_init_arg), do: {:ok, []}
@impl FooBehaviour
def foo(), do: :foo
end You will get these warnings:
The key point is mentioning that the behaviour does not exist, not that the behaviour does not implement that callback in particular. |
Oh OK, I didn't consider the case we have multiple callbacks. Makes sense to me! |
One thing that I noticed is that we have maybe too many different errors. Maybe we should reconsider unifying them.
|
@sabiwara I have implemented your suggestions |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
@josevalim I have already implemented your suggestions. So now the following modules will only give the following warnings: defmodule BehaviourNotDefined do
@behaviour FooBehaviour
@impl FooBehaviour
def foo(), do: :foo
end
defmodule BehaviourNotDefined2 do
@behaviour GenServer
@behaviour FooBehaviour
@impl GenServer
def init(_init_arg), do: {:ok, []}
@impl FooBehaviour
def foo(), do: :foo
end
defmodule ModuleDoesNotDefineBehaviour do
@behaviour Enum
@impl Enum
def foo(), do: :foo
end
|
💚 💙 💜 💛 ❤️ |
There was a bug in the warning when we try to implement a callback of a behaviour that does not exist.
Example:
Will give us two warnings:
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.