Support for special && and || error messages in new parser

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++)

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'.")
+#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 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 encountering an illegal command name

parse_tree.cpp

@@ -1,6 +1,7 @@
 #include "parse_productions.h"
 #include "tokenizer.h"
 #include "fallback.h"
+#include "wutil.h"
 #include "proc.h"
 #include <vector>
 #include <algorithm>
@@ -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();
+    fatal_errored = true;
-    this->parse_error(expected.c_str(), token);
+    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);
-            {
+            // the above sets fatal_errored, which ends the loop
-                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
         }
         else
         {

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 */

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 */
-   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'")
-*/
-#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 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,
+                          CMD_OR_ERR_MSG);
-                          tok_get_desc(tok_last_type(tok)));
                 }
                 else
                 {
@@ -2530,8 +2522,7 @@ void parser_t::eval_job(tokenizer_t *tok)
             {
                 error(SYNTAX_ERROR,
                       tok_get_pos(tok),
-                      CMD_AND_ERR_MSG,
+                      CMD_AND_ERR_MSG);
-                      tok_get_desc(tok_last_type(tok)));
             }
             else
             {