Skip to content

Commit 7b0f588

Browse files
nahuakangY-Nak
authored andcommitted
Refactor check_for_loop_over_map_kv to its own module
1 parent 408368a commit 7b0f588

File tree

2 files changed

+71
-65
lines changed

2 files changed

+71
-65
lines changed
Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,69 @@
1+
use crate::utils::visitors::LocalUsedVisitor;
2+
use crate::utils::{is_type_diagnostic_item, match_type, multispan_sugg, paths, snippet, span_lint_and_then, sugg};
3+
use rustc_hir::{BorrowKind, Expr, ExprKind, Mutability, Pat, PatKind};
4+
use rustc_lint::LateContext;
5+
use rustc_middle::ty;
6+
7+
/// Checks for the `FOR_KV_MAP` lint.
8+
pub(super) fn check_for_loop_over_map_kv<'tcx>(
9+
cx: &LateContext<'tcx>,
10+
pat: &'tcx Pat<'_>,
11+
arg: &'tcx Expr<'_>,
12+
body: &'tcx Expr<'_>,
13+
expr: &'tcx Expr<'_>,
14+
) {
15+
let pat_span = pat.span;
16+
17+
if let PatKind::Tuple(ref pat, _) = pat.kind {
18+
if pat.len() == 2 {
19+
let arg_span = arg.span;
20+
let (new_pat_span, kind, ty, mutbl) = match *cx.typeck_results().expr_ty(arg).kind() {
21+
ty::Ref(_, ty, mutbl) => match (&pat[0].kind, &pat[1].kind) {
22+
(key, _) if pat_is_wild(key, body) => (pat[1].span, "value", ty, mutbl),
23+
(_, value) if pat_is_wild(value, body) => (pat[0].span, "key", ty, Mutability::Not),
24+
_ => return,
25+
},
26+
_ => return,
27+
};
28+
let mutbl = match mutbl {
29+
Mutability::Not => "",
30+
Mutability::Mut => "_mut",
31+
};
32+
let arg = match arg.kind {
33+
ExprKind::AddrOf(BorrowKind::Ref, _, ref expr) => &**expr,
34+
_ => arg,
35+
};
36+
37+
if is_type_diagnostic_item(cx, ty, sym!(hashmap_type)) || match_type(cx, ty, &paths::BTREEMAP) {
38+
span_lint_and_then(
39+
cx,
40+
super::FOR_KV_MAP,
41+
expr.span,
42+
&format!("you seem to want to iterate on a map's {}s", kind),
43+
|diag| {
44+
let map = sugg::Sugg::hir(cx, arg, "map");
45+
multispan_sugg(
46+
diag,
47+
"use the corresponding method",
48+
vec![
49+
(pat_span, snippet(cx, new_pat_span, kind).into_owned()),
50+
(arg_span, format!("{}.{}s{}()", map.maybe_par(), kind, mutbl)),
51+
],
52+
);
53+
},
54+
);
55+
}
56+
}
57+
}
58+
}
59+
60+
/// Returns `true` if the pattern is a `PatWild` or an ident prefixed with `_`.
61+
fn pat_is_wild<'tcx>(pat: &'tcx PatKind<'_>, body: &'tcx Expr<'_>) -> bool {
62+
match *pat {
63+
PatKind::Wild => true,
64+
PatKind::Binding(_, id, ident, None) if ident.as_str().starts_with('_') => {
65+
!LocalUsedVisitor::new(id).check_expr(body)
66+
},
67+
_ => false,
68+
}
69+
}

clippy_lints/src/loops/mod.rs

Lines changed: 2 additions & 65 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
mod for_loop_arg;
2+
mod for_loop_over_map_kv;
23
mod manual_flatten;
34
mod utils;
45

