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 5.6.12



details:   https://anonhg.NetBSD.org/pkgsrc/rev/e5e9d9d57178
branches:  trunk
changeset: 318503:e5e9d9d57178
user:      rillig <rillig%pkgsrc.org@localhost>
date:      Sat Jan 26 16:31:33 2019 +0000

description:
pkgtools/pkglint: update to 5.6.12

Changes since 5.6.11:

* In buildlink3.mk files, print the paths relative
  to the line, not to the pkgsrc root.

* When explaining that a variable cannot be set/used because of wrong
  permissions, list the permissions. This provides more transparency
  than just stating that the desired action is not allowed.

* When pkglint checks a pkgsrc-wip package, don't warn about malformed
  lines in doc/CHANGES-* since pkgsrc-wip users typically cannot do
  anything about these errors.

* In profiling mode, not only the code coverage and the performance
  statistics are dumped, the whole heap is also dumped to see which
  parts of pkglint consume the most heap memory. Pkglint now needs
  less heap memory than before, which mainly affects full scans.

* The checks for absolute pathnames have gone. They were of questionable
  value since pkglint has failed to give proper advice on how to fix
  them properly, at least for the last 12 years.

* The check that pkgsrc-wip packages should only use exact CVS Ids
  (the unexpanded variant) has been disabled again. It occurred about
  16000 times but even fixing it wouldn't improve anything since it
  was mostly a formatting issue without any practical consequences.

* Warn about trailing variable modifiers like in ${VAR:S,from,to,extra}.

* Properly parse ${VAR:!command!}.

* Suggest to replace SUBST_SED with SUBST_VARS where possible, even
  with complicated shell quoting. Pkglint can autofix most of these
  overly verbose cases.

* Load builtin.mk whenever the corresponding buildlink3.mk file is
  included. This fixes several warnings about undefined variables
  (especially for packages using OpenSSL).

* Parse .for lines like bmake does since 2015, splitting words like
  in brk_string.

* Optionally show a warning even if it cannot be autofixed by pkglint.
  This is useful for the SUBST_VARS replacement since even when
  pkglint cannot automatically replace the code, there are still cases
  where it can warn at least.

* As always, several refactorings.

