More parsing fixes with tests

crates/nu-cli/src/errors.rs

@@ -127,6 +127,13 @@ pub fn report_parsing_error(
                     .with_labels(vec![Label::primary(diag_file_id, diag_range)
                         .with_message("short flag batches can't take args")])
             }
+            ParseError::KeywordMissingArgument(name, span) => {
+                let (diag_file_id, diag_range) = convert_span_to_diag(working_set, span)?;
+                Diagnostic::error()
+                    .with_message(format!("Missing argument to {}", name))
+                    .with_labels(vec![Label::primary(diag_file_id, diag_range)
+                        .with_message(format!("missing value that follows {}", name))])
+            }
             ParseError::MissingPositional(name, span) => {
                 let (diag_file_id, diag_range) = convert_span_to_diag(working_set, span)?;
                 Diagnostic::error()
@@ -212,6 +219,13 @@ pub fn report_parsing_error(
                     .with_labels(vec![Label::primary(diag_file_id, diag_range)
                         .with_message("parser support missing for this expression")])
             }
+            ParseError::RestNeedsName(span) => {
+                let (diag_file_id, diag_range) = convert_span_to_diag(working_set, span)?;
+                Diagnostic::error()
+                    .with_message("Rest parameter needs a name")
+                    .with_labels(vec![Label::primary(diag_file_id, diag_range)
+                        .with_message("needs a parameter name")])
+            }
         };
 
     // println!("DIAG");

crates/nu-parser/src/errors.rs

@@ -24,12 +24,14 @@ pub enum ParseError {
     MissingFlagParam(Span),
     ShortFlagBatchCantTakeArg(Span),
     MissingPositional(String, Span),
+    KeywordMissingArgument(String, Span),
     MissingType(Span),
     TypeMismatch(Type, Type, Span), // expected, found, span
     MissingRequiredFlag(String, Span),
     IncompleteMathExpression(Span),
     UnknownState(String, Span),
     IncompleteParser(Span),
+    RestNeedsName(Span),
 }
 
 impl<'a> codespan_reporting::files::Files<'a> for ParserWorkingSet<'a> {

crates/nu-parser/src/parser.rs

