Auto merge of rust-lang#13684 - unvalley:extract-expressions-from-format-string, r=Veykril

feat: extract_expressions_from_format_string

closes rust-lang#13640
- rename to `extract_expressions_from_format_string`
- leave identifier from format string
	- but this is from rustc version 1.65.0
	- Should I add flag or something?

Note: the assist behaves below cases for now. I'll create an issue for these.
```rs
let var = 1 + 1;
// ok
format!("{var} {1+1}");   // → format!("{var} {}", 1+1);
format!("{var:?} {1+1}"); // → format!("{var:?} {}", 1 + 1);
format!("{var} {var} {1+1}"); // → format!("{var} {var} {}", 1 + 1);

// breaks (need to handle minimum width by postfix`$`)
format!("{var:width$} {1+1}"); // → format!("{var:width\$} {}", 1+1);
format!("{var:.prec$} {1+1}"); // → format!("{var:.prec\$} {}", 1+1);
format!("Hello {:1$}! {1+1}", "x" 5); // → format("Hello {:1\$}! {}", "x", 1+1);
format!("Hello {:width$}! {1+1}", "x", width = 5); // → println!("Hello {:width\$}! {}", "x", 1+1);
```

https://user-images.githubusercontent.com/38400669/204344911-f1f8fbd2-706d-414e-b1ab-d309376efb9b.mov

Author: bors

crates/ide-assists/src/handlers/move_format_string_arg.rs renamed to crates/ide-assists/src/handlers/extract_expressions_from_format_string.rs

@@ -10,7 +10,7 @@ use itertools::Itertools;
 use stdx::format_to;
 use syntax::{ast, AstNode, AstToken, NodeOrToken, SyntaxKind::COMMA, TextRange};
 
-// Assist: move_format_string_arg
+// Assist: extract_expressions_from_format_string
 //
 // Move an expression out of a format string.
 //
@@ -23,7 +23,7 @@ use syntax::{ast, AstNode, AstToken, NodeOrToken, SyntaxKind::COMMA, TextRange};
 // }
 //
 // fn main() {
-//     print!("{x + 1}$0");
+//     print!("{var} {x + 1}$0");
 // }
 // ```
 // ->
@@ -36,11 +36,14 @@ use syntax::{ast, AstNode, AstToken, NodeOrToken, SyntaxKind::COMMA, TextRange};
 // }
 //
 // fn main() {
-//     print!("{}"$0, x + 1);
+//     print!("{var} {}"$0, x + 1);
 // }
 // ```
 
