mikros/rust
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Identify more cases of useless into_iter() calls

Browse Source 
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`.
This commit is contained in:
Samuel Tardieu 2022-12-01 17:58:28 +01:00
parent 38fce1235a
commit af39a8a4a8
4 changed files with 200 additions and 28 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

26
clippy_lints/src/useless_conversion.rs
View File

@ -1,11 +1,11 @@
use clippy_utils::diagnostics::{span_lint_and_help, span_lint_and_sugg};
use clippy_utils::source::{snippet, snippet_with_macro_callsite};
use clippy_utils::sugg::Sugg;
use clippy_utils::ty::{is_type_diagnostic_item, same_type_and_consts};
use clippy_utils::{get_parent_expr, is_trait_method, match_def_path, paths};
use clippy_utils::ty::{is_copy, is_type_diagnostic_item, same_type_and_consts};
use clippy_utils::{get_parent_expr, is_trait_method, match_def_path, path_to_local, paths};
use if_chain::if_chain;
use rustc_errors::Applicability;
use rustc_hir::{Expr, ExprKind, HirId, MatchSource};
use rustc_hir::{BindingAnnotation, Expr, ExprKind, HirId, MatchSource, Node, PatKind};
use rustc_lint::{LateContext, LateLintPass};
use rustc_middle::ty;
use rustc_session::{declare_tool_lint, impl_lint_pass};
@ -81,16 +81,24 @@ fn check_expr(&mut self, cx: &LateContext<'tcx>, e: &'tcx Expr<'_>) {
}
}
if is_trait_method(cx, e, sym::IntoIterator) && name.ident.name == sym::into_iter {
if let Some(parent_expr) = get_parent_expr(cx, e) {
if let ExprKind::MethodCall(parent_name, ..) = parent_expr.kind {
if parent_name.ident.name != sym::into_iter {
if get_parent_expr(cx, e).is_some() &&
let Some(id) = path_to_local(recv) &&
let Node::Pat(pat) = cx.tcx.hir().get(id) &&
let PatKind::Binding(ann, ..) = pat.kind &&
ann != BindingAnnotation::MUT
{
// Do not remove .into_iter() applied to a non-mutable local variable used in
// a larger expression context as it would differ in mutability.
return;
}
}
}
let a = cx.typeck_results().expr_ty(e);
let b = cx.typeck_results().expr_ty(recv);
if same_type_and_consts(a, b) {
// If the types are identical then .into_iter() can be removed, unless the type
// implements Copy, in which case .into_iter() returns a copy of the receiver and
// cannot be safely omitted.
if same_type_and_consts(a, b) && !is_copy(cx, b) {
let sugg = snippet(cx, recv.span, "<expr>").into_owned();
span_lint_and_sugg(
cx,

73
tests/ui/useless_conversion.fixed
View File

@ -33,12 +33,71 @@ fn test_issue_3913() -> Result<(), std::io::Error> {
Ok(())
}
fn test_issue_5833() -> Result<(), ()> {
fn dont_lint_into_iter_on_immutable_local_implementing_iterator_in_expr() {
let text = "foo\r\nbar\n\nbaz\n";
let lines = text.lines();
if Some("ok") == lines.into_iter().next() {}
}
Ok(())
fn lint_into_iter_on_mutable_local_implementing_iterator_in_expr() {
let text = "foo\r\nbar\n\nbaz\n";
let mut lines = text.lines();
if Some("ok") == lines.next() {}
}
fn lint_into_iter_on_expr_implementing_iterator() {
let text = "foo\r\nbar\n\nbaz\n";
let mut lines = text.lines();
if Some("ok") == lines.next() {}
}
fn lint_into_iter_on_expr_implementing_iterator_2() {
let text = "foo\r\nbar\n\nbaz\n";
if Some("ok") == text.lines().next() {}
}
#[allow(const_item_mutation)]
fn lint_into_iter_on_const_implementing_iterator() {
const NUMBERS: std::ops::Range<i32> = 0..10;
let _ = NUMBERS.next();
}
fn lint_into_iter_on_const_implementing_iterator_2() {
const NUMBERS: std::ops::Range<i32> = 0..10;
let mut n = NUMBERS;
n.next();
}
#[derive(Clone, Copy)]
struct CopiableCounter {
counter: u32,
}
impl Iterator for CopiableCounter {
type Item = u32;
fn next(&mut self) -> Option<Self::Item> {
self.counter = self.counter.wrapping_add(1);
Some(self.counter)
}
}
fn dont_lint_into_iter_on_copy_iter() {
let mut c = CopiableCounter { counter: 0 };
assert_eq!(c.into_iter().next(), Some(1));
assert_eq!(c.into_iter().next(), Some(1));
assert_eq!(c.next(), Some(1));
assert_eq!(c.next(), Some(2));
}
fn dont_lint_into_iter_on_static_copy_iter() {
static mut C: CopiableCounter = CopiableCounter { counter: 0 };
unsafe {
assert_eq!(C.into_iter().next(), Some(1));
assert_eq!(C.into_iter().next(), Some(1));
assert_eq!(C.next(), Some(1));
assert_eq!(C.next(), Some(2));
}
}
fn main() {
@ -46,7 +105,15 @@ fn main() {
test_generic2::<i32, i32>(10i32);
test_questionmark().unwrap();
test_issue_3913().unwrap();
test_issue_5833().unwrap();
dont_lint_into_iter_on_immutable_local_implementing_iterator_in_expr();
lint_into_iter_on_mutable_local_implementing_iterator_in_expr();
lint_into_iter_on_expr_implementing_iterator();
lint_into_iter_on_expr_implementing_iterator_2();
lint_into_iter_on_const_implementing_iterator();
lint_into_iter_on_const_implementing_iterator_2();
dont_lint_into_iter_on_copy_iter();
dont_lint_into_iter_on_static_copy_iter();
let _: String = "foo".into();
let _: String = From::from("foo");

73
tests/ui/useless_conversion.rs
View File

@ -33,12 +33,71 @@ fn test_issue_3913() -> Result<(), std::io::Error> {
Ok(())
}
fn test_issue_5833() -> Result<(), ()> {
fn dont_lint_into_iter_on_immutable_local_implementing_iterator_in_expr() {
let text = "foo\r\nbar\n\nbaz\n";
let lines = text.lines();
if Some("ok") == lines.into_iter().next() {}
}
Ok(())
fn lint_into_iter_on_mutable_local_implementing_iterator_in_expr() {
let text = "foo\r\nbar\n\nbaz\n";
let mut lines = text.lines();
if Some("ok") == lines.into_iter().next() {}
}
fn lint_into_iter_on_expr_implementing_iterator() {
let text = "foo\r\nbar\n\nbaz\n";
let mut lines = text.lines().into_iter();
if Some("ok") == lines.next() {}
}
fn lint_into_iter_on_expr_implementing_iterator_2() {
let text = "foo\r\nbar\n\nbaz\n";
if Some("ok") == text.lines().into_iter().next() {}
}
#[allow(const_item_mutation)]
fn lint_into_iter_on_const_implementing_iterator() {
const NUMBERS: std::ops::Range<i32> = 0..10;
let _ = NUMBERS.into_iter().next();
}
fn lint_into_iter_on_const_implementing_iterator_2() {
const NUMBERS: std::ops::Range<i32> = 0..10;
let mut n = NUMBERS.into_iter();
n.next();
}
#[derive(Clone, Copy)]
struct CopiableCounter {
counter: u32,
}
impl Iterator for CopiableCounter {
type Item = u32;
fn next(&mut self) -> Option<Self::Item> {
self.counter = self.counter.wrapping_add(1);
Some(self.counter)
}
}
fn dont_lint_into_iter_on_copy_iter() {
let mut c = CopiableCounter { counter: 0 };
assert_eq!(c.into_iter().next(), Some(1));
assert_eq!(c.into_iter().next(), Some(1));
assert_eq!(c.next(), Some(1));
assert_eq!(c.next(), Some(2));
}
fn dont_lint_into_iter_on_static_copy_iter() {
static mut C: CopiableCounter = CopiableCounter { counter: 0 };
unsafe {
assert_eq!(C.into_iter().next(), Some(1));
assert_eq!(C.into_iter().next(), Some(1));
assert_eq!(C.next(), Some(1));
assert_eq!(C.next(), Some(2));
}
}
fn main() {
@ -46,7 +105,15 @@ fn main() {
test_generic2::<i32, i32>(10i32);
test_questionmark().unwrap();
test_issue_3913().unwrap();
test_issue_5833().unwrap();
dont_lint_into_iter_on_immutable_local_implementing_iterator_in_expr();
lint_into_iter_on_mutable_local_implementing_iterator_in_expr();
lint_into_iter_on_expr_implementing_iterator();
lint_into_iter_on_expr_implementing_iterator_2();
lint_into_iter_on_const_implementing_iterator();
lint_into_iter_on_const_implementing_iterator_2();
dont_lint_into_iter_on_copy_iter();
dont_lint_into_iter_on_static_copy_iter();
let _: String = "foo".into();
let _: String = From::from("foo");

54
tests/ui/useless_conversion.stderr
View File

@ -22,71 +22,101 @@ error: useless conversion to the same type: `i32`
LL | let _: i32 = 0i32.into();
| ^^^^^^^^^^^ help: consider removing `.into()`: `0i32`
error: useless conversion to the same type: `std::str::Lines<'_>`
--> $DIR/useless_conversion.rs:45:22
|
LL | if Some("ok") == lines.into_iter().next() {}
| ^^^^^^^^^^^^^^^^^ help: consider removing `.into_iter()`: `lines`
error: useless conversion to the same type: `std::str::Lines<'_>`
--> $DIR/useless_conversion.rs:50:21
|
LL | let mut lines = text.lines().into_iter();
| ^^^^^^^^^^^^^^^^^^^^^^^^ help: consider removing `.into_iter()`: `text.lines()`
error: useless conversion to the same type: `std::str::Lines<'_>`
--> $DIR/useless_conversion.rs:56:22
|
LL | if Some("ok") == text.lines().into_iter().next() {}
| ^^^^^^^^^^^^^^^^^^^^^^^^ help: consider removing `.into_iter()`: `text.lines()`
error: useless conversion to the same type: `std::ops::Range<i32>`
--> $DIR/useless_conversion.rs:62:13
|
LL | let _ = NUMBERS.into_iter().next();
| ^^^^^^^^^^^^^^^^^^^ help: consider removing `.into_iter()`: `NUMBERS`
error: useless conversion to the same type: `std::ops::Range<i32>`
--> $DIR/useless_conversion.rs:67:17
|
LL | let mut n = NUMBERS.into_iter();
| ^^^^^^^^^^^^^^^^^^^ help: consider removing `.into_iter()`: `NUMBERS`
error: useless conversion to the same type: `std::string::String`
--> $DIR/useless_conversion.rs:61:21
--> $DIR/useless_conversion.rs:128:21
|
LL | let _: String = "foo".to_string().into();
| ^^^^^^^^^^^^^^^^^^^^^^^^ help: consider removing `.into()`: `"foo".to_string()`
error: useless conversion to the same type: `std::string::String`
--> $DIR/useless_conversion.rs:62:21
--> $DIR/useless_conversion.rs:129:21
|
LL | let _: String = From::from("foo".to_string());
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider removing `From::from()`: `"foo".to_string()`
error: useless conversion to the same type: `std::string::String`
--> $DIR/useless_conversion.rs:63:13
--> $DIR/useless_conversion.rs:130:13
|
LL | let _ = String::from("foo".to_string());
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider removing `String::from()`: `"foo".to_string()`
error: useless conversion to the same type: `std::string::String`
--> $DIR/useless_conversion.rs:64:13
--> $DIR/useless_conversion.rs:131:13
|
LL | let _ = String::from(format!("A: {:04}", 123));
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider removing `String::from()`: `format!("A: {:04}", 123)`
error: useless conversion to the same type: `std::str::Lines<'_>`
--> $DIR/useless_conversion.rs:65:13
--> $DIR/useless_conversion.rs:132:13
|
LL | let _ = "".lines().into_iter();
| ^^^^^^^^^^^^^^^^^^^^^^ help: consider removing `.into_iter()`: `"".lines()`
error: useless conversion to the same type: `std::vec::IntoIter<i32>`
--> $DIR/useless_conversion.rs:66:13
--> $DIR/useless_conversion.rs:133:13
|
LL | let _ = vec![1, 2, 3].into_iter().into_iter();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider removing `.into_iter()`: `vec![1, 2, 3].into_iter()`
error: useless conversion to the same type: `std::string::String`
--> $DIR/useless_conversion.rs:67:21
--> $DIR/useless_conversion.rs:134:21
|
LL | let _: String = format!("Hello {}", "world").into();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider removing `.into()`: `format!("Hello {}", "world")`
error: useless conversion to the same type: `i32`
--> $DIR/useless_conversion.rs:72:13
--> $DIR/useless_conversion.rs:139:13
|
LL | let _ = i32::from(a + b) * 3;
| ^^^^^^^^^^^^^^^^ help: consider removing `i32::from()`: `(a + b)`
error: useless conversion to the same type: `Foo<'a'>`
--> $DIR/useless_conversion.rs:78:23
--> $DIR/useless_conversion.rs:145:23
|
LL | let _: Foo<'a'> = s2.into();
| ^^^^^^^^^ help: consider removing `.into()`: `s2`
error: useless conversion to the same type: `Foo<'a'>`
--> $DIR/useless_conversion.rs:80:13
--> $DIR/useless_conversion.rs:147:13
|
LL | let _ = Foo::<'a'>::from(s3);
| ^^^^^^^^^^^^^^^^^^^^ help: consider removing `Foo::<'a'>::from()`: `s3`
error: useless conversion to the same type: `std::vec::IntoIter<Foo<'a'>>`
--> $DIR/useless_conversion.rs:82:13
--> $DIR/useless_conversion.rs:149:13
|
LL | let _ = vec![s4, s4, s4].into_iter().into_iter();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider removing `.into_iter()`: `vec![s4, s4, s4].into_iter()`
error: aborting due to 14 previous errors
error: aborting due to 19 previous errors