From 687fcf14c4bd8b60640253800755123b2ba4b02a Mon Sep 17 00:00:00 2001 From: "Samuel E. Moelius III" Date: Fri, 29 Jul 2022 05:34:49 -0400 Subject: [PATCH 1/2] Fix `to_string_in_format_args` false positive --- clippy_lints/src/format_args.rs | 67 ++++++++++++++++++++------------- tests/ui/format_args.fixed | 14 +++++++ tests/ui/format_args.rs | 14 +++++++ tests/ui/format_args.stderr | 8 +++- 4 files changed, 76 insertions(+), 27 deletions(-) diff --git a/clippy_lints/src/format_args.rs b/clippy_lints/src/format_args.rs index 5347ff880ce0..473c9a8675f1 100644 --- a/clippy_lints/src/format_args.rs +++ b/clippy_lints/src/format_args.rs @@ -131,34 +131,49 @@ fn check_to_string_in_format_args(cx: &LateContext<'_>, name: Symbol, value: &Ex if is_diag_trait_item(cx, method_def_id, sym::ToString); let receiver_ty = cx.typeck_results().expr_ty(receiver); if let Some(display_trait_id) = cx.tcx.get_diagnostic_item(sym::Display); + let (n_needed_derefs, target) = + count_needed_derefs(receiver_ty, cx.typeck_results().expr_adjustments(receiver).iter()); + if implements_trait(cx, target, display_trait_id, &[]); + if let Some(sized_trait_id) = cx.tcx.lang_items().sized_trait(); if let Some(receiver_snippet) = snippet_opt(cx, receiver.span); then { - let (n_needed_derefs, target) = count_needed_derefs( - receiver_ty, - cx.typeck_results().expr_adjustments(receiver).iter(), - ); - if implements_trait(cx, target, display_trait_id, &[]) { - if n_needed_derefs == 0 { - span_lint_and_sugg( - cx, - TO_STRING_IN_FORMAT_ARGS, - value.span.with_lo(receiver.span.hi()), - &format!("`to_string` applied to a type that implements `Display` in `{}!` args", name), - "remove this", - String::new(), - Applicability::MachineApplicable, - ); - } else { - span_lint_and_sugg( - cx, - TO_STRING_IN_FORMAT_ARGS, - value.span, - &format!("`to_string` applied to a type that implements `Display` in `{}!` args", name), - "use this", - format!("{:*>width$}{}", "", receiver_snippet, width = n_needed_derefs), - Applicability::MachineApplicable, - ); - } + let needed_ref = if implements_trait(cx, receiver_ty, sized_trait_id, &[]) { + "" + } else { + "&" + }; + if n_needed_derefs == 0 && needed_ref.is_empty() { + span_lint_and_sugg( + cx, + TO_STRING_IN_FORMAT_ARGS, + value.span.with_lo(receiver.span.hi()), + &format!( + "`to_string` applied to a type that implements `Display` in `{}!` args", + name + ), + "remove this", + String::new(), + Applicability::MachineApplicable, + ); + } else { + span_lint_and_sugg( + cx, + TO_STRING_IN_FORMAT_ARGS, + value.span, + &format!( + "`to_string` applied to a type that implements `Display` in `{}!` args", + name + ), + "use this", + format!( + "{}{:*>width$}{}", + needed_ref, + "", + receiver_snippet, + width = n_needed_derefs + ), + Applicability::MachineApplicable, + ); } } } diff --git a/tests/ui/format_args.fixed b/tests/ui/format_args.fixed index e07cae702770..e1c2d4d70be4 100644 --- a/tests/ui/format_args.fixed +++ b/tests/ui/format_args.fixed @@ -146,3 +146,17 @@ mod issue_8855 { dbg!(x); } } + +// https://github.com/rust-lang/rust-clippy/issues/9256 +mod issue_9256 { + #![allow(dead_code)] + + fn print_substring(original: &str) { + assert!(original.len() > 10); + println!("{}", &original[..10]); + } + + fn main() { + print_substring("Hello, world!"); + } +} diff --git a/tests/ui/format_args.rs b/tests/ui/format_args.rs index 86e66bce2c5f..b9a4d66c28ad 100644 --- a/tests/ui/format_args.rs +++ b/tests/ui/format_args.rs @@ -146,3 +146,17 @@ mod issue_8855 { dbg!(x); } } + +// https://github.com/rust-lang/rust-clippy/issues/9256 +mod issue_9256 { + #![allow(dead_code)] + + fn print_substring(original: &str) { + assert!(original.len() > 10); + println!("{}", original[..10].to_string()); + } + + fn main() { + print_substring("Hello, world!"); + } +} diff --git a/tests/ui/format_args.stderr b/tests/ui/format_args.stderr index e69999cc9d02..aa6e3659b43b 100644 --- a/tests/ui/format_args.stderr +++ b/tests/ui/format_args.stderr @@ -132,5 +132,11 @@ error: `to_string` applied to a type that implements `Display` in `format!` args LL | let x = format!("{} {}", a, b.to_string()); | ^^^^^^^^^^^^ help: remove this -error: aborting due to 22 previous errors +error: `to_string` applied to a type that implements `Display` in `println!` args + --> $DIR/format_args.rs:156:24 + | +LL | println!("{}", original[..10].to_string()); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use this: `&original[..10]` + +error: aborting due to 23 previous errors From 0bc26c811c96c05b2e60c94d06e2904b0ce25fe7 Mon Sep 17 00:00:00 2001 From: "Samuel E. Moelius III" Date: Fri, 29 Jul 2022 17:46:09 -0400 Subject: [PATCH 2/2] `needed_ref` -> `needs_ref` --- clippy_lints/src/format_args.rs | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/clippy_lints/src/format_args.rs b/clippy_lints/src/format_args.rs index 473c9a8675f1..9fb9fd99748b 100644 --- a/clippy_lints/src/format_args.rs +++ b/clippy_lints/src/format_args.rs @@ -137,12 +137,8 @@ fn check_to_string_in_format_args(cx: &LateContext<'_>, name: Symbol, value: &Ex if let Some(sized_trait_id) = cx.tcx.lang_items().sized_trait(); if let Some(receiver_snippet) = snippet_opt(cx, receiver.span); then { - let needed_ref = if implements_trait(cx, receiver_ty, sized_trait_id, &[]) { - "" - } else { - "&" - }; - if n_needed_derefs == 0 && needed_ref.is_empty() { + let needs_ref = !implements_trait(cx, receiver_ty, sized_trait_id, &[]); + if n_needed_derefs == 0 && !needs_ref { span_lint_and_sugg( cx, TO_STRING_IN_FORMAT_ARGS, @@ -167,7 +163,7 @@ fn check_to_string_in_format_args(cx: &LateContext<'_>, name: Symbol, value: &Ex "use this", format!( "{}{:*>width$}{}", - needed_ref, + if needs_ref { "&" } else { "" }, "", receiver_snippet, width = n_needed_derefs