Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
74 changes: 48 additions & 26 deletions clippy_lints/src/format.rs
Original file line number Diff line number Diff line change
@@ -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};
Expand Down Expand Up @@ -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)
Expand All @@ -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(), "<arg>", &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(), "<arg>", &mut applicability);
format!("{}.to_string()", sugg.maybe_paren())
};
span_lint(cx, call_site, "`.to_string()`", sugg, applicability);
}
}
},
_ => {},
Expand All @@ -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,
);
Expand Down
18 changes: 18 additions & 0 deletions tests/ui/format.fixed
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
18 changes: 18 additions & 0 deletions tests/ui/format.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Comment thread
nyurik marked this conversation as resolved.
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
Expand Down
76 changes: 71 additions & 5 deletions tests/ui/format.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -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