From bffacd2fbf8e111cea25c81252409c14a4fa6533 Mon Sep 17 00:00:00 2001 From: ridiculousfish Date: Sat, 4 May 2019 19:16:26 -0700 Subject: [PATCH] Thread a parser into expansion Expansion may perform command substitution, which needs to know the parser to use. --- src/complete.cpp | 15 ++++++++------- src/expand.cpp | 38 +++++++++++++++++++++++--------------- src/expand.h | 6 +++++- src/fish_tests.cpp | 33 +++++++++++++++++---------------- src/highlight.cpp | 6 +++--- src/parse_execution.cpp | 18 ++++++++++-------- src/parse_util.cpp | 3 ++- src/parser.cpp | 17 +++++++++-------- src/parser.h | 13 ++++++------- 9 files changed, 83 insertions(+), 66 deletions(-) diff --git a/src/complete.cpp b/src/complete.cpp index 3cccc71af..d7bb22017 100644 --- a/src/complete.cpp +++ b/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 possible_comp; - parser_t::expand_argument_list(args, eflags, vars, &possible_comp); + std::vector 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 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()); } } diff --git a/src/expand.cpp b/src/expand.cpp index fdb4e5a52..a163fce9c 100644 --- a/src/expand.cpp +++ b/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 *out_list, +static bool expand_cmdsubst(wcstring input, parser_t &parser, std::vector *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 *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 *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 *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 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; + /// 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 *out); expand_result_t stage_wildcards(wcstring path_to_expand, std::vector *out); - expander_t(const environment_t &vars, expand_flags_t flags, parse_error_list_t *errors) - : vars(vars), flags(flags), errors(errors) {} + expander_t(const environment_t &vars, const std::shared_ptr &parser, + 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 *out_completions, expand_flags_t flags, const environment_t &vars, + const std::shared_ptr &parser, parse_error_list_t *errors); }; @@ -911,7 +914,8 @@ expand_result_t expander_t::stage_cmdsubst(wcstring input, std::vector *out_completions, expand_flags_t flags, const environment_t &vars, + const std::shared_ptr &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 *out_completions, expand_flags_t flags, const environment_t &vars, - parse_error_list_t *errors) { - return expander_t::expand_string(std::move(input), out_completions, flags, vars, errors); + const shared_ptr &parser, parse_error_list_t *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, parse_error_list_t *errors) { std::vector 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) != - expand_result_t::error && + if (expand_string(string, &completions, flags | expand_flag::no_descriptions, vars, parser, + 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; diff --git a/src/expand.h b/src/expand.h index 07a05e17d..b492a2a18 100644 --- a/src/expand.h +++ b/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 *output, expand_flags_t flags, const environment_t &vars, + const std::shared_ptr &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 &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. diff --git a/src/fish_tests.cpp b/src/fish_tests.cpp index 3ac77c940..2d24a2978 100644 --- a/src/fish_tests.cpp +++ b/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; ", - io_chain_t(), TOP); + parser->eval(L"function recursive ; recursive ; end ; recursive; ", 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(), - TOP); + parser->eval(L"function '' ; echo fail; exit 42 ; end ; ''", io_chain_t(), TOP); say(L"Testing eval_args"); - completion_list_t comps; - parser_t::expand_argument_list(L"alpha 'beta gamma' delta", expand_flags_t{}, - null_environment_t{}, &comps); + completion_list_t comps = parser_t::expand_argument_list( + L"alpha 'beta gamma' delta", expand_flags_t{}, parser->vars(), parser); 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); - for (completion_list_t::const_iterator it = comps.begin(), end = comps.end(); it != end; ++it) { - argv.push_back(it->completion); + auto parser = parser_t::principal_parser().shared(); + completion_list_t comps = + 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); diff --git a/src/highlight.cpp b/src/highlight.cpp index 7d8f49bf3..e79627afc 100644 --- a/src/highlight.cpp +++ b/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> &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 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 { diff --git a/src/parse_execution.cpp b/src/parse_execution.cpp index cb79e9e5f..5ec8152e1 100644 --- a/src/parse_execution.cpp +++ b/src/parse_execution.cpp @@ -128,8 +128,8 @@ tnode_t parse_execution_context_t::infinite_recursive_statem if (plain_statement) { maybe_t cmd = command_for_plain_statement(plain_statement, pstree->src); if (cmd && - expand_one(*cmd, {expand_flag::skip_cmdsubst, expand_flag::skip_variables}, - nullenv) && + expand_one(*cmd, {expand_flag::skip_cmdsubst, expand_flag::skip_variables}, 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 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 switch_values_expanded; parse_error_list_t errors; - auto expand_ret = expand_string(switch_value, &switch_values_expanded, - expand_flag::no_descriptions, parser->vars(), &errors); + auto expand_ret = + 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_tsrc, &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( - target, no_exec ? expand_flag::skip_variables : expand_flags_t{}, parser->vars()); + bool target_expanded = + 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 = diff --git a/src/parse_util.cpp b/src/parse_util.cpp index 9a75507f8..00ad41d39 100644 --- a/src/parse_util.cpp +++ b/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()); diff --git a/src/parser.cpp b/src/parser.cpp index c440b7828..1f59adc7c 100644 --- a/src/parser.cpp +++ b/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, - const environment_t &vars, - std::vector *output_arg_list) { - assert(output_arg_list != NULL); - +std::vector parser_t::expand_argument_list(const wcstring &arg_list_src, + expand_flags_t eflags, + const environment_t &vars, + const std::shared_ptr &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 result; + assert(!tree.empty()); tnode_t arg_list(&tree, &tree.at(0)); while (auto arg = arg_list.next_in_list()) { 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 { diff --git a/src/parser.h b/src/parser.h index 858f5bf7d..83cecab7b 100644 --- a/src/parser.h +++ b/src/parser.h @@ -235,13 +235,12 @@ class parser_t : public std::enable_shared_from_this { block_type_t block_type, std::shared_ptr 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. - /// - /// \param arg_src String to evaluate as an argument list - /// \param flags Some expand flags to use - /// \param output List to insert output into - static void expand_argument_list(const wcstring &arg_src, expand_flags_t flags, - const environment_t &vars, std::vector *output); + /// cmdsubst execution on the tokens. Errors are ignored. If a parser is provided, it is used + /// for command substitution expansion. + static std::vector expand_argument_list(const wcstring &arg_list_src, + expand_flags_t flags, + const environment_t &vars, + const std::shared_ptr &parser); /// Returns a string describing the current parser position in the format 'FILENAME (line /// LINE_NUMBER): LINE'. Example: