new lint: move_format_string_arg

The name might need some improving.

extract format_like's parser to it's own module in ide-db

reworked the parser's API to be more direct

added assist to extract expressions in format args
This commit is contained in:
Kartavya Vashishtha 2022-09-10 20:12:47 +05:30
parent 2584d48508
commit cc7200891b
No known key found for this signature in database
GPG key ID: A50012C2324E5DF0
5 changed files with 185 additions and 125 deletions

View file

@ -1,100 +1,133 @@
use ide_db::{syntax_helpers::{format_string::is_format_string, format_string_exprs::{parse_format_exprs, Arg}}, assists::{AssistId, AssistKind}}; use crate::{AssistContext, Assists};
use ide_db::{
assists::{AssistId, AssistKind},
syntax_helpers::{
format_string::is_format_string,
format_string_exprs::{parse_format_exprs, Arg},
},
};
use itertools::Itertools; use itertools::Itertools;
use syntax::{ast, AstToken, AstNode, NodeOrToken, SyntaxKind::COMMA, TextRange}; use syntax::{ast, AstNode, AstToken, NodeOrToken, SyntaxKind::COMMA, TextRange};
// Assist: move_format_string_arg // Assist: move_format_string_arg
// //
// Move an expression out of a format string. // Move an expression out of a format string.
// //
// ``` // ```
// macro_rules! format_args {
// ($lit:literal $(tt:tt)*) => { 0 },
// }
// macro_rules! print {
// ($($arg:tt)*) => (std::io::_print(format_args!($($arg)*)));
// }
//
// fn main() { // fn main() {
// println!("{x + 1}$0"); // print!("{x + 1}$0");
// } // }
// ``` // ```
// -> // ->
// ``` // ```
// macro_rules! format_args {
// ($lit:literal $(tt:tt)*) => { 0 },
// }
// macro_rules! print {
// ($($arg:tt)*) => (std::io::_print(format_args!($($arg)*)));
// }
//
// fn main() { // fn main() {
// println!("{a}", a$0 = x + 1); // print!("{}"$0, x + 1);
// } // }
// ``` // ```
use crate::{AssistContext, /* AssistId, AssistKind, */ Assists}; pub(crate) fn move_format_string_arg(acc: &mut Assists, ctx: &AssistContext<'_>) -> Option<()> {
let fmt_string = ctx.find_token_at_offset::<ast::String>()?;
pub(crate) fn move_format_string_arg (acc: &mut Assists, ctx: &AssistContext<'_>) -> Option<()> { let tt = fmt_string.syntax().parent_ancestors().find_map(ast::TokenTree::cast)?;
let t = ctx.find_token_at_offset::<ast::String>()?;
let tt = t.syntax().parent_ancestors().find_map(ast::TokenTree::cast)?;
let expanded_t = ast::String::cast(ctx.sema.descend_into_macros_with_kind_preference(t.syntax().clone()))?;
let expanded_t = ast::String::cast(
ctx.sema.descend_into_macros_with_kind_preference(fmt_string.syntax().clone()),
)?;
if !is_format_string(&expanded_t) { if !is_format_string(&expanded_t) {
return None; return None;
} }
let target = tt.syntax().text_range(); let (new_fmt, extracted_args) = parse_format_exprs(fmt_string.text()).ok()?;
let extracted_args = parse_format_exprs(&t).ok()?;
let str_range = t.syntax().text_range();
let tokens = acc.add(
tt.token_trees_and_tokens() AssistId("move_format_string_arg", AssistKind::QuickFix),
.filter_map(NodeOrToken::into_token) "Extract format args",
.collect_vec(); tt.syntax().text_range(),
|edit| {
let fmt_range = fmt_string.syntax().text_range();
acc.add(AssistId("move_format_string_arg", AssistKind::QuickFix), "Extract format args", target, |edit| { // Replace old format string with new format string whose arguments have been extracted
let mut existing_args: Vec<String> = vec![]; edit.replace(fmt_range, new_fmt);
let mut current_arg = String::new();
if let [_opening_bracket, format_string, _args_start_comma, tokens @ .., end_bracket] = tokens.as_slice() { // Insert cursor at end of format string
for t in tokens { edit.insert(fmt_range.end(), "$0");
if t.kind() == COMMA {
existing_args.push(current_arg.trim().into()); // Extract existing arguments in macro
current_arg.clear(); let tokens =
} else { tt.token_trees_and_tokens().filter_map(NodeOrToken::into_token).collect_vec();
current_arg.push_str(t.text());
let mut existing_args: Vec<String> = vec![];
let mut current_arg = String::new();
if let [_opening_bracket, format_string, _args_start_comma, tokens @ .., end_bracket] =
tokens.as_slice()
{
for t in tokens {
if t.kind() == COMMA {
existing_args.push(current_arg.trim().into());
current_arg.clear();
} else {
current_arg.push_str(t.text());
}
} }
existing_args.push(current_arg.trim().into());
// delete everything after the format string till end bracket
// we're going to insert the new arguments later
edit.delete(TextRange::new(
format_string.text_range().end(),
end_bracket.text_range().start(),
));
} }
existing_args.push(current_arg.trim().into());
// delete everything after the format string to the end bracket // Start building the new args
// we're going to insert the new arguments later let mut existing_args = existing_args.into_iter();
edit.delete(TextRange::new(format_string.text_range().end(), end_bracket.text_range().start())); let mut args = String::new();
}
let mut existing_args = existing_args.into_iter(); let mut placeholder_idx = 1;
// insert cursor at end of format string for extracted_args in extracted_args {
edit.insert(str_range.end(), "$0"); // remove expr from format string
let mut placeholder_idx = 1; args.push_str(", ");
let mut args = String::new();
for (text, extracted_args) in extracted_args { match extracted_args {
// remove expr from format string Arg::Expr(s) => {
edit.delete(text); // insert arg
args.push_str(&s);
args.push_str(", "); }
Arg::Placeholder => {
match extracted_args { // try matching with existing argument
Arg::Expr(s) => { match existing_args.next() {
// insert arg Some(ea) => {
args.push_str(&s); args.push_str(&ea);
}, }
Arg::Placeholder => { None => {
// try matching with existing argument // insert placeholder
match existing_args.next() { args.push_str(&format!("${placeholder_idx}"));
Some(ea) => { placeholder_idx += 1;
args.push_str(&ea); }
},
None => {
// insert placeholder
args.push_str(&format!("${placeholder_idx}"));
placeholder_idx += 1;
} }
} }
} }
} }
}
edit.insert(str_range.end(), args); // Insert new args
}); edit.insert(fmt_range.end(), args);
},
);
Some(()) Some(())
} }
@ -113,7 +146,7 @@ macro_rules! print {
} }
"#; "#;
fn add_macro_decl (s: &'static str) -> String { fn add_macro_decl(s: &'static str) -> String {
MACRO_DECL.to_string() + s MACRO_DECL.to_string() + s
} }
@ -121,17 +154,20 @@ macro_rules! print {
fn multiple_middle_arg() { fn multiple_middle_arg() {
check_assist( check_assist(
move_format_string_arg, move_format_string_arg,
&add_macro_decl(r#" &add_macro_decl(
r#"
fn main() { fn main() {
print!("{} {x + 1:b} {}$0", y + 2, 2); print!("{} {x + 1:b} {}$0", y + 2, 2);
} }
"#), "#,
),
&add_macro_decl(r#" &add_macro_decl(
r#"
fn main() { fn main() {
print!("{} {:b} {}"$0, y + 2, x + 1, 2); print!("{} {:b} {}"$0, y + 2, x + 1, 2);
} }
"#), "#,
),
); );
} }
@ -139,16 +175,20 @@ fn main() {
fn single_arg() { fn single_arg() {
check_assist( check_assist(
move_format_string_arg, move_format_string_arg,
&add_macro_decl(r#" &add_macro_decl(
r#"
fn main() { fn main() {
print!("{obj.value:b}$0",); print!("{obj.value:b}$0",);
} }
"#), "#,
&add_macro_decl(r#" ),
&add_macro_decl(
r#"
fn main() { fn main() {
print!("{:b}"$0, obj.value); print!("{:b}"$0, obj.value);
} }
"#), "#,
),
); );
} }
@ -156,17 +196,20 @@ fn main() {
fn multiple_middle_placeholders_arg() { fn multiple_middle_placeholders_arg() {
check_assist( check_assist(
move_format_string_arg, move_format_string_arg,
&add_macro_decl(r#" &add_macro_decl(
r#"
fn main() { fn main() {
print!("{} {x + 1:b} {} {}$0", y + 2, 2); print!("{} {x + 1:b} {} {}$0", y + 2, 2);
} }
"#), "#,
),
&add_macro_decl(r#" &add_macro_decl(
r#"
fn main() { fn main() {
print!("{} {:b} {} {}"$0, y + 2, x + 1, 2, $1); print!("{} {:b} {} {}"$0, y + 2, x + 1, 2, $1);
} }
"#), "#,
),
); );
} }
@ -174,17 +217,20 @@ fn main() {
fn multiple_trailing_args() { fn multiple_trailing_args() {
check_assist( check_assist(
move_format_string_arg, move_format_string_arg,
&add_macro_decl(r#" &add_macro_decl(
r#"
fn main() { fn main() {
print!("{} {x + 1:b} {Struct(1, 2)}$0", 1); print!("{} {x + 1:b} {Struct(1, 2)}$0", 1);
} }
"#), "#,
),
&add_macro_decl(r#" &add_macro_decl(
r#"
fn main() { fn main() {
print!("{} {:b} {}"$0, 1, x + 1, Struct(1, 2)); print!("{} {:b} {}"$0, 1, x + 1, Struct(1, 2));
} }
"#), "#,
),
); );
} }
@ -192,18 +238,20 @@ fn main() {
fn improper_commas() { fn improper_commas() {
check_assist( check_assist(
move_format_string_arg, move_format_string_arg,
&add_macro_decl(r#" &add_macro_decl(
r#"
fn main() { fn main() {
print!("{} {x + 1:b} {Struct(1, 2)}$0", 1,); print!("{} {x + 1:b} {Struct(1, 2)}$0", 1,);
} }
"#), "#,
),
&add_macro_decl(r#" &add_macro_decl(
r#"
fn main() { fn main() {
print!("{} {:b} {}"$0, 1, x + 1, Struct(1, 2)); print!("{} {:b} {}"$0, 1, x + 1, Struct(1, 2));
} }
"#), "#,
),
); );
} }
} }

View file

@ -255,6 +255,7 @@ mod handlers {
merge_imports::merge_imports, merge_imports::merge_imports,
merge_match_arms::merge_match_arms, merge_match_arms::merge_match_arms,
move_bounds::move_bounds_to_where_clause, move_bounds::move_bounds_to_where_clause,
move_format_string_arg::move_format_string_arg,
move_guard::move_arm_cond_to_match_guard, move_guard::move_arm_cond_to_match_guard,
move_guard::move_guard_to_arm_body, move_guard::move_guard_to_arm_body,
move_module_to_file::move_module_to_file, move_module_to_file::move_module_to_file,

View file

@ -1596,13 +1596,27 @@ fn doctest_move_format_string_arg() {
check_doc_test( check_doc_test(
"move_format_string_arg", "move_format_string_arg",
r#####" r#####"
macro_rules! format_args {
($lit:literal $(tt:tt)*) => { 0 },
}
macro_rules! print {
($($arg:tt)*) => (std::io::_print(format_args!($($arg)*)));
}
fn main() { fn main() {
println!("{x + 1}$0"); print!("{x + 1}$0");
} }
"#####, "#####,
r#####" r#####"
macro_rules! format_args {
($lit:literal $(tt:tt)*) => { 0 },
}
macro_rules! print {
($($arg:tt)*) => (std::io::_print(format_args!($($arg)*)));
}
fn main() { fn main() {
println!("{a}", a$0 = x + 1); print!("{}"$0, x + 1);
} }
"#####, "#####,
) )

View file

@ -16,8 +16,11 @@
// //
// image::https://user-images.githubusercontent.com/48062697/113020656-b560f500-917a-11eb-87de-02991f61beb8.gif[] // image::https://user-images.githubusercontent.com/48062697/113020656-b560f500-917a-11eb-87de-02991f61beb8.gif[]
use ide_db::{syntax_helpers::format_string_exprs::{parse_format_exprs, add_placeholders}, SnippetCap}; use ide_db::{
use syntax::ast::{self, AstToken}; syntax_helpers::format_string_exprs::{parse_format_exprs, with_placeholders},
SnippetCap,
};
use syntax::{ast, AstToken};
use crate::{ use crate::{
completions::postfix::build_postfix_snippet_builder, context::CompletionContext, Completions, completions::postfix::build_postfix_snippet_builder, context::CompletionContext, Completions,
@ -48,10 +51,10 @@ pub(crate) fn add_format_like_completions(
None => return, None => return,
}; };
if let Ok((out, exprs)) = parse_format_exprs(receiver_text) { if let Ok((out, exprs)) = parse_format_exprs(receiver_text.text()) {
let exprs = add_placeholders(exprs.map(|e| e.1)).collect_vec(); let exprs = with_placeholders(exprs);
for (label, macro_name) in KINDS { for (label, macro_name) in KINDS {
let snippet = format!(r#"{}("{}", {})"#, macro_name, out, exprs.join(", ")); let snippet = format!(r#"{}({}, {})"#, macro_name, out, exprs.join(", "));
postfix_snippet(label, macro_name, &snippet).add_to(acc); postfix_snippet(label, macro_name, &snippet).add_to(acc);
} }
@ -77,7 +80,7 @@ mod tests {
for (kind, input, output) in test_vector { for (kind, input, output) in test_vector {
let (parsed_string, exprs) = parse_format_exprs(input).unwrap(); let (parsed_string, exprs) = parse_format_exprs(input).unwrap();
let exprs = add_placeholders(exprs.map(|e| e.1)).collect_vec();; let exprs = with_placeholders(exprs);
let snippet = format!(r#"{}("{}", {})"#, kind, parsed_string, exprs.join(", ")); let snippet = format!(r#"{}("{}", {})"#, kind, parsed_string, exprs.join(", "));
assert_eq!(&snippet, output); assert_eq!(&snippet, output);
} }

View file

@ -1,9 +1,13 @@
use syntax::{ast, TextRange, AstToken}; //! Tools to work with expressions present in format string literals for the `format_args!` family of macros.
//! Primarily meant for assists and completions.
/// Enum for represenging extraced format string args.
/// Can either be extracted expressions (which includes identifiers),
/// or placeholders `{}`.
#[derive(Debug)] #[derive(Debug)]
pub enum Arg { pub enum Arg {
Placeholder, Placeholder,
Expr(String) Expr(String),
} }
/** /**
@ -13,18 +17,18 @@ pub enum Arg {
``` ```
*/ */
pub fn add_placeholders (args: impl Iterator<Item = Arg>) -> impl Iterator<Item = String> { pub fn with_placeholders(args: Vec<Arg>) -> Vec<String> {
let mut placeholder_id = 1; let mut placeholder_id = 1;
args.map(move |a| args.into_iter()
match a { .map(move |a| match a {
Arg::Expr(s) => s, Arg::Expr(s) => s,
Arg::Placeholder => { Arg::Placeholder => {
let s = format!("${placeholder_id}"); let s = format!("${placeholder_id}");
placeholder_id += 1; placeholder_id += 1;
s s
} }
} })
) .collect()
} }
/** /**
@ -39,7 +43,7 @@ pub fn add_placeholders (args: impl Iterator<Item = Arg>) -> impl Iterator<Item
assert_eq!(parse("{expr} {} {expr} ").unwrap(), ("{} {} {}", vec![Arg::Expr("expr"), Arg::Placeholder, Arg::Expr("expr")])); assert_eq!(parse("{expr} {} {expr} ").unwrap(), ("{} {} {}", vec![Arg::Expr("expr"), Arg::Placeholder, Arg::Expr("expr")]));
``` ```
*/ */
pub fn parse_format_exprs(input: &ast::String) -> Result<Vec<(TextRange, Arg)>, ()> { pub fn parse_format_exprs(input: &str) -> Result<(String, Vec<Arg>), ()> {
#[derive(Debug, Clone, Copy, PartialEq)] #[derive(Debug, Clone, Copy, PartialEq)]
enum State { enum State {
NotExpr, NotExpr,
@ -49,9 +53,6 @@ pub fn parse_format_exprs(input: &ast::String) -> Result<Vec<(TextRange, Arg)>,
FormatOpts, FormatOpts,
} }
let start = input.syntax().text_range().start();
let mut expr_start = start;
let mut current_expr = String::new(); let mut current_expr = String::new();
let mut state = State::NotExpr; let mut state = State::NotExpr;
let mut extracted_expressions = Vec::new(); let mut extracted_expressions = Vec::new();
@ -62,8 +63,8 @@ pub fn parse_format_exprs(input: &ast::String) -> Result<Vec<(TextRange, Arg)>,
// "{MyStruct { val_a: 0, val_b: 1 }}". // "{MyStruct { val_a: 0, val_b: 1 }}".
let mut inexpr_open_count = 0; let mut inexpr_open_count = 0;
let mut chars = input.text().chars().zip(0u32..).peekable(); let mut chars = input.chars().peekable();
while let Some((chr, idx )) = chars.next() { while let Some(chr) = chars.next() {
match (state, chr) { match (state, chr) {
(State::NotExpr, '{') => { (State::NotExpr, '{') => {
output.push(chr); output.push(chr);
@ -95,7 +96,7 @@ pub fn parse_format_exprs(input: &ast::String) -> Result<Vec<(TextRange, Arg)>,
(State::MaybeExpr, '}') => { (State::MaybeExpr, '}') => {
// This is an empty sequence '{}'. Replace it with placeholder. // This is an empty sequence '{}'. Replace it with placeholder.
output.push(chr); output.push(chr);
extracted_expressions.push((TextRange::empty(expr_start), Arg::Placeholder)); extracted_expressions.push(Arg::Placeholder);
state = State::NotExpr; state = State::NotExpr;
} }
(State::MaybeExpr, _) => { (State::MaybeExpr, _) => {
@ -103,13 +104,12 @@ pub fn parse_format_exprs(input: &ast::String) -> Result<Vec<(TextRange, Arg)>,
current_expr.push('\\'); current_expr.push('\\');
} }
current_expr.push(chr); current_expr.push(chr);
expr_start = start.checked_add(idx.into()).ok_or(())?;
state = State::Expr; state = State::Expr;
} }
(State::Expr, '}') => { (State::Expr, '}') => {
if inexpr_open_count == 0 { if inexpr_open_count == 0 {
output.push(chr); output.push(chr);
extracted_expressions.push((TextRange::new(expr_start, start.checked_add(idx.into()).ok_or(())?), Arg::Expr(current_expr.trim().into()))); extracted_expressions.push(Arg::Expr(current_expr.trim().into()));
current_expr = String::new(); current_expr = String::new();
state = State::NotExpr; state = State::NotExpr;
} else { } else {
@ -118,7 +118,7 @@ pub fn parse_format_exprs(input: &ast::String) -> Result<Vec<(TextRange, Arg)>,
inexpr_open_count -= 1; inexpr_open_count -= 1;
} }
} }
(State::Expr, ':') if matches!(chars.peek(), Some((':', _))) => { (State::Expr, ':') if matches!(chars.peek(), Some(':')) => {
// path separator // path separator
current_expr.push_str("::"); current_expr.push_str("::");
chars.next(); chars.next();
@ -127,7 +127,7 @@ pub fn parse_format_exprs(input: &ast::String) -> Result<Vec<(TextRange, Arg)>,
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):?}" // We're outside of braces, thus assume that it's a specifier, like "{Some(value):?}"
output.push(chr); output.push(chr);
extracted_expressions.push((TextRange::new(expr_start, start.checked_add(idx.into()).ok_or(())?), Arg::Expr(current_expr.trim().into()))); extracted_expressions.push(Arg::Expr(current_expr.trim().into()));
current_expr = String::new(); current_expr = String::new();
state = State::FormatOpts; state = State::FormatOpts;
} else { } else {
@ -162,7 +162,7 @@ pub fn parse_format_exprs(input: &ast::String) -> Result<Vec<(TextRange, Arg)>,
return Err(()); return Err(());
} }
Ok(extracted_expressions) Ok((output, extracted_expressions))
} }
#[cfg(test)] #[cfg(test)]
@ -171,17 +171,11 @@ mod tests {
use expect_test::{expect, Expect}; use expect_test::{expect, Expect};
fn check(input: &str, expect: &Expect) { fn check(input: &str, expect: &Expect) {
let mut parser = FormatStrParser::new((*input).to_owned()); let (output, exprs) = parse_format_exprs(input).unwrap_or(("-".to_string(), vec![]));
let outcome_repr = if parser.parse().is_ok() { let outcome_repr = if !exprs.is_empty() {
// Parsing should be OK, expected repr is "string; expr_1, expr_2". format!("{}; {}", output, with_placeholders(exprs).join(", "))
if parser.extracted_expressions.is_empty() {
parser.output
} else {
format!("{}; {}", parser.output, parser.extracted_expressions.join(", "))
}
} else { } else {
// Parsing should fail, expected repr is "-". output
"-".to_owned()
}; };
expect.assert_eq(&outcome_repr); expect.assert_eq(&outcome_repr);