From cb6268026206e6596b4754fdedf8f53a0e632273 Mon Sep 17 00:00:00 2001 From: Vadim Petrochenkov Date: Thu, 18 Nov 2021 13:25:27 +0800 Subject: [PATCH 1/4] rustc: Remove `#[rustc_synthetic]` This function parameter attribute was introduced in https://github.com/rust-lang/rust/pull/44866 as an intermediate step in implementing `impl Trait`, it's not necessary or used anywhere by itself. --- clippy_lints/src/types/borrowed_box.rs | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/clippy_lints/src/types/borrowed_box.rs b/clippy_lints/src/types/borrowed_box.rs index bdeff035e5e..63ad65b8afd 100644 --- a/clippy_lints/src/types/borrowed_box.rs +++ b/clippy_lints/src/types/borrowed_box.rs @@ -3,10 +3,8 @@ use clippy_utils::source::snippet; use clippy_utils::{match_def_path, paths}; use if_chain::if_chain; use rustc_errors::Applicability; -use rustc_hir::{ - self as hir, GenericArg, GenericBounds, GenericParamKind, HirId, Lifetime, MutTy, Mutability, Node, QPath, - SyntheticTyParamKind, TyKind, -}; +use rustc_hir::{self as hir, GenericArg, GenericBounds, GenericParamKind}; +use rustc_hir::{HirId, Lifetime, MutTy, Mutability, Node, QPath, TyKind}; use rustc_lint::LateContext; use super::BORROWED_BOX; @@ -105,7 +103,7 @@ fn get_bounds_if_impl_trait<'tcx>(cx: &LateContext<'tcx>, qpath: &QPath<'_>, id: if let Some(did) = cx.qpath_res(qpath, id).opt_def_id(); if let Some(Node::GenericParam(generic_param)) = cx.tcx.hir().get_if_local(did); if let GenericParamKind::Type { synthetic, .. } = generic_param.kind; - if synthetic == Some(SyntheticTyParamKind::ImplTrait); + if synthetic; then { Some(generic_param.bounds) } else { From e58ffb88e69094b8ee170f15f66fa472ee060752 Mon Sep 17 00:00:00 2001 From: Cameron Steffen Date: Wed, 27 Oct 2021 09:48:06 -0500 Subject: [PATCH 2/4] Fix Clippy with changed for loop desugar --- .../src/loops/explicit_counter_loop.rs | 8 +-- clippy_lints/src/loops/manual_memcpy.rs | 4 +- clippy_lints/src/loops/mod.rs | 19 +++++- clippy_lints/src/loops/never_loop.rs | 58 +++++++++-------- clippy_lints/src/loops/single_element_loop.rs | 5 +- clippy_lints/src/loops/utils.rs | 12 ---- clippy_lints/src/misc.rs | 5 +- clippy_lints/src/unit_types/let_unit_value.rs | 4 -- clippy_utils/src/higher.rs | 62 +++++-------------- tests/ui/author/for_loop.stdout | 53 ++++++---------- 10 files changed, 90 insertions(+), 140 deletions(-) diff --git a/clippy_lints/src/loops/explicit_counter_loop.rs b/clippy_lints/src/loops/explicit_counter_loop.rs index 98e60f7ed85..6f213d7a699 100644 --- a/clippy_lints/src/loops/explicit_counter_loop.rs +++ b/clippy_lints/src/loops/explicit_counter_loop.rs @@ -1,6 +1,4 @@ -use super::{ - get_span_of_entire_for_loop, make_iterator_snippet, IncrementVisitor, InitializeVisitor, EXPLICIT_COUNTER_LOOP, -}; +use super::{make_iterator_snippet, IncrementVisitor, InitializeVisitor, EXPLICIT_COUNTER_LOOP}; use clippy_utils::diagnostics::span_lint_and_sugg; use clippy_utils::source::snippet_with_applicability; use clippy_utils::{get_enclosing_block, is_integer_const}; @@ -37,12 +35,10 @@ pub(super) fn check<'tcx>( then { let mut applicability = Applicability::MachineApplicable; - let for_span = get_span_of_entire_for_loop(expr); - span_lint_and_sugg( cx, EXPLICIT_COUNTER_LOOP, - for_span.with_hi(arg.span.hi()), + expr.span.with_hi(arg.span.hi()), &format!("the variable `{}` is used as a loop counter", name), "consider using", format!( diff --git a/clippy_lints/src/loops/manual_memcpy.rs b/clippy_lints/src/loops/manual_memcpy.rs index 72027a163af..2362b4b2067 100644 --- a/clippy_lints/src/loops/manual_memcpy.rs +++ b/clippy_lints/src/loops/manual_memcpy.rs @@ -1,4 +1,4 @@ -use super::{get_span_of_entire_for_loop, IncrementVisitor, InitializeVisitor, MANUAL_MEMCPY}; +use super::{IncrementVisitor, InitializeVisitor, MANUAL_MEMCPY}; use clippy_utils::diagnostics::span_lint_and_sugg; use clippy_utils::source::snippet; use clippy_utils::sugg::Sugg; @@ -86,7 +86,7 @@ pub(super) fn check<'tcx>( span_lint_and_sugg( cx, MANUAL_MEMCPY, - get_span_of_entire_for_loop(expr), + expr.span, "it looks like you're manually copying between slices", "try replacing the loop by", big_sugg, diff --git a/clippy_lints/src/loops/mod.rs b/clippy_lints/src/loops/mod.rs index 5df1b796401..fd4881b2947 100644 --- a/clippy_lints/src/loops/mod.rs +++ b/clippy_lints/src/loops/mod.rs @@ -23,7 +23,7 @@ use rustc_hir::{Expr, ExprKind, LoopSource, Pat}; use rustc_lint::{LateContext, LateLintPass}; use rustc_session::{declare_lint_pass, declare_tool_lint}; use rustc_span::source_map::Span; -use utils::{get_span_of_entire_for_loop, make_iterator_snippet, IncrementVisitor, InitializeVisitor}; +use utils::{make_iterator_snippet, IncrementVisitor, InitializeVisitor}; declare_clippy_lint! { /// ### What it does @@ -566,7 +566,15 @@ declare_lint_pass!(Loops => [ impl<'tcx> LateLintPass<'tcx> for Loops { #[allow(clippy::too_many_lines)] fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { - if let Some(higher::ForLoop { pat, arg, body, span }) = higher::ForLoop::hir(expr) { + let for_loop = higher::ForLoop::hir(expr); + if let Some(higher::ForLoop { + pat, + arg, + body, + loop_id, + span, + }) = for_loop + { // we don't want to check expanded macros // this check is not at the top of the function // since higher::for_loop expressions are marked as expansions @@ -574,6 +582,9 @@ impl<'tcx> LateLintPass<'tcx> for Loops { return; } check_for_loop(cx, pat, arg, body, expr, span); + if let ExprKind::Block(block, _) = body.kind { + never_loop::check(cx, block, loop_id, span, for_loop.as_ref()); + } } // we don't want to check expanded macros @@ -582,7 +593,9 @@ impl<'tcx> LateLintPass<'tcx> for Loops { } // check for never_loop - never_loop::check(cx, expr); + if let ExprKind::Loop(block, ..) = expr.kind { + never_loop::check(cx, block, expr.hir_id, expr.span, None); + } // check for `loop { if let {} else break }` that could be `while let` // (also matches an explicit "match" instead of "if let") diff --git a/clippy_lints/src/loops/never_loop.rs b/clippy_lints/src/loops/never_loop.rs index c0fde5e5166..86b7d6d989a 100644 --- a/clippy_lints/src/loops/never_loop.rs +++ b/clippy_lints/src/loops/never_loop.rs @@ -4,35 +4,41 @@ use clippy_utils::diagnostics::span_lint_and_then; use clippy_utils::higher::ForLoop; use clippy_utils::source::snippet; use rustc_errors::Applicability; -use rustc_hir::{Block, Expr, ExprKind, HirId, InlineAsmOperand, LoopSource, Node, Pat, Stmt, StmtKind}; +use rustc_hir::{Block, Expr, ExprKind, HirId, InlineAsmOperand, Pat, Stmt, StmtKind}; use rustc_lint::LateContext; +use rustc_span::Span; use std::iter::{once, Iterator}; -pub(super) fn check(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { - if let ExprKind::Loop(block, _, source, _) = expr.kind { - match never_loop_block(block, expr.hir_id) { - NeverLoopResult::AlwaysBreak => { - span_lint_and_then(cx, NEVER_LOOP, expr.span, "this loop never actually loops", |diag| { - if_chain! { - if source == LoopSource::ForLoop; - if let Some((_, Node::Expr(parent_match))) = cx.tcx.hir().parent_iter(expr.hir_id).nth(1); - if let Some(ForLoop { arg: iterator, pat, span: for_span, .. }) = ForLoop::hir(parent_match); - then { - // Suggests using an `if let` instead. This is `Unspecified` because the - // loop may (probably) contain `break` statements which would be invalid - // in an `if let`. - diag.span_suggestion_verbose( - for_span.with_hi(iterator.span.hi()), - "if you need the first element of the iterator, try writing", - for_to_if_let_sugg(cx, iterator, pat), - Applicability::Unspecified, - ); - } - }; - }); - }, - NeverLoopResult::MayContinueMainLoop | NeverLoopResult::Otherwise => (), - } +pub(super) fn check( + cx: &LateContext<'tcx>, + block: &'tcx Block<'_>, + loop_id: HirId, + span: Span, + for_loop: Option<&ForLoop<'_>>, +) { + match never_loop_block(block, loop_id) { + NeverLoopResult::AlwaysBreak => { + span_lint_and_then(cx, NEVER_LOOP, span, "this loop never actually loops", |diag| { + if let Some(ForLoop { + arg: iterator, + pat, + span: for_span, + .. + }) = for_loop + { + // Suggests using an `if let` instead. This is `Unspecified` because the + // loop may (probably) contain `break` statements which would be invalid + // in an `if let`. + diag.span_suggestion_verbose( + for_span.with_hi(iterator.span.hi()), + "if you need the first element of the iterator, try writing", + for_to_if_let_sugg(cx, iterator, pat), + Applicability::Unspecified, + ); + } + }); + }, + NeverLoopResult::MayContinueMainLoop | NeverLoopResult::Otherwise => (), } } diff --git a/clippy_lints/src/loops/single_element_loop.rs b/clippy_lints/src/loops/single_element_loop.rs index 0fd09ff7197..e39605f3e7d 100644 --- a/clippy_lints/src/loops/single_element_loop.rs +++ b/clippy_lints/src/loops/single_element_loop.rs @@ -1,4 +1,4 @@ -use super::{get_span_of_entire_for_loop, SINGLE_ELEMENT_LOOP}; +use super::SINGLE_ELEMENT_LOOP; use clippy_utils::diagnostics::span_lint_and_sugg; use clippy_utils::single_segment_path; use clippy_utils::source::{indent_of, snippet}; @@ -30,7 +30,6 @@ pub(super) fn check<'tcx>( if !block.stmts.is_empty(); then { - let for_span = get_span_of_entire_for_loop(expr); let mut block_str = snippet(cx, block.span, "..").into_owned(); block_str.remove(0); block_str.pop(); @@ -39,7 +38,7 @@ pub(super) fn check<'tcx>( span_lint_and_sugg( cx, SINGLE_ELEMENT_LOOP, - for_span, + expr.span, "for loop over a single element", "try", format!("{{\n{}let {} = &{};{}}}", " ".repeat(indent_of(cx, block.stmts[0].span).unwrap_or(0)), target.name, list_item_name, block_str), diff --git a/clippy_lints/src/loops/utils.rs b/clippy_lints/src/loops/utils.rs index f9f515cc40a..c3939a66c6a 100644 --- a/clippy_lints/src/loops/utils.rs +++ b/clippy_lints/src/loops/utils.rs @@ -7,7 +7,6 @@ use rustc_hir::HirIdMap; use rustc_hir::{BinOpKind, BorrowKind, Expr, ExprKind, HirId, Mutability, Pat, PatKind, Stmt, StmtKind}; use rustc_lint::LateContext; use rustc_middle::hir::map::Map; -use rustc_span::source_map::Span; use rustc_span::symbol::{sym, Symbol}; use std::iter::Iterator; @@ -300,17 +299,6 @@ impl<'tcx> Visitor<'tcx> for LoopNestVisitor { } } -// this function assumes the given expression is a `for` loop. -pub(super) fn get_span_of_entire_for_loop(expr: &Expr<'_>) -> Span { - // for some reason this is the only way to get the `Span` - // of the entire `for` loop - if let ExprKind::Match(_, arms, _) = &expr.kind { - arms[0].body.span - } else { - unreachable!() - } -} - /// If `arg` was the argument to a `for` loop, return the "cleanest" way of writing the /// actual `Iterator` that the loop uses. pub(super) fn make_iterator_snippet(cx: &LateContext<'_>, arg: &Expr<'_>, applic_ref: &mut Applicability) -> String { diff --git a/clippy_lints/src/misc.rs b/clippy_lints/src/misc.rs index 0f32cd9164e..78183add9cc 100644 --- a/clippy_lints/src/misc.rs +++ b/clippy_lints/src/misc.rs @@ -20,8 +20,8 @@ use rustc_span::symbol::sym; use clippy_utils::consts::{constant, Constant}; use clippy_utils::sugg::Sugg; use clippy_utils::{ - expr_path_res, get_item_name, get_parent_expr, higher, in_constant, is_diag_trait_item, is_integer_const, - iter_input_pats, last_path_segment, match_any_def_paths, paths, unsext, SpanlessEq, + expr_path_res, get_item_name, get_parent_expr, in_constant, is_diag_trait_item, is_integer_const, iter_input_pats, + last_path_segment, match_any_def_paths, paths, unsext, SpanlessEq, }; declare_clippy_lint! { @@ -312,7 +312,6 @@ impl<'tcx> LateLintPass<'tcx> for MiscLints { if let StmtKind::Local(local) = stmt.kind; if let PatKind::Binding(an, .., name, None) = local.pat.kind; if let Some(init) = local.init; - if !higher::is_from_for_desugar(local); if an == BindingAnnotation::Ref || an == BindingAnnotation::RefMut; then { // use the macro callsite when the init span (but not the whole local span) diff --git a/clippy_lints/src/unit_types/let_unit_value.rs b/clippy_lints/src/unit_types/let_unit_value.rs index fad647dfb26..b25a6e3375b 100644 --- a/clippy_lints/src/unit_types/let_unit_value.rs +++ b/clippy_lints/src/unit_types/let_unit_value.rs @@ -1,5 +1,4 @@ use clippy_utils::diagnostics::span_lint_and_then; -use clippy_utils::higher; use clippy_utils::source::snippet_with_macro_callsite; use rustc_errors::Applicability; use rustc_hir::{Stmt, StmtKind}; @@ -14,9 +13,6 @@ pub(super) fn check(cx: &LateContext<'_>, stmt: &Stmt<'_>) { if in_external_macro(cx.sess(), stmt.span) || local.pat.span.from_expansion() { return; } - if higher::is_from_for_desugar(local) { - return; - } span_lint_and_then( cx, LET_UNIT_VALUE, diff --git a/clippy_utils/src/higher.rs b/clippy_utils/src/higher.rs index 733cc97c845..7297265d08c 100644 --- a/clippy_utils/src/higher.rs +++ b/clippy_utils/src/higher.rs @@ -22,31 +22,31 @@ pub struct ForLoop<'tcx> { pub arg: &'tcx hir::Expr<'tcx>, /// `for` loop body pub body: &'tcx hir::Expr<'tcx>, + /// Compare this against `hir::Destination.target` + pub loop_id: HirId, /// entire `for` loop span pub span: Span, } impl<'tcx> ForLoop<'tcx> { - #[inline] /// Parses a desugared `for` loop pub fn hir(expr: &Expr<'tcx>) -> Option { if_chain! { - if let hir::ExprKind::Match(iterexpr, arms, hir::MatchSource::ForLoopDesugar) = expr.kind; - if let Some(first_arm) = arms.get(0); - if let hir::ExprKind::Call(_, iterargs) = iterexpr.kind; - if let Some(first_arg) = iterargs.get(0); - if iterargs.len() == 1 && arms.len() == 1 && first_arm.guard.is_none(); - if let hir::ExprKind::Loop(block, ..) = first_arm.body.kind; - if block.expr.is_none(); - if let [ _, _, ref let_stmt, ref body ] = *block.stmts; - if let hir::StmtKind::Local(local) = let_stmt.kind; - if let hir::StmtKind::Expr(body_expr) = body.kind; + if let hir::ExprKind::DropTemps(e) = expr.kind; + if let hir::ExprKind::Match(iterexpr, [arm], hir::MatchSource::ForLoopDesugar) = e.kind; + if let hir::ExprKind::Call(_, [arg]) = iterexpr.kind; + if let hir::ExprKind::Loop(block, ..) = arm.body.kind; + if let [stmt] = &*block.stmts; + if let hir::StmtKind::Expr(e) = stmt.kind; + if let hir::ExprKind::Match(_, [_, some_arm], _) = e.kind; + if let hir::PatKind::Struct(_, [field], _) = some_arm.pat.kind; then { return Some(Self { - pat: &*local.pat, - arg: first_arg, - body: body_expr, - span: first_arm.span + pat: field.pat, + arg, + body: some_arm.body, + loop_id: arm.body.hir_id, + span: expr.span.ctxt().outer_expn_data().call_site, }); } } @@ -678,38 +678,6 @@ impl<'tcx> FormatArgsArg<'tcx> { } } -/// Checks if a `let` statement is from a `for` loop desugaring. -pub fn is_from_for_desugar(local: &hir::Local<'_>) -> bool { - // This will detect plain for-loops without an actual variable binding: - // - // ``` - // for x in some_vec { - // // do stuff - // } - // ``` - if_chain! { - if let Some(expr) = local.init; - if let hir::ExprKind::Match(_, _, hir::MatchSource::ForLoopDesugar) = expr.kind; - then { - return true; - } - } - - // This detects a variable binding in for loop to avoid `let_unit_value` - // lint (see issue #1964). - // - // ``` - // for _ in vec![()] { - // // anything - // } - // ``` - if let hir::LocalSource::ForLoopDesugar = local.source { - return true; - } - - false -} - /// A parsed `panic!` expansion pub struct PanicExpn<'tcx> { /// Span of `panic!(..)` diff --git a/tests/ui/author/for_loop.stdout b/tests/ui/author/for_loop.stdout index f1b4d4e096e..4d0e13c833f 100644 --- a/tests/ui/author/for_loop.stdout +++ b/tests/ui/author/for_loop.stdout @@ -11,11 +11,8 @@ if_chain! { // unimplemented: field checks if arms.len() == 1; if let ExprKind::Loop(ref body, ref label, LoopSource::ForLoop) = arms[0].body.kind; - if body.stmts.len() == 4; - if let StmtKind::Local(ref local) = body.stmts[0].kind; - if let PatKind::Binding(BindingAnnotation::Mutable, _, name, None) = local.pat.kind; - if name.as_str() == "__next"; - if let StmtKind::Expr(ref e, _) = body.stmts[1].kind + if body.stmts.len() == 1; + if let StmtKind::Expr(ref e, _) = body.stmts[0].kind if let ExprKind::Match(ref expr2, ref arms1, MatchSource::ForLoopDesugar) = e.kind; if let ExprKind::Call(ref func1, ref args1) = expr2.kind; if let ExprKind::Path(ref path2) = func1.kind; @@ -25,39 +22,27 @@ if_chain! { if let ExprKind::Path(ref path3) = inner.kind; if match_qpath(path3, &["iter"]); if arms1.len() == 2; - if let ExprKind::Assign(ref target, ref value, ref _span) = arms1[0].body.kind; - if let ExprKind::Path(ref path4) = target.kind; - if match_qpath(path4, &["__next"]); - if let ExprKind::Path(ref path5) = value.kind; - if match_qpath(path5, &["val"]); - if let PatKind::Struct(ref path6, ref fields1, false) = arms1[0].pat.kind; - if matches!(path6, QPath::LangItem(LangItem::OptionSome, _)); - if fields1.len() == 1; + if let ExprKind::Break(ref destination, None) = arms1[0].body.kind; + if let PatKind::Struct(ref path4, ref fields1, false) = arms1[0].pat.kind; + if matches!(path4, QPath::LangItem(LangItem::OptionNone, _)); + if fields1.len() == 0; // unimplemented: field checks - if let ExprKind::Break(ref destination, None) = arms1[1].body.kind; - if let PatKind::Struct(ref path7, ref fields2, false) = arms1[1].pat.kind; - if matches!(path7, QPath::LangItem(LangItem::OptionNone, _)); - if fields2.len() == 0; - // unimplemented: field checks - if let StmtKind::Local(ref local1) = body.stmts[2].kind; - if let Some(ref init) = local1.init; - if let ExprKind::Path(ref path8) = init.kind; - if match_qpath(path8, &["__next"]); - if let PatKind::Binding(BindingAnnotation::Unannotated, _, name1, None) = local1.pat.kind; - if name1.as_str() == "y"; - if let StmtKind::Expr(ref e1, _) = body.stmts[3].kind - if let ExprKind::Block(ref block) = e1.kind; + if let ExprKind::Block(ref block) = arms1[1].body.kind; if block.stmts.len() == 1; - if let StmtKind::Local(ref local2) = block.stmts[0].kind; - if let Some(ref init1) = local2.init; - if let ExprKind::Path(ref path9) = init1.kind; - if match_qpath(path9, &["y"]); - if let PatKind::Binding(BindingAnnotation::Unannotated, _, name2, None) = local2.pat.kind; - if name2.as_str() == "z"; + if let StmtKind::Local(ref local) = block.stmts[0].kind; + if let Some(ref init) = local.init; + if let ExprKind::Path(ref path5) = init.kind; + if match_qpath(path5, &["y"]); + if let PatKind::Binding(BindingAnnotation::Unannotated, _, name, None) = local.pat.kind; + if name.as_str() == "z"; if block.expr.is_none(); + if let PatKind::Struct(ref path6, ref fields2, false) = arms1[1].pat.kind; + if matches!(path6, QPath::LangItem(LangItem::OptionSome, _)); + if fields2.len() == 1; + // unimplemented: field checks if body.expr.is_none(); - if let PatKind::Binding(BindingAnnotation::Mutable, _, name3, None) = arms[0].pat.kind; - if name3.as_str() == "iter"; + if let PatKind::Binding(BindingAnnotation::Mutable, _, name1, None) = arms[0].pat.kind; + if name1.as_str() == "iter"; then { // report your lint here } From 8c1c763c2d5897c66f52bd5e7c16f48ae66c894c Mon Sep 17 00:00:00 2001 From: Cameron Steffen Date: Fri, 19 Nov 2021 12:23:52 -0600 Subject: [PATCH 3/4] clippy: Fix pattern_type_mismatch for loop --- clippy_lints/src/pattern_type_mismatch.rs | 207 +++++----------------- 1 file changed, 43 insertions(+), 164 deletions(-) diff --git a/clippy_lints/src/pattern_type_mismatch.rs b/clippy_lints/src/pattern_type_mismatch.rs index e7bc2446590..018e6d611db 100644 --- a/clippy_lints/src/pattern_type_mismatch.rs +++ b/clippy_lints/src/pattern_type_mismatch.rs @@ -1,16 +1,12 @@ use clippy_utils::diagnostics::span_lint_and_help; -use clippy_utils::last_path_segment; use rustc_hir::{ - intravisit, Body, Expr, ExprKind, FnDecl, HirId, LocalSource, MatchSource, Mutability, Pat, PatField, PatKind, - QPath, Stmt, StmtKind, + intravisit, Body, Expr, ExprKind, FnDecl, HirId, LocalSource, Mutability, Pat, PatKind, Stmt, StmtKind, }; use rustc_lint::{LateContext, LateLintPass, LintContext}; use rustc_middle::lint::in_external_macro; -use rustc_middle::ty::subst::SubstsRef; -use rustc_middle::ty::{AdtDef, FieldDef, Ty, TyKind, VariantDef}; +use rustc_middle::ty; use rustc_session::{declare_lint_pass, declare_tool_lint}; use rustc_span::source_map::Span; -use std::iter; declare_clippy_lint! { /// ### What it does @@ -87,43 +83,28 @@ declare_lint_pass!(PatternTypeMismatch => [PATTERN_TYPE_MISMATCH]); impl<'tcx> LateLintPass<'tcx> for PatternTypeMismatch { fn check_stmt(&mut self, cx: &LateContext<'tcx>, stmt: &'tcx Stmt<'_>) { if let StmtKind::Local(local) = stmt.kind { - if let Some(init) = &local.init { - if let Some(init_ty) = cx.typeck_results().node_type_opt(init.hir_id) { - let pat = &local.pat; - if in_external_macro(cx.sess(), pat.span) { - return; - } - let deref_possible = match local.source { - LocalSource::Normal => DerefPossible::Possible, - _ => DerefPossible::Impossible, - }; - apply_lint(cx, pat, init_ty, deref_possible); - } + if in_external_macro(cx.sess(), local.pat.span) { + return; } + let deref_possible = match local.source { + LocalSource::Normal => DerefPossible::Possible, + _ => DerefPossible::Impossible, + }; + apply_lint(cx, local.pat, deref_possible); } } fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { - if let ExprKind::Match(scrutinee, arms, MatchSource::Normal) = expr.kind { - if let Some(expr_ty) = cx.typeck_results().node_type_opt(scrutinee.hir_id) { - 'pattern_checks: for arm in arms { - let pat = &arm.pat; - if in_external_macro(cx.sess(), pat.span) { - continue 'pattern_checks; - } - if apply_lint(cx, pat, expr_ty, DerefPossible::Possible) { - break 'pattern_checks; - } + if let ExprKind::Match(_, arms, _) = expr.kind { + for arm in arms { + let pat = &arm.pat; + if apply_lint(cx, pat, DerefPossible::Possible) { + break; } } } - if let ExprKind::Let(let_pat, let_expr, _) = expr.kind { - if let Some(expr_ty) = cx.typeck_results().node_type_opt(let_expr.hir_id) { - if in_external_macro(cx.sess(), let_pat.span) { - return; - } - apply_lint(cx, let_pat, expr_ty, DerefPossible::Possible); - } + if let ExprKind::Let(let_pat, ..) = expr.kind { + apply_lint(cx, let_pat, DerefPossible::Possible); } } @@ -134,12 +115,10 @@ impl<'tcx> LateLintPass<'tcx> for PatternTypeMismatch { _: &'tcx FnDecl<'_>, body: &'tcx Body<'_>, _: Span, - hir_id: HirId, + _: HirId, ) { - if let Some(fn_sig) = cx.typeck_results().liberated_fn_sigs().get(hir_id) { - for (param, ty) in iter::zip(body.params, fn_sig.inputs()) { - apply_lint(cx, param.pat, ty, DerefPossible::Impossible); - } + for param in body.params { + apply_lint(cx, param.pat, DerefPossible::Impossible); } } } @@ -150,8 +129,8 @@ enum DerefPossible { Impossible, } -fn apply_lint<'tcx>(cx: &LateContext<'tcx>, pat: &Pat<'_>, expr_ty: Ty<'tcx>, deref_possible: DerefPossible) -> bool { - let maybe_mismatch = find_first_mismatch(cx, pat, expr_ty, Level::Top); +fn apply_lint<'tcx>(cx: &LateContext<'tcx>, pat: &Pat<'_>, deref_possible: DerefPossible) -> bool { + let maybe_mismatch = find_first_mismatch(cx, pat); if let Some((span, mutability, level)) = maybe_mismatch { span_lint_and_help( cx, @@ -184,132 +163,32 @@ enum Level { } #[allow(rustc::usage_of_ty_tykind)] -fn find_first_mismatch<'tcx>( - cx: &LateContext<'tcx>, - pat: &Pat<'_>, - ty: Ty<'tcx>, - level: Level, -) -> Option<(Span, Mutability, Level)> { - if let PatKind::Ref(sub_pat, _) = pat.kind { - if let TyKind::Ref(_, sub_ty, _) = ty.kind() { - return find_first_mismatch(cx, sub_pat, sub_ty, Level::Lower); +fn find_first_mismatch<'tcx>(cx: &LateContext<'tcx>, pat: &Pat<'_>) -> Option<(Span, Mutability, Level)> { + let mut result = None; + pat.walk(|p| { + if result.is_some() { + return false; } - } - - if let TyKind::Ref(_, _, mutability) = *ty.kind() { - if is_non_ref_pattern(&pat.kind) { - return Some((pat.span, mutability, level)); + if in_external_macro(cx.sess(), p.span) { + return true; } - } - - if let PatKind::Struct(ref qpath, field_pats, _) = pat.kind { - if let TyKind::Adt(adt_def, substs_ref) = ty.kind() { - if let Some(variant) = get_variant(adt_def, qpath) { - let field_defs = &variant.fields; - return find_first_mismatch_in_struct(cx, field_pats, field_defs, substs_ref); - } - } - } - - if let PatKind::TupleStruct(ref qpath, pats, _) = pat.kind { - if let TyKind::Adt(adt_def, substs_ref) = ty.kind() { - if let Some(variant) = get_variant(adt_def, qpath) { - let field_defs = &variant.fields; - let ty_iter = field_defs.iter().map(|field_def| field_def.ty(cx.tcx, substs_ref)); - return find_first_mismatch_in_tuple(cx, pats, ty_iter); - } - } - } - - if let PatKind::Tuple(pats, _) = pat.kind { - if let TyKind::Tuple(..) = ty.kind() { - return find_first_mismatch_in_tuple(cx, pats, ty.tuple_fields()); - } - } - - if let PatKind::Or(sub_pats) = pat.kind { - for pat in sub_pats { - let maybe_mismatch = find_first_mismatch(cx, pat, ty, level); - if let Some(mismatch) = maybe_mismatch { - return Some(mismatch); - } - } - } - - None -} - -fn get_variant<'a>(adt_def: &'a AdtDef, qpath: &QPath<'_>) -> Option<&'a VariantDef> { - if adt_def.is_struct() { - if let Some(variant) = adt_def.variants.iter().next() { - return Some(variant); - } - } - - if adt_def.is_enum() { - let pat_ident = last_path_segment(qpath).ident; - for variant in &adt_def.variants { - if variant.ident == pat_ident { - return Some(variant); - } - } - } - - None -} - -fn find_first_mismatch_in_tuple<'tcx, I>( - cx: &LateContext<'tcx>, - pats: &[Pat<'_>], - ty_iter_src: I, -) -> Option<(Span, Mutability, Level)> -where - I: IntoIterator>, -{ - let mut field_tys = ty_iter_src.into_iter(); - 'fields: for pat in pats { - let field_ty = if let Some(ty) = field_tys.next() { - ty - } else { - break 'fields; + let adjust_pat = match p.kind { + PatKind::Or([p, ..]) => p, + _ => p, }; - - let maybe_mismatch = find_first_mismatch(cx, pat, field_ty, Level::Lower); - if let Some(mismatch) = maybe_mismatch { - return Some(mismatch); - } - } - - None -} - -fn find_first_mismatch_in_struct<'tcx>( - cx: &LateContext<'tcx>, - field_pats: &[PatField<'_>], - field_defs: &[FieldDef], - substs_ref: SubstsRef<'tcx>, -) -> Option<(Span, Mutability, Level)> { - for field_pat in field_pats { - 'definitions: for field_def in field_defs { - if field_pat.ident == field_def.ident { - let field_ty = field_def.ty(cx.tcx, substs_ref); - let pat = &field_pat.pat; - let maybe_mismatch = find_first_mismatch(cx, pat, field_ty, Level::Lower); - if let Some(mismatch) = maybe_mismatch { - return Some(mismatch); + if let Some(adjustments) = cx.typeck_results().pat_adjustments().get(adjust_pat.hir_id) { + if let [first, ..] = **adjustments { + if let ty::Ref(.., mutability) = *first.kind() { + let level = if p.hir_id == pat.hir_id { + Level::Top + } else { + Level::Lower + }; + result = Some((p.span, mutability, level)); } - break 'definitions; } } - } - - None -} - -fn is_non_ref_pattern(pat_kind: &PatKind<'_>) -> bool { - match pat_kind { - PatKind::Struct(..) | PatKind::Tuple(..) | PatKind::TupleStruct(..) | PatKind::Path(..) => true, - PatKind::Or(sub_pats) => sub_pats.iter().any(|pat| is_non_ref_pattern(&pat.kind)), - _ => false, - } + result.is_none() + }); + result } From c46c8c58e948b48505bdef3aef67d4d609356b97 Mon Sep 17 00:00:00 2001 From: flip1995 Date: Tue, 23 Nov 2021 11:22:49 +0100 Subject: [PATCH 4/4] Bump nightly version -> 2021-11-23 --- rust-toolchain | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rust-toolchain b/rust-toolchain index d042b6057b0..f02d0b18ddc 100644 --- a/rust-toolchain +++ b/rust-toolchain @@ -1,3 +1,3 @@ [toolchain] -channel = "nightly-2021-11-18" +channel = "nightly-2021-11-23" components = ["cargo", "llvm-tools-preview", "rust-src", "rust-std", "rustc", "rustc-dev", "rustfmt"]