40306 with doc tweaks: Change behaviour expanding alias in () function definition.

Now an error unless the () is part of the same error as the name.
Add ALIAS_FUNC_DEF option to allow it again.

8 files changed, 129 insertions, 6 deletions

diff --git a/ChangeLog b/ChangeLog
index 26f75ebf0..149c18130 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,12 @@
+2017-01-10  Peter Stephenson  <p.w.stephenson@ntlworld.com>
+
+	* 40306 with documentation additions: Doc/Zsh/options.yo,
+	README, Src/input.c, Src/options.c, Src/parse.c, Src/zsh.h,
+	Test/A02alias.ztst: Add ALIAS_FUNC_DEF option and make
+	the default behaviour to disallow functions where the
+	name is expanded as an alias (unless part of a complete
+	function definition within the alias).
+
 2017-01-10  Daniel Shahaf  <d.s@daniel.shahaf.name>
 
 	* 40303: Completion/Debian/Command/_bts: Add more subcommands.
diff --git a/Doc/Zsh/options.yo b/Doc/Zsh/options.yo
index f68a945ec..434b71094 100644
--- a/Doc/Zsh/options.yo
+++ b/Doc/Zsh/options.yo
@@ -1539,6 +1539,36 @@ enditem()
 
 subsect(Scripts and Functions)
 startitem()
+pindex(ALIAS_FUNC_DEF)
+pindex(NO_ALIAS_FUNC_DEF)
+pindex(ALIASFUNCDEF)
+pindex(NOALIASFUNCDEF)
+cindex(functions, defining with expanded aliases)
+cindex(aliases, expanding in function definition)
+item(tt(ALIAS_FUNC_DEF) <S>)(
+By default, zsh does not allow the definition of functions using
+the `var(name) tt(LPAR()RPAR())' syntax if var(name) was expanded as an
+alias: this causes an error.  This is usually the desired behaviour, as
+otherwise the combination of an alias and a function based on the same
+definition can easily cause problems.
+
+When this option is set, aliases can be used for defining functions.
+
+For example, consider the following definitions as they might
+occur in a startup file.
+
+example(alias foo=bar
+foo+LPAR()RPAR() {
+  print This probably does not do what you expect.
+})
+
+Here, tt(foo) is expanded as an alias to tt(bar) before the
+tt(LPAR()RPAR()) is encountered, so the function defined would be named
+tt(bar).  By default this is instead an error in native mode.  Note that
+quoting any part of the function name, or using the keyword
+tt(function), avoids the problem, so is recommended when the function
+name can also be an alias.
+)
 pindex(C_BASES)
 pindex(NO_C_BASES)
 pindex(CBASES)
diff --git a/README b/README
index 5646ff00a..414ee0b5d 100644
--- a/README
+++ b/README
@@ -29,8 +29,31 @@ Zsh is a shell with lots of features.  For a list of some of these, see the
 file FEATURES, and for the latest changes see NEWS.  For more
 details, see the documentation.
 
-Incompatibilities between 5.2 and 5.3.1
----------------------------------------
+Incompatibilities since 5.3.1
+-----------------------------
+
+The default behaviour of code like the following has changed:
+
+  alias foo='noglob foo'
+  foo() { print function body; }
+
+When this is encountered in a start-up file, or other place where input
+was read line by line, "foo" is in command position and is expanded as
+an alias before the function definition takes place.  In previous
+versions of the shell, this caused two functions "noglob" and "foo" to
+be defined.  Any expansion of an alias in a function definition is
+nearly always an unintended effect, as well as hard to detect, so has
+been made an error.  (The option setting NO_MULTI_FUNC_DEF turned this
+case into an error, but did not help with other cases and is off by
+default.)  The alternative, of not expanding the alias, was rejected as
+it was more difficult to achieve in the parser and also would silently
+change the shell's behaviur between versions.  A new option,
+ALIAS_FUNC_DEF, has been added, which can be set to make the shell
+behave as in previous versions.  It is in any case recommended to use
+the "function" keyword, as aliases are not expanded afterwards.
+
+Incompatibilities between 5.0.8 and 5.3.1
+-----------------------------------------
 
 1) In character classes delimited by "[" and "]" within patterns, whether
 used for filename generation (globbing) or other forms of pattern
@@ -159,10 +182,6 @@ following example illustrates how this differs from past versions.
      4 4 => 1  |  4 4 => 0   **
      4 5 => 1  |  4 5 => 1
 
-
-Incompatibilities between 5.0.8 and 5.2
----------------------------------------
-
 The behaviour of the parameter flag (P) has changed when it appears
 in a nested parameter group, in order to make it more useful in
 such cases.  A (P) in the outermost parameter group behaves as
diff --git a/Src/input.c b/Src/input.c
index eb968ea72..92abaec92 100644
--- a/Src/input.c
+++ b/Src/input.c
@@ -670,3 +670,30 @@ ingetptr(void)
 {
     return inbufptr;
 }
+
+/*
+ * Check if the current input line, including continuations, is
+ * expanding an alias.  This does not detect alias expansions that
+ * have been fully processed and popped from the input stack.
+ * If there is an alias, the most recently expanded is returned,
+ * else NULL.
+ */
+
+/**/
+char *input_hasalias(void)
+{
+    int flags = inbufflags;
+    struct instacks *instackptr = instacktop;
+
+    for (;;)
+    {
+	if (!(flags & INP_CONT))
+	    break;
+	instackptr--;
+	if (instackptr->alias)
+	    return instackptr->alias->node.nam;
+	flags = instackptr->flags;
+    }
+
+    return NULL;
+}
diff --git a/Src/options.c b/Src/options.c
index 18619c851..4729ba54a 100644
--- a/Src/options.c
+++ b/Src/options.c
@@ -78,6 +78,7 @@ mod_export HashTable optiontab;
  */
 static struct optname optns[] = {
 {{NULL, "aliases",	      OPT_EMULATE|OPT_ALL},	 ALIASESOPT},
+{{NULL, "aliasfuncdef",       OPT_EMULATE|OPT_BOURNE},	 ALIASFUNCDEF},
 {{NULL, "allexport",	      OPT_EMULATE},		 ALLEXPORT},
 {{NULL, "alwayslastprompt",   OPT_ALL},			 ALWAYSLASTPROMPT},
 {{NULL, "alwaystoend",	      0},			 ALWAYSTOEND},
diff --git a/Src/parse.c b/Src/parse.c
index 50a0d5f9f..ed6c4a8dd 100644
--- a/Src/parse.c
+++ b/Src/parse.c
@@ -1738,6 +1738,7 @@ par_simple(int *cmplx, int nr)
 {
     int oecused = ecused, isnull = 1, r, argc = 0, p, isfunc = 0, sr = 0;
     int c = *cmplx, nrediradd, assignments = 0, ppost = 0, is_typeset = 0;
+    char *hasalias = input_hasalias();
     wordcode postassigns = 0;
 
     r = ecused;
@@ -1809,6 +1810,8 @@ par_simple(int *cmplx, int nr)
 	} else
 	    break;
 	zshlex();
+	if (!hasalias)
+	    hasalias = input_hasalias();
     }
     if (tok == AMPER || tok == AMPERBANG)
 	YYERROR(oecused);
@@ -1839,6 +1842,8 @@ par_simple(int *cmplx, int nr)
 			char *idstring = dupstrpfx(tokstr+1, eptr-tokstr-1);
 			redir_var = 1;
 			zshlex();
+			if (!hasalias)
+			    hasalias = input_hasalias();
 
 			if (IS_REDIROP(tok) && tokfd == -1)
 			{
@@ -1874,6 +1879,8 @@ par_simple(int *cmplx, int nr)
 		    argc++;
 		}
 		zshlex();
+		if (!hasalias)
+		    hasalias = input_hasalias();
 	    }
 	} else if (IS_REDIROP(tok)) {
 	    *cmplx = c = 1;
@@ -1902,6 +1909,8 @@ par_simple(int *cmplx, int nr)
 	    ecstr(name);
 	    ecstr(str);
 	    zshlex();
+	    if (!hasalias)
+		hasalias = input_hasalias();
 	} else if (tok == ENVARRAY) {
 	    int n, parr;
 
@@ -1936,6 +1945,11 @@ par_simple(int *cmplx, int nr)
 	    /* Error if preceding assignments */
 	    if (assignments || postassigns)
 		YYERROR(oecused);
+	    if (hasalias && !isset(ALIASFUNCDEF) && argc &&
+		hasalias != input_hasalias()) {
+		zwarn("defining function based on alias `%s'", hasalias);
+		YYERROR(oecused);
+	    }
 
 	    *cmplx = c;
 	    lineno = 0;
