[PATCH] Re: namespaces limitation

[Bart Schaefer <schaefer@brasslantern.com>]

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

On Thu, May 18, 2023 at 5:14 PM Oliver Kiddle <opk@zsh.org> wrote:
>
>   integer .var.d=0
>   (( .var.d++ ))
>   zsh: bad floating point constant
>
> It'd be good if it could identify this as a namespace variable.

This should be correctly handled after this patch, test is included.
If you find other broken edge cases please point them out.

On Thu, May 25, 2023 at 3:09 PM Bart Schaefer <schaefer@brasslantern.com> wrote:
>
> On Wed, May 24, 2023 at 9:41 AM Oliver Kiddle <opk@zsh.org> wrote:
> >
> >   integer .var=56
> >
> > would be disallowed.
>
> I had it that way at one point and backed off because ksh allows it.

Patch below leaves this unchanged (so still allowed) and adds a test for it.

> Strictly speaking (to the extent that any of this is strict),
> namespaces are supposed to otherwise obey the rules for identifiers,
> which with the exception of positional parameters are not allowed to
> begin with a digit ... so I think I'll just go with that.
>
> I still haven't worked out how to deal correctly with .var.3x

The below enforces that identifiers following a namespace prefix must
either begin with a non-digit or must consist entirely of digits.  For
possible future enhancements, this does not apply when the identifier
before the first "." does not itself begin with a ".", that is,
${var.3x} is allowed.

I haven't determined how/where to document that latter special case.
Perhaps it's not necessary to do so, given that omitting the first "."
is explicitly discouraged.

> > Also questionable is the following which currently works:
> >
> >   .a.=ddd

This is rejected by the patch (not an identifier / bad substitution).
I think the extra checks add very little if any overhead to the
unaffected cases but I haven't run benchmarks.

> > We also currently allow a.=ddd but I think that is less
> > questionable. We allow empty association elements.
>
> I'm still not entirely sold on the x.y <=> x[y] mapping idea, but if
> we follow JavaScript's (somewhat questionable) lead on this, empty
> elements have to use bracket-notation rather than dot-notation.

I've not [yet] enforced the bracket-notation requirement, so as a
special case of the ${var.3x} case above, ${var.} is also allowed.  A
point against following JS is that JS doesn't have the braces around
the reference to delineate the empty element.

[-- Attachment #2: namespace-limiting.txt --]
[-- Type: text/plain, Size: 3928 bytes --]

diff --git a/Src/math.c b/Src/math.c
index 12c8d6f6b..a060181ed 100644
--- a/Src/math.c
+++ b/Src/math.c
@@ -641,7 +641,9 @@ zzlex(void)
 		return MINUSEQ;
 	    }
 	    if (unary) {
-		if (idigit(*ptr) || *ptr == '.') {
+		if (idigit(*ptr) ||
+		    (*ptr == '.' &&
+		     (idigit(ptr[1]) || !itype_end(ptr, INAMESPC, 0)))) {
 		    int ctype = lexconstant();
 		    if (ctype == NUM)
 		    {
@@ -835,7 +837,9 @@ zzlex(void)
 	case Dnull:
 	    break;
 	default:
-	    if (idigit(*--ptr) || *ptr == '.')
+	    if (idigit(*--ptr) ||
+		(*ptr == '.' &&
+		 (idigit(ptr[1]) || !itype_end(ptr, INAMESPC, 0))))
 		return lexconstant();
 	    if (*ptr == '#') {
 		if (*++ptr == '\\' || *ptr == '#') {
@@ -857,7 +861,7 @@ zzlex(void)
 		}
 		cct = 1;
 	    }
-	    if ((ie = itype_end(ptr, IIDENT, 0)) != ptr) {
+	    if ((ie = itype_end(ptr, INAMESPC, 0)) != ptr) {
 		int func = 0;
 		char *p;
 
diff --git a/Src/params.c b/Src/params.c
index 021d341e8..2b0837e03 100644
--- a/Src/params.c
+++ b/Src/params.c
@@ -1226,6 +1226,26 @@ isident(char *s)
     if (!*s)			/* empty string is definitely not valid */
 	return 0;
 
+    /* This partly duplicates code in itype_end(), but we need to
+     * distinguish the leading namespace at this point to check the
+     * correctness of the identifier that follows
+     */
+    if (*s == '.') {
+	if (idigit(s[1]))
+	    return 0;	/* Namespace must not start with a digit */
+	/* Reject identifiers beginning with a digit in namespaces.
+	 * Move this out below this block to also reject v.1x form.
+	 */
+	if ((ss = itype_end(s + (*s == '.'), IIDENT, 0))) {
+	    if (*ss == '.') {
+		if (!ss[1])
+		    return 0;
+		if (idigit(ss[1]))
+		    s = ss + 1;
+	    }
+	}
+    }
+
     if (idigit(*s)) {
 	/* If the first character is `s' is a digit, then all must be */
 	for (ss = ++s; *ss; ss++)
@@ -2148,7 +2168,12 @@ fetchvalue(Value v, char **pptr, int bracks, int flags)
 	    pm = (Param) paramtab->getnode2(paramtab, *t == '0' ? "0" : t);
 	else
 	    pm = (Param) paramtab->getnode(paramtab, *t == '0' ? "0" : t);
-	if (sav)
+	if (!pm && *t == '.' && !isident(t)) {
+	    /* badly formed namespace reference */
+	    if (sav)
+		*s = sav;
+	    return NULL;
+	} else if (sav)
 	    *s = sav;
 	*pptr = s;
 	if (!pm || ((pm->node.flags & PM_UNSET) &&
diff --git a/Test/K02parameter.ztst b/Test/K02parameter.ztst
index 8a1be1e36..0b1a8dd4a 100644
--- a/Test/K02parameter.ztst
+++ b/Test/K02parameter.ztst
@@ -100,7 +100,55 @@ F:Braces are required
 >.k02.array
 >characters
 
+  k.=empty
   k.2=test
-  print ${k.2}
+  print ${k.} ${k.2}
 0:Parse without leading dot (future proofing)
->test
+>empty test
+
+  .k=OK
+  print ${.k}
+0:Bare namespace is usable (ksh compatibility)
+>OK
+
+  .k.=empty
+1:Namespace must precede an identifier, assignment
+?(eval):1: not an identifier: .k.
+
+  typeset .k.=empty
+1:Namespace must precede an identifier, typeset
+?(eval):typeset:1: not valid in this context: .k.
+
+  print ${.k.}
+1:Namespace must precede an identifier, reference
+?(eval):1: bad substitution
+
+  .2.b=not
+1:Namespace identifier must not begin with a digit, assignment
+?(eval):1: not an identifier: .2.b
+
+  typeset .2.b=not
+1:Namespace identifier must not begin with a digit, typeset
+?(eval):typeset:1: not valid in this context: .2.b
+
+  print ${.2.b}
+1:Namespace identifier must not begin with a digit, reference
+?(eval):1: bad substitution
+
+  .not.2b=question
+1:Identifier starting with a digit must be all digits, assignment
+?(eval):1: not an identifier: .not.2b
+
+  typeset .not.2b=question
+1:Identifier starting with a digit must be all digits, typeset
+?(eval):typeset:1: not valid in this context: .not.2b
+
+  print ${.not.2b}
+1:Identifier starting with a digit must be all digits, reference
+?(eval):1: bad substitution
+
+  integer .var.d=0
+  float .var.f=.2
+  print $((.var.x = ++.var.d - -.var.f))
+0:Namespaces in math context
+>1.2