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.5.6



details:   https://anonhg.NetBSD.org/pkgsrc/rev/3360f38a37cf
branches:  trunk
changeset: 376602:3360f38a37cf
user:      rillig <rillig%pkgsrc.org@localhost>
date:      Sun Mar 04 20:34:32 2018 +0000

description:
pkgtools/pkglint: update to 5.5.6

Changes since 5.5.5:

* Only offer explanations if an explainable diagnostic has actually
  been logged.

* Clean up code.

* Improve a few diagnostics.

* In any Makefile, treat documented variables as used. This prevents
  warning about defined but unused variables.

* Add support for some variables not previously known to pkglint.

diffstat:

 pkgtools/pkglint/Makefile                    |    5 +-
 pkgtools/pkglint/files/autofix.go            |    3 +-
 pkgtools/pkglint/files/check_test.go         |    6 +-
 pkgtools/pkglint/files/codewalk.md           |  212 +++++++++++++++++++++++++++
 pkgtools/pkglint/files/globaldata.go         |    3 +
 pkgtools/pkglint/files/globalvars.go         |   88 -----------
 pkgtools/pkglint/files/line.go               |    3 +-
 pkgtools/pkglint/files/logging.go            |   40 ++--
 pkgtools/pkglint/files/logging_test.go       |   44 +++++
 pkgtools/pkglint/files/mkline.go             |   42 ++---
 pkgtools/pkglint/files/mklinechecker.go      |    8 +-
 pkgtools/pkglint/files/mklinechecker_test.go |   17 +-
 pkgtools/pkglint/files/mklines.go            |   92 ++++++++---
 pkgtools/pkglint/files/mklines_test.go       |   72 +++++++-
 pkgtools/pkglint/files/package.go            |   70 +++-----
 pkgtools/pkglint/files/package_test.go       |   25 +-
 pkgtools/pkglint/files/pkglint.go            |  209 +++++++++++++++++++-------
 pkgtools/pkglint/files/pkglint_test.go       |   36 ++--
 pkgtools/pkglint/files/plist.go              |   11 +-
 pkgtools/pkglint/files/plist_test.go         |    1 -
 pkgtools/pkglint/files/shell.go              |   11 +-
 pkgtools/pkglint/files/shell_test.go         |    4 +-
 pkgtools/pkglint/files/shtokenizer.go        |    6 +-
 pkgtools/pkglint/files/util.go               |  135 ++++++++++++++--
 pkgtools/pkglint/files/vardefs.go            |   11 +
 pkgtools/pkglint/files/vartypecheck.go       |   17 ++
 pkgtools/pkglint/files/vartypecheck_test.go  |   16 +-
 27 files changed, 840 insertions(+), 347 deletions(-)

diffs (truncated from 2175 to 300 lines):

diff -r 4bedb6a862d8 -r 3360f38a37cf pkgtools/pkglint/Makefile
--- a/pkgtools/pkglint/Makefile Sun Mar 04 18:26:42 2018 +0000
+++ b/pkgtools/pkglint/Makefile Sun Mar 04 20:34:32 2018 +0000
@@ -1,7 +1,6 @@
-# $NetBSD: Makefile,v 1.530 2018/03/04 15:52:18 bsiegert Exp $
+# $NetBSD: Makefile,v 1.531 2018/03/04 20:34:32 rillig Exp $
 
-PKGNAME=       pkglint-5.5.5
-PKGREVISION=   1
+PKGNAME=       pkglint-5.5.6
 DISTFILES=     # none
 CATEGORIES=    pkgtools
 
diff -r 4bedb6a862d8 -r 3360f38a37cf pkgtools/pkglint/files/autofix.go
--- a/pkgtools/pkglint/files/autofix.go Sun Mar 04 18:26:42 2018 +0000
+++ b/pkgtools/pkglint/files/autofix.go Sun Mar 04 20:34:32 2018 +0000
@@ -227,7 +227,8 @@
                return
        }
 
