Skip to content

Commit af39a8a

Browse files
samueltardieusamueltardieu
committedDec 17, 2022
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`.
·
rust-1.85.0rust-1.68.0
1 parent 38fce12 commit af39a8a

File tree

4 files changed

+200
-28
lines changed

4 files changed

+200
-28
lines changed
 

‎clippy_lints/src/useless_conversion.rs

+18-10
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

+70-3
Original file line numberDiff line numberDiff line change
@@ -33,20 +33,87 @@ fn test_issue_3913() -> Result<(), std::io::Error> {
3333
Ok(())
3434
}
3535

36-
fn test_issue_5833() -> Result<(), ()> {
36+
fn dont_lint_into_iter_on_immutable_local_implementing_iterator_in_expr() {
3737
let text = "foo\r\nbar\n\nbaz\n";
3838
let lines = text.lines();
3939
if Some("ok") == lines.into_iter().next() {}
40+
}
4041

41-
Ok(())
42+
fn lint_into_iter_on_mutable_local_implementing_iterator_in_expr() {
43+
let text = "foo\r\nbar\n\nbaz\n";
44+
let mut lines = text.lines();
45+
if Some("ok") == lines.next() {}
46+
}
47+
48+
fn lint_into_iter_on_expr_implementing_iterator() {
49+
let text = "foo\r\nbar\n\nbaz\n";
50+
let mut lines = text.lines();
51+
if Some("ok") == lines.next() {}
52+
}
53+
54+
fn lint_into_iter_on_expr_implementing_iterator_2() {
55+
let text = "foo\r\nbar\n\nbaz\n";
56+
if Some("ok") == text.lines().next() {}
57+
}
58+
59+
#[allow(const_item_mutation)]
60+
fn lint_into_iter_on_const_implementing_iterator() {
61+
const NUMBERS: std::ops::Range<i32> = 0..10;
62+
let _ = NUMBERS.next();
63+
}
64+
65+
fn lint_into_iter_on_const_implementing_iterator_2() {
66+
const NUMBERS: std::ops::Range<i32> = 0..10;
67+
let mut n = NUMBERS;
68+
n.next();
69+
}
70+
71+
#[derive(Clone, Copy)]
72+
struct CopiableCounter {
73+
counter: u32,
74+
}
75+
76+
impl Iterator for CopiableCounter {
77+
type Item = u32;
78+
79+
fn next(&mut self) -> Option<Self::Item> {
80+
self.counter = self.counter.wrapping_add(1);
81+
Some(self.counter)
82+
}
83+
}
84+
85+
fn dont_lint_into_iter_on_copy_iter() {
86+
let mut c = CopiableCounter { counter: 0 };
87+
assert_eq!(c.into_iter().next(), Some(1));
88+
assert_eq!(c.into_iter().next(), Some(1));
89+
assert_eq!(c.next(), Some(1));
90+
assert_eq!(c.next(), Some(2));
91+
}
92+
93+
fn dont_lint_into_iter_on_static_copy_iter() {
94+
static mut C: CopiableCounter = CopiableCounter { counter: 0 };
95+
unsafe {
96+
assert_eq!(C.into_iter().next(), Some(1));
97+
assert_eq!(C.into_iter().next(), Some(1));
98+
assert_eq!(C.next(), Some(1));
99+
assert_eq!(C.next(), Some(2));
100+
}
42101
}
43102

44103
fn main() {
45104
test_generic(10i32);
46105
test_generic2::<i32, i32>(10i32);
47106
test_questionmark().unwrap();
48107
test_issue_3913().unwrap();
49-
test_issue_5833().unwrap();
108+
109+
dont_lint_into_iter_on_immutable_local_implementing_iterator_in_expr();
110+
lint_into_iter_on_mutable_local_implementing_iterator_in_expr();
111+
lint_into_iter_on_expr_implementing_iterator();
112+
lint_into_iter_on_expr_implementing_iterator_2();
113+
lint_into_iter_on_const_implementing_iterator();
114+
lint_into_iter_on_const_implementing_iterator_2();
115+
dont_lint_into_iter_on_copy_iter();
116+
dont_lint_into_iter_on_static_copy_iter();
50117

51118
let _: String = "foo".into();
52119
let _: String = From::from("foo");

‎tests/ui/useless_conversion.rs

+70-3
Original file line numberDiff line numberDiff line change
@@ -33,20 +33,87 @@ fn test_issue_3913() -> Result<(), std::io::Error> {
3333
Ok(())
3434
}
3535

36-
fn test_issue_5833() -> Result<(), ()> {
36+
fn dont_lint_into_iter_on_immutable_local_implementing_iterator_in_expr() {
3737
let text = "foo\r\nbar\n\nbaz\n";
3838
let lines = text.lines();
3939
if Some("ok") == lines.into_iter().next() {}
40+
}
4041

41-
Ok(())
42+
fn lint_into_iter_on_mutable_local_implementing_iterator_in_expr() {
43+
let text = "foo\r\nbar\n\nbaz\n";
44+
let mut lines = text.lines();
45+
if Some("ok") == lines.into_iter().next() {}
46+
}
47+
48+
fn lint_into_iter_on_expr_implementing_iterator() {
49+
let text = "foo\r\nbar\n\nbaz\n";
50+
let mut lines = text.lines().into_iter();
51+
if Some("ok") == lines.next() {}
52+
}
53+
54+
fn lint_into_iter_on_expr_implementing_iterator_2() {
55+
let text = "foo\r\nbar\n\nbaz\n";
56+
if Some("ok") == text.lines().into_iter().next() {}
57+
}
58+
59+
#[allow(const_item_mutation)]
60+
fn lint_into_iter_on_const_implementing_iterator() {
61+
const NUMBERS: std::ops::Range<i32> = 0..10;
62+
let _ = NUMBERS.into_iter().next();
63+
}
64+
65+
fn lint_into_iter_on_const_implementing_iterator_2() {
66+
const NUMBERS: std::ops::Range<i32> = 0..10;
67+
let mut n = NUMBERS.into_iter();
68+
n.next();
69+
}
70+
71+
#[derive(Clone, Copy)]
72+
struct CopiableCounter {
73+
counter: u32,
74+
}
75+
76+
impl Iterator for CopiableCounter {
77+
type Item = u32;
78+
79+
fn next(&mut self) -> Option<Self::Item> {
80+
self.counter = self.counter.wrapping_add(1);
81+
Some(self.counter)
82+
}
83+
}
84+
85+
fn dont_lint_into_iter_on_copy_iter() {
86+
let mut c = CopiableCounter { counter: 0 };
87+
assert_eq!(c.into_iter().next(), Some(1));
88+
assert_eq!(c.into_iter().next(), Some(1));
89+
assert_eq!(c.next(), Some(1));
90+
assert_eq!(c.next(), Some(2));
91+
}
92+
93+
fn dont_lint_into_iter_on_static_copy_iter() {
94+
static mut C: CopiableCounter = CopiableCounter { counter: 0 };
95+
unsafe {
96+
assert_eq!(C.into_iter().next(), Some(1));
97+
assert_eq!(C.into_iter().next(), Some(1));
98+
assert_eq!(C.next(), Some(1));
99+
assert_eq!(C.next(), Some(2));
100+
}
42101
}
43102

44103
fn main() {
45104
test_generic(10i32);
46105
test_generic2::<i32, i32>(10i32);
47106
test_questionmark().unwrap();
48107
test_issue_3913().unwrap();
49-
test_issue_5833().unwrap();
108+
109+
dont_lint_into_iter_on_immutable_local_implementing_iterator_in_expr();
110+
lint_into_iter_on_mutable_local_implementing_iterator_in_expr();
111+
lint_into_iter_on_expr_implementing_iterator();
112+
lint_into_iter_on_expr_implementing_iterator_2();
113+
lint_into_iter_on_const_implementing_iterator();
114+
lint_into_iter_on_const_implementing_iterator_2();
115+
dont_lint_into_iter_on_copy_iter();
116+
dont_lint_into_iter_on_static_copy_iter();
50117

51118
let _: String = "foo".into();
52119
let _: String = From::from("foo");

‎tests/ui/useless_conversion.stderr

+42-12
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:45: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:50: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:56: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:62: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:67: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:128: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:129: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:130: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:131: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:132: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:133: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:134: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:139: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:145: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:147: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:149: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)
Please sign in to comment.