Skip to content

Commit e50f944

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 4350678 commit e50f944

File tree

6 files changed

+180
-63
lines changed

6 files changed

+180
-63
lines changed

‎clippy_lints/src/format_args.rs

Lines changed: 38 additions & 9 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,
@@ -324,8 +326,8 @@ impl<'a, 'tcx> FormatArgsExpr<'a, 'tcx> {
324326
// we cannot remove any other arguments in the format string,
325327
// because the index numbers might be wrong after inlining.
326328
// Example of an un-inlinable format: print!("{}{1}", foo, 2)
327-
for (pos, usage) in self.format_arg_positions() {
328-
if !self.check_one_arg(pos, usage, &mut fixes) {
329+
for (pos, usage, pl) in self.format_arg_positions() {
330+
if !self.check_one_arg(pos, usage, pl, &mut fixes) {
329331
return;
330332
}
331333
}
@@ -360,9 +362,19 @@ impl<'a, 'tcx> FormatArgsExpr<'a, 'tcx> {
360362
);
361363
}
362364

363-
fn check_one_arg(&self, pos: &FormatArgPosition, usage: FormatParamUsage, fixes: &mut Vec<(Span, String)>) -> bool {
365+
fn check_one_arg(
366+
&self,
367+
pos: &FormatArgPosition,
368+
usage: FormatParamUsage,
369+
placeholder: Option<&FormatPlaceholder>,
370+
fixes: &mut Vec<(Span, String)>,
371+
) -> bool {
364372
let index = pos.index.unwrap();
365-
let arg = &self.format_args.arguments.all_args()[index];
373+
// If the argument is not found by its index, something is weird, ignore and stop processing this
374+
// case.
375+
let Some(arg) = &self.format_args.arguments.by_index(index) else {
376+
return false;
377+
};
366378

367379
if !matches!(arg.kind, FormatArgumentKind::Captured(_))
368380
&& let rustc_ast::ExprKind::Path(None, path) = &arg.expr.kind
@@ -379,6 +391,21 @@ impl<'a, 'tcx> FormatArgsExpr<'a, 'tcx> {
379391
fixes.push((pos_span, replacement));
380392
fixes.push((arg_span, String::new()));
381393
true // successful inlining, continue checking
394+
} else if !matches!(arg.kind, FormatArgumentKind::Captured(_))
395+
&& let Some(FormatPlaceholder{span: Some(placeholder_span), ..}) = placeholder
396+
&& let rustc_ast::ExprKind::Lit(lit) = &arg.expr.kind
397+
&& lit.kind == LitKind::Str // FIXME: lit.kind must match the format string kind
398+
&& !arg.expr.span.from_expansion()
399+
&& let Some(value_string) = snippet_opt(self.cx, arg.expr.span)
400+
&& let Some((value_string, false)) = extract_str_literal(&value_string) // FIXME: handle raw strings
401+
// && let rustc_ast::ExprKind::Path(None, path) = &arg.expr.kind
402+
// && let [segment] = path.segments.as_slice()
403+
// && segment.args.is_none()
404+
&& let Some(arg_span) = format_arg_removal_span(self.format_args, index)
405+
{
406+
fixes.push((*placeholder_span, value_string));
407+
fixes.push((arg_span, String::new()));
408+
true // successful inlining, continue checking
382409
} else {
383410
// Do not continue inlining (return false) in case
384411
// * if we can't inline a numbered argument, e.g. `print!("{0} ...", foo.bar, ...)`
@@ -454,17 +481,19 @@ impl<'a, 'tcx> FormatArgsExpr<'a, 'tcx> {
454481
}
455482
}
456483

457-
fn format_arg_positions(&self) -> impl Iterator<Item = (&FormatArgPosition, FormatParamUsage)> {
484+
fn format_arg_positions(
485+
&self,
486+
) -> impl Iterator<Item = (&FormatArgPosition, FormatParamUsage, Option<&FormatPlaceholder>)> {
458487
self.format_args.template.iter().flat_map(|piece| match piece {
459488
FormatArgsPiece::Placeholder(placeholder) => {
460489
let mut positions = ArrayVec::<_, 3>::new();
461490

462-
positions.push((&placeholder.argument, FormatParamUsage::Argument));
491+
positions.push((&placeholder.argument, FormatParamUsage::Argument, Some(placeholder)));
463492
if let Some(FormatCount::Argument(position)) = &placeholder.format_options.width {
464-
positions.push((position, FormatParamUsage::Width));
493+
positions.push((position, FormatParamUsage::Width, None));
465494
}
466495
if let Some(FormatCount::Argument(position)) = &placeholder.format_options.precision {
467-
positions.push((position, FormatParamUsage::Precision));
496+
positions.push((position, FormatParamUsage::Precision, None));
468497
}
469498

470499
positions
@@ -476,7 +505,7 @@ impl<'a, 'tcx> FormatArgsExpr<'a, 'tcx> {
476505
/// Returns true if the format argument at `index` is referred to by multiple format params
477506
fn is_aliased(&self, index: usize) -> bool {
478507
self.format_arg_positions()
479-
.filter(|(position, _)| position.index == Ok(index))
508+
.filter(|(position, ..)| position.index == Ok(index))
480509
.at_most_one()
481510
.is_err()
482511
}

‎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)