Preserve parentheses in suggestion in presence of cascaded casts (#16483)

changelog: [`unnecessary_cast`]: preserve parentheses in presence of
cascaded casts

Fixes rust-lang/rust-clippy#16475

clippy_lints/src/casts/unnecessary_cast.rs

@@ -1,7 +1,8 @@
 use clippy_utils::diagnostics::span_lint_and_sugg;
 use clippy_utils::numeric_literal::NumericLiteral;
 use clippy_utils::res::MaybeResPath as _;
-use clippy_utils::source::{SpanRangeExt, snippet_opt};
+use clippy_utils::source::{SpanRangeExt, snippet, snippet_with_applicability};
+use clippy_utils::sugg::has_enclosing_paren;
 use clippy_utils::visitors::{Visitable, for_each_expr_without_closures};
 use clippy_utils::{get_parent_expr, is_hir_ty_cfg_dependant, is_ty_alias, sym};
 use rustc_ast::{LitFloatType, LitIntType, LitKind};
@@ -24,7 +25,8 @@ pub(super) fn check<'tcx>(
     cast_from: Ty<'tcx>,
     cast_to: Ty<'tcx>,
 ) -> bool {
-    let cast_str = snippet_opt(cx, cast_expr.span).unwrap_or_default();
+    let mut app = Applicability::MachineApplicable;
+    let cast_str = snippet_with_applicability(cx, cast_expr.span, "_", &mut app);
 
     if let ty::RawPtr(..) = cast_from.kind()
         // check both mutability and type are the same
@@ -47,16 +49,23 @@ pub(super) fn check<'tcx>(
             _ => {},
         }
 
-        span_lint_and_sugg(
+        // Preserve parentheses around `expr` in case of cascaded casts
+        let surrounding =
+            if matches!(cast_expr.kind, ExprKind::Cast(..)) && has_enclosing_paren(snippet(cx, expr.span, "")) {
+                MaybeParenOrBlock::Paren
+            } else {
+                MaybeParenOrBlock::Nothing
+            };
+
+        emit_lint(
             cx,
-            UNNECESSARY_CAST,
-            expr.span,
+            expr,
             format!(
                 "casting raw pointers to the same type and constness is unnecessary (`{cast_from}` -> `{cast_to}`)"
             ),
-            "try",
-            cast_str.clone(),
-            Applicability::MaybeIncorrect,
+            &cast_str,
+            surrounding,
+            app.max(Applicability::MaybeIncorrect),
         );
     }
 
@@ -143,12 +152,6 @@ 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
@@ -188,18 +191,13 @@ fn is_in_allowed_macro(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool {
             _ => MaybeParenOrBlock::Nothing,
         };
 
-        span_lint_and_sugg(
+        emit_lint(
             cx,
-            UNNECESSARY_CAST,
-            expr.span,
+            expr,
             format!("casting to the same type is unnecessary (`{cast_from}` -> `{cast_to}`)"),
-            "try",
-            match surrounding {
-                MaybeParenOrBlock::Paren => format!("({cast_str})"),
-                MaybeParenOrBlock::Block => format!("{{ {cast_str} }}"),
-                MaybeParenOrBlock::Nothing => cast_str,
-            },
-            Applicability::MachineApplicable,
+            &cast_str,
+            surrounding,
+            app,
         );
         return true;
     }
@@ -312,3 +310,33 @@ fn is_cast_from_ty_alias<'tcx>(cx: &LateContext<'tcx>, expr: impl Visitable<'tcx
     })
     .is_some()
 }
+
+#[derive(Clone, Copy)]
+enum MaybeParenOrBlock {
+    Paren,
+    Block,
+    Nothing,
+}
+
+fn emit_lint(
+    cx: &LateContext<'_>,
+    expr: &Expr<'_>,
+    msg: String,
+    sugg: &str,
+    surrounding: MaybeParenOrBlock,
+    applicability: Applicability,
+) {
+    span_lint_and_sugg(
+        cx,
+        UNNECESSARY_CAST,
+        expr.span,
+        msg,
+        "try",
+        match surrounding {
+            MaybeParenOrBlock::Paren => format!("({sugg})"),
+            MaybeParenOrBlock::Block => format!("{{ {sugg} }}"),
+            MaybeParenOrBlock::Nothing => sugg.to_string(),
+        },
+        applicability,
+    );
+}

tests/ui/unnecessary_cast.fixed

@@ -285,3 +285,11 @@ mod fixable {
         //~^ unnecessary_cast
     }
 }
+
+fn issue16475() -> *const u8 {
+    static NONE: Option<((), &'static u8)> = None;
+    unsafe {
+        *(&NONE as *const _ as *const _ as *const *const u8)
+        //~^ unnecessary_cast
+    }
+}

tests/ui/unnecessary_cast.rs

@@ -285,3 +285,11 @@ fn issue_14640() {
         //~^ unnecessary_cast
     }
 }
+
+fn issue16475() -> *const u8 {
+    static NONE: Option<((), &'static u8)> = None;
+    unsafe {
+        *(&NONE as *const _ as *const _ as *const *const u8 as *const *const u8)
+        //~^ unnecessary_cast
+    }
+}

tests/ui/unnecessary_cast.stderr

@@ -277,5 +277,11 @@ error: casting to the same type is unnecessary (`i64` -> `i64`)
 LL |         let _ = 5i32 as i64 as i64;
    |                 ^^^^^^^^^^^^^^^^^^ help: try: `5i32 as i64`
 
-error: aborting due to 46 previous errors
+error: casting raw pointers to the same type and constness is unnecessary (`*const *const u8` -> `*const *const u8`)
+  --> tests/ui/unnecessary_cast.rs:292:10
+   |
+LL |         *(&NONE as *const _ as *const _ as *const *const u8 as *const *const u8)
+   |          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `(&NONE as *const _ as *const _ as *const *const u8)`
+
+error: aborting due to 47 previous errors