Skip to content

Commit 86cfa0b

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 f9c1d15 commit 86cfa0b

File tree

6 files changed

+171
-62
lines changed

6 files changed

+171
-62
lines changed

‎clippy_lints/src/format_args.rs

Lines changed: 29 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_utils::diagnostics::{span_lint_and_sugg, span_lint_and_then};
34
use clippy_utils::is_diag_trait_item;
@@ -10,6 +11,7 @@ use clippy_utils::source::snippet_opt;
1011
use clippy_utils::ty::{implements_trait, is_type_lang_item};
1112
use if_chain::if_chain;
1213
use itertools::Itertools;
14+
use rustc_ast::token::LitKind;
1315
use rustc_ast::{
1416
FormatArgPosition, FormatArgPositionKind, FormatArgsPiece, FormatArgumentKind, FormatCount, FormatOptions,
1517
FormatPlaceholder, FormatTrait,
@@ -316,8 +318,8 @@ fn check_uninlined_args(
316318
// we cannot remove any other arguments in the format string,
317319
// because the index numbers might be wrong after inlining.
318320
// Example of an un-inlinable format: print!("{}{1}", foo, 2)
319-
for (pos, usage) in format_arg_positions(args) {
320-
if !check_one_arg(args, pos, usage, &mut fixes, ignore_mixed) {
321+
for (pos, usage, pl) in format_arg_positions(args) {
322+
if !check_one_arg(cx, args, pos, usage, pl, &mut fixes, ignore_mixed) {
321323
return;
322324
}
323325
}
@@ -351,14 +353,18 @@ fn check_uninlined_args(
351353
}
352354

353355
fn check_one_arg(
356+
cx: &LateContext<'_>,
354357
args: &rustc_ast::FormatArgs,
355358
pos: &FormatArgPosition,
356359
usage: FormatParamUsage,
360+
placeholder: Option<&FormatPlaceholder>,
357361
fixes: &mut Vec<(Span, String)>,
358362
ignore_mixed: bool,
359363
) -> bool {
360364
let index = pos.index.unwrap();
361-
let arg = &args.arguments.all_args()[index];
365+
// If the argument is not found by its index, something is weird, ignore and stop processing this
366+
// case.
367+
let Some(arg) = &args.arguments.by_index(index) else { return false; };
362368

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

458479
fn format_arg_positions(
459480
format_args: &rustc_ast::FormatArgs,
460-
) -> impl Iterator<Item = (&FormatArgPosition, FormatParamUsage)> {
481+
) -> impl Iterator<Item = (&FormatArgPosition, FormatParamUsage, Option<&FormatPlaceholder>)> {
461482
format_args.template.iter().flat_map(|piece| match piece {
462483
FormatArgsPiece::Placeholder(placeholder) => {
463484
let mut positions = ArrayVec::<_, 3>::new();
464485

465-
positions.push((&placeholder.argument, FormatParamUsage::Argument));
486+
positions.push((&placeholder.argument, FormatParamUsage::Argument, Some(placeholder)));
466487
if let Some(FormatCount::Argument(position)) = &placeholder.format_options.width {
467-
positions.push((position, FormatParamUsage::Width));
488+
positions.push((position, FormatParamUsage::Width, None));
468489
}
469490
if let Some(FormatCount::Argument(position)) = &placeholder.format_options.precision {
470-
positions.push((position, FormatParamUsage::Precision));
491+
positions.push((position, FormatParamUsage::Precision, None));
471492
}
472493

473494
positions
@@ -479,7 +500,7 @@ fn format_arg_positions(
479500
/// Returns true if the format argument at `index` is referred to by multiple format params
480501
fn is_aliased(format_args: &rustc_ast::FormatArgs, index: usize) -> bool {
481502
format_arg_positions(format_args)
482-
.filter(|(position, _)| position.index == Ok(index))
503+
.filter(|(position, ..)| position.index == Ok(index))
483504
.at_most_one()
484505
.is_err()
485506
}

‎clippy_lints/src/write.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -539,7 +539,7 @@ fn check_literal(cx: &LateContext<'_>, format_args: &FormatArgs, name: &str) {
539539
/// `r#"a"#` -> (`a`, true)
540540
///
541541
/// `"b"` -> (`b`, false)
542-
fn extract_str_literal(literal: &str) -> Option<(String, bool)> {
542+
pub fn extract_str_literal(literal: &str) -> Option<(String, bool)> {
543543
let (literal, raw) = match literal.strip_prefix('r') {
544544
Some(stripped) => (stripped.trim_matches('#'), true),
545545
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
@@ -20,7 +20,7 @@ LL | println!("Hello {} is {:.*}", "x", local_i32, local_f64);
2020
help: change this to
2121
|
2222
LL - println!("Hello {} is {:.*}", "x", local_i32, local_f64);
23-
LL + println!("Hello {} is {local_f64:.local_i32$}", "x");
23+
LL + println!("Hello x is {local_f64:.local_i32$}");
2424
|
2525

2626
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
@@ -54,7 +54,7 @@ fn tester(fn_arg: i32) {
5454
println!("{local_i32:<3}");
5555
println!("{local_i32:#010x}");
5656
println!("{local_f64:.1}");
57-
println!("Hello {} is {:.*}", "x", local_i32, local_f64);
57+
println!("Hello x is {local_f64:.local_i32$}");
5858
println!("Hello {} is {:.*}", local_i32, 5, local_f64);
5959
println!("Hello {} is {2:.*}", local_i32, 5, local_f64);
6060
println!("{local_i32} {local_f64}");
@@ -78,12 +78,12 @@ fn tester(fn_arg: i32) {
7878
println!("{local_i32} {local_f64}");
7979
println!("{local_f64} {local_i32}");
8080
println!("{local_f64} {local_i32} {local_f64} {local_i32}");
81-
println!("{1} {0}", "str", local_i32);
81+
println!("{local_i32} str");
8282
println!("{local_i32}");
83-
println!("{local_i32:width$}");
84-
println!("{local_i32:width$}");
85-
println!("{local_i32:.prec$}");
86-
println!("{local_i32:.prec$}");
83+
println!("{local_i32:0$}", width);
84+
println!("{local_i32:w$}", w = width);
85+
println!("{local_i32:.0$}", prec);
86+
println!("{local_i32:.p$}", p = prec);
8787
println!("{val:val$}");
8888
println!("{val:val$}");
8989
println!("{val:val$.val$}");
@@ -262,3 +262,17 @@ fn tester2() {
262262
local_i32,
263263
};
264264
}
265+
266+
fn literals() {
267+
let var = 5;
268+
println!("foo");
269+
println!("foo");
270+
println!("{:var$}", "foo");
271+
println!("foo");
272+
println!("{0:1$}", "foo", 5);
273+
println!("var {var} lit foo");
274+
println!("var {var} lit foo");
275+
println!("var foo lit foo");
276+
println!("var foo lit foo {var},");
277+
println!("var {0} lit {} {},", "foo", 5);
278+
}

‎tests/ui/uninlined_format_args.rs

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -267,3 +267,17 @@ fn tester2() {
267267
local_i32,
268268
};
269269
}
270+
271+
fn literals() {
272+
let var = 5;
273+
println!("{}", "foo");
274+
println!("{:5}", "foo");
275+
println!("{:var$}", "foo");
276+
println!("{:-5}", "foo");
277+
println!("{0:1$}", "foo", 5);
278+
println!("var {} lit {}", var, "foo");
279+
println!("var {1} lit {0}", "foo", var);
280+
println!("var {} lit {0}", "foo");
281+
println!("var {0} lit {} {},", "foo", var);
282+
println!("var {0} lit {} {},", "foo", 5);
283+
}

‎tests/ui/uninlined_format_args.stderr

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

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

408420
error: variables can be used directly in the `format!` string
409-
--> $DIR/uninlined_format_args.rs:84:5
410-
|
411-
LL | println!("{v}", v = local_i32);
412-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
413-
|
414-
help: change this to
415-
|
416-
LL - println!("{v}", v = local_i32);
417-
LL + println!("{local_i32}");
418-
|
419-
420-
error: variables can be used directly in the `format!` string
421-
--> $DIR/uninlined_format_args.rs:85:5
422-
|
423-
LL | println!("{local_i32:0$}", width);
424-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
425-
|
426-
help: change this to
427-
|
428-
LL - println!("{local_i32:0$}", width);
429-
LL + println!("{local_i32:width$}");
430-
|
431-
432-
error: variables can be used directly in the `format!` string
433-
--> $DIR/uninlined_format_args.rs:86:5
421+
--> $DIR/uninlined_format_args.rs:83:5
434422
|
435-
LL | println!("{local_i32:w$}", w = width);
423+
LL | println!("{1} {0}", "str", local_i32);
436424
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
437425
|
438426
help: change this to
439427
|
440-
LL - println!("{local_i32:w$}", w = width);
441-
LL + println!("{local_i32:width$}");
428+
LL - println!("{1} {0}", "str", local_i32);
429+
LL + println!("{local_i32} str");
442430
|
443431

444432
error: variables can be used directly in the `format!` string
445-
--> $DIR/uninlined_format_args.rs:87:5
446-
|
447-
LL | println!("{local_i32:.0$}", prec);
448-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
449-
|
450-
help: change this to
451-
|
452-
LL - println!("{local_i32:.0$}", prec);
453-
LL + println!("{local_i32:.prec$}");
454-
|
455-
456-
error: variables can be used directly in the `format!` string
457-
--> $DIR/uninlined_format_args.rs:88:5
433+
--> $DIR/uninlined_format_args.rs:84:5
458434
|
459-
LL | println!("{local_i32:.p$}", p = prec);
460-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
435+
LL | println!("{v}", v = local_i32);
436+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
461437
|
462438
help: change this to
463439
|
464-
LL - println!("{local_i32:.p$}", p = prec);
465-
LL + println!("{local_i32:.prec$}");
440+
LL - println!("{v}", v = local_i32);
441+
LL + println!("{local_i32}");
466442
|
467443

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

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

0 commit comments

Comments
 (0)