Fix multiple problems in rust-lang#15063 (rust-lang#15070)

llogiq · web-flow · commit 49ca220ca0a6 · 2025-07-09T21:55:36.000Z
Closes rust-lang/rust-clippy#15063 ---- changelog: [`op_ref`]: fix wrongly showed macro definition in suggestions changelog: [`needless_bool_assign`]: fix missing curlies when on else if
diff --git a/clippy_lints/src/needless_bool.rs b/clippy_lints/src/needless_bool.rs
@@ -199,11 +199,16 @@ impl<'tcx> LateLintPass<'tcx> for NeedlessBool {
                 let mut applicability = Applicability::MachineApplicable;
                 let cond = Sugg::hir_with_applicability(cx, cond, "..", &mut applicability);
                 let lhs = snippet_with_applicability(cx, lhs_a.span, "..", &mut applicability);
-                let sugg = if a == b {
+                let mut sugg = if a == b {
                     format!("{cond}; {lhs} = {a:?};")
                 } else {
                     format!("{lhs} = {};", if a { cond } else { !cond })
                 };
+
+                if is_else_clause(cx.tcx, e) {
+                    sugg = format!("{{ {sugg} }}");
+                }
+
                 span_lint_and_sugg(
                     cx,
                     NEEDLESS_BOOL_ASSIGN,
diff --git a/clippy_lints/src/operators/op_ref.rs b/clippy_lints/src/operators/op_ref.rs
@@ -1,6 +1,6 @@
 use clippy_utils::diagnostics::span_lint_and_then;
 use clippy_utils::get_enclosing_block;
-use clippy_utils::source::snippet;
+use clippy_utils::source::snippet_with_context;
 use clippy_utils::ty::{implements_trait, is_copy};
 use rustc_errors::Applicability;
 use rustc_hir::def::Res;
@@ -61,12 +61,13 @@ pub(crate) fn check<'tcx>(
                         e.span,
                         "needlessly taken reference of both operands",
                         |diag| {
-                            let lsnip = snippet(cx, l.span, "...").to_string();
-                            let rsnip = snippet(cx, r.span, "...").to_string();
+                            let mut applicability = Applicability::MachineApplicable;
+                            let (lsnip, _) = snippet_with_context(cx, l.span, e.span.ctxt(), "...", &mut applicability);
+                            let (rsnip, _) = snippet_with_context(cx, r.span, e.span.ctxt(), "...", &mut applicability);
                             diag.multipart_suggestion(
                                 "use the values directly",
-                                vec![(left.span, lsnip), (right.span, rsnip)],
-                                Applicability::MachineApplicable,
+                                vec![(left.span, lsnip.to_string()), (right.span, rsnip.to_string())],
+                                applicability,
                             );
                         },
                     );
@@ -80,13 +81,9 @@ pub(crate) fn check<'tcx>(
                         e.span,
                         "needlessly taken reference of left operand",
                         |diag| {
-                            let lsnip = snippet(cx, l.span, "...").to_string();
-                            diag.span_suggestion(
-                                left.span,
-                                "use the left value directly",
-                                lsnip,
-                                Applicability::MachineApplicable,
-                            );
+                            let mut applicability = Applicability::MachineApplicable;
+                            let (lsnip, _) = snippet_with_context(cx, l.span, e.span.ctxt(), "...", &mut applicability);
+                            diag.span_suggestion(left.span, "use the left value directly", lsnip, applicability);
                         },
                     );
                 } else if !lcpy
