Implement lint that checks for unidiomatic unwrap() (fixes #1770)

This checks for things like

    if x.is_some() {
        x.unwrap()
    }

which should be written using `if let` or `match` instead.

In the process I moved some logic to determine which variables are
mutated in an expression to utils/usage.rs.

clippy_lints/src/lib.rs

@@ -200,6 +200,7 @@ pub mod unicode;
 pub mod unsafe_removed_from_name;
 pub mod unused_io_amount;
 pub mod unused_label;
+pub mod unwrap;
 pub mod use_self;
 pub mod vec;
 pub mod write;
@@ -421,6 +422,8 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry) {
     reg.register_late_lint_pass(box infallible_destructuring_match::Pass);
     reg.register_late_lint_pass(box inherent_impl::Pass::default());
     reg.register_late_lint_pass(box neg_cmp_op_on_partial_ord::NoNegCompOpForPartialOrd);
+    reg.register_late_lint_pass(box unwrap::Pass);
+
 
     reg.register_lint_group("clippy_restriction", vec![
         arithmetic::FLOAT_ARITHMETIC,
@@ -837,6 +840,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry) {
         types::UNIT_ARG,
         types::UNNECESSARY_CAST,
         unused_label::UNUSED_LABEL,
+        unwrap::UNNECESSARY_UNWRAP,
         zero_div_zero::ZERO_DIVIDED_BY_ZERO,
     ]);
 

clippy_lints/src/loops.rs

@@ -18,6 +18,7 @@ use std::iter::{once, Iterator};
 use syntax::ast;
 use syntax::codemap::Span;
 use crate::utils::{sugg, sext};
+use crate::utils::usage::mutated_variables;
 use crate::consts::{constant, Constant};
 
 use crate::utils::{get_enclosing_block, get_parent_expr, higher, in_external_macro, is_integer_literal, is_refutable,
@@ -504,8 +505,8 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Pass {
         }
 
         // check for while loops which conditions never change
-        if let ExprWhile(ref cond, ref block, _) = expr.node {
-            check_infinite_loop(cx, cond, block, expr);
+        if let ExprWhile(ref cond, _, _) = expr.node {
+            check_infinite_loop(cx, cond, expr);
         }
     }
 
@@ -2145,35 +2146,30 @@ fn path_name(e: &Expr) -> Option<Name> {
     None
 }
 
-fn check_infinite_loop<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, cond: &'tcx Expr, block: &'tcx Block, expr: &'tcx Expr) {
+fn check_infinite_loop<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, cond: &'tcx Expr, expr: &'tcx Expr) {
     if constant(cx, cx.tables, cond).is_some() {
         // A pure constant condition (e.g. while false) is not linted.
         return;
     }
 
-    let mut mut_var_visitor = VarCollectorVisitor {
+    let mut var_visitor = VarCollectorVisitor {
         cx,
-        ids: HashMap::new(),
+        ids: HashSet::new(),
         def_ids: HashMap::new(),
         skip: false,
     };
-    mut_var_visitor.visit_expr(cond);
-    if mut_var_visitor.skip {
+    var_visitor.visit_expr(cond);
+    if var_visitor.skip {
         return;
     }
-
-    let mut delegate = MutVarsDelegate {
-        used_mutably: mut_var_visitor.ids,
-        skip: false,
+    let used_in_condition = &var_visitor.ids;
+    let no_cond_variable_mutated = if let Some(used_mutably) = mutated_variables(expr, cx) {
+        used_in_condition.is_disjoint(&used_mutably)
+    } else {
+        return
     };
-    let def_id = def_id::DefId::local(block.hir_id.owner);
-    let region_scope_tree = &cx.tcx.region_scope_tree(def_id);
-    ExprUseVisitor::new(&mut delegate, cx.tcx, cx.param_env, region_scope_tree, cx.tables, None).walk_expr(expr);
-
-    if delegate.skip {
-        return;
-    }
-    if !(delegate.used_mutably.iter().any(|(_, v)| *v) || mut_var_visitor.def_ids.iter().any(|(_, v)| *v)) {
+    let mutable_static_in_cond = var_visitor.def_ids.iter().any(|(_, v)| *v);
+    if no_cond_variable_mutated && !mutable_static_in_cond {
         span_lint(
             cx,
             WHILE_IMMUTABLE_CONDITION,
@@ -2189,7 +2185,7 @@ fn check_infinite_loop<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, cond: &'tcx Expr, b
 /// All variables definition IDs are collected
 struct VarCollectorVisitor<'a, 'tcx: 'a> {
     cx: &'a LateContext<'a, 'tcx>,
-    ids: HashMap<NodeId, bool>,
+    ids: HashSet<NodeId>,
     def_ids: HashMap<def_id::DefId, bool>,
     skip: bool,
 }
@@ -2203,7 +2199,7 @@ impl<'a, 'tcx> VarCollectorVisitor<'a, 'tcx> {
             then {
                 match def {
                     Def::Local(node_id) | Def::Upvar(node_id, ..) => {
-                        self.ids.insert(node_id, false);
+                        self.ids.insert(node_id);
                     },
                     Def::Static(def_id, mutable) => {
                         self.def_ids.insert(def_id, mutable);
@@ -2230,48 +2226,3 @@ impl<'a, 'tcx> Visitor<'tcx> for VarCollectorVisitor<'a, 'tcx> {
         NestedVisitorMap::None
     }
 }
-
-struct MutVarsDelegate {
-    used_mutably: HashMap<NodeId, bool>,
-    skip: bool,
-}
-
-impl<'tcx> MutVarsDelegate {
-    fn update(&mut self, cat: &'tcx Categorization) {
-        match *cat {
-            Categorization::Local(id) =>
-                if let Some(used) = self.used_mutably.get_mut(&id) {
-                    *used = true;
-                },
-            Categorization::Upvar(_) => {
-                //FIXME: This causes false negatives. We can't get the `NodeId` from
-                //`Categorization::Upvar(_)`. So we search for any `Upvar`s in the
-                //`while`-body, not just the ones in the condition.
-                self.skip = true
-            },
-            Categorization::Deref(ref cmt, _) | Categorization::Interior(ref cmt, _) => self.update(&cmt.cat),
-            _ => {}
-        }
-    }
-}
-
-
-impl<'tcx> Delegate<'tcx> for MutVarsDelegate {
-    fn consume(&mut self, _: NodeId, _: Span, _: &cmt_<'tcx>, _: ConsumeMode) {}
-
-    fn matched_pat(&mut self, _: &Pat, _: &cmt_<'tcx>, _: MatchMode) {}
-
-    fn consume_pat(&mut self, _: &Pat, _: &cmt_<'tcx>, _: ConsumeMode) {}
-
-    fn borrow(&mut self, _: NodeId, _: Span, cmt: &cmt_<'tcx>, _: ty::Region, bk: ty::BorrowKind, _: LoanCause) {
-        if let ty::BorrowKind::MutBorrow = bk {
-            self.update(&cmt.cat)
-        }
-    }
-
-    fn mutate(&mut self, _: NodeId, _: Span, cmt: &cmt_<'tcx>, _: MutateMode) {
-        self.update(&cmt.cat)
-    }
-
-    fn decl_without_init(&mut self, _: NodeId, _: Span) {}
-}

clippy_lints/src/unwrap.rs

@@ -0,0 +1,176 @@
+use rustc::lint::*;
+
+use rustc::hir::intravisit::*;
+use rustc::hir::*;
+use syntax::ast::NodeId;
+use syntax::codemap::Span;
+use crate::utils::{in_macro, match_type, paths, usage::is_potentially_mutated};
+
+/// **What it does:** Checks for calls of unwrap[_err]() that cannot fail.
+///
+/// **Why is this bad?** Using `if let` or `match` is more idiomatic.
+///
+/// **Known problems:** Limitations of the borrow checker might make unwrap() necessary sometimes?
+///
+/// **Example:**
+/// ```rust
+/// if option.is_some() {
+///     do_something_with(option.unwrap())
+/// }
+/// ```
+///
+/// Could be written:
+///
+/// ```rust
+/// if let Some(value) = option {
+///     do_something_with(value)
+/// }
+/// ```
+declare_clippy_lint! {
+    pub UNNECESSARY_UNWRAP,
+    complexity,
+    "checks for calls of unwrap[_err]() that cannot fail"
+}
+
+pub struct Pass;
+
+/// Visitor that keeps track of which variables are unwrappable.
+struct UnwrappableVariablesVisitor<'a, 'tcx: 'a> {
+    unwrappables: Vec<UnwrapInfo<'tcx>>,
+    cx: &'a LateContext<'a, 'tcx>,
+}
+/// Contains information about whether a variable can be unwrapped.
+#[derive(Copy, Clone, Debug)]
+struct UnwrapInfo<'tcx> {
+    /// The variable that is checked
+    ident: &'tcx Path,
+    /// The check, like `x.is_ok()`
+    check: &'tcx Expr,
+    /// Whether `is_some()` or `is_ok()` was called (as opposed to `is_err()` or `is_none()`).
+    safe_to_unwrap: bool,
+}
+
+/// Collects the information about unwrappable variables from an if condition
+/// The `invert` argument tells us whether the condition is negated.
+fn collect_unwrap_info<'a, 'tcx: 'a>(
+    cx: &'a LateContext<'a, 'tcx>,
+    expr: &'tcx Expr,
+    invert: bool,
+) -> Vec<UnwrapInfo<'tcx>> {
+    if let Expr_::ExprBinary(op, left, right) = &expr.node {
+        match (invert, op.node) {
+            (false, BinOp_::BiAnd) | (false, BinOp_::BiBitAnd) | (true, BinOp_::BiOr) | (true, BinOp_::BiBitOr) => {
+                let mut unwrap_info = collect_unwrap_info(cx, left, invert);
+                unwrap_info.append(&mut collect_unwrap_info(cx, right, invert));
+                return unwrap_info;
+            },
+            _ => (),
+        }
+    } else if let Expr_::ExprUnary(UnNot, expr) = &expr.node {
+        return collect_unwrap_info(cx, expr, !invert);
+    } else {
+        if_chain! {
+            if let Expr_::ExprMethodCall(method_name, _, args) = &expr.node;
+            if let Expr_::ExprPath(QPath::Resolved(None, path)) = &args[0].node;
+            let ty = cx.tables.expr_ty(&args[0]);
+            if match_type(cx, ty, &paths::OPTION) || match_type(cx, ty, &paths::RESULT);
+            let name = method_name.name.as_str();
+            if ["is_some", "is_none", "is_ok", "is_err"].contains(&&*name);
+            then {
+                assert!(args.len() == 1);
+                let unwrappable = match name.as_ref() {
+                    "is_some" | "is_ok" => true,
+                    "is_err" | "is_none" => false,
+                    _ => unreachable!(),
+                };
+                let safe_to_unwrap = unwrappable != invert;
+                return vec![UnwrapInfo { ident: path, check: expr, safe_to_unwrap }];
+            }
+        }
+    }
+    Vec::new()
+}
+
+impl<'a, 'tcx: 'a> UnwrappableVariablesVisitor<'a, 'tcx> {
+    fn visit_branch(&mut self, cond: &'tcx Expr, branch: &'tcx Expr, else_branch: bool) {
+        let prev_len = self.unwrappables.len();
+        for unwrap_info in collect_unwrap_info(self.cx, cond, else_branch) {
+            if is_potentially_mutated(unwrap_info.ident, cond, self.cx)
+                || is_potentially_mutated(unwrap_info.ident, branch, self.cx)
+            {
+                // if the variable is mutated, we don't know whether it can be unwrapped:
+                continue;
+            }
+            self.unwrappables.push(unwrap_info);
+        }
+        walk_expr(self, branch);
+        self.unwrappables.truncate(prev_len);
+    }
+}
+
+impl<'a, 'tcx: 'a> Visitor<'tcx> for UnwrappableVariablesVisitor<'a, 'tcx> {
+    fn visit_expr(&mut self, expr: &'tcx Expr) {
+        if let Expr_::ExprIf(cond, then, els) = &expr.node {
+            walk_expr(self, cond);
+            self.visit_branch(cond, then, false);
+            if let Some(els) = els {
+                self.visit_branch(cond, els, true);
+            }
+        } else {
+            // find `unwrap[_err]()` calls:
+            if_chain! {
+                if let Expr_::ExprMethodCall(ref method_name, _, ref args) = expr.node;
+                if let Expr_::ExprPath(QPath::Resolved(None, ref path)) = args[0].node;
+                if ["unwrap", "unwrap_err"].contains(&&*method_name.name.as_str());
+                let call_to_unwrap = method_name.name == "unwrap";
+                if let Some(unwrappable) = self.unwrappables.iter()
+                    .find(|u| u.ident.def == path.def && call_to_unwrap == u.safe_to_unwrap);
+                then {
+                    self.cx.span_lint_note(
+                        UNNECESSARY_UNWRAP,
+                        expr.span,
+                        &format!("You checked before that `{}()` cannot fail. \
+                        Instead of checking and unwrapping, it's better to use `if let` or `match`.",
+                        method_name.name),
+                        unwrappable.check.span,
+                        "the check is happening here",
+                    );
+                }
+            }
+            walk_expr(self, expr);
+        }
+    }
+
+    fn nested_visit_map<'this>(&'this mut self) -> NestedVisitorMap<'this, 'tcx> {
+        NestedVisitorMap::OnlyBodies(&self.cx.tcx.hir)
+    }
+}
+
+impl<'a> LintPass for Pass {
+    fn get_lints(&self) -> LintArray {
+        lint_array!(UNNECESSARY_UNWRAP)
+    }
+}
+
+impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Pass {
+    fn check_fn(
+        &mut self,
+        cx: &LateContext<'a, 'tcx>,
+        kind: FnKind<'tcx>,
+        decl: &'tcx FnDecl,
+        body: &'tcx Body,
+        span: Span,
+        fn_id: NodeId,
+    ) {
+        if in_macro(span) {
+            return;
+        }
+
+        let mut v = UnwrappableVariablesVisitor {
+            cx,
+            unwrappables: Vec::new(),
+        };
+
+        walk_fn(&mut v, kind, decl, body.id(), span, fn_id);
+    }
+}

clippy_lints/src/utils/mod.rs

@@ -32,6 +32,7 @@ pub mod inspector;
 pub mod internal_lints;
 pub mod author;
 pub mod ptr;
+pub mod usage;
 pub use self::hir_utils::{SpanlessEq, SpanlessHash};
 
 pub type MethodArgs = HirVec<P<Expr>>;

clippy_lints/src/utils/usage.rs

@@ -0,0 +1,82 @@
+use rustc::lint::*;
+
+use rustc::hir::def::Def;
+use rustc::hir::*;
+use rustc::middle::expr_use_visitor::*;
+use rustc::middle::mem_categorization::cmt_;
+use rustc::middle::mem_categorization::Categorization;
+use rustc::ty;
+use std::collections::HashSet;
+use syntax::ast::NodeId;
+use syntax::codemap::Span;
+
+/// Returns a set of mutated local variable ids or None if mutations could not be determined.
+pub fn mutated_variables<'a, 'tcx: 'a>(expr: &'tcx Expr, cx: &'a LateContext<'a, 'tcx>) -> Option<HashSet<NodeId>> {
+    let mut delegate = MutVarsDelegate {
+        used_mutably: HashSet::new(),
+        skip: false,
+    };
+    let def_id = def_id::DefId::local(expr.hir_id.owner);
+    let region_scope_tree = &cx.tcx.region_scope_tree(def_id);
+    ExprUseVisitor::new(&mut delegate, cx.tcx, cx.param_env, region_scope_tree, cx.tables, None).walk_expr(expr);
+
+    if delegate.skip {
+        return None;
+    }
+    Some(delegate.used_mutably)
+}
+
+pub fn is_potentially_mutated<'a, 'tcx: 'a>(
+    variable: &'tcx Path,
+    expr: &'tcx Expr,
+    cx: &'a LateContext<'a, 'tcx>,
+) -> bool {
+    let id = match variable.def {
+        Def::Local(id) | Def::Upvar(id, ..) => id,
+        _ => return true,
+    };
+    mutated_variables(expr, cx).map_or(true, |mutated| mutated.contains(&id))
+}
+
+struct MutVarsDelegate {
+    used_mutably: HashSet<NodeId>,
+    skip: bool,
+}
+
+impl<'tcx> MutVarsDelegate {
+    fn update(&mut self, cat: &'tcx Categorization) {
+        match *cat {
+            Categorization::Local(id) => {
+                self.used_mutably.insert(id);
+            },
+            Categorization::Upvar(_) => {
+                //FIXME: This causes false negatives. We can't get the `NodeId` from
+                //`Categorization::Upvar(_)`. So we search for any `Upvar`s in the
+                //`while`-body, not just the ones in the condition.
+                self.skip = true
+            },
+            Categorization::Deref(ref cmt, _) | Categorization::Interior(ref cmt, _) => self.update(&cmt.cat),
+            _ => {},
+        }
+    }
+}
+
+impl<'tcx> Delegate<'tcx> for MutVarsDelegate {
+    fn consume(&mut self, _: NodeId, _: Span, _: &cmt_<'tcx>, _: ConsumeMode) {}
+
+    fn matched_pat(&mut self, _: &Pat, _: &cmt_<'tcx>, _: MatchMode) {}
+
+    fn consume_pat(&mut self, _: &Pat, _: &cmt_<'tcx>, _: ConsumeMode) {}
+
+    fn borrow(&mut self, _: NodeId, _: Span, cmt: &cmt_<'tcx>, _: ty::Region, bk: ty::BorrowKind, _: LoanCause) {
+        if let ty::BorrowKind::MutBorrow = bk {
+            self.update(&cmt.cat)
+        }
+    }
+
+    fn mutate(&mut self, _: NodeId, _: Span, cmt: &cmt_<'tcx>, _: MutateMode) {
+        self.update(&cmt.cat)
+    }
+
+    fn decl_without_init(&mut self, _: NodeId, _: Span) {}
+}

tests/ui/checked_unwrap.rs

@@ -0,0 +1,73 @@
+fn main() {
+    let x = Some(());
+    if x.is_some() {
+        x.unwrap();
+    }
+    if x.is_none() {
+        // nothing to do here
+    } else {
+        x.unwrap();
+    }
+    let mut x: Result<(), ()> = Ok(());
+    if x.is_ok() {
+        x.unwrap();
+    } else {
+        x.unwrap_err();
+    }
+    if x.is_err() {
+        x.unwrap_err();
+    } else {
+        x.unwrap();
+    }
+    if x.is_ok() {
+        x = Err(());
+        x.unwrap();
+    } else {
+        x = Ok(());
+        x.unwrap_err();
+    }
+}
+
+fn test_complex_conditions() {
+    let x: Result<(), ()> = Ok(());
+    let y: Result<(), ()> = Ok(());
+    if x.is_ok() && y.is_err() {
+        x.unwrap();
+        y.unwrap_err();
+    } else {
+        // not clear whether unwrappable:
+        x.unwrap_err();
+        y.unwrap();
+    }
+
+    if x.is_ok() || y.is_ok() {
+        // not clear whether unwrappable:
+        x.unwrap();
+        y.unwrap();
+    } else {
+        x.unwrap_err();
+        y.unwrap_err();
+    }
+    let z: Result<(), ()> = Ok(());
+    if x.is_ok() && !(y.is_ok() || z.is_err()) {
+        x.unwrap();
+        y.unwrap_err();
+        z.unwrap();
+    }
+    if x.is_ok() || !(y.is_ok() && z.is_err()) {
+        // not clear what's unwrappable
+    } else {
+        x.unwrap_err();
+        y.unwrap();
+        z.unwrap_err();
+    }
+}
+
+fn test_nested() {
+    fn nested() {
+        let x = Some(());
+        if x.is_some() {
+            x.unwrap();
+        }
+    }
+}

tests/ui/checked_unwrap.stderr

@@ -0,0 +1,207 @@
+error: You checked before that `unwrap()` cannot fail. Instead of checking and unwrapping, it's better to use `if let` or `match`.
+ --> $DIR/checked_unwrap.rs:4:9
+  |
+4 |         x.unwrap();
+  |         ^^^^^^^^^^
+  |
+  = note: `-D unnecessary-unwrap` implied by `-D warnings`
+note: the check is happening here
+ --> $DIR/checked_unwrap.rs:3:8
+  |
+3 |     if x.is_some() {
+  |        ^^^^^^^^^^^
+
+error: You checked before that `unwrap()` cannot fail. Instead of checking and unwrapping, it's better to use `if let` or `match`.
+ --> $DIR/checked_unwrap.rs:9:9
+  |
+9 |         x.unwrap();
+  |         ^^^^^^^^^^
+  |
+note: the check is happening here
+ --> $DIR/checked_unwrap.rs:6:8
+  |
+6 |     if x.is_none() {
+  |        ^^^^^^^^^^^
+
+error: You checked before that `unwrap()` cannot fail. Instead of checking and unwrapping, it's better to use `if let` or `match`.
+  --> $DIR/checked_unwrap.rs:13:9
+   |
+13 |         x.unwrap();
+   |         ^^^^^^^^^^
+   |
+note: the check is happening here
+  --> $DIR/checked_unwrap.rs:12:8
+   |
+12 |     if x.is_ok() {
+   |        ^^^^^^^^^
+
+error: You checked before that `unwrap_err()` cannot fail. Instead of checking and unwrapping, it's better to use `if let` or `match`.
+  --> $DIR/checked_unwrap.rs:15:9
+   |
+15 |         x.unwrap_err();
+   |         ^^^^^^^^^^^^^^
+   |
+note: the check is happening here
+  --> $DIR/checked_unwrap.rs:12:8
+   |
+12 |     if x.is_ok() {
+   |        ^^^^^^^^^
+
+error: You checked before that `unwrap_err()` cannot fail. Instead of checking and unwrapping, it's better to use `if let` or `match`.
+  --> $DIR/checked_unwrap.rs:18:9
+   |
+18 |         x.unwrap_err();
+   |         ^^^^^^^^^^^^^^
+   |
+note: the check is happening here
+  --> $DIR/checked_unwrap.rs:17:8
+   |
+17 |     if x.is_err() {
+   |        ^^^^^^^^^^
+
+error: You checked before that `unwrap()` cannot fail. Instead of checking and unwrapping, it's better to use `if let` or `match`.
+  --> $DIR/checked_unwrap.rs:20:9
+   |
+20 |         x.unwrap();
+   |         ^^^^^^^^^^
+   |
+note: the check is happening here
+  --> $DIR/checked_unwrap.rs:17:8
+   |
+17 |     if x.is_err() {
+   |        ^^^^^^^^^^
+
+error: You checked before that `unwrap()` cannot fail. Instead of checking and unwrapping, it's better to use `if let` or `match`.
+  --> $DIR/checked_unwrap.rs:35:9
+   |
+35 |         x.unwrap();
+   |         ^^^^^^^^^^
+   |
+note: the check is happening here
+  --> $DIR/checked_unwrap.rs:34:8
+   |
+34 |     if x.is_ok() && y.is_err() {
+   |        ^^^^^^^^^
+
+error: You checked before that `unwrap_err()` cannot fail. Instead of checking and unwrapping, it's better to use `if let` or `match`.
+  --> $DIR/checked_unwrap.rs:36:9
+   |
+36 |         y.unwrap_err();
+   |         ^^^^^^^^^^^^^^
+   |
+note: the check is happening here
+  --> $DIR/checked_unwrap.rs:34:21
+   |
+34 |     if x.is_ok() && y.is_err() {
+   |                     ^^^^^^^^^^
+
+error: You checked before that `unwrap_err()` cannot fail. Instead of checking and unwrapping, it's better to use `if let` or `match`.
+  --> $DIR/checked_unwrap.rs:48:9
+   |
+48 |         x.unwrap_err();
+   |         ^^^^^^^^^^^^^^
+   |
+note: the check is happening here
+  --> $DIR/checked_unwrap.rs:43:8
+   |
+43 |     if x.is_ok() || y.is_ok() {
+   |        ^^^^^^^^^
+
+error: You checked before that `unwrap_err()` cannot fail. Instead of checking and unwrapping, it's better to use `if let` or `match`.
+  --> $DIR/checked_unwrap.rs:49:9
+   |
+49 |         y.unwrap_err();
+   |         ^^^^^^^^^^^^^^
+   |
+note: the check is happening here
+  --> $DIR/checked_unwrap.rs:43:21
+   |
+43 |     if x.is_ok() || y.is_ok() {
+   |                     ^^^^^^^^^
+
+error: You checked before that `unwrap()` cannot fail. Instead of checking and unwrapping, it's better to use `if let` or `match`.
+  --> $DIR/checked_unwrap.rs:53:9
+   |
+53 |         x.unwrap();
+   |         ^^^^^^^^^^
+   |
+note: the check is happening here
+  --> $DIR/checked_unwrap.rs:52:8
+   |
+52 |     if x.is_ok() && !(y.is_ok() || z.is_err()) {
+   |        ^^^^^^^^^
+
+error: You checked before that `unwrap_err()` cannot fail. Instead of checking and unwrapping, it's better to use `if let` or `match`.
+  --> $DIR/checked_unwrap.rs:54:9
+   |
+54 |         y.unwrap_err();
+   |         ^^^^^^^^^^^^^^
+   |
+note: the check is happening here
+  --> $DIR/checked_unwrap.rs:52:23
+   |
+52 |     if x.is_ok() && !(y.is_ok() || z.is_err()) {
+   |                       ^^^^^^^^^
+
+error: You checked before that `unwrap()` cannot fail. Instead of checking and unwrapping, it's better to use `if let` or `match`.
+  --> $DIR/checked_unwrap.rs:55:9
+   |
+55 |         z.unwrap();
+   |         ^^^^^^^^^^
+   |
+note: the check is happening here
+  --> $DIR/checked_unwrap.rs:52:36
+   |
+52 |     if x.is_ok() && !(y.is_ok() || z.is_err()) {
+   |                                    ^^^^^^^^^^
+
+error: You checked before that `unwrap_err()` cannot fail. Instead of checking and unwrapping, it's better to use `if let` or `match`.
+  --> $DIR/checked_unwrap.rs:60:9
+   |
+60 |         x.unwrap_err();
+   |         ^^^^^^^^^^^^^^
+   |
+note: the check is happening here
+  --> $DIR/checked_unwrap.rs:57:8
+   |
+57 |     if x.is_ok() || !(y.is_ok() && z.is_err()) {
+   |        ^^^^^^^^^
+
+error: You checked before that `unwrap()` cannot fail. Instead of checking and unwrapping, it's better to use `if let` or `match`.
+  --> $DIR/checked_unwrap.rs:61:9
+   |
+61 |         y.unwrap();
+   |         ^^^^^^^^^^
+   |
+note: the check is happening here
+  --> $DIR/checked_unwrap.rs:57:23
+   |
+57 |     if x.is_ok() || !(y.is_ok() && z.is_err()) {
+   |                       ^^^^^^^^^
+
+error: You checked before that `unwrap_err()` cannot fail. Instead of checking and unwrapping, it's better to use `if let` or `match`.
+  --> $DIR/checked_unwrap.rs:62:9
+   |
+62 |         z.unwrap_err();
+   |         ^^^^^^^^^^^^^^
+   |
+note: the check is happening here
+  --> $DIR/checked_unwrap.rs:57:36
+   |
+57 |     if x.is_ok() || !(y.is_ok() && z.is_err()) {
+   |                                    ^^^^^^^^^^
+
+error: You checked before that `unwrap()` cannot fail. Instead of checking and unwrapping, it's better to use `if let` or `match`.
+  --> $DIR/checked_unwrap.rs:70:13
+   |
+70 |             x.unwrap();
+   |             ^^^^^^^^^^
+   |
+note: the check is happening here
+  --> $DIR/checked_unwrap.rs:69:12
+   |
+69 |         if x.is_some() {
+   |            ^^^^^^^^^^^
+
+error: aborting due to 17 previous errors
+