Skip to content

Commit 33551cc

Browse files
committed
MapMap: prefer f2's iter-var name when only f2 names it
Reported by Cursor Bugbot: the comment on iteration_var_name said "if either side is an inline fn x -> ...", but the function only inspected f1. When f1 was a capture (e.g. &Mod.foo/1) and f2 was fn descriptive_name -> ..., the merged lambda defaulted to :arg1 instead of using f2's more meaningful name.
1 parent 1a55af9 commit 33551cc

2 files changed

Lines changed: 12 additions & 8 deletions

File tree

lib/style/pipes.ex

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -436,7 +436,7 @@ defmodule Styler.Style.Pipes do
436436
) = node
437437
) do
438438
with true <- inlineable?(f1) and inlineable?(f2) and placeholder_in_first_position?(f2),
439-
item_name = iteration_var_name(f1),
439+
item_name = iteration_var_name(f1, f2),
440440
false <- shadows_free_var?(item_name, f1, f2),
441441
item = {item_name, [line: fm[:line]], nil},
442442
inlined_f1 = inline_capture(f1, item, fm[:line]),
@@ -629,11 +629,13 @@ defmodule Styler.Style.Pipes do
629629
defp inlineable?(_), do: false
630630

631631
# If either side is an inline `fn x -> ...`, prefer that var name for the merged lambda - the source already named the
632-
# iteration value. Otherwise, fall back to `arg1`.
633-
defp iteration_var_name({:fn, _, [{:->, _, [[{name, _, ctx}], _]}]}) when is_atom(name) and is_atom(ctx) and name != :_,
632+
# iteration value. Prefer f1's name when both are named. Otherwise, fall back to `arg1`.
633+
defp iteration_var_name(f1, f2), do: fn_var_name(f1) || fn_var_name(f2) || :arg1
634+
635+
defp fn_var_name({:fn, _, [{:->, _, [[{name, _, ctx}], _]}]}) when is_atom(name) and is_atom(ctx) and name != :_,
634636
do: name
635637

636-
defp iteration_var_name(_), do: :arg1
638+
defp fn_var_name(_), do: nil
637639

638640
# The merged lambda introduces a fresh binding for `name`. If that same name appears as a free variable in either
639641
# side's body, it referred to a closure binding in the source - after merging, the new lambda's parameter would shadow

test/style/pipes_test.exs

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -790,15 +790,16 @@ defmodule Styler.Style.PipesTest do
790790
test "MapMap: pivots on the first arg of f1 when its placeholder isn't there" do
791791
# User case from billing_core/.../process_bloomberg_import.ex. Both sides are inlineable;
792792
# f1 puts the placeholder in position 2, so the merge pivots on `import_record` (f1's first
793-
# arg) and the iter-var defaults to `arg1` since neither side is an inline `fn x -> ...`.
793+
# arg). f1 is a capture so contributes no name; the merged lambda picks up `changeset` from
794+
# f2's `fn changeset -> ...`.
794795
assert_style(
795796
"""
796797
list
797798
|> Enum.map(&build_changeset(import_record, &1))
798799
|> Enum.map(fn changeset -> Repo.insert(changeset, on_conflict: :nothing) end)
799800
""",
800801
"""
801-
Enum.map(list, fn arg1 -> import_record |> build_changeset(arg1) |> Repo.insert(on_conflict: :nothing) end)
802+
Enum.map(list, fn changeset -> import_record |> build_changeset(changeset) |> Repo.insert(on_conflict: :nothing) end)
802803
"""
803804
)
804805
end
@@ -869,11 +870,12 @@ defmodule Styler.Style.PipesTest do
869870
|> Enum.map(&apply_with(&1, config))
870871
""")
871872

872-
# Default `:arg1` iter-var case: a closure named `arg1` in f1 must also block the merge.
873+
# Default `:arg1` iter-var case (neither side names its iter-var): a closure named `arg1` in
874+
# f1 must also block the merge.
873875
assert_style("""
874876
list
875877
|> Enum.map(&build_changeset(arg1, &1))
876-
|> Enum.map(fn cs -> Repo.insert(cs) end)
878+
|> Enum.map(&Repo.insert/1)
877879
""")
878880
end
879881

0 commit comments

Comments
 (0)