From 70e34077d590136fc1096ea03c887393a1246c93 Mon Sep 17 00:00:00 2001 From: Andre Bogus Date: Tue, 22 Aug 2017 23:45:08 +0200 Subject: [PATCH] new lint: naive_bytecount --- CHANGELOG.md | 3 ++ README.md | 3 +- clippy_lints/src/bytecount.rs | 80 +++++++++++++++++++++++++++++++++ clippy_lints/src/lib.rs | 3 ++ clippy_lints/src/utils/paths.rs | 1 + tests/ui/bytecount.rs | 14 ++++++ tests/ui/bytecount.stderr | 16 +++++++ 7 files changed, 119 insertions(+), 1 deletion(-) create mode 100644 clippy_lints/src/bytecount.rs create mode 100644 tests/ui/bytecount.rs create mode 100644 tests/ui/bytecount.stderr diff --git a/CHANGELOG.md b/CHANGELOG.md index e9cc471a1..7b72a6c1b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,8 @@ # Change Log All notable changes to this project will be documented in this file. +* New lint: [`naive_bytecount`] + ## 0.0.154 * Fix [`use_self`] triggering inside derives * Add support for linting an entire workspace with `cargo clippy --all` @@ -516,6 +518,7 @@ All notable changes to this project will be documented in this file. [`mut_mut`]: https://github.com/rust-lang-nursery/rust-clippy/wiki#mut_mut [`mutex_atomic`]: https://github.com/rust-lang-nursery/rust-clippy/wiki#mutex_atomic [`mutex_integer`]: https://github.com/rust-lang-nursery/rust-clippy/wiki#mutex_integer +[`naive_bytecount`]: https://github.com/rust-lang-nursery/rust-clippy/wiki#naive_bytecount [`needless_bool`]: https://github.com/rust-lang-nursery/rust-clippy/wiki#needless_bool [`needless_borrow`]: https://github.com/rust-lang-nursery/rust-clippy/wiki#needless_borrow [`needless_borrowed_reference`]: https://github.com/rust-lang-nursery/rust-clippy/wiki#needless_borrowed_reference diff --git a/README.md b/README.md index bad6d01b6..bc043ec62 100644 --- a/README.md +++ b/README.md @@ -180,7 +180,7 @@ transparently: ## Lints -There are 205 lints included in this crate: +There are 206 lints included in this crate: name | default | triggers on -----------------------------------------------------------------------------------------------------------------------------|---------|---------------------------------------------------------------------------------------------------------------------------------- @@ -290,6 +290,7 @@ name [mut_mut](https://github.com/rust-lang-nursery/rust-clippy/wiki#mut_mut) | allow | usage of double-mut refs, e.g. `&mut &mut ...` [mutex_atomic](https://github.com/rust-lang-nursery/rust-clippy/wiki#mutex_atomic) | warn | using a mutex where an atomic value could be used instead [mutex_integer](https://github.com/rust-lang-nursery/rust-clippy/wiki#mutex_integer) | allow | using a mutex for an integer type +[naive_bytecount](https://github.com/rust-lang-nursery/rust-clippy/wiki#naive_bytecount) | warn | use of naive `.filter(|&x| x == y).count()` to count byte values [needless_bool](https://github.com/rust-lang-nursery/rust-clippy/wiki#needless_bool) | warn | if-statements with plain booleans in the then- and else-clause, e.g. `if p { true } else { false }` [needless_borrow](https://github.com/rust-lang-nursery/rust-clippy/wiki#needless_borrow) | warn | taking a reference that is going to be automatically dereferenced [needless_borrowed_reference](https://github.com/rust-lang-nursery/rust-clippy/wiki#needless_borrowed_reference) | warn | taking a needless borrowed reference diff --git a/clippy_lints/src/bytecount.rs b/clippy_lints/src/bytecount.rs new file mode 100644 index 000000000..99987f84b --- /dev/null +++ b/clippy_lints/src/bytecount.rs @@ -0,0 +1,80 @@ +use consts::{constant, Constant}; +use rustc_const_math::ConstInt; +use rustc::hir::*; +use rustc::lint::*; +use utils::{match_type, paths, snippet, span_lint_and_sugg, walk_ptrs_ty}; + +/// **What it does:** Checks for naive byte counts +/// +/// **Why is this bad?** The [`bytecount`](https://crates.io/crates/bytecount) +/// crate has methods to count your bytes faster, especially for large slices. +/// +/// **Known problems:** If you have predominantly small slices, the +/// `bytecount::count(..)` method may actually be slower. However, if you can +/// ensure that less than 2³²-1 matches arise, the `naive_count_32(..)` can be +/// faster in those cases. +/// +/// **Example:** +/// +/// ```rust +/// &my_data.filter(|&x| x == 0u8).count() // use bytecount::count instead +/// ``` +declare_lint! { + pub NAIVE_BYTECOUNT, + Warn, + "use of naive `.filter(|&x| x == y).count()` to count byte values" +} + +#[derive(Copy, Clone)] +pub struct ByteCount; + +impl LintPass for ByteCount { + fn get_lints(&self) -> LintArray { + lint_array!(NAIVE_BYTECOUNT) + } +} + +impl<'a, 'tcx> LateLintPass<'a, 'tcx> for ByteCount { + fn check_expr(&mut self, cx: &LateContext, expr: &Expr) { + if_let_chain!([ + let ExprMethodCall(ref count, _, ref count_args) = expr.node, + count.name == "count", + count_args.len() == 1, + let ExprMethodCall(ref filter, _, ref filter_args) = count_args[0].node, + filter.name == "filter", + filter_args.len() == 2, + let ExprClosure(_, _, body_id, _) = filter_args[1].node, + ], { + let body = cx.tcx.hir.body(body_id); + if_let_chain!([ + let ExprBinary(ref op, ref l, ref r) = body.value.node, + op.node == BiEq, + match_type(cx, + walk_ptrs_ty(cx.tables.expr_ty(&filter_args[0])), + &paths::SLICE_ITER), + let Some((Constant::Int(ConstInt::U8(needle)), _)) = + constant(cx, l).or_else(|| constant(cx, r)) + ], { + let haystack = if let ExprMethodCall(ref path, _, ref args) = + filter_args[0].node { + let p = path.name; + if (p == "iter" || p == "iter_mut") && args.len() == 1 { + &args[0] + } else { + &filter_args[0] + } + } else { + &filter_args[0] + }; + span_lint_and_sugg(cx, + NAIVE_BYTECOUNT, + expr.span, + "You appear to be counting bytes the naive way", + "Consider using the bytecount crate", + format!("bytecount::count({}, {})", + snippet(cx, haystack.span, ".."), + needle)); + }); + }); + } +} diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index dc1b14735..c4c949c79 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -74,6 +74,7 @@ pub mod bit_mask; pub mod blacklisted_name; pub mod block_in_if_condition; pub mod booleans; +pub mod bytecount; pub mod collapsible_if; pub mod copies; pub mod cyclomatic_complexity; @@ -321,6 +322,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry) { reg.register_late_lint_pass(box needless_pass_by_value::NeedlessPassByValue); reg.register_early_lint_pass(box literal_digit_grouping::LiteralDigitGrouping); reg.register_late_lint_pass(box use_self::UseSelf); + reg.register_late_lint_pass(box bytecount::ByteCount); reg.register_lint_group("clippy_restrictions", vec![ arithmetic::FLOAT_ARITHMETIC, @@ -388,6 +390,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry) { block_in_if_condition::BLOCK_IN_IF_CONDITION_EXPR, block_in_if_condition::BLOCK_IN_IF_CONDITION_STMT, booleans::LOGIC_BUG, + bytecount::NAIVE_BYTECOUNT, collapsible_if::COLLAPSIBLE_IF, copies::IF_SAME_THEN_ELSE, copies::IFS_SAME_COND, diff --git a/clippy_lints/src/utils/paths.rs b/clippy_lints/src/utils/paths.rs index 675d70878..060cf4f97 100644 --- a/clippy_lints/src/utils/paths.rs +++ b/clippy_lints/src/utils/paths.rs @@ -69,6 +69,7 @@ pub const RESULT_ERR: [&'static str; 4] = ["core", "result", "Result", "Err"]; pub const RESULT_OK: [&'static str; 4] = ["core", "result", "Result", "Ok"]; pub const SERDE_DE_VISITOR: [&'static str; 3] = ["serde", "de", "Visitor"]; pub const SLICE_INTO_VEC: [&'static str; 4] = ["alloc", "slice", "", "into_vec"]; +pub const SLICE_ITER: [&str; 3] = ["core", "slice", "Iter"]; pub const STRING: [&'static str; 3] = ["alloc", "string", "String"]; pub const TO_OWNED: [&'static str; 3] = ["alloc", "borrow", "ToOwned"]; pub const TO_STRING: [&'static str; 3] = ["alloc", "string", "ToString"]; diff --git a/tests/ui/bytecount.rs b/tests/ui/bytecount.rs new file mode 100644 index 000000000..880a86eba --- /dev/null +++ b/tests/ui/bytecount.rs @@ -0,0 +1,14 @@ +#![feature(plugin)] +#![plugin(clippy)] + +fn main() { + let x = vec![0_u8; 16]; + + let _ = x.iter().filter(|&&a| a == 0).count(); // naive byte count + + let _ = (&x[..]).iter().filter(|&a| *a == 0).count(); // naive byte count + + let _ = x.iter().filter(|a| **a > 0).count(); // not an equality count, OK. + + let _ = x.iter().map(|a| a + 1).filter(|&a| a < 15).count(); // not a slice +} diff --git a/tests/ui/bytecount.stderr b/tests/ui/bytecount.stderr new file mode 100644 index 000000000..818fa7eab --- /dev/null +++ b/tests/ui/bytecount.stderr @@ -0,0 +1,16 @@ +error: You appear to be counting bytes the naive way + --> $DIR/bytecount.rs:7:13 + | +7 | let _ = x.iter().filter(|&&a| a == 0).count(); // naive byte count + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: Consider using the bytecount crate: `bytecount::count(x, 0)` + | + = note: `-D naive-bytecount` implied by `-D warnings` + +error: You appear to be counting bytes the naive way + --> $DIR/bytecount.rs:9:13 + | +9 | let _ = (&x[..]).iter().filter(|&a| *a == 0).count(); // naive byte count + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: Consider using the bytecount crate: `bytecount::count((&x[..]), 0)` + +error: aborting due to 2 previous errors +