zsh -n doesn't grok associate array indexes?

zsh -n doesn't grok associate array indexes?

[Rocky Bernstein]

[-- Attachment #1: Type: text/plain, Size: 1112 bytes --]

I tried zsh -n to see how good a tool it might be for lint checking. Overall
it works very well.

However I ran into a problem when using a string associative array index.
Here's a sample run:

$ cat /tmp/zsh-bug.sh
> typeset -A hash
> hash['display']=5
> echo ${hash['display']}
> $ zsh /tmp/zsh-bug.sh
> 5
> $ zsh -n /tmp/zsh-bug.sh
> /tmp/zsh-bug.sh:2: bad math expression: operand expected at `'display''
> /tmp/zsh-bug.sh:3: bad math expression: operand expected at `'display''
> $ zsh --version
> zsh 4.3.10 (i686-pc-linux-gnu)
> $


By the way, I also tried ksh -n from version 93u- 2010-09-22. It also
catches basic syntax errors such as with
complex statements (if, while, case, for) but offers a number of lint- like
suggestions such as to improve speed and and allow for better
ksh conformance. Out of 57 or so files I tried in the zshdb
distribution, there were about 5 errors which I don't think anythingn could
be done about, i.e. legitimate differences between zsh and ksh. However
there were many  warnings for performance improvements, removing deprecated
constructs and better ksh conformance.

Re: zsh -n doesn't grok associate array indexes?

[ZyX]

[-- Attachment #1: Type: Text/Plain, Size: 1968 bytes --]

Reply to message «zsh -n doesn't grok associate array indexes?», 
sent 20:18:09 10 January 2011, Monday
by Rocky Bernstein:

Confirmed:
    zsh << EOF
    emulate -LR zsh
    typeset -A hash
    hash['display']=5
    echo ${hash['display']}
    EOF
echoes 5 as expected, but «zsh -n» echoes the same error.

    % zsh --version
    zsh 4.3.10 (x86_64-pc-linux-gnu)

By the way, if I replace «'display'» with «display» (which is correct because 
zsh does not unquote subscripts, you can check it by using «echo ${(kv)hash}») I 
I get correct value without «-n» and «zsh: hash: assignment to invalid subscript 
range» with «-n».

By the way, I can't find description of «NO_EXEC» option in «man zshoptions».

Original message:
> I tried zsh -n to see how good a tool it might be for lint checking.
> Overall it works very well.
> 
> However I ran into a problem when using a string associative array index.
> Here's a sample run:
> 
> $ cat /tmp/zsh-bug.sh
> 
> > typeset -A hash
> > hash['display']=5
> > echo ${hash['display']}
> > $ zsh /tmp/zsh-bug.sh
> > 5
> > $ zsh -n /tmp/zsh-bug.sh
> > /tmp/zsh-bug.sh:2: bad math expression: operand expected at `'display''
> > /tmp/zsh-bug.sh:3: bad math expression: operand expected at `'display''
> > $ zsh --version
> > zsh 4.3.10 (i686-pc-linux-gnu)
> > $
> 
> By the way, I also tried ksh -n from version 93u- 2010-09-22. It also
> catches basic syntax errors such as with
> complex statements (if, while, case, for) but offers a number of lint- like
> suggestions such as to improve speed and and allow for better
> ksh conformance. Out of 57 or so files I tried in the zshdb
> distribution, there were about 5 errors which I don't think anythingn could
> be done about, i.e. legitimate differences between zsh and ksh. However
> there were many  warnings for performance improvements, removing deprecated
> constructs and better ksh conformance.

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: zsh -n doesn't grok associate array indexes?

[Peter Stephenson]

On Mon, 10 Jan 2011 12:18:09 -0500
Rocky Bernstein <rocky.bernstein@gmail.com> wrote:
> I tried zsh -n to see how good a tool it might be for lint checking.
> Overall it works very well.
> 
> However I ran into a problem when using a string associative array
> index. Here's a sample run:
> 
> $ cat /tmp/zsh-bug.sh
> > typeset -A hash
> > hash['display']=5
> > echo ${hash['display']}
> > $ zsh /tmp/zsh-bug.sh
> > 5
> > $ zsh -n /tmp/zsh-bug.sh
> > /tmp/zsh-bug.sh:2: bad math expression: operand expected at
> > `'display'' /tmp/zsh-bug.sh:3: bad math expression: operand
> > expected at `'display'' $ zsh --version
> > zsh 4.3.10 (i686-pc-linux-gnu)

Certainly, a subscript is expected to be a math expression unless you've
explicitly told it the variable is an associative array.  However, given
that it's not executing the assignment I can see how you'd want it to
be otherwise here...  Actually, it looks like it was executing the
assignment, which must surely be wrong, too, since, as here, the
resulting operation may be nonsense.

I don't think anyone's ever sanity-checked NO_EXEC to this depth before,
let alone optimised it for useful checking, so you could run across
anything.  I've run across one myself:  we do globbing, which might
throw up a "no match" error.  Presumably doing any form of globbing is
pointless; I'm not sure how far to go into the code before it's not
worth it, but 'not very far' is the default answer, since the expression
hasn't been through previous expansion stages properly.

Index: Src/glob.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/glob.c,v
retrieving revision 1.74
diff -p -u -r1.74 glob.c
--- Src/glob.c	5 Dec 2010 21:07:48 -0000	1.74
+++ Src/glob.c	10 Jan 2011 18:18:18 -0000
@@ -1111,7 +1111,7 @@ zglob(LinkList list, LinkNode np, int no
     struct globdata saved;		/* saved glob state              */
     int nobareglob = !isset(BAREGLOBQUAL);
 
-    if (unset(GLOBOPT) || !haswilds(ostr)) {
+    if (unset(GLOBOPT) || !haswilds(ostr) || unset(EXECOPT)) {
 	if (!nountok)
 	    untokenize(ostr);
 	return;
Index: Src/params.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/params.c,v
retrieving revision 1.166
diff -p -u -r1.166 params.c
--- Src/params.c	21 Dec 2010 16:54:30 -0000	1.166
+++ Src/params.c	10 Jan 2011 18:18:18 -0000
@@ -1055,6 +1055,14 @@ getarg(char **str, int *inv, Value v, in
     zlong num = 1, beg = 0, r = 0, quote_arg = 0;
     Patprog pprog = NULL;
 
+    /*
+     * If in NO_EXEC mode, the parameters won't be set up
+     * properly, so there's no point even doing any sanity checking.
+     * Just return 0 now.
+     */
+    if (unset(EXECOPT))
+	return 0;
+
     ishash = (v->pm && PM_TYPE(v->pm->node.flags) == PM_HASHED);
     if (prevcharlen)
 	*prevcharlen = 1;
@@ -2230,6 +2238,8 @@ export_param(Param pm)
 mod_export void
 setstrvalue(Value v, char *val)
 {
+    if (unset(EXECOPT))
+	return;
     if (v->pm->node.flags & PM_READONLY) {
 	zerr("read-only variable: %s", v->pm->node.nam);
 	zsfree(val);
@@ -2361,6 +2371,8 @@ setnumvalue(Value v, mnumber val)
 {
     char buf[BDIGBUFSIZE], *p;
 
+    if (unset(EXECOPT))
+	return;
     if (v->pm->node.flags & PM_READONLY) {
 	zerr("read-only variable: %s", v->pm->node.nam);
 	return;
@@ -2398,6 +2410,8 @@ setnumvalue(Value v, mnumber val)
 mod_export void
 setarrvalue(Value v, char **val)
 {
+    if (unset(EXECOPT))
+	return;
     if (v->pm->node.flags & PM_READONLY) {
 	zerr("read-only variable: %s", v->pm->node.nam);
 	freearray(val);
@@ -2808,6 +2822,8 @@ sethparam(char *s, char **val)
 	errflag = 1;
 	return NULL;
     }
+    if (unset(EXECOPT))
+	return;
     queue_signals();
     if (!(v = fetchvalue(&vbuf, &s, 1, SCANPM_ASSIGNING)))
 	createparam(t, PM_HASHED);
@@ -2846,6 +2862,8 @@ setnparam(char *s, mnumber val)
 	errflag = 1;
 	return NULL;
     }
+    if (unset(EXECOPT))
+	return;
     queue_signals();
     ss = strchr(s, '[');
     v = getvalue(&vbuf, &s, 1);
Index: Test/E01options.ztst
===================================================================
RCS file: /cvsroot/zsh/zsh/Test/E01options.ztst,v
retrieving revision 1.23
diff -p -u -r1.23 E01options.ztst
--- Test/E01options.ztst	22 Oct 2010 16:32:36 -0000	1.23
+++ Test/E01options.ztst	10 Jan 2011 18:18:18 -0000
@@ -344,6 +344,15 @@
 0:NO_EXEC option
 >before
 
+  (setopt noexec
+  typeset -A hash
+  hash['this is a string'])
+0:NO_EXEC option should not attempt to parse subscripts
+
+  (setopt noexec nomatch
+  echo *NonExistentFile*)
+0:NO_EXEC option should not do globbing
+
   setopt NO_eval_lineno
   eval 'print $LINENO'
   setopt eval_lineno

-- 
Peter Stephenson <pws@csr.com>            Software Engineer
Tel: +44 (0)1223 692070                   Cambridge Silicon Radio Limited
Churchill House, Cambridge Business Park, Cowley Road, Cambridge, CB4 0WZ, UK


Member of the CSR plc group of companies. CSR plc registered in England and Wales, registered number 4187346, registered office Churchill House, Cambridge Business Park, Cowley Road, Cambridge, CB4 0WZ, United Kingdom

Re: zsh -n doesn't grok associate array indexes?

[Rocky Bernstein]

[-- Attachment #1: Type: text/plain, Size: 6921 bytes --]

Thanks for the fixes and handling this quickly too.

My other (longish) observation is in line.

On Mon, Jan 10, 2011 at 1:20 PM, Peter Stephenson
<Peter.Stephenson@csr.com>wrote:

> On Mon, 10 Jan 2011 12:18:09 -0500
> Rocky Bernstein <rocky.bernstein@gmail.com> wrote:
> > I tried zsh -n to see how good a tool it might be for lint checking.
> > Overall it works very well.
> >
> > However I ran into a problem when using a string associative array
> > index. Here's a sample run:
> >
> > $ cat /tmp/zsh-bug.sh
> > > typeset -A hash
> > > hash['display']=5
> > > echo ${hash['display']}
> > > $ zsh /tmp/zsh-bug.sh
> > > 5
> > > $ zsh -n /tmp/zsh-bug.sh
> > > /tmp/zsh-bug.sh:2: bad math expression: operand expected at
> > > `'display'' /tmp/zsh-bug.sh:3: bad math expression: operand
> > > expected at `'display'' $ zsh --version
> > > zsh 4.3.10 (i686-pc-linux-gnu)
>
> Certainly, a subscript is expected to be a math expression unless you've
> explicitly told it the variable is an associative array.  However, given
> that it's not executing the assignment I can see how you'd want it to
> be otherwise here...  Actually, it looks like it was executing the
> assignment, which must surely be wrong, too, since, as here, the
> resulting operation may be nonsense.
>
> I don't think anyone's ever sanity-checked NO_EXEC to this depth before,
> let alone optimised it for useful checking, so you could run across
> anything.  I've run across one myself:  we do globbing, which might
> throw up a "no match" error.  Presumably doing any form of globbing is
> pointless; I'm not sure how far to go into the code before it's not
> worth it, but 'not very far' is the default answer, since the expression
> hasn't been through previous expansion stages properly.
>

Fair enough.

There still is a question (and it can remain a question) is:  is there is a
good lint tool for zsh?

The person who asked me about this (and I consider myself a novice in this
area), said he looked for something and only found a shell script circa
2003. I tried that script; it is far far worse than zsh -n. zsh -n can at
least report syntactic mistakes whereas this other script couldn't.

Thus, I think the expectation for the capabilities of lint checker is pretty
low. If the types in declarations are not tracked, so be it. Same for globs.

My own gut feeling about lint for zsh, is that ksh -n is pretty good if you
are willing to accept that you may get some errors because zsh is not ksh.

ksh -n will suggest removing shell expansion inside arithmetic expressions,
e.g. removing $ in (( $x == 0)). Or will suggest using == instead of = in
expressions, or will suggest replacing `xxx` by $(xxx).

I'm not suggesting zsh -n d do this as well. Probably one lint tool is
sufficient. I will probably use both zsh -n and ksh -n since each will
 undoubtedly catch something that the other won't.

But most importantly, thanks again for the patches below.


> Index: Src/glob.c
> ===================================================================
> RCS file: /cvsroot/zsh/zsh/Src/glob.c,v
> retrieving revision 1.74
> diff -p -u -r1.74 glob.c
> --- Src/glob.c  5 Dec 2010 21:07:48 -0000       1.74
> +++ Src/glob.c  10 Jan 2011 18:18:18 -0000
> @@ -1111,7 +1111,7 @@ zglob(LinkList list, LinkNode np, int no
>     struct globdata saved;             /* saved glob state              */
>     int nobareglob = !isset(BAREGLOBQUAL);
>
> -    if (unset(GLOBOPT) || !haswilds(ostr)) {
> +    if (unset(GLOBOPT) || !haswilds(ostr) || unset(EXECOPT)) {
>        if (!nountok)
>            untokenize(ostr);
>        return;
> Index: Src/params.c
> ===================================================================
> RCS file: /cvsroot/zsh/zsh/Src/params.c,v
> retrieving revision 1.166
> diff -p -u -r1.166 params.c
> --- Src/params.c        21 Dec 2010 16:54:30 -0000      1.166
> +++ Src/params.c        10 Jan 2011 18:18:18 -0000
> @@ -1055,6 +1055,14 @@ getarg(char **str, int *inv, Value v, in
>     zlong num = 1, beg = 0, r = 0, quote_arg = 0;
>     Patprog pprog = NULL;
>
> +    /*
> +     * If in NO_EXEC mode, the parameters won't be set up
> +     * properly, so there's no point even doing any sanity checking.
> +     * Just return 0 now.
> +     */
> +    if (unset(EXECOPT))
> +       return 0;
> +
>     ishash = (v->pm && PM_TYPE(v->pm->node.flags) == PM_HASHED);
>     if (prevcharlen)
>        *prevcharlen = 1;
> @@ -2230,6 +2238,8 @@ export_param(Param pm)
>  mod_export void
>  setstrvalue(Value v, char *val)
>  {
> +    if (unset(EXECOPT))
> +       return;
>     if (v->pm->node.flags & PM_READONLY) {
>        zerr("read-only variable: %s", v->pm->node.nam);
>        zsfree(val);
> @@ -2361,6 +2371,8 @@ setnumvalue(Value v, mnumber val)
>  {
>     char buf[BDIGBUFSIZE], *p;
>
> +    if (unset(EXECOPT))
> +       return;
>     if (v->pm->node.flags & PM_READONLY) {
>        zerr("read-only variable: %s", v->pm->node.nam);
>        return;
> @@ -2398,6 +2410,8 @@ setnumvalue(Value v, mnumber val)
>  mod_export void
>  setarrvalue(Value v, char **val)
>  {
> +    if (unset(EXECOPT))
> +       return;
>     if (v->pm->node.flags & PM_READONLY) {
>        zerr("read-only variable: %s", v->pm->node.nam);
>        freearray(val);
> @@ -2808,6 +2822,8 @@ sethparam(char *s, char **val)
>        errflag = 1;
>        return NULL;
>     }
> +    if (unset(EXECOPT))
> +       return;
>     queue_signals();
>     if (!(v = fetchvalue(&vbuf, &s, 1, SCANPM_ASSIGNING)))
>        createparam(t, PM_HASHED);
> @@ -2846,6 +2862,8 @@ setnparam(char *s, mnumber val)
>        errflag = 1;
>        return NULL;
>     }
> +    if (unset(EXECOPT))
> +       return;
>     queue_signals();
>     ss = strchr(s, '[');
>     v = getvalue(&vbuf, &s, 1);
> Index: Test/E01options.ztst
> ===================================================================
> RCS file: /cvsroot/zsh/zsh/Test/E01options.ztst,v
> retrieving revision 1.23
> diff -p -u -r1.23 E01options.ztst
> --- Test/E01options.ztst        22 Oct 2010 16:32:36 -0000      1.23
> +++ Test/E01options.ztst        10 Jan 2011 18:18:18 -0000
> @@ -344,6 +344,15 @@
>  0:NO_EXEC option
>  >before
>
> +  (setopt noexec
> +  typeset -A hash
> +  hash['this is a string'])
> +0:NO_EXEC option should not attempt to parse subscripts
> +
> +  (setopt noexec nomatch
> +  echo *NonExistentFile*)
> +0:NO_EXEC option should not do globbing
> +
>   setopt NO_eval_lineno
>   eval 'print $LINENO'
>   setopt eval_lineno
>
> --
> Peter Stephenson <pws@csr.com>            Software Engineer
> Tel: +44 (0)1223 692070                   Cambridge Silicon Radio Limited
> Churchill House, Cambridge Business Park, Cowley Road, Cambridge, CB4 0WZ,
> UK
>
>
> Member of the CSR plc group of companies. CSR plc registered in England and
> Wales, registered number 4187346, registered office Churchill House,
> Cambridge Business Park, Cowley Road, Cambridge, CB4 0WZ, United Kingdom
>