diff --git a/Src/zsh.h b/Src/zsh.h
index f22d8b135..2a41638db 100644
--- a/Src/zsh.h
+++ b/Src/zsh.h
@@ -2222,6 +2222,7 @@ struct histent {
 enum {
     OPT_INVALID,
     ALIASESOPT,
+    ALIASFUNCDEF,
     ALLEXPORT,
     ALWAYSLASTPROMPT,
     ALWAYSTOEND,
diff --git a/Test/A02alias.ztst b/Test/A02alias.ztst
index e52578ec3..e68e93e0d 100644
--- a/Test/A02alias.ztst
+++ b/Test/A02alias.ztst
@@ -82,6 +82,7 @@
 0:Global aliasing quotes
 > a string S 
 *>*5*echo S a string S "
+# "
 # Note there is a trailing space on the "> a string S " line
 
   (
@@ -115,3 +116,24 @@
 1:error message has the correct sign
 ?(eval):alias:1: bad option: +x
 ?(eval):alias:1: bad option: -z
+
+  # Usual issue that aliases aren't expanded until we
+  # trigger a new parse...
+  (alias badalias=notacommand
+  eval 'badalias() { print does not work; }')
+1:ALIAS_FUNC_DEF off by default.
+?(eval):1: defining function based on alias `badalias'
+?(eval):1: parse error near `()'
+
+  (alias goodalias=isafunc
+  setopt ALIAS_FUNC_DEF
+  eval 'goodalias() { print does now work; }'
+  isafunc)
+0:ALIAS_FUNC_DEF causes the icky behaviour to be avaliable
+>does now work
+
+  (alias thisisokthough='thisworks() { print That worked; }'
+  eval thisisokthough
+  thisworks)
+0:NO_ALIAS_FUNC_DEF works if the alias is a complete definition
+>That worked