Skip to content

Commit

Permalink
option_if_let_else: don't expand macros in suggestion
Browse files Browse the repository at this point in the history
Fixes #7973

changelog: don't expand macros in suggestion of clippy::option_if_let_else
  • Loading branch information
matthiaskrgr committed Nov 13, 2021
1 parent f51fb34 commit 3fe11e7
Show file tree
Hide file tree
Showing 4 changed files with 48 additions and 8 deletions.
6 changes: 3 additions & 3 deletions clippy_lints/src/option_if_let_else.rs
Expand Up @@ -111,7 +111,7 @@ fn extract_body_from_expr<'a>(expr: &'a Expr<'a>) -> Option<&'a Expr<'a>> {
fn format_option_in_sugg(cx: &LateContext<'_>, cond_expr: &Expr<'_>, as_ref: bool, as_mut: bool) -> String {
format!(
"{}{}",
Sugg::hir(cx, cond_expr, "..").maybe_par(),
Sugg::hir_with_macro_callsite(cx, cond_expr, "..").maybe_par(),
if as_mut {
".as_mut()"
} else if as_ref {
Expand Down Expand Up @@ -184,8 +184,8 @@ fn detect_option_if_let_else<'tcx>(cx: &LateContext<'tcx>, expr: &Expr<'tcx>) ->
Some(OptionIfLetElseOccurence {
option: format_option_in_sugg(cx, cond_expr, as_ref, as_mut),
method_sugg: method_sugg.to_string(),
some_expr: format!("|{}{}| {}", capture_mut, capture_name, Sugg::hir(cx, some_body, "..")),
none_expr: format!("{}{}", if method_sugg == "map_or" { "" } else { "|| " }, Sugg::hir(cx, none_body, "..")),
some_expr: format!("|{}{}| {}", capture_mut, capture_name, Sugg::hir_with_macro_callsite(cx, some_body, "..")),
none_expr: format!("{}{}", if method_sugg == "map_or" { "" } else { "|| " }, Sugg::hir_with_macro_callsite(cx, none_body, "..")),
})
} else {
None
Expand Down
13 changes: 13 additions & 0 deletions tests/ui/option_if_let_else.fixed
Expand Up @@ -75,6 +75,17 @@ fn negative_tests(arg: Option<u32>) -> u32 {
7
}

// #7973
fn pattern_to_vec(pattern: &str) -> Vec<String> {
pattern
.trim_matches('/')
.split('/')
.flat_map(|s| {
s.find('.').map_or_else(|| vec![s.to_string()], |idx| vec![s[..idx].to_string(), s[idx..].to_string()])
})
.collect::<Vec<_>>()
}

fn main() {
let optional = Some(5);
let _ = optional.map_or(5, |x| x + 2);
Expand Down Expand Up @@ -146,4 +157,6 @@ fn main() {
// Don't lint. `await` can't be moved into a closure.
let _ = if let Some(x) = Some(0) { _f1(x).await } else { 0 };
}

let _ = pattern_to_vec("hello world");
}
17 changes: 17 additions & 0 deletions tests/ui/option_if_let_else.rs
Expand Up @@ -94,6 +94,21 @@ fn negative_tests(arg: Option<u32>) -> u32 {
7
}

// #7973
fn pattern_to_vec(pattern: &str) -> Vec<String> {
pattern
.trim_matches('/')
.split('/')
.flat_map(|s| {
if let Some(idx) = s.find('.') {
vec![s[..idx].to_string(), s[idx..].to_string()]
} else {
vec![s.to_string()]
}
})
.collect::<Vec<_>>()
}

fn main() {
let optional = Some(5);
let _ = if let Some(x) = optional { x + 2 } else { 5 };
Expand Down Expand Up @@ -171,4 +186,6 @@ fn main() {
// Don't lint. `await` can't be moved into a closure.
let _ = if let Some(x) = Some(0) { _f1(x).await } else { 0 };
}

let _ = pattern_to_vec("hello world");
}
20 changes: 15 additions & 5 deletions tests/ui/option_if_let_else.stderr
Expand Up @@ -142,14 +142,24 @@ LL + y
LL ~ }, |x| x * x * x * x);
|

error: use Option::map_or_else instead of an if let/else
--> $DIR/option_if_let_else.rs:103:13
|
LL | / if let Some(idx) = s.find('.') {
LL | | vec![s[..idx].to_string(), s[idx..].to_string()]
LL | | } else {
LL | | vec![s.to_string()]
LL | | }
| |_____________^ help: try: `s.find('.').map_or_else(|| vec![s.to_string()], |idx| vec![s[..idx].to_string(), s[idx..].to_string()])`

error: use Option::map_or instead of an if let/else
--> $DIR/option_if_let_else.rs:99:13
--> $DIR/option_if_let_else.rs:114:13
|
LL | let _ = if let Some(x) = optional { x + 2 } else { 5 };
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `optional.map_or(5, |x| x + 2)`

error: use Option::map_or instead of an if let/else
--> $DIR/option_if_let_else.rs:108:13
--> $DIR/option_if_let_else.rs:123:13
|
LL | let _ = if let Some(x) = Some(0) {
| _____________^
Expand All @@ -171,13 +181,13 @@ LL ~ });
|

error: use Option::map_or_else instead of an if let/else
--> $DIR/option_if_let_else.rs:136:13
--> $DIR/option_if_let_else.rs:151:13
|
LL | let _ = if let Some(x) = Some(0) { s.len() + x } else { s.len() };
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `Some(0).map_or_else(|| s.len(), |x| s.len() + x)`

error: use Option::map_or instead of an if let/else
--> $DIR/option_if_let_else.rs:140:13
--> $DIR/option_if_let_else.rs:155:13
|
LL | let _ = if let Some(x) = Some(0) {
| _____________^
Expand All @@ -196,5 +206,5 @@ LL + s.len() + x
LL ~ });
|

error: aborting due to 14 previous errors
error: aborting due to 15 previous errors

0 comments on commit 3fe11e7

Please sign in to comment.