From b02e80c0124ad356f5b5be4f8ae4e23c0bf33efc Mon Sep 17 00:00:00 2001 From: swgillespie Date: Sun, 11 Oct 2015 19:22:13 -0700 Subject: [PATCH] implement 0.0/0.0 -> NaN lint as described in #370 casing of NaN --- README.md | 3 +- src/lib.rs | 3 ++ src/zero_div_zero.rs | 50 +++++++++++++++++++++++++++++ tests/compile-fail/zero_div_zero.rs | 16 +++++++++ 4 files changed, 71 insertions(+), 1 deletion(-) create mode 100644 src/zero_div_zero.rs create mode 100644 tests/compile-fail/zero_div_zero.rs diff --git a/README.md b/README.md index a5ab856fc21..44262eba8a1 100644 --- a/README.md +++ b/README.md @@ -6,7 +6,7 @@ A collection of lints to catch common mistakes and improve your Rust code. [Jump to usage instructions](#usage) ##Lints -There are 59 lints included in this crate: +There are 60 lints included in this crate: name | default | meaning -------------------------------------------------------------------------------------------------------|---------|------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ @@ -68,6 +68,7 @@ name [while_let_loop](https://github.com/Manishearth/rust-clippy/wiki#while_let_loop) | warn | `loop { if let { ... } else break }` can be written as a `while let` loop [wrong_pub_self_convention](https://github.com/Manishearth/rust-clippy/wiki#wrong_pub_self_convention) | allow | defining a public method named with an established prefix (like "into_") that takes `self` with the wrong convention [wrong_self_convention](https://github.com/Manishearth/rust-clippy/wiki#wrong_self_convention) | warn | defining a method named with an established prefix (like "into_") that takes `self` with the wrong convention +[zero_divided_by_zero](https://github.com/Manishearth/rust-clippy/wiki#zero_divided_by_zero) | warn | usage of `0.0 / 0.0` to obtain NaN instead of std::f32::NaN or std::f64::NaN [zero_width_space](https://github.com/Manishearth/rust-clippy/wiki#zero_width_space) | deny | using a zero-width space in a string literal, which is confusing More to come, please [file an issue](https://github.com/Manishearth/rust-clippy/issues) if you have ideas! diff --git a/src/lib.rs b/src/lib.rs index 6503c5e1006..8675eb01527 100755 --- a/src/lib.rs +++ b/src/lib.rs @@ -47,6 +47,7 @@ pub mod loops; pub mod ranges; pub mod matches; pub mod precedence; +pub mod zero_div_zero; mod reexport { pub use syntax::ast::{Name, Ident, NodeId}; @@ -88,6 +89,7 @@ pub fn plugin_registrar(reg: &mut Registry) { reg.register_late_lint_pass(box matches::MatchPass); reg.register_late_lint_pass(box misc::PatternPass); reg.register_late_lint_pass(box minmax::MinMaxPass); + reg.register_late_lint_pass(box zero_div_zero::ZeroDivZeroPass); reg.register_lint_group("clippy_pedantic", vec![ methods::OPTION_UNWRAP_USED, @@ -152,5 +154,6 @@ pub fn plugin_registrar(reg: &mut Registry) { types::TYPE_COMPLEXITY, types::UNIT_CMP, unicode::ZERO_WIDTH_SPACE, + zero_div_zero::ZERO_DIVIDED_BY_ZERO, ]); } diff --git a/src/zero_div_zero.rs b/src/zero_div_zero.rs new file mode 100644 index 00000000000..37d5d8904e1 --- /dev/null +++ b/src/zero_div_zero.rs @@ -0,0 +1,50 @@ +use rustc::lint::*; +use rustc_front::hir::*; + +use utils::{span_help_and_lint}; +use consts::{Constant, constant_simple, FloatWidth}; + +/// ZeroDivZeroPass is a pass that checks for a binary expression that consists +/// of 0.0/0.0, which is always NaN. It is more clear to replace instances of +/// 0.0/0.0 with std::f32::NaN or std::f64::NaN, depending on the precision. +pub struct ZeroDivZeroPass; + +declare_lint!(pub ZERO_DIVIDED_BY_ZERO, Warn, + "usage of `0.0 / 0.0` to obtain NaN instead of std::f32::NaN or std::f64::NaN"); + +impl LintPass for ZeroDivZeroPass { + fn get_lints(&self) -> LintArray { + lint_array!(ZERO_DIVIDED_BY_ZERO) + } +} + +impl LateLintPass for ZeroDivZeroPass { + fn check_expr(&mut self, cx: &LateContext, expr: &Expr) { + // check for instances of 0.0/0.0 + if_let_chain! { + [ + let ExprBinary(ref op, ref left, ref right) = expr.node, + let BinOp_::BiDiv = op.node, + // TODO - constant_simple does not fold many operations involving floats. + // That's probably fine for this lint - it's pretty unlikely that someone would + // do something like 0.0/(2.0 - 2.0), but it would be nice to warn on that case too. + let Some(Constant::ConstantFloat(ref lhs_value, lhs_width)) = constant_simple(left), + let Some(Constant::ConstantFloat(ref rhs_value, rhs_width)) = constant_simple(right), + let Some(0.0) = lhs_value.parse().ok(), + let Some(0.0) = rhs_value.parse().ok() + ], + { + // since we're about to suggest a use of std::f32::NaN or std::f64::NaN, + // match the precision of the literals that are given. + let float_type = match (lhs_width, rhs_width) { + (FloatWidth::Fw64, _) + | (_, FloatWidth::Fw64) => "f64", + _ => "f32" + }; + span_help_and_lint(cx, ZERO_DIVIDED_BY_ZERO, expr.span, + "constant division of 0.0 with 0.0 will always result in NaN", + &format!("Consider using `std::{}::NAN` if you would like a constant representing NaN", float_type)); + } + } + } +} diff --git a/tests/compile-fail/zero_div_zero.rs b/tests/compile-fail/zero_div_zero.rs new file mode 100644 index 00000000000..8c40923d3ed --- /dev/null +++ b/tests/compile-fail/zero_div_zero.rs @@ -0,0 +1,16 @@ +#![feature(plugin)] +#![plugin(clippy)] + +#[allow(unused_variables)] +#[deny(zero_divided_by_zero)] +fn main() { + let nan = 0.0 / 0.0; //~ERROR constant division of 0.0 with 0.0 will always result in NaN + let f64_nan = 0.0 / 0.0f64; //~ERROR constant division of 0.0 with 0.0 will always result in NaN + let other_f64_nan = 0.0f64 / 0.0; //~ERROR constant division of 0.0 with 0.0 will always result in NaN + let one_more_f64_nan = 0.0f64/0.0f64; //~ERROR constant division of 0.0 with 0.0 will always result in NaN + let zero = 0.0; + let other_zero = 0.0; + let other_nan = zero / other_zero; // fine - this lint doesn't propegate constants. + let not_nan = 2.0/0.0; // not an error: 2/0 = inf + let also_not_nan = 0.0/2.0; // not an error: 0/2 = 0 +}