Address review comments

This commit is contained in:
Geoffrey Copin 2020-10-22 23:53:50 +02:00
parent bb0ce32423
commit e8f12d2f02
4 changed files with 52 additions and 31 deletions

View file

@ -3,7 +3,6 @@
use std::borrow::Cow; use std::borrow::Cow;
use std::cmp::Ordering; use std::cmp::Ordering;
use std::collections::BTreeMap; use std::collections::BTreeMap;
use std::fmt::Display;
use if_chain::if_chain; use if_chain::if_chain;
use rustc_ast::{FloatTy, IntTy, LitFloatType, LitIntType, LitKind, UintTy}; use rustc_ast::{FloatTy, IntTy, LitFloatType, LitIntType, LitKind, UintTy};
@ -12,7 +11,7 @@ use rustc_hir as hir;
use rustc_hir::intravisit::{walk_body, walk_expr, walk_ty, FnKind, NestedVisitorMap, Visitor}; use rustc_hir::intravisit::{walk_body, walk_expr, walk_ty, FnKind, NestedVisitorMap, Visitor};
use rustc_hir::{ use rustc_hir::{
BinOpKind, Block, Body, Expr, ExprKind, FnDecl, FnRetTy, FnSig, GenericArg, GenericParamKind, HirId, ImplItem, BinOpKind, Block, Body, Expr, ExprKind, FnDecl, FnRetTy, FnSig, GenericArg, GenericParamKind, HirId, ImplItem,
ImplItemKind, Item, ItemKind, Lifetime, Local, MatchSource, MutTy, Mutability, Node, QPath, Stmt, StmtKind, ImplItemKind, Item, ItemKind, Lifetime, Lit, Local, MatchSource, MutTy, Mutability, Node, QPath, Stmt, StmtKind,
TraitFn, TraitItem, TraitItemKind, TyKind, UnOp, TraitFn, TraitItem, TraitItemKind, TyKind, UnOp,
}; };
use rustc_lint::{LateContext, LateLintPass, LintContext}; use rustc_lint::{LateContext, LateLintPass, LintContext};
@ -1225,7 +1224,8 @@ declare_clippy_lint! {
} }
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. /// **Why is this bad?** It's just unnecessary.
/// ///
@ -1234,6 +1234,7 @@ declare_clippy_lint! {
/// **Example:** /// **Example:**
/// ```rust /// ```rust
/// let _ = 2i32 as i32; /// let _ = 2i32 as i32;
/// let _ = 0.5 as f32;
/// ``` /// ```
pub UNNECESSARY_CAST, pub UNNECESSARY_CAST,
complexity, complexity,
@ -1599,7 +1600,9 @@ impl<'tcx> LateLintPass<'tcx> for Casts {
if let ExprKind::Cast(ref ex, _) = expr.kind { 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)); 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); 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_chain! {
if let LitKind::Int(n, _) = lit.node; if let LitKind::Int(n, _) = lit.node;
if let Some(src) = snippet_opt(cx, lit.span); 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); let to_nbits = fp_ty_mantissa_nbits(cast_to);
if from_nbits != 0 && to_nbits != 0 && from_nbits <= to_nbits && num_lit.is_decimal(); if from_nbits != 0 && to_nbits != 0 && from_nbits <= to_nbits && num_lit.is_decimal();
then { then {
show_unnecessary_cast(cx, expr, n , cast_from, cast_to); show_unnecessary_cast(cx, expr, num_lit.integer, cast_from, cast_to);
return; return;
} }
} }
match lit.node { match lit.node {
LitKind::Int(num, LitIntType::Unsuffixed) if cast_to.is_integral() => { LitKind::Int(_, LitIntType::Unsuffixed) if cast_to.is_integral() => {
show_unnecessary_cast(cx, expr, num, cast_from, cast_to); show_unnecessary_cast(cx, expr, &literal_str, cast_from, cast_to);
return;
}, },
LitKind::Float(num, LitFloatType::Unsuffixed) if cast_to.is_floating_point() => { LitKind::Float(_, LitFloatType::Unsuffixed) if cast_to.is_floating_point() => {
show_unnecessary_cast(cx, expr, num, cast_from, cast_to); show_unnecessary_cast(cx, expr, &literal_str, cast_from, cast_to);
return;
}, },
_ => (), LitKind::Int(_, LitIntType::Unsuffixed) | LitKind::Float(_, LitFloatType::Unsuffixed) => (),
};
match lit.node {
LitKind::Int(_, LitIntType::Unsuffixed) | LitKind::Float(_, LitFloatType::Unsuffixed) => {},
_ => { _ => {
if cast_from.kind() == cast_to.kind() && !in_external_macro(cx.sess(), expr.span) { if cast_from.kind() == cast_to.kind() && !in_external_macro(cx.sess(), expr.span) {
span_lint( span_lint(
@ -1652,13 +1649,21 @@ impl<'tcx> LateLintPass<'tcx> for Casts {
} }
} }
fn show_unnecessary_cast<Num: Display>( fn get_numeric_literal<'e>(expr: &'e Expr<'e>) -> Option<&'e Lit> {
cx: &LateContext<'_>, match expr.kind {
expr: &Expr<'_>, ExprKind::Lit(ref lit) => Some(lit),
num: Num, ExprKind::Unary(UnOp::UnNeg, e) => {
cast_from: Ty<'_>, if let ExprKind::Lit(ref lit) = e.kind {
cast_to: Ty<'_>, Some(lit)
) { } else {
None
}
},
_ => None,
}
}
fn show_unnecessary_cast(cx: &LateContext<'_>, expr: &Expr<'_>, literal_str: &str, cast_from: Ty<'_>, cast_to: Ty<'_>) {
let literal_kind_name = if cast_from.is_integral() { "integer" } else { "float" }; let literal_kind_name = if cast_from.is_integral() { "integer" } else { "float" };
span_lint_and_sugg( span_lint_and_sugg(
cx, cx,
@ -1666,7 +1671,7 @@ fn show_unnecessary_cast<Num: Display>(
expr.span, expr.span,
&format!("casting {} literal to `{}` is unnecessary", literal_kind_name, cast_to), &format!("casting {} literal to `{}` is unnecessary", literal_kind_name, cast_to),
"try", "try",
format!("{}_{}", num, cast_to), format!("{}_{}", literal_str, cast_to),
Applicability::MachineApplicable, Applicability::MachineApplicable,
); );
} }

View file

@ -20,8 +20,10 @@ fn main() {
0b11 as f64; 0b11 as f64;
1_u32; 1_u32;
16_i32; 0x10_i32;
2_usize; 0b10_usize;
0o73_u16;
1_000_000_000_u32;
1.0_f64; 1.0_f64;
0.5_f32; 0.5_f32;

View file

@ -22,6 +22,8 @@ fn main() {
1 as u32; 1 as u32;
0x10 as i32; 0x10 as i32;
0b10 as usize; 0b10 as usize;
0o73 as u16;
1_000_000_000 as u32;
1.0 as f64; 1.0 as f64;
0.5 as f32; 0.5 as f32;

View file

@ -28,25 +28,37 @@ error: casting integer literal to `i32` is unnecessary
--> $DIR/unnecessary_cast_fixable.rs:23:5 --> $DIR/unnecessary_cast_fixable.rs:23:5
| |
LL | 0x10 as i32; LL | 0x10 as i32;
| ^^^^^^^^^^^ help: try: `16_i32` | ^^^^^^^^^^^ help: try: `0x10_i32`
error: casting integer literal to `usize` is unnecessary error: casting integer literal to `usize` is unnecessary
--> $DIR/unnecessary_cast_fixable.rs:24:5 --> $DIR/unnecessary_cast_fixable.rs:24:5
| |
LL | 0b10 as usize; LL | 0b10 as usize;
| ^^^^^^^^^^^^^ help: try: `2_usize` | ^^^^^^^^^^^^^ help: try: `0b10_usize`
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 error: casting float literal to `f64` is unnecessary
--> $DIR/unnecessary_cast_fixable.rs:26:5 --> $DIR/unnecessary_cast_fixable.rs:28:5
| |
LL | 1.0 as f64; LL | 1.0 as f64;
| ^^^^^^^^^^ help: try: `1.0_f64` | ^^^^^^^^^^ help: try: `1.0_f64`
error: casting float literal to `f32` is unnecessary 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; LL | 0.5 as f32;
| ^^^^^^^^^^ help: try: `0.5_f32` | ^^^^^^^^^^ help: try: `0.5_f32`
error: aborting due to 8 previous errors error: aborting due to 10 previous errors