From 733a9ea6b906441b755fbcd983c93a466b331070 Mon Sep 17 00:00:00 2001 From: y21 <30553356+y21@users.noreply.github.com> Date: Mon, 26 Jun 2023 20:13:06 +0200 Subject: [PATCH 1/2] [`option_if_let_else`]: suggest `.as_ref()` if `&Option<_>` --- clippy_lints/src/option_if_let_else.rs | 3 +++ tests/ui/option_if_let_else.fixed | 18 ++++++++++++++++++ tests/ui/option_if_let_else.rs | 24 ++++++++++++++++++++++++ tests/ui/option_if_let_else.stderr | 20 +++++++++++++++++++- 4 files changed, 64 insertions(+), 1 deletion(-) diff --git a/clippy_lints/src/option_if_let_else.rs b/clippy_lints/src/option_if_let_else.rs index aa6d40042688..abdccc47f547 100644 --- a/clippy_lints/src/option_if_let_else.rs +++ b/clippy_lints/src/option_if_let_else.rs @@ -140,6 +140,9 @@ fn try_get_option_occurrence<'tcx>( let (as_ref, as_mut) = match &expr.kind { ExprKind::AddrOf(_, Mutability::Not, _) => (true, false), ExprKind::AddrOf(_, Mutability::Mut, _) => (false, true), + _ if let Some(mutb) = cx.typeck_results().expr_ty(expr).ref_mutability() => { + (mutb == Mutability::Not, mutb == Mutability::Mut) + } _ => (bind_annotation == BindingAnnotation::REF, bind_annotation == BindingAnnotation::REF_MUT), }; diff --git a/tests/ui/option_if_let_else.fixed b/tests/ui/option_if_let_else.fixed index 2b8ce5477cc6..44ab8ebb50aa 100644 --- a/tests/ui/option_if_let_else.fixed +++ b/tests/ui/option_if_let_else.fixed @@ -198,6 +198,8 @@ fn main() { let _ = res.map_or(1, |a| a + 1); let _ = res.map_or(1, |a| a + 1); let _ = res.map_or(5, |a| a + 1); + issue10729::reproduce(&None); + issue10729::reproduce2(&mut None); } #[allow(dead_code)] @@ -208,3 +210,19 @@ fn issue9742() -> Option<&'static str> { _ => None, } } + +mod issue10729 { + #![allow(clippy::unit_arg)] + + pub fn reproduce(initial: &Option) { + // 👇 needs `.as_ref()` because initial is an `&Option<_>` + initial.as_ref().map_or({}, |value| do_something(value)) + } + + pub fn reproduce2(initial: &mut Option) { + initial.as_mut().map_or({}, |value| do_something2(value)) + } + + fn do_something(_value: &str) {} + fn do_something2(_value: &mut str) {} +} diff --git a/tests/ui/option_if_let_else.rs b/tests/ui/option_if_let_else.rs index cfbec8cb27da..9358c20ab95a 100644 --- a/tests/ui/option_if_let_else.rs +++ b/tests/ui/option_if_let_else.rs @@ -239,6 +239,8 @@ fn main() { Ok(a) => a + 1, }; let _ = if let Ok(a) = res { a + 1 } else { 5 }; + issue10729::reproduce(&None); + issue10729::reproduce2(&mut None); } #[allow(dead_code)] @@ -249,3 +251,25 @@ fn issue9742() -> Option<&'static str> { _ => None, } } + +mod issue10729 { + #![allow(clippy::unit_arg)] + + pub fn reproduce(initial: &Option) { + // 👇 needs `.as_ref()` because initial is an `&Option<_>` + match initial { + Some(value) => do_something(value), + None => {}, + } + } + + pub fn reproduce2(initial: &mut Option) { + match initial { + Some(value) => do_something2(value), + None => {}, + } + } + + fn do_something(_value: &str) {} + fn do_something2(_value: &mut str) {} +} diff --git a/tests/ui/option_if_let_else.stderr b/tests/ui/option_if_let_else.stderr index 91d52fc79b81..fde27f216906 100644 --- a/tests/ui/option_if_let_else.stderr +++ b/tests/ui/option_if_let_else.stderr @@ -271,5 +271,23 @@ error: use Option::map_or instead of an if let/else LL | let _ = if let Ok(a) = res { a + 1 } else { 5 }; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `res.map_or(5, |a| a + 1)` -error: aborting due to 21 previous errors +error: use Option::map_or instead of an if let/else + --> $DIR/option_if_let_else.rs:260:9 + | +LL | / match initial { +LL | | Some(value) => do_something(value), +LL | | None => {}, +LL | | } + | |_________^ help: try: `initial.as_ref().map_or({}, |value| do_something(value))` + +error: use Option::map_or instead of an if let/else + --> $DIR/option_if_let_else.rs:267:9 + | +LL | / match initial { +LL | | Some(value) => do_something2(value), +LL | | None => {}, +LL | | } + | |_________^ help: try: `initial.as_mut().map_or({}, |value| do_something2(value))` + +error: aborting due to 23 previous errors From cee4c4169c0f98bafe2e2c6cd6d3d2d401e5ce18 Mon Sep 17 00:00:00 2001 From: y21 <30553356+y21@users.noreply.github.com> Date: Wed, 28 Jun 2023 17:00:54 +0200 Subject: [PATCH 2/2] allow dead code in the test --- tests/ui/option_if_let_else.fixed | 4 +--- tests/ui/option_if_let_else.rs | 4 +--- tests/ui/option_if_let_else.stderr | 4 ++-- 3 files changed, 4 insertions(+), 8 deletions(-) diff --git a/tests/ui/option_if_let_else.fixed b/tests/ui/option_if_let_else.fixed index 44ab8ebb50aa..8e59e4375d2e 100644 --- a/tests/ui/option_if_let_else.fixed +++ b/tests/ui/option_if_let_else.fixed @@ -198,8 +198,6 @@ fn main() { let _ = res.map_or(1, |a| a + 1); let _ = res.map_or(1, |a| a + 1); let _ = res.map_or(5, |a| a + 1); - issue10729::reproduce(&None); - issue10729::reproduce2(&mut None); } #[allow(dead_code)] @@ -212,7 +210,7 @@ fn issue9742() -> Option<&'static str> { } mod issue10729 { - #![allow(clippy::unit_arg)] + #![allow(clippy::unit_arg, dead_code)] pub fn reproduce(initial: &Option) { // 👇 needs `.as_ref()` because initial is an `&Option<_>` diff --git a/tests/ui/option_if_let_else.rs b/tests/ui/option_if_let_else.rs index 9358c20ab95a..e72edf2a8e31 100644 --- a/tests/ui/option_if_let_else.rs +++ b/tests/ui/option_if_let_else.rs @@ -239,8 +239,6 @@ fn main() { Ok(a) => a + 1, }; let _ = if let Ok(a) = res { a + 1 } else { 5 }; - issue10729::reproduce(&None); - issue10729::reproduce2(&mut None); } #[allow(dead_code)] @@ -253,7 +251,7 @@ fn issue9742() -> Option<&'static str> { } mod issue10729 { - #![allow(clippy::unit_arg)] + #![allow(clippy::unit_arg, dead_code)] pub fn reproduce(initial: &Option) { // 👇 needs `.as_ref()` because initial is an `&Option<_>` diff --git a/tests/ui/option_if_let_else.stderr b/tests/ui/option_if_let_else.stderr index fde27f216906..aa2da2174003 100644 --- a/tests/ui/option_if_let_else.stderr +++ b/tests/ui/option_if_let_else.stderr @@ -272,7 +272,7 @@ LL | let _ = if let Ok(a) = res { a + 1 } else { 5 }; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `res.map_or(5, |a| a + 1)` error: use Option::map_or instead of an if let/else - --> $DIR/option_if_let_else.rs:260:9 + --> $DIR/option_if_let_else.rs:258:9 | LL | / match initial { LL | | Some(value) => do_something(value), @@ -281,7 +281,7 @@ LL | | } | |_________^ help: try: `initial.as_ref().map_or({}, |value| do_something(value))` error: use Option::map_or instead of an if let/else - --> $DIR/option_if_let_else.rs:267:9 + --> $DIR/option_if_let_else.rs:265:9 | LL | / match initial { LL | | Some(value) => do_something2(value),