Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion clippy_lints/src/format_push_string.rs
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ fn is_format(cx: &LateContext<'_>, e: &Expr<'_>) -> bool {
Some(higher::IfLetOrMatch::Match(_, arms, MatchSource::Normal)) => {
arms.iter().any(|arm| is_format(cx, arm.body))
},
Some(higher::IfLetOrMatch::IfLet(_, _, then, r#else)) => {
Some(higher::IfLetOrMatch::IfLet(_, _, then, r#else, _)) => {
is_format(cx, then) || r#else.is_some_and(|e| is_format(cx, e))
},
_ => false,
Expand Down
2 changes: 1 addition & 1 deletion clippy_lints/src/loops/manual_flatten.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ pub(super) fn check<'tcx>(
span: Span,
) {
let inner_expr = peel_blocks_with_stmt(body);
if let Some(higher::IfLet { let_pat, let_expr, if_then, if_else: None })
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
&& let PatKind::Binding(_, pat_hir_id, _, _) = pat.kind
Expand Down
2 changes: 1 addition & 1 deletion clippy_lints/src/loops/while_let_on_iterator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ use rustc_span::symbol::sym;
use rustc_span::Symbol;

pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
if let Some(higher::WhileLet { if_then, let_pat, let_expr }) = higher::WhileLet::hir(expr)
if let Some(higher::WhileLet { if_then, let_pat, let_expr, .. }) = higher::WhileLet::hir(expr)
// check for `Some(..)` pattern
&& let PatKind::TupleStruct(ref pat_path, some_pat, _) = let_pat.kind
&& is_res_lang_ctor(cx, cx.qpath_res(pat_path, let_pat.hir_id), LangItem::OptionSome)
Expand Down
2 changes: 1 addition & 1 deletion clippy_lints/src/manual_let_else.rs
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ impl<'tcx> QuestionMark {
&& let Some(if_let_or_match) = IfLetOrMatch::parse(cx, init)
{
match if_let_or_match {
IfLetOrMatch::IfLet(if_let_expr, let_pat, if_then, if_else) => {
IfLetOrMatch::IfLet(if_let_expr, let_pat, if_then, if_else, ..) => {
if let Some(ident_map) = expr_simple_identity_map(local.pat, let_pat, if_then)
&& let Some(if_else) = if_else
&& is_never_expr(cx, if_else).is_some()
Expand Down
4 changes: 2 additions & 2 deletions clippy_lints/src/matches/collapsible_match.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ fn check_arm<'tcx>(
let inner_expr = peel_blocks_with_stmt(outer_then_body);
if let Some(inner) = IfLetOrMatch::parse(cx, inner_expr)
&& let Some((inner_scrutinee, inner_then_pat, inner_else_body)) = match inner {
IfLetOrMatch::IfLet(scrutinee, pat, _, els) => Some((scrutinee, pat, els)),
IfLetOrMatch::IfLet(scrutinee, pat, _, els, _) => Some((scrutinee, pat, els)),
IfLetOrMatch::Match(scrutinee, arms, ..) => if arms.len() == 2 && arms.iter().all(|a| a.guard.is_none())
// if there are more than two arms, collapsing would be non-trivial
// one of the arms must be "wild-like"
Expand Down Expand Up @@ -75,7 +75,7 @@ fn check_arm<'tcx>(
)
// ...or anywhere in the inner expression
&& match inner {
IfLetOrMatch::IfLet(_, _, body, els) => {
IfLetOrMatch::IfLet(_, _, body, els, _) => {
!is_local_used(cx, body, binding_id) && els.map_or(true, |e| !is_local_used(cx, e, binding_id))
},
IfLetOrMatch::Match(_, arms, ..) => !arms.iter().any(|arm| is_local_used(cx, arm, binding_id)),
Expand Down
22 changes: 17 additions & 5 deletions clippy_lints/src/matches/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -469,15 +469,15 @@ declare_clippy_lint! {
declare_clippy_lint! {
/// ### What it does
/// Lint for redundant pattern matching over `Result`, `Option`,
/// `std::task::Poll` or `std::net::IpAddr`
/// `std::task::Poll`, `std::net::IpAddr` or `bool`s
///
/// ### Why is this bad?
/// It's more concise and clear to just use the proper
/// utility function
/// utility function or using the condition directly
///
/// ### Known problems
/// This will change the drop order for the matched type. Both `if let` and
/// `while let` will drop the value at the end of the block, both `if` and `while` will drop the
/// For suggestions involving bindings in patterns, this will change the drop order for the matched type.
/// Both `if let` and `while let` will drop the value at the end of the block, both `if` and `while` will drop the
/// value before entering the block. For most types this change will not matter, but for a few
/// types this will not be an acceptable change (e.g. locks). See the
/// [reference](https://doc.rust-lang.org/reference/destructors.html#drop-scopes) for more about
Expand All @@ -499,6 +499,10 @@ declare_clippy_lint! {
/// Ok(_) => true,
/// Err(_) => false,
/// };
///
/// let cond = true;
/// if let true = cond {}
/// matches!(cond, true);
/// ```
///
/// The more idiomatic use would be:
Expand All @@ -515,6 +519,10 @@ declare_clippy_lint! {
/// if IpAddr::V4(Ipv4Addr::LOCALHOST).is_ipv4() {}
/// if IpAddr::V6(Ipv6Addr::LOCALHOST).is_ipv6() {}
/// Ok::<i32, i32>(42).is_ok();
///
/// let cond = true;
/// if cond {}
/// cond;
/// ```
#[clippy::version = "1.31.0"]
pub REDUNDANT_PATTERN_MATCHING,
Expand Down Expand Up @@ -1019,8 +1027,11 @@ impl<'tcx> LateLintPass<'tcx> for Matches {
let from_expansion = expr.span.from_expansion();

if let ExprKind::Match(ex, arms, source) = expr.kind {
if is_direct_expn_of(expr.span, "matches").is_some() {
if is_direct_expn_of(expr.span, "matches").is_some()
&& let [arm, _] = arms
{
redundant_pattern_match::check_match(cx, expr, ex, arms);
redundant_pattern_match::check_matches_true(cx, expr, arm, ex);
}

if source == MatchSource::Normal && !is_span_match(cx, expr.span) {
Expand Down Expand Up @@ -1104,6 +1115,7 @@ impl<'tcx> LateLintPass<'tcx> for Matches {
if_let.let_pat,
if_let.let_expr,
if_let.if_else.is_some(),
if_let.let_span,
);
needless_match::check_if_let(cx, expr, &if_let);
}
Expand Down
84 changes: 78 additions & 6 deletions clippy_lints/src/matches/redundant_pattern_match.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use super::REDUNDANT_PATTERN_MATCHING;
use clippy_utils::diagnostics::{span_lint_and_sugg, span_lint_and_then};
use clippy_utils::source::{snippet, walk_span_to_context};
use clippy_utils::sugg::Sugg;
use clippy_utils::sugg::{make_unop, Sugg};
use clippy_utils::ty::{is_type_diagnostic_item, needs_ordered_drop};
use clippy_utils::visitors::{any_temporaries_need_ordered_drop, for_each_expr};
use clippy_utils::{higher, is_expn_of, is_trait_method};
Expand All @@ -12,13 +12,20 @@ use rustc_hir::LangItem::{self, OptionNone, OptionSome, PollPending, PollReady,
use rustc_hir::{Arm, Expr, ExprKind, Guard, Node, Pat, PatKind, QPath, UnOp};
use rustc_lint::LateContext;
use rustc_middle::ty::{self, GenericArgKind, Ty};
use rustc_span::{sym, Symbol};
use rustc_span::{sym, Span, Symbol};
use std::fmt::Write;
use std::ops::ControlFlow;

pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
if let Some(higher::WhileLet { let_pat, let_expr, .. }) = higher::WhileLet::hir(expr) {
find_sugg_for_if_let(cx, expr, let_pat, let_expr, "while", false);
if let Some(higher::WhileLet {
let_pat,
let_expr,
let_span,
..
}) = higher::WhileLet::hir(expr)
{
find_method_sugg_for_if_let(cx, expr, let_pat, let_expr, "while", false);
find_if_let_true(cx, let_pat, let_expr, let_span);
}
}

Expand All @@ -28,8 +35,73 @@ pub(super) fn check_if_let<'tcx>(
pat: &'tcx Pat<'_>,
scrutinee: &'tcx Expr<'_>,
has_else: bool,
let_span: Span,
) {
find_sugg_for_if_let(cx, expr, pat, scrutinee, "if", has_else);
find_if_let_true(cx, pat, scrutinee, let_span);
find_method_sugg_for_if_let(cx, expr, pat, scrutinee, "if", has_else);
}

/// Looks for:
/// * `matches!(expr, true)`
pub fn check_matches_true<'tcx>(
cx: &LateContext<'tcx>,
expr: &'tcx Expr<'_>,
arm: &'tcx Arm<'_>,
scrutinee: &'tcx Expr<'_>,
) {
find_match_true(
cx,
arm.pat,
scrutinee,
expr.span.source_callsite(),
"using `matches!` to pattern match a bool",
);
}

/// Looks for any of:
/// * `if let true = ...`
/// * `if let false = ...`
/// * `while let true = ...`
fn find_if_let_true<'tcx>(cx: &LateContext<'tcx>, pat: &'tcx Pat<'_>, scrutinee: &'tcx Expr<'_>, let_span: Span) {
find_match_true(cx, pat, scrutinee, let_span, "using `if let` to pattern match a bool");
}

/// Common logic between `find_if_let_true` and `check_matches_true`
fn find_match_true<'tcx>(
cx: &LateContext<'tcx>,
pat: &'tcx Pat<'_>,
scrutinee: &'tcx Expr<'_>,
span: Span,
message: &str,
) {
if let PatKind::Lit(lit) = pat.kind
&& let ExprKind::Lit(lit) = lit.kind
&& let LitKind::Bool(pat_is_true) = lit.node
{
let mut applicability = Applicability::MachineApplicable;

let mut sugg = Sugg::hir_with_context(
cx,
scrutinee,
scrutinee.span.source_callsite().ctxt(),
"..",
&mut applicability,
);

if !pat_is_true {
sugg = make_unop("!", sugg);
}

span_lint_and_sugg(
cx,
REDUNDANT_PATTERN_MATCHING,
span,
message,
"consider using the condition directly",
sugg.to_string(),
applicability,
);
}
}

// Extract the generic arguments out of a type
Expand Down Expand Up @@ -100,7 +172,7 @@ fn find_method_and_type<'tcx>(
}
}

fn find_sugg_for_if_let<'tcx>(
fn find_method_sugg_for_if_let<'tcx>(
cx: &LateContext<'tcx>,
expr: &'tcx Expr<'_>,
let_pat: &Pat<'_>,
Expand Down
2 changes: 1 addition & 1 deletion clippy_lints/src/methods/filter_map.rs
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,7 @@ impl<'tcx> OffendingFilterExpr<'tcx> {
match higher::IfLetOrMatch::parse(cx, map_body.value) {
// For `if let` we want to check that the variant matching arm references the local created by
// its pattern
Some(higher::IfLetOrMatch::IfLet(sc, pat, then, Some(else_)))
Some(higher::IfLetOrMatch::IfLet(sc, pat, then, Some(else_), ..))
if let Some((ident, span)) = expr_uses_local(pat, then) =>
{
(sc, else_, ident, span)
Expand Down
1 change: 1 addition & 0 deletions clippy_lints/src/option_if_let_else.rs
Original file line number Diff line number Diff line change
Expand Up @@ -238,6 +238,7 @@ fn detect_option_if_let_else<'tcx>(cx: &LateContext<'tcx>, expr: &Expr<'tcx>) ->
let_expr,
if_then,
if_else: Some(if_else),
..
}) = higher::IfLet::hir(cx, expr)
&& !cx.typeck_results().expr_ty(expr).is_unit()
&& !is_else_clause(cx.tcx, expr)
Expand Down
1 change: 1 addition & 0 deletions clippy_lints/src/question_mark.rs
Original file line number Diff line number Diff line change
Expand Up @@ -256,6 +256,7 @@ impl QuestionMark {
let_expr,
if_then,
if_else,
..
}) = higher::IfLet::hir(cx, expr)
&& !is_else_clause(cx.tcx, expr)
&& let PatKind::TupleStruct(ref path1, [field], ddpos) = let_pat.kind
Expand Down
1 change: 1 addition & 0 deletions clippy_lints/src/utils/author.rs
Original file line number Diff line number Diff line change
Expand Up @@ -350,6 +350,7 @@ impl<'a, 'tcx> PrintVisitor<'a, 'tcx> {
let_pat,
let_expr,
if_then,
..
}) = higher::WhileLet::hir(expr.value)
{
bind!(self, let_pat, let_expr, if_then);
Expand Down
20 changes: 17 additions & 3 deletions clippy_utils/src/higher.rs
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,9 @@ pub struct IfLet<'hir> {
pub if_then: &'hir Expr<'hir>,
/// `if let` else expression
pub if_else: Option<&'hir Expr<'hir>>,
/// `if let PAT = EXPR`
/// ^^^^^^^^^^^^^^
pub let_span: Span,
}

impl<'hir> IfLet<'hir> {
Expand All @@ -99,9 +102,10 @@ impl<'hir> IfLet<'hir> {
if let ExprKind::If(
Expr {
kind:
ExprKind::Let(hir::Let {
ExprKind::Let(&hir::Let {
pat: let_pat,
init: let_expr,
span: let_span,
..
}),
..
Expand Down Expand Up @@ -129,6 +133,7 @@ impl<'hir> IfLet<'hir> {
let_expr,
if_then,
if_else,
let_span,
});
}
None
Expand All @@ -146,6 +151,9 @@ pub enum IfLetOrMatch<'hir> {
&'hir Pat<'hir>,
&'hir Expr<'hir>,
Option<&'hir Expr<'hir>>,
/// `if let PAT = EXPR`
/// ^^^^^^^^^^^^^^
Span,
),
}

Expand All @@ -160,7 +168,8 @@ impl<'hir> IfLetOrMatch<'hir> {
let_pat,
if_then,
if_else,
}| { Self::IfLet(let_expr, let_pat, if_then, if_else) },
let_span,
}| { Self::IfLet(let_expr, let_pat, if_then, if_else, let_span) },
),
}
}
Expand Down Expand Up @@ -353,6 +362,9 @@ pub struct WhileLet<'hir> {
pub let_expr: &'hir Expr<'hir>,
/// `while let` loop body
pub if_then: &'hir Expr<'hir>,
/// `while let PAT = EXPR`
/// ^^^^^^^^^^^^^^
pub let_span: Span,
}

impl<'hir> WhileLet<'hir> {
Expand All @@ -367,9 +379,10 @@ impl<'hir> WhileLet<'hir> {
ExprKind::If(
Expr {
kind:
ExprKind::Let(hir::Let {
ExprKind::Let(&hir::Let {
pat: let_pat,
init: let_expr,
span: let_span,
..
}),
..
Expand All @@ -390,6 +403,7 @@ impl<'hir> WhileLet<'hir> {
let_pat,
let_expr,
if_then,
let_span,
});
}
None
Expand Down
6 changes: 5 additions & 1 deletion tests/ui/author/loop.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
#![feature(stmt_expr_attributes)]
#![allow(clippy::never_loop, clippy::while_immutable_condition)]
#![allow(
clippy::never_loop,
clippy::while_immutable_condition,
clippy::redundant_pattern_matching
)]

fn main() {
#[clippy::author]
Expand Down
8 changes: 6 additions & 2 deletions tests/ui/branches_sharing_code/shared_at_bottom.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
#![deny(clippy::if_same_then_else, clippy::branches_sharing_code)]
#![allow(dead_code)]
#![allow(clippy::equatable_if_let, clippy::uninlined_format_args)]
#![allow(
clippy::equatable_if_let,
clippy::uninlined_format_args,
clippy::redundant_pattern_matching,
dead_code
)]
//@no-rustfix
// This tests the branches_sharing_code lint at the end of blocks

Expand Down
Loading