manual_unwrap_or: fix invalid code suggestion, due to macro expansion

rust-lang · Apr 26, 2021 · 53eeb29 · 53eeb29
1 parent a362a4d
commit 53eeb29
Show file tree

Hide file tree

Showing 4 changed files with 48 additions and 2 deletions.
diff --git a/clippy_lints/src/manual_unwrap_or.rs b/clippy_lints/src/manual_unwrap_or.rs
@@ -112,14 +112,23 @@ fn lint_manual_unwrap_or<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) {
         then {
             let reindented_or_body =
                 reindent_multiline(or_body_snippet.into(), true, Some(indent));
+
+            let suggestion = if scrutinee.span.from_expansion() {
+                    // we don't want parenthesis around macro, e.g. `(some_macro!()).unwrap_or(0)`
+                    sugg::Sugg::hir_with_macro_callsite(cx, scrutinee, "..")
+                }
+                else {
+                    sugg::Sugg::hir(cx, scrutinee, "..").maybe_par()
+                };
+
             span_lint_and_sugg(
                 cx,
                 MANUAL_UNWRAP_OR, expr.span,
                 &format!("this pattern reimplements `{}`", case.unwrap_fn_path()),
                 "replace with",
                 format!(
                     "{}.unwrap_or({})",
-                    sugg::Sugg::hir(cx, scrutinee, "..").maybe_par(),
+                    suggestion,
                     reindented_or_body,
                 ),
                 Applicability::MachineApplicable,

diff --git a/tests/ui/manual_unwrap_or.fixed b/tests/ui/manual_unwrap_or.fixed
@@ -151,4 +151,16 @@ const fn const_fn_result_unwrap_or() {
     };
 }
 
+mod issue6965 {
+    macro_rules! some_macro {
+        () => {
+            if 1 > 2 { Some(1) } else { None }
+        };
+    }
+
+    fn test() {
+        let _ = some_macro!().unwrap_or(0);
+    }
+}
+
 fn main() {}
diff --git a/tests/ui/manual_unwrap_or.rs b/tests/ui/manual_unwrap_or.rs
@@ -190,4 +190,19 @@ const fn const_fn_result_unwrap_or() {
     };
 }
 
+mod issue6965 {
+    macro_rules! some_macro {
+        () => {
+            if 1 > 2 { Some(1) } else { None }
+        };
+    }
+
+    fn test() {
+        let _ = match some_macro!() {
+            Some(val) => val,
+            None => 0,
+        };
+    }
+}
+
 fn main() {}
diff --git a/tests/ui/manual_unwrap_or.stderr b/tests/ui/manual_unwrap_or.stderr
@@ -141,5 +141,15 @@ LL | |         Err(_) => "Alice",
 LL | |     };
    | |_____^ help: replace with: `Ok::<&str, &str>("Bob").unwrap_or("Alice")`
 
-error: aborting due to 13 previous errors
+error: this pattern reimplements `Option::unwrap_or`
+  --> $DIR/manual_unwrap_or.rs:201:17
+   |
+LL |           let _ = match some_macro!() {
+   |  _________________^
+LL | |             Some(val) => val,
+LL | |             None => 0,
+LL | |         };
+   | |_________^ help: replace with: `some_macro!().unwrap_or(0)`
+
+error: aborting due to 14 previous errors