negated_is_nil.ex (2157B)
1 defmodule Credo.Check.Refactor.NegatedIsNil do 2 use Credo.Check, 3 base_priority: :low, 4 tags: [:controversial], 5 explanations: [ 6 check: """ 7 We should avoid negating the `is_nil` predicate function. 8 9 For example, the code here ... 10 11 def fun(%{external_id: external_id, id: id}) when not is_nil(external_id) do 12 # ... 13 end 14 15 ... can be refactored to look like this: 16 17 def fun(%{external_id: nil, id: id}) do 18 # ... 19 end 20 21 def fun(%{external_id: external_id, id: id}) do 22 # ... 23 end 24 25 ... or even better, can match on what you were expecting on the first place: 26 27 def fun(%{external_id: external_id, id: id}) when is_binary(external_id) do 28 # ... 29 end 30 31 def fun(%{external_id: nil, id: id}) do 32 # ... 33 end 34 35 def fun(%{external_id: external_id, id: id}) do 36 # ... 37 end 38 39 Similar to negating `unless` blocks, the reason for this check is not 40 technical, but a human one. If we can use the positive, more direct and human 41 friendly case, we should. 42 """ 43 ] 44 45 @doc false 46 @impl true 47 def run(%SourceFile{} = source_file, params \\ []) do 48 issue_meta = IssueMeta.for(source_file, params) 49 50 Credo.Code.prewalk(source_file, &traverse(&1, &2, issue_meta)) 51 end 52 53 defp traverse({:when, meta, [_, {negation, _, [{:is_nil, _, _}]}]} = ast, issues, issue_meta) 54 when negation in [:!, :not] do 55 issue = 56 format_issue( 57 issue_meta, 58 message: "Negated is_nil in guard clause found", 59 trigger: "when !/not is_nil", 60 line_no: meta[:line] 61 ) 62 63 {ast, [issue | issues]} 64 end 65 66 defp traverse({:when, meta, [fun, {_ , _, [first_op | second_op]}]} = ast, issues, issue_meta) do 67 {_, first_op_issues} = traverse({:when, meta, [fun, first_op]}, issues, issue_meta) 68 {_, second_op_issues} = traverse({:when, meta, [fun, second_op]}, issues, issue_meta) 69 70 {ast, first_op_issues ++ second_op_issues ++ issues} 71 end 72 73 defp traverse(ast, issues, _), do: {ast, issues} 74 end