clippy_lints/src/unwrap.rs

   1 use if_chain::if_chain;
   2 use rustc::lint::{LateContext, LateLintPass, LintArray, LintPass};
   3 use rustc::{declare_tool_lint, lint_array};
   4
   5 use crate::utils::{in_macro, match_type, paths, span_lint_and_then, usage::is_potentially_mutated};
   6 use rustc::hir::intravisit::*;
   7 use rustc::hir::*;
   8 use syntax::ast::NodeId;
   9 use syntax::source_map::Span;
  10
  11 /// **What it does:** Checks for calls of `unwrap[_err]()` that cannot fail.
  12 ///
  13 /// **Why is this bad?** Using `if let` or `match` is more idiomatic.
  14 ///
  15 /// **Known problems:** Limitations of the borrow checker might make unwrap() necessary sometimes?
  16 ///
  17 /// **Example:**
  18 /// ```rust
  19 /// if option.is_some() {
  20 ///     do_something_with(option.unwrap())
  21 /// }
  22 /// ```
  23 ///
  24 /// Could be written:
  25 ///
  26 /// ```rust
  27 /// if let Some(value) = option {
  28 ///     do_something_with(value)
  29 /// }
  30 /// ```
  31 declare_clippy_lint! {
  32     pub UNNECESSARY_UNWRAP,
  33     nursery,
  34     "checks for calls of unwrap[_err]() that cannot fail"
  35 }
  36
  37 /// **What it does:** Checks for calls of `unwrap[_err]()` that will always fail.
  38 ///
  39 /// **Why is this bad?** If panicking is desired, an explicit `panic!()` should be used.
  40 ///
  41 /// **Known problems:** This lint only checks `if` conditions not assignments.
  42 /// So something like `let x: Option<()> = None; x.unwrap();` will not be recognized.
  43 ///
  44 /// **Example:**
  45 /// ```rust
  46 /// if option.is_none() {
  47 ///     do_something_with(option.unwrap())
  48 /// }
  49 /// ```
  50 ///
  51 /// This code will always panic. The if condition should probably be inverted.
  52 declare_clippy_lint! {
  53     pub PANICKING_UNWRAP,
  54     nursery,
  55     "checks for calls of unwrap[_err]() that will always fail"
  56 }
  57
  58 pub struct Pass;
  59
  60 /// Visitor that keeps track of which variables are unwrappable.
  61 struct UnwrappableVariablesVisitor<'a, 'tcx: 'a> {
  62     unwrappables: Vec<UnwrapInfo<'tcx>>,
  63     cx: &'a LateContext<'a, 'tcx>,
  64 }
  65 /// Contains information about whether a variable can be unwrapped.
  66 #[derive(Copy, Clone, Debug)]
  67 struct UnwrapInfo<'tcx> {
  68     /// The variable that is checked
  69     ident: &'tcx Path,
  70     /// The check, like `x.is_ok()`
  71     check: &'tcx Expr,
  72     /// Whether `is_some()` or `is_ok()` was called (as opposed to `is_err()` or `is_none()`).
  73     safe_to_unwrap: bool,
  74 }
  75
  76 /// Collects the information about unwrappable variables from an if condition
  77 /// The `invert` argument tells us whether the condition is negated.
  78 fn collect_unwrap_info<'a, 'tcx: 'a>(
  79     cx: &'a LateContext<'a, 'tcx>,
  80     expr: &'tcx Expr,
  81     invert: bool,
  82 ) -> Vec<UnwrapInfo<'tcx>> {
  83     if let ExprKind::Binary(op, left, right) = &expr.node {
  84         match (invert, op.node) {
  85             (false, BinOpKind::And) | (false, BinOpKind::BitAnd) | (true, BinOpKind::Or) | (true, BinOpKind::BitOr) => {
  86                 let mut unwrap_info = collect_unwrap_info(cx, left, invert);
  87                 unwrap_info.append(&mut collect_unwrap_info(cx, right, invert));
  88                 return unwrap_info;
  89             },
  90             _ => (),
  91         }
  92     } else if let ExprKind::Unary(UnNot, expr) = &expr.node {
  93         return collect_unwrap_info(cx, expr, !invert);
  94     } else {
  95         if_chain! {
  96             if let ExprKind::MethodCall(method_name, _, args) = &expr.node;
  97             if let ExprKind::Path(QPath::Resolved(None, path)) = &args[0].node;
  98             let ty = cx.tables.expr_ty(&args[0]);
  99             if match_type(cx, ty, &paths::OPTION) || match_type(cx, ty, &paths::RESULT);
 100             let name = method_name.ident.as_str();
 101             if ["is_some", "is_none", "is_ok", "is_err"].contains(&&*name);
 102             then {
 103                 assert!(args.len() == 1);
 104                 let unwrappable = match name.as_ref() {
 105                     "is_some" | "is_ok" => true,
 106                     "is_err" | "is_none" => false,
 107                     _ => unreachable!(),
 108                 };
 109                 let safe_to_unwrap = unwrappable != invert;
 110                 return vec![UnwrapInfo { ident: path, check: expr, safe_to_unwrap }];
 111             }
 112         }
 113     }
 114     Vec::new()
 115 }
 116
 117 impl<'a, 'tcx: 'a> UnwrappableVariablesVisitor<'a, 'tcx> {
 118     fn visit_branch(&mut self, cond: &'tcx Expr, branch: &'tcx Expr, else_branch: bool) {
 119         let prev_len = self.unwrappables.len();
 120         for unwrap_info in collect_unwrap_info(self.cx, cond, else_branch) {
 121             if is_potentially_mutated(unwrap_info.ident, cond, self.cx)
 122                 || is_potentially_mutated(unwrap_info.ident, branch, self.cx)
 123             {
 124                 // if the variable is mutated, we don't know whether it can be unwrapped:
 125                 continue;
 126             }
 127             self.unwrappables.push(unwrap_info);
 128         }
 129         walk_expr(self, branch);
 130         self.unwrappables.truncate(prev_len);
 131     }
 132 }
 133
 134 impl<'a, 'tcx: 'a> Visitor<'tcx> for UnwrappableVariablesVisitor<'a, 'tcx> {
 135     fn visit_expr(&mut self, expr: &'tcx Expr) {
 136         if let ExprKind::If(cond, then, els) = &expr.node {
 137             walk_expr(self, cond);
 138             self.visit_branch(cond, then, false);
 139             if let Some(els) = els {
 140                 self.visit_branch(cond, els, true);
 141             }
 142         } else {
 143             // find `unwrap[_err]()` calls:
 144             if_chain! {
 145                 if let ExprKind::MethodCall(ref method_name, _, ref args) = expr.node;
 146                 if let ExprKind::Path(QPath::Resolved(None, ref path)) = args[0].node;
 147                 if ["unwrap", "unwrap_err"].contains(&&*method_name.ident.as_str());
 148                 let call_to_unwrap = method_name.ident.name == "unwrap";
 149                 if let Some(unwrappable) = self.unwrappables.iter()
 150                     .find(|u| u.ident.def == path.def);
 151                 then {
 152                     if call_to_unwrap == unwrappable.safe_to_unwrap {
 153                         span_lint_and_then(
 154                             self.cx,
 155                             UNNECESSARY_UNWRAP,
 156                             expr.span,
 157                             &format!("You checked before that `{}()` cannot fail. \
 158                             Instead of checking and unwrapping, it's better to use `if let` or `match`.",
 159                             method_name.ident.name),
 160                             |db| { db.span_label(unwrappable.check.span, "the check is happening here"); },
 161                         );
 162                     } else {
 163                         span_lint_and_then(
 164                             self.cx,
 165                             PANICKING_UNWRAP,
 166                             expr.span,
 167                             &format!("This call to `{}()` will always panic.",
 168                             method_name.ident.name),
 169                             |db| { db.span_label(unwrappable.check.span, "because of this check"); },
 170                         );
 171                     }
 172                 }
 173             }
 174             walk_expr(self, expr);
 175         }
 176     }
 177
 178     fn nested_visit_map<'this>(&'this mut self) -> NestedVisitorMap<'this, 'tcx> {
 179         NestedVisitorMap::OnlyBodies(&self.cx.tcx.hir())
 180     }
 181 }
 182
 183 impl<'a> LintPass for Pass {
 184     fn get_lints(&self) -> LintArray {
 185         lint_array!(PANICKING_UNWRAP, UNNECESSARY_UNWRAP)
 186     }
 187 }
 188
 189 impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Pass {
 190     fn check_fn(
 191         &mut self,
 192         cx: &LateContext<'a, 'tcx>,
 193         kind: FnKind<'tcx>,
 194         decl: &'tcx FnDecl,
 195         body: &'tcx Body,
 196         span: Span,
 197         fn_id: NodeId,
 198     ) {
 199         if in_macro(span) {
 200             return;
 201         }
 202
 203         let mut v = UnwrappableVariablesVisitor {
 204             cx,
 205             unwrappables: Vec::new(),
 206         };
 207
 208         walk_fn(&mut v, kind, decl, body.id(), span, fn_id);
 209     }
 210 }