with_single_clause.ex (3205B)
1 defmodule Credo.Check.Readability.WithSingleClause do 2 use Credo.Check, 3 explanations: [ 4 check: ~S""" 5 `with` statements are useful when you need to chain a sequence 6 of pattern matches, stopping at the first one that fails. 7 8 If the `with` has a single pattern matching clause and no `else` 9 branch, it means that if the clause doesn't match than the whole 10 `with` will return the value of that clause. 11 12 However, if that `with` has also an `else` clause, then you're using `with` exactly 13 like a `case` and a `case` should be used instead. 14 15 Take this code: 16 17 with {:ok, user} <- User.create(make_ref()) do 18 user 19 else 20 {:error, :db_down} -> 21 raise "DB is down!" 22 23 {:error, reason} -> 24 raise "error: #{inspect(reason)}" 25 end 26 27 It can be rewritten with a clearer use of `case`: 28 29 case User.create(make_ref()) do 30 {:ok, user} -> 31 user 32 33 {:error, :db_down} -> 34 raise "DB is down!" 35 36 {:error, reason} -> 37 raise "error: #{inspect(reason)}" 38 end 39 40 Like all `Readability` issues, this one is not a technical concern. 41 But you can improve the odds of others reading and liking your code by making 42 it easier to follow. 43 """ 44 ] 45 46 @doc false 47 @impl true 48 def run(%SourceFile{} = source_file, params) do 49 issue_meta = IssueMeta.for(source_file, params) 50 51 Credo.Code.prewalk(source_file, &traverse(&1, &2, issue_meta)) 52 end 53 54 # TODO: consider for experimental check front-loader (ast) 55 defp traverse({:with, meta, [_, _ | _] = clauses_and_body} = ast, issues, issue_meta) 56 when is_list(clauses_and_body) do 57 # If clauses_and_body is a list with at least two elements in it, we think 58 # this might be a call to the special form "with". To be sure of that, 59 # we get the last element of clauses_and_body and check that it's a keyword 60 # list with a :do key in it (the body). 61 62 # We can hard-match on [maybe_body] here since we know that clauses_and_body 63 # has at least two elements. 64 {maybe_clauses, [maybe_body]} = Enum.split(clauses_and_body, -1) 65 66 if Keyword.keyword?(maybe_body) and Keyword.has_key?(maybe_body, :do) do 67 issue = 68 issue_if_one_pattern_clause_with_else(maybe_clauses, maybe_body, meta[:line], issue_meta) 69 70 {ast, issue ++ issues} 71 else 72 {ast, issues} 73 end 74 end 75 76 defp traverse(ast, issues, _issue_meta) do 77 {ast, issues} 78 end 79 80 defp issue_if_one_pattern_clause_with_else(clauses, body, line, issue_meta) do 81 contains_unquote_splicing? = Enum.any?(clauses, &match?({:unquote_splicing, _, _}, &1)) 82 pattern_clauses_count = Enum.count(clauses, &match?({:<-, _, _}, &1)) 83 84 cond do 85 contains_unquote_splicing? -> 86 [] 87 88 pattern_clauses_count <= 1 and Keyword.has_key?(body, :else) -> 89 [ 90 format_issue(issue_meta, 91 message: 92 "`with` contains only one <- clause and an `else` branch, consider using `case` instead", 93 line_no: line 94 ) 95 ] 96 97 true -> 98 [] 99 end 100 end 101 end