Skip to content

Commit

Permalink
Refactors with peel_blocks
Browse files Browse the repository at this point in the history
  • Loading branch information
camsteffen committed Dec 6, 2021
1 parent 284b63a commit f690ef6
Show file tree
Hide file tree
Showing 15 changed files with 151 additions and 252 deletions.
12 changes: 2 additions & 10 deletions clippy_lints/src/assertions_on_constants.rs
Expand Up @@ -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};
Expand Down Expand Up @@ -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
Expand Down
20 changes: 3 additions & 17 deletions 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};
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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 {
Expand Down
12 changes: 4 additions & 8 deletions clippy_lints/src/floating_point_arithmetic.rs
Expand Up @@ -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};
Expand Down Expand Up @@ -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 = (
Expand Down
7 changes: 2 additions & 5 deletions 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};
Expand Down Expand Up @@ -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 {
Expand Down
16 changes: 5 additions & 11 deletions 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};

Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down
112 changes: 48 additions & 64 deletions clippy_lints/src/loops/manual_flatten.rs
Expand Up @@ -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;
Expand All @@ -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",
);
}
);
}
}
}
4 changes: 1 addition & 3 deletions clippy_lints/src/loops/never_loop.rs
Expand Up @@ -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)
}

Expand Down
15 changes: 2 additions & 13 deletions clippy_lints/src/manual_map.rs
Expand Up @@ -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;
Expand Down Expand Up @@ -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))
}
6 changes: 2 additions & 4 deletions 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;
Expand Down Expand Up @@ -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),
Expand Down Expand Up @@ -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,
}
}
Expand Down

0 comments on commit f690ef6

Please sign in to comment.