Auto merge of rust-lang#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`

Author: bors

clippy_lints/src/assertions_on_constants.rs

@@ -2,7 +2,7 @@ use clippy_utils::consts::{constant, Constant};
 use clippy_utils::diagnostics::span_lint_and_help;
 use clippy_utils::higher;
 use clippy_utils::source::snippet_opt;
-use clippy_utils::{is_direct_expn_of, is_expn_of, match_panic_call};
+use clippy_utils::{is_direct_expn_of, is_expn_of, match_panic_call, peel_blocks};
 use if_chain::if_chain;
 use rustc_hir::{Expr, ExprKind, UnOp};
 use rustc_lint::{LateContext, LateLintPass};
@@ -122,15 +122,7 @@ fn match_assert_with_message<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>)
         if let ExprKind::Unary(UnOp::Not, expr) = cond.kind;
         // bind the first argument of the `assert!` macro
         if let Some((Constant::Bool(is_true), _)) = constant(cx, cx.typeck_results(), expr);
-        // block
-        if let ExprKind::Block(block, _) = then.kind;
-        if block.stmts.is_empty();
-        if let Some(block_expr) = &block.expr;
-        // inner block is optional. unwrap it if it exists, or use the expression as is otherwise.
-        if let Some(begin_panic_call) = match block_expr.kind {
-            ExprKind::Block(inner_block, _) => &inner_block.expr,
-            _ => &block.expr,
-        };
+        let begin_panic_call = peel_blocks(then);
         // function call
         if let Some(arg) = match_panic_call(cx, begin_panic_call);
         // bind the second argument of the `assert!` macro if it exists

clippy_lints/src/bytecount.rs

@@ -2,7 +2,7 @@ use clippy_utils::diagnostics::span_lint_and_sugg;
 use clippy_utils::source::snippet_with_applicability;
 use clippy_utils::ty::match_type;
 use clippy_utils::visitors::is_local_used;
-use clippy_utils::{path_to_local_id, paths, peel_ref_operators, remove_blocks, strip_pat_refs};
+use clippy_utils::{path_to_local_id, paths, peel_blocks, peel_ref_operators, strip_pat_refs};
 use if_chain::if_chain;
 use rustc_errors::Applicability;
 use rustc_hir::{BinOpKind, Expr, ExprKind, PatKind};
