Skip to content

Commit 6d0b4e3

Browse files
borsbors
committed
Auto merge of #9945 - kraktus:uninlined_multiple_lines, r=llogiq
Re-enable `uninlined_format_args` on multiline `format!` fix #9719 There was an issue with the code suggestion which can be sometimes completely broken (fortunately when applied it's valid), so we do not show it. changelog: [`uninlined_format_args`] re-enable for multiline format expression, but do not show the literal code suggestion in those cases
2 parents 08a80d3 + 2fd10bc commit 6d0b4e3

File tree

5 files changed

+58
-33
lines changed

5 files changed

+58
-33
lines changed

‎clippy_lints/src/format_args.rs

+13-6
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,10 @@ use clippy_utils::source::snippet_opt;
99
use clippy_utils::ty::{implements_trait, is_type_diagnostic_item};
1010
use if_chain::if_chain;
1111
use itertools::Itertools;
12-
use rustc_errors::Applicability;
12+
use rustc_errors::{
13+
Applicability,
14+
SuggestionStyle::{CompletelyHidden, ShowCode},
15+
};
1316
use rustc_hir::{Expr, ExprKind, HirId, QPath};
1417
use rustc_lint::{LateContext, LateLintPass, LintContext};
1518
use rustc_middle::ty::adjustment::{Adjust, Adjustment};
@@ -286,18 +289,22 @@ fn check_uninlined_args(cx: &LateContext<'_>, args: &FormatArgsExpn<'_>, call_si
286289
return;
287290
}
288291

289-
// Temporarily ignore multiline spans: https://github.com/rust-lang/rust/pull/102729#discussion_r988704308
290-
if fixes.iter().any(|(span, _)| cx.sess().source_map().is_multiline(*span)) {
291-
return;
292-
}
292+
// multiline span display suggestion is sometimes broken: https://github.com/rust-lang/rust/pull/102729#discussion_r988704308
293+
// in those cases, make the code suggestion hidden
294+
let multiline_fix = fixes.iter().any(|(span, _)| cx.sess().source_map().is_multiline(*span));
293295

294296
span_lint_and_then(
295297
cx,
296298
UNINLINED_FORMAT_ARGS,
297299
call_site,
298300
"variables can be used directly in the `format!` string",
299301
|diag| {
300-
diag.multipart_suggestion("change this to", fixes, Applicability::MachineApplicable);
302+
diag.multipart_suggestion_with_style(
303+
"change this to",
304+
fixes,
305+
Applicability::MachineApplicable,
306+
if multiline_fix { CompletelyHidden } else { ShowCode },
307+
);
301308
},
302309
);
303310
}

‎clippy_utils/src/ty.rs

+5-10
Original file line numberDiff line numberDiff line change
@@ -1005,14 +1005,12 @@ pub fn make_projection<'tcx>(
10051005

