Skip to content

Commit 5b3a666

Browse files
committed
fix manual_filter false positive
do explicit checks for the other branch being None
1 parent 3905f51 commit 5b3a666

File tree

3 files changed

+65
-6
lines changed

3 files changed

+65
-6
lines changed

clippy_lints/src/matches/manual_filter.rs

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ use clippy_utils::ty::is_type_diagnostic_item;
33
use clippy_utils::visitors::contains_unsafe_block;
44
use clippy_utils::{is_res_lang_ctor, path_res, path_to_local_id};
55

6-
use rustc_hir::LangItem::OptionSome;
6+
use rustc_hir::LangItem::{OptionNone, OptionSome};
77
use rustc_hir::{Arm, Expr, ExprKind, HirId, Pat, PatKind};
88
use rustc_lint::LateContext;
99
use rustc_span::{sym, SyntaxContext};
@@ -25,15 +25,13 @@ fn get_cond_expr<'tcx>(
2525
if let Some(block_expr) = peels_blocks_incl_unsafe_opt(expr);
2626
if let ExprKind::If(cond, then_expr, Some(else_expr)) = block_expr.kind;
2727
if let PatKind::Binding(_,target, ..) = pat.kind;
28-
if let (then_visitor, else_visitor)
29-
= (is_some_expr(cx, target, ctxt, then_expr),
30-
is_some_expr(cx, target, ctxt, else_expr));
31-
if then_visitor != else_visitor; // check that one expr resolves to `Some(x)`, the other to `None`
28+
if is_some_expr(cx, target, ctxt, then_expr) && is_none_expr(cx, else_expr)
29+
|| is_none_expr(cx, then_expr) && is_some_expr(cx, target, ctxt, else_expr); // check that one expr resolves to `Some(x)`, the other to `None`
3230
then {
3331
return Some(SomeExpr {
3432
expr: peels_blocks_incl_unsafe(cond.peel_drop_temps()),
3533
needs_unsafe_block: contains_unsafe_block(cx, expr),
36-
needs_negated: !then_visitor // if the `then_expr` resolves to `None`, need to negate the cond
34+
needs_negated: is_none_expr(cx, then_expr) // if the `then_expr` resolves to `None`, need to negate the cond
3735
})
3836
}
3937
};
@@ -74,6 +72,13 @@ fn is_some_expr(cx: &LateContext<'_>, target: HirId, ctxt: SyntaxContext, expr:
7472
false
7573
}
7674

75+
fn is_none_expr(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool {
76+
if let Some(inner_expr) = peels_blocks_incl_unsafe_opt(expr) {
77+
return is_res_lang_ctor(cx, path_res(cx, inner_expr), OptionNone);
78+
};
79+
false
80+
}
81+
7782
// given the closure: `|<pattern>| <expr>`
7883
// returns `|&<pattern>| <expr>`
7984
fn add_ampersand_if_copy(body_str: String, has_copy_trait: bool) -> String {

tests/ui/manual_filter.fixed

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -116,4 +116,31 @@ fn main() {
116116
},
117117
None => None,
118118
};
119+
120+
match Some(20) {
121+
// Don't Lint, because `Some(3*x)` is not `None`
122+
None => None,
123+
Some(x) => {
124+
if x > 0 {
125+
Some(3 * x)
126+
} else {
127+
Some(x)
128+
}
129+
},
130+
};
131+
132+
// Don't lint: https://github.com/rust-lang/rust-clippy/issues/10088
133+
let result = if let Some(a) = maybe_some() {
134+
if let Some(b) = maybe_some() {
135+
Some(a + b)
136+
} else {
137+
Some(a)
138+
}
139+
} else {
140+
None
141+
};
142+
}
143+
144+
fn maybe_some() -> Option<u32> {
145+
Some(0)
119146
}

tests/ui/manual_filter.rs

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -240,4 +240,31 @@ fn main() {
240240
},
241241
None => None,
242242
};
243+
244+
match Some(20) {
245+
// Don't Lint, because `Some(3*x)` is not `None`
246+
None => None,
247+
Some(x) => {
248+
if x > 0 {
249+
Some(3 * x)
250+
} else {
251+
Some(x)
252+
}
253+
},
254+
};
255+
256+
// Don't lint: https://github.com/rust-lang/rust-clippy/issues/10088
257+
let result = if let Some(a) = maybe_some() {
258+
if let Some(b) = maybe_some() {
259+
Some(a + b)
260+
} else {
261+
Some(a)
262+
}
263+
} else {
264+
None
265+
};
266+
}
267+
268+
fn maybe_some() -> Option<u32> {
269+
Some(0)
243270
}

0 commit comments

Comments
 (0)