Skip to content

Commit

Permalink
Merge pull request #1121 from quangngd/ignore_predicate_name_when_impl
Browse files Browse the repository at this point in the history
  • Loading branch information
rrrene authored Mar 31, 2024
2 parents 3fb97c5 + 1b905d5 commit d066fc0
Show file tree
Hide file tree
Showing 2 changed files with 170 additions and 23 deletions.
108 changes: 85 additions & 23 deletions lib/credo/check/readability/predicate_function_names.ex
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -66,56 +122,62 @@ 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

defp traverse(ast, issues, _issue_meta) 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
85 changes: 85 additions & 0 deletions test/credo/check/readability/predicate_function_names_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -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
#
Expand Down

0 comments on commit d066fc0

Please sign in to comment.