summaryrefslogtreecommitdiff
path: root/pkgtools
diff options
context:
space:
mode:
authorrillig <rillig@pkgsrc.org>2021-06-25 14:15:00 +0000
committerrillig <rillig@pkgsrc.org>2021-06-25 14:15:00 +0000
commitb27ec57e6b96c779025703e8c121e2a66144d78e (patch)
tree029ca9a3ac3604018d05ad94efe532b57f3898a4 /pkgtools
parentdb7af6e3c79b7a2f8771c93d3a04479d79ff039f (diff)
downloadpkgsrc-b27ec57e6b96c779025703e8c121e2a66144d78e.tar.gz
pkgtools/pkglint: update to 21.2.1
Changes since 21.2.0: Files whose names ends in '~' are ignored by pkglint since they are ignored by CVS as well. Variables with name BUILDLINK_TRANSFORM.* may contain '-Wl,-rpath,' directly in commands of the form 'rm:*', just like their counterpart BUILDLINK_TRANSFORM without a package name in the variable name. Several new tests.
Diffstat (limited to 'pkgtools')
-rw-r--r--pkgtools/pkglint/Makefile4
-rw-r--r--pkgtools/pkglint/files/check_test.go7
-rw-r--r--pkgtools/pkglint/files/distinfo_test.go95
-rw-r--r--pkgtools/pkglint/files/files_test.go17
-rw-r--r--pkgtools/pkglint/files/intqa/qa.go20
-rw-r--r--pkgtools/pkglint/files/licenses.go32
-rw-r--r--pkgtools/pkglint/files/licenses_test.go28
-rw-r--r--pkgtools/pkglint/files/line_test.go228
-rw-r--r--pkgtools/pkglint/files/linechecker_test.go14
-rw-r--r--pkgtools/pkglint/files/lines_test.go77
-rw-r--r--pkgtools/pkglint/files/logging_test.go174
-rw-r--r--pkgtools/pkglint/files/makepat/pat_test.go1
-rw-r--r--pkgtools/pkglint/files/mklinechecker.go3
-rw-r--r--pkgtools/pkglint/files/mklinechecker_test.go12
-rw-r--r--pkgtools/pkglint/files/mklineparser_test.go3
-rw-r--r--pkgtools/pkglint/files/package.go26
-rw-r--r--pkgtools/pkglint/files/pkglint_test.go24
-rw-r--r--pkgtools/pkglint/files/pkgsrc.go2
-rw-r--r--pkgtools/pkglint/files/pkgver/vercmp_test.go2
-rw-r--r--pkgtools/pkglint/files/util.go4
-rw-r--r--pkgtools/pkglint/files/util_test.go6
-rw-r--r--pkgtools/pkglint/files/vartypecheck.go2
22 files changed, 727 insertions, 54 deletions
diff --git a/pkgtools/pkglint/Makefile b/pkgtools/pkglint/Makefile
index 36ce1832bc6..7db6c96df77 100644
--- a/pkgtools/pkglint/Makefile
+++ b/pkgtools/pkglint/Makefile
@@ -1,6 +1,6 @@
-# $NetBSD: Makefile,v 1.689 2021/06/17 09:11:55 rillig Exp $
+# $NetBSD: Makefile,v 1.690 2021/06/25 14:15:00 rillig Exp $
-PKGNAME= pkglint-21.2.0
+PKGNAME= pkglint-21.2.1
CATEGORIES= pkgtools
DISTNAME= tools
MASTER_SITES= ${MASTER_SITE_GITHUB:=golang/}
diff --git a/pkgtools/pkglint/files/check_test.go b/pkgtools/pkglint/files/check_test.go
index 96428730629..d0e9deb5539 100644
--- a/pkgtools/pkglint/files/check_test.go
+++ b/pkgtools/pkglint/files/check_test.go
@@ -104,13 +104,6 @@ func (s *Suite) TearDownTest(c *check.C) {
func Test__qa(t *testing.T) {
ck := intqa.NewQAChecker(t.Errorf)
- ck.Configure("distinfo.go", "*", "*", -intqa.EMissingTest) // TODO
- ck.Configure("files.go", "*", "*", -intqa.EMissingTest) // TODO
- ck.Configure("licenses.go", "*", "*", -intqa.EMissingTest) // TODO
- ck.Configure("line.go", "*", "*", -intqa.EMissingTest) // TODO
- ck.Configure("linechecker.go", "*", "*", -intqa.EMissingTest) // TODO
- ck.Configure("lines.go", "*", "*", -intqa.EMissingTest) // TODO
- ck.Configure("logging.go", "*", "*", -intqa.EMissingTest) // TODO
ck.Configure("mkline.go", "*", "*", -intqa.EMissingTest) // TODO
ck.Configure("mklineparser.go", "*", "*", -intqa.EMissingTest) // TODO
ck.Configure("mklinechecker.go", "*", "*", -intqa.EMissingTest) // TODO
diff --git a/pkgtools/pkglint/files/distinfo_test.go b/pkgtools/pkglint/files/distinfo_test.go
index 525fb2288fc..dfe065458ab 100644
--- a/pkgtools/pkglint/files/distinfo_test.go
+++ b/pkgtools/pkglint/files/distinfo_test.go
@@ -877,6 +877,63 @@ func (s *Suite) Test_distinfoLinesChecker_checkPatchSha1(c *check.C) {
"ERROR: ~/category/package/distinfo:5: Patch patch-nonexistent does not exist.")
}
+func (s *Suite) Test_distinfoFileInfo_filename(c *check.C) {
+ t := s.Init(c)
+
+ info := distinfoFileInfo{
+ isPatch: no,
+ hashes: []distinfoHash{{
+ line: t.NewLine("irrelevant", 12345, "irrelevant"),
+ filename: "distinfo",
+ algorithm: "irrelevant",
+ hash: "irrelevant",
+ }},
+ }
+
+ t.CheckEquals(info.filename(), RelPath("distinfo"))
+}
+
+func (s *Suite) Test_distinfoFileInfo_line(c *check.C) {
+ t := s.Init(c)
+
+ line := t.NewLine("irrelevant", 12345, "irrelevant")
+ info := distinfoFileInfo{
+ isPatch: no,
+ hashes: []distinfoHash{{
+ line: line,
+ filename: "irrelevant",
+ algorithm: "irrelevant",
+ hash: "irrelevant",
+ }},
+ }
+
+ t.CheckEquals(info.line(), line)
+}
+
+func (s *Suite) Test_distinfoFileInfo_algorithms(c *check.C) {
+ t := s.Init(c)
+
+ info := distinfoFileInfo{
+ isPatch: no,
+ hashes: []distinfoHash{
+ {
+ line: t.NewLine("irrelevant", 12345, "irrelevant"),
+ filename: "irrelevant",
+ algorithm: "SHA1",
+ hash: "irrelevant",
+ },
+ {
+ line: t.NewLine("irrelevant", 12346, "irrelevant"),
+ filename: "irrelevant",
+ algorithm: "Size",
+ hash: "irrelevant",
+ },
+ },
+ }
+
+ t.CheckEquals(info.algorithms(), "SHA1, Size")
+}
+
// The check for versioned distfiles only makes sense if the file
// has the usual hashes for distfiles.
func (s *Suite) Test_distinfoFileInfo_hasDistfileAlgorithms__code_coverage(c *check.C) {
@@ -920,3 +977,41 @@ func (s *Suite) Test_distinfoFileInfo_hasDistfileAlgorithms__code_coverage(c *ch
"ERROR: distinfo:15: Expected SHA1, RMD160, SHA512, Size checksums for "+
"\"dist-d.tar.gz\", got SHA1, RMD160, SHA512, other.")
}
+
+func (s *Suite) Test_computePatchSha1Hex(c *check.C) {
+ t := s.Init(c)
+
+ test := func(expectedHash string, lines ...string) {
+ fileLines := t.NewLines("irrelevant", lines...)
+
+ hash := computePatchSha1Hex(fileLines)
+
+ t.CheckEquals(hash, expectedHash)
+ }
+
+ // The well-known hash for an empty string.
+ test(
+ "da39a3ee5e6b4b0d3255bfef95601890afd80709",
+ nil...,
+ )
+
+ // Any line containing "$" + "NetBSD" is ignored for computing
+ // the hash since the CVS IDs are irrelevant. This assumes that
+ // the patch hunks themselves do not contain CVS IDs, and this
+ // is checked in PatchChecker.checktextCvsID.
+ test(
+ "da39a3ee5e6b4b0d3255bfef95601890afd80709",
+ CvsID,
+ CvsID+"$",
+ // This is not a CVS ID since the final '$' is missing, but it is
+ // close enough. See mk/checksum/distinfo.awk, function patchsum.
+ "Some text $"+"NetBSD-2020",
+ )
+
+ test(
+ // == sha1Hex("--- old\n+++ new\n")
+ "bb5a3903635415d367f47c02990e031469ab0346",
+ "--- old",
+ "+++ new",
+ )
+}
diff --git a/pkgtools/pkglint/files/files_test.go b/pkgtools/pkglint/files/files_test.go
index 6dcdc7d4d80..7cd406d9693 100644
--- a/pkgtools/pkglint/files/files_test.go
+++ b/pkgtools/pkglint/files/files_test.go
@@ -4,6 +4,23 @@ import (
"gopkg.in/check.v1"
)
+func (s *Suite) Test_LoadMk(c *check.C) {
+ t := s.Init(c)
+
+ t.Chdir(".")
+ t.CreateFileLines("filename.mk",
+ "# line 1 \\",
+ "# continues in line 2")
+
+ mklines := LoadMk("filename.mk", nil, 0)
+
+ t.CheckEquals(len(mklines.mklines), 1)
+ // The '#' from line 2 is discarded in nextLogicalLine, to properly
+ // parse multi-line variable assignments that are commented out.
+ // See Test_MkLineParser_MatchVarassign, 'multi-line variable'.
+ t.CheckEquals(mklines.mklines[0].Text, "# line 1 continues in line 2")
+}
+
func (s *Suite) Test_Load(c *check.C) {
t := s.Init(c)
diff --git a/pkgtools/pkglint/files/intqa/qa.go b/pkgtools/pkglint/files/intqa/qa.go
index 766e0b38a4e..83e058b474d 100644
--- a/pkgtools/pkglint/files/intqa/qa.go
+++ b/pkgtools/pkglint/files/intqa/qa.go
@@ -21,25 +21,27 @@ const (
ENone Error = iota + 1
EAll
- // A function or method does not have a corresponding test.
+ // EMissingTest complains if a function or method does not have a
+ // corresponding test.
EMissingTest
- // The name of a test function does not correspond to a program
- // element to be tested.
+ // EMissingTestee complains if the name of a test function does not
+ // correspond to a program element to be tested.
EMissingTestee
- // The tests are not in the same order as their corresponding
- // testees in the main code.
+ // EOrder complains if the tests are not in the same order as their
+ // corresponding testees in the main code.
EOrder
- // The test method does not have a valid name.
+ // EName complains if the test method does not have a valid name.
EName
- // The file of the test method does not correspond to the
- // file of the testee.
+ // EFile complains if the file of the test method does not correspond
+ // to the file of the testee.
EFile
- // All methods of a type must be in the same file as the type definition.
+ // EMethodsSameFile complains if a method of a type is in a different
+ // file than its corresponding type definition.
EMethodsSameFile
)
diff --git a/pkgtools/pkglint/files/licenses.go b/pkgtools/pkglint/files/licenses.go
index dd2a79c65c1..4f014531fcb 100644
--- a/pkgtools/pkglint/files/licenses.go
+++ b/pkgtools/pkglint/files/licenses.go
@@ -23,6 +23,22 @@ func (lc *LicenseChecker) Check(value string, op MkOperator) {
cond.Walk(lc.checkNode)
}
+func (lc *LicenseChecker) checkNode(cond *licenses.Condition) {
+ if name := cond.Name; name != "" && name != "append-placeholder" {
+ lc.checkName(name)
+ return
+ }
+
+ if cond.And && cond.Or {
+ lc.MkLine.Errorf("AND and OR operators in license conditions can only be combined using parentheses.")
+ lc.MkLine.Explain(
+ "Examples for valid license conditions are:",
+ "",
+ "\tlicense1 AND license2 AND (license3 OR license4)",
+ "\t(((license1 OR license2) AND (license3 OR license4)))")
+ }
+}
+
func (lc *LicenseChecker) checkName(license string) {
licenseFile := NewCurrPath("")
pkg := lc.MkLines.pkg
@@ -54,19 +70,3 @@ func (lc *LicenseChecker) checkName(license string) {
seeGuide("Handling licenses", "handling-licenses")))
}
}
-
-func (lc *LicenseChecker) checkNode(cond *licenses.Condition) {
- if name := cond.Name; name != "" && name != "append-placeholder" {
- lc.checkName(name)
- return
- }
-
- if cond.And && cond.Or {
- lc.MkLine.Errorf("AND and OR operators in license conditions can only be combined using parentheses.")
- lc.MkLine.Explain(
- "Examples for valid license conditions are:",
- "",
- "\tlicense1 AND license2 AND (license3 OR license4)",
- "\t(((license1 OR license2) AND (license3 OR license4)))")
- }
-}
diff --git a/pkgtools/pkglint/files/licenses_test.go b/pkgtools/pkglint/files/licenses_test.go
index 4fa8f20a7d3..be7263a4ca5 100644
--- a/pkgtools/pkglint/files/licenses_test.go
+++ b/pkgtools/pkglint/files/licenses_test.go
@@ -15,7 +15,8 @@ func (s *Suite) Test_LicenseChecker_Check(c *check.C) {
"LICENSE=\t"+licenseValue)
mklines.ForEach(func(mkline *MkLine) {
- (&LicenseChecker{mklines, mkline}).Check(mkline.Value(), opAssign)
+ ck := LicenseChecker{mklines, mkline}
+ ck.Check(mkline.Value(), mkline.Op())
})
t.CheckOutput(diagnostics)
@@ -49,6 +50,31 @@ func (s *Suite) Test_LicenseChecker_Check(c *check.C) {
nil...)
}
+func (s *Suite) Test_LicenseChecker_checkNode(c *check.C) {
+ t := s.Init(c)
+
+ t.CreateFileLines("licenses/gnu-gpl-v2",
+ "The licenses for most software are designed to take away ...")
+
+ test := func(assignment string, diagnostics ...string) {
+ mklines := t.SetUpFileMkLines("Makefile",
+ assignment)
+
+ mklines.ForEach(func(mkline *MkLine) {
+ ck := LicenseChecker{mklines, mkline}
+ ck.Check(mkline.Value(), mkline.Op())
+ })
+
+ t.CheckOutput(diagnostics)
+ }
+
+ test("LICENSE=\tfirst second",
+ "ERROR: ~/Makefile:1: Parse error for license condition \"first second\".")
+
+ test("LICENSE+=\tadded",
+ "ERROR: ~/Makefile:1: Parse error for appended license condition \"added\".")
+}
+
func (s *Suite) Test_LicenseChecker_checkName__LICENSE_FILE(c *check.C) {
t := s.Init(c)
diff --git a/pkgtools/pkglint/files/line_test.go b/pkgtools/pkglint/files/line_test.go
index fb18d6657b0..a3e90c88913 100644
--- a/pkgtools/pkglint/files/line_test.go
+++ b/pkgtools/pkglint/files/line_test.go
@@ -2,14 +2,105 @@ package pkglint
import (
"gopkg.in/check.v1"
+ "netbsd.org/pkglint/regex"
)
+func (s *Suite) Test_RawLine_Orig(c *check.C) {
+ t := s.Init(c)
+
+ line := t.NewLine("filename", 123, "text")
+
+ t.CheckEquals(line.raw[0].orignl, "text\n")
+ t.CheckEquals(line.raw[0].Orig(), "text")
+}
+
+func (s *Suite) Test_NewLocation(c *check.C) {
+ t := s.Init(c)
+
+ loc := NewLocation("subdir/filename", 123)
+
+ t.CheckEquals(loc.Filename, NewCurrPathString("subdir/filename"))
+ t.CheckEquals(loc.lineno, 123)
+}
+
+func (s *Suite) Test_Location_Lineno(c *check.C) {
+ t := s.Init(c)
+
+ loc := NewLocation("subdir/filename", 123)
+
+ t.CheckEquals(loc.Lineno(0), 123)
+}
+
+func (s *Suite) Test_Location_File(c *check.C) {
+ t := s.Init(c)
+
+ loc := NewLocation("subdir/filename", 123)
+
+ t.CheckEquals(loc.File("other"), NewCurrPathString("subdir/other"))
+}
+
func (s *Suite) Test_NewLine__assertion(c *check.C) {
t := s.Init(c)
t.ExpectAssert(func() { NewLine("filename", 123, "text", nil) })
}
+func (s *Suite) Test_NewLineMulti(c *check.C) {
+ t := s.Init(c)
+
+ line := NewLineMulti("subdir/filename", 123, "", []*RawLine{nil, nil, nil})
+
+ t.CheckEquals(line.Filename(), NewCurrPathString("subdir/filename"))
+ t.CheckEquals(line.Linenos(), "123--125")
+}
+
+func (s *Suite) Test_NewLineEOF(c *check.C) {
+ t := s.Init(c)
+
+ line := NewLineEOF("subdir/filename")
+
+ t.CheckEquals(line.Filename(), NewCurrPathString("subdir/filename"))
+ t.CheckEquals(line.Linenos(), "EOF")
+}
+
+func (s *Suite) Test_NewLineWhole(c *check.C) {
+ t := s.Init(c)
+
+ line := NewLineWhole("subdir/filename")
+
+ t.CheckEquals(line.Filename(), NewCurrPathString("subdir/filename"))
+ t.CheckEquals(line.Linenos(), "")
+}
+
+func (s *Suite) Test_Line_Filename(c *check.C) {
+ t := s.Init(c)
+
+ line := t.NewLine("subdir/filename", 123, "")
+
+ t.CheckEquals(line.Filename(), NewCurrPathString("subdir/filename"))
+}
+
+func (s *Suite) Test_Line_File(c *check.C) {
+ t := s.Init(c)
+
+ line := t.NewLine("subdir/filename", 123, "")
+
+ t.CheckEquals(line.File("subsub/nested"),
+ NewCurrPathString("subdir/subsub/nested"))
+}
+
+func (s *Suite) Test_Line_Linenos(c *check.C) {
+ t := s.Init(c)
+
+ single := t.NewLine("filename", 123, "")
+ whole := NewLineWhole("filename")
+ multi := NewLineMulti("filename", 123, "text", []*RawLine{nil, nil, nil})
+
+ t.CheckEquals(single.Linenos(), "123")
+ t.CheckEquals(whole.Linenos(), "")
+ t.CheckEquals(multi.Linenos(), "123--125")
+}
+
func (s *Suite) Test_Line_RelLine__assertion(c *check.C) {
t := s.Init(c)
@@ -28,13 +119,148 @@ func (s *Suite) Test_Line_RelLocation__assertion(c *check.C) {
t.ExpectAssert(func() { line.RelLocation(whole.Location) })
}
+func (s *Suite) Test_Line_Rel(c *check.C) {
+ t := s.Init(c)
+
+ t.Chdir(".")
+ line1 := t.NewLine("subdir/filename", 123, "from")
+
+ rel := line1.Rel("other/filename")
+
+ t.CheckEquals(rel, NewRelPathString("../other/filename"))
+}
+
func (s *Suite) Test_Line_IsMultiline(c *check.C) {
t := s.Init(c)
t.CheckEquals(t.NewLine("filename", 123, "text").IsMultiline(), false)
+
t.CheckEquals(NewLineEOF("filename").IsMultiline(), false)
- t.CheckEquals(NewLineMulti("filename", 123, "text", []*RawLine{nil, nil, nil}).IsMultiline(), true)
+ multi := NewLineMulti("filename", 123, "text", []*RawLine{nil, nil, nil})
+ t.CheckEquals(multi.IsMultiline(), true)
+}
+
+func (s *Suite) Test_Line_RawText(c *check.C) {
+ t := s.Init(c)
+
+ t.SetUpCommandLine("--show-autofix")
+ line := t.NewLine("filename", 123, "text")
+
+ t.CheckEquals(line.RawText(0), "text")
+
+ fix := line.Autofix()
+ fix.Notef("Replacing.")
+ fix.Replace("text", "replaced")
+ fix.Apply()
+
+ t.CheckEquals(line.RawText(0), "replaced")
+
+ t.CheckOutputLines(
+ "NOTE: filename:123: Replacing.",
+ "AUTOFIX: filename:123: Replacing \"text\" with \"replaced\".")
+}
+
+func (s *Suite) Test_Line_IsCvsID(c *check.C) {
+ t := s.Init(c)
+
+ test := func(text string, prefix regex.Pattern, expectedID bool, expectedExpanded bool) {
+ line := t.NewLine("filename", 123, text)
+
+ id, expanded := line.IsCvsID(prefix)
+
+ t.CheckEquals(id, expectedID)
+ t.CheckEquals(expanded, expectedExpanded)
+ }
+
+ test(CvsID, ``, true, false)
+
+ test("$"+"NetBSD: ... $", ``, true, true)
+}
+
+func (s *Suite) Test_Line_Errorf(c *check.C) {
+ t := s.Init(c)
+
+ line := t.NewLine("filename", 123, "text")
+
+ line.Errorf("For your information.")
+
+ t.CheckOutputLines(
+ "ERROR: filename:123: For your information.")
+}
+
+func (s *Suite) Test_Line_Warnf(c *check.C) {
+ t := s.Init(c)
+
+ line := t.NewLine("filename", 123, "text")
+
+ line.Warnf("For your information.")
+
+ t.CheckOutputLines(
+ "WARN: filename:123: For your information.")
+}
+
+func (s *Suite) Test_Line_Notef(c *check.C) {
+ t := s.Init(c)
+
+ line := t.NewLine("filename", 123, "text")
+
+ line.Notef("For your information.")
+
+ t.CheckOutputLines(
+ "NOTE: filename:123: For your information.")
+}
+
+func (s *Suite) Test_Line_Explain(c *check.C) {
+ t := s.Init(c)
+
+ t.SetUpCommandLine("--explain")
+ line := t.NewLine("filename", 123, "text")
+
+ line.Notef("For your information.")
+ line.Explain(
+ "First line",
+ "continues.")
+
+ t.CheckOutputLines(
+ "NOTE: filename:123: For your information.",
+ "",
+ "\tFirst line continues.",
+ "")
+}
+
+func (s *Suite) Test_Line_Explain__without_diagnostic(c *check.C) {
+ t := s.Init(c)
+
+ t.SetUpCommandLine("--explain")
+ line := t.NewLine("filename", 123, "text")
+
+ // It is unusual to have an explanation without any preceding
+ // diagnostic.
+ line.Explain(
+ "First line",
+ "continues.")
+
+ // There is no empty line above the explanation since that is the
+ // job of the SeparatorWriter, which only steps in if there is a
+ // diagnostic above the explanation.
+ t.CheckOutputLines(
+ "\tFirst line continues.",
+ "")
+}
+
+func (s *Suite) Test_Line_String(c *check.C) {
+ t := s.Init(c)
+
+ line := t.NewLine("filename", 123, "text")
+ t.CheckEquals(line.String(), "filename:123: text")
+
+ // Since Line.String is only used for debugging purposes, trailing
+ // whitespace is OK.
+ t.CheckEquals(NewLineEOF("filename").String(), "filename:EOF: ")
+
+ multi := NewLineMulti("filename", 123, "text", []*RawLine{nil, nil, nil})
+ t.CheckEquals(multi.String(), "filename:123--125: text")
}
func (s *Suite) Test_Line_Autofix__reuse_incomplete(c *check.C) {
diff --git a/pkgtools/pkglint/files/linechecker_test.go b/pkgtools/pkglint/files/linechecker_test.go
index fa14c4e497d..f9c00011622 100644
--- a/pkgtools/pkglint/files/linechecker_test.go
+++ b/pkgtools/pkglint/files/linechecker_test.go
@@ -17,6 +17,20 @@ func (s *Suite) Test_LineChecker_CheckLength(c *check.C) {
"WARN: DESCR:1: Line too long (should be no more than 20 characters).")
}
+func (s *Suite) Test_LineChecker_CheckValidCharacters(c *check.C) {
+ t := s.Init(c)
+
+ doTest := func(autofix bool) {
+ line := t.NewLine("filename", 32, "The letter \u00DC is an umlaut.")
+
+ LineChecker{line}.CheckValidCharacters()
+ }
+
+ t.ExpectDiagnosticsAutofix(
+ doTest,
+ "WARN: filename:32: Line contains invalid characters (U+00DC).")
+}
+
func (s *Suite) Test_LineChecker_CheckTrailingWhitespace(c *check.C) {
t := s.Init(c)
diff --git a/pkgtools/pkglint/files/lines_test.go b/pkgtools/pkglint/files/lines_test.go
index 42baed61c1f..4264708db36 100644
--- a/pkgtools/pkglint/files/lines_test.go
+++ b/pkgtools/pkglint/files/lines_test.go
@@ -2,6 +2,83 @@ package pkglint
import "gopkg.in/check.v1"
+func (s *Suite) Test_NewLines(c *check.C) {
+ t := s.Init(c)
+
+ lines := t.NewLines("filename",
+ "text")
+
+ t.CheckEquals(lines.Filename, NewCurrPathString("filename"))
+}
+
+func (s *Suite) Test_Lines_Len(c *check.C) {
+ t := s.Init(c)
+
+ lines := t.NewLines("filename",
+ "one",
+ "two",
+ "three")
+
+ t.CheckEquals(lines.Len(), 3)
+}
+
+func (s *Suite) Test_Lines_LastLine(c *check.C) {
+ t := s.Init(c)
+
+ lines := t.NewLines("filename",
+ "text")
+
+ whole := lines.LastLine()
+
+ t.CheckEquals(whole.String(), "filename:1: text")
+}
+
+func (s *Suite) Test_Lines_EOFLine(c *check.C) {
+ t := s.Init(c)
+
+ lines := t.NewLines("filename",
+ "text")
+
+ whole := lines.EOFLine()
+
+ // The text of the line after the ': ' is empty.
+ t.CheckEquals(whole.String(), "filename:EOF: ")
+}
+
+func (s *Suite) Test_Lines_Whole(c *check.C) {
+ t := s.Init(c)
+
+ lines := t.NewLines("filename",
+ "text")
+
+ whole := lines.Whole()
+
+ // The lineno between the '::' is empty.
+ // The text of the line after the ': ' is empty as well.
+ t.CheckEquals(whole.String(), "filename:: ")
+}
+
+func (s *Suite) Test_Lines_SaveAutofixChanges(c *check.C) {
+ t := s.Init(c)
+
+ doTest := func(autofix bool) {
+ lines := t.SetUpFileLines("filename",
+ "before")
+
+ fix := lines.Lines[0].Autofix()
+ fix.Notef("Replacing.")
+ fix.Replace("before", "after")
+ fix.Apply()
+
+ lines.SaveAutofixChanges()
+ }
+
+ t.ExpectDiagnosticsAutofix(
+ doTest,
+ "NOTE: ~/filename:1: Replacing.",
+ "AUTOFIX: ~/filename:1: Replacing \"before\" with \"after\".")
+}
+
func (s *Suite) Test_Lines_CheckCvsID(c *check.C) {
t := s.Init(c)
diff --git a/pkgtools/pkglint/files/logging_test.go b/pkgtools/pkglint/files/logging_test.go
index 4702902f954..1a3115864a8 100644
--- a/pkgtools/pkglint/files/logging_test.go
+++ b/pkgtools/pkglint/files/logging_test.go
@@ -286,6 +286,48 @@ func (s *Suite) Test_Logger_Diag__source_duplicates(c *check.C) {
"category/package1", "category/package2"))
}
+func (s *Suite) Test_Logger_FirstTime__not_verbose(c *check.C) {
+ t := s.Init(c)
+
+ G.Logger.verbose = false // as in a realistic run
+
+ t.CheckEquals(G.Logger.FirstTime("filename", "123", "Message."), true)
+ t.CheckEquals(G.Logger.FirstTime("filename", "123", "Message."), false)
+ t.CheckEquals(G.Logger.FirstTime("filename", "124", "Message."), true)
+ t.CheckEquals(G.Logger.FirstTime("filename", "124", "Message."), false)
+ t.CheckEquals(G.Logger.FirstTime("filename", "124", "Message."), false)
+}
+
+func (s *Suite) Test_Logger_Relevant(c *check.C) {
+ t := s.Init(c)
+
+ t.SetUpCommandLine(nil...)
+
+ t.CheckEquals(G.Logger.Relevant("Options should not contain whitespace."), true)
+ t.CheckEquals(G.Logger.suppressDiag, false)
+ t.CheckEquals(G.Logger.suppressExpl, false) // XXX: Why not true?
+
+ t.SetUpCommandLine("--only", "whitespace")
+
+ t.CheckEquals(G.Logger.Relevant("Options should not contain whitespace."), true)
+ t.CheckEquals(G.Logger.suppressDiag, false)
+ t.CheckEquals(G.Logger.suppressExpl, false) // XXX: Why not true?
+
+ t.CheckEquals(G.Logger.Relevant("Options should not contain space."), false)
+ t.CheckEquals(G.Logger.suppressDiag, true)
+ t.CheckEquals(G.Logger.suppressExpl, true)
+
+ t.SetUpCommandLine("--explain")
+
+ t.CheckEquals(G.Logger.Relevant("Options should not contain whitespace."), true)
+ t.CheckEquals(G.Logger.suppressDiag, false)
+ t.CheckEquals(G.Logger.suppressExpl, false)
+
+ t.CheckEquals(G.Logger.Relevant("Options should not contain space."), true)
+ t.CheckEquals(G.Logger.suppressDiag, false)
+ t.CheckEquals(G.Logger.suppressExpl, false)
+}
+
func (s *Suite) Test_Logger_shallBeLogged(c *check.C) {
t := s.Init(c)
@@ -642,6 +684,61 @@ func (s *Suite) Test_Logger_writeSource__first_warn_then_autofix(c *check.C) {
"+\tThe last line")
}
+func (s *Suite) Test_Logger_writeDiff(c *check.C) {
+ t := s.Init(c)
+
+ t.SetUpCommandLine("--show-autofix")
+ line := t.NewLine("filename", 123, "before")
+ fix := line.Autofix()
+ fix.Silent()
+ fix.Replace("before", "after")
+ fix.Apply()
+
+ G.Logger.writeDiff(line)
+
+ // The diff lines are indented with a tab so that the indentation
+ // from the actual lines is properly represented in the output.
+ // If a space had been used here instead of the tab, the output
+ // would become garbled.
+ t.CheckOutputLines(
+ "AUTOFIX: filename:123: Replacing \"before\" with \"after\".",
+ "-\tbefore",
+ "+\tafter")
+}
+
+func (s *Suite) Test_Logger_writeLine(c *check.C) {
+ t := s.Init(c)
+
+ G.Logger.writeLine("> ", "\u0007\u00FC text")
+
+ t.CheckOutputLines(
+ "> <U+0007><U+00FC> text")
+}
+
+func (s *Suite) Test_Logger_IsAutofix__default(c *check.C) {
+ t := s.Init(c)
+
+ t.SetUpCommandLine("-Wall")
+
+ t.CheckEquals(G.Logger.IsAutofix(), false)
+}
+
+func (s *Suite) Test_Logger_IsAutofix__show_autofix(c *check.C) {
+ t := s.Init(c)
+
+ t.SetUpCommandLine("-Wall", "--show-autofix")
+
+ t.CheckEquals(G.Logger.IsAutofix(), true)
+}
+
+func (s *Suite) Test_Logger_IsAutofix__autofix(c *check.C) {
+ t := s.Init(c)
+
+ t.SetUpCommandLine("-Wall", "--autofix")
+
+ t.CheckEquals(G.Logger.IsAutofix(), true)
+}
+
// Calling Logf without further preparation just logs the message.
// Suppressing duplicate messages or filtering messages happens
// in other methods of the Logger, namely Relevant, FirstTime, Diag.
@@ -1162,6 +1259,58 @@ func (s *Suite) Test_SeparatorWriter(c *check.C) {
t.CheckEquals(sb.String(), "a\nb\n\nc\n")
}
+func (s *Suite) Test_NewSeparatorWriter(c *check.C) {
+ t := s.Init(c)
+
+ var sb strings.Builder
+ wr := NewSeparatorWriter(&sb)
+
+ t.CheckEquals(wr.out, &sb)
+ t.CheckEquals(wr.state, uint8(3))
+ t.CheckEquals(wr.line.Len(), 0)
+}
+
+func (s *Suite) Test_SeparatorWriter_WriteLine(c *check.C) {
+ t := s.Init(c)
+
+ var sb strings.Builder
+ wr := NewSeparatorWriter(&sb)
+
+ wr.WriteLine("first")
+ wr.Separate()
+
+ t.CheckEquals(sb.String(), "first\n")
+
+ wr.WriteLine("second")
+
+ t.CheckEquals(sb.String(), "first\n\nsecond\n")
+}
+
+func (s *Suite) Test_SeparatorWriter_Write(c *check.C) {
+ t := s.Init(c)
+
+ var sb strings.Builder
+ wr := NewSeparatorWriter(&sb)
+
+ wr.Write("first")
+ wr.Write("")
+
+ t.CheckEquals(wr.line.String(), "first")
+
+ wr.Write("\n")
+
+ t.CheckEquals(wr.line.String(), "")
+ t.CheckEquals(sb.String(), "first\n")
+
+ wr.Separate()
+
+ t.CheckEquals(sb.String(), "first\n")
+
+ wr.WriteLine("second")
+
+ t.CheckEquals(sb.String(), "first\n\nsecond\n")
+}
+
func (s *Suite) Test_SeparatorWriter_Separate(c *check.C) {
t := s.Init(c)
@@ -1199,6 +1348,31 @@ func (s *Suite) Test_SeparatorWriter_Separate__at_the_beginning(c *check.C) {
t.CheckEquals(sb.String(), "a\n")
}
+func (s *Suite) Test_SeparatorWriter_write(c *check.C) {
+ t := s.Init(c)
+
+ var sb strings.Builder
+ wr := NewSeparatorWriter(&sb)
+
+ t.CheckEquals(wr.state, uint8(3))
+
+ wr.write('a')
+
+ t.CheckEquals(wr.state, uint8(1))
+
+ wr.write('\n')
+
+ t.CheckEquals(wr.state, uint8(0))
+
+ wr.Separate()
+
+ t.CheckEquals(wr.state, uint8(2))
+
+ wr.write('\n')
+
+ t.CheckEquals(wr.state, uint8(3))
+}
+
func (s *Suite) Test_SeparatorWriter_Flush(c *check.C) {
t := s.Init(c)
diff --git a/pkgtools/pkglint/files/makepat/pat_test.go b/pkgtools/pkglint/files/makepat/pat_test.go
index 543c6510575..3b9ec9ae56b 100644
--- a/pkgtools/pkglint/files/makepat/pat_test.go
+++ b/pkgtools/pkglint/files/makepat/pat_test.go
@@ -175,6 +175,7 @@ func Test_Intersect(t *testing.T) {
{"N-*", "N-*", "N-*", true, true},
{"N-9.99.*", "N-[1-9].*", "", false, true},
{"N-9.99.*", "N-[1-9][0-9].*", "", false, false},
+ {"*.c", "*.h", "", false, false},
}
for _, tt := range tests {
t.Run(tt.str, func(t *testing.T) {
diff --git a/pkgtools/pkglint/files/mklinechecker.go b/pkgtools/pkglint/files/mklinechecker.go
index 082a019549f..c52228a28e3 100644
--- a/pkgtools/pkglint/files/mklinechecker.go
+++ b/pkgtools/pkglint/files/mklinechecker.go
@@ -112,7 +112,8 @@ func (ck MkLineChecker) checkTextWrksrcDotDot(text string) {
func (ck MkLineChecker) checkTextRpath(text string) {
mkline := ck.MkLine
- if mkline.IsVarassign() && mkline.Varname() == "BUILDLINK_TRANSFORM" &&
+ if mkline.IsVarassign() &&
+ varnameBase(mkline.Varname()) == "BUILDLINK_TRANSFORM" &&
hasPrefix(mkline.Value(), "rm:") {
return
diff --git a/pkgtools/pkglint/files/mklinechecker_test.go b/pkgtools/pkglint/files/mklinechecker_test.go
index a412a2cd1b5..478eaa163b4 100644
--- a/pkgtools/pkglint/files/mklinechecker_test.go
+++ b/pkgtools/pkglint/files/mklinechecker_test.go
@@ -181,14 +181,18 @@ func (s *Suite) Test_MkLineChecker_checkTextRpath(c *check.C) {
t.SetUpVartypes()
mklines := t.NewMkLines("filename.mk",
MkCvsID,
- "BUILDLINK_TRANSFORM+=\trm:-Wl,-R/usr/lib",
- "BUILDLINK_TRANSFORM+=\trm:-Wl,-rpath,/usr/lib",
- "BUILDLINK_TRANSFORM+=\topt:-Wl,-rpath,/usr/lib")
+ "BUILDLINK_TRANSFORM+=\t\trm:-Wl,-R/usr/lib",
+ "BUILDLINK_TRANSFORM+=\t\trm:-Wl,-rpath,/usr/lib",
+ "BUILDLINK_TRANSFORM+=\t\topt:-Wl,-rpath,/usr/lib",
+ "BUILDLINK_TRANSFORM.pkgbase+=\trm:-Wl,-R/usr/lib",
+ "BUILDLINK_TRANSFORM.pkgbase+=\trm:-Wl,-rpath,/usr/lib",
+ "BUILDLINK_TRANSFORM.pkgbase+=\topt:-Wl,-rpath,/usr/lib")
mklines.Check()
t.CheckOutputLines(
- "WARN: filename.mk:4: Please use ${COMPILER_RPATH_FLAG} instead of \"-Wl,-rpath,\".")
+ "WARN: filename.mk:4: Please use ${COMPILER_RPATH_FLAG} instead of \"-Wl,-rpath,\".",
+ "WARN: filename.mk:7: Please use ${COMPILER_RPATH_FLAG} instead of \"-Wl,-rpath,\".")
}
func (s *Suite) Test_MkLineChecker_checkVartype__simple_type(c *check.C) {
diff --git a/pkgtools/pkglint/files/mklineparser_test.go b/pkgtools/pkglint/files/mklineparser_test.go
index ee113e4ccdc..8c7b255bc28 100644
--- a/pkgtools/pkglint/files/mklineparser_test.go
+++ b/pkgtools/pkglint/files/mklineparser_test.go
@@ -402,6 +402,9 @@ func (s *Suite) Test_MkLineParser_MatchVarassign(c *check.C) {
"",
"")
+ // A multi-line variable assignment that is commented out.
+ // The '#' from the continued lines are discarded since they are not
+ // part of the variable value.
testLine(
lines(
"#VAR=\t\t\t\\",
diff --git a/pkgtools/pkglint/files/package.go b/pkgtools/pkglint/files/package.go
index 0e922768b36..d273ee7557f 100644
--- a/pkgtools/pkglint/files/package.go
+++ b/pkgtools/pkglint/files/package.go
@@ -7,10 +7,6 @@ import (
"strings"
)
-// TODO: What about package names that refer to other variables?
-// TODO: Allow a hyphen in the middle of a version number.
-const rePkgname = `^([\w\-.+]+)-([0-9][.0-9A-Z_a-z]*)$`
-
// Package is the pkgsrc package that is currently checked.
//
// Most of the information is loaded first, and after loading the actual checks take place.
@@ -1266,7 +1262,7 @@ func (pkg *Package) determineEffectivePkgVars() {
pkg.checkPkgnameRedundant(pkgnameLine, pkgname, distname)
- if pkgname == "" && distnameLine != nil && !containsVarUse(distname) && !matches(distname, rePkgname) {
+ if pkgname == "" && distnameLine != nil && !containsVarUse(distname) && !matchesPkgname(distname) {
distnameLine.Warnf("As DISTNAME is not a valid package name, please define the PKGNAME explicitly.")
}
@@ -1275,7 +1271,7 @@ func (pkg *Package) determineEffectivePkgVars() {
}
if effname != "" && !containsVarUse(effname) {
- if m, m1, m2 := match2(effname, rePkgname); m {
+ if m, m1, m2 := matchPkgname(effname); m {
pkg.EffectivePkgname = effname + pkg.nbPart()
pkg.EffectivePkgnameLine = pkgnameLine
pkg.EffectivePkgbase = m1
@@ -1284,7 +1280,7 @@ func (pkg *Package) determineEffectivePkgVars() {
}
if pkg.EffectivePkgnameLine == nil && distname != "" && !containsVarUse(distname) {
- if m, m1, m2 := match2(distname, rePkgname); m {
+ if m, m1, m2 := matchPkgname(distname); m {
pkg.EffectivePkgname = distname + pkg.nbPart()
pkg.EffectivePkgnameLine = distnameLine
pkg.EffectivePkgbase = m1
@@ -1371,7 +1367,7 @@ func (pkg *Package) checkPossibleDowngrade() {
defer trace.Call0()()
}
- m, _, pkgversion := match2(pkg.EffectivePkgname, rePkgname)
+ m, _, pkgversion := matchPkgname(pkg.EffectivePkgname)
if !m {
return
}
@@ -1795,3 +1791,17 @@ func NewPlistContent() PlistContent {
make(map[RelPath]*PlistLine),
make(map[string]bool)}
}
+
+// matchPkgname tests whether the string has the form of a package name that
+// does not contain any variable expressions.
+func matchPkgname(s string) (m bool, base string, version string) {
+ // TODO: Allow a hyphen in the middle of a version number.
+ return match2(s, `^([\w\-.+]+)-([0-9][.0-9A-Z_a-z]*)$`)
+}
+
+// matchPkgname tests whether the string has the form of a package name that
+// does not contain any variable expressions.
+func matchesPkgname(s string) bool {
+ m, _, _ := matchPkgname(s)
+ return m
+}
diff --git a/pkgtools/pkglint/files/pkglint_test.go b/pkgtools/pkglint/files/pkglint_test.go
index f73c397b698..4983af4b02e 100644
--- a/pkgtools/pkglint/files/pkglint_test.go
+++ b/pkgtools/pkglint/files/pkglint_test.go
@@ -515,7 +515,6 @@ func (s *Suite) Test_Pkglint_Check__invalid_files_before_import(c *check.C) {
t.CheckOutputLines(
"ERROR: ~/category/package/Makefile.orig: Must be cleaned up before committing the package.",
"ERROR: ~/category/package/Makefile.rej: Must be cleaned up before committing the package.",
- "ERROR: ~/category/package/Makefile~: Must be cleaned up before committing the package.",
"ERROR: ~/category/package/work: Must be cleaned up before committing the package.")
}
@@ -1117,9 +1116,11 @@ func (s *Suite) Test_Pkglint_checkReg__unknown_file_in_patches(c *check.C) {
"Patch files should be named \"patch-\", followed by letters, '-', '_', '.', and digits only.")
}
-func (s *Suite) Test_Pkglint_checkReg__patch_for_Makefile_fragment(c *check.C) {
+func (s *Suite) Test_Pkglint_checkReg__patch_for_makefile_fragment(c *check.C) {
t := s.Init(c)
+ // This is a patch file, and even though its name ends with ".mk", it must
+ // not be interpreted as a makefile fragment.
t.CreateFileDummyPatch("category/package/patches/patch-compiler.mk")
t.Chdir("category/package")
@@ -1197,6 +1198,25 @@ func (s *Suite) Test_Pkglint_checkReg__wip_commit_message(c *check.C) {
t.CheckOutputEmpty()
}
+func (s *Suite) Test_Pkglint_checkReg__file_ignored_by_CVS(c *check.C) {
+ t := s.Init(c)
+
+ t.SetUpPackage("category/package")
+ t.Chdir("category/package")
+ t.CreateFileLines("PLIST",
+ PlistCvsID,
+ "bin/program")
+ t.CreateFileLines("PLIST.~1.7.~",
+ PlistCvsID,
+ "bin/program")
+ t.FinishSetUp()
+
+ G.Check(".")
+
+ // Files ending in "~" should be ignored since CVS ignores them as well.
+ t.CheckOutputEmpty()
+}
+
func (s *Suite) Test_Pkglint_checkRegCvsSubst(c *check.C) {
t := s.Init(c)
diff --git a/pkgtools/pkglint/files/pkgsrc.go b/pkgtools/pkglint/files/pkgsrc.go
index cbd3bb73e96..e131318fc23 100644
--- a/pkgtools/pkglint/files/pkgsrc.go
+++ b/pkgtools/pkglint/files/pkgsrc.go
@@ -471,7 +471,7 @@ func (src *Pkgsrc) parseSuggestedUpdates(lines *Lines) []SuggestedUpdate {
llex.Skip()
if m, pkgname, comment := match2(text, `^\to[\t ]([^\t ]+)(?:[\t ]*(.+))?$`); m {
- if m, pkgbase, pkgversion := match2(pkgname, rePkgname); m {
+ if m, pkgbase, pkgversion := matchPkgname(pkgname); m {
if hasPrefix(comment, "[") && hasSuffix(comment, "]") {
comment = comment[1 : len(comment)-1]
}
diff --git a/pkgtools/pkglint/files/pkgver/vercmp_test.go b/pkgtools/pkglint/files/pkgver/vercmp_test.go
index a3ecd8066c4..43ac7dfce0d 100644
--- a/pkgtools/pkglint/files/pkgver/vercmp_test.go
+++ b/pkgtools/pkglint/files/pkgver/vercmp_test.go
@@ -35,6 +35,8 @@ func (s *Suite) Test_Compare(c *check.C) {
{"5.0"},
{"5.0nb5"},
{"5.5", "5.005"},
+ {"2021.06.17", "2021.6.17"},
+ {"2021.12.01", "2021.12.1"},
{"20151110"},
}
diff --git a/pkgtools/pkglint/files/util.go b/pkgtools/pkglint/files/util.go
index 478dc1ff302..ca24e48f3c3 100644
--- a/pkgtools/pkglint/files/util.go
+++ b/pkgtools/pkglint/files/util.go
@@ -341,7 +341,9 @@ func isIgnoredFilename(filename string) bool {
case "CVS", ".svn", ".git", ".hg", ".idea":
return true
}
- return hasPrefix(filename, ".#")
+
+ // https://www.gnu.org/software/trans-coord/manual/cvs/cvs.html#cvsignore
+ return hasPrefix(filename, ".#") || hasSuffix(filename, "~")
}
// Checks whether a file is already committed to the CVS repository.
diff --git a/pkgtools/pkglint/files/util_test.go b/pkgtools/pkglint/files/util_test.go
index 812d419ad3b..53c4ee8e26e 100644
--- a/pkgtools/pkglint/files/util_test.go
+++ b/pkgtools/pkglint/files/util_test.go
@@ -214,6 +214,12 @@ func (s *Suite) Test_isIgnoredFilename(c *check.C) {
// There is actually an IDEA plugin for pkgsrc.
// See https://github.com/rillig/intellij-pkgsrc.
test(".idea", true)
+
+ // After editing a file, run 'cvs up -CA'. This creates a backup.
+ test(".#Makefile.1.689", true)
+
+ // https://www.gnu.org/software/trans-coord/manual/cvs/cvs.html#cvsignore
+ test("PLIST.~1.7.~", true)
}
func (s *Suite) Test_isLocallyModified(c *check.C) {
diff --git a/pkgtools/pkglint/files/vartypecheck.go b/pkgtools/pkglint/files/vartypecheck.go
index ad6c7d05446..393bf744919 100644
--- a/pkgtools/pkglint/files/vartypecheck.go
+++ b/pkgtools/pkglint/files/vartypecheck.go
@@ -1054,7 +1054,7 @@ func (cv *VartypeCheck) Perms() {
func (cv *VartypeCheck) Pkgname() {
value := cv.Value
- if cv.Op != opUseMatch && value == cv.ValueNoVar && !matches(value, rePkgname) {
+ if cv.Op != opUseMatch && value == cv.ValueNoVar && !matchesPkgname(value) {
cv.Warnf("%q is not a valid package name.", value)
cv.Explain(
"A valid package name has the form packagename-version, where version",