From 2584d48508a0a8cef8794eccfe30db583948eb96 Mon Sep 17 00:00:00 2001 From: Kartavya Vashishtha Date: Sat, 10 Sep 2022 19:06:50 +0530 Subject: [PATCH 1/5] wip --- .../src/handlers/move_format_string_arg.rs | 209 +++++++++++++++ crates/ide-assists/src/lib.rs | 1 + crates/ide-assists/src/tests/generated.rs | 17 ++ .../src/completions/postfix/format_like.rs | 242 +----------------- crates/ide-db/src/lib.rs | 1 + .../src/syntax_helpers/format_string_exprs.rs | 227 ++++++++++++++++ 6 files changed, 463 insertions(+), 234 deletions(-) create mode 100644 crates/ide-assists/src/handlers/move_format_string_arg.rs create mode 100644 crates/ide-db/src/syntax_helpers/format_string_exprs.rs diff --git a/crates/ide-assists/src/handlers/move_format_string_arg.rs b/crates/ide-assists/src/handlers/move_format_string_arg.rs new file mode 100644 index 000000000000..696fd50b5cb2 --- /dev/null +++ b/crates/ide-assists/src/handlers/move_format_string_arg.rs @@ -0,0 +1,209 @@ +use ide_db::{syntax_helpers::{format_string::is_format_string, format_string_exprs::{parse_format_exprs, Arg}}, assists::{AssistId, AssistKind}}; +use itertools::Itertools; +use syntax::{ast, AstToken, AstNode, NodeOrToken, SyntaxKind::COMMA, TextRange}; + +// Assist: move_format_string_arg +// +// Move an expression out of a format string. +// +// ``` +// fn main() { +// println!("{x + 1}$0"); +// } +// ``` +// -> +// ``` +// fn main() { +// println!("{a}", a$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::()?; + 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()))?; + + 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 = 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 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; + } + } + } + } + } + + edit.insert(str_range.end(), args); + }); + + Some(()) +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::tests::check_assist; + + const MACRO_DECL: &'static str = r#" +macro_rules! format_args { + ($lit:literal $(tt:tt)*) => { 0 }, +} +macro_rules! print { + ($($arg:tt)*) => (std::io::_print(format_args!($($arg)*))); +} +"#; + + fn add_macro_decl (s: &'static str) -> String { + MACRO_DECL.to_string() + s + } + + #[test] + fn multiple_middle_arg() { + check_assist( + move_format_string_arg, + &add_macro_decl(r#" +fn main() { + print!("{} {x + 1:b} {}$0", y + 2, 2); +} +"#), + + &add_macro_decl(r#" +fn main() { + print!("{} {:b} {}"$0, y + 2, x + 1, 2); +} +"#), + ); + } + + #[test] + fn single_arg() { + check_assist( + move_format_string_arg, + &add_macro_decl(r#" +fn main() { + print!("{obj.value:b}$0",); +} +"#), + &add_macro_decl(r#" +fn main() { + print!("{:b}"$0, obj.value); +} +"#), + ); + } + + #[test] + fn multiple_middle_placeholders_arg() { + check_assist( + move_format_string_arg, + &add_macro_decl(r#" +fn main() { + print!("{} {x + 1:b} {} {}$0", y + 2, 2); +} +"#), + + &add_macro_decl(r#" +fn main() { + print!("{} {:b} {} {}"$0, y + 2, x + 1, 2, $1); +} +"#), + ); + } + + #[test] + fn multiple_trailing_args() { + check_assist( + move_format_string_arg, + &add_macro_decl(r#" +fn main() { + print!("{} {x + 1:b} {Struct(1, 2)}$0", 1); +} +"#), + + &add_macro_decl(r#" +fn main() { + print!("{} {:b} {}"$0, 1, x + 1, Struct(1, 2)); +} +"#), + ); + } + + #[test] + fn improper_commas() { + check_assist( + move_format_string_arg, + &add_macro_decl(r#" +fn main() { + print!("{} {x + 1:b} {Struct(1, 2)}$0", 1,); +} +"#), + + &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 e52544db5f53..f881be9cf6d0 100644 --- a/crates/ide-assists/src/lib.rs +++ b/crates/ide-assists/src/lib.rs @@ -136,6 +136,7 @@ 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; diff --git a/crates/ide-assists/src/tests/generated.rs b/crates/ide-assists/src/tests/generated.rs index 227e2300f92a..fa88abd6c554 100644 --- a/crates/ide-assists/src/tests/generated.rs +++ b/crates/ide-assists/src/tests/generated.rs @@ -1591,6 +1591,23 @@ fn apply(f: F, x: T) -> U where F: FnOnce(T) -> U { ) } +#[test] +fn doctest_move_format_string_arg() { + check_doc_test( + "move_format_string_arg", + r#####" +fn main() { + println!("{x + 1}$0"); +} +"#####, + r#####" +fn main() { + println!("{a}", a$0 = x + 1); +} +"#####, + ) +} + #[test] fn doctest_move_from_mod_rs() { check_doc_test( diff --git a/crates/ide-completion/src/completions/postfix/format_like.rs b/crates/ide-completion/src/completions/postfix/format_like.rs index b273a4cb53ba..89bfdac74d21 100644 --- a/crates/ide-completion/src/completions/postfix/format_like.rs +++ b/crates/ide-completion/src/completions/postfix/format_like.rs @@ -16,7 +16,7 @@ // // image::https://user-images.githubusercontent.com/48062697/113020656-b560f500-917a-11eb-87de-02991f61beb8.gif[] -use ide_db::SnippetCap; +use ide_db::{syntax_helpers::format_string_exprs::{parse_format_exprs, add_placeholders}, SnippetCap}; use syntax::ast::{self, AstToken}; use crate::{ @@ -43,250 +43,24 @@ pub(crate) fn add_format_like_completions( cap: SnippetCap, receiver_text: &ast::String, ) { - let input = match string_literal_contents(receiver_text) { - // It's not a string literal, do not parse input. - Some(input) => input, - None => return, - }; - let postfix_snippet = match build_postfix_snippet_builder(ctx, cap, dot_receiver) { Some(it) => it, None => return, }; - let mut parser = FormatStrParser::new(input); - if parser.parse().is_ok() { + if let Ok((out, exprs)) = parse_format_exprs(receiver_text) { + let exprs = add_placeholders(exprs.map(|e| e.1)).collect_vec(); for (label, macro_name) in KINDS { - let snippet = parser.to_suggestion(macro_name); + let snippet = format!(r#"{}("{}", {})"#, macro_name, out, exprs.join(", ")); postfix_snippet(label, macro_name, &snippet).add_to(acc); } } } -/// Checks whether provided item is a string literal. -fn string_literal_contents(item: &ast::String) -> Option { - let item = item.text(); - if item.len() >= 2 && item.starts_with('\"') && item.ends_with('\"') { - return Some(item[1..item.len() - 1].to_owned()); - } - - None -} - -/// Parser for a format-like string. It is more allowing in terms of string contents, -/// as we expect variable placeholders to be filled with expressions. -#[derive(Debug)] -pub(crate) struct FormatStrParser { - input: String, - output: String, - extracted_expressions: Vec, - state: State, - parsed: bool, -} - -#[derive(Debug, Clone, Copy, PartialEq)] -enum State { - NotExpr, - MaybeExpr, - Expr, - MaybeIncorrect, - FormatOpts, -} - -impl FormatStrParser { - pub(crate) fn new(input: String) -> Self { - Self { - input, - output: String::new(), - extracted_expressions: Vec::new(), - state: State::NotExpr, - parsed: false, - } - } - - pub(crate) fn parse(&mut self) -> Result<(), ()> { - let mut current_expr = String::new(); - - let mut placeholder_id = 1; - - // Count of open braces inside of an expression. - // We assume that user knows what they're doing, thus we treat it like a correct pattern, e.g. - // "{MyStruct { val_a: 0, val_b: 1 }}". - let mut inexpr_open_count = 0; - - // We need to escape '\' and '$'. See the comments on `get_receiver_text()` for detail. - let mut chars = self.input.chars().peekable(); - while let Some(chr) = chars.next() { - match (self.state, chr) { - (State::NotExpr, '{') => { - self.output.push(chr); - self.state = State::MaybeExpr; - } - (State::NotExpr, '}') => { - self.output.push(chr); - self.state = State::MaybeIncorrect; - } - (State::NotExpr, _) => { - if matches!(chr, '\\' | '$') { - self.output.push('\\'); - } - self.output.push(chr); - } - (State::MaybeIncorrect, '}') => { - // It's okay, we met "}}". - self.output.push(chr); - self.state = State::NotExpr; - } - (State::MaybeIncorrect, _) => { - // Error in the string. - return Err(()); - } - (State::MaybeExpr, '{') => { - self.output.push(chr); - self.state = State::NotExpr; - } - (State::MaybeExpr, '}') => { - // This is an empty sequence '{}'. Replace it with placeholder. - self.output.push(chr); - self.extracted_expressions.push(format!("${}", placeholder_id)); - placeholder_id += 1; - self.state = State::NotExpr; - } - (State::MaybeExpr, _) => { - if matches!(chr, '\\' | '$') { - current_expr.push('\\'); - } - current_expr.push(chr); - self.state = State::Expr; - } - (State::Expr, '}') => { - if inexpr_open_count == 0 { - self.output.push(chr); - self.extracted_expressions.push(current_expr.trim().into()); - current_expr = String::new(); - self.state = State::NotExpr; - } else { - // We're closing one brace met before inside of the expression. - current_expr.push(chr); - inexpr_open_count -= 1; - } - } - (State::Expr, ':') if chars.peek().copied() == Some(':') => { - // path separator - current_expr.push_str("::"); - chars.next(); - } - (State::Expr, ':') => { - if inexpr_open_count == 0 { - // We're outside of braces, thus assume that it's a specifier, like "{Some(value):?}" - self.output.push(chr); - self.extracted_expressions.push(current_expr.trim().into()); - current_expr = String::new(); - self.state = State::FormatOpts; - } else { - // We're inside of braced expression, assume that it's a struct field name/value delimiter. - current_expr.push(chr); - } - } - (State::Expr, '{') => { - current_expr.push(chr); - inexpr_open_count += 1; - } - (State::Expr, _) => { - if matches!(chr, '\\' | '$') { - current_expr.push('\\'); - } - current_expr.push(chr); - } - (State::FormatOpts, '}') => { - self.output.push(chr); - self.state = State::NotExpr; - } - (State::FormatOpts, _) => { - if matches!(chr, '\\' | '$') { - self.output.push('\\'); - } - self.output.push(chr); - } - } - } - - if self.state != State::NotExpr { - return Err(()); - } - - self.parsed = true; - Ok(()) - } - - pub(crate) fn to_suggestion(&self, macro_name: &str) -> String { - assert!(self.parsed, "Attempt to get a suggestion from not parsed expression"); - - let expressions_as_string = self.extracted_expressions.join(", "); - format!(r#"{}("{}", {})"#, macro_name, self.output, expressions_as_string) - } -} - #[cfg(test)] mod tests { use super::*; - 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(", ")) - } - } else { - // Parsing should fail, expected repr is "-". - "-".to_owned() - }; - - expect.assert_eq(&outcome_repr); - } - - #[test] - fn format_str_parser() { - 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"]]), - ("{$0}", expect![[r"{}; \$0"]]), - ("{malformed", expect![["-"]]), - ("malformed}", expect![["-"]]), - ("{{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"]]), - ( - "{SomeStruct { val_a: 0, val_b: 1 }:?}", - expect![["{:?}; SomeStruct { val_a: 0, val_b: 1 }"]], - ), - ("{ 2 + 2 }", expect![["{}; 2 + 2"]]), - ("{strsim::jaro_winkle(a)}", expect![["{}; strsim::jaro_winkle(a)"]]), - ("{foo::bar::baz()}", expect![["{}; foo::bar::baz()"]]), - ("{foo::bar():?}", expect![["{:?}; foo::bar()"]]), - ]; - - for (input, output) in test_vector { - check(input, output) - } - } #[test] fn test_into_suggestion() { @@ -302,10 +76,10 @@ mod tests { ]; for (kind, input, output) in test_vector { - let mut parser = FormatStrParser::new((*input).to_owned()); - parser.parse().expect("Parsing must succeed"); - - assert_eq!(&parser.to_suggestion(*kind), output); + let (parsed_string, exprs) = parse_format_exprs(input).unwrap(); + let exprs = add_placeholders(exprs.map(|e| e.1)).collect_vec();; + let snippet = format!(r#"{}("{}", {})"#, kind, parsed_string, exprs.join(", ")); + assert_eq!(&snippet, output); } } } diff --git a/crates/ide-db/src/lib.rs b/crates/ide-db/src/lib.rs index 1ec62a8425a3..e0bc0f89f0a1 100644 --- a/crates/ide-db/src/lib.rs +++ b/crates/ide-db/src/lib.rs @@ -38,6 +38,7 @@ pub mod syntax_helpers { pub mod node_ext; pub mod insert_whitespace_into_node; pub mod format_string; + pub mod format_string_exprs; pub use parser::LexedStr; } diff --git a/crates/ide-db/src/syntax_helpers/format_string_exprs.rs b/crates/ide-db/src/syntax_helpers/format_string_exprs.rs new file mode 100644 index 000000000000..b6b2eb268d33 --- /dev/null +++ b/crates/ide-db/src/syntax_helpers/format_string_exprs.rs @@ -0,0 +1,227 @@ +use syntax::{ast, TextRange, AstToken}; + +#[derive(Debug)] +pub enum Arg { + Placeholder, + Expr(String) +} + +/** + Add placeholders like `$1` and `$2` in place of [`Arg::Placeholder`]. + ```rust + assert_eq!(vec![Arg::Expr("expr"), Arg::Placeholder, Arg::Expr("expr")], vec!["expr", "$1", "expr"]) + ``` +*/ + +pub fn add_placeholders (args: impl Iterator) -> impl Iterator { + let mut placeholder_id = 1; + args.map(move |a| + match a { + Arg::Expr(s) => s, + Arg::Placeholder => { + let s = format!("${placeholder_id}"); + placeholder_id += 1; + s + } + } + ) +} + +/** + Parser for a format-like string. It is more allowing in terms of string contents, + as we expect variable placeholders to be filled with expressions. + + Built for completions and assists, and escapes `\` and `$` in output. + (See the comments on `get_receiver_text()` for detail.) + Splits a format string that may contain expressions + like + ```rust + assert_eq!(parse("{expr} {} {expr} ").unwrap(), ("{} {} {}", vec![Arg::Expr("expr"), Arg::Placeholder, Arg::Expr("expr")])); + ``` +*/ +pub fn parse_format_exprs(input: &ast::String) -> Result, ()> { + #[derive(Debug, Clone, Copy, PartialEq)] + enum State { + NotExpr, + MaybeExpr, + Expr, + MaybeIncorrect, + 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(); + let mut output = String::new(); + + // Count of open braces inside of an expression. + // We assume that user knows what they're doing, thus we treat it like a correct pattern, e.g. + // "{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() { + match (state, chr) { + (State::NotExpr, '{') => { + output.push(chr); + state = State::MaybeExpr; + } + (State::NotExpr, '}') => { + output.push(chr); + state = State::MaybeIncorrect; + } + (State::NotExpr, _) => { + if matches!(chr, '\\' | '$') { + output.push('\\'); + } + output.push(chr); + } + (State::MaybeIncorrect, '}') => { + // It's okay, we met "}}". + output.push(chr); + state = State::NotExpr; + } + (State::MaybeIncorrect, _) => { + // Error in the string. + return Err(()); + } + (State::MaybeExpr, '{') => { + output.push(chr); + state = State::NotExpr; + } + (State::MaybeExpr, '}') => { + // This is an empty sequence '{}'. Replace it with placeholder. + output.push(chr); + extracted_expressions.push((TextRange::empty(expr_start), Arg::Placeholder)); + state = State::NotExpr; + } + (State::MaybeExpr, _) => { + if matches!(chr, '\\' | '$') { + 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()))); + current_expr = String::new(); + state = State::NotExpr; + } else { + // We're closing one brace met before inside of the expression. + current_expr.push(chr); + inexpr_open_count -= 1; + } + } + (State::Expr, ':') if matches!(chars.peek(), Some((':', _))) => { + // path separator + current_expr.push_str("::"); + chars.next(); + } + (State::Expr, ':') => { + 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()))); + current_expr = String::new(); + state = State::FormatOpts; + } else { + // We're inside of braced expression, assume that it's a struct field name/value delimiter. + current_expr.push(chr); + } + } + (State::Expr, '{') => { + current_expr.push(chr); + inexpr_open_count += 1; + } + (State::Expr, _) => { + if matches!(chr, '\\' | '$') { + current_expr.push('\\'); + } + current_expr.push(chr); + } + (State::FormatOpts, '}') => { + output.push(chr); + state = State::NotExpr; + } + (State::FormatOpts, _) => { + if matches!(chr, '\\' | '$') { + output.push('\\'); + } + output.push(chr); + } + } + } + + if state != State::NotExpr { + return Err(()); + } + + Ok(extracted_expressions) +} + +#[cfg(test)] +mod tests { + use super::*; + 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(", ")) + } + } else { + // Parsing should fail, expected repr is "-". + "-".to_owned() + }; + + expect.assert_eq(&outcome_repr); + } + + #[test] + fn format_str_parser() { + 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"]]), + ("{$0}", expect![[r"{}; \$0"]]), + ("{malformed", expect![["-"]]), + ("malformed}", expect![["-"]]), + ("{{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"]]), + ( + "{SomeStruct { val_a: 0, val_b: 1 }:?}", + expect![["{:?}; SomeStruct { val_a: 0, val_b: 1 }"]], + ), + ("{ 2 + 2 }", expect![["{}; 2 + 2"]]), + ("{strsim::jaro_winkle(a)}", expect![["{}; strsim::jaro_winkle(a)"]]), + ("{foo::bar::baz()}", expect![["{}; foo::bar::baz()"]]), + ("{foo::bar():?}", expect![["{:?}; foo::bar()"]]), + ]; + + for (input, output) in test_vector { + check(input, output) + } + } +} From cc7200891b3cf624f716b03ef24867650b485dca Mon Sep 17 00:00:00 2001 From: Kartavya Vashishtha Date: Sat, 10 Sep 2022 20:12:47 +0530 Subject: [PATCH 2/5] 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 --- .../src/handlers/move_format_string_arg.rs | 238 +++++++++++------- crates/ide-assists/src/lib.rs | 1 + crates/ide-assists/src/tests/generated.rs | 18 +- .../src/completions/postfix/format_like.rs | 15 +- .../src/syntax_helpers/format_string_exprs.rs | 52 ++-- 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 696fd50b5cb2..54b5bee9b7b0 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::()?; - 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::()?; + 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 = 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 = 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 f881be9cf6d0..812d22efbd79 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 fa88abd6c554..3a696635afd2 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 89bfdac74d21..b43bdb9ab9d1 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 b6b2eb268d33..e9006e06b110 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) -> impl Iterator { +pub fn with_placeholders(args: Vec) -> Vec { 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) -> impl Iterator Result, ()> { +pub fn parse_format_exprs(input: &str) -> Result<(String, Vec), ()> { #[derive(Debug, Clone, Copy, PartialEq)] enum State { NotExpr, @@ -49,9 +53,6 @@ pub fn parse_format_exprs(input: &ast::String) -> Result, 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, // "{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, (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, 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, 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, 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, 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); From a5cbee4d11ff0c4fca00493d4015d9b8e7f82d6d Mon Sep 17 00:00:00 2001 From: Kartavya Vashishtha Date: Sat, 10 Sep 2022 21:04:25 +0530 Subject: [PATCH 3/5] remove false positive --- crates/ide-assists/src/handlers/move_format_string_arg.rs | 3 +++ 1 file changed, 3 insertions(+) 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 54b5bee9b7b0..4a5dd09c8866 100644 --- a/crates/ide-assists/src/handlers/move_format_string_arg.rs +++ b/crates/ide-assists/src/handlers/move_format_string_arg.rs @@ -51,6 +51,9 @@ pub(crate) fn move_format_string_arg(acc: &mut Assists, ctx: &AssistContext<'_>) } let (new_fmt, extracted_args) = parse_format_exprs(fmt_string.text()).ok()?; + if extracted_args.is_empty() { + return None; + } acc.add( AssistId("move_format_string_arg", AssistKind::QuickFix), From fb5ae9906ba7961c8b3c39512181c966b82d180c Mon Sep 17 00:00:00 2001 From: Kartavya Vashishtha Date: Sun, 11 Sep 2022 10:39:25 +0530 Subject: [PATCH 4/5] suggest ExtractRefactor if no expressions found Added `Ident` variant to arg enum. --- .../src/handlers/move_format_string_arg.rs | 12 +- .../src/syntax_helpers/format_string_exprs.rs | 106 +++++++++++++----- 2 files changed, 86 insertions(+), 32 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 4a5dd09c8866..cb7c30b37d6b 100644 --- a/crates/ide-assists/src/handlers/move_format_string_arg.rs +++ b/crates/ide-assists/src/handlers/move_format_string_arg.rs @@ -56,7 +56,15 @@ pub(crate) fn move_format_string_arg(acc: &mut Assists, ctx: &AssistContext<'_>) } acc.add( - AssistId("move_format_string_arg", AssistKind::QuickFix), + AssistId( + "move_format_string_arg", + // 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", tt.syntax().text_range(), |edit| { @@ -107,7 +115,7 @@ pub(crate) fn move_format_string_arg(acc: &mut Assists, ctx: &AssistContext<'_>) args.push_str(", "); match extracted_args { - Arg::Expr(s) => { + Arg::Ident(s) | Arg::Expr(s) => { // insert arg args.push_str(&s); } 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 e9006e06b110..ac6c6e8feeea 100644 --- a/crates/ide-db/src/syntax_helpers/format_string_exprs.rs +++ b/crates/ide-db/src/syntax_helpers/format_string_exprs.rs @@ -4,16 +4,19 @@ /// Enum for represenging extraced format string args. /// Can either be extracted expressions (which includes identifiers), /// or placeholders `{}`. -#[derive(Debug)] +#[derive(Debug, PartialEq, Eq)] pub enum Arg { Placeholder, + Ident(String), Expr(String), } /** - Add placeholders like `$1` and `$2` in place of [`Arg::Placeholder`]. + Add placeholders like `$1` and `$2` in place of [`Arg::Placeholder`], + and unwraps the [`Arg::Ident`] and [`Arg::Expr`] enums. ```rust - assert_eq!(vec![Arg::Expr("expr"), Arg::Placeholder, Arg::Expr("expr")], vec!["expr", "$1", "expr"]) + # use ide_db::syntax_helpers::format_string_exprs::*; + assert_eq!(with_placeholders(vec![Arg::Ident("ident".to_owned()), Arg::Placeholder, Arg::Expr("expr + 2".to_owned())]), vec!["ident".to_owned(), "$1".to_owned(), "expr + 2".to_owned()]) ``` */ @@ -21,7 +24,7 @@ pub fn with_placeholders(args: Vec) -> Vec { let mut placeholder_id = 1; args.into_iter() .map(move |a| match a { - Arg::Expr(s) => s, + Arg::Expr(s) | Arg::Ident(s) => s, Arg::Placeholder => { let s = format!("${placeholder_id}"); placeholder_id += 1; @@ -40,21 +43,22 @@ pub fn with_placeholders(args: Vec) -> Vec { Splits a format string that may contain expressions like ```rust - assert_eq!(parse("{expr} {} {expr} ").unwrap(), ("{} {} {}", vec![Arg::Expr("expr"), Arg::Placeholder, Arg::Expr("expr")])); + assert_eq!(parse("{ident} {} {expr + 42} ").unwrap(), ("{} {} {}", vec![Arg::Ident("ident"), Arg::Placeholder, Arg::Expr("expr + 42")])); ``` */ pub fn parse_format_exprs(input: &str) -> Result<(String, Vec), ()> { #[derive(Debug, Clone, Copy, PartialEq)] enum State { - NotExpr, - MaybeExpr, + NotArg, + MaybeArg, Expr, + Ident, MaybeIncorrect, FormatOpts, } + let mut state = State::NotArg; let mut current_expr = String::new(); - let mut state = State::NotExpr; let mut extracted_expressions = Vec::new(); let mut output = String::new(); @@ -66,15 +70,15 @@ pub fn parse_format_exprs(input: &str) -> Result<(String, Vec), ()> { let mut chars = input.chars().peekable(); while let Some(chr) = chars.next() { match (state, chr) { - (State::NotExpr, '{') => { + (State::NotArg, '{') => { output.push(chr); - state = State::MaybeExpr; + state = State::MaybeArg; } - (State::NotExpr, '}') => { + (State::NotArg, '}') => { output.push(chr); state = State::MaybeIncorrect; } - (State::NotExpr, _) => { + (State::NotArg, _) => { if matches!(chr, '\\' | '$') { output.push('\\'); } @@ -83,51 +87,72 @@ pub fn parse_format_exprs(input: &str) -> Result<(String, Vec), ()> { (State::MaybeIncorrect, '}') => { // It's okay, we met "}}". output.push(chr); - state = State::NotExpr; + state = State::NotArg; } (State::MaybeIncorrect, _) => { // Error in the string. return Err(()); } - (State::MaybeExpr, '{') => { + // Escaped braces `{{` + (State::MaybeArg, '{') => { output.push(chr); - state = State::NotExpr; + state = State::NotArg; } - (State::MaybeExpr, '}') => { - // This is an empty sequence '{}'. Replace it with placeholder. + (State::MaybeArg, '}') => { + // This is an empty sequence '{}'. output.push(chr); extracted_expressions.push(Arg::Placeholder); - state = State::NotExpr; + state = State::NotArg; } - (State::MaybeExpr, _) => { + (State::MaybeArg, _) => { if matches!(chr, '\\' | '$') { current_expr.push('\\'); } current_expr.push(chr); - state = State::Expr; + + // While Rust uses the unicode sets of XID_start and XID_continue for Identifiers + // this is probably the best we can do to avoid a false positive + if chr.is_alphabetic() || chr == '_' { + state = State::Ident; + } else { + state = State::Expr; + } } - (State::Expr, '}') => { + (State::Ident | State::Expr, '}') => { if inexpr_open_count == 0 { output.push(chr); - extracted_expressions.push(Arg::Expr(current_expr.trim().into())); + + 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::NotExpr; + state = State::NotArg; } else { // We're closing one brace met before inside of the expression. current_expr.push(chr); inexpr_open_count -= 1; } } - (State::Expr, ':') if matches!(chars.peek(), Some(':')) => { + (State::Ident | State::Expr, ':') if matches!(chars.peek(), Some(':')) => { // path separator + state = State::Expr; current_expr.push_str("::"); chars.next(); } - (State::Expr, ':') => { + (State::Ident | State::Expr, ':') => { 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(Arg::Expr(current_expr.trim().into())); + + 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::FormatOpts; } else { @@ -135,11 +160,16 @@ pub fn parse_format_exprs(input: &str) -> Result<(String, Vec), ()> { current_expr.push(chr); } } - (State::Expr, '{') => { + (State::Ident | State::Expr, '{') => { + state = State::Expr; current_expr.push(chr); inexpr_open_count += 1; } - (State::Expr, _) => { + (State::Ident | State::Expr, _) => { + if !(chr.is_alphanumeric() || chr == '_' || chr == '#') { + state = State::Expr; + } + if matches!(chr, '\\' | '$') { current_expr.push('\\'); } @@ -147,7 +177,7 @@ pub fn parse_format_exprs(input: &str) -> Result<(String, Vec), ()> { } (State::FormatOpts, '}') => { output.push(chr); - state = State::NotExpr; + state = State::NotArg; } (State::FormatOpts, _) => { if matches!(chr, '\\' | '$') { @@ -158,7 +188,7 @@ pub fn parse_format_exprs(input: &str) -> Result<(String, Vec), ()> { } } - if state != State::NotExpr { + if state != State::NotArg { return Err(()); } @@ -218,4 +248,20 @@ mod tests { check(input, output) } } + + #[test] + fn arg_type() { + assert_eq!( + parse_format_exprs("{_ident} {r#raw_ident} {expr.obj} {name {thing: 42} } {}") + .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 + ] + ); + } } From 54e9324e93646433e8404af106223890ef52105c Mon Sep 17 00:00:00 2001 From: Kartavya Vashishtha Date: Mon, 12 Sep 2022 05:45:11 -0700 Subject: [PATCH 5/5] Update crates/ide-assists/src/handlers/move_format_string_arg.rs Co-authored-by: Lukas Wirth --- crates/ide-assists/src/handlers/move_format_string_arg.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 cb7c30b37d6b..92b2fa79d717 100644 --- a/crates/ide-assists/src/handlers/move_format_string_arg.rs +++ b/crates/ide-assists/src/handlers/move_format_string_arg.rs @@ -41,7 +41,7 @@ use syntax::{ast, AstNode, AstToken, NodeOrToken, SyntaxKind::COMMA, TextRange}; pub(crate) fn move_format_string_arg(acc: &mut Assists, ctx: &AssistContext<'_>) -> Option<()> { let fmt_string = ctx.find_token_at_offset::()?; - let tt = fmt_string.syntax().parent_ancestors().find_map(ast::TokenTree::cast)?; + let tt = fmt_string.syntax().parent().and_then(ast::TokenTree::cast)?; let expanded_t = ast::String::cast( ctx.sema.descend_into_macros_with_kind_preference(fmt_string.syntax().clone()),