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 May 4, 2023
1 parent f9c1d15 commit 86cfa0b
Show file tree
Hide file tree
Showing 6 changed files with 171 additions and 62 deletions.
37 changes: 29 additions & 8 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_utils::diagnostics::{span_lint_and_sugg, span_lint_and_then};
use clippy_utils::is_diag_trait_item;
Expand All @@ -10,6 +11,7 @@ use clippy_utils::source::snippet_opt;
use clippy_utils::ty::{implements_trait, is_type_lang_item};
use if_chain::if_chain;
use itertools::Itertools;
use rustc_ast::token::LitKind;
use rustc_ast::{
FormatArgPosition, FormatArgPositionKind, FormatArgsPiece, FormatArgumentKind, FormatCount, FormatOptions,
FormatPlaceholder, FormatTrait,
Expand Down Expand Up @@ -316,8 +318,8 @@ fn check_uninlined_args(
// 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 format_arg_positions(args) {
if !check_one_arg(args, pos, usage, &mut fixes, ignore_mixed) {
for (pos, usage, pl) in format_arg_positions(args) {
if !check_one_arg(cx, args, pos, usage, pl, &mut fixes, ignore_mixed) {
return;
}
}
Expand Down Expand Up @@ -351,14 +353,18 @@ fn check_uninlined_args(
}

fn check_one_arg(
cx: &LateContext<'_>,
args: &rustc_ast::FormatArgs,
pos: &FormatArgPosition,
usage: FormatParamUsage,
placeholder: Option<&FormatPlaceholder>,
fixes: &mut Vec<(Span, String)>,
ignore_mixed: bool,
) -> bool {
let index = pos.index.unwrap();
let arg = &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) = &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 @@ -375,6 +381,21 @@ fn check_one_arg(
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(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(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 @@ -457,17 +478,17 @@ fn check_to_string_in_format_args(cx: &LateContext<'_>, name: Symbol, value: &Ex

fn format_arg_positions(
format_args: &rustc_ast::FormatArgs,
) -> impl Iterator<Item = (&FormatArgPosition, FormatParamUsage)> {
) -> impl Iterator<Item = (&FormatArgPosition, FormatParamUsage, Option<&FormatPlaceholder>)> {
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 @@ -479,7 +500,7 @@ fn format_arg_positions(
/// Returns true if the format argument at `index` is referred to by multiple format params
fn is_aliased(format_args: &rustc_ast::FormatArgs, index: usize) -> bool {
format_arg_positions(format_args)
.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 @@ -539,7 +539,7 @@ fn check_literal(cx: &LateContext<'_>, format_args: &FormatArgs, name: &str) {
/// `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 @@ -20,7 +20,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 @@ -54,7 +54,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 @@ -78,12 +78,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 @@ -262,3 +262,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 @@ -267,3 +267,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 @@ -177,6 +177,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:59: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:62:5
|
Expand Down Expand Up @@ -406,63 +418,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:84: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:85: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:86:5
--> $DIR/uninlined_format_args.rs:83: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:87: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:88:5
--> $DIR/uninlined_format_args.rs:84: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 @@ -844,5 +820,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:273: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:274: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:276: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:278: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:279: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:280: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:281: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 86cfa0b

Please sign in to comment.