Thread a parser into expansion

Expansion may perform command substitution, which needs to know the parser
to use.

src/complete.cpp

@@ -543,7 +543,7 @@ void completer_t::complete_strings(const wcstring &wc_escaped, const description
     wcstring tmp = wc_escaped;
     if (!expand_one(tmp,
                     this->expand_flags() | expand_flag::skip_cmdsubst | expand_flag::skip_wildcards,
-                    vars))
+                    vars, parser))
         return;
 
     const wcstring wc = parse_util_unescape_wildcards(tmp);
@@ -666,7 +666,7 @@ void completer_t::complete_cmd(const wcstring &str_cmd, bool use_function, bool
             expand_string(str_cmd, &this->completions,
                           this->expand_flags() | expand_flag::special_for_command |
                               expand_flag::for_completions | expand_flag::executables_only,
-                          vars, NULL);
+                          vars, parser, NULL);
         if (result != expand_result_t::error && this->wants_descriptions()) {
             this->complete_cmd_desc(str_cmd);
         }
@@ -680,7 +680,7 @@ void completer_t::complete_cmd(const wcstring &str_cmd, bool use_function, bool
             expand_string(
                 str_cmd, &this->completions,
                 this->expand_flags() | expand_flag::for_completions | expand_flag::directories_only,
-                vars, NULL);
+                vars, parser, NULL);
         UNUSED(ignore);
     }
 
@@ -752,8 +752,8 @@ void completer_t::complete_from_args(const wcstring &str, const wcstring &args,
         eflags |= expand_flag::skip_cmdsubst;
     }
 
