about summary refs log tree commit diff
diff options
context:
space:
mode:
authorkraktus <kraktus@users.noreply.github.com>2022-10-03 13:57:42 +0200
committerkraktus <kraktus@users.noreply.github.com>2022-09-10 10:41:55 +0200
commit0958f9486ba0cb8ecd0093ff3100af45d19529f3 (patch)
tree74da20733241a4eb4aa2fd7acdcbe100d0479140
parent18e10ca29092c01115e0771e0d6753ab9dfb0ff1 (diff)
downloadrust-0958f9486ba0cb8ecd0093ff3100af45d19529f3.tar.gz
rust-0958f9486ba0cb8ecd0093ff3100af45d19529f3.zip
Add `manual_filter` lint for `Option`
Share much of its implementation with `manual_map` and should greatly benefit from its previous feedback.
-rw-r--r--CHANGELOG.md1
-rw-r--r--clippy_lints/src/lib.register_all.rs1
-rw-r--r--clippy_lints/src/lib.register_complexity.rs1
-rw-r--r--clippy_lints/src/lib.register_lints.rs1
-rw-r--r--clippy_lints/src/matches/manual_filter.rs181
-rw-r--r--clippy_lints/src/matches/manual_map.rs213
-rw-r--r--clippy_lints/src/matches/mod.rs39
-rw-r--r--clippy_utils/src/sugg.rs6
-rw-r--r--src/docs.rs1
-rw-r--r--src/docs/manual_filter.txt21
-rw-r--r--tests/ui/manual_filter.fixed119
-rw-r--r--tests/ui/manual_filter.rs243
-rw-r--r--tests/ui/manual_filter.stderr191
13 files changed, 951 insertions, 67 deletions
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 9f12a673596..cab2808ee3b 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -3986,6 +3986,7 @@ Released 2018-09-13
 [`manual_async_fn`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_async_fn
 [`manual_bits`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_bits
 [`manual_clamp`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_clamp
+[`manual_filter`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_filter
 [`manual_filter_map`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_filter_map
 [`manual_find`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_find
 [`manual_find_map`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_find_map
diff --git a/clippy_lints/src/lib.register_all.rs b/clippy_lints/src/lib.register_all.rs
index 9ad2a88eb26..21d686fcdf5 100644
--- a/clippy_lints/src/lib.register_all.rs
+++ b/clippy_lints/src/lib.register_all.rs
@@ -135,6 +135,7 @@ store.register_group(true, "clippy::all", Some("clippy_all"), vec![
     LintId::of(match_result_ok::MATCH_RESULT_OK),
     LintId::of(matches::COLLAPSIBLE_MATCH),
     LintId::of(matches::INFALLIBLE_DESTRUCTURING_MATCH),
+    LintId::of(matches::MANUAL_FILTER),
     LintId::of(matches::MANUAL_MAP),
     LintId::of(matches::MANUAL_UNWRAP_OR),
     LintId::of(matches::MATCH_AS_REF),
diff --git a/clippy_lints/src/lib.register_complexity.rs b/clippy_lints/src/lib.register_complexity.rs
index a58d066fa6b..e3849e5a626 100644
--- a/clippy_lints/src/lib.register_complexity.rs
+++ b/clippy_lints/src/lib.register_complexity.rs
@@ -27,6 +27,7 @@ store.register_group(true, "clippy::complexity", Some("clippy_complexity"), vec!
     LintId::of(manual_strip::MANUAL_STRIP),
     LintId::of(map_unit_fn::OPTION_MAP_UNIT_FN),
     LintId::of(map_unit_fn::RESULT_MAP_UNIT_FN),
+    LintId::of(matches::MANUAL_FILTER),
     LintId::of(matches::MANUAL_UNWRAP_OR),
     LintId::of(matches::MATCH_AS_REF),
     LintId::of(matches::MATCH_SINGLE_BINDING),
diff --git a/clippy_lints/src/lib.register_lints.rs b/clippy_lints/src/lib.register_lints.rs
index 307ec40f40b..70defe3bcbd 100644
--- a/clippy_lints/src/lib.register_lints.rs
+++ b/clippy_lints/src/lib.register_lints.rs
@@ -257,6 +257,7 @@ store.register_lints(&[
     match_result_ok::MATCH_RESULT_OK,
     matches::COLLAPSIBLE_MATCH,
     matches::INFALLIBLE_DESTRUCTURING_MATCH,
+    matches::MANUAL_FILTER,
     matches::MANUAL_MAP,
     matches::MANUAL_UNWRAP_OR,
     matches::MATCH_AS_REF,
diff --git a/clippy_lints/src/matches/manual_filter.rs b/clippy_lints/src/matches/manual_filter.rs
new file mode 100644
index 00000000000..9931f1268ab
--- /dev/null
+++ b/clippy_lints/src/matches/manual_filter.rs
@@ -0,0 +1,181 @@
+use clippy_utils::diagnostics::span_lint_and_sugg;
+use clippy_utils::ty::is_type_diagnostic_item;
+use clippy_utils::{is_res_lang_ctor, path_res, path_to_local_id};
+use rustc_hir::intravisit::{walk_expr, Visitor};
+use rustc_hir::LangItem::OptionSome;
+use rustc_hir::{Arm, Block, BlockCheckMode, Expr, ExprKind, HirId, Pat, PatKind, UnsafeSource};
+use rustc_lint::LateContext;
+use rustc_span::{sym, SyntaxContext};
+
+use super::manual_map::{check_with, SomeExpr};
+use super::MANUAL_FILTER;
+
+#[derive(Default)]
+struct NeedsUnsafeBlock(pub bool);
+
+impl<'tcx> Visitor<'tcx> for NeedsUnsafeBlock {
+    fn visit_expr(&mut self, expr: &'tcx Expr<'tcx>) {
+        match expr.kind {
+            ExprKind::Block(
+                Block {
+                    rules: BlockCheckMode::UnsafeBlock(UnsafeSource::UserProvided),
+                    ..
+                },
+                _,
+            ) => {
+                self.0 = true;
+            },
+            _ => walk_expr(self, expr),
+        }
+    }
+}
+
+// Function called on the `expr` of `[&+]Some((ref | ref mut) x) => <expr>`
+// Need to check if it's of the `if <cond> {<then_expr>} else {<else_expr>}`
+// AND that only one `then/else_expr` resolves to `Some(x)` while the other resolves to `None`
+// return `cond` if
+fn get_cond_expr<'tcx>(
+    cx: &LateContext<'tcx>,
+    pat: &Pat<'_>,
+    expr: &'tcx Expr<'_>,
+    ctxt: SyntaxContext,
+) -> Option<SomeExpr<'tcx>> {
+    if_chain! {
+        if let Some(block_expr) = peels_blocks_incl_unsafe_opt(expr);
+        if let ExprKind::If(cond, then_expr, Some(else_expr)) = block_expr.kind;
+        if let PatKind::Binding(_,target, ..) = pat.kind;
+        if let (then_visitor, else_visitor)
+            = (handle_if_or_else_expr(cx, target, ctxt, then_expr),
+                handle_if_or_else_expr(cx, target, ctxt, else_expr));
+        if then_visitor != else_visitor; // check that one expr resolves to `Some(x)`, the other to `None`
+        then {
+            let mut needs_unsafe_block = NeedsUnsafeBlock::default();
+            needs_unsafe_block.visit_expr(expr);
+            return Some(SomeExpr {
+                    expr: peels_blocks_incl_unsafe(cond.peel_drop_temps()),
+                    needs_unsafe_block: needs_unsafe_block.0,
+                    needs_negated: !then_visitor // if the `then_expr` resolves to `None`, need to negate the cond
+                })
+            }
+    };
+    None
+}
+
+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 `NeedsUnsafeBlock`
+    if let ExprKind::Block(block, None) = expr.kind {
+        if block.stmts.is_empty() {
+            return block.expr;
+        }
+    };
+    None
+}
+
+fn peels_blocks_incl_unsafe<'a>(expr: &'a Expr<'a>) -> &'a Expr<'a> {
+    peels_blocks_incl_unsafe_opt(expr).unwrap_or(expr)
+}
+
+// function called for each <ifelse> expression:
+// Some(x) => if <cond> {
+//    <ifelse>
+// } else {
+//    <ifelse>
+// }
+// Returns true if <ifelse> resolves to `Some(x)`, `false` otherwise
+fn handle_if_or_else_expr<'tcx>(
+    cx: &LateContext<'_>,
+    target: HirId,
+    ctxt: SyntaxContext,
+    if_or_else_expr: &'tcx Expr<'_>,
+) -> bool {
+    if let Some(inner_expr) = peels_blocks_incl_unsafe_opt(if_or_else_expr) {
+        // there can be not statements in the block as they would be removed when switching to `.filter`
+        if let ExprKind::Call(callee, [arg]) = inner_expr.kind {
+            return ctxt == if_or_else_expr.span.ctxt()
+                && is_res_lang_ctor(cx, path_res(cx, callee), OptionSome)
+                && path_to_local_id(arg, target);
+        }
+    };
+    false
+}
+
+// given the closure: `|<pattern>| <expr>`
+// returns `|&<pattern>| <expr>`
+fn add_ampersand_if_copy(body_str: String, has_copy_trait: bool) -> String {
+    if has_copy_trait {
+        let mut with_ampersand = body_str;
+        with_ampersand.insert(1, '&');
+        with_ampersand
+    } else {
+        body_str
+    }
+}
+
+pub(super) fn check_match<'tcx>(
+    cx: &LateContext<'tcx>,
+    scrutinee: &'tcx Expr<'_>,
+    arms: &'tcx [Arm<'_>],
+    expr: &'tcx Expr<'_>,
+) {
+    let ty = cx.typeck_results().expr_ty(expr);
+    if_chain! {
+        if is_type_diagnostic_item(cx, ty, sym::Option);
+        if arms.len() == 2;
+        if arms[0].guard.is_none();
+        if arms[1].guard.is_none();
+        then {
+            check(cx, expr, scrutinee, arms[0].pat, arms[0].body, Some(arms[1].pat), arms[1].body)
+        }
+    }
+}
+
+pub(super) fn check_if_let<'tcx>(
+    cx: &LateContext<'tcx>,
+    expr: &'tcx Expr<'_>,
+    let_pat: &'tcx Pat<'_>,
+    let_expr: &'tcx Expr<'_>,
+    then_expr: &'tcx Expr<'_>,
+    else_expr: &'tcx Expr<'_>,
+) {
+    check(cx, expr, let_expr, let_pat, then_expr, None, else_expr);
+}
+
+fn check<'tcx>(
+    cx: &LateContext<'tcx>,
+    expr: &'tcx Expr<'_>,
+    scrutinee: &'tcx Expr<'_>,
+    then_pat: &'tcx Pat<'_>,
+    then_body: &'tcx Expr<'_>,
+    else_pat: Option<&'tcx Pat<'_>>,
+    else_body: &'tcx Expr<'_>,
+) {
+    if let Some(sugg_info) = check_with(
+        cx,
+        expr,
+        scrutinee,
+        then_pat,
+        then_body,
+        else_pat,
+        else_body,
+        get_cond_expr,
+    ) {
+        let body_str = add_ampersand_if_copy(sugg_info.body_str, sugg_info.scrutinee_impl_copy);
+        span_lint_and_sugg(
+            cx,
+            MANUAL_FILTER,
+            expr.span,
+            "manual implementation of `Option::filter`",
+            "try this",
+            if sugg_info.needs_brackets {
+                format!(
+                    "{{ {}{}.filter({body_str}) }}",
+                    sugg_info.scrutinee_str, sugg_info.as_ref_str
+                )
+            } else {
+                format!("{}{}.filter({body_str})", sugg_info.scrutinee_str, sugg_info.as_ref_str)
+            },
+            sugg_info.app,
+        );
+    }
+}
diff --git a/clippy_lints/src/matches/manual_map.rs b/clippy_lints/src/matches/manual_map.rs
index 76f5e1c941c..2b6a07c5d74 100644
--- a/clippy_lints/src/matches/manual_map.rs
+++ b/clippy_lints/src/matches/manual_map.rs
@@ -1,10 +1,11 @@
+use super::MANUAL_MAP;
 use crate::{map_unit_fn::OPTION_MAP_UNIT_FN, matches::MATCH_AS_REF};
 use clippy_utils::diagnostics::span_lint_and_sugg;
 use clippy_utils::source::{snippet_with_applicability, snippet_with_context};
-use clippy_utils::ty::{is_type_diagnostic_item, peel_mid_ty_refs_is_mutable, type_is_unsafe_function};
+use clippy_utils::ty::{is_copy, is_type_diagnostic_item, peel_mid_ty_refs_is_mutable, type_is_unsafe_function};
 use clippy_utils::{
     can_move_expr_to_closure, is_else_clause, is_lint_allowed, is_res_lang_ctor, path_res, path_to_local_id,
-    peel_blocks, peel_hir_expr_refs, peel_hir_expr_while, CaptureKind,
+    peel_blocks, peel_hir_expr_refs, peel_hir_expr_while, sugg::Sugg, CaptureKind,
 };
 use rustc_ast::util::parser::PREC_POSTFIX;
 use rustc_errors::Applicability;
@@ -16,8 +17,6 @@ use rustc_hir::{
 use rustc_lint::LateContext;
 use rustc_span::{sym, SyntaxContext};
 
-use super::MANUAL_MAP;
-
 pub(super) fn check_match<'tcx>(
     cx: &LateContext<'tcx>,
     expr: &'tcx Expr<'_>,
@@ -43,7 +42,6 @@ pub(super) fn check_if_let<'tcx>(
     check(cx, expr, let_expr, let_pat, then_expr, None, else_expr);
 }
 
-#[expect(clippy::too_many_lines)]
 fn check<'tcx>(
     cx: &LateContext<'tcx>,
     expr: &'tcx Expr<'_>,
@@ -53,12 +51,59 @@ fn check<'tcx>(
     else_pat: Option<&'tcx Pat<'_>>,
     else_body: &'tcx Expr<'_>,
 ) {
+    if let Some(sugg_info) = check_with(
+        cx,
+        expr,
+        scrutinee,
+        then_pat,
+        then_body,
+        else_pat,
+        else_body,
+        get_some_expr,
+    ) {
+        span_lint_and_sugg(
+            cx,
+            MANUAL_MAP,
+            expr.span,
+            "manual implementation of `Option::map`",
+            "try this",
+            if sugg_info.needs_brackets {
+                format!(
+                    "{{ {}{}.map({}) }}",
+                    sugg_info.scrutinee_str, sugg_info.as_ref_str, sugg_info.body_str
+                )
+            } else {
+                format!(
+                    "{}{}.map({})",
+                    sugg_info.scrutinee_str, sugg_info.as_ref_str, sugg_info.body_str
+                )
+            },
+            sugg_info.app,
+        );
+    }
+}
+
+#[expect(clippy::too_many_arguments)]
+#[expect(clippy::too_many_lines)]
+pub(super) fn check_with<'tcx, F>(
+    cx: &LateContext<'tcx>,
+    expr: &'tcx Expr<'_>,
+    scrutinee: &'tcx Expr<'_>,
+    then_pat: &'tcx Pat<'_>,
+    then_body: &'tcx Expr<'_>,
+    else_pat: Option<&'tcx Pat<'_>>,
+    else_body: &'tcx Expr<'_>,
+    get_some_expr_fn: F,
+) -> Option<SuggInfo<'tcx>>
+where
+    F: Fn(&LateContext<'tcx>, &'tcx Pat<'_>, &'tcx Expr<'_>, SyntaxContext) -> Option<SomeExpr<'tcx>>,
+{
     let (scrutinee_ty, ty_ref_count, ty_mutability) =
         peel_mid_ty_refs_is_mutable(cx.typeck_results().expr_ty(scrutinee));
     if !(is_type_diagnostic_item(cx, scrutinee_ty, sym::Option)
         && is_type_diagnostic_item(cx, cx.typeck_results().expr_ty(expr), sym::Option))
     {
-        return;
+        return None;
     }
 
     let expr_ctxt = expr.span.ctxt();
@@ -78,29 +123,29 @@ fn check<'tcx>(
         (Some(OptionPat::Some { pattern, ref_count }), Some(OptionPat::None)) if is_none_expr(cx, else_body) => {
             (then_body, pattern, ref_count, false)
         },
-        _ => return,
+        _ => return None,
     };
 
     // Top level or patterns aren't allowed in closures.
     if matches!(some_pat.kind, PatKind::Or(_)) {
-        return;
+        return None;
     }
 
-    let some_expr = match get_some_expr(cx, some_expr, false, expr_ctxt) {
+    let some_expr = match get_some_expr_fn(cx, some_pat, some_expr, expr_ctxt) {
         Some(expr) => expr,
-        None => return,
+        None => return None,
     };
 
     // These two lints will go back and forth with each other.
     if cx.typeck_results().expr_ty(some_expr.expr) == cx.tcx.types.unit
         && !is_lint_allowed(cx, OPTION_MAP_UNIT_FN, expr.hir_id)
     {
-        return;
+        return None;
     }
 
     // `map` won't perform any adjustments.
     if !cx.typeck_results().expr_adjustments(some_expr.expr).is_empty() {
-        return;
+        return None;
     }
 
     // Determine which binding mode to use.
@@ -125,16 +170,16 @@ fn check<'tcx>(
                 });
                 if let ExprKind::Path(QPath::Resolved(None, Path { res: Res::Local(l), .. })) = e.kind {
                     match captures.get(l) {
-                        Some(CaptureKind::Value | CaptureKind::Ref(Mutability::Mut)) => return,
+                        Some(CaptureKind::Value | CaptureKind::Ref(Mutability::Mut)) => return None,
                         Some(CaptureKind::Ref(Mutability::Not)) if binding_ref_mutability == Mutability::Mut => {
-                            return;
+                            return None;
                         },
                         Some(CaptureKind::Ref(Mutability::Not)) | None => (),
                     }
                 }
             }
         },
