Skip to content

Commit 013fb42

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. There is only one context in which the call to `into_iter()` cannot be safely removed: when the receiver is a immutable local variable, calling `into_iter()` consumes the variable and returns its value as a temporary expression (which is mutable). It would not be always safe to remove the call to `into_iter()` if this temporary expression is used in the context of a larger expression, for example as the receiver of a mutable method. In a non-expression contexts this is safe though, for example as the RHS of an assignment statement. Another case that we want to avoid is omitting the call to `into_iter()` when the receiver is a `const` item used in a larger expression. In some cases, the compiler would give a warning although the code is equivalent: const R: std::ops::Range<i32> = 1..10; let _ = R.into_iter().next(); // No compiler warning let _ = R.next(); // Warning (const_item_migration) To avoid those two cases, we choose to skip all situations where: - the receiver of `into_iter()` is of kind `ExprKind::Path` and does not denote a mutable local variable; - and the result of `into_iter()` is used in a larger expression.
1 parent d822110 commit 013fb42

File tree

4 files changed

+140
-18
lines changed

4 files changed

+140
-18
lines changed

‎clippy_lints/src/useless_conversion.rs

+18-6
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,10 @@ 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;
44
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};
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,10 +81,22 @@ 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;
84+
if get_parent_expr(cx, e).is_some() {
85+
if_chain! {
86+
if let Some(id) = path_to_local(recv);
87+
if let Node::Pat(pat) = cx.tcx.hir().get(id);
88+
if let PatKind::Binding(ann, _, _, _) = pat.kind;
89+
90+
then {
91+
if ann != BindingAnnotation::MUT {
92+
return;
93+
}
94+
}
95+
96+
else {
97+
if matches!(recv.kind, ExprKind::Path(..)) {
98+
return;
99+
}
88100
}
89101
}
90102
}

‎tests/ui/useless_conversion.fixed

