From 19e5286c806978e094328559d172f9d7e4e8885f Mon Sep 17 00:00:00 2001 From: galileocap Date: Tue, 16 Sep 2025 19:20:39 -0300 Subject: [PATCH] Fix `unnecessary_unwrap` false negative when unwrapping a known value inside a macro call Fixes https://github.com/rust-lang/rust-clippy/issues/12295 changelog: [`unnecessary_unwrap`]: Fix false negative when unwrapping a known value inside a macro call --- clippy_lints/src/unwrap.rs | 1 + .../ui/checked_unwrap/simple_conditionals.rs | 22 ++++++++++++ .../checked_unwrap/simple_conditionals.stderr | 36 ++++++++++++++++++- 3 files changed, 58 insertions(+), 1 deletion(-) diff --git a/clippy_lints/src/unwrap.rs b/clippy_lints/src/unwrap.rs index 490da4f1e037..34dfe5b6546f 100644 --- a/clippy_lints/src/unwrap.rs +++ b/clippy_lints/src/unwrap.rs @@ -292,6 +292,7 @@ impl<'tcx> Visitor<'tcx> for UnwrappableVariablesVisitor<'_, 'tcx> { fn visit_expr(&mut self, expr: &'tcx Expr<'_>) { // Shouldn't lint when `expr` is in macro. if expr.span.in_external_macro(self.cx.tcx.sess.source_map()) { + walk_expr(self, expr); return; } // Skip checking inside closures since they are visited through `Unwrap::check_fn()` already. diff --git a/tests/ui/checked_unwrap/simple_conditionals.rs b/tests/ui/checked_unwrap/simple_conditionals.rs index 785b2473c053..bba264080b40 100644 --- a/tests/ui/checked_unwrap/simple_conditionals.rs +++ b/tests/ui/checked_unwrap/simple_conditionals.rs @@ -273,6 +273,28 @@ const ISSUE14763: fn(Option) = |x| { } }; +fn issue12295() { + let option = Some(()); + + if option.is_some() { + println!("{:?}", option.unwrap()); + //~^ unnecessary_unwrap + } else { + println!("{:?}", option.unwrap()); + //~^ panicking_unwrap + } + + let result = Ok::<(), ()>(()); + + if result.is_ok() { + println!("{:?}", result.unwrap()); + //~^ unnecessary_unwrap + } else { + println!("{:?}", result.unwrap()); + //~^ panicking_unwrap + } +} + fn check_expect() { let x = Some(()); if x.is_some() { diff --git a/tests/ui/checked_unwrap/simple_conditionals.stderr b/tests/ui/checked_unwrap/simple_conditionals.stderr index 939b509d85c9..2007a8595413 100644 --- a/tests/ui/checked_unwrap/simple_conditionals.stderr +++ b/tests/ui/checked_unwrap/simple_conditionals.stderr @@ -322,6 +322,40 @@ LL | if x.is_some() { LL | _ = x.unwrap(); | ^^^^^^^^^^ +error: called `unwrap` on `option` after checking its variant with `is_some` + --> tests/ui/checked_unwrap/simple_conditionals.rs:280:26 + | +LL | if option.is_some() { + | ------------------- help: try: `if let Some() = option` +LL | println!("{:?}", option.unwrap()); + | ^^^^^^^^^^^^^^^ + +error: this call to `unwrap()` will always panic + --> tests/ui/checked_unwrap/simple_conditionals.rs:283:26 + | +LL | if option.is_some() { + | ---------------- because of this check +... +LL | println!("{:?}", option.unwrap()); + | ^^^^^^^^^^^^^^^ + +error: called `unwrap` on `result` after checking its variant with `is_ok` + --> tests/ui/checked_unwrap/simple_conditionals.rs:290:26 + | +LL | if result.is_ok() { + | ----------------- help: try: `if let Ok() = result` +LL | println!("{:?}", result.unwrap()); + | ^^^^^^^^^^^^^^^ + +error: this call to `unwrap()` will always panic + --> tests/ui/checked_unwrap/simple_conditionals.rs:293:26 + | +LL | if result.is_ok() { + | -------------- because of this check +... +LL | println!("{:?}", result.unwrap()); + | ^^^^^^^^^^^^^^^ + error: creating a shared reference to mutable static --> tests/ui/checked_unwrap/simple_conditionals.rs:183:12 | @@ -332,5 +366,5 @@ LL | if X.is_some() { = note: shared references to mutable statics are dangerous; it's undefined behavior if the static is mutated or if a mutable reference is created for it while the shared reference lives = note: `#[deny(static_mut_refs)]` (part of `#[deny(rust_2024_compatibility)]`) on by default -error: aborting due to 36 previous errors +error: aborting due to 40 previous errors