-        None => return,
+        None => return None,
     };
 
     let mut app = Applicability::MachineApplicable;
@@ -149,6 +194,7 @@ fn check<'tcx>(
         scrutinee_str.into()
     };
 
+    let closure_expr_snip = some_expr.to_snippet_with_context(cx, expr_ctxt, &mut app);
     let body_str = if let PatKind::Binding(annotation, id, some_binding, None) = some_pat.kind {
         if_chain! {
             if !some_expr.needs_unsafe_block;
@@ -161,7 +207,7 @@ fn check<'tcx>(
                     && !is_lint_allowed(cx, MATCH_AS_REF, expr.hir_id)
                     && binding_ref.is_some()
                 {
-                    return;
+                    return None;
                 }
 
                 // `ref` and `ref mut` annotations were handled earlier.
@@ -170,41 +216,47 @@ fn check<'tcx>(
                 } else {
                     ""
                 };
-                let expr_snip = snippet_with_context(cx, some_expr.expr.span, expr_ctxt, "..", &mut app).0;
+
                 if some_expr.needs_unsafe_block {
-                    format!("|{annotation}{some_binding}| unsafe {{ {expr_snip} }}")
+                    format!("|{annotation}{some_binding}| unsafe {{ {closure_expr_snip} }}")
                 } else {
-                    format!("|{annotation}{some_binding}| {expr_snip}")
+                    format!("|{annotation}{some_binding}| {closure_expr_snip}")
                 }
             }
         }
     } else if !is_wild_none && explicit_ref.is_none() {
         // TODO: handle explicit reference annotations.
         let pat_snip = snippet_with_context(cx, some_pat.span, expr_ctxt, "..", &mut app).0;
-        let expr_snip = snippet_with_context(cx, some_expr.expr.span, expr_ctxt, "..", &mut app).0;
         if some_expr.needs_unsafe_block {
-            format!("|{pat_snip}| unsafe {{ {expr_snip} }}")
+            format!("|{pat_snip}| unsafe {{ {closure_expr_snip} }}")
         } else {
-            format!("|{pat_snip}| {expr_snip}")
+            format!("|{pat_snip}| {closure_expr_snip}")
         }
     } else {
         // Refutable bindings and mixed reference annotations can't be handled by `map`.
-        return;
+        return None;
     };
 