diffstat:

 pkgtools/pkglint/Makefile                          |    5 +-
 pkgtools/pkglint/PLIST                             |   10 +-
 pkgtools/pkglint/files/autofix.go                  |    9 +-
 pkgtools/pkglint/files/autofix_test.go             |   46 +++++-
 pkgtools/pkglint/files/buildlink3.go               |  103 ++++++-----
 pkgtools/pkglint/files/buildlink3_test.go          |   96 +++++++++-
 pkgtools/pkglint/files/category.go                 |   30 +-
 pkgtools/pkglint/files/check_test.go               |   34 ++-
 pkgtools/pkglint/files/cmd/pkglint/main.go         |   12 +
 pkgtools/pkglint/files/cmd/pkglint/main_test.go    |   48 +++++
 pkgtools/pkglint/files/cmd/pkglint/pkglint.go      |   12 -
 pkgtools/pkglint/files/cmd/pkglint/pkglint_test.go |   48 -----
 pkgtools/pkglint/files/distinfo.go                 |   34 +++-
 pkgtools/pkglint/files/distinfo_test.go            |   25 ++-
 pkgtools/pkglint/files/expecter.go                 |  144 ----------------
 pkgtools/pkglint/files/expecter_test.go            |   36 ----
 pkgtools/pkglint/files/files_test.go               |    4 +-
 pkgtools/pkglint/files/licenses.go                 |    2 +-
 pkgtools/pkglint/files/licenses_test.go            |    8 +-
 pkgtools/pkglint/files/line.go                     |   77 +++++---
 pkgtools/pkglint/files/linechecker.go              |   90 ----------
 pkgtools/pkglint/files/linechecker_test.go         |   76 --------
 pkgtools/pkglint/files/linelexer.go                |  139 ++++++++++++++++
 pkgtools/pkglint/files/linelexer_test.go           |   36 ++++
 pkgtools/pkglint/files/lines.go                    |   10 +-
 pkgtools/pkglint/files/lines_test.go               |    3 +-
 pkgtools/pkglint/files/logging_test.go             |    2 +-
 pkgtools/pkglint/files/mkline.go                   |  107 +++++++----
 pkgtools/pkglint/files/mkline_test.go              |   43 ++++-
 pkgtools/pkglint/files/mklinechecker.go            |   61 +++++--
 pkgtools/pkglint/files/mklinechecker_test.go       |   94 ++++++++++-
 pkgtools/pkglint/files/mklines.go                  |   11 +-
 pkgtools/pkglint/files/mklines_test.go             |   24 ++-
 pkgtools/pkglint/files/mkparser.go                 |   98 ++++++-----
 pkgtools/pkglint/files/mkparser_test.go            |  113 ++++++++++++-
 pkgtools/pkglint/files/mkshparser.go               |    6 +-
 pkgtools/pkglint/files/mkshparser_test.go          |   68 +++++++-
 pkgtools/pkglint/files/mktypes_test.go             |    3 -
 pkgtools/pkglint/files/options.go                  |   26 +-
 pkgtools/pkglint/files/options_test.go             |   63 +++++++
 pkgtools/pkglint/files/package.go                  |   25 +-
 pkgtools/pkglint/files/package_test.go             |   36 +++-
 pkgtools/pkglint/files/patches.go                  |  178 +++++---------------
 pkgtools/pkglint/files/patches_test.go             |   87 ----------
 pkgtools/pkglint/files/pkglint.go                  |   58 +++++-
 pkgtools/pkglint/files/pkglint_test.go             |   13 +-
 pkgtools/pkglint/files/pkgsrc.go                   |  140 ++++++++++++----
 pkgtools/pkglint/files/pkgsrc_test.go              |  117 ++++++++++---
 pkgtools/pkglint/files/plist.go                    |   30 ++-
 pkgtools/pkglint/files/plist_test.go               |    7 +-
 pkgtools/pkglint/files/shell.go                    |    5 +-
 pkgtools/pkglint/files/shell_test.go               |   19 +-
 pkgtools/pkglint/files/shtokenizer_test.go         |    8 +-
 pkgtools/pkglint/files/shtypes.go                  |   12 +-
 pkgtools/pkglint/files/substcontext.go             |   23 ++-
 pkgtools/pkglint/files/substcontext_test.go        |   57 +++++-
 pkgtools/pkglint/files/util.go                     |  158 ++++++++++++++++--
 pkgtools/pkglint/files/util_test.go                |   99 ++++++++++-
 pkgtools/pkglint/files/vardefs.go                  |   32 ++-
 pkgtools/pkglint/files/vartype.go                  |   52 +++--
 pkgtools/pkglint/files/vartype_test.go             |    2 +-
 pkgtools/pkglint/files/vartypecheck.go             |    5 +-
 pkgtools/pkglint/files/vartypecheck_test.go        |    6 +-
 63 files changed, 1873 insertions(+), 1152 deletions(-)

diffs (truncated from 5043 to 300 lines):

diff -r 10dc5275f322 -r e5e9d9d57178 pkgtools/pkglint/Makefile
--- a/pkgtools/pkglint/Makefile Sat Jan 26 11:52:19 2019 +0000
+++ b/pkgtools/pkglint/Makefile Sat Jan 26 16:31:33 2019 +0000
@@ -1,7 +1,6 @@
-# $NetBSD: Makefile,v 1.565 2019/01/24 10:00:42 bsiegert Exp $
+# $NetBSD: Makefile,v 1.566 2019/01/26 16:31:33 rillig Exp $
 
-PKGNAME=       pkglint-5.6.11
-PKGREVISION=   1
+PKGNAME=       pkglint-5.6.12
 CATEGORIES=    pkgtools
 DISTNAME=      tools
 MASTER_SITES=  ${MASTER_SITE_GITHUB:=golang/}
diff -r 10dc5275f322 -r e5e9d9d57178 pkgtools/pkglint/PLIST
--- a/pkgtools/pkglint/PLIST    Sat Jan 26 11:52:19 2019 +0000
+++ b/pkgtools/pkglint/PLIST    Sat Jan 26 16:31:33 2019 +0000
@@ -1,4 +1,4 @@
-@comment $NetBSD: PLIST,v 1.9 2019/01/13 19:55:52 rillig Exp $
+@comment $NetBSD: PLIST,v 1.10 2019/01/26 16:31:33 rillig Exp $
 bin/pkglint
 gopkg/pkg/${GO_PLATFORM}/netbsd.org/pkglint.a
 gopkg/pkg/${GO_PLATFORM}/netbsd.org/pkglint/getopt.a
