Elixir Docs Review

Elixir Documentation Review

Quick Reference

Review Checklist

Module Documentation

• [ ] All public modules have @moduledoc
• [ ] First-line summary is concise (one line, used by tools as summary)
• [ ] @moduledoc includes ## Examples where appropriate
• [ ] @moduledoc false only on internal/implementation modules

Function Documentation

• [ ] All public functions have @doc
• [ ] All public functions have @spec
• [ ] @doc describes return values clearly
• [ ] Multi-clause functions documented before first clause
• [ ] Function head declared when arg names need clarification

Doctests

• [ ] Doctests present for pure, deterministic functions
• [ ] No doctests for side-effectful operations (DB, HTTP, etc.)
• [ ] Doctests actually run (module included in test file)

Cross-References

• [ ] Module references use backtick auto-linking (MyModule)
• [ ] Function refs use proper arity format (function/2)
• [ ] Type refs use t: prefix (t:typename/0)
• [ ] No plain-text references where auto-links are possible

Metadata

• [ ] @since annotations on new public API additions
• [ ] @deprecated with migration guidance where appropriate

Valid Patterns (Do NOT Flag)

• @doc false on callback implementations - Documented at behaviour level
• @doc false on protocol implementations - Protocol docs cover the intent
• Missing @spec on private functions - @spec optional for internals
• Short @moduledoc without ## Examples on simple utility modules - Not every module needs examples
• Using @impl true without separate @doc - Inherits documentation from behaviour

Context-Sensitive Rules

Gates (sequenced — do not skip)

Work in order. Do not draft or ship a finding until the prior step passes.

1. Scope lock — Pass when: You listed the exact .ex/.exs file paths (or Module names) under review; no vague “the project” scope. 2. Full-context read — Pass when: For each candidate issue, you read the full surrounding definition (all clauses for multi-clause functions; full @moduledoc block for module-level claims), not only a diff hunk or search snippet. 3. Evidence bundle — Pass when: Every draft finding uses the [FILE:LINE] ISSUE_TITLE header (line range allowed) and includes a verbatim quote or pointer to the @doc / @spec / doctest text in question. Module.function/arity may appear as supporting context but does not replace the [FILE:LINE] anchor. For “doctest fails” claims, Pass when: you cite mix test output for the relevant file or line, or the exact error string. 4. Protocol before report — Pass when: You loaded and followed review-verification-protocol (its Pre-Report checklist) before finalizing the issue list—not after.

When to Load References

• Reviewing @moduledoc or @doc quality, seeing anti-patterns -> doc-quality.md
• Reviewing @spec, @type, or @typedoc coverage -> spec-coverage.md

Documentation Quality

What Makes Good Module Docs

A well-documented module tells the reader four things: what it does, when to use it, how to use it, and how to configure it.

Structure

defmodule MyApp.RateLimiter do
  @moduledoc """
  Token bucket rate limiter for API endpoints.

  Use this module to throttle incoming requests per client. It tracks
  request counts in ETS and supports configurable burst and refill rates.

  ## Examples

      iex> {:ok, limiter} = RateLimiter.start_link(rate: 100, interval: :timer.seconds(1))
      iex> RateLimiter.allow?(limiter, "client-123")
      true

  ## Configuration

  Expects the following options:

    * `:rate` - Maximum requests per interval (required)
    * `:interval` - Refill interval in milliseconds (default: 1000)
    * `:burst` - Maximum burst size (default: same as `:rate`)
  """
end

The first line ("Token bucket rate limiter for API endpoints.") is critical -- ExDoc uses it as the module summary in sidebar listings and search results. Keep it to one sentence.

What Makes Good Function Docs

Good function docs answer: what does it do, what are the inputs, what does it return, and what are the edge cases.

@doc """
Checks whether a client is allowed to make a request.

Decrements the token count for the given `client_id` and returns
whether the request should proceed. When tokens are exhausted,
returns `false` until the next refill interval.

Returns `{:ok, remaining}` with the remaining token count, or
`{:error, :rate_limited}` when the limit is exceeded.

## Examples

    iex> RateLimiter.check("client-123")
    {:ok, 99}

    iex> RateLimiter.check("exhausted-client")
    {:error, :rate_limited}
"""
@spec check(client_id :: String.t()) :: {:ok, non_neg_integer()} | {:error, :rate_limited}
def check(client_id) do
  # ...
end

Anti-Patterns

Empty @moduledoc String

# BAD - empty string still shows in ExDoc as a blank page
defmodule MyApp.Internal.Parser do
  @moduledoc ""
end

# GOOD - explicitly hidden from ExDoc output
defmodule MyApp.Internal.Parser do
  @moduledoc false
end

If you want to hide a module from documentation, use @moduledoc false. An empty string creates a confusing blank entry in generated docs.

Restating the Function Name

# BAD - tells the reader nothing they didn't already know
@doc "Gets the user."
@spec get_user(integer()) :: User.t() | nil
def get_user(id), do: Repo.get(User, id)

