Skip to content

Commit

Permalink
Auto merge of #10038 - xFrednet:9231-sugg-try-from, r=Jarcho
Browse files Browse the repository at this point in the history
`cast_possible_truncation` Suggest TryFrom when truncation possible

This fixes the last issues from #9664 as the author seems to be inactive. The PR author was sadly kept during the rebase, due to the conflict resolution.

IDK if it's worth it do to a full review, I only added the last commit, everything else remained the same, besides a rebase.

---

changelog: Sugg: [`cast_possible_truncation`]: Now suggests using `try_from` or allowing the lint
[#10038](#10038)
<!-- changelog_checked -->

closes: #9231
  • Loading branch information
bors committed Jan 14, 2023
2 parents 91c8ecc + 5cb6246 commit 483b7ac
Show file tree
Hide file tree
Showing 5 changed files with 244 additions and 25 deletions.
32 changes: 28 additions & 4 deletions clippy_lints/src/casts/cast_possible_truncation.rs
Original file line number Diff line number Diff line change
@@ -1,11 +1,14 @@
use clippy_utils::consts::{constant, Constant};
use clippy_utils::diagnostics::span_lint;
use clippy_utils::diagnostics::{span_lint, span_lint_and_then};
use clippy_utils::expr_or_init;
use clippy_utils::source::snippet;
use clippy_utils::ty::{get_discriminant_value, is_isize_or_usize};
use rustc_errors::{Applicability, SuggestionStyle};
use rustc_hir::def::{DefKind, Res};
use rustc_hir::{BinOpKind, Expr, ExprKind};
use rustc_lint::LateContext;
use rustc_middle::ty::{self, FloatTy, Ty};
use rustc_span::Span;
use rustc_target::abi::IntegerType;

use super::{utils, CAST_ENUM_TRUNCATION, CAST_POSSIBLE_TRUNCATION};
Expand Down Expand Up @@ -74,7 +77,14 @@ fn apply_reductions(cx: &LateContext<'_>, nbits: u64, expr: &Expr<'_>, signed: b
}
}

pub(super) fn check(cx: &LateContext<'_>, expr: &Expr<'_>, cast_expr: &Expr<'_>, cast_from: Ty<'_>, cast_to: Ty<'_>) {
pub(super) fn check(
cx: &LateContext<'_>,
expr: &Expr<'_>,
cast_expr: &Expr<'_>,
cast_from: Ty<'_>,
cast_to: Ty<'_>,
cast_to_span: Span,
) {
let msg = match (cast_from.kind(), cast_to.is_integral()) {
(ty::Int(_) | ty::Uint(_), true) => {
let from_nbits = apply_reductions(
Expand Down Expand Up @@ -139,7 +149,7 @@ pub(super) fn check(cx: &LateContext<'_>, expr: &Expr<'_>, cast_expr: &Expr<'_>,
);
return;
}
format!("casting `{cast_from}` to `{cast_to}` may truncate the value{suffix}",)
format!("casting `{cast_from}` to `{cast_to}` may truncate the value{suffix}")
},

(ty::Float(_), true) => {
Expand All @@ -153,5 +163,19 @@ pub(super) fn check(cx: &LateContext<'_>, expr: &Expr<'_>, cast_expr: &Expr<'_>,
_ => return,
};

span_lint(cx, CAST_POSSIBLE_TRUNCATION, expr.span, &msg);
let name_of_cast_from = snippet(cx, cast_expr.span, "..");
let cast_to_snip = snippet(cx, cast_to_span, "..");
let suggestion = format!("{cast_to_snip}::try_from({name_of_cast_from})");

span_lint_and_then(cx, CAST_POSSIBLE_TRUNCATION, expr.span, &msg, |diag| {
diag.help("if this is intentional allow the lint with `#[allow(clippy::cast_precision_loss)]` ...");
diag.span_suggestion_with_style(
expr.span,
"... or use `try_from` and handle the error accordingly",
suggestion,
Applicability::Unspecified,
// always show the suggestion in a separate line
SuggestionStyle::ShowAlways,
);
});
}
20 changes: 18 additions & 2 deletions clippy_lints/src/casts/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,8 @@ declare_clippy_lint! {
/// ### What it does
/// Checks for casts between numerical types that may
/// truncate large values. This is expected behavior, so the cast is `Allow` by
/// default.
/// default. It suggests user either explicitly ignore the lint,
/// or use `try_from()` and handle the truncation, default, or panic explicitly.
///
/// ### Why is this bad?
/// In some problem domains, it is good practice to avoid
Expand All @@ -93,6 +94,21 @@ declare_clippy_lint! {
/// x as u8
/// }
/// ```
/// Use instead:
/// ```
/// fn as_u8(x: u64) -> u8 {
/// if let Ok(x) = u8::try_from(x) {
/// x
/// } else {
/// todo!();
/// }
/// }
/// // Or
/// #[allow(clippy::cast_possible_truncation)]
/// fn as_u16(x: u64) -> u16 {
/// x as u16
/// }
/// ```
#[clippy::version = "pre 1.29.0"]
pub CAST_POSSIBLE_TRUNCATION,
pedantic,
Expand Down Expand Up @@ -712,7 +728,7 @@ impl<'tcx> LateLintPass<'tcx> for Casts {
fn_to_numeric_cast_with_truncation::check(cx, expr, cast_expr, cast_from, cast_to);

if cast_to.is_numeric() && !in_external_macro(cx.sess(), expr.span) {
cast_possible_truncation::check(cx, expr, cast_expr, cast_from, cast_to);
cast_possible_truncation::check(cx, expr, cast_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);
Expand Down
1 change: 1 addition & 0 deletions tests/ui/cast.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ fn main() {
1i32 as u8;
1f64 as isize;
1f64 as usize;
1f32 as u32 as u16;
// Test clippy::cast_possible_wrap
1u8 as i8;
1u16 as i16;
Expand Down
Loading

0 comments on commit 483b7ac

Please sign in to comment.