From 8138e3162053f3194f3e4e4884957345c260678e Mon Sep 17 00:00:00 2001 From: Andrew Haust Date: Wed, 26 Jun 2024 21:01:59 -0400 Subject: [PATCH 01/19] Basic case passes --- lib/recode/task/comparisons.ex | 75 +++++++++++++++++++++++++++ test/recode/task/comparisons_test.exs | 25 +++++++++ 2 files changed, 100 insertions(+) create mode 100644 lib/recode/task/comparisons.ex create mode 100644 test/recode/task/comparisons_test.exs diff --git a/lib/recode/task/comparisons.ex b/lib/recode/task/comparisons.ex new file mode 100644 index 0000000..7e4cf23 --- /dev/null +++ b/lib/recode/task/comparisons.ex @@ -0,0 +1,75 @@ +defmodule Recode.Task.Comparisons do + @shortdoc "Removes extraneous conditionals" + + @moduledoc """ + Multi aliases makes module uses harder to search for in large code bases. + + # preferred + foo == bar + + # not preferred + if foo == bar do + true + else + false + end + + This task rewrites the code when `mix recode` runs with `autocorrect: true`. + """ + + use Recode.Task, corrector: true, category: :readability + + # alias Recode.Issue + alias Recode.Task.Comparisons + alias Rewrite.Source + alias Sourceror.Zipper + + @impl Recode.Task + def run(source, opts) do + {zipper, issues} = + source + |> Source.get(:quoted) + |> Zipper.zip() + |> Zipper.traverse([], fn zipper, issues -> + simplify_comparison(zipper, issues, opts[:autocorrect]) + end) + + case opts[:autocorrect] do + true -> + Source.update(source, Comparisons, :quoted, Zipper.root(zipper)) + + false -> + Source.add_issues(source, issues) + end + end + + defp simplify_comparison(%Zipper{node: {conditional, _, args}} = zipper, issues, true) + when conditional == :if do + case args do + [ + expr, + [ + {{:__block__, _, [:do]}, + {:__block__, _, [true]}}, + {{:__block__, _, [:else]}, + {:__block__, _, [false]}} + ] + ] -> + {Zipper.replace(zipper, expr), issues} + + _ -> + {zipper, issues} + end + end + + defp simplify_comparison( + %Zipper{node: {conditional, _, _args} = _ast} = zipper, + issues, + false + ) + when conditional in [:if, :unless] do + {zipper, issues} + end + + defp simplify_comparison(zipper, issues, _autocorrect), do: {zipper, issues} +end diff --git a/test/recode/task/comparisons_test.exs b/test/recode/task/comparisons_test.exs new file mode 100644 index 0000000..777c7ac --- /dev/null +++ b/test/recode/task/comparisons_test.exs @@ -0,0 +1,25 @@ +defmodule Recode.Task.ComparisonsTest do + use RecodeCase + + alias Recode.Task.Comparisons + + describe "run/1" do + test "equals" do + code = """ + if foo == bar do + true + else + false + end + """ + + expected = """ + foo == bar + """ + + code + |> run_task(Comparisons, autocorrect: true) + |> assert_code(expected) + end + end +end From 541c9be8c0a738a8a0217cb525ca1c937da94735 Mon Sep 17 00:00:00 2001 From: Andrew Haust Date: Wed, 26 Jun 2024 21:04:52 -0400 Subject: [PATCH 02/19] Add some test cases --- test/recode/task/comparisons_test.exs | 36 +++++++++++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/test/recode/task/comparisons_test.exs b/test/recode/task/comparisons_test.exs index 777c7ac..6de5c35 100644 --- a/test/recode/task/comparisons_test.exs +++ b/test/recode/task/comparisons_test.exs @@ -21,5 +21,41 @@ defmodule Recode.Task.ComparisonsTest do |> run_task(Comparisons, autocorrect: true) |> assert_code(expected) end + + test "greater than" do + code = """ + if foo > bar do + true + else + false + end + """ + + expected = """ + foo > bar + """ + + code + |> run_task(Comparisons, autocorrect: true) + |> assert_code(expected) + end + + test "predicate" do + code = """ + if foo?(bar) do + true + else + false + end + """ + + expected = """ + foo?(bar) + """ + + code + |> run_task(Comparisons, autocorrect: true) + |> assert_code(expected) + end end end From 390cdca28f3d60fc0a6cf1fb5008e2df7d24b242 Mon Sep 17 00:00:00 2001 From: Andrew Haust Date: Wed, 26 Jun 2024 21:09:34 -0400 Subject: [PATCH 03/19] Keeps code as is --- test/recode/task/comparisons_test.exs | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/test/recode/task/comparisons_test.exs b/test/recode/task/comparisons_test.exs index 6de5c35..0426c81 100644 --- a/test/recode/task/comparisons_test.exs +++ b/test/recode/task/comparisons_test.exs @@ -22,9 +22,9 @@ defmodule Recode.Task.ComparisonsTest do |> assert_code(expected) end - test "greater than" do + test "predicate" do code = """ - if foo > bar do + if foo?(bar) do true else false @@ -32,7 +32,7 @@ defmodule Recode.Task.ComparisonsTest do """ expected = """ - foo > bar + foo?(bar) """ code @@ -40,17 +40,21 @@ defmodule Recode.Task.ComparisonsTest do |> assert_code(expected) end - test "predicate" do + test "keeps code as is" do code = """ if foo?(bar) do - true - else false + else + true end """ expected = """ - foo?(bar) + if foo?(bar) do + false + else + true + end """ code From cac86fbb3de7b1ebf964d5be83f32a6596ab2507 Mon Sep 17 00:00:00 2001 From: Andrew Haust Date: Wed, 26 Jun 2024 21:19:53 -0400 Subject: [PATCH 04/19] Refactor and cleanup --- lib/recode/task/comparisons.ex | 36 ++++++++++++++++------------------ 1 file changed, 17 insertions(+), 19 deletions(-) diff --git a/lib/recode/task/comparisons.ex b/lib/recode/task/comparisons.ex index 7e4cf23..e390b8e 100644 --- a/lib/recode/task/comparisons.ex +++ b/lib/recode/task/comparisons.ex @@ -43,33 +43,31 @@ defmodule Recode.Task.Comparisons do end end - defp simplify_comparison(%Zipper{node: {conditional, _, args}} = zipper, issues, true) - when conditional == :if do - case args do - [ - expr, - [ - {{:__block__, _, [:do]}, - {:__block__, _, [true]}}, - {{:__block__, _, [:else]}, - {:__block__, _, [false]}} - ] - ] -> + defp simplify_comparison(%Zipper{node: {:if, _, body}} = zipper, issues, true) do + case extract(body) do + {:ok, expr} -> {Zipper.replace(zipper, expr), issues} - _ -> + :error -> {zipper, issues} end end - defp simplify_comparison( - %Zipper{node: {conditional, _, _args} = _ast} = zipper, - issues, - false - ) - when conditional in [:if, :unless] do + defp simplify_comparison(%Zipper{node: {:if, _, _args} = _ast} = zipper, issues, false) do {zipper, issues} end defp simplify_comparison(zipper, issues, _autocorrect), do: {zipper, issues} + + defp extract([ + expr, + [ + {{:__block__, _, [:do]}, {:__block__, _, [true]}}, + {{:__block__, _, [:else]}, {:__block__, _, [false]}} + ] + ]) do + {:ok, expr} + end + + defp extract(_), do: :error end From 9517d97b1613e079b8c79e473dfb1d97f9a7fb20 Mon Sep 17 00:00:00 2001 From: Andrew Haust Date: Wed, 26 Jun 2024 21:28:56 -0400 Subject: [PATCH 05/19] Report issues --- lib/recode/task/comparisons.ex | 15 +++++++++++++-- test/recode/task/comparisons_test.exs | 20 ++++++++++++++++++++ 2 files changed, 33 insertions(+), 2 deletions(-) diff --git a/lib/recode/task/comparisons.ex b/lib/recode/task/comparisons.ex index e390b8e..7bcf0ba 100644 --- a/lib/recode/task/comparisons.ex +++ b/lib/recode/task/comparisons.ex @@ -19,7 +19,7 @@ defmodule Recode.Task.Comparisons do use Recode.Task, corrector: true, category: :readability - # alias Recode.Issue + alias Recode.Issue alias Recode.Task.Comparisons alias Rewrite.Source alias Sourceror.Zipper @@ -53,7 +53,18 @@ defmodule Recode.Task.Comparisons do end end - defp simplify_comparison(%Zipper{node: {:if, _, _args} = _ast} = zipper, issues, false) do + defp simplify_comparison(%Zipper{node: {:if, meta, body}} = zipper, issues, false) do + issues = + case extract(body) do + {:ok, _expr} -> + message = "Avoid if true else false" + issue = Issue.new(Comparisons, message, meta) + [issue | issues] + + :error -> + issues + end + {zipper, issues} end diff --git a/test/recode/task/comparisons_test.exs b/test/recode/task/comparisons_test.exs index 0426c81..2b87361 100644 --- a/test/recode/task/comparisons_test.exs +++ b/test/recode/task/comparisons_test.exs @@ -61,5 +61,25 @@ defmodule Recode.Task.ComparisonsTest do |> run_task(Comparisons, autocorrect: true) |> assert_code(expected) end + + test "reports no issues" do + """ + foo == bar + """ + |> run_task(Comparisons, autocorrect: false) + |> refute_issues() + end + + test "reports an issue" do + """ + if foo == bar do + true + else + false + end + """ + |> run_task(Comparisons, autocorrect: false) + |> assert_issue_with(reporter: Comparisons) + end end end From 07e20ae8ce7067f7de04f651879ad5ed5cf770a5 Mon Sep 17 00:00:00 2001 From: Andrew Haust Date: Wed, 26 Jun 2024 21:35:31 -0400 Subject: [PATCH 06/19] Rename task --- .../{comparisons.ex => redundant_booleans.ex} | 12 ++++++------ ...sons_test.exs => redundant_booleans_test.exs} | 16 ++++++++-------- 2 files changed, 14 insertions(+), 14 deletions(-) rename lib/recode/task/{comparisons.ex => redundant_booleans.ex} (83%) rename test/recode/task/{comparisons_test.exs => redundant_booleans_test.exs} (70%) diff --git a/lib/recode/task/comparisons.ex b/lib/recode/task/redundant_booleans.ex similarity index 83% rename from lib/recode/task/comparisons.ex rename to lib/recode/task/redundant_booleans.ex index 7bcf0ba..d4f9270 100644 --- a/lib/recode/task/comparisons.ex +++ b/lib/recode/task/redundant_booleans.ex @@ -1,8 +1,8 @@ -defmodule Recode.Task.Comparisons do - @shortdoc "Removes extraneous conditionals" +defmodule Recode.Task.RedundantBooleans do + @shortdoc "Removes redundant booleans" @moduledoc """ - Multi aliases makes module uses harder to search for in large code bases. + Redudant booleans make code needlesly verbose. # preferred foo == bar @@ -20,7 +20,7 @@ defmodule Recode.Task.Comparisons do use Recode.Task, corrector: true, category: :readability alias Recode.Issue - alias Recode.Task.Comparisons + alias Recode.Task.RedundantBooleans alias Rewrite.Source alias Sourceror.Zipper @@ -36,7 +36,7 @@ defmodule Recode.Task.Comparisons do case opts[:autocorrect] do true -> - Source.update(source, Comparisons, :quoted, Zipper.root(zipper)) + Source.update(source, RedundantBooleans, :quoted, Zipper.root(zipper)) false -> Source.add_issues(source, issues) @@ -58,7 +58,7 @@ defmodule Recode.Task.Comparisons do case extract(body) do {:ok, _expr} -> message = "Avoid if true else false" - issue = Issue.new(Comparisons, message, meta) + issue = Issue.new(RedundantBooleans, message, meta) [issue | issues] :error -> diff --git a/test/recode/task/comparisons_test.exs b/test/recode/task/redundant_booleans_test.exs similarity index 70% rename from test/recode/task/comparisons_test.exs rename to test/recode/task/redundant_booleans_test.exs index 2b87361..623e952 100644 --- a/test/recode/task/comparisons_test.exs +++ b/test/recode/task/redundant_booleans_test.exs @@ -1,7 +1,7 @@ -defmodule Recode.Task.ComparisonsTest do +defmodule Recode.Task.RedundantBooleansTest do use RecodeCase - alias Recode.Task.Comparisons + alias Recode.Task.RedundantBooleans describe "run/1" do test "equals" do @@ -18,7 +18,7 @@ defmodule Recode.Task.ComparisonsTest do """ code - |> run_task(Comparisons, autocorrect: true) + |> run_task(RedundantBooleans, autocorrect: true) |> assert_code(expected) end @@ -36,7 +36,7 @@ defmodule Recode.Task.ComparisonsTest do """ code - |> run_task(Comparisons, autocorrect: true) + |> run_task(RedundantBooleans, autocorrect: true) |> assert_code(expected) end @@ -58,7 +58,7 @@ defmodule Recode.Task.ComparisonsTest do """ code - |> run_task(Comparisons, autocorrect: true) + |> run_task(RedundantBooleans, autocorrect: true) |> assert_code(expected) end @@ -66,7 +66,7 @@ defmodule Recode.Task.ComparisonsTest do """ foo == bar """ - |> run_task(Comparisons, autocorrect: false) + |> run_task(RedundantBooleans, autocorrect: false) |> refute_issues() end @@ -78,8 +78,8 @@ defmodule Recode.Task.ComparisonsTest do false end """ - |> run_task(Comparisons, autocorrect: false) - |> assert_issue_with(reporter: Comparisons) + |> run_task(RedundantBooleans, autocorrect: false) + |> assert_issue_with(reporter: RedundantBooleans) end end end From ae9f714375f0dc1d4d82c98db7186ce7ac1f47a9 Mon Sep 17 00:00:00 2001 From: Andrew Haust Date: Wed, 26 Jun 2024 21:47:55 -0400 Subject: [PATCH 07/19] Add more tests --- test/recode/task/redundant_booleans_test.exs | 38 +++++++++++++++++++- 1 file changed, 37 insertions(+), 1 deletion(-) diff --git a/test/recode/task/redundant_booleans_test.exs b/test/recode/task/redundant_booleans_test.exs index 623e952..0853cf2 100644 --- a/test/recode/task/redundant_booleans_test.exs +++ b/test/recode/task/redundant_booleans_test.exs @@ -40,7 +40,21 @@ defmodule Recode.Task.RedundantBooleansTest do |> assert_code(expected) end - test "keeps code as is" do + test "keyword syntax" do + code = """ + if foo?(bar), do: true, else: false + """ + + expected = """ + foo?(bar) + """ + + code + |> run_task(RedundantBooleans, autocorrect: true) + |> assert_code(expected) + end + + test "keeps code as is with reverse booleans" do code = """ if foo?(bar) do false @@ -62,6 +76,28 @@ defmodule Recode.Task.RedundantBooleansTest do |> assert_code(expected) end + test "keeps code as is without booleans" do + code = """ + if foo?(bar) do + bar + else + false + end + """ + + expected = """ + if foo?(bar) do + bar + else + false + end + """ + + code + |> run_task(RedundantBooleans, autocorrect: true) + |> assert_code(expected) + end + test "reports no issues" do """ foo == bar From 90aeee475ad9137f4b5472a59298dbb98191696c Mon Sep 17 00:00:00 2001 From: Andrew Haust Date: Wed, 26 Jun 2024 21:59:04 -0400 Subject: [PATCH 08/19] Add Docs and config gen --- README.md | 2 ++ lib/recode/config.ex | 1 + lib/recode/task/redundant_booleans.ex | 2 +- 3 files changed, 4 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index 85a5392..452c63e 100644 --- a/README.md +++ b/README.md @@ -31,6 +31,7 @@ Specs # Checker - Checks for specs. Refactor tasks: FilterCount # Corrector - Checks calls like Enum.filter(...) |> Enum.count(). Nesting # Checker - Checks code nesting depth in functions and macros. +RedundantBooleans # Corrector - Removes `do: true, else: false` Warning tasks: Dbg # Corrector - There should be no calls to dbg. IOInspect # Corrector - There should be no calls to IO.inspect. @@ -109,6 +110,7 @@ This mix task generates the config file `.recode.exs`. {Recode.Task.IOInspect, [autocorrect: false]}, {Recode.Task.Nesting, []}, {Recode.Task.PipeFunOne, []}, + {Recode.Task.RedundantBooleans, []}, {Recode.Task.SinglePipe, []}, {Recode.Task.Specs, [exclude: "test/**/*.{ex,exs}", config: [only: :visible]]}, {Recode.Task.TagFIXME, [exit_code: 2]}, diff --git a/lib/recode/config.ex b/lib/recode/config.ex index 8d48050..e2ffa5b 100644 --- a/lib/recode/config.ex +++ b/lib/recode/config.ex @@ -37,6 +37,7 @@ defmodule Recode.Config do # {Recode.Task.Moduledoc, []}, {Recode.Task.Nesting, []}, {Recode.Task.PipeFunOne, []}, + {Recode.Task.RedundantBooleans, []}, {Recode.Task.SinglePipe, []}, {Recode.Task.Specs, [exclude: ["test/**/*.{ex,exs}", "mix.exs"], config: [only: :visible]]}, {Recode.Task.TagFIXME, exit_code: 2}, diff --git a/lib/recode/task/redundant_booleans.ex b/lib/recode/task/redundant_booleans.ex index d4f9270..be41741 100644 --- a/lib/recode/task/redundant_booleans.ex +++ b/lib/recode/task/redundant_booleans.ex @@ -57,7 +57,7 @@ defmodule Recode.Task.RedundantBooleans do issues = case extract(body) do {:ok, _expr} -> - message = "Avoid if true else false" + message = "Avoid `do: true, else: false`" issue = Issue.new(RedundantBooleans, message, meta) [issue | issues] From db6eff1bbc9da30fbf340339309e9530ef3393ed Mon Sep 17 00:00:00 2001 From: Andrew Haust Date: Thu, 27 Jun 2024 18:02:13 -0400 Subject: [PATCH 09/19] Maintain leading comments --- lib/recode/task/redundant_booleans.ex | 11 +++++- test/recode/task/redundant_booleans_test.exs | 38 ++++++++++++++++++++ 2 files changed, 48 insertions(+), 1 deletion(-) diff --git a/lib/recode/task/redundant_booleans.ex b/lib/recode/task/redundant_booleans.ex index be41741..f207df8 100644 --- a/lib/recode/task/redundant_booleans.ex +++ b/lib/recode/task/redundant_booleans.ex @@ -43,9 +43,13 @@ defmodule Recode.Task.RedundantBooleans do end end - defp simplify_comparison(%Zipper{node: {:if, _, body}} = zipper, issues, true) do + defp simplify_comparison(%Zipper{node: {:if, meta, body}} = zipper, issues, true) do case extract(body) do {:ok, expr} -> + expr = + expr + |> put_leading_comments(meta) + {Zipper.replace(zipper, expr), issues} :error -> @@ -81,4 +85,9 @@ defmodule Recode.Task.RedundantBooleans do end defp extract(_), do: :error + + defp put_leading_comments(expr, meta) do + comments = meta[:leading_comments] || [] + Sourceror.append_comments(expr, comments) + end end diff --git a/test/recode/task/redundant_booleans_test.exs b/test/recode/task/redundant_booleans_test.exs index 0853cf2..b6a21e3 100644 --- a/test/recode/task/redundant_booleans_test.exs +++ b/test/recode/task/redundant_booleans_test.exs @@ -76,6 +76,44 @@ defmodule Recode.Task.RedundantBooleansTest do |> assert_code(expected) end + test "maintains leading comments" do + code = """ + # I'm a comment + if foo?(bar) do + true + else + false + end + """ + + expected = """ + # I'm a comment + foo?(bar) + """ + + code + |> run_task(RedundantBooleans, autocorrect: true) + |> assert_code(expected) + end + + test "works with negation" do + code = """ + if !foo?(bar) do + false + else + true + end + """ + + expected = """ + !foo?(bar) + """ + + code + |> run_task(RedundantBooleans, autocorrect: true) + |> assert_code(expected) + end + test "keeps code as is without booleans" do code = """ if foo?(bar) do From f22e62a6023be8716a208b4dd8e44956ef526a7c Mon Sep 17 00:00:00 2001 From: Andrew Haust Date: Thu, 27 Jun 2024 18:03:15 -0400 Subject: [PATCH 10/19] Remove bogus test --- test/recode/task/redundant_booleans_test.exs | 18 ------------------ 1 file changed, 18 deletions(-) diff --git a/test/recode/task/redundant_booleans_test.exs b/test/recode/task/redundant_booleans_test.exs index b6a21e3..565bb00 100644 --- a/test/recode/task/redundant_booleans_test.exs +++ b/test/recode/task/redundant_booleans_test.exs @@ -96,24 +96,6 @@ defmodule Recode.Task.RedundantBooleansTest do |> assert_code(expected) end - test "works with negation" do - code = """ - if !foo?(bar) do - false - else - true - end - """ - - expected = """ - !foo?(bar) - """ - - code - |> run_task(RedundantBooleans, autocorrect: true) - |> assert_code(expected) - end - test "keeps code as is without booleans" do code = """ if foo?(bar) do From ce42670f4af1684983f948124468d449b7446809 Mon Sep 17 00:00:00 2001 From: Andrew Haust Date: Thu, 27 Jun 2024 18:07:25 -0400 Subject: [PATCH 11/19] Inline --- lib/recode/task/redundant_booleans.ex | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/lib/recode/task/redundant_booleans.ex b/lib/recode/task/redundant_booleans.ex index f207df8..1ea4683 100644 --- a/lib/recode/task/redundant_booleans.ex +++ b/lib/recode/task/redundant_booleans.ex @@ -46,9 +46,7 @@ defmodule Recode.Task.RedundantBooleans do defp simplify_comparison(%Zipper{node: {:if, meta, body}} = zipper, issues, true) do case extract(body) do {:ok, expr} -> - expr = - expr - |> put_leading_comments(meta) + expr = put_leading_comments(expr, meta) {Zipper.replace(zipper, expr), issues} From e009cb0667e167c0ddecbf229ae25453bc62dd72 Mon Sep 17 00:00:00 2001 From: Andrew Haust Date: Thu, 27 Jun 2024 18:20:56 -0400 Subject: [PATCH 12/19] Fix naming --- lib/recode/task/redundant_booleans.ex | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/recode/task/redundant_booleans.ex b/lib/recode/task/redundant_booleans.ex index 1ea4683..264f586 100644 --- a/lib/recode/task/redundant_booleans.ex +++ b/lib/recode/task/redundant_booleans.ex @@ -31,7 +31,7 @@ defmodule Recode.Task.RedundantBooleans do |> Source.get(:quoted) |> Zipper.zip() |> Zipper.traverse([], fn zipper, issues -> - simplify_comparison(zipper, issues, opts[:autocorrect]) + collapse_redundant_booleans(zipper, issues, opts[:autocorrect]) end) case opts[:autocorrect] do @@ -43,7 +43,7 @@ defmodule Recode.Task.RedundantBooleans do end end - defp simplify_comparison(%Zipper{node: {:if, meta, body}} = zipper, issues, true) do + defp collapse_redundant_booleans(%Zipper{node: {:if, meta, body}} = zipper, issues, true) do case extract(body) do {:ok, expr} -> expr = put_leading_comments(expr, meta) @@ -55,7 +55,7 @@ defmodule Recode.Task.RedundantBooleans do end end - defp simplify_comparison(%Zipper{node: {:if, meta, body}} = zipper, issues, false) do + defp collapse_redundant_booleans(%Zipper{node: {:if, meta, body}} = zipper, issues, false) do issues = case extract(body) do {:ok, _expr} -> @@ -70,7 +70,7 @@ defmodule Recode.Task.RedundantBooleans do {zipper, issues} end - defp simplify_comparison(zipper, issues, _autocorrect), do: {zipper, issues} + defp collapse_redundant_booleans(zipper, issues, _autocorrect), do: {zipper, issues} defp extract([ expr, From d46cde9ed648929488bd1db883c2460b237098c2 Mon Sep 17 00:00:00 2001 From: Andrew Haust Date: Sat, 29 Jun 2024 09:05:25 -0400 Subject: [PATCH 13/19] Negate reversed booleans --- lib/recode/task/redundant_booleans.ex | 10 ++++++++++ test/recode/task/redundant_booleans_test.exs | 10 +++------- 2 files changed, 13 insertions(+), 7 deletions(-) diff --git a/lib/recode/task/redundant_booleans.ex b/lib/recode/task/redundant_booleans.ex index 264f586..a40b361 100644 --- a/lib/recode/task/redundant_booleans.ex +++ b/lib/recode/task/redundant_booleans.ex @@ -82,6 +82,16 @@ defmodule Recode.Task.RedundantBooleans do {:ok, expr} end + defp extract([ + expr, + [ + {{:__block__, _, [:do]}, {:__block__, _, [false]}}, + {{:__block__, _, [:else]}, {:__block__, _, [true]}} + ] + ]) do + {:ok, {:not, [], [expr]}} + end + defp extract(_), do: :error defp put_leading_comments(expr, meta) do diff --git a/test/recode/task/redundant_booleans_test.exs b/test/recode/task/redundant_booleans_test.exs index 565bb00..bcac4c2 100644 --- a/test/recode/task/redundant_booleans_test.exs +++ b/test/recode/task/redundant_booleans_test.exs @@ -54,9 +54,9 @@ defmodule Recode.Task.RedundantBooleansTest do |> assert_code(expected) end - test "keeps code as is with reverse booleans" do + test "negates code with reverse booleans" do code = """ - if foo?(bar) do + if some_call?() && (some_other_call?() || another_call?()) do false else true @@ -64,11 +64,7 @@ defmodule Recode.Task.RedundantBooleansTest do """ expected = """ - if foo?(bar) do - false - else - true - end + not (some_call?() && (some_other_call?() || another_call?())) """ code From 76bac4ad2e3c6ea86df49b881da5e15dc5a1866e Mon Sep 17 00:00:00 2001 From: Andrew Haust Date: Sat, 29 Jun 2024 09:14:15 -0400 Subject: [PATCH 14/19] Cover `unless` as well --- lib/recode/task/redundant_booleans.ex | 44 ++++++++++++++++---- test/recode/task/redundant_booleans_test.exs | 36 ++++++++++++++++ 2 files changed, 73 insertions(+), 7 deletions(-) diff --git a/lib/recode/task/redundant_booleans.ex b/lib/recode/task/redundant_booleans.ex index a40b361..ff21ef3 100644 --- a/lib/recode/task/redundant_booleans.ex +++ b/lib/recode/task/redundant_booleans.ex @@ -43,8 +43,13 @@ defmodule Recode.Task.RedundantBooleans do end end - defp collapse_redundant_booleans(%Zipper{node: {:if, meta, body}} = zipper, issues, true) do - case extract(body) do + defp collapse_redundant_booleans( + %Zipper{node: {conditional, meta, body}} = zipper, + issues, + true + ) + when conditional in [:if, :unless] do + case extract(body, conditional) do {:ok, expr} -> expr = put_leading_comments(expr, meta) @@ -55,9 +60,14 @@ defmodule Recode.Task.RedundantBooleans do end end - defp collapse_redundant_booleans(%Zipper{node: {:if, meta, body}} = zipper, issues, false) do + defp collapse_redundant_booleans( + %Zipper{node: {conditional, meta, body}} = zipper, + issues, + false + ) + when conditional in [:if, :unless] do issues = - case extract(body) do + case extract(body, conditional) do {:ok, _expr} -> message = "Avoid `do: true, else: false`" issue = Issue.new(RedundantBooleans, message, meta) @@ -78,7 +88,7 @@ defmodule Recode.Task.RedundantBooleans do {{:__block__, _, [:do]}, {:__block__, _, [true]}}, {{:__block__, _, [:else]}, {:__block__, _, [false]}} ] - ]) do + ], :if) do {:ok, expr} end @@ -88,11 +98,31 @@ defmodule Recode.Task.RedundantBooleans do {{:__block__, _, [:do]}, {:__block__, _, [false]}}, {{:__block__, _, [:else]}, {:__block__, _, [true]}} ] - ]) do + ], :unless) do + {:ok, expr} + end + + defp extract([ + expr, + [ + {{:__block__, _, [:do]}, {:__block__, _, [false]}}, + {{:__block__, _, [:else]}, {:__block__, _, [true]}} + ] + ], :if) do + {:ok, {:not, [], [expr]}} + end + + defp extract([ + expr, + [ + {{:__block__, _, [:do]}, {:__block__, _, [true]}}, + {{:__block__, _, [:else]}, {:__block__, _, [false]}} + ] + ], :unless) do {:ok, {:not, [], [expr]}} end - defp extract(_), do: :error + defp extract(_, _), do: :error defp put_leading_comments(expr, meta) do comments = meta[:leading_comments] || [] diff --git a/test/recode/task/redundant_booleans_test.exs b/test/recode/task/redundant_booleans_test.exs index bcac4c2..f0c95b3 100644 --- a/test/recode/task/redundant_booleans_test.exs +++ b/test/recode/task/redundant_booleans_test.exs @@ -114,6 +114,42 @@ defmodule Recode.Task.RedundantBooleansTest do |> assert_code(expected) end + test "works with unless" do + code = """ + unless foo?(bar) do + false + else + true + end + """ + + expected = """ + foo?(bar) + """ + + code + |> run_task(RedundantBooleans, autocorrect: true) + |> assert_code(expected) + end + + test "negates unless" do + code = """ + unless foo?(bar) do + true + else + false + end + """ + + expected = """ + not foo?(bar) + """ + + code + |> run_task(RedundantBooleans, autocorrect: true) + |> assert_code(expected) + end + test "reports no issues" do """ foo == bar From 3094ae05ef980e221bdea4d0d6fdaad0915d9894 Mon Sep 17 00:00:00 2001 From: Andrew Haust Date: Sat, 29 Jun 2024 09:18:53 -0400 Subject: [PATCH 15/19] Refactor --- lib/recode/task/redundant_booleans.ex | 53 ++++++++------------------- 1 file changed, 16 insertions(+), 37 deletions(-) diff --git a/lib/recode/task/redundant_booleans.ex b/lib/recode/task/redundant_booleans.ex index ff21ef3..71f9bba 100644 --- a/lib/recode/task/redundant_booleans.ex +++ b/lib/recode/task/redundant_booleans.ex @@ -82,44 +82,23 @@ defmodule Recode.Task.RedundantBooleans do defp collapse_redundant_booleans(zipper, issues, _autocorrect), do: {zipper, issues} - defp extract([ - expr, + defp extract( [ - {{:__block__, _, [:do]}, {:__block__, _, [true]}}, - {{:__block__, _, [:else]}, {:__block__, _, [false]}} - ] - ], :if) do - {:ok, expr} - end - - defp extract([ - expr, - [ - {{:__block__, _, [:do]}, {:__block__, _, [false]}}, - {{:__block__, _, [:else]}, {:__block__, _, [true]}} - ] - ], :unless) do - {:ok, expr} - end - - defp extract([ - expr, - [ - {{:__block__, _, [:do]}, {:__block__, _, [false]}}, - {{:__block__, _, [:else]}, {:__block__, _, [true]}} - ] - ], :if) do - {:ok, {:not, [], [expr]}} - end - - defp extract([ - expr, - [ - {{:__block__, _, [:do]}, {:__block__, _, [true]}}, - {{:__block__, _, [:else]}, {:__block__, _, [false]}} - ] - ], :unless) do - {:ok, {:not, [], [expr]}} + expr, + [ + {{:__block__, _, [:do]}, {:__block__, _, [left]}}, + {{:__block__, _, [:else]}, {:__block__, _, [right]}} + ] + ], + conditional + ) + when is_boolean(left) and is_boolean(right) do + case {conditional, left, right} do + {:if, true, false} -> {:ok, expr} + {:if, false, true} -> {:ok, {:not, [], [expr]}} + {:unless, true, false} -> {:ok, {:not, [], [expr]}} + {:unless, false, true} -> {:ok, expr} + end end defp extract(_, _), do: :error From 125307ac156863afbcab645023e60be04340e849 Mon Sep 17 00:00:00 2001 From: Andrew Haust Date: Sat, 29 Jun 2024 09:23:06 -0400 Subject: [PATCH 16/19] Rename check to `UnecessaryIfUnless` --- README.md | 36 +++++++++---------- lib/recode/config.ex | 2 +- ...t_booleans.ex => unnecessary_if_unless.ex} | 8 ++--- ...est.exs => unnecessary_if_unless_test.exs} | 26 +++++++------- 4 files changed, 36 insertions(+), 36 deletions(-) rename lib/recode/task/{redundant_booleans.ex => unnecessary_if_unless.ex} (91%) rename test/recode/task/{redundant_booleans_test.exs => unnecessary_if_unless_test.exs} (76%) diff --git a/README.md b/README.md index 452c63e..61696a2 100644 --- a/README.md +++ b/README.md @@ -18,25 +18,25 @@ on `sourceror`. > mix recode.help Design tasks: -TagFIXME # Checker - Checks if there are FIXME tags in the sources. -TagTODO # Checker - Checks if there are TODO tags in the sources. -Readability tasks: -AliasExpansion # Corrector - Exapnds multi aliases to separate aliases. -AliasOrder # Corrector - Checks if aliases are sorted alphabetically. -EnforceLineLength # Corrector - Forces expressions to one line. -Format # Corrector - Does the same as `mix format`. -PipeFunOne # Corrector - Add parentheses to one-arity functions. -SinglePipe # Corrector - Pipes should only be used when piping data through multiple calls. -Specs # Checker - Checks for specs. +TagFIXME # Checker - Checks if there are FIXME tags in the sources. +TagTODO # Checker - Checks if there are TODO tags in the sources. +Readability tasks: +AliasExpansion # Corrector - Exapnds multi aliases to separate aliases. +AliasOrder # Corrector - Checks if aliases are sorted alphabetically. +EnforceLineLength # Corrector - Forces expressions to one line. +Format # Corrector - Does the same as `mix format`. +PipeFunOne # Corrector - Add parentheses to one-arity functions. +SinglePipe # Corrector - Pipes should only be used when piping data through multiple calls. +Specs # Checker - Checks for specs. Refactor tasks: -FilterCount # Corrector - Checks calls like Enum.filter(...) |> Enum.count(). -Nesting # Checker - Checks code nesting depth in functions and macros. -RedundantBooleans # Corrector - Removes `do: true, else: false` +FilterCount # Corrector - Checks calls like Enum.filter(...) |> Enum.count(). +Nesting # Checker - Checks code nesting depth in functions and macros. +UnnecessaryIfUnless # Corrector - Changes `if expr, do: true, else: false` to `expr` Warning tasks: -Dbg # Corrector - There should be no calls to dbg. -IOInspect # Corrector - There should be no calls to IO.inspect. -TestFileExt # Corrector - Checks the file extension of test files. -UnusedVariable # Corrector - Checks if unused variables occur. +Dbg # Corrector - There should be no calls to dbg. +IOInspect # Corrector - There should be no calls to IO.inspect. +TestFileExt # Corrector - Checks the file extension of test files. +UnusedVariable # Corrector - Checks if unused variables occur. ``` It is also possible to run `recode` in a none-autocorrect mode to just lint your @@ -110,7 +110,7 @@ This mix task generates the config file `.recode.exs`. {Recode.Task.IOInspect, [autocorrect: false]}, {Recode.Task.Nesting, []}, {Recode.Task.PipeFunOne, []}, - {Recode.Task.RedundantBooleans, []}, + {Recode.Task.UnnecessaryIfUnless, []}, {Recode.Task.SinglePipe, []}, {Recode.Task.Specs, [exclude: "test/**/*.{ex,exs}", config: [only: :visible]]}, {Recode.Task.TagFIXME, [exit_code: 2]}, diff --git a/lib/recode/config.ex b/lib/recode/config.ex index e2ffa5b..323af9d 100644 --- a/lib/recode/config.ex +++ b/lib/recode/config.ex @@ -37,7 +37,7 @@ defmodule Recode.Config do # {Recode.Task.Moduledoc, []}, {Recode.Task.Nesting, []}, {Recode.Task.PipeFunOne, []}, - {Recode.Task.RedundantBooleans, []}, + {Recode.Task.UnnecessaryIfUnless, []}, {Recode.Task.SinglePipe, []}, {Recode.Task.Specs, [exclude: ["test/**/*.{ex,exs}", "mix.exs"], config: [only: :visible]]}, {Recode.Task.TagFIXME, exit_code: 2}, diff --git a/lib/recode/task/redundant_booleans.ex b/lib/recode/task/unnecessary_if_unless.ex similarity index 91% rename from lib/recode/task/redundant_booleans.ex rename to lib/recode/task/unnecessary_if_unless.ex index 71f9bba..2c52739 100644 --- a/lib/recode/task/redundant_booleans.ex +++ b/lib/recode/task/unnecessary_if_unless.ex @@ -1,4 +1,4 @@ -defmodule Recode.Task.RedundantBooleans do +defmodule Recode.Task.UnnecessaryIfUnless do @shortdoc "Removes redundant booleans" @moduledoc """ @@ -20,7 +20,7 @@ defmodule Recode.Task.RedundantBooleans do use Recode.Task, corrector: true, category: :readability alias Recode.Issue - alias Recode.Task.RedundantBooleans + alias Recode.Task.UnnecessaryIfUnless alias Rewrite.Source alias Sourceror.Zipper @@ -36,7 +36,7 @@ defmodule Recode.Task.RedundantBooleans do case opts[:autocorrect] do true -> - Source.update(source, RedundantBooleans, :quoted, Zipper.root(zipper)) + Source.update(source, UnnecessaryIfUnless, :quoted, Zipper.root(zipper)) false -> Source.add_issues(source, issues) @@ -70,7 +70,7 @@ defmodule Recode.Task.RedundantBooleans do case extract(body, conditional) do {:ok, _expr} -> message = "Avoid `do: true, else: false`" - issue = Issue.new(RedundantBooleans, message, meta) + issue = Issue.new(UnnecessaryIfUnless, message, meta) [issue | issues] :error -> diff --git a/test/recode/task/redundant_booleans_test.exs b/test/recode/task/unnecessary_if_unless_test.exs similarity index 76% rename from test/recode/task/redundant_booleans_test.exs rename to test/recode/task/unnecessary_if_unless_test.exs index f0c95b3..e47ba73 100644 --- a/test/recode/task/redundant_booleans_test.exs +++ b/test/recode/task/unnecessary_if_unless_test.exs @@ -1,7 +1,7 @@ -defmodule Recode.Task.RedundantBooleansTest do +defmodule Recode.Task.UnnecessaryIfUnlessTest do use RecodeCase - alias Recode.Task.RedundantBooleans + alias Recode.Task.UnnecessaryIfUnless describe "run/1" do test "equals" do @@ -18,7 +18,7 @@ defmodule Recode.Task.RedundantBooleansTest do """ code - |> run_task(RedundantBooleans, autocorrect: true) + |> run_task(UnnecessaryIfUnless, autocorrect: true) |> assert_code(expected) end @@ -36,7 +36,7 @@ defmodule Recode.Task.RedundantBooleansTest do """ code - |> run_task(RedundantBooleans, autocorrect: true) + |> run_task(UnnecessaryIfUnless, autocorrect: true) |> assert_code(expected) end @@ -50,7 +50,7 @@ defmodule Recode.Task.RedundantBooleansTest do """ code - |> run_task(RedundantBooleans, autocorrect: true) + |> run_task(UnnecessaryIfUnless, autocorrect: true) |> assert_code(expected) end @@ -68,7 +68,7 @@ defmodule Recode.Task.RedundantBooleansTest do """ code - |> run_task(RedundantBooleans, autocorrect: true) + |> run_task(UnnecessaryIfUnless, autocorrect: true) |> assert_code(expected) end @@ -88,7 +88,7 @@ defmodule Recode.Task.RedundantBooleansTest do """ code - |> run_task(RedundantBooleans, autocorrect: true) + |> run_task(UnnecessaryIfUnless, autocorrect: true) |> assert_code(expected) end @@ -110,7 +110,7 @@ defmodule Recode.Task.RedundantBooleansTest do """ code - |> run_task(RedundantBooleans, autocorrect: true) + |> run_task(UnnecessaryIfUnless, autocorrect: true) |> assert_code(expected) end @@ -128,7 +128,7 @@ defmodule Recode.Task.RedundantBooleansTest do """ code - |> run_task(RedundantBooleans, autocorrect: true) + |> run_task(UnnecessaryIfUnless, autocorrect: true) |> assert_code(expected) end @@ -146,7 +146,7 @@ defmodule Recode.Task.RedundantBooleansTest do """ code - |> run_task(RedundantBooleans, autocorrect: true) + |> run_task(UnnecessaryIfUnless, autocorrect: true) |> assert_code(expected) end @@ -154,7 +154,7 @@ defmodule Recode.Task.RedundantBooleansTest do """ foo == bar """ - |> run_task(RedundantBooleans, autocorrect: false) + |> run_task(UnnecessaryIfUnless, autocorrect: false) |> refute_issues() end @@ -166,8 +166,8 @@ defmodule Recode.Task.RedundantBooleansTest do false end """ - |> run_task(RedundantBooleans, autocorrect: false) - |> assert_issue_with(reporter: RedundantBooleans) + |> run_task(UnnecessaryIfUnless, autocorrect: false) + |> assert_issue_with(reporter: UnnecessaryIfUnless) end end end From 522469ed95d29ba7e60afd7f704326322b163257 Mon Sep 17 00:00:00 2001 From: Andrew Haust Date: Sat, 29 Jun 2024 09:30:14 -0400 Subject: [PATCH 17/19] Fix message --- lib/recode/task/unnecessary_if_unless.ex | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/recode/task/unnecessary_if_unless.ex b/lib/recode/task/unnecessary_if_unless.ex index 2c52739..008bdfe 100644 --- a/lib/recode/task/unnecessary_if_unless.ex +++ b/lib/recode/task/unnecessary_if_unless.ex @@ -69,7 +69,7 @@ defmodule Recode.Task.UnnecessaryIfUnless do issues = case extract(body, conditional) do {:ok, _expr} -> - message = "Avoid `do: true, else: false`" + message = "Avoid unnecessary `if` and `unless`" issue = Issue.new(UnnecessaryIfUnless, message, meta) [issue | issues] From 635eb3763b3d6b61571eb5466a63bb669ee1c89e Mon Sep 17 00:00:00 2001 From: Andrew Haust Date: Sat, 29 Jun 2024 11:29:22 -0400 Subject: [PATCH 18/19] Rename function --- lib/recode/task/unnecessary_if_unless.ex | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/recode/task/unnecessary_if_unless.ex b/lib/recode/task/unnecessary_if_unless.ex index 008bdfe..7269bdc 100644 --- a/lib/recode/task/unnecessary_if_unless.ex +++ b/lib/recode/task/unnecessary_if_unless.ex @@ -31,7 +31,7 @@ defmodule Recode.Task.UnnecessaryIfUnless do |> Source.get(:quoted) |> Zipper.zip() |> Zipper.traverse([], fn zipper, issues -> - collapse_redundant_booleans(zipper, issues, opts[:autocorrect]) + remove_unecessary_if_unless(zipper, issues, opts[:autocorrect]) end) case opts[:autocorrect] do @@ -43,7 +43,7 @@ defmodule Recode.Task.UnnecessaryIfUnless do end end - defp collapse_redundant_booleans( + defp remove_unecessary_if_unless( %Zipper{node: {conditional, meta, body}} = zipper, issues, true @@ -60,7 +60,7 @@ defmodule Recode.Task.UnnecessaryIfUnless do end end - defp collapse_redundant_booleans( + defp remove_unecessary_if_unless( %Zipper{node: {conditional, meta, body}} = zipper, issues, false @@ -80,7 +80,7 @@ defmodule Recode.Task.UnnecessaryIfUnless do {zipper, issues} end - defp collapse_redundant_booleans(zipper, issues, _autocorrect), do: {zipper, issues} + defp remove_unecessary_if_unless(zipper, issues, _autocorrect), do: {zipper, issues} defp extract( [ From 298454e56b33de8ccb84c5e6d60c1c5841f3358b Mon Sep 17 00:00:00 2001 From: Andrew Haust Date: Sat, 29 Jun 2024 13:29:00 -0400 Subject: [PATCH 19/19] Fix test and docs --- README.md | 2 +- lib/recode/config.ex | 2 +- mix.exs | 1 + 3 files changed, 3 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 61696a2..8cd17bc 100644 --- a/README.md +++ b/README.md @@ -110,12 +110,12 @@ This mix task generates the config file `.recode.exs`. {Recode.Task.IOInspect, [autocorrect: false]}, {Recode.Task.Nesting, []}, {Recode.Task.PipeFunOne, []}, - {Recode.Task.UnnecessaryIfUnless, []}, {Recode.Task.SinglePipe, []}, {Recode.Task.Specs, [exclude: "test/**/*.{ex,exs}", config: [only: :visible]]}, {Recode.Task.TagFIXME, [exit_code: 2]}, {Recode.Task.TagTODO, [exit_code: 4]}, {Recode.Task.TestFileExt, []}, + {Recode.Task.UnnecessaryIfUnless, []}, {Recode.Task.UnusedVariable, [active: false]} ] ] diff --git a/lib/recode/config.ex b/lib/recode/config.ex index 323af9d..af95956 100644 --- a/lib/recode/config.ex +++ b/lib/recode/config.ex @@ -37,12 +37,12 @@ defmodule Recode.Config do # {Recode.Task.Moduledoc, []}, {Recode.Task.Nesting, []}, {Recode.Task.PipeFunOne, []}, - {Recode.Task.UnnecessaryIfUnless, []}, {Recode.Task.SinglePipe, []}, {Recode.Task.Specs, [exclude: ["test/**/*.{ex,exs}", "mix.exs"], config: [only: :visible]]}, {Recode.Task.TagFIXME, exit_code: 2}, {Recode.Task.TagTODO, exit_code: 4}, {Recode.Task.TestFileExt, []}, + {Recode.Task.UnnecessaryIfUnless, []}, {Recode.Task.UnusedVariable, [active: false]} ] ] diff --git a/mix.exs b/mix.exs index 8ad1416..9426be8 100644 --- a/mix.exs +++ b/mix.exs @@ -61,6 +61,7 @@ defmodule Recode.MixProject do Recode.Task.TagFIXME, Recode.Task.TagTODO, Recode.Task.TestFileExt, + Recode.Task.UnnecessaryIfInless, Recode.Task.UnusedVariable ] ]