Fix for issue where 'function' would not define a function if the

arguments came before its name. Fixes #1240

fish_tests.cpp

@@ -2417,6 +2417,38 @@ static void test_new_parser_ll2(void)
         if (deco != tests[i].deco)
             err(L"When parsing '%ls', expected decoration %d but got %d on line %ld", tests[i].src.c_str(), (int)tests[i].deco, (int)deco, (long)__LINE__);
     }
+    
+    /* Verify that 'function -h' and 'function --help' are plain statements but 'function --foo' is not (#1240) */
+    const struct
+    {
+        wcstring src;
+        parse_token_type_t type;
+    }
+    tests2[] =
+    {
+        {L"function -h", symbol_plain_statement},
+        {L"function --help", symbol_plain_statement},
+        {L"function --foo ; end", symbol_function_header},
+        {L"function foo ; end", symbol_function_header},
+    };
+    for (size_t i=0; i < sizeof tests2 / sizeof *tests2; i++)
+    {
+        parse_node_tree_t tree;
+        if (! parse_tree_from_string(tests2[i].src, parse_flag_none, &tree, NULL))
+        {
+            err(L"Failed to parse '%ls'", tests2[i].src.c_str());
+        }
+        
+        const parse_node_tree_t::parse_node_list_t node_list = tree.find_nodes(tree.at(0), tests2[i].type);
+        if (node_list.size() == 0)
+        {
+            err(L"Failed to find node of type '%ls'", token_type_description(tests2[i].type).c_str());
+        }
+        else if (node_list.size() > 1)
+        {
+            err(L"Found too many nodes of type '%ls'", token_type_description(tests2[i].type).c_str());
+        }
+    }
 }
 
 static void test_new_parser_ad_hoc()

parse_productions.cpp

@@ -116,12 +116,22 @@ PRODUCTIONS(statement) =
 };
 RESOLVE(statement)
 {
-    // Go to decorated statements if the subsequent token looks like '--'
-    // If we are 'begin', then we expect to be invoked with no arguments. But if we are anything else, we require an argument, so do the same thing if the subsequent token is a line end.
+    /* The only block-like builtin that takes any parameters is 'function' So go to decorated statements if the subsequent token looks like '--'.
+        The logic here is subtle:
+            If we are 'begin', then we expect to be invoked with no arguments.
+            If we are 'function', then we are a non-block if we are invoked with -h or --help
+            If we are anything else, we require an argument, so do the same thing if the subsequent token is a statement terminator.
+    */
+     
     if (token1.type == parse_token_type_string)
     {
-        // If the next token looks like an option (starts with a dash), then parse it as a decorated statement
-        if (token2.has_dash_prefix)
+        // If we are a function, then look for help arguments
+        // Othewrise, if the next token looks like an option (starts with a dash), then parse it as a decorated statement
+        if (token1.keyword == parse_keyword_function && token2.is_help_argument)
+        {
+            return 4;
+        }
+        else if (token1.keyword != parse_keyword_function && token2.has_dash_prefix)
         {
             return 4;
         }

parse_tree.cpp

@@ -1070,6 +1070,11 @@ static const parse_token_t kInvalidToken = {token_type_invalid, parse_keyword_no
 /* Terminal token */
 static const parse_token_t kTerminalToken = {parse_token_type_terminate, parse_keyword_none, false, -1, -1};
 
+static inline bool is_help_argument(const wchar_t *txt)
+{
+    return ! wcscmp(txt, L"-h") || ! wcscmp(txt, L"--help");
+}
+
 /* Return a new parse token, advancing the tokenizer */
 static inline parse_token_t next_parse_token(tokenizer_t *tok)
 {
@@ -1090,6 +1095,7 @@ static inline parse_token_t next_parse_token(tokenizer_t *tok)
     result.type = parse_token_type_from_tokenizer_token(tok_type);
     result.keyword = keyword_for_token(tok_type, tok_txt);
     result.has_dash_prefix = (tok_txt[0] == L'-');
+    result.is_help_argument = result.has_dash_prefix && is_help_argument(tok_txt);
     result.source_start = (size_t)tok_start;
     result.source_length = tok_extent;
     

parse_tree.h

@@ -47,6 +47,7 @@ struct parse_token_t
     enum parse_token_type_t type; // The type of the token as represented by the parser
     enum parse_keyword_t keyword; // Any keyword represented by this token
     bool has_dash_prefix; // Hackish: whether the source contains a dash prefix
+    bool is_help_argument; // Hackish: whether the source looks like '-h' or '--help'
     size_t source_start;
     size_t source_length;