-    span_lint_and_sugg(
-        cx,
-        MANUAL_MAP,
-        expr.span,
-        "manual implementation of `Option::map`",
-        "try this",
-        if else_pat.is_none() && is_else_clause(cx.tcx, expr) {
-            format!("{{ {scrutinee_str}{as_ref_str}.map({body_str}) }}")
-        } else {
-            format!("{scrutinee_str}{as_ref_str}.map({body_str})")
-        },
+    // relies on the fact that Option<T>: Copy where T: copy
+    let scrutinee_impl_copy = is_copy(cx, scrutinee_ty);
+
+    Some(SuggInfo {
+        needs_brackets: else_pat.is_none() && is_else_clause(cx.tcx, expr),
+        scrutinee_impl_copy,
+        scrutinee_str,
+        as_ref_str,
+        body_str,
         app,
-    );
+    })
+}
+
+pub struct SuggInfo<'a> {
+    pub needs_brackets: bool,
+    pub scrutinee_impl_copy: bool,
+    pub scrutinee_str: String,
+    pub as_ref_str: &'a str,
+    pub body_str: String,
+    pub app: Applicability,
 }
 
 // Checks whether the expression could be passed as a function, or whether a closure is needed.
@@ -222,7 +274,8 @@ fn can_pass_as_func<'tcx>(cx: &LateContext<'tcx>, binding: HirId, expr: &'tcx Ex
     }
 }
 
