diff options
| author | rillig <rillig@pkgsrc.org> | 2018-01-13 23:56:14 +0000 |
|---|---|---|
| committer | rillig <rillig@pkgsrc.org> | 2018-01-13 23:56:14 +0000 |
| commit | b78b40cabee20aeb2f37dd3a876e9b6714be440e (patch) | |
| tree | e215bf58ea6c2ad3c8a67e4e5adf1a650cb705f8 /pkgtools | |
| parent | f7af8cc66466da03ee5d400edd01fbe41187ed54 (diff) | |
| download | pkgsrc-b78b40cabee20aeb2f37dd3a876e9b6714be440e.tar.gz | |
pkglint: Update to 5.5.1
Changes since 5.5:
* Allow filtering log messages by keyword (--only)
* In --autofix and --show-autofix mode, show only fixable diagnostics
* When called with --source, show the source below the diagnostics
* Don't warn about USE_LANGUAGES in ../../mk/compiler.mk
* Fix autofix for .gz in PLIST
Diffstat (limited to 'pkgtools')
45 files changed, 1477 insertions, 864 deletions
diff --git a/pkgtools/pkglint/DESCR b/pkgtools/pkglint/DESCR index bff78e0924e..4691b28794c 100644 --- a/pkgtools/pkglint/DESCR +++ b/pkgtools/pkglint/DESCR @@ -1,6 +1,6 @@ -pkglint is for pkgsrc packages what lint(1) is for C files. It checks -for various things that the used languages cannot detect, for example -application-specific restrictions on certain variables. +pkglint checks whether a pkgsrc package conforms to the various +conventions established over the years. It produces warnings, errors and +notes and, upon request, explains them. Before importing a new package or making changes to an existing package, pkglint should be run in the package's directory to check for common diff --git a/pkgtools/pkglint/Makefile b/pkgtools/pkglint/Makefile index c28d8e2f2f5..56b6364fd58 100644 --- a/pkgtools/pkglint/Makefile +++ b/pkgtools/pkglint/Makefile @@ -1,6 +1,6 @@ -# $NetBSD: Makefile,v 1.524 2018/01/10 00:39:52 rillig Exp $ +# $NetBSD: Makefile,v 1.525 2018/01/13 23:56:14 rillig Exp $ -PKGNAME= pkglint-5.5 +PKGNAME= pkglint-5.5.1 DISTFILES= # none CATEGORIES= pkgtools diff --git a/pkgtools/pkglint/files/autofix.go b/pkgtools/pkglint/files/autofix.go new file mode 100644 index 00000000000..20e277bd77c --- /dev/null +++ b/pkgtools/pkglint/files/autofix.go @@ -0,0 +1,238 @@ +package main + +import ( + "fmt" + "io/ioutil" + "netbsd.org/pkglint/regex" + "netbsd.org/pkglint/trace" + "os" + "strings" +) + +// Autofix handles all modifications to a single line, +// describes them in a human-readable form and formats the output. +// The modifications are kept in memory only, +// until they are written to disk by SaveAutofixChanges. +type Autofix struct { + line Line + linesBefore []string // Newly inserted lines, including \n + lines []*RawLine // Original lines, available for diff + linesAfter []string // Newly inserted lines, including \n + modified bool // Modified in memory, but not necessarily written back to disk + descrFormat string // Human-readable description of the latest modification + descrArgs []interface{} // + level *LogLevel // + diagFormat string // Is printed only if it couldn't be fixed automatically + diagArgs []interface{} // + explanation []string // Is printed together with the diagnostic +} + +func NewAutofix(line Line) *Autofix { + return &Autofix{ + line: line, + lines: append([]*RawLine{}, line.raw...)} +} + +func (fix *Autofix) Replace(from string, to string) { + if fix.skip() { + return + } + + for _, rawLine := range fix.lines { + if rawLine.Lineno != 0 { + if replaced := strings.Replace(rawLine.textnl, from, to, 1); replaced != rawLine.textnl { + if G.opts.PrintAutofix || G.opts.Autofix { + rawLine.textnl = replaced + } + fix.Describef("Replacing %q with %q.", from, to) + } + } + } +} + +func (fix *Autofix) ReplaceRegex(from regex.Pattern, to string) { + if fix.skip() { + return + } + + for _, rawLine := range fix.lines { + if rawLine.Lineno != 0 { + if replaced := regex.Compile(from).ReplaceAllString(rawLine.textnl, to); replaced != rawLine.textnl { + if G.opts.PrintAutofix || G.opts.Autofix { + rawLine.textnl = replaced + } + fix.Describef("Replacing regular expression %q with %q.", from, to) + } + } + } +} + +func (fix *Autofix) InsertBefore(text string) { + if fix.skip() { + return + } + + fix.linesBefore = append(fix.linesBefore, text+"\n") + fix.Describef("Inserting a line %q before this line.", text) +} + +func (fix *Autofix) InsertAfter(text string) { + if fix.skip() { + return + } + + fix.linesAfter = append(fix.linesAfter, text+"\n") + fix.Describef("Inserting a line %q after this line.", text) +} + +func (fix *Autofix) Delete() { + if fix.skip() { + return + } + + for _, line := range fix.lines { + line.textnl = "" + } + fix.Describef("Deleting this line.") +} + +func (fix *Autofix) Describef(format string, args ...interface{}) { + fix.descrFormat = format + fix.descrArgs = args +} + +func (fix *Autofix) Notef(format string, args ...interface{}) { + fix.level = llNote + fix.diagFormat = format + fix.diagArgs = args +} + +func (fix *Autofix) Warnf(format string, args ...interface{}) { + fix.level = llWarn + fix.diagFormat = format + fix.diagArgs = args +} + +func (fix *Autofix) Errorf(format string, args ...interface{}) { + fix.level = llError + fix.diagFormat = format + fix.diagArgs = args +} + +func (fix *Autofix) Explain(explanation ...string) { + fix.explanation = explanation +} + +// Depending on the pkglint mode, either: +// +// * logs the associated message (default) +// * logs what would be fixed (--show-autofix) +// * records the fixes in the line (--autofix) +func (fix *Autofix) Apply() { + line := fix.line + if line.firstLine < 1 { + return + } + + if shallBeLogged(fix.diagFormat) && fix.descrFormat != "" { + logDiagnostic := fix.level != nil && fix.diagFormat != "Silent-Magic-Diagnostic" && !G.opts.Autofix + if logDiagnostic { + msg := fmt.Sprintf(fix.diagFormat, fix.diagArgs...) + logs(fix.level, line.Filename, line.Linenos(), fix.diagFormat, msg) + } + + logRepair := G.opts.Autofix || G.opts.PrintAutofix + if logRepair { + msg := fmt.Sprintf(fix.descrFormat, fix.descrArgs...) + logs(llAutofix, line.Filename, line.Linenos(), "", msg) + } + + if logDiagnostic || logRepair { + line.printSource(G.logOut) + if G.opts.Explain && logDiagnostic && len(fix.explanation) != 0 { + Explain(fix.explanation...) + } else if G.opts.PrintSource { + G.logOut.Separate() + } + } + } + + fix.modified = fix.modified || fix.descrFormat != "" + + fix.descrFormat = "" + fix.descrArgs = nil + fix.level = nil + fix.diagFormat = "" + fix.diagArgs = nil + fix.explanation = nil +} + +func (fix *Autofix) skip() bool { + if fix.diagFormat == "" { + panic("Autofix: The diagnostic must be given before the action.") + } + return !shallBeLogged(fix.diagFormat) +} + +// SaveAutofixChanges writes the given lines back into their files, +// applying the autofix changes. +// The lines may come from different files. +// Only files that actually have changed lines are saved. +func SaveAutofixChanges(lines []Line) (autofixed bool) { + if trace.Tracing { + defer trace.Call0()() + } + + if !G.opts.Autofix { + for _, line := range lines { + if line.autofix != nil && line.autofix.modified { + G.autofixAvailable = true + } + } + return + } + + changes := make(map[string][]string) + changed := make(map[string]bool) + for _, line := range lines { + chlines := changes[line.Filename] + if fix := line.autofix; fix != nil { + if fix.modified { + changed[line.Filename] = true + } + chlines = append(chlines, fix.linesBefore...) + for _, raw := range line.raw { + chlines = append(chlines, raw.textnl) + } + chlines = append(chlines, fix.linesAfter...) + } else { + for _, raw := range line.raw { + chlines = append(chlines, raw.textnl) + } + } + changes[line.Filename] = chlines + } + + for fname := range changed { + changedLines := changes[fname] + tmpname := fname + ".pkglint.tmp" + text := "" + for _, changedLine := range changedLines { + text += changedLine + } + err := ioutil.WriteFile(tmpname, []byte(text), 0666) + if err != nil { + NewLineWhole(tmpname).Errorf("Cannot write.") + continue + } + err = os.Rename(tmpname, fname) + if err != nil { + NewLineWhole(fname).Errorf("Cannot overwrite with auto-fixed content.") + continue + } + msg := "Has been auto-fixed. Please re-run pkglint." + logs(llAutofix, fname, "", msg, msg) + autofixed = true + } + return +} diff --git a/pkgtools/pkglint/files/autofix_test.go b/pkgtools/pkglint/files/autofix_test.go new file mode 100644 index 00000000000..1b2f407342f --- /dev/null +++ b/pkgtools/pkglint/files/autofix_test.go @@ -0,0 +1,338 @@ +package main + +import "gopkg.in/check.v1" + +func (s *Suite) Test_Autofix_ReplaceRegex(c *check.C) { + s.Init(c) + s.UseCommandLine("--show-autofix") + fname := s.CreateTmpFile("Makefile", ""+ + "line1\n"+ + "line2\n"+ + "line3\n") + lines := LoadExistingLines(fname, true) + + fix := lines[1].Autofix() + fix.Warnf("Something's wrong here.") + fix.ReplaceRegex(`.`, "X") + fix.Apply() + SaveAutofixChanges(lines) + + c.Check(lines[1].raw[0].textnl, equals, "XXXXX\n") + c.Check(s.LoadTmpFile("Makefile"), equals, "line1\nline2\nline3\n") + s.CheckOutputLines( + "WARN: ~/Makefile:2: Something's wrong here.", + "AUTOFIX: ~/Makefile:2: Replacing regular expression \".\" with \"X\".") +} + +func (s *Suite) Test_Autofix_ReplaceRegex_with_autofix(c *check.C) { + s.Init(c) + s.UseCommandLine("--autofix", "--source") + fname := s.CreateTmpFile("Makefile", ""+ + "line1\n"+ + "line2\n"+ + "line3\n") + lines := LoadExistingLines(fname, true) + + fix := lines[1].Autofix() + fix.Warnf("Something's wrong here.") + fix.ReplaceRegex(`.`, "X") + fix.Apply() + + fix.Warnf("Use Y instead of X.") + fix.Replace("X", "Y") + fix.Apply() + + SaveAutofixChanges(lines) + + c.Check(s.LoadTmpFile("Makefile"), equals, ""+ + "line1\n"+ + "YXXXX\n"+ + "line3\n") + s.CheckOutputLines( + "AUTOFIX: ~/Makefile:2: Replacing regular expression \".\" with \"X\".", + "- line2", + "+ XXXXX", + "", + "AUTOFIX: ~/Makefile:2: Replacing \"X\" with \"Y\".", + "- line2", + "+ YXXXX", + "", + "AUTOFIX: ~/Makefile: Has been auto-fixed. Please re-run pkglint.") +} + +func (s *Suite) Test_Autofix_ReplaceRegex_with_show_autofix(c *check.C) { + s.Init(c) + s.UseCommandLine("--show-autofix", "--source") + fname := s.CreateTmpFile("Makefile", ""+ + "line1\n"+ + "line2\n"+ + "line3\n") + lines := LoadExistingLines(fname, true) + + fix := lines[1].Autofix() + fix.Warnf("Something's wrong here.") + fix.ReplaceRegex(`.`, "X") + fix.Apply() + + fix.Warnf("Use Y instead of X.") + fix.Replace("X", "Y") + fix.Apply() + + SaveAutofixChanges(lines) + + s.CheckOutputLines( + "WARN: ~/Makefile:2: Something's wrong here.", + "AUTOFIX: ~/Makefile:2: Replacing regular expression \".\" with \"X\".", + "- line2", + "+ XXXXX", + "", + "WARN: ~/Makefile:2: Use Y instead of X.", + "AUTOFIX: ~/Makefile:2: Replacing \"X\" with \"Y\".", + "- line2", + "+ YXXXX") +} + +func (s *Suite) Test_autofix_MkLines(c *check.C) { + s.Init(c) + s.UseCommandLine("--autofix") + fname := s.CreateTmpFile("Makefile", ""+ + "line1 := value1\n"+ + "line2 := value2\n"+ + "line3 := value3\n") + pkg := NewPackage("category/basename") + G.Pkg = pkg + mklines := pkg.loadPackageMakefile(fname) + G.Pkg = nil + + fix := mklines.mklines[1].Autofix() + fix.Warnf("Something's wrong here.") + fix.ReplaceRegex(`.`, "X") + fix.Apply() + SaveAutofixChanges(mklines.lines) + + c.Check(s.LoadTmpFile("Makefile"), equals, ""+ + "line1 := value1\n"+ + "XXXXXXXXXXXXXXX\n"+ + "line3 := value3\n") + s.CheckOutputLines( + "AUTOFIX: ~/Makefile:2: Replacing regular expression \".\" with \"X\".", + "AUTOFIX: ~/Makefile: Has been auto-fixed. Please re-run pkglint.") +} + +func (s *Suite) Test_Autofix_multiple_modifications(c *check.C) { + s.Init(c) + s.UseCommandLine("--show-autofix", "--explain") + + line := NewLine("fname", 1, "dummy", T.NewRawLines(1, "original\n")) + + c.Check(line.autofix, check.IsNil) + c.Check(line.raw, check.DeepEquals, T.NewRawLines(1, "original\n")) + + { + fix := line.Autofix() + fix.Warnf("Silent-Magic-Diagnostic") + fix.ReplaceRegex(`(.)(.*)(.)`, "$3$2$1") + fix.Apply() + } + + c.Check(line.autofix, check.NotNil) + c.Check(line.raw, check.DeepEquals, T.NewRawLines(1, "original\n", "lriginao\n")) + s.CheckOutputLines( + "AUTOFIX: fname:1: Replacing regular expression \"(.)(.*)(.)\" with \"$3$2$1\".") + + { + fix := line.Autofix() + fix.Warnf("Silent-Magic-Diagnostic") + fix.Replace("i", "u") + fix.Apply() + } + + c.Check(line.autofix, check.NotNil) + c.Check(line.raw, check.DeepEquals, T.NewRawLines(1, "original\n", "lruginao\n")) + c.Check(line.raw[0].textnl, equals, "lruginao\n") + s.CheckOutputLines( + "AUTOFIX: fname:1: Replacing \"i\" with \"u\".") + + { + fix := line.Autofix() + fix.Warnf("Silent-Magic-Diagnostic") + fix.Replace("lruginao", "middle") + fix.Apply() + } + + c.Check(line.autofix, check.NotNil) + c.Check(line.raw, check.DeepEquals, T.NewRawLines(1, "original\n", "middle\n")) + c.Check(line.raw[0].textnl, equals, "middle\n") + s.CheckOutputLines( + "AUTOFIX: fname:1: Replacing \"lruginao\" with \"middle\".") + + { + fix := line.Autofix() + fix.Warnf("Silent-Magic-Diagnostic") + fix.InsertBefore("before") + fix.Apply() + + fix.Warnf("Silent-Magic-Diagnostic") + fix.InsertBefore("between before and middle") + fix.Apply() + + fix.Warnf("Silent-Magic-Diagnostic") + fix.InsertAfter("between middle and after") + fix.Apply() + + fix.Notef("This diagnostic is necessary for the following explanation.") + fix.Explain( + "When inserting multiple lines, Apply must be called in-between.", + "Otherwise the changes are not described to the human reader.") + fix.InsertAfter("after") + fix.Apply() + } + + c.Check(line.autofix.linesBefore, check.DeepEquals, []string{ + "before\n", + "between before and middle\n"}) + c.Check(line.autofix.lines[0].textnl, equals, "middle\n") + c.Check(line.autofix.linesAfter, deepEquals, []string{ + "between middle and after\n", + "after\n"}) + s.CheckOutputLines( + "AUTOFIX: fname:1: Inserting a line \"before\" before this line.", + "AUTOFIX: fname:1: Inserting a line \"between before and middle\" before this line.", + "AUTOFIX: fname:1: Inserting a line \"between middle and after\" after this line.", + "NOTE: fname:1: This diagnostic is necessary for the following explanation.", + "AUTOFIX: fname:1: Inserting a line \"after\" after this line.", + "", + "\tWhen inserting multiple lines, Apply must be called in-between.", + "\tOtherwise the changes are not described to the human reader.", + "") + + { + fix := line.Autofix() + fix.Warnf("Silent-Magic-Diagnostic") + fix.Delete() + fix.Apply() + } + + c.Check(line.autofix.linesBefore, check.DeepEquals, []string{ + "before\n", + "between before and middle\n"}) + c.Check(line.autofix.lines[0].textnl, equals, "") + c.Check(line.autofix.linesAfter, deepEquals, []string{ + "between middle and after\n", + "after\n"}) + s.CheckOutputLines( + "AUTOFIX: fname:1: Deleting this line.") +} + +func (s *Suite) Test_Autofix_show_source_code(c *check.C) { + s.Init(c) + s.UseCommandLine("--show-autofix", "--source") + line := NewLineMulti("Makefile", 27, 29, "# old", T.NewRawLines( + 27, "before\n", + 28, "The old song\n", + 29, "after\n")) + + { + fix := line.Autofix() + fix.Warnf("Using \"old\" is deprecated.") + fix.Replace("old", "new") + fix.Apply() + } + + s.CheckOutputLines( + "WARN: Makefile:27--29: Using \"old\" is deprecated.", + "AUTOFIX: Makefile:27--29: Replacing \"old\" with \"new\".", + "> before", + "- The old song", + "+ The new song", + "> after") +} + +func (s *Suite) Test_Autofix_InsertBefore(c *check.C) { + s.Init(c) + s.UseCommandLine("--show-autofix", "--source") + line := NewLine("Makefile", 30, "original", T.NewRawLines(30, "original\n")) + + fix := line.Autofix() + fix.Warnf("Dummy") + fix.InsertBefore("inserted") + fix.Apply() + + s.CheckOutputLines( + "WARN: Makefile:30: Dummy", + "AUTOFIX: Makefile:30: Inserting a line \"inserted\" before this line.", + "+ inserted", + "> original") +} + +func (s *Suite) Test_Autofix_Delete(c *check.C) { + s.Init(c) + s.UseCommandLine("--show-autofix", "--source") + line := NewLine("Makefile", 30, "to be deleted", T.NewRawLines(30, "to be deleted\n")) + + fix := line.Autofix() + fix.Warnf("Dummy") + fix.Delete() + fix.Apply() + + s.CheckOutputLines( + "WARN: Makefile:30: Dummy", + "AUTOFIX: Makefile:30: Deleting this line.", + "- to be deleted") +} + +// Demonstrates that the --show-autofix option only shows those diagnostics +// that would be fixed. +func (s *Suite) Test_Autofix_suppress_unfixable_warnings(c *check.C) { + s.Init(c) + s.UseCommandLine("--show-autofix", "--source") + lines := T.NewLines("Makefile", + "line1", + "line2", + "line3") + + lines[0].Warnf("This warning is not shown since it is not automatically fixed.") + + fix := lines[1].Autofix() + fix.Warnf("Something's wrong here.") + fix.ReplaceRegex(`.`, "X") + fix.Apply() + + fix.Warnf("The XXX marks are usually not fixed, use TODO instead.") + fix.Replace("XXX", "TODO") + fix.Apply() + + lines[2].Warnf("Neither is this warning shown.") + + s.CheckOutputLines( + "WARN: Makefile:2: Something's wrong here.", + "AUTOFIX: Makefile:2: Replacing regular expression \".\" with \"X\".", + "- line2", + "+ XXXXX", + "", + "WARN: Makefile:2: The XXX marks are usually not fixed, use TODO instead.", + "AUTOFIX: Makefile:2: Replacing \"XXX\" with \"TODO\".", + "- line2", + "+ TODOXX") +} + +func (s *Suite) Test_SaveAutofixChanges(c *check.C) { + s.Init(c) + s.UseCommandLine("--autofix") + filename := s.CreateTmpFileLines("DESCR", + "Line 1", + "Line 2") + lines := LoadExistingLines(filename, false) + + fix := lines[0].Autofix() + fix.Warnf("Dummy warning.") + fix.Replace("X", "Y") + fix.Apply() + + // Since nothing has been effectively changed, + // nothing needs to be saved. + SaveAutofixChanges(lines) + + // And therefore, no AUTOFIX action must appear in the log. + s.CheckOutputEmpty() +} diff --git a/pkgtools/pkglint/files/buildlink3_test.go b/pkgtools/pkglint/files/buildlink3_test.go index 3ec8bf1fb0f..4c12a975df6 100644 --- a/pkgtools/pkglint/files/buildlink3_test.go +++ b/pkgtools/pkglint/files/buildlink3_test.go @@ -1,13 +1,11 @@ package main -import ( - check "gopkg.in/check.v1" -) +import "gopkg.in/check.v1" func (s *Suite) Test_ChecklinesBuildlink3Mk(c *check.C) { s.Init(c) G.globalData.InitVartypes() - mklines := s.NewMkLines("buildlink3.mk", + mklines := T.NewMkLines("buildlink3.mk", mkrcsid, "# XXX This file was created automatically using createbuildlink-@PKGVERSION@", "", @@ -43,8 +41,8 @@ func (s *Suite) Test_ChecklinesBuildlink3Mk_name_mismatch(c *check.C) { G.globalData.InitVartypes() G.Pkg = NewPackage("x11/hs-X11") G.Pkg.EffectivePkgbase = "X11" - G.Pkg.EffectivePkgnameLine = NewMkLine(NewLine("Makefile", 3, "DISTNAME=\tX11-1.0", nil)) - mklines := s.NewMkLines("buildlink3.mk", + G.Pkg.EffectivePkgnameLine = T.NewMkLine("Makefile", 3, "DISTNAME=\tX11-1.0") + mklines := T.NewMkLines("buildlink3.mk", mkrcsid, "", "BUILDLINK_TREE+=\ths-X11", @@ -68,7 +66,7 @@ func (s *Suite) Test_ChecklinesBuildlink3Mk_name_mismatch(c *check.C) { func (s *Suite) Test_ChecklinesBuildlink3Mk_name_mismatch_multiple_inclusion(c *check.C) { s.Init(c) G.globalData.InitVartypes() - mklines := s.NewMkLines("buildlink3.mk", + mklines := T.NewMkLines("buildlink3.mk", mkrcsid, "", "BUILDLINK_TREE+=\tpkgbase1", @@ -90,7 +88,7 @@ func (s *Suite) Test_ChecklinesBuildlink3Mk_name_mismatch_multiple_inclusion(c * func (s *Suite) Test_ChecklinesBuildlink3Mk_name_mismatch_abi_api(c *check.C) { s.Init(c) G.globalData.InitVartypes() - mklines := s.NewMkLines("buildlink3.mk", + mklines := T.NewMkLines("buildlink3.mk", mkrcsid, "", "BUILDLINK_TREE+=\ths-X11", @@ -114,7 +112,7 @@ func (s *Suite) Test_ChecklinesBuildlink3Mk_name_mismatch_abi_api(c *check.C) { func (s *Suite) Test_ChecklinesBuildlink3Mk_abi_api_versions(c *check.C) { s.Init(c) G.globalData.InitVartypes() - mklines := s.NewMkLines("buildlink3.mk", + mklines := T.NewMkLines("buildlink3.mk", mkrcsid, "", "BUILDLINK_TREE+=\ths-X11", @@ -138,7 +136,7 @@ func (s *Suite) Test_ChecklinesBuildlink3Mk_abi_api_versions(c *check.C) { func (s *Suite) Test_ChecklinesBuildlink3Mk_no_BUILDLINK_TREE_at_beginning(c *check.C) { s.Init(c) G.globalData.InitVartypes() - mklines := s.NewMkLines("buildlink3.mk", + mklines := T.NewMkLines("buildlink3.mk", mkrcsid, "", ".if !defined(HS_X11_BUILDLINK3_MK)", @@ -161,7 +159,7 @@ func (s *Suite) Test_ChecklinesBuildlink3Mk_no_BUILDLINK_TREE_at_beginning(c *ch func (s *Suite) Test_ChecklinesBuildlink3Mk_no_BUILDLINK_TREE_at_end(c *check.C) { s.Init(c) G.globalData.InitVartypes() - mklines := s.NewMkLines("buildlink3.mk", + mklines := T.NewMkLines("buildlink3.mk", mkrcsid, "", "BUILDLINK_DEPMETHOD.hs-X11?=\tfull", @@ -189,7 +187,7 @@ func (s *Suite) Test_ChecklinesBuildlink3Mk_no_BUILDLINK_TREE_at_end(c *check.C) func (s *Suite) Test_ChecklinesBuildlink3Mk_multiple_inclusion_wrong(c *check.C) { s.Init(c) G.globalData.InitVartypes() - mklines := s.NewMkLines("buildlink3.mk", + mklines := T.NewMkLines("buildlink3.mk", mkrcsid, "", "BUILDLINK_TREE+=\ths-X11", @@ -207,7 +205,7 @@ func (s *Suite) Test_ChecklinesBuildlink3Mk_multiple_inclusion_wrong(c *check.C) func (s *Suite) Test_ChecklinesBuildlink3Mk_missing_endif(c *check.C) { s.Init(c) G.globalData.InitVartypes() - mklines := s.NewMkLines("buildlink3.mk", + mklines := T.NewMkLines("buildlink3.mk", mkrcsid, "", "BUILDLINK_TREE+=\tpkgbase1", @@ -224,7 +222,7 @@ func (s *Suite) Test_ChecklinesBuildlink3Mk_missing_endif(c *check.C) { func (s *Suite) Test_ChecklinesBuildlink3Mk_unknown_dependency_patterns(c *check.C) { s.Init(c) G.globalData.InitVartypes() - mklines := s.NewMkLines("buildlink3.mk", + mklines := T.NewMkLines("buildlink3.mk", mkrcsid, "", "BUILDLINK_TREE+= hs-X11", @@ -250,7 +248,7 @@ func (s *Suite) Test_ChecklinesBuildlink3Mk_unknown_dependency_patterns(c *check func (s *Suite) Test_ChecklinesBuildlink3Mk_PKGBASE_with_variable(c *check.C) { s.Init(c) G.globalData.InitVartypes() - mklines := s.NewMkLines("buildlink3.mk", + mklines := T.NewMkLines("buildlink3.mk", mkrcsid, "", "BUILDLINK_TREE+=\t${PYPKGPREFIX}-wxWidgets", @@ -274,7 +272,7 @@ func (s *Suite) Test_ChecklinesBuildlink3Mk_PKGBASE_with_variable(c *check.C) { func (s *Suite) Test_ChecklinesBuildlink3Mk_PKGBASE_with_unknown_variable(c *check.C) { s.Init(c) G.globalData.InitVartypes() - mklines := s.NewMkLines("buildlink3.mk", + mklines := T.NewMkLines("buildlink3.mk", mkrcsid, "", "BUILDLINK_TREE+=\t${LICENSE}-wxWidgets", @@ -304,7 +302,7 @@ func (s *Suite) Test_ChecklinesBuildlink3Mk_indentation(c *check.C) { s.Init(c) s.UseCommandLine("-Wall") G.globalData.InitVartypes() - mklines := s.NewMkLines("buildlink3.mk", + mklines := T.NewMkLines("buildlink3.mk", mkrcsid, "", ".if ${VAAPI_AVAILABLE} == \"yes\"", diff --git a/pkgtools/pkglint/files/category.go b/pkgtools/pkglint/files/category.go index 98d5b6886d4..c5f7ecced45 100644 --- a/pkgtools/pkglint/files/category.go +++ b/pkgtools/pkglint/files/category.go @@ -127,17 +127,19 @@ func CheckdirCategory() { if !fAtend && (mAtend || fCurrent < mCurrent) { if !mCheck[fCurrent] { - if !line.AutofixInsertBefore("SUBDIR+=\t" + fCurrent) { - line.Errorf("%q exists in the file system, but not in the Makefile.", fCurrent) - } + fix := line.Autofix() + fix.Errorf("%q exists in the file system, but not in the Makefile.", fCurrent) + fix.InsertBefore("SUBDIR+=\t" + fCurrent) + fix.Apply() } fNeednext = true } else if !mAtend && (fAtend || mCurrent < fCurrent) { if !fCheck[mCurrent] { - if !line.AutofixDelete() { - line.Errorf("%q exists in the Makefile, but not in the file system.", mCurrent) - } + fix := line.Autofix() + fix.Errorf("%q exists in the Makefile, but not in the file system.", mCurrent) + fix.Delete() + fix.Apply() } mNeednext = true diff --git a/pkgtools/pkglint/files/check_test.go b/pkgtools/pkglint/files/check_test.go index 59d0defa21b..80359d219d8 100644 --- a/pkgtools/pkglint/files/check_test.go +++ b/pkgtools/pkglint/files/check_test.go @@ -10,7 +10,7 @@ import ( "strings" "testing" - check "gopkg.in/check.v1" + "gopkg.in/check.v1" "netbsd.org/pkglint/textproc" "netbsd.org/pkglint/trace" ) @@ -73,54 +73,24 @@ func (s *Suite) CheckOutputLines(expectedLines ...string) { s.c().Check(s.Output(), equals, expectedOutput) } -// Arguments are either (lineno, orignl) or (lineno, orignl, textnl). -func (s *Suite) NewRawLines(args ...interface{}) []*RawLine { - rawlines := make([]*RawLine, len(args)/2) - j := 0 - for i := 0; i < len(args); i += 2 { - lineno := args[i].(int) - orignl := args[i+1].(string) - textnl := orignl - if i+2 < len(args) { - if s, ok := args[i+2].(string); ok { - textnl = s - i++ - } - } - rawlines[j] = &RawLine{lineno, orignl, textnl} - j++ - } - return rawlines[:j] -} - -func (s *Suite) NewLines(fname string, texts ...string) []Line { - result := make([]Line, len(texts)) - for i, text := range texts { - textnl := text + "\n" - result[i] = NewLine(fname, i+1, text, s.NewRawLines(i+1, textnl)) - } - return result -} - -func (s *Suite) NewMkLines(fname string, lines ...string) *MkLines { - return NewMkLines(s.NewLines(fname, lines...)) -} - func (s *Suite) BeginDebugToStdout() { - G.logOut = os.Stdout + G.logOut = NewSeparatorWriter(os.Stdout) trace.Out = os.Stdout trace.Tracing = true } func (s *Suite) EndDebugToStdout() { - G.logOut = &s.stdout + G.logOut = NewSeparatorWriter(&s.stdout) trace.Out = &s.stdout trace.Tracing = false } +// UseCommandLine simulates a command line for the remainder of the test. +// See Pkglint.ParseCommandLine func (s *Suite) UseCommandLine(args ...string) { exitcode := new(Pkglint).ParseCommandLine(append([]string{"pkglint"}, args...)) if exitcode != nil && *exitcode != 0 { + s.CheckOutputEmpty() s.c().Fatalf("Cannot parse command line: %#v", args) } G.opts.LogVerbose = true // See SetUpTest @@ -200,7 +170,7 @@ func (s *Suite) ExpectFatalError(action func()) { func (s *Suite) SetUpTest(c *check.C) { G = GlobalVars{Testing: true} textproc.Testing = true - G.logOut, G.logErr, trace.Out = &s.stdout, &s.stderr, &s.stdout + G.logOut, G.logErr, trace.Out = NewSeparatorWriter(&s.stdout), NewSeparatorWriter(&s.stderr), &s.stdout s.checkC = c s.UseCommandLine( /* no arguments */ ) s.checkC = nil @@ -219,3 +189,66 @@ func (s *Suite) TearDownTest(c *check.C) { var _ = check.Suite(new(Suite)) func Test(t *testing.T) { check.TestingT(t) } + +var T TestObjectCreator + +type TestObjectCreator struct{} + +// Arguments are either (lineno, orignl) or (lineno, orignl, textnl). +func (t TestObjectCreator) NewRawLines(args ...interface{}) []*RawLine { + rawlines := make([]*RawLine, len(args)/2) + j := 0 + for i := 0; i < len(args); i += 2 { + lineno := args[i].(int) + orignl := args[i+1].(string) + textnl := orignl + if i+2 < len(args) { + if s, ok := args[i+2].(string); ok { + textnl = s + i++ + } + } + rawlines[j] = &RawLine{lineno, orignl, textnl} + j++ + } + return rawlines[:j] +} + +func (t TestObjectCreator) NewLine(filename string, lineno int, text string) Line { + textnl := text + "\n" + rawLine := RawLine{lineno, textnl, textnl} + return NewLine(filename, lineno, text, []*RawLine{&rawLine}) +} + +func (t TestObjectCreator) NewMkLine(fileName string, lineno int, text string) MkLine { + return NewMkLine(t.NewLine(fileName, lineno, text)) +} + +func (t TestObjectCreator) NewShellLine(fileName string, lineno int, text string) *ShellLine { + return NewShellLine(t.NewMkLine(fileName, lineno, text)) +} + +// NewLines generates a slice of simple lines, +// i.e. each logical line has exactly one physical line. +// To work with line continuations like in Makefiles, +// use Suite.CreateTmpFileLines together with Suite.LoadExistingLines. +func (t TestObjectCreator) NewLines(fname string, texts ...string) []Line { + return t.NewLinesAt(fname, 1, texts...) +} + +func (t TestObjectCreator) NewMkLines(fname string, lines ...string) *MkLines { + return NewMkLines(t.NewLines(fname, lines...)) +} + +// NewLinesAt generates a slice of simple lines, +// i.e. each logical line has exactly one physical line. +// To work with line continuations like in Makefiles, +// use Suite.CreateTmpFileLines together with Suite.LoadExistingLines. +func (t TestObjectCreator) NewLinesAt(fname string, firstLine int, texts ...string) []Line { + result := make([]Line, len(texts)) + for i, text := range texts { + textnl := text + "\n" + result[i] = NewLine(fname, i+firstLine, text, t.NewRawLines(i+firstLine, textnl)) + } + return result +} diff --git a/pkgtools/pkglint/files/distinfo.go b/pkgtools/pkglint/files/distinfo.go index e2e3895af63..cc19592423d 100644 --- a/pkgtools/pkglint/files/distinfo.go +++ b/pkgtools/pkglint/files/distinfo.go @@ -129,9 +129,11 @@ func (ck *distinfoLinesChecker) checkPatchSha1(line Line, patchFname, distinfoSh return } if distinfoSha1Hex != fileSha1Hex { - if !line.AutofixReplace(distinfoSha1Hex, fileSha1Hex) { - line.Errorf("%s hash of %s differs (distinfo has %s, patch file has %s). Run \"%s makepatchsum\".", "SHA1", patchFname, distinfoSha1Hex, fileSha1Hex, confMake) - } + fix := line.Autofix() + fix.Errorf("%s hash of %s differs (distinfo has %s, patch file has %s). Run \"%s makepatchsum\".", + "SHA1", patchFname, distinfoSha1Hex, fileSha1Hex, confMake) + fix.Replace(distinfoSha1Hex, fileSha1Hex) + fix.Apply() } } @@ -183,7 +185,10 @@ func AutofixDistinfo(oldSha1, newSha1 string) { distinfoFilename := G.CurrentDir + "/" + G.Pkg.DistinfoFile if lines, err := readLines(distinfoFilename, false); err == nil { for _, line := range lines { - line.AutofixReplace(oldSha1, newSha1) + fix := line.Autofix() + fix.Warnf("Silent-Magic-Diagnostic") + fix.Replace(oldSha1, newSha1) + fix.Apply() } SaveAutofixChanges(lines) } diff --git a/pkgtools/pkglint/files/distinfo_test.go b/pkgtools/pkglint/files/distinfo_test.go index abaabd93486..beff06a94cf 100644 --- a/pkgtools/pkglint/files/distinfo_test.go +++ b/pkgtools/pkglint/files/distinfo_test.go @@ -1,8 +1,6 @@ package main -import ( - check "gopkg.in/check.v1" -) +import "gopkg.in/check.v1" func (s *Suite) Test_ChecklinesDistinfo(c *check.C) { s.Init(c) @@ -13,7 +11,7 @@ func (s *Suite) Test_ChecklinesDistinfo(c *check.C) { "patch contents\n") G.CurrentDir = s.tmpdir - ChecklinesDistinfo(s.NewLines("distinfo", + ChecklinesDistinfo(T.NewLines("distinfo", "should be the RCS ID", "should be empty", "MD5 (distfile.tar.gz) = 12345678901234567890123456789012", @@ -31,11 +29,11 @@ func (s *Suite) Test_ChecklinesDistinfo(c *check.C) { func (s *Suite) Test_ChecklinesDistinfo_global_hash_mismatch(c *check.C) { s.Init(c) - otherLine := NewLine("other/distinfo", 7, "dummy", nil) + otherLine := T.NewLine("other/distinfo", 7, "dummy") G.Hash = make(map[string]*Hash) G.Hash["SHA512:pkgname-1.0.tar.gz"] = &Hash{"asdfasdf", otherLine} - ChecklinesDistinfo(s.NewLines("distinfo", + ChecklinesDistinfo(T.NewLines("distinfo", "$"+"NetBSD$", "", "SHA512 (pkgname-1.0.tar.gz) = 12341234")) @@ -59,7 +57,7 @@ func (s *Suite) Test_ChecklinesDistinfo_uncommitted_patch(c *check.C) { "/distinfo/...\n") G.CurrentDir = s.tmpdir - ChecklinesDistinfo(s.NewLines(s.tmpdir+"/distinfo", + ChecklinesDistinfo(T.NewLines(s.tmpdir+"/distinfo", "$"+"NetBSD$", "", "SHA1 (patch-aa) = 5ad1fb9b3c328fff5caa1a23e8f330e707dd50c0")) @@ -75,7 +73,7 @@ func (s *Suite) Test_ChecklinesDistinfo_unrecorded_patches(c *check.C) { s.CreateTmpFile("patches/patch-src-Makefile", "") G.CurrentDir = s.tmpdir - ChecklinesDistinfo(s.NewLines(s.tmpdir+"/distinfo", + ChecklinesDistinfo(T.NewLines(s.tmpdir+"/distinfo", "$"+"NetBSD$", "", "SHA1 (distfile.tar.gz) = ...", @@ -93,7 +91,7 @@ func (s *Suite) Test_ChecklinesDistinfo_manual_patches(c *check.C) { s.CreateTmpFile("patches/manual-libtool.m4", "") G.CurrentDir = s.tmpdir - ChecklinesDistinfo(s.NewLines(s.tmpdir+"/distinfo", + ChecklinesDistinfo(T.NewLines(s.tmpdir+"/distinfo", "$"+"NetBSD$", "", "SHA1 (patch-aa) = ...")) diff --git a/pkgtools/pkglint/files/expecter.go b/pkgtools/pkglint/files/expecter.go index 9612a0eebb2..608ea69822d 100644 --- a/pkgtools/pkglint/files/expecter.go +++ b/pkgtools/pkglint/files/expecter.go @@ -88,9 +88,10 @@ func (exp *Expecter) ExpectEmptyLine(warnSpace bool) bool { } if warnSpace { - if !exp.CurrentLine().AutofixInsertBefore("") { - exp.CurrentLine().Notef("Empty line expected.") - } + fix := exp.CurrentLine().Autofix() + fix.Notef("Empty line expected.") + fix.InsertBefore("") + fix.Apply() } return false } diff --git a/pkgtools/pkglint/files/files.go b/pkgtools/pkglint/files/files.go index 2609a2b9c01..347642b7a9a 100644 --- a/pkgtools/pkglint/files/files.go +++ b/pkgtools/pkglint/files/files.go @@ -2,8 +2,6 @@ package main import ( "io/ioutil" - "netbsd.org/pkglint/trace" - "os" "strings" ) @@ -138,50 +136,3 @@ func convertToLogicalLines(fname string, rawText string, joinBackslashLines bool return loglines } - -func SaveAutofixChanges(lines []Line) (autofixed bool) { - if trace.Tracing { - defer trace.Call0()() - } - - if !G.opts.Autofix { - for _, line := range lines { - if line.Changed { - G.autofixAvailable = true - } - } - return - } - - changes := make(map[string][]string) - changed := make(map[string]bool) - for _, line := range lines { - if line.Changed { - changed[line.Filename] = true - } - changes[line.Filename] = append(changes[line.Filename], line.modifiedLines()...) - } - - for fname := range changed { - changedLines := changes[fname] - tmpname := fname + ".pkglint.tmp" - text := "" - for _, changedLine := range changedLines { - text += changedLine - } - err := ioutil.WriteFile(tmpname, []byte(text), 0666) - if err != nil { - NewLineWhole(tmpname).Errorf("Cannot write.") - continue - } - err = os.Rename(tmpname, fname) - if err != nil { - NewLineWhole(fname).Errorf("Cannot overwrite with auto-fixed content.") - continue - } - msg := "Has been auto-fixed. Please re-run pkglint." - logs(llAutofix, fname, "", msg, msg) - autofixed = true - } - return -} diff --git a/pkgtools/pkglint/files/files_test.go b/pkgtools/pkglint/files/files_test.go index 094ddc43d00..35d6af515d8 100644 --- a/pkgtools/pkglint/files/files_test.go +++ b/pkgtools/pkglint/files/files_test.go @@ -55,70 +55,3 @@ func (s *Suite) Test_splitRawLine(c *check.C) { c.Check(trailingWhitespace, equals, " ") c.Check(continuation, equals, "\\") } - -func (s *Suite) Test_show_autofix(c *check.C) { - s.Init(c) - s.UseCommandLine("--show-autofix") - fname := s.CreateTmpFile("Makefile", ""+ - "line1\n"+ - "line2\n"+ - "line3\n") - lines := LoadExistingLines(fname, true) - - if !lines[1].AutofixReplaceRegexp(`.`, "X") { - lines[1].Warnf("Something's wrong here.") // Prints the autofix NOTE afterwards - } - SaveAutofixChanges(lines) - - c.Check(lines[1].raw[0].textnl, equals, "XXXXX\n") - c.Check(s.LoadTmpFile("Makefile"), equals, "line1\nline2\nline3\n") - s.CheckOutputLines( - "WARN: ~/Makefile:2: Something's wrong here.", - "AUTOFIX: ~/Makefile:2: Replacing regular expression \".\" with \"X\".") -} - -func (s *Suite) Test_autofix(c *check.C) { - s.Init(c) - s.UseCommandLine("--autofix") - fname := s.CreateTmpFile("Makefile", ""+ - "line1\n"+ - "line2\n"+ - "line3\n") - lines := LoadExistingLines(fname, true) - - if !lines[1].AutofixReplaceRegexp(`.`, "X") { - lines[1].Warnf("Something's wrong here.") // Prints the autofix NOTE afterwards - } - SaveAutofixChanges(lines) - - c.Check(s.LoadTmpFile("Makefile"), equals, "line1\nXXXXX\nline3\n") - s.CheckOutputLines( - "AUTOFIX: ~/Makefile:2: Replacing regular expression \".\" with \"X\".", - "AUTOFIX: ~/Makefile: Has been auto-fixed. Please re-run pkglint.") -} - -func (s *Suite) Test_autofix_MkLines(c *check.C) { - s.Init(c) - s.UseCommandLine("--autofix") - fname := s.CreateTmpFile("Makefile", ""+ - "line1 := value1\n"+ - "line2 := value2\n"+ - "line3 := value3\n") - pkg := NewPackage("category/basename") - G.Pkg = pkg - mklines := pkg.loadPackageMakefile(fname) - G.Pkg = nil - - if !mklines.mklines[1].AutofixReplaceRegexp(`.`, "X") { - mklines.mklines[1].Warnf("Something's wrong here.") // Prints the autofix NOTE afterwards - } - SaveAutofixChanges(mklines.lines) - - c.Check(s.LoadTmpFile("Makefile"), equals, ""+ - "line1 := value1\n"+ - "XXXXXXXXXXXXXXX\n"+ - "line3 := value3\n") - s.CheckOutputLines( - "AUTOFIX: ~/Makefile:2: Replacing regular expression \".\" with \"X\".", - "AUTOFIX: ~/Makefile: Has been auto-fixed. Please re-run pkglint.") -} diff --git a/pkgtools/pkglint/files/getopt/getopt.go b/pkgtools/pkglint/files/getopt/getopt.go index eda37a3cf13..cfc997031a5 100644 --- a/pkgtools/pkglint/files/getopt/getopt.go +++ b/pkgtools/pkglint/files/getopt/getopt.go @@ -31,6 +31,12 @@ func (o *Options) AddFlagVar(shortName rune, longName string, pflag *bool, defva o.options = append(o.options, opt) } +func (o *Options) AddStrList(shortName rune, longName string, plist *[]string, description string) { + *plist = []string{} + opt := &option{shortName, longName, "", description, plist} + o.options = append(o.options, opt) +} + func (o *Options) Parse(args []string) (remainingArgs []string, err error) { var skip int for i := 1; i < len(args) && err == nil; i++ { @@ -101,6 +107,16 @@ func (o *Options) handleLongOption(args []string, i int, opt *option, argval *st } } return 0, nil + case *[]string: + if argval != nil { + *data = append(*data, *argval) + return 0, nil + } else if i+1 < len(args) { + *data = append(*data, args[i+1]) + return 1, nil + } else { + return 0, optErr("option requires an argument: --" + opt.longName) + } case *FlagGroup: if argval == nil { return 1, data.parse("--"+opt.longName+"=", args[i+1]) @@ -120,6 +136,19 @@ optchar: case *bool: *data = true continue optchar + + case *[]string: + argarg := optchars[ai+utf8.RuneLen(optchar):] + if argarg != "" { + *data = append(*data, argarg) + return 0, nil + } else if i+1 < len(args) { + *data = append(*data, argarg) + return 1, nil + } else { + return 0, optErr("option requires an argument: -" + string([]rune{optchar})) + } + case *FlagGroup: argarg := optchars[ai+utf8.RuneLen(optchar):] if argarg != "" { diff --git a/pkgtools/pkglint/files/getopt/getopt_test.go b/pkgtools/pkglint/files/getopt/getopt_test.go index 509d158d414..c3b0137df73 100644 --- a/pkgtools/pkglint/files/getopt/getopt_test.go +++ b/pkgtools/pkglint/files/getopt/getopt_test.go @@ -104,3 +104,34 @@ func (s *Suite) Test_Options_Parse_mixed_args_and_options(c *check.C) { c.Check(aflag, check.Equals, true) c.Check(bflag, check.Equals, false) } + +func (s *Suite) Test_Options_Parse_string_list(c *check.C) { + opts := NewOptions() + var verbose bool + var includes []string + var excludes []string + opts.AddStrList('e', "exclude", &excludes, "") + opts.AddStrList('i', "include", &includes, "") + opts.AddFlagVar('v', "verbose", &verbose, false, "") + + args, err := opts.Parse([]string{"progname", + "-viincluded1", + "--include=included2", + "--include", "included3", + "-eexcluded1", + "--exclude=excluded2", + "--exclude", "excluded3"}) + + c.Check(args, check.IsNil) + c.Check(err, check.IsNil) + c.Check(includes, check.DeepEquals, []string{"included1", "included2", "included3"}) + c.Check(excludes, check.DeepEquals, []string{"excluded1", "excluded2", "excluded3"}) + + args, err = opts.Parse([]string{"progname", "-i"}) + + c.Check(err.Error(), check.Equals, "progname: option requires an argument: -i") + + args, err = opts.Parse([]string{"progname", "--include"}) + + c.Check(err.Error(), check.Equals, "progname: option requires an argument: --include") +} diff --git a/pkgtools/pkglint/files/globaldata_test.go b/pkgtools/pkglint/files/globaldata_test.go index 9cff067f77a..2fd145eb73b 100644 --- a/pkgtools/pkglint/files/globaldata_test.go +++ b/pkgtools/pkglint/files/globaldata_test.go @@ -1,7 +1,7 @@ package main import ( - check "gopkg.in/check.v1" + "gopkg.in/check.v1" "netbsd.org/pkglint/trace" ) @@ -13,7 +13,7 @@ func (s *Suite) Test_GlobalData_InitVartypes(c *check.C) { } func (s *Suite) Test_parselinesSuggestedUpdates(c *check.C) { - lines := s.NewLines("doc/TODO", + lines := T.NewLines("doc/TODO", "", "Suggested package updates", "==============", @@ -102,9 +102,9 @@ func (s *Suite) Test_GlobalData_loadDocChangesFromFile(c *check.C) { func (s *Suite) Test_GlobalData_deprecated(c *check.C) { s.Init(c) G.globalData.loadDeprecatedVars() + mkline := T.NewMkLine("Makefile", 5, "USE_PERL5=\tyes") - line := NewLine("Makefile", 5, "USE_PERL5=\tyes", nil) - MkLineChecker{NewMkLine(line)}.checkVarassign() + MkLineChecker{mkline}.checkVarassign() s.CheckOutputLines( "WARN: Makefile:5: Definition of USE_PERL5 is deprecated. Use USE_TOOLS+=perl or USE_TOOLS+=perl:run instead.") diff --git a/pkgtools/pkglint/files/globalvars.go b/pkgtools/pkglint/files/globalvars.go index 40b1842b13d..8379fc095ce 100644 --- a/pkgtools/pkglint/files/globalvars.go +++ b/pkgtools/pkglint/files/globalvars.go @@ -1,7 +1,6 @@ package main import ( - "io" "netbsd.org/pkglint/histogram" ) @@ -30,8 +29,8 @@ type GlobalVars struct { explanationsAvailable bool explanationsGiven map[string]bool autofixAvailable bool - logOut io.Writer - logErr io.Writer + logOut *SeparatorWriter + logErr *SeparatorWriter loghisto *histogram.Histogram } @@ -76,6 +75,8 @@ type CmdOpts struct { PrintSource, PrintVersion bool + LogOnly []string + args []string } diff --git a/pkgtools/pkglint/files/licenses_test.go b/pkgtools/pkglint/files/licenses_test.go index ba1ee4268e7..7146b06a6d1 100644 --- a/pkgtools/pkglint/files/licenses_test.go +++ b/pkgtools/pkglint/files/licenses_test.go @@ -7,7 +7,7 @@ import ( func (s *Suite) Test_checklineLicense(c *check.C) { s.Init(c) s.CreateTmpFile("licenses/gnu-gpl-v2", "Most software \u2026") - mkline := NewMkLine(NewLine("Makefile", 7, "LICENSE=dummy", nil)) + mkline := T.NewMkLine("Makefile", 7, "LICENSE=dummy") G.globalData.Pkgsrcdir = s.tmpdir G.CurrentDir = s.tmpdir diff --git a/pkgtools/pkglint/files/line.go b/pkgtools/pkglint/files/line.go index 37896088251..2d29e448bab 100644 --- a/pkgtools/pkglint/files/line.go +++ b/pkgtools/pkglint/files/line.go @@ -15,11 +15,8 @@ package main import ( "fmt" - "io" - "netbsd.org/pkglint/regex" "path" "strconv" - "strings" ) type Line = *LineImpl @@ -35,15 +32,12 @@ func (rline *RawLine) String() string { } type LineImpl struct { - Filename string - firstLine int32 // Zero means not applicable, -1 means EOF - lastLine int32 // Usually the same as firstLine, may differ in Makefiles - Text string - raw []*RawLine - Changed bool - before []string - after []string - autofixMessage string + Filename string + firstLine int32 // Zero means not applicable, -1 means EOF + lastLine int32 // Usually the same as firstLine, may differ in Makefiles + Text string + raw []*RawLine + autofix *Autofix } func NewLine(fname string, lineno int, text string, rawLines []*RawLine) Line { @@ -52,7 +46,7 @@ func NewLine(fname string, lineno int, text string, rawLines []*RawLine) Line { // NewLineMulti is for logical Makefile lines that end with backslash. func NewLineMulti(fname string, firstLine, lastLine int, text string, rawLines []*RawLine) Line { - return &LineImpl{fname, int32(firstLine), int32(lastLine), text, rawLines, false, nil, nil, ""} + return &LineImpl{fname, int32(firstLine), int32(lastLine), text, rawLines, nil} } // NewLineEOF creates a dummy line for logging, with the "line number" EOF. @@ -65,17 +59,6 @@ func NewLineWhole(fname string) Line { return NewLine(fname, 0, "", nil) } -// modifiedLines returns the text after the fixes, including line breaks and newly inserted lines -func (line *LineImpl) modifiedLines() []string { - var result []string - result = append(result, line.before...) - for _, raw := range line.raw { - result = append(result, raw.textnl) - } - result = append(result, line.after...) - return result -} - func (line *LineImpl) Linenos() string { switch { case line.firstLine == -1: @@ -100,132 +83,91 @@ func (line *LineImpl) IsMultiline() bool { return line.firstLine > 0 && line.firstLine != line.lastLine } -func (line *LineImpl) printSource(out io.Writer) { - if G.opts.PrintSource { - io.WriteString(out, "\n") - for _, before := range line.before { - io.WriteString(out, "+ "+before) - } - for _, rawLine := range line.raw { +func (line *LineImpl) printSource(out *SeparatorWriter) { + if !G.opts.PrintSource { + return + } + + printDiff := func(rawLines []*RawLine) { + for _, rawLine := range rawLines { if rawLine.textnl != rawLine.orignl { if rawLine.orignl != "" { - io.WriteString(out, "- "+rawLine.orignl) + out.Write("- " + rawLine.orignl) } if rawLine.textnl != "" { - io.WriteString(out, "+ "+rawLine.textnl) + out.Write("+ " + rawLine.textnl) } } else { - io.WriteString(out, "> "+rawLine.orignl) + out.Write("> " + rawLine.orignl) } } - for _, after := range line.after { - io.WriteString(out, "+ "+after) + } + + if line.autofix != nil { + for _, before := range line.autofix.linesBefore { + out.Write("+ " + before) + } + printDiff(line.autofix.lines) + for _, after := range line.autofix.linesAfter { + out.Write("+ " + after) } + } else { + printDiff(line.raw) + } +} + +func (line *LineImpl) log(level *LogLevel, format string, args []interface{}) { + if G.opts.PrintAutofix || G.opts.Autofix { + // In these two cases, the only interesting diagnostics are + // those that can be fixed automatically. + // These are logged by Autofix.Apply. + return + } + if !shallBeLogged(format) { + return + } + + out := G.logOut + if level == llError { + out = G.logErr + } + + logs(level, line.Filename, line.Linenos(), format, fmt.Sprintf(format, args...)) + if !G.opts.PrintAutofix && G.opts.PrintSource { + line.printSource(out) + out.Separate() } } func (line *LineImpl) Fatalf(format string, args ...interface{}) { - line.printSource(G.logErr) - logs(llFatal, line.Filename, line.Linenos(), format, fmt.Sprintf(format, args...)) + line.log(llFatal, format, args) } func (line *LineImpl) Errorf(format string, args ...interface{}) { - line.printSource(G.logOut) - logs(llError, line.Filename, line.Linenos(), format, fmt.Sprintf(format, args...)) - line.logAutofix() + line.log(llError, format, args) } func (line *LineImpl) Warnf(format string, args ...interface{}) { - line.printSource(G.logOut) - logs(llWarn, line.Filename, line.Linenos(), format, fmt.Sprintf(format, args...)) - line.logAutofix() + line.log(llWarn, format, args) } func (line *LineImpl) Notef(format string, args ...interface{}) { - line.printSource(G.logOut) - logs(llNote, line.Filename, line.Linenos(), format, fmt.Sprintf(format, args...)) - line.logAutofix() + line.log(llNote, format, args) } func (line *LineImpl) String() string { return line.Filename + ":" + line.Linenos() + ": " + line.Text } -func (line *LineImpl) logAutofix() { - if line.autofixMessage != "" { - logs(llAutofix, line.Filename, line.Linenos(), "%s", line.autofixMessage) - line.autofixMessage = "" - } -} - -func (line *LineImpl) AutofixInsertBefore(text string) bool { - if G.opts.PrintAutofix || G.opts.Autofix { - line.before = append(line.before, text+"\n") - } - return line.RememberAutofix("Inserting a line %q before this line.", text) -} - -func (line *LineImpl) AutofixInsertAfter(text string) bool { - if G.opts.PrintAutofix || G.opts.Autofix { - line.after = append(line.after, text+"\n") - } - return line.RememberAutofix("Inserting a line %q after this line.", text) -} - -func (line *LineImpl) AutofixDelete() bool { - if G.opts.PrintAutofix || G.opts.Autofix { - for _, rawLine := range line.raw { - rawLine.textnl = "" - } - } - return line.RememberAutofix("Deleting this line.") -} - -func (line *LineImpl) AutofixReplace(from, to string) bool { - for _, rawLine := range line.raw { - if rawLine.Lineno != 0 { - if replaced := strings.Replace(rawLine.textnl, from, to, 1); replaced != rawLine.textnl { - if G.opts.PrintAutofix || G.opts.Autofix { - rawLine.textnl = replaced - } - return line.RememberAutofix("Replacing %q with %q.", from, to) - } - } - } - return false -} - -func (line *LineImpl) AutofixReplaceRegexp(from regex.Pattern, to string) bool { - for _, rawLine := range line.raw { - if rawLine.Lineno != 0 { - if replaced := regex.Compile(from).ReplaceAllString(rawLine.textnl, to); replaced != rawLine.textnl { - if G.opts.PrintAutofix || G.opts.Autofix { - rawLine.textnl = replaced - } - return line.RememberAutofix("Replacing regular expression %q with %q.", from, to) - } - } - } - return false -} - -func (line *LineImpl) RememberAutofix(format string, args ...interface{}) (hasBeenFixed bool) { - if line.firstLine < 1 { - return false - } - line.Changed = true - if G.opts.Autofix { - logs(llAutofix, line.Filename, line.Linenos(), format, fmt.Sprintf(format, args...)) - return true - } - if G.opts.PrintAutofix { - line.autofixMessage = fmt.Sprintf(format, args...) +// Autofix returns a builder object for automatically fixing the line. +// After building the object, call Apply to actually apply the changes to the line. +// +// The changed lines are not written back to disk immediately. +// This is done by SaveAutofixChanges. +// +func (line *LineImpl) Autofix() *Autofix { + if line.autofix == nil { + line.autofix = NewAutofix(line) } - return false -} - -func (line *LineImpl) AutofixMark(reason string) { - line.RememberAutofix(reason) - line.logAutofix() - line.Changed = true + return line.autofix } diff --git a/pkgtools/pkglint/files/line_test.go b/pkgtools/pkglint/files/line_test.go deleted file mode 100644 index dbd5a55de32..00000000000 --- a/pkgtools/pkglint/files/line_test.go +++ /dev/null @@ -1,110 +0,0 @@ -package main - -import ( - "gopkg.in/check.v1" -) - -func (s *Suite) Test_Line_modifications(c *check.C) { - s.Init(c) - s.UseCommandLine("--show-autofix") - - line := NewLine("fname", 1, "dummy", s.NewRawLines(1, "original\n")) - - c.Check(line.Changed, equals, false) - c.Check(line.raw, check.DeepEquals, s.NewRawLines(1, "original\n")) - - line.AutofixReplaceRegexp(`(.)(.*)(.)`, "$3$2$1") - - c.Check(line.Changed, equals, true) - c.Check(line.raw, check.DeepEquals, s.NewRawLines(1, "original\n", "lriginao\n")) - - line.Changed = false - line.AutofixReplace("i", "u") - - c.Check(line.Changed, equals, true) - c.Check(line.raw, check.DeepEquals, s.NewRawLines(1, "original\n", "lruginao\n")) - c.Check(line.raw[0].textnl, equals, "lruginao\n") - - line.Changed = false - line.AutofixReplace("lruginao", "middle") - - c.Check(line.Changed, equals, true) - c.Check(line.raw, check.DeepEquals, s.NewRawLines(1, "original\n", "middle\n")) - c.Check(line.raw[0].textnl, equals, "middle\n") - - line.AutofixInsertBefore("before") - line.AutofixInsertBefore("between before and middle") - line.AutofixInsertAfter("between middle and after") - line.AutofixInsertAfter("after") - - c.Check(line.modifiedLines(), check.DeepEquals, []string{ - "before\n", - "between before and middle\n", - "middle\n", - "between middle and after\n", - "after\n"}) - - line.AutofixDelete() - - c.Check(line.modifiedLines(), check.DeepEquals, []string{ - "before\n", - "between before and middle\n", - "", - "between middle and after\n", - "after\n"}) -} - -func (s *Suite) Test_Line_show_autofix_AutofixReplace(c *check.C) { - s.Init(c) - s.UseCommandLine("--show-autofix", "--source") - line := NewLineMulti("Makefile", 27, 29, "# old", s.NewRawLines( - 27, "before\n", - 28, "The old song\n", - 29, "after\n")) - - if !line.AutofixReplace("old", "new") { - line.Warnf("Using \"old\" is deprecated.") - } - - s.CheckOutputLines( - "", - "> before", - "- The old song", - "+ The new song", - "> after", - "WARN: Makefile:27--29: Using \"old\" is deprecated.", - "AUTOFIX: Makefile:27--29: Replacing \"old\" with \"new\".") -} - -func (s *Suite) Test_Line_show_autofix_AutofixInsertBefore(c *check.C) { - s.Init(c) - s.UseCommandLine("--show-autofix", "--source") - line := NewLine("Makefile", 30, "original", s.NewRawLines(30, "original\n")) - - if !line.AutofixInsertBefore("inserted") { - line.Warnf("Dummy") - } - - s.CheckOutputLines( - "", - "+ inserted", - "> original", - "WARN: Makefile:30: Dummy", - "AUTOFIX: Makefile:30: Inserting a line \"inserted\" before this line.") -} - -func (s *Suite) Test_Line_show_autofix_AutofixDelete(c *check.C) { - s.Init(c) - s.UseCommandLine("--show-autofix", "--source") - line := NewLine("Makefile", 30, "to be deleted", s.NewRawLines(30, "to be deleted\n")) - - if !line.AutofixDelete() { - line.Warnf("Dummy") - } - - s.CheckOutputLines( - "", - "- to be deleted", - "WARN: Makefile:30: Dummy", - "AUTOFIX: Makefile:30: Deleting this line.") -} diff --git a/pkgtools/pkglint/files/linechecker.go b/pkgtools/pkglint/files/linechecker.go index 8db34873153..37aaa989e4f 100644 --- a/pkgtools/pkglint/files/linechecker.go +++ b/pkgtools/pkglint/files/linechecker.go @@ -49,12 +49,13 @@ func CheckLineValidCharacters(line Line, reChar regex.Pattern) { func CheckLineTrailingWhitespace(line Line) { if strings.HasSuffix(line.Text, " ") || strings.HasSuffix(line.Text, "\t") { - if !line.AutofixReplaceRegexp(`\s+\n$`, "\n") { - line.Notef("Trailing white-space.") - Explain( - "When a line ends with some white-space, that space is in most cases", - "irrelevant and can be removed.") - } + fix := line.Autofix() + fix.Notef("Trailing white-space.") + fix.Explain( + "When a line ends with some white-space, that space is in most cases", + "irrelevant and can be removed.") + fix.ReplaceRegex(`\s+\n$`, "\n") + fix.Apply() } } @@ -67,13 +68,15 @@ func CheckLineRcsid(line Line, prefixRe regex.Pattern, suggestedPrefix string) b return true } - if !line.AutofixInsertBefore(suggestedPrefix + "$" + "NetBSD$") { - line.Errorf("Expected %q.", suggestedPrefix+"$"+"NetBSD$") - Explain( - "Several files in pkgsrc must contain the CVS Id, so that their", - "current version can be traced back later from a binary package.", - "This is to ensure reproducible builds, for example for finding bugs.") - } + fix := line.Autofix() + fix.Errorf("Expected %q.", suggestedPrefix+"$"+"NetBSD$") + fix.Explain( + "Several files in pkgsrc must contain the CVS Id, so that their", + "current version can be traced back later from a binary package.", + "This is to ensure reproducible builds, for example for finding bugs.") + fix.InsertBefore(suggestedPrefix + "$" + "NetBSD$") + fix.Apply() + return false } diff --git a/pkgtools/pkglint/files/linechecker_test.go b/pkgtools/pkglint/files/linechecker_test.go index 8dd3bc84d9b..8eeeccabf0d 100644 --- a/pkgtools/pkglint/files/linechecker_test.go +++ b/pkgtools/pkglint/files/linechecker_test.go @@ -6,7 +6,7 @@ import ( func (s *Suite) Test_LineChecker_CheckAbsolutePathname(c *check.C) { s.Init(c) - line := NewLine("Makefile", 1, "# dummy", nil) + line := T.NewLine("Makefile", 1, "# dummy") CheckLineAbsolutePathname(line, "bindir=/bin") CheckLineAbsolutePathname(line, "bindir=/../lib") @@ -17,7 +17,7 @@ func (s *Suite) Test_LineChecker_CheckAbsolutePathname(c *check.C) { func (s *Suite) Test_LineChecker_CheckTrailingWhitespace(c *check.C) { s.Init(c) - line := NewLine("Makefile", 32, "The line must go on ", nil) + line := T.NewLine("Makefile", 32, "The line must go on ") CheckLineTrailingWhitespace(line) @@ -27,7 +27,7 @@ func (s *Suite) Test_LineChecker_CheckTrailingWhitespace(c *check.C) { func (s *Suite) Test_LineChecker_CheckRcsid(c *check.C) { s.Init(c) - lines := s.NewLines("fname", + lines := T.NewLines("fname", "$"+"NetBSD: dummy $", "$"+"NetBSD$", "$"+"Id: dummy $", diff --git a/pkgtools/pkglint/files/logging.go b/pkgtools/pkglint/files/logging.go index 81ea2af3b9d..35efaf08b34 100644 --- a/pkgtools/pkglint/files/logging.go +++ b/pkgtools/pkglint/files/logging.go @@ -22,17 +22,34 @@ var ( var dummyLine = NewLine("", 0, "", nil) -func shallBeLogged(fname, lineno, msg string) bool { +func shallBeLogged(msg string) bool { + if len(G.opts.LogOnly) > 0 { + found := false + for _, substr := range G.opts.LogOnly { + if contains(msg, substr) { + found = true + break + } + } + if !found { + return false + } + } + + return true +} + +func loggedAlready(fname, lineno, msg string) bool { uniq := path.Clean(fname) + ":" + lineno + ":" + msg if G.logged[uniq] { - return false + return true } if G.logged == nil { G.logged = make(map[string]bool) } G.logged[uniq] = true - return true + return false } func logs(level *LogLevel, fname, lineno, format, msg string) bool { @@ -40,7 +57,7 @@ func logs(level *LogLevel, fname, lineno, format, msg string) bool { fname = cleanpath(fname) } - if !G.opts.LogVerbose && !shallBeLogged(fname, lineno, msg) { + if !G.opts.LogVerbose && loggedAlready(fname, lineno, msg) { return false } @@ -60,7 +77,7 @@ func logs(level *LogLevel, fname, lineno, format, msg string) bool { text += sep + level.GccName + ":" sep = " " } - if G.opts.Profiling { + if G.opts.Profiling && format != "" { G.loghisto.Add(format, 1) } text += sep + msg + "\n" @@ -70,7 +87,7 @@ func logs(level *LogLevel, fname, lineno, format, msg string) bool { out = G.logErr } - io.WriteString(out, text) + out.Write(text) switch level { case llFatal: @@ -94,11 +111,11 @@ func Explain(explanation ...string) { } G.explanationsGiven[complete] = true - io.WriteString(G.logOut, "\n") + G.logOut.WriteLine("") for _, explanationLine := range explanation { - io.WriteString(G.logOut, "\t"+explanationLine+"\n") + G.logOut.WriteLine("\t" + explanationLine) } - io.WriteString(G.logOut, "\n") + G.logOut.WriteLine("") } if G.Testing { @@ -118,3 +135,39 @@ func Explain(explanation ...string) { } type pkglintFatal struct{} + +// SeparatorWriter writes output, occasionally separated by an +// empty line. This is used for layouting the diagnostics in +// --source mode combined with --show-autofix, where each +// log message consists of multiple lines. +type SeparatorWriter struct { + out io.Writer + needSeparator bool + wroteSomething bool +} + +func NewSeparatorWriter(out io.Writer) *SeparatorWriter { + return &SeparatorWriter{out, false, false} +} + +func (wr *SeparatorWriter) WriteLine(text string) { + wr.Write(text) + io.WriteString(wr.out, "\n") +} + +func (wr *SeparatorWriter) Write(text string) { + if wr.needSeparator && wr.wroteSomething { + io.WriteString(wr.out, "\n") + wr.needSeparator = false + } + io.WriteString(wr.out, text) + wr.wroteSomething = true +} + +func (wr *SeparatorWriter) Printf(format string, args ...interface{}) { + wr.Write(fmt.Sprintf(format, args...)) +} + +func (wr *SeparatorWriter) Separate() { + wr.needSeparator = true +} diff --git a/pkgtools/pkglint/files/logging_test.go b/pkgtools/pkglint/files/logging_test.go new file mode 100644 index 00000000000..046ecedde87 --- /dev/null +++ b/pkgtools/pkglint/files/logging_test.go @@ -0,0 +1,140 @@ +package main + +import "gopkg.in/check.v1" + +// Since the --source option generates multi-line diagnostics, +// they are separated by an empty line. +// +// The quoted source code is written below the diagnostics. +// In the --show-autofix and --autofix modes, this order +// is the most useful since it first states the general rule, +// then states how to fix this instance and then shows a concrete +// example. Understanding the general rule is considered most +// important of these three. +// +// To keep the output layout consistent between all these +// modes, the source code is written below the diagnostic +// even in the default (check-only) mode, for consistency. +func (s *Suite) Test_show_source_separator(c *check.C) { + s.Init(c) + s.UseCommandLine("--source") + fileName := s.CreateTmpFileLines("DESCR", + "The first line", + "The second line", + "The third line") + lines := LoadExistingLines(fileName, true) + + fix := lines[1].Autofix() + fix.Warnf("Using \"second\" is deprecated.") + fix.Replace("second", "silver medal") + fix.Apply() + + lines[2].Warnf("Dummy warning.") + + fix = lines[2].Autofix() + fix.Warnf("Using \"third\" is deprecated.") + fix.Replace("third", "bronze medal") + fix.Apply() + + s.CheckOutputLines( + "WARN: ~/DESCR:2: Using \"second\" is deprecated.", + "> The second line", + "", + "WARN: ~/DESCR:3: Dummy warning.", + "> The third line", + "", + "WARN: ~/DESCR:3: Using \"third\" is deprecated.", + "> The third line") +} + +func (s *Suite) Test_show_source_separator_show_autofix(c *check.C) { + s.Init(c) + s.UseCommandLine("--source", "--show-autofix") + fileName := s.CreateTmpFileLines("DESCR", + "The first line", + "The second line", + "The third line") + lines := LoadExistingLines(fileName, true) + + fix := lines[1].Autofix() + fix.Warnf("Using \"second\" is deprecated.") + fix.Replace("second", "silver medal") + fix.Apply() + + lines[2].Warnf("Dummy warning.") + + fix = lines[2].Autofix() + fix.Warnf("Using \"third\" is deprecated.") + fix.Replace("third", "bronze medal") + fix.Apply() + + s.CheckOutputLines( + "WARN: ~/DESCR:2: Using \"second\" is deprecated.", + "AUTOFIX: ~/DESCR:2: Replacing \"second\" with \"silver medal\".", + "- The second line", + "+ The silver medal line", + "", + "WARN: ~/DESCR:3: Using \"third\" is deprecated.", + "AUTOFIX: ~/DESCR:3: Replacing \"third\" with \"bronze medal\".", + "- The third line", + "+ The bronze medal line") +} + +func (s *Suite) Test_show_source_separator_autofix(c *check.C) { + s.Init(c) + s.UseCommandLine("--source", "--autofix") + fileName := s.CreateTmpFileLines("DESCR", + "The first line", + "The second line", + "The third line") + lines := LoadExistingLines(fileName, true) + + fix := lines[1].Autofix() + fix.Warnf("Using \"second\" is deprecated.") + fix.Replace("second", "silver medal") + fix.Apply() + + lines[2].Warnf("Dummy warning.") + + fix = lines[2].Autofix() + fix.Warnf("Using \"third\" is deprecated.") + fix.Replace("third", "bronze medal") + fix.Apply() + + s.CheckOutputLines( + "AUTOFIX: ~/DESCR:2: Replacing \"second\" with \"silver medal\".", + "- The second line", + "+ The silver medal line", + "", + "AUTOFIX: ~/DESCR:3: Replacing \"third\" with \"bronze medal\".", + "- The third line", + "+ The bronze medal line") +} + +// Demonstrates how to filter log messages. +// This is useful in combination with the --autofix option, +// to restrict the fixes to exactly one group or topic. +func (s *Suite) Test_Line_log_only(c *check.C) { + s.Init(c) + s.UseCommandLine("--autofix", "--source", "--only", "interesting") + line := NewLineMulti("Makefile", 27, 29, "Dummy text", T.NewRawLines( + 27, "before\n", + 28, "The old song\n", + 29, "after\n")) + + fix := line.Autofix() + fix.Warnf("Using \"old\" is deprecated.") + fix.Replace("old", "new1") + fix.Apply() + + fix.Warnf("Using \"old\" is interesting.") + fix.Replace("old", "new2") + fix.Apply() + + s.CheckOutputLines( + "AUTOFIX: Makefile:27--29: Replacing \"old\" with \"new2\".", + "> before", + "- The old song", + "+ The new2 song", + "> after") +} diff --git a/pkgtools/pkglint/files/mkline.go b/pkgtools/pkglint/files/mkline.go index f0772aced2a..a1aec16c9a8 100644 --- a/pkgtools/pkglint/files/mkline.go +++ b/pkgtools/pkglint/files/mkline.go @@ -67,9 +67,10 @@ func NewMkLine(line Line) (mkline *MkLineImpl) { case matches(varname, `^[a-z]`) && op == ":=": break default: - if !line.AutofixReplace(varname+spaceAfterVarname+op, varname+op) { - line.Warnf("Unnecessary space after variable name %q.", varname) - } + fix := line.Autofix() + fix.Warnf("Unnecessary space after variable name %q.", varname) + fix.Replace(varname+spaceAfterVarname+op, varname+op) + fix.Apply() } } diff --git a/pkgtools/pkglint/files/mkline_test.go b/pkgtools/pkglint/files/mkline_test.go index c2c07ebd4c9..b775b1e1e42 100644 --- a/pkgtools/pkglint/files/mkline_test.go +++ b/pkgtools/pkglint/files/mkline_test.go @@ -1,13 +1,12 @@ package main -import ( - check "gopkg.in/check.v1" -) +import "gopkg.in/check.v1" func (s *Suite) Test_VaralignBlock_Check_autofix(c *check.C) { s.Init(c) - s.UseCommandLine("-Wspace", "-f") - lines := s.NewLines("file.mk", + s.UseCommandLine("-Wspace", "--show-autofix") + + lines := T.NewLines("file.mk", "VAR= value", // Indentation 7, fixed to 8. "", // "VAR= value", // Indentation 8, fixed to 8. @@ -28,39 +27,25 @@ func (s *Suite) Test_VaralignBlock_Check_autofix(c *check.C) { } varalign.Finish() - c.Check(lines[0].Changed, equals, true) - c.Check(lines[0].raw[0].String(), equals, "1:VAR=\tvalue\n") - c.Check(lines[2].Changed, equals, true) - c.Check(lines[2].raw[0].String(), equals, "3:VAR=\tvalue\n") - c.Check(lines[4].Changed, equals, true) - c.Check(lines[4].raw[0].String(), equals, "5:VAR=\tvalue\n") - c.Check(lines[6].Changed, equals, true) - c.Check(lines[6].raw[0].String(), equals, "7:VAR=\tvalue\n") - c.Check(lines[8].Changed, equals, true) - c.Check(lines[8].raw[0].String(), equals, "9:VAR=\tvalue\n") - c.Check(lines[10].Changed, equals, true) - c.Check(lines[10].raw[0].String(), equals, "11:VAR=\tvalue\n") - c.Check(lines[12].Changed, equals, false) - c.Check(lines[12].raw[0].String(), equals, "13:VAR=\tvalue\n") - c.Check(s.Output(), equals, ""+ - "NOTE: file.mk:1: This variable value should be aligned with tabs, not spaces, to column 9.\n"+ - "AUTOFIX: file.mk:1: Replacing \"VAR= \" with \"VAR=\\t\".\n"+ - "NOTE: file.mk:3: Variable values should be aligned with tabs, not spaces.\n"+ - "AUTOFIX: file.mk:3: Replacing \"VAR= \" with \"VAR=\\t\".\n"+ - "NOTE: file.mk:5: This variable value should be aligned with tabs, not spaces, to column 9.\n"+ - "AUTOFIX: file.mk:5: Replacing \"VAR= \" with \"VAR=\\t\".\n"+ - "NOTE: file.mk:7: Variable values should be aligned with tabs, not spaces.\n"+ - "AUTOFIX: file.mk:7: Replacing \"VAR= \\t\" with \"VAR=\\t\".\n"+ - "NOTE: file.mk:9: Variable values should be aligned with tabs, not spaces.\n"+ - "AUTOFIX: file.mk:9: Replacing \"VAR= \\t\" with \"VAR=\\t\".\n"+ - "NOTE: file.mk:11: This variable value should be aligned with tabs, not spaces, to column 9.\n"+ - "AUTOFIX: file.mk:11: Replacing \"VAR= \\t\" with \"VAR=\\t\".\n") + s.CheckOutputLines( + "NOTE: file.mk:1: This variable value should be aligned with tabs, not spaces, to column 9.", + "AUTOFIX: file.mk:1: Replacing \"VAR= \" with \"VAR=\\t\".", + "NOTE: file.mk:3: Variable values should be aligned with tabs, not spaces.", + "AUTOFIX: file.mk:3: Replacing \"VAR= \" with \"VAR=\\t\".", + "NOTE: file.mk:5: This variable value should be aligned with tabs, not spaces, to column 9.", + "AUTOFIX: file.mk:5: Replacing \"VAR= \" with \"VAR=\\t\".", + "NOTE: file.mk:7: Variable values should be aligned with tabs, not spaces.", + "AUTOFIX: file.mk:7: Replacing \"VAR= \\t\" with \"VAR=\\t\".", + "NOTE: file.mk:9: Variable values should be aligned with tabs, not spaces.", + "AUTOFIX: file.mk:9: Replacing \"VAR= \\t\" with \"VAR=\\t\".", + "NOTE: file.mk:11: This variable value should be aligned with tabs, not spaces, to column 9.", + "AUTOFIX: file.mk:11: Replacing \"VAR= \\t\" with \"VAR=\\t\".") } func (s *Suite) Test_VaralignBlock_Check__reduce_indentation(c *check.C) { s.Init(c) s.UseCommandLine("-Wspace") - mklines := s.NewMkLines("file.mk", + mklines := T.NewMkLines("file.mk", "VAR= \tvalue", "VAR= \tvalue", "VAR=\t\t\t\tvalue", @@ -84,7 +69,7 @@ func (s *Suite) Test_VaralignBlock_Check__reduce_indentation(c *check.C) { func (s *Suite) Test_VaralignBlock_Check_longest_line_no_space(c *check.C) { s.Init(c) s.UseCommandLine("-Wspace") - mklines := s.NewMkLines("file.mk", + mklines := T.NewMkLines("file.mk", "SUBST_CLASSES+= aaaaaaaa", "SUBST_STAGE.aaaaaaaa= pre-configure", "SUBST_FILES.aaaaaaaa= *.pl", @@ -106,7 +91,7 @@ func (s *Suite) Test_VaralignBlock_Check_longest_line_no_space(c *check.C) { func (s *Suite) Test_VaralignBlock_Check_only_spaces(c *check.C) { s.Init(c) s.UseCommandLine("-Wspace") - mklines := s.NewMkLines("file.mk", + mklines := T.NewMkLines("file.mk", "SUBST_CLASSES+= aaaaaaaa", "SUBST_STAGE.aaaaaaaa= pre-configure", "SUBST_FILES.aaaaaaaa= *.pl", @@ -128,7 +113,7 @@ func (s *Suite) Test_VaralignBlock_Check_only_spaces(c *check.C) { func (s *Suite) Test_NewMkLine(c *check.C) { s.Init(c) s.UseCommandLine("-Wspace") - mklines := NewMkLines(s.NewLines("test.mk", + mklines := T.NewMkLines("test.mk", "VARNAME.param?=value # varassign comment", "\tshell command # shell comment", "# whole line comment", @@ -138,7 +123,7 @@ func (s *Suite) Test_NewMkLine(c *check.C) { ". include <subdir.mk> # sysinclude comment", "target1 target2: source1 source2", "target : source", - "VARNAME+=value")) + "VARNAME+=value") ln := mklines.mklines c.Check(ln[0].IsVarassign(), equals, true) @@ -218,7 +203,7 @@ func (s *Suite) Test_NewMkLine__autofix_space_after_varname(c *check.C) { } func (s *Suite) Test_MkLine_VariableType_varparam(c *check.C) { - mkline := NewMkLine(NewLine("fname", 1, "# dummy", nil)) + mkline := T.NewMkLine("fname", 1, "# dummy") G.globalData.InitVartypes() t1 := mkline.VariableType("FONT_DIRS") @@ -234,7 +219,7 @@ func (s *Suite) Test_MkLine_VariableType_varparam(c *check.C) { func (s *Suite) Test_VarUseContext_String(c *check.C) { G.globalData.InitVartypes() - mkline := NewMkLine(NewLine("fname", 1, "# dummy", nil)) + mkline := T.NewMkLine("fname", 1, "# dummy") vartype := mkline.VariableType("PKGNAME") vuc := &VarUseContext{vartype, vucTimeUnknown, vucQuotBackt, false} @@ -246,22 +231,22 @@ func (s *Suite) Test_VarUseContext_String(c *check.C) { // is interpreted literally. func (s *Suite) Test_NewMkLine_numbersign(c *check.C) { s.Init(c) - mklineVarassignEscaped := NewMkLine(NewLine("fname", 1, "SED_CMD=\t's,\\#,hash,g'", nil)) + mklineVarassignEscaped := T.NewMkLine("fname", 1, "SED_CMD=\t's,\\#,hash,g'") c.Check(mklineVarassignEscaped.Varname(), equals, "SED_CMD") c.Check(mklineVarassignEscaped.Value(), equals, "'s,#,hash,g'") - mklineCommandEscaped := NewMkLine(NewLine("fname", 1, "\tsed -e 's,\\#,hash,g'", nil)) + mklineCommandEscaped := T.NewMkLine("fname", 1, "\tsed -e 's,\\#,hash,g'") c.Check(mklineCommandEscaped.Shellcmd(), equals, "sed -e 's,\\#,hash,g'") // From shells/zsh/Makefile.common, rev. 1.78 - mklineCommandUnescaped := NewMkLine(NewLine("fname", 1, "\t# $ sha1 patches/patch-ac", nil)) + mklineCommandUnescaped := T.NewMkLine("fname", 1, "\t# $ sha1 patches/patch-ac") c.Check(mklineCommandUnescaped.Shellcmd(), equals, "# $ sha1 patches/patch-ac") s.CheckOutputEmpty() // No warning about parsing the lonely dollar sign. - mklineVarassignUnescaped := NewMkLine(NewLine("fname", 1, "SED_CMD=\t's,#,hash,'", nil)) + mklineVarassignUnescaped := T.NewMkLine("fname", 1, "SED_CMD=\t's,#,hash,'") c.Check(mklineVarassignUnescaped.Value(), equals, "'s,") s.CheckOutputLines( @@ -270,7 +255,7 @@ func (s *Suite) Test_NewMkLine_numbersign(c *check.C) { func (s *Suite) Test_NewMkLine_leading_space(c *check.C) { s.Init(c) - _ = NewMkLine(NewLine("rubyversion.mk", 427, " _RUBYVER=\t2.15", nil)) + _ = T.NewMkLine("rubyversion.mk", 427, " _RUBYVER=\t2.15") s.CheckOutputLines( "WARN: rubyversion.mk:427: Makefile lines should not start with space characters.") @@ -281,7 +266,7 @@ func (s *Suite) Test_MkLines_Check__extra(c *check.C) { s.UseCommandLine("-Wextra") G.globalData.InitVartypes() G.Pkg = NewPackage("category/pkgbase") - G.Mk = s.NewMkLines("options.mk", + G.Mk = T.NewMkLines("options.mk", mkrcsid, ".for word in ${PKG_FAIL_REASON}", "PYTHON_VERSIONS_ACCEPTED=\t27 35 30", @@ -304,7 +289,7 @@ func (s *Suite) Test_MkLines_Check__extra(c *check.C) { } func (s *Suite) Test_MkLine_variableNeedsQuoting__unknown_rhs(c *check.C) { - mkline := NewMkLine(NewLine("fname", 1, "PKGNAME := ${UNKNOWN}", nil)) + mkline := T.NewMkLine("fname", 1, "PKGNAME := ${UNKNOWN}") G.globalData.InitVartypes() vuc := &VarUseContext{G.globalData.vartypes["PKGNAME"], vucTimeParse, vucQuotUnknown, false} @@ -318,7 +303,7 @@ func (s *Suite) Test_MkLine_variableNeedsQuoting__append_URL_to_list_of_URLs(c * s.UseCommandLine("-Wall") G.globalData.InitVartypes() s.RegisterMasterSite("MASTER_SITE_SOURCEFORGE", "http://downloads.sourceforge.net/sourceforge/") - mkline := NewMkLine(NewLine("Makefile", 95, "MASTER_SITES=\t${HOMEPAGE}", nil)) + mkline := T.NewMkLine("Makefile", 95, "MASTER_SITES=\t${HOMEPAGE}") vuc := &VarUseContext{G.globalData.vartypes["MASTER_SITES"], vucTimeRun, vucQuotPlain, false} nq := mkline.VariableNeedsQuoting("HOMEPAGE", G.globalData.vartypes["HOMEPAGE"], vuc) @@ -336,7 +321,7 @@ func (s *Suite) Test_MkLine_variableNeedsQuoting__append_list_to_list(c *check.C s.UseCommandLine("-Wall") G.globalData.InitVartypes() s.RegisterMasterSite("MASTER_SITE_SOURCEFORGE", "http://downloads.sourceforge.net/sourceforge/") - mkline := NewMkLine(NewLine("Makefile", 96, "MASTER_SITES=\t${MASTER_SITE_SOURCEFORGE:=squirrel-sql/}", nil)) + mkline := T.NewMkLine("Makefile", 96, "MASTER_SITES=\t${MASTER_SITE_SOURCEFORGE:=squirrel-sql/}") MkLineChecker{mkline}.checkVarassign() @@ -347,7 +332,7 @@ func (s *Suite) Test_MkLine_variableNeedsQuoting__eval_shell(c *check.C) { s.Init(c) s.UseCommandLine("-Wall") G.globalData.InitVartypes() - mkline := NewMkLine(NewLine("builtin.mk", 3, "USE_BUILTIN.Xfixes!=\t${PKG_ADMIN} pmatch 'pkg-[0-9]*' ${BUILTIN_PKG.Xfixes:Q}", nil)) + mkline := T.NewMkLine("builtin.mk", 3, "USE_BUILTIN.Xfixes!=\t${PKG_ADMIN} pmatch 'pkg-[0-9]*' ${BUILTIN_PKG.Xfixes:Q}") MkLineChecker{mkline}.checkVarassign() @@ -360,7 +345,7 @@ func (s *Suite) Test_MkLine_variableNeedsQuoting__command_in_single_quotes(c *ch s.Init(c) s.UseCommandLine("-Wall") G.globalData.InitVartypes() - mkline := NewMkLine(NewLine("Makefile", 3, "SUBST_SED.hpath=\t-e 's|^\\(INSTALL[\t:]*=\\).*|\\1${INSTALL}|'", nil)) + mkline := T.NewMkLine("Makefile", 3, "SUBST_SED.hpath=\t-e 's|^\\(INSTALL[\t:]*=\\).*|\\1${INSTALL}|'") MkLineChecker{mkline}.checkVarassign() @@ -375,7 +360,7 @@ func (s *Suite) Test_MkLine_variableNeedsQuoting__command_in_command(c *check.C) s.RegisterTool(&Tool{Name: "find", Varname: "FIND", Predefined: true}) s.RegisterTool(&Tool{Name: "sort", Varname: "SORT", Predefined: true}) G.Pkg = NewPackage("category/pkgbase") - G.Mk = s.NewMkLines("Makefile", + G.Mk = T.NewMkLines("Makefile", mkrcsid, "GENERATE_PLIST= cd ${DESTDIR}${PREFIX}; ${FIND} * \\( -type f -or -type l \\) | ${SORT};") @@ -390,7 +375,7 @@ func (s *Suite) Test_MkLine_variableNeedsQuoting__word_as_part_of_word(c *check. s.Init(c) s.UseCommandLine("-Wall") G.globalData.InitVartypes() - G.Mk = s.NewMkLines("Makefile", + G.Mk = T.NewMkLines("Makefile", mkrcsid, "EGDIR=\t${EGDIR}/${MACHINE_GNU_PLATFORM}") @@ -411,7 +396,7 @@ func (s *Suite) Test_MkLine_variableNeedsQuoting__command_as_command_argument(c s.RegisterTool(&Tool{Name: "perl", Varname: "PERL5", Predefined: true}) s.RegisterTool(&Tool{Name: "bash", Varname: "BASH", Predefined: true}) G.globalData.InitVartypes() - G.Mk = s.NewMkLines("Makefile", + G.Mk = T.NewMkLines("Makefile", mkrcsid, "\t${RUN} cd ${WRKSRC} && ( ${ECHO} ${PERL5:Q} ; ${ECHO} ) | ${BASH} ./install", "\t${RUN} cd ${WRKSRC} && ( ${ECHO} ${PERL5} ; ${ECHO} ) | ${BASH} ./install") @@ -429,7 +414,7 @@ func (s *Suite) Test_MkLine_variableNeedsQuoting__URL_as_part_of_word_in_list(c s.Init(c) s.UseCommandLine("-Wall") G.globalData.InitVartypes() - G.Mk = s.NewMkLines("Makefile", + G.Mk = T.NewMkLines("Makefile", mkrcsid, "MASTER_SITES=${HOMEPAGE}archive/") @@ -449,7 +434,7 @@ func (s *Suite) Test_MkLine_variableNeedsQuoting__command_in_subshell(c *check.C G.globalData.InitVartypes() s.RegisterTool(&Tool{Name: "awk", Varname: "AWK", Predefined: true}) s.RegisterTool(&Tool{Name: "echo", Varname: "ECHO", Predefined: true}) - G.Mk = s.NewMkLines("xpi.mk", + G.Mk = T.NewMkLines("xpi.mk", mkrcsid, "\t id=$$(${AWK} '{print}' < ${WRKSRC}/idfile) && echo \"$$id\"", "\t id=`${AWK} '{print}' < ${WRKSRC}/idfile` && echo \"$$id\"") @@ -468,7 +453,7 @@ func (s *Suite) Test_MkLine_variableNeedsQuoting__LDFLAGS_in_single_quotes(c *ch s.Init(c) s.UseCommandLine("-Wall") G.globalData.InitVartypes() - G.Mk = s.NewMkLines("x11/mlterm/Makefile", + G.Mk = T.NewMkLines("x11/mlterm/Makefile", mkrcsid, "SUBST_SED.link=-e 's|(LIBTOOL_LINK).*(LIBS)|& ${LDFLAGS:M*:Q}|g'", "SUBST_SED.link=-e 's|(LIBTOOL_LINK).*(LIBS)|& '${LDFLAGS:M*:Q}'|g'") @@ -490,7 +475,7 @@ func (s *Suite) Test_MkLine_variableNeedsQuoting__package_options(c *check.C) { s.Init(c) s.UseCommandLine("-Wall") G.globalData.InitVartypes() - G.Mk = s.NewMkLines("Makefile", + G.Mk = T.NewMkLines("Makefile", mkrcsid, "PKG_SUGGESTED_OPTIONS+=\t${PKG_DEFAULT_OPTIONS:Mcdecimal} ${PKG_OPTIONS.py-trytond:Mcdecimal}") @@ -504,7 +489,7 @@ func (s *Suite) Test_MkLines_Check__MASTER_SITE_in_HOMEPAGE(c *check.C) { s.UseCommandLine("-Wall") s.RegisterMasterSite("MASTER_SITE_GITHUB", "https://github.com/") G.globalData.InitVartypes() - G.Mk = s.NewMkLines("devel/catch/Makefile", + G.Mk = T.NewMkLines("devel/catch/Makefile", mkrcsid, "HOMEPAGE=\t${MASTER_SITE_GITHUB:=philsquared/Catch/}", "HOMEPAGE=\t${MASTER_SITE_GITHUB}", @@ -526,7 +511,7 @@ func (s *Suite) Test_MkLine_variableNeedsQuoting__tool_in_quotes_in_subshell_in_ s.RegisterTool(&Tool{Name: "echo", Varname: "ECHO", Predefined: true}) s.RegisterTool(&Tool{Name: "sh", Varname: "SH", Predefined: true}) G.globalData.InitVartypes() - G.Mk = s.NewMkLines("x11/labltk/Makefile", + G.Mk = T.NewMkLines("x11/labltk/Makefile", mkrcsid, "CONFIGURE_ARGS+=\t-tklibs \"`${SH} -c '${ECHO} $$TK_LD_FLAGS'`\"") @@ -539,7 +524,7 @@ func (s *Suite) Test_MkLine_variableNeedsQuoting__LDADD_in_BUILDLINK_TRANSFORM(c s.Init(c) s.UseCommandLine("-Wall") G.globalData.InitVartypes() - G.Mk = s.NewMkLines("x11/qt5-qtbase/Makefile.common", + G.Mk = T.NewMkLines("x11/qt5-qtbase/Makefile.common", "BUILDLINK_TRANSFORM+=opt:-ldl:${BUILDLINK_LDADD.dl:M*}") MkLineChecker{G.Mk.mklines[0]}.Check() @@ -553,7 +538,7 @@ func (s *Suite) Test_MkLine_variableNeedsQuoting__command_in_message(c *check.C) s.Init(c) s.UseCommandLine("-Wall") G.globalData.InitVartypes() - G.Mk = s.NewMkLines("benchmarks/iozone/Makefile", + G.Mk = T.NewMkLines("benchmarks/iozone/Makefile", "SUBST_MESSAGE.crlf=\tStripping EOL CR in ${REPLACE_PERL}") MkLineChecker{G.Mk.mklines[0]}.Check() @@ -565,7 +550,7 @@ func (s *Suite) Test_MkLine_variableNeedsQuoting__guessed_list_variable_in_quote s.Init(c) s.UseCommandLine("-Wall") G.globalData.InitVartypes() - G.Mk = s.NewMkLines("audio/jack-rack/Makefile", + G.Mk = T.NewMkLines("audio/jack-rack/Makefile", mkrcsid, "LADSPA_PLUGIN_PATH?=\t${PREFIX}/lib/ladspa", "CPPFLAGS+=\t\t-DLADSPA_PATH=\"\\\"${LADSPA_PLUGIN_PATH}\\\"\"") @@ -580,7 +565,7 @@ func (s *Suite) Test_MkLine_variableNeedsQuoting__list_in_list(c *check.C) { s.Init(c) s.UseCommandLine("-Wall") G.globalData.InitVartypes() - G.Mk = s.NewMkLines("x11/eterm/Makefile", + G.Mk = T.NewMkLines("x11/eterm/Makefile", mkrcsid, "DISTFILES=\t${DEFAULT_DISTFILES} ${PIXMAP_FILES}") @@ -594,7 +579,7 @@ func (s *Suite) Test_MkLine_variableNeedsQuoting__PKGNAME_and_URL_list_in_URL_li s.UseCommandLine("-Wall") s.RegisterMasterSite("MASTER_SITE_GNOME", "http://ftp.gnome.org/") G.globalData.InitVartypes() - G.Mk = s.NewMkLines("x11/gtk3/Makefile", + G.Mk = T.NewMkLines("x11/gtk3/Makefile", mkrcsid, "MASTER_SITES=\tftp://ftp.gtk.org/${PKGNAME}/ ${MASTER_SITE_GNOME:=subdir/}") @@ -610,7 +595,7 @@ func (s *Suite) Test_MkLine_variableNeedsQuoting__tool_in_CONFIGURE_ENV(c *check G.globalData.Tools = NewToolRegistry() G.globalData.Tools.RegisterVarname("tar", "TAR") - mklines := s.NewMkLines("Makefile", + mklines := T.NewMkLines("Makefile", mkrcsid, "", "CONFIGURE_ENV+=\tSYS_TAR_COMMAND_PATH=${TOOLS_TAR:Q}") @@ -627,7 +612,7 @@ func (s *Suite) Test_MkLine_Pkgmandir(c *check.C) { s.Init(c) s.UseCommandLine("-Wall") G.globalData.InitVartypes() - G.Mk = s.NewMkLines("chat/ircII/Makefile", + G.Mk = T.NewMkLines("chat/ircII/Makefile", mkrcsid, "CONFIGURE_ARGS+=--mandir=${DESTDIR}${PREFIX}/man", "CONFIGURE_ARGS+=--mandir=${DESTDIR}${PREFIX}/${PKGMANDIR}") @@ -642,7 +627,7 @@ func (s *Suite) Test_MkLines_Check__VERSION_as_wordpart_in_MASTER_SITES(c *check s.Init(c) s.UseCommandLine("-Wall") G.globalData.InitVartypes() - mklines := s.NewMkLines("geography/viking/Makefile", + mklines := T.NewMkLines("geography/viking/Makefile", mkrcsid, "MASTER_SITES=\t${MASTER_SITE_SOURCEFORGE:=viking/}${VERSION}/") @@ -656,7 +641,7 @@ func (s *Suite) Test_MkLines_Check__shell_command_as_wordpart_in_ENV_list(c *che s.Init(c) s.UseCommandLine("-Wall") G.globalData.InitVartypes() - mklines := s.NewMkLines("x11/lablgtk1/Makefile", + mklines := T.NewMkLines("x11/lablgtk1/Makefile", mkrcsid, "CONFIGURE_ENV+=\tCC=${CC}") @@ -671,7 +656,7 @@ func (s *Suite) Test_MkLine_shell_varuse_in_backt_dquot(c *check.C) { s.Init(c) s.UseCommandLine("-Wall") G.globalData.InitVartypes() - mklines := s.NewMkLines("x11/motif/Makefile", + mklines := T.NewMkLines("x11/motif/Makefile", mkrcsid, "post-patch:", "\tfiles=`${GREP} -l \".fB$${name}.fP(3)\" *.3`") @@ -695,7 +680,7 @@ func (s *Suite) Test_MkLine_VariableType(c *check.C) { func (s *Suite) Test_MkLine__comment_in_comment(c *check.C) { s.Init(c) G.globalData.InitVartypes() - mklines := s.NewMkLines("Makefile", + mklines := T.NewMkLines("Makefile", mkrcsid, "COMMENT=\tPKCS#5 v2.0 PBKDF2 Module") @@ -707,7 +692,7 @@ func (s *Suite) Test_MkLine__comment_in_comment(c *check.C) { func (s *Suite) Test_MkLine_ConditionVars(c *check.C) { s.Init(c) - var mkline MkLine = NewMkLine(NewLine("Makefile", 45, ".include \"../../category/package/buildlink3.mk\"", nil)) + mkline := T.NewMkLine("Makefile", 45, ".include \"../../category/package/buildlink3.mk\"") c.Check(mkline.ConditionVars(), equals, "") diff --git a/pkgtools/pkglint/files/mklinechecker.go b/pkgtools/pkglint/files/mklinechecker.go index 4e25f7546a6..66ff5f884c3 100644 --- a/pkgtools/pkglint/files/mklinechecker.go +++ b/pkgtools/pkglint/files/mklinechecker.go @@ -183,15 +183,16 @@ func (ck MkLineChecker) checkCond(forVars map[string]bool, indentation *Indentat } func (ck MkLineChecker) checkDirectiveIndentation(expectedDepth int) { - if G.Mk == nil { + if G.Mk == nil || !G.opts.WarnSpace { return } mkline := ck.MkLine indent := mkline.Indent() if expected := strings.Repeat(" ", expectedDepth); indent != expected { - if G.opts.WarnSpace && !mkline.AutofixReplace("."+indent, "."+expected) { - mkline.Notef("This directive should be indented by %d spaces.", expectedDepth) - } + fix := mkline.Line.Autofix() + fix.Notef("This directive should be indented by %d spaces.", expectedDepth) + fix.Replace("."+indent, "."+expected) + fix.Apply() } } @@ -547,9 +548,10 @@ func (ck MkLineChecker) CheckVaruseShellword(varname string, vartype *Vartype, v } else if mod != correctMod { if vuc.quoting == vucQuotPlain { - if !mkline.AutofixReplace("${"+varname+mod+"}", "${"+varname+correctMod+"}") { - mkline.Warnf("Please use ${%s%s} instead of ${%s%s}.", varname, correctMod, varname, mod) - } + fix := mkline.Line.Autofix() + fix.Warnf("Please use ${%s%s} instead of ${%s%s}.", varname, correctMod, varname, mod) + fix.Replace("${"+varname+mod+"}", "${"+varname+correctMod+"}") + fix.Apply() } else { mkline.Warnf("Please use ${%s%s} instead of ${%s%s} and make sure"+ " the variable appears outside of any quoting characters.", varname, correctMod, varname, mod) @@ -574,28 +576,26 @@ func (ck MkLineChecker) CheckVaruseShellword(varname string, vartype *Vartype, v if hasSuffix(mod, ":Q") && (needsQuoting == nqNo || needsQuoting == nqDoesntMatter) { bad := "${" + varname + mod + "}" good := "${" + varname + strings.TrimSuffix(mod, ":Q") + "}" - needExplain := false - if needsQuoting == nqNo && !mkline.AutofixReplace(bad, good) { - needExplain = true - mkline.Warnf("The :Q operator should not be used for ${%s} here.", varname) - } - if needsQuoting == nqDoesntMatter && !mkline.AutofixReplace(bad, good) { - needExplain = true - mkline.Notef("The :Q operator isn't necessary for ${%s} here.", varname) - } - if needExplain { - Explain( - "Many variables in pkgsrc do not need the :Q operator, since they", - "are not expected to contain white-space or other special characters.", - "Examples for these \"safe\" variables are:", - "", - "\t* filenames", - "\t* directory names", - "\t* user and group names", - "\t* tool names and tool paths", - "\t* variable names", - "\t* PKGNAME") + + fix := mkline.Line.Autofix() + if needsQuoting == nqNo { + fix.Warnf("The :Q operator should not be used for ${%s} here.", varname) + } else { + fix.Notef("The :Q operator isn't necessary for ${%s} here.", varname) } + fix.Explain( + "Many variables in pkgsrc do not need the :Q operator, since they", + "are not expected to contain white-space or other special characters.", + "Examples for these \"safe\" variables are:", + "", + "\t* filenames", + "\t* directory names", + "\t* user and group names", + "\t* tool names and tool paths", + "\t* variable names", + "\t* PKGNAME") + fix.Replace(bad, good) + fix.Apply() } } diff --git a/pkgtools/pkglint/files/mklinechecker_test.go b/pkgtools/pkglint/files/mklinechecker_test.go index c4b889f1d40..5d5acd9198c 100644 --- a/pkgtools/pkglint/files/mklinechecker_test.go +++ b/pkgtools/pkglint/files/mklinechecker_test.go @@ -6,7 +6,7 @@ func (s *Suite) Test_MkLineChecker_CheckVartype__simple_type(c *check.C) { s.Init(c) s.UseCommandLine("-Wtypes") G.globalData.InitVartypes() - mkline := NewMkLine(NewLine("fname", 1, "COMMENT=\tA nice package", nil)) + mkline := T.NewMkLine("fname", 1, "COMMENT=\tA nice package") vartype1 := G.globalData.vartypes["COMMENT"] c.Assert(vartype1, check.NotNil) @@ -26,7 +26,7 @@ func (s *Suite) Test_MkLineChecker_CheckVartype__simple_type(c *check.C) { func (s *Suite) Test_MkLineChecker_CheckVartype(c *check.C) { G.globalData.InitVartypes() - mkline := NewMkLine(NewLine("fname", 1, "DISTNAME=gcc-${GCC_VERSION}", nil)) + mkline := T.NewMkLine("fname", 1, "DISTNAME=gcc-${GCC_VERSION}") MkLineChecker{mkline}.CheckVartype("DISTNAME", opAssign, "gcc-${GCC_VERSION}", "") } @@ -37,7 +37,7 @@ func (s *Suite) Test_MkLineChecker_checkVarassign__URL_with_shell_special_charac s.Init(c) G.Pkg = NewPackage("graphics/gimp-fix-ca") G.globalData.InitVartypes() - mkline := NewMkLine(NewLine("fname", 10, "MASTER_SITES=http://registry.gimp.org/file/fix-ca.c?action=download&id=9884&file=", nil)) + mkline := T.NewMkLine("fname", 10, "MASTER_SITES=http://registry.gimp.org/file/fix-ca.c?action=download&id=9884&file=") MkLineChecker{mkline}.checkVarassign() @@ -49,46 +49,46 @@ func (s *Suite) Test_MkLineChecker_Check__conditions(c *check.C) { s.UseCommandLine("-Wtypes") G.globalData.InitVartypes() - MkLineChecker{NewMkLine(NewLine("fname", 1, ".if !empty(PKGSRC_COMPILER:Mmycc)", nil))}.CheckCond() + MkLineChecker{T.NewMkLine("fname", 1, ".if !empty(PKGSRC_COMPILER:Mmycc)")}.CheckCond() c.Check(s.Stdout(), equals, "WARN: fname:1: The pattern \"mycc\" cannot match any of "+ "{ ccache ccc clang distcc f2c gcc hp icc ido "+ "mipspro mipspro-ucode pcc sunpro xlc } for PKGSRC_COMPILER.\n") - MkLineChecker{NewMkLine(NewLine("fname", 1, ".elif ${A} != ${B}", nil))}.CheckCond() + MkLineChecker{T.NewMkLine("fname", 1, ".elif ${A} != ${B}")}.CheckCond() c.Check(s.Stdout(), equals, "") - MkLineChecker{NewMkLine(NewLine("fname", 1, ".if ${HOMEPAGE} == \"mailto:someone@example.org\"", nil))}.CheckCond() + MkLineChecker{T.NewMkLine("fname", 1, ".if ${HOMEPAGE} == \"mailto:someone@example.org\"")}.CheckCond() s.CheckOutputLines( "WARN: fname:1: \"mailto:someone@example.org\" is not a valid URL.") - MkLineChecker{NewMkLine(NewLine("fname", 1, ".if !empty(PKGSRC_RUN_TEST:M[Y][eE][sS])", nil))}.CheckCond() + MkLineChecker{T.NewMkLine("fname", 1, ".if !empty(PKGSRC_RUN_TEST:M[Y][eE][sS])")}.CheckCond() s.CheckOutputLines( "WARN: fname:1: PKGSRC_RUN_TEST should be matched against \"[yY][eE][sS]\" or \"[nN][oO]\", not \"[Y][eE][sS]\".") - MkLineChecker{NewMkLine(NewLine("fname", 1, ".if !empty(IS_BUILTIN.Xfixes:M[yY][eE][sS])", nil))}.CheckCond() + MkLineChecker{T.NewMkLine("fname", 1, ".if !empty(IS_BUILTIN.Xfixes:M[yY][eE][sS])")}.CheckCond() s.CheckOutputEmpty() - MkLineChecker{NewMkLine(NewLine("fname", 1, ".if !empty(${IS_BUILTIN.Xfixes:M[yY][eE][sS]})", nil))}.CheckCond() + MkLineChecker{T.NewMkLine("fname", 1, ".if !empty(${IS_BUILTIN.Xfixes:M[yY][eE][sS]})")}.CheckCond() s.CheckOutputLines( "WARN: fname:1: The empty() function takes a variable name as parameter, not a variable expression.") - MkLineChecker{NewMkLine(NewLine("fname", 1, ".if ${EMUL_PLATFORM} == \"linux-x386\"", nil))}.CheckCond() + MkLineChecker{T.NewMkLine("fname", 1, ".if ${EMUL_PLATFORM} == \"linux-x386\"")}.CheckCond() s.CheckOutputLines( "WARN: fname:1: \"x386\" is not valid for the hardware architecture part of EMUL_PLATFORM. Use one of { aarch64 aarch64eb alpha amd64 arc arm arm26 arm32 cobalt coldfire convex dreamcast earm earmeb earmhf earmhfeb earmv4 earmv4eb earmv5 earmv5eb earmv6 earmv6eb earmv6hf earmv6hfeb earmv7 earmv7eb earmv7hf earmv7hfeb evbarm hpcmips hpcsh hppa hppa64 i386 i586 i686 ia64 m68000 m68k m88k mips mips64 mips64eb mips64el mipseb mipsel mipsn32 mlrisc ns32k pc532 pmax powerpc powerpc64 rs6000 s390 sh3eb sh3el sparc sparc64 vax x86_64 } instead.") - MkLineChecker{NewMkLine(NewLine("fname", 1, ".if ${EMUL_PLATFORM:Mlinux-x386}", nil))}.CheckCond() + MkLineChecker{T.NewMkLine("fname", 1, ".if ${EMUL_PLATFORM:Mlinux-x386}")}.CheckCond() s.CheckOutputLines( "WARN: fname:1: The pattern \"x386\" cannot match any of { aarch64 aarch64eb alpha amd64 arc arm arm26 arm32 cobalt coldfire convex dreamcast earm earmeb earmhf earmhfeb earmv4 earmv4eb earmv5 earmv5eb earmv6 earmv6eb earmv6hf earmv6hfeb earmv7 earmv7eb earmv7hf earmv7hfeb evbarm hpcmips hpcsh hppa hppa64 i386 i586 i686 ia64 m68000 m68k m88k mips mips64 mips64eb mips64el mipseb mipsel mipsn32 mlrisc ns32k pc532 pmax powerpc powerpc64 rs6000 s390 sh3eb sh3el sparc sparc64 vax x86_64 } for the hardware architecture part of EMUL_PLATFORM.") - MkLineChecker{NewMkLine(NewLine("fname", 98, ".if ${MACHINE_PLATFORM:MUnknownOS-*-*} || ${MACHINE_ARCH:Mx86}", nil))}.CheckCond() + MkLineChecker{T.NewMkLine("fname", 98, ".if ${MACHINE_PLATFORM:MUnknownOS-*-*} || ${MACHINE_ARCH:Mx86}")}.CheckCond() s.CheckOutputLines( "WARN: fname:98: The pattern \"UnknownOS\" cannot match any of { AIX BSDOS Bitrig Cygwin Darwin DragonFly FreeBSD FreeMiNT GNUkFreeBSD HPUX Haiku IRIX Interix Linux Minix MirBSD NetBSD OSF1 OpenBSD QNX SCO_SV SunOS UnixWare } for the operating system part of MACHINE_PLATFORM.", @@ -99,7 +99,7 @@ func (s *Suite) Test_MkLineChecker_checkVarassign(c *check.C) { s.Init(c) G.globalData.InitVartypes() - G.Mk = s.NewMkLines("Makefile", + G.Mk = T.NewMkLines("Makefile", mkrcsid, "ac_cv_libpari_libs+=\t-L${BUILDLINK_PREFIX.pari}/lib") // From math/clisp-pari/Makefile, rev. 1.8 @@ -113,7 +113,7 @@ func (s *Suite) Test_MkLineChecker_checkVarassignDefPermissions(c *check.C) { s.Init(c) s.UseCommandLine("-Wall") G.globalData.InitVartypes() - mkline := NewMkLine(NewLine("options.mk", 2, "PKG_DEVELOPER?=\tyes", nil)) + mkline := T.NewMkLine("options.mk", 2, "PKG_DEVELOPER?=\tyes") MkLineChecker{mkline}.checkVarassignDefPermissions() @@ -125,7 +125,7 @@ func (s *Suite) Test_MkLineChecker_CheckVarusePermissions(c *check.C) { s.Init(c) s.UseCommandLine("-Wall") G.globalData.InitVartypes() - mklines := s.NewMkLines("options.mk", + mklines := T.NewMkLines("options.mk", mkrcsid, "COMMENT=\t${GAMES_USER}", "COMMENT:=\t${PKGBASE}", @@ -148,7 +148,7 @@ func (s *Suite) Test_MkLineChecker_CheckVarusePermissions__load_time(c *check.C) s.Init(c) s.UseCommandLine("-Wall") G.globalData.InitVartypes() - mklines := s.NewMkLines("options.mk", + mklines := T.NewMkLines("options.mk", mkrcsid, "WRKSRC:=${.CURDIR}") @@ -159,7 +159,7 @@ func (s *Suite) Test_MkLineChecker_CheckVarusePermissions__load_time(c *check.C) func (s *Suite) Test_MkLineChecker_WarnVaruseLocalbase(c *check.C) { s.Init(c) - mkline := NewMkLine(NewLine("options.mk", 56, "PKGNAME=${LOCALBASE}", nil)) + mkline := T.NewMkLine("options.mk", 56, "PKGNAME=${LOCALBASE}") MkLineChecker{mkline}.WarnVaruseLocalbase() @@ -169,7 +169,7 @@ func (s *Suite) Test_MkLineChecker_WarnVaruseLocalbase(c *check.C) { func (s *Suite) Test_MkLineChecker_CheckRelativePkgdir(c *check.C) { s.Init(c) - mkline := NewMkLine(NewLine("Makefile", 46, "# dummy", nil)) + mkline := T.NewMkLine("Makefile", 46, "# dummy") MkLineChecker{mkline}.CheckRelativePkgdir("../pkgbase") @@ -181,7 +181,7 @@ func (s *Suite) Test_MkLineChecker_CheckRelativePkgdir(c *check.C) { // PR pkg/46570, item 2 func (s *Suite) Test_MkLineChecker__unclosed_varuse(c *check.C) { s.Init(c) - mkline := NewMkLine(NewLine("Makefile", 93, "EGDIRS=${EGDIR/apparmor.d ${EGDIR/dbus-1/system.d ${EGDIR/pam.d", nil)) + mkline := T.NewMkLine("Makefile", 93, "EGDIRS=${EGDIR/apparmor.d ${EGDIR/dbus-1/system.d ${EGDIR/pam.d") MkLineChecker{mkline}.checkVarassign() @@ -195,7 +195,7 @@ func (s *Suite) Test_MkLineChecker__Varuse_Modifier_L(c *check.C) { s.Init(c) s.UseCommandLine("-Wall") G.globalData.InitVartypes() - G.Mk = s.NewMkLines("x11/xkeyboard-config/Makefile", + G.Mk = T.NewMkLines("x11/xkeyboard-config/Makefile", "FILES_SUBST+=XKBCOMP_SYMLINK=${${XKBBASE}/xkbcomp:L:Q}") MkLineChecker{G.Mk.mklines[0]}.Check() @@ -207,7 +207,7 @@ func (s *Suite) Test_MkLineChecker_CheckCond__comparison_with_shell_command(c *c s.Init(c) s.UseCommandLine("-Wall") G.globalData.InitVartypes() - G.Mk = s.NewMkLines("security/openssl/Makefile", + G.Mk = T.NewMkLines("security/openssl/Makefile", mkrcsid, ".if ${PKGSRC_COMPILER} == \"gcc\" && ${CC} == \"cc\"", ".endif") @@ -223,7 +223,7 @@ func (s *Suite) Test_MkLine_CheckCond_comparing_PKGSRC_COMPILER_with_eqeq(c *che s.Init(c) s.UseCommandLine("-Wall") G.globalData.InitVartypes() - G.Mk = s.NewMkLines("audio/pulseaudio/Makefile", + G.Mk = T.NewMkLines("audio/pulseaudio/Makefile", mkrcsid, ".if ${OPSYS} == \"Darwin\" && ${PKGSRC_COMPILER} == \"clang\"", ".endif") @@ -238,7 +238,7 @@ func (s *Suite) Test_MkLineChecker_CheckVartype__CFLAGS_with_backticks(c *check. s.Init(c) s.UseCommandLine("-Wall") G.globalData.InitVartypes() - G.Mk = s.NewMkLines("chat/pidgin-icb/Makefile", + G.Mk = T.NewMkLines("chat/pidgin-icb/Makefile", mkrcsid, "CFLAGS+=\t`pkg-config pidgin --cflags`") mkline := G.Mk.mklines[1] @@ -259,7 +259,7 @@ func (s *Suite) Test_MkLineChecker_CheckVartype__CFLAGS_with_backticks(c *check. func (s *Suite) Test_MkLineChecker_CheckVartype_CFLAGS(c *check.C) { s.Init(c) G.globalData.InitVartypes() - mklines := s.NewMkLines("Makefile", + mklines := T.NewMkLines("Makefile", mkrcsid, "CPPFLAGS.SunOS+=\t-DPIPECOMMAND=\\\"/usr/sbin/sendmail -bs %s\\\"") diff --git a/pkgtools/pkglint/files/mklines.go b/pkgtools/pkglint/files/mklines.go index a418b3487f0..53f109b1bfe 100644 --- a/pkgtools/pkglint/files/mklines.go +++ b/pkgtools/pkglint/files/mklines.go @@ -336,30 +336,31 @@ func (va *VaralignBlock) fixalign(mkline MkLine, prefix, oldalign string) { return } - if !mkline.AutofixReplace(prefix+oldalign, prefix+newalign) { - wrongColumn := tabLength(prefix+oldalign) != tabLength(prefix+newalign) - switch { - case hasSpace && wrongColumn: - mkline.Notef("This variable value should be aligned with tabs, not spaces, to column %d.", goodWidth+1) - case hasSpace: - mkline.Notef("Variable values should be aligned with tabs, not spaces.") - case wrongColumn: - mkline.Notef("This variable value should be aligned to column %d.", goodWidth+1) - } - if wrongColumn { - Explain( - "Normally, all variable values in a block should start at the same", - "column. There are some exceptions to this rule:", - "", - "Definitions for long variable names may be indented with a single", - "space instead of tabs, but only if they appear in a block that is", - "otherwise indented using tabs.", - "", - "Variable definitions that span multiple lines are not checked for", - "alignment at all.", - "", - "When the block contains something else than variable definitions,", - "it is not checked at all.") - } + fix := mkline.Line.Autofix() + wrongColumn := tabLength(prefix+oldalign) != tabLength(prefix+newalign) + switch { + case hasSpace && wrongColumn: + fix.Notef("This variable value should be aligned with tabs, not spaces, to column %d.", goodWidth+1) + case hasSpace: + fix.Notef("Variable values should be aligned with tabs, not spaces.") + case wrongColumn: + fix.Notef("This variable value should be aligned to column %d.", goodWidth+1) + } + if wrongColumn { + fix.Explain( + "Normally, all variable values in a block should start at the same", + "column. There are some exceptions to this rule:", + "", + "Definitions for long variable names may be indented with a single", + "space instead of tabs, but only if they appear in a block that is", + "otherwise indented using tabs.", + "", + "Variable definitions that span multiple lines are not checked for", + "alignment at all.", + "", + "When the block contains something else than variable definitions,", + "it is not checked at all.") } + fix.Replace(prefix+oldalign, prefix+newalign) + fix.Apply() } diff --git a/pkgtools/pkglint/files/mklines_test.go b/pkgtools/pkglint/files/mklines_test.go index 6f46a0e8483..24a7728995b 100644 --- a/pkgtools/pkglint/files/mklines_test.go +++ b/pkgtools/pkglint/files/mklines_test.go @@ -10,7 +10,7 @@ func (s *Suite) Test_MkLines_Check__autofix_conditional_indentation(c *check.C) s.Init(c) s.UseCommandLine("--autofix", "-Wspace") tmpfile := s.CreateTmpFile("fname.mk", "") - mklines := s.NewMkLines(tmpfile, + mklines := T.NewMkLines(tmpfile, mkrcsid, ".if defined(A)", ".for a in ${A}", @@ -39,7 +39,7 @@ func (s *Suite) Test_MkLines_Check__autofix_conditional_indentation(c *check.C) func (s *Suite) Test_MkLines_Check__unusual_target(c *check.C) { s.Init(c) - mklines := s.NewMkLines("Makefile", + mklines := T.NewMkLines("Makefile", mkrcsid, "", "echo: echo.c", @@ -53,7 +53,7 @@ func (s *Suite) Test_MkLines_Check__unusual_target(c *check.C) { func (s *Suite) Test_MkLineChecker_checkInclude__Makefile(c *check.C) { s.Init(c) - mkline := NewMkLine(NewLine("Makefile", 2, ".include \"../../other/package/Makefile\"", nil)) + mkline := T.NewMkLine("Makefile", 2, ".include \"../../other/package/Makefile\"") MkLineChecker{mkline}.checkInclude() @@ -67,7 +67,7 @@ func (s *Suite) Test_MkLines_quoting_LDFLAGS_for_GNU_configure(c *check.C) { s.UseCommandLine("-Wall") G.globalData.InitVartypes() G.Pkg = NewPackage("category/pkgbase") - mklines := s.NewMkLines("Makefile", + mklines := T.NewMkLines("Makefile", mkrcsid, "GNU_CONFIGURE=\tyes", "CONFIGURE_ENV+=\tX_LIBS=${X11_LDFLAGS:Q}") @@ -157,7 +157,7 @@ func (s *Suite) Test_MkLines__variable_alignment_advanced(c *check.C) { func (s *Suite) Test_MkLines__variable_alignment_space_and_tab(c *check.C) { s.Init(c) s.UseCommandLine("-Wspace") - mklines := s.NewMkLines("Makefile", + mklines := T.NewMkLines("Makefile", mkrcsid, "", "VAR= space", @@ -175,7 +175,7 @@ func (s *Suite) Test_MkLines__for_loop_multiple_variables(c *check.C) { s.RegisterTool(&Tool{Name: "echo", Varname: "ECHO", Predefined: true}) s.RegisterTool(&Tool{Name: "find", Varname: "FIND", Predefined: true}) s.RegisterTool(&Tool{Name: "pax", Varname: "PAX", Predefined: true}) - mklines := s.NewMkLines("audio/squeezeboxserver/Makefile", + mklines := T.NewMkLines("audio/squeezeboxserver/Makefile", mkrcsid, "", ".for _list_ _dir_ in ${SBS_COPY}", @@ -195,7 +195,7 @@ func (s *Suite) Test_MkLines__comparing_YesNo_variable_to_string(c *check.C) { s.Init(c) s.UseCommandLine("-Wall") G.globalData.InitVartypes() - mklines := s.NewMkLines("databases/gdbm_compat/builtin.mk", + mklines := T.NewMkLines("databases/gdbm_compat/builtin.mk", mkrcsid, ".if ${USE_BUILTIN.gdbm} == \"no\"", ".endif", @@ -212,7 +212,7 @@ func (s *Suite) Test_MkLines__varuse_sh_modifier(c *check.C) { s.Init(c) s.UseCommandLine("-Wall") G.globalData.InitVartypes() - mklines := s.NewMkLines("lang/qore/module.mk", + mklines := T.NewMkLines("lang/qore/module.mk", mkrcsid, "qore-version=\tqore --short-version | ${SED} -e s/-.*//", "PLIST_SUBST+=\tQORE_VERSION=\"${qore-version:sh}\"") @@ -234,7 +234,7 @@ func (s *Suite) Test_MkLines__varuse_parameterized(c *check.C) { s.Init(c) s.UseCommandLine("-Wall") G.globalData.InitVartypes() - mklines := s.NewMkLines("converters/wv2/Makefile", + mklines := T.NewMkLines("converters/wv2/Makefile", mkrcsid, "CONFIGURE_ARGS+=\t\t${CONFIGURE_ARGS.${ICONV_TYPE}-iconv}", "CONFIGURE_ARGS.gnu-iconv=\t--with-libiconv=${BUILDLINK_PREFIX.iconv}") @@ -248,7 +248,7 @@ func (s *Suite) Test_MkLines__loop_modifier(c *check.C) { s.Init(c) s.UseCommandLine("-Wall") G.globalData.InitVartypes() - mklines := s.NewMkLines("chat/xchat/Makefile", + mklines := T.NewMkLines("chat/xchat/Makefile", mkrcsid, "GCONF_SCHEMAS=\tapps_xchat_url_handler.schemas", "post-install:", @@ -266,7 +266,7 @@ func (s *Suite) Test_MkLines__loop_modifier(c *check.C) { func (s *Suite) Test_MkLines__PKG_SKIP_REASON_depending_on_OPSYS(c *check.C) { s.Init(c) G.globalData.InitVartypes() - mklines := s.NewMkLines("Makefile", + mklines := T.NewMkLines("Makefile", mkrcsid, "PKG_SKIP_REASON+=\t\"Fails everywhere\"", ".if ${OPSYS} == \"Cygwin\"", @@ -283,7 +283,7 @@ func (s *Suite) Test_MkLines__PKG_SKIP_REASON_depending_on_OPSYS(c *check.C) { func (s *Suite) Test_MkLines__indirect_variables(c *check.C) { s.Init(c) s.UseCommandLine("-Wall") - mklines := s.NewMkLines("net/uucp/Makefile", + mklines := T.NewMkLines("net/uucp/Makefile", mkrcsid, "", "post-configure:", @@ -301,7 +301,7 @@ func (s *Suite) Test_MkLines__indirect_variables(c *check.C) { func (s *Suite) Test_MkLines_Check__list_variable_as_part_of_word(c *check.C) { s.Init(c) s.UseCommandLine("-Wall") - mklines := s.NewMkLines("converters/chef/Makefile", + mklines := T.NewMkLines("converters/chef/Makefile", mkrcsid, "\tcd ${WRKSRC} && tr '\\r' '\\n' < ${DISTDIR}/${DIST_SUBDIR}/${DISTFILES} > chef.l") @@ -316,7 +316,7 @@ func (s *Suite) Test_MkLines_Check__absolute_pathname_depending_on_OPSYS(c *chec s.Init(c) s.UseCommandLine("-Wall") G.globalData.InitVartypes() - mklines := s.NewMkLines("games/heretic2-demo/Makefile", + mklines := T.NewMkLines("games/heretic2-demo/Makefile", mkrcsid, ".if ${OPSYS} == \"DragonFly\"", "TOOLS_PLATFORM.gtar=\t/usr/bin/bsdtar", @@ -336,7 +336,7 @@ func (s *Suite) Test_MkLines_Check__absolute_pathname_depending_on_OPSYS(c *chec func (s *Suite) Test_MkLines_checkForUsedComment(c *check.C) { s.Init(c) s.UseCommandLine("--show-autofix") - s.NewMkLines("Makefile.common", + T.NewMkLines("Makefile.common", mkrcsid, "", "# used by sysutils/mc", @@ -344,24 +344,24 @@ func (s *Suite) Test_MkLines_checkForUsedComment(c *check.C) { s.CheckOutputEmpty() - s.NewMkLines("Makefile.common").checkForUsedComment("category/package") + T.NewMkLines("Makefile.common").checkForUsedComment("category/package") s.CheckOutputEmpty() - s.NewMkLines("Makefile.common", + T.NewMkLines("Makefile.common", mkrcsid, ).checkForUsedComment("category/package") s.CheckOutputEmpty() - s.NewMkLines("Makefile.common", + T.NewMkLines("Makefile.common", mkrcsid, "", ).checkForUsedComment("category/package") s.CheckOutputEmpty() - s.NewMkLines("Makefile.common", + T.NewMkLines("Makefile.common", mkrcsid, "", "VARNAME=\tvalue", @@ -371,7 +371,7 @@ func (s *Suite) Test_MkLines_checkForUsedComment(c *check.C) { "WARN: Makefile.common:2: Please add a line \"# used by category/package\" here.", "AUTOFIX: Makefile.common:2: Inserting a line \"# used by category/package\" before this line.") - s.NewMkLines("Makefile.common", + T.NewMkLines("Makefile.common", mkrcsid, "#", "#", @@ -380,10 +380,12 @@ func (s *Suite) Test_MkLines_checkForUsedComment(c *check.C) { s.CheckOutputLines( "WARN: Makefile.common:3: Please add a line \"# used by category/package\" here.", "AUTOFIX: Makefile.common:3: Inserting a line \"# used by category/package\" before this line.") + + c.Check(G.autofixAvailable, equals, true) } func (s *Suite) Test_MkLines_DetermineUsedVariables__simple(c *check.C) { - mklines := s.NewMkLines("fname", + mklines := T.NewMkLines("fname", "\t${VAR}") mkline := mklines.mklines[0] G.Mk = mklines @@ -395,7 +397,7 @@ func (s *Suite) Test_MkLines_DetermineUsedVariables__simple(c *check.C) { } func (s *Suite) Test_MkLines_DetermineUsedVariables__nested(c *check.C) { - mklines := s.NewMkLines("fname", + mklines := T.NewMkLines("fname", "\t${outer.${inner}}") mkline := mklines.mklines[0] G.Mk = mklines @@ -412,7 +414,7 @@ func (s *Suite) Test_MkLines_PrivateTool_Undefined(c *check.C) { s.Init(c) s.UseCommandLine("-Wall") G.globalData.InitVartypes() - mklines := s.NewMkLines("fname", + mklines := T.NewMkLines("fname", mkrcsid, "", "\tmd5sum filename") @@ -427,7 +429,7 @@ func (s *Suite) Test_MkLines_PrivateTool_Defined(c *check.C) { s.Init(c) s.UseCommandLine("-Wall") G.globalData.InitVartypes() - mklines := s.NewMkLines("fname", + mklines := T.NewMkLines("fname", mkrcsid, "TOOLS_CREATE+=\tmd5sum", "", @@ -441,7 +443,7 @@ func (s *Suite) Test_MkLines_PrivateTool_Defined(c *check.C) { func (s *Suite) Test_MkLines_Check_indentation(c *check.C) { s.Init(c) s.UseCommandLine("-Wall") - mklines := s.NewMkLines("options.mk", + mklines := T.NewMkLines("options.mk", mkrcsid, ". if !defined(GUARD_MK)", ". if ${OPSYS} == ${OPSYS}", @@ -486,7 +488,7 @@ func (s *Suite) Test_MkLines_wip_category_Makefile(c *check.C) { s.UseCommandLine("-Wall") G.globalData.InitVartypes() s.RegisterTool(&Tool{Name: "rm", Varname: "RM", Predefined: true}) - mklines := s.NewMkLines("Makefile", + mklines := T.NewMkLines("Makefile", mkrcsid, "", "COMMENT=\tWIP pkgsrc packages", diff --git a/pkgtools/pkglint/files/mkparser.go b/pkgtools/pkglint/files/mkparser.go index 62af835324e..a0ae0200d62 100644 --- a/pkgtools/pkglint/files/mkparser.go +++ b/pkgtools/pkglint/files/mkparser.go @@ -65,9 +65,10 @@ func (p *MkParser) VarUse() *MkVarUse { if usingRoundParen && p.EmitWarnings { parenVaruse := repl.Since(mark) bracesVaruse := "${" + parenVaruse[2:len(parenVaruse)-1] + "}" - if !p.Line.AutofixReplace(parenVaruse, bracesVaruse) { - p.Line.Warnf("Please use curly braces {} instead of round parentheses () for %s.", varname) - } + fix := p.Line.Autofix() + fix.Warnf("Please use curly braces {} instead of round parentheses () for %s.", varname) + fix.Replace(parenVaruse, bracesVaruse) + fix.Apply() } return &MkVarUse{varname, modifiers} } diff --git a/pkgtools/pkglint/files/mkparser_test.go b/pkgtools/pkglint/files/mkparser_test.go index a9bc039aa42..ef70a045e85 100644 --- a/pkgtools/pkglint/files/mkparser_test.go +++ b/pkgtools/pkglint/files/mkparser_test.go @@ -7,7 +7,8 @@ import ( func (s *Suite) Test_MkParser_MkTokens(c *check.C) { s.Init(c) checkRest := func(input string, expectedTokens []*MkToken, expectedRest string) { - p := NewMkParser(dummyLine, input, true) + line := T.NewLines("Test_MkParser_MkTokens.mk", input)[0] + p := NewMkParser(line, input, true) actualTokens := p.MkTokens() c.Check(actualTokens, deepEquals, expectedTokens) for i, expectedToken := range expectedTokens { @@ -110,7 +111,7 @@ func (s *Suite) Test_MkParser_MkTokens(c *check.C) { check("$(GNUSTEP_USER_ROOT)", varuseText("$(GNUSTEP_USER_ROOT)", "GNUSTEP_USER_ROOT")) s.CheckOutputLines( - "WARN: Please use curly braces {} instead of round parentheses () for GNUSTEP_USER_ROOT.") + "WARN: Test_MkParser_MkTokens.mk:1: Please use curly braces {} instead of round parentheses () for GNUSTEP_USER_ROOT.") checkRest("${VAR)", nil, "${VAR)") // Opening brace, closing parenthesis checkRest("$(VAR}", nil, "$(VAR}") // Opening parenthesis, closing brace @@ -119,7 +120,7 @@ func (s *Suite) Test_MkParser_MkTokens(c *check.C) { check("${PLIST_SUBST_VARS:@var@${var}=${${var}:Q}@}", varuse("PLIST_SUBST_VARS", "@var@${var}=${${var}:Q}@")) check("${PLIST_SUBST_VARS:@var@${var}=${${var}:Q}}", varuse("PLIST_SUBST_VARS", "@var@${var}=${${var}:Q}")) // Missing @ at the end s.CheckOutputLines( - "WARN: Modifier ${PLIST_SUBST_VARS:@var@...@} is missing the final \"@\".") + "WARN: Test_MkParser_MkTokens.mk:1: Modifier ${PLIST_SUBST_VARS:@var@...@} is missing the final \"@\".") checkRest("hello, ${W:L:tl}orld", []*MkToken{ literal("hello, "), @@ -220,7 +221,7 @@ func (s *Suite) Test_MkParser__varuse_parentheses_autofix(c *check.C) { s.UseCommandLine("--autofix") G.globalData.InitVartypes() filename := s.CreateTmpFile("Makefile", "") - mklines := s.NewMkLines(filename, + mklines := T.NewMkLines(filename, mkrcsid, "COMMENT=$(P1) $(P2)) $(P3:Q) ${BRACES}") diff --git a/pkgtools/pkglint/files/package.go b/pkgtools/pkglint/files/package.go index 7f7c1e2ded0..a0c820932fe 100644 --- a/pkgtools/pkglint/files/package.go +++ b/pkgtools/pkglint/files/package.go @@ -414,6 +414,9 @@ func (pkg *Package) checkfilePackageMakefile(fname string, mklines *MkLines) { // probably contains a statement that C is // really not needed. + } else if !G.Infrastructure && useLine.Filename == "../../mk/compiler.mk" { + // Ignore this one + } else if !matches(useLine.Value(), `(?:^|\s+)(?:c|c99|objc)(?:\s+|$)`) { gnuLine.Warnf("GNU_CONFIGURE almost always needs a C compiler, but \"c\" is not added to USE_LANGUAGES in %s.", useLine.ReferenceFrom(gnuLine.Line)) @@ -804,19 +807,20 @@ func (mklines *MkLines) checkForUsedComment(relativeName string) { i++ } - insertLine := lines[i] - if !insertLine.AutofixInsertBefore(expected) { - insertLine.Warnf("Please add a line %q here.", expected) - Explain( - "Since Makefile.common files usually don't have any comments and", - "therefore not a clearly defined interface, they should at least", - "contain references to all files that include them, so that it is", - "easier to see what effects future changes may have.", - "", - "If there are more than five packages that use a Makefile.common,", - "you should think about giving it a proper name (maybe plugin.mk) and", - "documenting its interface.") - } + fix := lines[i].Autofix() + fix.Warnf("Please add a line %q here.", expected) + fix.Explain( + "Since Makefile.common files usually don't have any comments and", + "therefore not a clearly defined interface, they should at least", + "contain references to all files that include them, so that it is", + "easier to see what effects future changes may have.", + "", + "If there are more than five packages that use a Makefile.common,", + "you should think about giving it a proper name (maybe plugin.mk) and", + "documenting its interface.") + fix.InsertBefore(expected) + fix.Apply() + SaveAutofixChanges(lines) } diff --git a/pkgtools/pkglint/files/package_test.go b/pkgtools/pkglint/files/package_test.go index 0dce87c0aa7..e4743673c26 100644 --- a/pkgtools/pkglint/files/package_test.go +++ b/pkgtools/pkglint/files/package_test.go @@ -1,13 +1,11 @@ package main -import ( - check "gopkg.in/check.v1" -) +import "gopkg.in/check.v1" func (s *Suite) Test_Package_pkgnameFromDistname(c *check.C) { s.Init(c) pkg := NewPackage("dummy") - pkg.vardef["PKGNAME"] = NewMkLine(NewLine("Makefile", 5, "PKGNAME=dummy", nil)) + pkg.vardef["PKGNAME"] = T.NewMkLine("Makefile", 5, "PKGNAME=dummy") c.Check(pkg.pkgnameFromDistname("pkgname-1.0", "whatever"), equals, "pkgname-1.0") c.Check(pkg.pkgnameFromDistname("${DISTNAME}", "distname-1.0"), equals, "distname-1.0") @@ -27,7 +25,7 @@ func (s *Suite) Test_Package_ChecklinesPackageMakefileVarorder(c *check.C) { s.UseCommandLine("-Worder") pkg := NewPackage("x11/9term") - pkg.ChecklinesPackageMakefileVarorder(s.NewMkLines("Makefile", + pkg.ChecklinesPackageMakefileVarorder(T.NewMkLines("Makefile", mkrcsid, "", "DISTNAME=9term", @@ -35,7 +33,7 @@ func (s *Suite) Test_Package_ChecklinesPackageMakefileVarorder(c *check.C) { s.CheckOutputEmpty() - pkg.ChecklinesPackageMakefileVarorder(s.NewMkLines("Makefile", + pkg.ChecklinesPackageMakefileVarorder(T.NewMkLines("Makefile", mkrcsid, "", "DISTNAME=9term", @@ -83,7 +81,7 @@ func (s *Suite) Test_Package_ChecklinesPackageMakefileVarorder__MASTER_SITES(c * s.UseCommandLine("-Worder") pkg := NewPackage("category/package") - pkg.ChecklinesPackageMakefileVarorder(s.NewMkLines("Makefile", + pkg.ChecklinesPackageMakefileVarorder(T.NewMkLines("Makefile", mkrcsid, "", "PKGNAME=\tpackage-1.0", @@ -96,20 +94,20 @@ func (s *Suite) Test_Package_ChecklinesPackageMakefileVarorder__MASTER_SITES(c * func (s *Suite) Test_Package_getNbpart(c *check.C) { pkg := NewPackage("category/pkgbase") - pkg.vardef["PKGREVISION"] = NewMkLine(NewLine("Makefile", 1, "PKGREVISION=14", nil)) + pkg.vardef["PKGREVISION"] = T.NewMkLine("Makefile", 1, "PKGREVISION=14") c.Check(pkg.getNbpart(), equals, "nb14") - pkg.vardef["PKGREVISION"] = NewMkLine(NewLine("Makefile", 1, "PKGREVISION=asdf", nil)) + pkg.vardef["PKGREVISION"] = T.NewMkLine("Makefile", 1, "PKGREVISION=asdf") c.Check(pkg.getNbpart(), equals, "") } func (s *Suite) Test_Package_determineEffectivePkgVars__precedence(c *check.C) { pkg := NewPackage("category/pkgbase") - pkgnameLine := NewMkLine(NewLine("Makefile", 3, "PKGNAME=pkgname-1.0", nil)) - distnameLine := NewMkLine(NewLine("Makefile", 4, "DISTNAME=distname-1.0", nil)) - pkgrevisionLine := NewMkLine(NewLine("Makefile", 5, "PKGREVISION=13", nil)) + pkgnameLine := T.NewMkLine("Makefile", 3, "PKGNAME=pkgname-1.0") + distnameLine := T.NewMkLine("Makefile", 4, "DISTNAME=distname-1.0") + pkgrevisionLine := T.NewMkLine("Makefile", 5, "PKGREVISION=13") pkg.defineVar(pkgnameLine, pkgnameLine.Varname()) pkg.defineVar(distnameLine, distnameLine.Varname()) @@ -127,12 +125,12 @@ func (s *Suite) Test_Package_checkPossibleDowngrade(c *check.C) { G.Pkg = NewPackage("category/pkgbase") G.CurPkgsrcdir = "../.." G.Pkg.EffectivePkgname = "package-1.0nb15" - G.Pkg.EffectivePkgnameLine = NewMkLine(NewLine("category/pkgbase/Makefile", 5, "PKGNAME=dummy", nil)) + G.Pkg.EffectivePkgnameLine = T.NewMkLine("category/pkgbase/Makefile", 5, "PKGNAME=dummy") G.globalData.LastChange = map[string]*Change{ - "category/pkgbase": &Change{ + "category/pkgbase": { Action: "Updated", Version: "1.8", - Line: NewLine("doc/CHANGES", 116, "dummy", nil), + Line: T.NewLine("doc/CHANGES", 116, "dummy"), }, } diff --git a/pkgtools/pkglint/files/patches.go b/pkgtools/pkglint/files/patches.go index 633a6d5bbb9..e1b6805076c 100644 --- a/pkgtools/pkglint/files/patches.go +++ b/pkgtools/pkglint/files/patches.go @@ -181,9 +181,10 @@ func (ck *PatchChecker) checkBeginDiff(line Line, patchedFiles int) { "be mentioned in this file, to prevent duplicate work.") } if G.opts.WarnSpace && !ck.previousLineEmpty { - if !line.AutofixInsertBefore("") { - line.Notef("Empty line expected.") - } + fix := line.Autofix() + fix.Notef("Empty line expected.") + fix.InsertBefore("") + fix.Apply() } } @@ -235,11 +236,12 @@ func (ck *PatchChecker) checktextUniHunkCr() { line := ck.exp.PreviousLine() if hasSuffix(line.Text, "\r") { - if !line.AutofixReplace("\r\n", "\n") { - line.Errorf("The hunk header must not end with a CR character.") - Explain( - "The MacOS X patch utility cannot handle these.") - } + fix := line.Autofix() + fix.Errorf("The hunk header must not end with a CR character.") + fix.Explain( + "The MacOS X patch utility cannot handle these.") + fix.Replace("\r\n", "\n") + fix.Apply() } } diff --git a/pkgtools/pkglint/files/patches_test.go b/pkgtools/pkglint/files/patches_test.go index 28259d4bb1d..d4e0983beb4 100644 --- a/pkgtools/pkglint/files/patches_test.go +++ b/pkgtools/pkglint/files/patches_test.go @@ -8,7 +8,7 @@ import ( func (s *Suite) Test_ChecklinesPatch__with_comment(c *check.C) { s.Init(c) s.UseCommandLine("-Wall") - lines := s.NewLines("patch-WithComment", + lines := T.NewLines("patch-WithComment", "$"+"NetBSD$", "", "Text", @@ -84,7 +84,7 @@ func (s *Suite) Test_ChecklinesPatch__without_empty_line__autofix(c *check.C) { func (s *Suite) Test_ChecklinesPatch__without_comment(c *check.C) { s.Init(c) s.UseCommandLine("-Wall") - lines := s.NewLines("patch-WithoutComment", + lines := T.NewLines("patch-WithoutComment", "$"+"NetBSD$", "", "--- file.orig", @@ -104,7 +104,7 @@ func (s *Suite) Test_ChecklinesPatch__without_comment(c *check.C) { func (s *Suite) Test_ChecklinesPatch__git_without_comment(c *check.C) { s.Init(c) s.UseCommandLine("-Wall") - lines := s.NewLines("patch-aa", + lines := T.NewLines("patch-aa", "$"+"NetBSD$", "", "diff --git a/aa b/aa", @@ -123,7 +123,7 @@ func (s *Suite) Test_ChecklinesPatch__git_without_comment(c *check.C) { func (s *Suite) Test_checklineOtherAbsolutePathname(c *check.C) { s.Init(c) - line := NewLine("patch-ag", 1, "+$install -s -c ./bin/rosegarden ${DESTDIR}$BINDIR", nil) + line := T.NewLine("patch-ag", 1, "+$install -s -c ./bin/rosegarden ${DESTDIR}$BINDIR") checklineOtherAbsolutePathname(line, line.Text) @@ -133,7 +133,7 @@ func (s *Suite) Test_checklineOtherAbsolutePathname(c *check.C) { func (s *Suite) Test_ChecklinesPatch__error_code(c *check.C) { s.Init(c) s.UseCommandLine("-Wall") - lines := s.NewLines("patch-ErrorCode", + lines := T.NewLines("patch-ErrorCode", "$"+"NetBSD$", "", "*** Error code 1", // Looks like a context diff, but isn't. @@ -154,7 +154,7 @@ func (s *Suite) Test_ChecklinesPatch__error_code(c *check.C) { func (s *Suite) Test_ChecklinesPatch__wrong_header_order(c *check.C) { s.Init(c) s.UseCommandLine("-Wall") - lines := s.NewLines("patch-WrongOrder", + lines := T.NewLines("patch-WrongOrder", "$"+"NetBSD$", "", "Text", @@ -177,7 +177,7 @@ func (s *Suite) Test_ChecklinesPatch__wrong_header_order(c *check.C) { func (s *Suite) Test_ChecklinesPatch__context_diff(c *check.C) { s.Init(c) s.UseCommandLine("-Wall") - lines := s.NewLines("patch-ctx", + lines := T.NewLines("patch-ctx", "$"+"NetBSD$", "", "diff -cr history.c.orig history.c", @@ -193,7 +193,7 @@ func (s *Suite) Test_ChecklinesPatch__context_diff(c *check.C) { func (s *Suite) Test_ChecklinesPatch__no_patch(c *check.C) { s.Init(c) - lines := s.NewLines("patch-aa", + lines := T.NewLines("patch-aa", "$"+"NetBSD$", "", "-- oldfile", @@ -207,7 +207,7 @@ func (s *Suite) Test_ChecklinesPatch__no_patch(c *check.C) { func (s *Suite) Test_ChecklinesPatch__two_patched_files(c *check.C) { s.Init(c) - lines := s.NewLines("patch-aa", + lines := T.NewLines("patch-aa", "$"+"NetBSD$", "", "--- oldfile", @@ -230,7 +230,7 @@ func (s *Suite) Test_ChecklinesPatch__two_patched_files(c *check.C) { func (s *Suite) Test_ChecklinesPatch__documentation_that_looks_like_patch_lines(c *check.C) { s.Init(c) - lines := s.NewLines("patch-aa", + lines := T.NewLines("patch-aa", "$"+"NetBSD$", "", "--- oldfile", @@ -247,7 +247,7 @@ func (s *Suite) Test_ChecklinesPatch__documentation_that_looks_like_patch_lines( func (s *Suite) Test_ChecklinesPatch__only_unified_header_but_no_content(c *check.C) { s.Init(c) - lines := s.NewLines("patch-unified", + lines := T.NewLines("patch-unified", "$"+"NetBSD$", "", "Documentation for the patch", @@ -263,7 +263,7 @@ func (s *Suite) Test_ChecklinesPatch__only_unified_header_but_no_content(c *chec func (s *Suite) Test_ChecklinesPatch__only_context_header_but_no_content(c *check.C) { s.Init(c) - lines := s.NewLines("patch-context", + lines := T.NewLines("patch-context", "$"+"NetBSD$", "", "Documentation for the patch", @@ -281,7 +281,7 @@ func (s *Suite) Test_ChecklinesPatch__only_context_header_but_no_content(c *chec func (s *Suite) Test_ChecklinesPatch__Makefile_with_absolute_pathnames(c *check.C) { s.Init(c) - lines := s.NewLines("patch-unified", + lines := T.NewLines("patch-unified", "$"+"NetBSD$", "", "Documentation for the patch", @@ -317,7 +317,7 @@ func (s *Suite) Test_ChecklinesPatch__Makefile_with_absolute_pathnames(c *check. func (s *Suite) Test_ChecklinesPatch__no_newline_with_text_following(c *check.C) { s.Init(c) - lines := s.NewLines("patch-aa", + lines := T.NewLines("patch-aa", "$"+"NetBSD$", "", "comment", @@ -339,7 +339,7 @@ func (s *Suite) Test_ChecklinesPatch__no_newline_with_text_following(c *check.C) func (s *Suite) Test_ChecklinesPatch__no_newline(c *check.C) { s.Init(c) - lines := s.NewLines("patch-aa", + lines := T.NewLines("patch-aa", "$"+"NetBSD$", "", "comment", @@ -359,7 +359,7 @@ func (s *Suite) Test_ChecklinesPatch__no_newline(c *check.C) { func (s *Suite) Test_ChecklinesPatch__empty_lines_left_out_at_eof(c *check.C) { s.Init(c) - lines := s.NewLines("patch-aa", + lines := T.NewLines("patch-aa", "$"+"NetBSD$", "", "comment", @@ -383,7 +383,7 @@ func (s *Suite) Test_ChecklinesPatch__empty_lines_left_out_at_eof(c *check.C) { // Since this is no problem for patch(1), pkglint also doesn't complain. func (s *Suite) Test_ChecklinesPatch__context_lines_with_tab_instead_of_space(c *check.C) { s.Init(c) - lines := s.NewLines("patch-aa", + lines := T.NewLines("patch-aa", "$"+"NetBSD$", "", "comment", diff --git a/pkgtools/pkglint/files/pkglint.0 b/pkgtools/pkglint/files/pkglint.0 index 0db26929158..b13868c078c 100644 --- a/pkgtools/pkglint/files/pkglint.0 +++ b/pkgtools/pkglint/files/pkglint.0 @@ -14,21 +14,12 @@ DDEESSCCRRIIPPTTIIOONN --CC{{[[nnoo--]]cchheecckk,,......}} Enable or disable specific checks. For a list of checks, see below. - --FF|----aauuttooffiixx Repair trivial things automatically. - - --II Show the _M_a_k_e_f_i_l_e that is constructed by including - all the files that are slurped in via `.include' - directives. This flag is mainly for debugging. - - --VV|----vveerrssiioonn Print the current ppkkgglliinntt version number and exit. - - --WW{{[[nnoo--]]wwaarrnn,,......}} Enable or disable specific warnings. For a list of - warnings, see below. - --dd|----ddeebbuugg Enable or disable verbose log for debugging pkglint. --ee|----eexxppllaaiinn Print verbose explanations for diagnostics. + --FF|----aauuttooffiixx Repair some of the warnings automatically. + --gg|----ggcccc--oouuttppuutt--ffoorrmmaatt Use a format for the diagnostics that is understood by most programs, especially editors, so they can @@ -36,10 +27,20 @@ DDEESSCCRRIIPPTTIIOONN --hh|----hheellpp Show the summary of command line options, then exit. + --II Show the _M_a_k_e_f_i_l_e that is constructed by including + all the files that are slurped in via `.include' + directives. This flag is mainly for debugging. + --ii|----iimmppoorrtt Check if a package is ready to be imported into pkgsrc. This is especially useful for packages from the pkgsrc-wip project. + --oo|----oonnllyy _s_u_b_s_t_r_i_n_g + Only handle those diagnostics that have _s_u_b_s_t_r_i_n_g in + their text. This is useful in combination with + ----aauuttooffiixx and ----rreeccuurrssiivvee,, to fix only a single kind + of warning in a large number of files. + --qq|----qquuiieett Don't print the errors and warnings summary before terminating. @@ -51,6 +52,11 @@ DDEESSCCRRIIPPTTIIOONN diagnostics. This is especially useful together with the --ff|----sshhooww--aauuttooffiixx option. + --VV|----vveerrssiioonn Print the current ppkkgglliinntt version number and exit. + + --WW{{[[nnoo--]]wwaarrnn,,......}} Enable or disable specific warnings. For a list of + warnings, see below. + CChheecckkss aallll Enable all checks. @@ -146,7 +152,7 @@ AAUUTTHHOORRSS Roland Illig <_r_i_l_l_i_g_@_N_e_t_B_S_D_._o_r_g> BBUUGGSS - If you don't understand the messages, feel free to ask on the - <tech-pkg@NetBSD.org> mailing list. + If you don't understand the messages, feel free to ask the author or on + the <pkgsrc-users@pkgsrc.org> mailing list. -NetBSD 7.0 June 5, 2016 NetBSD 7.0 +NetBSD 7.0.2 January 14, 2018 NetBSD 7.0.2 diff --git a/pkgtools/pkglint/files/pkglint.1 b/pkgtools/pkglint/files/pkglint.1 index e0d93d19c9f..b3f2cc858a7 100644 --- a/pkgtools/pkglint/files/pkglint.1 +++ b/pkgtools/pkglint/files/pkglint.1 @@ -1,4 +1,4 @@ -.\" $NetBSD: pkglint.1,v 1.51 2016/06/05 11:24:32 rillig Exp $ +.\" $NetBSD: pkglint.1,v 1.52 2018/01/13 23:56:14 rillig Exp $ .\" From FreeBSD: portlint.1,v 1.8 1997/11/25 14:53:14 itojun Exp .\" .\" Copyright (c) 1997 by Jun-ichiro Itoh <itojun@itojun.org>. @@ -6,9 +6,9 @@ .\" .\" Roland Illig <roland.illig@gmx.de>, 2004, 2005. .\" Thomas Klausner <wiz@NetBSD.org>, 2012. -.\" Roland Illig <rillig@NetBSD.org>, 2015, 2016. +.\" Roland Illig <rillig@NetBSD.org>, 2015-2018. .\" -.Dd June 5, 2016 +.Dd January 14, 2018 .Dt PKGLINT 1 .Os .Sh NAME @@ -29,35 +29,37 @@ to be bugs, or that are simply deprecated. .It Fl C{[no-]check,...} Enable or disable specific checks. For a list of checks, see below. -.It Fl F Ns | Ns Fl -autofix -Repair trivial things automatically. -.It Fl I -Show the -.Pa Makefile -that is constructed by including all the files that -are slurped in via -.Ql .include -directives. -This flag is mainly for debugging. -.It Fl V Ns | Ns Fl -version -Print the current -.Nm -version number and exit. -.It Fl W{[no-]warn,...} -Enable or disable specific warnings. -For a list of warnings, see below. .It Fl d Ns | Ns Fl -debug Enable or disable verbose log for debugging pkglint. .It Fl e Ns | Ns Fl -explain Print verbose explanations for diagnostics. +.It Fl F Ns | Ns Fl -autofix +Repair some of the warnings automatically. .It Fl g Ns | Ns Fl -gcc-output-format Use a format for the diagnostics that is understood by most programs, especially editors, so they can provide a point-and-goto interface. .It Fl h Ns | Ns Fl -help Show the summary of command line options, then exit. +.It Fl I +Show the +.Pa Makefile +that is constructed by including all the files that +are slurped in via +.Ql .include +directives. +This flag is mainly for debugging. .It Fl i Ns | Ns Fl -import Check if a package is ready to be imported into pkgsrc. This is especially useful for packages from the pkgsrc-wip project. +.It Fl o Ns | Ns Fl -only Ar substring +Only handle those diagnostics that have +.Ar substring +in their text. +This is useful in combination with +.Fl -autofix +and +.Fl -recursive, +to fix only a single kind of warning in a large number of files. .It Fl q Ns | Ns Fl -quiet Don't print the errors and warnings summary before terminating. .It Fl r Ns | Ns Fl -recursive @@ -71,6 +73,13 @@ source code along with the diagnostics. This is especially useful together with the .Fl f Ns | Ns Fl -show-autofix option. +.It Fl V Ns | Ns Fl -version +Print the current +.Nm +version number and exit. +.It Fl W{[no-]warn,...} +Enable or disable specific warnings. +For a list of warnings, see below. .El .\" ======================================================================= .Ss Checks @@ -176,6 +185,7 @@ errors. .Sh AUTHORS .An Roland Illig Aq Mt rillig@NetBSD.org .Sh BUGS -If you don't understand the messages, feel free to ask on the -.Aq tech-pkg@NetBSD.org +If you don't understand the messages, feel free to ask the author or +on the +.Aq pkgsrc-users@pkgsrc.org mailing list. diff --git a/pkgtools/pkglint/files/pkglint.go b/pkgtools/pkglint/files/pkglint.go index 614ed54638c..6e99b7117de 100644 --- a/pkgtools/pkglint/files/pkglint.go +++ b/pkgtools/pkglint/files/pkglint.go @@ -2,7 +2,6 @@ package main import ( "fmt" - "io" "netbsd.org/pkglint/getopt" "netbsd.org/pkglint/histogram" "netbsd.org/pkglint/regex" @@ -19,7 +18,7 @@ const confMake = "@BMAKE@" const confVersion = "@VERSION@" func main() { - G.logOut, G.logErr, trace.Out = os.Stdout, os.Stderr, os.Stdout + G.logOut, G.logErr, trace.Out = NewSeparatorWriter(os.Stdout), NewSeparatorWriter(os.Stderr), os.Stdout os.Exit(new(Pkglint).Main(os.Args...)) } @@ -51,7 +50,8 @@ func (pkglint *Pkglint) Main(args ...string) (exitcode int) { regex.Profiling = true G.loghisto = histogram.New() defer func() { - G.loghisto.PrintStats("loghisto", G.logOut, 0) + G.logOut.Write("") + G.loghisto.PrintStats("loghisto", G.logOut.out, 0) regex.PrintStats() }() } @@ -99,6 +99,7 @@ func (pkglint *Pkglint) ParseCommandLine(args []string) *int { opts.AddFlagVar('I', "dumpmakefile", &gopts.DumpMakefile, false, "dump the Makefile after parsing") opts.AddFlagVar('i', "import", &gopts.Import, false, "prepare the import of a wip package") opts.AddFlagVar('m', "log-verbose", &gopts.LogVerbose, false, "allow the same log message more than once") + opts.AddStrList('o', "only", &gopts.LogOnly, "only log messages containing the given text") opts.AddFlagVar('p', "profiling", &gopts.Profiling, false, "profile the executing program") opts.AddFlagVar('q', "quiet", &gopts.Quiet, false, "don't print a summary line when finishing") opts.AddFlagVar('r', "recursive", &gopts.Recursive, false, "check subdirectories, too") @@ -133,21 +134,21 @@ func (pkglint *Pkglint) ParseCommandLine(args []string) *int { remainingArgs, err := opts.Parse(args) if err != nil { - fmt.Fprintf(G.logErr, "%s\n\n", err) - opts.Help(G.logErr, "pkglint [options] dir...") + fmt.Fprintf(G.logErr.out, "%s\n\n", err) + opts.Help(G.logErr.out, "pkglint [options] dir...") exitcode := 1 return &exitcode } gopts.args = remainingArgs if gopts.PrintHelp { - opts.Help(G.logOut, "pkglint [options] dir...") + opts.Help(G.logOut.out, "pkglint [options] dir...") exitcode := 0 return &exitcode } if G.opts.PrintVersion { - fmt.Fprintf(G.logOut, "%s\n", confVersion) + fmt.Fprintf(G.logOut.out, "%s\n", confVersion) exitcode := 0 return &exitcode } @@ -158,20 +159,20 @@ func (pkglint *Pkglint) ParseCommandLine(args []string) *int { func (pkglint *Pkglint) PrintSummary() { if !G.opts.Quiet { if G.errors != 0 || G.warnings != 0 { - fmt.Fprintf(G.logOut, "%d %s and %d %s found.\n", + G.logOut.Printf("%d %s and %d %s found.\n", G.errors, ifelseStr(G.errors == 1, "error", "errors"), G.warnings, ifelseStr(G.warnings == 1, "warning", "warnings")) } else { - io.WriteString(G.logOut, "Looks fine.\n") + G.logOut.WriteLine("Looks fine.") } if G.explanationsAvailable && !G.opts.Explain { - fmt.Fprint(G.logOut, "(Run \"pkglint -e\" to show explanations.)\n") + G.logOut.WriteLine("(Run \"pkglint -e\" to show explanations.)") } if G.autofixAvailable && !G.opts.PrintAutofix && !G.opts.Autofix { - fmt.Fprint(G.logOut, "(Run \"pkglint -fs\" to show what can be fixed automatically.)\n") + G.logOut.WriteLine("(Run \"pkglint -fs\" to show what can be fixed automatically.)") } if G.autofixAvailable && !G.opts.Autofix { - fmt.Fprint(G.logOut, "(Run \"pkglint -F\" to automatically fix some issues.)\n") + G.logOut.WriteLine("(Run \"pkglint -F\" to automatically fix some issues.)") } } } diff --git a/pkgtools/pkglint/files/pkglint_test.go b/pkgtools/pkglint/files/pkglint_test.go index 29184ea3fff..7cded294bae 100644 --- a/pkgtools/pkglint/files/pkglint_test.go +++ b/pkgtools/pkglint/files/pkglint_test.go @@ -36,7 +36,7 @@ func (s *Suite) Test_Pkglint_Main_no_args(c *check.C) { func (s *Suite) Test_Pkglint_coverage(c *check.C) { cmdline := os.Getenv("PKGLINT_TESTCMDLINE") if cmdline != "" { - G.logOut, G.logErr, trace.Out = os.Stdout, os.Stderr, os.Stdout + G.logOut, G.logErr, trace.Out = NewSeparatorWriter(os.Stdout), NewSeparatorWriter(os.Stderr), os.Stdout new(Pkglint).Main(append([]string{"pkglint"}, splitOnSpace(cmdline)...)...) } } @@ -82,7 +82,7 @@ func (s *Suite) Test_Pkglint_CheckDirent(c *check.C) { } func (s *Suite) Test_resolveVariableRefs__circular_reference(c *check.C) { - mkline := NewMkLine(NewLine("fname", 1, "GCC_VERSION=${GCC_VERSION}", nil)) + mkline := T.NewMkLine("fname", 1, "GCC_VERSION=${GCC_VERSION}") G.Pkg = NewPackage(".") G.Pkg.vardef["GCC_VERSION"] = mkline @@ -92,9 +92,9 @@ func (s *Suite) Test_resolveVariableRefs__circular_reference(c *check.C) { } func (s *Suite) Test_resolveVariableRefs__multilevel(c *check.C) { - mkline1 := NewMkLine(NewLine("fname", 10, "_=${SECOND}", nil)) - mkline2 := NewMkLine(NewLine("fname", 11, "_=${THIRD}", nil)) - mkline3 := NewMkLine(NewLine("fname", 12, "_=got it", nil)) + mkline1 := T.NewMkLine("fname", 10, "_=${SECOND}") + mkline2 := T.NewMkLine("fname", 11, "_=${THIRD}") + mkline3 := T.NewMkLine("fname", 12, "_=got it") G.Pkg = NewPackage(".") defineVar(mkline1, "FIRST") defineVar(mkline2, "SECOND") @@ -106,7 +106,7 @@ func (s *Suite) Test_resolveVariableRefs__multilevel(c *check.C) { } func (s *Suite) Test_resolveVariableRefs__special_chars(c *check.C) { - mkline := NewMkLine(NewLine("fname", 10, "_=x11", nil)) + mkline := T.NewMkLine("fname", 10, "_=x11") G.Pkg = NewPackage("category/pkg") G.Pkg.vardef["GST_PLUGINS0.10_TYPE"] = mkline @@ -117,7 +117,7 @@ func (s *Suite) Test_resolveVariableRefs__special_chars(c *check.C) { func (s *Suite) Test_ChecklinesDescr(c *check.C) { s.Init(c) - lines := s.NewLines("DESCR", + lines := T.NewLines("DESCR", strings.Repeat("X", 90), "", "", "", "", "", "", "", "", "10", "Try ${PREFIX}", @@ -134,7 +134,7 @@ func (s *Suite) Test_ChecklinesDescr(c *check.C) { func (s *Suite) Test_ChecklinesMessage__short(c *check.C) { s.Init(c) - lines := s.NewLines("MESSAGE", + lines := T.NewLines("MESSAGE", "one line") ChecklinesMessage(lines) @@ -145,7 +145,7 @@ func (s *Suite) Test_ChecklinesMessage__short(c *check.C) { func (s *Suite) Test_ChecklinesMessage__malformed(c *check.C) { s.Init(c) - lines := s.NewLines("MESSAGE", + lines := T.NewLines("MESSAGE", "1", "2", "3", diff --git a/pkgtools/pkglint/files/plist.go b/pkgtools/pkglint/files/plist.go index 67c58d17745..dfa7a8bad7b 100644 --- a/pkgtools/pkglint/files/plist.go +++ b/pkgtools/pkglint/files/plist.go @@ -128,9 +128,10 @@ func (ck *PlistChecker) checkline(pline *PlistLine) { } else if hasPrefix(text, "$") { ck.checkpath(pline) } else if text == "" { - if !pline.line.AutofixDelete() { - pline.line.Warnf("PLISTs should not contain empty lines.") - } + fix := pline.line.Autofix() + fix.Warnf("PLISTs should not contain empty lines.") + fix.Delete() + fix.Apply() } else { pline.line.Warnf("Unknown line type.") } @@ -147,14 +148,15 @@ func (ck *PlistChecker) checkpath(pline *PlistLine) { pline.warnImakeMannewsuffix() } if hasPrefix(text, "${PKGMANDIR}/") { - if line.AutofixReplace("${PKGMANDIR}/", "man/") { - pline.text = strings.Replace(pline.text, "${PKGMANDIR}/", "man/", 1) - } else { - line.Notef("PLIST files should mention \"man/\" instead of \"${PKGMANDIR}\".") - Explain( - "The pkgsrc infrastructure takes care of replacing the correct value", - "when generating the actual PLIST for the package.") - } + fix := pline.line.Autofix() + fix.Notef("PLIST files should mention \"man/\" instead of \"${PKGMANDIR}\".") + fix.Explain( + "The pkgsrc infrastructure takes care of replacing the correct value", + "when generating the actual PLIST for the package.") + fix.Replace("${PKGMANDIR}/", "man/") + fix.Apply() + + pline.text = strings.Replace(pline.text, "${PKGMANDIR}/", "man/", 1) } topdir := "" @@ -210,9 +212,10 @@ func (ck *PlistChecker) checkSorted(pline *PlistLine) { "To fix this, run \"pkglint -F PLIST\".") } if prev := ck.allFiles[text]; prev != nil && prev != pline { - if !pline.line.AutofixDelete() { - pline.line.Errorf("Duplicate filename %q, already appeared in %s.", text, prev.line.ReferenceFrom(pline.line)) - } + fix := pline.line.Autofix() + fix.Errorf("Duplicate filename %q, already appeared in %s.", text, prev.line.ReferenceFrom(pline.line)) + fix.Delete() + fix.Apply() } } ck.lastFname = text @@ -309,13 +312,16 @@ func (ck *PlistChecker) checkpathMan(pline *PlistLine) { } } - if gz != "" && !line.AutofixReplaceRegexp(`\.gz$`, "") { - line.Notef("The .gz extension is unnecessary for manual pages.") - Explain( + if gz != "" { + fix := line.Autofix() + fix.Notef("The .gz extension is unnecessary for manual pages.") + fix.Explain( "Whether the manual pages are installed in compressed form or not is", "configured by the pkgsrc user. Compression and decompression takes", "place automatically, no matter if the .gz extension is mentioned in", "the PLIST or not.") + fix.ReplaceRegex(`\.gz\n`, "\n") + fix.Apply() } } @@ -516,7 +522,10 @@ func (s *plistLineSorter) Sort() { return } - firstLine.AutofixMark("Sorting the whole file.") + fix := firstLine.Autofix() + fix.Describef("Sorting the whole file.") + fix.Apply() + var lines []Line for _, pline := range s.header { lines = append(lines, pline.line) @@ -527,6 +536,6 @@ func (s *plistLineSorter) Sort() { for _, pline := range s.footer { lines = append(lines, pline.line) } - lines[0].Changed = true // Without this, autofix doesn't know that anything changed + s.autofixed = SaveAutofixChanges(lines) } diff --git a/pkgtools/pkglint/files/plist_test.go b/pkgtools/pkglint/files/plist_test.go index 398b338bcd2..861ec2a92dd 100644 --- a/pkgtools/pkglint/files/plist_test.go +++ b/pkgtools/pkglint/files/plist_test.go @@ -8,7 +8,7 @@ func (s *Suite) Test_ChecklinesPlist(c *check.C) { s.Init(c) s.UseCommandLine("-Wall") G.Pkg = NewPackage("category/pkgbase") - lines := s.NewLines("PLIST", + lines := T.NewLines("PLIST", "bin/i386/6c", "bin/program", "etc/my.cnf", @@ -51,7 +51,7 @@ func (s *Suite) Test_ChecklinesPlist(c *check.C) { func (s *Suite) Test_ChecklinesPlist__empty(c *check.C) { s.Init(c) - lines := s.NewLines("PLIST", + lines := T.NewLines("PLIST", "@comment $"+"NetBSD$") ChecklinesPlist(lines) @@ -78,7 +78,7 @@ func (s *Suite) Test_ChecklinesPlist__conditional(c *check.C) { s.Init(c) G.Pkg = NewPackage("category/pkgbase") G.Pkg.plistSubstCond["PLIST.bincmds"] = true - lines := s.NewLines("PLIST", + lines := T.NewLines("PLIST", "@comment $"+"NetBSD$", "${PLIST.bincmds}bin/subdir/command") @@ -91,7 +91,7 @@ func (s *Suite) Test_ChecklinesPlist__conditional(c *check.C) { func (s *Suite) Test_ChecklinesPlist__sorting(c *check.C) { s.Init(c) s.UseCommandLine("-Wplist-sort") - lines := s.NewLines("PLIST", + lines := T.NewLines("PLIST", "@comment $"+"NetBSD$", "@comment Do not remove", "sbin/i386/6c", @@ -110,7 +110,7 @@ func (s *Suite) Test_PlistLineSorter_Sort(c *check.C) { s.Init(c) s.UseCommandLine("--autofix") tmpfile := s.CreateTmpFile("PLIST", "dummy\n") - lines := s.NewLines(tmpfile, + lines := T.NewLines(tmpfile, "@comment $"+"NetBSD$", "@comment Do not remove", "A", @@ -171,7 +171,7 @@ func (s *Suite) Test_PlistChecker_checkpathShare_Desktop(c *check.C) { s.UseCommandLine("-Wextra") G.Pkg = NewPackage("category/pkgpath") - ChecklinesPlist(s.NewLines("PLIST", + ChecklinesPlist(T.NewLines("PLIST", "@comment $"+"NetBSD$", "share/applications/pkgbase.desktop")) @@ -182,10 +182,11 @@ func (s *Suite) Test_PlistChecker_checkpathShare_Desktop(c *check.C) { func (s *Suite) Test_PlistChecker_checkpathMan_gz(c *check.C) { s.Init(c) G.Pkg = NewPackage("category/pkgbase") - - ChecklinesPlist(s.NewLines("PLIST", + lines := T.NewLines("PLIST", "@comment $"+"NetBSD$", - "man/man3/strerror.3.gz")) + "man/man3/strerror.3.gz") + + ChecklinesPlist(lines) s.CheckOutputLines( "NOTE: PLIST:2: The .gz extension is unnecessary for manual pages.") @@ -193,7 +194,7 @@ func (s *Suite) Test_PlistChecker_checkpathMan_gz(c *check.C) { func (s *Suite) TestPlistChecker_checkpath__PKGMANDIR(c *check.C) { s.Init(c) - lines := s.NewLines("PLIST", + lines := T.NewLines("PLIST", "@comment $"+"NetBSD$", "${PKGMANDIR}/man1/sh.1") @@ -205,7 +206,7 @@ func (s *Suite) TestPlistChecker_checkpath__PKGMANDIR(c *check.C) { func (s *Suite) TestPlistChecker_checkpath__python_egg(c *check.C) { s.Init(c) - lines := s.NewLines("PLIST", + lines := T.NewLines("PLIST", "@comment $"+"NetBSD$", "${PYSITELIB}/gdspy-${PKGVERSION}-py${PYVERSSUFFIX}.egg-info/PKG-INFO") diff --git a/pkgtools/pkglint/files/shell_test.go b/pkgtools/pkglint/files/shell_test.go index c9395b08478..e99670e1bb6 100644 --- a/pkgtools/pkglint/files/shell_test.go +++ b/pkgtools/pkglint/files/shell_test.go @@ -1,7 +1,7 @@ package main import ( - check "gopkg.in/check.v1" + "gopkg.in/check.v1" "netbsd.org/pkglint/textproc" ) @@ -107,15 +107,20 @@ func (s *Suite) Test_splitIntoShellTokens__redirect(c *check.C) { func (s *Suite) Test_ShellLine_CheckShellCommandLine(c *check.C) { s.Init(c) s.UseCommandLine("-Wall") - G.Mk = s.NewMkLines("fname", - "# dummy") - shline := NewShellLine(G.Mk.mklines[0]) - shline.CheckShellCommandLine("@# Comment") + checkShellCommandLine := func(shellCommand string) { + G.Mk = T.NewMkLines("fname", + "\t"+shellCommand) + shline := NewShellLine(G.Mk.mklines[0]) + + shline.CheckShellCommandLine(shline.mkline.Shellcmd()) + } + + checkShellCommandLine("@# Comment") s.CheckOutputEmpty() - shline.CheckShellCommandLine("uname=`uname`; echo $$uname; echo") + checkShellCommandLine("uname=`uname`; echo $$uname; echo") s.CheckOutputLines( "WARN: fname:1: Unknown shell command \"uname\".", @@ -123,60 +128,58 @@ func (s *Suite) Test_ShellLine_CheckShellCommandLine(c *check.C) { "WARN: fname:1: Unknown shell command \"echo\".") s.RegisterTool(&Tool{Name: "echo", Predefined: true}) - G.Mk = s.NewMkLines("fname", - "# dummy") G.globalData.InitVartypes() - shline.CheckShellCommandLine("echo ${PKGNAME:Q}") // vucQuotPlain + checkShellCommandLine("echo ${PKGNAME:Q}") // vucQuotPlain s.CheckOutputLines( "WARN: fname:1: PKGNAME may not be used in this file; it would be ok in Makefile, Makefile.*, *.mk.", "NOTE: fname:1: The :Q operator isn't necessary for ${PKGNAME} here.") - shline.CheckShellCommandLine("echo \"${CFLAGS:Q}\"") // vucQuotDquot + checkShellCommandLine("echo \"${CFLAGS:Q}\"") // vucQuotDquot s.CheckOutputLines( "WARN: fname:1: Please don't use the :Q operator in double quotes.", "WARN: fname:1: CFLAGS may not be used in this file; it would be ok in Makefile, Makefile.common, options.mk, *.mk.", "WARN: fname:1: Please use ${CFLAGS:M*:Q} instead of ${CFLAGS:Q} and make sure the variable appears outside of any quoting characters.") - shline.CheckShellCommandLine("echo '${COMMENT:Q}'") // vucQuotSquot + checkShellCommandLine("echo '${COMMENT:Q}'") // vucQuotSquot s.CheckOutputLines( "WARN: fname:1: COMMENT may not be used in any file; it is a write-only variable.", "WARN: fname:1: Please move ${COMMENT:Q} outside of any quoting characters.") - shline.CheckShellCommandLine("echo target=$@ exitcode=$$? '$$' \"\\$$\"") + checkShellCommandLine("echo target=$@ exitcode=$$? '$$' \"\\$$\"") s.CheckOutputLines( "WARN: fname:1: Please use \"${.TARGET}\" instead of \"$@\".", "WARN: fname:1: The $? shell variable is often not available in \"set -e\" mode.") - shline.CheckShellCommandLine("echo $$@") + checkShellCommandLine("echo $$@") s.CheckOutputLines( "WARN: fname:1: The $@ shell variable should only be used in double quotes.") - shline.CheckShellCommandLine("echo \"$$\"") // As seen by make(1); the shell sees: echo "$" + checkShellCommandLine("echo \"$$\"") // As seen by make(1); the shell sees: echo "$" s.CheckOutputLines( "WARN: fname:1: Pkglint parse error in ShTokenizer.ShAtom at \"$$\\\"\" (quoting=d)", "WARN: fname:1: Pkglint ShellLine.CheckShellCommand: parse error at [\"]") - shline.CheckShellCommandLine("echo \"\\n\"") + checkShellCommandLine("echo \"\\n\"") s.CheckOutputEmpty() - shline.CheckShellCommandLine("${RUN} for f in *.c; do echo $${f%.c}; done") + checkShellCommandLine("${RUN} for f in *.c; do echo $${f%.c}; done") s.CheckOutputEmpty() - shline.CheckShellCommandLine("${RUN} echo $${variable+set}") + checkShellCommandLine("${RUN} echo $${variable+set}") s.CheckOutputEmpty() // Based on mail/thunderbird/Makefile, rev. 1.159 - shline.CheckShellCommandLine("${RUN} subdir=\"`unzip -c \"$$e\" install.rdf | awk '/re/ { print \"hello\" }'`\"") + checkShellCommandLine("${RUN} subdir=\"`unzip -c \"$$e\" install.rdf | awk '/re/ { print \"hello\" }'`\"") s.CheckOutputLines( "WARN: fname:1: The exitcode of \"unzip\" at the left of the | operator is ignored.", @@ -184,7 +187,7 @@ func (s *Suite) Test_ShellLine_CheckShellCommandLine(c *check.C) { "WARN: fname:1: Unknown shell command \"awk\".") // From mail/thunderbird/Makefile, rev. 1.159 - shline.CheckShellCommandLine("" + + checkShellCommandLine("" + "${RUN} for e in ${XPI_FILES}; do " + " subdir=\"`${UNZIP_CMD} -c \"$$e\" install.rdf | awk '/^ <em:id>/ {sub(\".*<em:id>\",\"\");sub(\"</em:id>.*\",\"\");print;exit;}'`\" && " + " ${MKDIR} \"${WRKDIR}/extensions/$$subdir\" && " + @@ -202,7 +205,7 @@ func (s *Suite) Test_ShellLine_CheckShellCommandLine(c *check.C) { "WARN: fname:1: UNZIP_CMD is used but not defined. Spelling mistake?") // From x11/wxGTK28/Makefile - shline.CheckShellCommandLine("" + + checkShellCommandLine("" + "set -e; cd ${WRKSRC}/locale; " + "for lang in *.po; do " + " [ \"$${lang}\" = \"wxstd.po\" ] && continue; " + @@ -214,7 +217,7 @@ func (s *Suite) Test_ShellLine_CheckShellCommandLine(c *check.C) { "WARN: fname:1: Unknown shell command \"[\".", "WARN: fname:1: Unknown shell command \"${TOOLS_PATH.msgfmt}\".") - shline.CheckShellCommandLine("@cp from to") + checkShellCommandLine("@cp from to") s.CheckOutputLines( "WARN: fname:1: The shell command \"cp\" should not be hidden.", @@ -224,14 +227,14 @@ func (s *Suite) Test_ShellLine_CheckShellCommandLine(c *check.C) { G.Pkg.PlistDirs["share/pkgbase"] = true // A directory that is found in the PLIST. - shline.CheckShellCommandLine("${RUN} ${INSTALL_DATA_DIR} share/pkgbase ${PREFIX}/share/pkgbase") + checkShellCommandLine("${RUN} ${INSTALL_DATA_DIR} share/pkgbase ${PREFIX}/share/pkgbase") s.CheckOutputLines( "NOTE: fname:1: You can use AUTO_MKDIRS=yes or \"INSTALLATION_DIRS+= share/pkgbase\" instead of \"${INSTALL_DATA_DIR}\".", "WARN: fname:1: The INSTALL_*_DIR commands can only handle one directory at a time.") // A directory that is not found in the PLIST. - shline.CheckShellCommandLine("${RUN} ${INSTALL_DATA_DIR} ${PREFIX}/share/other") + checkShellCommandLine("${RUN} ${INSTALL_DATA_DIR} ${PREFIX}/share/other") s.CheckOutputLines( "NOTE: fname:1: You can use \"INSTALLATION_DIRS+= share/other\" instead of \"${INSTALL_DATA_DIR}\".") @@ -239,7 +242,7 @@ func (s *Suite) Test_ShellLine_CheckShellCommandLine(c *check.C) { G.Pkg = nil // See PR 46570, item "1. It does not" - shline.CheckShellCommandLine("for x in 1 2 3; do echo \"$$x\" || exit 1; done") + checkShellCommandLine("for x in 1 2 3; do echo \"$$x\" || exit 1; done") s.CheckOutputEmpty() // No warning about missing error checking. } @@ -249,7 +252,7 @@ func (s *Suite) Test_ShellLine_CheckShellCommandLine__nofix(c *check.C) { s.UseCommandLine("-Wall") G.globalData.InitVartypes() s.RegisterTool(&Tool{Name: "echo", Predefined: true}) - G.Mk = s.NewMkLines("Makefile", + G.Mk = T.NewMkLines("Makefile", "\techo ${PKGNAME:Q}") shline := NewShellLine(G.Mk.mklines[0]) @@ -264,7 +267,7 @@ func (s *Suite) Test_ShellLine_CheckShellCommandLine__show_autofix(c *check.C) { s.UseCommandLine("-Wall", "--show-autofix") G.globalData.InitVartypes() s.RegisterTool(&Tool{Name: "echo", Predefined: true}) - G.Mk = s.NewMkLines("Makefile", + G.Mk = T.NewMkLines("Makefile", "\techo ${PKGNAME:Q}") shline := NewShellLine(G.Mk.mklines[0]) @@ -285,7 +288,7 @@ func (s *Suite) Test_ShellLine_CheckShellCommandLine__exitcode(c *check.C) { s.RegisterTool(&Tool{Name: "printf", Predefined: true}) s.RegisterTool(&Tool{Name: "sed", Predefined: true}) s.RegisterTool(&Tool{Name: "right-side", Predefined: true}) - G.Mk = s.NewMkLines("Makefile", + G.Mk = T.NewMkLines("Makefile", "\t echo | right-side", "\t sed s,s,s, | right-side", "\t printf | sed s,s,s, | right-side ", @@ -313,7 +316,7 @@ func (s *Suite) Test_ShellLine_CheckShellCommandLine__autofix(c *check.C) { s.UseCommandLine("-Wall", "--autofix") G.globalData.InitVartypes() s.RegisterTool(&Tool{Name: "echo", Predefined: true}) - G.Mk = s.NewMkLines("Makefile", + G.Mk = T.NewMkLines("Makefile", "\techo ${PKGNAME:Q}") shline := NewShellLine(G.Mk.mklines[0]) @@ -327,7 +330,7 @@ func (s *Suite) Test_ShellLine_CheckShellCommandLine__implementation(c *check.C) s.Init(c) s.UseCommandLine("-Wall") G.globalData.InitVartypes() - G.Mk = s.NewMkLines("fname", + G.Mk = T.NewMkLines("fname", "# dummy") shline := NewShellLine(G.Mk.mklines[0]) @@ -353,7 +356,7 @@ func (s *Suite) Test_ShellLine_CheckShellCommandLine__implementation(c *check.C) func (s *Suite) Test_ShellLine_CheckShelltext__dollar_without_variable(c *check.C) { s.Init(c) G.globalData.InitVartypes() - G.Mk = s.NewMkLines("fname", + G.Mk = T.NewMkLines("fname", "# dummy") shline := NewShellLine(G.Mk.mklines[0]) s.RegisterTool(&Tool{Name: "pax", Varname: "PAX"}) @@ -368,50 +371,55 @@ func (s *Suite) Test_ShellLine_CheckWord(c *check.C) { s.Init(c) s.UseCommandLine("-Wall") G.globalData.InitVartypes() - shline := NewShellLine(NewMkLine(NewLine("fname", 1, "# dummy", nil))) - shline.CheckWord("${${list}}", false) + checkWord := func(shellWord string, checkQuoting bool) { + shline := T.NewShellLine("dummy.mk", 1, "\t echo "+shellWord) + + shline.CheckWord(shellWord, checkQuoting) + } + + checkWord("${${list}}", false) + + checkWord("${${list}}", false) s.CheckOutputEmpty() // No warning for variables that are completely indirect. - shline.CheckWord("${SED_FILE.${id}}", false) + checkWord("${SED_FILE.${id}}", false) s.CheckOutputEmpty() // No warning for variables that are partly indirect. - shline.CheckWord("\"$@\"", false) + checkWord("\"$@\"", false) s.CheckOutputLines( - "WARN: fname:1: Please use \"${.TARGET}\" instead of \"$@\".") + "WARN: dummy.mk:1: Please use \"${.TARGET}\" instead of \"$@\".") - shline.CheckWord("${COMMENT:Q}", true) + checkWord("${COMMENT:Q}", true) s.CheckOutputLines( - "WARN: fname:1: COMMENT may not be used in any file; it is a write-only variable.") + "WARN: dummy.mk:1: COMMENT may not be used in any file; it is a write-only variable.") - shline.CheckWord("\"${DISTINFO_FILE:Q}\"", true) + checkWord("\"${DISTINFO_FILE:Q}\"", true) s.CheckOutputLines( - "WARN: fname:1: DISTINFO_FILE may not be used in this file; it would be ok in Makefile, Makefile.*, *.mk.", - "NOTE: fname:1: The :Q operator isn't necessary for ${DISTINFO_FILE} here.") + "NOTE: dummy.mk:1: The :Q operator isn't necessary for ${DISTINFO_FILE} here.") - shline.CheckWord("embed${DISTINFO_FILE:Q}ded", true) + checkWord("embed${DISTINFO_FILE:Q}ded", true) s.CheckOutputLines( - "WARN: fname:1: DISTINFO_FILE may not be used in this file; it would be ok in Makefile, Makefile.*, *.mk.", - "NOTE: fname:1: The :Q operator isn't necessary for ${DISTINFO_FILE} here.") + "NOTE: dummy.mk:1: The :Q operator isn't necessary for ${DISTINFO_FILE} here.") - shline.CheckWord("s,\\.,,", true) + checkWord("s,\\.,,", true) s.CheckOutputEmpty() - shline.CheckWord("\"s,\\.,,\"", true) + checkWord("\"s,\\.,,\"", true) s.CheckOutputEmpty() } func (s *Suite) Test_ShellLine_CheckWord__dollar_without_variable(c *check.C) { s.Init(c) - shline := NewShellLine(NewMkLine(NewLine("fname", 1, "# dummy", nil))) + shline := T.NewShellLine("fname", 1, "# dummy") shline.CheckWord("/.*~$$//g", false) // Typical argument to pax(1). @@ -422,9 +430,9 @@ func (s *Suite) Test_ShellLine_CheckShellCommandLine__echo(c *check.C) { s.Init(c) s.UseCommandLine("-Wall") s.RegisterTool(&Tool{Name: "echo", Varname: "ECHO", MustUseVarForm: true, Predefined: true}) - G.Mk = s.NewMkLines("fname", + G.Mk = T.NewMkLines("fname", "# dummy") - mkline := NewMkLine(NewLine("fname", 3, "# dummy", nil)) + mkline := T.NewMkLine("fname", 3, "# dummy") MkLineChecker{mkline}.checkText("echo \"hello, world\"") @@ -440,7 +448,7 @@ func (s *Suite) Test_ShellLine_CheckShellCommandLine__shell_variables(c *check.C s.Init(c) text := "\tfor f in *.pl; do ${SED} s,@PREFIX@,${PREFIX}, < $f > $f.tmp && ${MV} $f.tmp $f; done" - shline := NewShellLine(NewMkLine(NewLine("Makefile", 3, text, nil))) + shline := T.NewShellLine("Makefile", 3, text) shline.CheckShellCommandLine(text) s.CheckOutputLines( @@ -463,11 +471,11 @@ func (s *Suite) Test_ShellLine_CheckShellCommandLine__shell_variables(c *check.C func (s *Suite) Test_ShellLine_checkCommandUse(c *check.C) { s.Init(c) - G.Mk = s.NewMkLines("fname", + G.Mk = T.NewMkLines("fname", "# dummy") G.Mk.target = "do-install" - shline := NewShellLine(NewMkLine(NewLine("fname", 1, "\tdummy", nil))) + shline := T.NewShellLine("fname", 1, "\tdummy") shline.checkCommandUse("sed") @@ -501,7 +509,7 @@ func (s *Suite) Test_splitIntoMkWords(c *check.C) { func (s *Suite) Test_ShellLine_CheckShellCommandLine__sed_and_mv(c *check.C) { s.Init(c) - shline := NewShellLine(NewMkLine(NewLine("Makefile", 85, "\t${RUN} ${SED} 's,#,// comment:,g' fname > fname.tmp; ${MV} fname.tmp fname", nil))) + shline := T.NewShellLine("Makefile", 85, "\t${RUN} ${SED} 's,#,// comment:,g' fname > fname.tmp; ${MV} fname.tmp fname") shline.CheckShellCommandLine(shline.mkline.Shellcmd()) @@ -511,7 +519,7 @@ func (s *Suite) Test_ShellLine_CheckShellCommandLine__sed_and_mv(c *check.C) { func (s *Suite) Test_ShellLine_CheckShellCommandLine__subshell(c *check.C) { s.Init(c) - shline := NewShellLine(NewMkLine(NewLine("Makefile", 85, "\t${RUN} uname=$$(uname)", nil))) + shline := T.NewShellLine("Makefile", 85, "\t${RUN} uname=$$(uname)") shline.CheckShellCommandLine(shline.mkline.Shellcmd()) @@ -521,7 +529,7 @@ func (s *Suite) Test_ShellLine_CheckShellCommandLine__subshell(c *check.C) { func (s *Suite) Test_ShellLine_CheckShellCommandLine__install_dir(c *check.C) { s.Init(c) - shline := NewShellLine(NewMkLine(NewLine("Makefile", 85, "\t${RUN} ${INSTALL_DATA_DIR} ${DESTDIR}${PREFIX}/dir1 ${DESTDIR}${PREFIX}/dir2", nil))) + shline := T.NewShellLine("Makefile", 85, "\t${RUN} ${INSTALL_DATA_DIR} ${DESTDIR}${PREFIX}/dir1 ${DESTDIR}${PREFIX}/dir2") shline.CheckShellCommandLine(shline.mkline.Shellcmd()) @@ -546,7 +554,7 @@ func (s *Suite) Test_ShellLine_CheckShellCommandLine__install_dir(c *check.C) { func (s *Suite) Test_ShellLine_CheckShellCommandLine__install_option_d(c *check.C) { s.Init(c) - shline := NewShellLine(NewMkLine(NewLine("Makefile", 85, "\t${RUN} ${INSTALL} -d ${DESTDIR}${PREFIX}/dir1 ${DESTDIR}${PREFIX}/dir2", nil))) + shline := T.NewShellLine("Makefile", 85, "\t${RUN} ${INSTALL} -d ${DESTDIR}${PREFIX}/dir1 ${DESTDIR}${PREFIX}/dir2") shline.CheckShellCommandLine(shline.mkline.Shellcmd()) @@ -571,13 +579,14 @@ func (s *Suite) Test_ShellLine__shell_comment_with_line_continuation(c *check.C) } func (s *Suite) Test_ShellLine_unescapeBackticks(c *check.C) { - shline := NewShellLine(NewMkLine(dummyLine)) + shline := T.NewShellLine("dummy.mk", 13, "# dummy") // foobar="`echo \"foo bar\"`" text := "foobar=\"`echo \\\"foo bar\\\"`\"" repl := textproc.NewPrefixReplacer(text) repl.AdvanceStr("foobar=\"`") backtCommand, newQuoting := shline.unescapeBackticks(text, repl, shqDquotBackt) + c.Check(backtCommand, equals, "echo \"foo bar\"") c.Check(newQuoting, equals, shqDquot) c.Check(repl.Rest(), equals, "\"") diff --git a/pkgtools/pkglint/files/substcontext_test.go b/pkgtools/pkglint/files/substcontext_test.go index 6ddeedea2cb..add6048a85c 100644 --- a/pkgtools/pkglint/files/substcontext_test.go +++ b/pkgtools/pkglint/files/substcontext_test.go @@ -186,5 +186,5 @@ func simulateSubstLines(texts ...string) { } func newSubstLine(lineno int, text string) MkLine { - return NewMkLine(NewLine("Makefile", lineno, text, nil)) + return T.NewMkLine("Makefile", lineno, text) } diff --git a/pkgtools/pkglint/files/vartypecheck.go b/pkgtools/pkglint/files/vartypecheck.go index c7a40e3516b..2f0b27d59db 100644 --- a/pkgtools/pkglint/files/vartypecheck.go +++ b/pkgtools/pkglint/files/vartypecheck.go @@ -448,25 +448,21 @@ func (cv *VartypeCheck) Homepage() { } } fixedURL := baseURL + subdir - explain := false + fix := cv.Line.Autofix() if baseURL != "" { - if !cv.Line.AutofixReplace(wrong, fixedURL) { - cv.Line.Warnf("HOMEPAGE should not be defined in terms of MASTER_SITEs. Use %s directly.", fixedURL) - explain = true - } + fix.Warnf("HOMEPAGE should not be defined in terms of MASTER_SITEs. Use %s directly.", fixedURL) } else { - cv.Line.Warnf("HOMEPAGE should not be defined in terms of MASTER_SITEs.") - explain = true - } - if explain { - Explain( - "The HOMEPAGE is a single URL, while MASTER_SITES is a list of URLs.", - "As long as this list has exactly one element, this works, but as", - "soon as another site is added, the HOMEPAGE would not be a valid", - "URL anymore.", - "", - "Defining MASTER_SITES=${HOMEPAGE} is ok, though.") + fix.Warnf("HOMEPAGE should not be defined in terms of MASTER_SITEs.") } + fix.Explain( + "The HOMEPAGE is a single URL, while MASTER_SITES is a list of URLs.", + "As long as this list has exactly one element, this works, but as", + "soon as another site is added, the HOMEPAGE would not be a valid", + "URL anymore.", + "", + "Defining MASTER_SITES=${HOMEPAGE} is ok, though.") + fix.Replace(wrong, fixedURL) + fix.Apply() } } diff --git a/pkgtools/pkglint/files/vartypecheck_test.go b/pkgtools/pkglint/files/vartypecheck_test.go index 3728a9bca43..cb460591654 100644 --- a/pkgtools/pkglint/files/vartypecheck_test.go +++ b/pkgtools/pkglint/files/vartypecheck_test.go @@ -3,7 +3,7 @@ package main import ( "fmt" - check "gopkg.in/check.v1" + "gopkg.in/check.v1" ) func (s *Suite) Test_VartypeCheck_AwkCommand(c *check.C) { @@ -535,7 +535,7 @@ func runVartypeChecks(varname string, op MkOperator, checker func(*VartypeCheck) panic("runVartypeChecks needs an assignment operator") } for i, value := range values { - mkline := NewMkLine(NewLine("fname", i+1, varname+op.String()+value, nil)) + mkline := T.NewMkLine("fname", i+1, varname+op.String()+value) valueNovar := mkline.WithoutMakeVariables(mkline.Value()) vc := &VartypeCheck{mkline, mkline.Line, mkline.Varname(), mkline.Op(), mkline.Value(), valueNovar, "", false} checker(vc) @@ -545,7 +545,7 @@ func runVartypeChecks(varname string, op MkOperator, checker func(*VartypeCheck) func runVartypeMatchChecks(varname string, checker func(*VartypeCheck), values ...string) { for i, value := range values { text := fmt.Sprintf(".if ${%s:M%s} == \"\"", varname, value) - mkline := NewMkLine(NewLine("fname", i+1, text, nil)) + mkline := T.NewMkLine("fname", i+1, text) valueNovar := mkline.WithoutMakeVariables(value) vc := &VartypeCheck{mkline, mkline.Line, varname, opUseMatch, value, valueNovar, "", false} checker(vc) @@ -554,7 +554,7 @@ func runVartypeMatchChecks(varname string, checker func(*VartypeCheck), values . func runVartypeChecksFname(fname, varname string, op MkOperator, checker func(*VartypeCheck), values ...string) { for i, value := range values { - mkline := NewMkLine(NewLine(fname, i+1, varname+op.String()+value, nil)) + mkline := T.NewMkLine(fname, i+1, varname+op.String()+value) valueNovar := mkline.WithoutMakeVariables(value) vc := &VartypeCheck{mkline, mkline.Line, varname, op, value, valueNovar, "", false} checker(vc) |