Skip to content

Commit 20dfaba

Browse files
committed
Put into one pass
1 parent c6be621 commit 20dfaba

File tree

3 files changed

+74
-78
lines changed

3 files changed

+74
-78
lines changed

clippy_lints/src/lib.rs

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -662,7 +662,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
662662
});
663663
store.register_late_pass(move |_| Box::new(matches::Matches::new(msrv())));
664664
let matches_for_let_else = conf.matches_for_let_else;
665-
store.register_late_pass(move |_| Box::new(manual_let_else::ManualLetElse::new(msrv(), matches_for_let_else)));
666665
store.register_early_pass(move || Box::new(manual_non_exhaustive::ManualNonExhaustiveStruct::new(msrv())));
667666
store.register_late_pass(move |_| Box::new(manual_non_exhaustive::ManualNonExhaustiveEnum::new(msrv())));
668667
store.register_late_pass(move |_| Box::new(manual_strip::ManualStrip::new(msrv())));
@@ -771,7 +770,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
771770
store.register_late_pass(|_| Box::<useless_conversion::UselessConversion>::default());
772771
store.register_late_pass(|_| Box::new(implicit_hasher::ImplicitHasher));
773772
store.register_late_pass(|_| Box::new(fallible_impl_from::FallibleImplFrom));
774-
store.register_late_pass(|_| Box::<question_mark::QuestionMark>::default());
773+
store.register_late_pass(move |_| Box::new(question_mark::QuestionMark::new(msrv(), matches_for_let_else)));
775774
store.register_late_pass(|_| Box::new(question_mark_used::QuestionMarkUsed));
776775
store.register_early_pass(|| Box::new(suspicious_operation_groupings::SuspiciousOperationGroupings));
777776
store.register_late_pass(|_| Box::new(suspicious_trait_impl::SuspiciousImpl));

clippy_lints/src/manual_let_else.rs

Lines changed: 7 additions & 70 deletions
Original file line numberDiff line numberDiff line change
@@ -1,19 +1,19 @@
1+
use crate::question_mark::{pat_and_expr_can_be_question_mark, QuestionMark};
12
use clippy_utils::diagnostics::span_lint_and_then;
23
use clippy_utils::higher::IfLetOrMatch;
3-
use clippy_utils::msrvs::{self, Msrv};
4+
use clippy_utils::msrvs;
5+
use clippy_utils::peel_blocks;
46
use clippy_utils::source::snippet_with_context;
57
use clippy_utils::ty::is_type_diagnostic_item;
68
use clippy_utils::visitors::{Descend, Visitable};
7-
use clippy_utils::{is_refutable, is_res_lang_ctor, peel_blocks};
89
use if_chain::if_chain;
910
use rustc_data_structures::fx::{FxHashMap, FxHashSet};
1011
use rustc_errors::Applicability;
1112
use rustc_hir::intravisit::{walk_expr, Visitor};
12-
use rustc_hir::LangItem::{OptionNone, OptionSome};
1313
use rustc_hir::{Expr, ExprKind, HirId, ItemId, Local, MatchSource, Pat, PatKind, QPath, Stmt, StmtKind, Ty};
14-
use rustc_lint::{LateContext, LateLintPass, LintContext};
14+
use rustc_lint::{LateContext, LintContext};
1515
use rustc_middle::lint::in_external_macro;
16-
use rustc_session::{declare_tool_lint, impl_lint_pass};
16+
use rustc_session::declare_tool_lint;
1717
use rustc_span::symbol::{sym, Symbol};
1818
use rustc_span::Span;
1919
use serde::Deserialize;
@@ -51,25 +51,8 @@ declare_clippy_lint! {
5151
"manual implementation of a let...else statement"
5252
}
5353

54-
pub struct ManualLetElse {
55-
msrv: Msrv,
56-
matches_behaviour: MatchLintBehaviour,
57-
}
58-
59-
impl ManualLetElse {
60-
#[must_use]
61-
pub fn new(msrv: Msrv, matches_behaviour: MatchLintBehaviour) -> Self {
62-
Self {
63-
msrv,
64-
matches_behaviour,
65-
}
66-
}
67-
}
68-
69-
impl_lint_pass!(ManualLetElse => [MANUAL_LET_ELSE]);
70-
71-
impl<'tcx> LateLintPass<'tcx> for ManualLetElse {
72-
fn check_stmt(&mut self, cx: &LateContext<'_>, stmt: &'tcx Stmt<'tcx>) {
54+
impl<'tcx> QuestionMark {
55+
pub(crate) fn check_manual_let_else(&mut self, cx: &LateContext<'_>, stmt: &'tcx Stmt<'tcx>) {
7356
if !self.msrv.meets(msrvs::LET_ELSE) || in_external_macro(cx.sess(), stmt.span) {
7457
return;
7558
}
@@ -130,8 +113,6 @@ impl<'tcx> LateLintPass<'tcx> for ManualLetElse {
130113
}
131114
};
132115
}
133-
134-
extract_msrv_attr!(LateContext);
135116
}
136117

