Skip to content

Commit

Permalink
Auto merge of #11035 - y21:issue10729, r=Jarcho
Browse files Browse the repository at this point in the history
[`option_if_let_else`]: suggest `.as_ref()` if scrutinee is of type `&Option<_>`

Fixes #10729

`Option::map_or` takes ownership, so if matching on an `&Option<_>`, we need to suggest `.as_ref()` before calling `map_or` to get the same effect and to not cause a borrowck error.

changelog: [`option_if_let_else`]: suggest `.as_ref()`/`.as_mut()` if scrutinee is of type `&Option<_>`/`&mut Option<_>`
  • Loading branch information
bors committed Jun 28, 2023
2 parents 750c7a1 + cee4c41 commit ea4c5c5
Show file tree
Hide file tree
Showing 4 changed files with 60 additions and 1 deletion.
3 changes: 3 additions & 0 deletions clippy_lints/src/option_if_let_else.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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),
};

Expand Down
16 changes: 16 additions & 0 deletions tests/ui/option_if_let_else.fixed
Original file line number Diff line number Diff line change
Expand Up @@ -208,3 +208,19 @@ fn issue9742() -> Option<&'static str> {
_ => None,
}
}

mod issue10729 {
#![allow(clippy::unit_arg, dead_code)]

pub fn reproduce(initial: &Option<String>) {
// 👇 needs `.as_ref()` because initial is an `&Option<_>`
initial.as_ref().map_or({}, |value| do_something(value))
}

pub fn reproduce2(initial: &mut Option<String>) {
initial.as_mut().map_or({}, |value| do_something2(value))
}

fn do_something(_value: &str) {}
fn do_something2(_value: &mut str) {}
}
22 changes: 22 additions & 0 deletions tests/ui/option_if_let_else.rs
Original file line number Diff line number Diff line change
Expand Up @@ -249,3 +249,25 @@ fn issue9742() -> Option<&'static str> {
_ => None,
}
}

mod issue10729 {
#![allow(clippy::unit_arg, dead_code)]

pub fn reproduce(initial: &Option<String>) {
// 👇 needs `.as_ref()` because initial is an `&Option<_>`
match initial {
Some(value) => do_something(value),
None => {},
}
}

pub fn reproduce2(initial: &mut Option<String>) {
match initial {
Some(value) => do_something2(value),
None => {},
}
}

fn do_something(_value: &str) {}
fn do_something2(_value: &mut str) {}
}
20 changes: 19 additions & 1 deletion tests/ui/option_if_let_else.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -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:258: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:265: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

0 comments on commit ea4c5c5

Please sign in to comment.