X-Git-Url: https://git.lizzy.rs/?a=blobdiff_plain;f=clippy_lints%2Fsrc%2Floops%2Fwhile_let_on_iterator.rs;h=20a8294a0d1acf0132adf4222149a3baec66ddf4;hb=d1b087fdee1024e11498e2d6c3e36d31e9371d3b;hp=ef822e0cbe540225f9ad4317ecb19110973a70a0;hpb=1ad54642008b185f5691e5cac1148147d29fc00f;p=rust.git diff --git a/clippy_lints/src/loops/while_let_on_iterator.rs b/clippy_lints/src/loops/while_let_on_iterator.rs index ef822e0cbe5..20a8294a0d1 100644 --- a/clippy_lints/src/loops/while_let_on_iterator.rs +++ b/clippy_lints/src/loops/while_let_on_iterator.rs @@ -1,33 +1,34 @@ use super::WHILE_LET_ON_ITERATOR; use clippy_utils::diagnostics::span_lint_and_sugg; +use clippy_utils::higher; use clippy_utils::source::snippet_with_applicability; use clippy_utils::{ get_enclosing_loop_or_closure, is_refutable, is_trait_method, match_def_path, paths, visitors::is_res_used, }; use if_chain::if_chain; use rustc_errors::Applicability; -use rustc_hir::intravisit::{walk_expr, ErasedMap, NestedVisitorMap, Visitor}; -use rustc_hir::{def::Res, Expr, ExprKind, HirId, Local, MatchSource, Mutability, Node, PatKind, QPath, UnOp}; +use rustc_hir::intravisit::{walk_expr, Visitor}; +use rustc_hir::{def::Res, Expr, ExprKind, HirId, Local, Mutability, PatKind, QPath, UnOp}; use rustc_lint::LateContext; -use rustc_span::{symbol::sym, Span, Symbol}; +use rustc_middle::ty::adjustment::Adjust; +use rustc_span::{symbol::sym, Symbol}; -pub(super) fn check(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { - let (scrutinee_expr, iter_expr, some_pat, loop_expr) = if_chain! { - if let ExprKind::Match(scrutinee_expr, [arm, _], MatchSource::WhileLetDesugar) = expr.kind; +pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { + let (scrutinee_expr, iter_expr_struct, iter_expr, some_pat, loop_expr) = if_chain! { + if let Some(higher::WhileLet { if_then, let_pat, let_expr }) = higher::WhileLet::hir(expr); // check for `Some(..)` pattern - if let PatKind::TupleStruct(QPath::Resolved(None, pat_path), some_pat, _) = arm.pat.kind; + if let PatKind::TupleStruct(QPath::Resolved(None, pat_path), some_pat, _) = let_pat.kind; if let Res::Def(_, pat_did) = pat_path.res; if match_def_path(cx, pat_did, &paths::OPTION_SOME); // check for call to `Iterator::next` - if let ExprKind::MethodCall(method_name, _, [iter_expr], _) = scrutinee_expr.kind; + if let ExprKind::MethodCall(method_name, [iter_expr], _) = let_expr.kind; if method_name.ident.name == sym::next; - if is_trait_method(cx, scrutinee_expr, sym::Iterator); - if let Some(iter_expr) = try_parse_iter_expr(cx, iter_expr); + if is_trait_method(cx, let_expr, sym::Iterator); + if let Some(iter_expr_struct) = try_parse_iter_expr(cx, iter_expr); // get the loop containing the match expression - if let Some((_, Node::Expr(loop_expr))) = cx.tcx.hir().parent_iter(expr.hir_id).nth(1); - if !uses_iter(cx, &iter_expr, arm.body); + if !uses_iter(cx, &iter_expr_struct, if_then); then { - (scrutinee_expr, iter_expr, some_pat, loop_expr) + (let_expr, iter_expr_struct, iter_expr, some_pat, expr) } else { return; } @@ -47,13 +48,12 @@ pub(super) fn check(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { // If the iterator is a field or the iterator is accessed after the loop is complete it needs to be // borrowed mutably. TODO: If the struct can be partially moved from and the struct isn't used // afterwards a mutable borrow of a field isn't necessary. - let ref_mut = if !iter_expr.fields.is_empty() || needs_mutable_borrow(cx, &iter_expr, loop_expr) { - if cx.typeck_results().node_type(iter_expr.hir_id).ref_mutability() == Some(Mutability::Mut) { - // Reborrow for mutable references. It may not be possible to get a mutable reference here. - "&mut *" - } else { - "&mut " - } + let by_ref = if cx.typeck_results().expr_ty(iter_expr).ref_mutability() == Some(Mutability::Mut) + || !iter_expr_struct.can_move + || !iter_expr_struct.fields.is_empty() + || needs_mutable_borrow(cx, &iter_expr_struct, loop_expr) + { + ".by_ref()" } else { "" }; @@ -65,36 +65,43 @@ pub(super) fn check(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { expr.span.with_hi(scrutinee_expr.span.hi()), "this loop could be written as a `for` loop", "try", - format!("for {} in {}{}", loop_var, ref_mut, iterator), + format!("for {} in {}{}", loop_var, iterator, by_ref), applicability, ); } #[derive(Debug)] struct IterExpr { - /// The span of the whole expression, not just the path and fields stored here. - span: Span, - /// The HIR id of the whole expression, not just the path and fields stored here. - hir_id: HirId, /// The fields used, in order of child to parent. fields: Vec, /// The path being used. path: Res, + /// Whether or not the iterator can be moved. + can_move: bool, } + /// Parses any expression to find out which field of which variable is used. Will return `None` if /// the expression might have side effects. fn try_parse_iter_expr(cx: &LateContext<'_>, mut e: &Expr<'_>) -> Option { - let span = e.span; - let hir_id = e.hir_id; let mut fields = Vec::new(); + let mut can_move = true; loop { + if cx + .typeck_results() + .expr_adjustments(e) + .iter() + .any(|a| matches!(a.kind, Adjust::Deref(Some(..)))) + { + // Custom deref impls need to borrow the whole value as it's captured by reference + can_move = false; + fields.clear(); + } match e.kind { ExprKind::Path(ref path) => { break Some(IterExpr { - span, - hir_id, fields, path: cx.qpath_res(path, e.hir_id), + can_move, }); }, ExprKind::Field(base, name) => { @@ -107,10 +114,12 @@ fn try_parse_iter_expr(cx: &LateContext<'_>, mut e: &Expr<'_>) -> Option { + can_move = false; fields.clear(); e = base; }, ExprKind::Unary(UnOp::Deref, base) => { + can_move = false; fields.clear(); e = base; }, @@ -182,7 +191,7 @@ fn is_expr_same_child_or_parent_field(cx: &LateContext<'_>, expr: &Expr<'_>, fie /// Strips off all field and path expressions. This will return true if a field or path has been /// skipped. Used to skip them after failing to check for equality. -fn skip_fields_and_path(expr: &'tcx Expr<'_>) -> (Option<&'tcx Expr<'tcx>>, bool) { +fn skip_fields_and_path<'tcx>(expr: &'tcx Expr<'_>) -> (Option<&'tcx Expr<'tcx>>, bool) { let mut e = expr; let e = loop { match e.kind { @@ -195,18 +204,13 @@ fn skip_fields_and_path(expr: &'tcx Expr<'_>) -> (Option<&'tcx Expr<'tcx>>, bool } /// Checks if the given expression uses the iterator. -fn uses_iter(cx: &LateContext<'tcx>, iter_expr: &IterExpr, container: &'tcx Expr<'_>) -> bool { +fn uses_iter<'tcx>(cx: &LateContext<'tcx>, iter_expr: &IterExpr, container: &'tcx Expr<'_>) -> bool { struct V<'a, 'b, 'tcx> { cx: &'a LateContext<'tcx>, iter_expr: &'b IterExpr, uses_iter: bool, } - impl Visitor<'tcx> for V<'_, '_, 'tcx> { - type Map = ErasedMap<'tcx>; - fn nested_visit_map(&mut self) -> NestedVisitorMap { - NestedVisitorMap::None - } - + impl<'tcx> Visitor<'tcx> for V<'_, '_, 'tcx> { fn visit_expr(&mut self, e: &'tcx Expr<'_>) { if self.uses_iter { // return @@ -236,7 +240,7 @@ fn visit_expr(&mut self, e: &'tcx Expr<'_>) { } #[allow(clippy::too_many_lines)] -fn needs_mutable_borrow(cx: &LateContext<'tcx>, iter_expr: &IterExpr, loop_expr: &'tcx Expr<'_>) -> bool { +fn needs_mutable_borrow(cx: &LateContext<'_>, iter_expr: &IterExpr, loop_expr: &Expr<'_>) -> bool { struct AfterLoopVisitor<'a, 'b, 'tcx> { cx: &'a LateContext<'tcx>, iter_expr: &'b IterExpr, @@ -244,12 +248,7 @@ struct AfterLoopVisitor<'a, 'b, 'tcx> { after_loop: bool, used_iter: bool, } - impl Visitor<'tcx> for AfterLoopVisitor<'_, '_, 'tcx> { - type Map = ErasedMap<'tcx>; - fn nested_visit_map(&mut self) -> NestedVisitorMap { - NestedVisitorMap::None - } - + impl<'tcx> Visitor<'tcx> for AfterLoopVisitor<'_, '_, 'tcx> { fn visit_expr(&mut self, e: &'tcx Expr<'_>) { if self.used_iter { return; @@ -283,12 +282,7 @@ struct NestedLoopVisitor<'a, 'b, 'tcx> { found_local: bool, used_after: bool, } - impl Visitor<'tcx> for NestedLoopVisitor<'a, 'b, 'tcx> { - type Map = ErasedMap<'tcx>; - fn nested_visit_map(&mut self) -> NestedVisitorMap { - NestedVisitorMap::None - } - + impl<'a, 'b, 'tcx> Visitor<'tcx> for NestedLoopVisitor<'a, 'b, 'tcx> { fn visit_local(&mut self, l: &'tcx Local<'_>) { if !self.after_loop { l.pat.each_binding_or_first(&mut |_, id, _, _| {