Reject unparenthesized trailing-comma tuples in the LALRPOP parser

Summary:
Make the current LALRPOP parser reject spec-invalid unparenthesized tuple forms with trailing commas, while still allowing parenthesized tuples such as `(1,)`.

This follows the Starlark language spec:
https://starlark-lang.org/spec.html
which says that "Starlark, unlike Python, does not permit a trailing comma to appear in an unparenthesized tuple expression".

It also aligns with the upstream starlark-go parser tests in:
https://github.com/google/starlark-go/blob/master/syntax/testdata/errors.star
which cover `_ = 1,`, `a, b, = 1, 2`, and `a, b = 1, 2,`.

Add matching regressions for the current parser, including `for` and comprehension target cases that exercise the same rule through assignment targets. Update the unmatched-parens golden to reflect the broader `")" or ","` expectation introduced by the grammar split.

An Eden-backed exhaustive `BUCK`/`TARGETS` parse sweep found one real in-repo user of the old bug in `fbcode/third-party-source/java/com.google.tsunami/BUCK`, where a commented-out rule leaves `test_type = "junit",` as a top-level assignment. That follow-up is tracked separately in D100062221.

Reviewed By: cjhopman

Differential Revision: D100064428

fbshipit-source-id: 3d64085fe6e8b9e6cdff3c28f088bf8341886339

Author: RiskRunner0

starlark_syntax/src/syntax/grammar.lalrpop

@@ -221,7 +221,7 @@ LoadStmtSyms: (AstAssignIdent, AstString) = <id:LoadStmtBindingName?> <n:string>
 };
 
 // Expression
-L<E>: AstExpr = <l:@L> <v:(<E> ",")*> <e:E> <f:","?> <r:@R>
+ParenTuple<E>: AstExpr = <l:@L> <v:(<E> ",")*> <e:E> <f:","?> <r:@R>
     => {
         if f.is_some() || !v.is_empty() {
             Expr::Tuple(v.into_iter().chain(vec![e].into_iter()).collect())
@@ -231,9 +231,19 @@ L<E>: AstExpr = <l:@L> <v:(<E> ",")*> <e:E> <f:","?> <r:@R>
         }
     };
 
-ExprList: AstExpr = L<Expr>;
+UnparenTuple<E>: AstExpr = {
+    <l:@L> <e:E> "," <r:@R>
+        =>? Ok(grammar_util::reject_unparenthesized_tuple_trailing_comma(state.codemap, l, r)?),
+    <l:@L> <v:(<E> ",")+> <e:E> "," <r:@R>
+        =>? Ok(grammar_util::reject_unparenthesized_tuple_trailing_comma(state.codemap, l, r)?),
+    <l:@L> <v:(<E> ",")+> <e:E> <r:@R>
+        => Expr::Tuple(v.into_iter().chain(vec![e].into_iter()).collect()).ast(l, r),
+    E,
+};
+
+ExprList: AstExpr = UnparenTuple<Expr>;
 
