document that write!ing into a string never fails

This commit is contained in:
y21 2023-07-07 18:20:36 +02:00
parent c3881569af
commit c83d58f507
4 changed files with 41 additions and 13 deletions

View File

@ -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)
{

View File

@ -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

View File

@ -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)

View File

@ -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