From e48fbba864dad14bb554bc60b445da41c8dd72d5 Mon Sep 17 00:00:00 2001 From: scurest Date: Fri, 29 Jan 2016 00:39:13 -0600 Subject: [PATCH] Add a lint to suggest uint == 0 over uint <= 0 --- README.md | 3 +- src/escape.rs | 2 +- src/lib.rs | 2 + src/types.rs | 52 +++++++++++++++++++ .../absurd_unsigned_comparisons.rs | 14 +++++ 5 files changed, 71 insertions(+), 2 deletions(-) create mode 100644 tests/compile-fail/absurd_unsigned_comparisons.rs diff --git a/README.md b/README.md index ec1dd7f6dbb..0c94fb3383e 100644 --- a/README.md +++ b/README.md @@ -6,10 +6,11 @@ A collection of lints to catch common mistakes and improve your Rust code. [Jump to usage instructions](#usage) ##Lints -There are 104 lints included in this crate: +There are 105 lints included in this crate: name | default | meaning ---------------------------------------------------------------------------------------------------------------|---------|------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ +[absurd_unsigned_comparisons](https://github.com/Manishearth/rust-clippy/wiki#absurd_unsigned_comparisons) | warn | testing whether an unsigned integer is non-positive [approx_constant](https://github.com/Manishearth/rust-clippy/wiki#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](https://github.com/Manishearth/rust-clippy/wiki#bad_bit_mask) | warn | 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) [block_in_if_condition_expr](https://github.com/Manishearth/rust-clippy/wiki#block_in_if_condition_expr) | warn | braces can be eliminated in conditions that are expressions, e.g `if { true } ...` diff --git a/src/escape.rs b/src/escape.rs index 32dbbb99226..c54c4395335 100644 --- a/src/escape.rs +++ b/src/escape.rs @@ -135,7 +135,7 @@ impl<'a, 'tcx: 'a> Delegate<'tcx> for EscapeDelegate<'a, 'tcx> { .get(&borrow_id) { if LoanCause::AutoRef == loan_cause { // x.foo() - if adj.autoderefs <= 0 { + if adj.autoderefs == 0 { self.set.remove(&lid); // Used without autodereffing (i.e. x.clone()) } } else { diff --git a/src/lib.rs b/src/lib.rs index 82bd2d39a12..3519fb9ee88 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -146,6 +146,7 @@ pub fn plugin_registrar(reg: &mut Registry) { reg.register_late_lint_pass(box types::CharLitAsU8); reg.register_late_lint_pass(box print::PrintLint); reg.register_late_lint_pass(box vec::UselessVec); + reg.register_late_lint_pass(box types::AbsurdUnsignedComparisons); reg.register_lint_group("clippy_pedantic", vec![ @@ -247,6 +248,7 @@ pub fn plugin_registrar(reg: &mut Registry) { strings::STRING_LIT_AS_BYTES, temporary_assignment::TEMPORARY_ASSIGNMENT, transmute::USELESS_TRANSMUTE, + types::ABSURD_UNSIGNED_COMPARISONS, types::BOX_VEC, types::CHAR_LIT_AS_U8, types::LET_UNIT_VALUE, diff --git a/src/types.rs b/src/types.rs index d41896cd490..0ab373bfa35 100644 --- a/src/types.rs +++ b/src/types.rs @@ -557,3 +557,55 @@ impl LateLintPass for CharLitAsU8 { } } } + +/// **What it does:** This lint checks for expressions where an unsigned integer is tested to be non-positive and suggests testing for equality with zero instead. +/// +/// **Why is this bad?** `x <= 0` may mislead the reader into thinking `x` can be negative. `x == 0` makes explicit that zero is the only possibility. +/// +/// **Known problems:** None +/// +/// **Example:** `vec.len() <= 0` +declare_lint!(pub ABSURD_UNSIGNED_COMPARISONS, Warn, + "testing whether an unsigned integer is non-positive"); + +pub struct AbsurdUnsignedComparisons; + +impl LintPass for AbsurdUnsignedComparisons { + fn get_lints(&self) -> LintArray { + lint_array!(ABSURD_UNSIGNED_COMPARISONS) + } +} + +fn is_zero_lit(expr: &Expr) -> bool { + use syntax::ast::Lit_; + + if let ExprLit(ref l) = expr.node { + if let Lit_::LitInt(val, _) = l.node { + return val == 0; + } + } + false +} + +impl LateLintPass for AbsurdUnsignedComparisons { + fn check_expr(&mut self, cx: &LateContext, expr: &Expr) { + if let ExprBinary(ref cmp, ref lhs, ref rhs) = expr.node { + let op = cmp.node; + + let comparee = match op { + BiLe if is_zero_lit(rhs) => lhs, // x <= 0 + BiGe if is_zero_lit(lhs) => rhs, // 0 >= x + _ => return, + }; + + if let ty::TyUint(_) = cx.tcx.expr_ty(comparee).sty { + if !in_macro(cx, expr.span) { + let msg = "testing whether an unsigned integer is non-positive"; + let help = format!("consider using {} == 0 instead", + snippet(cx, comparee.span, "x")); + span_help_and_lint(cx, ABSURD_UNSIGNED_COMPARISONS, expr.span, msg, &help); + } + } + } + } +} diff --git a/tests/compile-fail/absurd_unsigned_comparisons.rs b/tests/compile-fail/absurd_unsigned_comparisons.rs new file mode 100644 index 00000000000..d7817daf204 --- /dev/null +++ b/tests/compile-fail/absurd_unsigned_comparisons.rs @@ -0,0 +1,14 @@ +#![feature(plugin)] +#![plugin(clippy)] + +#![allow(unused)] + +#[deny(absurd_unsigned_comparisons)] +fn main() { + 1u32 <= 0; //~ERROR testing whether an unsigned integer is non-positive + 1u8 <= 0; //~ERROR testing whether an unsigned integer is non-positive + 1i32 <= 0; + 0 >= 1u32; //~ERROR testing whether an unsigned integer is non-positive + 0 >= 1; + 1u32 > 0; +}