pkgsrc-Changes-HG archive
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]
[pkgsrc/trunk]: pkgsrc/pkgtools/pkglint pkgtools/pkglint: update to 22.2.3
details: https://anonhg.NetBSD.org/pkgsrc/rev/c7c2ea10fe4b
branches: trunk
changeset: 382280:c7c2ea10fe4b
user: rillig <rillig%pkgsrc.org@localhost>
date: Sun Jul 24 20:07:20 2022 +0000
description:
pkgtools/pkglint: update to 22.2.3
Changes since 22.2.2:
CHECK_WRKREF is known to pkglint, which prevents conditions using this
variable from being simplified in a wrong way.
For variables that are guaranteed to be defined, suggest to simplify the
condition '!empty(VAR:M[Yy][Ee][Ss])' to '${VAR:M[Yy][Ee][Ss]}', as that
reduces the number of negations in the condition.
Detect redundant WRKSRC definitions and suggest to remove them.
Fix wrong "c99 is not valid for USE_LANGUAGES" warnings.
diffstat:
pkgtools/pkglint/Makefile | 5 +-
pkgtools/pkglint/files/mkassignchecker.go | 5 +
pkgtools/pkglint/files/mkcondchecker.go | 143 +----
pkgtools/pkglint/files/mkcondchecker_test.go | 717 ---------------------
pkgtools/pkglint/files/mkcondsimplifier.go | 194 +++++
pkgtools/pkglint/files/mkcondsimplifier_test.go | 801 ++++++++++++++++++++++++
pkgtools/pkglint/files/mkparser.go | 2 +-
pkgtools/pkglint/files/mktypes.go | 10 +-
pkgtools/pkglint/files/package_test.go | 13 +
pkgtools/pkglint/files/redundantscope.go | 2 +-
pkgtools/pkglint/files/vardefs.go | 3 +-
pkgtools/pkglint/files/vartypecheck.go | 16 +
pkgtools/pkglint/files/vartypecheck_test.go | 32 +-
13 files changed, 1074 insertions(+), 869 deletions(-)
diffs (truncated from 2096 to 300 lines):
diff -r c9fc9fe6fae0 -r c7c2ea10fe4b pkgtools/pkglint/Makefile
--- a/pkgtools/pkglint/Makefile Sun Jul 24 19:52:41 2022 +0000
+++ b/pkgtools/pkglint/Makefile Sun Jul 24 20:07:20 2022 +0000
@@ -1,7 +1,6 @@
-# $NetBSD: Makefile,v 1.722 2022/07/13 16:03:04 bsiegert Exp $
+# $NetBSD: Makefile,v 1.723 2022/07/24 20:07:20 rillig Exp $
-PKGNAME= pkglint-22.2.2
-PKGREVISION= 1
+PKGNAME= pkglint-22.2.3
CATEGORIES= pkgtools
DISTNAME= tools
MASTER_SITES= ${MASTER_SITE_GITHUB:=golang/}
diff -r c9fc9fe6fae0 -r c7c2ea10fe4b pkgtools/pkglint/files/mkassignchecker.go
--- a/pkgtools/pkglint/files/mkassignchecker.go Sun Jul 24 19:52:41 2022 +0000
+++ b/pkgtools/pkglint/files/mkassignchecker.go Sun Jul 24 20:07:20 2022 +0000
@@ -494,6 +494,11 @@
mkLineChecker := NewMkLineChecker(ck.MkLines, ck.MkLine)
mkLineChecker.checkText(value)
mkLineChecker.checkVartype(varname, op, value, comment)
+ if mkline.IsEmpty() {
+ // The line type can change due to an Autofix, see for example
+ // VartypeCheck.WrkdirSubdirectory.
+ return
+ }
ck.checkMisc()
diff -r c9fc9fe6fae0 -r c7c2ea10fe4b pkgtools/pkglint/files/mkcondchecker.go
--- a/pkgtools/pkglint/files/mkcondchecker.go Sun Jul 24 19:52:41 2022 +0000
+++ b/pkgtools/pkglint/files/mkcondchecker.go Sun Jul 24 20:07:20 2022 +0000
@@ -109,7 +109,9 @@
func (ck *MkCondChecker) checkEmpty(varuse *MkVarUse, fromEmpty bool, neg bool) {
ck.checkEmptyExpr(varuse)
ck.checkEmptyType(varuse)
- ck.simplify(varuse, fromEmpty, neg)
+
+ s := MkCondSimplifier{ck.MkLines, ck.MkLine}
+ s.SimplifyVarUse(varuse, fromEmpty, neg)
}
func (ck *MkCondChecker) checkEmptyExpr(varuse *MkVarUse) {
@@ -160,145 +162,6 @@
// TODO: Check whether the ',' really needs to be here.
var mkCondModifierPatternLiteral = textproc.NewByteSet("-+,./0-9<=>@A-Z_a-z")
-// simplify replaces an unnecessarily complex condition with
-// a simpler condition that's still equivalent.
-//
-// * fromEmpty is true for the form empty(VAR...), and false for ${VAR...}.
-//
-// * neg is true for the form !empty(VAR...), and false for empty(VAR...).
-func (ck *MkCondChecker) simplify(varuse *MkVarUse, fromEmpty bool, neg bool) {
- varname := varuse.varname
- modifiers := varuse.modifiers
-
- n := len(modifiers)
- if n == 0 {
- return
- }
- modsExceptLast := NewMkVarUse("", modifiers[:n-1]...).Mod()
- vartype := G.Pkgsrc.VariableType(ck.MkLines, varname)
-
- isDefined := func() bool {
- if vartype.IsAlwaysInScope() && vartype.IsDefinedIfInScope() {
- return true
- }
-
- // For run time expressions, such as ${${VAR} == value:?yes:no},
- // the scope would need to be changed to ck.MkLines.allVars.
- if ck.MkLines.checkAllData.vars.IsDefined(varname) {
- return true
- }
-
- return ck.MkLines.Tools.SeenPrefs &&
- vartype.Union().Contains(aclpUseLoadtime) &&
- vartype.IsDefinedIfInScope()
- }
-
- // replace constructs the state before and after the autofix.
- // The before state is constructed to ensure that only very simple
- // patterns get replaced automatically.
- //
- // Before putting any cases involving special characters into
- // production, there need to be more tests for the edge cases.
- replace := func(positive bool, pattern string) (bool, string, string) {
- defined := isDefined()
- if !defined && !positive {
- // TODO: This is a double negation, maybe even triple.
- // There is an :N pattern, and the variable may be undefined.
- // If it is indeed undefined, should the whole condition
- // evaluate to true or false?
- // The cases to be distinguished are: undefined, empty, filled.
-
- // For now, be conservative and don't suggest anything wrong.
- 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, ")", "}"))
-
- needsQuotes := textproc.NewLexer(pattern).NextBytesSet(mkCondStringLiteralUnquoted) != pattern ||
- pattern == "" ||
- matches(pattern, `^\d+\.?\d*$`)
- quote := condStr(needsQuotes, "\"", "")
-
- to := sprintf(
- "${%s%s%s} %s %s%s%s",
- varname, uMod, modsExceptLast, op, quote, pattern, quote)
-
- return true, from, to
- }
-
- modifier := modifiers[n-1]
- ok, positive, pattern, exact := modifier.MatchMatch()
- if !ok || !positive && n != 1 {
- return
- }
-
- // Replace !empty(VAR:M*.c) with ${VAR:M*.c}.
- // Replace empty(VAR:M*.c) with !${VAR:M*.c}.
- if fromEmpty && positive && !exact && vartype != nil && isDefined() &&
- // Restrict replacements to very simple patterns with only few
- // special characters, for now.
- // Before generalizing this to arbitrary strings, there has to be
- // a proper code generator for these conditions that handles all
- // possible escaping.
- // The same reasoning applies to the variable name, even though the
- // variable name typically only uses a restricted character set.
- matches(varuse.Mod(), `^[*.:\w]+$`) {
-
- fixedPart := varname + modsExceptLast + ":M" + pattern
- from := condStr(neg, "!", "") + "empty(" + fixedPart + ")"
- to := condStr(neg, "", "!") + "${" + fixedPart + "}"
-
- fix := ck.MkLine.Autofix()
- fix.Notef("%q can be simplified to %q.", from, to)
- fix.Explain(
- "This variable is guaranteed to be defined at this point.",
- "Therefore it may occur on the left-hand side of a comparison",
- "and doesn't have to be guarded by the function 'empty'.")
- fix.Replace(from, to)
- fix.Apply()
-
- return
- }
-
- switch {
- case !exact,
- vartype == nil,
- vartype.IsList(),
- textproc.NewLexer(pattern).NextBytesSet(mkCondModifierPatternLiteral) != pattern:
- return
- }
-
- ok, from, to := replace(positive, pattern)
- if !ok {
- return
- }
-
- fix := ck.MkLine.Autofix()
- fix.Notef("%s can be compared using the simpler \"%s\" "+
- "instead of matching against %q.",
- varname, to, ":"+modifier.String()) // TODO: Quoted
- fix.Explain(
- "This variable has a single value, not a list of values.",
- "Therefore it feels strange to apply list operators like :M and :N onto it.",
- "A more direct approach is to use the == and != operators.",
- "",
- "An entirely different case is when the pattern contains",
- "wildcards like *, ?, [].",
- "In such a case, using the :M or :N modifiers is useful and preferred.")
- fix.Replace(from, to)
- fix.Apply()
-}
-
func (ck *MkCondChecker) checkCompare(left *MkCondTerm, op string, right *MkCondTerm) {
switch {
case right.Num != "":
diff -r c9fc9fe6fae0 -r c7c2ea10fe4b pkgtools/pkglint/files/mkcondchecker_test.go
--- a/pkgtools/pkglint/files/mkcondchecker_test.go Sun Jul 24 19:52:41 2022 +0000
+++ b/pkgtools/pkglint/files/mkcondchecker_test.go Sun Jul 24 20:07:20 2022 +0000
@@ -530,723 +530,6 @@
nil...)
}
-func (s *Suite) Test_MkCondChecker_simplify(c *check.C) {
- t := s.Init(c)
-
- t.CreateFileLines("mk/bsd.prefs.mk")
- t.Chdir("category/package")
-
- // The Anything type suppresses the warnings from type checking.
- // BtUnknown would not work here, see Pkgsrc.VariableType.
- btAnything := &BasicType{"Anything", func(cv *VartypeCheck) {}}
-
- // For simplifying the expressions, it is necessary to know whether
- // a variable can be undefined. Undefined variables need the
- // :U modifier or must be enclosed in double quotes, otherwise
- // bmake will complain about a "Malformed conditional". That error
- // message is not entirely precise since the expression
- // is syntactically valid, it's just the evaluation that fails.
- //
- // Some variables such as MACHINE_ARCH are in scope from the very
- // beginning.
- //
- // Some variables such as PKGPATH are in scope after bsd.prefs.mk
- // has been included.
- //
- // Some variables such as PREFIX (as of December 2019) are only in
- // scope after bsd.pkg.mk has been included. These cannot be used
- // in .if conditions at all.
- //
- // Even when they are in scope, some variables such as PKGREVISION
- // or MAKE_JOBS may be undefined.
-
- t.SetUpVarType("IN_SCOPE_DEFINED", btAnything, AlwaysInScope|DefinedIfInScope,
- "*.mk: use, use-loadtime")
- t.SetUpVarType("IN_SCOPE", btAnything, AlwaysInScope,
- "*.mk: use, use-loadtime")
- t.SetUpVarType("PREFS_DEFINED", btAnything, DefinedIfInScope,
- "*.mk: use, use-loadtime")
- t.SetUpVarType("PREFS", btAnything, NoVartypeOptions,
- "*.mk: use, use-loadtime")
- t.SetUpVarType("LATER_DEFINED", btAnything, DefinedIfInScope,
- "*.mk: use")
- t.SetUpVarType("LATER", btAnything, NoVartypeOptions,
- "*.mk: use")
- // UNDEFINED is also used in the following tests, but is obviously
- // not defined here.
-
- // prefs: whether to include bsd.prefs.mk before
- // before: the directive before the condition is simplified
- // after: the directive after the condition is simplified
- doTest := func(prefs bool, before, after string, diagnostics ...string) {
- if !matches(before, `IN_SCOPE|PREFS|LATER|UNDEFINED`) {
- c.Errorf("Condition %q must include one of the above variable names.", before)
- }
- mklines := t.SetUpFileMkLines("filename.mk",
- MkCvsID,
- condStr(prefs, ".include \"../../mk/bsd.prefs.mk\"", ""),
- before,
- ".endif")
-
- action := func(autofix bool) {
- mklines.ForEach(func(mkline *MkLine) {
- // Sets mklines.Tools.SeenPrefs, which decides whether the :U modifier
- // is necessary; see MkLines.checkLine.
- mklines.Tools.ParseToolLine(mklines, mkline, false, false)
-
- if mkline.IsDirective() && mkline.Directive() != "endif" {
- // TODO: Replace Check with a more
- // specific method that does not do the type checks.
- NewMkCondChecker(mkline, mklines).Check()
- }
- })
-
- if autofix {
- afterMklines := LoadMk(t.File("filename.mk"), nil, MustSucceed)
- t.CheckEquals(afterMklines.mklines[2].Text, after)
- }
- }
-
- t.ExpectDiagnosticsAutofix(action, diagnostics...)
- }
-
- testBeforePrefs := func(before, after string, diagnostics ...string) {
- doTest(false, before, after, diagnostics...)
- }
- testAfterPrefs := func(before, after string, diagnostics ...string) {
- doTest(true, before, after, diagnostics...)
- }
- testBeforeAndAfterPrefs := func(before, after string, diagnostics ...string) {
- doTest(false, before, after, diagnostics...)
- doTest(true, before, after, diagnostics...)
- }
-
- testBeforeAndAfterPrefs(
- ".if ${IN_SCOPE_DEFINED:Mpattern}",
- ".if ${IN_SCOPE_DEFINED} == pattern",
-
- "NOTE: filename.mk:3: IN_SCOPE_DEFINED can be "+
- "compared using the simpler \"${IN_SCOPE_DEFINED} == pattern\" "+
- "instead of matching against \":Mpattern\".",
- "AUTOFIX: filename.mk:3: Replacing \"${IN_SCOPE_DEFINED:Mpattern}\" "+
- "with \"${IN_SCOPE_DEFINED} == pattern\".")
-
- testBeforeAndAfterPrefs(
- ".if ${IN_SCOPE:Mpattern}",
- ".if ${IN_SCOPE:U} == pattern",
-
Home |
Main Index |
Thread Index |
Old Index