@@ -18,12 +18,10 @@
 gopkg/src/netbsd.org/pkglint/category.go
 gopkg/src/netbsd.org/pkglint/category_test.go
 gopkg/src/netbsd.org/pkglint/check_test.go
-gopkg/src/netbsd.org/pkglint/cmd/pkglint/pkglint.go
-gopkg/src/netbsd.org/pkglint/cmd/pkglint/pkglint_test.go
+gopkg/src/netbsd.org/pkglint/cmd/pkglint/main.go
+gopkg/src/netbsd.org/pkglint/cmd/pkglint/main_test.go
 gopkg/src/netbsd.org/pkglint/distinfo.go
 gopkg/src/netbsd.org/pkglint/distinfo_test.go
-gopkg/src/netbsd.org/pkglint/expecter.go
-gopkg/src/netbsd.org/pkglint/expecter_test.go
 gopkg/src/netbsd.org/pkglint/files.go
 gopkg/src/netbsd.org/pkglint/files_test.go
 gopkg/src/netbsd.org/pkglint/fuzzer_test.go
@@ -44,6 +42,8 @@
 gopkg/src/netbsd.org/pkglint/line_test.go
 gopkg/src/netbsd.org/pkglint/linechecker.go
 gopkg/src/netbsd.org/pkglint/linechecker_test.go
+gopkg/src/netbsd.org/pkglint/linelexer.go
+gopkg/src/netbsd.org/pkglint/linelexer_test.go
 gopkg/src/netbsd.org/pkglint/lines.go
 gopkg/src/netbsd.org/pkglint/lines_test.go
 gopkg/src/netbsd.org/pkglint/logging.go
diff -r 10dc5275f322 -r e5e9d9d57178 pkgtools/pkglint/files/autofix.go
--- a/pkgtools/pkglint/files/autofix.go Sat Jan 26 11:52:19 2019 +0000
+++ b/pkgtools/pkglint/files/autofix.go Sat Jan 26 16:31:33 2019 +0000
@@ -30,6 +30,7 @@
        diagFormat  string          // Is logged only if it couldn't be fixed automatically
        diagArgs    []interface{}   //
        explanation []string        // Is printed together with the diagnostic
+       anyway      bool            // Print the diagnostic even if it cannot be autofixed
 }
 
 type autofixAction struct {
@@ -229,6 +230,12 @@
        }
 }
 
+// Anyway has the effect of showing the diagnostic even when nothing can
+// be fixed automatically.
+func (fix *Autofix) Anyway() {
+       fix.anyway = true
+}
+
 // Apply does the actual work.
 // Depending on the pkglint mode, it either:
 //
@@ -255,7 +262,7 @@
                fix.autofixShortTerm = autofixShortTerm{}
        }
 
-       if !G.Logger.Relevant(fix.diagFormat) || len(fix.actions) == 0 {
+       if !G.Logger.Relevant(fix.diagFormat) || !fix.anyway && len(fix.actions) == 0 {
                reset()
                return
        }
diff -r 10dc5275f322 -r e5e9d9d57178 pkgtools/pkglint/files/autofix_test.go
--- a/pkgtools/pkglint/files/autofix_test.go    Sat Jan 26 11:52:19 2019 +0000
+++ b/pkgtools/pkglint/files/autofix_test.go    Sat Jan 26 16:31:33 2019 +0000
@@ -547,9 +547,53 @@
                "+\tbelow")
 }
 
