Skip to content

Commit de80373

Browse files
nyuriknyurik
committed
WIP format literal arg inlining
A rough draft of #10739 implementation. The goal is to allow any kind of literal string arguments to be inlined automatically. ```rust format!("hello {}", "world") // is replaced with format!("hello world") ```
1 parent b0e7640 commit de80373

File tree

6 files changed

+173
-62
lines changed

6 files changed

+173
-62
lines changed

‎clippy_lints/src/format_args.rs

Lines changed: 31 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
use crate::write::extract_str_literal;
12
use arrayvec::ArrayVec;
23
use clippy_config::msrvs::{self, Msrv};
34
use clippy_utils::diagnostics::{span_lint_and_sugg, span_lint_and_then};
@@ -9,6 +10,7 @@ use clippy_utils::macros::{
910
use clippy_utils::source::snippet_opt;
1011
use clippy_utils::ty::{implements_trait, is_type_lang_item};
1112
use itertools::Itertools;
13+
use rustc_ast::token::LitKind;
1214
use rustc_ast::{
1315
FormatArgPosition, FormatArgPositionKind, FormatArgsPiece, FormatArgumentKind, FormatCount, FormatOptions,
1416
FormatPlaceholder, FormatTrait,
@@ -311,8 +313,8 @@ fn check_uninlined_args(
311313
// we cannot remove any other arguments in the format string,
312314
// because the index numbers might be wrong after inlining.
313315
// Example of an un-inlinable format: print!("{}{1}", foo, 2)
314-
for (pos, usage) in format_arg_positions(args) {
315-
if !check_one_arg(args, pos, usage, &mut fixes, ignore_mixed) {
316+
for (pos, usage, pl) in format_arg_positions(args) {
317+
if !check_one_arg(cx, args, pos, usage, pl, &mut fixes, ignore_mixed) {
316318
return;
317319
}
318320
}
@@ -346,14 +348,20 @@ fn check_uninlined_args(
346348
}
347349

348350
fn check_one_arg(
351+
cx: &LateContext<'_>,
349352
args: &rustc_ast::FormatArgs,
350353
pos: &FormatArgPosition,
351354
usage: FormatParamUsage,
355+
placeholder: Option<&FormatPlaceholder>,
352356
fixes: &mut Vec<(Span, String)>,
353357
ignore_mixed: bool,
354358
) -> bool {
355359
let index = pos.index.unwrap();
356-
let arg = &args.arguments.all_args()[index];
360+
// If the argument is not found by its index, something is weird, ignore and stop processing this
361+
// case.
362+
let Some(arg) = &args.arguments.by_index(index) else {
363+
return false;
364+
};
357365

358366
if !matches!(arg.kind, FormatArgumentKind::Captured(_))
359367
&& let rustc_ast::ExprKind::Path(None, path) = &arg.expr.kind
@@ -370,6 +378,21 @@ fn check_one_arg(
370378
fixes.push((pos_span, replacement));
371379
fixes.push((arg_span, String::new()));
372380
true // successful inlining, continue checking
381+
} else if !matches!(arg.kind, FormatArgumentKind::Captured(_))
382+
&& let Some(FormatPlaceholder{span: Some(placeholder_span), ..}) = placeholder
383+
&& let rustc_ast::ExprKind::Lit(lit) = &arg.expr.kind
384+
&& lit.kind == LitKind::Str // FIXME: lit.kind must match the format string kind
385+
&& !arg.expr.span.from_expansion()
386+
&& let Some(value_string) = snippet_opt(cx, arg.expr.span)
387+
&& let Some((value_string, false)) = extract_str_literal(&value_string) // FIXME: handle raw strings
388+
// && let rustc_ast::ExprKind::Path(None, path) = &arg.expr.kind
389+
// && let [segment] = path.segments.as_slice()
390+
// && segment.args.is_none()
391+
&& let Some(arg_span) = format_arg_removal_span(args, index)
392+
{
393+
fixes.push((*placeholder_span, value_string));
394+
fixes.push((arg_span, String::new()));
395+
true // successful inlining, continue checking
373396
} else {
374397
// Do not continue inlining (return false) in case
375398
// * if we can't inline a numbered argument, e.g. `print!("{0} ...", foo.bar, ...)`
@@ -446,17 +469,17 @@ fn check_to_string_in_format_args(cx: &LateContext<'_>, name: Symbol, value: &Ex
446469

447470
fn format_arg_positions(
448471
format_args: &rustc_ast::FormatArgs,
449-
) -> impl Iterator<Item = (&FormatArgPosition, FormatParamUsage)> {
472+
) -> impl Iterator<Item = (&FormatArgPosition, FormatParamUsage, Option<&FormatPlaceholder>)> {
450473
format_args.template.iter().flat_map(|piece| match piece {
451474
FormatArgsPiece::Placeholder(placeholder) => {
452475
let mut positions = ArrayVec::<_, 3>::new();
453476

454-
positions.push((&placeholder.argument, FormatParamUsage::Argument));
477+
positions.push((&placeholder.argument, FormatParamUsage::Argument, Some(placeholder)));
455478
if let Some(FormatCount::Argument(position)) = &placeholder.format_options.width {
456-
positions.push((position, FormatParamUsage::Width));
479+
positions.push((position, FormatParamUsage::Width, None));
457480
}
458481
if let Some(FormatCount::Argument(position)) = &placeholder.format_options.precision {
459-
positions.push((position, FormatParamUsage::Precision));
482+
positions.push((position, FormatParamUsage::Precision, None));
460483
}
461484

462485
positions
@@ -468,7 +491,7 @@ fn format_arg_positions(
468491
/// Returns true if the format argument at `index` is referred to by multiple format params
469492
fn is_aliased(format_args: &rustc_ast::FormatArgs, index: usize) -> bool {
470493
format_arg_positions(format_args)
471-
.filter(|(position, _)| position.index == Ok(index))
494+
.filter(|(position, ..)| position.index == Ok(index))
472495
.at_most_one()
473496
.is_err()
474497
}

‎clippy_lints/src/write.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -594,7 +594,7 @@ fn positional_arg_piece_span(piece: &FormatArgsPiece) -> Option<(Span, usize)> {
594594
/// `r#"a"#` -> (`a`, true)
595595
///
596596
/// `"b"` -> (`b`, false)
597-
fn extract_str_literal(literal: &str) -> Option<(String, bool)> {
597+
pub fn extract_str_literal(literal: &str) -> Option<(String, bool)> {
598598
let (literal, raw) = match literal.strip_prefix('r') {
599599
Some(stripped) => (stripped.trim_matches('#'), true),
600600
None => (literal, false),

‎tests/ui-toml/allow_mixed_uninlined_format_args/uninlined_format_args.stderr

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ LL | println!("Hello {} is {:.*}", "x", local_i32, local_f64);
2121
help: change this to
2222
|
2323
LL - println!("Hello {} is {:.*}", "x", local_i32, local_f64);
24-
LL + println!("Hello {} is {local_f64:.local_i32$}", "x");
24+
LL + println!("Hello x is {local_f64:.local_i32$}");
2525
|
2626

2727
error: literal with an empty format string

‎tests/ui/uninlined_format_args.fixed

Lines changed: 20 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ fn tester(fn_arg: i32) {
5959
println!("{local_i32:<3}");
6060
println!("{local_i32:#010x}");
6161
println!("{local_f64:.1}");
62-
println!("Hello {} is {:.*}", "x", local_i32, local_f64);
62+
println!("Hello x is {local_f64:.local_i32$}");
6363
println!("Hello {} is {:.*}", local_i32, 5, local_f64);
6464
println!("Hello {} is {2:.*}", local_i32, 5, local_f64);
6565
println!("{local_i32} {local_f64}");
@@ -83,12 +83,12 @@ fn tester(fn_arg: i32) {
8383
println!("{local_i32} {local_f64}");
8484
println!("{local_f64} {local_i32}");
8585
println!("{local_f64} {local_i32} {local_f64} {local_i32}");
86-
println!("{1} {0}", "str", local_i32);
86+
println!("{local_i32} str");
8787
println!("{local_i32}");
88-
println!("{local_i32:width$}");
89-
println!("{local_i32:width$}");
90-
println!("{local_i32:.prec$}");
91-
println!("{local_i32:.prec$}");
88+
println!("{local_i32:0$}", width);
89+
println!("{local_i32:w$}", w = width);
90+
println!("{local_i32:.0$}", prec);
91+
println!("{local_i32:.p$}", p = prec);
9292
println!("{val:val$}");
9393
println!("{val:val$}");
9494
println!("{val:val$.val$}");
@@ -267,3 +267,17 @@ fn tester2() {
267267
local_i32,
268268
};
269269
}
270+
271+
fn literals() {
272+
let var = 5;
273+
println!("foo");
274+
println!("foo");
275+
println!("{:var$}", "foo");
276+
println!("foo");
277+
println!("{0:1$}", "foo", 5);
278+
println!("var {var} lit foo");
279+
println!("var {var} lit foo");
280+
println!("var foo lit foo");
281+
println!("var foo lit foo {var},");
282+
println!("var {0} lit {} {},", "foo", 5);
283+
}

‎tests/ui/uninlined_format_args.rs

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -272,3 +272,17 @@ fn tester2() {
272272
local_i32,
273273
};
274274
}
275+
276+
fn literals() {
277+
let var = 5;
278+
println!("{}", "foo");
279+
println!("{:5}", "foo");
280+
println!("{:var$}", "foo");
281+
println!("{:-5}", "foo");
282+
println!("{0:1$}", "foo", 5);
283+
println!("var {} lit {}", var, "foo");
284+
println!("var {1} lit {0}", "foo", var);
285+
println!("var {} lit {0}", "foo");
286+
println!("var {0} lit {} {},", "foo", var);
287+
println!("var {0} lit {} {},", "foo", 5);
288+
}

‎tests/ui/uninlined_format_args.stderr

Lines changed: 106 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -178,6 +178,18 @@ LL - println!("{:.1}", local_f64);
178178
LL + println!("{local_f64:.1}");
179179
|
180180

181+
error: variables can be used directly in the `format!` string
182+
--> $DIR/uninlined_format_args.rs:64:5
183+
|
184+
LL | println!("Hello {} is {:.*}", "x", local_i32, local_f64);
185+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
186+
|
187+
help: change this to
188+
|
189+
LL - println!("Hello {} is {:.*}", "x", local_i32, local_f64);
190+
LL + println!("Hello x is {local_f64:.local_i32$}");
191+
|
192+
181193
error: variables can be used directly in the `format!` string
182194
--> $DIR/uninlined_format_args.rs:67:5
183195
|
@@ -407,63 +419,27 @@ LL + println!("{local_f64} {local_i32} {local_f64} {local_i32}");
407419
|
408420

409421
error: variables can be used directly in the `format!` string
410-
--> $DIR/uninlined_format_args.rs:89:5
411-
|
412-
LL | println!("{v}", v = local_i32);
413-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
414-
|
415-
help: change this to
416-
|
417-
LL - println!("{v}", v = local_i32);
418-
LL + println!("{local_i32}");
419-
|
420-
421-
error: variables can be used directly in the `format!` string
422-
--> $DIR/uninlined_format_args.rs:90:5
423-
|
424-
LL | println!("{local_i32:0$}", width);
425-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
426-
|
427-
help: change this to
428-
|
429-
LL - println!("{local_i32:0$}", width);
430-
LL + println!("{local_i32:width$}");
431-
|
432-
433-
error: variables can be used directly in the `format!` string
434-
--> $DIR/uninlined_format_args.rs:91:5
422+
--> $DIR/uninlined_format_args.rs:88:5
435423
|
436-
LL | println!("{local_i32:w$}", w = width);
424+
LL | println!("{1} {0}", "str", local_i32);
437425
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
438426
|
439427
help: change this to
440428
|
441-
LL - println!("{local_i32:w$}", w = width);
442-
LL + println!("{local_i32:width$}");
429+
LL - println!("{1} {0}", "str", local_i32);
430+
LL + println!("{local_i32} str");
443431
|
444432

445433
error: variables can be used directly in the `format!` string
446-
--> $DIR/uninlined_format_args.rs:92:5
447-
|
448-
LL | println!("{local_i32:.0$}", prec);
449-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
450-
|
451-
help: change this to
452-
|
453-
LL - println!("{local_i32:.0$}", prec);
454-
LL + println!("{local_i32:.prec$}");
455-
|
456-
457-
error: variables can be used directly in the `format!` string
458-
--> $DIR/uninlined_format_args.rs:93:5
434+
--> $DIR/uninlined_format_args.rs:89:5
459435
|
460-
LL | println!("{local_i32:.p$}", p = prec);
461-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
436+
LL | println!("{v}", v = local_i32);
437+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
462438
|
463439
help: change this to
464440
|
465-
LL - println!("{local_i32:.p$}", p = prec);
466-
LL + println!("{local_i32:.prec$}");
441+
LL - println!("{v}", v = local_i32);
442+
LL + println!("{local_i32}");
467443
|
468444

469445
error: variables can be used directly in the `format!` string
@@ -845,5 +821,89 @@ LL - println!("expand='{}'", local_i32);
845821
LL + println!("expand='{local_i32}'");
846822
|
847823

848-
error: aborting due to 71 previous errors
824+
error: variables can be used directly in the `format!` string
825+
--> $DIR/uninlined_format_args.rs:278:5
826+
|
827+
LL | println!("{}", "foo");
828+
| ^^^^^^^^^^^^^^^^^^^^^
829+
|
830+
help: change this to
831+
|
832+
LL - println!("{}", "foo");
833+
LL + println!("foo");
834+
|
835+
836+
error: variables can be used directly in the `format!` string
837+
--> $DIR/uninlined_format_args.rs:279:5
838+
|
839+
LL | println!("{:5}", "foo");
840+
| ^^^^^^^^^^^^^^^^^^^^^^^
841+
|
842+
help: change this to
843+
|
844+
LL - println!("{:5}", "foo");
845+
LL + println!("foo");
846+
|
847+
848+
error: variables can be used directly in the `format!` string
849+
--> $DIR/uninlined_format_args.rs:281:5
850+
|
851+
LL | println!("{:-5}", "foo");
852+
| ^^^^^^^^^^^^^^^^^^^^^^^^
853+
|
854+
help: change this to
855+
|
856+
LL - println!("{:-5}", "foo");
857+
LL + println!("foo");
858+
|
859+
860+
error: variables can be used directly in the `format!` string
861+
--> $DIR/uninlined_format_args.rs:283:5
862+
|
863+
LL | println!("var {} lit {}", var, "foo");
864+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
865+
|
866+
help: change this to
867+
|
868+
LL - println!("var {} lit {}", var, "foo");
869+
LL + println!("var {var} lit foo");
870+
|
871+
872+
error: variables can be used directly in the `format!` string
873+
--> $DIR/uninlined_format_args.rs:284:5
874+
|
875+
LL | println!("var {1} lit {0}", "foo", var);
876+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
877+
|
878+
help: change this to
879+
|
880+
LL - println!("var {1} lit {0}", "foo", var);
881+
LL + println!("var {var} lit foo");
882+
|
883+
884+
error: variables can be used directly in the `format!` string
885+
--> $DIR/uninlined_format_args.rs:285:5
886+
|
887+
LL | println!("var {} lit {0}", "foo");
888+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
889+
|
890+
help: change this to
891+
|
892+
LL - println!("var {} lit {0}", "foo");
893+
LL + println!("var foo lit foo");
894+
|
895+
896+
error: variables can be used directly in the `format!` string
897+
--> $DIR/uninlined_format_args.rs:286:5
898+
|
899+
LL | println!("var {0} lit {} {},", "foo", var);
900+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
901+
|
902+
help: change this to
903+
|
904+
LL - println!("var {0} lit {} {},", "foo", var);
905+
LL + println!("var foo lit foo {var},");
906+
|
907+
908+
error: aborting due to 76 previous errors
849909

0 commit comments

Comments
 (0)