-TestList: AstExpr = L<Test>;
+TestList: AstExpr = UnparenTuple<Test>;
 
 
 PrimaryExpr: AstExpr = {
@@ -282,7 +292,7 @@ Operand: AstExpr = {
     <l:@L> "{" <e:COMMA<DictEntry>> "}" <r:@R>
         => Expr::Dict(e).ast(l, r),
     DictComp,
-    <l:@L> "(" <e:TestList?> ")" <r:@R>
+    <l:@L> "(" <e:ParenTuple<Test>?> ")" <r:@R>
         => match e {
             Some(t) => t,
             None => Expr::Tuple(vec![]).ast(l, r)

starlark_syntax/src/syntax/grammar_tests.rs

@@ -752,6 +752,21 @@ fn test_if_elif_span_covers_full_chain() {
         "if True:\n  a\nelif False:\n  b\nelse:\n  c",
     );
 }
+
+#[test]
+fn test_error_tuple_trailing_comma() {
+    parse_fails(
+        "error_tuple_trailing_comma",
+        &[
+            "_ = 1,",
+            "for k, v, in dict.items():\n  pass",
+            "_ = [(v, k) for k, v, in dict.items()]",
+            "a, b, = 1, 2",
+            "a, b = 1, 2,",
+        ],
+    );
+}
+
 pub fn parse(program: &str) -> String {
     parse_ast(program).statement.to_string()
 }

starlark_syntax/src/syntax/grammar_tests/error_tuple_trailing_comma.golden

@@ -0,0 +1,65 @@
+# @generated
+# To regenerate, run:
+# ```
+# STARLARK_RUST_REGENERATE_GOLDEN_TESTS=1 cargo test -p starlark --lib
+# ```
+
+Program:
+_ = 1,
+
+Error:
+error: unparenthesized tuple with trailing comma
+ --> error_tuple_trailing_comma:1:5
+  |
+1 | _ = 1,
+  |     ^^
+  |
+
+
+Program:
+for k, v, in dict.items():
+  pass
+
+Error:
+error: unparenthesized tuple with trailing comma
+ --> error_tuple_trailing_comma:1:5
+  |
+1 | for k, v, in dict.items():
+  |     ^^^^^
+  |
+
+
+Program:
+_ = [(v, k) for k, v, in dict.items()]
+
+Error:
+error: unparenthesized tuple with trailing comma
+ --> error_tuple_trailing_comma:1:17
+  |
+1 | _ = [(v, k) for k, v, in dict.items()]
+  |                 ^^^^^
+  |
+
+
+Program:
+a, b, = 1, 2
+
+Error:
+error: unparenthesized tuple with trailing comma
+ --> error_tuple_trailing_comma:1:1
+  |
+1 | a, b, = 1, 2
+  | ^^^^^
+  |
+
+
+Program:
+a, b = 1, 2,
+
+Error:
+error: unparenthesized tuple with trailing comma
+ --> error_tuple_trailing_comma:1:8
+  |
+1 | a, b = 1, 2,
+  |        ^^^^^
+  |

starlark_syntax/src/syntax/grammar_tests/error_unexpected_token.golden

@@ -129,7 +129,7 @@ Program:
 a = max(range(10)))
 
 Error:
-error: Parse error: unexpected symbol ')' here, expected one of "\n" or ";"
+error: Parse error: unexpected symbol ')' here, expected one of "\n", "," or ";"
  --> error_unexpected_token:1:19
   |
 1 | a = max(range(10)))

starlark_syntax/src/syntax/grammar_tests/error_unmatched_parens.golden

@@ -8,7 +8,7 @@ Program:
 (1 + 2
 
 Error:
-error: Parse error: unexpected new line here, expected one of ")"
+error: Parse error: unexpected new line here, expected one of ")" or ","
  --> error_unmatched_parens:1:7
   |
 1 | (1 + 2
@@ -58,7 +58,7 @@ x = (1 +
   3
 
 Error:
-error: Parse error: unexpected new line here, expected one of ")"
+error: Parse error: unexpected new line here, expected one of ")" or ","
  --> error_unmatched_parens:3:4
   |
 3 |   3

starlark_syntax/src/syntax/grammar_util.rs

@@ -68,6 +68,8 @@ enum GrammarUtilError {
     TypeAnnotationOnTupleAssign,
     #[error("`load` statement requires at least two arguments")]
     LoadRequiresAtLeastTwoArguments,
+    #[error("unparenthesized tuple with trailing comma")]
+    UnparenthesizedTupleTrailingComma,
 }
 
 /// Ensure we produce normalised Statements, rather than singleton Statements
@@ -146,6 +148,18 @@ pub fn check_assignment(
     })
 }
 
+pub(crate) fn reject_unparenthesized_tuple_trailing_comma<T>(
+    codemap: &CodeMap,
+    begin: usize,
+    end: usize,
+) -> Result<T, EvalException> {
+    Err(EvalException::new_anyhow(
+        GrammarUtilError::UnparenthesizedTupleTrailingComma.into(),
+        Span::new(Pos::new(begin as _), Pos::new(end as _)),
+        codemap,
+    ))
+}
+
 pub(crate) fn check_load_0(module: AstString, parser_state: &mut ParserState) -> Stmt {
     parser_state.errors.push(EvalException::new_anyhow(
         GrammarUtilError::LoadRequiresAtLeastTwoArguments.into(),