@behaviour, @callback and @spec

You don’t need them as Dialyzer will give a callback does not match spec error, but please include them anyway. As a reader I do not want to have to jump to the behaviour definition to find out what the arguments are. In that vein if you include a using macro where you define default callbacks that are overridable with defoverridable, elide the spec in the using macro, otherwise you’ll get a compiler error for duplicating the spec if anyone actually overrides and wants to spec the override.

YES

defmodule MyBehaviour do
  @callback foo() :: :ok
end

defmodule MyImpl do
  @impl MyBehaviour
  @spec foo() :: :ok
  def foo(), do: :ok
end

YES

defmodule MyBehaviour do
  @callback foo() :: :ok

  defmacro __using__(_) do
    quote do
      @impl MyBehaviour
      @spec foo() :: :ok
      def foo(), do: :ok
    end
  end
end

defmodule MyImpl do
  use MyBehaviour
end

YES

defmodule MyBehaviour do
  @callback foo() :: :ok

  defmacro __using__(_) do
    quote do
      @impl MyBehaviour
      def foo(), do: :ok
    
      defoverridable [foo: 0]
    end
  end
end

defmodule MyImpl do
  use MyBehaviour

  @impl MyBehaviour
  @spec foo() :: :ok
  def foo(), do: :ok
end

NO

defmodule MyBehaviour do
  @callback foo() :: :ok

  defmacro __using__(_) do
    quote do
      @impl MyBehaviour
      @spec foo() :: :ok
      def foo(), do: :ok
    
      defoverridable [foo: 0]
    end
  end
end

defmodule MyImpl do
  use MyBehaviour
  
  # this errors if you include the spec
  # @spec foo() :: :ok
  @impl MyBehaviour
  def foo(), do: :ok
end

I don’t like such ideas as it’s really a pain for maintainers of libraries in case multiple libraries are implementing specified behaviour. Imagine that one behaviour is used by 10 libraries and 3 of them are not updated - what to do in such case? Should someone fork it just to update @docs?

Personalyl I think that introducing macro for documentation is a bit too overcomplicated. In such cases I would prefer to use h helper in iex (or just html documentation) and docs delegating feature:

@doc delegate_to: {Foo, :bar, 3}

This would add a link which could be used in h helper (or just clicked on html page). In such case there is no need to change same documentation for multiple implementations or write any macros.

I’m very unclear which part of my suggestion you’re taking umbrage with. Can you give an example of what you’re talking about?

Sure, here is your changed code:

defmodule MyBehaviour do
  @callback foo() :: :ok
end

defmodule MyImpl do
  @behaviour MyBehaviour

  @doc delegate_to: {MyBehaviour, :foo, 0}
  @doc "Implementation-specific docs goes here …"
  @impl MyBehaviour
  def foo(), do: :ok
end

which would give:

iex(1)> h MyImpl.foo       

                                   def foo()                                    

delegate_to: MyBehaviour.foo/0

Implementation-specific docs goes here …

iex(2)> b MyBehaviour.foo/0
@callback foo() :: :ok

This is much simpler than writing macros or copy-paste documentation and spec.

Generally we should avoid using macros unless it’s required.

I wasn’t suggesting to use a macro. Simply saying that if you do have a macro and the function is overridable to elide the spec in the macro (as they do in e.g. GenServer but do NOT elide in HTTPoison). I don’t think they’re incompatible, unless I’m misunderstanding something. This thread is about @spec not @doc.

Yes, it is - look that delegated callback/function have also its specification (not only documentation - I did not even added documentation to your behaviour code).

As said if there is really no need to write macro then we should avoid doing it + it makes exactly no sense to copy @doc and @spec over all implementations (especially manually i.e. without macro).

Simply look how much code we wrote + how much nested spaces you have in macro which is not needed in such case.

If we want to tell that some documentation and/or specification is same for our function then we can simply delegate it in @doc which would give others enough information.

Again, I’m not advocating for the macro, at all, in any way shape or form. I agree in this case it’s not needed. I’m contrasting the approaches taken between GenServer and HTTPoison.Base.

