add parens around suggestion if additional methods

Author: Jacherr

clippy_lints/src/methods/unnecessary_map_or.rs

@@ -1,10 +1,11 @@
 use clippy_config::msrvs::{self, Msrv};
 use clippy_utils::diagnostics::span_lint_and_sugg;
 use clippy_utils::eager_or_lazy::switch_to_eager_eval;
-use clippy_utils::path_to_local_id;
 use clippy_utils::source::snippet;
 use clippy_utils::ty::is_type_diagnostic_item;
 use clippy_utils::visitors::is_local_used;
+use clippy_utils::{get_parent_expr, path_to_local_id};
+use rustc_ast::util::parser::AssocOp;
 use rustc_ast::LitKind::Bool;
 use rustc_errors::Applicability;
 use rustc_hir::{BinOpKind, Expr, ExprKind, PatKind};
@@ -52,13 +53,22 @@ pub(super) fn check(
             };
             let comparator = if BinOpKind::Eq == op.node { "==" } else { "!=" };
 
+            // we may need to add parens around the suggestion
+            // in case the parent expression has additional method calls,
+            // since for example `Some(5).map_or(false, |x| x == 5).then(|| 1)`
+            // being converted to `Some(5) == Some(5).then(|| 1)` isnt
+            // the same thing
+            let should_add_parens = get_parent_expr(cx, expr)
+                .is_some_and(|expr| expr.precedence().order() > AssocOp::Equal.precedence() as i8);
             (
                 format!(
-                    "{} {} {}({})",
+                    "{}{} {} {}({}){}",
+                    if should_add_parens { "(" } else { "" },
                     snippet(cx, recv.span, ".."),
                     comparator,
                     wrap,
-                    snippet(cx, non_binding_location.span.source_callsite(), "..")
+                    snippet(cx, non_binding_location.span.source_callsite(), ".."),
+                    if should_add_parens { ")" } else { "" }
                 ),
                 "standard comparison",
             )

tests/ui/unnecessary_map_or.fixed

@@ -1,6 +1,7 @@
 #![warn(clippy::unnecessary_map_or)]
 #![allow(clippy::no_effect)]
 #![allow(clippy::eq_op)]
