From cd562b16e2e586c4c4b5d5aaaf1fe2d1ca164a42 Mon Sep 17 00:00:00 2001
From: Johnothan King <johnothanking@protonmail.com>
Date: Sun, 12 Dec 2021 13:25:07 -0800
Subject: [PATCH] Port more shell lint improvements from illumos and ksh93v-
 (#374)

This commit adds onto <https://github.com/ksh93/ksh/pull/353> by porting
over two additional improvements to the shell linter:

1) The changes in the aforementioned pull request were merged into
   illumos-gate with an additional change.[*] The illumos revision of
   the patch improved the warning for (( $foo = $? )) to specify '$foo'
   causes the warning.[**] Example:
      $ ksh -n -c '(( $? != $bar ))'
      ksh: warning: line 1: in '(( $? != $bar ))', using '$' as in '$bar' is slower and can introduce rounding errors
   While I was porting the illumos patch I did notice one problem. The
   string it uses from paramsub() skips over the initial '{' in
   '${var}', resulting in the warning printing '$var}' instead:
      $ ksh -n -c '(( ${.sh.pid} != $$ ))'
      ...  in '(( ${.sh.pid} != $$ ))', using '$' as in '$.sh.pid}' is slower  ...
   This was fixed by including the missing '{' in the string returned by
   paramsub for ${var} variables.

2) In ksh93v-, parsing x=$((expr)) with the shell linter will cause ksh
   to warn the user x=$((expr)) is slower than ((x=expr)). This
   improvement has been backported with a modified warning:
      # Result from this commit
      $ ksh -n -c 'x=$((1 + 2))'
      ksh: warning: line 1: x=$((1 + 2)) is slower than ((x=1 + 2))
      # Result from ksh93v-
      $ ksh93v -n -c 'x=$((1 + 2))'
      ksh93v: warning: line 1: ((x=1 + 2)) is more efficient than x=$((1 + 2))
   Minor note: the ksh93v- patch had an invalid use of memcmp; this
   version of the patch uses strncmp instead.

References:
https://github.com/illumos/illumos-gate/commit/be548e87bcb4979e0b8a7bce5620a9916b888fdc
https://code.illumos.org/c/illumos-gate/+/1834/comment/65722363_22fdf8e7/
---
 NEWS                              |  6 ++++++
 src/cmd/ksh93/data/lexstates.c    |  3 ++-
 src/cmd/ksh93/include/lexstates.h |  1 +
 src/cmd/ksh93/sh/lex.c            |  9 +++++++++
 src/cmd/ksh93/sh/parse.c          | 25 ++++++++++++++++++-------
 5 files changed, 36 insertions(+), 8 deletions(-)

diff --git a/NEWS b/NEWS
index 7fbdb0740..acb2adc1b 100644
--- a/NEWS
+++ b/NEWS
@@ -9,6 +9,12 @@ Any uppercase BUG_* names are modernish shell bug IDs.
   as SIGINT due to Ctrl+C) while the user is entering a multi-line command
   substitution in an interactive shell.
 
+- The shell linter's warning for variable expansion in ((...)) now tells the
+  user which variable is causing performance degradation.
+
+- The shell linter now warns the user x=$((expr)) is slower than ((x=expr))
+  when assigning a value to a variable.
+
 2021-12-09:
 
 - Increased the general robustness of discipline function handling, fixing
diff --git a/src/cmd/ksh93/data/lexstates.c b/src/cmd/ksh93/data/lexstates.c
index 19a8dd7c4..90f1cd560 100644
--- a/src/cmd/ksh93/data/lexstates.c
+++ b/src/cmd/ksh93/data/lexstates.c
@@ -430,7 +430,8 @@ const char e_lexsyntax2[]	= "syntax error: `%s' %s";
 const char e_lexsyntax3[]	= "syntax error at line %d: duplicate label %s";
 const char e_lexsyntax4[]	= "syntax error at line %d: invalid reference list";
 const char e_lexsyntax5[]	= "syntax error at line %d: `<<%s' here-document not contained within command substitution";
-const char e_lexwarnvar[]	= "line %d: in '((%s))', using '$' is slower and can introduce rounding errors";
+const char e_lexwarnvar[]	= "line %d: in '((%s))', using '$' as in '$%.*s' is slower and can introduce rounding errors";
+const char e_lexarithwarn[]	= "line %d: %s is slower than ((%.*s%s";
 const char e_lexlabignore[]	= "line %d: label %s ignored";
 const char e_lexlabunknown[]	= "line %d: %s unknown label";
 const char e_lexobsolete1[]	= "line %d: `...` obsolete, use $(...)";
