From 28c7094f5bef662442de4c9168d2d464ff503e61 Mon Sep 17 00:00:00 2001
From: ridiculousfish <corydoras@ridiculousfish.com>
Date: Tue, 14 Jan 2014 02:29:53 -0800
Subject: [PATCH] Fix for issue where 'function' would not define a function if
 the arguments came before its name. Fixes #1240

---
 fish_tests.cpp        | 32 ++++++++++++++++++++++++++++++++
 parse_productions.cpp | 18 ++++++++++++++----
 parse_tree.cpp        |  6 ++++++
 parse_tree.h          |  1 +
 4 files changed, 53 insertions(+), 4 deletions(-)

diff --git a/fish_tests.cpp b/fish_tests.cpp
index b78d31fd4..26a6f5b11 100644
--- a/fish_tests.cpp
+++ b/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()
diff --git a/parse_productions.cpp b/parse_productions.cpp
index 53a90a56a..311d68086 100644
--- a/parse_productions.cpp
+++ b/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;
         }
diff --git a/parse_tree.cpp b/parse_tree.cpp
index 4a08f9033..051c8d8fb 100644
--- a/parse_tree.cpp
+++ b/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;
     
diff --git a/parse_tree.h b/parse_tree.h
index f2c6702e5..530b5c25c 100644
--- a/parse_tree.h
+++ b/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;