X-Git-Url: https://git.lizzy.rs/?a=blobdiff_plain;f=clippy_lints%2Fsrc%2Fmatches.rs;h=5fa8f249e701e91d9b3cadf574f27ab873e0154b;hb=dda2aef64fb5b4903a28e5d4fb8d63483642cc6f;hp=6d5ce3373f79d523060a219922875acf3a3cec7c;hpb=81904a413eb1d90d64e2450f075e38d7e0cf1c00;p=rust.git diff --git a/clippy_lints/src/matches.rs b/clippy_lints/src/matches.rs index 6d5ce3373f7..5fa8f249e70 100644 --- a/clippy_lints/src/matches.rs +++ b/clippy_lints/src/matches.rs @@ -1,19 +1,21 @@ -use clippy_utils::consts::{constant, miri_to_const, Constant}; +use clippy_utils::consts::{constant, constant_full_int, miri_to_const, FullInt}; use clippy_utils::diagnostics::{ multispan_sugg, span_lint_and_help, span_lint_and_note, span_lint_and_sugg, span_lint_and_then, }; +use clippy_utils::higher; use clippy_utils::source::{expr_block, indent_of, snippet, snippet_block, snippet_opt, snippet_with_applicability}; use clippy_utils::sugg::Sugg; use clippy_utils::ty::{implements_trait, is_type_diagnostic_item, match_type, peel_mid_ty_refs}; -use clippy_utils::visitors::LocalUsedVisitor; +use clippy_utils::visitors::is_local_used; use clippy_utils::{ - get_parent_expr, in_macro, is_expn_of, is_lang_ctor, is_lint_allowed, is_refutable, is_wild, meets_msrv, msrvs, - path_to_local, path_to_local_id, peel_hir_pat_refs, peel_n_hir_expr_refs, recurse_or_patterns, remove_blocks, + get_parent_expr, is_expn_of, is_lang_ctor, is_lint_allowed, is_refutable, is_unit_expr, is_wild, meets_msrv, msrvs, + path_to_local, path_to_local_id, peel_blocks, peel_hir_pat_refs, peel_n_hir_expr_refs, recurse_or_patterns, strip_pat_refs, }; use clippy_utils::{paths, search_same, SpanlessEq, SpanlessHash}; +use core::iter::{once, ExactSizeIterator}; use if_chain::if_chain; -use rustc_ast::ast::LitKind; +use rustc_ast::ast::{Attribute, LitKind}; use rustc_errors::Applicability; use rustc_hir::def::{CtorKind, DefKind, Res}; use rustc_hir::LangItem::{OptionNone, OptionSome}; @@ -23,7 +25,6 @@ }; use rustc_hir::{HirIdMap, HirIdSet}; use rustc_lint::{LateContext, LateLintPass, LintContext}; -use rustc_middle::lint::in_external_macro; use rustc_middle::ty::{self, Ty, TyS, VariantDef}; use rustc_semver::RustcVersion; use rustc_session::{declare_tool_lint, impl_lint_pass}; @@ -31,18 +32,16 @@ use rustc_span::sym; use std::cmp::Ordering; use std::collections::hash_map::Entry; -use std::iter; -use std::ops::Bound; declare_clippy_lint! { - /// **What it does:** Checks for matches with a single arm where an `if let` + /// ### What it does + /// Checks for matches with a single arm where an `if let` /// will usually suffice. /// - /// **Why is this bad?** Just readability – `if let` nests less than a `match`. + /// ### Why is this bad? + /// Just readability – `if let` nests less than a `match`. /// - /// **Known problems:** None. - /// - /// **Example:** + /// ### Example /// ```rust /// # fn bar(stool: &str) {} /// # let x = Some("abc"); @@ -57,21 +56,24 @@ /// bar(foo); /// } /// ``` + #[clippy::version = "pre 1.29.0"] pub SINGLE_MATCH, style, "a `match` statement with a single nontrivial arm (i.e., where the other arm is `_ => {}`) instead of `if let`" } declare_clippy_lint! { - /// **What it does:** Checks for matches with two arms where an `if let else` will + /// ### What it does + /// Checks for matches with two arms where an `if let else` will /// usually suffice. /// - /// **Why is this bad?** Just readability – `if let` nests less than a `match`. - /// - /// **Known problems:** Personal style preferences may differ. + /// ### Why is this bad? + /// Just readability – `if let` nests less than a `match`. /// - /// **Example:** + /// ### Known problems + /// Personal style preferences may differ. /// + /// ### Example /// Using `match`: /// /// ```rust @@ -96,22 +98,23 @@ /// bar(&other_ref); /// } /// ``` + #[clippy::version = "pre 1.29.0"] pub SINGLE_MATCH_ELSE, pedantic, "a `match` statement with two arms where the second arm's pattern is a placeholder instead of a specific match pattern" } declare_clippy_lint! { - /// **What it does:** Checks for matches where all arms match a reference, + /// ### What it does + /// Checks for matches where all arms match a reference, /// suggesting to remove the reference and deref the matched expression /// instead. It also checks for `if let &foo = bar` blocks. /// - /// **Why is this bad?** It just makes the code less readable. That reference + /// ### Why is this bad? + /// It just makes the code less readable. That reference /// destructuring adds nothing to the code. /// - /// **Known problems:** None. - /// - /// **Example:** + /// ### Example /// ```rust,ignore /// // Bad /// match x { @@ -127,20 +130,21 @@ /// _ => frob(x), /// } /// ``` + #[clippy::version = "pre 1.29.0"] pub MATCH_REF_PATS, style, "a `match` or `if let` with all arms prefixed with `&` instead of deref-ing the match expression" } declare_clippy_lint! { - /// **What it does:** Checks for matches where match expression is a `bool`. It + /// ### What it does + /// Checks for matches where match expression is a `bool`. It /// suggests to replace the expression with an `if...else` block. /// - /// **Why is this bad?** It makes the code less readable. - /// - /// **Known problems:** None. + /// ### Why is this bad? + /// It makes the code less readable. /// - /// **Example:** + /// ### Example /// ```rust /// # fn foo() {} /// # fn bar() {} @@ -161,43 +165,45 @@ /// bar(); /// } /// ``` + #[clippy::version = "pre 1.29.0"] pub MATCH_BOOL, pedantic, "a `match` on a boolean expression instead of an `if..else` block" } declare_clippy_lint! { - /// **What it does:** Checks for overlapping match arms. + /// ### What it does + /// Checks for overlapping match arms. /// - /// **Why is this bad?** It is likely to be an error and if not, makes the code + /// ### Why is this bad? + /// It is likely to be an error and if not, makes the code /// less obvious. /// - /// **Known problems:** None. - /// - /// **Example:** + /// ### Example /// ```rust /// let x = 5; /// match x { - /// 1...10 => println!("1 ... 10"), - /// 5...15 => println!("5 ... 15"), + /// 1..=10 => println!("1 ... 10"), + /// 5..=15 => println!("5 ... 15"), /// _ => (), /// } /// ``` + #[clippy::version = "pre 1.29.0"] pub MATCH_OVERLAPPING_ARM, style, "a `match` with overlapping arms" } declare_clippy_lint! { - /// **What it does:** Checks for arm which matches all errors with `Err(_)` + /// ### What it does + /// Checks for arm which matches all errors with `Err(_)` /// and take drastic actions like `panic!`. /// - /// **Why is this bad?** It is generally a bad practice, similar to + /// ### Why is this bad? + /// It is generally a bad practice, similar to /// catching all exceptions in java with `catch(Exception)` /// - /// **Known problems:** None. - /// - /// **Example:** + /// ### Example /// ```rust /// let x: Result = Ok(3); /// match x { @@ -205,20 +211,21 @@ /// Err(_) => panic!("err"), /// } /// ``` + #[clippy::version = "pre 1.29.0"] pub MATCH_WILD_ERR_ARM, pedantic, "a `match` with `Err(_)` arm and take drastic actions" } declare_clippy_lint! { - /// **What it does:** Checks for match which is used to add a reference to an + /// ### What it does + /// Checks for match which is used to add a reference to an /// `Option` value. /// - /// **Why is this bad?** Using `as_ref()` or `as_mut()` instead is shorter. - /// - /// **Known problems:** None. + /// ### Why is this bad? + /// Using `as_ref()` or `as_mut()` instead is shorter. /// - /// **Example:** + /// ### Example /// ```rust /// let x: Option<()> = None; /// @@ -231,20 +238,24 @@ /// // Good /// let r: Option<&()> = x.as_ref(); /// ``` + #[clippy::version = "pre 1.29.0"] pub MATCH_AS_REF, complexity, "a `match` on an Option value instead of using `as_ref()` or `as_mut`" } declare_clippy_lint! { - /// **What it does:** Checks for wildcard enum matches using `_`. + /// ### What it does + /// Checks for wildcard enum matches using `_`. /// - /// **Why is this bad?** New enum variants added by library updates can be missed. + /// ### Why is this bad? + /// New enum variants added by library updates can be missed. /// - /// **Known problems:** Suggested replacements may be incorrect if guards exhaustively cover some + /// ### Known problems + /// Suggested replacements may be incorrect if guards exhaustively cover some /// variants, and also may not use correct path to enum if it's not present in the current scope. /// - /// **Example:** + /// ### Example /// ```rust /// # enum Foo { A(usize), B(usize) } /// # let x = Foo::B(1); @@ -260,21 +271,24 @@ /// Foo::B(_) => {}, /// } /// ``` + #[clippy::version = "1.34.0"] pub WILDCARD_ENUM_MATCH_ARM, restriction, "a wildcard enum match arm using `_`" } declare_clippy_lint! { - /// **What it does:** Checks for wildcard enum matches for a single variant. + /// ### What it does + /// Checks for wildcard enum matches for a single variant. /// - /// **Why is this bad?** New enum variants added by library updates can be missed. + /// ### Why is this bad? + /// New enum variants added by library updates can be missed. /// - /// **Known problems:** Suggested replacements may not use correct path to enum + /// ### Known problems + /// Suggested replacements may not use correct path to enum /// if it's not present in the current scope. /// - /// **Example:** - /// + /// ### Example /// ```rust /// # enum Foo { A, B, C } /// # let x = Foo::B; @@ -292,20 +306,21 @@ /// Foo::C => {}, /// } /// ``` + #[clippy::version = "1.45.0"] pub MATCH_WILDCARD_FOR_SINGLE_VARIANTS, pedantic, "a wildcard enum match for a single variant" } declare_clippy_lint! { - /// **What it does:** Checks for wildcard pattern used with others patterns in same match arm. + /// ### What it does + /// Checks for wildcard pattern used with others patterns in same match arm. /// - /// **Why is this bad?** Wildcard pattern already covers any other pattern as it will match anyway. + /// ### Why is this bad? + /// Wildcard pattern already covers any other pattern as it will match anyway. /// It makes the code less readable, especially to spot wildcard pattern use in match arm. /// - /// **Known problems:** None. - /// - /// **Example:** + /// ### Example /// ```rust /// // Bad /// match "foo" { @@ -319,20 +334,21 @@ /// _ => {}, /// } /// ``` + #[clippy::version = "1.42.0"] pub WILDCARD_IN_OR_PATTERNS, complexity, "a wildcard pattern used with others patterns in same match arm" } declare_clippy_lint! { - /// **What it does:** Checks for matches being used to destructure a single-variant enum + /// ### What it does + /// Checks for matches being used to destructure a single-variant enum /// or tuple struct where a `let` will suffice. /// - /// **Why is this bad?** Just readability – `let` doesn't nest, whereas a `match` does. + /// ### Why is this bad? + /// Just readability – `let` doesn't nest, whereas a `match` does. /// - /// **Known problems:** None. - /// - /// **Example:** + /// ### Example /// ```rust /// enum Wrapper { /// Data(i32), @@ -354,20 +370,24 @@ /// let wrapper = Wrapper::Data(42); /// let Wrapper::Data(data) = wrapper; /// ``` + #[clippy::version = "pre 1.29.0"] pub INFALLIBLE_DESTRUCTURING_MATCH, style, "a `match` statement with a single infallible arm instead of a `let`" } declare_clippy_lint! { - /// **What it does:** Checks for useless match that binds to only one value. + /// ### What it does + /// Checks for useless match that binds to only one value. /// - /// **Why is this bad?** Readability and needless complexity. + /// ### Why is this bad? + /// Readability and needless complexity. /// - /// **Known problems:** Suggested replacements may be incorrect when `match` + /// ### Known problems + /// Suggested replacements may be incorrect when `match` /// is actually binding temporary value, bringing a 'dropped while borrowed' error. /// - /// **Example:** + /// ### Example /// ```rust /// # let a = 1; /// # let b = 2; @@ -382,20 +402,21 @@ /// // Good /// let (c, d) = (a, b); /// ``` + #[clippy::version = "1.43.0"] pub MATCH_SINGLE_BINDING, complexity, "a match with a single binding instead of using `let` statement" } declare_clippy_lint! { - /// **What it does:** Checks for unnecessary '..' pattern binding on struct when all fields are explicitly matched. + /// ### What it does + /// Checks for unnecessary '..' pattern binding on struct when all fields are explicitly matched. /// - /// **Why is this bad?** Correctness and readability. It's like having a wildcard pattern after + /// ### Why is this bad? + /// Correctness and readability. It's like having a wildcard pattern after /// matching all enum variants explicitly. /// - /// **Known problems:** None. - /// - /// **Example:** + /// ### Example /// ```rust /// # struct A { a: i32 } /// let a = A { a: 5 }; @@ -412,27 +433,30 @@ /// _ => {}, /// } /// ``` + #[clippy::version = "1.43.0"] pub REST_PAT_IN_FULLY_BOUND_STRUCTS, restriction, "a match on a struct that binds all fields but still uses the wildcard pattern" } declare_clippy_lint! { - /// **What it does:** Lint for redundant pattern matching over `Result`, `Option`, + /// ### What it does + /// Lint for redundant pattern matching over `Result`, `Option`, /// `std::task::Poll` or `std::net::IpAddr` /// - /// **Why is this bad?** It's more concise and clear to just use the proper + /// ### Why is this bad? + /// It's more concise and clear to just use the proper /// utility function /// - /// **Known problems:** This will change the drop order for the matched type. Both `if let` and + /// ### 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 /// 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 /// drop order. /// - /// **Example:** - /// + /// ### Example /// ```rust /// # use std::task::Poll; /// # use std::net::{IpAddr, Ipv4Addr, Ipv6Addr}; @@ -465,21 +489,25 @@ /// if IpAddr::V6(Ipv6Addr::LOCALHOST).is_ipv6() {} /// Ok::(42).is_ok(); /// ``` + #[clippy::version = "1.31.0"] pub REDUNDANT_PATTERN_MATCHING, style, "use the proper utility function avoiding an `if let`" } declare_clippy_lint! { - /// **What it does:** Checks for `match` or `if let` expressions producing a + /// ### What it does + /// Checks for `match` or `if let` expressions producing a /// `bool` that could be written using `matches!` /// - /// **Why is this bad?** Readability and needless complexity. + /// ### Why is this bad? + /// Readability and needless complexity. /// - /// **Known problems:** This lint falsely triggers, if there are arms with + /// ### Known problems + /// This lint falsely triggers, if there are arms with /// `cfg` attributes that remove an arm evaluating to `false`. /// - /// **Example:** + /// ### Example /// ```rust /// let x = Some(5); /// @@ -498,23 +526,27 @@ /// // Good /// let a = matches!(x, Some(0)); /// ``` + #[clippy::version = "1.47.0"] pub MATCH_LIKE_MATCHES_MACRO, style, "a match that could be written with the matches! macro" } declare_clippy_lint! { - /// **What it does:** Checks for `match` with identical arm bodies. + /// ### What it does + /// Checks for `match` with identical arm bodies. /// - /// **Why is this bad?** This is probably a copy & paste error. If arm bodies + /// ### Why is this bad? + /// This is probably a copy & paste error. If arm bodies /// are the same on purpose, you can factor them /// [using `|`](https://doc.rust-lang.org/book/patterns.html#multiple-patterns). /// - /// **Known problems:** False positive possible with order dependent `match` + /// ### Known problems + /// False positive possible with order dependent `match` /// (see issue /// [#860](https://github.com/rust-lang/rust-clippy/issues/860)). /// - /// **Example:** + /// ### Example /// ```rust,ignore /// match foo { /// Bar => bar(), @@ -539,6 +571,7 @@ /// Quz => quz(), /// } /// ``` + #[clippy::version = "pre 1.29.0"] pub MATCH_SAME_ARMS, pedantic, "`match` with identical arm bodies" @@ -581,7 +614,7 @@ pub fn new(msrv: Option) -> Self { impl<'tcx> LateLintPass<'tcx> for Matches { fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { - if in_external_macro(cx.sess(), expr.span) || in_macro(expr.span) { + if expr.span.from_expansion() { return; } @@ -611,14 +644,13 @@ fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { } } if let ExprKind::Match(ex, arms, _) = expr.kind { - check_match_ref_pats(cx, ex, arms, expr); + check_match_ref_pats(cx, ex, arms.iter().map(|el| el.pat), expr); } } fn check_local(&mut self, cx: &LateContext<'tcx>, local: &'tcx Local<'_>) { if_chain! { - if !in_external_macro(cx.sess(), local.span); - if !in_macro(local.span); + if !local.span.from_expansion(); if let Some(expr) = local.init; if let ExprKind::Match(target, arms, MatchSource::Normal) = expr.kind; if arms.len() == 1 && arms[0].guard.is_none(); @@ -626,7 +658,7 @@ fn check_local(&mut self, cx: &LateContext<'tcx>, local: &'tcx Local<'_>) { QPath::Resolved(None, variant_name), args, _) = arms[0].pat.kind; if args.len() == 1; if let PatKind::Binding(_, arg, ..) = strip_pat_refs(&args[0]).kind; - let body = remove_blocks(arms[0].body); + let body = peel_blocks(arms[0].body); if path_to_local_id(body, arg); then { @@ -653,8 +685,7 @@ fn check_local(&mut self, cx: &LateContext<'tcx>, local: &'tcx Local<'_>) { fn check_pat(&mut self, cx: &LateContext<'tcx>, pat: &'tcx Pat<'_>) { if_chain! { - if !in_external_macro(cx.sess(), pat.span); - if !in_macro(pat.span); + if !pat.span.from_expansion(); if let PatKind::Struct(QPath::Resolved(_, path), fields, true) = pat.kind; if let Some(def_id) = path.res.opt_def_id(); let ty = cx.tcx.type_of(def_id); @@ -681,7 +712,7 @@ fn check_pat(&mut self, cx: &LateContext<'tcx>, pat: &'tcx Pat<'_>) { #[rustfmt::skip] fn check_single_match(cx: &LateContext<'_>, ex: &Expr<'_>, arms: &[Arm<'_>], expr: &Expr<'_>) { if arms.len() == 2 && arms[0].guard.is_none() && arms[1].guard.is_none() { - if in_macro(expr.span) { + if expr.span.from_expansion() { // Don't lint match expressions present in // macro_rules! block return; @@ -692,7 +723,7 @@ fn check_single_match(cx: &LateContext<'_>, ex: &Expr<'_>, arms: &[Arm<'_>], exp return; } let els = arms[1].body; - let els = if is_unit_expr(remove_blocks(els)) { + let els = if is_unit_expr(peel_blocks(els)) { None } else if let ExprKind::Block(Block { stmts, expr: block_expr, .. }, _) = els.kind { if stmts.len() == 1 && block_expr.is_none() || stmts.is_empty() && block_expr.is_some() { @@ -721,7 +752,7 @@ fn check_single_match_single_pattern( expr: &Expr<'_>, els: Option<&Expr<'_>>, ) { - if is_wild(&arms[1].pat) { + if is_wild(arms[1].pat) { report_single_match_single_pattern(cx, ex, arms, expr, els); } } @@ -906,9 +937,8 @@ fn check_match_bool(cx: &LateContext<'_>, ex: &Expr<'_>, arms: &[Arm<'_>], expr: fn check_overlapping_arms<'tcx>(cx: &LateContext<'tcx>, ex: &'tcx Expr<'_>, arms: &'tcx [Arm<'_>]) { if arms.len() >= 2 && cx.typeck_results().expr_ty(ex).is_integral() { let ranges = all_ranges(cx, arms, cx.typeck_results().expr_ty(ex)); - let type_ranges = type_ranges(&ranges); - if !type_ranges.is_empty() { - if let Some((start, end)) = overlapping(&type_ranges) { + if !ranges.is_empty() { + if let Some((start, end)) = overlapping(&ranges) { span_lint_and_note( cx, MATCH_OVERLAPPING_ARM, @@ -924,7 +954,7 @@ fn check_overlapping_arms<'tcx>(cx: &LateContext<'tcx>, ex: &'tcx Expr<'_>, arms fn check_wild_err_arm<'tcx>(cx: &LateContext<'tcx>, ex: &Expr<'tcx>, arms: &[Arm<'tcx>]) { let ex_ty = cx.typeck_results().expr_ty(ex).peel_refs(); - if is_type_diagnostic_item(cx, ex_ty, sym::result_type) { + if is_type_diagnostic_item(cx, ex_ty, sym::Result) { for arm in arms { if let PatKind::TupleStruct(ref path, inner, _) = arm.pat.kind { let path_str = rustc_hir_pretty::to_string(rustc_hir_pretty::NO_ANN, |s| s.print_qpath(path, false)); @@ -935,10 +965,8 @@ fn check_wild_err_arm<'tcx>(cx: &LateContext<'tcx>, ex: &Expr<'tcx>, arms: &[Arm // Looking for unused bindings (i.e.: `_e`) for pat in inner.iter() { if let PatKind::Binding(_, id, ident, None) = pat.kind { - if ident.as_str().starts_with('_') - && !LocalUsedVisitor::new(cx, id).check_expr(arm.body) - { - ident_bind_name = (&ident.name.as_str()).to_string(); + if ident.as_str().starts_with('_') && !is_local_used(cx, arm.body, id) { + ident_bind_name = ident.name.as_str().to_string(); matching_wild = true; } } @@ -946,8 +974,7 @@ fn check_wild_err_arm<'tcx>(cx: &LateContext<'tcx>, ex: &Expr<'tcx>, arms: &[Arm } if_chain! { if matching_wild; - if let ExprKind::Block(block, _) = arm.body.kind; - if is_panic_block(block); + if is_panic_call(arm.body); then { // `Err(_)` or `Err(_e)` arm with `panic!` found span_lint_and_note(cx, @@ -1003,8 +1030,7 @@ fn check_wild_enum_match(cx: &LateContext<'_>, ex: &Expr<'_>, arms: &[Arm<'_>]) let adt_def = match ty.kind() { ty::Adt(adt_def, _) if adt_def.is_enum() - && !(is_type_diagnostic_item(cx, ty, sym::option_type) - || is_type_diagnostic_item(cx, ty, sym::result_type)) => + && !(is_type_diagnostic_item(cx, ty, sym::Option) || is_type_diagnostic_item(cx, ty, sym::Result)) => { adt_def }, @@ -1046,7 +1072,10 @@ fn check_wild_enum_match(cx: &LateContext<'_>, ex: &Expr<'_>, arms: &[Arm<'_>]) PatKind::Path(path) => { #[allow(clippy::match_same_arms)] let id = match cx.qpath_res(path, pat.hir_id) { - Res::Def(DefKind::Const | DefKind::ConstParam | DefKind::AnonConst, _) => return, + Res::Def( + DefKind::Const | DefKind::ConstParam | DefKind::AnonConst | DefKind::InlineConst, + _, + ) => return, Res::Def(_, id) => id, _ => return, }; @@ -1098,7 +1127,7 @@ fn check_wild_enum_match(cx: &LateContext<'_>, ex: &Expr<'_>, arms: &[Arm<'_>]) if let CommonPrefixSearcher::Path(path_prefix) = path_prefix { let mut s = String::new(); for seg in path_prefix { - s.push_str(&seg.ident.as_str()); + s.push_str(seg.ident.as_str()); s.push_str("::"); } s @@ -1107,7 +1136,7 @@ fn check_wild_enum_match(cx: &LateContext<'_>, ex: &Expr<'_>, arms: &[Arm<'_>]) s.push_str("::"); s }, - variant.ident.name, + variant.name, match variant.ctor_kind { CtorKind::Fn if variant.fields.len() == 1 => "(_)", CtorKind::Fn => "(..)", @@ -1151,49 +1180,55 @@ fn check_wild_enum_match(cx: &LateContext<'_>, ex: &Expr<'_>, arms: &[Arm<'_>]) } // If the block contains only a `panic!` macro (as expression or statement) -fn is_panic_block(block: &Block<'_>) -> bool { - match (&block.expr, block.stmts.len(), block.stmts.first()) { - (&Some(exp), 0, _) => is_expn_of(exp.span, "panic").is_some() && is_expn_of(exp.span, "unreachable").is_none(), - (&None, 1, Some(stmt)) => { - is_expn_of(stmt.span, "panic").is_some() && is_expn_of(stmt.span, "unreachable").is_none() - }, - _ => false, - } -} +fn is_panic_call(expr: &Expr<'_>) -> bool { + // Unwrap any wrapping blocks + let span = if let ExprKind::Block(block, _) = expr.kind { + match (&block.expr, block.stmts.len(), block.stmts.first()) { + (&Some(exp), 0, _) => exp.span, + (&None, 1, Some(stmt)) => stmt.span, + _ => return false, + } + } else { + expr.span + }; -fn check_match_ref_pats(cx: &LateContext<'_>, ex: &Expr<'_>, arms: &[Arm<'_>], expr: &Expr<'_>) { - if has_only_ref_pats(arms) { - let mut suggs = Vec::with_capacity(arms.len() + 1); - let (title, msg) = if let ExprKind::AddrOf(BorrowKind::Ref, Mutability::Not, inner) = ex.kind { - let span = ex.span.source_callsite(); - suggs.push((span, Sugg::hir_with_macro_callsite(cx, inner, "..").to_string())); - ( - "you don't need to add `&` to both the expression and the patterns", - "try", - ) - } else { - let span = ex.span.source_callsite(); - suggs.push((span, Sugg::hir_with_macro_callsite(cx, ex, "..").deref().to_string())); - ( - "you don't need to add `&` to all patterns", - "instead of prefixing all patterns with `&`, you can dereference the expression", - ) - }; + is_expn_of(span, "panic").is_some() && is_expn_of(span, "unreachable").is_none() +} - suggs.extend(arms.iter().filter_map(|a| { - if let PatKind::Ref(refp, _) = a.pat.kind { - Some((a.pat.span, snippet(cx, refp.span, "..").to_string())) - } else { - None - } - })); +fn check_match_ref_pats<'a, 'b, I>(cx: &LateContext<'_>, ex: &Expr<'_>, pats: I, expr: &Expr<'_>) +where + 'b: 'a, + I: Clone + Iterator>, +{ + if !has_multiple_ref_pats(pats.clone()) { + return; + } - span_lint_and_then(cx, MATCH_REF_PATS, expr.span, title, |diag| { - if !expr.span.from_expansion() { - multispan_sugg(diag, msg, suggs); - } - }); + let (first_sugg, msg, title); + let span = ex.span.source_callsite(); + if let ExprKind::AddrOf(BorrowKind::Ref, Mutability::Not, inner) = ex.kind { + first_sugg = once((span, Sugg::hir_with_macro_callsite(cx, inner, "..").to_string())); + msg = "try"; + title = "you don't need to add `&` to both the expression and the patterns"; + } else { + first_sugg = once((span, Sugg::hir_with_macro_callsite(cx, ex, "..").deref().to_string())); + msg = "instead of prefixing all patterns with `&`, you can dereference the expression"; + title = "you don't need to add `&` to all patterns"; } + + let remaining_suggs = pats.filter_map(|pat| { + if let PatKind::Ref(refp, _) = pat.kind { + Some((pat.span, snippet(cx, refp.span, "..").to_string())) + } else { + None + } + }); + + span_lint_and_then(cx, MATCH_REF_PATS, expr.span, title, |diag| { + if !expr.span.from_expansion() { + multispan_sugg(diag, msg, first_sugg.chain(remaining_suggs)); + } + }); } fn check_match_as_ref(cx: &LateContext<'_>, ex: &Expr<'_>, arms: &[Arm<'_>], expr: &Expr<'_>) { @@ -1268,46 +1303,99 @@ fn check_wild_in_or_pats(cx: &LateContext<'_>, arms: &[Arm<'_>]) { /// Lint a `match` or `if let .. { .. } else { .. }` expr that could be replaced by `matches!` fn check_match_like_matches<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) -> bool { - if let ExprKind::Match(ex, arms, ref match_source) = &expr.kind { - match match_source { - MatchSource::Normal => find_matches_sugg(cx, ex, arms, expr, false), - MatchSource::IfLetDesugar { .. } => find_matches_sugg(cx, ex, arms, expr, true), - _ => false, - } - } else { - false + if let Some(higher::IfLet { + let_pat, + let_expr, + if_then, + if_else: Some(if_else), + }) = higher::IfLet::hir(cx, expr) + { + return find_matches_sugg( + cx, + let_expr, + IntoIterator::into_iter([(&[][..], Some(let_pat), if_then, None), (&[][..], None, if_else, None)]), + expr, + true, + ); } + + if let ExprKind::Match(scrut, arms, MatchSource::Normal) = expr.kind { + return find_matches_sugg( + cx, + scrut, + arms.iter().map(|arm| { + ( + cx.tcx.hir().attrs(arm.hir_id), + Some(arm.pat), + arm.body, + arm.guard.as_ref(), + ) + }), + expr, + false, + ); + } + + false } -/// Lint a `match` or desugared `if let` for replacement by `matches!` -fn find_matches_sugg(cx: &LateContext<'_>, ex: &Expr<'_>, arms: &[Arm<'_>], expr: &Expr<'_>, desugared: bool) -> bool { +/// Lint a `match` or `if let` for replacement by `matches!` +fn find_matches_sugg<'a, 'b, I>( + cx: &LateContext<'_>, + ex: &Expr<'_>, + mut iter: I, + expr: &Expr<'_>, + is_if_let: bool, +) -> bool +where + 'b: 'a, + I: Clone + + DoubleEndedIterator + + ExactSizeIterator + + Iterator< + Item = ( + &'a [Attribute], + Option<&'a Pat<'b>>, + &'a Expr<'b>, + Option<&'a Guard<'b>>, + ), + >, +{ if_chain! { - if arms.len() >= 2; + if iter.len() >= 2; if cx.typeck_results().expr_ty(expr).is_bool(); - if let Some((b1_arm, b0_arms)) = arms.split_last(); - if let Some(b0) = find_bool_lit(&b0_arms[0].body.kind, desugared); - if let Some(b1) = find_bool_lit(&b1_arm.body.kind, desugared); - if is_wild(&b1_arm.pat); + if let Some((_, last_pat_opt, last_expr, _)) = iter.next_back(); + let iter_without_last = iter.clone(); + if let Some((first_attrs, _, first_expr, first_guard)) = iter.next(); + if let Some(b0) = find_bool_lit(&first_expr.kind, is_if_let); + if let Some(b1) = find_bool_lit(&last_expr.kind, is_if_let); if b0 != b1; - let if_guard = &b0_arms[0].guard; - if if_guard.is_none() || b0_arms.len() == 1; - if cx.tcx.hir().attrs(b0_arms[0].hir_id).is_empty(); - if b0_arms[1..].iter() + if first_guard.is_none() || iter.len() == 0; + if first_attrs.is_empty(); + if iter .all(|arm| { - find_bool_lit(&arm.body.kind, desugared).map_or(false, |b| b == b0) && - arm.guard.is_none() && cx.tcx.hir().attrs(arm.hir_id).is_empty() + find_bool_lit(&arm.2.kind, is_if_let).map_or(false, |b| b == b0) && arm.3.is_none() && arm.0.is_empty() }); then { + if let Some(last_pat) = last_pat_opt { + if !is_wild(last_pat) { + return false; + } + } + // The suggestion may be incorrect, because some arms can have `cfg` attributes // evaluated into `false` and so such arms will be stripped before. let mut applicability = Applicability::MaybeIncorrect; let pat = { use itertools::Itertools as _; - b0_arms.iter() - .map(|arm| snippet_with_applicability(cx, arm.pat.span, "..", &mut applicability)) + iter_without_last + .filter_map(|arm| { + let pat_span = arm.1?.span; + Some(snippet_with_applicability(cx, pat_span, "..", &mut applicability)) + }) .join(" | ") }; - let pat_and_guard = if let Some(Guard::If(g)) = if_guard { + let pat_and_guard = if let Some(Guard::If(g)) = first_guard { format!("{} if {}", pat, snippet_with_applicability(cx, g.span, "..", &mut applicability)) } else { pat @@ -1324,7 +1412,7 @@ fn find_matches_sugg(cx: &LateContext<'_>, ex: &Expr<'_>, arms: &[Arm<'_>], expr cx, MATCH_LIKE_MATCHES_MACRO, expr.span, - &format!("{} expression looks like `matches!` macro", if desugared { "if let .. else" } else { "match" }), + &format!("{} expression looks like `matches!` macro", if is_if_let { "if let .. else" } else { "match" }), "try this", format!( "{}matches!({}, {})", @@ -1342,7 +1430,7 @@ fn find_matches_sugg(cx: &LateContext<'_>, ex: &Expr<'_>, arms: &[Arm<'_>], expr } /// Extract a `bool` or `{ bool }` -fn find_bool_lit(ex: &ExprKind<'_>, desugared: bool) -> Option { +fn find_bool_lit(ex: &ExprKind<'_>, is_if_let: bool) -> Option { match ex { ExprKind::Lit(Spanned { node: LitKind::Bool(b), .. @@ -1354,7 +1442,7 @@ fn find_bool_lit(ex: &ExprKind<'_>, desugared: bool) -> Option { .. }, _, - ) if desugared => { + ) if is_if_let => { if let ExprKind::Lit(Spanned { node: LitKind::Bool(b), .. }) = exp.kind @@ -1370,7 +1458,7 @@ fn find_bool_lit(ex: &ExprKind<'_>, desugared: bool) -> Option { #[allow(clippy::too_many_lines)] fn check_match_single_binding<'a>(cx: &LateContext<'a>, ex: &Expr<'a>, arms: &[Arm<'_>], expr: &Expr<'_>) { - if in_macro(expr.span) || arms.len() != 1 || is_refutable(cx, arms[0].pat) { + if expr.span.from_expansion() || arms.len() != 1 || is_refutable(cx, arms[0].pat) { return; } @@ -1393,7 +1481,7 @@ fn check_match_single_binding<'a>(cx: &LateContext<'a>, ex: &Expr<'a>, arms: &[A let matched_vars = ex.span; let bind_names = arms[0].pat.span; - let match_body = remove_blocks(arms[0].body); + let match_body = peel_blocks(arms[0].body); let mut snippet_body = if match_body.span.from_expansion() { Sugg::hir_with_macro_callsite(cx, match_body, "..").to_string() } else { @@ -1525,35 +1613,39 @@ fn opt_parent_let<'a>(cx: &LateContext<'a>, ex: &Expr<'a>) -> Option<&'a Local<' None } -/// Gets all arms that are unbounded `PatRange`s. -fn all_ranges<'tcx>(cx: &LateContext<'tcx>, arms: &'tcx [Arm<'_>], ty: Ty<'tcx>) -> Vec> { +/// Gets the ranges for each range pattern arm. Applies `ty` bounds for open ranges. +fn all_ranges<'tcx>(cx: &LateContext<'tcx>, arms: &'tcx [Arm<'_>], ty: Ty<'tcx>) -> Vec> { arms.iter() .filter_map(|arm| { if let Arm { pat, guard: None, .. } = *arm { if let PatKind::Range(ref lhs, ref rhs, range_end) = pat.kind { - let lhs = match lhs { + let lhs_const = match lhs { Some(lhs) => constant(cx, cx.typeck_results(), lhs)?.0, None => miri_to_const(ty.numeric_min_val(cx.tcx)?)?, }; - let rhs = match rhs { + let rhs_const = match rhs { Some(rhs) => constant(cx, cx.typeck_results(), rhs)?.0, None => miri_to_const(ty.numeric_max_val(cx.tcx)?)?, }; - let rhs = match range_end { - RangeEnd::Included => Bound::Included(rhs), - RangeEnd::Excluded => Bound::Excluded(rhs), + + let lhs_val = lhs_const.int_value(cx, ty)?; + let rhs_val = rhs_const.int_value(cx, ty)?; + + let rhs_bound = match range_end { + RangeEnd::Included => EndBound::Included(rhs_val), + RangeEnd::Excluded => EndBound::Excluded(rhs_val), }; return Some(SpannedRange { span: pat.span, - node: (lhs, rhs), + node: (lhs_val, rhs_bound), }); } if let PatKind::Lit(value) = pat.kind { - let value = constant(cx, cx.typeck_results(), value)?.0; + let value = constant_full_int(cx, cx.typeck_results(), value)?; return Some(SpannedRange { span: pat.span, - node: (value.clone(), Bound::Included(value)), + node: (value, EndBound::Included(value)), }); } } @@ -1562,44 +1654,16 @@ fn all_ranges<'tcx>(cx: &LateContext<'tcx>, arms: &'tcx [Arm<'_>], ty: Ty<'tcx>) .collect() } -#[derive(Debug, Eq, PartialEq)] -pub struct SpannedRange { - pub span: Span, - pub node: (T, Bound), +#[derive(Clone, Copy, Debug, Eq, PartialEq)] +pub enum EndBound { + Included(T), + Excluded(T), } -type TypedRanges = Vec>; - -/// Gets all `Int` ranges or all `Uint` ranges. Mixed types are an error anyway -/// and other types than -/// `Uint` and `Int` probably don't make sense. -fn type_ranges(ranges: &[SpannedRange]) -> TypedRanges { - ranges - .iter() - .filter_map(|range| match range.node { - (Constant::Int(start), Bound::Included(Constant::Int(end))) => Some(SpannedRange { - span: range.span, - node: (start, Bound::Included(end)), - }), - (Constant::Int(start), Bound::Excluded(Constant::Int(end))) => Some(SpannedRange { - span: range.span, - node: (start, Bound::Excluded(end)), - }), - (Constant::Int(start), Bound::Unbounded) => Some(SpannedRange { - span: range.span, - node: (start, Bound::Unbounded), - }), - _ => None, - }) - .collect() -} - -fn is_unit_expr(expr: &Expr<'_>) -> bool { - match expr.kind { - ExprKind::Tup(v) if v.is_empty() => true, - ExprKind::Block(b, _) if b.stmts.is_empty() && b.expr.is_none() => true, - _ => false, - } +#[derive(Debug, Eq, PartialEq)] +struct SpannedRange { + pub span: Span, + pub node: (T, EndBound), } // Checks if arm has the form `None => None` @@ -1614,7 +1678,7 @@ fn is_ref_some_arm(cx: &LateContext<'_>, arm: &Arm<'_>) -> Option, arm: &Arm<'_>) -> Option]) -> bool { - let mapped = arms - .iter() - .map(|a| { - match a.pat.kind { - PatKind::Ref(..) => Some(true), // &-patterns - PatKind::Wild => Some(false), // an "anything" wildcard is also fine - _ => None, // any other pattern is not fine +fn has_multiple_ref_pats<'a, 'b, I>(pats: I) -> bool +where + 'b: 'a, + I: Iterator>, +{ + let mut ref_count = 0; + for opt in pats.map(|pat| match pat.kind { + PatKind::Ref(..) => Some(true), // &-patterns + PatKind::Wild => Some(false), // an "anything" wildcard is also fine + _ => None, // any other pattern is not fine + }) { + if let Some(inner) = opt { + if inner { + ref_count += 1; } - }) - .collect::>>(); - // look for Some(v) where there's at least one true element - mapped.map_or(false, |v| v.iter().any(|el| *el)) + } else { + return false; + } + } + ref_count > 1 } -pub fn overlapping(ranges: &[SpannedRange]) -> Option<(&SpannedRange, &SpannedRange)> +fn overlapping(ranges: &[SpannedRange]) -> Option<(&SpannedRange, &SpannedRange)> where T: Copy + Ord, { - #[derive(Copy, Clone, Debug, Eq, PartialEq)] - enum Kind<'a, T> { - Start(T, &'a SpannedRange), - End(Bound, &'a SpannedRange), + #[derive(Copy, Clone, Debug, Eq, Ord, PartialEq, PartialOrd)] + enum BoundKind { + EndExcluded, + Start, + EndIncluded, } - impl<'a, T: Copy> Kind<'a, T> { - fn range(&self) -> &'a SpannedRange { - match *self { - Kind::Start(_, r) | Kind::End(_, r) => r, - } - } - - fn value(self) -> Bound { - match self { - Kind::Start(t, _) => Bound::Included(t), - Kind::End(t, _) => t, - } - } - } + #[derive(Copy, Clone, Debug, Eq, PartialEq)] + struct RangeBound<'a, T>(T, BoundKind, &'a SpannedRange); - impl<'a, T: Copy + Ord> PartialOrd for Kind<'a, T> { + impl<'a, T: Copy + Ord> PartialOrd for RangeBound<'a, T> { fn partial_cmp(&self, other: &Self) -> Option { Some(self.cmp(other)) } } - impl<'a, T: Copy + Ord> Ord for Kind<'a, T> { - fn cmp(&self, other: &Self) -> Ordering { - match (self.value(), other.value()) { - (Bound::Included(a), Bound::Included(b)) | (Bound::Excluded(a), Bound::Excluded(b)) => a.cmp(&b), - // Range patterns cannot be unbounded (yet) - (Bound::Unbounded, _) | (_, Bound::Unbounded) => unimplemented!(), - (Bound::Included(a), Bound::Excluded(b)) => match a.cmp(&b) { - Ordering::Equal => Ordering::Greater, - other => other, - }, - (Bound::Excluded(a), Bound::Included(b)) => match a.cmp(&b) { - Ordering::Equal => Ordering::Less, - other => other, - }, - } + impl<'a, T: Copy + Ord> Ord for RangeBound<'a, T> { + fn cmp(&self, RangeBound(other_value, other_kind, _): &Self) -> Ordering { + let RangeBound(self_value, self_kind, _) = *self; + (self_value, self_kind).cmp(&(*other_value, *other_kind)) } } let mut values = Vec::with_capacity(2 * ranges.len()); - for r in ranges { - values.push(Kind::Start(r.node.0, r)); - values.push(Kind::End(r.node.1, r)); + for r @ SpannedRange { node: (start, end), .. } in ranges { + values.push(RangeBound(*start, BoundKind::Start, r)); + values.push(match end { + EndBound::Excluded(val) => RangeBound(*val, BoundKind::EndExcluded, r), + EndBound::Included(val) => RangeBound(*val, BoundKind::EndIncluded, r), + }); } values.sort(); - for (a, b) in iter::zip(&values, values.iter().skip(1)) { - match (a, b) { - (&Kind::Start(_, ra), &Kind::End(_, rb)) => { - if ra.node != rb.node { - return Some((ra, rb)); - } - }, - (&Kind::End(a, _), &Kind::Start(b, _)) if a != Bound::Included(b) => (), - _ => { - // skip if the range `a` is completely included into the range `b` - if let Ordering::Equal | Ordering::Less = a.cmp(b) { - let kind_a = Kind::End(a.range().node.1, a.range()); - let kind_b = Kind::End(b.range().node.1, b.range()); - if let Ordering::Equal | Ordering::Greater = kind_a.cmp(&kind_b) { - return None; + let mut started = vec![]; + + for RangeBound(_, kind, range) in values { + match kind { + BoundKind::Start => started.push(range), + BoundKind::EndExcluded | BoundKind::EndIncluded => { + let mut overlap = None; + + while let Some(last_started) = started.pop() { + if last_started == range { + break; } + overlap = Some(last_started); + } + + if let Some(first_overlapping) = overlap { + return Some((range, first_overlapping)); } - return Some((a.range(), b.range())); }, } } @@ -1727,7 +1779,9 @@ fn cmp(&self, other: &Self) -> Ordering { mod redundant_pattern_match { use super::REDUNDANT_PATTERN_MATCHING; use clippy_utils::diagnostics::span_lint_and_then; - use clippy_utils::source::{snippet, snippet_with_applicability}; + use clippy_utils::higher; + use clippy_utils::source::snippet; + use clippy_utils::sugg::Sugg; use clippy_utils::ty::{implements_trait, is_type_diagnostic_item, is_type_lang_item, match_type}; use clippy_utils::{is_lang_ctor, is_qpath_def_path, is_trait_method, paths}; use if_chain::if_chain; @@ -1737,22 +1791,27 @@ mod redundant_pattern_match { use rustc_hir::LangItem::{OptionNone, OptionSome, PollPending, PollReady, ResultErr, ResultOk}; use rustc_hir::{ intravisit::{walk_expr, ErasedMap, NestedVisitorMap, Visitor}, - Arm, Block, Expr, ExprKind, LangItem, MatchSource, Node, PatKind, QPath, + Arm, Block, Expr, ExprKind, LangItem, MatchSource, Node, Pat, PatKind, QPath, UnOp, }; use rustc_lint::LateContext; use rustc_middle::ty::{self, subst::GenericArgKind, Ty}; use rustc_span::sym; pub fn check<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { - if let ExprKind::Match(op, arms, ref match_source) = &expr.kind { - match match_source { - MatchSource::Normal => find_sugg_for_match(cx, expr, op, arms), - MatchSource::IfLetDesugar { contains_else_clause } => { - find_sugg_for_if_let(cx, expr, op, &arms[0], "if", *contains_else_clause); - }, - MatchSource::WhileLetDesugar => find_sugg_for_if_let(cx, expr, op, &arms[0], "while", false), - _ => {}, - } + if let Some(higher::IfLet { + if_else, + let_pat, + let_expr, + .. + }) = higher::IfLet::hir(cx, expr) + { + find_sugg_for_if_let(cx, expr, let_pat, let_expr, "if", if_else.is_some()); + } + if let ExprKind::Match(op, arms, MatchSource::Normal) = &expr.kind { + find_sugg_for_match(cx, expr, op, arms); + } + 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); } } @@ -1788,13 +1847,13 @@ fn type_needs_ordered_drop_inner(cx: &LateContext<'tcx>, ty: Ty<'tcx>, seen: &mu } } // Check for std types which implement drop, but only for memory allocation. - else if is_type_diagnostic_item(cx, ty, sym::vec_type) + else if is_type_diagnostic_item(cx, ty, sym::Vec) || is_type_lang_item(cx, ty, LangItem::OwnedBox) || is_type_diagnostic_item(cx, ty, sym::Rc) || is_type_diagnostic_item(cx, ty, sym::Arc) || is_type_diagnostic_item(cx, ty, sym::cstring_type) - || match_type(cx, ty, &paths::BTREEMAP) - || match_type(cx, ty, &paths::LINKED_LIST) + || is_type_diagnostic_item(cx, ty, sym::BTreeMap) + || is_type_diagnostic_item(cx, ty, sym::LinkedList) || match_type(cx, ty, &paths::WEAK_RC) || match_type(cx, ty, &paths::WEAK_ARC) { @@ -1906,18 +1965,18 @@ fn visit_expr(&mut self, expr: &'tcx Expr<'tcx>) { fn find_sugg_for_if_let<'tcx>( cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>, - op: &'tcx Expr<'tcx>, - arm: &Arm<'_>, + let_pat: &Pat<'_>, + let_expr: &'tcx Expr<'_>, keyword: &'static str, has_else: bool, ) { // also look inside refs - let mut kind = &arm.pat.kind; + let mut kind = &let_pat.kind; // if we have &None for example, peel it so we can detect "if let None = x" if let PatKind::Ref(inner, _mutability) = kind { kind = &inner.kind; } - let op_ty = cx.typeck_results().expr_ty(op); + let op_ty = cx.typeck_results().expr_ty(let_expr); // Determine which function should be used, and the type contained by the corresponding // variant. let (good_method, inner_ty) = match kind { @@ -1971,47 +2030,52 @@ fn find_sugg_for_if_let<'tcx>( // scrutinee would be, so they have to be considered as well. // e.g. in `if let Some(x) = foo.lock().unwrap().baz.as_ref() { .. }` the lock will be held // for the duration if body. - let needs_drop = type_needs_ordered_drop(cx, check_ty) || temporaries_need_ordered_drop(cx, op); + let needs_drop = type_needs_ordered_drop(cx, check_ty) || temporaries_need_ordered_drop(cx, let_expr); // check that `while_let_on_iterator` lint does not trigger if_chain! { if keyword == "while"; - if let ExprKind::MethodCall(method_path, _, _, _) = op.kind; + if let ExprKind::MethodCall(method_path, _, _, _) = let_expr.kind; if method_path.ident.name == sym::next; - if is_trait_method(cx, op, sym::Iterator); + if is_trait_method(cx, let_expr, sym::Iterator); then { return; } } - let result_expr = match &op.kind { + let result_expr = match &let_expr.kind { ExprKind::AddrOf(_, _, borrowed) => borrowed, - _ => op, + ExprKind::Unary(UnOp::Deref, deref) => deref, + _ => let_expr, }; + span_lint_and_then( cx, REDUNDANT_PATTERN_MATCHING, - arm.pat.span, + let_pat.span, &format!("redundant pattern matching, consider using `{}`", good_method), |diag| { - // while let ... = ... { ... } + // if/while let ... = ... { ... } // ^^^^^^^^^^^^^^^^^^^^^^^^^^^ let expr_span = expr.span; - // while let ... = ... { ... } + // if/while let ... = ... { ... } // ^^^ let op_span = result_expr.span.source_callsite(); - // while let ... = ... { ... } + // if/while let ... = ... { ... } // ^^^^^^^^^^^^^^^^^^^ let span = expr_span.until(op_span.shrink_to_hi()); - let mut app = if needs_drop { + let app = if needs_drop { Applicability::MaybeIncorrect } else { Applicability::MachineApplicable }; - let sugg = snippet_with_applicability(cx, op_span, "_", &mut app); + + let sugg = Sugg::hir_with_macro_callsite(cx, result_expr, "_") + .maybe_par() + .to_string(); diag.span_suggestion(span, "try this", format!("{} {}.{}", keyword, sugg, good_method), app); @@ -2161,29 +2225,29 @@ fn test_overlapping() { }; assert_eq!(None, overlapping::(&[])); - assert_eq!(None, overlapping(&[sp(1, Bound::Included(4))])); + assert_eq!(None, overlapping(&[sp(1, EndBound::Included(4))])); assert_eq!( None, - overlapping(&[sp(1, Bound::Included(4)), sp(5, Bound::Included(6))]) + overlapping(&[sp(1, EndBound::Included(4)), sp(5, EndBound::Included(6))]) ); assert_eq!( None, overlapping(&[ - sp(1, Bound::Included(4)), - sp(5, Bound::Included(6)), - sp(10, Bound::Included(11)) + sp(1, EndBound::Included(4)), + sp(5, EndBound::Included(6)), + sp(10, EndBound::Included(11)) ],) ); assert_eq!( - Some((&sp(1, Bound::Included(4)), &sp(3, Bound::Included(6)))), - overlapping(&[sp(1, Bound::Included(4)), sp(3, Bound::Included(6))]) + Some((&sp(1, EndBound::Included(4)), &sp(3, EndBound::Included(6)))), + overlapping(&[sp(1, EndBound::Included(4)), sp(3, EndBound::Included(6))]) ); assert_eq!( - Some((&sp(5, Bound::Included(6)), &sp(6, Bound::Included(11)))), + Some((&sp(5, EndBound::Included(6)), &sp(6, EndBound::Included(11)))), overlapping(&[ - sp(1, Bound::Included(4)), - sp(5, Bound::Included(6)), - sp(6, Bound::Included(11)) + sp(1, EndBound::Included(4)), + sp(5, EndBound::Included(6)), + sp(6, EndBound::Included(11)) ],) ); }