pkgtools/pkglint: update to 5.5.6

Changes since 5.5.5:

* Only offer explanations if an explainable diagnostic has actually
  been logged.

* Clean up code.

* Improve a few diagnostics.

* In any Makefile, treat documented variables as used. This prevents
  warning about defined but unused variables.

* Add support for some variables not previously known to pkglint.

27 files changed, 840 insertions, 347 deletions

diff --git a/pkgtools/pkglint/Makefile b/pkgtools/pkglint/Makefile
index 14355931c86..74a70a3e7af 100644
--- a/pkgtools/pkglint/Makefile
+++ b/pkgtools/pkglint/Makefile
@@ -1,7 +1,6 @@
-# $NetBSD: Makefile,v 1.530 2018/03/04 15:52:18 bsiegert Exp $
+# $NetBSD: Makefile,v 1.531 2018/03/04 20:34:32 rillig Exp $
 
-PKGNAME=	pkglint-5.5.5
-PKGREVISION=	1
+PKGNAME=	pkglint-5.5.6
 DISTFILES=	# none
 CATEGORIES=	pkgtools
 
diff --git a/pkgtools/pkglint/files/autofix.go b/pkgtools/pkglint/files/autofix.go
index 5a760a1bca3..38b0abcd4cc 100644
--- a/pkgtools/pkglint/files/autofix.go
+++ b/pkgtools/pkglint/files/autofix.go
@@ -227,7 +227,8 @@ func (fix *Autofix) Apply() {
 		return
 	}
 