-enum OptionPat<'a> {
+#[derive(Debug)]
+pub(super) enum OptionPat<'a> {
     Wild,
     None,
     Some {
@@ -234,14 +287,39 @@ enum OptionPat<'a> {
     },
 }
 
-struct SomeExpr<'tcx> {
-    expr: &'tcx Expr<'tcx>,
-    needs_unsafe_block: bool,
+pub(super) struct SomeExpr<'tcx> {
+    pub expr: &'tcx Expr<'tcx>,
+    pub needs_unsafe_block: bool,
+    pub needs_negated: bool, // for `manual_filter` lint
+}
+
+impl<'tcx> SomeExpr<'tcx> {
+    pub fn new_no_negated(expr: &'tcx Expr<'tcx>, needs_unsafe_block: bool) -> Self {
+        Self {
+            expr,
+            needs_unsafe_block,
+            needs_negated: false,
+        }
+    }
+
+    pub fn to_snippet_with_context(
+        &self,
+        cx: &LateContext<'tcx>,
+        ctxt: SyntaxContext,
+        app: &mut Applicability,
+    ) -> Sugg<'tcx> {
+        let sugg = Sugg::hir_with_context(cx, self.expr, ctxt, "..", app);
+        if self.needs_negated { !sugg } else { sugg }
+    }
 }
 
 // Try to parse into a recognized `Option` pattern.
 // i.e. `_`, `None`, `Some(..)`, or a reference to any of those.
