Current section
Files
Jump to
Current section
Files
lib/pattern/no_map_keys_enum_lookup.ex
defmodule Credence.Pattern.NoMapKeysEnumLookup do
@moduledoc """
Detects `Map.keys(var)` piped into an `Enum` function whose callback
also looks up values from the same map variable.
## Why this matters
LLMs frequently port the Python idiom `for key in dict: ... dict[key]`
into Elixir as `Map.keys(map) |> Enum.xxx(fn k -> ... map[k] ... end)`.
This creates an unnecessary intermediate list and performs redundant
lookups. In Elixir, maps are directly enumerable as `{key, value}`
pairs:
# Flagged — extra allocation + redundant lookups
Map.keys(freqs)
|> Enum.all?(fn char -> Map.get(other, char, 0) >= freqs[char] end)
# Idiomatic — single traversal, values already in hand
Enum.all?(freqs, fn {char, count} ->
Map.get(other, char, 0) >= count
end)
## Detection scope (strict)
Only flagged when **all three** conditions hold:
1. `Map.keys(var)` is called on a simple variable,
2. The result is passed to `Enum.all?` or `Enum.any?`, and
3. The callback body references `var` via `var[key]`,
`Map.get(var, ...)`, `Map.fetch(var, ...)`, or
`Map.fetch!(var, ...)`.
Patterns where only keys are needed (no value lookup in the callback)
are **not** flagged.
Only `all?`/`any?` are flagged because their result is a boolean — a
commutative AND/OR over the elements — so iterating the map directly (a
different order than `Map.keys/1` once a map exceeds 32 entries) gives the
same answer. `map`/`filter`/`reject`/`flat_map` return order-observable lists
and `each`'s side effects are order-sensitive, so rewriting them would not be
behaviour-preserving and they are left alone.
"""
use Credence.Pattern.Rule
alias Credence.Issue
# Only the boolean-returning sinks are order-insensitive: `all?`/`any?` compute
# a commutative AND/OR over the elements, so iterating the map directly (which
# visits entries in a different order than `Map.keys/1` once a map exceeds 32
# keys) yields the SAME boolean. `map`/`flat_map`/`filter`/`reject` return a
# list whose order is observable, and `each`'s side effects are order-sensitive
# — rewriting those is not behaviour-preserving for >32-key maps, so they are
# NOT flagged. See [[map-keys-order-vs-enum-order]].
@flagged_enum_fns [:all?, :any?]
@impl true
def check(ast, _opts) do
{_ast, issues} =
Macro.prewalk(ast, [], fn node, issues ->
case check_node(node) do
{:ok, issue} -> {node, [issue | issues]}
:error -> {node, issues}
end
end)
Enum.reverse(issues)
end
@impl true
def fix_patches(ast, _opts) do
Credence.RuleHelpers.patches_from_postwalk(ast, fn node ->
case try_fix_node(node) do
{:ok, fixed} -> fixed
_ -> node
end
end)
end
defp try_fix_node({:|>, _, _} = node) do
steps = flatten_pipeline(node)
case try_fix_pipeline_two_step(steps) do
{:ok, _} = result -> result
:error -> try_fix_pipeline_three_step(steps)
end
end
# Direct call: Enum.xxx(Map.keys(var), callback)
defp try_fix_node({{:., _, [mod, fn_name]}, _meta, [first_arg, callback | _rest]})
when fn_name in @flagged_enum_fns do
with true <- enum_module?(mod),
{:ok, var_name, var_expr} <- extract_map_keys_var_with_expr(first_arg) do
apply_direct_fix(var_expr, var_name, mod, fn_name, callback)
end
end
# Direct call: Enum.xxx(Map.keys(var), callback)
defp try_fix_node({{:., _, [mod, fn_name]}, _meta, [first_arg, callback | _rest]})
when fn_name in @flagged_enum_fns do
with true <- enum_module?(mod),
{:ok, var_name, var_expr} <- extract_map_keys_var_with_expr(first_arg) do
apply_direct_fix(var_expr, var_name, mod, fn_name, callback)
end
end
defp try_fix_node(_), do: :error
# Two-step: Map.keys(var) |> Enum.xxx(callback) [|> rest]
defp try_fix_pipeline_two_step([map_keys_call, enum_call | rest]) do
with {:ok, var_name, var_expr} <- extract_map_keys_var_with_expr(map_keys_call),
{:ok, fn_name, callback} <- extract_piped_enum(enum_call) do
apply_pipeline_fix(var_expr, var_name, fn_name, callback, rest)
end
end
defp try_fix_pipeline_two_step(_), do: :error
# Three-step: var |> Map.keys() |> Enum.xxx(callback) [|> rest]
defp try_fix_pipeline_three_step([var_expr, map_keys_call, enum_call | rest]) do
with {:ok, var_name} <- extract_simple_var(var_expr),
true <- map_keys_no_args?(map_keys_call),
{:ok, fn_name, callback} <- extract_piped_enum(enum_call) do
apply_three_step_fix(var_expr, var_name, fn_name, callback, rest)
end
end
defp try_fix_pipeline_three_step(_), do: :error
# Map.keys(var) |> Enum.xxx(callback) [|> rest]
#
# Every apply_* path requires `references_map_var?` — the SAME third condition
# the check enforces — so the fix rewrites EXACTLY what the check flags. Without
# it the fix fired on `Map.keys(m) |> Enum.any?(fn k -> k in @const end)`, whose
# callback never looks up `m`: a needless rewrite (and an unused `v` binding) on
# code the check leaves clean.
defp apply_pipeline_fix(var_expr, var_name, fn_name, callback, rest) do
with true <- references_map_var?(callback, var_name),
{:ok, new_callback} <- transform_callback(callback, var_name) do
mod = {:__aliases__, [], [:Enum]}
enum_call = {{:., [], [mod, fn_name]}, [], [var_expr, new_callback]}
{:ok, rebuild_pipeline([enum_call | rest])}
else
_ -> :error
end
end
# var |> Map.keys() |> Enum.xxx(callback) [|> rest]
defp apply_three_step_fix(var_expr, var_name, fn_name, callback, rest) do
with true <- references_map_var?(callback, var_name),
{:ok, new_callback} <- transform_callback(callback, var_name) do
mod = {:__aliases__, [], [:Enum]}
enum_step = {{:., [], [mod, fn_name]}, [], [new_callback]}
{:ok, rebuild_pipeline([var_expr, enum_step | rest])}
else
_ -> :error
end
end
# Enum.xxx(Map.keys(var), callback)
defp apply_direct_fix(var_expr, var_name, mod, fn_name, callback) do
with true <- references_map_var?(callback, var_name),
{:ok, new_callback} <- transform_callback(callback, var_name) do
{:ok, {{:., [], [mod, fn_name]}, [], [var_expr, new_callback]}}
else
_ -> :error
end
end
defp rebuild_pipeline([single]), do: single
defp rebuild_pipeline([first, second | rest]) do
Enum.reduce(rest, {:|>, [], [first, second]}, fn step, acc ->
{:|>, [], [acc, step]}
end)
end
defp transform_callback(callback, var_name) do
case extract_callback_param(callback) do
{:ok, key_name} ->
new_callback =
callback
|> rewrite_callback_param(key_name)
|> replace_map_lookups(var_name, key_name)
{:ok, new_callback}
:error ->
:error
end
end
# Extract the key variable name from the callback's parameter.
# Handles single-clause: fn k -> ... end
# with guard: fn k when guard -> ... end
defp extract_callback_param({:fn, _, [{:->, _, [params, _]}]}) do
case params do
[{name, _, ctx}] when is_atom(name) and is_atom(ctx) ->
{:ok, name}
[{:when, _, [{name, _, ctx}, _]}] when is_atom(name) and is_atom(ctx) ->
{:ok, name}
_ ->
:error
end
end
defp extract_callback_param(_), do: :error
# Rewrite the parameter from `k` to `{k, v}`.
defp rewrite_callback_param({:fn, fn_meta, [{:->, arrow_meta, [params, body]}]}, key_name) do
case params do
[{^key_name, pmeta, pctx}] ->
new_params = [{{key_name, pmeta, pctx}, {:v, [], pctx}}]
{:fn, fn_meta, [{:->, arrow_meta, [new_params, body]}]}
[{:when, when_meta, [{^key_name, pmeta, pctx}, guard]}] ->
new_params = [{:when, when_meta, [{{key_name, pmeta, pctx}, {:v, [], pctx}}, guard]}]
{:fn, fn_meta, [{:->, arrow_meta, [new_params, body]}]}
_ ->
{:fn, fn_meta, [{:->, arrow_meta, [params, body]}]}
end
end
# Walk the callback body and replace lookups on the source map.
#
# Replacement rules (key == callback parameter):
# var[key] → v
# Map.get(var, key) → v
# Map.get(var, key, default) → v
# Map.fetch(var, key) → {:ok, v}
# Map.fetch!(var, key) → v
defp replace_map_lookups(callback, map_var_name, key_var_name) do
Macro.prewalk(callback, fn
# var[key]
{{:., _, [Access, :get]}, _, [{name, _, _}, {key, _, _}]}
when name == map_var_name and key == key_var_name ->
{:v, [], nil}
# Map.get(var, key)
{{:., _, [{:__aliases__, _, [:Map]}, :get]}, _, [{name, _, _}, {key, _, _}]}
when name == map_var_name and key == key_var_name ->
{:v, [], nil}
# Map.get(var, key, _default)
{{:., _, [{:__aliases__, _, [:Map]}, :get]}, _, [{name, _, _}, {key, _, _}, _]}
when name == map_var_name and key == key_var_name ->
{:v, [], nil}
# Map.fetch(var, key) → {:ok, v}
{{:., _, [{:__aliases__, _, [:Map]}, :fetch]}, _, [{name, _, _}, {key, _, _}]}
when name == map_var_name and key == key_var_name ->
{:ok, {:v, [], nil}}
# Map.fetch!(var, key) → v
{{:., _, [{:__aliases__, _, [:Map]}, :fetch!]}, _, [{name, _, _}, {key, _, _}]}
when name == map_var_name and key == key_var_name ->
{:v, [], nil}
node ->
node
end)
end
# Pipeline form: Map.keys(var) |> Enum.xxx(fn ...)
# or: var |> Map.keys() |> Enum.xxx(fn ...)
defp check_node({:|>, meta, _} = node) do
pipeline = flatten_pipeline(node)
check_pipeline(pipeline, meta)
end
# Direct call form: Enum.xxx(Map.keys(var), fn ...)
defp check_node({{:., _, [mod, fn_name]}, meta, [first_arg, callback | _]})
when fn_name in @flagged_enum_fns do
with true <- enum_module?(mod),
{:ok, var_name} <- extract_map_keys_var(first_arg),
true <- references_map_var?(callback, var_name) do
{:ok, build_issue(fn_name, var_name, meta)}
else
_ -> :error
end
end
defp check_node(_), do: :error
defp check_pipeline(steps, meta) do
case check_two_step(steps, meta) do
{:ok, _} = result -> result
:error -> check_three_step(steps, meta)
end
end
# Pattern A: Map.keys(var) |> Enum.xxx(callback)
defp check_two_step([map_keys_call, enum_call | _], meta) do
with {:ok, var_name} <- extract_map_keys_var(map_keys_call),
{:ok, fn_name, callback} <- extract_piped_enum(enum_call),
true <- references_map_var?(callback, var_name) do
{:ok, build_issue(fn_name, var_name, meta)}
else
_ -> :error
end
end
defp check_two_step(_, _), do: :error
# Pattern B: var |> Map.keys() |> Enum.xxx(callback)
defp check_three_step([var_expr, map_keys_call, enum_call | _], meta) do
with {:ok, var_name} <- extract_simple_var(var_expr),
true <- map_keys_no_args?(map_keys_call),
{:ok, fn_name, callback} <- extract_piped_enum(enum_call),
true <- references_map_var?(callback, var_name) do
{:ok, build_issue(fn_name, var_name, meta)}
else
_ -> :error
end
end
defp check_three_step(_, _), do: :error
# Map.keys(var) — full call with one argument (check: returns name only)
defp extract_map_keys_var({{:., _, [mod, :keys]}, _, [arg]}) do
if map_module?(mod), do: extract_simple_var(arg), else: :error
end
defp extract_map_keys_var(_), do: :error
# Map.keys(var) — full call with one argument (fix: returns name + expr)
defp extract_map_keys_var_with_expr({{:., _, [mod, :keys]}, _, [arg]}) do
if map_module?(mod) do
case extract_simple_var(arg) do
{:ok, name} -> {:ok, name, arg}
:error -> :error
end
else
:error
end
end
defp extract_map_keys_var_with_expr(_), do: :error
# Map.keys() — pipeline form with no explicit args
defp map_keys_no_args?({{:., _, [mod, :keys]}, _, []}) do
map_module?(mod)
end
defp map_keys_no_args?(_), do: false
# Enum.xxx(callback) — pipeline form, one explicit arg
defp extract_piped_enum({{:., _, [mod, fn_name]}, _, [callback]})
when fn_name in @flagged_enum_fns do
if enum_module?(mod), do: {:ok, fn_name, callback}, else: :error
end
defp extract_piped_enum(_), do: :error
# Simple variable: {name, _, context}
defp extract_simple_var({name, _, ctx}) when is_atom(name) and is_atom(ctx) do
{:ok, name}
end
defp extract_simple_var(_), do: :error
defp references_map_var?(callback, target_var) do
{_, found} =
Macro.prewalk(callback, false, fn
node, true ->
{node, true}
{{:., _, [Access, :get]}, _, [{name, _, ctx}, _]} = node, acc
when is_atom(name) and is_atom(ctx) ->
if name == target_var, do: {node, true}, else: {node, acc}
{{:., _, [mod, fn_name]}, _, [{name, _, ctx} | _]} = node, acc
when is_atom(name) and is_atom(ctx) and fn_name in [:get, :fetch, :fetch!] ->
if map_module?(mod) and name == target_var,
do: {node, true},
else: {node, acc}
node, acc ->
{node, acc}
end)
found
end
defp flatten_pipeline({:|>, _, [left, right]}) do
flatten_pipeline(left) ++ [right]
end
defp flatten_pipeline(expr), do: [expr]
defp enum_module?({:__aliases__, _, [:Enum]}), do: true
defp enum_module?(_), do: false
defp map_module?({:__aliases__, _, [:Map]}), do: true
defp map_module?(_), do: false
defp build_issue(fn_name, var_name, meta) do
%Issue{
rule: :no_map_keys_enum_lookup,
message: build_message(fn_name, var_name),
meta: %{line: Keyword.get(meta, :line)}
}
end
defp build_message(fn_name, var_name) do
"""
`Map.keys(#{var_name})` piped into `Enum.#{fn_name}/2` while also \
looking up values from `#{var_name}` inside the callback.
Iterate the map directly to get both keys and values in one pass:
Enum.#{fn_name}(#{var_name}, fn {key, value} -> ... end)
"""
end
end