Skip to content

Commit

Permalink
Merge pull request #673 from whatyouhide/master
Browse files Browse the repository at this point in the history
Add an option to enforce parens or not in zero-arity defs
  • Loading branch information
rrrene committed Jul 12, 2019
2 parents 1a1ac4e + 42dd760 commit 39e3f2f
Show file tree
Hide file tree
Showing 2 changed files with 54 additions and 24 deletions.
64 changes: 41 additions & 23 deletions lib/credo/check/readability/parentheses_on_zero_arity_defs.ex
Expand Up @@ -2,17 +2,19 @@ defmodule Credo.Check.Readability.ParenthesesOnZeroArityDefs do
@moduledoc false

@checkdoc """
Do not use parentheses when defining a function which has no arguments.
Either use parentheses or not when defininig a function with no arguments.
The code in this example ...
By default, this check enforces no parentheses, so zero-arity function
and macro definitions should look like this:
def summer?() do
def summer? do
# ...
end
... should be refactored to look like this:
If the `:parens` option is set to `true` for this check, then the check
enforces zero-arity function and macro definitions to have parens:
def summer? do
def summer?() do
# ...
end
Expand All @@ -23,49 +25,60 @@ defmodule Credo.Check.Readability.ParenthesesOnZeroArityDefs do
@explanation [check: @checkdoc]
@def_ops [:def, :defp, :defmacro, :defmacrop]

@default_params [parens: false]

use Credo.Check, base_priority: :low

alias Credo.Check.Params

@doc false
def run(source_file, params \\ []) do
parens? = Params.get(params, :parens, @default_params)
issue_meta = IssueMeta.for(source_file, params)

Credo.Code.prewalk(source_file, &traverse(&1, &2, issue_meta))
Credo.Code.prewalk(source_file, &traverse(&1, &2, issue_meta, parens?))
end

for op <- @def_ops do
# catch variables named e.g. `defp`
defp traverse({unquote(op), _, nil} = ast, issues, _issue_meta) do
defp traverse({unquote(op), _, nil} = ast, issues, _issue_meta, _parens?) do
{ast, issues}
end

defp traverse({unquote(op), _, body} = ast, issues, issue_meta) do
defp traverse({unquote(op), _, body} = ast, issues, issue_meta, parens?) do
function_head = Enum.at(body, 0)

{ast, issues_for_definition(function_head, issues, issue_meta)}
{ast, issues_for_definition(function_head, issues, issue_meta, parens?)}
end
end

defp traverse(ast, issues, _issue_meta) do
defp traverse(ast, issues, _issue_meta, _parens?) do
{ast, issues}
end

# skip the false positive for a metaprogrammed definition
defp issues_for_definition({{:unquote, _, _}, _, _}, issues, _) do
defp issues_for_definition({{:unquote, _, _}, _, _}, issues, _, _parens?) do
issues
end

defp issues_for_definition({_, _, args}, issues, _) when length(args) > 0 do
defp issues_for_definition({_, _, args}, issues, _, _parens?) when length(args) > 0 do
issues
end

defp issues_for_definition({name, meta, _}, issues, issue_meta) do
defp issues_for_definition({name, meta, _}, issues, issue_meta, enforce_parens?) do
line_no = meta[:line]
text = remaining_line_after(issue_meta, line_no, name)
parens? = String.match?(text, ~r/^\((\w*)\)(.)*/)

if String.match?(text, ~r/^\((\w*)\)(.)*/) do
issues ++ [issue_for(issue_meta, line_no)]
else
issues
cond do
parens? and not enforce_parens? ->
issues ++ [issue_for(issue_meta, line_no, :present)]

not parens? and enforce_parens? ->
issues ++ [issue_for(issue_meta, line_no, :missing)]

true ->
issues
end
end

Expand All @@ -78,11 +91,16 @@ defmodule Credo.Check.Readability.ParenthesesOnZeroArityDefs do
String.slice(line, skip..-1)
end

defp issue_for(issue_meta, line_no) do
format_issue(
issue_meta,
message: "Do not use parentheses when defining a function which has no arguments.",
line_no: line_no
)
defp issue_for(issue_meta, line_no, kind) do
message =
case kind do
:present ->
"Do not use parentheses when defining a function which has no arguments."

:missing ->
"Use parentheses () when defining a function which has no arguments."
end

format_issue(issue_meta, message: message, line_no: line_no)
end
end
Expand Up @@ -27,7 +27,7 @@ defmodule Credo.Check.Readability.ParenthesesOnZeroArityDefsTest do
# cases raising issues
#

test "it should report a violation" do
test "it should report a violation with parens (by default)" do
"""
defmodule Mix.Tasks.Credo do
def run() do
Expand All @@ -39,6 +39,18 @@ defmodule Credo.Check.Readability.ParenthesesOnZeroArityDefsTest do
|> assert_issue(@described_check)
end

test "it should report a violation with no parens if parens: true" do
"""
defmodule Mix.Tasks.Credo do
def run do
21
end
end
"""
|> to_source_file
|> assert_issue(@described_check, [parens: true], _callback = nil)
end

test "it should not crash on macros creating zero arity functions" do
"""
defmodule Credo.Sample.Module do
Expand Down

0 comments on commit 39e3f2f

Please sign in to comment.