137118
fn emit_manual_let_else(
@@ -169,50 +150,6 @@ fn emit_manual_let_else(
169150
);
170151
}
171152

172-
/// Returns whether the given let pattern and else body can be turned into a question mark
173-
///
174-
/// For this example:
175-
/// ```ignore
176-
/// let FooBar { a, b } = if let Some(a) = ex { a } else { return None };
177-
/// ```
178-
/// We get as parameters:
179-
/// ```ignore
180-
/// pat: Some(a)
181-
/// else_body: return None
182-
/// ```
183-
184-
/// And for this example:
185-
/// ```ignore
186-
/// let Some(FooBar { a, b }) = ex else { return None };
187-
/// ```
188-
/// We get as parameters:
189-
/// ```ignore
190-
/// pat: Some(FooBar { a, b })
191-
/// else_body: return None
192-
/// ```
193-
194-
/// We output `Some(a)` in the first instance, and `Some(FooBar { a, b })` in the second, because
195-
/// the question mark operator is applicable here. Callers have to check whether we are in a
196-
/// constant or not.
197-
pub fn pat_and_expr_can_be_question_mark<'a, 'hir>(
198-
cx: &LateContext<'_>,
199-
pat: &'a Pat<'hir>,
200-
else_body: &Expr<'_>,
201-
) -> Option<&'a Pat<'hir>> {
202-
if let PatKind::TupleStruct(pat_path, [inner_pat], _) = pat.kind &&
203-
is_res_lang_ctor(cx, cx.qpath_res(&pat_path, pat.hir_id), OptionSome) &&
204-
!is_refutable(cx, inner_pat) &&
205-
let else_body = peel_blocks(else_body) &&
206-
let ExprKind::Ret(Some(ret_val)) = else_body.kind &&
207-
let ExprKind::Path(ret_path) = ret_val.kind &&
208-
is_res_lang_ctor(cx, cx.qpath_res(&ret_path, ret_val.hir_id), OptionNone)
209-
{
210-
Some(inner_pat)
211-
} else {
212-
None
213-
}
214-
}
215-
216153
/// Replaces the locals in the pattern
217154
///
218155
/// For this example:

clippy_lints/src/question_mark.rs

Lines changed: 66 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,18 +1,19 @@
1-
use crate::manual_let_else::pat_and_expr_can_be_question_mark;
1+
use crate::manual_let_else::{MatchLintBehaviour, MANUAL_LET_ELSE};
22
use clippy_utils::diagnostics::span_lint_and_sugg;
3+
use clippy_utils::msrvs::Msrv;
34
use clippy_utils::source::snippet_with_applicability;
45
use clippy_utils::ty::is_type_diagnostic_item;
56
use clippy_utils::{
6-
eq_expr_value, get_parent_node, in_constant, is_else_clause, is_res_lang_ctor, path_to_local, path_to_local_id,
7-
peel_blocks, peel_blocks_with_stmt,
7+
eq_expr_value, get_parent_node, in_constant, is_else_clause, is_refutable, is_res_lang_ctor, path_to_local,
8+
path_to_local_id, peel_blocks, peel_blocks_with_stmt,
89
};
910
use clippy_utils::{higher, is_path_lang_item};
1011
use if_chain::if_chain;
1112
use rustc_errors::Applicability;
1213
use rustc_hir::def::Res;
1314
use rustc_hir::LangItem::{self, OptionNone, OptionSome, ResultErr, ResultOk};
1415
use rustc_hir::{
15-
BindingAnnotation, Block, ByRef, Expr, ExprKind, Local, Node, PatKind, PathSegment, QPath, Stmt, StmtKind,
16+
BindingAnnotation, Block, ByRef, Expr, ExprKind, Local, Node, Pat, PatKind, PathSegment, QPath, Stmt, StmtKind,
1617
};
1718
use rustc_lint::{LateContext, LateLintPass};
1819
use rustc_middle::ty::Ty;
@@ -45,16 +46,29 @@ declare_clippy_lint! {
4546
"checks for expressions that could be replaced by the question mark operator"
4647
}
4748

