pkgsrc-Changes archive
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]
CVS commit: pkgsrc/pkgtools/pkglint
Module Name: pkgsrc
Committed By: rillig
Date: Mon Nov 4 18:44:21 UTC 2019
Modified Files:
pkgsrc/pkgtools/pkglint: Makefile
pkgsrc/pkgtools/pkglint/files: mklinechecker.go mklinechecker_test.go
mkshparser.go options_test.go shell.go shell_test.go
varalignblock.go varalignblock_test.go vardefs.go vartype.go
vartypecheck_test.go
Log Message:
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.
To generate a diff of this commit:
cvs rdiff -u -r1.605 -r1.606 pkgsrc/pkgtools/pkglint/Makefile
cvs rdiff -u -r1.51 -r1.52 pkgsrc/pkgtools/pkglint/files/mklinechecker.go
cvs rdiff -u -r1.46 -r1.47 \
pkgsrc/pkgtools/pkglint/files/mklinechecker_test.go \
pkgsrc/pkgtools/pkglint/files/shell.go
cvs rdiff -u -r1.15 -r1.16 pkgsrc/pkgtools/pkglint/files/mkshparser.go
cvs rdiff -u -r1.17 -r1.18 pkgsrc/pkgtools/pkglint/files/options_test.go
cvs rdiff -u -r1.54 -r1.55 pkgsrc/pkgtools/pkglint/files/shell_test.go
cvs rdiff -u -r1.6 -r1.7 pkgsrc/pkgtools/pkglint/files/varalignblock.go
cvs rdiff -u -r1.5 -r1.6 pkgsrc/pkgtools/pkglint/files/varalignblock_test.go
cvs rdiff -u -r1.75 -r1.76 pkgsrc/pkgtools/pkglint/files/vardefs.go
cvs rdiff -u -r1.37 -r1.38 pkgsrc/pkgtools/pkglint/files/vartype.go
cvs rdiff -u -r1.57 -r1.58 pkgsrc/pkgtools/pkglint/files/vartypecheck_test.go
Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.
Modified files:
Index: pkgsrc/pkgtools/pkglint/Makefile
diff -u pkgsrc/pkgtools/pkglint/Makefile:1.605 pkgsrc/pkgtools/pkglint/Makefile:1.606
--- pkgsrc/pkgtools/pkglint/Makefile:1.605 Sat Nov 2 16:37:48 2019
+++ pkgsrc/pkgtools/pkglint/Makefile Mon Nov 4 18:44:21 2019
@@ -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/}
Index: pkgsrc/pkgtools/pkglint/files/mklinechecker.go
diff -u pkgsrc/pkgtools/pkglint/files/mklinechecker.go:1.51 pkgsrc/pkgtools/pkglint/files/mklinechecker.go:1.52
--- pkgsrc/pkgtools/pkglint/files/mklinechecker.go:1.51 Sat Nov 2 16:37:48 2019
+++ pkgsrc/pkgtools/pkglint/files/mklinechecker.go Mon Nov 4 18:44:21 2019
@@ -66,6 +66,9 @@ func (ck MkLineChecker) checkShellComman
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 @@ func (ck MkLineChecker) checkShellComman
"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 @@ func (ck MkLineChecker) checkVartype(var
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())
}
Index: pkgsrc/pkgtools/pkglint/files/mklinechecker_test.go
diff -u pkgsrc/pkgtools/pkglint/files/mklinechecker_test.go:1.46 pkgsrc/pkgtools/pkglint/files/mklinechecker_test.go:1.47
--- pkgsrc/pkgtools/pkglint/files/mklinechecker_test.go:1.46 Sat Nov 2 16:37:48 2019
+++ pkgsrc/pkgtools/pkglint/files/mklinechecker_test.go Mon Nov 4 18:44:21 2019
@@ -25,6 +25,38 @@ func (s *Suite) Test_MkLineChecker_check
"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 @@ func (s *Suite) Test_MkLineChecker_check
"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 @@ func (s *Suite) Test_MkLineChecker_Check
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.
Index: pkgsrc/pkgtools/pkglint/files/shell.go
diff -u pkgsrc/pkgtools/pkglint/files/shell.go:1.46 pkgsrc/pkgtools/pkglint/files/shell.go:1.47
--- pkgsrc/pkgtools/pkglint/files/shell.go:1.46 Tue Oct 1 21:37:59 2019
+++ pkgsrc/pkgtools/pkglint/files/shell.go Mon Nov 4 18:44:21 2019
@@ -591,9 +591,6 @@ func (scc *SimpleCommandChecker) handleC
defer trace.Call0()()
}
- // FIXME: Research and explain how pkglint can ever interpret
- // a shell comment as a simple command. That just doesn't fit.
-
shellword := scc.strcmd.Name
if trace.Tracing {
defer trace.Call1(shellword)()
@@ -603,36 +600,26 @@ func (scc *SimpleCommandChecker) handleC
return false
}
- semicolon := contains(shellword, ";")
- multiline := scc.mkline.IsMultiline()
-
- if semicolon {
- scc.mkline.Warnf("A shell comment should not contain semicolons.")
- // TODO: Explain.
- // TODO: Check whether the existing warnings are useful.
- }
-
- if multiline {
- scc.mkline.Warnf("A shell comment does not stop at the end of line.")
+ if !scc.mkline.IsMultiline() {
+ return false
}
- if semicolon || multiline {
- scc.Explain(
- "When a shell command is split into multiple lines that are",
- "continued with a backslash, they will nevertheless be converted to",
- "a single line before the shell sees them.",
- "",
- "This means that even if it looks as if the comment only spanned",
- "one line in the Makefile, in fact it spans until the end of the whole",
- "shell command.",
- "",
- "To insert a comment into shell code, you can write it like this:",
- "",
- "\t"+"${SHCOMMENT} \"The following command might fail; this is ok.\"",
- "",
- "Note that any special characters in the comment are still",
- "interpreted by the shell.")
- }
+ scc.mkline.Warnf("A shell comment does not stop at the end of line.")
+ scc.Explain(
+ "When a shell command is split into multiple lines that are",
+ "continued with a backslash, they will nevertheless be converted to",
+ "a single line before the shell sees them.",
+ "",
+ "This means that even if it looks as if the comment only spanned",
+ "one line in the Makefile, in fact it spans until the end of the whole",
+ "shell command.",
+ "",
+ "To insert a comment into shell code, you can write it like this:",
+ "",
+ "\t"+"${SHCOMMENT} \"The following command might fail; this is ok.\"",
+ "",
+ "Note that any special characters in the comment are still",
+ "interpreted by the shell.")
return true
}
Index: pkgsrc/pkgtools/pkglint/files/mkshparser.go
diff -u pkgsrc/pkgtools/pkglint/files/mkshparser.go:1.15 pkgsrc/pkgtools/pkglint/files/mkshparser.go:1.16
--- pkgsrc/pkgtools/pkglint/files/mkshparser.go:1.15 Wed Aug 21 16:45:17 2019
+++ pkgsrc/pkgtools/pkglint/files/mkshparser.go Mon Nov 4 18:44:21 2019
@@ -80,6 +80,10 @@ func (lex *ShellLexer) Lex(lval *shyySym
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 @@ func (lex *ShellLexer) Lex(lval *shyySym
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
Index: pkgsrc/pkgtools/pkglint/files/options_test.go
diff -u pkgsrc/pkgtools/pkglint/files/options_test.go:1.17 pkgsrc/pkgtools/pkglint/files/options_test.go:1.18
--- pkgsrc/pkgtools/pkglint/files/options_test.go:1.17 Sat Nov 2 16:37:48 2019
+++ pkgsrc/pkgtools/pkglint/files/options_test.go Mon Nov 4 18:44:21 2019
@@ -103,6 +103,92 @@ func (s *Suite) Test_CheckLinesOptionsMk
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 @@ func (s *Suite) Test_CheckLinesOptionsMk
// 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)
-
- t.CheckOutputLines(
- "ERROR: ~/category/package/options.mk: "+
- "Each options.mk file must .include \"../../mk/bsd.options.mk\".",
- "WARN: ~/category/package/options.mk:3: "+
- "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/bsd.options.mk\"",
- "",
- ".if !empty(PKG_OPTIONS:O:u:Moption1) "+
- "|| !empty(PKG_OPTIONS:Noption1) "+
- "|| !empty(PKG_OPTIONS:O) "+
- "|| !empty(X11_TYPE) "+
- "|| !empty(PKG_OPTIONS:M${X11_TYPE})",
- ".endif")
-
- CheckLinesOptionsMk(mklines)
-
- // Although technically this option is handled by the :Noption1 modifier,
- // this is so unusual that the warning is justified.
- t.CheckOutputLines(
- "WARN: ~/category/package/options.mk:3: Option \"option1\" should be handled below in an .if block.")
-}
-
// If there is no .include line after the declaration of the package-settable
// variables, the whole analysis stops.
//
Index: pkgsrc/pkgtools/pkglint/files/shell_test.go
diff -u pkgsrc/pkgtools/pkglint/files/shell_test.go:1.54 pkgsrc/pkgtools/pkglint/files/shell_test.go:1.55
--- pkgsrc/pkgtools/pkglint/files/shell_test.go:1.54 Sat Oct 26 11:43:36 2019
+++ pkgsrc/pkgtools/pkglint/files/shell_test.go Mon Nov 4 18:44:21 2019
@@ -833,8 +833,8 @@ func (s *Suite) Test_ShellLineChecker__s
mklines.Check()
- t.CheckOutputLines(
- "WARN: ~/Makefile:3--4: A shell comment does not stop at the end of line.")
+ // TODO: "WARN: ~/Makefile:3--4: A shell comment does not stop at the end of line."
+ t.CheckOutputEmpty()
}
func (s *Suite) Test_ShellLineChecker_checkWordQuoting(c *check.C) {
@@ -1264,19 +1264,6 @@ func (s *Suite) Test_SimpleCommandChecke
t.CheckOutputEmpty()
}
-func (s *Suite) Test_SimpleCommandChecker_handleComment(c *check.C) {
- t := s.Init(c)
-
- mklines := t.NewMkLines("file.mk",
- MkCvsID,
- "\t# comment; continuation")
-
- mklines.Check()
-
- t.CheckOutputLines(
- "WARN: file.mk:2: A shell comment should not contain semicolons.")
-}
-
// This test ensures that the command line options to INSTALL_*_DIR are properly
// parsed and do not lead to "can only handle one directory at a time" warnings.
func (s *Suite) Test_SimpleCommandChecker_checkInstallMulti(c *check.C) {
Index: pkgsrc/pkgtools/pkglint/files/varalignblock.go
diff -u pkgsrc/pkgtools/pkglint/files/varalignblock.go:1.6 pkgsrc/pkgtools/pkglint/files/varalignblock.go:1.7
--- pkgsrc/pkgtools/pkglint/files/varalignblock.go:1.6 Sat Oct 26 09:51:48 2019
+++ pkgsrc/pkgtools/pkglint/files/varalignblock.go Mon Nov 4 18:44:21 2019
@@ -87,6 +87,9 @@ type varalignLine struct {
// of the first value found.
multiEmpty bool
+ // Whether the line is so long that it may use a single tab as indentation.
+ long bool
+
varalignParts
}
@@ -133,7 +136,7 @@ func (va *VaralignBlock) processVarassig
follow := false
for i, raw := range mkline.raw {
parts := NewVaralignSplitter().split(strings.TrimSuffix(raw.textnl, "\n"), i == 0)
- info := varalignLine{mkline, i, follow, parts}
+ info := varalignLine{mkline, i, follow, false, parts}
if i == 0 && info.isEmptyContinuation() {
follow = true
@@ -158,6 +161,7 @@ func (va *VaralignBlock) Finish() {
}
newWidth := va.optimalWidth(infos)
+ va.adjustLong(newWidth, infos)
rightMargin := 0
// When the indentation of the initial line of a multiline is
@@ -179,7 +183,7 @@ func (va *VaralignBlock) Finish() {
rightMargin = va.rightMargin(infos[restIndex:])
}
- va.checkContinuationIndentation(info, newWidth, rightMargin)
+ va.checkRightMargin(info, newWidth, rightMargin)
if newWidth > 0 || info.rawIndex > 0 {
va.realign(info, newWidth, &indentDiffSet, &indentDiff)
@@ -287,7 +291,20 @@ func (*VaralignBlock) optimalWidth(infos
return (minVarnameOpWidth & -8) + 8
}
-func (va *VaralignBlock) checkContinuationIndentation(info *varalignLine, newWidth int, rightMargin int) {
+func (va *VaralignBlock) adjustLong(newWidth int, infos []*varalignLine) {
+ long := false
+ for _, info := range infos {
+ if info.rawIndex == 0 {
+ long = false
+ }
+ if !info.multiEmpty && info.spaceBeforeValue == "\t" && info.varnameOpSpaceWidth() != newWidth && info.widthAlignedAt(newWidth) > 72 {
+ long = true
+ }
+ info.long = long
+ }
+}
+
+func (va *VaralignBlock) checkRightMargin(info *varalignLine, newWidth int, rightMargin int) {
if !info.isContinuation() {
return
}
@@ -352,7 +369,7 @@ func (*VaralignBlock) realignMultiEmptyI
}
if newSpace == " " {
- return // This case is handled by checkContinuationIndentation.
+ return // This case is handled by checkRightMargin.
}
hasSpace := strings.IndexByte(oldSpace, ' ') != -1
@@ -435,7 +452,7 @@ func (va *VaralignBlock) realignMultiFol
if tabWidth(newSpace) < newWidth {
newSpace = indent(newWidth)
}
- if newSpace == oldSpace || (oldSpace == "\t" && info.widthAlignedAt(newWidth) > 72) {
+ if newSpace == oldSpace || info.long {
return
}
Index: pkgsrc/pkgtools/pkglint/files/varalignblock_test.go
diff -u pkgsrc/pkgtools/pkglint/files/varalignblock_test.go:1.5 pkgsrc/pkgtools/pkglint/files/varalignblock_test.go:1.6
--- pkgsrc/pkgtools/pkglint/files/varalignblock_test.go:1.5 Sun Sep 8 22:47:47 2019
+++ pkgsrc/pkgtools/pkglint/files/varalignblock_test.go Mon Nov 4 18:44:21 2019
@@ -118,6 +118,13 @@ func (vt *VaralignTester) checkTestName(
name string
width int
}
+ descriptorsString := func(ds []descriptor) string {
+ var strs []string
+ for _, d := range ds {
+ strs = append(strs, sprintf("%s%d", d.name, d.width))
+ }
+ return strings.Join(strs, "_")
+ }
var actual []descriptor
width := 0
@@ -195,6 +202,7 @@ func (vt *VaralignTester) checkTestName(
expected = append(expected, descriptor{name, width})
}
+ vt.tester.CheckDeepEquals(descriptorsString(actual), descriptorsString(expected))
vt.tester.CheckDeepEquals(actual, expected)
}
@@ -2002,6 +2010,38 @@ func (s *Suite) Test_VaralignBlock__lead
vt.Run()
}
+// Before 19.3.6, pkglint would indent the last line in column 16.
+//
+// The value in the first line starts in column 16, which means that all
+// follow-up lines should also start in column 16 or further to the right.
+// Line 2 though is already quite long, and since its right margin is in
+// column 72, it may keep its lower-than-usual indentation of 8.
+// Line 3 is not that long, therefore the rule from line 2 doesn't apply
+// here, and it needs to be indented to column 16.
+//
+// Since the above result would look inconsistent, all follow-up lines
+// after a long line may be indented in column 8 as well.
+func (s *Suite) Test_VaralignBlock__var_tab_value63_space_cont_tab8_value71_space_cont_tab8_value(c *check.C) {
+ vt := NewVaralignTester(s, c)
+ vt.Input(
+ "PROGFILES=\t67890 234567890 234567890 234567890 234567890 2 \\",
+ "\t890 234567890 234567890 234567890 234567890 234567890 234567890 \\",
+ "\tvalue")
+ vt.Internals(
+ "10 16 64",
+ " 08 72",
+ " 08")
+ vt.Diagnostics(
+ nil...)
+ vt.Autofixes(
+ nil...)
+ vt.Fixed(
+ "PROGFILES= 67890 234567890 234567890 234567890 234567890 2 \\",
+ " 890 234567890 234567890 234567890 234567890 234567890 234567890 \\",
+ " value")
+ vt.Run()
+}
+
// Up to 2018-01-27, it could happen that some source code was logged
// without a corresponding diagnostic. This was unintended and confusing.
func (s *Suite) Test_VaralignBlock__fix_without_diagnostic(c *check.C) {
@@ -2215,6 +2255,24 @@ func (s *Suite) Test_VaralignBlock__mixe
vt.Run()
}
+func (s *Suite) Test_VaralignBlock__long_line_followed_by_short_line_with_small_indentation(c *check.C) {
+ vt := NewVaralignTester(s, c)
+ vt.Input(
+ "VAR.567890123456+=\t----30 -------40 -------50 -------60 -------70 234567 \\",
+ "\t\t--20 -------30")
+ vt.Internals(
+ "18 24 78",
+ " 16")
+ vt.Diagnostics(
+ "NOTE: Makefile:2: This continuation line should be indented with \"\\t\\t\\t\".")
+ vt.Autofixes(
+ "AUTOFIX: Makefile:2: Replacing \"\\t\\t\" with \"\\t\\t\\t\".")
+ vt.Fixed(
+ "VAR.567890123456+= ----30 -------40 -------50 -------60 -------70 234567 \\",
+ " --20 -------30")
+ vt.Run()
+}
+
// Ensure that the end-of-line comment is properly aligned
// to the variable values.
//
Index: pkgsrc/pkgtools/pkglint/files/vardefs.go
diff -u pkgsrc/pkgtools/pkglint/files/vardefs.go:1.75 pkgsrc/pkgtools/pkglint/files/vardefs.go:1.76
--- pkgsrc/pkgtools/pkglint/files/vardefs.go:1.75 Sat Nov 2 16:37:48 2019
+++ pkgsrc/pkgtools/pkglint/files/vardefs.go Mon Nov 4 18:44:21 2019
@@ -156,6 +156,15 @@ func (reg *VarTypeRegistry) pkglistrat(v
"Makefile, Makefile.*, *.mk: default, set, append, use")
}
+// Like pkglist, but only one value per line should be given.
+// Typical example: PKG_FAIL_REASON.
+func (reg *VarTypeRegistry) pkglistone(varname string, basicType *BasicType) {
+ reg.acllist(varname, basicType,
+ List|PackageSettable|OnePerLine,
+ "buildlink3.mk, builtin.mk: none",
+ "Makefile, Makefile.*, *.mk: default, set, append, use")
+}
+
// A package-defined load-time list may be used or defined or appended to in
// all Makefiles except buildlink3.mk and builtin.mk. Simple assignment
// (instead of appending) is also allowed. If this leads to an unconditional
@@ -816,8 +825,7 @@ func (reg *VarTypeRegistry) Init(src *Pk
reg.sys("BINOWN", BtUserGroupName)
reg.pkglist("BOOTSTRAP_DEPENDS", BtDependencyWithPath)
reg.pkg("BOOTSTRAP_PKG", BtYesNo)
- // BROKEN should better be a list of messages instead of a simple string.
- reg.pkgappend("BROKEN", BtMessage)
+ reg.pkglistone("BROKEN", BtShellWord)
reg.pkg("BROKEN_GETTEXT_DETECTION", BtYesNo)
reg.pkglistrat("BROKEN_EXCEPT_ON_PLATFORM", BtMachinePlatformPattern)
reg.pkglistrat("BROKEN_ON_PLATFORM", BtMachinePlatformPattern)
@@ -1391,7 +1399,7 @@ func (reg *VarTypeRegistry) Init(src *Pk
reg.usrlist("PKG_DEFAULT_OPTIONS", BtOption)
reg.sys("PKG_DELETE", BtShellCommand)
reg.pkglist("PKG_DESTDIR_SUPPORT", enum("destdir user-destdir"))
- reg.pkglist("PKG_FAIL_REASON", BtShellWord)
+ reg.pkglistone("PKG_FAIL_REASON", BtShellWord)
reg.sysload("PKG_FORMAT", BtIdentifier)
reg.pkg("PKG_GECOS.*", BtMessage)
reg.pkg("PKG_GID.*", BtInteger)
@@ -1440,7 +1448,7 @@ func (reg *VarTypeRegistry) Init(src *Pk
reg.pkg("PKG_SHELL", BtPathname)
reg.pkg("PKG_SHELL.*", BtPathname)
reg.sys("PKG_SHLIBTOOL", BtPathname)
- reg.pkglist("PKG_SKIP_REASON", BtShellWord)
+ reg.pkglistone("PKG_SKIP_REASON", BtShellWord)
// The special exception for buildlink3.mk is only here because
// of textproc/xmlcatmgr.
reg.acl("PKG_SYSCONFDIR*", BtPathname,
Index: pkgsrc/pkgtools/pkglint/files/vartype.go
diff -u pkgsrc/pkgtools/pkglint/files/vartype.go:1.37 pkgsrc/pkgtools/pkglint/files/vartype.go:1.38
--- pkgsrc/pkgtools/pkglint/files/vartype.go:1.37 Tue Oct 1 21:37:59 2019
+++ pkgsrc/pkgtools/pkglint/files/vartype.go Mon Nov 4 18:44:21 2019
@@ -38,6 +38,8 @@ const (
// describing why they are set. Typical examples are NOT_FOR_* variables.
NeedsRationale
+ OnePerLine
+
NoVartypeOptions = 0
)
@@ -101,6 +103,7 @@ func (vt *Vartype) UserSettable() bool
func (vt *Vartype) SystemProvided() bool { return vt.options&SystemProvided != 0 }
func (vt *Vartype) CommandLineProvided() bool { return vt.options&CommandLineProvided != 0 }
func (vt *Vartype) NeedsRationale() bool { return vt.options&NeedsRationale != 0 }
+func (vt *Vartype) OnePerLine() bool { return vt.options&OnePerLine != 0 }
func (vt *Vartype) EffectivePermissions(basename string) ACLPermissions {
for _, aclEntry := range vt.aclEntries {
Index: pkgsrc/pkgtools/pkglint/files/vartypecheck_test.go
diff -u pkgsrc/pkgtools/pkglint/files/vartypecheck_test.go:1.57 pkgsrc/pkgtools/pkglint/files/vartypecheck_test.go:1.58
--- pkgsrc/pkgtools/pkglint/files/vartypecheck_test.go:1.57 Sat Oct 26 09:51:48 2019
+++ pkgsrc/pkgtools/pkglint/files/vartypecheck_test.go Mon Nov 4 18:44:21 2019
@@ -1457,6 +1457,23 @@ func (s *Suite) Test_VartypeCheck_ShellC
"WARN: filename.mk:1: This shell command list should end with a semicolon.")
}
+func (s *Suite) Test_VartypeCheck_ShellWord(c *check.C) {
+ t := s.Init(c)
+ t.SetUpVartypes()
+ vt := NewVartypeCheckTester(t, BtShellWord)
+
+ vt.Varname("PKG_FAIL_REASON")
+ vt.Values(
+ "The package does not work here.",
+ "\"Properly quoted reason.\"")
+
+ // At this level, there can be no warning for line 1 since each word
+ // is analyzed on its own.
+ //
+ // See Test_MkLineChecker_checkVartype__one_per_line.
+ vt.OutputEmpty()
+}
+
func (s *Suite) Test_VartypeCheck_Stage(c *check.C) {
vt := NewVartypeCheckTester(s.Init(c), BtStage)
Home |
Main Index |
Thread Index |
Old Index