From f14e915483f0a1c1b09d83c750f952f695544f15 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?W=C3=ADgny?= Date: Mon, 9 Dec 2024 11:30:06 -0400 Subject: [PATCH 1/4] Check in two elem tuples --- .../unused_variable_names/collector.ex | 6 +++- .../unused_variable_names_test.exs | 33 +++++++++++++++++++ 2 files changed, 38 insertions(+), 1 deletion(-) diff --git a/lib/credo/check/consistency/unused_variable_names/collector.ex b/lib/credo/check/consistency/unused_variable_names/collector.ex index 02997ebcc..919b48084 100644 --- a/lib/credo/check/consistency/unused_variable_names/collector.ex +++ b/lib/credo/check/consistency/unused_variable_names/collector.ex @@ -17,7 +17,7 @@ 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 @@ -39,6 +39,10 @@ defmodule Credo.Check.Consistency.UnusedVariableNames.Collector do {_, _, params}, param_acc when is_list(params) -> reduce_unused_variables(params, callback, param_acc) + # two elements tuple + {left, right}, param_acc -> + reduce_unused_variables([left, right], callback, param_acc) + param_ast, param_acc -> if unused_variable_ast?(param_ast) do callback.(param_ast, param_acc) diff --git a/test/credo/check/consistency/unused_variable_names_test.exs b/test/credo/check/consistency/unused_variable_names_test.exs index 6c7e6e890..60f6edb8b 100644 --- a/test/credo/check/consistency/unused_variable_names_test.exs +++ b/test/credo/check/consistency/unused_variable_names_test.exs @@ -221,6 +221,39 @@ defmodule Credo.Check.Consistency.UnusedVariableNamesTest do end) end + test "it should report a violation for different naming schemes with 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 naming schemes other than the forced one" do [ """ From 2d36c6d19575d8cf1b0851a610617594a4139ad2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?W=C3=ADgny?= Date: Mon, 9 Dec 2024 11:39:28 -0400 Subject: [PATCH 2/4] Check in lists --- .../unused_variable_names/collector.ex | 4 +++ .../unused_variable_names_test.exs | 34 ++++++++++++++++++- 2 files changed, 37 insertions(+), 1 deletion(-) diff --git a/lib/credo/check/consistency/unused_variable_names/collector.ex b/lib/credo/check/consistency/unused_variable_names/collector.ex index 919b48084..5378dd8a6 100644 --- a/lib/credo/check/consistency/unused_variable_names/collector.ex +++ b/lib/credo/check/consistency/unused_variable_names/collector.ex @@ -43,8 +43,12 @@ defmodule Credo.Check.Consistency.UnusedVariableNames.Collector do {left, right}, param_acc -> reduce_unused_variables([left, right], 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 + IO.inspect(param_ast, label: :param_ast) callback.(param_ast, param_acc) else param_acc diff --git a/test/credo/check/consistency/unused_variable_names_test.exs b/test/credo/check/consistency/unused_variable_names_test.exs index 60f6edb8b..9af2d4cbe 100644 --- a/test/credo/check/consistency/unused_variable_names_test.exs +++ b/test/credo/check/consistency/unused_variable_names_test.exs @@ -221,7 +221,7 @@ defmodule Credo.Check.Consistency.UnusedVariableNamesTest do end) end - test "it should report a violation for different naming schemes with two elem tuple match (expects meaningful)" do + test "it should report a violation for different naming schemes in a two elem tuple match (expects meaningful)" do [ """ defmodule Credo.SampleOne do @@ -254,6 +254,38 @@ defmodule Credo.Check.Consistency.UnusedVariableNamesTest do end) end + test "it should report a violation for different naming schemes with a list (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 naming schemes other than the forced one" do [ """ From 264ce87659fd8a5b3711faf8d3287416f56dd824 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?W=C3=ADgny?= Date: Mon, 9 Dec 2024 11:45:25 -0400 Subject: [PATCH 3/4] Check in macros --- .../unused_variable_names/collector.ex | 2 +- .../unused_variable_names_test.exs | 30 +++++++++++++++++++ 2 files changed, 31 insertions(+), 1 deletion(-) diff --git a/lib/credo/check/consistency/unused_variable_names/collector.ex b/lib/credo/check/consistency/unused_variable_names/collector.ex index 5378dd8a6..ff235e9bf 100644 --- a/lib/credo/check/consistency/unused_variable_names/collector.ex +++ b/lib/credo/check/consistency/unused_variable_names/collector.ex @@ -22,7 +22,7 @@ defmodule Credo.Check.Consistency.UnusedVariableNames.Collector do 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 diff --git a/test/credo/check/consistency/unused_variable_names_test.exs b/test/credo/check/consistency/unused_variable_names_test.exs index 9af2d4cbe..5708e2e6f 100644 --- a/test/credo/check/consistency/unused_variable_names_test.exs +++ b/test/credo/check/consistency/unused_variable_names_test.exs @@ -286,6 +286,36 @@ defmodule Credo.Check.Consistency.UnusedVariableNamesTest do 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 [ """ From 68aaace28eece64888efaf39a5c8f13ef3db2180 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?W=C3=ADgny?= Date: Mon, 9 Dec 2024 12:08:18 -0400 Subject: [PATCH 4/4] Check in maps --- .../unused_variable_names/collector.ex | 6 ++-- .../unused_variable_names_test.exs | 36 ++++++++++++++++++- 2 files changed, 37 insertions(+), 5 deletions(-) diff --git a/lib/credo/check/consistency/unused_variable_names/collector.ex b/lib/credo/check/consistency/unused_variable_names/collector.ex index ff235e9bf..f2298543a 100644 --- a/lib/credo/check/consistency/unused_variable_names/collector.ex +++ b/lib/credo/check/consistency/unused_variable_names/collector.ex @@ -39,16 +39,14 @@ defmodule Credo.Check.Consistency.UnusedVariableNames.Collector do {_, _, params}, param_acc when is_list(params) -> reduce_unused_variables(params, callback, param_acc) - # two elements tuple - {left, right}, param_acc -> - reduce_unused_variables([left, right], 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 - IO.inspect(param_ast, label: :param_ast) callback.(param_ast, param_acc) else param_acc diff --git a/test/credo/check/consistency/unused_variable_names_test.exs b/test/credo/check/consistency/unused_variable_names_test.exs index 5708e2e6f..0f0c5524d 100644 --- a/test/credo/check/consistency/unused_variable_names_test.exs +++ b/test/credo/check/consistency/unused_variable_names_test.exs @@ -254,7 +254,41 @@ defmodule Credo.Check.Consistency.UnusedVariableNamesTest do end) end - test "it should report a violation for different naming schemes with a list (expects meaningful)" do + 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