Skip to content

Commit 6990eaa

Browse files
committed
Don't suppress manual_let_else if question_mark is allowed
If question_mark is allowed, there is no overlap any more, so we can just not suppress it.
1 parent 20dfaba commit 6990eaa

File tree

4 files changed

+31
-11
lines changed

4 files changed

+31
-11
lines changed

clippy_lints/src/manual_let_else.rs

+10-10
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,10 @@
1-
use crate::question_mark::{pat_and_expr_can_be_question_mark, QuestionMark};
1+
use crate::question_mark::{pat_and_expr_can_be_question_mark, QuestionMark, QUESTION_MARK};
22
use clippy_utils::diagnostics::span_lint_and_then;
33
use clippy_utils::higher::IfLetOrMatch;
4-
use clippy_utils::msrvs;
5-
use clippy_utils::peel_blocks;
64
use clippy_utils::source::snippet_with_context;
75
use clippy_utils::ty::is_type_diagnostic_item;
86
use clippy_utils::visitors::{Descend, Visitable};
9-
use if_chain::if_chain;
7+
use clippy_utils::{is_lint_allowed, msrvs, peel_blocks};
108
use rustc_data_structures::fx::{FxHashMap, FxHashSet};
119
use rustc_errors::Applicability;
1210
use rustc_hir::intravisit::{walk_expr, Visitor};
@@ -65,12 +63,14 @@ impl<'tcx> QuestionMark {
6563
let Some(if_let_or_match) = IfLetOrMatch::parse(cx, init)
6664
{
6765
match if_let_or_match {
68-
IfLetOrMatch::IfLet(if_let_expr, let_pat, if_then, if_else) => if_chain! {
69-
if let Some(ident_map) = expr_simple_identity_map(local.pat, let_pat, if_then);
70-
if let Some(if_else) = if_else;
71-
if expr_diverges(cx, if_else);
72-
if pat_and_expr_can_be_question_mark(cx, let_pat, if_else).is_none();
73-
then {
66+
IfLetOrMatch::IfLet(if_let_expr, let_pat, if_then, if_else) => {
67+
if
68+
let Some(ident_map) = expr_simple_identity_map(local.pat, let_pat, if_then) &&
69+
let Some(if_else) = if_else &&
70+
expr_diverges(cx, if_else) &&
71+
let qm_allowed = is_lint_allowed(cx, QUESTION_MARK, stmt.hir_id) &&
72+
(qm_allowed || pat_and_expr_can_be_question_mark(cx, let_pat, if_else).is_none())
73+
{
7474
emit_manual_let_else(cx, stmt.span, if_let_expr, &ident_map, let_pat, if_else);
7575
}
7676
},

tests/ui/manual_let_else_question_mark.fixed

+7
Original file line numberDiff line numberDiff line change
@@ -52,5 +52,12 @@ fn foo() -> Option<()> {
5252
let Some(v) = g() else { return None };
5353
}
5454

55+
// This is a copy of the case above where we'd fire the question_mark lint, but here we have allowed
56+
// it. Make sure that manual_let_else is fired as the fallback.
57+
#[allow(clippy::question_mark)]
58+
{
59+
let Some(v) = g() else { return None };
60+
}
61+
5562
Some(())
5663
}

tests/ui/manual_let_else_question_mark.rs

+7
Original file line numberDiff line numberDiff line change
@@ -57,5 +57,12 @@ fn foo() -> Option<()> {
5757
};
5858
}
5959

60+
// This is a copy of the case above where we'd fire the question_mark lint, but here we have allowed
61+
// it. Make sure that manual_let_else is fired as the fallback.
62+
#[allow(clippy::question_mark)]
63+
{
64+
let v = if let Some(v_some) = g() { v_some } else { return None };
65+
}
66+
6067
Some(())
6168
}

tests/ui/manual_let_else_question_mark.stderr

+7-1
Original file line numberDiff line numberDiff line change
@@ -45,5 +45,11 @@ LL | | _ => return None,
4545
LL | | };
4646
| |__________^ help: consider writing: `let Some(v) = g() else { return None };`
4747

48-
error: aborting due to 5 previous errors
48+
error: this could be rewritten as `let...else`
49+
--> $DIR/manual_let_else_question_mark.rs:64:9
50+
|
51+
LL | let v = if let Some(v_some) = g() { v_some } else { return None };
52+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider writing: `let Some(v) = g() else { return None };`
53+
54+
error: aborting due to 6 previous errors
4955

0 commit comments

Comments
 (0)