Skip to content

Commit 4746c00

Browse files
author
ceptontech
committed
Add a check for some followed by filter
The program will suggest using the `then_some` method.
1 parent af0bc98 commit 4746c00

File tree

11 files changed

+127
-4
lines changed

11 files changed

+127
-4
lines changed

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6135,6 +6135,7 @@ Released 2018-09-13
61356135
[`filter_map_identity`]: https://rust-lang.github.io/rust-clippy/master/index.html#filter_map_identity
61366136
[`filter_map_next`]: https://rust-lang.github.io/rust-clippy/master/index.html#filter_map_next
61376137
[`filter_next`]: https://rust-lang.github.io/rust-clippy/master/index.html#filter_next
6138+
[`filter_some`]: https://rust-lang.github.io/rust-clippy/master/index.html#filter_some
61386139
[`find_map`]: https://rust-lang.github.io/rust-clippy/master/index.html#find_map
61396140
[`flat_map_identity`]: https://rust-lang.github.io/rust-clippy/master/index.html#flat_map_identity
61406141
[`flat_map_option`]: https://rust-lang.github.io/rust-clippy/master/index.html#flat_map_option

clippy_lints/src/declared_lints.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -375,6 +375,7 @@ pub static LINTS: &[&::declare_clippy_lint::LintInfo] = &[
375375
crate::methods::FILTER_MAP_IDENTITY_INFO,
376376
crate::methods::FILTER_MAP_NEXT_INFO,
377377
crate::methods::FILTER_NEXT_INFO,
378+
crate::methods::FILTER_SOME_INFO,
378379
crate::methods::FLAT_MAP_IDENTITY_INFO,
379380
crate::methods::FLAT_MAP_OPTION_INFO,
380381
crate::methods::FORMAT_COLLECT_INFO,
Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,61 @@
1+
use clippy_utils::diagnostics::span_lint_and_then;
2+
use clippy_utils::source::{indent_of, reindent_multiline, snippet};
3+
use clippy_utils::{is_res_lang_ctor, pat_is_wild, path_res};
4+
use rustc_ast::util::parser::ExprPrecedence;
5+
use rustc_errors::Applicability;
6+
use rustc_hir::LangItem::OptionSome;
7+
use rustc_hir::{Body, Expr, ExprKind};
8+
use rustc_lint::LateContext;
9+
10+
use super::FILTER_SOME;
11+
12+
pub(super) fn check<'tcx>(
13+
cx: &LateContext<'tcx>,
14+
expr: &'tcx Expr<'tcx>,
15+
recv: &'tcx Expr<'tcx>,
16+
arg: &'tcx Expr<'tcx>,
17+
) {
18+
let value = match recv.kind {
19+
ExprKind::Call(f, [value]) if is_res_lang_ctor(cx, path_res(cx, f), OptionSome) => value,
20+
_ => return,
21+
};
22+
let condition = if let ExprKind::Closure(c) = arg.kind
23+
&& let Body {
24+
params: [p],
25+
value: condition,
26+
} = cx.tcx.hir_body(c.body)
27+
&& pat_is_wild(cx, &p.pat.kind, arg)
28+
{
29+
Some(condition)
30+
} else {
31+
None
32+
};
33+
span_lint_and_then(cx, FILTER_SOME, expr.span, "`filter` for `Some`", |diag| {
34+
let help = "consider using `then_some` instead";
35+
if let Some(condition) = condition {
36+
let parentheses = cx.precedence(condition) < ExprPrecedence::Unambiguous;
37+
diag.span_suggestion(
38+
expr.span,
39+
help,
40+
reindent_multiline(
41+
&format!(
42+
"{}{}{}.then_some({})",
43+
if parentheses { "(" } else { "" },
44+
snippet(cx, condition.span, ".."),
45+
if parentheses { ")" } else { "" },
46+
snippet(cx, value.span, "..")
47+
),
48+
true,
49+
indent_of(cx, expr.span),
50+
),
51+
Applicability::MaybeIncorrect,
52+
);
53+
diag.note(
54+
"this change will alter the order in which the condition and \
55+
the value are evaluated",
56+
);
57+
} else {
58+
diag.help(help);
59+
}
60+
});
61+
}

clippy_lints/src/methods/mod.rs

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ mod filter_map_bool_then;
2525
mod filter_map_identity;
2626
mod filter_map_next;
2727
mod filter_next;
28+
mod filter_some;
2829
mod flat_map_identity;
2930
mod flat_map_option;
3031
mod format_collect;
@@ -4639,6 +4640,27 @@ declare_clippy_lint! {
46394640
"detects redundant calls to `Iterator::cloned`"
46404641
}
46414642

