diff --git a/CHANGELOG.md b/CHANGELOG.md index 2493ae250..d02136421 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -145,6 +145,7 @@ All notable changes to this project will be documented in this file. [`needless_range_loop`]: https://github.com/Manishearth/rust-clippy/wiki#needless_range_loop [`needless_return`]: https://github.com/Manishearth/rust-clippy/wiki#needless_return [`needless_update`]: https://github.com/Manishearth/rust-clippy/wiki#needless_update +[`neg_multiply`]: https://github.com/Manishearth/rust-clippy/wiki#neg_multiply [`new_ret_no_self`]: https://github.com/Manishearth/rust-clippy/wiki#new_ret_no_self [`new_without_default`]: https://github.com/Manishearth/rust-clippy/wiki#new_without_default [`no_effect`]: https://github.com/Manishearth/rust-clippy/wiki#no_effect diff --git a/README.md b/README.md index 80d26cded..3951418ea 100644 --- a/README.md +++ b/README.md @@ -14,7 +14,7 @@ Table of contents: * [License](#license) ##Lints -There are 141 lints included in this crate: +There are 142 lints included in this crate: name | default | meaning ---------------------------------------------------------------------------------------------------------------------|---------|------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ @@ -95,6 +95,7 @@ name [needless_range_loop](https://github.com/Manishearth/rust-clippy/wiki#needless_range_loop) | warn | for-looping over a range of indices where an iterator over items would do [needless_return](https://github.com/Manishearth/rust-clippy/wiki#needless_return) | warn | using a return statement like `return expr;` where an expression would suffice [needless_update](https://github.com/Manishearth/rust-clippy/wiki#needless_update) | warn | using `{ ..base }` when there are no missing fields +[neg_multiply](https://github.com/Manishearth/rust-clippy/wiki#neg_multiply) | warn | Warns on multiplying integers with -1 [new_ret_no_self](https://github.com/Manishearth/rust-clippy/wiki#new_ret_no_self) | warn | not returning `Self` in a `new` method [new_without_default](https://github.com/Manishearth/rust-clippy/wiki#new_without_default) | warn | `fn new() -> Self` method without `Default` implementation [no_effect](https://github.com/Manishearth/rust-clippy/wiki#no_effect) | warn | statements with no effect diff --git a/src/consts.rs b/src/consts.rs index 0eed34a80..4a5f457ed 100644 --- a/src/consts.rs +++ b/src/consts.rs @@ -177,8 +177,9 @@ impl PartialOrd for Constant { } } +/// parse a `LitKind` to a `Constant` #[allow(cast_possible_wrap)] -fn lit_to_constant(lit: &LitKind) -> Constant { +pub fn lit_to_constant(lit: &LitKind) -> Constant { match *lit { LitKind::Str(ref is, style) => Constant::Str(is.to_string(), style), LitKind::Byte(b) => Constant::Int(ConstInt::U8(b)), diff --git a/src/lib.rs b/src/lib.rs index 0f0051522..6d98cf55f 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -88,6 +88,7 @@ pub mod mut_reference; pub mod mutex_atomic; pub mod needless_bool; pub mod needless_update; +pub mod neg_multiply; pub mod new_without_default; pub mod no_effect; pub mod non_expressive_names; @@ -232,6 +233,7 @@ pub fn plugin_registrar(reg: &mut Registry) { reg.register_late_lint_pass(box blacklisted_name::BlackListedName::new(conf.blacklisted_names)); reg.register_late_lint_pass(box functions::Functions::new(conf.too_many_arguments_threshold)); reg.register_early_lint_pass(box doc::Doc::new(conf.doc_valid_idents)); + reg.register_late_lint_pass(box neg_multiply::NegMultiply); reg.register_lint_group("clippy_pedantic", vec![ array_indexing::INDEXING_SLICING, @@ -343,6 +345,7 @@ pub fn plugin_registrar(reg: &mut Registry) { needless_bool::BOOL_COMPARISON, needless_bool::NEEDLESS_BOOL, needless_update::NEEDLESS_UPDATE, + neg_multiply::NEG_MULTIPLY, new_without_default::NEW_WITHOUT_DEFAULT, no_effect::NO_EFFECT, non_expressive_names::MANY_SINGLE_CHAR_NAMES, diff --git a/src/neg_multiply.rs b/src/neg_multiply.rs new file mode 100644 index 000000000..fb986409a --- /dev/null +++ b/src/neg_multiply.rs @@ -0,0 +1,57 @@ +use rustc::hir::*; +use rustc::lint::*; +use syntax::codemap::{Span, Spanned}; + +use consts::{self, Constant}; +use utils::span_lint; + +/// **What it does:** Checks for multiplication by -1 as a form of negation. +/// +/// **Why is this bad?** It's more readable to just negate. +/// +/// **Known problems:** This only catches integers (for now) +/// +/// **Example:** `x * -1` +declare_lint! { + pub NEG_MULTIPLY, + Warn, + "Warns on multiplying integers with -1" +} + +#[derive(Copy, Clone)] +pub struct NegMultiply; + +impl LintPass for NegMultiply { + fn get_lints(&self) -> LintArray { + lint_array!(NEG_MULTIPLY) + } +} + +#[allow(match_same_arms)] +impl LateLintPass for NegMultiply { + fn check_expr(&mut self, cx: &LateContext, e: &Expr) { + if let ExprBinary(Spanned { node: BiMul, .. }, ref l, ref r) = e.node { + match (&l.node, &r.node) { + (&ExprUnary(..), &ExprUnary(..)) => (), + (&ExprUnary(UnNeg, ref lit), _) => check_mul(cx, e.span, lit, r), + (_, &ExprUnary(UnNeg, ref lit)) => check_mul(cx, e.span, lit, l), + _ => () + } + } + } +} + +fn check_mul(cx: &LateContext, span: Span, lit: &Expr, exp: &Expr) { + if_let_chain!([ + let ExprLit(ref l) = lit.node, + let Constant::Int(ref ci) = consts::lit_to_constant(&l.node), + let Some(val) = ci.to_u64(), + val == 1, + cx.tcx.expr_ty(exp).is_integral() + ], { + span_lint(cx, + NEG_MULTIPLY, + span, + "Negation by multiplying with -1"); + }) +} diff --git a/tests/compile-fail/neg_multiply.rs b/tests/compile-fail/neg_multiply.rs new file mode 100644 index 000000000..9deb38920 --- /dev/null +++ b/tests/compile-fail/neg_multiply.rs @@ -0,0 +1,40 @@ +#![feature(plugin)] + +#![plugin(clippy)] +#![deny(neg_multiply)] +#![allow(no_effect)] + +use std::ops::Mul; + +struct X; + +impl Mul for X { + type Output = X; + + fn mul(self, _r: isize) -> Self { + self + } +} + +impl Mul for isize { + type Output = X; + + fn mul(self, _r: X) -> X { + X + } +} + +fn main() { + let x = 0; + + x * -1; + //~^ ERROR Negation by multiplying with -1 + + -1 * x; + //~^ ERROR Negation by multiplying with -1 + + -1 * -1; // should be ok + + X * -1; // should be ok + -1 * X; // should also be ok +}