Skip to content

Commit a6bd2d0

Browse files
devonhollowooddevonhollowood
committed
Add SEARCH_IS_SOME lint
1 parent bbd439e commit a6bd2d0

File tree

4 files changed

+102
-8
lines changed

4 files changed

+102
-8
lines changed

‎README.md‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ A collection of lints to catch common mistakes and improve your Rust code.
66
[Jump to usage instructions](#usage)
77

88
##Lints
9-
There are 87 lints included in this crate:
9+
There are 88 lints included in this crate:
1010

1111
name | default | meaning
1212
---------------------------------------------------------------------------------------------------------|---------|------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
@@ -69,6 +69,7 @@ name
6969
[redundant_pattern](https://github.com/Manishearth/rust-clippy/wiki#redundant_pattern) | warn | using `name @ _` in a pattern
7070
[result_unwrap_used](https://github.com/Manishearth/rust-clippy/wiki#result_unwrap_used) | allow | using `Result.unwrap()`, which might be better handled
7171
[reverse_range_loop](https://github.com/Manishearth/rust-clippy/wiki#reverse_range_loop) | warn | Iterating over an empty range, such as `10..0` or `5..5`
72+
[search_is_some](https://github.com/Manishearth/rust-clippy/wiki#search_is_some) | warn | using an iterator search followed by `is_some()`, which is more succinctly expressed as a call to `any()`
7273
[shadow_reuse](https://github.com/Manishearth/rust-clippy/wiki#shadow_reuse) | allow | rebinding a name to an expression that re-uses the original value, e.g. `let x = x + 1`
7374
[shadow_same](https://github.com/Manishearth/rust-clippy/wiki#shadow_same) | allow | rebinding a name to itself, e.g. `let mut x = &mut x`
7475
[shadow_unrelated](https://github.com/Manishearth/rust-clippy/wiki#shadow_unrelated) | allow | The name is re-bound without even using the original value

‎src/lib.rs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -180,6 +180,7 @@ pub fn plugin_registrar(reg: &mut Registry) {
180180
methods::OK_EXPECT,
181181
methods::OPTION_MAP_UNWRAP_OR,
182182
methods::OPTION_MAP_UNWRAP_OR_ELSE,
183+
methods::SEARCH_IS_SOME,
183184
methods::SHOULD_IMPLEMENT_TRAIT,
184185
methods::STR_TO_STRING,
185186
methods::STRING_TO_STRING,

‎src/methods.rs‎

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -158,6 +158,18 @@ declare_lint!(pub OPTION_MAP_UNWRAP_OR_ELSE, Warn,
158158
declare_lint!(pub FILTER_NEXT, Warn,
159159
"using `filter(p).next()`, which is more succinctly expressed as `.find(p)`");
160160

161+
/// **What it does:** This lint `Warn`s on an iterator search (such as `find()`, `position()`, or
162+
/// `rposition()`) followed by a call to `is_some()`.
163+
///
164+
/// **Why is this bad?** Readability, this can be written more concisely as `_.any(_)`.
165+
///
166+
/// **Known problems:** None.
167+
///
168+
/// **Example:** `iter.find(|x| x == 0).is_some()`
169+
declare_lint!(pub SEARCH_IS_SOME, Warn,
170+
"using an iterator search followed by `is_some()`, which is more succinctly \
171+
expressed as a call to `any()`");
172+
161173
impl LintPass for MethodsPass {
162174
fn get_lints(&self) -> LintArray {
163175
lint_array!(OPTION_UNWRAP_USED, RESULT_UNWRAP_USED, STR_TO_STRING, STRING_TO_STRING,
@@ -187,6 +199,15 @@ impl LateLintPass for MethodsPass {
187199
else if let Some(arglists) = method_chain_args(expr, &["filter", "next"]) {
188200
lint_filter_next(cx, expr, arglists[0]);
189201
}
202+
else if let Some(arglists) = method_chain_args(expr, &["find", "is_some"]) {
203+
lint_search_is_some(cx, expr, "find", arglists[0], arglists[1]);
204+
}
205+
else if let Some(arglists) = method_chain_args(expr, &["position", "is_some"]) {
206+
lint_search_is_some(cx, expr, "position", arglists[0], arglists[1]);
207+
}
208+
else if let Some(arglists) = method_chain_args(expr, &["rposition", "is_some"]) {
209+
lint_search_is_some(cx, expr, "rposition", arglists[0], arglists[1]);
210+
}
190211
}
191212
}
192213

@@ -362,6 +383,25 @@ fn lint_filter_next(cx: &LateContext, expr: &Expr, filter_args: &MethodArgs) {
362383
}
363384
}
364385

386+
#[allow(ptr_arg)] // Type of MethodArgs is potentially a Vec
387+
/// lint searching an Iterator followed by `is_some()`
388+
fn lint_search_is_some(cx: &LateContext, expr: &Expr, search_method: &str, search_args: &MethodArgs,
389+
is_some_args: &MethodArgs) {
390+
// lint if caller of search is an Iterator
391+
if match_trait_method(cx, &*is_some_args[0], &["core", "iter", "Iterator"]) {
392+
let msg = format!("called `is_some()` after searching an iterator with {}. This is more \
393+
succinctly expressed by calling `any()`.", search_method);
394+
let search_snippet = snippet(cx, search_args[1].span, "..");
395+
if search_snippet.lines().count() <= 1 { // add note if not multi-line
396+
span_note_and_lint(cx, SEARCH_IS_SOME, expr.span, &msg, expr.span,
397+
&format!("replace this with `any({})`)", search_snippet));
398+
}
399+
else {
400+
span_lint(cx, SEARCH_IS_SOME, expr.span, &msg);
401+
}
402+
}
403+
}
404+
365405
// Given a `Result<T, E>` type, return its error type (`E`)
366406
fn get_error_type<'a>(cx: &LateContext, ty: ty::Ty<'a>) -> Option<ty::Ty<'a>> {
367407
if !match_type(cx, ty, &RESULT_PATH) {

‎tests/compile-fail/methods.rs‎

Lines changed: 59 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -83,18 +83,32 @@ fn option_methods() {
8383

8484
}
8585

86-
/// Struct to generate false positive for FILTER_NEXT lint
87-
struct FilterNextTest {
88-
_foo: u32,
86+
/// Struct to generate false positive for Iterator-based lints
87+
#[derive(Copy, Clone)]
88+
struct IteratorFalsePositives {
89+
foo: u32,
8990
}
9091

91-
impl FilterNextTest {
92-
fn filter(self) -> FilterNextTest {
92+
impl IteratorFalsePositives {
93+
fn filter(self) -> IteratorFalsePositives {
9394
self
9495
}
95-
fn next(self) -> FilterNextTest {
96+
97+
fn next(self) -> IteratorFalsePositives {
9698
self
9799
}
100+
101+
fn find(self) -> Option<u32> {
102+
Some(self.foo)
103+
}
104+
105+
fn position(self) -> Option<u32> {
106+
Some(self.foo)
107+
}
108+
109+
fn rposition(self) -> Option<u32> {
110+
Some(self.foo)
111+
}
98112
}
99113

100114
/// Checks implementation of FILTER_NEXT lint
@@ -112,10 +126,48 @@ fn filter_next() {
112126
).next();
113127

114128
// check that we don't lint if the caller is not an Iterator
115-
let foo = FilterNextTest { _foo: 0 };
129+
let foo = IteratorFalsePositives { foo: 0 };
116130
let _ = foo.filter().next();
117131
}
118132

133+
/// Checks implementation of SEARCH_IS_SOME lint
134+
fn search_is_some() {
135+
let v = vec![3, 2, 1, 0, -1, -2, -3];
136+
137+
// check `find().is_some()`, single-line
138+
let _ = v.iter().find(|&x| *x < 0).is_some(); //~ERROR called `is_some()` after searching
139+
//~| NOTE replace this
140+
// check `find().is_some()`, multi-line
141+
let _ = v.iter().find(|&x| { //~ERROR called `is_some()` after searching
142+
*x < 0
143+
}
144+
).is_some();
145+
146+
// check `position().is_some()`, single-line
147+
let _ = v.iter().position(|&x| x < 0).is_some(); //~ERROR called `is_some()` after searching
148+
//~| NOTE replace this
149+
// check `position().is_some()`, multi-line
150+
let _ = v.iter().position(|&x| { //~ERROR called `is_some()` after searching
151+
x < 0
152+
}
153+
).is_some();
154+
155+
// check `rposition().is_some()`, single-line
156+
let _ = v.iter().rposition(|&x| x < 0).is_some(); //~ERROR called `is_some()` after searching
157+
//~| NOTE replace this
158+
// check `rposition().is_some()`, multi-line
159+
let _ = v.iter().rposition(|&x| { //~ERROR called `is_some()` after searching
160+
x < 0
161+
}
162+
).is_some();
163+
164+
// check that we don't lint if the caller is not an Iterator
165+
let foo = IteratorFalsePositives { foo: 0 };
166+
let _ = foo.find().is_some();
167+
let _ = foo.position().is_some();
168+
let _ = foo.rposition().is_some();
169+
}
170+
119171
fn main() {
120172
use std::io;
121173

0 commit comments

Comments
 (0)