clippy_lints/src/loops/while_let_on_iterator.rs

   1 use super::WHILE_LET_ON_ITERATOR;
   2 use clippy_utils::diagnostics::span_lint_and_sugg;
   3 use clippy_utils::source::snippet_with_applicability;
   4 use clippy_utils::{get_enclosing_loop, is_refutable, is_trait_method, match_def_path, paths, visitors::is_res_used};
   5 use rustc_errors::Applicability;
   6 use rustc_hir::intravisit::{walk_expr, ErasedMap, NestedVisitorMap, Visitor};
   7 use rustc_hir::{def::Res, Expr, ExprKind, HirId, Local, MatchSource, Node, PatKind, QPath, UnOp};
   8 use rustc_lint::LateContext;
   9 use rustc_span::{symbol::sym, Span, Symbol};
  10
  11 pub(super) fn check(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
  12     if let ExprKind::Match(scrutinee_expr, [arm, _], MatchSource::WhileLetDesugar) = expr.kind {
  13         let some_pat = match arm.pat.kind {
  14             PatKind::TupleStruct(QPath::Resolved(None, path), sub_pats, _) => match path.res {
  15                 Res::Def(_, id) if match_def_path(cx, id, &paths::OPTION_SOME) => sub_pats.first(),
  16                 _ => return,
  17             },
  18             _ => return,
  19         };
  20
  21         let iter_expr = match scrutinee_expr.kind {
  22             ExprKind::MethodCall(name, _, [iter_expr], _)
  23                 if name.ident.name == sym::next && is_trait_method(cx, scrutinee_expr, sym::Iterator) =>
  24             {
  25                 if let Some(iter_expr) = try_parse_iter_expr(cx, iter_expr) {
  26                     iter_expr
  27                 } else {
  28                     return;
  29                 }
  30             }
  31             _ => return,
  32         };
  33
  34         // Needed to find an outer loop, if there are any.
  35         let loop_expr = if let Some((_, Node::Expr(e))) = cx.tcx.hir().parent_iter(expr.hir_id).nth(1) {
  36             e
  37         } else {
  38             return;
  39         };
  40
  41         // Refutable patterns don't work with for loops.
  42         // The iterator also can't be accessed withing the loop.
  43         if some_pat.map_or(true, |p| is_refutable(cx, p)) || uses_iter(cx, &iter_expr, arm.body) {
  44             return;
  45         }
  46
  47         // If the iterator is a field or the iterator is accessed after the loop is complete it needs to be
  48         // borrowed mutably.
  49         // TODO: If the struct can be partially moved from and the struct isn't used afterwards a mutable
  50         // borrow of a field isn't necessary.
  51         let ref_mut = if !iter_expr.fields.is_empty() || needs_mutable_borrow(cx, &iter_expr, loop_expr) {
  52             "&mut "
  53         } else {
  54             ""
  55         };
  56         let mut applicability = Applicability::MachineApplicable;
  57         let iterator = snippet_with_applicability(cx, iter_expr.span, "_", &mut applicability);
  58         let loop_var = some_pat.map_or_else(
  59             || "_".into(),
  60             |pat| snippet_with_applicability(cx, pat.span, "_", &mut applicability).into_owned(),
  61         );
  62         span_lint_and_sugg(
  63             cx,
  64             WHILE_LET_ON_ITERATOR,
  65             expr.span.with_hi(scrutinee_expr.span.hi()),
  66             "this loop could be written as a `for` loop",
  67             "try",
  68             format!("for {} in {}{}", loop_var, ref_mut, iterator),
  69             applicability,
  70         );
  71     }
  72 }
  73
  74 #[derive(Debug)]
  75 struct IterExpr {
  76     /// The span of the whole expression, not just the path and fields stored here.
  77     span: Span,
  78     /// The fields used, in order of child to parent.
  79     fields: Vec<Symbol>,
  80     /// The path being used.
  81     path: Res,
  82 }
  83 /// Parses any expression to find out which field of which variable is used. Will return `None` if
  84 /// the expression might have side effects.
  85 fn try_parse_iter_expr(cx: &LateContext<'_>, mut e: &Expr<'_>) -> Option<IterExpr> {
  86     let span = e.span;
  87     let mut fields = Vec::new();
  88     loop {
  89         match e.kind {
  90             ExprKind::Path(ref path) => {
  91                 break Some(IterExpr {
  92                     span,
  93                     fields,
  94                     path: cx.qpath_res(path, e.hir_id),
  95                 });
  96             },
  97             ExprKind::Field(base, name) => {
  98                 fields.push(name.name);
  99                 e = base;
 100             },
 101             // Dereferencing a pointer has no side effects and doesn't affect which field is being used.
 102             ExprKind::Unary(UnOp::Deref, base) if cx.typeck_results().expr_ty(base).is_ref() => e = base,
 103
 104             // Shouldn't have side effects, but there's no way to trace which field is used. So forget which fields have
 105             // already been seen.
 106             ExprKind::Index(base, idx) if !idx.can_have_side_effects() => {
 107                 fields.clear();
 108                 e = base;
 109             },
 110             ExprKind::Unary(UnOp::Deref, base) => {
 111                 fields.clear();
 112                 e = base;
 113             },
 114
 115             // No effect and doesn't affect which field is being used.
 116             ExprKind::DropTemps(base) | ExprKind::AddrOf(_, _, base) | ExprKind::Type(base, _) => e = base,
 117             _ => break None,
 118         }
 119     }
 120 }
 121
 122 fn is_expr_same_field(cx: &LateContext<'_>, mut e: &Expr<'_>, mut fields: &[Symbol], path_res: Res) -> bool {
 123     loop {
 124         match (&e.kind, fields) {
 125             (&ExprKind::Field(base, name), [head_field, tail_fields @ ..]) if name.name == *head_field => {
 126                 e = base;
 127                 fields = tail_fields;
 128             },
 129             (ExprKind::Path(path), []) => {
 130                 break cx.qpath_res(path, e.hir_id) == path_res;
 131             },
 132             (&(ExprKind::DropTemps(base) | ExprKind::AddrOf(_, _, base) | ExprKind::Type(base, _)), _) => e = base,
 133             _ => break false,
 134         }
 135     }
 136 }
 137
 138 /// Checks if the given expression is the same field as, is a child of, of the parent of the given
 139 /// field. Used to check if the expression can be used while the given field is borrowed.
 140 fn is_expr_same_child_or_parent_field(cx: &LateContext<'_>, expr: &Expr<'_>, fields: &[Symbol], path_res: Res) -> bool {
 141     match expr.kind {
 142         ExprKind::Field(base, name) => {
 143             if let Some((head_field, tail_fields)) = fields.split_first() {
 144                 if name.name == *head_field && is_expr_same_field(cx, base, fields, path_res) {
 145                     return true;
 146                 }
 147                 // Check if the expression is a parent field
 148                 let mut fields_iter = tail_fields.iter();
 149                 while let Some(field) = fields_iter.next() {
 150                     if *field == name.name && is_expr_same_field(cx, base, fields_iter.as_slice(), path_res) {
 151                         return true;
 152                     }
 153                 }
 154             }
 155
 156             // Check if the expression is a child field.
 157             let mut e = base;
 158             loop {
 159                 match e.kind {
 160                     ExprKind::Field(..) if is_expr_same_field(cx, e, fields, path_res) => break true,
 161                     ExprKind::Field(base, _) | ExprKind::DropTemps(base) | ExprKind::Type(base, _) => e = base,
 162                     ExprKind::Path(ref path) if fields.is_empty() => {
 163                         break cx.qpath_res(path, e.hir_id) == path_res;
 164                     },
 165                     _ => break false,
 166                 }
 167             }
 168         },
 169         // If the path matches, this is either an exact match, of the expression is a parent of the field.
 170         ExprKind::Path(ref path) => cx.qpath_res(path, expr.hir_id) == path_res,
 171         ExprKind::DropTemps(base) | ExprKind::Type(base, _) | ExprKind::AddrOf(_, _, base) => {
 172             is_expr_same_child_or_parent_field(cx, base, fields, path_res)
 173         },
 174         _ => false,
 175     }
 176 }
 177
 178 /// Strips off all field and path expressions. Used to skip them after failing to check for
 179 /// equality.
 180 fn skip_fields_and_path(expr: &'tcx Expr<'_>) -> (Option<&'tcx Expr<'tcx>>, bool) {
 181     let mut e = expr;
 182     let e = loop {
 183         match e.kind {
 184             ExprKind::Field(base, _) | ExprKind::DropTemps(base) | ExprKind::Type(base, _) => e = base,
 185             ExprKind::Path(_) => return (None, true),
 186             _ => break e,
 187         }
 188     };
 189     (Some(e), e.hir_id != expr.hir_id)
 190 }
 191
 192 /// Checks if the given expression uses the iterator.
 193 fn uses_iter(cx: &LateContext<'tcx>, iter_expr: &IterExpr, container: &'tcx Expr<'_>) -> bool {
 194     struct V<'a, 'b, 'tcx> {
 195         cx: &'a LateContext<'tcx>,
 196         iter_expr: &'b IterExpr,
 197         uses_iter: bool,
 198     }
 199     impl Visitor<'tcx> for V<'_, '_, 'tcx> {
 200         type Map = ErasedMap<'tcx>;
 201         fn nested_visit_map(&mut self) -> NestedVisitorMap<Self::Map> {
 202             NestedVisitorMap::None
 203         }
 204
 205         fn visit_expr(&mut self, e: &'tcx Expr<'_>) {
 206             if self.uses_iter {
 207                 // return
 208             } else if is_expr_same_child_or_parent_field(self.cx, e, &self.iter_expr.fields, self.iter_expr.path) {
 209                 self.uses_iter = true;
 210             } else if let (e, true) = skip_fields_and_path(e) {
 211                 if let Some(e) = e {
 212                     self.visit_expr(e);
 213                 }
 214             } else if let ExprKind::Closure(_, _, id, _, _) = e.kind {
 215                 if is_res_used(self.cx, self.iter_expr.path, id) {
 216                     self.uses_iter = true;
 217                 }
 218             } else {
 219                 walk_expr(self, e);
 220             }
 221         }
 222     }
 223
 224     let mut v = V {
 225         cx,
 226         iter_expr,
 227         uses_iter: false,
 228     };
 229     v.visit_expr(container);
 230     v.uses_iter
 231 }
 232
 233 #[allow(clippy::too_many_lines)]
 234 fn needs_mutable_borrow(cx: &LateContext<'tcx>, iter_expr: &IterExpr, loop_expr: &'tcx Expr<'_>) -> bool {
 235     struct AfterLoopVisitor<'a, 'b, 'tcx> {
 236         cx: &'a LateContext<'tcx>,
 237         iter_expr: &'b IterExpr,
 238         loop_id: HirId,
 239         after_loop: bool,
 240         used_iter: bool,
 241     }
 242     impl Visitor<'tcx> for AfterLoopVisitor<'_, '_, 'tcx> {
 243         type Map = ErasedMap<'tcx>;
 244         fn nested_visit_map(&mut self) -> NestedVisitorMap<Self::Map> {
 245             NestedVisitorMap::None
 246         }
 247
 248         fn visit_expr(&mut self, e: &'tcx Expr<'_>) {
 249             if self.used_iter {
 250                 return;
 251             }
 252             if self.after_loop {
 253                 if is_expr_same_child_or_parent_field(self.cx, e, &self.iter_expr.fields, self.iter_expr.path) {
 254                     self.used_iter = true;
 255                 } else if let (e, true) = skip_fields_and_path(e) {
 256                     if let Some(e) = e {
 257                         self.visit_expr(e);
 258                     }
 259                 } else if let ExprKind::Closure(_, _, id, _, _) = e.kind {
 260                     self.used_iter |= is_res_used(self.cx, self.iter_expr.path, id);
 261                 } else {
 262                     walk_expr(self, e);
 263                 }
 264             } else if self.loop_id == e.hir_id {
 265                 self.after_loop = true;
 266             } else {
 267                 walk_expr(self, e);
 268             }
 269         }
 270     }
 271
 272     struct NestedLoopVisitor<'a, 'b, 'tcx> {
 273         cx: &'a LateContext<'tcx>,
 274         iter_expr: &'b IterExpr,
 275         local_id: HirId,
 276         loop_id: HirId,
 277         after_loop: bool,
 278         found_local: bool,
 279         used_after: bool,
 280     }
 281     impl Visitor<'tcx> for NestedLoopVisitor<'a, 'b, 'tcx> {
 282         type Map = ErasedMap<'tcx>;
 283         fn nested_visit_map(&mut self) -> NestedVisitorMap<Self::Map> {
 284             NestedVisitorMap::None
 285         }
 286
 287         fn visit_local(&mut self, l: &'tcx Local<'_>) {
 288             if !self.after_loop {
 289                 l.pat.each_binding_or_first(&mut |_, id, _, _| {
 290                     if id == self.local_id {
 291                         self.found_local = true;
 292                     }
 293                 });
 294             }
 295             if let Some(e) = l.init {
 296                 self.visit_expr(e);
 297             }
 298         }
 299
 300         fn visit_expr(&mut self, e: &'tcx Expr<'_>) {
 301             if self.used_after {
 302                 return;
 303             }
 304             if self.after_loop {
 305                 if is_expr_same_child_or_parent_field(self.cx, e, &self.iter_expr.fields, self.iter_expr.path) {
 306                     self.used_after = true;
 307                 } else if let (e, true) = skip_fields_and_path(e) {
 308                     if let Some(e) = e {
 309                         self.visit_expr(e);
 310                     }
 311                 } else if let ExprKind::Closure(_, _, id, _, _) = e.kind {
 312                     self.used_after |= is_res_used(self.cx, self.iter_expr.path, id);
 313                 } else {
 314                     walk_expr(self, e);
 315                 }
 316             } else if e.hir_id == self.loop_id {
 317                 self.after_loop = true;
 318             } else {
 319                 walk_expr(self, e);
 320             }
 321         }
 322     }
 323
 324     if let Some(e) = get_enclosing_loop(cx.tcx, loop_expr) {
 325         // The iterator expression will be used on the next iteration unless it is declared within the outer
 326         // loop.
 327         let local_id = match iter_expr.path {
 328             Res::Local(id) => id,
 329             _ => return true,
 330         };
 331         let mut v = NestedLoopVisitor {
 332             cx,
 333             iter_expr,
 334             local_id,
 335             loop_id: loop_expr.hir_id,
 336             after_loop: false,
 337             found_local: false,
 338             used_after: false,
 339         };
 340         v.visit_expr(e);
 341         v.used_after || !v.found_local
 342     } else {
 343         let mut v = AfterLoopVisitor {
 344             cx,
 345             iter_expr,
 346             loop_id: loop_expr.hir_id,
 347             after_loop: false,
 348             used_iter: false,
 349         };
 350         v.visit_expr(&cx.tcx.hir().body(cx.enclosing_body.unwrap()).value);
 351         v.used_iter
 352     }
 353 }