Auto merge of #13641 - DesmondWillowbrook:fix-move-format-string, r=Veykril

fix: format expression parsing edge-cases

- Handle positional arguments with formatting options (i.e. `{:b}`). Previously copied `:b` as an argument, producing broken code.

- Handle indexed positional arguments (`{0}`) ([reference](https://doc.rust-lang.org/std/fmt/#positional-parameters)). Previously copied over `0` as an argument.

Note: the assist also breaks when named arguments are used (`"{name}$0", name = 2 + 2` is converted to `"{}"$0, name`. I'm working on fix for that as well.
This commit is contained in:
bors 2022-11-19 12:46:29 +00:00
commit 2d9ed4fd48
2 changed files with 36 additions and 30 deletions

View file

@ -92,7 +92,7 @@ pub(crate) fn move_format_string_arg(acc: &mut Assists, ctx: &AssistContext<'_>)
NodeOrToken::Node(n) => { NodeOrToken::Node(n) => {
format_to!(current_arg, "{n}"); format_to!(current_arg, "{n}");
}, },
NodeOrToken::Token(t) if t.kind() == COMMA=> { NodeOrToken::Token(t) if t.kind() == COMMA => {
existing_args.push(current_arg.trim().into()); existing_args.push(current_arg.trim().into());
current_arg.clear(); current_arg.clear();
}, },
@ -238,14 +238,14 @@ fn main() {
&add_macro_decl( &add_macro_decl(
r#" r#"
fn main() { fn main() {
print!("{} {x + 1:b} {Struct(1, 2)}$0", 1); print!("{:b} {x + 1:b} {Struct(1, 2)}$0", 1);
} }
"#, "#,
), ),
&add_macro_decl( &add_macro_decl(
r#" r#"
fn main() { fn main() {
print!("{} {:b} {}"$0, 1, x + 1, Struct(1, 2)); print!("{:b} {:b} {}"$0, 1, x + 1, Struct(1, 2));
} }
"#, "#,
), ),

View file

@ -104,6 +104,11 @@ pub fn parse_format_exprs(input: &str) -> Result<(String, Vec<Arg>), ()> {
extracted_expressions.push(Arg::Placeholder); extracted_expressions.push(Arg::Placeholder);
state = State::NotArg; state = State::NotArg;
} }
(State::MaybeArg, ':') => {
output.push(chr);
extracted_expressions.push(Arg::Placeholder);
state = State::FormatOpts;
}
(State::MaybeArg, _) => { (State::MaybeArg, _) => {
if matches!(chr, '\\' | '$') { if matches!(chr, '\\' | '$') {
current_expr.push('\\'); current_expr.push('\\');
@ -118,44 +123,41 @@ pub fn parse_format_exprs(input: &str) -> Result<(String, Vec<Arg>), ()> {
state = State::Expr; state = State::Expr;
} }
} }
(State::Ident | State::Expr, '}') => {
if inexpr_open_count == 0 {
output.push(chr);
if matches!(state, State::Expr) {
extracted_expressions.push(Arg::Expr(current_expr.trim().into()));
} else {
extracted_expressions.push(Arg::Ident(current_expr.trim().into()));
}
current_expr = String::new();
state = State::NotArg;
} else {
// We're closing one brace met before inside of the expression.
current_expr.push(chr);
inexpr_open_count -= 1;
}
}
(State::Ident | State::Expr, ':') if matches!(chars.peek(), Some(':')) => { (State::Ident | State::Expr, ':') if matches!(chars.peek(), Some(':')) => {
// path separator // path separator
state = State::Expr; state = State::Expr;
current_expr.push_str("::"); current_expr.push_str("::");
chars.next(); chars.next();
} }
(State::Ident | State::Expr, ':') => { (State::Ident | State::Expr, ':' | '}') => {
if inexpr_open_count == 0 { if inexpr_open_count == 0 {
// We're outside of braces, thus assume that it's a specifier, like "{Some(value):?}" let trimmed = current_expr.trim();
output.push(chr);
if matches!(state, State::Expr) { // if the expression consists of a single number, like "0" or "12", it can refer to
extracted_expressions.push(Arg::Expr(current_expr.trim().into())); // format args in the order they are specified.
// see: https://doc.rust-lang.org/std/fmt/#positional-parameters
if trimmed.chars().fold(true, |only_num, c| c.is_ascii_digit() && only_num) {
output.push_str(trimmed);
} else if matches!(state, State::Expr) {
extracted_expressions.push(Arg::Expr(trimmed.into()));
} else { } else {
extracted_expressions.push(Arg::Ident(current_expr.trim().into())); extracted_expressions.push(Arg::Ident(trimmed.into()));
} }
current_expr = String::new(); output.push(chr);
state = State::FormatOpts; current_expr.clear();
} else { state = if chr == ':' {
State::FormatOpts
} else if chr == '}' {
State::NotArg
} else {
unreachable!()
};
} else if chr == '}' {
// We're closing one brace met before inside of the expression.
current_expr.push(chr);
inexpr_open_count -= 1;
} else if chr == ':' {
// We're inside of braced expression, assume that it's a struct field name/value delimiter. // We're inside of braced expression, assume that it's a struct field name/value delimiter.
current_expr.push(chr); current_expr.push(chr);
} }
@ -219,6 +221,10 @@ mod tests {
("{expr} is {2 + 2}", expect![["{} is {}; expr, 2 + 2"]]), ("{expr} is {2 + 2}", expect![["{} is {}; expr, 2 + 2"]]),
("{expr:?}", expect![["{:?}; expr"]]), ("{expr:?}", expect![["{:?}; expr"]]),
("{expr:1$}", expect![[r"{:1\$}; expr"]]), ("{expr:1$}", expect![[r"{:1\$}; expr"]]),
("{:1$}", expect![[r"{:1\$}; $1"]]),
("{:>padding$}", expect![[r"{:>padding\$}; $1"]]),
("{}, {}, {0}", expect![[r"{}, {}, {0}; $1, $2"]]),
("{}, {}, {0:b}", expect![[r"{}, {}, {0:b}; $1, $2"]]),
("{$0}", expect![[r"{}; \$0"]]), ("{$0}", expect![[r"{}; \$0"]]),
("{malformed", expect![["-"]]), ("{malformed", expect![["-"]]),
("malformed}", expect![["-"]]), ("malformed}", expect![["-"]]),