From 0097a903fde9f3729a87bd6f1c46487e1e969c65 Mon Sep 17 00:00:00 2001 From: David Bar-On Date: Wed, 27 Jul 2022 20:29:36 +0300 Subject: [PATCH] Fix for issue #4609 - prevent unlimitted indentations in macro formatting when repeating formatting --- src/macros.rs | 65 ++++++++++++++++++++--- tests/source/issue-4609/one.rs | 94 ++++++++++++++++++++++++++++++++++ tests/source/issue-4609/two.rs | 94 ++++++++++++++++++++++++++++++++++ tests/target/issue-4609/one.rs | 94 ++++++++++++++++++++++++++++++++++ tests/target/issue-4609/two.rs | 94 ++++++++++++++++++++++++++++++++++ 5 files changed, 435 insertions(+), 6 deletions(-) create mode 100644 tests/source/issue-4609/one.rs create mode 100644 tests/source/issue-4609/two.rs create mode 100644 tests/target/issue-4609/one.rs create mode 100644 tests/target/issue-4609/two.rs diff --git a/src/macros.rs b/src/macros.rs index e78ef1f8066..5289d7d6a56 100644 --- a/src/macros.rs +++ b/src/macros.rs @@ -9,6 +9,7 @@ // List-like invocations with parentheses will be formatted as function calls, // and those with brackets will be formatted as array literals. +use std::cmp::min; use std::collections::HashMap; use std::panic::{catch_unwind, AssertUnwindSafe}; @@ -472,35 +473,82 @@ fn register_metavariable( let mut new_name = "$".repeat(dollar_count - 1); let mut old_name = "$".repeat(dollar_count); - new_name.push('z'); - new_name.push_str(name); + // Make sure one new name will not be a subset of another new name + const DOLLAR_SUBS_CHARS: &str = "zZyYxXwWvVuUtTsSrRqQpPoOmMlLkKjJiIhHgGfFeEdDcCbBaA"; + let identifies_count = min( + name.chars().filter(|c| *c == ' ').count(), + DOLLAR_SUBS_CHARS.len() - 1, + ); + let dollar_replacement = &DOLLAR_SUBS_CHARS[identifies_count..identifies_count + 1]; + + new_name.push_str(dollar_replacement); + // Replace internal `$` and ` ` + new_name.push_str(&name.replace("$", &dollar_replacement).replace(" ", "_")); + old_name.push_str(name); result.push_str(&new_name); map.insert(old_name, new_name); } -// Replaces `$foo` with `zfoo`. We must check for name overlap to ensure we -// aren't causing problems. -// This should also work for escaped `$` variables, where we leave earlier `$`s. +// Replaces `$foo` with `zfoo`. +// Allows the `$foo` to be handled as one item during formatting. +// We must check for name overlap to ensure we aren't causing problems. +// This should also work for escaped `$` variables, where we leave earlier `$`s. +// Also replaces `$foo bar` or `$foo $bar` with `zfoo_bar` or `zfoo_zbar`. +// Allows the two identities to be handled as one identity during formatting. +// Otherwise formatting fails as a BinOp, etc. is expected between the two identities. +// Consecutive spaces between the identities are replaced with one space. fn replace_names(input: &str) -> Option<(String, HashMap)> { // Each substitution will require five or six extra bytes. let mut result = String::with_capacity(input.len() + 64); let mut substs = HashMap::new(); let mut dollar_count = 0; let mut cur_name = String::new(); + let mut cur_spaces = String::new(); + let white_space: &[_] = &[' ', '\t']; for (kind, c) in CharClasses::new(input.chars()) { + // Handle the `$foo bar`/`$foo $bar` cases + let mut follows_spaces = false; + if dollar_count > 0 && white_space.contains(&c) { + cur_spaces.push(c); + continue; + } else if dollar_count > 0 && !white_space.contains(&c) { + if !cur_spaces.is_empty() { + if c == '$' || c.is_alphanumeric() { + // Keep `$foo` and a following `bar`/`$bar` as one name + cur_name.push(' '); + cur_spaces.clear(); + follows_spaces = true; + } + } + } + + // Handle the `$foo` name replacement if kind != FullCodeCharKind::Normal { result.push(c); } else if c == '$' { - dollar_count += 1; + if dollar_count == 0 { + dollar_count += 1; + } else { + if !follows_spaces { + dollar_count += 1; + } else { + cur_name.push(c); + } + } } else if dollar_count == 0 { result.push(c); } else if !c.is_alphanumeric() && !cur_name.is_empty() { // Terminates a name following one or more dollars. register_metavariable(&mut substs, &mut result, &cur_name, dollar_count); + if !cur_spaces.is_empty() { + result.push_str(&cur_spaces); + cur_spaces.clear(); + } + result.push(c); dollar_count = 0; cur_name.clear(); @@ -516,6 +564,11 @@ fn replace_names(input: &str) -> Option<(String, HashMap)> { register_metavariable(&mut substs, &mut result, &cur_name, dollar_count); } + // Reserving trailing spaces at the end (following `$foo`) + if !cur_spaces.is_empty() { + result.push_str(&cur_spaces) + } + debug!("replace_names `{}` {:?}", result, substs); Some((result, substs)) diff --git a/tests/source/issue-4609/one.rs b/tests/source/issue-4609/one.rs new file mode 100644 index 00000000000..6251b4e6d5d --- /dev/null +++ b/tests/source/issue-4609/one.rs @@ -0,0 +1,94 @@ +// rustfmt-version: One + +// Original issue - parameters in macro call + +macro_rules! outer { + ($d:tt) => { + macro_rules! inner { + ($d s:expr) => { + println!("{}", $d s); + } + } + }; +} + +outer!($); + +fn main() { + inner!("hi"); +} + +macro_rules! outer { + ($d:tt) => { + macro_rules! inner { + ($d s:expr) => { + println!("{}", $d s); + } + } + }; +} + +// Variations of the original issue + +macro_rules! outer { +($d:tt) => { +macro_rules! inner { +($d s:expr) => { +println!("{}", $d s); +} +} +}; +} + +macro_rules! outer { +($d:tt) => { +macro_rules! inner { +($d s:expr) => { +println!("{}", $d s); +} +} +}; +} + +fn main() { +macro_rules! outer { +($ d:tt) => { +macro_rules! inner { +($ d s:expr) => { +println!("INNER1: {}", $d s); +println!("INNER2: {}", $ s); +} +} +}; +} + +outer!($); + +inner!("hi"); +} + +// Consecutive identities in macro body + +fn main() { +macro_rules! uniop { +($op:tt, $s:expr) => { +$op $s +}; +} +let x = uniop!(!, true); +println!("{}", x); +let x = uniop!(-, 7); +println!("{}", x); +} + +fn main() { +macro_rules! binop { +($l:expr, $op:tt, $r:expr) => { +$l $op $r +}; +} +let x = binop!(10, -, 7); +println!("{}", x); +let x = binop!(10, +, 7); +println!("{}", x); +} diff --git a/tests/source/issue-4609/two.rs b/tests/source/issue-4609/two.rs new file mode 100644 index 00000000000..01ef9184ed2 --- /dev/null +++ b/tests/source/issue-4609/two.rs @@ -0,0 +1,94 @@ +// rustfmt-version: Two + +// Original issue - parameters in macro call + +macro_rules! outer { + ($d:tt) => { + macro_rules! inner { + ($d s:expr) => { + println!("{}", $d s); + } + } + }; +} + +outer!($); + +fn main() { + inner!("hi"); +} + +macro_rules! outer { + ($d:tt) => { + macro_rules! inner { + ($d s:expr) => { + println!("{}", $d s); + } + } + }; +} + +// Variations of the original issue + +macro_rules! outer { +($d:tt) => { +macro_rules! inner { +($d s:expr) => { +println!("{}", $d s); +} +} +}; +} + +macro_rules! outer { +($d:tt) => { +macro_rules! inner { +($d s:expr) => { +println!("{}", $d s); +} +} +}; +} + +fn main() { +macro_rules! outer { +($ d:tt) => { +macro_rules! inner { +($ d s:expr) => { +println!("INNER1: {}", $d s); +println!("INNER2: {}", $ s); +} +} +}; +} + +outer!($); + +inner!("hi"); +} + +// Consecutive identities in macro body + +fn main() { +macro_rules! uniop { +($op:tt, $s:expr) => { +$op $s +}; +} +let x = uniop!(!, true); +println!("{}", x); +let x = uniop!(-, 7); +println!("{}", x); +} + +fn main() { +macro_rules! binop { +($l:expr, $op:tt, $r:expr) => { +$l $op $r +}; +} +let x = binop!(10, -, 7); +println!("{}", x); +let x = binop!(10, +, 7); +println!("{}", x); +} diff --git a/tests/target/issue-4609/one.rs b/tests/target/issue-4609/one.rs new file mode 100644 index 00000000000..aecee0d7a4d --- /dev/null +++ b/tests/target/issue-4609/one.rs @@ -0,0 +1,94 @@ +// rustfmt-version: One + +// Original issue - parameters in macro call + +macro_rules! outer { + ($d:tt) => { + macro_rules! inner { + ($d s:expr) => { + println!("{}", $d s); + }; + } + }; +} + +outer!($); + +fn main() { + inner!("hi"); +} + +macro_rules! outer { + ($d:tt) => { + macro_rules! inner { + ($d s:expr) => { + println!("{}", $d s); + }; + } + }; +} + +// Variations of the original issue + +macro_rules! outer { + ($d:tt) => { + macro_rules! inner { + ($d s:expr) => { + println!("{}", $d s); + }; + } + }; +} + +macro_rules! outer { + ($d:tt) => { + macro_rules! inner { + ($d s:expr) => { + println!("{}", $d s); + }; + } + }; +} + +fn main() { + macro_rules! outer { + ($ d:tt) => { + macro_rules! inner { + ($ d s:expr) => { + println!("INNER1: {}", $d s); + println!("INNER2: {}", $ s); + }; + } + }; + } + + outer!($); + + inner!("hi"); +} + +// Consecutive identities in macro body + +fn main() { + macro_rules! uniop { + ($op:tt, $s:expr) => { + $op $s + }; + } + let x = uniop!(!, true); + println!("{}", x); + let x = uniop!(-, 7); + println!("{}", x); +} + +fn main() { + macro_rules! binop { + ($l:expr, $op:tt, $r:expr) => { + $l $op $r + }; + } + let x = binop!(10, -, 7); + println!("{}", x); + let x = binop!(10, +, 7); + println!("{}", x); +} diff --git a/tests/target/issue-4609/two.rs b/tests/target/issue-4609/two.rs new file mode 100644 index 00000000000..e8d9b61054f --- /dev/null +++ b/tests/target/issue-4609/two.rs @@ -0,0 +1,94 @@ +// rustfmt-version: Two + +// Original issue - parameters in macro call + +macro_rules! outer { + ($d:tt) => { + macro_rules! inner { + ($d s:expr) => { + println!("{}", $d s); + }; + } + }; +} + +outer!($); + +fn main() { + inner!("hi"); +} + +macro_rules! outer { + ($d:tt) => { + macro_rules! inner { + ($d s:expr) => { + println!("{}", $d s); + }; + } + }; +} + +// Variations of the original issue + +macro_rules! outer { + ($d:tt) => { + macro_rules! inner { + ($d s:expr) => { + println!("{}", $d s); + }; + } + }; +} + +macro_rules! outer { + ($d:tt) => { + macro_rules! inner { + ($d s:expr) => { + println!("{}", $d s); + }; + } + }; +} + +fn main() { + macro_rules! outer { + ($ d:tt) => { + macro_rules! inner { + ($ d s:expr) => { + println!("INNER1: {}", $d s); + println!("INNER2: {}", $ s); + }; + } + }; + } + + outer!($); + + inner!("hi"); +} + +// Consecutive identities in macro body + +fn main() { + macro_rules! uniop { + ($op:tt, $s:expr) => { + $op $s + }; + } + let x = uniop!(!, true); + println!("{}", x); + let x = uniop!(-, 7); + println!("{}", x); +} + +fn main() { + macro_rules! binop { + ($l:expr, $op:tt, $r:expr) => { + $l $op $r + }; + } + let x = binop!(10, -, 7); + println!("{}", x); + let x = binop!(10, +, 7); + println!("{}", x); +}