-	if shallBeLogged(fix.diagFormat) {
+	G.explainNext = shallBeLogged(fix.diagFormat)
+	if G.explainNext {
 		logDiagnostic := fix.level != nil && fix.diagFormat != "Silent-Magic-Diagnostic" &&
 			!(G.opts.Autofix && !G.opts.PrintAutofix) && len(fix.actions) > 0
 		if logDiagnostic {
diff --git a/pkgtools/pkglint/files/check_test.go b/pkgtools/pkglint/files/check_test.go
index dee97a87eb2..42d91404222 100644
--- a/pkgtools/pkglint/files/check_test.go
+++ b/pkgtools/pkglint/files/check_test.go
@@ -45,7 +45,7 @@ func (s *Suite) SetUpTest(c *check.C) {
 	t := &Tester{checkC: c}
 	s.Tester = t
 
-	G = GlobalVars{Testing: true}
+	G = Pkglint{Testing: true}
 	textproc.Testing = true
 	G.logOut = NewSeparatorWriter(&t.stdout)
 	G.logErr = NewSeparatorWriter(&t.stderr)
@@ -62,7 +62,7 @@ func (s *Suite) TearDownTest(c *check.C) {
 	t := s.Tester
 	t.checkC = nil // No longer usable; see https://github.com/go-check/check/issues/22
 
-	G = GlobalVars{}
+	G = Pkglint{} // unusable because of missing logOut and logErr
 	textproc.Testing = false
 	if out := t.Output(); out != "" {
 		fmt.Fprintf(os.Stderr, "Unchecked output in %q; check with: t.CheckOutputLines(%v)",
@@ -96,7 +96,7 @@ func (t *Tester) c() *check.C {
 // SetupCommandLine simulates a command line for the remainder of the test.
 // See Pkglint.ParseCommandLine.
 func (t *Tester) SetupCommandLine(args ...string) {
-	exitcode := new(Pkglint).ParseCommandLine(append([]string{"pkglint"}, args...))
+	exitcode := G.ParseCommandLine(append([]string{"pkglint"}, args...))
 	if exitcode != nil && *exitcode != 0 {
 		t.CheckOutputEmpty()
 		t.c().Fatalf("Cannot parse command line: %#v", args)
diff --git a/pkgtools/pkglint/files/codewalk.md b/pkgtools/pkglint/files/codewalk.md
new file mode 100755
index 00000000000..8258f415e6b
--- /dev/null
+++ b/pkgtools/pkglint/files/codewalk.md
@@ -0,0 +1,212 @@
+# The pkglint tour
+
+> Note: I wish there were a tool for nicely rendering the below codewalk blocks.
+> If you know of such a tool, please tell me.
+> `godoc` codewalks don't count since [they are only supported for the Go distribution 
+> itself](https://github.com/golang/go/issues/14369).
+>
+> See also:
+>
+> * [github/markup/#172](https://github.com/github/markup/issues/172#issuecomment-33241601) (closed)
+> * [MarkdownHelper](https://github.com/BurdetteLamar/MarkdownHelper)
+
+## The entry points
+
+### Running pkglint
+
+```codewalk
+file   pkglint.go
+start  ^func main
+end    ^\}
+```
+
+When running pkglint, the `G` variable is set up first.
+It contains the whole global state of pkglint.
+(Except for some of the subpackages, which have to be initialized separately.)
+All the interesting code is in the `Pkglint` type.
+This code structure makes it easy to test the code.
+
+### Testing pkglint
+
+Very similar code is used to set up the test and tear it down again:
+
+```codewalk
+file   check_test.go
+start  ^func .* SetUpTest
+end    ^\}
+```
+
+```codewalk
+file   check_test.go
+start  ^func .* TearDownTest
+end    ^\}
+```
+
+## First contact: checking a single DESCR file
+
+To learn how pkglint works internally, it is a good idea to start with
+a very simple example.
+Since the `DESCR` files have a very simple structure (they only contain
+text for human consumption), they are the ideal target.
+Let's trace an invocation of the command `pkglint DESCR` down to where
+the actual checks happen.
+
+```codewalk
+file   pkglint.go
+start  func main
+```
+
+```codewalk
+file   pkglint.go
+start  ^[\t]if exitcode
+end    ^\t\}$
+```
+
+Since there are no command line options starting with a hyphen, we can
+skip the command line parsing for this example.
+
+```codewalk
+file   pkglint.go
+start  ^[\t]for _, arg
+end    ^\}
+```
+
+The argument `DESCR` is saved in the `TODO` list, and then the pkgsrc
+infrastructure data is loaded by `Initialize`.
+This must happen in this order because pkglint needs to determine the
+pkgsrc root directory, just in case there are two or more pkgsrc trees
+in the local system.
+The path of the pkgsrc directory is determined from the first command
+line argument, which in this file is `DESCR`. From there, the pkgsrc
+root is usually reachable via `../../`, and this is what pkglint tries.
+
+After the initialization of everything,
+all items from the TODO list are worked off and handed over to `CheckDirent`.
+
+```codewalk
+file   pkglint.go
+start  func CheckDirent
+```
+
+Since `DESCR` is a regular file, the next method to call is `Checkfile`.
+
+```codewalk
+file   pkglint.go
+start  func Checkfile
+```
+
+```codewalk
+file   pkglint.go
+start  /basename, "DESCR"/
+end    ^$
+```
+
+When compared to the code blocks around this one, it looks strange that
+this one uses `hasPrefix` and the others use a direct string comparison.
+But indeed, there are a few packages that actually have `DESCR.common`
+files. So everything's fine here.
+
+At this point, the file is loaded and converted to lines.
+For DESCR files, this is very simple, so there's no need to dive into that.
+The actual checks usually work on `Line` objects instead of files
+because the lines offer nice methods for logging the diagnostics
+and for automatically fixing the text (in pkglint's `--autofix` mode).
+
+```codewalk
+file   pkglint.go
+start  func ChecklinesDescr
+end    ^\}
+```
+
+Now we are where the actual action takes place.
+The code looks straight-forward here.
+First, each line is checked on its own,
+and the final check is for too long files. 
+
+The call to `SaveAutofixChanges` at the end looks a bit strange
+since none of the visible checks fixes anything.
+The autofix feature must be hidden in one of the line checks,
+and indeed, the code for `CheckLineTrailingWhitespace` says:
+
+```codewalk
+file   linechecker.go
+start  ^func CheckLineTrailingWhitespace
+end    ^\}
+```
+
+This code is a typical example for using the autofix feature.
+Some more details are described at the `Autofix` type itself:
+
+```codewalk
+file   linechecker.go
+start  /^type Autofix/ upwhile /^\/\//
+end    /^type Autofix/
+```
+
+The journey ends here, and it hasn't been that difficult.
+If that was too easy, have a look at the complex cases here:
+
+```codewalk
+file   mkline.go
+start  /^func .* VariableNeedsQuoting
+```
+
+## Basic ingredients
+
+Pkglint checks packages, and a package consists of several different files.
+All pkgsrc files are text files, which are organized in lines.
+Most pkglint diagnostics refer to a specific line,
+therefore the `Line` type is responsible for producing the diagnostics.
+
+### Line
+
+Most checks in pkgsrc only need to look at a single line.
+Lines that are independent of the file type are implemented in the `Line` type.
+This type contains the methods `Errorf`, `Warnf` and `Notef` to produce diagnostics
+of the following form:
+
+```text
+WARN: Makefile:3: COMMENT should not start with "A" or "An".
+```
+
+The definition for the `Line` type is:
+
+```codewalk
+file   line.go
+start  ^type Line =
+```
+
+```codewalk
+file   line.go
+start  ^type LineImpl struct
+end    ^\}
+```
+
+### MkLine
+
+```codewalk
+file   mkline.go
+start  ^type MkLine =
+```
+
+```codewalk
+file   mkline.go
+start  ^type MkLineImpl struct
+end    ^\}
+```
+
+Most of the pkgsrc infrastructure is written in Makefiles. 
+In these, there may be line continuations  (the ones ending in backslash).
+Plus, they may contain Make variables of the form `${VARNAME}` or `${VARNAME:Modifiers}`,
+and these are handled specially.
+
+### ShellLine
+
+The instructions for building and installing packages are written in Shell.
+The `ShellLine` type provides methods for checking shell commands and their individual parts.
+
+```codewalk
+file   shell.go
+start  ^type ShellLine struct
+end    ^\}
+```
diff --git a/pkgtools/pkglint/files/globaldata.go b/pkgtools/pkglint/files/globaldata.go
index a8eda72105a..0b4a40acad7 100644
--- a/pkgtools/pkglint/files/globaldata.go
+++ b/pkgtools/pkglint/files/globaldata.go
@@ -44,6 +44,9 @@ type SuggestedUpdate struct {
 	Comment string
 }
 
+// Initialize reads the pkgsrc infrastructure files to
+// extract information like the tools, packages to update,
+// user-defined variables.
 func (gd *GlobalData) Initialize() {
 	firstArg := G.Todo[0]
 	if fileExists(firstArg) {
diff --git a/pkgtools/pkglint/files/globalvars.go b/pkgtools/pkglint/files/globalvars.go
deleted file mode 100644
index 8379fc095ce..00000000000
--- a/pkgtools/pkglint/files/globalvars.go
+++ /dev/null
@@ -1,88 +0,0 @@
-package main
-
-import (
-	"netbsd.org/pkglint/histogram"
-)
-
-type GlobalVars struct {
-	opts       CmdOpts    //
-	globalData GlobalData //
-	Pkg        *Package   // The package that is currently checked.
-	Mk         *MkLines   // The Makefile (or fragment) that is currently checked.
-
-	Todo            []string // The files or directories that still need to be checked.
-	CurrentDir      string   // The currently checked directory, relative to the cwd
-	CurPkgsrcdir    string   // The pkgsrc directory, relative to currentDir
-	Wip             bool     // Is the currently checked directory from pkgsrc-wip?
-	Infrastructure  bool     // Is the currently checked item from the pkgsrc infrastructure?
-	Testing         bool     // Is pkglint in self-testing mode (only during development)?
-	CurrentUsername string   // For checking against OWNER and MAINTAINER
-	CvsEntriesDir   string   // Cached to avoid I/O
-	CvsEntriesLines []Line
-
-	Hash         map[string]*Hash // Maps "alg:fname" => hash (inter-package check).
-	UsedLicenses map[string]bool  // Maps "license name" => true (inter-package check).
-
-	errors                int
-	warnings              int
-	logged                map[string]bool
-	explanationsAvailable bool
-	explanationsGiven     map[string]bool
-	autofixAvailable      bool
-	logOut                *SeparatorWriter
-	logErr                *SeparatorWriter
-
-	loghisto *histogram.Histogram
-}
-
-type CmdOpts struct {
-	CheckAlternatives,
-	CheckBuildlink3,
-	CheckDescr,
-	CheckDistinfo,
-	CheckExtra,
-	CheckGlobal,
-	CheckInstall,
-	CheckMakefile,
-	CheckMessage,
-	CheckMk,
-	CheckPatches,
-	CheckPlist bool
-
-	WarnAbsname,
-	WarnDirectcmd,
-	WarnExtra,
-	WarnOrder,
-	WarnPerm,
-	WarnPlistDepr,
-	WarnPlistSort,
-	WarnQuoting,
-	WarnSpace,
-	WarnStyle,
-	WarnTypes bool
-
-	Explain,
-	Autofix,
-	GccOutput,
-	PrintHelp,
-	DumpMakefile,
-	Import,
-	LogVerbose,
-	Profiling,
-	Quiet,
-	Recursive,
-	PrintAutofix,
-	PrintSource,
-	PrintVersion bool
-
-	LogOnly []string
-
-	args []string
-}
-
-type Hash struct {
-	hash string
-	line Line
-}
-
-var G GlobalVars
diff --git a/pkgtools/pkglint/files/line.go b/pkgtools/pkglint/files/line.go
index 6c83b34464c..2b01bbac0f8 100644
--- a/pkgtools/pkglint/files/line.go
+++ b/pkgtools/pkglint/files/line.go
@@ -123,7 +123,8 @@ func (line *LineImpl) log(level *LogLevel, format string, args []interface{}) {
 		// These are logged by Autofix.Apply.
 		return
 	}
-	if !shallBeLogged(format) {
+	G.explainNext = shallBeLogged(format)
+	if !G.explainNext {
 		return
 	}
 
diff --git a/pkgtools/pkglint/files/logging.go b/pkgtools/pkglint/files/logging.go
index f98d3f870d9..27da2c3d697 100644
--- a/pkgtools/pkglint/files/logging.go
+++ b/pkgtools/pkglint/files/logging.go
@@ -101,23 +101,6 @@ func logs(level *LogLevel, fname, lineno, format, msg string) bool {
 }
 
 func Explain(explanation ...string) {
-	if G.opts.Explain {
-		complete := strings.Join(explanation, "\n")
-		if G.explanationsGiven[complete] {
-			return
-		}
-		if G.explanationsGiven == nil {
-			G.explanationsGiven = make(map[string]bool)
-		}
-		G.explanationsGiven[complete] = true
-
-		G.logOut.WriteLine("")
-		for _, explanationLine := range explanation {
-			G.logOut.WriteLine("\t" + explanationLine)
-		}
-		G.logOut.WriteLine("")
-	}
-
 	if G.Testing {
 		for _, s := range explanation {
 			if l := tabWidth(s); l > 68 && contains(s, " ") {
@@ -131,7 +114,30 @@ func Explain(explanation ...string) {
 			}
 		}
 	}
+
+	if !G.explainNext {
+		return
+	}
 	G.explanationsAvailable = true
+	if !G.opts.Explain {
+		return
+	}
+
+	complete := strings.Join(explanation, "\n")
+	if G.explanationsGiven[complete] {
+		return
+	}
+	if G.explanationsGiven == nil {
+		G.explanationsGiven = make(map[string]bool)
+	}
+	G.explanationsGiven[complete] = true
+
+	G.logOut.WriteLine("")
+	for _, explanationLine := range explanation {
+		G.logOut.WriteLine("\t" + explanationLine)
+	}
+	G.logOut.WriteLine("")
+
 }
 
 type pkglintFatal struct{}
diff --git a/pkgtools/pkglint/files/logging_test.go b/pkgtools/pkglint/files/logging_test.go
index c893690ec4a..28fd030fed4 100644
--- a/pkgtools/pkglint/files/logging_test.go
+++ b/pkgtools/pkglint/files/logging_test.go
@@ -135,3 +135,47 @@ func (s *Suite) Test_Line_log_only(c *check.C) {
 		"-\tThe old song",
 		"+\tThe new2 song")
 }
+
+func (s *Suite) Test_collect_explanations_with_only(c *check.C) {
+	t := s.Init(c)
+
+	t.SetupCommandLine("--only", "interesting")
+	line := t.NewLine("Makefile", 27, "The old song")
+
+	line.Warnf("Filtered warning.")               // Is not logged.
+	Explain("Explanation for the above warning.") // Neither would this explanation be logged.
+	G.PrintSummary()
+
+	c.Check(G.explanationsAvailable, equals, false)
+	t.CheckOutputLines(
+		"Looks fine.") // "pkglint -e" is not advertised since the above explanation is not relevant.
+
+	line.Warnf("What an interesting line.")
+	Explain("This explanation is available.")
+	G.PrintSummary()
+
+	c.Check(G.explanationsAvailable, equals, true)
+	t.CheckOutputLines(
+		"WARN: Makefile:27: What an interesting line.",
+		"0 errors and 1 warning found.",
+		"(Run \"pkglint -e\" to show explanations.)")
+}
+
+func (s *Suite) Test_explain_with_only(c *check.C) {
+	t := s.Init(c)
+
+	t.SetupCommandLine("--only", "interesting", "--explain")
+	line := t.NewLine("Makefile", 27, "The old song")
+
+	line.Warnf("Filtered warning.")               // Is not logged.
+	Explain("Explanation for the above warning.") // Neither is this explanation logged.
+
+	line.Warnf("What an interesting line.")
+	Explain("This explanation is logged.")
+
+	t.CheckOutputLines(
+		"WARN: Makefile:27: What an interesting line.",
+		"",
+		"\tThis explanation is logged.",
+		"")
+}
diff --git a/pkgtools/pkglint/files/mkline.go b/pkgtools/pkglint/files/mkline.go
index a1724fbea40..59cba2292f4 100644
--- a/pkgtools/pkglint/files/mkline.go
+++ b/pkgtools/pkglint/files/mkline.go
@@ -9,6 +9,10 @@ import (
 	"strings"
 )
 
+// MkLine is a line from a Makefile fragment.
+// There are several types of lines.
+// The most common types in pkgsrc are variable assignments,
+// shell commands and preprocessor instructions.
 type MkLine = *MkLineImpl
 
 type MkLineImpl struct {
@@ -47,13 +51,11 @@ type mkLineDependency struct {
 	sources string
 }
 
-func NewMkLine(line Line) (mkline *MkLineImpl) {
-	mkline = &MkLineImpl{Line: line}
-
+func NewMkLine(line Line) *MkLineImpl {
 	text := line.Text
 
 	if hasPrefix(text, " ") {
-		mkline.Warnf("Makefile lines should not start with space characters.")
+		line.Warnf("Makefile lines should not start with space characters.")
 		Explain(
 			"If you want this line to contain a shell program, use a tab",
 			"character for indentation.  Otherwise please remove the leading",
@@ -85,7 +87,7 @@ func NewMkLine(line Line) (mkline *MkLineImpl) {
 		value = strings.Replace(value, "\\#", "#", -1)
 		varparam := varnameParam(varname)
 
-		mkline.data = mkLineAssign{
+		return &MkLineImpl{line, mkLineAssign{
 			commented,
 			varname,
 			varnameCanon(varname),
@@ -93,58 +95,48 @@ func NewMkLine(line Line) (mkline *MkLineImpl) {
 			NewMkOperator(op),
 			valueAlign,
 			value,
-			comment}
-		mkline.Tokenize(value)
-		return
+			comment}}
 	}
 
 	if hasPrefix(text, "\t") {
 		shellcmd := text[1:]
-		mkline.data = mkLineShell{shellcmd}
-		mkline.Tokenize(shellcmd)
-		return
+		return &MkLineImpl{line, mkLineShell{shellcmd}}
 	}
 
 	trimmedText := strings.TrimSpace(text)
 	if strings.HasPrefix(trimmedText, "#") {
-		mkline.data = mkLineComment{}
-		return
+		return &MkLineImpl{line, mkLineComment{}}
 	}
 
 	if trimmedText == "" {
-		mkline.data = mkLineEmpty{}
-		return
+		return &MkLineImpl{line, mkLineEmpty{}}
 	}
 
 	if m, indent, directive, args := matchMkCond(text); m {
-		mkline.data = mkLineConditional{indent, directive, args}
-		return
+		return &MkLineImpl{line, mkLineConditional{indent, directive, args}}
 	}
 
 	if m, indent, directive, includefile := MatchMkInclude(text); m {
-		mkline.data = mkLineInclude{directive == "include", false, indent, includefile, ""}
-		return
+		return &MkLineImpl{line, mkLineInclude{directive == "include", false, indent, includefile, ""}}
 	}
 
 	if m, indent, directive, includefile := match3(text, `^\.(\s*)(s?include)\s+<([^>]+)>\s*(?:#.*)?$`); m {
-		mkline.data = mkLineInclude{directive == "include", true, indent, includefile, ""}
-		return
+		return &MkLineImpl{line, mkLineInclude{directive == "include", true, indent, includefile, ""}}
 	}
 
 	if m, targets, whitespace, sources := match3(text, `^([^\s:]+(?:\s*[^\s:]+)*)(\s*):\s*([^#]*?)(?:\s*#.*)?$`); m {
-		mkline.data = mkLineDependency{targets, sources}
 		if whitespace != "" {
 			line.Warnf("Space before colon in dependency line.")
 		}
-		return
+		return &MkLineImpl{line, mkLineDependency{targets, sources}}
 	}
 
 	if matches(text, `^(<<<<<<<|=======|>>>>>>>)`) {
-		return
+		return &MkLineImpl{line, nil}
 	}
 
 	line.Errorf("Unknown Makefile line format.")
-	return mkline
+	return &MkLineImpl{line, nil}
 }
 
 func (mkline *MkLineImpl) String() string {
diff --git a/pkgtools/pkglint/files/mklinechecker.go b/pkgtools/pkglint/files/mklinechecker.go
index bd3af967319..2a4f5396901 100644
--- a/pkgtools/pkglint/files/mklinechecker.go
+++ b/pkgtools/pkglint/files/mklinechecker.go
@@ -454,7 +454,7 @@ func (ck MkLineChecker) CheckVarusePermissions(varname string, vartype *Vartype,
 }
 
 func (ck MkLineChecker) WarnVaruseLocalbase() {
-	ck.MkLine.Warnf("The LOCALBASE variable should not be used by packages.")
+	ck.MkLine.Warnf("Please use PREFIX instead of LOCALBASE.")
 	Explain(
 		// from jlam via private mail.
 		"Currently, LOCALBASE is typically used in these cases:",
@@ -465,6 +465,7 @@ func (ck MkLineChecker) WarnVaruseLocalbase() {
 		"Example for (1):",
 		"",
 		"	STRLIST=        ${LOCALBASE}/bin/strlist",
+		"",
 		"	do-build:",
 		"		cd ${WRKSRC} && ${STRLIST} *.str",
 		"",
@@ -472,6 +473,7 @@ func (ck MkLineChecker) WarnVaruseLocalbase() {
 		"",
 		"	EVAL_PREFIX=    STRLIST_PREFIX=strlist",
 		"	STRLIST=        ${STRLIST_PREFIX}/bin/strlist",
+		"",
 		"	do-build:",
 		"		cd ${WRKSRC} && ${STRLIST} *.str",
 		"",
@@ -515,7 +517,7 @@ func (ck MkLineChecker) CheckVaruseShellword(varname string, vartype *Vartype, v
 	//
 	// When doing checks outside a package, the :M* operator is needed for safety.
 	needMstar := matches(varname, `^(?:.*_)?(?:CFLAGS||CPPFLAGS|CXXFLAGS|FFLAGS|LDFLAGS|LIBS)$`) &&
-		(G.Pkg == nil || G.Pkg.vardef["GNU_CONFIGURE"] != nil)
+		(G.Pkg == nil || G.Pkg.vars.Defined("GNU_CONFIGURE"))
 
 	strippedMod := mod
 	if m, stripped := match1(mod, `(.*?)(?::M\*)?(?::Q)?$`); m {
@@ -678,7 +680,7 @@ func (ck MkLineChecker) checkVarassign() {
 			// The variables mentioned in EVAL_PREFIX will later be
 			// defined by find-prefix.mk. Therefore, they are marked
 			// as known in the current file.
-			G.Mk.vardef[evalVarname] = mkline
+			G.Mk.vars.Define(evalVarname, mkline)
 		}
 	}
 
diff --git a/pkgtools/pkglint/files/mklinechecker_test.go b/pkgtools/pkglint/files/mklinechecker_test.go
index bb6ffe5ed94..ad053637c50 100644
--- a/pkgtools/pkglint/files/mklinechecker_test.go
+++ b/pkgtools/pkglint/files/mklinechecker_test.go
@@ -206,7 +206,7 @@ func (s *Suite) Test_MkLineChecker_WarnVaruseLocalbase(c *check.C) {
 	MkLineChecker{mkline}.WarnVaruseLocalbase()
 
 	t.CheckOutputLines(
-		"WARN: options.mk:56: The LOCALBASE variable should not be used by packages.")
+		"WARN: options.mk:56: Please use PREFIX instead of LOCALBASE.")
 }
 
 func (s *Suite) Test_MkLineChecker_CheckRelativePkgdir(c *check.C) {
@@ -225,16 +225,17 @@ func (s *Suite) Test_MkLineChecker_CheckRelativePkgdir(c *check.C) {
 func (s *Suite) Test_MkLineChecker__unclosed_varuse(c *check.C) {
 	t := s.Init(c)
 
-	mkline := t.NewMkLine("Makefile", 93, "EGDIRS=${EGDIR/apparmor.d ${EGDIR/dbus-1/system.d ${EGDIR/pam.d")
+	mklines := t.NewMkLines("Makefile",
+		MkRcsID,
+		"EGDIRS=${EGDIR/apparmor.d ${EGDIR/dbus-1/system.d ${EGDIR/pam.d")
 
-	MkLineChecker{mkline}.checkVarassign()
+	mklines.Check()
 
 	t.CheckOutputLines(
-		"WARN: Makefile:93: Pkglint parse error in MkLine.Tokenize at "+
-			"\"${EGDIR/apparmor.d ${EGDIR/dbus-1/system.d ${EGDIR/pam.d\".",
-		"WARN: Makefile:93: Pkglint parse error in ShTokenizer.ShAtom at "+
-			"\"${EGDIR/apparmor.d ${EGDIR/dbus-1/system.d ${EGDIR/pam.d\" (quoting=plain)",
-		"WARN: Makefile:93: EGDIRS is defined but not used. Spelling mistake?")
+		"WARN: Makefile:2: Unclosed Make variable starting at \"${EGDIR/apparmor.d $...\"",
+		"WARN: Makefile:2: EGDIRS is defined but not used. Spelling mistake?",
+		"WARN: Makefile:2: Pkglint parse error in MkLine.Tokenize at "+
+			"\"${EGDIR/apparmor.d ${EGDIR/dbus-1/system.d ${EGDIR/pam.d\".")
 }
 
 func (s *Suite) Test_MkLineChecker__Varuse_Modifier_L(c *check.C) {
diff --git a/pkgtools/pkglint/files/mklines.go b/pkgtools/pkglint/files/mklines.go
index a23f9c366b2..187f93e9aeb 100644
--- a/pkgtools/pkglint/files/mklines.go
+++ b/pkgtools/pkglint/files/mklines.go
@@ -10,14 +10,13 @@ import (
 type MkLines struct {
 	mklines        []MkLine
 	lines          []Line
-	forVars        map[string]bool   // The variables currently used in .for loops
-	target         string            // Current make(1) target
-	vardef         map[string]MkLine // varname => line; for all variables that are defined in the current file
-	varuse         map[string]MkLine // varname => line; for all variables that are used in the current file
-	buildDefs      map[string]bool   // Variables that are registered in BUILD_DEFS, to ensure that all user-defined variables are added to it.
-	plistVars      map[string]bool   // Variables that are registered in PLIST_VARS, to ensure that all user-defined variables are added to it.
-	tools          map[string]bool   // Set of tools that are declared to be used.
-	toolRegistry   ToolRegistry      // Tools defined in file scope.
+	forVars        map[string]bool // The variables currently used in .for loops
+	target         string          // Current make(1) target
+	vars           Scope
+	buildDefs      map[string]bool // Variables that are registered in BUILD_DEFS, to ensure that all user-defined variables are added to it.
+	plistVars      map[string]bool // Variables that are registered in PLIST_VARS, to ensure that all user-defined variables are added to it.
+	tools          map[string]bool // Set of tools that are declared to be used.
+	toolRegistry   ToolRegistry    // Tools defined in file scope.
 	SeenBsdPrefsMk bool
 	indentation    Indentation // Indentation depth of preprocessing directives
 }
@@ -39,8 +38,7 @@ func NewMkLines(lines []Line) *MkLines {
 		lines,
 		make(map[string]bool),
 		"",
-		make(map[string]MkLine),
-		make(map[string]MkLine),
+		NewScope(),
 		make(map[string]bool),
 		make(map[string]bool),
 		tools,
@@ -49,28 +47,15 @@ func NewMkLines(lines []Line) *MkLines {
 		Indentation{}}
 }
 
-func (mklines *MkLines) DefineVar(mkline MkLine, varname string) {
-	if mklines.vardef[varname] == nil {
-		mklines.vardef[varname] = mkline
-	}
-	varcanon := varnameCanon(varname)
-	if mklines.vardef[varcanon] == nil {
-		mklines.vardef[varcanon] = mkline
-	}
-}
-
 func (mklines *MkLines) UseVar(mkline MkLine, varname string) {
-	varcanon := varnameCanon(varname)
-	mklines.varuse[varname] = mkline
-	mklines.varuse[varcanon] = mkline
+	mklines.vars.Use(varname, mkline)
 	if G.Pkg != nil {
-		G.Pkg.varuse[varname] = mkline
-		G.Pkg.varuse[varcanon] = mkline
+		G.Pkg.vars.Use(varname, mkline)
 	}
 }
 
 func (mklines *MkLines) VarValue(varname string) (value string, found bool) {
-	if mkline := mklines.vardef[varname]; mkline != nil {
+	if mkline := mklines.vars.FirstDefinition(varname); mkline != nil {
 		return mkline.Value(), true
 	}
 	return "", false
@@ -117,6 +102,7 @@ func (mklines *MkLines) Check() {
 
 		case mkline.IsVarassign():
 			mklines.target = ""
+			mkline.Tokenize(mkline.Value())
 			substcontext.Varassign(mkline)
 
 		case mkline.IsInclude():
@@ -136,6 +122,9 @@ func (mklines *MkLines) Check() {
 		case mkline.IsDependency():
 			ck.checkDependencyRule(allowedTargets)
 			mklines.target = mkline.Targets()
+
+		case mkline.IsShellcmd():
+			mkline.Tokenize(mkline.Shellcmd())
 		}
 	}
 	lastMkline := mklines.mklines[len(mklines.mklines)-1]
@@ -222,6 +211,57 @@ func (mklines *MkLines) DetermineUsedVariables() {
 			mklines.UseVar(mkline, varname)
 		}
 	}
+	mklines.determineDocumentedVariables()
+}
+
+// Loosely based on mk/help/help.awk, revision 1.28
+func (mklines *MkLines) determineDocumentedVariables() {
+	scope := NewScope()
+	commentLines := 0
+	relevant := true
+
+	finish := func() {
+		if commentLines >= 3 && relevant {
+			for varname, mkline := range scope.used {
+				mklines.vars.Use(varname, mkline)
+			}
+		}
+
+		scope = NewScope()
+		commentLines = 0
+		relevant = true
+	}
+
+	for _, mkline := range mklines.mklines {
+		text := mkline.Text
+		words := splitOnSpace(text)
+
+		if 1 < len(words) && words[0] == "#" {
+			commentLines++
+
+			parser := NewMkParser(mkline.Line, words[1], false)
+			varname := parser.Varname()
+			if hasSuffix(varname, ".") && parser.repl.AdvanceRegexp(`^<\w+>`) {
+				varname += "*"
+			}
+			parser.repl.AdvanceStr(":")
+
+			varbase := varnameBase(varname)
+			if varbase == strings.ToUpper(varbase) && matches(varbase, `[A-Z]`) && parser.EOF() {
+				scope.Use(varname, mkline)
+			}
+
+			if 1 < len(words) && words[1] == "Copyright" {
+				relevant = false
+			}
+		}
+
+		if text == "" {
+			finish()
+		}
+	}
+
+	finish()
 }
 
 func (mklines *MkLines) setSeenBsdPrefsMk() {
diff --git a/pkgtools/pkglint/files/mklines_test.go b/pkgtools/pkglint/files/mklines_test.go
index 106ed4ffebb..fb4368705fd 100644
--- a/pkgtools/pkglint/files/mklines_test.go
+++ b/pkgtools/pkglint/files/mklines_test.go
@@ -1,7 +1,9 @@
 package main
 
 import (
+	"fmt"
 	"gopkg.in/check.v1"
+	"sort"
 )
 
 func (s *Suite) Test_MkLines_Check__autofix_conditional_indentation(c *check.C) {
@@ -162,6 +164,10 @@ func (s *Suite) Test_MkLines__varuse_parameterized(c *check.C) {
 	t.CheckOutputEmpty()
 }
 
+// Even very complicated shell commands are parsed correctly.
+// Since the variable is defined in this same Makefile, it is
+// assumed to be a known shell command and therefore does not need
+// USE_TOOLS or a similar declaration.
 func (s *Suite) Test_MkLines__loop_modifier(c *check.C) {
 	t := s.Init(c)
 
@@ -176,11 +182,8 @@ func (s *Suite) Test_MkLines__loop_modifier(c *check.C) {
 
 	mklines.Check()
 
-	t.CheckOutputLines(
-		// No warning about missing @ at the end
-		"WARN: chat/xchat/Makefile:4: " +
-			"Unknown shell command \"${GCONF_SCHEMAS:@.s.@" +
-			"${INSTALL_DATA} ${WRKSRC}/src/common/dbus/${.s.} ${DESTDIR}${GCONF_SCHEMAS_DIR}/@}\".")
+	// Earlier versions of pkglint warned about a missing @ at the end.
+	t.CheckOutputEmpty()
 }
 
 // PR 46570
@@ -320,8 +323,8 @@ func (s *Suite) Test_MkLines_DetermineUsedVariables__simple(c *check.C) {
 
 	mklines.DetermineUsedVariables()
 
-	c.Check(len(mklines.varuse), equals, 1)
-	c.Check(mklines.varuse["VAR"], equals, mkline)
+	c.Check(len(mklines.vars.used), equals, 1)
+	c.Check(mklines.vars.FirstUse("VAR"), equals, mkline)
 }
 
 func (s *Suite) Test_MkLines_DetermineUsedVariables__nested(c *check.C) {
@@ -334,10 +337,10 @@ func (s *Suite) Test_MkLines_DetermineUsedVariables__nested(c *check.C) {
 
 	mklines.DetermineUsedVariables()
 
-	c.Check(len(mklines.varuse), equals, 3)
-	c.Check(mklines.varuse["inner"], equals, mkline)
-	c.Check(mklines.varuse["outer."], equals, mkline)
-	c.Check(mklines.varuse["outer.*"], equals, mkline)
+	c.Check(len(mklines.vars.used), equals, 3)
+	c.Check(mklines.vars.FirstUse("inner"), equals, mkline)
+	c.Check(mklines.vars.FirstUse("outer."), equals, mkline) // XXX: why the . at the end?
+	c.Check(mklines.vars.FirstUse("outer.*"), equals, mkline)
 }
 
 func (s *Suite) Test_MkLines_PrivateTool_Undefined(c *check.C) {
@@ -443,3 +446,50 @@ func (s *Suite) Test_MkLines_wip_category_Makefile(c *check.C) {
 	t.CheckOutputLines(
 		"ERROR: Makefile:14: \"/mk/misc/category.mk\" does not exist.")
 }
+
+func (s *Suite) Test_MkLines_ExtractDocumentedVariables(c *check.C) {
+	t := s.Init(c)
+
+	t.SetupCommandLine("-Wall")
+	G.globalData.InitVartypes()
+	t.SetupTool(&Tool{Name: "rm", Varname: "RM", Predefined: true})
+	mklines := t.NewMkLines("Makefile",
+		MkRcsID,
+		"#",
+		"# User-settable variables:",
+		"#",
+		"# PKG_DEBUG_LEVEL",
+		"#\tHow verbose should pkgsrc be when running shell commands?",
+		"#",
+		"#\t* 0:\tdon't show most shell ...",
+		"",
+		"# PKG_VERBOSE",
+		"#\tWhen this variable is defined, pkgsrc gets a bit more verbose",
+		"#\t(i.e. \"-v\" option is passed to some commands ...",
+		"",
+		"# VARIABLE",
+		"#\tA paragraph of a single line is not enough to be recognized as \"relevant\".",
+		"",
+		"# VARBASE1.<param1>",
+		"# VARBASE2.*",
+		"# VARBASE3.${id}")
+
+	// The variables that appear in the documentation are marked as
+	// used, to prevent the "defined but not used" warnings.
+	mklines.determineDocumentedVariables()
+
+	var varnames []string
+	for varname, mkline := range mklines.vars.used {
+		varnames = append(varnames, fmt.Sprintf("%s (line %s)", varname, mkline.Linenos()))
+	}
+	sort.Strings(varnames)
+
+	expected := []string{
+		"PKG_DEBUG_LEVEL (line 5)",
+		"PKG_VERBOSE (line 10)",
+		"VARBASE1.* (line 17)",
+		"VARBASE2.* (line 18)",
+		"VARBASE3.${id} (line 19)",
+		"VARBASE3.* (line 19)"}
+	c.Check(varnames, deepEquals, expected)
+}
diff --git a/pkgtools/pkglint/files/package.go b/pkgtools/pkglint/files/package.go
index 8d99e4c6167..cca34c2f8a3 100644
--- a/pkgtools/pkglint/files/package.go
+++ b/pkgtools/pkglint/files/package.go
@@ -26,13 +26,12 @@ type Package struct {
 	SeenBsdPrefsMk       bool            // Has bsd.prefs.mk already been included?
 	PlistDirs            map[string]bool // Directories mentioned in the PLIST files
 
-	vardef                map[string]MkLine // (varname, varcanon) => line
-	varuse                map[string]MkLine // (varname, varcanon) => line
-	bl3                   map[string]Line   // buildlink3.mk name => line; contains only buildlink3.mk files that are directly included.
-	plistSubstCond        map[string]bool   // varname => true; list of all variables that are used as conditionals (@comment or nothing) in PLISTs.
-	included              map[string]Line   // fname => line
-	seenMakefileCommon    bool              // Does the package have any .includes?
-	loadTimeTools         map[string]bool   // true=ok, false=not ok, absent=not mentioned in USE_TOOLS.
+	vars                  Scope
+	bl3                   map[string]Line // buildlink3.mk name => line; contains only buildlink3.mk files that are directly included.
+	plistSubstCond        map[string]bool // varname => true; list of all variables that are used as conditionals (@comment or nothing) in PLISTs.
+	included              map[string]Line // fname => line
+	seenMakefileCommon    bool            // Does the package have any .includes?
+	loadTimeTools         map[string]bool // true=ok, false=not ok, absent=not mentioned in USE_TOOLS.
 	conditionalIncludes   map[string]MkLine
 	unconditionalIncludes map[string]MkLine
 	once                  Once
@@ -42,8 +41,7 @@ func NewPackage(pkgpath string) *Package {
 	pkg := &Package{
 		Pkgpath:               pkgpath,
 		PlistDirs:             make(map[string]bool),
-		vardef:                make(map[string]MkLine),
-		varuse:                make(map[string]MkLine),
+		vars:                  NewScope(),
 		bl3:                   make(map[string]Line),
 		plistSubstCond:        make(map[string]bool),
 		included:              make(map[string]Line),
@@ -52,21 +50,11 @@ func NewPackage(pkgpath string) *Package {
 		unconditionalIncludes: make(map[string]MkLine),
 	}
 	for varname, line := range G.globalData.UserDefinedVars {
-		pkg.vardef[varname] = line
+		pkg.vars.Define(varname, line)
 	}
 	return pkg
 }
 
-func (pkg *Package) defineVar(mkline MkLine, varname string) {
-	if pkg.vardef[varname] == nil {
-		pkg.vardef[varname] = mkline
-	}
-	varcanon := varnameCanon(varname)
-	if pkg.vardef[varcanon] == nil {
-		pkg.vardef[varcanon] = mkline
-	}
-}
-
 func (pkg *Package) varValue(varname string) (string, bool) {
 	switch varname {
 	case "KRB5_TYPE":
@@ -74,7 +62,7 @@ func (pkg *Package) varValue(varname string) (string, bool) {
 	case "PGSQL_VERSION":
 		return "95", true
 	}
-	if mkline := pkg.vardef[varname]; mkline != nil {
+	if mkline := pkg.vars.FirstDefinition(varname); mkline != nil {
 		return mkline.Value(), true
 	}
 	return "", false
@@ -150,7 +138,7 @@ func (pkg *Package) checklinesBuildlink3Inclusion(mklines *MkLines) {
 	}
 }
 
-func checkdirPackage(pkgpath string) {
+func (pkglint *Pkglint) checkdirPackage(pkgpath string) {
 	if trace.Tracing {
 		defer trace.Call1(pkgpath)()
 	}
@@ -203,7 +191,7 @@ func checkdirPackage(pkgpath string) {
 				pkg.checkfilePackageMakefile(fname, lines)
 			}
 		} else {
-			Checkfile(fname)
+			pkglint.Checkfile(fname)
 		}
 		if contains(fname, "/patches/patch-") {
 			havePatches = true
@@ -359,11 +347,11 @@ func (pkg *Package) readMakefile(fname string, mainLines *MkLines, allLines *MkL
 		if mkline.IsVarassign() {
 			varname, op, value := mkline.Varname(), mkline.Op(), mkline.Value()
 
-			if op != opAssignDefault || G.Pkg.vardef[varname] == nil {
+			if op != opAssignDefault || !G.Pkg.vars.Defined(varname) {
 				if trace.Tracing {
 					trace.Stepf("varassign(%q, %q, %q)", varname, op, value)
 				}
-				G.Pkg.vardef[varname] = mkline
+				G.Pkg.vars.Define(varname, mkline)
 			}
 		}
 	}
@@ -380,16 +368,16 @@ func (pkg *Package) checkfilePackageMakefile(fname string, mklines *MkLines) {
 		defer trace.Call1(fname)()
 	}
 
-	vardef := pkg.vardef
-	if vardef["PLIST_SRC"] == nil &&
-		vardef["GENERATE_PLIST"] == nil &&
-		vardef["META_PACKAGE"] == nil &&
+	vars := pkg.vars
+	if !vars.Defined("PLIST_SRC") &&
+		!vars.Defined("GENERATE_PLIST") &&
+		!vars.Defined("META_PACKAGE") &&
 		!fileExists(G.CurrentDir+"/"+pkg.Pkgdir+"/PLIST") &&
 		!fileExists(G.CurrentDir+"/"+pkg.Pkgdir+"/PLIST.common") {
 		NewLineWhole(fname).Warnf("Neither PLIST nor PLIST.common exist, and PLIST_SRC is unset. Are you sure PLIST handling is ok?")
 	}
 
-	if (vardef["NO_CHECKSUM"] != nil || vardef["META_PACKAGE"] != nil) && isEmptyDir(G.CurrentDir+"/"+pkg.Patchdir) {
+	if (vars.Defined("NO_CHECKSUM") || vars.Defined("META_PACKAGE")) && isEmptyDir(G.CurrentDir+"/"+pkg.Patchdir) {
 		if distinfoFile := G.CurrentDir + "/" + pkg.DistinfoFile; fileExists(distinfoFile) {
 			NewLineWhole(distinfoFile).Warnf("This file should not exist if NO_CHECKSUM or META_PACKAGE is set.")
 		}
@@ -399,15 +387,15 @@ func (pkg *Package) checkfilePackageMakefile(fname string, mklines *MkLines) {
 		}
 	}
 
-	if perlLine, noconfLine := vardef["REPLACE_PERL"], vardef["NO_CONFIGURE"]; perlLine != nil && noconfLine != nil {
+	if perlLine, noconfLine := vars.FirstDefinition("REPLACE_PERL"), vars.FirstDefinition("NO_CONFIGURE"); perlLine != nil && noconfLine != nil {
 		perlLine.Warnf("REPLACE_PERL is ignored when NO_CONFIGURE is set (in %s)", noconfLine.ReferenceFrom(perlLine.Line))
 	}
 
-	if vardef["LICENSE"] == nil && vardef["META_PACKAGE"] == nil && pkg.once.FirstTime("LICENSE") {
+	if !vars.Defined("LICENSE") && !vars.Defined("META_PACKAGE") && pkg.once.FirstTime("LICENSE") {
 		NewLineWhole(fname).Errorf("Each package must define its LICENSE.")
 	}
 
-	if gnuLine, useLine := vardef["GNU_CONFIGURE"], vardef["USE_LANGUAGES"]; gnuLine != nil && useLine != nil {
+	if gnuLine, useLine := vars.FirstDefinition("GNU_CONFIGURE"), vars.FirstDefinition("USE_LANGUAGES"); gnuLine != nil && useLine != nil {
 		if matches(useLine.VarassignComment(), `(?-i)\b(?:c|empty|none)\b`) {
 			// Don't emit a warning, since the comment
 			// probably contains a statement that C is
@@ -425,11 +413,11 @@ func (pkg *Package) checkfilePackageMakefile(fname string, mklines *MkLines) {
 	pkg.determineEffectivePkgVars()
 	pkg.checkPossibleDowngrade()
 
-	if vardef["COMMENT"] == nil {
+	if !vars.Defined("COMMENT") {
 		NewLineWhole(fname).Warnf("No COMMENT given.")
 	}
 
-	if imake, x11 := vardef["USE_IMAKE"], vardef["USE_X11"]; imake != nil && x11 != nil {
+	if imake, x11 := vars.FirstDefinition("USE_IMAKE"), vars.FirstDefinition("USE_X11"); imake != nil && x11 != nil {
 		if !hasSuffix(x11.Filename, "/mk/x11.buildlink3.mk") {
 			imake.Notef("USE_IMAKE makes USE_X11 in %s superfluous.", x11.ReferenceFrom(imake.Line))
 		}
@@ -442,7 +430,7 @@ func (pkg *Package) checkfilePackageMakefile(fname string, mklines *MkLines) {
 }
 
 func (pkg *Package) getNbpart() string {
-	line := pkg.vardef["PKGREVISION"]
+	line := pkg.vars.FirstDefinition("PKGREVISION")
 	if line == nil {
 		return ""
 	}
@@ -454,8 +442,8 @@ func (pkg *Package) getNbpart() string {
 }
 
 func (pkg *Package) determineEffectivePkgVars() {
-	distnameLine := pkg.vardef["DISTNAME"]
-	pkgnameLine := pkg.vardef["PKGNAME"]
+	distnameLine := pkg.vars.FirstDefinition("DISTNAME")
+	pkgnameLine := pkg.vars.FirstDefinition("PKGNAME")
 
 	distname := ""
 	if distnameLine != nil {
@@ -543,7 +531,7 @@ func (pkg *Package) pkgnameFromDistname(pkgname, distname string) string {
 }
 
 func (pkg *Package) expandVariableWithDefault(varname, defaultValue string) string {
-	mkline := G.Pkg.vardef[varname]
+	mkline := G.Pkg.vars.FirstDefinition(varname)
 	if mkline == nil {
 		return defaultValue
 	}
@@ -834,8 +822,8 @@ func (pkg *Package) checkLocallyModified(fname string) {
 		defer trace.Call(fname)()
 	}
 
-	ownerLine := pkg.vardef["OWNER"]
-	maintainerLine := pkg.vardef["MAINTAINER"]
+	ownerLine := pkg.vars.FirstDefinition("OWNER")
+	maintainerLine := pkg.vars.FirstDefinition("MAINTAINER")
 	owner := ""
 	maintainer := ""
 	if ownerLine != nil && !containsVarRef(ownerLine.Value()) {
diff --git a/pkgtools/pkglint/files/package_test.go b/pkgtools/pkglint/files/package_test.go
index 20352ad2f04..30ba8ce7bbd 100644
--- a/pkgtools/pkglint/files/package_test.go
+++ b/pkgtools/pkglint/files/package_test.go
@@ -6,7 +6,7 @@ func (s *Suite) Test_Package_pkgnameFromDistname(c *check.C) {
 	t := s.Init(c)
 
 	pkg := NewPackage("dummy")
-	pkg.vardef["PKGNAME"] = t.NewMkLine("Makefile", 5, "PKGNAME=dummy")
+	pkg.vars.Define("PKGNAME", t.NewMkLine("Makefile", 5, "PKGNAME=dummy"))
 
 	c.Check(pkg.pkgnameFromDistname("pkgname-1.0", "whatever"), equals, "pkgname-1.0")
 	c.Check(pkg.pkgnameFromDistname("${DISTNAME}", "distname-1.0"), equals, "distname-1.0")
@@ -162,7 +162,7 @@ func (s *Suite) Test_Package_varorder_license(c *check.C) {
 	G.globalData.Pkgsrcdir = t.TmpDir()
 	G.CurrentDir = t.TmpDir()
 
-	(&Pkglint{}).CheckDirent(t.TmpDir() + "/x11/9term")
+	G.CheckDirent(t.TmpDir() + "/x11/9term")
 
 	// Since the error is grave enough, the warning about the correct position is suppressed.
 	t.CheckOutputLines(
@@ -249,11 +249,12 @@ func (s *Suite) Test_Package_getNbpart(c *check.C) {
 	t := s.Init(c)
 
 	pkg := NewPackage("category/pkgbase")
-	pkg.vardef["PKGREVISION"] = t.NewMkLine("Makefile", 1, "PKGREVISION=14")
+	pkg.vars.Define("PKGREVISION", t.NewMkLine("Makefile", 1, "PKGREVISION=14"))
 
 	c.Check(pkg.getNbpart(), equals, "nb14")
 
-	pkg.vardef["PKGREVISION"] = t.NewMkLine("Makefile", 1, "PKGREVISION=asdf")
+	pkg.vars = NewScope()
+	pkg.vars.Define("PKGREVISION", t.NewMkLine("Makefile", 1, "PKGREVISION=asdf"))
 
 	c.Check(pkg.getNbpart(), equals, "")
 }
@@ -266,9 +267,9 @@ func (s *Suite) Test_Package_determineEffectivePkgVars__precedence(c *check.C) {
 	distnameLine := t.NewMkLine("Makefile", 4, "DISTNAME=distname-1.0")
 	pkgrevisionLine := t.NewMkLine("Makefile", 5, "PKGREVISION=13")
 
-	pkg.defineVar(pkgnameLine, pkgnameLine.Varname())
-	pkg.defineVar(distnameLine, distnameLine.Varname())
-	pkg.defineVar(pkgrevisionLine, pkgrevisionLine.Varname())
+	pkg.vars.Define(pkgnameLine.Varname(), pkgnameLine)
+	pkg.vars.Define(distnameLine.Varname(), distnameLine)
+	pkg.vars.Define(pkgrevisionLine.Varname(), pkgrevisionLine)
 
 	pkg.determineEffectivePkgVars()
 
@@ -311,11 +312,11 @@ func (s *Suite) Test_checkdirPackage(c *check.C) {
 		MkRcsID)
 	G.CurrentDir = t.TmpDir()
 
-	checkdirPackage(t.TmpDir())
+	G.checkdirPackage(t.TmpDir())
 
 	t.CheckOutputLines(
 		"WARN: ~/Makefile: Neither PLIST nor PLIST.common exist, and PLIST_SRC is unset. Are you sure PLIST handling is ok?",
-		"WARN: ~/distinfo: File not found. Please run \"@BMAKE@ makesum\".",
+		"WARN: ~/distinfo: File not found. Please run \""+confMake+" makesum\".",
 		"ERROR: ~/Makefile: Each package must define its LICENSE.",
 		"WARN: ~/Makefile: No COMMENT given.")
 }
@@ -330,7 +331,7 @@ func (s *Suite) Test_checkdirPackage__meta_package_without_license(c *check.C) {
 	G.CurrentDir = t.TmpDir()
 	G.globalData.InitVartypes()
 
-	checkdirPackage(t.TmpDir())
+	G.checkdirPackage(t.TmpDir())
 
 	t.CheckOutputLines(
 		"WARN: ~/Makefile: No COMMENT given.") // No error about missing LICENSE.
@@ -391,7 +392,7 @@ func (s *Suite) Test_Package__varuse_at_load_time(c *check.C) {
 	t.CreateFileLines("category/pkgbase/distinfo",
 		RcsID)
 
-	(&Pkglint{}).Main("pkglint", "-q", "-Wperm", t.TmpDir()+"/category/pkgbase")
+	G.Main("pkglint", "-q", "-Wperm", t.TmpDir()+"/category/pkgbase")
 
 	t.CheckOutputLines(
 		"WARN: ~/category/pkgbase/Makefile:8: To use the tool \"FALSE\" at load time, bsd.prefs.mk has to be included before.",
@@ -459,7 +460,7 @@ func (s *Suite) Test_Package_conditionalAndUnconditionalInclude(c *check.C) {
 	G.CurPkgsrcdir = "../.."
 	G.Pkg = pkg
 
-	checkdirPackage("category/package")
+	G.checkdirPackage("category/package")
 
 	t.CheckOutputLines(
 		"WARN: ~/category/package/options.mk:3: Unknown option \"zlib\".",
diff --git a/pkgtools/pkglint/files/pkglint.go b/pkgtools/pkglint/files/pkglint.go
index 42579e21262..8160c33a556 100644
--- a/pkgtools/pkglint/files/pkglint.go
+++ b/pkgtools/pkglint/files/pkglint.go
@@ -17,15 +17,107 @@ import (
 const confMake = "@BMAKE@"
 const confVersion = "@VERSION@"
 
+// Pkglint contains all global variables of this Go package.
+// The rest of the global state is in the other packages:
+//  regex.Profiling    (not thread-local)
+//  regex.res          (and related variables; not thread-safe)
+//  textproc.Testing   (not thread-local; harmless)
+//  tracing.Tracing    (not thread-safe)
+//  tracing.Out        (not thread-safe)
+//  tracing.traceDepth (not thread-safe)
+type Pkglint struct {
+	opts       CmdOpts    //
+	globalData GlobalData //
+	Pkg        *Package   // The package that is currently checked.
+	Mk         *MkLines   // The Makefile (or fragment) that is currently checked.
+
+	Todo            []string // The files or directories that still need to be checked.
+	CurrentDir      string   // The currently checked directory, relative to the cwd
+	CurPkgsrcdir    string   // The pkgsrc directory, relative to currentDir
+	Wip             bool     // Is the currently checked directory from pkgsrc-wip?
+	Infrastructure  bool     // Is the currently checked item from the pkgsrc infrastructure?
+	Testing         bool     // Is pkglint in self-testing mode (only during development)?
+	CurrentUsername string   // For checking against OWNER and MAINTAINER
+	CvsEntriesDir   string   // Cached to avoid I/O
+	CvsEntriesLines []Line
+
+	Hash         map[string]*Hash // Maps "alg:fname" => hash (inter-package check).
+	UsedLicenses map[string]bool  // Maps "license name" => true (inter-package check).
+
+	errors                int
+	warnings              int
+	explainNext           bool
+	logged                map[string]bool
+	explanationsAvailable bool
+	explanationsGiven     map[string]bool
+	autofixAvailable      bool
+	logOut                *SeparatorWriter
+	logErr                *SeparatorWriter
+
+	loghisto *histogram.Histogram
+}
+
+type CmdOpts struct {
+	CheckAlternatives,
+	CheckBuildlink3,
+	CheckDescr,
+	CheckDistinfo,
+	CheckExtra,
+	CheckGlobal,
+	CheckInstall,
+	CheckMakefile,
+	CheckMessage,
+	CheckMk,
+	CheckPatches,
+	CheckPlist bool
+
+	WarnAbsname,
+	WarnDirectcmd,
+	WarnExtra,
+	WarnOrder,
+	WarnPerm,
+	WarnPlistDepr,
+	WarnPlistSort,
+	WarnQuoting,
+	WarnSpace,
+	WarnStyle,
+	WarnTypes bool
+
+	Explain,
+	Autofix,
+	GccOutput,
+	PrintHelp,
+	DumpMakefile,
+	Import,
+	LogVerbose,
+	Profiling,
+	Quiet,
+	Recursive,
+	PrintAutofix,
+	PrintSource,
+	PrintVersion bool
+
+	LogOnly []string
+
+	args []string
+}
+
+type Hash struct {
+	hash string
+	line Line
+}
+
+// G is the abbreviation for "global state";
+// it is the only global variable in this Go package
+var G Pkglint
+
 func main() {
 	G.logOut = NewSeparatorWriter(os.Stdout)
 	G.logErr = NewSeparatorWriter(os.Stderr)
 	trace.Out = os.Stdout
-	os.Exit(new(Pkglint).Main(os.Args...))
+	os.Exit(G.Main(os.Args...))
 }
 
-type Pkglint struct{}
-
 // Main runs the main program with the given arguments.
 // args[0] is the program name.
 func (pkglint *Pkglint) Main(args ...string) (exitcode int) {
@@ -43,7 +135,7 @@ func (pkglint *Pkglint) Main(args ...string) (exitcode int) {
 		return *exitcode
 	}
 
-	if G.opts.Profiling {
+	if pkglint.opts.Profiling {
 		f, err := os.Create("pkglint.pprof")
 		if err != nil {
 			dummyLine.Fatalf("Cannot create profiling file: %s", err)
@@ -52,45 +144,45 @@ func (pkglint *Pkglint) Main(args ...string) (exitcode int) {
 		defer pprof.StopCPUProfile()
 
 		regex.Profiling = true
-		G.loghisto = histogram.New()
+		pkglint.loghisto = histogram.New()
 		defer func() {
-			G.logOut.Write("")
-			G.loghisto.PrintStats("loghisto", G.logOut.out, 0)
+			pkglint.logOut.Write("")
+			pkglint.loghisto.PrintStats("loghisto", pkglint.logOut.out, 0)
 			regex.PrintStats()
 		}()
 	}
 
-	for _, arg := range G.opts.args {
-		G.Todo = append(G.Todo, filepath.ToSlash(arg))
+	for _, arg := range pkglint.opts.args {
+		pkglint.Todo = append(pkglint.Todo, filepath.ToSlash(arg))
 	}
-	if len(G.Todo) == 0 {
-		G.Todo = []string{"."}
+	if len(pkglint.Todo) == 0 {
+		pkglint.Todo = []string{"."}
 	}
 
-	G.globalData.Initialize()
+	pkglint.globalData.Initialize()
 
 	currentUser, err := user.Current()
 	if err == nil {
 		// On Windows, this is `Computername\Username`.
-		G.CurrentUsername = regex.Compile(`^.*\\`).ReplaceAllString(currentUser.Username, "")
+		pkglint.CurrentUsername = regex.Compile(`^.*\\`).ReplaceAllString(currentUser.Username, "")
 	}
 
-	for len(G.Todo) != 0 {
-		item := G.Todo[0]
-		G.Todo = G.Todo[1:]
+	for len(pkglint.Todo) != 0 {
+		item := pkglint.Todo[0]
+		pkglint.Todo = pkglint.Todo[1:]
 		pkglint.CheckDirent(item)
 	}
 
 	checkToplevelUnusedLicenses()
 	pkglint.PrintSummary()
-	if G.errors != 0 {
+	if pkglint.errors != 0 {
 		return 1
 	}
 	return 0
 }
 
 func (pkglint *Pkglint) ParseCommandLine(args []string) *int {
-	gopts := &G.opts
+	gopts := &pkglint.opts
 	opts := getopt.NewOptions()
 
 	check := opts.AddFlagGroup('C', "check", "check,...", "enable or disable specific checks")
@@ -138,21 +230,21 @@ func (pkglint *Pkglint) ParseCommandLine(args []string) *int {
 
 	remainingArgs, err := opts.Parse(args)
 	if err != nil {
-		fmt.Fprintf(G.logErr.out, "%s\n\n", err)
-		opts.Help(G.logErr.out, "pkglint [options] dir...")
+		fmt.Fprintf(pkglint.logErr.out, "%s\n\n", err)
+		opts.Help(pkglint.logErr.out, "pkglint [options] dir...")
 		exitcode := 1
 		return &exitcode
 	}
 	gopts.args = remainingArgs
 
 	if gopts.PrintHelp {
-		opts.Help(G.logOut.out, "pkglint [options] dir...")
+		opts.Help(pkglint.logOut.out, "pkglint [options] dir...")
 		exitcode := 0
 		return &exitcode
 	}
 
-	if G.opts.PrintVersion {
-		fmt.Fprintf(G.logOut.out, "%s\n", confVersion)
+	if pkglint.opts.PrintVersion {
+		fmt.Fprintf(pkglint.logOut.out, "%s\n", confVersion)
 		exitcode := 0
 		return &exitcode
 	}
@@ -161,22 +253,22 @@ func (pkglint *Pkglint) ParseCommandLine(args []string) *int {
 }
 
 func (pkglint *Pkglint) PrintSummary() {
-	if !G.opts.Quiet && !G.opts.Autofix {
-		if G.errors != 0 || G.warnings != 0 {
-			G.logOut.Printf("%d %s and %d %s found.\n",
-				G.errors, ifelseStr(G.errors == 1, "error", "errors"),
-				G.warnings, ifelseStr(G.warnings == 1, "warning", "warnings"))
+	if !pkglint.opts.Quiet && !pkglint.opts.Autofix {
+		if pkglint.errors != 0 || pkglint.warnings != 0 {
+			pkglint.logOut.Printf("%d %s and %d %s found.\n",
+				pkglint.errors, ifelseStr(pkglint.errors == 1, "error", "errors"),
+				pkglint.warnings, ifelseStr(pkglint.warnings == 1, "warning", "warnings"))
 		} else {
-			G.logOut.WriteLine("Looks fine.")
+			pkglint.logOut.WriteLine("Looks fine.")
 		}
-		if G.explanationsAvailable && !G.opts.Explain {
-			G.logOut.WriteLine("(Run \"pkglint -e\" to show explanations.)")
+		if pkglint.explanationsAvailable && !pkglint.opts.Explain {
+			pkglint.logOut.WriteLine("(Run \"pkglint -e\" to show explanations.)")
 		}
-		if G.autofixAvailable && !G.opts.PrintAutofix {
-			G.logOut.WriteLine("(Run \"pkglint -fs\" to show what can be fixed automatically.)")
+		if pkglint.autofixAvailable && !pkglint.opts.PrintAutofix {
+			pkglint.logOut.WriteLine("(Run \"pkglint -fs\" to show what can be fixed automatically.)")
 		}
-		if G.autofixAvailable && !G.opts.Autofix {
-			G.logOut.WriteLine("(Run \"pkglint -F\" to automatically fix some issues.)")
+		if pkglint.autofixAvailable && !pkglint.opts.Autofix {
+			pkglint.logOut.WriteLine("(Run \"pkglint -F\" to automatically fix some issues.)")
 		}
 	}
 }
@@ -194,13 +286,14 @@ func (pkglint *Pkglint) CheckDirent(fname string) {
 	isDir := st.Mode().IsDir()
 	isReg := st.Mode().IsRegular()
 
-	G.CurrentDir = ifelseStr(isReg, path.Dir(fname), fname)
-	absCurrentDir := abspath(G.CurrentDir)
-	G.Wip = !G.opts.Import && matches(absCurrentDir, `/wip/|/wip$`)
-	G.Infrastructure = matches(absCurrentDir, `/mk/|/mk$`)
-	G.CurPkgsrcdir = findPkgsrcTopdir(G.CurrentDir)
-	if G.CurPkgsrcdir == "" {
-		NewLineWhole(fname).Errorf("Cannot determine the pkgsrc root directory for %q.", G.CurrentDir)
+	currentDir := ifelseStr(isReg, path.Dir(fname), fname)
+	pkglint.CurrentDir = currentDir
+	absCurrentDir := abspath(currentDir)
+	pkglint.Wip = !pkglint.opts.Import && matches(absCurrentDir, `/wip/|/wip$`)
+	pkglint.Infrastructure = matches(absCurrentDir, `/mk/|/mk$`)
+	pkglint.CurPkgsrcdir = findPkgsrcTopdir(currentDir)
+	if pkglint.CurPkgsrcdir == "" {
+		NewLineWhole(fname).Errorf("Cannot determine the pkgsrc root directory for %q.", currentDir)
 		return
 	}
 
@@ -208,13 +301,13 @@ func (pkglint *Pkglint) CheckDirent(fname string) {
 	case isDir && isEmptyDir(fname):
 		return
 	case isReg:
-		Checkfile(fname)
+		pkglint.Checkfile(fname)
 		return
 	}
 
-	switch G.CurPkgsrcdir {
+	switch pkglint.CurPkgsrcdir {
 	case "../..":
-		checkdirPackage(relpath(G.globalData.Pkgsrcdir, G.CurrentDir))
+		pkglint.checkdirPackage(relpath(pkglint.globalData.Pkgsrcdir, currentDir))
 	case "..":
 		CheckdirCategory()
 	case ".":
@@ -365,14 +458,14 @@ func CheckfileMk(fname string) {
 	SaveAutofixChanges(lines)
 }
 
-func Checkfile(fname string) {
+func (pkglint *Pkglint) Checkfile(fname string) {
 	if trace.Tracing {
 		defer trace.Call1(fname)()
 	}
 
 	basename := path.Base(fname)
 	if hasPrefix(basename, "work") || hasSuffix(basename, "~") || hasSuffix(basename, ".orig") || hasSuffix(basename, ".rej") {
-		if G.opts.Import {
+		if pkglint.opts.Import {
 			NewLineWhole(fname).Errorf("Must be cleaned up before committing the package.")
 		}
 		return
@@ -414,45 +507,45 @@ func Checkfile(fname string) {
 		NewLineWhole(fname).Errorf("Only files and directories are allowed in pkgsrc.")
 
 	case basename == "ALTERNATIVES":
-		if G.opts.CheckAlternatives {
+		if pkglint.opts.CheckAlternatives {
 			CheckfileExtra(fname)
 		}
 
 	case basename == "buildlink3.mk":
-		if G.opts.CheckBuildlink3 {
+		if pkglint.opts.CheckBuildlink3 {
 			if lines := LoadNonemptyLines(fname, true); lines != nil {
 				ChecklinesBuildlink3Mk(NewMkLines(lines))
 			}
 		}
 
 	case hasPrefix(basename, "DESCR"):
-		if G.opts.CheckDescr {
+		if pkglint.opts.CheckDescr {
 			if lines := LoadNonemptyLines(fname, false); lines != nil {
 				ChecklinesDescr(lines)
 			}
 		}
 
 	case basename == "distinfo":
-		if G.opts.CheckDistinfo {
+		if pkglint.opts.CheckDistinfo {
 			if lines := LoadNonemptyLines(fname, false); lines != nil {
 				ChecklinesDistinfo(lines)
 			}
 		}
 
 	case basename == "DEINSTALL" || basename == "INSTALL":
-		if G.opts.CheckInstall {
+		if pkglint.opts.CheckInstall {
 			CheckfileExtra(fname)
 		}
 
 	case hasPrefix(basename, "MESSAGE"):
-		if G.opts.CheckMessage {
+		if pkglint.opts.CheckMessage {
 			if lines := LoadNonemptyLines(fname, false); lines != nil {
 				ChecklinesMessage(lines)
 			}
 		}
 
 	case matches(basename, `^patch-[-A-Za-z0-9_.~+]*[A-Za-z0-9_]$`):
-		if G.opts.CheckPatches {
+		if pkglint.opts.CheckPatches {
 			if lines := LoadNonemptyLines(fname, false); lines != nil {
 				ChecklinesPatch(lines)
 			}
@@ -467,12 +560,12 @@ func Checkfile(fname string) {
 		NewLineWhole(fname).Warnf("Patch files should be named \"patch-\", followed by letters, '-', '_', '.', and digits only.")
 
 	case matches(basename, `^(?:.*\.mk|Makefile.*)$`) && !matches(fname, `files/`) && !matches(fname, `patches/`):
-		if G.opts.CheckMk {
+		if pkglint.opts.CheckMk {
 			CheckfileMk(fname)
 		}
 
 	case hasPrefix(basename, "PLIST"):
-		if G.opts.CheckPlist {
+		if pkglint.opts.CheckPlist {
 			if lines := LoadNonemptyLines(fname, false); lines != nil {
 				ChecklinesPlist(lines)
 			}
@@ -483,7 +576,7 @@ func Checkfile(fname string) {
 
 	case hasPrefix(basename, "CHANGES-"):
 		// This only checks the file, but doesn't register the changes globally.
-		_ = G.globalData.loadDocChangesFromFile(fname)
+		_ = pkglint.globalData.loadDocChangesFromFile(fname)
 
 	case matches(fname, `(?:^|/)files/[^/]*$`):
 		// Skip
@@ -493,7 +586,7 @@ func Checkfile(fname string) {
 
 	default:
 		NewLineWhole(fname).Warnf("Unexpected file found.")
-		if G.opts.CheckExtra {
+		if pkglint.opts.CheckExtra {
 			CheckfileExtra(fname)
 		}
 	}
diff --git a/pkgtools/pkglint/files/pkglint_test.go b/pkgtools/pkglint/files/pkglint_test.go
index 02a37e776ac..fa56a69a85c 100644
--- a/pkgtools/pkglint/files/pkglint_test.go
+++ b/pkgtools/pkglint/files/pkglint_test.go
@@ -11,7 +11,7 @@ import (
 func (s *Suite) Test_Pkglint_Main_help(c *check.C) {
 	t := s.Init(c)
 
-	exitcode := new(Pkglint).Main("pkglint", "-h")
+	exitcode := G.Main("pkglint", "-h")
 
 	c.Check(exitcode, equals, 0)
 	c.Check(t.Output(), check.Matches, `^\Qusage: pkglint [options] dir...\E\n(?s).+`)
@@ -20,7 +20,7 @@ func (s *Suite) Test_Pkglint_Main_help(c *check.C) {
 func (s *Suite) Test_Pkglint_Main_version(c *check.C) {
 	t := s.Init(c)
 
-	exitcode := new(Pkglint).Main("pkglint", "--version")
+	exitcode := G.Main("pkglint", "--version")
 
 	c.Check(exitcode, equals, 0)
 	t.CheckOutputLines(
@@ -30,7 +30,7 @@ func (s *Suite) Test_Pkglint_Main_version(c *check.C) {
 func (s *Suite) Test_Pkglint_Main_no_args(c *check.C) {
 	t := s.Init(c)
 
-	exitcode := new(Pkglint).Main("pkglint")
+	exitcode := G.Main("pkglint")
 
 	c.Check(exitcode, equals, 1)
 	t.CheckOutputLines(
@@ -40,20 +40,20 @@ func (s *Suite) Test_Pkglint_Main_no_args(c *check.C) {
 func (s *Suite) Test_Pkglint_Main__only(c *check.C) {
 	t := s.Init(c)
 
-	exitcode := new(Pkglint).ParseCommandLine([]string{"pkglint", "-Wall", "-o", ":Q", "--version"})
+	exitcode := G.ParseCommandLine([]string{"pkglint", "-Wall", "-o", ":Q", "--version"})
 
 	if c.Check(exitcode, check.NotNil) {
 		c.Check(*exitcode, equals, 0)
 	}
 	c.Check(G.opts.LogOnly, deepEquals, []string{":Q"})
 	t.CheckOutputLines(
-		"@VERSION@")
+		confVersion)
 }
 
 func (s *Suite) Test_Pkglint_Main__unknown_option(c *check.C) {
 	t := s.Init(c)
 
-	exitcode := new(Pkglint).Main("pkglint", "--unknown-option")
+	exitcode := G.Main("pkglint", "--unknown-option")
 
 	c.Check(exitcode, equals, 1)
 	t.CheckOutputLines(
@@ -242,14 +242,14 @@ func (s *Suite) Test_Pkglint_Main__complete_package(c *check.C) {
 		"Size (checkperms-1.12.tar.gz) = 6621 bytes",
 		"SHA1 (patch-checkperms.c) = asdfasdf") // Invalid SHA-1 checksum
 
-	new(Pkglint).Main("pkglint", "-Wall", "-Call", t.TempFilename("sysutils/checkperms"))
+	G.Main("pkglint", "-Wall", "-Call", t.TempFilename("sysutils/checkperms"))
 
 	t.CheckOutputLines(
 		"WARN: ~/sysutils/checkperms/Makefile:3: This package should be updated to 1.13 ([supports more file formats]).",
 		"ERROR: ~/sysutils/checkperms/Makefile:4: Invalid category \"tools\".",
 		"ERROR: ~/sysutils/checkperms/distinfo:7: SHA1 hash of patches/patch-checkperms.c differs "+
 			"(distinfo has asdfasdf, patch file has e775969de639ec703866c0336c4c8e0fdd96309c). "+
-			"Run \"@BMAKE@ makepatchsum\".",
+			"Run \""+confMake+" makepatchsum\".",
 		"WARN: ~/sysutils/checkperms/patches/patch-checkperms.c:12: Premature end of patch hunk "+
 			"(expected 1 lines to be deleted and 0 lines to be added)",
 		"2 errors and 2 warnings found.",
@@ -266,7 +266,7 @@ func (s *Suite) Test_Pkglint_coverage(c *check.C) {
 	cmdline := os.Getenv("PKGLINT_TESTCMDLINE")
 	if cmdline != "" {
 		G.logOut, G.logErr, trace.Out = NewSeparatorWriter(os.Stdout), NewSeparatorWriter(os.Stderr), os.Stdout
-		new(Pkglint).Main(append([]string{"pkglint"}, splitOnSpace(cmdline)...)...)
+		G.Main(append([]string{"pkglint"}, splitOnSpace(cmdline)...)...)
 	}
 }
 
@@ -275,7 +275,7 @@ func (s *Suite) Test_Pkglint_CheckDirent__outside(c *check.C) {
 
 	t.SetupFileLines("empty")
 
-	new(Pkglint).CheckDirent(t.TmpDir())
+	G.CheckDirent(t.TmpDir())
 
 	t.CheckOutputLines(
 		"ERROR: ~: Cannot determine the pkgsrc root directory for \"~\".")
@@ -289,24 +289,23 @@ func (s *Suite) Test_Pkglint_CheckDirent(c *check.C) {
 	t.SetupFileLines("category/Makefile")
 	t.SetupFileLines("Makefile")
 	G.globalData.Pkgsrcdir = t.TmpDir()
-	pkglint := new(Pkglint)
 
-	pkglint.CheckDirent(t.TmpDir())
+	G.CheckDirent(t.TmpDir())
 
 	t.CheckOutputLines(
 		"ERROR: ~/Makefile: Must not be empty.")
 
-	pkglint.CheckDirent(t.TempFilename("category"))
+	G.CheckDirent(t.TempFilename("category"))
 
 	t.CheckOutputLines(
 		"ERROR: ~/category/Makefile: Must not be empty.")
 
-	pkglint.CheckDirent(t.TempFilename("category/package"))
+	G.CheckDirent(t.TempFilename("category/package"))
 
 	t.CheckOutputLines(
 		"ERROR: ~/category/package/Makefile: Must not be empty.")
 
-	pkglint.CheckDirent(t.TempFilename("category/package/nonexistent"))
+	G.CheckDirent(t.TempFilename("category/package/nonexistent"))
 
 	t.CheckOutputLines(
 		"ERROR: ~/category/package/nonexistent: No such file or directory.")
@@ -317,7 +316,7 @@ func (s *Suite) Test_resolveVariableRefs__circular_reference(c *check.C) {
 
 	mkline := t.NewMkLine("fname", 1, "GCC_VERSION=${GCC_VERSION}")
 	G.Pkg = NewPackage(".")
-	G.Pkg.vardef["GCC_VERSION"] = mkline
+	G.Pkg.vars.Define("GCC_VERSION", mkline)
 
 	resolved := resolveVariableRefs("gcc-${GCC_VERSION}")
 
@@ -340,12 +339,15 @@ func (s *Suite) Test_resolveVariableRefs__multilevel(c *check.C) {
 	c.Check(resolved, equals, "you got it")
 }
 
+// Usually, a dot in a variable name means a parameterized form.
+// In this case, it is part of a version number. Resolving these
+// variables from the scope works nevertheless.
 func (s *Suite) Test_resolveVariableRefs__special_chars(c *check.C) {
 	t := s.Init(c)
 
 	mkline := t.NewMkLine("fname", 10, "_=x11")
 	G.Pkg = NewPackage("category/pkg")
-	G.Pkg.vardef["GST_PLUGINS0.10_TYPE"] = mkline
+	G.Pkg.vars.Define("GST_PLUGINS0.10_TYPE", mkline)
 
 	resolved := resolveVariableRefs("gst-plugins0.10-${GST_PLUGINS0.10_TYPE}/distinfo")
 
diff --git a/pkgtools/pkglint/files/plist.go b/pkgtools/pkglint/files/plist.go
index feb829b28e0..b1298a69cbe 100644
--- a/pkgtools/pkglint/files/plist.go
+++ b/pkgtools/pkglint/files/plist.go
@@ -182,7 +182,7 @@ func (ck *PlistChecker) checkpath(pline *PlistLine) {
 		ck.checkpathShare(pline)
 	}
 
-	if contains(text, "${PKGLOCALEDIR}") && G.Pkg != nil && G.Pkg.vardef["USE_PKGLOCALEDIR"] == nil {
+	if contains(text, "${PKGLOCALEDIR}") && G.Pkg != nil && !G.Pkg.vars.Defined("USE_PKGLOCALEDIR") {
 		line.Warnf("PLIST contains ${PKGLOCALEDIR}, but USE_PKGLOCALEDIR was not found.")
 	}
 
@@ -258,7 +258,7 @@ func (ck *PlistChecker) checkpathInfo(pline *PlistLine, dirname, basename string
 		return
 	}
 
-	if G.Pkg != nil && G.Pkg.vardef["INFO_FILES"] == nil {
+	if G.Pkg != nil && !G.Pkg.vars.Defined("INFO_FILES") {
 		pline.line.Warnf("Packages that install info files should set INFO_FILES.")
 	}
 }
@@ -279,11 +279,8 @@ func (ck *PlistChecker) checkpathLib(pline *PlistLine, dirname, basename string)
 
 	switch ext := path.Ext(basename); ext {
 	case ".a", ".la", ".so":
-		if G.opts.WarnExtra && dirname == "lib" && !hasPrefix(basename, "lib") {
-			pline.line.Warnf("Library filename %q should start with \"lib\".", basename)
-		}
 		if ext == "la" {
-			if G.Pkg != nil && G.Pkg.vardef["USE_LIBTOOL"] == nil {
+			if G.Pkg != nil && !G.Pkg.vars.Defined("USE_LIBTOOL") {
 				pline.line.Warnf("Packages that install libtool libraries should define USE_LIBTOOL.")
 			}
 		}
@@ -360,7 +357,7 @@ func (ck *PlistChecker) checkpathShare(pline *PlistLine) {
 			}
 		}
 
-		if contains(text[12:], "/") && G.Pkg.vardef["ICON_THEMES"] == nil && ck.once.FirstTime("ICON_THEMES") {
+		if contains(text[12:], "/") && !G.Pkg.vars.Defined("ICON_THEMES") && ck.once.FirstTime("ICON_THEMES") {
 			line.Warnf("Packages that install icon theme files should set ICON_THEMES.")
 		}
 
diff --git a/pkgtools/pkglint/files/plist_test.go b/pkgtools/pkglint/files/plist_test.go
index 952da23a98f..b5acb562658 100644
--- a/pkgtools/pkglint/files/plist_test.go
+++ b/pkgtools/pkglint/files/plist_test.go
@@ -36,7 +36,6 @@ func (s *Suite) Test_ChecklinesPlist(c *check.C) {
 			"Please use the CONF_FILES framework, which is described in mk/pkginstall/bsd.pkginstall.mk.",
 		"ERROR: PLIST:4: RCD_SCRIPTS must not be registered in the PLIST. Please use the RCD_SCRIPTS framework.",
 		"ERROR: PLIST:6: \"info/dir\" must not be listed. Use install-info to add/remove an entry.",
-		"WARN: PLIST:7: Library filename \"c.so\" should start with \"lib\".",
 		"WARN: PLIST:8: Redundant library found. The libtool library is in line 9.",
 		"WARN: PLIST:9: \"lib/libc.la\" should be sorted before \"lib/libc.so.6\".",
 		"WARN: PLIST:10: Preformatted manual page without unformatted one.",
diff --git a/pkgtools/pkglint/files/shell.go b/pkgtools/pkglint/files/shell.go
index 935dbf4a819..79db84e7c44 100644
--- a/pkgtools/pkglint/files/shell.go
+++ b/pkgtools/pkglint/files/shell.go
@@ -457,7 +457,7 @@ func (scc *SimpleCommandChecker) checkCommandStart() {
 	case scc.handleCommandVariable():
 	case matches(shellword, `^(?::|break|cd|continue|eval|exec|exit|export|read|set|shift|umask|unset)$`):
 	case hasPrefix(shellword, "./"): // All commands from the current directory are fine.
-	case hasPrefix(shellword, "${PKGSRCDIR"): // With or without the :Q modifier
+	case matches(shellword, `\$\{(PKGSRCDIR|PREFIX)(:Q)?\}`):
 	case scc.handleComment():
 	default:
 		if G.opts.WarnExtra && !(G.Mk != nil && G.Mk.indentation.DependsOn("OPSYS")) {
@@ -520,7 +520,9 @@ func (scc *SimpleCommandChecker) handleCommandVariable() bool {
 	}
 
 	shellword := scc.strcmd.Name
-	if m, varname := match1(shellword, `^\$\{([\w_]+)\}$`); m {
+	parser := NewMkParser(scc.shline.mkline.Line, shellword, false)
+	if varuse := parser.VarUse(); varuse != nil && parser.EOF() {
+		varname := varuse.varname
 
 		if tool := G.globalData.Tools.byVarname[varname]; tool != nil {
 			if !G.Mk.tools[tool.Name] {
@@ -537,7 +539,10 @@ func (scc *SimpleCommandChecker) handleCommandVariable() bool {
 
 		// When the package author has explicitly defined a command
 		// variable, assume it to be valid.
-		if G.Pkg != nil && G.Pkg.vardef[varname] != nil {
+		if G.Mk != nil && G.Mk.vars.DefinedSimilar(varname) {
+			return true
+		}
+		if G.Pkg != nil && G.Pkg.vars.DefinedSimilar(varname) {
 			return true
 		}
 	}
diff --git a/pkgtools/pkglint/files/shell_test.go b/pkgtools/pkglint/files/shell_test.go
index bd819303e5c..701e0f59a81 100644
--- a/pkgtools/pkglint/files/shell_test.go
+++ b/pkgtools/pkglint/files/shell_test.go
@@ -109,6 +109,7 @@ func (s *Suite) Test_ShellLine_CheckShellCommandLine(c *check.C) {
 	t := s.Init(c)
 
 	t.SetupCommandLine("-Wall")
+	G.globalData.InitVartypes()
 
 	checkShellCommandLine := func(shellCommand string) {
 		G.Mk = t.NewMkLines("fname",
@@ -122,7 +123,7 @@ func (s *Suite) Test_ShellLine_CheckShellCommandLine(c *check.C) {
 
 	t.CheckOutputEmpty()
 
-	checkShellCommandLine("uname=`uname`; echo $$uname; echo")
+	checkShellCommandLine("uname=`uname`; echo $$uname; echo; ${PREFIX}/bin/command")
 
 	t.CheckOutputLines(
 		"WARN: fname:1: Unknown shell command \"uname\".",
@@ -466,6 +467,7 @@ func (s *Suite) Test_ShellLine_CheckShellCommandLine__shell_variables(c *check.C
 	text := "\tfor f in *.pl; do ${SED} s,@PREFIX@,${PREFIX}, < $f > $f.tmp && ${MV} $f.tmp $f; done"
 
 	shline := t.NewShellLine("Makefile", 3, text)
+	shline.mkline.Tokenize(shline.mkline.Shellcmd())
 	shline.CheckShellCommandLine(text)
 
 	t.CheckOutputLines(
diff --git a/pkgtools/pkglint/files/shtokenizer.go b/pkgtools/pkglint/files/shtokenizer.go
index 9b9bbbabff6..94290e2c1c8 100644
--- a/pkgtools/pkglint/files/shtokenizer.go
+++ b/pkgtools/pkglint/files/shtokenizer.go
@@ -56,7 +56,11 @@ func (p *ShTokenizer) ShAtom(quoting ShQuoting) *ShAtom {
 
 	if atom == nil {
 		repl.Reset(mark)
-		p.parser.Line.Warnf("Pkglint parse error in ShTokenizer.ShAtom at %q (quoting=%s)", repl.Rest(), quoting)
+		if hasPrefix(repl.Rest(), "${") {
+			p.parser.Line.Warnf("Unclosed Make variable starting at %q", shorten(repl.Rest(), 20))
+		} else {
+			p.parser.Line.Warnf("Pkglint parse error in ShTokenizer.ShAtom at %q (quoting=%s)", repl.Rest(), quoting)
+		}
 	}
 	return atom
 }
diff --git a/pkgtools/pkglint/files/util.go b/pkgtools/pkglint/files/util.go
index 9d533744770..6d5bfdce8b4 100644
--- a/pkgtools/pkglint/files/util.go
+++ b/pkgtools/pkglint/files/util.go
@@ -182,6 +182,18 @@ func detab(s string) string {
 	return detabbed
 }
 
+func shorten(s string, maxChars int) string {
+	chars := 0
+	for i := range s {
+		if chars >= maxChars {
+			return s[:i] + "..."
+			break
+		}
+		chars++
+	}
+	return s
+}
+
 func varnameBase(varname string) string {
 	dot := strings.IndexByte(varname, '.')
 	if dot != -1 {
@@ -204,34 +216,30 @@ func varnameParam(varname string) string {
 	return ""
 }
 
+// defineVar marks a variable as defined in both the current package and the current file.
 func defineVar(mkline MkLine, varname string) {
 	if G.Mk != nil {
-		G.Mk.DefineVar(mkline, varname)
+		G.Mk.vars.Define(varname, mkline)
 	}
 	if G.Pkg != nil {
-		G.Pkg.defineVar(mkline, varname)
+		G.Pkg.vars.Define(varname, mkline)
 	}
 }
+
+// varIsDefined tests whether the variable (or its canonicalized form)
+// is defined in the current package or in the current file.
+// TODO: rename to similar
 func varIsDefined(varname string) bool {
-	varcanon := varnameCanon(varname)
-	if G.Mk != nil && (G.Mk.vardef[varname] != nil || G.Mk.vardef[varcanon] != nil) {
-		return true
-	}
-	if G.Pkg != nil && (G.Pkg.vardef[varname] != nil || G.Pkg.vardef[varcanon] != nil) {
-		return true
-	}
-	return false
+	return G.Mk != nil && G.Mk.vars.DefinedSimilar(varname) ||
+		G.Pkg != nil && G.Pkg.vars.DefinedSimilar(varname)
 }
 
+// varIsUsed tests whether the variable (or its canonicalized form)
+// is used in the current package or in the current file.
+// TODO: rename to similar
 func varIsUsed(varname string) bool {
-	varcanon := varnameCanon(varname)
-	if G.Mk != nil && (G.Mk.varuse[varname] != nil || G.Mk.varuse[varcanon] != nil) {
-		return true
-	}
-	if G.Pkg != nil && (G.Pkg.varuse[varname] != nil || G.Pkg.varuse[varcanon] != nil) {
-		return true
-	}
-	return false
+	return G.Mk != nil && G.Mk.vars.UsedSimilar(varname) ||
+		G.Pkg != nil && G.Pkg.vars.UsedSimilar(varname)
 }
 
 func splitOnSpace(s string) []string {
@@ -372,3 +380,94 @@ func (o *Once) FirstTime(what string) bool {
 	o.seen[what] = true
 	return true
 }
+
+// Scope remembers which variables are defined and which are used
+// in a certain scope, such as a package or a file.
+type Scope struct {
+	defined map[string]MkLine
+	used    map[string]MkLine
+}
+
+func NewScope() Scope {
+	return Scope{make(map[string]MkLine), make(map[string]MkLine)}
+}
+
+// Define marks the variable and its canonicalized form as defined.
+func (s *Scope) Define(varname string, line MkLine) {
+	if s.defined[varname] == nil {
+		s.defined[varname] = line
+		if trace.Tracing {
+			trace.Step2("Defining %q in line %s", varname, line.Linenos())
+		}
+	}
+	varcanon := varnameCanon(varname)
+	if varcanon != varname && s.defined[varcanon] == nil {
+		s.defined[varcanon] = line
+		if trace.Tracing {
+			trace.Step2("Defining %q in line %s", varcanon, line.Linenos())
+		}
+	}
+}
+
+// Use marks the variable and its canonicalized form as used.
+func (s *Scope) Use(varname string, line MkLine) {
+	if s.used[varname] == nil {
+		s.used[varname] = line
+		if trace.Tracing {
+			trace.Step2("Using %q in line %s", varname, line.Linenos())
+		}
+	}
+	varcanon := varnameCanon(varname)
+	if varcanon != varname && s.used[varcanon] == nil {
+		s.used[varcanon] = line
+		if trace.Tracing {
+			trace.Step2("Using %q in line %s", varcanon, line.Linenos())
+		}
+	}
+}
+
+// Defined tests whether the variable is defined.
+// It does NOT test the canonicalized variable name.
+func (s *Scope) Defined(varname string) bool {
+	return s.defined[varname] != nil
+}
+
+// DefinedSimilar tests whether the variable or its canonicalized form is defined.
+func (s *Scope) DefinedSimilar(varname string) bool {
+	if s.defined[varname] != nil {
+		if trace.Tracing {
+			trace.Step1("Variable %q is defined", varname)
+		}
+		return true
+	}
+	varcanon := varnameCanon(varname)
+	if s.defined[varcanon] != nil {
+		if trace.Tracing {
+			trace.Step2("Variable %q (similar to %q) is defined", varcanon, varname)
+		}
+		return true
+	}
+	return false
+}
+
+// Used tests whether the variable is used.
+// It does NOT test the canonicalized variable name.
+func (s *Scope) Used(varname string) bool {
+	return s.used[varname] != nil
+}
+
+// UsedSimilar tests whether the variable or its canonicalized form is used.
+func (s *Scope) UsedSimilar(varname string) bool {
+	if s.used[varname] != nil {
+		return true
+	}
+	return s.used[varnameCanon(varname)] != nil
+}
+
+func (s *Scope) FirstDefinition(varname string) MkLine {
+	return s.defined[varname]
+}
+
+func (s *Scope) FirstUse(varname string) MkLine {
+	return s.used[varname]
+}
diff --git a/pkgtools/pkglint/files/vardefs.go b/pkgtools/pkglint/files/vardefs.go
index 5a9184b6d9d..a79920729be 100644
--- a/pkgtools/pkglint/files/vardefs.go
+++ b/pkgtools/pkglint/files/vardefs.go
@@ -362,6 +362,7 @@ func (gd *GlobalData) InitVartypes() {
 	sys("BSD_MAKE_ENV", lkShell, BtShellWord)
 	acl("BUILDLINK_ABI_DEPENDS.*", lkSpace, BtDependency, "builtin.mk: append, use-loadtime; *: append")
 	acl("BUILDLINK_API_DEPENDS.*", lkSpace, BtDependency, "builtin.mk: append, use-loadtime; *: append")
+	acl("BUILDLINK_AUTO_DIRS.*", lkNone, BtYesNo, "buildlink3.mk: append")
 	acl("BUILDLINK_CONTENTS_FILTER", lkNone, BtShellCommand, "")
 	sys("BUILDLINK_CFLAGS", lkShell, BtCFlag)
 	bl3list("BUILDLINK_CFLAGS.*", lkShell, BtCFlag)
@@ -579,6 +580,7 @@ func (gd *GlobalData) InitVartypes() {
 	pkg("GITHUB_TYPE", lkNone, enum("tag release"))
 	pkg("GMAKE_REQD", lkNone, BtVersion)
 	acl("GNU_ARCH", lkNone, enum("mips"), "")
+	acl("GNU_ARCH.*", lkNone, BtIdentifier, "buildlink3.mk:; *: set, use")
 	acl("GNU_CONFIGURE", lkNone, BtYes, "Makefile, Makefile.common: set")
 	acl("GNU_CONFIGURE_INFODIR", lkNone, BtPathname, "Makefile, Makefile.common: set")
 	acl("GNU_CONFIGURE_LIBDIR", lkNone, BtPathname, "Makefile, Makefile.common: set")
@@ -969,6 +971,7 @@ func (gd *GlobalData) InitVartypes() {
 	acl("USE_TOOLS", lkShell, BtTool, "*: append")
 	acl("USE_TOOLS.*", lkShell, BtTool, "*: append")
 	pkg("USE_X11", lkNone, BtYes)
+	sys("WARNINGS", lkShell, BtShellWord)
 	sys("WARNING_MSG", lkNone, BtShellCommand)
 	sys("WARNING_CAT", lkNone, BtShellCommand)
 	acl("WRAPPER_REORDER_CMDS", lkShell, BtWrapperReorder, "Makefile, Makefile.common, buildlink3.mk: append")
@@ -980,6 +983,14 @@ func (gd *GlobalData) InitVartypes() {
 	usr("XAW_TYPE", lkNone, enum("3d neXtaw standard xpm"))
 	acl("XMKMF_FLAGS", lkShell, BtShellWord, "")
 	pkglist("_WRAP_EXTRA_ARGS.*", lkShell, BtShellWord)
+
+	// Only for infrastructure files; see mk/misc/show.mk
+	acl("_VARGROUPS", lkSpace, BtIdentifier, "*: append")
+	acl("_USER_VARS.*", lkSpace, BtIdentifier, "*: append")
+	acl("_PKG_VARS.*", lkSpace, BtIdentifier, "*: append")
+	acl("_SYS_VARS.*", lkSpace, BtIdentifier, "*: append")
+	acl("_DEF_VARS.*", lkSpace, BtIdentifier, "*: append")
+	acl("_USE_VARS.*", lkSpace, BtIdentifier, "*: append")
 }
 
 func enum(values string) *BasicType {
diff --git a/pkgtools/pkglint/files/vartypecheck.go b/pkgtools/pkglint/files/vartypecheck.go
index 78e684130f4..0d423566da1 100644
--- a/pkgtools/pkglint/files/vartypecheck.go
+++ b/pkgtools/pkglint/files/vartypecheck.go
@@ -193,6 +193,23 @@ func (cv *VartypeCheck) Comment() {
 	if m, first := match1(value, `^(?i)(a|an)\s`); m {
 		line.Warnf("COMMENT should not begin with %q.", first)
 	}
+	if m, isA := match1(value, ` (is a|is an) `); m {
+		line.Warnf("COMMENT should not contain %q.", isA)
+		Explain(
+			"The words \"package is a\" are redundant.  Since every package comment",
+			"could start with them, it is better to remove this redundancy in all",
+			"cases.")
+	}
+	if G.Pkg != nil && G.Pkg.EffectivePkgbase != "" {
+		pkgbase := G.Pkg.EffectivePkgbase
+		if strings.HasPrefix(strings.ToLower(value), strings.ToLower(pkgbase+" ")) {
+			line.Warnf("COMMENT should not start with the package name.")
+			Explain(
+				"The COMMENT is usually displayed together with the package name.",
+				"Therefore it does not need to repeat the package name but should",
+				"provide additional information instead.")
+		}
+	}
 	if matches(value, `^[a-z]`) {
 		line.Warnf("COMMENT should start with a capital letter.")
 	}
diff --git a/pkgtools/pkglint/files/vartypecheck_test.go b/pkgtools/pkglint/files/vartypecheck_test.go
index eb772ea00d6..602e7ffe8de 100644
--- a/pkgtools/pkglint/files/vartypecheck_test.go
+++ b/pkgtools/pkglint/files/vartypecheck_test.go
@@ -80,13 +80,20 @@ func (s *Suite) Test_VartypeCheck_CFlag(c *check.C) {
 func (s *Suite) Test_VartypeCheck_Comment(c *check.C) {
 	t := s.Init(c)
 
+	G.Pkg = NewPackage("category/converter")
+	G.Pkg.EffectivePkgbase = "converter"
+
 	runVartypeChecks(t, "COMMENT", opAssign, (*VartypeCheck).Comment,
 		"Versatile Programming Language",
 		"TODO: Short description of the package",
 		"A great package.",
 		"some packages need a very very long comment to explain their basic usefulness",
 		"\"Quoting the comment is wrong\"",
-		"'Quoting the comment is wrong'")
+		"'Quoting the comment is wrong'",
+		"Package is a great package",
+		"Package is an awesome package",
+		"The Big New Package is a great package",
+		"Converter converts between measurement units")
 
 	t.CheckOutputLines(
 		"ERROR: fname:2: COMMENT must be set.",
@@ -95,7 +102,11 @@ func (s *Suite) Test_VartypeCheck_Comment(c *check.C) {
 		"WARN: fname:4: COMMENT should start with a capital letter.",
 		"WARN: fname:4: COMMENT should not be longer than 70 characters.",
 		"WARN: fname:5: COMMENT should not be enclosed in quotes.",
-		"WARN: fname:6: COMMENT should not be enclosed in quotes.")
+		"WARN: fname:6: COMMENT should not be enclosed in quotes.",
+		"WARN: fname:7: COMMENT should not contain \"is a\".",
+		"WARN: fname:8: COMMENT should not contain \"is an\".",
+		"WARN: fname:9: COMMENT should not contain \"is a\".",
+		"WARN: fname:10: COMMENT should not start with the package name.")
 }
 
 func (s *Suite) Test_VartypeCheck_ConfFiles(c *check.C) {
@@ -616,6 +627,7 @@ func runVartypeChecks(t *Tester, varname string, op MkOperator, checker func(*Va
 	}
 	for i, value := range values {
 		mkline := t.NewMkLine("fname", i+1, varname+op.String()+value)
+		mkline.Tokenize(mkline.Value())
 		valueNovar := mkline.WithoutMakeVariables(mkline.Value())
 		vc := &VartypeCheck{mkline, mkline.Line, mkline.Varname(), mkline.Op(), mkline.Value(), valueNovar, "", false}
 		checker(vc)