Support for error detection in arguments in new parser. Restores error

reporting for bad arguments (e.g. with bad variable names)

expand.cpp

@@ -53,31 +53,6 @@ parameter expansion.
 
 #include "parse_util.h"
 
-/**
-   Error issued on invalid variable name
-*/
-#define COMPLETE_VAR_DESC _( L"The '$' character begins a variable name. The character '%lc', which directly followed a '$', is not allowed as a part of a variable name, and variable names may not be zero characters long. To learn more about variable expansion in fish, type 'help expand-variable'.")
-
-/**
-   Error issued on $?
-*/
-#define COMPLETE_YOU_WANT_STATUS _( L"$? is not a valid variable in fish. If you want the exit status of the last command, try $status.")
-
-/**
-   Error issued on invalid variable name
-*/
-#define COMPLETE_VAR_NULL_DESC _( L"The '$' begins a variable name. It was given at the end of an argument. Variable names may not be zero characters long. To learn more about variable expansion in fish, type 'help expand-variable'.")
-
-/**
-   Error issued on invalid variable name
-*/
-#define COMPLETE_VAR_BRACKET_DESC _( L"Did you mean %ls{$%ls}%ls? The '$' character begins a variable name. A bracket, which directly followed a '$', is not allowed as a part of a variable name, and variable names may not be zero characters long. To learn more about variable expansion in fish, type 'help expand-variable'." )
-
-/**
-   Error issued on invalid variable name
-*/
-#define COMPLETE_VAR_PARAN_DESC _( L"Did you mean (COMMAND)? In fish, the '$' character is only used for accessing variables. To learn more about command substitution in fish, type 'help expand-command-substitution'.")
-
 /**
    Description for child process
 */

parse_constants.h

@@ -178,6 +178,31 @@ typedef unsigned int parser_test_error_bits_t;
 /** Error message for Posix-style assignment: foo=bar */
 #define COMMAND_ASSIGN_ERR_MSG _( L"Unknown command '%ls'. Did you mean 'set %ls %ls'? See the help section on the set command by typing 'help set'.")
 
+/**
+   Error issued on invalid variable name
+*/
+#define COMPLETE_VAR_DESC _( L"The '$' character begins a variable name. The character '%lc', which directly followed a '$', is not allowed as a part of a variable name, and variable names may not be zero characters long. To learn more about variable expansion in fish, type 'help expand-variable'.")
+
+/**
+   Error issued on $?
+*/
+#define COMPLETE_YOU_WANT_STATUS _( L"$? is not a valid variable in fish. If you want the exit status of the last command, try $status.")
+
+/**
+   Error issued on invalid variable name
+*/
+#define COMPLETE_VAR_NULL_DESC _( L"The '$' begins a variable name. It was given at the end of an argument. Variable names may not be zero characters long. To learn more about variable expansion in fish, type 'help expand-variable'.")
+
+/**
+   Error issued on invalid variable name
+*/
+#define COMPLETE_VAR_BRACKET_DESC _( L"Did you mean %ls{$%ls}%ls? The '$' character begins a variable name. A bracket, which directly followed a '$', is not allowed as a part of a variable name, and variable names may not be zero characters long. To learn more about variable expansion in fish, type 'help expand-variable'." )
+
+/**
+   Error issued on invalid variable name
+*/
+#define COMPLETE_VAR_PARAN_DESC _( L"Did you mean (COMMAND)? In fish, the '$' character is only used for accessing variables. To learn more about command substitution in fish, type 'help expand-command-substitution'.")
+
 /**
    While block description
 */

parse_util.cpp

@@ -1001,11 +1001,205 @@ static bool first_argument_is_help(const parse_node_tree_t &node_tree, const par
     return is_help;
 }
 
