Make parsing for unknown args in known externals like normal external calls

crates/nu-parser/src/flatten.rs

@@ -5,7 +5,7 @@ use nu_protocol::{
         RecordItem,
     },
     engine::StateWorkingSet,
-    DeclId, Span, VarId,
+    DeclId, Span, SyntaxShape, VarId,
 };
 use std::fmt::{Display, Formatter, Result};
 
@@ -166,6 +166,22 @@ fn flatten_pipeline_element_into(
     }
 }
 
+fn flatten_positional_arg_into(
+    working_set: &StateWorkingSet,
+    positional: &Expression,
+    shape: &SyntaxShape,
+    output: &mut Vec<(Span, FlatShape)>,
+) {
+    if matches!(shape, SyntaxShape::ExternalArgument)
+        && matches!(positional.expr, Expr::String(..) | Expr::GlobPattern(..))
+    {
+        // Make known external arguments look more like external arguments
+        output.push((positional.span, FlatShape::ExternalArg));
+    } else {
+        flatten_expression_into(working_set, positional, output)
+    }
+}
+
 fn flatten_expression_into(
     working_set: &StateWorkingSet,
     expr: &Expression,
@@ -249,16 +265,40 @@ fn flatten_expression_into(
             }
         }
         Expr::Call(call) => {
+            let decl = working_set.get_decl(call.decl_id);
+
             if call.head.end != 0 {
                 // Make sure we don't push synthetic calls
                 output.push((call.head, FlatShape::InternalCall(call.decl_id)));
             }
 
+            // Follow positional arguments from the signature.
+            let signature = decl.signature();
+            let mut positional_args = signature
+                .required_positional
+                .iter()
+                .chain(&signature.optional_positional);
+
             let arg_start = output.len();
             for arg in &call.arguments {
                 match arg {
-                    Argument::Positional(positional) | Argument::Unknown(positional) => {
-                        flatten_expression_into(working_set, positional, output)
+                    Argument::Positional(positional) => {
+                        let positional_arg = positional_args.next();
+                        let shape = positional_arg
+                            .or(signature.rest_positional.as_ref())
+                            .map(|arg| &arg.shape)
+                            .unwrap_or(&SyntaxShape::Any);
+
+                        flatten_positional_arg_into(working_set, positional, shape, output)
+                    }
+                    Argument::Unknown(positional) => {
+                        let shape = signature
+                            .rest_positional
+                            .as_ref()
+                            .map(|arg| &arg.shape)
+                            .unwrap_or(&SyntaxShape::Any);
+
+                        flatten_positional_arg_into(working_set, positional, shape, output)
                     }
                     Argument::Named(named) => {
                         if named.0.span.end != 0 {

crates/nu-parser/src/parse_keywords.rs

@@ -783,6 +783,16 @@ pub fn parse_extern(
                         working_set.get_block_mut(block_id).signature = signature;
                     }
                 } else {
+                    if signature.rest_positional.is_none() {
+                        // Make sure that a known external takes rest args with ExternalArgument
+                        // shape
+                        *signature = signature.rest(
+                            "args",
+                            SyntaxShape::ExternalArgument,
+                            "all other arguments to the command",
+                        );
+                    }
+
                     let decl = KnownExternal {
                         name: external_name,
                         usage,

crates/nu-parser/src/parser.rs

@@ -221,6 +221,22 @@ pub(crate) fn check_call(
     }
 }
 
+/// Parses an unknown argument for the given signature. This handles the parsing as appropriate to
+/// the rest type of the command.
+fn parse_unknown_arg(
+    working_set: &mut StateWorkingSet,
+    span: Span,
+    signature: &Signature,
+) -> Expression {
+    let shape = signature
+        .rest_positional
+        .as_ref()
+        .map(|arg| arg.shape.clone())
+        .unwrap_or(SyntaxShape::Any);
+
+    parse_value(working_set, span, &shape)
+}
+
 /// Parses a string in the arg or head position of an external call.
 ///
 /// If the string begins with `r#`, it is parsed as a raw string. If it doesn't contain any quotes
@@ -427,11 +443,7 @@ fn parse_external_string(working_set: &mut StateWorkingSet, span: Span) -> Expre
 fn parse_external_arg(working_set: &mut StateWorkingSet, span: Span) -> ExternalArgument {
     let contents = working_set.get_span_contents(span);
 
-    if contents.starts_with(b"$") || contents.starts_with(b"(") {
-        ExternalArgument::Regular(parse_dollar_expr(working_set, span))
-    } else if contents.starts_with(b"[") {
-        ExternalArgument::Regular(parse_list_expression(working_set, span, &SyntaxShape::Any))
-    } else if contents.len() > 3
+    if contents.len() > 3
         && contents.starts_with(b"...")
         && (contents[3] == b'$' || contents[3] == b'[' || contents[3] == b'(')
     {
@@ -441,7 +453,19 @@ fn parse_external_arg(working_set: &mut StateWorkingSet, span: Span) -> External
             &SyntaxShape::List(Box::new(SyntaxShape::Any)),
         ))
     } else {
-        ExternalArgument::Regular(parse_external_string(working_set, span))
+        ExternalArgument::Regular(parse_regular_external_arg(working_set, span))
+    }
+}
+
+fn parse_regular_external_arg(working_set: &mut StateWorkingSet, span: Span) -> Expression {
+    let contents = working_set.get_span_contents(span);
+
+    if contents.starts_with(b"$") || contents.starts_with(b"(") {
+        parse_dollar_expr(working_set, span)
+    } else if contents.starts_with(b"[") {
+        parse_list_expression(working_set, span, &SyntaxShape::Any)
+    } else {
+        parse_external_string(working_set, span)
     }
 }
 
@@ -998,7 +1022,7 @@ pub fn parse_internal_call(
                 && signature.allows_unknown_args
             {
                 working_set.parse_errors.truncate(starting_error_count);
-                let arg = parse_value(working_set, arg_span, &SyntaxShape::Any);
+                let arg = parse_unknown_arg(working_set, arg_span, &signature);
 
                 call.add_unknown(arg);
             } else {
@@ -1040,7 +1064,7 @@ pub fn parse_internal_call(
                 && signature.allows_unknown_args
             {
                 working_set.parse_errors.truncate(starting_error_count);
-                let arg = parse_value(working_set, arg_span, &SyntaxShape::Any);
+                let arg = parse_unknown_arg(working_set, arg_span, &signature);
 
                 call.add_unknown(arg);
             } else {
@@ -1196,7 +1220,7 @@ pub fn parse_internal_call(
             call.add_positional(arg);
             positional_idx += 1;
         } else if signature.allows_unknown_args {
-            let arg = parse_value(working_set, arg_span, &SyntaxShape::Any);
+            let arg = parse_unknown_arg(working_set, arg_span, &signature);
 
             call.add_unknown(arg);
         } else {
@@ -4670,7 +4694,8 @@ pub fn parse_value(
             | SyntaxShape::Signature
             | SyntaxShape::Filepath
             | SyntaxShape::String
-            | SyntaxShape::GlobPattern => {}
+            | SyntaxShape::GlobPattern
+            | SyntaxShape::ExternalArgument => {}
             _ => {
                 working_set.error(ParseError::Expected("non-[] value", span));
                 return Expression::garbage(working_set, span);
@@ -4747,6 +4772,8 @@ pub fn parse_value(
             Expression::garbage(working_set, span)
         }
 
+        SyntaxShape::ExternalArgument => parse_regular_external_arg(working_set, span),
+
         SyntaxShape::Any => {
             if bytes.starts_with(b"[") {
                 //parse_value(working_set, span, &SyntaxShape::Table)

crates/nu-protocol/src/syntax_shape.rs

@@ -47,6 +47,12 @@ pub enum SyntaxShape {
     /// A general expression, eg `1 + 2` or `foo --bar`
     Expression,
 
+    /// A (typically) string argument that follows external command argument parsing rules.
+    ///
+    /// Filepaths are expanded if unquoted, globs are allowed, and quotes embedded within unknown
+    /// args are unquoted.
+    ExternalArgument,
+
     /// A filepath is allowed
     Filepath,
 
@@ -145,6 +151,7 @@ impl SyntaxShape {
             SyntaxShape::DateTime => Type::Date,
             SyntaxShape::Duration => Type::Duration,
             SyntaxShape::Expression => Type::Any,
+            SyntaxShape::ExternalArgument => Type::Any,
             SyntaxShape::Filepath => Type::String,
             SyntaxShape::Directory => Type::String,
             SyntaxShape::Float => Type::Float,
@@ -238,6 +245,7 @@ impl Display for SyntaxShape {
             SyntaxShape::Signature => write!(f, "signature"),
             SyntaxShape::MatchBlock => write!(f, "match-block"),
             SyntaxShape::Expression => write!(f, "expression"),
+            SyntaxShape::ExternalArgument => write!(f, "external-argument"),
             SyntaxShape::Boolean => write!(f, "bool"),
             SyntaxShape::Error => write!(f, "error"),
             SyntaxShape::CompleterWrapper(x, _) => write!(f, "completable<{x}>"),

tests/repl/test_custom_commands.rs

@@ -186,8 +186,12 @@ fn help_present_in_def() -> TestResult {
 #[test]
 fn help_not_present_in_extern() -> TestResult {
     run_test(
-        "module test {export extern \"git fetch\" []}; use test `git fetch`; help git fetch | ansi strip",
-        "Usage:\n  > git fetch",
+        r#"
+            module test {export extern "git fetch" []};
+            use test `git fetch`;
+            help git fetch | find help | to text | ansi strip
+        "#,
+        "",
     )
 }
 

tests/repl/test_known_external.rs

@@ -137,3 +137,39 @@ fn known_external_aliased_subcommand_from_module() -> TestResult {
         String::from_utf8(output.stdout)?.trim(),
     )
 }
+
+#[test]
+fn known_external_arg_expansion() -> TestResult {
+    run_test(
+        r#"
+            extern echo [];
+            echo ~/foo
+        "#,
+        &dirs::home_dir()
+            .expect("can't find home dir")
+            .join("foo")
+            .to_string_lossy(),
+    )
+}
+
+#[test]
+fn known_external_arg_quoted_no_expand() -> TestResult {
+    run_test(
+        r#"
+            extern echo [];
+            echo "~/foo"
+        "#,
+        "~/foo",
+    )
+}
+
+#[test]
+fn known_external_arg_internally_quoted_options() -> TestResult {
+    run_test(
+        r#"
+            extern echo [];
+            echo --option="test"
+        "#,
+        "--option=test",
+    )
+}