option_if_let_else: don't expand macros in suggestion

Fixes #7973 changelog: don't expand macros in suggestion of clippy::option_if_let_else
rust-lang · Nov 13, 2021 · 3fe11e7 · 3fe11e7
1 parent f51fb34
commit 3fe11e7
Show file tree

Hide file tree

Showing 4 changed files with 48 additions and 8 deletions.
diff --git a/clippy_lints/src/option_if_let_else.rs b/clippy_lints/src/option_if_let_else.rs
@@ -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 {
@@ -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

diff --git a/tests/ui/option_if_let_else.fixed b/tests/ui/option_if_let_else.fixed
@@ -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);
@@ -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");
 }
diff --git a/tests/ui/option_if_let_else.rs b/tests/ui/option_if_let_else.rs
@@ -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 };
@@ -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");
 }
diff --git a/tests/ui/option_if_let_else.stderr b/tests/ui/option_if_let_else.stderr
@@ -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) {
    |  _____________^
@@ -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) {
    |  _____________^
@@ -196,5 +206,5 @@ LL +         s.len() + x
 LL ~     });
    |
 
-error: aborting due to 14 previous errors
+error: aborting due to 15 previous errors