From 5610d22c8d58d12748308efb6e331e65618d11dc Mon Sep 17 00:00:00 2001 From: kraktus Date: Fri, 25 Nov 2022 16:36:22 +0100 Subject: [PATCH 1/2] Re-enable `uninlined_format_args` on multiline `format!` But do not display the code suggestion which can be sometimes completely broken (fortunately when applied it's valid) --- clippy_lints/src/format_args.rs | 19 ++++++++++------ tests/ui/uninlined_format_args.fixed | 11 +++------- tests/ui/uninlined_format_args.stderr | 31 ++++++++++++++++++++++++++- 3 files changed, 46 insertions(+), 15 deletions(-) diff --git a/clippy_lints/src/format_args.rs b/clippy_lints/src/format_args.rs index ec45be558f14..fd3ce2f3d6cd 100644 --- a/clippy_lints/src/format_args.rs +++ b/clippy_lints/src/format_args.rs @@ -9,7 +9,10 @@ use clippy_utils::source::snippet_opt; use clippy_utils::ty::{implements_trait, is_type_diagnostic_item}; use if_chain::if_chain; use itertools::Itertools; -use rustc_errors::Applicability; +use rustc_errors::{ + Applicability, + SuggestionStyle::{CompletelyHidden, ShowCode}, +}; use rustc_hir::{Expr, ExprKind, HirId, QPath}; use rustc_lint::{LateContext, LateLintPass, LintContext}; use rustc_middle::ty::adjustment::{Adjust, Adjustment}; @@ -286,10 +289,9 @@ fn check_uninlined_args(cx: &LateContext<'_>, args: &FormatArgsExpn<'_>, call_si return; } - // Temporarily ignore multiline spans: https://github.com/rust-lang/rust/pull/102729#discussion_r988704308 - if fixes.iter().any(|(span, _)| cx.sess().source_map().is_multiline(*span)) { - return; - } + // multiline span display suggestion is sometimes broken: https://github.com/rust-lang/rust/pull/102729#discussion_r988704308 + // in those cases, make the code suggestion hidden + let multiline_fix = fixes.iter().any(|(span, _)| cx.sess().source_map().is_multiline(*span)); span_lint_and_then( cx, @@ -297,7 +299,12 @@ fn check_uninlined_args(cx: &LateContext<'_>, args: &FormatArgsExpn<'_>, call_si call_site, "variables can be used directly in the `format!` string", |diag| { - diag.multipart_suggestion("change this to", fixes, Applicability::MachineApplicable); + diag.multipart_suggestion_with_style( + "change this to", + fixes, + Applicability::MachineApplicable, + if multiline_fix { CompletelyHidden } else { ShowCode }, + ); }, ); } diff --git a/tests/ui/uninlined_format_args.fixed b/tests/ui/uninlined_format_args.fixed index 106274479751..ca56c95c23f4 100644 --- a/tests/ui/uninlined_format_args.fixed +++ b/tests/ui/uninlined_format_args.fixed @@ -44,9 +44,7 @@ fn tester(fn_arg: i32) { println!("val='{local_i32}'"); // space+tab println!("val='{local_i32}'"); // tab+space println!( - "val='{ - }'", - local_i32 + "val='{local_i32}'" ); println!("{local_i32}"); println!("{fn_arg}"); @@ -110,8 +108,7 @@ fn tester(fn_arg: i32) { println!("{local_f64:width$.prec$}"); println!("{local_f64:width$.prec$} {local_f64} {width} {prec}"); println!( - "{0:1$.2$} {0:2$.1$} {1:0$.2$} {1:2$.0$} {2:0$.1$} {2:1$.0$}", - local_i32, width, prec, + "{local_i32:width$.prec$} {local_i32:prec$.width$} {width:local_i32$.prec$} {width:prec$.local_i32$} {prec:local_i32$.width$} {prec:width$.local_i32$}", ); println!( "{0:1$.2$} {0:2$.1$} {1:0$.2$} {1:2$.0$} {2:0$.1$} {2:1$.0$} {3}", @@ -142,9 +139,7 @@ fn tester(fn_arg: i32) { println!(no_param_str!(), local_i32); println!( - "{}", - // comment with a comma , in it - val, + "{val}", ); println!("{val}"); diff --git a/tests/ui/uninlined_format_args.stderr b/tests/ui/uninlined_format_args.stderr index 2ce3b7fa960c..1182d57ce9b7 100644 --- a/tests/ui/uninlined_format_args.stderr +++ b/tests/ui/uninlined_format_args.stderr @@ -59,6 +59,16 @@ LL - println!("val='{ }'", local_i32); // tab+space LL + println!("val='{local_i32}'"); // tab+space | +error: variables can be used directly in the `format!` string + --> $DIR/uninlined_format_args.rs:46:5 + | +LL | / println!( +LL | | "val='{ +LL | | }'", +LL | | local_i32 +LL | | ); + | |_____^ + error: variables can be used directly in the `format!` string --> $DIR/uninlined_format_args.rs:51:5 | @@ -767,6 +777,15 @@ LL - println!("{:1$.2$} {0} {1} {2}", local_f64, width, prec); LL + println!("{local_f64:width$.prec$} {local_f64} {width} {prec}"); | +error: variables can be used directly in the `format!` string + --> $DIR/uninlined_format_args.rs:112:5 + | +LL | / println!( +LL | | "{0:1$.2$} {0:2$.1$} {1:0$.2$} {1:2$.0$} {2:0$.1$} {2:1$.0$}", +LL | | local_i32, width, prec, +LL | | ); + | |_____^ + error: variables can be used directly in the `format!` string --> $DIR/uninlined_format_args.rs:123:5 | @@ -815,6 +834,16 @@ LL - println!("{}", format!("{}", local_i32)); LL + println!("{}", format!("{local_i32}")); | +error: variables can be used directly in the `format!` string + --> $DIR/uninlined_format_args.rs:144:5 + | +LL | / println!( +LL | | "{}", +LL | | // comment with a comma , in it +LL | | val, +LL | | ); + | |_____^ + error: variables can be used directly in the `format!` string --> $DIR/uninlined_format_args.rs:149:5 | @@ -875,5 +904,5 @@ LL - println!("expand='{}'", local_i32); LL + println!("expand='{local_i32}'"); | -error: aborting due to 73 previous errors +error: aborting due to 76 previous errors From 2fd10bc59b2c4c39691a1a9ec9de318a01cbf60c Mon Sep 17 00:00:00 2001 From: kraktus Date: Fri, 25 Nov 2022 16:41:08 +0100 Subject: [PATCH 2/2] dogfood with expanded `uninlined_format_args` --- clippy_utils/src/ty.rs | 15 +++++---------- lintcheck/src/main.rs | 15 +++++++-------- 2 files changed, 12 insertions(+), 18 deletions(-) diff --git a/clippy_utils/src/ty.rs b/clippy_utils/src/ty.rs index 897edfc5495f..09967f317f89 100644 --- a/clippy_utils/src/ty.rs +++ b/clippy_utils/src/ty.rs @@ -1005,14 +1005,12 @@ pub fn make_projection<'tcx>( debug_assert!( generic_count == substs.len(), - "wrong number of substs for `{:?}`: found `{}` expected `{}`.\n\ + "wrong number of substs for `{:?}`: found `{}` expected `{generic_count}`.\n\ note: the expected parameters are: {:#?}\n\ - the given arguments are: `{:#?}`", + the given arguments are: `{substs:#?}`", assoc_item.def_id, substs.len(), - generic_count, params.map(GenericParamDefKind::descr).collect::>(), - substs, ); if let Some((idx, (param, arg))) = params @@ -1030,14 +1028,11 @@ pub fn make_projection<'tcx>( { debug_assert!( false, - "mismatched subst type at index {}: expected a {}, found `{:?}`\n\ + "mismatched subst type at index {idx}: expected a {}, found `{arg:?}`\n\ note: the expected parameters are {:#?}\n\ - the given arguments are {:#?}", - idx, + the given arguments are {substs:#?}", param.descr(), - arg, - params.map(GenericParamDefKind::descr).collect::>(), - substs, + params.map(GenericParamDefKind::descr).collect::>() ); } } diff --git a/lintcheck/src/main.rs b/lintcheck/src/main.rs index ee8ab7c1d7cb..bd49f0960726 100644 --- a/lintcheck/src/main.rs +++ b/lintcheck/src/main.rs @@ -120,8 +120,8 @@ impl ClippyWarning { format!("$CARGO_HOME/{}", stripped.display()) } else { format!( - "target/lintcheck/sources/{}-{}/{}", - crate_name, crate_version, span.file_name + "target/lintcheck/sources/{crate_name}-{crate_version}/{}", + span.file_name ) }; @@ -322,13 +322,13 @@ impl Crate { if config.max_jobs == 1 { println!( - "{}/{} {}% Linting {} {}", - index, total_crates_to_lint, perc, &self.name, &self.version + "{index}/{total_crates_to_lint} {perc}% Linting {} {}", + &self.name, &self.version ); } else { println!( - "{}/{} {}% Linting {} {} in target dir {:?}", - index, total_crates_to_lint, perc, &self.name, &self.version, thread_index + "{index}/{total_crates_to_lint} {perc}% Linting {} {} in target dir {thread_index:?}", + &self.name, &self.version ); } @@ -398,8 +398,7 @@ impl Crate { .output() .unwrap_or_else(|error| { panic!( - "Encountered error:\n{:?}\ncargo_clippy_path: {}\ncrate path:{}\n", - error, + "Encountered error:\n{error:?}\ncargo_clippy_path: {}\ncrate path:{}\n", &cargo_clippy_path.display(), &self.path.display() );