From fcf094281be1abcc24b0fa5609a08a3567e6748b Mon Sep 17 00:00:00 2001 From: Yuri Astrakhan Date: Fri, 20 Feb 2026 01:18:58 -0500 Subject: [PATCH] fix(useless_format): improve suggestion --- clippy_lints/src/format.rs | 74 ++++++++++++++++++++++++------------- tests/ui/format.fixed | 18 +++++++++ tests/ui/format.rs | 18 +++++++++ tests/ui/format.stderr | 76 +++++++++++++++++++++++++++++++++++--- 4 files changed, 155 insertions(+), 31 deletions(-) diff --git a/clippy_lints/src/format.rs b/clippy_lints/src/format.rs index f8e4a779da6f..be48dd3695ca 100644 --- a/clippy_lints/src/format.rs +++ b/clippy_lints/src/format.rs @@ -1,10 +1,11 @@ use clippy_utils::diagnostics::span_lint_and_sugg; +use clippy_utils::get_parent_expr; use clippy_utils::macros::{FormatArgsStorage, find_format_arg_expr, first_node_in_macro, matching_root_macro_call}; use clippy_utils::source::{SpanExt as _, snippet_with_context}; use clippy_utils::sugg::Sugg; use rustc_ast::{FormatArgsPiece, FormatOptions, FormatTrait}; use rustc_errors::Applicability; -use rustc_hir::{Expr, ExprKind}; +use rustc_hir::{BorrowKind, Expr, ExprKind, Mutability}; use rustc_lint::{LateContext, LateLintPass, impl_lint_pass}; use rustc_middle::ty; use rustc_span::{Span, sym}; @@ -68,15 +69,29 @@ impl<'tcx> LateLintPass<'tcx> for UselessFormat { let call_site = macro_call.span; match (format_args.arguments.all_args(), &format_args.template[..]) { - ([], []) => span_useless_format_empty(cx, call_site, "String::new()".to_owned(), applicability), + ([], []) => span_lint( + cx, + call_site, + "`String::new()`", + "String::new()".to_owned(), + applicability, + ), ([], [_]) => { // Simulate macro expansion, converting {{ and }} to { and }. let Some(snippet) = format_args.span.get_text(cx) else { return; }; let s_expand = snippet.replace("{{", "{").replace("}}", "}"); - let sugg = format!("{s_expand}.to_string()"); - span_useless_format(cx, call_site, sugg, applicability); + // If the format! is immediately borrowed as `&format!("literal")`, + // suggest using the string literal directly (no `.to_string()` needed). + if let Some(parent) = get_parent_expr(cx, expr) + && let ExprKind::AddrOf(BorrowKind::Ref, Mutability::Not, _) = parent.kind + { + span_lint(cx, parent.span, "the string literal directly", s_expand, applicability); + } else { + let sugg = format!("{s_expand}.to_string()"); + span_lint(cx, call_site, "`.to_string()`", sugg, applicability); + } }, ([arg], [piece]) => { if let Some(value) = find_format_arg_expr(expr, arg) @@ -94,15 +109,34 @@ impl<'tcx> LateLintPass<'tcx> for UselessFormat { ExprKind::MethodCall(path, ..) => path.ident.name == sym::to_string, _ => false, }; - let sugg = if is_new_string { - snippet_with_context(cx, value.span, call_site.ctxt(), "..", &mut applicability) - .0 - .into_owned() - } else { + // If the arg is `&str` (not a newly-constructed string) and the format! is + // immediately borrowed as `&format!("{}", lit)`, suggest the arg directly. + // This avoids producing `&lit.to_string()` which `unnecessary_to_owned` would flag. + if !is_new_string + && matches!(cx.typeck_results().expr_ty(value).peel_refs().kind(), ty::Str) + && let Some(parent) = get_parent_expr(cx, expr) + && let ExprKind::AddrOf(BorrowKind::Ref, Mutability::Not, _) = parent.kind + { let sugg = Sugg::hir_with_context(cx, value, call_site.ctxt(), "", &mut applicability); - format!("{}.to_string()", sugg.maybe_paren()) - }; - span_useless_format(cx, call_site, sugg, applicability); + span_lint( + cx, + parent.span, + "the argument directly", + sugg.to_string(), + applicability, + ); + } else { + let sugg = if is_new_string { + snippet_with_context(cx, value.span, call_site.ctxt(), "..", &mut applicability) + .0 + .into_owned() + } else { + let sugg = + Sugg::hir_with_context(cx, value, call_site.ctxt(), "", &mut applicability); + format!("{}.to_string()", sugg.maybe_paren()) + }; + span_lint(cx, call_site, "`.to_string()`", sugg, applicability); + } } }, _ => {}, @@ -111,25 +145,13 @@ impl<'tcx> LateLintPass<'tcx> for UselessFormat { } } -fn span_useless_format_empty(cx: &LateContext<'_>, span: Span, sugg: String, applicability: Applicability) { - span_lint_and_sugg( - cx, - USELESS_FORMAT, - span, - "useless use of `format!`", - "consider using `String::new()`", - sugg, - applicability, - ); -} - -fn span_useless_format(cx: &LateContext<'_>, span: Span, sugg: String, applicability: Applicability) { +fn span_lint(cx: &LateContext<'_>, span: Span, expl: &str, sugg: String, applicability: Applicability) { span_lint_and_sugg( cx, USELESS_FORMAT, span, "useless use of `format!`", - "consider using `.to_string()`", + format!("consider using {expl}"), sugg, applicability, ); diff --git a/tests/ui/format.fixed b/tests/ui/format.fixed index 7f01a88f932b..2995d3f9896d 100644 --- a/tests/ui/format.fixed +++ b/tests/ui/format.fixed @@ -95,6 +95,24 @@ fn main() { let xx = "xx"; let _ = xx.to_string(); //~^ useless_format + + // Issue #3361: `&format!("literal")` should suggest the literal directly + let _: &str = "hello"; //~ useless_format + let _: &str = "{}"; //~ useless_format + let _: &str = r"hello"; //~ useless_format + let _: &str = r#"world"#; //~ useless_format + let _: &str = r##"foo"##; //~ useless_format + let _: &str = r##"hello "\u{007B}""##; //~ useless_format + // Only immutable borrows: `&mut format!` is not handled + let _: &mut String = &mut "hello".to_string(); //~ useless_format + + fn literal(lit: &str) {} + literal("hello"); //~ useless_format + literal(&"hello"); //~ useless_format + + let lit = "hello"; + literal(lit); //~ useless_format + literal(lit); //~ useless_format } // `format!` as the tail expression of a block emitted by another macro diff --git a/tests/ui/format.rs b/tests/ui/format.rs index 92654e87a5ee..be89ff0ab72f 100644 --- a/tests/ui/format.rs +++ b/tests/ui/format.rs @@ -98,6 +98,24 @@ fn main() { let xx = "xx"; let _ = format!("{xx}"); //~^ useless_format + + // Issue #3361: `&format!("literal")` should suggest the literal directly + let _: &str = &format!("hello"); //~ useless_format + let _: &str = &format!("{{}}"); //~ useless_format + let _: &str = &format!(r"hello"); //~ useless_format + let _: &str = &format!(r#"world"#); //~ useless_format + let _: &str = &format!(r##"foo"##); //~ useless_format + let _: &str = &format!(r##"hello "\u{{007B}}""##); //~ useless_format + // Only immutable borrows: `&mut format!` is not handled + let _: &mut String = &mut format!("hello"); //~ useless_format + + fn literal(lit: &str) {} + literal(&format!("hello")); //~ useless_format + literal(&&format!("hello")); //~ useless_format + + let lit = "hello"; + literal(&format!("{}", lit)); //~ useless_format + literal(&format!("{lit}")); //~ useless_format } // `format!` as the tail expression of a block emitted by another macro diff --git a/tests/ui/format.stderr b/tests/ui/format.stderr index 964041d8e87d..d166017ce162 100644 --- a/tests/ui/format.stderr +++ b/tests/ui/format.stderr @@ -102,28 +102,94 @@ LL | let _ = format!("{xx}"); | ^^^^^^^^^^^^^^^ help: consider using `.to_string()`: `xx.to_string()` error: useless use of `format!` - --> tests/ui/format.rs:130:27 + --> tests/ui/format.rs:103:19 + | +LL | let _: &str = &format!("hello"); + | ^^^^^^^^^^^^^^^^^ help: consider using the string literal directly: `"hello"` + +error: useless use of `format!` + --> tests/ui/format.rs:104:19 + | +LL | let _: &str = &format!("{{}}"); + | ^^^^^^^^^^^^^^^^ help: consider using the string literal directly: `"{}"` + +error: useless use of `format!` + --> tests/ui/format.rs:105:19 + | +LL | let _: &str = &format!(r"hello"); + | ^^^^^^^^^^^^^^^^^^ help: consider using the string literal directly: `r"hello"` + +error: useless use of `format!` + --> tests/ui/format.rs:106:19 + | +LL | let _: &str = &format!(r#"world"#); + | ^^^^^^^^^^^^^^^^^^^^ help: consider using the string literal directly: `r#"world"#` + +error: useless use of `format!` + --> tests/ui/format.rs:107:19 + | +LL | let _: &str = &format!(r##"foo"##); + | ^^^^^^^^^^^^^^^^^^^^ help: consider using the string literal directly: `r##"foo"##` + +error: useless use of `format!` + --> tests/ui/format.rs:108:19 + | +LL | let _: &str = &format!(r##"hello "\u{{007B}}""##); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using the string literal directly: `r##"hello "\u{007B}""##` + +error: useless use of `format!` + --> tests/ui/format.rs:110:31 + | +LL | let _: &mut String = &mut format!("hello"); + | ^^^^^^^^^^^^^^^^ help: consider using `.to_string()`: `"hello".to_string()` + +error: useless use of `format!` + --> tests/ui/format.rs:113:13 + | +LL | literal(&format!("hello")); + | ^^^^^^^^^^^^^^^^^ help: consider using the string literal directly: `"hello"` + +error: useless use of `format!` + --> tests/ui/format.rs:114:14 + | +LL | literal(&&format!("hello")); + | ^^^^^^^^^^^^^^^^^ help: consider using the string literal directly: `"hello"` + +error: useless use of `format!` + --> tests/ui/format.rs:117:13 + | +LL | literal(&format!("{}", lit)); + | ^^^^^^^^^^^^^^^^^^^ help: consider using the argument directly: `lit` + +error: useless use of `format!` + --> tests/ui/format.rs:118:13 + | +LL | literal(&format!("{lit}")); + | ^^^^^^^^^^^^^^^^^ help: consider using the argument directly: `lit` + +error: useless use of `format!` + --> tests/ui/format.rs:148:27 | LL | let _ = plain_mr!(format!("{}", s())); | ^^^^^^^^^^^^^^^^^^ help: consider using `.to_string()`: `s().to_string()` error: useless use of `format!` - --> tests/ui/format.rs:132:27 + --> tests/ui/format.rs:150:27 | LL | let _ = block_mr!(format!("{}", s())); | ^^^^^^^^^^^^^^^^^^ help: consider using `.to_string()`: `s().to_string()` error: useless use of `format!` - --> tests/ui/format.rs:134:27 + --> tests/ui/format.rs:152:27 | LL | let _ = plain_dm!(format!("{}", s())); | ^^^^^^^^^^^^^^^^^^ help: consider using `.to_string()`: `s().to_string()` error: useless use of `format!` - --> tests/ui/format.rs:136:27 + --> tests/ui/format.rs:154:27 | LL | let _ = block_dm!(format!("{}", s())); | ^^^^^^^^^^^^^^^^^^ help: consider using `.to_string()`: `s().to_string()` -error: aborting due to 19 previous errors +error: aborting due to 30 previous errors