fix: unnecessary_map_or don't add parens if the parent expr comes f… (#15345)

…rom a macro

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

fixes rust-lang/rust-clippy#14714

Author: y21

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

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");

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");

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