Auto merge of #13146 - Alexendoo:cast-lossless-128, r=y21

Lint casts to `u128` in `cast_lossless`

Reverts #12496 per https://rust-lang.zulipchat.com/#narrow/stream/257328-clippy/topic/Should.20.60as.20u128.60.20trigger.20cast_lossless

Also changes the lint messages and refactors the suggestion production - Fixes #12695

changelog: [`cast_lossless`]: lint casts to `u128`

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::sugg::Sugg;
 use clippy_utils::ty::is_isize_or_usize;
 use rustc_errors::Applicability;
-use rustc_hir::{Expr, ExprKind, QPath, TyKind};
+use rustc_hir::{Expr, 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,54 @@ 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 mut applicability = Applicability::MachineApplicable;
+ let from_sugg = Sugg::hir_with_context(cx, cast_from_expr, expr.span.ctxt(), "<from>", &mut applicability);
+ let Some(ty) = snippet_opt(cx, hygiene::walk_chain(cast_to_hir.span, expr.span.ctxt())) else {
+ return;
+ };
+ match cast_to_hir.kind {
+ TyKind::Infer => {
+ diag.span_suggestion_verbose(
+ expr.span,
+ "use `Into::into` instead",
+ format!("{}.into()", from_sugg.maybe_par()),
+ applicability,
+ );
+ },
+ // 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_sugg})"),
+ applicability,
+ );
+ },
+ _ => {
+ diag.span_suggestion_verbose(
+ expr.span,
+ format!("use `{ty}::from` instead"),
+ format!("{ty}::from({from_sugg})"),
+ applicability,
+ );
+ },
+ }
+ },
  );
 }
 
 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 +102,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);
  }
  }
 

clippy_utils/src/sugg.rs

@@ -52,7 +52,8 @@ impl Display for Sugg<'_> {
 impl<'a> Sugg<'a> {
  /// Prepare a suggestion from an expression.
  pub fn hir_opt(cx: &LateContext<'_>, expr: &hir::Expr<'_>) -> Option<Self> {
- let get_snippet = |span| snippet(cx, span, "");
+ let ctxt = expr.span.ctxt();
+ let get_snippet = |span| snippet_with_context(cx, span, ctxt, "", &mut Applicability::Unspecified).0;
  snippet_opt(cx, expr.span).map(|_| Self::hir_from_snippet(expr, get_snippet))
  }
 
@@ -100,7 +101,9 @@ impl<'a> Sugg<'a> {
  applicability: &mut Applicability,
  ) -> Self {
  if expr.span.ctxt() == ctxt {
- Self::hir_from_snippet(expr, |span| snippet(cx, span, default))
+ Self::hir_from_snippet(expr, |span| {
+ snippet_with_context(cx, span, ctxt, default, applicability).0
+ })
  } else {
  let (snip, _) = snippet_with_context(cx, expr.span, ctxt, default, applicability);
  Sugg::NonParen(snip)
@@ -109,7 +112,7 @@ impl<'a> Sugg<'a> {
 
  /// Generate a suggestion for an expression with the given snippet. This is used by the `hir_*`
  /// function variants of `Sugg`, since these use different snippet functions.
- fn hir_from_snippet(expr: &hir::Expr<'_>, get_snippet: impl Fn(Span) -> Cow<'a, str>) -> Self {
+ fn hir_from_snippet(expr: &hir::Expr<'_>, mut get_snippet: impl FnMut(Span) -> Cow<'a, str>) -> Self {
  if let Some(range) = higher::Range::hir(expr) {
  let op = match range.limits {
  ast::RangeLimits::HalfOpen => AssocOp::DotDot,