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



details:   https://anonhg.NetBSD.org/pkgsrc/rev/14b5c82509c1
branches:  trunk
changeset: 343470:14b5c82509c1
user:      rillig <rillig%pkgsrc.org@localhost>
date:      Mon Nov 04 18:44:21 2019 +0000

description:
pkgtools/pkglint: update to 19.3.6

Changes since 19.3.5:

Improved indentation and alignment of multi-line variable assignments.

Improved indentation of multi-line shell commands.

Added warning for adding unquoted words to PKG_FAIL_REASON, which is a
list of messages, one per line.

Lines that start with tabs followed by a # are not shell commands, they
are comments. Bmake treats them in the same way.

Change the type of BROKEN to be a list of messages, instead of a single
message. This allows at least a bit of formatting in the error messages.

diffstat:

 pkgtools/pkglint/Makefile                    |    4 +-
 pkgtools/pkglint/files/mklinechecker.go      |   17 ++-
 pkgtools/pkglint/files/mklinechecker_test.go |   52 ++++++++-
 pkgtools/pkglint/files/mkshparser.go         |    8 +
 pkgtools/pkglint/files/options_test.go       |  159 ++++++++++++++------------
 pkgtools/pkglint/files/shell.go              |   49 +++-----
 pkgtools/pkglint/files/shell_test.go         |   17 +--
 pkgtools/pkglint/files/varalignblock.go      |   27 +++-
 pkgtools/pkglint/files/varalignblock_test.go |   58 +++++++++
 pkgtools/pkglint/files/vardefs.go            |   16 ++-
 pkgtools/pkglint/files/vartype.go            |    3 +
 pkgtools/pkglint/files/vartypecheck_test.go  |   17 ++
 12 files changed, 295 insertions(+), 132 deletions(-)

diffs (truncated from 694 to 300 lines):

diff -r b204bd9c277a -r 14b5c82509c1 pkgtools/pkglint/Makefile
--- a/pkgtools/pkglint/Makefile Mon Nov 04 18:35:21 2019 +0000
+++ b/pkgtools/pkglint/Makefile Mon Nov 04 18:44:21 2019 +0000
@@ -1,6 +1,6 @@
-# $NetBSD: Makefile,v 1.605 2019/11/02 16:37:48 rillig Exp $
+# $NetBSD: Makefile,v 1.606 2019/11/04 18:44:21 rillig Exp $
 
-PKGNAME=       pkglint-19.3.5
+PKGNAME=       pkglint-19.3.6
 CATEGORIES=    pkgtools
 DISTNAME=      tools
 MASTER_SITES=  ${MASTER_SITE_GITHUB:=golang/}
diff -r b204bd9c277a -r 14b5c82509c1 pkgtools/pkglint/files/mklinechecker.go
--- a/pkgtools/pkglint/files/mklinechecker.go   Mon Nov 04 18:35:21 2019 +0000
+++ b/pkgtools/pkglint/files/mklinechecker.go   Mon Nov 04 18:44:21 2019 +0000
@@ -66,6 +66,9 @@
 
        shellCommand := mkline.ShellCommand()
        if G.Opts.WarnSpace && hasPrefix(mkline.Text, "\t\t") {
+               lexer := textproc.NewLexer(mkline.raw[0].textnl)
+               tabs := lexer.NextBytesFunc(func(b byte) bool { return b == '\t' })
+
                fix := mkline.Autofix()
                fix.Notef("Shell programs should be indented with a single tab.")
                fix.Explain(
@@ -73,7 +76,12 @@
                        "Since every line of shell commands starts with a completely new shell environment,",
                        "there is no need to indent some of the commands,",
                        "or to use more horizontal space than necessary.")
-               fix.ReplaceRegex(`^\t\t+`, "\t", 1)
+
+               for i, raw := range mkline.Line.raw {
+                       if hasPrefix(raw.textnl, tabs) {
+                               fix.ReplaceAt(i, 0, tabs, "\t")
+                       }
+               }
                fix.Apply()
        }
 
