Skip to content

Commit 48d939f

Browse files
borsbors
committed
Auto merge of #8042 - camsteffen:peel-blocks, r=xFrednet
Peel blocks and statements utils changelog: none * Rename `remove_blocks` to `peel_blocks` * Add `peel_blocks_and_stmts` * Various refactors to use the above utils * The utils also now check `block.rules`
2 parents 9e08527 + 16bbd24 commit 48d939f

24 files changed

+197
-319
lines changed

‎clippy_lints/src/assertions_on_constants.rs

Lines changed: 2 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ use clippy_utils::consts::{constant, Constant};
22
use clippy_utils::diagnostics::span_lint_and_help;
33
use clippy_utils::higher;
44
use clippy_utils::source::snippet_opt;
5-
use clippy_utils::{is_direct_expn_of, is_expn_of, match_panic_call};
5+
use clippy_utils::{is_direct_expn_of, is_expn_of, match_panic_call, peel_blocks};
66
use if_chain::if_chain;
77
use rustc_hir::{Expr, ExprKind, UnOp};
88
use rustc_lint::{LateContext, LateLintPass};
@@ -122,15 +122,7 @@ fn match_assert_with_message<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>)
122122
if let ExprKind::Unary(UnOp::Not, expr) = cond.kind;
123123
// bind the first argument of the `assert!` macro
124124
if let Some((Constant::Bool(is_true), _)) = constant(cx, cx.typeck_results(), expr);
125-
// block
126-
if let ExprKind::Block(block, _) = then.kind;
127-
if block.stmts.is_empty();
128-
if let Some(block_expr) = &block.expr;
129-
// inner block is optional. unwrap it if it exists, or use the expression as is otherwise.
130-
if let Some(begin_panic_call) = match block_expr.kind {
131-
ExprKind::Block(inner_block, _) => &inner_block.expr,
132-
_ => &block.expr,
133-
};
125+
let begin_panic_call = peel_blocks(then);
134126
// function call
135127
if let Some(arg) = match_panic_call(cx, begin_panic_call);
136128
// bind the second argument of the `assert!` macro if it exists

