Extend manual_filter to cover and_then

clippy_lints/src/matches/manual_filter.rs

@@ -1,12 +1,15 @@
 use clippy_utils::as_some_expr;
-use clippy_utils::diagnostics::span_lint_and_sugg;
+use clippy_utils::diagnostics::{span_lint_and_sugg, span_lint_and_then};
 use clippy_utils::res::{MaybeDef, MaybeQPath, MaybeResPath};
+use clippy_utils::source::snippet_with_context;
+use clippy_utils::sugg::Sugg;
+use clippy_utils::ty::is_copy;
 use clippy_utils::visitors::contains_unsafe_block;
-
+use rustc_errors::Applicability;
 use rustc_hir::LangItem::OptionNone;
 use rustc_hir::{Arm, Expr, ExprKind, HirId, Pat, PatKind};
 use rustc_lint::LateContext;
-use rustc_span::{SyntaxContext, sym};
+use rustc_span::{Span, SyntaxContext, sym};
 
 use super::MANUAL_FILTER;
 use super::manual_utils::{SomeExpr, check_with};
@@ -41,12 +44,11 @@ fn get_cond_expr<'tcx>(
 fn peels_blocks_incl_unsafe_opt<'a>(expr: &'a Expr<'a>) -> Option<&'a Expr<'a>> {
     // we don't want to use `peel_blocks` here because we don't care if the block is unsafe, it's
     // checked by `contains_unsafe_block`
-    if let ExprKind::Block(block, None) = expr.kind
-        && block.stmts.is_empty()
-    {
-        return block.expr;
+    if let ExprKind::Block(block, None) = expr.kind {
+        return if block.stmts.is_empty() { block.expr } else { None };
     }
-    None
+
+    Some(expr)
 }
 
 fn peels_blocks_incl_unsafe<'a>(expr: &'a Expr<'a>) -> &'a Expr<'a> {
@@ -89,6 +91,62 @@ fn add_ampersand_if_copy(body_str: String, has_copy_trait: bool) -> String {
     }
 }
 
+pub(crate) fn check_method_call<'tcx>(
+    cx: &LateContext<'tcx>,
+    scrutinee: &'tcx Expr<'_>,
+    arg: &'tcx Expr<'_>,
+    call_span: Span,
+    expr: &'tcx Expr<'_>,
+) {
+    let ty = cx.typeck_results().expr_ty(scrutinee).peel_refs();
+    if ty.is_diag_item(cx, sym::Option)
+        && let ExprKind::Closure(closure) = arg.kind
+        && let body = cx.tcx.hir_body(closure.body)
+        && let Some(fn_arg_span) = closure.fn_arg_span
+        && let [param] = body.params
+        && let expr_span_ctxt = expr.span.ctxt()
+        && let Some(some_expr) = get_cond_expr(cx, param.pat, body.value, expr_span_ctxt)
+    {
+        span_lint_and_then(
+            cx,
+            MANUAL_FILTER,
+            call_span,
+            "manual implementation of `Option::filter`",
+            |diag| {
+                let mut applicability = Applicability::MachineApplicable;
+
+                let mut cond_snip =
+                    Sugg::hir_with_context(cx, some_expr.expr, expr_span_ctxt, "..", &mut applicability);
+                if some_expr.needs_unsafe_block {
+                    cond_snip = cond_snip.unsafeify();
+                }
+                if some_expr.needs_negated {
+                    cond_snip = !cond_snip;
+                }
+
+                let (prefix_snip, _) = snippet_with_context(
+                    cx,
+                    closure.fn_decl_span.shrink_to_lo().until(fn_arg_span),
+                    expr_span_ctxt,
+                    "..",
+                    &mut applicability,
+                );
+                let (param_snip, _) =
+                    snippet_with_context(cx, param.pat.span, expr_span_ctxt, "..", &mut applicability);
+                diag.span_suggestion(
+                    call_span,
+                    "try",
+                    format!(
+                        "filter({prefix_snip}|{}{param_snip}| {cond_snip})",
+                        if is_copy(cx, ty) { "&" } else { "" }
+                    ),
+                    applicability,
+                );
+            },
+        );
+    }
+}
+
 pub(super) fn check_match<'tcx>(
     cx: &LateContext<'tcx>,
     scrutinee: &'tcx Expr<'_>,

