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>

diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs
index b6fb3d06934ed40a5fcbb011f810813a4b1d9c08..bd04a95e4a193339ba2e700373aa6438a2673d04 100644 (file)
--- a/clippy_lints/src/methods/mod.rs
+++ b/clippy_lints/src/methods/mod.rs
@@ -515,11 +515,11 @@
 }
 
 declare_clippy_lint! {
-    /// **What it does:** Checks for an iterator 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()`.
     ///
     /// **Why is this bad?** Readability, this can be written more concisely as
-    /// `_.any(_)`.
+    /// `_.any(_)` or `_.contains(_)`.
     ///
     /// **Known problems:** None.
     ///
@@ -535,7 +535,7 @@
     /// ```
     pub SEARCH_IS_SOME,
     complexity,
-    "using an iterator search followed by `is_some()`, which is more succinctly expressed as a call to `any()`"
+    "using an iterator or string search followed by `is_some()`, which is more succinctly expressed as a call to `any()` or `contains()`"
 }
 
 declare_clippy_lint! {
@@ -3041,6 +3041,7 @@ fn lint_flat_map_identity<'tcx>(
 }
 
 /// lint searching an Iterator followed by `is_some()`
+/// or calling `find()` on a string followed by `is_some()`
 fn lint_search_is_some<'tcx>(
     cx: &LateContext<'tcx>,
     expr: &'tcx hir::Expr<'_>,
@@ -3094,6 +3095,37 @@ fn lint_search_is_some<'tcx>(
             span_lint(cx, SEARCH_IS_SOME, expr.span, &msg);
         }
     }
+    // lint if `find()` is called by `String` or `&str`
+    else if search_method == "find" {
+        let is_string_or_str_slice = |e| {
+            let self_ty = cx.typeck_results().expr_ty(e).peel_refs();
+            if is_type_diagnostic_item(cx, self_ty, sym!(string_type)) {
+                true
+            } else {
+                *self_ty.kind() == ty::Str
+            }
+        };
+        if_chain! {
+            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. This is more succinctly expressed by calling \
+                    `contains()`.".to_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,
+                    "try this",
+                    format!("contains({})", find_arg),
+                    applicability,
+                );
+            }
+        }
+    }
 }
 
 /// Used for `lint_binary_expr_with_method_call`.

diff --git a/src/lintlist/mod.rs b/src/lintlist/mod.rs
index 4f1b56ed9bee5ca5092ab0639e4a19a5992b6701..69acd3d9b8bc5cefdcfac28a02a2531e4b367dba 100644 (file)
--- a/src/lintlist/mod.rs
+++ b/src/lintlist/mod.rs
@@ -2121,7 +2121,7 @@
     Lint {
         name: "search_is_some",
         group: "complexity",
-        desc: "using an iterator search followed by `is_some()`, which is more succinctly expressed as a call to `any()`",
+        desc: "using an iterator or string search followed by `is_some()`, which is more succinctly expressed as a call to `any()` or `contains()`",
         deprecation: None,
         module: "methods",
     },

diff --git a/tests/ui/methods.rs b/tests/ui/methods.rs
index d93e5b114ecfa7a8933431848efc8a8543f442d8..92ec00a11d27e48a90148fd94f2e6c64f61adca9 100644 (file)
--- a/tests/ui/methods.rs
+++ b/tests/ui/methods.rs
@@ -168,8 +168,27 @@ fn search_is_some() {
                                    x < 0
                                }
                    ).is_some();
-
-    // Check that we don't lint if the caller is not an `Iterator`.
+    
+    let s1 = String::from("hello world");
+    let s2 = String::from("world");
+    // Check caller `find()` is a &`static str case
+    let _ = "hello world".find("world").is_some();
+    let _ = "hello world".find(&s2).is_some();
+    let _ = "hello world".find(&s2[2..]).is_some();
+    // Check caller of `find()` is a String case
+    let _ = s1.find("world").is_some();
+    let _ = s1.find(&s2).is_some();
+    let _ = s1.find(&s2[2..]).is_some();
+    //  Check caller of `find()` is a slice of String case
+    let _ = s1[2..].find("world").is_some();
+    let _ = s1[2..].find(&s2).is_some();
+    let _ = s1[2..].find(&s2[2..]).is_some();
+
+    // Check that we don't lint if `find()` is called with
+    // Pattern that is not a string
+    let _ = s1.find(|c: char| c == 'o' || c == 'l').is_some();
+
+    // Check that we don't lint if the caller is not an `Iterator` or string
     let foo = IteratorFalsePositives { foo: 0 };
     let _ = foo.find().is_some();
     let _ = foo.position().is_some();

diff --git a/tests/ui/methods.stderr b/tests/ui/methods.stderr
index 8a281c2dbd25c2ec3feee082f9735bf89dff00f4..b2b551bd5f868b5e2c6e2cf039e597308634839d 100644 (file)
--- a/tests/ui/methods.stderr
+++ b/tests/ui/methods.stderr
@@ -88,5 +88,3 @@ LL | |                                }
 LL | |                    ).is_some();
    | |______________________________^
 
-error: aborting due to 11 previous errors
-