Parse trailing commas in Cedar policy files (cedar-policy#1606)

Signed-off-by: Mats Jun Larsen <[email protected]>
Signed-off-by: Mish Jude <[email protected]>

Author: junlarsen

cedar-policy-core/src/parser.rs

@@ -1190,20 +1190,20 @@ mod tests {
             "permit(principal,",
             "unexpected end of input",
             "",
-            "expected identifier",
+            "expected `)` or identifier",
         );
         assert_labeled_span(
             "permit(principal,action,",
             "unexpected end of input",
             "",
-            "expected identifier",
+            "expected `)` or identifier",
         );
         // Nothing will actually convert to an AST here.
         assert_labeled_span(
             "permit(principal,action,resource,",
             "unexpected end of input",
             "",
-            "expected identifier",
+            "expected `)` or identifier",
         );
         // We still list out `if` as an expected token because it doesn't get
         // parsed as an ident in this position.

cedar-policy-core/src/parser/grammar.lalrpop

@@ -85,11 +85,13 @@ match {
 }
 
 Comma<E>: Vec<E> = {
-    <e:E?> => e.into_iter().collect(),
-    <mut es:(<E> ",")+> <e:E> => {
-        es.push(e);
-        es
-    },
+    <mut es:(<E> ",")*> <e:E?> => match e {
+        None => es,
+        Some(e) => {
+            es.push(e);
+            es
+        }
+    }
 }
 
 // Policies := {Policy}

cedar-policy-core/src/parser/text_to_cst.rs

@@ -1255,7 +1255,7 @@ mod tests {
             src,
             &errs,
             &ExpectedErrorMessageBuilder::error("unexpected token `!`")
-                .exactly_one_underline_with_label("!", "expected identifier")
+                .exactly_one_underline_with_label("!", "expected `)` or identifier")
                 .build(),
         );
 
@@ -1269,14 +1269,14 @@ mod tests {
             src,
             &errs,
             &ExpectedErrorMessageBuilder::error("unexpected token `!`")
-                .exactly_one_underline_with_label("!", "expected identifier")
+                .exactly_one_underline_with_label("!", "expected `)` or identifier")
                 .build(),
         );
         expect_some_error_matches(
             src,
             &errs,
             &ExpectedErrorMessageBuilder::error("unexpected token `+`")
-                .exactly_one_underline_with_label("+", "expected identifier")
+                .exactly_one_underline_with_label("+", "expected `)` or identifier")
                 .build(),
         );
         expect_n_errors(src, &errs, 2);
@@ -1290,7 +1290,7 @@ mod tests {
             src,
             &errs,
             &ExpectedErrorMessageBuilder::error("unexpected token `!`")
-                .exactly_one_underline_with_label("!", "expected identifier")
+                .exactly_one_underline_with_label("!", "expected `)` or identifier")
                 .build(),
         );
     }
@@ -1721,6 +1721,20 @@ mod tests {
         }
     }
 
+    #[test]
+    fn trailing_comma() {
+        assert_parse_succeeds(
+            parse_policy,
+            r#"
+        permit(principal, action, resource,);
+        "#,
+        );
+        assert_parse_succeeds(parse_expr, r#"foo(a, b, c,)"#);
+        assert_parse_succeeds(parse_expr, r#"[A, B, C,]"#);
+        assert_parse_succeeds(parse_expr, r#"{ A: B, C: D, }"#);
+        assert_parse_succeeds(parse_ref, r#"Principal::{uid: "123", role: "admin",}"#);
+    }
+
     #[test]
     #[cfg(feature = "tolerant-ast")]
     fn policies_tolerant_success() {

cedar-policy/CHANGELOG.md

@@ -28,6 +28,8 @@ Cedar Language Version: TBD
   `PolicySet`, `Policy`, and `Template`. These APIs don't retain source code information, trading detailed error
   reporting for reduced memory usage and faster parsing. The feature is only intended for use with raw parsing
   APIs, as regular parsing performance is degraded when the `raw-parsing` feature is enabled.
+- Implemented [RFC 71 (trailing commas)](https://github.com/cedar-policy/rfcs/blob/main/text/0071-trailing-commas.md)
+  for Cedar policy files. (#1606)
 
 ### Changed