pkgtools/check-portability: automatic test, refactoring

4 files changed, 67 insertions, 8 deletions

diff --git a/pkgtools/check-portability/Makefile b/pkgtools/check-portability/Makefile
index ac84267c1ba..d1687809725 100644
--- a/pkgtools/check-portability/Makefile
+++ b/pkgtools/check-portability/Makefile
@@ -1,4 +1,4 @@
-# $NetBSD: Makefile,v 1.7 2020/03/14 09:18:49 rillig Exp $
+# $NetBSD: Makefile,v 1.8 2020/03/21 15:02:20 rillig Exp $
 
 PKGNAME=	check-portability-19.4.3
 CATEGORIES=	pkgtools
@@ -19,4 +19,8 @@ AUTO_MKDIRS=		yes
 do-extract:
 	cd ${FILESDIR} && ${CP} -R . ${WRKSRC}
 
+do-test:
+	${WRKSRC}/check-portability files/testdata/* \
+	| ${DIFF} -u files/testdata/zzz_expected -
+
 .include "../../mk/bsd.pkg.mk"
diff --git a/pkgtools/check-portability/files/check-portability.c b/pkgtools/check-portability/files/check-portability.c
index 7e5afc8717c..e9644e6f2d0 100644
--- a/pkgtools/check-portability/files/check-portability.c
+++ b/pkgtools/check-portability/files/check-portability.c
@@ -1,4 +1,4 @@
-/* $NetBSD: check-portability.c,v 1.11 2020/03/14 09:47:09 rillig Exp $ */
+/* $NetBSD: check-portability.c,v 1.12 2020/03/21 15:02:20 rillig Exp $ */
 
 /*
  Copyright (c) 2020 Roland Illig
@@ -159,6 +159,15 @@ cstr_right_of_last(cstr s, cstr delimiter)
 	return cstr_substr(s, i + delimiter.len, s.len);
 }
 
+static bool
+cstr_has_word_boundary(cstr s, size_t idx)
+{
+	assert(idx <= s.len);
+	if (idx == 0 || idx == s.len)
+		return true;
+	return is_alnum(s.data[idx - 1]) != is_alnum(s.data[idx]);
+}
+
 // str is a modifiable string buffer.
 typedef struct {
 	char *data;
@@ -364,7 +373,10 @@ checkline_sh_dollar_random(cstr filename, size_t lineno, cstr line)
 	// a safe usage of $RANDOM, it will pass the test.
 	if (is_shell_comment(line))
 		return;
-	if (!cstr_contains(line, CSTR("$RANDOM")))
+	size_t idx = cstr_index(line, CSTR("$RANDOM"));
+
+	// Variable names that only start with RANDOM are not special.
+	if (idx == npos || !cstr_has_word_boundary(line, idx + 7))
 		return;
 
 	// $RANDOM together with the PID is often found in GNU-style
@@ -374,11 +386,6 @@ checkline_sh_dollar_random(cstr filename, size_t lineno, cstr line)
 	if (cstr_contains(line, CSTR("$RANDOM-$$")))
 		return;
 
-	// Variable names that only start with RANDOM are not special.
-	size_t idx = cstr_index(line, CSTR("$RANDOM"));
-	if (idx != npos && idx + 7 < line.len && is_alnum(line.data[idx + 7]))
-		return;
-
 	printf("%s:%zu:%zu: $RANDOM: %s\n",
 	    cstr_charptr(filename), lineno, idx + 1,
 	    cstr_charptr(line));
diff --git a/pkgtools/check-portability/files/testdata/random b/pkgtools/check-portability/files/testdata/random
index b26a1d7f8a3..302c34a6af7 100644
--- a/pkgtools/check-portability/files/testdata/random
+++ b/pkgtools/check-portability/files/testdata/random
@@ -13,3 +13,6 @@ $$-$RANDOM
 # This is not the style used in GNU configure scripts, thus no warning
 # is necessary. This doesn't occur in practice.
 ${RANDOM}
+
+# This is not a special variable.
+$RANDOMNESS
diff --git a/pkgtools/check-portability/files/testdata/zzz_expected b/pkgtools/check-portability/files/testdata/zzz_expected
new file mode 100644
index 00000000000..a342f2f02a3
--- /dev/null
+++ b/pkgtools/check-portability/files/testdata/zzz_expected
@@ -0,0 +1,45 @@
+files/testdata/Makefile.am:7:5: double brackets: 	if [[ cond ]]; then :; fi
+
+	The keyword [[ is only available in bash, not in other shells.
+	Since it is typically used inside an if statement, if that
+	command is missing, it is interpreted as a "no".
+
+	An error message of the form "[[: command not found"
+	is logged, but that is easy to overlook in the large
+	output of the build process.
+
+
+	To fix this message, decide whether this file is necessary
+	for the package to build. Then choose any of these variants:
+
+	1.  Add a patch for the file
+	    (see https://www.netbsd.org/docs/pkgsrc/pkgsrc.html#components.patches)
+	2.  Add a SUBST block for the file to the package Makefile
+	    (see mk/subst.mk)
+	3.  Add CHECK_PORTABILITY_SKIP+= shell/glob to the package Makefile
+	    (see mk/check/check-portability.mk)
+
+files/testdata/Makefile.am:8:5: double brackets: 	if [[ ${COND} ]] || [[ $(COND) ]]; then :; fi
+files/testdata/double-brackets:8:4: double brackets: if [[ test ]]; then
+files/testdata/double-brackets:10:1: double brackets: [[ test ]]
+files/testdata/double-brackets:12:1: double brackets: [[ test ]] || echo
+files/testdata/env-sh:8:4: found test ... == ...: [ a == b ]
+
+	The "test" command, as well as the "[" command, are not required to know
+	the "==" operator. Only a few implementations like bash and some
+	versions of ksh support it.
+
+	When you run "test foo == foo" on a platform that does not support the
+	"==" operator, the result will be "false" instead of "true". This can
+	lead to unexpected behavior.
+
+files/testdata/random:7:1: $RANDOM: $RANDOM
+
+	The variable $RANDOM is not required for a POSIX-conforming shell, and
+	many implementations of /bin/sh do not support it. It should therefore
+	not be used in shell programs that are meant to be portable across a
+	large number of POSIX-like systems.
+
+files/testdata/test-eqeq:7:7: found test ... == ...: test a == b	# bad
+files/testdata/test-eqeq:10:4: found test ... == ...: [ a == b ]	# bad
+files/testdata/test-eqeq:14:4: found test ... == ...: [ a == b -a c == d ]