pkgsrc-Changes-HG archive
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]
[pkgsrc/trunk]: pkgsrc/pkgtools/pkglint pkgtools/pkglint: update to 19.3.19
details: https://anonhg.NetBSD.org/pkgsrc/rev/d3ed084718c8
branches: trunk
changeset: 419368:d3ed084718c8
user: rillig <rillig%pkgsrc.org@localhost>
date: Mon Dec 16 17:06:05 2019 +0000
description:
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.
diffstat:
pkgtools/pkglint/Makefile | 4 +-
pkgtools/pkglint/files/autofix.go | 136 +++++---
pkgtools/pkglint/files/autofix_test.go | 75 +++-
pkgtools/pkglint/files/check_test.go | 5 +-
pkgtools/pkglint/files/logging.go | 4 +-
pkgtools/pkglint/files/mklinechecker.go | 14 +-
pkgtools/pkglint/files/mklinechecker_test.go | 31 +-
pkgtools/pkglint/files/mktypes.go | 13 +-
pkgtools/pkglint/files/mktypes_test.go | 103 +++---
pkgtools/pkglint/files/package.go | 2 +-
pkgtools/pkglint/files/substcontext.go | 60 ++-
pkgtools/pkglint/files/substcontext_test.go | 338 ++++++++++++++++++--
pkgtools/pkglint/files/util.go | 25 +
pkgtools/pkglint/files/util_test.go | 23 +
pkgtools/pkglint/files/varalignblock.go | 430 +++++++++++++++-----------
pkgtools/pkglint/files/varalignblock_test.go | 333 +++++++++++++++-----
pkgtools/pkglint/files/vartypecheck.go | 9 +-
pkgtools/pkglint/files/vartypecheck_test.go | 21 +-
18 files changed, 1111 insertions(+), 515 deletions(-)
diffs (truncated from 2571 to 300 lines):
diff -r e87bb1c494a2 -r d3ed084718c8 pkgtools/pkglint/Makefile
--- a/pkgtools/pkglint/Makefile Mon Dec 16 17:03:27 2019 +0000
+++ b/pkgtools/pkglint/Makefile Mon Dec 16 17:06:05 2019 +0000
@@ -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/}
diff -r e87bb1c494a2 -r d3ed084718c8 pkgtools/pkglint/files/autofix.go
--- a/pkgtools/pkglint/files/autofix.go Mon Dec 16 17:03:27 2019 +0000
+++ b/pkgtools/pkglint/files/autofix.go Mon Dec 16 17:06:05 2019 +0000
@@ -21,8 +21,7 @@
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 @@
// 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 @@
// 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 @@
// 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 @@
// 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 @@
}
// 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):]
+
+ 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)
}
- replaced = rawLine.textnl[:textIndex] + to + rawLine.textnl[textIndex+len(from):]
-
- if G.Logger.IsAutofix() {
- 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.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 @@
// 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 @@
// 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 @@
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.
+//
+// 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.
//
-// * logs the associated message (default) but does not record the fixes in the line
+// In --show-autofix mode, only those diagnostics are logged that actually fix
+// something. This is done to hide possibly distracting, unrelated diagnostics.
//
-// * logs what would be fixed (--show-autofix) and records the fixes in the line
+// In --autofix mode, only the actual changes are logged, but not the
+// corresponding diagnostics. To get both, specify --show-autofix as well.
//
-// * records the fixes in the line (--autofix), ready for SaveAutofixChanges
+// 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 @@
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) 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 @@
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)
}
diff -r e87bb1c494a2 -r d3ed084718c8 pkgtools/pkglint/files/autofix_test.go
--- a/pkgtools/pkglint/files/autofix_test.go Mon Dec 16 17:03:27 2019 +0000
+++ b/pkgtools/pkglint/files/autofix_test.go Mon Dec 16 17:06:05 2019 +0000
@@ -555,21 +555,43 @@
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()
+ }
+
Home |
Main Index |
Thread Index |
Old Index