Skip to content

Commit a1cf2d3

Browse files
rsulli55flip1995
andcommitted
Added a lint as suggested in 6010 which recommends using contains()
instead of `find()` follows by `is_some()` on strings Update clippy_lints/src/find_is_some_on_strs.rs Co-authored-by: Takayuki Nakata <f.seasons017@gmail.com> Update clippy_lints/src/methods/mod.rs Co-authored-by: Philipp Krones <hello@philkrones.com>
1 parent c4fc076 commit a1cf2d3

File tree

4 files changed

+57
-8
lines changed

4 files changed

+57
-8
lines changed

clippy_lints/src/methods/mod.rs

Lines changed: 35 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -515,11 +515,11 @@ declare_clippy_lint! {
515515
}
516516

517517
declare_clippy_lint! {
518-
/// **What it does:** Checks for an iterator search (such as `find()`,
518+
/// **What it does:** Checks for an iterator or string search (such as `find()`,
519519
/// `position()`, or `rposition()`) followed by a call to `is_some()`.
520520
///
521521
/// **Why is this bad?** Readability, this can be written more concisely as
522-
/// `_.any(_)`.
522+
/// `_.any(_)` or `_.contains(_)`.
523523
///
524524
/// **Known problems:** None.
525525
///
@@ -535,7 +535,7 @@ declare_clippy_lint! {
535535
/// ```
536536
pub SEARCH_IS_SOME,
537537
complexity,
538-
"using an iterator search followed by `is_some()`, which is more succinctly expressed as a call to `any()`"
538+
"using an iterator or string search followed by `is_some()`, which is more succinctly expressed as a call to `any()` or `contains()`"
539539
}
540540

541541
declare_clippy_lint! {
@@ -3041,6 +3041,7 @@ fn lint_flat_map_identity<'tcx>(
30413041
}
30423042

30433043
/// lint searching an Iterator followed by `is_some()`
3044+
/// or calling `find()` on a string followed by `is_some()`
30443045
fn lint_search_is_some<'tcx>(
30453046
cx: &LateContext<'tcx>,
30463047
expr: &'tcx hir::Expr<'_>,
@@ -3094,6 +3095,37 @@ fn lint_search_is_some<'tcx>(
30943095
span_lint(cx, SEARCH_IS_SOME, expr.span, &msg);
30953096
}
30963097
}
3098+
// lint if `find()` is called by `String` or `&str`
3099+
else if search_method == "find" {
3100+
let is_string_or_str_slice = |e| {
3101+
let self_ty = cx.typeck_results().expr_ty(e).peel_refs();
3102+
if is_type_diagnostic_item(cx, self_ty, sym!(string_type)) {
3103+
true
3104+
} else {
3105+
*self_ty.kind() == ty::Str
3106+
}
3107+
};
3108+
if_chain! {
3109+
if is_string_or_str_slice(&search_args[0]);
3110+
if is_string_or_str_slice(&search_args[1]);
3111+
then {
3112+
let msg = "called `is_some()` after calling `find()` \
3113+
on a string. This is more succinctly expressed by calling \
3114+
`contains()`.".to_string();
3115+
let mut applicability = Applicability::MachineApplicable;
3116+
let find_arg = snippet_with_applicability(cx, search_args[1].span, "..", &mut applicability);
3117+
span_lint_and_sugg(
3118+
cx,
3119+
SEARCH_IS_SOME,
3120+
method_span.with_hi(expr.span.hi()),
3121+
&msg,
3122+
"try this",
3123+
format!("contains({})", find_arg),
3124+
applicability,
3125+
);
3126+
}
3127+
}
3128+
}
30973129
}
30983130

30993131
/// Used for `lint_binary_expr_with_method_call`.

src/lintlist/mod.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2121,7 +2121,7 @@ vec![
21212121
Lint {
21222122
name: "search_is_some",
21232123
group: "complexity",
2124-
desc: "using an iterator search followed by `is_some()`, which is more succinctly expressed as a call to `any()`",
2124+
desc: "using an iterator or string search followed by `is_some()`, which is more succinctly expressed as a call to `any()` or `contains()`",
21252125
deprecation: None,
21262126
module: "methods",
21272127
},

tests/ui/methods.rs

Lines changed: 21 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -168,8 +168,27 @@ fn search_is_some() {
168168
x < 0
169169
}
170170
).is_some();
171-
172-
// Check that we don't lint if the caller is not an `Iterator`.
171+
172+
let s1 = String::from("hello world");
173+
let s2 = String::from("world");
174+
// Check caller `find()` is a &`static str case
175+
let _ = "hello world".find("world").is_some();
176+
let _ = "hello world".find(&s2).is_some();
177+
let _ = "hello world".find(&s2[2..]).is_some();
178+
// Check caller of `find()` is a String case
179+
let _ = s1.find("world").is_some();
180+
let _ = s1.find(&s2).is_some();
181+
let _ = s1.find(&s2[2..]).is_some();
182+
// Check caller of `find()` is a slice of String case
183+
let _ = s1[2..].find("world").is_some();
184+
let _ = s1[2..].find(&s2).is_some();
185+
let _ = s1[2..].find(&s2[2..]).is_some();
186+
187+
// Check that we don't lint if `find()` is called with
188+
// Pattern that is not a string
189+
let _ = s1.find(|c: char| c == 'o' || c == 'l').is_some();
190+
191+
// Check that we don't lint if the caller is not an `Iterator` or string
173192
let foo = IteratorFalsePositives { foo: 0 };
174193
let _ = foo.find().is_some();
175194
let _ = foo.position().is_some();

tests/ui/methods.stderr

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -88,5 +88,3 @@ LL | | }
8888
LL | | ).is_some();
8989
| |______________________________^
9090

91-
error: aborting due to 11 previous errors
92-

0 commit comments

Comments
 (0)