Skip to content

Commit 327fede

Browse files
committed
question_mark: Add check for else branch
cc #2841
1 parent 553c9e2 commit 327fede

File tree

3 files changed

+44
-4
lines changed

3 files changed

+44
-4
lines changed

clippy_lints/src/question_mark.rs

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ use if_chain::if_chain;
1717

1818
use crate::rustc_errors::Applicability;
1919
use crate::utils::paths::*;
20-
use crate::utils::{match_def_path, match_type, span_lint_and_then};
20+
use crate::utils::{match_def_path, match_type, span_lint_and_then, SpanlessEq};
2121

2222
/// **What it does:** Checks for expressions that could be replaced by the question mark operator
2323
///
@@ -64,14 +64,27 @@ impl Pass {
6464
/// If it matches, it will suggest to use the question mark operator instead
6565
fn check_is_none_and_early_return_none(cx: &LateContext<'_, '_>, expr: &Expr) {
6666
if_chain! {
67-
if let ExprKind::If(ref if_expr, ref body, _) = expr.node;
68-
if let ExprKind::MethodCall(ref segment, _, ref args) = if_expr.node;
67+
if let ExprKind::If(if_expr, body, else_) = &expr.node;
68+
if let ExprKind::MethodCall(segment, _, args) = &if_expr.node;
6969
if segment.ident.name == "is_none";
7070
if Self::expression_returns_none(cx, body);
7171
if let Some(subject) = args.get(0);
7272
if Self::is_option(cx, subject);
7373

7474
then {
75+
if let Some(else_) = else_ {
76+
if_chain! {
77+
if let ExprKind::Block(block, None) = &else_.node;
78+
if block.stmts.len() == 0;
79+
if let Some(expr) = &block.expr;
80+
if SpanlessEq::new(cx).ignore_fn().eq_expr(subject, expr);
81+
then {
82+
} else {
83+
return
84+
}
85+
}
86+
}
87+
7588
span_lint_and_then(
7689
cx,
7790
QUESTION_MARK,

tests/ui/question_mark.rs

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,22 @@ impl SomeStruct {
5050

5151
self.opt
5252
}
53+
54+
pub fn func2(&self) -> Option<u32> {
55+
let _ = if self.opt.is_none() {
56+
return None;
57+
} else {
58+
self.opt
59+
};
60+
61+
let _ = if self.opt.is_none() {
62+
return None;
63+
} else {
64+
0
65+
};
66+
67+
self.opt
68+
}
5369
}
5470

5571
fn main() {

tests/ui/question_mark.stderr

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,5 +18,16 @@ error: this block may be rewritten with the `?` operator
1818
49 | | }
1919
| |_________^ help: replace_it_with: `(self.opt)?;`
2020

21-
error: aborting due to 2 previous errors
21+
error: this block may be rewritten with the `?` operator
22+
--> $DIR/question_mark.rs:55:17
23+
|
24+
55 | let _ = if self.opt.is_none() {
25+
| _________________^
26+
56 | | return None;
27+
57 | | } else {
28+
58 | | self.opt
29+
59 | | };
30+
| |_________^ help: replace_it_with: `self.opt?;`
31+
32+
error: aborting due to 3 previous errors
2233

0 commit comments

Comments
 (0)