+// Demonstrates that without the --show-autofix option, diagnostics are
+// shown even when they cannot be autofixed.
+//
+// This is typical when an autofix is provided for simple scenarios,
+// but the code actually found is a little more complicated.
+func (s *Suite) Test_Autofix__show_unfixable_diagnostics_in_default_mode(c *check.C) {
+       t := s.Init(c)
+
+       t.SetUpCommandLine("--source")
+       lines := t.NewLines("Makefile",
+               "line1",
+               "line2",
+               "line3")
+
+       lines.Lines[0].Warnf("This warning is shown since the --show-autofix option is not given.")
+
+       fix := lines.Lines[1].Autofix()
+       fix.Warnf("This warning cannot be fixed and is therefore not shown.")
+       fix.Replace("XXX", "TODO")
+       fix.Apply()
+
+       fix.Warnf("This warning cannot be fixed automatically but should be shown anyway.")
+       fix.Replace("XXX", "TODO")
+       fix.Anyway()
+       fix.Apply()
+
+       // If this warning should ever appear it is probably because fix.anyway is not reset properly.
+       fix.Warnf("This warning cannot be fixed and is therefore not shown.")
+       fix.Replace("XXX", "TODO")
+       fix.Apply()
+
+       lines.Lines[2].Warnf("This warning is also shown.")
+
+       t.CheckOutputLines(
+               ">\tline1",
+               "WARN: Makefile:1: This warning is shown since the --show-autofix option is not given.",
+               "",
+               ">\tline2",
+               "WARN: Makefile:2: This warning cannot be fixed automatically but should be shown anyway.",
+               "",
+               ">\tline3",
+               "WARN: Makefile:3: This warning is also shown.")
+}
+
 // Demonstrates that the --show-autofix option only shows those diagnostics
 // that would be fixed.
