Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
97 changes: 73 additions & 24 deletions lib/style/pipes.ex
Original file line number Diff line number Diff line change
Expand Up @@ -372,43 +372,55 @@ defmodule Styler.Style.Pipes do
when mod in @enum,
do: {:|>, pm, [lhs, {count, meta, [filterer]}]}

# `lhs |> Enum.filter(f1) |> Enum.filter(f2)` => `lhs |> Enum.filter(fn item -> f1.(item) && f2.(item) end)`
# `lhs |> Enum.filter(f1) |> Enum.filter(f2)` => `lhs |> Enum.filter(fn item -> f1_inlined && f2_inlined end)`
# (Credo.Check.Refactor.FilterFilter)
defp fix_pipe(
pipe_chain(
pm,
lhs,
{{:., _, [{_, _, [:Enum]}, :filter]} = filter, fm, [f1]},
{{:., _, [{_, _, [:Enum]}, :filter]}, _, [f2]}
)
),
do: {:|>, pm, [lhs, {filter, fm, [combined_predicate(f1, f2, :&&, fm)]}]}
) = node
) do
case combined_predicate(f1, f2, :&&, fm) do
nil -> node
combined -> {:|>, pm, [lhs, {filter, fm, [combined]}]}
end
end

# `lhs |> Enum.reject(f1) |> Enum.reject(f2)` => `lhs |> Enum.reject(fn item -> f1.(item) || f2.(item) end)`
# `lhs |> Enum.reject(f1) |> Enum.reject(f2)` => `lhs |> Enum.reject(fn item -> f1_inlined || f2_inlined end)`
# (Credo.Check.Refactor.RejectReject)
defp fix_pipe(
pipe_chain(
pm,
lhs,
{{:., _, [{_, _, [:Enum]}, :reject]} = reject, fm, [f1]},
{{:., _, [{_, _, [:Enum]}, :reject]}, _, [f2]}
)
),
do: {:|>, pm, [lhs, {reject, fm, [combined_predicate(f1, f2, :||, fm)]}]}
) = node
) do
case combined_predicate(f1, f2, :||, fm) do
nil -> node
combined -> {:|>, pm, [lhs, {reject, fm, [combined]}]}
end
end

# `lhs |> Enum.filter(f1) |> Enum.reject(f2)` => `lhs |> Enum.filter(fn item -> f1.(item) && !f2.(item) end)`
# `lhs |> Enum.filter(f1) |> Enum.reject(f2)` => `lhs |> Enum.filter(fn item -> f1_inlined && !f2_inlined end)`
# (Credo.Check.Refactor.FilterReject)
defp fix_pipe(
pipe_chain(
pm,
lhs,
{{:., _, [{_, _, [:Enum]}, :filter]} = filter, fm, [f1]},
{{:., _, [{_, _, [:Enum]}, :reject]}, _, [f2]}
)
),
do: {:|>, pm, [lhs, {filter, fm, [combined_predicate(f1, f2, :&&, fm, negate_f2: true)]}]}
) = node
) do
case combined_predicate(f1, f2, :&&, fm, negate_f2: true) do
nil -> node
combined -> {:|>, pm, [lhs, {filter, fm, [combined]}]}
end
end

# `lhs |> Enum.reject(f1) |> Enum.filter(f2)` => `lhs |> Enum.filter(fn item -> !f1.(item) && f2.(item) end)`
# `lhs |> Enum.reject(f1) |> Enum.filter(f2)` => `lhs |> Enum.filter(fn item -> !f1_inlined && f2_inlined end)`
# The merged call collapses to `Enum.filter` (as Credo recommends) — `f1` was the original reject,
# so we negate it; `f2` was the original filter, so it stays.
# (Credo.Check.Refactor.RejectFilter)
Expand All @@ -418,9 +430,13 @@ defmodule Styler.Style.Pipes do
lhs,
{{:., _, [{_, _, [:Enum]}, :reject]}, fm, [f1]},
{{:., _, [{_, _, [:Enum]}, :filter]} = filter, _, [f2]}
)
),
do: {:|>, pm, [lhs, {filter, fm, [combined_predicate(f1, f2, :&&, fm, negate_f1: true)]}]}
) = node
) do
case combined_predicate(f1, f2, :&&, fm, negate_f1: true) do
nil -> node
combined -> {:|>, pm, [lhs, {filter, fm, [combined]}]}
end
end

# `lhs |> Enum.map(f1) |> Enum.map(f2)` => single `Enum.map` whose body is the inlined nested call. We seed the body
# with a one-step pipe inside f1's slot - Styler's existing `f(pipe, args)` walk then unfolds the f2 call into the
Expand Down Expand Up @@ -558,19 +574,52 @@ defmodule Styler.Style.Pipes do
defp valid_pipe_start?({fun, _, _args}) when is_atom(fun), do: String.match?("#{fun}", ~r/^sigil_[a-zA-Z]$/)
defp valid_pipe_start?(_), do: true

