user0/rust
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Remove method_calls

Browse source
This commit is contained in:
Yoshitomo Nakanishi 2021-03-13 15:11:39 +09:00
parent 527fbbef48
commit f2cc995bcf
5 changed files with 92 additions and 55 deletions
Download patch file Download diff file Expand all files Collapse all files

2
clippy_lints/src/lib.rs
View file

@ -1326,7 +1326,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
LintId::of(&missing_doc::MISSING_DOCS_IN_PRIVATE_ITEMS),
LintId::of(&missing_inline::MISSING_INLINE_IN_PUBLIC_ITEMS),
LintId::of(&modulo_arithmetic::MODULO_ARITHMETIC),
LintId::of(&needless_for_each::NEEDLESS_FOR_EACH),
LintId::of(&panic_in_result_fn::PANIC_IN_RESULT_FN),
LintId::of(&panic_unimplemented::PANIC),
LintId::of(&panic_unimplemented::TODO),
@ -1409,6 +1408,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
LintId::of(&misc_early::UNSEPARATED_LITERAL_SUFFIX),
LintId::of(&mut_mut::MUT_MUT),
LintId::of(&needless_continue::NEEDLESS_CONTINUE),
LintId::of(&needless_for_each::NEEDLESS_FOR_EACH),
LintId::of(&needless_pass_by_value::NEEDLESS_PASS_BY_VALUE),
LintId::of(&non_expressive_names::SIMILAR_NAMES),
LintId::of(&option_if_let_else::OPTION_IF_LET_ELSE),

40
clippy_lints/src/needless_for_each.rs
View file