@@ -55,7 +55,7 @@ impl<'tcx> LateLintPass<'tcx> for ByteCount {
                        cx.typeck_results().expr_ty(filter_recv).peel_refs(),
                        &paths::SLICE_ITER);
             let operand_is_arg = |expr| {
-                let expr = peel_ref_operators(cx, remove_blocks(expr));
+                let expr = peel_ref_operators(cx, peel_blocks(expr));
                 path_to_local_id(expr, arg_id)
             };
             let needle = if operand_is_arg(l) {

clippy_lints/src/collapsible_match.rs

@@ -1,10 +1,10 @@
 use clippy_utils::diagnostics::span_lint_and_then;
 use clippy_utils::higher::IfLetOrMatch;
 use clippy_utils::visitors::is_local_used;
-use clippy_utils::{is_lang_ctor, is_unit_expr, path_to_local, peel_ref_operators, SpanlessEq};
+use clippy_utils::{is_lang_ctor, is_unit_expr, path_to_local, peel_blocks_with_stmt, peel_ref_operators, SpanlessEq};
 use if_chain::if_chain;
 use rustc_hir::LangItem::OptionNone;
-use rustc_hir::{Arm, Expr, ExprKind, Guard, HirId, Pat, PatKind, StmtKind};
+use rustc_hir::{Arm, Expr, Guard, HirId, Pat, PatKind};
 use rustc_lint::{LateContext, LateLintPass};
 use rustc_session::{declare_lint_pass, declare_tool_lint};
 use rustc_span::{MultiSpan, Span};
@@ -75,7 +75,7 @@ fn check_arm<'tcx>(
     outer_guard: Option<&'tcx Guard<'tcx>>,
     outer_else_body: Option<&'tcx Expr<'tcx>>,
 ) {
-    let inner_expr = strip_singleton_blocks(outer_then_body);
+    let inner_expr = peel_blocks_with_stmt(outer_then_body);
     if_chain! {
         if let Some(inner) = IfLetOrMatch::parse(cx, inner_expr);
         if let Some((inner_scrutinee, inner_then_pat, inner_else_body)) = match inner {
@@ -138,20 +138,6 @@ fn check_arm<'tcx>(
     }
 }
 
-fn strip_singleton_blocks<'hir>(mut expr: &'hir Expr<'hir>) -> &'hir Expr<'hir> {
-    while let ExprKind::Block(block, _) = expr.kind {
-        match (block.stmts, block.expr) {
-            ([stmt], None) => match stmt.kind {
-                StmtKind::Expr(e) | StmtKind::Semi(e) => expr = e,
-                _ => break,
-            },
-            ([], Some(e)) => expr = e,
-            _ => break,
-        }
-    }
-    expr
-}
-
 /// A "wild-like" arm has a wild (`_`) or `None` pattern and no guard. Such arms can be "collapsed"
 /// into a single wild arm without any significant loss in semantics or readability.
 fn arm_is_wild_like(cx: &LateContext<'_>, arm: &Arm<'_>) -> bool {

clippy_lints/src/derivable_impls.rs

@@ -1,5 +1,5 @@
 use clippy_utils::diagnostics::span_lint_and_help;
-use clippy_utils::{is_automatically_derived, is_default_equivalent, remove_blocks};
+use clippy_utils::{is_automatically_derived, is_default_equivalent, peel_blocks};
 use rustc_hir::{
     def::{DefKind, Res},
     Body, Expr, ExprKind, GenericArg, Impl, ImplItemKind, Item, ItemKind, Node, PathSegment, QPath, TyKind,
@@ -95,7 +95,7 @@ impl<'tcx> LateLintPass<'tcx> for DerivableImpls {
                         }
                     }
                 }
