Skip to content

Commit 55570c9

Browse files
authored
Fix unnecessary_unwrap false negative (#12295) (#15689)
Fix `unnecessary_unwrap` false negative when unwrapping a known value inside a macro call Fixes #12295 Produces no changes to `lintcheck` changelog: [`unnecessary_unwrap`]: Fix false negative when unwrapping a known value inside a macro call I made sue to run `cargo dev fmt`. I also ran the following checks (in order) on a fresh clone of mi fork: 1. `cargo build` 2. `cargo test` 3. `cargo lintcheck` Running `cargo test` after `lintcheck` causes a test to fail, but this also happens on a clean fork without my changes
2 parents f37b9c0 + 19e5286 commit 55570c9

File tree

3 files changed

+58
-1
lines changed

3 files changed

+58
-1
lines changed

clippy_lints/src/unwrap.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -292,6 +292,7 @@ impl<'tcx> Visitor<'tcx> for UnwrappableVariablesVisitor<'_, 'tcx> {
292292
fn visit_expr(&mut self, expr: &'tcx Expr<'_>) {
293293
// Shouldn't lint when `expr` is in macro.
294294
if expr.span.in_external_macro(self.cx.tcx.sess.source_map()) {
295+
walk_expr(self, expr);
295296
return;
296297
}
297298
// Skip checking inside closures since they are visited through `Unwrap::check_fn()` already.

tests/ui/checked_unwrap/simple_conditionals.rs

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -273,6 +273,28 @@ const ISSUE14763: fn(Option<String>) = |x| {
273273
}
274274
};
275275

276+
fn issue12295() {
277+
let option = Some(());
278+
279+
if option.is_some() {
280+
println!("{:?}", option.unwrap());
281+
//~^ unnecessary_unwrap
282+
} else {
283+
println!("{:?}", option.unwrap());
284+
//~^ panicking_unwrap
285+
}
286+
287+
let result = Ok::<(), ()>(());
288+
289+
if result.is_ok() {
290+
println!("{:?}", result.unwrap());
291+
//~^ unnecessary_unwrap
292+
} else {
293+
println!("{:?}", result.unwrap());
294+
//~^ panicking_unwrap
295+
}
296+
}
297+
276298
fn check_expect() {
277299
let x = Some(());
278300
if x.is_some() {

tests/ui/checked_unwrap/simple_conditionals.stderr

Lines changed: 35 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -322,6 +322,40 @@ LL | if x.is_some() {
322322
LL | _ = x.unwrap();
323323
| ^^^^^^^^^^
324324

325+
error: called `unwrap` on `option` after checking its variant with `is_some`
326+
--> tests/ui/checked_unwrap/simple_conditionals.rs:280:26
327+
|
328+
LL | if option.is_some() {
329+
| ------------------- help: try: `if let Some(<item>) = option`
330+
LL | println!("{:?}", option.unwrap());
331+
| ^^^^^^^^^^^^^^^
332+
333+
error: this call to `unwrap()` will always panic
334+
--> tests/ui/checked_unwrap/simple_conditionals.rs:283:26
335+
|
336+
LL | if option.is_some() {
337+
| ---------------- because of this check
338+
...
339+
LL | println!("{:?}", option.unwrap());
340+
| ^^^^^^^^^^^^^^^
341+
342+
error: called `unwrap` on `result` after checking its variant with `is_ok`
343+
--> tests/ui/checked_unwrap/simple_conditionals.rs:290:26
344+
|
345+
LL | if result.is_ok() {
346+
| ----------------- help: try: `if let Ok(<item>) = result`
347+
LL | println!("{:?}", result.unwrap());
348+
| ^^^^^^^^^^^^^^^
349+
350+
error: this call to `unwrap()` will always panic
351+
--> tests/ui/checked_unwrap/simple_conditionals.rs:293:26
352+
|
353+
LL | if result.is_ok() {
354+
| -------------- because of this check
355+
...
356+
LL | println!("{:?}", result.unwrap());
357+
| ^^^^^^^^^^^^^^^
358+
325359
error: creating a shared reference to mutable static
326360
--> tests/ui/checked_unwrap/simple_conditionals.rs:183:12
327361
|
@@ -332,5 +366,5 @@ LL | if X.is_some() {
332366
= 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
333367
= note: `#[deny(static_mut_refs)]` (part of `#[deny(rust_2024_compatibility)]`) on by default
334368

335-
error: aborting due to 36 previous errors
369+
error: aborting due to 40 previous errors
336370

0 commit comments

Comments
 (0)