document that `write!`ing into a string never fails

4 files changed, 41 insertions, 13 deletions

diff --git a/clippy_lints/src/methods/format_collect.rs b/clippy_lints/src/methods/format_collect.rs
index 74eb80782d6..1f8863f8521 100644
--- a/clippy_lints/src/methods/format_collect.rs
+++ b/clippy_lints/src/methods/format_collect.rs
@@ -1,17 +1,17 @@
 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::macros::{is_format_macro, 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_hir::{Expr, ExprKind, LangItem};
 use rustc_lint::LateContext;
 use rustc_span::Span;
 
-fn tail_expr<'tcx>(expr: &'tcx Expr<'tcx>) -> Option<&'tcx Expr<'tcx>> {
+/// Same as `peel_blocks` but only actually considers blocks that are not from an expansion.
+/// This is needed because always calling `peel_blocks` would otherwise remove parts of the
+/// `format!` macro, which would cause `root_macro_call_first_node` to return `None`.
+fn peel_non_expn_blocks<'tcx>(expr: &'tcx Expr<'tcx>) -> Option<&'tcx Expr<'tcx>> {
     match expr.kind {
-        ExprKind::Block(block, _) if !expr.span.from_expansion() => block.expr,
+        ExprKind::Block(block, _) if !expr.span.from_expansion() => peel_non_expn_blocks(block.expr?),
         _ => Some(expr),
     }
 }
@@ -20,7 +20,7 @@ pub(super) fn check(cx: &LateContext<'_>, expr: &Expr<'_>, map_arg: &Expr<'_>, m
     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(value) = peel_non_expn_blocks(body.value)
         && let Some(mac) = root_macro_call_first_node(cx, value)
         && is_format_macro(cx, mac.def_id)
     {
diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs
index f55a7727fad..81f59f68ec6 100644
--- a/clippy_lints/src/methods/mod.rs
+++ b/clippy_lints/src/methods/mod.rs
@@ -3385,7 +3385,12 @@ declare_clippy_lint! {
     ///
     /// ### 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`.
+    /// This can be done more efficiently by creating the `String` once and appending to it in `Iterator::fold`,
+    /// using either the `write!` macro which supports exactly the same syntax as the `format!` macro,
+    /// or concatenating with `+` in case the iterator yields `&str`/`String`.
+    ///
+    /// Note also that `write!`-ing into a `String` can never fail, despite the return type of `write!` being `std::fmt::Result`,
+    /// so it can be safely ignored or unwrapped.
     ///
     /// ### Example
     /// ```rust
diff --git a/tests/ui/format_collect.rs b/tests/ui/format_collect.rs
index ecadb43ecb4..c7f2b7b6950 100644
--- a/tests/ui/format_collect.rs
+++ b/tests/ui/format_collect.rs
@@ -5,6 +5,11 @@ fn hex_encode(bytes: &[u8]) -> String {
     bytes.iter().map(|b| format!("{b:02X}")).collect()
 }
 
+#[rustfmt::skip]
+fn hex_encode_deep(bytes: &[u8]) -> String {
+    bytes.iter().map(|b| {{{{{ format!("{b:02X}") }}}}}).collect()
+}
+
 macro_rules! fmt {
     ($x:ident) => {
         format!("{x:02X}", x = $x)
diff --git a/tests/ui/format_collect.stderr b/tests/ui/format_collect.stderr
index cd71a853762..d918f1ed466 100644
--- a/tests/ui/format_collect.stderr
+++ b/tests/ui/format_collect.stderr
@@ -18,7 +18,25 @@ LL |     bytes.iter().map(|b| format!("{b:02X}")).collect()
    = 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
+  --> $DIR/format_collect.rs:10:5
+   |
+LL |     bytes.iter().map(|b| {{{{{ format!("{b:02X}") }}}}}).collect()
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+help: call `fold` instead
+  --> $DIR/format_collect.rs:10:18
+   |
+LL |     bytes.iter().map(|b| {{{{{ format!("{b:02X}") }}}}}).collect()
+   |                  ^^^
+help: ... and use the `write!` macro here
+  --> $DIR/format_collect.rs:10:32
+   |
+LL |     bytes.iter().map(|b| {{{{{ format!("{b:02X}") }}}}}).collect()
+   |                                ^^^^^^^^^^^^^^^^^^
+   = note: this can be written more efficiently by appending to a `String` directly
+
+error: use of `format!` to build up a string from an iterator
+  --> $DIR/format_collect.rs:24:5
    |
 LL | /     (1..10)
 LL | |         .map(|s| {
@@ -29,16 +47,16 @@ LL | |         .collect()
    | |__________________^
    |
 help: call `fold` instead
-  --> $DIR/format_collect.rs:20:10
+  --> $DIR/format_collect.rs:25:10
    |
 LL |         .map(|s| {
    |          ^^^
 help: ... and use the `write!` macro here
-  --> $DIR/format_collect.rs:22:13
+  --> $DIR/format_collect.rs:27: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
+error: aborting due to 3 previous errors