diff options
| author | rillig <rillig@pkgsrc.org> | 2019-12-30 16:27:13 +0000 |
|---|---|---|
| committer | rillig <rillig@pkgsrc.org> | 2019-12-30 16:27:13 +0000 |
| commit | 8016e18500458b1cac3587ed42638011c12cf499 (patch) | |
| tree | 7409933f2a6c90eab3c1d3174bf1eca73d019ddd /pkgtools | |
| parent | 6fe3e88ada46e4fa0abd8a04e5387084e7ca1a4a (diff) | |
| download | pkgsrc-8016e18500458b1cac3587ed42638011c12cf499.tar.gz | |
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.
Diffstat (limited to 'pkgtools')
32 files changed, 907 insertions, 654 deletions
diff --git a/pkgtools/pkglint/Makefile b/pkgtools/pkglint/Makefile index 80cb5a9dbc4..b72868caecd 100644 --- a/pkgtools/pkglint/Makefile +++ b/pkgtools/pkglint/Makefile @@ -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/} diff --git a/pkgtools/pkglint/files/autofix.go b/pkgtools/pkglint/files/autofix.go index 6790ec4e62d..78b050d03bc 100644 --- a/pkgtools/pkglint/files/autofix.go +++ b/pkgtools/pkglint/files/autofix.go @@ -1,7 +1,6 @@ package pkglint import ( - "netbsd.org/pkglint/regex" "os" "strconv" "strings" @@ -117,8 +116,8 @@ func (fix *Autofix) ReplaceAfter(prefix, from string, to string) { } 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, from string, to string) { // 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 int, textIndex int, from string, to strin // 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 string) { 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 string) { 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, format string, args ...interface{}) { // 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 { diff --git a/pkgtools/pkglint/files/autofix_test.go b/pkgtools/pkglint/files/autofix_test.go index 3adabdc1289..303060a63b9 100644 --- a/pkgtools/pkglint/files/autofix_test.go +++ b/pkgtools/pkglint/files/autofix_test.go @@ -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_leaves_line_unchanged(c *check.C) { 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_autofix_modifies_line(c *check.C) { 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_autofix_modifies_line(c *check.C) { 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_fixes(c *check.C) { { 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_unfixable_warnings_with_show_autofix(c *c 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_replace(c *check.C) { 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__silent_with_diagnostic(c *check.C) { "\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_ReplaceAfter__after_inserting_a_line(c *check.C) { 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_ReplaceAfter__after_inserting_a_line(c *check.C) { "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 *check.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_replacing_string(c *check.C) { // 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_after_replacing_regex(c *check.C) { 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 *check.C) { 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__file_busy_Windows(c *check.C) { "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(c *check.C) { 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) diff --git a/pkgtools/pkglint/files/category.go b/pkgtools/pkglint/files/category.go index 0f8e540f7ee..54336461f7e 100644 --- a/pkgtools/pkglint/files/category.go +++ b/pkgtools/pkglint/files/category.go @@ -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 { diff --git a/pkgtools/pkglint/files/check_test.go b/pkgtools/pkglint/files/check_test.go index 417ffba7664..e96c133ce18 100644 --- a/pkgtools/pkglint/files/check_test.go +++ b/pkgtools/pkglint/files/check_test.go @@ -956,6 +956,10 @@ func (t *Tester) ExpectAssert(action func()) { // 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) diff --git a/pkgtools/pkglint/files/intqa/qa.go b/pkgtools/pkglint/files/intqa/qa.go index d1733b2a091..766e0b38a4e 100644 --- a/pkgtools/pkglint/files/intqa/qa.go +++ b/pkgtools/pkglint/files/intqa/qa.go @@ -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) diff --git a/pkgtools/pkglint/files/intqa/qa_test.go b/pkgtools/pkglint/files/intqa/qa_test.go index 65c1f3ee69c..f647b8ff062 100644 --- a/pkgtools/pkglint/files/intqa/qa_test.go +++ b/pkgtools/pkglint/files/intqa/qa_test.go @@ -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 *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_checkMethodsSameFile(c *check.C) { 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_errors(c *check.C) { 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) diff --git a/pkgtools/pkglint/files/linechecker.go b/pkgtools/pkglint/files/linechecker.go index 525f26ae079..d96c221f9c2 100644 --- a/pkgtools/pkglint/files/linechecker.go +++ b/pkgtools/pkglint/files/linechecker.go @@ -1,7 +1,7 @@ package pkglint import ( - "fmt" + "netbsd.org/pkglint/textproc" "strings" ) @@ -15,42 +15,43 @@ func (ck LineChecker) CheckLength(maxLength int) { } 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() } diff --git a/pkgtools/pkglint/files/linechecker_test.go b/pkgtools/pkglint/files/linechecker_test.go index 9a63147a652..fa14c4e497d 100644 --- a/pkgtools/pkglint/files/linechecker_test.go +++ b/pkgtools/pkglint/files/linechecker_test.go @@ -20,21 +20,55 @@ func (s *Suite) Test_LineChecker_CheckLength(c *check.C) { 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...) } diff --git a/pkgtools/pkglint/files/lines.go b/pkgtools/pkglint/files/lines.go index 348d2135589..95f1d157302 100644 --- a/pkgtools/pkglint/files/lines.go +++ b/pkgtools/pkglint/files/lines.go @@ -58,7 +58,7 @@ func (ls *Lines) CheckCvsID(index int, prefixRe regex.Pattern, suggestedPrefix s "", "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() } diff --git a/pkgtools/pkglint/files/logging.go b/pkgtools/pkglint/files/logging.go index fd2117178a2..022798a1cad 100644 --- a/pkgtools/pkglint/files/logging.go +++ b/pkgtools/pkglint/files/logging.go @@ -139,7 +139,7 @@ func (l *Logger) Diag(line *Line, level *LogLevel, format string, args ...interf 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 string) bool { 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 } diff --git a/pkgtools/pkglint/files/logging_test.go b/pkgtools/pkglint/files/logging_test.go index a6b5cb85b07..9bfabeb5e7c 100644 --- a/pkgtools/pkglint/files/logging_test.go +++ b/pkgtools/pkglint/files/logging_test.go @@ -317,14 +317,15 @@ func (s *Suite) Test_Logger_shallBeLogged(c *check.C) { // 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__separator(c *check.C) { 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__with_explanation(c *check.C) { // 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__with_explanation_in_same_line(c *check.C // 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__without_explanation_in_same_line(c *chec // 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__separator_show_autofix(c *check.C) { 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__separator_show_autofix(c *check.C) { "+\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__separator_show_autofix_with_explanation( 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__separator_show_autofix_with_explanation( "") } +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__separator_autofix(c *check.C) { 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__duplicate_autofix(c *check.C) { 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\".") } diff --git a/pkgtools/pkglint/files/mkline_test.go b/pkgtools/pkglint/files/mkline_test.go index 0c4cd50a919..839ba2d0972 100644 --- a/pkgtools/pkglint/files/mkline_test.go +++ b/pkgtools/pkglint/files/mkline_test.go @@ -1076,6 +1076,34 @@ func (s *Suite) Test_MkLine_VariableNeedsQuoting__tool_in_unknown_quotes(c *chec 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) diff --git a/pkgtools/pkglint/files/mklinechecker.go b/pkgtools/pkglint/files/mklinechecker.go index 977c28fe4b3..ebd94951093 100644 --- a/pkgtools/pkglint/files/mklinechecker.go +++ b/pkgtools/pkglint/files/mklinechecker.go @@ -1,7 +1,6 @@ package pkglint import ( - "netbsd.org/pkglint/regex" "netbsd.org/pkglint/textproc" "strings" ) @@ -286,7 +285,9 @@ func (ck MkLineChecker) checkDirectiveIndentation(expectedDepth int) { 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() } } diff --git a/pkgtools/pkglint/files/mklinechecker_test.go b/pkgtools/pkglint/files/mklinechecker_test.go index 925366bed39..ae9d7f645f6 100644 --- a/pkgtools/pkglint/files/mklinechecker_test.go +++ b/pkgtools/pkglint/files/mklinechecker_test.go @@ -127,7 +127,6 @@ func (s *Suite) Test_MkLineChecker_checkEmptyContinuation(c *check.C) { 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_checkDirectiveIndentation__autofix_multiline( ".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) diff --git a/pkgtools/pkglint/files/mklineparser.go b/pkgtools/pkglint/files/mklineparser.go index dea95dfdd75..549796ebad0 100644 --- a/pkgtools/pkglint/files/mklineparser.go +++ b/pkgtools/pkglint/files/mklineparser.go @@ -100,7 +100,7 @@ func (p MkLineParser) MatchVarassign(line *Line, text string, splitResult *mkLin } 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_") { diff --git a/pkgtools/pkglint/files/mkvarusechecker.go b/pkgtools/pkglint/files/mkvarusechecker.go index bb4f9700ed6..b99fe286e56 100644 --- a/pkgtools/pkglint/files/mkvarusechecker.go +++ b/pkgtools/pkglint/files/mkvarusechecker.go @@ -142,7 +142,7 @@ func (ck *MkVarUseChecker) checkVarname(time VucTime) { 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() } } diff --git a/pkgtools/pkglint/files/mkvarusechecker_test.go b/pkgtools/pkglint/files/mkvarusechecker_test.go index 6fee1c066a8..58656aaed2c 100644 --- a/pkgtools/pkglint/files/mkvarusechecker_test.go +++ b/pkgtools/pkglint/files/mkvarusechecker_test.go @@ -363,16 +363,20 @@ func (s *Suite) Test_MkVarUseChecker_checkVarname(c *check.C) { "\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) { diff --git a/pkgtools/pkglint/files/package_test.go b/pkgtools/pkglint/files/package_test.go index 5a49bc2aeff..e7817320e30 100644 --- a/pkgtools/pkglint/files/package_test.go +++ b/pkgtools/pkglint/files/package_test.go @@ -1547,6 +1547,23 @@ func (s *Suite) Test_Package_checkfilePackageMakefile__redundancy_in_infra(c *ch // 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 diff --git a/pkgtools/pkglint/files/paragraph.go b/pkgtools/pkglint/files/paragraph.go index 2708620cd75..fa635992302 100644 --- a/pkgtools/pkglint/files/paragraph.go +++ b/pkgtools/pkglint/files/paragraph.go @@ -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 diff --git a/pkgtools/pkglint/files/plist.go b/pkgtools/pkglint/files/plist.go index bdbd9d08625..bd4a26cf9af 100644 --- a/pkgtools/pkglint/files/plist.go +++ b/pkgtools/pkglint/files/plist.go @@ -15,7 +15,7 @@ func CheckLinesPlist(pkg *Package, lines *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(pline *PlistLine) { "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 []*PlistLine) *plistLineSorter { 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 diff --git a/pkgtools/pkglint/files/plist_test.go b/pkgtools/pkglint/files/plist_test.go index 8dfc3f51e02..58b1fdfc205 100644 --- a/pkgtools/pkglint/files/plist_test.go +++ b/pkgtools/pkglint/files/plist_test.go @@ -88,7 +88,7 @@ func (s *Suite) Test_CheckLinesPlist__empty(c *check.C) { 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_checkPathMan(c *check.C) { 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) { diff --git a/pkgtools/pkglint/files/redundantscope_test.go b/pkgtools/pkglint/files/redundantscope_test.go index 94139c00aa6..bdf7e929114 100644 --- a/pkgtools/pkglint/files/redundantscope_test.go +++ b/pkgtools/pkglint/files/redundantscope_test.go @@ -1427,15 +1427,17 @@ func (s *Suite) Test_RedundantScope__procedure_parameters(c *check.C) { 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__infra(c *check.C) { } 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 diff --git a/pkgtools/pkglint/files/shell.go b/pkgtools/pkglint/files/shell.go index 6a1a269c2ff..f9f401e172a 100644 --- a/pkgtools/pkglint/files/shell.go +++ b/pkgtools/pkglint/files/shell.go @@ -277,7 +277,7 @@ func (scc *SimpleCommandChecker) checkAutoMkdirs() { } 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) checkPipeExitcode(pipeline *MkShPipeline) { 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(token string, checkQuoting bool, time Tool // 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 } diff --git a/pkgtools/pkglint/files/util.go b/pkgtools/pkglint/files/util.go index c453082c8d4..31df2c79431 100644 --- a/pkgtools/pkglint/files/util.go +++ b/pkgtools/pkglint/files/util.go @@ -159,6 +159,15 @@ func trimCommon(a, b string) (string, string) { 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) } diff --git a/pkgtools/pkglint/files/util_test.go b/pkgtools/pkglint/files/util_test.go index f2ef99a98ae..4b4ab3bdeb9 100644 --- a/pkgtools/pkglint/files/util_test.go +++ b/pkgtools/pkglint/files/util_test.go @@ -59,6 +59,32 @@ func (s *Suite) Test_trimCommon(c *check.C) { "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) diff --git a/pkgtools/pkglint/files/varalignblock.go b/pkgtools/pkglint/files/varalignblock.go index 9ccf615b309..1958e4b72d0 100644 --- a/pkgtools/pkglint/files/varalignblock.go +++ b/pkgtools/pkglint/files/varalignblock.go @@ -186,7 +186,7 @@ func (va *VaralignBlock) processVarassign(mkline *MkLine) { 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(newWidth int) { // 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(minTotalWidth int, maxTotalWidth int, minVa } } -// 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) alignValueSingle(newWidth int) { 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) alignValueMultiInitial(column int) { 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) alignContinuation(valueColumn, rightMarginColumn int) 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) isCanonicalInitial(column int) bool { // 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(valueColumn int) bool { } 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) } diff --git a/pkgtools/pkglint/files/varalignblock_test.go b/pkgtools/pkglint/files/varalignblock_test.go index e3f8ed967b4..90b17f739e9 100644 --- a/pkgtools/pkglint/files/varalignblock_test.go +++ b/pkgtools/pkglint/files/varalignblock_test.go @@ -3109,74 +3109,6 @@ func (s *Suite) Test_varalignLine_realignDetails(c *check.C) { 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_split(c *check.C) { 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_rightMargin(c *check.C) { actualCommon, actualMargin := mkinfo.rightMargin() t.CheckDeepEquals( []interface{}{actualCommon, actualMargin}, - []interface{}{common, margin}) + []interface{}{common, expectedRightMargin}) } } @@ -3574,6 +3506,18 @@ func (s *Suite) Test_varalignMkLine_rightMargin(c *check.C) { "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_alignValueSingle(c *check.C) { 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_alignValueSingle(c *check.C) { "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_alignValueMultiEmptyInitial(c *check.C) { "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_alignValueMultiEmptyInitial__spaces(c *check.C 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_alignValueMultiInitial(c *check.C) { 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_alignValueMultiInitial(c *check.C) { "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_alignValueMultiFollow(c *check.C) { 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_alignValueMultiFollow(c *check.C) { "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_alignValueMultiFollow(c *check.C) { "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_alignValueMultiFollow__unindent_long_initial_l 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_alignContinuation(c *check.C) { 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_spaceBeforeContinuation__continued_comment(c 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. diff --git a/pkgtools/pkglint/files/vardefs.go b/pkgtools/pkglint/files/vardefs.go index df471079ffd..6a20877dd0f 100644 --- a/pkgtools/pkglint/files/vardefs.go +++ b/pkgtools/pkglint/files/vardefs.go @@ -1444,7 +1444,7 @@ func (reg *VarTypeRegistry) Init(src *Pkgsrc) { // 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, diff --git a/pkgtools/pkglint/files/vartype_test.go b/pkgtools/pkglint/files/vartype_test.go index 7cce1d06f98..113e4742804 100644 --- a/pkgtools/pkglint/files/vartype_test.go +++ b/pkgtools/pkglint/files/vartype_test.go @@ -32,6 +32,23 @@ func (s *Suite) Test_ACLPermissions_HumanString(c *check.C) { "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) diff --git a/pkgtools/pkglint/files/vartypecheck.go b/pkgtools/pkglint/files/vartypecheck.go index b5b959728e5..fdb232a7ae9 100644 --- a/pkgtools/pkglint/files/vartypecheck.go +++ b/pkgtools/pkglint/files/vartypecheck.go @@ -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() } diff --git a/pkgtools/pkglint/files/vartypecheck_test.go b/pkgtools/pkglint/files/vartypecheck_test.go index 17c9cc678ae..057229614b0 100644 --- a/pkgtools/pkglint/files/vartypecheck_test.go +++ b/pkgtools/pkglint/files/vartypecheck_test.go @@ -1891,6 +1891,16 @@ func (s *Suite) Test_VartypeCheck_URL(c *check.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) { |