From 42dd76001345cdeceec1e95a85bdb3c5a8be31e8 Mon Sep 17 00:00:00 2001
From: Andrea Leopardi <an.leopardi@gmail.com>
Date: Sun, 30 Jun 2019 23:46:52 +0200
Subject: [PATCH] Add an option to enforce parens or not in zero-arity defs

---
 .../parentheses_on_zero_arity_defs.ex         | 64 ++++++++++++-------
 .../parentheses_on_zero_arity_defs_test.exs   | 14 +++-
 2 files changed, 54 insertions(+), 24 deletions(-)

diff --git a/lib/credo/check/readability/parentheses_on_zero_arity_defs.ex b/lib/credo/check/readability/parentheses_on_zero_arity_defs.ex
index 5173efd23..fa95fbb76 100644
--- a/lib/credo/check/readability/parentheses_on_zero_arity_defs.ex
+++ b/lib/credo/check/readability/parentheses_on_zero_arity_defs.ex
@@ -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
 
@@ -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
 
@@ -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
diff --git a/test/credo/check/readability/parentheses_on_zero_arity_defs_test.exs b/test/credo/check/readability/parentheses_on_zero_arity_defs_test.exs
index 8f2fd5398..bc5dcc1b4 100644
--- a/test/credo/check/readability/parentheses_on_zero_arity_defs_test.exs
+++ b/test/credo/check/readability/parentheses_on_zero_arity_defs_test.exs
@@ -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
@@ -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