diff --git a/builtin_complete.cpp b/builtin_complete.cpp index fd899465a..ecfcceaf7 100644 --- a/builtin_complete.cpp +++ b/builtin_complete.cpp @@ -497,14 +497,14 @@ static int builtin_complete(parser_t &parser, wchar_t **argv) { if (condition && wcslen(condition)) { - if (parser.test(condition, 0, 0, 0)) + if (parser.test(condition)) { append_format(stderr_buffer, L"%ls: Condition '%ls' contained a syntax error\n", argv[0], condition); - parser.test(condition, 0, &stderr_buffer, argv[0]); + parser.test(condition, NULL, &stderr_buffer, argv[0]); res = true; } diff --git a/fish_tests.cpp b/fish_tests.cpp index 7f2543389..3ec8334e4 100644 --- a/fish_tests.cpp +++ b/fish_tests.cpp @@ -463,52 +463,52 @@ static void test_parser() parser_t parser(PARSER_TYPE_GENERAL, true); say(L"Testing null input to parser"); - if (!parser.test(0, 0, 0, 0)) + if (!parser.test(NULL)) { err(L"Null input to parser.test undetected"); } say(L"Testing block nesting"); - if (!parser.test(L"if; end", 0, 0, 0)) + if (!parser.test(L"if; end")) { err(L"Incomplete if statement undetected"); } - if (!parser.test(L"if test; echo", 0, 0, 0)) + if (!parser.test(L"if test; echo")) { err(L"Missing end undetected"); } - if (!parser.test(L"if test; end; end", 0, 0, 0)) + if (!parser.test(L"if test; end; end")) { err(L"Unbalanced end undetected"); } say(L"Testing detection of invalid use of builtin commands"); - if (!parser.test(L"case foo", 0, 0, 0)) + if (!parser.test(L"case foo")) { err(L"'case' command outside of block context undetected"); } - if (!parser.test(L"switch ggg; if true; case foo;end;end", 0, 0, 0)) + if (!parser.test(L"switch ggg; if true; case foo;end;end")) { err(L"'case' command outside of switch block context undetected"); } - if (!parser.test(L"else", 0, 0, 0)) + if (!parser.test(L"else")) { err(L"'else' command outside of conditional block context undetected"); } - if (!parser.test(L"else if", 0, 0, 0)) + if (!parser.test(L"else if")) { err(L"'else if' command outside of conditional block context undetected"); } - if (!parser.test(L"if false; else if; end", 0, 0, 0)) + if (!parser.test(L"if false; else if; end")) { err(L"'else if' missing command undetected"); } - if (!parser.test(L"break", 0, 0, 0)) + if (!parser.test(L"break")) { err(L"'break' command outside of loop block context undetected"); } - if (!parser.test(L"exec ls|less", 0, 0, 0) || !parser.test(L"echo|return", 0, 0, 0)) + if (!parser.test(L"exec ls|less") || !parser.test(L"echo|return")) { err(L"Invalid pipe command undetected"); } diff --git a/parser.cpp b/parser.cpp index 11b3745f9..ca524b8db 100644 --- a/parser.cpp +++ b/parser.cpp @@ -45,13 +45,6 @@ The fish parser. Contains functions for parsing and evaluating code. #include "signal.h" #include "complete.h" -/** - Maximum number of block levels in code. This is not the same as - maximum recursion depth, this only has to do with how many block - levels are legal in the source code, not at evaluation. -*/ -#define BLOCK_MAX_COUNT 64 - /** Maximum number of function calls, i.e. recursion depth. */ @@ -304,65 +297,21 @@ struct block_lookup_entry */ static const struct block_lookup_entry block_lookup[]= { - { - WHILE, L"while", WHILE_BLOCK - } - , - { - FOR, L"for", FOR_BLOCK - } - , - { - IF, L"if", IF_BLOCK - } - , - { - FUNCTION_DEF, L"function", FUNCTION_DEF_BLOCK - } - , - { - FUNCTION_CALL, 0, FUNCTION_CALL_BLOCK - } - , - { - FUNCTION_CALL_NO_SHADOW, 0, FUNCTION_CALL_NO_SHADOW_BLOCK - } - , - { - SWITCH, L"switch", SWITCH_BLOCK - } - , - { - FAKE, 0, FAKE_BLOCK - } - , - { - TOP, 0, TOP_BLOCK - } - , - { - SUBST, 0, SUBST_BLOCK - } - , - { - BEGIN, L"begin", BEGIN_BLOCK - } - , - { - SOURCE, L".", SOURCE_BLOCK - } - , - { - EVENT, 0, EVENT_BLOCK - } - , - { - BREAKPOINT, L"breakpoint", BREAKPOINT_BLOCK - } - , - { - (block_type_t)0, 0, 0 - } + { WHILE, L"while", WHILE_BLOCK }, + { FOR, L"for", FOR_BLOCK }, + { IF, L"if", IF_BLOCK }, + { FUNCTION_DEF, L"function", FUNCTION_DEF_BLOCK }, + { FUNCTION_CALL, 0, FUNCTION_CALL_BLOCK }, + { FUNCTION_CALL_NO_SHADOW, 0, FUNCTION_CALL_NO_SHADOW_BLOCK }, + { SWITCH, L"switch", SWITCH_BLOCK }, + { FAKE, 0, FAKE_BLOCK }, + { TOP, 0, TOP_BLOCK }, + { SUBST, 0, SUBST_BLOCK }, + { BEGIN, L"begin", BEGIN_BLOCK }, + { SOURCE, L".", SOURCE_BLOCK }, + { EVENT, 0, EVENT_BLOCK }, + { BREAKPOINT, L"breakpoint", BREAKPOINT_BLOCK }, + { (block_type_t)0, 0, 0 } }; static bool job_should_skip_elseif(const job_t *job, const block_t *current_block); @@ -413,19 +362,6 @@ void parser_t::skip_all_blocks(void) } } -/** - Return the current number of block nestings -*/ -/* -static int block_count( block_t *b ) -{ - - if( b==0) - return 0; - return( block_count(b->outer)+1); -} -*/ - void parser_t::push_block(block_t *newv) { const enum block_type_t type = newv->type(); @@ -495,9 +431,7 @@ void parser_t::pop_block() const wchar_t *parser_t::get_block_desc(int block) const { - int i; - - for (i=0; block_lookup[i].desc; i++) + for (size_t i=0; block_lookup[i].desc; i++) { if (block_lookup[i].type == block) { @@ -2747,9 +2681,7 @@ int parser_t::eval(const wcstring &cmdStr, const io_chain_t &io, enum block_type */ block_type_t parser_get_block_type(const wcstring &cmd) { - int i; - - for (i=0; block_lookup[i].desc; i++) + for (size_t i=0; block_lookup[i].desc; i++) { if (block_lookup[i].name && cmd == block_lookup[i].name) { @@ -2764,16 +2696,14 @@ block_type_t parser_get_block_type(const wcstring &cmd) */ const wchar_t *parser_get_block_command(int type) { - int i; - - for (i=0; block_lookup[i].desc; i++) + for (size_t i=0; block_lookup[i].desc; i++) { if (block_lookup[i].type == type) { return block_lookup[i].name; } } - return 0; + return NULL; } /** @@ -2966,10 +2896,16 @@ int parser_t::test_args(const wchar_t * buff, wcstring *out, const wchar_t *pre return err; } -int parser_t::test(const wchar_t * buff, - int *block_level, - wcstring *out, - const wchar_t *prefix) +// helper type used in parser::test below +struct block_info_t { + int position; //tokenizer position + block_type_t type; //type of the block + int indentation; //indentation associated with the block + + bool has_had_case; //if we are a switch, whether we've encountered a case +}; + +int parser_t::test(const wchar_t *buff, int *block_level, wcstring *out, const wchar_t *prefix) { ASSERT_IS_MAIN_THREAD(); @@ -2983,10 +2919,10 @@ int parser_t::test(const wchar_t * buff, tokenizer_t * const previous_tokenizer=current_tokenizer; const int previous_pos=current_tokenizer_pos; - - int block_pos[BLOCK_MAX_COUNT] = {}; - block_type_t block_type[BLOCK_MAX_COUNT] = {}; - int count = 0; + + // These are very nearly stacks, but sometimes we have to inspect non-top elements (e.g. return) + std::vector block_infos; + int indentation_sum = 0; //sum of indentation in block_infos int res = 0; /* @@ -3100,8 +3036,29 @@ int parser_t::test(const wchar_t * buff, if (command == L"end") { tok_next(&tok); - count--; tok_set_pos(&tok, mark); + + /* Test that end is not used when not inside any block */ + if (block_infos.empty()) + { + err = 1; + if (out) + { + error(SYNTAX_ERROR, + tok_get_pos(&tok), + INVALID_END_ERR_MSG); + print_errors(*out, prefix); + const wcstring h = builtin_help_get(*this, L"end"); + if (! h.empty()) + append_format(*out, L"%ls", h.c_str()); + } + } + else + { + indentation_sum -= block_infos.back().indentation; + block_infos.pop_back(); + + } } /* @@ -3109,10 +3066,32 @@ int parser_t::test(const wchar_t * buff, _after_ checking for end commands, but _before_ checking for block opening commands. */ - bool is_else_or_elseif = (command == L"else"); - if (block_level) + if (block_level != NULL) { - block_level[tok_get_pos(&tok)] = count + (is_else_or_elseif?-1:0); + int indentation_adjust = 0; + if (command == L"else") + { + // if or else if goes back + indentation_adjust = -1; + } + else if (command == L"case") + { + if (! block_infos.empty() && block_infos.back().type == SWITCH) + { + // mark that we've encountered a case, and increase the indentation + // by doing this now, we avoid overly indenting the first case as the user types it + if (! block_infos.back().has_had_case) + { + block_infos.back().has_had_case = true; + block_infos.back().indentation += 1; + indentation_sum += 1; + } + // unindent this case + indentation_adjust = -1; + } + } + + block_level[tok_get_pos(&tok)] = indentation_sum + indentation_adjust; } /* @@ -3120,25 +3099,11 @@ int parser_t::test(const wchar_t * buff, */ if (parser_keywords_is_block(command)) { - if (count >= BLOCK_MAX_COUNT) - { - if (out) - { - error(SYNTAX_ERROR, - tok_get_pos(&tok), - BLOCK_ERR_MSG); - - print_errors(*out, prefix); - } - } - else - { - block_type[count] = parser_get_block_type(command); - block_pos[count] = current_tokenizer_pos; - tok_next(&tok); - count++; - tok_set_pos(&tok, mark); - } + struct block_info_t info = {current_tokenizer_pos, parser_get_block_type(command), 1 /* indent */}; + block_infos.push_back(info); + indentation_sum += info.indentation; + tok_next(&tok); + tok_set_pos(&tok, mark); } /* @@ -3203,7 +3168,7 @@ int parser_t::test(const wchar_t * buff, */ if (command == L"case") { - if (!count || block_type[count-1]!=SWITCH) + if (block_infos.empty() || block_infos.back().type != SWITCH) { err=1; @@ -3226,13 +3191,13 @@ int parser_t::test(const wchar_t * buff, */ if (command == L"return") { - int found_func=0; - int i; - for (i=count-1; i>=0; i--) + bool found_func = false; + size_t block_idx = block_infos.size(); + while (block_idx--) { - if (block_type[i]==FUNCTION_DEF) + if (block_infos.at(block_idx).type == FUNCTION_DEF) { - found_func=1; + found_func = true; break; } } @@ -3281,14 +3246,14 @@ int parser_t::test(const wchar_t * buff, */ if (contains(command, L"break", L"continue")) { - int found_loop=0; - int i; - for (i=count-1; i>=0; i--) + bool found_loop = false; + size_t block_idx = block_infos.size(); + while (block_idx--) { - if ((block_type[i]==WHILE) || - (block_type[i]==FOR)) + block_type_t type = block_infos.at(block_idx).type; + if (type == WHILE || type == FOR) { - found_loop=1; + found_loop = true; break; } } @@ -3336,7 +3301,7 @@ int parser_t::test(const wchar_t * buff, */ if (command == L"else") { - if (!count || block_type[count-1]!=IF) + if (block_infos.empty() || block_infos.back().type != IF) { err=1; if (out) @@ -3350,25 +3315,6 @@ int parser_t::test(const wchar_t * buff, } } } - - /* - Test that end is not used when not inside any block - */ - if (count < 0) - { - err = 1; - if (out) - { - error(SYNTAX_ERROR, - tok_get_pos(&tok), - INVALID_END_ERR_MSG); - print_errors(*out, prefix); - const wcstring h = builtin_help_get(*this, L"end"); - if (h.size()) - append_format(*out, L"%ls", h.c_str()); - } - } - } else { @@ -3677,17 +3623,17 @@ int parser_t::test(const wchar_t * buff, } - if (out && count>0) + if (out != NULL && ! block_infos.empty()) { const wchar_t *cmd; + int bad_pos = block_infos.back().position; + block_type_t bad_type = block_infos.back().type; - error(SYNTAX_ERROR, - block_pos[count-1], - BLOCK_END_ERR_MSG); + error(SYNTAX_ERROR, bad_pos, BLOCK_END_ERR_MSG); print_errors(*out, prefix); - cmd = parser_get_block_command(block_type[count -1]); + cmd = parser_get_block_command(bad_type); if (cmd) { const wcstring h = builtin_help_get(*this, cmd); @@ -3736,19 +3682,20 @@ int parser_t::test(const wchar_t * buff, validator had at exit. This makes sure a new line is correctly indented even if it is empty. */ + int last_indent = block_infos.empty() ? 0 : block_infos.back().indentation; size_t suffix_idx = len; while (suffix_idx--) { if (!wcschr(L" \n\t\r", buff[suffix_idx])) break; - block_level[suffix_idx] = count; + block_level[suffix_idx] = last_indent; } } /* Calculate exit status */ - if (count!= 0) + if (! block_infos.empty()) unfinished = 1; if (err) diff --git a/parser.h b/parser.h index 811c15270..7677754b3 100644 --- a/parser.h +++ b/parser.h @@ -484,7 +484,7 @@ 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 */ - int test(const wchar_t * buff, int *block_level, wcstring *out, const wchar_t *prefix); + int test(const wchar_t * buff, int *block_level = NULL, wcstring *out = NULL, const wchar_t *prefix = NULL); /** Test if the specified string can be parsed as an argument list, diff --git a/reader.cpp b/reader.cpp index f90df311f..011fc008a 100644 --- a/reader.cpp +++ b/reader.cpp @@ -505,7 +505,7 @@ wcstring combine_command_and_autosuggestion(const wcstring &cmdline, const wcstr static void reader_repaint() { // Update the indentation - parser_t::principal_parser().test(data->command_line.c_str(), &data->indents[0], 0, 0); + parser_t::principal_parser().test(data->command_line.c_str(), &data->indents[0]); // Combine the command and autosuggestion into one string wcstring full_line = combine_command_and_autosuggestion(data->command_line, data->autosuggestion); @@ -2263,7 +2263,7 @@ void reader_run_command(parser_t &parser, const wchar_t *cmd) int reader_shell_test(const wchar_t *b) { - int res = parser_t::principal_parser().test(b, 0, 0, 0); + int res = parser_t::principal_parser().test(b); if (res & PARSER_TEST_ERROR) { @@ -2283,7 +2283,7 @@ int reader_shell_test(const wchar_t *b) 0); - parser_t::principal_parser().test(b, 0, &sb, L"fish"); + parser_t::principal_parser().test(b, NULL, &sb, L"fish"); fwprintf(stderr, L"%ls", sb.c_str()); } return res;