Skip to content

Commit 5f5ca79

Browse files
kennytmkennytm
committed
Added lints into_iter_on_ref and into_iter_on_array. Fix #1565.
1 parent b1d0343 commit 5f5ca79

File tree

4 files changed

+339
-2
lines changed

4 files changed

+339
-2
lines changed

‎clippy_lints/src/methods/mod.rs

+116-1
Original file line numberDiff line numberDiff line change
@@ -738,6 +738,51 @@ declare_clippy_lint! {
738738
"using `filter_map` when a more succinct alternative exists"
739739
}
740740

741+
/// **What it does:** Checks for `into_iter` calls on types which should be replaced by `iter` or
742+
/// `iter_mut`.
743+
///
744+
/// **Why is this bad?** Arrays and `PathBuf` do not yet have an `into_iter` method which move out
745+
/// their content into an iterator. Calling `into_iter` instead just forwards to `iter` or
746+
/// `iter_mut` due to auto-referencing, of which only yield references. Furthermore, when the
747+
/// standard library actually [implements the `into_iter` method][25725] which moves the content out
748+
/// of the array, the original use of `into_iter` got inferred with the wrong type and the code will
749+
/// be broken.
750+
///
751+
/// **Known problems:** None
752+
///
753+
/// **Example:**
754+
///
755+
/// ```rust
756+
/// let _ = [1, 2, 3].into_iter().map(|x| *x).collect::<Vec<u32>>();
757+
/// ```
758+
///
759+
/// [25725]: https://github.com/rust-lang/rust/issues/25725
760+
declare_clippy_lint! {
761+
pub INTO_ITER_ON_ARRAY,
762+
correctness,
763+
"using `.into_iter()` on an array"
764+
}
765+
766+
/// **What it does:** Checks for `into_iter` calls on references which should be replaced by `iter`
767+
/// or `iter_mut`.
768+
///
769+
/// **Why is this bad?** Readability. Calling `into_iter` on a reference will not move out its
770+
/// content into the resulting iterator, which is confusing. It is better just call `iter` or
771+
/// `iter_mut` directly.
772+
///
773+
/// **Known problems:** None
774+
///
775+
/// **Example:**
776+
///
777+
/// ```rust
778+
/// let _ = (&vec![3, 4, 5]).into_iter();
779+
/// ```
780+
declare_clippy_lint! {
781+
pub INTO_ITER_ON_REF,
782+
style,
783+
"using `.into_iter()` on a reference"
784+
}
785+
741786
impl LintPass for Pass {
742787
fn get_lints(&self) -> LintArray {
743788
lint_array!(
@@ -772,7 +817,9 @@ impl LintPass for Pass {
772817
ITER_CLONED_COLLECT,
773818
USELESS_ASREF,
774819
UNNECESSARY_FOLD,
775-
UNNECESSARY_FILTER_MAP
820+
UNNECESSARY_FILTER_MAP,
821+
INTO_ITER_ON_ARRAY,
822+
INTO_ITER_ON_REF,
776823
)
777824
}
778825
}
@@ -857,6 +904,9 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Pass {
857904
lint_single_char_pattern(cx, expr, &args[pos]);
858905
}
859906
},
907+
ty::Ref(..) if method_call.ident.name == "into_iter" => {
908+
lint_into_iter(cx, expr, self_ty, *method_span);
909+
},
860910
_ => (),
861911
}
862912
},
@@ -2093,6 +2143,71 @@ fn lint_asref(cx: &LateContext<'_, '_>, expr: &hir::Expr, call_name: &str, as_re
20932143
}
20942144
}
20952145

