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

5 files changed, 192 insertions, 132 deletions

diff --git a/crates/ide-assists/src/handlers/move_format_string_arg.rs b/crates/ide-assists/src/handlers/move_format_string_arg.rs
index 696fd50b5cb..54b5bee9b7b 100644
--- a/crates/ide-assists/src/handlers/move_format_string_arg.rs
+++ b/crates/ide-assists/src/handlers/move_format_string_arg.rs
@@ -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 syntax::{ast, AstToken, AstNode, NodeOrToken, SyntaxKind::COMMA, TextRange};
+use syntax::{ast, AstNode, AstToken, NodeOrToken, SyntaxKind::COMMA, TextRange};
 
 // Assist: move_format_string_arg
 //
 // 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() {
-//     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() {
-//     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 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()))?;
+pub(crate) fn move_format_string_arg(acc: &mut Assists, ctx: &AssistContext<'_>) -> Option<()> {
+    let fmt_string = ctx.find_token_at_offset::<ast::String>()?;
+    let tt = fmt_string.syntax().parent_ancestors().find_map(ast::TokenTree::cast)?;
 
+    let expanded_t = ast::String::cast(
+        ctx.sema.descend_into_macros_with_kind_preference(fmt_string.syntax().clone()),
+    )?;
     if !is_format_string(&expanded_t) {
         return None;
     }
 
-    let target = tt.syntax().text_range();
-    let extracted_args = parse_format_exprs(&t).ok()?;
-    let str_range = t.syntax().text_range();
-
-    let tokens =
-        tt.token_trees_and_tokens()
-            .filter_map(NodeOrToken::into_token)
-            .collect_vec();
-
-    acc.add(AssistId("move_format_string_arg", AssistKind::QuickFix), "Extract format args", target, |edit| {
-        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());
+    let (new_fmt, extracted_args) = parse_format_exprs(fmt_string.text()).ok()?;
+
+    acc.add(
+        AssistId("move_format_string_arg", AssistKind::QuickFix),
+        "Extract format args",
+        tt.syntax().text_range(),
+        |edit| {
+            let fmt_range = fmt_string.syntax().text_range();
+
+            // Replace old format string with new format string whose arguments have been extracted
+            edit.replace(fmt_range, new_fmt);
+
+            // Insert cursor at end of format string
+            edit.insert(fmt_range.end(), "$0");
+
+            // Extract existing arguments in macro
+            let tokens =
+                tt.token_trees_and_tokens().filter_map(NodeOrToken::into_token).collect_vec();
+
+            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
-            // we're going to insert the new arguments later
-            edit.delete(TextRange::new(format_string.text_range().end(), end_bracket.text_range().start()));
-        }
-
-        let mut existing_args = existing_args.into_iter();
-
-        // insert cursor at end of format string
-        edit.insert(str_range.end(), "$0");
-        let mut placeholder_idx = 1;
-        let mut args = String::new();
-
-        for (text, extracted_args) in extracted_args {
-            // remove expr from format string
-            edit.delete(text);
-
-            args.push_str(", ");
-
-            match extracted_args {
-                Arg::Expr(s) => {
-                    // insert arg
-                    args.push_str(&s);
-                },
-                Arg::Placeholder => {
-                    // try matching with existing argument
-                    match existing_args.next() {
-                        Some(ea) => {
-                            args.push_str(&ea);
-                        },
-                        None => {
-                            // insert placeholder
-                            args.push_str(&format!("${placeholder_idx}"));
-                            placeholder_idx += 1;
+
+            // Start building the new args
+            let mut existing_args = existing_args.into_iter();
+            let mut args = String::new();
+
+            let mut placeholder_idx = 1;
+
+            for extracted_args in extracted_args {
+                // remove expr from format string
+                args.push_str(", ");
+
+                match extracted_args {
+                    Arg::Expr(s) => {
+                        // insert arg
+                        args.push_str(&s);
+                    }
+                    Arg::Placeholder => {
+                        // try matching with existing argument
+                        match existing_args.next() {
+                            Some(ea) => {
+                                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(())
 }
@@ -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
     }
 
@@ -121,17 +154,20 @@ macro_rules! print {
     fn multiple_middle_arg() {
         check_assist(
             move_format_string_arg,
-            &add_macro_decl(r#"
+            &add_macro_decl(
+                r#"
 fn main() {
     print!("{} {x + 1:b} {}$0", y + 2, 2);
 }
-"#),
-
-            &add_macro_decl(r#"
+"#,
+            ),
+            &add_macro_decl(
+                r#"
 fn main() {
     print!("{} {:b} {}"$0, y + 2, x + 1, 2);
 }
-"#),
+"#,
+            ),
         );
     }
 
@@ -139,16 +175,20 @@ fn main() {
     fn single_arg() {
         check_assist(
             move_format_string_arg,
-            &add_macro_decl(r#"
+            &add_macro_decl(
+                r#"
 fn main() {
     print!("{obj.value:b}$0",);
 }
-"#),
-            &add_macro_decl(r#"
+"#,
+            ),
+            &add_macro_decl(
+                r#"
 fn main() {
     print!("{:b}"$0, obj.value);
 }
-"#),
+"#,
+            ),
         );
     }
 
@@ -156,17 +196,20 @@ fn main() {
     fn multiple_middle_placeholders_arg() {
         check_assist(
             move_format_string_arg,
-            &add_macro_decl(r#"
+            &add_macro_decl(
+                r#"
 fn main() {
     print!("{} {x + 1:b} {} {}$0", y + 2, 2);
 }
-"#),
-
-            &add_macro_decl(r#"
+"#,
+            ),
+            &add_macro_decl(
+                r#"
 fn main() {
     print!("{} {:b} {} {}"$0, y + 2, x + 1, 2, $1);
 }
-"#),
+"#,
+            ),
         );
     }
 
@@ -174,17 +217,20 @@ fn main() {
     fn multiple_trailing_args() {
         check_assist(
             move_format_string_arg,
-            &add_macro_decl(r#"
+            &add_macro_decl(
+                r#"
 fn main() {
     print!("{} {x + 1:b} {Struct(1, 2)}$0", 1);
 }
-"#),
-
-            &add_macro_decl(r#"
+"#,
+            ),
+            &add_macro_decl(
+                r#"
 fn main() {
     print!("{} {:b} {}"$0, 1, x + 1, Struct(1, 2));
 }
-"#),
+"#,
+            ),
         );
     }
 
@@ -192,18 +238,20 @@ fn main() {
     fn improper_commas() {
         check_assist(
             move_format_string_arg,
-            &add_macro_decl(r#"
+            &add_macro_decl(
+                r#"
 fn main() {
     print!("{} {x + 1:b} {Struct(1, 2)}$0", 1,);
 }
-"#),
-
-            &add_macro_decl(r#"
+"#,
+            ),
+            &add_macro_decl(
+                r#"
 fn main() {
     print!("{} {:b} {}"$0, 1, x + 1, Struct(1, 2));
 }
-"#),
+"#,
+            ),
         );
     }
-
 }
diff --git a/crates/ide-assists/src/lib.rs b/crates/ide-assists/src/lib.rs
index f881be9cf6d..812d22efbd7 100644
--- a/crates/ide-assists/src/lib.rs
+++ b/crates/ide-assists/src/lib.rs
@@ -255,6 +255,7 @@ mod handlers {
             merge_imports::merge_imports,
             merge_match_arms::merge_match_arms,
             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_guard_to_arm_body,
             move_module_to_file::move_module_to_file,
diff --git a/crates/ide-assists/src/tests/generated.rs b/crates/ide-assists/src/tests/generated.rs
index fa88abd6c55..3a696635afd 100644
--- a/crates/ide-assists/src/tests/generated.rs
+++ b/crates/ide-assists/src/tests/generated.rs
@@ -1596,13 +1596,27 @@ fn doctest_move_format_string_arg() {
     check_doc_test(
         "move_format_string_arg",
         r#####"
+macro_rules! format_args {
+    ($lit:literal $(tt:tt)*) => { 0 },
+}
+macro_rules! print {
+    ($($arg:tt)*) => (std::io::_print(format_args!($($arg)*)));
+}
+
 fn main() {
-    println!("{x + 1}$0");
+    print!("{x + 1}$0");
 }
 "#####,
         r#####"
+macro_rules! format_args {
+    ($lit:literal $(tt:tt)*) => { 0 },
+}
+macro_rules! print {
+    ($($arg:tt)*) => (std::io::_print(format_args!($($arg)*)));
+}
+
 fn main() {
-    println!("{a}", a$0 = x + 1);
+    print!("{}"$0, x + 1);
 }
 "#####,
     )
diff --git a/crates/ide-completion/src/completions/postfix/format_like.rs b/crates/ide-completion/src/completions/postfix/format_like.rs
index 89bfdac74d2..b43bdb9ab9d 100644
--- a/crates/ide-completion/src/completions/postfix/format_like.rs
+++ b/crates/ide-completion/src/completions/postfix/format_like.rs
@@ -16,8 +16,11 @@
 //
 // 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 syntax::ast::{self, AstToken};
+use ide_db::{
+    syntax_helpers::format_string_exprs::{parse_format_exprs, with_placeholders},
+    SnippetCap,
+};
+use syntax::{ast, AstToken};
 
 use crate::{
     completions::postfix::build_postfix_snippet_builder, context::CompletionContext, Completions,
@@ -48,10 +51,10 @@ pub(crate) fn add_format_like_completions(
         None => return,
     };
 
-    if let Ok((out, exprs)) = parse_format_exprs(receiver_text) {
-        let exprs = add_placeholders(exprs.map(|e| e.1)).collect_vec();
+    if let Ok((out, exprs)) = parse_format_exprs(receiver_text.text()) {
+        let exprs = with_placeholders(exprs);
         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);
         }
@@ -77,7 +80,7 @@ mod tests {
 
         for (kind, input, output) in test_vector {
             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(", "));
             assert_eq!(&snippet, output);
         }
diff --git a/crates/ide-db/src/syntax_helpers/format_string_exprs.rs b/crates/ide-db/src/syntax_helpers/format_string_exprs.rs
index b6b2eb268d3..e9006e06b11 100644
--- a/crates/ide-db/src/syntax_helpers/format_string_exprs.rs
+++ b/crates/ide-db/src/syntax_helpers/format_string_exprs.rs
@@ -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)]
 pub enum Arg {
     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;
-    args.map(move |a|
-        match a {
+    args.into_iter()
+        .map(move |a| match a {
             Arg::Expr(s) => s,
             Arg::Placeholder => {
                 let s = format!("${placeholder_id}");
                 placeholder_id += 1;
                 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")]));
  ```
 */
-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)]
     enum State {
         NotExpr,
@@ -49,9 +53,6 @@ pub fn parse_format_exprs(input: &ast::String) -> Result<Vec<(TextRange, Arg)>,
         FormatOpts,
     }
 
-    let start = input.syntax().text_range().start();
-
-    let mut expr_start = start;
     let mut current_expr = String::new();
     let mut state = State::NotExpr;
     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 }}".
     let mut inexpr_open_count = 0;
 
-    let mut chars = input.text().chars().zip(0u32..).peekable();
-    while let Some((chr, idx )) = chars.next() {
+    let mut chars = input.chars().peekable();
+    while let Some(chr) = chars.next() {
         match (state, chr) {
             (State::NotExpr, '{') => {
                 output.push(chr);
@@ -95,7 +96,7 @@ pub fn parse_format_exprs(input: &ast::String) -> Result<Vec<(TextRange, Arg)>,
             (State::MaybeExpr, '}') => {
                 // This is an empty sequence '{}'. Replace it with placeholder.
                 output.push(chr);
-                extracted_expressions.push((TextRange::empty(expr_start), Arg::Placeholder));
+                extracted_expressions.push(Arg::Placeholder);
                 state = State::NotExpr;
             }
             (State::MaybeExpr, _) => {
@@ -103,13 +104,12 @@ pub fn parse_format_exprs(input: &ast::String) -> Result<Vec<(TextRange, Arg)>,
                     current_expr.push('\\');
                 }
                 current_expr.push(chr);
-                expr_start = start.checked_add(idx.into()).ok_or(())?;
                 state = State::Expr;
             }
             (State::Expr, '}') => {
                 if inexpr_open_count == 0 {
                     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();
                     state = State::NotExpr;
                 } else {
@@ -118,7 +118,7 @@ pub fn parse_format_exprs(input: &ast::String) -> Result<Vec<(TextRange, Arg)>,
                     inexpr_open_count -= 1;
                 }
             }
-            (State::Expr, ':') if matches!(chars.peek(), Some((':', _))) => {
+            (State::Expr, ':') if matches!(chars.peek(), Some(':')) => {
                 // path separator
                 current_expr.push_str("::");
                 chars.next();
@@ -127,7 +127,7 @@ pub fn parse_format_exprs(input: &ast::String) -> Result<Vec<(TextRange, Arg)>,
                 if inexpr_open_count == 0 {
                     // We're outside of braces, thus assume that it's a specifier, like "{Some(value):?}"
                     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();
                     state = State::FormatOpts;
                 } else {
@@ -162,7 +162,7 @@ pub fn parse_format_exprs(input: &ast::String) -> Result<Vec<(TextRange, Arg)>,
         return Err(());
     }
 
-    Ok(extracted_expressions)
+    Ok((output, extracted_expressions))
 }
 
 #[cfg(test)]
@@ -171,17 +171,11 @@ mod tests {
     use expect_test::{expect, Expect};
 
     fn check(input: &str, expect: &Expect) {
-        let mut parser = FormatStrParser::new((*input).to_owned());
-        let outcome_repr = if parser.parse().is_ok() {
-            // Parsing should be OK, expected repr is "string; expr_1, expr_2".
-            if parser.extracted_expressions.is_empty() {
-                parser.output
-            } else {
-                format!("{}; {}", parser.output, parser.extracted_expressions.join(", "))
-            }
+        let (output, exprs) = parse_format_exprs(input).unwrap_or(("-".to_string(), vec![]));
+        let outcome_repr = if !exprs.is_empty() {
+            format!("{}; {}", output, with_placeholders(exprs).join(", "))
         } else {
-            // Parsing should fail, expected repr is "-".
-            "-".to_owned()
+            output
         };
 
         expect.assert_eq(&outcome_repr);