@@ -99,7 +96,8 @@ pub(crate) fn check<'tcx>(
                         e.span,
                         "needlessly taken reference of right operand",
                         |diag| {
-                            let rsnip = snippet(cx, r.span, "...").to_string();
+                            let mut applicability = Applicability::MachineApplicable;
+                            let (rsnip, _) = snippet_with_context(cx, r.span, e.span.ctxt(), "...", &mut applicability);
                             diag.span_suggestion(
                                 right.span,
                                 "use the right value directly",
@@ -131,7 +129,8 @@ pub(crate) fn check<'tcx>(
                         e.span,
                         "needlessly taken reference of left operand",
                         |diag| {
-                            let lsnip = snippet(cx, l.span, "...").to_string();
+                            let mut applicability = Applicability::MachineApplicable;
+                            let (lsnip, _) = snippet_with_context(cx, l.span, e.span.ctxt(), "...", &mut applicability);
                             diag.span_suggestion(
                                 left.span,
                                 "use the left value directly",
@@ -158,7 +157,8 @@ pub(crate) fn check<'tcx>(
                     && implements_trait(cx, cx.typeck_results().expr_ty(left), trait_id, &[rty.into()])
                 {
                     span_lint_and_then(cx, OP_REF, e.span, "taken reference of right operand", |diag| {
-                        let rsnip = snippet(cx, r.span, "...").to_string();
+                        let mut applicability = Applicability::MachineApplicable;
+                        let (rsnip, _) = snippet_with_context(cx, r.span, e.span.ctxt(), "...", &mut applicability);
                         diag.span_suggestion(
                             right.span,
                             "use the right value directly",
diff --git a/tests/ui/needless_bool_assign.fixed b/tests/ui/needless_bool_assign.fixed
@@ -33,3 +33,12 @@ fn main() {
         b = true;
     }
 }
+
+fn issue15063(x: bool, y: bool) {
+    let mut z = false;
+
+    if x && y {
+        todo!()
+    } else { z = x || y; }
+    //~^^^^^ needless_bool_assign
+}
diff --git a/tests/ui/needless_bool_assign.rs b/tests/ui/needless_bool_assign.rs
@@ -45,3 +45,16 @@ fn main() {
         b = true;
     }
 }
+
+fn issue15063(x: bool, y: bool) {
+    let mut z = false;
+
+    if x && y {
+        todo!()
+    } else if x || y {
+        z = true;
+    } else {
+        z = false;
+    }
+    //~^^^^^ needless_bool_assign
+}
diff --git a/tests/ui/needless_bool_assign.stderr b/tests/ui/needless_bool_assign.stderr
@@ -51,5 +51,16 @@ LL | |     }
    = note: `-D clippy::if-same-then-else` implied by `-D warnings`
    = help: to override `-D warnings` add `#[allow(clippy::if_same_then_else)]`
 
-error: aborting due to 4 previous errors
+error: this if-then-else expression assigns a bool literal
+  --> tests/ui/needless_bool_assign.rs:54:12
+   |
+LL |       } else if x || y {
+   |  ____________^
+LL | |         z = true;
+LL | |     } else {
+LL | |         z = false;
+LL | |     }
+   | |_____^ help: you can reduce it to: `{ z = x || y; }`
+
+error: aborting due to 5 previous errors
 
diff --git a/tests/ui/op_ref.fixed b/tests/ui/op_ref.fixed
@@ -110,3 +110,37 @@ mod issue_2597 {
         &array[idx] < val
     }
 }
+
+#[allow(clippy::needless_if)]
+fn issue15063() {
+    use std::ops::BitAnd;
+
+    macro_rules! mac {
+        ($e:expr) => {
+            $e.clone()
+        };
+    }
+
+    let x = 1;
+    if x == mac!(1) {}
+    //~^ op_ref
+
+    #[derive(Copy, Clone)]
+    struct Y(i32);
+    impl BitAnd for Y {
+        type Output = Y;
+        fn bitand(self, rhs: Y) -> Y {
+            Y(self.0 & rhs.0)
+        }
+    }
+    impl<'a> BitAnd<&'a Y> for Y {
+        type Output = Y;
+        fn bitand(self, rhs: &'a Y) -> Y {
+            Y(self.0 & rhs.0)
+        }
+    }
+    let x = Y(1);
+    let y = Y(2);
+    let z = x & mac!(y);
+    //~^ op_ref
+}
diff --git a/tests/ui/op_ref.rs b/tests/ui/op_ref.rs
@@ -110,3 +110,37 @@ mod issue_2597 {
         &array[idx] < val
     }
 }
+
+#[allow(clippy::needless_if)]
+fn issue15063() {
+    use std::ops::BitAnd;
+
+    macro_rules! mac {
+        ($e:expr) => {
+            $e.clone()
+        };
+    }
+
+    let x = 1;
+    if &x == &mac!(1) {}
+    //~^ op_ref
+
+    #[derive(Copy, Clone)]
+    struct Y(i32);
+    impl BitAnd for Y {
+        type Output = Y;
+        fn bitand(self, rhs: Y) -> Y {
+            Y(self.0 & rhs.0)
+        }
+    }
+    impl<'a> BitAnd<&'a Y> for Y {
+        type Output = Y;
+        fn bitand(self, rhs: &'a Y) -> Y {
+            Y(self.0 & rhs.0)
+        }
+    }
+    let x = Y(1);
+    let y = Y(2);
+    let z = x & &mac!(y);
+    //~^ op_ref
+}
diff --git a/tests/ui/op_ref.stderr b/tests/ui/op_ref.stderr
@@ -36,5 +36,25 @@ LL |         let _ = two + &three;
    |                       |
    |                       help: use the right value directly: `three`
 
-error: aborting due to 4 previous errors
+error: needlessly taken reference of both operands
+  --> tests/ui/op_ref.rs:125:8
+   |
+LL |     if &x == &mac!(1) {}
+   |        ^^^^^^^^^^^^^^
+   |
+help: use the values directly
+   |
+LL -     if &x == &mac!(1) {}
+LL +     if x == mac!(1) {}
+   |
+
+error: taken reference of right operand
+  --> tests/ui/op_ref.rs:144:13
+   |
+LL |     let z = x & &mac!(y);
+   |             ^^^^--------
+   |                 |
+   |                 help: use the right value directly: `mac!(y)`
+
+error: aborting due to 6 previous errors
 

Original file line number	Diff line number	Diff line change
`@@ -33,3 +33,12 @@ fn main() {`
`33`	`33`	`b = true;`
`34`	`34`	`}`
`35`	`35`	`}`
	`36`	`+`
	`37`	`+fn issue15063(x: bool, y: bool) {`
	`38`	`+ let mut z = false;`
	`39`	`+`
	`40`	`+ if x && y {`
	`41`	`+ todo!()`
	`42`	`+ } else { z = x \|\| y; }`
	`43`	`+ //~^^^^^ needless_bool_assign`
	`44`	`+}`