From 6990eaa9720e45b3b09eb00d9476b89839935c10 Mon Sep 17 00:00:00 2001 From: est31 Date: Sun, 2 Jul 2023 04:03:20 +0200 Subject: [PATCH] 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. --- clippy_lints/src/manual_let_else.rs | 20 +++++++++---------- tests/ui/manual_let_else_question_mark.fixed | 7 +++++++ tests/ui/manual_let_else_question_mark.rs | 7 +++++++ tests/ui/manual_let_else_question_mark.stderr | 8 +++++++- 4 files changed, 31 insertions(+), 11 deletions(-) diff --git a/clippy_lints/src/manual_let_else.rs b/clippy_lints/src/manual_let_else.rs index 363018f1381..18739e9d8e2 100644 --- a/clippy_lints/src/manual_let_else.rs +++ b/clippy_lints/src/manual_let_else.rs @@ -1,12 +1,10 @@ -use crate::question_mark::{pat_and_expr_can_be_question_mark, QuestionMark}; +use crate::question_mark::{pat_and_expr_can_be_question_mark, QuestionMark, QUESTION_MARK}; use clippy_utils::diagnostics::span_lint_and_then; use clippy_utils::higher::IfLetOrMatch; -use clippy_utils::msrvs; -use clippy_utils::peel_blocks; use clippy_utils::source::snippet_with_context; use clippy_utils::ty::is_type_diagnostic_item; use clippy_utils::visitors::{Descend, Visitable}; -use if_chain::if_chain; +use clippy_utils::{is_lint_allowed, msrvs, peel_blocks}; use rustc_data_structures::fx::{FxHashMap, FxHashSet}; use rustc_errors::Applicability; use rustc_hir::intravisit::{walk_expr, Visitor}; @@ -65,12 +63,14 @@ pub(crate) fn check_manual_let_else(&mut self, cx: &LateContext<'_>, stmt: &'tcx let Some(if_let_or_match) = IfLetOrMatch::parse(cx, init) { match if_let_or_match { - IfLetOrMatch::IfLet(if_let_expr, let_pat, if_then, if_else) => if_chain! { - if let Some(ident_map) = expr_simple_identity_map(local.pat, let_pat, if_then); - if let Some(if_else) = if_else; - if expr_diverges(cx, if_else); - if pat_and_expr_can_be_question_mark(cx, let_pat, if_else).is_none(); - then { + IfLetOrMatch::IfLet(if_let_expr, let_pat, if_then, if_else) => { + if + let Some(ident_map) = expr_simple_identity_map(local.pat, let_pat, if_then) && + let Some(if_else) = if_else && + expr_diverges(cx, if_else) && + let qm_allowed = is_lint_allowed(cx, QUESTION_MARK, stmt.hir_id) && + (qm_allowed || pat_and_expr_can_be_question_mark(cx, let_pat, if_else).is_none()) + { emit_manual_let_else(cx, stmt.span, if_let_expr, &ident_map, let_pat, if_else); } }, diff --git a/tests/ui/manual_let_else_question_mark.fixed b/tests/ui/manual_let_else_question_mark.fixed index 617f127b9a8..02308bc7c4c 100644 --- a/tests/ui/manual_let_else_question_mark.fixed +++ b/tests/ui/manual_let_else_question_mark.fixed @@ -52,5 +52,12 @@ fn foo() -> Option<()> { let Some(v) = g() else { return None }; } + // This is a copy of the case above where we'd fire the question_mark lint, but here we have allowed + // it. Make sure that manual_let_else is fired as the fallback. + #[allow(clippy::question_mark)] + { + let Some(v) = g() else { return None }; + } + Some(()) } diff --git a/tests/ui/manual_let_else_question_mark.rs b/tests/ui/manual_let_else_question_mark.rs index 3cf1eca8673..9c7ad386dc9 100644 --- a/tests/ui/manual_let_else_question_mark.rs +++ b/tests/ui/manual_let_else_question_mark.rs @@ -57,5 +57,12 @@ fn foo() -> Option<()> { }; } + // This is a copy of the case above where we'd fire the question_mark lint, but here we have allowed + // it. Make sure that manual_let_else is fired as the fallback. + #[allow(clippy::question_mark)] + { + let v = if let Some(v_some) = g() { v_some } else { return None }; + } + Some(()) } diff --git a/tests/ui/manual_let_else_question_mark.stderr b/tests/ui/manual_let_else_question_mark.stderr index 31052acf75c..d7d2e127ea3 100644 --- a/tests/ui/manual_let_else_question_mark.stderr +++ b/tests/ui/manual_let_else_question_mark.stderr @@ -45,5 +45,11 @@ LL | | _ => return None, LL | | }; | |__________^ help: consider writing: `let Some(v) = g() else { return None };` -error: aborting due to 5 previous errors +error: this could be rewritten as `let...else` + --> $DIR/manual_let_else_question_mark.rs:64:9 + | +LL | let v = if let Some(v_some) = g() { v_some } else { return None }; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider writing: `let Some(v) = g() else { return None };` + +error: aborting due to 6 previous errors