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 Dec 30 16:27:14 UTC 2019
Modified Files:
pkgsrc/pkgtools/pkglint: Makefile
pkgsrc/pkgtools/pkglint/files: autofix.go autofix_test.go category.go
check_test.go linechecker.go linechecker_test.go lines.go
logging.go logging_test.go mkline_test.go mklinechecker.go
mklinechecker_test.go mklineparser.go mkvarusechecker.go
mkvarusechecker_test.go package_test.go paragraph.go plist.go
plist_test.go redundantscope_test.go shell.go util.go util_test.go
varalignblock.go varalignblock_test.go vardefs.go vartype_test.go
vartypecheck.go vartypecheck_test.go
pkgsrc/pkgtools/pkglint/files/intqa: qa.go qa_test.go
Log Message:
pkgtools/pkglint: update to 19.4.0
Changes since 19.3.19:
Empty PLIST files now generate an error instead of a warning since there
is no reason for having these empty files.
If a follow-up line in a Makefile is completely empty, there is no note
about trailing whitespace anymore since the warning about the misleading
empty line already covers this case.
The remaining code changes are only refactorings.
To generate a diff of this commit:
cvs rdiff -u -r1.620 -r1.621 pkgsrc/pkgtools/pkglint/Makefile
cvs rdiff -u -r1.35 -r1.36 pkgsrc/pkgtools/pkglint/files/autofix.go
cvs rdiff -u -r1.36 -r1.37 pkgsrc/pkgtools/pkglint/files/autofix_test.go \
pkgsrc/pkgtools/pkglint/files/logging.go
cvs rdiff -u -r1.29 -r1.30 pkgsrc/pkgtools/pkglint/files/category.go
cvs rdiff -u -r1.61 -r1.62 pkgsrc/pkgtools/pkglint/files/check_test.go
cvs rdiff -u -r1.15 -r1.16 pkgsrc/pkgtools/pkglint/files/linechecker.go
cvs rdiff -u -r1.14 -r1.15 pkgsrc/pkgtools/pkglint/files/linechecker_test.go
cvs rdiff -u -r1.11 -r1.12 pkgsrc/pkgtools/pkglint/files/lines.go \
pkgsrc/pkgtools/pkglint/files/redundantscope_test.go
cvs rdiff -u -r1.25 -r1.26 pkgsrc/pkgtools/pkglint/files/logging_test.go
cvs rdiff -u -r1.76 -r1.77 pkgsrc/pkgtools/pkglint/files/mkline_test.go
cvs rdiff -u -r1.59 -r1.60 pkgsrc/pkgtools/pkglint/files/mklinechecker.go
cvs rdiff -u -r1.54 -r1.55 \
pkgsrc/pkgtools/pkglint/files/mklinechecker_test.go
cvs rdiff -u -r1.9 -r1.10 pkgsrc/pkgtools/pkglint/files/mklineparser.go \
pkgsrc/pkgtools/pkglint/files/varalignblock_test.go
cvs rdiff -u -r1.3 -r1.4 pkgsrc/pkgtools/pkglint/files/mkvarusechecker.go \
pkgsrc/pkgtools/pkglint/files/mkvarusechecker_test.go \
pkgsrc/pkgtools/pkglint/files/paragraph.go
cvs rdiff -u -r1.65 -r1.66 pkgsrc/pkgtools/pkglint/files/package_test.go
cvs rdiff -u -r1.49 -r1.50 pkgsrc/pkgtools/pkglint/files/plist.go
cvs rdiff -u -r1.42 -r1.43 pkgsrc/pkgtools/pkglint/files/plist_test.go
cvs rdiff -u -r1.55 -r1.56 pkgsrc/pkgtools/pkglint/files/shell.go
cvs rdiff -u -r1.67 -r1.68 pkgsrc/pkgtools/pkglint/files/util.go
cvs rdiff -u -r1.44 -r1.45 pkgsrc/pkgtools/pkglint/files/util_test.go
cvs rdiff -u -r1.13 -r1.14 pkgsrc/pkgtools/pkglint/files/varalignblock.go
cvs rdiff -u -r1.84 -r1.85 pkgsrc/pkgtools/pkglint/files/vardefs.go
cvs rdiff -u -r1.24 -r1.25 pkgsrc/pkgtools/pkglint/files/vartype_test.go
cvs rdiff -u -r1.75 -r1.76 pkgsrc/pkgtools/pkglint/files/vartypecheck.go
cvs rdiff -u -r1.68 -r1.69 pkgsrc/pkgtools/pkglint/files/vartypecheck_test.go
cvs rdiff -u -r1.3 -r1.4 pkgsrc/pkgtools/pkglint/files/intqa/qa.go
cvs rdiff -u -r1.4 -r1.5 pkgsrc/pkgtools/pkglint/files/intqa/qa_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.620 pkgsrc/pkgtools/pkglint/Makefile:1.621
--- pkgsrc/pkgtools/pkglint/Makefile:1.620 Mon Dec 16 17:06:05 2019
+++ pkgsrc/pkgtools/pkglint/Makefile Mon Dec 30 16:27:13 2019
@@ -1,6 +1,6 @@
-# $NetBSD: Makefile,v 1.620 2019/12/16 17:06:05 rillig Exp $
+# $NetBSD: Makefile,v 1.621 2019/12/30 16:27:13 rillig Exp $
-PKGNAME= pkglint-19.3.19
+PKGNAME= pkglint-19.4.0
CATEGORIES= pkgtools
DISTNAME= tools
MASTER_SITES= ${MASTER_SITE_GITHUB:=golang/}
Index: pkgsrc/pkgtools/pkglint/files/autofix.go
diff -u pkgsrc/pkgtools/pkglint/files/autofix.go:1.35 pkgsrc/pkgtools/pkglint/files/autofix.go:1.36
--- pkgsrc/pkgtools/pkglint/files/autofix.go:1.35 Mon Dec 16 17:06:05 2019
+++ pkgsrc/pkgtools/pkglint/files/autofix.go Mon Dec 30 16:27:13 2019
@@ -1,7 +1,6 @@
package pkglint
import (
- "netbsd.org/pkglint/regex"
"os"
"strconv"
"strings"
@@ -117,8 +116,8 @@ func (fix *Autofix) ReplaceAfter(prefix,
}
for _, rawLine := range fix.line.raw {
- replaced := strings.Replace(rawLine.textnl, prefixFrom, prefixTo, 1)
- if replaced != rawLine.textnl {
+ ok, replaced := replaceOnce(rawLine.textnl, prefixFrom, prefixTo)
+ if ok {
if G.Logger.IsAutofix() {
rawLine.textnl = replaced
@@ -129,10 +128,7 @@ func (fix *Autofix) ReplaceAfter(prefix,
// TODO: Do this properly by parsing the whole line again,
// and ideally everything that depends on the parsed line.
// This probably requires a generic notification mechanism.
- //
- // FIXME: Only actually update fix.line.Text if the replacement
- // has been done exactly once; see ReplaceAt.
- fix.line.Text = strings.Replace(fix.line.Text, prefixFrom, prefixTo, 1)
+ _, fix.line.Text = replaceOnce(fix.line.Text, prefixFrom, prefixTo)
}
fix.Describef(rawLine.Lineno, "Replacing %q with %q.", from, to)
return
@@ -168,71 +164,11 @@ func (fix *Autofix) ReplaceAt(rawIndex i
// TODO: Do this properly by parsing the whole line again,
// and ideally everything that depends on the parsed line.
// This probably requires a generic notification mechanism.
- if strings.Count(fix.line.Text, from) == 1 {
- fix.line.Text = strings.Replace(fix.line.Text, from, to, 1)
- }
+ _, fix.line.Text = replaceOnce(fix.line.Text, from, to)
fix.Describef(rawLine.Lineno, "Replacing %q with %q.", from, to)
}
-// ReplaceRegex replaces the first howOften or all occurrences (if negative)
-// of the `from` pattern with the fixed string `toText`.
-//
-// Placeholders like `$1` are _not_ expanded in the `toText`.
-// (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) {
- fix.assertRealLine()
- if fix.skip() {
- return
- }
-
- done := 0
- for _, rawLine := range fix.line.raw {
- 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
- }
-
- replaced := replaceAllFunc(rawLine.textnl, from, replace)
- if replaced != rawLine.textnl {
- if G.Logger.IsAutofix() {
- rawLine.textnl = replaced
- }
- for _, fromText := range froms {
- fix.Describef(rawLine.Lineno, "Replacing %q with %q.", fromText, toText)
- }
- }
- }
-
- // Fix the parsed text as well.
- // This is only approximate and won't work in some edge cases
- // that involve escaped comments or replacements across line breaks.
- //
- // TODO: Do this properly by parsing the whole line again,
- // and ideally everything that depends on the parsed line.
- // This probably requires a generic notification mechanism.
- //
- // FIXME: Only actually update fix.line.Text if the replacement
- // has been done exactly once.
- done = 0
- fix.line.Text = replaceAllFunc(
- fix.line.Text,
- from,
- func(fromText string) string {
- if howOften >= 0 && done >= howOften {
- return fromText
- }
- done++
- return toText
- })
-}
-
// InsertBefore prepends a line before the current line.
// The newline is added internally.
func (fix *Autofix) InsertBefore(text string) {
@@ -241,9 +177,7 @@ func (fix *Autofix) InsertBefore(text st
return
}
- if G.Logger.IsAutofix() {
- fix.linesBefore = append(fix.linesBefore, text+"\n")
- }
+ fix.linesBefore = append(fix.linesBefore, text+"\n")
fix.Describef(fix.line.raw[0].Lineno, "Inserting a line %q before this line.", text)
}
@@ -255,9 +189,7 @@ func (fix *Autofix) InsertAfter(text str
return
}
- if G.Logger.IsAutofix() {
- fix.linesAfter = append(fix.linesAfter, text+"\n")
- }
+ fix.linesAfter = append(fix.linesAfter, text+"\n")
fix.Describef(fix.line.raw[len(fix.line.raw)-1].Lineno, "Inserting a line %q after this line.", text)
}
@@ -271,9 +203,7 @@ func (fix *Autofix) Delete() {
}
for _, line := range fix.line.raw {
- if G.Logger.IsAutofix() {
- line.textnl = ""
- }
+ line.textnl = ""
fix.Describef(line.Lineno, "Deleting this line.")
}
}
@@ -332,6 +262,12 @@ func (fix *Autofix) Describef(lineno int
// SaveAutofixChanges needs to be called. For example, this is done by
// MkLines.Check.
func (fix *Autofix) Apply() {
+ // XXX: Make the following annotations actually do something.
+ // gobco:beforeCall:!G.Opts.ShowAutofix && !G.Opts.Autofix
+ // gobco:beforeCall:G.Opts.ShowAutofix
+ // gobco:beforeCall:G.Opts.Autofix
+ // See https://github.com/rillig/gobco
+
line := fix.line
// Each autofix must have a log level and a diagnostic.
@@ -366,7 +302,7 @@ func (fix *Autofix) Apply() {
linenos := fix.affectedLinenos()
msg := sprintf(fix.diagFormat, fix.diagArgs...)
if !logFix && G.Logger.FirstTime(line.Filename, linenos, msg) {
- G.Logger.showSource(line)
+ G.Logger.writeSource(line)
}
G.Logger.Logf(fix.level, line.Filename, linenos, fix.diagFormat, msg)
}
@@ -379,7 +315,7 @@ func (fix *Autofix) Apply() {
}
G.Logger.Logf(AutofixLogLevel, line.Filename, lineno, autofixFormat, action.description)
}
- G.Logger.showSource(line)
+ G.Logger.writeSource(line)
}
if logDiagnostic && len(fix.explanation) > 0 {
Index: pkgsrc/pkgtools/pkglint/files/autofix_test.go
diff -u pkgsrc/pkgtools/pkglint/files/autofix_test.go:1.36 pkgsrc/pkgtools/pkglint/files/autofix_test.go:1.37
--- pkgsrc/pkgtools/pkglint/files/autofix_test.go:1.36 Mon Dec 16 17:06:05 2019
+++ pkgsrc/pkgtools/pkglint/files/autofix_test.go Mon Dec 30 16:27:13 2019
@@ -7,7 +7,7 @@ import (
"strings"
)
-func (s *Suite) Test_Autofix__default_leaves_line_unchanged(c *check.C) {
+func (s *Suite) Test_Autofix__default_also_updates_line(c *check.C) {
t := s.Init(c)
t.SetUpCommandLine("--source")
@@ -19,15 +19,14 @@ func (s *Suite) Test_Autofix__default_le
fix := line.Autofix()
fix.Warnf("Row should be replaced with line.")
fix.Replace("row", "line")
- fix.ReplaceRegex(`row \d+`, "the above line", -1)
fix.InsertBefore("above")
fix.InsertAfter("below")
fix.Delete()
fix.Apply()
t.CheckEquals(fix.RawText(), ""+
- "# row 1 \\\n"+
- "continuation of row 1\n")
+ "above\n"+
+ "below\n")
t.CheckOutputLines(
">\t# row 1 \\",
">\tcontinuation of row 1",
@@ -47,7 +46,6 @@ func (s *Suite) Test_Autofix__show_autof
fix := line.Autofix()
fix.Warnf("Row should be replaced with line.")
fix.ReplaceAfter("", "# row", "# line")
- fix.ReplaceRegex(`row \d+`, "the above line", -1)
fix.InsertBefore("above")
fix.InsertAfter("below")
fix.Delete()
@@ -59,7 +57,6 @@ func (s *Suite) Test_Autofix__show_autof
t.CheckOutputLines(
"WARN: ~/Makefile:1--2: Row should be replaced with line.",
"AUTOFIX: ~/Makefile:1: Replacing \"# row\" with \"# line\".",
- "AUTOFIX: ~/Makefile:2: Replacing \"row 1\" with \"the above line\".",
"AUTOFIX: ~/Makefile:1: Inserting a line \"above\" before this line.",
"AUTOFIX: ~/Makefile:2: Inserting a line \"below\" after this line.",
"AUTOFIX: ~/Makefile:1: Deleting this line.",
@@ -84,7 +81,7 @@ func (s *Suite) Test_Autofix__multiple_f
{
fix := line.Autofix()
fix.Warnf(SilentAutofixFormat)
- fix.ReplaceRegex(`(.)(.*)(.)`, "lriginao", 1) // XXX: the replacement should be "$3$2$1"
+ fix.Replace("original", "lriginao")
fix.Apply()
}
@@ -300,7 +297,7 @@ func (s *Suite) Test_Autofix__suppress_u
fix := lines.Lines[1].Autofix()
fix.Warnf("Something's wrong here.")
- fix.ReplaceRegex(`.....`, "XXX", 1)
+ fix.ReplaceAt(0, 0, "line2", "XXX")
fix.Apply()
fix.Warnf("Since XXX marks are usually not fixed, use TODO instead to draw attention.")
@@ -328,18 +325,20 @@ func (s *Suite) Test_Autofix__noop_repla
line := t.NewLine("Makefile", 14, "Original text")
fix := line.Autofix()
- fix.Warnf("All-uppercase words should not be used at all.")
- fix.ReplaceRegex(`\b[A-Z]{3,}\b`, "---censored---", -1)
+ fix.Warnf("The word ABC should not be used.")
+ fix.Replace("ABC", "---censored---")
fix.Apply()
- // This warning is wrong. This test therefore demonstrates that each
- // autofix must be properly guarded to only apply when it actually
- // does something.
+ // This warning is wrong since the actual line doesn't contain the
+ // word ABC.
+ //
+ // This test therefore demonstrates that each autofix must be properly
+ // guarded to only apply when it actually does something.
//
// As of November 2019 there is no Rollback method since it was not
// needed yet.
t.CheckOutputLines(
- "WARN: Makefile:14: All-uppercase words should not be used at all.")
+ "WARN: Makefile:14: The word ABC should not be used.")
}
func (s *Suite) Test_Autofix_Warnf__duplicate(c *check.C) {
@@ -463,7 +462,7 @@ func (s *Suite) Test_Autofix_Explain__si
"\tWhen inserting multiple lines, Apply must be called in-between.",
"\tOtherwise the changes are not described to the human reader.",
"")
- t.CheckEquals(fix.RawText(), "Text\n")
+ t.CheckEquals(fix.RawText(), "before\nText\nafter\n")
}
func (s *Suite) Test_Autofix_ReplaceAfter__autofix_in_continuation_line(c *check.C) {
@@ -540,7 +539,7 @@ func (s *Suite) Test_Autofix_ReplaceAfte
fix.Notef("Replacing text.")
fix.Replace("l", "L")
- fix.ReplaceRegex(`i`, "I", 1)
+ fix.ReplaceAt(0, 0, "i", "I")
fix.Apply()
t.CheckOutputLines(
@@ -552,6 +551,59 @@ func (s *Suite) Test_Autofix_ReplaceAfte
"AUTOFIX: filename:5: Replacing \"i\" with \"I\".")
}
+func (s *Suite) Test_Autofix_ReplaceAfter__replace_once(c *check.C) {
+ t := s.Init(c)
+
+ doTest := func(autofix bool) {
+ mklines := t.NewMkLines("filename.mk",
+ "# before ##### after")
+ mklines.ForEach(func(mkline *MkLine) {
+ fix := mkline.Autofix()
+ fix.Warnf("Warning.")
+ fix.ReplaceAfter("", "###", "replaced")
+ fix.Apply()
+ })
+ }
+
+ t.ExpectDiagnosticsAutofix(
+ doTest,
+ "WARN: filename.mk:1: Warning.")
+ // No autofix since it is not clear which of the 3 possible
+ // ### is meant.
+}
+
+func (s *Suite) Test_Autofix_ReplaceAfter__replace_once_escaped(c *check.C) {
+ t := s.Init(c)
+
+ doTest := func(autofix bool) {
+ G.Logger.Opts.ShowSource = true
+ mklines := t.NewMkLines("filename.mk",
+ "VAR=\tvalue \\#\\#\\# # comment ###")
+ mklines.ForEach(func(mkline *MkLine) {
+ fix := mkline.Autofix()
+ fix.Warnf("Warning.")
+ fix.ReplaceAfter("", "###", "replaced")
+ fix.Apply()
+ })
+ }
+
+ // This may be the wrong replacement since the part before the
+ // comment is already unescaped when most of the checks run,
+ // and the tests then try to replace the parsed text instead of
+ // the original text as it appears in the actual file.
+ //
+ // This is most probably an edge case. As soon as pkglint parses
+ // the lines into tokens containing exact positioning information,
+ // this can be easily fixed as a by-product.
+ t.ExpectDiagnosticsAutofix(
+ doTest,
+ ">\tVAR=\tvalue \\#\\#\\# # comment ###",
+ "WARN: filename.mk:1: Warning.",
+ "AUTOFIX: filename.mk:1: Replacing \"###\" with \"replaced\".",
+ "-\tVAR=\tvalue \\#\\#\\# # comment ###",
+ "+\tVAR=\tvalue \\#\\#\\# # comment replaced")
+}
+
func (s *Suite) Test_Autofix_ReplaceAt(c *check.C) {
t := s.Init(c)
@@ -638,110 +690,6 @@ func (s *Suite) Test_Autofix_ReplaceAt(c
0, 20, "?", "+=")
}
-func (s *Suite) Test_Autofix_ReplaceRegex__show_autofix(c *check.C) {
- t := s.Init(c)
-
- t.SetUpCommandLine("--show-autofix")
- lines := t.SetUpFileLines("Makefile",
- "line1",
- "line2",
- "line3")
-
- fix := lines.Lines[1].Autofix()
- fix.Warnf("Something's wrong here.")
- fix.ReplaceRegex(`.`, "X", -1)
- fix.Apply()
- SaveAutofixChanges(lines)
-
- t.CheckEquals(lines.Lines[1].raw[0].textnl, "XXXXX\n")
- t.CheckFileLines("Makefile",
- "line1",
- "line2",
- "line3")
- t.CheckOutputLines(
- "WARN: ~/Makefile:2: Something's wrong here.",
- "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__autofix(c *check.C) {
- t := s.Init(c)
-
- t.SetUpCommandLine("--autofix", "--source")
- lines := t.SetUpFileLines("Makefile",
- "line1",
- "line2",
- "line3")
-
- fix := lines.Lines[1].Autofix()
- fix.Warnf("Something's wrong here.")
- 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")
-
- // After calling fix.Apply above, the autofix is ready to be used again.
- fix.Warnf("Use Y instead of X.")
- fix.Replace("XXX", "YYY")
- fix.Apply()
-
- t.CheckOutputLines(
- "AUTOFIX: ~/Makefile:2: Replacing \"XXX\" with \"YYY\".",
- "-\tline2",
- "+\tYYYe2")
-
- SaveAutofixChanges(lines)
-
- t.CheckFileLines("Makefile",
- "line1",
- "YYYe2",
- "line3")
-}
-
-func (s *Suite) Test_Autofix_ReplaceRegex__show_autofix_and_source(c *check.C) {
- t := s.Init(c)
-
- t.SetUpCommandLine("--show-autofix", "--source")
- lines := t.SetUpFileLines("Makefile",
- "line1",
- "line2",
- "line3")
-
- fix := lines.Lines[1].Autofix()
- fix.Warnf("Something's wrong here.")
- fix.ReplaceRegex(`.`, "X", -1)
- fix.Apply()
-
- fix.Warnf("Use Y instead of X.")
- fix.Replace("XXXXX", "YYYYY")
- fix.Apply()
-
- SaveAutofixChanges(lines)
-
- t.CheckOutputLines(
- "WARN: ~/Makefile:2: Something's wrong here.",
- "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",
- "",
- "WARN: ~/Makefile:2: Use Y instead of X.",
- "AUTOFIX: ~/Makefile:2: Replacing \"XXXXX\" with \"YYYYY\".",
- "-\tline2",
- "+\tYYYYY")
-}
-
func (s *Suite) Test_Autofix_InsertBefore(c *check.C) {
t := s.Init(c)
@@ -1127,7 +1075,7 @@ func (s *Suite) Test_Autofix_Apply__text
// After fixing part of a line, the whole line needs to be parsed again.
//
// As of May 2019, this is not done yet.
-func (s *Suite) Test_Autofix_Apply__text_after_replacing_regex(c *check.C) {
+func (s *Suite) Test_Autofix_Apply__text_after_replacing(c *check.C) {
t := s.Init(c)
t.SetUpCommandLine("-Wall", "--autofix")
@@ -1135,7 +1083,7 @@ func (s *Suite) Test_Autofix_Apply__text
fix := mkline.Autofix()
fix.Notef("Just a demo.")
- fix.ReplaceRegex(`va...`, "new value", -1)
+ fix.Replace("value", "new value")
fix.Apply()
t.CheckOutputLines(
@@ -1240,7 +1188,6 @@ func (s *Suite) Test_Autofix_skip(c *che
fix.Replace("111", "___")
fix.ReplaceAfter(" ", "222", "___")
fix.ReplaceAt(0, 0, "VAR", "NEW")
- fix.ReplaceRegex(`\d+`, "___", 1)
fix.InsertBefore("before")
fix.InsertAfter("after")
fix.Delete()
@@ -1301,7 +1248,7 @@ func (s *Suite) Test_SaveAutofixChanges_
"line 1")
// As long as the file is kept open, it cannot be overwritten or deleted.
- openFile, err := os.OpenFile(t.File("subdir/file.txt").String(), 0, 0666) // TODO: replace with Path.Open
+ openFile, err := os.OpenFile(t.File("subdir/file.txt").String(), 0, 0666)
defer func() { assertNil(openFile.Close(), "") }()
c.Check(err, check.IsNil)
@@ -1355,7 +1302,8 @@ func (s *Suite) Test_SaveAutofixChanges(
fix := lines.Lines[1].Autofix()
fix.Warnf("Something's wrong here.")
- fix.ReplaceRegex(`...`, "XXX", 2)
+ fix.Replace("lin", "XXX")
+ fix.Replace("e2 ", "XXX")
fix.Apply()
SaveAutofixChanges(lines)
Index: pkgsrc/pkgtools/pkglint/files/logging.go
diff -u pkgsrc/pkgtools/pkglint/files/logging.go:1.36 pkgsrc/pkgtools/pkglint/files/logging.go:1.37
--- pkgsrc/pkgtools/pkglint/files/logging.go:1.36 Mon Dec 16 17:06:05 2019
+++ pkgsrc/pkgtools/pkglint/files/logging.go Mon Dec 30 16:27:13 2019
@@ -139,7 +139,7 @@ func (l *Logger) Diag(line *Line, level
if line != l.prevLine {
l.out.Separate()
}
- l.showSource(line)
+ l.writeSource(line)
}
l.Logf(level, filename, linenos, format, msg)
@@ -186,7 +186,7 @@ func (l *Logger) shallBeLogged(format st
return false
}
-func (l *Logger) showSource(line *Line) {
+func (l *Logger) writeSource(line *Line) {
if !G.Logger.Opts.ShowSource {
return
}
@@ -198,54 +198,60 @@ func (l *Logger) showSource(line *Line)
l.prevLine = line
}
- out := l.out
- writeLine := func(prefix, line string) {
- out.Write(prefix)
- out.Write(escapePrintable(line))
- if !hasSuffix(line, "\n") {
- out.Write("\n")
- }
- }
-
- printDiff := func(rawLines []*RawLine) {
- prefix := ">\t"
- for _, rawLine := range rawLines {
- if rawLine.textnl != rawLine.orignl {
- prefix = "\t" // Make it look like an actual diff
- }
- }
-
- for _, rawLine := range rawLines {
- if rawLine.textnl != rawLine.orignl {
- writeLine("-\t", rawLine.orignl)
- if rawLine.textnl != "" {
- writeLine("+\t", rawLine.textnl)
- }
- } else {
- writeLine(prefix, rawLine.orignl)
- }
- }
- }
-
if !l.IsAutofix() {
l.out.Separate()
}
- if line.autofix != nil {
+ if l.IsAutofix() && line.autofix != nil {
for _, before := range line.autofix.linesBefore {
- writeLine("+\t", before)
+ l.writeLine("+\t", before)
}
- printDiff(line.raw)
+ l.writeDiff(line)
for _, after := range line.autofix.linesAfter {
- writeLine("+\t", after)
+ l.writeLine("+\t", after)
}
} else {
- printDiff(line.raw)
+ l.writeDiff(line)
}
if l.IsAutofix() {
l.out.Separate()
}
}
+func (l *Logger) writeDiff(line *Line) {
+ showAsChanged := func(rawLine *RawLine) bool {
+ return l.IsAutofix() && rawLine.textnl != rawLine.orignl
+ }
+
+ rawLines := line.raw
+
+ prefix := ">\t"
+ for _, rawLine := range rawLines {
+ if showAsChanged(rawLine) {
+ prefix = "\t" // Make it look like an actual diff
+ }
+ }
+
+ for _, rawLine := range rawLines {
+ if showAsChanged(rawLine) {
+ l.writeLine("-\t", rawLine.orignl)
+ if rawLine.textnl != "" {
+ l.writeLine("+\t", rawLine.textnl)
+ }
+ } else {
+ l.writeLine(prefix, rawLine.orignl)
+ }
+ }
+}
+
+func (l *Logger) writeLine(prefix, line string) {
+ out := l.out
+ out.Write(prefix)
+ out.Write(escapePrintable(line))
+ if !hasSuffix(line, "\n") {
+ out.Write("\n")
+ }
+}
+
// IsAutofix returns whether one of the --show-autofix or --autofix options is active.
func (l *Logger) IsAutofix() bool { return l.Opts.Autofix || l.Opts.ShowAutofix }
Index: pkgsrc/pkgtools/pkglint/files/category.go
diff -u pkgsrc/pkgtools/pkglint/files/category.go:1.29 pkgsrc/pkgtools/pkglint/files/category.go:1.30
--- pkgsrc/pkgtools/pkglint/files/category.go:1.29 Sun Dec 8 22:03:38 2019
+++ pkgsrc/pkgtools/pkglint/files/category.go Mon Dec 30 16:27:13 2019
@@ -1,10 +1,6 @@
package pkglint
-import (
- "fmt"
- "netbsd.org/pkglint/textproc"
- "strings"
-)
+import "netbsd.org/pkglint/textproc"
func CheckdirCategory(dir CurrPath) {
if trace.Tracing {
@@ -26,21 +22,11 @@ func CheckdirCategory(dir CurrPath) {
if mlex.SkipIf(func(mkline *MkLine) bool { return mkline.IsVarassign() && mkline.Varname() == "COMMENT" }) {
mkline := mlex.PreviousMkLine()
- lex := textproc.NewLexer(mkline.Value())
valid := textproc.NewByteSet("--- '(),/0-9A-Za-z")
- invalid := valid.Inverse()
- var uni strings.Builder
-
- for !lex.EOF() {
- _ = lex.NextBytesSet(valid)
- ch := lex.NextByteSet(invalid)
- if ch != -1 {
- _, _ = fmt.Fprintf(&uni, " %U", ch)
- }
- }
-
- if uni.Len() > 0 {
- mkline.Warnf("%s contains invalid characters (%s).", mkline.Varname(), uni.String()[1:])
+ invalid := invalidCharacters(mkline.Value(), valid)
+ if invalid != "" {
+ mkline.Warnf("%s contains invalid characters (%s).",
+ mkline.Varname(), invalid)
}
} else {
Index: pkgsrc/pkgtools/pkglint/files/check_test.go
diff -u pkgsrc/pkgtools/pkglint/files/check_test.go:1.61 pkgsrc/pkgtools/pkglint/files/check_test.go:1.62
--- pkgsrc/pkgtools/pkglint/files/check_test.go:1.61 Mon Dec 16 17:06:05 2019
+++ pkgsrc/pkgtools/pkglint/files/check_test.go Mon Dec 30 16:27:13 2019
@@ -956,6 +956,10 @@ func (t *Tester) ExpectAssert(action fun
// ExpectDiagnosticsAutofix first runs the given action with -Wall, and
// then another time with -Wall --autofix.
+//
+// Note that to be realistic, the action must work with completely freshly
+// created objects, to prevent suppression of duplicate diagnostics and
+// changes to the text due to autofixes.
func (t *Tester) ExpectDiagnosticsAutofix(action func(autofix bool), diagnostics ...string) {
t.SetUpCommandLine("-Wall")
action(false)
Index: pkgsrc/pkgtools/pkglint/files/linechecker.go
diff -u pkgsrc/pkgtools/pkglint/files/linechecker.go:1.15 pkgsrc/pkgtools/pkglint/files/linechecker.go:1.16
--- pkgsrc/pkgtools/pkglint/files/linechecker.go:1.15 Sun Jun 30 20:56:19 2019
+++ pkgsrc/pkgtools/pkglint/files/linechecker.go Mon Dec 30 16:27:13 2019
@@ -1,7 +1,7 @@
package pkglint
import (
- "fmt"
+ "netbsd.org/pkglint/textproc"
"strings"
)
@@ -15,42 +15,43 @@ func (ck LineChecker) CheckLength(maxLen
}
prefix := ck.line.Text[0:maxLength]
- for i := 0; i < len(prefix); i++ {
- if isHspace(prefix[i]) {
- ck.line.Warnf("Line too long (should be no more than %d characters).", maxLength)
- ck.line.Explain(
- "Back in the old time, terminals with 80x25 characters were common.",
- "And this is still the default size of many terminal emulators.",
- "Moderately short lines also make reading easier.")
- return
- }
+ if !strings.ContainsAny(prefix, " \t") {
+ return
}
+
+ ck.line.Warnf("Line too long (should be no more than %d characters).",
+ maxLength)
+ ck.line.Explain(
+ "Back in the old time, terminals with 80x25 characters were common.",
+ "And this is still the default size of many terminal emulators.",
+ "Moderately short lines also make reading easier.")
}
func (ck LineChecker) CheckValidCharacters() {
- var uni strings.Builder
- for _, r := range ck.line.Text {
- if r != '\t' && !(' ' <= r && r <= '~') {
- _, _ = fmt.Fprintf(&uni, " %U", r)
- }
- }
- if uni.Len() > 0 {
- ck.line.Warnf("Line contains invalid characters (%s).", uni.String()[1:])
+ invalid := invalidCharacters(ck.line.Text, textproc.XPrint)
+ if invalid == "" {
+ return
}
+
+ ck.line.Warnf("Line contains invalid characters (%s).", invalid)
}
func (ck LineChecker) CheckTrailingWhitespace() {
- // XXX: Markdown files may need trailing whitespace. If there should ever
+ // Markdown files may need trailing whitespace. If there should ever
// be Markdown files in pkgsrc, this code has to be adjusted.
- if strings.HasSuffix(ck.line.Text, " ") || strings.HasSuffix(ck.line.Text, "\t") {
- fix := ck.line.Autofix()
- fix.Notef("Trailing whitespace.")
- fix.Explain(
- "When a line ends with some whitespace, that space is in most cases",
- "irrelevant and can be removed.")
- fix.ReplaceRegex(`[ \t\r]+\n$`, "\n", 1)
- fix.Apply()
+ rawIndex := len(ck.line.raw) - 1
+ text := ck.line.raw[rawIndex].text()
+ trimmed := rtrimHspace(text)
+ if len(trimmed) == len(text) {
+ return
}
+
+ fix := ck.line.Autofix()
+ fix.Notef("Trailing whitespace.")
+ fix.Explain(
+ "This whitespace is irrelevant and can be removed.")
+ fix.ReplaceAt(rawIndex, len(trimmed), text[len(trimmed):], "")
+ fix.Apply()
}
Index: pkgsrc/pkgtools/pkglint/files/linechecker_test.go
diff -u pkgsrc/pkgtools/pkglint/files/linechecker_test.go:1.14 pkgsrc/pkgtools/pkglint/files/linechecker_test.go:1.15
--- pkgsrc/pkgtools/pkglint/files/linechecker_test.go:1.14 Sat Jan 26 16:31:33 2019
+++ pkgsrc/pkgtools/pkglint/files/linechecker_test.go Mon Dec 30 16:27:13 2019
@@ -20,21 +20,55 @@ func (s *Suite) Test_LineChecker_CheckLe
func (s *Suite) Test_LineChecker_CheckTrailingWhitespace(c *check.C) {
t := s.Init(c)
- line := t.NewLine("Makefile", 32, "The line must go on ")
+ doTest := func(autofix bool) {
+ line := t.NewLine("Makefile", 32, "The line must go on ")
- LineChecker{line}.CheckTrailingWhitespace()
+ LineChecker{line}.CheckTrailingWhitespace()
+ }
- t.CheckOutputLines(
- "NOTE: Makefile:32: Trailing whitespace.")
+ t.ExpectDiagnosticsAutofix(
+ doTest,
+ "NOTE: Makefile:32: Trailing whitespace.",
+ "AUTOFIX: Makefile:32: Replacing \" \" with \"\".")
}
func (s *Suite) Test_LineChecker_CheckTrailingWhitespace__tab(c *check.C) {
t := s.Init(c)
- line := t.NewLine("Makefile", 32, "The line must go on\t")
+ doTest := func(autofix bool) {
+ line := t.NewLine("Makefile", 32, "The line must go on\t")
- LineChecker{line}.CheckTrailingWhitespace()
+ LineChecker{line}.CheckTrailingWhitespace()
+ }
- t.CheckOutputLines(
- "NOTE: Makefile:32: Trailing whitespace.")
+ t.ExpectDiagnosticsAutofix(
+ doTest,
+ "NOTE: Makefile:32: Trailing whitespace.",
+ "AUTOFIX: Makefile:32: Replacing \"\\t\" with \"\".")
+}
+
+// Even though the logical text of the Makefile line ends with a space,
+// the check for trailing whitespace doesn't catch it.
+//
+// The check only looks at the actual lines, not at the logical text after
+// joining the continuation lines. Line 2 is empty and thus doesn't contain
+// any whitespace that might be trailing.
+//
+// See Test_MkLineChecker_checkEmptyContinuation.
+func (s *Suite) Test_LineChecker_CheckTrailingWhitespace__multi(c *check.C) {
+ t := s.Init(c)
+
+ doTest := func(autofix bool) {
+ mklines := t.NewMkLines("Makefile",
+ MkCvsID,
+ "VAR=\tThis line \\",
+ "")
+ mkline := mklines.mklines[0]
+
+ LineChecker{mkline.Line}.CheckTrailingWhitespace()
+ }
+
+ t.ExpectDiagnosticsAutofix(
+ doTest,
+ nil...)
}
Index: pkgsrc/pkgtools/pkglint/files/lines.go
diff -u pkgsrc/pkgtools/pkglint/files/lines.go:1.11 pkgsrc/pkgtools/pkglint/files/lines.go:1.12
--- pkgsrc/pkgtools/pkglint/files/lines.go:1.11 Mon Dec 2 23:32:09 2019
+++ pkgsrc/pkgtools/pkglint/files/lines.go Mon Dec 30 16:27:13 2019
@@ -58,7 +58,7 @@ func (ls *Lines) CheckCvsID(index int, p
"",
"To preserve the history of the CVS Id, should that ever be needed,",
"remove the leading $.")
- fix.ReplaceRegex(`.*`, suggestedPrefix+"$"+"NetBSD$", 1)
+ fix.Replace(line.Text, suggestedPrefix+"$"+"NetBSD$")
fix.Apply()
}
Index: pkgsrc/pkgtools/pkglint/files/redundantscope_test.go
diff -u pkgsrc/pkgtools/pkglint/files/redundantscope_test.go:1.11 pkgsrc/pkgtools/pkglint/files/redundantscope_test.go:1.12
--- pkgsrc/pkgtools/pkglint/files/redundantscope_test.go:1.11 Fri Dec 13 01:39:23 2019
+++ pkgsrc/pkgtools/pkglint/files/redundantscope_test.go Mon Dec 30 16:27:13 2019
@@ -1427,15 +1427,17 @@ func (s *Suite) Test_RedundantScope__pro
t.CheckOutputEmpty()
}
-func (s *Suite) Test_RedundantScope__infra(c *check.C) {
+func (s *Suite) Test_RedundantScope__is_relevant_for_infrastructure(c *check.C) {
t := s.Init(c)
t.CreateFileLines("mk/bsd.options.mk",
"PKG_OPTIONS:=\t# empty",
- "PKG_OPTIONS=\t# empty")
+ "PKG_OPTIONS=\t# empty",
+ "PKG_OPTIONS=\toverwritten")
t.CreateFileLines("options.mk",
"OUTSIDE:=\t# empty",
"OUTSIDE=\t# empty",
+ "OUTSIDE=\toverwritten",
".include \"mk/bsd.options.mk\"")
test := func(diagnostics ...string) {
@@ -1457,16 +1459,22 @@ func (s *Suite) Test_RedundantScope__inf
}
test(
- "NOTE: ~/options.mk:2: " +
- "Definition of OUTSIDE is redundant because of line 1.")
+ "NOTE: ~/options.mk:2: "+
+ "Definition of OUTSIDE is redundant because of line 1.",
+ "WARN: ~/options.mk:2: "+
+ "Variable OUTSIDE is overwritten in line 3.")
t.SetUpCommandLine("-Cglobal")
test(
"NOTE: ~/options.mk:2: "+
"Definition of OUTSIDE is redundant because of line 1.",
+ "WARN: ~/options.mk:2: "+
+ "Variable OUTSIDE is overwritten in line 3.",
"NOTE: ~/mk/bsd.options.mk:2: "+
- "Definition of PKG_OPTIONS is redundant because of line 1.")
+ "Definition of PKG_OPTIONS is redundant because of line 1.",
+ "WARN: ~/mk/bsd.options.mk:2: "+
+ "Variable PKG_OPTIONS is overwritten in line 3.")
}
// Branch coverage for info.vari.IsConstant(). The other tests typically
Index: pkgsrc/pkgtools/pkglint/files/logging_test.go
diff -u pkgsrc/pkgtools/pkglint/files/logging_test.go:1.25 pkgsrc/pkgtools/pkglint/files/logging_test.go:1.26
--- pkgsrc/pkgtools/pkglint/files/logging_test.go:1.25 Mon Dec 9 20:38:16 2019
+++ pkgsrc/pkgtools/pkglint/files/logging_test.go Mon Dec 30 16:27:13 2019
@@ -317,14 +317,15 @@ func (s *Suite) Test_Logger_shallBeLogge
// to first show the code and then show the diagnostic. This allows
// the diagnostics to underline the relevant part of the source code
// and reminds of the squiggly line used for spellchecking.
-func (s *Suite) Test_Logger_showSource__separator(c *check.C) {
+func (s *Suite) Test_Logger_writeSource__separator(c *check.C) {
t := s.Init(c)
t.SetUpCommandLine("--source")
lines := t.SetUpFileLines("DESCR",
"The first line",
"The second line",
- "The third line")
+ "The third line",
+ "The fourth line")
fix := lines.Lines[1].Autofix()
fix.Warnf("Using \"second\" is deprecated.")
@@ -338,16 +339,21 @@ func (s *Suite) Test_Logger_showSource__
fix.Replace("third", "bronze medal")
fix.Apply()
+ lines.Lines[3].Warnf("No autofix, just a warning.")
+
t.CheckOutputLines(
">\tThe second line",
"WARN: ~/DESCR:2: Using \"second\" is deprecated.",
"",
">\tThe third line",
"WARN: ~/DESCR:3: Dummy warning.",
- "WARN: ~/DESCR:3: Using \"third\" is deprecated.")
+ "WARN: ~/DESCR:3: Using \"third\" is deprecated.",
+ "",
+ ">\tThe fourth line",
+ "WARN: ~/DESCR:4: No autofix, just a warning.")
}
-func (s *Suite) Test_Logger_showSource__with_explanation(c *check.C) {
+func (s *Suite) Test_Logger_writeSource__with_explanation(c *check.C) {
t := s.Init(c)
t.SetUpCommandLine("--source", "--explain")
@@ -388,7 +394,7 @@ func (s *Suite) Test_Logger_showSource__
// if there are several diagnostics for the same line. In this case though,
// there is an explanation between the diagnostics, and because it may get
// quite long, it's better to repeat the source code once again.
-func (s *Suite) Test_Logger_showSource__with_explanation_in_same_line(c *check.C) {
+func (s *Suite) Test_Logger_writeSource__with_explanation_in_same_line(c *check.C) {
t := s.Init(c)
t.SetUpCommandLine("--source", "--explain")
@@ -421,7 +427,7 @@ func (s *Suite) Test_Logger_showSource__
// When there is no explanation after the first diagnostic, it is not
// necessary to repeat the source code again for the second diagnostic.
-func (s *Suite) Test_Logger_showSource__without_explanation_in_same_line(c *check.C) {
+func (s *Suite) Test_Logger_writeSource__without_explanation_in_same_line(c *check.C) {
t := s.Init(c)
t.SetUpCommandLine("--source", "--explain")
@@ -453,14 +459,15 @@ func (s *Suite) Test_Logger_showSource__
// the "Replacing" message. Since these are shown in diff style, they
// must be kept together. And since the "+" line must be below the "Replacing"
// line, this order of lines seems to be the most intuitive.
-func (s *Suite) Test_Logger_showSource__separator_show_autofix(c *check.C) {
+func (s *Suite) Test_Logger_writeSource__separator_show_autofix(c *check.C) {
t := s.Init(c)
t.SetUpCommandLine("--source", "--show-autofix")
lines := t.SetUpFileLines("DESCR",
"The first line",
"The second line",
- "The third line")
+ "The third line",
+ "The fourth line")
fix := lines.Lines[1].Autofix()
fix.Warnf("Using \"second\" is deprecated.")
@@ -474,6 +481,8 @@ func (s *Suite) Test_Logger_showSource__
fix.Replace("third", "bronze medal")
fix.Apply()
+ lines.Lines[3].Warnf("No autofix, just a warning.")
+
t.CheckOutputLines(
"WARN: ~/DESCR:2: Using \"second\" is deprecated.",
"AUTOFIX: ~/DESCR:2: Replacing \"second\" with \"silver medal\".",
@@ -486,14 +495,15 @@ func (s *Suite) Test_Logger_showSource__
"+\tThe bronze medal line")
}
-func (s *Suite) Test_Logger_showSource__separator_show_autofix_with_explanation(c *check.C) {
+func (s *Suite) Test_Logger_writeSource__separator_show_autofix_with_explanation(c *check.C) {
t := s.Init(c)
t.SetUpCommandLine("--source", "--show-autofix", "--explain")
lines := t.SetUpFileLines("DESCR",
"The first line",
"The second line",
- "The third line")
+ "The third line",
+ "The fourth line")
fix := lines.Lines[1].Autofix()
fix.Warnf("Using \"second\" is deprecated.")
@@ -509,6 +519,8 @@ func (s *Suite) Test_Logger_showSource__
fix.Replace("third", "bronze medal")
fix.Apply()
+ lines.Lines[3].Warnf("No autofix, just a warning.")
+
t.CheckOutputLines(
"WARN: ~/DESCR:2: Using \"second\" is deprecated.",
"AUTOFIX: ~/DESCR:2: Replacing \"second\" with \"silver medal\".",
@@ -526,19 +538,41 @@ func (s *Suite) Test_Logger_showSource__
"")
}
+func (s *Suite) Test_Logger_writeSource__fatal_with_show_autofix(c *check.C) {
+ t := s.Init(c)
+
+ t.SetUpCommandLine("--source", "--show-autofix")
+ lines := t.SetUpFileLines("DESCR",
+ "The first line")
+
+ // In the unusual constellation where a fatal error occurs with both
+ // --source and --show-autofix, and the line has not had any autofix,
+ // the cited source code is shown above the diagnostic. This is
+ // different from the usual order in --show-autofix mode, which is to
+ // show the diagnostic first and then its effects.
+ //
+ // This inconsistency does not matter though since it is extremely
+ // rare.
+ t.ExpectFatal(
+ func() { lines.Lines[0].Fatalf("Fatal.") },
+ ">\tThe first line",
+ "FATAL: ~/DESCR:1: Fatal.")
+}
+
// See Test__show_source_separator_show_autofix for the ordering of the
// output lines.
//
// TODO: Giving the diagnostics again would be useful, but the warning and
// error counters should not be affected, as well as the exitcode.
-func (s *Suite) Test_Logger_showSource__separator_autofix(c *check.C) {
+func (s *Suite) Test_Logger_writeSource__separator_autofix(c *check.C) {
t := s.Init(c)
t.SetUpCommandLine("--source", "--autofix")
lines := t.SetUpFileLines("DESCR",
"The first line",
"The second line",
- "The third line")
+ "The third line",
+ "The fourth line")
fix := lines.Lines[1].Autofix()
fix.Warnf("Using \"second\" is deprecated.")
@@ -552,6 +586,8 @@ func (s *Suite) Test_Logger_showSource__
fix.Replace("third", "bronze medal")
fix.Apply()
+ lines.Lines[3].Warnf("No autofix, just a warning.")
+
t.CheckOutputLines(
"AUTOFIX: ~/DESCR:2: Replacing \"second\" with \"silver medal\".",
"-\tThe second line",
@@ -789,11 +825,12 @@ func (s *Suite) Test_Logger_Logf__duplic
fix := line.Autofix()
fix.Warnf("T should always be uppercase.")
- fix.ReplaceRegex(`t`, "T", -1)
+ fix.Replace("te", "Te")
+ fix.Replace("t", "T")
fix.Apply()
t.CheckOutputLines(
- "AUTOFIX: README.txt:123: Replacing \"t\" with \"T\".",
+ "AUTOFIX: README.txt:123: Replacing \"te\" with \"Te\".",
"AUTOFIX: README.txt:123: Replacing \"t\" with \"T\".")
}
Index: pkgsrc/pkgtools/pkglint/files/mkline_test.go
diff -u pkgsrc/pkgtools/pkglint/files/mkline_test.go:1.76 pkgsrc/pkgtools/pkglint/files/mkline_test.go:1.77
--- pkgsrc/pkgtools/pkglint/files/mkline_test.go:1.76 Mon Dec 9 20:38:16 2019
+++ pkgsrc/pkgtools/pkglint/files/mkline_test.go Mon Dec 30 16:27:13 2019
@@ -1076,6 +1076,34 @@ func (s *Suite) Test_MkLine_VariableNeed
t.CheckEquals(needsQuoting, yes)
}
+func (s *Suite) Test_MkLine_VariableNeedsQuoting__tool_in_quoted_word_part(c *check.C) {
+ t := s.Init(c)
+
+ t.SetUpVartypes()
+ t.SetUpTool("tool", "TOOL", AtRunTime)
+
+ test := func(line string, diagnostics ...string) {
+ mklines := t.SetUpFileMkLines("Makefile",
+ MkCvsID,
+ line)
+
+ mklines.Check()
+
+ t.CheckOutput(diagnostics)
+ }
+
+ test("\t: x${TOOL}",
+ "WARN: ~/Makefile:2: Please use ${TOOL:Q} instead of ${TOOL}.")
+
+ test("\t: \"x${TOOL}\"")
+
+ test("\t: 'x${TOOL}'")
+
+ test("\t: `x${TOOL}`",
+ "WARN: ~/Makefile:2: Unknown shell command \"x${TOOL}\".",
+ "WARN: ~/Makefile:2: Please use ${TOOL:Q} instead of ${TOOL}.")
+}
+
func (s *Suite) Test_MkLine_VariableNeedsQuoting__D_and_U_modifiers(c *check.C) {
t := s.Init(c)
Index: pkgsrc/pkgtools/pkglint/files/mklinechecker.go
diff -u pkgsrc/pkgtools/pkglint/files/mklinechecker.go:1.59 pkgsrc/pkgtools/pkglint/files/mklinechecker.go:1.60
--- pkgsrc/pkgtools/pkglint/files/mklinechecker.go:1.59 Mon Dec 16 17:06:05 2019
+++ pkgsrc/pkgtools/pkglint/files/mklinechecker.go Mon Dec 30 16:27:13 2019
@@ -1,7 +1,6 @@
package pkglint
import (
- "netbsd.org/pkglint/regex"
"netbsd.org/pkglint/textproc"
"strings"
)
@@ -286,7 +285,9 @@ 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.ReplaceRegex(regex.Pattern(`^\.`+indent), "."+expected, 1)
+ if hasPrefix(mkline.Line.raw[0].text(), "."+indent) {
+ fix.ReplaceAt(0, 0, "."+indent, "."+expected)
+ }
fix.Apply()
}
}
Index: pkgsrc/pkgtools/pkglint/files/mklinechecker_test.go
diff -u pkgsrc/pkgtools/pkglint/files/mklinechecker_test.go:1.54 pkgsrc/pkgtools/pkglint/files/mklinechecker_test.go:1.55
--- pkgsrc/pkgtools/pkglint/files/mklinechecker_test.go:1.54 Mon Dec 16 17:06:05 2019
+++ pkgsrc/pkgtools/pkglint/files/mklinechecker_test.go Mon Dec 30 16:27:13 2019
@@ -127,7 +127,6 @@ func (s *Suite) Test_MkLineChecker_check
mklines.Check()
t.CheckOutputLines(
- "NOTE: ~/filename.mk:2--3: Trailing whitespace.",
"WARN: ~/filename.mk:3: This line looks empty but continues the previous line.")
}
@@ -551,6 +550,66 @@ func (s *Suite) Test_MkLineChecker_check
".endif")
}
+// Having a continuation line between the dot and the directive is so
+// unusual that pkglint doesn't fix it automatically. It also doesn't panic.
+func (s *Suite) Test_MkLineChecker_checkDirectiveIndentation__multiline(c *check.C) {
+ t := s.Init(c)
+
+ t.SetUpVartypes()
+
+ t.ExpectDiagnosticsAutofix(
+ func(autofix bool) {
+ mklines := t.SetUpFileMkLines("options.mk",
+ MkCvsID,
+ ".\\",
+ "if ${MACHINE_PLATFORM:MNetBSD-4.*}",
+ ".endif")
+
+ mklines.Check()
+ },
+ "NOTE: ~/options.mk:2--3: "+
+ "This directive should be indented by 0 spaces.",
+ "WARN: ~/options.mk:2--3: "+
+ "To use MACHINE_PLATFORM at load time, "+
+ ".include \"mk/bsd.prefs.mk\" first.")
+}
+
+// Another strange edge case that doesn't occur in practice.
+func (s *Suite) Test_MkLineChecker_checkDirectiveIndentation__multiline_indented(c *check.C) {
+ t := s.Init(c)
+
+ t.SetUpVartypes()
+
+ doTest := func(autofix bool) {
+ mklines := t.SetUpFileMkLines("options.mk",
+ MkCvsID,
+ ". \\",
+ "if ${PLATFORM:MNetBSD-4.*}",
+ ".endif")
+
+ mklines.Check()
+ }
+
+ t.ExpectDiagnosticsAutofix(
+ doTest,
+ "NOTE: ~/options.mk:2: This directive should be indented by 0 spaces.",
+ "WARN: ~/options.mk:2--3: PLATFORM is used but not defined.",
+ // If the indentation should ever change here, it is probably
+ // because MkLineParser.parseDirective has been changed to
+ // behave more like bmake, which preserves a bit more of the
+ // whitespace.
+ "AUTOFIX: ~/options.mk:2: Replacing \". \" with \".\".")
+
+ // It's not really fixed since the backslash is still replaced
+ // with a single space when being parsed.
+ // At least pkglint doesn't make the situation worse than before.
+ t.CheckFileLines("options.mk",
+ MkCvsID,
+ ".\\",
+ "if ${PLATFORM:MNetBSD-4.*}",
+ ".endif")
+}
+
func (s *Suite) Test_MkLineChecker_CheckRelativePath(c *check.C) {
t := s.Init(c)
Index: pkgsrc/pkgtools/pkglint/files/mklineparser.go
diff -u pkgsrc/pkgtools/pkglint/files/mklineparser.go:1.9 pkgsrc/pkgtools/pkglint/files/mklineparser.go:1.10
--- pkgsrc/pkgtools/pkglint/files/mklineparser.go:1.9 Fri Dec 13 01:39:23 2019
+++ pkgsrc/pkgtools/pkglint/files/mklineparser.go Mon Dec 30 16:27:13 2019
@@ -100,7 +100,7 @@ func (p MkLineParser) MatchVarassign(lin
}
varnameStart := lexer.Mark()
- // TODO: duplicated code in MkParser.Varname
+ // TODO: duplicated code in MkLexer.Varname
for lexer.NextBytesSet(VarbaseBytes) != "" || lexer.NextVarUse() != nil {
}
if lexer.SkipByte('.') || hasPrefix(splitResult.main, "SITES_") {
Index: pkgsrc/pkgtools/pkglint/files/varalignblock_test.go
diff -u pkgsrc/pkgtools/pkglint/files/varalignblock_test.go:1.9 pkgsrc/pkgtools/pkglint/files/varalignblock_test.go:1.10
--- pkgsrc/pkgtools/pkglint/files/varalignblock_test.go:1.9 Mon Dec 16 17:06:05 2019
+++ pkgsrc/pkgtools/pkglint/files/varalignblock_test.go Mon Dec 30 16:27:13 2019
@@ -3109,74 +3109,6 @@ func (s *Suite) Test_varalignLine_realig
t.CheckOutputEmpty()
}
-// This example is quite unrealistic since typically the first line is
-// the least indented.
-//
-// All follow-up lines are indented with at least one tab, to make clear
-// they are continuation lines.
-func (s *Suite) Test_varalignLine_realignMultiEmptyFollow(c *check.C) {
- vt := NewVaralignTester(s, c)
- vt.Input(
- "VAR= \\",
- " value1 \\",
- " value2 \\",
- " value3 \\",
- "value4 \\",
- "\\",
- "# comment")
- vt.Internals(
- "04 05 05",
- " 08 15",
- " 10 17",
- " 06 13",
- " 00 07",
- " 00 00",
- " 00")
- vt.Diagnostics(
- "NOTE: Makefile:2: This continuation line should be indented with \"\\t\".",
- "NOTE: Makefile:3: This continuation line should be indented with \"\\t \".",
- "NOTE: Makefile:4: This continuation line should be indented with \"\\t\".",
- "NOTE: Makefile:5: This continuation line should be indented with \"\\t\".",
- "NOTE: Makefile:6: This continuation line should be indented with \"\\t\".",
- "NOTE: Makefile:7: This continuation line should be indented with \"\\t\".")
- vt.Autofixes(
- "AUTOFIX: Makefile:2: Replacing \" \" with \"\\t\".",
- "AUTOFIX: Makefile:3: Replacing \" \" with \"\\t \".",
- "AUTOFIX: Makefile:4: Replacing \" \" with \"\\t\".",
- "AUTOFIX: Makefile:5: Replacing \"\" with \"\\t\".",
- "AUTOFIX: Makefile:6: Replacing \"\" with \"\\t\".",
- "AUTOFIX: Makefile:7: Replacing \"\" with \"\\t\".")
- vt.Fixed(
- "VAR= \\",
- " value1 \\",
- " value2 \\",
- " value3 \\",
- " value4 \\",
- " \\",
- " # comment")
- vt.Run()
-}
-
-func (s *Suite) Test_varalignLine_realignMultiInitial__spaces(c *check.C) {
- vt := NewVaralignTester(s, c)
- vt.Input(
- "VAR= value1 \\",
- " value2")
- vt.Internals(
- "04 08 15",
- " 08")
- vt.Diagnostics(
- "NOTE: Makefile:1: Variable values should be aligned with tabs, not spaces.",
- "NOTE: Makefile:2: This continuation line should be indented with \"\\t\".")
- vt.Autofixes(
- "AUTOFIX: Makefile:1: Replacing \" \" with \"\\t\".",
- "AUTOFIX: Makefile:2: Replacing \" \" with \"\\t\".")
- vt.Fixed(
- "VAR= value1 \\",
- " value2")
- vt.Run()
-}
-
func (s *Suite) Test_VaralignSplitter_split(c *check.C) {
t := s.Init(c)
@@ -3402,7 +3334,7 @@ func (s *Suite) Test_VaralignSplitter_sp
func (s *Suite) Test_varalignMkLine_rightMargin(c *check.C) {
t := s.Init(c)
- test := func(common bool, margin int, lines ...string) {
+ test := func(common bool, expectedRightMargin int, lines ...string) {
t.CheckDotColumns(lines...)
mklines := t.NewMkLines("filename.mk",
lines...)
@@ -3417,7 +3349,7 @@ func (s *Suite) Test_varalignMkLine_righ
actualCommon, actualMargin := mkinfo.rightMargin()
t.CheckDeepEquals(
[]interface{}{actualCommon, actualMargin},
- []interface{}{common, margin})
+ []interface{}{common, expectedRightMargin})
}
}
@@ -3574,6 +3506,18 @@ func (s *Suite) Test_varalignMkLine_righ
"VAR....8......16..=\t\t......40......48.\t\t\t\t\\", // column 80
"\t\t\t......32......40......48......56......64..\t\\", // column 72
"\t\t\t...29")
+
+ // If the right margin lies completely off-screen (that is, beyond
+ // column 72), it counts as a right margin, but not as canonical.
+ // The preferred right margin is shifted left to column 72,
+ // to make it actually visible.
+ test(false, 72,
+ "VAR=\t\\", // column 16
+ "\tv\t\t\t\t\t\t\t\t\t\t\t\\", // column 96
+ "\tv\t\t\t\t\t\t\t\t\t\t\t\\", // column 96
+ "\tv\t\t\t\t\t\t\t\t\t\t\t\\", // column 96
+ "\tv\t\t\t\t\t\t\t\t\t\t\t\\", // column 96
+ "\tv")
}
func (s *Suite) Test_varalignLine_alignValueSingle(c *check.C) {
@@ -3585,7 +3529,7 @@ func (s *Suite) Test_varalignLine_alignV
t.CheckDotColumns(before)
mkline := t.NewMkLine("filename.mk", 123, before)
parts := NewVaralignSplitter().split(before, true)
- info := &varalignLine{mkline, 0, false, false, parts}
+ info := &varalignLine{mkline, 0, false, parts}
info.alignValueSingle(column)
@@ -3676,7 +3620,7 @@ func (s *Suite) Test_varalignLine_alignV
"AUTOFIX: filename.mk:123: Replacing \"\\t\\t\\t \\t\\t\" with \" \".")
}
-func (s *Suite) Test_varalignLine_alignValueMultiEmptyInitial(c *check.C) {
+func (s *Suite) Test_varalignLine_alignValueInitial__empty(c *check.C) {
t := s.Init(c)
mklines := t.NewMkLines("filename.mk",
@@ -3691,7 +3635,7 @@ func (s *Suite) Test_varalignLine_alignV
"NOTE: filename.mk:3: The continuation backslash should be preceded by a single space or tab.")
}
-func (s *Suite) Test_varalignLine_alignValueMultiEmptyInitial__spaces(c *check.C) {
+func (s *Suite) Test_varalignLine_alignValueInitial__empty_spaces(c *check.C) {
vt := NewVaralignTester(s, c)
vt.Input(
"VAR= \\",
@@ -3715,7 +3659,27 @@ func (s *Suite) Test_varalignLine_alignV
vt.Run()
}
-func (s *Suite) Test_varalignLine_alignValueMultiInitial(c *check.C) {
+func (s *Suite) Test_varalignLine_alignValueInitial__spaces(c *check.C) {
+ vt := NewVaralignTester(s, c)
+ vt.Input(
+ "VAR= value1 \\",
+ " value2")
+ vt.Internals(
+ "04 08 15",
+ " 08")
+ vt.Diagnostics(
+ "NOTE: Makefile:1: Variable values should be aligned with tabs, not spaces.",
+ "NOTE: Makefile:2: This continuation line should be indented with \"\\t\".")
+ vt.Autofixes(
+ "AUTOFIX: Makefile:1: Replacing \" \" with \"\\t\".",
+ "AUTOFIX: Makefile:2: Replacing \" \" with \"\\t\".")
+ vt.Fixed(
+ "VAR= value1 \\",
+ " value2")
+ vt.Run()
+}
+
+func (s *Suite) Test_varalignLine_alignValueInitial(c *check.C) {
t := s.Init(c)
test := func(before string, column int, after string, diagnostics ...string) {
@@ -3730,9 +3694,9 @@ func (s *Suite) Test_varalignLine_alignV
text := mkline.raw[0].text()
parts := NewVaralignSplitter().split(text, true)
- info := &varalignLine{mkline, 0, false, false, parts}
+ info := &varalignLine{mkline, 0, false, parts}
- info.alignValueMultiInitial(column)
+ info.alignValueInitial(column)
t.CheckEqualsf(
mkline.raw[0].text(), after,
@@ -3782,22 +3746,59 @@ func (s *Suite) Test_varalignLine_alignV
"AUTOFIX: filename.mk:1: Replacing \" \\t \" with \"\\t\\t\".")
}
-func (s *Suite) Test_varalignLine_alignValueMultiEmptyFollow(c *check.C) {
- t := s.Init(c)
-
- test := func(diagnostics ...string) {
- // FIXME
- t.CheckOutput(diagnostics)
- }
-
- test()
+// This example is quite unrealistic since typically the first line is
+// the least indented.
+//
+// All follow-up lines are indented with at least one tab, to make clear
+// they are continuation lines.
+func (s *Suite) Test_varalignLine_alignValueMultiFollow__unrealistic(c *check.C) {
+ vt := NewVaralignTester(s, c)
+ vt.Input(
+ "VAR= \\",
+ " value1 \\",
+ " value2 \\",
+ " value3 \\",
+ "value4 \\",
+ "\\",
+ "# comment")
+ vt.Internals(
+ "04 05 05",
+ " 08 15",
+ " 10 17",
+ " 06 13",
+ " 00 07",
+ " 00 00",
+ " 00")
+ vt.Diagnostics(
+ "NOTE: Makefile:2: This continuation line should be indented with \"\\t\".",
+ "NOTE: Makefile:3: This continuation line should be indented with \"\\t \".",
+ "NOTE: Makefile:4: This continuation line should be indented with \"\\t\".",
+ "NOTE: Makefile:5: This continuation line should be indented with \"\\t\".",
+ "NOTE: Makefile:6: This continuation line should be indented with \"\\t\".",
+ "NOTE: Makefile:7: This continuation line should be indented with \"\\t\".")
+ vt.Autofixes(
+ "AUTOFIX: Makefile:2: Replacing \" \" with \"\\t\".",
+ "AUTOFIX: Makefile:3: Replacing \" \" with \"\\t \".",
+ "AUTOFIX: Makefile:4: Replacing \" \" with \"\\t\".",
+ "AUTOFIX: Makefile:5: Replacing \"\" with \"\\t\".",
+ "AUTOFIX: Makefile:6: Replacing \"\" with \"\\t\".",
+ "AUTOFIX: Makefile:7: Replacing \"\" with \"\\t\".")
+ vt.Fixed(
+ "VAR= \\",
+ " value1 \\",
+ " value2 \\",
+ " value3 \\",
+ " value4 \\",
+ " \\",
+ " # comment")
+ vt.Run()
}
func (s *Suite) Test_varalignLine_alignValueMultiFollow(c *check.C) {
t := s.Init(c)
// newLine creates a line consisting of either 2 or 3 physical lines.
- // The text ends up in the raw line with index 1.
+ // The given text ends up in the raw line with index 1.
newLine := func(text string, column, indentDiff int) (*varalignLine, *RawLine) {
t.CheckDotColumns("", text)
@@ -3812,16 +3813,16 @@ func (s *Suite) Test_varalignLine_alignV
mkline := mklines.mklines[0]
parts := NewVaralignSplitter().split(text, false)
- isLong := parts.isTooLongFor(column + indentDiff)
- return &varalignLine{mkline, 1, isLong, false, parts}, mkline.raw[1]
+ return &varalignLine{mkline, 1, false, parts}, mkline.raw[1]
}
test := func(before string, column, indentDiff int, after string, diagnostics ...string) {
doTest := func(autofix bool) {
info, raw := newLine(before, column, indentDiff)
+ width := imax(column, info.valueColumn()+indentDiff)
- info.alignValueMultiFollow(column, indentDiff)
+ info.alignValueMultiFollow(width)
t.CheckEquals(raw.text(), after)
}
@@ -3856,8 +3857,18 @@ func (s *Suite) Test_varalignLine_alignV
"AUTOFIX: filename.mk:2: Replacing \"\" with \"\\t\\t\\t\".",
"AUTOFIX: filename.mk:2: Replacing \" \\\\\" with \" \\\\\".")
- // As a special case, a continuation backslash in column 72 is preserved.
- // TODO: Make this more general.
+ // The position of the continuation backslash is preserved.
+ test(
+ "value\t\t\t\t\t\t\\", 24, 0,
+ "\t\t\tvalue\t\t\t\\",
+
+ "NOTE: filename.mk:2: This continuation line should be indented "+
+ "with \"\\t\\t\\t\".",
+ "AUTOFIX: filename.mk:2: Replacing \"\" with \"\\t\\t\\t\".",
+ "AUTOFIX: filename.mk:2: "+
+ "Replacing \"\\t\\t\\t\\t\\t\\t\\\\\" "+
+ "with \"\\t\\t\\t\\\\\".")
+
test(
"value\t\t\t\t\t\t\t\t\t\\", 24, 0,
"\t\t\tvalue\t\t\t\t\t\t\\",
@@ -3879,6 +3890,69 @@ func (s *Suite) Test_varalignLine_alignV
"NOTE: filename.mk:2: This continuation line should be indented with \"\\t\\t\\t\".",
"AUTOFIX: filename.mk:2: Replacing \"\" with \"\\t\\t\\t\".",
"AUTOFIX: filename.mk:2: Replacing \"\\t\\\\\" with \" \\\\\".")
+
+ // The indentation of the continuation backslash is even preserved
+ // when it is not aligned with tabs only but with spaces.
+ test(
+ "value\t\t\t\t\t\t \\", 24, 0,
+ "\t\t\tvalue\t\t\t \\",
+
+ "NOTE: filename.mk:2: This continuation line should be indented "+
+ "with \"\\t\\t\\t\".",
+ "AUTOFIX: filename.mk:2: Replacing \"\" with \"\\t\\t\\t\".",
+ "AUTOFIX: filename.mk:2: "+
+ "Replacing \"\\t\\t\\t\\t\\t\\t \\\\\" "+
+ "with \"\\t\\t\\t \\\\\".")
+
+ // Since the value is shifted to the right, the position of the
+ // continuation backslash cannot be preserved, therefore it is
+ // shifted to the right along with the value.
+ test(
+ "value \\", 24, 0,
+ "\t\t\tvalue \\",
+
+ "NOTE: filename.mk:2: This continuation line should be indented "+
+ "with \"\\t\\t\\t\".",
+ "AUTOFIX: filename.mk:2: Replacing \"\" with \"\\t\\t\\t\".")
+
+ // The second line is indented deeper (40) than the first line (24).
+ // Assuming that the relative indentation was intended, it is preserved.
+ test(
+ "\t\t\t\t\t...45 \\", 24, 0,
+ "\t\t\t\t\t...45 \\",
+
+ nil...)
+
+ // The second line is indented deeper (35) than the first line (24).
+ // It uses tabs and spaces for indentation.
+ // Assuming that the relative indentation was intended, it is preserved.
+ test(
+ "\t\t\t\t ...40 \\", 24, 0,
+ "\t\t\t\t ...40 \\",
+
+ nil...)
+
+ // The value is missing in this line, there is only the continuation
+ // backslash. It is preserved as well.
+ test(
+ "\t\t\t\t \\", 24, 0,
+ "\t\t\t\t \\",
+
+ nil...)
+
+ // If the indentation is not in the canonical form, it is corrected.
+ test(
+ "\t \t\t\t \\", 24, 0,
+ "\t\t\t\t \\",
+
+ // The diagnostic is carefully chosen to just say how this
+ // particular line should be indented. It does not mention the rule
+ // that the indentation should be preferably tabs and up to 7 spaces.
+ //
+ // Above all, the diagnostic does not say "be indented with tabs",
+ // as that would be wrong for follow-up lines.
+ "NOTE: filename.mk:2: This continuation line should be indented with \"\\t\\t\\t\\t \".",
+ "AUTOFIX: filename.mk:2: Replacing \"\\t \\t\\t\\t \" with \"\\t\\t\\t\\t \".")
}
func (s *Suite) Test_varalignLine_alignValueMultiFollow__unindent_long_lines(c *check.C) {
@@ -3991,6 +4065,98 @@ func (s *Suite) Test_varalignLine_alignV
vt.Run()
}
+func (s *Suite) Test_varalignLine_alignValue(c *check.C) {
+ t := s.Init(c)
+
+ test := func(before string, column int, after string, diagnostics ...string) {
+ t.CheckDotColumns(before)
+
+ doTest := func(autofix bool) {
+ mklines := t.NewMkLines("filename.mk",
+ before,
+ "\t...13")
+ assert(len(mklines.mklines) == 1)
+ mkline := mklines.mklines[0]
+
+ text := mkline.raw[0].text()
+ parts := NewVaralignSplitter().split(text, true)
+ info := &varalignLine{mkline, 0, false, parts}
+
+ info.alignValue(column)
+
+ t.CheckEqualsf(
+ mkline.raw[0].text(), after,
+ "Line.raw.text, autofix=%v", autofix)
+
+ t.CheckEqualsf(info.String(), after,
+ "info.String, autofix=%v", autofix)
+ }
+
+ t.ExpectDiagnosticsAutofix(doTest, diagnostics...)
+ }
+
+ testAssert := func(before string, column int) {
+ t.CheckDotColumns(before)
+
+ doTest := func(autofix bool) {
+ mklines := t.NewMkLines("filename.mk",
+ before,
+ "\t...13")
+ assert(len(mklines.mklines) == 1)
+ mkline := mklines.mklines[0]
+
+ text := mkline.raw[0].text()
+ parts := NewVaralignSplitter().split(text, true)
+ info := &varalignLine{mkline, 0, false, parts}
+
+ t.ExpectAssert(
+ func() { info.alignValue(column) })
+
+ t.CheckEqualsf(
+ mkline.raw[0].text(), before,
+ "Line.raw.text, autofix=%v", autofix)
+
+ t.CheckEqualsf(info.String(), before,
+ "info.String, autofix=%v", autofix)
+ }
+
+ t.ExpectDiagnosticsAutofix(doTest, nil...)
+ }
+
+ // The value is already in column 8, thus nothing to do.
+ testAssert(
+ "VAR=\tvalue \\",
+ 8)
+
+ test(
+ "VAR=\tvalue \\",
+ 16,
+
+ "VAR=\t\tvalue \\",
+ "NOTE: filename.mk:1: This variable value should be aligned to column 17.",
+ "AUTOFIX: filename.mk:1: Replacing \"\\t\" with \"\\t\\t\".")
+
+ // The column is already correct,
+ // but the alignment should be done with tabs, not spaces.
+ test(
+ "VAR= \t \tvalue \\",
+ 16,
+
+ "VAR=\t\tvalue \\",
+ "NOTE: filename.mk:1: Variable values should be aligned with tabs, not spaces.",
+ "AUTOFIX: filename.mk:1: Replacing \" \\t \\t\" with \"\\t\\t\".")
+
+ // Both the column and the use of spaces in the alignment
+ // need to be fixed.
+ test(
+ "VAR= \t value \\",
+ 16,
+
+ "VAR=\t\tvalue \\",
+ "NOTE: filename.mk:1: This variable value should be aligned with tabs, not spaces, to column 17.",
+ "AUTOFIX: filename.mk:1: Replacing \" \\t \" with \"\\t\\t\".")
+}
+
// The seemingly empty line 3 is actually a continuation from the line above it.
// Its indentation is is not fixed since that would lead to trailing whitespace.
func (s *Suite) Test_varalignLine_alignFollow(c *check.C) {
@@ -4028,7 +4194,7 @@ func (s *Suite) Test_varalignLine_alignC
text := mkline.raw[rawIndex].text()
parts := NewVaralignSplitter().split(text, rawIndex == 0)
- info := &varalignLine{mkline, rawIndex, false, false, parts}
+ info := &varalignLine{mkline, rawIndex, false, parts}
info.alignContinuation(valueColumn, rightMarginColumn)
@@ -4253,6 +4419,21 @@ func (s *Suite) Test_varalignParts_space
vt.Run()
}
+func (s *Suite) Test_varalignParts_spaceBeforeContinuationIndex__assertion(c *check.C) {
+ t := s.Init(c)
+
+ test := func(value, continuation string) {
+ parts := varalignParts{value: value, continuation: continuation}
+
+ t.ExpectAssert(
+ func() { _ = parts.spaceBeforeContinuationIndex() })
+ }
+
+ test("", "")
+ test("value", "")
+ test("", "\\")
+}
+
// This test runs isCanonicalInitial directly since as of August 2019
// that function is only used in a single place, and from this place
// varnameOpSpaceWidth is always bigger than width.
Index: pkgsrc/pkgtools/pkglint/files/mkvarusechecker.go
diff -u pkgsrc/pkgtools/pkglint/files/mkvarusechecker.go:1.3 pkgsrc/pkgtools/pkglint/files/mkvarusechecker.go:1.4
--- pkgsrc/pkgtools/pkglint/files/mkvarusechecker.go:1.3 Mon Dec 9 20:38:16 2019
+++ pkgsrc/pkgtools/pkglint/files/mkvarusechecker.go Mon Dec 30 16:27:13 2019
@@ -142,7 +142,7 @@ func (ck *MkVarUseChecker) checkVarname(
if varname == "LOCALBASE" && !G.Infrastructure && time == VucRunTime {
fix := ck.MkLine.Autofix()
fix.Warnf("Please use PREFIX instead of LOCALBASE.")
- fix.ReplaceRegex(`\$\{LOCALBASE\b`, "${PREFIX", 1)
+ fix.ReplaceAfter("${", "LOCALBASE", "PREFIX")
fix.Apply()
}
}
Index: pkgsrc/pkgtools/pkglint/files/mkvarusechecker_test.go
diff -u pkgsrc/pkgtools/pkglint/files/mkvarusechecker_test.go:1.3 pkgsrc/pkgtools/pkglint/files/mkvarusechecker_test.go:1.4
--- pkgsrc/pkgtools/pkglint/files/mkvarusechecker_test.go:1.3 Mon Dec 9 20:38:16 2019
+++ pkgsrc/pkgtools/pkglint/files/mkvarusechecker_test.go Mon Dec 30 16:27:13 2019
@@ -363,16 +363,20 @@ func (s *Suite) Test_MkVarUseChecker_che
"\t: ${LOCALBASE}", // Use at run time
".endif")
- mklines.ForEach(func(mkline *MkLine) {
- mkline.ForEachUsed(func(varUse *MkVarUse, time VucTime) {
- ck := NewMkVarUseChecker(varUse, mklines, mkline)
- ck.checkVarname(time)
+ doTest := func(autofix bool) {
+ mklines.ForEach(func(mkline *MkLine) {
+ mkline.ForEachUsed(func(varUse *MkVarUse, time VucTime) {
+ ck := NewMkVarUseChecker(varUse, mklines, mkline)
+ ck.checkVarname(time)
+ })
})
- })
+ }
- t.CheckOutputLines(
+ t.ExpectDiagnosticsAutofix(
+ doTest,
"WARN: filename.mk:1: Please use \"${.TARGET}\" instead of \"$@\".",
- "WARN: filename.mk:3: Please use PREFIX instead of LOCALBASE.")
+ "WARN: filename.mk:3: Please use PREFIX instead of LOCALBASE.",
+ "AUTOFIX: filename.mk:3: Replacing \"LOCALBASE\" with \"PREFIX\".")
}
func (s *Suite) Test_MkVarUseChecker_checkPermissions(c *check.C) {
Index: pkgsrc/pkgtools/pkglint/files/paragraph.go
diff -u pkgsrc/pkgtools/pkglint/files/paragraph.go:1.3 pkgsrc/pkgtools/pkglint/files/paragraph.go:1.4
--- pkgsrc/pkgtools/pkglint/files/paragraph.go:1.3 Sun Jul 14 21:25:47 2019
+++ pkgsrc/pkgtools/pkglint/files/paragraph.go Mon Dec 30 16:27:13 2019
@@ -5,9 +5,6 @@ import "strings"
// Paragraph is a slice of Makefile lines that is surrounded by empty lines.
//
// All variable assignments in a paragraph should be aligned in the same column.
-//
-// If the paragraph adds an identifier to SUBST_CLASSES, the rest of the SUBST
-// block should be defined in the same paragraph.
type Paragraph struct {
mklines *MkLines
from int
Index: pkgsrc/pkgtools/pkglint/files/package_test.go
diff -u pkgsrc/pkgtools/pkglint/files/package_test.go:1.65 pkgsrc/pkgtools/pkglint/files/package_test.go:1.66
--- pkgsrc/pkgtools/pkglint/files/package_test.go:1.65 Fri Dec 13 01:39:23 2019
+++ pkgsrc/pkgtools/pkglint/files/package_test.go Mon Dec 30 16:27:13 2019
@@ -1547,6 +1547,23 @@ func (s *Suite) Test_Package_checkfilePa
// The redundancy check is only performed when a whole package
// is checked. Therefore nothing is reported here.
t.CheckOutputEmpty()
+
+ // If the global checks are enabled, redundancy warnings from the
+ // pkgsrc infrastructure are reported as well.
+ //
+ // This prevents the redundant PKG_OPTIONS definition from
+ // mk/bsd.options.mk to be shown when checking a normal package.
+ t.SetUpCommandLine("-Wall", "-Cglobal")
+
+ G.checkdirPackage("category/package")
+
+ t.CheckOutputLines(
+ "NOTE: category/package/../../mk/redundant.mk:3: "+
+ "Definition of INFRA_REDUNDANT is redundant because of line 2.",
+ "NOTE: category/package/redundant.mk:3: "+
+ "Definition of PKG_REDUNDANT is redundant because of line 2.",
+ "WARN: category/package/redundant.mk:2: "+
+ "PKG_REDUNDANT is defined but not used.")
}
// When a package defines PLIST_SRC, it may or may not use the
Index: pkgsrc/pkgtools/pkglint/files/plist.go
diff -u pkgsrc/pkgtools/pkglint/files/plist.go:1.49 pkgsrc/pkgtools/pkglint/files/plist.go:1.50
--- pkgsrc/pkgtools/pkglint/files/plist.go:1.49 Mon Dec 9 20:38:16 2019
+++ pkgsrc/pkgtools/pkglint/files/plist.go Mon Dec 30 16:27:13 2019
@@ -15,7 +15,7 @@ func CheckLinesPlist(pkg *Package, lines
if idOk && lines.Len() == 1 {
line := lines.Lines[0]
- line.Warnf("PLIST files shouldn't be empty.")
+ line.Errorf("PLIST files must not be empty.")
line.Explain(
"One reason for empty PLISTs is that this is a newly created package",
sprintf("and that the author didn't run %q after installing the files.", bmake("print-PLIST")),
@@ -400,7 +400,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", 1)
+ fix.ReplaceAt(0, len(pline.Text)-len(".gz"), ".gz", "")
fix.Apply()
}
}
@@ -578,7 +578,7 @@ func NewPlistLineSorter(plines []*PlistL
func (s *plistLineSorter) Sort() {
if line := s.unsortable; line != nil {
- if G.Logger.IsAutofix() && trace.Tracing {
+ if trace.Tracing {
trace.Stepf("%s: This line prevents pkglint from sorting the PLIST automatically.", line)
}
return
Index: pkgsrc/pkgtools/pkglint/files/plist_test.go
diff -u pkgsrc/pkgtools/pkglint/files/plist_test.go:1.42 pkgsrc/pkgtools/pkglint/files/plist_test.go:1.43
--- pkgsrc/pkgtools/pkglint/files/plist_test.go:1.42 Sun Dec 8 22:03:38 2019
+++ pkgsrc/pkgtools/pkglint/files/plist_test.go Mon Dec 30 16:27:13 2019
@@ -88,7 +88,7 @@ func (s *Suite) Test_CheckLinesPlist__em
CheckLinesPlist(nil, lines)
t.CheckOutputLines(
- "WARN: PLIST:1: PLIST files shouldn't be empty.")
+ "ERROR: PLIST:1: PLIST files must not be empty.")
}
func (s *Suite) Test_CheckLinesPlist__common_end(c *check.C) {
@@ -877,15 +877,21 @@ func (s *Suite) Test_PlistChecker_checkP
func (s *Suite) Test_PlistChecker_checkPathMan__gz(c *check.C) {
t := s.Init(c)
- G.Pkg = NewPackage(t.File("category/pkgbase"))
- lines := t.NewLines("PLIST",
- PlistCvsID,
- "man/man3/strerror.3.gz")
+ G.Pkg = NewPackage(t.File("category/package"))
+ t.Chdir("category/package")
- CheckLinesPlist(G.Pkg, lines)
+ doTest := func(bool) {
+ lines := t.NewLines("PLIST",
+ PlistCvsID,
+ "man/man3/strerror.3.gz")
- t.CheckOutputLines(
- "NOTE: PLIST:2: The .gz extension is unnecessary for manual pages.")
+ CheckLinesPlist(G.Pkg, lines)
+ }
+
+ t.ExpectDiagnosticsAutofix(
+ doTest,
+ "NOTE: PLIST:2: The .gz extension is unnecessary for manual pages.",
+ "AUTOFIX: PLIST:2: Replacing \".gz\" with \"\".")
}
func (s *Suite) Test_PlistChecker_checkPathShare(c *check.C) {
Index: pkgsrc/pkgtools/pkglint/files/shell.go
diff -u pkgsrc/pkgtools/pkglint/files/shell.go:1.55 pkgsrc/pkgtools/pkglint/files/shell.go:1.56
--- pkgsrc/pkgtools/pkglint/files/shell.go:1.55 Fri Dec 13 01:39:23 2019
+++ pkgsrc/pkgtools/pkglint/files/shell.go Mon Dec 30 16:27:13 2019
@@ -277,7 +277,7 @@ func (scc *SimpleCommandChecker) checkAu
}
autoMkdirs := false
- if G.Pkg != nil && prefixRel != "." {
+ if G.Pkg != nil {
plistLine := G.Pkg.Plist.Dirs[prefixRel]
if plistLine != nil && !containsVarRef(plistLine.Text) {
autoMkdirs = true
@@ -597,8 +597,7 @@ func (ck *ShellLineChecker) checkPipeExi
var shellCommandsType = NewVartype(BtShellCommands, NoVartypeOptions, NewACLEntry("*", aclpAllRuntime))
-// XXX: Why is this called shell_Word_Vuc and not shell_Commands_Vuc?
-var shellWordVuc = &VarUseContext{shellCommandsType, VucUnknownTime, VucQuotPlain, false}
+var shellCommandsVuc = &VarUseContext{shellCommandsType, VucUnknownTime, VucQuotPlain, false}
func NewShellLineChecker(mklines *MkLines, mkline *MkLine) *ShellLineChecker {
assertNotNil(mklines)
@@ -776,7 +775,8 @@ func (ck *ShellLineChecker) CheckWord(to
// to the MkLineChecker. Examples for these are ${VAR:Mpattern} or $@.
if varuse := ToVarUse(token); varuse != nil {
if ck.checkVarUse {
- NewMkVarUseChecker(varuse, ck.MkLines, ck.mkline).Check(shellWordVuc)
+ varUseChecker := NewMkVarUseChecker(varuse, ck.MkLines, ck.mkline)
+ varUseChecker.Check(shellCommandsVuc)
}
return
}
Index: pkgsrc/pkgtools/pkglint/files/util.go
diff -u pkgsrc/pkgtools/pkglint/files/util.go:1.67 pkgsrc/pkgtools/pkglint/files/util.go:1.68
--- pkgsrc/pkgtools/pkglint/files/util.go:1.67 Mon Dec 16 17:06:05 2019
+++ pkgsrc/pkgtools/pkglint/files/util.go Mon Dec 30 16:27:13 2019
@@ -159,6 +159,15 @@ func trimCommon(a, b string) (string, st
return a, b
}
+func replaceOnce(s, from, to string) (ok bool, replaced string) {
+
+ index := strings.Index(s, from)
+ if index != -1 && index == strings.LastIndex(s, from) {
+ return true, s[:index] + to + s[index+len(from):]
+ }
+ return false, s
+}
+
func isHspace(ch byte) bool {
return ch == ' ' || ch == '\t'
}
@@ -1405,4 +1414,49 @@ func (i *interval) add(x int) {
}
}
-func (i *interval) isEmpty() bool { return i.min > i.max }
+type optInt struct {
+ isSet bool
+ value int
+}
+
+func (i *optInt) get() int {
+ assert(i.isSet)
+ return i.value
+}
+
+func (i *optInt) set(value int) {
+ i.value = value
+ i.isSet = true
+}
+
+type bag struct {
+ slice []struct {
+ key interface{}
+ value int
+ }
+}
+
+func (mi *bag) sortDesc() {
+ less := func(i, j int) bool { return mi.slice[j].value < mi.slice[i].value }
+ sort.SliceStable(mi.slice, less)
+}
+
+func (mi *bag) opt(index int) int {
+ if uint(index) < uint(len(mi.slice)) {
+ return mi.slice[index].value
+ }
+ return 0
+}
+
+func (mi *bag) key(index int) interface{} {
+ return mi.slice[index].key
+}
+
+func (mi *bag) add(key interface{}, value int) {
+ mi.slice = append(mi.slice, struct {
+ key interface{}
+ value int
+ }{key, value})
+}
+
+func (mi *bag) len() int { return len(mi.slice) }
Index: pkgsrc/pkgtools/pkglint/files/util_test.go
diff -u pkgsrc/pkgtools/pkglint/files/util_test.go:1.44 pkgsrc/pkgtools/pkglint/files/util_test.go:1.45
--- pkgsrc/pkgtools/pkglint/files/util_test.go:1.44 Mon Dec 16 17:06:05 2019
+++ pkgsrc/pkgtools/pkglint/files/util_test.go Mon Dec 30 16:27:13 2019
@@ -59,6 +59,32 @@ func (s *Suite) Test_trimCommon(c *check
"a", "")
}
+func (s *Suite) Test_replaceOnce(c *check.C) {
+ t := s.Init(c)
+
+ test := func(s, from, to, result string) {
+ ok, actualResult := replaceOnce(s, from, to)
+
+ t.CheckEquals(actualResult, result)
+ t.CheckEquals(ok, result != s)
+ }
+
+ // The text does not occur at all.
+ test("something else", "from", "to", "something else")
+
+ // The text occurs exactly once.
+ test("from", "from", "to", "to")
+
+ // The text occurs at two places, non-overlapping.
+ test("from from", "from", "to", "from from")
+
+ // The text occurs at three places, non-overlapping.
+ test("aaa", "a", "b", "aaa")
+
+ // The text occurs at two places, the occurrences overlap.
+ test("aaa", "aa", "b", "aaa")
+}
+
func (s *Suite) Test_assertNil(c *check.C) {
t := s.Init(c)
Index: pkgsrc/pkgtools/pkglint/files/varalignblock.go
diff -u pkgsrc/pkgtools/pkglint/files/varalignblock.go:1.13 pkgsrc/pkgtools/pkglint/files/varalignblock.go:1.14
--- pkgsrc/pkgtools/pkglint/files/varalignblock.go:1.13 Mon Dec 16 17:06:05 2019
+++ pkgsrc/pkgtools/pkglint/files/varalignblock.go Mon Dec 30 16:27:13 2019
@@ -186,7 +186,7 @@ func (va *VaralignBlock) processVarassig
var infos []*varalignLine
for i, raw := range mkline.raw {
parts := NewVaralignSplitter().split(strings.TrimSuffix(raw.textnl, "\n"), i == 0)
- info := varalignLine{mkline, i, false, false, parts}
+ info := varalignLine{mkline, i, false, parts}
infos = append(infos, &info)
}
va.mkinfos = append(va.mkinfos, &varalignMkLine{infos})
@@ -203,8 +203,6 @@ func (va *VaralignBlock) Finish() {
}
newWidth := va.optimalWidth()
- va.adjustLong(newWidth)
-
for _, mkinfo := range va.mkinfos {
mkinfo.realign(newWidth)
}
@@ -218,19 +216,23 @@ func (l *varalignMkLine) realign(newWidt
// amount and in the same direction. Typical examples are
// SUBST_SED, shell programs and AWK programs like in
// GENERATE_PLIST.
- indentDiffSet := false
-
+ //
// The amount by which the follow-up lines are shifted.
// Positive values mean shifting to the right, negative values
// mean shifting to the left.
- indentDiff := 0
+ var indentDiff optInt
_, rightMargin := l.rightMargin()
isMultiEmpty := l.isMultiEmpty()
+
+ if info := l.infos[0]; !isMultiEmpty && info.isContinuation() {
+ indentDiff.set(newWidth - info.valueColumn())
+ }
+
for _, info := range l.infos {
if newWidth > 0 || info.rawIndex > 0 {
- info.realignDetails(newWidth, &indentDiffSet, &indentDiff, isMultiEmpty)
+ info.realignDetails(newWidth, &indentDiff, isMultiEmpty)
}
if !info.fixedSpaceBeforeContinuation {
@@ -317,70 +319,35 @@ func (va *VaralignBlock) traceWidths(min
}
}
-// adjustLong allows any follow-up line to start either in column 8 or at
-// least in column newWidth. But only if there is at least one continuation
-// line that starts in column 8 and needs the full width up to column 72.
-func (va *VaralignBlock) adjustLong(newWidth int) {
- anyLong := false
- for _, mkinfo := range va.mkinfos {
- infos := mkinfo.infos
- isMultiEmpty := mkinfo.isMultiEmpty()
- for i, info := range infos {
- if info.rawIndex == 0 {
- anyLong = false
- for _, follow := range infos[i+1:] {
- if follow.rawIndex == 0 {
- break
- }
- if !isMultiEmpty && follow.spaceBeforeValue == "\t" && follow.valueColumn() < newWidth && follow.widthAlignedAt(newWidth) > 72 {
- anyLong = true
- break
- }
+func (info *varalignLine) realignDetails(newWidth int, indentDiff *optInt, isMultiEmpty bool) {
+ switch {
+
+ case info.rawIndex == 0 && info.isContinuation():
+ info.alignValueInitial(newWidth)
+
+ case isMultiEmpty:
+ oldWidth := tabWidth(info.spaceBeforeValue)
+ if !indentDiff.isSet {
+ diff := 0
+ if newWidth != 0 {
+ diff = newWidth - oldWidth
+ if diff > 0 && !info.isCommentedOut() {
+ diff = 0
}
}
-
- info.long = anyLong && info.valueColumn() == 8
+ indentDiff.set(diff)
}
- }
-}
-func (info *varalignLine) realignDetails(newWidth int, indentDiffSet *bool, indentDiff *int, isMultiEmpty bool) {
- if isMultiEmpty {
- if info.rawIndex == 0 {
- info.alignValueMultiEmptyInitial(newWidth)
- } else {
- info.realignMultiEmptyFollow(newWidth, indentDiffSet, indentDiff)
- }
- } else if info.rawIndex == 0 && info.isContinuation() {
- info.realignMultiInitial(newWidth, indentDiffSet, indentDiff)
- } else if info.rawIndex > 0 {
- assert(*indentDiffSet)
- info.alignValueMultiFollow(newWidth, *indentDiff)
- } else {
- info.alignValueSingle(newWidth)
- }
-}
+ width := imax(oldWidth+indentDiff.get(), 8)
+ info.alignValueMultiFollow(width)
-func (info *varalignLine) realignMultiEmptyFollow(newWidth int, indentDiffSet *bool, indentDiff *int) {
- oldSpace := info.spaceBeforeValue
- oldWidth := tabWidth(oldSpace)
+ case info.rawIndex > 0:
+ width := imax(newWidth, info.valueColumn()+indentDiff.get())
+ info.alignValueMultiFollow(width)
- if !*indentDiffSet {
- *indentDiffSet = true
- *indentDiff = condInt(newWidth != 0, newWidth-oldWidth, 0)
- if *indentDiff > 0 && !info.isCommentedOut() {
- *indentDiff = 0
- }
+ default:
+ info.alignValueSingle(newWidth)
}
-
- info.alignValueMultiEmptyFollow(imax(oldWidth+*indentDiff, 8))
-}
-
-func (info *varalignLine) realignMultiInitial(newWidth int, indentDiffSet *bool, indentDiff *int) {
- *indentDiffSet = true
- *indentDiff = newWidth - info.valueColumn()
-
- info.alignValueMultiInitial(newWidth)
}
// VaralignSplitter parses the text of a raw line into those parts that
@@ -544,9 +511,6 @@ type varalignLine struct {
fixer Autofixer
rawIndex int
- // Whether the line is so long that it may use a single tab as indentation.
- long bool
-
fixedSpaceBeforeContinuation bool
varalignParts
@@ -592,61 +556,36 @@ func (info *varalignLine) alignValueSing
fix.Apply()
}
-func (info *varalignLine) alignValueMultiEmptyInitial(newWidth int) {
- leadingComment := info.leadingComment
- varnameOp := info.varnameOp
- oldSpace := info.spaceBeforeValue
-
- // Indent the outlier and any other lines that stick out
- // with a space instead of a tab to keep the line short.
- newSpace := " "
- if info.valueColumn() <= newWidth {
- newSpace = alignmentAfter(leadingComment+varnameOp, newWidth)
+func (info *varalignLine) alignValueInitial(newWidth int) {
+ if info.isEmptyContinuation() && info.valueColumn() > newWidth {
+ return
}
-
- if newSpace == oldSpace {
+ newSpace := alignmentToWidths(info.spaceBeforeValueColumn(), newWidth)
+ if newSpace == info.spaceBeforeValue {
return
}
+ info.alignValue(newWidth)
+}
- if newSpace == " " {
- return // This case is handled by checkRightMargin.
+func (info *varalignLine) alignValueMultiFollow(newWidth int) {
+ newSpace := indent(newWidth)
+ if newSpace == info.spaceBeforeValue {
+ return
}
- hasSpace := strings.IndexByte(oldSpace, ' ') != -1
- oldColumn := info.valueColumn()
- column := tabWidthSlice(leadingComment, varnameOp, newSpace)
-
- fix := info.fixer.Autofix()
- if hasSpace && column != oldColumn {
- fix.Notef("This variable value should be aligned with tabs, not spaces, to column %d.", column+1)
- } else if column != oldColumn {
- fix.Notef("This variable value should be aligned to column %d.", column+1) // TODO: to column %d instead of %d.
- } else {
- fix.Notef("Variable values should be aligned with tabs, not spaces.")
- }
- fix.ReplaceAt(info.rawIndex, info.spaceBeforeValueIndex(), oldSpace, newSpace)
- fix.Apply()
- info.spaceBeforeValue = newSpace
+ info.alignFollow(newSpace)
}
-func (info *varalignLine) alignValueMultiInitial(column int) {
- leadingComment := info.leadingComment
- varnameOp := info.varnameOp
+func (info *varalignLine) alignValue(width int) {
oldSpace := info.spaceBeforeValue
-
- newSpace := alignmentAfter(leadingComment+varnameOp, column)
- if newSpace == oldSpace {
- return
- }
-
oldWidth := info.valueColumn()
- width := tabWidthSlice(leadingComment, varnameOp, newSpace)
+ newSpace := alignmentToWidths(info.spaceBeforeValueColumn(), width)
fix := info.fixer.Autofix()
if width != oldWidth && contains(oldSpace, " ") {
fix.Notef("This variable value should be aligned with tabs, not spaces, to column %d.", width+1)
} else if width != oldWidth {
- fix.Notef("This variable value should be aligned to column %d.", width+1)
+ fix.Notef("This variable value should be aligned to column %d.", width+1) // TODO: to column %d instead of %d.
} else {
fix.Notef("Variable values should be aligned with tabs, not spaces.")
}
@@ -655,27 +594,6 @@ func (info *varalignLine) alignValueMult
info.spaceBeforeValue = newSpace
}
-func (info *varalignLine) alignValueMultiEmptyFollow(column int) {
- oldSpace := info.spaceBeforeValue
- newSpace := indent(column)
- if newSpace == oldSpace {
- return
- }
-
- info.alignFollow(newSpace)
-}
-
-func (info *varalignLine) alignValueMultiFollow(column, indentDiff int) {
- oldSpace := info.spaceBeforeValue
- newWidth := imax(column, tabWidth(oldSpace)+indentDiff)
- newSpace := indent(newWidth)
- if newSpace == oldSpace {
- return
- }
-
- info.alignFollow(newSpace)
-}
-
func (info *varalignLine) alignFollow(newSpace string) {
// Below a continuation marker, there may be a completely empty line.
// This is confusing to the human readers, but technically allowed.
@@ -721,8 +639,6 @@ func (info *varalignLine) alignContinuat
fix.Notef("The continuation backslash should be preceded by a single space or tab.")
} else if info.isTooLongFor(valueColumn) {
fix.Notef("The continuation backslash should be preceded by a single space.")
- } else if info.uptoValueWidth() >= rightMarginColumn {
- fix.Notef("The continuation backslash should be preceded by a single space or tab.")
} else {
newSpace = alignmentToWidths(info.uptoValueWidth(), rightMarginColumn)
fix.Notef(
@@ -917,25 +833,8 @@ func (p *varalignParts) isCanonicalIniti
// isCanonicalFollow returns whether the space before the value has its
// canonical form, which is at least one tab, followed by up to 7 spaces.
func (p *varalignParts) isCanonicalFollow() bool {
- lexer := textproc.NewLexer(p.spaceBeforeValue)
-
- tabs := 0
- for lexer.SkipByte('\t') {
- tabs++
- }
-
- spaces := 0
- for lexer.SkipByte(' ') {
- spaces++
- }
-
- return tabs >= 1 && spaces <= 7
-}
-
-func (p *varalignParts) widthAlignedAt(valueAlign int) int {
- return tabWidthAppend(
- valueAlign,
- p.value+p.spaceAfterValue+p.continuation)
+ column := p.valueColumn()
+ return column >= 8 && p.spaceBeforeValue == indent(column)
}
func (p *varalignParts) isTooLongFor(valueColumn int) bool {
@@ -945,39 +844,3 @@ func (p *varalignParts) isTooLongFor(val
}
return column > 73
}
-
-type bag struct {
- slice []struct {
- key interface{}
- value int
- }
-}
-
-func (mi *bag) sortDesc() {
- less := func(i, j int) bool { return mi.slice[j].value < mi.slice[i].value }
- sort.SliceStable(mi.slice, less)
-}
-
-func (mi *bag) opt(index int) int {
- if uint(index) < uint(len(mi.slice)) {
- return mi.slice[index].value
- }
- return 0
-}
-
-func (mi *bag) key(index int) interface{} {
- return mi.slice[index].key
-}
-
-func (mi *bag) add(key interface{}, value int) {
- mi.slice = append(mi.slice, struct {
- key interface{}
- value int
- }{key, value})
-}
-
-func (mi *bag) first() int { return mi.opt(0) }
-
-func (mi *bag) last() int { return mi.opt(len(mi.slice) - 1) }
-
-func (mi *bag) len() int { return len(mi.slice) }
Index: pkgsrc/pkgtools/pkglint/files/vardefs.go
diff -u pkgsrc/pkgtools/pkglint/files/vardefs.go:1.84 pkgsrc/pkgtools/pkglint/files/vardefs.go:1.85
--- pkgsrc/pkgtools/pkglint/files/vardefs.go:1.84 Sat Dec 14 18:04:16 2019
+++ pkgsrc/pkgtools/pkglint/files/vardefs.go Mon Dec 30 16:27:13 2019
@@ -1444,7 +1444,7 @@ func (reg *VarTypeRegistry) Init(src *Pk
// be set in a package Makefile.
// See VartypeCheck.Pkgrevision for details.
reg.acl("PKGREVISION", BtPkgrevision,
- PackageSettable,
+ PackageSettable|NonemptyIfDefined,
"Makefile: set")
reg.sysload("PKGSRCDIR", BtPathname, DefinedIfInScope|NonemptyIfDefined)
// This definition is only valid in the top-level Makefile,
Index: pkgsrc/pkgtools/pkglint/files/vartype_test.go
diff -u pkgsrc/pkgtools/pkglint/files/vartype_test.go:1.24 pkgsrc/pkgtools/pkglint/files/vartype_test.go:1.25
--- pkgsrc/pkgtools/pkglint/files/vartype_test.go:1.24 Fri Dec 13 01:39:23 2019
+++ pkgsrc/pkgtools/pkglint/files/vartype_test.go Mon Dec 30 16:27:13 2019
@@ -32,6 +32,23 @@ func (s *Suite) Test_ACLPermissions_Huma
"set, given a default value, appended to, used at load time, or used")
}
+func (s *Suite) Test_Vartype_IsNonemptyIfDefined(c *check.C) {
+ t := s.Init(c)
+
+ t.SetUpVartypes()
+
+ test := func(varname string, isNonempty bool) {
+ vartype := G.Pkgsrc.VariableType(nil, varname)
+
+ t.CheckEquals(vartype.IsNonemptyIfDefined(), isNonempty)
+ }
+
+ test("PKGPATH", true)
+ test("PKGREVISION", true)
+ test("OPSYS", true)
+ test("OS_VERSION", false)
+}
+
func (s *Suite) Test_Vartype_EffectivePermissions(c *check.C) {
t := s.Init(c)
Index: pkgsrc/pkgtools/pkglint/files/vartypecheck.go
diff -u pkgsrc/pkgtools/pkglint/files/vartypecheck.go:1.75 pkgsrc/pkgtools/pkglint/files/vartypecheck.go:1.76
--- pkgsrc/pkgtools/pkglint/files/vartypecheck.go:1.75 Mon Dec 16 17:06:05 2019
+++ pkgsrc/pkgtools/pkglint/files/vartypecheck.go Mon Dec 30 16:27:13 2019
@@ -1310,9 +1310,10 @@ func (cv *VartypeCheck) URL() {
} else if m, _, host, _, _ := match4(value, `^(https?|ftp|gopher)://([-0-9A-Za-z.]+)(?::(\d+))?/([-%&+,./0-9:;=?@A-Z_a-z~]|#)*$`); m {
if matches(host, `(?i)\.NetBSD\.org$`) && !matches(host, `\.NetBSD\.org$`) {
+ prefix := host[:len(host)-len(".NetBSD.org")]
fix := cv.Autofix()
fix.Warnf("Please write NetBSD.org instead of %s.", host)
- fix.ReplaceRegex(`(?i)NetBSD\.org`, "NetBSD.org", 1)
+ fix.Replace(host, prefix+".NetBSD.org")
fix.Apply()
}
Index: pkgsrc/pkgtools/pkglint/files/vartypecheck_test.go
diff -u pkgsrc/pkgtools/pkglint/files/vartypecheck_test.go:1.68 pkgsrc/pkgtools/pkglint/files/vartypecheck_test.go:1.69
--- pkgsrc/pkgtools/pkglint/files/vartypecheck_test.go:1.68 Mon Dec 16 17:06:05 2019
+++ pkgsrc/pkgtools/pkglint/files/vartypecheck_test.go Mon Dec 30 16:27:13 2019
@@ -1891,6 +1891,16 @@ func (s *Suite) Test_VartypeCheck_URL(c
vt.Values(
"gopher://bitreich.org/1/scm/geomyidae";)
vt.OutputEmpty()
+
+ G.Logger.Opts.Autofix = true
+ vt.Values(
+ "# none",
+ "${OTHER_VAR}",
+ "https://www.NetBSD.org/";,
+ "https://www.netbsd.org/";)
+ vt.Output(
+ "AUTOFIX: filename.mk:34: " +
+ "Replacing \"www.netbsd.org\" with \"www.NetBSD.org\".")
}
func (s *Suite) Test_VartypeCheck_UserGroupName(c *check.C) {
Index: pkgsrc/pkgtools/pkglint/files/intqa/qa.go
diff -u pkgsrc/pkgtools/pkglint/files/intqa/qa.go:1.3 pkgsrc/pkgtools/pkglint/files/intqa/qa.go:1.4
--- pkgsrc/pkgtools/pkglint/files/intqa/qa.go:1.3 Sun Dec 8 00:06:38 2019
+++ pkgsrc/pkgtools/pkglint/files/intqa/qa.go Mon Dec 30 16:27:14 2019
@@ -466,33 +466,16 @@ func (ck *QAChecker) print() {
}
func (ck *QAChecker) insertionSuggestion(newTest *test) string {
- // Find the testee directly above the testee of newTest.
- var before *testee
- for _, testee := range ck.testees {
- if testee.order > newTest.testee.order {
- break
- }
- if testee.testFile() != newTest.file {
- continue
- }
- if before != nil && before.order > testee.order {
- break
- }
- before = testee
- }
-
- if before == nil {
- return fmt.Sprintf("Insert it at the top of %q.", newTest.file)
- }
+ testFile := newTest.testee.testFile()
for _, test := range ck.tests {
- if test.testee == nil || test.testee.order < newTest.testee.order {
- continue
- }
- if test.file != before.testFile() {
- break
+ if test.testee != nil &&
+ test.testee.order >= newTest.testee.order &&
+ test.file == testFile {
+
+ return fmt.Sprintf("Insert it in %q, above %q.",
+ newTest.file, test.fullName())
}
- return fmt.Sprintf("Insert it in %q, above %q.", newTest.file, test.fullName())
}
return fmt.Sprintf("Insert it at the bottom of %q.", newTest.file)
Index: pkgsrc/pkgtools/pkglint/files/intqa/qa_test.go
diff -u pkgsrc/pkgtools/pkglint/files/intqa/qa_test.go:1.4 pkgsrc/pkgtools/pkglint/files/intqa/qa_test.go:1.5
--- pkgsrc/pkgtools/pkglint/files/intqa/qa_test.go:1.4 Fri Dec 13 01:39:23 2019
+++ pkgsrc/pkgtools/pkglint/files/intqa/qa_test.go Mon Dec 30 16:27:14 2019
@@ -9,6 +9,7 @@ import (
"gopkg.in/check.v1"
"io/ioutil"
"path"
+ "strings"
"testing"
)
@@ -145,10 +146,8 @@ func (s *Suite) Test_QAChecker_Check(c *
"Missing unit test \"Test_QAChecker_relate\" for \"QAChecker.relate\". "+
"Insert it in \"qa_test.go\", above \"Suite.Test_QAChecker_checkTests\".",
"Missing unit test \"Test_QAChecker_isRelevant\" for \"QAChecker.isRelevant\". "+
- "Insert it in \"qa_test.go\", above \"Suite.Test_QAChecker_errorsMask\".",
- "Missing unit test \"Test_QAChecker_insertionSuggestion\" for \"QAChecker.insertionSuggestion\". "+
- "Insert it in \"qa_test.go\", above \"Suite.Test_code_fullName\".")
- s.CheckSummary("4 errors.")
+ "Insert it in \"qa_test.go\", above \"Suite.Test_QAChecker_errorsMask\".")
+ s.CheckSummary("3 errors.")
}
func (s *Suite) Test_QAChecker_load__filtered_nothing(c *check.C) {
@@ -487,6 +486,7 @@ func (s *Suite) Test_QAChecker_checkMeth
ck.addTestee(code{"other.go", "Main", "MethodWrong", 2})
ck.addTestee(code{"main_test.go", "Main", "MethodOkTest", 3})
ck.addTestee(code{"other_test.go", "Main", "MethodWrongTest", 4})
+ ck.addTestee(code{"other_test.go", "Elsewhere", "Func", 4})
ck.addTestee(code{"main_test.go", "T", "", 100})
ck.addTestee(code{"main_test.go", "T", "MethodOk", 101})
ck.addTestee(code{"other_test.go", "T", "MethodWrong", 102})
@@ -589,6 +589,52 @@ func (s *Suite) Test_QAChecker_print__2_
s.CheckSummary("2 errors.")
}
+func (s *Suite) Test_QAChecker_insertionSuggestion(c *check.C) {
+ ck := s.Init(c)
+
+ ck.addTestee(code{"file1.go", "Type1", "", 1})
+ ck.addTestee(code{"file2.go", "", "Func4", 2})
+ ck.addTestee(code{"file2.go", "", "Func5", 3})
+ ck.addTestee(code{"file2.go", "", "Func6", 4})
+ ck.addTestee(code{"file3.go", "Type3", "Method", 5})
+ ck.addTest(code{"file2_test.go", "", "Test_Func5", 6})
+ ck.relate()
+
+ testeeFor := func(name string) *testee {
+ for _, testee := range ck.testees {
+ if testee.fullName() == name {
+ return testee
+ }
+ }
+ panic(name)
+ }
+
+ test := func(testFile, testName, msg string) {
+ testTarget := strings.SplitN(testName, "_", 2)[1]
+ testeeName := strings.Replace(testTarget, "_", ".", -1)
+ testee := testeeFor(testeeName)
+ actual := ck.insertionSuggestion(
+ &test{code{testFile, "", testName, 0}, testName, "", testee})
+ c.Check(actual, check.Equals, msg)
+ }
+
+ test(
+ "file1_test.go", "Test_Type1",
+ "Insert it at the bottom of \"file1_test.go\".")
+
+ test(
+ "file2_test.go", "Test_Func4",
+ "Insert it in \"file2_test.go\", above \"Test_Func5\".")
+
+ test(
+ "file2_test.go", "Test_Func5",
+ "Insert it in \"file2_test.go\", above \"Test_Func5\".")
+
+ test(
+ "file2_test.go", "Test_Func6",
+ "Insert it at the bottom of \"file2_test.go\".")
+}
+
func (s *Suite) Test_code_fullName(c *check.C) {
_ = s.Init(c)
Home |
Main Index |
Thread Index |
Old Index