+#[allow(clippy::unnecessary_lazy_evaluations)]
 
 #[clippy::msrv = "1.70.0"]
 fn main() {
@@ -18,6 +19,7 @@ fn main() {
     let _ = Some(5).is_some_and(|n| n == if 2 > 1 { n } else { 0 });
     let _ = Ok::<Vec<i32>, i32>(vec![5]).is_ok_and(|n| n == [5]);
     let _ = Ok::<i32, i32>(5) == Ok(5);
+    let _ = (Some(5) == Some(5)).then(|| 1);
 
     // shouldnt trigger
     let _ = Some(5).map_or(true, |n| n == 5);

tests/ui/unnecessary_map_or.rs

@@ -1,6 +1,7 @@
 #![warn(clippy::unnecessary_map_or)]
 #![allow(clippy::no_effect)]
 #![allow(clippy::eq_op)]
+#[allow(clippy::unnecessary_lazy_evaluations)]
 
 #[clippy::msrv = "1.70.0"]
 fn main() {
@@ -21,6 +22,7 @@ fn main() {
     let _ = Some(5).map_or(false, |n| n == if 2 > 1 { n } else { 0 });
     let _ = Ok::<Vec<i32>, i32>(vec![5]).map_or(false, |n| n == [5]);
     let _ = Ok::<i32, i32>(5).map_or(false, |n| n == 5);
+    let _ = Some(5).map_or(false, |n| n == 5).then(|| 1);
 
     // shouldnt trigger
     let _ = Some(5).map_or(true, |n| n == 5);

tests/ui/unnecessary_map_or.stderr

@@ -1,5 +1,5 @@
 error: `map_or` is redundant, use standard comparison instead
-  --> $DIR/unnecessary_map_or.rs:8:13
+  --> $DIR/unnecessary_map_or.rs:9:13
    |
 LL |     let _ = Some(5).map_or(false, |n| n == 5);
    |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `Some(5) == Some(5)`
@@ -8,13 +8,13 @@ LL |     let _ = Some(5).map_or(false, |n| n == 5);
    = help: to override `-D warnings` add `#[allow(clippy::unnecessary_map_or)]`
 
 error: `map_or` is redundant, use standard comparison instead
-  --> $DIR/unnecessary_map_or.rs:9:13
+  --> $DIR/unnecessary_map_or.rs:10:13
    |
 LL |     let _ = Some(5).map_or(true, |n| n != 5);
    |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `Some(5) != Some(5)`
 
 error: `map_or` is redundant, use standard comparison instead
-  --> $DIR/unnecessary_map_or.rs:10:13
+  --> $DIR/unnecessary_map_or.rs:11:13
    |
 LL |       let _ = Some(5).map_or(false, |n| {
    |  _____________^
@@ -24,7 +24,7 @@ LL | |     });
    | |______^ help: try: `Some(5) == Some(5)`
 
 error: `map_or` is redundant, use is_some_and instead
-  --> $DIR/unnecessary_map_or.rs:14:13
+  --> $DIR/unnecessary_map_or.rs:15:13
    |
 LL |       let _ = Some(5).map_or(false, |n| {
    |  _____________^
@@ -42,40 +42,46 @@ LL ~     });
    |
 
 error: `map_or` is redundant, use is_some_and instead
-  --> $DIR/unnecessary_map_or.rs:18:13
+  --> $DIR/unnecessary_map_or.rs:19:13
    |
 LL |     let _ = Some(vec![5]).map_or(false, |n| n == [5]);
    |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `Some(vec![5]).is_some_and(|n| n == [5])`
 
 error: `map_or` is redundant, use is_some_and instead
-  --> $DIR/unnecessary_map_or.rs:19:13
+  --> $DIR/unnecessary_map_or.rs:20:13
    |
 LL |     let _ = Some(vec![1]).map_or(false, |n| vec![2] == n);
    |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `Some(vec![1]).is_some_and(|n| vec![2] == n)`
 
 error: `map_or` is redundant, use is_some_and instead
-  --> $DIR/unnecessary_map_or.rs:20:13
+  --> $DIR/unnecessary_map_or.rs:21:13
    |
 LL |     let _ = Some(5).map_or(false, |n| n == n);
    |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `Some(5).is_some_and(|n| n == n)`
 
 error: `map_or` is redundant, use is_some_and instead
-  --> $DIR/unnecessary_map_or.rs:21:13
+  --> $DIR/unnecessary_map_or.rs:22:13
    |
 LL |     let _ = Some(5).map_or(false, |n| n == if 2 > 1 { n } else { 0 });
    |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `Some(5).is_some_and(|n| n == if 2 > 1 { n } else { 0 })`
 
 error: `map_or` is redundant, use is_ok_and instead
-  --> $DIR/unnecessary_map_or.rs:22:13
+  --> $DIR/unnecessary_map_or.rs:23:13
    |
 LL |     let _ = Ok::<Vec<i32>, i32>(vec![5]).map_or(false, |n| n == [5]);
    |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `Ok::<Vec<i32>, i32>(vec![5]).is_ok_and(|n| n == [5])`
 
 error: `map_or` is redundant, use standard comparison instead
-  --> $DIR/unnecessary_map_or.rs:23:13
+  --> $DIR/unnecessary_map_or.rs:24:13
    |
 LL |     let _ = Ok::<i32, i32>(5).map_or(false, |n| n == 5);
    |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `Ok::<i32, i32>(5) == Ok(5)`
 
-error: aborting due to 10 previous errors
+error: `map_or` is redundant, use standard comparison instead
+  --> $DIR/unnecessary_map_or.rs:25:13
+   |
+LL |     let _ = Some(5).map_or(false, |n| n == 5).then(|| 1);
+   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `(Some(5) == Some(5))`
+
+error: aborting due to 11 previous errors