diff --git a/crates/nu-parser/src/parser.rs b/crates/nu-parser/src/parser.rs index 57be8ec82..909fd75d6 100644 --- a/crates/nu-parser/src/parser.rs +++ b/crates/nu-parser/src/parser.rs @@ -734,22 +734,23 @@ fn calculate_end_span( // keywords, they get to set this by being present let positionals_between = kw_pos - positional_idx - 1; - if positionals_between > (kw_idx - spans_idx) { + if positionals_between >= (kw_idx - spans_idx) { kw_idx } else { kw_idx - positionals_between } } else { // Make space for the remaining require positionals, if we can - if signature.num_positionals_after(positional_idx) == 0 { - spans.len() - } else if positional_idx < signature.required_positional.len() - && spans.len() > (signature.required_positional.len() - positional_idx) - { - spans.len() - (signature.required_positional.len() - positional_idx - 1) - } else { - spans_idx + 1 - } + // spans_idx < spans.len() is an invariant + let remaining_spans = spans.len() - (spans_idx + 1); + // positional_idx can be larger than required_positional.len() if we have optional args + let remaining_positional = signature + .required_positional + .len() + .saturating_sub(positional_idx + 1); + // Saturates to 0 when we have too few args + let extra_spans = remaining_spans.saturating_sub(remaining_positional); + spans_idx + 1 + extra_spans } } } @@ -1164,11 +1165,24 @@ pub fn parse_internal_call( if let Some(positional) = signature.get_positional(positional_idx) { let end = calculate_end_span(working_set, &signature, spans, spans_idx, positional_idx); - let end = if spans.len() > spans_idx && end == spans_idx { - end + 1 - } else { - end - }; + // Missing arguments before next keyword + if end == spans_idx { + let prev_span = if spans_idx == 0 { + command_span + } else { + spans[spans_idx - 1] + }; + let whitespace_span = Span::new(prev_span.end, spans[spans_idx].start); + working_set.error(ParseError::MissingPositional( + positional.name.clone(), + whitespace_span, + signature.call_signature(), + )); + call.add_positional(Expression::garbage(working_set, whitespace_span)); + positional_idx += 1; + continue; + } + debug_assert!(end <= spans.len()); if spans[..end].is_empty() || spans_idx == end { working_set.error(ParseError::MissingPositional( diff --git a/crates/nu-protocol/src/signature.rs b/crates/nu-protocol/src/signature.rs index 4c5cae9e0..30f8b280d 100644 --- a/crates/nu-protocol/src/signature.rs +++ b/crates/nu-protocol/src/signature.rs @@ -522,27 +522,6 @@ impl Signature { total } - pub fn num_positionals_after(&self, idx: usize) -> usize { - let mut total = 0; - - for (curr, positional) in self.required_positional.iter().enumerate() { - match positional.shape { - SyntaxShape::Keyword(..) => { - // Keywords have a required argument, so account for that - if curr > idx { - total += 2; - } - } - _ => { - if curr > idx { - total += 1; - } - } - } - } - total - } - /// Find the matching long flag pub fn get_long_flag(&self, name: &str) -> Option { for flag in &self.named { diff --git a/tests/repl/test_parser.rs b/tests/repl/test_parser.rs index b5ab6d735..8ccc15080 100644 --- a/tests/repl/test_parser.rs +++ b/tests/repl/test_parser.rs @@ -189,7 +189,31 @@ fn assignment_with_no_var() -> TestResult { "mut = 'foo' | $in; $x | describe", ]; - let expected = "valid variable"; + let expecteds = [ + "missing var_name", + "missing var_name", + "missing const_name", + "missing var_name", + "missing var_name", + ]; + + for (case, expected) in std::iter::zip(cases, expecteds) { + fail_test(case, expected)?; + } + + Ok(()) +} + +#[test] +fn too_few_arguments() -> TestResult { + // Test for https://github.com/nushell/nushell/issues/9072 + let cases = [ + "def a [b: bool, c: bool, d: float, e: float, f: float] {}; a true true 1 1", + "def a [b: bool, c: bool, d: float, e: float, f: float, g: float] {}; a true true 1 1", + "def a [b: bool, c: bool, d: float, e: float, f: float, g: float, h: float] {}; a true true 1 1", + ]; + + let expected = "missing f"; for case in cases { fail_test(case, expected)?;