+43
Original file line numberDiff line numberDiff line change
@@ -41,12 +41,55 @@ 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+
fn test_const_into_iter1() -> Result<(), ()> {
68+
const NUMBERS: std::ops::Range<i32> = 0..10;
69+
let _ = NUMBERS.into_iter().next();
70+
71+
Ok(())
72+
}
73+
74+
fn test_const_into_iter2() -> Result<(), ()> {
75+
const NUMBERS: std::ops::Range<i32> = 0..10;
76+
let mut n = NUMBERS;
77+
n.next();
78+
79+
Ok(())
80+
}
81+
4482
fn main() {
4583
test_generic(10i32);
4684
test_generic2::<i32, i32>(10i32);
4785
test_questionmark().unwrap();
4886
test_issue_3913().unwrap();
4987
test_issue_5833().unwrap();
88+
test_issue_5833_alt1().unwrap();
89+
test_issue_5833_alt2().unwrap();
90+
test_issue_5833_alt3().unwrap();
91+
test_const_into_iter1().unwrap();
92+
test_const_into_iter2().unwrap();
5093

5194
let _: String = "foo".into();
5295
let _: String = From::from("foo");

‎tests/ui/useless_conversion.rs

+43
Original file line numberDiff line numberDiff line change
@@ -41,12 +41,55 @@ 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+
fn test_const_into_iter1() -> Result<(), ()> {
68+
const NUMBERS: std::ops::Range<i32> = 0..10;
69+
let _ = NUMBERS.into_iter().next();
70+
71+
Ok(())
72+
}
73+
74+
fn test_const_into_iter2() -> Result<(), ()> {
75+
const NUMBERS: std::ops::Range<i32> = 0..10;
76+
let mut n = NUMBERS.into_iter();
77+
n.next();
78+
79+
Ok(())
80+
}
81+
4482
fn main() {
4583
test_generic(10i32);
4684
test_generic2::<i32, i32>(10i32);
4785
test_questionmark().unwrap();
4886
test_issue_3913().unwrap();
4987
test_issue_5833().unwrap();
88+
test_issue_5833_alt1().unwrap();
89+
test_issue_5833_alt2().unwrap();
90+
test_issue_5833_alt3().unwrap();
91+
test_const_into_iter1().unwrap();
92+
test_const_into_iter2().unwrap();
5093

5194
let _: String = "foo".into();
5295
let _: String = From::from("foo");

‎tests/ui/useless_conversion.stderr

+36-12
Original file line numberDiff line numberDiff line change
@@ -22,71 +22,95 @@ 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:76:17
45+
|
46+
LL | let mut n = NUMBERS.into_iter();
47+
| ^^^^^^^^^^^^^^^^^^^ help: consider removing `.into_iter()`: `NUMBERS`
48+
2549
error: useless conversion to the same type: `std::string::String`
26-
--> $DIR/useless_conversion.rs:61:21
50+
--> $DIR/useless_conversion.rs:104:21
2751
|
2852
LL | let _: String = "foo".to_string().into();
2953
| ^^^^^^^^^^^^^^^^^^^^^^^^ help: consider removing `.into()`: `"foo".to_string()`
3054

3155
error: useless conversion to the same type: `std::string::String`
32-
--> $DIR/useless_conversion.rs:62:21
56+
--> $DIR/useless_conversion.rs:105:21
3357
|
3458
LL | let _: String = From::from("foo".to_string());
3559
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider removing `From::from()`: `"foo".to_string()`
3660

3761
error: useless conversion to the same type: `std::string::String`
38-
--> $DIR/useless_conversion.rs:63:13
62+
--> $DIR/useless_conversion.rs:106:13
3963
|
4064
LL | let _ = String::from("foo".to_string());
4165
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider removing `String::from()`: `"foo".to_string()`
4266

4367
error: useless conversion to the same type: `std::string::String`
44-
--> $DIR/useless_conversion.rs:64:13
68+
--> $DIR/useless_conversion.rs:107:13
4569
|
4670
LL | let _ = String::from(format!("A: {:04}", 123));
4771
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider removing `String::from()`: `format!("A: {:04}", 123)`
4872

4973
error: useless conversion to the same type: `std::str::Lines<'_>`
50-
--> $DIR/useless_conversion.rs:65:13
74+
--> $DIR/useless_conversion.rs:108:13
5175
|
5276
LL | let _ = "".lines().into_iter();
5377
| ^^^^^^^^^^^^^^^^^^^^^^ help: consider removing `.into_iter()`: `"".lines()`
5478

5579
error: useless conversion to the same type: `std::vec::IntoIter<i32>`
56-
--> $DIR/useless_conversion.rs:66:13
80+
--> $DIR/useless_conversion.rs:109:13
5781
|
5882
LL | let _ = vec![1, 2, 3].into_iter().into_iter();
5983
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider removing `.into_iter()`: `vec![1, 2, 3].into_iter()`
6084

6185
error: useless conversion to the same type: `std::string::String`
62-
--> $DIR/useless_conversion.rs:67:21
86+
--> $DIR/useless_conversion.rs:110:21
6387
|
6488
LL | let _: String = format!("Hello {}", "world").into();
6589
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider removing `.into()`: `format!("Hello {}", "world")`
6690

6791
error: useless conversion to the same type: `i32`
68-
--> $DIR/useless_conversion.rs:72:13
92+
--> $DIR/useless_conversion.rs:115:13
6993
|
7094
LL | let _ = i32::from(a + b) * 3;
7195
| ^^^^^^^^^^^^^^^^ help: consider removing `i32::from()`: `(a + b)`
7296

7397
error: useless conversion to the same type: `Foo<'a'>`
74-
--> $DIR/useless_conversion.rs:78:23
98+
--> $DIR/useless_conversion.rs:121:23
7599
|
76100
LL | let _: Foo<'a'> = s2.into();
77101
| ^^^^^^^^^ help: consider removing `.into()`: `s2`
78102

79103
error: useless conversion to the same type: `Foo<'a'>`
80-
--> $DIR/useless_conversion.rs:80:13
104+
--> $DIR/useless_conversion.rs:123:13
81105
|
82106
LL | let _ = Foo::<'a'>::from(s3);
83107
| ^^^^^^^^^^^^^^^^^^^^ help: consider removing `Foo::<'a'>::from()`: `s3`
84108

85109
error: useless conversion to the same type: `std::vec::IntoIter<Foo<'a'>>`
86-
--> $DIR/useless_conversion.rs:82:13
110+
--> $DIR/useless_conversion.rs:125:13
87111
|
88112
LL | let _ = vec![s4, s4, s4].into_iter().into_iter();
89113
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider removing `.into_iter()`: `vec![s4, s4, s4].into_iter()`
90114

91-
error: aborting due to 14 previous errors
115+
error: aborting due to 18 previous errors
92116

0 commit comments

Comments
 (0)