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.16
details: https://anonhg.NetBSD.org/pkgsrc/rev/d814cfcaf86f
branches: trunk
changeset: 345321:d814cfcaf86f
user: rillig <rillig%pkgsrc.org@localhost>
date: Mon Dec 09 20:38:15 2019 +0000
description:
pkgtools/pkglint: update to 19.3.16
Changes since 19.3.15:
When a package-settable variable gets a default value using the ?=
operator, pkglint no longer suggests to include bsd.prefs.mk, since that
doesn't make sense. Including bsd.prefs.mk only defines user-settable
and system-provided variables.
User and group names may be a single character only. While not widely
used, it's syntactically valid and there's no reason to prevent this.
In variable assignments, when pkglint removes unnecessary whitespace
between the variable name and the operator, it keeps the indentation of
the variable value the same as before. Previously, the indentation had
been changed, which required another run of pkglint --autofix.
PREFIX can only be used as a replacement for LOCALBASE after the whole
package Makefile has been loaded. This is because PREFIX is defined
very late, by bsd.pkg.mk. Therefore, don't suggest to replace LOCALBASE
with PREFIX in .if conditions.
When pkglint suggests to replace INSTALL_DATA_DIR commands with setting
INSTALLATION_DIRS instead, paths with a trailing slash are correctly
looked up in the PLIST. This suggests to use AUTO_MKDIRS more often.
diffstat:
pkgtools/pkglint/Makefile | 4 +-
pkgtools/pkglint/files/logging.go | 8 +-
pkgtools/pkglint/files/logging_test.go | 17 +
pkgtools/pkglint/files/mkline.go | 65 ++++--
pkgtools/pkglint/files/mkline_test.go | 29 ++-
pkgtools/pkglint/files/mklineparser.go | 68 ++++---
pkgtools/pkglint/files/mklineparser_test.go | 134 +++++++++------
pkgtools/pkglint/files/mklines_test.go | 22 ++
pkgtools/pkglint/files/mkvarusechecker.go | 16 +-
pkgtools/pkglint/files/mkvarusechecker_test.go | 92 +++++++---
pkgtools/pkglint/files/package_test.go | 29 ++-
pkgtools/pkglint/files/path_test.go | 6 +-
pkgtools/pkglint/files/pkglint.1 | 2 +-
pkgtools/pkglint/files/pkglint_test.go | 18 +-
pkgtools/pkglint/files/plist.go | 3 +-
pkgtools/pkglint/files/shell.go | 201 ++++++++++++----------
pkgtools/pkglint/files/shell_test.go | 216 +++++++++++++++++-------
pkgtools/pkglint/files/util.go | 34 +++-
pkgtools/pkglint/files/util_test.go | 12 +-
pkgtools/pkglint/files/vartypecheck.go | 22 ++-
pkgtools/pkglint/files/vartypecheck_test.go | 39 +++-
21 files changed, 685 insertions(+), 352 deletions(-)
diffs (truncated from 1481 to 300 lines):
diff -r 8f681717ba3b -r d814cfcaf86f pkgtools/pkglint/Makefile
--- a/pkgtools/pkglint/Makefile Mon Dec 09 20:15:57 2019 +0000
+++ b/pkgtools/pkglint/Makefile Mon Dec 09 20:38:15 2019 +0000
@@ -1,6 +1,6 @@
-# $NetBSD: Makefile,v 1.615 2019/12/08 22:03:37 rillig Exp $
+# $NetBSD: Makefile,v 1.616 2019/12/09 20:38:15 rillig Exp $
-PKGNAME= pkglint-19.3.15
+PKGNAME= pkglint-19.3.16
CATEGORIES= pkgtools
DISTNAME= tools
MASTER_SITES= ${MASTER_SITE_GITHUB:=golang/}
diff -r 8f681717ba3b -r d814cfcaf86f pkgtools/pkglint/files/logging.go
--- a/pkgtools/pkglint/files/logging.go Mon Dec 09 20:15:57 2019 +0000
+++ b/pkgtools/pkglint/files/logging.go Mon Dec 09 20:38:15 2019 +0000
@@ -257,10 +257,14 @@
if G.Testing {
if level != Error {
- assertf(!contains(format, "must"), "Must must only appear in errors: %s", format)
+ assertf(
+ !contains(format, "must"),
+ "The word \"must\" must only appear in errors: %s", format)
}
if level != Warn && level != Note {
- assertf(!contains(format, "should"), "Should must only appear in warnings: %s", format)
+ assertf(
+ !contains(format, "should"),
+ "The word \"should\" must only appear in warnings: %s", format)
}
}
diff -r 8f681717ba3b -r d814cfcaf86f pkgtools/pkglint/files/logging_test.go
--- a/pkgtools/pkglint/files/logging_test.go Mon Dec 09 20:15:57 2019 +0000
+++ b/pkgtools/pkglint/files/logging_test.go Mon Dec 09 20:38:15 2019 +0000
@@ -805,6 +805,23 @@
"Pkglint internal error: Diagnostic format \"No period\" must end in a period.")
}
+func (s *Suite) Test_Logger_Logf__wording(c *check.C) {
+ t := s.Init(c)
+
+ t.ExpectPanic(
+ func() { G.Logger.Logf(Error, "filename", "13", "This should.", "This should.") },
+ "Pkglint internal error: The word \"should\" must only appear in warnings: This should.")
+
+ t.ExpectPanic(
+ func() { G.Logger.Logf(Warn, "filename", "13", "This must.", "This must.") },
+ "Pkglint internal error: The word \"must\" must only appear in errors: This must.")
+
+ G.Logger.Logf(Note, "filename", "13", "This should.", "This should.")
+
+ t.CheckOutputLines(
+ "NOTE: filename:13: This should.")
+}
+
func (s *Suite) Test_Logger_TechErrorf__gcc_format(c *check.C) {
t := s.Init(c)
diff -r 8f681717ba3b -r d814cfcaf86f pkgtools/pkglint/files/mkline.go
--- a/pkgtools/pkglint/files/mkline.go Mon Dec 09 20:15:57 2019 +0000
+++ b/pkgtools/pkglint/files/mkline.go Mon Dec 09 20:38:15 2019 +0000
@@ -1277,29 +1277,46 @@
)
func MatchMkInclude(text string) (m bool, indentation, directive string, filename RelPath) {
- lexer := textproc.NewLexer(text)
- if lexer.SkipByte('.') {
- indentation = lexer.NextHspace()
- directive = lexer.NextString("include")
- if directive == "" {
- directive = lexer.NextString("sinclude")
- }
- if directive != "" {
- lexer.NextHspace()
- if lexer.SkipByte('"') {
- // Note: strictly speaking, the full MkVarUse would have to be parsed
- // here. But since these usually don't contain double quotes, it has
- // worked fine up to now.
- filename = NewRelPathString(lexer.NextBytesFunc(func(c byte) bool { return c != '"' }))
- if !filename.IsEmpty() && lexer.SkipByte('"') {
- lexer.NextHspace()
- if lexer.EOF() {
- m = true
- return
- }
- }
- }
- }
+ tokens, rest := NewMkLexer(text, nil).MkTokens()
+ if rest != "" {
+ return false, "", "", ""
+ }
+
+ lexer := NewMkTokensLexer(tokens)
+ if !lexer.SkipByte('.') {
+ return false, "", "", ""
+ }
+
+ indentation = lexer.NextHspace()
+
+ directive = lexer.NextString("include")
+ if directive == "" {
+ directive = lexer.NextString("sinclude")
+ }
+ if directive == "" {
+ return false, "", "", ""
}
- return false, "", "", ""
+
+ lexer.SkipHspace()
+ if !lexer.SkipByte('"') {
+ return false, "", "", ""
+ }
+
+ mark := lexer.Mark()
+ for lexer.NextBytesFunc(func(c byte) bool { return c != '"' && c != '$' }) != "" ||
+ lexer.NextVarUse() != nil {
+ }
+ enclosed := NewPath(lexer.Since(mark))
+
+ if enclosed.IsEmpty() || enclosed.IsAbs() || !lexer.SkipByte('"') {
+ return false, "", "", ""
+ }
+ lexer.SkipHspace()
+ if !lexer.EOF() {
+ return false, "", "", ""
+ }
+
+ filename = NewRelPath(enclosed)
+ m = true
+ return
}
diff -r 8f681717ba3b -r d814cfcaf86f pkgtools/pkglint/files/mkline_test.go
--- a/pkgtools/pkglint/files/mkline_test.go Mon Dec 09 20:15:57 2019 +0000
+++ b/pkgtools/pkglint/files/mkline_test.go Mon Dec 09 20:38:15 2019 +0000
@@ -1547,13 +1547,6 @@
}
}
- testFail("")
- testFail(".")
- testFail(".include")
- testFail(".include \"")
- testFail(".include \"other.mk")
- testFail(".include \"other.mk\" \"")
-
test(".include \"other.mk\"",
"", "include", "other.mk", "")
@@ -1566,5 +1559,25 @@
test(".include \"other.mk\"\t# comment",
"", "include", "other.mk", "# comment")
- t.CheckOutputEmpty()
+ test(". include \"${DIR}/file.mk\"",
+ " ", "include", "${DIR}/file.mk", "")
+
+ test(". include \"${DIR:S,\",',g}/file.mk\"",
+ " ", "include", "${DIR:S,\",',g}/file.mk", "")
+
+ test(".include \"${}\"",
+ "", "include", "${}", "")
+
+ testFail("")
+ testFail(".")
+ testFail(".include")
+ testFail(".include \"")
+ testFail(".include \"\"")
+ testFail(".include \"$\"")
+ testFail(".include \"$$\"")
+ testFail(".include \"other.mk")
+ testFail(".include \"other.mk\" \"")
+ testFail(".include \"/absolute\"")
+ testFail(".include \"/absolute\"rest")
+ testFail(".include \"/absolute\" rest")
}
diff -r 8f681717ba3b -r d814cfcaf86f pkgtools/pkglint/files/mklineparser.go
--- a/pkgtools/pkglint/files/mklineparser.go Mon Dec 09 20:15:57 2019 +0000
+++ b/pkgtools/pkglint/files/mklineparser.go Mon Dec 09 20:38:15 2019 +0000
@@ -72,33 +72,8 @@
return nil
}
- if a.spaceAfterVarname != "" {
- varname := a.varname
- op := a.op
- switch {
- case hasSuffix(varname, "+") && (op == opAssign || op == opAssignAppend):
- break
- case matches(varname, `^[a-z]`) && op == opAssignEval:
- break
- default:
- fix := line.Autofix()
- fix.Notef("Unnecessary space after variable name %q.", varname)
- fix.Replace(varname+a.spaceAfterVarname+op.String(), varname+op.String())
- fix.Apply()
- // FIXME: Preserve the alignment of the variable value.
- }
- }
-
- if splitResult.hasComment && a.value != "" && splitResult.spaceBeforeComment == "" {
- line.Warnf("The # character starts a Makefile comment.")
- line.Explain(
- "In a variable assignment, an unescaped # starts a comment that",
- "continues until the end of the line.",
- "To escape the #, write \\#.",
- "",
- "If this # character intentionally starts a comment,",
- "it should be preceded by a space in order to make it more visible.")
- }
+ p.fixSpaceAfterVarname(line, a)
+ p.checkUnintendedComment(&splitResult, a, line)
return &MkLine{line, splitResult, a}
}
@@ -181,6 +156,45 @@
}
}
+func (p MkLineParser) fixSpaceAfterVarname(line *Line, a *mkLineAssign) {
+ if !(a.spaceAfterVarname != "") {
+ return
+ }
+
+ varname := a.varname
+ op := a.op
+ switch {
+ case hasSuffix(varname, "+") && (op == opAssign || op == opAssignAppend):
+ break
+ case matches(varname, `^[a-z]`) && op == opAssignEval:
+ break
+
+ default:
+ before := a.valueAlign
+ after := alignWith(varname+op.String(), before)
+
+ fix := line.Autofix()
+ fix.Notef("Unnecessary space after variable name %q.", varname)
+ fix.Replace(before, after)
+ fix.Apply()
+ }
+}
+
+func (p MkLineParser) checkUnintendedComment(splitResult *mkLineSplitResult, a *mkLineAssign, line *Line) {
+ if !(splitResult.hasComment && a.value != "" && splitResult.spaceBeforeComment == "") {
+ return
+ }
+
+ line.Warnf("The # character starts a Makefile comment.")
+ line.Explain(
+ "In a variable assignment, an unescaped # starts a comment that",
+ "continues until the end of the line.",
+ "To escape the #, write \\#.",
+ "",
+ "If this # character intentionally starts a comment,",
+ "it should be preceded by a space in order to make it more visible.")
+}
+
func (p MkLineParser) parseShellcmd(line *Line, splitResult mkLineSplitResult) *MkLine {
return &MkLine{line, splitResult, mkLineShell{line.Text[1:]}}
}
diff -r 8f681717ba3b -r d814cfcaf86f pkgtools/pkglint/files/mklineparser_test.go
--- a/pkgtools/pkglint/files/mklineparser_test.go Mon Dec 09 20:15:57 2019 +0000
+++ b/pkgtools/pkglint/files/mklineparser_test.go Mon Dec 09 20:38:15 2019 +0000
@@ -146,58 +146,6 @@
"WARN: rubyversion.mk:427: Makefile lines should not start with space characters.")
}
-func (s *Suite) Test_MkLineParser_parseVarassign__space_around_operator(c *check.C) {
- t := s.Init(c)
-
- t.SetUpCommandLine("--show-autofix", "--source")
- t.NewMkLine("test.mk", 101,
- "pkgbase = package")
-
- t.CheckOutputLines(
- "NOTE: test.mk:101: Unnecessary space after variable name \"pkgbase\".",
- "AUTOFIX: test.mk:101: Replacing \"pkgbase =\" with \"pkgbase=\".",
- "-\tpkgbase = package",
- "+\tpkgbase= package")
-}
-
-func (s *Suite) Test_MkLineParser_parseVarassign__autofix_space_after_varname(c *check.C) {
- t := s.Init(c)
-
- filename := t.CreateFileLines("Makefile",
- MkCvsID,
- "VARNAME +=\t${VARNAME}",
- "VARNAME+ =\t${VARNAME+}",
- "VARNAME+ +=\t${VARNAME+}",
- "VARNAME+ ?=\t${VARNAME}",
- "pkgbase := pkglint")
-
- CheckFileMk(filename)
-
- t.CheckOutputLines(
- "NOTE: ~/Makefile:2: Unnecessary space after variable name \"VARNAME\".",
-
- // The assignment operators other than = and += cannot lead to ambiguities.
Home |
Main Index |
Thread Index |
Old Index