From 81821acd59a051b5fb0b6e6dc9eeac894c92d758 Mon Sep 17 00:00:00 2001 From: Fabian Zaiser Date: Mon, 28 May 2018 00:02:38 +0200 Subject: [PATCH] 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 | 4 + clippy_lints/src/loops.rs | 83 +++---------- clippy_lints/src/unwrap.rs | 176 +++++++++++++++++++++++++++ clippy_lints/src/utils/mod.rs | 1 + clippy_lints/src/utils/usage.rs | 82 +++++++++++++ tests/ui/checked_unwrap.rs | 73 +++++++++++ tests/ui/checked_unwrap.stderr | 207 ++++++++++++++++++++++++++++++++ 7 files changed, 560 insertions(+), 66 deletions(-) create mode 100644 clippy_lints/src/unwrap.rs create mode 100644 clippy_lints/src/utils/usage.rs create mode 100644 tests/ui/checked_unwrap.rs create mode 100644 tests/ui/checked_unwrap.stderr diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index f4bf3c44711..c4dfc1f0168 100644 --- a/clippy_lints/src/lib.rs +++ b/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, ]); diff --git a/clippy_lints/src/loops.rs b/clippy_lints/src/loops.rs index 4d312b79818..20b10db0d66 100644 --- a/clippy_lints/src/loops.rs +++ b/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 { 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, + ids: HashSet, def_ids: HashMap, 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, - 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) {} -} diff --git a/clippy_lints/src/unwrap.rs b/clippy_lints/src/unwrap.rs new file mode 100644 index 00000000000..6d209acef39 --- /dev/null +++ b/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>, + 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> { + 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); + } +} diff --git a/clippy_lints/src/utils/mod.rs b/clippy_lints/src/utils/mod.rs index a49464b021a..6c3f0c25a3e 100644 --- a/clippy_lints/src/utils/mod.rs +++ b/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>; diff --git a/clippy_lints/src/utils/usage.rs b/clippy_lints/src/utils/usage.rs new file mode 100644 index 00000000000..6d75dfb486d --- /dev/null +++ b/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> { + 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, + 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) {} +} diff --git a/tests/ui/checked_unwrap.rs b/tests/ui/checked_unwrap.rs new file mode 100644 index 00000000000..75a5a2a2baa --- /dev/null +++ b/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(); + } + } +} diff --git a/tests/ui/checked_unwrap.stderr b/tests/ui/checked_unwrap.stderr new file mode 100644 index 00000000000..337950f2e66 --- /dev/null +++ b/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 +