Auto merge of #7059 - camsteffen:filter-map, r=flip1995

Deprecate `filter_map`

Since #6591, `filter_map` does not even lint `filter().map()`. The cases that are still linted make no sense IMO. So this just removes/deprecates it.

changelog: Deprecate `filter_map` lint

Closes #3424
Fixes #7050

clippy_lints/src/deprecated_lints.rs

@@ -125,3 +125,12 @@ declare_deprecated_lint! {
     pub FIND_MAP,
     "this lint has been replaced by `manual_find_map`, a more specific lint"
 }
+
+declare_deprecated_lint! {
+    /// **What it does:** Nothing. This lint has been deprecated.
+    ///
+    /// **Deprecation reason:** This lint has been replaced by `manual_filter_map`, a
+    /// more specific lint.
+    pub FILTER_MAP,
+    "this lint has been replaced by `manual_filter_map`, a more specific lint"
+}

clippy_lints/src/lib.rs

@@ -509,6 +509,10 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
         "clippy::find_map",
         "this lint has been replaced by `manual_find_map`, a more specific lint",
     );
+    store.register_removed(
+        "clippy::filter_map",
+        "this lint has been replaced by `manual_filter_map`, a more specific lint",
+    );
     // end deprecated lints, do not remove this comment, it’s used in `update_lints`
 
     // begin register lints, do not remove this comment, it’s used in `update_lints`
@@ -760,7 +764,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
         methods::EXPECT_FUN_CALL,
         methods::EXPECT_USED,
         methods::FILETYPE_IS_FILE,
-        methods::FILTER_MAP,
         methods::FILTER_MAP_IDENTITY,
         methods::FILTER_MAP_NEXT,
         methods::FILTER_NEXT,
@@ -1376,7 +1379,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
         LintId::of(matches::MATCH_WILDCARD_FOR_SINGLE_VARIANTS),
         LintId::of(matches::MATCH_WILD_ERR_ARM),
         LintId::of(matches::SINGLE_MATCH_ELSE),
-        LintId::of(methods::FILTER_MAP),
         LintId::of(methods::FILTER_MAP_NEXT),
         LintId::of(methods::IMPLICIT_CLONE),
         LintId::of(methods::INEFFICIENT_TO_STRING),

clippy_lints/src/methods/filter_flat_map.rs

@@ -1,18 +0,0 @@
-use clippy_utils::diagnostics::span_lint_and_help;
-use clippy_utils::is_trait_method;
-use rustc_hir as hir;
-use rustc_lint::LateContext;
-use rustc_span::sym;
-
-use super::FILTER_MAP;
-
-/// lint use of `filter().flat_map()` for `Iterators`
-pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx hir::Expr<'_>) {
-    // lint if caller of `.filter().flat_map()` is an Iterator
-    if is_trait_method(cx, expr, sym::Iterator) {
-        let msg = "called `filter(..).flat_map(..)` on an `Iterator`";
-        let hint = "this is more succinctly expressed by calling `.flat_map(..)` \
-                    and filtering by returning `iter::empty()`";
-        span_lint_and_help(cx, FILTER_MAP, expr.span, msg, None, hint);
-    }
-}

clippy_lints/src/methods/filter_map_flat_map.rs

@@ -1,18 +0,0 @@
-use clippy_utils::diagnostics::span_lint_and_help;
-use clippy_utils::is_trait_method;
-use rustc_hir as hir;
-use rustc_lint::LateContext;
-use rustc_span::sym;
-
-use super::FILTER_MAP;
-
-/// lint use of `filter_map().flat_map()` for `Iterators`
-pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx hir::Expr<'_>) {
-    // lint if caller of `.filter_map().flat_map()` is an Iterator
-    if is_trait_method(cx, expr, sym::Iterator) {
-        let msg = "called `filter_map(..).flat_map(..)` on an `Iterator`";
-        let hint = "this is more succinctly expressed by calling `.flat_map(..)` \
-                    and filtering by returning `iter::empty()`";
-        span_lint_and_help(cx, FILTER_MAP, expr.span, msg, None, hint);
-    }
-}

clippy_lints/src/methods/filter_map_map.rs