diff --git a/src/cmd/ksh93/include/lexstates.h b/src/cmd/ksh93/include/lexstates.h
index fb882a66e..3a57b5f56 100644
--- a/src/cmd/ksh93/include/lexstates.h
+++ b/src/cmd/ksh93/include/lexstates.h
@@ -138,6 +138,7 @@ extern const char e_lexsyntax3[];
 extern const char e_lexsyntax4[];
 extern const char e_lexsyntax5[];
 extern const char e_lexwarnvar[];
+extern const char e_lexarithwarn[];
 extern const char e_lexobsolete1[];
 extern const char e_lexobsolete2[];
 extern const char e_lexobsolete3[];
diff --git a/src/cmd/ksh93/sh/lex.c b/src/cmd/ksh93/sh/lex.c
index 9136f45fd..2f14c6161 100644
--- a/src/cmd/ksh93/sh/lex.c
+++ b/src/cmd/ksh93/sh/lex.c
@@ -1327,7 +1327,16 @@ breakloop:
 	state = lp->arg->argval;
 	lp->comp_assign = assignment;
 	if(assignment)
+	{
 		lp->arg->argflag |= ARG_ASSIGN;
+		if(sh_isoption(SH_NOEXEC))
+		{
+			char *cp = strchr(state, '=');
+			if(cp && strncmp(++cp, "$((", 3) == 0)
+				errormsg(SH_DICT, ERROR_warn(0), e_lexarithwarn, shp->inlineno,
+					state, cp - state, state, cp + 3);
+		}
+	}
 	else if(!lp->lex.skipword)
 		lp->assignok = 0;
 	lp->arg->argchn.cp = 0;
diff --git a/src/cmd/ksh93/sh/parse.c b/src/cmd/ksh93/sh/parse.c
index 1806d01bc..4f5a90ddc 100644
--- a/src/cmd/ksh93/sh/parse.c
+++ b/src/cmd/ksh93/sh/parse.c
@@ -250,7 +250,7 @@ static Shnode_t	*makeparent(Lex_t *lp, int flag, Shnode_t *child)
 	return(par);
 }
 
-static int paramsub(const char *str)
+static const char *paramsub(const char *str)
 {
 	register int c,sub=0,lit=0;
 	while(c= *str++)
@@ -258,16 +258,20 @@ static int paramsub(const char *str)
 		if(c=='$' && !lit)
 		{
 			if(*str=='(')
-				return(0);
+				return(NIL(const char*));
 			if(sub)
 				continue;
 			if(*str=='{')
 				str++;
 			if(!isdigit(*str) && strchr("?#@*!$ ",*str)==0)
-				return(1);
+			{
+				if(str[-1]=='{')
+					str--;  /* variable in the form of ${var} */
+				return(str);
+			}
 		}
 		else if(c=='`')
-			return(0);
+			return(NIL(const char*));
 		else if(c=='[' && !lit)
 			sub++;
 		else if(c==']' && !lit)
@@ -275,7 +279,7 @@ static int paramsub(const char *str)
 		else if(c=='\'')
 			lit = !lit;
 	}
-	return(0);
+	return(NIL(const char*));
 }
 
 static Shnode_t *getanode(Lex_t *lp, struct argnod *ap)
@@ -288,8 +292,15 @@ static Shnode_t *getanode(Lex_t *lp, struct argnod *ap)
 		t->ar.arcomp = sh_arithcomp(lp->sh,ap->argval);
 	else
 	{
-		if(sh_isoption(SH_NOEXEC) && (ap->argflag&ARG_MAC) && paramsub(ap->argval))
-			errormsg(SH_DICT,ERROR_warn(0),e_lexwarnvar,lp->sh->inlineno,ap->argval);
+		const char *p, *q;
+		if(sh_isoption(SH_NOEXEC) && (ap->argflag&ARG_MAC) &&
+			((p = paramsub(ap->argval)) != NIL(const char*)))
+		{
+			for(q = p; !isspace(*q) && *q != '\0'; q++)
+				;
+			errormsg(SH_DICT, ERROR_warn(0), e_lexwarnvar,
+				lp->sh->inlineno, ap->argval, q - p, p);
+		}
 		t->ar.arcomp = 0;
 	}
 	return(t);