10061006
debug_assert!(
10071007
generic_count == substs.len(),
1008-
"wrong number of substs for `{:?}`: found `{}` expected `{}`.\n\
1008+
"wrong number of substs for `{:?}`: found `{}` expected `{generic_count}`.\n\
10091009
note: the expected parameters are: {:#?}\n\
1010-
the given arguments are: `{:#?}`",
1010+
the given arguments are: `{substs:#?}`",
10111011
assoc_item.def_id,
10121012
substs.len(),
1013-
generic_count,
10141013
params.map(GenericParamDefKind::descr).collect::<Vec<_>>(),
1015-
substs,
10161014
);
10171015

10181016
if let Some((idx, (param, arg))) = params
@@ -1030,14 +1028,11 @@ pub fn make_projection<'tcx>(
10301028
{
10311029
debug_assert!(
10321030
false,
1033-
"mismatched subst type at index {}: expected a {}, found `{:?}`\n\
1031+
"mismatched subst type at index {idx}: expected a {}, found `{arg:?}`\n\
10341032
note: the expected parameters are {:#?}\n\
1035-
the given arguments are {:#?}",
1036-
idx,
1033+
the given arguments are {substs:#?}",
10371034
param.descr(),
1038-
arg,
1039-
params.map(GenericParamDefKind::descr).collect::<Vec<_>>(),
1040-
substs,
1035+
params.map(GenericParamDefKind::descr).collect::<Vec<_>>()
10411036
);
10421037
}
10431038
}

‎lintcheck/src/main.rs

+7-8
Original file line numberDiff line numberDiff line change
@@ -120,8 +120,8 @@ impl ClippyWarning {
120120
format!("$CARGO_HOME/{}", stripped.display())
121121
} else {
122122
format!(
123-
"target/lintcheck/sources/{}-{}/{}",
124-
crate_name, crate_version, span.file_name
123+
"target/lintcheck/sources/{crate_name}-{crate_version}/{}",
124+
span.file_name
125125
)
126126
};
127127

@@ -322,13 +322,13 @@ impl Crate {
322322

323323
if config.max_jobs == 1 {
324324
println!(
325-
"{}/{} {}% Linting {} {}",
326-
index, total_crates_to_lint, perc, &self.name, &self.version
325+
"{index}/{total_crates_to_lint} {perc}% Linting {} {}",
326+
&self.name, &self.version
327327
);
328328
} else {
329329
println!(
330-
"{}/{} {}% Linting {} {} in target dir {:?}",
331-
index, total_crates_to_lint, perc, &self.name, &self.version, thread_index
330+
"{index}/{total_crates_to_lint} {perc}% Linting {} {} in target dir {thread_index:?}",
331+
&self.name, &self.version
332332
);
333333
}
334334

@@ -398,8 +398,7 @@ impl Crate {
398398
.output()
399399
.unwrap_or_else(|error| {
400400
panic!(
401-
"Encountered error:\n{:?}\ncargo_clippy_path: {}\ncrate path:{}\n",
402-
error,
401+
"Encountered error:\n{error:?}\ncargo_clippy_path: {}\ncrate path:{}\n",
403402
&cargo_clippy_path.display(),
404403
&self.path.display()
405404
);

‎tests/ui/uninlined_format_args.fixed

+3-8
Original file line numberDiff line numberDiff line change
@@ -44,9 +44,7 @@ fn tester(fn_arg: i32) {
4444
println!("val='{local_i32}'"); // space+tab
4545
println!("val='{local_i32}'"); // tab+space
4646
println!(
47-
"val='{
48-
}'",
49-
local_i32
47+
"val='{local_i32}'"
5048
);
5149
println!("{local_i32}");
5250
println!("{fn_arg}");
@@ -110,8 +108,7 @@ fn tester(fn_arg: i32) {
110108
println!("{local_f64:width$.prec$}");
111109
println!("{local_f64:width$.prec$} {local_f64} {width} {prec}");
112110
println!(
113-
"{0:1$.2$} {0:2$.1$} {1:0$.2$} {1:2$.0$} {2:0$.1$} {2:1$.0$}",
114-
local_i32, width, prec,
111+
"{local_i32:width$.prec$} {local_i32:prec$.width$} {width:local_i32$.prec$} {width:prec$.local_i32$} {prec:local_i32$.width$} {prec:width$.local_i32$}",
115112
);
116113
println!(
117114
"{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) {
142139
println!(no_param_str!(), local_i32);
143140

144141
println!(
145-
"{}",
146-
// comment with a comma , in it
147-
val,
142+
"{val}",
148143
);
149144
println!("{val}");
150145

‎tests/ui/uninlined_format_args.stderr

+30-1
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,16 @@ LL - println!("val='{ }'", local_i32); // tab+space
5959
LL + println!("val='{local_i32}'"); // tab+space
6060
|
6161

62+
error: variables can be used directly in the `format!` string
63+
--> $DIR/uninlined_format_args.rs:46:5
64+
|
65+
LL | / println!(
66+
LL | | "val='{
67+
LL | | }'",
68+
LL | | local_i32
69+
LL | | );
70+
| |_____^
71+
6272
error: variables can be used directly in the `format!` string
6373
--> $DIR/uninlined_format_args.rs:51:5
6474
|
@@ -767,6 +777,15 @@ LL - println!("{:1$.2$} {0} {1} {2}", local_f64, width, prec);
767777
LL + println!("{local_f64:width$.prec$} {local_f64} {width} {prec}");
768778
|
769779

780+
error: variables can be used directly in the `format!` string
781+
--> $DIR/uninlined_format_args.rs:112:5
782+
|
783+
LL | / println!(
784+
LL | | "{0:1$.2$} {0:2$.1$} {1:0$.2$} {1:2$.0$} {2:0$.1$} {2:1$.0$}",
785+
LL | | local_i32, width, prec,
786+
LL | | );
787+
| |_____^
788+
770789
error: variables can be used directly in the `format!` string
771790
--> $DIR/uninlined_format_args.rs:123:5
772791
|
@@ -815,6 +834,16 @@ LL - println!("{}", format!("{}", local_i32));
815834
LL + println!("{}", format!("{local_i32}"));
816835
|
817836

837+
error: variables can be used directly in the `format!` string
838+
--> $DIR/uninlined_format_args.rs:144:5
839+
|
840+
LL | / println!(
841+
LL | | "{}",
842+
LL | | // comment with a comma , in it
843+
LL | | val,
844+
LL | | );
845+
| |_____^
846+
818847
error: variables can be used directly in the `format!` string
819848
--> $DIR/uninlined_format_args.rs:149:5
820849
|
@@ -875,5 +904,5 @@ LL - println!("expand='{}'", local_i32);
875904
LL + println!("expand='{local_i32}'");
876905
|
877906

878-
error: aborting due to 73 previous errors
907+
error: aborting due to 76 previous errors
879908

0 commit comments

Comments
 (0)