-       if shallBeLogged(fix.diagFormat) {
+       G.explainNext = shallBeLogged(fix.diagFormat)
+       if G.explainNext {
                logDiagnostic := fix.level != nil && fix.diagFormat != "Silent-Magic-Diagnostic" &&
                        !(G.opts.Autofix && !G.opts.PrintAutofix) && len(fix.actions) > 0
                if logDiagnostic {
diff -r 4bedb6a862d8 -r 3360f38a37cf pkgtools/pkglint/files/check_test.go
--- a/pkgtools/pkglint/files/check_test.go      Sun Mar 04 18:26:42 2018 +0000
+++ b/pkgtools/pkglint/files/check_test.go      Sun Mar 04 20:34:32 2018 +0000
@@ -45,7 +45,7 @@
        t := &Tester{checkC: c}
        s.Tester = t
 
-       G = GlobalVars{Testing: true}
+       G = Pkglint{Testing: true}
        textproc.Testing = true
        G.logOut = NewSeparatorWriter(&t.stdout)
        G.logErr = NewSeparatorWriter(&t.stderr)
@@ -62,7 +62,7 @@
        t := s.Tester
        t.checkC = nil // No longer usable; see https://github.com/go-check/check/issues/22
 
-       G = GlobalVars{}
+       G = Pkglint{} // unusable because of missing logOut and logErr
        textproc.Testing = false
        if out := t.Output(); out != "" {
                fmt.Fprintf(os.Stderr, "Unchecked output in %q; check with: t.CheckOutputLines(%v)",
@@ -96,7 +96,7 @@
 // SetupCommandLine simulates a command line for the remainder of the test.
 // See Pkglint.ParseCommandLine.
 func (t *Tester) SetupCommandLine(args ...string) {
-       exitcode := new(Pkglint).ParseCommandLine(append([]string{"pkglint"}, args...))
+       exitcode := G.ParseCommandLine(append([]string{"pkglint"}, args...))
        if exitcode != nil && *exitcode != 0 {
                t.CheckOutputEmpty()
                t.c().Fatalf("Cannot parse command line: %#v", args)
diff -r 4bedb6a862d8 -r 3360f38a37cf pkgtools/pkglint/files/codewalk.md
--- /dev/null   Thu Jan 01 00:00:00 1970 +0000
+++ b/pkgtools/pkglint/files/codewalk.md        Sun Mar 04 20:34:32 2018 +0000
@@ -0,0 +1,212 @@
+# The pkglint tour
+
+> Note: I wish there were a tool for nicely rendering the below codewalk blocks.
+> If you know of such a tool, please tell me.
+> `godoc` codewalks don't count since [they are only supported for the Go distribution 
+> itself](https://github.com/golang/go/issues/14369).
+>
+> See also:
+>
+> * [github/markup/#172](https://github.com/github/markup/issues/172#issuecomment-33241601) (closed)
+> * [MarkdownHelper](https://github.com/BurdetteLamar/MarkdownHelper)
+
+## The entry points
+
+### Running pkglint
+
+```codewalk
+file   pkglint.go
+start  ^func main
+end    ^\}
+```
+
+When running pkglint, the `G` variable is set up first.
+It contains the whole global state of pkglint.
+(Except for some of the subpackages, which have to be initialized separately.)
+All the interesting code is in the `Pkglint` type.
+This code structure makes it easy to test the code.
+
+### Testing pkglint
+
+Very similar code is used to set up the test and tear it down again:
+
+```codewalk
+file   check_test.go
+start  ^func .* SetUpTest
+end    ^\}
+```
+
+```codewalk
+file   check_test.go
+start  ^func .* TearDownTest
+end    ^\}
+```
+
+## First contact: checking a single DESCR file
+
+To learn how pkglint works internally, it is a good idea to start with
+a very simple example.
+Since the `DESCR` files have a very simple structure (they only contain
+text for human consumption), they are the ideal target.
+Let's trace an invocation of the command `pkglint DESCR` down to where
+the actual checks happen.
+
+```codewalk
+file   pkglint.go
+start  func main
+```
+
+```codewalk
+file   pkglint.go
+start  ^[\t]if exitcode
+end    ^\t\}$
+```
+
+Since there are no command line options starting with a hyphen, we can
+skip the command line parsing for this example.
+
+```codewalk
+file   pkglint.go
+start  ^[\t]for _, arg
+end    ^\}
+```
+
+The argument `DESCR` is saved in the `TODO` list, and then the pkgsrc
+infrastructure data is loaded by `Initialize`.
+This must happen in this order because pkglint needs to determine the
+pkgsrc root directory, just in case there are two or more pkgsrc trees
+in the local system.
+The path of the pkgsrc directory is determined from the first command
+line argument, which in this file is `DESCR`. From there, the pkgsrc
+root is usually reachable via `../../`, and this is what pkglint tries.
+
+After the initialization of everything,
+all items from the TODO list are worked off and handed over to `CheckDirent`.
+
+```codewalk
+file   pkglint.go
+start  func CheckDirent
+```
+
+Since `DESCR` is a regular file, the next method to call is `Checkfile`.
+
+```codewalk
+file   pkglint.go
+start  func Checkfile
+```
+
+```codewalk
+file   pkglint.go
+start  /basename, "DESCR"/
+end    ^$
+```
+
+When compared to the code blocks around this one, it looks strange that
+this one uses `hasPrefix` and the others use a direct string comparison.
+But indeed, there are a few packages that actually have `DESCR.common`
+files. So everything's fine here.
+
+At this point, the file is loaded and converted to lines.
+For DESCR files, this is very simple, so there's no need to dive into that.
+The actual checks usually work on `Line` objects instead of files
+because the lines offer nice methods for logging the diagnostics
+and for automatically fixing the text (in pkglint's `--autofix` mode).
+
+```codewalk
+file   pkglint.go
+start  func ChecklinesDescr
+end    ^\}
+```
+
+Now we are where the actual action takes place.
+The code looks straight-forward here.
+First, each line is checked on its own,
+and the final check is for too long files. 
+
+The call to `SaveAutofixChanges` at the end looks a bit strange
+since none of the visible checks fixes anything.
+The autofix feature must be hidden in one of the line checks,
+and indeed, the code for `CheckLineTrailingWhitespace` says:
+
+```codewalk
+file   linechecker.go
+start  ^func CheckLineTrailingWhitespace
+end    ^\}
+```
+
+This code is a typical example for using the autofix feature.
+Some more details are described at the `Autofix` type itself:
+
+```codewalk
+file   linechecker.go
+start  /^type Autofix/ upwhile /^\/\//
+end    /^type Autofix/
+```
+
+The journey ends here, and it hasn't been that difficult.
+If that was too easy, have a look at the complex cases here:
+
+```codewalk
+file   mkline.go
+start  /^func .* VariableNeedsQuoting
+```
+
+## Basic ingredients
+
+Pkglint checks packages, and a package consists of several different files.
+All pkgsrc files are text files, which are organized in lines.
+Most pkglint diagnostics refer to a specific line,
+therefore the `Line` type is responsible for producing the diagnostics.
+
+### Line
+
+Most checks in pkgsrc only need to look at a single line.
+Lines that are independent of the file type are implemented in the `Line` type.
+This type contains the methods `Errorf`, `Warnf` and `Notef` to produce diagnostics
+of the following form:
+
+```text
+WARN: Makefile:3: COMMENT should not start with "A" or "An".
+```
+
+The definition for the `Line` type is:
+
+```codewalk
+file   line.go
+start  ^type Line =
+```
+
+```codewalk
+file   line.go
+start  ^type LineImpl struct
+end    ^\}
+```
+
+### MkLine
+
+```codewalk
+file   mkline.go
+start  ^type MkLine =
+```
+
+```codewalk
+file   mkline.go
+start  ^type MkLineImpl struct
+end    ^\}
+```
+
+Most of the pkgsrc infrastructure is written in Makefiles. 
+In these, there may be line continuations  (the ones ending in backslash).
+Plus, they may contain Make variables of the form `${VARNAME}` or `${VARNAME:Modifiers}`,
+and these are handled specially.
+
+### ShellLine
+
+The instructions for building and installing packages are written in Shell.
+The `ShellLine` type provides methods for checking shell commands and their individual parts.
+
+```codewalk
+file   shell.go
+start  ^type ShellLine struct
+end    ^\}
+```
diff -r 4bedb6a862d8 -r 3360f38a37cf pkgtools/pkglint/files/globaldata.go
--- a/pkgtools/pkglint/files/globaldata.go      Sun Mar 04 18:26:42 2018 +0000
+++ b/pkgtools/pkglint/files/globaldata.go      Sun Mar 04 20:34:32 2018 +0000
@@ -44,6 +44,9 @@
        Comment string
 }
 
+// Initialize reads the pkgsrc infrastructure files to
+// extract information like the tools, packages to update,
+// user-defined variables.
 func (gd *GlobalData) Initialize() {
        firstArg := G.Todo[0]
        if fileExists(firstArg) {
diff -r 4bedb6a862d8 -r 3360f38a37cf pkgtools/pkglint/files/globalvars.go
--- a/pkgtools/pkglint/files/globalvars.go      Sun Mar 04 18:26:42 2018 +0000
+++ /dev/null   Thu Jan 01 00:00:00 1970 +0000
@@ -1,88 +0,0 @@
-package main
-
-import (
-       "netbsd.org/pkglint/histogram"
-)
-
-type GlobalVars struct {
-       opts       CmdOpts    //
-       globalData GlobalData //
-       Pkg        *Package   // The package that is currently checked.
-       Mk         *MkLines   // The Makefile (or fragment) that is currently checked.



Home | Main Index | Thread Index | Old Index