pkgsrc-Changes archive
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]
CVS commit: pkgsrc/pkgtools/pkglint
Module Name: pkgsrc
Committed By: rillig
Date: Sat Nov 19 10:51:07 UTC 2022
Modified Files:
pkgsrc/pkgtools/pkglint: Makefile
pkgsrc/pkgtools/pkglint/files: mkcondchecker_test.go
mkcondsimplifier.go mkcondsimplifier_test.go mktypes.go
mktypes_test.go pkgsrc.go pkgsrc_test.go
Log Message:
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
To generate a diff of this commit:
cvs rdiff -u -r1.733 -r1.734 pkgsrc/pkgtools/pkglint/Makefile
cvs rdiff -u -r1.12 -r1.13 \
pkgsrc/pkgtools/pkglint/files/mkcondchecker_test.go
cvs rdiff -u -r1.2 -r1.3 pkgsrc/pkgtools/pkglint/files/mkcondsimplifier.go
cvs rdiff -u -r1.1 -r1.2 \
pkgsrc/pkgtools/pkglint/files/mkcondsimplifier_test.go
cvs rdiff -u -r1.27 -r1.28 pkgsrc/pkgtools/pkglint/files/mktypes.go
cvs rdiff -u -r1.24 -r1.25 pkgsrc/pkgtools/pkglint/files/mktypes_test.go
cvs rdiff -u -r1.67 -r1.68 pkgsrc/pkgtools/pkglint/files/pkgsrc.go
cvs rdiff -u -r1.55 -r1.56 pkgsrc/pkgtools/pkglint/files/pkgsrc_test.go
Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.
Modified files:
Index: pkgsrc/pkgtools/pkglint/Makefile
diff -u pkgsrc/pkgtools/pkglint/Makefile:1.733 pkgsrc/pkgtools/pkglint/Makefile:1.734
--- pkgsrc/pkgtools/pkglint/Makefile:1.733 Wed Nov 2 19:39:53 2022
+++ pkgsrc/pkgtools/pkglint/Makefile Sat Nov 19 10:51:07 2022
@@ -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@localhost
@@ -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
Index: pkgsrc/pkgtools/pkglint/files/mkcondchecker_test.go
diff -u pkgsrc/pkgtools/pkglint/files/mkcondchecker_test.go:1.12 pkgsrc/pkgtools/pkglint/files/mkcondchecker_test.go:1.13
--- pkgsrc/pkgtools/pkglint/files/mkcondchecker_test.go:1.12 Sun Jul 24 20:07:20 2022
+++ pkgsrc/pkgtools/pkglint/files/mkcondchecker_test.go Sat Nov 19 10:51:07 2022
@@ -51,7 +51,11 @@ func (s *Suite) Test_MkCondChecker_Check
"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, "+
Index: pkgsrc/pkgtools/pkglint/files/mkcondsimplifier.go
diff -u pkgsrc/pkgtools/pkglint/files/mkcondsimplifier.go:1.2 pkgsrc/pkgtools/pkglint/files/mkcondsimplifier.go:1.3
--- pkgsrc/pkgtools/pkglint/files/mkcondsimplifier.go:1.2 Thu Jul 28 06:37:04 2022
+++ pkgsrc/pkgtools/pkglint/files/mkcondsimplifier.go Sat Nov 19 10:51:07 2022
@@ -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(
}
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(
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(
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}
Index: pkgsrc/pkgtools/pkglint/files/mkcondsimplifier_test.go
diff -u pkgsrc/pkgtools/pkglint/files/mkcondsimplifier_test.go:1.1 pkgsrc/pkgtools/pkglint/files/mkcondsimplifier_test.go:1.2
--- pkgsrc/pkgtools/pkglint/files/mkcondsimplifier_test.go:1.1 Sun Jul 24 20:07:20 2022
+++ pkgsrc/pkgtools/pkglint/files/mkcondsimplifier_test.go Sat Nov 19 10:51:07 2022
@@ -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) testBef
// 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(
}
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_Si
"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_si
"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_si
"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_si
"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_si
"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_si
"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_si
"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_si
"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_si
"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_si
// 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_si
".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_si
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_si
"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_si
"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) {
Index: pkgsrc/pkgtools/pkglint/files/mktypes.go
diff -u pkgsrc/pkgtools/pkglint/files/mktypes.go:1.27 pkgsrc/pkgtools/pkglint/files/mktypes.go:1.28
--- pkgsrc/pkgtools/pkglint/files/mktypes.go:1.27 Sun Jul 24 20:07:20 2022
+++ pkgsrc/pkgtools/pkglint/files/mktypes.go Sat Nov 19 10:51:07 2022
@@ -47,8 +47,15 @@ func (m MkVarUseModifier) String() strin
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 stri
// 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()
Index: pkgsrc/pkgtools/pkglint/files/mktypes_test.go
diff -u pkgsrc/pkgtools/pkglint/files/mktypes_test.go:1.24 pkgsrc/pkgtools/pkglint/files/mktypes_test.go:1.25
--- pkgsrc/pkgtools/pkglint/files/mktypes_test.go:1.24 Wed Jul 1 13:17:41 2020
+++ pkgsrc/pkgtools/pkglint/files/mktypes_test.go Sat Nov 19 10:51:07 2022
@@ -69,12 +69,14 @@ func (s *Suite) Test_MkVarUseModifier_Qu
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) {
Index: pkgsrc/pkgtools/pkglint/files/pkgsrc.go
diff -u pkgsrc/pkgtools/pkglint/files/pkgsrc.go:1.67 pkgsrc/pkgtools/pkglint/files/pkgsrc.go:1.68
--- pkgsrc/pkgtools/pkglint/files/pkgsrc.go:1.67 Sun Oct 2 14:39:37 2022
+++ pkgsrc/pkgtools/pkglint/files/pkgsrc.go Sat Nov 19 10:51:07 2022
@@ -363,39 +363,8 @@ func (*Pkgsrc) parseDocChange(line *Line
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
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
Index: pkgsrc/pkgtools/pkglint/files/pkgsrc_test.go
diff -u pkgsrc/pkgtools/pkglint/files/pkgsrc_test.go:1.55 pkgsrc/pkgtools/pkglint/files/pkgsrc_test.go:1.56
--- pkgsrc/pkgtools/pkglint/files/pkgsrc_test.go:1.55 Sat Jan 1 12:44:25 2022
+++ pkgsrc/pkgtools/pkglint/files/pkgsrc_test.go Sat Nov 19 10:51:07 2022
@@ -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_parseDocChan
"\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)
Home |
Main Index |
Thread Index |
Old Index