pkgtools/pkglint: update to 22.2.0

Changes since 22.1.0:

In ALTERNATIVES files, the wrapper path must be either in bin,
@PKGMANDIR@ or sbin.  This catches typos like "in" instead of "bin", as
well as hard-coded "man".

11 files changed, 112 insertions, 60 deletions

diff --git a/pkgtools/pkglint/Makefile b/pkgtools/pkglint/Makefile
index 4eec8c7cdc1..45ed9d5072a 100644
--- a/pkgtools/pkglint/Makefile
+++ b/pkgtools/pkglint/Makefile
@@ -1,7 +1,6 @@
-# $NetBSD: Makefile,v 1.717 2022/06/02 18:52:05 bsiegert Exp $
+# $NetBSD: Makefile,v 1.718 2022/06/24 07:16:23 rillig Exp $
 
-PKGNAME=	pkglint-22.1.0
-PKGREVISION=	3
+PKGNAME=	pkglint-22.2.0
 CATEGORIES=	pkgtools
 DISTNAME=	tools
 MASTER_SITES=	${MASTER_SITE_GITHUB:=golang/}
diff --git a/pkgtools/pkglint/files/alternatives.go b/pkgtools/pkglint/files/alternatives.go
index 4d3d8526a11..41839ff4dc8 100644
--- a/pkgtools/pkglint/files/alternatives.go
+++ b/pkgtools/pkglint/files/alternatives.go
@@ -29,7 +29,7 @@ func (ck *AlternativesChecker) Check(lines *Lines, pkg *Package) {
 }
 
 // checkLine checks a single line for the following format:
