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: Sun Jan 28 23:21:16 UTC 2018
Modified Files:
pkgsrc/pkgtools/pkglint: Makefile
pkgsrc/pkgtools/pkglint/files: autofix.go autofix_test.go category.go
files.go files_test.go globaldata.go linechecker.go mkline.go
mkline_test.go mklinechecker.go mklinechecker_test.go mklines.go
mklines_varalign_test.go patches.go patches_test.go pkglint.go
pkglint_test.go plist.go plist_test.go
pkgsrc/pkgtools/pkglint/files/histogram: histogram.go
Log Message:
pkgtools/pkglint: update to 5.5.3
Changes since 5.5.2:
* Fixed lots of bugs regarding autofixing variable assignments in
continuation lines.
* Fixed checking of MESSAGE files, which also get fixed now.
* In variable assignments, commented assignments are aligned too.
* Fixed a crash when checking an empty patch file.
* The :Q modifier is only checked on predefined variables, to prevent
the --autofix mode from removing :Q from user-defined variables.
* Fixed lots of bugs in PLIST autofixing: relevant lines had been
removed, and the sorting was not correct.
To generate a diff of this commit:
cvs rdiff -u -r1.527 -r1.528 pkgsrc/pkgtools/pkglint/Makefile
cvs rdiff -u -r1.2 -r1.3 pkgsrc/pkgtools/pkglint/files/autofix.go \
pkgsrc/pkgtools/pkglint/files/autofix_test.go
cvs rdiff -u -r1.10 -r1.11 pkgsrc/pkgtools/pkglint/files/category.go
cvs rdiff -u -r1.14 -r1.15 pkgsrc/pkgtools/pkglint/files/files.go \
pkgsrc/pkgtools/pkglint/files/patches_test.go
cvs rdiff -u -r1.13 -r1.14 pkgsrc/pkgtools/pkglint/files/files_test.go \
pkgsrc/pkgtools/pkglint/files/pkglint_test.go
cvs rdiff -u -r1.23 -r1.24 pkgsrc/pkgtools/pkglint/files/globaldata.go
cvs rdiff -u -r1.5 -r1.6 pkgsrc/pkgtools/pkglint/files/linechecker.go \
pkgsrc/pkgtools/pkglint/files/mklinechecker_test.go
cvs rdiff -u -r1.26 -r1.27 pkgsrc/pkgtools/pkglint/files/mkline.go
cvs rdiff -u -r1.29 -r1.30 pkgsrc/pkgtools/pkglint/files/mkline_test.go
cvs rdiff -u -r1.7 -r1.8 pkgsrc/pkgtools/pkglint/files/mklinechecker.go
cvs rdiff -u -r1.19 -r1.20 pkgsrc/pkgtools/pkglint/files/mklines.go \
pkgsrc/pkgtools/pkglint/files/plist_test.go
cvs rdiff -u -r1.1 -r1.2 \
pkgsrc/pkgtools/pkglint/files/mklines_varalign_test.go
cvs rdiff -u -r1.18 -r1.19 pkgsrc/pkgtools/pkglint/files/patches.go
cvs rdiff -u -r1.25 -r1.26 pkgsrc/pkgtools/pkglint/files/pkglint.go
cvs rdiff -u -r1.20 -r1.21 pkgsrc/pkgtools/pkglint/files/plist.go
cvs rdiff -u -r1.1 -r1.2 pkgsrc/pkgtools/pkglint/files/histogram/histogram.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.527 pkgsrc/pkgtools/pkglint/Makefile:1.528
--- pkgsrc/pkgtools/pkglint/Makefile:1.527 Sun Jan 28 13:40:22 2018
+++ pkgsrc/pkgtools/pkglint/Makefile Sun Jan 28 23:21:16 2018
@@ -1,6 +1,6 @@
-# $NetBSD: Makefile,v 1.527 2018/01/28 13:40:22 rillig Exp $
+# $NetBSD: Makefile,v 1.528 2018/01/28 23:21:16 rillig Exp $
-PKGNAME= pkglint-5.5.2
+PKGNAME= pkglint-5.5.3
DISTFILES= # none
CATEGORIES= pkgtools
Index: pkgsrc/pkgtools/pkglint/files/autofix.go
diff -u pkgsrc/pkgtools/pkglint/files/autofix.go:1.2 pkgsrc/pkgtools/pkglint/files/autofix.go:1.3
--- pkgsrc/pkgtools/pkglint/files/autofix.go:1.2 Sat Jan 27 18:50:36 2018
+++ pkgsrc/pkgtools/pkglint/files/autofix.go Sun Jan 28 23:21:16 2018
@@ -62,25 +62,42 @@ func (fix *Autofix) ReplaceAfter(prefix,
}
}
-func (fix *Autofix) ReplaceRegex(from regex.Pattern, to string) {
+// ReplaceRegex replaces the first or all occurrences of the `from` pattern
+// with the fixed string `toText`. Placeholders like `$1` are _not_ expanded.
+// (If you know how to do the expansion correctly, feel free to implement it.)
+func (fix *Autofix) ReplaceRegex(from regex.Pattern, toText string, howOften int) {
if fix.skip() {
return
}
+ done := 0
for _, rawLine := range fix.lines {
if rawLine.Lineno != 0 {
- if replaced := regex.Compile(from).ReplaceAllString(rawLine.textnl, to); replaced != rawLine.textnl {
+ var froms []string // The strings that have actually changed
+
+ replace := func(fromText string) string {
+ if howOften >= 0 && done >= howOften {
+ return fromText
+ }
+ froms = append(froms, fromText)
+ done++
+ return toText
+ }
+
+ if replaced := regex.Compile(from).ReplaceAllStringFunc(rawLine.textnl, replace); replaced != rawLine.textnl {
if G.opts.PrintAutofix || G.opts.Autofix {
rawLine.textnl = replaced
}
- fix.Describef(rawLine.Lineno, "Replacing regular expression %q with %q.", from, to)
+ for _, fromText := range froms {
+ fix.Describef(rawLine.Lineno, "Replacing %q with %q.", fromText, toText)
+ }
}
}
}
}
func (fix *Autofix) Realign(mkline MkLine, newWidth int) {
- if fix.skip() || !mkline.IsMultiline() || !mkline.IsVarassign() {
+ if fix.skip() || !mkline.IsMultiline() || !(mkline.IsVarassign() || mkline.IsCommentedVarassign()) {
return
}
@@ -90,19 +107,19 @@ func (fix *Autofix) Realign(mkline MkLin
{
// Interpreting the continuation marker as variable value
// is cheating, but works well.
- m, _, _, _, valueAlign, value, _, _ := MatchVarassign(mkline.raw[0].orignl)
+ m, _, _, _, _, valueAlign, value, _, _ := MatchVarassign(mkline.raw[0].orignl)
if m && value != "\\" {
oldWidth = tabWidth(valueAlign)
}
}
for _, rawLine := range fix.lines[1:] {
- _, space := regex.Match1(rawLine.textnl, `^(\s*)`)
- width := tabWidth(space)
- if oldWidth == 0 || width < oldWidth {
+ _, comment, space := regex.Match2(rawLine.textnl, `^(#?)([ \t]*)`)
+ width := tabWidth(comment + space)
+ if (oldWidth == 0 || width < oldWidth) && width >= 8 && rawLine.textnl != "\n" {
oldWidth = width
}
- if !regex.Matches(space, `^\t*\s{0,7}`) {
+ if !regex.Matches(space, `^\t* {0,7}`) {
normalized = false
}
}
@@ -119,10 +136,11 @@ func (fix *Autofix) Realign(mkline MkLin
}
for _, rawLine := range fix.lines[1:] {
- _, oldSpace := regex.Match1(rawLine.textnl, `^(\s*)`)
+ _, comment, oldSpace := regex.Match2(rawLine.textnl, `^(#?)([ \t]*)`)
newWidth := tabWidth(oldSpace) - oldWidth + newWidth
newSpace := strings.Repeat("\t", newWidth/8) + strings.Repeat(" ", newWidth%8)
- if replaced := strings.Replace(rawLine.textnl, oldSpace, newSpace, 1); replaced != rawLine.textnl {
+ replaced := strings.Replace(rawLine.textnl, comment+oldSpace, comment+newSpace, 1)
+ if replaced != rawLine.textnl {
if G.opts.PrintAutofix || G.opts.Autofix {
rawLine.textnl = replaced
}
@@ -131,6 +149,8 @@ func (fix *Autofix) Realign(mkline MkLin
}
}
+// InsertBefore prepends a line before the current line.
+// The newline is added internally.
func (fix *Autofix) InsertBefore(text string) {
if fix.skip() {
return
@@ -140,6 +160,8 @@ func (fix *Autofix) InsertBefore(text st
fix.Describef(fix.lines[0].Lineno, "Inserting a line %q before this line.", text)
}
+// InsertBefore appends a line after the current line.
+// The newline is added internally.
func (fix *Autofix) InsertAfter(text string) {
if fix.skip() {
return
Index: pkgsrc/pkgtools/pkglint/files/autofix_test.go
diff -u pkgsrc/pkgtools/pkglint/files/autofix_test.go:1.2 pkgsrc/pkgtools/pkglint/files/autofix_test.go:1.3
--- pkgsrc/pkgtools/pkglint/files/autofix_test.go:1.2 Sat Jan 27 18:50:36 2018
+++ pkgsrc/pkgtools/pkglint/files/autofix_test.go Sun Jan 28 23:21:16 2018
@@ -13,7 +13,7 @@ func (s *Suite) Test_Autofix_ReplaceRege
fix := lines[1].Autofix()
fix.Warnf("Something's wrong here.")
- fix.ReplaceRegex(`.`, "X")
+ fix.ReplaceRegex(`.`, "X", -1)
fix.Apply()
SaveAutofixChanges(lines)
@@ -24,7 +24,11 @@ func (s *Suite) Test_Autofix_ReplaceRege
"line3")
t.CheckOutputLines(
"WARN: ~/Makefile:2: Something's wrong here.",
- "AUTOFIX: ~/Makefile:2: Replacing regular expression \".\" with \"X\".")
+ "AUTOFIX: ~/Makefile:2: Replacing \"l\" with \"X\".",
+ "AUTOFIX: ~/Makefile:2: Replacing \"i\" with \"X\".",
+ "AUTOFIX: ~/Makefile:2: Replacing \"n\" with \"X\".",
+ "AUTOFIX: ~/Makefile:2: Replacing \"e\" with \"X\".",
+ "AUTOFIX: ~/Makefile:2: Replacing \"2\" with \"X\".")
}
func (s *Suite) Test_Autofix_ReplaceRegex_with_autofix(c *check.C) {
@@ -38,27 +42,32 @@ func (s *Suite) Test_Autofix_ReplaceRege
fix := lines[1].Autofix()
fix.Warnf("Something's wrong here.")
- fix.ReplaceRegex(`.`, "X")
+ fix.ReplaceRegex(`.`, "X", 3)
fix.Apply()
+ t.CheckOutputLines(
+ "AUTOFIX: ~/Makefile:2: Replacing \"l\" with \"X\".",
+ "AUTOFIX: ~/Makefile:2: Replacing \"i\" with \"X\".",
+ "AUTOFIX: ~/Makefile:2: Replacing \"n\" with \"X\".",
+ "-\tline2",
+ "+\tXXXe2")
+
fix.Warnf("Use Y instead of X.")
fix.Replace("X", "Y")
fix.Apply()
+ t.CheckOutputLines(
+ "",
+ "AUTOFIX: ~/Makefile:2: Replacing \"X\" with \"Y\".",
+ "-\tline2",
+ "+\tYXXe2")
+
SaveAutofixChanges(lines)
t.CheckFileLines("Makefile",
"line1",
- "YXXXX",
+ "YXXe2",
"line3")
- t.CheckOutputLines(
- "AUTOFIX: ~/Makefile:2: Replacing regular expression \".\" with \"X\".",
- "-\tline2",
- "+\tXXXXX",
- "",
- "AUTOFIX: ~/Makefile:2: Replacing \"X\" with \"Y\".",
- "-\tline2",
- "+\tYXXXX")
}
func (s *Suite) Test_Autofix_ReplaceRegex_with_show_autofix(c *check.C) {
@@ -72,7 +81,7 @@ func (s *Suite) Test_Autofix_ReplaceRege
fix := lines[1].Autofix()
fix.Warnf("Something's wrong here.")
- fix.ReplaceRegex(`.`, "X")
+ fix.ReplaceRegex(`.`, "X", -1)
fix.Apply()
fix.Warnf("Use Y instead of X.")
@@ -83,7 +92,11 @@ func (s *Suite) Test_Autofix_ReplaceRege
t.CheckOutputLines(
"WARN: ~/Makefile:2: Something's wrong here.",
- "AUTOFIX: ~/Makefile:2: Replacing regular expression \".\" with \"X\".",
+ "AUTOFIX: ~/Makefile:2: Replacing \"l\" with \"X\".",
+ "AUTOFIX: ~/Makefile:2: Replacing \"i\" with \"X\".",
+ "AUTOFIX: ~/Makefile:2: Replacing \"n\" with \"X\".",
+ "AUTOFIX: ~/Makefile:2: Replacing \"e\" with \"X\".",
+ "AUTOFIX: ~/Makefile:2: Replacing \"2\" with \"X\".",
"-\tline2",
"+\tXXXXX",
"",
@@ -108,17 +121,27 @@ func (s *Suite) Test_autofix_MkLines(c *
fix := mklines.mklines[1].Autofix()
fix.Warnf("Something's wrong here.")
- fix.ReplaceRegex(`.`, "X")
+ fix.ReplaceRegex(`...`, "XXX", -1)
+ fix.Apply()
+
+ fix = mklines.mklines[2].Autofix()
+ fix.Warnf("Something's wrong here.")
+ fix.ReplaceRegex(`...`, "XXX", 1)
fix.Apply()
SaveAutofixChanges(mklines.lines)
+ t.CheckOutputLines(
+ "AUTOFIX: ~/Makefile:2: Replacing \"lin\" with \"XXX\".",
+ "AUTOFIX: ~/Makefile:2: Replacing \"e2 \" with \"XXX\".",
+ "AUTOFIX: ~/Makefile:2: Replacing \":= \" with \"XXX\".",
+ "AUTOFIX: ~/Makefile:2: Replacing \"val\" with \"XXX\".",
+ "AUTOFIX: ~/Makefile:2: Replacing \"ue2\" with \"XXX\".",
+ "AUTOFIX: ~/Makefile:3: Replacing \"lin\" with \"XXX\".")
t.CheckFileLines("Makefile",
"line1 := value1",
"XXXXXXXXXXXXXXX",
- "line3 := value3")
- t.CheckOutputLines(
- "AUTOFIX: ~/Makefile:2: Replacing regular expression \".\" with \"X\".")
+ "XXXe3 := value3")
}
func (s *Suite) Test_Autofix_multiple_modifications(c *check.C) {
@@ -134,14 +157,14 @@ func (s *Suite) Test_Autofix_multiple_mo
{
fix := line.Autofix()
fix.Warnf("Silent-Magic-Diagnostic")
- fix.ReplaceRegex(`(.)(.*)(.)`, "$3$2$1")
+ fix.ReplaceRegex(`(.)(.*)(.)`, "lriginao", 1) // XXX: the replacement should be "$3$2$1"
fix.Apply()
}
c.Check(line.autofix, check.NotNil)
c.Check(line.raw, check.DeepEquals, t.NewRawLines(1, "original\n", "lriginao\n"))
t.CheckOutputLines(
- "AUTOFIX: fname:1: Replacing regular expression \"(.)(.*)(.)\" with \"$3$2$1\".")
+ "AUTOFIX: fname:1: Replacing \"original\" with \"lriginao\".")
{
fix := line.Autofix()
@@ -304,7 +327,7 @@ func (s *Suite) Test_Autofix_suppress_un
fix := lines[1].Autofix()
fix.Warnf("Something's wrong here.")
- fix.ReplaceRegex(`.`, "X")
+ fix.ReplaceRegex(`.`, "X", -1)
fix.Apply()
fix.Warnf("The XXX marks are usually not fixed, use TODO instead.")
@@ -315,7 +338,11 @@ func (s *Suite) Test_Autofix_suppress_un
t.CheckOutputLines(
"WARN: Makefile:2: Something's wrong here.",
- "AUTOFIX: Makefile:2: Replacing regular expression \".\" with \"X\".",
+ "AUTOFIX: Makefile:2: Replacing \"l\" with \"X\".",
+ "AUTOFIX: Makefile:2: Replacing \"i\" with \"X\".",
+ "AUTOFIX: Makefile:2: Replacing \"n\" with \"X\".",
+ "AUTOFIX: Makefile:2: Replacing \"e\" with \"X\".",
+ "AUTOFIX: Makefile:2: Replacing \"2\" with \"X\".",
"-\tline2",
"+\tXXXXX",
"",
@@ -333,7 +360,7 @@ func (s *Suite) Test_Autofix_failed_repl
fix := line.Autofix()
fix.Warnf("All-uppercase words should not be used at all.")
- fix.ReplaceRegex(`\b[A-Z]{3,}\b`, "---censored---")
+ fix.ReplaceRegex(`\b[A-Z]{3,}\b`, "---censored---", -1)
fix.Apply()
// No output since there was no all-uppercase word in the text.
Index: pkgsrc/pkgtools/pkglint/files/category.go
diff -u pkgsrc/pkgtools/pkglint/files/category.go:1.10 pkgsrc/pkgtools/pkglint/files/category.go:1.11
--- pkgsrc/pkgtools/pkglint/files/category.go:1.10 Sat Jan 13 23:56:14 2018
+++ pkgsrc/pkgtools/pkglint/files/category.go Sun Jan 28 23:21:16 2018
@@ -41,7 +41,7 @@ func CheckdirCategory() {
// collect the SUBDIRs in the Makefile and in the file system.
fSubdirs := getSubdirs(G.CurrentDir)
- sort.Sort(sort.StringSlice(fSubdirs))
+ sort.Strings(fSubdirs)
var mSubdirs []subdir
prevSubdir := ""
Index: pkgsrc/pkgtools/pkglint/files/files.go
diff -u pkgsrc/pkgtools/pkglint/files/files.go:1.14 pkgsrc/pkgtools/pkglint/files/files.go:1.15
--- pkgsrc/pkgtools/pkglint/files/files.go:1.14 Sat Jan 27 18:50:36 2018
+++ pkgsrc/pkgtools/pkglint/files/files.go Sun Jan 28 23:21:16 2018
@@ -26,7 +26,7 @@ func LoadExistingLines(fname string, joi
return lines
}
-func getLogicalLine(fname string, rawLines []*RawLine, pindex *int) Line {
+func nextLogicalLine(fname string, rawLines []*RawLine, pindex *int) Line {
{ // Handle the common case efficiently
index := *pindex
rawLine := rawLines[index]
@@ -68,26 +68,30 @@ func getLogicalLine(fname string, rawLin
}
func splitRawLine(textnl string) (leadingWhitespace, text, trailingWhitespace, cont string) {
- i, m := 0, len(textnl)
+ end := len(textnl)
- if m > i && textnl[m-1] == '\n' {
- m--
+ if end-1 >= 0 && textnl[end-1] == '\n' {
+ end--
}
- if m > i && textnl[m-1] == '\\' {
- m--
- cont = textnl[m : m+1]
+ backslashes := 0
+ for end-1 >= 0 && textnl[end-1] == '\\' {
+ end--
+ backslashes++
}
+ cont = textnl[end : end+backslashes%2]
+ end += backslashes / 2
- trailingEnd := m
- for m > i && (textnl[m-1] == ' ' || textnl[m-1] == '\t') {
- m--
+ trailingEnd := end
+ for end-1 >= 0 && (textnl[end-1] == ' ' || textnl[end-1] == '\t') {
+ end--
}
- trailingStart := m
+ trailingStart := end
trailingWhitespace = textnl[trailingStart:trailingEnd]
+ i := 0
leadingStart := i
- for i < m && (textnl[i] == ' ' || textnl[i] == '\t') {
+ for i < end && (textnl[i] == ' ' || textnl[i] == '\t') {
i++
}
leadingEnd := i
@@ -117,7 +121,7 @@ func convertToLogicalLines(fname string,
var loglines []Line
if joinBackslashLines {
for lineno := 0; lineno < len(rawLines); {
- loglines = append(loglines, getLogicalLine(fname, rawLines, &lineno))
+ loglines = append(loglines, nextLogicalLine(fname, rawLines, &lineno))
}
} else {
for _, rawLine := range rawLines {
Index: pkgsrc/pkgtools/pkglint/files/patches_test.go
diff -u pkgsrc/pkgtools/pkglint/files/patches_test.go:1.14 pkgsrc/pkgtools/pkglint/files/patches_test.go:1.15
--- pkgsrc/pkgtools/pkglint/files/patches_test.go:1.14 Sat Jan 27 18:50:36 2018
+++ pkgsrc/pkgtools/pkglint/files/patches_test.go Sun Jan 28 23:21:16 2018
@@ -408,3 +408,30 @@ func (s *Suite) Test_ChecklinesPatch__co
t.CheckOutputEmpty()
}
+
+// Must not panic.
+func (s *Suite) Test_ChecklinesPatch__autofix_empty_patch(c *check.C) {
+ t := s.Init(c)
+
+ t.SetupCommandLine("-Wall", "--autofix")
+ lines := t.NewLines("patch-aa",
+ RcsId)
+
+ ChecklinesPatch(lines)
+
+ t.CheckOutputEmpty()
+}
+
+// Must not panic.
+func (s *Suite) Test_ChecklinesPatch__autofix_long_empty_patch(c *check.C) {
+ t := s.Init(c)
+
+ t.SetupCommandLine("-Wall", "--autofix")
+ lines := t.NewLines("patch-aa",
+ RcsId,
+ "")
+
+ ChecklinesPatch(lines)
+
+ t.CheckOutputEmpty()
+}
Index: pkgsrc/pkgtools/pkglint/files/files_test.go
diff -u pkgsrc/pkgtools/pkglint/files/files_test.go:1.13 pkgsrc/pkgtools/pkglint/files/files_test.go:1.14
--- pkgsrc/pkgtools/pkglint/files/files_test.go:1.13 Sat Jan 27 18:50:36 2018
+++ pkgsrc/pkgtools/pkglint/files/files_test.go Sun Jan 28 23:21:16 2018
@@ -1,7 +1,7 @@
package main
import (
- check "gopkg.in/check.v1"
+ "gopkg.in/check.v1"
)
func (s *Suite) Test_convertToLogicalLines_no_continuation(c *check.C) {
@@ -29,6 +29,92 @@ func (s *Suite) Test_convertToLogicalLin
c.Check(lines[1].String(), equals, "fname_cont:3: third")
}
+// In Makefiles, comment lines can also have continuations.
+// See devel/bmake/files/unit-tests/comment.mk
+func (s *Suite) Test_convertToLogicalLines__comments(c *check.C) {
+ t := s.Init(c)
+
+ lines := t.SetupFileLinesContinuation("comment.mk",
+ "# This is a comment",
+ "",
+ "#\\",
+ "\tMultiline comment",
+ "# Another escaped comment \\",
+ "that \\",
+ "goes \\",
+ "on",
+ "# This is NOT an escaped comment due to the double backslashes \\\\",
+ "VAR=\tThis is not a comment",
+ "",
+ "#\\",
+ "This is a comment",
+ "#\\\\",
+ "This is no comment",
+ "#\\\\\\",
+ "This is a comment",
+ "#\\\\\\\\",
+ "This is no comment",
+ "#\\\\\\\\\\",
+ "This is a comment",
+ "#\\\\\\\\\\\\",
+ "This is no comment")
+
+ var texts []string
+ for _, line := range lines {
+ texts = append(texts, line.Text)
+ }
+
+ c.Check(texts, deepEquals, []string{
+ "# This is a comment",
+ "",
+ "# Multiline comment",
+ "# Another escaped comment that goes on",
+ "# This is NOT an escaped comment due to the double backslashes \\",
+ "VAR=\tThis is not a comment",
+ "",
+ "# This is a comment",
+ "#\\",
+ "This is no comment",
+ "#\\ This is a comment",
+ "#\\\\",
+ "This is no comment",
+ "#\\\\ This is a comment",
+ "#\\\\\\",
+ "This is no comment"})
+
+ var rawTexts []string
+ for _, line := range lines {
+ for _, rawLine := range line.raw {
+ rawTexts = append(rawTexts, rawLine.textnl)
+ }
+ }
+
+ c.Check(rawTexts, deepEquals, []string{
+ "# This is a comment\n",
+ "\n",
+ "#\\\n",
+ "\tMultiline comment\n",
+ "# Another escaped comment \\\n",
+ "that \\\n",
+ "goes \\\n",
+ "on\n",
+ "# This is NOT an escaped comment due to the double backslashes \\\\\n",
+ "VAR=\tThis is not a comment\n",
+ "\n",
+ "#\\\n",
+ "This is a comment\n",
+ "#\\\\\n",
+ "This is no comment\n",
+ "#\\\\\\\n",
+ "This is a comment\n",
+ "#\\\\\\\\\n",
+ "This is no comment\n",
+ "#\\\\\\\\\\\n",
+ "This is a comment\n",
+ "#\\\\\\\\\\\\\n",
+ "This is no comment\n"})
+}
+
func (s *Suite) Test_convertToLogicalLines_continuationInLastLine(c *check.C) {
t := s.Init(c)
Index: pkgsrc/pkgtools/pkglint/files/pkglint_test.go
diff -u pkgsrc/pkgtools/pkglint/files/pkglint_test.go:1.13 pkgsrc/pkgtools/pkglint/files/pkglint_test.go:1.14
--- pkgsrc/pkgtools/pkglint/files/pkglint_test.go:1.13 Sat Jan 27 18:50:36 2018
+++ pkgsrc/pkgtools/pkglint/files/pkglint_test.go Sun Jan 28 23:21:16 2018
@@ -186,10 +186,40 @@ func (s *Suite) Test_ChecklinesMessage__
t.CheckOutputLines(
"WARN: MESSAGE:1: Expected a line of exactly 75 \"=\" characters.",
- "ERROR: MESSAGE:2: Expected \"$"+"NetBSD$\".",
+ "ERROR: MESSAGE:1: Expected \"$"+"NetBSD$\".",
"WARN: MESSAGE:5: Expected a line of exactly 75 \"=\" characters.")
}
+func (s *Suite) Test_ChecklinesMessage__autofix(c *check.C) {
+ t := s.Init(c)
+
+ t.SetupCommandLine("-Wall", "--autofix")
+ lines := t.SetupFileLines("MESSAGE",
+ "1",
+ "2",
+ "3",
+ "4",
+ "5")
+
+ ChecklinesMessage(lines)
+
+ t.CheckOutputLines(
+ "AUTOFIX: ~/MESSAGE:1: Inserting a line \"===================================="+
+ "=======================================\" before this line.",
+ "AUTOFIX: ~/MESSAGE:1: Inserting a line \"$NetBSD: pkglint_test.go,v 1.14 2018/01/28 23:21:16 rillig Exp $\" before this line.",
+ "AUTOFIX: ~/MESSAGE:5: Inserting a line \"===================================="+
+ "=======================================\" after this line.")
+ t.CheckFileLines("MESSAGE",
+ "===========================================================================",
+ "$NetBSD: pkglint_test.go,v 1.14 2018/01/28 23:21:16 rillig Exp $",
+ "1",
+ "2",
+ "3",
+ "4",
+ "5",
+ "===========================================================================")
+}
+
func (s *Suite) Test_GlobalData_Latest(c *check.C) {
t := s.Init(c)
Index: pkgsrc/pkgtools/pkglint/files/globaldata.go
diff -u pkgsrc/pkgtools/pkglint/files/globaldata.go:1.23 pkgsrc/pkgtools/pkglint/files/globaldata.go:1.24
--- pkgsrc/pkgtools/pkglint/files/globaldata.go:1.23 Sun Jan 7 01:13:21 2018
+++ pkgsrc/pkgtools/pkglint/files/globaldata.go Sun Jan 28 23:21:16 2018
@@ -108,8 +108,8 @@ func (gd *GlobalData) loadDistSites() {
name2url := make(map[string]string)
url2name := make(map[string]string)
for _, line := range lines {
- if m, varname, _, _, _, urls, _, _ := MatchVarassign(line.Text); m {
- if hasPrefix(varname, "MASTER_SITE_") && varname != "MASTER_SITE_BACKUP" {
+ if m, commented, varname, _, _, _, urls, _, _ := MatchVarassign(line.Text); m {
+ if !commented && hasPrefix(varname, "MASTER_SITE_") && varname != "MASTER_SITE_BACKUP" {
for _, url := range splitOnSpace(urls) {
if matches(url, `^(?:http://|https://|ftp://)`) {
if name2url[varname] == "" {
@@ -187,8 +187,11 @@ func (gd *GlobalData) loadTools() {
lines := LoadExistingLines(fname, true)
for _, line := range lines {
text := line.Text
+ if hasPrefix(text, "#") {
+ continue
+ }
- if m, varname, _, _, _, value, _, _ := MatchVarassign(text); m {
+ if m, _, varname, _, _, _, value, _, _ := MatchVarassign(text); m {
if varname == "USE_TOOLS" {
if trace.Tracing {
trace.Stepf("[condDepth=%d] %s", condDepth, value)
@@ -618,7 +621,10 @@ func (tr *ToolRegistry) Trace() {
}
func (tr *ToolRegistry) ParseToolLine(line Line) {
- if m, varname, _, _, _, value, _, _ := MatchVarassign(line.Text); m {
+ if m, commented, varname, _, _, _, value, _, _ := MatchVarassign(line.Text); m {
+ if commented {
+ return
+ }
if varname == "TOOLS_CREATE" && (value == "[" || matches(value, `^?[-\w.]+$`)) {
tr.Register(value)
Index: pkgsrc/pkgtools/pkglint/files/linechecker.go
diff -u pkgsrc/pkgtools/pkglint/files/linechecker.go:1.5 pkgsrc/pkgtools/pkglint/files/linechecker.go:1.6
--- pkgsrc/pkgtools/pkglint/files/linechecker.go:1.5 Sat Jan 13 23:56:14 2018
+++ pkgsrc/pkgtools/pkglint/files/linechecker.go Sun Jan 28 23:21:16 2018
@@ -54,7 +54,7 @@ func CheckLineTrailingWhitespace(line Li
fix.Explain(
"When a line ends with some white-space, that space is in most cases",
"irrelevant and can be removed.")
- fix.ReplaceRegex(`\s+\n$`, "\n")
+ fix.ReplaceRegex(`[ \t\r]+\n$`, "\n", 1)
fix.Apply()
}
}
Index: pkgsrc/pkgtools/pkglint/files/mklinechecker_test.go
diff -u pkgsrc/pkgtools/pkglint/files/mklinechecker_test.go:1.5 pkgsrc/pkgtools/pkglint/files/mklinechecker_test.go:1.6
--- pkgsrc/pkgtools/pkglint/files/mklinechecker_test.go:1.5 Sat Jan 27 18:50:36 2018
+++ pkgsrc/pkgtools/pkglint/files/mklinechecker_test.go Sun Jan 28 23:21:16 2018
@@ -322,3 +322,34 @@ func (s *Suite) Test_MkLineChecker_Check
"WARN: Makefile:2: Unknown compiler flag \"-bs\".",
"WARN: Makefile:2: Compiler flag \"%s\\\\\\\"\" should start with a hyphen.")
}
+
+// Up to 2018-01-28, pkglint applied the autofix also to the continuation
+// lines, which is incorrect. It replaced the dot in "4.*" with spaces.
+func (s *Suite) Test_MkLineChecker_checkDirectiveIndentation_autofix(c *check.C) {
+ t := s.Init(c)
+
+ t.SetupCommandLine("-Wall", "--autofix")
+ G.globalData.InitVartypes()
+ lines := t.SetupFileLinesContinuation("options.mk",
+ MkRcsId,
+ ".if ${PKGNAME} == pkgname",
+ ".if \\",
+ " ${PLATFORM:MNetBSD-4.*}",
+ ".endif",
+ ".endif")
+ mklines := NewMkLines(lines)
+
+ mklines.Check()
+
+ t.CheckOutputLines(
+ "AUTOFIX: ~/options.mk:3: Replacing \".\" with \". \".",
+ "AUTOFIX: ~/options.mk:5: Replacing \".\" with \". \".")
+
+ t.CheckFileLines("options.mk",
+ MkRcsId,
+ ".if ${PKGNAME} == pkgname",
+ ". if \\",
+ " ${PLATFORM:MNetBSD-4.*}",
+ ". endif",
+ ".endif")
+}
Index: pkgsrc/pkgtools/pkglint/files/mkline.go
diff -u pkgsrc/pkgtools/pkglint/files/mkline.go:1.26 pkgsrc/pkgtools/pkglint/files/mkline.go:1.27
--- pkgsrc/pkgtools/pkglint/files/mkline.go:1.26 Sat Jan 27 18:50:36 2018
+++ pkgsrc/pkgtools/pkglint/files/mkline.go Sun Jan 28 23:21:16 2018
@@ -16,6 +16,7 @@ type MkLineImpl struct {
data interface{} // One of the following mkLine* types
}
type mkLineAssign struct {
+ commented bool // Whether the whole variable assignment is commented out
varname string // e.g. "HOMEPAGE", "SUBST_SED.perl"
varcanon string // e.g. "HOMEPAGE", "SUBST_SED.*"
varparam string // e.g. "", "perl"
@@ -59,7 +60,7 @@ func NewMkLine(line Line) (mkline *MkLin
"white-space.")
}
- if m, varname, spaceAfterVarname, op, valueAlign, value, spaceAfterValue, comment := MatchVarassign(text); m {
+ if m, commented, varname, spaceAfterVarname, op, valueAlign, value, spaceAfterValue, comment := MatchVarassign(text); m {
if G.opts.WarnSpace && spaceAfterVarname != "" {
switch {
case hasSuffix(varname, "+") && op == "=":
@@ -85,6 +86,7 @@ func NewMkLine(line Line) (mkline *MkLin
varparam := varnameParam(varname)
mkline.data = mkLineAssign{
+ commented,
varname,
varnameCanon(varname),
varparam,
@@ -148,13 +150,46 @@ func NewMkLine(line Line) (mkline *MkLin
func (mkline *MkLineImpl) String() string {
return fmt.Sprintf("%s:%s", mkline.Filename, mkline.Linenos())
}
-func (mkline *MkLineImpl) IsVarassign() bool { _, ok := mkline.data.(mkLineAssign); return ok }
-func (mkline *MkLineImpl) IsShellcmd() bool { _, ok := mkline.data.(mkLineShell); return ok }
-func (mkline *MkLineImpl) IsComment() bool { _, ok := mkline.data.(mkLineComment); return ok }
-func (mkline *MkLineImpl) IsEmpty() bool { _, ok := mkline.data.(mkLineEmpty); return ok }
+
+func (mkline *MkLineImpl) IsVarassign() bool {
+ data, ok := mkline.data.(mkLineAssign)
+ return ok && !data.commented
+}
+
+// IsCommentedVarassign returns true for commented-out variable assignments.
+// In most cases these are treated as ordinary comments, but in some others
+// they are treated like variable assignments, just inactive ones.
+func (mkline *MkLineImpl) IsCommentedVarassign() bool {
+ data, ok := mkline.data.(mkLineAssign)
+ return ok && data.commented
+}
+
+// IsShellcmd returns true for tab-indented lines that are assigned to a Make
+// target. Example:
+//
+// pre-configure: # IsDependency
+// ${ECHO} # IsShellcmd
+func (mkline *MkLineImpl) IsShellcmd() bool {
+ _, ok := mkline.data.(mkLineShell)
+ return ok
+}
+
+func (mkline *MkLineImpl) IsComment() bool {
+ _, ok := mkline.data.(mkLineComment)
+ return ok || mkline.IsCommentedVarassign()
+}
+
+func (mkline *MkLineImpl) IsEmpty() bool {
+ _, ok := mkline.data.(mkLineEmpty)
+ return ok
+}
// IsCond checks whether the line is a conditional (.if/.ifelse/.else/.if) or a loop (.for/.endfor).
-func (mkline *MkLineImpl) IsCond() bool { _, ok := mkline.data.(mkLineConditional); return ok }
+func (mkline *MkLineImpl) IsCond() bool {
+ _, ok := mkline.data.(mkLineConditional)
+ return ok
+}
+
func (mkline *MkLineImpl) IsInclude() bool {
incl, ok := mkline.data.(mkLineInclude)
return ok && !incl.sys
@@ -163,11 +198,15 @@ func (mkline *MkLineImpl) IsSysinclude()
incl, ok := mkline.data.(mkLineInclude)
return ok && incl.sys
}
-func (mkline *MkLineImpl) IsDependency() bool { _, ok := mkline.data.(mkLineDependency); return ok }
-func (mkline *MkLineImpl) Varname() string { return mkline.data.(mkLineAssign).varname }
-func (mkline *MkLineImpl) Varcanon() string { return mkline.data.(mkLineAssign).varcanon }
-func (mkline *MkLineImpl) Varparam() string { return mkline.data.(mkLineAssign).varparam }
-func (mkline *MkLineImpl) Op() MkOperator { return mkline.data.(mkLineAssign).op }
+func (mkline *MkLineImpl) IsDependency() bool {
+ _, ok := mkline.data.(mkLineDependency)
+ return ok
+}
+
+func (mkline *MkLineImpl) Varname() string { return mkline.data.(mkLineAssign).varname }
+func (mkline *MkLineImpl) Varcanon() string { return mkline.data.(mkLineAssign).varcanon }
+func (mkline *MkLineImpl) Varparam() string { return mkline.data.(mkLineAssign).varparam }
+func (mkline *MkLineImpl) Op() MkOperator { return mkline.data.(mkLineAssign).op }
// For a variable assignment, the text up to and including the assignment operator, e.g. VARNAME+=\t
func (mkline *MkLineImpl) ValueAlign() string { return mkline.data.(mkLineAssign).valueAlign }
@@ -356,6 +395,9 @@ func (mkline *MkLineImpl) VariableNeedsQ
if vartype.basicType.IsEnum() || vartype.IsBasicSafe() {
if vartype.kindOfList == lkNone {
+ if vartype.guessed {
+ return nqDontKnow
+ }
return nqDoesntMatter
}
if vartype.kindOfList == lkShell && !vuc.IsWordPart {
@@ -740,11 +782,16 @@ func (ind *Indentation) Varnames() strin
return varnames
}
-func MatchVarassign(text string) (m bool, varname, spaceAfterVarname, op, valueAlign, value, spaceAfterValue, comment string) {
+func MatchVarassign(text string) (m, commented bool, varname, spaceAfterVarname, op, valueAlign, value, spaceAfterValue, comment string) {
i, n := 0, len(text)
- for i < n && text[i] == ' ' {
+ if i < n && text[i] == '#' {
+ commented = true
i++
+ } else {
+ for i < n && text[i] == ' ' {
+ i++
+ }
}
varnameStart := i
Index: pkgsrc/pkgtools/pkglint/files/mkline_test.go
diff -u pkgsrc/pkgtools/pkglint/files/mkline_test.go:1.29 pkgsrc/pkgtools/pkglint/files/mkline_test.go:1.30
--- pkgsrc/pkgtools/pkglint/files/mkline_test.go:1.29 Sat Jan 27 18:50:36 2018
+++ pkgsrc/pkgtools/pkglint/files/mkline_test.go Sun Jan 28 23:21:16 2018
@@ -644,6 +644,36 @@ func (s *Suite) Test_MkLine_variableNeed
"NOTE: Makefile:3: The :Q operator isn't necessary for ${TOOLS_TAR} here.")
}
+// For some well-known directory variables like WRKDIR, PREFIX, LOCALBASE,
+// the :Q modifier can be safely removed since pkgsrc will never support
+// having special characters in these directory names.
+// For guessed variable types be cautious and don't autofix them.
+func (s *Suite) Test_MkLine_variableNeedsQuoting__only_remove_known(c *check.C) {
+ t := s.Init(c)
+
+ t.SetupCommandLine("-Wall", "--autofix")
+ G.globalData.InitVartypes()
+
+ lines := t.SetupFileLinesContinuation("Makefile",
+ MkRcsId,
+ "",
+ "demo: .PHONY",
+ "\t${ECHO} ${WRKSRC:Q}",
+ "\t${ECHO} ${FOODIR:Q}")
+ mklines := NewMkLines(lines)
+
+ mklines.Check()
+
+ t.CheckOutputLines(
+ "AUTOFIX: ~/Makefile:4: Replacing \"${WRKSRC:Q}\" with \"${WRKSRC}\".")
+ t.CheckFileLines("Makefile",
+ MkRcsId,
+ "",
+ "demo: .PHONY",
+ "\t${ECHO} ${WRKSRC}",
+ "\t${ECHO} ${FOODIR:Q}")
+}
+
func (s *Suite) Test_MkLine_Pkgmandir(c *check.C) {
t := s.Init(c)
@@ -750,38 +780,46 @@ func (s *Suite) Test_MkLine_ConditionVar
func (s *Suite) Test_MatchVarassign(c *check.C) {
s.Init(c)
- checkVarassign := func(text string, ck check.Checker, varname, spaceAfterVarname, op, align, value, spaceAfterValue, comment string) {
+ checkVarassign := func(text string, commented bool, varname, spaceAfterVarname, op, align, value, spaceAfterValue, comment string) {
type VarAssign struct {
+ commented bool
varname, spaceAfterVarname, op, align, value, spaceAfterValue, comment string
}
- expected := VarAssign{varname, spaceAfterVarname, op, align, value, spaceAfterValue, comment}
- am, avarname, aspaceAfterVarname, aop, aalign, avalue, aspaceAfterValue, acomment := MatchVarassign(text)
+ expected := VarAssign{commented, varname, spaceAfterVarname, op, align, value, spaceAfterValue, comment}
+ am, acommented, avarname, aspaceAfterVarname, aop, aalign, avalue, aspaceAfterValue, acomment := MatchVarassign(text)
if !am {
c.Errorf("Text %q doesn't match variable assignment", text)
return
}
- actual := VarAssign{avarname, aspaceAfterVarname, aop, aalign, avalue, aspaceAfterValue, acomment}
- c.Check(actual, ck, expected)
+ actual := VarAssign{acommented, avarname, aspaceAfterVarname, aop, aalign, avalue, aspaceAfterValue, acomment}
+ c.Check(actual, equals, expected)
}
checkNotVarassign := func(text string) {
- m, _, _, _, _, _, _, _ := MatchVarassign(text)
+ m, _, _, _, _, _, _, _, _ := MatchVarassign(text)
if m {
c.Errorf("Text %q matches variable assignment, but shouldn't.", text)
}
}
- checkVarassign("C++=c11", equals, "C+", "", "+=", "C++=", "c11", "", "")
- checkVarassign("V=v", equals, "V", "", "=", "V=", "v", "", "")
- checkVarassign("VAR=#comment", equals, "VAR", "", "=", "VAR=", "", "", "#comment")
- checkVarassign("VAR=\\#comment", equals, "VAR", "", "=", "VAR=", "#comment", "", "")
- checkVarassign("VAR=\\\\\\##comment", equals, "VAR", "", "=", "VAR=", "\\\\#", "", "#comment")
- checkVarassign("VAR=\\", equals, "VAR", "", "=", "VAR=", "\\", "", "")
- checkVarassign("VAR += value", equals, "VAR", " ", "+=", "VAR += ", "value", "", "")
- checkVarassign(" VAR=value", equals, "VAR", "", "=", " VAR=", "value", "", "")
- checkVarassign("VAR=value #comment", equals, "VAR", "", "=", "VAR=", "value", " ", "#comment")
+ checkVarassign("C++=c11", false, "C+", "", "+=", "C++=", "c11", "", "")
+ checkVarassign("V=v", false, "V", "", "=", "V=", "v", "", "")
+ checkVarassign("VAR=#comment", false, "VAR", "", "=", "VAR=", "", "", "#comment")
+ checkVarassign("VAR=\\#comment", false, "VAR", "", "=", "VAR=", "#comment", "", "")
+ checkVarassign("VAR=\\\\\\##comment", false, "VAR", "", "=", "VAR=", "\\\\#", "", "#comment")
+ checkVarassign("VAR=\\", false, "VAR", "", "=", "VAR=", "\\", "", "")
+ checkVarassign("VAR += value", false, "VAR", " ", "+=", "VAR += ", "value", "", "")
+ checkVarassign(" VAR=value", false, "VAR", "", "=", " VAR=", "value", "", "")
+ checkVarassign("VAR=value #comment", false, "VAR", "", "=", "VAR=", "value", " ", "#comment")
+ checkVarassign("#VAR=value", true, "VAR", "", "=", "#VAR=", "value", "", "")
+
checkNotVarassign("\tVAR=value")
checkNotVarassign("?=value")
checkNotVarassign("<=value")
+ checkNotVarassign("#")
+
+ // A single space is typically used for writing documentation,
+ // not for commenting out code.
+ checkNotVarassign("# VAR=value")
}
func (s *Suite) Test_Indentation(c *check.C) {
Index: pkgsrc/pkgtools/pkglint/files/mklinechecker.go
diff -u pkgsrc/pkgtools/pkglint/files/mklinechecker.go:1.7 pkgsrc/pkgtools/pkglint/files/mklinechecker.go:1.8
--- pkgsrc/pkgtools/pkglint/files/mklinechecker.go:1.7 Sat Jan 13 23:56:14 2018
+++ pkgsrc/pkgtools/pkglint/files/mklinechecker.go Sun Jan 28 23:21:16 2018
@@ -191,7 +191,7 @@ func (ck MkLineChecker) checkDirectiveIn
if expected := strings.Repeat(" ", expectedDepth); indent != expected {
fix := mkline.Line.Autofix()
fix.Notef("This directive should be indented by %d spaces.", expectedDepth)
- fix.Replace("."+indent, "."+expected)
+ fix.ReplaceRegex(regex.Pattern(`^\.`+indent), "."+expected, 1)
fix.Apply()
}
}
Index: pkgsrc/pkgtools/pkglint/files/mklines.go
diff -u pkgsrc/pkgtools/pkglint/files/mklines.go:1.19 pkgsrc/pkgtools/pkglint/files/mklines.go:1.20
--- pkgsrc/pkgtools/pkglint/files/mklines.go:1.19 Sat Jan 27 18:50:36 2018
+++ pkgsrc/pkgtools/pkglint/files/mklines.go Sun Jan 28 23:21:16 2018
@@ -260,28 +260,42 @@ func (va *VaralignBlock) Check(mkline Mk
case !G.opts.WarnSpace:
return
- case mkline.IsComment():
+ case mkline.IsEmpty():
+ va.Finish()
return
- case mkline.IsCond():
+ case mkline.IsCommentedVarassign():
+ break
+
+ case mkline.IsComment():
return
- case mkline.IsEmpty():
- va.Finish()
+ case mkline.IsCond():
return
case !mkline.IsVarassign():
trace.Stepf("Skipping")
va.skip = true
return
+ }
+ switch {
case mkline.Op() == opAssignEval && matches(mkline.Varname(), `^[a-z]`):
// Arguments to procedures do not take part in block alignment.
+ //
+ // Example:
+ // pkgpath := ${PKGPATH}
return
case mkline.Value() == "" && mkline.VarassignComment() == "":
// Multiple-inclusion guards usually appear in a block of
// their own and therefore do not need alignment.
+ //
+ // Example:
+ // .if !defined(INCLUSION_GUARD_MK)
+ // INCLUSION_GUARD_MK:=
+ // # ...
+ // .endif
return
}
@@ -289,7 +303,7 @@ func (va *VaralignBlock) Check(mkline Mk
if mkline.IsMultiline() {
// Interpreting the continuation marker as variable value
// is cheating, but works well.
- m, _, _, _, _, value, _, _ := MatchVarassign(mkline.raw[0].orignl)
+ m, _, _, _, _, _, value, _, _ := MatchVarassign(mkline.raw[0].orignl)
continuation = m && value == "\\"
}
@@ -330,8 +344,8 @@ func (va *VaralignBlock) Finish() {
// variable from forcing the rest of the paragraph to be indented too
// far to the right.
func (va *VaralignBlock) optimalWidth(infos []*varalignBlockInfo) int {
- longest := 0
- secondLongest := 0
+ longest := 0 // The longest seen varnameOpWidth
+ secondLongest := 0 // The second-longest seen varnameOpWidth
for _, info := range infos {
if info.continuation {
continue
Index: pkgsrc/pkgtools/pkglint/files/plist_test.go
diff -u pkgsrc/pkgtools/pkglint/files/plist_test.go:1.19 pkgsrc/pkgtools/pkglint/files/plist_test.go:1.20
--- pkgsrc/pkgtools/pkglint/files/plist_test.go:1.19 Sat Jan 27 18:50:36 2018
+++ pkgsrc/pkgtools/pkglint/files/plist_test.go Sun Jan 28 23:21:16 2018
@@ -292,3 +292,47 @@ func (s *Suite) Test_PlistChecker__autof
"@pkgdir etc/logrotate.d",
"@pkgdir etc/sasl2")
}
+
+// When the same entry appears both with and without a conditional,
+// the one with the conditional can be removed.
+// When the same entry appears with several different conditionals,
+// all of them must stay.
+func (s *Suite) Test_PlistChecker__remove_same_entries(c *check.C) {
+ t := s.Init(c)
+
+ t.SetupCommandLine("-Wall")
+ lines := t.SetupFileLines("PLIST",
+ PlistRcsId,
+ "${PLIST.option1}bin/true",
+ "bin/true",
+ "${PLIST.option1}bin/true",
+ "bin/true",
+ "${PLIST.option3}bin/false",
+ "${PLIST.option2}bin/false",
+ "bin/true")
+
+ ChecklinesPlist(lines)
+
+ t.CheckOutputLines(
+ "ERROR: ~/PLIST:2: Duplicate filename \"bin/true\", already appeared in line 3.",
+ "ERROR: ~/PLIST:4: Duplicate filename \"bin/true\", already appeared in line 3.",
+ "ERROR: ~/PLIST:5: Duplicate filename \"bin/true\", already appeared in line 3.",
+ "WARN: ~/PLIST:6: \"bin/false\" should be sorted before \"bin/true\".",
+ "ERROR: ~/PLIST:8: Duplicate filename \"bin/true\", already appeared in line 3.")
+
+ t.SetupCommandLine("-Wall", "--autofix")
+
+ ChecklinesPlist(lines)
+
+ t.CheckOutputLines(
+ "AUTOFIX: ~/PLIST:2: Deleting this line.",
+ "AUTOFIX: ~/PLIST:4: Deleting this line.",
+ "AUTOFIX: ~/PLIST:5: Deleting this line.",
+ "AUTOFIX: ~/PLIST:8: Deleting this line.",
+ "AUTOFIX: ~/PLIST:2: Sorting the whole file.")
+ t.CheckFileLines("PLIST",
+ PlistRcsId,
+ "${PLIST.option2}bin/false",
+ "${PLIST.option3}bin/false",
+ "bin/true")
+}
Index: pkgsrc/pkgtools/pkglint/files/mklines_varalign_test.go
diff -u pkgsrc/pkgtools/pkglint/files/mklines_varalign_test.go:1.1 pkgsrc/pkgtools/pkglint/files/mklines_varalign_test.go:1.2
--- pkgsrc/pkgtools/pkglint/files/mklines_varalign_test.go:1.1 Sat Jan 27 18:50:36 2018
+++ pkgsrc/pkgtools/pkglint/files/mklines_varalign_test.go Sun Jan 28 23:21:16 2018
@@ -850,8 +850,8 @@ func (s *Suite) Test_Varalign__indented_
}
// Up to 2018-01-27, it could happen that some source code was logged
-// without a corresponding diagnostic. This was unintented and confusing.
-func (s *Suite) Test_Varalign__bla(c *check.C) {
+// without a corresponding diagnostic. This was unintended and confusing.
+func (s *Suite) Test_Varalign__fix_without_diagnostic(c *check.C) {
vt := NewVaralignTester(s, c)
vt.Input(
"MESSAGE_SUBST+=\t\tRUBY_DISTNAME=${RUBY_DISTNAME}",
@@ -870,3 +870,114 @@ func (s *Suite) Test_Varalign__bla(c *ch
vt.source = true
vt.Run()
}
+
+// The two variables look like they were in two separate paragraphs, but
+// they aren't. This is because the continuation line from the DISTFILES
+// eats up the empty line that would otherwise separate the paragraphs.
+func (s *Suite) Test_Varalign__continuation_line_last_empty(c *check.C) {
+ vt := NewVaralignTester(s, c)
+ vt.Input(
+ "DISTFILES= \\",
+ "\ta \\",
+ "\tb \\",
+ "\tc \\",
+ "",
+ "NEXT_VAR=\tmust not be indented")
+ vt.Diagnostics(
+ "NOTE: ~/Makefile:1--5: This variable value should be aligned with tabs, not spaces, to column 17.")
+ vt.Autofixes(
+ "AUTOFIX: ~/Makefile:1: Replacing \" \" with \"\\t\".")
+ vt.Fixed(
+ "DISTFILES= \\",
+ " a \\",
+ " b \\",
+ " c \\",
+ "",
+ "NEXT_VAR= must not be indented")
+ vt.Run()
+}
+
+// Commented-out variables take part in the realignment.
+// The TZ=UTC below is part of the two-line comment since make(1)
+// interprets it in the same way.
+func (s *Suite) Test_Varalign__realign_commented_single_lines(c *check.C) {
+ vt := NewVaralignTester(s, c)
+ vt.Input(
+ "SHORT=\tvalue",
+ "#DISTFILES=\tdistfile-1.0.0.tar.gz",
+ "#CONTINUATION= \\",
+ "#\t\tcontinued",
+ "#CONFIGURE_ENV+= \\",
+ "#TZ=UTC",
+ "SHORT=\tvalue")
+ vt.Diagnostics(
+ "NOTE: ~/Makefile:1: This variable value should be aligned to column 17.",
+ "NOTE: ~/Makefile:3--4: This variable value should be aligned with tabs, not spaces, to column 17.",
+ "NOTE: ~/Makefile:5--6: This line should be aligned with \"\\t\\t\".",
+ "NOTE: ~/Makefile:7: This variable value should be aligned to column 17.")
+ vt.Autofixes(
+ "AUTOFIX: ~/Makefile:1: Replacing \"\\t\" with \"\\t\\t\".",
+ "AUTOFIX: ~/Makefile:3: Replacing \" \" with \"\\t\".",
+ "AUTOFIX: ~/Makefile:6: Replacing indentation \"\" with \"\\t\\t\".",
+ "AUTOFIX: ~/Makefile:7: Replacing \"\\t\" with \"\\t\\t\".")
+ vt.Fixed(
+ "SHORT= value",
+ "#DISTFILES= distfile-1.0.0.tar.gz",
+ "#CONTINUATION= \\",
+ "# continued",
+ "#CONFIGURE_ENV+= \\",
+ "# TZ=UTC",
+ "SHORT= value")
+ vt.Run()
+}
+
+func (s *Suite) Test_Varalign__realign_commented_continuation_line(c *check.C) {
+ vt := NewVaralignTester(s, c)
+ vt.Input(
+ "BEFORE=\tvalue",
+ "#COMMENTED= \\",
+ "#\tvalue1 \\",
+ "#\tvalue2 \\",
+ "#\tvalue3 \\",
+ "AFTER=\tafter") // This line continues the comment.
+ vt.Diagnostics()
+ vt.Autofixes()
+ vt.Fixed(
+ "BEFORE= value",
+ "#COMMENTED= \\",
+ "# value1 \\",
+ "# value2 \\",
+ "# value3 \\",
+ "AFTER= after")
+ vt.Run()
+}
+
+// The HOMEPAGE is completely ignored. Since its value is empty it doesn't
+// need any alignment. Whether it is commented out doesn't matter.
+func (s *Suite) Test_Varalign__realign_variable_without_value(c *check.C) {
+ vt := NewVaralignTester(s, c)
+ vt.Input(
+ "COMMENT=\t\tShort description of the package",
+ "#HOMEPAGE=")
+ vt.Diagnostics()
+ vt.Autofixes()
+ vt.Fixed(
+ "COMMENT= Short description of the package",
+ "#HOMEPAGE=")
+ vt.Run()
+}
+
+// This commented multiline variable is already perfectly aligned.
+// Nothing needs to be fixed.
+func (s *Suite) Test_Varalign__realign_commented_multiline(c *check.C) {
+ vt := NewVaralignTester(s, c)
+ vt.Input(
+ "#CONF_FILES+=\t\tfile1 \\",
+ "#\t\t\tfile2")
+ vt.Diagnostics()
+ vt.Autofixes()
+ vt.Fixed(
+ "#CONF_FILES+= file1 \\",
+ "# file2")
+ vt.Run()
+}
Index: pkgsrc/pkgtools/pkglint/files/patches.go
diff -u pkgsrc/pkgtools/pkglint/files/patches.go:1.18 pkgsrc/pkgtools/pkglint/files/patches.go:1.19
--- pkgsrc/pkgtools/pkglint/files/patches.go:1.18 Sat Jan 13 23:56:14 2018
+++ pkgsrc/pkgtools/pkglint/files/patches.go Sun Jan 28 23:21:16 2018
@@ -37,6 +37,11 @@ func (ck *PatchChecker) Check() {
if CheckLineRcsid(ck.lines[0], ``, "") {
ck.exp.Advance()
}
+ if ck.exp.EOF() {
+ ck.lines[0].Errorf("Patch files must not be empty.")
+ return
+ }
+
ck.previousLineEmpty = ck.exp.ExpectEmptyLine(G.opts.WarnSpace)
patchedFiles := 0
Index: pkgsrc/pkgtools/pkglint/files/pkglint.go
diff -u pkgsrc/pkgtools/pkglint/files/pkglint.go:1.25 pkgsrc/pkgtools/pkglint/files/pkglint.go:1.26
--- pkgsrc/pkgtools/pkglint/files/pkglint.go:1.25 Sat Jan 27 18:50:36 2018
+++ pkgsrc/pkgtools/pkglint/files/pkglint.go Sun Jan 28 23:21:16 2018
@@ -308,37 +308,45 @@ func ChecklinesMessage(lines []Line) {
defer trace.Call1(lines[0].Filename)()
}
- explainMessage := func() {
- Explain(
- "A MESSAGE file should consist of a header line, having 75 \"=\"",
- "characters, followed by a line containing only the RCS Id, then an",
- "empty line, your text and finally the footer line, which is the",
- "same as the header line.")
- }
+ explanation := []string{
+ "A MESSAGE file should consist of a header line, having 75 \"=\"",
+ "characters, followed by a line containing only the RCS Id, then an",
+ "empty line, your text and finally the footer line, which is the",
+ "same as the header line."}
if len(lines) < 3 {
lastLine := lines[len(lines)-1]
lastLine.Warnf("File too short.")
- explainMessage()
+ Explain(explanation...)
return
}
hline := strings.Repeat("=", 75)
if line := lines[0]; line.Text != hline {
- line.Warnf("Expected a line of exactly 75 \"=\" characters.")
- explainMessage()
+ fix := line.Autofix()
+ fix.Warnf("Expected a line of exactly 75 \"=\" characters.")
+ fix.Explain(explanation...)
+ fix.InsertBefore(hline)
+ fix.Apply()
+ CheckLineRcsid(lines[0], ``, "")
+ } else if 1 < len(lines) {
+ CheckLineRcsid(lines[1], ``, "")
}
- CheckLineRcsid(lines[1], ``, "")
for _, line := range lines {
CheckLineLength(line, 80)
CheckLineTrailingWhitespace(line)
CheckLineValidCharacters(line, `[\t -~]`)
}
if lastLine := lines[len(lines)-1]; lastLine.Text != hline {
- lastLine.Warnf("Expected a line of exactly 75 \"=\" characters.")
- explainMessage()
+ fix := lastLine.Autofix()
+ fix.Warnf("Expected a line of exactly 75 \"=\" characters.")
+ fix.Explain(explanation...)
+ fix.InsertAfter(hline)
+ fix.Apply()
}
ChecklinesTrailingEmptyLines(lines)
+
+ SaveAutofixChanges(lines)
}
func CheckfileMk(fname string) {
Index: pkgsrc/pkgtools/pkglint/files/plist.go
diff -u pkgsrc/pkgtools/pkglint/files/plist.go:1.20 pkgsrc/pkgtools/pkglint/files/plist.go:1.21
--- pkgsrc/pkgtools/pkglint/files/plist.go:1.20 Sat Jan 27 18:50:36 2018
+++ pkgsrc/pkgtools/pkglint/files/plist.go Sun Jan 28 23:21:16 2018
@@ -101,7 +101,7 @@ func (ck *PlistChecker) collectFilesAndD
first == '$',
'A' <= first && first <= 'Z',
'0' <= first && first <= '9':
- if ck.allFiles[text] == nil {
+ if prev := ck.allFiles[text]; prev == nil || pline.conditional < prev.conditional {
ck.allFiles[text] = pline
}
for dir := path.Dir(text); dir != "."; dir = path.Dir(dir) {
@@ -143,6 +143,7 @@ func (ck *PlistChecker) checkpath(pline
dirname := strings.TrimSuffix(sdirname, "/")
ck.checkSorted(pline)
+ ck.checkDuplicate(pline)
if contains(basename, "${IMAKE_MANNEWSUFFIX}") {
pline.warnImakeMannewsuffix()
@@ -211,17 +212,29 @@ func (ck *PlistChecker) checkSorted(plin
"The files in the PLIST should be sorted alphabetically.",
"To fix this, run \"pkglint -F PLIST\".")
}
- if prev := ck.allFiles[text]; prev != nil && prev != pline {
- fix := pline.line.Autofix()
- fix.Errorf("Duplicate filename %q, already appeared in %s.", text, prev.line.ReferenceFrom(pline.line))
- fix.Delete()
- fix.Apply()
- }
}
ck.lastFname = text
}
}
+func (ck *PlistChecker) checkDuplicate(pline *PlistLine) {
+ text := pline.text
+ if !hasAlnumPrefix(text) || containsVarRef(text) {
+ return
+ }
+
+ prev := ck.allFiles[text]
+ if prev == pline || prev.conditional != "" {
+ return
+ }
+
+ fix := pline.line.Autofix()
+ fix.Errorf("Duplicate filename %q, already appeared in %s.",
+ text, prev.line.ReferenceFrom(pline.line))
+ fix.Delete()
+ fix.Apply()
+}
+
func (ck *PlistChecker) checkpathBin(pline *PlistLine, dirname, basename string) {
if contains(dirname, "/") {
pline.line.Warnf("The bin/ directory should not have subdirectories.")
@@ -279,7 +292,8 @@ func (ck *PlistChecker) checkpathLib(pli
if contains(basename, ".a") || contains(basename, ".so") {
if m, noext := match1(pline.text, `^(.*)(?:\.a|\.so[0-9.]*)$`); m {
if laLine := ck.allFiles[noext+".la"]; laLine != nil {
- pline.line.Warnf("Redundant library found. The libtool library is in %s.", laLine.line.ReferenceFrom(pline.line))
+ pline.line.Warnf("Redundant library found. The libtool library is in %s.",
+ laLine.line.ReferenceFrom(pline.line))
}
}
}
@@ -320,7 +334,7 @@ func (ck *PlistChecker) checkpathMan(pli
"configured by the pkgsrc user. Compression and decompression takes",
"place automatically, no matter if the .gz extension is mentioned in",
"the PLIST or not.")
- fix.ReplaceRegex(`\.gz\n`, "\n")
+ fix.ReplaceRegex(`\.gz\n`, "\n", 1)
fix.Apply()
}
}
@@ -493,17 +507,6 @@ func NewPlistLineSorter(plines []*PlistL
return &plistLineSorter{header, middle, footer, unsortable, false, false}
}
-func (s *plistLineSorter) Len() int {
- return len(s.middle)
-}
-func (s *plistLineSorter) Less(i, j int) bool {
- return s.middle[i].text < s.middle[j].text
-}
-func (s *plistLineSorter) Swap(i, j int) {
- s.changed = true
- s.middle[i], s.middle[j] = s.middle[j], s.middle[i]
-}
-
func (s *plistLineSorter) Sort() {
if line := s.unsortable; line != nil {
if G.opts.PrintAutofix || G.opts.Autofix {
@@ -516,7 +519,17 @@ func (s *plistLineSorter) Sort() {
return
}
firstLine := s.middle[0].line
- sort.Stable(s)
+
+ sort.SliceStable(s.middle, func(i, j int) bool {
+ mi := s.middle[i]
+ mj := s.middle[j]
+ less := mi.text < mj.text || (mi.text == mj.text &&
+ mi.conditional < mj.conditional)
+ if (i < j) != less {
+ s.changed = true
+ }
+ return less
+ })
if !s.changed {
return
Index: pkgsrc/pkgtools/pkglint/files/histogram/histogram.go
diff -u pkgsrc/pkgtools/pkglint/files/histogram/histogram.go:1.1 pkgsrc/pkgtools/pkglint/files/histogram/histogram.go:1.2
--- pkgsrc/pkgtools/pkglint/files/histogram/histogram.go:1.1 Tue Jan 17 22:37:27 2017
+++ pkgsrc/pkgtools/pkglint/files/histogram/histogram.go Sun Jan 28 23:21:16 2018
@@ -19,6 +19,11 @@ func (h *Histogram) Add(s string, n int)
}
func (h *Histogram) PrintStats(caption string, out io.Writer, limit int) {
+ type entry struct {
+ s string
+ count int
+ }
+
entries := make([]entry, len(h.histo))
i := 0
@@ -27,7 +32,11 @@ func (h *Histogram) PrintStats(caption s
i++
}
- sort.Sort(byCountDesc(entries))
+ sort.SliceStable(entries, func(i, j int) bool {
+ ei := entries[i]
+ ej := entries[j]
+ return ej.count < ei.count || (ei.count == ej.count && ei.s < ej.s)
+ })
for i, entry := range entries {
fmt.Fprintf(out, "%s %6d %s\n", caption, entry.count, entry.s)
@@ -36,20 +45,3 @@ func (h *Histogram) PrintStats(caption s
}
}
}
-
-type entry struct {
- s string
- count int
-}
-
-type byCountDesc []entry
-
-func (a byCountDesc) Len() int {
- return len(a)
-}
-func (a byCountDesc) Swap(i, j int) {
- a[i], a[j] = a[j], a[i]
-}
-func (a byCountDesc) Less(i, j int) bool {
- return a[j].count < a[i].count || (a[i].count == a[j].count && a[i].s < a[j].s)
-}
Home |
Main Index |
Thread Index |
Old Index