Hi José, I really like the idea of changing the behaviour of defoverridable, although I would rather go with the check on the implementation to raise warnings as the default instead of setting @impl true.
Em quinta-feira, 19 de janeiro de 2017 09:52:42 UTC-2, José Valim escreveu: > > Hi everyone, > > One of the features added to Elixir early on to help integration with > Erlang code was the idea of overridable function definitions. This is what > allowed our GenServer definition to be as simple as: > > defmodule MyServer do > use GenServerend > > Implementation-wise, use GenServer defines functions such as: > > def terminate(reason, state) do > :okend > > and then mark them as overridable: > > defoverridable terminate: 2 > > As the community grew, defoverridable/1 started to show some flaws in its > implementation. Furthermore, the community did not always follow up on best > practices, often times marking functions as overridable but without > defining a proper Behaviour behind the scenes. > > The goal of this proposal is to clarify the existing functionality and > propose extensions that will push the community towards best practices. > Using @optional_callbacks > > In the example above, we have used defoverridable terminate: 2 to make > the definition of the terminate/2 function optional. > > However, in some cases, the use of defoverridable seems to be unnecessary. > For instance, we provide a default implementation for handle_call/3 and > mark it as overridable, but the default implementation simply raises when > invoked. That's counter-intuitive as it would be best to simply not define > a default implementation in the first place, truly making the > handle_call/3 callback optional. > > Luckily, Erlang 18 added support for marking callbacks as optional, which > we support on Elixir v1.4. We propose Elixir and libraries to leverage this > feature and no longer define default implementations for the handle_* > functions > and instead mark them as optional. > > Instead of the version we have today: > > defmodule GenServer do > @callback handle_call(message, from, state) > > defmacro __using__(_) do > quote do > @behaviour GenServer > > def handle_call(_message, _from, _state) do > raise "handle_call/3 not implemented" > end > > # ... > > defoverridable handle_call: 3 > end > endend > > We propose: > > defmodule GenServer do > @callback handle_call(message, from, state) > @optional_callbacks handle_call: 3 > > defmacro __using__(_) do > quote do > @behaviour GenServer > > # ... > end > endend > > The proposed code is much simpler conceptually since we are using the > @optional_callbacks feature instead of defoverridable to correctly mark > optional callbacks as optional. defoverridable will still be used for > functions such as terminate/2, which are truly required. > > For developers using GenServer, no change will be necessary to their code > base. The goal is that, by removing unnecessary uses of defoverridable/1, > the Elixir code base can lead by example and hopefully push the community > to rely less on such tools when they are not necessary. > The @impl annotation > > Even with the improvements above, the usage of defoverridable/1 and > @optional_callbacks still have one major downside: the lack of warnings > for implementation mismatches. For example, imagine that instead of > defining handle_call/3, you accidentally define a non-callback > handle_call/2. Because handle_call/3 is optional, Elixir won't emit any > warnings, so it may take a while for developers to understand why their > handle_call/2 callback is not being invoked. > > We plan to solve this issue by introducing the @impl true annotation that > will check the following function is the implementation of a behaviour. > Therefore, if someone writes a code like this: > > @impl truedef handle_call(message, state) do > ...end > > The Elixir compiler will warn that the current module has no behaviour > that requires the handle_call/2 function to be implemented, forcing the > developer to correctly define a handle_call/3 function. This is a > fantastic tool that will not only help the compiler to emit warnings but > will also make the code more readable, as any developer that later uses the > codebase will understand the purpose of such function is to be a callback > implementation. > > The @impl annotation is optional. When @impl true is given, we will also > add @doc false unless documentation has been given. We will also support > a module name to be given. When a module name is given, Elixir will check > the following function is an implementation of a callback in the given > behaviour: > > @impl GenServerdef handle_call(message, from, state) do > ...end > > defoverridable with behaviours > > While @impl will give more confidence and assistance to developers, it is > only useful if developers are defining behaviours for their contracts. > Elixir has always advocated that a behaviour must always be defined when a > set of functions is marked as overridable but it has never provided any > convenience or mechanism to enforce such rules. > > Therefore we propose the addition of defoverridable BehaviourName, which > will make all of the callbacks in the given behaviour overridable. This > will help reduce the duplication between behaviour and defoverridable > definitions and push the community towards best practice. Therefore, > instead of: > > defmodule GenServer do > defmacro __using__(_) do > quote do > @behaviour GenServer > > def init(...) do ... end > def terminate(..., ...) do ... end > def code_change(..., ..., ...) do ... end > > defoverridable init: 1, terminate: 2, code_change: 3 > end > endend > > We propose: > > defmodule GenServer do > defmacro __using__(_) do > quote do > def init(...) do ... end > def terminate(..., ...) do ... end > def code_change(..., ..., ...) do ... end > defoverridable GenServer > end > endend > > By promoting new defoverridable API above, we hope library developers will > consistently define behaviours for their overridable functions, also > enabling developers to use the @impl true annotation to guarantee the > proper callbacks are being implemented. > > PS: Notice defoverridable always comes after the function definitions, > currently and as well as in this proposal. This is required because Elixir > functions have multiple clauses and if the defoverridable came before, we > would be unable to know in some cases when the overridable function > definition ends and when the user overriding starts. By having > defoverridable at the end, this boundary is explicit. > Summing up > > This proposal promotes the use the of @optional_callbacks, which is > already supported by Elixir, and introduces defoverridable(behaviour_name) > which > will push library developers to define proper behaviours and callbacks for > overridable code. > > We also propose the addition of the @impl true or @impl > behaviour_nameannotation, > that will check the following function has been listed as a callback by any > behaviour used by the current module. > > Feedback? > > > *José Valim* > www.plataformatec.com.br > Skype: jv.ptec > Founder and Director of R&D > -- You received this message because you are subscribed to the Google Groups "elixir-lang-core" group. To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. To view this discussion on the web visit https://groups.google.com/d/msgid/elixir-lang-core/85e62305-2b5f-4ae2-8e20-19f5cdf6e6d8%40googlegroups.com. For more options, visit https://groups.google.com/d/optout.
