diff options
| -rw-r--r-- | clippy_lints/src/methods/mod.rs | 57 | ||||
| -rw-r--r-- | clippy_lints/src/methods/search_is_some.rs | 107 | ||||
| -rw-r--r-- | tests/ui/search_is_some.rs | 37 | ||||
| -rw-r--r-- | tests/ui/search_is_some.stderr | 44 | ||||
| -rw-r--r-- | tests/ui/search_is_some_fixable.fixed | 35 | ||||
| -rw-r--r-- | tests/ui/search_is_some_fixable.rs | 35 | ||||
| -rw-r--r-- | tests/ui/search_is_some_fixable.stderr | 92 |
7 files changed, 359 insertions, 48 deletions
diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs index 46deb20f97d..5be28354769 100644 --- a/clippy_lints/src/methods/mod.rs +++ b/clippy_lints/src/methods/mod.rs @@ -588,26 +588,31 @@ declare_clippy_lint! { declare_clippy_lint! { /// **What it does:** Checks for an iterator or string search (such as `find()`, - /// `position()`, or `rposition()`) followed by a call to `is_some()`. + /// `position()`, or `rposition()`) followed by a call to `is_some()` or `is_none()`. /// - /// **Why is this bad?** Readability, this can be written more concisely as - /// `_.any(_)` or `_.contains(_)`. + /// **Why is this bad?** Readability, this can be written more concisely as: + /// * `_.any(_)`, or `_.contains(_)` for `is_some()`, + /// * `!_.any(_)`, or `!_.contains(_)` for `is_none()`. /// /// **Known problems:** None. /// /// **Example:** /// ```rust - /// # let vec = vec![1]; + /// let vec = vec![1]; /// vec.iter().find(|x| **x == 0).is_some(); + /// + /// let _ = "hello world".find("world").is_none(); /// ``` /// Could be written as /// ```rust - /// # let vec = vec![1]; + /// let vec = vec![1]; /// vec.iter().any(|x| *x == 0); + /// + /// let _ = !"hello world".contains("world"); /// ``` pub SEARCH_IS_SOME, complexity, - "using an iterator or string search followed by `is_some()`, which is more succinctly expressed as a call to `any()` or `contains()`" + "using an iterator or string search followed by `is_some()` or `is_none()`, which is more succinctly expressed as a call to `any()` or `contains()` (with negation in case of `is_none()`)" } declare_clippy_lint! { @@ -1720,12 +1725,42 @@ impl<'tcx> LateLintPass<'tcx> for Methods { ["flat_map", "filter_map"] => filter_map_flat_map::check(cx, expr, arg_lists[1], arg_lists[0]), ["flat_map", ..] => flat_map_identity::check(cx, expr, arg_lists[0], method_spans[0]), ["flatten", "map"] => map_flatten::check(cx, expr, arg_lists[1]), - ["is_some", "find"] => search_is_some::check(cx, expr, "find", arg_lists[1], arg_lists[0], method_spans[1]), - ["is_some", "position"] => { - search_is_some::check(cx, expr, "position", arg_lists[1], arg_lists[0], method_spans[1]) + [option_check_method, "find"] if "is_some" == *option_check_method || "is_none" == *option_check_method => { + search_is_some::check( + cx, + expr, + "find", + option_check_method, + arg_lists[1], + arg_lists[0], + method_spans[1], + ) }, - ["is_some", "rposition"] => { - search_is_some::check(cx, expr, "rposition", arg_lists[1], arg_lists[0], method_spans[1]) + [option_check_method, "position"] + if "is_some" == *option_check_method || "is_none" == *option_check_method => + { + search_is_some::check( + cx, + expr, + "position", + option_check_method, + arg_lists[1], + arg_lists[0], + method_spans[1], + ) + }, + [option_check_method, "rposition"] + if "is_some" == *option_check_method || "is_none" == *option_check_method => + { + search_is_some::check( + cx, + expr, + "rposition", + option_check_method, + arg_lists[1], + arg_lists[0], + method_spans[1], + ) }, ["extend", ..] => string_extend_chars::check(cx, expr, arg_lists[0]), ["count", "into_iter"] => iter_count::check(cx, expr, &arg_lists[1], "into_iter"), diff --git a/clippy_lints/src/methods/search_is_some.rs b/clippy_lints/src/methods/search_is_some.rs index 13546dc1779..de7d168295f 100644 --- a/clippy_lints/src/methods/search_is_some.rs +++ b/clippy_lints/src/methods/search_is_some.rs @@ -14,11 +14,13 @@ use rustc_span::symbol::sym; use super::SEARCH_IS_SOME; /// lint searching an Iterator followed by `is_some()` -/// or calling `find()` on a string followed by `is_some()` +/// or calling `find()` on a string followed by `is_some()` or `is_none()` +#[allow(clippy::too_many_lines)] pub(super) fn check<'tcx>( cx: &LateContext<'tcx>, expr: &'tcx hir::Expr<'_>, search_method: &str, + option_check_method: &str, search_args: &'tcx [hir::Expr<'_>], is_some_args: &'tcx [hir::Expr<'_>], method_span: Span, @@ -26,10 +28,9 @@ pub(super) fn check<'tcx>( // lint if caller of search is an Iterator if is_trait_method(cx, &is_some_args[0], sym::Iterator) { let msg = format!( - "called `is_some()` after searching an `Iterator` with `{}`", - search_method + "called `{}()` after searching an `Iterator` with `{}`", + option_check_method, search_method ); - let hint = "this is more succinctly expressed by calling `any()`"; let search_snippet = snippet(cx, search_args[1].span, ".."); if search_snippet.lines().count() <= 1 { // suggest `any(|x| ..)` instead of `any(|&x| ..)` for `find(|&x| ..).is_some()` @@ -53,20 +54,49 @@ pub(super) fn check<'tcx>( } }; // add note if not multi-line - span_lint_and_sugg( - cx, - SEARCH_IS_SOME, - method_span.with_hi(expr.span.hi()), - &msg, - "use `any()` instead", - format!( - "any({})", - any_search_snippet.as_ref().map_or(&*search_snippet, String::as_str) - ), - Applicability::MachineApplicable, - ); + match option_check_method { + "is_some" => { + span_lint_and_sugg( + cx, + SEARCH_IS_SOME, + method_span.with_hi(expr.span.hi()), + &msg, + "use `any()` instead", + format!( + "any({})", + any_search_snippet.as_ref().map_or(&*search_snippet, String::as_str) + ), + Applicability::MachineApplicable, + ); + }, + "is_none" => { + let iter = snippet(cx, search_args[0].span, ".."); + span_lint_and_sugg( + cx, + SEARCH_IS_SOME, + expr.span, + &msg, + "use `!_.any()` instead", + format!( + "!{}.any({})", + iter, + any_search_snippet.as_ref().map_or(&*search_snippet, String::as_str) + ), + Applicability::MachineApplicable, + ); + }, + _ => (), + } } else { - span_lint_and_help(cx, SEARCH_IS_SOME, expr.span, &msg, None, hint); + let hint = format!( + "this is more succinctly expressed by calling `any()`{}", + if option_check_method == "is_none" { + " with negation" + } else { + "" + } + ); + span_lint_and_help(cx, SEARCH_IS_SOME, expr.span, &msg, None, &hint); } } // lint if `find()` is called by `String` or `&str` @@ -83,18 +113,37 @@ pub(super) fn check<'tcx>( if is_string_or_str_slice(&search_args[0]); if is_string_or_str_slice(&search_args[1]); then { - let msg = "called `is_some()` after calling `find()` on a string"; - let mut applicability = Applicability::MachineApplicable; - let find_arg = snippet_with_applicability(cx, search_args[1].span, "..", &mut applicability); - span_lint_and_sugg( - cx, - SEARCH_IS_SOME, - method_span.with_hi(expr.span.hi()), - msg, - "use `contains()` instead", - format!("contains({})", find_arg), - applicability, - ); + let msg = format!("called `{}()` after calling `find()` on a string", option_check_method); + match option_check_method { + "is_some" => { + let mut applicability = Applicability::MachineApplicable; + let find_arg = snippet_with_applicability(cx, search_args[1].span, "..", &mut applicability); + span_lint_and_sugg( + cx, + SEARCH_IS_SOME, + method_span.with_hi(expr.span.hi()), + &msg, + "use `contains()` instead", + format!("contains({})", find_arg), + applicability, + ); + }, + "is_none" => { + let string = snippet(cx, search_args[0].span, ".."); + let mut applicability = Applicability::MachineApplicable; + let find_arg = snippet_with_applicability(cx, search_args[1].span, "..", &mut applicability); + span_lint_and_sugg( + cx, + SEARCH_IS_SOME, + expr.span, + &msg, + "use `!_.contains()` instead", + format!("!{}.contains({})", string, find_arg), + applicability, + ); + }, + _ => (), + } } } } diff --git a/tests/ui/search_is_some.rs b/tests/ui/search_is_some.rs index f0dc3b3d06b..72bc6ef35d3 100644 --- a/tests/ui/search_is_some.rs +++ b/tests/ui/search_is_some.rs @@ -1,8 +1,9 @@ // aux-build:option_helpers.rs +#![warn(clippy::search_is_some)] +#![allow(dead_code)] extern crate option_helpers; use option_helpers::IteratorFalsePositives; -#[warn(clippy::search_is_some)] #[rustfmt::skip] fn main() { let v = vec![3, 2, 1, 0, -1, -2, -3]; @@ -36,3 +37,37 @@ fn main() { // `Pattern` that is not a string let _ = "hello world".find(|c: char| c == 'o' || c == 'l').is_some(); } + +#[rustfmt::skip] +fn is_none() { + let v = vec![3, 2, 1, 0, -1, -2, -3]; + let y = &&42; + + + // Check `find().is_none()`, multi-line case. + let _ = v.iter().find(|&x| { + *x < 0 + } + ).is_none(); + + // Check `position().is_none()`, multi-line case. + let _ = v.iter().position(|&x| { + x < 0 + } + ).is_none(); + + // Check `rposition().is_none()`, multi-line case. + let _ = v.iter().rposition(|&x| { + x < 0 + } + ).is_none(); + + // Check that we don't lint if the caller is not an `Iterator` or string + let falsepos = IteratorFalsePositives { foo: 0 }; + let _ = falsepos.find().is_none(); + let _ = falsepos.position().is_none(); + let _ = falsepos.rposition().is_none(); + // check that we don't lint if `find()` is called with + // `Pattern` that is not a string + let _ = "hello world".find(|c: char| c == 'o' || c == 'l').is_none(); +} diff --git a/tests/ui/search_is_some.stderr b/tests/ui/search_is_some.stderr index c601f568c60..f3c758e451e 100644 --- a/tests/ui/search_is_some.stderr +++ b/tests/ui/search_is_some.stderr @@ -1,5 +1,5 @@ error: called `is_some()` after searching an `Iterator` with `find` - --> $DIR/search_is_some.rs:13:13 + --> $DIR/search_is_some.rs:14:13 | LL | let _ = v.iter().find(|&x| { | _____________^ @@ -12,7 +12,7 @@ LL | | ).is_some(); = help: this is more succinctly expressed by calling `any()` error: called `is_some()` after searching an `Iterator` with `position` - --> $DIR/search_is_some.rs:19:13 + --> $DIR/search_is_some.rs:20:13 | LL | let _ = v.iter().position(|&x| { | _____________^ @@ -24,7 +24,7 @@ LL | | ).is_some(); = help: this is more succinctly expressed by calling `any()` error: called `is_some()` after searching an `Iterator` with `rposition` - --> $DIR/search_is_some.rs:25:13 + --> $DIR/search_is_some.rs:26:13 | LL | let _ = v.iter().rposition(|&x| { | _____________^ @@ -35,5 +35,41 @@ LL | | ).is_some(); | = help: this is more succinctly expressed by calling `any()` -error: aborting due to 3 previous errors +error: called `is_none()` after searching an `Iterator` with `find` + --> $DIR/search_is_some.rs:48:13 + | +LL | let _ = v.iter().find(|&x| { + | _____________^ +LL | | *x < 0 +LL | | } +LL | | ).is_none(); + | |______________________________^ + | + = help: this is more succinctly expressed by calling `any()` with negation + +error: called `is_none()` after searching an `Iterator` with `position` + --> $DIR/search_is_some.rs:54:13 + | +LL | let _ = v.iter().position(|&x| { + | _____________^ +LL | | x < 0 +LL | | } +LL | | ).is_none(); + | |______________________________^ + | + = help: this is more succinctly expressed by calling `any()` with negation + +error: called `is_none()` after searching an `Iterator` with `rposition` + --> $DIR/search_is_some.rs:60:13 + | +LL | let _ = v.iter().rposition(|&x| { + | _____________^ +LL | | x < 0 +LL | | } +LL | | ).is_none(); + | |______________________________^ + | + = help: this is more succinctly expressed by calling `any()` with negation + +error: aborting due to 6 previous errors diff --git a/tests/ui/search_is_some_fixable.fixed b/tests/ui/search_is_some_fixable.fixed index dc3f290e562..62ff16f67f4 100644 --- a/tests/ui/search_is_some_fixable.fixed +++ b/tests/ui/search_is_some_fixable.fixed @@ -1,5 +1,5 @@ // run-rustfix - +#![allow(dead_code)] #![warn(clippy::search_is_some)] fn main() { @@ -33,3 +33,36 @@ fn main() { let _ = s1[2..].contains(&s2); let _ = s1[2..].contains(&s2[2..]); } + +fn is_none() { + let v = vec![3, 2, 1, 0, -1, -2, -3]; + let y = &&42; + + // Check `find().is_none()`, single-line case. + let _ = !v.iter().any(|x| *x < 0); + let _ = !(0..1).any(|x| **y == x); // one dereference less + let _ = !(0..1).any(|x| x == 0); + let _ = !v.iter().any(|x| *x == 0); + + // Check `position().is_none()`, single-line case. + let _ = !v.iter().any(|&x| x < 0); + + // Check `rposition().is_none()`, single-line case. + let _ = !v.iter().any(|&x| x < 0); + + let s1 = String::from("hello world"); + let s2 = String::from("world"); + + // caller of `find()` is a `&`static str` + let _ = !"hello world".contains("world"); + let _ = !"hello world".contains(&s2); + let _ = !"hello world".contains(&s2[2..]); + // caller of `find()` is a `String` + let _ = !s1.contains("world"); + let _ = !s1.contains(&s2); + let _ = !s1.contains(&s2[2..]); + // caller of `find()` is slice of `String` + let _ = !s1[2..].contains("world"); + let _ = !s1[2..].contains(&s2); + let _ = !s1[2..].contains(&s2[2..]); +} diff --git a/tests/ui/search_is_some_fixable.rs b/tests/ui/search_is_some_fixable.rs index 146cf5adf1b..8407f716647 100644 --- a/tests/ui/search_is_some_fixable.rs +++ b/tests/ui/search_is_some_fixable.rs @@ -1,5 +1,5 @@ // run-rustfix - +#![allow(dead_code)] #![warn(clippy::search_is_some)] fn main() { @@ -33,3 +33,36 @@ fn main() { let _ = s1[2..].find(&s2).is_some(); let _ = s1[2..].find(&s2[2..]).is_some(); } + +fn is_none() { + let v = vec![3, 2, 1, 0, -1, -2, -3]; + let y = &&42; + + // Check `find().is_none()`, single-line case. + let _ = v.iter().find(|&x| *x < 0).is_none(); + let _ = (0..1).find(|x| **y == *x).is_none(); // one dereference less + let _ = (0..1).find(|x| *x == 0).is_none(); + let _ = v.iter().find(|x| **x == 0).is_none(); + + // Check `position().is_none()`, single-line case. + let _ = v.iter().position(|&x| x < 0).is_none(); + + // Check `rposition().is_none()`, single-line case. + let _ = v.iter().rposition(|&x| x < 0).is_none(); + + let s1 = String::from("hello world"); + let s2 = String::from("world"); + + // caller of `find()` is a `&`static str` + let _ = "hello world".find("world").is_none(); + let _ = "hello world".find(&s2).is_none(); + let _ = "hello world".find(&s2[2..]).is_none(); + // caller of `find()` is a `String` + let _ = s1.find("world").is_none(); + let _ = s1.find(&s2).is_none(); + let _ = s1.find(&s2[2..]).is_none(); + // caller of `find()` is slice of `String` + let _ = s1[2..].find("world").is_none(); + let _ = s1[2..].find(&s2).is_none(); + let _ = s1[2..].find(&s2[2..]).is_none(); +} diff --git a/tests/ui/search_is_some_fixable.stderr b/tests/ui/search_is_some_fixable.stderr index 23c1d9a901b..bd1b6955a97 100644 --- a/tests/ui/search_is_some_fixable.stderr +++ b/tests/ui/search_is_some_fixable.stderr @@ -90,5 +90,95 @@ error: called `is_some()` after calling `find()` on a string LL | let _ = s1[2..].find(&s2[2..]).is_some(); | ^^^^^^^^^^^^^^^^^^^^^^^^ help: use `contains()` instead: `contains(&s2[2..])` -error: aborting due to 15 previous errors +error: called `is_none()` after searching an `Iterator` with `find` + --> $DIR/search_is_some_fixable.rs:42:13 + | +LL | let _ = v.iter().find(|&x| *x < 0).is_none(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `!_.any()` instead: `!v.iter().any(|x| *x < 0)` + +error: called `is_none()` after searching an `Iterator` with `find` + --> $DIR/search_is_some_fixable.rs:43:13 + | +LL | let _ = (0..1).find(|x| **y == *x).is_none(); // one dereference less + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `!_.any()` instead: `!(0..1).any(|x| **y == x)` + +error: called `is_none()` after searching an `Iterator` with `find` + --> $DIR/search_is_some_fixable.rs:44:13 + | +LL | let _ = (0..1).find(|x| *x == 0).is_none(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `!_.any()` instead: `!(0..1).any(|x| x == 0)` + +error: called `is_none()` after searching an `Iterator` with `find` + --> $DIR/search_is_some_fixable.rs:45:13 + | +LL | let _ = v.iter().find(|x| **x == 0).is_none(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `!_.any()` instead: `!v.iter().any(|x| *x == 0)` + +error: called `is_none()` after searching an `Iterator` with `position` + --> $DIR/search_is_some_fixable.rs:48:13 + | +LL | let _ = v.iter().position(|&x| x < 0).is_none(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `!_.any()` instead: `!v.iter().any(|&x| x < 0)` + +error: called `is_none()` after searching an `Iterator` with `rposition` + --> $DIR/search_is_some_fixable.rs:51:13 + | +LL | let _ = v.iter().rposition(|&x| x < 0).is_none(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `!_.any()` instead: `!v.iter().any(|&x| x < 0)` + +error: called `is_none()` after calling `find()` on a string + --> $DIR/search_is_some_fixable.rs:57:13 + | +LL | let _ = "hello world".find("world").is_none(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `!_.contains()` instead: `!"hello world".contains("world")` + +error: called `is_none()` after calling `find()` on a string + --> $DIR/search_is_some_fixable.rs:58:13 + | +LL | let _ = "hello world".find(&s2).is_none(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `!_.contains()` instead: `!"hello world".contains(&s2)` + +error: called `is_none()` after calling `find()` on a string + --> $DIR/search_is_some_fixable.rs:59:13 + | +LL | let _ = "hello world".find(&s2[2..]).is_none(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `!_.contains()` instead: `!"hello world".contains(&s2[2..])` + +error: called `is_none()` after calling `find()` on a string + --> $DIR/search_is_some_fixable.rs:61:13 + | +LL | let _ = s1.find("world").is_none(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `!_.contains()` instead: `!s1.contains("world")` + +error: called `is_none()` after calling `find()` on a string + --> $DIR/search_is_some_fixable.rs:62:13 + | +LL | let _ = s1.find(&s2).is_none(); + | ^^^^^^^^^^^^^^^^^^^^^^ help: use `!_.contains()` instead: `!s1.contains(&s2)` + +error: called `is_none()` after calling `find()` on a string + --> $DIR/search_is_some_fixable.rs:63:13 + | +LL | let _ = s1.find(&s2[2..]).is_none(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `!_.contains()` instead: `!s1.contains(&s2[2..])` + +error: called `is_none()` after calling `find()` on a string + --> $DIR/search_is_some_fixable.rs:65:13 + | +LL | let _ = s1[2..].find("world").is_none(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `!_.contains()` instead: `!s1[2..].contains("world")` + +error: called `is_none()` after calling `find()` on a string + --> $DIR/search_is_some_fixable.rs:66:13 + | +LL | let _ = s1[2..].find(&s2).is_none(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `!_.contains()` instead: `!s1[2..].contains(&s2)` + +error: called `is_none()` after calling `find()` on a string + --> $DIR/search_is_some_fixable.rs:67:13 + | +LL | let _ = s1[2..].find(&s2[2..]).is_none(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `!_.contains()` instead: `!s1[2..].contains(&s2[2..])` + +error: aborting due to 30 previous errors |