diff options
| -rw-r--r-- | expand.cpp | 119 | ||||
| -rw-r--r-- | expand.h | 5 | ||||
| -rw-r--r-- | highlight.cpp | 32 | ||||
| -rw-r--r-- | tests/expansion.err | 30 | ||||
| -rw-r--r-- | tests/expansion.in | 82 | ||||
| -rw-r--r-- | tests/expansion.out | 42 | ||||
| -rw-r--r-- | tests/expansion.status | 1 | ||||
| -rw-r--r-- | tests/top.out | 1 |
8 files changed, 283 insertions, 29 deletions
@@ -953,8 +953,11 @@ void expand_variable_error(parser_t &parser, const wcstring &token, size_t token /** Parse an array slicing specification + Returns 0 on success. + If a parse error occurs, returns the index of the bad token. + Note that 0 can never be a bad index because the string always starts with [. */ -static int parse_slice(const wchar_t *in, wchar_t **end_ptr, std::vector<long> &idx, std::vector<size_t> &source_positions, size_t array_size) +static size_t parse_slice(const wchar_t *in, wchar_t **end_ptr, std::vector<long> &idx, std::vector<size_t> &source_positions, size_t array_size) { wchar_t *end; @@ -981,7 +984,7 @@ static int parse_slice(const wchar_t *in, wchar_t **end_ptr, std::vector<long> & tmp = wcstol(&in[pos], &end, 10); if ((errno) || (end == &in[pos])) { - return 1; + return pos; } // debug( 0, L"Push idx %d", tmp ); @@ -999,7 +1002,7 @@ static int parse_slice(const wchar_t *in, wchar_t **end_ptr, std::vector<long> & long tmp1 = wcstol(&in[pos], &end, 10); if ((errno) || (end == &in[pos])) { - return 1; + return pos; } pos = end-in; @@ -1086,10 +1089,15 @@ static int expand_variables_internal(parser_t &parser, wchar_t * const in, std:: while (1) { - if (!(in[stop_pos ])) + const wchar_t nc = in[stop_pos]; + if (!(nc)) break; - if (!(iswalnum(in[stop_pos]) || - (wcschr(L"_", in[stop_pos])!= 0))) + if (nc == VARIABLE_EXPAND_EMPTY) + { + stop_pos++; + break; + } + if (!(wcsvarchr(nc))) break; stop_pos++; @@ -1108,7 +1116,15 @@ static int expand_variables_internal(parser_t &parser, wchar_t * const in, std:: } var_tmp.append(in + start_pos, var_len); - env_var_t var_val = expand_var(var_tmp.c_str()); + env_var_t var_val; + if (var_len == 1 && var_tmp[0] == VARIABLE_EXPAND_EMPTY) + { + var_val = env_var_t::missing_var(); + } + else + { + var_val = expand_var(var_tmp.c_str()); + } if (! var_val.missing()) { @@ -1123,12 +1139,14 @@ static int expand_variables_internal(parser_t &parser, wchar_t * const in, std:: if (in[slice_start] == L'[') { wchar_t *slice_end; + size_t bad_pos; all_vars=0; - if (parse_slice(in + slice_start, &slice_end, var_idx_list, var_pos_list, var_item_list.size())) + bad_pos = parse_slice(in + slice_start, &slice_end, var_idx_list, var_pos_list, var_item_list.size()); + if (bad_pos != 0) { append_syntax_error(errors, - stop_pos, + stop_pos + bad_pos, L"Invalid index value"); is_ok = 0; break; @@ -1142,10 +1160,10 @@ static int expand_variables_internal(parser_t &parser, wchar_t * const in, std:: for (size_t j=0; j<var_idx_list.size(); j++) { long tmp = var_idx_list.at(j); - size_t var_src_pos = var_pos_list.at(j); /* Check that we are within array bounds. If not, truncate the list to exit. */ if (tmp < 1 || (size_t)tmp > var_item_list.size()) { + size_t var_src_pos = var_pos_list.at(j); /* The slice was parsed starting at stop_pos, so we have to add that to the error position */ append_syntax_error(errors, slice_start + var_src_pos, @@ -1174,7 +1192,18 @@ static int expand_variables_internal(parser_t &parser, wchar_t * const in, std:: { in[i]=0; wcstring res = in; - res.push_back(INTERNAL_SEPARATOR); + if (i > 0) + { + if (in[i-1] != VARIABLE_EXPAND_SINGLE) + { + res.push_back(INTERNAL_SEPARATOR); + } + else if (var_item_list.empty() || var_item_list.front().empty()) + { + // first expansion is empty, but we need to recursively expand + res.push_back(VARIABLE_EXPAND_EMPTY); + } + } for (size_t j=0; j<var_item_list.size(); j++) { @@ -1204,14 +1233,18 @@ static int expand_variables_internal(parser_t &parser, wchar_t * const in, std:: if (is_ok) { wcstring new_in; + new_in.append(in, i); - if (start_pos > 0) - new_in.append(in, start_pos - 1); - - // at this point new_in.size() is start_pos - 1 - if (start_pos>1 && new_in[start_pos-2]!=VARIABLE_EXPAND) + if (i > 0) { - new_in.push_back(INTERNAL_SEPARATOR); + if (in[i-1] != VARIABLE_EXPAND) + { + new_in.push_back(INTERNAL_SEPARATOR); + } + else if (next.empty()) + { + new_in.push_back(VARIABLE_EXPAND_EMPTY); + } } new_in.append(next); new_in.append(in + stop_pos); @@ -1227,6 +1260,41 @@ static int expand_variables_internal(parser_t &parser, wchar_t * const in, std:: } else { + // even with no value, we still need to parse out slice syntax + // Behave as though we had 1 value, so $foo[1] always works. + const size_t slice_start = stop_pos; + if (in[slice_start] == L'[') + { + wchar_t *slice_end; + size_t bad_pos; + + bad_pos = parse_slice(in + slice_start, &slice_end, var_idx_list, var_pos_list, 1); + if (bad_pos != 0) + { + append_syntax_error(errors, + stop_pos + bad_pos, + L"Invalid index value"); + is_ok = 0; + return is_ok; + } + stop_pos = (slice_end-in); + + // validate that the parsed indexes are valid + for (size_t j=0; j<var_idx_list.size(); j++) + { + long tmp = var_idx_list.at(j); + if (tmp != 1) + { + size_t var_src_pos = var_pos_list.at(j); + append_syntax_error(errors, + slice_start + var_src_pos, + ARRAY_BOUNDS_ERR); + is_ok = 0; + return is_ok; + } + } + } + /* Expand a non-existing variable */ @@ -1243,8 +1311,11 @@ static int expand_variables_internal(parser_t &parser, wchar_t * const in, std:: Expansion to single argument. */ wcstring res; - in[i] = 0; - res.append(in); + res.append(in, i); + if (i > 0 && in[i-1] == VARIABLE_EXPAND_SINGLE) + { + res.push_back(VARIABLE_EXPAND_EMPTY); + } res.append(in + stop_pos); is_ok &= expand_variables2(parser, res, out, i, errors); @@ -1435,11 +1506,14 @@ static int expand_cmdsubst(parser_t &parser, const wcstring &input, std::vector< { std::vector<long> slice_idx; std::vector<size_t> slice_source_positions; + const wchar_t * const slice_begin = tail_begin; wchar_t *slice_end; + size_t bad_pos; - if (parse_slice(tail_begin, &slice_end, slice_idx, slice_source_positions, sub_res.size())) + bad_pos = parse_slice(slice_begin, &slice_end, slice_idx, slice_source_positions, sub_res.size()); + if (bad_pos != 0) { - append_syntax_error(errors, SOURCE_LOCATION_UNKNOWN, L"Invalid index value"); + append_syntax_error(errors, slice_begin - in + bad_pos, L"Invalid index value"); return 0; } else @@ -1451,8 +1525,9 @@ static int expand_cmdsubst(parser_t &parser, const wcstring &input, std::vector< long idx = slice_idx.at(i); if (idx < 1 || (size_t)idx > sub_res.size()) { + size_t pos = slice_source_positions.at(i); append_syntax_error(errors, - SOURCE_LOCATION_UNKNOWN, + slice_begin - in + pos, ARRAY_BOUNDS_ERR); return 0; } @@ -102,6 +102,11 @@ enum */ INTERNAL_SEPARATOR, + /** + Character representing an empty variable expansion. + Only used transitively while expanding variables. + */ + VARIABLE_EXPAND_EMPTY, } ; diff --git a/highlight.cpp b/highlight.cpp index 287f1a3b..3fdb6c8f 100644 --- a/highlight.cpp +++ b/highlight.cpp @@ -617,12 +617,26 @@ static size_t color_variable(const wchar_t *in, size_t in_len, std::vector<highl if (in[idx] == L'[') { wchar_t *slice_begin = NULL, *slice_end = NULL; - if (1 == parse_util_locate_slice(in, &slice_begin, &slice_end, false)) + switch (parse_util_locate_slice(in, &slice_begin, &slice_end, false)) { - size_t slice_begin_idx = slice_begin - in, slice_end_idx = slice_end - in; - assert(slice_end_idx > slice_begin_idx); - colors[slice_begin_idx] = highlight_spec_operator; - colors[slice_end_idx] = highlight_spec_operator; + case 1: + { + size_t slice_begin_idx = slice_begin - in, slice_end_idx = slice_end - in; + assert(slice_end_idx > slice_begin_idx); + colors[slice_begin_idx] = highlight_spec_operator; + colors[slice_end_idx] = highlight_spec_operator; + break; + } + case -1: + { + // syntax error + // Normally the entire token is colored red for us, but inside a double-quoted string + // that doesn't happen. As such, color the variable + the slice start red. Coloring any + // more than that looks bad, unless we're willing to try and detect where the double-quoted + // string ends, and I'd rather not do that. + std::fill(colors, colors + idx + 1, (highlight_spec_t)highlight_spec_error); + break; + } } } return idx; @@ -861,7 +875,11 @@ static void color_argument_internal(const wcstring &buffstr, std::vector<highlig */ case e_double_quoted: { - colors[in_pos] = highlight_spec_quote; + // slices are colored in advance, past `in_pos`, and we don't want to overwrite that + if (colors[in_pos] == highlight_spec_param) + { + colors[in_pos] = highlight_spec_quote; + } switch (c) { case L'"': @@ -876,7 +894,7 @@ static void color_argument_internal(const wcstring &buffstr, std::vector<highlig if (in_pos + 1 < buff_len) { const wchar_t escaped_char = buffstr.at(in_pos + 1); - if (escaped_char == L'\\' || escaped_char == L'\'' || escaped_char == L'$') + if (wcschr(L"\\\"\n$", escaped_char)) { colors[in_pos] = highlight_spec_escape; //backslash colors[in_pos + 1] = highlight_spec_escape; //escaped char diff --git a/tests/expansion.err b/tests/expansion.err new file mode 100644 index 00000000..e60c203d --- /dev/null +++ b/tests/expansion.err @@ -0,0 +1,30 @@ +Array index out of bounds +fish: show "$foo[2]" + ^ +Array index out of bounds +fish: show $foo[2] + ^ +Array index out of bounds +fish: show "$foo[1 2]" + ^ +Array index out of bounds +fish: show $foo[1 2] + ^ +Array index out of bounds +fish: show "$foo[2 1]" + ^ +Array index out of bounds +fish: show $foo[2 1] + ^ +Invalid index value +fish: echo "$foo[d]" + ^ +Invalid index value +fish: echo $foo[d] + ^ +Array index out of bounds +fish: echo ()[1] + ^ +Invalid index value +fish: echo ()[d] + ^ diff --git a/tests/expansion.in b/tests/expansion.in new file mode 100644 index 00000000..a2225127 --- /dev/null +++ b/tests/expansion.in @@ -0,0 +1,82 @@ +# Test expansion of variables + +function show --description 'Prints argument count followed by arguments' + echo (count $argv) $argv +end + +set -l foo +show "$foo" +show $foo +show "prefix$foo" +show prefix$foo + +show "$$foo" +show $$foo +show "prefix$$foo" +show prefix$$foo + +set -l foo '' +show "$foo" +show $foo +show "prefix$foo" +show prefix$foo + +show "$$foo" +show $$foo +show "prefix$$foo" +show prefix$$foo + +set -l foo bar +set -l bar +show "$$foo" +show $$foo +show "prefix$$foo" +show prefix$$foo + +set -l bar baz +show "$$foo" +show $$foo +show "prefix$$foo" +show prefix$$foo + +set -l bar baz quux +show "$$foo" +show $$foo +show "prefix$$foo" +show prefix$$foo + +set -l foo bar fooer fooest +set -l fooer +set -l fooest +show "$$foo" +show $$foo +show "prefix$$foo" +show prefix$$foo + +set -l fooer '' +show $$foo +show prefix$$foo + +set -l foo bar '' fooest +show "$$foo" +show $$foo +show "prefix$$foo" +show prefix$$foo + +set -l foo +show "$foo[1]" +show $foo[1] +show "$foo[-1]" +show $foo[-1] +show "$foo[2]" +show $foo[2] +show "$foo[1 2]" +show $foo[1 2] +show "$foo[2 1]" +show $foo[2 1] + +echo "$foo[d]" +echo $foo[d] + +echo ()[1] +echo ()[d] diff --git a/tests/expansion.out b/tests/expansion.out new file mode 100644 index 00000000..fe594539 --- /dev/null +++ b/tests/expansion.out @@ -0,0 +1,42 @@ +1 +0 +1 prefix +0 +1 +0 +1 prefix +0 +1 +1 +1 prefix +1 prefix +1 +0 +1 prefix +0 +1 +0 +1 prefix +0 +1 baz +1 baz +1 prefixbaz +1 prefixbaz +1 baz quux +2 baz quux +1 prefixbaz quux +2 prefixbaz prefixquux +1 baz quux fooer fooest +2 baz quux +1 prefixbaz quux fooer fooest +2 prefixbaz prefixquux +3 baz quux +3 prefixbaz prefixquux prefix +1 baz quux fooest +2 baz quux +1 prefixbaz quux fooest +2 prefixbaz prefixquux +1 +0 +1 +0 diff --git a/tests/expansion.status b/tests/expansion.status new file mode 100644 index 00000000..573541ac --- /dev/null +++ b/tests/expansion.status @@ -0,0 +1 @@ +0 diff --git a/tests/top.out b/tests/top.out index f2873b15..a62ec047 100644 --- a/tests/top.out +++ b/tests/top.out @@ -1,4 +1,5 @@ Testing high level script functionality +File expansion.in tested ok File printf.in tested ok File read.in tested ok File test1.in tested ok |