clippy_lints/src/matches/mod.rs

@@ -1,6 +1,6 @@
 mod collapsible_match;
 mod infallible_destructuring_match;
-mod manual_filter;
+pub(crate) mod manual_filter;
 mod manual_map;
 mod manual_ok_err;
 mod manual_unwrap_or;

clippy_lints/src/methods/mod.rs

@@ -163,6 +163,8 @@ use rustc_middle::ty::TraitRef;
 use rustc_session::impl_lint_pass;
 use rustc_span::{Span, Symbol};
 
+use crate::matches::manual_filter;
+
 declare_clippy_lint! {
     /// ### What it does
     /// Checks for usage of `cloned()` on an `Iterator` or `Option` where
@@ -5089,6 +5091,8 @@ impl Methods {
                             return_and_then::check(cx, expr, recv, arg);
                         }
                     }
+
+                    manual_filter::check_method_call(cx, recv, arg, call_span, expr);
                 },
                 (sym::any, [arg]) => {
                     needless_character_iteration::check(cx, expr, recv, arg, false);

clippy_utils/src/sugg.rs

@@ -335,6 +335,11 @@ impl<'a> Sugg<'a> {
         Sugg::NonParen(Cow::Owned(format!("{{ {self} }}")))
     }
 
+    /// Convenience method to wrap the expression in an `unsafe` block.
+    pub fn unsafeify(self) -> Sugg<'static> {
+        Sugg::NonParen(Cow::Owned(format!("unsafe {{ {self} }}")))
+    }
+
     /// Convenience method to prefix the expression with the `async` keyword.
     /// Can be used after `blockify` to create an async block.
     pub fn asyncify(self) -> Sugg<'static> {

tests/ui/bind_instead_of_map.fixed

@@ -1,4 +1,5 @@
 #![deny(clippy::bind_instead_of_map)]
+#![allow(clippy::manual_filter)]
 
 // need a main anyway, use it get rid of unused warnings too
 pub fn main() {

tests/ui/bind_instead_of_map.rs

@@ -1,4 +1,5 @@
 #![deny(clippy::bind_instead_of_map)]
+#![allow(clippy::manual_filter)]
 
 // need a main anyway, use it get rid of unused warnings too
 pub fn main() {

tests/ui/bind_instead_of_map.stderr

@@ -1,5 +1,5 @@
 error: using `Option.and_then(Some)`, which is a no-op
-  --> tests/ui/bind_instead_of_map.rs:7:13
+  --> tests/ui/bind_instead_of_map.rs:8:13
    |
 LL |     let _ = x.and_then(Some);
    |             ^^^^^^^^^^^^^^^^ help: use the expression directly: `x`
@@ -11,13 +11,13 @@ LL | #![deny(clippy::bind_instead_of_map)]
    |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
 error: using `Option.and_then(|x| Some(y))`, which is more succinctly expressed as `map(|x| y)`
-  --> tests/ui/bind_instead_of_map.rs:9:13
+  --> tests/ui/bind_instead_of_map.rs:10:13
    |
 LL |     let _ = x.and_then(|o| Some(o + 1));
    |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `x.map(|o| o + 1)`
 
 error: using `Result.and_then(Ok)`, which is a no-op
-  --> tests/ui/bind_instead_of_map.rs:16:13
+  --> tests/ui/bind_instead_of_map.rs:17:13
    |
 LL |     let _ = x.and_then(Ok);
    |             ^^^^^^^^^^^^^^ help: use the expression directly: `x`

tests/ui/if_then_some_else_none.fixed

@@ -1,5 +1,9 @@
 #![warn(clippy::if_then_some_else_none)]
-#![allow(clippy::redundant_pattern_matching, clippy::unnecessary_lazy_evaluations)]
+#![allow(
+    clippy::redundant_pattern_matching,
+    clippy::unnecessary_lazy_evaluations,
+    clippy::manual_filter
+)]
 
 fn main() {
     // Should issue an error.

tests/ui/if_then_some_else_none.rs

@@ -1,5 +1,9 @@
 #![warn(clippy::if_then_some_else_none)]
-#![allow(clippy::redundant_pattern_matching, clippy::unnecessary_lazy_evaluations)]
+#![allow(
+    clippy::redundant_pattern_matching,
+    clippy::unnecessary_lazy_evaluations,
+    clippy::manual_filter
+)]
 
 fn main() {
     // Should issue an error.

tests/ui/if_then_some_else_none.stderr

@@ -1,5 +1,5 @@
 error: this could be simplified with `bool::then`
-  --> tests/ui/if_then_some_else_none.rs:6:13
+  --> tests/ui/if_then_some_else_none.rs:10:13
    |
 LL |       let _ = if foo() {
    |  _____________^
@@ -24,7 +24,7 @@ LL ~     });
    |
 
 error: this could be simplified with `bool::then`
