]> git.lizzy.rs Git - rust.git/blobdiff - clippy_lints/src/methods/mod.rs
Auto merge of #6942 - mgacek8:issue_6815_search_is_none, r=llogiq
[rust.git] / clippy_lints / src / methods / mod.rs
index 6e0ae9416ffa8070b389ea86cd3743d678a2e585..5be28354769ab858f985ed0ba02ba90c231827ca 100644 (file)
 mod zst_offset;
 
 use bind_instead_of_map::BindInsteadOfMap;
+use clippy_utils::diagnostics::{span_lint, span_lint_and_help, span_lint_and_sugg};
 use clippy_utils::source::snippet_with_applicability;
 use clippy_utils::ty::{contains_ty, implements_trait, is_copy, is_type_diagnostic_item};
+use clippy_utils::{
+    contains_return, get_trait_def_id, in_macro, iter_input_pats, match_def_path, match_qpath, method_calls,
+    method_chain_args, paths, return_ty, single_segment_path, SpanlessEq,
+};
 use if_chain::if_chain;
 use rustc_ast::ast;
 use rustc_errors::Applicability;
 use rustc_span::symbol::{sym, SymbolStr};
 use rustc_typeck::hir_ty_to_ty;
 
-use crate::utils::{
-    contains_return, get_trait_def_id, in_macro, iter_input_pats, match_def_path, match_qpath, method_calls,
-    method_chain_args, paths, return_ty, single_segment_path, span_lint, span_lint_and_help, span_lint_and_sugg,
-    SpanlessEq,
-};
-
 declare_clippy_lint! {
     /// **What it does:** Checks for `.unwrap()` calls on `Option`s and on `Result`s.
     ///
     /// **What it does:** Checks for methods with certain name prefixes and which
     /// doesn't match how self is taken. The actual rules are:
     ///
-    /// |Prefix |`self` taken          |
-    /// |-------|----------------------|
-    /// |`as_`  |`&self` or `&mut self`|
-    /// |`from_`| none                 |
-    /// |`into_`|`self`                |
-    /// |`is_`  |`&self` or none       |
-    /// |`to_`  |`&self`               |
+    /// |Prefix |Postfix     |`self` taken           | `self` type  |
+    /// |-------|------------|-----------------------|--------------|
+    /// |`as_`  | none       |`&self` or `&mut self` | any          |
+    /// |`from_`| none       | none                  | any          |
+    /// |`into_`| none       |`self`                 | any          |
+    /// |`is_`  | none       |`&self` or none        | any          |
+    /// |`to_`  | `_mut`     |`&mut self`            | any          |
+    /// |`to_`  | not `_mut` |`self`                 | `Copy`       |
+    /// |`to_`  | not `_mut` |`&self`                | not `Copy`   |
+    ///
+    /// Please find more info here:
+    /// https://rust-lang.github.io/api-guidelines/naming.html#ad-hoc-conversions-follow-as_-to_-into_-conventions-c-conv
     ///
     /// **Why is this bad?** Consistency breeds readability. If you follow the
     /// conventions, your users won't be surprised that they, e.g., need to supply a
 
 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! {
@@ -1716,12 +1725,42 @@ fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx hir::Expr<'_>) {
             ["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"),
@@ -1836,10 +1875,7 @@ fn check_impl_item(&mut self, cx: &LateContext<'tcx>, impl_item: &'tcx hir::Impl
         let item = cx.tcx.hir().expect_item(parent);
         let self_ty = cx.tcx.type_of(item.def_id);
 
-        // if this impl block implements a trait, lint in trait definition instead
-        if let hir::ItemKind::Impl(hir::Impl { of_trait: Some(_), .. }) = item.kind {
-            return;
-        }
+        let implements_trait = matches!(item.kind, hir::ItemKind::Impl(hir::Impl { of_trait: Some(_), .. }));
 
         if_chain! {
             if let hir::ImplItemKind::Fn(ref sig, id) = impl_item.kind;
@@ -1854,7 +1890,8 @@ fn check_impl_item(&mut self, cx: &LateContext<'tcx>, impl_item: &'tcx hir::Impl
             if let Some(first_arg_ty) = first_arg_ty;
 
             then {
-                if cx.access_levels.is_exported(impl_item.hir_id()) {
+                // if this impl block implements a trait, lint in trait definition instead
+                if !implements_trait && cx.access_levels.is_exported(impl_item.hir_id()) {
                     // check missing trait implementations
                     for method_config in &TRAIT_METHODS {
                         if name == method_config.method_name &&
@@ -1890,11 +1927,17 @@ fn check_impl_item(&mut self, cx: &LateContext<'tcx>, impl_item: &'tcx hir::Impl
                     item.vis.node.is_pub(),
                     self_ty,
                     first_arg_ty,
-                    first_arg.pat.span
+                    first_arg.pat.span,
+                    false
                 );
             }
         }
 
+        // if this impl block implements a trait, lint in trait definition instead
+        if implements_trait {
+            return;
+        }
+
         if let hir::ImplItemKind::Fn(_, _) = impl_item.kind {
             let ret_ty = return_ty(cx, impl_item.hir_id());
 
@@ -1946,7 +1989,8 @@ fn check_trait_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx TraitItem<'_>
                     false,
                     self_ty,
                     first_arg_ty,
-                    first_arg_span
+                    first_arg_span,
+                    true
                 );
             }
         }
@@ -2338,10 +2382,10 @@ fn matches_ref<'a>(cx: &LateContext<'a>, mutability: hir::Mutability, parent_ty:
     #[must_use]
     fn description(self) -> &'static str {
         match self {
-            Self::Value => "self by value",
-            Self::Ref => "self by reference",
-            Self::RefMut => "self by mutable reference",
-            Self::No => "no self",
+            Self::Value => "`self` by value",
+            Self::Ref => "`self` by reference",
+            Self::RefMut => "`self` by mutable reference",
+            Self::No => "no `self`",
         }
     }
 }