Repository: rrrene/credo
Pull request #774 (2020-05-08 13:40:12)

Correctly deduplicate multiclauses
In the previous version, module part analysis would treat multiclauses as distinct functions. That cause problems with the `StrictModuleLayout`.

For example:

```elixir
@impl
def handle_call(...), do:
def handle_call(...), do:
```

The second clause would be incorrectly interpreted as a public function.


Patch:
diff --git a/lib/credo/code/module.ex b/lib/credo/code/module.ex
index 14699af7..8ffd3828 100644
--- a/lib/credo/code/module.ex
+++ b/lib/credo/code/module.ex
@@ -381,11 +381,18 @@ defmodule Credo.Code.Module do
        when clause in ~w/use import alias require defstruct/a,
        do: add_module_element(state, clause, meta)
 
-  defp analyze(state, {clause, meta, _})
+  defp analyze(state, {clause, meta, definition})
        when clause in ~w/def defmacro defguard defp defmacrop defguardp/a do
-    state
-    |> add_module_element(code_type(clause, state.next_fun_modifier), meta)
-    |> clear_next_fun_modifier()
+    fun_name = fun_name(definition)
+
+    if fun_name != state.last_fun_name do
+      state
+      |> add_module_element(code_type(clause, state.next_fun_modifier), meta)
+      |> Map.put(:last_fun_name, fun_name)
+      |> clear_next_fun_modifier()
+    else
+      state
+    end
   end
 
   defp analyze(state, {:do, _code}) do
@@ -399,6 +406,10 @@ defmodule Credo.Code.Module do
 
   defp analyze(_state, _ast), do: nil
 
+  defp fun_name([{name, _context, arity} | _]) when is_list(arity), do: {name, length(arity)}
+  defp fun_name([{name, _context, _} | _]), do: {name, 0}
+  defp fun_name(_), do: nil
+
   defp code_type(:def, nil), do: :public_fun
   defp code_type(:def, :impl), do: :callback_fun
   defp code_type(:def, :private), do: :private_fun
@@ -413,7 +424,8 @@ defmodule Credo.Code.Module do
 
   # Internal state
 
-  defp initial_state, do: %{modules: [], current_module: nil, next_fun_modifier: nil}
+  defp initial_state,
+    do: %{modules: [], current_module: nil, next_fun_modifier: nil, last_fun_name: nil}
 
   defp set_next_fun_modifier(state, value), do: %{state | next_fun_modifier: value}
 


Test patch:
diff --git a/test/credo/code/module_test.exs b/test/credo/code/module_test.exs
index f890934e..c9c39c6b 100644
--- a/test/credo/code/module_test.exs
+++ b/test/credo/code/module_test.exs
@@ -511,6 +511,30 @@ defmodule Credo.Code.ModuleTest do
                [{Test, [callback_fun: [line: 3, column: 3]]}]
     end
 
+    test "deduplicates multiclauses" do
+      assert analyze("""
+             def a(1), do: true
+             def a(2), do: false
+
+             @impl true
+             def b(1), do: true
+             def b(2), do: false
+
+             @doc false
+             def c(1), do: true
+             def c(2), do: false
+
+             defp d(1), do: true
+             defp d(2), do: false
+             """) == [
+               {Test,
+                public_fun: [line: 2, column: 3],
+                callback_fun: [line: 6, column: 3],
+                private_fun: [line: 10, column: 3],
+                private_fun: [line: 13, column: 3]}
+             ]
+    end
+
     test "handles multiple modules" do
       full_source =
         """