Excise use of parser_t's error() functionality. Thread a

parse_error_list_t through all of the expand functions, enabling them to report errors more directly. Improve aspects of error reporting for expansion failures.
2026-05-27 16:51:15 -03:00 · 2014-03-21 17:13:33 -07:00
parent c71b168402
commit ad6367018b
12 changed files with 221 additions and 152 deletions
--- a/expand.cpp
+++ b/expand.cpp
@@ -131,6 +131,46 @@ int expand_is_clean(const wchar_t *in)
    return 1;
 }

+
+/* Append a syntax error to the given error list */
+static void append_syntax_error(parse_error_list_t *errors, size_t source_start, const wchar_t *fmt, ...)
+{
+    if (errors != NULL)
+    {
+        parse_error_t error;
+        error.source_start = source_start;
+        error.source_length = 0;
+        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);
+    }
+}
+
+/* Append a cmdsub error to the given error list */
+static void append_cmdsub_error(parse_error_list_t *errors, size_t source_start, const wchar_t *fmt, ...)
+{
+    if (errors != NULL)
+    {
+        parse_error_t error;
+        error.source_start = source_start;
+        error.source_length = 0;
+        error.code = parse_error_cmdsubst;
+
+        va_list va;
+        va_start(va, fmt);
+        error.text = vformat_string(fmt, va);
+        va_end(va);
+
+        errors->push_back(error);
+    }
+}
+
+
 /**
   Return the environment variable value for the string starting at \c in.
 */
@@ -834,7 +874,7 @@ static int expand_pid(const wcstring &instr_with_sep,
 }


