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, is_refutable, is_trait_method, match_def_path, paths, visitors::is_res_used};
+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, 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;
}
// 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) {
- "&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 {
""
};
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 fields used, in order of child to parent.
fields: Vec<Symbol>,
/// 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<IterExpr> {
- let span = e.span;
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,
fields,
path: cx.qpath_res(path, e.hir_id),
+ can_move,
});
},
ExprKind::Field(base, name) => {
// Shouldn't have side effects, but there's no way to trace which field is used. So forget which fields have
// already been seen.
ExprKind::Index(base, idx) if !idx.can_have_side_effects() => {
+ can_move = false;
fields.clear();
e = base;
},
ExprKind::Unary(UnOp::Deref, base) => {
+ can_move = false;
fields.clear();
e = base;
},
match expr.kind {
ExprKind::Field(base, name) => {
if let Some((head_field, tail_fields)) = fields.split_first() {
- if name.name == *head_field && is_expr_same_field(cx, base, fields, path_res) {
+ if name.name == *head_field && is_expr_same_field(cx, base, tail_fields, path_res) {
return true;
}
// Check if the expression is a parent field
/// 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 {
}
/// 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<Self::Map> {
- NestedVisitorMap::None
- }
-
+ impl<'tcx> Visitor<'tcx> for V<'_, '_, 'tcx> {
fn visit_expr(&mut self, e: &'tcx Expr<'_>) {
if self.uses_iter {
// return
}
#[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,
after_loop: bool,
used_iter: bool,
}
- impl Visitor<'tcx> for AfterLoopVisitor<'_, '_, 'tcx> {
- type Map = ErasedMap<'tcx>;
- fn nested_visit_map(&mut self) -> NestedVisitorMap<Self::Map> {
- NestedVisitorMap::None
- }
-
+ impl<'tcx> Visitor<'tcx> for AfterLoopVisitor<'_, '_, 'tcx> {
fn visit_expr(&mut self, e: &'tcx Expr<'_>) {
if self.used_iter {
return;
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<Self::Map> {
- 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, _, _| {
}
}
- if let Some(e) = get_enclosing_loop(cx.tcx, loop_expr) {
- // The iterator expression will be used on the next iteration unless it is declared within the outer
- // loop.
+ if let Some(e) = get_enclosing_loop_or_closure(cx.tcx, loop_expr) {
+ // The iterator expression will be used on the next iteration (for loops), or on the next call (for
+ // closures) unless it is declared within the enclosing expression. TODO: Check for closures
+ // used where an `FnOnce` type is expected.
let local_id = match iter_expr.path {
Res::Local(id) => id,
_ => return true,
projects / rust.git / blobdiff