pkgtools/pkglint: Update to 22.3.1

Changes since 22.3.0:

In doc/CHANGES files, check for typos in month and day of the dates.

In conditions for YesNo variables, suggest to replace the modifier
':M[yY][eE][sS]' with a simpler comparison.

https://mail-index.netbsd.org/tech-pkg/2022/11/16/msg026992.html

8 files changed, 599 insertions, 338 deletions

diff --git a/pkgtools/pkglint/Makefile b/pkgtools/pkglint/Makefile
index 2af76810833..6ecea60d2e4 100644
--- a/pkgtools/pkglint/Makefile
+++ b/pkgtools/pkglint/Makefile
@@ -1,7 +1,6 @@
-# $NetBSD: Makefile,v 1.733 2022/11/02 19:39:53 bsiegert Exp $
+# $NetBSD: Makefile,v 1.734 2022/11/19 10:51:07 rillig Exp $
 
-PKGNAME=	pkglint-22.3.0
-PKGREVISION=	2
+PKGNAME=	pkglint-22.3.1
 CATEGORIES=	pkgtools
 
 MAINTAINER=	rillig@NetBSD.org
@@ -16,7 +15,7 @@ CHECK_RELRO_SKIP+=	bin/pkglint
 
 SUBST_CLASSES+=		pkglint
 SUBST_STAGE.pkglint=	post-configure
-SUBST_FILES.pkglint+=	${WRKSRC}/pkglint.go
+SUBST_FILES.pkglint+=	pkglint.go
 SUBST_SED.pkglint+=	-e s\|@VERSION@\|${PKGVERSION}\|g
 SUBST_SED.pkglint+=	-e s\|@BMAKE@\|${MAKE:T:Q}\|g
 
@@ -46,7 +45,7 @@ post-install: do-install-man
 PLIST_VARS+=		catinstall0 catinstall maninstall
 .if ${MANINSTALL:Mcatinstall}
 INSTALLATION_DIRS+=	man/cat1
-.  if ${CATMAN_SECTION_SUFFIX:M[Yy][Ee][Ss]}
+.  if ${CATMAN_SECTION_SUFFIX:U:tl} == yes
 PLIST.catinstall=	yes
 .  else
 PLIST.catinstall0=	yes
diff --git a/pkgtools/pkglint/files/mkcondchecker_test.go b/pkgtools/pkglint/files/mkcondchecker_test.go
index 3518b465721..207a6f309e3 100644
--- a/pkgtools/pkglint/files/mkcondchecker_test.go
+++ b/pkgtools/pkglint/files/mkcondchecker_test.go
@@ -51,7 +51,11 @@ func (s *Suite) Test_MkCondChecker_Check(c *check.C) {
 		"WARN: filename.mk:4: PKGSRC_RUN_TEST should be matched "+
 			"against \"[yY][eE][sS]\" or \"[nN][oO]\", not \"[Y][eE][sS]\".")
 