-void expand_variable_error(parser_t &parser, const wcstring &token, size_t token_pos, int error_pos)
+void expand_variable_error(parser_t &parser, const wcstring &token, size_t token_pos, int error_pos, parse_error_list_t *errors)
 {
    size_t stop_pos = token_pos+1;

@@ -861,7 +901,7 @@ void expand_variable_error(parser_t &parser, const wcstring &token, size_t token

            if (is_var)
            {
-                parser.error(SYNTAX_ERROR,
+                append_syntax_error(errors,
                             error_pos,
                             COMPLETE_VAR_BRACKET_DESC,
                             cpy,
@@ -870,7 +910,7 @@ void expand_variable_error(parser_t &parser, const wcstring &token, size_t token
            }
            else
            {
-                parser.error(SYNTAX_ERROR,
+                append_syntax_error(errors,
                             error_pos,
                             COMPLETE_VAR_BRACKET_DESC,
                             L"",
@@ -884,7 +924,7 @@ void expand_variable_error(parser_t &parser, const wcstring &token, size_t token

        case INTERNAL_SEPARATOR:
        {
-            parser.error(SYNTAX_ERROR,
+            append_syntax_error(errors,
                         error_pos,
                         COMPLETE_VAR_PARAN_DESC);
            break;
@@ -892,7 +932,7 @@ void expand_variable_error(parser_t &parser, const wcstring &token, size_t token

        case 0:
        {
-            parser.error(SYNTAX_ERROR,
+            append_syntax_error(errors,
                         error_pos,
                         COMPLETE_VAR_NULL_DESC);
            break;
@@ -907,7 +947,7 @@ void expand_variable_error(parser_t &parser, const wcstring &token, size_t token
            else if (token_stop_char == ANY_STRING || token_stop_char == ANY_STRING_RECURSIVE)
                token_stop_char = L'*';

-            parser.error(SYNTAX_ERROR,
+            append_syntax_error(errors,
                         error_pos,
                         (token_stop_char == L'?' ? COMPLETE_YOU_WANT_STATUS : COMPLETE_VAR_DESC),
                         token_stop_char);
@@ -919,7 +959,7 @@ void expand_variable_error(parser_t &parser, const wcstring &token, size_t token
 /**
   Parse an array slicing specification
 */
-static int parse_slice(const wchar_t *in, wchar_t **end_ptr, std::vector<long> &idx, size_t array_size)
+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)
 {
    wchar_t *end;

@@ -942,6 +982,7 @@ static int parse_slice(const wchar_t *in, wchar_t **end_ptr, std::vector<long> &
        }

        errno=0;
+        const size_t i1_src_pos = pos;
        tmp = wcstol(&in[pos], &end, 10);
        if ((errno) || (end == &in[pos]))
        {
@@ -958,6 +999,8 @@ static int parse_slice(const wchar_t *in, wchar_t **end_ptr, std::vector<long> &
            pos+=2;
            while (in[pos]==INTERNAL_SEPARATOR)
                pos++;
+            
+            const size_t number_start = pos;
            long tmp1 = wcstol(&in[pos], &end, 10);
            if ((errno) || (end == &in[pos]))
            {
@@ -973,12 +1016,14 @@ static int parse_slice(const wchar_t *in, wchar_t **end_ptr, std::vector<long> &
            {
                // debug(0, L"Expand range [subst]: %i\n", jjj);
                idx.push_back(jjj);
+                source_positions.push_back(number_start);
            }
            continue;
        }

        // debug( 0, L"Push idx %d", tmp );
        idx.push_back(i1);
+        source_positions.push_back(i1_src_pos);
    }

    if (end_ptr)
@@ -993,7 +1038,6 @@ static int parse_slice(const wchar_t *in, wchar_t **end_ptr, std::vector<long> &
 }


-
 /**
   Expand all environment variables in the string *ptr.

@@ -1008,23 +1052,28 @@ static int parse_slice(const wchar_t *in, wchar_t **end_ptr, std::vector<long> &
   happens, don't edit it unless you know exactly what you are doing,
   and do proper testing afterwards.
 */
-static int expand_variables_internal(parser_t &parser, wchar_t * const in, std::vector<completion_t> &out, long last_idx);
+static int expand_variables_internal(parser_t &parser, wchar_t * const in, std::vector<completion_t> &out, long last_idx, parse_error_list_t *errors);

-static int expand_variables2(parser_t &parser, const wcstring &instr, std::vector<completion_t> &out, long last_idx)
+static int expand_variables2(parser_t &parser, const wcstring &instr, std::vector<completion_t> &out, long last_idx, parse_error_list_t *errors)
 {
    wchar_t *in = wcsdup(instr.c_str());
-    int result = expand_variables_internal(parser, in, out, last_idx);
+    int result = expand_variables_internal(parser, in, out, last_idx, errors);
    free(in);
    return result;
 }

-static int expand_variables_internal(parser_t &parser, wchar_t * const in, std::vector<completion_t> &out, long last_idx)
+static int expand_variables_internal(parser_t &parser, wchar_t * const in, std::vector<completion_t> &out, long last_idx, parse_error_list_t *errors)
 {
    int is_ok= 1;
    int empty=0;

    wcstring var_tmp;
+    
+    // list of indexes
    std::vector<long> var_idx_list;
+    
+    // parallel array of source positions of each index in the variable list
+    std::vector<size_t> var_pos_list;

    //  CHECK( out, 0 );

@@ -1057,7 +1106,7 @@ static int expand_variables_internal(parser_t &parser, wchar_t * const in, std::

            if (var_len == 0)
            {
-                expand_variable_error(parser, in, stop_pos-1, -1);
+                expand_variable_error(parser, in, stop_pos-1, -1, errors);

                is_ok = 0;
                break;
@@ -1074,16 +1123,17 @@ static int expand_variables_internal(parser_t &parser, wchar_t * const in, std::
                if (is_ok)
                {
                    tokenize_variable_array(var_val.c_str(), var_item_list);
-
-                    if (in[stop_pos] == L'[')
+                    
+                    const size_t slice_start = stop_pos;
+                    if (in[slice_start] == L'[')
                    {
                        wchar_t *slice_end;
                        all_vars=0;

-                        if (parse_slice(in + stop_pos, &slice_end, var_idx_list, var_item_list.size()))
+                        if (parse_slice(in + slice_start, &slice_end, var_idx_list, var_pos_list, var_item_list.size()))
                        {
-                            parser.error(SYNTAX_ERROR,
-                                         -1,
+                            append_syntax_error(errors,
+                                         stop_pos,
                                         L"Invalid index value");
                            is_ok = 0;
                            break;
@@ -1094,19 +1144,16 @@ static int expand_variables_internal(parser_t &parser, wchar_t * const in, std::
                    if (!all_vars)
                    {
                        wcstring_list_t string_values(var_idx_list.size());
-
                        for (size_t j=0; j<var_idx_list.size(); j++)
                        {
                            long tmp = var_idx_list.at(j);
-                            /*
-                                           Check that we are within array
-                                           bounds. If not, truncate the list to
-                                           exit.
-                                           */
+                            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())
                            {
-                                parser.error(SYNTAX_ERROR,
-                                             -1,
+                                /* 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,
                                             ARRAY_BOUNDS_ERR);
                                is_ok=0;
                                var_idx_list.resize(j);
@@ -1145,7 +1192,7 @@ static int expand_variables_internal(parser_t &parser, wchar_t * const in, std::
                            }
                        }
                        res.append(in + stop_pos);
-                        is_ok &= expand_variables2(parser, res, out, i);
+                        is_ok &= expand_variables2(parser, res, out, i, errors);
                    }
                    else
                    {
@@ -1173,7 +1220,7 @@ static int expand_variables_internal(parser_t &parser, wchar_t * const in, std::
                                    }
                                    new_in.append(next);
                                    new_in.append(in + stop_pos);
-                                    is_ok &= expand_variables2(parser, new_in, out, i);
+                                    is_ok &= expand_variables2(parser, new_in, out, i, errors);
                                }
                            }

@@ -1205,7 +1252,7 @@ static int expand_variables_internal(parser_t &parser, wchar_t * const in, std::
                    res.append(in);
                    res.append(in + stop_pos);

-                    is_ok &= expand_variables2(parser, res, out, i);
+                    is_ok &= expand_variables2(parser, res, out, i, errors);
                    return is_ok;
                }
            }
@@ -1225,7 +1272,7 @@ static int expand_variables_internal(parser_t &parser, wchar_t * const in, std::
 /**
   Perform bracket expansion
 */
-static int expand_brackets(parser_t &parser, const wcstring &instr, int flags, std::vector<completion_t> &out)
+static int expand_brackets(parser_t &parser, const wcstring &instr, int flags, std::vector<completion_t> &out, parse_error_list_t *errors)
 {
    bool syntax_error = false;
    int bracket_count=0;
@@ -1295,14 +1342,14 @@ static int expand_brackets(parser_t &parser, const wcstring &instr, int flags, s
                mod.push_back(BRACKET_END);
            }

-            return expand_brackets(parser, mod, 1, out);
+            return expand_brackets(parser, mod, 1, out, errors);
        }
    }

    if (syntax_error)
    {
-        parser.error(SYNTAX_ERROR,
-                     -1,
+        append_syntax_error(errors,
+                     SOURCE_LOCATION_UNKNOWN,
                     _(L"Mismatched brackets"));
        return 0;
    }
@@ -1331,7 +1378,7 @@ static int expand_brackets(parser_t &parser, const wcstring &instr, int flags, s
                whole_item.append(in, length_preceding_brackets);
                whole_item.append(item_begin, item_len);
                whole_item.append(bracket_end + 1);
-                expand_brackets(parser, whole_item, flags, out);
+                expand_brackets(parser, whole_item, flags, out, errors);

                item_begin = pos+1;
                if (pos == bracket_end)
@@ -1355,7 +1402,7 @@ static int expand_brackets(parser_t &parser, const wcstring &instr, int flags, s
 /**
 Perform cmdsubst expansion
 */
-static int expand_cmdsubst(parser_t &parser, const wcstring &input, std::vector<completion_t> &out_list)
+static int expand_cmdsubst(parser_t &parser, const wcstring &input, std::vector<completion_t> &out_list, parse_error_list_t *errors)
 {
    wchar_t *paran_begin=0, *paran_end=0;
    std::vector<wcstring> sub_res;
@@ -1368,8 +1415,8 @@ static int expand_cmdsubst(parser_t &parser, const wcstring &input, std::vector<
    switch (parse_ret = parse_util_locate_cmdsubst(in, &paran_begin, &paran_end, false))
    {
        case -1:
-            parser.error(SYNTAX_ERROR,
-                         -1,
+            append_syntax_error(errors,
+                         SOURCE_LOCATION_UNKNOWN,
                         L"Mismatched parenthesis");
            return 0;
        case 0:
@@ -1384,7 +1431,7 @@ static int expand_cmdsubst(parser_t &parser, const wcstring &input, std::vector<

    if (exec_subshell(subcmd, sub_res, true /* do apply exit status */) == -1)
    {
-        parser.error(CMDSUBST_ERROR, -1, L"Unknown error while evaulating command substitution");
+        append_cmdsub_error(errors, SOURCE_LOCATION_UNKNOWN, L"Unknown error while evaulating command substitution");
        return 0;
    }

@@ -1392,24 +1439,25 @@ static int expand_cmdsubst(parser_t &parser, const wcstring &input, std::vector<
    if (*tail_begin == L'[')
    {
        std::vector<long> slice_idx;
+        std::vector<size_t> slice_source_positions;
        wchar_t *slice_end;

-        if (parse_slice(tail_begin, &slice_end, slice_idx, sub_res.size()))
+        if (parse_slice(tail_begin, &slice_end, slice_idx, slice_source_positions, sub_res.size()))
        {
-            parser.error(SYNTAX_ERROR, -1, L"Invalid index value");
+            append_syntax_error(errors, SOURCE_LOCATION_UNKNOWN, L"Invalid index value");
            return 0;
        }
        else
        {
-            std::vector<wcstring> sub_res2;
+            wcstring_list_t sub_res2;
            tail_begin = slice_end;
            for (i=0; i < slice_idx.size(); i++)
            {
                long idx = slice_idx.at(i);
                if (idx < 1 || (size_t)idx > sub_res.size())
                {
-                    parser.error(SYNTAX_ERROR,
-                                 -1,
+                    append_syntax_error(errors,
+                                 SOURCE_LOCATION_UNKNOWN,
                                 ARRAY_BOUNDS_ERR);
                    return 0;
                }
@@ -1430,7 +1478,7 @@ static int expand_cmdsubst(parser_t &parser, const wcstring &input, std::vector<
       of the string is inserted into the tail_expand array list
       */
    std::vector<completion_t> tail_expand;
-    expand_cmdsubst(parser, tail_begin, tail_expand);
+    expand_cmdsubst(parser, tail_begin, tail_expand, errors /* TODO: offset error locations */);

    /*
       Combine the result of the current command substitution with the
@@ -1639,7 +1687,7 @@ static void remove_internal_separator(wcstring &str, bool conv)
 }


-int expand_string(const wcstring &input, std::vector<completion_t> &output, expand_flags_t flags)
+int expand_string(const wcstring &input, std::vector<completion_t> &output, expand_flags_t flags, parse_error_list_t *errors)
 {
    parser_t parser(PARSER_TYPE_ERRORS_ONLY, true /* show errors */);

@@ -1661,14 +1709,14 @@ int expand_string(const wcstring &input, std::vector<completion_t> &output, expa

        if (parse_util_locate_cmdsubst(input.c_str(), &begin, &end, true) != 0)
        {
-            parser.error(CMDSUBST_ERROR, -1, L"Command substitutions not allowed");
+            append_cmdsub_error(errors, SOURCE_LOCATION_UNKNOWN, L"Command substitutions not allowed");
            return EXPAND_ERROR;
        }
        append_completion(*in, input);
    }
    else
    {
-        int cmdsubst_ok = expand_cmdsubst(parser, input, *in);
+        int cmdsubst_ok = expand_cmdsubst(parser, input, *in, errors);
        if (! cmdsubst_ok)
            return EXPAND_ERROR;
    }
@@ -1696,7 +1744,7 @@ int expand_string(const wcstring &input, std::vector<completion_t> &output, expa
        }
        else
        {
-            if (!expand_variables2(parser, next, *out, next.size() - 1))
+            if (!expand_variables2(parser, next, *out, next.size() - 1, errors))
            {
                return EXPAND_ERROR;
            }
@@ -1710,7 +1758,7 @@ int expand_string(const wcstring &input, std::vector<completion_t> &output, expa
    {
        const wcstring &next = in->at(i).completion;

-        if (!expand_brackets(parser, next, flags, *out))
+        if (!expand_brackets(parser, next, flags, *out, errors))
        {
            return EXPAND_ERROR;
        }
@@ -1840,7 +1888,7 @@ int expand_string(const wcstring &input, std::vector<completion_t> &output, expa
    return res;
 }

-bool expand_one(wcstring &string, expand_flags_t flags)
+bool expand_one(wcstring &string, expand_flags_t flags, parse_error_list_t *errors)
 {
    std::vector<completion_t> completions;
    bool result = false;
@@ -1850,7 +1898,7 @@ bool expand_one(wcstring &string, expand_flags_t flags)
        return true;
    }

-    if (expand_string(string, completions, flags | EXPAND_NO_DESCRIPTIONS))
+    if (expand_string(string, completions, flags | EXPAND_NO_DESCRIPTIONS, errors))
    {
        if (completions.size() == 1)
        {