diff --git a/README.md b/README.md index 85a5392..8cd17bc 100644 --- a/README.md +++ b/README.md @@ -18,24 +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. +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 @@ -114,6 +115,7 @@ This mix task generates the config file `.recode.exs`. {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 8d48050..af95956 100644 --- a/lib/recode/config.ex +++ b/lib/recode/config.ex @@ -42,6 +42,7 @@ defmodule Recode.Config do {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/task/unnecessary_if_unless.ex b/lib/recode/task/unnecessary_if_unless.ex new file mode 100644 index 0000000..7269bdc --- /dev/null +++ b/lib/recode/task/unnecessary_if_unless.ex @@ -0,0 +1,110 @@ +defmodule Recode.Task.UnnecessaryIfUnless do + @shortdoc "Removes redundant booleans" + + @moduledoc """ + Redudant booleans make code needlesly verbose. + + # 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.UnnecessaryIfUnless + 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 -> + remove_unecessary_if_unless(zipper, issues, opts[:autocorrect]) + end) + + case opts[:autocorrect] do + true -> + Source.update(source, UnnecessaryIfUnless, :quoted, Zipper.root(zipper)) + + false -> + Source.add_issues(source, issues) + end + end + + defp remove_unecessary_if_unless( + %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) + + {Zipper.replace(zipper, expr), issues} + + :error -> + {zipper, issues} + end + end + + defp remove_unecessary_if_unless( + %Zipper{node: {conditional, meta, body}} = zipper, + issues, + false + ) + when conditional in [:if, :unless] do + issues = + case extract(body, conditional) do + {:ok, _expr} -> + message = "Avoid unnecessary `if` and `unless`" + issue = Issue.new(UnnecessaryIfUnless, message, meta) + [issue | issues] + + :error -> + issues + end + + {zipper, issues} + end + + defp remove_unecessary_if_unless(zipper, issues, _autocorrect), do: {zipper, issues} + + defp extract( + [ + 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 + + defp put_leading_comments(expr, meta) do + comments = meta[:leading_comments] || [] + Sourceror.append_comments(expr, comments) + end +end 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 ] ] diff --git a/test/recode/task/unnecessary_if_unless_test.exs b/test/recode/task/unnecessary_if_unless_test.exs new file mode 100644 index 0000000..e47ba73 --- /dev/null +++ b/test/recode/task/unnecessary_if_unless_test.exs @@ -0,0 +1,173 @@ +defmodule Recode.Task.UnnecessaryIfUnlessTest do + use RecodeCase + + alias Recode.Task.UnnecessaryIfUnless + + describe "run/1" do + test "equals" do + code = """ + if foo == bar do + true + else + false + end + """ + + expected = """ + foo == bar + """ + + code + |> run_task(UnnecessaryIfUnless, autocorrect: true) + |> assert_code(expected) + end + + test "predicate" do + code = """ + if foo?(bar) do + true + else + false + end + """ + + expected = """ + foo?(bar) + """ + + code + |> run_task(UnnecessaryIfUnless, autocorrect: true) + |> assert_code(expected) + end + + test "keyword syntax" do + code = """ + if foo?(bar), do: true, else: false + """ + + expected = """ + foo?(bar) + """ + + code + |> run_task(UnnecessaryIfUnless, autocorrect: true) + |> assert_code(expected) + end + + test "negates code with reverse booleans" do + code = """ + if some_call?() && (some_other_call?() || another_call?()) do + false + else + true + end + """ + + expected = """ + not (some_call?() && (some_other_call?() || another_call?())) + """ + + code + |> run_task(UnnecessaryIfUnless, autocorrect: true) + |> 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(UnnecessaryIfUnless, autocorrect: true) + |> 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(UnnecessaryIfUnless, autocorrect: true) + |> assert_code(expected) + end + + test "works with unless" do + code = """ + unless foo?(bar) do + false + else + true + end + """ + + expected = """ + foo?(bar) + """ + + code + |> run_task(UnnecessaryIfUnless, 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(UnnecessaryIfUnless, autocorrect: true) + |> assert_code(expected) + end + + test "reports no issues" do + """ + foo == bar + """ + |> run_task(UnnecessaryIfUnless, autocorrect: false) + |> refute_issues() + end + + test "reports an issue" do + """ + if foo == bar do + true + else + false + end + """ + |> run_task(UnnecessaryIfUnless, autocorrect: false) + |> assert_issue_with(reporter: UnnecessaryIfUnless) + end + end +end