Credence.Pattern.NoFetchThenUpdate (credence v0.7.0)

Copy Markdown

Detects Map.update!/3 or Map.update/4 called inside a case Map.fetch/2 {:ok, val} branch on the same map and key, and rewrites the redundant update into a Map.put/3.

Inside the {:ok, val} branch the key is known to be present and its current value is bound to val. Map.update!/Map.update then look the key up a second time only to apply a function to that same value. Since the value is already in hand as val, the same result is produced by Map.put(map, key, fun.(val)) — one map lookup is saved.

Bad

case Map.fetch(counts, key) do
  {:ok, n} ->
    {n, Map.update!(counts, key, &(&1 + 1))}

  :error ->
    {0, Map.put(counts, key, 1)}
end

Good

case Map.fetch(counts, key) do
  {:ok, n} ->
    {n, Map.put(counts, key, (&(&1 + 1)).(n))}

  :error ->
    {0, Map.put(counts, key, 1)}
end

Why the fix applies the captured function instead of inlining it

Map.update!(map, key, fun) with the key present is exactly Map.put(map, key, fun.(val)): fun is evaluated once and applied once to the current value, which is val. Rather than inlining the body of an arbitrary fun (hard and error-prone for captures), the fix simply applies it — fun.(val) — which is behaviour-identical for every input. mix format tidies the rendered call afterwards.

Safety narrowing

The rewrite is only behaviour-preserving when:

  • the fetched map and key are simple (a bare variable or a literal), so re-mentioning them in Map.put cannot evaluate a side effect twice or read a different value, and
  • the {:ok, val} branch body contains no binding constructs (=, fn, for, with, case, cond, receive, try), so the map, key and val bindings observed at the Map.update call are the same ones the Map.fetch produced (no rebinding or shadowing).

Calls that fall outside this core are deliberately left untouched.