Skip to content

Commit 77ffe14

Browse files
samueltardieusamueltardieu
committed
Identify more cases of useless into_iter() calls
If the type of the result of a call to `IntoIterator::into_iter()` and the type of the receiver are the same, then the receiver implements `Iterator` and `into_iter()` is the identity function. The call to `into_iter()` may be removed in all but two cases: - If the receiver implements `Copy`, `into_iter()` will produce a copy of the receiver and cannot be removed. For example, `x.into_iter().next()` will not advance `x` while `x.next()` will. - If the receiver is an immutable local variable and the call to `into_iter()` appears in a larger expression, removing the call to `into_iter()` might cause mutability issues. For example, if `x` is an immutable local variable, `x.into_iter().next()` will compile while `x.next()` will not as `next()` receives `&mut self`.
1 parent 38fce12 commit 77ffe14

File tree

4 files changed

+216
-22
lines changed

4 files changed

+216
-22
lines changed

‎clippy_lints/src/useless_conversion.rs

Lines changed: 18 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,11 @@
11
use clippy_utils::diagnostics::{span_lint_and_help, span_lint_and_sugg};
22
use clippy_utils::source::{snippet, snippet_with_macro_callsite};
33
use clippy_utils::sugg::Sugg;
4-
use clippy_utils::ty::{is_type_diagnostic_item, same_type_and_consts};
5-
use clippy_utils::{get_parent_expr, is_trait_method, match_def_path, paths};
4+
use clippy_utils::ty::{is_copy, is_type_diagnostic_item, same_type_and_consts};
5+
use clippy_utils::{get_parent_expr, is_trait_method, match_def_path, path_to_local, paths};
66
use if_chain::if_chain;
77
use rustc_errors::Applicability;
8-
use rustc_hir::{Expr, ExprKind, HirId, MatchSource};
8+
use rustc_hir::{BindingAnnotation, Expr, ExprKind, HirId, MatchSource, Node, PatKind};
99
use rustc_lint::{LateContext, LateLintPass};
1010
use rustc_middle::ty;
1111
use rustc_session::{declare_tool_lint, impl_lint_pass};
@@ -81,16 +81,24 @@ impl<'tcx> LateLintPass<'tcx> for UselessConversion {
8181
}
8282
}
8383
if is_trait_method(cx, e, sym::IntoIterator) && name.ident.name == sym::into_iter {
84-
if let Some(parent_expr) = get_parent_expr(cx, e) {
85-
if let ExprKind::MethodCall(parent_name, ..) = parent_expr.kind {
86-
if parent_name.ident.name != sym::into_iter {
87-
return;
88-
}
89-
}
84+
if get_parent_expr(cx, e).is_some() &&
85+
let Some(id) = path_to_local(recv) &&
86+
let Node::Pat(pat) = cx.tcx.hir().get(id) &&
87+
let PatKind::Binding(ann, ..) = pat.kind &&
88+
ann != BindingAnnotation::MUT
89+
{
90+
// Do not remove .into_iter() applied to a non-mutable local variable used in
91+
// a larger expression context as it would differ in mutability.
92+
return;
9093
}
94+
9195
let a = cx.typeck_results().expr_ty(e);
9296
let b = cx.typeck_results().expr_ty(recv);
93-
if same_type_and_consts(a, b) {
97+
98+
// If the types are identical then .into_iter() can be removed, unless the type
99+
// implements Copy, in which case .into_iter() returns a copy of the receiver and
100+
// cannot be safely omitted.
101+
if same_type_and_consts(a, b) && !is_copy(cx, b) {
94102
let sugg = snippet(cx, recv.span, "<expr>").into_owned();
95103
span_lint_and_sugg(
96104
cx,

‎tests/ui/useless_conversion.fixed

Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,12 +41,90 @@ fn test_issue_5833() -> Result<(), ()> {
4141
Ok(())
4242
}
4343

44+
fn test_issue_5833_alt1() -> Result<(), ()> {
45+
let text = "foo\r\nbar\n\nbaz\n";
46+
let mut lines = text.lines();
47+
if Some("ok") == lines.next() {}
48+
49+
Ok(())
50+
}
51+
52+
fn test_issue_5833_alt2() -> Result<(), ()> {
53+
let text = "foo\r\nbar\n\nbaz\n";
54+
let mut lines = text.lines();
55+
if Some("ok") == lines.next() {}
56+
57+
Ok(())
58+
}
59+
60+
fn test_issue_5833_alt3() -> Result<(), ()> {
61+
let text = "foo\r\nbar\n\nbaz\n";
62+
if Some("ok") == text.lines().next() {}
63+
64+
Ok(())
65+
}
66+
67+
#[allow(const_item_mutation)]
68+
fn test_const_into_iter1() -> Result<(), ()> {
69+
const NUMBERS: std::ops::Range<i32> = 0..10;
70+
let _ = NUMBERS.next();
71+
72+
Ok(())
73+
}
74+
75+
fn test_const_into_iter2() -> Result<(), ()> {
76+
const NUMBERS: std::ops::Range<i32> = 0..10;
77+
let mut n = NUMBERS;
78+
n.next();
79+
80+
Ok(())
81+
}
82+
83+
#[derive(Clone, Copy)]
84+
struct CopiableCounter {
85+
counter: u32,
86+
}
87+
88+
impl Iterator for CopiableCounter {
89+
type Item = u32;
90+
91+
fn next(&mut self) -> Option<Self::Item> {
92+
self.counter = self.counter.wrapping_add(1);
93+
Some(self.counter)
94+
}
95+
}
96+
97+
fn test_copiable_into_iter() {
98+
let mut c = CopiableCounter { counter: 0 };
99+
assert_eq!(c.into_iter().next(), Some(1));
100+
assert_eq!(c.into_iter().next(), Some(1));
101+
assert_eq!(c.next(), Some(1));
102+
assert_eq!(c.next(), Some(2));
103+
}
104+
105+
fn test_static_copiable_into_iter() {
106+
static mut C: CopiableCounter = CopiableCounter { counter: 0 };
107+
unsafe {
108+
assert_eq!(C.into_iter().next(), Some(1));
109+
assert_eq!(C.into_iter().next(), Some(1));
110+
assert_eq!(C.next(), Some(1));
111+
assert_eq!(C.next(), Some(2));
112+
}
113+
}
114+
44115
fn main() {
45116
test_generic(10i32);
46117
test_generic2::<i32, i32>(10i32);
47118
test_questionmark().unwrap();
48119
test_issue_3913().unwrap();
49120
test_issue_5833().unwrap();
121+
test_issue_5833_alt1().unwrap();
122+
test_issue_5833_alt2().unwrap();
123+
test_issue_5833_alt3().unwrap();
124+
test_const_into_iter1().unwrap();
125+
test_const_into_iter2().unwrap();
126+
test_copiable_into_iter();
127+
test_static_copiable_into_iter();
50128

51129
let _: String = "foo".into();
52130
let _: String = From::from("foo");

‎tests/ui/useless_conversion.rs

Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,12 +41,90 @@ fn test_issue_5833() -> Result<(), ()> {
4141
Ok(())
4242
}
4343

44+
fn test_issue_5833_alt1() -> Result<(), ()> {
45+
let text = "foo\r\nbar\n\nbaz\n";
46+
let mut lines = text.lines();
47+
if Some("ok") == lines.into_iter().next() {}
48+
49+
Ok(())
50+
}
51+
52+
fn test_issue_5833_alt2() -> Result<(), ()> {
53+
let text = "foo\r\nbar\n\nbaz\n";
54+
let mut lines = text.lines().into_iter();
55+
if Some("ok") == lines.next() {}
56+
57+
Ok(())
58+
}
59+
60+
fn test_issue_5833_alt3() -> Result<(), ()> {
61+
let text = "foo\r\nbar\n\nbaz\n";
62+
if Some("ok") == text.lines().into_iter().next() {}
63+
64+
Ok(())
65+
}
66+
67+
#[allow(const_item_mutation)]
68+
fn test_const_into_iter1() -> Result<(), ()> {
69+
const NUMBERS: std::ops::Range<i32> = 0..10;
70+
let _ = NUMBERS.into_iter().next();
71+
72+
Ok(())
73+
}
74+
75+
fn test_const_into_iter2() -> Result<(), ()> {
76+
const NUMBERS: std::ops::Range<i32> = 0..10;
77+
let mut n = NUMBERS.into_iter();
78+
n.next();
79+
80+
Ok(())
81+
}
82+
83+
#[derive(Clone, Copy)]
84+
struct CopiableCounter {
85+
counter: u32,
86+
}
87+
88+
impl Iterator for CopiableCounter {
89+
type Item = u32;
90+
91+
fn next(&mut self) -> Option<Self::Item> {
92+
self.counter = self.counter.wrapping_add(1);
93+
Some(self.counter)
94+
}
95+
}
96+
97+
fn test_copiable_into_iter() {
98+
let mut c = CopiableCounter { counter: 0 };
99+
assert_eq!(c.into_iter().next(), Some(1));
100+
assert_eq!(c.into_iter().next(), Some(1));
101+
assert_eq!(c.next(), Some(1));
102+
assert_eq!(c.next(), Some(2));
103+
}
104+
105+
fn test_static_copiable_into_iter() {
106+
static mut C: CopiableCounter = CopiableCounter { counter: 0 };
107+
unsafe {
108+
assert_eq!(C.into_iter().next(), Some(1));
109+
assert_eq!(C.into_iter().next(), Some(1));
110+
assert_eq!(C.next(), Some(1));
111+
assert_eq!(C.next(), Some(2));
112+
}
113+
}
114+
44115
fn main() {
45116
test_generic(10i32);
46117
test_generic2::<i32, i32>(10i32);
47118
test_questionmark().unwrap();
48119
test_issue_3913().unwrap();
49120
test_issue_5833().unwrap();
121+
test_issue_5833_alt1().unwrap();
122+
test_issue_5833_alt2().unwrap();
123+
test_issue_5833_alt3().unwrap();
124+
test_const_into_iter1().unwrap();
125+
test_const_into_iter2().unwrap();
126+
test_copiable_into_iter();
127+
test_static_copiable_into_iter();
50128

51129
let _: String = "foo".into();
52130
let _: String = From::from("foo");

‎tests/ui/useless_conversion.stderr

Lines changed: 42 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -22,71 +22,101 @@ error: useless conversion to the same type: `i32`
2222
LL | let _: i32 = 0i32.into();
2323
| ^^^^^^^^^^^ help: consider removing `.into()`: `0i32`
2424

25+
error: useless conversion to the same type: `std::str::Lines<'_>`
26+
--> $DIR/useless_conversion.rs:47:22
27+
|
28+
LL | if Some("ok") == lines.into_iter().next() {}
29+
| ^^^^^^^^^^^^^^^^^ help: consider removing `.into_iter()`: `lines`
30+
31+
error: useless conversion to the same type: `std::str::Lines<'_>`
32+
--> $DIR/useless_conversion.rs:54:21
33+
|
34+
LL | let mut lines = text.lines().into_iter();
35+
| ^^^^^^^^^^^^^^^^^^^^^^^^ help: consider removing `.into_iter()`: `text.lines()`
36+
37+
error: useless conversion to the same type: `std::str::Lines<'_>`
38+
--> $DIR/useless_conversion.rs:62:22
39+
|
40+
LL | if Some("ok") == text.lines().into_iter().next() {}
41+
| ^^^^^^^^^^^^^^^^^^^^^^^^ help: consider removing `.into_iter()`: `text.lines()`
42+
43+
error: useless conversion to the same type: `std::ops::Range<i32>`
44+
--> $DIR/useless_conversion.rs:70:13
45+
|
46+
LL | let _ = NUMBERS.into_iter().next();
47+
| ^^^^^^^^^^^^^^^^^^^ help: consider removing `.into_iter()`: `NUMBERS`
48+
49+
error: useless conversion to the same type: `std::ops::Range<i32>`
50+
--> $DIR/useless_conversion.rs:77:17
51+
|
52+
LL | let mut n = NUMBERS.into_iter();
53+
| ^^^^^^^^^^^^^^^^^^^ help: consider removing `.into_iter()`: `NUMBERS`
54+
2555
error: useless conversion to the same type: `std::string::String`
26-
--> $DIR/useless_conversion.rs:61:21
56+
--> $DIR/useless_conversion.rs:139:21
2757
|
2858
LL | let _: String = "foo".to_string().into();
2959
| ^^^^^^^^^^^^^^^^^^^^^^^^ help: consider removing `.into()`: `"foo".to_string()`
3060

3161
error: useless conversion to the same type: `std::string::String`
32-
--> $DIR/useless_conversion.rs:62:21
62+
--> $DIR/useless_conversion.rs:140:21
3363
|
3464
LL | let _: String = From::from("foo".to_string());
3565
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider removing `From::from()`: `"foo".to_string()`
3666

3767
error: useless conversion to the same type: `std::string::String`
38-
--> $DIR/useless_conversion.rs:63:13
68+
--> $DIR/useless_conversion.rs:141:13
3969
|
4070
LL | let _ = String::from("foo".to_string());
4171
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider removing `String::from()`: `"foo".to_string()`
4272

4373
error: useless conversion to the same type: `std::string::String`
44-
--> $DIR/useless_conversion.rs:64:13
74+
--> $DIR/useless_conversion.rs:142:13
4575
|
4676
LL | let _ = String::from(format!("A: {:04}", 123));
4777
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider removing `String::from()`: `format!("A: {:04}", 123)`
4878

4979
error: useless conversion to the same type: `std::str::Lines<'_>`
50-
--> $DIR/useless_conversion.rs:65:13
80+
--> $DIR/useless_conversion.rs:143:13
5181
|
5282
LL | let _ = "".lines().into_iter();
5383
| ^^^^^^^^^^^^^^^^^^^^^^ help: consider removing `.into_iter()`: `"".lines()`
5484

5585
error: useless conversion to the same type: `std::vec::IntoIter<i32>`
56-
--> $DIR/useless_conversion.rs:66:13
86+
--> $DIR/useless_conversion.rs:144:13
5787
|
5888
LL | let _ = vec![1, 2, 3].into_iter().into_iter();
5989
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider removing `.into_iter()`: `vec![1, 2, 3].into_iter()`
6090

6191
error: useless conversion to the same type: `std::string::String`
62-
--> $DIR/useless_conversion.rs:67:21
92+
--> $DIR/useless_conversion.rs:145:21
6393
|
6494
LL | let _: String = format!("Hello {}", "world").into();
6595
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider removing `.into()`: `format!("Hello {}", "world")`
6696

6797
error: useless conversion to the same type: `i32`
68-
--> $DIR/useless_conversion.rs:72:13
98+
--> $DIR/useless_conversion.rs:150:13
6999
|
70100
LL | let _ = i32::from(a + b) * 3;
71101
| ^^^^^^^^^^^^^^^^ help: consider removing `i32::from()`: `(a + b)`
72102

73103
error: useless conversion to the same type: `Foo<'a'>`
74-
--> $DIR/useless_conversion.rs:78:23
104+
--> $DIR/useless_conversion.rs:156:23
75105
|
76106
LL | let _: Foo<'a'> = s2.into();
77107
| ^^^^^^^^^ help: consider removing `.into()`: `s2`
78108

79109
error: useless conversion to the same type: `Foo<'a'>`
80-
--> $DIR/useless_conversion.rs:80:13
110+
--> $DIR/useless_conversion.rs:158:13
81111
|
82112
LL | let _ = Foo::<'a'>::from(s3);
83113
| ^^^^^^^^^^^^^^^^^^^^ help: consider removing `Foo::<'a'>::from()`: `s3`
84114

85115
error: useless conversion to the same type: `std::vec::IntoIter<Foo<'a'>>`
86-
--> $DIR/useless_conversion.rs:82:13
116+
--> $DIR/useless_conversion.rs:160:13
87117
|
88118
LL | let _ = vec![s4, s4, s4].into_iter().into_iter();
89119
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider removing `.into_iter()`: `vec![s4, s4, s4].into_iter()`
90120

91-
error: aborting due to 14 previous errors
121+
error: aborting due to 19 previous errors
92122

0 commit comments

Comments
 (0)