From f690ef6f5c0d6af58e8e908b8e08c09c4921ac35 Mon Sep 17 00:00:00 2001 From: Cameron Steffen Date: Mon, 29 Nov 2021 08:34:36 -0600 Subject: [PATCH] Refactors with peel_blocks --- clippy_lints/src/assertions_on_constants.rs | 12 +- clippy_lints/src/collapsible_match.rs | 20 +--- clippy_lints/src/floating_point_arithmetic.rs | 12 +- clippy_lints/src/if_then_some_else_none.rs | 7 +- clippy_lints/src/implicit_saturating_sub.rs | 16 +-- clippy_lints/src/loops/manual_flatten.rs | 112 ++++++++---------- clippy_lints/src/loops/never_loop.rs | 4 +- clippy_lints/src/manual_map.rs | 15 +-- clippy_lints/src/no_effect.rs | 6 +- clippy_lints/src/option_if_let_else.rs | 33 +----- clippy_lints/src/question_mark.rs | 63 ++-------- clippy_lints/src/strings.rs | 7 +- clippy_lints/src/utils/internal_lints.rs | 10 +- .../internal_lints/metadata_collector.rs | 11 +- clippy_utils/src/lib.rs | 75 +++++++++--- 15 files changed, 151 insertions(+), 252 deletions(-) diff --git a/clippy_lints/src/assertions_on_constants.rs b/clippy_lints/src/assertions_on_constants.rs index 521fc84ee..b7f414742 100644 --- a/clippy_lints/src/assertions_on_constants.rs +++ b/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 diff --git a/clippy_lints/src/collapsible_match.rs b/clippy_lints/src/collapsible_match.rs index 626f9971f..c71e9f10f 100644 --- a/clippy_lints/src/collapsible_match.rs +++ b/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 { diff --git a/clippy_lints/src/floating_point_arithmetic.rs b/clippy_lints/src/floating_point_arithmetic.rs index 3df511ea8..2de2bfc04 100644 --- a/clippy_lints/src/floating_point_arithmetic.rs +++ b/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 = ( diff --git a/clippy_lints/src/if_then_some_else_none.rs b/clippy_lints/src/if_then_some_else_none.rs index 30d222bd7..16e5c5ca6 100644 --- a/clippy_lints/src/if_then_some_else_none.rs +++ b/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 { diff --git a/clippy_lints/src/implicit_saturating_sub.rs b/clippy_lints/src/implicit_saturating_sub.rs index 4088c5462..26a196aab 100644 --- a/clippy_lints/src/implicit_saturating_sub.rs +++ b/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; diff --git a/clippy_lints/src/loops/manual_flatten.rs b/clippy_lints/src/loops/manual_flatten.rs index 5b6e27085..d276c9010 100644 --- a/clippy_lints/src/loops/manual_flatten.rs +++ b/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", + ); + } + ); } } } diff --git a/clippy_lints/src/loops/never_loop.rs b/clippy_lints/src/loops/never_loop.rs index 86b7d6d98..68ffcd1ab 100644 --- a/clippy_lints/src/loops/never_loop.rs +++ b/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) } diff --git a/clippy_lints/src/manual_map.rs b/clippy_lints/src/manual_map.rs index 4d8ad566e..34a70ca76 100644 --- a/clippy_lints/src/manual_map.rs +++ b/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)) } diff --git a/clippy_lints/src/no_effect.rs b/clippy_lints/src/no_effect.rs index 6fcc9ca29..9d5babc5d 100644 --- a/clippy_lints/src/no_effect.rs +++ b/clippy_lints/src/no_effect.rs @@ -1,5 +1,6 @@ use clippy_utils::diagnostics::{span_lint_hir, span_lint_hir_and_then}; use clippy_utils::is_lint_allowed; +use clippy_utils::peel_blocks; use clippy_utils::source::snippet_opt; use clippy_utils::ty::has_drop; use rustc_errors::Applicability; @@ -114,7 +115,7 @@ fn has_no_effect(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool { if expr.span.from_expansion() { return false; } - match expr.kind { + match peel_blocks(expr).kind { ExprKind::Lit(..) | ExprKind::Closure(..) => true, ExprKind::Path(..) => !has_drop(cx, cx.typeck_results().expr_ty(expr)), ExprKind::Index(a, b) | ExprKind::Binary(_, a, b) => has_no_effect(cx, a) && has_no_effect(cx, b), @@ -150,9 +151,6 @@ fn has_no_effect(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool { false } }, - ExprKind::Block(block, _) => { - block.stmts.is_empty() && block.expr.as_ref().map_or(false, |expr| has_no_effect(cx, expr)) - }, _ => false, } } diff --git a/clippy_lints/src/option_if_let_else.rs b/clippy_lints/src/option_if_let_else.rs index 262be17f6..897207ebf 100644 --- a/clippy_lints/src/option_if_let_else.rs +++ b/clippy_lints/src/option_if_let_else.rs @@ -1,15 +1,14 @@ use clippy_utils::diagnostics::span_lint_and_sugg; -use clippy_utils::higher; use clippy_utils::sugg::Sugg; use clippy_utils::ty::is_type_diagnostic_item; use clippy_utils::{ - can_move_expr_to_closure, eager_or_lazy, in_constant, is_else_clause, is_lang_ctor, peel_hir_expr_while, - CaptureKind, + can_move_expr_to_closure, eager_or_lazy, higher, in_constant, is_else_clause, is_lang_ctor, peel_blocks, + peel_hir_expr_while, CaptureKind, }; use if_chain::if_chain; use rustc_errors::Applicability; use rustc_hir::LangItem::OptionSome; -use rustc_hir::{def::Res, BindingAnnotation, Block, Expr, ExprKind, Mutability, PatKind, Path, QPath, UnOp}; +use rustc_hir::{def::Res, BindingAnnotation, Expr, ExprKind, Mutability, PatKind, Path, QPath, UnOp}; use rustc_lint::{LateContext, LateLintPass}; use rustc_session::{declare_lint_pass, declare_tool_lint}; use rustc_span::sym; @@ -86,28 +85,6 @@ struct OptionIfLetElseOccurence { none_expr: String, } -/// Extracts the body of a given arm. If the arm contains only an expression, -/// then it returns the expression. Otherwise, it returns the entire block -fn extract_body_from_expr<'a>(expr: &'a Expr<'a>) -> Option<&'a Expr<'a>> { - if let ExprKind::Block( - Block { - stmts: block_stmts, - expr: Some(block_expr), - .. - }, - _, - ) = expr.kind - { - if let [] = block_stmts { - Some(block_expr) - } else { - Some(expr) - } - } else { - None - } -} - fn format_option_in_sugg(cx: &LateContext<'_>, cond_expr: &Expr<'_>, as_ref: bool, as_mut: bool) -> String { format!( "{}{}", @@ -145,8 +122,8 @@ fn detect_option_if_let_else<'tcx>(cx: &LateContext<'tcx>, expr: &Expr<'tcx>) -> then { let capture_mut = if bind_annotation == &BindingAnnotation::Mutable { "mut " } else { "" }; - let some_body = extract_body_from_expr(if_then)?; - let none_body = extract_body_from_expr(if_else)?; + let some_body = peel_blocks(if_then); + let none_body = peel_blocks(if_else); let method_sugg = if eager_or_lazy::switch_to_eager_eval(cx, none_body) { "map_or" } else { "map_or_else" }; let capture_name = id.name.to_ident_string(); let (as_ref, as_mut) = match &let_expr.kind { diff --git a/clippy_lints/src/question_mark.rs b/clippy_lints/src/question_mark.rs index a5531993e..65a962dcd 100644 --- a/clippy_lints/src/question_mark.rs +++ b/clippy_lints/src/question_mark.rs @@ -1,14 +1,13 @@ use clippy_utils::diagnostics::span_lint_and_sugg; use clippy_utils::higher; -use clippy_utils::is_lang_ctor; use clippy_utils::source::snippet_with_applicability; use clippy_utils::sugg::Sugg; use clippy_utils::ty::is_type_diagnostic_item; -use clippy_utils::{eq_expr_value, path_to_local, path_to_local_id}; +use clippy_utils::{eq_expr_value, is_lang_ctor, path_to_local, path_to_local_id, peel_blocks, peel_blocks_with_stmt}; use if_chain::if_chain; use rustc_errors::Applicability; use rustc_hir::LangItem::{OptionNone, OptionSome, ResultOk}; -use rustc_hir::{BindingAnnotation, Block, Expr, ExprKind, PatKind, StmtKind}; +use rustc_hir::{BindingAnnotation, Expr, ExprKind, PatKind}; use rustc_lint::{LateContext, LateLintPass}; use rustc_session::{declare_lint_pass, declare_tool_lint}; use rustc_span::sym; @@ -68,14 +67,8 @@ impl QuestionMark { let receiver_str = &Sugg::hir_with_applicability(cx, subject, "..", &mut applicability); let mut replacement: Option = None; if let Some(else_inner) = r#else { - if_chain! { - if let ExprKind::Block(block, None) = &else_inner.kind; - if block.stmts.is_empty(); - if let Some(block_expr) = &block.expr; - if eq_expr_value(cx, subject, block_expr); - then { - replacement = Some(format!("Some({}?)", receiver_str)); - } + if eq_expr_value(cx, subject, peel_blocks(else_inner)) { + replacement = Some(format!("Some({}?)", receiver_str)); } } else if Self::moves_by_default(cx, subject) && !matches!(subject.kind, ExprKind::Call(..) | ExprKind::MethodCall(..)) @@ -110,10 +103,7 @@ impl QuestionMark { if let PatKind::Binding(annot, bind_id, _, _) = fields[0].kind; let by_ref = matches!(annot, BindingAnnotation::Ref | BindingAnnotation::RefMut); - if let ExprKind::Block(block, None) = if_then.kind; - if block.stmts.is_empty(); - if let Some(trailing_expr) = &block.expr; - if path_to_local_id(trailing_expr, bind_id); + if path_to_local_id(peel_blocks(if_then), bind_id); then { let mut applicability = Applicability::MachineApplicable; let receiver_str = snippet_with_applicability(cx, let_expr.span, "..", &mut applicability); @@ -159,14 +149,7 @@ impl QuestionMark { } fn expression_returns_none(cx: &LateContext<'_>, expression: &Expr<'_>) -> bool { - match expression.kind { - ExprKind::Block(block, _) => { - if let Some(return_expression) = Self::return_expression(block) { - return Self::expression_returns_none(cx, return_expression); - } - - false - }, + match peel_blocks_with_stmt(expression).kind { ExprKind::Ret(Some(expr)) => Self::expression_returns_none(cx, expr), ExprKind::Path(ref qpath) => is_lang_ctor(cx, qpath, OptionNone), _ => false, @@ -174,44 +157,12 @@ impl QuestionMark { } fn expression_returns_unmodified_err(cx: &LateContext<'_>, expr: &Expr<'_>, cond_expr: &Expr<'_>) -> bool { - match expr.kind { - ExprKind::Block(block, _) => { - if let Some(return_expression) = Self::return_expression(block) { - return Self::expression_returns_unmodified_err(cx, return_expression, cond_expr); - } - - false - }, + match peel_blocks_with_stmt(expr).kind { ExprKind::Ret(Some(ret_expr)) => Self::expression_returns_unmodified_err(cx, ret_expr, cond_expr), ExprKind::Path(_) => path_to_local(expr) == path_to_local(cond_expr), _ => false, } } - - fn return_expression<'tcx>(block: &Block<'tcx>) -> Option<&'tcx Expr<'tcx>> { - // Check if last expression is a return statement. Then, return the expression - if_chain! { - if block.stmts.len() == 1; - if let Some(expr) = block.stmts.iter().last(); - if let StmtKind::Semi(expr) = expr.kind; - if let ExprKind::Ret(Some(ret_expr)) = expr.kind; - - then { - return Some(ret_expr); - } - } - - // Check for `return` without a semicolon. - if_chain! { - if block.stmts.is_empty(); - if let Some(ExprKind::Ret(Some(ret_expr))) = block.expr.as_ref().map(|e| &e.kind); - then { - return Some(ret_expr); - } - } - - None - } } impl<'tcx> LateLintPass<'tcx> for QuestionMark { diff --git a/clippy_lints/src/strings.rs b/clippy_lints/src/strings.rs index 368274440..b03f45833 100644 --- a/clippy_lints/src/strings.rs +++ b/clippy_lints/src/strings.rs @@ -1,8 +1,8 @@ use clippy_utils::diagnostics::{span_lint, span_lint_and_help, span_lint_and_sugg}; use clippy_utils::source::{snippet, snippet_with_applicability}; use clippy_utils::ty::is_type_diagnostic_item; -use clippy_utils::SpanlessEq; use clippy_utils::{get_parent_expr, is_lint_allowed, match_function_call, method_calls, paths}; +use clippy_utils::{peel_blocks, SpanlessEq}; use if_chain::if_chain; use rustc_errors::Applicability; use rustc_hir::{BinOpKind, BorrowKind, Expr, ExprKind, LangItem, QPath}; @@ -201,7 +201,7 @@ fn is_string(cx: &LateContext<'_>, e: &Expr<'_>) -> bool { } fn is_add(cx: &LateContext<'_>, src: &Expr<'_>, target: &Expr<'_>) -> bool { - match src.kind { + match peel_blocks(src).kind { ExprKind::Binary( Spanned { node: BinOpKind::Add, .. @@ -209,9 +209,6 @@ fn is_add(cx: &LateContext<'_>, src: &Expr<'_>, target: &Expr<'_>) -> bool { left, _, ) => SpanlessEq::new(cx).eq_expr(target, left), - ExprKind::Block(block, _) => { - block.stmts.is_empty() && block.expr.as_ref().map_or(false, |expr| is_add(cx, expr, target)) - }, _ => false, } } diff --git a/clippy_lints/src/utils/internal_lints.rs b/clippy_lints/src/utils/internal_lints.rs index 1f97c8ba7..81e9c2e15 100644 --- a/clippy_lints/src/utils/internal_lints.rs +++ b/clippy_lints/src/utils/internal_lints.rs @@ -1,11 +1,10 @@ use clippy_utils::consts::{constant_simple, Constant}; use clippy_utils::diagnostics::{span_lint, span_lint_and_help, span_lint_and_sugg, span_lint_and_then}; -use clippy_utils::higher; use clippy_utils::source::snippet; use clippy_utils::ty::match_type; use clippy_utils::{ - is_else_clause, is_expn_of, is_expr_path_def_path, is_lint_allowed, match_def_path, method_calls, path_to_res, - paths, SpanlessEq, + higher, is_else_clause, is_expn_of, is_expr_path_def_path, is_lint_allowed, match_def_path, method_calls, + path_to_res, paths, peel_blocks_with_stmt, SpanlessEq, }; use if_chain::if_chain; use rustc_ast as ast; @@ -662,10 +661,7 @@ impl<'tcx> LateLintPass<'tcx> for CollapsibleCalls { if and_then_args.len() == 5; if let ExprKind::Closure(_, _, body_id, _, _) = &and_then_args[4].kind; let body = cx.tcx.hir().body(*body_id); - if let ExprKind::Block(block, _) = &body.value.kind; - let stmts = &block.stmts; - if stmts.len() == 1 && block.expr.is_none(); - if let StmtKind::Semi(only_expr) = &stmts[0].kind; + let only_expr = peel_blocks_with_stmt(&body.value); if let ExprKind::MethodCall(ps, _, span_call_args, _) = &only_expr.kind; then { let and_then_snippets = get_and_then_snippets(cx, and_then_args); diff --git a/clippy_lints/src/utils/internal_lints/metadata_collector.rs b/clippy_lints/src/utils/internal_lints/metadata_collector.rs index 8051c58ba..7707eebd6 100644 --- a/clippy_lints/src/utils/internal_lints/metadata_collector.rs +++ b/clippy_lints/src/utils/internal_lints/metadata_collector.rs @@ -8,6 +8,11 @@ //! during any comparison or mapping. (Please take care of this, it's not fun to spend time on such //! a simple mistake) +use crate::utils::internal_lints::{extract_clippy_version_value, is_lint_ref_type}; + +use clippy_utils::diagnostics::span_lint; +use clippy_utils::ty::{match_type, walk_ptrs_ty_depth}; +use clippy_utils::{last_path_segment, match_def_path, match_function_call, match_path, paths}; use if_chain::if_chain; use rustc_ast as ast; use rustc_data_structures::fx::FxHashMap; @@ -25,12 +30,6 @@ use std::fs::{self, OpenOptions}; use std::io::prelude::*; use std::path::Path; -use crate::utils::internal_lints::{extract_clippy_version_value, is_lint_ref_type}; -use clippy_utils::{ - diagnostics::span_lint, last_path_segment, match_def_path, match_function_call, match_path, paths, ty::match_type, - ty::walk_ptrs_ty_depth, -}; - /// This is the output file of the lint collector. const OUTPUT_FILE: &str = "../util/gh-pages/lints.json"; /// These lints are excluded from the export. diff --git a/clippy_utils/src/lib.rs b/clippy_utils/src/lib.rs index 2991c5880..4e2143614 100644 --- a/clippy_utils/src/lib.rs +++ b/clippy_utils/src/lib.rs @@ -73,10 +73,10 @@ use rustc_hir::intravisit::{walk_expr, ErasedMap, FnKind, NestedVisitorMap, Visi use rustc_hir::itemlikevisit::ItemLikeVisitor; use rustc_hir::LangItem::{OptionNone, ResultErr, ResultOk}; use rustc_hir::{ - def, Arm, BindingAnnotation, Block, Body, Constness, Destination, Expr, ExprKind, FnDecl, ForeignItem, GenericArgs, - HirId, Impl, ImplItem, ImplItemKind, IsAsync, Item, ItemKind, LangItem, Local, MatchSource, Mutability, Node, - Param, Pat, PatKind, Path, PathSegment, PrimTy, QPath, Stmt, StmtKind, TraitItem, TraitItemKind, TraitRef, TyKind, - UnOp, + def, Arm, BindingAnnotation, Block, BlockCheckMode, Body, Constness, Destination, Expr, ExprKind, FnDecl, + ForeignItem, GenericArgs, HirId, Impl, ImplItem, ImplItemKind, IsAsync, Item, ItemKind, LangItem, Local, + MatchSource, Mutability, Node, Param, Pat, PatKind, Path, PathSegment, PrimTy, QPath, Stmt, StmtKind, TraitItem, + TraitItemKind, TraitRef, TyKind, UnOp, }; use rustc_lint::{LateContext, Level, Lint, LintContext}; use rustc_middle::hir::exports::Export; @@ -1225,20 +1225,65 @@ pub fn get_parent_as_impl(tcx: TyCtxt<'_>, id: HirId) -> Option<&Impl<'_>> { } /// Removes blocks around an expression, only if the block contains just one expression -/// and no statements. +/// and no statements. Unsafe blocks are not removed. /// /// Examples: -/// * `{}` -> `{}` -/// * `{ x }` -> `x` -/// * `{{ x }}` -> `x` -/// * `{ x; }` -> `{ x; }` -/// * `{ x; y }` -> `{ x; y }` -pub fn peel_blocks<'tcx>(mut expr: &'tcx Expr<'tcx>) -> &'tcx Expr<'tcx> { - while let ExprKind::Block(block, ..) = expr.kind { - match (block.stmts.is_empty(), block.expr.as_ref()) { - (true, Some(e)) => expr = e, - _ => break, +/// * `{}` -> `{}` +/// * `{ x }` -> `x` +/// * `{{ x }}` -> `x` +/// * `{ x; }` -> `{ x; }` +/// * `{ x; y }` -> `{ x; y }` +/// * `{ unsafe { x } }` -> `unsafe { x }` +pub fn peel_blocks<'a>(mut expr: &'a Expr<'a>) -> &'a Expr<'a> { + while let ExprKind::Block( + Block { + stmts: [], + expr: Some(inner), + rules: BlockCheckMode::DefaultBlock, + .. + }, + _, + ) = expr.kind + { + expr = inner; + } + expr +} + +/// Removes blocks around an expression, only if the block contains just one expression +/// or just one expression statement with a semicolon. Unsafe blocks are not removed. +/// +/// Examples: +/// * `{}` -> `{}` +/// * `{ x }` -> `x` +/// * `{ x; }` -> `x` +/// * `{{ x; }}` -> `x` +/// * `{ x; y }` -> `{ x; y }` +/// * `{ unsafe { x } }` -> `unsafe { x }` +pub fn peel_blocks_with_stmt<'a>(mut expr: &'a Expr<'a>) -> &'a Expr<'a> { + while let ExprKind::Block( + Block { + stmts: [], + expr: Some(inner), + rules: BlockCheckMode::DefaultBlock, + .. } + | Block { + stmts: + [ + Stmt { + kind: StmtKind::Expr(inner) | StmtKind::Semi(inner), + .. + }, + ], + expr: None, + rules: BlockCheckMode::DefaultBlock, + .. + }, + _, + ) = expr.kind + { + expr = inner; } expr }