-fn try_parse_pattern<'tcx>(cx: &LateContext<'tcx>, pat: &'tcx Pat<'_>, ctxt: SyntaxContext) -> Option<OptionPat<'tcx>> {
+pub(super) fn try_parse_pattern<'tcx>(
+    cx: &LateContext<'tcx>,
+    pat: &'tcx Pat<'_>,
+    ctxt: SyntaxContext,
+) -> Option<OptionPat<'tcx>> {
     fn f<'tcx>(
         cx: &LateContext<'tcx>,
         pat: &'tcx Pat<'_>,
@@ -268,36 +346,41 @@ fn try_parse_pattern<'tcx>(cx: &LateContext<'tcx>, pat: &'tcx Pat<'_>, ctxt: Syn
 // Checks for an expression wrapped by the `Some` constructor. Returns the contained expression.
 fn get_some_expr<'tcx>(
     cx: &LateContext<'tcx>,
+    _: &'tcx Pat<'_>,
     expr: &'tcx Expr<'_>,
-    needs_unsafe_block: bool,
     ctxt: SyntaxContext,
 ) -> Option<SomeExpr<'tcx>> {
-    // TODO: Allow more complex expressions.
-    match expr.kind {
-        ExprKind::Call(callee, [arg])
-            if ctxt == expr.span.ctxt() && is_res_lang_ctor(cx, path_res(cx, callee), OptionSome) =>
-        {
-            Some(SomeExpr {
-                expr: arg,
-                needs_unsafe_block,
-            })
-        },
-        ExprKind::Block(
-            Block {
-                stmts: [],
-                expr: Some(expr),
-                rules,
-                ..
+    fn get_some_expr_internal<'tcx>(
+        cx: &LateContext<'tcx>,
+        expr: &'tcx Expr<'_>,
+        needs_unsafe_block: bool,
+        ctxt: SyntaxContext,
+    ) -> Option<SomeExpr<'tcx>> {
+        // TODO: Allow more complex expressions.
+        match expr.kind {
+            ExprKind::Call(callee, [arg])
+                if ctxt == expr.span.ctxt() && is_res_lang_ctor(cx, path_res(cx, callee), OptionSome) =>
+            {
+                Some(SomeExpr::new_no_negated(arg, needs_unsafe_block))
             },
-            _,
-        ) => get_some_expr(
-            cx,
-            expr,
-            needs_unsafe_block || *rules == BlockCheckMode::UnsafeBlock(UnsafeSource::UserProvided),
-            ctxt,
-        ),
-        _ => None,
+            ExprKind::Block(
+                Block {
+                    stmts: [],
+                    expr: Some(expr),
+                    rules,
+                    ..
+                },
+                _,
+            ) => get_some_expr_internal(
+                cx,
+                expr,
+                needs_unsafe_block || *rules == BlockCheckMode::UnsafeBlock(UnsafeSource::UserProvided),
+                ctxt,
+            ),
+            _ => None,
+        }
     }
+    get_some_expr_internal(cx, expr, false, ctxt)
 }
 
 // Checks for the `None` value.
diff --git a/clippy_lints/src/matches/mod.rs b/clippy_lints/src/matches/mod.rs
index e6b183fc05f..cf1bd7a12ad 100644
--- a/clippy_lints/src/matches/mod.rs
+++ b/clippy_lints/src/matches/mod.rs
@@ -1,5 +1,6 @@
 mod collapsible_match;
 mod infallible_destructuring_match;
+mod manual_filter;
 mod manual_map;
 mod manual_unwrap_or;
 mod match_as_ref;
@@ -898,6 +899,34 @@ declare_clippy_lint! {
     "reimplementation of `map`"
 }
 