@@ -607,82 +607,19 @@ impl<'a> ParserWorkingSet<'a> {
             } else {
                 // Make space for the remaining require positionals, if we can
                 if positional_idx < decl.signature.required_positional.len()
-                    && spans.len() > (decl.signature.required_positional.len() - positional_idx - 1)
+                    && spans.len() > (decl.signature.required_positional.len() - positional_idx)
                 {
                     spans.len() - (decl.signature.required_positional.len() - positional_idx - 1)
                 } else {
+                    if decl.signature.num_positionals_after(positional_idx) == 0 {
                         spans.len()
-                }
-            }
-        }
-    }
-
-    /*
-    fn calculate_end_span(
-        &self,
-        decl: &Declaration,
-        spans: &[Span],
-        spans_idx: usize,
-        positional_idx: usize,
-    ) -> usize {
-        if decl.signature.rest_positional.is_some() {
-            spans.len()
-        } else {
-            // println!("num_positionals: {}", decl.signature.num_positionals());
-            // println!("positional_idx: {}", positional_idx);
-            // println!("spans.len(): {}", spans.len());
-            // println!("spans_idx: {}", spans_idx);
-
-            // check to see if a keyword follows the current position.
-
-            let mut next_keyword_idx = spans.len();
-            for idx in (positional_idx + 1)..decl.signature.num_positionals() {
-                if let Some(PositionalArg {
-                    shape: SyntaxShape::Keyword(kw, ..),
-                    ..
-                }) = decl.signature.get_positional(idx)
-                {
-                    #[allow(clippy::needless_range_loop)]
-                    for span_idx in spans_idx..spans.len() {
-                        let contents = self.get_span_contents(spans[span_idx]);
-
-                        if contents == kw {
-                            next_keyword_idx = span_idx - (idx - (positional_idx + 1));
-                            break;
-                        }
-                    }
-                }
-            }
-
-            let remainder = decl.signature.num_positionals_after(positional_idx);
-            let remainder_idx = if remainder < spans.len() {
-                spans.len() - remainder + 1
                     } else {
                         spans_idx + 1
-            };
-
-            let end = [next_keyword_idx, remainder_idx, spans.len()]
-                .iter()
-                .min()
-                .copied()
-                .expect("internal error: can't find min");
-
-            println!(
-                "{:?}",
-                [
-                    next_keyword_idx,
-                    remainder_idx,
-                    spans.len(),
-                    spans_idx,
-                    remainder,
-                    positional_idx,
-                    end,
-                ]
-            );
-            end
                     }
                 }
-    */
+            }
+        }
+    }
 
     fn parse_multispan_value(
         &mut self,
@@ -732,7 +669,7 @@ impl<'a> ParserWorkingSet<'a> {
                 *spans_idx += 1;
                 if *spans_idx >= spans.len() {
                     error = error.or_else(|| {
-                        Some(ParseError::MissingPositional(
+                        Some(ParseError::KeywordMissingArgument(
                             String::from_utf8_lossy(keyword).into(),
                             spans[*spans_idx - 1],
                         ))
@@ -842,6 +779,11 @@ impl<'a> ParserWorkingSet<'a> {
 
                 let end = self.calculate_end_span(&decl, spans, spans_idx, positional_idx);
 
+                // println!(
+                //     "start: {} end: {} positional_idx: {}",
+                //     spans_idx, end, positional_idx
+                // );
+
                 let orig_idx = spans_idx;
                 let (arg, err) =
                     self.parse_multispan_value(&spans[..end], &mut spans_idx, &positional.shape);
@@ -1516,7 +1458,6 @@ impl<'a> ParserWorkingSet<'a> {
         error = error.or(err);
 
         let mut args: Vec<Arg> = vec![];
-        let mut rest: Option<Arg> = None;
         let mut parse_mode = ParseMode::ArgMode;
 
         for token in &output {
@@ -1781,12 +1722,12 @@ impl<'a> ParserWorkingSet<'a> {
         for arg in args {
             match arg {
                 Arg::Positional(positional, required) => {
-                    if positional.name == "...rest" {
+                    if positional.name.starts_with("...") {
-                        if sig.rest_positional.is_none() {
+                        let name = positional.name[3..].to_string();
-                            sig.rest_positional = Some(PositionalArg {
+                        if name.is_empty() {
-                                name: "rest".into(),
+                            error = error.or(Some(ParseError::RestNeedsName(span)))
-                                ..positional
+                        } 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)))

crates/nu-parser/src/signature.rs

@@ -248,22 +248,6 @@ impl Signature {
             }
             curr += 1;
         }
-        // for positional in &self.optional_positional {
-        //     match positional.shape {
-        //         SyntaxShape::Keyword(..) => {
-        //             // Keywords have a required argument, so account for that
-        //             if curr > idx {
-        //                 total += 2;
-        //             }
-        //         }
-        //         _ => {
-        //             if curr > idx {
-        //                 total += 1;
-        //             }
-        //         }
-        //     }
-        //     curr += 1;
-        // }
         total
     }
 

src/tests.rs

@@ -109,6 +109,26 @@ fn if_cond4() -> TestResult {
     run_test("if 2 > 3 { 5 } else { 4 } ", "4")
 }
 
+#[test]
+fn if_elseif1() -> TestResult {
+    run_test("if 2 > 3 { 5 } else if 6 < 7 { 4 } ", "4")
+}
+
+#[test]
+fn if_elseif2() -> TestResult {
+    run_test("if 2 < 3 { 5 } else if 6 < 7 { 4 } else { 8 } ", "5")
+}
+
+#[test]
+fn if_elseif3() -> TestResult {
+    run_test("if 2 > 3 { 5 } else if 6 > 7 { 4 } else { 8 } ", "8")
+}
+
+#[test]
+fn if_elseif4() -> TestResult {
+    run_test("if 2 > 3 { 5 } else if 6 < 7 { 4 } else { 8 } ", "4")
+}
+
 #[test]
 fn no_scope_leak1() -> TestResult {
     fail_test(