4643+
declare_clippy_lint! {
4644+
/// ### What it does
4645+
/// Checks for usage of `Some(_).filter(_)`.
4646+
///
4647+
/// ### Why is this bad?
4648+
/// Readability, this can be written more concisely as `_.then_some(_)`.
4649+
///
4650+
/// ### Example
4651+
/// ```no_run
4652+
/// Some(0).filter(|_| x);
4653+
/// ```
4654+
/// Use instead:
4655+
/// ```no_run
4656+
/// x.then_some(0);
4657+
/// ```
4658+
#[clippy::version = "1.91.0"]
4659+
pub FILTER_SOME,
4660+
complexity,
4661+
"using `Some(x).filter`, which is more succinctly expressed as `.then(x)`"
4662+
}
4663+
46424664
#[expect(clippy::struct_excessive_bools)]
46434665
pub struct Methods {
46444666
avoid_breaking_exported_api: bool,
@@ -4820,6 +4842,7 @@ impl_lint_pass!(Methods => [
48204842
SWAP_WITH_TEMPORARY,
48214843
IP_CONSTANT,
48224844
REDUNDANT_ITER_CLONED,
4845+
FILTER_SOME,
48234846
]);
48244847

48254848
/// Extracts a method call name, args, and `Span` of the method name.
@@ -5191,6 +5214,7 @@ impl Methods {
51915214
// use the sourcemap to get the span of the closure
51925215
iter_filter::check(cx, expr, arg, span);
51935216
}
5217+
filter_some::check(cx, expr, recv, arg);
51945218
},
51955219
(sym::find, [arg]) => {
51965220
if let Some((sym::cloned, recv2, [], _span2, _)) = method_call(recv) {

tests/ui/filter_some.fixed

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
#![warn(clippy::filter_some)]
2+
3+
fn main() {
4+
let _ = false.then_some(0);
5+
//~^ filter_some
6+
let _ = (1 == 0).then_some(0);
7+
//~^ filter_some
8+
}

tests/ui/filter_some.rs

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
#![warn(clippy::filter_some)]
2+
3+
fn main() {
4+
let _ = Some(0).filter(|_| false);
5+
//~^ filter_some
6+
let _ = Some(0).filter(|_| 1 == 0);
7+
//~^ filter_some
8+
}

tests/ui/filter_some.stderr

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
error: `filter` for `Some`
2+
--> tests/ui/filter_some.rs:4:13
3+
|
4+
LL | let _ = Some(0).filter(|_| false);
5+
| ^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using `then_some` instead: `false.then_some(0)`
6+
|
7+
= note: this change will alter the order in which the condition and the value are evaluated
8+
= note: `-D clippy::filter-some` implied by `-D warnings`
9+
= help: to override `-D warnings` add `#[allow(clippy::filter_some)]`
10+
11+
error: `filter` for `Some`
12+
--> tests/ui/filter_some.rs:6:13
13+
|
14+
LL | let _ = Some(0).filter(|_| 1 == 0);
15+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using `then_some` instead: `(1 == 0).then_some(0)`
16+
|
17+
= note: this change will alter the order in which the condition and the value are evaluated
18+
19+
error: aborting due to 2 previous errors
20+

tests/ui/manual_filter.fixed

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
#![warn(clippy::manual_filter)]
2-
#![allow(unused_variables, dead_code, clippy::useless_vec)]
2+
#![allow(unused_variables, dead_code, clippy::filter_some, clippy::useless_vec)]
33

44
fn main() {
55
Some(0).filter(|&x| x <= 0);

tests/ui/manual_filter.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
#![warn(clippy::manual_filter)]
2-
#![allow(unused_variables, dead_code, clippy::useless_vec)]
2+
#![allow(unused_variables, dead_code, clippy::filter_some, clippy::useless_vec)]
33

44
fn main() {
55
match Some(0) {

tests/ui/option_filter_map.fixed

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
#![warn(clippy::option_filter_map)]
2-
#![allow(clippy::map_flatten, clippy::unnecessary_map_on_constructor)]
2+
#![allow(clippy::filter_some, clippy::map_flatten, clippy::unnecessary_map_on_constructor)]
33

44
fn main() {
55
let _ = Some(Some(1)).flatten();

0 commit comments

Comments
 (0)