pkgsrc-Changes archive
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]
CVS commit: pkgsrc/pkgtools/pkglint
Module Name: pkgsrc
Committed By: rillig
Date: Mon Dec 16 17:06:05 UTC 2019
Modified Files:
pkgsrc/pkgtools/pkglint: Makefile
pkgsrc/pkgtools/pkglint/files: autofix.go autofix_test.go check_test.go
logging.go mklinechecker.go mklinechecker_test.go mktypes.go
mktypes_test.go package.go substcontext.go substcontext_test.go
util.go util_test.go varalignblock.go varalignblock_test.go
vartypecheck.go vartypecheck_test.go
Log Message:
pkgtools/pkglint: update to 19.3.19
Changes since 19.3.18:
Small improvements to edge cases in SUBST blocks.
Small improvements to edge cases in variable assignment and alignment
of the continuation backslashes.
The --source option shows the changes from autofix, even when the
--show-autofix option is not given. This is a welcome side-effect of
making the autofix logging simpler and more predictable.
To generate a diff of this commit:
cvs rdiff -u -r1.619 -r1.620 pkgsrc/pkgtools/pkglint/Makefile
cvs rdiff -u -r1.34 -r1.35 pkgsrc/pkgtools/pkglint/files/autofix.go \
pkgsrc/pkgtools/pkglint/files/substcontext.go
cvs rdiff -u -r1.35 -r1.36 pkgsrc/pkgtools/pkglint/files/autofix_test.go \
pkgsrc/pkgtools/pkglint/files/logging.go
cvs rdiff -u -r1.60 -r1.61 pkgsrc/pkgtools/pkglint/files/check_test.go
cvs rdiff -u -r1.58 -r1.59 pkgsrc/pkgtools/pkglint/files/mklinechecker.go
cvs rdiff -u -r1.53 -r1.54 \
pkgsrc/pkgtools/pkglint/files/mklinechecker_test.go
cvs rdiff -u -r1.23 -r1.24 pkgsrc/pkgtools/pkglint/files/mktypes.go
cvs rdiff -u -r1.21 -r1.22 pkgsrc/pkgtools/pkglint/files/mktypes_test.go
cvs rdiff -u -r1.74 -r1.75 pkgsrc/pkgtools/pkglint/files/package.go \
pkgsrc/pkgtools/pkglint/files/vartypecheck.go
cvs rdiff -u -r1.33 -r1.34 pkgsrc/pkgtools/pkglint/files/substcontext_test.go
cvs rdiff -u -r1.66 -r1.67 pkgsrc/pkgtools/pkglint/files/util.go
cvs rdiff -u -r1.43 -r1.44 pkgsrc/pkgtools/pkglint/files/util_test.go
cvs rdiff -u -r1.12 -r1.13 pkgsrc/pkgtools/pkglint/files/varalignblock.go
cvs rdiff -u -r1.8 -r1.9 pkgsrc/pkgtools/pkglint/files/varalignblock_test.go
cvs rdiff -u -r1.67 -r1.68 pkgsrc/pkgtools/pkglint/files/vartypecheck_test.go
Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.
Modified files:
Index: pkgsrc/pkgtools/pkglint/Makefile
diff -u pkgsrc/pkgtools/pkglint/Makefile:1.619 pkgsrc/pkgtools/pkglint/Makefile:1.620
--- pkgsrc/pkgtools/pkglint/Makefile:1.619 Sat Dec 14 18:04:15 2019
+++ pkgsrc/pkgtools/pkglint/Makefile Mon Dec 16 17:06:05 2019
@@ -1,6 +1,6 @@
-# $NetBSD: Makefile,v 1.619 2019/12/14 18:04:15 rillig Exp $
+# $NetBSD: Makefile,v 1.620 2019/12/16 17:06:05 rillig Exp $
-PKGNAME= pkglint-19.3.18
+PKGNAME= pkglint-19.3.19
CATEGORIES= pkgtools
DISTNAME= tools
MASTER_SITES= ${MASTER_SITE_GITHUB:=golang/}
Index: pkgsrc/pkgtools/pkglint/files/autofix.go
diff -u pkgsrc/pkgtools/pkglint/files/autofix.go:1.34 pkgsrc/pkgtools/pkglint/files/autofix.go:1.35
--- pkgsrc/pkgtools/pkglint/files/autofix.go:1.34 Sun Dec 8 00:06:38 2019
+++ pkgsrc/pkgtools/pkglint/files/autofix.go Mon Dec 16 17:06:05 2019
@@ -21,8 +21,7 @@ type Autofix struct {
line *Line
linesBefore []string // Newly inserted lines, including \n
linesAfter []string // Newly inserted lines, including \n
- // Whether an actual fix has been applied (or, without --show-autofix,
- // whether a fix is applicable)
+ // Whether an actual fix has been applied to the text of the raw lines
modified bool
autofixShortTerm
@@ -30,11 +29,18 @@ type Autofix struct {
// autofixShortTerm is the part of the Autofix that is reset after each call to Apply.
type autofixShortTerm struct {
- actions []autofixAction // Human-readable description of the actual autofix actions
- level *LogLevel //
- diagFormat string // Is logged only if it couldn't be fixed automatically
- diagArgs []interface{} //
- explanation []string // Is printed together with the diagnostic
+ // Human-readable description of the actual autofix actions.
+ // There can be more than one action in cases where a follow-up
+ // fix is necessary.
+ actions []autofixAction
+
+ // The diagnostic to be logged.
+ // It is subject to the --only command line option.
+ // In --autofix mode it is suppressed if there were no actual actions.
+ level *LogLevel
+ diagFormat string
+ diagArgs []interface{}
+ explanation []string
}
type autofixAction struct {
@@ -49,11 +55,11 @@ type autofixAction struct {
// to log a diagnostic by other means.
const SilentAutofixFormat = "SilentAutofixFormat"
-// AutofixFormat is a special value that is used for logging
+// autofixFormat is a special value that is used for logging
// diagnostics like "Replacing \"old\" with \"new\".".
//
// Since these are not really diagnostics, duplicates are not suppressed.
-const AutofixFormat = "AutofixFormat"
+const autofixFormat = "AutofixFormat"
func NewAutofix(line *Line) *Autofix {
return &Autofix{line: line}
@@ -77,20 +83,22 @@ func (fix *Autofix) Notef(format string,
// Explain remembers the explanation for logging it later when Apply is called.
func (fix *Autofix) Explain(explanation ...string) {
// Since a silent fix doesn't have a diagnostic, its explanation would
- // not provide any clue as to what diagnostic it belongs. That would
- // be confusing, therefore this case is not allowed.
+ // not provide any clue as to what diagnostic it belongs.
+ // That would be confusing, therefore this case is not allowed.
assert(fix.diagFormat != SilentAutofixFormat)
fix.explanation = explanation
}
// Replace replaces "from" with "to", a single time.
+// If the text is not found exactly once, nothing is replaced at all.
func (fix *Autofix) Replace(from string, to string) {
fix.ReplaceAfter("", from, to)
}
// ReplaceAfter replaces the text "prefix+from" with "prefix+to", a single time.
// In the diagnostic, only the replacement of "from" with "to" is mentioned.
+// If the text is not found exactly once, nothing is replaced at all.
func (fix *Autofix) ReplaceAfter(prefix, from string, to string) {
fix.assertRealLine()
if fix.skip() {
@@ -121,6 +129,9 @@ func (fix *Autofix) ReplaceAfter(prefix,
// TODO: Do this properly by parsing the whole line again,
// and ideally everything that depends on the parsed line.
// This probably requires a generic notification mechanism.
+ //
+ // FIXME: Only actually update fix.line.Text if the replacement
+ // has been done exactly once; see ReplaceAt.
fix.line.Text = strings.Replace(fix.line.Text, prefixFrom, prefixTo, 1)
}
fix.Describef(rawLine.Lineno, "Replacing %q with %q.", from, to)
@@ -130,38 +141,38 @@ func (fix *Autofix) ReplaceAfter(prefix,
}
// ReplaceAt replaces the text "from" with "to", a single time.
-// But only if the text at the given position is indeed "from".
-func (fix *Autofix) ReplaceAt(rawIndex int, textIndex int, from string, to string) (modified bool, replaced string) {
+// If the text at the given position does not match, ReplaceAt panics.
+func (fix *Autofix) ReplaceAt(rawIndex int, textIndex int, from string, to string) {
assert(from != to)
fix.assertRealLine()
+ // XXX: This should only affect the diagnostics, but not the modifications
+ // to the text of the affected line, since that text will be used in
+ // further checks.
if fix.skip() {
- return false, ""
+ return
}
rawLine := fix.line.raw[rawIndex]
- if textIndex >= len(rawLine.textnl) || !hasPrefix(rawLine.textnl[textIndex:], from) {
- return false, ""
- }
+ assert(textIndex < len(rawLine.textnl))
+ assert(hasPrefix(rawLine.textnl[textIndex:], from))
- replaced = rawLine.textnl[:textIndex] + to + rawLine.textnl[textIndex+len(from):]
+ replaced := rawLine.textnl[:textIndex] + to + rawLine.textnl[textIndex+len(from):]
- if G.Logger.IsAutofix() {
- rawLine.textnl = replaced
+ rawLine.textnl = replaced
- // Fix the parsed text as well.
- // This is only approximate and won't work in some edge cases
- // that involve escaped comments or replacements across line breaks.
- //
- // TODO: Do this properly by parsing the whole line again,
- // and ideally everything that depends on the parsed line.
- // This probably requires a generic notification mechanism.
- if strings.Count(fix.line.Text, from) == 1 {
- fix.line.Text = strings.Replace(fix.line.Text, from, to, 1)
- }
+ // Fix the parsed text as well.
+ // This is only approximate and won't work in some edge cases
+ // that involve escaped comments or replacements across line breaks.
+ //
+ // TODO: Do this properly by parsing the whole line again,
+ // and ideally everything that depends on the parsed line.
+ // This probably requires a generic notification mechanism.
+ if strings.Count(fix.line.Text, from) == 1 {
+ fix.line.Text = strings.Replace(fix.line.Text, from, to, 1)
}
+
fix.Describef(rawLine.Lineno, "Replacing %q with %q.", from, to)
- return true, replaced
}
// ReplaceRegex replaces the first howOften or all occurrences (if negative)
@@ -206,6 +217,9 @@ func (fix *Autofix) ReplaceRegex(from re
// TODO: Do this properly by parsing the whole line again,
// and ideally everything that depends on the parsed line.
// This probably requires a generic notification mechanism.
+ //
+ // FIXME: Only actually update fix.line.Text if the replacement
+ // has been done exactly once.
done = 0
fix.line.Text = replaceAllFunc(
fix.line.Text,
@@ -270,19 +284,18 @@ func (fix *Autofix) Delete() {
// The fixer function must check whether it can actually fix something,
// and if so, call Describef to describe the actual fix.
//
-// If showAutofix and autofix are both false, the fix must only be
-// described by calling Describef. No observable modification must be done,
-// not even in memory.
-//
-// If showAutofix is true but autofix is false, the fix should be done in
-// memory as far as possible. For example, changing the text of Line.raw
-// is appropriate, but changing files in the file system is not.
+// If autofix is false, the the fix should be applied, as far as only
+// in-memory data structures are effected, and these are not written
+// back to disk. No externally observable modification must be done.
+// For example, changing the text of Line.raw is appropriate,
+// but changing files in the file system is not.
//
// Only if autofix is true, fixes other than modifying the current Line
// should be done persistently, such as changes to the file system.
//
-// In any case, changes to the current Line will be written back to disk
-// by SaveAutofixChanges, after fixing all the lines in the file at once.
+// If pkglint is run in --autofix mode, all changes to the lines of a
+// file will be collected in memory and are written back to disk by
+// SaveAutofixChanges, once at the end.
func (fix *Autofix) Custom(fixer func(showAutofix, autofix bool)) {
// Contrary to the fixes that modify the line text, this one
// can be run even on dummy lines (like those standing for a
@@ -295,21 +308,29 @@ func (fix *Autofix) Custom(fixer func(sh
fixer(G.Logger.Opts.ShowAutofix, G.Logger.Opts.Autofix)
}
-// Describef is used while Autofix.Custom is called to remember a description
-// of the actual fix for logging it later when Apply is called.
+// Describef can be called from within an Autofix.Custom call to remember a
+// description of the actual fix for logging it later when Apply is called.
// Describef may be called multiple times before calling Apply.
func (fix *Autofix) Describef(lineno int, format string, args ...interface{}) {
fix.actions = append(fix.actions, autofixAction{sprintf(format, args...), lineno})
}
-// Apply does the actual work.
-// Depending on the pkglint mode, it either:
+// Apply does the actual work that has been prepared by previous calls to
+// Errorf, Warnf, Notef, Describef, Replace, Delete and so on.
//
-// * logs the associated message (default) but does not record the fixes in the line
-//
-// * logs what would be fixed (--show-autofix) and records the fixes in the line
-//
-// * records the fixes in the line (--autofix), ready for SaveAutofixChanges
+// In default mode, the diagnostic is logged even when nothing has actually
+// been fixed. This frees the calling code from distinguishing the cases where
+// a fix can or cannot be applied automatically.
+//
+// In --show-autofix mode, only those diagnostics are logged that actually fix
+// something. This is done to hide possibly distracting, unrelated diagnostics.
+//
+// In --autofix mode, only the actual changes are logged, but not the
+// corresponding diagnostics. To get both, specify --show-autofix as well.
+//
+// Apply does the modifications only in memory. To actually save them to disk,
+// SaveAutofixChanges needs to be called. For example, this is done by
+// MkLines.Check.
func (fix *Autofix) Apply() {
line := fix.line
@@ -356,7 +377,7 @@ func (fix *Autofix) Apply() {
if action.lineno != 0 {
lineno = strconv.Itoa(action.lineno)
}
- G.Logger.Logf(AutofixLogLevel, line.Filename, lineno, AutofixFormat, action.description)
+ G.Logger.Logf(AutofixLogLevel, line.Filename, lineno, autofixFormat, action.description)
}
G.Logger.showSource(line)
}
@@ -416,18 +437,21 @@ func (fix *Autofix) affectedLinenos() st
func (fix *Autofix) skip() bool {
assert(fix.diagFormat != "") // The diagnostic must be given before the action.
- // This check is necessary for the --only command line option.
return !G.Logger.shallBeLogged(fix.diagFormat)
}
func (fix *Autofix) assertRealLine() {
- assert(fix.line.firstLine >= 1) // Cannot autofix this line since it is not a real line.
+ // Some Line objects do not correspond to real lines of a file.
+ // These cannot be fixed since they are neither part of Lines nor of MkLines.
+ assert(fix.line.firstLine >= 1)
}
// 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.
+//
+// This only happens in --autofix mode.
func SaveAutofixChanges(lines *Lines) (autofixed bool) {
if trace.Tracing {
defer trace.Call0()()
@@ -450,6 +474,14 @@ func SaveAutofixChanges(lines *Lines) (a
if G.Testing {
abs := G.Abs(lines.Filename)
absTmp := G.Abs(NewCurrPathSlash(os.TempDir()))
+
+ // This assertion prevents the pkglint tests from overwriting files
+ // on disk. This can easily happen if a test creates Lines or MkLines
+ // using a relative path.
+ //
+ // By default, these paths are relative to the current working
+ // directory. To let them refer to the temporary directory used by
+ // the tests, call t.Chdir(".").
assertf(abs.HasPrefixPath(absTmp), "%q must be inside %q", abs, absTmp)
}
Index: pkgsrc/pkgtools/pkglint/files/substcontext.go
diff -u pkgsrc/pkgtools/pkglint/files/substcontext.go:1.34 pkgsrc/pkgtools/pkglint/files/substcontext.go:1.35
--- pkgsrc/pkgtools/pkglint/files/substcontext.go:1.34 Sat Dec 14 18:04:15 2019
+++ pkgsrc/pkgtools/pkglint/files/substcontext.go Mon Dec 16 17:06:05 2019
@@ -66,27 +66,14 @@ func (ctx *SubstContext) varassign(mklin
}
}
- if hasPrefix(mkline.Varname(), "SUBST_") && mkline.Varparam() != ctx.activeId() {
+ if mkline.Varparam() != ctx.activeId() {
if !ctx.varassignDifferentClass(mkline) {
return
}
}
block := ctx.block()
- switch varcanon {
- case "SUBST_STAGE.*":
- block.varassignStage(mkline)
- case "SUBST_MESSAGE.*":
- block.varassignMessages(mkline)
- case "SUBST_FILES.*":
- block.varassignFiles(mkline)
- case "SUBST_SED.*":
- block.varassignSed(mkline)
- case "SUBST_VARS.*":
- block.varassignVars(mkline)
- case "SUBST_FILTER_CMD.*":
- block.varassignFilterCmd(mkline)
- }
+ block.varassign(mkline)
}
func (ctx *SubstContext) varassignClasses(mkline *MkLine) {
@@ -138,7 +125,7 @@ func (ctx *SubstContext) varassignOutsid
func (ctx *SubstContext) varassignDifferentClass(mkline *MkLine) (ok bool) {
varname := mkline.Varname()
unknown := ctx.lookup(mkline.Varparam()) == nil
- if unknown && ctx.isActive() && !ctx.block().isComplete() {
+ if unknown && !ctx.block().isComplete() {
mkline.Warnf("Variable %q does not match SUBST class %q.",
varname, ctx.activeId())
if !ctx.block().seenEmpty {
@@ -319,8 +306,22 @@ func (s *substScope) emptyLine() {
// finish brings all blocks that are defined in the current scope
// to an end.
func (s *substScope) finish(diag Diagnoser) {
+ foreignOk := map[string]bool{}
+ for _, def := range s.defs {
+ for allowed := range def.foreignAllowed {
+ foreignOk[allowed] = true
+ }
+ }
+
for _, block := range s.defs {
block.finish(diag)
+
+ for _, mkline := range block.foreign {
+ if !foreignOk[mkline.Varname()] {
+ mkline.Warnf("Foreign variable %q in SUBST block.",
+ mkline.Varname())
+ }
+ }
}
}
@@ -395,6 +396,23 @@ func newSubstBlock(id string) *substBloc
return &substBlock{id: id, conds: []*substCond{newSubstCond()}}
}
+func (b *substBlock) varassign(mkline *MkLine) {
+ switch mkline.Varcanon() {
+ case "SUBST_STAGE.*":
+ b.varassignStage(mkline)
+ case "SUBST_MESSAGE.*":
+ b.varassignMessages(mkline)
+ case "SUBST_FILES.*":
+ b.varassignFiles(mkline)
+ case "SUBST_SED.*":
+ b.varassignSed(mkline)
+ case "SUBST_VARS.*":
+ b.varassignVars(mkline)
+ default:
+ b.varassignFilterCmd(mkline)
+ }
+}
+
func (b *substBlock) varassignStage(mkline *MkLine) {
if b.isConditional() {
mkline.Warnf("%s should not be defined conditionally.", mkline.Varname())
@@ -661,16 +679,6 @@ func (b *substBlock) finish(diag Diagnos
"SUBST_VARS.%[1]s or SUBST_FILTER_CMD.%[1]s missing.",
b.id)
}
-
- b.checkForeignVariables()
-}
-
-func (b *substBlock) checkForeignVariables() {
- for _, mkline := range b.foreign {
- if _, ok := b.foreignAllowed[mkline.Varname()]; !ok {
- mkline.Warnf("Foreign variable %q in SUBST block.", mkline.Varname())
- }
- }
}
func (b *substBlock) hasStarted() bool {
Index: pkgsrc/pkgtools/pkglint/files/autofix_test.go
diff -u pkgsrc/pkgtools/pkglint/files/autofix_test.go:1.35 pkgsrc/pkgtools/pkglint/files/autofix_test.go:1.36
--- pkgsrc/pkgtools/pkglint/files/autofix_test.go:1.35 Sun Dec 8 22:03:38 2019
+++ pkgsrc/pkgtools/pkglint/files/autofix_test.go Mon Dec 16 17:06:05 2019
@@ -555,21 +555,43 @@ func (s *Suite) Test_Autofix_ReplaceAfte
func (s *Suite) Test_Autofix_ReplaceAt(c *check.C) {
t := s.Init(c)
+ mainPart := func(texts []string, rawIndex int, column int, from, to string) {
+ mklines := t.NewMkLines("filename.mk", texts...)
+ assert(len(mklines.mklines) == 1)
+ mkline := mklines.mklines[0]
+
+ fix := mkline.Autofix()
+ fix.Notef("Should be appended instead of assigned.")
+ fix.ReplaceAt(rawIndex, column, from, to)
+ fix.Apply()
+ }
+
lines := func(lines ...string) []string { return lines }
test := func(texts []string, rawIndex int, column int, from, to string, diagnostics ...string) {
+ doTest := func(bool) {
+ mainPart(texts, rawIndex, column, from, to)
+ }
- mainPart := func(autofix bool) {
- mklines := t.NewMkLines("filename.mk", texts...)
- assert(len(mklines.mklines) == 1)
- mkline := mklines.mklines[0]
-
- fix := mkline.Autofix()
- fix.Notef("Should be appended instead of assigned.")
- fix.ReplaceAt(rawIndex, column, from, to)
- fix.Apply()
+ t.ExpectDiagnosticsAutofix(doTest, diagnostics...)
+ }
+
+ testAssert := func(texts []string, rawIndex int, column int, from, to string) {
+ doTest := func(bool) {
+ t.ExpectAssert(
+ func() { mainPart(texts, rawIndex, column, from, to) })
}
- t.ExpectDiagnosticsAutofix(mainPart, diagnostics...)
+ t.ExpectDiagnosticsAutofix(doTest, nil...)
+ }
+
+ testPanicMatches := func(texts []string, rawIndex int, column int, from, to, panicMessage string) {
+ doTest := func(bool) {
+ t.ExpectPanicMatches(
+ func() { mainPart(texts, rawIndex, column, from, to) },
+ panicMessage)
+ }
+
+ t.ExpectDiagnosticsAutofix(doTest, nil...)
}
test(
@@ -582,35 +604,38 @@ func (s *Suite) Test_Autofix_ReplaceAt(c
"AUTOFIX: filename.mk:1: Replacing \"=\" with \"+=\".")
// If the text at the precisely given position does not match,
- // the note is still printed, but nothing is replaced automatically.
- test(
+ // it is a programming mistake, therefore pkglint panics.
+ testAssert(
lines(
"VAR=value1 \\",
"\tvalue2"),
- 0, 3, "?", "+=",
-
- "NOTE: filename.mk:1--2: Should be appended instead of assigned.")
+ 0, 3, "?", "+=")
// Getting the line number wrong is a strange programming error, and
// there does not need to be any code checking for this in the main code.
- t.ExpectPanicMatches(
- func() { test(lines("VAR=value"), 10, 3, "from", "to", nil...) },
+ testPanicMatches(
+ lines(
+ "VAR=value"),
+ 10, 3, "from", "to",
`runtime error: index out of range.*`)
// It is a programming error to replace a string with itself, since that
// would produce confusing diagnostics.
- t.ExpectAssert(
- func() { test(lines("VAR=value"), 0, 4, "value", "value", nil...) })
+ testAssert(
+ lines(
+ "VAR=value"),
+ 0, 4, "value", "value")
// Getting the column number wrong may happen when a previous replacement
- // has made the string shorter than before, therefore no panic in this case.
- test(
+ // has made the string shorter than before.
+ // This is a programming mistake, therefore panic.
+ // All fixes that work on the raw lines are supposed to work exactly and
+ // know what they are doing.
+ testAssert(
lines(
"VAR=value1 \\",
"\tvalue2"),
- 0, 20, "?", "+=",
-
- "NOTE: filename.mk:1--2: Should be appended instead of assigned.")
+ 0, 20, "?", "+=")
}
func (s *Suite) Test_Autofix_ReplaceRegex__show_autofix(c *check.C) {
Index: pkgsrc/pkgtools/pkglint/files/logging.go
diff -u pkgsrc/pkgtools/pkglint/files/logging.go:1.35 pkgsrc/pkgtools/pkglint/files/logging.go:1.36
--- pkgsrc/pkgtools/pkglint/files/logging.go:1.35 Mon Dec 9 20:38:16 2019
+++ pkgsrc/pkgtools/pkglint/files/logging.go Mon Dec 16 17:06:05 2019
@@ -268,7 +268,7 @@ func (l *Logger) Logf(level *LogLevel, f
}
}
- if G.Testing && format != AutofixFormat {
+ if G.Testing && format != autofixFormat {
if textproc.Alpha.Contains(format[0]) {
assertf(
textproc.Upper.Contains(format[0]),
@@ -284,7 +284,7 @@ func (l *Logger) Logf(level *LogLevel, f
if !filename.IsEmpty() {
filename = filename.CleanPath()
}
- if G.Opts.Profiling && format != AutofixFormat && level != Fatal {
+ if G.Opts.Profiling && format != autofixFormat && level != Fatal {
l.histo.Add(format, 1)
}
Index: pkgsrc/pkgtools/pkglint/files/check_test.go
diff -u pkgsrc/pkgtools/pkglint/files/check_test.go:1.60 pkgsrc/pkgtools/pkglint/files/check_test.go:1.61
--- pkgsrc/pkgtools/pkglint/files/check_test.go:1.60 Fri Dec 13 01:39:23 2019
+++ pkgsrc/pkgtools/pkglint/files/check_test.go Mon Dec 16 17:06:05 2019
@@ -61,8 +61,6 @@ func (s *Suite) SetUpTest(c *check.C) {
G.Testing = true
trace.Out = &t.stdout
- // XXX: Maybe the tests can run a bit faster when they don't
- // create a temporary directory each.
G.Pkgsrc = NewPkgsrc(t.File("."))
t.c = c
@@ -1327,7 +1325,8 @@ func (t *Tester) CheckDotColumns(lines .
width := tabWidth(prefix)
num, err := strconv.Atoi(line[m[2]:m[3]])
assertNil(err, "")
- t.CheckEqualsf(num, width, "lines[%d]", index)
+ t.CheckEqualsf(num, width,
+ "The dots in lines[%d] are wrong.", index)
}
}
}
Index: pkgsrc/pkgtools/pkglint/files/mklinechecker.go
diff -u pkgsrc/pkgtools/pkglint/files/mklinechecker.go:1.58 pkgsrc/pkgtools/pkglint/files/mklinechecker.go:1.59
--- pkgsrc/pkgtools/pkglint/files/mklinechecker.go:1.58 Sun Dec 8 22:03:38 2019
+++ pkgsrc/pkgtools/pkglint/files/mklinechecker.go Mon Dec 16 17:06:05 2019
@@ -64,14 +64,14 @@ func (ck MkLineChecker) checkTextVarUse(
tokens, _ := NewMkLexer(text, nil).MkTokens()
for i, token := range tokens {
- // TODO: flatten
- if token.Varuse != nil {
- spaceLeft := i-1 < 0 || matches(tokens[i-1].Text, `[\t ]$`)
- spaceRight := i+1 >= len(tokens) || matches(tokens[i+1].Text, `^[\t ]`)
- isWordPart := !(spaceLeft && spaceRight)
- vuc := VarUseContext{vartype, time, VucQuotPlain, isWordPart}
- NewMkVarUseChecker(token.Varuse, ck.MkLines, ck.MkLine).Check(&vuc)
+ if token.Varuse == nil {
+ continue
}
+ spaceLeft := i-1 < 0 || matches(tokens[i-1].Text, `[\t ]$`)
+ spaceRight := i+1 >= len(tokens) || matches(tokens[i+1].Text, `^[\t ]`)
+ isWordPart := !(spaceLeft && spaceRight)
+ vuc := VarUseContext{vartype, time, VucQuotPlain, isWordPart}
+ NewMkVarUseChecker(token.Varuse, ck.MkLines, ck.MkLine).Check(&vuc)
}
}
Index: pkgsrc/pkgtools/pkglint/files/mklinechecker_test.go
diff -u pkgsrc/pkgtools/pkglint/files/mklinechecker_test.go:1.53 pkgsrc/pkgtools/pkglint/files/mklinechecker_test.go:1.54
--- pkgsrc/pkgtools/pkglint/files/mklinechecker_test.go:1.53 Sun Dec 8 22:03:38 2019
+++ pkgsrc/pkgtools/pkglint/files/mklinechecker_test.go Mon Dec 16 17:06:05 2019
@@ -310,23 +310,24 @@ func (s *Suite) Test_MkLineChecker_check
func (s *Suite) Test_MkLineChecker_checkShellCommand__indentation(c *check.C) {
t := s.Init(c)
- mklines := t.SetUpFileMkLines("filename.mk",
- MkCvsID,
- "",
- "do-install:",
- "\t\techo 'unnecessarily indented'",
- "\t\tfor var in 1 2 3; do \\",
- "\t\t\techo \"$$var\"; \\",
- "\t echo \"spaces\"; \\",
- "\t\tdone",
- "",
- "\t\t\t\t\t# comment, not a shell command")
+ doTest := func(bool) {
+ mklines := t.SetUpFileMkLines("filename.mk",
+ MkCvsID,
+ "",
+ "do-install:",
+ "\t\techo 'unnecessarily indented'",
+ "\t\tfor var in 1 2 3; do \\",
+ "\t\t\techo \"$$var\"; \\",
+ "\t echo \"spaces\"; \\",
+ "\t\tdone",
+ "",
+ "\t\t\t\t\t# comment, not a shell command")
- mklines.Check()
- t.SetUpCommandLine("-Wall", "--autofix")
- mklines.Check()
+ mklines.Check()
+ }
- t.CheckOutputLines(
+ t.ExpectDiagnosticsAutofix(
+ doTest,
"NOTE: ~/filename.mk:4: Shell programs should be indented with a single tab.",
"WARN: ~/filename.mk:4: Unknown shell command \"echo\".",
"NOTE: ~/filename.mk:5--8: Shell programs should be indented with a single tab.",
Index: pkgsrc/pkgtools/pkglint/files/mktypes.go
diff -u pkgsrc/pkgtools/pkglint/files/mktypes.go:1.23 pkgsrc/pkgtools/pkglint/files/mktypes.go:1.24
--- pkgsrc/pkgtools/pkglint/files/mktypes.go:1.23 Fri Dec 13 01:39:23 2019
+++ pkgsrc/pkgtools/pkglint/files/mktypes.go Mon Dec 16 17:06:05 2019
@@ -61,8 +61,9 @@ func (m MkVarUseModifier) MatchSubst() (
// MkVarUseModifier{"S,name,file,g"}.Subst("distname-1.0") => "distfile-1.0"
func (m MkVarUseModifier) Subst(str string) (string, bool) {
// XXX: The call to MatchSubst is usually redundant because MatchSubst
- // is typically called directly before calling Subst.
- ok, regex, from, to, options := m.MatchSubst()
+ // is typically called directly before calling Subst.
+ // This comes from a time when there was no boolean return value.
+ ok, isRegex, from, to, options := m.MatchSubst()
if !ok {
return "", false
}
@@ -77,14 +78,14 @@ func (m MkVarUseModifier) Subst(str stri
from = from[:len(from)-1]
}
- if regex && matches(from, `^[\w-]+$`) && matches(to, `^[^&$\\]*$`) {
+ if isRegex && matches(from, `^[\w-]+$`) && matches(to, `^[^&$\\]*$`) {
// The "from" pattern is so simple that it doesn't matter whether
// the modifier is :S or :C, therefore treat it like the simpler :S.
- regex = false
+ isRegex = false
}
- if regex {
- // TODO: Maybe implement regular expression substitutions later.
+ if isRegex {
+ // XXX: Maybe implement regular expression substitutions later.
return "", false
}
Index: pkgsrc/pkgtools/pkglint/files/mktypes_test.go
diff -u pkgsrc/pkgtools/pkglint/files/mktypes_test.go:1.21 pkgsrc/pkgtools/pkglint/files/mktypes_test.go:1.22
--- pkgsrc/pkgtools/pkglint/files/mktypes_test.go:1.21 Fri Dec 13 01:39:23 2019
+++ pkgsrc/pkgtools/pkglint/files/mktypes_test.go Mon Dec 16 17:06:05 2019
@@ -87,72 +87,67 @@ func (s *Suite) Test_MkVarUseModifier_Ma
t.CheckEquals(options, "1")
}
-// As of 2019-03-24, pkglint doesn't know how to handle complicated
-// :C modifiers.
-func (s *Suite) Test_MkVarUseModifier_Subst__regexp(c *check.C) {
+func (s *Suite) Test_MkVarUseModifier_Subst(c *check.C) {
t := s.Init(c)
- mod := MkVarUseModifier{"C,.*,,"}
+ test := func(mod, str, result string, ok bool) {
+ m := MkVarUseModifier{mod}
- empty, ok := mod.Subst("anything")
+ actualResult, actualOk := m.Subst(str)
- t.CheckEquals(ok, false)
- t.CheckEquals(empty, "")
-}
-
-// When given a modifier that is not actually a :S or :C, Subst
-// doesn't do anything.
-func (s *Suite) Test_MkVarUseModifier_Subst__invalid_argument(c *check.C) {
- t := s.Init(c)
-
- mod := MkVarUseModifier{"Mpattern"}
-
- empty, ok := mod.Subst("anything")
-
- t.CheckEquals(ok, false)
- t.CheckEquals(empty, "")
-}
-
-func (s *Suite) Test_MkVarUseModifier_Subst__no_tracing(c *check.C) {
- t := s.Init(c)
-
- mod := MkVarUseModifier{"S,from,to,"}
- t.DisableTracing()
+ t.CheckDeepEquals(
+ []interface{}{actualResult, actualOk},
+ []interface{}{result, ok})
+ }
- result, ok := mod.Subst("from a to b")
+ test("???", "anything", "", false)
- t.CheckEquals(ok, true)
- t.CheckEquals(result, "to a to b")
-}
+ test("S,from,to,", "from", "to", true)
-// Since the replacement text is not a simple string, the :C modifier
-// cannot be treated like the :S modifier. The variable could contain
-// one of the special characters that would need to be escaped in the
-// replacement text.
-func (s *Suite) Test_MkVarUseModifier_Subst__C_with_complex_replacement(c *check.C) {
- t := s.Init(c)
+ test("C,from,to,", "from", "to", true)
- mod := MkVarUseModifier{"C,from,${VAR},"}
+ test("C,syntax error", "anything", "", false)
- result, ok := mod.Subst("from a to b")
+ // The substitution modifier does not match, therefore
+ // the value is returned unmodified, but successful.
+ test("C,no_match,replacement,", "value", "value", true)
- t.CheckEquals(ok, false)
- t.CheckEquals(result, "")
-}
+ // As of December 2019, pkglint doesn't know how to handle
+ // complicated :C modifiers.
+ test("C,.*,,", "anything", "", false)
-func (s *Suite) Test_MkVarUseModifier_Subst__S_dollar_at(c *check.C) {
- t := s.Init(c)
+ // When given a modifier that is not actually a :S or :C, Subst
+ // doesn't do anything.
+ test("Mpattern", "anything", "", false)
- mod := MkVarUseModifier{"S/$@/replaced/"}
+ test("S,from,to,", "from a to b", "to a to b", true)
- result, ok := mod.Subst("The target")
+ // Since the replacement text is not a simple string, the :C modifier
+ // cannot be treated like the :S modifier. The variable could contain
+ // one of the special characters that would need to be escaped in the
+ // replacement text.
+ test("C,from,${VAR},", "from a to b", "", false)
// As of December 2019, nothing is substituted. If pkglint should ever
- // handle variables in the modifier, this test would been to provide a
+ // handle variables in the modifier, this test would need to provide a
// context in which to resolve the variables. If that happens, the
// .TARGET variable needs to be set to "target".
- t.CheckEquals(ok, true)
- t.CheckEquals(result, "The target")
+ test("S/$@/replaced/", "The target", "The target", true)
+ test("S,${PREFIX},/prefix,", "${PREFIX}/dir", "", false)
+
+ // Just for code coverage.
+ t.DisableTracing()
+ test("S,long,long long,g", "A long story", "A long long story", true)
+ t.EnableTracing()
+
+ // And now again with full tracing, to investigate cases where
+ // pkglint produces results that are not easily understandable.
+ t.EnableTracingToLog()
+ test("S,long,long long,g", "A long story", "A long long story", true)
+ t.EnableTracing()
+ t.CheckOutputLines(
+ "TRACE: Subst: \"A long story\" " +
+ "\"S,long,long long,g\" => \"A long long story\"")
}
func (s *Suite) Test_MkVarUseModifier_EvalSubst(c *check.C) {
Index: pkgsrc/pkgtools/pkglint/files/package.go
diff -u pkgsrc/pkgtools/pkglint/files/package.go:1.74 pkgsrc/pkgtools/pkglint/files/package.go:1.75
--- pkgsrc/pkgtools/pkglint/files/package.go:1.74 Sun Dec 8 22:03:38 2019
+++ pkgsrc/pkgtools/pkglint/files/package.go Mon Dec 16 17:06:05 2019
@@ -403,7 +403,7 @@ func (pkg *Package) resolveIncludedFile(
if mkline.Basename != "buildlink3.mk" {
if includedFile.HasSuffixPath("buildlink3.mk") {
curr := mkline.File(includedFile)
- if G.Pkg != nil && !curr.ContainsText("$") && !curr.IsFile() {
+ if G.Pkg != nil && !curr.IsFile() {
curr = G.Pkg.File(PackagePath(includedFile))
}
packagePath := pkg.Rel(curr)
Index: pkgsrc/pkgtools/pkglint/files/vartypecheck.go
diff -u pkgsrc/pkgtools/pkglint/files/vartypecheck.go:1.74 pkgsrc/pkgtools/pkglint/files/vartypecheck.go:1.75
--- pkgsrc/pkgtools/pkglint/files/vartypecheck.go:1.74 Sat Dec 14 18:04:16 2019
+++ pkgsrc/pkgtools/pkglint/files/vartypecheck.go Mon Dec 16 17:06:05 2019
@@ -689,14 +689,7 @@ func (cv *VartypeCheck) IdentifierDirect
return
}
- switch {
- case matches(cv.ValueNoVar, `^[+\-.\w]+$`):
- // Fine.
-
- case cv.Value != "" && cv.ValueNoVar == "":
- // Don't warn here.
-
- default:
+ if !matches(cv.ValueNoVar, `^[+\-.\w]+$`) {
cv.Warnf("Invalid identifier %q.", cv.Value)
}
}
Index: pkgsrc/pkgtools/pkglint/files/substcontext_test.go
diff -u pkgsrc/pkgtools/pkglint/files/substcontext_test.go:1.33 pkgsrc/pkgtools/pkglint/files/substcontext_test.go:1.34
--- pkgsrc/pkgtools/pkglint/files/substcontext_test.go:1.33 Sat Dec 14 18:04:15 2019
+++ pkgsrc/pkgtools/pkglint/files/substcontext_test.go Mon Dec 16 17:06:05 2019
@@ -410,6 +410,54 @@ func (s *Suite) Test_SubstContext_varass
"the SUBST class should be declared using \"SUBST_CLASSES+= libs\".")
}
+func (s *Suite) Test_SubstContext_varassignDifferentClass__same_paragraph(c *check.C) {
+ t := s.Init(c)
+
+ t.RunSubst(
+ "SUBST_CLASSES+= 1",
+ "SUBST_STAGE.1= pre-configure",
+ "SUBST_FILES.1= files",
+ "SUBST_VARS.x= VAR",
+ "SUBST_VARS.x= VAR")
+
+ // There is a switch of the SUBST class in the middle of the paragraph.
+ // This is often a typo, therefore pkglint still expects the SUBST class
+ // 1 to be continued in line 4.
+ //
+ // If there were an empty line before line 4, pkglint would have
+ // interpreted that as an intention to start a new block in the next
+ // paragraph.
+ t.CheckOutputLines(
+ "WARN: filename.mk:4: Variable \"SUBST_VARS.x\" "+
+ "does not match SUBST class \"1\".",
+ "WARN: filename.mk:5: Variable \"SUBST_VARS.x\" "+
+ "does not match SUBST class \"1\".",
+ "WARN: filename.mk:EOF: Incomplete SUBST block: "+
+ "SUBST_SED.1, SUBST_VARS.1 or SUBST_FILTER_CMD.1 missing.")
+}
+
+func (s *Suite) Test_SubstContext_varassignDifferentClass__next_paragraph(c *check.C) {
+ t := s.Init(c)
+
+ t.RunSubst(
+ "SUBST_CLASSES+= 1",
+ "SUBST_STAGE.1= pre-configure",
+ "SUBST_FILES.1= files",
+ "",
+ "SUBST_VARS.x= VAR",
+ "SUBST_VARS.x= VAR")
+
+ // There is a switch of the SUBST class at the end of the paragraph.
+ // Pkglint sees that as an intention to start a new SUBST block.
+ t.CheckOutputLines(
+ "WARN: filename.mk:5: Variable \"SUBST_VARS.x\" "+
+ "does not match SUBST class \"1\".",
+ "WARN: filename.mk:5: Incomplete SUBST block: "+
+ "SUBST_SED.1, SUBST_VARS.1 or SUBST_FILTER_CMD.1 missing.",
+ "WARN: filename.mk:5: Before defining SUBST_VARS.x, "+
+ "the SUBST class should be declared using \"SUBST_CLASSES+= x\".")
+}
+
// Unbalanced conditionals must not lead to a panic.
func (s *Suite) Test_SubstContext_directive__before_SUBST_CLASSES(c *check.C) {
t := s.Init(c)
@@ -721,6 +769,56 @@ func (s *Suite) Test_SubstContext_leave_
"WARN: filename.mk:EOF: Incomplete SUBST block: SUBST_FILES.os missing.")
}
+func (s *Suite) Test_SubstContext_activeId__SUBST_CLASSES_in_separate_paragraph(c *check.C) {
+ t := s.Init(c)
+
+ ctx := NewSubstContext()
+
+ checkNoActiveId := func() {
+ t.CheckEquals(ctx.isActive(), false)
+ }
+ checkActiveId := func(id string) {
+ t.CheckEquals(ctx.activeId(), id)
+ }
+ lineno := 1
+ line := func(text string) {
+ ctx.Process(t.NewMkLine("filename.mk", lineno, text))
+ lineno++
+ }
+
+ line("SUBST_CLASSES+= 1 2 3 4")
+ checkNoActiveId()
+
+ line("")
+ checkNoActiveId()
+
+ line("SUBST_STAGE.1= post-configure")
+ checkActiveId("1")
+
+ line("SUBST_FILES.1= files")
+ line("SUBST_VARS.1= VAR1")
+ checkActiveId("1")
+
+ line("")
+ checkActiveId("1")
+
+ line("SUBST_STAGE.2= post-configure")
+ checkActiveId("2")
+
+ line("SUBST_FILES.2= files")
+ line("SUBST_VARS.2= VAR1")
+ line("")
+ line("SUBST_STAGE.3= post-configure")
+ line("SUBST_FILES.3= files")
+ line("SUBST_VARS.3= VAR1")
+
+ ctx.Finish(NewLineEOF("filename.mk"))
+
+ t.CheckOutputLines(
+ "NOTE: filename.mk:1: Please add only one class at a time to SUBST_CLASSES.",
+ "WARN: filename.mk:EOF: Missing SUBST block for \"4\".")
+}
+
// With every .if directive, a new scope is created, to properly
// keep track of the conditional level at which the SUBST classes
// are declared.
@@ -774,6 +872,85 @@ func (s *Suite) Test_substScope__conditi
ctx.Finish(NewLineEOF("filename.mk"))
}
+func (s *Suite) Test_substScope_define__assertion(c *check.C) {
+ t := s.Init(c)
+
+ scope := newSubstScope()
+ scope.define("id")
+
+ t.ExpectAssert(
+ func() { scope.define("id") })
+}
+
+// Variables mentioned in SUBST_VARS may appear in the same paragraph,
+// or alternatively anywhere else in the file.
+func (s *Suite) Test_substScope_finish__foreign_in_next_paragraph(c *check.C) {
+ t := s.Init(c)
+
+ t.RunSubst(
+ "SUBST_CLASSES+=\tos",
+ "SUBST_STAGE.os=\tpre-configure",
+ "SUBST_FILES.os=\tguess-os.h",
+ "SUBST_VARS.os=\tTODAY1",
+ "",
+ "TODAY1!=\tdate",
+ "TODAY2!=\tdate")
+
+ t.CheckOutputEmpty()
+}
+
+func (s *Suite) Test_substScope_finish__foreign_mixed_separate(c *check.C) {
+ t := s.Init(c)
+
+ t.RunSubst(
+ "SUBST_CLASSES+= 1",
+ "SUBST_STAGE.1= post-configure",
+ "SUBST_FILES.1= files",
+ "",
+ "SUBST_VARS.1= VAR",
+ "USE_TOOLS+= gmake")
+
+ // The USE_TOOLS is not in the SUBST block anymore since there is
+ // an empty line between SUBST_CLASSES and SUBST_VARS.
+ t.CheckOutputEmpty()
+}
+
+// Variables mentioned in SUBST_VARS are not considered "foreign"
+// in the block and may be mixed with the other SUBST variables.
+func (s *Suite) Test_substScope_finish__foreign_in_block(c *check.C) {
+ t := s.Init(c)
+
+ t.RunSubst(
+ "SUBST_CLASSES+=\tos",
+ "SUBST_STAGE.os=\tpre-configure",
+ "SUBST_FILES.os=\tguess-os.h",
+ "SUBST_VARS.os=\tTODAY1",
+ "TODAY1!=\tdate",
+ "TODAY2!=\tdate")
+
+ t.CheckOutputLines(
+ "WARN: filename.mk:6: Foreign variable \"TODAY2\" in SUBST block.")
+}
+
+func (s *Suite) Test_substScope_finish__foreign_two_blocks_one_paragraph(c *check.C) {
+ t := s.Init(c)
+
+ t.RunSubst(
+ "SUBST_CLASSES+= 1 2",
+ "SUBST_STAGE.1= pre-configure",
+ "VAR2= value2",
+ "SUBST_FILES.1= files",
+ "SUBST_VARS.1= VAR1",
+ "SUBST_STAGE.2= pre-configure",
+ "SUBST_FILES.2= files",
+ "VAR1= value1",
+ "SUBST_VARS.2= VAR2")
+
+ t.CheckOutputLines(
+ "NOTE: filename.mk:1: " +
+ "Please add only one class at a time to SUBST_CLASSES.")
+}
+
func (s *Suite) Test_substScope_prepareSubstClasses(c *check.C) {
t := s.Init(c)
@@ -809,6 +986,113 @@ func (s *Suite) Test_substScope_prepareS
"SUBST_SED.1, SUBST_VARS.1 or SUBST_FILTER_CMD.1 missing.")
}
+func (s *Suite) Test_substBlock__enter_leave_and_finish(c *check.C) {
+ t := s.Init(c)
+
+ mkline := t.NewMkLine("filename.mk", 123, "")
+ b := newSubstBlock("id")
+
+ t.CheckEquals(len(b.conds), 1)
+
+ b.enter()
+
+ t.CheckEquals(len(b.conds), 2)
+
+ b.leave()
+
+ t.CheckEquals(len(b.conds), 1)
+
+ b.finish(mkline)
+
+ t.CheckOutputLines(
+ "WARN: filename.mk:123: Missing SUBST block for \"id\".")
+}
+
+// In a conditional without an else branch, none of the variable
+// definitions from the then branch are seen in the outer scope.
+func (s *Suite) Test_substBlock__enter_and_leave_without_else(c *check.C) {
+ t := s.Init(c)
+
+ b := newSubstBlock("id")
+
+ b.enter() // .if
+ b.addSeen(ssSed) // SUBST_SED
+ b.leave() // .endif
+
+ t.CheckEquals(b.allSeen(), ssNone)
+ t.CheckEquals(b.done, false)
+}
+
+func (s *Suite) Test_substBlock__enter_and_leave_with_else(c *check.C) {
+ t := s.Init(c)
+
+ b := newSubstBlock("id")
+
+ b.enter() // .if
+ b.addSeen(ssVars) // SUBST_VARS
+ b.addSeen(ssTransform) // SUBST_VARS
+ b.nextBranch(true) // .else
+ b.addSeen(ssSed) // SUBST_SED
+ b.addSeen(ssTransform) // SUBST_SED
+ b.leave() // .endif
+
+ t.CheckEquals(b.hasSeen(ssTransform), true)
+ t.CheckEquals(b.done, false)
+}
+
+func (s *Suite) Test_substBlock__enter_and_leave_with_elif(c *check.C) {
+ t := s.Init(c)
+
+ b := newSubstBlock("id")
+
+ b.enter() // .if
+ b.addSeen(ssFiles) // SUBST_FILES
+ b.addSeen(ssVars) // SUBST_VARS
+ b.nextBranch(false) // .elif
+ b.addSeen(ssFiles) // SUBST_FILES
+ b.addSeen(ssSed) // SUBST_SED
+ b.nextBranch(true) // .else
+ b.addSeen(ssFiles) // SUBST_FILES
+ b.addSeen(ssSed) // SUBST_SED
+ b.leave() // .endif
+
+ t.CheckEquals(b.hasSeen(ssFiles), true)
+ t.CheckEquals(b.done, false)
+}
+
+func (s *Suite) Test_newSubstBlock(c *check.C) {
+ t := s.Init(c)
+
+ b := newSubstBlock("id")
+
+ t.CheckEquals(b.id, "id")
+ t.CheckEquals(len(b.conds), 1)
+ t.CheckEquals(b.done, false)
+ t.CheckEquals(b.allSeen(), ssNone)
+}
+
+func (s *Suite) Test_newSubstBlock__assertion(c *check.C) {
+ t := s.Init(c)
+
+ t.ExpectAssert(
+ func() { newSubstBlock("") })
+}
+
+func (s *Suite) Test_substBlock_varassign__typo_in_subst_variable(c *check.C) {
+ t := s.Init(c)
+
+ t.RunSubst(
+ "SUBST_CLASSES+=\tos",
+ "SUBST_STAGE.os=\tdo-patch",
+ "SUBST_FILES.os=\tguess-os.h",
+ "SUBST_DED.os=\t-e s,@OPSYS@,Darwin,")
+
+ t.CheckOutputLines(
+ "WARN: filename.mk:EOF: Incomplete SUBST block: "+
+ "SUBST_SED.os, SUBST_VARS.os or SUBST_FILTER_CMD.os missing.",
+ "WARN: filename.mk:4: Foreign variable \"SUBST_DED.os\" in SUBST block.")
+}
+
func (s *Suite) Test_substBlock_varassignStage__do_patch(c *check.C) {
t := s.Init(c)
@@ -1501,52 +1785,32 @@ func (s *Suite) Test_substBlock_finish__
"SUBST_SED.one, SUBST_VARS.one or SUBST_FILTER_CMD.one missing.")
}
-// Variables mentioned in SUBST_VARS may appear in the same paragraph,
-// or alternatively anywhere else in the file.
-func (s *Suite) Test_substBlock_checkForeignVariables__in_next_paragraph(c *check.C) {
+func (s *Suite) Test_substBlock_finish__assertion(c *check.C) {
t := s.Init(c)
- t.RunSubst(
- "SUBST_CLASSES+=\tos",
- "SUBST_STAGE.os=\tpre-configure",
- "SUBST_FILES.os=\tguess-os.h",
- "SUBST_VARS.os=\tTODAY1",
- "",
- "TODAY1!=\tdate",
- "TODAY2!=\tdate")
+ b := newSubstBlock("id")
+ b.enter()
- t.CheckOutputEmpty()
+ t.ExpectAssert(
+ func() { b.finish(nil) })
}
-func (s *Suite) Test_substBlock_checkForeignVariables__mixed_separate(c *check.C) {
+func (s *Suite) Test_substSeen_set__assertion(c *check.C) {
t := s.Init(c)
- t.RunSubst(
- "SUBST_CLASSES+= 1",
- "SUBST_STAGE.1= post-configure",
- "SUBST_FILES.1= files",
- "",
- "SUBST_VARS.1= VAR",
- "USE_TOOLS+= gmake")
-
- // The USE_TOOLS is not in the SUBST block anymore since there is
- // an empty line between SUBST_CLASSES and SUBST_VARS.
- t.CheckOutputEmpty()
+ t.ExpectAssert(
+ func() {
+ seen := ssAll
+ seen.set(ssAll)
+ })
}
-// Variables mentioned in SUBST_VARS are not considered "foreign"
-// in the block and may be mixed with the other SUBST variables.
-func (s *Suite) Test_substBlock_checkForeignVariables__in_block(c *check.C) {
+func (s *Suite) Test_substSeen_has__assertion(c *check.C) {
t := s.Init(c)
- t.RunSubst(
- "SUBST_CLASSES+=\tos",
- "SUBST_STAGE.os=\tpre-configure",
- "SUBST_FILES.os=\tguess-os.h",
- "SUBST_VARS.os=\tTODAY1",
- "TODAY1!=\tdate",
- "TODAY2!=\tdate")
-
- t.CheckOutputLines(
- "WARN: filename.mk:6: Foreign variable \"TODAY2\" in SUBST block.")
+ t.ExpectAssert(
+ func() {
+ seen := ssAll
+ seen.has(ssAll)
+ })
}
Index: pkgsrc/pkgtools/pkglint/files/util.go
diff -u pkgsrc/pkgtools/pkglint/files/util.go:1.66 pkgsrc/pkgtools/pkglint/files/util.go:1.67
--- pkgsrc/pkgtools/pkglint/files/util.go:1.66 Fri Dec 13 01:39:23 2019
+++ pkgsrc/pkgtools/pkglint/files/util.go Mon Dec 16 17:06:05 2019
@@ -574,6 +574,11 @@ func (o *Once) check(key uint64) bool {
//
// TODO: Merge this code with Var, which defines essentially the
// same features.
+//
+// See also substScope, which already analyzes the possible variable values
+// based on the conditional code paths.
+//
+// See also RedundantScope.
type Scope struct {
firstDef map[string]*MkLine // TODO: Can this be removed?
lastDef map[string]*MkLine
@@ -1381,3 +1386,23 @@ func (b *LazyStringBuilder) String() str
}
return b.expected[:b.len]
}
+
+type interval struct {
+ min int
+ max int
+}
+
+func newInterval() *interval {
+ return &interval{int(^uint(0) >> 1), ^int(^uint(0) >> 1)}
+}
+
+func (i *interval) add(x int) {
+ if x < i.min {
+ i.min = x
+ }
+ if x > i.max {
+ i.max = x
+ }
+}
+
+func (i *interval) isEmpty() bool { return i.min > i.max }
Index: pkgsrc/pkgtools/pkglint/files/util_test.go
diff -u pkgsrc/pkgtools/pkglint/files/util_test.go:1.43 pkgsrc/pkgtools/pkglint/files/util_test.go:1.44
--- pkgsrc/pkgtools/pkglint/files/util_test.go:1.43 Fri Dec 13 01:39:23 2019
+++ pkgsrc/pkgtools/pkglint/files/util_test.go Mon Dec 16 17:06:05 2019
@@ -1150,3 +1150,26 @@ func keys(m interface{}) []string {
sort.Strings(keys)
return keys
}
+
+func (s *Suite) Test_interval(c *check.C) {
+ t := s.Init(c)
+
+ i := newInterval()
+
+ t.CheckEquals(i.min > i.max, true)
+
+ i.add(3)
+
+ t.CheckEquals(i.min, 3)
+ t.CheckEquals(i.max, 3)
+
+ i.add(7)
+
+ t.CheckEquals(i.min, 3)
+ t.CheckEquals(i.max, 7)
+
+ i.add(-5)
+
+ t.CheckEquals(i.min, -5)
+ t.CheckEquals(i.max, 7)
+}
Index: pkgsrc/pkgtools/pkglint/files/varalignblock.go
diff -u pkgsrc/pkgtools/pkglint/files/varalignblock.go:1.12 pkgsrc/pkgtools/pkglint/files/varalignblock.go:1.13
--- pkgsrc/pkgtools/pkglint/files/varalignblock.go:1.12 Fri Dec 13 01:39:23 2019
+++ pkgsrc/pkgtools/pkglint/files/varalignblock.go Mon Dec 16 17:06:05 2019
@@ -183,28 +183,18 @@ func (va *VaralignBlock) processVarassig
return
}
- follow := false
var infos []*varalignLine
for i, raw := range mkline.raw {
parts := NewVaralignSplitter().split(strings.TrimSuffix(raw.textnl, "\n"), i == 0)
- info := varalignLine{mkline, i, follow, false, parts}
-
- if i == 0 && info.isEmptyContinuation() {
- follow = true
- info.multiEmpty = true
- }
-
+ info := varalignLine{mkline, i, false, false, parts}
infos = append(infos, &info)
}
va.mkinfos = append(va.mkinfos, &varalignMkLine{infos})
}
func (va *VaralignBlock) Finish() {
- mkinfos := va.mkinfos
- skip := va.skip
- *va = VaralignBlock{} // overwrites infos and skip
-
- if len(mkinfos) == 0 || skip {
+ if len(va.mkinfos) == 0 || va.skip {
+ *va = VaralignBlock{}
return
}
@@ -212,32 +202,39 @@ func (va *VaralignBlock) Finish() {
defer trace.Call()()
}
- newWidth := va.optimalWidth(mkinfos)
- va.adjustLong(newWidth, mkinfos)
+ newWidth := va.optimalWidth()
+ va.adjustLong(newWidth)
+
+ for _, mkinfo := range va.mkinfos {
+ mkinfo.realign(newWidth)
+ }
- for _, mkinfo := range mkinfos {
+ *va = VaralignBlock{}
+}
- // When the indentation of the initial line of a multiline is
- // changed, all its follow-up lines are shifted by the same
- // amount and in the same direction. Typical examples are
- // SUBST_SED, shell programs and AWK programs like in
- // GENERATE_PLIST.
- indentDiffSet := false
-
- // The amount by which the follow-up lines are shifted.
- // Positive values mean shifting to the right, negative values
- // mean shifting to the left.
- indentDiff := 0
+func (l *varalignMkLine) realign(newWidth int) {
+ // When the indentation of the initial line of a multiline is
+ // changed, all its follow-up lines are shifted by the same
+ // amount and in the same direction. Typical examples are
+ // SUBST_SED, shell programs and AWK programs like in
+ // GENERATE_PLIST.
+ indentDiffSet := false
- _, rightMargin := mkinfo.rightMargin()
- for _, info := range mkinfo.infos {
+ // The amount by which the follow-up lines are shifted.
+ // Positive values mean shifting to the right, negative values
+ // mean shifting to the left.
+ indentDiff := 0
- // TODO: move below va.realign
- info.alignContinuation(newWidth, rightMargin)
+ _, rightMargin := l.rightMargin()
+ isMultiEmpty := l.isMultiEmpty()
+ for _, info := range l.infos {
- if newWidth > 0 || info.rawIndex > 0 {
- va.realign(info, newWidth, &indentDiffSet, &indentDiff)
- }
+ if newWidth > 0 || info.rawIndex > 0 {
+ info.realignDetails(newWidth, &indentDiffSet, &indentDiff, isMultiEmpty)
+ }
+
+ if !info.fixedSpaceBeforeContinuation {
+ info.alignContinuation(newWidth, rightMargin)
}
}
}
@@ -248,23 +245,41 @@ func (va *VaralignBlock) Finish() {
// There may be a single line sticking out from the others (called outlier).
// This is to prevent a single SITES.* variable from forcing the rest of the
// paragraph to be indented too far to the right.
-func (*VaralignBlock) optimalWidth(mkinfos []*varalignMkLine) int {
+func (va *VaralignBlock) optimalWidth() int {
+
+ minVarnameOpWidth, outlier := va.varnameOpWidths()
+ minTotalWidth, maxTotalWidth := va.spaceWidths(outlier)
+ va.traceWidths(minTotalWidth, maxTotalWidth, minVarnameOpWidth, outlier)
+
+ if minTotalWidth > minVarnameOpWidth && minTotalWidth == maxTotalWidth && minTotalWidth%8 == 0 {
+ // The whole paragraph is already indented to the same width.
+ return minTotalWidth
+ }
+
+ if minVarnameOpWidth == 0 {
+ // Only continuation lines in this paragraph.
+ return 0
+ }
+
+ return minVarnameOpWidth&-8 + 8
+}
+func (va *VaralignBlock) varnameOpWidths() (int, int) {
var widths bag
- for _, mkinfo := range mkinfos {
- for _, info := range mkinfo.infos {
- if !info.multiEmpty && info.rawIndex == 0 {
- widths.Add(info.fixer, info.varnameOpWidth())
- }
+ for _, mkinfo := range va.mkinfos {
+ if !mkinfo.isMultiEmpty() {
+ info := mkinfo.infos[0]
+ widths.add(info.fixer, info.spaceBeforeValueColumn())
}
}
+ if widths.len() == 0 {
+ return 0, 0
+ }
+
widths.sortDesc()
longest := widths.opt(0)
- var longestLine *MkLine
- if len(widths.slice) > 0 {
- longestLine = widths.key(0).(*MkLine)
- }
+ longestLine := widths.key(0).(*MkLine)
secondLongest := widths.opt(1)
haveOutlier := secondLongest != 0 &&
@@ -274,22 +289,24 @@ func (*VaralignBlock) optimalWidth(mkinf
// Minimum required width of varnameOp, without the trailing whitespace.
minVarnameOpWidth := condInt(haveOutlier, secondLongest, longest)
outlier := condInt(haveOutlier, longest, 0)
+ return minVarnameOpWidth, outlier
+}
+func (va *VaralignBlock) spaceWidths(outlier int) (min, max int) {
// Widths of the current indentation (including whitespace)
- var spaceWidths bag
- for _, mkinfo := range mkinfos {
- for _, info := range mkinfo.infos {
- if info.multiEmpty || info.rawIndex > 0 || outlier > 0 && info.varnameOpWidth() == outlier {
- continue
- }
- spaceWidths.Add(nil, info.varnameOpSpaceWidth())
+ spaceWidths := newInterval()
+ for _, mkinfo := range va.mkinfos {
+ info := mkinfo.infos[0]
+ if mkinfo.isMultiEmpty() || outlier > 0 && info.spaceBeforeValueColumn() == outlier {
+ continue
}
+ spaceWidths.add(info.valueColumn())
}
- spaceWidths.sortDesc()
- minTotalWidth := spaceWidths.min()
- maxTotalWidth := spaceWidths.max()
+ return spaceWidths.min, spaceWidths.max
+}
+func (va *VaralignBlock) traceWidths(minTotalWidth int, maxTotalWidth int, minVarnameOpWidth int, outlier int) {
if trace.Tracing {
trace.Stepf("Indentation including whitespace is between %d and %d.",
minTotalWidth, maxTotalWidth)
@@ -298,27 +315,16 @@ func (*VaralignBlock) optimalWidth(mkinf
trace.Stepf("The outlier is at indentation %d.", outlier)
}
}
-
- if minTotalWidth > minVarnameOpWidth && minTotalWidth == maxTotalWidth && minTotalWidth%8 == 0 {
- // The whole paragraph is already indented to the same width.
- return minTotalWidth
- }
-
- if minVarnameOpWidth == 0 {
- // Only continuation lines in this paragraph.
- return 0
- }
-
- return minVarnameOpWidth&-8 + 8
}
// adjustLong allows any follow-up line to start either in column 8 or at
// least in column newWidth. But only if there is at least one continuation
// line that starts in column 8 and needs the full width up to column 72.
-func (va *VaralignBlock) adjustLong(newWidth int, mkinfos []*varalignMkLine) {
+func (va *VaralignBlock) adjustLong(newWidth int) {
anyLong := false
- for _, mkinfo := range mkinfos {
+ for _, mkinfo := range va.mkinfos {
infos := mkinfo.infos
+ isMultiEmpty := mkinfo.isMultiEmpty()
for i, info := range infos {
if info.rawIndex == 0 {
anyLong = false
@@ -326,27 +332,27 @@ func (va *VaralignBlock) adjustLong(newW
if follow.rawIndex == 0 {
break
}
- if !follow.multiEmpty && follow.spaceBeforeValue == "\t" && follow.varnameOpSpaceWidth() < newWidth && follow.widthAlignedAt(newWidth) > 72 {
+ if !isMultiEmpty && follow.spaceBeforeValue == "\t" && follow.valueColumn() < newWidth && follow.widthAlignedAt(newWidth) > 72 {
anyLong = true
break
}
}
}
- info.long = anyLong && info.varnameOpSpaceWidth() == 8
+ info.long = anyLong && info.valueColumn() == 8
}
}
}
-func (va *VaralignBlock) realign(info *varalignLine, newWidth int, indentDiffSet *bool, indentDiff *int) {
- if info.multiEmpty {
+func (info *varalignLine) realignDetails(newWidth int, indentDiffSet *bool, indentDiff *int, isMultiEmpty bool) {
+ if isMultiEmpty {
if info.rawIndex == 0 {
info.alignValueMultiEmptyInitial(newWidth)
} else {
- va.realignMultiEmptyFollow(info, newWidth, indentDiffSet, indentDiff)
+ info.realignMultiEmptyFollow(newWidth, indentDiffSet, indentDiff)
}
} else if info.rawIndex == 0 && info.isContinuation() {
- va.realignMultiInitial(info, newWidth, indentDiffSet, indentDiff)
+ info.realignMultiInitial(newWidth, indentDiffSet, indentDiff)
} else if info.rawIndex > 0 {
assert(*indentDiffSet)
info.alignValueMultiFollow(newWidth, *indentDiff)
@@ -355,7 +361,7 @@ func (va *VaralignBlock) realign(info *v
}
}
-func (va *VaralignBlock) realignMultiEmptyFollow(info *varalignLine, newWidth int, indentDiffSet *bool, indentDiff *int) {
+func (info *varalignLine) realignMultiEmptyFollow(newWidth int, indentDiffSet *bool, indentDiff *int) {
oldSpace := info.spaceBeforeValue
oldWidth := tabWidth(oldSpace)
@@ -370,9 +376,9 @@ func (va *VaralignBlock) realignMultiEmp
info.alignValueMultiEmptyFollow(imax(oldWidth+*indentDiff, 8))
}
-func (va *VaralignBlock) realignMultiInitial(info *varalignLine, newWidth int, indentDiffSet *bool, indentDiff *int) {
+func (info *varalignLine) realignMultiInitial(newWidth int, indentDiffSet *bool, indentDiff *int) {
*indentDiffSet = true
- *indentDiff = newWidth - info.varnameOpSpaceWidth()
+ *indentDiff = newWidth - info.valueColumn()
info.alignValueMultiInitial(newWidth)
}
@@ -476,6 +482,20 @@ type varalignMkLine struct {
infos []*varalignLine
}
+// Is true for multilines that don't have a value in their first
+// physical line.
+//
+// The follow-up lines of these lines may be indented with as few
+// as a single tab. Example:
+// VAR= \
+// value1 \
+// value2
+// In all other lines, the indentation must be at least the indentation
+// of the first value found.
+func (l *varalignMkLine) isMultiEmpty() bool {
+ return l.infos[0].isEmptyContinuation()
+}
+
// rightMargin calculates the column in which the continuation backslashes
// should be placed.
// In addition it returns whether the right margin is already in its
@@ -524,21 +544,11 @@ type varalignLine struct {
fixer Autofixer
rawIndex int
- // Is true for multilines that don't have a value in their first
- // physical line.
- //
- // The follow-up lines of these lines may be indented with as few
- // as a single tab. Example:
- // VAR= \
- // value1 \
- // value2
- // In all other lines, the indentation must be at least the indentation
- // of the first value found.
- multiEmpty bool
-
// Whether the line is so long that it may use a single tab as indentation.
long bool
+ fixedSpaceBeforeContinuation bool
+
varalignParts
}
@@ -590,7 +600,7 @@ func (info *varalignLine) alignValueMult
// Indent the outlier and any other lines that stick out
// with a space instead of a tab to keep the line short.
newSpace := " "
- if info.varnameOpSpaceWidth() <= newWidth {
+ if info.valueColumn() <= newWidth {
newSpace = alignmentAfter(leadingComment+varnameOp, newWidth)
}
@@ -603,7 +613,7 @@ func (info *varalignLine) alignValueMult
}
hasSpace := strings.IndexByte(oldSpace, ' ') != -1
- oldColumn := info.varnameOpSpaceWidth()
+ oldColumn := info.valueColumn()
column := tabWidthSlice(leadingComment, varnameOp, newSpace)
fix := info.fixer.Autofix()
@@ -616,6 +626,7 @@ func (info *varalignLine) alignValueMult
}
fix.ReplaceAt(info.rawIndex, info.spaceBeforeValueIndex(), oldSpace, newSpace)
fix.Apply()
+ info.spaceBeforeValue = newSpace
}
func (info *varalignLine) alignValueMultiInitial(column int) {
@@ -628,7 +639,7 @@ func (info *varalignLine) alignValueMult
return
}
- oldWidth := info.varnameOpSpaceWidth()
+ oldWidth := info.valueColumn()
width := tabWidthSlice(leadingComment, varnameOp, newSpace)
fix := info.fixer.Autofix()
@@ -641,6 +652,7 @@ func (info *varalignLine) alignValueMult
}
fix.ReplaceAt(info.rawIndex, info.spaceBeforeValueIndex(), oldSpace, newSpace)
fix.Apply()
+ info.spaceBeforeValue = newSpace
}
func (info *varalignLine) alignValueMultiEmptyFollow(column int) {
@@ -650,44 +662,37 @@ func (info *varalignLine) alignValueMult
return
}
- // Below a continuation marker, there may be a completely empty line.
- // This is confusing to the human readers, but technically allowed.
- if info.varalignParts.isEmpty() {
- return
- }
-
- fix := info.fixer.Autofix()
- fix.Notef("This continuation line should be indented with %q.", newSpace)
- fix.ReplaceAt(info.rawIndex, info.spaceBeforeValueIndex(), oldSpace, newSpace)
- fix.Apply()
-
- // TODO: Merge with alignValueMultiFollow
+ info.alignFollow(newSpace)
}
func (info *varalignLine) alignValueMultiFollow(column, indentDiff int) {
oldSpace := info.spaceBeforeValue
newWidth := imax(column, tabWidth(oldSpace)+indentDiff)
newSpace := indent(newWidth)
- if newSpace == oldSpace || info.long || info.isTooLongFor(newWidth) {
+ if newSpace == oldSpace {
+ return
+ }
+
+ info.alignFollow(newSpace)
+}
+
+func (info *varalignLine) alignFollow(newSpace string) {
+ // Below a continuation marker, there may be a completely empty line.
+ // This is confusing to the human readers, but technically allowed.
+ if info.varalignParts.isEmpty() {
return
}
+ continuationColumn := 0
+ if info.spaceBeforeContinuation() != " " {
+ continuationColumn = imin(72, info.continuationColumn())
+ }
+
fix := info.fixer.Autofix()
fix.Notef("This continuation line should be indented with %q.", newSpace)
- modified, replaced := fix.ReplaceAt(info.rawIndex, info.spaceBeforeValueIndex(), oldSpace, newSpace)
- assert(modified)
- if info.continuation != "" && info.continuationColumn() == 72 {
- orig := strings.TrimSuffix(replaced, "\n")
- base := rtrimHspace(strings.TrimSuffix(orig, "\\"))
- spaceIndex := len(base)
- oldSuffix := orig[spaceIndex:]
- newSuffix := " \\"
- if tabWidth(base) < 72 {
- newSuffix = alignmentAfter(base, 72) + "\\"
- }
- if newSuffix != oldSuffix {
- fix.ReplaceAt(info.rawIndex, spaceIndex, oldSuffix, newSuffix)
- }
+ info.replaceSpaceBeforeValue(fix, newSpace)
+ if info.isContinuation() {
+ info.replaceSpaceBeforeContinuationSilently(fix, continuationColumn)
}
fix.Apply()
}
@@ -698,11 +703,14 @@ func (info *varalignLine) alignContinuat
}
oldSpace := info.spaceBeforeContinuation()
- if oldSpace == " " || oldSpace == "\t" {
+ if oldSpace == " " {
return
}
column := info.continuationColumn()
+ if column <= 72 && oldSpace == "\t" {
+ return
+ }
if column == 72 || column == rightMarginColumn || column <= valueColumn {
return
}
@@ -716,16 +724,47 @@ func (info *varalignLine) alignContinuat
} else if info.uptoValueWidth() >= rightMarginColumn {
fix.Notef("The continuation backslash should be preceded by a single space or tab.")
} else {
- newSpace = alignmentAfter(info.uptoValue(), rightMarginColumn)
+ newSpace = alignmentToWidths(info.uptoValueWidth(), rightMarginColumn)
fix.Notef(
"The continuation backslash should be in column %d, not %d.",
rightMarginColumn+1, column+1)
}
index := info.continuationIndex() - len(oldSpace)
fix.ReplaceAt(info.rawIndex, index, oldSpace, newSpace)
+ info.setSpaceBeforeContinuation(newSpace)
fix.Apply()
}
+func (info *varalignLine) replaceSpaceBeforeValue(fix *Autofix, newSpace string) {
+ index := info.spaceBeforeValueIndex()
+ fix.ReplaceAt(info.rawIndex, index, info.spaceBeforeValue, newSpace)
+ info.spaceBeforeValue = newSpace
+}
+
+func (info *varalignLine) replaceSpaceBeforeContinuationSilently(fix *Autofix, column int) {
+ if info.value == "" {
+ return
+ }
+
+ oldSpace := info.spaceBeforeContinuation()
+ if oldSpace == " " {
+ return
+ }
+ newSpaceColumn := info.uptoValueWidth()
+ newSpace := alignmentToWidths(newSpaceColumn, column)
+ if newSpace == "" {
+ newSpace = " "
+ }
+ if oldSpace == newSpace {
+ return
+ }
+
+ index := info.spaceBeforeContinuationIndex()
+ fix.ReplaceAt(info.rawIndex, index, oldSpace+"\\", newSpace+"\\")
+ info.varalignParts.setSpaceBeforeContinuation(newSpace)
+ info.fixedSpaceBeforeContinuation = true
+}
+
func (*varalignLine) explainWrongColumn(fix *Autofix) {
fix.Explain(
"Normally, all variable values in a block should start at the same column.",
@@ -760,6 +799,58 @@ func (p *varalignParts) String() string
p.continuation
}
+func (p *varalignParts) leadingCommentIndex() int {
+ return 0
+}
+
+func (p *varalignParts) varnameOpIndex() int {
+ return p.leadingCommentIndex() + len(p.leadingComment)
+}
+
+// spaceBeforeValueIndex returns the string index at which the space before the value starts.
+// It's the same as the end of the assignment operator. Example:
+// #VAR= value
+// The index is 5.
+func (p *varalignParts) spaceBeforeValueIndex() int {
+ return p.varnameOpIndex() + len(p.varnameOp)
+}
+
+func (p *varalignParts) valueIndex() int {
+ return p.spaceBeforeValueIndex() + len(p.spaceBeforeValue)
+}
+
+func (p *varalignParts) spaceAfterValueIndex() int {
+ return p.valueIndex() + len(p.value)
+}
+
+func (p *varalignParts) continuationIndex() int {
+ return p.spaceAfterValueIndex() + len(p.spaceAfterValue)
+}
+
+func (p *varalignParts) leadingCommentColumn() int {
+ return 0
+}
+
+func (p *varalignParts) varnameOpColumn() int {
+ return tabWidthAppend(p.leadingCommentColumn(), p.leadingComment)
+}
+
+func (p *varalignParts) spaceBeforeValueColumn() int {
+ return tabWidthAppend(p.varnameOpColumn(), p.varnameOp)
+}
+
+func (p *varalignParts) valueColumn() int {
+ return tabWidthAppend(p.spaceBeforeValueColumn(), p.spaceBeforeValue)
+}
+
+func (p *varalignParts) spaceAfterValueColumn() int {
+ return tabWidthAppend(p.valueColumn(), p.value)
+}
+
+func (p *varalignParts) continuationColumn() int {
+ return tabWidthAppend(p.spaceAfterValueColumn(), p.spaceAfterValue)
+}
+
// continuation returns whether this line ends with a backslash.
func (p *varalignParts) isContinuation() bool {
return p.continuation != ""
@@ -773,22 +864,6 @@ func (p *varalignParts) isEmpty() bool {
return p.value == "" && !p.isContinuation()
}
-func (p *varalignParts) varnameOpWidth() int {
- return tabWidthSlice(p.leadingComment, p.varnameOp)
-}
-
-func (p *varalignParts) varnameOpSpaceWidth() int {
- return tabWidthSlice(p.leadingComment, p.varnameOp, p.spaceBeforeValue)
-}
-
-// spaceBeforeValueIndex returns the string index at which the space before the value starts.
-// It's the same as the end of the assignment operator. Example:
-// #VAR= value
-// The index is 5.
-func (p *varalignParts) spaceBeforeValueIndex() int {
- return len(p.leadingComment) + len(p.varnameOp)
-}
-
func (p *varalignParts) spaceBeforeContinuation() string {
if p.value == "" {
return p.spaceBeforeValue
@@ -796,28 +871,27 @@ func (p *varalignParts) spaceBeforeConti
return p.spaceAfterValue
}
-func (p *varalignParts) uptoValueWidth() int {
- return tabWidth(rtrimHspace(p.leadingComment +
- p.varnameOp + p.spaceBeforeValue +
- p.value))
+func (p *varalignParts) setSpaceBeforeContinuation(space string) {
+ if p.value == "" {
+ p.spaceBeforeValue = space
+ } else {
+ p.spaceAfterValue = space
+ }
}
-func (p *varalignParts) uptoValue() string {
- return rtrimHspace(p.leadingComment +
- p.varnameOp + p.spaceBeforeValue +
- p.value)
-}
+func (p *varalignParts) spaceBeforeContinuationIndex() int {
+ assert(p.isContinuation())
+ assert(p.value != "")
-func (p *varalignParts) continuationColumn() int {
- return tabWidthSlice(p.leadingComment,
- p.varnameOp, p.spaceBeforeValue,
- p.value, p.spaceAfterValue)
+ return p.spaceAfterValueIndex()
}
-func (p *varalignParts) continuationIndex() int {
- return len(p.leadingComment) +
- len(p.varnameOp) + len(p.spaceBeforeValue) +
- len(p.value) + len(p.spaceAfterValue)
+func (p *varalignParts) uptoValueWidth() int {
+ if p.value != "" {
+ return p.spaceAfterValueColumn()
+ } else {
+ return p.varnameOpColumn()
+ }
}
func (p *varalignParts) isCommentedOut() bool {
@@ -827,13 +901,13 @@ func (p *varalignParts) isCommentedOut()
// isCanonicalInitial returns whether the space between the assignment
// operator and the value has its canonical form, which is either
// at least one tab, or a single space, but only for lines that stick out.
-func (p *varalignParts) isCanonicalInitial(width int) bool {
+func (p *varalignParts) isCanonicalInitial(column int) bool {
space := p.spaceBeforeValue
if space == "" {
return false
}
- if space == " " && p.varnameOpSpaceWidth() > width {
+ if space == " " && p.valueColumn() > column {
return true
}
@@ -895,13 +969,15 @@ func (mi *bag) key(index int) interface{
return mi.slice[index].key
}
-func (mi *bag) Add(key interface{}, value int) {
+func (mi *bag) add(key interface{}, value int) {
mi.slice = append(mi.slice, struct {
key interface{}
value int
}{key, value})
}
-func (mi *bag) min() int { return mi.opt(0) }
+func (mi *bag) first() int { return mi.opt(0) }
+
+func (mi *bag) last() int { return mi.opt(len(mi.slice) - 1) }
-func (mi *bag) max() int { return mi.opt(len(mi.slice) - 1) }
+func (mi *bag) len() int { return len(mi.slice) }
Index: pkgsrc/pkgtools/pkglint/files/varalignblock_test.go
diff -u pkgsrc/pkgtools/pkglint/files/varalignblock_test.go:1.8 pkgsrc/pkgtools/pkglint/files/varalignblock_test.go:1.9
--- pkgsrc/pkgtools/pkglint/files/varalignblock_test.go:1.8 Fri Dec 13 01:39:23 2019
+++ pkgsrc/pkgtools/pkglint/files/varalignblock_test.go Mon Dec 16 17:06:05 2019
@@ -95,23 +95,28 @@ func (vt *VaralignTester) run(autofix bo
varalign.Process(mkline)
}
- mkinfos := varalign.mkinfos // since they are overwritten by Finish
- varalign.Finish()
+ var actualInternals []string
if len(vt.internals) > 0 {
var actual []string
- for _, mkinfo := range mkinfos {
+ for _, mkinfo := range varalign.mkinfos {
for _, info := range mkinfo.infos {
var minWidth, curWidth, continuation string
- minWidth = condStr(info.rawIndex == 0, sprintf("%02d", info.varnameOpWidth()), " ")
- curWidth = sprintf(" %02d", info.varnameOpSpaceWidth())
+ minWidth = condStr(info.rawIndex == 0, sprintf("%02d", info.spaceBeforeValueColumn()), " ")
+ curWidth = sprintf(" %02d", info.valueColumn())
if info.isContinuation() {
continuation = sprintf(" %02d", info.continuationColumn())
}
actual = append(actual, minWidth+curWidth+continuation)
}
}
- t.CheckDeepEquals(actual, vt.internals)
+ actualInternals = actual
+ }
+
+ varalign.Finish()
+
+ if len(vt.internals) > 0 {
+ t.CheckDeepEquals(actualInternals, vt.internals)
}
if autofix {
@@ -2024,13 +2029,10 @@ func (s *Suite) Test_VaralignBlock__lead
//
// The value in the first line starts in column 16, which means that all
// follow-up lines should also start in column 16 or further to the right.
-// Line 2 though is already quite long, and since its right margin is in
-// column 72, it may keep its lower-than-usual indentation of 8.
-// Line 3 is not that long, therefore the rule from line 2 doesn't apply
-// here, and it needs to be indented to column 16.
//
-// Since the above result would look inconsistent, all follow-up lines
-// after a long line may be indented in column 8 as well.
+// Line 2 though is already quite long, and even though its right margin
+// is in column 72, it needs to be indented at least as much as the value
+// of the first line.
func (s *Suite) Test_VaralignBlock__var_tab_value63_space_cont_tab8_value71_space_cont_tab8_value(c *check.C) {
vt := NewVaralignTester(s, c)
vt.Input(
@@ -2042,13 +2044,17 @@ func (s *Suite) Test_VaralignBlock__var_
" 08 72",
" 08")
vt.Diagnostics(
- nil...)
+ "NOTE: Makefile:2: This continuation line should be "+
+ "indented with \"\\t\\t\".",
+ "NOTE: Makefile:3: This continuation line should be "+
+ "indented with \"\\t\\t\".")
vt.Autofixes(
- nil...)
+ "AUTOFIX: Makefile:2: Replacing \"\\t\" with \"\\t\\t\".",
+ "AUTOFIX: Makefile:3: Replacing \"\\t\" with \"\\t\\t\".")
vt.Fixed(
"PROGFILES= 67890 234567890 234567890 234567890 234567890 2 \\",
- " 890 234567890 234567890 234567890 234567890 234567890 234567890 \\",
- " value")
+ " 890 234567890 234567890 234567890 234567890 234567890 234567890 \\",
+ " value")
vt.Run()
}
@@ -2271,8 +2277,8 @@ func (s *Suite) Test_VaralignBlock__mixe
func (s *Suite) Test_VaralignBlock__long_line_followed_by_short_line_with_small_indentation(c *check.C) {
vt := NewVaralignTester(s, c)
vt.Input(
- "VAR.567890123456+=\t----30 -------40 -------50 -------60 -------70 234567 \\",
- "\t\t--20 -------30")
+ "VAR...........16+=\t....30........40........50........60........70 234567 \\",
+ "\t\t..20........30")
vt.Internals(
"18 24 78",
" 16")
@@ -2281,8 +2287,8 @@ func (s *Suite) Test_VaralignBlock__long
vt.Autofixes(
"AUTOFIX: Makefile:2: Replacing \"\\t\\t\" with \"\\t\\t\\t\".")
vt.Fixed(
- "VAR.567890123456+= ----30 -------40 -------50 -------60 -------70 234567 \\",
- " --20 -------30")
+ "VAR...........16+= ....30........40........50........60........70 234567 \\",
+ " ..20........30")
vt.Run()
}
@@ -2314,14 +2320,13 @@ func (s *Suite) Test_VaralignBlock__shif
vt := NewVaralignTester(s, c)
vt.Input(
"INSTALLATION_DIRS+=\tvalue",
- "CONF_FILES=\t--20 -------30 -------40 -------50 -------60 -------70 \\",
- "\t\t--20")
+ "CONF_FILES=\t..20........30........40........50........60........70 \\",
+ "\t\t..20")
vt.Internals(
"19 24",
"11 16 71",
" 16")
vt.Diagnostics(
- // FIXME: No, it shouldn't, as that would make the continuation marker invisible on 80x25.
"NOTE: Makefile:2: This variable value should be aligned to column 25.",
"NOTE: Makefile:3: This continuation line should be indented with \"\\t\\t\\t\".")
vt.Autofixes(
@@ -2329,8 +2334,8 @@ func (s *Suite) Test_VaralignBlock__shif
"AUTOFIX: Makefile:3: Replacing \"\\t\\t\" with \"\\t\\t\\t\".")
vt.Fixed(
"INSTALLATION_DIRS+= value",
- "CONF_FILES= --20 -------30 -------40 -------50 -------60 -------70 \\",
- " --20")
+ "CONF_FILES= ..20........30........40........50........60........70 \\",
+ " ..20")
vt.Run()
}
@@ -2540,11 +2545,20 @@ func (s *Suite) Test_VaralignBlock__alig
false)
// The second line is indented with a single tab because otherwise
- // it would be longer than 72 characters. In this case it is ok to
- // use the smaller indentation.
+ // it would be longer than 72 characters. It could be argued that
+ // in this case it is ok to use the smaller indentation. That would
+ // make the indentation rules more complicated than necessary though.
+ // If absolutely necessary, it is possible to use a continued line
+ // with only the backslash in the first line. These may be indented
+ // with a single tab.
test(
"VAR.param=\tvalue \\",
"\t10........20........30........40........50........60...65",
+ false)
+
+ test(
+ "VAR.param=\tvalue \\",
+ "\t\t\t......32\t\t\t\t\t......80.",
true)
// Having the continuation line in column 0 looks even more confusing.
@@ -2722,13 +2736,13 @@ func (s *Suite) Test_VaralignBlock__cont
func (s *Suite) Test_VaralignBlock__realign_continuation_backslashes(c *check.C) {
vt := NewVaralignTester(s, c)
vt.Input(
- "VAR4567890.234567890=\t----30--------40--------50\t\t\t\\",
- "\t\t--20--------30--------40--------50\t\t\t\\",
- "\t\t--20--------30--------40--------50")
+ "VAR4567890.234567890=\t....30........40........50\t\t\t\\",
+ "\t\t..20........30........40........50\t\t\t\\",
+ "\t\t..20........30........40........50")
vt.InputDetab(
- "VAR4567890.234567890= ----30--------40--------50 \\",
- " --20--------30--------40--------50 \\",
- " --20--------30--------40--------50")
+ "VAR4567890.234567890= ....30........40........50 \\",
+ " ..20........30........40........50 \\",
+ " ..20........30........40........50")
vt.Internals(
"21 24 72",
" 16 72",
@@ -2741,9 +2755,9 @@ func (s *Suite) Test_VaralignBlock__real
"AUTOFIX: Makefile:2: Replacing \"\\t\\t\\t\\\\\" with \"\\t\\t\\\\\".",
"AUTOFIX: Makefile:3: Replacing \"\\t\\t\" with \"\\t\\t\\t\".")
vt.Fixed(
- "VAR4567890.234567890= ----30--------40--------50 \\",
- " --20--------30--------40--------50 \\",
- " --20--------30--------40--------50")
+ "VAR4567890.234567890= ....30........40........50 \\",
+ " ..20........30........40........50 \\",
+ " ..20........30........40........50")
vt.Run()
}
@@ -2788,13 +2802,11 @@ func (s *Suite) Test_VaralignBlock__long
vt.Autofixes(
"AUTOFIX: Makefile:1: Replacing \"\\t\\t \" with \"\\t\".",
"AUTOFIX: Makefile:2: Replacing \"\\t\" with \"\\t\\t\\t\\t\\t\\t\".",
+ "AUTOFIX: Makefile:2: Replacing \" \\t \\\\\" with \" \\\\\".",
"AUTOFIX: Makefile:3: Replacing \"\\t\" with \"\\t\\t\\t\\t\\t\\t\".")
vt.Fixed(
"VAR= value \\",
- // FIXME: The backslash should be aligned properly.
- // It is not replaced because alignContinuation is called before fixAlign,
- // which is the wrong order.
- " value \\",
+ " value \\",
" value")
vt.Run()
}
@@ -3073,26 +3085,36 @@ func (s *Suite) Test_VaralignBlock_Finis
// of the variable value.
"NOTE: Makefile:1: The continuation backslash should be "+
"in column 73, not 81.",
- // Line 2 is not indented to column 32
- // since that would make the line longer than 72 columns.
+ "NOTE: Makefile:2: This continuation line should be "+
+ "indented with \"\\t\\t\\t\\t\".",
"NOTE: Makefile:3: This continuation line should be "+
"indented with \"\\t\\t\\t\\t\".")
vt.Autofixes(
"AUTOFIX: Makefile:1: Replacing \"\\t\\t\\t\\t\" with \"\\t\\t\\t\".",
+ "AUTOFIX: Makefile:2: Replacing \"\\t\\t\\t\" with \"\\t\\t\\t\\t\".",
+ "AUTOFIX: Makefile:2: Replacing \"\\t\\\\\" with \" \\\\\".",
"AUTOFIX: Makefile:3: Replacing \"\\t\\t\\t\" with \"\\t\\t\\t\\t\".")
vt.Fixed(
"VAR....8......16..= ......40......48. \\",
- " ......32......40......48......56......64.. \\",
+ " ......32......40......48......56......64.. \\",
" ...29")
vt.Run()
}
+func (s *Suite) Test_varalignLine_realignDetails(c *check.C) {
+ t := s.Init(c)
+
+ // FIXME
+
+ t.CheckOutputEmpty()
+}
+
// This example is quite unrealistic since typically the first line is
// the least indented.
//
// All follow-up lines are indented with at least one tab, to make clear
// they are continuation lines.
-func (s *Suite) Test_VaralignBlock_realignMultiEmptyFollow(c *check.C) {
+func (s *Suite) Test_varalignLine_realignMultiEmptyFollow(c *check.C) {
vt := NewVaralignTester(s, c)
vt.Input(
"VAR= \\",
@@ -3135,7 +3157,7 @@ func (s *Suite) Test_VaralignBlock_reali
vt.Run()
}
-func (s *Suite) Test_VaralignBlock_realignMultiInitial__spaces(c *check.C) {
+func (s *Suite) Test_varalignLine_realignMultiInitial__spaces(c *check.C) {
vt := NewVaralignTester(s, c)
vt.Input(
"VAR= value1 \\",
@@ -3568,8 +3590,7 @@ func (s *Suite) Test_varalignLine_alignV
info.alignValueSingle(column)
t.CheckEqualsf(
- mkline.raw[0].text(),
- condStr(autofix, after, before),
+ mkline.raw[0].text(), after,
"Line.raw.text, autofix=%v", autofix)
// As of 2019-12-11, the info fields are not updated
@@ -3714,14 +3735,10 @@ func (s *Suite) Test_varalignLine_alignV
info.alignValueMultiInitial(column)
t.CheckEqualsf(
- mkline.raw[0].text(),
- condStr(autofix, after, before),
+ mkline.raw[0].text(), after,
"Line.raw.text, autofix=%v", autofix)
- // As of 2019-12-11, the info fields are not updated
- // accordingly, but they should.
- // TODO: update info accordingly
- t.CheckEqualsf(info.String(), before,
+ t.CheckEqualsf(info.String(), after,
"info.String, autofix=%v", autofix)
}
@@ -3776,24 +3793,112 @@ func (s *Suite) Test_varalignLine_alignV
test()
}
+func (s *Suite) Test_varalignLine_alignValueMultiFollow(c *check.C) {
+ t := s.Init(c)
+
+ // newLine creates a line consisting of either 2 or 3 physical lines.
+ // The text ends up in the raw line with index 1.
+ newLine := func(text string, column, indentDiff int) (*varalignLine, *RawLine) {
+ t.CheckDotColumns("", text)
+
+ leading := alignWith("VAR=", indent(column)) + "value \\"
+ trailing := indent(column) + "trailing"
+ n := condInt(hasSuffix(text, "\\"), 3, 2)
+ lines := []string{leading, text, trailing}[:n]
+
+ mklines := t.NewMkLines("filename.mk",
+ lines...)
+ assert(len(mklines.mklines) == 1)
+ mkline := mklines.mklines[0]
+
+ parts := NewVaralignSplitter().split(text, false)
+ isLong := parts.isTooLongFor(column + indentDiff)
+ return &varalignLine{mkline, 1, isLong, false, parts}, mkline.raw[1]
+ }
+
+ test := func(before string, column, indentDiff int, after string, diagnostics ...string) {
+
+ doTest := func(autofix bool) {
+ info, raw := newLine(before, column, indentDiff)
+
+ info.alignValueMultiFollow(column, indentDiff)
+
+ t.CheckEquals(raw.text(), after)
+ }
+
+ t.ExpectDiagnosticsAutofix(
+ doTest,
+ diagnostics...)
+ }
+
+ test(
+ "value", 24, 0,
+ "\t\t\tvalue",
+
+ "NOTE: filename.mk:2: This continuation line should be "+
+ "indented with \"\\t\\t\\t\".",
+ "AUTOFIX: filename.mk:2: Replacing \"\" with \"\\t\\t\\t\".")
+
+ test(
+ "value \\", 24, 0,
+ "\t\t\tvalue \\",
+
+ "NOTE: filename.mk:2: This continuation line should be "+
+ "indented with \"\\t\\t\\t\".",
+ "AUTOFIX: filename.mk:2: Replacing \"\" with \"\\t\\t\\t\".")
+
+ test(
+ "value \\", 24, 0,
+ "\t\t\tvalue \\",
+
+ "NOTE: filename.mk:2: This continuation line should be "+
+ "indented with \"\\t\\t\\t\".",
+ "AUTOFIX: filename.mk:2: Replacing \"\" with \"\\t\\t\\t\".",
+ "AUTOFIX: filename.mk:2: Replacing \" \\\\\" with \" \\\\\".")
+
+ // As a special case, a continuation backslash in column 72 is preserved.
+ // TODO: Make this more general.
+ test(
+ "value\t\t\t\t\t\t\t\t\t\\", 24, 0,
+ "\t\t\tvalue\t\t\t\t\t\t\\",
+
+ "NOTE: filename.mk:2: This continuation line should be indented "+
+ "with \"\\t\\t\\t\".",
+ "AUTOFIX: filename.mk:2: Replacing \"\" with \"\\t\\t\\t\".",
+ "AUTOFIX: filename.mk:2: "+
+ "Replacing \"\\t\\t\\t\\t\\t\\t\\t\\t\\t\\\\\" "+
+ "with \"\\t\\t\\t\\t\\t\\t\\\\\".")
+
+ // If the value is so wide that the continuation backslash cannot
+ // be kept in column 72, the line is still adjusted, and the
+ // continuation backslash is separated with a single space.
+ test(
+ "value\t\t\t\t\t\t\t......64\t\\", 24, 0,
+ "\t\t\tvalue\t\t\t\t\t\t\t......64 \\",
+
+ "NOTE: filename.mk:2: This continuation line should be indented with \"\\t\\t\\t\".",
+ "AUTOFIX: filename.mk:2: Replacing \"\" with \"\\t\\t\\t\".",
+ "AUTOFIX: filename.mk:2: Replacing \"\\t\\\\\" with \" \\\\\".")
+}
+
func (s *Suite) Test_varalignLine_alignValueMultiFollow__unindent_long_lines(c *check.C) {
vt := NewVaralignTester(s, c)
vt.Input(
"SHORT=\tvalue",
- "PROGRAM_AWK=\t\t\t\t--------50--------60--------70 \\",
+ "PROGRAM_AWK=\t\t\t\t........50........60........70 \\",
"\t\t\t\t\t\t\t\t\t3 \\",
"\t\t\t\t\t\t\t\t\t74 \\",
"\t\t\t\t\t\t\t\t\t-75 \t\t\t \\",
- "\t\t\t\t\t\t\t\t\t--76 \\",
+ "\t\t\t\t\t\t\t\t\t..76 \\",
"\t\t\t\t\t\t\t\t66 \\",
"\t\t\t\t\t\t\t\t1")
vt.InputDetab(
"SHORT= value",
- "PROGRAM_AWK= --------50--------60--------70 \\",
+ "PROGRAM_AWK= ........50........60........70 \\",
" 3 \\",
" 74 \\",
" -75 \\",
- " --76 \\",
+ " ..76 \\",
" 66 \\",
" 1")
vt.Internals(
@@ -3808,12 +3913,8 @@ func (s *Suite) Test_varalignLine_alignV
vt.Diagnostics(
"NOTE: Makefile:1: This variable value should be aligned to column 17.",
"NOTE: Makefile:2: This variable value should be aligned to column 17.",
- // XXX: Wrong order; should be strictly from left to right.
- "NOTE: Makefile:3: The continuation backslash should be preceded by a single space or tab.",
"NOTE: Makefile:3: This continuation line should be indented with \"\\t\\t\\t\\t\\t\\t\".",
- "NOTE: Makefile:4: The continuation backslash should be preceded by a single space or tab.",
"NOTE: Makefile:4: This continuation line should be indented with \"\\t\\t\\t\\t\\t\\t\".",
- "NOTE: Makefile:5: The continuation backslash should be preceded by a single space or tab.",
"NOTE: Makefile:5: This continuation line should be indented with \"\\t\\t\\t\\t\\t\\t\".",
"NOTE: Makefile:6: This continuation line should be indented with \"\\t\\t\\t\\t\\t\\t\".",
"NOTE: Makefile:7: This continuation line should be indented with \"\\t\\t\\t\\t\\t\".",
@@ -3821,12 +3922,12 @@ func (s *Suite) Test_varalignLine_alignV
vt.Autofixes(
"AUTOFIX: Makefile:1: Replacing \"\\t\" with \"\\t\\t\".",
"AUTOFIX: Makefile:2: Replacing \"\\t\\t\\t\\t\" with \"\\t\".",
- "AUTOFIX: Makefile:3: Replacing \" \" with \" \".",
"AUTOFIX: Makefile:3: Replacing \"\\t\\t\\t\\t\\t\\t\\t\\t\\t\" with \"\\t\\t\\t\\t\\t\\t\".",
- "AUTOFIX: Makefile:4: Replacing \" \" with \" \".",
+ "AUTOFIX: Makefile:3: Replacing \" \\\\\" with \"\\t\\t\\t\\\\\".",
"AUTOFIX: Makefile:4: Replacing \"\\t\\t\\t\\t\\t\\t\\t\\t\\t\" with \"\\t\\t\\t\\t\\t\\t\".",
- "AUTOFIX: Makefile:5: Replacing \" \\t\\t\\t \" with \" \".",
+ "AUTOFIX: Makefile:4: Replacing \" \\\\\" with \"\\t\\t\\t\\\\\".",
"AUTOFIX: Makefile:5: Replacing \"\\t\\t\\t\\t\\t\\t\\t\\t\\t\" with \"\\t\\t\\t\\t\\t\\t\".",
+ "AUTOFIX: Makefile:5: Replacing \" \\t\\t\\t \\\\\" with \"\\t\\t\\t\\\\\".",
"AUTOFIX: Makefile:6: Replacing \"\\t\\t\\t\\t\\t\\t\\t\\t\\t\" with \"\\t\\t\\t\\t\\t\\t\".",
"AUTOFIX: Makefile:7: Replacing \"\\t\\t\\t\\t\\t\\t\\t\\t\" with \"\\t\\t\\t\\t\\t\".",
"AUTOFIX: Makefile:8: Replacing \"\\t\\t\\t\\t\\t\\t\\t\\t\" with \"\\t\\t\\t\\t\\t\".")
@@ -3835,29 +3936,32 @@ func (s *Suite) Test_varalignLine_alignV
// considered "long" anymore, therefore the backslashes are not
// kept in column 72. Nevertheless they look unorganized right now.
"SHORT= value",
- "PROGRAM_AWK= --------50--------60--------70 \\",
- " 3 \\",
- " 74 \\",
- " -75 \\",
- " --76 \\",
+ "PROGRAM_AWK= ........50........60........70 \\",
+ " 3 \\",
+ " 74 \\",
+ " -75 \\",
+ " ..76 \\",
" 66 \\",
" 1")
vt.Run()
}
+// In this example, the continued lines are indented less than the line
+// containing the first value. This is not the common style, therefore all
+// continuation lines are aligned to the value of the first line.
func (s *Suite) Test_varalignLine_alignValueMultiFollow__unindent_long_initial_line(c *check.C) {
vt := NewVaralignTester(s, c)
vt.Input(
- "VAR-----10!=\t\t----30--------40--------50-----6\t\t\t\\",
- "\t\t --------30--------40-\t\t\t\t\\",
- "\t\t --------30--------40--------50--------60-------8\t\\",
- "\t\t ----5\t\t\t\t\t\t\\",
+ "VAR.....10!=\t\t....30........40........50....56\t\t\t\\",
+ "\t\t ........30........40.\t\t\t\t\\",
+ "\t\t ........30........40........50........60......68\t\\",
+ "\t\t ...25\t\t\t\t\t\t\\",
"\t\t-7")
vt.InputDetab(
- "VAR-----10!= ----30--------40--------50-----6 \\",
- " --------30--------40- \\",
- " --------30--------40--------50--------60-------8 \\",
- " ----5 \\",
+ "VAR.....10!= ....30........40........50....56 \\",
+ " ........30........40. \\",
+ " ........30........40........50........60......68 \\",
+ " ...25 \\",
" -7")
vt.Internals(
"12 24 80",
@@ -3868,26 +3972,48 @@ func (s *Suite) Test_varalignLine_alignV
vt.Diagnostics(
"NOTE: Makefile:1: The continuation backslash should be in column 73, not 81.",
"NOTE: Makefile:2: This continuation line should be indented with \"\\t\\t\\t\".",
+ "NOTE: Makefile:3: This continuation line should be indented with \"\\t\\t\\t\".",
"NOTE: Makefile:4: This continuation line should be indented with \"\\t\\t\\t\".",
"NOTE: Makefile:5: This continuation line should be indented with \"\\t\\t\\t\".")
vt.Autofixes(
- // FIXME: Mention the continuation backslash in the replacement.
"AUTOFIX: Makefile:1: Replacing \"\\t\\t\\t\" with \"\\t\\t\".",
"AUTOFIX: Makefile:2: Replacing \"\\t\\t \" with \"\\t\\t\\t\".",
+ "AUTOFIX: Makefile:3: Replacing \"\\t\\t \" with \"\\t\\t\\t\".",
+ "AUTOFIX: Makefile:3: Replacing \"\\t\\\\\" with \" \\\\\".",
"AUTOFIX: Makefile:4: Replacing \"\\t\\t \" with \"\\t\\t\\t\".",
"AUTOFIX: Makefile:5: Replacing \"\\t\\t\" with \"\\t\\t\\t\".")
vt.Fixed(
- "VAR-----10!= ----30--------40--------50-----6 \\",
- // FIXME: Preserve the original relative indentation.
- " --------30--------40- \\",
- // FIXME: Preserve the original relative indentation.
- " --------30--------40--------50--------60-------8 \\",
- // FIXME: Preserve the original relative indentation.
- " ----5 \\",
+ "VAR.....10!= ....30........40........50....56 \\",
+ " ........30........40. \\",
+ " ........30........40........50........60......68 \\",
+ " ...25 \\",
" -7")
vt.Run()
}
+// The seemingly empty line 3 is actually a continuation from the line above it.
+// Its indentation is is not fixed since that would lead to trailing whitespace.
+func (s *Suite) Test_varalignLine_alignFollow(c *check.C) {
+ vt := NewVaralignTester(s, c)
+ vt.Input(
+ "VAR=\t\t...21 \\",
+ "\t\t...21 \\",
+ "")
+ vt.InputDetab(
+ "VAR= ...21 \\",
+ " ...21 \\",
+ "")
+ vt.Diagnostics(
+ nil...)
+ vt.Autofixes(
+ nil...)
+ vt.Fixed(
+ "VAR= ...21 \\",
+ " ...21 \\",
+ "")
+ vt.Run()
+}
+
func (s *Suite) Test_varalignLine_alignContinuation(c *check.C) {
t := s.Init(c)
@@ -3907,14 +4033,10 @@ func (s *Suite) Test_varalignLine_alignC
info.alignContinuation(valueColumn, rightMarginColumn)
t.CheckEqualsf(
- mkline.raw[rawIndex].text(),
- condStr(autofix, after, before[rawIndex]),
+ mkline.raw[rawIndex].text(), after,
"Line.raw.text, autofix=%v", autofix)
- // As of 2019-12-11, the info fields are not updated
- // accordingly, but they should.
- // TODO: update info accordingly
- t.CheckEqualsf(info.String(), before[rawIndex],
+ t.CheckEqualsf(info.String(), after,
"info.String, autofix=%v", autofix)
}
@@ -4047,6 +4169,17 @@ func (s *Suite) Test_varalignLine_alignC
// a single space, to keep it as close to the text as possible.
test(
lines(
+ "VAR=\t...13\t\t\t\t\t\t\t\t...77\t\\",
+ "\t...13"),
+ 0, 32, 48,
+
+ "VAR=\t...13\t\t\t\t\t\t\t\t...77 \\",
+ "NOTE: filename.mk:1: The continuation backslash should be "+
+ "preceded by a single space.",
+ "AUTOFIX: filename.mk:1: Replacing \"\\t\" with \" \".")
+
+ test(
+ lines(
"VAR=\t...13\t\t\t\t\t\t\t\t...77\t\t\t\\",
"\t...13"),
0, 32, 48,
@@ -4057,6 +4190,22 @@ func (s *Suite) Test_varalignLine_alignC
"AUTOFIX: filename.mk:1: Replacing \"\\t\\t\\t\" with \" \".")
}
+func (s *Suite) Test_varalignLine_replaceSpaceBeforeValue(c *check.C) {
+ t := s.Init(c)
+
+ // FIXME
+
+ t.CheckOutputEmpty()
+}
+
+func (s *Suite) Test_varalignLine_replaceSpaceBeforeContinuationSilently(c *check.C) {
+ t := s.Init(c)
+
+ // FIXME
+
+ t.CheckOutputEmpty()
+}
+
func (s *Suite) Test_varalignLine_explainWrongColumn(c *check.C) {
t := s.Init(c)
Index: pkgsrc/pkgtools/pkglint/files/vartypecheck_test.go
diff -u pkgsrc/pkgtools/pkglint/files/vartypecheck_test.go:1.67 pkgsrc/pkgtools/pkglint/files/vartypecheck_test.go:1.68
--- pkgsrc/pkgtools/pkglint/files/vartypecheck_test.go:1.67 Sat Dec 14 18:04:16 2019
+++ pkgsrc/pkgtools/pkglint/files/vartypecheck_test.go Mon Dec 16 17:06:05 2019
@@ -1641,7 +1641,8 @@ func (s *Suite) Test_VartypeCheck_SedCom
"1d",
"-e",
"-i s,from,to,",
- "-e s,$${unclosedShellVar") // Just for code coverage.
+ "-e s,$${unclosedShellVar", // Just for code coverage.
+ "-e s,...") // Syntactically invalid sed command.
vt.Output(
"NOTE: filename.mk:1: Please always use \"-e\" in sed commands, even if there is only one substitution.",
@@ -1652,14 +1653,13 @@ func (s *Suite) Test_VartypeCheck_SedCom
"ERROR: filename.mk:9: The -e option to sed requires an argument.",
"WARN: filename.mk:10: Unknown sed command \"-i\".",
"NOTE: filename.mk:10: Please always use \"-e\" in sed commands, even if there is only one substitution.",
- // TODO: duplicate warning
+ // XXX: duplicate warning
"WARN: filename.mk:11: Unclosed shell variable starting at \"$${unclosedShellVar\".",
"WARN: filename.mk:11: Unclosed shell variable starting at \"$${unclosedShellVar\".")
}
func (s *Suite) Test_VartypeCheck_SedCommands__experimental(c *check.C) {
vt := NewVartypeCheckTester(s.Init(c), BtSedCommands)
- G.Experimental = true
vt.Varname("SUBST_SED.dummy")
@@ -2072,12 +2072,15 @@ func (s *Suite) Test_VartypeCheck_Yes(c
vt.Varname("APACHE_MODULE")
vt.Values(
"yes",
+ "YES",
"no",
+ "NO",
"${YESVAR}")
vt.Output(
- "WARN: filename.mk:2: APACHE_MODULE should be set to YES or yes.",
- "WARN: filename.mk:3: APACHE_MODULE should be set to YES or yes.")
+ "WARN: filename.mk:3: APACHE_MODULE should be set to YES or yes.",
+ "WARN: filename.mk:4: APACHE_MODULE should be set to YES or yes.",
+ "WARN: filename.mk:5: APACHE_MODULE should be set to YES or yes.")
vt.Varname("BUILD_USES_MSGFMT")
vt.Op(opUseMatch)
@@ -2106,7 +2109,9 @@ func (s *Suite) Test_VartypeCheck_YesNo(
vt.Varname("PKG_DEVELOPER")
vt.Values(
"yes",
+ "YES",
"no",
+ "NO",
"ja",
"${YESVAR}",
"yes # comment",
@@ -2114,9 +2119,9 @@ func (s *Suite) Test_VartypeCheck_YesNo(
"Yes indeed")
vt.Output(
- "WARN: filename.mk:3: PKG_DEVELOPER should be set to YES, yes, NO, or no.",
- "WARN: filename.mk:4: PKG_DEVELOPER should be set to YES, yes, NO, or no.",
- "WARN: filename.mk:7: PKG_DEVELOPER should be set to YES, yes, NO, or no.")
+ "WARN: filename.mk:5: PKG_DEVELOPER should be set to YES, yes, NO, or no.",
+ "WARN: filename.mk:6: PKG_DEVELOPER should be set to YES, yes, NO, or no.",
+ "WARN: filename.mk:9: PKG_DEVELOPER should be set to YES, yes, NO, or no.")
vt.Op(opUseMatch)
vt.Values(
Home |
Main Index |
Thread Index |
Old Index