# GOOD - explains behavior, return semantics, edge cases
@doc """
Fetches a user by primary key.

Returns the `%User{}` struct if found, or `nil` if no user exists
with the given `id`. Does not raise on missing records.
"""
@spec get_user(integer()) :: User.t() | nil
def get_user(id), do: Repo.get(User, id)

Missing Return Value Documentation

# BAD - what does it return on success? On failure?
@doc "Processes the payment."
def process_payment(order), do: # ...

# GOOD - return values are explicit
@doc """
Submits a payment for the given order to the payment gateway.

Returns `{:ok, %Transaction{}}` on successful charge, or
`{:error, %PaymentError{}}` if the charge is declined or
the gateway is unavailable.
"""
def process_payment(order), do: # ...

Wrong or Outdated Doctest Examples

# BAD - doctest will fail because the function now returns a tuple
@doc """
Formats a price in cents as a dollar string.

## Examples

    iex> format_price(1999)
    "$19.99"
"""
def format_price(cents) do
  {:ok, "$#{cents / 100}"}  # Return type changed but doctest wasn't updated
end

# GOOD - doctest matches actual return value
@doc """
Formats a price in cents as a dollar string.

## Examples

    iex> format_price(1999)
    {:ok, "$19.99"}
"""
def format_price(cents) do
  {:ok, "$#{cents / 100}"}
end

Documenting Obvious Params but Not Edge Cases

# BAD - documents obvious params, ignores what matters
@doc """
Divides `a` by `b`.

## Parameters

  * `a` - The numerator
  * `b` - The denominator
"""
def divide(a, b), do: a / b

# GOOD - documents the interesting behavior
@doc """
Divides `a` by `b`.

Raises `ArithmeticError` when `b` is zero. Returns a float
even when both arguments are integers.

## Examples

    iex> divide(10, 3)
    3.3333333333333335

    iex> divide(10, 0)
    ** (ArithmeticError) bad argument in arithmetic expression
"""
def divide(a, b), do: a / b

Using @doc When @impl true Would Suffice

# BAD - redundant doc that duplicates the behaviour's documentation
defmodule MyApp.Cache do
  @behaviour MyApp.Store

  @doc "Initializes the store."
  @impl true
  def init(opts), do: # ...
end

# GOOD - @impl true inherits docs from the behaviour
defmodule MyApp.Cache do
  @behaviour MyApp.Store

  @impl true
  def init(opts), do: # ...
end

When a module implements a behaviour, using @impl true signals that the function's contract is defined by the behaviour. Adding a separate @doc that just restates the behaviour's docs creates maintenance burden with no benefit. Only add @doc on @impl true callbacks when the implementation has important details the behaviour docs don't cover.

The "Write for the Reader" Principle

Documentation is read by developers who don't have your current context. Ask yourself:

1. Would a new team member understand this module's purpose from @moduledoc alone? 2. Would a caller know what to pass and what to expect back from @doc alone? 3. Would someone debugging a failure understand the error cases from the docs?

If the answer to any of these is no, the docs need improvement -- regardless of whether they technically exist.

Review Questions

1. Does the @moduledoc first line work as a standalone summary? 2. Do @doc blocks describe return values and error cases? 3. Are doctests current and matching actual function behavior? 4. Do docs add value beyond what the function name and @spec already convey?

Spec Coverage

Common @spec Patterns

Basic Types

@spec greet(String.t()) :: String.t()
def greet(name), do: "Hello, #{name}!"

@spec count_items(list()) :: non_neg_integer()
def count_items(items), do: length(items)

@spec enabled?() :: boolean()
def enabled?, do: Application.get_env(:my_app, :enabled, false)

Union Types

@spec fetch_account(integer()) :: {:ok, Account.t()} | {:error, :not_found | :suspended}
def fetch_account(id) do
  case Repo.get(Account, id) do
    nil -> {:error, :not_found}
    %Account{status: :suspended} = account -> {:error, :suspended}
    account -> {:ok, account}
  end
end

Custom Types

@type id :: pos_integer()
@type reason :: :not_found | :unauthorized | :timeout
@type result :: {:ok, t()} | {:error, reason()}

@spec find(id()) :: result()
def find(id), do: # ...

Keyword Options

@type option :: {:timeout, pos_integer()} | {:retries, non_neg_integer()}

@spec request(String.t(), [option()]) :: {:ok, Response.t()} | {:error, term()}
def request(url, opts \\ []) do
  timeout = Keyword.get(opts, :timeout, 5_000)
  retries = Keyword.get(opts, :retries, 3)
  # ...
end

When Clauses

@spec transform(list(a), (a -> b)) :: list(b) when a: term(), b: term()
def transform(items, func), do: Enum.map(items, func)

@spec wrap(value) :: [value] when value: term()
def wrap(value), do: [value]

@type and @typedoc

Use custom types to name domain concepts and reduce repetition.