# Combines two 1-arity predicates into a single anonymous function: `fn item -> f1.(item) <op> f2.(item) end`.
# Universal form that's correct regardless of whether each predicate is a capture, an `&(...)` shortform,
# or an explicit `fn x -> ... end`. Used by FilterFilter (op: `&&`), RejectReject (op: `||`), and
# the mixed FilterReject / RejectFilter rules (op: `&&` with one side wrapped in `!`).
# Combines two 1-arity predicates into a single anonymous function: `fn item -> f1_term <op> f2_term end`.
# Returns nil if either side can't be combined cleanly — the caller skips the rewrite. We inline
# captures and inline-fn predicates whose body is a single expression (so combining with `&&`/`||`
# is sound). Bare references (like `f1`) fall back to `f1.(item)`. Anything else (multi-statement
# `fn` bodies, captures with multiple `&1` uses, nested closures) is left alone — the original
# two-pipe form is more readable than what we'd produce. Used by FilterFilter (op: `&&`),
# RejectReject (op: `||`), and the mixed FilterReject / RejectFilter rules.
defp combined_predicate(f1, f2, op, m, opts \\ []) do
line = m[:line]
item = {:item, [line: line], nil}
call_f1 = maybe_negate(predicate_call(f1, item, line), opts[:negate_f1] == true, line)
call_f2 = maybe_negate(predicate_call(f2, item, line), opts[:negate_f2] == true, line)
body = {op, [line: line], [call_f1, call_f2]}
{:fn, [closing: [line: line], line: line], [{:->, [line: line], [[item], body]}]}

with call_f1 when not is_nil(call_f1) <- predicate_term(f1, item, line),
call_f2 when not is_nil(call_f2) <- predicate_term(f2, item, line) do
call_f1 = maybe_negate(call_f1, opts[:negate_f1] == true, line)
call_f2 = maybe_negate(call_f2, opts[:negate_f2] == true, line)
body = {op, [line: line], [call_f1, call_f2]}
{:fn, [closing: [line: line], line: line], [{:->, [line: line], [[item], body]}]}
end
Comment thread
cursor[bot] marked this conversation as resolved.
end

defp predicate_term(fun, item, line) do
cond do
inlineable_simple?(fun) -> Style.set_line(inline_capture(fun, item, line), line)
bare_reference?(fun) -> predicate_call(fun, item, line)
true -> nil
end
end

# Inlineable AND the body is a single expression. Multi-statement `fn`/`&` bodies don't compose
# cleanly under `&&`/`||` — short-circuit semantics would change and the formatted output is uglier
# than the original two-pipe chain.
defp inlineable_simple?({:&, _, [{:/, _, _}]} = fun), do: inlineable?(fun)

defp inlineable_simple?({:&, _, [body]} = fun), do: inlineable?(fun) and not multi_statement_block?(body)

defp inlineable_simple?({:fn, _, [{:->, _, [_, body]}]} = fun),
do: inlineable?(fun) and not multi_statement_block?(body)

defp inlineable_simple?(_), do: false

defp multi_statement_block?({:__block__, _, [_, _ | _]}), do: true
defp multi_statement_block?(_), do: false

defp bare_reference?({var, _, ctx}) when is_atom(var) and is_atom(ctx), do: true
defp bare_reference?(_), do: false

defp maybe_negate(call, true, line), do: {:!, [line: line], [call]}
defp maybe_negate(call, false, _line), do: call

Expand Down
39 changes: 37 additions & 2 deletions test/style/pipes_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -656,19 +656,54 @@ defmodule Styler.Style.PipesTest do
)
end

test "FilterFilter: combines captures and inline predicates" do
test "FilterFilter: inlines captures and inline-fn predicates" do
assert_style(
"""
list
|> Enum.filter(&String.contains?(&1, "x"))
|> Enum.filter(fn s -> String.contains?(s, "a") end)
""",
"""
Enum.filter(list, fn item -> (&String.contains?(&1, "x")).(item) && (fn s -> String.contains?(s, "a") end).(item) end)
Enum.filter(list, fn item -> String.contains?(item, "x") && String.contains?(item, "a") end)
"""
)
end

test "RejectFilter: inlines `&fun/1` style captures" do
assert_style(
"""
list
|> Enum.reject(&is_nil/1)
|> Enum.filter(&is_map/1)
""",
"""
Enum.filter(list, fn item -> not is_nil(item) && is_map(item) end)
"""
)
end

test "FilterFilter / RejectFilter: skips rewrite when a predicate has a multi-statement fn body" do
# The collapsed lambda would be uglier than the original chain — leave it alone.
assert_style("""
schema_modules
|> Enum.reject(&(&1 in context.ignore_list))
|> Enum.filter(fn schema_module ->
schema = schema_module.schema()
schema_title = schema.title
not is_nil(schema.example) and not MapSet.member?(set, schema_title)
end)
""")

assert_style("""
list
|> Enum.filter(fn x ->
y = x.a
y > 0
end)
|> Enum.filter(f2)
""")
end

test "FilterFilter: leaves non-Enum.filter chains alone" do
assert_style("""
list
Expand Down
Loading