Skip to content

Commit 778cdfc

Browse files
committed
question_mark: Add check for else branch
cc #2841
1 parent d2e5a8c commit 778cdfc

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
@@ -49,6 +49,22 @@ impl SomeStruct {
4949

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

5470
fn main() {

tests/ui/question_mark.stderr

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

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

0 commit comments

Comments
 (0)