Skip to content

Commit 610c5e2

Browse files
committed
Lint casts to u128 in cast_lossless
1 parent 1807580 commit 610c5e2

8 files changed

+826
-278
lines changed
+34-64
Original file line numberDiff line numberDiff line change
@@ -1,19 +1,20 @@
11
use clippy_config::msrvs::{self, Msrv};
2-
use clippy_utils::diagnostics::span_lint_and_sugg;
2+
use clippy_utils::diagnostics::span_lint_and_then;
33
use clippy_utils::in_constant;
4-
use clippy_utils::source::{snippet_opt, snippet_with_applicability};
4+
use clippy_utils::source::snippet_opt;
55
use clippy_utils::ty::is_isize_or_usize;
66
use rustc_errors::Applicability;
7-
use rustc_hir::{Expr, ExprKind, QPath, TyKind};
7+
use rustc_hir::{Expr, ExprKind};
88
use rustc_lint::LateContext;
9-
use rustc_middle::ty::{self, FloatTy, Ty, UintTy};
9+
use rustc_middle::ty::{self, FloatTy, Ty};
10+
use rustc_span::hygiene;
1011

1112
use super::{utils, CAST_LOSSLESS};
1213

1314
pub(super) fn check(
1415
cx: &LateContext<'_>,
1516
expr: &Expr<'_>,
16-
cast_op: &Expr<'_>,
17+
cast_from_expr: &Expr<'_>,
1718
cast_from: Ty<'_>,
1819
cast_to: Ty<'_>,
1920
cast_to_hir: &rustc_hir::Ty<'_>,
@@ -23,64 +24,42 @@ pub(super) fn check(
2324
return;
2425
}
2526

26-
// The suggestion is to use a function call, so if the original expression
27-
// has parens on the outside, they are no longer needed.
28-
let mut app = Applicability::MachineApplicable;
29-
let opt = snippet_opt(cx, cast_op.span.source_callsite());
30-
let sugg = opt.as_ref().map_or_else(
31-
|| {
32-
app = Applicability::HasPlaceholders;
33-
".."
34-
},
35-
|snip| {
36-
if should_strip_parens(cast_op, snip) {
37-
&snip[1..snip.len() - 1]
38-
} else {
39-
snip.as_str()
40-
}
41-
},
42-
);
43-
44-
// Display the type alias instead of the aliased type. Fixes #11285
45-
//
46-
// FIXME: Once `lazy_type_alias` is stabilized(?) we should use `rustc_middle` types instead,
47-
// this will allow us to display the right type with `cast_from` as well.
48-
let cast_to_fmt = if let TyKind::Path(QPath::Resolved(None, path)) = cast_to_hir.kind
49-
// It's a bit annoying but the turbofish is optional for types. A type in an `as` cast
50-
// shouldn't have these if they're primitives, which are the only things we deal with.
51-
//
52-
// This could be removed for performance if this check is determined to have a pretty major
53-
// effect.
54-
&& path.segments.iter().all(|segment| segment.args.is_none())
55-
{
56-
snippet_with_applicability(cx, cast_to_hir.span, "..", &mut app)
57-
} else {
58-
cast_to.to_string().into()
59-
};
60-
61-
let message = if cast_from.is_bool() {
62-
format!("casting `{cast_from}` to `{cast_to_fmt}` is more cleanly stated with `{cast_to_fmt}::from(_)`")
63-
} else {
64-
format!("casting `{cast_from}` to `{cast_to_fmt}` may become silently lossy if you later change the type")
65-
};
66-
67-
span_lint_and_sugg(
27+
span_lint_and_then(
6828
cx,
6929
CAST_LOSSLESS,
7030
expr.span,
71-
message,
72-
"try",
73-
format!("{cast_to_fmt}::from({sugg})"),
74-
app,
31+
format!("casts from `{cast_from}` to `{cast_to}` can expressed infallibly using `From`"),
32+
|diag| {
33+
diag.help("an `as` cast can become silently lossy if the types change in the future");
34+
let Some(from_snippet) = snippet_opt(cx, hygiene::walk_chain(cast_from_expr.span, expr.span.ctxt())) else {
35+
return;
36+
};
37+
let Some(ty) = snippet_opt(cx, hygiene::walk_chain(cast_to_hir.span, expr.span.ctxt())) else {
38+
return;
39+
};
40+
// The suggestion is to use a function call, so if the original expression
41+
// has parens on the outside, they are no longer needed.
42+
let from = if let ExprKind::Binary(_, _, _) = cast_from_expr.kind
43+
&& let Some(from_snippet) = from_snippet.strip_prefix('(')
44+
&& let Some(from_snippet) = from_snippet.strip_suffix(')')
45+
{
46+
from_snippet
47+
} else {
48+
&from_snippet
49+
};
50+
diag.span_suggestion_verbose(
51+
expr.span,
52+
format!("use `{ty}::from` instead"),
53+
format!("{ty}::from({from})"),
54+
Applicability::MachineApplicable,
55+
);
56+
},
7557
);
7658
}
7759

7860
fn should_lint(cx: &LateContext<'_>, expr: &Expr<'_>, cast_from: Ty<'_>, cast_to: Ty<'_>, msrv: &Msrv) -> bool {
7961
// Do not suggest using From in consts/statics until it is valid to do so (see #2267).
80-
//
81-
// If destination is u128, do not lint because source type cannot be larger
82-
// If source is bool, still lint due to the lint message differing (refers to style)
83-
if in_constant(cx, expr.hir_id) || (!cast_from.is_bool() && matches!(cast_to.kind(), ty::Uint(UintTy::U128))) {
62+
if in_constant(cx, expr.hir_id) {
8463
return false;
8564
}
8665

@@ -110,12 +89,3 @@ fn should_lint(cx: &LateContext<'_>, expr: &Expr<'_>, cast_from: Ty<'_>, cast_to
11089
},
11190
}
11291
}
113-
114-
fn should_strip_parens(cast_expr: &Expr<'_>, snip: &str) -> bool {
115-
if let ExprKind::Binary(_, _, _) = cast_expr.kind {
116-
if snip.starts_with('(') && snip.ends_with(')') {
117-
return true;
118-
}
119-
}
120-
false
121-
}

clippy_lints/src/casts/mod.rs

+18-18
Original file line numberDiff line numberDiff line change
@@ -763,45 +763,45 @@ impl<'tcx> LateLintPass<'tcx> for Casts {
763763
return;
764764
}
765765

766-
if let ExprKind::Cast(cast_expr, cast_to_hir) = expr.kind {
766+
if let ExprKind::Cast(cast_from_expr, cast_to_hir) = expr.kind {
767767
if is_hir_ty_cfg_dependant(cx, cast_to_hir) {
768768
return;
769769
}
770770
let (cast_from, cast_to) = (
771-
cx.typeck_results().expr_ty(cast_expr),
771+
cx.typeck_results().expr_ty(cast_from_expr),
772772
cx.typeck_results().expr_ty(expr),
773773
);
774774

775-
if !expr.span.from_expansion() && unnecessary_cast::check(cx, expr, cast_expr, cast_from, cast_to) {
775+
if !expr.span.from_expansion() && unnecessary_cast::check(cx, expr, cast_from_expr, cast_from, cast_to) {
776776
return;
777777
}
778-
cast_slice_from_raw_parts::check(cx, expr, cast_expr, cast_to, &self.msrv);
779-
ptr_cast_constness::check(cx, expr, cast_expr, cast_from, cast_to, &self.msrv);
780-
as_ptr_cast_mut::check(cx, expr, cast_expr, cast_to);
781-
fn_to_numeric_cast_any::check(cx, expr, cast_expr, cast_from, cast_to);
782-
fn_to_numeric_cast::check(cx, expr, cast_expr, cast_from, cast_to);
783-
fn_to_numeric_cast_with_truncation::check(cx, expr, cast_expr, cast_from, cast_to);
784-
zero_ptr::check(cx, expr, cast_expr, cast_to_hir);
778+
cast_slice_from_raw_parts::check(cx, expr, cast_from_expr, cast_to, &self.msrv);
779+
ptr_cast_constness::check(cx, expr, cast_from_expr, cast_from, cast_to, &self.msrv);
780+
as_ptr_cast_mut::check(cx, expr, cast_from_expr, cast_to);
781+
fn_to_numeric_cast_any::check(cx, expr, cast_from_expr, cast_from, cast_to);
782+
fn_to_numeric_cast::check(cx, expr, cast_from_expr, cast_from, cast_to);
783+
fn_to_numeric_cast_with_truncation::check(cx, expr, cast_from_expr, cast_from, cast_to);
784+
zero_ptr::check(cx, expr, cast_from_expr, cast_to_hir);
785785

786786
if cast_to.is_numeric() {
787-
cast_possible_truncation::check(cx, expr, cast_expr, cast_from, cast_to, cast_to_hir.span);
787+
cast_possible_truncation::check(cx, expr, cast_from_expr, cast_from, cast_to, cast_to_hir.span);
788788
if cast_from.is_numeric() {
789789
cast_possible_wrap::check(cx, expr, cast_from, cast_to);
790790
cast_precision_loss::check(cx, expr, cast_from, cast_to);
791-
cast_sign_loss::check(cx, expr, cast_expr, cast_from, cast_to);
792-
cast_abs_to_unsigned::check(cx, expr, cast_expr, cast_from, cast_to, &self.msrv);
793-
cast_nan_to_int::check(cx, expr, cast_expr, cast_from, cast_to);
791+
cast_sign_loss::check(cx, expr, cast_from_expr, cast_from, cast_to);
792+
cast_abs_to_unsigned::check(cx, expr, cast_from_expr, cast_from, cast_to, &self.msrv);
793+
cast_nan_to_int::check(cx, expr, cast_from_expr, cast_from, cast_to);
794794
}
795-
cast_lossless::check(cx, expr, cast_expr, cast_from, cast_to, cast_to_hir, &self.msrv);
796-
cast_enum_constructor::check(cx, expr, cast_expr, cast_from);
795+
cast_lossless::check(cx, expr, cast_from_expr, cast_from, cast_to, cast_to_hir, &self.msrv);
796+
cast_enum_constructor::check(cx, expr, cast_from_expr, cast_from);
797797
}
798798

799799
as_underscore::check(cx, expr, cast_to_hir);
800800

801801
if self.msrv.meets(msrvs::PTR_FROM_REF) {
802-
ref_as_ptr::check(cx, expr, cast_expr, cast_to_hir);
802+
ref_as_ptr::check(cx, expr, cast_from_expr, cast_to_hir);
803803
} else if self.msrv.meets(msrvs::BORROW_AS_PTR) {
804-
borrow_as_ptr::check(cx, expr, cast_expr, cast_to_hir);
804+
borrow_as_ptr::check(cx, expr, cast_from_expr, cast_to_hir);
805805
}
806806
}
807807

0 commit comments

Comments
 (0)