Add a check for some followed by filter

CHANGELOG.md

@@ -6135,6 +6135,7 @@ Released 2018-09-13
 [`filter_map_identity`]: https://rust-lang.github.io/rust-clippy/master/index.html#filter_map_identity
 [`filter_map_next`]: https://rust-lang.github.io/rust-clippy/master/index.html#filter_map_next
 [`filter_next`]: https://rust-lang.github.io/rust-clippy/master/index.html#filter_next
+[`filter_some`]: https://rust-lang.github.io/rust-clippy/master/index.html#filter_some
 [`find_map`]: https://rust-lang.github.io/rust-clippy/master/index.html#find_map
 [`flat_map_identity`]: https://rust-lang.github.io/rust-clippy/master/index.html#flat_map_identity
 [`flat_map_option`]: https://rust-lang.github.io/rust-clippy/master/index.html#flat_map_option

clippy_lints/src/declared_lints.rs

@@ -375,6 +375,7 @@ pub static LINTS: &[&::declare_clippy_lint::LintInfo] = &[
     crate::methods::FILTER_MAP_IDENTITY_INFO,
     crate::methods::FILTER_MAP_NEXT_INFO,
     crate::methods::FILTER_NEXT_INFO,
+    crate::methods::FILTER_SOME_INFO,
     crate::methods::FLAT_MAP_IDENTITY_INFO,
     crate::methods::FLAT_MAP_OPTION_INFO,
     crate::methods::FORMAT_COLLECT_INFO,

clippy_lints/src/methods/filter_some.rs

@@ -0,0 +1,63 @@
+use clippy_utils::diagnostics::span_lint_and_then;
+use clippy_utils::msrvs::{self, Msrv};
+use clippy_utils::source::{indent_of, reindent_multiline, snippet, snippet_with_context};
+use clippy_utils::{is_res_lang_ctor, pat_is_wild, path_res};
+use rustc_ast::util::parser::ExprPrecedence;
+use rustc_errors::Applicability;
+use rustc_hir::LangItem::OptionSome;
+use rustc_hir::{Body, Expr, ExprKind};
+use rustc_lint::LateContext;
+
+use super::FILTER_SOME;
+
+pub(super) fn check<'tcx>(
+    cx: &LateContext<'tcx>,
+    expr: &'tcx Expr<'tcx>,
+    recv: &'tcx Expr<'tcx>,
+    arg: &'tcx Expr<'tcx>,
+    msrv: Msrv,
+) {
+    if !msrv.meets(cx, msrvs::BOOL_THEN_SOME) {
+        return;
+    }
+    let mut applicability = Applicability::MaybeIncorrect;
+    let value = match recv.kind {
+        ExprKind::Call(f, [value]) if is_res_lang_ctor(cx, path_res(cx, f), OptionSome) => value,
+        _ => return,
+    };
+    let condition = if let ExprKind::Closure(c) = arg.kind
+        && let Body {
+            params: [p],
+            value: condition,
+        } = cx.tcx.hir_body(c.body)
+        && pat_is_wild(cx, &p.pat.kind, arg)
+    {
+        condition
+    } else {
+        return;
+    };
+    let (condition_text, condition_is_macro) =
+        snippet_with_context(cx, condition.span, arg.span.ctxt(), "..", &mut applicability);
+    let parentheses = !condition_is_macro && cx.precedence(condition) < ExprPrecedence::Unambiguous;
+    let value_text = snippet(cx, value.span, "..");
+    span_lint_and_then(cx, FILTER_SOME, expr.span, "`filter` for `Some`", |diag| {
+        diag.span_suggestion(
+            expr.span,
+            "consider using `then_some` instead",
+            reindent_multiline(
+                &format!(
+                    "{}{condition_text}{}.then_some({value_text})",
+                    if parentheses { "(" } else { "" },
+                    if parentheses { ")" } else { "" },
+                ),
+                true,
+                indent_of(cx, expr.span),
+            ),
+            applicability,
+        );
+        diag.note(
+            "this change will alter the order in which the condition and the \
+             value are evaluated",
+        );
+    });
+}

clippy_lints/src/methods/mod.rs

