diff --git a/clippy_lints/src/unwrap.rs b/clippy_lints/src/unwrap.rs index b21cf41ee523..4fd421e4af94 100644 --- a/clippy_lints/src/unwrap.rs +++ b/clippy_lints/src/unwrap.rs @@ -1,6 +1,7 @@ use crate::utils::{higher::if_block, match_type, paths, span_lint_and_then, usage::is_potentially_mutated}; use if_chain::if_chain; use rustc::hir::map::Map; +use rustc::lint::in_external_macro; use rustc_hir::intravisit::*; use rustc_hir::*; use rustc_lint::{LateContext, LateLintPass}; @@ -138,6 +139,10 @@ impl<'a, 'tcx> Visitor<'tcx> for UnwrappableVariablesVisitor<'a, 'tcx> { type Map = Map<'tcx>; fn visit_expr(&mut self, expr: &'tcx Expr<'_>) { + // Shouldn't lint when `expr` is in macro. + if in_external_macro(self.cx.tcx.sess, expr.span) { + return; + } if let Some((cond, then, els)) = if_block(&expr) { walk_expr(self, cond); self.visit_branch(cond, then, false); diff --git a/tests/ui/checked_unwrap/simple_conditionals.rs b/tests/ui/checked_unwrap/simple_conditionals.rs index c080ae82697a..b0fc26ff76de 100644 --- a/tests/ui/checked_unwrap/simple_conditionals.rs +++ b/tests/ui/checked_unwrap/simple_conditionals.rs @@ -1,6 +1,14 @@ #![deny(clippy::panicking_unwrap, clippy::unnecessary_unwrap)] #![allow(clippy::if_same_then_else)] +macro_rules! m { + ($a:expr) => { + if $a.is_some() { + $a.unwrap(); // unnecessary + } + }; +} + fn main() { let x = Some(()); if x.is_some() { @@ -13,6 +21,7 @@ fn main() { } else { x.unwrap(); // unnecessary } + m!(x); let mut x: Result<(), ()> = Ok(()); if x.is_ok() { x.unwrap(); // unnecessary @@ -39,4 +48,6 @@ fn main() { // it will always panic but the lint is not smart enough to see this (it // only checks if conditions). } + + assert!(x.is_ok(), "{:?}", x.unwrap_err()); // ok, it's a common test pattern } diff --git a/tests/ui/checked_unwrap/simple_conditionals.stderr b/tests/ui/checked_unwrap/simple_conditionals.stderr index c25543304963..b226bd726403 100644 --- a/tests/ui/checked_unwrap/simple_conditionals.stderr +++ b/tests/ui/checked_unwrap/simple_conditionals.stderr @@ -1,5 +1,5 @@ error: You checked before that `unwrap()` cannot fail. Instead of checking and unwrapping, it's better to use `if let` or `match`. - --> $DIR/simple_conditionals.rs:7:9 + --> $DIR/simple_conditionals.rs:15:9 | LL | if x.is_some() { | ----------- the check is happening here @@ -13,7 +13,7 @@ LL | #![deny(clippy::panicking_unwrap, clippy::unnecessary_unwrap)] | ^^^^^^^^^^^^^^^^^^^^^^^^^^ error: This call to `unwrap()` will always panic. - --> $DIR/simple_conditionals.rs:9:9 + --> $DIR/simple_conditionals.rs:17:9 | LL | if x.is_some() { | ----------- because of this check @@ -28,7 +28,7 @@ LL | #![deny(clippy::panicking_unwrap, clippy::unnecessary_unwrap)] | ^^^^^^^^^^^^^^^^^^^^^^^^ error: This call to `unwrap()` will always panic. - --> $DIR/simple_conditionals.rs:12:9 + --> $DIR/simple_conditionals.rs:20:9 | LL | if x.is_none() { | ----------- because of this check @@ -36,7 +36,7 @@ LL | x.unwrap(); // will panic | ^^^^^^^^^^ error: You checked before that `unwrap()` cannot fail. Instead of checking and unwrapping, it's better to use `if let` or `match`. - --> $DIR/simple_conditionals.rs:14:9 + --> $DIR/simple_conditionals.rs:22:9 | LL | if x.is_none() { | ----------- the check is happening here @@ -45,7 +45,18 @@ LL | x.unwrap(); // unnecessary | ^^^^^^^^^^ error: You checked before that `unwrap()` cannot fail. Instead of checking and unwrapping, it's better to use `if let` or `match`. - --> $DIR/simple_conditionals.rs:18:9 + --> $DIR/simple_conditionals.rs:7:13 + | +LL | if $a.is_some() { + | ------------ the check is happening here +LL | $a.unwrap(); // unnecessary + | ^^^^^^^^^^^ +... +LL | m!(x); + | ------ in this macro invocation + +error: You checked before that `unwrap()` cannot fail. Instead of checking and unwrapping, it's better to use `if let` or `match`. + --> $DIR/simple_conditionals.rs:27:9 | LL | if x.is_ok() { | --------- the check is happening here @@ -53,7 +64,7 @@ LL | x.unwrap(); // unnecessary | ^^^^^^^^^^ error: This call to `unwrap_err()` will always panic. - --> $DIR/simple_conditionals.rs:19:9 + --> $DIR/simple_conditionals.rs:28:9 | LL | if x.is_ok() { | --------- because of this check @@ -62,7 +73,7 @@ LL | x.unwrap_err(); // will panic | ^^^^^^^^^^^^^^ error: This call to `unwrap()` will always panic. - --> $DIR/simple_conditionals.rs:21:9 + --> $DIR/simple_conditionals.rs:30:9 | LL | if x.is_ok() { | --------- because of this check @@ -71,7 +82,7 @@ LL | x.unwrap(); // will panic | ^^^^^^^^^^ error: You checked before that `unwrap_err()` cannot fail. Instead of checking and unwrapping, it's better to use `if let` or `match`. - --> $DIR/simple_conditionals.rs:22:9 + --> $DIR/simple_conditionals.rs:31:9 | LL | if x.is_ok() { | --------- the check is happening here @@ -80,7 +91,7 @@ LL | x.unwrap_err(); // unnecessary | ^^^^^^^^^^^^^^ error: This call to `unwrap()` will always panic. - --> $DIR/simple_conditionals.rs:25:9 + --> $DIR/simple_conditionals.rs:34:9 | LL | if x.is_err() { | ---------- because of this check @@ -88,7 +99,7 @@ LL | x.unwrap(); // will panic | ^^^^^^^^^^ error: You checked before that `unwrap_err()` cannot fail. Instead of checking and unwrapping, it's better to use `if let` or `match`. - --> $DIR/simple_conditionals.rs:26:9 + --> $DIR/simple_conditionals.rs:35:9 | LL | if x.is_err() { | ---------- the check is happening here @@ -97,7 +108,7 @@ LL | x.unwrap_err(); // unnecessary | ^^^^^^^^^^^^^^ error: You checked before that `unwrap()` cannot fail. Instead of checking and unwrapping, it's better to use `if let` or `match`. - --> $DIR/simple_conditionals.rs:28:9 + --> $DIR/simple_conditionals.rs:37:9 | LL | if x.is_err() { | ---------- the check is happening here @@ -106,7 +117,7 @@ LL | x.unwrap(); // unnecessary | ^^^^^^^^^^ error: This call to `unwrap_err()` will always panic. - --> $DIR/simple_conditionals.rs:29:9 + --> $DIR/simple_conditionals.rs:38:9 | LL | if x.is_err() { | ---------- because of this check @@ -114,5 +125,5 @@ LL | if x.is_err() { LL | x.unwrap_err(); // will panic | ^^^^^^^^^^^^^^ -error: aborting due to 12 previous errors +error: aborting due to 13 previous errors