From b33accf37cecbfa40a9302d76b7aacbd94d9101f Mon Sep 17 00:00:00 2001 From: Aaron Weiker Date: Fri, 1 May 2026 21:29:47 -0700 Subject: [PATCH] docs: extend @impl pattern with multi-clause and @spec guidance --- patterns/genserver.md | 100 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 100 insertions(+) diff --git a/patterns/genserver.md b/patterns/genserver.md index 47aebd8..66e328f 100644 --- a/patterns/genserver.md +++ b/patterns/genserver.md @@ -189,6 +189,106 @@ end **Why:** `@impl true` only makes sense in the context of a declared behaviour. Using it without one causes a compiler warning, not a benefit. +### Multi-Clause Callbacks — `@impl` on First Clause Only + +**Source:** [lib/elixir/lib/module.ex#L72](https://github.com/elixir-lang/elixir/blob/f4e1b34617ef92052b65781f18eae5b88a490098/lib/elixir/lib/module.ex#L72) (`@impl` documentation) + +**What it does:** `@impl true` is a module attribute that applies to the function as a whole — all clauses, not just the one it precedes. Place it once before the first clause; subsequent clauses of the same function inherit the annotation. + +**Why:** Repeating `@impl true` on every clause is noise. Worse, it can mislead readers into thinking each clause is a separate function. The compiler already associates all clauses with the behaviour callback after seeing `@impl` on the first one. Additionally, if you mark one callback with `@impl`, the compiler requires all callbacks in that module to be marked — this enforcement operates at the function level, not the clause level. + +**Anti-pattern:** Annotating every clause: + +```elixir +# BAD — @impl repeated on each clause of the same function +@impl true +def handle_call({:get, key}, _from, state) do + {:reply, Map.get(state, key), state} +end + +@impl true +def handle_call({:keys}, _from, state) do + {:reply, Map.keys(state), state} +end + +@impl true +def handle_call({:size}, _from, state) do + {:reply, map_size(state), state} +end +``` + +**Example — after:** + +```elixir +@impl true +def handle_call({:get, key}, _from, state) do + {:reply, Map.get(state, key), state} +end + +def handle_call({:keys}, _from, state) do + {:reply, Map.keys(state), state} +end + +def handle_call({:size}, _from, state) do + {:reply, map_size(state), state} +end +``` + +**When NOT to apply this:** If clauses of the same callback are separated by other functions (non-consecutive definitions), the compiler may not associate them. Keep multi-clause callbacks grouped together — this is independently good practice for readability. + +### Omit `@spec` on `@impl true` Callbacks + +**Source:** [lib/elixir/lib/module.ex#L121](https://github.com/elixir-lang/elixir/blob/f4e1b34617ef92052b65781f18eae5b88a490098/lib/elixir/lib/module.ex#L121) (`@impl` marks function as `@doc false`) + +**What it does:** When `@impl true` is present, omit the `@spec`. The behaviour's `@callback` already defines the type contract for the function. Dialyzer uses the callback spec to check implementations — a redundant `@spec` on the implementation adds no safety and creates a second source of truth that can drift. + +**Why:** The behaviour owns the contract. Adding `@spec init(term()) :: {:ok, map()}` on a GenServer callback just restates `@callback init(init_arg :: term) :: {:ok, state} | ...` with less information (the callback documents the full union of valid return types). When the behaviour updates its callback spec, implementations with stale `@spec` annotations silently disagree. + +**Anti-pattern:** Verbatim-copying the callback spec onto the implementation: + +```elixir +# BAD — spec restates what the @callback already defines +@spec init(term()) :: {:ok, map()} +@impl true +def init(opts) do + {:ok, %{started_at: DateTime.utc_now(), config: opts}} +end + +@spec handle_call(term(), GenServer.from(), map()) :: {:reply, term(), map()} +@impl true +def handle_call(:status, _from, state) do + {:reply, state.started_at, state} +end +``` + +**Example — after:** + +```elixir +@impl true +def init(opts) do + {:ok, %{started_at: DateTime.utc_now(), config: opts}} +end + +@impl true +def handle_call(:status, _from, state) do + {:reply, state.started_at, state} +end +``` + +**When NOT to apply this:** When the implementation intentionally accepts a narrower type than the callback declares, a more specific `@spec` documents that constraint for readers and Dialyzer: + +```elixir +# Justified — documents that this init/1 only accepts keyword lists, +# not arbitrary term() as the callback permits +@spec init(keyword()) :: {:ok, Config.t()} +@impl true +def init(opts) when is_list(opts) do + {:ok, Config.new!(opts)} +end +``` + +If the `@spec` would be identical to (or wider than) the `@callback`, omit it. If it's meaningfully narrower, keep it. + --- ## Pattern 3: Guard-Protected `start_link`