@@ -1,17 +0,0 @@
-use clippy_utils::diagnostics::span_lint_and_help;
-use clippy_utils::is_trait_method;
-use rustc_hir as hir;
-use rustc_lint::LateContext;
-use rustc_span::sym;
-
-use super::FILTER_MAP;
-
-/// lint use of `filter_map().map()` for `Iterators`
-pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx hir::Expr<'_>) {
-    // lint if caller of `.filter_map().map()` is an Iterator
-    if is_trait_method(cx, expr, sym::Iterator) {
-        let msg = "called `filter_map(..).map(..)` on an `Iterator`";
-        let hint = "this is more succinctly expressed by only calling `.filter_map(..)` instead";
-        span_lint_and_help(cx, FILTER_MAP, expr.span, msg, None, hint);
-    }
-}

clippy_lints/src/methods/mod.rs

@@ -11,11 +11,8 @@ mod clone_on_ref_ptr;
 mod expect_fun_call;
 mod expect_used;
 mod filetype_is_file;
-mod filter_flat_map;
 mod filter_map;
-mod filter_map_flat_map;
 mod filter_map_identity;
-mod filter_map_map;
 mod filter_map_next;
 mod filter_next;
 mod flat_map_identity;
@@ -472,35 +469,6 @@ declare_clippy_lint! {
     "using combinations of `flatten` and `map` which can usually be written as a single method call"
 }
 
