Lint casts to u128 in cast_lossless

Author: Alexendoo

clippy_lints/src/casts/cast_lossless.rs

@@ -1,19 +1,21 @@
 use clippy_config::msrvs::{self, Msrv};
-use clippy_utils::diagnostics::span_lint_and_sugg;
+use clippy_utils::diagnostics::span_lint_and_then;
 use clippy_utils::in_constant;
-use clippy_utils::source::{snippet_opt, snippet_with_applicability};
+use clippy_utils::source::snippet_opt;
 use clippy_utils::ty::is_isize_or_usize;
+use rustc_ast::util::parser::PREC_PREFIX;
 use rustc_errors::Applicability;
 use rustc_hir::{Expr, ExprKind, QPath, TyKind};
 use rustc_lint::LateContext;
-use rustc_middle::ty::{self, FloatTy, Ty, UintTy};
+use rustc_middle::ty::{self, FloatTy, Ty};
+use rustc_span::hygiene;
 
 use super::{utils, CAST_LOSSLESS};
 
 pub(super) fn check(
     cx: &LateContext<'_>,
     expr: &Expr<'_>,
-    cast_op: &Expr<'_>,
+    cast_from_expr: &Expr<'_>,
     cast_from: Ty<'_>,
     cast_to: Ty<'_>,
     cast_to_hir: &rustc_hir::Ty<'_>,
@@ -23,64 +25,70 @@ pub(super) fn check(
         return;
     }
 
-    // The suggestion is to use a function call, so if the original expression
-    // has parens on the outside, they are no longer needed.
-    let mut app = Applicability::MachineApplicable;
-    let opt = snippet_opt(cx, cast_op.span.source_callsite());
-    let sugg = opt.as_ref().map_or_else(
-        || {
-            app = Applicability::HasPlaceholders;
-            ".."
-        },
-        |snip| {
-            if should_strip_parens(cast_op, snip) {
-                &snip[1..snip.len() - 1]
-            } else {
-                snip.as_str()
-            }
-        },
-    );
-
-    // Display the type alias instead of the aliased type. Fixes #11285
-    //
-    // FIXME: Once `lazy_type_alias` is stabilized(?) we should use `rustc_middle` types instead,
-    // this will allow us to display the right type with `cast_from` as well.
-    let cast_to_fmt = if let TyKind::Path(QPath::Resolved(None, path)) = cast_to_hir.kind
-        // It's a bit annoying but the turbofish is optional for types. A type in an `as` cast
-        // shouldn't have these if they're primitives, which are the only things we deal with.
-        //
-        // This could be removed for performance if this check is determined to have a pretty major
-        // effect.
-        && path.segments.iter().all(|segment| segment.args.is_none())
-    {
-        snippet_with_applicability(cx, cast_to_hir.span, "..", &mut app)
-    } else {
-        cast_to.to_string().into()
-    };
-
-    let message = if cast_from.is_bool() {
-        format!("casting `{cast_from}` to `{cast_to_fmt}` is more cleanly stated with `{cast_to_fmt}::from(_)`")
-    } else {
-        format!("casting `{cast_from}` to `{cast_to_fmt}` may become silently lossy if you later change the type")
-    };
-
-    span_lint_and_sugg(
+    span_lint_and_then(
         cx,
         CAST_LOSSLESS,
         expr.span,
-        message,
-        "try",
-        format!("{cast_to_fmt}::from({sugg})"),
-        app,
+        format!("casts from `{cast_from}` to `{cast_to}` can be expressed infallibly using `From`"),
+        |diag| {
+            diag.help("an `as` cast can become silently lossy if the types change in the future");
+            let Some(from_snippet) = snippet_opt(cx, hygiene::walk_chain(cast_from_expr.span, expr.span.ctxt())) else {
+                return;
+            };
+            let Some(ty) = snippet_opt(cx, hygiene::walk_chain(cast_to_hir.span, expr.span.ctxt())) else {
+                return;
+            };
+            // The suggestion is to use a function call, so if the original expression
+            // has parens on the outside, they are no longer needed.
+            let from = if let ExprKind::Binary(_, _, _) = cast_from_expr.kind
+                && let Some(from_snippet) = from_snippet.strip_prefix('(')
+                && let Some(from_snippet) = from_snippet.strip_suffix(')')
+            {
+                from_snippet
+            } else {
+                &from_snippet
+            };
+            match cast_to_hir.kind {
+                TyKind::Infer => {
+                    let sugg = if cast_from_expr.precedence().order() == PREC_PREFIX {
+                        format!("({from}")
+                    } else {
+                        from.to_string()
+                    };
+                    diag.span_suggestion_verbose(
+                        expr.span,
+                        "use `Into::into` instead",
+                        format!("{sugg}.into()"),
+                        Applicability::MachineApplicable,
+                    );
+                },
+                // Don't suggest `A<_>::B::From(x)` or `macro!()::from(x)`
+                kind if matches!(kind, TyKind::Path(QPath::Resolved(_, path)) if path.segments.iter().any(|s| s.args.is_some()))
+                    || !cast_to_hir.span.eq_ctxt(expr.span) =>
+                {
+                    diag.span_suggestion_verbose(
+                        expr.span,
+                        format!("use `<{ty}>::from` instead"),
+                        format!("<{ty}>::from({from})"),
+                        Applicability::MachineApplicable,
+                    );
+                },
+                _ => {
+                    diag.span_suggestion_verbose(
+                        expr.span,
+                        format!("use `{ty}::from` instead"),
+                        format!("{ty}::from({from})"),
+                        Applicability::MachineApplicable,
+                    );
+                },
+            }
+        },
     );
 }
 
 fn should_lint(cx: &LateContext<'_>, expr: &Expr<'_>, cast_from: Ty<'_>, cast_to: Ty<'_>, msrv: &Msrv) -> bool {
     // Do not suggest using From in consts/statics until it is valid to do so (see #2267).
-    //
-    // If destination is u128, do not lint because source type cannot be larger
-    // If source is bool, still lint due to the lint message differing (refers to style)
-    if in_constant(cx, expr.hir_id) || (!cast_from.is_bool() && matches!(cast_to.kind(), ty::Uint(UintTy::U128))) {
+    if in_constant(cx, expr.hir_id) {
         return false;
     }
 
@@ -110,12 +118,3 @@ fn should_lint(cx: &LateContext<'_>, expr: &Expr<'_>, cast_from: Ty<'_>, cast_to
         },
     }
 }
-
-fn should_strip_parens(cast_expr: &Expr<'_>, snip: &str) -> bool {
-    if let ExprKind::Binary(_, _, _) = cast_expr.kind {
-        if snip.starts_with('(') && snip.ends_with(')') {
-            return true;
-        }
-    }
-    false
-}

clippy_lints/src/casts/mod.rs

@@ -763,45 +763,45 @@ impl<'tcx> LateLintPass<'tcx> for Casts {
             return;
         }
 
-        if let ExprKind::Cast(cast_expr, cast_to_hir) = expr.kind {
+        if let ExprKind::Cast(cast_from_expr, cast_to_hir) = expr.kind {
             if is_hir_ty_cfg_dependant(cx, cast_to_hir) {
                 return;
             }
             let (cast_from, cast_to) = (
-                cx.typeck_results().expr_ty(cast_expr),
+                cx.typeck_results().expr_ty(cast_from_expr),
                 cx.typeck_results().expr_ty(expr),
             );
 
-            if !expr.span.from_expansion() && unnecessary_cast::check(cx, expr, cast_expr, cast_from, cast_to) {
+            if !expr.span.from_expansion() && unnecessary_cast::check(cx, expr, cast_from_expr, cast_from, cast_to) {
                 return;
             }
-            cast_slice_from_raw_parts::check(cx, expr, cast_expr, cast_to, &self.msrv);
-            ptr_cast_constness::check(cx, expr, cast_expr, cast_from, cast_to, &self.msrv);
-            as_ptr_cast_mut::check(cx, expr, cast_expr, cast_to);
-            fn_to_numeric_cast_any::check(cx, expr, cast_expr, cast_from, cast_to);
-            fn_to_numeric_cast::check(cx, expr, cast_expr, cast_from, cast_to);
-            fn_to_numeric_cast_with_truncation::check(cx, expr, cast_expr, cast_from, cast_to);
-            zero_ptr::check(cx, expr, cast_expr, cast_to_hir);
+            cast_slice_from_raw_parts::check(cx, expr, cast_from_expr, cast_to, &self.msrv);
+            ptr_cast_constness::check(cx, expr, cast_from_expr, cast_from, cast_to, &self.msrv);
+            as_ptr_cast_mut::check(cx, expr, cast_from_expr, cast_to);
+            fn_to_numeric_cast_any::check(cx, expr, cast_from_expr, cast_from, cast_to);
+            fn_to_numeric_cast::check(cx, expr, cast_from_expr, cast_from, cast_to);
+            fn_to_numeric_cast_with_truncation::check(cx, expr, cast_from_expr, cast_from, cast_to);
+            zero_ptr::check(cx, expr, cast_from_expr, cast_to_hir);
 
             if cast_to.is_numeric() {
-                cast_possible_truncation::check(cx, expr, cast_expr, cast_from, cast_to, cast_to_hir.span);
+                cast_possible_truncation::check(cx, expr, cast_from_expr, cast_from, cast_to, cast_to_hir.span);
                 if cast_from.is_numeric() {
                     cast_possible_wrap::check(cx, expr, cast_from, cast_to);
                     cast_precision_loss::check(cx, expr, cast_from, cast_to);
-                    cast_sign_loss::check(cx, expr, cast_expr, cast_from, cast_to);
-                    cast_abs_to_unsigned::check(cx, expr, cast_expr, cast_from, cast_to, &self.msrv);
-                    cast_nan_to_int::check(cx, expr, cast_expr, cast_from, cast_to);
+                    cast_sign_loss::check(cx, expr, cast_from_expr, cast_from, cast_to);
+                    cast_abs_to_unsigned::check(cx, expr, cast_from_expr, cast_from, cast_to, &self.msrv);
+                    cast_nan_to_int::check(cx, expr, cast_from_expr, cast_from, cast_to);
                 }
-                cast_lossless::check(cx, expr, cast_expr, cast_from, cast_to, cast_to_hir, &self.msrv);
-                cast_enum_constructor::check(cx, expr, cast_expr, cast_from);
+                cast_lossless::check(cx, expr, cast_from_expr, cast_from, cast_to, cast_to_hir, &self.msrv);
+                cast_enum_constructor::check(cx, expr, cast_from_expr, cast_from);
             }
 
             as_underscore::check(cx, expr, cast_to_hir);
 
             if self.msrv.meets(msrvs::PTR_FROM_REF) {
-                ref_as_ptr::check(cx, expr, cast_expr, cast_to_hir);
+                ref_as_ptr::check(cx, expr, cast_from_expr, cast_to_hir);
             } else if self.msrv.meets(msrvs::BORROW_AS_PTR) {
-                borrow_as_ptr::check(cx, expr, cast_expr, cast_to_hir);
+                borrow_as_ptr::check(cx, expr, cast_from_expr, cast_to_hir);
             }
         }