diff --git a/README.md b/README.md index 2c8b20f83..9840eda8d 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 86 lints included in this crate: +There are 87 lints included in this crate: name | default | meaning ---------------------------------------------------------------------------------------------------------|---------|------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ @@ -28,6 +28,7 @@ name [eq_op](https://github.com/Manishearth/rust-clippy/wiki#eq_op) | warn | equal operands on both sides of a comparison or bitwise combination (e.g. `x == x`) [explicit_counter_loop](https://github.com/Manishearth/rust-clippy/wiki#explicit_counter_loop) | warn | for-looping with an explicit counter when `_.enumerate()` would do [explicit_iter_loop](https://github.com/Manishearth/rust-clippy/wiki#explicit_iter_loop) | warn | for-looping over `_.iter()` or `_.iter_mut()` when `&_` or `&mut _` would do +[filter_next](https://github.com/Manishearth/rust-clippy/wiki#filter_next) | warn | using `filter(p).next()`, which is more succinctly expressed as `.find(p)` [float_cmp](https://github.com/Manishearth/rust-clippy/wiki#float_cmp) | warn | using `==` or `!=` on float values (as floating-point operations usually involve rounding errors, it is always better to check for approximate equality within small bounds) [identity_op](https://github.com/Manishearth/rust-clippy/wiki#identity_op) | warn | using identity operations, e.g. `x + 0` or `y / 1` [ineffective_bit_mask](https://github.com/Manishearth/rust-clippy/wiki#ineffective_bit_mask) | warn | expressions where a bit mask will be rendered useless by a comparison, e.g. `(x | 1) > 2` @@ -55,8 +56,8 @@ name [non_ascii_literal](https://github.com/Manishearth/rust-clippy/wiki#non_ascii_literal) | allow | using any literal non-ASCII chars in a string literal; suggests using the \\u escape instead [nonsensical_open_options](https://github.com/Manishearth/rust-clippy/wiki#nonsensical_open_options) | warn | nonsensical combination of options for opening a file [ok_expect](https://github.com/Manishearth/rust-clippy/wiki#ok_expect) | warn | using `ok().expect()`, which gives worse error messages than calling `expect` directly on the Result -[option_map_unwrap_or](https://github.com/Manishearth/rust-clippy/wiki#option_map_unwrap_or) | warn | using `Option.map(f).unwrap_or(a)`, which is more succinctly expressed as `map_or(a, f)`) -[option_map_unwrap_or_else](https://github.com/Manishearth/rust-clippy/wiki#option_map_unwrap_or_else) | warn | using `Option.map(f).unwrap_or_else(g)`, which is more succinctly expressed as `map_or_else(g, f)`) +[option_map_unwrap_or](https://github.com/Manishearth/rust-clippy/wiki#option_map_unwrap_or) | warn | using `Option.map(f).unwrap_or(a)`, which is more succinctly expressed as `map_or(a, f)` +[option_map_unwrap_or_else](https://github.com/Manishearth/rust-clippy/wiki#option_map_unwrap_or_else) | warn | using `Option.map(f).unwrap_or_else(g)`, which is more succinctly expressed as `map_or_else(g, f)` [option_unwrap_used](https://github.com/Manishearth/rust-clippy/wiki#option_unwrap_used) | allow | using `Option.unwrap()`, which should at least get a better message using `expect()` [out_of_bounds_indexing](https://github.com/Manishearth/rust-clippy/wiki#out_of_bounds_indexing) | deny | out of bound constant indexing [panic_params](https://github.com/Manishearth/rust-clippy/wiki#panic_params) | warn | missing parameters in `panic!` diff --git a/src/lib.rs b/src/lib.rs index adc1d402e..4caf008e3 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -176,6 +176,7 @@ pub fn plugin_registrar(reg: &mut Registry) { matches::MATCH_BOOL, matches::MATCH_REF_PATS, matches::SINGLE_MATCH, + methods::FILTER_NEXT, methods::OK_EXPECT, methods::OPTION_MAP_UNWRAP_OR, methods::OPTION_MAP_UNWRAP_OR_ELSE, diff --git a/src/methods.rs b/src/methods.rs index d4809d374..f2f7dbdea 100644 --- a/src/methods.rs +++ b/src/methods.rs @@ -6,7 +6,7 @@ use std::iter; use std::borrow::Cow; use utils::{snippet, span_lint, span_note_and_lint, match_path, match_type, method_chain_args, - walk_ptrs_ty_depth, walk_ptrs_ty}; + match_trait_method, walk_ptrs_ty_depth, walk_ptrs_ty}; use utils::{OPTION_PATH, RESULT_PATH, STRING_PATH}; use utils::MethodArgs; @@ -135,7 +135,7 @@ declare_lint!(pub OK_EXPECT, Warn, /// **Example:** `x.map(|a| a + 1).unwrap_or(0)` declare_lint!(pub OPTION_MAP_UNWRAP_OR, Warn, "using `Option.map(f).unwrap_or(a)`, which is more succinctly expressed as \ - `map_or(a, f)`)"); + `map_or(a, f)`"); /// **What it does:** This lint `Warn`s on `_.map(_).unwrap_or_else(_)`. /// @@ -146,7 +146,17 @@ declare_lint!(pub OPTION_MAP_UNWRAP_OR, Warn, /// **Example:** `x.map(|a| a + 1).unwrap_or_else(some_function)` declare_lint!(pub OPTION_MAP_UNWRAP_OR_ELSE, Warn, "using `Option.map(f).unwrap_or_else(g)`, which is more succinctly expressed as \ - `map_or_else(g, f)`)"); + `map_or_else(g, f)`"); + +/// **What it does:** This lint `Warn`s on `_.filter(_).next()`. +/// +/// **Why is this bad?** Readability, this can be written more concisely as `_.find(_)`. +/// +/// **Known problems:** None. +/// +/// **Example:** `iter.filter(|x| x == 0).next()` +declare_lint!(pub FILTER_NEXT, Warn, + "using `filter(p).next()`, which is more succinctly expressed as `.find(p)`"); impl LintPass for MethodsPass { fn get_lints(&self) -> LintArray { @@ -174,6 +184,9 @@ impl LateLintPass for MethodsPass { else if let Some(arglists) = method_chain_args(expr, &["map", "unwrap_or_else"]) { lint_map_unwrap_or_else(cx, expr, arglists[0], arglists[1]); } + else if let Some(arglists) = method_chain_args(expr, &["filter", "next"]) { + lint_filter_next(cx, expr, arglists[0]); + } } } @@ -331,6 +344,24 @@ fn lint_map_unwrap_or_else(cx: &LateContext, expr: &Expr, unwrap_args: &MethodAr } } +#[allow(ptr_arg)] // Type of MethodArgs is potentially a Vec +/// lint use of `filter().next() for Iterators` +fn lint_filter_next(cx: &LateContext, expr: &Expr, filter_args: &MethodArgs) { + // lint if caller of `.filter().next()` is an Iterator + if match_trait_method(cx, expr, &["core", "iter", "Iterator"]) { + let msg = "called `filter(p).next()` on an Iterator. This is more succinctly expressed by \ + calling `.find(p)` instead."; + let filter_snippet = snippet(cx, filter_args[1].span, ".."); + if filter_snippet.lines().count() <= 1 { // add note if not multi-line + span_note_and_lint(cx, FILTER_NEXT, expr.span, msg, expr.span, + &format!("replace this with `find({})`)", filter_snippet)); + } + else { + span_lint(cx, FILTER_NEXT, expr.span, msg); + } + } +} + // Given a `Result` type, return its error type (`E`) fn get_error_type<'a>(cx: &LateContext, ty: ty::Ty<'a>) -> Option> { if !match_type(cx, ty, &RESULT_PATH) { diff --git a/tests/compile-fail/methods.rs b/tests/compile-fail/methods.rs index 9078a78d6..d2fed0f90 100644 --- a/tests/compile-fail/methods.rs +++ b/tests/compile-fail/methods.rs @@ -83,6 +83,39 @@ fn option_methods() { } +/// Struct to generate false positive for FILTER_NEXT lint +struct FilterNextTest { + _foo: u32, +} + +impl FilterNextTest { + fn filter(self) -> FilterNextTest { + self + } + fn next(self) -> FilterNextTest { + self + } +} + +/// Checks implementation of FILTER_NEXT lint +fn filter_next() { + let v = vec![3, 2, 1, 0, -1, -2, -3]; + + // check single-line case + let _ = v.iter().filter(|&x| *x < 0).next(); //~ERROR called `filter(p).next()` on an Iterator. + //~| NOTE replace this + + // check multi-line case + let _ = v.iter().filter(|&x| { //~ERROR called `filter(p).next()` on an Iterator. + *x < 0 + } + ).next(); + + // check that we don't lint if the caller is not an Iterator + let foo = FilterNextTest { _foo: 0 }; + let _ = foo.filter().next(); +} + fn main() { use std::io;