diff options
| -rw-r--r-- | CHANGELOG.md | 1 | ||||
| -rw-r--r-- | clippy_lints/src/lib.register_lints.rs | 1 | ||||
| -rw-r--r-- | clippy_lints/src/lib.register_pedantic.rs | 1 | ||||
| -rw-r--r-- | clippy_lints/src/methods/mod.rs | 36 | ||||
| -rw-r--r-- | clippy_lints/src/methods/unnecessary_join.rs | 41 | ||||
| -rw-r--r-- | tests/ui/unnecessary_join.fixed | 35 | ||||
| -rw-r--r-- | tests/ui/unnecessary_join.rs | 37 | ||||
| -rw-r--r-- | tests/ui/unnecessary_join.stderr | 20 |
8 files changed, 172 insertions, 0 deletions
diff --git a/CHANGELOG.md b/CHANGELOG.md index dc83de66554..88f71931d92 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 65ad64f1901..21f1ef562b5 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 00d30513181..eb6534cb8ca 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 c586a0444f0..9d4e1fa3994 100644 --- a/clippy_lints/src/methods/mod.rs +++ b/clippy_lints/src/methods/mod.rs @@ -60,6 +60,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; @@ -2049,6 +2050,35 @@ declare_clippy_lint! { "unnecessary calls to `to_owned`-like functions" } +declare_clippy_lint! { + /// ### What it does + /// Checks for use of `.collect::<Vec<String>>().join("")` on iterators. + /// + /// ### Why is this bad? + /// `.collect::<String>()` is more concise and usually more performant + /// + /// ### Example + /// ```rust + /// let vector = vec!["hello", "world"]; + /// let output = vector.iter().map(|item| item.to_uppercase()).collect::<Vec<String>>().join(""); + /// println!("{}", output); + /// ``` + /// The correct use would be: + /// ```rust + /// let vector = vec!["hello", "world"]; + /// let output = vector.iter().map(|item| item.to_uppercase()).collect::<String>(); + /// println!("{}", output); + /// ``` + /// ### Known problems + /// While `.collect::<String>()` is more performant in most cases, there are cases where + /// using `.collect::<String>()` over `.collect::<Vec<String>>().join("")` + /// will prevent loop unrolling and will result in a negative performance impact. + #[clippy::version = "1.61.0"] + pub UNNECESSARY_JOIN, + pedantic, + "using `.collect::<Vec<String>>().join(\"\")` on an iterator" +} + pub struct Methods { avoid_breaking_exported_api: bool, msrv: Option<RustcVersion>, @@ -2134,6 +2164,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. @@ -2429,6 +2460,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", [join_arg]) => { + if let Some(("collect", _, span)) = method_call(recv) { + unnecessary_join::check(cx, expr, recv, join_arg, span); + } + }, ("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 00000000000..973b8a7e6bf --- /dev/null +++ b/clippy_lints/src/methods/unnecessary_join.rs @@ -0,0 +1,41 @@ +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}; +use rustc_lint::LateContext; +use rustc_middle::ty::{Ref, Slice}; +use rustc_span::{sym, Span}; + +use super::UNNECESSARY_JOIN; + +pub(super) fn check<'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 = cx.typeck_results().expr_ty_adjusted(join_self_arg); + if_chain! { + // the turbofish for collect is ::<Vec<String>> + if let Ref(_, ref_type, _) = collect_output_adjusted_type.kind(); + if let Slice(slice) = ref_type.kind(); + 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( + cx, + UNNECESSARY_JOIN, + span.with_hi(expr.span.hi()), + r#"called `.collect<Vec<String>>().join("")` on an iterator"#, + "try using", + "collect::<String>()".to_owned(), + applicability, + ); + } + } +} diff --git a/tests/ui/unnecessary_join.fixed b/tests/ui/unnecessary_join.fixed new file mode 100644 index 00000000000..7e12c6ae4be --- /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::<String>(); + println!("{}", output); + + // should be linted + let vector = vec!["hello", "world"]; + let output = vector + .iter() + .map(|item| item.to_uppercase()) + .collect::<String>(); + println!("{}", output); + + // should not be linted + let vector = vec!["hello", "world"]; + let output = vector + .iter() + .map(|item| item.to_uppercase()) + .collect::<Vec<String>>() + .join("\n"); + println!("{}", output); + + // should not be linted + let vector = vec!["hello", "world"]; + let output = vector.iter().map(|item| item.to_uppercase()).collect::<String>(); + println!("{}", output); +} diff --git a/tests/ui/unnecessary_join.rs b/tests/ui/unnecessary_join.rs new file mode 100644 index 00000000000..0a21656a755 --- /dev/null +++ b/tests/ui/unnecessary_join.rs @@ -0,0 +1,37 @@ +// 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::<Vec<String>>() + .join(""); + println!("{}", output); + + // should be linted + let vector = vec!["hello", "world"]; + let output = vector + .iter() + .map(|item| item.to_uppercase()) + .collect::<Vec<_>>() + .join(""); + println!("{}", output); + + // should not be linted + let vector = vec!["hello", "world"]; + let output = vector + .iter() + .map(|item| item.to_uppercase()) + .collect::<Vec<String>>() + .join("\n"); + println!("{}", output); + + // should not be linted + let vector = vec!["hello", "world"]; + let output = vector.iter().map(|item| item.to_uppercase()).collect::<String>(); + println!("{}", output); +} diff --git a/tests/ui/unnecessary_join.stderr b/tests/ui/unnecessary_join.stderr new file mode 100644 index 00000000000..0b14b143aff --- /dev/null +++ b/tests/ui/unnecessary_join.stderr @@ -0,0 +1,20 @@ +error: called `.collect<Vec<String>>().join("")` on an iterator + --> $DIR/unnecessary_join.rs:11:10 + | +LL | .collect::<Vec<String>>() + | __________^ +LL | | .join(""); + | |_________________^ help: try using: `collect::<String>()` + | + = note: `-D clippy::unnecessary-join` implied by `-D warnings` + +error: called `.collect<Vec<String>>().join("")` on an iterator + --> $DIR/unnecessary_join.rs:20:10 + | +LL | .collect::<Vec<_>>() + | __________^ +LL | | .join(""); + | |_________________^ help: try using: `collect::<String>()` + +error: aborting due to 2 previous errors + |