docs: extend @impl pattern with multi-clause and @spec guidance
This commit is contained in:
@@ -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.
|
**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`
|
## Pattern 3: Guard-Protected `start_link`
|
||||||
|
|||||||
Reference in New Issue
Block a user