From 8906040445a6700be2aaf40ec09937b84dea5d6a Mon Sep 17 00:00:00 2001 From: Piti the little Light Date: Thu, 1 Oct 2020 18:04:05 +0200 Subject: [PATCH] Improvements from PR feedback --- clippy_lints/src/methods/mod.rs | 14 ++++++++------ clippy_lints/src/utils/paths.rs | 2 +- tests/ui/from_iter_instead_of_collect.rs | 10 +++------- tests/ui/from_iter_instead_of_collect.stderr | 15 ++++++--------- tests/ui/from_iter_instead_of_collect.stdout | 0 5 files changed, 18 insertions(+), 23 deletions(-) delete mode 100644 tests/ui/from_iter_instead_of_collect.stdout diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs index c0c39dae8..70be8909c 100644 --- a/clippy_lints/src/methods/mod.rs +++ b/clippy_lints/src/methods/mod.rs @@ -1370,10 +1370,11 @@ declare_clippy_lint! { } declare_clippy_lint! { - /// **What it does:** Checks for `from_iter()` function calls that implements `FromIterator` + /// **What it does:** Checks for `from_iter()` function calls on types that implement the `FromIterator` /// trait. /// - /// **Why is this bad?** Makes code less readable especially in method chaining. + /// **Why is this bad?** It is recommended style to use collect. See + /// [FromIterator documentation](https://doc.rust-lang.org/std/iter/trait.FromIterator.html) /// /// **Known problems:** None. /// @@ -3873,18 +3874,19 @@ fn lint_filetype_is_file(cx: &LateContext<'_>, expr: &hir::Expr<'_>, args: &[hir fn lint_from_iter(cx: &LateContext<'_>, expr: &hir::Expr<'_>, args: &[hir::Expr<'_>]) { let ty = cx.typeck_results().expr_ty(expr); - let id = get_trait_def_id(cx, &paths::FROM_ITERATOR_TRAIT).unwrap(); + let id = get_trait_def_id(cx, &paths::FROM_ITERATOR).unwrap(); if implements_trait(cx, ty, id, &[]) { // `expr` implements `FromIterator` trait let iter_expr = snippet(cx, args[0].span, ".."); - span_lint_and_help( + span_lint_and_sugg( cx, FROM_ITER_INSTEAD_OF_COLLECT, expr.span, "use `.collect()` instead of `::from_iter()`", - None, - &format!("consider using `{}.collect()`", iter_expr), + "consider using", + format!("`{}.collect()`", iter_expr), + Applicability::MaybeIncorrect ); } } diff --git a/clippy_lints/src/utils/paths.rs b/clippy_lints/src/utils/paths.rs index 3aade8ca8..8afbd8930 100644 --- a/clippy_lints/src/utils/paths.rs +++ b/clippy_lints/src/utils/paths.rs @@ -44,7 +44,7 @@ pub const FN: [&str; 3] = ["core", "ops", "Fn"]; pub const FN_MUT: [&str; 3] = ["core", "ops", "FnMut"]; pub const FN_ONCE: [&str; 3] = ["core", "ops", "FnOnce"]; pub const FROM_FROM: [&str; 4] = ["core", "convert", "From", "from"]; -pub const FROM_ITERATOR_TRAIT: [&str; 3] = ["std", "iter", "FromIterator"]; +pub const FROM_ITERATOR: [&str; 3] = ["std", "iter", "FromIterator"]; pub const FROM_TRAIT: [&str; 3] = ["core", "convert", "From"]; pub const FUTURE_FROM_GENERATOR: [&str; 3] = ["core", "future", "from_generator"]; pub const HASH: [&str; 3] = ["core", "hash", "Hash"]; diff --git a/tests/ui/from_iter_instead_of_collect.rs b/tests/ui/from_iter_instead_of_collect.rs index 7a1bc64f4..9071be33c 100644 --- a/tests/ui/from_iter_instead_of_collect.rs +++ b/tests/ui/from_iter_instead_of_collect.rs @@ -4,12 +4,8 @@ use std::collections::HashMap; use std::iter::FromIterator; fn main() { - { - let iter_expr = std::iter::repeat(5).take(5); + let iter_expr = std::iter::repeat(5).take(5); - Vec::from_iter(iter_expr); - HashMap::::from_iter(vec![5, 5, 5, 5].iter().enumerate()); - //let v: Vec = iter_expr.collect(); - let a: Vec = Vec::new(); - } + Vec::from_iter(iter_expr); + HashMap::::from_iter(vec![5, 5, 5, 5].iter().enumerate()); } diff --git a/tests/ui/from_iter_instead_of_collect.stderr b/tests/ui/from_iter_instead_of_collect.stderr index cfb92dfbb..1bc787aa7 100644 --- a/tests/ui/from_iter_instead_of_collect.stderr +++ b/tests/ui/from_iter_instead_of_collect.stderr @@ -1,19 +1,16 @@ error: use `.collect()` instead of `::from_iter()` - --> $DIR/from_iter_instead_of_collect.rs:10:9 + --> $DIR/from_iter_instead_of_collect.rs:9:5 | -LL | Vec::from_iter(iter_expr); - | ^^^^^^^^^^^^^^^^^^^^^^^^^ +LL | Vec::from_iter(iter_expr); + | ^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using: ``iter_expr.collect()`` | = note: `-D clippy::from-iter-instead-of-collect` implied by `-D warnings` - = help: consider using `iter_expr.collect()` error: use `.collect()` instead of `::from_iter()` - --> $DIR/from_iter_instead_of_collect.rs:11:9 + --> $DIR/from_iter_instead_of_collect.rs:10:5 | -LL | HashMap::::from_iter(vec![5, 5, 5, 5].iter().enumerate()); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ - | - = help: consider using `vec![5, 5, 5, 5].iter().enumerate().collect()` +LL | HashMap::::from_iter(vec![5, 5, 5, 5].iter().enumerate()); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using: ``vec![5, 5, 5, 5].iter().enumerate().collect()`` error: aborting due to 2 previous errors diff --git a/tests/ui/from_iter_instead_of_collect.stdout b/tests/ui/from_iter_instead_of_collect.stdout deleted file mode 100644 index e69de29bb..000000000