From afce85e96dd8a2157b2362e71e420789e2305de7 Mon Sep 17 00:00:00 2001 From: Bood Qian Date: Wed, 15 Feb 2017 22:20:20 +0800 Subject: [PATCH 1/6] Add lint for unnecessary casts (cast to same type) --- CHANGELOG.md | 1 + README.md | 3 ++- clippy_lints/src/lib.rs | 1 + clippy_lints/src/types.rs | 25 ++++++++++++++++++++++++- tests/ui/cast.rs | 11 +++++++++++ tests/ui/cast.stderr | 32 ++++++++++++++++++++++++++++++++ 6 files changed, 71 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7ca1336afaf8..a32224c1de1f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -293,6 +293,7 @@ All notable changes to this project will be documented in this file. [`cast_possible_wrap`]: https://github.com/Manishearth/rust-clippy/wiki#cast_possible_wrap [`cast_precision_loss`]: https://github.com/Manishearth/rust-clippy/wiki#cast_precision_loss [`cast_sign_loss`]: https://github.com/Manishearth/rust-clippy/wiki#cast_sign_loss +[`cast_unnecessary`]: https://github.com/Manishearth/rust-clippy/wiki#cast_unnecessary [`char_lit_as_u8`]: https://github.com/Manishearth/rust-clippy/wiki#char_lit_as_u8 [`chars_next_cmp`]: https://github.com/Manishearth/rust-clippy/wiki#chars_next_cmp [`clone_double_ref`]: https://github.com/Manishearth/rust-clippy/wiki#clone_double_ref diff --git a/README.md b/README.md index 0a6a62e91852..a402b87f079c 100644 --- a/README.md +++ b/README.md @@ -180,7 +180,7 @@ transparently: ## Lints -There are 189 lints included in this crate: +There are 190 lints included in this crate: name | default | triggers on -----------------------------------------------------------------------------------------------------------------------|---------|---------------------------------------------------------------------------------------------------------------------------------- @@ -201,6 +201,7 @@ name [cast_possible_wrap](https://github.com/Manishearth/rust-clippy/wiki#cast_possible_wrap) | allow | casts that may cause wrapping around the value, e.g `x as i32` where `x: u32` and `x > i32::MAX` [cast_precision_loss](https://github.com/Manishearth/rust-clippy/wiki#cast_precision_loss) | allow | casts that cause loss of precision, e.g `x as f32` where `x: u64` [cast_sign_loss](https://github.com/Manishearth/rust-clippy/wiki#cast_sign_loss) | allow | casts from signed types to unsigned types, e.g `x as u32` where `x: i32` +[cast_unnecessary](https://github.com/Manishearth/rust-clippy/wiki#cast_unnecessary) | warn | cast to the same type, e.g `x as i32` where `x: i32` [char_lit_as_u8](https://github.com/Manishearth/rust-clippy/wiki#char_lit_as_u8) | warn | casting a character literal to u8 [chars_next_cmp](https://github.com/Manishearth/rust-clippy/wiki#chars_next_cmp) | warn | using `.chars().next()` to check if a string starts with a char [clone_double_ref](https://github.com/Manishearth/rust-clippy/wiki#clone_double_ref) | warn | using `clone` on `&&T` diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 80a369513e2c..93eb449ebe06 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -488,6 +488,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry) { transmute::WRONG_TRANSMUTE, types::ABSURD_EXTREME_COMPARISONS, types::BOX_VEC, + types::CAST_UNNECESSARY, types::CHAR_LIT_AS_U8, types::LET_UNIT_VALUE, types::LINKEDLIST, diff --git a/clippy_lints/src/types.rs b/clippy_lints/src/types.rs index b1581127c325..161b2a1226cd 100644 --- a/clippy_lints/src/types.rs +++ b/clippy_lints/src/types.rs @@ -379,6 +379,22 @@ declare_lint! { and `x > i32::MAX`" } +/// **What it does:** Checks for casts to the same type +/// +/// **Why is this bad?** It's just unnecessary +/// +/// **Known problems:** None. +/// +/// **Example:** +/// ```rust +/// let _ = 2i32 as i32 +/// ``` +declare_lint! { + pub CAST_UNNECESSARY, + Warn, + "cast to the same type, e.g `x as i32` where `x: i32`" +} + /// Returns the size in bits of an integral type. /// Will return 0 if the type is not an int or uint variant fn int_ty_to_nbits(typ: &ty::TyS) -> usize { @@ -503,7 +519,8 @@ impl LintPass for CastPass { lint_array!(CAST_PRECISION_LOSS, CAST_SIGN_LOSS, CAST_POSSIBLE_TRUNCATION, - CAST_POSSIBLE_WRAP) + CAST_POSSIBLE_WRAP, + CAST_UNNECESSARY) } } @@ -511,6 +528,12 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for CastPass { fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr) { if let ExprCast(ref ex, _) = expr.node { let (cast_from, cast_to) = (cx.tables.expr_ty(ex), cx.tables.expr_ty(expr)); + if cast_from.sty == cast_to.sty && !in_external_macro(cx, expr.span) { + span_lint(cx, + CAST_UNNECESSARY, + expr.span, + &format!("casting to the same type is unnecessary ({} -> {})", cast_from, cast_to)); + } if cast_from.is_numeric() && cast_to.is_numeric() && !in_external_macro(cx, expr.span) { match (cast_from.is_integral(), cast_to.is_integral()) { (true, false) => { diff --git a/tests/ui/cast.rs b/tests/ui/cast.rs index ca5106102e9c..7b64b50a1caf 100644 --- a/tests/ui/cast.rs +++ b/tests/ui/cast.rs @@ -61,4 +61,15 @@ fn main() { 1u32 as usize; // Should not trigger any lint 1i32 as isize; // Neither should this 1i32 as usize; + + // Test cast_unnecessary + 1i32 as i32; + 1f32 as f32; + false as bool; + &1i32 as &i32; + + 1i32 as i64; // Should not trigger + + let v = vec!(1); + &v as &[i32]; // Should not trigger } diff --git a/tests/ui/cast.stderr b/tests/ui/cast.stderr index f4d8ec21d8d9..289d1d0ac4ff 100644 --- a/tests/ui/cast.stderr +++ b/tests/ui/cast.stderr @@ -274,5 +274,37 @@ error: casting i32 to usize may lose the sign of the value 63 | 1i32 as usize; | ^^^^^^^^^^^^^ +warning: casting to the same type is unnecessary (i32 -> i32) + --> $DIR/cast.rs:66:5 + | +66 | 1i32 as i32; + | ^^^^^^^^^^^ + | + = note: #[warn(cast_unnecessary)] on by default + +warning: casting to the same type is unnecessary (f32 -> f32) + --> $DIR/cast.rs:67:5 + | +67 | 1f32 as f32; + | ^^^^^^^^^^^ + | + = note: #[warn(cast_unnecessary)] on by default + +warning: casting to the same type is unnecessary (bool -> bool) + --> $DIR/cast.rs:68:5 + | +68 | false as bool; + | ^^^^^^^^^^^^^ + | + = note: #[warn(cast_unnecessary)] on by default + +warning: casting to the same type is unnecessary (&i32 -> &i32) + --> $DIR/cast.rs:69:5 + | +69 | &1i32 as &i32; + | ^^^^^^^^^^^^^ + | + = note: #[warn(cast_unnecessary)] on by default + error: aborting due to 42 previous errors From 2f00ea3a0727a33b39ee0246accb6a941f0f804d Mon Sep 17 00:00:00 2001 From: Bood Qian Date: Thu, 16 Feb 2017 22:55:41 +0800 Subject: [PATCH 2/6] Suppress lint for unsuffixed number casts --- CHANGELOG.md | 2 +- README.md | 2 +- clippy_lints/src/lib.rs | 2 +- clippy_lints/src/types.rs | 27 ++++++++++++++++++--------- tests/ui/cast.rs | 8 +++++--- tests/ui/cast.stderr | 20 ++++++-------------- 6 files changed, 32 insertions(+), 29 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a32224c1de1f..e5f7fe4aa9d2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -293,7 +293,6 @@ All notable changes to this project will be documented in this file. [`cast_possible_wrap`]: https://github.com/Manishearth/rust-clippy/wiki#cast_possible_wrap [`cast_precision_loss`]: https://github.com/Manishearth/rust-clippy/wiki#cast_precision_loss [`cast_sign_loss`]: https://github.com/Manishearth/rust-clippy/wiki#cast_sign_loss -[`cast_unnecessary`]: https://github.com/Manishearth/rust-clippy/wiki#cast_unnecessary [`char_lit_as_u8`]: https://github.com/Manishearth/rust-clippy/wiki#char_lit_as_u8 [`chars_next_cmp`]: https://github.com/Manishearth/rust-clippy/wiki#chars_next_cmp [`clone_double_ref`]: https://github.com/Manishearth/rust-clippy/wiki#clone_double_ref @@ -445,6 +444,7 @@ All notable changes to this project will be documented in this file. [`type_complexity`]: https://github.com/Manishearth/rust-clippy/wiki#type_complexity [`unicode_not_nfc`]: https://github.com/Manishearth/rust-clippy/wiki#unicode_not_nfc [`unit_cmp`]: https://github.com/Manishearth/rust-clippy/wiki#unit_cmp +[`unnecessary_cast`]: https://github.com/Manishearth/rust-clippy/wiki#unnecessary_cast [`unnecessary_mut_passed`]: https://github.com/Manishearth/rust-clippy/wiki#unnecessary_mut_passed [`unnecessary_operation`]: https://github.com/Manishearth/rust-clippy/wiki#unnecessary_operation [`unneeded_field_pattern`]: https://github.com/Manishearth/rust-clippy/wiki#unneeded_field_pattern diff --git a/README.md b/README.md index a402b87f079c..067368129e8b 100644 --- a/README.md +++ b/README.md @@ -201,7 +201,6 @@ name [cast_possible_wrap](https://github.com/Manishearth/rust-clippy/wiki#cast_possible_wrap) | allow | casts that may cause wrapping around the value, e.g `x as i32` where `x: u32` and `x > i32::MAX` [cast_precision_loss](https://github.com/Manishearth/rust-clippy/wiki#cast_precision_loss) | allow | casts that cause loss of precision, e.g `x as f32` where `x: u64` [cast_sign_loss](https://github.com/Manishearth/rust-clippy/wiki#cast_sign_loss) | allow | casts from signed types to unsigned types, e.g `x as u32` where `x: i32` -[cast_unnecessary](https://github.com/Manishearth/rust-clippy/wiki#cast_unnecessary) | warn | cast to the same type, e.g `x as i32` where `x: i32` [char_lit_as_u8](https://github.com/Manishearth/rust-clippy/wiki#char_lit_as_u8) | warn | casting a character literal to u8 [chars_next_cmp](https://github.com/Manishearth/rust-clippy/wiki#chars_next_cmp) | warn | using `.chars().next()` to check if a string starts with a char [clone_double_ref](https://github.com/Manishearth/rust-clippy/wiki#clone_double_ref) | warn | using `clone` on `&&T` @@ -350,6 +349,7 @@ name [type_complexity](https://github.com/Manishearth/rust-clippy/wiki#type_complexity) | warn | usage of very complex types that might be better factored into `type` definitions [unicode_not_nfc](https://github.com/Manishearth/rust-clippy/wiki#unicode_not_nfc) | allow | using a unicode literal not in NFC normal form (see [unicode tr15](http://www.unicode.org/reports/tr15/) for further information) [unit_cmp](https://github.com/Manishearth/rust-clippy/wiki#unit_cmp) | warn | comparing unit values +[unnecessary_cast](https://github.com/Manishearth/rust-clippy/wiki#unnecessary_cast) | warn | cast to the same type, e.g `x as i32` where `x: i32` [unnecessary_mut_passed](https://github.com/Manishearth/rust-clippy/wiki#unnecessary_mut_passed) | warn | an argument passed as a mutable reference although the callee only demands an immutable reference [unnecessary_operation](https://github.com/Manishearth/rust-clippy/wiki#unnecessary_operation) | warn | outer expressions with no effect [unneeded_field_pattern](https://github.com/Manishearth/rust-clippy/wiki#unneeded_field_pattern) | warn | struct fields bound to a wildcard instead of using `..` diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 93eb449ebe06..59a01d719188 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -488,12 +488,12 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry) { transmute::WRONG_TRANSMUTE, types::ABSURD_EXTREME_COMPARISONS, types::BOX_VEC, - types::CAST_UNNECESSARY, types::CHAR_LIT_AS_U8, types::LET_UNIT_VALUE, types::LINKEDLIST, types::TYPE_COMPLEXITY, types::UNIT_CMP, + types::UNNECESSARY_CAST, unicode::ZERO_WIDTH_SPACE, unsafe_removed_from_name::UNSAFE_REMOVED_FROM_NAME, unused_io_amount::UNUSED_IO_AMOUNT, diff --git a/clippy_lints/src/types.rs b/clippy_lints/src/types.rs index 161b2a1226cd..c8cd531d364d 100644 --- a/clippy_lints/src/types.rs +++ b/clippy_lints/src/types.rs @@ -379,9 +379,9 @@ declare_lint! { and `x > i32::MAX`" } -/// **What it does:** Checks for casts to the same type +/// **What it does:** Checks for casts to the same type. /// -/// **Why is this bad?** It's just unnecessary +/// **Why is this bad?** It's just unnecessary. /// /// **Known problems:** None. /// @@ -390,7 +390,7 @@ declare_lint! { /// let _ = 2i32 as i32 /// ``` declare_lint! { - pub CAST_UNNECESSARY, + pub UNNECESSARY_CAST, Warn, "cast to the same type, e.g `x as i32` where `x: i32`" } @@ -520,7 +520,7 @@ impl LintPass for CastPass { CAST_SIGN_LOSS, CAST_POSSIBLE_TRUNCATION, CAST_POSSIBLE_WRAP, - CAST_UNNECESSARY) + UNNECESSARY_CAST) } } @@ -528,11 +528,20 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for CastPass { fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr) { if let ExprCast(ref ex, _) = expr.node { let (cast_from, cast_to) = (cx.tables.expr_ty(ex), cx.tables.expr_ty(expr)); - if cast_from.sty == cast_to.sty && !in_external_macro(cx, expr.span) { - span_lint(cx, - CAST_UNNECESSARY, - expr.span, - &format!("casting to the same type is unnecessary ({} -> {})", cast_from, cast_to)); + if let ExprLit(ref lit) = ex.node { + use syntax::ast::{LitKind, LitIntType}; + match lit.node { + LitKind::Int(_, LitIntType::Unsuffixed) => (), + LitKind::FloatUnsuffixed(_) => (), + _ => { + if cast_from.sty == cast_to.sty && !in_external_macro(cx, expr.span) { + span_lint(cx, + UNNECESSARY_CAST, + expr.span, + &format!("casting to the same type is unnecessary (`{}` -> `{}`)", cast_from, cast_to)); + } + } + } } if cast_from.is_numeric() && cast_to.is_numeric() && !in_external_macro(cx, expr.span) { match (cast_from.is_integral(), cast_to.is_integral()) { diff --git a/tests/ui/cast.rs b/tests/ui/cast.rs index 7b64b50a1caf..d63e0b102787 100644 --- a/tests/ui/cast.rs +++ b/tests/ui/cast.rs @@ -68,8 +68,10 @@ fn main() { false as bool; &1i32 as &i32; - 1i32 as i64; // Should not trigger - + // Should not trigger + 1i32 as i64; let v = vec!(1); - &v as &[i32]; // Should not trigger + &v as &[i32]; + 1.0 as f64; + 1 as u64; } diff --git a/tests/ui/cast.stderr b/tests/ui/cast.stderr index 289d1d0ac4ff..7238daa2b411 100644 --- a/tests/ui/cast.stderr +++ b/tests/ui/cast.stderr @@ -274,37 +274,29 @@ error: casting i32 to usize may lose the sign of the value 63 | 1i32 as usize; | ^^^^^^^^^^^^^ -warning: casting to the same type is unnecessary (i32 -> i32) +warning: casting to the same type is unnecessary (`i32` -> `i32`) --> $DIR/cast.rs:66:5 | 66 | 1i32 as i32; | ^^^^^^^^^^^ | - = note: #[warn(cast_unnecessary)] on by default + = note: #[warn(unnecessary_cast)] on by default -warning: casting to the same type is unnecessary (f32 -> f32) +warning: casting to the same type is unnecessary (`f32` -> `f32`) --> $DIR/cast.rs:67:5 | 67 | 1f32 as f32; | ^^^^^^^^^^^ | - = note: #[warn(cast_unnecessary)] on by default + = note: #[warn(unnecessary_cast)] on by default -warning: casting to the same type is unnecessary (bool -> bool) +warning: casting to the same type is unnecessary (`bool` -> `bool`) --> $DIR/cast.rs:68:5 | 68 | false as bool; | ^^^^^^^^^^^^^ | - = note: #[warn(cast_unnecessary)] on by default - -warning: casting to the same type is unnecessary (&i32 -> &i32) - --> $DIR/cast.rs:69:5 - | -69 | &1i32 as &i32; - | ^^^^^^^^^^^^^ - | - = note: #[warn(cast_unnecessary)] on by default + = note: #[warn(unnecessary_cast)] on by default error: aborting due to 42 previous errors From 05a6945adc9047b36fa0f98b82ba64ac5b9dbeb8 Mon Sep 17 00:00:00 2001 From: Bood Qian Date: Fri, 17 Feb 2017 08:39:58 +0800 Subject: [PATCH 3/6] Rust fmt --- clippy_lints/src/types.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/clippy_lints/src/types.rs b/clippy_lints/src/types.rs index c8cd531d364d..ce5afb7eac84 100644 --- a/clippy_lints/src/types.rs +++ b/clippy_lints/src/types.rs @@ -538,9 +538,10 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for CastPass { span_lint(cx, UNNECESSARY_CAST, expr.span, - &format!("casting to the same type is unnecessary (`{}` -> `{}`)", cast_from, cast_to)); + &format!("casting to the same type is unnecessary (`{}` -> `{}`)", + cast_from, cast_to)); } - } + }, } } if cast_from.is_numeric() && cast_to.is_numeric() && !in_external_macro(cx, expr.span) { From b1f766b37bb65bd6a1ae57300df7e47dfd81f7de Mon Sep 17 00:00:00 2001 From: Oliver Schneider Date: Fri, 17 Feb 2017 13:23:19 +0100 Subject: [PATCH 4/6] fix dogfood tests --- clippy_lints/src/types.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/clippy_lints/src/types.rs b/clippy_lints/src/types.rs index ce5afb7eac84..0d7459e18926 100644 --- a/clippy_lints/src/types.rs +++ b/clippy_lints/src/types.rs @@ -531,8 +531,8 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for CastPass { if let ExprLit(ref lit) = ex.node { use syntax::ast::{LitKind, LitIntType}; match lit.node { - LitKind::Int(_, LitIntType::Unsuffixed) => (), - LitKind::FloatUnsuffixed(_) => (), + LitKind::Int(_, LitIntType::Unsuffixed) | + LitKind::FloatUnsuffixed(_) => {}, _ => { if cast_from.sty == cast_to.sty && !in_external_macro(cx, expr.span) { span_lint(cx, From 9d8dc689c73ae1738a0c02e68e5748e16499c0cd Mon Sep 17 00:00:00 2001 From: Oliver Schneider Date: Fri, 17 Feb 2017 13:41:59 +0100 Subject: [PATCH 5/6] rustfmt again --- clippy_lints/src/types.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/clippy_lints/src/types.rs b/clippy_lints/src/types.rs index 0d7459e18926..17d4ebd87081 100644 --- a/clippy_lints/src/types.rs +++ b/clippy_lints/src/types.rs @@ -539,7 +539,8 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for CastPass { UNNECESSARY_CAST, expr.span, &format!("casting to the same type is unnecessary (`{}` -> `{}`)", - cast_from, cast_to)); + cast_from, + cast_to)); } }, } From ee86d46cd1555d06e2a01d9bd928b44db07cb853 Mon Sep 17 00:00:00 2001 From: Bood Qian Date: Sat, 18 Feb 2017 11:13:38 +0800 Subject: [PATCH 6/6] Update README --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index a2f82d76d449..ead1138d54ad 100644 --- a/README.md +++ b/README.md @@ -180,7 +180,7 @@ transparently: ## Lints -There are 190 lints included in this crate: +There are 191 lints included in this crate: name | default | triggers on -----------------------------------------------------------------------------------------------------------------------|---------|----------------------------------------------------------------------------------------------------------------------------------