‎clippy_lints/src/bytecount.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ use clippy_utils::diagnostics::span_lint_and_sugg;
22
use clippy_utils::source::snippet_with_applicability;
33
use clippy_utils::ty::match_type;
44
use clippy_utils::visitors::is_local_used;
5-
use clippy_utils::{path_to_local_id, paths, peel_ref_operators, remove_blocks, strip_pat_refs};
5+
use clippy_utils::{path_to_local_id, paths, peel_blocks, peel_ref_operators, strip_pat_refs};
66
use if_chain::if_chain;
77
use rustc_errors::Applicability;
88
use rustc_hir::{BinOpKind, Expr, ExprKind, PatKind};
@@ -55,7 +55,7 @@ impl<'tcx> LateLintPass<'tcx> for ByteCount {
5555
cx.typeck_results().expr_ty(filter_recv).peel_refs(),
5656
&paths::SLICE_ITER);
5757
let operand_is_arg = |expr| {
58-
let expr = peel_ref_operators(cx, remove_blocks(expr));
58+
let expr = peel_ref_operators(cx, peel_blocks(expr));
5959
path_to_local_id(expr, arg_id)
6060
};
6161
let needle = if operand_is_arg(l) {

‎clippy_lints/src/collapsible_match.rs

Lines changed: 3 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,10 @@
11
use clippy_utils::diagnostics::span_lint_and_then;
22
use clippy_utils::higher::IfLetOrMatch;
33
use clippy_utils::visitors::is_local_used;
4-
use clippy_utils::{is_lang_ctor, is_unit_expr, path_to_local, peel_ref_operators, SpanlessEq};
4+
use clippy_utils::{is_lang_ctor, is_unit_expr, path_to_local, peel_blocks_with_stmt, peel_ref_operators, SpanlessEq};
55
use if_chain::if_chain;
66
use rustc_hir::LangItem::OptionNone;
7-
use rustc_hir::{Arm, Expr, ExprKind, Guard, HirId, Pat, PatKind, StmtKind};
7+
use rustc_hir::{Arm, Expr, Guard, HirId, Pat, PatKind};
88
use rustc_lint::{LateContext, LateLintPass};
99
use rustc_session::{declare_lint_pass, declare_tool_lint};
1010
use rustc_span::{MultiSpan, Span};
@@ -75,7 +75,7 @@ fn check_arm<'tcx>(
7575
outer_guard: Option<&'tcx Guard<'tcx>>,
7676
outer_else_body: Option<&'tcx Expr<'tcx>>,
7777
) {
78-
let inner_expr = strip_singleton_blocks(outer_then_body);
78+
let inner_expr = peel_blocks_with_stmt(outer_then_body);
7979
if_chain! {
8080
if let Some(inner) = IfLetOrMatch::parse(cx, inner_expr);
8181
if let Some((inner_scrutinee, inner_then_pat, inner_else_body)) = match inner {
@@ -138,20 +138,6 @@ fn check_arm<'tcx>(
138138
}
139139
}
140140

141-
fn strip_singleton_blocks<'hir>(mut expr: &'hir Expr<'hir>) -> &'hir Expr<'hir> {
142-
while let ExprKind::Block(block, _) = expr.kind {
143-
match (block.stmts, block.expr) {
144-
([stmt], None) => match stmt.kind {
145-
StmtKind::Expr(e) | StmtKind::Semi(e) => expr = e,
146-
_ => break,
147-
},
148-
([], Some(e)) => expr = e,
149-
_ => break,
150-
}
151-
}
152-
expr
153-
}
154-
155141
/// A "wild-like" arm has a wild (`_`) or `None` pattern and no guard. Such arms can be "collapsed"
156142
/// into a single wild arm without any significant loss in semantics or readability.
157143
fn arm_is_wild_like(cx: &LateContext<'_>, arm: &Arm<'_>) -> bool {

‎clippy_lints/src/derivable_impls.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
use clippy_utils::diagnostics::span_lint_and_help;
2-
use clippy_utils::{is_automatically_derived, is_default_equivalent, remove_blocks};
2+
use clippy_utils::{is_automatically_derived, is_default_equivalent, peel_blocks};
33
use rustc_hir::{
44
def::{DefKind, Res},
55
Body, Expr, ExprKind, GenericArg, Impl, ImplItemKind, Item, ItemKind, Node, PathSegment, QPath, TyKind,
@@ -95,7 +95,7 @@ impl<'tcx> LateLintPass<'tcx> for DerivableImpls {
9595
}
9696
}
9797
}
98-
let should_emit = match remove_blocks(func_expr).kind {
98+
let should_emit = match peel_blocks(func_expr).kind {
9999
ExprKind::Tup(fields) => fields.iter().all(|e| is_default_equivalent(cx, e)),
100100
ExprKind::Call(callee, args)
101101
if is_path_self(callee) => args.iter().all(|e| is_default_equivalent(cx, e)),

‎clippy_lints/src/floating_point_arithmetic.rs

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ use clippy_utils::consts::{
44
};
55
use clippy_utils::diagnostics::span_lint_and_sugg;
66
use clippy_utils::higher;
7-
use clippy_utils::{eq_expr_value, get_parent_expr, in_constant, numeric_literal, sugg};
7+
use clippy_utils::{eq_expr_value, get_parent_expr, in_constant, numeric_literal, peel_blocks, sugg};
88
use if_chain::if_chain;
99
use rustc_errors::Applicability;
1010
use rustc_hir::{BinOpKind, Expr, ExprKind, PathSegment, UnOp};
@@ -546,13 +546,9 @@ fn are_negated<'a>(cx: &LateContext<'_>, expr1: &'a Expr<'a>, expr2: &'a Expr<'a
546546

547547
fn check_custom_abs(cx: &LateContext<'_>, expr: &Expr<'_>) {
548548
if_chain! {
549-
if let Some(higher::If { cond, then, r#else }) = higher::If::hir(expr);
550-
if let ExprKind::Block(block, _) = then.kind;
551-
if block.stmts.is_empty();
552-
if let Some(if_body_expr) = block.expr;
553-
if let Some(ExprKind::Block(else_block, _)) = r#else.map(|el| &el.kind);
554-
if else_block.stmts.is_empty();
555-
if let Some(else_body_expr) = else_block.expr;
549+
if let Some(higher::If { cond, then, r#else: Some(r#else) }) = higher::If::hir(expr);
550+
let if_body_expr = peel_blocks(then);
551+
let else_body_expr = peel_blocks(r#else);
556552
if let Some((if_expr_positive, body)) = are_negated(cx, if_body_expr, else_body_expr);
557553
then {
558554
let positive_abs_sugg = (

‎clippy_lints/src/if_then_some_else_none.rs

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
use clippy_utils::diagnostics::span_lint_and_help;
22
use clippy_utils::source::snippet_with_macro_callsite;
3-
use clippy_utils::{contains_return, higher, is_else_clause, is_lang_ctor, meets_msrv, msrvs};
3+
use clippy_utils::{contains_return, higher, is_else_clause, is_lang_ctor, meets_msrv, msrvs, peel_blocks};
44
use if_chain::if_chain;
55
use rustc_hir::LangItem::{OptionNone, OptionSome};
66
use rustc_hir::{Expr, ExprKind, Stmt, StmtKind};
@@ -77,10 +77,7 @@ impl LateLintPass<'_> for IfThenSomeElseNone {
7777
if let ExprKind::Call(then_call, [then_arg]) = then_expr.kind;
7878
if let ExprKind::Path(ref then_call_qpath) = then_call.kind;
7979
if is_lang_ctor(cx, then_call_qpath, OptionSome);
80-
if let ExprKind::Block(els_block, _) = els.kind;
81-
if els_block.stmts.is_empty();
82-
if let Some(els_expr) = els_block.expr;
83-
if let ExprKind::Path(ref qpath) = els_expr.kind;
80+
if let ExprKind::Path(ref qpath) = peel_blocks(els).kind;
8481
if is_lang_ctor(cx, qpath, OptionNone);
8582
if !stmts_contains_early_return(then_block.stmts);
8683
then {

‎clippy_lints/src/implicit_saturating_sub.rs

Lines changed: 5 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,9 @@
11
use clippy_utils::diagnostics::span_lint_and_sugg;
2-
use clippy_utils::higher;
3-
use clippy_utils::SpanlessEq;
2+
use clippy_utils::{higher, peel_blocks_with_stmt, SpanlessEq};
43
use if_chain::if_chain;
54
use rustc_ast::ast::LitKind;
65
use rustc_errors::Applicability;
7-
use rustc_hir::{lang_items::LangItem, BinOpKind, Expr, ExprKind, QPath, StmtKind};
6+
use rustc_hir::{lang_items::LangItem, BinOpKind, Expr, ExprKind, QPath};
87
use rustc_lint::{LateContext, LateLintPass};
98
use rustc_session::{declare_lint_pass, declare_tool_lint};
109

@@ -52,13 +51,8 @@ impl<'tcx> LateLintPass<'tcx> for ImplicitSaturatingSub {
5251
// Ensure that the binary operator is >, != and <
5352
if BinOpKind::Ne == cond_op.node || BinOpKind::Gt == cond_op.node || BinOpKind::Lt == cond_op.node;
5453

55-
// Check if the true condition block has only one statement
56-
if let ExprKind::Block(block, _) = then.kind;
57-
if block.stmts.len() == 1 && block.expr.is_none();
58-
5954
// Check if assign operation is done
60-
if let StmtKind::Semi(e) = block.stmts[0].kind;
61-
if let Some(target) = subtracts_one(cx, e);
55+
if let Some(target) = subtracts_one(cx, then);
6256

6357
// Extracting out the variable name
6458
if let ExprKind::Path(QPath::Resolved(_, ares_path)) = target.kind;
@@ -138,8 +132,8 @@ impl<'tcx> LateLintPass<'tcx> for ImplicitSaturatingSub {
138132
}
139133
}
140134

141-
fn subtracts_one<'a>(cx: &LateContext<'_>, expr: &Expr<'a>) -> Option<&'a Expr<'a>> {
142-
match expr.kind {
135+
fn subtracts_one<'a>(cx: &LateContext<'_>, expr: &'a Expr<'a>) -> Option<&'a Expr<'a>> {
136+
match peel_blocks_with_stmt(expr).kind {
143137
ExprKind::AssignOp(ref op1, target, value) => {
144138
if_chain! {
145139
if BinOpKind::Sub == op1.node;

‎clippy_lints/src/loops/manual_flatten.rs

Lines changed: 48 additions & 64 deletions
Original file line numberDiff line numberDiff line change
@@ -3,11 +3,11 @@ use super::MANUAL_FLATTEN;
33
use clippy_utils::diagnostics::span_lint_and_then;
44
use clippy_utils::higher;
55
use clippy_utils::visitors::is_local_used;
6-
use clippy_utils::{is_lang_ctor, path_to_local_id};
6+
use clippy_utils::{is_lang_ctor, path_to_local_id, peel_blocks_with_stmt};
77
use if_chain::if_chain;
88
use rustc_errors::Applicability;
99
use rustc_hir::LangItem::{OptionSome, ResultOk};
10-
use rustc_hir::{Expr, ExprKind, Pat, PatKind, StmtKind};
10+
use rustc_hir::{Expr, Pat, PatKind};
1111
use rustc_lint::LateContext;
1212
use rustc_middle::ty;
1313
use rustc_span::source_map::Span;
@@ -21,71 +21,55 @@ pub(super) fn check<'tcx>(
2121
body: &'tcx Expr<'_>,
2222
span: Span,
2323
) {
24-
if let ExprKind::Block(block, _) = body.kind {
25-
// Ensure the `if let` statement is the only expression or statement in the for-loop
26-
let inner_expr = if block.stmts.len() == 1 && block.expr.is_none() {
27-
let match_stmt = &block.stmts[0];
28-
if let StmtKind::Semi(inner_expr) = match_stmt.kind {
29-
Some(inner_expr)
30-
} else {
31-
None
32-
}
33-
} else if block.stmts.is_empty() {
34-
block.expr
35-
} else {
36-
None
37-
};
24+
let inner_expr = peel_blocks_with_stmt(body);
25+
if_chain! {
26+
if let Some(higher::IfLet { let_pat, let_expr, if_then, if_else: None })
27+
= higher::IfLet::hir(cx, inner_expr);
28+
// Ensure match_expr in `if let` statement is the same as the pat from the for-loop
29+
if let PatKind::Binding(_, pat_hir_id, _, _) = pat.kind;
30+
if path_to_local_id(let_expr, pat_hir_id);
31+
// Ensure the `if let` statement is for the `Some` variant of `Option` or the `Ok` variant of `Result`
32+
if let PatKind::TupleStruct(ref qpath, _, _) = let_pat.kind;
33+
let some_ctor = is_lang_ctor(cx, qpath, OptionSome);
34+
let ok_ctor = is_lang_ctor(cx, qpath, ResultOk);
35+
if some_ctor || ok_ctor;
36+
// Ensure expr in `if let` is not used afterwards
37+
if !is_local_used(cx, if_then, pat_hir_id);
38+
then {
39+
let if_let_type = if some_ctor { "Some" } else { "Ok" };
40+
// Prepare the error message
41+
let msg = format!("unnecessary `if let` since only the `{}` variant of the iterator element is used", if_let_type);
3842

39-
if_chain! {
40-
if let Some(inner_expr) = inner_expr;
41-
if let Some(higher::IfLet { let_pat, let_expr, if_then, if_else: None })
42-
= higher::IfLet::hir(cx, inner_expr);
43-
// Ensure match_expr in `if let` statement is the same as the pat from the for-loop
44-
if let PatKind::Binding(_, pat_hir_id, _, _) = pat.kind;
45-
if path_to_local_id(let_expr, pat_hir_id);
46-
// Ensure the `if let` statement is for the `Some` variant of `Option` or the `Ok` variant of `Result`
47-
if let PatKind::TupleStruct(ref qpath, _, _) = let_pat.kind;
48-
let some_ctor = is_lang_ctor(cx, qpath, OptionSome);
49-
let ok_ctor = is_lang_ctor(cx, qpath, ResultOk);
50-
if some_ctor || ok_ctor;
51-
// Ensure epxr in `if let` is not used afterwards
52-
if !is_local_used(cx, if_then, pat_hir_id);
53-
then {
54-
let if_let_type = if some_ctor { "Some" } else { "Ok" };
55-
// Prepare the error message
56-
let msg = format!("unnecessary `if let` since only the `{}` variant of the iterator element is used", if_let_type);
57-
58-
// Prepare the help message
59-
let mut applicability = Applicability::MaybeIncorrect;
60-
let arg_snippet = make_iterator_snippet(cx, arg, &mut applicability);
61-
let copied = match cx.typeck_results().expr_ty(let_expr).kind() {
62-
ty::Ref(_, inner, _) => match inner.kind() {
63-
ty::Ref(..) => ".copied()",
64-
_ => ""
65-
}
43+
// Prepare the help message
44+
let mut applicability = Applicability::MaybeIncorrect;
45+
let arg_snippet = make_iterator_snippet(cx, arg, &mut applicability);
46+
let copied = match cx.typeck_results().expr_ty(let_expr).kind() {
47+
ty::Ref(_, inner, _) => match inner.kind() {
48+
ty::Ref(..) => ".copied()",
6649
_ => ""
67-
};
50+
}
51+
_ => ""
52+
};
6853

69-
span_lint_and_then(
70-
cx,
71-
MANUAL_FLATTEN,
72-
span,
73-
&msg,
74-
|diag| {
75-
let sugg = format!("{}{}.flatten()", arg_snippet, copied);
76-
diag.span_suggestion(
77-
arg.span,
78-
"try",
79-
sugg,
80-
Applicability::MaybeIncorrect,
81-
);
82-
diag.span_help(
83-
inner_expr.span,
84-
"...and remove the `if let` statement in the for loop",
85-
);
86-
}
87-
);
88-
}
54+
span_lint_and_then(
55+
cx,
56+
MANUAL_FLATTEN,
57+
span,
58+
&msg,
59+
|diag| {
60+
let sugg = format!("{}{}.flatten()", arg_snippet, copied);
61+
diag.span_suggestion(
62+
arg.span,
63+
"try",
64+
sugg,
65+
Applicability::MaybeIncorrect,
66+
);
67+
diag.span_help(
68+
inner_expr.span,
69+
"...and remove the `if let` statement in the for loop",
70+
);
71+
}
72+
);
8973
}
9074
}
9175
}

‎clippy_lints/src/loops/never_loop.rs

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -92,9 +92,7 @@ fn combine_branches(b1: NeverLoopResult, b2: NeverLoopResult) -> NeverLoopResult
9292
}
9393

9494
fn never_loop_block(block: &Block<'_>, main_loop_id: HirId) -> NeverLoopResult {
95-
let stmts = block.stmts.iter().map(stmt_to_expr);
96-
let expr = once(block.expr);
97-
let mut iter = stmts.chain(expr).flatten();
95+
let mut iter = block.stmts.iter().filter_map(stmt_to_expr).chain(block.expr);
9896
never_loop_expr_seq(&mut iter, main_loop_id)
9997
}
10098

‎clippy_lints/src/manual_map.rs

Lines changed: 2 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ use clippy_utils::source::{snippet_with_applicability, snippet_with_context};
55
use clippy_utils::ty::{is_type_diagnostic_item, peel_mid_ty_refs_is_mutable, type_is_unsafe_function};
66
use clippy_utils::{
77
can_move_expr_to_closure, in_constant, is_else_clause, is_lang_ctor, is_lint_allowed, path_to_local_id,
8-
peel_hir_expr_refs, peel_hir_expr_while, CaptureKind,
8+
peel_blocks, peel_hir_expr_refs, peel_hir_expr_while, CaptureKind,
99
};
1010
use rustc_ast::util::parser::PREC_POSTFIX;
1111
use rustc_errors::Applicability;
@@ -307,16 +307,5 @@ fn get_some_expr(
307307

308308
// Checks for the `None` value.
309309
fn is_none_expr(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) -> bool {
310-
match expr.kind {
311-
ExprKind::Path(ref qpath) => is_lang_ctor(cx, qpath, OptionNone),
312-
ExprKind::Block(
313-
Block {
314-
stmts: [],
315-
expr: Some(expr),
316-
..
317-
},
318-
_,
319-
) => is_none_expr(cx, expr),
320-
_ => false,
321-
}
310+
matches!(peel_blocks(expr).kind, ExprKind::Path(ref qpath) if is_lang_ctor(cx, qpath, OptionNone))
322311
}

‎clippy_lints/src/map_clone.rs

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,7 @@
11
use clippy_utils::diagnostics::span_lint_and_sugg;
2-
use clippy_utils::is_trait_method;
3-
use clippy_utils::remove_blocks;
42
use clippy_utils::source::snippet_with_applicability;
53
use clippy_utils::ty::{is_copy, is_type_diagnostic_item};
4+
use clippy_utils::{is_trait_method, peel_blocks};
65
use if_chain::if_chain;
76
use rustc_errors::Applicability;
87
use rustc_hir as hir;
@@ -60,7 +59,7 @@ impl<'tcx> LateLintPass<'tcx> for MapClone {
6059
if let hir::ExprKind::Closure(_, _, body_id, _, _) = args[1].kind;
6160
then {
6261
let closure_body = cx.tcx.hir().body(body_id);
63-
let closure_expr = remove_blocks(&closure_body.value);
62+
let closure_expr = peel_blocks(&closure_body.value);
6463
match closure_body.params[0].pat.kind {
6564
hir::PatKind::Ref(inner, hir::Mutability::Not) => if let hir::PatKind::Binding(
6665
hir::BindingAnnotation::Unannotated, .., name, None

0 commit comments

Comments
 (0)