Refactor external command (#6083)

Co-authored-by: Frank <v-frankz@microsoft.com>

crates/nu-command/src/core_commands/help.rs

@@ -142,7 +142,7 @@ fn help(
 
                 cols.push("is_custom".into());
                 vals.push(Value::Bool {
-                    val: decl.get_block_id().is_some(),
+                    val: decl.is_custom_command(),
                     span: head,
                 });
 
@@ -243,7 +243,7 @@ fn help(
 
                 cols.push("is_custom".into());
                 vals.push(Value::Bool {
-                    val: decl.get_block_id().is_some(),
+                    val: decl.is_custom_command(),
                     span: head,
                 });
 

crates/nu-command/src/system/which_.rs

@@ -95,7 +95,7 @@ fn get_entry_in_aliases(engine_state: &EngineState, name: &str, span: Span) -> O
 
 fn get_entry_in_commands(engine_state: &EngineState, name: &str, span: Span) -> Option<Value> {
     if let Some(decl_id) = engine_state.find_decl(name.as_bytes(), &[]) {
-        let (msg, is_builtin) = if engine_state.get_decl(decl_id).get_block_id().is_some() {
+        let (msg, is_builtin) = if engine_state.get_decl(decl_id).is_custom_command() {
             ("Nushell custom command", false)
         } else {
             ("Nushell built-in command", true)

crates/nu-engine/src/eval.rs

@@ -1121,7 +1121,7 @@ pub fn create_scope(
             cols.push("is_builtin".to_string());
             // we can only be a is_builtin or is_custom, not both
             vals.push(Value::Bool {
-                val: decl.get_block_id().is_none(),
+                val: !decl.is_custom_command(),
                 span,
             });
 
@@ -1139,7 +1139,7 @@ pub fn create_scope(
 
             cols.push("is_custom".to_string());
             vals.push(Value::Bool {
-                val: decl.get_block_id().is_some(),
+                val: decl.is_custom_command(),
                 span,
             });
 

crates/nu-parser/src/known_external.rs

@@ -30,6 +30,10 @@ impl Command for KnownExternal {
         true
     }
 
+    fn is_builtin(&self) -> bool {
+        false
+    }
+
     fn run(
         &self,
         engine_state: &EngineState,

crates/nu-parser/src/parser.rs

@@ -1079,6 +1079,8 @@ pub fn parse_call(
             }
         }
 
+        let decl = working_set.get_decl(decl_id);
+        if decl.is_builtin() {
             trace!("parsing: internal call");
 
             // parse internal command
@@ -1099,6 +1101,10 @@ pub fn parse_call(
                 },
                 parsed_call.error,
             )
+        } else {
+            trace!("parsing: `extern` custom command as external call");
+            parse_external_call(working_set, spans, expand_aliases_denylist)
+        }
     } else {
         // We might be parsing left-unbounded range ("..10")
         let bytes = working_set.get_span_contents(spans[0]);

crates/nu-protocol/src/engine/command.rs

@@ -37,6 +37,16 @@ pub trait Command: Send + Sync + CommandClone {
         false
     }
 
+    // This is an enhanced method to determine if a command is custom command or not
+    // since extern "foo" [] and def "foo" [] behaves differently
+    fn is_custom_command(&self) -> bool {
+        if self.get_block_id().is_some() {
+            true
+        } else {
+            self.is_known_external()
+        }
+    }
+
     // Is a sub command
     fn is_sub(&self) -> bool {
         self.name().contains(' ')

src/tests/test_known_external.rs

@@ -12,7 +12,23 @@ fn known_external_runs() -> TestResult {
 fn known_external_unknown_flag() -> TestResult {
     fail_test(
         r#"extern "cargo version" []; cargo version --no-such-flag"#,
-        "command doesn't have flag",
+        "Found argument '--no-such-flag' which wasn't expected, or isn't valid in this context",
+    )
+}
+
+#[test]
+fn known_external_not_defined_flag() -> TestResult {
+    run_test_contains(
+        r#"extern "cargo version" []; cargo version --help"#,
+        "Show version information",
+    )
+}
+
+#[test]
+fn known_external_is_custom() -> TestResult {
+    run_test_contains(
+        r#"extern "cargo version" []; help commands | where is_custom == true | get name"#,
+        "cargo version",
     )
 }