diff --git a/CHANGELOG.md b/CHANGELOG.md index 96af6eeb4..b14d8d9ae 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -31,6 +31,7 @@ This section is for changes merged to the `major` branch that are not also merge - `cd` tab completions no longer descend into the deepest unambiguous path (#4649) - Setting `$PATH` no longer warns on non-existent directories, allowing for a single $PATH to be shared across machines (e.g. via dotfiles). - `funced` now has a `-s` and `--save` option to automatically save the edited function after successfully editing (#4668). +- Arguments to `end` are now errors, instead of being silently ignored. ## Other significant changes - Command substitution output is now limited to 10 MB by default (#3822). diff --git a/src/fish_tests.cpp b/src/fish_tests.cpp index d6eb34bcf..1ae6882a3 100644 --- a/src/fish_tests.cpp +++ b/src/fish_tests.cpp @@ -675,6 +675,26 @@ static void test_parser() { err(L"'exec' command in pipeline not reported as error"); } + if (!parse_util_detect_errors(L"begin ; end arg")) { + err(L"argument to 'end' not reported as error"); + } + + if (!parse_util_detect_errors(L"switch foo ; end arg")) { + err(L"argument to 'end' not reported as error"); + } + + if (!parse_util_detect_errors(L"if true; else if false ; end arg")) { + err(L"argument to 'end' not reported as error"); + } + + if (!parse_util_detect_errors(L"if true; else ; end arg")) { + err(L"argument to 'end' not reported as error"); + } + + if (parse_util_detect_errors(L"begin ; end 2> /dev/null")) { + err(L"redirection after 'end' wrongly reported as error"); + } + if (detect_argument_errors(L"foo")) { err(L"simple argument reported as error"); } diff --git a/src/parse_util.cpp b/src/parse_util.cpp index 493eaaa49..66e56523a 100644 --- a/src/parse_util.cpp +++ b/src/parse_util.cpp @@ -35,6 +35,9 @@ #define BACKGROUND_IN_CONDITIONAL_ERROR_MSG \ _(L"Backgrounded commands can not be used as conditionals") +/// Error message for arguments to 'end' +#define END_ARG_ERR_MSG _(L"'end' does not take arguments. Did you forget a ';'?") + int parse_util_lineno(const wchar_t *str, size_t offset) { if (!str) return 0; @@ -1082,6 +1085,7 @@ parser_test_error_bits_t parse_util_detect_errors(const wcstring &buff_src, parse_error_list_t *out_errors, bool allow_incomplete, parsed_source_ref_t *out_pstree) { + namespace g = grammar; parse_node_tree_t node_tree; parse_error_list_t parse_errors; @@ -1141,16 +1145,16 @@ parser_test_error_bits_t parse_util_detect_errors(const wcstring &buff_src, has_unclosed_block = true; } else if (node.type == symbol_boolean_statement) { // 'or' and 'and' can be in a pipeline, as long as they're first. - tnode_t gbs{&node_tree, &node}; + tnode_t gbs{&node_tree, &node}; parse_bool_statement_type_t type = bool_statement_type(gbs); if ((type == parse_bool_and || type == parse_bool_or) && - statement_is_in_pipeline(gbs.try_get_parent(), + statement_is_in_pipeline(gbs.try_get_parent(), false /* don't count first */)) { errored = append_syntax_error(&parse_errors, node.source_start, EXEC_ERR_MSG, (type == parse_bool_and) ? L"and" : L"or"); } } else if (node.type == symbol_argument) { - tnode_t arg{&node_tree, &node}; + tnode_t arg{&node_tree, &node}; const wcstring arg_src = node.get_source(buff_src); res |= parse_util_detect_errors_in_argument(arg, arg_src, &parse_errors); } else if (node.type == symbol_job) { @@ -1161,10 +1165,21 @@ parser_test_error_bits_t parse_util_detect_errors(const wcstring &buff_src, // if foo & ; end // while foo & ; end // If it's not a background job, nothing to do. - auto job = tnode_t{&node_tree, &node}; + auto job = tnode_t{&node_tree, &node}; if (job_node_is_background(job)) { errored |= detect_errors_in_backgrounded_job(job, &parse_errors); } + } else if (node.type == symbol_arguments_or_redirections_list) { + // verify no arguments to the end command of if, switch, begin (#986). + auto list = tnode_t{&node_tree, &node}; + if (list.try_get_parent() || + list.try_get_parent() || + list.try_get_parent()) { + if (auto arg = list.next_in_list()) { + errored = append_syntax_error(&parse_errors, arg.source_range()->start, + END_ARG_ERR_MSG); + } + } } else if (node.type == symbol_plain_statement) { using namespace grammar; tnode_t pst{&node_tree, &node};