Current section

Files

Jump to
credence lib pattern no_redundant_assignment.ex
Raw

lib/pattern/no_redundant_assignment.ex

defmodule Credence.Pattern.NoRedundantAssignment do
@moduledoc """
Detects a variable (or tuple/list of plain variables) being assigned and
immediately returned as the last two statements of a block.
This is a common LLM verbosity pattern where the assignment adds no value.
In Elixir, the last expression in a block is its return value, so the
intermediate binding is unnecessary.
## Tier 1 — simple variable
# Bad
result = compute(x)
result
# Good
compute(x)
## Tier 2 — tuple/list of plain variables
# Bad
{a, b} = process(input)
{a, b}
# Good
process(input)
Patterns containing literals (e.g. `{:ok, result}`) are NOT fixed because
the match acts as an assertion — removing it would change error behavior.
Map patterns are never fixed because reconstruction produces a subset.
## Auto-fix
Replaces the last two statements with just the RHS of the assignment.
"""
use Credence.Pattern.Rule
alias Credence.Issue
@impl true
def fixable?, do: true
# ── Check ─────────────────────────────────────────────────────────
@impl true
def check(ast, _opts) do
{_ast, issues} =
Macro.prewalk(ast, [], fn
{:__block__, _meta, statements} = node, acc when is_list(statements) ->
case check_last_pair(statements) do
{:flag, issue} -> {node, [issue | acc]}
:clean -> {node, acc}
end
node, acc ->
{node, acc}
end)
Enum.reverse(issues)
end
# ── Fix ───────────────────────────────────────────────────────────
@impl true
def fix(source, _opts) do
case Sourceror.parse_string(source) do
{:ok, ast} ->
if has_fixable_block?(ast) do
ast
|> Macro.postwalk(&maybe_rewrite_block/1)
|> Sourceror.to_string()
else
source
end
{:error, _} ->
source
end
end
# ── Detection: last-pair check ────────────────────────────────────
# Checks if the last two statements in a block form a redundant
# assign-and-return pattern.
defp check_last_pair(statements) when length(statements) >= 2 do
second_to_last = Enum.at(statements, -2)
last = Enum.at(statements, -1)
case second_to_last do
{:=, meta, [lhs, _rhs]} ->
if fixable_pattern?(lhs) and structurally_identical?(lhs, last) do
{:flag, build_issue(meta)}
else
:clean
end
_ ->
:clean
end
end
defp check_last_pair(_), do: :clean
# ── Pattern classification ────────────────────────────────────────
# A pattern is fixable if it consists entirely of plain variables.
# Patterns with literals, pins, maps, or underscore are NOT fixable.
# Dispatch on type explicitly to avoid clause-matching ambiguity
# with cons cells and tuples.
defp fixable_pattern?(pattern) when is_tuple(pattern) do
case pattern do
# Sourceror __block__ wrapper — unwrap and retry
{:__block__, _, [inner]} -> fixable_pattern?(inner)
# Plain variable
{name, _, ctx} when is_atom(name) and is_atom(ctx) and name != :_ -> true
# 3+ element tuple: {:{}, _, elements}
{:{}, _, elements} when is_list(elements) -> Enum.all?(elements, &all_plain_variables?/1)
# 2-element tuple (only matches when tuple_size is 2)
{a, b} -> all_plain_variables?(a) and all_plain_variables?(b)
_ -> false
end
end
defp fixable_pattern?(pattern) when is_list(pattern) and pattern != [] do
fixable_list_or_cons?(pattern)
end
defp fixable_pattern?(_), do: false
# Recursively checks that every leaf in a pattern is a plain variable.
# Handles Sourceror __block__ wrapping and the cons operator {:|, _, [h, t]}.
defp all_plain_variables?({:__block__, _, [inner]}), do: all_plain_variables?(inner)
defp all_plain_variables?({:|, _, [head, tail]}),
do: all_plain_variables?(head) and all_plain_variables?(tail)
defp all_plain_variables?({name, _, ctx})
when is_atom(name) and is_atom(ctx) and name != :_,
do: true
defp all_plain_variables?(_), do: false
# Walks a list pattern (proper or cons cell) checking all leaves are plain variables.
defp fixable_list_or_cons?([head | tail]) do
all_plain_variables?(head) and
cond do
is_list(tail) and tail != [] -> fixable_list_or_cons?(tail)
tail == [] -> true
true -> all_plain_variables?(tail)
end
end
defp fixable_list_or_cons?(_), do: false
# ── Structural comparison ─────────────────────────────────────────
# Two AST nodes are structurally identical if they represent the
# same source code, ignoring position metadata. Using Macro.to_string
# as the normalizer handles all AST representation differences
# (2-tuples vs {:{}, _, _}, __block__ wrapping, cons cells, etc).
defp structurally_identical?(a, b) do
Macro.to_string(a) == Macro.to_string(b)
end
# ── Check: issue construction ─────────────────────────────────────
defp build_issue(meta) do
%Issue{
rule: :no_redundant_assignment,
message:
"Variable is assigned and immediately returned. " <>
"The assignment is redundant — return the expression directly.",
meta: %{line: Keyword.get(meta, :line)}
}
end
# ── Fix: block rewriting ─────────────────────────────────────────
defp has_fixable_block?(ast) do
{_, found} =
Macro.prewalk(ast, false, fn
_node, true ->
{nil, true}
{:__block__, _, statements} = node, false when is_list(statements) ->
{node, block_is_fixable?(statements)}
node, acc ->
{node, acc}
end)
found
end
defp block_is_fixable?(statements) when length(statements) >= 2 do
second_to_last = Enum.at(statements, -2)
last = Enum.at(statements, -1)
case second_to_last do
{:=, _, [lhs, _rhs]} ->
fixable_pattern?(lhs) and structurally_identical?(lhs, last)
_ ->
false
end
end
defp block_is_fixable?(_), do: false
# Postwalk callback: rewrites a __block__ if its last two statements
# form a redundant assign-and-return.
defp maybe_rewrite_block({:__block__, meta, statements} = node)
when is_list(statements) and length(statements) >= 2 do
second_to_last = Enum.at(statements, -2)
last = Enum.at(statements, -1)
case second_to_last do
{:=, _, [lhs, rhs]} ->
if fixable_pattern?(lhs) and structurally_identical?(lhs, last) do
{preceding, _last_two} = Enum.split(statements, length(statements) - 2)
{:__block__, meta, preceding ++ [rhs]}
else
node
end
_ ->
node
end
end
defp maybe_rewrite_block(node), do: node
end