From 95f87cdd56602fe7c2746519f875741c325d2df8 Mon Sep 17 00:00:00 2001 From: ridiculousfish Date: Mon, 13 Jan 2014 02:24:11 -0800 Subject: [PATCH] Support for special && and || error messages in new parser --- fish_tests.cpp | 5 ++++- parse_constants.h | 16 +++++++-------- parse_tree.cpp | 51 +++++++++++++++++++++++++++++++++++++---------- parse_tree.h | 2 +- parser.cpp | 17 ++++------------ 5 files changed, 57 insertions(+), 34 deletions(-) diff --git a/fish_tests.cpp b/fish_tests.cpp index e3da9a7f3..319a3872a 100644 --- a/fish_tests.cpp +++ b/fish_tests.cpp @@ -2444,7 +2444,10 @@ static void test_new_parser_errors(void) {L"if true ; end ; else", parse_error_unbalancing_else}, {L"case", parse_error_unbalancing_case}, - {L"if true ; case ; end", parse_error_unbalancing_case} + {L"if true ; case ; end", parse_error_unbalancing_case}, + + {L"foo || bar", parse_error_double_pipe}, + {L"foo && bar", parse_error_double_background}, }; for (size_t i = 0; i < sizeof tests / sizeof *tests; i++) diff --git a/parse_constants.h b/parse_constants.h index 266d27e1c..e910507a6 100644 --- a/parse_constants.h +++ b/parse_constants.h @@ -119,6 +119,9 @@ enum parse_error_code_t parse_error_unbalancing_end, //end outside of block parse_error_unbalancing_else, //else outside of if parse_error_unbalancing_case, //case outside of switch + + parse_error_double_pipe, // foo || bar, has special error message + parse_error_double_background // foo && bar, has special error message }; enum { @@ -138,18 +141,15 @@ typedef unsigned int parser_test_error_bits_t; /** Error message on reaching maximum call stack depth */ #define CALL_STACK_LIMIT_EXCEEDED_ERR_MSG _( L"The function call stack limit has been exceeded. Do you have an accidental infinite loop?") -/** Error message when a non-string token is found when expecting a command name */ -#define CMD_ERR_MSG _( L"Expected a command name, got token of type '%ls'") +/** + Error message when a non-string token is found when expecting a command name +*/ +#define CMD_OR_ERR_MSG _( L"Expected a command, but instead found a pipe. Did you mean 'COMMAND; or COMMAND'? See the help section for the 'or' builtin command by typing 'help or'.") /** Error message when a non-string token is found when expecting a command name */ -#define CMD_OR_ERR_MSG _( L"Expected a command name, got token of type '%ls'. Did you mean 'COMMAND; or COMMAND'? See the help section for the 'or' builtin command by typing 'help or'.") - -/** - Error message when a non-string token is found when expecting a command name -*/ -#define CMD_AND_ERR_MSG _( L"Expected a command name, got token of type '%ls'. Did you mean 'COMMAND; and COMMAND'? See the help section for the 'and' builtin command by typing 'help and'.") +#define CMD_AND_ERR_MSG _( L"Expected a command, but instead found a '&'. Did you mean 'COMMAND; and COMMAND'? See the help section for the 'and' builtin command by typing 'help and'.") /** Error message when encountering an illegal command name diff --git a/parse_tree.cpp b/parse_tree.cpp index b55f43bb5..41873bbe9 100644 --- a/parse_tree.cpp +++ b/parse_tree.cpp @@ -1,6 +1,7 @@ #include "parse_productions.h" #include "tokenizer.h" #include "fallback.h" +#include "wutil.h" #include "proc.h" #include #include @@ -746,8 +747,43 @@ void parse_ll_t::parse_error_unbalancing_token(parse_token_t token) // This is a 'generic' parse error when we can't match the top of the stack element void parse_ll_t::parse_error_failed_production(struct parse_stack_element_t &stack_elem, parse_token_t token) { - const wcstring expected = stack_elem.user_presentable_description(); - this->parse_error(expected.c_str(), token); + fatal_errored = true; + if (this->should_generate_error_messages) + { + bool done = false; + + /* Check for || */ + if (token.type == parse_token_type_pipe && token.source_start > 0) + { + /* Here we wanted a statement and instead got a pipe. See if this is a double pipe: foo || bar. If so, we have a special error message. */ + const parse_node_t *prev_pipe = nodes.find_node_matching_source_location(parse_token_type_pipe, token.source_start - 1, NULL); + if (prev_pipe != NULL) + { + /* The pipe of the previous job abuts our current token. So we have ||. */ + this->parse_error(token, parse_error_double_pipe, CMD_OR_ERR_MSG); + done = true; + } + } + + /* Check for && */ + if (! done && token.type == parse_token_type_background && token.source_start > 0) + { + /* Check to see if there was a previous token_background */ + const parse_node_t *prev_background = nodes.find_node_matching_source_location(parse_token_type_background, token.source_start - 1, NULL); + if (prev_background != NULL) + { + /* We have &&. */ + this->parse_error(token, parse_error_double_background, CMD_AND_ERR_MSG); + done = true; + } + } + + if (! done) + { + const wcstring expected = stack_elem.user_presentable_description(); + this->parse_error(expected.c_str(), token); + } + } } void parse_ll_t::report_tokenizer_error(parse_token_t token, const wchar_t *tok_error) @@ -936,15 +972,8 @@ void parse_ll_t::accept_tokens(parse_token_t token1, parse_token_t token2) const production_t *production = production_for_token(stack_elem.type, token1, token2, &node.production_idx, NULL /* error text */); if (production == NULL) { - if (should_generate_error_messages) - { - parse_error_failed_production(stack_elem, token1); - } - else - { - this->parse_error(token1, parse_error_generic, NULL); - } - // parse_error sets fatal_errored, which ends the loop + parse_error_failed_production(stack_elem, token1); + // the above sets fatal_errored, which ends the loop } else { diff --git a/parse_tree.h b/parse_tree.h index 6ce82299d..5cc0a4ccc 100644 --- a/parse_tree.h +++ b/parse_tree.h @@ -166,7 +166,7 @@ public: /* Finds the last node of a given type underneath a given node, or NULL if it could not be found. If parent is NULL, this finds the last node in the tree of that type. */ const parse_node_t *find_last_node_of_type(parse_token_type_t type, const parse_node_t *parent = NULL) const; - /* Finds a node containing the given source location */ + /* Finds a node containing the given source location. If 'parent' is not NULL, it must be an ancestor. */ const parse_node_t *find_node_matching_source_location(parse_token_type_t type, size_t source_loc, const parse_node_t *parent) const; /* Indicate if the given argument_list or arguments_or_redirections_list is a root list, or has a parent */ diff --git a/parser.cpp b/parser.cpp index 6831290d1..3995d0393 100644 --- a/parser.cpp +++ b/parser.cpp @@ -78,15 +78,8 @@ The fish parser. Contains functions for parsing and evaluating code. */ #define BLOCK_END_ERR_MSG _( L"Could not locate end of block. The 'end' command is missing, misspelled or a ';' is missing.") -/** - Error message when a non-string token is found when expecting a command name -*/ -#define CMD_OR_ERR_MSG _( L"Expected a command name, got token of type '%ls'. Did you mean 'COMMAND; or COMMAND'? See the help section for the 'or' builtin command by typing 'help or'.") - -/** - Error message when a non-string token is found when expecting a command name -*/ -#define CMD_AND_ERR_MSG _( L"Expected a command name, got token of type '%ls'. Did you mean 'COMMAND; and COMMAND'? See the help section for the 'and' builtin command by typing 'help and'.") +/** Error message when a non-string token is found when expecting a command name */ +#define CMD_ERR_MSG _( L"Expected a command name, got token of type '%ls'") /** Error message when encountering an illegal command name @@ -1692,8 +1685,7 @@ int parser_t::parse_job(process_t *p, job_t *j, tokenizer_t *tok) { error(SYNTAX_ERROR, tok_get_pos(tok), - CMD_OR_ERR_MSG, - tok_get_desc(tok_last_type(tok))); + CMD_OR_ERR_MSG); } else { @@ -2530,8 +2522,7 @@ void parser_t::eval_job(tokenizer_t *tok) { error(SYNTAX_ERROR, tok_get_pos(tok), - CMD_AND_ERR_MSG, - tok_get_desc(tok_last_type(tok))); + CMD_AND_ERR_MSG); } else {