Merge #4139

4139: Add check for rules that no repetition which could match an empty token r=matklad a=edwin0cheng

Fix #4103

for `/ui/issues/issue-57597.rs`

This is `ParseError` of the macro rules , because it is how rustc handle it : 

https://github.com/rust-lang/rust/blob/a58b1ed44f5e06976de2bdc4d7dc81c36a96934f/src/librustc_expand/mbe/macro_rules.rs#L558


Co-authored-by: Edwin Cheng <[email protected]>

2 files changed, 75 insertions, 12 deletions

diff --git a/crates/ra_mbe/src/lib.rs b/crates/ra_mbe/src/lib.rs
index 535b7daa0..1a020398e 100644
--- a/crates/ra_mbe/src/lib.rs
+++ b/crates/ra_mbe/src/lib.rs
@@ -19,6 +19,7 @@ use crate::{
 #[derive(Debug, PartialEq, Eq)]
 pub enum ParseError {
     Expected(String),
+    RepetitionEmtpyTokenTree,
 }
 
 #[derive(Debug, PartialEq, Eq)]
@@ -194,20 +195,46 @@ impl Rule {
     }
 }
 
+fn to_parse_error(e: ExpandError) -> ParseError {
+    let msg = match e {
+        ExpandError::InvalidRepeat => "invalid repeat".to_string(),
+        _ => "invalid macro definition".to_string(),
+    };
+    ParseError::Expected(msg)
+}
+
 fn validate(pattern: &tt::Subtree) -> Result<(), ParseError> {
     for op in parse_pattern(pattern) {
-        let op = match op {
-            Ok(it) => it,
-            Err(e) => {
-                let msg = match e {
-                    ExpandError::InvalidRepeat => "invalid repeat".to_string(),
-                    _ => "invalid macro definition".to_string(),
-                };
-                return Err(ParseError::Expected(msg));
-            }
-        };
+        let op = op.map_err(to_parse_error)?;
+
         match op {
-            Op::TokenTree(tt::TokenTree::Subtree(subtree)) | Op::Repeat { subtree, .. } => {
+            Op::TokenTree(tt::TokenTree::Subtree(subtree)) => validate(subtree)?,
+            Op::Repeat { subtree, separator, .. } => {
+                // Checks that no repetition which could match an empty token
+                // https://github.com/rust-lang/rust/blob/a58b1ed44f5e06976de2bdc4d7dc81c36a96934f/src/librustc_expand/mbe/macro_rules.rs#L558
+
+                if separator.is_none() {
+                    if parse_pattern(subtree).all(|child_op| {
+                        match child_op.map_err(to_parse_error) {
+                            Ok(Op::Var { kind, .. }) => {
+                                // vis is optional
+                                if kind.map_or(false, |it| it == "vis") {
+                                    return true;
+                                }
+                            }
+                            Ok(Op::Repeat { kind, .. }) => {
+                                return matches!(
+                                    kind,
+                                    parser::RepeatKind::ZeroOrMore | parser::RepeatKind::ZeroOrOne
+                                )
+                            }
+                            _ => {}
+                        }
+                        false
+                    }) {
+                        return Err(ParseError::RepetitionEmtpyTokenTree);
+                    }
+                }
                 validate(subtree)?
             }
             _ => (),
@@ -216,6 +243,7 @@ fn validate(pattern: &tt::Subtree) -> Result<(), ParseError> {
     Ok(())
 }
 
+#[derive(Debug)]
 pub struct ExpandResult<T>(pub T, pub Option<ExpandError>);
 
 impl<T> ExpandResult<T> {
diff --git a/crates/ra_mbe/src/tests.rs b/crates/ra_mbe/src/tests.rs
index 7ceec7752..0d924ce58 100644
--- a/crates/ra_mbe/src/tests.rs
+++ b/crates/ra_mbe/src/tests.rs
@@ -1657,7 +1657,7 @@ impl MacroFixture {
     }
 }
 
-pub(crate) fn parse_macro(ra_fixture: &str) -> MacroFixture {
+fn parse_macro_to_tt(ra_fixture: &str) -> tt::Subtree {
     let source_file = ast::SourceFile::parse(ra_fixture).ok().unwrap();
     let macro_definition =
         source_file.syntax().descendants().find_map(ast::MacroCall::cast).unwrap();
@@ -1671,10 +1671,24 @@ pub(crate) fn parse_macro(ra_fixture: &str) -> MacroFixture {
     .0;
     assert_eq!(definition_tt, parsed);
 
+    definition_tt
+}
+
+pub(crate) fn parse_macro(ra_fixture: &str) -> MacroFixture {
+    let definition_tt = parse_macro_to_tt(ra_fixture);
     let rules = MacroRules::parse(&definition_tt).unwrap();
     MacroFixture { rules }
 }
 
+pub(crate) fn parse_macro_error(ra_fixture: &str) -> ParseError {
+    let definition_tt = parse_macro_to_tt(ra_fixture);
+
+    match MacroRules::parse(&definition_tt) {
+        Ok(_) => panic!("Expect error"),
+        Err(err) => err,
+    }
+}
+
 pub(crate) fn parse_to_token_tree_by_syntax(ra_fixture: &str) -> tt::Subtree {
     let source_file = ast::SourceFile::parse(ra_fixture).ok().unwrap();
     let tt = syntax_node_to_token_tree(source_file.syntax()).unwrap().0;
@@ -1840,6 +1854,27 @@ fn test_no_space_after_semi_colon() {
     );
 }
 
+// https://github.com/rust-lang/rust/blob/master/src/test/ui/issues/issue-57597.rs
+#[test]
+fn test_rustc_issue_57597() {
+    fn test_error(fixture: &str) {
+        assert_eq!(parse_macro_error(fixture), ParseError::RepetitionEmtpyTokenTree);
+    }
+
+    test_error("macro_rules! foo { ($($($i:ident)?)+) => {}; }");
+    test_error("macro_rules! foo { ($($($i:ident)?)*) => {}; }");
+    test_error("macro_rules! foo { ($($($i:ident)?)?) => {}; }");
+    test_error("macro_rules! foo { ($($($($i:ident)?)?)?) => {}; }");
+    test_error("macro_rules! foo { ($($($($i:ident)*)?)?) => {}; }");
+    test_error("macro_rules! foo { ($($($($i:ident)?)*)?) => {}; }");
+    test_error("macro_rules! foo { ($($($($i:ident)?)?)*) => {}; }");
+    test_error("macro_rules! foo { ($($($($i:ident)*)*)?) => {}; }");
+    test_error("macro_rules! foo { ($($($($i:ident)?)*)*) => {}; }");
+    test_error("macro_rules! foo { ($($($($i:ident)?)*)+) => {}; }");
+    test_error("macro_rules! foo { ($($($($i:ident)+)?)*) => {}; }");
+    test_error("macro_rules! foo { ($($($($i:ident)+)*)?) => {}; }");
+}
+
 #[test]
 fn test_expand_bad_literal() {
     parse_macro(