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

[Rocky Bernstein <rocky.bernstein@xxxxxxxxx>]

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@xxxxxxx>wrote:

> On Mon, 10 Jan 2011 12:18:09 -0500
> Rocky Bernstein <rocky.bernstein@xxxxxxxxx> 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@xxxxxxx>            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
>