Initial work on backtrace support with new parser

7 files changed, 237 insertions, 190 deletions

diff --git a/builtin_complete.cpp b/builtin_complete.cpp
index 0cc3b7e7..f3773f48 100644
--- a/builtin_complete.cpp
+++ b/builtin_complete.cpp
@@ -499,7 +499,7 @@ static int builtin_complete(parser_t &parser, wchar_t **argv)
         {
             const wcstring condition_string = condition;
             parse_error_list_t errors;
-            if (parser.detect_errors(condition_string, &errors))
+            if (parse_util_detect_errors(condition_string, &errors))
             {
                 append_format(stderr_buffer,
                               L"%ls: Condition '%ls' contained a syntax error",
diff --git a/parse_constants.h b/parse_constants.h
index 7ccc962c..e3eebbf5 100644
--- a/parse_constants.h
+++ b/parse_constants.h
@@ -119,6 +119,13 @@ enum parse_error_code_t
     parse_error_unbalancing_case, //case outside of switch
 };
 
+enum {
+  PARSER_TEST_ERROR = 1,
+  PARSER_TEST_INCOMPLETE = 2
+};
+typedef unsigned int parser_test_error_bits_t;
+
+
 
 /**
    Error message for tokenizer error. The tokenizer message is
diff --git a/parse_util.cpp b/parse_util.cpp
index cb33915e..f9567959 100644
--- a/parse_util.cpp
+++ b/parse_util.cpp
@@ -39,18 +39,12 @@
 #include "signal.h"
 #include "wildcard.h"
 #include "parse_tree.h"
+#include "parser.h"
 
 /**
-   Maximum number of autoloaded items opf a specific type to keep in
-   memory at a time.
+   Error message for improper use of the exec builtin
 */
-#define AUTOLOAD_MAX 10
-
-/**
-   Minimum time, in seconds, before an autoloaded item will be
-   unloaded
-*/
-#define AUTOLOAD_MIN_AGE 60
+#define EXEC_ERR_MSG _(L"This command can not be used in a pipeline")
 
 int parse_util_lineno(const wchar_t *str, size_t offset)
 {
@@ -940,3 +934,192 @@ std::vector<int> parse_util_compute_indents(const wcstring &src)
     
     return indents;
 }
+
+/* Append a syntax error to the given error list */
+static bool append_syntax_error(parse_error_list_t *errors, const parse_node_t &node, const wchar_t *fmt, ...)
+{
+    parse_error_t error;
+    error.source_start = node.source_start;
+    error.source_length = node.source_length;
+    error.code = parse_error_syntax;
+    
+    va_list va;
+    va_start(va, fmt);
+    error.text = vformat_string(fmt, va);
+    va_end(va);
+    
+    errors->push_back(error);
+    return true;
+}
+
+/**
+   Returns 1 if the specified command is a builtin that may not be used in a pipeline
+*/
+static int parser_is_pipe_forbidden(const wcstring &word)
+{
+    return contains(word,
+                    L"exec",
+                    L"case",
+                    L"break",
+                    L"return",
+                    L"continue");
+}
+
+// Check if the first argument under the given node is --help
+static bool first_argument_is_help(const parse_node_tree_t &node_tree, const parse_node_t &node, const wcstring &src)
+{
+    bool is_help = false;
+    const parse_node_tree_t::parse_node_list_t arg_nodes = node_tree.find_nodes(node, symbol_argument, 1);
+    if (! arg_nodes.empty())
+    {
+        // Check the first argument only
+        const parse_node_t &arg = *arg_nodes.at(0);
+        const wcstring first_arg_src = arg.get_source(src);
+        is_help = parser_t::is_help(first_arg_src.c_str(), 3);
+    }
+    return is_help;
+}
+
+parser_test_error_bits_t parse_util_detect_errors(const wcstring &buff_src, parse_error_list_t *out_errors)
+{
+    parse_node_tree_t node_tree;
+    parse_error_list_t parse_errors;
+    
+    // Whether we encountered a parse error
+    bool errored = false;
+    
+    // Whether we encountered an unclosed block
+    // We detect this via an 'end_command' block without source
+    bool has_unclosed_block = false;
+    
+    // Parse the input string into a parse tree
+    // Some errors are detected here
+    bool parsed = parse_t::parse(buff_src, 0, &node_tree, &parse_errors);
+    if (! parsed)
+    {
+        errored = true;
+    }
+    
+    // Expand all commands
+    // Verify 'or' and 'and' not used inside pipelines
+    // Verify pipes via parser_is_pipe_forbidden
+    // Verify return only within a function
+    
+    if (! errored)
+    {
+        const size_t node_tree_size = node_tree.size();
+        for (size_t i=0; i < node_tree_size; i++)
+        {
+            const parse_node_t &node = node_tree.at(i);
+            if (node.type == symbol_end_command && ! node.has_source())
+            {
+                // an 'end' without source is an unclosed block
+                has_unclosed_block = true;
+            }
+            else if (node.type == symbol_plain_statement)
+            {
+                wcstring command;
+                if (node_tree.command_for_plain_statement(node, buff_src, &command))
+                {
+                    // Check that we can expand the command
+                    if (! expand_one(command, EXPAND_SKIP_CMDSUBST | EXPAND_SKIP_VARIABLES | EXPAND_SKIP_JOBS))
+                    {
+                        errored = append_syntax_error(&parse_errors, node, ILLEGAL_CMD_ERR_MSG, command.c_str());
+                    }
+                    
+                    // Check that pipes are sound
+                    bool is_boolean_command = contains(command, L"or", L"and");
+                    bool is_pipe_forbidden = parser_is_pipe_forbidden(command);
+                    if (! errored && (is_boolean_command || is_pipe_forbidden))
+                    {
+                        // 'or' and 'and' can be first in the pipeline. forbidden commands cannot be in a pipeline at all
+                        if (node_tree.plain_statement_is_in_pipeline(node, is_pipe_forbidden))
+                        {
+                            errored = append_syntax_error(&parse_errors, node, EXEC_ERR_MSG);
+                        }
+                    }
+                    
+                    // Check that we don't return from outside a function
+                    // But we allow it if it's 'return --help'
+                    if (! errored && command == L"return")
+                    {
+                        const parse_node_t *ancestor = &node;
+                        bool found_function = false;
+                        while (ancestor != NULL)
+                        {
+                            const parse_node_t *possible_function_header = node_tree.header_node_for_block_statement(*ancestor);
+                            if (possible_function_header != NULL && possible_function_header->type == symbol_function_header)
+                            {
+                                found_function = true;
+                                break;
+                            }
+                            ancestor = node_tree.get_parent(*ancestor);
+
+                        }
+                        if (! found_function && ! first_argument_is_help(node_tree, node, buff_src))
+                        {
+                            errored = append_syntax_error(&parse_errors, node, INVALID_RETURN_ERR_MSG);
+                        }
+                    }
+                    
+                    // Check that we don't return from outside a function
+                    if (! errored && (command == L"break" || command == L"continue"))
+                    {
+                        // Walk up until we hit a 'for' or 'while' loop. If we hit a function first, stop the search; we can't break an outer loop from inside a function.
+                        // This is a little funny because we can't tell if it's a 'for' or 'while' loop from the ancestor alone; we need the header. That is, we hit a block_statement, and have to check its header.
+                        bool found_loop = false, end_search = false;
+                        const parse_node_t *ancestor = &node;
+                        while (ancestor != NULL && ! end_search)
+                        {
+                            const parse_node_t *loop_or_function_header = node_tree.header_node_for_block_statement(*ancestor);
+                            if (loop_or_function_header != NULL)
+                            {
+                                switch (loop_or_function_header->type)
+                                {
+                                    case symbol_while_header:
+                                    case symbol_for_header:
+                                        // this is a loop header, so we can break or continue
+                                        found_loop = true;
+                                        end_search = true;
+                                        break;
+                                        
+                                    case symbol_function_header:
+                                        // this is a function header, so we cannot break or continue. We stop our search here.
+                                        found_loop = false;
+                                        end_search = true;
+                                        break;
+                                        
+                                    default:
+                                        // most likely begin / end style block, which makes no difference
+                                        break;
+                                }
+                            }
+                            ancestor = node_tree.get_parent(*ancestor);
+                        }
+                        
+                        if (! found_loop && ! first_argument_is_help(node_tree, node, buff_src))
+                        {
+                            errored = append_syntax_error(&parse_errors, node, INVALID_LOOP_ERR_MSG);
+                        }
+                    }
+                }
+            }
+        }
+    }
+
+    parser_test_error_bits_t res = 0;
+    
+    if (errored)
+        res |= PARSER_TEST_ERROR;
+
+    if (has_unclosed_block)
+        res |= PARSER_TEST_INCOMPLETE;
+    
+    if (out_errors)
+    {
+        out_errors->swap(parse_errors);
+    }
+
+    return res;
+
+}
diff --git a/parse_util.h b/parse_util.h
index b5b5262a..28e263ed 100644
--- a/parse_util.h
+++ b/parse_util.h
@@ -8,6 +8,7 @@
 #define FISH_PARSE_UTIL_H
 
 #include "autoload.h"
+#include "parse_tree.h"
 #include <wchar.h>
 #include <map>
 #include <set>
@@ -162,4 +163,6 @@ wcstring parse_util_escape_string_with_quote(const wcstring &cmd, wchar_t quote)
 /** Given a string, parse it as fish code and then return the indents. The return value has the same size as the string */
 std::vector<int> parse_util_compute_indents(const wcstring &src);
 
+parser_test_error_bits_t parse_util_detect_errors(const wcstring &buff_src, parse_error_list_t *out_errors);
+
 #endif
diff --git a/parser.cpp b/parser.cpp
index 136bc74a..1c6162a7 100644
--- a/parser.cpp
+++ b/parser.cpp
@@ -801,7 +801,7 @@ void parser_t::eval_args(const wchar_t *line, std::vector<completion_t> &args)
         proc_pop_interactive();
 }
 
-void parser_t::stack_trace(block_t *b, wcstring &buff)
+void parser_t::stack_trace(block_t *b, wcstring &buff) const
 {
     /*
       Check if we should end the recursion
@@ -844,7 +844,7 @@ void parser_t::stack_trace(block_t *b, wcstring &buff)
             {
                 const source_block_t *sb = static_cast<const source_block_t*>(b);
                 const wchar_t *source_dest = sb->source_file;
-                append_format(buff, _(L"in . (source) call of file '%ls',\n"), source_dest);
+                append_format(buff, _(L"from sourcing file '%ls',\n"), source_dest);
                 break;
             }
             case FUNCTION_CALL:
@@ -2762,7 +2762,7 @@ int parser_t::parser_test_argument(const wchar_t *arg, wcstring *out, const wcha
 //        debug( 1, L"%ls -> %ls %ls", arg_cpy, subst, tmp.buff );
 
                 parse_error_list_t errors;
-                err |= parser_t::detect_errors(subst, &errors);
+                err |= parse_util_detect_errors(subst, &errors);
                 if (out && ! errors.empty())
                 {
                     out->append(parse_errors_description(errors, subst, prefix));
@@ -2890,21 +2890,6 @@ int parser_t::test_args(const  wchar_t * buff, wcstring *out, const wchar_t *pre
     return err;
 }
 
-// Check if the first argument under the given node is --help
-static bool first_argument_is_help(const parse_node_tree_t &node_tree, const parse_node_t &node, const wcstring &src)
-{
-    bool is_help = false;
-    const parse_node_tree_t::parse_node_list_t arg_nodes = node_tree.find_nodes(node, symbol_argument, 1);
-    if (! arg_nodes.empty())
-    {
-        // Check the first argument only
-        const parse_node_t &arg = *arg_nodes.at(0);
-        const wcstring first_arg_src = arg.get_source(src);
-        is_help = parser_t::is_help(first_arg_src.c_str(), 3);
-    }
-    return is_help;
-}
-
 // helper type used in parser::test below
 struct block_info_t
 {
@@ -2929,153 +2914,31 @@ static bool append_syntax_error(parse_error_list_t *errors, const parse_node_t &
     return true;
 }
 
-parser_test_error_bits_t parser_t::detect_errors(const wcstring &buff_src, parse_error_list_t *out_errors, const wchar_t *prefix)
+void parser_t::get_backtrace(const wcstring &src, const parse_error_list_t &errors, wcstring *output) const
 {
-    ASSERT_IS_MAIN_THREAD();
-    
-    parse_node_tree_t node_tree;
-    parse_error_list_t parse_errors;
-    
-    // Whether we encountered a parse error
-    bool errored = false;
-    
-    // Whether we encountered an unclosed block
-    // We detect this via an 'end_command' block without source
-    bool has_unclosed_block = false;
-    
-    // Parse the input string into a parse tree
-    // Some errors are detected here
-    bool parsed = parse_t::parse(buff_src, 0, &node_tree, &parse_errors);
-    if (! parsed)
-    {
-        errored = true;
-    }
-    
-    // Expand all commands
-    // Verify 'or' and 'and' not used inside pipelines
-    // Verify pipes via parser_is_pipe_forbidden
-    // Verify return only within a function
-    
-    if (! errored)
+    assert(output != NULL);
+    if (! errors.empty())
     {
-        const size_t node_tree_size = node_tree.size();
-        for (size_t i=0; i < node_tree_size; i++)
+        const parse_error_t err = errors.at(0);
+        output->append(err.describe(src));
+        output->push_back(L'\n');
+        
+        // Determine which line we're on
+        assert(err.source_start <= src.size());
+        size_t which_line = 1 + std::count(src.begin(), src.begin() + err.source_start, L'\n');
+        
+        const wchar_t *filename = this->current_filename();
+        if (filename)
         {
-            const parse_node_t &node = node_tree.at(i);
-            if (node.type == symbol_end_command && ! node.has_source())
-            {
-                // an 'end' without source is an unclosed block
-                has_unclosed_block = true;
-            }
-            else if (node.type == symbol_plain_statement)
-            {
-                wcstring command;
-                if (node_tree.command_for_plain_statement(node, buff_src, &command))
-                {
-                    // Check that we can expand the command
-                    if (! expand_one(command, EXPAND_SKIP_CMDSUBST | EXPAND_SKIP_VARIABLES | EXPAND_SKIP_JOBS))
-                    {
-                        errored = append_syntax_error(&parse_errors, node, ILLEGAL_CMD_ERR_MSG, command.c_str());
-                    }
-                    
-                    // Check that pipes are sound
-                    bool is_boolean_command = contains(command, L"or", L"and");
-                    bool is_pipe_forbidden = parser_is_pipe_forbidden(command);
-                    if (! errored && (is_boolean_command || is_pipe_forbidden))
-                    {
-                        // 'or' and 'and' can be first in the pipeline. forbidden commands cannot be in a pipeline at all
-                        if (node_tree.plain_statement_is_in_pipeline(node, is_pipe_forbidden))
-                        {
-                            errored = append_syntax_error(&parse_errors, node, EXEC_ERR_MSG);
-                        }
-                    }
-                    
-                    // Check that we don't return from outside a function
-                    // But we allow it if it's 'return --help'
-                    if (! errored && command == L"return")
-                    {
-                        const parse_node_t *ancestor = &node;
-                        bool found_function = false;
-                        while (ancestor != NULL)
-                        {
-                            const parse_node_t *possible_function_header = node_tree.header_node_for_block_statement(*ancestor);
-                            if (possible_function_header != NULL && possible_function_header->type == symbol_function_header)
-                            {
-                                found_function = true;
-                                break;
-                            }
-                            ancestor = node_tree.get_parent(*ancestor);
-
-                        }
-                        if (! found_function && ! first_argument_is_help(node_tree, node, buff_src))
-                        {
-                            errored = append_syntax_error(&parse_errors, node, INVALID_RETURN_ERR_MSG);
-                        }
-                    }
-                    
-                    // Check that we don't return from outside a function
-                    if (! errored && (command == L"break" || command == L"continue"))
-                    {
-                        // Walk up until we hit a 'for' or 'while' loop. If we hit a function first, stop the search; we can't break an outer loop from inside a function.
-                        // This is a little funny because we can't tell if it's a 'for' or 'while' loop from the ancestor alone; we need the header. That is, we hit a block_statement, and have to check its header.
-                        bool found_loop = false, end_search = false;
-                        const parse_node_t *ancestor = &node;
-                        while (ancestor != NULL && ! end_search)
-                        {
-                            const parse_node_t *loop_or_function_header = node_tree.header_node_for_block_statement(*ancestor);
-                            if (loop_or_function_header != NULL)
-                            {
-                                switch (loop_or_function_header->type)
-                                {
-                                    case symbol_while_header:
-                                    case symbol_for_header:
-                                        // this is a loop header, so we can break or continue
-                                        found_loop = true;
-                                        end_search = true;
-                                        break;
-                                        
-                                    case symbol_function_header:
-                                        // this is a function header, so we cannot break or continue. We stop our search here.
-                                        found_loop = false;
-                                        end_search = true;
-                                        break;
-                                        
-                                    default:
-                                        // most likely begin / end style block, which makes no difference
-                                        break;
-                                }
-                            }
-                            ancestor = node_tree.get_parent(*ancestor);
-                        }
-                        
-                        if (! found_loop && ! first_argument_is_help(node_tree, node, buff_src))
-                        {
-                            errored = append_syntax_error(&parse_errors, node, INVALID_LOOP_ERR_MSG);
-                        }
-                    }
-                }
-            }
+            append_format(*output, _(L"line %lu of '%ls'\n"), which_line, filename);
         }
+        else
+        {
+            append_format(*output, L"%ls: ", _(L"Standard input"), which_line);
+        }
+        
+        this->stack_trace(current_block, *output);
     }
-
-    parser_test_error_bits_t res = 0;
-    
-    if (errored)
-        res |= PARSER_TEST_ERROR;
-
-    if (has_unclosed_block)
-        res |= PARSER_TEST_INCOMPLETE;
-    
-    if (out_errors)
-    {
-        out_errors->swap(parse_errors);
-    }
-    
-    error_code=0;
-
-
-    return res;
-
 }
 
 parser_test_error_bits_t parser_t::detect_errors2(const wchar_t *buff, wcstring *out, const wchar_t *prefix)
diff --git a/parser.h b/parser.h
index fb3efad8..90b6a1c4 100644
--- a/parser.h
+++ b/parser.h
@@ -14,12 +14,6 @@
 #include "parse_tree.h"
 #include <vector>
 
-enum {
-  PARSER_TEST_ERROR = 1,
-  PARSER_TEST_INCOMPLETE = 2
-};
-typedef unsigned int parser_test_error_bits_t;
-
 /**
    event_blockage_t represents a block on events of the specified type
 */
@@ -488,8 +482,8 @@ public:
        \param out if non-null, any errors in the command will be filled out into this buffer
        \param prefix the prefix string to prepend to each error message written to the \c out buffer
     */
-    parser_test_error_bits_t detect_errors(const wcstring &buff, parse_error_list_t *out_errors = NULL, const wchar_t *prefix = NULL);
-    parser_test_error_bits_t detect_errors2(const wchar_t * buff, wcstring *out = NULL, const wchar_t *prefix = NULL);
+    parser_test_error_bits_t detect_errors2(const wchar_t *buff, wcstring *out_error_desc, const wchar_t *prefix);
+    void get_backtrace(const wcstring &src, const parse_error_list_t &errors, wcstring *output) const;
 
     /**
        Test if the specified string can be parsed as an argument list,
@@ -538,7 +532,7 @@ public:
     /**
        Write a stack trace starting at the specified block to the specified wcstring
     */
-    void stack_trace(block_t *b, wcstring &buff);
+    void stack_trace(block_t *b, wcstring &buff) const;
 
     int get_block_type(const wchar_t *cmd) const;
     const wchar_t *get_block_command(int type) const;
diff --git a/reader.cpp b/reader.cpp
index 3eeae627..87c01464 100644
--- a/reader.cpp
+++ b/reader.cpp
@@ -2473,7 +2473,8 @@ void reader_run_command(parser_t &parser, const wcstring &cmd)
 int reader_shell_test(const wchar_t *b)
 {
     wcstring bstr = b;
-    int res = parser_t::principal_parser().detect_errors(bstr);
+    parse_error_list_t errors;
+    int res = parse_util_detect_errors(bstr, &errors);
 
     if (res & PARSER_TEST_ERROR)
     {
@@ -2490,14 +2491,9 @@ int reader_shell_test(const wchar_t *b)
                 tmp2,
                 0);
         
-        parse_error_list_t errors;
-        parser_t::principal_parser().detect_errors(bstr, &errors, L"fish");
-        
-        if (! errors.empty())
-        {
-            const wcstring sb = parse_errors_description(errors, b, L"fish");
-            fwprintf(stderr, L"%ls", sb.c_str());
-        }
+        wcstring sb;
+        parser_t::principal_parser().get_backtrace(bstr, errors, &sb);
+        fwprintf(stderr, L"%ls", sb.c_str());
     }
     return res;
 }
@@ -3908,13 +3904,14 @@ static int read_ni(int fd, const io_chain_t &io)
         }
 
         parse_error_list_t errors;
-        if (! parser.detect_errors(str, &errors, L"fish"))
+        if (! parse_util_detect_errors(str, &errors))
         {
             parser.eval(str, io, TOP);
         }
         else
         {
-            const wcstring sb = parse_errors_description(errors, str);
+            wcstring sb;
+            parser.get_backtrace(str, errors, &sb);
             fwprintf(stderr, L"%ls", sb.c_str());
             res = 1;
         }