Mirrors/fish-shell
2
0
Fork 0
mirror of https://github.com/fish-shell/fish-shell synced 2024-12-26 04:43:10 +00:00
Code Issues Projects Releases Packages Wiki Activity

Disallow backgrounding in conditionals and before and/or bool statements

Browse source 
Fixes #1136
This commit is contained in:
ridiculousfish 2014-11-02 13:11:27 -08:00
parent c33a3862cc
commit c31ad3ed07
6 changed files with 144 additions and 21 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

30
fish_tests.cpp
View file

@ -691,7 +691,37 @@ static void test_parser()
{
err(L"Bad escape in nested command substitution not reported as error");
}
if (! parse_util_detect_errors(L"false & ; and cat"))
{
err(L"'and' command after background not reported as error");
}
if (! parse_util_detect_errors(L"true & ; or cat"))
{
err(L"'or' command after background not reported as error");
}
if (parse_util_detect_errors(L"true & ; not cat"))
{
err(L"'not' command after background falsely reported as error");
}
if (! parse_util_detect_errors(L"if true & ; end"))
{
err(L"backgrounded 'if' conditional not reported as error");
}
if (! parse_util_detect_errors(L"if false; else if true & ; end"))
{
err(L"backgrounded 'else if' conditional not reported as error");
}
if (! parse_util_detect_errors(L"while true & ; end"))
{
err(L"backgrounded 'while' conditional not reported as error");
}
say(L"Testing basic evaluation");
#if 0

8
parse_constants.h
View file

