diff --git a/clippy_lints/src/format_push_string.rs b/clippy_lints/src/format_push_string.rs index b64d608c0c70..35178b1efa2a 100644 --- a/clippy_lints/src/format_push_string.rs +++ b/clippy_lints/src/format_push_string.rs @@ -1,10 +1,16 @@ use clippy_utils::diagnostics::span_lint_and_then; -use clippy_utils::higher; +use clippy_utils::macros::{FormatArgsStorage, format_args_inputs_span, root_macro_call_first_node}; +use clippy_utils::source::{snippet_indent, snippet_with_applicability, snippet_with_context}; +use clippy_utils::std_or_core; use clippy_utils::ty::is_type_lang_item; +use rustc_data_structures::fx::FxHashSet; +use rustc_errors::Applicability; +use rustc_hir::def_id::{DefId, LocalModDefId}; use rustc_hir::{AssignOpKind, Expr, ExprKind, LangItem, MatchSource}; -use rustc_lint::{LateContext, LateLintPass}; -use rustc_session::declare_lint_pass; -use rustc_span::sym; +use rustc_lint::{LateContext, LateLintPass, LintContext}; +use rustc_middle::ty::TyCtxt; +use rustc_session::impl_lint_pass; +use rustc_span::{Span, sym}; declare_clippy_lint! { /// ### What it does @@ -38,59 +44,185 @@ declare_clippy_lint! { pedantic, "`format!(..)` appended to existing `String`" } -declare_lint_pass!(FormatPushString => [FORMAT_PUSH_STRING]); +impl_lint_pass!(FormatPushString => [FORMAT_PUSH_STRING]); -fn is_string(cx: &LateContext<'_>, e: &Expr<'_>) -> bool { - is_type_lang_item(cx, cx.typeck_results().expr_ty(e).peel_refs(), LangItem::String) +pub(crate) struct FormatPushString { + format_args: FormatArgsStorage, + write_trait: Option, + mods_with_import_added: FxHashSet, } -fn is_format(cx: &LateContext<'_>, e: &Expr<'_>) -> bool { - let e = e.peel_blocks().peel_borrows(); - if e.span.from_expansion() - && let Some(macro_def_id) = e.span.ctxt().outer_expn_data().macro_def_id - { - cx.tcx.get_diagnostic_name(macro_def_id) == Some(sym::format_macro) - } else if let Some(higher::If { then, r#else, .. }) = higher::If::hir(e) { - is_format(cx, then) || r#else.is_some_and(|e| is_format(cx, e)) - } else { - match higher::IfLetOrMatch::parse(cx, e) { - Some(higher::IfLetOrMatch::Match(_, arms, MatchSource::Normal)) => { - arms.iter().any(|arm| is_format(cx, arm.body)) - }, - Some(higher::IfLetOrMatch::IfLet(_, _, then, r#else, _)) => { - is_format(cx, then) || r#else.is_some_and(|e| is_format(cx, e)) - }, - _ => false, +enum FormatSearchResults { + /// The expression is itself a `format!()` invocation -- we can make a suggestion to replace it + Direct(Span), + /// The expression contains zero or more `format!()`s, e.g.: + /// ```ignore + /// if true { + /// format!("hello") + /// } else { + /// format!("world") + /// } + /// ``` + /// or + /// ```ignore + /// match true { + /// true => format!("hello"), + /// false => format!("world"), + /// } + Nested(Vec), +} + +impl FormatPushString { + pub(crate) fn new(tcx: TyCtxt<'_>, format_args: FormatArgsStorage) -> Self { + Self { + format_args, + write_trait: tcx.get_diagnostic_item(sym::FmtWrite), + mods_with_import_added: FxHashSet::default(), + } + } + + fn find_formats<'tcx>(&self, cx: &LateContext<'_>, e: &'tcx Expr<'tcx>) -> FormatSearchResults { + let expr_as_format = |e| { + if let Some(macro_call) = root_macro_call_first_node(cx, e) + && cx.tcx.is_diagnostic_item(sym::format_macro, macro_call.def_id) + && let Some(format_args) = self.format_args.get(cx, e, macro_call.expn) + { + Some(format_args_inputs_span(format_args)) + } else { + None + } + }; + + let e = e.peel_blocks().peel_borrows(); + if let Some(fmt) = expr_as_format(e) { + FormatSearchResults::Direct(fmt) + } else { + fn inner<'tcx>( + e: &'tcx Expr<'tcx>, + expr_as_format: &impl Fn(&'tcx Expr<'tcx>) -> Option, + out: &mut Vec, + ) { + let e = e.peel_blocks().peel_borrows(); + + match e.kind { + _ if expr_as_format(e).is_some() => out.push(e.span), + ExprKind::Match(_, arms, MatchSource::Normal) => { + for arm in arms { + inner(arm.body, expr_as_format, out); + } + }, + ExprKind::If(_, then, els) => { + inner(then, expr_as_format, out); + if let Some(els) = els { + inner(els, expr_as_format, out); + } + }, + _ => {}, + } + } + let mut spans = vec![]; + inner(e, &expr_as_format, &mut spans); + FormatSearchResults::Nested(spans) } } } impl<'tcx> LateLintPass<'tcx> for FormatPushString { fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { - let arg = match expr.kind { - ExprKind::MethodCall(_, _, [arg], _) => { + let (recv, arg) = match expr.kind { + ExprKind::MethodCall(_, recv, [arg], _) => { if let Some(fn_def_id) = cx.typeck_results().type_dependent_def_id(expr.hir_id) && cx.tcx.is_diagnostic_item(sym::string_push_str, fn_def_id) { - arg + (recv, arg) } else { return; } }, - ExprKind::AssignOp(op, left, arg) if op.node == AssignOpKind::AddAssign && is_string(cx, left) => arg, + ExprKind::AssignOp(op, recv, arg) if op.node == AssignOpKind::AddAssign && is_string(cx, recv) => { + (recv, arg) + }, _ => return, }; - if is_format(cx, arg) { - #[expect(clippy::collapsible_span_lint_calls, reason = "rust-clippy#7797")] - span_lint_and_then( - cx, - FORMAT_PUSH_STRING, - expr.span, - "`format!(..)` appended to existing `String`", - |diag| { - diag.help("consider using `write!` to avoid the extra allocation"); - }, - ); + match self.find_formats(cx, arg) { + FormatSearchResults::Direct(format_args) => { + span_lint_and_then( + cx, + FORMAT_PUSH_STRING, + expr.span, + "`format!(..)` appended to existing `String`", + |diag| { + let mut app = Applicability::MachineApplicable; + let msg = "consider using `write!` to avoid the extra allocation"; + + let main_sugg = ( + expr.span, + format!( + "let _ = write!({recv}, {format_args})", + recv = snippet_with_context(cx.sess(), recv.span, expr.span.ctxt(), "_", &mut app).0, + format_args = snippet_with_applicability(cx.sess(), format_args, "..", &mut app), + ), + ); + + let is_write_in_scope = if let Some(did) = self.write_trait + && let Some(trait_candidates) = cx.tcx.in_scope_traits(expr.hir_id) + && trait_candidates + .iter() + .any(|trait_candidate| trait_candidate.def_id == did) + { + true + } else { + false + }; + + if is_write_in_scope { + diag.span_suggestion_verbose(main_sugg.0, msg, main_sugg.1, app); + } else { + let body_id = cx.tcx.hir_enclosing_body_owner(expr.hir_id); + let scope = cx.tcx.parent_module_from_def_id(body_id); + let (module, _, _) = cx.tcx.hir_get_module(scope); + + if self.mods_with_import_added.insert(scope) { + // The trait is not in scope -- we'll need to import it + let import_span = module.spans.inject_use_span; + let import_sugg = ( + import_span, + format!( + "use {std}::fmt::Write as _;\n{indent_of_imports}", + std = std_or_core(cx).unwrap(), + indent_of_imports = + snippet_indent(cx.sess(), import_span).as_deref().unwrap_or(""), + ), + ); + let suggs = vec![main_sugg, import_sugg]; + + diag.multipart_suggestion_verbose(msg, suggs, app); + } else { + // Another suggestion will have taken care of importing the trait + diag.span_suggestion_verbose(main_sugg.0, msg, main_sugg.1, app); + } + } + }, + ); + }, + FormatSearchResults::Nested(spans) => { + if !spans.is_empty() { + span_lint_and_then( + cx, + FORMAT_PUSH_STRING, + expr.span, + "`format!(..)` appended to existing `String`", + |diag| { + diag.help("consider using `write!` to avoid the extra allocation"); + diag.span_labels(spans, "`format!` used here"); + }, + ); + } + }, } } } + +fn is_string(cx: &LateContext<'_>, e: &Expr<'_>) -> bool { + is_type_lang_item(cx, cx.typeck_results().expr_ty(e).peel_refs(), LangItem::String) +} diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 815411348aa6..4462936087e6 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -700,7 +700,8 @@ pub fn register_lint_passes(store: &mut rustc_lint::LintStore, conf: &'static Co store.register_late_pass(|_| Box::new(empty_with_brackets::EmptyWithBrackets::default())); store.register_late_pass(|_| Box::new(unnecessary_owned_empty_strings::UnnecessaryOwnedEmptyStrings)); store.register_early_pass(|| Box::new(pub_use::PubUse)); - store.register_late_pass(|_| Box::new(format_push_string::FormatPushString)); + let format_args = format_args_storage.clone(); + store.register_late_pass(move |tcx| Box::new(format_push_string::FormatPushString::new(tcx, format_args.clone()))); store.register_late_pass(move |_| Box::new(large_include_file::LargeIncludeFile::new(conf))); store.register_early_pass(move || Box::new(large_include_file::LargeIncludeFile::new(conf))); store.register_late_pass(|_| Box::new(strings::TrimSplitWhitespace)); diff --git a/tests/ui/format_push_string.fixed b/tests/ui/format_push_string.fixed new file mode 100644 index 000000000000..63370b06d11f --- /dev/null +++ b/tests/ui/format_push_string.fixed @@ -0,0 +1,243 @@ +#![warn(clippy::format_push_string)] + +use std::fmt::Write as _; +fn main() { + let mut string = String::new(); + let _ = write!(string, "{:?}", 1234); + //~^ format_push_string + + let _ = write!(string, "{:?}", 5678); + //~^ format_push_string + + macro_rules! string { + () => { + String::new() + }; + } + let _ = write!(string!(), "{:?}", 5678); + //~^ format_push_string +} + +mod import_write { + mod push_str { + mod not_imported { + use std::fmt::Write as _; + fn main(string: &mut String) { + let _ = write!(string, "{:?}", 1234); + //~^ format_push_string + } + } + + mod imported_anonymously { + use std::fmt::Write as _; + fn main(string: &mut String) { + use std::fmt::Write as _; + + let _ = write!(string, "{:?}", 1234); + //~^ format_push_string + } + } + + mod imported { + use std::fmt::Write as _; + fn main(string: &mut String) { + use std::fmt::Write; + + let _ = write!(string, "{:?}", 1234); + //~^ format_push_string + } + } + + mod imported_anonymously_in_module { + use std::fmt::Write as _; + use std::fmt::Write as _; + + fn main(string: &mut String) { + let _ = write!(string, "{:?}", 1234); + //~^ format_push_string + } + } + + mod imported_in_module { + use std::fmt::Write as _; + use std::fmt::Write; + + fn main(string: &mut String) { + let _ = write!(string, "{:?}", 1234); + //~^ format_push_string + } + } + + // make sure that we won't import for the second time + mod not_imported_and_not_imported { + use std::fmt::Write as _; + fn foo(string: &mut String) { + let _ = write!(string, "{:?}", 1234); + //~^ format_push_string + } + + fn bar(string: &mut String) { + let _ = write!(string, "{:?}", 1234); + //~^ format_push_string + } + } + + mod not_imported_and_imported { + use std::fmt::Write as _; + fn foo(string: &mut String) { + let _ = write!(string, "{:?}", 1234); + //~^ format_push_string + } + + fn bar(string: &mut String) { + use std::fmt::Write; + + let _ = write!(string, "{:?}", 1234); + //~^ format_push_string + } + } + + mod imported_and_not_imported { + use std::fmt::Write as _; + fn foo(string: &mut String) { + use std::fmt::Write; + + let _ = write!(string, "{:?}", 1234); + //~^ format_push_string + } + + fn bar(string: &mut String) { + let _ = write!(string, "{:?}", 1234); + //~^ format_push_string + } + } + + mod imported_and_imported { + use std::fmt::Write as _; + fn foo(string: &mut String) { + use std::fmt::Write; + + let _ = write!(string, "{:?}", 1234); + //~^ format_push_string + } + + fn bar(string: &mut String) { + use std::fmt::Write; + + let _ = write!(string, "{:?}", 1234); + //~^ format_push_string + } + } + } + + mod add_assign { + mod not_imported { + use std::fmt::Write as _; + fn main(string: &mut String) { + let _ = write!(string, "{:?}", 1234); + //~^ format_push_string + } + } + + mod imported_anonymously { + use std::fmt::Write as _; + fn main(string: &mut String) { + use std::fmt::Write as _; + + let _ = write!(string, "{:?}", 1234); + //~^ format_push_string + } + } + + mod imported { + use std::fmt::Write as _; + fn main(string: &mut String) { + use std::fmt::Write; + + let _ = write!(string, "{:?}", 1234); + //~^ format_push_string + } + } + + mod imported_anonymously_in_module { + use std::fmt::Write as _; + use std::fmt::Write as _; + + fn main(string: &mut String) { + let _ = write!(string, "{:?}", 1234); + //~^ format_push_string + } + } + + mod imported_in_module { + use std::fmt::Write as _; + use std::fmt::Write; + + fn main(string: &mut String) { + let _ = write!(string, "{:?}", 1234); + //~^ format_push_string + } + } + + // make sure that we won't import for the second time + mod not_imported_and_not_imported { + use std::fmt::Write as _; + fn foo(string: &mut String) { + let _ = write!(string, "{:?}", 1234); + //~^ format_push_string + } + + fn bar(string: &mut String) { + let _ = write!(string, "{:?}", 1234); + //~^ format_push_string + } + } + + mod not_imported_and_imported { + use std::fmt::Write as _; + fn foo(string: &mut String) { + let _ = write!(string, "{:?}", 1234); + //~^ format_push_string + } + + fn bar(string: &mut String) { + use std::fmt::Write; + + let _ = write!(string, "{:?}", 1234); + //~^ format_push_string + } + } + + mod imported_and_not_imported { + use std::fmt::Write as _; + fn foo(string: &mut String) { + use std::fmt::Write; + + let _ = write!(string, "{:?}", 1234); + //~^ format_push_string + } + + fn bar(string: &mut String) { + let _ = write!(string, "{:?}", 1234); + //~^ format_push_string + } + } + + mod imported_and_imported { + use std::fmt::Write as _; + fn foo(string: &mut String) { + use std::fmt::Write; + + let _ = write!(string, "{:?}", 1234); + //~^ format_push_string + } + + fn bar(string: &mut String) { + use std::fmt::Write; + + let _ = write!(string, "{:?}", 1234); + //~^ format_push_string + } + } + } +} diff --git a/tests/ui/format_push_string.rs b/tests/ui/format_push_string.rs index 056ef59ff0e2..d49780f520c1 100644 --- a/tests/ui/format_push_string.rs +++ b/tests/ui/format_push_string.rs @@ -7,38 +7,218 @@ fn main() { string.push_str(&format!("{:?}", 5678)); //~^ format_push_string + + macro_rules! string { + () => { + String::new() + }; + } + string!().push_str(&format!("{:?}", 5678)); + //~^ format_push_string } -mod issue9493 { - pub fn u8vec_to_hex(vector: &Vec, upper: bool) -> String { - let mut hex = String::with_capacity(vector.len() * 2); - for byte in vector { - hex += &(if upper { +mod import_write { + mod push_str { + mod not_imported { + fn main(string: &mut String) { + string.push_str(&format!("{:?}", 1234)); + //~^ format_push_string + } + } + + mod imported_anonymously { + fn main(string: &mut String) { + use std::fmt::Write as _; + + string.push_str(&format!("{:?}", 1234)); + //~^ format_push_string + } + } + + mod imported { + fn main(string: &mut String) { + use std::fmt::Write; + + string.push_str(&format!("{:?}", 1234)); + //~^ format_push_string + } + } + + mod imported_anonymously_in_module { + use std::fmt::Write as _; + + fn main(string: &mut String) { + string.push_str(&format!("{:?}", 1234)); + //~^ format_push_string + } + } + + mod imported_in_module { + use std::fmt::Write; + + fn main(string: &mut String) { + string.push_str(&format!("{:?}", 1234)); + //~^ format_push_string + } + } + + // make sure that we won't import for the second time + mod not_imported_and_not_imported { + fn foo(string: &mut String) { + string.push_str(&format!("{:?}", 1234)); + //~^ format_push_string + } + + fn bar(string: &mut String) { + string.push_str(&format!("{:?}", 1234)); + //~^ format_push_string + } + } + + mod not_imported_and_imported { + fn foo(string: &mut String) { + string.push_str(&format!("{:?}", 1234)); //~^ format_push_string + } + + fn bar(string: &mut String) { + use std::fmt::Write; + + string.push_str(&format!("{:?}", 1234)); + //~^ format_push_string + } + } + + mod imported_and_not_imported { + fn foo(string: &mut String) { + use std::fmt::Write; - format!("{byte:02X}") - } else { - format!("{byte:02x}") - }); + string.push_str(&format!("{:?}", 1234)); + //~^ format_push_string + } + + fn bar(string: &mut String) { + string.push_str(&format!("{:?}", 1234)); + //~^ format_push_string + } + } + + mod imported_and_imported { + fn foo(string: &mut String) { + use std::fmt::Write; + + string.push_str(&format!("{:?}", 1234)); + //~^ format_push_string + } + + fn bar(string: &mut String) { + use std::fmt::Write; + + string.push_str(&format!("{:?}", 1234)); + //~^ format_push_string + } } - hex } - pub fn other_cases() { - let mut s = String::new(); - // if let - s += &(if let Some(_a) = Some(1234) { - //~^ format_push_string - - format!("{}", 1234) - } else { - format!("{}", 1234) - }); - // match - s += &(match Some(1234) { - //~^ format_push_string - Some(_) => format!("{}", 1234), - None => format!("{}", 1234), - }); + mod add_assign { + mod not_imported { + fn main(string: &mut String) { + string.push_str(&format!("{:?}", 1234)); + //~^ format_push_string + } + } + + mod imported_anonymously { + fn main(string: &mut String) { + use std::fmt::Write as _; + + string.push_str(&format!("{:?}", 1234)); + //~^ format_push_string + } + } + + mod imported { + fn main(string: &mut String) { + use std::fmt::Write; + + string.push_str(&format!("{:?}", 1234)); + //~^ format_push_string + } + } + + mod imported_anonymously_in_module { + use std::fmt::Write as _; + + fn main(string: &mut String) { + string.push_str(&format!("{:?}", 1234)); + //~^ format_push_string + } + } + + mod imported_in_module { + use std::fmt::Write; + + fn main(string: &mut String) { + string.push_str(&format!("{:?}", 1234)); + //~^ format_push_string + } + } + + // make sure that we won't import for the second time + mod not_imported_and_not_imported { + fn foo(string: &mut String) { + string.push_str(&format!("{:?}", 1234)); + //~^ format_push_string + } + + fn bar(string: &mut String) { + string.push_str(&format!("{:?}", 1234)); + //~^ format_push_string + } + } + + mod not_imported_and_imported { + fn foo(string: &mut String) { + string.push_str(&format!("{:?}", 1234)); + //~^ format_push_string + } + + fn bar(string: &mut String) { + use std::fmt::Write; + + string.push_str(&format!("{:?}", 1234)); + //~^ format_push_string + } + } + + mod imported_and_not_imported { + fn foo(string: &mut String) { + use std::fmt::Write; + + string.push_str(&format!("{:?}", 1234)); + //~^ format_push_string + } + + fn bar(string: &mut String) { + string.push_str(&format!("{:?}", 1234)); + //~^ format_push_string + } + } + + mod imported_and_imported { + fn foo(string: &mut String) { + use std::fmt::Write; + + string.push_str(&format!("{:?}", 1234)); + //~^ format_push_string + } + + fn bar(string: &mut String) { + use std::fmt::Write; + + string.push_str(&format!("{:?}", 1234)); + //~^ format_push_string + } + } } } diff --git a/tests/ui/format_push_string.stderr b/tests/ui/format_push_string.stderr index bba2a8947c43..f4d5c7fa019e 100644 --- a/tests/ui/format_push_string.stderr +++ b/tests/ui/format_push_string.stderr @@ -4,9 +4,15 @@ error: `format!(..)` appended to existing `String` LL | string += &format!("{:?}", 1234); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | - = help: consider using `write!` to avoid the extra allocation = note: `-D clippy::format-push-string` implied by `-D warnings` = help: to override `-D warnings` add `#[allow(clippy::format_push_string)]` +help: consider using `write!` to avoid the extra allocation + | +LL + use std::fmt::Write as _; +LL ~ fn main() { +LL | let mut string = String::new(); +LL ~ let _ = write!(string, "{:?}", 1234); + | error: `format!(..)` appended to existing `String` --> tests/ui/format_push_string.rs:8:5 @@ -14,47 +20,377 @@ error: `format!(..)` appended to existing `String` LL | string.push_str(&format!("{:?}", 5678)); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | - = help: consider using `write!` to avoid the extra allocation +help: consider using `write!` to avoid the extra allocation + | +LL - string.push_str(&format!("{:?}", 5678)); +LL + let _ = write!(string, "{:?}", 5678); + | + +error: `format!(..)` appended to existing `String` + --> tests/ui/format_push_string.rs:16:5 + | +LL | string!().push_str(&format!("{:?}", 5678)); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: consider using `write!` to avoid the extra allocation + | +LL - string!().push_str(&format!("{:?}", 5678)); +LL + let _ = write!(string!(), "{:?}", 5678); + | + +error: `format!(..)` appended to existing `String` + --> tests/ui/format_push_string.rs:24:17 + | +LL | string.push_str(&format!("{:?}", 1234)); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: consider using `write!` to avoid the extra allocation + | +LL ~ use std::fmt::Write as _; +LL ~ fn main(string: &mut String) { +LL ~ let _ = write!(string, "{:?}", 1234); + | + +error: `format!(..)` appended to existing `String` + --> tests/ui/format_push_string.rs:33:17 + | +LL | string.push_str(&format!("{:?}", 1234)); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: consider using `write!` to avoid the extra allocation + | +LL ~ use std::fmt::Write as _; +LL ~ fn main(string: &mut String) { +LL | use std::fmt::Write as _; +LL | +LL ~ let _ = write!(string, "{:?}", 1234); + | + +error: `format!(..)` appended to existing `String` + --> tests/ui/format_push_string.rs:42:17 + | +LL | string.push_str(&format!("{:?}", 1234)); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: consider using `write!` to avoid the extra allocation + | +LL ~ use std::fmt::Write as _; +LL ~ fn main(string: &mut String) { +LL | use std::fmt::Write; +LL | +LL ~ let _ = write!(string, "{:?}", 1234); + | + +error: `format!(..)` appended to existing `String` + --> tests/ui/format_push_string.rs:51:17 + | +LL | string.push_str(&format!("{:?}", 1234)); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: consider using `write!` to avoid the extra allocation + | +LL ~ use std::fmt::Write as _; +LL ~ use std::fmt::Write as _; +LL | +LL | fn main(string: &mut String) { +LL ~ let _ = write!(string, "{:?}", 1234); + | + +error: `format!(..)` appended to existing `String` + --> tests/ui/format_push_string.rs:60:17 + | +LL | string.push_str(&format!("{:?}", 1234)); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: consider using `write!` to avoid the extra allocation + | +LL ~ use std::fmt::Write as _; +LL ~ use std::fmt::Write; +LL | +LL | fn main(string: &mut String) { +LL ~ let _ = write!(string, "{:?}", 1234); + | + +error: `format!(..)` appended to existing `String` + --> tests/ui/format_push_string.rs:68:17 + | +LL | string.push_str(&format!("{:?}", 1234)); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: consider using `write!` to avoid the extra allocation + | +LL ~ use std::fmt::Write as _; +LL ~ fn foo(string: &mut String) { +LL ~ let _ = write!(string, "{:?}", 1234); + | + +error: `format!(..)` appended to existing `String` + --> tests/ui/format_push_string.rs:73:17 + | +LL | string.push_str(&format!("{:?}", 1234)); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: consider using `write!` to avoid the extra allocation + | +LL - string.push_str(&format!("{:?}", 1234)); +LL + let _ = write!(string, "{:?}", 1234); + | + +error: `format!(..)` appended to existing `String` + --> tests/ui/format_push_string.rs:80:17 + | +LL | string.push_str(&format!("{:?}", 1234)); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: consider using `write!` to avoid the extra allocation + | +LL ~ use std::fmt::Write as _; +LL ~ fn foo(string: &mut String) { +LL ~ let _ = write!(string, "{:?}", 1234); + | + +error: `format!(..)` appended to existing `String` + --> tests/ui/format_push_string.rs:87:17 + | +LL | string.push_str(&format!("{:?}", 1234)); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: consider using `write!` to avoid the extra allocation + | +LL - string.push_str(&format!("{:?}", 1234)); +LL + let _ = write!(string, "{:?}", 1234); + | + +error: `format!(..)` appended to existing `String` + --> tests/ui/format_push_string.rs:96:17 + | +LL | string.push_str(&format!("{:?}", 1234)); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: consider using `write!` to avoid the extra allocation + | +LL ~ use std::fmt::Write as _; +LL ~ fn foo(string: &mut String) { +LL | use std::fmt::Write; +LL | +LL ~ let _ = write!(string, "{:?}", 1234); + | + +error: `format!(..)` appended to existing `String` + --> tests/ui/format_push_string.rs:101:17 + | +LL | string.push_str(&format!("{:?}", 1234)); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: consider using `write!` to avoid the extra allocation + | +LL - string.push_str(&format!("{:?}", 1234)); +LL + let _ = write!(string, "{:?}", 1234); + | + +error: `format!(..)` appended to existing `String` + --> tests/ui/format_push_string.rs:110:17 + | +LL | string.push_str(&format!("{:?}", 1234)); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: consider using `write!` to avoid the extra allocation + | +LL ~ use std::fmt::Write as _; +LL ~ fn foo(string: &mut String) { +LL | use std::fmt::Write; +LL | +LL ~ let _ = write!(string, "{:?}", 1234); + | + +error: `format!(..)` appended to existing `String` + --> tests/ui/format_push_string.rs:117:17 + | +LL | string.push_str(&format!("{:?}", 1234)); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: consider using `write!` to avoid the extra allocation + | +LL - string.push_str(&format!("{:?}", 1234)); +LL + let _ = write!(string, "{:?}", 1234); + | + +error: `format!(..)` appended to existing `String` + --> tests/ui/format_push_string.rs:126:17 + | +LL | string.push_str(&format!("{:?}", 1234)); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: consider using `write!` to avoid the extra allocation + | +LL ~ use std::fmt::Write as _; +LL ~ fn main(string: &mut String) { +LL ~ let _ = write!(string, "{:?}", 1234); + | error: `format!(..)` appended to existing `String` - --> tests/ui/format_push_string.rs:16:13 + --> tests/ui/format_push_string.rs:135:17 + | +LL | string.push_str(&format!("{:?}", 1234)); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | -LL | / hex += &(if upper { -LL | | -LL | | -LL | | format!("{byte:02X}") -LL | | } else { -LL | | format!("{byte:02x}") -LL | | }); - | |______________^ +help: consider using `write!` to avoid the extra allocation + | +LL ~ use std::fmt::Write as _; +LL ~ fn main(string: &mut String) { +LL | use std::fmt::Write as _; +LL | +LL ~ let _ = write!(string, "{:?}", 1234); | - = help: consider using `write!` to avoid the extra allocation error: `format!(..)` appended to existing `String` - --> tests/ui/format_push_string.rs:30:9 + --> tests/ui/format_push_string.rs:144:17 + | +LL | string.push_str(&format!("{:?}", 1234)); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: consider using `write!` to avoid the extra allocation + | +LL ~ use std::fmt::Write as _; +LL ~ fn main(string: &mut String) { +LL | use std::fmt::Write; +LL | +LL ~ let _ = write!(string, "{:?}", 1234); + | + +error: `format!(..)` appended to existing `String` + --> tests/ui/format_push_string.rs:153:17 + | +LL | string.push_str(&format!("{:?}", 1234)); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: consider using `write!` to avoid the extra allocation + | +LL ~ use std::fmt::Write as _; +LL ~ use std::fmt::Write as _; +LL | +LL | fn main(string: &mut String) { +LL ~ let _ = write!(string, "{:?}", 1234); + | + +error: `format!(..)` appended to existing `String` + --> tests/ui/format_push_string.rs:162:17 + | +LL | string.push_str(&format!("{:?}", 1234)); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: consider using `write!` to avoid the extra allocation + | +LL ~ use std::fmt::Write as _; +LL ~ use std::fmt::Write; +LL | +LL | fn main(string: &mut String) { +LL ~ let _ = write!(string, "{:?}", 1234); + | + +error: `format!(..)` appended to existing `String` + --> tests/ui/format_push_string.rs:170:17 + | +LL | string.push_str(&format!("{:?}", 1234)); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: consider using `write!` to avoid the extra allocation | -LL | / s += &(if let Some(_a) = Some(1234) { -LL | | -LL | | -LL | | format!("{}", 1234) -LL | | } else { -LL | | format!("{}", 1234) -LL | | }); - | |__________^ +LL ~ use std::fmt::Write as _; +LL ~ fn foo(string: &mut String) { +LL ~ let _ = write!(string, "{:?}", 1234); | - = help: consider using `write!` to avoid the extra allocation error: `format!(..)` appended to existing `String` - --> tests/ui/format_push_string.rs:38:9 + --> tests/ui/format_push_string.rs:175:17 + | +LL | string.push_str(&format!("{:?}", 1234)); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: consider using `write!` to avoid the extra allocation + | +LL - string.push_str(&format!("{:?}", 1234)); +LL + let _ = write!(string, "{:?}", 1234); + | + +error: `format!(..)` appended to existing `String` + --> tests/ui/format_push_string.rs:182:17 + | +LL | string.push_str(&format!("{:?}", 1234)); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: consider using `write!` to avoid the extra allocation + | +LL ~ use std::fmt::Write as _; +LL ~ fn foo(string: &mut String) { +LL ~ let _ = write!(string, "{:?}", 1234); + | + +error: `format!(..)` appended to existing `String` + --> tests/ui/format_push_string.rs:189:17 + | +LL | string.push_str(&format!("{:?}", 1234)); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: consider using `write!` to avoid the extra allocation + | +LL - string.push_str(&format!("{:?}", 1234)); +LL + let _ = write!(string, "{:?}", 1234); + | + +error: `format!(..)` appended to existing `String` + --> tests/ui/format_push_string.rs:198:17 + | +LL | string.push_str(&format!("{:?}", 1234)); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: consider using `write!` to avoid the extra allocation + | +LL ~ use std::fmt::Write as _; +LL ~ fn foo(string: &mut String) { +LL | use std::fmt::Write; +LL | +LL ~ let _ = write!(string, "{:?}", 1234); + | + +error: `format!(..)` appended to existing `String` + --> tests/ui/format_push_string.rs:203:17 + | +LL | string.push_str(&format!("{:?}", 1234)); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: consider using `write!` to avoid the extra allocation + | +LL - string.push_str(&format!("{:?}", 1234)); +LL + let _ = write!(string, "{:?}", 1234); + | + +error: `format!(..)` appended to existing `String` + --> tests/ui/format_push_string.rs:212:17 + | +LL | string.push_str(&format!("{:?}", 1234)); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: consider using `write!` to avoid the extra allocation + | +LL ~ use std::fmt::Write as _; +LL ~ fn foo(string: &mut String) { +LL | use std::fmt::Write; +LL | +LL ~ let _ = write!(string, "{:?}", 1234); + | + +error: `format!(..)` appended to existing `String` + --> tests/ui/format_push_string.rs:219:17 + | +LL | string.push_str(&format!("{:?}", 1234)); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: consider using `write!` to avoid the extra allocation | -LL | / s += &(match Some(1234) { -LL | | -LL | | Some(_) => format!("{}", 1234), -LL | | None => format!("{}", 1234), -LL | | }); - | |__________^ +LL - string.push_str(&format!("{:?}", 1234)); +LL + let _ = write!(string, "{:?}", 1234); | - = help: consider using `write!` to avoid the extra allocation -error: aborting due to 5 previous errors +error: aborting due to 29 previous errors diff --git a/tests/ui/format_push_string_no_std.fixed b/tests/ui/format_push_string_no_std.fixed new file mode 100644 index 000000000000..d3037196b6a1 --- /dev/null +++ b/tests/ui/format_push_string_no_std.fixed @@ -0,0 +1,13 @@ +#![warn(clippy::format_push_string)] +#![no_std] + +use core::fmt::Write as _; +extern crate alloc; + +use alloc::format; +use alloc::string::String; + +fn foo(string: &mut String) { + let _ = write!(string, "{:?}", 1234); + //~^ format_push_string +} diff --git a/tests/ui/format_push_string_no_std.rs b/tests/ui/format_push_string_no_std.rs new file mode 100644 index 000000000000..e5d71998f901 --- /dev/null +++ b/tests/ui/format_push_string_no_std.rs @@ -0,0 +1,12 @@ +#![warn(clippy::format_push_string)] +#![no_std] + +extern crate alloc; + +use alloc::format; +use alloc::string::String; + +fn foo(string: &mut String) { + string.push_str(&format!("{:?}", 1234)); + //~^ format_push_string +} diff --git a/tests/ui/format_push_string_no_std.stderr b/tests/ui/format_push_string_no_std.stderr new file mode 100644 index 000000000000..ace73435b7cb --- /dev/null +++ b/tests/ui/format_push_string_no_std.stderr @@ -0,0 +1,20 @@ +error: `format!(..)` appended to existing `String` + --> tests/ui/format_push_string_no_std.rs:10:5 + | +LL | string.push_str(&format!("{:?}", 1234)); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: `-D clippy::format-push-string` implied by `-D warnings` + = help: to override `-D warnings` add `#[allow(clippy::format_push_string)]` +help: consider using `write!` to avoid the extra allocation + | +LL + use core::fmt::Write as _; +LL ~ extern crate alloc; +LL | +... +LL | fn foo(string: &mut String) { +LL ~ let _ = write!(string, "{:?}", 1234); + | + +error: aborting due to 1 previous error + diff --git a/tests/ui/format_push_string_unfixable.rs b/tests/ui/format_push_string_unfixable.rs new file mode 100644 index 000000000000..82aa6c34455b --- /dev/null +++ b/tests/ui/format_push_string_unfixable.rs @@ -0,0 +1,36 @@ +//@no-rustfix +#![warn(clippy::format_push_string)] + +mod issue9493 { + pub fn u8vec_to_hex(vector: &Vec, upper: bool) -> String { + let mut hex = String::with_capacity(vector.len() * 2); + for byte in vector { + hex += &(if upper { + format!("{byte:02X}") + //~^ format_push_string + } else { + format!("{byte:02x}") + }); + } + hex + } + + pub fn other_cases() { + let mut s = String::new(); + // if let + s += &(if let Some(_a) = Some(1234) { + format!("{}", 1234) + //~^ format_push_string + } else { + format!("{}", 1234) + }); + // match + s += &(match Some(1234) { + Some(_) => format!("{}", 1234), + //~^ format_push_string + None => format!("{}", 1234), + }); + } +} + +fn main() {} diff --git a/tests/ui/format_push_string_unfixable.stderr b/tests/ui/format_push_string_unfixable.stderr new file mode 100644 index 000000000000..5b946551443f --- /dev/null +++ b/tests/ui/format_push_string_unfixable.stderr @@ -0,0 +1,48 @@ +error: `format!(..)` appended to existing `String` + --> tests/ui/format_push_string_unfixable.rs:8:13 + | +LL | / hex += &(if upper { +LL | | format!("{byte:02X}") + | | --------------------- `format!` used here +LL | | +LL | | } else { +LL | | format!("{byte:02x}") + | | --------------------- `format!` used here +LL | | }); + | |______________^ + | + = help: consider using `write!` to avoid the extra allocation + = note: `-D clippy::format-push-string` implied by `-D warnings` + = help: to override `-D warnings` add `#[allow(clippy::format_push_string)]` + +error: `format!(..)` appended to existing `String` + --> tests/ui/format_push_string_unfixable.rs:21:9 + | +LL | / s += &(if let Some(_a) = Some(1234) { +LL | | format!("{}", 1234) + | | ------------------- `format!` used here +LL | | +LL | | } else { +LL | | format!("{}", 1234) + | | ------------------- `format!` used here +LL | | }); + | |__________^ + | + = help: consider using `write!` to avoid the extra allocation + +error: `format!(..)` appended to existing `String` + --> tests/ui/format_push_string_unfixable.rs:28:9 + | +LL | / s += &(match Some(1234) { +LL | | Some(_) => format!("{}", 1234), + | | ------------------- `format!` used here +LL | | +LL | | None => format!("{}", 1234), + | | ------------------- `format!` used here +LL | | }); + | |__________^ + | + = help: consider using `write!` to avoid the extra allocation + +error: aborting due to 3 previous errors +