From a2723c2ba42f2e50500bbad92400284e6908ae0b Mon Sep 17 00:00:00 2001
From: JT <547158+jntrnr@users.noreply.github.com>
Date: Mon, 7 Mar 2022 11:44:27 -0500
Subject: [PATCH] Fix rest parsing (#4765)

* More nuon tests, fix table print

* Fix rest type parsing
---
 crates/nu-parser/src/parser.rs | 59 ++++++++++++++++++----------------
 src/tests/test_parser.rs       | 10 ++++++
 2 files changed, 42 insertions(+), 27 deletions(-)

diff --git a/crates/nu-parser/src/parser.rs b/crates/nu-parser/src/parser.rs
index a039afe92d4d..8bfdb9efa169 100644
--- a/crates/nu-parser/src/parser.rs
+++ b/crates/nu-parser/src/parser.rs
@@ -2749,6 +2749,7 @@ pub fn parse_signature_helper(
 
     enum Arg {
         Positional(PositionalArg, bool), // bool - required
+        RestPositional(PositionalArg),
         Flag(Flag),
     }
 
@@ -2759,7 +2760,6 @@ pub fn parse_signature_helper(
     error = error.or(err);
 
     let mut args: Vec<Arg> = vec![];
-    let mut rest_arg = None;
     let mut parse_mode = ParseMode::ArgMode;
 
     for token in &output {
@@ -2930,19 +2930,12 @@ pub fn parse_signature_helper(
 
                                 let var_id = working_set.add_variable(contents_vec, Type::Unknown);
 
-                                if rest_arg.is_none() {
-                                    rest_arg = Some(Arg::Positional(
-                                        PositionalArg {
-                                            desc: String::new(),
-                                            name,
-                                            shape: SyntaxShape::Any,
-                                            var_id: Some(var_id),
-                                        },
-                                        false,
-                                    ));
-                                } else {
-                                    error = error.or(Some(ParseError::MultipleRestParams(span)))
-                                }
+                                args.push(Arg::RestPositional(PositionalArg {
+                                    desc: String::new(),
+                                    name,
+                                    shape: SyntaxShape::Any,
+                                    var_id: Some(var_id),
+                                }));
                             } else {
                                 let name = String::from_utf8_lossy(contents).to_string();
                                 let contents_vec = contents.to_vec();
@@ -2972,6 +2965,12 @@ pub fn parse_signature_helper(
                                         working_set.set_variable_type(var_id.expect("internal error: all custom parameters must have var_ids"), syntax_shape.to_type());
                                         *shape = syntax_shape;
                                     }
+                                    Arg::RestPositional(PositionalArg {
+                                        shape, var_id, ..
+                                    }) => {
+                                        working_set.set_variable_type(var_id.expect("internal error: all custom parameters must have var_ids"), syntax_shape.to_type());
+                                        *shape = syntax_shape;
+                                    }
                                     Arg::Flag(Flag { arg, var_id, .. }) => {
                                         // Flags with a boolean type are just present/not-present switches
                                         if syntax_shape != SyntaxShape::Boolean {
@@ -3012,6 +3011,12 @@ pub fn parse_signature_helper(
                             }
                             positional.desc.push_str(&contents);
                         }
+                        Arg::RestPositional(positional) => {
+                            if !positional.desc.is_empty() {
+                                positional.desc.push('\n');
+                            }
+                            positional.desc.push_str(&contents);
+                        }
                     }
                 }
             }
@@ -3021,19 +3026,6 @@ pub fn parse_signature_helper(
 
     let mut sig = Signature::new(String::new());
 
-    if let Some(Arg::Positional(positional, ..)) = rest_arg {
-        if positional.name.is_empty() {
-            error = error.or(Some(ParseError::RestNeedsName(span)))
-        } else if sig.rest_positional.is_none() {
-            sig.rest_positional = Some(PositionalArg {
-                name: positional.name,
-                ..positional
-            })
-        } else {
-            // Too many rest params
-            error = error.or(Some(ParseError::MultipleRestParams(span)))
-        }
-    }
     for arg in args {
         match arg {
             Arg::Positional(positional, required) => {
@@ -3044,6 +3036,19 @@ pub fn parse_signature_helper(
                 }
             }
             Arg::Flag(flag) => sig.named.push(flag),
+            Arg::RestPositional(positional) => {
+                if positional.name.is_empty() {
+                    error = error.or(Some(ParseError::RestNeedsName(span)))
+                } else if sig.rest_positional.is_none() {
+                    sig.rest_positional = Some(PositionalArg {
+                        name: positional.name,
+                        ..positional
+                    })
+                } else {
+                    // Too many rest params
+                    error = error.or(Some(ParseError::MultipleRestParams(span)))
+                }
+            }
         }
     }
 
diff --git a/src/tests/test_parser.rs b/src/tests/test_parser.rs
index ac498985ddd5..481df1afc65f 100644
--- a/src/tests/test_parser.rs
+++ b/src/tests/test_parser.rs
@@ -339,3 +339,13 @@ fn string_escape() -> TestResult {
 fn string_escape_interpolation() -> TestResult {
     run_test(r#"$"\u015B(char hamburger)abc""#, "ś≡abc")
 }
+
+#[test]
+fn proper_rest_types() -> TestResult {
+    run_test(
+        r#"def foo [--verbose(-v): bool, # my test flag
+                   ...rest: int # my rest comment
+                ] { if $verbose { print "verbose!" } else { print "not verbose!" } }; foo"#,
+        "not verbose!",
+    )
+}