fix: `unnecessary_map_or` don't add parens if the parent expr comes from a macro

4 files changed, 77 insertions, 7 deletions

diff --git a/clippy_lints/src/methods/unnecessary_map_or.rs b/clippy_lints/src/methods/unnecessary_map_or.rs
index 4a9007c607c..1f5e3de6e7a 100644
--- a/clippy_lints/src/methods/unnecessary_map_or.rs
+++ b/clippy_lints/src/methods/unnecessary_map_or.rs
@@ -109,10 +109,16 @@ pub(super) fn check<'a>(
         );
 
         let sugg = if let Some(parent_expr) = get_parent_expr(cx, expr) {
-            match parent_expr.kind {
-                ExprKind::Binary(..) | ExprKind::Unary(..) | ExprKind::Cast(..) => binop.maybe_paren(),
-                ExprKind::MethodCall(_, receiver, _, _) if receiver.hir_id == expr.hir_id => binop.maybe_paren(),
-                _ => binop,
+            if parent_expr.span.eq_ctxt(expr.span) {
+                match parent_expr.kind {
+                    ExprKind::Binary(..) | ExprKind::Unary(..) | ExprKind::Cast(..) => binop.maybe_paren(),
+                    ExprKind::MethodCall(_, receiver, _, _) if receiver.hir_id == expr.hir_id => binop.maybe_paren(),
+                    _ => binop,
+                }
+            } else {
+                // if our parent expr is created by a macro, then it should be the one taking care of
+                // parenthesising us if necessary
+                binop
             }
         } else {
             binop
diff --git a/tests/ui/unnecessary_map_or.fixed b/tests/ui/unnecessary_map_or.fixed
index 3109c4af8e2..10552431d65 100644
--- a/tests/ui/unnecessary_map_or.fixed
+++ b/tests/ui/unnecessary_map_or.fixed
@@ -131,6 +131,26 @@ fn issue14201(a: Option<String>, b: Option<String>, s: &String) -> bool {
     x && y
 }
 
+fn issue14714() {
+    assert!(Some("test") == Some("test"));
+    //~^ unnecessary_map_or
+
+    // even though we're in a macro context, we still need to parenthesise because of the `then`
+    assert!((Some("test") == Some("test")).then(|| 1).is_some());
+    //~^ unnecessary_map_or
+
+    // method lints don't fire on macros
+    macro_rules! m {
+        ($x:expr) => {
+            // should become !($x == Some(1))
+            let _ = !$x.map_or(false, |v| v == 1);
+            // should become $x == Some(1)
+            let _ = $x.map_or(false, |v| v == 1);
+        };
+    }
+    m!(Some(5));
+}
+
 fn issue15180() {
     let s = std::sync::Mutex::new(Some("foo"));
     _ = s.lock().unwrap().is_some_and(|s| s == "foo");
diff --git a/tests/ui/unnecessary_map_or.rs b/tests/ui/unnecessary_map_or.rs
index 52a55f9fc9e..4b406ec2998 100644
--- a/tests/ui/unnecessary_map_or.rs
+++ b/tests/ui/unnecessary_map_or.rs
@@ -135,6 +135,26 @@ fn issue14201(a: Option<String>, b: Option<String>, s: &String) -> bool {
     x && y
 }
 
+fn issue14714() {
+    assert!(Some("test").map_or(false, |x| x == "test"));
+    //~^ unnecessary_map_or
+
+    // even though we're in a macro context, we still need to parenthesise because of the `then`
+    assert!(Some("test").map_or(false, |x| x == "test").then(|| 1).is_some());
+    //~^ unnecessary_map_or
+
+    // method lints don't fire on macros
+    macro_rules! m {
+        ($x:expr) => {
+            // should become !($x == Some(1))
+            let _ = !$x.map_or(false, |v| v == 1);
+            // should become $x == Some(1)
+            let _ = $x.map_or(false, |v| v == 1);
+        };
+    }
+    m!(Some(5));
+}
+
 fn issue15180() {
     let s = std::sync::Mutex::new(Some("foo"));
     _ = s.lock().unwrap().map_or(false, |s| s == "foo");
diff --git a/tests/ui/unnecessary_map_or.stderr b/tests/ui/unnecessary_map_or.stderr
index 99e17e8b34b..b8a22346c37 100644
--- a/tests/ui/unnecessary_map_or.stderr
+++ b/tests/ui/unnecessary_map_or.stderr
@@ -327,7 +327,31 @@ LL +     let y = b.is_none_or(|b| b == *s);
    |
 
 error: this `map_or` can be simplified
-  --> tests/ui/unnecessary_map_or.rs:140:9
+  --> tests/ui/unnecessary_map_or.rs:139:13
+   |
+LL |     assert!(Some("test").map_or(false, |x| x == "test"));
+   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+help: use a standard comparison instead
+   |
+LL -     assert!(Some("test").map_or(false, |x| x == "test"));
+LL +     assert!(Some("test") == Some("test"));
+   |
+
+error: this `map_or` can be simplified
+  --> tests/ui/unnecessary_map_or.rs:143:13
+   |
+LL |     assert!(Some("test").map_or(false, |x| x == "test").then(|| 1).is_some());
+   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+help: use a standard comparison instead
+   |
+LL -     assert!(Some("test").map_or(false, |x| x == "test").then(|| 1).is_some());
+LL +     assert!((Some("test") == Some("test")).then(|| 1).is_some());
+   |
+
+error: this `map_or` can be simplified
+  --> tests/ui/unnecessary_map_or.rs:160:9
    |
 LL |     _ = s.lock().unwrap().map_or(false, |s| s == "foo");
    |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
@@ -339,7 +363,7 @@ LL +     _ = s.lock().unwrap().is_some_and(|s| s == "foo");
    |
 
 error: this `map_or` can be simplified
-  --> tests/ui/unnecessary_map_or.rs:144:9
+  --> tests/ui/unnecessary_map_or.rs:164:9
    |
 LL |     _ = s.map_or(false, |s| s == "foo");
    |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
@@ -350,5 +374,5 @@ LL -     _ = s.map_or(false, |s| s == "foo");
 LL +     _ = s.is_some_and(|s| s == "foo");
    |
 
-error: aborting due to 28 previous errors
+error: aborting due to 30 previous errors