diff --git a/clippy_lints/src/loops/for_kv_map.rs b/clippy_lints/src/loops/for_kv_map.rs index 7fb8e51377a2..d28029790732 100644 --- a/clippy_lints/src/loops/for_kv_map.rs +++ b/clippy_lints/src/loops/for_kv_map.rs @@ -23,7 +23,20 @@ pub(super) fn check<'tcx>( && pat.len() == 2 { let arg_span = arg.span; - let (new_pat_span, kind, ty, mutbl) = match *cx.typeck_results().expr_ty(arg).kind() { + let (arg, arg_ty) = match arg.kind { + // `for x in &expr` or `for x in &mut expr` + ExprKind::AddrOf(BorrowKind::Ref, _, expr) => (expr, cx.typeck_results().expr_ty(arg)), + // `for x in receiver.iter()` or `for x in receiver.iter_mut()` + ExprKind::MethodCall(path, receiver, [], ..) + if path.ident.name == sym::iter || path.ident.name == sym::iter_mut => + { + // Use `expr_ty_adjusted` because `.iter()` / `.iter_mut()` may introduce auto deferences + (receiver, cx.typeck_results().expr_ty_adjusted(receiver)) + }, + _ => (arg, cx.typeck_results().expr_ty(arg)), + }; + + let (new_pat_span, kind, ty, mutbl) = match *arg_ty.kind() { ty::Ref(_, ty, mutbl) => match (&pat[0].kind, &pat[1].kind) { (key, _) if pat_is_wild(cx, key, body) => (pat[1].span, "value", ty, mutbl), (_, value) if pat_is_wild(cx, value, body) => (pat[0].span, "key", ty, Mutability::Not), @@ -35,10 +48,6 @@ pub(super) fn check<'tcx>( Mutability::Not => "", Mutability::Mut => "_mut", }; - let arg = match arg.kind { - ExprKind::AddrOf(BorrowKind::Ref, _, expr) => expr, - _ => arg, - }; if matches!(ty.opt_diag_name(cx), Some(sym::HashMap | sym::BTreeMap)) && let Some(arg_span) = walk_span_to_context(arg_span, span.ctxt()) diff --git a/tests/ui/for_kv_map.fixed b/tests/ui/for_kv_map.fixed index 6ec4cb01ffd1..2fd6d9ca499d 100644 --- a/tests/ui/for_kv_map.fixed +++ b/tests/ui/for_kv_map.fixed @@ -86,3 +86,25 @@ fn wrongly_unmangled_macros() { let _v = v; } } + +fn issue16822(mut x: HashMap) { + for v in x.values() { + //~^ for_kv_map + println!("{}", v); + } + + for v in x.values_mut() { + //~^ for_kv_map + *v += 1; + } + + for k in x.keys() { + //~^ for_kv_map + println!("{}", k); + } + + for k in x.keys() { + //~^ for_kv_map + println!("{}", k); + } +} diff --git a/tests/ui/for_kv_map.rs b/tests/ui/for_kv_map.rs index 19e907ff10a6..12e16d1dbfd6 100644 --- a/tests/ui/for_kv_map.rs +++ b/tests/ui/for_kv_map.rs @@ -86,3 +86,25 @@ macro_rules! test_map { let _v = v; } } + +fn issue16822(mut x: HashMap) { + for (_, v) in x.iter() { + //~^ for_kv_map + println!("{}", v); + } + + for (_, v) in x.iter_mut() { + //~^ for_kv_map + *v += 1; + } + + for (k, _) in x.iter() { + //~^ for_kv_map + println!("{}", k); + } + + for (k, _) in x.iter_mut() { + //~^ for_kv_map + println!("{}", k); + } +} diff --git a/tests/ui/for_kv_map.stderr b/tests/ui/for_kv_map.stderr index 5436592f2ab6..fac0407e1003 100644 --- a/tests/ui/for_kv_map.stderr +++ b/tests/ui/for_kv_map.stderr @@ -84,5 +84,53 @@ LL - for (_, v) in test_map!(wrapped) { LL + for v in test_map!(wrapped).values() { | -error: aborting due to 7 previous errors +error: you seem to want to iterate on a map's values + --> tests/ui/for_kv_map.rs:91:19 + | +LL | for (_, v) in x.iter() { + | ^^^^^^^^ + | +help: use the corresponding method + | +LL - for (_, v) in x.iter() { +LL + for v in x.values() { + | + +error: you seem to want to iterate on a map's values + --> tests/ui/for_kv_map.rs:96:19 + | +LL | for (_, v) in x.iter_mut() { + | ^^^^^^^^^^^^ + | +help: use the corresponding method + | +LL - for (_, v) in x.iter_mut() { +LL + for v in x.values_mut() { + | + +error: you seem to want to iterate on a map's keys + --> tests/ui/for_kv_map.rs:101:19 + | +LL | for (k, _) in x.iter() { + | ^^^^^^^^ + | +help: use the corresponding method + | +LL - for (k, _) in x.iter() { +LL + for k in x.keys() { + | + +error: you seem to want to iterate on a map's keys + --> tests/ui/for_kv_map.rs:106:19 + | +LL | for (k, _) in x.iter_mut() { + | ^^^^^^^^^^^^ + | +help: use the corresponding method + | +LL - for (k, _) in x.iter_mut() { +LL + for k in x.keys() { + | + +error: aborting due to 11 previous errors