@ -109,6 +109,14 @@ enum parse_statement_decoration_t
parse_statement_decoration_exec
};
/* Boolean statement types */
enum parse_bool_statement_type_t
{
parse_bool_and,
parse_bool_or,
parse_bool_not
};
/* Parse error code list */
enum parse_error_code_t
{

16
parse_execution.cpp
View file

@ -1128,30 +1128,22 @@ parse_execution_result_t parse_execution_context_t::populate_boolean_process(job
// Handle a boolean statement
bool skip_job = false;
assert(bool_statement.type == symbol_boolean_statement);
switch (bool_statement.production_idx)
switch (parse_node_tree_t::statement_boolean_type(bool_statement))
{
// These magic numbers correspond to productions for boolean_statement
case 0:
case parse_bool_and:
// AND. Skip if the last job failed.
skip_job = (proc_get_last_status() != 0);
break;
case 1:
case parse_bool_or:
// OR. Skip if the last job succeeded.
skip_job = (proc_get_last_status() == 0);
break;
case 2:
case parse_bool_not:
// NOT. Negate it.
job_set_flag(job, JOB_NEGATE, !job_get_flag(job, JOB_NEGATE));
break;
default:
{
fprintf(stderr, "Unexpected production in boolean statement\n");
PARSER_DIE();
break;
}
}
if (skip_job)

27
parse_tree.cpp
View file

@ -1649,6 +1649,30 @@ parse_node_tree_t::parse_node_list_t parse_node_tree_t::specific_statements_for_
return result;
}
enum parse_bool_statement_type_t parse_node_tree_t::statement_boolean_type(const parse_node_t &node)
{
assert(node.type == symbol_boolean_statement);
switch (node.production_idx)
{
// These magic numbers correspond to productions for boolean_statement
case 0:
return parse_bool_and;
case 1:
return parse_bool_or;
case 2:
return parse_bool_not;
default:
{
fprintf(stderr, "Unexpected production in boolean statement\n");
PARSER_DIE();
return (enum parse_bool_statement_type_t)(-1);
}
}
}
bool parse_node_tree_t::job_should_be_backgrounded(const parse_node_t &job) const
{
assert(job.type == symbol_job);
@ -1657,7 +1681,8 @@ bool parse_node_tree_t::job_should_be_backgrounded(const parse_node_t &job) cons
const parse_node_t *opt_background = get_child(job, 2, symbol_optional_background);
if (opt_background != NULL)
{
assert(opt_background->production_idx <= 1);
// We may get the value -1 if the node is not yet materialized (i.e. an incomplete parse tree)
assert(opt_background->production_idx == uint8_t(-1) || opt_background->production_idx <= 1);
result = (opt_background->production_idx == 1);
}
return result;

4
parse_tree.h
View file

@ -184,11 +184,13 @@ public:
/* Given a job, return all of its statements. These are 'specific statements' (e.g. symbol_decorated_statement, not symbol_statement) */
parse_node_list_t specific_statements_for_job(const parse_node_t &job) const;
/* Returns the boolean type for a boolean node */
static enum parse_bool_statement_type_t statement_boolean_type(const parse_node_t &node);
/* Given a job, return whether it should be backgrounded, because it has a & specifier */
bool job_should_be_backgrounded(const parse_node_t &job) const;
};
/* The big entry point. Parse a string, attempting to produce a tree for the given goal type */
bool parse_tree_from_string(const wcstring &str, parse_tree_flags_t flags, parse_node_tree_t *output, parse_error_list_t *errors, parse_token_type_t goal = symbol_job_list);

80
parse_util.cpp
View file

@ -42,11 +42,16 @@
#include "parser.h"
#include "builtin.h"
/**
Error message for improper use of the exec builtin
*/
/** Error message for improper use of the exec builtin */
#define EXEC_ERR_MSG _(L"The '%ls' command can not be used in a pipeline")
/** Error message for use of backgrounded commands before and/or */
#define BOOL_AFTER_BACKGROUND_ERROR_MSG _(L"The '%ls' command can not be used immediately after a backgrounded job")
/** Error message for backgrounded commands as conditionals */
#define BACKGROUND_IN_CONDITIONAL_ERROR_MSG _(L"Backgrounded commands can not be used as conditionals")
int parse_util_lineno(const wchar_t *str, size_t offset)
{
if (! str)
@ -1315,11 +1320,10 @@ parser_test_error_bits_t parse_util_detect_errors(const wcstring &buff_src, pars
else if (node.type == symbol_boolean_statement)
{
// 'or' and 'and' can be in a pipeline, as long as they're first
// These numbers 0 and 1 correspond to productions for boolean_statement. This should be cleaned up.
bool is_and = (node.production_idx == 0), is_or = (node.production_idx == 1);
if ((is_and || is_or) && node_tree.statement_is_in_pipeline(node, false /* don't count first */))
parse_bool_statement_type_t type = parse_node_tree_t::statement_boolean_type(node);
if ((type == parse_bool_and || type == parse_bool_or) && node_tree.statement_is_in_pipeline(node, false /* don't count first */))
{
errored = append_syntax_error(&parse_errors, node, EXEC_ERR_MSG, is_and ? L"and" : L"or");
errored = append_syntax_error(&parse_errors, node, EXEC_ERR_MSG, (type == parse_bool_and) ? L"and" : L"or");
}
}
else if (node.type == symbol_argument)
@ -1327,6 +1331,68 @@ parser_test_error_bits_t parse_util_detect_errors(const wcstring &buff_src, pars
const wcstring arg_src = node.get_source(buff_src);
res |= parse_util_detect_errors_in_argument(node, arg_src, &parse_errors);
}
else if (node.type == symbol_job)
{
if (node_tree.job_should_be_backgrounded(node))
{
/* Disallow background in the following cases:
foo & ; and bar
foo & ; or bar
if foo & ; end
while foo & ; end
*/
const parse_node_t *job_parent = node_tree.get_parent(node);
assert(job_parent != NULL);
switch (job_parent->type)
{
case symbol_if_clause:
case symbol_while_header:
{
assert(node_tree.get_child(*job_parent, 1) == &node);
errored = append_syntax_error(&parse_errors, node, BACKGROUND_IN_CONDITIONAL_ERROR_MSG);
break;
}
case symbol_job_list:
{
// This isn't very complete, e.g. we don't catch 'foo & ; not and bar'
assert(node_tree.get_child(*job_parent, 0) == &node);
const parse_node_t *next_job_list = node_tree.get_child(*job_parent, 1, symbol_job_list);
assert(next_job_list != NULL);
const parse_node_t *next_job = node_tree.next_node_in_node_list(*next_job_list, symbol_job, NULL);
if (next_job != NULL)
{
const parse_node_t *next_statement = node_tree.get_child(*next_job, 0, symbol_statement);
if (next_statement != NULL)
{
const parse_node_t *spec_statement = node_tree.get_child(*next_statement, 0);
if (spec_statement && spec_statement->type == symbol_boolean_statement)
{
switch (parse_node_tree_t::statement_boolean_type(*spec_statement))
{
// These are not allowed
case parse_bool_and:
errored = append_syntax_error(&parse_errors, *spec_statement, BOOL_AFTER_BACKGROUND_ERROR_MSG, L"and");
break;
case parse_bool_or:
errored = append_syntax_error(&parse_errors, *spec_statement, BOOL_AFTER_BACKGROUND_ERROR_MSG, L"or");
break;
case parse_bool_not:
// This one is OK
break;
}
}
}
}
break;
}
default:
break;
}
}
}
else if (node.type == symbol_plain_statement)
{
// In a few places below, we want to know if we are in a pipeline