Refuse internal command execution given unexpected arguments. (#1383)

crates/nu-errors/src/lib.rs

@@ -133,6 +133,10 @@ pub enum ArgumentError {
     MissingMandatoryPositional(String),
     /// A flag was found, and it should have been followed by a value, but no value was found
     MissingValueForName(String),
+    /// An argument was found, but the command does not recognize it
+    UnexpectedArgument(Spanned<String>),
+    /// An flag was found, but the command does not recognize it
+    UnexpectedFlag(Spanned<String>),
     /// A sequence of characters was found that was not syntactically valid (but would have
     /// been valid if the command was an external command)
     InvalidExternalWord,
@@ -146,6 +150,16 @@ impl PrettyDebug for ArgumentError {
                     + b::description(flag)
                     + b::description("` as mandatory flag")
             }
+            ArgumentError::UnexpectedArgument(name) => {
+                b::description("unexpected `")
+                    + b::description(&name.item)
+                    + b::description("` is not supported")
+            }
+            ArgumentError::UnexpectedFlag(name) => {
+                b::description("unexpected `")
+                    + b::description(&name.item)
+                    + b::description("` is not supported")
+            }
             ArgumentError::MissingMandatoryPositional(pos) => {
                 b::description("missing `")
                     + b::description(pos)
@@ -452,6 +466,30 @@ impl ShellError {
                     Severity::Error,
                     "Invalid bare word for Nu command (did you intend to invoke an external command?)".to_string())
                 .with_label(Label::new_primary(command.span)),
+                ArgumentError::UnexpectedArgument(argument) => Diagnostic::new(
+                    Severity::Error,
+                    format!(
+                        "{} unexpected {}",
+                        Color::Cyan.paint(&command.item),
+                        Color::Green.bold().paint(&argument.item)
+                    ),
+                )
+                .with_label(
+                    Label::new_primary(argument.span).with_message(
+                        format!("unexpected argument (try {} -h)", &command.item))
+                ),
+                ArgumentError::UnexpectedFlag(flag) => Diagnostic::new(
+                    Severity::Error,
+                    format!(
+                        "{} unexpected {}",
+                        Color::Cyan.paint(&command.item),
+                        Color::Green.bold().paint(&flag.item)
+                    ),
+                )
+                .with_label(
+                    Label::new_primary(flag.span).with_message(
+                    format!("unexpected flag (try {} -h)", &command.item))
+                    ),
                 ArgumentError::MissingMandatoryFlag(name) => Diagnostic::new(
                     Severity::Error,
                     format!(

crates/nu-parser/src/parse_command.rs

@@ -2,11 +2,11 @@ use crate::hir::syntax_shape::{
     BackoffColoringMode, ExpandSyntax, MaybeSpaceShape, MaybeWhitespaceEof,
 };
 use crate::hir::SpannedExpression;
-use crate::TokensIterator;
 use crate::{
     hir::{self, NamedArguments},
     Flag,
 };
+use crate::{Token, TokensIterator};
 use log::trace;
 use nu_errors::{ArgumentError, ParseError};
 use nu_protocol::{NamedType, PositionalType, Signature, SyntaxShape};
@@ -150,6 +150,15 @@ pub fn parse_command_tail(
         positional.extend(out);
     }
 
+    trace_remaining("after rest", &tail);
+
+    if found_error.is_none() {
+        if let Some(unexpected_argument_error) = find_unexpected_tokens(config, tail, command_span)
+        {
+            found_error = Some(unexpected_argument_error);
+        }
+    }
+
     eat_any_whitespace(tail);
 
     // Consume any remaining tokens with backoff coloring mode
@@ -159,12 +168,6 @@ pub fn parse_command_tail(
     // this solution.
     tail.sort_shapes();
 
-    if let Some(err) = found_error {
-        return Err(err);
-    }
-
-    trace_remaining("after rest", &tail);
-
     trace!(target: "nu::parse::trace_remaining", "Constructed positional={:?} named={:?}", positional, named);
 
     let positional = if positional.is_empty() {
@@ -173,8 +176,6 @@ pub fn parse_command_tail(
         Some(positional)
     };
 
-    // TODO: Error if extra unconsumed positional arguments
-
     let named = if named.named.is_empty() {
         None
     } else {
@@ -183,6 +184,10 @@ pub fn parse_command_tail(
 
     trace!(target: "nu::parse::trace_remaining", "Normalized positional={:?} named={:?}", positional, named);
 
+    if let Some(err) = found_error {
+        return Err(err);
+    }
+
     Ok(Some((positional, named)))
 }
 
@@ -338,6 +343,48 @@ fn extract_optional(
     }
 }
 
+fn find_unexpected_tokens(
+    config: &Signature,
+    tail: &hir::TokensIterator,
+    command_span: Span,
+) -> Option<ParseError> {
+    let mut tokens = tail.clone();
+    let source = tail.source();
+
+    loop {
+        tokens.move_to(0);
+
+        if let Some(node) = tokens.peek().commit() {
+            match &node.unspanned() {
+                Token::Whitespace => {}
+                Token::Flag { .. } => {
+                    return Some(ParseError::argument_error(
+                        config.name.clone().spanned(command_span),
+                        ArgumentError::UnexpectedFlag(Spanned {
+                            item: node.span().slice(&source).to_string(),
+                            span: node.span(),
+                        }),
+                    ));
+                }
+                _ => {
+                    return Some(ParseError::argument_error(
+                        config.name.clone().spanned(command_span),
+                        ArgumentError::UnexpectedArgument(Spanned {
+                            item: node.span().slice(&source).to_string(),
+                            span: node.span(),
+                        }),
+                    ));
+                }
+            }
+        }
+
+        if tokens.at_end() {
+            break;
+        }
+    }
+    None
+}
+
 pub fn trace_remaining(desc: &'static str, tail: &hir::TokensIterator<'_>) {
     let offset = tail.clone().span_at_cursor();
     let source = tail.source();

tests/shell/pipeline/commands/internal.rs

@@ -42,6 +42,47 @@ fn can_process_one_row_from_internal_and_pipes_it_to_stdin_of_external() {
     assert_eq!(actual, "nushell");
 }
 
+mod parse {
+    use nu_test_support::nu_error;
+
+    /*
+        The debug command's signature is:
+
+        Usage:
+        > debug {flags}
+
+        flags:
+        -h, --help: Display this help message
+        -r, --raw: Prints the raw value representation.
+    */
+
+    #[test]
+    fn errors_if_flag_is_not_supported() {
+        let actual = nu_error!(cwd: ".", "debug --ferris");
+
+        assert!(
+            actual.contains("unexpected flag"),
+            format!(
+                "error message '{}' should contain 'unexpected flag'",
+                actual
+            )
+        );
+    }
+
+    #[test]
+    fn errors_if_passed_an_unexpected_argument() {
+        let actual = nu_error!(cwd: ".", "debug ferris");
+
+        assert!(
+            actual.contains("unexpected argument"),
+            format!(
+                "error message '{}' should contain 'unexpected argument'",
+                actual
+            )
+        );
+    }
+}
+
 mod tilde_expansion {
     use super::nu;