pkgtools/pkglint: update to 5.5.3

Changes since 5.5.2:

* Fixed lots of bugs regarding autofixing variable assignments in
  continuation lines.
* Fixed checking of MESSAGE files, which also get fixed now.
* In variable assignments, commented assignments are aligned too.
* Fixed a crash when checking an empty patch file.
* The :Q modifier is only checked on predefined variables, to prevent
  the --autofix mode from removing :Q from user-defined variables.
* Fixed lots of bugs in PLIST autofixing: relevant lines had been
  removed, and the sorting was not correct.
This commit is contained in:
rillig 2018-01-28 23:21:16 +00:00
parent bff4597ffc
commit deaf48767a
21 changed files with 652 additions and 147 deletions

View file

@ -1,6 +1,6 @@
# $NetBSD: Makefile,v 1.527 2018/01/28 13:40:22 rillig Exp $
# $NetBSD: Makefile,v 1.528 2018/01/28 23:21:16 rillig Exp $
PKGNAME= pkglint-5.5.2
PKGNAME= pkglint-5.5.3
DISTFILES= # none
CATEGORIES= pkgtools

View file

@ -62,25 +62,42 @@ func (fix *Autofix) ReplaceAfter(prefix, from string, to string) {
}
}
func (fix *Autofix) ReplaceRegex(from regex.Pattern, to string) {
// ReplaceRegex replaces the first or all occurrences of the `from` pattern
// with the fixed string `toText`. Placeholders like `$1` are _not_ expanded.
// (If you know how to do the expansion correctly, feel free to implement it.)
func (fix *Autofix) ReplaceRegex(from regex.Pattern, toText string, howOften int) {
if fix.skip() {
return
}
done := 0
for _, rawLine := range fix.lines {
if rawLine.Lineno != 0 {
if replaced := regex.Compile(from).ReplaceAllString(rawLine.textnl, to); replaced != rawLine.textnl {
var froms []string // The strings that have actually changed
replace := func(fromText string) string {
if howOften >= 0 && done >= howOften {
return fromText
}
froms = append(froms, fromText)
done++
return toText
}
if replaced := regex.Compile(from).ReplaceAllStringFunc(rawLine.textnl, replace); replaced != rawLine.textnl {
if G.opts.PrintAutofix || G.opts.Autofix {
rawLine.textnl = replaced
}
fix.Describef(rawLine.Lineno, "Replacing regular expression %q with %q.", from, to)
for _, fromText := range froms {
fix.Describef(rawLine.Lineno, "Replacing %q with %q.", fromText, toText)
}
}
}
}
}
func (fix *Autofix) Realign(mkline MkLine, newWidth int) {
if fix.skip() || !mkline.IsMultiline() || !mkline.IsVarassign() {
if fix.skip() || !mkline.IsMultiline() || !(mkline.IsVarassign() || mkline.IsCommentedVarassign()) {
return
}
@ -90,19 +107,19 @@ func (fix *Autofix) Realign(mkline MkLine, newWidth int) {
{
// Interpreting the continuation marker as variable value
// is cheating, but works well.
m, _, _, _, valueAlign, value, _, _ := MatchVarassign(mkline.raw[0].orignl)
m, _, _, _, _, valueAlign, value, _, _ := MatchVarassign(mkline.raw[0].orignl)
if m && value != "\\" {
oldWidth = tabWidth(valueAlign)
}
}
for _, rawLine := range fix.lines[1:] {
_, space := regex.Match1(rawLine.textnl, `^(\s*)`)
width := tabWidth(space)
if oldWidth == 0 || width < oldWidth {
_, comment, space := regex.Match2(rawLine.textnl, `^(#?)([ \t]*)`)
width := tabWidth(comment + space)
if (oldWidth == 0 || width < oldWidth) && width >= 8 && rawLine.textnl != "\n" {
oldWidth = width
}
if !regex.Matches(space, `^\t*\s{0,7}`) {
if !regex.Matches(space, `^\t* {0,7}`) {
normalized = false
}
}
@ -119,10 +136,11 @@ func (fix *Autofix) Realign(mkline MkLine, newWidth int) {
}
for _, rawLine := range fix.lines[1:] {
_, oldSpace := regex.Match1(rawLine.textnl, `^(\s*)`)
_, comment, oldSpace := regex.Match2(rawLine.textnl, `^(#?)([ \t]*)`)
newWidth := tabWidth(oldSpace) - oldWidth + newWidth
newSpace := strings.Repeat("\t", newWidth/8) + strings.Repeat(" ", newWidth%8)
if replaced := strings.Replace(rawLine.textnl, oldSpace, newSpace, 1); replaced != rawLine.textnl {
replaced := strings.Replace(rawLine.textnl, comment+oldSpace, comment+newSpace, 1)
if replaced != rawLine.textnl {
if G.opts.PrintAutofix || G.opts.Autofix {
rawLine.textnl = replaced
}
@ -131,6 +149,8 @@ func (fix *Autofix) Realign(mkline MkLine, newWidth int) {
}
}
// InsertBefore prepends a line before the current line.
// The newline is added internally.
func (fix *Autofix) InsertBefore(text string) {
if fix.skip() {
return
@ -140,6 +160,8 @@ func (fix *Autofix) InsertBefore(text string) {
fix.Describef(fix.lines[0].Lineno, "Inserting a line %q before this line.", text)
}
// InsertBefore appends a line after the current line.
// The newline is added internally.
func (fix *Autofix) InsertAfter(text string) {
if fix.skip() {
return

View file

@ -13,7 +13,7 @@ func (s *Suite) Test_Autofix_ReplaceRegex(c *check.C) {
fix := lines[1].Autofix()
fix.Warnf("Something's wrong here.")
fix.ReplaceRegex(`.`, "X")
fix.ReplaceRegex(`.`, "X", -1)
fix.Apply()
SaveAutofixChanges(lines)
@ -24,7 +24,11 @@ func (s *Suite) Test_Autofix_ReplaceRegex(c *check.C) {
"line3")
t.CheckOutputLines(
"WARN: ~/Makefile:2: Something's wrong here.",
"AUTOFIX: ~/Makefile:2: Replacing regular expression \".\" with \"X\".")
"AUTOFIX: ~/Makefile:2: Replacing \"l\" with \"X\".",
"AUTOFIX: ~/Makefile:2: Replacing \"i\" with \"X\".",
"AUTOFIX: ~/Makefile:2: Replacing \"n\" with \"X\".",
"AUTOFIX: ~/Makefile:2: Replacing \"e\" with \"X\".",
"AUTOFIX: ~/Makefile:2: Replacing \"2\" with \"X\".")
}
func (s *Suite) Test_Autofix_ReplaceRegex_with_autofix(c *check.C) {
@ -38,27 +42,32 @@ func (s *Suite) Test_Autofix_ReplaceRegex_with_autofix(c *check.C) {
fix := lines[1].Autofix()
fix.Warnf("Something's wrong here.")
fix.ReplaceRegex(`.`, "X")
fix.ReplaceRegex(`.`, "X", 3)
fix.Apply()
t.CheckOutputLines(
"AUTOFIX: ~/Makefile:2: Replacing \"l\" with \"X\".",
"AUTOFIX: ~/Makefile:2: Replacing \"i\" with \"X\".",
"AUTOFIX: ~/Makefile:2: Replacing \"n\" with \"X\".",
"-\tline2",
"+\tXXXe2")
fix.Warnf("Use Y instead of X.")
fix.Replace("X", "Y")
fix.Apply()
t.CheckOutputLines(
"",
"AUTOFIX: ~/Makefile:2: Replacing \"X\" with \"Y\".",
"-\tline2",
"+\tYXXe2")
SaveAutofixChanges(lines)
t.CheckFileLines("Makefile",
"line1",
"YXXXX",
"YXXe2",
"line3")
t.CheckOutputLines(
"AUTOFIX: ~/Makefile:2: Replacing regular expression \".\" with \"X\".",
"-\tline2",
"+\tXXXXX",
"",
"AUTOFIX: ~/Makefile:2: Replacing \"X\" with \"Y\".",
"-\tline2",
"+\tYXXXX")
}
func (s *Suite) Test_Autofix_ReplaceRegex_with_show_autofix(c *check.C) {
@ -72,7 +81,7 @@ func (s *Suite) Test_Autofix_ReplaceRegex_with_show_autofix(c *check.C) {
fix := lines[1].Autofix()
fix.Warnf("Something's wrong here.")
fix.ReplaceRegex(`.`, "X")
fix.ReplaceRegex(`.`, "X", -1)
fix.Apply()
fix.Warnf("Use Y instead of X.")
@ -83,7 +92,11 @@ func (s *Suite) Test_Autofix_ReplaceRegex_with_show_autofix(c *check.C) {
t.CheckOutputLines(
"WARN: ~/Makefile:2: Something's wrong here.",
"AUTOFIX: ~/Makefile:2: Replacing regular expression \".\" with \"X\".",
"AUTOFIX: ~/Makefile:2: Replacing \"l\" with \"X\".",
"AUTOFIX: ~/Makefile:2: Replacing \"i\" with \"X\".",
"AUTOFIX: ~/Makefile:2: Replacing \"n\" with \"X\".",
"AUTOFIX: ~/Makefile:2: Replacing \"e\" with \"X\".",
"AUTOFIX: ~/Makefile:2: Replacing \"2\" with \"X\".",
"-\tline2",
"+\tXXXXX",
"",
@ -108,17 +121,27 @@ func (s *Suite) Test_autofix_MkLines(c *check.C) {
fix := mklines.mklines[1].Autofix()
fix.Warnf("Something's wrong here.")
fix.ReplaceRegex(`.`, "X")
fix.ReplaceRegex(`...`, "XXX", -1)
fix.Apply()
fix = mklines.mklines[2].Autofix()
fix.Warnf("Something's wrong here.")
fix.ReplaceRegex(`...`, "XXX", 1)
fix.Apply()
SaveAutofixChanges(mklines.lines)
t.CheckOutputLines(
"AUTOFIX: ~/Makefile:2: Replacing \"lin\" with \"XXX\".",
"AUTOFIX: ~/Makefile:2: Replacing \"e2 \" with \"XXX\".",
"AUTOFIX: ~/Makefile:2: Replacing \":= \" with \"XXX\".",
"AUTOFIX: ~/Makefile:2: Replacing \"val\" with \"XXX\".",
"AUTOFIX: ~/Makefile:2: Replacing \"ue2\" with \"XXX\".",
"AUTOFIX: ~/Makefile:3: Replacing \"lin\" with \"XXX\".")
t.CheckFileLines("Makefile",
"line1 := value1",
"XXXXXXXXXXXXXXX",
"line3 := value3")
t.CheckOutputLines(
"AUTOFIX: ~/Makefile:2: Replacing regular expression \".\" with \"X\".")
"XXXe3 := value3")
}
func (s *Suite) Test_Autofix_multiple_modifications(c *check.C) {
@ -134,14 +157,14 @@ func (s *Suite) Test_Autofix_multiple_modifications(c *check.C) {
{
fix := line.Autofix()
fix.Warnf("Silent-Magic-Diagnostic")
fix.ReplaceRegex(`(.)(.*)(.)`, "$3$2$1")
fix.ReplaceRegex(`(.)(.*)(.)`, "lriginao", 1) // XXX: the replacement should be "$3$2$1"
fix.Apply()
}
c.Check(line.autofix, check.NotNil)
c.Check(line.raw, check.DeepEquals, t.NewRawLines(1, "original\n", "lriginao\n"))
t.CheckOutputLines(
"AUTOFIX: fname:1: Replacing regular expression \"(.)(.*)(.)\" with \"$3$2$1\".")
"AUTOFIX: fname:1: Replacing \"original\" with \"lriginao\".")
{
fix := line.Autofix()
@ -304,7 +327,7 @@ func (s *Suite) Test_Autofix_suppress_unfixable_warnings(c *check.C) {
fix := lines[1].Autofix()
fix.Warnf("Something's wrong here.")
fix.ReplaceRegex(`.`, "X")
fix.ReplaceRegex(`.`, "X", -1)
fix.Apply()
fix.Warnf("The XXX marks are usually not fixed, use TODO instead.")
@ -315,7 +338,11 @@ func (s *Suite) Test_Autofix_suppress_unfixable_warnings(c *check.C) {
t.CheckOutputLines(
"WARN: Makefile:2: Something's wrong here.",
"AUTOFIX: Makefile:2: Replacing regular expression \".\" with \"X\".",
"AUTOFIX: Makefile:2: Replacing \"l\" with \"X\".",
"AUTOFIX: Makefile:2: Replacing \"i\" with \"X\".",
"AUTOFIX: Makefile:2: Replacing \"n\" with \"X\".",
"AUTOFIX: Makefile:2: Replacing \"e\" with \"X\".",
"AUTOFIX: Makefile:2: Replacing \"2\" with \"X\".",
"-\tline2",
"+\tXXXXX",
"",
@ -333,7 +360,7 @@ func (s *Suite) Test_Autofix_failed_replace(c *check.C) {
fix := line.Autofix()
fix.Warnf("All-uppercase words should not be used at all.")
fix.ReplaceRegex(`\b[A-Z]{3,}\b`, "---censored---")
fix.ReplaceRegex(`\b[A-Z]{3,}\b`, "---censored---", -1)
fix.Apply()
// No output since there was no all-uppercase word in the text.

View file

@ -41,7 +41,7 @@ func CheckdirCategory() {
// collect the SUBDIRs in the Makefile and in the file system.
fSubdirs := getSubdirs(G.CurrentDir)
sort.Sort(sort.StringSlice(fSubdirs))
sort.Strings(fSubdirs)
var mSubdirs []subdir
prevSubdir := ""

View file

@ -26,7 +26,7 @@ func LoadExistingLines(fname string, joinBackslashLines bool) []Line {
return lines
}
func getLogicalLine(fname string, rawLines []*RawLine, pindex *int) Line {
func nextLogicalLine(fname string, rawLines []*RawLine, pindex *int) Line {
{ // Handle the common case efficiently
index := *pindex
rawLine := rawLines[index]
@ -68,26 +68,30 @@ func getLogicalLine(fname string, rawLines []*RawLine, pindex *int) Line {
}
func splitRawLine(textnl string) (leadingWhitespace, text, trailingWhitespace, cont string) {
i, m := 0, len(textnl)
end := len(textnl)
if m > i && textnl[m-1] == '\n' {
m--
if end-1 >= 0 && textnl[end-1] == '\n' {
end--
}
if m > i && textnl[m-1] == '\\' {
m--
cont = textnl[m : m+1]
backslashes := 0
for end-1 >= 0 && textnl[end-1] == '\\' {
end--
backslashes++
}
cont = textnl[end : end+backslashes%2]
end += backslashes / 2
trailingEnd := m
for m > i && (textnl[m-1] == ' ' || textnl[m-1] == '\t') {
m--
trailingEnd := end
for end-1 >= 0 && (textnl[end-1] == ' ' || textnl[end-1] == '\t') {
end--
}
trailingStart := m
trailingStart := end
trailingWhitespace = textnl[trailingStart:trailingEnd]
i := 0
leadingStart := i
for i < m && (textnl[i] == ' ' || textnl[i] == '\t') {
for i < end && (textnl[i] == ' ' || textnl[i] == '\t') {
i++
}
leadingEnd := i
@ -117,7 +121,7 @@ func convertToLogicalLines(fname string, rawText string, joinBackslashLines bool
var loglines []Line
if joinBackslashLines {
for lineno := 0; lineno < len(rawLines); {
loglines = append(loglines, getLogicalLine(fname, rawLines, &lineno))
loglines = append(loglines, nextLogicalLine(fname, rawLines, &lineno))
}
} else {
for _, rawLine := range rawLines {

View file

@ -1,7 +1,7 @@
package main
import (
check "gopkg.in/check.v1"
"gopkg.in/check.v1"
)
func (s *Suite) Test_convertToLogicalLines_no_continuation(c *check.C) {
@ -29,6 +29,92 @@ func (s *Suite) Test_convertToLogicalLines_continuation(c *check.C) {
c.Check(lines[1].String(), equals, "fname_cont:3: third")
}
// In Makefiles, comment lines can also have continuations.
// See devel/bmake/files/unit-tests/comment.mk
func (s *Suite) Test_convertToLogicalLines__comments(c *check.C) {
t := s.Init(c)
lines := t.SetupFileLinesContinuation("comment.mk",
"# This is a comment",
"",
"#\\",
"\tMultiline comment",
"# Another escaped comment \\",
"that \\",
"goes \\",
"on",
"# This is NOT an escaped comment due to the double backslashes \\\\",
"VAR=\tThis is not a comment",
"",
"#\\",
"This is a comment",
"#\\\\",
"This is no comment",
"#\\\\\\",
"This is a comment",
"#\\\\\\\\",
"This is no comment",
"#\\\\\\\\\\",
"This is a comment",
"#\\\\\\\\\\\\",
"This is no comment")
var texts []string
for _, line := range lines {
texts = append(texts, line.Text)
}
c.Check(texts, deepEquals, []string{
"# This is a comment",
"",
"# Multiline comment",
"# Another escaped comment that goes on",
"# This is NOT an escaped comment due to the double backslashes \\",
"VAR=\tThis is not a comment",
"",
"# This is a comment",
"#\\",
"This is no comment",
"#\\ This is a comment",
"#\\\\",
"This is no comment",
"#\\\\ This is a comment",
"#\\\\\\",
"This is no comment"})
var rawTexts []string
for _, line := range lines {
for _, rawLine := range line.raw {
rawTexts = append(rawTexts, rawLine.textnl)
}
}
c.Check(rawTexts, deepEquals, []string{
"# This is a comment\n",
"\n",
"#\\\n",
"\tMultiline comment\n",
"# Another escaped comment \\\n",
"that \\\n",
"goes \\\n",
"on\n",
"# This is NOT an escaped comment due to the double backslashes \\\\\n",
"VAR=\tThis is not a comment\n",
"\n",
"#\\\n",
"This is a comment\n",
"#\\\\\n",
"This is no comment\n",
"#\\\\\\\n",
"This is a comment\n",
"#\\\\\\\\\n",
"This is no comment\n",
"#\\\\\\\\\\\n",
"This is a comment\n",
"#\\\\\\\\\\\\\n",
"This is no comment\n"})
}
func (s *Suite) Test_convertToLogicalLines_continuationInLastLine(c *check.C) {
t := s.Init(c)

View file

@ -108,8 +108,8 @@ func (gd *GlobalData) loadDistSites() {
name2url := make(map[string]string)
url2name := make(map[string]string)
for _, line := range lines {
if m, varname, _, _, _, urls, _, _ := MatchVarassign(line.Text); m {
if hasPrefix(varname, "MASTER_SITE_") && varname != "MASTER_SITE_BACKUP" {
if m, commented, varname, _, _, _, urls, _, _ := MatchVarassign(line.Text); m {
if !commented && hasPrefix(varname, "MASTER_SITE_") && varname != "MASTER_SITE_BACKUP" {
for _, url := range splitOnSpace(urls) {
if matches(url, `^(?:http://|https://|ftp://)`) {
if name2url[varname] == "" {
@ -187,8 +187,11 @@ func (gd *GlobalData) loadTools() {
lines := LoadExistingLines(fname, true)
for _, line := range lines {
text := line.Text
if hasPrefix(text, "#") {
continue
}
if m, varname, _, _, _, value, _, _ := MatchVarassign(text); m {
if m, _, varname, _, _, _, value, _, _ := MatchVarassign(text); m {
if varname == "USE_TOOLS" {
if trace.Tracing {
trace.Stepf("[condDepth=%d] %s", condDepth, value)
@ -618,7 +621,10 @@ func (tr *ToolRegistry) Trace() {
}
func (tr *ToolRegistry) ParseToolLine(line Line) {
if m, varname, _, _, _, value, _, _ := MatchVarassign(line.Text); m {
if m, commented, varname, _, _, _, value, _, _ := MatchVarassign(line.Text); m {
if commented {
return
}
if varname == "TOOLS_CREATE" && (value == "[" || matches(value, `^?[-\w.]+$`)) {
tr.Register(value)

View file

@ -19,6 +19,11 @@ func (h *Histogram) Add(s string, n int) {
}
func (h *Histogram) PrintStats(caption string, out io.Writer, limit int) {
type entry struct {
s string
count int
}
entries := make([]entry, len(h.histo))
i := 0
@ -27,7 +32,11 @@ func (h *Histogram) PrintStats(caption string, out io.Writer, limit int) {
i++
}
sort.Sort(byCountDesc(entries))
sort.SliceStable(entries, func(i, j int) bool {
ei := entries[i]
ej := entries[j]
return ej.count < ei.count || (ei.count == ej.count && ei.s < ej.s)
})
for i, entry := range entries {
fmt.Fprintf(out, "%s %6d %s\n", caption, entry.count, entry.s)
@ -36,20 +45,3 @@ func (h *Histogram) PrintStats(caption string, out io.Writer, limit int) {
}
}
}
type entry struct {
s string
count int
}
type byCountDesc []entry
func (a byCountDesc) Len() int {
return len(a)
}
func (a byCountDesc) Swap(i, j int) {
a[i], a[j] = a[j], a[i]
}
func (a byCountDesc) Less(i, j int) bool {
return a[j].count < a[i].count || (a[i].count == a[j].count && a[i].s < a[j].s)
}

View file

@ -54,7 +54,7 @@ func CheckLineTrailingWhitespace(line Line) {
fix.Explain(
"When a line ends with some white-space, that space is in most cases",
"irrelevant and can be removed.")
fix.ReplaceRegex(`\s+\n$`, "\n")
fix.ReplaceRegex(`[ \t\r]+\n$`, "\n", 1)
fix.Apply()
}
}

View file

@ -16,6 +16,7 @@ type MkLineImpl struct {
data interface{} // One of the following mkLine* types
}
type mkLineAssign struct {
commented bool // Whether the whole variable assignment is commented out
varname string // e.g. "HOMEPAGE", "SUBST_SED.perl"
varcanon string // e.g. "HOMEPAGE", "SUBST_SED.*"
varparam string // e.g. "", "perl"
@ -59,7 +60,7 @@ func NewMkLine(line Line) (mkline *MkLineImpl) {
"white-space.")
}
if m, varname, spaceAfterVarname, op, valueAlign, value, spaceAfterValue, comment := MatchVarassign(text); m {
if m, commented, varname, spaceAfterVarname, op, valueAlign, value, spaceAfterValue, comment := MatchVarassign(text); m {
if G.opts.WarnSpace && spaceAfterVarname != "" {
switch {
case hasSuffix(varname, "+") && op == "=":
@ -85,6 +86,7 @@ func NewMkLine(line Line) (mkline *MkLineImpl) {
varparam := varnameParam(varname)
mkline.data = mkLineAssign{
commented,
varname,
varnameCanon(varname),
varparam,
@ -148,13 +150,46 @@ func NewMkLine(line Line) (mkline *MkLineImpl) {
func (mkline *MkLineImpl) String() string {
return fmt.Sprintf("%s:%s", mkline.Filename, mkline.Linenos())
}
func (mkline *MkLineImpl) IsVarassign() bool { _, ok := mkline.data.(mkLineAssign); return ok }
func (mkline *MkLineImpl) IsShellcmd() bool { _, ok := mkline.data.(mkLineShell); return ok }
func (mkline *MkLineImpl) IsComment() bool { _, ok := mkline.data.(mkLineComment); return ok }
func (mkline *MkLineImpl) IsEmpty() bool { _, ok := mkline.data.(mkLineEmpty); return ok }
func (mkline *MkLineImpl) IsVarassign() bool {
data, ok := mkline.data.(mkLineAssign)
return ok && !data.commented
}
// IsCommentedVarassign returns true for commented-out variable assignments.
// In most cases these are treated as ordinary comments, but in some others
// they are treated like variable assignments, just inactive ones.
func (mkline *MkLineImpl) IsCommentedVarassign() bool {
data, ok := mkline.data.(mkLineAssign)
return ok && data.commented
}
// IsShellcmd returns true for tab-indented lines that are assigned to a Make
// target. Example:
//
// pre-configure: # IsDependency
// ${ECHO} # IsShellcmd
func (mkline *MkLineImpl) IsShellcmd() bool {
_, ok := mkline.data.(mkLineShell)
return ok
}
func (mkline *MkLineImpl) IsComment() bool {
_, ok := mkline.data.(mkLineComment)
return ok || mkline.IsCommentedVarassign()
}
func (mkline *MkLineImpl) IsEmpty() bool {
_, ok := mkline.data.(mkLineEmpty)
return ok
}
// IsCond checks whether the line is a conditional (.if/.ifelse/.else/.if) or a loop (.for/.endfor).
func (mkline *MkLineImpl) IsCond() bool { _, ok := mkline.data.(mkLineConditional); return ok }
func (mkline *MkLineImpl) IsCond() bool {
_, ok := mkline.data.(mkLineConditional)
return ok
}
func (mkline *MkLineImpl) IsInclude() bool {
incl, ok := mkline.data.(mkLineInclude)
return ok && !incl.sys
@ -163,11 +198,15 @@ func (mkline *MkLineImpl) IsSysinclude() bool {
incl, ok := mkline.data.(mkLineInclude)
return ok && incl.sys
}
func (mkline *MkLineImpl) IsDependency() bool { _, ok := mkline.data.(mkLineDependency); return ok }
func (mkline *MkLineImpl) Varname() string { return mkline.data.(mkLineAssign).varname }
func (mkline *MkLineImpl) Varcanon() string { return mkline.data.(mkLineAssign).varcanon }
func (mkline *MkLineImpl) Varparam() string { return mkline.data.(mkLineAssign).varparam }
func (mkline *MkLineImpl) Op() MkOperator { return mkline.data.(mkLineAssign).op }
func (mkline *MkLineImpl) IsDependency() bool {
_, ok := mkline.data.(mkLineDependency)
return ok
}
func (mkline *MkLineImpl) Varname() string { return mkline.data.(mkLineAssign).varname }
func (mkline *MkLineImpl) Varcanon() string { return mkline.data.(mkLineAssign).varcanon }
func (mkline *MkLineImpl) Varparam() string { return mkline.data.(mkLineAssign).varparam }
func (mkline *MkLineImpl) Op() MkOperator { return mkline.data.(mkLineAssign).op }
// For a variable assignment, the text up to and including the assignment operator, e.g. VARNAME+=\t
func (mkline *MkLineImpl) ValueAlign() string { return mkline.data.(mkLineAssign).valueAlign }
@ -356,6 +395,9 @@ func (mkline *MkLineImpl) VariableNeedsQuoting(varname string, vartype *Vartype,
if vartype.basicType.IsEnum() || vartype.IsBasicSafe() {
if vartype.kindOfList == lkNone {
if vartype.guessed {
return nqDontKnow
}
return nqDoesntMatter
}
if vartype.kindOfList == lkShell && !vuc.IsWordPart {
@ -740,11 +782,16 @@ func (ind *Indentation) Varnames() string {
return varnames
}
func MatchVarassign(text string) (m bool, varname, spaceAfterVarname, op, valueAlign, value, spaceAfterValue, comment string) {
func MatchVarassign(text string) (m, commented bool, varname, spaceAfterVarname, op, valueAlign, value, spaceAfterValue, comment string) {
i, n := 0, len(text)
for i < n && text[i] == ' ' {
if i < n && text[i] == '#' {
commented = true
i++
} else {
for i < n && text[i] == ' ' {
i++
}
}
varnameStart := i

View file

@ -644,6 +644,36 @@ func (s *Suite) Test_MkLine_variableNeedsQuoting__tool_in_CONFIGURE_ENV(c *check
"NOTE: Makefile:3: The :Q operator isn't necessary for ${TOOLS_TAR} here.")
}
// For some well-known directory variables like WRKDIR, PREFIX, LOCALBASE,
// the :Q modifier can be safely removed since pkgsrc will never support
// having special characters in these directory names.
// For guessed variable types be cautious and don't autofix them.
func (s *Suite) Test_MkLine_variableNeedsQuoting__only_remove_known(c *check.C) {
t := s.Init(c)
t.SetupCommandLine("-Wall", "--autofix")
G.globalData.InitVartypes()
lines := t.SetupFileLinesContinuation("Makefile",
MkRcsId,
"",
"demo: .PHONY",
"\t${ECHO} ${WRKSRC:Q}",
"\t${ECHO} ${FOODIR:Q}")
mklines := NewMkLines(lines)
mklines.Check()
t.CheckOutputLines(
"AUTOFIX: ~/Makefile:4: Replacing \"${WRKSRC:Q}\" with \"${WRKSRC}\".")
t.CheckFileLines("Makefile",
MkRcsId,
"",
"demo: .PHONY",
"\t${ECHO} ${WRKSRC}",
"\t${ECHO} ${FOODIR:Q}")
}
func (s *Suite) Test_MkLine_Pkgmandir(c *check.C) {
t := s.Init(c)
@ -750,38 +780,46 @@ func (s *Suite) Test_MkLine_ConditionVars(c *check.C) {
func (s *Suite) Test_MatchVarassign(c *check.C) {
s.Init(c)
checkVarassign := func(text string, ck check.Checker, varname, spaceAfterVarname, op, align, value, spaceAfterValue, comment string) {
checkVarassign := func(text string, commented bool, varname, spaceAfterVarname, op, align, value, spaceAfterValue, comment string) {
type VarAssign struct {
commented bool
varname, spaceAfterVarname, op, align, value, spaceAfterValue, comment string
}
expected := VarAssign{varname, spaceAfterVarname, op, align, value, spaceAfterValue, comment}
am, avarname, aspaceAfterVarname, aop, aalign, avalue, aspaceAfterValue, acomment := MatchVarassign(text)
expected := VarAssign{commented, varname, spaceAfterVarname, op, align, value, spaceAfterValue, comment}
am, acommented, avarname, aspaceAfterVarname, aop, aalign, avalue, aspaceAfterValue, acomment := MatchVarassign(text)
if !am {
c.Errorf("Text %q doesn't match variable assignment", text)
return
}
actual := VarAssign{avarname, aspaceAfterVarname, aop, aalign, avalue, aspaceAfterValue, acomment}
c.Check(actual, ck, expected)
actual := VarAssign{acommented, avarname, aspaceAfterVarname, aop, aalign, avalue, aspaceAfterValue, acomment}
c.Check(actual, equals, expected)
}
checkNotVarassign := func(text string) {
m, _, _, _, _, _, _, _ := MatchVarassign(text)
m, _, _, _, _, _, _, _, _ := MatchVarassign(text)
if m {
c.Errorf("Text %q matches variable assignment, but shouldn't.", text)
}
}
checkVarassign("C++=c11", equals, "C+", "", "+=", "C++=", "c11", "", "")
checkVarassign("V=v", equals, "V", "", "=", "V=", "v", "", "")
checkVarassign("VAR=#comment", equals, "VAR", "", "=", "VAR=", "", "", "#comment")
checkVarassign("VAR=\\#comment", equals, "VAR", "", "=", "VAR=", "#comment", "", "")
checkVarassign("VAR=\\\\\\##comment", equals, "VAR", "", "=", "VAR=", "\\\\#", "", "#comment")
checkVarassign("VAR=\\", equals, "VAR", "", "=", "VAR=", "\\", "", "")
checkVarassign("VAR += value", equals, "VAR", " ", "+=", "VAR += ", "value", "", "")
checkVarassign(" VAR=value", equals, "VAR", "", "=", " VAR=", "value", "", "")
checkVarassign("VAR=value #comment", equals, "VAR", "", "=", "VAR=", "value", " ", "#comment")
checkVarassign("C++=c11", false, "C+", "", "+=", "C++=", "c11", "", "")
checkVarassign("V=v", false, "V", "", "=", "V=", "v", "", "")
checkVarassign("VAR=#comment", false, "VAR", "", "=", "VAR=", "", "", "#comment")
checkVarassign("VAR=\\#comment", false, "VAR", "", "=", "VAR=", "#comment", "", "")
checkVarassign("VAR=\\\\\\##comment", false, "VAR", "", "=", "VAR=", "\\\\#", "", "#comment")
checkVarassign("VAR=\\", false, "VAR", "", "=", "VAR=", "\\", "", "")
checkVarassign("VAR += value", false, "VAR", " ", "+=", "VAR += ", "value", "", "")
checkVarassign(" VAR=value", false, "VAR", "", "=", " VAR=", "value", "", "")
checkVarassign("VAR=value #comment", false, "VAR", "", "=", "VAR=", "value", " ", "#comment")
checkVarassign("#VAR=value", true, "VAR", "", "=", "#VAR=", "value", "", "")
checkNotVarassign("\tVAR=value")
checkNotVarassign("?=value")
checkNotVarassign("<=value")
checkNotVarassign("#")
// A single space is typically used for writing documentation,
// not for commenting out code.
checkNotVarassign("# VAR=value")
}
func (s *Suite) Test_Indentation(c *check.C) {

View file

@ -191,7 +191,7 @@ func (ck MkLineChecker) checkDirectiveIndentation(expectedDepth int) {
if expected := strings.Repeat(" ", expectedDepth); indent != expected {
fix := mkline.Line.Autofix()
fix.Notef("This directive should be indented by %d spaces.", expectedDepth)
fix.Replace("."+indent, "."+expected)
fix.ReplaceRegex(regex.Pattern(`^\.`+indent), "."+expected, 1)
fix.Apply()
}
}

View file

@ -322,3 +322,34 @@ func (s *Suite) Test_MkLineChecker_CheckVartype_CFLAGS(c *check.C) {
"WARN: Makefile:2: Unknown compiler flag \"-bs\".",
"WARN: Makefile:2: Compiler flag \"%s\\\\\\\"\" should start with a hyphen.")
}
// Up to 2018-01-28, pkglint applied the autofix also to the continuation
// lines, which is incorrect. It replaced the dot in "4.*" with spaces.
func (s *Suite) Test_MkLineChecker_checkDirectiveIndentation_autofix(c *check.C) {
t := s.Init(c)
t.SetupCommandLine("-Wall", "--autofix")
G.globalData.InitVartypes()
lines := t.SetupFileLinesContinuation("options.mk",
MkRcsId,
".if ${PKGNAME} == pkgname",
".if \\",
" ${PLATFORM:MNetBSD-4.*}",
".endif",
".endif")
mklines := NewMkLines(lines)
mklines.Check()
t.CheckOutputLines(
"AUTOFIX: ~/options.mk:3: Replacing \".\" with \". \".",
"AUTOFIX: ~/options.mk:5: Replacing \".\" with \". \".")
t.CheckFileLines("options.mk",
MkRcsId,
".if ${PKGNAME} == pkgname",
". if \\",
" ${PLATFORM:MNetBSD-4.*}",
". endif",
".endif")
}

View file

@ -260,28 +260,42 @@ func (va *VaralignBlock) Check(mkline MkLine) {
case !G.opts.WarnSpace:
return
case mkline.IsEmpty():
va.Finish()
return
case mkline.IsCommentedVarassign():
break
case mkline.IsComment():
return
case mkline.IsCond():
return
case mkline.IsEmpty():
va.Finish()
return
case !mkline.IsVarassign():
trace.Stepf("Skipping")
va.skip = true
return
}
switch {
case mkline.Op() == opAssignEval && matches(mkline.Varname(), `^[a-z]`):
// Arguments to procedures do not take part in block alignment.
//
// Example:
// pkgpath := ${PKGPATH}
return
case mkline.Value() == "" && mkline.VarassignComment() == "":
// Multiple-inclusion guards usually appear in a block of
// their own and therefore do not need alignment.
//
// Example:
// .if !defined(INCLUSION_GUARD_MK)
// INCLUSION_GUARD_MK:=
// # ...
// .endif
return
}
@ -289,7 +303,7 @@ func (va *VaralignBlock) Check(mkline MkLine) {
if mkline.IsMultiline() {
// Interpreting the continuation marker as variable value
// is cheating, but works well.
m, _, _, _, _, value, _, _ := MatchVarassign(mkline.raw[0].orignl)
m, _, _, _, _, _, value, _, _ := MatchVarassign(mkline.raw[0].orignl)
continuation = m && value == "\\"
}
@ -330,8 +344,8 @@ func (va *VaralignBlock) Finish() {
// variable from forcing the rest of the paragraph to be indented too
// far to the right.
func (va *VaralignBlock) optimalWidth(infos []*varalignBlockInfo) int {
longest := 0
secondLongest := 0
longest := 0 // The longest seen varnameOpWidth
secondLongest := 0 // The second-longest seen varnameOpWidth
for _, info := range infos {
if info.continuation {
continue

View file

@ -850,8 +850,8 @@ func (s *Suite) Test_Varalign__indented_continuation_line_in_paragraph(c *check.
}
// Up to 2018-01-27, it could happen that some source code was logged
// without a corresponding diagnostic. This was unintented and confusing.
func (s *Suite) Test_Varalign__bla(c *check.C) {
// without a corresponding diagnostic. This was unintended and confusing.
func (s *Suite) Test_Varalign__fix_without_diagnostic(c *check.C) {
vt := NewVaralignTester(s, c)
vt.Input(
"MESSAGE_SUBST+=\t\tRUBY_DISTNAME=${RUBY_DISTNAME}",
@ -870,3 +870,114 @@ func (s *Suite) Test_Varalign__bla(c *check.C) {
vt.source = true
vt.Run()
}
// The two variables look like they were in two separate paragraphs, but
// they aren't. This is because the continuation line from the DISTFILES
// eats up the empty line that would otherwise separate the paragraphs.
func (s *Suite) Test_Varalign__continuation_line_last_empty(c *check.C) {
vt := NewVaralignTester(s, c)
vt.Input(
"DISTFILES= \\",
"\ta \\",
"\tb \\",
"\tc \\",
"",
"NEXT_VAR=\tmust not be indented")
vt.Diagnostics(
"NOTE: ~/Makefile:1--5: This variable value should be aligned with tabs, not spaces, to column 17.")
vt.Autofixes(
"AUTOFIX: ~/Makefile:1: Replacing \" \" with \"\\t\".")
vt.Fixed(
"DISTFILES= \\",
" a \\",
" b \\",
" c \\",
"",
"NEXT_VAR= must not be indented")
vt.Run()
}
// Commented-out variables take part in the realignment.
// The TZ=UTC below is part of the two-line comment since make(1)
// interprets it in the same way.
func (s *Suite) Test_Varalign__realign_commented_single_lines(c *check.C) {
vt := NewVaralignTester(s, c)
vt.Input(
"SHORT=\tvalue",
"#DISTFILES=\tdistfile-1.0.0.tar.gz",
"#CONTINUATION= \\",
"#\t\tcontinued",
"#CONFIGURE_ENV+= \\",
"#TZ=UTC",
"SHORT=\tvalue")
vt.Diagnostics(
"NOTE: ~/Makefile:1: This variable value should be aligned to column 17.",
"NOTE: ~/Makefile:3--4: This variable value should be aligned with tabs, not spaces, to column 17.",
"NOTE: ~/Makefile:5--6: This line should be aligned with \"\\t\\t\".",
"NOTE: ~/Makefile:7: This variable value should be aligned to column 17.")
vt.Autofixes(
"AUTOFIX: ~/Makefile:1: Replacing \"\\t\" with \"\\t\\t\".",
"AUTOFIX: ~/Makefile:3: Replacing \" \" with \"\\t\".",
"AUTOFIX: ~/Makefile:6: Replacing indentation \"\" with \"\\t\\t\".",
"AUTOFIX: ~/Makefile:7: Replacing \"\\t\" with \"\\t\\t\".")
vt.Fixed(
"SHORT= value",
"#DISTFILES= distfile-1.0.0.tar.gz",
"#CONTINUATION= \\",
"# continued",
"#CONFIGURE_ENV+= \\",
"# TZ=UTC",
"SHORT= value")
vt.Run()
}
func (s *Suite) Test_Varalign__realign_commented_continuation_line(c *check.C) {
vt := NewVaralignTester(s, c)
vt.Input(
"BEFORE=\tvalue",
"#COMMENTED= \\",
"#\tvalue1 \\",
"#\tvalue2 \\",
"#\tvalue3 \\",
"AFTER=\tafter") // This line continues the comment.
vt.Diagnostics()
vt.Autofixes()
vt.Fixed(
"BEFORE= value",
"#COMMENTED= \\",
"# value1 \\",
"# value2 \\",
"# value3 \\",
"AFTER= after")
vt.Run()
}
// The HOMEPAGE is completely ignored. Since its value is empty it doesn't
// need any alignment. Whether it is commented out doesn't matter.
func (s *Suite) Test_Varalign__realign_variable_without_value(c *check.C) {
vt := NewVaralignTester(s, c)
vt.Input(
"COMMENT=\t\tShort description of the package",
"#HOMEPAGE=")
vt.Diagnostics()
vt.Autofixes()
vt.Fixed(
"COMMENT= Short description of the package",
"#HOMEPAGE=")
vt.Run()
}
// This commented multiline variable is already perfectly aligned.
// Nothing needs to be fixed.
func (s *Suite) Test_Varalign__realign_commented_multiline(c *check.C) {
vt := NewVaralignTester(s, c)
vt.Input(
"#CONF_FILES+=\t\tfile1 \\",
"#\t\t\tfile2")
vt.Diagnostics()
vt.Autofixes()
vt.Fixed(
"#CONF_FILES+= file1 \\",
"# file2")
vt.Run()
}

View file

@ -37,6 +37,11 @@ func (ck *PatchChecker) Check() {
if CheckLineRcsid(ck.lines[0], ``, "") {
ck.exp.Advance()
}
if ck.exp.EOF() {
ck.lines[0].Errorf("Patch files must not be empty.")
return
}
ck.previousLineEmpty = ck.exp.ExpectEmptyLine(G.opts.WarnSpace)
patchedFiles := 0

View file

@ -408,3 +408,30 @@ func (s *Suite) Test_ChecklinesPatch__context_lines_with_tab_instead_of_space(c
t.CheckOutputEmpty()
}
// Must not panic.
func (s *Suite) Test_ChecklinesPatch__autofix_empty_patch(c *check.C) {
t := s.Init(c)
t.SetupCommandLine("-Wall", "--autofix")
lines := t.NewLines("patch-aa",
RcsId)
ChecklinesPatch(lines)
t.CheckOutputEmpty()
}
// Must not panic.
func (s *Suite) Test_ChecklinesPatch__autofix_long_empty_patch(c *check.C) {
t := s.Init(c)
t.SetupCommandLine("-Wall", "--autofix")
lines := t.NewLines("patch-aa",
RcsId,
"")
ChecklinesPatch(lines)
t.CheckOutputEmpty()
}

View file

@ -308,37 +308,45 @@ func ChecklinesMessage(lines []Line) {
defer trace.Call1(lines[0].Filename)()
}
explainMessage := func() {
Explain(
"A MESSAGE file should consist of a header line, having 75 \"=\"",
"characters, followed by a line containing only the RCS Id, then an",
"empty line, your text and finally the footer line, which is the",
"same as the header line.")
}
explanation := []string{
"A MESSAGE file should consist of a header line, having 75 \"=\"",
"characters, followed by a line containing only the RCS Id, then an",
"empty line, your text and finally the footer line, which is the",
"same as the header line."}
if len(lines) < 3 {
lastLine := lines[len(lines)-1]
lastLine.Warnf("File too short.")
explainMessage()
Explain(explanation...)
return
}
hline := strings.Repeat("=", 75)
if line := lines[0]; line.Text != hline {
line.Warnf("Expected a line of exactly 75 \"=\" characters.")
explainMessage()
fix := line.Autofix()
fix.Warnf("Expected a line of exactly 75 \"=\" characters.")
fix.Explain(explanation...)
fix.InsertBefore(hline)
fix.Apply()
CheckLineRcsid(lines[0], ``, "")
} else if 1 < len(lines) {
CheckLineRcsid(lines[1], ``, "")
}
CheckLineRcsid(lines[1], ``, "")
for _, line := range lines {
CheckLineLength(line, 80)
CheckLineTrailingWhitespace(line)
CheckLineValidCharacters(line, `[\t -~]`)
}
if lastLine := lines[len(lines)-1]; lastLine.Text != hline {
lastLine.Warnf("Expected a line of exactly 75 \"=\" characters.")
explainMessage()
fix := lastLine.Autofix()
fix.Warnf("Expected a line of exactly 75 \"=\" characters.")
fix.Explain(explanation...)
fix.InsertAfter(hline)
fix.Apply()
}
ChecklinesTrailingEmptyLines(lines)
SaveAutofixChanges(lines)
}
func CheckfileMk(fname string) {

View file

@ -186,10 +186,40 @@ func (s *Suite) Test_ChecklinesMessage__malformed(c *check.C) {
t.CheckOutputLines(
"WARN: MESSAGE:1: Expected a line of exactly 75 \"=\" characters.",
"ERROR: MESSAGE:2: Expected \"$"+"NetBSD$\".",
"ERROR: MESSAGE:1: Expected \"$"+"NetBSD$\".",
"WARN: MESSAGE:5: Expected a line of exactly 75 \"=\" characters.")
}
func (s *Suite) Test_ChecklinesMessage__autofix(c *check.C) {
t := s.Init(c)
t.SetupCommandLine("-Wall", "--autofix")
lines := t.SetupFileLines("MESSAGE",
"1",
"2",
"3",
"4",
"5")
ChecklinesMessage(lines)
t.CheckOutputLines(
"AUTOFIX: ~/MESSAGE:1: Inserting a line \"===================================="+
"=======================================\" before this line.",
"AUTOFIX: ~/MESSAGE:1: Inserting a line \"$NetBSD: pkglint_test.go,v 1.14 2018/01/28 23:21:16 rillig Exp $\" before this line.",
"AUTOFIX: ~/MESSAGE:5: Inserting a line \"===================================="+
"=======================================\" after this line.")
t.CheckFileLines("MESSAGE",
"===========================================================================",
"$NetBSD: pkglint_test.go,v 1.14 2018/01/28 23:21:16 rillig Exp $",
"1",
"2",
"3",
"4",
"5",
"===========================================================================")
}
func (s *Suite) Test_GlobalData_Latest(c *check.C) {
t := s.Init(c)

View file

@ -101,7 +101,7 @@ func (ck *PlistChecker) collectFilesAndDirs(plines []*PlistLine) {
first == '$',
'A' <= first && first <= 'Z',
'0' <= first && first <= '9':
if ck.allFiles[text] == nil {
if prev := ck.allFiles[text]; prev == nil || pline.conditional < prev.conditional {
ck.allFiles[text] = pline
}
for dir := path.Dir(text); dir != "."; dir = path.Dir(dir) {
@ -143,6 +143,7 @@ func (ck *PlistChecker) checkpath(pline *PlistLine) {
dirname := strings.TrimSuffix(sdirname, "/")
ck.checkSorted(pline)
ck.checkDuplicate(pline)
if contains(basename, "${IMAKE_MANNEWSUFFIX}") {
pline.warnImakeMannewsuffix()
@ -211,17 +212,29 @@ func (ck *PlistChecker) checkSorted(pline *PlistLine) {
"The files in the PLIST should be sorted alphabetically.",
"To fix this, run \"pkglint -F PLIST\".")
}
if prev := ck.allFiles[text]; prev != nil && prev != pline {
fix := pline.line.Autofix()
fix.Errorf("Duplicate filename %q, already appeared in %s.", text, prev.line.ReferenceFrom(pline.line))
fix.Delete()
fix.Apply()
}
}
ck.lastFname = text
}
}
func (ck *PlistChecker) checkDuplicate(pline *PlistLine) {
text := pline.text
if !hasAlnumPrefix(text) || containsVarRef(text) {
return
}
prev := ck.allFiles[text]
if prev == pline || prev.conditional != "" {
return
}
fix := pline.line.Autofix()
fix.Errorf("Duplicate filename %q, already appeared in %s.",
text, prev.line.ReferenceFrom(pline.line))
fix.Delete()
fix.Apply()
}
func (ck *PlistChecker) checkpathBin(pline *PlistLine, dirname, basename string) {
if contains(dirname, "/") {
pline.line.Warnf("The bin/ directory should not have subdirectories.")
@ -279,7 +292,8 @@ func (ck *PlistChecker) checkpathLib(pline *PlistLine, dirname, basename string)
if contains(basename, ".a") || contains(basename, ".so") {
if m, noext := match1(pline.text, `^(.*)(?:\.a|\.so[0-9.]*)$`); m {
if laLine := ck.allFiles[noext+".la"]; laLine != nil {
pline.line.Warnf("Redundant library found. The libtool library is in %s.", laLine.line.ReferenceFrom(pline.line))
pline.line.Warnf("Redundant library found. The libtool library is in %s.",
laLine.line.ReferenceFrom(pline.line))
}
}
}
@ -320,7 +334,7 @@ func (ck *PlistChecker) checkpathMan(pline *PlistLine) {
"configured by the pkgsrc user. Compression and decompression takes",
"place automatically, no matter if the .gz extension is mentioned in",
"the PLIST or not.")
fix.ReplaceRegex(`\.gz\n`, "\n")
fix.ReplaceRegex(`\.gz\n`, "\n", 1)
fix.Apply()
}
}
@ -493,17 +507,6 @@ func NewPlistLineSorter(plines []*PlistLine) *plistLineSorter {
return &plistLineSorter{header, middle, footer, unsortable, false, false}
}
func (s *plistLineSorter) Len() int {
return len(s.middle)
}
func (s *plistLineSorter) Less(i, j int) bool {
return s.middle[i].text < s.middle[j].text
}
func (s *plistLineSorter) Swap(i, j int) {
s.changed = true
s.middle[i], s.middle[j] = s.middle[j], s.middle[i]
}
func (s *plistLineSorter) Sort() {
if line := s.unsortable; line != nil {
if G.opts.PrintAutofix || G.opts.Autofix {
@ -516,7 +519,17 @@ func (s *plistLineSorter) Sort() {
return
}
firstLine := s.middle[0].line
sort.Stable(s)
sort.SliceStable(s.middle, func(i, j int) bool {
mi := s.middle[i]
mj := s.middle[j]
less := mi.text < mj.text || (mi.text == mj.text &&
mi.conditional < mj.conditional)
if (i < j) != less {
s.changed = true
}
return less
})
if !s.changed {
return

View file

@ -292,3 +292,47 @@ func (s *Suite) Test_PlistChecker__autofix(c *check.C) {
"@pkgdir etc/logrotate.d",
"@pkgdir etc/sasl2")
}
// When the same entry appears both with and without a conditional,
// the one with the conditional can be removed.
// When the same entry appears with several different conditionals,
// all of them must stay.
func (s *Suite) Test_PlistChecker__remove_same_entries(c *check.C) {
t := s.Init(c)
t.SetupCommandLine("-Wall")
lines := t.SetupFileLines("PLIST",
PlistRcsId,
"${PLIST.option1}bin/true",
"bin/true",
"${PLIST.option1}bin/true",
"bin/true",
"${PLIST.option3}bin/false",
"${PLIST.option2}bin/false",
"bin/true")
ChecklinesPlist(lines)
t.CheckOutputLines(
"ERROR: ~/PLIST:2: Duplicate filename \"bin/true\", already appeared in line 3.",
"ERROR: ~/PLIST:4: Duplicate filename \"bin/true\", already appeared in line 3.",
"ERROR: ~/PLIST:5: Duplicate filename \"bin/true\", already appeared in line 3.",
"WARN: ~/PLIST:6: \"bin/false\" should be sorted before \"bin/true\".",
"ERROR: ~/PLIST:8: Duplicate filename \"bin/true\", already appeared in line 3.")
t.SetupCommandLine("-Wall", "--autofix")
ChecklinesPlist(lines)
t.CheckOutputLines(
"AUTOFIX: ~/PLIST:2: Deleting this line.",
"AUTOFIX: ~/PLIST:4: Deleting this line.",
"AUTOFIX: ~/PLIST:5: Deleting this line.",
"AUTOFIX: ~/PLIST:8: Deleting this line.",
"AUTOFIX: ~/PLIST:2: Sorting the whole file.")
t.CheckFileLines("PLIST",
PlistRcsId,
"${PLIST.option2}bin/false",
"${PLIST.option3}bin/false",
"bin/true")
}