with statement: drop singleton identity else. closes #84

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

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

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
         """,
         """