new lint: format_collect

Author: y21

clippy_lints/src/declared_lints.rs

@@ -336,6 +336,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[
     crate::methods::FILTER_NEXT_INFO,
     crate::methods::FLAT_MAP_IDENTITY_INFO,
     crate::methods::FLAT_MAP_OPTION_INFO,
+    crate::methods::FORMAT_COLLECT_INFO,
     crate::methods::FROM_ITER_INSTEAD_OF_COLLECT_INFO,
     crate::methods::GET_FIRST_INFO,
     crate::methods::GET_LAST_WITH_LEN_INFO,

clippy_lints/src/methods/format_collect.rs

@@ -0,0 +1,33 @@
+use super::FORMAT_COLLECT;
+use clippy_utils::diagnostics::span_lint_and_then;
+use clippy_utils::macros::is_format_macro;
+use clippy_utils::macros::root_macro_call_first_node;
+use clippy_utils::ty::is_type_lang_item;
+use rustc_hir::Expr;
+use rustc_hir::ExprKind;
+use rustc_hir::LangItem;
+use rustc_lint::LateContext;
+use rustc_span::Span;
+
+fn tail_expr<'tcx>(expr: &'tcx Expr<'tcx>) -> Option<&'tcx Expr<'tcx>> {
+    match expr.kind {
+        ExprKind::Block(block, _) if !expr.span.from_expansion() => block.expr,
+        _ => Some(expr),
+    }
+}
+
+pub(super) fn check(cx: &LateContext<'_>, expr: &Expr<'_>, map_arg: &Expr<'_>, map_span: Span) {
+    if is_type_lang_item(cx, cx.typeck_results().expr_ty(expr), LangItem::String)
+        && let ExprKind::Closure(closure) = map_arg.kind
+        && let body = cx.tcx.hir().body(closure.body)
+        && let Some(value) = tail_expr(body.value)
+        && let Some(mac) = root_macro_call_first_node(cx, value)
+        && is_format_macro(cx, mac.def_id)
+    {
+        span_lint_and_then(cx, FORMAT_COLLECT, expr.span, "use of `format!` to build up a string from an iterator", |diag| {
+            diag.span_help(map_span, "call `fold` instead")
+                .span_help(value.span.source_callsite(), "... and use the `write!` macro here")
+                .note("this can be written more efficiently by appending to a `String` directly");
+        });
+    }
+}

clippy_lints/src/methods/mod.rs

@@ -26,6 +26,7 @@ mod filter_map_next;
 mod filter_next;
 mod flat_map_identity;
 mod flat_map_option;
+mod format_collect;
 mod from_iter_instead_of_collect;
 mod get_first;
 mod get_last_with_len;
@@ -3348,6 +3349,35 @@ declare_clippy_lint! {
     "checks for usage of `Iterator::fold` with a type that implements `Try`"
 }
 
+declare_clippy_lint! {
+    /// ### What it does
+    /// Checks for usage of `.map(|_| format!(..)).collect::<String>()`.
+    ///
+    /// ### Why is this bad?
+    /// This allocates a new string for every element in the iterator.
+    /// This can be done more efficiently by creating the `String` once and appending to it using `Iterator::fold`.
+    ///
+    /// ### Example
+    /// ```rust
+    /// fn hex_encode(bytes: &[u8]) -> String {
+    ///     bytes.iter().map(|b| format!("{b:02X}")).collect()
+    /// }
+    /// ```
+    /// Use instead:
+    /// ```rust
+    /// fn hex_encode(bytes: &[u8]) -> String {
+    ///     bytes.iter().fold(String::new(), |mut output, b| {
+    ///         let _ = write!(output, "{b:02X}");
+    ///         output
+    ///     })
+    /// }
+    /// ```
+    #[clippy::version = "1.72.0"]
+    pub FORMAT_COLLECT,
+    perf,
+    "`format!`ing every element in a collection, then collecting the strings into a new `String`"
+}
+
 pub struct Methods {
     avoid_breaking_exported_api: bool,
     msrv: Msrv,
@@ -3481,6 +3511,7 @@ impl_lint_pass!(Methods => [
     UNNECESSARY_LITERAL_UNWRAP,
     DRAIN_COLLECT,
     MANUAL_TRY_FOLD,
+    FORMAT_COLLECT,
 ]);
 
 /// Extracts a method call name, args, and `Span` of the method name.
@@ -3702,8 +3733,9 @@ impl Methods {
                         Some((name @ ("cloned" | "copied"), recv2, [], _, _)) => {
                             iter_cloned_collect::check(cx, name, expr, recv2);
                         },
-                        Some(("map", m_recv, [m_arg], _, _)) => {
+                        Some(("map", m_recv, [m_arg], m_ident_span, _)) => {
                             map_collect_result_unit::check(cx, expr, m_recv, m_arg);
+                            format_collect::check(cx, expr, m_arg, m_ident_span);
                         },
                         Some(("take", take_self_arg, [take_arg], _, _)) => {
                             if self.msrv.meets(msrvs::STR_REPEAT) {

tests/ui/format_collect.rs

@@ -0,0 +1,26 @@
+#![allow(unused, dead_code)]
+#![warn(clippy::format_collect)]
+
+fn hex_encode(bytes: &[u8]) -> String {
+    bytes.iter().map(|b| format!("{b:02X}")).collect()
+}
+
+macro_rules! fmt {
+    ($x:ident) => {
+        format!("{x:02X}", x = $x)
+    };
+}
+
+fn from_macro(bytes: &[u8]) -> String {
+    bytes.iter().map(|x| fmt!(x)).collect()
+}
+
+fn with_block() -> String {
+    (1..10)
+        .map(|s| {
+            let y = 1;
+            format!("{s} {y}")
+        })
+        .collect()
+}
+fn main() {}

tests/ui/format_collect.stderr

@@ -0,0 +1,44 @@
+error: use of `format!` to build up a string from an iterator
+  --> $DIR/format_collect.rs:5:5
+   |
+LL |     bytes.iter().map(|b| format!("{b:02X}")).collect()
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+help: call `fold` instead
+  --> $DIR/format_collect.rs:5:18
+   |
+LL |     bytes.iter().map(|b| format!("{b:02X}")).collect()
+   |                  ^^^
+help: ... and use the `write!` macro here
+  --> $DIR/format_collect.rs:5:26
+   |
+LL |     bytes.iter().map(|b| format!("{b:02X}")).collect()
+   |                          ^^^^^^^^^^^^^^^^^^
+   = note: this can be written more efficiently by appending to a `String` directly
+   = note: `-D clippy::format-collect` implied by `-D warnings`
+
+error: use of `format!` to build up a string from an iterator
+  --> $DIR/format_collect.rs:19:5
+   |
+LL | /     (1..10)
+LL | |         .map(|s| {
+LL | |             let y = 1;
+LL | |             format!("{s} {y}")
+LL | |         })
+LL | |         .collect()
+   | |__________________^
+   |
+help: call `fold` instead
+  --> $DIR/format_collect.rs:20:10
+   |
+LL |         .map(|s| {
+   |          ^^^
+help: ... and use the `write!` macro here
+  --> $DIR/format_collect.rs:22:13
+   |
+LL |             format!("{s} {y}")
+   |             ^^^^^^^^^^^^^^^^^^
+   = note: this can be written more efficiently by appending to a `String` directly
+
+error: aborting due to 2 previous errors
+