fix(semantic): reject a trailing macro repetition separator like rustc

corelib/src/test/language_features/macro_test.cairo

@@ -211,22 +211,30 @@ mod repetition_macro_matcher {
         ($($x:ident) *) => { 444 };
     }
 
+    // A separator only ever stands between two groups, so a trailing one is not part of the
+    // repetition and the calls above reject it - as rustc's `macro_rules!` does. A macro that wants
+    // to accept a trailing separator asks for it, and `$(,)?` is how rustc spells "an optional
+    // trailing comma".
+    macro matcher_trailing_comma {
+        ($($x:expr),* $(,)?) => { 555 };
+    }
+
     #[test]
     fn repetition_macro_matcher() {
         assert_eq!(matcher_plus!(1), 111);
         assert_eq!(matcher_plus!(1, 2), 111);
-        #[cairofmt::skip]
-        assert_eq!(matcher_plus!(1, 2,), 111);
-        #[cairofmt::skip]
-        assert_eq!(matcher_plus!(1,), 111);
 
         assert_eq!(matcher_star!(), 222);
         assert_eq!(matcher_star!(3), 222);
         assert_eq!(matcher_star!(4, 5), 222);
+
+        assert_eq!(matcher_trailing_comma!(), 555);
+        assert_eq!(matcher_trailing_comma!(1), 555);
+        assert_eq!(matcher_trailing_comma!(4, 5), 555);
         #[cairofmt::skip]
-        assert_eq!(matcher_star!(3,), 222);
+        assert_eq!(matcher_trailing_comma!(3,), 555);
         #[cairofmt::skip]
-        assert_eq!(matcher_star!(4, 5,), 222);
+        assert_eq!(matcher_trailing_comma!(4, 5,), 555);
 
         assert_eq!(matcher_optional!(), 333);
         assert_eq!(matcher_optional!(9), 333);

crates/cairo-lang-semantic/src/expr/expansion_test_data/inline_macros

@@ -1131,3 +1131,58 @@ error[E2200]: Plugin diagnostic: Macro cannot be parsed as legacy macro. Expecte
  --> lib.cairo:12:16
 let expanded = m!(A and B);
                ^^^^^^^^^^^
+
+//! > ==========================================================================
+
+//! > Test a trailing separator picking the rule that asks for it.
+
+//! > test_runner_name
+test_expand_expr(expect_diagnostics: false)
+
+//! > module_code
+// The two rules differ only in where the comma sits: rule 1 declares it as the repetition's
+// separator, so it only ever stands between two groups, while rule 2 makes it part of the group's
+// own body, so every group carries one - which is how rustc spells trailing-separator tolerance.
+// A call with a trailing comma therefore falls through rule 1 to rule 2, and one without it stops
+// at rule 1. This call used to stop at rule 1 and expand to `1`. rustc's `macro_rules!` picks rule
+// 2 for it as well, printing `2`.
+#[feature("user_defined_inline_macros")]
+macro m {
+    ($($x:expr),*) => { 1 };
+
+    ($($x:expr,)*) => { 2 };
+}
+
+//! > expr_code
+let expanded = m!(1, 2,);
+
+//! > expanded_code
+expanded: 2
+
+//! > diagnostics
+
+//! > ==========================================================================
+
+//! > Test a trailing `;` separator picking the rule that asks for it.
+
+//! > test_runner_name
+test_expand_expr(expect_diagnostics: false)
+
+//! > module_code
+// The comma case above with `;` as the separator, to pin that the rule choice is driven by the
+// declared separator rather than by the comma being special. This call used to stop at rule 1 and
+// expand to `1`. rustc's `macro_rules!` picks rule 2 for it as well, printing `2`.
+#[feature("user_defined_inline_macros")]
+macro m {
+    ($($x:expr);*) => { 1 };
+
+    ($($x:expr;)*) => { 2 };
+}
+
+//! > expr_code
+let expanded = m!(1; 2;);
+
+//! > expanded_code
+expanded: 2
+
+//! > diagnostics

crates/cairo-lang-semantic/src/expr/test_data/inline_macros

@@ -3011,29 +3011,44 @@ fn foo() -> felt252 {
 
 //! > ==========================================================================
 
-//! > Test a trailing separator absorbed by a repetition (deliberate divergence from rustc).
+//! > Test a trailing separator rejected by a repetition, as rustc does.
 
 //! > test_runner_name
-test_function_diagnostics(expect_diagnostics: false)
+test_function_diagnostics(expect_diagnostics: true)
 
 //! > cairo_code
-// The trailing separator is absorbed by the repetition, and the tuple type pins that it does not
-// add an empty match. rustc's `macro_rules!` rejects the equivalent call, and aligning with it is a
-// separate, deliberate decision.
+// A separator only stands between two groups, so the trailing one belongs to no group and is left
+// over - and nothing in the rule can match it. rustc's `macro_rules!` rejects the two-element call
+// with "unexpected end of macro invocation" and the lone-separator call with "no rules expected
+// `,`". The two-element call is the one that changed: it used to be accepted, with the trailing
+// separator absorbed.
 macro m {
     ($($x:ident),*) => { ($($x),*) };
 }
 fn foo() {
-    let long_named_first_argument_of_the_macro_call: felt252 = 1;
-    let long_named_second_argument_of_the_macro_call: felt252 = 2;
-    // The call is spread over several lines on purpose: the formatter strips a trailing separator
-    // from a single-line macro call, which would silently drop what this test pins.
-    let _t: (felt252, felt252) = m!(
-        long_named_first_argument_of_the_macro_call, long_named_second_argument_of_the_macro_call,
-    );
+    let a: felt252 = 1;
+    let b: felt252 = 2;
+    // The `cairofmt::skip` is load-bearing: the formatter strips a trailing comma from a
+    // single-line macro call, which would silently drop what this test pins.
+    #[cairofmt::skip]
+    let _t = m!(a, b,);
+    // A lone separator is not a group either, so it is rejected for the same reason.
+    let _u = m!(,);
+    // Neither rejection is about the call having two elements or none: the same rule accepts these.
+    let _v: (felt252, felt252) = m!(a, b);
+    let _w: () = m!();
 }
 
 //! > expected_diagnostics
+error[E2158]: No matching rule found in inline macro `m`.
+ --> lib.cairo:15:14
+    let _t = m!(a, b,);
+             ^^^^^^^^^
+
+error[E2158]: No matching rule found in inline macro `m`.
+ --> lib.cairo:17:14
+    let _u = m!(,);
+             ^^^^^
 
 //! > ==========================================================================
 
@@ -4021,3 +4036,99 @@ error[E2209]: Macro placeholder 'x' captures an expression, so the pattern may o
  --> lib.cairo:6:16
     ($($x:expr)|*) => { 0 $(+ $x)* };
                ^
+
+//! > ==========================================================================
+
+//! > Test a trailing `;` separator rejected by a repetition, as rustc does.
+
+//! > test_runner_name
+test_function_diagnostics(expect_diagnostics: true)
+
+//! > cairo_code
+// The separator being `;` rather than `,` changes nothing: a trailing one still belongs to no
+// group. rustc's `macro_rules!` rejects the two-element call with "unexpected end of macro
+// invocation" and the lone-separator call with "no rules expected `;`". The two-element call is the
+// one that changed: it used to be accepted, with the trailing separator absorbed.
+macro m {
+    ($($x:ident);*) => { ($($x),*) };
+}
+fn foo() {
+    let a: felt252 = 1;
+    let b: felt252 = 2;
+    // Unlike a trailing comma, a trailing `;` survives the formatter, so no `cairofmt::skip` here.
+    let _t = m!(a; b;);
+    // A lone separator is not a group either, so it is rejected for the same reason.
+    let _u = m!(;);
+    // Neither rejection is about the call having two elements or none: the same rule accepts these.
+    let _v: (felt252, felt252) = m!(a; b);
+    let _w: () = m!();
+}
+
+//! > expected_diagnostics
+error[E2158]: No matching rule found in inline macro `m`.
+ --> lib.cairo:12:14
+    let _t = m!(a; b;);
+             ^^^^^^^^^
+
+error[E2158]: No matching rule found in inline macro `m`.
+ --> lib.cairo:14:14
+    let _u = m!(;);
+             ^^^^^
+
+//! > ==========================================================================
+
+//! > Test which rule a trailing separator picks, across every group count.
+
+//! > test_runner_name
+test_function_diagnostics(expect_diagnostics: false)
+
+//! > cairo_code
+// Rule 1 declares the comma as the repetition's separator, so it only ever stands between two
+// groups; rule 2 makes it part of the group body, so every group carries one - which is how rustc
+// spells trailing-separator tolerance. rustc's `macro_rules!` prints `(1, 2, 2, 1)` for these four
+// calls against the equivalent macro. The two trailing-comma calls are what changed: both used to
+// stop at rule 1.
+macro m {
+    ($($x:expr),*) => { 1_u8 };
+
+    ($($x:expr,)*) => { 2_u16 };
+}
+fn foo() {
+    // Each annotation is the assertion: rule 1 yields a `u8` and rule 2 a `u16`, so a call that
+    // picked the other rule would be a type mismatch here. The two `cairofmt::skip`s keep the
+    // formatter from stripping the trailing comma this test is about.
+    let _a: u8 = m!(1, 2);
+    #[cairofmt::skip]
+    let _b: u16 = m!(1, 2,);
+    #[cairofmt::skip]
+    let _c: u16 = m!(1,);
+    let _d: u8 = m!();
+}
+
+//! > expected_diagnostics
+
+//! > ==========================================================================
+
+//! > Test which rule a trailing `;` separator picks, across every group count.
+
+//! > test_runner_name
+test_function_diagnostics(expect_diagnostics: false)
+
+//! > cairo_code
+// The comma case above with `;` as the separator, to pin that the rule choice is driven by the
+// declared separator rather than by the comma being special. rustc's `macro_rules!` prints
+// `(1, 2, 2, 1)` for these four calls against the equivalent macro. The two trailing-`;` calls are
+// what changed: both used to stop at rule 1.
+macro m {
+    ($($x:expr);*) => { 1_u8 };
+
+    ($($x:expr;)*) => { 2_u16 };
+}
+fn foo() {
+    let _a: u8 = m!(1; 2);
+    let _b: u16 = m!(1; 2;);
+    let _c: u16 = m!(1;);
+    let _d: u8 = m!();
+}
+
+//! > expected_diagnostics

crates/cairo-lang-semantic/src/items/macro_declaration.rs

@@ -954,6 +954,23 @@ fn is_macro_rule_match_ex<'db>(
                 loop {
                     let mut inner_ctx = ctx.clone();
                     let mut temp_iter = input_iter.clone();
+                    // A separator only ever stands *between* two groups, so from the second group
+                    // on it has to be consumed before the group - and only together with it. A
+                    // separator that no group follows is not part of the repetition, and is left
+                    // in the input for whatever comes after it to match, exactly as rustc's
+                    // `macro_rules!` does.
+                    if match_count > 0
+                        && let Some(expected_sep) = &expected_separator
+                    {
+                        let Some(ast::TokenTree::Token(token_leaf)) = temp_iter.peek() else {
+                            break;
+                        };
+                        if token_leaf.as_syntax_node().get_text_without_trivia(db) != *expected_sep
+                        {
+                            break;
+                        }
+                        temp_iter.next();
+                    }
                     let Some(true) = is_macro_rule_match_ex(
                         db,
                         elements.clone(),
@@ -967,18 +984,6 @@ fn is_macro_rule_match_ex<'db>(
                     *ctx = inner_ctx;
                     *input_iter = temp_iter;
                     match_count += 1;
-                    if let Some(expected_sep) = &expected_separator {
-                        if let Some(ast::TokenTree::Token(token_leaf)) = input_iter.peek() {
-                            let actual = token_leaf.as_syntax_node().get_text_without_trivia(db);
-                            if actual == *expected_sep {
-                                input_iter.next();
-                            } else {
-                                break;
-                            }
-                        } else {
-                            break;
-                        }
-                    }
                 }
                 ctx.repetition_match_counts.insert(rep_id, match_count);
                 ctx.repetition_operators.insert(rep_id, operator.clone());