-//  wrapper alternative [optional arguments]
+//  wrapper alternative [arguments]
 func (ck *AlternativesChecker) checkLine(line *Line, plistFiles map[RelPath]*PlistLine, pkg *Package) {
 	m, wrapper, space, alternative := match3(line.Text, `^([^\t ]+)([ \t]+)([^\t ]+).*$`)
 	if !m {
@@ -39,9 +39,21 @@ func (ck *AlternativesChecker) checkLine(line *Line, plistFiles map[RelPath]*Pli
 		return
 	}
 
-	if ck.checkWrapperAbs(line, NewPath(wrapper)) && plistFiles != nil {
-		ck.checkWrapperPlist(line, NewRelPathString(wrapper), plistFiles)
+	if wrapper := NewPath(wrapper); wrapper.IsAbs() {
+		line.Errorf("Alternative wrapper %q must be relative to PREFIX.", wrapper.String())
+	} else {
+		wrapper := NewRelPath(wrapper)
+		if plistFiles[wrapper] != nil {
+			line.Errorf("Alternative wrapper %q must not appear in the PLIST.", wrapper)
+		}
+		if !wrapper.HasPrefixText("bin/") &&
+			!wrapper.HasPrefixText("@PKGMANDIR@/") &&
+			!wrapper.HasPrefixText("sbin/") {
+			line.Errorf("Alternative wrapper %q must be in "+
+				"\"bin\", \"@PKGMANDIR@\" or \"sbin\".", wrapper)
+		}
 	}
+
 	if plistFiles != nil {
 		ck.checkAlternativePlist(line, alternative, plistFiles, pkg)
 	}
@@ -51,23 +63,6 @@ func (ck *AlternativesChecker) checkLine(line *Line, plistFiles map[RelPath]*Pli
 	LineChecker{line}.CheckTrailingWhitespace()
 }
 
-func (ck *AlternativesChecker) checkWrapperAbs(line *Line, wrapper Path) bool {
-	if !wrapper.IsAbs() {
-		return true
-	}
-
-	line.Errorf("Alternative wrapper %q must be relative to PREFIX.", wrapper.String())
-	return false
-}
-
-func (ck *AlternativesChecker) checkWrapperPlist(line *Line, wrapper RelPath,
-	plistFiles map[RelPath]*PlistLine) {
-
-	if plistFiles[wrapper] != nil {
-		line.Errorf("Alternative wrapper %q must not appear in the PLIST.", wrapper)
-	}
-}
-
 func (ck *AlternativesChecker) checkAlternativeAbs(alternative string, line *Line, space string) {
 	lex := textproc.NewLexer(alternative)
 
diff --git a/pkgtools/pkglint/files/alternatives_test.go b/pkgtools/pkglint/files/alternatives_test.go
index eeda4d183de..a6733de5e38 100644
--- a/pkgtools/pkglint/files/alternatives_test.go
+++ b/pkgtools/pkglint/files/alternatives_test.go
@@ -71,6 +71,8 @@ func (s *Suite) Test_AlternativesChecker_Check__PLIST(c *check.C) {
 		"ERROR: ALTERNATIVES:5: Invalid line \"invalid\".",
 		"ERROR: ALTERNATIVES:6: Alternative implementation \"${PREFIX}/bin/firefox\" must appear in the PLIST.",
 		"ERROR: ALTERNATIVES:6: Alternative implementation \"${PREFIX}/bin/firefox\" must be an absolute path.",
+		"ERROR: ALTERNATIVES:7: Alternative wrapper \"highscores\" "+
+			"must be in \"bin\", \"@PKGMANDIR@\" or \"sbin\".",
 		"ERROR: ALTERNATIVES:7: Alternative implementation \"@VARBASE@/game/scores\" "+
 			"must appear in the PLIST as \"${VARBASE}/game/scores\".")
 
@@ -107,7 +109,7 @@ func (s *Suite) Test_AlternativesChecker_checkLine(c *check.C) {
 			"must be relative to PREFIX.")
 }
 
-func (s *Suite) Test_AlternativesChecker_checkWrapperAbs(c *check.C) {
+func (s *Suite) Test_AlternativesChecker_checkLine__absolute(c *check.C) {
 	t := s.Init(c)
 
 	t.CreateFileLines("ALTERNATIVES",
@@ -117,11 +119,13 @@ func (s *Suite) Test_AlternativesChecker_checkWrapperAbs(c *check.C) {
 	CheckFileAlternatives(t.File("ALTERNATIVES"), nil)
 
 	t.CheckOutputLines(
-		"ERROR: ~/ALTERNATIVES:2: Alternative wrapper \"/absolute\" " +
+		"ERROR: ~/ALTERNATIVES:1: Alternative wrapper \"relative\" "+
+			"must be in \"bin\", \"@PKGMANDIR@\" or \"sbin\".",
+		"ERROR: ~/ALTERNATIVES:2: Alternative wrapper \"/absolute\" "+
 			"must be relative to PREFIX.")
 }
 
-func (s *Suite) Test_AlternativesChecker_checkWrapperPlist(c *check.C) {
+func (s *Suite) Test_AlternativesChecker_checkLine__PLIST(c *check.C) {
 	t := s.Init(c)
 
 	t.SetUpPackage("category/package")
@@ -143,6 +147,26 @@ func (s *Suite) Test_AlternativesChecker_checkWrapperPlist(c *check.C) {
 			"must not appear in the PLIST.")
 }
 
+func (s *Suite) Test_AlternativesChecker_checkLine__dir(c *check.C) {
+	t := s.Init(c)
+
+	t.CreateFileLines("ALTERNATIVES",
+		"in/typo @PREFIX@/bin/typo",
+		"sbin/daemon @PREFIX@/sbin/daemon-impl",
+		"typo/program @PREFIX@/typo/program-impl",
+		"man/man1/program.1 @PREFIX@/man/man1/program-impl.1")
+
+	CheckFileAlternatives(t.File("ALTERNATIVES"), nil)
+
+	t.CheckOutputLines(
+		"ERROR: ~/ALTERNATIVES:1: Alternative wrapper \"in/typo\" "+
+			"must be in \"bin\", \"@PKGMANDIR@\" or \"sbin\".",
+		"ERROR: ~/ALTERNATIVES:3: Alternative wrapper \"typo/program\" "+
+			"must be in \"bin\", \"@PKGMANDIR@\" or \"sbin\".",
+		"ERROR: ~/ALTERNATIVES:4: Alternative wrapper \"man/man1/program.1\" "+
+			"must be in \"bin\", \"@PKGMANDIR@\" or \"sbin\".")
+}
+
 func (s *Suite) Test_AlternativesChecker_checkAlternativeAbs(c *check.C) {
 	t := s.Init(c)
 
diff --git a/pkgtools/pkglint/files/autofix.go b/pkgtools/pkglint/files/autofix.go
index 6522bc92d75..6755a329c54 100644
--- a/pkgtools/pkglint/files/autofix.go
+++ b/pkgtools/pkglint/files/autofix.go
@@ -279,9 +279,14 @@ func (fix *Autofix) Describef(rawIndex int, format string, args ...interface{})
 // MkLines.Check.
 func (fix *Autofix) Apply() {
 	// XXX: Make the following annotations actually do something.
+	// Their intention is to insert the following conditions around each
+	// function or method call expression in this function, ensuring that this
+	// function is properly covered in all 3 of pkglint's autofix modes.
+	//
 	// gobco:beforeCall:!G.Opts.ShowAutofix && !G.Opts.Autofix
 	// gobco:beforeCall:G.Opts.ShowAutofix
 	// gobco:beforeCall:G.Opts.Autofix
+	//
 	// See https://github.com/rillig/gobco
 
 	line := fix.line
diff --git a/pkgtools/pkglint/files/check_test.go b/pkgtools/pkglint/files/check_test.go
index 79ee97acad2..351ffac0c6b 100644
--- a/pkgtools/pkglint/files/check_test.go
+++ b/pkgtools/pkglint/files/check_test.go
@@ -233,7 +233,7 @@ func (t *Tester) SetUpVartypes() {
 
 func (t *Tester) SetUpMasterSite(varname string, urls ...string) {
 	if !G.Pkgsrc.vartypes.IsDefinedExact(varname) {
-		t.SetUpType(varname, BtFetchURL,
+		t.SetUpVarType(varname, BtFetchURL,
 			List|SystemProvided,
 			"buildlink3.mk: none",
 			"*: use")
@@ -255,13 +255,13 @@ func (t *Tester) SetUpTool(name, varname string, validity Validity) *Tool {
 	return G.Pkgsrc.Tools.def(name, varname, false, validity, nil)
 }
 
-// SetUpType defines a variable to have a certain type and access permissions,
-// like in the type definitions in vardefs.go.
+// SetUpVarType registers the type and access permissions for a variable, like
+// in the variable definitions in vardefs.go.
 //
 // Example:
-//  SetUpType("PKGPATH", BtPkgpath, DefinedIfInScope|NonemptyIfDefined,
+//  SetUpVarType("PKGPATH", BtPkgpath, DefinedIfInScope|NonemptyIfDefined,
 //      "Makefile, *.mk: default, set, append, use, use-loadtime")
-func (t *Tester) SetUpType(varname string, basicType *BasicType,
+func (t *Tester) SetUpVarType(varname string, basicType *BasicType,
 	options vartypeOptions, aclEntries ...string) {
 
 	if len(aclEntries) == 0 {
diff --git a/pkgtools/pkglint/files/mkassignchecker_test.go b/pkgtools/pkglint/files/mkassignchecker_test.go
index 6c937b81f0c..40c8518bf3c 100644
--- a/pkgtools/pkglint/files/mkassignchecker_test.go
+++ b/pkgtools/pkglint/files/mkassignchecker_test.go
@@ -457,9 +457,9 @@ func (s *Suite) Test_MkAssignChecker_checkLeftPermissions(c *check.C) {
 
 	t.SetUpVartypes()
 	t.SetUpTool("awk", "AWK", AtRunTime)
-	t.SetUpType("SET_ONLY", BtUnknown, NoVartypeOptions,
+	t.SetUpVarType("SET_ONLY", BtUnknown, NoVartypeOptions,
 		"options.mk: set")
-	t.SetUpType("SET_ONLY_DEFAULT_ELSEWHERE", BtUnknown, NoVartypeOptions,
+	t.SetUpVarType("SET_ONLY_DEFAULT_ELSEWHERE", BtUnknown, NoVartypeOptions,
 		"options.mk: set",
 		"*.mk: default, set")
 	mklines := t.NewMkLines("options.mk",
diff --git a/pkgtools/pkglint/files/mkcondchecker_test.go b/pkgtools/pkglint/files/mkcondchecker_test.go
index 331e5a9c64c..7cdaaf1b2cf 100644
--- a/pkgtools/pkglint/files/mkcondchecker_test.go
+++ b/pkgtools/pkglint/files/mkcondchecker_test.go
@@ -560,17 +560,17 @@ func (s *Suite) Test_MkCondChecker_simplify(c *check.C) {
 	// Even when they are in scope, some variables such as PKGREVISION
 	// or MAKE_JOBS may be undefined.
 
-	t.SetUpType("IN_SCOPE_DEFINED", btAnything, AlwaysInScope|DefinedIfInScope,
+	t.SetUpVarType("IN_SCOPE_DEFINED", btAnything, AlwaysInScope|DefinedIfInScope,
 		"*.mk: use, use-loadtime")
-	t.SetUpType("IN_SCOPE", btAnything, AlwaysInScope,
+	t.SetUpVarType("IN_SCOPE", btAnything, AlwaysInScope,
 		"*.mk: use, use-loadtime")
-	t.SetUpType("PREFS_DEFINED", btAnything, DefinedIfInScope,
+	t.SetUpVarType("PREFS_DEFINED", btAnything, DefinedIfInScope,
 		"*.mk: use, use-loadtime")
-	t.SetUpType("PREFS", btAnything, NoVartypeOptions,
+	t.SetUpVarType("PREFS", btAnything, NoVartypeOptions,
 		"*.mk: use, use-loadtime")
-	t.SetUpType("LATER_DEFINED", btAnything, DefinedIfInScope,
+	t.SetUpVarType("LATER_DEFINED", btAnything, DefinedIfInScope,
 		"*.mk: use")
-	t.SetUpType("LATER", btAnything, NoVartypeOptions,
+	t.SetUpVarType("LATER", btAnything, NoVartypeOptions,
 		"*.mk: use")
 	// UNDEFINED is also used in the following tests, but is obviously
 	// not defined here.
@@ -1132,6 +1132,33 @@ func (s *Suite) Test_MkCondChecker_simplify(c *check.C) {
 		".if ${IN_SCOPE_DEFINED:M\"}",
 
 		nil...)
+
+	// FIXME: Syntax error in the generated code.
+	testBeforeAndAfterPrefs(
+		".if !empty(IN_SCOPE_DEFINED:M)",
+		".if ${IN_SCOPE_DEFINED} == ",
+
+		"NOTE: filename.mk:3: IN_SCOPE_DEFINED can be "+
+			"compared using the simpler "+"\"${IN_SCOPE_DEFINED} == \" "+
+			"instead of matching against \":M\".",
+		"AUTOFIX: filename.mk:3: "+
+			"Replacing \"!empty(IN_SCOPE_DEFINED:M)\" "+
+			"with \"${IN_SCOPE_DEFINED} == \".",
+	)
+
+	// TODO: Suggest the simpler '${IN_SCOPE_DEFINED:M*.c}'.
+	testBeforeAndAfterPrefs(
+		".if !empty(IN_SCOPE_DEFINED:M*.c)",
+		".if !empty(IN_SCOPE_DEFINED:M*.c)",
+
+		nil...)
+
+	// TODO: Suggest the simpler '!${IN_SCOPE_DEFINED:M*.c}'.
+	testBeforeAndAfterPrefs(
+		".if empty(IN_SCOPE_DEFINED:M*.c)",
+		".if empty(IN_SCOPE_DEFINED:M*.c)",
+
+		nil...)
 }
 
 func (s *Suite) Test_MkCondChecker_simplify__defined_in_same_file(c *check.C) {
diff --git a/pkgtools/pkglint/files/mkline.go b/pkgtools/pkglint/files/mkline.go
index e667c68050e..a8d504549d9 100644
--- a/pkgtools/pkglint/files/mkline.go
+++ b/pkgtools/pkglint/files/mkline.go
@@ -83,7 +83,7 @@ func (mkline *MkLine) HasComment() bool { return mkline.splitResult.hasComment }
 // and HOMEPAGE using http instead of https.
 //
 // To qualify as a rationale, the comment must contain any of the given
-// keywords. If no keywords are given, any comment qualifies.
+// keywords. If no keywords are given, any nonempty comment qualifies.
 func (mkline *MkLine) HasRationale(keywords ...string) bool {
 	rationale := mkline.splitResult.rationale
 	if rationale == "" {
@@ -116,8 +116,9 @@ func (mkline *MkLine) HasRationale(keywords ...string) bool {
 // entirely, they still count as variable assignments, which means that
 // their comment is the one after the value, if any.
 //
-// Shell commands (lines that start with a tab) cannot have comments, as
-// the # characters are passed uninterpreted to the shell.
+// In shell commands (lines that start with a tab), comments can only start at
+// the beginning of a line, as the first non-whitespace character. Any later
+// '#' is passed uninterpreted to the shell.
 //
 // Example:
 //  VAR=value # comment
@@ -166,8 +167,9 @@ func (mkline *MkLine) IsVarassignMaybeCommented() bool {
 }
 
 // IsShellCommand returns true for tab-indented lines that are assigned to a Make
-// target. Example:
+// target.
 //
+// Example:
 //  pre-configure:    # IsDependency
 //          ${ECHO}   # IsShellCommand
 func (mkline *MkLine) IsShellCommand() bool {
diff --git a/pkgtools/pkglint/files/mkshparser.go b/pkgtools/pkglint/files/mkshparser.go
index 145473d1b47..060839f32ba 100644
--- a/pkgtools/pkglint/files/mkshparser.go
+++ b/pkgtools/pkglint/files/mkshparser.go
@@ -82,7 +82,7 @@ func (lex *ShellLexer) Lex(lval *shyySymType) (ttype int) {
 				trace.Stepf("lex EOF because of a comment")
 				return
 			}
-			tname := shyyTokname(shyyTok2[ttype-shyyPrivate])
+			tname := shyyTokname(int(shyyTok2[ttype-shyyPrivate]))
 			switch ttype {
 			case tkWORD, tkASSIGNMENT_WORD:
 				trace.Stepf("lex %v %q", tname, lval.Word.MkText)
diff --git a/pkgtools/pkglint/files/mkvarusechecker_test.go b/pkgtools/pkglint/files/mkvarusechecker_test.go
index bf504733b80..5a490d86c24 100644
--- a/pkgtools/pkglint/files/mkvarusechecker_test.go
+++ b/pkgtools/pkglint/files/mkvarusechecker_test.go
@@ -670,9 +670,9 @@ func (s *Suite) Test_MkVarUseChecker_checkPermissions__load_time_in_condition(c
 	t := s.Init(c)
 
 	t.SetUpPkgsrc()
-	t.SetUpType("LOAD_TIME", BtPathPattern, List,
+	t.SetUpVarType("LOAD_TIME", BtPathPattern, List,
 		"special:filename.mk: use-loadtime")
-	t.SetUpType("RUN_TIME", BtPathPattern, List,
+	t.SetUpVarType("RUN_TIME", BtPathPattern, List,
 		"special:filename.mk: use")
 	t.Chdir(".")
 	t.FinishSetUp()
@@ -694,9 +694,9 @@ func (s *Suite) Test_MkVarUseChecker_checkPermissions__load_time_in_for_loop(c *
 	t := s.Init(c)
 
 	t.SetUpPkgsrc()
-	t.SetUpType("LOAD_TIME", BtPathPattern, List,
+	t.SetUpVarType("LOAD_TIME", BtPathPattern, List,
 		"special:filename.mk: use-loadtime")
-	t.SetUpType("RUN_TIME", BtPathPattern, List,
+	t.SetUpVarType("RUN_TIME", BtPathPattern, List,
 		"special:filename.mk: use")
 	t.Chdir(".")
 	t.FinishSetUp()
@@ -747,16 +747,16 @@ func (s *Suite) Test_MkVarUseChecker_checkPermissions__load_time_run_time(c *che
 	t := s.Init(c)
 
 	t.SetUpPkgsrc()
-	t.SetUpType("LOAD_TIME", BtUnknown, NoVartypeOptions,
+	t.SetUpVarType("LOAD_TIME", BtUnknown, NoVartypeOptions,
 		"*.mk: use, use-loadtime")
-	t.SetUpType("RUN_TIME", BtUnknown, NoVartypeOptions,
+	t.SetUpVarType("RUN_TIME", BtUnknown, NoVartypeOptions,
 		"*.mk: use")
-	t.SetUpType("WRITE_ONLY", BtUnknown, NoVartypeOptions,
+	t.SetUpVarType("WRITE_ONLY", BtUnknown, NoVartypeOptions,
 		"*.mk: set")
-	t.SetUpType("LOAD_TIME_ELSEWHERE", BtUnknown, NoVartypeOptions,
+	t.SetUpVarType("LOAD_TIME_ELSEWHERE", BtUnknown, NoVartypeOptions,
 		"Makefile: use-loadtime",
 		"*.mk: set")
-	t.SetUpType("RUN_TIME_ELSEWHERE", BtUnknown, NoVartypeOptions,
+	t.SetUpVarType("RUN_TIME_ELSEWHERE", BtUnknown, NoVartypeOptions,
 		"Makefile: use",
 		"*.mk: set")
 	t.Chdir(".")
@@ -852,7 +852,7 @@ func (s *Suite) Test_MkVarUseChecker_checkPermissions__write_only_usable_in_othe
 func (s *Suite) Test_MkVarUseChecker_checkPermissions__usable_only_at_loadtime_in_other_file(c *check.C) {
 	t := s.Init(c)
 
-	t.SetUpType("VAR", BtFilename, NoVartypeOptions,
+	t.SetUpVarType("VAR", BtFilename, NoVartypeOptions,
 		"*: set, use-loadtime")
 	mklines := t.NewMkLines("Makefile",
 		MkCvsID,
@@ -871,8 +871,8 @@ func (s *Suite) Test_MkVarUseChecker_checkPermissions__assigned_to_infrastructur
 
 	// This combination of BtUnknown and all permissions is typical for
 	// otherwise unknown variables from the pkgsrc infrastructure.
-	t.SetUpType("INFRA", BtUnknown, NoVartypeOptions)
-	t.SetUpType("VAR", BtUnknown, NoVartypeOptions,
+	t.SetUpVarType("INFRA", BtUnknown, NoVartypeOptions)
+	t.SetUpVarType("VAR", BtUnknown, NoVartypeOptions,
 		"buildlink3.mk: none",
 		"*: use")
 	mklines := t.NewMkLines("buildlink3.mk",
@@ -906,10 +906,10 @@ func (s *Suite) Test_MkVarUseChecker_checkPermissions__assigned_to_load_time(c *
 	// to use its value in LOAD_TIME, as the latter might be evaluated later
 	// at load time, and at that point VAR would be evaluated as well.
 
-	t.SetUpType("LOAD_TIME", BtMessage, NoVartypeOptions,
+	t.SetUpVarType("LOAD_TIME", BtMessage, NoVartypeOptions,
 		"buildlink3.mk: set",
 		"*.mk: use-loadtime")
-	t.SetUpType("VAR", BtUnknown, NoVartypeOptions,
+	t.SetUpVarType("VAR", BtUnknown, NoVartypeOptions,
 		"buildlink3.mk: none",
 		"*.mk: use")
 	mklines := t.NewMkLines("buildlink3.mk",
@@ -1090,7 +1090,7 @@ func (s *Suite) Test_MkVarUseChecker_checkUseAtLoadTime__package_settable(c *che
 	t := s.Init(c)
 
 	btAnything := &BasicType{"Anything", func(cv *VartypeCheck) {}}
-	t.SetUpType("PKG", btAnything, PackageSettable)
+	t.SetUpVarType("PKG", btAnything, PackageSettable)
 	t.Chdir("category/package")
 
 	test := func(filename CurrPath, diagnostics ...string) {
diff --git a/pkgtools/pkglint/files/vartypecheck_test.go b/pkgtools/pkglint/files/vartypecheck_test.go
index e9cee078e33..c0e5fc8affa 100644
--- a/pkgtools/pkglint/files/vartypecheck_test.go
+++ b/pkgtools/pkglint/files/vartypecheck_test.go
@@ -1621,7 +1621,7 @@ func (s *Suite) Test_VartypeCheck_PathnameSpace(c *check.C) {
 	t := s.Init(c)
 	// Invent a variable name since this data type is only used as part
 	// of CONF_FILES.
-	t.SetUpType("CONFIG_FILE", BtPathnameSpace,
+	t.SetUpVarType("CONFIG_FILE", BtPathnameSpace,
 		NoVartypeOptions, "*.mk: set, use")
 	vt := NewVartypeCheckTester(t, BtPathnameSpace)