@@ -1422,6 +1430,13 @@
 
        default:
                words := mkline.ValueFields(value)
+               if len(words) > 1 && vartype.OnePerLine() {
+                       mkline.Warnf("%s should only get one item per line.", varname)
+                       mkline.Explain(
+                               "Use the += operator to append each of the items.",
+                               "",
+                               "Or, enclose the words in quotes to group them.")
+               }
                for _, word := range words {
                        ck.CheckVartypeBasic(varname, vartype.basicType, op, word, comment, vartype.Guessed())
                }
diff -r b204bd9c277a -r 14b5c82509c1 pkgtools/pkglint/files/mklinechecker_test.go
--- a/pkgtools/pkglint/files/mklinechecker_test.go      Mon Nov 04 18:35:21 2019 +0000
+++ b/pkgtools/pkglint/files/mklinechecker_test.go      Mon Nov 04 18:44:21 2019 +0000
@@ -25,6 +25,38 @@
                "WARN: ~/filename.mk:3: This line looks empty but continues the previous line.")
 }
 
+func (s *Suite) Test_MkLineChecker_checkShellCommand__indentation(c *check.C) {
+       t := s.Init(c)
+
+       t.SetUpCommandLine("-Wall", "--autofix")
+       mklines := t.SetUpFileMkLines("filename.mk",
+               MkCvsID,
+               "",
+               "do-install:",
+               "\t\techo 'unnecessarily indented'",
+               "\t\tfor var in 1 2 3; do \\",
+               "\t\t\techo \"$$var\"; \\",
+               "\t                echo \"spaces\"; \\",
+               "\t\tdone")
+
+       mklines.Check()
+
+       t.CheckOutputLines(
+               "AUTOFIX: ~/filename.mk:4: Replacing \"\\t\\t\" with \"\\t\".",
+               "AUTOFIX: ~/filename.mk:5: Replacing \"\\t\\t\" with \"\\t\".",
+               "AUTOFIX: ~/filename.mk:6: Replacing \"\\t\\t\" with \"\\t\".",
+               "AUTOFIX: ~/filename.mk:8: Replacing \"\\t\\t\" with \"\\t\".")
+       t.CheckFileLinesDetab("filename.mk",
+               MkCvsID,
+               "",
+               "do-install:",
+               "        echo 'unnecessarily indented'",
+               "        for var in 1 2 3; do \\",
+               "                echo \"$$var\"; \\",
+               "                        echo \"spaces\"; \\", // not changed
+               "        done")
+}
+
 func (s *Suite) Test_MkLineChecker_checkVarassignLeft(c *check.C) {
        t := s.Init(c)
 
@@ -724,6 +756,23 @@
                "WARN: filename.mk:3: CUR_DIR is defined but not used.")
 }
 
+func (s *Suite) Test_MkLineChecker_checkVartype__one_per_line(c *check.C) {
+       t := s.Init(c)
+
+       t.SetUpVartypes()
+       mklines := t.NewMkLines("filename.mk",
+               MkCvsID,
+               "PKG_FAIL_REASON+=\tSeveral words are wrong.",
+               "PKG_FAIL_REASON+=\t\"Properly quoted\"",
+               "PKG_FAIL_REASON+=\t# none")
+       t.DisableTracing()
+
+       mklines.Check()
+
+       t.CheckOutputLines(
+               "WARN: filename.mk:2: PKG_FAIL_REASON should only get one item per line.")
+}
+
 // Pkglint once interpreted all lists as consisting of shell tokens,
 // splitting this URL at the ampersand.
 func (s *Suite) Test_MkLineChecker_checkVarassign__URL_with_shell_special_characters(c *check.C) {
@@ -2535,7 +2584,8 @@
        mklines.Check()
 
        t.CheckOutputLines(
-               "WARN: ~/options.mk:2: The user-defined variable VARBASE is used but not added to BUILD_DEFS.")
+               "WARN: ~/options.mk:2: The user-defined variable VARBASE is used but not added to BUILD_DEFS.",
+               "WARN: ~/options.mk:3: PKG_FAIL_REASON should only get one item per line.")
 }
 
 // The LOCALBASE variable may be defined and used in the infrastructure.
