Fix status code when bad command name is entered

This commit fixes a bug which causes that

   fish -c ')'; echo $status

("Illegal command name" error) returns 0. This is inconsistent with
e.g. when trying to run non-existent command:

   fish -c 'invalid-command'; echo $status

("Unknown command" error) which correctly returns 127.

A new status code,

    STATUS_ILLEGAL_CMD = 123

is introduced - which is returned whenever the 'Illegal command name *'
message is printed.

This commit also adds a test which checks if valid commands return 0,
while commands with illegal name return status code 123.

Fixes #3606.

doc_src/index.hdr.in

@@ -865,6 +865,8 @@ If `fish` encounters a problem while executing a command, the status variable ma
 
 - 1 is the generally the exit status from fish builtin commands if they were supplied with invalid arguments
 
+- 123 means that the command was not executed because the command name contained invalid characters
+
 - 124 means that the command was not executed because none of the wildcards in the command produced any matches
 
 - 125 means that while an executable with the specified name was located, the operating system could not actually execute the command

src/fish_tests.cpp

@@ -4010,6 +4010,44 @@ static void test_env_vars(void) {
     // TODO: Add tests for the locale and ncurses vars.
 }
 
+static void test_illegal_command_exit_code(void) {
+    say(L"Testing illegal command exit code");
+
+    struct command_result_tuple_t {
+        const wchar_t *txt;
+        int result;
+    };
+
+    const command_result_tuple_t tests[] = {
+        {L"echo", STATUS_BUILTIN_OK},
+        {L"pwd", STATUS_BUILTIN_OK},
+        {L")", STATUS_ILLEGAL_CMD},
+        {L") ", STATUS_ILLEGAL_CMD},
+        {L"*", STATUS_ILLEGAL_CMD},
+        {L"**", STATUS_ILLEGAL_CMD},
+        {L"%", STATUS_ILLEGAL_CMD},
+        {L"%test", STATUS_ILLEGAL_CMD},
+    // the following two inputs cause errors during tests for unknown reasons
+    // ("terminate called after throwing an instance of 'std::bad_alloc'")
+    //     {L"?", STATUS_ILLEGAL_CMD},
+    //     {L"abc?def", STATUS_ILLEGAL_CMD},
+        {L") ", STATUS_ILLEGAL_CMD}};
+
+    int res = 0;
+    const io_chain_t empty_ios;
+    parser_t &parser = parser_t::principal_parser();
+
+    size_t i = 0;
+    for (i = 0; i < sizeof tests / sizeof *tests; i++) {
+        res = parser.eval(tests[i].txt, empty_ios, TOP);
+
+        int exit_status = res ? STATUS_UNKNOWN_COMMAND : proc_get_last_status();
+        if (exit_status != tests[i].result) {
+            err(L"command '%ls': expected exit code %d , got %d", tests[i].txt, tests[i].result, exit_status);
+        }
+    }
+}
+
 /// Main test.
 int main(int argc, char **argv) {
     UNUSED(argc);
@@ -4102,6 +4140,7 @@ int main(int argc, char **argv) {
     if (should_test_function("history_formats")) history_tests_t::test_history_formats();
     if (should_test_function("string")) test_string();
     if (should_test_function("env_vars")) test_env_vars();
+    if (should_test_function("illegal_command_exit_code")) test_illegal_command_exit_code();
     // history_tests_t::test_history_speed();
 
     say(L"Encountered %d errors in low-level tests", err_count);

src/parse_execution.cpp

@@ -832,6 +832,7 @@ parse_execution_result_t parse_execution_context_t::populate_plain_process(
     bool expanded = expand_one(cmd, EXPAND_SKIP_CMDSUBST | EXPAND_SKIP_VARIABLES, NULL);
     if (!expanded) {
         report_error(statement, ILLEGAL_CMD_ERR_MSG, cmd.c_str());
+        proc_set_last_status(STATUS_ILLEGAL_CMD);
         return parse_execution_errored;
     }
 

src/proc.h

@@ -29,6 +29,9 @@
 /// The status code use when a wildcard had no matches.
 #define STATUS_UNMATCHED_WILDCARD 124
 
+/// The status code use when illegal command name is encountered.
+#define STATUS_ILLEGAL_CMD 123
+
 /// The status code used for normal exit in a  builtin.
 #define STATUS_BUILTIN_OK 0