@@ -863,7 +864,7 @@ fn check_for_loop<'tcx>(
863864
check_for_loop_explicit_counter(cx, pat, arg, body, expr);
864865
}
865866
for_loop_arg::check_for_loop_arg(cx, pat, arg, expr);
866-
check_for_loop_over_map_kv(cx, pat, arg, body, expr);
867+
for_loop_over_map_kv::check_for_loop_over_map_kv(cx, pat, arg, body, expr);
867868
check_for_mut_range_bound(cx, arg, body);
868869
check_for_single_element_loop(cx, pat, arg, body, expr);
869870
detect_same_item_push(cx, pat, arg, body, expr);
@@ -1732,59 +1733,6 @@ fn check_for_loop_explicit_counter<'tcx>(
17321733
}
17331734
}
17341735

1735-
/// Checks for the `FOR_KV_MAP` lint.
1736-
fn check_for_loop_over_map_kv<'tcx>(
1737-
cx: &LateContext<'tcx>,
1738-
pat: &'tcx Pat<'_>,
1739-
arg: &'tcx Expr<'_>,
1740-
body: &'tcx Expr<'_>,
1741-
expr: &'tcx Expr<'_>,
1742-
) {
1743-
let pat_span = pat.span;
1744-
1745-
if let PatKind::Tuple(ref pat, _) = pat.kind {
1746-
if pat.len() == 2 {
1747-
let arg_span = arg.span;
1748-
let (new_pat_span, kind, ty, mutbl) = match *cx.typeck_results().expr_ty(arg).kind() {
1749-
ty::Ref(_, ty, mutbl) => match (&pat[0].kind, &pat[1].kind) {
1750-
(key, _) if pat_is_wild(cx, key, body) => (pat[1].span, "value", ty, mutbl),
1751-
(_, value) if pat_is_wild(cx, value, body) => (pat[0].span, "key", ty, Mutability::Not),
1752-
_ => return,
1753-
},
1754-
_ => return,
1755-
};
1756-
let mutbl = match mutbl {
1757-
Mutability::Not => "",
1758-
Mutability::Mut => "_mut",
1759-
};
1760-
let arg = match arg.kind {
1761-
ExprKind::AddrOf(BorrowKind::Ref, _, ref expr) => &**expr,
1762-
_ => arg,
1763-
};
1764-
1765-
if is_type_diagnostic_item(cx, ty, sym!(hashmap_type)) || match_type(cx, ty, &paths::BTREEMAP) {
1766-
span_lint_and_then(
1767-
cx,
1768-
FOR_KV_MAP,
1769-
expr.span,
1770-
&format!("you seem to want to iterate on a map's {}s", kind),
1771-
|diag| {
1772-
let map = sugg::Sugg::hir(cx, arg, "map");
1773-
multispan_sugg(
1774-
diag,
1775-
"use the corresponding method",
1776-
vec![
1777-
(pat_span, snippet(cx, new_pat_span, kind).into_owned()),
1778-
(arg_span, format!("{}.{}s{}()", map.maybe_par(), kind, mutbl)),
1779-
],
1780-
);
1781-
},
1782-
);
1783-
}
1784-
}
1785-
}
1786-
}
1787-
17881736
fn check_for_single_element_loop<'tcx>(
17891737
cx: &LateContext<'tcx>,
17901738
pat: &'tcx Pat<'_>,
@@ -1927,17 +1875,6 @@ fn check_for_mutation<'tcx>(
19271875
delegate.mutation_span()
19281876
}
19291877

1930-
/// Returns `true` if the pattern is a `PatWild` or an ident prefixed with `_`.
1931-
fn pat_is_wild<'tcx>(cx: &LateContext<'tcx>, pat: &'tcx PatKind<'_>, body: &'tcx Expr<'_>) -> bool {
1932-
match *pat {
1933-
PatKind::Wild => true,
1934-
PatKind::Binding(_, id, ident, None) if ident.as_str().starts_with('_') => {
1935-
!LocalUsedVisitor::new(cx, id).check_expr(body)
1936-
},
1937-
_ => false,
1938-
}
1939-
}
1940-
19411878
struct VarVisitor<'a, 'tcx> {
19421879
/// context reference
19431880
cx: &'a LateContext<'tcx>,

0 commit comments

Comments
 (0)