make command signature parsing more strict (#14309)

# User-Facing Changes

The parser now errors on more invalid command signatures:

```nushell
# expected parameter or flag
def foo [ bar: int: ] {}

# expected type
def foo [ bar: =  ] {}
def foo [ bar: ] {}

# expected default value
def foo [ bar = ] {}
```

crates/nu-command/tests/commands/def.rs

@@ -2,6 +2,13 @@ use nu_test_support::nu;
 use nu_test_support::playground::Playground;
 use std::fs;
 
+#[test]
+fn def_with_trailing_comma() {
+    let actual = nu!("def test-command [ foo: int, ] { $foo }; test-command 1");
+
+    assert!(actual.out == "1");
+}
+
 #[test]
 fn def_with_comment() {
     Playground::setup("def_with_comment", |dirs, _| {
@@ -72,6 +79,13 @@ fn def_errors_with_comma_before_equals() {
     assert!(actual.err.contains("expected parameter"));
 }
 
+#[test]
+fn def_errors_with_colon_before_equals() {
+    let actual = nu!("def test-command [ foo: = 1 ] {}");
+
+    assert!(actual.err.contains("expected type"));
+}
+
 #[test]
 fn def_errors_with_comma_before_colon() {
     let actual = nu!("def test-command [ foo, : int ] {}");
@@ -85,7 +99,6 @@ fn def_errors_with_multiple_colons() {
     assert!(actual.err.contains("expected type"));
 }
 
-#[ignore = "This error condition is not implemented yet"]
 #[test]
 fn def_errors_with_multiple_types() {
     let actual = nu!("def test-command [ foo:int:string ] {}");
@@ -93,6 +106,20 @@ fn def_errors_with_multiple_types() {
     assert!(actual.err.contains("expected parameter"));
 }
 
+#[test]
+fn def_errors_with_trailing_colon() {
+    let actual = nu!("def test-command [ foo: int: ] {}");
+
+    assert!(actual.err.contains("expected parameter"));
+}
+
+#[test]
+fn def_errors_with_trailing_default_value() {
+    let actual = nu!("def test-command [ foo: int = ] {}");
+
+    assert!(actual.err.contains("expected default value"));
+}
+
 #[test]
 fn def_errors_with_multiple_commas() {
     let actual = nu!("def test-command [ foo,,bar ] {}");

crates/nu-parser/src/parser.rs

@@ -3392,6 +3392,7 @@ pub fn parse_signature_helper(working_set: &mut StateWorkingSet, span: Span) ->
         Arg,
         AfterCommaArg,
         Type,
+        AfterType,
         DefaultValue,
     }
 
@@ -3425,7 +3426,9 @@ pub fn parse_signature_helper(working_set: &mut StateWorkingSet, span: Span) ->
     let mut args: Vec<Arg> = vec![];
     let mut parse_mode = ParseMode::Arg;
 
-    for token in &output {
+    for (index, token) in output.iter().enumerate() {
+        let last_token = index == output.len() - 1;
+
         match token {
             Token {
                 contents: crate::TokenContents::Item | crate::TokenContents::AssignmentOperator,
@@ -3437,10 +3440,12 @@ pub fn parse_signature_helper(working_set: &mut StateWorkingSet, span: Span) ->
                 // The : symbol separates types
                 if contents == b":" {
                     match parse_mode {
+                        ParseMode::Arg if last_token => working_set
+                            .error(ParseError::Expected("type", Span::new(span.end, span.end))),
                         ParseMode::Arg => {
                             parse_mode = ParseMode::Type;
                         }
-                        ParseMode::AfterCommaArg => {
+                        ParseMode::AfterCommaArg | ParseMode::AfterType => {
                             working_set.error(ParseError::Expected("parameter or flag", span));
                         }
                         ParseMode::Type | ParseMode::DefaultValue => {
@@ -3452,9 +3457,15 @@ pub fn parse_signature_helper(working_set: &mut StateWorkingSet, span: Span) ->
                 // The = symbol separates a variable from its default value
                 else if contents == b"=" {
                     match parse_mode {
-                        ParseMode::Type | ParseMode::Arg => {
+                        ParseMode::Arg | ParseMode::AfterType if last_token => working_set.error(
+                            ParseError::Expected("default value", Span::new(span.end, span.end)),
+                        ),
+                        ParseMode::Arg | ParseMode::AfterType => {
                             parse_mode = ParseMode::DefaultValue;
                         }
+                        ParseMode::Type => {
+                            working_set.error(ParseError::Expected("type", span));
+                        }
                         ParseMode::AfterCommaArg => {
                             working_set.error(ParseError::Expected("parameter or flag", span));
                         }
@@ -3467,7 +3478,9 @@ pub fn parse_signature_helper(working_set: &mut StateWorkingSet, span: Span) ->
                 // The , symbol separates params only
                 else if contents == b"," {
                     match parse_mode {
-                        ParseMode::Arg => parse_mode = ParseMode::AfterCommaArg,
+                        ParseMode::Arg | ParseMode::AfterType => {
+                            parse_mode = ParseMode::AfterCommaArg
+                        }
                         ParseMode::AfterCommaArg => {
                             working_set.error(ParseError::Expected("parameter or flag", span));
                         }
@@ -3480,7 +3493,7 @@ pub fn parse_signature_helper(working_set: &mut StateWorkingSet, span: Span) ->
                     }
                 } else {
                     match parse_mode {
-                        ParseMode::Arg | ParseMode::AfterCommaArg => {
+                        ParseMode::Arg | ParseMode::AfterCommaArg | ParseMode::AfterType => {
                             // Long flag with optional short form following with no whitespace, e.g. --output, --age(-a)
                             if contents.starts_with(b"--") && contents.len() > 2 {
                                 // Split the long flag from the short flag with the ( character as delimiter.
@@ -3790,7 +3803,7 @@ pub fn parse_signature_helper(working_set: &mut StateWorkingSet, span: Span) ->
                                     }
                                 }
                             }
-                            parse_mode = ParseMode::Arg;
+                            parse_mode = ParseMode::AfterType;
                         }
                         ParseMode::DefaultValue => {
                             if let Some(last) = args.last_mut() {