Merge pull request #676 from sasa1977/improve-function-names

Improve Readability.FunctionNames

lib/credo/check/readability/function_names.ex

@@ -2,7 +2,7 @@ defmodule Credo.Check.Readability.FunctionNames do
   @moduledoc false
 
   @checkdoc """
-  Function and macro names are always written in snake_case in Elixir.
+  Function, macro, and guard names are always written in snake_case in Elixir.
 
       # snake_case
 
@@ -19,7 +19,7 @@ defmodule Credo.Check.Readability.FunctionNames do
   it easier to follow.
   """
   @explanation [check: @checkdoc]
-  @def_ops [:def, :defp, :defmacro]
+  @def_ops [:def, :defp, :defmacro, :defmacrop, :defguard, :defguardp]
 
   use Credo.Check, base_priority: :high
 
@@ -29,7 +29,19 @@ defmodule Credo.Check.Readability.FunctionNames do
   def run(%SourceFile{} = source_file, params \\ []) do
     issue_meta = IssueMeta.for(source_file, params)
 
-    Credo.Code.prewalk(source_file, &traverse(&1, &2, issue_meta))
+    source_file
+    |> Credo.Code.prewalk(&traverse(&1, &2, issue_meta), empty_issues())
+    |> issues_list()
+  end
+
+  defp empty_issues(), do: %{}
+
+  defp add_issue(issues, name, arity, issue), do: Map.put_new(issues, {name, arity}, issue)
+
+  defp issues_list(issues) do
+    issues
+    |> Map.values()
+    |> Enum.sort_by(& &1.line_no)
   end
 
   for op <- @def_ops do
@@ -49,26 +61,31 @@ defmodule Credo.Check.Readability.FunctionNames do
 
   defp issues_for_definition(body, issues, issue_meta) do
     case Enum.at(body, 0) do
-      {name, meta, nil} ->
-        issues_for_name(name, meta, issues, issue_meta)
+      {:when, _when_meta, [{name, meta, args} | _guard]} ->
+        issues_for_name(name, args, meta, issues, issue_meta)
+
+      {name, meta, args} when is_atom(name) ->
+        issues_for_name(name, args, meta, issues, issue_meta)
 
       _ ->
         issues
     end
   end
 
-  defp issues_for_name(name, meta, issues, issue_meta) do
+  defp issues_for_name(name, args, meta, issues, issue_meta) do
     if name |> to_string |> Name.snake_case?() do
       issues
     else
-      [issue_for(issue_meta, meta[:line], name) | issues]
+      issue = issue_for(issue_meta, meta[:line], name)
+      arity = length(args || [])
+      add_issue(issues, name, arity, issue)
     end
   end
 
   defp issue_for(issue_meta, line_no, trigger) do
     format_issue(
       issue_meta,
-      message: "Function/macro names should be written in snake_case.",
+      message: "Function/macro/guard names should be written in snake_case.",
       trigger: trigger,
       line_no: line_no
     )

test/credo/check/readability/function_names_test.exs

@@ -15,6 +15,10 @@ defmodule Credo.Check.Readability.FunctionNamesTest do
     end
     defmacro credo_sample_macro do
     end
+    defmacrop credo_sample_macro_p do
+    end
+    defguard credo_sample_guard(x) when is_integer(x)
+    defguardp credo_sample_guard(x) when is_integer(x)
     """
     |> to_source_file
     |> refute_issues(@described_check)
@@ -63,10 +67,100 @@ defmodule Credo.Check.Readability.FunctionNamesTest do
 
   test "it should report a violation /4" do
     """
-    defmacro credo_Sample_Function do
+    defmacro credo_Sample_Macro do
+    end
+    """
+    |> to_source_file
+    |> assert_issue(@described_check)
+  end
+
+  test "it should report a violation /5" do
+    """
+    defmacrop credo_Sample_Macro_p do
+    end
+    """
+    |> to_source_file
+    |> assert_issue(@described_check)
+  end
+
+  test "it should report a violation /6" do
+    """
+    def credoSampleFunction() do
+    end
+    """
+    |> to_source_file
+    |> assert_issue(@described_check)
+  end
+
+  test "it should report a violation /7" do
+    """
+    def credoSampleFunction(x, y) do
+    end
+    """
+    |> to_source_file
+    |> assert_issue(@described_check)
+  end
+
+  test "it should report a violation /8" do
+    """
+    defmacro credoSampleMacro() do
+    end
+    """
+    |> to_source_file
+    |> assert_issue(@described_check)
+  end
+
+  test "it should report a violation /9" do
+    """
+    defmacro credoSampleMacro(x, y) do
+    end
+    """
+    |> to_source_file
+    |> assert_issue(@described_check)
+  end
+
+  test "it should report a violation /10" do
+    """
+    def credo_SampleFunction when 1 == 1 do
     end
     """
     |> to_source_file
     |> assert_issue(@described_check)
   end
+
+  test "it should report a violation /11" do
+    """
+    def credo_SampleFunction(x, y) when x == y do
+    end
+    """
+    |> to_source_file
+    |> assert_issue(@described_check)
+  end
+
+  test "it should report a violation /12" do
+    """
+    defguard credo_SampleGuard(x) when is_integer(x)
+    """
+    |> to_source_file
+    |> assert_issue(@described_check)
+  end
+
+  test "it should report a violation /13" do
+    """
+    defguardp credo_SampleGuard(x) when is_integer(x)
+    """
+    |> to_source_file
+    |> assert_issue(@described_check)
+  end
+
+  test "it should report a violation /14" do
+    """
+    def credoSampleFunction(0), do: :ok
+    def credoSampleFunction(1), do: :ok
+    def credoSampleFunction(2), do: :ok
+    def credoSampleFunction(_), do: :ok
+    """
+    |> to_source_file
+    |> assert_issue(@described_check)
+  end
 end