2146+
fn ty_has_iter_method(cx: &LateContext<'_, '_>, self_ref_ty: ty::Ty<'_>) -> Option<(&'static Lint, &'static str, &'static str)> {
2147+
let (self_ty, mutbl) = match self_ref_ty.sty {
2148+
ty::TyKind::Ref(_, self_ty, mutbl) => (self_ty, mutbl),
2149+
_ => unreachable!(),
2150+
};
2151+
let method_name = match mutbl {
2152+
hir::MutImmutable => "iter",
2153+
hir::MutMutable => "iter_mut",
2154+
};
2155+
2156+
let def_id = match self_ty.sty {
2157+
ty::TyKind::Array(..) => return Some((INTO_ITER_ON_ARRAY, "array", method_name)),
2158+
ty::TyKind::Slice(..) => return Some((INTO_ITER_ON_REF, "slice", method_name)),
2159+
ty::Adt(adt, _) => adt.did,
2160+
_ => return None,
2161+
};
2162+
2163+
// FIXME: instead of this hard-coded list, we should check if `<adt>::iter`
2164+
// exists and has the desired signature. Unfortunately FnCtxt is not exported
2165+
// so we can't use its `lookup_method` method.
2166+
static INTO_ITER_COLLECTIONS: [(&Lint, &[&str]); 13] = [
2167+
(INTO_ITER_ON_REF, &paths::VEC),
2168+
(INTO_ITER_ON_REF, &paths::OPTION),
2169+
(INTO_ITER_ON_REF, &paths::RESULT),
2170+
(INTO_ITER_ON_REF, &paths::BTREESET),
2171+
(INTO_ITER_ON_REF, &paths::BTREEMAP),
2172+
(INTO_ITER_ON_REF, &paths::VEC_DEQUE),
2173+
(INTO_ITER_ON_REF, &paths::LINKED_LIST),
2174+
(INTO_ITER_ON_REF, &paths::BINARY_HEAP),
2175+
(INTO_ITER_ON_REF, &paths::HASHSET),
2176+
(INTO_ITER_ON_REF, &paths::HASHMAP),
2177+
(INTO_ITER_ON_ARRAY, &["std", "path", "PathBuf"]),
2178+
(INTO_ITER_ON_REF, &["std", "path", "Path"]),
2179+
(INTO_ITER_ON_REF, &["std", "sync", "mpsc", "Receiver"]),
2180+
];
2181+
2182+
for (lint, path) in &INTO_ITER_COLLECTIONS {
2183+
if match_def_path(cx.tcx, def_id, path) {
2184+
return Some((lint, path.last().unwrap(), method_name))
2185+
}
2186+
}
2187+
None
2188+
}
2189+
2190+
fn lint_into_iter(cx: &LateContext<'_, '_>, expr: &hir::Expr, self_ref_ty: ty::Ty<'_>, method_span: Span) {
2191+
if !match_trait_method(cx, expr, &paths::INTO_ITERATOR) {
2192+
return;
2193+
}
2194+
if let Some((lint, kind, method_name)) = ty_has_iter_method(cx, self_ref_ty) {
2195+
span_lint_and_sugg(
2196+
cx,
2197+
lint,
2198+
method_span,
2199+
&format!(
2200+
"this .into_iter() call is equivalent to .{}() and will not move the {}",
2201+
method_name,
2202+
kind,
2203+
),
2204+
"call directly",
2205+
method_name.to_owned(),
2206+
);
2207+
}
2208+
}
2209+
2210+
20962211
/// Given a `Result<T, E>` type, return its error type (`E`).
20972212
fn get_error_type<'a>(cx: &LateContext<'_, '_>, ty: Ty<'a>) -> Option<Ty<'a>> {
20982213
if let ty::Adt(_, substs) = ty.sty {

‎tests/ui/for_loop.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,7 @@ impl Unrelated {
8787
clippy::explicit_counter_loop, clippy::for_kv_map)]
8888
#[warn(clippy::unused_collect)]
8989
#[allow(clippy::linkedlist, clippy::shadow_unrelated, clippy::unnecessary_mut_passed, clippy::cyclomatic_complexity, clippy::similar_names)]
90-
#[allow(clippy::many_single_char_names, unused_variables)]
90+
#[allow(clippy::many_single_char_names, unused_variables, clippy::into_iter_on_array)]
9191
fn main() {
9292
const MAX_LEN: usize = 42;
9393

‎tests/ui/into_iter_on_ref.rs

+44
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
#![warn(clippy::into_iter_on_ref)]
2+
#![deny(clippy::into_iter_on_array)]
3+
4+
struct X;
5+
use std::collections::*;
6+
7+
fn main() {
8+
for _ in &[1,2,3] {}
9+
for _ in vec![X, X] {}
10+
for _ in &vec![X, X] {}
11+
for _ in [1,2,3].into_iter() {} //~ ERROR equivalent to .iter()
12+
13+
let _ = [1,2,3].into_iter(); //~ ERROR equivalent to .iter()
14+
let _ = vec![1,2,3].into_iter();
15+
let _ = (&vec![1,2,3]).into_iter(); //~ WARN equivalent to .iter()
16+
let _ = vec![1,2,3].into_boxed_slice().into_iter(); //~ WARN equivalent to .iter()
17+
let _ = std::rc::Rc::from(&[X][..]).into_iter(); //~ WARN equivalent to .iter()
18+
let _ = std::sync::Arc::from(&[X][..]).into_iter(); //~ WARN equivalent to .iter()
19+
20+
let _ = (&&&&&&&[1,2,3]).into_iter(); //~ ERROR equivalent to .iter()
21+
let _ = (&&&&mut &&&[1,2,3]).into_iter(); //~ ERROR equivalent to .iter()
22+
let _ = (&mut &mut &mut [1,2,3]).into_iter(); //~ ERROR equivalent to .iter_mut()
23+
24+
let _ = (&Some(4)).into_iter(); //~ WARN equivalent to .iter()
25+
let _ = (&mut Some(5)).into_iter(); //~ WARN equivalent to .iter_mut()
26+
let _ = (&Ok::<_, i32>(6)).into_iter(); //~ WARN equivalent to .iter()
27+
let _ = (&mut Err::<i32, _>(7)).into_iter(); //~ WARN equivalent to .iter_mut()
28+
let _ = (&Vec::<i32>::new()).into_iter(); //~ WARN equivalent to .iter()
29+
let _ = (&mut Vec::<i32>::new()).into_iter(); //~ WARN equivalent to .iter_mut()
30+
let _ = (&BTreeMap::<i32, u64>::new()).into_iter(); //~ WARN equivalent to .iter()
31+
let _ = (&mut BTreeMap::<i32, u64>::new()).into_iter(); //~ WARN equivalent to .iter_mut()
32+
let _ = (&VecDeque::<i32>::new()).into_iter(); //~ WARN equivalent to .iter()
33+
let _ = (&mut VecDeque::<i32>::new()).into_iter(); //~ WARN equivalent to .iter_mut()
34+
let _ = (&LinkedList::<i32>::new()).into_iter(); //~ WARN equivalent to .iter()
35+
let _ = (&mut LinkedList::<i32>::new()).into_iter(); //~ WARN equivalent to .iter_mut()
36+
let _ = (&HashMap::<i32, u64>::new()).into_iter(); //~ WARN equivalent to .iter()
37+
let _ = (&mut HashMap::<i32, u64>::new()).into_iter(); //~ WARN equivalent to .iter_mut()
38+
39+
let _ = (&BTreeSet::<i32>::new()).into_iter(); //~ WARN equivalent to .iter()
40+
let _ = (&BinaryHeap::<i32>::new()).into_iter(); //~ WARN equivalent to .iter()
41+
let _ = (&HashSet::<i32>::new()).into_iter(); //~ WARN equivalent to .iter()
42+
let _ = std::path::Path::new("12/34").into_iter(); //~ WARN equivalent to .iter()
43+
let _ = std::path::PathBuf::from("12/34").into_iter(); //~ ERROR equivalent to .iter()
44+
}

‎tests/ui/into_iter_on_ref.stderr

+178
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,178 @@
1+
error: this .into_iter() call is equivalent to .iter() and will not move the array
2+
--> $DIR/into_iter_on_ref.rs:11:22
3+
|
4+
11 | for _ in [1,2,3].into_iter() {} //~ ERROR equivalent to .iter()
5+
| ^^^^^^^^^ help: call directly: `iter`
6+
|
7+
note: lint level defined here
8+
--> $DIR/into_iter_on_ref.rs:2:9
9+
|
10+
2 | #![deny(clippy::into_iter_on_array)]
11+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^
12+
13+
error: this .into_iter() call is equivalent to .iter() and will not move the array
14+
--> $DIR/into_iter_on_ref.rs:13:21
15+
|
16+
13 | let _ = [1,2,3].into_iter(); //~ ERROR equivalent to .iter()
17+
| ^^^^^^^^^ help: call directly: `iter`
18+
19+
error: this .into_iter() call is equivalent to .iter() and will not move the Vec
20+
--> $DIR/into_iter_on_ref.rs:15:28
21+
|
22+
15 | let _ = (&vec![1,2,3]).into_iter(); //~ WARN equivalent to .iter()
23+
| ^^^^^^^^^ help: call directly: `iter`
24+
|
25+
= note: `-D clippy::into-iter-on-ref` implied by `-D warnings`
26+
27+
error: this .into_iter() call is equivalent to .iter() and will not move the slice
28+
--> $DIR/into_iter_on_ref.rs:16:44
29+
|
30+
16 | let _ = vec![1,2,3].into_boxed_slice().into_iter(); //~ WARN equivalent to .iter()
31+
| ^^^^^^^^^ help: call directly: `iter`
32+
33+
error: this .into_iter() call is equivalent to .iter() and will not move the slice
34+
--> $DIR/into_iter_on_ref.rs:17:41
35+
|
36+
17 | let _ = std::rc::Rc::from(&[X][..]).into_iter(); //~ WARN equivalent to .iter()
37+
| ^^^^^^^^^ help: call directly: `iter`
38+
39+
error: this .into_iter() call is equivalent to .iter() and will not move the slice
40+
--> $DIR/into_iter_on_ref.rs:18:44
41+
|
42+
18 | let _ = std::sync::Arc::from(&[X][..]).into_iter(); //~ WARN equivalent to .iter()
43+
| ^^^^^^^^^ help: call directly: `iter`
44+
45+
error: this .into_iter() call is equivalent to .iter() and will not move the array
46+
--> $DIR/into_iter_on_ref.rs:20:30
47+
|
48+
20 | let _ = (&&&&&&&[1,2,3]).into_iter(); //~ ERROR equivalent to .iter()
49+
| ^^^^^^^^^ help: call directly: `iter`
50+
51+
error: this .into_iter() call is equivalent to .iter() and will not move the array
52+
--> $DIR/into_iter_on_ref.rs:21:34
53+
|
54+
21 | let _ = (&&&&mut &&&[1,2,3]).into_iter(); //~ ERROR equivalent to .iter()
55+
| ^^^^^^^^^ help: call directly: `iter`
56+
57+
error: this .into_iter() call is equivalent to .iter_mut() and will not move the array
58+
--> $DIR/into_iter_on_ref.rs:22:38
59+
|
60+
22 | let _ = (&mut &mut &mut [1,2,3]).into_iter(); //~ ERROR equivalent to .iter_mut()
61+
| ^^^^^^^^^ help: call directly: `iter_mut`
62+
63+
error: this .into_iter() call is equivalent to .iter() and will not move the Option
64+
--> $DIR/into_iter_on_ref.rs:24:24
65+
|
66+
24 | let _ = (&Some(4)).into_iter(); //~ WARN equivalent to .iter()
67+
| ^^^^^^^^^ help: call directly: `iter`
68+
69+
error: this .into_iter() call is equivalent to .iter_mut() and will not move the Option
70+
--> $DIR/into_iter_on_ref.rs:25:28
71+
|
72+
25 | let _ = (&mut Some(5)).into_iter(); //~ WARN equivalent to .iter_mut()
73+
| ^^^^^^^^^ help: call directly: `iter_mut`
74+
75+
error: this .into_iter() call is equivalent to .iter() and will not move the Result
76+
--> $DIR/into_iter_on_ref.rs:26:32
77+
|
78+
26 | let _ = (&Ok::<_, i32>(6)).into_iter(); //~ WARN equivalent to .iter()
79+
| ^^^^^^^^^ help: call directly: `iter`
80+
81+
error: this .into_iter() call is equivalent to .iter_mut() and will not move the Result
82+
--> $DIR/into_iter_on_ref.rs:27:37
83+
|
84+
27 | let _ = (&mut Err::<i32, _>(7)).into_iter(); //~ WARN equivalent to .iter_mut()
85+
| ^^^^^^^^^ help: call directly: `iter_mut`
86+
87+
error: this .into_iter() call is equivalent to .iter() and will not move the Vec
88+
--> $DIR/into_iter_on_ref.rs:28:34
89+
|
90+
28 | let _ = (&Vec::<i32>::new()).into_iter(); //~ WARN equivalent to .iter()
91+
| ^^^^^^^^^ help: call directly: `iter`
92+
93+
error: this .into_iter() call is equivalent to .iter_mut() and will not move the Vec
94+
--> $DIR/into_iter_on_ref.rs:29:38
95+
|
96+
29 | let _ = (&mut Vec::<i32>::new()).into_iter(); //~ WARN equivalent to .iter_mut()
97+
| ^^^^^^^^^ help: call directly: `iter_mut`
98+
99+
error: this .into_iter() call is equivalent to .iter() and will not move the BTreeMap
100+
--> $DIR/into_iter_on_ref.rs:30:44
101+
|
102+
30 | let _ = (&BTreeMap::<i32, u64>::new()).into_iter(); //~ WARN equivalent to .iter()
103+
| ^^^^^^^^^ help: call directly: `iter`
104+
105+
error: this .into_iter() call is equivalent to .iter_mut() and will not move the BTreeMap
106+
--> $DIR/into_iter_on_ref.rs:31:48
107+
|
108+
31 | let _ = (&mut BTreeMap::<i32, u64>::new()).into_iter(); //~ WARN equivalent to .iter_mut()
109+
| ^^^^^^^^^ help: call directly: `iter_mut`
110+
111+
error: this .into_iter() call is equivalent to .iter() and will not move the VecDeque
112+
--> $DIR/into_iter_on_ref.rs:32:39
113+
|
114+
32 | let _ = (&VecDeque::<i32>::new()).into_iter(); //~ WARN equivalent to .iter()
115+
| ^^^^^^^^^ help: call directly: `iter`
116+
117+
error: this .into_iter() call is equivalent to .iter_mut() and will not move the VecDeque
118+
--> $DIR/into_iter_on_ref.rs:33:43
119+
|
120+
33 | let _ = (&mut VecDeque::<i32>::new()).into_iter(); //~ WARN equivalent to .iter_mut()
121+
| ^^^^^^^^^ help: call directly: `iter_mut`
122+
123+
error: this .into_iter() call is equivalent to .iter() and will not move the LinkedList
124+
--> $DIR/into_iter_on_ref.rs:34:41
125+
|
126+
34 | let _ = (&LinkedList::<i32>::new()).into_iter(); //~ WARN equivalent to .iter()
127+
| ^^^^^^^^^ help: call directly: `iter`
128+
129+
error: this .into_iter() call is equivalent to .iter_mut() and will not move the LinkedList
130+
--> $DIR/into_iter_on_ref.rs:35:45
131+
|
132+
35 | let _ = (&mut LinkedList::<i32>::new()).into_iter(); //~ WARN equivalent to .iter_mut()
133+
| ^^^^^^^^^ help: call directly: `iter_mut`
134+
135+
error: this .into_iter() call is equivalent to .iter() and will not move the HashMap
136+
--> $DIR/into_iter_on_ref.rs:36:43
137+
|
138+
36 | let _ = (&HashMap::<i32, u64>::new()).into_iter(); //~ WARN equivalent to .iter()
139+
| ^^^^^^^^^ help: call directly: `iter`
140+
141+
error: this .into_iter() call is equivalent to .iter_mut() and will not move the HashMap
142+
--> $DIR/into_iter_on_ref.rs:37:47
143+
|
144+
37 | let _ = (&mut HashMap::<i32, u64>::new()).into_iter(); //~ WARN equivalent to .iter_mut()
145+
| ^^^^^^^^^ help: call directly: `iter_mut`
146+
147+
error: this .into_iter() call is equivalent to .iter() and will not move the BTreeSet
148+
--> $DIR/into_iter_on_ref.rs:39:39
149+
|
150+
39 | let _ = (&BTreeSet::<i32>::new()).into_iter(); //~ WARN equivalent to .iter()
151+
| ^^^^^^^^^ help: call directly: `iter`
152+
153+
error: this .into_iter() call is equivalent to .iter() and will not move the BinaryHeap
154+
--> $DIR/into_iter_on_ref.rs:40:41
155+
|
156+
40 | let _ = (&BinaryHeap::<i32>::new()).into_iter(); //~ WARN equivalent to .iter()
157+
| ^^^^^^^^^ help: call directly: `iter`
158+
159+
error: this .into_iter() call is equivalent to .iter() and will not move the HashSet
160+
--> $DIR/into_iter_on_ref.rs:41:38
161+
|
162+
41 | let _ = (&HashSet::<i32>::new()).into_iter(); //~ WARN equivalent to .iter()
163+
| ^^^^^^^^^ help: call directly: `iter`
164+
165+
error: this .into_iter() call is equivalent to .iter() and will not move the Path
166+
--> $DIR/into_iter_on_ref.rs:42:43
167+
|
168+
42 | let _ = std::path::Path::new("12/34").into_iter(); //~ WARN equivalent to .iter()
169+
| ^^^^^^^^^ help: call directly: `iter`
170+
171+
error: this .into_iter() call is equivalent to .iter() and will not move the PathBuf
172+
--> $DIR/into_iter_on_ref.rs:43:47
173+
|
174+
43 | let _ = std::path::PathBuf::from("12/34").into_iter(); //~ ERROR equivalent to .iter()
175+
| ^^^^^^^^^ help: call directly: `iter`
176+
177+
error: aborting due to 28 previous errors
178+

0 commit comments

Comments
 (0)