Skip to content

Commit

Permalink
Fix LocalsWithoutParens task (#104)
Browse files Browse the repository at this point in the history
  • Loading branch information
@NickNeck
NickNeck authored Jan 19, 2025
1 parent 394ffe3 commit 60fdb11
Show file tree
Hide file tree
Showing 5 changed files with 130 additions and 23 deletions.
6 changes: 3 additions & 3 deletions .credo.exs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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:
#
Expand Down Expand Up @@ -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, []},
Expand All @@ -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, []},
Expand Down Expand Up @@ -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 `[]`)

Expand Down
84 changes: 69 additions & 15 deletions lib/recode/task/locals_without_parens.ex
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 =
Expand All @@ -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)
Expand All @@ -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
Expand All @@ -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
6 changes: 3 additions & 3 deletions mix.exs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
4 changes: 3 additions & 1 deletion mix.lock
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"},
Expand All @@ -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"},
}
53 changes: 52 additions & 1 deletion test/recode/task/locals_without_parens_test.exs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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])

Expand All @@ -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
Expand All @@ -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

0 comments on commit 60fdb11

Please sign in to comment.