From cc7200891b3cf624f716b03ef24867650b485dca Mon Sep 17 00:00:00 2001 From: Kartavya Vashishtha Date: Sat, 10 Sep 2022 20:12:47 +0530 Subject: [PATCH] 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 | 224 +++++++++++------- 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, 185 insertions(+), 125 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 (new_fmt, extracted_args) = parse_format_exprs(fmt_string.text()).ok()?; - 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", + 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| { - let mut existing_args: Vec = vec![]; - let mut current_arg = String::new(); + // Replace old format string with new format string whose arguments have been extracted + edit.replace(fmt_range, new_fmt); - 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()); + // 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())); - } + // Start building the new args + let mut existing_args = existing_args.into_iter(); + 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 - edit.insert(str_range.end(), "$0"); - let mut placeholder_idx = 1; - let mut args = String::new(); + for extracted_args in extracted_args { + // remove expr from format string + args.push_str(", "); - 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; + 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);