diff --git a/README.md b/README.md index 211833bac..3014a1a3e 100644 --- a/README.md +++ b/README.md @@ -16,6 +16,15 @@ Lints included in this crate: - `needless_bool` : Warns on if-statements with plain booleans in the then- and else-clause, e.g. `if p { true } else { false }` - `ptr_arg`: Warns on fn arguments of the type `&Vec<...>` or `&String`, suggesting to use `&[...]` or `&str` instead, respectively - `approx_constant`: Warns if the approximate of a known float constant (in `std::f64::consts` or `std::f32::consts`) is found and suggests to use the constant + - `cmp_nan`: Denies comparisons to NAN (which will always return false, which is probably not intended) + - `float_cmp`: Warns on `==` or `!=` comparisons of floaty typed values. As floating-point operations usually involve rounding errors, it is always better to check for approximate equality within some small bounds + +To use, add the following lines to your Cargo.toml: + +``` +[dev-dependencies.rust-clippy] +git = "https://github.com/Manishearth/rust-clippy" +``` In your code, you may add `#![plugin(clippy)]` to use it (you may also need to include a `#![feature(plugin)]` line) diff --git a/src/lib.rs b/src/lib.rs index 8137eccf1..5b2f71bb4 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -34,6 +34,7 @@ pub fn plugin_registrar(reg: &mut Registry) { reg.register_lint_pass(box ptr_arg::PtrArg as LintPassObject); reg.register_lint_pass(box needless_bool::NeedlessBool as LintPassObject); reg.register_lint_pass(box approx_const::ApproxConstant as LintPassObject); + reg.register_lint_pass(box misc::FloatCmp as LintPassObject); reg.register_lint_group("clippy", vec![types::BOX_VEC, types::LINKEDLIST, misc::SINGLE_MATCH, misc::STR_TO_STRING, @@ -41,6 +42,6 @@ pub fn plugin_registrar(reg: &mut Registry) { bit_mask::BAD_BIT_MASK, ptr_arg::PTR_ARG, needless_bool::NEEDLESS_BOOL, approx_const::APPROX_CONSTANT, - misc::CMP_NAN + misc::CMP_NAN, misc::FLOAT_CMP, ]); } diff --git a/src/misc.rs b/src/misc.rs index 17de26807..cdc39a4b8 100644 --- a/src/misc.rs +++ b/src/misc.rs @@ -1,14 +1,22 @@ use syntax::ptr::P; use syntax::ast; use syntax::ast::*; -use syntax::ast_util::is_comparison_binop; +use syntax::ast_util::{is_comparison_binop, binop_to_string}; use syntax::visit::{FnKind}; use rustc::lint::{Context, LintPass, LintArray, Lint, Level}; -use rustc::middle::ty::{self, expr_ty, ty_str, ty_ptr, ty_rptr}; +use rustc::middle::ty::{self, expr_ty, ty_str, ty_ptr, ty_rptr, ty_float}; use syntax::codemap::Span; + use types::span_note_and_lint; +fn walk_ty<'t>(ty: ty::Ty<'t>) -> ty::Ty<'t> { + match ty.sty { + ty_ptr(ref tm) | ty_rptr(_, ref tm) => walk_ty(tm.ty), + _ => ty + } +} + /// Handles uncategorized lints /// Currently handles linting of if-let-able matches #[allow(missing_copy_implementations)] @@ -71,13 +79,6 @@ impl LintPass for StrToStringPass { } fn is_str(cx: &Context, expr: &ast::Expr) -> bool { - fn walk_ty<'t>(ty: ty::Ty<'t>) -> ty::Ty<'t> { - //println!("{}: -> {}", depth, ty); - match ty.sty { - ty_ptr(ref tm) | ty_rptr(_, ref tm) => walk_ty(tm.ty), - _ => ty - } - } match walk_ty(expr_ty(cx.tcx, expr)).sty { ty_str => true, _ => false @@ -110,7 +111,7 @@ impl LintPass for TopLevelRefPass { } } -declare_lint!(pub CMP_NAN, Allow, "Deny comparisons to std::f32::NAN or std::f64::NAN"); +declare_lint!(pub CMP_NAN, Deny, "Deny comparisons to std::f32::NAN or std::f64::NAN"); #[derive(Copy,Clone)] pub struct CmpNan; @@ -139,3 +140,32 @@ fn check_nan(cx: &Context, path: &Path, span: Span) { cx.span_lint(CMP_NAN, span, "Doomed comparison with NAN, use std::{f32,f64}::is_nan instead"); }); } + +declare_lint!(pub FLOAT_CMP, Warn, + "Warn on ==/!= comparison of floaty values"); + +#[derive(Copy,Clone)] +pub struct FloatCmp; + +impl LintPass for FloatCmp { + fn get_lints(&self) -> LintArray { + lint_array!(FLOAT_CMP) + } + + fn check_expr(&mut self, cx: &Context, expr: &Expr) { + if let ExprBinary(ref cmp, ref left, ref right) = expr.node { + let op = cmp.node; + if (op == BiEq || op == BiNe) && (is_float(cx, left) || is_float(cx, right)) { + let map = cx.sess().codemap(); + cx.span_lint(FLOAT_CMP, expr.span, &format!( + "{}-Comparison of f32 or f64 detected. You may want to change this to 'abs({} - {}) < epsilon' for some suitable value of epsilon", + binop_to_string(op), &*map.span_to_snippet(left.span).unwrap_or("..".to_string()), + &*map.span_to_snippet(right.span).unwrap_or("..".to_string()))); + } + } + } +} + +fn is_float(cx: &Context, expr: &Expr) -> bool { + if let ty_float(_) = walk_ty(expr_ty(cx.tcx, expr)).sty { true } else { false } +} diff --git a/tests/compile-fail/cmp_nan.rs b/tests/compile-fail/cmp_nan.rs index b876bfcc6..696313314 100644 --- a/tests/compile-fail/cmp_nan.rs +++ b/tests/compile-fail/cmp_nan.rs @@ -2,6 +2,7 @@ #![plugin(clippy)] #[deny(cmp_nan)] +#[allow(float_cmp)] fn main() { let x = 5f32; x == std::f32::NAN; //~ERROR diff --git a/tests/compile-fail/float_cmp.rs b/tests/compile-fail/float_cmp.rs new file mode 100644 index 000000000..dce8dba1e --- /dev/null +++ b/tests/compile-fail/float_cmp.rs @@ -0,0 +1,35 @@ +#![feature(plugin)] +#![plugin(clippy)] + +use std::ops::Add; + +const ZERO : f32 = 0.0; +const ONE : f32 = ZERO + 1.0; + +fn twice(x : T) -> T where T : Add, T : Copy { + x + x +} + +#[deny(float_cmp)] +#[allow(unused)] +fn main() { + ZERO == 0f32; //~ERROR + ZERO == 0.0; //~ERROR + ZERO + ZERO != 1.0; //~ERROR + + ONE != 0.0; //~ERROR + twice(ONE) != ONE; //~ERROR + ONE as f64 != 0.0; //~ERROR + + let x : f64 = 1.0; + + x == 1.0; //~ERROR + x != 0f64; //~ERROR + + twice(x) != twice(ONE as f64); //~ERROR + + x < 0.0; + x > 0.0; + x <= 0.0; + x >= 0.0; +}