fix: unnecessary_cast suggests extra brackets when in macro

Author: profetia

clippy_lints/src/casts/unnecessary_cast.rs

@@ -8,6 +8,7 @@ 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 std::ops::ControlFlow;
 
@@ -142,6 +143,21 @@ 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)
+                    .iter()
+                    .any(|adj| matches!(adj.kind, Adjust::Borrow(_)))
+        }
+
         if let Some(id) = path_to_local(cast_expr)
             && !cx.tcx.hir_span(id).eq_ctxt(cast_expr.span)
         {
@@ -150,26 +166,26 @@ 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(..)));
+        let surrounding = match cx.tcx.parent_hir_node(expr.hir_id) {
+            // 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.
+            Node::Expr(parent) if !parent.span.from_expansion() && is_borrow_expr(cx, parent) => {
+                MaybeParenOrBlock::Block
+            },
+            Node::Expr(parent) if cast_expr.precedence() < parent.precedence() => MaybeParenOrBlock::Paren,
+            _ => MaybeParenOrBlock::Nothing,
+        };
 
         span_lint_and_sugg(
             cx,
             UNNECESSARY_CAST,
             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,
         );

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
     }
 }

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
+    }
 }

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