search_is_some: check also when search is none

This commit is contained in:
Mateusz Gacek 2021-03-20 14:30:45 +01:00
parent 1d3c539fbb
commit 2ffee89b75
7 changed files with 359 additions and 48 deletions

View file

@ -588,26 +588,31 @@ declare_clippy_lint! {
declare_clippy_lint! { declare_clippy_lint! {
/// **What it does:** Checks for an iterator or string search (such as `find()`, /// **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 /// **Why is this bad?** Readability, this can be written more concisely as:
/// `_.any(_)` or `_.contains(_)`. /// * `_.any(_)`, or `_.contains(_)` for `is_some()`,
/// * `!_.any(_)`, or `!_.contains(_)` for `is_none()`.
/// ///
/// **Known problems:** None. /// **Known problems:** None.
/// ///
/// **Example:** /// **Example:**
/// ```rust /// ```rust
/// # let vec = vec![1]; /// let vec = vec![1];
/// vec.iter().find(|x| **x == 0).is_some(); /// vec.iter().find(|x| **x == 0).is_some();
///
/// let _ = "hello world".find("world").is_none();
/// ``` /// ```
/// Could be written as /// Could be written as
/// ```rust /// ```rust
/// # let vec = vec![1]; /// let vec = vec![1];
/// vec.iter().any(|x| *x == 0); /// vec.iter().any(|x| *x == 0);
///
/// let _ = !"hello world".contains("world");
/// ``` /// ```
pub SEARCH_IS_SOME, pub SEARCH_IS_SOME,
complexity, 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! { 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", "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]), ["flat_map", ..] => flat_map_identity::check(cx, expr, arg_lists[0], method_spans[0]),
["flatten", "map"] => map_flatten::check(cx, expr, arg_lists[1]), ["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]), [option_check_method, "find"] if "is_some" == *option_check_method || "is_none" == *option_check_method => {
["is_some", "position"] => { search_is_some::check(
search_is_some::check(cx, expr, "position", arg_lists[1], arg_lists[0], method_spans[1]) cx,
expr,
"find",
option_check_method,
arg_lists[1],
arg_lists[0],
method_spans[1],
)
}, },
["is_some", "rposition"] => { [option_check_method, "position"]
search_is_some::check(cx, expr, "rposition", arg_lists[1], arg_lists[0], method_spans[1]) 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]), ["extend", ..] => string_extend_chars::check(cx, expr, arg_lists[0]),
["count", "into_iter"] => iter_count::check(cx, expr, &arg_lists[1], "into_iter"), ["count", "into_iter"] => iter_count::check(cx, expr, &arg_lists[1], "into_iter"),

View file

@ -14,11 +14,13 @@ use rustc_span::symbol::sym;
use super::SEARCH_IS_SOME; use super::SEARCH_IS_SOME;
/// lint searching an Iterator followed by `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>( pub(super) fn check<'tcx>(
cx: &LateContext<'tcx>, cx: &LateContext<'tcx>,
expr: &'tcx hir::Expr<'_>, expr: &'tcx hir::Expr<'_>,
search_method: &str, search_method: &str,
option_check_method: &str,
search_args: &'tcx [hir::Expr<'_>], search_args: &'tcx [hir::Expr<'_>],
is_some_args: &'tcx [hir::Expr<'_>], is_some_args: &'tcx [hir::Expr<'_>],
method_span: Span, method_span: Span,
@ -26,10 +28,9 @@ pub(super) fn check<'tcx>(
// lint if caller of search is an Iterator // lint if caller of search is an Iterator
if is_trait_method(cx, &is_some_args[0], sym::Iterator) { if is_trait_method(cx, &is_some_args[0], sym::Iterator) {
let msg = format!( let msg = format!(
"called `is_some()` after searching an `Iterator` with `{}`", "called `{}()` after searching an `Iterator` with `{}`",
search_method option_check_method, search_method
); );
let hint = "this is more succinctly expressed by calling `any()`";
let search_snippet = snippet(cx, search_args[1].span, ".."); let search_snippet = snippet(cx, search_args[1].span, "..");
if search_snippet.lines().count() <= 1 { if search_snippet.lines().count() <= 1 {
// suggest `any(|x| ..)` instead of `any(|&x| ..)` for `find(|&x| ..).is_some()` // suggest `any(|x| ..)` instead of `any(|&x| ..)` for `find(|&x| ..).is_some()`
@ -53,6 +54,8 @@ pub(super) fn check<'tcx>(
} }
}; };
// add note if not multi-line // add note if not multi-line
match option_check_method {
"is_some" => {
span_lint_and_sugg( span_lint_and_sugg(
cx, cx,
SEARCH_IS_SOME, SEARCH_IS_SOME,
@ -65,8 +68,35 @@ pub(super) fn check<'tcx>(
), ),
Applicability::MachineApplicable, 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 { } 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` // 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[0]);
if is_string_or_str_slice(&search_args[1]); if is_string_or_str_slice(&search_args[1]);
then { then {
let msg = "called `is_some()` after calling `find()` on a string"; 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 mut applicability = Applicability::MachineApplicable;
let find_arg = snippet_with_applicability(cx, search_args[1].span, "..", &mut applicability); let find_arg = snippet_with_applicability(cx, search_args[1].span, "..", &mut applicability);
span_lint_and_sugg( span_lint_and_sugg(
cx, cx,
SEARCH_IS_SOME, SEARCH_IS_SOME,
method_span.with_hi(expr.span.hi()), method_span.with_hi(expr.span.hi()),
msg, &msg,
"use `contains()` instead", "use `contains()` instead",
format!("contains({})", find_arg), format!("contains({})", find_arg),
applicability, 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,
);
},
_ => (),
}
} }
} }
} }

View file

@ -1,8 +1,9 @@
// aux-build:option_helpers.rs // aux-build:option_helpers.rs
#![warn(clippy::search_is_some)]
#![allow(dead_code)]
extern crate option_helpers; extern crate option_helpers;
use option_helpers::IteratorFalsePositives; use option_helpers::IteratorFalsePositives;
#[warn(clippy::search_is_some)]
#[rustfmt::skip] #[rustfmt::skip]
fn main() { fn main() {
let v = vec![3, 2, 1, 0, -1, -2, -3]; let v = vec![3, 2, 1, 0, -1, -2, -3];
@ -36,3 +37,37 @@ fn main() {
// `Pattern` that is not a string // `Pattern` that is not a string
let _ = "hello world".find(|c: char| c == 'o' || c == 'l').is_some(); 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();
}

View file

@ -1,5 +1,5 @@
error: called `is_some()` after searching an `Iterator` with `find` 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| { LL | let _ = v.iter().find(|&x| {
| _____________^ | _____________^
@ -12,7 +12,7 @@ LL | | ).is_some();
= help: this is more succinctly expressed by calling `any()` = help: this is more succinctly expressed by calling `any()`
error: called `is_some()` after searching an `Iterator` with `position` 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| { LL | let _ = v.iter().position(|&x| {
| _____________^ | _____________^
@ -24,7 +24,7 @@ LL | | ).is_some();
= help: this is more succinctly expressed by calling `any()` = help: this is more succinctly expressed by calling `any()`
error: called `is_some()` after searching an `Iterator` with `rposition` 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| { LL | let _ = v.iter().rposition(|&x| {
| _____________^ | _____________^
@ -35,5 +35,41 @@ LL | | ).is_some();
| |
= help: this is more succinctly expressed by calling `any()` = 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

View file

@ -1,5 +1,5 @@
// run-rustfix // run-rustfix
#![allow(dead_code)]
#![warn(clippy::search_is_some)] #![warn(clippy::search_is_some)]
fn main() { fn main() {
@ -33,3 +33,36 @@ fn main() {
let _ = s1[2..].contains(&s2); let _ = s1[2..].contains(&s2);
let _ = s1[2..].contains(&s2[2..]); 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..]);
}

View file

@ -1,5 +1,5 @@
// run-rustfix // run-rustfix
#![allow(dead_code)]
#![warn(clippy::search_is_some)] #![warn(clippy::search_is_some)]
fn main() { fn main() {
@ -33,3 +33,36 @@ fn main() {
let _ = s1[2..].find(&s2).is_some(); let _ = s1[2..].find(&s2).is_some();
let _ = s1[2..].find(&s2[2..]).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();
}

View file

@ -90,5 +90,95 @@ error: called `is_some()` after calling `find()` on a string
LL | let _ = s1[2..].find(&s2[2..]).is_some(); LL | let _ = s1[2..].find(&s2[2..]).is_some();
| ^^^^^^^^^^^^^^^^^^^^^^^^ help: use `contains()` instead: `contains(&s2[2..])` | ^^^^^^^^^^^^^^^^^^^^^^^^ 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