From 5210a058a3322282594f4c2fc2624f9d6f676c63 Mon Sep 17 00:00:00 2001 From: Yoav Lavi Date: Tue, 22 Mar 2022 10:48:16 +0100 Subject: [PATCH 01/35] add unnecessary join lint --- CHANGELOG.md | 1 + clippy_lints/src/lib.register_all.rs | 1 + clippy_lints/src/lib.register_lints.rs | 1 + clippy_lints/src/lib.register_perf.rs | 1 + clippy_lints/src/methods/mod.rs | 32 ++++++++++ clippy_lints/src/methods/unnecessary_join.rs | 63 ++++++++++++++++++++ 6 files changed, 99 insertions(+) create mode 100644 clippy_lints/src/methods/unnecessary_join.rs diff --git a/CHANGELOG.md b/CHANGELOG.md index 04e072d0abf6..c4eabbe025a4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3507,6 +3507,7 @@ Released 2018-09-13 [`unnecessary_filter_map`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_filter_map [`unnecessary_find_map`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_find_map [`unnecessary_fold`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_fold +[`unnecessary_join`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_join [`unnecessary_lazy_evaluations`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_lazy_evaluations [`unnecessary_mut_passed`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_mut_passed [`unnecessary_operation`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_operation diff --git a/clippy_lints/src/lib.register_all.rs b/clippy_lints/src/lib.register_all.rs index e360757f781f..cf9c928deee9 100644 --- a/clippy_lints/src/lib.register_all.rs +++ b/clippy_lints/src/lib.register_all.rs @@ -192,6 +192,7 @@ store.register_group(true, "clippy::all", Some("clippy_all"), vec![ LintId::of(methods::SUSPICIOUS_MAP), LintId::of(methods::SUSPICIOUS_SPLITN), LintId::of(methods::UNINIT_ASSUMED_INIT), + LintId::of(methods::UNNECESSARY_JOIN), LintId::of(methods::UNNECESSARY_FILTER_MAP), LintId::of(methods::UNNECESSARY_FIND_MAP), LintId::of(methods::UNNECESSARY_FOLD), diff --git a/clippy_lints/src/lib.register_lints.rs b/clippy_lints/src/lib.register_lints.rs index a1a9ca37c95b..84cf95df500a 100644 --- a/clippy_lints/src/lib.register_lints.rs +++ b/clippy_lints/src/lib.register_lints.rs @@ -330,6 +330,7 @@ store.register_lints(&[ methods::SUSPICIOUS_MAP, methods::SUSPICIOUS_SPLITN, methods::UNINIT_ASSUMED_INIT, + methods::UNNECESSARY_JOIN, methods::UNNECESSARY_FILTER_MAP, methods::UNNECESSARY_FIND_MAP, methods::UNNECESSARY_FOLD, diff --git a/clippy_lints/src/lib.register_perf.rs b/clippy_lints/src/lib.register_perf.rs index f2f5c988d8f9..c3bdd16acc49 100644 --- a/clippy_lints/src/lib.register_perf.rs +++ b/clippy_lints/src/lib.register_perf.rs @@ -19,6 +19,7 @@ store.register_group(true, "clippy::perf", Some("clippy_perf"), vec![ LintId::of(methods::MANUAL_STR_REPEAT), LintId::of(methods::OR_FUN_CALL), LintId::of(methods::SINGLE_CHAR_PATTERN), + LintId::of(methods::UNNECESSARY_JOIN), LintId::of(methods::UNNECESSARY_TO_OWNED), LintId::of(misc::CMP_OWNED), LintId::of(redundant_clone::REDUNDANT_CLONE), diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs index aa9f86f292c0..272688035599 100644 --- a/clippy_lints/src/methods/mod.rs +++ b/clippy_lints/src/methods/mod.rs @@ -59,6 +59,7 @@ mod uninit_assumed_init; mod unnecessary_filter_map; mod unnecessary_fold; mod unnecessary_iter_cloned; +mod unnecessary_join; mod unnecessary_lazy_eval; mod unnecessary_to_owned; mod unwrap_or_else_default; @@ -2012,6 +2013,31 @@ declare_clippy_lint! { "unnecessary calls to `to_owned`-like functions" } +declare_clippy_lint! { + /// ### What it does + /// Checks for use of `.collect::>.join(\"\")` on iterators. + /// + /// ### Why is this bad? + /// `.collect::` is more performant and cleaner + /// + /// ### Example + /// ```rust + /// let vector = vec!["hello", "world"]; + /// let output = vector.iter().map(|item| item.to_uppercase()).collect::>().join(""); + /// println!("{}", output); + /// ``` + /// The correct use would be: + /// ```rust + /// let vector = vec!["hello", "world"]; + /// let output = vector.iter().map(|item| item.to_uppercase()).collect::(); + /// println!("{}", output); + /// ``` + #[clippy::version = "pre 1.29.0"] + pub UNNECESSARY_JOIN, + perf, + "using `.collect::>.join(\"\")` on an iterator" +} + pub struct Methods { avoid_breaking_exported_api: bool, msrv: Option, @@ -2096,6 +2122,7 @@ impl_lint_pass!(Methods => [ MANUAL_SPLIT_ONCE, NEEDLESS_SPLITN, UNNECESSARY_TO_OWNED, + UNNECESSARY_JOIN, ]); /// Extracts a method call name, args, and `Span` of the method name. @@ -2391,6 +2418,11 @@ fn check_methods<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>, msrv: Optio ("is_file", []) => filetype_is_file::check(cx, expr, recv), ("is_none", []) => check_is_some_is_none(cx, expr, recv, false), ("is_some", []) => check_is_some_is_none(cx, expr, recv, true), + ("join", [_arg]) => { + if let Some(("collect", ..)) = method_call(recv) { + unnecessary_join::check(cx, recv); + } + }, ("last", args @ []) | ("skip", args @ [_]) => { if let Some((name2, [recv2, args2 @ ..], _span2)) = method_call(recv) { if let ("cloned", []) = (name2, args2) { diff --git a/clippy_lints/src/methods/unnecessary_join.rs b/clippy_lints/src/methods/unnecessary_join.rs new file mode 100644 index 000000000000..bdd08dfb148a --- /dev/null +++ b/clippy_lints/src/methods/unnecessary_join.rs @@ -0,0 +1,63 @@ +use clippy_utils::ty::is_type_diagnostic_item; +use clippy_utils::{get_parent_expr, get_parent_node}; +use hir::ExprKind; +use rustc_hir as hir; +use rustc_lint::LateContext; +use rustc_span::sym; + +// use super::UNNECESSARY_JOIN; + +pub(super) fn check<'tcx>(context: &LateContext<'tcx>, expression: &'tcx hir::Expr<'tcx>) { + // this section should not pass the dogfood test + // TODO: remove and use a proper test + let vector = vec!["hello", "world"]; + let output = vector + .iter() + .map(|item| item.to_uppercase()) + .collect::>() + .join(""); + println!("{}", output); + + // let mut applicability = Applicability::MachineApplicable; + + let parent = get_parent_node(context.tcx, expression.hir_id); + + if parent.is_none() { + return; + } + + if_chain! { + let current_ty = context.typeck_results().expr_ty(expression); + // the current join method is being called on a vector + // e.g .join("") + if is_type_diagnostic_item(context, current_ty, sym::Vec); + if let Some(parent) = get_parent_expr(context, expression); + if let ExprKind::MethodCall(_, [self_arg, ..], _) = &parent.kind; + // the parent collect method is being called on an iterator + // e.g. .collect>() + let caller_ty = context.typeck_results().expr_ty(self_arg); + if is_type_diagnostic_item(context, caller_ty, sym::Vec); + + // check that the argument for join is an empty string + // check that the turbofish for collect is > or > if the iterator has String items + then { + // span_lint_and_sugg( + // cx, + // UNNECESSARY_JOIN, + // span, + // &format!( + // "called `.collect>().join("")` on a {1}. Using `.collect::()` is + // more clear and more concise", caller_type + // ), + // "try this", + // format!( + // "{}.collect::()", + // snippet_with_applicability(cx, recv.span, "..", &mut applicability), + // ), + // applicability, + // ); + dbg!("{:#?} {:#?}", expression, parent); + panic!(".collect().join() called on an iterator"); + } + } +} From 4546e1c76760b64494fdec14a36ca09c762074b7 Mon Sep 17 00:00:00 2001 From: Yoav Lavi Date: Tue, 22 Mar 2022 10:50:09 +0100 Subject: [PATCH 02/35] update lints --- clippy_lints/src/lib.register_all.rs | 2 +- clippy_lints/src/lib.register_lints.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/clippy_lints/src/lib.register_all.rs b/clippy_lints/src/lib.register_all.rs index cf9c928deee9..5d0d2c214bee 100644 --- a/clippy_lints/src/lib.register_all.rs +++ b/clippy_lints/src/lib.register_all.rs @@ -192,10 +192,10 @@ store.register_group(true, "clippy::all", Some("clippy_all"), vec![ LintId::of(methods::SUSPICIOUS_MAP), LintId::of(methods::SUSPICIOUS_SPLITN), LintId::of(methods::UNINIT_ASSUMED_INIT), - LintId::of(methods::UNNECESSARY_JOIN), LintId::of(methods::UNNECESSARY_FILTER_MAP), LintId::of(methods::UNNECESSARY_FIND_MAP), LintId::of(methods::UNNECESSARY_FOLD), + LintId::of(methods::UNNECESSARY_JOIN), LintId::of(methods::UNNECESSARY_LAZY_EVALUATIONS), LintId::of(methods::UNNECESSARY_TO_OWNED), LintId::of(methods::UNWRAP_OR_ELSE_DEFAULT), diff --git a/clippy_lints/src/lib.register_lints.rs b/clippy_lints/src/lib.register_lints.rs index 84cf95df500a..cd53ece5baba 100644 --- a/clippy_lints/src/lib.register_lints.rs +++ b/clippy_lints/src/lib.register_lints.rs @@ -330,10 +330,10 @@ store.register_lints(&[ methods::SUSPICIOUS_MAP, methods::SUSPICIOUS_SPLITN, methods::UNINIT_ASSUMED_INIT, - methods::UNNECESSARY_JOIN, methods::UNNECESSARY_FILTER_MAP, methods::UNNECESSARY_FIND_MAP, methods::UNNECESSARY_FOLD, + methods::UNNECESSARY_JOIN, methods::UNNECESSARY_LAZY_EVALUATIONS, methods::UNNECESSARY_TO_OWNED, methods::UNWRAP_OR_ELSE_DEFAULT, From b47aefd34b3b732e563b2a637f62ef75c8617041 Mon Sep 17 00:00:00 2001 From: Yoav Lavi Date: Tue, 22 Mar 2022 11:07:55 +0100 Subject: [PATCH 03/35] fix description --- clippy_lints/src/methods/mod.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs index d0fcff7b8de2..3bcb78514930 100644 --- a/clippy_lints/src/methods/mod.rs +++ b/clippy_lints/src/methods/mod.rs @@ -2052,10 +2052,10 @@ declare_clippy_lint! { declare_clippy_lint! { /// ### What it does - /// Checks for use of `.collect::>.join(\"\")` on iterators. + /// Checks for use of `.collect::>().join(\"\")` on iterators. /// /// ### Why is this bad? - /// `.collect::` is more performant and cleaner + /// `.collect::()` is more performant and cleaner /// /// ### Example /// ```rust From 40f2e0eaaf454bd9756937267d078a1cd881c579 Mon Sep 17 00:00:00 2001 From: Yoav Lavi Date: Tue, 22 Mar 2022 11:08:42 +0100 Subject: [PATCH 04/35] fix description --- clippy_lints/src/methods/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs index 3bcb78514930..577283d3967e 100644 --- a/clippy_lints/src/methods/mod.rs +++ b/clippy_lints/src/methods/mod.rs @@ -2072,7 +2072,7 @@ declare_clippy_lint! { #[clippy::version = "pre 1.29.0"] pub UNNECESSARY_JOIN, perf, - "using `.collect::>.join(\"\")` on an iterator" + "using `.collect::>().join(\"\")` on an iterator" } pub struct Methods { From 12a46323c480868bf39a73f5b332345ce24716a1 Mon Sep 17 00:00:00 2001 From: Yoav Lavi Date: Tue, 22 Mar 2022 11:41:19 +0100 Subject: [PATCH 05/35] cleanup --- clippy_lints/src/methods/unnecessary_join.rs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/clippy_lints/src/methods/unnecessary_join.rs b/clippy_lints/src/methods/unnecessary_join.rs index bdd08dfb148a..dd0375189aa2 100644 --- a/clippy_lints/src/methods/unnecessary_join.rs +++ b/clippy_lints/src/methods/unnecessary_join.rs @@ -27,16 +27,16 @@ pub(super) fn check<'tcx>(context: &LateContext<'tcx>, expression: &'tcx hir::Ex } if_chain! { - let current_ty = context.typeck_results().expr_ty(expression); + let current_method_target_type = context.typeck_results().expr_ty(expression); // the current join method is being called on a vector // e.g .join("") - if is_type_diagnostic_item(context, current_ty, sym::Vec); - if let Some(parent) = get_parent_expr(context, expression); - if let ExprKind::MethodCall(_, [self_arg, ..], _) = &parent.kind; + if is_type_diagnostic_item(context, current_method_target_type, sym::Vec); + if let Some(parent_expression) = get_parent_expr(context, expression); + if let ExprKind::MethodCall(_, [self_argument, ..], _) = &parent_expression.kind; // the parent collect method is being called on an iterator // e.g. .collect>() - let caller_ty = context.typeck_results().expr_ty(self_arg); - if is_type_diagnostic_item(context, caller_ty, sym::Vec); + let previous_method_target_type = context.typeck_results().expr_ty(self_argument); + if is_type_diagnostic_item(context, previous_method_target_type, sym::Vec); // check that the argument for join is an empty string // check that the turbofish for collect is > or > if the iterator has String items From a03ee324ce024a856d43aa9870bbe4ea7738fa35 Mon Sep 17 00:00:00 2001 From: Yoav Lavi Date: Tue, 22 Mar 2022 18:41:36 +0100 Subject: [PATCH 06/35] fixes --- clippy_lints/src/methods/mod.rs | 4 +- clippy_lints/src/methods/unnecessary_join.rs | 78 ++++++++------------ tests/ui/unnecessary_join.rs | 35 +++++++++ tests/ui/unnecessary_join.stderr | 26 +++++++ 4 files changed, 93 insertions(+), 50 deletions(-) create mode 100644 tests/ui/unnecessary_join.rs create mode 100644 tests/ui/unnecessary_join.stderr diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs index 577283d3967e..e81ee63f8868 100644 --- a/clippy_lints/src/methods/mod.rs +++ b/clippy_lints/src/methods/mod.rs @@ -2456,9 +2456,9 @@ fn check_methods<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>, msrv: Optio ("is_file", []) => filetype_is_file::check(cx, expr, recv), ("is_none", []) => check_is_some_is_none(cx, expr, recv, false), ("is_some", []) => check_is_some_is_none(cx, expr, recv, true), - ("join", [_arg]) => { + ("join", [_]) => { if let Some(("collect", ..)) = method_call(recv) { - unnecessary_join::check(cx, recv); + unnecessary_join::check(cx, expr); } }, ("last", args @ []) | ("skip", args @ [_]) => { diff --git a/clippy_lints/src/methods/unnecessary_join.rs b/clippy_lints/src/methods/unnecessary_join.rs index dd0375189aa2..566ba2bdd9cb 100644 --- a/clippy_lints/src/methods/unnecessary_join.rs +++ b/clippy_lints/src/methods/unnecessary_join.rs @@ -1,63 +1,45 @@ +use clippy_utils::diagnostics::span_lint_and_sugg; use clippy_utils::ty::is_type_diagnostic_item; -use clippy_utils::{get_parent_expr, get_parent_node}; use hir::ExprKind; +use rustc_ast::ast::LitKind; +use rustc_errors::Applicability; use rustc_hir as hir; use rustc_lint::LateContext; +use rustc_middle::ty; use rustc_span::sym; -// use super::UNNECESSARY_JOIN; +use super::UNNECESSARY_JOIN; -pub(super) fn check<'tcx>(context: &LateContext<'tcx>, expression: &'tcx hir::Expr<'tcx>) { - // this section should not pass the dogfood test - // TODO: remove and use a proper test - let vector = vec!["hello", "world"]; - let output = vector - .iter() - .map(|item| item.to_uppercase()) - .collect::>() - .join(""); - println!("{}", output); - - // let mut applicability = Applicability::MachineApplicable; - - let parent = get_parent_node(context.tcx, expression.hir_id); - - if parent.is_none() { - return; - } +pub(super) fn check<'tcx>(context: &LateContext<'tcx>, join: &'tcx hir::Expr<'tcx>) { + let applicability = Applicability::MachineApplicable; if_chain! { - let current_method_target_type = context.typeck_results().expr_ty(expression); + if let ExprKind::MethodCall(_path, [join_self_arg, join_arg], _span) = &join.kind; + let collect_output_type = context.typeck_results().expr_ty(join_self_arg); // the current join method is being called on a vector // e.g .join("") - if is_type_diagnostic_item(context, current_method_target_type, sym::Vec); - if let Some(parent_expression) = get_parent_expr(context, expression); - if let ExprKind::MethodCall(_, [self_argument, ..], _) = &parent_expression.kind; - // the parent collect method is being called on an iterator - // e.g. .collect>() - let previous_method_target_type = context.typeck_results().expr_ty(self_argument); - if is_type_diagnostic_item(context, previous_method_target_type, sym::Vec); - - // check that the argument for join is an empty string - // check that the turbofish for collect is > or > if the iterator has String items + if is_type_diagnostic_item(context, collect_output_type, sym::Vec); + // the argument for join is "" + if let ExprKind::Lit(spanned) = &join_arg.kind; + if let LitKind::Str(symbol, _) = spanned.node; + if symbol.is_empty(); + // the turbofish for collect is ::> + let collect_output_adjusted_type = &context.typeck_results().expr_ty_adjusted(join_self_arg); + if let ty::Ref(_, ref_type, _) = collect_output_adjusted_type.kind(); + if let ty::Slice(slice) = ref_type.kind(); + if is_type_diagnostic_item(context, *slice, sym::String); then { - // span_lint_and_sugg( - // cx, - // UNNECESSARY_JOIN, - // span, - // &format!( - // "called `.collect>().join("")` on a {1}. Using `.collect::()` is - // more clear and more concise", caller_type - // ), - // "try this", - // format!( - // "{}.collect::()", - // snippet_with_applicability(cx, recv.span, "..", &mut applicability), - // ), - // applicability, - // ); - dbg!("{:#?} {:#?}", expression, parent); - panic!(".collect().join() called on an iterator"); + span_lint_and_sugg( + context, + UNNECESSARY_JOIN, + join.span, + &format!( + "called `.collect>().join(\"\")` on a {}. Using `.collect::()` is clearer and more concise", collect_output_type, + ), + "try using", + ".collect::()".to_owned(), + applicability, + ); } } } diff --git a/tests/ui/unnecessary_join.rs b/tests/ui/unnecessary_join.rs new file mode 100644 index 000000000000..8dc852797c25 --- /dev/null +++ b/tests/ui/unnecessary_join.rs @@ -0,0 +1,35 @@ +#![warn(clippy::unnecessary_join)] + +fn main() { + // should be linted + let vector = vec!["hello", "world"]; + let output = vector + .iter() + .map(|item| item.to_uppercase()) + .collect::>() + .join(""); + println!("{}", output); + + // should be linted + let vector = vec!["hello", "world"]; + let output = vector + .iter() + .map(|item| item.to_uppercase()) + .collect::>() + .join(""); + println!("{}", output); + + // should not be linted + let vector = vec!["hello", "world"]; + let output = vector + .iter() + .map(|item| item.to_uppercase()) + .collect::>() + .join("\n"); + println!("{}", output); + + // should not be linted + let vector = vec!["hello", "world"]; + let output = vector.iter().map(|item| item.to_uppercase()).collect::(); + println!("{}", output); +} diff --git a/tests/ui/unnecessary_join.stderr b/tests/ui/unnecessary_join.stderr new file mode 100644 index 000000000000..30c746b39316 --- /dev/null +++ b/tests/ui/unnecessary_join.stderr @@ -0,0 +1,26 @@ +error: called `.collect>().join("")` on a std::vec::Vec. Using `.collect::()` is clearer and more concise + --> $DIR/unnecessary_join.rs:6:18 + | +LL | let output = vector + | __________________^ +LL | | .iter() +LL | | .map(|item| item.to_uppercase()) +LL | | .collect::>() +LL | | .join(""); + | |_________________^ help: try using: `.collect::()` + | + = note: `-D clippy::unnecessary-join` implied by `-D warnings` + +error: called `.collect>().join("")` on a std::vec::Vec. Using `.collect::()` is clearer and more concise + --> $DIR/unnecessary_join.rs:15:18 + | +LL | let output = vector + | __________________^ +LL | | .iter() +LL | | .map(|item| item.to_uppercase()) +LL | | .collect::>() +LL | | .join(""); + | |_________________^ help: try using: `.collect::()` + +error: aborting due to 2 previous errors + From d801dd4f106e64c07ee77e8fcaf597bff4511d11 Mon Sep 17 00:00:00 2001 From: Yoav Lavi Date: Tue, 22 Mar 2022 18:44:25 +0100 Subject: [PATCH 07/35] fix desc --- clippy_lints/src/methods/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs index e81ee63f8868..05458251715f 100644 --- a/clippy_lints/src/methods/mod.rs +++ b/clippy_lints/src/methods/mod.rs @@ -2052,7 +2052,7 @@ declare_clippy_lint! { declare_clippy_lint! { /// ### What it does - /// Checks for use of `.collect::>().join(\"\")` on iterators. + /// Checks for use of `.collect::>().join("")` on iterators. /// /// ### Why is this bad? /// `.collect::()` is more performant and cleaner From d34f5db3629d472b9d385a1b239ff791b1674cb3 Mon Sep 17 00:00:00 2001 From: Yoav Lavi Date: Tue, 22 Mar 2022 19:09:04 +0100 Subject: [PATCH 08/35] move comment --- clippy_lints/src/methods/unnecessary_join.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/clippy_lints/src/methods/unnecessary_join.rs b/clippy_lints/src/methods/unnecessary_join.rs index 566ba2bdd9cb..a4facd5e08e7 100644 --- a/clippy_lints/src/methods/unnecessary_join.rs +++ b/clippy_lints/src/methods/unnecessary_join.rs @@ -14,10 +14,10 @@ pub(super) fn check<'tcx>(context: &LateContext<'tcx>, join: &'tcx hir::Expr<'tc let applicability = Applicability::MachineApplicable; if_chain! { - if let ExprKind::MethodCall(_path, [join_self_arg, join_arg], _span) = &join.kind; - let collect_output_type = context.typeck_results().expr_ty(join_self_arg); // the current join method is being called on a vector // e.g .join("") + if let ExprKind::MethodCall(_path, [join_self_arg, join_arg], _span) = &join.kind; + let collect_output_type = context.typeck_results().expr_ty(join_self_arg); if is_type_diagnostic_item(context, collect_output_type, sym::Vec); // the argument for join is "" if let ExprKind::Lit(spanned) = &join_arg.kind; From 907e91301b3f4c0e65c8a10d74dcf8dbfa609248 Mon Sep 17 00:00:00 2001 From: Yoav Lavi Date: Tue, 22 Mar 2022 19:10:47 +0100 Subject: [PATCH 09/35] change explanation --- clippy_lints/src/methods/mod.rs | 2 +- clippy_lints/src/methods/unnecessary_join.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs index 05458251715f..67d2a88c0c54 100644 --- a/clippy_lints/src/methods/mod.rs +++ b/clippy_lints/src/methods/mod.rs @@ -2055,7 +2055,7 @@ declare_clippy_lint! { /// Checks for use of `.collect::>().join("")` on iterators. /// /// ### Why is this bad? - /// `.collect::()` is more performant and cleaner + /// `.collect::()` is more performant and more concise /// /// ### Example /// ```rust diff --git a/clippy_lints/src/methods/unnecessary_join.rs b/clippy_lints/src/methods/unnecessary_join.rs index a4facd5e08e7..8ce032bb6ecb 100644 --- a/clippy_lints/src/methods/unnecessary_join.rs +++ b/clippy_lints/src/methods/unnecessary_join.rs @@ -34,7 +34,7 @@ pub(super) fn check<'tcx>(context: &LateContext<'tcx>, join: &'tcx hir::Expr<'tc UNNECESSARY_JOIN, join.span, &format!( - "called `.collect>().join(\"\")` on a {}. Using `.collect::()` is clearer and more concise", collect_output_type, + "called `.collect>().join(\"\")` on a {}. Using `.collect::()` is more performant and more concise", collect_output_type, ), "try using", ".collect::()".to_owned(), From 9ff0dddc2c9524d678fd22c3bd22669527d1bb73 Mon Sep 17 00:00:00 2001 From: Yoav Lavi Date: Tue, 22 Mar 2022 19:37:04 +0100 Subject: [PATCH 10/35] Update tests/ui/unnecessary_join.rs Co-authored-by: Philipp Krones --- tests/ui/unnecessary_join.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/ui/unnecessary_join.rs b/tests/ui/unnecessary_join.rs index 8dc852797c25..0a21656a7558 100644 --- a/tests/ui/unnecessary_join.rs +++ b/tests/ui/unnecessary_join.rs @@ -1,3 +1,5 @@ +// run-rustfix + #![warn(clippy::unnecessary_join)] fn main() { From 3fd42662230670467ae793eb736a6bfc4da725e9 Mon Sep 17 00:00:00 2001 From: Yoav Lavi Date: Tue, 22 Mar 2022 19:38:45 +0100 Subject: [PATCH 11/35] fix redundant message --- clippy_lints/src/methods/unnecessary_join.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clippy_lints/src/methods/unnecessary_join.rs b/clippy_lints/src/methods/unnecessary_join.rs index 8ce032bb6ecb..5cab27bfd0a6 100644 --- a/clippy_lints/src/methods/unnecessary_join.rs +++ b/clippy_lints/src/methods/unnecessary_join.rs @@ -34,7 +34,7 @@ pub(super) fn check<'tcx>(context: &LateContext<'tcx>, join: &'tcx hir::Expr<'tc UNNECESSARY_JOIN, join.span, &format!( - "called `.collect>().join(\"\")` on a {}. Using `.collect::()` is more performant and more concise", collect_output_type, + "called `.collect>().join(\"\")` on a {}", collect_output_type, ), "try using", ".collect::()".to_owned(), From 6c0277131c1330a811ec97f903a3cb7f4cdc7f66 Mon Sep 17 00:00:00 2001 From: Yoav Lavi Date: Tue, 22 Mar 2022 19:41:17 +0100 Subject: [PATCH 12/35] add version --- clippy_lints/src/methods/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs index 67d2a88c0c54..a92f9b2a32aa 100644 --- a/clippy_lints/src/methods/mod.rs +++ b/clippy_lints/src/methods/mod.rs @@ -2069,7 +2069,7 @@ declare_clippy_lint! { /// let output = vector.iter().map(|item| item.to_uppercase()).collect::(); /// println!("{}", output); /// ``` - #[clippy::version = "pre 1.29.0"] + #[clippy::version = "pre 1.61.0"] pub UNNECESSARY_JOIN, perf, "using `.collect::>().join(\"\")` on an iterator" From ac1fab4ca0ad9bb3124badbcd632a5799f1a54f7 Mon Sep 17 00:00:00 2001 From: Yoav Lavi Date: Tue, 22 Mar 2022 19:43:15 +0100 Subject: [PATCH 13/35] move up check --- clippy_lints/src/methods/unnecessary_join.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/clippy_lints/src/methods/unnecessary_join.rs b/clippy_lints/src/methods/unnecessary_join.rs index 5cab27bfd0a6..ffe19e12bb2d 100644 --- a/clippy_lints/src/methods/unnecessary_join.rs +++ b/clippy_lints/src/methods/unnecessary_join.rs @@ -18,16 +18,16 @@ pub(super) fn check<'tcx>(context: &LateContext<'tcx>, join: &'tcx hir::Expr<'tc // e.g .join("") if let ExprKind::MethodCall(_path, [join_self_arg, join_arg], _span) = &join.kind; let collect_output_type = context.typeck_results().expr_ty(join_self_arg); - if is_type_diagnostic_item(context, collect_output_type, sym::Vec); - // the argument for join is "" - if let ExprKind::Lit(spanned) = &join_arg.kind; - if let LitKind::Str(symbol, _) = spanned.node; - if symbol.is_empty(); // the turbofish for collect is ::> let collect_output_adjusted_type = &context.typeck_results().expr_ty_adjusted(join_self_arg); if let ty::Ref(_, ref_type, _) = collect_output_adjusted_type.kind(); if let ty::Slice(slice) = ref_type.kind(); if is_type_diagnostic_item(context, *slice, sym::String); + // the argument for join is "" + if let ExprKind::Lit(spanned) = &join_arg.kind; + if let LitKind::Str(symbol, _) = spanned.node; + if symbol.is_empty(); + then { span_lint_and_sugg( context, From 98c8c26c0f615686fd91adb97e04c62d59d1aa18 Mon Sep 17 00:00:00 2001 From: Yoav Lavi Date: Tue, 22 Mar 2022 20:03:56 +0100 Subject: [PATCH 14/35] pass arguments, fix span --- clippy_lints/src/methods/mod.rs | 6 +++--- clippy_lints/src/methods/unnecessary_join.rs | 12 +++++++++--- 2 files changed, 12 insertions(+), 6 deletions(-) diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs index a92f9b2a32aa..c15740398027 100644 --- a/clippy_lints/src/methods/mod.rs +++ b/clippy_lints/src/methods/mod.rs @@ -2456,9 +2456,9 @@ fn check_methods<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>, msrv: Optio ("is_file", []) => filetype_is_file::check(cx, expr, recv), ("is_none", []) => check_is_some_is_none(cx, expr, recv, false), ("is_some", []) => check_is_some_is_none(cx, expr, recv, true), - ("join", [_]) => { - if let Some(("collect", ..)) = method_call(recv) { - unnecessary_join::check(cx, expr); + ("join", [join_arg]) => { + if let Some(("collect", .., collect_span)) = method_call(recv) { + unnecessary_join::check(cx, recv, join_arg, collect_span, expr); } }, ("last", args @ []) | ("skip", args @ [_]) => { diff --git a/clippy_lints/src/methods/unnecessary_join.rs b/clippy_lints/src/methods/unnecessary_join.rs index ffe19e12bb2d..51be16105916 100644 --- a/clippy_lints/src/methods/unnecessary_join.rs +++ b/clippy_lints/src/methods/unnecessary_join.rs @@ -7,16 +7,22 @@ use rustc_hir as hir; use rustc_lint::LateContext; use rustc_middle::ty; use rustc_span::sym; +use rustc_span::Span; use super::UNNECESSARY_JOIN; -pub(super) fn check<'tcx>(context: &LateContext<'tcx>, join: &'tcx hir::Expr<'tcx>) { +pub(super) fn check<'tcx>( + context: &LateContext<'tcx>, + join_self_arg: &'tcx hir::Expr<'tcx>, + join_arg: &'tcx hir::Expr<'tcx>, + collect_span: Span, + expression: &'tcx hir::Expr<'tcx>, +) { let applicability = Applicability::MachineApplicable; if_chain! { // the current join method is being called on a vector // e.g .join("") - if let ExprKind::MethodCall(_path, [join_self_arg, join_arg], _span) = &join.kind; let collect_output_type = context.typeck_results().expr_ty(join_self_arg); // the turbofish for collect is ::> let collect_output_adjusted_type = &context.typeck_results().expr_ty_adjusted(join_self_arg); @@ -32,7 +38,7 @@ pub(super) fn check<'tcx>(context: &LateContext<'tcx>, join: &'tcx hir::Expr<'tc span_lint_and_sugg( context, UNNECESSARY_JOIN, - join.span, + collect_span.with_hi(expression.span.hi()), &format!( "called `.collect>().join(\"\")` on a {}", collect_output_type, ), From ee7791b2c9537daaec05cdf75bf24fa58c6d5818 Mon Sep 17 00:00:00 2001 From: Yoav Lavi Date: Tue, 22 Mar 2022 20:04:56 +0100 Subject: [PATCH 15/35] cleanup --- clippy_lints/src/methods/unnecessary_join.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/clippy_lints/src/methods/unnecessary_join.rs b/clippy_lints/src/methods/unnecessary_join.rs index 51be16105916..42396d35ca35 100644 --- a/clippy_lints/src/methods/unnecessary_join.rs +++ b/clippy_lints/src/methods/unnecessary_join.rs @@ -6,8 +6,7 @@ use rustc_errors::Applicability; use rustc_hir as hir; use rustc_lint::LateContext; use rustc_middle::ty; -use rustc_span::sym; -use rustc_span::Span; +use rustc_span::{sym, Span}; use super::UNNECESSARY_JOIN; From 3322ba9c6d4349c295dc2c7c23f221e764d08f79 Mon Sep 17 00:00:00 2001 From: Yoav Lavi Date: Tue, 22 Mar 2022 20:27:38 +0100 Subject: [PATCH 16/35] fix spans --- clippy_lints/src/methods/mod.rs | 4 +-- clippy_lints/src/methods/unnecessary_join.rs | 6 ++-- tests/ui/unnecessary_join.fixed | 35 ++++++++++++++++++++ tests/ui/unnecessary_join.stderr | 26 ++++++--------- 4 files changed, 51 insertions(+), 20 deletions(-) create mode 100644 tests/ui/unnecessary_join.fixed diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs index c15740398027..f49cd100effd 100644 --- a/clippy_lints/src/methods/mod.rs +++ b/clippy_lints/src/methods/mod.rs @@ -2457,8 +2457,8 @@ fn check_methods<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>, msrv: Optio ("is_none", []) => check_is_some_is_none(cx, expr, recv, false), ("is_some", []) => check_is_some_is_none(cx, expr, recv, true), ("join", [join_arg]) => { - if let Some(("collect", .., collect_span)) = method_call(recv) { - unnecessary_join::check(cx, recv, join_arg, collect_span, expr); + if let Some(("collect", [recv2], collect_span)) = method_call(recv) { + unnecessary_join::check(cx, recv, join_arg, collect_span, span, expr, recv2); } }, ("last", args @ []) | ("skip", args @ [_]) => { diff --git a/clippy_lints/src/methods/unnecessary_join.rs b/clippy_lints/src/methods/unnecessary_join.rs index 42396d35ca35..4bf0a418a480 100644 --- a/clippy_lints/src/methods/unnecessary_join.rs +++ b/clippy_lints/src/methods/unnecessary_join.rs @@ -15,7 +15,9 @@ pub(super) fn check<'tcx>( join_self_arg: &'tcx hir::Expr<'tcx>, join_arg: &'tcx hir::Expr<'tcx>, collect_span: Span, - expression: &'tcx hir::Expr<'tcx>, + recv_span: Span, + expr: &'tcx hir::Expr<'tcx>, + recv2: &'tcx hir::Expr<'tcx>, ) { let applicability = Applicability::MachineApplicable; @@ -37,7 +39,7 @@ pub(super) fn check<'tcx>( span_lint_and_sugg( context, UNNECESSARY_JOIN, - collect_span.with_hi(expression.span.hi()), + recv2.span.with_lo(expr.span.hi()), &format!( "called `.collect>().join(\"\")` on a {}", collect_output_type, ), diff --git a/tests/ui/unnecessary_join.fixed b/tests/ui/unnecessary_join.fixed new file mode 100644 index 000000000000..7e12c6ae4be9 --- /dev/null +++ b/tests/ui/unnecessary_join.fixed @@ -0,0 +1,35 @@ +// run-rustfix + +#![warn(clippy::unnecessary_join)] + +fn main() { + // should be linted + let vector = vec!["hello", "world"]; + let output = vector + .iter() + .map(|item| item.to_uppercase()) + .collect::(); + println!("{}", output); + + // should be linted + let vector = vec!["hello", "world"]; + let output = vector + .iter() + .map(|item| item.to_uppercase()) + .collect::(); + println!("{}", output); + + // should not be linted + let vector = vec!["hello", "world"]; + let output = vector + .iter() + .map(|item| item.to_uppercase()) + .collect::>() + .join("\n"); + println!("{}", output); + + // should not be linted + let vector = vec!["hello", "world"]; + let output = vector.iter().map(|item| item.to_uppercase()).collect::(); + println!("{}", output); +} diff --git a/tests/ui/unnecessary_join.stderr b/tests/ui/unnecessary_join.stderr index 30c746b39316..d02d647833d1 100644 --- a/tests/ui/unnecessary_join.stderr +++ b/tests/ui/unnecessary_join.stderr @@ -1,26 +1,20 @@ -error: called `.collect>().join("")` on a std::vec::Vec. Using `.collect::()` is clearer and more concise - --> $DIR/unnecessary_join.rs:6:18 +error: called `.collect>().join("")` on a std::vec::Vec + --> $DIR/unnecessary_join.rs:11:10 | -LL | let output = vector - | __________________^ -LL | | .iter() -LL | | .map(|item| item.to_uppercase()) -LL | | .collect::>() +LL | .collect::>() + | __________^ LL | | .join(""); - | |_________________^ help: try using: `.collect::()` + | |_________________^ help: try using: `collect::()` | = note: `-D clippy::unnecessary-join` implied by `-D warnings` -error: called `.collect>().join("")` on a std::vec::Vec. Using `.collect::()` is clearer and more concise - --> $DIR/unnecessary_join.rs:15:18 +error: called `.collect>().join("")` on a std::vec::Vec + --> $DIR/unnecessary_join.rs:20:10 | -LL | let output = vector - | __________________^ -LL | | .iter() -LL | | .map(|item| item.to_uppercase()) -LL | | .collect::>() +LL | .collect::>() + | __________^ LL | | .join(""); - | |_________________^ help: try using: `.collect::()` + | |_________________^ help: try using: `collect::()` error: aborting due to 2 previous errors From 52f47ef7e678d44373af1c2cf1947bb3a27dc5b7 Mon Sep 17 00:00:00 2001 From: Yoav Lavi Date: Tue, 22 Mar 2022 20:29:03 +0100 Subject: [PATCH 17/35] cleanup --- clippy_lints/src/methods/mod.rs | 4 ++-- clippy_lints/src/methods/unnecessary_join.rs | 2 -- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs index f49cd100effd..6baf8b39c98b 100644 --- a/clippy_lints/src/methods/mod.rs +++ b/clippy_lints/src/methods/mod.rs @@ -2457,8 +2457,8 @@ fn check_methods<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>, msrv: Optio ("is_none", []) => check_is_some_is_none(cx, expr, recv, false), ("is_some", []) => check_is_some_is_none(cx, expr, recv, true), ("join", [join_arg]) => { - if let Some(("collect", [recv2], collect_span)) = method_call(recv) { - unnecessary_join::check(cx, recv, join_arg, collect_span, span, expr, recv2); + if let Some(("collect", [recv2], ..)) = method_call(recv) { + unnecessary_join::check(cx, recv, join_arg, expr, recv2); } }, ("last", args @ []) | ("skip", args @ [_]) => { diff --git a/clippy_lints/src/methods/unnecessary_join.rs b/clippy_lints/src/methods/unnecessary_join.rs index 4bf0a418a480..8280da0d6890 100644 --- a/clippy_lints/src/methods/unnecessary_join.rs +++ b/clippy_lints/src/methods/unnecessary_join.rs @@ -14,8 +14,6 @@ pub(super) fn check<'tcx>( context: &LateContext<'tcx>, join_self_arg: &'tcx hir::Expr<'tcx>, join_arg: &'tcx hir::Expr<'tcx>, - collect_span: Span, - recv_span: Span, expr: &'tcx hir::Expr<'tcx>, recv2: &'tcx hir::Expr<'tcx>, ) { From 7b40cfa9755d2511f647b641da5679a03a34dbc5 Mon Sep 17 00:00:00 2001 From: Yoav Lavi Date: Tue, 22 Mar 2022 20:31:36 +0100 Subject: [PATCH 18/35] update test --- clippy_lints/src/methods/unnecessary_join.rs | 2 +- tests/ui/unnecessary_join.fixed | 6 ++---- tests/ui/unnecessary_join.stderr | 18 ++++++++++-------- 3 files changed, 13 insertions(+), 13 deletions(-) diff --git a/clippy_lints/src/methods/unnecessary_join.rs b/clippy_lints/src/methods/unnecessary_join.rs index 8280da0d6890..c2dc19814c82 100644 --- a/clippy_lints/src/methods/unnecessary_join.rs +++ b/clippy_lints/src/methods/unnecessary_join.rs @@ -6,7 +6,7 @@ use rustc_errors::Applicability; use rustc_hir as hir; use rustc_lint::LateContext; use rustc_middle::ty; -use rustc_span::{sym, Span}; +use rustc_span::sym; use super::UNNECESSARY_JOIN; diff --git a/tests/ui/unnecessary_join.fixed b/tests/ui/unnecessary_join.fixed index 7e12c6ae4be9..c85ffdf69a46 100644 --- a/tests/ui/unnecessary_join.fixed +++ b/tests/ui/unnecessary_join.fixed @@ -7,16 +7,14 @@ fn main() { let vector = vec!["hello", "world"]; let output = vector .iter() - .map(|item| item.to_uppercase()) - .collect::(); + .map(|item| item.to_uppercase()).collect::(); println!("{}", output); // should be linted let vector = vec!["hello", "world"]; let output = vector .iter() - .map(|item| item.to_uppercase()) - .collect::(); + .map(|item| item.to_uppercase()).collect::(); println!("{}", output); // should not be linted diff --git a/tests/ui/unnecessary_join.stderr b/tests/ui/unnecessary_join.stderr index d02d647833d1..5c9f89c854fe 100644 --- a/tests/ui/unnecessary_join.stderr +++ b/tests/ui/unnecessary_join.stderr @@ -1,20 +1,22 @@ error: called `.collect>().join("")` on a std::vec::Vec - --> $DIR/unnecessary_join.rs:11:10 + --> $DIR/unnecessary_join.rs:10:41 | -LL | .collect::>() - | __________^ +LL | .map(|item| item.to_uppercase()) + | _________________________________________^ +LL | | .collect::>() LL | | .join(""); - | |_________________^ help: try using: `collect::()` + | |_________________^ help: try using: `.collect::()` | = note: `-D clippy::unnecessary-join` implied by `-D warnings` error: called `.collect>().join("")` on a std::vec::Vec - --> $DIR/unnecessary_join.rs:20:10 + --> $DIR/unnecessary_join.rs:19:41 | -LL | .collect::>() - | __________^ +LL | .map(|item| item.to_uppercase()) + | _________________________________________^ +LL | | .collect::>() LL | | .join(""); - | |_________________^ help: try using: `collect::()` + | |_________________^ help: try using: `.collect::()` error: aborting due to 2 previous errors From 14405eb0b357095cfe3ff384dbcf8c7add68fc4a Mon Sep 17 00:00:00 2001 From: Yoav Lavi Date: Tue, 22 Mar 2022 21:05:30 +0100 Subject: [PATCH 19/35] move lets out of if chain --- clippy_lints/src/methods/unnecessary_join.rs | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/clippy_lints/src/methods/unnecessary_join.rs b/clippy_lints/src/methods/unnecessary_join.rs index c2dc19814c82..8b03809dabd3 100644 --- a/clippy_lints/src/methods/unnecessary_join.rs +++ b/clippy_lints/src/methods/unnecessary_join.rs @@ -18,13 +18,10 @@ pub(super) fn check<'tcx>( recv2: &'tcx hir::Expr<'tcx>, ) { let applicability = Applicability::MachineApplicable; - + let collect_output_type = context.typeck_results().expr_ty(join_self_arg); + let collect_output_adjusted_type = &context.typeck_results().expr_ty_adjusted(join_self_arg); if_chain! { - // the current join method is being called on a vector - // e.g .join("") - let collect_output_type = context.typeck_results().expr_ty(join_self_arg); // the turbofish for collect is ::> - let collect_output_adjusted_type = &context.typeck_results().expr_ty_adjusted(join_self_arg); if let ty::Ref(_, ref_type, _) = collect_output_adjusted_type.kind(); if let ty::Slice(slice) = ref_type.kind(); if is_type_diagnostic_item(context, *slice, sym::String); @@ -32,7 +29,6 @@ pub(super) fn check<'tcx>( if let ExprKind::Lit(spanned) = &join_arg.kind; if let LitKind::Str(symbol, _) = spanned.node; if symbol.is_empty(); - then { span_lint_and_sugg( context, From 8d41a8f58be1b8a3591ab87a29bef441be97ee1a Mon Sep 17 00:00:00 2001 From: Yoav Lavi Date: Wed, 23 Mar 2022 10:00:06 +0100 Subject: [PATCH 20/35] fix span --- Cargo.toml | 7 ++++++ clippy_lints/Cargo.toml | 24 ++++++++++++++++++++ clippy_lints/src/methods/mod.rs | 6 ++--- clippy_lints/src/methods/unnecessary_join.rs | 8 +++---- clippy_utils/Cargo.toml | 17 ++++++++++++++ tests/ui/unnecessary_join.fixed | 6 +++-- tests/ui/unnecessary_join.stderr | 18 +++++++-------- 7 files changed, 67 insertions(+), 19 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 123af23881b6..a6efbd5823f6 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -20,6 +20,13 @@ path = "src/main.rs" name = "clippy-driver" path = "src/driver.rs" +[target.'cfg(NOT_A_PLATFORM)'.dependencies] +rustc_driver = { path = "/Users/yoavlavi/dev/rust/compiler/rustc_driver" } +rustc_errors = { path = "/Users/yoavlavi/dev/rust/compiler/rustc_errors" } +rustc_interface = { path = "/Users/yoavlavi/dev/rust/compiler/rustc_interface" } +rustc_session = { path = "/Users/yoavlavi/dev/rust/compiler/rustc_session" } +rustc_span = { path = "/Users/yoavlavi/dev/rust/compiler/rustc_span" } + [dependencies] clippy_lints = { version = "0.1", path = "clippy_lints" } semver = "1.0" diff --git a/clippy_lints/Cargo.toml b/clippy_lints/Cargo.toml index 66e61660d313..6e59fbc8f559 100644 --- a/clippy_lints/Cargo.toml +++ b/clippy_lints/Cargo.toml @@ -8,6 +8,30 @@ license = "MIT OR Apache-2.0" keywords = ["clippy", "lint", "plugin"] edition = "2021" +[target.'cfg(NOT_A_PLATFORM)'.dependencies] +rustc_arena = { path = "/Users/yoavlavi/dev/rust/compiler/rustc_arena" } +rustc_ast = { path = "/Users/yoavlavi/dev/rust/compiler/rustc_ast" } +rustc_ast_pretty = { path = "/Users/yoavlavi/dev/rust/compiler/rustc_ast_pretty" } +rustc_attr = { path = "/Users/yoavlavi/dev/rust/compiler/rustc_attr" } +rustc_data_structures = { path = "/Users/yoavlavi/dev/rust/compiler/rustc_data_structures" } +rustc_driver = { path = "/Users/yoavlavi/dev/rust/compiler/rustc_driver" } +rustc_errors = { path = "/Users/yoavlavi/dev/rust/compiler/rustc_errors" } +rustc_hir = { path = "/Users/yoavlavi/dev/rust/compiler/rustc_hir" } +rustc_hir_pretty = { path = "/Users/yoavlavi/dev/rust/compiler/rustc_hir_pretty" } +rustc_index = { path = "/Users/yoavlavi/dev/rust/compiler/rustc_index" } +rustc_infer = { path = "/Users/yoavlavi/dev/rust/compiler/rustc_infer" } +rustc_lexer = { path = "/Users/yoavlavi/dev/rust/compiler/rustc_lexer" } +rustc_lint = { path = "/Users/yoavlavi/dev/rust/compiler/rustc_lint" } +rustc_middle = { path = "/Users/yoavlavi/dev/rust/compiler/rustc_middle" } +rustc_mir_dataflow = { path = "/Users/yoavlavi/dev/rust/compiler/rustc_mir_dataflow" } +rustc_parse = { path = "/Users/yoavlavi/dev/rust/compiler/rustc_parse" } +rustc_parse_format = { path = "/Users/yoavlavi/dev/rust/compiler/rustc_parse_format" } +rustc_session = { path = "/Users/yoavlavi/dev/rust/compiler/rustc_session" } +rustc_span = { path = "/Users/yoavlavi/dev/rust/compiler/rustc_span" } +rustc_target = { path = "/Users/yoavlavi/dev/rust/compiler/rustc_target" } +rustc_trait_selection = { path = "/Users/yoavlavi/dev/rust/compiler/rustc_trait_selection" } +rustc_typeck = { path = "/Users/yoavlavi/dev/rust/compiler/rustc_typeck" } + [dependencies] cargo_metadata = "0.14" clippy_utils = { path = "../clippy_utils" } diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs index 6baf8b39c98b..94c680a6539c 100644 --- a/clippy_lints/src/methods/mod.rs +++ b/clippy_lints/src/methods/mod.rs @@ -2055,7 +2055,7 @@ declare_clippy_lint! { /// Checks for use of `.collect::>().join("")` on iterators. /// /// ### Why is this bad? - /// `.collect::()` is more performant and more concise + /// `.collect::()` is more concise and usually more performant /// /// ### Example /// ```rust @@ -2457,8 +2457,8 @@ fn check_methods<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>, msrv: Optio ("is_none", []) => check_is_some_is_none(cx, expr, recv, false), ("is_some", []) => check_is_some_is_none(cx, expr, recv, true), ("join", [join_arg]) => { - if let Some(("collect", [recv2], ..)) = method_call(recv) { - unnecessary_join::check(cx, recv, join_arg, expr, recv2); + if let Some(("collect", _, span)) = method_call(recv) { + unnecessary_join::check(cx, recv, join_arg, expr, span); } }, ("last", args @ []) | ("skip", args @ [_]) => { diff --git a/clippy_lints/src/methods/unnecessary_join.rs b/clippy_lints/src/methods/unnecessary_join.rs index 8b03809dabd3..79e2e71b7a58 100644 --- a/clippy_lints/src/methods/unnecessary_join.rs +++ b/clippy_lints/src/methods/unnecessary_join.rs @@ -6,7 +6,7 @@ use rustc_errors::Applicability; use rustc_hir as hir; use rustc_lint::LateContext; use rustc_middle::ty; -use rustc_span::sym; +use rustc_span::{Span, sym}; use super::UNNECESSARY_JOIN; @@ -15,7 +15,7 @@ pub(super) fn check<'tcx>( join_self_arg: &'tcx hir::Expr<'tcx>, join_arg: &'tcx hir::Expr<'tcx>, expr: &'tcx hir::Expr<'tcx>, - recv2: &'tcx hir::Expr<'tcx>, + span: Span, ) { let applicability = Applicability::MachineApplicable; let collect_output_type = context.typeck_results().expr_ty(join_self_arg); @@ -33,12 +33,12 @@ pub(super) fn check<'tcx>( span_lint_and_sugg( context, UNNECESSARY_JOIN, - recv2.span.with_lo(expr.span.hi()), + span.with_hi(expr.span.hi()), &format!( "called `.collect>().join(\"\")` on a {}", collect_output_type, ), "try using", - ".collect::()".to_owned(), + "collect::()".to_owned(), applicability, ); } diff --git a/clippy_utils/Cargo.toml b/clippy_utils/Cargo.toml index d3ed8da4499f..0b9f88c9971e 100644 --- a/clippy_utils/Cargo.toml +++ b/clippy_utils/Cargo.toml @@ -4,6 +4,23 @@ version = "0.1.61" edition = "2021" publish = false +[target.'cfg(NOT_A_PLATFORM)'.dependencies] +rustc_ast = { path = "/Users/yoavlavi/dev/rust/compiler/rustc_ast" } +rustc_ast_pretty = { path = "/Users/yoavlavi/dev/rust/compiler/rustc_ast_pretty" } +rustc_attr = { path = "/Users/yoavlavi/dev/rust/compiler/rustc_attr" } +rustc_data_structures = { path = "/Users/yoavlavi/dev/rust/compiler/rustc_data_structures" } +rustc_errors = { path = "/Users/yoavlavi/dev/rust/compiler/rustc_errors" } +rustc_hir = { path = "/Users/yoavlavi/dev/rust/compiler/rustc_hir" } +rustc_infer = { path = "/Users/yoavlavi/dev/rust/compiler/rustc_infer" } +rustc_lexer = { path = "/Users/yoavlavi/dev/rust/compiler/rustc_lexer" } +rustc_lint = { path = "/Users/yoavlavi/dev/rust/compiler/rustc_lint" } +rustc_middle = { path = "/Users/yoavlavi/dev/rust/compiler/rustc_middle" } +rustc_session = { path = "/Users/yoavlavi/dev/rust/compiler/rustc_session" } +rustc_span = { path = "/Users/yoavlavi/dev/rust/compiler/rustc_span" } +rustc_target = { path = "/Users/yoavlavi/dev/rust/compiler/rustc_target" } +rustc_trait_selection = { path = "/Users/yoavlavi/dev/rust/compiler/rustc_trait_selection" } +rustc_typeck = { path = "/Users/yoavlavi/dev/rust/compiler/rustc_typeck" } + [dependencies] arrayvec = { version = "0.7", default-features = false } if_chain = "1.0" diff --git a/tests/ui/unnecessary_join.fixed b/tests/ui/unnecessary_join.fixed index c85ffdf69a46..7e12c6ae4be9 100644 --- a/tests/ui/unnecessary_join.fixed +++ b/tests/ui/unnecessary_join.fixed @@ -7,14 +7,16 @@ fn main() { let vector = vec!["hello", "world"]; let output = vector .iter() - .map(|item| item.to_uppercase()).collect::(); + .map(|item| item.to_uppercase()) + .collect::(); println!("{}", output); // should be linted let vector = vec!["hello", "world"]; let output = vector .iter() - .map(|item| item.to_uppercase()).collect::(); + .map(|item| item.to_uppercase()) + .collect::(); println!("{}", output); // should not be linted diff --git a/tests/ui/unnecessary_join.stderr b/tests/ui/unnecessary_join.stderr index 5c9f89c854fe..d02d647833d1 100644 --- a/tests/ui/unnecessary_join.stderr +++ b/tests/ui/unnecessary_join.stderr @@ -1,22 +1,20 @@ error: called `.collect>().join("")` on a std::vec::Vec - --> $DIR/unnecessary_join.rs:10:41 + --> $DIR/unnecessary_join.rs:11:10 | -LL | .map(|item| item.to_uppercase()) - | _________________________________________^ -LL | | .collect::>() +LL | .collect::>() + | __________^ LL | | .join(""); - | |_________________^ help: try using: `.collect::()` + | |_________________^ help: try using: `collect::()` | = note: `-D clippy::unnecessary-join` implied by `-D warnings` error: called `.collect>().join("")` on a std::vec::Vec - --> $DIR/unnecessary_join.rs:19:41 + --> $DIR/unnecessary_join.rs:20:10 | -LL | .map(|item| item.to_uppercase()) - | _________________________________________^ -LL | | .collect::>() +LL | .collect::>() + | __________^ LL | | .join(""); - | |_________________^ help: try using: `.collect::()` + | |_________________^ help: try using: `collect::()` error: aborting due to 2 previous errors From bcdc5580c1918762c0a40509087bc4901c731eb0 Mon Sep 17 00:00:00 2001 From: Yoav Lavi Date: Wed, 23 Mar 2022 10:01:56 +0100 Subject: [PATCH 21/35] remove intellij settings --- Cargo.toml | 7 ------- clippy_lints/Cargo.toml | 24 ------------------------ clippy_utils/Cargo.toml | 17 ----------------- 3 files changed, 48 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index a6efbd5823f6..123af23881b6 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -20,13 +20,6 @@ path = "src/main.rs" name = "clippy-driver" path = "src/driver.rs" -[target.'cfg(NOT_A_PLATFORM)'.dependencies] -rustc_driver = { path = "/Users/yoavlavi/dev/rust/compiler/rustc_driver" } -rustc_errors = { path = "/Users/yoavlavi/dev/rust/compiler/rustc_errors" } -rustc_interface = { path = "/Users/yoavlavi/dev/rust/compiler/rustc_interface" } -rustc_session = { path = "/Users/yoavlavi/dev/rust/compiler/rustc_session" } -rustc_span = { path = "/Users/yoavlavi/dev/rust/compiler/rustc_span" } - [dependencies] clippy_lints = { version = "0.1", path = "clippy_lints" } semver = "1.0" diff --git a/clippy_lints/Cargo.toml b/clippy_lints/Cargo.toml index 6e59fbc8f559..66e61660d313 100644 --- a/clippy_lints/Cargo.toml +++ b/clippy_lints/Cargo.toml @@ -8,30 +8,6 @@ license = "MIT OR Apache-2.0" keywords = ["clippy", "lint", "plugin"] edition = "2021" -[target.'cfg(NOT_A_PLATFORM)'.dependencies] -rustc_arena = { path = "/Users/yoavlavi/dev/rust/compiler/rustc_arena" } -rustc_ast = { path = "/Users/yoavlavi/dev/rust/compiler/rustc_ast" } -rustc_ast_pretty = { path = "/Users/yoavlavi/dev/rust/compiler/rustc_ast_pretty" } -rustc_attr = { path = "/Users/yoavlavi/dev/rust/compiler/rustc_attr" } -rustc_data_structures = { path = "/Users/yoavlavi/dev/rust/compiler/rustc_data_structures" } -rustc_driver = { path = "/Users/yoavlavi/dev/rust/compiler/rustc_driver" } -rustc_errors = { path = "/Users/yoavlavi/dev/rust/compiler/rustc_errors" } -rustc_hir = { path = "/Users/yoavlavi/dev/rust/compiler/rustc_hir" } -rustc_hir_pretty = { path = "/Users/yoavlavi/dev/rust/compiler/rustc_hir_pretty" } -rustc_index = { path = "/Users/yoavlavi/dev/rust/compiler/rustc_index" } -rustc_infer = { path = "/Users/yoavlavi/dev/rust/compiler/rustc_infer" } -rustc_lexer = { path = "/Users/yoavlavi/dev/rust/compiler/rustc_lexer" } -rustc_lint = { path = "/Users/yoavlavi/dev/rust/compiler/rustc_lint" } -rustc_middle = { path = "/Users/yoavlavi/dev/rust/compiler/rustc_middle" } -rustc_mir_dataflow = { path = "/Users/yoavlavi/dev/rust/compiler/rustc_mir_dataflow" } -rustc_parse = { path = "/Users/yoavlavi/dev/rust/compiler/rustc_parse" } -rustc_parse_format = { path = "/Users/yoavlavi/dev/rust/compiler/rustc_parse_format" } -rustc_session = { path = "/Users/yoavlavi/dev/rust/compiler/rustc_session" } -rustc_span = { path = "/Users/yoavlavi/dev/rust/compiler/rustc_span" } -rustc_target = { path = "/Users/yoavlavi/dev/rust/compiler/rustc_target" } -rustc_trait_selection = { path = "/Users/yoavlavi/dev/rust/compiler/rustc_trait_selection" } -rustc_typeck = { path = "/Users/yoavlavi/dev/rust/compiler/rustc_typeck" } - [dependencies] cargo_metadata = "0.14" clippy_utils = { path = "../clippy_utils" } diff --git a/clippy_utils/Cargo.toml b/clippy_utils/Cargo.toml index 0b9f88c9971e..d3ed8da4499f 100644 --- a/clippy_utils/Cargo.toml +++ b/clippy_utils/Cargo.toml @@ -4,23 +4,6 @@ version = "0.1.61" edition = "2021" publish = false -[target.'cfg(NOT_A_PLATFORM)'.dependencies] -rustc_ast = { path = "/Users/yoavlavi/dev/rust/compiler/rustc_ast" } -rustc_ast_pretty = { path = "/Users/yoavlavi/dev/rust/compiler/rustc_ast_pretty" } -rustc_attr = { path = "/Users/yoavlavi/dev/rust/compiler/rustc_attr" } -rustc_data_structures = { path = "/Users/yoavlavi/dev/rust/compiler/rustc_data_structures" } -rustc_errors = { path = "/Users/yoavlavi/dev/rust/compiler/rustc_errors" } -rustc_hir = { path = "/Users/yoavlavi/dev/rust/compiler/rustc_hir" } -rustc_infer = { path = "/Users/yoavlavi/dev/rust/compiler/rustc_infer" } -rustc_lexer = { path = "/Users/yoavlavi/dev/rust/compiler/rustc_lexer" } -rustc_lint = { path = "/Users/yoavlavi/dev/rust/compiler/rustc_lint" } -rustc_middle = { path = "/Users/yoavlavi/dev/rust/compiler/rustc_middle" } -rustc_session = { path = "/Users/yoavlavi/dev/rust/compiler/rustc_session" } -rustc_span = { path = "/Users/yoavlavi/dev/rust/compiler/rustc_span" } -rustc_target = { path = "/Users/yoavlavi/dev/rust/compiler/rustc_target" } -rustc_trait_selection = { path = "/Users/yoavlavi/dev/rust/compiler/rustc_trait_selection" } -rustc_typeck = { path = "/Users/yoavlavi/dev/rust/compiler/rustc_typeck" } - [dependencies] arrayvec = { version = "0.7", default-features = false } if_chain = "1.0" From b4fa64d9ca378e7b718c5bacf6a3666e89be7bd0 Mon Sep 17 00:00:00 2001 From: Yoav Lavi Date: Wed, 23 Mar 2022 10:05:20 +0100 Subject: [PATCH 22/35] fix message --- clippy_lints/src/methods/unnecessary_join.rs | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/clippy_lints/src/methods/unnecessary_join.rs b/clippy_lints/src/methods/unnecessary_join.rs index 79e2e71b7a58..31ef3826dcb7 100644 --- a/clippy_lints/src/methods/unnecessary_join.rs +++ b/clippy_lints/src/methods/unnecessary_join.rs @@ -6,7 +6,7 @@ use rustc_errors::Applicability; use rustc_hir as hir; use rustc_lint::LateContext; use rustc_middle::ty; -use rustc_span::{Span, sym}; +use rustc_span::{sym, Span}; use super::UNNECESSARY_JOIN; @@ -34,9 +34,7 @@ pub(super) fn check<'tcx>( context, UNNECESSARY_JOIN, span.with_hi(expr.span.hi()), - &format!( - "called `.collect>().join(\"\")` on a {}", collect_output_type, - ), + "called `.collect>().join(\"\")` on an iterator", "try using", "collect::()".to_owned(), applicability, From 78c107b983a218e427e7df3c1744cb7ec7039dcc Mon Sep 17 00:00:00 2001 From: Yoav Lavi Date: Wed, 23 Mar 2022 10:05:39 +0100 Subject: [PATCH 23/35] cleanup --- clippy_lints/src/methods/unnecessary_join.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/clippy_lints/src/methods/unnecessary_join.rs b/clippy_lints/src/methods/unnecessary_join.rs index 31ef3826dcb7..4e9ea24fd7a2 100644 --- a/clippy_lints/src/methods/unnecessary_join.rs +++ b/clippy_lints/src/methods/unnecessary_join.rs @@ -18,7 +18,6 @@ pub(super) fn check<'tcx>( span: Span, ) { let applicability = Applicability::MachineApplicable; - let collect_output_type = context.typeck_results().expr_ty(join_self_arg); let collect_output_adjusted_type = &context.typeck_results().expr_ty_adjusted(join_self_arg); if_chain! { // the turbofish for collect is ::> From caddb9fa0f2b6e18ea601b24b02b5b54d18bb259 Mon Sep 17 00:00:00 2001 From: Yoav Lavi Date: Wed, 23 Mar 2022 10:09:02 +0100 Subject: [PATCH 24/35] cleanup --- clippy_lints/src/methods/mod.rs | 2 +- clippy_lints/src/methods/unnecessary_join.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs index 94c680a6539c..2f4c534779d5 100644 --- a/clippy_lints/src/methods/mod.rs +++ b/clippy_lints/src/methods/mod.rs @@ -2072,7 +2072,7 @@ declare_clippy_lint! { #[clippy::version = "pre 1.61.0"] pub UNNECESSARY_JOIN, perf, - "using `.collect::>().join(\"\")` on an iterator" + r#"using `.collect::>().join("")` on an iterator"# } pub struct Methods { diff --git a/clippy_lints/src/methods/unnecessary_join.rs b/clippy_lints/src/methods/unnecessary_join.rs index 4e9ea24fd7a2..e08c88c01177 100644 --- a/clippy_lints/src/methods/unnecessary_join.rs +++ b/clippy_lints/src/methods/unnecessary_join.rs @@ -33,7 +33,7 @@ pub(super) fn check<'tcx>( context, UNNECESSARY_JOIN, span.with_hi(expr.span.hi()), - "called `.collect>().join(\"\")` on an iterator", + r#"called `.collect>().join("")` on an iterator"#, "try using", "collect::()".to_owned(), applicability, From 86b652350b94cb20ef35be9693be00c3c2330121 Mon Sep 17 00:00:00 2001 From: Yoav Lavi Date: Wed, 23 Mar 2022 10:12:01 +0100 Subject: [PATCH 25/35] update lints --- CHANGELOG.md | 1 - clippy_lints/src/lib.register_all.rs | 1 - clippy_lints/src/lib.register_lints.rs | 1 - clippy_lints/src/lib.register_perf.rs | 1 - 4 files changed, 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 88f71931d92b..dc83de665548 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3508,7 +3508,6 @@ Released 2018-09-13 [`unnecessary_filter_map`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_filter_map [`unnecessary_find_map`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_find_map [`unnecessary_fold`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_fold -[`unnecessary_join`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_join [`unnecessary_lazy_evaluations`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_lazy_evaluations [`unnecessary_mut_passed`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_mut_passed [`unnecessary_operation`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_operation diff --git a/clippy_lints/src/lib.register_all.rs b/clippy_lints/src/lib.register_all.rs index 26a602d0649e..132a46626762 100644 --- a/clippy_lints/src/lib.register_all.rs +++ b/clippy_lints/src/lib.register_all.rs @@ -196,7 +196,6 @@ store.register_group(true, "clippy::all", Some("clippy_all"), vec![ LintId::of(methods::UNNECESSARY_FILTER_MAP), LintId::of(methods::UNNECESSARY_FIND_MAP), LintId::of(methods::UNNECESSARY_FOLD), - LintId::of(methods::UNNECESSARY_JOIN), LintId::of(methods::UNNECESSARY_LAZY_EVALUATIONS), LintId::of(methods::UNNECESSARY_TO_OWNED), LintId::of(methods::UNWRAP_OR_ELSE_DEFAULT), diff --git a/clippy_lints/src/lib.register_lints.rs b/clippy_lints/src/lib.register_lints.rs index 21f1ef562b5a..65ad64f19018 100644 --- a/clippy_lints/src/lib.register_lints.rs +++ b/clippy_lints/src/lib.register_lints.rs @@ -334,7 +334,6 @@ store.register_lints(&[ methods::UNNECESSARY_FILTER_MAP, methods::UNNECESSARY_FIND_MAP, methods::UNNECESSARY_FOLD, - methods::UNNECESSARY_JOIN, methods::UNNECESSARY_LAZY_EVALUATIONS, methods::UNNECESSARY_TO_OWNED, methods::UNWRAP_OR_ELSE_DEFAULT, diff --git a/clippy_lints/src/lib.register_perf.rs b/clippy_lints/src/lib.register_perf.rs index c3bdd16acc49..f2f5c988d8f9 100644 --- a/clippy_lints/src/lib.register_perf.rs +++ b/clippy_lints/src/lib.register_perf.rs @@ -19,7 +19,6 @@ store.register_group(true, "clippy::perf", Some("clippy_perf"), vec![ LintId::of(methods::MANUAL_STR_REPEAT), LintId::of(methods::OR_FUN_CALL), LintId::of(methods::SINGLE_CHAR_PATTERN), - LintId::of(methods::UNNECESSARY_JOIN), LintId::of(methods::UNNECESSARY_TO_OWNED), LintId::of(misc::CMP_OWNED), LintId::of(redundant_clone::REDUNDANT_CLONE), From dccddd6fa5c5cc2ae91db79f411751d75ea9fb3e Mon Sep 17 00:00:00 2001 From: Yoav Lavi Date: Wed, 23 Mar 2022 10:21:19 +0100 Subject: [PATCH 26/35] temporarily remove unknown lint --- tests/ui/unnecessary_join.rs | 2 -- 1 file changed, 2 deletions(-) diff --git a/tests/ui/unnecessary_join.rs b/tests/ui/unnecessary_join.rs index 0a21656a7558..69c0693b7394 100644 --- a/tests/ui/unnecessary_join.rs +++ b/tests/ui/unnecessary_join.rs @@ -1,7 +1,5 @@ // run-rustfix -#![warn(clippy::unnecessary_join)] - fn main() { // should be linted let vector = vec!["hello", "world"]; From c4de8cffc71ede50aaee55b8b0f619685e5403a1 Mon Sep 17 00:00:00 2001 From: Yoav Lavi Date: Wed, 23 Mar 2022 10:28:45 +0100 Subject: [PATCH 27/35] update tests --- tests/ui/unnecessary_join.fixed | 2 -- tests/ui/unnecessary_join.stderr | 8 ++++---- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/tests/ui/unnecessary_join.fixed b/tests/ui/unnecessary_join.fixed index 7e12c6ae4be9..a16dd2d33263 100644 --- a/tests/ui/unnecessary_join.fixed +++ b/tests/ui/unnecessary_join.fixed @@ -1,7 +1,5 @@ // run-rustfix -#![warn(clippy::unnecessary_join)] - fn main() { // should be linted let vector = vec!["hello", "world"]; diff --git a/tests/ui/unnecessary_join.stderr b/tests/ui/unnecessary_join.stderr index d02d647833d1..9f57d5f17463 100644 --- a/tests/ui/unnecessary_join.stderr +++ b/tests/ui/unnecessary_join.stderr @@ -1,5 +1,5 @@ -error: called `.collect>().join("")` on a std::vec::Vec - --> $DIR/unnecessary_join.rs:11:10 +error: called `.collect>().join("")` on an iterator + --> $DIR/unnecessary_join.rs:9:10 | LL | .collect::>() | __________^ @@ -8,8 +8,8 @@ LL | | .join(""); | = note: `-D clippy::unnecessary-join` implied by `-D warnings` -error: called `.collect>().join("")` on a std::vec::Vec - --> $DIR/unnecessary_join.rs:20:10 +error: called `.collect>().join("")` on an iterator + --> $DIR/unnecessary_join.rs:18:10 | LL | .collect::>() | __________^ From fbe086f1eade6729e76b1d9954505f50c90a0d65 Mon Sep 17 00:00:00 2001 From: Yoav Lavi Date: Wed, 23 Mar 2022 10:32:38 +0100 Subject: [PATCH 28/35] cleanup --- clippy_lints/src/methods/unnecessary_join.rs | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/clippy_lints/src/methods/unnecessary_join.rs b/clippy_lints/src/methods/unnecessary_join.rs index e08c88c01177..00121f988816 100644 --- a/clippy_lints/src/methods/unnecessary_join.rs +++ b/clippy_lints/src/methods/unnecessary_join.rs @@ -1,28 +1,27 @@ use clippy_utils::diagnostics::span_lint_and_sugg; use clippy_utils::ty::is_type_diagnostic_item; -use hir::ExprKind; use rustc_ast::ast::LitKind; use rustc_errors::Applicability; -use rustc_hir as hir; +use rustc_hir::{Expr, ExprKind}; use rustc_lint::LateContext; -use rustc_middle::ty; +use rustc_middle::ty::{Ref, Slice}; use rustc_span::{sym, Span}; use super::UNNECESSARY_JOIN; pub(super) fn check<'tcx>( context: &LateContext<'tcx>, - join_self_arg: &'tcx hir::Expr<'tcx>, - join_arg: &'tcx hir::Expr<'tcx>, - expr: &'tcx hir::Expr<'tcx>, + join_self_arg: &'tcx Expr<'tcx>, + join_arg: &'tcx Expr<'tcx>, + expr: &'tcx Expr<'tcx>, span: Span, ) { let applicability = Applicability::MachineApplicable; let collect_output_adjusted_type = &context.typeck_results().expr_ty_adjusted(join_self_arg); if_chain! { // the turbofish for collect is ::> - if let ty::Ref(_, ref_type, _) = collect_output_adjusted_type.kind(); - if let ty::Slice(slice) = ref_type.kind(); + if let Ref(_, ref_type, _) = collect_output_adjusted_type.kind(); + if let Slice(slice) = ref_type.kind(); if is_type_diagnostic_item(context, *slice, sym::String); // the argument for join is "" if let ExprKind::Lit(spanned) = &join_arg.kind; From 95a7aa22c0e678e9f61c5fb34fb9d023c962e85a Mon Sep 17 00:00:00 2001 From: Yoav Lavi Date: Wed, 23 Mar 2022 10:33:59 +0100 Subject: [PATCH 29/35] remove unneeded ref --- clippy_lints/src/methods/unnecessary_join.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clippy_lints/src/methods/unnecessary_join.rs b/clippy_lints/src/methods/unnecessary_join.rs index 00121f988816..9652a5dafa43 100644 --- a/clippy_lints/src/methods/unnecessary_join.rs +++ b/clippy_lints/src/methods/unnecessary_join.rs @@ -17,7 +17,7 @@ pub(super) fn check<'tcx>( span: Span, ) { let applicability = Applicability::MachineApplicable; - let collect_output_adjusted_type = &context.typeck_results().expr_ty_adjusted(join_self_arg); + let collect_output_adjusted_type = context.typeck_results().expr_ty_adjusted(join_self_arg); if_chain! { // the turbofish for collect is ::> if let Ref(_, ref_type, _) = collect_output_adjusted_type.kind(); From 88be26555453dc860d872541372bdc67dc4ed130 Mon Sep 17 00:00:00 2001 From: Yoav Lavi Date: Wed, 23 Mar 2022 10:55:44 +0100 Subject: [PATCH 30/35] order arguments --- clippy_lints/src/methods/mod.rs | 2 +- clippy_lints/src/methods/unnecessary_join.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs index 2f4c534779d5..be70472d9b63 100644 --- a/clippy_lints/src/methods/mod.rs +++ b/clippy_lints/src/methods/mod.rs @@ -2458,7 +2458,7 @@ fn check_methods<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>, msrv: Optio ("is_some", []) => check_is_some_is_none(cx, expr, recv, true), ("join", [join_arg]) => { if let Some(("collect", _, span)) = method_call(recv) { - unnecessary_join::check(cx, recv, join_arg, expr, span); + unnecessary_join::check(cx, expr, recv, join_arg, span); } }, ("last", args @ []) | ("skip", args @ [_]) => { diff --git a/clippy_lints/src/methods/unnecessary_join.rs b/clippy_lints/src/methods/unnecessary_join.rs index 9652a5dafa43..c11bd6d9c95b 100644 --- a/clippy_lints/src/methods/unnecessary_join.rs +++ b/clippy_lints/src/methods/unnecessary_join.rs @@ -11,9 +11,9 @@ use super::UNNECESSARY_JOIN; pub(super) fn check<'tcx>( context: &LateContext<'tcx>, + expr: &'tcx Expr<'tcx>, join_self_arg: &'tcx Expr<'tcx>, join_arg: &'tcx Expr<'tcx>, - expr: &'tcx Expr<'tcx>, span: Span, ) { let applicability = Applicability::MachineApplicable; From 509031f44c7a0950bb60229c14196ab3c55e4c27 Mon Sep 17 00:00:00 2001 From: Yoav Lavi Date: Wed, 23 Mar 2022 10:58:58 +0100 Subject: [PATCH 31/35] normalize argument names --- clippy_lints/src/methods/unnecessary_join.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/clippy_lints/src/methods/unnecessary_join.rs b/clippy_lints/src/methods/unnecessary_join.rs index c11bd6d9c95b..6586f2351d15 100644 --- a/clippy_lints/src/methods/unnecessary_join.rs +++ b/clippy_lints/src/methods/unnecessary_join.rs @@ -10,26 +10,26 @@ use rustc_span::{sym, Span}; use super::UNNECESSARY_JOIN; pub(super) fn check<'tcx>( - context: &LateContext<'tcx>, + cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>, join_self_arg: &'tcx Expr<'tcx>, join_arg: &'tcx Expr<'tcx>, span: Span, ) { let applicability = Applicability::MachineApplicable; - let collect_output_adjusted_type = context.typeck_results().expr_ty_adjusted(join_self_arg); + let collect_output_adjusted_type = cx.typeck_results().expr_ty_adjusted(join_self_arg); if_chain! { // the turbofish for collect is ::> if let Ref(_, ref_type, _) = collect_output_adjusted_type.kind(); if let Slice(slice) = ref_type.kind(); - if is_type_diagnostic_item(context, *slice, sym::String); + if is_type_diagnostic_item(cx, *slice, sym::String); // the argument for join is "" if let ExprKind::Lit(spanned) = &join_arg.kind; if let LitKind::Str(symbol, _) = spanned.node; if symbol.is_empty(); then { span_lint_and_sugg( - context, + cx, UNNECESSARY_JOIN, span.with_hi(expr.span.hi()), r#"called `.collect>().join("")` on an iterator"#, From ecfffb13f05e78931c374db6afb19e813512b1f9 Mon Sep 17 00:00:00 2001 From: Yoav Lavi Date: Wed, 23 Mar 2022 11:01:40 +0100 Subject: [PATCH 32/35] cleanup --- clippy_lints/src/methods/unnecessary_join.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/clippy_lints/src/methods/unnecessary_join.rs b/clippy_lints/src/methods/unnecessary_join.rs index 6586f2351d15..973b8a7e6bf6 100644 --- a/clippy_lints/src/methods/unnecessary_join.rs +++ b/clippy_lints/src/methods/unnecessary_join.rs @@ -1,5 +1,4 @@ -use clippy_utils::diagnostics::span_lint_and_sugg; -use clippy_utils::ty::is_type_diagnostic_item; +use clippy_utils::{diagnostics::span_lint_and_sugg, ty::is_type_diagnostic_item}; use rustc_ast::ast::LitKind; use rustc_errors::Applicability; use rustc_hir::{Expr, ExprKind}; From 53dfc753a6f5d85a0956c7f72166ff79094fb078 Mon Sep 17 00:00:00 2001 From: Yoav Lavi Date: Thu, 24 Mar 2022 12:00:55 +0100 Subject: [PATCH 33/35] fix version --- clippy_lints/src/methods/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs index be70472d9b63..28a0ea357f39 100644 --- a/clippy_lints/src/methods/mod.rs +++ b/clippy_lints/src/methods/mod.rs @@ -2069,7 +2069,7 @@ declare_clippy_lint! { /// let output = vector.iter().map(|item| item.to_uppercase()).collect::(); /// println!("{}", output); /// ``` - #[clippy::version = "pre 1.61.0"] + #[clippy::version = "1.61.0"] pub UNNECESSARY_JOIN, perf, r#"using `.collect::>().join("")` on an iterator"# From 61bc156d60d06e341b34e62761f4a2b4716cf167 Mon Sep 17 00:00:00 2001 From: Yoav Lavi Date: Thu, 24 Mar 2022 12:11:32 +0100 Subject: [PATCH 34/35] switch to pedantic, add warning --- clippy_lints/src/methods/mod.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs index 28a0ea357f39..3c8d6faed37c 100644 --- a/clippy_lints/src/methods/mod.rs +++ b/clippy_lints/src/methods/mod.rs @@ -2069,9 +2069,13 @@ declare_clippy_lint! { /// let output = vector.iter().map(|item| item.to_uppercase()).collect::(); /// println!("{}", output); /// ``` + /// ### Known problems + /// While `.collect::()` is more performant in most cases, there are cases where + /// using `.collect::()` over `.collect::>().join("")` + /// will prevent loop unrolling and will result in a negative performance impact. #[clippy::version = "1.61.0"] pub UNNECESSARY_JOIN, - perf, + pedantic, r#"using `.collect::>().join("")` on an iterator"# } From 8f706ef27c42216bd80403e33f71fb607812d898 Mon Sep 17 00:00:00 2001 From: Yoav Lavi Date: Thu, 24 Mar 2022 12:27:06 +0100 Subject: [PATCH 35/35] fix tests and update lints --- CHANGELOG.md | 1 + clippy_lints/src/lib.register_lints.rs | 1 + clippy_lints/src/lib.register_pedantic.rs | 1 + clippy_lints/src/methods/mod.rs | 2 +- tests/ui/unnecessary_join.fixed | 2 ++ tests/ui/unnecessary_join.rs | 2 ++ tests/ui/unnecessary_join.stderr | 4 ++-- 7 files changed, 10 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index dc83de665548..88f71931d92b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3508,6 +3508,7 @@ Released 2018-09-13 [`unnecessary_filter_map`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_filter_map [`unnecessary_find_map`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_find_map [`unnecessary_fold`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_fold +[`unnecessary_join`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_join [`unnecessary_lazy_evaluations`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_lazy_evaluations [`unnecessary_mut_passed`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_mut_passed [`unnecessary_operation`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_operation diff --git a/clippy_lints/src/lib.register_lints.rs b/clippy_lints/src/lib.register_lints.rs index 65ad64f19018..21f1ef562b5a 100644 --- a/clippy_lints/src/lib.register_lints.rs +++ b/clippy_lints/src/lib.register_lints.rs @@ -334,6 +334,7 @@ store.register_lints(&[ methods::UNNECESSARY_FILTER_MAP, methods::UNNECESSARY_FIND_MAP, methods::UNNECESSARY_FOLD, + methods::UNNECESSARY_JOIN, methods::UNNECESSARY_LAZY_EVALUATIONS, methods::UNNECESSARY_TO_OWNED, methods::UNWRAP_OR_ELSE_DEFAULT, diff --git a/clippy_lints/src/lib.register_pedantic.rs b/clippy_lints/src/lib.register_pedantic.rs index 00d305131810..eb6534cb8cae 100644 --- a/clippy_lints/src/lib.register_pedantic.rs +++ b/clippy_lints/src/lib.register_pedantic.rs @@ -63,6 +63,7 @@ store.register_group(true, "clippy::pedantic", Some("clippy_pedantic"), vec![ LintId::of(methods::IMPLICIT_CLONE), LintId::of(methods::INEFFICIENT_TO_STRING), LintId::of(methods::MAP_UNWRAP_OR), + LintId::of(methods::UNNECESSARY_JOIN), LintId::of(misc::FLOAT_CMP), LintId::of(misc::USED_UNDERSCORE_BINDING), LintId::of(mut_mut::MUT_MUT), diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs index 3c8d6faed37c..9d4e1fa39940 100644 --- a/clippy_lints/src/methods/mod.rs +++ b/clippy_lints/src/methods/mod.rs @@ -2076,7 +2076,7 @@ declare_clippy_lint! { #[clippy::version = "1.61.0"] pub UNNECESSARY_JOIN, pedantic, - r#"using `.collect::>().join("")` on an iterator"# + "using `.collect::>().join(\"\")` on an iterator" } pub struct Methods { diff --git a/tests/ui/unnecessary_join.fixed b/tests/ui/unnecessary_join.fixed index a16dd2d33263..7e12c6ae4be9 100644 --- a/tests/ui/unnecessary_join.fixed +++ b/tests/ui/unnecessary_join.fixed @@ -1,5 +1,7 @@ // run-rustfix +#![warn(clippy::unnecessary_join)] + fn main() { // should be linted let vector = vec!["hello", "world"]; diff --git a/tests/ui/unnecessary_join.rs b/tests/ui/unnecessary_join.rs index 69c0693b7394..0a21656a7558 100644 --- a/tests/ui/unnecessary_join.rs +++ b/tests/ui/unnecessary_join.rs @@ -1,5 +1,7 @@ // run-rustfix +#![warn(clippy::unnecessary_join)] + fn main() { // should be linted let vector = vec!["hello", "world"]; diff --git a/tests/ui/unnecessary_join.stderr b/tests/ui/unnecessary_join.stderr index 9f57d5f17463..0b14b143affd 100644 --- a/tests/ui/unnecessary_join.stderr +++ b/tests/ui/unnecessary_join.stderr @@ -1,5 +1,5 @@ error: called `.collect>().join("")` on an iterator - --> $DIR/unnecessary_join.rs:9:10 + --> $DIR/unnecessary_join.rs:11:10 | LL | .collect::>() | __________^ @@ -9,7 +9,7 @@ LL | | .join(""); = note: `-D clippy::unnecessary-join` implied by `-D warnings` error: called `.collect>().join("")` on an iterator - --> $DIR/unnecessary_join.rs:18:10 + --> $DIR/unnecessary_join.rs:20:10 | LL | .collect::>() | __________^