diff --git a/CHANGELOG.md b/CHANGELOG.md index 5403a0b5..d438a645 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,10 @@ ## main +### Improvements + +* `with`: remove identity singleton else clause (eg `else {:error, e} -> {:error, e} end`, `else error -> error end` + ## v0.10.1 ### Fixes diff --git a/lib/style/blocks.ex b/lib/style/blocks.ex index 8387c818..7485d1d5 100644 --- a/lib/style/blocks.ex +++ b/lib/style/blocks.ex @@ -79,29 +79,35 @@ defmodule Styler.Style.Blocks do {postroll, reversed_clauses} = Enum.split_while(reversed_clauses, &(not left_arrow?(&1))) [{:<-, _, [lhs, rhs]} = _final_clause | rest] = reversed_clauses - # Credo.Check.Refactor.RedundantWithClauseResult - rewrite_body? = Enum.empty?(postroll) and Enum.empty?(elses) and nodes_equivalent?(lhs, do_body) - {_, do_body_meta, _} = do_body + # drop singleton identity else clauses like `else foo -> foo end` + elses = + case elses do + [{{_, _, [:else]}, [{:->, _, [[left], right]}]}] -> if nodes_equivalent?(left, right), do: [], else: elses + _ -> elses + end {reversed_clauses, do_body} = - if rewrite_body?, - do: {rest, [rhs]}, - else: {reversed_clauses, Enum.reverse(postroll, [do_body])} - - do_else = [{do_block, {:__block__, do_body_meta, do_body}} | elses] - children = Enum.reverse(reversed_clauses, [do_else]) - - # only rewrite if it needs rewriting! - cond do - Enum.any?(preroll) -> - {:__block__, m, preroll ++ [{:with, m, children}]} - - rewrite_body? or Enum.any?(postroll) -> - {:with, m, children} - - true -> - with - end + cond do + # Put the postroll into the body + Enum.any?(postroll) -> + {_, do_body_meta, _} = do_body + do_body = {:__block__, do_body_meta, Enum.reverse(postroll, [do_body])} + {reversed_clauses, do_body} + + # Credo.Check.Refactor.RedundantWithClauseResult + Enum.empty?(elses) and nodes_equivalent?(lhs, do_body) -> + {rest, rhs} + + # no change + true -> + {reversed_clauses, do_body} + end + + children = Enum.reverse(reversed_clauses, [[{do_block, do_body} | elses]]) + + if Enum.any?(preroll), + do: {:__block__, m, preroll ++ [{:with, m, children}]}, + else: {:with, m, children} else # maybe this isn't a with statement - could be a function named `with` # or it's just a with statement with no arrows, but that's too saddening to imagine diff --git a/test/style/blocks_test.exs b/test/style/blocks_test.exs index cd9fc29e..d38b0c01 100644 --- a/test/style/blocks_test.exs +++ b/test/style/blocks_test.exs @@ -117,6 +117,25 @@ defmodule Styler.Style.BlocksTest do """) end + test "removes identity else clauses" do + assert_style( + """ + with :ok <- b(), :ok <- b() do + weeee() + :ok + else + :what -> :what + end + """, + """ + with :ok <- b(), :ok <- b() do + weeee() + :ok + end + """ + ) + end + test "Credo.Check.Readability.WithSingleClause" do assert_style( """ @@ -164,8 +183,6 @@ defmodule Styler.Style.BlocksTest do a = bop boop :horay! - else - :error -> :error end """ ) @@ -183,8 +200,6 @@ defmodule Styler.Style.BlocksTest do a = bop boop :horay! - else - :error -> :error end """ ) @@ -196,6 +211,8 @@ defmodule Styler.Style.BlocksTest do with {:ok, a} <- foo(), {:ok, b} <- bar(a) do {:ok, b} + else + error -> error end """, """