-pub(crate) fn move_format_string_arg(acc: &mut Assists, ctx: &AssistContext<'_>) -> Option<()> {
+pub(crate) fn extract_expressions_from_format_string(
+    acc: &mut Assists,
+    ctx: &AssistContext<'_>,
+) -> Option<()> {
     let fmt_string = ctx.find_token_at_offset::<ast::String>()?;
     let tt = fmt_string.syntax().parent().and_then(ast::TokenTree::cast)?;
 
@@ -58,15 +61,15 @@ pub(crate) fn move_format_string_arg(acc: &mut Assists, ctx: &AssistContext<'_>)
 
     acc.add(
         AssistId(
-            "move_format_string_arg",
+            "extract_expressions_from_format_string",
             // if there aren't any expressions, then make the assist a RefactorExtract
             if extracted_args.iter().filter(|f| matches!(f, Arg::Expr(_))).count() == 0 {
                 AssistKind::RefactorExtract
             } else {
                 AssistKind::QuickFix
             },
         ),
-        "Extract format args",
+        "Extract format expressions",
         tt.syntax().text_range(),
         |edit| {
             let fmt_range = fmt_string.syntax().text_range();
@@ -118,15 +121,14 @@ pub(crate) fn move_format_string_arg(acc: &mut Assists, ctx: &AssistContext<'_>)
             let mut placeholder_idx = 1;
 
             for extracted_args in extracted_args {
-                // remove expr from format string
-                args.push_str(", ");
-
                 match extracted_args {
-                    Arg::Ident(s) | Arg::Expr(s) => {
+                    Arg::Expr(s)=> {
+                        args.push_str(", ");
                         // insert arg
                         args.push_str(&s);
                     }
                     Arg::Placeholder => {
+                        args.push_str(", ");
                         // try matching with existing argument
                         match existing_args.next() {
                             Some(ea) => {
@@ -139,6 +141,7 @@ pub(crate) fn move_format_string_arg(acc: &mut Assists, ctx: &AssistContext<'_>)
                             }
                         }
                     }
+                    Arg::Ident(_s) => (),
                 }
             }
 
@@ -171,7 +174,7 @@ macro_rules! print {
     #[test]
     fn multiple_middle_arg() {
         check_assist(
-            move_format_string_arg,
+            extract_expressions_from_format_string,
             &add_macro_decl(
                 r#"
 fn main() {
@@ -192,7 +195,7 @@ fn main() {
     #[test]
     fn single_arg() {
         check_assist(
-            move_format_string_arg,
+            extract_expressions_from_format_string,
             &add_macro_decl(
                 r#"
 fn main() {
@@ -213,7 +216,7 @@ fn main() {
     #[test]
     fn multiple_middle_placeholders_arg() {
         check_assist(
-            move_format_string_arg,
+            extract_expressions_from_format_string,
             &add_macro_decl(
                 r#"
 fn main() {
@@ -234,7 +237,7 @@ fn main() {
     #[test]
     fn multiple_trailing_args() {
         check_assist(
-            move_format_string_arg,
+            extract_expressions_from_format_string,
             &add_macro_decl(
                 r#"
 fn main() {
@@ -255,7 +258,7 @@ fn main() {
     #[test]
     fn improper_commas() {
         check_assist(
-            move_format_string_arg,
+            extract_expressions_from_format_string,
             &add_macro_decl(
                 r#"
 fn main() {
@@ -276,7 +279,7 @@ fn main() {
     #[test]
     fn nested_tt() {
         check_assist(
-            move_format_string_arg,
+            extract_expressions_from_format_string,
             &add_macro_decl(
                 r#"
 fn main() {
@@ -289,6 +292,29 @@ fn main() {
 fn main() {
     print!("My name is {} {}"$0, stringify!(Paperino), x + x)
 }
+"#,
+            ),
+        );
+    }
+
+    #[test]
+    fn extract_only_expressions() {
+        check_assist(
+            extract_expressions_from_format_string,
+            &add_macro_decl(
+                r#"
+fn main() {
+    let var = 1 + 1;
+    print!("foobar {var} {var:?} {x$0 + x}")
+}
+"#,
+            ),
+            &add_macro_decl(
+                r#"
+fn main() {
+    let var = 1 + 1;
+    print!("foobar {var} {var:?} {}"$0, x + x)
+}
 "#,
             ),
         );

crates/ide-assists/src/lib.rs

@@ -128,6 +128,7 @@ mod handlers {
     mod convert_while_to_loop;
     mod destructure_tuple_binding;
     mod expand_glob_import;
+    mod extract_expressions_from_format_string;
     mod extract_function;
     mod extract_module;
     mod extract_struct_from_enum_variant;
@@ -138,7 +139,6 @@ mod handlers {
     mod flip_binexpr;
     mod flip_comma;
     mod flip_trait_bound;
-    mod move_format_string_arg;
     mod generate_constant;
     mod generate_default_from_enum_variant;
     mod generate_default_from_new;
@@ -231,6 +231,7 @@ mod handlers {
             convert_while_to_loop::convert_while_to_loop,
             destructure_tuple_binding::destructure_tuple_binding,
             expand_glob_import::expand_glob_import,
+            extract_expressions_from_format_string::extract_expressions_from_format_string,
             extract_struct_from_enum_variant::extract_struct_from_enum_variant,
             extract_type_alias::extract_type_alias,
             fix_visibility::fix_visibility,
@@ -265,7 +266,6 @@ mod handlers {
             merge_match_arms::merge_match_arms,
             move_bounds::move_bounds_to_where_clause,
             move_const_to_impl::move_const_to_impl,
-            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,

crates/ide-assists/src/tests/generated.rs

@@ -624,6 +624,37 @@ fn qux(bar: Bar, baz: Baz) {}
     )
 }
 
+#[test]
+fn doctest_extract_expressions_from_format_string() {
+    check_doc_test(
+        "extract_expressions_from_format_string",
+        r#####"
+macro_rules! format_args {
+    ($lit:literal $(tt:tt)*) => { 0 },
+}
+macro_rules! print {
+    ($($arg:tt)*) => (std::io::_print(format_args!($($arg)*)));
+}
+
+fn main() {
+    print!("{var} {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() {
+    print!("{var} {}"$0, x + 1);
+}
+"#####,
+    )
+}
+
 #[test]
 fn doctest_extract_function() {
     check_doc_test(
@@ -1703,37 +1734,6 @@ impl S {
     )
 }
 
-#[test]
-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() {
-    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() {
-    print!("{}"$0, x + 1);
-}
-"#####,
-    )
-}
-
 #[test]
 fn doctest_move_from_mod_rs() {
     check_doc_test(

crates/ide-completion/src/completions/postfix.rs

@@ -595,12 +595,12 @@ fn main() {
         check_edit(
             "format",
             r#"fn main() { "{some_var:?}".$0 }"#,
-            r#"fn main() { format!("{:?}", some_var) }"#,
+            r#"fn main() { format!("{some_var:?}") }"#,
         );
         check_edit(
             "panic",
             r#"fn main() { "Panic with {a}".$0 }"#,
-            r#"fn main() { panic!("Panic with {}", a) }"#,
+            r#"fn main() { panic!("Panic with {a}") }"#,
         );
         check_edit(
             "println",

crates/ide-completion/src/completions/postfix/format_like.rs

@@ -54,7 +54,11 @@ pub(crate) fn add_format_like_completions(
     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 = if exprs.is_empty() {
+                format!(r#"{}({})"#, macro_name, out)
+            } else {
+                format!(r#"{}({}, {})"#, macro_name, out, exprs.join(", "))
+            };
 
             postfix_snippet(label, macro_name, &snippet).add_to(acc);
         }
@@ -72,10 +76,9 @@ mod tests {
             ("eprintln!", "{}", r#"eprintln!("{}", $1)"#),
             (
                 "log::info!",
-                "{} {expr} {} {2 + 2}",
-                r#"log::info!("{} {} {} {}", $1, expr, $2, 2 + 2)"#,
+                "{} {ident} {} {2 + 2}",
+                r#"log::info!("{} {ident} {} {}", $1, $2, 2 + 2)"#,
             ),
-            ("format!", "{expr:?}", r#"format!("{:?}", expr)"#),
         ];
 
         for (kind, input, output) in test_vector {
@@ -85,4 +88,18 @@ mod tests {
             assert_eq!(&snippet, output);
         }
     }
+
+    #[test]
+    fn test_into_suggestion_no_epxrs() {
+        let test_vector = &[
+            ("println!", "{ident}", r#"println!("{ident}")"#),
+            ("format!", "{ident:?}", r#"format!("{ident:?}")"#),
+        ];
+
+        for (kind, input, output) in test_vector {
+            let (parsed_string, _exprs) = parse_format_exprs(input).unwrap();
+            let snippet = format!(r#"{}("{}")"#, kind, parsed_string);
+            assert_eq!(&snippet, output);
+        }
+    }
 }

crates/ide-db/src/syntax_helpers/format_string_exprs.rs

@@ -140,8 +140,8 @@ pub fn parse_format_exprs(input: &str) -> Result<(String, Vec<Arg>), ()> {
                         output.push_str(trimmed);
                     } else if matches!(state, State::Expr) {
                         extracted_expressions.push(Arg::Expr(trimmed.into()));
-                    } else {
-                        extracted_expressions.push(Arg::Ident(trimmed.into()));
+                    } else if matches!(state, State::Ident) {
+                        output.push_str(trimmed);
                     }
 
                     output.push(chr);
@@ -218,9 +218,9 @@ mod tests {
         let test_vector = &[
             ("no expressions", expect![["no expressions"]]),
             (r"no expressions with \$0$1", expect![r"no expressions with \\\$0\$1"]),
-            ("{expr} is {2 + 2}", expect![["{} is {}; expr, 2 + 2"]]),
-            ("{expr:?}", expect![["{:?}; expr"]]),
-            ("{expr:1$}", expect![[r"{:1\$}; expr"]]),
+            ("{expr} is {2 + 2}", expect![["{expr} is {}; 2 + 2"]]),
+            ("{expr:?}", expect![["{expr:?}"]]),
+            ("{expr:1$}", expect![[r"{expr:1\$}"]]),
             ("{:1$}", expect![[r"{:1\$}; $1"]]),
             ("{:>padding$}", expect![[r"{:>padding\$}; $1"]]),
             ("{}, {}, {0}", expect![[r"{}, {}, {0}; $1, $2"]]),
@@ -230,16 +230,16 @@ mod tests {
             ("malformed}", expect![["-"]]),
             ("{{correct", expect![["{{correct"]]),
             ("correct}}", expect![["correct}}"]]),
-            ("{correct}}}", expect![["{}}}; correct"]]),
-            ("{correct}}}}}", expect![["{}}}}}; correct"]]),
+            ("{correct}}}", expect![["{correct}}}"]]),
+            ("{correct}}}}}", expect![["{correct}}}}}"]]),
             ("{incorrect}}", expect![["-"]]),
             ("placeholders {} {}", expect![["placeholders {} {}; $1, $2"]]),
             ("mixed {} {2 + 2} {}", expect![["mixed {} {} {}; $1, 2 + 2, $2"]]),
             (
                 "{SomeStruct { val_a: 0, val_b: 1 }}",
                 expect![["{}; SomeStruct { val_a: 0, val_b: 1 }"]],
             ),
-            ("{expr:?} is {2.32f64:.5}", expect![["{:?} is {:.5}; expr, 2.32f64"]]),
+            ("{expr:?} is {2.32f64:.5}", expect![["{expr:?} is {:.5}; 2.32f64"]]),
             (
                 "{SomeStruct { val_a: 0, val_b: 1 }:?}",
                 expect![["{:?}; SomeStruct { val_a: 0, val_b: 1 }"]],
@@ -262,8 +262,6 @@ mod tests {
                 .unwrap()
                 .1,
             vec![
-                Arg::Ident("_ident".to_owned()),
-                Arg::Ident("r#raw_ident".to_owned()),
                 Arg::Expr("expr.obj".to_owned()),
                 Arg::Expr("name {thing: 42}".to_owned()),
                 Arg::Placeholder