Skip to content

Commit

Permalink
Fix trigger for NegatedConditionsWithElse
Browse files Browse the repository at this point in the history
  • Loading branch information
rrrene committed Feb 1, 2024
1 parent 599fd4c commit 6bd912e
Show file tree
Hide file tree
Showing 2 changed files with 20 additions and 18 deletions.
28 changes: 12 additions & 16 deletions lib/credo/check/refactor/negated_conditions_with_else.ex
Original file line number Diff line number Diff line change
Expand Up @@ -49,8 +49,10 @@ defmodule Credo.Check.Refactor.NegatedConditionsWithElse do
end

defp traverse({:if, meta, arguments} = ast, issues, issue_meta) do
if negated_condition?(arguments) && Credo.Code.Block.else_block?(ast) do
new_issue = issue_for(issue_meta, meta[:line], "!")
negator = negated_condition(arguments)

if negator && Credo.Code.Block.else_block?(ast) do
new_issue = issue_for(issue_meta, meta[:line], negator)

{ast, issues ++ [new_issue]}
else
Expand All @@ -62,27 +64,21 @@ defmodule Credo.Check.Refactor.NegatedConditionsWithElse do
{ast, issues}
end

defp negated_condition?(arguments) when is_list(arguments) do
arguments |> List.first() |> negated_condition?()
end

defp negated_condition?({:!, _meta, _arguments}) do
true
end
defp negated_condition({:!, _, _}), do: "!"

defp negated_condition?({:not, _meta, _arguments}) do
true
end
defp negated_condition({:not, _, _}), do: "not"

# parentheses around the condition wrap it in a __block__
defp negated_condition?({:__block__, _meta, arguments}) do
negated_condition?(arguments)
defp negated_condition({:__block__, _, arguments}) do
negated_condition(arguments)
end

defp negated_condition?(_) do
false
defp negated_condition(arguments) when is_list(arguments) do
arguments |> List.first() |> negated_condition()
end

defp negated_condition(_), do: nil

defp issue_for(issue_meta, line_no, trigger) do
format_issue(
issue_meta,
Expand Down
10 changes: 8 additions & 2 deletions test/credo/check/refactor/negated_conditions_with_else_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,10 @@ defmodule Credo.Check.Refactor.NegatedConditionsWithElseTest do
"""
|> to_source_file
|> run_check(@described_check)
|> assert_issue()
|> assert_issue(fn issue ->
assert issue.line_no == 3
assert issue.trigger == "!"
end)
end

test "it should report a violation with not/2 as well" do
Expand All @@ -77,6 +80,9 @@ defmodule Credo.Check.Refactor.NegatedConditionsWithElseTest do
"""
|> to_source_file
|> run_check(@described_check)
|> assert_issue()
|> assert_issue(fn issue ->
assert issue.line_no == 3
assert issue.trigger == "not"
end)
end
end

0 comments on commit 6bd912e

Please sign in to comment.