Current section

Files

Jump to
credence lib pattern no_enum_drop_negative.ex
Raw

lib/pattern/no_enum_drop_negative.ex

defmodule Credence.Pattern.NoEnumDropNegative do
@moduledoc """
Performance rule: Detects `Enum.drop(list, -n)` where `n` is a positive
integer literal.
For linked lists, `Enum.drop(list, -n)` must traverse to the end of the
list to figure out where to cut, making it O(n). This often indicates
the algorithm should be restructured to avoid needing to trim from the
tail of a linked list.
The auto-fix replaces `Enum.drop(list, -n)` with `Enum.slice(list, 0..-(n+1)//1)`,
which has equivalent semantics. If performance is critical, consider
restructuring to avoid tail-trimming entirely.
## Bad
list |> Enum.drop(-1)
Enum.drop(list, -3)
## Good
# If building the list yourself, drop the head before reversing:
[_ | rest] = reversed_list
Enum.reverse(rest)
# Or use Enum.slice/2 if you know the desired length:
Enum.slice(list, 0..-2//1)
"""
use Credence.Pattern.Rule
alias Credence.Issue
@impl true
def fixable?, do: true
@impl true
def check(ast, _opts) do
{_ast, issues} =
Macro.prewalk(ast, [], fn
# Direct: Enum.drop(list, -1)
{{:., _, [{:__aliases__, _, [:Enum]}, :drop]}, meta, [_, {:-, _, [n]}]} = node, issues
when is_integer(n) and n > 0 ->
{node, [build_issue(n, meta) | issues]}
# Piped: list |> Enum.drop(-1)
{{:., _, [{:__aliases__, _, [:Enum]}, :drop]}, meta, [{:-, _, [n]}]} = node, issues
when is_integer(n) and n > 0 ->
{node, [build_issue(n, meta) | issues]}
node, issues ->
{node, issues}
end)
Enum.reverse(issues)
end
@impl true
def fix(source, _opts) do
source
|> Sourceror.parse_string!()
|> Macro.postwalk(fn
# Direct: Enum.drop(list, -n)
{{:., _, [{:__aliases__, _, [:Enum]}, :drop]}, _, [list_arg, second]} = node ->
case extract_negative(second) do
{:ok, n} -> enum_slice_call(list_arg, n)
:error -> node
end
# Piped: |> Enum.drop(-n)
{{:., _, [{:__aliases__, _, [:Enum]}, :drop]}, _, [single]} = node ->
case extract_negative(single) do
{:ok, n} -> enum_slice_piped(n)
:error -> node
end
node ->
node
end)
|> Sourceror.to_string()
end
# Sourceror wraps literals in {:__block__, meta, [value]}, so -1 becomes:
# {:-, meta, [{:__block__, meta, [1]}]}
# Code.string_to_quoted produces the simpler:
# {:-, meta, [1]}
# We handle both, plus a bare negative integer just in case.
defp extract_negative({:-, _, [{:__block__, _, [n]}]}) when is_integer(n) and n > 0,
do: {:ok, n}
defp extract_negative({:-, _, [n]}) when is_integer(n) and n > 0,
do: {:ok, n}
defp extract_negative(n) when is_integer(n) and n < 0,
do: {:ok, abs(n)}
defp extract_negative(_), do: :error
# Enum.slice(list_arg, 0..-(n+1)//1)
defp enum_slice_call(list_arg, n) do
{{:., [], [{:__aliases__, [], [:Enum]}, :slice]}, [], [list_arg, build_range(n)]}
end
# Enum.slice(0..-(n+1)//1) — for piped usage
defp enum_slice_piped(n) do
{{:., [], [{:__aliases__, [], [:Enum]}, :slice]}, [], [build_range(n)]}
end
# Builds AST for 0..-(n+1)//1
# The :..// operator takes three flat args: start, end, step
defp build_range(n) do
{:..//, [], [0, -(n + 1), 1]}
end
defp build_issue(n, meta) do
%Issue{
rule: :no_enum_drop_negative,
message:
"`Enum.drop(list, -#{n})` traverses the entire list to drop from the end. " <>
"Restructure the algorithm to avoid tail-trimming on linked lists, " <>
"or drop from the head of a reversed list instead.",
meta: %{line: Keyword.get(meta, :line)}
}
end
end