-    std::vector<completion_t> possible_comp;
+    std::vector<completion_t> possible_comp =
-    parser_t::expand_argument_list(args, eflags, vars, &possible_comp);
+        parser_t::expand_argument_list(args, eflags, vars, parser);
 
     if (!is_autosuggest) {
         proc_pop_interactive();
@@ -1103,7 +1103,7 @@ void completer_t::complete_param_expand(const wcstring &str, bool do_file,
         // See #4954.
         const wcstring sep_string = wcstring(str, sep_index + 1);
         std::vector<completion_t> local_completions;
-        if (expand_string(sep_string, &local_completions, flags, vars, NULL) ==
+        if (expand_string(sep_string, &local_completions, flags, vars, parser, NULL) ==
             expand_result_t::error) {
             debug(3, L"Error while expanding string '%ls'", sep_string.c_str());
         }
@@ -1123,7 +1123,8 @@ void completer_t::complete_param_expand(const wcstring &str, bool do_file,
         // consider relaxing this if there was a preceding double-dash argument.
         if (string_prefixes_string(L"-", str)) flags.clear(expand_flag::fuzzy_match);
 
-        if (expand_string(str, &this->completions, flags, vars, NULL) == expand_result_t::error) {
+        if (expand_string(str, &this->completions, flags, vars, parser, NULL) ==
+            expand_result_t::error) {
             debug(3, L"Error while expanding string '%ls'", str.c_str());
         }
     }

src/expand.cpp

@@ -589,7 +589,7 @@ static expand_result_t expand_braces(const wcstring &instr, expand_flags_t flags
 }
 
 /// Perform cmdsubst expansion.
-static bool expand_cmdsubst(const wcstring &input, std::vector<completion_t> *out_list,
+static bool expand_cmdsubst(wcstring input, parser_t &parser, std::vector<completion_t> *out_list,
                             parse_error_list_t *errors) {
     wchar_t *paren_begin = nullptr, *paren_end = nullptr;
     wchar_t *tail_begin = nullptr;
@@ -603,7 +603,7 @@ static bool expand_cmdsubst(const wcstring &input, std::vector<completion_t> *ou
             return false;
         }
         case 0: {
-            append_completion(out_list, input);
+            append_completion(out_list, std::move(input));
             return true;
         }
         case 1: {
@@ -617,8 +617,6 @@ static bool expand_cmdsubst(const wcstring &input, std::vector<completion_t> *ou
 
     wcstring_list_t sub_res;
     const wcstring subcmd(paren_begin + 1, paren_end - paren_begin - 1);
-    // TODO: justify this parser_t::principal_parser
-    auto &parser = parser_t::principal_parser();
     if (exec_subshell(subcmd, parser, sub_res, true /* apply_exit_status */,
                       true /* is_subcmd */) == -1) {
         append_cmdsub_error(errors, SOURCE_LOCATION_UNKNOWN,
@@ -667,7 +665,7 @@ static bool expand_cmdsubst(const wcstring &input, std::vector<completion_t> *ou
     // Recursively call ourselves to expand any remaining command substitutions. The result of this
     // recursive call using the tail of the string is inserted into the tail_expand array list
     std::vector<completion_t> tail_expand;
-    expand_cmdsubst(tail_begin, &tail_expand, errors);  // TODO: offset error locations
+    expand_cmdsubst(tail_begin, parser, &tail_expand, errors);  // TODO: offset error locations
 
     // Combine the result of the current command substitution with the result of the recursive tail
     // expansion.
@@ -871,6 +869,9 @@ class expander_t {
     /// Variables to use in expansion.
     const environment_t &vars;
 
+    /// The parser for expanding command substitutions, or nullptr if not enabled.
+    std::shared_ptr<parser_t> parser;
+
     /// Flags to use during expansion.
     const expand_flags_t flags;
 
@@ -888,12 +889,14 @@ class expander_t {
     expand_result_t stage_home_and_self(wcstring input, std::vector<completion_t> *out);
     expand_result_t stage_wildcards(wcstring path_to_expand, std::vector<completion_t> *out);
 
-    expander_t(const environment_t &vars, expand_flags_t flags, parse_error_list_t *errors)
+    expander_t(const environment_t &vars, const std::shared_ptr<parser_t> &parser,
-        : vars(vars), flags(flags), errors(errors) {}
+               expand_flags_t flags, parse_error_list_t *errors)
+        : vars(vars), parser(parser), flags(flags), errors(errors) {}
 
    public:
     static expand_result_t expand_string(wcstring input, std::vector<completion_t> *out_completions,
                                          expand_flags_t flags, const environment_t &vars,
+                                         const std::shared_ptr<parser_t> &parser,
                                          parse_error_list_t *errors);
 };
 
@@ -911,7 +914,8 @@ expand_result_t expander_t::stage_cmdsubst(wcstring input, std::vector<completio
                 return expand_result_t::error;
         }
     } else {
-        bool cmdsubst_ok = expand_cmdsubst(std::move(input), out, errors);
+        assert(parser && "Must have a parser to expand command substitutions");
+        bool cmdsubst_ok = expand_cmdsubst(std::move(input), *parser, out, errors);
         if (!cmdsubst_ok) return expand_result_t::error;
     }
 
@@ -1049,14 +1053,17 @@ expand_result_t expander_t::stage_wildcards(wcstring path_to_expand,
 expand_result_t expander_t::expand_string(wcstring input,
                                           std::vector<completion_t> *out_completions,
                                           expand_flags_t flags, const environment_t &vars,
+                                          const std::shared_ptr<parser_t> &parser,
                                           parse_error_list_t *errors) {
+    assert(((flags & expand_flag::skip_cmdsubst) || parser) &&
+           "Must have a parser if not skipping command substitutions");
     // Early out. If we're not completing, and there's no magic in the input, we're done.
     if (!(flags & expand_flag::for_completions) && expand_is_clean(input)) {
         append_completion(out_completions, std::move(input));
         return expand_result_t::ok;
     }
 
-    expander_t expand(vars, flags, errors);
+    expander_t expand(vars, parser, flags, errors);
 
     // Our expansion stages.
     const stage_t stages[] = {&expander_t::stage_cmdsubst, &expander_t::stage_variables,
@@ -1106,20 +1113,21 @@ expand_result_t expander_t::expand_string(wcstring input,
 
 expand_result_t expand_string(wcstring input, std::vector<completion_t> *out_completions,
                               expand_flags_t flags, const environment_t &vars,
-                              parse_error_list_t *errors) {
+                              const shared_ptr<parser_t> &parser, parse_error_list_t *errors) {
-    return expander_t::expand_string(std::move(input), out_completions, flags, vars, errors);
+    return expander_t::expand_string(std::move(input), out_completions, flags, vars, parser,
+                                     errors);
 }
 
 bool expand_one(wcstring &string, expand_flags_t flags, const environment_t &vars,
-                parse_error_list_t *errors) {
+                const shared_ptr<parser_t> &parser, parse_error_list_t *errors) {
     std::vector<completion_t> completions;
 
     if (!flags.get(expand_flag::for_completions) && expand_is_clean(string)) {
         return true;
     }
 
-    if (expand_string(string, &completions, flags | expand_flag::no_descriptions, vars, errors) !=
+    if (expand_string(string, &completions, flags | expand_flag::no_descriptions, vars, parser,
-            expand_result_t::error &&
+                      errors) != expand_result_t::error &&
         completions.size() == 1) {
         string = std::move(completions.at(0).completion);
         return true;
@@ -1141,7 +1149,7 @@ expand_result_t expand_to_command_and_args(const wcstring &instr, const environm
     expand_result_t expand_err = expand_string(
         instr, &completions,
         {expand_flag::skip_cmdsubst, expand_flag::no_descriptions, expand_flag::skip_jobs}, vars,
-        errors);
+        nullptr, errors);
     if (expand_err == expand_result_t::ok || expand_err == expand_result_t::wildcard_match) {
         // The first completion is the command, any remaning are arguments.
         bool first = true;

src/expand.h

@@ -125,13 +125,16 @@ enum class expand_result_t {
 /// \param flags Specifies if any expansion pass should be skipped. Legal values are any combination
 /// of skip_cmdsubst skip_variables and skip_wildcards
 /// \param vars variables used during expansion.
+/// \param parser the parser to use for command substitutions, or nullptr to disable.
 /// \param errors Resulting errors, or NULL to ignore
 ///
 /// \return An expand_result_t.
 /// wildcard_no_match and wildcard_match are normal exit conditions used only on
 /// strings containing wildcards to tell if the wildcard produced any matches.
+class parser_t;
 __warn_unused expand_result_t expand_string(wcstring input, std::vector<completion_t> *output,
                                             expand_flags_t flags, const environment_t &vars,
+                                            const std::shared_ptr<parser_t> &parser,
                                             parse_error_list_t *errors);
 
 /// expand_one is identical to expand_string, except it will fail if in expands to more than one
@@ -140,11 +143,12 @@ __warn_unused expand_result_t expand_string(wcstring input, std::vector<completi
 /// \param inout_str The parameter to expand in-place
 /// \param flags Specifies if any expansion pass should be skipped. Legal values are any combination
 /// of skip_cmdsubst skip_variables and skip_wildcards
+/// \param parser the parser to use for command substitutions, or nullptr to disable.
 /// \param errors Resulting errors, or NULL to ignore
 ///
 /// \return Whether expansion succeded
 bool expand_one(wcstring &inout_str, expand_flags_t flags, const environment_t &vars,
-                parse_error_list_t *errors = NULL);
+                const std::shared_ptr<parser_t> &parser, parse_error_list_t *errors = NULL);
 
 /// Expand a command string like $HOME/bin/cmd into a command and list of arguments.
 /// Return the command and arguments by reference.

src/fish_tests.cpp

@@ -760,6 +760,8 @@ static parser_test_error_bits_t detect_argument_errors(const wcstring &src) {
 static void test_parser() {
     say(L"Testing parser");
 
+    auto parser = parser_t::principal_parser().shared();
+
     say(L"Testing block nesting");
     if (!parse_util_detect_errors(L"if; end")) {
         err(L"Incomplete if statement undetected");
@@ -963,23 +965,20 @@ static void test_parser() {
     // Ensure that we don't crash on infinite self recursion and mutual recursion. These must use
     // the principal parser because we cannot yet execute jobs on other parsers.
     say(L"Testing recursion detection");
-    parser_t::principal_parser().eval(L"function recursive ; recursive ; end ; recursive; ",
+    parser->eval(L"function recursive ; recursive ; end ; recursive; ", io_chain_t(), TOP);
-                                      io_chain_t(), TOP);
 #if 0
     // This is disabled since it produces a long backtrace. We should find a way to either visually
     // compress the backtrace, or disable error spewing.
-    parser_t::principal_parser().eval(L"function recursive1 ; recursive2 ; end ; "
+    parser->.eval(L"function recursive1 ; recursive2 ; end ; "
             L"function recursive2 ; recursive1 ; end ; recursive1; ", io_chain_t(), TOP);
 #endif
 
     say(L"Testing empty function name");
-    parser_t::principal_parser().eval(L"function '' ; echo fail; exit 42 ; end ; ''", io_chain_t(),
+    parser->eval(L"function '' ; echo fail; exit 42 ; end ; ''", io_chain_t(), TOP);
-                                      TOP);
 
     say(L"Testing eval_args");
-    completion_list_t comps;
+    completion_list_t comps = parser_t::expand_argument_list(
-    parser_t::expand_argument_list(L"alpha 'beta gamma' delta", expand_flags_t{},
+        L"alpha 'beta gamma' delta", expand_flags_t{}, parser->vars(), parser);
-                                   null_environment_t{}, &comps);
     do_test(comps.size() == 3);
     do_test(comps.at(0).completion == L"alpha");
     do_test(comps.at(1).completion == L"beta gamma");
@@ -1600,8 +1599,10 @@ static bool expand_test(const wchar_t *in, expand_flags_t flags, ...) {
     bool res = true;
     wchar_t *arg;
     parse_error_list_t errors;
+    auto parser = parser_t::principal_parser().shared();
 
-    if (expand_string(in, &output, flags, pwd_environment_t{}, &errors) == expand_result_t::error) {
+    if (expand_string(in, &output, flags, pwd_environment_t{}, parser, &errors) ==
+        expand_result_t::error) {
         if (errors.empty()) {
             err(L"Bug: Parse error reported but no error text found.");
         } else {
@@ -2259,15 +2260,15 @@ static bool run_one_test_test(int expected, wcstring_list_t &lst, bool bracket)
 }
 
 static bool run_test_test(int expected, const wcstring &str) {
-    using namespace std;
-    wcstring_list_t argv;
-    completion_list_t comps;
-
     // We need to tokenize the string in the same manner a normal shell would do. This is because we
     // need to test things like quoted strings that have leading and trailing whitespace.
-    parser_t::expand_argument_list(str, expand_flags_t{}, null_environment_t{}, &comps);
+    auto parser = parser_t::principal_parser().shared();
-    for (completion_list_t::const_iterator it = comps.begin(), end = comps.end(); it != end; ++it) {
+    completion_list_t comps =
-        argv.push_back(it->completion);
+        parser_t::expand_argument_list(str, expand_flags_t{}, null_environment_t{}, parser);
+
+    wcstring_list_t argv;
+    for (const auto &c : comps) {
+        argv.push_back(c.completion);
     }
 
     bool bracket = run_one_test_test(expected, argv, true);

src/highlight.cpp

@@ -432,7 +432,7 @@ bool autosuggest_validate_from_history(const history_item_t &item,
 
     if (parsed_command == L"cd" && !cd_dir.empty()) {
         // We can possibly handle this specially.
-        if (expand_one(cd_dir, expand_flag::skip_cmdsubst, vars)) {
+        if (expand_one(cd_dir, expand_flag::skip_cmdsubst, vars, nullptr)) {
             handled = true;
             bool is_help =
                 string_prefixes_string(cd_dir, L"--help") || string_prefixes_string(cd_dir, L"-h");
@@ -926,7 +926,7 @@ void highlighter_t::color_arguments(const std::vector<tnode_t<g::argument>> &arg
         if (cmd_is_cd) {
             // Mark this as an error if it's not 'help' and not a valid cd path.
             wcstring param = arg.get_source(this->buff);
-            if (expand_one(param, expand_flag::skip_cmdsubst, vars)) {
+            if (expand_one(param, expand_flag::skip_cmdsubst, vars, nullptr)) {
                 bool is_help = string_prefixes_string(param, L"--help") ||
                                string_prefixes_string(param, L"-h");
                 if (!is_help && this->io_ok &&
@@ -969,7 +969,7 @@ void highlighter_t::color_redirection(tnode_t<g::redirection> redirection_node)
                 // I/O is disallowed, so we don't have much hope of catching anything but gross
                 // errors. Assume it's valid.
                 target_is_valid = true;
-            } else if (!expand_one(target, expand_flag::skip_cmdsubst, vars)) {
+            } else if (!expand_one(target, expand_flag::skip_cmdsubst, vars, nullptr)) {
                 // Could not be expanded.
                 target_is_valid = false;
             } else {

src/parse_execution.cpp

@@ -128,8 +128,8 @@ tnode_t<g::plain_statement> parse_execution_context_t::infinite_recursive_statem
         if (plain_statement) {
             maybe_t<wcstring> cmd = command_for_plain_statement(plain_statement, pstree->src);
             if (cmd &&
-                expand_one(*cmd, {expand_flag::skip_cmdsubst, expand_flag::skip_variables},
+                expand_one(*cmd, {expand_flag::skip_cmdsubst, expand_flag::skip_variables}, nullenv,
-                           nullenv) &&
+                           nullptr) &&
                 cmd == forbidden_function_name) {
                 // This is it.
                 infinite_recursive_statement = plain_statement;
@@ -378,7 +378,7 @@ parse_execution_result_t parse_execution_context_t::run_for_statement(
     // in just one.
     tnode_t<g::tok_string> var_name_node = header.child<1>();
     wcstring for_var_name = get_source(var_name_node);
-    if (!expand_one(for_var_name, expand_flags_t{}, parser->vars())) {
+    if (!expand_one(for_var_name, expand_flags_t{}, parser->vars(), parser->shared())) {
         report_error(var_name_node, FAILED_EXPANSION_VARIABLE_NAME_ERR_MSG, for_var_name.c_str());
         return parse_execution_errored;
     }
@@ -451,8 +451,9 @@ parse_execution_result_t parse_execution_context_t::run_switch_statement(
     // Expand it. We need to offset any errors by the position of the string.
     std::vector<completion_t> switch_values_expanded;
     parse_error_list_t errors;
-    auto expand_ret = expand_string(switch_value, &switch_values_expanded,
+    auto expand_ret =
-                                    expand_flag::no_descriptions, parser->vars(), &errors);
+        expand_string(switch_value, &switch_values_expanded, expand_flag::no_descriptions,
+                      parser->vars(), parser->shared(), &errors);
     parse_error_offset_source_start(&errors, switch_value_n.source_range()->start);
 
     switch (expand_ret) {
@@ -909,7 +910,7 @@ parse_execution_result_t parse_execution_context_t::expand_arguments_from_nodes(
         parse_error_list_t errors;
         arg_expanded.clear();
         auto expand_ret = expand_string(arg_str, &arg_expanded, expand_flag::no_descriptions,
-                                        parser->vars(), &errors);
+                                        parser->vars(), parser->shared(), &errors);
         parse_error_offset_source_start(&errors, arg_node.source_range()->start);
         switch (expand_ret) {
             case expand_result_t::error: {
@@ -957,8 +958,9 @@ bool parse_execution_context_t::determine_io_chain(tnode_t<g::arguments_or_redir
         auto redirect_type = redirection_type(redirect_node, pstree->src, &source_fd, &target);
 
         // PCA: I can't justify this skip_variables flag. It was like this when I got here.
-        bool target_expanded = expand_one(
+        bool target_expanded =
-            target, no_exec ? expand_flag::skip_variables : expand_flags_t{}, parser->vars());
+            expand_one(target, no_exec ? expand_flag::skip_variables : expand_flags_t{},
+                       parser->vars(), parser->shared());
         if (!target_expanded || target.empty()) {
             // TODO: Improve this error message.
             errored =

src/parse_util.cpp

@@ -1197,7 +1197,8 @@ static bool detect_errors_in_plain_statement(const wcstring &buff_src,
 
         // Check that we don't do an invalid builtin (issue #1252).
         if (!errored && decoration == parse_statement_decoration_builtin &&
-            expand_one(*unexp_command, expand_flags_t{}, null_environment_t{}, parse_errors) &&
+            expand_one(*unexp_command, expand_flag::skip_cmdsubst, null_environment_t{}, nullptr,
+                       parse_errors) &&
             !builtin_exists(*unexp_command)) {
             errored = append_syntax_error(parse_errors, source_start, UNKNOWN_BUILTIN_ERR_MSG,
                                           unexp_command->c_str());

src/parser.cpp

@@ -313,29 +313,30 @@ void parser_t::emit_profiling(const char *path) const {
     }
 }
 
-void parser_t::expand_argument_list(const wcstring &arg_list_src, expand_flags_t eflags,
+std::vector<completion_t> parser_t::expand_argument_list(const wcstring &arg_list_src,
-                                    const environment_t &vars,
+                                                         expand_flags_t eflags,
-                                    std::vector<completion_t> *output_arg_list) {
+                                                         const environment_t &vars,
-    assert(output_arg_list != NULL);
+                                                         const std::shared_ptr<parser_t> &parser) {
-
     // Parse the string as an argument list.
     parse_node_tree_t tree;
     if (!parse_tree_from_string(arg_list_src, parse_flag_none, &tree, NULL /* errors */,
                                 symbol_freestanding_argument_list)) {
         // Failed to parse. Here we expect to have reported any errors in test_args.
-        return;
+        return {};
     }
 
     // Get the root argument list and extract arguments from it.
-    assert(!tree.empty());  //!OCLINT(multiple unary operator)
+    std::vector<completion_t> result;
+    assert(!tree.empty());
     tnode_t<grammar::freestanding_argument_list> arg_list(&tree, &tree.at(0));
     while (auto arg = arg_list.next_in_list<grammar::argument>()) {
         const wcstring arg_src = arg.get_source(arg_list_src);
-        if (expand_string(arg_src, output_arg_list, eflags, vars, NULL /* errors */) ==
+        if (expand_string(arg_src, &result, eflags, vars, parser, NULL /* errors */) ==
             expand_result_t::error) {
             break;  // failed to expand a string
         }
     }
+    return result;
 }
 
 wcstring parser_t::stack_trace() const {

src/parser.h

@@ -235,13 +235,12 @@ class parser_t : public std::enable_shared_from_this<parser_t> {
                   block_type_t block_type, std::shared_ptr<job_t> parent_job);
 
     /// Evaluate line as a list of parameters, i.e. tokenize it and perform parameter expansion and
-    /// cmdsubst execution on the tokens. The output is inserted into output. Errors are ignored.
+    /// cmdsubst execution on the tokens. Errors are ignored. If a parser is provided, it is used
-    ///
+    /// for command substitution expansion.
-    /// \param arg_src String to evaluate as an argument list
+    static std::vector<completion_t> expand_argument_list(const wcstring &arg_list_src,
-    /// \param flags Some expand flags to use
+                                                          expand_flags_t flags,
-    /// \param output List to insert output into
+                                                          const environment_t &vars,
-    static void expand_argument_list(const wcstring &arg_src, expand_flags_t flags,
+                                                          const std::shared_ptr<parser_t> &parser);
-                                     const environment_t &vars, std::vector<completion_t> *output);
 
     /// Returns a string describing the current parser position in the format 'FILENAME (line
     /// LINE_NUMBER): LINE'. Example: