diff --git a/CHANGELOG.md b/CHANGELOG.md index be47eeaa0737..11a3e6a613f6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1732,6 +1732,7 @@ Released 2018-09-13 [`for_loops_over_fallibles`]: https://rust-lang.github.io/rust-clippy/master/index.html#for_loops_over_fallibles [`forget_copy`]: https://rust-lang.github.io/rust-clippy/master/index.html#forget_copy [`forget_ref`]: https://rust-lang.github.io/rust-clippy/master/index.html#forget_ref +[`from_iter_instead_of_collect`]: https://rust-lang.github.io/rust-clippy/master/index.html#from_iter_instead_of_collect [`future_not_send`]: https://rust-lang.github.io/rust-clippy/master/index.html#future_not_send [`get_last_with_len`]: https://rust-lang.github.io/rust-clippy/master/index.html#get_last_with_len [`get_unwrap`]: https://rust-lang.github.io/rust-clippy/master/index.html#get_unwrap diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index db31b213464a..643e56671db5 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -693,6 +693,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: &methods::FILTER_NEXT, &methods::FIND_MAP, &methods::FLAT_MAP_IDENTITY, + &methods::FROM_ITER_INSTEAD_OF_COLLECT, &methods::GET_UNWRAP, &methods::INEFFICIENT_TO_STRING, &methods::INTO_ITER_ON_REF, @@ -1422,6 +1423,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(&methods::EXPECT_FUN_CALL), LintId::of(&methods::FILTER_NEXT), LintId::of(&methods::FLAT_MAP_IDENTITY), + LintId::of(&methods::FROM_ITER_INSTEAD_OF_COLLECT), LintId::of(&methods::INTO_ITER_ON_REF), LintId::of(&methods::ITERATOR_STEP_BY_ZERO), LintId::of(&methods::ITER_CLONED_COLLECT), @@ -1620,6 +1622,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(&mem_replace::MEM_REPLACE_WITH_DEFAULT), LintId::of(&methods::CHARS_LAST_CMP), LintId::of(&methods::CHARS_NEXT_CMP), + LintId::of(&methods::FROM_ITER_INSTEAD_OF_COLLECT), LintId::of(&methods::INTO_ITER_ON_REF), LintId::of(&methods::ITER_CLONED_COLLECT), LintId::of(&methods::ITER_NEXT_SLICE), diff --git a/clippy_lints/src/lifetimes.rs b/clippy_lints/src/lifetimes.rs index c8a5a9c94313..4d737b3f49b0 100644 --- a/clippy_lints/src/lifetimes.rs +++ b/clippy_lints/src/lifetimes.rs @@ -16,7 +16,6 @@ use rustc_middle::hir::map::Map; use rustc_session::{declare_lint_pass, declare_tool_lint}; use rustc_span::source_map::Span; use rustc_span::symbol::{kw, Symbol}; -use std::iter::FromIterator; declare_clippy_lint! { /// **What it does:** Checks for lifetime annotations which can be removed by @@ -214,14 +213,15 @@ fn could_use_elision<'tcx>( } if allowed_lts - .intersection(&FxHashSet::from_iter( - input_visitor + .intersection( + &input_visitor .nested_elision_site_lts .iter() .chain(output_visitor.nested_elision_site_lts.iter()) .cloned() - .filter(|v| matches!(v, RefLt::Named(_))), - )) + .filter(|v| matches!(v, RefLt::Named(_))) + .collect(), + ) .next() .is_some() { diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs index 558c90249ccd..9ed2edd2a429 100644 --- a/clippy_lints/src/methods/mod.rs +++ b/clippy_lints/src/methods/mod.rs @@ -1371,6 +1371,39 @@ declare_clippy_lint! { "using `.map(_).collect::()`, which can be replaced with `try_for_each`" } +declare_clippy_lint! { + /// **What it does:** Checks for `from_iter()` function calls on types that implement the `FromIterator` + /// trait. + /// + /// **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. + /// + /// **Example:** + /// + /// ```rust + /// use std::iter::FromIterator; + /// + /// let five_fives = std::iter::repeat(5).take(5); + /// + /// let v = Vec::from_iter(five_fives); + /// + /// assert_eq!(v, vec![5, 5, 5, 5, 5]); + /// ``` + /// Use instead: + /// ```rust + /// let five_fives = std::iter::repeat(5).take(5); + /// + /// let v: Vec = five_fives.collect(); + /// + /// assert_eq!(v, vec![5, 5, 5, 5, 5]); + /// ``` + pub FROM_ITER_INSTEAD_OF_COLLECT, + style, + "use `.collect()` instead of `::from_iter()`" +} + declare_lint_pass!(Methods => [ UNWRAP_USED, EXPECT_USED, @@ -1421,6 +1454,7 @@ declare_lint_pass!(Methods => [ OPTION_AS_REF_DEREF, UNNECESSARY_LAZY_EVALUATIONS, MAP_COLLECT_RESULT_UNIT, + FROM_ITER_INSTEAD_OF_COLLECT, ]); impl<'tcx> LateLintPass<'tcx> for Methods { @@ -1507,6 +1541,13 @@ impl<'tcx> LateLintPass<'tcx> for Methods { } match expr.kind { + hir::ExprKind::Call(ref func, ref args) => { + if let hir::ExprKind::Path(path) = &func.kind { + if match_qpath(path, &["from_iter"]) { + lint_from_iter(cx, expr, args); + } + } + }, hir::ExprKind::MethodCall(ref method_call, ref method_span, ref args, _) => { lint_or_fun_call(cx, expr, *method_span, &method_call.ident.as_str(), args); lint_expect_fun_call(cx, expr, *method_span, &method_call.ident.as_str(), args); @@ -3856,6 +3897,28 @@ fn lint_filetype_is_file(cx: &LateContext<'_>, expr: &hir::Expr<'_>, args: &[hir span_lint_and_help(cx, FILETYPE_IS_FILE, span, &lint_msg, None, &help_msg); } +fn lint_from_iter(cx: &LateContext<'_>, expr: &hir::Expr<'_>, args: &[hir::Expr<'_>]) { + let ty = cx.typeck_results().expr_ty(expr); + let arg_ty = cx.typeck_results().expr_ty(&args[0]); + + let from_iter_id = get_trait_def_id(cx, &paths::FROM_ITERATOR).unwrap(); + let iter_id = get_trait_def_id(cx, &paths::ITERATOR).unwrap(); + + if implements_trait(cx, ty, from_iter_id, &[]) && implements_trait(cx, arg_ty, iter_id, &[]) { + // `expr` implements `FromIterator` trait + let iter_expr = snippet(cx, args[0].span, ".."); + span_lint_and_sugg( + cx, + FROM_ITER_INSTEAD_OF_COLLECT, + expr.span, + "usage of `FromIterator::from_iter`", + "use `.collect()` instead of `::from_iter()`", + format!("{}.collect()", iter_expr), + Applicability::MaybeIncorrect, + ); + } +} + fn fn_header_equals(expected: hir::FnHeader, actual: hir::FnHeader) -> bool { expected.constness == actual.constness && expected.unsafety == actual.unsafety diff --git a/clippy_lints/src/utils/paths.rs b/clippy_lints/src/utils/paths.rs index a6e9c2331896..1ad8c6029860 100644 --- a/clippy_lints/src/utils/paths.rs +++ b/clippy_lints/src/utils/paths.rs @@ -44,6 +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: [&str; 5] = ["core", "iter", "traits", "collect", "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/src/lintlist/mod.rs b/src/lintlist/mod.rs index b22bb0742387..44d2dd746d59 100644 --- a/src/lintlist/mod.rs +++ b/src/lintlist/mod.rs @@ -753,6 +753,13 @@ vec![ deprecation: None, module: "drop_forget_ref", }, + Lint { + name: "from_iter_instead_of_collect", + group: "style", + desc: "use `.collect()` instead of `::from_iter()`", + deprecation: None, + module: "methods", + }, Lint { name: "future_not_send", group: "nursery", diff --git a/tests/ui/from_iter_instead_of_collect.rs b/tests/ui/from_iter_instead_of_collect.rs new file mode 100644 index 000000000000..045eb3133d3c --- /dev/null +++ b/tests/ui/from_iter_instead_of_collect.rs @@ -0,0 +1,13 @@ +#![warn(clippy::from_iter_instead_of_collect)] + +use std::collections::HashMap; +use std::iter::FromIterator; + +fn main() { + let iter_expr = std::iter::repeat(5).take(5); + Vec::from_iter(iter_expr); + + HashMap::::from_iter(vec![5, 5, 5, 5].iter().enumerate()); + + Vec::from_iter(vec![42u32]); +} diff --git a/tests/ui/from_iter_instead_of_collect.stderr b/tests/ui/from_iter_instead_of_collect.stderr new file mode 100644 index 000000000000..46bdc2f4e199 --- /dev/null +++ b/tests/ui/from_iter_instead_of_collect.stderr @@ -0,0 +1,16 @@ +error: usage of `FromIterator::from_iter` + --> $DIR/from_iter_instead_of_collect.rs:8:5 + | +LL | Vec::from_iter(iter_expr); + | ^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `.collect()` instead of `::from_iter()`: `iter_expr.collect()` + | + = note: `-D clippy::from-iter-instead-of-collect` implied by `-D warnings` + +error: usage of `FromIterator::from_iter` + --> $DIR/from_iter_instead_of_collect.rs:10:5 + | +LL | HashMap::::from_iter(vec![5, 5, 5, 5].iter().enumerate()); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `.collect()` instead of `::from_iter()`: `vec![5, 5, 5, 5].iter().enumerate().collect()` + +error: aborting due to 2 previous errors + diff --git a/tests/ui/get_unwrap.fixed b/tests/ui/get_unwrap.fixed index 97e6b20f471f..924c02a4054d 100644 --- a/tests/ui/get_unwrap.fixed +++ b/tests/ui/get_unwrap.fixed @@ -1,5 +1,5 @@ // run-rustfix -#![allow(unused_mut)] +#![allow(unused_mut, clippy::from_iter_instead_of_collect)] #![deny(clippy::get_unwrap)] use std::collections::BTreeMap; diff --git a/tests/ui/get_unwrap.rs b/tests/ui/get_unwrap.rs index 1c9a71c09699..c0c37bb72066 100644 --- a/tests/ui/get_unwrap.rs +++ b/tests/ui/get_unwrap.rs @@ -1,5 +1,5 @@ // run-rustfix -#![allow(unused_mut)] +#![allow(unused_mut, clippy::from_iter_instead_of_collect)] #![deny(clippy::get_unwrap)] use std::collections::BTreeMap;