diff -r b204bd9c277a -r 14b5c82509c1 pkgtools/pkglint/files/mkshparser.go
--- a/pkgtools/pkglint/files/mkshparser.go      Mon Nov 04 18:35:21 2019 +0000
+++ b/pkgtools/pkglint/files/mkshparser.go      Mon Nov 04 18:44:21 2019 +0000
@@ -80,6 +80,10 @@
 
        if trace.Tracing {
                defer func() {
+                       if ttype == 0 {
+                               trace.Stepf("lex EOF because of a comment")
+                               return
+                       }
                        tname := shyyTokname(shyyTok2[ttype-shyyPrivate])
                        switch ttype {
                        case tkWORD, tkASSIGNMENT_WORD:
@@ -238,6 +242,10 @@
                ttype = tkASSIGNMENT_WORD
                p := NewShTokenizer(dummyLine, token, false) // Just for converting the string to a ShToken
                lval.Word = p.ShToken()
+       case hasPrefix(token, "#"):
+               // This doesn't work for multiline shell programs.
+               // Since pkglint only processes single lines, that's ok.
+               return 0
        default:
                ttype = tkWORD
                p := NewShTokenizer(dummyLine, token, false) // Just for converting the string to a ShToken
diff -r b204bd9c277a -r 14b5c82509c1 pkgtools/pkglint/files/options_test.go
--- a/pkgtools/pkglint/files/options_test.go    Mon Nov 04 18:35:21 2019 +0000
+++ b/pkgtools/pkglint/files/options_test.go    Mon Nov 04 18:44:21 2019 +0000
@@ -103,6 +103,92 @@
        t.CheckOutputEmpty()
 }
 
+func (s *Suite) Test_CheckLinesOptionsMk__variable_order(c *check.C) {
+       t := s.Init(c)
+
+       t.SetUpOption("option", "")
+       t.SetUpPackage("category/package",
+               ".include \"options.mk\"")
+       t.CreateFileLines("mk/bsd.options.mk",
+               MkCvsID)
+       mklines := t.SetUpFileMkLines("category/package/options.mk",
+               MkCvsID,
+               "",
+               "PKG_SUPPORTED_OPTIONS=\toption",
+               "PKG_OPTIONS_VAR=\tPKG_OPTIONS.package",
+               "",
+               ".include \"../../mk/bsd.options.mk\"",
+               "",
+               ".if ${PKG_OPTIONS:Moption}",
+               ".endif")
+       t.Chdir("category/package")
+       t.FinishSetUp()
+
+       CheckLinesOptionsMk(mklines)
+
+       t.CheckOutputLines(
+               "WARN: ~/category/package/options.mk:3: " +
+                       "Expected definition of PKG_OPTIONS_VAR.")
+}
+
+func (s *Suite) Test_CheckLinesOptionsMk__empty(c *check.C) {
+       t := s.Init(c)
+
+       t.SetUpPackage("category/package",
+               ".include \"options.mk\"")
+       mklines := t.SetUpFileMkLines("category/package/options.mk",
+               MkCvsID)
+       t.Chdir("category/package")
+       t.FinishSetUp()
+
+       CheckLinesOptionsMk(mklines)
+
+       t.CheckOutputLines(
+               "ERROR: ~/category/package/options.mk: "+
+                       "Each options.mk file must define PKG_OPTIONS_VAR.",
+               "ERROR: ~/category/package/options.mk: "+
+                       "Each options.mk file must .include \"../../mk/bsd.options.mk\".")
+}
+
+func (s *Suite) Test_CheckLinesOptionsMk__conditionals(c *check.C) {
+       t := s.Init(c)
+
+       t.SetUpOption("option", "")
+       t.SetUpPackage("category/package",
+               ".include \"../../mk/bsd.options.mk\"")
+       t.CreateFileLines("mk/bsd.options.mk",
+               MkCvsID)
+       t.Chdir("category/package")
+       mklines := t.SetUpFileMkLines("options.mk",
+               MkCvsID,
+               "",
+               "PKG_OPTIONS_VAR=\tPKG_OPTIONS.package",
+               "PKG_SUPPORTED_OPTIONS=\toption",
+               "",
+               ".include \"../../mk/bsd.options.mk\"",
+               "",
+               ".if ${PKG_OPTIONS}", // typo: should be ${PKG_OPTIONS:Moption}
+               ".endif",
+               "",
+               ".if ${PKG_OPTIONS:Nnegative}", // :N instead of :M, is ignored
+               ".endif",
+               "",
+               ".if ${PKG_OPTIONS:Ncodec-*}",
+               ".endif",
+               "",
+               ".if ${PKG_OPTIONS:tl}", // doesn't make sense, just for branch coverage
+               ".endif")
+       t.FinishSetUp()
+
+       CheckLinesOptionsMk(mklines)
+
+       t.CheckOutputLines(
+               // This warning comes from VarTypeCheck.PkgOption
+               "WARN: options.mk:11: Unknown option \"negative\".",
+               "WARN: options.mk:4: "+
+                       "Option \"option\" should be handled below in an .if block.")
+}
+
 func (s *Suite) Test_CheckLinesOptionsMk(c *check.C) {
        t := s.Init(c)
 
@@ -170,79 +256,6 @@
        //  disables possible wrong warnings, but a few too many.
 }
 
-// This test is provided for code coverage. Similarities to existing files are purely coincidental.
-func (s *Suite) Test_CheckLinesOptionsMk__edge_cases(c *check.C) {
-       t := s.Init(c)
-
-       t.SetUpCommandLine("-Wall,no-space")
-       t.SetUpVartypes()
-       t.SetUpOption("option1", "Description for option1")
-       t.CreateFileLines("mk/compiler.mk",
-               MkCvsID)
-       t.CreateFileLines("mk/bsd.options.mk",
-               MkCvsID)
-       t.DisableTracing()
-
-       mklines := t.SetUpFileMkLines("category/package/options.mk",
-               MkCvsID)
-
-       CheckLinesOptionsMk(mklines)
-
-       t.CheckOutputLines(
-               "ERROR: ~/category/package/options.mk: Each options.mk file must define PKG_OPTIONS_VAR.",
-               "ERROR: ~/category/package/options.mk: Each options.mk file must .include \"../../mk/bsd.options.mk\".")
-
-       mklines = t.SetUpFileMkLines("category/package/options.mk",
-               MkCvsID,
-               "PKG_SUPPORTED_OPTIONS=\toption1")
-
-       CheckLinesOptionsMk(mklines)
-
-       t.CheckOutputLines(
-               "WARN: ~/category/package/options.mk:2: "+
-                       "Expected definition of PKG_OPTIONS_VAR.",
-               "ERROR: ~/category/package/options.mk: "+
-                       "Each options.mk file must define PKG_OPTIONS_VAR.",
-               "ERROR: ~/category/package/options.mk: "+
-                       "Each options.mk file must .include \"../../mk/bsd.options.mk\".",
-               "WARN: ~/category/package/options.mk:2: "+
-                       "Option \"option1\" should be handled below in an .if block.")
-
-       mklines = t.SetUpFileMkLines("category/package/options.mk",
-               MkCvsID,
-               "PKG_OPTIONS_VAR=\tPKG_OPTIONS.pkgbase",
-               "PKG_SUPPORTED_OPTIONS=\toption1",
-               ".include \"../../mk/compiler.mk\"")
-
-       CheckLinesOptionsMk(mklines)
-



Home | Main Index | Thread Index | Old Index