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

Changes to iter_overeager_cloned

Browse Source 
* Don't lint on `.cloned().flatten()` when `T::Item` doesn't implement `IntoIterator`
* Reduce verbosity of lint message
* Narrow down the scope of the replacement range
This commit is contained in:
Jason Newcomb 2022-06-07 00:40:32 -04:00
parent 72f5ff6903
commit bf3ab592f0
6 changed files with 92 additions and 84 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

97
clippy_lints/src/methods/iter_overeager_cloned.rs
View File

@ -1,70 +1,59 @@
use clippy_utils::diagnostics::span_lint_and_sugg;
use clippy_utils::source::snippet;
use clippy_utils::ty::{get_iterator_item_ty, implements_trait, is_copy};
use itertools::Itertools;
use clippy_utils::diagnostics::span_lint_and_then;
use clippy_utils::source::snippet_opt;
use clippy_utils::ty::{get_associated_type, implements_trait, is_copy};
use rustc_errors::Applicability;
use rustc_hir as hir;
use rustc_hir::Expr;
use rustc_lint::LateContext;
use rustc_middle::ty;
use rustc_span::sym;
use std::ops::Not;
use super::ITER_OVEREAGER_CLONED;
use crate::redundant_clone::REDUNDANT_CLONE;
/// lint overeager use of `cloned()` for `Iterator`s
pub(super) fn check<'tcx>(
cx: &LateContext<'tcx>,
expr: &'tcx hir::Expr<'_>,
recv: &'tcx hir::Expr<'_>,
name: &str,
map_arg: &[hir::Expr<'_>],
expr: &'tcx Expr<'_>,
cloned_call: &'tcx Expr<'_>,
cloned_recv: &'tcx Expr<'_>,
is_count: bool,
needs_into_iter: bool,
) {
// Check if it's iterator and get type associated with `Item`.
let inner_ty = if_chain! {
if let Some(iterator_trait_id) = cx.tcx.get_diagnostic_item(sym::Iterator);
let recv_ty = cx.typeck_results().expr_ty(recv);
if implements_trait(cx, recv_ty, iterator_trait_id, &[]);
if let Some(inner_ty) = get_iterator_item_ty(cx, cx.typeck_results().expr_ty_adjusted(recv));
then {
inner_ty
} else {
let typeck = cx.typeck_results();
if let Some(iter_id) = cx.tcx.get_diagnostic_item(sym::Iterator)
&& let Some(method_id) = typeck.type_dependent_def_id(expr.hir_id)
&& cx.tcx.trait_of_item(method_id) == Some(iter_id)
&& let Some(method_id) = typeck.type_dependent_def_id(cloned_call.hir_id)
&& cx.tcx.trait_of_item(method_id) == Some(iter_id)
&& let cloned_recv_ty = typeck.expr_ty_adjusted(cloned_recv)
&& let Some(iter_assoc_ty) = get_associated_type(cx, cloned_recv_ty, iter_id, "Item")
&& matches!(*iter_assoc_ty.kind(), ty::Ref(_, ty, _) if !is_copy(cx, ty))
{
if needs_into_iter
&& let Some(into_iter_id) = cx.tcx.get_diagnostic_item(sym::IntoIterator)
&& !implements_trait(cx, iter_assoc_ty, into_iter_id, &[])
{
return;
}
};
match inner_ty.kind() {
ty::Ref(_, ty, _) if !is_copy(cx, *ty) => {},
_ => return,
};
let (lint, msg, trailing_clone) = if is_count {
(REDUNDANT_CLONE, "unneeded cloning of iterator items", "")
} else {
(ITER_OVEREAGER_CLONED, "unnecessarily eager cloning of iterator items", ".cloned()")
};
let (lint, preserve_cloned) = match name {
"count" => (REDUNDANT_CLONE, false),
_ => (ITER_OVEREAGER_CLONED, true),
};
let wildcard_params = map_arg.is_empty().not().then(|| "...").unwrap_or_default();
let msg = format!(
"called `cloned().{}({})` on an `Iterator`. It may be more efficient to call `{}({}){}` instead",
name,
wildcard_params,
name,
wildcard_params,
preserve_cloned.then(|| ".cloned()").unwrap_or_default(),
);
span_lint_and_sugg(
cx,
lint,
expr.span,
&msg,
"try this",
format!(
"{}.{}({}){}",
snippet(cx, recv.span, ".."),
name,
map_arg.iter().map(|a| snippet(cx, a.span, "..")).join(", "),
preserve_cloned.then(|| ".cloned()").unwrap_or_default(),
),
Applicability::MachineApplicable,
);
span_lint_and_then(
cx,
lint,
expr.span,
msg,
|diag| {
let method_span = expr.span.with_lo(cloned_call.span.hi());
if let Some(mut snip) = snippet_opt(cx, method_span) {
snip.push_str(trailing_clone);
let replace_span = expr.span.with_lo(cloned_recv.span.hi());
diag.span_suggestion(replace_span, "try this", snip, Applicability::MachineApplicable);
}
}
);
}
}

24
clippy_lints/src/methods/mod.rs
View File

@ -2583,8 +2583,8 @@ fn check_methods<'tcx>(&self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
},
_ => {},
},
(name @ "count", args @ []) => match method_call(recv) {
Some(("cloned", [recv2], _)) => iter_overeager_cloned::check(cx, expr, recv2, name, args),
("count", []) => match method_call(recv) {
Some(("cloned", [recv2], _)) => iter_overeager_cloned::check(cx, expr, recv, recv2, true, false),
Some((name2 @ ("into_iter" | "iter" | "iter_mut"), [recv2], _)) => {
iter_count::check(cx, expr, recv2, name2);
},
@ -2614,9 +2614,9 @@ fn check_methods<'tcx>(&self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
flat_map_identity::check(cx, expr, arg, span);
flat_map_option::check(cx, expr, arg, span);
},
(name @ "flatten", args @ []) => match method_call(recv) {
("flatten", []) => match method_call(recv) {
Some(("map", [recv, map_arg], map_span)) => map_flatten::check(cx, expr, recv, map_arg, map_span),
Some(("cloned", [recv2], _)) => iter_overeager_cloned::check(cx, expr, recv2, name, args),
Some(("cloned", [recv2], _)) => iter_overeager_cloned::check(cx, expr, recv, recv2, false, true),
_ => {},
},
("fold", [init, acc]) => unnecessary_fold::check(cx, expr, init, acc, span),
@ -2636,10 +2636,10 @@ fn check_methods<'tcx>(&self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
unnecessary_join::check(cx, expr, recv, join_arg, span);
}
},
("last", args @ []) | ("skip", args @ [_]) => {
("last", []) | ("skip", [_]) => {
if let Some((name2, [recv2, args2 @ ..], _span2)) = method_call(recv) {
if let ("cloned", []) = (name2, args2) {
iter_overeager_cloned::check(cx, expr, recv2, name, args);
iter_overeager_cloned::check(cx, expr, recv, recv2, false, false);
}
}
},
@ -2660,10 +2660,10 @@ fn check_methods<'tcx>(&self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
map_identity::check(cx, expr, recv, m_arg, name, span);
},
("map_or", [def, map]) => option_map_or_none::check(cx, expr, recv, def, map),
(name @ "next", args @ []) => {
("next", []) => {
if let Some((name2, [recv2, args2 @ ..], _)) = method_call(recv) {
match (name2, args2) {
("cloned", []) => iter_overeager_cloned::check(cx, expr, recv2, name, args),
("cloned", []) => iter_overeager_cloned::check(cx, expr, recv, recv2, false, false),
("filter", [arg]) => filter_next::check(cx, expr, recv2, arg),
("filter_map", [arg]) => filter_map_next::check(cx, expr, recv2, arg, self.msrv),
("iter", []) => iter_next_slice::check(cx, expr, recv2),
@ -2673,9 +2673,9 @@ fn check_methods<'tcx>(&self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
}
}
},
("nth", args @ [n_arg]) => match method_call(recv) {
("nth", [n_arg]) => match method_call(recv) {
Some(("bytes", [recv2], _)) => bytes_nth::check(cx, expr, recv2, n_arg),
Some(("cloned", [recv2], _)) => iter_overeager_cloned::check(cx, expr, recv2, name, args),
Some(("cloned", [recv2], _)) => iter_overeager_cloned::check(cx, expr, recv, recv2, false, false),
Some(("iter", [recv2], _)) => iter_nth::check(cx, expr, recv2, recv, n_arg, false),
Some(("iter_mut", [recv2], _)) => iter_nth::check(cx, expr, recv2, recv, n_arg, true),
_ => iter_nth_zero::check(cx, expr, recv, n_arg),
@ -2698,10 +2698,10 @@ fn check_methods<'tcx>(&self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
}
},
("step_by", [arg]) => iterator_step_by_zero::check(cx, expr, arg),
("take", args @ [_arg]) => {
("take", [_arg]) => {
if let Some((name2, [recv2, args2 @ ..], _span2)) = method_call(recv) {
if let ("cloned", []) = (name2, args2) {
iter_overeager_cloned::check(cx, expr, recv2, name, args);
iter_overeager_cloned::check(cx, expr, recv, recv2, false, false);
}
}
},

4
clippy_utils/src/ty.rs
View File

@ -78,9 +78,9 @@ pub fn get_associated_type<'tcx>(
cx.tcx
.associated_items(trait_id)
.find_by_name_and_kind(cx.tcx, Ident::from_str(name), ty::AssocKind::Type, trait_id)
.map(|assoc| {
.and_then(|assoc| {
let proj = cx.tcx.mk_projection(assoc.def_id, cx.tcx.mk_substs_trait(ty, &[]));
cx.tcx.normalize_erasing_regions(cx.param_env, proj)
cx.tcx.try_normalize_erasing_regions(cx.param_env, proj).ok()
})
}

6
tests/ui/iter_overeager_cloned.fixed
View File

@ -18,7 +18,8 @@ fn main() {
let _ = vec.iter().filter(|x| x == &"2").nth(2).cloned();
let _ = [Some(Some("str".to_string())), Some(Some("str".to_string()))]
.iter().flatten().cloned();
.iter()
.flatten().cloned();
// Not implemented yet
let _ = vec.iter().cloned().filter(|x| x.starts_with('2'));
@ -43,6 +44,9 @@ fn main() {
// Should probably stay as it is.
let _ = [0, 1, 2, 3, 4].iter().cloned().take(10);
// `&Range<_>` doesn't implement `IntoIterator`
let _ = [0..1, 2..5].iter().cloned().flatten();
}
// #8527

3
tests/ui/iter_overeager_cloned.rs
View File

@ -45,6 +45,9 @@ fn main() {
// Should probably stay as it is.
let _ = [0, 1, 2, 3, 4].iter().cloned().take(10);
// `&Range<_>` doesn't implement `IntoIterator`
let _ = [0..1, 2..5].iter().cloned().flatten();
}
// #8527

42
tests/ui/iter_overeager_cloned.stderr
View File

@ -1,44 +1,56 @@
error: called `cloned().last()` on an `Iterator`. It may be more efficient to call `last().cloned()` instead
error: unnecessarily eager cloning of iterator items
--> $DIR/iter_overeager_cloned.rs:8:29
|
LL | let _: Option<String> = vec.iter().cloned().last();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `vec.iter().last().cloned()`
| ^^^^^^^^^^----------------
| |
| help: try this: `.last().cloned()`
|
= note: `-D clippy::iter-overeager-cloned` implied by `-D warnings`
error: called `cloned().next()` on an `Iterator`. It may be more efficient to call `next().cloned()` instead
error: unnecessarily eager cloning of iterator items
--> $DIR/iter_overeager_cloned.rs:10:29
|
LL | let _: Option<String> = vec.iter().chain(vec.iter()).cloned().next();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `vec.iter().chain(vec.iter()).next().cloned()`
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^----------------
| |
| help: try this: `.next().cloned()`
error: called `cloned().count()` on an `Iterator`. It may be more efficient to call `count()` instead
error: unneeded cloning of iterator items
--> $DIR/iter_overeager_cloned.rs:12:20
|
LL | let _: usize = vec.iter().filter(|x| x == &"2").cloned().count();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `vec.iter().filter(|x| x == &"2").count()`
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^-----------------
| |
| help: try this: `.count()`
|
= note: `-D clippy::redundant-clone` implied by `-D warnings`
error: called `cloned().take(...)` on an `Iterator`. It may be more efficient to call `take(...).cloned()` instead
error: unnecessarily eager cloning of iterator items
--> $DIR/iter_overeager_cloned.rs:14:21
|
LL | let _: Vec<_> = vec.iter().cloned().take(2).collect();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `vec.iter().take(2).cloned()`
| ^^^^^^^^^^-----------------
| |
| help: try this: `.take(2).cloned()`
error: called `cloned().skip(...)` on an `Iterator`. It may be more efficient to call `skip(...).cloned()` instead
error: unnecessarily eager cloning of iterator items
--> $DIR/iter_overeager_cloned.rs:16:21
|
LL | let _: Vec<_> = vec.iter().cloned().skip(2).collect();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `vec.iter().skip(2).cloned()`
| ^^^^^^^^^^-----------------
| |
| help: try this: `.skip(2).cloned()`
error: called `cloned().nth(...)` on an `Iterator`. It may be more efficient to call `nth(...).cloned()` instead
error: unnecessarily eager cloning of iterator items
--> $DIR/iter_overeager_cloned.rs:18:13
|
LL | let _ = vec.iter().filter(|x| x == &"2").cloned().nth(2);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `vec.iter().filter(|x| x == &"2").nth(2).cloned()`
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^----------------
| |
| help: try this: `.nth(2).cloned()`
error: called `cloned().flatten()` on an `Iterator`. It may be more efficient to call `flatten().cloned()` instead
error: unnecessarily eager cloning of iterator items
--> $DIR/iter_overeager_cloned.rs:20:13
|
LL | let _ = [Some(Some("str".to_string())), Some(Some("str".to_string()))]
@ -50,8 +62,8 @@ LL | | .flatten();
|
help: try this
|
LL ~ let _ = [Some(Some("str".to_string())), Some(Some("str".to_string()))]
LL ~ .iter().flatten().cloned();
LL ~ .iter()
LL ~ .flatten().cloned();
|
error: aborting due to 7 previous errors