fix: `unnecessary_cast` suggests extra brackets when in macro (#14643)

Closes rust-lang/rust-clippy#14640

changelog: [`unnecessary_cast`] fix extra brackets in suggestions when
in macro

4 files changed, 91 insertions, 16 deletions

diff --git a/clippy_lints/src/casts/unnecessary_cast.rs b/clippy_lints/src/casts/unnecessary_cast.rs
index ae994e94a32..8e8c55cf383 100644
--- a/clippy_lints/src/casts/unnecessary_cast.rs
+++ b/clippy_lints/src/casts/unnecessary_cast.rs
@@ -8,7 +8,9 @@ use rustc_errors::Applicability;
 use rustc_hir::def::{DefKind, Res};
 use rustc_hir::{Expr, ExprKind, Lit, Node, Path, QPath, TyKind, UnOp};
 use rustc_lint::{LateContext, LintContext};
+use rustc_middle::ty::adjustment::Adjust;
 use rustc_middle::ty::{self, FloatTy, InferTy, Ty};
+use rustc_span::{Symbol, sym};
 use std::ops::ControlFlow;
 
 use super::UNNECESSARY_CAST;
@@ -142,6 +144,33 @@ pub(super) fn check<'tcx>(
     }
 
     if cast_from.kind() == cast_to.kind() && !expr.span.in_external_macro(cx.sess().source_map()) {
+        enum MaybeParenOrBlock {
+            Paren,
+            Block,
+            Nothing,
+        }
+
+        fn is_borrow_expr(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool {
+            matches!(expr.kind, ExprKind::AddrOf(..))
+                || cx
+                    .typeck_results()
+                    .expr_adjustments(expr)
+                    .first()
+                    .is_some_and(|adj| matches!(adj.kind, Adjust::Borrow(_)))
+        }
+
+        fn is_in_allowed_macro(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool {
+            const ALLOWED_MACROS: &[Symbol] = &[
+                sym::format_args_macro,
+                sym::assert_eq_macro,
+                sym::debug_assert_eq_macro,
+                sym::assert_ne_macro,
+                sym::debug_assert_ne_macro,
+            ];
+            matches!(expr.span.ctxt().outer_expn_data().macro_def_id, Some(def_id) if 
+                cx.tcx.get_diagnostic_name(def_id).is_some_and(|sym| ALLOWED_MACROS.contains(&sym)))
+        }
+
         if let Some(id) = path_to_local(cast_expr)
             && !cx.tcx.hir_span(id).eq_ctxt(cast_expr.span)
         {
@@ -150,15 +179,15 @@ pub(super) fn check<'tcx>(
             return false;
         }
 
-        // If the whole cast expression is a unary expression (`(*x as T)`) or an addressof
-        // expression (`(&x as T)`), then not surrounding the suggestion into a block risks us
-        // changing the precedence of operators if the cast expression is followed by an operation
-        // with higher precedence than the unary operator (`(*x as T).foo()` would become
-        // `*x.foo()`, which changes what the `*` applies on).
-        // The same is true if the expression encompassing the cast expression is a unary
-        // expression or an addressof expression.
-        let needs_block = matches!(cast_expr.kind, ExprKind::Unary(..) | ExprKind::AddrOf(..))
-            || get_parent_expr(cx, expr).is_some_and(|e| matches!(e.kind, ExprKind::Unary(..) | ExprKind::AddrOf(..)));
+        // Changing `&(x as i32)` to `&x` would change the meaning of the code because the previous creates
+        // a reference to the temporary while the latter creates a reference to the original value.
+        let surrounding = match cx.tcx.parent_hir_node(expr.hir_id) {
+            Node::Expr(parent) if is_borrow_expr(cx, parent) && !is_in_allowed_macro(cx, parent) => {
+                MaybeParenOrBlock::Block
+            },
+            Node::Expr(parent) if cast_expr.precedence() < parent.precedence() => MaybeParenOrBlock::Paren,
+            _ => MaybeParenOrBlock::Nothing,
+        };
 
         span_lint_and_sugg(
             cx,
@@ -166,10 +195,10 @@ pub(super) fn check<'tcx>(
             expr.span,
             format!("casting to the same type is unnecessary (`{cast_from}` -> `{cast_to}`)"),
             "try",
-            if needs_block {
-                format!("{{ {cast_str} }}")
-            } else {
-                cast_str
+            match surrounding {
+                MaybeParenOrBlock::Paren => format!("({cast_str})"),
+                MaybeParenOrBlock::Block => format!("{{ {cast_str} }}"),
+                MaybeParenOrBlock::Nothing => cast_str,
             },
             Applicability::MachineApplicable,
         );
diff --git a/tests/ui/unnecessary_cast.fixed b/tests/ui/unnecessary_cast.fixed
index ba167e79a30..91ff4b9ee77 100644
--- a/tests/ui/unnecessary_cast.fixed
+++ b/tests/ui/unnecessary_cast.fixed
@@ -266,7 +266,21 @@ mod fixable {
     // Issue #11968: The suggestion for this lint removes the parentheses and leave the code as
     // `*x.pow(2)` which tries to dereference the return value rather than `x`.
     fn issue_11968(x: &usize) -> usize {
-        { *x }.pow(2)
+        (*x).pow(2)
+        //~^ unnecessary_cast
+    }
+
+    #[allow(clippy::cast_lossless)]
+    fn issue_14640() {
+        let x = 5usize;
+        let vec: Vec<u64> = vec![1, 2, 3, 4, 5];
+        assert_eq!(vec.len(), x);
+        //~^ unnecessary_cast
+
+        let _ = (5i32 as i64).abs();
+        //~^ unnecessary_cast
+
+        let _ = 5i32 as i64;
         //~^ unnecessary_cast
     }
 }
diff --git a/tests/ui/unnecessary_cast.rs b/tests/ui/unnecessary_cast.rs
index 0f90a8b0596..5444a914db1 100644
--- a/tests/ui/unnecessary_cast.rs
+++ b/tests/ui/unnecessary_cast.rs
@@ -269,4 +269,18 @@ mod fixable {
         (*x as usize).pow(2)
         //~^ unnecessary_cast
     }
+
+    #[allow(clippy::cast_lossless)]
+    fn issue_14640() {
+        let x = 5usize;
+        let vec: Vec<u64> = vec![1, 2, 3, 4, 5];
+        assert_eq!(vec.len(), x as usize);
+        //~^ unnecessary_cast
+
+        let _ = (5i32 as i64 as i64).abs();
+        //~^ unnecessary_cast
+
+        let _ = 5i32 as i64 as i64;
+        //~^ unnecessary_cast
+    }
 }
diff --git a/tests/ui/unnecessary_cast.stderr b/tests/ui/unnecessary_cast.stderr
index c83770c1a29..3e3c5eb81c1 100644
--- a/tests/ui/unnecessary_cast.stderr
+++ b/tests/ui/unnecessary_cast.stderr
@@ -245,7 +245,25 @@ error: casting to the same type is unnecessary (`usize` -> `usize`)
   --> tests/ui/unnecessary_cast.rs:269:9
    |
 LL |         (*x as usize).pow(2)
-   |         ^^^^^^^^^^^^^ help: try: `{ *x }`
+   |         ^^^^^^^^^^^^^ help: try: `(*x)`
 
-error: aborting due to 41 previous errors
+error: casting to the same type is unnecessary (`usize` -> `usize`)
+  --> tests/ui/unnecessary_cast.rs:277:31
+   |
+LL |         assert_eq!(vec.len(), x as usize);
+   |                               ^^^^^^^^^^ help: try: `x`
+
+error: casting to the same type is unnecessary (`i64` -> `i64`)
+  --> tests/ui/unnecessary_cast.rs:280:17
+   |
+LL |         let _ = (5i32 as i64 as i64).abs();
+   |                 ^^^^^^^^^^^^^^^^^^^^ help: try: `(5i32 as i64)`
+
+error: casting to the same type is unnecessary (`i64` -> `i64`)
+  --> tests/ui/unnecessary_cast.rs:283:17
+   |
+LL |         let _ = 5i32 as i64 as i64;
+   |                 ^^^^^^^^^^^^^^^^^^ help: try: `5i32 as i64`
+
+error: aborting due to 44 previous errors