diff --git a/.credo.exs b/.credo.exs index 9fa8375..c4189b5 100644 --- a/.credo.exs +++ b/.credo.exs @@ -46,7 +46,7 @@ # If you want to enforce a style guide and need a more traditional linting # experience, you can change `strict` to `true` below: # - strict: false, + strict: true, # # To modify the timeout for parsing files, change this value: # @@ -102,7 +102,6 @@ {Credo.Check.Readability.ModuleDoc, []}, {Credo.Check.Readability.ModuleNames, []}, {Credo.Check.Readability.ParenthesesInCondition, []}, - {Credo.Check.Readability.ParenthesesOnZeroArityDefs, []}, {Credo.Check.Readability.PipeIntoAnonymousFunctions, []}, {Credo.Check.Readability.PredicateFunctionNames, []}, {Credo.Check.Readability.PreferImplicitTry, []}, @@ -124,7 +123,6 @@ {Credo.Check.Refactor.CyclomaticComplexity, []}, {Credo.Check.Refactor.FilterCount, []}, {Credo.Check.Refactor.FilterFilter, []}, - {Credo.Check.Refactor.FunctionArity, []}, {Credo.Check.Refactor.LongQuoteBlocks, []}, {Credo.Check.Refactor.MapJoin, []}, {Credo.Check.Refactor.MatchInCondition, []}, @@ -162,6 +160,8 @@ {Credo.Check.Warning.WrongTestFileExtension, []} ], disabled: [ + {Credo.Check.Readability.ParenthesesOnZeroArityDefs, []}, + {Credo.Check.Refactor.FunctionArity, []}, # # Checks scheduled for next check update (opt-in for now, just replace `false` with `[]`) diff --git a/lib/recode/task/locals_without_parens.ex b/lib/recode/task/locals_without_parens.ex index b02ecb9..79998c5 100644 --- a/lib/recode/task/locals_without_parens.ex +++ b/lib/recode/task/locals_without_parens.ex @@ -23,6 +23,9 @@ defmodule Recode.Task.LocalsWithoutParens do alias Rewrite.Source alias Sourceror.Zipper + @def [:def, :defp, :defmacro, :defmacrop] + @exclude [:{}, :%{}] + @impl Recode.Task def run(source, opts) do locals_without_parens = @@ -35,7 +38,7 @@ defmodule Recode.Task.LocalsWithoutParens do source |> Source.get(:quoted) |> Zipper.zip() - |> Zipper.traverse([], fn zipper, issues -> + |> Zipper.traverse_while([], fn zipper, issues -> remove_parens(zipper, issues, locals_without_parens, opts[:autocorrect]) end) |> update(source, opts) @@ -46,27 +49,58 @@ defmodule Recode.Task.LocalsWithoutParens do end defp remove_parens( - %Zipper{node: {fun, meta, [_ | _] = args}} = zipper, + %Zipper{node: {{:__block__, meta, [:do]}, _block}} = zipper, issues, - locals_without_parens, - autocorrect? + _locals_without_parens, + _autocorrect? ) do - if Keyword.has_key?(meta, :closing) and - local_without_parens?(locals_without_parens, fun, args) do - if autocorrect? do - node = {fun, Keyword.delete(meta, :closing), args} - {Zipper.replace(zipper, node), issues} - else - issue = new_issue("Unnecessary parens", meta) - {zipper, [issue | issues]} - end + if meta[:format] == :keyword do + {:skip, zipper, issues} else - {zipper, issues} + {:cont, zipper, issues} end end + defp remove_parens( + %Zipper{node: {fun, _meta, _args}} = zipper, + issues, + _locals_without_parens, + _autocorrect? + ) + when fun in @def do + {:cont, Zipper.next(zipper), issues} + end + + defp remove_parens( + %Zipper{node: {_one, _two}} = zipper, + issues, + _locals_without_parens, + _autocorrect? + ) do + {:skip, zipper, issues} + end + + defp remove_parens( + %Zipper{node: {form, _meta, _args}} = zipper, + issues, + _locals_without_parens, + _autocorrect? + ) + when form in @exclude do + {:skip, zipper, issues} + end + + defp remove_parens( + %Zipper{node: {_fun, _meta, [_ | _]}} = zipper, + issues, + locals_without_parens, + autocorrect? + ) do + do_remove_parens(zipper, issues, locals_without_parens, autocorrect?) + end + defp remove_parens(zipper, issues, _locals_without_parens, _autocorrect) do - {zipper, issues} + {:cont, zipper, issues} end defp local_without_parens?(locals_without_parens, fun, args) do @@ -78,4 +112,24 @@ defmodule Recode.Task.LocalsWithoutParens do _other -> false end) end + + defp do_remove_parens( + %Zipper{node: {fun, meta, args}} = zipper, + issues, + locals_without_parens, + autocorrect? + ) do + if Keyword.has_key?(meta, :closing) and + local_without_parens?(locals_without_parens, fun, args) do + if autocorrect? do + node = {fun, Keyword.delete(meta, :closing), args} + {:cont, Zipper.replace(zipper, node), issues} + else + issue = new_issue("Unnecessary parens", meta) + {:cont, zipper, [issue | issues]} + end + else + {:cont, zipper, issues} + end + end end diff --git a/mix.exs b/mix.exs index 468d73f..a4ed391 100644 --- a/mix.exs +++ b/mix.exs @@ -111,10 +111,10 @@ defmodule Recode.MixProject do {:excoveralls, "~> 0.15", only: :test}, {:mox, "~> 1.0", only: :test} ] ++ - if System.get_env("CI") == "true" do - [] - else + if Version.match?(System.version(), "~> 1.18") do [{:freedom_formatter, "~> 2.1", only: :test}] + else + [] end end diff --git a/mix.lock b/mix.lock index 6d2f72b..298e185 100644 --- a/mix.lock +++ b/mix.lock @@ -8,6 +8,7 @@ "ex_doc": {:hex, :ex_doc, "0.36.1", "4197d034f93e0b89ec79fac56e226107824adcce8d2dd0a26f5ed3a95efc36b1", [:mix], [{:earmark_parser, "~> 1.4.42", [hex: :earmark_parser, repo: "hexpm", optional: false]}, {:makeup_c, ">= 0.1.0", [hex: :makeup_c, repo: "hexpm", optional: true]}, {:makeup_elixir, "~> 0.14 or ~> 1.0", [hex: :makeup_elixir, repo: "hexpm", optional: false]}, {:makeup_erlang, "~> 0.1 or ~> 1.0", [hex: :makeup_erlang, repo: "hexpm", optional: false]}, {:makeup_html, ">= 0.1.0", [hex: :makeup_html, repo: "hexpm", optional: true]}], "hexpm", "d7d26a7cf965dacadcd48f9fa7b5953d7d0cfa3b44fa7a65514427da44eafd89"}, "excoveralls": {:hex, :excoveralls, "0.18.4", "70f70dc37b9bd90cf66868c12778d2f60b792b79e9e12aed000972a8046dc093", [:mix], [{:castore, "~> 1.0", [hex: :castore, repo: "hexpm", optional: true]}, {:jason, "~> 1.0", [hex: :jason, repo: "hexpm", optional: false]}], "hexpm", "cda8bb587d9deaa0da6158cf0a18929189abbe53bf42b10fe70016d5f4f5d6a9"}, "file_system": {:hex, :file_system, "1.1.0", "08d232062284546c6c34426997dd7ef6ec9f8bbd090eb91780283c9016840e8f", [:mix], [], "hexpm", "bfcf81244f416871f2a2e15c1b515287faa5db9c6bcf290222206d120b3d43f6"}, + "freedom_formatter": {:hex, :freedom_formatter, "2.1.1", "5f33efc2c605ce01c0d1d854ec7cf091c6a6ec846f38de99b1d3fac7775a68bd", [:mix], [], "hexpm", "373b659b9763ca4f4d5fbccb4827b00dbd6c6898d528d5dce91eb60b29ac5fc1"}, "glob_ex": {:hex, :glob_ex, "0.1.11", "cb50d3f1ef53f6ca04d6252c7fde09fd7a1cf63387714fe96f340a1349e62c93", [:mix], [], "hexpm", "342729363056e3145e61766b416769984c329e4378f1d558b63e341020525de4"}, "jason": {:hex, :jason, "1.4.4", "b9226785a9aa77b6857ca22832cffa5d5011a667207eb2a0ad56adb5db443b8a", [:mix], [{:decimal, "~> 1.0 or ~> 2.0", [hex: :decimal, repo: "hexpm", optional: true]}], "hexpm", "c5eb0cab91f094599f94d55bc63409236a8ec69a21a67814529e8d5f6cc90b3b"}, "makeup": {:hex, :makeup, "1.2.1", "e90ac1c65589ef354378def3ba19d401e739ee7ee06fb47f94c687016e3713d1", [:mix], [{:nimble_parsec, "~> 1.4", [hex: :nimble_parsec, repo: "hexpm", optional: false]}], "hexpm", "d36484867b0bae0fea568d10131197a4c2e47056a6fbe84922bf6ba71c8d17ce"}, @@ -16,6 +17,7 @@ "mox": {:hex, :mox, "1.2.0", "a2cd96b4b80a3883e3100a221e8adc1b98e4c3a332a8fc434c39526babafd5b3", [:mix], [{:nimble_ownership, "~> 1.0", [hex: :nimble_ownership, repo: "hexpm", optional: false]}], "hexpm", "c7b92b3cc69ee24a7eeeaf944cd7be22013c52fcb580c1f33f50845ec821089a"}, "nimble_ownership": {:hex, :nimble_ownership, "1.0.1", "f69fae0cdd451b1614364013544e66e4f5d25f36a2056a9698b793305c5aa3a6", [:mix], [], "hexpm", "3825e461025464f519f3f3e4a1f9b68c47dc151369611629ad08b636b73bb22d"}, "nimble_parsec": {:hex, :nimble_parsec, "1.4.1", "f41275a0354c736db4b1d255b5d2a27c91028e55c21ea3145b938e22649ffa3f", [:mix], [], "hexpm", "605e44204998f138d6e13be366c8e81af860e726c8177caf50067e1b618fe522"}, - "rewrite": {:hex, :rewrite, "0.10.5", "6afadeae0b9d843b27ac6225e88e165884875e0aed333ef4ad3bf36f9c101bed", [:mix], [{:glob_ex, "~> 0.1", [hex: :glob_ex, repo: "hexpm", optional: false]}, {:sourceror, "~> 1.0", [hex: :sourceror, repo: "hexpm", optional: false]}], "hexpm", "51cc347a4269ad3a1e7a2c4122dbac9198302b082f5615964358b4635ebf3d4f"}, + "rewrite": {:hex, :rewrite, "1.1.2", "f5a5d10f5fed1491a6ff48e078d4585882695962ccc9e6c779bae025d1f92eda", [:mix], [{:glob_ex, "~> 0.1", [hex: :glob_ex, repo: "hexpm", optional: false]}, {:sourceror, "~> 1.0", [hex: :sourceror, repo: "hexpm", optional: false]}, {:text_diff, "~> 0.1", [hex: :text_diff, repo: "hexpm", optional: false]}], "hexpm", "7f8b94b1e3528d0a47b3e8b7bfeca559d2948a65fa7418a9ad7d7712703d39d4"}, "sourceror": {:hex, :sourceror, "1.7.1", "599d78f4cc2be7d55c9c4fd0a8d772fd0478e3a50e726697c20d13d02aa056d4", [:mix], [], "hexpm", "cd6f268fe29fa00afbc535e215158680a0662b357dc784646d7dff28ac65a0fc"}, + "text_diff": {:hex, :text_diff, "0.1.0", "1caf3175e11a53a9a139bc9339bd607c47b9e376b073d4571c031913317fecaa", [:mix], [], "hexpm", "d1ffaaecab338e49357b6daa82e435f877e0649041ace7755583a0ea3362dbd7"}, } diff --git a/test/recode/task/locals_without_parens_test.exs b/test/recode/task/locals_without_parens_test.exs index b4a81c3..73dbd4e 100644 --- a/test/recode/task/locals_without_parens_test.exs +++ b/test/recode/task/locals_without_parens_test.exs @@ -31,6 +31,35 @@ defmodule Recode.Task.LocalsWithoutParensTest do |> assert_code(expected) end + test "does not remove parens in def*" do + dot_formatter = + DotFormatter.from_formatter_opts(locals_without_parens: [foo: 1, bar: 2, baz: :*]) + + code = """ + def foo(x) do + bar(x, 2) + end + + def bar(_x, y) do + foo(y) + end + """ + + expected = """ + def foo(x) do + bar x, 2 + end + + def bar(_x, y) do + foo y + end + """ + + code + |> run_task(LocalsWithoutParens, autocorrect: true, dot_formatter: dot_formatter) + |> assert_code(expected) + end + test "adds issue" do dot_formatter = DotFormatter.from_formatter_opts(locals_without_parens: [foo: 1]) @@ -51,7 +80,7 @@ defmodule Recode.Task.LocalsWithoutParensTest do |> refute_issues() end - test "adds no issue for dev" do + test "adds no issue for def" do """ def hello do :world @@ -60,4 +89,26 @@ defmodule Recode.Task.LocalsWithoutParensTest do |> run_task(LocalsWithoutParens, autocorrect: false) |> refute_issues() end + + test "add no issue for parens in do:" do + dot_formatter = DotFormatter.from_formatter_opts(locals_without_parens: [foo: 1]) + + """ + def bar(x) when is_binary(x), do: foo(x) + """ + |> run_task(LocalsWithoutParens, autocorrect: false, dot_formatter: dot_formatter) + |> refute_issues() + end + + test "add no issue for parens in tuples and maps" do + dot_formatter = DotFormatter.from_formatter_opts(locals_without_parens: [foo: 1]) + + """ + {a, foo(b)} + {a, foo(b), c} + %{a: foo b} + """ + |> run_task(LocalsWithoutParens, autocorrect: false, dot_formatter: dot_formatter) + |> refute_issues() + end end