From bdce34676a561b60c04e1ed1362fae79ad512cc2 Mon Sep 17 00:00:00 2001
From: JT <jonathan.d.turner@gmail.com>
Date: Tue, 7 Sep 2021 15:37:02 +1200
Subject: [PATCH] Allow rest vars to have a custom name

---
 TODO.md                                    |  2 +-
 crates/nu-command/src/build_string.rs      |  2 +-
 crates/nu-engine/src/eval.rs               | 28 ++++++++++++++
 crates/nu-parser/src/parser.rs             | 45 ++++++++++++++++------
 crates/nu-protocol/src/signature.rs        |  9 ++++-
 crates/nu-protocol/tests/test_signature.rs |  2 +-
 src/tests.rs                               |  5 +++
 7 files changed, 77 insertions(+), 16 deletions(-)

diff --git a/TODO.md b/TODO.md
index 3123a6af7..36c294ace 100644
--- a/TODO.md
+++ b/TODO.md
@@ -15,9 +15,9 @@
 - [x] Block params
 - [x] Ranges
 - [x] Column path
+- [x] ...rest without calling it rest
 - [ ] Iteration (`each`) over tables
 - [ ] ctrl-c support
-- [ ] ...rest without calling it rest
 - [ ] operator overflow
 - [ ] finish operator type-checking
 - [ ] Source
diff --git a/crates/nu-command/src/build_string.rs b/crates/nu-command/src/build_string.rs
index 1d6d5e51f..90516cde0 100644
--- a/crates/nu-command/src/build_string.rs
+++ b/crates/nu-command/src/build_string.rs
@@ -15,7 +15,7 @@ impl Command for BuildString {
     }
 
     fn signature(&self) -> nu_protocol::Signature {
-        Signature::build("build-string").rest(SyntaxShape::String, "list of string")
+        Signature::build("build-string").rest("rest", SyntaxShape::String, "list of string")
     }
 
     fn run(
diff --git a/crates/nu-engine/src/eval.rs b/crates/nu-engine/src/eval.rs
index c4f7b7344..ba7ac8593 100644
--- a/crates/nu-engine/src/eval.rs
+++ b/crates/nu-engine/src/eval.rs
@@ -32,6 +32,34 @@ fn eval_call(context: &EvaluationContext, call: &Call, input: Value) -> Result<V
 
             state.add_var(var_id, result);
         }
+
+        if let Some(rest_positional) = decl.signature().rest_positional {
+            let mut rest_items = vec![];
+
+            for arg in call.positional.iter().skip(
+                decl.signature().required_positional.len()
+                    + decl.signature().optional_positional.len(),
+            ) {
+                let result = eval_expression(&state, arg)?;
+                rest_items.push(result);
+            }
+
+            let span = if let Some(rest_item) = rest_items.first() {
+                rest_item.span()
+            } else {
+                Span::unknown()
+            };
+
+            state.add_var(
+                rest_positional
+                    .var_id
+                    .expect("Internal error: rest positional parameter lacks var_id"),
+                Value::List {
+                    val: rest_items,
+                    span,
+                },
+            )
+        }
         let engine_state = state.engine_state.borrow();
         let block = engine_state.get_block(block_id);
         eval_block(&state, block, input)
diff --git a/crates/nu-parser/src/parser.rs b/crates/nu-parser/src/parser.rs
index 24b898bd5..6b483d1c1 100644
--- a/crates/nu-parser/src/parser.rs
+++ b/crates/nu-parser/src/parser.rs
@@ -1361,6 +1361,7 @@ 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 {
@@ -1535,6 +1536,25 @@ pub fn parse_signature_helper(
                                     },
                                     false,
                                 ))
+                            } else if let Some(contents) = contents.strip_prefix(b"...") {
+                                let name = String::from_utf8_lossy(contents).to_string();
+                                let contents_vec: Vec<u8> = contents.to_vec();
+
+                                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)))
+                                }
                             } else {
                                 let name = String::from_utf8_lossy(contents).to_string();
                                 let contents_vec = contents.to_vec();
@@ -1610,20 +1630,23 @@ 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) => {
-                if positional.name.starts_with("...") {
-                    let name = positional.name[3..].to_string();
-                    if name.is_empty() {
-                        error = error.or(Some(ParseError::RestNeedsName(span)))
-                    } else if sig.rest_positional.is_none() {
-                        sig.rest_positional = Some(PositionalArg { name, ..positional })
-                    } else {
-                        // Too many rest params
-                        error = error.or(Some(ParseError::MultipleRestParams(span)))
-                    }
-                } else if required {
+                if required {
                     sig.required_positional.push(positional)
                 } else {
                     sig.optional_positional.push(positional)
diff --git a/crates/nu-protocol/src/signature.rs b/crates/nu-protocol/src/signature.rs
index 0bf8fa095..49bae4280 100644
--- a/crates/nu-protocol/src/signature.rs
+++ b/crates/nu-protocol/src/signature.rs
@@ -108,9 +108,14 @@ impl Signature {
         self
     }
 
-    pub fn rest(mut self, shape: impl Into<SyntaxShape>, desc: impl Into<String>) -> Signature {
+    pub fn rest(
+        mut self,
+        name: &str,
+        shape: impl Into<SyntaxShape>,
+        desc: impl Into<String>,
+    ) -> Signature {
         self.rest_positional = Some(PositionalArg {
-            name: "rest".into(),
+            name: name.into(),
             desc: desc.into(),
             shape: shape.into(),
             var_id: None,
diff --git a/crates/nu-protocol/tests/test_signature.rs b/crates/nu-protocol/tests/test_signature.rs
index 2516409f0..a81432580 100644
--- a/crates/nu-protocol/tests/test_signature.rs
+++ b/crates/nu-protocol/tests/test_signature.rs
@@ -27,7 +27,7 @@ fn test_signature_chained() {
         )
         .named("named", SyntaxShape::String, "named description", Some('n'))
         .switch("switch", "switch description", None)
-        .rest(SyntaxShape::String, "rest description");
+        .rest("rest", SyntaxShape::String, "rest description");
 
     assert_eq!(signature.required_positional.len(), 1);
     assert_eq!(signature.optional_positional.len(), 1);
diff --git a/src/tests.rs b/src/tests.rs
index 50e33440d..1d8e9c80a 100644
--- a/src/tests.rs
+++ b/src/tests.rs
@@ -274,3 +274,8 @@ fn cell_path_var1() -> TestResult {
 fn cell_path_var2() -> TestResult {
     run_test("let x = [[lang, gems]; [nu, 100]]; $x.lang.0", "nu")
 }
+
+#[test]
+fn custom_rest_var() -> TestResult {
+    run_test("def foo [...x] { $x.0 + $x.1 }; foo 10 80", "90")
+}