From a1c9bbe776eb1d66a9ad906c5f59ca01461a6aa8 Mon Sep 17 00:00:00 2001
From: Zack Slayton <zslayton@amazon.com>
Date: Thu, 15 Aug 2024 17:01:29 -0400
Subject: [PATCH 1/2] allow ? and * parameters to be omitted in tail position

---
 src/lazy/expanded/template.rs |  7 ++++++
 src/lazy/text/buffer.rs       | 41 +++++++++++++++++++++++++++++------
 2 files changed, 41 insertions(+), 7 deletions(-)

diff --git a/src/lazy/expanded/template.rs b/src/lazy/expanded/template.rs
index 48f12210..8bf0d295 100644
--- a/src/lazy/expanded/template.rs
+++ b/src/lazy/expanded/template.rs
@@ -49,6 +49,13 @@ impl Parameter {
     pub fn is_variadic(&self) -> bool {
         !matches!(self.cardinality, ParameterCardinality::ExactlyOne)
     }
+    /// Returns true if a text e-expression can omit this argument.
+    ///
+    /// Arguments for parameters with a cardinality of zero-or-one (`?`) or zero-or-more (`*`) can
+    /// be omitted if there are no other arguments being passed.
+    pub fn can_be_omitted(&self) -> bool {
+        matches!(self.cardinality, ParameterCardinality::ZeroOrOne | ParameterCardinality::ZeroOrMore)
+    }
 }
 
 /// The encoding used to serialize and deserialize the associated parameter.
diff --git a/src/lazy/text/buffer.rs b/src/lazy/text/buffer.rs
index 825af328..87b44148 100644
--- a/src/lazy/text/buffer.rs
+++ b/src/lazy/text/buffer.rs
@@ -1142,7 +1142,7 @@ impl<'top> TextBufferView<'top> {
                 Some(arg) => arg_expr_cache.push(arg),
                 None => {
                     for param in &signature_params[index..] {
-                        if param.rest_syntax_policy() == RestSyntaxPolicy::NotAllowed {
+                        if !param.can_be_omitted() {
                             return fatal_parse_error(
                                 self,
                                 format!(
@@ -1242,8 +1242,16 @@ impl<'top> TextBufferView<'top> {
         }
     }
 
-    pub fn match_empty_arg_group(self) -> IonMatchResult<'top> {
-        recognize(pair(tag("(:"), whitespace_and_then(tag(")"))))(self)
+    pub fn match_empty_arg_group(
+        self,
+        parameter: &'top Parameter,
+    ) -> IonParseResult<'top, EExpArg<'top, TextEncoding_1_1>> {
+        recognize(pair(tag("(:"), whitespace_and_then(tag(")"))))
+            .map(|matched_expr| {
+                let arg_group = TextEExpArgGroup::new(parameter, matched_expr, &[]);
+                EExpArg::new(parameter, EExpArgExpr::ArgGroup(arg_group))
+            })
+            .parse(self)
     }
 
     pub fn match_zero_or_one(
@@ -1251,7 +1259,7 @@ impl<'top> TextBufferView<'top> {
         parameter: &'top Parameter,
     ) -> IonParseResult<'top, Option<EExpArg<'top, TextEncoding_1_1>>> {
         whitespace_and_then(alt((
-            Self::match_empty_arg_group.map(|_| None),
+            Self::parser_with_arg(Self::match_empty_arg_group, parameter).map(Some),
             // TODO: Match a non-empty arg group and turn it into a failure with a helpful error message
             Self::match_sexp_value_1_1.map(|maybe_expr| {
                 maybe_expr.map(|expr| {
@@ -1285,7 +1293,7 @@ impl<'top> TextBufferView<'top> {
         self,
         parameter: &'top Parameter,
     ) -> IonParseResult<'top, Option<EExpArg<'top, TextEncoding_1_1>>> {
-        if self.match_empty_arg_group().is_ok() {
+        if self.match_empty_arg_group(parameter).is_ok() {
             return Err(nom::Err::Failure(IonParseError::Invalid(
                 InvalidInputError::new(self).with_description(format!(
                     "parameter '{}' is one-or-more (`+`) and cannot accept an empty stream",
@@ -2660,8 +2668,8 @@ mod tests {
     }
 
     macro_rules! matcher_tests_with_macro {
-        ($parser:ident $macro_src:literal $($expect:ident: [$($input:literal),+$(,)?]),+$(,)?) => {
-            mod $parser {
+        ($mod_name:ident $parser:ident $macro_src:literal $($expect:ident: [$($input:literal),+$(,)?]),+$(,)?) => {
+            mod $mod_name {
                 use super::*;
                 $(
                 #[test]
@@ -2967,6 +2975,7 @@ mod tests {
     }
 
     matcher_tests_with_macro! {
+        parsing_sexps
         match_sexp_1_1
         "(macro foo (x*) null)"
         expect_match: [
@@ -3001,6 +3010,7 @@ mod tests {
     }
 
     matcher_tests_with_macro! {
+        parsing_lists
         match_list_1_1
         "(macro foo (x*) null)"
         expect_match: [
@@ -3015,6 +3025,7 @@ mod tests {
     }
 
     matcher_tests_with_macro! {
+        parsing_eexps
         match_e_expression
         "(macro foo (x*) null)"
         expect_match: [
@@ -3046,6 +3057,22 @@ mod tests {
         ]
     }
 
+    matcher_tests_with_macro! {
+        allow_omitting_trailing_optionals
+        match_e_expression
+        "(macro foo (a b+ c? d*) null)"
+        expect_match: [
+            "(:foo 1 2)",
+            "(:foo 1 2 3)",
+            "(:foo 1 2 3 4)",
+            "(:foo 1 2 3 4 5 6)", // implicit rest
+        ],
+        expect_mismatch: [
+            "(:foo 1)",
+            "(:foo)",
+        ]
+    }
+
     #[rstest]
     #[case::empty("(:)")]
     #[case::empty_with_extra_spacing("(: )")]

From 1c0456c2e2322504fdc5db7f077b4b8863b726ef Mon Sep 17 00:00:00 2001
From: Zack Slayton <zslayton@amazon.com>
Date: Thu, 15 Aug 2024 17:17:06 -0400
Subject: [PATCH 2/2] adds unit tests for explicit empty stream

---
 src/lazy/text/buffer.rs | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/src/lazy/text/buffer.rs b/src/lazy/text/buffer.rs
index 87b44148..278ecfff 100644
--- a/src/lazy/text/buffer.rs
+++ b/src/lazy/text/buffer.rs
@@ -3066,6 +3066,9 @@ mod tests {
             "(:foo 1 2 3)",
             "(:foo 1 2 3 4)",
             "(:foo 1 2 3 4 5 6)", // implicit rest
+            "(:foo 1 2 3 (:))",   // explicit empty stream
+            "(:foo 1 2 (:) 4)",
+            "(:foo 1 2 (:) (:))",
         ],
         expect_mismatch: [
             "(:foo 1)",