Credence.Pattern.NoManualFind (credence v0.7.0)

Copy Markdown

Detects hand-rolled recursive "find first matching element" functions that should use Enum.find/3.

Why this matters

The classic 3-clause recursive find — empty-list base returning a default, a guarded clause returning the head, a fallback clause recursing on the tail — reimplements Enum.find/3 with no benefit:

# Flagged (arity 1)
defp find_odd([]), do: -1
defp find_odd([h | _t]) when rem_safe(h), do: h
defp find_odd([_h | t]), do: find_odd(t)

# Flagged (arity 2)
defp find_first([], default), do: default
defp find_first([h | _t], _default) when h > 0, do: h
defp find_first([_h | t], default), do: find_first(t, default)

The fix

Each flagged group collapses to a single clause delegating to Enum.find/3:

defp find_first(list, default) when is_list(list),
  do: Enum.find(list, default, fn h -> h > 0 end)

This is behaviour-preserving for every input:

  • Domain. The collapsed clause keeps the original name/arity and guards when is_list(list). The multi-clause original only ever matched [] or [_ | _] (and raised FunctionClauseError otherwise). is_list/1 is true for both proper and improper lists and false for everything else — exactly the original domain. Maps/ranges/scalars raise FunctionClauseError on both sides; an improper list whose match occurs before the improper tail returns the same element on both sides (verified), and one with no match raises FunctionClauseError on both sides.
  • First match, same order. Both walk a list head-to-tail and stop at the first element satisfying the predicate, returning that element (not the predicate's value — so a nil/false matched element is returned as-is, just like the original head clause).
  • Default. Returned only when nothing matches. See the narrowing below for why the default must be eager-evaluation-safe.

Detection scope (only the safely-fixable cases)

Exactly 3 clauses, same name, all arity 1 or all arity 2, where some assignment of the clauses fills these three roles:

  1. Base — ([]) (arity 1) / ([], default) (arity 2), no guard, whose body is a single expression:
    • arity 1: a literal (number incl. negative, atom/nil/booleans, binary, []);
    • arity 2: the second parameter variable returned unchanged.

  2. Match — ([h | _], …) when <guard>, no other guard vars, body is exactly the head variable h.

  3. Recurse — ([_ | t], …), no guard, body is exactly the self-call on the tail (fn(t) / fn(t, default)), every other argument threaded through unchanged.

Why these narrowings (each drops a different-answer case)

  • Non-raising boolean guard only. The <guard> must be a non-raising boolean: comparisons, and/or/not, and unary is_* checks over the head variable and literals. A guard that can raise (e.g. rem(h, 2) != 0 on a non-integer) silently skips the element in the multi-clause form, but the same expression as an Enum.find/3 predicate would raise — a different answer (verified). Such groups are left untouched.
  • Eager-evaluation-safe default only. An Enum.find/3 default argument is evaluated eagerly, whereas the base clause runs only when the list is exhausted. A literal (arity 1) or an already-bound parameter (arity 2) has no side effect and the same value either way; an arbitrary expression default (e.g. (IO.puts(...); -1)) would run eagerly even when an element matches (verified) — not flagged.
  • Single-expression bodies only. Each role's body must be exactly its one expression (head var / self-call / default), never a multi-statement block. A block could carry a side effect that the predicate-based rewrite would drop or move — a different answer.