48-
#[derive(Default)]
4949
pub struct QuestionMark {
50+
pub(crate) msrv: Msrv,
51+
pub(crate) matches_behaviour: MatchLintBehaviour,
5052
/// Keeps track of how many try blocks we are in at any point during linting.
5153
/// This allows us to answer the question "are we inside of a try block"
5254
/// very quickly, without having to walk up the parent chain, by simply checking
5355
/// if it is greater than zero.
5456
/// As for why we need this in the first place: <https://github.com/rust-lang/rust-clippy/issues/8628>
5557
try_block_depth_stack: Vec<u32>,
5658
}
57-
impl_lint_pass!(QuestionMark => [QUESTION_MARK]);
59+
60+
impl_lint_pass!(QuestionMark => [QUESTION_MARK, MANUAL_LET_ELSE]);
61+
62+
impl QuestionMark {
63+
#[must_use]
64+
pub fn new(msrv: Msrv, matches_behaviour: MatchLintBehaviour) -> Self {
65+
Self {
66+
msrv,
67+
matches_behaviour,
68+
try_block_depth_stack: Vec::new(),
69+
}
70+
}
71+
}
5872

5973
enum IfBlockType<'hir> {
6074
/// An `if x.is_xxx() { a } else { b } ` expression.
@@ -81,6 +95,50 @@ enum IfBlockType<'hir> {
8195
),
8296
}
8397

98+
/// Returns whether the given let pattern and else body can be turned into a question mark
99+
///
100+
/// For this example:
101+
/// ```ignore
102+
/// let FooBar { a, b } = if let Some(a) = ex { a } else { return None };
103+
/// ```
104+
/// We get as parameters:
105+
/// ```ignore
106+
/// pat: Some(a)
107+
/// else_body: return None
108+
/// ```
109+
110+
/// And for this example:
111+
/// ```ignore
112+
/// let Some(FooBar { a, b }) = ex else { return None };
113+
/// ```
114+
/// We get as parameters:
115+
/// ```ignore
116+
/// pat: Some(FooBar { a, b })
117+
/// else_body: return None
118+
/// ```
119+
120+
/// We output `Some(a)` in the first instance, and `Some(FooBar { a, b })` in the second, because
121+
/// the question mark operator is applicable here. Callers have to check whether we are in a
122+
/// constant or not.
123+
pub(crate) fn pat_and_expr_can_be_question_mark<'a, 'hir>(
124+
cx: &LateContext<'_>,
125+
pat: &'a Pat<'hir>,
126+
else_body: &Expr<'_>,
127+
) -> Option<&'a Pat<'hir>> {
128+
if let PatKind::TupleStruct(pat_path, [inner_pat], _) = pat.kind &&
129+
is_res_lang_ctor(cx, cx.qpath_res(&pat_path, pat.hir_id), OptionSome) &&
130+
!is_refutable(cx, inner_pat) &&
131+
let else_body = peel_blocks(else_body) &&
132+
let ExprKind::Ret(Some(ret_val)) = else_body.kind &&
133+
let ExprKind::Path(ret_path) = ret_val.kind &&
134+
is_res_lang_ctor(cx, cx.qpath_res(&ret_path, ret_val.hir_id), OptionNone)
135+
{
136+
Some(inner_pat)
137+
} else {
138+
None
139+
}
140+
}
141+
84142
fn check_let_some_else_return_none(cx: &LateContext<'_>, stmt: &Stmt<'_>) {
85143
if let StmtKind::Local(Local { pat, init: Some(init_expr), els: Some(els), .. }) = stmt.kind &&
86144
let Block { stmts: &[], expr: Some(els), .. } = els &&
@@ -289,6 +347,7 @@ impl<'tcx> LateLintPass<'tcx> for QuestionMark {
289347
if !in_constant(cx, stmt.hir_id) {
290348
check_let_some_else_return_none(cx, stmt);
291349
}
350+
self.check_manual_let_else(cx, stmt);
292351
}
293352
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
294353
if !in_constant(cx, expr.hir_id) {
@@ -322,4 +381,5 @@ impl<'tcx> LateLintPass<'tcx> for QuestionMark {
322381
.expect("blocks are always part of bodies and must have a depth") -= 1;
323382
}
324383
}
384+
extract_msrv_attr!(LateContext);
325385
}

0 commit comments

Comments
 (0)