@ -10,9 +10,7 @@ use rustc_span::{source_map::Span, sym, Symbol};
use if_chain::if_chain;
use crate::utils::{
has_iter_method, is_diagnostic_assoc_item, method_calls, snippet_with_applicability, span_lint_and_then,
};
use crate::utils::{has_iter_method, is_trait_method, snippet_with_applicability, span_lint_and_then};
declare_clippy_lint! {
/// **What it does:** Checks for usage of `for_each` that would be more simply written as a
@ -41,7 +39,7 @@ declare_clippy_lint! {
/// }
/// ```
pub NEEDLESS_FOR_EACH,
restriction,
pedantic,
"using `for_each` where a `for` loop would be simpler"
}
@ -55,22 +53,28 @@ impl LateLintPass<'_> for NeedlessForEach {
_ => return,
};
// Max depth is set to 3 because we need to check the method chain length is just two.
let (method_names, arg_lists, _) = method_calls(expr, 3);
if_chain! {
// This assures the length of this method chain is two.
if let [for_each_args, iter_args] = arg_lists.as_slice();
if let Some(for_each_sym) = method_names.first();
if *for_each_sym == Symbol::intern("for_each");
if let Some(did) = cx.typeck_results().type_dependent_def_id(expr.hir_id);
if is_diagnostic_assoc_item(cx, did, sym::Iterator);
// Checks the type of the first method receiver is NOT a user defined type.
if has_iter_method(cx, cx.typeck_results().expr_ty(&iter_args[0])).is_some();
if let ExprKind::Closure(_, _, body_id, ..) = for_each_args[1].kind;
let body = cx.tcx.hir().body(body_id);
// Check the method name is `for_each`.
if let ExprKind::MethodCall(method_name, _, for_each_args, _) = expr.kind;
if method_name.ident.name == Symbol::intern("for_each");
// Check `for_each` is an associated function of `Iterator`.
if is_trait_method(cx, expr, sym::Iterator);
// Checks the receiver of `for_each` is also a method call.
if let Some(for_each_receiver) = for_each_args.get(0);
if let ExprKind::MethodCall(_, _, iter_args, _) = for_each_receiver.kind;
// Skip the lint if the call chain is too long. e.g. `v.field.iter().for_each()` or
// `v.foo().iter().for_each()` must be skipped.
if let Some(iter_receiver) = iter_args.get(0);
if matches!(
iter_receiver.kind,
ExprKind::Array(..) | ExprKind::Call(..) | ExprKind::Path(..)
);
// Checks the type of the `iter` method receiver is NOT a user defined type.
if has_iter_method(cx, cx.typeck_results().expr_ty(&iter_receiver)).is_some();
// Skip the lint if the body is not block because this is simpler than `for` loop.
// e.g. `v.iter().for_each(f)` is simpler and clearer than using `for` loop.
if let ExprKind::Closure(_, _, body_id, ..) = for_each_args[1].kind;
let body = cx.tcx.hir().body(body_id);
if let ExprKind::Block(..) = body.value.kind;
then {
let mut ret_collector = RetCollector::default();
@ -99,7 +103,7 @@ impl LateLintPass<'_> for NeedlessForEach {
)));
for span in &ret_collector.spans {
suggs.push((*span, "return".to_string()));
suggs.push((*span, "continue".to_string()));
}
span_lint_and_then(

39
tests/ui/needless_for_each_fixable.fixed
View file

@ -14,6 +14,10 @@ fn should_lint() {
acc += elem;
}
for elem in [1, 2, 3].iter() {
acc += elem;
}
let mut hash_map: HashMap<i32, i32> = HashMap::new();
for (k, v) in hash_map.iter() {
acc += k + v;
@ -46,11 +50,30 @@ fn should_not_lint() {
}
v.iter().for_each(print);
// User defined type.
struct MyStruct {
v: Vec<i32>,
}
impl MyStruct {
fn iter(&self) -> impl Iterator<Item = &i32> {
self.v.iter()
}
}
let s = MyStruct { v: Vec::new() };
s.iter().for_each(|elem| {
acc += elem;
});
// `for_each` follows long iterator chain.
v.iter().chain(v.iter()).for_each(|v| println!("{}", v));
v.iter().chain(v.iter()).for_each(|v| {
acc += v;
});
v.as_slice().iter().for_each(|v| {
acc += v;
});
s.v.iter().for_each(|v| {
acc += v;
});
// `return` is used in `Loop` of the closure.
v.iter().for_each(|v| {
@ -68,20 +91,6 @@ fn should_not_lint() {
}
});
// User defined type.
struct MyStruct {
v: Vec<i32>,
}
impl MyStruct {
fn iter(&self) -> impl Iterator<Item = &i32> {
self.v.iter()
}
}
let s = MyStruct { v: Vec::new() };
s.iter().for_each(|elem| {
acc += elem;
});
// Previously transformed iterator variable.
let it = v.iter();
it.chain(v.iter()).for_each(|elem| {

39
tests/ui/needless_for_each_fixable.rs
View file

@ -14,6 +14,10 @@ fn should_lint() {
acc += elem;
});
[1, 2, 3].iter().for_each(|elem| {
acc += elem;
});
let mut hash_map: HashMap<i32, i32> = HashMap::new();
hash_map.iter().for_each(|(k, v)| {
acc += k + v;
@ -46,11 +50,30 @@ fn should_not_lint() {
}
v.iter().for_each(print);
// User defined type.
struct MyStruct {
v: Vec<i32>,
}
impl MyStruct {
fn iter(&self) -> impl Iterator<Item = &i32> {
self.v.iter()
}
}
let s = MyStruct { v: Vec::new() };
s.iter().for_each(|elem| {
acc += elem;
});
// `for_each` follows long iterator chain.
v.iter().chain(v.iter()).for_each(|v| println!("{}", v));
v.iter().chain(v.iter()).for_each(|v| {
acc += v;
});
v.as_slice().iter().for_each(|v| {
acc += v;
});
s.v.iter().for_each(|v| {
acc += v;
});
// `return` is used in `Loop` of the closure.
v.iter().for_each(|v| {
@ -68,20 +91,6 @@ fn should_not_lint() {
}
});
// User defined type.
struct MyStruct {
v: Vec<i32>,
}
impl MyStruct {
fn iter(&self) -> impl Iterator<Item = &i32> {
self.v.iter()
}
}
let s = MyStruct { v: Vec::new() };
s.iter().for_each(|elem| {
acc += elem;
});
// Previously transformed iterator variable.
let it = v.iter();
it.chain(v.iter()).for_each(|elem| {

27
tests/ui/needless_for_each_fixable.stderr
View file

@ -30,7 +30,22 @@ LL | }
|
error: needless use of `for_each`
--> $DIR/needless_for_each_fixable.rs:18:5
--> $DIR/needless_for_each_fixable.rs:17:5
|
LL | / [1, 2, 3].iter().for_each(|elem| {
LL | | acc += elem;
LL | | });
| |_______^
|
help: try
|
LL | for elem in [1, 2, 3].iter() {
LL | acc += elem;
LL | }
|
error: needless use of `for_each`
--> $DIR/needless_for_each_fixable.rs:22:5
|
LL | / hash_map.iter().for_each(|(k, v)| {
LL | | acc += k + v;
@ -45,7 +60,7 @@ LL | }
|
error: needless use of `for_each`
--> $DIR/needless_for_each_fixable.rs:21:5
--> $DIR/needless_for_each_fixable.rs:25:5
|
LL | / hash_map.iter_mut().for_each(|(k, v)| {
LL | | acc += *k + *v;
@ -60,7 +75,7 @@ LL | }
|
error: needless use of `for_each`
--> $DIR/needless_for_each_fixable.rs:24:5
--> $DIR/needless_for_each_fixable.rs:28:5
|
LL | / hash_map.keys().for_each(|k| {
LL | | acc += k;
@ -75,7 +90,7 @@ LL | }
|
error: needless use of `for_each`
--> $DIR/needless_for_each_fixable.rs:27:5
--> $DIR/needless_for_each_fixable.rs:31:5
|
LL | / hash_map.values().for_each(|v| {
LL | | acc += v;
@ -90,7 +105,7 @@ LL | }
|
error: needless use of `for_each`
--> $DIR/needless_for_each_fixable.rs:34:5
--> $DIR/needless_for_each_fixable.rs:38:5
|
LL | / my_vec().iter().for_each(|elem| {
LL | | acc += elem;
@ -104,5 +119,5 @@ LL | acc += elem;
LL | }
|
error: aborting due to 7 previous errors
error: aborting due to 8 previous errors