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