clippy_lints/src/unwrap.rs

   1 use clippy_utils::diagnostics::span_lint_and_then;
   2 use clippy_utils::higher;
   3 use clippy_utils::ty::is_type_diagnostic_item;
   4 use clippy_utils::{differing_macro_contexts, path_to_local, usage::is_potentially_mutated};
   5 use if_chain::if_chain;
   6 use rustc_errors::Applicability;
   7 use rustc_hir::intravisit::{walk_expr, walk_fn, FnKind, NestedVisitorMap, Visitor};
   8 use rustc_hir::{BinOpKind, Body, Expr, ExprKind, FnDecl, HirId, PathSegment, UnOp};
   9 use rustc_lint::{LateContext, LateLintPass};
  10 use rustc_middle::hir::map::Map;
  11 use rustc_middle::lint::in_external_macro;
  12 use rustc_middle::ty::Ty;
  13 use rustc_session::{declare_lint_pass, declare_tool_lint};
  14 use rustc_span::source_map::Span;
  15 use rustc_span::sym;
  16
  17 declare_clippy_lint! {
  18     /// ### What it does
  19     /// Checks for calls of `unwrap[_err]()` that cannot fail.
  20     ///
  21     /// ### Why is this bad?
  22     /// Using `if let` or `match` is more idiomatic.
  23     ///
  24     /// ### Example
  25     /// ```rust
  26     /// # let option = Some(0);
  27     /// # fn do_something_with(_x: usize) {}
  28     /// if option.is_some() {
  29     ///     do_something_with(option.unwrap())
  30     /// }
  31     /// ```
  32     ///
  33     /// Could be written:
  34     ///
  35     /// ```rust
  36     /// # let option = Some(0);
  37     /// # fn do_something_with(_x: usize) {}
  38     /// if let Some(value) = option {
  39     ///     do_something_with(value)
  40     /// }
  41     /// ```
  42     pub UNNECESSARY_UNWRAP,
  43     complexity,
  44     "checks for calls of `unwrap[_err]()` that cannot fail"
  45 }
  46
  47 declare_clippy_lint! {
  48     /// ### What it does
  49     /// Checks for calls of `unwrap[_err]()` that will always fail.
  50     ///
  51     /// ### Why is this bad?
  52     /// If panicking is desired, an explicit `panic!()` should be used.
  53     ///
  54     /// ### Known problems
  55     /// This lint only checks `if` conditions not assignments.
  56     /// So something like `let x: Option<()> = None; x.unwrap();` will not be recognized.
  57     ///
  58     /// ### Example
  59     /// ```rust
  60     /// # let option = Some(0);
  61     /// # fn do_something_with(_x: usize) {}
  62     /// if option.is_none() {
  63     ///     do_something_with(option.unwrap())
  64     /// }
  65     /// ```
  66     ///
  67     /// This code will always panic. The if condition should probably be inverted.
  68     pub PANICKING_UNWRAP,
  69     correctness,
  70     "checks for calls of `unwrap[_err]()` that will always fail"
  71 }
  72
  73 /// Visitor that keeps track of which variables are unwrappable.
  74 struct UnwrappableVariablesVisitor<'a, 'tcx> {
  75     unwrappables: Vec<UnwrapInfo<'tcx>>,
  76     cx: &'a LateContext<'tcx>,
  77 }
  78
  79 /// What kind of unwrappable this is.
  80 #[derive(Copy, Clone, Debug)]
  81 enum UnwrappableKind {
  82     Option,
  83     Result,
  84 }
  85
  86 impl UnwrappableKind {
  87     fn success_variant_pattern(self) -> &'static str {
  88         match self {
  89             UnwrappableKind::Option => "Some(..)",
  90             UnwrappableKind::Result => "Ok(..)",
  91         }
  92     }
  93
  94     fn error_variant_pattern(self) -> &'static str {
  95         match self {
  96             UnwrappableKind::Option => "None",
  97             UnwrappableKind::Result => "Err(..)",
  98         }
  99     }
 100 }
 101
 102 /// Contains information about whether a variable can be unwrapped.
 103 #[derive(Copy, Clone, Debug)]
 104 struct UnwrapInfo<'tcx> {
 105     /// The variable that is checked
 106     local_id: HirId,
 107     /// The if itself
 108     if_expr: &'tcx Expr<'tcx>,
 109     /// The check, like `x.is_ok()`
 110     check: &'tcx Expr<'tcx>,
 111     /// The check's name, like `is_ok`
 112     check_name: &'tcx PathSegment<'tcx>,
 113     /// The branch where the check takes place, like `if x.is_ok() { .. }`
 114     branch: &'tcx Expr<'tcx>,
 115     /// Whether `is_some()` or `is_ok()` was called (as opposed to `is_err()` or `is_none()`).
 116     safe_to_unwrap: bool,
 117     /// What kind of unwrappable this is.
 118     kind: UnwrappableKind,
 119     /// If the check is the entire condition (`if x.is_ok()`) or only a part of it (`foo() &&
 120     /// x.is_ok()`)
 121     is_entire_condition: bool,
 122 }
 123
 124 /// Collects the information about unwrappable variables from an if condition
 125 /// The `invert` argument tells us whether the condition is negated.
 126 fn collect_unwrap_info<'tcx>(
 127     cx: &LateContext<'tcx>,
 128     if_expr: &'tcx Expr<'_>,
 129     expr: &'tcx Expr<'_>,
 130     branch: &'tcx Expr<'_>,
 131     invert: bool,
 132     is_entire_condition: bool,
 133 ) -> Vec<UnwrapInfo<'tcx>> {
 134     fn is_relevant_option_call(cx: &LateContext<'_>, ty: Ty<'_>, method_name: &str) -> bool {
 135         is_type_diagnostic_item(cx, ty, sym::Option) && ["is_some", "is_none"].contains(&method_name)
 136     }
 137
 138     fn is_relevant_result_call(cx: &LateContext<'_>, ty: Ty<'_>, method_name: &str) -> bool {
 139         is_type_diagnostic_item(cx, ty, sym::Result) && ["is_ok", "is_err"].contains(&method_name)
 140     }
 141
 142     if let ExprKind::Binary(op, left, right) = &expr.kind {
 143         match (invert, op.node) {
 144             (false, BinOpKind::And | BinOpKind::BitAnd) | (true, BinOpKind::Or | BinOpKind::BitOr) => {
 145                 let mut unwrap_info = collect_unwrap_info(cx, if_expr, left, branch, invert, false);
 146                 unwrap_info.append(&mut collect_unwrap_info(cx, if_expr, right, branch, invert, false));
 147                 return unwrap_info;
 148             },
 149             _ => (),
 150         }
 151     } else if let ExprKind::Unary(UnOp::Not, expr) = &expr.kind {
 152         return collect_unwrap_info(cx, if_expr, expr, branch, !invert, false);
 153     } else {
 154         if_chain! {
 155             if let ExprKind::MethodCall(method_name, _, args, _) = &expr.kind;
 156             if let Some(local_id) = path_to_local(&args[0]);
 157             let ty = cx.typeck_results().expr_ty(&args[0]);
 158             let name = method_name.ident.as_str();
 159             if is_relevant_option_call(cx, ty, &name) || is_relevant_result_call(cx, ty, &name);
 160             then {
 161                 assert!(args.len() == 1);
 162                 let unwrappable = match name.as_ref() {
 163                     "is_some" | "is_ok" => true,
 164                     "is_err" | "is_none" => false,
 165                     _ => unreachable!(),
 166                 };
 167                 let safe_to_unwrap = unwrappable != invert;
 168                 let kind = if is_type_diagnostic_item(cx, ty, sym::Option) {
 169                     UnwrappableKind::Option
 170                 } else {
 171                     UnwrappableKind::Result
 172                 };
 173
 174                 return vec![
 175                     UnwrapInfo {
 176                         local_id,
 177                         if_expr,
 178                         check: expr,
 179                         check_name: method_name,
 180                         branch,
 181                         safe_to_unwrap,
 182                         kind,
 183                         is_entire_condition,
 184                     }
 185                 ]
 186             }
 187         }
 188     }
 189     Vec::new()
 190 }
 191
 192 impl<'a, 'tcx> UnwrappableVariablesVisitor<'a, 'tcx> {
 193     fn visit_branch(
 194         &mut self,
 195         if_expr: &'tcx Expr<'_>,
 196         cond: &'tcx Expr<'_>,
 197         branch: &'tcx Expr<'_>,
 198         else_branch: bool,
 199     ) {
 200         let prev_len = self.unwrappables.len();
 201         for unwrap_info in collect_unwrap_info(self.cx, if_expr, cond, branch, else_branch, true) {
 202             if is_potentially_mutated(unwrap_info.local_id, cond, self.cx)
 203                 || is_potentially_mutated(unwrap_info.local_id, branch, self.cx)
 204             {
 205                 // if the variable is mutated, we don't know whether it can be unwrapped:
 206                 continue;
 207             }
 208             self.unwrappables.push(unwrap_info);
 209         }
 210         walk_expr(self, branch);
 211         self.unwrappables.truncate(prev_len);
 212     }
 213 }
 214
 215 impl<'a, 'tcx> Visitor<'tcx> for UnwrappableVariablesVisitor<'a, 'tcx> {
 216     type Map = Map<'tcx>;
 217
 218     fn visit_expr(&mut self, expr: &'tcx Expr<'_>) {
 219         // Shouldn't lint when `expr` is in macro.
 220         if in_external_macro(self.cx.tcx.sess, expr.span) {
 221             return;
 222         }
 223         if let Some(higher::If { cond, then, r#else }) = higher::If::hir(expr) {
 224             walk_expr(self, cond);
 225             self.visit_branch(expr, cond, then, false);
 226             if let Some(else_inner) = r#else {
 227                 self.visit_branch(expr, cond, else_inner, true);
 228             }
 229         } else {
 230             // find `unwrap[_err]()` calls:
 231             if_chain! {
 232                 if let ExprKind::MethodCall(method_name, _, args, _) = expr.kind;
 233                 if let Some(id) = path_to_local(&args[0]);
 234                 if [sym::unwrap, sym::expect, sym!(unwrap_err)].contains(&method_name.ident.name);
 235                 let call_to_unwrap = [sym::unwrap, sym::expect].contains(&method_name.ident.name);
 236                 if let Some(unwrappable) = self.unwrappables.iter()
 237                     .find(|u| u.local_id == id);
 238                 // Span contexts should not differ with the conditional branch
 239                 if !differing_macro_contexts(unwrappable.branch.span, expr.span);
 240                 if !differing_macro_contexts(unwrappable.branch.span, unwrappable.check.span);
 241                 then {
 242                     if call_to_unwrap == unwrappable.safe_to_unwrap {
 243                         let is_entire_condition = unwrappable.is_entire_condition;
 244                         let unwrappable_variable_name = self.cx.tcx.hir().name(unwrappable.local_id);
 245                         let suggested_pattern = if call_to_unwrap {
 246                             unwrappable.kind.success_variant_pattern()
 247                         } else {
 248                             unwrappable.kind.error_variant_pattern()
 249                         };
 250
 251                         span_lint_and_then(
 252                             self.cx,
 253                             UNNECESSARY_UNWRAP,
 254                             expr.span,
 255                             &format!(
 256                                 "called `{}` on `{}` after checking its variant with `{}`",
 257                                 method_name.ident.name,
 258                                 unwrappable_variable_name,
 259                                 unwrappable.check_name.ident.as_str(),
 260                             ),
 261                             |diag| {
 262                                 if is_entire_condition {
 263                                     diag.span_suggestion(
 264                                         unwrappable.check.span.with_lo(unwrappable.if_expr.span.lo()),
 265                                         "try",
 266                                         format!(
 267                                             "if let {} = {}",
 268                                             suggested_pattern,
 269                                             unwrappable_variable_name,
 270                                         ),
 271                                         // We don't track how the unwrapped value is used inside the
 272                                         // block or suggest deleting the unwrap, so we can't offer a
 273                                         // fixable solution.
 274                                         Applicability::Unspecified,
 275                                     );
 276                                 } else {
 277                                     diag.span_label(unwrappable.check.span, "the check is happening here");
 278                                     diag.help("try using `if let` or `match`");
 279                                 }
 280                             },
 281                         );
 282                     } else {
 283                         span_lint_and_then(
 284                             self.cx,
 285                             PANICKING_UNWRAP,
 286                             expr.span,
 287                             &format!("this call to `{}()` will always panic",
 288                             method_name.ident.name),
 289                             |diag| { diag.span_label(unwrappable.check.span, "because of this check"); },
 290                         );
 291                     }
 292                 }
 293             }
 294             walk_expr(self, expr);
 295         }
 296     }
 297
 298     fn nested_visit_map(&mut self) -> NestedVisitorMap<Self::Map> {
 299         NestedVisitorMap::OnlyBodies(self.cx.tcx.hir())
 300     }
 301 }
 302
 303 declare_lint_pass!(Unwrap => [PANICKING_UNWRAP, UNNECESSARY_UNWRAP]);
 304
 305 impl<'tcx> LateLintPass<'tcx> for Unwrap {
 306     fn check_fn(
 307         &mut self,
 308         cx: &LateContext<'tcx>,
 309         kind: FnKind<'tcx>,
 310         decl: &'tcx FnDecl<'_>,
 311         body: &'tcx Body<'_>,
 312         span: Span,
 313         fn_id: HirId,
 314     ) {
 315         if span.from_expansion() {
 316             return;
 317         }
 318
 319         let mut v = UnwrappableVariablesVisitor {
 320             cx,
 321             unwrappables: Vec::new(),
 322         };
 323
 324         walk_fn(&mut v, kind, decl, body.id(), span, fn_id);
 325     }
 326 }