Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Lint unnecessary int-to-int and float-to-float casts #6187

Merged
merged 6 commits into from
Oct 24, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
66 changes: 54 additions & 12 deletions clippy_lints/src/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ use rustc_hir as hir;
use rustc_hir::intravisit::{walk_body, walk_expr, walk_ty, FnKind, NestedVisitorMap, Visitor};
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,
ImplItemKind, Item, ItemKind, Lifetime, Lit, Local, MatchSource, MutTy, Mutability, Node, QPath, Stmt, StmtKind,
TraitFn, TraitItem, TraitItemKind, TyKind, UnOp,
};
use rustc_lint::{LateContext, LateLintPass, LintContext};
Expand Down Expand Up @@ -1224,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.
///
Expand All @@ -1233,6 +1234,14 @@ declare_clippy_lint! {
/// **Example:**
/// ```rust
/// let _ = 2i32 as i32;
/// let _ = 0.5 as f32;
ebroto marked this conversation as resolved.
Show resolved Hide resolved
/// ```
///
/// Better:
///
/// ```rust
/// let _ = 2_i32;
/// let _ = 0.5_f32;
/// ```
pub UNNECESSARY_CAST,
complexity,
Expand Down Expand Up @@ -1598,7 +1607,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) {
ebroto marked this conversation as resolved.
Show resolved Hide resolved
let literal_str = snippet_opt(cx, ex.span).unwrap_or_default();

if_chain! {
if let LitKind::Int(n, _) = lit.node;
if let Some(src) = snippet_opt(cx, lit.span);
Expand All @@ -1608,19 +1619,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 {
span_lint_and_sugg(
cx,
UNNECESSARY_CAST,
expr.span,
&format!("casting integer literal to `{}` is unnecessary", cast_to),
"try",
format!("{}_{}", n, cast_to),
Applicability::MachineApplicable,
);
let literal_str = if is_unary_neg(ex) { format!("-{}", num_lit.integer) } else { num_lit.integer.into() };
show_unnecessary_cast(cx, expr, &literal_str, cast_from, cast_to);
return;
}
}

match lit.node {
ebroto marked this conversation as resolved.
Show resolved Hide resolved
LitKind::Int(_, LitIntType::Unsuffixed) if cast_to.is_integral() => {
show_unnecessary_cast(cx, expr, &literal_str, cast_from, cast_to);
},
LitKind::Float(_, LitFloatType::Unsuffixed) if cast_to.is_floating_point() => {
show_unnecessary_cast(cx, expr, &literal_str, cast_from, cast_to);
},
LitKind::Int(_, LitIntType::Unsuffixed) | LitKind::Float(_, LitFloatType::Unsuffixed) => {},
_ => {
if cast_from.kind() == cast_to.kind() && !in_external_macro(cx.sess(), expr.span) {
Expand All @@ -1646,6 +1657,37 @@ impl<'tcx> LateLintPass<'tcx> for Casts {
}
}

fn is_unary_neg(expr: &Expr<'_>) -> bool {
matches!(expr.kind, ExprKind::Unary(UnOp::UnNeg, _))
}

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<'_>, literal_str: &str, cast_from: Ty<'_>, cast_to: Ty<'_>) {
let literal_kind_name = if cast_from.is_integral() { "integer" } else { "float" };
span_lint_and_sugg(
cx,
UNNECESSARY_CAST,
expr.span,
&format!("casting {} literal to `{}` is unnecessary", literal_kind_name, cast_to),
"try",
format!("{}_{}", literal_str, cast_to),
Applicability::MachineApplicable,
);
}

fn lint_numeric_casts<'tcx>(
cx: &LateContext<'tcx>,
expr: &Expr<'tcx>,
Expand Down
2 changes: 1 addition & 1 deletion tests/ui/cast_size_32bit.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ error: casting integer literal to `f64` is unnecessary
--> $DIR/cast_size_32bit.rs:34:5
|
LL | 3_999_999_999usize as f64;
| ^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `3999999999_f64`
| ^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `3_999_999_999_f64`
|
= note: `-D clippy::unnecessary-cast` implied by `-D warnings`

Expand Down
1 change: 1 addition & 0 deletions tests/ui/eq_op.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
#[allow(clippy::no_effect, unused_variables, clippy::unnecessary_operation, clippy::short_circuit_statement)]
#[allow(clippy::nonminimal_bool)]
#[allow(unused)]
#[allow(clippy::unnecessary_cast)]
fn main() {
// simple values and comparisons
1 == 1;
Expand Down
54 changes: 27 additions & 27 deletions tests/ui/eq_op.stderr
Original file line number Diff line number Diff line change
@@ -1,163 +1,163 @@
error: equal expressions as operands to `==`
--> $DIR/eq_op.rs:11:5
--> $DIR/eq_op.rs:12:5
|
LL | 1 == 1;
| ^^^^^^
|
= note: `-D clippy::eq-op` implied by `-D warnings`

error: equal expressions as operands to `==`
--> $DIR/eq_op.rs:12:5
--> $DIR/eq_op.rs:13:5
|
LL | "no" == "no";
| ^^^^^^^^^^^^

error: equal expressions as operands to `!=`
--> $DIR/eq_op.rs:14:5
--> $DIR/eq_op.rs:15:5
|
LL | false != false;
| ^^^^^^^^^^^^^^

error: equal expressions as operands to `<`
--> $DIR/eq_op.rs:15:5
--> $DIR/eq_op.rs:16:5
|
LL | 1.5 < 1.5;
| ^^^^^^^^^

error: equal expressions as operands to `>=`
--> $DIR/eq_op.rs:16:5
--> $DIR/eq_op.rs:17:5
|
LL | 1u64 >= 1u64;
| ^^^^^^^^^^^^

error: equal expressions as operands to `&`
--> $DIR/eq_op.rs:19:5
--> $DIR/eq_op.rs:20:5
|
LL | (1 as u64) & (1 as u64);
| ^^^^^^^^^^^^^^^^^^^^^^^

error: equal expressions as operands to `^`
--> $DIR/eq_op.rs:20:5
--> $DIR/eq_op.rs:21:5
|
LL | 1 ^ ((((((1))))));
| ^^^^^^^^^^^^^^^^^

error: equal expressions as operands to `<`
--> $DIR/eq_op.rs:23:5
--> $DIR/eq_op.rs:24:5
|
LL | (-(2) < -(2));
| ^^^^^^^^^^^^^

error: equal expressions as operands to `==`
--> $DIR/eq_op.rs:24:5
--> $DIR/eq_op.rs:25:5
|
LL | ((1 + 1) & (1 + 1) == (1 + 1) & (1 + 1));
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error: equal expressions as operands to `&`
--> $DIR/eq_op.rs:24:6
--> $DIR/eq_op.rs:25:6
|
LL | ((1 + 1) & (1 + 1) == (1 + 1) & (1 + 1));
| ^^^^^^^^^^^^^^^^^

error: equal expressions as operands to `&`
--> $DIR/eq_op.rs:24:27
--> $DIR/eq_op.rs:25:27
|
LL | ((1 + 1) & (1 + 1) == (1 + 1) & (1 + 1));
| ^^^^^^^^^^^^^^^^^

error: equal expressions as operands to `==`
--> $DIR/eq_op.rs:25:5
--> $DIR/eq_op.rs:26:5
|
LL | (1 * 2) + (3 * 4) == 1 * 2 + 3 * 4;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error: equal expressions as operands to `!=`
--> $DIR/eq_op.rs:28:5
--> $DIR/eq_op.rs:29:5
|
LL | ([1] != [1]);
| ^^^^^^^^^^^^

error: equal expressions as operands to `!=`
--> $DIR/eq_op.rs:29:5
--> $DIR/eq_op.rs:30:5
|
LL | ((1, 2) != (1, 2));
| ^^^^^^^^^^^^^^^^^^

error: equal expressions as operands to `==`
--> $DIR/eq_op.rs:33:5
--> $DIR/eq_op.rs:34:5
|
LL | 1 + 1 == 2;
| ^^^^^^^^^^

error: equal expressions as operands to `==`
--> $DIR/eq_op.rs:34:5
--> $DIR/eq_op.rs:35:5
|
LL | 1 - 1 == 0;
| ^^^^^^^^^^

error: equal expressions as operands to `-`
--> $DIR/eq_op.rs:34:5
--> $DIR/eq_op.rs:35:5
|
LL | 1 - 1 == 0;
| ^^^^^

error: equal expressions as operands to `-`
--> $DIR/eq_op.rs:36:5
--> $DIR/eq_op.rs:37:5
|
LL | 1 - 1;
| ^^^^^

error: equal expressions as operands to `/`
--> $DIR/eq_op.rs:37:5
--> $DIR/eq_op.rs:38:5
|
LL | 1 / 1;
| ^^^^^

error: equal expressions as operands to `&&`
--> $DIR/eq_op.rs:38:5
--> $DIR/eq_op.rs:39:5
|
LL | true && true;
| ^^^^^^^^^^^^

error: equal expressions as operands to `||`
--> $DIR/eq_op.rs:40:5
--> $DIR/eq_op.rs:41:5
|
LL | true || true;
| ^^^^^^^^^^^^

error: equal expressions as operands to `&&`
--> $DIR/eq_op.rs:46:5
--> $DIR/eq_op.rs:47:5
|
LL | a == b && b == a;
| ^^^^^^^^^^^^^^^^

error: equal expressions as operands to `&&`
--> $DIR/eq_op.rs:47:5
--> $DIR/eq_op.rs:48:5
|
LL | a != b && b != a;
| ^^^^^^^^^^^^^^^^

error: equal expressions as operands to `&&`
--> $DIR/eq_op.rs:48:5
--> $DIR/eq_op.rs:49:5
|
LL | a < b && b > a;
| ^^^^^^^^^^^^^^

error: equal expressions as operands to `&&`
--> $DIR/eq_op.rs:49:5
--> $DIR/eq_op.rs:50:5
|
LL | a <= b && b >= a;
| ^^^^^^^^^^^^^^^^

error: equal expressions as operands to `==`
--> $DIR/eq_op.rs:52:5
--> $DIR/eq_op.rs:53:5
|
LL | a == a;
| ^^^^^^

error: equal expressions as operands to `/`
--> $DIR/eq_op.rs:62:20
--> $DIR/eq_op.rs:63:20
|
LL | const D: u32 = A / A;
| ^^^^^
Expand Down
19 changes: 17 additions & 2 deletions tests/ui/unnecessary_cast_fixable.fixed
Original file line number Diff line number Diff line change
Expand Up @@ -8,16 +8,31 @@ fn main() {
100_f32;
100_f64;
100_f64;
let _ = -100_f32;
let _ = -100_f64;
let _ = -100_f64;
// Should not trigger
#[rustfmt::skip]
let v = vec!(1);
&v as &[i32];
1.0 as f64;
1 as u64;
0x10 as f32;
0o10 as f32;
0b10 as f32;
0x11 as f64;
0o11 as f64;
0b11 as f64;

1_u32;
0x10_i32;
0b10_usize;
0o73_u16;
1_000_000_000_u32;

1.0_f64;
0.5_f32;

1.0 as u16;

let _ = -1_i32;
let _ = -1.0_f32;
}
19 changes: 17 additions & 2 deletions tests/ui/unnecessary_cast_fixable.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,16 +8,31 @@ fn main() {
100 as f32;
100 as f64;
100_i32 as f64;
let _ = -100 as f32;
let _ = -100 as f64;
let _ = -100_i32 as f64;
// Should not trigger
#[rustfmt::skip]
let v = vec!(1);
&v as &[i32];
1.0 as f64;
1 as u64;
0x10 as f32;
0o10 as f32;
0b10 as f32;
0x11 as f64;
0o11 as f64;
0b11 as f64;

1 as u32;
0x10 as i32;
0b10 as usize;
0o73 as u16;
1_000_000_000 as u32;

ebroto marked this conversation as resolved.
Show resolved Hide resolved
ebroto marked this conversation as resolved.
Show resolved Hide resolved
1.0 as f64;
0.5 as f32;

1.0 as u16;

let _ = -1 as i32;
let _ = -1.0 as f32;
}
Loading