Refactors with peel_blocks

This commit is contained in:
Cameron Steffen 2021-11-29 08:34:36 -06:00
parent 284b63a687
commit f690ef6f5c
15 changed files with 151 additions and 252 deletions

View file

@ -2,7 +2,7 @@ use clippy_utils::consts::{constant, Constant};
use clippy_utils::diagnostics::span_lint_and_help; use clippy_utils::diagnostics::span_lint_and_help;
use clippy_utils::higher; use clippy_utils::higher;
use clippy_utils::source::snippet_opt; 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 if_chain::if_chain;
use rustc_hir::{Expr, ExprKind, UnOp}; use rustc_hir::{Expr, ExprKind, UnOp};
use rustc_lint::{LateContext, LateLintPass}; 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; if let ExprKind::Unary(UnOp::Not, expr) = cond.kind;
// bind the first argument of the `assert!` macro // bind the first argument of the `assert!` macro
if let Some((Constant::Bool(is_true), _)) = constant(cx, cx.typeck_results(), expr); if let Some((Constant::Bool(is_true), _)) = constant(cx, cx.typeck_results(), expr);
// block let begin_panic_call = peel_blocks(then);
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,
};
// function call // function call
if let Some(arg) = match_panic_call(cx, begin_panic_call); if let Some(arg) = match_panic_call(cx, begin_panic_call);
// bind the second argument of the `assert!` macro if it exists // bind the second argument of the `assert!` macro if it exists

View file

