pkgtools/pkglint: update to 22.3.0

Changes since 22.2.7:

Pkglint no longer wrongly warns about package options from optional or
required option groups. Fixes PR 57038.

9 files changed, 96 insertions, 17 deletions

diff --git a/pkgtools/pkglint/Makefile b/pkgtools/pkglint/Makefile
index f62f6cbaf58..53e3373b5b4 100644
--- a/pkgtools/pkglint/Makefile
+++ b/pkgtools/pkglint/Makefile
@@ -1,7 +1,6 @@
-# $NetBSD: Makefile,v 1.730 2022/09/26 18:40:18 bsiegert Exp $
+# $NetBSD: Makefile,v 1.731 2022/10/02 14:39:36 rillig Exp $
 
-PKGNAME=	pkglint-22.2.7
-PKGREVISION=	1
+PKGNAME=	pkglint-22.3.0
 CATEGORIES=	pkgtools
 
 MAINTAINER=	rillig@NetBSD.org
diff --git a/pkgtools/pkglint/files/mklines.go b/pkgtools/pkglint/files/mklines.go
index 977471c217e..5826794e619 100644
--- a/pkgtools/pkglint/files/mklines.go
+++ b/pkgtools/pkglint/files/mklines.go
@@ -368,6 +368,22 @@ func (mklines *MkLines) defineVar(mkline *MkLine, varname string) {
 func (mklines *MkLines) collectPlistVars() {
 	// TODO: The PLIST_VARS code above looks very similar.
 	for _, mkline := range mklines.mklines {
+
+		// In options.mk, add option groups to PKG_SUPPORTED_OPTIONS.
+		if mkline.IsInclude() && mkline.IncludedFile() == "../../mk/bsd.options.mk" {
+			mklines.allVars.def("PKG_SUPPORTED_OPTIONS", mkline)
+			supported := mklines.allVars.vs["PKG_SUPPORTED_OPTIONS"]
+			optional := mklines.allVars.LastValue("PKG_OPTIONS_OPTIONAL_GROUPS")
+			required := mklines.allVars.LastValue("PKG_OPTIONS_REQUIRED_GROUPS")
+
+			for _, opt := range mkline.ValueFields(optional) {
+				supported.value += mklines.allVars.LastValue("PKG_OPTIONS_GROUP." + opt)
+			}
+			for _, opt := range mkline.ValueFields(required) {
+				supported.value += mklines.allVars.LastValue("PKG_OPTIONS_GROUP." + opt)
+			}
+		}
+
 		if mkline.IsVarassign() {
 			switch mkline.Varcanon() {
 			case "PLIST_VARS":
diff --git a/pkgtools/pkglint/files/options_test.go b/pkgtools/pkglint/files/options_test.go
index f39e0c5d843..b86f46aa398 100755
--- a/pkgtools/pkglint/files/options_test.go
+++ b/pkgtools/pkglint/files/options_test.go
@@ -423,6 +423,69 @@ func (s *Suite) Test_CheckLinesOptionsMk__PLIST_VARS_based_on_PKG_SUPPORTED_OPTI
 		"WARN: options.mk:5: Option \"two\" should be handled below in an .if block.")
 }
 
+// https://gnats.netbsd.org/57038
+func (s *Suite) Test_CheckLinesOptionsMk__PLIST_VARS_based_on_groups(c *check.C) {
+	t := s.Init(c)
+
+	t.SetUpOption("one", "")
+	t.SetUpOption("two", "")
+	t.SetUpOption("opt-one", "")
+	t.SetUpOption("opt-two", "")
+	t.SetUpOption("req-one", "")
+	t.SetUpOption("req-two", "")
+	t.SetUpPackage("category/package")
+	t.CreateFileLines("mk/bsd.options.mk")
+	t.SetUpFileMkLines("category/package/options.mk",
+		MkCvsID,
+		"",
+		"PKG_OPTIONS_VAR=\tPKG_OPTIONS.package",
+		"PKG_SUPPORTED_OPTIONS+=\tone two",
+		"",
+		"PKG_OPTIONS_OPTIONAL_GROUPS+=\topt",
+		"PKG_OPTIONS_GROUP.opt+=\t\topt-one opt-two",
+		"",
+		"PKG_OPTIONS_REQUIRED_GROUPS+=\treq",
+		"PKG_OPTIONS_GROUP.req+=\t\treq-one req-two",
+		"",
+		// bsd.options.mk adds the options from the groups to
+		// PKG_SUPPORTED_OPTIONS. So does MkLines.collectPlistVars.
+		".include \"../../mk/bsd.options.mk\"",
+		"",
+		// All 6 options are added at this point.
+		"PLIST_VARS+=\t${PKG_SUPPORTED_OPTIONS}",
+		"",
+		// Only the 'one' options are covered properly, the 'two'
+		// options produce warnings, to demonstrate that all cases of
+		// option groups are covered.
+		".if ${PKG_OPTIONS:Mone}",
+		"PLIST.one=\tyes",
+		".endif",
+		".if ${PKG_OPTIONS:Mopt-one}",
+		"PLIST.opt-one=\tyes",
+		".endif",
+		".if ${PKG_OPTIONS:Mreq-one}",
+		"PLIST.req-one=\tyes",
+		".endif")
+	t.Chdir("category/package")
+	t.FinishSetUp()
+
+	G.Check(".")
+
+	t.CheckOutputLines(
+		"WARN: options.mk:14: \"two\" is added to PLIST_VARS, "+
+			"but PLIST.two is not defined in this file.",
+		"WARN: options.mk:14: \"opt-two\" is added to PLIST_VARS, "+
+			"but PLIST.opt-two is not defined in this file.",
+		"WARN: options.mk:14: \"req-two\" is added to PLIST_VARS, "+
+			"but PLIST.req-two is not defined in this file.",
+		"WARN: options.mk:4: Option \"two\" "+
+			"should be handled below in an .if block.",
+		"WARN: options.mk:7: Option \"opt-two\" "+
+			"should be handled below in an .if block.",
+		"WARN: options.mk:10: Option \"req-two\" "+
+			"should be handled below in an .if block.")
+}
+
 // A few packages (such as www/w3m) define several options that are
 // handled by a single .if block in the lower part.
 func (s *Suite) Test_CheckLinesOptionsMk__combined_option_handling(c *check.C) {
diff --git a/pkgtools/pkglint/files/pkgsrc.go b/pkgtools/pkglint/files/pkgsrc.go
index 8b9ff639773..f36714e1d55 100644
--- a/pkgtools/pkglint/files/pkgsrc.go
+++ b/pkgtools/pkglint/files/pkgsrc.go
@@ -438,7 +438,7 @@ func (src *Pkgsrc) checkRemovedAfterLastFreeze() {
 
 	for _, change := range wrong {
 		// The original line of the change is not available anymore.
-		// Therefore it is necessary to load the whole file again.
+		// Therefore, it is necessary to load the whole file again.
 		lines := Load(change.Location.Filename, MustSucceed)
 		line := lines.Lines[change.Location.lineno-1]
 		line.Errorf("Package %s must either exist or be marked as removed.", change.Pkgpath.String())
@@ -1127,7 +1127,7 @@ func (src *Pkgsrc) guessVariableType(varname string) (vartype *Vartype) {
 	// looked up from the pkgsrc infrastructure.
 	//
 	// As of May 2019, pkglint only distinguishes plain variables and
-	// list variables, but not "unknown". Therefore the above patterns
+	// list variables, but not "unknown". Therefore, the above patterns
 	// must take precedence over this rule, because otherwise, list
 	// variables from the infrastructure would be guessed to be plain
 	// variables.
diff --git a/pkgtools/pkglint/files/substcontext.go b/pkgtools/pkglint/files/substcontext.go
index 84a807a772a..20922739c31 100644
--- a/pkgtools/pkglint/files/substcontext.go
+++ b/pkgtools/pkglint/files/substcontext.go
@@ -526,7 +526,7 @@ func (b *substBlock) suggestSubstVars(mkline *MkLine) {
 		fix.Apply()
 
 		// At this point the number of SUBST_SED assignments is one
-		// less than before. Therefore it is possible to adjust the
+		// less than before. Therefore, it is possible to adjust the
 		// assignment operators on them. It's probably not worth the
 		// effort, though.
 
@@ -607,7 +607,8 @@ func (*substBlock) extractVarname(token string) string {
 }
 
 func (b *substBlock) isComplete() bool {
-	return b.allSeen().hasAll(ssStage | ssFiles | ssTransform)
+	seen := b.allSeen()
+	return seen.hasAll(ssStage | ssFiles | ssTransform)
 }
 
 func (b *substBlock) hasSeen(part substSeen) bool {
@@ -789,6 +790,6 @@ func (s *substSeen) has(part substSeen) bool {
 	return *s&part != 0
 }
 
-func (s substSeen) hasAll(other substSeen) bool { return s&other == other }
-func (s *substSeen) addAll(other substSeen)     { *s |= other }
-func (s *substSeen) retainAll(other substSeen)  { *s &= other }
+func (s *substSeen) hasAll(other substSeen) bool { return *s&other == other }
+func (s *substSeen) addAll(other substSeen)      { *s |= other }
+func (s *substSeen) retainAll(other substSeen)   { *s &= other }
diff --git a/pkgtools/pkglint/files/tools.go b/pkgtools/pkglint/files/tools.go
index 20834aa2940..74b22b3a245 100644
--- a/pkgtools/pkglint/files/tools.go
+++ b/pkgtools/pkglint/files/tools.go
@@ -14,7 +14,7 @@ type Tool struct {
 	Name    string // e.g. "sed", "gzip"
 	Varname string // e.g. "SED", "GZIP_CMD"
 
-	// Some of the very simple tools (echo, printf, test) differ in their implementations.
+	// Some basic tools (echo, printf, test) differ in their implementations.
 	//
 	// When bmake encounters a "simple" command line, it bypasses the
 	// call to a shell (see devel/bmake/files/compat.c:/useShell/).
diff --git a/pkgtools/pkglint/files/util.go b/pkgtools/pkglint/files/util.go
index 1cbf90852aa..d28e79aa8f6 100644
--- a/pkgtools/pkglint/files/util.go
+++ b/pkgtools/pkglint/files/util.go
@@ -275,7 +275,7 @@ func assertNotNil(obj interface{}) {
 	}
 }
 
-// assert checks that the condition is true. Otherwise it terminates the
+// assert checks that the condition is true. Otherwise, it terminates the
 // process with a fatal error message, prefixed with "Pkglint internal error".
 //
 // This method must only be used for programming errors.
@@ -286,7 +286,7 @@ func assert(cond bool) {
 	}
 }
 
-// assertf checks that the condition is true. Otherwise it terminates the
+// assertf checks that the condition is true. Otherwise, it terminates the
 // process with a fatal error message, prefixed with "Pkglint internal error".
 //
 // This method must only be used for programming errors.
@@ -434,7 +434,7 @@ func alignWith(str, other string) string {
 }
 
 // alignmentTo returns the whitespace that is necessary to
-// bring str to the same width as other.
+// bring str to the same width as the other one.
 func alignmentTo(str, other string) string {
 	strWidth := tabWidth(str)
 	otherWidth := tabWidth(other)
diff --git a/pkgtools/pkglint/files/varalignblock.go b/pkgtools/pkglint/files/varalignblock.go
index 68e77b9a674..114614cfbef 100644
--- a/pkgtools/pkglint/files/varalignblock.go
+++ b/pkgtools/pkglint/files/varalignblock.go
@@ -466,7 +466,7 @@ func (l *varalignMkLine) isMultiEmpty() bool {
 
 // rightMargin calculates the column in which the continuation backslashes
 // should be placed.
-// In addition it returns whether the right margin is already in its
+// In addition, it returns whether the right margin is already in its
 // canonical form.
 func (l *varalignMkLine) rightMargin() (ok bool, margin int) {
 	restIndex := condInt(l.infos[0].value == "", 1, 0)
diff --git a/pkgtools/pkglint/files/vardefs.go b/pkgtools/pkglint/files/vardefs.go
index ffdf1d53d86..d7718b80a1b 100644
--- a/pkgtools/pkglint/files/vardefs.go
+++ b/pkgtools/pkglint/files/vardefs.go
@@ -1926,8 +1926,8 @@ func (reg *VarTypeRegistry) parsePermissions(varname, globs, perms string) ACLPe
 	// more general. Most variables that can be used at load time
 	// can also be used at run time.
 	//
-	// Using a variable at load time is a special access that
-	// applies to fewer variables. Therefore it comes last.
+	// Using a variable at load time occurs rarely, and it
+	// applies to fewer variables. Therefore, it comes last.
 	remove("use", aclpUse)
 	remove("use-loadtime", aclpUseLoadtime)