+declare_clippy_lint! {
+    /// ### What it does
+    /// Checks for usages of `match` which could be implemented using `filter`
+    ///
+    /// ### Why is this bad?
+    /// Using the `filter` method is clearer and more concise.
+    ///
+    /// ### Example
+    /// ```rust
+    /// match Some(0) {
+    ///     Some(x) => if x % 2 == 0 {
+    ///                     Some(x)
+    ///                } else {
+    ///                     None
+    ///                 },
+    ///     None => None,
+    /// };
+    /// ```
+    /// Use instead:
+    /// ```rust
+    /// Some(0).filter(|&x| x % 2 == 0);
+    /// ```
+    #[clippy::version = "1.65.0"]
+    pub MANUAL_FILTER,
+    complexity,
+    "reimplentation of `filter`"
+}
+
 #[derive(Default)]
 pub struct Matches {
     msrv: Option<RustcVersion>,
@@ -939,6 +968,7 @@ impl_lint_pass!(Matches => [
     SIGNIFICANT_DROP_IN_SCRUTINEE,
     TRY_ERR,
     MANUAL_MAP,
+    MANUAL_FILTER,
 ]);
 
 impl<'tcx> LateLintPass<'tcx> for Matches {
@@ -988,6 +1018,7 @@ impl<'tcx> LateLintPass<'tcx> for Matches {
                     if !in_constant(cx, expr.hir_id) {
                         manual_unwrap_or::check(cx, expr, ex, arms);
                         manual_map::check_match(cx, expr, ex, arms);
+                        manual_filter::check_match(cx, ex, arms, expr);
                     }
 
                     if self.infallible_destructuring_match_linted {
@@ -1014,6 +1045,14 @@ impl<'tcx> LateLintPass<'tcx> for Matches {
                     }
                     if !in_constant(cx, expr.hir_id) {
                         manual_map::check_if_let(cx, expr, if_let.let_pat, if_let.let_expr, if_let.if_then, else_expr);
+                        manual_filter::check_if_let(
+                            cx,
+                            expr,
+                            if_let.let_pat,
+                            if_let.let_expr,
+                            if_let.if_then,
+                            else_expr,
+                        );
                     }
                 }
                 redundant_pattern_match::check_if_let(
diff --git a/clippy_utils/src/sugg.rs b/clippy_utils/src/sugg.rs
index ef836e84829..e88542b77a6 100644
--- a/clippy_utils/src/sugg.rs
+++ b/clippy_utils/src/sugg.rs
@@ -1,7 +1,9 @@
 //! Contains utility functions to generate suggestions.
 #![deny(clippy::missing_docs_in_private_items)]
 
-use crate::source::{snippet, snippet_opt, snippet_with_applicability, snippet_with_macro_callsite};
+use crate::source::{
+    snippet, snippet_opt, snippet_with_applicability, snippet_with_context, snippet_with_macro_callsite,
+};
 use crate::ty::expr_sig;
 use crate::{get_parent_expr_for_hir, higher};
 use rustc_ast::util::parser::AssocOp;
@@ -110,7 +112,7 @@ impl<'a> Sugg<'a> {
         if expr.span.ctxt() == ctxt {
             Self::hir_from_snippet(expr, |span| snippet(cx, span, default))
         } else {
-            let snip = snippet_with_applicability(cx, expr.span, default, applicability);
+            let snip = snippet_with_context(cx, expr.span, ctxt, default, applicability).0;
             Sugg::NonParen(snip)
         }
     }
diff --git a/src/docs.rs b/src/docs.rs
index 39540e4b048..d24342076b6 100644
--- a/src/docs.rs
+++ b/src/docs.rs
@@ -256,6 +256,7 @@ docs! {
     "manual_async_fn",
     "manual_bits",
     "manual_clamp",
+    "manual_filter",
     "manual_filter_map",
     "manual_find",
     "manual_find_map",
diff --git a/src/docs/manual_filter.txt b/src/docs/manual_filter.txt
new file mode 100644
index 00000000000..19a4d9319d9
--- /dev/null
+++ b/src/docs/manual_filter.txt
@@ -0,0 +1,21 @@
+### What it does
+Checks for usages of `match` which could be implemented using `filter`
+
+### Why is this bad?
+Using the `filter` method is clearer and more concise.
+
+### Example
+```
+match Some(0) {
+    Some(x) => if x % 2 == 0 {
+                    Some(x)
+               } else {
+                    None
+                },
+    None => None,
+};
+```
+Use instead:
+```
+Some(0).filter(|&x| x % 2 == 0);
+```
\ No newline at end of file
diff --git a/tests/ui/manual_filter.fixed b/tests/ui/manual_filter.fixed
new file mode 100644
index 00000000000..3553291b87d
--- /dev/null
+++ b/tests/ui/manual_filter.fixed
@@ -0,0 +1,119 @@
+// run-rustfix
+
+#![warn(clippy::manual_filter)]
+#![allow(unused_variables, dead_code)]
+
+fn main() {
+    Some(0).filter(|&x| x <= 0);
+
+    Some(1).filter(|&x| x <= 0);
+
+    Some(2).filter(|&x| x <= 0);
+
+    Some(3).filter(|&x| x > 0);
+
+    let y = Some(4);
+    y.filter(|&x| x <= 0);
+
+    Some(5).filter(|&x| x > 0);
+
+    Some(6).as_ref().filter(|&x| x > &0);
+
+    let external_cond = true;
+    Some(String::new()).filter(|x| external_cond);
+
+    Some(7).filter(|&x| external_cond);
+
+    Some(8).filter(|&x| x != 0);
+
+    Some(9).filter(|&x| x > 10 && x < 100);
+
+    const fn f1() {
+        // Don't lint, `.filter` is not const
+        match Some(10) {
+            Some(x) => {
+                if x > 10 && x < 100 {
+                    Some(x)
+                } else {
+                    None
+                }
+            },
+            None => None,
+        };
+    }
+
+    #[allow(clippy::blocks_in_if_conditions)]
+    Some(11).filter(|&x| {
+                println!("foo");
+                x > 10 && x < 100
+            });
+
+    match Some(12) {
+        // Don't Lint, statement is lost by `.filter`
+        Some(x) => {
+            if x > 10 && x < 100 {
+                println!("foo");
+                Some(x)
+            } else {
+                None
+            }
+        },
+        None => None,
+    };
+
+    match Some(13) {
+        // Don't Lint, because of `None => Some(1)`
+        Some(x) => {
+            if x > 10 && x < 100 {
+                println!("foo");
+                Some(x)
+            } else {
+                None
+            }
+        },
+        None => Some(1),
+    };
+
+    unsafe fn f(x: u32) -> bool {
+        true
+    }
+    let _ = Some(14).filter(|&x| unsafe { f(x) });
+    let _ = Some(15).filter(|&x| unsafe { f(x) });
+
+    #[allow(clippy::redundant_pattern_matching)]
+    if let Some(_) = Some(16) {
+        Some(16)
+    } else { Some(16).filter(|&x| x % 2 == 0) };
+
+    match Some((17, 17)) {
+        // Not linted for now could be
+        Some((x, y)) => {
+            if y != x {
+                Some((x, y))
+            } else {
+                None
+            }
+        },
+        None => None,
+    };
+
+    struct NamedTuple {
+        pub x: u8,
+        pub y: (i32, u32),
+    }
+
+    match Some(NamedTuple {
+        // Not linted for now could be
+        x: 17,
+        y: (18, 19),
+    }) {
+        Some(NamedTuple { x, y }) => {
+            if y.1 != x as u32 {
+                Some(NamedTuple { x, y })
+            } else {
+                None
+            }
+        },
+        None => None,
+    };
+}
diff --git a/tests/ui/manual_filter.rs b/tests/ui/manual_filter.rs
new file mode 100644
index 00000000000..aa9f90f752b
--- /dev/null
+++ b/tests/ui/manual_filter.rs
@@ -0,0 +1,243 @@
+// run-rustfix
+
+#![warn(clippy::manual_filter)]
+#![allow(unused_variables, dead_code)]
+
+fn main() {
+    match Some(0) {
+        None => None,
+        Some(x) => {
+            if x > 0 {
+                None
+            } else {
+                Some(x)
+            }
+        },
+    };
+
+    match Some(1) {
+        Some(x) => {
+            if x > 0 {
+                None
+            } else {
+                Some(x)
+            }
+        },
+        None => None,
+    };
+
+    match Some(2) {
+        Some(x) => {
+            if x > 0 {
+                None
+            } else {
+                Some(x)
+            }
+        },
+        _ => None,
+    };
+
+    match Some(3) {
+        Some(x) => {
+            if x > 0 {
+                Some(x)
+            } else {
+                None
+            }
+        },
+        None => None,
+    };
+
+    let y = Some(4);
+    match y {
+        // Some(4)
+        None => None,
+        Some(x) => {
+            if x > 0 {
+                None
+            } else {
+                Some(x)
+            }
+        },
+    };
+
+    match Some(5) {
+        Some(x) => {
+            if x > 0 {
+                Some(x)
+            } else {
+                None
+            }
+        },
+        _ => None,
+    };
+
+    match Some(6) {
+        Some(ref x) => {
+            if x > &0 {
+                Some(x)
+            } else {
+                None
+            }
+        },
+        _ => None,
+    };
+
+    let external_cond = true;
+    match Some(String::new()) {
+        Some(x) => {
+            if external_cond {
+                Some(x)
+            } else {
+                None
+            }
+        },
+        _ => None,
+    };
+
+    if let Some(x) = Some(7) {
+        if external_cond { Some(x) } else { None }
+    } else {
+        None
+    };
+
+    match &Some(8) {
+        &Some(x) => {
+            if x != 0 {
+                Some(x)
+            } else {
+                None
+            }
+        },
+        _ => None,
+    };
+
+    match Some(9) {
+        Some(x) => {
+            if x > 10 && x < 100 {
+                Some(x)
+            } else {
+                None
+            }
+        },
+        None => None,
+    };
+
+    const fn f1() {
+        // Don't lint, `.filter` is not const
+        match Some(10) {
+            Some(x) => {
+                if x > 10 && x < 100 {
+                    Some(x)
+                } else {
+                    None
+                }
+            },
+            None => None,
+        };
+    }
+
+    #[allow(clippy::blocks_in_if_conditions)]
+    match Some(11) {
+        // Lint, statement is preserved by `.filter`
+        Some(x) => {
+            if {
+                println!("foo");
+                x > 10 && x < 100
+            } {
+                Some(x)
+            } else {
+                None
+            }
+        },
+        None => None,
+    };
+
+    match Some(12) {
+        // Don't Lint, statement is lost by `.filter`
+        Some(x) => {
+            if x > 10 && x < 100 {
+                println!("foo");
+                Some(x)
+            } else {
+                None
+            }
+        },
+        None => None,
+    };
+
+    match Some(13) {
+        // Don't Lint, because of `None => Some(1)`
+        Some(x) => {
+            if x > 10 && x < 100 {
+                println!("foo");
+                Some(x)
+            } else {
+                None
+            }
+        },
+        None => Some(1),
+    };
+
+    unsafe fn f(x: u32) -> bool {
+        true
+    }
+    let _ = match Some(14) {
+        Some(x) => {
+            if unsafe { f(x) } {
+                Some(x)
+            } else {
+                None
+            }
+        },
+        None => None,
+    };
+    let _ = match Some(15) {
+        Some(x) => unsafe {
+            if f(x) { Some(x) } else { None }
+        },
+        None => None,
+    };
+
+    #[allow(clippy::redundant_pattern_matching)]
+    if let Some(_) = Some(16) {
+        Some(16)
+    } else if let Some(x) = Some(16) {
+        // Lint starting from here
+        if x % 2 == 0 { Some(x) } else { None }
+    } else {
+        None
+    };
+
+    match Some((17, 17)) {
+        // Not linted for now could be
+        Some((x, y)) => {
+            if y != x {
+                Some((x, y))
+            } else {
+                None
+            }
+        },
+        None => None,
+    };
+
+    struct NamedTuple {
+        pub x: u8,
+        pub y: (i32, u32),
+    }
+
+    match Some(NamedTuple {
+        // Not linted for now could be
+        x: 17,
+        y: (18, 19),
+    }) {
+        Some(NamedTuple { x, y }) => {
+            if y.1 != x as u32 {
+                Some(NamedTuple { x, y })
+            } else {
+                None
+            }
+        },
+        None => None,
+    };
+}
diff --git a/tests/ui/manual_filter.stderr b/tests/ui/manual_filter.stderr
new file mode 100644
index 00000000000..53dea922930
--- /dev/null
+++ b/tests/ui/manual_filter.stderr
@@ -0,0 +1,191 @@
+error: manual implementation of `Option::filter`
+  --> $DIR/manual_filter.rs:7:5
+   |
+LL | /     match Some(0) {
+LL | |         None => None,
+LL | |         Some(x) => {
+LL | |             if x > 0 {
+...  |
+LL | |         },
+LL | |     };
+   | |_____^ help: try this: `Some(0).filter(|&x| x <= 0)`
+   |
+   = note: `-D clippy::manual-filter` implied by `-D warnings`
+
+error: manual implementation of `Option::filter`
+  --> $DIR/manual_filter.rs:18:5
+   |
+LL | /     match Some(1) {
+LL | |         Some(x) => {
+LL | |             if x > 0 {
+LL | |                 None
+...  |
+LL | |         None => None,
+LL | |     };
+   | |_____^ help: try this: `Some(1).filter(|&x| x <= 0)`
+
+error: manual implementation of `Option::filter`
+  --> $DIR/manual_filter.rs:29:5
+   |
+LL | /     match Some(2) {
+LL | |         Some(x) => {
+LL | |             if x > 0 {
+LL | |                 None
+...  |
+LL | |         _ => None,
+LL | |     };
+   | |_____^ help: try this: `Some(2).filter(|&x| x <= 0)`
+
+error: manual implementation of `Option::filter`
+  --> $DIR/manual_filter.rs:40:5
+   |
+LL | /     match Some(3) {
+LL | |         Some(x) => {
+LL | |             if x > 0 {
+LL | |                 Some(x)
+...  |
+LL | |         None => None,
+LL | |     };
+   | |_____^ help: try this: `Some(3).filter(|&x| x > 0)`
+
+error: manual implementation of `Option::filter`
+  --> $DIR/manual_filter.rs:52:5
+   |
+LL | /     match y {
+LL | |         // Some(4)
+LL | |         None => None,
+LL | |         Some(x) => {
+...  |
+LL | |         },
+LL | |     };
+   | |_____^ help: try this: `y.filter(|&x| x <= 0)`
+
+error: manual implementation of `Option::filter`
+  --> $DIR/manual_filter.rs:64:5
+   |
+LL | /     match Some(5) {
+LL | |         Some(x) => {
+LL | |             if x > 0 {
+LL | |                 Some(x)
+...  |
+LL | |         _ => None,
+LL | |     };
+   | |_____^ help: try this: `Some(5).filter(|&x| x > 0)`
+
+error: manual implementation of `Option::filter`
+  --> $DIR/manual_filter.rs:75:5
+   |
+LL | /     match Some(6) {
+LL | |         Some(ref x) => {
+LL | |             if x > &0 {
+LL | |                 Some(x)
+...  |
+LL | |         _ => None,
+LL | |     };
+   | |_____^ help: try this: `Some(6).as_ref().filter(|&x| x > &0)`
+
+error: manual implementation of `Option::filter`
+  --> $DIR/manual_filter.rs:87:5
+   |
+LL | /     match Some(String::new()) {
+LL | |         Some(x) => {
+LL | |             if external_cond {
+LL | |                 Some(x)
+...  |
+LL | |         _ => None,
+LL | |     };
+   | |_____^ help: try this: `Some(String::new()).filter(|x| external_cond)`
+
+error: manual implementation of `Option::filter`
+  --> $DIR/manual_filter.rs:98:5
+   |
+LL | /     if let Some(x) = Some(7) {
+LL | |         if external_cond { Some(x) } else { None }
+LL | |     } else {
+LL | |         None
+LL | |     };
+   | |_____^ help: try this: `Some(7).filter(|&x| external_cond)`
+
+error: manual implementation of `Option::filter`
+  --> $DIR/manual_filter.rs:104:5
+   |
+LL | /     match &Some(8) {
+LL | |         &Some(x) => {
+LL | |             if x != 0 {
+LL | |                 Some(x)
+...  |
+LL | |         _ => None,
+LL | |     };
+   | |_____^ help: try this: `Some(8).filter(|&x| x != 0)`
+
+error: manual implementation of `Option::filter`
+  --> $DIR/manual_filter.rs:115:5
+   |
+LL | /     match Some(9) {
+LL | |         Some(x) => {
+LL | |             if x > 10 && x < 100 {
+LL | |                 Some(x)
+...  |
+LL | |         None => None,
+LL | |     };
+   | |_____^ help: try this: `Some(9).filter(|&x| x > 10 && x < 100)`
+
+error: manual implementation of `Option::filter`
+  --> $DIR/manual_filter.rs:141:5
+   |
+LL | /     match Some(11) {
+LL | |         // Lint, statement is preserved by `.filter`
+LL | |         Some(x) => {
+LL | |             if {
+...  |
+LL | |         None => None,
+LL | |     };
+   | |_____^
+   |
+help: try this
+   |
+LL ~     Some(11).filter(|&x| {
+LL +                 println!("foo");
+LL +                 x > 10 && x < 100
+LL ~             });
+   |
+
+error: manual implementation of `Option::filter`
+  --> $DIR/manual_filter.rs:185:13
+   |
+LL |       let _ = match Some(14) {
+   |  _____________^
+LL | |         Some(x) => {
+LL | |             if unsafe { f(x) } {
+LL | |                 Some(x)
+...  |
+LL | |         None => None,
+LL | |     };
+   | |_____^ help: try this: `Some(14).filter(|&x| unsafe { f(x) })`
+
+error: manual implementation of `Option::filter`
+  --> $DIR/manual_filter.rs:195:13
+   |
+LL |       let _ = match Some(15) {
+   |  _____________^
+LL | |         Some(x) => unsafe {
+LL | |             if f(x) { Some(x) } else { None }
+LL | |         },
+LL | |         None => None,
+LL | |     };
+   | |_____^ help: try this: `Some(15).filter(|&x| unsafe { f(x) })`
+
+error: manual implementation of `Option::filter`
+  --> $DIR/manual_filter.rs:205:12
+   |
+LL |       } else if let Some(x) = Some(16) {
+   |  ____________^
+LL | |         // Lint starting from here
+LL | |         if x % 2 == 0 { Some(x) } else { None }
+LL | |     } else {
+LL | |         None
+LL | |     };
+   | |_____^ help: try this: `{ Some(16).filter(|&x| x % 2 == 0) }`
+
+error: aborting due to 15 previous errors
+