-                let should_emit = match remove_blocks(func_expr).kind {
+                let should_emit = match peel_blocks(func_expr).kind {
                     ExprKind::Tup(fields) => fields.iter().all(|e| is_default_equivalent(cx, e)),
                     ExprKind::Call(callee, args)
                         if is_path_self(callee) => args.iter().all(|e| is_default_equivalent(cx, e)),

clippy_lints/src/floating_point_arithmetic.rs

@@ -4,7 +4,7 @@ use clippy_utils::consts::{
 };
 use clippy_utils::diagnostics::span_lint_and_sugg;
 use clippy_utils::higher;
-use clippy_utils::{eq_expr_value, get_parent_expr, in_constant, numeric_literal, sugg};
+use clippy_utils::{eq_expr_value, get_parent_expr, in_constant, numeric_literal, peel_blocks, sugg};
 use if_chain::if_chain;
 use rustc_errors::Applicability;
 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
 
 fn check_custom_abs(cx: &LateContext<'_>, expr: &Expr<'_>) {
     if_chain! {
-        if let Some(higher::If { cond, then, r#else }) = higher::If::hir(expr);
-        if let ExprKind::Block(block, _) = then.kind;
-        if block.stmts.is_empty();
-        if let Some(if_body_expr) = block.expr;
-        if let Some(ExprKind::Block(else_block, _)) = r#else.map(|el| &el.kind);
-        if else_block.stmts.is_empty();
-        if let Some(else_body_expr) = else_block.expr;
+        if let Some(higher::If { cond, then, r#else: Some(r#else) }) = higher::If::hir(expr);
+        let if_body_expr = peel_blocks(then);
+        let else_body_expr = peel_blocks(r#else);
         if let Some((if_expr_positive, body)) = are_negated(cx, if_body_expr, else_body_expr);
         then {
             let positive_abs_sugg = (

clippy_lints/src/if_then_some_else_none.rs

@@ -1,6 +1,6 @@
 use clippy_utils::diagnostics::span_lint_and_help;
 use clippy_utils::source::snippet_with_macro_callsite;
-use clippy_utils::{contains_return, higher, is_else_clause, is_lang_ctor, meets_msrv, msrvs};
+use clippy_utils::{contains_return, higher, is_else_clause, is_lang_ctor, meets_msrv, msrvs, peel_blocks};
 use if_chain::if_chain;
 use rustc_hir::LangItem::{OptionNone, OptionSome};
 use rustc_hir::{Expr, ExprKind, Stmt, StmtKind};
@@ -77,10 +77,7 @@ impl LateLintPass<'_> for IfThenSomeElseNone {
             if let ExprKind::Call(then_call, [then_arg]) = then_expr.kind;
             if let ExprKind::Path(ref then_call_qpath) = then_call.kind;
             if is_lang_ctor(cx, then_call_qpath, OptionSome);
-            if let ExprKind::Block(els_block, _) = els.kind;
-            if els_block.stmts.is_empty();
-            if let Some(els_expr) = els_block.expr;
-            if let ExprKind::Path(ref qpath) = els_expr.kind;
+            if let ExprKind::Path(ref qpath) = peel_blocks(els).kind;
             if is_lang_ctor(cx, qpath, OptionNone);
             if !stmts_contains_early_return(then_block.stmts);
             then {

clippy_lints/src/implicit_saturating_sub.rs

@@ -1,10 +1,9 @@
 use clippy_utils::diagnostics::span_lint_and_sugg;
-use clippy_utils::higher;
-use clippy_utils::SpanlessEq;
+use clippy_utils::{higher, peel_blocks_with_stmt, SpanlessEq};
 use if_chain::if_chain;
 use rustc_ast::ast::LitKind;
 use rustc_errors::Applicability;
-use rustc_hir::{lang_items::LangItem, BinOpKind, Expr, ExprKind, QPath, StmtKind};
+use rustc_hir::{lang_items::LangItem, BinOpKind, Expr, ExprKind, QPath};
 use rustc_lint::{LateContext, LateLintPass};
 use rustc_session::{declare_lint_pass, declare_tool_lint};
 
@@ -52,13 +51,8 @@ impl<'tcx> LateLintPass<'tcx> for ImplicitSaturatingSub {
             // Ensure that the binary operator is >, != and <
             if BinOpKind::Ne == cond_op.node || BinOpKind::Gt == cond_op.node || BinOpKind::Lt == cond_op.node;
 
-            // Check if the true condition block has only one statement
-            if let ExprKind::Block(block, _) = then.kind;
-            if block.stmts.len() == 1 && block.expr.is_none();
-
             // Check if assign operation is done
-            if let StmtKind::Semi(e) = block.stmts[0].kind;
-            if let Some(target) = subtracts_one(cx, e);
+            if let Some(target) = subtracts_one(cx, then);
 
             // Extracting out the variable name
             if let ExprKind::Path(QPath::Resolved(_, ares_path)) = target.kind;
@@ -138,8 +132,8 @@ impl<'tcx> LateLintPass<'tcx> for ImplicitSaturatingSub {
     }
 }
 
-fn subtracts_one<'a>(cx: &LateContext<'_>, expr: &Expr<'a>) -> Option<&'a Expr<'a>> {
-    match expr.kind {
+fn subtracts_one<'a>(cx: &LateContext<'_>, expr: &'a Expr<'a>) -> Option<&'a Expr<'a>> {
+    match peel_blocks_with_stmt(expr).kind {
         ExprKind::AssignOp(ref op1, target, value) => {
             if_chain! {
                 if BinOpKind::Sub == op1.node;

clippy_lints/src/loops/manual_flatten.rs

@@ -3,11 +3,11 @@ use super::MANUAL_FLATTEN;
 use clippy_utils::diagnostics::span_lint_and_then;
 use clippy_utils::higher;
 use clippy_utils::visitors::is_local_used;
-use clippy_utils::{is_lang_ctor, path_to_local_id};
+use clippy_utils::{is_lang_ctor, path_to_local_id, peel_blocks_with_stmt};
 use if_chain::if_chain;
 use rustc_errors::Applicability;
 use rustc_hir::LangItem::{OptionSome, ResultOk};
-use rustc_hir::{Expr, ExprKind, Pat, PatKind, StmtKind};
+use rustc_hir::{Expr, Pat, PatKind};
 use rustc_lint::LateContext;
 use rustc_middle::ty;
 use rustc_span::source_map::Span;
@@ -21,71 +21,55 @@ pub(super) fn check<'tcx>(
     body: &'tcx Expr<'_>,
     span: Span,
 ) {
-    if let ExprKind::Block(block, _) = body.kind {
-        // Ensure the `if let` statement is the only expression or statement in the for-loop
-        let inner_expr = if block.stmts.len() == 1 && block.expr.is_none() {
-            let match_stmt = &block.stmts[0];
-            if let StmtKind::Semi(inner_expr) = match_stmt.kind {
-                Some(inner_expr)
-            } else {
-                None
-            }
-        } else if block.stmts.is_empty() {
-            block.expr
-        } else {
-            None
-        };
+    let inner_expr = peel_blocks_with_stmt(body);
+    if_chain! {
+        if let Some(higher::IfLet { let_pat, let_expr, if_then, if_else: None })
+            = higher::IfLet::hir(cx, inner_expr);
+        // Ensure match_expr in `if let` statement is the same as the pat from the for-loop
+        if let PatKind::Binding(_, pat_hir_id, _, _) = pat.kind;
+        if path_to_local_id(let_expr, pat_hir_id);
+        // Ensure the `if let` statement is for the `Some` variant of `Option` or the `Ok` variant of `Result`
+        if let PatKind::TupleStruct(ref qpath, _, _) = let_pat.kind;
+        let some_ctor = is_lang_ctor(cx, qpath, OptionSome);
+        let ok_ctor = is_lang_ctor(cx, qpath, ResultOk);
+        if some_ctor || ok_ctor;
+        // Ensure expr in `if let` is not used afterwards
+        if !is_local_used(cx, if_then, pat_hir_id);
+        then {
+            let if_let_type = if some_ctor { "Some" } else { "Ok" };
+            // Prepare the error message
+            let msg = format!("unnecessary `if let` since only the `{}` variant of the iterator element is used", if_let_type);
 
-        if_chain! {
-            if let Some(inner_expr) = inner_expr;
-            if let Some(higher::IfLet { let_pat, let_expr, if_then, if_else: None })
-                = higher::IfLet::hir(cx, inner_expr);
-            // Ensure match_expr in `if let` statement is the same as the pat from the for-loop
-            if let PatKind::Binding(_, pat_hir_id, _, _) = pat.kind;
-            if path_to_local_id(let_expr, pat_hir_id);
-            // Ensure the `if let` statement is for the `Some` variant of `Option` or the `Ok` variant of `Result`
-            if let PatKind::TupleStruct(ref qpath, _, _) = let_pat.kind;
-            let some_ctor = is_lang_ctor(cx, qpath, OptionSome);
-            let ok_ctor = is_lang_ctor(cx, qpath, ResultOk);
-            if some_ctor || ok_ctor;
-            // Ensure epxr in `if let` is not used afterwards
-            if !is_local_used(cx, if_then, pat_hir_id);
-            then {
-                let if_let_type = if some_ctor { "Some" } else { "Ok" };
-                // Prepare the error message
-                let msg = format!("unnecessary `if let` since only the `{}` variant of the iterator element is used", if_let_type);
-
-                // Prepare the help message
-                let mut applicability = Applicability::MaybeIncorrect;
-                let arg_snippet = make_iterator_snippet(cx, arg, &mut applicability);
-                let copied = match cx.typeck_results().expr_ty(let_expr).kind() {
-                    ty::Ref(_, inner, _) => match inner.kind() {
-                        ty::Ref(..) => ".copied()",
-                        _ => ""
-                    }
+            // Prepare the help message
+            let mut applicability = Applicability::MaybeIncorrect;
+            let arg_snippet = make_iterator_snippet(cx, arg, &mut applicability);
+            let copied = match cx.typeck_results().expr_ty(let_expr).kind() {
+                ty::Ref(_, inner, _) => match inner.kind() {
+                    ty::Ref(..) => ".copied()",
                     _ => ""
-                };
+                }
+                _ => ""
+            };
 
-                span_lint_and_then(
-                    cx,
-                    MANUAL_FLATTEN,
-                    span,
-                    &msg,
-                    |diag| {
-                        let sugg = format!("{}{}.flatten()", arg_snippet, copied);
-                        diag.span_suggestion(
-                            arg.span,
-                            "try",
-                            sugg,
-                            Applicability::MaybeIncorrect,
-                        );
-                        diag.span_help(
-                            inner_expr.span,
-                            "...and remove the `if let` statement in the for loop",
-                        );
-                    }
-                );
-            }
+            span_lint_and_then(
+                cx,
+                MANUAL_FLATTEN,
+                span,
+                &msg,
+                |diag| {
+                    let sugg = format!("{}{}.flatten()", arg_snippet, copied);
+                    diag.span_suggestion(
+                        arg.span,
+                        "try",
+                        sugg,
+                        Applicability::MaybeIncorrect,
+                    );
+                    diag.span_help(
+                        inner_expr.span,
+                        "...and remove the `if let` statement in the for loop",
+                    );
+                }
+            );
         }
     }
 }

clippy_lints/src/loops/never_loop.rs

@@ -92,9 +92,7 @@ fn combine_branches(b1: NeverLoopResult, b2: NeverLoopResult) -> NeverLoopResult
 }
 
 fn never_loop_block(block: &Block<'_>, main_loop_id: HirId) -> NeverLoopResult {
-    let stmts = block.stmts.iter().map(stmt_to_expr);
-    let expr = once(block.expr);
-    let mut iter = stmts.chain(expr).flatten();
+    let mut iter = block.stmts.iter().filter_map(stmt_to_expr).chain(block.expr);
     never_loop_expr_seq(&mut iter, main_loop_id)
 }
 

clippy_lints/src/manual_map.rs

@@ -5,7 +5,7 @@ use clippy_utils::source::{snippet_with_applicability, snippet_with_context};
 use clippy_utils::ty::{is_type_diagnostic_item, peel_mid_ty_refs_is_mutable, type_is_unsafe_function};
 use clippy_utils::{
     can_move_expr_to_closure, in_constant, is_else_clause, is_lang_ctor, is_lint_allowed, path_to_local_id,
-    peel_hir_expr_refs, peel_hir_expr_while, CaptureKind,
+    peel_blocks, peel_hir_expr_refs, peel_hir_expr_while, CaptureKind,
 };
 use rustc_ast::util::parser::PREC_POSTFIX;
 use rustc_errors::Applicability;
@@ -307,16 +307,5 @@ fn get_some_expr(
 
 // Checks for the `None` value.
 fn is_none_expr(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) -> bool {
-    match expr.kind {
-        ExprKind::Path(ref qpath) => is_lang_ctor(cx, qpath, OptionNone),
-        ExprKind::Block(
-            Block {
-                stmts: [],
-                expr: Some(expr),
-                ..
-            },
-            _,
-        ) => is_none_expr(cx, expr),
-        _ => false,
-    }
+    matches!(peel_blocks(expr).kind, ExprKind::Path(ref qpath) if is_lang_ctor(cx, qpath, OptionNone))
 }