zf

zenflows testing
git clone https://s.sonu.ch/~srfsh/zf.git
Log | Files | Refs | Submodules | README | LICENSE

negated_conditions_with_else.ex (2526B)

      1 defmodule Credo.Check.Refactor.NegatedConditionsWithElse do
      2   use Credo.Check,
      3     base_priority: :high,
      4     explanations: [
      5       check: """
      6       An `if` block with a negated condition should not contain an else block.
      7 
      8       So while this is fine:
      9 
     10           if not allowed? do
     11             raise "Not allowed!"
     12           end
     13 
     14       The code in this example ...
     15 
     16           if not allowed? do
     17             raise "Not allowed!"
     18           else
     19             proceed_as_planned()
     20           end
     21 
     22       ... should be refactored to look like this:
     23 
     24           if allowed? do
     25             proceed_as_planned()
     26           else
     27             raise "Not allowed!"
     28           end
     29 
     30       The same goes for negation through `!` instead of `not`.
     31 
     32       The reason for this is not a technical but a human one. It is easier to wrap
     33       your head around a positive condition and then thinking "and else we do ...".
     34 
     35       In the above example raising the error in case something is not allowed
     36       might seem so important to put it first. But when you revisit this code a
     37       while later or have to introduce a colleague to it, you might be surprised
     38       how much clearer things get when the "happy path" comes first.
     39       """
     40     ]
     41 
     42   @doc false
     43   @impl true
     44   def run(%SourceFile{} = source_file, params) do
     45     issue_meta = IssueMeta.for(source_file, params)
     46 
     47     Credo.Code.prewalk(source_file, &traverse(&1, &2, issue_meta))
     48   end
     49 
     50   # TODO: consider for experimental check front-loader (ast)
     51   defp traverse({:if, meta, arguments} = ast, issues, issue_meta) do
     52     if negated_condition?(arguments) && Credo.Code.Block.else_block?(ast) do
     53       new_issue = issue_for(issue_meta, meta[:line], "!")
     54 
     55       {ast, issues ++ [new_issue]}
     56     else
     57       {ast, issues}
     58     end
     59   end
     60 
     61   defp traverse(ast, issues, _issue_meta) do
     62     {ast, issues}
     63   end
     64 
     65   defp negated_condition?(arguments) when is_list(arguments) do
     66     arguments |> List.first() |> negated_condition?()
     67   end
     68 
     69   defp negated_condition?({:!, _meta, _arguments}) do
     70     true
     71   end
     72 
     73   defp negated_condition?({:not, _meta, _arguments}) do
     74     true
     75   end
     76 
     77   # parentheses around the condition wrap it in a __block__
     78   defp negated_condition?({:__block__, _meta, arguments}) do
     79     negated_condition?(arguments)
     80   end
     81 
     82   defp negated_condition?(_) do
     83     false
     84   end
     85 
     86   defp issue_for(issue_meta, line_no, trigger) do
     87     format_issue(
     88       issue_meta,
     89       message: "Avoid negated conditions in if-else blocks.",
     90       trigger: trigger,
     91       line_no: line_no
     92     )
     93   end
     94 end