diff --git a/lib/credo/check/readability/predicate_function_names.ex b/lib/credo/check/readability/predicate_function_names.ex index 4a51b50b7..fe2ec3a3a 100644 --- a/lib/credo/check/readability/predicate_function_names.ex +++ b/lib/credo/check/readability/predicate_function_names.ex @@ -52,7 +52,63 @@ defmodule Credo.Check.Readability.PredicateFunctionNames 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)) + issue_candidates = Credo.Code.prewalk(source_file, &traverse(&1, &2, issue_meta)) + + if issue_candidates == [] do + [] + else + impl_list = Credo.Code.prewalk(source_file, &find_impls(&1, &2)) + + issue_candidates + |> Enum.reject(fn {_, signature} -> signature in impl_list end) + |> Enum.map(fn {issue, _} -> issue end) + end + end + + defp find_impls({:__block__, _meta, args} = ast, impls) do + block_impls = find_impls_in_block(args) + {ast, block_impls ++ impls} + end + + defp find_impls(ast, impls) do + {ast, impls} + end + + defp find_impls_in_block(block_args) when is_list(block_args) do + block_args + |> Enum.reduce([], &do_find_impls_in_block/2) + end + + defp do_find_impls_in_block({:@, _, [{:impl, _, [impl]}]}, acc) when impl != false do + [:record_next_definition | acc] + end + + # def when + defp do_find_impls_in_block({keyword, meta, [{:when, _, def_ast} | _]}, [ + :record_next_definition | impls + ]) + when keyword in @def_ops do + do_find_impls_in_block({keyword, meta, def_ast}, [:record_next_definition | impls]) + end + + # def 0 arity + defp do_find_impls_in_block({keyword, _meta, [{name, _, nil} | _]}, [ + :record_next_definition | impls + ]) + when keyword in @def_ops do + [{to_string(name), 0} | impls] + end + + # def n arity + defp do_find_impls_in_block({keyword, _meta, [{name, _, args} | _]}, [ + :record_next_definition | impls + ]) + when keyword in @def_ops do + [{to_string(name), length(args)} | impls] + end + + defp do_find_impls_in_block(_, acc) do + acc end for op <- @def_ops do @@ -66,7 +122,7 @@ defmodule Credo.Check.Readability.PredicateFunctionNames do issues, issue_meta ) do - {ast, issues_for_definition(op, arguments, issues, issue_meta)} + {ast, issues_candidate_for_definition(op, arguments, issues, issue_meta)} end end @@ -74,48 +130,54 @@ defmodule Credo.Check.Readability.PredicateFunctionNames do {ast, issues} end - defp issues_for_definition(op, body, issues, issue_meta) do - case Enum.at(body, 0) do - {name, meta, nil} -> - issues_for_name(op, name, meta, issues, issue_meta) + defp issues_candidate_for_definition(op, [{name, meta, nil} | _], issues, issue_meta) do + issues_candidate_for_definition(op, [{name, meta, []}], issues, issue_meta) + end - {name, meta, [_ | _]} -> - issues_for_name(op, name, meta, issues, issue_meta) + defp issues_candidate_for_definition(op, [{name, meta, args} | _], issues, issue_meta) do + issues_candidate_for_name(op, name, meta, issues, issue_meta, args) + end - _ -> - issues - end + defp issues_candidate_for_definition(_op, _, issues, _issue_meta) do + issues end - defp issues_for_name(_op, {:unquote, _, [_ | _]} = _name, _meta, issues, _issue_meta) do + defp issues_candidate_for_name( + _op, + {:unquote, _, [_ | _]} = _name, + _meta, + issues, + _issue_meta, + _args + ) do issues end - defp issues_for_name(op, name, meta, issues, issue_meta) do + defp issues_candidate_for_name(op, name, meta, issues, issue_meta, args) do name = to_string(name) cond do String.starts_with?(name, "is_") && String.ends_with?(name, "?") -> [ - issue_for(issue_meta, meta[:line], name, :predicate_and_question_mark) + issue_candidate_for(issue_meta, meta[:line], name, args, :predicate_and_question_mark) | issues ] String.starts_with?(name, "is_") && op != :defmacro -> - [issue_for(issue_meta, meta[:line], name, :only_predicate) | issues] + [issue_candidate_for(issue_meta, meta[:line], name, args, :only_predicate) | issues] true -> issues end end - defp issue_for(issue_meta, line_no, trigger, _) do - format_issue( - issue_meta, - message: - "Predicate function names should not start with 'is', and should end in a question mark.", - trigger: trigger, - line_no: line_no - ) + defp issue_candidate_for(issue_meta, line_no, trigger, args, _) do + {format_issue( + issue_meta, + message: + "Predicate function names should not start with 'is', and should end in a question mark.", + trigger: trigger, + line_no: line_no + ), {trigger, length(args)}} end end diff --git a/test/credo/check/readability/predicate_function_names_test.exs b/test/credo/check/readability/predicate_function_names_test.exs index 2ff3301e0..69ef8ee0c 100644 --- a/test/credo/check/readability/predicate_function_names_test.exs +++ b/test/credo/check/readability/predicate_function_names_test.exs @@ -81,6 +81,91 @@ defmodule Credo.Check.Readability.PredicateFunctionNamesTest do |> refute_issues() end + test "it should NOT report a violation with callback" do + """ + defmodule Foo do + @callback is_bar + @callback is_bar(a) + end + + defmodule FooImpl do + @behaviour Foo + + @impl Foo + def is_bar do + end + + @impl Foo + def is_bar(a) when is_binary(a) do + end + + @impl Foo + def is_bar(a) do + end + end + """ + |> to_source_file + |> run_check(@described_check) + |> refute_issues() + end + + test "it should report a violation with false negatives" do + """ + defmodule FooImpl do + def impl(false), do: false + def impl(true), do: true + def is_bar do + end + end + """ + |> to_source_file + |> run_check(@described_check) + |> assert_issue() + end + + test "it should report a violation with false negatives /2" do + """ + defmodule FooImpl do + impl(true) + def is_bar do + end + end + """ + |> to_source_file + |> run_check(@described_check) + |> assert_issue() + end + + test "it should report a violation with false negatives /3" do + """ + defmodule Foo do + @impl is_bar(a) + end + defmodule FooImpl do + def is_bar do + end + end + """ + |> to_source_file + |> run_check(@described_check) + |> assert_issue() + end + + test "it should report a violation with false negatives /4" do + """ + defmodule Foo do + @impl true + end + defmodule FooImpl do + def is_bar do + end + end + """ + |> to_source_file + |> run_check(@described_check) + |> assert_issue() + end + # # cases raising issues #