test foo =~ foo should fail with exit status 2 (#245)

When test is passed the '=~' operator, it will silently fail with
exit status 1:
    $ test foo =~ foo; echo $?
    1
This bug is caused by test_binop reaching the 'NOTREACHED' area of
code. The bugfix was adapted from ksh2020:
https://github.com/att/ast/issues/1152

src/cmd/ksh93/bltins/test.c: test_binop():
- Error out with a message suggesting usage of '[[ ... ]]' if '=~'
  is passed to the test builtin.
- Special-case TEST_END (']]') as that is not really an operator.

Co-authored-by: Martijn Dekker <martijn@inlv.org>

NEWS

@@ -3,6 +3,13 @@ For full details, see the git log at: https://github.com/ksh93/ksh
 
 Any uppercase BUG_* names are modernish shell bug IDs.
 
+2021-03-27:
+
+- The 'test' builtin will now show an error message when given the invalid ']]'
+  or '=~' operators; it also properly returns with exit status 2 now (instead
+  of exit status 1). If the invalid operator is supported by [[ ... ]] (such
+  as '=~'), test will now suggest the usage of [[ ... ]] instead.
+
 2021-03-22:
 
 - A new --globcasedetect shell option is added to ksh on OSs where we can check

src/cmd/ksh93/bltins/test.c

@@ -510,7 +510,6 @@ int test_binop(Shell_t *shp,register int op,const char *left,const char *right)
 	}
 	switch(op)
 	{
-		/* op must be one of the following values */
 		case TEST_AND:
 		case TEST_OR:
 			return(*left!=0);
@@ -544,9 +543,20 @@ int test_binop(Shell_t *shp,register int op,const char *left,const char *right)
 			return(lnum>=rnum);
 		case TEST_LE:
 			return(lnum<=rnum);
+		default:
+		{
+			/* fallback for operators not supported by the test builtin */
+			int i=0;
+			char *e_msg;
+			while(shtab_testops[i].sh_number && shtab_testops[i].sh_number != op)
+				i++;
+			if(op==TEST_END)
+				e_msg = e_badop;
+			else
+				e_msg = e_unsupported_op;
+			errormsg(SH_DICT,ERROR_exit(2),e_msg,shtab_testops[i].sh_name);
+		}
 	}
-	/* NOTREACHED */
-	return(0);
 }
 
 /*

src/cmd/ksh93/data/testops.c

@@ -20,7 +20,7 @@
 #pragma prototyped
 
 /*
- * tables for the test builtin [[...]] and [...]
+ * tables for the test builtin [[ ... ]] and [ ... ]
  */
 
 #include	<ast.h>
@@ -29,7 +29,7 @@
 #include	"test.h"
 
 /*
- * This is the list of binary test and [[...]] operators
+ * This is the list of binary test and [[ ... ]] operators
  */
 
 const Shtable_t shtab_testops[] =
@@ -166,5 +166,6 @@ const char test_opchars[]	= "HLNRSVOGCaeohrwxdcbfugkv"
 const char e_argument[]		= "argument expected";
 const char e_missing[]		= "%s missing";
 const char e_badop[]		= "%s: unknown operator";
+const char e_unsupported_op[]	= "%s: operator not supported; use [[ ... ]]";
 const char e_tstbegin[]		= "[[ ! ";
 const char e_tstend[]		= " ]]\n";

src/cmd/ksh93/include/test.h

@@ -66,6 +66,7 @@ extern const char	test_opchars[];
 extern const char	e_argument[];
 extern const char	e_missing[];
 extern const char	e_badop[];
+extern const char	e_unsupported_op[];
 extern const char	e_tstbegin[];
 extern const char	e_tstend[];
 

src/cmd/ksh93/include/version.h

@@ -20,7 +20,7 @@
 
 #define SH_RELEASE_FORK	"93u+m"		/* only change if you develop a new ksh93 fork */
 #define SH_RELEASE_SVER	"1.0.0-alpha"	/* semantic version number: https://semver.org */
-#define SH_RELEASE_DATE	"2021-03-22"	/* must be in this format for $((.sh.version)) */
+#define SH_RELEASE_DATE	"2021-03-27"	/* must be in this format for $((.sh.version)) */
 #define SH_RELEASE_CPYR	"(c) 2020-2021 Contributors to ksh " SH_RELEASE_FORK
 
 /* Scripts sometimes field-split ${.sh.version}, so don't change amount of whitespace. */

src/cmd/ksh93/tests/builtins.sh

@@ -996,6 +996,32 @@ then	got=$( { "$SHELL" -c '
 		"(got status $e$( ((e>128)) && print -n / && kill -l "$e"), $(printf %q "$got"))"
 fi
 
+# ==========
+# Verify that the POSIX 'test' builtin complains loudly when the '=~' operator is used rather than
+# failing silently. See https://github.com/att/ast/issues/1152.
+actual=$($SHELL -c 'test foo =~ foo' 2>&1)
+actual_status=$?
+actual=${actual#*: }
+expect='test: =~: operator not supported; use [[...]]'
+expect_status=2
+[[ "$actual" = "$expect" ]] || err_exit "test =~ failed (expected $expect, got $actual)"
+[[ "$actual_status" = "$expect_status" ]] ||
+    err_exit "test =~ failed with the wrong exit status (expected $expect_status, got $actual_status)"
+
+# Invalid operators 'test' and '[[...]]' both reject should also cause an error with exit status 2.
+for operator in '===' ']]'
+do
+	actual="$($SHELL -c "test foo $operator foo" 2>&1)"
+	actual_status=$?
+	actual=${actual#*: }
+	expect="test: $operator: unknown operator"
+	expect_status=2
+	[[ "$actual" = "$expect" ]] || err_exit "test $operator failed" \
+		"(expected $(printf %q "$expect"), got $(printf %q "$actual"))"
+	[[ "$actual_status" = "$expect_status" ]] ||
+		err_exit "'test foo $operator foo' failed with the wrong exit status (expected $expect_status, got $actual_status)"
+done
+
 # ======
 # Regression test for https://github.com/att/ast/issues/1402
 #