-declare_clippy_lint! {
-    /// **What it does:** Checks for usage of `_.filter(_).map(_)`,
-    /// `_.filter(_).flat_map(_)`, `_.filter_map(_).flat_map(_)` and similar.
-    ///
-    /// **Why is this bad?** Readability, this can be written more concisely as
-    /// `_.filter_map(_)`.
-    ///
-    /// **Known problems:** Often requires a condition + Option/Iterator creation
-    /// inside the closure.
-    ///
-    /// **Example:**
-    /// ```rust
-    /// let vec = vec![1];
-    ///
-    /// // Bad
-    /// vec.iter().filter(|x| **x == 0).map(|x| *x * 2);
-    ///
-    /// // Good
-    /// vec.iter().filter_map(|x| if *x == 0 {
-    ///     Some(*x * 2)
-    /// } else {
-    ///     None
-    /// });
-    /// ```
-    pub FILTER_MAP,
-    pedantic,
-    "using combinations of `filter`, `map`, `filter_map` and `flat_map` which can usually be written as a single method call"
-}
-
 declare_clippy_lint! {
     /// **What it does:** Checks for usage of `_.filter(_).map(_)` that can be written more simply
     /// as `filter_map(_)`.
@@ -1677,7 +1645,6 @@ impl_lint_pass!(Methods => [
     SEARCH_IS_SOME,
     FILTER_NEXT,
     SKIP_WHILE_NEXT,
-    FILTER_MAP,
     FILTER_MAP_IDENTITY,
     MANUAL_FILTER_MAP,
     MANUAL_FIND_MAP,
@@ -1965,11 +1932,7 @@ fn check_methods<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>, msrv: Optio
                 unnecessary_filter_map::check(cx, expr, arg);
                 filter_map_identity::check(cx, expr, arg, span);
             },
-            ("flat_map", [flm_arg]) => match method_call!(recv) {
+            ("flat_map", [flm_arg]) => flat_map_identity::check(cx, expr, flm_arg, span),
-                Some(("filter", [_, _], _)) => filter_flat_map::check(cx, expr),
-                Some(("filter_map", [_, _], _)) => filter_map_flat_map::check(cx, expr),
-                _ => flat_map_identity::check(cx, expr, flm_arg, span),
-            },
             ("flatten", []) => {
                 if let Some(("map", [recv, map_arg], _)) = method_call!(recv) {
                     map_flatten::check(cx, expr, recv, map_arg);
@@ -1993,7 +1956,6 @@ fn check_methods<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>, msrv: Optio
                         ("filter", [f_arg]) => {
                             filter_map::check(cx, expr, recv2, f_arg, span2, recv, m_arg, span, false)
                         },
-                        ("filter_map", [_]) => filter_map_map::check(cx, expr),
                         ("find", [f_arg]) => filter_map::check(cx, expr, recv2, f_arg, span2, recv, m_arg, span, true),
                         _ => {},
                     }

clippy_utils/src/attrs.rs

@@ -151,7 +151,6 @@ pub fn is_proc_macro(sess: &Session, attrs: &[ast::Attribute]) -> bool {
 
 /// Return true if the attributes contain `#[doc(hidden)]`
 pub fn is_doc_hidden(attrs: &[ast::Attribute]) -> bool {
-    #[allow(clippy::filter_map)]
     attrs
         .iter()
         .filter(|attr| attr.has_name(sym::doc))

lintcheck/src/main.rs

@@ -5,7 +5,7 @@
 // When a new lint is introduced, we can search the results for new warnings and check for false
 // positives.
 
-#![allow(clippy::filter_map, clippy::collapsible_else_if)]
+#![allow(clippy::collapsible_else_if)]
 
 use std::ffi::OsStr;
 use std::process::Command;

tests/ui/deprecated.rs

@@ -11,5 +11,6 @@
 #[warn(clippy::panic_params)]
 #[warn(clippy::unknown_clippy_lints)]
 #[warn(clippy::find_map)]
+#[warn(clippy::filter_map)]
 
 fn main() {}

tests/ui/deprecated.stderr

@@ -78,11 +78,17 @@ error: lint `clippy::find_map` has been removed: this lint has been replaced by
 LL | #[warn(clippy::find_map)]
    |        ^^^^^^^^^^^^^^^^
 
+error: lint `clippy::filter_map` has been removed: this lint has been replaced by `manual_filter_map`, a more specific lint
+  --> $DIR/deprecated.rs:14:8
+   |
+LL | #[warn(clippy::filter_map)]
+   |        ^^^^^^^^^^^^^^^^^^
+
 error: lint `clippy::unstable_as_slice` has been removed: `Vec::as_slice` has been stabilized in 1.7
   --> $DIR/deprecated.rs:1:8
    |
 LL | #[warn(clippy::unstable_as_slice)]
    |        ^^^^^^^^^^^^^^^^^^^^^^^^^
 
-error: aborting due to 14 previous errors
+error: aborting due to 15 previous errors
 

tests/ui/filter_methods.rs

@@ -1,25 +0,0 @@
-#![warn(clippy::all, clippy::pedantic)]
-#![allow(clippy::clippy::let_underscore_drop)]
-#![allow(clippy::missing_docs_in_private_items)]
-
-fn main() {
-    let _: Vec<_> = vec![5; 6].into_iter().filter(|&x| x == 0).map(|x| x * 2).collect();
-
-    let _: Vec<_> = vec![5_i8; 6]
-        .into_iter()
-        .filter(|&x| x == 0)
-        .flat_map(|x| x.checked_mul(2))
-        .collect();
-
-    let _: Vec<_> = vec![5_i8; 6]
-        .into_iter()
-        .filter_map(|x| x.checked_mul(2))
-        .flat_map(|x| x.checked_mul(2))
-        .collect();
-
-    let _: Vec<_> = vec![5_i8; 6]
-        .into_iter()
-        .filter_map(|x| x.checked_mul(2))
-        .map(|x| x.checked_mul(2))
-        .collect();
-}

tests/ui/filter_methods.stderr

@@ -1,39 +0,0 @@
-error: called `filter(..).flat_map(..)` on an `Iterator`
-  --> $DIR/filter_methods.rs:8:21
-   |
-LL |       let _: Vec<_> = vec![5_i8; 6]
-   |  _____________________^
-LL | |         .into_iter()
-LL | |         .filter(|&x| x == 0)
-LL | |         .flat_map(|x| x.checked_mul(2))
-   | |_______________________________________^
-   |
-   = note: `-D clippy::filter-map` implied by `-D warnings`
-   = help: this is more succinctly expressed by calling `.flat_map(..)` and filtering by returning `iter::empty()`
-
-error: called `filter_map(..).flat_map(..)` on an `Iterator`
-  --> $DIR/filter_methods.rs:14:21
-   |
-LL |       let _: Vec<_> = vec![5_i8; 6]
-   |  _____________________^
-LL | |         .into_iter()
-LL | |         .filter_map(|x| x.checked_mul(2))
-LL | |         .flat_map(|x| x.checked_mul(2))
-   | |_______________________________________^
-   |
-   = help: this is more succinctly expressed by calling `.flat_map(..)` and filtering by returning `iter::empty()`
-
-error: called `filter_map(..).map(..)` on an `Iterator`
-  --> $DIR/filter_methods.rs:20:21
-   |
-LL |       let _: Vec<_> = vec![5_i8; 6]
-   |  _____________________^
-LL | |         .into_iter()
-LL | |         .filter_map(|x| x.checked_mul(2))
-LL | |         .map(|x| x.checked_mul(2))
-   | |__________________________________^
-   |
-   = help: this is more succinctly expressed by only calling `.filter_map(..)` instead
-
-error: aborting due to 3 previous errors
-