OpenE2K/rust
2
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Initial implementation of lossy cast lints.

Browse Source 
Introduces 3 lints :
  cast_possible_overflow
  cast_precision_loss
  cast_sign_loss
Add a compile-test test case.
Fix errors spotted by dogfood script.
This commit is contained in:
R.Chavignat 2015-08-20 00:04:01 +02:00
parent 150840667e
commit 993239d33a
5 changed files with 223 additions and 51 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

85
README.md
View File

@ -6,47 +6,50 @@ A collection of lints that give helpful tips to newbies and catch oversights.
##Lints
Lints included in this crate:
name | default | meaning
---------------------|---------|-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------
approx_constant | warn | the approximate of a known float constant (in `std::f64::consts` or `std::f32::consts`) is found; suggests to use the constant
bad_bit_mask | deny | expressions of the form `_ & mask == select` that will only ever return `true` or `false` (because in the example `select` containing bits that `mask` doesn't have)
box_vec | warn | usage of `Box<Vec<T>>`, vector elements are already on the heap
cmp_nan | deny | comparisons to NAN (which will always return false, which is probably not intended)
cmp_owned | warn | creating owned instances for comparing with others, e.g. `x == "foo".to_string()`
collapsible_if | warn | two nested `if`-expressions can be collapsed into one, e.g. `if x { if y { foo() } }` can be written as `if x && y { foo() }`
eq_op | warn | equal operands on both sides of a comparison or bitwise combination (e.g. `x == x`)
explicit_iter_loop | warn | for-looping over `_.iter()` or `_.iter_mut()` when `&_` or `&mut _` would do
float_cmp | warn | using `==` or `!=` on float values (as floating-point operations usually involve rounding errors, it is always better to check for approximate equality within small bounds)
identity_op | warn | using identity operations, e.g. `x + 0` or `y / 1`
ineffective_bit_mask | warn | expressions where a bit mask will be rendered useless by a comparison, e.g. `(x | 1) > 2`
inline_always | warn | `#[inline(always)]` is a bad idea in most cases
iter_next_loop | warn | for-looping over `_.next()` which is probably not intended
len_without_is_empty | warn | traits and impls that have `.len()` but not `.is_empty()`
len_zero | warn | checking `.len() == 0` or `.len() > 0` (or similar) when `.is_empty()` could be used instead
let_and_return | warn | creating a let-binding and then immediately returning it like `let x = expr; x` at the end of a function
let_unit_value | warn | creating a let binding to a value of unit type, which usually can't be used afterwards
linkedlist | warn | usage of LinkedList, usually a vector is faster, or a more specialized data structure like a RingBuf
modulo_one | warn | taking a number modulo 1, which always returns 0
mut_mut | warn | usage of double-mut refs, e.g. `&mut &mut ...` (either copy'n'paste error, or shows a fundamental misunderstanding of references)
needless_bool | warn | if-statements with plain booleans in the then- and else-clause, e.g. `if p { true } else { false }`
needless_lifetimes | warn | using explicit lifetimes for references in function arguments when elision rules would allow omitting them
needless_range_loop | warn | for-looping over a range of indices where an iterator over items would do
needless_return | warn | using a return statement like `return expr;` where an expression would suffice
non_ascii_literal | allow | using any literal non-ASCII chars in a string literal; suggests using the \\u escape instead
option_unwrap_used | allow | using `Option.unwrap()`, which should at least get a better message using `expect()`
precedence | warn | expressions where precedence may trip up the unwary reader of the source; suggests adding parentheses, e.g. `x << 2 + y` will be parsed as `x << (2 + y)`
ptr_arg | allow | fn arguments of the type `&Vec<...>` or `&String`, suggesting to use `&[...]` or `&str` instead, respectively
range_step_by_zero | warn | using Range::step_by(0), which produces an infinite iterator
redundant_closure | warn | using redundant closures, i.e. `|a| foo(a)` (which can be written as just `foo`)
result_unwrap_used | allow | using `Result.unwrap()`, which might be better handled
single_match | warn | a match statement with a single nontrivial arm (i.e, where the other arm is `_ => {}`) is used; recommends `if let` instead
str_to_string | warn | using `to_string()` on a str, which should be `to_owned()`
string_add | allow | using `x + ..` where x is a `String`; suggests using `push_str()` instead
string_add_assign | allow | using `x = x + ..` where x is a `String`; suggests using `push_str()` instead
string_to_string | warn | calling `String.to_string()` which is a no-op
toplevel_ref_arg | warn | a function argument is declared `ref` (i.e. `fn foo(ref x: u8)`, but not `fn foo((ref x, ref y): (u8, u8))`)
unit_cmp | warn | comparing unit values (which is always `true` or `false`, respectively)
zero_width_space | deny | using a zero-width space in a string literal, which is confusing
name | default | meaning
-----------------------|---------|-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------
approx_constant | warn | the approximate of a known float constant (in `std::f64::consts` or `std::f32::consts`) is found; suggests to use the constant
bad_bit_mask | deny | expressions of the form `_ & mask == select` that will only ever return `true` or `false` (because in the example `select` containing bits that `mask` doesn't have)
box_vec | warn | usage of `Box<Vec<T>>`, vector elements are already on the heap
cast_possible_overflow | allow | casts that may cause overflow
cast_precision_loss | allow | casts that cause loss of precision
cast_sign_loss | allow | casts from signed types to unsigned types
cmp_nan | deny | comparisons to NAN (which will always return false, which is probably not intended)
cmp_owned | warn | creating owned instances for comparing with others, e.g. `x == "foo".to_string()`
collapsible_if | warn | two nested `if`-expressions can be collapsed into one, e.g. `if x { if y { foo() } }` can be written as `if x && y { foo() }`
eq_op | warn | equal operands on both sides of a comparison or bitwise combination (e.g. `x == x`)
explicit_iter_loop | warn | for-looping over `_.iter()` or `_.iter_mut()` when `&_` or `&mut _` would do
float_cmp | warn | using `==` or `!=` on float values (as floating-point operations usually involve rounding errors, it is always better to check for approximate equality within small bounds)
identity_op | warn | using identity operations, e.g. `x + 0` or `y / 1`
ineffective_bit_mask | warn | expressions where a bit mask will be rendered useless by a comparison, e.g. `(x | 1) > 2`
inline_always | warn | `#[inline(always)]` is a bad idea in most cases
iter_next_loop | warn | for-looping over `_.next()` which is probably not intended
len_without_is_empty | warn | traits and impls that have `.len()` but not `.is_empty()`
len_zero | warn | checking `.len() == 0` or `.len() > 0` (or similar) when `.is_empty()` could be used instead
let_and_return | warn | creating a let-binding and then immediately returning it like `let x = expr; x` at the end of a function
let_unit_value | warn | creating a let binding to a value of unit type, which usually can't be used afterwards
linkedlist | warn | usage of LinkedList, usually a vector is faster, or a more specialized data structure like a RingBuf
modulo_one | warn | taking a number modulo 1, which always returns 0
mut_mut | warn | usage of double-mut refs, e.g. `&mut &mut ...` (either copy'n'paste error, or shows a fundamental misunderstanding of references)
needless_bool | warn | if-statements with plain booleans in the then- and else-clause, e.g. `if p { true } else { false }`
needless_lifetimes | warn | using explicit lifetimes for references in function arguments when elision rules would allow omitting them
needless_range_loop | warn | for-looping over a range of indices where an iterator over items would do
needless_return | warn | using a return statement like `return expr;` where an expression would suffice
non_ascii_literal | allow | using any literal non-ASCII chars in a string literal; suggests using the \\u escape instead
option_unwrap_used | allow | using `Option.unwrap()`, which should at least get a better message using `expect()`
precedence | warn | expressions where precedence may trip up the unwary reader of the source; suggests adding parentheses, e.g. `x << 2 + y` will be parsed as `x << (2 + y)`
ptr_arg | allow | fn arguments of the type `&Vec<...>` or `&String`, suggesting to use `&[...]` or `&str` instead, respectively
range_step_by_zero | warn | using Range::step_by(0), which produces an infinite iterator
redundant_closure | warn | using redundant closures, i.e. `|a| foo(a)` (which can be written as just `foo`)
result_unwrap_used | allow | using `Result.unwrap()`, which might be better handled
single_match | warn | a match statement with a single nontrivial arm (i.e, where the other arm is `_ => {}`) is used; recommends `if let` instead
str_to_string | warn | using `to_string()` on a str, which should be `to_owned()`
string_add | allow | using `x + ..` where x is a `String`; suggests using `push_str()` instead
string_add_assign | allow | using `x = x + ..` where x is a `String`; suggests using `push_str()` instead
string_to_string | warn | calling `String.to_string()` which is a no-op
toplevel_ref_arg | warn | a function argument is declared `ref` (i.e. `fn foo(ref x: u8)`, but not `fn foo((ref x, ref y): (u8, u8))`)
unit_cmp | warn | comparing unit values (which is always `true` or `false`, respectively)
zero_width_space | deny | using a zero-width space in a string literal, which is confusing
To use, add the following lines to your Cargo.toml:

19
src/consts.rs
View File

@ -67,15 +67,16 @@ impl Constant {
}
/// convert this constant to a f64, if possible
pub fn as_float(&self) -> Option<f64> {
match *self {
ConstantByte(b) => Some(b as f64),
ConstantFloat(ref s, _) => s.parse().ok(),
ConstantInt(i, ty) => Some(if is_negative(ty) {
-(i as f64) } else { i as f64 }),
_ => None
}
}
#[allow(unknown_lints,cast_precision_loss)]
pub fn as_float(&self) -> Option<f64> {
match *self {
ConstantByte(b) => Some(b as f64),
ConstantFloat(ref s, _) => s.parse().ok(),
ConstantInt(i, ty) => Some(if is_negative(ty) {
-(i as f64) } else { i as f64 }),
_ => None
}
}
}
impl PartialEq for Constant {

4
src/lib.rs
View File

@ -68,6 +68,7 @@ pub fn plugin_registrar(reg: &mut Registry) {
reg.register_lint_pass(box loops::LoopsPass as LintPassObject);
reg.register_lint_pass(box lifetimes::LifetimePass as LintPassObject);
reg.register_lint_pass(box ranges::StepByZero as LintPassObject);
reg.register_lint_pass(box types::CastPass as LintPassObject);
reg.register_lint_group("clippy", vec![
approx_const::APPROX_CONSTANT,
@ -104,6 +105,9 @@ pub fn plugin_registrar(reg: &mut Registry) {
strings::STRING_ADD,
strings::STRING_ADD_ASSIGN,
types::BOX_VEC,
types::CAST_POSSIBLE_OVERFLOW,
types::CAST_PRECISION_LOSS,
types::CAST_SIGN_LOSS,
types::LET_UNIT_VALUE,
types::LINKEDLIST,
types::UNIT_CMP,

135
src/types.rs
View File

@ -6,7 +6,7 @@ use syntax::ptr::P;
use rustc::middle::ty;
use syntax::codemap::ExpnInfo;
use utils::{in_macro, snippet, span_lint, span_help_and_lint};
use utils::{in_macro, snippet, span_lint, span_help_and_lint, in_external_macro};
/// Handles all the linting of funky types
#[allow(missing_copy_implementations)]
@ -136,3 +136,136 @@ impl LintPass for UnitCmp {
}
}
}
pub struct CastPass;
declare_lint!(pub CAST_PRECISION_LOSS, Allow,
"casts that cause loss of precision");
declare_lint!(pub CAST_SIGN_LOSS, Allow,
"casts from signed types to unsigned types");
declare_lint!(pub CAST_POSSIBLE_OVERFLOW, Allow,
"casts that may cause overflow");
impl LintPass for CastPass {
fn get_lints(&self) -> LintArray {
lint_array!(CAST_PRECISION_LOSS,
CAST_SIGN_LOSS,
CAST_POSSIBLE_OVERFLOW)
}
fn check_expr(&mut self, cx: &Context, expr: &Expr) {
if let ExprCast(ref ex, _) = expr.node {
let (cast_from, cast_to) = (cx.tcx.expr_ty(&*ex), cx.tcx.expr_ty(expr));
if cast_from.is_numeric() && !in_external_macro(cx, expr.span) {
match (cast_from.is_integral(), cast_to.is_integral()) {
(true, false) => {
match (&cast_from.sty, &cast_to.sty) {
(&ty::TypeVariants::TyInt(i), &ty::TypeVariants::TyFloat(f)) => {
match (i, f) {
(ast::IntTy::TyI32, ast::FloatTy::TyF32) |
(ast::IntTy::TyI64, ast::FloatTy::TyF32) |
(ast::IntTy::TyI64, ast::FloatTy::TyF64) => {
span_lint(cx, CAST_PRECISION_LOSS, expr.span,
&format!("converting from {} to {}, which causes a loss of precision",
i, f));
},
_ => ()
}
}
(&ty::TypeVariants::TyUint(u), &ty::TypeVariants::TyFloat(f)) => {
match (u, f) {
(ast::UintTy::TyU32, ast::FloatTy::TyF32) |
(ast::UintTy::TyU64, ast::FloatTy::TyF32) |
(ast::UintTy::TyU64, ast::FloatTy::TyF64) => {
span_lint(cx, CAST_PRECISION_LOSS, expr.span,
&format!("converting from {} to {}, which causes a loss of precision",
u, f));
},
_ => ()
}
},
_ => ()
}
},
(false, true) => {
span_lint(cx, CAST_POSSIBLE_OVERFLOW, expr.span,
&format!("the contents of a {} may overflow a {}", cast_from, cast_to));
if !cx.tcx.expr_ty(expr).is_signed() {
span_lint(cx, CAST_SIGN_LOSS, expr.span,
&format!("casting from {} to {} loses the sign of the value", cast_from, cast_to));
}
},
(true, true) => {
match (&cast_from.sty, &cast_to.sty) {
(&ty::TypeVariants::TyInt(i1), &ty::TypeVariants::TyInt(i2)) => {
match (i1, i2) {
(ast::IntTy::TyI64, ast::IntTy::TyI32) |
(ast::IntTy::TyI64, ast::IntTy::TyI16) |
(ast::IntTy::TyI64, ast::IntTy::TyI8) |
(ast::IntTy::TyI32, ast::IntTy::TyI16) |
(ast::IntTy::TyI32, ast::IntTy::TyI8) |
(ast::IntTy::TyI16, ast::IntTy::TyI8) =>
span_lint(cx, CAST_POSSIBLE_OVERFLOW, expr.span,
&format!("the contents of a {} may overflow a {}", i1, i2)),
_ => ()
}
},
(&ty::TypeVariants::TyInt(i), &ty::TypeVariants::TyUint(u)) => {
span_lint(cx, CAST_SIGN_LOSS, expr.span,
&format!("casting from {} to {} loses the sign of the value", i, u));
match (i, u) {
(ast::IntTy::TyI64, ast::UintTy::TyU32) |
(ast::IntTy::TyI64, ast::UintTy::TyU16) |
(ast::IntTy::TyI64, ast::UintTy::TyU8) |
(ast::IntTy::TyI32, ast::UintTy::TyU16) |
(ast::IntTy::TyI32, ast::UintTy::TyU8) |
(ast::IntTy::TyI16, ast::UintTy::TyU8) =>
span_lint(cx, CAST_POSSIBLE_OVERFLOW, expr.span,
&format!("the contents of a {} may overflow a {}", i, u)),
_ => ()
}
},
(&ty::TypeVariants::TyUint(u), &ty::TypeVariants::TyInt(i)) => {
match (u, i) {
(ast::UintTy::TyU64, ast::IntTy::TyI32) |
(ast::UintTy::TyU64, ast::IntTy::TyI64) |
(ast::UintTy::TyU64, ast::IntTy::TyI16) |
(ast::UintTy::TyU64, ast::IntTy::TyI8) |
(ast::UintTy::TyU32, ast::IntTy::TyI32) |
(ast::UintTy::TyU32, ast::IntTy::TyI16) |
(ast::UintTy::TyU32, ast::IntTy::TyI8) |
(ast::UintTy::TyU16, ast::IntTy::TyI16) |
(ast::UintTy::TyU16, ast::IntTy::TyI8) |
(ast::UintTy::TyU8, ast::IntTy::TyI8) =>
span_lint(cx, CAST_POSSIBLE_OVERFLOW, expr.span,
&format!("the contents of a {} may overflow a {}", u, i)),
_ => ()
}
},
(&ty::TypeVariants::TyUint(u1), &ty::TypeVariants::TyUint(u2)) => {
match (u1, u2) {
(ast::UintTy::TyU64, ast::UintTy::TyU32) |
(ast::UintTy::TyU64, ast::UintTy::TyU16) |
(ast::UintTy::TyU64, ast::UintTy::TyU8) |
(ast::UintTy::TyU32, ast::UintTy::TyU16) |
(ast::UintTy::TyU32, ast::UintTy::TyU8) |
(ast::UintTy::TyU16, ast::UintTy::TyU8) =>
span_lint(cx, CAST_POSSIBLE_OVERFLOW, expr.span,
&format!("the contents of a {} may overflow a {}", u1, u2)),
_ => ()
}
},
_ => ()
}
}
(false, false) => {
if let (&ty::TypeVariants::TyFloat(ast::FloatTy::TyF64),
&ty::TypeVariants::TyFloat(ast::FloatTy::TyF32)) = (&cast_from.sty, &cast_to.sty) {
span_lint(cx, CAST_POSSIBLE_OVERFLOW, expr.span, "the contents of a f64 may overflow a f32");
}
}
}
}
}
}
}

31
tests/compile-fail/cast.rs Normal file
View File

@ -0,0 +1,31 @@
#![feature(plugin)]
#![plugin(clippy)]
#[deny(cast_precision_loss, cast_possible_overflow, cast_sign_loss)]
fn main() {
let i : i32 = 42;
let u : u32 = 42;
let f : f32 = 42.0;
// Test cast_precision_loss
i as f32; //~ERROR converting from i32 to f32, which causes a loss of precision
(i as i64) as f32; //~ERROR converting from i64 to f32, which causes a loss of precision
(i as i64) as f64; //~ERROR converting from i64 to f64, which causes a loss of precision
u as f32; //~ERROR converting from u32 to f32, which causes a loss of precision
(u as u64) as f32; //~ERROR converting from u64 to f32, which causes a loss of precision
(u as u64) as f64; //~ERROR converting from u64 to f64, which causes a loss of precision
i as f64; // Should not trigger the lint
u as f64; // Should not trigger the lint
// Test cast_possible_overflow
f as i32; //~ERROR the contents of a f32 may overflow a i32
f as u32; //~ERROR the contents of a f32 may overflow a u32
//~^ERROR casting from f32 to u32 loses the sign of the value
i as u8; //~ERROR the contents of a i32 may overflow a u8
//~^ERROR casting from i32 to u8 loses the sign of the value
(f as f64) as f32; //~ERROR the contents of a f64 may overflow a f32
i as i8; //~ERROR the contents of a i32 may overflow a i8
// Test cast_sign_loss
i as u32; //~ERROR casting from i32 to u32 loses the sign of the value
}