defmodule MyApp.Shipping do
  @typedoc "Weight in grams."
  @type weight :: non_neg_integer()

  @typedoc "A geographic coordinate pair."
  @type coordinates :: {latitude :: float(), longitude :: float()}

  @typedoc "Shipping status throughout the delivery lifecycle."
  @type status :: :pending | :in_transit | :delivered | :returned

  @spec estimate_cost(weight(), coordinates(), coordinates()) :: {:ok, Decimal.t()}
  def estimate_cost(weight, origin, destination) do
    # ...
  end
end

Benefits:

• weight() communicates intent better than non_neg_integer()
• status() centralizes valid values -- add a new status in one place
• @typedoc appears in ExDoc, making types self-documenting

When @spec Is Required

All public exported functions should have @spec. This includes:

defmodule MyApp.Accounts do
  # Required: public function
  @spec create_user(map()) :: {:ok, User.t()} | {:error, Ecto.Changeset.t()}
  def create_user(attrs), do: # ...

  # Required: public function with default args
  @spec list_users(keyword()) :: [User.t()]
  def list_users(opts \\ []), do: # ...

  # Required: public function used as callback
  @spec child_spec(keyword()) :: Supervisor.child_spec()
  def child_spec(opts), do: # ...
end

When @spec Is Optional

defmodule MyApp.Accounts do
  # Optional: private function
  defp normalize_email(email), do: String.downcase(email)

  # Optional: macro-generated functions (e.g., Ecto schema fields)
  # These are generated by `schema` and `field` macros

  # Optional: @impl true callbacks where the behaviour defines the spec
  @impl true
  def handle_call(:ping, _from, state), do: {:reply, :pong, state}
end

Common @spec Mistakes

Overly Broad term() or any()

# BAD - term() hides what the function actually accepts
@spec process(term()) :: term()
def process(%Order{} = order), do: # ...

# GOOD - spec reflects the actual types
@spec process(Order.t()) :: {:ok, Receipt.t()} | {:error, String.t()}
def process(%Order{} = order), do: # ...

Using term() or any() defeats the purpose of specs. If you know the type, declare it.

Missing Union Branches

# BAD - forgets the nil case from Repo.get
@spec find_user(integer()) :: {:ok, User.t()}
def find_user(id) do
  case Repo.get(User, id) do
    nil -> {:error, :not_found}  # Not reflected in spec!
    user -> {:ok, user}
  end
end

# GOOD - all return paths represented
@spec find_user(integer()) :: {:ok, User.t()} | {:error, :not_found}
def find_user(id) do
  case Repo.get(User, id) do
    nil -> {:error, :not_found}
    user -> {:ok, user}
  end
end

Not Using Custom Types for Repeated Patterns

# BAD - same tuple pattern repeated across many functions
@spec create(map()) :: {:ok, User.t()} | {:error, Ecto.Changeset.t()}
@spec update(User.t(), map()) :: {:ok, User.t()} | {:error, Ecto.Changeset.t()}
@spec delete(User.t()) :: {:ok, User.t()} | {:error, Ecto.Changeset.t()}

# GOOD - define a type once, reuse it
@type changeset_result :: {:ok, User.t()} | {:error, Ecto.Changeset.t()}

@spec create(map()) :: changeset_result()
@spec update(User.t(), map()) :: changeset_result()
@spec delete(User.t()) :: changeset_result()

Specs That Don't Match Function Clauses

# BAD - spec says it only accepts String.t() but function also handles nil
@spec normalize(String.t()) :: String.t()
def normalize(nil), do: ""
def normalize(value), do: String.trim(value)

# GOOD - spec covers all clauses
@spec normalize(String.t() | nil) :: String.t()
def normalize(nil), do: ""
def normalize(value), do: String.trim(value)

Using list() When a Specific Element Type Is Known

# BAD - list() tells the caller nothing about contents
@spec active_users() :: list()
def active_users, do: Repo.all(from u in User, where: u.active == true)

# GOOD - caller knows what's in the list
@spec active_users() :: [User.t()]
def active_users, do: Repo.all(from u in User, where: u.active == true)

Dialyzer-Friendly Specs

Dialyzer uses @spec to perform success typing analysis. Specs that help Dialyzer catch real bugs:

# Dialyzer can catch callers passing wrong types
@spec send_notification(User.t(), String.t()) :: :ok | {:error, :delivery_failed}
def send_notification(%User{email: email}, message) do
  # ...
end

# Dialyzer can verify pattern match exhaustiveness
@type role :: :admin | :editor | :viewer

@spec permissions(role()) :: [atom()]
def permissions(:admin), do: [:read, :write, :delete]
def permissions(:editor), do: [:read, :write]
def permissions(:viewer), do: [:read]
# Dialyzer warns if a new role is added to the type but not handled here

Tips for Dialyzer compatibility:

• Avoid @spec function() :: no_return() unless the function truly never returns (e.g., raises always)
• Use String.t() instead of binary() for text data -- they're equivalent to Dialyzer but String.t() communicates intent
• Declare @opaque types when internal representation should not leak to callers

Review Questions

1. Do all public functions have @spec? 2. Do specs accurately reflect all return paths (including error tuples)? 3. Are custom @type definitions used for repeated patterns? 4. Are specs specific enough to catch real bugs (no unnecessary term() or any())?