-  --> tests/ui/if_then_some_else_none.rs:16:13
+  --> tests/ui/if_then_some_else_none.rs:20:13
    |
 LL |       let _ = if matches!(true, true) {
    |  _____________^
@@ -47,19 +47,19 @@ LL ~     });
    |
 
 error: this could be simplified with `bool::then_some`
-  --> tests/ui/if_then_some_else_none.rs:27:28
+  --> tests/ui/if_then_some_else_none.rs:31:28
    |
 LL |     let _ = x.and_then(|o| if o < 32 { Some(o) } else { None });
    |                            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `(o < 32).then_some(o)`
 
 error: this could be simplified with `bool::then_some`
-  --> tests/ui/if_then_some_else_none.rs:32:13
+  --> tests/ui/if_then_some_else_none.rs:36:13
    |
 LL |     let _ = if !x { Some(0) } else { None };
    |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `(!x).then_some(0)`
 
 error: this could be simplified with `bool::then`
-  --> tests/ui/if_then_some_else_none.rs:88:13
+  --> tests/ui/if_then_some_else_none.rs:92:13
    |
 LL |       let _ = if foo() {
    |  _____________^
@@ -82,13 +82,13 @@ LL ~     });
    |
 
 error: this could be simplified with `bool::then`
-  --> tests/ui/if_then_some_else_none.rs:138:5
+  --> tests/ui/if_then_some_else_none.rs:142:5
    |
 LL |     if s == "1" { Some(true) } else { None }
    |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `(s == "1").then(|| true)`
 
 error: this could be simplified with `bool::then`
-  --> tests/ui/if_then_some_else_none.rs:154:9
+  --> tests/ui/if_then_some_else_none.rs:158:9
    |
 LL | /         if rs.len() == 1 && rs[0].start == rs[0].end {
 LL | |
@@ -99,7 +99,7 @@ LL | |         }
    | |_________^ help: try: `(rs.len() == 1 && rs[0].start == rs[0].end).then(|| vec![rs[0].start])`
 
 error: this could be simplified with `bool::then_some`
-  --> tests/ui/if_then_some_else_none.rs:164:9
+  --> tests/ui/if_then_some_else_none.rs:168:9
    |
 LL | /         if modulo == 0 {
 LL | |
@@ -110,7 +110,7 @@ LL | |         }
    | |_________^ help: try: `(modulo == 0).then_some(i)`
 
 error: this could be simplified with `bool::then_some`
-  --> tests/ui/if_then_some_else_none.rs:181:22
+  --> tests/ui/if_then_some_else_none.rs:185:22
    |
 LL |           do_something(if i % 2 == 0 {
    |  ______________________^
@@ -122,7 +122,7 @@ LL | |         });
    | |_________^ help: try: `(i % 2 == 0).then_some(item_fn)`
 
 error: this could be simplified with `bool::then_some`
-  --> tests/ui/if_then_some_else_none.rs:198:22
+  --> tests/ui/if_then_some_else_none.rs:202:22
    |
 LL |           do_something(if i % 2 == 0 {
    |  ______________________^
@@ -134,7 +134,7 @@ LL | |         });
    | |_________^ help: try: `(i % 2 == 0).then_some(item_fn)`
 
 error: this could be simplified with `bool::then_some`
-  --> tests/ui/if_then_some_else_none.rs:206:22
+  --> tests/ui/if_then_some_else_none.rs:210:22
    |
 LL |           do_something(if i % 2 == 0 {
    |  ______________________^
@@ -146,7 +146,7 @@ LL | |         });
    | |_________^ help: try: `(i % 2 == 0).then_some(closure_fn)`
 
 error: this could be simplified with `bool::then`
