The problem is that `check_fn` is triggered by both function and closure, and `visit_expr` can visit expressions in another closures within a function or closure. So just skip walking in a inner closure. changelog: Fix [`unnecessary_unwrap`] emitted twice in closure which inside in a function or another closure. Fixes: rust-lang/rust-clippy#14763
This commit is contained in:
commit
5262ab2416
3 changed files with 89 additions and 1 deletions
|
|
@ -292,6 +292,10 @@ impl<'tcx> Visitor<'tcx> for UnwrappableVariablesVisitor<'_, 'tcx> {
|
|||
if expr.span.in_external_macro(self.cx.tcx.sess.source_map()) {
|
||||
return;
|
||||
}
|
||||
// Skip checking inside closures since they are visited through `Unwrap::check_fn()` already.
|
||||
if matches!(expr.kind, ExprKind::Closure(_)) {
|
||||
return;
|
||||
}
|
||||
if let Some(higher::If { cond, then, r#else }) = higher::If::hir(expr) {
|
||||
walk_expr(self, cond);
|
||||
self.visit_branch(expr, cond, then, false);
|
||||
|
|
|
|||
|
|
@ -240,6 +240,39 @@ fn issue14725() {
|
|||
}
|
||||
}
|
||||
|
||||
fn issue14763(x: Option<String>, r: Result<(), ()>) {
|
||||
_ = || {
|
||||
if x.is_some() {
|
||||
_ = x.unwrap();
|
||||
//~^ unnecessary_unwrap
|
||||
} else {
|
||||
_ = x.unwrap();
|
||||
//~^ panicking_unwrap
|
||||
}
|
||||
};
|
||||
_ = || {
|
||||
if r.is_ok() {
|
||||
_ = r.as_ref().unwrap();
|
||||
//~^ unnecessary_unwrap
|
||||
} else {
|
||||
_ = r.as_ref().unwrap();
|
||||
//~^ panicking_unwrap
|
||||
}
|
||||
};
|
||||
}
|
||||
|
||||
const ISSUE14763: fn(Option<String>) = |x| {
|
||||
_ = || {
|
||||
if x.is_some() {
|
||||
_ = x.unwrap();
|
||||
//~^ unnecessary_unwrap
|
||||
} else {
|
||||
_ = x.unwrap();
|
||||
//~^ panicking_unwrap
|
||||
}
|
||||
}
|
||||
};
|
||||
|
||||
fn check_expect() {
|
||||
let x = Some(());
|
||||
if x.is_some() {
|
||||
|
|
|
|||
|
|
@ -271,6 +271,57 @@ LL | if result.is_ok() {
|
|||
LL | result.as_ref().unwrap();
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^^^
|
||||
|
||||
error: called `unwrap` on `x` after checking its variant with `is_some`
|
||||
--> tests/ui/checked_unwrap/simple_conditionals.rs:246:17
|
||||
|
|
||||
LL | if x.is_some() {
|
||||
| -------------- help: try: `if let Some(<item>) = x`
|
||||
LL | _ = x.unwrap();
|
||||
| ^^^^^^^^^^
|
||||
|
||||
error: this call to `unwrap()` will always panic
|
||||
--> tests/ui/checked_unwrap/simple_conditionals.rs:249:17
|
||||
|
|
||||
LL | if x.is_some() {
|
||||
| ----------- because of this check
|
||||
...
|
||||
LL | _ = x.unwrap();
|
||||
| ^^^^^^^^^^
|
||||
|
||||
error: called `unwrap` on `r` after checking its variant with `is_ok`
|
||||
--> tests/ui/checked_unwrap/simple_conditionals.rs:255:17
|
||||
|
|
||||
LL | if r.is_ok() {
|
||||
| ------------ help: try: `if let Ok(<item>) = &r`
|
||||
LL | _ = r.as_ref().unwrap();
|
||||
| ^^^^^^^^^^^^^^^^^^^
|
||||
|
||||
error: this call to `unwrap()` will always panic
|
||||
--> tests/ui/checked_unwrap/simple_conditionals.rs:258:17
|
||||
|
|
||||
LL | if r.is_ok() {
|
||||
| --------- because of this check
|
||||
...
|
||||
LL | _ = r.as_ref().unwrap();
|
||||
| ^^^^^^^^^^^^^^^^^^^
|
||||
|
||||
error: called `unwrap` on `x` after checking its variant with `is_some`
|
||||
--> tests/ui/checked_unwrap/simple_conditionals.rs:267:17
|
||||
|
|
||||
LL | if x.is_some() {
|
||||
| -------------- help: try: `if let Some(<item>) = x`
|
||||
LL | _ = x.unwrap();
|
||||
| ^^^^^^^^^^
|
||||
|
||||
error: this call to `unwrap()` will always panic
|
||||
--> tests/ui/checked_unwrap/simple_conditionals.rs:270:17
|
||||
|
|
||||
LL | if x.is_some() {
|
||||
| ----------- because of this check
|
||||
...
|
||||
LL | _ = x.unwrap();
|
||||
| ^^^^^^^^^^
|
||||
|
||||
error: creating a shared reference to mutable static
|
||||
--> tests/ui/checked_unwrap/simple_conditionals.rs:183:12
|
||||
|
|
||||
|
|
@ -281,5 +332,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)]` on by default
|
||||
|
||||
error: aborting due to 30 previous errors
|
||||
error: aborting due to 36 previous errors
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue