Rewrite the type builtin in rust

CMakeLists.txt

@@ -108,7 +108,7 @@ set(FISH_BUILTIN_SRCS
     src/builtins/jobs.cpp src/builtins/math.cpp src/builtins/path.cpp
     src/builtins/read.cpp src/builtins/set.cpp
     src/builtins/set_color.cpp src/builtins/source.cpp src/builtins/status.cpp
-    src/builtins/string.cpp src/builtins/test.cpp src/builtins/type.cpp src/builtins/ulimit.cpp
+    src/builtins/string.cpp src/builtins/test.cpp src/builtins/ulimit.cpp
 )
 
 # List of other sources.

fish-rust/src/builtins/mod.rs

@@ -12,4 +12,5 @@ pub mod pwd;
 pub mod random;
 pub mod realpath;
 pub mod r#return;
+pub mod r#type;
 pub mod wait;

fish-rust/src/builtins/shared.rs

@@ -38,6 +38,8 @@ pub const BUILTIN_ERR_NOT_NUMBER: &str = "%ls: %ls: invalid integer\n";
 
 pub const BUILTIN_ERR_ARG_COUNT1: &str = "%ls: expected %d arguments; got %d\n";
 
+pub const BUILTIN_ERR_COMBO: &str = "%ls: invalid option combination\n";
+
 // Return values (`$status` values for fish scripts) for various situations.
 
 /// The status code used for normal exit in a command.
@@ -153,6 +155,7 @@ pub fn run_builtin(
         RustBuiltin::Random => super::random::random(parser, streams, args),
         RustBuiltin::Realpath => super::realpath::realpath(parser, streams, args),
         RustBuiltin::Return => super::r#return::r#return(parser, streams, args),
+        RustBuiltin::Type => super::r#type::r#type(parser, streams, args),
         RustBuiltin::Wait => wait::wait(parser, streams, args),
         RustBuiltin::Printf => printf::printf(parser, streams, args),
     }

fish-rust/src/builtins/type.rs

@@ -0,0 +1,233 @@
+use libc::c_int;
+use libc::isatty;
+use libc::STDOUT_FILENO;
+
+use crate::builtins::shared::{
+    builtin_missing_argument, builtin_print_help, builtin_unknown_option, io_streams_t,
+    BUILTIN_ERR_COMBO, STATUS_CMD_ERROR, STATUS_CMD_OK, STATUS_INVALID_ARGS,
+};
+use crate::ffi::parser_t;
+use crate::ffi::Repin;
+use crate::ffi::{
+    builtin_exists, colorize_shell, function_get_annotated_definition,
+    function_get_copy_definition_file, function_get_copy_definition_lineno,
+    function_get_definition_file, function_get_definition_lineno, function_get_props_autoload,
+    function_is_copy, path_get_paths_ffi,
+};
+use crate::wchar::{wstr, WString, L};
+use crate::wchar_ffi::WCharFromFFI;
+use crate::wchar_ffi::WCharToFFI;
+use crate::wgetopt::{wgetopter_t, wopt, woption, woption_argument_t};
+use crate::wutil::{sprintf, wgettext, wgettext_fmt};
+
+#[derive(Default)]
+struct type_cmd_opts_t {
+    all: bool,
+    short_output: bool,
+    no_functions: bool,
+    get_type: bool,
+    path: bool,
+    force_path: bool,
+    print_help: bool,
+    query: bool,
+}
+
+pub fn r#type(
+    parser: &mut parser_t,
+    streams: &mut io_streams_t,
+    argv: &mut [&wstr],
+) -> Option<c_int> {
+    let cmd = argv[0];
+    let argc = argv.len();
+    let print_hints = false;
+    let mut opts: type_cmd_opts_t = Default::default();
+
+    const shortopts: &wstr = L!(":hasftpPq");
+    const longopts: &[woption] = &[
+        wopt(L!("help"), woption_argument_t::no_argument, 'h'),
+        wopt(L!("all"), woption_argument_t::no_argument, 'a'),
+        wopt(L!("short"), woption_argument_t::no_argument, 's'),
+        wopt(L!("no-functions"), woption_argument_t::no_argument, 'f'),
+        wopt(L!("type"), woption_argument_t::no_argument, 't'),
+        wopt(L!("path"), woption_argument_t::no_argument, 'p'),
+        wopt(L!("force-path"), woption_argument_t::no_argument, 'P'),
+        wopt(L!("query"), woption_argument_t::no_argument, 'q'),
+        wopt(L!("quiet"), woption_argument_t::no_argument, 'q'),
+    ];
+
+    let mut w = wgetopter_t::new(shortopts, longopts, argv);
+    while let Some(c) = w.wgetopt_long() {
+        match c {
+            'a' => opts.all = true,
+            's' => opts.short_output = true,
+            'f' => opts.no_functions = true,
+            't' => opts.get_type = true,
+            'p' => opts.path = true,
+            'P' => opts.force_path = true,
+            'q' => opts.query = true,
+            'h' => {
+                builtin_print_help(parser, streams, cmd);
+                return STATUS_CMD_OK;
+            }
+            ':' => {
+                builtin_missing_argument(parser, streams, cmd, argv[w.woptind - 1], print_hints);
+                return STATUS_INVALID_ARGS;
+            }
+            '?' => {
+                builtin_unknown_option(parser, streams, cmd, argv[w.woptind - 1], print_hints);
+                return STATUS_INVALID_ARGS;
+            }
+            _ => {
+                panic!("unexpected retval from wgeopter.next()");
+            }
+        }
+    }
+
+    if opts.query as i64 + opts.path as i64 + opts.get_type as i64 + opts.force_path as i64 > 1 {
+        streams.err.append(wgettext_fmt!(BUILTIN_ERR_COMBO, cmd));
+        return STATUS_INVALID_ARGS;
+    }
+
+    let mut res = false;
+
+    let optind = w.woptind;
+    for arg in argv.iter().take(argc).skip(optind) {
+        let mut found = 0;
+        if !opts.force_path && !opts.no_functions {
+            let props = function_get_props_autoload(&arg.to_ffi(), parser.pin());
+            if !props.is_null() {
+                found += 1;
+                res = true;
+                // Early out - query means *any of the args exists*.
+                if opts.query {
+                    return STATUS_CMD_OK;
+                }
+                if !opts.get_type {
+                    let path = function_get_definition_file(&props).from_ffi();
+                    let mut comment = WString::new();
+
+                    if path.is_empty() {
+                        comment.push_utfstr(&wgettext_fmt!("Defined interactively"));
+                    } else if path == "-" {
+                        comment.push_utfstr(&wgettext_fmt!("Defined via `source`"));
+                    } else {
+                        let lineno: i32 = i32::from(function_get_definition_lineno(&props));
+                        comment.push_utfstr(&wgettext_fmt!(
+                            "Defined in %ls @ line %d",
+                            path,
+                            lineno
+                        ));
+                    }
+
+                    if function_is_copy(&props) {
+                        let path = function_get_copy_definition_file(&props).from_ffi();
+                        if path.is_empty() {
+                            comment.push_utfstr(&wgettext_fmt!(", copied interactively"));
+                        } else if path == "-" {
+                            comment.push_utfstr(&wgettext_fmt!(", copied via `source`"));
+                        } else {
+                            let lineno: i32 =
+                                i32::from(function_get_copy_definition_lineno(&props));
+                            comment.push_utfstr(&wgettext_fmt!(
+                                ", copied in %ls @ line %d",
+                                path,
+                                lineno
+                            ));
+                        }
+                    }
+                    if opts.path {
+                        if function_is_copy(&props) {
+                            let path = function_get_copy_definition_file(&props).from_ffi();
+                            streams.out.append(path);
+                        } else {
+                            streams.out.append(path);
+                        }
+                        streams.out.append(L!("\n"));
+                    } else if !opts.short_output {
+                        streams.out.append(wgettext_fmt!("%ls is a function", arg));
+                        streams.out.append(wgettext_fmt!(" with definition"));
+                        streams.out.append(L!("\n"));
+                        let mut def = WString::new();
+                        def.push_utfstr(&sprintf!(
+                            "# %ls\n%ls",
+                            comment,
+                            function_get_annotated_definition(&props, &arg.to_ffi()).from_ffi()
+                        ));
+
+                        if !streams.out_is_redirected && unsafe { isatty(STDOUT_FILENO) == 1 } {
+                            let col = colorize_shell(&def.to_ffi(), parser.pin()).from_ffi();
+                            streams.out.append(col);
+                        } else {
+                            streams.out.append(def);
+                        }
+                    } else {
+                        streams.out.append(wgettext_fmt!("%ls is a function", arg));
+                        streams.out.append(wgettext_fmt!(" (%ls)\n", comment));
+                    }
+                } else if opts.get_type {
+                    streams.out.append(L!("function\n"));
+                }
+                if !opts.all {
+                    continue;
+                }
+            }
+        }
+
+        if !opts.force_path && builtin_exists(&arg.to_ffi()) {
+            found += 1;
+            res = true;
+            if opts.query {
+                return STATUS_CMD_OK;
+            }
+            if !opts.get_type {
+                streams.out.append(wgettext_fmt!("%ls is a builtin\n", arg));
+            } else if opts.get_type {
+                streams.out.append(wgettext!("builtin\n"));
+            }
+            if !opts.all {
+                continue;
+            }
+        }
+
+        let paths: Vec<WString> = path_get_paths_ffi(&arg.to_ffi(), parser).from_ffi();
+
+        for path in paths.iter() {
+            found += 1;
+            res = true;
+            if opts.query {
+                return STATUS_CMD_OK;
+            }
+            if !opts.get_type {
+                if opts.path || opts.force_path {
+                    streams.out.append(sprintf!("%ls\n", path));
+                } else {
+                    streams.out.append(wgettext_fmt!("%ls is %ls\n", arg, path));
+                }
+            } else if opts.get_type {
+                streams.out.append(L!("file\n"));
+                break;
+            }
+            if !opts.all {
+                // We need to *break* out of this loop
+                // and continue on to the next argument,
+                // otherwise we would print every other path
+                // for a given argument.
+                break;
+            }
+        }
+
+        if found == 0 && !opts.query && !opts.path {
+            streams.err.append(wgettext_fmt!(
+                "%ls: Could not find '%ls'\n",
+                L!("type"),
+                arg
+            ));
+        }
+    }
+
+    if res {
+        STATUS_CMD_OK
+    } else {
+        STATUS_CMD_ERROR
+    }
+}

fish-rust/src/ffi.rs

@@ -31,6 +31,7 @@ include_cpp! {
     #include "parse_constants.h"
     #include "parser.h"
     #include "parse_util.h"
+    #include "path.h"
     #include "proc.h"
     #include "tokenizer.h"
     #include "wildcard.h"
@@ -81,6 +82,7 @@ include_cpp! {
     generate_pod!("RustFFIProcList")
     generate_pod!("RustBuiltin")
 
+    generate!("builtin_exists")
     generate!("builtin_missing_argument")
     generate!("builtin_unknown_option")
     generate!("builtin_print_help")
@@ -103,6 +105,9 @@ include_cpp! {
 
     generate!("env_var_t")
 
+    generate!("function_properties_t")
+    generate!("function_properties_ref_t")
+    generate!("function_get_props_autoload")
     generate!("function_get_definition_file")
     generate!("function_get_copy_definition_file")
     generate!("function_get_definition_lineno")
@@ -110,6 +115,7 @@ include_cpp! {
     generate!("function_get_annotated_definition")
     generate!("function_is_copy")
     generate!("function_exists")
+    generate!("path_get_paths_ffi")
 
     generate!("colorize_shell")
 }
@@ -288,6 +294,7 @@ impl Repin for job_t {}
 impl Repin for output_stream_t {}
 impl Repin for parser_t {}
 impl Repin for process_t {}
+impl Repin for function_properties_ref_t {}
 
 pub use autocxx::c_int;
 pub use ffi::*;

src/builtin.cpp

@@ -52,7 +52,6 @@
 #include "builtins/status.h"
 #include "builtins/string.h"
 #include "builtins/test.h"
-#include "builtins/type.h"
 #include "builtins/ulimit.h"
 #include "complete.h"
 #include "cxx.h"
@@ -407,7 +406,7 @@ static constexpr builtin_data_t builtin_datas[] = {
     {L"test", &builtin_test, N_(L"Test a condition")},
     {L"time", &builtin_generic, N_(L"Measure how long a command or block takes")},
     {L"true", &builtin_true, N_(L"Return a successful result")},
-    {L"type", &builtin_type, N_(L"Check if a thing is a thing")},
+    {L"type", &implemented_in_rust, N_(L"Check if a thing is a thing")},
     {L"ulimit", &builtin_ulimit, N_(L"Get/set resource usage limits")},
     {L"wait", &implemented_in_rust, N_(L"Wait for background processes completed")},
     {L"while", &builtin_generic, N_(L"Perform a command multiple times")},
@@ -554,6 +553,9 @@ static maybe_t<RustBuiltin> try_get_rust_builtin(const wcstring &cmd) {
     if (cmd == L"realpath") {
         return RustBuiltin::Realpath;
     }
+    if (cmd == L"type") {
+        return RustBuiltin::Type;
+    }
     if (cmd == L"wait") {
         return RustBuiltin::Wait;
     }

src/builtin.h

@@ -121,6 +121,7 @@ enum RustBuiltin : int32_t {
     Random,
     Realpath,
     Return,
+    Type,
     Wait,
 };
 #endif

src/builtins/type.cpp

@@ -1,227 +0,0 @@
-// Implementation of the type builtin.
-#include "config.h"  // IWYU pragma: keep
-
-#include "type.h"
-
-#include <unistd.h>
-
-#include <memory>
-#include <string>
-#include <vector>
-
-#include "../builtin.h"
-#include "../common.h"
-#include "../env.h"
-#include "../fallback.h"  // IWYU pragma: keep
-#include "../function.h"
-#include "../highlight.h"
-#include "../io.h"
-#include "../maybe.h"
-#include "../parser.h"
-#include "../path.h"
-#include "../wgetopt.h"
-#include "../wutil.h"  // IWYU pragma: keep
-
-struct type_cmd_opts_t {
-    bool all = false;
-    bool short_output = false;
-    bool no_functions = false;
-    bool type = false;
-    bool path = false;
-    bool force_path = false;
-    bool print_help = false;
-    bool query = false;
-};
-static const wchar_t *const short_options = L":hasftpPq";
-static const struct woption long_options[] = {
-    {L"help", no_argument, 'h'},       {L"all", no_argument, 'a'},
-    {L"short", no_argument, 's'},      {L"no-functions", no_argument, 'f'},
-    {L"type", no_argument, 't'},       {L"path", no_argument, 'p'},
-    {L"force-path", no_argument, 'P'}, {L"query", no_argument, 'q'},
-    {L"quiet", no_argument, 'q'},      {}};
-
-static int parse_cmd_opts(type_cmd_opts_t &opts, int *optind, int argc, const wchar_t **argv,
-                          parser_t &parser, io_streams_t &streams) {
-    UNUSED(parser);
-    const wchar_t *cmd = argv[0];
-    int opt;
-    wgetopter_t w;
-    while ((opt = w.wgetopt_long(argc, argv, short_options, long_options, nullptr)) != -1) {
-        switch (opt) {
-            case 'h': {
-                opts.print_help = true;
-                break;
-            }
-            case 'a': {
-                opts.all = true;
-                break;
-            }
-            case 's': {
-                opts.short_output = true;
-                break;
-            }
-            case 'f': {
-                opts.no_functions = true;
-                break;
-            }
-            case 't': {
-                opts.type = true;
-                break;
-            }
-            case 'p': {
-                opts.path = true;
-                break;
-            }
-            case 'P': {
-                opts.force_path = true;
-                break;
-            }
-            case 'q': {
-                opts.query = true;
-                break;
-            }
-            case ':': {
-                streams.err.append_format(BUILTIN_ERR_MISSING, cmd, argv[w.woptind - 1]);
-                return STATUS_INVALID_ARGS;
-            }
-            case '?': {
-                streams.err.append_format(BUILTIN_ERR_UNKNOWN, cmd, argv[w.woptind - 1]);
-                return STATUS_INVALID_ARGS;
-            }
-            default: {
-                DIE("unexpected retval from wgetopt_long");
-            }
-        }
-    }
-
-    *optind = w.woptind;
-    return STATUS_CMD_OK;
-}
-
-/// Implementation of the builtin 'type'.
-maybe_t<int> builtin_type(parser_t &parser, io_streams_t &streams, const wchar_t **argv) {
-    UNUSED(parser);
-    const wchar_t *cmd = argv[0];
-    int argc = builtin_count_args(argv);
-    type_cmd_opts_t opts;
-
-    int optind;
-    int retval = parse_cmd_opts(opts, &optind, argc, argv, parser, streams);
-    if (retval != STATUS_CMD_OK) return retval;
-
-    if (opts.print_help) {
-        builtin_print_help(parser, streams, cmd);
-        return STATUS_CMD_OK;
-    }
-
-    // Mutually exclusive options
-    if (opts.query + opts.path + opts.type + opts.force_path > 1) {
-        streams.err.append_format(BUILTIN_ERR_COMBO, cmd);
-        return STATUS_INVALID_ARGS;
-    }
-
-    wcstring_list_t builtins = builtin_get_names();
-    bool res = false;
-    for (int idx = optind; argv[idx]; ++idx) {
-        int found = 0;
-        const wchar_t *name = argv[idx];
-        // Functions
-        function_properties_ref_t func{};
-        if (!opts.force_path && !opts.no_functions &&
-            (func = function_get_props_autoload(name, parser))) {
-            ++found;
-            res = true;
-            if (!opts.query && !opts.type) {
-                auto path = func->definition_file;
-                auto copy_path = func->copy_definition_file;
-                auto final_path = func->is_copy ? copy_path : path;
-                wcstring comment;
-
-                if (!path) {
-                    append_format(comment, _(L"Defined interactively"));
-                } else if (*path == L"-") {
-                    append_format(comment, _(L"Defined via `source`"));
-                } else {
-                    append_format(comment, _(L"Defined in %ls @ line %d"), path->c_str(),
-                                  func->definition_lineno());
-                }
-
-                if (func->is_copy) {
-                    if (!copy_path) {
-                        append_format(comment, _(L", copied interactively"));
-                    } else if (*copy_path == L"-") {
-                        append_format(comment, _(L", copied via `source`"));
-                    } else {
-                        append_format(comment, _(L", copied in %ls @ line %d"), copy_path->c_str(),
-                                      func->copy_definition_lineno);
-                    }
-                }
-
-                if (opts.path) {
-                    if (final_path) {
-                        streams.out.append(*final_path);
-                        streams.out.append(L"\n");
-                    }
-                } else if (!opts.short_output) {
-                    streams.out.append_format(_(L"%ls is a function"), name);
-                    streams.out.append(_(L" with definition"));
-                    streams.out.append(L"\n");
-
-                    wcstring def;
-                    append_format(def, L"# %ls\n%ls", comment.c_str(),
-                                  func->annotated_definition(name).c_str());
-
-                    if (!streams.out_is_redirected && isatty(STDOUT_FILENO)) {
-                        std::vector<highlight_spec_t> colors;
-                        highlight_shell(def, colors, parser.context());
-                        streams.out.append(str2wcstring(colorize(def, colors, parser.vars())));
-                    } else {
-                        streams.out.append(def);
-                    }
-                } else {
-                    streams.out.append_format(_(L"%ls is a function"), name);
-                    streams.out.append_format(_(L" (%ls)\n"), comment.c_str());
-                }
-            } else if (opts.type) {
-                streams.out.append(L"function\n");
-            }
-            if (!opts.all) continue;
-        }
-
-        // Builtins
-        if (!opts.force_path && contains(builtins, name)) {
-            ++found;
-            res = true;
-            if (!opts.query && !opts.type) {
-                streams.out.append_format(_(L"%ls is a builtin\n"), name);
-            } else if (opts.type) {
-                streams.out.append(_(L"builtin\n"));
-            }
-            if (!opts.all) continue;
-        }
-
-        // Commands
-        wcstring_list_t paths = path_get_paths(name, parser.vars());
-        for (const auto &path : paths) {
-            ++found;
-            res = true;
-            if (!opts.query && !opts.type) {
-                if (opts.path || opts.force_path) {
-                    streams.out.append_format(L"%ls\n", path.c_str());
-                } else {
-                    streams.out.append_format(_(L"%ls is %ls\n"), name, path.c_str());
-                }
-            } else if (opts.type) {
-                streams.out.append(_(L"file\n"));
-                break;
-            }
-            if (!opts.all) break;
-        }
-
-        if (!found && !opts.query && !opts.path) {
-            streams.err.append_format(_(L"%ls: Could not find '%ls'\n"), L"type", name);
-        }
-    }
-
-    return res ? STATUS_CMD_OK : STATUS_CMD_ERROR;
-}

src/builtins/type.h

@@ -1,11 +0,0 @@
-// Prototypes for executing builtin_type function.
-#ifndef FISH_BUILTIN_TYPE_H
-#define FISH_BUILTIN_TYPE_H
-
-#include "../maybe.h"
-
-class parser_t;
-struct io_streams_t;
-
-maybe_t<int> builtin_type(parser_t &parser, io_streams_t &streams, const wchar_t **argv);
-#endif

tests/checks/type.fish

@@ -134,3 +134,9 @@ type -p other-test-type3
 
 type -s other-test-type3
 # CHECK: other-test-type3 is a function (Defined via `source`, copied via `source`)
+
+touch ./test
+chmod +x ./test
+
+PATH=.:$PATH type -P test
+# CHECK: ./test