Merge pull request #1166 from Wigny/wigny/add-unusedvariablenames-checks

Add `UnusedVariableNames` missing checks

Author: rrrene

lib/credo/check/consistency/unused_variable_names/collector.ex

@@ -17,12 +17,12 @@ defmodule Credo.Check.Consistency.UnusedVariableNames.Collector do
     |> Enum.reverse()
   end
 
-  defp traverse(callback, {:=, _, params} = ast, acc) do
+  defp traverse(callback, {match, _, params} = ast, acc) when match in ~w[= <-]a do
     {ast, reduce_unused_variables(params, callback, acc)}
   end
 
   defp traverse(callback, {def, _, [{_, _, params} | _]} = ast, acc)
-       when def in [:def, :defp] do
+       when def in [:def, :defp, :defmacro, :defmacrop] do
     {ast, reduce_unused_variables(params, callback, acc)}
   end
 
@@ -39,6 +39,12 @@ defmodule Credo.Check.Consistency.UnusedVariableNames.Collector do
       {_, _, params}, param_acc when is_list(params) ->
         reduce_unused_variables(params, callback, param_acc)
 
+      tuple_ast, param_acc when tuple_size(tuple_ast) == 2 ->
+        reduce_unused_variables(Tuple.to_list(tuple_ast), callback, param_acc)
+
+      list_ast, param_acc when is_list(list_ast) ->
+        reduce_unused_variables(list_ast, callback, param_acc)
+
       param_ast, param_acc ->
         if unused_variable_ast?(param_ast) do
           callback.(param_ast, param_acc)

test/credo/check/consistency/unused_variable_names_test.exs

@@ -221,6 +221,135 @@ defmodule Credo.Check.Consistency.UnusedVariableNamesTest do
     end)
   end
 
+  test "it should report a violation for different naming schemes in a two elem tuple match (expects meaningful)" do
+    [
+      """
+      defmodule Credo.SampleOne do
+        defmodule Foo do
+          def bar(x1, x2) do
+            {_a, _b} = x1
+            {_c, _} = x2
+          end
+        end
+      end
+      """,
+      """
+      defmodule Credo.SampleTwo do
+        defmodule Foo do
+          def bar(x1, x2) do
+            with {:ok, _} <- x1,
+                 {:ok, _b} <- x2, do: :ok
+          end
+        end
+      end
+      """
+    ]
+    |> to_source_files()
+    |> run_check(@described_check)
+    |> assert_issues(fn issues ->
+      assert length(issues) == 2
+
+      assert Enum.find(issues, &match?(%{trigger: "_", line_no: 5}, &1))
+      assert Enum.find(issues, &match?(%{trigger: "_", line_no: 4}, &1))
+    end)
+  end
+
+  test "it should report a violation for different naming schemes with a map match (expects meaningful)" do
+    [
+      """
+      defmodule Credo.SampleOne do
+        defmodule Foo do
+          def bar(%{a: _a, b: _b, c: _}) do
+            :ok
+          end
+        end
+      end
+      """,
+      """
+      defmodule Credo.SampleTwo do
+        defmodule Foo do
+          def bar(map) do
+            case map do
+              %{a: _} -> :ok
+              _map -> :error
+            end
+          end
+        end
+      end
+      """
+    ]
+    |> to_source_files()
+    |> run_check(@described_check)
+    |> assert_issues(fn issues ->
+      assert length(issues) == 2
+
+      assert Enum.find(issues, &match?(%{trigger: "_", line_no: 3}, &1))
+      assert Enum.find(issues, &match?(%{trigger: "_", line_no: 5}, &1))
+    end)
+  end
+
+  test "it should report a violation for different naming schemes with a list match (expects meaningful)" do
+    [
+      """
+      defmodule Credo.SampleOne do
+        defmodule Foo do
+          def bar(list) do
+            case list do
+              [] -> :empty
+              [head | _] -> head
+            end
+          end
+        end
+      end
+      """,
+      """
+      defmodule Credo.SampleTwo do
+        defmodule Foo do
+          def bar([_a, _b | rest]) do
+            rest
+          end
+        end
+      end
+      """
+    ]
+    |> to_source_files()
+    |> run_check(@described_check)
+    |> assert_issue(fn issue ->
+      assert "_" == issue.trigger
+      assert 6 == issue.line_no
+    end)
+  end
+
+  test "it should report a violation for different naming schemes with a macro (expects meaningful)" do
+    [
+      """
+      defmodule Credo.SampleOne do
+        defmodule Foo do
+          defmacro __using__(_) do
+          end
+        end
+
+        def bar(_opts) do
+        end
+      end
+      """,
+      """
+      defmodule Credo.SampleTwo do
+        defmodule Foo do
+          defmacrop bar(_opts) do
+          end
+        end
+      end
+      """
+    ]
+    |> to_source_files()
+    |> run_check(@described_check)
+    |> assert_issue(fn issue ->
+      assert "_" == issue.trigger
+      assert 3 == issue.line_no
+    end)
+  end
+
   test "it should report a violation for naming schemes other than the forced one" do
     [
       """