Skip to content

Commit

Permalink
WIP format literal arg inlining
Browse files Browse the repository at this point in the history 
A rough draft of rust-lang#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")
```
  • Loading branch information
@nyurik
nyurik committed Feb 12, 2024
1 parent 4350678 commit e50f944
Show file tree
Hide file tree
Showing 6 changed files with 180 additions and 63 deletions.
47 changes: 38 additions & 9 deletions clippy_lints/src/format_args.rs
View file
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
use crate::write::extract_str_literal;
use arrayvec::ArrayVec;
use clippy_config::msrvs::{self, Msrv};
use clippy_utils::diagnostics::{span_lint_and_sugg, span_lint_and_then};
Expand All @@ -9,6 +10,7 @@ use clippy_utils::macros::{
use clippy_utils::source::snippet_opt;
use clippy_utils::ty::{implements_trait, is_type_lang_item};
use itertools::Itertools;
use rustc_ast::token::LitKind;
use rustc_ast::{
FormatArgPosition, FormatArgPositionKind, FormatArgsPiece, FormatArgumentKind, FormatCount, FormatOptions,
FormatPlaceholder, FormatTrait,
Expand Down Expand Up @@ -324,8 +326,8 @@ impl<'a, 'tcx> FormatArgsExpr<'a, 'tcx> {
// we cannot remove any other arguments in the format string,
// because the index numbers might be wrong after inlining.
// Example of an un-inlinable format: print!("{}{1}", foo, 2)
for (pos, usage) in self.format_arg_positions() {
if !self.check_one_arg(pos, usage, &mut fixes) {
for (pos, usage, pl) in self.format_arg_positions() {
if !self.check_one_arg(pos, usage, pl, &mut fixes) {
return;
}
}
Expand Down Expand Up @@ -360,9 +362,19 @@ impl<'a, 'tcx> FormatArgsExpr<'a, 'tcx> {
);
}

fn check_one_arg(&self, pos: &FormatArgPosition, usage: FormatParamUsage, fixes: &mut Vec<(Span, String)>) -> bool {
fn check_one_arg(
&self,
pos: &FormatArgPosition,
usage: FormatParamUsage,
placeholder: Option<&FormatPlaceholder>,
fixes: &mut Vec<(Span, String)>,
) -> bool {
let index = pos.index.unwrap();
let arg = &self.format_args.arguments.all_args()[index];
// If the argument is not found by its index, something is weird, ignore and stop processing this
// case.
let Some(arg) = &self.format_args.arguments.by_index(index) else {
return false;
};

if !matches!(arg.kind, FormatArgumentKind::Captured(_))
&& let rustc_ast::ExprKind::Path(None, path) = &arg.expr.kind
Expand All @@ -379,6 +391,21 @@ impl<'a, 'tcx> FormatArgsExpr<'a, 'tcx> {
fixes.push((pos_span, replacement));
fixes.push((arg_span, String::new()));
true // successful inlining, continue checking
} else if !matches!(arg.kind, FormatArgumentKind::Captured(_))
&& let Some(FormatPlaceholder{span: Some(placeholder_span), ..}) = placeholder
&& let rustc_ast::ExprKind::Lit(lit) = &arg.expr.kind
&& lit.kind == LitKind::Str // FIXME: lit.kind must match the format string kind
&& !arg.expr.span.from_expansion()
&& let Some(value_string) = snippet_opt(self.cx, arg.expr.span)
&& let Some((value_string, false)) = extract_str_literal(&value_string) // FIXME: handle raw strings
// && let rustc_ast::ExprKind::Path(None, path) = &arg.expr.kind
// && let [segment] = path.segments.as_slice()
// && segment.args.is_none()
&& let Some(arg_span) = format_arg_removal_span(self.format_args, index)
{
fixes.push((*placeholder_span, value_string));
fixes.push((arg_span, String::new()));
true // successful inlining, continue checking
} else {
// Do not continue inlining (return false) in case
// * if we can't inline a numbered argument, e.g. `print!("{0} ...", foo.bar, ...)`
Expand Down Expand Up @@ -454,17 +481,19 @@ impl<'a, 'tcx> FormatArgsExpr<'a, 'tcx> {
}
}

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

positions.push((&placeholder.argument, FormatParamUsage::Argument));
positions.push((&placeholder.argument, FormatParamUsage::Argument, Some(placeholder)));
if let Some(FormatCount::Argument(position)) = &placeholder.format_options.width {
positions.push((position, FormatParamUsage::Width));
positions.push((position, FormatParamUsage::Width, None));
}
if let Some(FormatCount::Argument(position)) = &placeholder.format_options.precision {
positions.push((position, FormatParamUsage::Precision));
positions.push((position, FormatParamUsage::Precision, None));
}

positions
Expand All @@ -476,7 +505,7 @@ impl<'a, 'tcx> FormatArgsExpr<'a, 'tcx> {
/// Returns true if the format argument at `index` is referred to by multiple format params
fn is_aliased(&self, index: usize) -> bool {
self.format_arg_positions()
.filter(|(position, _)| position.index == Ok(index))
.filter(|(position, ..)| position.index == Ok(index))
.at_most_one()
.is_err()
}
Expand Down
2 changes: 1 addition & 1 deletion clippy_lints/src/write.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -594,7 +594,7 @@ fn positional_arg_piece_span(piece: &FormatArgsPiece) -> Option<(Span, usize)> {
/// `r#"a"#` -> (`a`, true)
///
/// `"b"` -> (`b`, false)
fn extract_str_literal(literal: &str) -> Option<(String, bool)> {
pub fn extract_str_literal(literal: &str) -> Option<(String, bool)> {
let (literal, raw) = match literal.strip_prefix('r') {
Some(stripped) => (stripped.trim_matches('#'), true),
None => (literal, false),
Expand Down
2 changes: 1 addition & 1 deletion tests/ui-toml/allow_mixed_uninlined_format_args/uninlined_format_args.stderr
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ LL | println!("Hello {} is {:.*}", "x", local_i32, local_f64);
help: change this to
|
LL - println!("Hello {} is {:.*}", "x", local_i32, local_f64);
LL + println!("Hello {} is {local_f64:.local_i32$}", "x");
LL + println!("Hello x is {local_f64:.local_i32$}");
|

error: literal with an empty format string
Expand Down
26 changes: 20 additions & 6 deletions tests/ui/uninlined_format_args.fixed
View file
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ fn tester(fn_arg: i32) {
println!("{local_i32:<3}");
println!("{local_i32:#010x}");
println!("{local_f64:.1}");
println!("Hello {} is {:.*}", "x", local_i32, local_f64);
println!("Hello x is {local_f64:.local_i32$}");
println!("Hello {} is {:.*}", local_i32, 5, local_f64);
println!("Hello {} is {2:.*}", local_i32, 5, local_f64);
println!("{local_i32} {local_f64}");
Expand All @@ -83,12 +83,12 @@ fn tester(fn_arg: i32) {
println!("{local_i32} {local_f64}");
println!("{local_f64} {local_i32}");
println!("{local_f64} {local_i32} {local_f64} {local_i32}");
println!("{1} {0}", "str", local_i32);
println!("{local_i32} str");
println!("{local_i32}");
println!("{local_i32:width$}");
println!("{local_i32:width$}");
println!("{local_i32:.prec$}");
println!("{local_i32:.prec$}");
println!("{local_i32:0$}", width);
println!("{local_i32:w$}", w = width);
println!("{local_i32:.0$}", prec);
println!("{local_i32:.p$}", p = prec);
println!("{val:val$}");
println!("{val:val$}");
println!("{val:val$.val$}");
Expand Down Expand Up @@ -267,3 +267,17 @@ fn tester2() {
local_i32,
};
}

fn literals() {
let var = 5;
println!("foo");
println!("foo");
println!("{:var$}", "foo");
println!("foo");
println!("{0:1$}", "foo", 5);
println!("var {var} lit foo");
println!("var {var} lit foo");
println!("var foo lit foo");
println!("var foo lit foo {var},");
println!("var {0} lit {} {},", "foo", 5);
}
14 changes: 14 additions & 0 deletions tests/ui/uninlined_format_args.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -272,3 +272,17 @@ fn tester2() {
local_i32,
};
}

fn literals() {
let var = 5;
println!("{}", "foo");
println!("{:5}", "foo");
println!("{:var$}", "foo");
println!("{:-5}", "foo");
println!("{0:1$}", "foo", 5);
println!("var {} lit {}", var, "foo");
println!("var {1} lit {0}", "foo", var);
println!("var {} lit {0}", "foo");
println!("var {0} lit {} {},", "foo", var);
println!("var {0} lit {} {},", "foo", 5);
}
152 changes: 106 additions & 46 deletions tests/ui/uninlined_format_args.stderr
View file
Original file line number Diff line number Diff line change
Expand Up @@ -178,6 +178,18 @@ LL - println!("{:.1}", local_f64);
LL + println!("{local_f64:.1}");
|

error: variables can be used directly in the `format!` string
--> $DIR/uninlined_format_args.rs:64:5
|
LL | println!("Hello {} is {:.*}", "x", local_i32, local_f64);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
help: change this to
|
LL - println!("Hello {} is {:.*}", "x", local_i32, local_f64);
LL + println!("Hello x is {local_f64:.local_i32$}");
|

error: variables can be used directly in the `format!` string
--> $DIR/uninlined_format_args.rs:67:5
|
Expand Down Expand Up @@ -407,63 +419,27 @@ LL + println!("{local_f64} {local_i32} {local_f64} {local_i32}");
|

error: variables can be used directly in the `format!` string
--> $DIR/uninlined_format_args.rs:89:5
|
LL | println!("{v}", v = local_i32);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
help: change this to
|
LL - println!("{v}", v = local_i32);
LL + println!("{local_i32}");
|

error: variables can be used directly in the `format!` string
--> $DIR/uninlined_format_args.rs:90:5
|
LL | println!("{local_i32:0$}", width);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
help: change this to
|
LL - println!("{local_i32:0$}", width);
LL + println!("{local_i32:width$}");
|

error: variables can be used directly in the `format!` string
--> $DIR/uninlined_format_args.rs:91:5
--> $DIR/uninlined_format_args.rs:88:5
|
LL | println!("{local_i32:w$}", w = width);
LL | println!("{1} {0}", "str", local_i32);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
help: change this to
|
LL - println!("{local_i32:w$}", w = width);
LL + println!("{local_i32:width$}");
LL - println!("{1} {0}", "str", local_i32);
LL + println!("{local_i32} str");
|

error: variables can be used directly in the `format!` string
--> $DIR/uninlined_format_args.rs:92:5
|
LL | println!("{local_i32:.0$}", prec);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
help: change this to
|
LL - println!("{local_i32:.0$}", prec);
LL + println!("{local_i32:.prec$}");
|

error: variables can be used directly in the `format!` string
--> $DIR/uninlined_format_args.rs:93:5
--> $DIR/uninlined_format_args.rs:89:5
|
LL | println!("{local_i32:.p$}", p = prec);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
LL | println!("{v}", v = local_i32);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
help: change this to
|
LL - println!("{local_i32:.p$}", p = prec);
LL + println!("{local_i32:.prec$}");
LL - println!("{v}", v = local_i32);
LL + println!("{local_i32}");
|

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

error: aborting due to 71 previous errors
error: variables can be used directly in the `format!` string
--> $DIR/uninlined_format_args.rs:278:5
|
LL | println!("{}", "foo");
| ^^^^^^^^^^^^^^^^^^^^^
|
help: change this to
|
LL - println!("{}", "foo");
LL + println!("foo");
|

error: variables can be used directly in the `format!` string
--> $DIR/uninlined_format_args.rs:279:5
|
LL | println!("{:5}", "foo");
| ^^^^^^^^^^^^^^^^^^^^^^^
|
help: change this to
|
LL - println!("{:5}", "foo");
LL + println!("foo");
|

error: variables can be used directly in the `format!` string
--> $DIR/uninlined_format_args.rs:281:5
|
LL | println!("{:-5}", "foo");
| ^^^^^^^^^^^^^^^^^^^^^^^^
|
help: change this to
|
LL - println!("{:-5}", "foo");
LL + println!("foo");
|

error: variables can be used directly in the `format!` string
--> $DIR/uninlined_format_args.rs:283:5
|
LL | println!("var {} lit {}", var, "foo");
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
help: change this to
|
LL - println!("var {} lit {}", var, "foo");
LL + println!("var {var} lit foo");
|

error: variables can be used directly in the `format!` string
--> $DIR/uninlined_format_args.rs:284:5
|
LL | println!("var {1} lit {0}", "foo", var);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
help: change this to
|
LL - println!("var {1} lit {0}", "foo", var);
LL + println!("var {var} lit foo");
|

error: variables can be used directly in the `format!` string
--> $DIR/uninlined_format_args.rs:285:5
|
LL | println!("var {} lit {0}", "foo");
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
help: change this to
|
LL - println!("var {} lit {0}", "foo");
LL + println!("var foo lit foo");
|

error: variables can be used directly in the `format!` string
--> $DIR/uninlined_format_args.rs:286:5
|
LL | println!("var {0} lit {} {},", "foo", var);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
help: change this to
|
LL - println!("var {0} lit {} {},", "foo", var);
LL + println!("var foo lit foo {var},");
|

error: aborting due to 76 previous errors

0 comments on commit e50f944

Please sign in to comment.