-func (s *Suite) Test_Autofix__suppress_unfixable_warnings(c *check.C) {
+func (s *Suite) Test_Autofix__suppress_unfixable_warnings_with_show_autofix(c *check.C) {
        t := s.Init(c)
 
        t.SetUpCommandLine("--show-autofix", "--source")
diff -r 10dc5275f322 -r e5e9d9d57178 pkgtools/pkglint/files/buildlink3.go
--- a/pkgtools/pkglint/files/buildlink3.go      Sat Jan 26 11:52:19 2019 +0000
+++ b/pkgtools/pkglint/files/buildlink3.go      Sat Jan 26 16:31:33 2019 +0000
@@ -25,66 +25,66 @@
 
        mklines.Check()
 
-       exp := NewMkExpecter(mklines)
+       llex := NewMkLinesLexer(mklines)
 
-       for exp.AdvanceIf(MkLine.IsComment) {
-               line := exp.PreviousLine()
+       for llex.SkipIf(MkLine.IsComment) {
+               line := llex.PreviousLine()
                // See pkgtools/createbuildlink/files/createbuildlink
                if hasPrefix(line.Text, "# XXX This file was created automatically") {
                        line.Errorf("This comment indicates unfinished work (url2pkg).")
                }
        }
 
-       exp.ExpectEmptyLine()
+       llex.SkipEmptyOrNote()
 
-       if exp.AdvanceIfMatches(`^BUILDLINK_DEPMETHOD\.([^\t ]+)\?=.*$`) {
-               exp.PreviousLine().Warnf("This line belongs inside the .ifdef block.")
-               for exp.AdvanceIfEquals("") {
+       if llex.SkipRegexp(`^BUILDLINK_DEPMETHOD\.([^\t ]+)\?=.*$`) {
+               llex.PreviousLine().Warnf("This line belongs inside the .ifdef block.")
+               for llex.SkipString("") {
                }
        }
 
-       if !ck.checkFirstParagraph(exp) {
+       if !ck.checkFirstParagraph(llex) {
                return
        }
-       if !ck.checkSecondParagraph(exp) {
+       if !ck.checkSecondParagraph(llex) {
                return
        }
-       if !ck.checkMainPart(exp) {
+       if !ck.checkMainPart(llex) {
                return
        }
 
        // Fourth paragraph: Cleanup, corresponding to the first paragraph.
-       if !exp.ExpectText("BUILDLINK_TREE+=\t-" + ck.pkgbase) {
+       if !llex.SkipContainsOrWarn("BUILDLINK_TREE+=\t-" + ck.pkgbase) {
                return
        }
 
-       if !exp.EOF() {
-               exp.CurrentLine().Warnf("The file should end here.")
+       if !llex.EOF() {
+               llex.CurrentLine().Warnf("The file should end here.")
        }
 
        if G.Pkg != nil {
-               // TODO: Commenting this line doesn't make any test fail, but it should.
                G.Pkg.checkLinesBuildlink3Inclusion(mklines)
        }
 
        mklines.SaveAutofixChanges()
 }
 
-func (ck *Buildlink3Checker) checkFirstParagraph(exp *MkExpecter) bool {
+func (ck *Buildlink3Checker) checkFirstParagraph(mlex *MkLinesLexer) bool {
 
        // First paragraph: Introduction of the package identifier
-       if !exp.AdvanceIfMatches(`^BUILDLINK_TREE\+=[\t ]*([^\t ]+)$`) {
-               exp.CurrentLine().Warnf("Expected a BUILDLINK_TREE line.")
+       m := mlex.NextRegexp(`^BUILDLINK_TREE\+=[\t ]*([^\t ]+)$`)
+       if m == nil {
+               mlex.CurrentLine().Warnf("Expected a BUILDLINK_TREE line.")
                return false
        }
 
-       pkgbase := exp.Group(1)
-       pkgbaseLine := exp.PreviousMkLine()
+       pkgbase := m[1]
+       pkgbaseLine := mlex.PreviousMkLine()
 
        if containsVarRef(pkgbase) {
                ck.checkVaruseInPkgbase(pkgbase, pkgbaseLine)
        }
-       exp.ExpectEmptyLine()
+       mlex.SkipEmptyOrNote()
        ck.pkgbase = pkgbase
        ck.pkgbaseLine = pkgbaseLine
        return true
@@ -92,19 +92,20 @@
 
 // checkSecondParagraph checks the multiple inclusion protection and
 // introduces the uppercase package identifier.
-func (ck *Buildlink3Checker) checkSecondParagraph(exp *MkExpecter) bool {
+func (ck *Buildlink3Checker) checkSecondParagraph(mlex *MkLinesLexer) bool {
        pkgbase := ck.pkgbase
        pkgbaseLine := ck.pkgbaseLine
 
-       if !exp.AdvanceIfMatches(`^\.if !defined\(([^\t ]+)_BUILDLINK3_MK\)$`) {
+       m := mlex.NextRegexp(`^\.if !defined\(([^\t ]+)_BUILDLINK3_MK\)$`)
+       if m == nil {
                return false
        }
-       pkgupperLine, pkgupper := exp.PreviousMkLine(), exp.Group(1)
+       pkgupperLine, pkgupper := mlex.PreviousMkLine(), m[1]
 
-       if !exp.ExpectText(pkgupper + "_BUILDLINK3_MK:=") {
+       if !mlex.SkipContainsOrWarn(pkgupper + "_BUILDLINK3_MK:=") {
                return false
        }
-       exp.ExpectEmptyLine()
+       mlex.SkipEmptyOrNote()
 
        // See pkgtools/createbuildlink/files/createbuildlink, keyword PKGUPPER
        ucPkgbase := strings.ToUpper(strings.Replace(pkgbase, "-", "_", -1))
@@ -123,19 +124,19 @@
 }
 
 // Third paragraph: Package information.
-func (ck *Buildlink3Checker) checkMainPart(exp *MkExpecter) bool {
+func (ck *Buildlink3Checker) checkMainPart(mlex *MkLinesLexer) bool {
        pkgbase := ck.pkgbase
 
        // The first .if is from the second paragraph.
        indentLevel := 1
 
-       for !exp.EOF() && indentLevel > 0 {
-               mkline := exp.CurrentMkLine()
-               exp.Advance()
+       for !mlex.EOF() && indentLevel > 0 {
+               mkline := mlex.CurrentMkLine()
+               mlex.Skip()
 
                switch {
                case mkline.IsVarassign():
-                       ck.checkVarassign(exp, mkline, pkgbase)
+                       ck.checkVarassign(mlex, mkline, pkgbase)
 
                case mkline.IsDirective() && mkline.Directive() == "if":
                        indentLevel++
@@ -150,13 +151,13 @@
        }
 
        if ck.apiLine == nil {
-               exp.CurrentLine().Warnf("Definition of BUILDLINK_API_DEPENDS is missing.")
+               mlex.CurrentLine().Warnf("Definition of BUILDLINK_API_DEPENDS is missing.")
        }
-       exp.ExpectEmptyLine()
+       mlex.SkipEmptyOrNote()
        return true
 }
 
-func (ck *Buildlink3Checker) checkVarassign(exp *MkExpecter, mkline MkLine, pkgbase string) {
+func (ck *Buildlink3Checker) checkVarassign(mlex *MkLinesLexer, mkline MkLine, pkgbase string) {
        varname, value := mkline.Varname(), mkline.Value()
        doCheck := false
 
@@ -204,28 +205,30 @@
 }
 
 func (ck *Buildlink3Checker) checkVaruseInPkgbase(pkgbase string, pkgbaseLine MkLine) {
-       checkSpecificVar := func(varuse, simple string) bool {



Home | Main Index | Thread Index | Old Index