-  --> tests/ui/if_then_some_else_none.rs:231:13
+  --> tests/ui/if_then_some_else_none.rs:235:13
    |
 LL | /             if self.count < 5 {
 LL | |                 self.count += 1;
@@ -165,7 +165,7 @@ LL +             })
    |
 
 error: this could be simplified with `bool::then`
-  --> tests/ui/if_then_some_else_none.rs:249:13
+  --> tests/ui/if_then_some_else_none.rs:253:13
    |
 LL |       let _ = if true {
    |  _____________^
@@ -185,7 +185,7 @@ LL ~     });
    |
 
 error: this could be simplified with `bool::then`
-  --> tests/ui/if_then_some_else_none.rs:292:5
+  --> tests/ui/if_then_some_else_none.rs:296:5
    |
 LL | /     if 1 <= 3 {
 LL | |         let a = UnsafeCell::new(1);

tests/ui/manual_filter.fixed

@@ -1,5 +1,5 @@
 #![warn(clippy::manual_filter)]
-#![allow(unused_variables, dead_code, clippy::useless_vec)]
+#![allow(unused_variables, dead_code, clippy::useless_vec, clippy::nonminimal_bool)]
 
 fn main() {
     Some(0).filter(|&x| x <= 0);
@@ -156,3 +156,16 @@ fn main() {
 fn maybe_some() -> Option<u32> {
     Some(0)
 }
+
+fn issue14440(opt: Option<i32>) {
+    opt.filter(|&x| x != 0);
+    //~^ manual_filter
+
+    let y = 1i32;
+    opt.filter(move |&x| x == y);
+    //~^ manual_filter
+
+    let opt1 = Some("123".to_string());
+    opt1.filter(|s| s.len() > 2);
+    //~^ manual_filter
+}

tests/ui/manual_filter.rs

@@ -1,5 +1,5 @@
 #![warn(clippy::manual_filter)]
-#![allow(unused_variables, dead_code, clippy::useless_vec)]
+#![allow(unused_variables, dead_code, clippy::useless_vec, clippy::nonminimal_bool)]
 
 fn main() {
     match Some(0) {
@@ -293,3 +293,16 @@ fn main() {
 fn maybe_some() -> Option<u32> {
     Some(0)
 }
+
+fn issue14440(opt: Option<i32>) {
+    opt.and_then(|x| if x == 0 { None } else { Some(x) });
+    //~^ manual_filter
+
+    let y = 1i32;
+    opt.and_then(move |x| if x == y { Some(x) } else { None });
+    //~^ manual_filter
+
+    let opt1 = Some("123".to_string());
+    opt1.and_then(|s| if s.len() > 2 { Some(s) } else { None });
+    //~^ manual_filter
+}

tests/ui/manual_filter.stderr

@@ -189,5 +189,23 @@ LL | |         None
 LL | |     };
    | |_____^ help: try: `{ Some(16).filter(|&x| x % 2 == 0) }`
 
-error: aborting due to 15 previous errors
+error: manual implementation of `Option::filter`
+  --> tests/ui/manual_filter.rs:298:9
+   |
+LL |     opt.and_then(|x| if x == 0 { None } else { Some(x) });
+   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `filter(|&x| x != 0)`
+
+error: manual implementation of `Option::filter`
+  --> tests/ui/manual_filter.rs:302:9
+   |
+LL |     opt.and_then(move |x| if x == y { Some(x) } else { None });
+   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `filter(move |&x| x == y)`
+
+error: manual implementation of `Option::filter`
+  --> tests/ui/manual_filter.rs:306:10
+   |
+LL |     opt1.and_then(|s| if s.len() > 2 { Some(s) } else { None });
+   |          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `filter(|s| s.len() > 2)`
+
+error: aborting due to 18 previous errors
 

tests/ui/return_and_then.fixed

@@ -1,4 +1,5 @@
 #![warn(clippy::return_and_then)]
+#![allow(clippy::manual_filter)]
 
 fn main() {
     fn test_opt_block(opt: Option<i32>) -> Option<i32> {

tests/ui/return_and_then.rs

@@ -1,4 +1,5 @@
 #![warn(clippy::return_and_then)]
+#![allow(clippy::manual_filter)]
 
 fn main() {
     fn test_opt_block(opt: Option<i32>) -> Option<i32> {