+void parse_util_expand_variable_error(const parse_node_t &node, const wcstring &token, size_t token_pos, size_t error_pos, parse_error_list_t *out_errors)
+{
+    size_t stop_pos = token_pos+1;
+
+    switch (token[stop_pos])
+    {
+        case BRACKET_BEGIN:
+        {
+            wchar_t *cpy = wcsdup(token.c_str());
+            *(cpy+token_pos)=0;
+            wchar_t *name = &cpy[stop_pos+1];
+            wchar_t *end = wcschr(name, BRACKET_END);
+            wchar_t *post;
+            int is_var=0;
+            if (end)
+            {
+                post = end+1;
+                *end = 0;
+
+                if (!wcsvarname(name))
+                {
+                    is_var = 1;
+                }
+            }
+
+            if (is_var)
+            {
+                append_syntax_error(out_errors,
+                                    node,
+                                    COMPLETE_VAR_BRACKET_DESC,
+                                    cpy,
+                                    name,
+                                    post);
+            }
+            else
+            {
+                append_syntax_error(out_errors,
+                                    node,
+                                    COMPLETE_VAR_BRACKET_DESC,
+                                    L"",
+                                    L"VARIABLE",
+                                    L"");
+            }
+            free(cpy);
+
+            break;
+        }
+
+        case INTERNAL_SEPARATOR:
+        {
+            append_syntax_error(out_errors,
+                                node,
+                                COMPLETE_VAR_PARAN_DESC);
+            break;
+        }
+
+        case 0:
+        {
+            append_syntax_error(out_errors,
+                                node,
+                                COMPLETE_VAR_NULL_DESC);
+            break;
+        }
+
+        default:
+        {
+            wchar_t token_stop_char = token[stop_pos];
+            // Unescape (see http://github.com/fish-shell/fish-shell/issues/50)
+            if (token_stop_char == ANY_CHAR)
+                token_stop_char = L'?';
+            else if (token_stop_char == ANY_STRING || token_stop_char == ANY_STRING_RECURSIVE)
+                token_stop_char = L'*';
+
+            append_syntax_error(out_errors,
+                                node,
+                                (token_stop_char == L'?' ? COMPLETE_YOU_WANT_STATUS : COMPLETE_VAR_DESC),
+                                token_stop_char);
+            break;
+        }
+    }
+}
+
+
+/**
+   Test if this argument contains any errors. Detected errors include
+   syntax errors in command substitutions, improperly escaped
+   characters and improper use of the variable expansion operator.
+*/
+parser_test_error_bits_t parse_util_detect_errors_in_argument(const parse_node_t &node, const wcstring &arg_src, parse_error_list_t *out_errors)
+{
+    int err=0;
+
+    wchar_t *paran_begin, *paran_end;
+    wchar_t *arg_cpy;
+    int do_loop = 1;
+
+    arg_cpy = wcsdup(arg_src.c_str());
+
+    while (do_loop)
+    {
+        switch (parse_util_locate_cmdsubst(arg_cpy,
+                                           &paran_begin,
+                                           &paran_end,
+                                           false))
+        {
+            case -1:
+            {
+                err=1;
+                if (out_errors)
+                {
+                    append_syntax_error(out_errors, node, L"Mismatched parenthesis");
+                }
+                free(arg_cpy);
+                return err;
+            }
+
+            case 0:
+            {
+                do_loop = 0;
+                break;
+            }
+
+            case 1:
+            {
+
+                const wcstring subst(paran_begin + 1, paran_end);
+                wcstring tmp;
+
+                tmp.append(arg_cpy, paran_begin - arg_cpy);
+                tmp.push_back(INTERNAL_SEPARATOR);
+                tmp.append(paran_end+1);
+
+                parse_error_list_t errors;
+                err |= parse_util_detect_errors(subst, &errors);
+                if (out_errors != NULL)
+                {
+                    /* Todo: have to tweak the offsets of the errors in the command substitution */
+                    out_errors->insert(out_errors->end(), errors.begin(), errors.end());
+                }
+
+                free(arg_cpy);
+                arg_cpy = wcsdup(tmp.c_str());
+
+                break;
+            }
+        }
+    }
+
+    wcstring unesc;
+    if (! unescape_string(arg_cpy, &unesc, UNESCAPE_SPECIAL))
+    {
+        if (out_errors)
+        {
+            append_syntax_error(out_errors, node, L"Invalid token '%ls'", arg_cpy);
+        }
+        return 1;
+    }
+    else
+    {
+        /* Check for invalid variable expansions */
+        const size_t unesc_size = unesc.size();
+        for (size_t idx = 0; idx < unesc_size; idx++)
+        {
+            switch (unesc.at(idx))
+            {
+                case VARIABLE_EXPAND:
+                case VARIABLE_EXPAND_SINGLE:
+                {
+                    wchar_t next_char = (idx + 1 < unesc_size ? unesc.at(idx + 1) : L'\0');
+
+                    if (next_char != VARIABLE_EXPAND &&
+                            next_char != VARIABLE_EXPAND_SINGLE &&
+                            ! wcsvarchr(next_char))
+                    {
+                        err=1;
+                        if (out_errors)
+                        {
+                            parse_util_expand_variable_error(node, unesc, idx, node.source_start, out_errors);
+                        }
+                    }
+
+                    break;
+                }
+            }
+        }
+    }
+
+    free(arg_cpy);
+
+    return err;
+}
+
 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;
 
+    parser_test_error_bits_t res = 0;
+
     // Whether we encountered a parse error
     bool errored = false;
 
@@ -1065,6 +1259,11 @@ parser_test_error_bits_t parse_util_detect_errors(const wcstring &buff_src, pars
                     errored = append_syntax_error(&parse_errors, node, EXEC_ERR_MSG, is_and ? L"and" : L"or");
                 }
             }
+            else if (node.type == symbol_argument)
+            {
+                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_plain_statement)
             {
                 // In a few places below, we want to know if we are in a pipeline
@@ -1159,8 +1358,6 @@ parser_test_error_bits_t parse_util_detect_errors(const wcstring &buff_src, pars
         }
     }
 
-    parser_test_error_bits_t res = 0;
-
     if (errored)
         res |= PARSER_TEST_ERROR;
 

parse_util.h

@@ -171,4 +171,6 @@ 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 = NULL);
 
+parser_test_error_bits_t parse_util_detect_errors_in_argument(const parse_node_t &node, const wcstring &arg_src, parse_error_list_t *out_errors = NULL);
+
 #endif