@ -1,10 +1,10 @@
use clippy_utils::diagnostics::span_lint_and_then; use clippy_utils::diagnostics::span_lint_and_then;
use clippy_utils::higher::IfLetOrMatch; use clippy_utils::higher::IfLetOrMatch;
use clippy_utils::visitors::is_local_used; 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 if_chain::if_chain;
use rustc_hir::LangItem::OptionNone; 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_lint::{LateContext, LateLintPass};
use rustc_session::{declare_lint_pass, declare_tool_lint}; use rustc_session::{declare_lint_pass, declare_tool_lint};
use rustc_span::{MultiSpan, Span}; use rustc_span::{MultiSpan, Span};
@ -75,7 +75,7 @@ fn check_arm<'tcx>(
outer_guard: Option<&'tcx Guard<'tcx>>, outer_guard: Option<&'tcx Guard<'tcx>>,
outer_else_body: Option<&'tcx Expr<'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_chain! {
if let Some(inner) = IfLetOrMatch::parse(cx, inner_expr); if let Some(inner) = IfLetOrMatch::parse(cx, inner_expr);
if let Some((inner_scrutinee, inner_then_pat, inner_else_body)) = match inner { 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" /// 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. /// into a single wild arm without any significant loss in semantics or readability.
fn arm_is_wild_like(cx: &LateContext<'_>, arm: &Arm<'_>) -> bool { fn arm_is_wild_like(cx: &LateContext<'_>, arm: &Arm<'_>) -> bool {

View file

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

View file

@ -1,6 +1,6 @@
use clippy_utils::diagnostics::span_lint_and_help; use clippy_utils::diagnostics::span_lint_and_help;
use clippy_utils::source::snippet_with_macro_callsite; 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 if_chain::if_chain;
use rustc_hir::LangItem::{OptionNone, OptionSome}; use rustc_hir::LangItem::{OptionNone, OptionSome};
use rustc_hir::{Expr, ExprKind, Stmt, StmtKind}; 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::Call(then_call, [then_arg]) = then_expr.kind;
if let ExprKind::Path(ref then_call_qpath) = then_call.kind; if let ExprKind::Path(ref then_call_qpath) = then_call.kind;
if is_lang_ctor(cx, then_call_qpath, OptionSome); if is_lang_ctor(cx, then_call_qpath, OptionSome);
if let ExprKind::Block(els_block, _) = els.kind; if let ExprKind::Path(ref qpath) = peel_blocks(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 is_lang_ctor(cx, qpath, OptionNone); if is_lang_ctor(cx, qpath, OptionNone);
if !stmts_contains_early_return(then_block.stmts); if !stmts_contains_early_return(then_block.stmts);
then { then {

View file

@ -1,10 +1,9 @@
use clippy_utils::diagnostics::span_lint_and_sugg; use clippy_utils::diagnostics::span_lint_and_sugg;
use clippy_utils::higher; use clippy_utils::{higher, peel_blocks_with_stmt, SpanlessEq};
use clippy_utils::SpanlessEq;
use if_chain::if_chain; use if_chain::if_chain;
use rustc_ast::ast::LitKind; use rustc_ast::ast::LitKind;
use rustc_errors::Applicability; 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_lint::{LateContext, LateLintPass};
use rustc_session::{declare_lint_pass, declare_tool_lint}; 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 < // Ensure that the binary operator is >, != and <
if BinOpKind::Ne == cond_op.node || BinOpKind::Gt == cond_op.node || BinOpKind::Lt == cond_op.node; 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 // Check if assign operation is done
if let StmtKind::Semi(e) = block.stmts[0].kind; if let Some(target) = subtracts_one(cx, then);
if let Some(target) = subtracts_one(cx, e);
// Extracting out the variable name // Extracting out the variable name
if let ExprKind::Path(QPath::Resolved(_, ares_path)) = target.kind; 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>> { fn subtracts_one<'a>(cx: &LateContext<'_>, expr: &'a Expr<'a>) -> Option<&'a Expr<'a>> {
match expr.kind { match peel_blocks_with_stmt(expr).kind {
ExprKind::AssignOp(ref op1, target, value) => { ExprKind::AssignOp(ref op1, target, value) => {
if_chain! { if_chain! {
if BinOpKind::Sub == op1.node; if BinOpKind::Sub == op1.node;

View file

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

View file

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

View file

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

View file

@ -1,5 +1,6 @@
use clippy_utils::diagnostics::{span_lint_hir, span_lint_hir_and_then}; use clippy_utils::diagnostics::{span_lint_hir, span_lint_hir_and_then};
use clippy_utils::is_lint_allowed; use clippy_utils::is_lint_allowed;
use clippy_utils::peel_blocks;
use clippy_utils::source::snippet_opt; use clippy_utils::source::snippet_opt;
use clippy_utils::ty::has_drop; use clippy_utils::ty::has_drop;
use rustc_errors::Applicability; use rustc_errors::Applicability;
@ -114,7 +115,7 @@ fn has_no_effect(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool {
if expr.span.from_expansion() { if expr.span.from_expansion() {
return false; return false;
} }
match expr.kind { match peel_blocks(expr).kind {
ExprKind::Lit(..) | ExprKind::Closure(..) => true, ExprKind::Lit(..) | ExprKind::Closure(..) => true,
ExprKind::Path(..) => !has_drop(cx, cx.typeck_results().expr_ty(expr)), 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), 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 false
} }
}, },
ExprKind::Block(block, _) => {
block.stmts.is_empty() && block.expr.as_ref().map_or(false, |expr| has_no_effect(cx, expr))
},
_ => false, _ => false,
} }
} }

View file

@ -1,15 +1,14 @@
use clippy_utils::diagnostics::span_lint_and_sugg; use clippy_utils::diagnostics::span_lint_and_sugg;
use clippy_utils::higher;
use clippy_utils::sugg::Sugg; use clippy_utils::sugg::Sugg;
use clippy_utils::ty::is_type_diagnostic_item; use clippy_utils::ty::is_type_diagnostic_item;
use clippy_utils::{ use clippy_utils::{
can_move_expr_to_closure, eager_or_lazy, in_constant, is_else_clause, is_lang_ctor, peel_hir_expr_while, can_move_expr_to_closure, eager_or_lazy, higher, in_constant, is_else_clause, is_lang_ctor, peel_blocks,
CaptureKind, peel_hir_expr_while, CaptureKind,
}; };
use if_chain::if_chain; use if_chain::if_chain;
use rustc_errors::Applicability; use rustc_errors::Applicability;
use rustc_hir::LangItem::OptionSome; 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_lint::{LateContext, LateLintPass};
use rustc_session::{declare_lint_pass, declare_tool_lint}; use rustc_session::{declare_lint_pass, declare_tool_lint};
use rustc_span::sym; use rustc_span::sym;
@ -86,28 +85,6 @@ struct OptionIfLetElseOccurence {
none_expr: String, 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 { fn format_option_in_sugg(cx: &LateContext<'_>, cond_expr: &Expr<'_>, as_ref: bool, as_mut: bool) -> String {
format!( format!(
"{}{}", "{}{}",
@ -145,8 +122,8 @@ fn detect_option_if_let_else<'tcx>(cx: &LateContext<'tcx>, expr: &Expr<'tcx>) ->
then { then {
let capture_mut = if bind_annotation == &BindingAnnotation::Mutable { "mut " } else { "" }; let capture_mut = if bind_annotation == &BindingAnnotation::Mutable { "mut " } else { "" };
let some_body = extract_body_from_expr(if_then)?; let some_body = peel_blocks(if_then);
let none_body = extract_body_from_expr(if_else)?; 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 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 capture_name = id.name.to_ident_string();
let (as_ref, as_mut) = match &let_expr.kind { let (as_ref, as_mut) = match &let_expr.kind {

View file

@ -1,14 +1,13 @@
use clippy_utils::diagnostics::span_lint_and_sugg; use clippy_utils::diagnostics::span_lint_and_sugg;
use clippy_utils::higher; use clippy_utils::higher;
use clippy_utils::is_lang_ctor;
use clippy_utils::source::snippet_with_applicability; use clippy_utils::source::snippet_with_applicability;
use clippy_utils::sugg::Sugg; use clippy_utils::sugg::Sugg;
use clippy_utils::ty::is_type_diagnostic_item; 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 if_chain::if_chain;
use rustc_errors::Applicability; use rustc_errors::Applicability;
use rustc_hir::LangItem::{OptionNone, OptionSome, ResultOk}; 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_lint::{LateContext, LateLintPass};
use rustc_session::{declare_lint_pass, declare_tool_lint}; use rustc_session::{declare_lint_pass, declare_tool_lint};
use rustc_span::sym; use rustc_span::sym;
@ -68,14 +67,8 @@ impl QuestionMark {
let receiver_str = &Sugg::hir_with_applicability(cx, subject, "..", &mut applicability); let receiver_str = &Sugg::hir_with_applicability(cx, subject, "..", &mut applicability);
let mut replacement: Option<String> = None; let mut replacement: Option<String> = None;
if let Some(else_inner) = r#else { if let Some(else_inner) = r#else {
if_chain! { if eq_expr_value(cx, subject, peel_blocks(else_inner)) {
if let ExprKind::Block(block, None) = &else_inner.kind; replacement = Some(format!("Some({}?)", receiver_str));
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));
}
} }
} else if Self::moves_by_default(cx, subject) } else if Self::moves_by_default(cx, subject)
&& !matches!(subject.kind, ExprKind::Call(..) | ExprKind::MethodCall(..)) && !matches!(subject.kind, ExprKind::Call(..) | ExprKind::MethodCall(..))
@ -110,10 +103,7 @@ impl QuestionMark {
if let PatKind::Binding(annot, bind_id, _, _) = fields[0].kind; if let PatKind::Binding(annot, bind_id, _, _) = fields[0].kind;
let by_ref = matches!(annot, BindingAnnotation::Ref | BindingAnnotation::RefMut); let by_ref = matches!(annot, BindingAnnotation::Ref | BindingAnnotation::RefMut);
if let ExprKind::Block(block, None) = if_then.kind; if path_to_local_id(peel_blocks(if_then), bind_id);
if block.stmts.is_empty();
if let Some(trailing_expr) = &block.expr;
if path_to_local_id(trailing_expr, bind_id);
then { then {
let mut applicability = Applicability::MachineApplicable; let mut applicability = Applicability::MachineApplicable;
let receiver_str = snippet_with_applicability(cx, let_expr.span, "..", &mut applicability); 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 { fn expression_returns_none(cx: &LateContext<'_>, expression: &Expr<'_>) -> bool {
match expression.kind { match peel_blocks_with_stmt(expression).kind {
ExprKind::Block(block, _) => {
if let Some(return_expression) = Self::return_expression(block) {
return Self::expression_returns_none(cx, return_expression);
}
false
},
ExprKind::Ret(Some(expr)) => Self::expression_returns_none(cx, expr), ExprKind::Ret(Some(expr)) => Self::expression_returns_none(cx, expr),
ExprKind::Path(ref qpath) => is_lang_ctor(cx, qpath, OptionNone), ExprKind::Path(ref qpath) => is_lang_ctor(cx, qpath, OptionNone),
_ => false, _ => false,
@ -174,44 +157,12 @@ impl QuestionMark {
} }
fn expression_returns_unmodified_err(cx: &LateContext<'_>, expr: &Expr<'_>, cond_expr: &Expr<'_>) -> bool { fn expression_returns_unmodified_err(cx: &LateContext<'_>, expr: &Expr<'_>, cond_expr: &Expr<'_>) -> bool {
match expr.kind { match peel_blocks_with_stmt(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
},
ExprKind::Ret(Some(ret_expr)) => Self::expression_returns_unmodified_err(cx, ret_expr, cond_expr), 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), ExprKind::Path(_) => path_to_local(expr) == path_to_local(cond_expr),
_ => false, _ => 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 { impl<'tcx> LateLintPass<'tcx> for QuestionMark {

View file

@ -1,8 +1,8 @@
use clippy_utils::diagnostics::{span_lint, span_lint_and_help, span_lint_and_sugg}; 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::source::{snippet, snippet_with_applicability};
use clippy_utils::ty::is_type_diagnostic_item; 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::{get_parent_expr, is_lint_allowed, match_function_call, method_calls, paths};
use clippy_utils::{peel_blocks, SpanlessEq};
use if_chain::if_chain; use if_chain::if_chain;
use rustc_errors::Applicability; use rustc_errors::Applicability;
use rustc_hir::{BinOpKind, BorrowKind, Expr, ExprKind, LangItem, QPath}; 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 { fn is_add(cx: &LateContext<'_>, src: &Expr<'_>, target: &Expr<'_>) -> bool {
match src.kind { match peel_blocks(src).kind {
ExprKind::Binary( ExprKind::Binary(
Spanned { Spanned {
node: BinOpKind::Add, .. node: BinOpKind::Add, ..
@ -209,9 +209,6 @@ fn is_add(cx: &LateContext<'_>, src: &Expr<'_>, target: &Expr<'_>) -> bool {
left, left,
_, _,
) => SpanlessEq::new(cx).eq_expr(target, 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, _ => false,
} }
} }

View file

@ -1,11 +1,10 @@
use clippy_utils::consts::{constant_simple, Constant}; 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::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::source::snippet;
use clippy_utils::ty::match_type; use clippy_utils::ty::match_type;
use clippy_utils::{ 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, higher, is_else_clause, is_expn_of, is_expr_path_def_path, is_lint_allowed, match_def_path, method_calls,
paths, SpanlessEq, path_to_res, paths, peel_blocks_with_stmt, SpanlessEq,
}; };
use if_chain::if_chain; use if_chain::if_chain;
use rustc_ast as ast; use rustc_ast as ast;
@ -662,10 +661,7 @@ impl<'tcx> LateLintPass<'tcx> for CollapsibleCalls {
if and_then_args.len() == 5; if and_then_args.len() == 5;
if let ExprKind::Closure(_, _, body_id, _, _) = &and_then_args[4].kind; if let ExprKind::Closure(_, _, body_id, _, _) = &and_then_args[4].kind;
let body = cx.tcx.hir().body(*body_id); let body = cx.tcx.hir().body(*body_id);
if let ExprKind::Block(block, _) = &body.value.kind; let only_expr = peel_blocks_with_stmt(&body.value);
let stmts = &block.stmts;
if stmts.len() == 1 && block.expr.is_none();
if let StmtKind::Semi(only_expr) = &stmts[0].kind;
if let ExprKind::MethodCall(ps, _, span_call_args, _) = &only_expr.kind; if let ExprKind::MethodCall(ps, _, span_call_args, _) = &only_expr.kind;
then { then {
let and_then_snippets = get_and_then_snippets(cx, and_then_args); let and_then_snippets = get_and_then_snippets(cx, and_then_args);

View file

@ -8,6 +8,11 @@
//! during any comparison or mapping. (Please take care of this, it's not fun to spend time on such //! during any comparison or mapping. (Please take care of this, it's not fun to spend time on such
//! a simple mistake) //! 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 if_chain::if_chain;
use rustc_ast as ast; use rustc_ast as ast;
use rustc_data_structures::fx::FxHashMap; use rustc_data_structures::fx::FxHashMap;
@ -25,12 +30,6 @@ use std::fs::{self, OpenOptions};
use std::io::prelude::*; use std::io::prelude::*;
use std::path::Path; 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. /// This is the output file of the lint collector.
const OUTPUT_FILE: &str = "../util/gh-pages/lints.json"; const OUTPUT_FILE: &str = "../util/gh-pages/lints.json";
/// These lints are excluded from the export. /// These lints are excluded from the export.

View file

@ -73,10 +73,10 @@ use rustc_hir::intravisit::{walk_expr, ErasedMap, FnKind, NestedVisitorMap, Visi
use rustc_hir::itemlikevisit::ItemLikeVisitor; use rustc_hir::itemlikevisit::ItemLikeVisitor;
use rustc_hir::LangItem::{OptionNone, ResultErr, ResultOk}; use rustc_hir::LangItem::{OptionNone, ResultErr, ResultOk};
use rustc_hir::{ use rustc_hir::{
def, Arm, BindingAnnotation, Block, Body, Constness, Destination, Expr, ExprKind, FnDecl, ForeignItem, GenericArgs, def, Arm, BindingAnnotation, Block, BlockCheckMode, Body, Constness, Destination, Expr, ExprKind, FnDecl,
HirId, Impl, ImplItem, ImplItemKind, IsAsync, Item, ItemKind, LangItem, Local, MatchSource, Mutability, Node, ForeignItem, GenericArgs, HirId, Impl, ImplItem, ImplItemKind, IsAsync, Item, ItemKind, LangItem, Local,
Param, Pat, PatKind, Path, PathSegment, PrimTy, QPath, Stmt, StmtKind, TraitItem, TraitItemKind, TraitRef, TyKind, MatchSource, Mutability, Node, Param, Pat, PatKind, Path, PathSegment, PrimTy, QPath, Stmt, StmtKind, TraitItem,
UnOp, TraitItemKind, TraitRef, TyKind, UnOp,
}; };
use rustc_lint::{LateContext, Level, Lint, LintContext}; use rustc_lint::{LateContext, Level, Lint, LintContext};
use rustc_middle::hir::exports::Export; 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 /// 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: /// Examples:
/// * `{}` -> `{}` /// * `{}` -> `{}`
/// * `{ x }` -> `x` /// * `{ x }` -> `x`
/// * `{{ x }}` -> `x` /// * `{{ x }}` -> `x`
/// * `{ x; }` -> `{ x; }` /// * `{ x; }` -> `{ x; }`
/// * `{ x; y }` -> `{ x; y }` /// * `{ x; y }` -> `{ x; y }`
pub fn peel_blocks<'tcx>(mut expr: &'tcx Expr<'tcx>) -> &'tcx Expr<'tcx> { /// * `{ unsafe { x } }` -> `unsafe { x }`
while let ExprKind::Block(block, ..) = expr.kind { pub fn peel_blocks<'a>(mut expr: &'a Expr<'a>) -> &'a Expr<'a> {
match (block.stmts.is_empty(), block.expr.as_ref()) { while let ExprKind::Block(
(true, Some(e)) => expr = e, Block {
_ => break, 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 expr
} }