-	test(".if !empty(IS_BUILTIN.Xfixes:M[yY][eE][sS])")
+	test(".if !empty(IS_BUILTIN.Xfixes:M[yY][eE][sS])",
+		"NOTE: filename.mk:4: "+
+			"\"!empty(IS_BUILTIN.Xfixes:M[yY][eE][sS])\" "+
+			"can be simplified to "+
+			"\"${IS_BUILTIN.Xfixes:U:tl} == yes\".")
 
 	test(".if !empty(${IS_BUILTIN.Xfixes:M[yY][eE][sS]})",
 		"WARN: filename.mk:4: The empty() function takes a variable name as parameter, "+
diff --git a/pkgtools/pkglint/files/mkcondsimplifier.go b/pkgtools/pkglint/files/mkcondsimplifier.go
index 5e1e9ab3893..c62d2235a90 100644
--- a/pkgtools/pkglint/files/mkcondsimplifier.go
+++ b/pkgtools/pkglint/files/mkcondsimplifier.go
@@ -1,6 +1,9 @@
 package pkglint
 
-import "netbsd.org/pkglint/textproc"
+import (
+	"netbsd.org/pkglint/textproc"
+	"strings"
+)
 
 // MkCondSimplifier replaces unnecessarily complex conditions with simpler yet
 // equivalent conditions.
@@ -16,6 +19,9 @@ type MkCondSimplifier struct {
 //
 // * neg is true for the form !empty(VAR...), and false for empty(VAR...).
 func (s *MkCondSimplifier) SimplifyVarUse(varuse *MkVarUse, fromEmpty bool, neg bool) {
+	if s.simplifyYesNo(varuse, fromEmpty, neg) {
+		return
+	}
 	s.simplifyMatch(varuse, fromEmpty, neg)
 	s.simplifyWord(varuse, fromEmpty, neg)
 }
@@ -33,6 +39,9 @@ func (s *MkCondSimplifier) simplifyWord(varuse *MkVarUse, fromEmpty bool, neg bo
 	}
 	modsExceptLast := NewMkVarUse("", modifiers[:n-1]...).Mod()
 	vartype := G.Pkgsrc.VariableType(s.MkLines, varname)
+	if vartype == nil || vartype.IsList() {
+		return
+	}
 
 	// replace constructs the state before and after the autofix.
 	// The before state is constructed to ensure that only very simple
@@ -82,12 +91,10 @@ func (s *MkCondSimplifier) simplifyWord(varuse *MkVarUse, fromEmpty bool, neg bo
 	if !ok || !positive && n != 1 {
 		return
 	}
-
-	switch {
-	case !exact,
-		vartype == nil,
-		vartype.IsList(),
-		textproc.NewLexer(pattern).NextBytesSet(mkCondModifierPatternLiteral) != pattern:
+	if !exact {
+		return
+	}
+	if textproc.NewLexer(pattern).NextBytesSet(mkCondModifierPatternLiteral) != pattern {
 		return
 	}
 
@@ -112,6 +119,100 @@ func (s *MkCondSimplifier) simplifyWord(varuse *MkVarUse, fromEmpty bool, neg bo
 	fix.Apply()
 }
 
+// simplifyYesNo replaces conditions of the form '${VAR:M[yY][eE][sS]}' with
+// the equivalent ${VAR:tl} == yes.
+func (s *MkCondSimplifier) simplifyYesNo(varuse *MkVarUse, fromEmpty bool, neg bool) (done bool) {
+
+	// TODO: Merge the common code from simplifyWord and simplifyYesNo.
+	//  Even better would be to manipulate the conditions in an AST
+	//  instead of working directly with strings, but as of November 2022,
+	//  that is not implemented yet.
+	//  .
+	//  Another useful feature would be to chain multiple autofixes
+	//  together, but to do that, pkglint needs to be able to convert an
+	//  AST back into the source code form.
+
+	toLower := func(p string) string {
+		var sb strings.Builder
+		upper := textproc.Upper
+		lower := textproc.Lower
+		for ; len(p) >= 4 && p[0] == '[' && p[3] == ']'; p = p[4:] {
+			if upper.Contains(p[1]) && p[2] == p[1]-'A'+'a' {
+				sb.WriteByte(p[2])
+			} else if lower.Contains(p[1]) && p[2] == p[1]-'a'+'A' {
+				sb.WriteByte(p[1])
+			} else {
+				return ""
+			}
+		}
+		if p != "" {
+			return ""
+		}
+		return sb.String()
+	}
+
+	varname := varuse.varname
+	modifiers := varuse.modifiers
+
+	n := len(modifiers)
+	if n == 0 {
+		return
+	}
+	modsExceptLast := NewMkVarUse("", modifiers[:n-1]...).Mod()
+	vartype := G.Pkgsrc.VariableType(s.MkLines, varname)
+	if vartype == nil || vartype.IsList() {
+		return
+	}
+
+	// replace constructs the state before and after the autofix.
+	replace := func(positive bool, pattern, lower string) (bool, string, string) {
+		defined := s.isDefined(varname, vartype)
+		if !defined && !positive {
+			// Too many negations; maybe handle this case later.
+			return false, "", ""
+		}
+		uMod := condStr(!defined && !varuse.HasModifier("U"), ":U", "")
+
+		op := condStr(neg == positive, "==", "!=")
+
+		from := sprintf("%s%s%s%s%s%s%s",
+			condStr(neg != fromEmpty, "", "!"),
+			condStr(fromEmpty, "empty(", "${"),
+			varname,
+			modsExceptLast,
+			condStr(positive, ":M", ":N"),
+			pattern,
+			condStr(fromEmpty, ")", "}"))
+
+		to := sprintf(
+			"${%s%s%s:tl} %s %s",
+			varname, uMod, modsExceptLast, op, lower)
+
+		return true, from, to
+	}
+
+	modifier := modifiers[n-1]
+	ok, positive, pattern, exact := modifier.MatchMatch()
+	if !ok || !positive && n != 1 || exact {
+		return
+	}
+	lower := toLower(pattern)
+	if lower == "" {
+		return
+	}
+
+	ok, from, to := replace(positive, pattern, lower)
+	if !ok {
+		return
+	}
+
+	fix := s.MkLine.Autofix()
+	fix.Notef("\"%s\" can be simplified to \"%s\".", from, to)
+	fix.Replace(from, to)
+	fix.Apply()
+	return true
+}
+
 // simplifyMatch replaces:
 //  !empty(VAR:M*.c) with ${VAR:M*.c}
 //  empty(VAR:M*.c) with !${VAR:M*.c}
diff --git a/pkgtools/pkglint/files/mkcondsimplifier_test.go b/pkgtools/pkglint/files/mkcondsimplifier_test.go
index 87a16f217f2..1b77081c4f7 100644
--- a/pkgtools/pkglint/files/mkcondsimplifier_test.go
+++ b/pkgtools/pkglint/files/mkcondsimplifier_test.go
@@ -2,11 +2,21 @@ package pkglint
 
 import (
 	"gopkg.in/check.v1"
+	"netbsd.org/pkglint/regex"
 )
 
 type MkCondSimplifierTester struct {
 	c *check.C
 	*Tester
+	allowedVariableNames regex.Pattern
+}
+
+func NewMkCondSimplifierTester(c *check.C, s *Suite) *MkCondSimplifierTester {
+	return &MkCondSimplifierTester{
+		c,
+		s.Init(c),
+		`IN_SCOPE|PREFS|LATER|UNDEFINED`,
+	}
 }
 
 func (t *MkCondSimplifierTester) setUp() {
@@ -37,6 +47,10 @@ func (t *MkCondSimplifierTester) setUp() {
 	// Even when they are in scope, some variables such as PKGREVISION
 	// or MAKE_JOBS may be undefined.
 
+	// TODO: Test list variables; they differ in that a ':M' modifier
+	//  cannot be replaced with '==', as the variable may contain
+	//  multiple words.
+
 	t.SetUpVarType("IN_SCOPE_DEFINED", btAnything, AlwaysInScope|DefinedIfInScope,
 		"*.mk: use, use-loadtime")
 	t.SetUpVarType("IN_SCOPE", btAnything, AlwaysInScope,
@@ -69,8 +83,10 @@ func (t *MkCondSimplifierTester) testBeforeAndAfterPrefs(before, after string, d
 // before: the directive before the condition is simplified
 // after: the directive after the condition is simplified
 func (t *MkCondSimplifierTester) doTest(prefs bool, before, after string, diagnostics ...string) {
-	if !matches(before, `IN_SCOPE|PREFS|LATER|UNDEFINED`) {
-		t.c.Errorf("Condition %q must include one of the above variable names.", before)
+	if !matches(before, t.allowedVariableNames) {
+		// Prevent typos in the variable names used in the test.
+		t.c.Errorf("Condition %q must include one of the variable names %q.",
+			before, t.allowedVariableNames)
 	}
 	mklines := t.SetUpFileMkLines("filename.mk",
 		MkCvsID,
@@ -104,7 +120,7 @@ func (t *MkCondSimplifierTester) doTest(prefs bool, before, after string, diagno
 }
 
 func (s *Suite) Test_MkCondSimplifier_SimplifyVarUse(c *check.C) {
-	t := MkCondSimplifierTester{c, s.Init(c)}
+	t := NewMkCondSimplifierTester(c, s)
 
 	t.setUp()
 
@@ -118,22 +134,31 @@ func (s *Suite) Test_MkCondSimplifier_SimplifyVarUse(c *check.C) {
 		"AUTOFIX: filename.mk:6: Replacing \"${IN_SCOPE_DEFINED:Mpattern}\" "+
 			"with \"${IN_SCOPE_DEFINED} == pattern\".")
 
+	// From simplifyYesNo.
 	t.testBeforeAndAfterPrefs(
 		".if !empty(IN_SCOPE_DEFINED:M[Nn][Oo])",
-		".if ${IN_SCOPE_DEFINED:M[Nn][Oo]}",
+		".if ${IN_SCOPE_DEFINED:tl} == no",
 
 		"NOTE: filename.mk:6: \"!empty(IN_SCOPE_DEFINED:M[Nn][Oo])\" "+
-			"can be simplified to \"${IN_SCOPE_DEFINED:M[Nn][Oo]}\".",
+			"can be simplified to \"${IN_SCOPE_DEFINED:tl} == no\".",
 		"AUTOFIX: filename.mk:6: "+
 			"Replacing \"!empty(IN_SCOPE_DEFINED:M[Nn][Oo])\" "+
-			"with \"${IN_SCOPE_DEFINED:M[Nn][Oo]}\".")
+			"with \"${IN_SCOPE_DEFINED:tl} == no\".")
 }
 
-func (s *Suite) Test_MkCondSimplifier_simplifyWord(c *check.C) {
-	t := MkCondSimplifierTester{c, s.Init(c)}
+// Show in which cases the ':U' modifier is needed, and how including
+// bsd.prefs.mk influences the resulting conditions.
+//
+// The ':U' modifier can be omitted if the variable is guaranteed to be
+// defined.
+func (s *Suite) Test_MkCondSimplifier_simplifyWord__undefined(c *check.C) {
+	t := NewMkCondSimplifierTester(c, s)
 
+	// Define the variables that are used in the below tests.
 	t.setUp()
 
+	// The variable is guaranteed to be defined,
+	// therefore no ':U' modifier is needed.
 	t.testBeforeAndAfterPrefs(
 		".if ${IN_SCOPE_DEFINED:Mpattern}",
 		".if ${IN_SCOPE_DEFINED} == pattern",
@@ -144,6 +169,8 @@ func (s *Suite) Test_MkCondSimplifier_simplifyWord(c *check.C) {
 		"AUTOFIX: filename.mk:6: Replacing \"${IN_SCOPE_DEFINED:Mpattern}\" "+
 			"with \"${IN_SCOPE_DEFINED} == pattern\".")
 
+	// The variable may be undefined,
+	// therefore the ':U' modifier is needed.
 	t.testBeforeAndAfterPrefs(
 		".if ${IN_SCOPE:Mpattern}",
 		".if ${IN_SCOPE:U} == pattern",
@@ -155,9 +182,10 @@ func (s *Suite) Test_MkCondSimplifier_simplifyWord(c *check.C) {
 			"with \"${IN_SCOPE:U} == pattern\".")
 
 	// Even though PREFS_DEFINED is declared as DefinedIfInScope,
-	// it is not in scope yet. Therefore it needs the :U modifier.
-	// The warning that this variable is not yet in scope comes from
-	// a different part of pkglint.
+	// it is not yet in scope, due to the "BeforePrefs".
+	// Therefore, the ':U' modifier is needed.
+	// The warning about "at load time" comes from a different part of
+	// pkglint.
 	t.testBeforePrefs(
 		".if ${PREFS_DEFINED:Mpattern}",
 		".if ${PREFS_DEFINED:U} == pattern",
@@ -170,6 +198,8 @@ func (s *Suite) Test_MkCondSimplifier_simplifyWord(c *check.C) {
 		"AUTOFIX: filename.mk:6: Replacing \"${PREFS_DEFINED:Mpattern}\" "+
 			"with \"${PREFS_DEFINED:U} == pattern\".")
 
+	// Now that bsd.prefs.mk has been included ("AfterPrefs"),
+	// the ':U' modifier is not needed anymore.
 	t.testAfterPrefs(
 		".if ${PREFS_DEFINED:Mpattern}",
 		".if ${PREFS_DEFINED} == pattern",
@@ -180,6 +210,8 @@ func (s *Suite) Test_MkCondSimplifier_simplifyWord(c *check.C) {
 		"AUTOFIX: filename.mk:6: Replacing \"${PREFS_DEFINED:Mpattern}\" "+
 			"with \"${PREFS_DEFINED} == pattern\".")
 
+	// The 'PREFS' variable is probably undefined before bsd.prefs.mk,
+	// and after bsd.prefs.mk it _may_ be defined.
 	t.testBeforePrefs(
 		".if ${PREFS:Mpattern}",
 		".if ${PREFS:U} == pattern",
@@ -192,7 +224,7 @@ func (s *Suite) Test_MkCondSimplifier_simplifyWord(c *check.C) {
 		"AUTOFIX: filename.mk:6: Replacing \"${PREFS:Mpattern}\" "+
 			"with \"${PREFS:U} == pattern\".")
 
-	// Preferences that may be undefined always need the :U modifier,
+	// Preferences that may be undefined always need the ':U' modifier,
 	// even when they are in scope.
 	t.testAfterPrefs(
 		".if ${PREFS:Mpattern}",
@@ -204,8 +236,11 @@ func (s *Suite) Test_MkCondSimplifier_simplifyWord(c *check.C) {
 		"AUTOFIX: filename.mk:6: Replacing \"${PREFS:Mpattern}\" "+
 			"with \"${PREFS:U} == pattern\".")
 
-	// Variables that are defined later always need the :U modifier.
-	// It is probably a mistake to use them in conditions at all.
+	// The variable is declared as being defined later (bsd.pkg.mk),
+	// but that point is not yet reached.
+	// Therefore, the ':U' modifier is needed,
+	// even if the variable is guaranteed to be defined later.
+	// It is probably a mistake to use it in conditions at all.
 	t.testBeforeAndAfterPrefs(
 		".if ${LATER_DEFINED:Mpattern}",
 		".if ${LATER_DEFINED:U} == pattern",
@@ -218,8 +253,10 @@ func (s *Suite) Test_MkCondSimplifier_simplifyWord(c *check.C) {
 		"AUTOFIX: filename.mk:6: Replacing \"${LATER_DEFINED:Mpattern}\" "+
 			"with \"${LATER_DEFINED:U} == pattern\".")
 
-	// Variables that are defined later always need the :U modifier.
-	// It is probably a mistake to use them in conditions at all.
+	// The variable is declared as being defined later (bsd.pkg.mk),
+	// but that point is not yet reached.
+	// Therefore, the ':U' modifier is needed.
+	// It is probably a mistake to use it in conditions at all.
 	t.testBeforeAndAfterPrefs(
 		".if ${LATER:Mpattern}",
 		".if ${LATER:U} == pattern",
@@ -232,175 +269,194 @@ func (s *Suite) Test_MkCondSimplifier_simplifyWord(c *check.C) {
 		"AUTOFIX: filename.mk:6: Replacing \"${LATER:Mpattern}\" "+
 			"with \"${LATER:U} == pattern\".")
 
+	// The variable is neither defined nor known.
+	// Since the replacement only works for single-word variables
+	// but not for lists, leave the condition as-is.
 	t.testBeforeAndAfterPrefs(
 		".if ${UNDEFINED:Mpattern}",
 		".if ${UNDEFINED:Mpattern}",
 
 		"WARN: filename.mk:6: UNDEFINED is used but not defined.")
+}
+
+// Show how different kinds of ':M'-style patterns are replaced with simpler
+// comparisons.
+func (s *Suite) Test_MkCondSimplifier_simplifyWord__patterns(c *check.C) {
+	t := NewMkCondSimplifierTester(c, s)
+
+	// Define the variables that are used in the below tests.
+	t.setUp()
 
-	// When the pattern contains placeholders, it cannot be converted to == or !=.
+	// When the pattern contains placeholders such as '*',
+	// it cannot be converted to == or !=.
 	t.testAfterPrefs(
 		".if ${PREFS_DEFINED:Mpa*n}",
 		".if ${PREFS_DEFINED:Mpa*n}",
 
 		nil...)
 
-	// When deciding whether to replace the expression, only the
-	// last modifier is inspected. All the others are copied.
-	t.testAfterPrefs(
-		".if ${PREFS_DEFINED:tl:Mpattern}",
-		".if ${PREFS_DEFINED:tl} == pattern",
+	// This pattern with spaces doesn't make sense at all in the :M
+	// modifier since it can never match.
+	// Or can it, if the PKGPATH contains quotes?
+	// TODO: How exactly does bmake apply the matching here,
+	//  are both values unquoted first? Probably not, but who knows.
+	t.testBeforeAndAfterPrefs(
+		".if ${IN_SCOPE_DEFINED:Mpattern with spaces}",
+		".if ${IN_SCOPE_DEFINED:Mpattern with spaces}",
 
-		"NOTE: filename.mk:6: PREFS_DEFINED can be "+
-			"compared using the simpler \"${PREFS_DEFINED:tl} == pattern\" "+
-			"instead of matching against \":Mpattern\".",
-		"AUTOFIX: filename.mk:6: Replacing \"${PREFS_DEFINED:tl:Mpattern}\" "+
-			"with \"${PREFS_DEFINED:tl} == pattern\".")
+		nil...)
+	// TODO: ".if ${PKGPATH} == \"pattern with spaces\"")
 
-	// Negated pattern matches are supported as well,
-	// as long as the variable is guaranteed to be nonempty.
-	// TODO: Actually implement this.
-	//  As of December 2019, IsNonemptyIfDefined is not used anywhere.
-	t.testAfterPrefs(
-		".if ${PREFS_DEFINED:Npattern}",
-		".if ${PREFS_DEFINED} != pattern",
+	t.testBeforeAndAfterPrefs(
+		".if ${IN_SCOPE_DEFINED:M'pattern with spaces'}",
+		".if ${IN_SCOPE_DEFINED:M'pattern with spaces'}",
 
-		"NOTE: filename.mk:6: PREFS_DEFINED can be "+
-			"compared using the simpler \"${PREFS_DEFINED} != pattern\" "+
-			"instead of matching against \":Npattern\".",
-		"AUTOFIX: filename.mk:6: Replacing \"${PREFS_DEFINED:Npattern}\" "+
-			"with \"${PREFS_DEFINED} != pattern\".")
+		nil...)
+	// TODO: ".if ${PKGPATH} == 'pattern with spaces'")
 
-	// ${PREFS_DEFINED:None:Ntwo} is a short variant of
-	// ${PREFS_DEFINED} != "one" && ${PREFS_DEFINED} != "two".
-	// Applying the transformation would make the condition longer
-	// than before, therefore nothing can be simplified here,
-	// even though all patterns are exact matches.
-	t.testAfterPrefs(
-		".if ${PREFS_DEFINED:None:Ntwo}",
-		".if ${PREFS_DEFINED:None:Ntwo}",
+	t.testBeforeAndAfterPrefs(
+		".if ${IN_SCOPE_DEFINED:M&&}",
+		".if ${IN_SCOPE_DEFINED:M&&}",
 
 		nil...)
+	// TODO: ".if ${PKGPATH} == '&&'")
 
-	// Note: this combination doesn't make sense since the patterns
-	// "one" and "two" don't overlap.
-	// Nevertheless it is possible and valid to simplify the condition.
+	// Numbers must be enclosed in quotes, otherwise they are compared
+	// as numbers, not as strings.
+	// The :M and :N modifiers always compare strings.
 	t.testAfterPrefs(
-		".if ${PREFS_DEFINED:Mone:Mtwo}",
-		".if ${PREFS_DEFINED:Mone} == two",
+		".if empty(PREFS_DEFINED:M64)",
+		".if ${PREFS_DEFINED} != \"64\"",
 
 		"NOTE: filename.mk:6: PREFS_DEFINED can be "+
-			"compared using the simpler \"${PREFS_DEFINED:Mone} == two\" "+
-			"instead of matching against \":Mtwo\".",
-		"AUTOFIX: filename.mk:6: Replacing \"${PREFS_DEFINED:Mone:Mtwo}\" "+
-			"with \"${PREFS_DEFINED:Mone} == two\".")
+			"compared using the simpler \"${PREFS_DEFINED} != \"64\"\" "+
+			"instead of matching against \":M64\".",
+		"AUTOFIX: filename.mk:6: Replacing \"empty(PREFS_DEFINED:M64)\" "+
+			"with \"${PREFS_DEFINED} != \\\"64\\\"\".")
 
-	// There is no ! before the empty, which is easy to miss.
-	// Because of this missing negation, the comparison operator is !=.
+	// Fractional numbers must also be enclosed in quotes.
 	t.testAfterPrefs(
-		".if empty(PREFS_DEFINED:Mpattern)",
-		".if ${PREFS_DEFINED} != pattern",
+		".if empty(PREFS_DEFINED:M19.12)",
+		".if ${PREFS_DEFINED} != \"19.12\"",
 
 		"NOTE: filename.mk:6: PREFS_DEFINED can be "+
-			"compared using the simpler \"${PREFS_DEFINED} != pattern\" "+
-			"instead of matching against \":Mpattern\".",
-		"AUTOFIX: filename.mk:6: Replacing \"empty(PREFS_DEFINED:Mpattern)\" "+
-			"with \"${PREFS_DEFINED} != pattern\".")
+			"compared using the simpler \"${PREFS_DEFINED} != \"19.12\"\" "+
+			"instead of matching against \":M19.12\".",
+		"AUTOFIX: filename.mk:6: Replacing \"empty(PREFS_DEFINED:M19.12)\" "+
+			"with \"${PREFS_DEFINED} != \\\"19.12\\\"\".")
 
+	// The quotes are only needed if the whole pattern is a number,
+	// not if the number is surrounded by other text.
+	// The dot is just an ordinary character in a pattern.
 	t.testAfterPrefs(
-		".if !!empty(PREFS_DEFINED:Mpattern)",
-		// TODO: The ! and == could be combined into a !=.
-		//  Luckily the !! pattern doesn't occur in practice.
-		".if !${PREFS_DEFINED} == pattern",
+		".if !empty(PREFS_DEFINED:Mpackage1.2)",
+		".if ${PREFS_DEFINED} == package1.2",
 
-		// TODO: When taking all the ! into account, this is actually a
-		//  test for emptiness, therefore the diagnostics should suggest
-		//  the != operator instead of ==.
 		"NOTE: filename.mk:6: PREFS_DEFINED can be "+
-			"compared using the simpler \"${PREFS_DEFINED} == pattern\" "+
-			"instead of matching against \":Mpattern\".",
-		"AUTOFIX: filename.mk:6: Replacing \"!empty(PREFS_DEFINED:Mpattern)\" "+
-			"with \"${PREFS_DEFINED} == pattern\".")
+			"compared using the simpler \"${PREFS_DEFINED} == package1.2\" "+
+			"instead of matching against \":Mpackage1.2\".",
+		"AUTOFIX: filename.mk:6: Replacing \"!empty(PREFS_DEFINED:Mpackage1.2)\" "+
+			"with \"${PREFS_DEFINED} == package1.2\".")
 
-	// Simplifying the condition also works in complex expressions.
-	t.testAfterPrefs(".if empty(PREFS_DEFINED:Mpattern) || 0",
-		".if ${PREFS_DEFINED} != pattern || 0",
+	// Special characters must be enclosed in quotes when they are
+	// used in string literals.
+	// As of December 2019, strings with special characters are not yet
+	// replaced automatically, see mkCondLiteralChars.
+	// TODO: Add tests for all characters that are special in string literals or patterns.
+	// TODO: Then, extend the set of characters for which the expressions are simplified.
+	t.testAfterPrefs(
+		".if ${PREFS_DEFINED:M&&}",
+		".if ${PREFS_DEFINED:M&&}",
 
-		"NOTE: filename.mk:6: PREFS_DEFINED can be "+
-			"compared using the simpler \"${PREFS_DEFINED} != pattern\" "+
-			"instead of matching against \":Mpattern\".",
-		"AUTOFIX: filename.mk:6: Replacing \"empty(PREFS_DEFINED:Mpattern)\" "+
-			"with \"${PREFS_DEFINED} != pattern\".")
+		nil...)
 
-	// No note in this case since there is no implicit !empty around the varUse.
-	// There is no obvious way of writing this expression in a simpler form.
+	// The '+' is contained in both mkCondStringLiteralUnquoted and
+	// mkCondModifierPatternLiteral, therefore it is copied verbatim.
 	t.testAfterPrefs(
-		".if ${PREFS_DEFINED:Mpattern} != ${OTHER}",
-		".if ${PREFS_DEFINED:Mpattern} != ${OTHER}",
+		".if ${PREFS_DEFINED:Mcategory/gtk+}",
+		".if ${PREFS_DEFINED} == category/gtk+",
 
-		"WARN: filename.mk:6: OTHER is used but not defined.")
+		"NOTE: filename.mk:6: PREFS_DEFINED can be "+
+			"compared using the simpler \"${PREFS_DEFINED} == category/gtk+\" "+
+			"instead of matching against \":Mcategory/gtk+\".",
+		"AUTOFIX: filename.mk:6: "+
+			"Replacing \"${PREFS_DEFINED:Mcategory/gtk+}\" "+
+			"with \"${PREFS_DEFINED} == category/gtk+\".")
 
-	// The condition is also simplified if it doesn't use the !empty
-	// form but the implicit conversion to boolean.
+	// The characters '<=>' may be used unescaped in ':M' and ':N' patterns
+	// but not in '.if' conditions. There, they must be enclosed in quotes.
 	t.testAfterPrefs(
-		".if ${PREFS_DEFINED:Mpattern}",
-		".if ${PREFS_DEFINED} == pattern",
+		".if ${PREFS_DEFINED:M<=>}",
+		".if ${PREFS_DEFINED} == \"<=>\"",
 
 		"NOTE: filename.mk:6: PREFS_DEFINED can be "+
-			"compared using the simpler \"${PREFS_DEFINED} == pattern\" "+
-			"instead of matching against \":Mpattern\".",
-		"AUTOFIX: filename.mk:6: Replacing \"${PREFS_DEFINED:Mpattern}\" "+
-			"with \"${PREFS_DEFINED} == pattern\".")
+			"compared using the simpler \"${PREFS_DEFINED} == \"<=>\"\" "+
+			"instead of matching against \":M<=>\".",
+		"AUTOFIX: filename.mk:6: "+
+			"Replacing \"${PREFS_DEFINED:M<=>}\" "+
+			"with \"${PREFS_DEFINED} == \\\"<=>\\\"\".")
 
-	// A single negation outside the implicit conversion is taken into
-	// account when simplifying the condition.
+	// When replacing a pattern containing '"', which is unusual,
+	// the resulting string literal must be escaped properly.
+	t.testBeforeAndAfterPrefs(
+		".if ${IN_SCOPE_DEFINED:M\"}",
+		".if ${IN_SCOPE_DEFINED:M\"}",
+
+		nil...)
+
+	// Matching for the empty pattern doesn't make sense,
+	// as the resulting string is always empty.
+	// Nevertheless, pkglint simplifies it.
+	// FIXME: This replacement does not preserve the behavior.
+	t.testBeforeAndAfterPrefs(
+		".if !empty(IN_SCOPE_DEFINED:M)",
+		".if ${IN_SCOPE_DEFINED} == \"\"",
+
+		"NOTE: filename.mk:6: IN_SCOPE_DEFINED can be "+
+			"compared using the simpler "+"\"${IN_SCOPE_DEFINED} == \"\"\" "+
+			"instead of matching against \":M\".",
+		"AUTOFIX: filename.mk:6: "+
+			"Replacing \"!empty(IN_SCOPE_DEFINED:M)\" "+
+			"with \"${IN_SCOPE_DEFINED} == \\\"\\\"\".")
+}
+
+// Show in which cases the ':N' modifier is replaced.
+// That modifier is used less often than ':M',
+// therefore pkglint doesn't do much about it.
+func (s *Suite) Test_MkCondSimplifier_simplifyWord__N(c *check.C) {
+	t := NewMkCondSimplifierTester(c, s)
+
+	// Define the variables that are used in the below tests.
+	t.setUp()
+
+	// Negated pattern matches are supported as well,
+	// as long as the variable is guaranteed to be nonempty.
+	// TODO: Actually implement the "as long as".
+	//  As of December 2019, IsNonemptyIfDefined is not used anywhere,
+	//  which means that this replacement wrongly applies in cases where
+	//  the variable may be empty.
 	t.testAfterPrefs(
-		".if !${PREFS_DEFINED:Mpattern}",
+		".if ${PREFS_DEFINED:Npattern}",
 		".if ${PREFS_DEFINED} != pattern",
 
 		"NOTE: filename.mk:6: PREFS_DEFINED can be "+
 			"compared using the simpler \"${PREFS_DEFINED} != pattern\" "+
-			"instead of matching against \":Mpattern\".",
-		"AUTOFIX: filename.mk:6: Replacing \"!${PREFS_DEFINED:Mpattern}\" "+
+			"instead of matching against \":Npattern\".",
+		"AUTOFIX: filename.mk:6: Replacing \"${PREFS_DEFINED:Npattern}\" "+
 			"with \"${PREFS_DEFINED} != pattern\".")
 
-	// TODO: Merge the double negation into the comparison operator.
+	// There is no '!' before the empty, which is easy to miss.
+	// Because there is no '!', the comparison operator is !=.
 	t.testAfterPrefs(
-		".if !!${PREFS_DEFINED:Mpattern}",
-		".if !${PREFS_DEFINED} != pattern",
+		".if empty(PREFS_DEFINED:Mpattern)",
+		".if ${PREFS_DEFINED} != pattern",
 
 		"NOTE: filename.mk:6: PREFS_DEFINED can be "+
 			"compared using the simpler \"${PREFS_DEFINED} != pattern\" "+
 			"instead of matching against \":Mpattern\".",
-		"AUTOFIX: filename.mk:6: Replacing \"!${PREFS_DEFINED:Mpattern}\" "+
+		"AUTOFIX: filename.mk:6: Replacing \"empty(PREFS_DEFINED:Mpattern)\" "+
 			"with \"${PREFS_DEFINED} != pattern\".")
 
-	// This pattern with spaces doesn't make sense at all in the :M
-	// modifier since it can never match.
-	// Or can it, if the PKGPATH contains quotes?
-	// TODO: How exactly does bmake apply the matching here,
-	//  are both values unquoted first? Probably not, but who knows.
-	t.testBeforeAndAfterPrefs(
-		".if ${IN_SCOPE_DEFINED:Mpattern with spaces}",
-		".if ${IN_SCOPE_DEFINED:Mpattern with spaces}",
-
-		nil...)
-	// TODO: ".if ${PKGPATH} == \"pattern with spaces\"")
-
-	t.testBeforeAndAfterPrefs(
-		".if ${IN_SCOPE_DEFINED:M'pattern with spaces'}",
-		".if ${IN_SCOPE_DEFINED:M'pattern with spaces'}",
-
-		nil...)
-	// TODO: ".if ${PKGPATH} == 'pattern with spaces'")
-
-	t.testBeforeAndAfterPrefs(
-		".if ${IN_SCOPE_DEFINED:M&&}",
-		".if ${IN_SCOPE_DEFINED:M&&}",
-
-		nil...)
-	// TODO: ".if ${PKGPATH} == '&&'")
-
 	// The :N modifier involves another negation and is therefore more
 	// difficult to understand. That's even more reason to use the
 	// well-known == and != comparison operators instead.
@@ -432,47 +488,79 @@ func (s *Suite) Test_MkCondSimplifier_simplifyWord(c *check.C) {
 	// Since UNDEFINED is not a well-known variable that is
 	// guaranteed to be non-empty (see the previous example), it is not
 	// transformed at all.
-	t.testBeforePrefs(
+	t.testBeforeAndAfterPrefs(
 		".if ${UNDEFINED:Nnegative-pattern}",
 		".if ${UNDEFINED:Nnegative-pattern}",
 
 		"WARN: filename.mk:6: UNDEFINED is used but not defined.")
 
 	t.testAfterPrefs(
-		".if ${UNDEFINED:Nnegative-pattern}",
-		".if ${UNDEFINED:Nnegative-pattern}",
+		".if !empty(LATER:Npattern)",
+		".if !empty(LATER:Npattern)",
 
-		"WARN: filename.mk:6: UNDEFINED is used but not defined.")
+		// No diagnostics about the :N modifier yet,
+		// see MkCondSimplifier.simplifyWord.replace.
+		"WARN: filename.mk:6: LATER should not be used "+
+			"at load time in any file.")
 
-	// A complex condition may contain several simple conditions
-	// that are each simplified independently, in the same go.
+	// TODO: Add a note that the :U is unnecessary, and explain why.
 	t.testAfterPrefs(
-		".if ${PREFS_DEFINED:Mpath1} || ${PREFS_DEFINED:Mpath2}",
-		".if ${PREFS_DEFINED} == path1 || ${PREFS_DEFINED} == path2",
+		".if ${PREFS_DEFINED:U:Mpattern}",
+		".if ${PREFS_DEFINED:U} == pattern",
 
 		"NOTE: filename.mk:6: PREFS_DEFINED can be "+
-			"compared using the simpler \"${PREFS_DEFINED} == path1\" "+
-			"instead of matching against \":Mpath1\".",
+			"compared using the simpler \"${PREFS_DEFINED:U} == pattern\" "+
+			"instead of matching against \":Mpattern\".",
+		"AUTOFIX: filename.mk:6: "+
+			"Replacing \"${PREFS_DEFINED:U:Mpattern}\" "+
+			"with \"${PREFS_DEFINED:U} == pattern\".")
+}
+
+// Show how the conditions are simplified when the expression contains
+// several modifiers.
+func (s *Suite) Test_MkCondSimplifier_simplifyWord__modifiers(c *check.C) {
+	t := NewMkCondSimplifierTester(c, s)
+
+	// Define the variables that are used in the below tests.
+	t.setUp()
+
+	// When deciding whether to replace the expression,
+	// only the last modifier is inspected.
+	// All the others are copied, as the modifier ':M'
+	// does not change whether the expression is defined or not.
+	t.testAfterPrefs(
+		".if ${PREFS_DEFINED:tl:Mpattern}",
+		".if ${PREFS_DEFINED:tl} == pattern",
+
 		"NOTE: filename.mk:6: PREFS_DEFINED can be "+
-			"compared using the simpler \"${PREFS_DEFINED} == path2\" "+
-			"instead of matching against \":Mpath2\".",
-		"AUTOFIX: filename.mk:6: Replacing \"${PREFS_DEFINED:Mpath1}\" "+
-			"with \"${PREFS_DEFINED} == path1\".",
-		"AUTOFIX: filename.mk:6: Replacing \"${PREFS_DEFINED:Mpath2}\" "+
-			"with \"${PREFS_DEFINED} == path2\".")
+			"compared using the simpler \"${PREFS_DEFINED:tl} == pattern\" "+
+			"instead of matching against \":Mpattern\".",
+		"AUTOFIX: filename.mk:6: Replacing \"${PREFS_DEFINED:tl:Mpattern}\" "+
+			"with \"${PREFS_DEFINED:tl} == pattern\".")
 
-	// Removing redundant parentheses may be useful one day but is
-	// not urgent.
-	// Simplifying the inner expression keeps all parentheses as-is.
+	// ${PREFS_DEFINED:None:Ntwo} is a short variant of
+	// ${PREFS_DEFINED} != "one" && ${PREFS_DEFINED} != "two".
+	// Applying the transformation would make the condition longer
+	// than before, therefore nothing can be simplified here,
+	// even though all patterns are exact matches.
 	t.testAfterPrefs(
-		".if (((((${PREFS_DEFINED:Mpath})))))",
-		".if (((((${PREFS_DEFINED} == path)))))",
+		".if ${PREFS_DEFINED:None:Ntwo}",
+		".if ${PREFS_DEFINED:None:Ntwo}",
+
+		nil...)
+
+	// Note: this combination doesn't make sense since the patterns
+	// "one" and "two" don't overlap.
+	// Nevertheless, it is possible and valid to simplify the condition.
+	t.testAfterPrefs(
+		".if ${PREFS_DEFINED:Mone:Mtwo}",
+		".if ${PREFS_DEFINED:Mone} == two",
 
 		"NOTE: filename.mk:6: PREFS_DEFINED can be "+
-			"compared using the simpler \"${PREFS_DEFINED} == path\" "+
-			"instead of matching against \":Mpath\".",
-		"AUTOFIX: filename.mk:6: Replacing \"${PREFS_DEFINED:Mpath}\" "+
-			"with \"${PREFS_DEFINED} == path\".")
+			"compared using the simpler \"${PREFS_DEFINED:Mone} == two\" "+
+			"instead of matching against \":Mtwo\".",
+		"AUTOFIX: filename.mk:6: Replacing \"${PREFS_DEFINED:Mone:Mtwo}\" "+
+			"with \"${PREFS_DEFINED:Mone} == two\".")
 
 	// Several modifiers like :S and :C may change the variable value.
 	// Whether the condition can be simplified or not only depends on the
@@ -504,63 +592,14 @@ func (s *Suite) Test_MkCondSimplifier_simplifyWord(c *check.C) {
 		".if !empty(PREFS_DEFINED:O:MUnknown:S,a,b,)",
 
 		nil...)
+}
 
-	// The dot is just an ordinary character in a pattern.
-	// In comparisons, an unquoted 1.2 is interpreted as a number though.
-	t.testAfterPrefs(
-		".if !empty(PREFS_DEFINED:Mpackage1.2)",
-		".if ${PREFS_DEFINED} == package1.2",
-
-		"NOTE: filename.mk:6: PREFS_DEFINED can be "+
-			"compared using the simpler \"${PREFS_DEFINED} == package1.2\" "+
-			"instead of matching against \":Mpackage1.2\".",
-		"AUTOFIX: filename.mk:6: Replacing \"!empty(PREFS_DEFINED:Mpackage1.2)\" "+
-			"with \"${PREFS_DEFINED} == package1.2\".")
-
-	// Numbers must be enclosed in quotes, otherwise they are compared
-	// as numbers, not as strings.
-	// The :M and :N modifiers always compare strings.
-	t.testAfterPrefs(
-		".if empty(PREFS:U:M64)",
-		".if ${PREFS:U} != \"64\"",
-
-		"NOTE: filename.mk:6: PREFS can be "+
-			"compared using the simpler \"${PREFS:U} != \"64\"\" "+
-			"instead of matching against \":M64\".",
-		"AUTOFIX: filename.mk:6: Replacing \"empty(PREFS:U:M64)\" "+
-			"with \"${PREFS:U} != \\\"64\\\"\".")
-
-	// Fractional numbers must also be enclosed in quotes.
-	t.testAfterPrefs(
-		".if empty(PREFS:U:M19.12)",
-		".if ${PREFS:U} != \"19.12\"",
-
-		"NOTE: filename.mk:6: PREFS can be "+
-			"compared using the simpler \"${PREFS:U} != \"19.12\"\" "+
-			"instead of matching against \":M19.12\".",
-		"AUTOFIX: filename.mk:6: Replacing \"empty(PREFS:U:M19.12)\" "+
-			"with \"${PREFS:U} != \\\"19.12\\\"\".")
-
-	t.testAfterPrefs(
-		".if !empty(LATER:Npattern)",
-		".if !empty(LATER:Npattern)",
-
-		// No diagnostics about the :N modifier yet,
-		// see MkCondChecker.simplify.replace.
-		"WARN: filename.mk:6: LATER should not be used "+
-			"at load time in any file.")
-
-	// TODO: Add a note that the :U is unnecessary, and explain why.
-	t.testAfterPrefs(
-		".if ${PREFS_DEFINED:U:Mpattern}",
-		".if ${PREFS_DEFINED:U} == pattern",
+// Show how expressions in complex conditions are simplified.
+func (s *Suite) Test_MkCondSimplifier_simplifyWord__complex(c *check.C) {
+	t := NewMkCondSimplifierTester(c, s)
 
-		"NOTE: filename.mk:6: PREFS_DEFINED can be "+
-			"compared using the simpler \"${PREFS_DEFINED:U} == pattern\" "+
-			"instead of matching against \":Mpattern\".",
-		"AUTOFIX: filename.mk:6: "+
-			"Replacing \"${PREFS_DEFINED:U:Mpattern}\" "+
-			"with \"${PREFS_DEFINED:U} == pattern\".")
+	// Define the variables that are used in the below tests.
+	t.setUp()
 
 	// Conditions without any modifiers cannot be simplified
 	// and are therefore skipped.
@@ -570,93 +609,107 @@ func (s *Suite) Test_MkCondSimplifier_simplifyWord(c *check.C) {
 
 		nil...)
 
-	// Special characters must be enclosed in quotes when they are
-	// used in string literals.
-	// As of December 2019, strings with special characters are not yet
-	// replaced automatically, see mkCondLiteralChars.
-	// TODO: Add tests for all characters that are special in string literals or patterns.
-	// TODO: Then, extend the set of characters for which the expressions are simplified.
-	t.testBeforePrefs(
-		".if ${PREFS_DEFINED:M&&}",
-		".if ${PREFS_DEFINED:M&&}",
-
-		"WARN: filename.mk:6: To use PREFS_DEFINED at load time, .include \"../../mk/bsd.prefs.mk\" first.")
+	// Double negations are not needed in practice,
+	// therefore pkglint doesn't care about simplifying them.
 	t.testAfterPrefs(
-		".if ${PREFS_DEFINED:M&&}",
-		".if ${PREFS_DEFINED:M&&}",
+		".if !!empty(PREFS_DEFINED:Mpattern)",
+		// The '!' and '==' could be combined into a '!='.
+		".if !${PREFS_DEFINED} == pattern",
 
-		nil...)
+		// TODO: When taking all the ! into account, this is actually a
+		//  test for emptiness, therefore the diagnostics should suggest
+		//  '!= pattern' instead of '== pattern'.
+		"NOTE: filename.mk:6: PREFS_DEFINED can be "+
+			"compared using the simpler \"${PREFS_DEFINED} == pattern\" "+
+			"instead of matching against \":Mpattern\".",
+		"AUTOFIX: filename.mk:6: Replacing \"!empty(PREFS_DEFINED:Mpattern)\" "+
+			"with \"${PREFS_DEFINED} == pattern\".")
 
-	t.testBeforePrefs(
-		".if ${PREFS:M&&}",
-		".if ${PREFS:M&&}",
+	// Simplifying the condition also works in complex expressions.
+	t.testAfterPrefs(
+		".if empty(PREFS_DEFINED:Mpattern) || 0",
+		".if ${PREFS_DEFINED} != pattern || 0",
+
+		"NOTE: filename.mk:6: PREFS_DEFINED can be "+
+			"compared using the simpler \"${PREFS_DEFINED} != pattern\" "+
+			"instead of matching against \":Mpattern\".",
+		"AUTOFIX: filename.mk:6: Replacing \"empty(PREFS_DEFINED:Mpattern)\" "+
+			"with \"${PREFS_DEFINED} != pattern\".")
 
-		// TODO: Warn that the :U is missing.
-		"WARN: filename.mk:6: To use PREFS at load time, .include \"../../mk/bsd.prefs.mk\" first.")
+	// If the expression ${VAR:Mpattern} is part of a comparison using the
+	// '!=' or '==' operators, there is no implicit '!empty' around the
+	// expression.
+	// This condition cannot be simplified.
 	t.testAfterPrefs(
-		".if ${PREFS:M&&}",
-		".if ${PREFS:M&&}",
+		".if ${PREFS_DEFINED:Mpattern} != ${OTHER}",
+		".if ${PREFS_DEFINED:Mpattern} != ${OTHER}",
 
-		// TODO: Warn that the :U is missing.
-		nil...)
+		"WARN: filename.mk:6: OTHER is used but not defined.")
 
-	// The + is contained in both mkCondStringLiteralUnquoted and
-	// mkCondModifierPatternLiteral, therefore it is copied verbatim.
+	// The condition is also simplified if it doesn't use the '!empty'
+	// form but the implicit conversion to boolean.
 	t.testAfterPrefs(
-		".if ${PREFS_DEFINED:Mcategory/gtk+}",
-		".if ${PREFS_DEFINED} == category/gtk+",
+		".if ${PREFS_DEFINED:Mpattern}",
+		".if ${PREFS_DEFINED} == pattern",
 
 		"NOTE: filename.mk:6: PREFS_DEFINED can be "+
-			"compared using the simpler \"${PREFS_DEFINED} == category/gtk+\" "+
-			"instead of matching against \":Mcategory/gtk+\".",
-		"AUTOFIX: filename.mk:6: "+
-			"Replacing \"${PREFS_DEFINED:Mcategory/gtk+}\" "+
-			"with \"${PREFS_DEFINED} == category/gtk+\".")
+			"compared using the simpler \"${PREFS_DEFINED} == pattern\" "+
+			"instead of matching against \":Mpattern\".",
+		"AUTOFIX: filename.mk:6: Replacing \"${PREFS_DEFINED:Mpattern}\" "+
+			"with \"${PREFS_DEFINED} == pattern\".")
 
-	// The characters <=> may be used unescaped in :M and :N patterns
-	// but not in .if conditions. There they must be enclosed in quotes.
-	t.testBeforePrefs(
-		".if ${PREFS_DEFINED:M<=>}",
-		".if ${PREFS_DEFINED:U} == \"<=>\"",
+	// A single negation outside the implicit conversion is taken into
+	// account when simplifying the condition.
+	t.testAfterPrefs(
+		".if !${PREFS_DEFINED:Mpattern}",
+		".if ${PREFS_DEFINED} != pattern",
 
 		"NOTE: filename.mk:6: PREFS_DEFINED can be "+
-			"compared using the simpler \"${PREFS_DEFINED:U} == \"<=>\"\" "+
-			"instead of matching against \":M<=>\".",
-		"WARN: filename.mk:6: To use PREFS_DEFINED at load time, "+
-			".include \"../../mk/bsd.prefs.mk\" first.",
-		"AUTOFIX: filename.mk:6: "+
-			"Replacing \"${PREFS_DEFINED:M<=>}\" "+
-			"with \"${PREFS_DEFINED:U} == \\\"<=>\\\"\".")
+			"compared using the simpler \"${PREFS_DEFINED} != pattern\" "+
+			"instead of matching against \":Mpattern\".",
+		"AUTOFIX: filename.mk:6: Replacing \"!${PREFS_DEFINED:Mpattern}\" "+
+			"with \"${PREFS_DEFINED} != pattern\".")
+
+	// TODO: Merge the double negation into the comparison operator.
 	t.testAfterPrefs(
-		".if ${PREFS_DEFINED:M<=>}",
-		".if ${PREFS_DEFINED} == \"<=>\"",
+		".if !!${PREFS_DEFINED:Mpattern}",
+		".if !${PREFS_DEFINED} != pattern",
 
 		"NOTE: filename.mk:6: PREFS_DEFINED can be "+
-			"compared using the simpler \"${PREFS_DEFINED} == \"<=>\"\" "+
-			"instead of matching against \":M<=>\".",
-		"AUTOFIX: filename.mk:6: "+
-			"Replacing \"${PREFS_DEFINED:M<=>}\" "+
-			"with \"${PREFS_DEFINED} == \\\"<=>\\\"\".")
+			"compared using the simpler \"${PREFS_DEFINED} != pattern\" "+
+			"instead of matching against \":Mpattern\".",
+		"AUTOFIX: filename.mk:6: Replacing \"!${PREFS_DEFINED:Mpattern}\" "+
+			"with \"${PREFS_DEFINED} != pattern\".")
 
-	// If pkglint replaces this particular pattern, the resulting string
-	// literal must be escaped properly.
-	t.testBeforeAndAfterPrefs(
-		".if ${IN_SCOPE_DEFINED:M\"}",
-		".if ${IN_SCOPE_DEFINED:M\"}",
+	// A complex condition may contain several simple conditions
+	// that are each simplified independently, in the same go.
+	t.testAfterPrefs(
+		".if ${PREFS_DEFINED:Mpath1} || ${PREFS_DEFINED:Mpath2}",
+		".if ${PREFS_DEFINED} == path1 || ${PREFS_DEFINED} == path2",
 
-		nil...)
+		"NOTE: filename.mk:6: PREFS_DEFINED can be "+
+			"compared using the simpler \"${PREFS_DEFINED} == path1\" "+
+			"instead of matching against \":Mpath1\".",
+		"NOTE: filename.mk:6: PREFS_DEFINED can be "+
+			"compared using the simpler \"${PREFS_DEFINED} == path2\" "+
+			"instead of matching against \":Mpath2\".",
+		"AUTOFIX: filename.mk:6: Replacing \"${PREFS_DEFINED:Mpath1}\" "+
+			"with \"${PREFS_DEFINED} == path1\".",
+		"AUTOFIX: filename.mk:6: Replacing \"${PREFS_DEFINED:Mpath2}\" "+
+			"with \"${PREFS_DEFINED} == path2\".")
 
-	t.testBeforeAndAfterPrefs(
-		".if !empty(IN_SCOPE_DEFINED:M)",
-		".if ${IN_SCOPE_DEFINED} == \"\"",
+	// Removing redundant parentheses may be useful one day but is
+	// not urgent.
+	// Simplifying the inner expression keeps all parentheses as-is.
+	t.testAfterPrefs(
+		".if (((((${PREFS_DEFINED:Mpath})))))",
+		".if (((((${PREFS_DEFINED} == path)))))",
 
-		"NOTE: filename.mk:6: IN_SCOPE_DEFINED can be "+
-			"compared using the simpler "+"\"${IN_SCOPE_DEFINED} == \"\"\" "+
-			"instead of matching against \":M\".",
-		"AUTOFIX: filename.mk:6: "+
-			"Replacing \"!empty(IN_SCOPE_DEFINED:M)\" "+
-			"with \"${IN_SCOPE_DEFINED} == \\\"\\\"\".",
-	)
+		"NOTE: filename.mk:6: PREFS_DEFINED can be "+
+			"compared using the simpler \"${PREFS_DEFINED} == path\" "+
+			"instead of matching against \":Mpath\".",
+		"AUTOFIX: filename.mk:6: Replacing \"${PREFS_DEFINED:Mpath}\" "+
+			"with \"${PREFS_DEFINED} == path\".")
 }
 
 func (s *Suite) Test_MkCondSimplifier_simplifyWord__defined_in_same_file(c *check.C) {
@@ -740,8 +793,57 @@ func (s *Suite) Test_MkCondSimplifier_simplifyWord__defined_in_same_file(c *chec
 			"with \"${LATER_DIR:U} == pattern\".")
 }
 
+// Show how patterns like ':M[yY][eE][sS]' are replaced with simpler
+// conditions.
+func (s *Suite) Test_MkCondSimplifier_simplifyYesNo(c *check.C) {
+	t := NewMkCondSimplifierTester(c, s)
+
+	t.setUp()
+	t.SetUpVarType("VAR", BtYesNo, AlwaysInScope|DefinedIfInScope,
+		"*.mk: use, use-loadtime")
+	t.allowedVariableNames = `VAR`
+
+	// The most common pattern for testing YesNo variables lists the
+	// lowercase letters before the uppercase letters.
+	t.testAfterPrefs(
+		".if ${VAR:M[yY][eE][sS]}",
+		".if ${VAR:tl} == yes",
+
+		"NOTE: filename.mk:6: "+
+			"\"${VAR:M[yY][eE][sS]}\" "+
+			"can be simplified to "+
+			"\"${VAR:tl} == yes\".",
+		"AUTOFIX: filename.mk:6: "+
+			"Replacing \"${VAR:M[yY][eE][sS]}\" "+
+			"with \"${VAR:tl} == yes\".")
+
+	// The less popular pattern for testing YesNo variables lists the
+	// uppercase letters before the lowercase letters.
+	t.testAfterPrefs(
+		".if ${VAR:M[Yy][Ee][Ss]}",
+		".if ${VAR:tl} == yes",
+
+		"NOTE: filename.mk:6: "+
+			"\"${VAR:M[Yy][Ee][Ss]}\" "+
+			"can be simplified to "+
+			"\"${VAR:tl} == yes\".",
+		"AUTOFIX: filename.mk:6: "+
+			"Replacing \"${VAR:M[Yy][Ee][Ss]}\" "+
+			"with \"${VAR:tl} == yes\".")
+
+	// The last letter only has the lowercase form, therefore the pattern
+	// does not match the word 'YES'. Therefore, don't replace it with
+	// ':tl', as that would match the word 'YES'.
+	t.testAfterPrefs(
+		".if ${VAR:M[Yy][Ee][s]}",
+		".if ${VAR:M[Yy][Ee][s]}",
+
+		"WARN: filename.mk:6: VAR should be matched against "+
+			"\"[yY][eE][sS]\" or \"[nN][oO]\", not \"[Yy][Ee][s]\".")
+}
+
 func (s *Suite) Test_MkCondSimplifier_simplifyMatch(c *check.C) {
-	t := MkCondSimplifierTester{c, s.Init(c)}
+	t := NewMkCondSimplifierTester(c, s)
 
 	t.setUp()
 
@@ -765,15 +867,16 @@ func (s *Suite) Test_MkCondSimplifier_simplifyMatch(c *check.C) {
 			"Replacing \"empty(IN_SCOPE_DEFINED:M*.c)\" "+
 			"with \"!${IN_SCOPE_DEFINED:M*.c}\".")
 
+	// From simplifyYesNo.
 	t.testBeforeAndAfterPrefs(
 		".if !empty(IN_SCOPE_DEFINED:M[Nn][Oo])",
-		".if ${IN_SCOPE_DEFINED:M[Nn][Oo]}",
+		".if ${IN_SCOPE_DEFINED:tl} == no",
 
 		"NOTE: filename.mk:6: \"!empty(IN_SCOPE_DEFINED:M[Nn][Oo])\" "+
-			"can be simplified to \"${IN_SCOPE_DEFINED:M[Nn][Oo]}\".",
+			"can be simplified to \"${IN_SCOPE_DEFINED:tl} == no\".",
 		"AUTOFIX: filename.mk:6: "+
 			"Replacing \"!empty(IN_SCOPE_DEFINED:M[Nn][Oo])\" "+
-			"with \"${IN_SCOPE_DEFINED:M[Nn][Oo]}\".")
+			"with \"${IN_SCOPE_DEFINED:tl} == no\".")
 }
 
 func (s *Suite) Test_MkCondSimplifier_isDefined(c *check.C) {
diff --git a/pkgtools/pkglint/files/mktypes.go b/pkgtools/pkglint/files/mktypes.go
index 6f0b1e63889..4152b7f0573 100644
--- a/pkgtools/pkglint/files/mktypes.go
+++ b/pkgtools/pkglint/files/mktypes.go
@@ -47,8 +47,15 @@ func (m MkVarUseModifier) String() string {
 	return string(m)
 }
 
-func (m MkVarUseModifier) Quoted() string {
-	return strings.Replace(string(m), ":", "\\:", -1)
+// Quoted returns the source code representation of the modifier, quoting
+// all characters so that they are interpreted literally.
+func (m MkVarUseModifier) Quoted(end string) string {
+	mod := string(m)
+	mod = strings.Replace(mod, ":", "\\:", -1)
+	mod = strings.Replace(mod, end, "\\"+end, -1)
+	mod = strings.Replace(mod, "#", "\\#", -1)
+	// TODO: There may still be uncovered edge cases.
+	return mod
 }
 
 func (m MkVarUseModifier) HasPrefix(prefix string) bool {
@@ -130,13 +137,13 @@ func (MkVarUseModifier) EvalSubst(s string, left bool, from string, right bool,
 
 // MatchMatch tries to match the modifier to a :M or a :N pattern matching.
 // Examples:
-//  modifier    => ok     positive pattern    exact
-//  ------------------------------------------------
-//  :Mpattern   => true,  true,    "pattern", true
-//  :M*         => true,  true,    "*",       false
-//  :M${VAR}    => true,  true,    "${VAR}",  false
-//  :Npattern   => true,  false,   "pattern", true
-//  :X          => false
+//  modifier    =>   ok     positive  pattern    exact
+//  --------------------------------------------------
+//  :Mpattern   =>   true   true      "pattern"  true
+//  :M*         =>   true   true      "*"        false
+//  :M${VAR}    =>   true   true      "${VAR}"   false
+//  :Npattern   =>   true   false     "pattern"  true
+//  :X          =>   false
 func (m MkVarUseModifier) MatchMatch() (ok bool, positive bool, pattern string, exact bool) {
 	if m.HasPrefix("M") || m.HasPrefix("N") {
 		str := m.String()
diff --git a/pkgtools/pkglint/files/mktypes_test.go b/pkgtools/pkglint/files/mktypes_test.go
index d4fd590cf6f..807662a162f 100644
--- a/pkgtools/pkglint/files/mktypes_test.go
+++ b/pkgtools/pkglint/files/mktypes_test.go
@@ -69,12 +69,14 @@ func (s *Suite) Test_MkVarUseModifier_Quoted(c *check.C) {
 	t := s.Init(c)
 
 	test := func(mod MkVarUseModifier, quoted string) {
-		t.CheckEquals(mod.Quoted(), quoted)
+		t.CheckEquals(mod.Quoted("}"), quoted)
 	}
 
 	test("Q", "Q")
 	test("S/from/to/1g", "S/from/to/1g")
 	test(":", "\\:")
+	test("}", "\\}")
+	test("#", "\\#")
 }
 
 func (s *Suite) Test_MkVarUseModifier_HasPrefix(c *check.C) {
diff --git a/pkgtools/pkglint/files/pkgsrc.go b/pkgtools/pkglint/files/pkgsrc.go
index f36714e1d55..4e2f4a5c8a0 100644
--- a/pkgtools/pkglint/files/pkgsrc.go
+++ b/pkgtools/pkglint/files/pkgsrc.go
@@ -363,39 +363,8 @@ func (*Pkgsrc) parseDocChange(line *Line, warn bool) *Change {
 		author = f[n-2]
 	}
 
-	parseAuthorAndDate := func(authorPtr *string, datePtr *string) bool {
-		alex := textproc.NewLexer(*authorPtr)
-		if !alex.SkipByte('[') {
-			return false
-		}
-		*authorPtr = alex.NextBytesSet(textproc.AlnumU)
-		if !alex.EOF() {
-			return false
-		}
-
-		isDigit := func(b byte) bool { return '0' <= b && b <= '9' }
-
-		date := *datePtr
-		if len(date) == 11 &&
-			isDigit(date[0]) &&
-			isDigit(date[1]) &&
-			isDigit(date[2]) &&
-			isDigit(date[3]) &&
-			date[4] == '-' &&
-			isDigit(date[5]) &&
-			isDigit(date[6]) &&
-			date[7] == '-' &&
-			isDigit(date[8]) &&
-			isDigit(date[9]) &&
-			date[10] == ']' {
-			*datePtr = date[:10]
-			return true
-		}
-
-		return false
-	}
-
-	if !parseAuthorAndDate(&author, &date) {
+	author, date = (*Pkgsrc).parseAuthorAndDate(nil, author, date)
+	if author == "" {
 		return invalid()
 	}
 
@@ -419,6 +388,42 @@ func (*Pkgsrc) parseDocChange(line *Line, warn bool) *Change {
 	return invalid()
 }
 
+// parseAuthorAndDate parses the author and date from a line in doc/CHANGES.
+func (*Pkgsrc) parseAuthorAndDate(author, date string) (string, string) {
+	alex := textproc.NewLexer(author)
+	if !alex.SkipByte('[') {
+		return "", ""
+	}
+	author = alex.NextBytesSet(textproc.AlnumU)
+	if !alex.EOF() {
+		return "", ""
+	}
+
+	isDigit := func(b byte) bool { return '0' <= b && b <= '9' }
+
+	if len(date) == 11 &&
+		isDigit(date[0]) &&
+		isDigit(date[1]) &&
+		isDigit(date[2]) &&
+		isDigit(date[3]) &&
+		date[4] == '-' &&
+		isDigit(date[5]) &&
+		isDigit(date[6]) &&
+		10*(date[5]-'0')+(date[6]-'0') >= 1 &&
+		10*(date[5]-'0')+(date[6]-'0') <= 12 &&
+		date[7] == '-' &&
+		isDigit(date[8]) &&
+		isDigit(date[9]) &&
+		10*(date[8]-'0')+(date[9]-'0') >= 1 &&
+		10*(date[8]-'0')+(date[9]-'0') <= 31 &&
+		date[10] == ']' {
+		date = date[:10]
+		return author, date
+	}
+
+	return "", ""
+}
+
 func (src *Pkgsrc) checkRemovedAfterLastFreeze() {
 	if src.LastFreezeStart == "" || G.Wip || !G.CheckGlobal {
 		return
diff --git a/pkgtools/pkglint/files/pkgsrc_test.go b/pkgtools/pkglint/files/pkgsrc_test.go
index 6ecbcc16df8..4d53d853160 100644
--- a/pkgtools/pkglint/files/pkgsrc_test.go
+++ b/pkgtools/pkglint/files/pkgsrc_test.go
@@ -4,6 +4,7 @@ import (
 	"gopkg.in/check.v1"
 	"os"
 	"path/filepath"
+	"strings"
 )
 
 func (s *Suite) Test_Pkgsrc__frozen(c *check.C) {
@@ -518,6 +519,45 @@ func (s *Suite) Test_Pkgsrc_parseDocChange(c *check.C) {
 			"\tUpdated category/pkgpath to version 1.0 [author 2019-01-01]")
 }
 
+func (s *Suite) Test_Pkgsrc_parseAuthorAndDate(c *check.C) {
+	t := s.Init(c)
+
+	test := func(dateAndAuthor string, expectedAuthor, expectedDate string) {
+		fields := strings.Split(dateAndAuthor, " ")
+		authorIn, dateIn := fields[0], fields[1]
+		author, date := (*Pkgsrc).parseAuthorAndDate(nil, authorIn, dateIn)
+		t.CheckEquals(author, expectedAuthor)
+		t.CheckEquals(date, expectedDate)
+	}
+
+	test("[author 20!9-01-01]", "", "") // bad digit '!' in year
+	test("[author x019-01-01]", "", "") // bad digit 'x' in year
+	test("[author 2x19-01-01]", "", "") // bad digit 'x' in year
+	test("[author 20x9-01-01]", "", "") // bad digit 'x' in year
+	test("[author 201x-01-01]", "", "") // bad digit 'x' in year
+
+	test("[author 2019/01-01]", "", "") // bad separator '/'
+
+	test("[author 2019-x0-01]", "", "") // bad digit 'x' in month
+	test("[author 2019-0x-01]", "", "") // bad digit 'x' in month
+	test("[author 2019-00-01]", "", "") // bad month '00'
+	test("[author 2019-13-01]", "", "") // bad month '13'
+
+	test("[author 2019-01/01]", "", "") // bad separator '/'
+
+	test("[author 2019-01-x0]", "", "") // bad digit 'x' in day
+	test("[author 2019-01-0x]", "", "") // bad digit 'x' in day
+	test("[author 2019-01-00]", "", "") // bad day '00'
+	test("[author 2019-01-32]", "", "") // bad day '32'
+	// No leap year detection, to keep the code fast.
+	test("[author 2019-02-29]", "author", "2019-02-29") // 2019 is not a leap year.
+
+	test("[author 2019-01-01", "", "")  // missing trailing ']'
+	test("[author 2019-01-01+", "", "") // trailing '+' instead of ']'
+
+	test("[author 2019-01-01]", "author", "2019-01-01")
+}
+
 func (s *Suite) Test_Pkgsrc_checkRemovedAfterLastFreeze(c *check.C) {
 	t := s.Init(c)