From cfdef27b5aab79f62ccae9b48be53a39f5161753 Mon Sep 17 00:00:00 2001 From: Geoffrey Copin Date: Thu, 22 Oct 2020 23:53:50 +0200 Subject: [PATCH] Address review comments --- clippy_lints/src/types.rs | 51 ++++++++++++++---------- tests/ui/unnecessary_cast_fixable.fixed | 6 ++- tests/ui/unnecessary_cast_fixable.rs | 2 + tests/ui/unnecessary_cast_fixable.stderr | 22 +++++++--- 4 files changed, 54 insertions(+), 27 deletions(-) diff --git a/clippy_lints/src/types.rs b/clippy_lints/src/types.rs index 716d027e434d..f4c72264a47a 100644 --- a/clippy_lints/src/types.rs +++ b/clippy_lints/src/types.rs @@ -3,7 +3,6 @@ use std::borrow::Cow; use std::cmp::Ordering; use std::collections::BTreeMap; -use std::fmt::Display; use if_chain::if_chain; use rustc_ast::{FloatTy, IntTy, LitFloatType, LitIntType, LitKind, UintTy}; @@ -13,7 +12,7 @@ use rustc_hir::intravisit::{walk_body, walk_expr, walk_ty, FnKind, NestedVisitor use rustc_hir::{ BinOpKind, Block, Body, Expr, ExprKind, FnDecl, FnRetTy, FnSig, GenericArg, GenericParamKind, HirId, ImplItem, ImplItemKind, Item, ItemKind, Lifetime, Local, MatchSource, MutTy, Mutability, Node, QPath, Stmt, StmtKind, - TraitFn, TraitItem, TraitItemKind, TyKind, UnOp, + TraitFn, TraitItem, TraitItemKind, TyKind, UnOp, Lit, }; use rustc_lint::{LateContext, LateLintPass, LintContext}; use rustc_middle::hir::map::Map; @@ -1225,7 +1224,8 @@ declare_clippy_lint! { } declare_clippy_lint! { - /// **What it does:** Checks for casts to the same type. + /// **What it does:** Checks for casts to the same type, casts of int literals to integer types + /// and casts of float literals to float types. /// /// **Why is this bad?** It's just unnecessary. /// @@ -1234,6 +1234,7 @@ declare_clippy_lint! { /// **Example:** /// ```rust /// let _ = 2i32 as i32; + /// let _ = 0.5 as f32; /// ``` pub UNNECESSARY_CAST, complexity, @@ -1599,7 +1600,9 @@ impl<'tcx> LateLintPass<'tcx> for Casts { if let ExprKind::Cast(ref ex, _) = expr.kind { let (cast_from, cast_to) = (cx.typeck_results().expr_ty(ex), cx.typeck_results().expr_ty(expr)); lint_fn_to_numeric_cast(cx, expr, ex, cast_from, cast_to); - if let ExprKind::Lit(ref lit) = ex.kind { + if let Some(lit) = get_numeric_literal(ex) { + let literal_str = snippet_opt(cx, lit.span).unwrap_or_default(); + if_chain! { if let LitKind::Int(n, _) = lit.node; if let Some(src) = snippet_opt(cx, lit.span); @@ -1609,25 +1612,19 @@ impl<'tcx> LateLintPass<'tcx> for Casts { let to_nbits = fp_ty_mantissa_nbits(cast_to); if from_nbits != 0 && to_nbits != 0 && from_nbits <= to_nbits && num_lit.is_decimal(); then { - show_unnecessary_cast(cx, expr, n , cast_from, cast_to); + show_unnecessary_cast(cx, expr, num_lit.integer, cast_from, cast_to); return; } } match lit.node { - LitKind::Int(num, LitIntType::Unsuffixed) if cast_to.is_integral() => { - show_unnecessary_cast(cx, expr, num, cast_from, cast_to); - return; + LitKind::Int(_, LitIntType::Unsuffixed) if cast_to.is_integral() => { + show_unnecessary_cast(cx, expr, &literal_str, cast_from, cast_to); }, - LitKind::Float(num, LitFloatType::Unsuffixed) if cast_to.is_floating_point() => { - show_unnecessary_cast(cx, expr, num, cast_from, cast_to); - return; + LitKind::Float(_, LitFloatType::Unsuffixed) if cast_to.is_floating_point() => { + show_unnecessary_cast(cx, expr, &literal_str, cast_from, cast_to); }, - _ => (), - }; - - match lit.node { - LitKind::Int(_, LitIntType::Unsuffixed) | LitKind::Float(_, LitFloatType::Unsuffixed) => {}, + LitKind::Int(_, LitIntType::Unsuffixed) | LitKind::Float(_, LitFloatType::Unsuffixed) => (), _ => { if cast_from.kind() == cast_to.kind() && !in_external_macro(cx.sess(), expr.span) { span_lint( @@ -1640,7 +1637,7 @@ impl<'tcx> LateLintPass<'tcx> for Casts { ), ); } - }, + } } } if cast_from.is_numeric() && cast_to.is_numeric() && !in_external_macro(cx.sess(), expr.span) { @@ -1652,10 +1649,24 @@ impl<'tcx> LateLintPass<'tcx> for Casts { } } -fn show_unnecessary_cast( +fn get_numeric_literal<'e>(expr: &'e Expr<'e>) -> Option<&'e Lit> { + match expr.kind { + ExprKind::Lit(ref lit) => Some(lit), + ExprKind::Unary(UnOp::UnNeg, e) => { + if let ExprKind::Lit(ref lit) = e.kind { + Some(lit) + } else { + None + } + }, + _ => None, + } +} + +fn show_unnecessary_cast( cx: &LateContext<'_>, expr: &Expr<'_>, - num: Num, + literal_str: &str, cast_from: Ty<'_>, cast_to: Ty<'_>, ) { @@ -1666,7 +1677,7 @@ fn show_unnecessary_cast( expr.span, &format!("casting {} literal to `{}` is unnecessary", literal_kind_name, cast_to), "try", - format!("{}_{}", num, cast_to), + format!("{}_{}", literal_str, cast_to), Applicability::MachineApplicable, ); } diff --git a/tests/ui/unnecessary_cast_fixable.fixed b/tests/ui/unnecessary_cast_fixable.fixed index ba52fc2703f2..54853f4b8a26 100644 --- a/tests/ui/unnecessary_cast_fixable.fixed +++ b/tests/ui/unnecessary_cast_fixable.fixed @@ -20,8 +20,10 @@ fn main() { 0b11 as f64; 1_u32; - 16_i32; - 2_usize; + 0x10_i32; + 0b10_usize; + 0o73_u16; + 1_000_000_000_u32; 1.0_f64; 0.5_f32; diff --git a/tests/ui/unnecessary_cast_fixable.rs b/tests/ui/unnecessary_cast_fixable.rs index 0d2115548fd2..8da3d9477024 100644 --- a/tests/ui/unnecessary_cast_fixable.rs +++ b/tests/ui/unnecessary_cast_fixable.rs @@ -22,6 +22,8 @@ fn main() { 1 as u32; 0x10 as i32; 0b10 as usize; + 0o73 as u16; + 1_000_000_000 as u32; 1.0 as f64; 0.5 as f32; diff --git a/tests/ui/unnecessary_cast_fixable.stderr b/tests/ui/unnecessary_cast_fixable.stderr index 474e62c30d50..28fb9540afc0 100644 --- a/tests/ui/unnecessary_cast_fixable.stderr +++ b/tests/ui/unnecessary_cast_fixable.stderr @@ -28,25 +28,37 @@ error: casting integer literal to `i32` is unnecessary --> $DIR/unnecessary_cast_fixable.rs:23:5 | LL | 0x10 as i32; - | ^^^^^^^^^^^ help: try: `16_i32` + | ^^^^^^^^^^^ help: try: `0x10_i32` error: casting integer literal to `usize` is unnecessary --> $DIR/unnecessary_cast_fixable.rs:24:5 | LL | 0b10 as usize; - | ^^^^^^^^^^^^^ help: try: `2_usize` + | ^^^^^^^^^^^^^ help: try: `0b10_usize` -error: casting float literal to `f64` is unnecessary +error: casting integer literal to `u16` is unnecessary + --> $DIR/unnecessary_cast_fixable.rs:25:5 + | +LL | 0o73 as u16; + | ^^^^^^^^^^^ help: try: `0o73_u16` + +error: casting integer literal to `u32` is unnecessary --> $DIR/unnecessary_cast_fixable.rs:26:5 | +LL | 1_000_000_000 as u32; + | ^^^^^^^^^^^^^^^^^^^^ help: try: `1_000_000_000_u32` + +error: casting float literal to `f64` is unnecessary + --> $DIR/unnecessary_cast_fixable.rs:28:5 + | LL | 1.0 as f64; | ^^^^^^^^^^ help: try: `1.0_f64` error: casting float literal to `f32` is unnecessary - --> $DIR/unnecessary_cast_fixable.rs:27:5 + --> $DIR/unnecessary_cast_fixable.rs:29:5 | LL | 0.5 as f32; | ^^^^^^^^^^ help: try: `0.5_f32` -error: aborting due to 8 previous errors +error: aborting due to 10 previous errors