@@ -25,6 +25,7 @@ mod filter_map_bool_then;
 mod filter_map_identity;
 mod filter_map_next;
 mod filter_next;
+mod filter_some;
 mod flat_map_identity;
 mod flat_map_option;
 mod format_collect;
@@ -4639,6 +4640,29 @@ declare_clippy_lint! {
     "detects redundant calls to `Iterator::cloned`"
 }
 
+declare_clippy_lint! {
+    /// ### What it does
+    /// Checks for usage of `Some(_).filter(_)`.
+    ///
+    /// ### Why is this bad?
+    /// Readability, this can be written more concisely as `_.then_some(_)`.
+    ///
+    /// ### Example
+    /// ```no_run
+    /// let x = false;
+    /// Some(0).filter(|_| x);
+    /// ```
+    /// Use instead:
+    /// ```no_run
+    /// let x = false;
+    /// x.then_some(0);
+    /// ```
+    #[clippy::version = "1.91.0"]
+    pub FILTER_SOME,
+    complexity,
+    "using `Some(x).filter`, which is more succinctly expressed as `.then(x)`"
+}
+
 #[expect(clippy::struct_excessive_bools)]
 pub struct Methods {
     avoid_breaking_exported_api: bool,
@@ -4820,6 +4844,7 @@ impl_lint_pass!(Methods => [
     SWAP_WITH_TEMPORARY,
     IP_CONSTANT,
     REDUNDANT_ITER_CLONED,
+    FILTER_SOME,
 ]);
 
 /// Extracts a method call name, args, and `Span` of the method name.
@@ -5191,6 +5216,7 @@ impl Methods {
                         // use the sourcemap to get the span of the closure
                         iter_filter::check(cx, expr, arg, span);
                     }
+                    filter_some::check(cx, expr, recv, arg, self.msrv);
                 },
                 (sym::find, [arg]) => {
                     if let Some((sym::cloned, recv2, [], _span2, _)) = method_call(recv) {

tests/ui/filter_some.fixed

@@ -0,0 +1,40 @@
+#![warn(clippy::filter_some)]
+
+macro_rules! unchanged {
+    ($result:expr) => {
+        $result
+    };
+}
+
+macro_rules! condition {
+    ($condition:expr) => {
+        $condition || false
+    };
+}
+
+fn main() {
+    let _ = false.then_some(0);
+    //~^ filter_some
+    // The condition contains an operator.  The program should add parentheses.
+    let _ = (1 == 0).then_some(0);
+    //~^ filter_some
+    let _ = match 0 {
+        //~^ filter_some
+        0 => false,
+        1 => true,
+        _ => true,
+    }.then_some(0);
+    // The argument to filter requires the value in the option.  The program
+    // can't figure out how to change it.  It should leave it alone for now.
+    let _ = Some(0).filter(|x| *x == 0);
+    // The expression is a macro argument.  The program should change the macro
+    // argument.  It should not expand the macro.
+    let _ = unchanged!(false.then_some(0));
+    //~^ filter_some
+    let _ = vec![false].is_empty().then_some(0);
+    //~^ filter_some
+    // The condition is a macro that expands to an expression containing an
+    // operator.  The program should not add parentheses.
+    let _ = condition!(false).then_some(0);
+    //~^ filter_some
+}

tests/ui/filter_some.rs

@@ -0,0 +1,40 @@
+#![warn(clippy::filter_some)]
+
+macro_rules! unchanged {
+    ($result:expr) => {
+        $result
+    };
+}
+
+macro_rules! condition {
+    ($condition:expr) => {
+        $condition || false
+    };
+}
+
+fn main() {
+    let _ = Some(0).filter(|_| false);
+    //~^ filter_some
+    // The condition contains an operator.  The program should add parentheses.
+    let _ = Some(0).filter(|_| 1 == 0);
+    //~^ filter_some
+    let _ = Some(0).filter(|_| match 0 {
+        //~^ filter_some
+        0 => false,
+        1 => true,
+        _ => true,
+    });
+    // The argument to filter requires the value in the option.  The program
+    // can't figure out how to change it.  It should leave it alone for now.
+    let _ = Some(0).filter(|x| *x == 0);
+    // The expression is a macro argument.  The program should change the macro
+    // argument.  It should not expand the macro.
+    let _ = unchanged!(Some(0).filter(|_| false));
+    //~^ filter_some
+    let _ = Some(0).filter(|_| vec![false].is_empty());
+    //~^ filter_some
+    // The condition is a macro that expands to an expression containing an
+    // operator.  The program should not add parentheses.
+    let _ = Some(0).filter(|_| condition!(false));
+    //~^ filter_some
+}

tests/ui/filter_some.stderr

@@ -0,0 +1,67 @@
+error: `filter` for `Some`
+  --> tests/ui/filter_some.rs:16:13
+   |
+LL |     let _ = Some(0).filter(|_| false);
+   |             ^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using `then_some` instead: `false.then_some(0)`
+   |
+   = note: this change will alter the order in which the condition and the value are evaluated
+   = note: `-D clippy::filter-some` implied by `-D warnings`
+   = help: to override `-D warnings` add `#[allow(clippy::filter_some)]`
+
+error: `filter` for `Some`
+  --> tests/ui/filter_some.rs:19:13
+   |
+LL |     let _ = Some(0).filter(|_| 1 == 0);
+   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using `then_some` instead: `(1 == 0).then_some(0)`
+   |
+   = note: this change will alter the order in which the condition and the value are evaluated
+
+error: `filter` for `Some`
+  --> tests/ui/filter_some.rs:21:13
+   |
+LL |       let _ = Some(0).filter(|_| match 0 {
+   |  _____________^
+LL | |
+LL | |         0 => false,
+LL | |         1 => true,
+LL | |         _ => true,
+LL | |     });
+   | |______^
+   |
+   = note: this change will alter the order in which the condition and the value are evaluated
+help: consider using `then_some` instead
+   |
+LL ~     let _ = match 0 {
+LL +
+LL +         0 => false,
+LL +         1 => true,
+LL +         _ => true,
+LL ~     }.then_some(0);
+   |
+
+error: `filter` for `Some`
+  --> tests/ui/filter_some.rs:32:24
+   |
+LL |     let _ = unchanged!(Some(0).filter(|_| false));
+   |                        ^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using `then_some` instead: `false.then_some(0)`
+   |
+   = note: this change will alter the order in which the condition and the value are evaluated
+
+error: `filter` for `Some`
+  --> tests/ui/filter_some.rs:34:13
+   |
+LL |     let _ = Some(0).filter(|_| vec![false].is_empty());
+   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using `then_some` instead: `vec![false].is_empty().then_some(0)`
+   |
+   = note: this change will alter the order in which the condition and the value are evaluated
+
+error: `filter` for `Some`
+  --> tests/ui/filter_some.rs:38:13
+   |
+LL |     let _ = Some(0).filter(|_| condition!(false));
+   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using `then_some` instead: `condition!(false).then_some(0)`
+   |
+   = note: this change will alter the order in which the condition and the value are evaluated
+
+error: aborting due to 6 previous errors
+

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::filter_some, clippy::useless_vec)]
 
 fn main() {
     Some(0).filter(|&x| x <= 0);

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::filter_some, clippy::useless_vec)]
 
 fn main() {
     match Some(0) {

tests/ui/option_filter_map.fixed

@@ -1,5 +1,5 @@
 #![warn(clippy::option_filter_map)]
-#![allow(clippy::map_flatten, clippy::unnecessary_map_on_constructor)]
+#![allow(clippy::filter_some, clippy::map_flatten, clippy::unnecessary_map_on_constructor)]
 
 fn main() {
     let _ = Some(Some(1)).flatten();

tests/ui/option_filter_map.rs

@@ -1,5 +1,5 @@
 #![warn(clippy::option_filter_map)]
-#![allow(clippy::map_flatten, clippy::unnecessary_map_on_constructor)]
+#![allow(clippy::filter_some, clippy::map_flatten, clippy::unnecessary_map_on_constructor)]
 
 fn main() {
     let _ = Some(Some(1)).filter(Option::is_some).map(Option::unwrap);