Packages
credo
0.3.0-dev2
1.7.19
1.7.18
1.7.17
1.7.16
1.7.15
1.7.14
1.7.13
1.7.12
1.7.11
1.7.10
1.7.9
1.7.8
1.7.7
1.7.7-rc.0
1.7.6
1.7.5
1.7.4
1.7.3
1.7.2
1.7.2-rc.4
1.7.2-rc.3
1.7.2-rc.2
1.7.2-rc.1
1.7.2-rc.0
1.7.1
1.7.0
1.7.0-rc.2
1.7.0-rc.1
1.6.7
1.6.6
1.6.5
1.6.4
1.6.3
1.6.2
1.6.1
1.6.0
1.6.0-rc.1
1.6.0-rc.0
1.5.6
1.5.5
1.5.4
1.5.3
1.5.2
1.5.1
1.5.0
1.5.0-rc.5
1.5.0-rc.4
1.5.0-rc.3
1.5.0-rc.2
1.5.0-rc.1
1.4.1
1.4.0
1.4.0-rc.2
1.4.0-rc.1
1.3.2
1.3.1
1.3.0
1.3.0-rc3
1.3.0-rc2
1.3.0-rc1
1.2.3
1.2.2
1.2.1
1.2.0
1.2.0-rc4
1.2.0-rc3
1.2.0-rc2
1.2.0-rc1
1.1.5
1.1.4
1.1.3
1.1.2
1.1.1
1.1.0
1.1.0-rc3
1.1.0-rc2
1.1.0-rc1
1.0.5
1.0.4
1.0.3
1.0.2
1.0.1
1.0.1-rc1
1.0.0
1.0.0-rc1
0.10.2
0.10.1
0.10.0
0.9.3
0.9.2
0.9.1
0.9.0
0.9.0-rc8
0.9.0-rc7
0.9.0-rc6
0.9.0-rc5
0.9.0-rc4
0.9.0-rc3
0.9.0-rc2
0.9.0-rc1
0.8.10
0.8.9
0.8.8
0.8.7
0.8.6
0.8.5
0.8.4
0.8.3
0.8.2
0.8.1
0.8.0
0.8.0-rc7
0.8.0-rc6
0.8.0-rc5
0.8.0-rc4
0.8.0-rc3
0.8.0-rc2
0.8.0-rc1
0.7.4
0.7.3
0.7.2
0.7.1
0.7.0
0.6.1
0.6.0
0.6.0-rc2
0.6.0-rc1
0.5.3
0.5.2
0.5.1
0.5.0
0.4.14
0.4.13
0.4.12
0.4.11
0.4.10
0.4.10-dev
0.4.9
0.4.8
0.4.7
0.4.6
0.4.5
0.4.4
0.4.3
0.4.2
0.4.1
0.4.0
0.4.0-beta5
0.4.0-beta4
0.4.0-beta3
0.4.0-beta2
0.4.0-beta1
0.3.13
0.3.12
0.3.11
0.3.10
0.3.9
0.3.8
0.3.7
0.3.6
0.3.5
0.3.4
0.3.3
0.3.2
0.3.1
0.3.0
0.3.0-dev2
0.3.0-dev
0.2.6
0.2.5
0.2.4
0.2.3
0.2.2
0.2.1
0.2.0
0.1.10
0.1.9
0.1.8
0.1.7
0.1.6
0.1.5
0.1.4
0.1.3
0.1.2
0.1.1
0.1.0
0.0.1-dev
A static code analysis tool with a focus on code consistency and teaching.
Current section
Files
Jump to
Current section
Files
lib/credo/check/refactor/negated_conditions_with_else.ex
defmodule Credo.Check.Refactor.NegatedConditionsWithElse do
@moduledoc """
An `if` block with a negated condition should not contain an else block.
So while this is fine:
if !allowed? do
raise "Not allowed!"
end
The code in this example ...
if !allowed? do
raise "Not allowed!"
else
proceed_as_planned
end
... should be refactored to look like this:
if allowed? do
proceed_as_planned
else
raise "Not allowed!"
end
The reason for this is not a technical but a human one. It is easier to wrap
your head around a positive condition and then thinking "and else we do ...".
In the above example raising the error in case something is not allowed
might seem so important to put it first. But when you revisit this code a
while later or have to introduce a colleague to it, you might be surprised
how much clearer things get when the "happy path" comes first.
"""
@explanation [check: @moduledoc]
use Credo.Check, base_priority: :high
def run(source_file, params \\ []) do
issue_meta = IssueMeta.for(source_file, params)
Credo.Code.traverse(source_file, &traverse(&1, &2, issue_meta))
end
defp traverse({:if, meta, arguments} = ast, issues, issue_meta) do
if negated_condition?(arguments) && CodeHelper.else_block?(ast) do
new_issue =
issue_for(meta[:line], "!", issue_meta)
{ast, issues ++ [new_issue]}
else
{ast, issues}
end
end
defp traverse(ast, issues, _issue_meta) do
{ast, issues}
end
defp negated_condition?(arguments) when is_list(arguments) do
negated_condition?(arguments |> List.first)
end
defp negated_condition?({:!, _meta, _arguments}) do
true
end
defp negated_condition?(_) do
false
end
defp issue_for(line_no, trigger, issue_meta) do
format_issue issue_meta,
message: "If/else blocks should not have a negated condition in `if`.",
trigger: trigger,
line_no: line_no
end
end