https://github.com/elixir-lang/elixir/blob/master/lib/elixir/lib/gen_server.ex#L760
vs.
https://github.com/edgurgel/httpoison/blob/master/lib/httpoison/base.ex#L220

Dialyzer will pick this all up even if you drop all the @specs. But for readers of the code, giving them the ability to @spec the implementation is much more pleasant (even if they choose not to), because they do not need to look outside the file or inside IEx to be able to figure out what’s going on. And if they have the Credo rule on to require @spec for all functions, you must ignore in the implementation because the compiler will complain about duplicate specs for the implemented callbacks.

GenServer is not good example here is it’s most probably intended to not add @doc and @spec to those functions. Anyway I can see what you are talking about. For sure adding @doc false when you really expect documentation is really bad, but @doc delegate_to is still ok here

More … for this use case when most of projects don’t even document those functions it could be nice to properly link them to GenServer documentation page. I believe that core team does not wanted to add extra documentation for every module which uses GenServer as reading documentation could be a bit harder due to more documentation.

I have similar feeling to HTTPoison. Omitting that you are linking to deprecated function we can still use @doc delegate_to in such case without any problem.

ok, so for me it’s even not an option to consider

Personally I don’t like be forced to something. I’m like Erlang/Elixir - I can fail as much as I need. No matter how much times - sooner or later I would be better. If I would be limited in order to protect myself then I’m not going to make fails and learn on them. Look that Elixir is written to be as much extensible as possible.

The goal here is to introduce well known standards (just like adding optional @spec support), but not force them. Imagine what would happen if suddenly all hex libraries would fail, because @spec would be required for all functions. Look that @spec everywhere would be like a dream for readers, but also huge pain for maintainers.

Sooner or later you would get an edge-case. There is no rule in world to cover all cases, so forcing anything is never a good idea. It’s why phoenix is not called a framework, but library.

You have lots of cases when you need to take a look at other modules to understand code properly especially in cases like GenServer. You just need to remind from time to time handle_call, handle_cast and handle_info.

Personally I think that delegating documentation is much better, because same documentation and spec does not need to be written multiple times. Of course we do not see it in such simple examples.

Simply compare:

@doc delegate_to: {MyBehaviour, :foo, 0}

which is never going to change with copy-paste long specifications especially with map (optional and required keys).

There is no even need to imagine long map specification. Just look at really simple init/1 specification:
https://github.com/elixir-lang/elixir/blob/0a81b278619324e088641abe9d486dca8a6510b5/lib/elixir/lib/gen_server.ex#L447
You would have few extra lines for each implementation’s function just to not make one click on HTML page and it’s not even middle size of typical real world specification..

Hi

Wow, thanks for all comments! Did not know that this would stir up this many opinions.

A see your point @asummers, but since this is not a public library (“only” used within our company), I prefer that the implementers spend the extra time to go into the definition of the behaviour rather than using multiple specs that will effect the maintenance in the long run.

I also found that the @spec may state less than the actual @callback without Dialyzer telling me, witch gives me another argument not to use the @specs…

Example given:

defmodule Register.DocEvents do
  @callback initialize(soure :: binary() | atom()) :: :ok | {:error, String.t()}
end

defmodule Register do
  @behaviour Register.DocEvents

  @impl Register.DocEvents
  @spec initialize(atom()) :: :ok
  def initialize(source) do
    ...
  end
end

Dialyzer signals this is ok, and I guess it is since the actual implementation fits within the original specification. In this case, the @spec makes sense since this implementation is not the same as the @callback stated, but if they are expected to be the same then adding an extra @spec just creates more maintenance burden.

For sure if you have different @spec for specific implementation than @callback then you should use @spec. It’s important to let other knows that you will not meet exactly all cases expected by @behaviour. Otherwise I suggest delegating to callback documentation as I have mentioned previously. For dialyzer it’s ok probably because both arguments and return value match @callback specification - only small part, but matches.