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 19.4.4
details: https://anonhg.NetBSD.org/pkgsrc/rev/ee32590c0ad6
branches: trunk
changeset: 421846:ee32590c0ad6
user: rillig <rillig%pkgsrc.org@localhost>
date: Sat Jan 18 21:56:09 2020 +0000
description:
pkgtools/pkglint: update to 19.4.4
Changes since 19.4.3:
Packages that still use http in their HOMEPAGE URL generate warnings that
the URL should use https instead.
diffstat:
pkgtools/pkglint/Makefile | 4 +-
pkgtools/pkglint/files/buildlink3.go | 4 +-
pkgtools/pkglint/files/check_test.go | 3 +
pkgtools/pkglint/files/licenses.go | 6 +-
pkgtools/pkglint/files/mkassignchecker.go | 2 +-
pkgtools/pkglint/files/mkcondchecker.go | 33 +++-
pkgtools/pkglint/files/mkcondchecker_test.go | 51 ++++++
pkgtools/pkglint/files/mkline.go | 10 +-
pkgtools/pkglint/files/mkline_test.go | 4 +-
pkgtools/pkglint/files/mklinechecker.go | 29 ++-
pkgtools/pkglint/files/mklinechecker_test.go | 5 +-
pkgtools/pkglint/files/mklines_test.go | 2 +-
pkgtools/pkglint/files/mkparser.go | 19 +-
pkgtools/pkglint/files/mkparser_test.go | 191 ++++++++++++++----------
pkgtools/pkglint/files/mkvarusechecker_test.go | 2 +-
pkgtools/pkglint/files/package.go | 3 +-
pkgtools/pkglint/files/path.go | 20 ++
pkgtools/pkglint/files/path_test.go | 59 +++++++
pkgtools/pkglint/files/vardefs.go | 10 +-
pkgtools/pkglint/files/vartype.go | 2 +-
pkgtools/pkglint/files/vartypecheck.go | 98 +++++++++++-
pkgtools/pkglint/files/vartypecheck_test.go | 43 +++++-
22 files changed, 457 insertions(+), 143 deletions(-)
diffs (truncated from 1254 to 300 lines):
diff -r f52c7338a1e4 -r ee32590c0ad6 pkgtools/pkglint/Makefile
--- a/pkgtools/pkglint/Makefile Sat Jan 18 21:54:07 2020 +0000
+++ b/pkgtools/pkglint/Makefile Sat Jan 18 21:56:09 2020 +0000
@@ -1,6 +1,6 @@
-# $NetBSD: Makefile,v 1.625 2020/01/11 15:48:28 rillig Exp $
+# $NetBSD: Makefile,v 1.626 2020/01/18 21:56:09 rillig Exp $
-PKGNAME= pkglint-19.4.3
+PKGNAME= pkglint-19.4.4
CATEGORIES= pkgtools
DISTNAME= tools
MASTER_SITES= ${MASTER_SITE_GITHUB:=golang/}
diff -r f52c7338a1e4 -r ee32590c0ad6 pkgtools/pkglint/files/buildlink3.go
--- a/pkgtools/pkglint/files/buildlink3.go Sat Jan 18 21:54:07 2020 +0000
+++ b/pkgtools/pkglint/files/buildlink3.go Sat Jan 18 21:56:09 2020 +0000
@@ -202,7 +202,7 @@
if varname == "BUILDLINK_ABI_DEPENDS."+pkgbase {
ck.abiLine = mkline
parser := NewMkParser(nil, value)
- if dp := parser.Dependency(); dp != nil && parser.EOF() {
+ if dp := parser.DependencyPattern(); dp != nil && parser.EOF() {
ck.abi = dp
}
doCheck = true
@@ -211,7 +211,7 @@
if varname == "BUILDLINK_API_DEPENDS."+pkgbase {
ck.apiLine = mkline
parser := NewMkParser(nil, value)
- if dp := parser.Dependency(); dp != nil && parser.EOF() {
+ if dp := parser.DependencyPattern(); dp != nil && parser.EOF() {
ck.api = dp
}
doCheck = true
diff -r f52c7338a1e4 -r ee32590c0ad6 pkgtools/pkglint/files/check_test.go
--- a/pkgtools/pkglint/files/check_test.go Sat Jan 18 21:54:07 2020 +0000
+++ b/pkgtools/pkglint/files/check_test.go Sat Jan 18 21:56:09 2020 +0000
@@ -145,6 +145,9 @@
ck.Configure("vargroups.go", "*", "*", -intqa.EMissingTest) // TODO
ck.Configure("vartype.go", "*", "*", -intqa.EMissingTest) // TODO
+ // Don't require tests for helper methods.
+ ck.Configure("*.go", "VartypeCheck", "[a-z]*", -intqa.EMissingTest)
+
// For now, don't require tests for all the test code.
// Having good coverage for the main code is more important.
ck.Configure("*_test.go", "*", "*", -intqa.EMissingTest)
diff -r f52c7338a1e4 -r ee32590c0ad6 pkgtools/pkglint/files/licenses.go
--- a/pkgtools/pkglint/files/licenses.go Sat Jan 18 21:54:07 2020 +0000
+++ b/pkgtools/pkglint/files/licenses.go Sat Jan 18 21:56:09 2020 +0000
@@ -28,8 +28,10 @@
pkg := lc.MkLines.pkg
if pkg != nil {
if mkline := pkg.vars.FirstDefinition("LICENSE_FILE"); mkline != nil {
- rel := mkline.ResolveVarsInRelativePath(NewRelPathString(mkline.Value()), lc.MkLines.pkg)
- licenseFile = pkg.File(NewPackagePath(rel))
+ // TODO: Not every path is relative to the package directory.
+ rel := NewPackagePathString(mkline.Value())
+ relResolved := mkline.ResolveVarsInRelativePath(rel, lc.MkLines.pkg)
+ licenseFile = pkg.File(relResolved)
}
}
if licenseFile.IsEmpty() {
diff -r f52c7338a1e4 -r ee32590c0ad6 pkgtools/pkglint/files/mkassignchecker.go
--- a/pkgtools/pkglint/files/mkassignchecker.go Sat Jan 18 21:54:07 2020 +0000
+++ b/pkgtools/pkglint/files/mkassignchecker.go Sat Jan 18 21:56:09 2020 +0000
@@ -284,7 +284,7 @@
return
}
- if mkline.HasRationale() {
+ if mkline.Rationale() != "" {
return
}
diff -r f52c7338a1e4 -r ee32590c0ad6 pkgtools/pkglint/files/mkcondchecker.go
--- a/pkgtools/pkglint/files/mkcondchecker.go Sat Jan 18 21:54:07 2020 +0000
+++ b/pkgtools/pkglint/files/mkcondchecker.go Sat Jan 18 21:56:09 2020 +0000
@@ -36,9 +36,10 @@
// Skip subconditions that have already been handled as part of the !(...).
done := make(map[interface{}]bool)
- checkNotEmpty := func(not *MkCond) {
+ checkNot := func(not *MkCond) {
empty := not.Empty
if empty != nil {
+ ck.checkNotEmpty(not)
ck.checkEmpty(empty, true, true)
done[empty] = true
}
@@ -48,6 +49,8 @@
ck.checkEmpty(varUse, false, false)
done[varUse] = true
}
+
+ ck.checkNotCompare(not)
}
checkEmpty := func(empty *MkVarUse) {
@@ -63,13 +66,29 @@
}
cond.Walk(&MkCondCallback{
- Not: checkNotEmpty,
+ Not: checkNot,
Empty: checkEmpty,
Var: checkVar,
Compare: ck.checkCompare,
VarUse: checkVarUse})
}
+func (ck *MkCondChecker) checkNotEmpty(not *MkCond) {
+ // Consider suggesting ${VAR} instead of !empty(VAR) since it is
+ // shorter and avoids unnecessary negation, which makes the
+ // expression less confusing.
+ //
+ // This applies especially to the ${VAR:Mpattern} form.
+ //
+ // See MkCondChecker.simplify.
+ if !G.Experimental {
+ return
+ }
+
+ ck.MkLine.Notef("!empty(%s%s) can be replaced with the simpler %s.",
+ not.Empty.varname, not.Empty.Mod(), not.Empty.String())
+}
+
// checkEmpty checks a condition of the form empty(VAR),
// empty(VAR:Mpattern) or ${VAR:Mpattern} in an .if directive.
func (ck *MkCondChecker) checkEmpty(varuse *MkVarUse, fromEmpty bool, neg bool) {
@@ -90,6 +109,7 @@
"",
"\tempty(VARNAME:Mpattern)",
"\t${VARNAME:Mpattern} == \"\"",
+ "\t!${VARNAME:Mpattern}",
"",
"Instead of !empty(${VARNAME:Mpattern}), you should write either of the following:",
"",
@@ -129,7 +149,6 @@
// * 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...).
-// It also applies to the ${VAR} form.
func (ck *MkCondChecker) simplify(varuse *MkVarUse, fromEmpty bool, neg bool) {
varname := varuse.varname
mods := varuse.modifiers
@@ -300,3 +319,11 @@
fix.Replace("${PKGSRC_COMPILER} "+op+" \""+value+"\"", "${PKGSRC_COMPILER:"+matchOp+value+"}")
fix.Apply()
}
+
+func (ck *MkCondChecker) checkNotCompare(not *MkCond) {
+ if not.Compare == nil {
+ return
+ }
+
+ ck.MkLine.Warnf("The ! should use parentheses or be merged into the comparison operator.")
+}
diff -r f52c7338a1e4 -r ee32590c0ad6 pkgtools/pkglint/files/mkcondchecker_test.go
--- a/pkgtools/pkglint/files/mkcondchecker_test.go Sat Jan 18 21:54:07 2020 +0000
+++ b/pkgtools/pkglint/files/mkcondchecker_test.go Sat Jan 18 21:56:09 2020 +0000
@@ -90,6 +90,7 @@
// Doesn't occur in practice since it is surprising that the ! applies
// to the comparison operator, and not to one of its arguments.
test(".if !${VAR} == value",
+ "WARN: filename.mk:4: The ! should use parentheses or be merged into the comparison operator.",
"WARN: filename.mk:4: VAR is used but not defined.")
// Doesn't occur in practice since this string can never be empty.
@@ -215,6 +216,27 @@
"ERROR: Makefile:6: Use ${PKGSRC_COMPILER:Ngcc} instead of the != operator.")
}
+func (s *Suite) Test_MkCondChecker_checkNotEmpty(c *check.C) {
+ t := s.Init(c)
+
+ G.Experimental = true
+
+ test := func(cond string, diagnostics ...string) {
+ mklines := t.NewMkLines("filename.mk",
+ ".if "+cond)
+ mkline := mklines.mklines[0]
+ ck := NewMkCondChecker(mkline, mklines)
+
+ ck.checkNotEmpty(mkline.Cond().Not)
+
+ t.CheckOutput(diagnostics)
+ }
+
+ test("!empty(VAR)",
+ // FIXME: Add a :U modifier if VAR might be undefined.
+ "NOTE: filename.mk:1: !empty(VAR) can be replaced with the simpler ${VAR}.")
+}
+
func (s *Suite) Test_MkCondChecker_checkEmpty(c *check.C) {
t := s.Init(c)
@@ -1150,3 +1172,32 @@
nil...)
}
+
+func (s *Suite) Test_MkCondChecker_checkNotCompare(c *check.C) {
+ t := s.Init(c)
+
+ test := func(cond string, diagnostics ...string) {
+ mklines := t.NewMkLines("filename.mk",
+ ".if "+cond)
+ mkline := mklines.mklines[0]
+ ck := NewMkCondChecker(mkline, mklines)
+
+ ck.checkNotCompare(mkline.Cond().Not)
+
+ t.CheckOutput(diagnostics)
+ }
+
+ test("!${VAR} == value",
+ "WARN: filename.mk:1: The ! should use parentheses "+
+ "or be merged into the comparison operator.")
+
+ test("!${VAR} != value",
+ "WARN: filename.mk:1: The ! should use parentheses "+
+ "or be merged into the comparison operator.")
+
+ test("!(${VAR} == value)",
+ nil...)
+
+ test("!${VAR}",
+ nil...)
+}
diff -r f52c7338a1e4 -r ee32590c0ad6 pkgtools/pkglint/files/mkline.go
--- a/pkgtools/pkglint/files/mkline.go Sat Jan 18 21:54:07 2020 +0000
+++ b/pkgtools/pkglint/files/mkline.go Sat Jan 18 21:56:09 2020 +0000
@@ -74,8 +74,11 @@
func (mkline *MkLine) HasComment() bool { return mkline.splitResult.hasComment }
-func (mkline *MkLine) HasRationale() bool { return mkline.splitResult.rationale != "" }
-
+// Rationale returns the comments that are close enough to this line.
+//
+// These comments are used to suppress pkglint warnings,
+// such as for BROKEN, NOT_FOR_PLATFORMS, MAKE_JOBS_SAFE,
+// and HOMEPAGE using http instead of https.
func (mkline *MkLine) Rationale() string { return mkline.splitResult.rationale }
// Comment returns the comment after the first unescaped #.
@@ -564,7 +567,8 @@
return valueNovar.String()
}
-func (mkline *MkLine) ResolveVarsInRelativePath(relativePath RelPath, pkg *Package) RelPath {
+func (mkline *MkLine) ResolveVarsInRelativePath(relativePath PackagePath, pkg *Package) PackagePath {
+ // TODO: Not every path is relative to the package directory.
if !containsVarUse(relativePath.String()) {
return relativePath.CleanPath()
}
diff -r f52c7338a1e4 -r ee32590c0ad6 pkgtools/pkglint/files/mkline_test.go
--- a/pkgtools/pkglint/files/mkline_test.go Sat Jan 18 21:54:07 2020 +0000
+++ b/pkgtools/pkglint/files/mkline_test.go Sat Jan 18 21:56:09 2020 +0000
@@ -535,7 +535,7 @@
mkline := mklines.mklines[0]
var pkg *Package = nil
- test := func(before RelPath, after RelPath) {
+ test := func(before PackagePath, after PackagePath) {
t.CheckEquals(mkline.ResolveVarsInRelativePath(before, pkg), after)
}
@@ -1164,7 +1164,7 @@
"",
"GO_SRCPATH=\t\t${HOMEPAGE:S,https://,,}";,
"LINKER_RPATH_FLAG:=\t${LINKER_RPATH_FLAG:S/-rpath/& /}",
- "HOMEPAGE=\t\thttp://godoc.org/${GO_SRCPATH}";,
+ "HOMEPAGE=\t\thttps://godoc.org/${GO_SRCPATH}";,
"PATH:=\t\t\t${PREFIX}/cross/bin:${PATH}",
"NO_SRC_ON_FTP=\t\t${RESTRICTED}")
diff -r f52c7338a1e4 -r ee32590c0ad6 pkgtools/pkglint/files/mklinechecker.go
--- a/pkgtools/pkglint/files/mklinechecker.go Sat Jan 18 21:54:07 2020 +0000
+++ b/pkgtools/pkglint/files/mklinechecker.go Sat Jan 18 21:56:09 2020 +0000
@@ -236,7 +236,8 @@
if trace.Tracing {
trace.Stepf("includingFile=%s includedFile=%s", mkline.Filename, includedFile)
}
- ck.CheckRelativePath(includedFile, mustExist)
+ // TODO: Not every path is relative to the package directory.
+ ck.CheckRelativePath(NewPackagePath(includedFile), includedFile, mustExist)
switch {
case includedFile.HasBase("Makefile"):
@@ -269,7 +270,7 @@
mkline.Warnf("Please write \"USE_TOOLS+= intltool\" instead of this line.")
case includedFile != "builtin.mk" && includedFile.HasSuffixPath("builtin.mk"):
- if mkline.Basename != "hacks.mk" && !mkline.HasRationale() {
+ if mkline.Basename != "hacks.mk" && mkline.Rationale() == "" {
fix := mkline.Autofix()
fix.Errorf("%q must not be included directly. Include %q instead.",
includedFile, includedFile.DirNoClean().JoinNoClean("buildlink3.mk"))
@@ -294,26 +295,28 @@
Home |
Main Index |
Thread Index |
Old Index