diff options
| author | rillig <rillig@pkgsrc.org> | 2019-09-08 22:47:47 +0000 |
|---|---|---|
| committer | rillig <rillig@pkgsrc.org> | 2019-09-08 22:47:47 +0000 |
| commit | 16373ff0777a2b84caca8f1a6e9f6d87e922d00d (patch) | |
| tree | baa90ce7eba145730685f911ba6a030599bb6b1b /pkgtools | |
| parent | 82ca01b68c17584dee09ef5a8f2a29eaaf3c2f1d (diff) | |
| download | pkgsrc-16373ff0777a2b84caca8f1a6e9f6d87e922d00d.tar.gz | |
pkgtools/pkglint: update to 5.7.23
Changes since 5.7.22:
* Added a warning for lines that look empty but are actually follow-up
lines from a previous line.
* Added notes for unusual placement of the continuation backslash. It
should always be preceded by a single space or tab, or be in column
73.
* Improved check for needlessly complicated !empty(PKGPATH:Mcat/pkg)
that is transformed into the simpler ${PKGPATH} == cat/pkg, even if
the package name contains hyphens, dots, plus or slashes.
* Added check for the _VARGROUPS section since that section contains
many redundancies that can easily be checked. For example, in
mk/compiler/gcc.mk the _VARGROUPS section is 67 lines long and
contains far more than 100 variables. It's tedious to manually check
this file for internal consistency. That's better left to pkglint.
* The empty variable is no longer flagged as "used but not defined".
It appears in expressions like ${:Ustring}.
* When --source is combined with --explain, don't unnecessarily
repeat the source code for a single line if there are several
diagnostics. Instead, even omit the empty line between the diagnostics
for the same line.
Diffstat (limited to 'pkgtools')
29 files changed, 2205 insertions, 1064 deletions
diff --git a/pkgtools/pkglint/Makefile b/pkgtools/pkglint/Makefile index 56b31ec2d89..90428517451 100644 --- a/pkgtools/pkglint/Makefile +++ b/pkgtools/pkglint/Makefile @@ -1,6 +1,6 @@ -# $NetBSD: Makefile,v 1.595 2019/08/25 21:47:11 rillig Exp $ +# $NetBSD: Makefile,v 1.596 2019/09/08 22:47:47 rillig Exp $ -PKGNAME= pkglint-5.7.22 +PKGNAME= pkglint-5.7.23 CATEGORIES= pkgtools DISTNAME= tools MASTER_SITES= ${MASTER_SITE_GITHUB:=golang/} diff --git a/pkgtools/pkglint/PLIST b/pkgtools/pkglint/PLIST index bf3bf592960..e5bc128d6f4 100644 --- a/pkgtools/pkglint/PLIST +++ b/pkgtools/pkglint/PLIST @@ -1,4 +1,4 @@ -@comment $NetBSD: PLIST,v 1.13 2019/07/14 21:25:47 rillig Exp $ +@comment $NetBSD: PLIST,v 1.14 2019/09/08 22:47:47 rillig Exp $ bin/pkglint gopkg/pkg/${GO_PLATFORM}/netbsd.org/pkglint.a gopkg/pkg/${GO_PLATFORM}/netbsd.org/pkglint/getopt.a @@ -116,6 +116,8 @@ gopkg/src/netbsd.org/pkglint/varalignblock.go gopkg/src/netbsd.org/pkglint/varalignblock_test.go gopkg/src/netbsd.org/pkglint/vardefs.go gopkg/src/netbsd.org/pkglint/vardefs_test.go +gopkg/src/netbsd.org/pkglint/vargroups.go +gopkg/src/netbsd.org/pkglint/vargroups_test.go gopkg/src/netbsd.org/pkglint/vartype.go gopkg/src/netbsd.org/pkglint/vartype_test.go gopkg/src/netbsd.org/pkglint/vartypecheck.go diff --git a/pkgtools/pkglint/files/autofix.go b/pkgtools/pkglint/files/autofix.go index 936f09f3611..d31717849f3 100644 --- a/pkgtools/pkglint/files/autofix.go +++ b/pkgtools/pkglint/files/autofix.go @@ -127,7 +127,7 @@ func (fix *Autofix) ReplaceAfter(prefix, from string, to string) { // 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, column int, from string, to string) { +func (fix *Autofix) ReplaceAt(rawIndex int, textIndex int, from string, to string) { assert(from != to) fix.assertRealLine() @@ -136,11 +136,11 @@ func (fix *Autofix) ReplaceAt(rawIndex int, column int, from string, to string) } rawLine := fix.line.raw[rawIndex] - if column >= len(rawLine.textnl) || !hasPrefix(rawLine.textnl[column:], from) { + if textIndex >= len(rawLine.textnl) || !hasPrefix(rawLine.textnl[textIndex:], from) { return } - replaced := rawLine.textnl[:column] + to + rawLine.textnl[column+len(from):] + replaced := rawLine.textnl[:textIndex] + to + rawLine.textnl[textIndex+len(from):] if G.Logger.IsAutofix() { rawLine.textnl = replaced @@ -328,7 +328,6 @@ func (fix *Autofix) Apply() { fix.modified = true } - // Reduce number of calls to runtime.writeBarrier. fix.autofixShortTerm = autofixShortTerm{} } @@ -364,20 +363,11 @@ func (fix *Autofix) Apply() { } G.Logger.Logf(AutofixLogLevel, line.Filename, lineno, AutofixFormat, action.description) } + G.Logger.showSource(line) } - if logDiagnostic || logFix { - if logFix { - G.Logger.showSource(line) - } - if logDiagnostic && len(fix.explanation) > 0 { - line.Explain(fix.explanation...) - } - if G.Logger.Opts.ShowSource { - if !(G.Logger.Opts.Explain && logDiagnostic && len(fix.explanation) > 0) { - G.Logger.out.Separate() - } - } + if logDiagnostic && len(fix.explanation) > 0 { + line.Explain(fix.explanation...) } reset() diff --git a/pkgtools/pkglint/files/autofix_test.go b/pkgtools/pkglint/files/autofix_test.go index 321f8bd6199..f336d9292c3 100644 --- a/pkgtools/pkglint/files/autofix_test.go +++ b/pkgtools/pkglint/files/autofix_test.go @@ -966,8 +966,6 @@ func (s *Suite) Test_Autofix_Apply__explanation_followed_by_note(c *check.C) { t.CheckOutputLines( ">\ttext", "WARN: README.txt:123: A warning with autofix.", - "", - ">\ttext", "NOTE: README.txt:123: A note without fix.") } diff --git a/pkgtools/pkglint/files/category_test.go b/pkgtools/pkglint/files/category_test.go index fa1e6a104f4..e279d41fee0 100644 --- a/pkgtools/pkglint/files/category_test.go +++ b/pkgtools/pkglint/files/category_test.go @@ -343,3 +343,28 @@ func (s *Suite) Test_CheckdirCategory__no_Makefile(c *check.C) { t.CheckOutputLines( "ERROR: ~/category/Makefile: Cannot be read.") } + +func (s *Suite) Test_CheckdirCategory__case_mismatch(c *check.C) { + t := s.Init(c) + + t.SetUpPkgsrc() + t.CreateFileLines("mk/misc/category.mk") + t.CreateFileLines("category/p5-Net-DNS/Makefile") + t.CreateFileLines("category/Makefile", + MkCvsID, + "", + "COMMENT=\tCategory comment", + "", + "SUBDIR+=\tp5-net-dns", + "", + ".include \"../mk/misc/category.mk\"") + t.FinishSetUp() + + G.Check(t.File("category")) + + t.CheckOutputLines( + "ERROR: ~/category/Makefile:5: \"p5-Net-DNS\" "+ + "exists in the file system but not in the Makefile.", + "ERROR: ~/category/Makefile:5: \"p5-net-dns\" "+ + "exists in the Makefile but not in the file system.") +} diff --git a/pkgtools/pkglint/files/lines.go b/pkgtools/pkglint/files/lines.go index 77aef21e675..7d76fb318ea 100644 --- a/pkgtools/pkglint/files/lines.go +++ b/pkgtools/pkglint/files/lines.go @@ -42,7 +42,11 @@ func (ls *Lines) CheckCvsID(index int, prefixRe regex.Pattern, suggestedPrefix s line := ls.Lines[index] if m, expanded := line.IsCvsID(prefixRe); m { - if G.Testing && G.Wip && expanded { + // This check is considered experimental because it produces a few + // thousand notes and doesn't really affect the functionality of + // the packages. The worst thing that might happen is that a file + // looks older than it really is. + if G.Experimental && G.Wip && expanded { fix := line.Autofix() fix.Notef("Expected exactly %q.", suggestedPrefix+"$"+"NetBSD$") fix.Explain( diff --git a/pkgtools/pkglint/files/lines_test.go b/pkgtools/pkglint/files/lines_test.go index 3d105d74cd9..a065bd030b4 100644 --- a/pkgtools/pkglint/files/lines_test.go +++ b/pkgtools/pkglint/files/lines_test.go @@ -30,6 +30,7 @@ func (s *Suite) Test_Lines_CheckCvsID(c *check.C) { func (s *Suite) Test_Lines_CheckCvsID__wip(c *check.C) { t := s.Init(c) + G.Experimental = true t.SetUpPkgsrc() t.SetUpPackage("wip/package") t.CreateFileLines("wip/package/file1.mk", diff --git a/pkgtools/pkglint/files/logging.go b/pkgtools/pkglint/files/logging.go index ea1cbdb5cd5..53bc5219a20 100644 --- a/pkgtools/pkglint/files/logging.go +++ b/pkgtools/pkglint/files/logging.go @@ -16,6 +16,7 @@ type Logger struct { suppressDiag bool suppressExpl bool + prevLine *Line logged Once explained Once @@ -98,8 +99,16 @@ func (l *Logger) Explain(explanation ...string) { return } + // The explanation should fit nicely on a screen that is 80 + // characters wide. The explanation is indented using a tab, and + // there should be a little margin at the right. The resulting + // number comes remarkably close to the line width recommended + // by typographers, which is 66. + const explanationWidth = 80 - 8 - 4 + + l.prevLine = nil l.out.Separate() - wrapped := wrap(68, explanation...) + wrapped := wrap(explanationWidth, explanation...) for _, explanationLine := range wrapped { if explanationLine != "" { l.out.Write("\t") @@ -114,6 +123,10 @@ func (l *Logger) ShowSummary() { return } + if l.Opts.ShowSource { + l.out.Separate() + } + if l.errors != 0 || l.warnings != 0 { num := func(n int, singular, plural string) string { if n == 0 { @@ -187,12 +200,13 @@ func (l *Logger) Diag(line *Line, level *LogLevel, format string, args ...interf } if l.Opts.ShowSource { + if !l.IsAutofix() && line != l.prevLine && level != Fatal { + l.out.Separate() + } l.showSource(line) - l.Logf(level, filename, linenos, format, msg) - l.out.Separate() - } else { - l.Logf(level, filename, linenos, format, msg) } + + l.Logf(level, filename, linenos, format, msg) } func (l *Logger) showSource(line *Line) { @@ -200,6 +214,13 @@ func (l *Logger) showSource(line *Line) { return } + if !l.IsAutofix() { + if line == l.prevLine { + return + } + l.prevLine = line + } + out := l.out writeLine := func(prefix, line string) { out.Write(prefix) @@ -229,6 +250,9 @@ func (l *Logger) showSource(line *Line) { } } + if !l.IsAutofix() { + l.out.Separate() + } if line.autofix != nil { for _, before := range line.autofix.linesBefore { writeLine("+\t", before) @@ -240,6 +264,9 @@ func (l *Logger) showSource(line *Line) { } else { printDiff(line.raw) } + if l.IsAutofix() { + l.out.Separate() + } } func (l *Logger) Logf(level *LogLevel, filename, lineno, format, msg string) { @@ -324,7 +351,7 @@ type SeparatorWriter struct { } func NewSeparatorWriter(out io.Writer) *SeparatorWriter { - return &SeparatorWriter{out: out} + return &SeparatorWriter{out: out, state: 3} } func (wr *SeparatorWriter) WriteLine(text string) { @@ -339,11 +366,10 @@ func (wr *SeparatorWriter) Write(text string) { } // Separate remembers to output an empty line before the next character. -// If the writer is currently in the middle of a line, that line is terminated immediately. +// +// The writer must not be in the middle of a line. func (wr *SeparatorWriter) Separate() { - if wr.state == 1 { - wr.write('\n') - } + assert(wr.state != 1) if wr.state < 2 { wr.state = 2 } diff --git a/pkgtools/pkglint/files/logging_test.go b/pkgtools/pkglint/files/logging_test.go index 8b8f66e04b4..835c448749c 100644 --- a/pkgtools/pkglint/files/logging_test.go +++ b/pkgtools/pkglint/files/logging_test.go @@ -206,9 +206,107 @@ func (s *Suite) Test_Logger__show_source_separator(c *check.C) { "", ">\tThe third line", "WARN: ~/DESCR:3: Dummy warning.", + "WARN: ~/DESCR:3: Using \"third\" is deprecated.") +} + +func (s *Suite) Test_Logger__show_source_with_explanation(c *check.C) { + t := s.Init(c) + + t.SetUpCommandLine("--source", "--explain") + lines := t.SetUpFileLines("DESCR", + "The first line", + "The second line", + "The third line") + + fix := lines.Lines[1].Autofix() + fix.Warnf("Using \"second\" is deprecated.") + fix.Explain("Explanation 1.") + fix.Replace("second", "silver medal") + fix.Apply() + + lines.Lines[2].Warnf("Dummy warning.") + + fix = lines.Lines[2].Autofix() + fix.Warnf("Using \"third\" is deprecated.") + fix.Explain("Explanation 2.") + fix.Replace("third", "bronze medal") + fix.Apply() + + t.CheckOutputLines( + ">\tThe second line", + "WARN: ~/DESCR:2: Using \"second\" is deprecated.", + "", + "\tExplanation 1.", "", ">\tThe third line", - "WARN: ~/DESCR:3: Using \"third\" is deprecated.") + "WARN: ~/DESCR:3: Dummy warning.", + "WARN: ~/DESCR:3: Using \"third\" is deprecated.", + "", + "\tExplanation 2.", + "") +} + +// In general, it is not necessary to repeat the source code for a line +// if there are several diagnostics for the same line. In this case though, +// there is an explanation between the diagnostics, and because it may get +// quite long, it's better to repeat the source code once again. +func (s *Suite) Test_Logger__show_source_with_explanation_in_same_line(c *check.C) { + t := s.Init(c) + + t.SetUpCommandLine("--source", "--explain") + lines := t.SetUpFileLines("DESCR", + "The first line") + + fix := lines.Lines[0].Autofix() + fix.Warnf("Using \"The\" is deprecated.") + fix.Explain("Explanation 1.") + fix.Replace("The", "A") + fix.Apply() + + fix.Warnf("Using \"first\" is deprecated.") + fix.Explain("Explanation 2.") + fix.Replace("first", "1st") + fix.Apply() + + t.CheckOutputLines( + ">\tThe first line", + "WARN: ~/DESCR:1: Using \"The\" is deprecated.", + "", + "\tExplanation 1.", + "", + ">\tThe first line", + "WARN: ~/DESCR:1: Using \"first\" is deprecated.", + "", + "\tExplanation 2.", + "") +} + +// When there is no explanation after the first diagnostic, it is not +// necessary to repeat the source code again for the second diagnostic. +func (s *Suite) Test_Logger__show_source_without_explanation_in_same_line(c *check.C) { + t := s.Init(c) + + t.SetUpCommandLine("--source", "--explain") + lines := t.SetUpFileLines("DESCR", + "The first line") + + fix := lines.Lines[0].Autofix() + fix.Warnf("Using \"The\" is deprecated.") + fix.Replace("The", "A") + fix.Apply() + + fix.Warnf("Using \"first\" is deprecated.") + fix.Explain("Explanation 2.") + fix.Replace("first", "1st") + fix.Apply() + + t.CheckOutputLines( + ">\tThe first line", + "WARN: ~/DESCR:1: Using \"The\" is deprecated.", + "WARN: ~/DESCR:1: Using \"first\" is deprecated.", + "", + "\tExplanation 2.", + "") } // When the --show-autofix option is given, the warning is shown first, @@ -250,6 +348,46 @@ func (s *Suite) Test__show_source_separator_show_autofix(c *check.C) { "+\tThe bronze medal line") } +func (s *Suite) Test__show_source_separator_show_autofix_with_explanation(c *check.C) { + t := s.Init(c) + + t.SetUpCommandLine("--source", "--show-autofix", "--explain") + lines := t.SetUpFileLines("DESCR", + "The first line", + "The second line", + "The third line") + + fix := lines.Lines[1].Autofix() + fix.Warnf("Using \"second\" is deprecated.") + fix.Explain("Explanation 1.") + fix.Replace("second", "silver medal") + fix.Apply() + + lines.Lines[2].Warnf("Dummy warning.") + + fix = lines.Lines[2].Autofix() + fix.Warnf("Using \"third\" is deprecated.") + fix.Explain("Explanation 2.") + fix.Replace("third", "bronze medal") + fix.Apply() + + t.CheckOutputLines( + "WARN: ~/DESCR:2: Using \"second\" is deprecated.", + "AUTOFIX: ~/DESCR:2: Replacing \"second\" with \"silver medal\".", + "-\tThe second line", + "+\tThe silver medal line", + "", + "\tExplanation 1.", + "", + "WARN: ~/DESCR:3: Using \"third\" is deprecated.", + "AUTOFIX: ~/DESCR:3: Replacing \"third\" with \"bronze medal\".", + "-\tThe third line", + "+\tThe bronze medal line", + "", + "\tExplanation 2.", + "") +} + // See Test__show_source_separator_show_autofix for the ordering of the // output lines. // @@ -892,6 +1030,10 @@ func (s *Suite) Test_SeparatorWriter_Flush(c *check.C) { t.CheckEquals(sb.String(), "ab") + t.ExpectAssert(wr.Separate) // Must not be called in the middle of a line. + + wr.WriteLine("") + wr.Separate() // The current line is terminated immediately by the above Separate(), diff --git a/pkgtools/pkglint/files/mkline_test.go b/pkgtools/pkglint/files/mkline_test.go index 81d9b1d3e06..36e1eeba162 100644 --- a/pkgtools/pkglint/files/mkline_test.go +++ b/pkgtools/pkglint/files/mkline_test.go @@ -337,7 +337,11 @@ func (s *Suite) Test_MkLine__aligned(c *check.C) { varalign.Finish() output := t.Output() - t.CheckEquals(output == "", expected) + if expected { + t.CheckEquals(output, "") + } else if output == "" { + t.Check(output, check.Not(check.Equals), "") + } } // The first line uses a space for indentation, which is typical of @@ -380,12 +384,17 @@ func (s *Suite) Test_MkLine__aligned(c *check.C) { // The second line is indented less than the first line. This looks // confusing to the human reader because the actual values do not // appear in a rectangular shape in the source code. - // - // There are several cases though where the follow-up lines are quite - // long, therefore it is allowed to indent them with a single tab. test( - "CONFIGURE_ENV+=\tAWK=${AWK:Q} \\", - "\tSED=${SED:Q}", + "VAR.param=\tvalue \\", + "\t10........20........30........40........50........60...4", + 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. + test( + "VAR.param=\tvalue \\", + "\t10........20........30........40........50........60....5", true) // Having the continuation line in column 0 looks even more confusing. @@ -397,7 +406,7 @@ func (s *Suite) Test_MkLine__aligned(c *check.C) { // Longer continuation lines may use internal indentation to represent // AWK or shell code. test( - "GENERATE_PLIST+=\t/pattern/ {\\", + "GENERATE_PLIST+=\t/pattern/ { \\", "\t\t\t action(); \\", "\t\t\t}", true) @@ -405,7 +414,7 @@ func (s *Suite) Test_MkLine__aligned(c *check.C) { // If any of the continuation lines is indented less than the first // line, it looks confusing. test( - "GENERATE_PLIST+=\t/pattern/ {\\", + "GENERATE_PLIST+=\t/pattern/ { \\", "\t action(); \\", "\t}", false) @@ -415,7 +424,7 @@ func (s *Suite) Test_MkLine__aligned(c *check.C) { // the right as the second line. test( "GENERATE_PLIST+= \\", - "\t/pattern/ {\\", + "\t/pattern/ { \\", "\t action(); \\", "\t}", true) @@ -424,7 +433,7 @@ func (s *Suite) Test_MkLine__aligned(c *check.C) { // line is not indented properly. test( "GENERATE_PLIST+= \\", - "\t/pattern/ {\\", + "\t/pattern/ { \\", "\t action(); \\", "}", false) diff --git a/pkgtools/pkglint/files/mklinechecker.go b/pkgtools/pkglint/files/mklinechecker.go index 9dfa903f033..cd6931ed3c6 100644 --- a/pkgtools/pkglint/files/mklinechecker.go +++ b/pkgtools/pkglint/files/mklinechecker.go @@ -2,6 +2,7 @@ package pkglint import ( "netbsd.org/pkglint/regex" + "netbsd.org/pkglint/textproc" "os" "path" "path/filepath" @@ -20,6 +21,7 @@ func (ck MkLineChecker) Check() { LineChecker{mkline.Line}.CheckTrailingWhitespace() LineChecker{mkline.Line}.CheckValidCharacters() + ck.checkEmptyContinuation() switch { case mkline.IsVarassign(): @@ -36,6 +38,21 @@ func (ck MkLineChecker) Check() { } } +func (ck MkLineChecker) checkEmptyContinuation() { + if !ck.MkLine.IsMultiline() { + return + } + + line := ck.MkLine.Line + if line.raw[len(line.raw)-1].orignl == "\n" { + lastLine := NewLine(line.Filename, int(line.lastLine), "", line.raw[len(line.raw)-1]) + lastLine.Warnf("This line looks empty but continues the previous line.") + lastLine.Explain( + "This line should be indented like other continuation lines,", + "and to make things clear, should be a comment line.") + } +} + func (ck MkLineChecker) checkComment() { mkline := ck.MkLine @@ -529,37 +546,22 @@ func (ck MkLineChecker) checkVarUseBuildDefs(varname string) { func (ck MkLineChecker) checkVaruseUndefined(vartype *Vartype, varname string) { switch { - - case !G.Opts.WarnExtra: - return - - case vartype != nil && !vartype.Guessed(): + case !G.Opts.WarnExtra, // Well-known variables are probably defined by the infrastructure. - return - - case ck.MkLines.vars.DefinedSimilar(varname): - return - - case ck.MkLines.forVars[varname]: - return - - case ck.MkLines.vars.Mentioned(varname) != nil: - return - - case G.Pkg != nil && G.Pkg.vars.DefinedSimilar(varname): - return - - case containsVarRef(varname): - return - - case G.Pkgsrc.vartypes.DefinedCanon(varname): - return - - case !ck.MkLines.once.FirstTimeSlice("used but not defined: ", varname): + vartype != nil && !vartype.Guessed(), + ck.MkLines.vars.DefinedSimilar(varname), + ck.MkLines.forVars[varname], + ck.MkLines.vars.Mentioned(varname) != nil, + G.Pkg != nil && G.Pkg.vars.DefinedSimilar(varname), + containsVarRef(varname), + G.Pkgsrc.vartypes.DefinedCanon(varname), + varname == "": return } - ck.MkLine.Warnf("%s is used but not defined.", varname) + if ck.MkLines.once.FirstTimeSlice("used but not defined", varname) { + ck.MkLine.Warnf("%s is used but not defined.", varname) + } } func (ck MkLineChecker) checkVaruseModifiers(varuse *MkVarUse, vartype *Vartype) { @@ -1600,27 +1602,35 @@ func (ck MkLineChecker) simplifyCondition(varuse *MkVarUse, fromEmpty bool, notE modifiers := varuse.modifiers for _, modifier := range modifiers { - if m, positive, pattern, exact := modifier.MatchMatch(); m && (positive || len(modifiers) == 1) { - ck.checkVartype(varname, opUseMatch, pattern, "") + m, positive, pattern, exact := modifier.MatchMatch() + if !m || !positive && len(modifiers) != 1 { + continue + } - vartype := G.Pkgsrc.VariableType(ck.MkLines, varname) - if exact && matches(pattern, `^[\w-/]+$`) && vartype != nil && !vartype.List() { + ck.checkVartype(varname, opUseMatch, pattern, "") - fix := ck.MkLine.Autofix() - fix.Notef("%s should be compared using %s instead of matching against %q.", - varname, condStr(positive == notEmpty, "==", "!="), ":"+modifier.Text) - fix.Explain( - "This variable has a single value, not a list of values.", - "Therefore it feels strange to apply list operators like :M and :N onto it.", - "A more direct approach is to use the == and != operators.", - "", - "An entirely different case is when the pattern contains wildcards like ^, *, $.", - "In such a case, using the :M or :N modifiers is useful and preferred.") - fix.Replace(replace(varname, positive, pattern)) - fix.Anyway() - fix.Apply() - } + vartype := G.Pkgsrc.VariableType(ck.MkLines, varname) + switch { + case !exact, + vartype == nil, + vartype.List(), + textproc.NewLexer(pattern).NextBytesSet(mkCondLiteralChars) != pattern: + continue } + + fix := ck.MkLine.Autofix() + fix.Notef("%s should be compared using %s instead of matching against %q.", + varname, condStr(positive == notEmpty, "==", "!="), ":"+modifier.Text) + fix.Explain( + "This variable has a single value, not a list of values.", + "Therefore it feels strange to apply list operators like :M and :N onto it.", + "A more direct approach is to use the == and != operators.", + "", + "An entirely different case is when the pattern contains wildcards like ^, *, $.", + "In such a case, using the :M or :N modifiers is useful and preferred.") + fix.Replace(replace(varname, positive, pattern)) + fix.Anyway() + fix.Apply() } } diff --git a/pkgtools/pkglint/files/mklinechecker_test.go b/pkgtools/pkglint/files/mklinechecker_test.go index e2f258ffd06..e891ba78edf 100644 --- a/pkgtools/pkglint/files/mklinechecker_test.go +++ b/pkgtools/pkglint/files/mklinechecker_test.go @@ -5,6 +5,26 @@ import ( "runtime" ) +func (s *Suite) Test_MkLineChecker_checkEmptyContinuation(c *check.C) { + t := s.Init(c) + + mklines := t.SetUpFileMkLines("filename.mk", + MkCvsID, + "# line 1 \\", + "", + "# line 2") + + // Don't check this when loading a file, since otherwise the infrastructure + // files could possibly get this warning. Sure, they should be fixed, but + // it's not in the focus of the package maintainer. + t.CheckOutputEmpty() + + mklines.Check() + + t.CheckOutputLines( + "WARN: ~/filename.mk:3: This line looks empty but continues the previous line.") +} + func (s *Suite) Test_MkLineChecker_checkVarassignLeft(c *check.C) { t := s.Init(c) diff --git a/pkgtools/pkglint/files/mklines.go b/pkgtools/pkglint/files/mklines.go index 9fedaf309c3..0bbb14ed785 100644 --- a/pkgtools/pkglint/files/mklines.go +++ b/pkgtools/pkglint/files/mklines.go @@ -118,6 +118,7 @@ func (mklines *MkLines) checkAll() { substContext := NewSubstContext() var varalign VaralignBlock + vargroupsChecker := NewVargroupsChecker(mklines) isHacksMk := mklines.lines.BaseName == "hacks.mk" lineAction := func(mkline *MkLine) bool { @@ -128,6 +129,7 @@ func (mklines *MkLines) checkAll() { ck := MkLineChecker{mklines, mkline} ck.Check() + vargroupsChecker.Check(mkline) varalign.Process(mkline) mklines.Tools.ParseToolLine(mklines, mkline, false, false) @@ -163,6 +165,7 @@ func (mklines *MkLines) checkAll() { atEnd := func(mkline *MkLine) { mklines.indentation.CheckFinish(mklines.lines.Filename) + vargroupsChecker.Finish(mkline) } if trace.Tracing { diff --git a/pkgtools/pkglint/files/mkparser.go b/pkgtools/pkglint/files/mkparser.go index daf0ac6f41a..9c3032d2d3c 100644 --- a/pkgtools/pkglint/files/mkparser.go +++ b/pkgtools/pkglint/files/mkparser.go @@ -458,6 +458,10 @@ func (p *MkParser) mkCondAnd() *MkCond { return &MkCond{And: atoms} } +// mkCondLiteralChars contains the characters that may be used outside +// quotes in a comparison condition such as ${PKGPATH} == category/package. +var mkCondLiteralChars = textproc.NewByteSet("+---./0-9A-Z_a-z") + func (p *MkParser) mkCondCompare() *MkCond { if trace.Tracing { defer trace.Call1(p.Rest())() @@ -522,7 +526,7 @@ func (p *MkParser) mkCondCompare() *MkCond { return &MkCond{Compare: &MkCondCompare{*lhs, op, *rhs}} } - if str := lexer.NextBytesSet(textproc.AlnumU); str != "" { + if str := lexer.NextBytesSet(mkCondLiteralChars); str != "" { return &MkCond{Compare: &MkCondCompare{*lhs, op, MkCondTerm{Str: str}}} } } diff --git a/pkgtools/pkglint/files/mkparser_test.go b/pkgtools/pkglint/files/mkparser_test.go index 6dbb5e65100..0c5f60a713f 100644 --- a/pkgtools/pkglint/files/mkparser_test.go +++ b/pkgtools/pkglint/files/mkparser_test.go @@ -548,7 +548,7 @@ func (s *Suite) Test_MkParser_MkCond(c *check.C) { testRest := func(input string, expectedTree *MkCond, expectedRest string) { // As of July 2019 p.MkCond does not emit warnings; - // this is left to MkLineChecker) checkDirectiveCond. + // this is left to MkLineChecker.checkDirectiveCond. line := t.NewLine("filename.mk", 1, ".if "+input) p := NewMkParser(line, input) actualTree := p.MkCond() @@ -786,6 +786,25 @@ func (s *Suite) Test_MkParser_MkCond(c *check.C) { "empty{USE_CROSS_COMPILE:M[yY][eE][sS]}") } +func (s *Suite) Test_MkParser_mkCondCompare(c *check.C) { + t := s.Init(c) + + mkline := t.NewMkLine("Makefile", 123, ".if ${PKGPATH} == category/pack.age-3+") + p := NewMkParser(mkline.Line, mkline.Args()) + cond := p.MkCond() + + t.CheckEquals(p.Rest(), "") + t.CheckDeepEquals( + cond, + &MkCond{ + Compare: &MkCondCompare{ + Left: MkCondTerm{Var: NewMkVarUse("PKGPATH")}, + Op: "==", + Right: MkCondTerm{Str: "category/pack.age-3+"}}}) + + t.CheckOutputEmpty() +} + func (s *Suite) Test_MkParser_Varname(c *check.C) { t := s.Init(c) diff --git a/pkgtools/pkglint/files/package.go b/pkgtools/pkglint/files/package.go index 9270846ed81..fc18ca9f8a5 100644 --- a/pkgtools/pkglint/files/package.go +++ b/pkgtools/pkglint/files/package.go @@ -695,7 +695,10 @@ func (pkg *Package) checkfilePackageMakefile(filename string, mklines *MkLines, mklines.Tools = allLines.Tools // TODO: also copy the other collected data mklines.Check() - if false && pkg.EffectivePkgname != "" && pkg.Includes("../../lang/python/extension.mk") { + // This check is experimental because it's not yet clear how to + // classify the various Python packages and whether all Python + // packages really need the prefix. + if G.Experimental && pkg.EffectivePkgname != "" && pkg.Includes("../../lang/python/extension.mk") { pkg.EffectivePkgnameLine.Warnf("The PKGNAME of Python extensions should start with ${PYPKGPREFIX}.") } diff --git a/pkgtools/pkglint/files/package_test.go b/pkgtools/pkglint/files/package_test.go index 38b9e25fd16..199ada23d93 100644 --- a/pkgtools/pkglint/files/package_test.go +++ b/pkgtools/pkglint/files/package_test.go @@ -719,6 +719,7 @@ func (s *Suite) Test_Package_determineEffectivePkgVars__ineffective_C_modifier(c func (s *Suite) Test_Package_determineEffectivePkgVars__Python_prefix(c *check.C) { t := s.Init(c) + G.Experimental = true t.SetUpPackage("category/package", "PKGNAME=\tpackage-2.0", ".include \"../../lang/python/extension.mk\"") @@ -728,16 +729,14 @@ func (s *Suite) Test_Package_determineEffectivePkgVars__Python_prefix(c *check.C t.Main("-Wall", "category/package") t.CheckOutputLines( - "Looks fine.") - // TODO: Wait for joerg's answer before enabling this check. - //t.CheckOutputLines( - // "WARN: ~/category/package/Makefile:4: The PKGNAME of Python extensions should start with ${PYPKGPREFIX}.", - // "1 warning found.") + "WARN: ~/category/package/Makefile:4: The PKGNAME of Python extensions should start with ${PYPKGPREFIX}.", + "1 warning found.") } func (s *Suite) Test_Package_determineEffectivePkgVars__Python_prefix_PKGNAME_variable(c *check.C) { t := s.Init(c) + G.Experimental = true t.SetUpPackage("category/package", "PKGNAME=\t${VAR}-package-2.0", ".include \"../../lang/python/extension.mk\"") @@ -765,6 +764,7 @@ func (s *Suite) Test_Package_determineEffectivePkgVars__Python_prefix_PKGNAME_va func (s *Suite) Test_Package_determineEffectivePkgVars__Python_prefix_late(c *check.C) { t := s.Init(c) + G.Experimental = true t.SetUpPackage("category/package", "PKGNAME=\tpackage-2.0", ".include \"common.mk\"") @@ -776,13 +776,10 @@ func (s *Suite) Test_Package_determineEffectivePkgVars__Python_prefix_late(c *ch t.Main("-Wall", "category/package") - // TODO: Wait for joerg's answer before enabling this check. t.CheckOutputLines( - "Looks fine.") - //t.CheckOutputLines( - // "WARN: ~/category/package/Makefile:4: "+ - // "The PKGNAME of Python extensions should start with ${PYPKGPREFIX}.", - // "1 warning found.") + "WARN: ~/category/package/Makefile:4: "+ + "The PKGNAME of Python extensions should start with ${PYPKGPREFIX}.", + "1 warning found.") } func (s *Suite) Test_Package_checkPossibleDowngrade(c *check.C) { @@ -2944,3 +2941,23 @@ func (s *Suite) Test_Package_Includes(c *check.C) { // Indentation.IsConditional for the current implementation. t.CheckEquals(pkg.conditionalIncludes["never.mk"], (*MkLine)(nil)) } + +func (s *Suite) Test_Package__case_insensitive(c *check.C) { + t := s.Init(c) + + t.SetUpPkgsrc() + t.SetUpPackage("net/p5-Net-DNS") + t.SetUpPackage("category/package", + "DEPENDS+=\tp5-Net-DNS>=0:../../net/p5-net-dns") + t.FinishSetUp() + + // this test is only interesting on a case-insensitive filesystem + if !fileExists(t.File("mk/BSD.PKG.MK")) { + return + } + + G.Check(t.File("category/package")) + + // FIXME: On a case-sensitive filesystem, p5-net-dns would not be found. + t.CheckOutputEmpty() +} diff --git a/pkgtools/pkglint/files/pkglint.go b/pkgtools/pkglint/files/pkglint.go index f1a016521af..646c4d785d4 100644 --- a/pkgtools/pkglint/files/pkglint.go +++ b/pkgtools/pkglint/files/pkglint.go @@ -30,6 +30,7 @@ type Pkglint struct { Wip bool // Is the currently checked file or package from pkgsrc-wip? Infrastructure bool // Is the currently checked file from the pkgsrc infrastructure? Testing bool // Is pkglint in self-testing mode (only during development)? + Experimental bool // For experimental features, only enabled individually in tests Username string // For checking against OWNER and MAINTAINER cvsEntriesDir string // Cached to avoid I/O diff --git a/pkgtools/pkglint/files/pkglint_test.go b/pkgtools/pkglint/files/pkglint_test.go index adf8c9ecaa0..15d0f625cef 100644 --- a/pkgtools/pkglint/files/pkglint_test.go +++ b/pkgtools/pkglint/files/pkglint_test.go @@ -726,7 +726,7 @@ func (s *Suite) Test_Pkglint_Tool__lookup_by_name_fallback(c *check.C) { loadTimeTool, loadTimeUsable := G.Tool(mklines, "tool", LoadTime) runTimeTool, runTimeUsable := G.Tool(mklines, "tool", RunTime) - // The tool is returned even though it may not be used at the moment. + // The tool is returned even though it cannot be used at the moment. // The calling code must explicitly check for usability. t.CheckEquals(loadTimeTool.String(), "tool:::Nowhere") diff --git a/pkgtools/pkglint/files/pkgsrc.go b/pkgtools/pkglint/files/pkgsrc.go index c94deeaf22c..0aa6fbbb672 100644 --- a/pkgtools/pkglint/files/pkgsrc.go +++ b/pkgtools/pkglint/files/pkgsrc.go @@ -819,6 +819,15 @@ func (src *Pkgsrc) LoadMk(filename string, options LoadOptions) *MkLines { return LoadMk(src.File(filename), options) } +func (src *Pkgsrc) LoadMkInfra(filename string, options LoadOptions) *MkLines { + if G.Testing { + // During testing, the infrastructure files don't have to exist. + // They are often emulated by setting their data structures manually. + options &^= MustSucceed + } + return src.LoadMk(filename, options) +} + // ReadDir lists the files and subdirectories from the given directory // (relative to the pkgsrc root), filtering out any ignored files (CVS/*) // and empty directories. diff --git a/pkgtools/pkglint/files/shell.go b/pkgtools/pkglint/files/shell.go index b2210752db6..19da783e293 100644 --- a/pkgtools/pkglint/files/shell.go +++ b/pkgtools/pkglint/files/shell.go @@ -422,6 +422,7 @@ func (ck *ShellLineChecker) checkHiddenAndSuppress(hiddenAndSuppress, rest strin "${DO_NADA}", "${ECHO}", "${ECHO_MSG}", "${ECHO_N}", "${ERROR_CAT}", "${ERROR_MSG}", "${FAIL_MSG}", + "${INFO_MSG}", "${PHASE_MSG}", "${PRINTF}", "${SHCOMMENT}", "${STEP_MSG}", "${WARNING_CAT}", "${WARNING_MSG}": diff --git a/pkgtools/pkglint/files/tools.go b/pkgtools/pkglint/files/tools.go index e54a2771a85..6eb01c21bbc 100644 --- a/pkgtools/pkglint/files/tools.go +++ b/pkgtools/pkglint/files/tools.go @@ -415,6 +415,8 @@ func (tr *Tools) adjustValidity(tool *Tool) { } } +func (tr *Tools) ExistsVar(varname string) bool { return tr.byVarname[varname] != nil } + type Validity uint8 const ( diff --git a/pkgtools/pkglint/files/util.go b/pkgtools/pkglint/files/util.go index 45f89524390..3e9c20648f6 100644 --- a/pkgtools/pkglint/files/util.go +++ b/pkgtools/pkglint/files/util.go @@ -149,6 +149,25 @@ func keysJoined(m map[string]bool) string { return strings.Join(keys, " ") } +func copyStringMkLine(m map[string]*MkLine) map[string]*MkLine { + c := make(map[string]*MkLine, len(m)) + for k, v := range m { + c[k] = v + } + return c +} + +func forEachStringMkLine(m map[string]*MkLine, action func(s string, mkline *MkLine)) { + var keys []string + for key := range m { + keys = append(keys, key) + } + sort.Strings(keys) + for _, key := range keys { + action(key, m[key]) + } +} + func imax(a, b int) int { if a > b { return a @@ -245,7 +264,7 @@ func getSubdirs(filename string) []string { func isIgnoredFilename(filename string) bool { switch filename { - case ".", "..", "CVS", ".svn", ".git", ".hg": + case ".", "..", "CVS", ".svn", ".git", ".hg", ".idea": return true } return false @@ -312,16 +331,18 @@ type CvsEntry struct { // Returns the number of columns that a string occupies when printed with // a tabulator size of 8. -func tabWidth(s string) int { - length := 0 +func tabWidth(s string) int { return tabWidthAppend(0, s) } + +func tabWidthAppend(width int, s string) int { for _, r := range s { + assert(r != '\n') if r == '\t' { - length = length&-8 + 8 + width = width&-8 + 8 } else { - length++ + width++ } } - return length + return width } func detab(s string) string { @@ -1331,3 +1352,8 @@ func (s *StringSet) AddAll(elements []string) { s.Add(element) } } + +func (s *StringSet) Contains(needle string) bool { + _, found := s.seen[needle] + return found +} diff --git a/pkgtools/pkglint/files/util_test.go b/pkgtools/pkglint/files/util_test.go index fc3937baceb..a3f08f7b741 100644 --- a/pkgtools/pkglint/files/util_test.go +++ b/pkgtools/pkglint/files/util_test.go @@ -105,6 +105,15 @@ func (s *Suite) Test_tabWidth(c *check.C) { t.CheckEquals(tabWidth("12345678\t"), 16) } +// Since tabWidthAppend is used with logical lines (Line.Text) as well as with +// raw lines (RawLine.textnl or RawLine.orignl), and since the width only +// makes sense for a single line, better panic. +func (s *Suite) Test_tabWidthAppend__panic(c *check.C) { + t := s.Init(c) + + t.ExpectAssert(func() { tabWidthAppend(0, "\n") }) +} + func (s *Suite) Test_cleanpath(c *check.C) { t := s.Init(c) diff --git a/pkgtools/pkglint/files/varalignblock.go b/pkgtools/pkglint/files/varalignblock.go index 2656ab43c65..ce020b30253 100644 --- a/pkgtools/pkglint/files/varalignblock.go +++ b/pkgtools/pkglint/files/varalignblock.go @@ -2,6 +2,7 @@ package pkglint import ( "netbsd.org/pkglint/textproc" + "sort" "strings" ) @@ -16,29 +17,31 @@ import ( // between HOMEPAGE and DISTFILES. // // Continuation lines are also aligned to the single-line assignments. -// There are two types of continuation lines. The first type is an -// empty multiline: +// There are two types of continuation lines. The first type has just +// the continuation backslash in the first line: // -// MULTI_EMPTY_LINE= \ +// MULTI_LINE= \ // The value starts in the second line. // // The backslash in the first line is usually aligned to the other variables -// in the same paragraph. If the variable name is so long that it is an -// outlier, it may be indented with a single space, just like a single-line -// variable. In multi-line shell commands or AWK programs, the backslash is +// in the same paragraph. If the variable name is longer than the indentation +// of the paragraph, it may be indented with a single space. +// In multi-line shell commands or AWK programs, the backslash is // often indented to column 73, as are the backslashes from the follow-up // lines, to act as a visual guideline. // -// Since this type is often used for URLs or other long values, the first -// follow-up line may be indented with a single tab, even if the other +// The indentation of the first value of the variable determines the minimum +// indentation for the remaining continuation lines. To allow long variable +// values to be indented as little as possible, the follow-up lines only need +// to be indented by a single tab, even if the other // variables in the paragraph are aligned further to the right. If the // indentation is not a single tab, it must match the indentation of the // other lines in the paragraph. // -// INITIAL_LINE= The value starts in the first line \ +// MULTI_LINE= The value starts in the first line \ // and continues in the second line. // -// In lists or plain text, like in the INITIAL_LINE above, all values are +// In lists or plain text, like in the example above, all values are // aligned in the same column. Some variables also contain code, and in // these variables, the line containing the first word defines how deep // the follow-up lines must be indented at least. @@ -55,31 +58,17 @@ import ( // especially true for CONFIGURE_ENV, since the environment variables are // typically uppercase as well. // -// TODO: An initial line has this form: -// comment? varname+op space? value? space? comment? space? backslash? +// For the purpose of aligning the variable assignments, each raw line is +// split into several parts, as described by the varalignParts type. // -// TODO: A follow-up line has the form: -// comment? space? value? space? comment? space? backslash? -// -// TODO: The alignment checks are performed on the raw lines instead of -// the logical lines, since this check is about the visual appearance -// as opposed to the meaning of the variable assignment. +// The alignment checks are performed on the raw lines instead of +// the logical lines, since this check is about the visual appearance +// as opposed to the meaning of the variable assignment. // // FIXME: Implement each requirement from the above documentation. type VaralignBlock struct { infos []*varalignLine skip bool - - // 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 bool - // 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 int } type varalignLine struct { @@ -98,18 +87,7 @@ type varalignLine struct { // of the first value found. multiEmpty bool - parts varalignSplitResult -} - -type varalignSplitResult struct { - leadingComment string // either the # or some rarely used U+0020 spaces - varnameOp string // empty iff it is a follow-up line - spaceBeforeValue string // for follow-up lines, this is the indentation - value string - spaceAfterValue string - trailingComment string - spaceAfterComment string - continuation string + varalignParts } func (va *VaralignBlock) Process(mkline *MkLine) { @@ -154,9 +132,10 @@ func (va *VaralignBlock) processVarassign(mkline *MkLine) { follow := false for i, raw := range mkline.raw { - info := varalignLine{mkline, i, follow, va.split(raw.textnl, i == 0)} + parts := NewVaralignSplitter().split(strings.TrimSuffix(raw.textnl, "\n"), i == 0) + info := varalignLine{mkline, i, follow, parts} - if i == 0 && info.parts.value == "" && info.continuation() { + if i == 0 && info.isEmptyContinuation() { follow = true info.multiEmpty = true } @@ -165,135 +144,84 @@ func (va *VaralignBlock) processVarassign(mkline *MkLine) { } } -func (*VaralignBlock) split(textnl string, initial bool) varalignSplitResult { +func (va *VaralignBlock) Finish() { + infos := va.infos + skip := va.skip + *va = VaralignBlock{} // overwrites infos and skip - // See MkLineParser.unescapeComment for very similar code. + if len(infos) == 0 || skip { + return + } - p := NewMkParser(nil, textnl) - lexer := p.lexer + if trace.Tracing { + defer trace.Call(infos[0].mkline.Line)() + } - parseLeadingComment := func() string { - if hasPrefix(lexer.Rest(), "# ") { - return "" - } + newWidth := va.optimalWidth(infos) + rightMargin := 0 - mark := lexer.Mark() + // 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 - if !lexer.SkipByte('#') && initial && lexer.SkipByte(' ') { - lexer.SkipHspace() + for i, info := range infos { + if info.rawIndex == 0 { + indentDiffSet = false + indentDiff = 0 + restIndex := i + condInt(info.value != "", 0, 1) + rightMargin = va.rightMargin(infos[restIndex:]) } - return lexer.Since(mark) - } + va.checkContinuationIndentation(info, newWidth, rightMargin) - parseVarnameOp := func() string { - if !initial { - return "" + if newWidth > 0 || info.rawIndex > 0 { + va.realign(info, newWidth, &indentDiffSet, &indentDiff) } - - mark := lexer.Mark() - _ = p.Varname() - lexer.SkipHspace() - ok, _ := p.Op() - assert(ok) - return lexer.Since(mark) } +} - parseValue := func() (string, string) { - mark := lexer.Mark() - - for !lexer.EOF() && - lexer.PeekByte() != '#' && - lexer.PeekByte() != '\n' && - !hasPrefix(lexer.Rest(), "\\\n") { - - if lexer.NextBytesSet(unescapeMkCommentSafeChars) != "" || - lexer.SkipString("[#") || - lexer.SkipByte('[') { - continue - } - - if len(lexer.Rest()) < 2 { - break +// rightMargin calculates the column in which the continuation backslashes +// should be placed. +func (*VaralignBlock) rightMargin(infos []*varalignLine) int { + var columns []int + for _, info := range infos { + if info.isContinuation() { + space := info.spaceBeforeContinuation() + if space != "" && space != " " { + columns = append(columns, info.continuationColumn()) } - - assert(lexer.SkipByte('\\')) - lexer.Skip(1) } - - valueSpace := lexer.Since(mark) - value := rtrimHspace(valueSpace) - space := valueSpace[len(value):] - return value, space } - parseComment := func() (string, string, string) { - rest := lexer.Rest() - - newline := len(rest) - for newline > 0 && rest[newline-1] == '\n' { - newline-- - } - - backslash := newline - for backslash > 0 && rest[backslash-1] == '\\' { - backslash-- - } + sort.Ints(columns) - if (newline-backslash)%2 == 1 { - continuation := rest[backslash:] - commentSpace := rest[:backslash] - comment := rtrimHspace(commentSpace) - space := commentSpace[len(comment):] - return comment, space, continuation + for i := len(columns) - 2; i >= 0; i-- { + if columns[i] == columns[i+1] { + return columns[i] } - - return rest[:newline], "", rest[newline:] - } - - leadingComment := parseLeadingComment() - varnameOp := parseVarnameOp() - spaceBeforeValue := lexer.NextHspace() - value, spaceAfterValue := parseValue() - trailingComment, spaceAfterComment, continuation := parseComment() - - return varalignSplitResult{ - leadingComment, - varnameOp, - spaceBeforeValue, - value, - spaceAfterValue, - trailingComment, - spaceAfterComment, - continuation, } -} -func (va *VaralignBlock) Finish() { - infos := va.infos - skip := va.skip - *va = VaralignBlock{} // overwrites infos and skip - - if len(infos) == 0 || skip { - return - } - - if trace.Tracing { - defer trace.Call(infos[0].mkline.Line)() + if len(columns) <= 1 { + return 0 } - newWidth := va.optimalWidth(infos) - + var min int for _, info := range infos { - if info.rawIndex == 0 { - va.indentDiffSet = false - va.indentDiff = 0 - } - - if newWidth > 0 || info.rawIndex > 0 { - va.realign(info, newWidth) + if info.isContinuation() { + mainWidth := tabWidth(info.beforeContinuation()) + if mainWidth > min { + min = mainWidth + } } } + return (min & -8) + 8 } // optimalWidth computes the desired screen width for the variable assignment @@ -303,47 +231,39 @@ func (va *VaralignBlock) Finish() { // 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(infos []*varalignLine) int { - longest := 0 // The longest seen varnameOpWidth - var longestLine *MkLine - secondLongest := 0 // The second-longest seen varnameOpWidth + var widths mklineInts for _, info := range infos { - if info.multiEmpty || info.rawIndex > 0 { - continue - } - - width := info.varnameOpWidth() - if width >= longest { - secondLongest = longest - longest = width - longestLine = info.mkline - } else if width > secondLongest { - secondLongest = width + if !info.multiEmpty && info.rawIndex == 0 { + widths.append(info.mkline, info.varnameOpWidth()) } } + widths.sortDesc() + + longest := widths.opt(0) + longestLine := widths.optLine(0) + secondLongest := widths.opt(1) haveOutlier := secondLongest != 0 && - longest/8 >= secondLongest/8+2 && - !longestLine.IsMultiline() + secondLongest/8+1 < longest/8 && + !longestLine.IsMultiline() // TODO: may be too imprecise // Minimum required width of varnameOp, without the trailing whitespace. minVarnameOpWidth := condInt(haveOutlier, secondLongest, longest) outlier := condInt(haveOutlier, longest, 0) // Widths of the current indentation (including whitespace) - minTotalWidth := 0 - maxTotalWidth := 0 + var spaceWidths mklineInts for _, info := range infos { if info.multiEmpty || info.rawIndex > 0 || (outlier > 0 && info.varnameOpWidth() == outlier) { continue } - - width := info.varnameOpSpaceWidth() - if minTotalWidth == 0 || width < minTotalWidth { - minTotalWidth = width - } - maxTotalWidth = imax(maxTotalWidth, width) + spaceWidths.append(info.mkline, info.varnameOpSpaceWidth()) } + spaceWidths.sortDesc() + + minTotalWidth := spaceWidths.min() + maxTotalWidth := spaceWidths.max() if trace.Tracing { trace.Stepf("Indentation including whitespace is between %d and %d.", @@ -367,26 +287,58 @@ func (*VaralignBlock) optimalWidth(infos []*varalignLine) int { return (minVarnameOpWidth & -8) + 8 } -func (va *VaralignBlock) realign(info *varalignLine, newWidth int) { +func (va *VaralignBlock) checkContinuationIndentation(info *varalignLine, newWidth int, rightMargin int) { + if !info.isContinuation() { + return + } + + oldSpace := info.spaceBeforeContinuation() + if oldSpace == " " || oldSpace == "\t" { + return + } + + column := info.continuationColumn() + if column == 72 || column == rightMargin || column <= newWidth { + return + } + + newSpace := " " + fix := info.mkline.Autofix() + if oldSpace == "" || rightMargin == 0 || tabWidth(info.beforeContinuation()) >= rightMargin { + fix.Notef("The continuation backslash should be preceded by a single space or tab.") + } else { + newSpace = alignmentAfter(info.beforeContinuation(), rightMargin) + fix.Notef( + "The continuation backslash should be preceded by a single space or tab, "+ + "or be in column %d, not %d.", + rightMargin+1, column+1) + } + fix.ReplaceAt(info.rawIndex, info.continuationIndex()-len(oldSpace), oldSpace, newSpace) + fix.Apply() +} + +func (va *VaralignBlock) realign(info *varalignLine, newWidth int, indentDiffSet *bool, indentDiff *int) { if info.multiEmpty { if info.rawIndex == 0 { va.realignMultiEmptyInitial(info, newWidth) } else { - va.realignMultiEmptyFollow(info, newWidth) + va.realignMultiEmptyFollow(info, newWidth, indentDiffSet, indentDiff) } - } else if info.rawIndex == 0 && info.continuation() { - va.realignMultiInitial(info, newWidth) + } else if info.rawIndex == 0 && info.isContinuation() { + va.realignMultiInitial(info, newWidth, indentDiffSet, indentDiff) } else if info.rawIndex > 0 { - va.realignMultiFollow(info, newWidth) + assert(*indentDiffSet) + va.realignMultiFollow(info, newWidth, *indentDiff) } else { + assert(!*indentDiffSet) va.realignSingle(info, newWidth) } } func (*VaralignBlock) realignMultiEmptyInitial(info *varalignLine, newWidth int) { - leadingComment := info.parts.leadingComment - varnameOp := info.parts.varnameOp - oldSpace := info.parts.spaceBeforeValue + leadingComment := info.leadingComment + varnameOp := info.varnameOp + oldSpace := info.spaceBeforeValue // Indent the outlier and any other lines that stick out // with a space instead of a tab to keep the line short. @@ -399,20 +351,16 @@ func (*VaralignBlock) realignMultiEmptyInitial(info *varalignLine, newWidth int) return } - if newSpace == " " && oldSpace != "" && oldSpace == strings.Repeat("\t", len(oldSpace)) { - return + if newSpace == " " { + return // This case is handled by checkContinuationIndentation. } hasSpace := strings.IndexByte(oldSpace, ' ') != -1 oldColumn := info.varnameOpSpaceWidth() column := tabWidth(leadingComment + varnameOp + newSpace) - // TODO: explicitly mention "single space", "tabs to the newWidth", "tabs to column 72" - fix := info.mkline.Autofix() - if newSpace == " " { - fix.Notef("This outlier variable value should be aligned with a single space.") - } else if hasSpace && column != oldColumn { + if hasSpace && column != oldColumn { fix.Notef("This variable value should be aligned with tabs, not spaces, to column %d.", column+1) } else if column != oldColumn { fix.Notef("This variable value should be aligned to column %d.", column+1) @@ -423,26 +371,26 @@ func (*VaralignBlock) realignMultiEmptyInitial(info *varalignLine, newWidth int) fix.Apply() } -func (va *VaralignBlock) realignMultiEmptyFollow(info *varalignLine, newWidth int) { - oldSpace := info.parts.spaceBeforeValue +func (va *VaralignBlock) realignMultiEmptyFollow(info *varalignLine, newWidth int, indentDiffSet *bool, indentDiff *int) { + oldSpace := info.spaceBeforeValue oldWidth := tabWidth(oldSpace) - if !va.indentDiffSet { - va.indentDiffSet = true - va.indentDiff = condInt(newWidth != 0, newWidth-oldWidth, 0) - if va.indentDiff > 0 && !info.commentedOut() { - va.indentDiff = 0 + if !*indentDiffSet { + *indentDiffSet = true + *indentDiff = condInt(newWidth != 0, newWidth-oldWidth, 0) + if *indentDiff > 0 && !info.commentedOut() { + *indentDiff = 0 } } - newSpace := indent(imax(oldWidth+va.indentDiff, 8)) + newSpace := indent(imax(oldWidth+*indentDiff, 8)) if newSpace == oldSpace { return } // Below a continuation marker, there may be a completely empty line. // This is confusing to the human readers, but technically allowed. - if info.parts.value == "" && info.parts.trailingComment == "" && !info.continuation() { + if info.varalignParts.isEmpty() { return } @@ -452,14 +400,14 @@ func (va *VaralignBlock) realignMultiEmptyFollow(info *varalignLine, newWidth in fix.Apply() } -func (va *VaralignBlock) realignMultiInitial(info *varalignLine, newWidth int) { - leadingComment := info.parts.leadingComment - varnameOp := info.parts.varnameOp - oldSpace := info.parts.spaceBeforeValue +func (va *VaralignBlock) realignMultiInitial(info *varalignLine, newWidth int, indentDiffSet *bool, indentDiff *int) { + leadingComment := info.leadingComment + varnameOp := info.varnameOp + oldSpace := info.spaceBeforeValue - va.indentDiffSet = true + *indentDiffSet = true oldWidth := info.varnameOpSpaceWidth() - va.indentDiff = newWidth - oldWidth + *indentDiff = newWidth - oldWidth newSpace := alignmentAfter(leadingComment+varnameOp, newWidth) if newSpace == oldSpace { @@ -481,15 +429,13 @@ func (va *VaralignBlock) realignMultiInitial(info *varalignLine, newWidth int) { fix.Apply() } -func (va *VaralignBlock) realignMultiFollow(info *varalignLine, newWidth int) { - assert(va.indentDiffSet) - - oldSpace := info.parts.spaceBeforeValue - newSpace := indent(tabWidth(oldSpace) + va.indentDiff) +func (va *VaralignBlock) realignMultiFollow(info *varalignLine, newWidth int, indentDiff int) { + oldSpace := info.spaceBeforeValue + newSpace := indent(tabWidth(oldSpace) + indentDiff) if tabWidth(newSpace) < newWidth { newSpace = indent(newWidth) } - if newSpace == oldSpace || oldSpace == "\t" { + if newSpace == oldSpace || (oldSpace == "\t" && info.widthAlignedAt(newWidth) > 72) { return } @@ -500,11 +446,9 @@ func (va *VaralignBlock) realignMultiFollow(info *varalignLine, newWidth int) { } func (va *VaralignBlock) realignSingle(info *varalignLine, newWidth int) { - assert(!va.indentDiffSet) - - leadingComment := info.parts.leadingComment - varnameOp := info.parts.varnameOp - oldSpace := info.parts.spaceBeforeValue + leadingComment := info.leadingComment + varnameOp := info.varnameOp + oldSpace := info.spaceBeforeValue newSpace := "" for tabWidth(leadingComment+varnameOp+newSpace) < newWidth { @@ -562,41 +506,196 @@ func (va *VaralignBlock) explainWrongColumn(fix *Autofix) { "and directives like .if or .for, it is not checked at all.") } -func (l *varalignLine) varnameOpWidth() int { - return tabWidth(l.parts.leadingComment + l.parts.varnameOp) +// VaralignSplitter parses the text of a raw line into those parts that +// are relevant for aligning the variable values, and the backslash +// continuation markers. +// +// See MkLineParser.unescapeComment for very similar code. +type VaralignSplitter struct { } -func (l *varalignLine) varnameOpSpaceWidth() int { - return tabWidth(l.parts.leadingComment + l.parts.varnameOp + l.parts.spaceBeforeValue) +func NewVaralignSplitter() VaralignSplitter { + return VaralignSplitter{} +} + +func (s VaralignSplitter) split(rawText string, initial bool) varalignParts { + assert(!hasSuffix(rawText, "\n")) + parser := NewMkParser(nil, rawText) + lexer := parser.lexer + + leadingComment := s.parseLeadingComment(lexer, initial) + varnameOp, spaceBeforeValue := s.parseVarnameOp(parser, initial) + value, spaceAfterValue := s.parseValue(lexer) + trailingComment, spaceAfterComment, continuation := s.parseComment(lexer.Rest()) + + return varalignParts{ + leadingComment, + varnameOp, + spaceBeforeValue, + value, + spaceAfterValue, + trailingComment, + spaceAfterComment, + continuation, + } +} + +func (VaralignSplitter) parseLeadingComment(lexer *textproc.Lexer, initial bool) string { + if hasPrefix(lexer.Rest(), "# ") { + return "" + } + + comment := lexer.NextString("#") + if comment != "" { + return comment + } + + mark := lexer.Mark() + if initial && lexer.SkipByte(' ') { + lexer.SkipHspace() + } + return lexer.Since(mark) +} + +func (VaralignSplitter) parseVarnameOp(parser *MkParser, initial bool) (string, string) { + lexer := parser.lexer + if !initial { + return "", lexer.NextHspace() + } + + mark := lexer.Mark() + _ = parser.Varname() + lexer.SkipHspace() + ok, _ := parser.Op() + assert(ok) + return lexer.Since(mark), lexer.NextHspace() +} + +func (VaralignSplitter) parseValue(lexer *textproc.Lexer) (string, string) { + mark := lexer.Mark() + + for !lexer.EOF() && lexer.PeekByte() != '#' && lexer.Rest() != "\\" { + + if lexer.NextBytesSet(unescapeMkCommentSafeChars) != "" || + lexer.SkipString("[#") || + lexer.SkipByte('[') { + continue + } + + assert(lexer.SkipByte('\\')) + lexer.Skip(1) + } + + valueSpace := lexer.Since(mark) + value := rtrimHspace(valueSpace) + space := valueSpace[len(value):] + return value, space +} + +func (VaralignSplitter) parseComment(rest string) (string, string, string) { + end := len(rest) + + backslash := end + for backslash > 0 && rest[backslash-1] == '\\' { + backslash-- + } + + if (end-backslash)&1 == 0 { // see https://github.com/golang/go/issues/34166 + return rest[:end], "", "" + } + + continuation := rest[backslash:] + commentSpace := rest[:backslash] + comment := rtrimHspace(commentSpace) + space := commentSpace[len(comment):] + return comment, space, continuation +} + +type varalignParts struct { + leadingComment string // either the # or some rarely used U+0020 spaces + varnameOp string // empty iff it is a follow-up line + spaceBeforeValue string // for follow-up lines, this is the indentation + value string + spaceAfterValue string // only set if there is a value + trailingComment string + spaceAfterComment string // only set if there is a trailing comment + continuation string // either a single backslash or empty +} + +// continuation returns whether this line ends with a backslash. +func (p *varalignParts) isContinuation() bool { + return p.continuation != "" +} + +func (p *varalignParts) isEmptyContinuation() bool { + return p.value == "" && p.trailingComment == "" && p.isContinuation() +} + +func (p *varalignParts) isEmpty() bool { + return p.value == "" && p.trailingComment == "" && !p.isContinuation() +} + +func (p *varalignParts) varnameOpWidth() int { + return tabWidth(p.leadingComment + p.varnameOp) +} + +func (p *varalignParts) varnameOpSpaceWidth() int { + return tabWidth(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 (l *varalignLine) spaceBeforeValueIndex() int { - return len(l.parts.leadingComment) + len(l.parts.varnameOp) +func (p *varalignParts) spaceBeforeValueIndex() int { + return len(p.leadingComment) + len(p.varnameOp) } -// continuation returns whether this line ends with a backslash. -func (l *varalignLine) continuation() bool { - return hasPrefix(l.parts.continuation, "\\") +func (p *varalignParts) spaceBeforeContinuation() string { + if p.trailingComment == "" { + if p.value == "" { + return p.spaceBeforeValue + } + return p.spaceAfterValue + } + return p.spaceAfterComment +} + +func (p *varalignParts) beforeContinuation() string { + return rtrimHspace(p.leadingComment + + p.varnameOp + p.spaceBeforeValue + + p.value + p.spaceAfterValue + + p.trailingComment + p.spaceAfterComment) } -func (l *varalignLine) commentedOut() bool { - return hasPrefix(l.parts.leadingComment, "#") +func (p *varalignParts) continuationColumn() int { + return tabWidth(p.leadingComment + + p.varnameOp + p.spaceBeforeValue + + p.value + p.spaceAfterValue + + p.trailingComment + p.spaceAfterComment) +} + +func (p *varalignParts) continuationIndex() int { + return len(p.leadingComment) + + len(p.varnameOp) + len(p.spaceBeforeValue) + + len(p.value) + len(p.spaceAfterValue) + + len(p.trailingComment) + len(p.spaceAfterComment) +} + +func (p *varalignParts) commentedOut() bool { + return hasPrefix(p.leadingComment, "#") } // canonicalInitial 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 (l *varalignLine) canonicalInitial(width int) bool { - space := l.parts.spaceBeforeValue +func (p *varalignParts) canonicalInitial(width int) bool { + space := p.spaceBeforeValue if space == "" { return false } - if space == " " && l.varnameOpSpaceWidth() > width { + if space == " " && p.varnameOpSpaceWidth() > width { return true } @@ -605,8 +704,8 @@ func (l *varalignLine) canonicalInitial(width int) bool { // canonicalFollow returns whether the space before the value has its // canonical form, which is at least one tab, followed by up to 7 spaces. -func (l *varalignLine) canonicalFollow() bool { - lexer := textproc.NewLexer(l.parts.spaceBeforeValue) +func (p *varalignParts) canonicalFollow() bool { + lexer := textproc.NewLexer(p.spaceBeforeValue) tabs := 0 for lexer.SkipByte('\t') { @@ -620,3 +719,46 @@ func (l *varalignLine) canonicalFollow() bool { return tabs >= 1 && spaces <= 7 } + +func (p *varalignParts) widthAlignedAt(valueAlign int) int { + return tabWidthAppend( + valueAlign, + p.value+p.spaceAfterValue+p.trailingComment+p.spaceAfterComment+p.continuation) +} + +type mklineInts struct { + slice []struct { + mkline *MkLine + value int + } +} + +func (mi mklineInts) sortDesc() { + less := func(i, j int) bool { return mi.slice[j].value < mi.slice[i].value } + sort.SliceStable(mi.slice, less) +} + +func (mi mklineInts) opt(index int) int { + if uint(index) < uint(len(mi.slice)) { + return mi.slice[index].value + } + return 0 +} + +func (mi mklineInts) optLine(index int) *MkLine { + if uint(index) < uint(len(mi.slice)) { + return mi.slice[index].mkline + } + return nil +} + +func (mi *mklineInts) append(mkline *MkLine, value int) { + mi.slice = append(mi.slice, struct { + mkline *MkLine + value int + }{mkline, value}) +} + +func (mi mklineInts) min() int { return mi.opt(0) } + +func (mi mklineInts) max() int { return mi.opt(len(mi.slice) - 1) } diff --git a/pkgtools/pkglint/files/varalignblock_test.go b/pkgtools/pkglint/files/varalignblock_test.go index 46c29942cc7..b0bbf0d0fb4 100644 --- a/pkgtools/pkglint/files/varalignblock_test.go +++ b/pkgtools/pkglint/files/varalignblock_test.go @@ -1,6 +1,10 @@ package pkglint -import "gopkg.in/check.v1" +import ( + "gopkg.in/check.v1" + "strconv" + "strings" +) // VaralignTester reduces the amount of test code for aligning variable // assignments in Makefiles. @@ -48,8 +52,10 @@ func (vt *VaralignTester) Fixed(lines ...string) { vt.fixed = lines } // Run is called after setting up the data and runs the varalign checks twice. // Once for getting the diagnostics and once for automatically fixing them. func (vt *VaralignTester) Run() { + vt.tester.Chdir(".") vt.run(false) vt.run(true) + vt.checkTestName() } func (vt *VaralignTester) run(autofix bool) { @@ -78,13 +84,19 @@ func (vt *VaralignTester) run(autofix bool) { infos := varalign.infos // since they are overwritten by Finish varalign.Finish() - var actual []string - for _, info := range infos { - minWidth := condStr(info.rawIndex == 0, sprintf("%02d", info.varnameOpWidth()), " ") - infoStr := sprintf("%s %02d", minWidth, info.varnameOpSpaceWidth()) - actual = append(actual, infoStr) + if len(vt.internals) > 0 { + var actual []string + for _, info := range infos { + var minWidth, curWidth, continuation string + minWidth = condStr(info.rawIndex == 0, sprintf("%02d", info.varnameOpWidth()), " ") + curWidth = sprintf(" %02d", info.varnameOpSpaceWidth()) + if info.isContinuation() { + continuation = sprintf(" %02d", info.continuationColumn()) + } + actual = append(actual, minWidth+curWidth+continuation) + } + t.CheckDeepEquals(actual, vt.internals) } - t.CheckDeepEquals(actual, vt.internals) if autofix { t.CheckOutput(vt.autofixes) @@ -96,37 +108,185 @@ func (vt *VaralignTester) run(autofix bool) { } } -func (s *Suite) Test_VaralignBlock__one_line_simple_none(c *check.C) { +func (vt *VaralignTester) checkTestName() { + testName := vt.tester.c.TestName() + if !matches(testName, `__(lead|var)\d*_`) { + return + } + + type descriptor struct { + name string + width int + } + + var actual []descriptor + width := 0 + describe := func(str string, descr string) { + width = tabWidthAppend(width, str) + actual = append(actual, descriptor{descr, width}) + } + describeHspace := func(hspace string) { + width = tabWidthAppend(width, hspace) + var descr string + switch { + case hspace == "": + descr = "none" + case hspace == "\t": + descr = "tab" + case hspace == " ": + descr = "space" + case matches(hspace, `^\t+$`): + descr = "tabs" + case matches(hspace, `^ +$`): + descr = "spaces" + case matches(hspace, `^\t+ {0,7}$`): + descr = "indent" + default: + descr = strings.Replace(strings.Replace(hspace, " ", "s", -1), "\t", "t", -1) + } + actual = append(actual, descriptor{descr, width}) + } + + vt.tester.NewMkLines("Makefile", vt.input...).ForEach(func(mkline *MkLine) { + if !mkline.IsVarassignMaybeCommented() { + return + } + + for i, input := range mkline.raw { + parts := NewVaralignSplitter().split(strings.TrimSuffix(input.orignl, "\n"), i == 0) + width = 0 + if parts.leadingComment != "" { + describe(parts.leadingComment, "lead") + } + if parts.varnameOp != "" { + describe(parts.varnameOp, "var") + } + describeHspace(parts.spaceBeforeValue) + if parts.value != "" { + describe(parts.value, "value") + if parts.trailingComment != "" || parts.continuation != "" { + describeHspace(parts.spaceAfterValue) + } + } + if parts.trailingComment != "" { + describe(parts.trailingComment, "comment") + if parts.continuation != "" { + describeHspace(parts.spaceAfterComment) + } + } + if parts.continuation != "" { + describe(parts.continuation, "cont") + } + } + }) + + var expected []descriptor + testDescr := strings.SplitN(testName, "__", 2)[1] + for i, part := range strings.Split(testDescr, "_") { + m, name, widthStr := match2(part, `^([a-z]+)(\d*)$`) + assert(m) + width, err := strconv.Atoi(widthStr) + if err != nil || width == 0 { + width = 0 + if i < len(actual) && name == actual[i].name { + width = actual[i].width + } + } + expected = append(expected, descriptor{name, width}) + } + + vt.tester.CheckDeepEquals(actual, expected) +} + +func (s *Suite) Test_VaralignBlock__var_none_value(c *check.C) { vt := NewVaralignTester(s, c) vt.Input( "VVVVVVVVVVVVVVVVVVV=value") vt.Internals( "20 20") vt.Diagnostics( - "NOTE: ~/Makefile:1: This variable value should be aligned to column 25.") + "NOTE: Makefile:1: This variable value should be aligned to column 25.") vt.Autofixes( - "AUTOFIX: ~/Makefile:1: Replacing \"\" with \"\\t\".") + "AUTOFIX: Makefile:1: Replacing \"\" with \"\\t\".") vt.Fixed( "VVVVVVVVVVVVVVVVVVV= value") vt.Run() } -func (s *Suite) Test_VaralignBlock__one_line_simple_space(c *check.C) { +// Indentations with a single space are only allowed in some very few +// places, such as continuation lines or very long variable names. +// In a single paragraph of its own, indentation with a single space +// doesn't make sense, therefore it is replaced with a tab. +func (s *Suite) Test_VaralignBlock__var_space_value(c *check.C) { vt := NewVaralignTester(s, c) vt.Input( "VVVVVVVVVVVVVVVVVVV= value") vt.Internals( "20 21") vt.Diagnostics( - "NOTE: ~/Makefile:1: This variable value should be aligned with tabs, not spaces, to column 25.") + "NOTE: Makefile:1: This variable value should be aligned with tabs, not spaces, to column 25.") vt.Autofixes( - "AUTOFIX: ~/Makefile:1: Replacing \" \" with \"\\t\".") + "AUTOFIX: Makefile:1: Replacing \" \" with \"\\t\".") vt.Fixed( "VVVVVVVVVVVVVVVVVVV= value") vt.Run() } -func (s *Suite) Test_VaralignBlock__one_line_simple_tab(c *check.C) { +// While indentation with a single space is allowed in a few cases, +// indentation with several spaces is never allowed and is replaced +// with tabs. +func (s *Suite) Test_VaralignBlock__var_spaces7_value(c *check.C) { + vt := NewVaralignTester(s, c) + vt.Input( + "VAR= several spaces") + vt.Internals( + "04 07") + vt.Diagnostics( + "NOTE: Makefile:1: This variable value should be aligned with tabs, not spaces, to column 9.") + vt.Autofixes( + "AUTOFIX: Makefile:1: Replacing \" \" with \"\\t\".") + vt.Fixed( + "VAR= several spaces") + vt.Run() +} + +// Inconsistently aligned lines for variables of the same length are +// replaced with tabs, so that they align nicely. +func (s *Suite) Test_VaralignBlock__var4_space5_value_var4_spaces6_value(c *check.C) { + vt := NewVaralignTester(s, c) + vt.Input( + "VAR= indented with one space", + "VAR= indented with two spaces") + vt.Internals( + "04 05", + "04 06") + vt.Diagnostics( + "NOTE: Makefile:1: This variable value should be aligned with tabs, not spaces, to column 9.", + "NOTE: Makefile:2: This variable value should be aligned with tabs, not spaces, to column 9.") + vt.Autofixes( + "AUTOFIX: Makefile:1: Replacing \" \" with \"\\t\".", + "AUTOFIX: Makefile:2: Replacing \" \" with \"\\t\".") + vt.Fixed( + "VAR= indented with one space", + "VAR= indented with two spaces") + vt.Run() +} + +// Generally, the value in variable assignments is aligned at the next tab. +func (s *Suite) Test_VaralignBlock__var_tab8_value(c *check.C) { + vt := NewVaralignTester(s, c) + vt.Input( + "VAR=\tone tab") + vt.Internals( + "04 08") + vt.Diagnostics() + vt.Autofixes() + vt.Fixed( + "VAR= one tab") + vt.Run() +} + +func (s *Suite) Test_VaralignBlock__var_tab24_value(c *check.C) { vt := NewVaralignTester(s, c) vt.Input( "VVVVVVVVVVVVVVVVVVV=\tvalue") @@ -139,22 +299,24 @@ func (s *Suite) Test_VaralignBlock__one_line_simple_tab(c *check.C) { vt.Run() } -func (s *Suite) Test_VaralignBlock__one_line_simple_sss(c *check.C) { +func (s *Suite) Test_VaralignBlock__var_spaces_value(c *check.C) { vt := NewVaralignTester(s, c) vt.Input( "VVVVVVVVVVVVVVVVVVV= value") vt.Internals( "20 23") vt.Diagnostics( - "NOTE: ~/Makefile:1: This variable value should be aligned with tabs, not spaces, to column 25.") + "NOTE: Makefile:1: This variable value should be aligned with tabs, not spaces, to column 25.") vt.Autofixes( - "AUTOFIX: ~/Makefile:1: Replacing \" \" with \"\\t\".") + "AUTOFIX: Makefile:1: Replacing \" \" with \"\\t\".") vt.Fixed( "VVVVVVVVVVVVVVVVVVV= value") vt.Run() } -func (s *Suite) Test_VaralignBlock__one_line_simple_ttt(c *check.C) { +// Having more tabs than necessary is allowed. This can be for aesthetic +// reasons to align this paragraph with the others in the same file. +func (s *Suite) Test_VaralignBlock__var_tabs_value(c *check.C) { vt := NewVaralignTester(s, c) vt.Input( "VVVVVVVVVVVVVVVVVVV=\t\t\tvalue") @@ -167,47 +329,46 @@ func (s *Suite) Test_VaralignBlock__one_line_simple_ttt(c *check.C) { vt.Run() } -func (s *Suite) Test_VaralignBlock__one_line_simple_tsts(c *check.C) { +func (s *Suite) Test_VaralignBlock__var_tsts_value(c *check.C) { vt := NewVaralignTester(s, c) vt.Input( "VVVVVVVVVVVVVVVVVVV=\t \t value") vt.Internals( "20 33") vt.Diagnostics( - "NOTE: ~/Makefile:1: This variable value should be aligned with tabs, not spaces, to column 25.") + "NOTE: Makefile:1: This variable value should be aligned with tabs, not spaces, to column 25.") vt.Autofixes( - "AUTOFIX: ~/Makefile:1: Replacing \"\\t \\t \" with \"\\t\".") + "AUTOFIX: Makefile:1: Replacing \"\\t \\t \" with \"\\t\".") vt.Fixed( "VVVVVVVVVVVVVVVVVVV= value") vt.Run() } -func (s *Suite) Test_VaralignBlock__one_line_follow_none(c *check.C) { +func (s *Suite) Test_VaralignBlock__var_none_cont_tab_value(c *check.C) { vt := NewVaralignTester(s, c) vt.Input( "VVVVVVVVVVVVVVVVVVV=\\", "\tvalue") vt.Internals( - "20 20", + "20 20 20", " 08") vt.Diagnostics( - // TODO: There should be a space to the left of the backslash. - nil...) + "NOTE: Makefile:1: The continuation backslash should be preceded by a single space or tab.") vt.Autofixes( - nil...) + "AUTOFIX: Makefile:1: Replacing \"\" with \" \".") vt.Fixed( - "VVVVVVVVVVVVVVVVVVV=\\", + "VVVVVVVVVVVVVVVVVVV= \\", " value") vt.Run() } -func (s *Suite) Test_VaralignBlock__one_line_follow_space(c *check.C) { +func (s *Suite) Test_VaralignBlock__var_space_cont_tab_value(c *check.C) { vt := NewVaralignTester(s, c) vt.Input( "VVVVVVVVVVVVVVVVVVV= \\", "\tvalue") vt.Internals( - "20 21", + "20 21 21", " 08") vt.Diagnostics() vt.Autofixes() @@ -217,13 +378,13 @@ func (s *Suite) Test_VaralignBlock__one_line_follow_space(c *check.C) { vt.Run() } -func (s *Suite) Test_VaralignBlock__one_line_follow_tab(c *check.C) { +func (s *Suite) Test_VaralignBlock__var_tab_cont_tab_value(c *check.C) { vt := NewVaralignTester(s, c) vt.Input( "VVVVVVVVVVVVVVVVVVV=\t\\", "\tvalue") vt.Internals( - "20 24", + "20 24 24", " 08") vt.Diagnostics() vt.Autofixes() @@ -233,45 +394,49 @@ func (s *Suite) Test_VaralignBlock__one_line_follow_tab(c *check.C) { vt.Run() } -func (s *Suite) Test_VaralignBlock__one_line_follow_sss(c *check.C) { +func (s *Suite) Test_VaralignBlock__var_spaces_cont_tab_value(c *check.C) { vt := NewVaralignTester(s, c) vt.Input( "VVVVVVVVVVVVVVVVVVV= \\", "\tvalue") vt.Internals( - "20 23", + "20 23 23", " 08") - vt.Diagnostics() - vt.Autofixes() + vt.Diagnostics( + "NOTE: Makefile:1: The continuation backslash should be preceded by a single space or tab.") + vt.Autofixes( + "AUTOFIX: Makefile:1: Replacing \" \" with \" \".") vt.Fixed( - "VVVVVVVVVVVVVVVVVVV= \\", + "VVVVVVVVVVVVVVVVVVV= \\", " value") vt.Run() } -func (s *Suite) Test_VaralignBlock__one_line_follow_ttt(c *check.C) { +func (s *Suite) Test_VaralignBlock__var_tabs_cont_tab_value(c *check.C) { vt := NewVaralignTester(s, c) vt.Input( "VVVVVVVVVVVVVVVVVVV=\t\t\t\\", "\tvalue") vt.Internals( - "20 40", + "20 40 40", " 08") - vt.Diagnostics() - vt.Autofixes() + vt.Diagnostics( + "NOTE: Makefile:1: The continuation backslash should be preceded by a single space or tab.") + vt.Autofixes( + "AUTOFIX: Makefile:1: Replacing \"\\t\\t\\t\" with \" \".") vt.Fixed( - "VVVVVVVVVVVVVVVVVVV= \\", + "VVVVVVVVVVVVVVVVVVV= \\", " value") vt.Run() } -func (s *Suite) Test_VaralignBlock__one_line_follow_tab72(c *check.C) { +func (s *Suite) Test_VaralignBlock__var_tabs72_cont_tab_value(c *check.C) { vt := NewVaralignTester(s, c) vt.Input( "VVVVVVVVVVVVVVVVVVV=\t\t\t\t\t\t\t\\", "\tvalue") vt.Internals( - "20 72", + "20 72 72", " 08") vt.Diagnostics() vt.Autofixes() @@ -281,83 +446,67 @@ func (s *Suite) Test_VaralignBlock__one_line_follow_tab72(c *check.C) { vt.Run() } -func (s *Suite) Test_VaralignBlock__one_line_follow_indent_none(c *check.C) { +func (s *Suite) Test_VaralignBlock__var_space_cont_none_value(c *check.C) { vt := NewVaralignTester(s, c) vt.Input( "VVVVVVVVVVVVVVVVVVV= \\", "value") vt.Internals( - "20 21", + "20 21 21", " 00") vt.Diagnostics( - "NOTE: ~/Makefile:2: This continuation line should be indented with \"\\t\".") + "NOTE: Makefile:2: This continuation line should be indented with \"\\t\".") vt.Autofixes( - "AUTOFIX: ~/Makefile:2: Replacing \"\" with \"\\t\".") + "AUTOFIX: Makefile:2: Replacing \"\" with \"\\t\".") vt.Fixed( "VVVVVVVVVVVVVVVVVVV= \\", " value") vt.Run() } -func (s *Suite) Test_VaralignBlock__one_line_follow_indent_space(c *check.C) { +func (s *Suite) Test_VaralignBlock__var_space_cont_space_value(c *check.C) { vt := NewVaralignTester(s, c) vt.Input( "VVVVVVVVVVVVVVVVVVV= \\", " value") vt.Internals( - "20 21", + "20 21 21", " 01") vt.Diagnostics( - "NOTE: ~/Makefile:2: This continuation line should be indented with \"\\t\".") + "NOTE: Makefile:2: This continuation line should be indented with \"\\t\".") vt.Autofixes( - "AUTOFIX: ~/Makefile:2: Replacing \" \" with \"\\t\".") - vt.Fixed( - "VVVVVVVVVVVVVVVVVVV= \\", - " value") - vt.Run() -} - -func (s *Suite) Test_VaralignBlock__one_line_follow_indent_tab(c *check.C) { - vt := NewVaralignTester(s, c) - vt.Input( - "VVVVVVVVVVVVVVVVVVV= \\", - "\tvalue") - vt.Internals( - "20 21", - " 08") - vt.Diagnostics() - vt.Autofixes() + "AUTOFIX: Makefile:2: Replacing \" \" with \"\\t\".") vt.Fixed( "VVVVVVVVVVVVVVVVVVV= \\", " value") vt.Run() } -func (s *Suite) Test_VaralignBlock__one_line_follow_indent_sss(c *check.C) { +func (s *Suite) Test_VaralignBlock__var_space_cont_spaces3_value(c *check.C) { vt := NewVaralignTester(s, c) vt.Input( "VVVVVVVVVVVVVVVVVVV= \\", " value") vt.Internals( - "20 21", + "20 21 21", " 03") vt.Diagnostics( - "NOTE: ~/Makefile:2: This continuation line should be indented with \"\\t\".") + "NOTE: Makefile:2: This continuation line should be indented with \"\\t\".") vt.Autofixes( - "AUTOFIX: ~/Makefile:2: Replacing \" \" with \"\\t\".") + "AUTOFIX: Makefile:2: Replacing \" \" with \"\\t\".") vt.Fixed( "VVVVVVVVVVVVVVVVVVV= \\", " value") vt.Run() } -func (s *Suite) Test_VaralignBlock__one_line_follow_indent_tt(c *check.C) { +func (s *Suite) Test_VaralignBlock__var20_space_cont_tabs16_value(c *check.C) { vt := NewVaralignTester(s, c) vt.Input( "VVVVVVVVVVVVVVVVVVV= \\", "\t\tvalue") vt.Internals( - "20 21", + "20 21 21", " 16") vt.Diagnostics() vt.Autofixes() @@ -367,13 +516,13 @@ func (s *Suite) Test_VaralignBlock__one_line_follow_indent_tt(c *check.C) { vt.Run() } -func (s *Suite) Test_VaralignBlock__one_line_follow_indent_ttt(c *check.C) { +func (s *Suite) Test_VaralignBlock__var20_space_cont_tabs24_value(c *check.C) { vt := NewVaralignTester(s, c) vt.Input( "VVVVVVVVVVVVVVVVVVV= \\", "\t\t\tvalue") vt.Internals( - "20 21", + "20 21 21", " 24") vt.Diagnostics() vt.Autofixes() @@ -383,71 +532,71 @@ func (s *Suite) Test_VaralignBlock__one_line_follow_indent_ttt(c *check.C) { vt.Run() } -func (s *Suite) Test_VaralignBlock__one_line_follow_indent_tsts(c *check.C) { +func (s *Suite) Test_VaralignBlock__var20_space_cont_tsts_value(c *check.C) { vt := NewVaralignTester(s, c) vt.Input( "VVVVVVVVVVVVVVVVVVV= \\", "\t \t value") vt.Internals( - "20 21", + "20 21 21", " 17") vt.Diagnostics( - "NOTE: ~/Makefile:2: This continuation line should be indented with \"\\t\\t \".") + "NOTE: Makefile:2: This continuation line should be indented with \"\\t\\t \".") vt.Autofixes( - "AUTOFIX: ~/Makefile:2: Replacing \"\\t \\t \" with \"\\t\\t \".") + "AUTOFIX: Makefile:2: Replacing \"\\t \\t \" with \"\\t\\t \".") vt.Fixed( "VVVVVVVVVVVVVVVVVVV= \\", " value") vt.Run() } -func (s *Suite) Test_VaralignBlock__one_line_initial_none(c *check.C) { +func (s *Suite) Test_VaralignBlock__var20_none_value_space_cont_indent20_value(c *check.C) { vt := NewVaralignTester(s, c) vt.Input( "VVVVVVVVVVVVVVVVVVV=value \\", "\t\t value") vt.Internals( - "20 20", + "20 20 26", " 20") vt.Diagnostics( - "NOTE: ~/Makefile:1: This variable value should be aligned to column 25.", - "NOTE: ~/Makefile:2: This continuation line should be indented with \"\\t\\t\\t\".") + "NOTE: Makefile:1: This variable value should be aligned to column 25.", + "NOTE: Makefile:2: This continuation line should be indented with \"\\t\\t\\t\".") vt.Autofixes( - "AUTOFIX: ~/Makefile:1: Replacing \"\" with \"\\t\".", - "AUTOFIX: ~/Makefile:2: Replacing \"\\t\\t \" with \"\\t\\t\\t\".") + "AUTOFIX: Makefile:1: Replacing \"\" with \"\\t\".", + "AUTOFIX: Makefile:2: Replacing \"\\t\\t \" with \"\\t\\t\\t\".") vt.Fixed( "VVVVVVVVVVVVVVVVVVV= value \\", " value") vt.Run() } -func (s *Suite) Test_VaralignBlock__one_line_initial_space(c *check.C) { +func (s *Suite) Test_VaralignBlock__var20_space_value_space_cont_spaces21_value(c *check.C) { vt := NewVaralignTester(s, c) vt.Input( "VVVVVVVVVVVVVVVVVVV= value \\", " value") vt.Internals( - "20 21", + "20 21 27", " 21") vt.Diagnostics( - "NOTE: ~/Makefile:1: This variable value should be aligned with tabs, not spaces, to column 25.", - "NOTE: ~/Makefile:2: This continuation line should be indented with \"\\t\\t\\t\".") + "NOTE: Makefile:1: This variable value should be aligned with tabs, not spaces, to column 25.", + "NOTE: Makefile:2: This continuation line should be indented with \"\\t\\t\\t\".") vt.Autofixes( - "AUTOFIX: ~/Makefile:1: Replacing \" \" with \"\\t\".", - "AUTOFIX: ~/Makefile:2: Replacing \" \" with \"\\t\\t\\t\".") + "AUTOFIX: Makefile:1: Replacing \" \" with \"\\t\".", + "AUTOFIX: Makefile:2: Replacing \" \" with \"\\t\\t\\t\".") vt.Fixed( "VVVVVVVVVVVVVVVVVVV= value \\", " value") vt.Run() } -func (s *Suite) Test_VaralignBlock__one_line_initial_tab(c *check.C) { +func (s *Suite) Test_VaralignBlock__var_tab24_value_space_cont_tabs24_value(c *check.C) { vt := NewVaralignTester(s, c) vt.Input( "VVVVVVVVVVVVVVVVVVV=\tvalue \\", "\t\t\tvalue") vt.Internals( - "20 24", + "20 24 30", " 24") vt.Diagnostics() vt.Autofixes() @@ -457,33 +606,37 @@ func (s *Suite) Test_VaralignBlock__one_line_initial_tab(c *check.C) { vt.Run() } -func (s *Suite) Test_VaralignBlock__one_line_initial_sss(c *check.C) { +func (s *Suite) Test_VaralignBlock__var_spaces_value_space_cont_spaces_value(c *check.C) { vt := NewVaralignTester(s, c) vt.Input( "VVVVVVVVVVVVVVVVVVV= value \\", " value") vt.Internals( - "20 23", + "20 23 29", " 23") vt.Diagnostics( - "NOTE: ~/Makefile:1: This variable value should be aligned with tabs, not spaces, to column 25.", - "NOTE: ~/Makefile:2: This continuation line should be indented with \"\\t\\t\\t\".") + "NOTE: Makefile:1: This variable value should be aligned with tabs, not spaces, to column 25.", + "NOTE: Makefile:2: This continuation line should be indented with \"\\t\\t\\t\".") vt.Autofixes( - "AUTOFIX: ~/Makefile:1: Replacing \" \" with \"\\t\".", - "AUTOFIX: ~/Makefile:2: Replacing \" \" with \"\\t\\t\\t\".") + "AUTOFIX: Makefile:1: Replacing \" \" with \"\\t\".", + "AUTOFIX: Makefile:2: Replacing \" \" with \"\\t\\t\\t\".") vt.Fixed( "VVVVVVVVVVVVVVVVVVV= value \\", " value") vt.Run() } -func (s *Suite) Test_VaralignBlock__one_line_initial_ttt(c *check.C) { +// The indentation is deeper than necessary, but that's ok. +// +// In most cases, this deeper indentation matches the other +// paragraphs in the same file. +func (s *Suite) Test_VaralignBlock__var_tabs40_value_space_cont_tabs40_value(c *check.C) { vt := NewVaralignTester(s, c) vt.Input( "VVVVVVVVVVVVVVVVVVV=\t\t\tvalue \\", "\t\t\t\t\tvalue") vt.Internals( - "20 40", + "20 40 46", " 40") vt.Diagnostics() vt.Autofixes() @@ -493,34 +646,32 @@ func (s *Suite) Test_VaralignBlock__one_line_initial_ttt(c *check.C) { vt.Run() } -func (s *Suite) Test_VaralignBlock__one_line_initial_tab64(c *check.C) { +func (s *Suite) Test_VaralignBlock__var_tab24_value_tabs64_cont_tabs24_value(c *check.C) { vt := NewVaralignTester(s, c) vt.Input( "VVVVVVVVVVVVVVVVVVV=\tvalue\t\t\t\t\t\\", "\t\t\tvalue") vt.Internals( - "20 24", + "20 24 64", " 24") vt.Diagnostics( - // FIXME: backslash indentation must be space, tab or at column 73. - nil...) + // TODO: Or the continuation may be in column 73. + "NOTE: Makefile:1: The continuation backslash should be preceded by a single space or tab.") vt.Autofixes( - // FIXME: replace many tabs with a single space, since there are - // no more backslashes in this logical line. - nil...) + "AUTOFIX: Makefile:1: Replacing \"\\t\\t\\t\\t\\t\" with \" \".") vt.Fixed( - "VVVVVVVVVVVVVVVVVVV= value \\", + "VVVVVVVVVVVVVVVVVVV= value \\", " value") vt.Run() } -func (s *Suite) Test_VaralignBlock__one_line_initial_tab72(c *check.C) { +func (s *Suite) Test_VaralignBlock__var_tab24_value_tabs72_cont_tabs24_value(c *check.C) { vt := NewVaralignTester(s, c) vt.Input( "VVVVVVVVVVVVVVVVVVV=\tvalue\t\t\t\t\t\t\\", "\t\t\tvalue") vt.Internals( - "20 24", + "20 24 72", " 24") vt.Diagnostics() vt.Autofixes() @@ -530,137 +681,122 @@ func (s *Suite) Test_VaralignBlock__one_line_initial_tab72(c *check.C) { vt.Run() } -func (s *Suite) Test_VaralignBlock__one_line_initial_indent_none(c *check.C) { +func (s *Suite) Test_VaralignBlock__var_tab_value_space_cont_none_value(c *check.C) { vt := NewVaralignTester(s, c) vt.Input( "VVVVVVVVVVVVVVVVVVV=\tvalue \\", "value") vt.Internals( - "20 24", + "20 24 30", " 00") vt.Diagnostics( - "NOTE: ~/Makefile:2: This continuation line should be indented with \"\\t\\t\\t\".") + "NOTE: Makefile:2: This continuation line should be indented with \"\\t\\t\\t\".") vt.Autofixes( - "AUTOFIX: ~/Makefile:2: Replacing \"\" with \"\\t\\t\\t\".") + "AUTOFIX: Makefile:2: Replacing \"\" with \"\\t\\t\\t\".") vt.Fixed( "VVVVVVVVVVVVVVVVVVV= value \\", " value") vt.Run() } -func (s *Suite) Test_VaralignBlock__one_line_initial_indent_space(c *check.C) { +func (s *Suite) Test_VaralignBlock__var_tab_value_space_cont_space_value(c *check.C) { vt := NewVaralignTester(s, c) vt.Input( "VVVVVVVVVVVVVVVVVVV=\tvalue \\", " value") vt.Internals( - "20 24", + "20 24 30", " 01") vt.Diagnostics( - "NOTE: ~/Makefile:2: This continuation line should be indented with \"\\t\\t\\t\".") + "NOTE: Makefile:2: This continuation line should be indented with \"\\t\\t\\t\".") vt.Autofixes( - "AUTOFIX: ~/Makefile:2: Replacing \" \" with \"\\t\\t\\t\".") + "AUTOFIX: Makefile:2: Replacing \" \" with \"\\t\\t\\t\".") vt.Fixed( "VVVVVVVVVVVVVVVVVVV= value \\", " value") vt.Run() } -func (s *Suite) Test_VaralignBlock__one_line_initial_indent_tab(c *check.C) { +// The follow-up values should have at least the same indentation as the initial value. +func (s *Suite) Test_VaralignBlock__var_tab24_value_space_cont_tab_value(c *check.C) { vt := NewVaralignTester(s, c) vt.Input( "VVVVVVVVVVVVVVVVVVV=\tvalue \\", "\tvalue") vt.Internals( - "20 24", + "20 24 30", " 08") vt.Diagnostics( - nil...) + "NOTE: Makefile:2: This continuation line should be indented with \"\\t\\t\\t\".") vt.Autofixes( - nil...) + "AUTOFIX: Makefile:2: Replacing \"\\t\" with \"\\t\\t\\t\".") vt.Fixed( "VVVVVVVVVVVVVVVVVVV= value \\", - " value") + " value") vt.Run() } -func (s *Suite) Test_VaralignBlock__one_line_initial_indent_sss(c *check.C) { +func (s *Suite) Test_VaralignBlock__var_tab24_value_space_cont_spaces3_value(c *check.C) { vt := NewVaralignTester(s, c) vt.Input( "VVVVVVVVVVVVVVVVVVV=\tvalue \\", " value") vt.Internals( - "20 24", + "20 24 30", " 03") vt.Diagnostics( - "NOTE: ~/Makefile:2: This continuation line should be indented with \"\\t\\t\\t\".") + "NOTE: Makefile:2: This continuation line should be indented with \"\\t\\t\\t\".") vt.Autofixes( - "AUTOFIX: ~/Makefile:2: Replacing \" \" with \"\\t\\t\\t\".") + "AUTOFIX: Makefile:2: Replacing \" \" with \"\\t\\t\\t\".") vt.Fixed( "VVVVVVVVVVVVVVVVVVV= value \\", " value") vt.Run() } -func (s *Suite) Test_VaralignBlock__one_line_initial_indent_tt(c *check.C) { +func (s *Suite) Test_VaralignBlock__var_tab24_value_space_cont_tabs16_value(c *check.C) { vt := NewVaralignTester(s, c) vt.Input( "VVVVVVVVVVVVVVVVVVV=\tvalue \\", "\t\tvalue") vt.Internals( - "20 24", + "20 24 30", " 16") vt.Diagnostics( - "NOTE: ~/Makefile:2: This continuation line should be indented with \"\\t\\t\\t\".") + "NOTE: Makefile:2: This continuation line should be indented with \"\\t\\t\\t\".") vt.Autofixes( - "AUTOFIX: ~/Makefile:2: Replacing \"\\t\\t\" with \"\\t\\t\\t\".") - vt.Fixed( - "VVVVVVVVVVVVVVVVVVV= value \\", - " value") - vt.Run() -} - -func (s *Suite) Test_VaralignBlock__one_line_initial_indent_ttt(c *check.C) { - vt := NewVaralignTester(s, c) - vt.Input( - "VVVVVVVVVVVVVVVVVVV=\tvalue \\", - "\t\t\tvalue") - vt.Internals( - "20 24", - " 24") - vt.Diagnostics() - vt.Autofixes() + "AUTOFIX: Makefile:2: Replacing \"\\t\\t\" with \"\\t\\t\\t\".") vt.Fixed( "VVVVVVVVVVVVVVVVVVV= value \\", " value") vt.Run() } -func (s *Suite) Test_VaralignBlock__one_line_initial_indent_tsts(c *check.C) { +func (s *Suite) Test_VaralignBlock__var_tab24_value_space_cont_tsts_value(c *check.C) { vt := NewVaralignTester(s, c) vt.Input( "VVVVVVVVVVVVVVVVVVV=\tvalue \\", "\t \t value") vt.Internals( - "20 24", + "20 24 30", " 17") vt.Diagnostics( - "NOTE: ~/Makefile:2: This continuation line should be indented with \"\\t\\t\\t\".") + "NOTE: Makefile:2: This continuation line should be indented with \"\\t\\t\\t\".") vt.Autofixes( - "AUTOFIX: ~/Makefile:2: Replacing \"\\t \\t \" with \"\\t\\t\\t\".") + "AUTOFIX: Makefile:2: Replacing \"\\t \\t \" with \"\\t\\t\\t\".") vt.Fixed( "VVVVVVVVVVVVVVVVVVV= value \\", " value") vt.Run() } -func (s *Suite) Test_VaralignBlock__one_line_initial_indent_plus_sss(c *check.C) { +func (s *Suite) Test_VaralignBlock__var_tab24_value_space_cont_indent27_value(c *check.C) { vt := NewVaralignTester(s, c) vt.Input( "VVVVVVVVVVVVVVVVVVV=\tvalue \\", "\t\t\t value") vt.Internals( - "20 24", + "20 24 30", " 27") vt.Diagnostics() vt.Autofixes() @@ -670,13 +806,13 @@ func (s *Suite) Test_VaralignBlock__one_line_initial_indent_plus_sss(c *check.C) vt.Run() } -func (s *Suite) Test_VaralignBlock__one_line_initial_indent_plus_tab(c *check.C) { +func (s *Suite) Test_VaralignBlock__var_tab24_value_space_cont_tabs32_value(c *check.C) { vt := NewVaralignTester(s, c) vt.Input( "VVVVVVVVVVVVVVVVVVV=\tvalue \\", "\t\t\t\tvalue") vt.Internals( - "20 24", + "20 24 30", " 32") vt.Diagnostics() vt.Autofixes() @@ -695,97 +831,8 @@ func (s *Suite) Test_VaralignBlock__one_line_initial_indent_plus_tab(c *check.C) // TODO: add systematic tests for commented lines -// Generally, the value in variable assignments is aligned -// at the next tab. -func (s *Suite) Test_VaralignBlock__one_var_tab(c *check.C) { - vt := NewVaralignTester(s, c) - vt.Input( - "VAR=\tone tab") - vt.Internals( - "04 08") - vt.Diagnostics() - vt.Autofixes() - vt.Fixed( - "VAR= one tab") - vt.Run() -} - -// Having more tabs than necessary is allowed. This can be for aesthetic -// reasons to align this paragraph with the others in the same file. -func (s *Suite) Test_VaralignBlock__one_var_tabs(c *check.C) { - vt := NewVaralignTester(s, c) - vt.Input( - "VAR=\t\t\tseveral tabs") - vt.Internals( - "04 24") - vt.Diagnostics() - vt.Autofixes() - vt.Fixed( - "VAR= several tabs") - vt.Run() -} - -// Indentations with a single space are only allowed in some very few -// places, such as continuation lines or very long variable names. -// In a single paragraph of its own, indentation with a single space -// doesn't make sense, therefore it is replaced with a tab. -func (s *Suite) Test_VaralignBlock__one_var_space(c *check.C) { - vt := NewVaralignTester(s, c) - vt.Input( - "VAR= indented with one space") - vt.Internals( - "04 05") - vt.Diagnostics( - "NOTE: ~/Makefile:1: This variable value should be aligned with tabs, not spaces, to column 9.") - vt.Autofixes( - "AUTOFIX: ~/Makefile:1: Replacing \" \" with \"\\t\".") - vt.Fixed( - "VAR= indented with one space") - vt.Run() -} - -// While indentation with a single space is allowed in a few cases, -// indentation with several spaces is never allowed and is replaced -// with tabs. -func (s *Suite) Test_VaralignBlock__one_var_spaces(c *check.C) { - vt := NewVaralignTester(s, c) - vt.Input( - "VAR= several spaces") - vt.Internals( - "04 07") - vt.Diagnostics( - "NOTE: ~/Makefile:1: This variable value should be aligned with tabs, not spaces, to column 9.") - vt.Autofixes( - "AUTOFIX: ~/Makefile:1: Replacing \" \" with \"\\t\".") - vt.Fixed( - "VAR= several spaces") - vt.Run() -} - -// Inconsistently aligned lines for variables of the same length are -// replaced with tabs, so that they align nicely. -func (s *Suite) Test_VaralignBlock__two_vars__spaces(c *check.C) { - vt := NewVaralignTester(s, c) - vt.Input( - "VAR= indented with one space", - "VAR= indented with two spaces") - vt.Internals( - "04 05", - "04 06") - vt.Diagnostics( - "NOTE: ~/Makefile:1: This variable value should be aligned with tabs, not spaces, to column 9.", - "NOTE: ~/Makefile:2: This variable value should be aligned with tabs, not spaces, to column 9.") - vt.Autofixes( - "AUTOFIX: ~/Makefile:1: Replacing \" \" with \"\\t\".", - "AUTOFIX: ~/Makefile:2: Replacing \" \" with \"\\t\".") - vt.Fixed( - "VAR= indented with one space", - "VAR= indented with two spaces") - vt.Run() -} - // All variables in a block are aligned to the same depth. -func (s *Suite) Test_VaralignBlock__several_vars__spaces(c *check.C) { +func (s *Suite) Test_VaralignBlock__var6_space_value_var7_space_value_var8_space_value_var9_space_value(c *check.C) { vt := NewVaralignTester(s, c) vt.Input( "GRP_A= value", @@ -798,15 +845,15 @@ func (s *Suite) Test_VaralignBlock__several_vars__spaces(c *check.C) { "08 09", "09 10") vt.Diagnostics( - "NOTE: ~/Makefile:1: This variable value should be aligned with tabs, not spaces, to column 17.", - "NOTE: ~/Makefile:2: This variable value should be aligned with tabs, not spaces, to column 17.", - "NOTE: ~/Makefile:3: This variable value should be aligned with tabs, not spaces, to column 17.", - "NOTE: ~/Makefile:4: This variable value should be aligned with tabs, not spaces, to column 17.") + "NOTE: Makefile:1: This variable value should be aligned with tabs, not spaces, to column 17.", + "NOTE: Makefile:2: This variable value should be aligned with tabs, not spaces, to column 17.", + "NOTE: Makefile:3: This variable value should be aligned with tabs, not spaces, to column 17.", + "NOTE: Makefile:4: This variable value should be aligned with tabs, not spaces, to column 17.") vt.Autofixes( - "AUTOFIX: ~/Makefile:1: Replacing \" \" with \"\\t\\t\".", - "AUTOFIX: ~/Makefile:2: Replacing \" \" with \"\\t\\t\".", - "AUTOFIX: ~/Makefile:3: Replacing \" \" with \"\\t\".", - "AUTOFIX: ~/Makefile:4: Replacing \" \" with \"\\t\".") + "AUTOFIX: Makefile:1: Replacing \" \" with \"\\t\\t\".", + "AUTOFIX: Makefile:2: Replacing \" \" with \"\\t\\t\".", + "AUTOFIX: Makefile:3: Replacing \" \" with \"\\t\".", + "AUTOFIX: Makefile:4: Replacing \" \" with \"\\t\".") vt.Fixed( "GRP_A= value", "GRP_AA= value", @@ -815,18 +862,21 @@ func (s *Suite) Test_VaralignBlock__several_vars__spaces(c *check.C) { vt.Run() } -// Lines that are continued may be indented with a single space -// if the first line of the variable definition has no value. -func (s *Suite) Test_VaralignBlock__continuation(c *check.C) { +// In lines that are continued, the continuation backslash may be separated +// with a single space if the first line contains only the backslash, but +// not a variable value. +func (s *Suite) Test_VaralignBlock__var4_space_cont_tab_value(c *check.C) { vt := NewVaralignTester(s, c) vt.Input( "VAR= \\", "\tvalue") vt.Internals( - "04 05", + "04 05 05", " 08") - vt.Diagnostics() - vt.Autofixes() + vt.Diagnostics( + nil...) + vt.Autofixes( + nil...) vt.Fixed( "VAR= \\", " value") @@ -837,7 +887,7 @@ func (s *Suite) Test_VaralignBlock__continuation(c *check.C) { // The second line is further to the right but doesn't count as // an outlier since it is not far enough. // Adding one more tab to the indentation is generally considered ok. -func (s *Suite) Test_VaralignBlock__short_tab__long_space(c *check.C) { +func (s *Suite) Test_VaralignBlock__var_tab8_value_var_space15_value(c *check.C) { vt := NewVaralignTester(s, c) vt.Input( "BLOCK=\tindented with tab", @@ -846,11 +896,11 @@ func (s *Suite) Test_VaralignBlock__short_tab__long_space(c *check.C) { "06 08", "14 15") vt.Diagnostics( - "NOTE: ~/Makefile:1: This variable value should be aligned to column 17.", - "NOTE: ~/Makefile:2: This variable value should be aligned with tabs, not spaces, to column 17.") + "NOTE: Makefile:1: This variable value should be aligned to column 17.", + "NOTE: Makefile:2: This variable value should be aligned with tabs, not spaces, to column 17.") vt.Autofixes( - "AUTOFIX: ~/Makefile:1: Replacing \"\\t\" with \"\\t\\t\".", - "AUTOFIX: ~/Makefile:2: Replacing \" \" with \"\\t\".") + "AUTOFIX: Makefile:1: Replacing \"\\t\" with \"\\t\\t\".", + "AUTOFIX: Makefile:2: Replacing \" \" with \"\\t\".") vt.Fixed( "BLOCK= indented with tab", "BLOCK_LONGVAR= indented with space") @@ -859,7 +909,7 @@ func (s *Suite) Test_VaralignBlock__short_tab__long_space(c *check.C) { // When the indentation differs, the indentation is adjusted to the // minimum necessary. -func (s *Suite) Test_VaralignBlock__short_long__tab(c *check.C) { +func (s *Suite) Test_VaralignBlock__var_tab8_value_var14_tabs40_value(c *check.C) { vt := NewVaralignTester(s, c) vt.Input( "BLOCK=\tshort", @@ -868,11 +918,11 @@ func (s *Suite) Test_VaralignBlock__short_long__tab(c *check.C) { "06 08", "14 40") 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.") + "NOTE: Makefile:1: This variable value should be aligned to column 17.", + "NOTE: Makefile:2: This variable value should be aligned to column 17.") vt.Autofixes( - "AUTOFIX: ~/Makefile:1: Replacing \"\\t\" with \"\\t\\t\".", - "AUTOFIX: ~/Makefile:2: Replacing \"\\t\\t\\t\\t\" with \"\\t\".") + "AUTOFIX: Makefile:1: Replacing \"\\t\" with \"\\t\\t\".", + "AUTOFIX: Makefile:2: Replacing \"\\t\\t\\t\\t\" with \"\\t\".") vt.Fixed( "BLOCK= short", "BLOCK_LONGVAR= long") @@ -882,7 +932,7 @@ func (s *Suite) Test_VaralignBlock__short_long__tab(c *check.C) { // For differing indentation, it doesn't matter whether the indentation // is done with tabs or with spaces. It is aligned to the minimum // necessary depth. -func (s *Suite) Test_VaralignBlock__space_and_tab(c *check.C) { +func (s *Suite) Test_VaralignBlock__var_spaces8_value_var_tab_value(c *check.C) { vt := NewVaralignTester(s, c) vt.Input( "VAR= space", @@ -891,69 +941,55 @@ func (s *Suite) Test_VaralignBlock__space_and_tab(c *check.C) { "04 08", "04 08") vt.Diagnostics( - "NOTE: ~/Makefile:1: Variable values should be aligned with tabs, not spaces.") + "NOTE: Makefile:1: Variable values should be aligned with tabs, not spaces.") vt.Autofixes( - "AUTOFIX: ~/Makefile:1: Replacing \" \" with \"\\t\".") + "AUTOFIX: Makefile:1: Replacing \" \" with \"\\t\".") vt.Fixed( "VAR= space", "VAR= tab ${VAR}") vt.Run() } -// There must always be a visible space between the assignment operator +// There should always be a visible space between the assignment operator // and the value. -func (s *Suite) Test_VaralignBlock__no_space_at_all(c *check.C) { +func (s *Suite) Test_VaralignBlock__var17_none_value(c *check.C) { vt := NewVaralignTester(s, c) vt.Input( "PKG_FAIL_REASON+=\"Message\"") vt.Internals( "17 17") vt.Diagnostics( - "NOTE: ~/Makefile:1: This variable value should be aligned to column 25.") + "NOTE: Makefile:1: This variable value should be aligned to column 25.") vt.Autofixes( - "AUTOFIX: ~/Makefile:1: Replacing \"\" with \"\\t\".") + "AUTOFIX: Makefile:1: Replacing \"\" with \"\\t\".") vt.Fixed( "PKG_FAIL_REASON+= \"Message\"") vt.Run() } -func (s *Suite) Test_VaralignBlock__empty_continuation_in_column_1(c *check.C) { +func (s *Suite) Test_VaralignBlock__var4_space_cont_none_value(c *check.C) { vt := NewVaralignTester(s, c) vt.Input( "VAR= \\", "no indentation") vt.Internals( - "04 05", + "04 05 05", " 00") vt.Diagnostics( - "NOTE: ~/Makefile:2: This continuation line should be indented with \"\\t\".") + "NOTE: Makefile:2: This continuation line should be indented with \"\\t\".") vt.Autofixes( - "AUTOFIX: ~/Makefile:2: Replacing \"\" with \"\\t\".") + "AUTOFIX: Makefile:2: Replacing \"\" with \"\\t\".") vt.Fixed( "VAR= \\", " no indentation") vt.Run() } -func (s *Suite) Test_VaralignBlock__empty_continuation_in_column_9(c *check.C) { - vt := NewVaralignTester(s, c) - vt.Input( - "VAR= \\", - "\tminimum indentation") - vt.Internals( - "04 05", - " 08") - vt.Diagnostics( - nil...) - vt.Autofixes( - nil...) - vt.Fixed( - "VAR= \\", - " minimum indentation") - vt.Run() -} - -func (s *Suite) Test_VaralignBlock__empty_continuation_space(c *check.C) { +// The simple variable assignment in line 1 determines the main alignment +// column, to which line 2 is then aligned. +// +// If the variable name in line 2 were longer, the space would be ok. +func (s *Suite) Test_VaralignBlock__var_tab8_value_var4_space_cont_tab_value(c *check.C) { vt := NewVaralignTester(s, c) vt.Input( "REF=\tvalue", @@ -961,12 +997,12 @@ func (s *Suite) Test_VaralignBlock__empty_continuation_space(c *check.C) { "\tminimum indentation") vt.Internals( "04 08", - "04 05", + "04 05 05", " 08") vt.Diagnostics( - "NOTE: ~/Makefile:2: This variable value should be aligned with tabs, not spaces, to column 9.") + "NOTE: Makefile:2: This variable value should be aligned with tabs, not spaces, to column 9.") vt.Autofixes( - "AUTOFIX: ~/Makefile:2: Replacing \" \" with \"\\t\".") + "AUTOFIX: Makefile:2: Replacing \" \" with \"\\t\".") vt.Fixed( "REF= value", "VAR= \\", @@ -974,7 +1010,7 @@ func (s *Suite) Test_VaralignBlock__empty_continuation_space(c *check.C) { vt.Run() } -func (s *Suite) Test_VaralignBlock__empty_continuation_properly_indented(c *check.C) { +func (s *Suite) Test_VaralignBlock__var_tab8_value_var_tab8_cont_tab_value(c *check.C) { vt := NewVaralignTester(s, c) vt.Input( "REF=\tvalue", @@ -982,7 +1018,7 @@ func (s *Suite) Test_VaralignBlock__empty_continuation_properly_indented(c *chec "\tminimum indentation") vt.Internals( "04 08", - "04 08", + "04 08 08", " 08") vt.Diagnostics() vt.Autofixes() @@ -993,7 +1029,10 @@ func (s *Suite) Test_VaralignBlock__empty_continuation_properly_indented(c *chec vt.Run() } -func (s *Suite) Test_VaralignBlock__empty_continuation_too_narrow(c *check.C) { +// The difference in the variable names is 10 characters, but when aligned +// with tabs, their values only differ by a single tab. This is not enough +// to count as an outlier. +func (s *Suite) Test_VaralignBlock__var14_tab_value_var4_tab_cont_tab_value(c *check.C) { vt := NewVaralignTester(s, c) vt.Input( "LONG_VARIABLE=\tvalue", @@ -1001,12 +1040,12 @@ func (s *Suite) Test_VaralignBlock__empty_continuation_too_narrow(c *check.C) { "\tminimum indentation") vt.Internals( "14 16", - "04 08", + "04 08 08", " 08") vt.Diagnostics( - "NOTE: ~/Makefile:2: This variable value should be aligned to column 17.") + "NOTE: Makefile:2: This variable value should be aligned to column 17.") vt.Autofixes( - "AUTOFIX: ~/Makefile:2: Replacing \"\\t\" with \"\\t\\t\".") + "AUTOFIX: Makefile:2: Replacing \"\\t\" with \"\\t\\t\".") vt.Fixed( "LONG_VARIABLE= value", "VAR= \\", @@ -1016,7 +1055,7 @@ func (s *Suite) Test_VaralignBlock__empty_continuation_too_narrow(c *check.C) { vt.Run() } -func (s *Suite) Test_VaralignBlock__empty_continuation_too_wide(c *check.C) { +func (s *Suite) Test_VaralignBlock__var14_tab_value_var21_tab_cont_tab_value(c *check.C) { vt := NewVaralignTester(s, c) vt.Input( "LONG_VARIABLE=\tvalue", @@ -1024,7 +1063,7 @@ func (s *Suite) Test_VaralignBlock__empty_continuation_too_wide(c *check.C) { "\tminimum indentation") vt.Internals( "14 16", - "21 24", + "21 24 24", " 08") vt.Diagnostics( nil...) @@ -1044,7 +1083,7 @@ func (s *Suite) Test_VaralignBlock__empty_continuation_too_wide(c *check.C) { // It only contains a backslash, without the usual space to the left. // This space should be inserted to the left of the backslash. // Everything else is fine. -func (s *Suite) Test_VaralignBlock__outlier_in_follow_continuation(c *check.C) { +func (s *Suite) Test_VaralignBlock__var12_tabs24_value_var38_none_cont_tabs24_value(c *check.C) { vt := NewVaralignTester(s, c) vt.Input( "PATCHFILES+=\t\temacs20-jumbo-patch-20170723.gz", @@ -1052,12 +1091,12 @@ func (s *Suite) Test_VaralignBlock__outlier_in_follow_continuation(c *check.C) { "\t\t\thttp://www.NetBSD.org/~dholland/patchkits/emacs20/") vt.Internals( "12 24", - "38 38", + "38 38 38", " 24") vt.Diagnostics( - "NOTE: ~/Makefile:2: This outlier variable value should be aligned with a single space.") + "NOTE: Makefile:2: The continuation backslash should be preceded by a single space or tab.") vt.Autofixes( - "AUTOFIX: ~/Makefile:2: Replacing \"\" with \" \".") + "AUTOFIX: Makefile:2: Replacing \"\" with \" \".") vt.Fixed( "PATCHFILES+= emacs20-jumbo-patch-20170723.gz", "SITES.emacs20-jumbo-patch-20170723.gz= \\", @@ -1065,7 +1104,7 @@ func (s *Suite) Test_VaralignBlock__outlier_in_follow_continuation(c *check.C) { vt.Run() } -func (s *Suite) Test_VaralignBlock__continuation_lines(c *check.C) { +func (s *Suite) Test_VaralignBlock__var_tab16_value_var_space_cont_tabs24_value_var_tabs32_value_var11_space_value(c *check.C) { vt := NewVaralignTester(s, c) vt.Input( "DISTFILES+=\tvalue", @@ -1075,20 +1114,20 @@ func (s *Suite) Test_VaralignBlock__continuation_lines(c *check.C) { "DISTFILES+= value") vt.Internals( "11 16", - "11 12", + "11 12 12", " 24", "11 32", "11 12") vt.Diagnostics( - "NOTE: ~/Makefile:2: This variable value should be aligned with tabs, not spaces, to column 17.", - "NOTE: ~/Makefile:3: This continuation line should be indented with \"\\t\\t\".", - "NOTE: ~/Makefile:4: This variable value should be aligned to column 17.", - "NOTE: ~/Makefile:5: This variable value should be aligned with tabs, not spaces, to column 17.") + "NOTE: Makefile:2: This variable value should be aligned with tabs, not spaces, to column 17.", + "NOTE: Makefile:3: This continuation line should be indented with \"\\t\\t\".", + "NOTE: Makefile:4: This variable value should be aligned to column 17.", + "NOTE: Makefile:5: This variable value should be aligned with tabs, not spaces, to column 17.") vt.Autofixes( - "AUTOFIX: ~/Makefile:2: Replacing \" \" with \"\\t\".", - "AUTOFIX: ~/Makefile:3: Replacing \"\\t\\t\\t\" with \"\\t\\t\".", - "AUTOFIX: ~/Makefile:4: Replacing \"\\t\\t\\t\" with \"\\t\".", - "AUTOFIX: ~/Makefile:5: Replacing \" \" with \"\\t\".") + "AUTOFIX: Makefile:2: Replacing \" \" with \"\\t\".", + "AUTOFIX: Makefile:3: Replacing \"\\t\\t\\t\" with \"\\t\\t\".", + "AUTOFIX: Makefile:4: Replacing \"\\t\\t\\t\" with \"\\t\".", + "AUTOFIX: Makefile:5: Replacing \" \" with \"\\t\".") vt.Fixed( "DISTFILES+= value", "DISTFILES+= \\", @@ -1115,7 +1154,7 @@ func (s *Suite) Test_VaralignBlock__continuation_lines(c *check.C) { // indentation is the same, this logical line doesn't count as an outlier. // // Because line 2--3 is not an outlier, line 1 is realigned to column 25. -func (s *Suite) Test_VaralignBlock__continuation_line_one_tab_ahead(c *check.C) { +func (s *Suite) Test_VaralignBlock__var_tabs16_value_var_tab24_value_space_cont_tabs24_value(c *check.C) { vt := NewVaralignTester(s, c) vt.Input( "VAR=\t\tvalue", @@ -1123,12 +1162,12 @@ func (s *Suite) Test_VaralignBlock__continuation_line_one_tab_ahead(c *check.C) "\t\t\thttps://example.org") vt.Internals( "04 16", - "18 24", + "18 24 44", " 24") vt.Diagnostics( - "NOTE: ~/Makefile:1: This variable value should be aligned to column 25.") + "NOTE: Makefile:1: This variable value should be aligned to column 25.") vt.Autofixes( - "AUTOFIX: ~/Makefile:1: Replacing \"\\t\\t\" with \"\\t\\t\\t\".") + "AUTOFIX: Makefile:1: Replacing \"\\t\\t\" with \"\\t\\t\\t\".") vt.Fixed( "VAR= value", "MASTER_SITE_NEDIT= https://example.org \\", @@ -1144,7 +1183,7 @@ func (s *Suite) Test_VaralignBlock__continuation_line_one_tab_ahead(c *check.C) // As soon as the V2 value would be properly indented with a tab, the // visual difference would not be as much, therefore the current // behavior is appropriate. -func (s *Suite) Test_VaralignBlock__outlier_more_than_8_spaces(c *check.C) { +func (s *Suite) Test_VaralignBlock__var_tab8_value_var_tab16_value(c *check.C) { vt := NewVaralignTester(s, c) vt.Input( "V2=\tvalue", @@ -1153,9 +1192,9 @@ func (s *Suite) Test_VaralignBlock__outlier_more_than_8_spaces(c *check.C) { "03 08", "15 16") vt.Diagnostics( - "NOTE: ~/Makefile:1: This variable value should be aligned to column 17.") + "NOTE: Makefile:1: This variable value should be aligned to column 17.") vt.Autofixes( - "AUTOFIX: ~/Makefile:1: Replacing \"\\t\" with \"\\t\\t\".") + "AUTOFIX: Makefile:1: Replacing \"\\t\" with \"\\t\\t\".") vt.Fixed( "V2= value", "V0000000000014= value") @@ -1165,13 +1204,13 @@ func (s *Suite) Test_VaralignBlock__outlier_more_than_8_spaces(c *check.C) { // Ensures that a wrong warning introduced in ccb56a5 is not logged. // The warning was about continuation lines that should be reindented. // In this case though, everything is already perfectly aligned. -func (s *Suite) Test_VaralignBlock__aligned_continuation(c *check.C) { +func (s *Suite) Test_VaralignBlock__var_tab16_value_space_cont_tabs16_value(c *check.C) { vt := NewVaralignTester(s, c) vt.Input( "USE_TOOLS+=\t[ awk \\", "\t\tsed") vt.Internals( - "11 16", + "11 16 22", " 16") vt.Diagnostics() vt.Autofixes() @@ -1184,7 +1223,7 @@ func (s *Suite) Test_VaralignBlock__aligned_continuation(c *check.C) { // Shell commands in continuation lines are assumed to be already nicely indented. // This particular example is not, but pkglint cannot decide this as of // version 5.7.14 (July 2019). -func (s *Suite) Test_VaralignBlock__shell_command(c *check.C) { +func (s *Suite) Test_VaralignBlock__var_tab24_value_var20_tabs72_cont_tab_value_tabs72_cont_tabs16_value(c *check.C) { vt := NewVaralignTester(s, c) vt.Input( "USE_BUILTIN.Xfixes=\tyes", @@ -1193,8 +1232,8 @@ func (s *Suite) Test_VaralignBlock__shell_command(c *check.C) { "\t\t:; else :; fi") vt.Internals( "19 24", - "20 72", - " 08", + "20 72 72", + " 08 72", " 16") vt.Diagnostics( nil...) @@ -1208,18 +1247,21 @@ func (s *Suite) Test_VaralignBlock__shell_command(c *check.C) { vt.Run() } +// For escaped variable names, the number of actual characters in the +// Makefile is relevant for indenting the source code. Therefore, the +// parsed an unescaped mkline.Varname cannot be used here. func (s *Suite) Test_VaralignBlock__escaped_varname(c *check.C) { vt := NewVaralignTester(s, c) vt.Input( "V.${v:S,\\#,,g}=\tvalue", "V2345678123456781234=\tvalue") vt.Internals( - "15 16", // 15, since the number sign is not escaped when computing the indentation + "15 16", // 15, since the number sign is still escaped when computing the indentation "21 24") vt.Diagnostics( - "NOTE: ~/Makefile:1: This variable value should be aligned to column 25.") + "NOTE: Makefile:1: This variable value should be aligned to column 25.") vt.Autofixes( - "AUTOFIX: ~/Makefile:1: Replacing \"\\t\" with \"\\t\\t\".") + "AUTOFIX: Makefile:1: Replacing \"\\t\" with \"\\t\\t\".") vt.Fixed( "V.${v:S,\\#,,g}= value", // looks misaligned because of the backslash "V2345678123456781234= value") @@ -1230,7 +1272,7 @@ func (s *Suite) Test_VaralignBlock__escaped_varname(c *check.C) { // values in the continuation lines, one value per line, all indented to the same depth. // The depth is either a single tab (see the test below) or aligns with the other // variables in the paragraph (this test). -func (s *Suite) Test_VaralignBlock__continuation_value_starts_in_second_line(c *check.C) { +func (s *Suite) Test_VaralignBlock__var_tab8_value_var_tab16_value_var28_space_cont_tabs24_value_space_cont_tabs24_value(c *check.C) { vt := NewVaralignTester(s, c) vt.Input( "WRKSRC=\t${WRKDIR}", @@ -1241,17 +1283,17 @@ func (s *Suite) Test_VaralignBlock__continuation_value_starts_in_second_line(c * vt.Internals( "07 08", "10 16", - "28 29", - " 24", + "28 29 29", + " 24 52", " 24") vt.Diagnostics( - "NOTE: ~/Makefile:1: This variable value should be aligned to column 17.", - "NOTE: ~/Makefile:4: This continuation line should be indented with \"\\t\\t\".", - "NOTE: ~/Makefile:5: This continuation line should be indented with \"\\t\\t\".") + "NOTE: Makefile:1: This variable value should be aligned to column 17.", + "NOTE: Makefile:4: This continuation line should be indented with \"\\t\\t\".", + "NOTE: Makefile:5: This continuation line should be indented with \"\\t\\t\".") vt.Autofixes( - "AUTOFIX: ~/Makefile:1: Replacing \"\\t\" with \"\\t\\t\".", - "AUTOFIX: ~/Makefile:4: Replacing \"\\t\\t\\t\" with \"\\t\\t\".", - "AUTOFIX: ~/Makefile:5: Replacing \"\\t\\t\\t\" with \"\\t\\t\".") + "AUTOFIX: Makefile:1: Replacing \"\\t\" with \"\\t\\t\".", + "AUTOFIX: Makefile:4: Replacing \"\\t\\t\\t\" with \"\\t\\t\".", + "AUTOFIX: Makefile:5: Replacing \"\\t\\t\\t\" with \"\\t\\t\".") vt.Fixed( "WRKSRC= ${WRKDIR}", "DISTFILES= distfile-1.0.0.tar.gz", @@ -1265,7 +1307,7 @@ func (s *Suite) Test_VaralignBlock__continuation_value_starts_in_second_line(c * // values in the continuation lines, one value per line, all indented to the same depth. // The depth is either a single tab (this test) or aligns with the other // variables in the paragraph (see the test above). -func (s *Suite) Test_VaralignBlock__continuation_value_starts_in_second_line_with_single_tab(c *check.C) { +func (s *Suite) Test_VaralignBlock__var_tab8_value_var_tab16_value_var28_space_cont_tab_value_space_cont_tab_value(c *check.C) { vt := NewVaralignTester(s, c) vt.Input( "WRKSRC=\t${WRKDIR}", @@ -1276,13 +1318,13 @@ func (s *Suite) Test_VaralignBlock__continuation_value_starts_in_second_line_wit vt.Internals( "07 08", "10 16", - "28 29", - " 08", + "28 29 29", + " 08 36", " 08") vt.Diagnostics( - "NOTE: ~/Makefile:1: This variable value should be aligned to column 17.") + "NOTE: Makefile:1: This variable value should be aligned to column 17.") vt.Autofixes( - "AUTOFIX: ~/Makefile:1: Replacing \"\\t\" with \"\\t\\t\".") + "AUTOFIX: Makefile:1: Replacing \"\\t\" with \"\\t\\t\".") vt.Fixed( "WRKSRC= ${WRKDIR}", "DISTFILES= distfile-1.0.0.tar.gz", @@ -1303,7 +1345,7 @@ func (s *Suite) Test_VaralignBlock__continuation_value_starts_in_second_line_wit // The whole paragraph could be indented less by making the SITES line a // pure continuation line, having only a backslash in its first line. Then // everything could be aligned to column 17. -func (s *Suite) Test_VaralignBlock__continuation_value_starts_in_first_line(c *check.C) { +func (s *Suite) Test_VaralignBlock__var_tab8_value_var_tab16_value_var28_tab_value_space_cont_tabs32_value(c *check.C) { vt := NewVaralignTester(s, c) vt.Input( "WRKSRC=\t${WRKDIR}", @@ -1313,14 +1355,14 @@ func (s *Suite) Test_VaralignBlock__continuation_value_starts_in_first_line(c *c vt.Internals( "07 08", "10 16", - "28 32", + "28 32 60", " 32") vt.Diagnostics( - "NOTE: ~/Makefile:1: This variable value should be aligned to column 33.", - "NOTE: ~/Makefile:2: This variable value should be aligned to column 33.") + "NOTE: Makefile:1: This variable value should be aligned to column 33.", + "NOTE: Makefile:2: This variable value should be aligned to column 33.") vt.Autofixes( - "AUTOFIX: ~/Makefile:1: Replacing \"\\t\" with \"\\t\\t\\t\\t\".", - "AUTOFIX: ~/Makefile:2: Replacing \"\\t\" with \"\\t\\t\\t\".") + "AUTOFIX: Makefile:1: Replacing \"\\t\" with \"\\t\\t\\t\\t\".", + "AUTOFIX: Makefile:2: Replacing \"\\t\" with \"\\t\\t\\t\".") vt.Fixed( "WRKSRC= ${WRKDIR}", "DISTFILES= distfile-1.0.0.tar.gz", @@ -1334,7 +1376,7 @@ func (s *Suite) Test_VaralignBlock__continuation_value_starts_in_first_line(c *c // other lines. The lines that are indented further should keep their // relative indentation depth, no matter if that is done with spaces or // with tabs. -func (s *Suite) Test_VaralignBlock__continuation_mixed_indentation_in_second_line(c *check.C) { +func (s *Suite) Test_VaralignBlock__var_tab8_value_var_tab16_value_var13_space_cont_indent34_value_space_cont_indent36_value_space_cont_indent34_value(c *check.C) { vt := NewVaralignTester(s, c) vt.Input( "WRKSRC=\t${WRKDIR}", @@ -1346,22 +1388,22 @@ func (s *Suite) Test_VaralignBlock__continuation_mixed_indentation_in_second_lin vt.Internals( "07 08", "10 16", - "13 14", - " 34", - " 36", + "13 14 14", + " 34 45", + " 36 46", " 34") vt.Diagnostics( - "NOTE: ~/Makefile:1: This variable value should be aligned to column 17.", - "NOTE: ~/Makefile:3: This variable value should be aligned with tabs, not spaces, to column 17.", - "NOTE: ~/Makefile:4: This continuation line should be indented with \"\\t\\t\".", - "NOTE: ~/Makefile:5: This continuation line should be indented with \"\\t\\t \".", - "NOTE: ~/Makefile:6: This continuation line should be indented with \"\\t\\t\".") + "NOTE: Makefile:1: This variable value should be aligned to column 17.", + "NOTE: Makefile:3: This variable value should be aligned with tabs, not spaces, to column 17.", + "NOTE: Makefile:4: This continuation line should be indented with \"\\t\\t\".", + "NOTE: Makefile:5: This continuation line should be indented with \"\\t\\t \".", + "NOTE: Makefile:6: This continuation line should be indented with \"\\t\\t\".") vt.Autofixes( - "AUTOFIX: ~/Makefile:1: Replacing \"\\t\" with \"\\t\\t\".", - "AUTOFIX: ~/Makefile:3: Replacing \" \" with \"\\t\".", - "AUTOFIX: ~/Makefile:4: Replacing \"\\t\\t\\t\\t \" with \"\\t\\t\".", - "AUTOFIX: ~/Makefile:5: Replacing \"\\t\\t\\t\\t \" with \"\\t\\t \".", - "AUTOFIX: ~/Makefile:6: Replacing \"\\t\\t\\t\\t \" with \"\\t\\t\".") + "AUTOFIX: Makefile:1: Replacing \"\\t\" with \"\\t\\t\".", + "AUTOFIX: Makefile:3: Replacing \" \" with \"\\t\".", + "AUTOFIX: Makefile:4: Replacing \"\\t\\t\\t\\t \" with \"\\t\\t\".", + "AUTOFIX: Makefile:5: Replacing \"\\t\\t\\t\\t \" with \"\\t\\t \".", + "AUTOFIX: Makefile:6: Replacing \"\\t\\t\\t\\t \" with \"\\t\\t\".") vt.Fixed( "WRKSRC= ${WRKDIR}", "DISTFILES= distfile-1.0.0.tar.gz", @@ -1387,19 +1429,19 @@ func (s *Suite) Test_VaralignBlock__continuation_mixed_indentation_in_first_line vt.Internals( "07 08", "10 16", - "13 34", - " 36", + "13 34 45", + " 36 46", " 34") vt.Diagnostics( - "NOTE: ~/Makefile:1: This variable value should be aligned to column 17.", - "NOTE: ~/Makefile:3: This variable value should be aligned with tabs, not spaces, to column 17.", - "NOTE: ~/Makefile:4: This continuation line should be indented with \"\\t\\t\".", - "NOTE: ~/Makefile:5: This continuation line should be indented with \"\\t\\t\".") + "NOTE: Makefile:1: This variable value should be aligned to column 17.", + "NOTE: Makefile:3: This variable value should be aligned with tabs, not spaces, to column 17.", + "NOTE: Makefile:4: This continuation line should be indented with \"\\t\\t\".", + "NOTE: Makefile:5: This continuation line should be indented with \"\\t\\t\".") vt.Autofixes( - "AUTOFIX: ~/Makefile:1: Replacing \"\\t\" with \"\\t\\t\".", - "AUTOFIX: ~/Makefile:3: Replacing \"\\t\\t\\t \" with \"\\t\".", - "AUTOFIX: ~/Makefile:4: Replacing \"\\t\\t\\t\\t \" with \"\\t\\t \".", - "AUTOFIX: ~/Makefile:5: Replacing \"\\t\\t\\t\\t \" with \"\\t\\t\".") + "AUTOFIX: Makefile:1: Replacing \"\\t\" with \"\\t\\t\".", + "AUTOFIX: Makefile:3: Replacing \"\\t\\t\\t \" with \"\\t\".", + "AUTOFIX: Makefile:4: Replacing \"\\t\\t\\t\\t \" with \"\\t\\t \".", + "AUTOFIX: Makefile:5: Replacing \"\\t\\t\\t\\t \" with \"\\t\\t\".") vt.Fixed( "WRKSRC= ${WRKDIR}", "DISTFILES= distfile-1.0.0.tar.gz", @@ -1410,9 +1452,6 @@ func (s *Suite) Test_VaralignBlock__continuation_mixed_indentation_in_first_line } func (s *Suite) Test_VaralignBlock__follow_up_indented_with_spaces(c *check.C) { - // FIXME: warn about the misleading empty line 6, - // but not in this test - vt := NewVaralignTester(s, c) vt.Input( "DISTFILES= \\", @@ -1422,20 +1461,20 @@ func (s *Suite) Test_VaralignBlock__follow_up_indented_with_spaces(c *check.C) { "\tand a tab \\", " ") // trailing whitespace vt.Internals( - "10 11", - " 01", - " 03", - " 08", - " 08", + "10 11 11", + " 01 11", + " 03 16", + " 08 21", + " 08 18", " 03") vt.Diagnostics( - "NOTE: ~/Makefile:2: This continuation line should be indented with \"\\t\".", - "NOTE: ~/Makefile:3: This continuation line should be indented with \"\\t\".", - "NOTE: ~/Makefile:4: This continuation line should be indented with \"\\t\".") + "NOTE: Makefile:2: This continuation line should be indented with \"\\t\".", + "NOTE: Makefile:3: This continuation line should be indented with \"\\t\".", + "NOTE: Makefile:4: This continuation line should be indented with \"\\t\".") vt.Autofixes( - "AUTOFIX: ~/Makefile:2: Replacing \" \" with \"\\t\".", - "AUTOFIX: ~/Makefile:3: Replacing \" \" with \"\\t\".", - "AUTOFIX: ~/Makefile:4: Replacing \" \" with \"\\t\".") + "AUTOFIX: Makefile:2: Replacing \" \" with \"\\t\".", + "AUTOFIX: Makefile:3: Replacing \" \" with \"\\t\".", + "AUTOFIX: Makefile:4: Replacing \" \" with \"\\t\".") vt.Fixed( "DISTFILES= \\", " one space \\", @@ -1450,8 +1489,9 @@ func (s *Suite) Test_VaralignBlock__follow_up_indented_with_spaces(c *check.C) { // fix the whole block to use the indentation of the second-longest line. // In this case, all of the remaining lines have the same indentation // (as there is only 1 line at all). -// Therefore this existing indentation is used instead of the minimum necessary, which would only be a single tab. -func (s *Suite) Test_VaralignBlock__tab_outlier(c *check.C) { +// Therefore this existing indentation is used instead of the minimum +// necessary, which would only be a single tab. +func (s *Suite) Test_VaralignBlock__var_tabs24_value_var45_tab_value(c *check.C) { vt := NewVaralignTester(s, c) vt.Input( "DISTFILES=\t\tvery-very-very-very-long-distfile-name", @@ -1469,10 +1509,9 @@ func (s *Suite) Test_VaralignBlock__tab_outlier(c *check.C) { // The SITES.* definition is indented less than the other lines, // therefore the whole paragraph will be realigned to that depth. -func (s *Suite) Test_VaralignBlock__multiline(c *check.C) { +func (s *Suite) Test_VaralignBlock__var_spaces24_value_space_cont_spaces24_value_var_spaces16_value_var_spaces24_value(c *check.C) { vt := NewVaralignTester(s, c) vt.Input( - "DIST_SUBDIR= asc", "DISTFILES= ${DISTNAME}${EXTRACT_SUFX} frontiers.mp3 \\", " machine_wars.mp3 time_to_strike.mp3", ".for file in frontiers.mp3 machine_wars.mp3 time_to_strike.mp3", @@ -1480,25 +1519,21 @@ func (s *Suite) Test_VaralignBlock__multiline(c *check.C) { ".endfor", "WRKSRC= ${WRKDIR}/${PKGNAME_NOREV}") vt.Internals( - "12 24", - "10 24", + "10 24 65", " 24", "14 16", "07 24") vt.Diagnostics( - "NOTE: ~/Makefile:1: This variable value should be aligned with tabs, not spaces, to column 17.", - "NOTE: ~/Makefile:2: This variable value should be aligned with tabs, not spaces, to column 17.", - "NOTE: ~/Makefile:3: This continuation line should be indented with \"\\t\\t\".", - "NOTE: ~/Makefile:5: Variable values should be aligned with tabs, not spaces.", - "NOTE: ~/Makefile:7: This variable value should be aligned with tabs, not spaces, to column 17.") + "NOTE: Makefile:1: This variable value should be aligned with tabs, not spaces, to column 17.", + "NOTE: Makefile:2: This continuation line should be indented with \"\\t\\t\".", + "NOTE: Makefile:4: Variable values should be aligned with tabs, not spaces.", + "NOTE: Makefile:6: This variable value should be aligned with tabs, not spaces, to column 17.") vt.Autofixes( - "AUTOFIX: ~/Makefile:1: Replacing \" \" with \"\\t\".", - "AUTOFIX: ~/Makefile:2: Replacing \" \" with \"\\t\".", - "AUTOFIX: ~/Makefile:3: Replacing \" \" with \"\\t\\t\".", - "AUTOFIX: ~/Makefile:5: Replacing \" \" with \"\\t\".", - "AUTOFIX: ~/Makefile:7: Replacing \" \" with \"\\t\\t\".") + "AUTOFIX: Makefile:1: Replacing \" \" with \"\\t\".", + "AUTOFIX: Makefile:2: Replacing \" \" with \"\\t\\t\".", + "AUTOFIX: Makefile:4: Replacing \" \" with \"\\t\".", + "AUTOFIX: Makefile:6: Replacing \" \" with \"\\t\\t\".") vt.Fixed( - "DIST_SUBDIR= asc", "DISTFILES= ${DISTNAME}${EXTRACT_SUFX} frontiers.mp3 \\", " machine_wars.mp3 time_to_strike.mp3", ".for file in frontiers.mp3 machine_wars.mp3 time_to_strike.mp3", @@ -1508,41 +1543,31 @@ func (s *Suite) Test_VaralignBlock__multiline(c *check.C) { vt.Run() } -// The CDROM variables align exactly at a tab position, therefore they must +// The CDROM variable aligns exactly at a tab position, therefore it must // be indented by at least one more space. Since that one space is not -// enough to count as an outlier, everything is indented by one more tab. -func (s *Suite) Test_VaralignBlock__single_space(c *check.C) { +// enough to count as an outlier, each line is indented by one more tab. +func (s *Suite) Test_VaralignBlock__var_tab16_value_var16_space_value_var14_tab_value(c *check.C) { vt := NewVaralignTester(s, c) vt.Input( "RESTRICTED=\tDo not sell, do not rent", "NO_BIN_ON_CDROM= ${RESTRICTED}", - "NO_BIN_ON_FTP=\t${RESTRICTED}", - "NO_SRC_ON_CDROM= ${RESTRICTED}", - "NO_SRC_ON_FTP=\t${RESTRICTED}") + "NO_BIN_ON_FTP=\t${RESTRICTED}") vt.Internals( "11 16", "16 17", - "14 16", - "16 17", "14 16") vt.Diagnostics( - "NOTE: ~/Makefile:1: This variable value should be aligned to column 25.", - "NOTE: ~/Makefile:2: This variable value should be aligned with tabs, not spaces, to column 25.", - "NOTE: ~/Makefile:3: This variable value should be aligned to column 25.", - "NOTE: ~/Makefile:4: This variable value should be aligned with tabs, not spaces, to column 25.", - "NOTE: ~/Makefile:5: This variable value should be aligned to column 25.") + "NOTE: Makefile:1: This variable value should be aligned to column 25.", + "NOTE: Makefile:2: This variable value should be aligned with tabs, not spaces, to column 25.", + "NOTE: Makefile:3: This variable value should be aligned to column 25.") vt.Autofixes( - "AUTOFIX: ~/Makefile:1: Replacing \"\\t\" with \"\\t\\t\".", - "AUTOFIX: ~/Makefile:2: Replacing \" \" with \"\\t\".", - "AUTOFIX: ~/Makefile:3: Replacing \"\\t\" with \"\\t\\t\".", - "AUTOFIX: ~/Makefile:4: Replacing \" \" with \"\\t\".", - "AUTOFIX: ~/Makefile:5: Replacing \"\\t\" with \"\\t\\t\".") + "AUTOFIX: Makefile:1: Replacing \"\\t\" with \"\\t\\t\".", + "AUTOFIX: Makefile:2: Replacing \" \" with \"\\t\".", + "AUTOFIX: Makefile:3: Replacing \"\\t\" with \"\\t\\t\".") vt.Fixed( "RESTRICTED= Do not sell, do not rent", "NO_BIN_ON_CDROM= ${RESTRICTED}", - "NO_BIN_ON_FTP= ${RESTRICTED}", - "NO_SRC_ON_CDROM= ${RESTRICTED}", - "NO_SRC_ON_FTP= ${RESTRICTED}") + "NO_BIN_ON_FTP= ${RESTRICTED}") vt.Run() } @@ -1550,7 +1575,7 @@ func (s *Suite) Test_VaralignBlock__single_space(c *check.C) { // The spaces are replaced with tabs, which makes the indentation 4 spaces deeper in the first paragraph. // In the second paragraph it's even 7 additional spaces. // This is ok though since it is the prevailing indentation style in pkgsrc. -func (s *Suite) Test_VaralignBlock__only_space(c *check.C) { +func (s *Suite) Test_VaralignBlock__var16_space_value_var16_space_value_var16_space_value(c *check.C) { vt := NewVaralignTester(s, c) vt.Input( "REPLACE_PYTHON+= *.py", @@ -1561,13 +1586,13 @@ func (s *Suite) Test_VaralignBlock__only_space(c *check.C) { "16 17", "16 17") vt.Diagnostics( - "NOTE: ~/Makefile:1: This variable value should be aligned with tabs, not spaces, to column 25.", - "NOTE: ~/Makefile:2: This variable value should be aligned with tabs, not spaces, to column 25.", - "NOTE: ~/Makefile:3: This variable value should be aligned with tabs, not spaces, to column 25.") + "NOTE: Makefile:1: This variable value should be aligned with tabs, not spaces, to column 25.", + "NOTE: Makefile:2: This variable value should be aligned with tabs, not spaces, to column 25.", + "NOTE: Makefile:3: This variable value should be aligned with tabs, not spaces, to column 25.") vt.Autofixes( - "AUTOFIX: ~/Makefile:1: Replacing \" \" with \"\\t\".", - "AUTOFIX: ~/Makefile:2: Replacing \" \" with \"\\t\".", - "AUTOFIX: ~/Makefile:3: Replacing \" \" with \"\\t\".") + "AUTOFIX: Makefile:1: Replacing \" \" with \"\\t\".", + "AUTOFIX: Makefile:2: Replacing \" \" with \"\\t\".", + "AUTOFIX: Makefile:3: Replacing \" \" with \"\\t\".") vt.Fixed( "REPLACE_PYTHON+= *.py", "REPLACE_PYTHON+= lib/*.py", @@ -1581,7 +1606,7 @@ func (s *Suite) Test_VaralignBlock__only_space(c *check.C) { // // As of December 2018, pkglint only looks at a single paragraph at a time, // therefore it cannot reliably decide whether this deep indentation is necessary. -func (s *Suite) Test_VaralignBlock__mixed_tabs_and_spaces_same_column(c *check.C) { +func (s *Suite) Test_VaralignBlock__var_spaces24_value_var_tabs24_value(c *check.C) { vt := NewVaralignTester(s, c) vt.Input( "DISTFILES+= space", @@ -1590,9 +1615,9 @@ func (s *Suite) Test_VaralignBlock__mixed_tabs_and_spaces_same_column(c *check.C "11 24", "11 24") vt.Diagnostics( - "NOTE: ~/Makefile:1: Variable values should be aligned with tabs, not spaces.") + "NOTE: Makefile:1: Variable values should be aligned with tabs, not spaces.") vt.Autofixes( - "AUTOFIX: ~/Makefile:1: Replacing \" \" with \"\\t\\t\".") + "AUTOFIX: Makefile:1: Replacing \" \" with \"\\t\\t\".") vt.Fixed( "DISTFILES+= space", "DISTFILES+= tab") @@ -1600,7 +1625,7 @@ func (s *Suite) Test_VaralignBlock__mixed_tabs_and_spaces_same_column(c *check.C } // Both lines are indented to the same column. Therefore none of them is considered an outlier. -func (s *Suite) Test_VaralignBlock__outlier_1(c *check.C) { +func (s *Suite) Test_VaralignBlock__var2_space_value_var2_tab_value(c *check.C) { vt := NewVaralignTester(s, c) vt.Input( "V= value", @@ -1609,9 +1634,9 @@ func (s *Suite) Test_VaralignBlock__outlier_1(c *check.C) { "02 03", "02 08") vt.Diagnostics( - "NOTE: ~/Makefile:1: This variable value should be aligned with tabs, not spaces, to column 9.") + "NOTE: Makefile:1: This variable value should be aligned with tabs, not spaces, to column 9.") vt.Autofixes( - "AUTOFIX: ~/Makefile:1: Replacing \" \" with \"\\t\".") + "AUTOFIX: Makefile:1: Replacing \" \" with \"\\t\".") vt.Fixed( "V= value", "V= value") @@ -1619,7 +1644,7 @@ func (s *Suite) Test_VaralignBlock__outlier_1(c *check.C) { } // A single space that ends at the same depth as a tab is replaced with a tab, for consistency. -func (s *Suite) Test_VaralignBlock__outlier_2(c *check.C) { +func (s *Suite) Test_VaralignBlock__var7_space_value_var2_tab_value(c *check.C) { vt := NewVaralignTester(s, c) vt.Input( "V.0008= value", @@ -1628,9 +1653,9 @@ func (s *Suite) Test_VaralignBlock__outlier_2(c *check.C) { "07 08", "02 08") vt.Diagnostics( - "NOTE: ~/Makefile:1: Variable values should be aligned with tabs, not spaces.") + "NOTE: Makefile:1: Variable values should be aligned with tabs, not spaces.") vt.Autofixes( - "AUTOFIX: ~/Makefile:1: Replacing \" \" with \"\\t\".") + "AUTOFIX: Makefile:1: Replacing \" \" with \"\\t\".") vt.Fixed( "V.0008= value", "V= value") @@ -1641,7 +1666,7 @@ func (s *Suite) Test_VaralignBlock__outlier_2(c *check.C) { // that is indented with a space. This is because space-indented lines are // only allowed when their indentation is much deeper than the tab-indented // ones (so-called outliers), or as the first line of a continuation line. -func (s *Suite) Test_VaralignBlock__outlier_3(c *check.C) { +func (s *Suite) Test_VaralignBlock__var8_space_value_var2_tab_value(c *check.C) { vt := NewVaralignTester(s, c) vt.Input( "V.00009= value", @@ -1650,11 +1675,11 @@ func (s *Suite) Test_VaralignBlock__outlier_3(c *check.C) { "08 09", "02 08") vt.Diagnostics( - "NOTE: ~/Makefile:1: This variable value should be aligned with tabs, not spaces, to column 17.", - "NOTE: ~/Makefile:2: This variable value should be aligned to column 17.") + "NOTE: Makefile:1: This variable value should be aligned with tabs, not spaces, to column 17.", + "NOTE: Makefile:2: This variable value should be aligned to column 17.") vt.Autofixes( - "AUTOFIX: ~/Makefile:1: Replacing \" \" with \"\\t\".", - "AUTOFIX: ~/Makefile:2: Replacing \"\\t\" with \"\\t\\t\".") + "AUTOFIX: Makefile:1: Replacing \" \" with \"\\t\".", + "AUTOFIX: Makefile:2: Replacing \"\\t\" with \"\\t\\t\".") vt.Fixed( "V.00009= value", "V= value") @@ -1664,7 +1689,7 @@ func (s *Suite) Test_VaralignBlock__outlier_3(c *check.C) { // This space-indented line doesn't count as an outlier yet because it // is only a single tab away. The limit is two tabs. // Therefore both lines are indented with tabs. -func (s *Suite) Test_VaralignBlock__outlier_4(c *check.C) { +func (s *Suite) Test_VaralignBlock__var15_space_value_var2_tab_value(c *check.C) { vt := NewVaralignTester(s, c) vt.Input( "V.000000000016= value", @@ -1673,11 +1698,11 @@ func (s *Suite) Test_VaralignBlock__outlier_4(c *check.C) { "15 16", "02 08") vt.Diagnostics( - "NOTE: ~/Makefile:1: Variable values should be aligned with tabs, not spaces.", - "NOTE: ~/Makefile:2: This variable value should be aligned to column 17.") + "NOTE: Makefile:1: Variable values should be aligned with tabs, not spaces.", + "NOTE: Makefile:2: This variable value should be aligned to column 17.") vt.Autofixes( - "AUTOFIX: ~/Makefile:1: Replacing \" \" with \"\\t\".", - "AUTOFIX: ~/Makefile:2: Replacing \"\\t\" with \"\\t\\t\".") + "AUTOFIX: Makefile:1: Replacing \" \" with \"\\t\".", + "AUTOFIX: Makefile:2: Replacing \"\\t\" with \"\\t\\t\".") vt.Fixed( "V.000000000016= value", "V= value") @@ -1688,7 +1713,7 @@ func (s *Suite) Test_VaralignBlock__outlier_4(c *check.C) { // tab-indented line. The latter would require 2 tabs to align to the former. // Therefore the short line is not indented to the long line, in order to // keep the indentation reasonably short for a large amount of the lines. -func (s *Suite) Test_VaralignBlock__outlier_5(c *check.C) { +func (s *Suite) Test_VaralignBlock__var16_space_value_var2_tab_value(c *check.C) { vt := NewVaralignTester(s, c) vt.Input( "V.0000000000017= value", @@ -1705,7 +1730,7 @@ func (s *Suite) Test_VaralignBlock__outlier_5(c *check.C) { } // Short space-indented lines do not count as outliers. They are are aligned to the longer tab-indented line. -func (s *Suite) Test_VaralignBlock__outlier_6(c *check.C) { +func (s *Suite) Test_VaralignBlock__var2_space_value_var9_tab_value(c *check.C) { vt := NewVaralignTester(s, c) vt.Input( "V= value", @@ -1714,9 +1739,9 @@ func (s *Suite) Test_VaralignBlock__outlier_6(c *check.C) { "02 03", "09 16") vt.Diagnostics( - "NOTE: ~/Makefile:1: This variable value should be aligned with tabs, not spaces, to column 17.") + "NOTE: Makefile:1: This variable value should be aligned with tabs, not spaces, to column 17.") vt.Autofixes( - "AUTOFIX: ~/Makefile:1: Replacing \" \" with \"\\t\\t\".") + "AUTOFIX: Makefile:1: Replacing \" \" with \"\\t\\t\".") vt.Fixed( "V= value", "V.000010= value") @@ -1724,7 +1749,7 @@ func (s *Suite) Test_VaralignBlock__outlier_6(c *check.C) { } // The long line is not an outlier but very close. One more space, and it would count. -func (s *Suite) Test_VaralignBlock__outlier_10(c *check.C) { +func (s *Suite) Test_VaralignBlock__var22_space_value_var9_tab_value(c *check.C) { vt := NewVaralignTester(s, c) vt.Input( "V.0000000000000000023= value", // Adjust from 23 to 24 (+ 1 tab) @@ -1733,18 +1758,18 @@ func (s *Suite) Test_VaralignBlock__outlier_10(c *check.C) { "22 23", "09 16") vt.Diagnostics( - "NOTE: ~/Makefile:1: This variable value should be aligned with tabs, not spaces, to column 25.", - "NOTE: ~/Makefile:2: This variable value should be aligned to column 25.") + "NOTE: Makefile:1: This variable value should be aligned with tabs, not spaces, to column 25.", + "NOTE: Makefile:2: This variable value should be aligned to column 25.") vt.Autofixes( - "AUTOFIX: ~/Makefile:1: Replacing \" \" with \"\\t\".", - "AUTOFIX: ~/Makefile:2: Replacing \"\\t\" with \"\\t\\t\".") + "AUTOFIX: Makefile:1: Replacing \" \" with \"\\t\".", + "AUTOFIX: Makefile:2: Replacing \"\\t\" with \"\\t\\t\".") vt.Fixed( "V.0000000000000000023= value", "V.000010= value") vt.Run() } -func (s *Suite) Test_VaralignBlock__outlier_11(c *check.C) { +func (s *Suite) Test_VaralignBlock__var23_space_value_var9_tab_value(c *check.C) { vt := NewVaralignTester(s, c) vt.Input( "V.00000000000000000024= value", // Keep at 24 (space to tab) @@ -1753,18 +1778,18 @@ func (s *Suite) Test_VaralignBlock__outlier_11(c *check.C) { "23 24", "09 16") vt.Diagnostics( - "NOTE: ~/Makefile:1: Variable values should be aligned with tabs, not spaces.", - "NOTE: ~/Makefile:2: This variable value should be aligned to column 25.") + "NOTE: Makefile:1: Variable values should be aligned with tabs, not spaces.", + "NOTE: Makefile:2: This variable value should be aligned to column 25.") vt.Autofixes( - "AUTOFIX: ~/Makefile:1: Replacing \" \" with \"\\t\".", - "AUTOFIX: ~/Makefile:2: Replacing \"\\t\" with \"\\t\\t\".") + "AUTOFIX: Makefile:1: Replacing \" \" with \"\\t\".", + "AUTOFIX: Makefile:2: Replacing \"\\t\" with \"\\t\\t\".") vt.Fixed( "V.00000000000000000024= value", "V.000010= value") vt.Run() } -func (s *Suite) Test_VaralignBlock__outlier_12(c *check.C) { +func (s *Suite) Test_VaralignBlock__var24_space_value_var9_tab_value(c *check.C) { vt := NewVaralignTester(s, c) vt.Input( "V.000000000000000000025= value", // Keep at 25 (outlier) @@ -1782,7 +1807,7 @@ func (s *Suite) Test_VaralignBlock__outlier_12(c *check.C) { // When the lines are indented inconsistently, the indentation is reduced // to the required minimum. -func (s *Suite) Test_VaralignBlock__outlier_14(c *check.C) { +func (s *Suite) Test_VaralignBlock__var_tabs24_value_var_tabs40_value(c *check.C) { vt := NewVaralignTester(s, c) vt.Input( "V.00008=\t\tvalue", // Adjust from 24 to 16 (removes 1 tab) @@ -1791,18 +1816,18 @@ func (s *Suite) Test_VaralignBlock__outlier_14(c *check.C) { "08 24", "08 40") 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.") + "NOTE: Makefile:1: This variable value should be aligned to column 17.", + "NOTE: Makefile:2: This variable value should be aligned to column 17.") vt.Autofixes( - "AUTOFIX: ~/Makefile:1: Replacing \"\\t\\t\" with \"\\t\".", - "AUTOFIX: ~/Makefile:2: Replacing \"\\t\\t\\t\\t\" with \"\\t\".") + "AUTOFIX: Makefile:1: Replacing \"\\t\\t\" with \"\\t\".", + "AUTOFIX: Makefile:2: Replacing \"\\t\\t\\t\\t\" with \"\\t\".") vt.Fixed( "V.00008= value", "V.00008= value") vt.Run() } -func (s *Suite) Test_VaralignBlock__outlier_with_several_spaces(c *check.C) { +func (s *Suite) Test_VaralignBlock__var_tab8_value_var29_spaces32_value(c *check.C) { vt := NewVaralignTester(s, c) vt.Input( "SHORT=\tvalue", @@ -1811,16 +1836,16 @@ func (s *Suite) Test_VaralignBlock__outlier_with_several_spaces(c *check.C) { "06 08", "29 32") vt.Diagnostics( - "NOTE: ~/Makefile:2: This outlier variable value should be aligned with a single space.") + "NOTE: Makefile:2: This outlier variable value should be aligned with a single space.") vt.Autofixes( - "AUTOFIX: ~/Makefile:2: Replacing \" \" with \" \".") + "AUTOFIX: Makefile:2: Replacing \" \" with \" \".") vt.Fixed( "SHORT= value", "VERY_VERY_LONG_VARIABLE_NAME= value") vt.Run() } -func (s *Suite) Test_VaralignBlock__single_space_in_short_line(c *check.C) { +func (s *Suite) Test_VaralignBlock__var6_space_value_var10_tab_value(c *check.C) { vt := NewVaralignTester(s, c) vt.Input( "SHORT= value", @@ -1829,16 +1854,16 @@ func (s *Suite) Test_VaralignBlock__single_space_in_short_line(c *check.C) { "06 07", "10 16") vt.Diagnostics( - "NOTE: ~/Makefile:1: This variable value should be aligned with tabs, not spaces, to column 17.") + "NOTE: Makefile:1: This variable value should be aligned with tabs, not spaces, to column 17.") vt.Autofixes( - "AUTOFIX: ~/Makefile:1: Replacing \" \" with \"\\t\\t\".") + "AUTOFIX: Makefile:1: Replacing \" \" with \"\\t\\t\".") vt.Fixed( "SHORT= value", "LONG_NAME= value") vt.Run() } -func (s *Suite) Test_VaralignBlock__outlier_without_space(c *check.C) { +func (s *Suite) Test_VaralignBlock__var6_tab_value_var21_none_value(c *check.C) { vt := NewVaralignTester(s, c) vt.Input( "SHORT=\tvalue", @@ -1847,9 +1872,9 @@ func (s *Suite) Test_VaralignBlock__outlier_without_space(c *check.C) { "06 08", "21 21") vt.Diagnostics( - "NOTE: ~/Makefile:2: This outlier variable value should be aligned with a single space.") + "NOTE: Makefile:2: This outlier variable value should be aligned with a single space.") vt.Autofixes( - "AUTOFIX: ~/Makefile:2: Replacing \"\" with \" \".") + "AUTOFIX: Makefile:2: Replacing \"\" with \" \".") vt.Fixed( "SHORT= value", "LONG.678901234567890= value") @@ -1858,9 +1883,10 @@ func (s *Suite) Test_VaralignBlock__outlier_without_space(c *check.C) { // The INSTALLATION_DIRS line is so long that it is considered an outlier, // since compared to the DIST line, it is at least two tabs away. +// // Pkglint before 2018-01-26 suggested that it "should be aligned to column 9", // which is not possible since the variable name is already longer. -func (s *Suite) Test_VaralignBlock__long_short(c *check.C) { +func (s *Suite) Test_VaralignBlock__var_tab24_value_var_tab8_value(c *check.C) { vt := NewVaralignTester(s, c) vt.Input( "INSTALLATION_DIRS=\tbin", @@ -1885,14 +1911,12 @@ func (s *Suite) Test_VaralignBlock__long_short(c *check.C) { // FIXME: The definition of an outlier should be based on the actual indentation, // not on the minimum indentation. Or maybe even better on the corrected indentation. // In the below paragraph, the outlier is not indented enough to qualify as a visual outlier. -func (s *Suite) Test_VaralignBlock__tabbed_outlier(c *check.C) { +func (s *Suite) Test_VaralignBlock__var_tab32_value_var_tabs24_value_var_tabs24_value(c *check.C) { vt := NewVaralignTester(s, c) vt.Input( - ".if !empty(PKG_OPTIONS:Minspircd-sqloper)", - "INSPIRCD_STORAGE_DRIVER?=\tmysql", - "MODULES+=\t\tm_sqloper.cpp m_sqlutils.cpp", - "HEADERS+=\t\tm_sqlutils.h", - ".endif") + "INSPIRCD_STORAGE_DRIVER?=\tvalue", + "MODULES+=\t\tvalue", + "HEADERS+=\t\tvalue") vt.Internals( "25 32", "09 24", @@ -1900,11 +1924,9 @@ func (s *Suite) Test_VaralignBlock__tabbed_outlier(c *check.C) { vt.Diagnostics() vt.Autofixes() vt.Fixed( - ".if !empty(PKG_OPTIONS:Minspircd-sqloper)", - "INSPIRCD_STORAGE_DRIVER?= mysql", - "MODULES+= m_sqloper.cpp m_sqlutils.cpp", - "HEADERS+= m_sqlutils.h", - ".endif") + "INSPIRCD_STORAGE_DRIVER?= value", + "MODULES+= value", + "HEADERS+= value") vt.Run() } @@ -1913,15 +1935,15 @@ func (s *Suite) Test_VaralignBlock__tabbed_outlier(c *check.C) { // // TODO: Make this rule more general: if the indentation of the continuation // lines is more than the initial line, it is intentional. -func (s *Suite) Test_VaralignBlock__indented_continuation_line(c *check.C) { +func (s *Suite) Test_VaralignBlock__var_tab24_value_space_cont_tabs32_value_space_cont_tabs32_value(c *check.C) { vt := NewVaralignTester(s, c) vt.Input( "CONF_FILES_PERMS=\tsource \\", "\t\t\t\tdestination \\", "\t\t\t\tuser group 0644") vt.Internals( - "17 24", - " 32", + "17 24 31", + " 32 44", " 32") vt.Diagnostics() vt.Autofixes() @@ -1932,7 +1954,7 @@ func (s *Suite) Test_VaralignBlock__indented_continuation_line(c *check.C) { vt.Run() } -func (s *Suite) Test_VaralignBlock__indented_continuation_line_in_paragraph(c *check.C) { +func (s *Suite) Test_VaralignBlock__var_tabs24_value_var_tab24_value_var_space_cont_tab_value_space_cont_tab_value_space_cont_tab_value(c *check.C) { vt := NewVaralignTester(s, c) vt.Input( "SUBST_CLASSES+=\t\tfix", @@ -1944,14 +1966,14 @@ func (s *Suite) Test_VaralignBlock__indented_continuation_line_in_paragraph(c *c vt.Internals( "15 24", "16 24", - "14 15", - " 08", - " 08", + "14 15 15", + " 08 23", + " 08 23", " 08") vt.Diagnostics( - "NOTE: ~/Makefile:3: This variable value should be aligned with tabs, not spaces, to column 25.") + "NOTE: Makefile:3: This variable value should be aligned with tabs, not spaces, to column 25.") vt.Autofixes( - "AUTOFIX: ~/Makefile:3: Replacing \" \" with \"\\t\\t\".") + "AUTOFIX: Makefile:3: Replacing \" \" with \"\\t\\t\".") vt.Fixed( "SUBST_CLASSES+= fix", "SUBST_STAGE.fix= post-patch", @@ -1962,6 +1984,24 @@ func (s *Suite) Test_VaralignBlock__indented_continuation_line_in_paragraph(c *c vt.Run() } +func (s *Suite) Test_VaralignBlock__lead_var_tab8_value_lead_var_tab16_value(c *check.C) { + vt := NewVaralignTester(s, c) + vt.Input( + "#VAR=\tvalue", + "#VAR.param=\tvalue") + vt.Internals( + "05 08", + "11 16") + vt.Diagnostics( + "NOTE: Makefile:1: This variable value should be aligned to column 17.") + vt.Autofixes( + "AUTOFIX: Makefile:1: Replacing \"\\t\" with \"\\t\\t\".") + vt.Fixed( + "#VAR= value", + "#VAR.param= value") + vt.Run() +} + // Up to 2018-01-27, it could happen that some source code was logged // without a corresponding diagnostic. This was unintended and confusing. func (s *Suite) Test_VaralignBlock__fix_without_diagnostic(c *check.C) { @@ -1974,9 +2014,9 @@ func (s *Suite) Test_VaralignBlock__fix_without_diagnostic(c *check.C) { "\t\t\tRUBY_NAME=${RUBY_NAME:Q}") vt.Internals( "15 24", - "13 24", - " 24", - " 24", + "13 24 57", + " 24 61", + " 24 63", " 24") vt.Diagnostics() vt.Autofixes() @@ -1996,7 +2036,6 @@ func (s *Suite) Test_VaralignBlock__fix_without_diagnostic(c *check.C) { func (s *Suite) Test_VaralignBlock__continuation_line_last_empty(c *check.C) { vt := NewVaralignTester(s, c) vt.Input( - // FIXME: Add a test for MkParser to warn about this apparently empty line. "DISTFILES= \\", "\ta \\", "\tb \\", @@ -2004,16 +2043,16 @@ func (s *Suite) Test_VaralignBlock__continuation_line_last_empty(c *check.C) { "", // This is the final line of the variable assignment. "NEXT_VAR=\tsecond line") vt.Internals( - "10 11", - " 08", - " 08", - " 08", + "10 11 11", + " 08 10", + " 08 10", + " 08 10", " 00", "09 16") vt.Diagnostics( - "NOTE: ~/Makefile:1: This variable value should be aligned with tabs, not spaces, to column 17.") + "NOTE: Makefile:1: This variable value should be aligned with tabs, not spaces, to column 17.") vt.Autofixes( - "AUTOFIX: ~/Makefile:1: Replacing \" \" with \"\\t\".") + "AUTOFIX: Makefile:1: Replacing \" \" with \"\\t\".") vt.Fixed( "DISTFILES= \\", " a \\", @@ -2042,21 +2081,21 @@ func (s *Suite) Test_VaralignBlock__realign_commented_single_lines(c *check.C) { vt.Internals( "06 08", "11 16", - "14 15", + "14 15 15", " 16", - "16 17", + "16 17 17", " 01", "06 08") vt.Diagnostics( - "NOTE: ~/Makefile:1: This variable value should be aligned to column 17.", - "NOTE: ~/Makefile:3: This variable value should be aligned with tabs, not spaces, to column 17.", - "NOTE: ~/Makefile:6: This continuation line should be indented with \"\\t\\t\".", - "NOTE: ~/Makefile:7: This variable value should be aligned to column 17.") + "NOTE: Makefile:1: This variable value should be aligned to column 17.", + "NOTE: Makefile:3: This variable value should be aligned with tabs, not spaces, to column 17.", + "NOTE: Makefile:6: This continuation line should be indented with \"\\t\\t\".", + "NOTE: Makefile:7: This variable value should be aligned to column 17.") vt.Autofixes( - "AUTOFIX: ~/Makefile:1: Replacing \"\\t\" with \"\\t\\t\".", - "AUTOFIX: ~/Makefile:3: Replacing \" \" with \"\\t\".", - "AUTOFIX: ~/Makefile:6: Replacing \"\" with \"\\t\\t\".", - "AUTOFIX: ~/Makefile:7: Replacing \"\\t\" with \"\\t\\t\".") + "AUTOFIX: Makefile:1: Replacing \"\\t\" with \"\\t\\t\".", + "AUTOFIX: Makefile:3: Replacing \" \" with \"\\t\".", + "AUTOFIX: Makefile:6: Replacing \"\" with \"\\t\\t\".", + "AUTOFIX: Makefile:7: Replacing \"\\t\" with \"\\t\\t\".") vt.Fixed( "SHORT= value", "#DISTFILES= distfile-1.0.0.tar.gz", @@ -2083,15 +2122,15 @@ func (s *Suite) Test_VaralignBlock__realign_commented_continuation_line(c *check "AFTER=\tafter") vt.Internals( "07 08", - "11 12", - " 08", - " 08", - " 08", + "11 12 12", + " 08 15", + " 08 15", + " 08 15", " 00") vt.Diagnostics( - "NOTE: ~/Makefile:6: This continuation line should be indented with \"\\t\".") + "NOTE: Makefile:6: This continuation line should be indented with \"\\t\".") vt.Autofixes( - "AUTOFIX: ~/Makefile:6: Replacing \"\" with \"\\t\".") + "AUTOFIX: Makefile:6: Replacing \"\" with \"\\t\".") vt.Fixed( "BEFORE= value", "#COMMENTED= \\", @@ -2135,7 +2174,7 @@ func (s *Suite) Test_VaralignBlock__realign_commented_multiline(c *check.C) { "#CONF_FILES+=\t\tfile1 \\", "#\t\t\tfile2") vt.Internals( - "13 24", + "13 24 30", " 24") vt.Diagnostics() vt.Autofixes() @@ -2163,12 +2202,12 @@ func (s *Suite) Test_VaralignBlock__mixed_indentation(c *check.C) { " \t \t value2 continued") vt.Internals( "05 08", - "05 08", + "05 08 15", " 17") vt.Diagnostics( - "NOTE: ~/Makefile:3: This continuation line should be indented with \"\\t\".") + "NOTE: Makefile:3: This continuation line should be indented with \"\\t\".") vt.Autofixes( - "AUTOFIX: ~/Makefile:3: Replacing \" \\t \\t \" with \"\\t\\t \".") + "AUTOFIX: Makefile:3: Replacing \" \\t \\t \" with \"\\t\\t \".") vt.Fixed( "VAR1= value1", "VAR2= value2 \\", @@ -2211,7 +2250,7 @@ func (s *Suite) Test_VaralignBlock__follow_up_indentation(c *check.C) { "EGDIR=\t\t\t${PREFIX}/share/examples/rtunes") vt.Internals( "06 24", - "11 24", + "11 24 45", " 32", "06 24") vt.Diagnostics( @@ -2235,10 +2274,10 @@ func (s *Suite) Test_VaralignBlock__staircase(c *check.C) { "\t\t\t${PREFIX}/bin/my-cmd\t\t\t\t\\", "\t\t\t\t-options arg...") vt.Internals( - "12 16", - " 08", - " 16", - " 24", + "12 16 16", + " 08 72", + " 16 72", + " 24 72", " 32") vt.Diagnostics( nil...) @@ -2253,9 +2292,9 @@ func (s *Suite) Test_VaralignBlock__staircase(c *check.C) { vt.Run() } -// The follow-up lines may always start in column 9. -// This is used for long variable values, to prevent wrapping them -// into multiple lines. +// The follow-up lines may only start in column 9 if they are longer than +// 72 characters. Since this is not the case in this test, they are realigned +// to match the initial line. func (s *Suite) Test_VaralignBlock__command_with_arguments(c *check.C) { vt := NewVaralignTester(s, c) vt.Input( @@ -2264,19 +2303,25 @@ func (s *Suite) Test_VaralignBlock__command_with_arguments(c *check.C) { "\t-e s,a,b, \\", "\t-e s,a,b,") vt.Internals( - "20 21", - " 08", - " 08", + "20 21 31", + " 08 18", + " 08 18", " 08") vt.Diagnostics( - "NOTE: ~/Makefile:1: This variable value should be aligned with tabs, not spaces, to column 25.") + "NOTE: Makefile:1: This variable value should be aligned with tabs, not spaces, to column 25.", + "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\".") vt.Autofixes( - "AUTOFIX: ~/Makefile:1: Replacing \" \" with \"\\t\".") + "AUTOFIX: Makefile:1: Replacing \" \" with \"\\t\".", + "AUTOFIX: Makefile:2: Replacing \"\\t\" with \"\\t\\t\\t\".", + "AUTOFIX: Makefile:3: Replacing \"\\t\" with \"\\t\\t\\t\".", + "AUTOFIX: Makefile:4: Replacing \"\\t\" with \"\\t\\t\\t\".") vt.Fixed( "SED_REPLACEMENT_CMD= ${SED} -n \\", - " -e s,a,b, \\", - " -e s,a,b, \\", - " -e s,a,b,") + " -e s,a,b, \\", + " -e s,a,b, \\", + " -e s,a,b,") vt.Run() } @@ -2299,45 +2344,84 @@ func (s *Suite) Test_VaralignBlock__empty_value(c *check.C) { vt.Run() } -func (s *Suite) Test_VaralignBlock_Process__autofix(c *check.C) { - t := s.Init(c) +func (s *Suite) Test_VaralignBlock_Process__var_spaces7_value(c *check.C) { + vt := NewVaralignTester(s, c) + vt.Input( + "VAR= value") + vt.Diagnostics( + "NOTE: Makefile:1: This variable value should be aligned with tabs, not spaces, to column 9.") + vt.Autofixes( + "AUTOFIX: Makefile:1: Replacing \" \" with \"\\t\".") + vt.Fixed( + "VAR= value") + vt.Run() +} - t.SetUpCommandLine("-Wspace", "--show-autofix") +func (s *Suite) Test_VaralignBlock_Process__var_spaces8_value(c *check.C) { + vt := NewVaralignTester(s, c) + vt.Input( + "VAR= value") + vt.Diagnostics( + "NOTE: Makefile:1: Variable values should be aligned with tabs, not spaces.") + vt.Autofixes( + "AUTOFIX: Makefile:1: Replacing \" \" with \"\\t\".") + vt.Fixed( + "VAR= value") + vt.Run() +} - mklines := t.NewMkLines("file.mk", - "VAR= value", // Indentation 7, fixed to 8. - "", // - "VAR= value", // Indentation 8, fixed to 8. - "", // - "VAR= value", // Indentation 9, fixed to 8. - "", // - "VAR= \tvalue", // Mixed indentation 8, fixed to 8. - "", // - "VAR= \tvalue", // Mixed indentation 8, fixed to 8. - "", // - "VAR= \tvalue", // Mixed indentation 16, fixed to 16. - "", // - "VAR=\tvalue") // Already aligned with tabs only, left unchanged. +func (s *Suite) Test_VaralignBlock_Process__var_spaces9_value(c *check.C) { + vt := NewVaralignTester(s, c) + vt.Input( + "VAR= value") + vt.Diagnostics( + "NOTE: Makefile:1: This variable value should be aligned with tabs, not spaces, to column 9.") + vt.Autofixes( + "AUTOFIX: Makefile:1: Replacing \" \" with \"\\t\".") + vt.Fixed( + "VAR= value") + vt.Run() +} - var varalign VaralignBlock - for _, line := range mklines.mklines { - varalign.Process(line) - } - varalign.Finish() +func (s *Suite) Test_VaralignBlock_Process__var_st8_value(c *check.C) { + vt := NewVaralignTester(s, c) + vt.Input( + "VAR= \tvalue") + vt.Diagnostics( + "NOTE: Makefile:1: Variable values should be aligned with tabs, not spaces.") + vt.Autofixes( + "AUTOFIX: Makefile:1: Replacing \" \\t\" with \"\\t\".") + vt.Fixed( + "VAR= value") + vt.Run() +} - t.CheckOutputLines( - "NOTE: file.mk:1: This variable value should be aligned with tabs, not spaces, to column 9.", - "AUTOFIX: file.mk:1: Replacing \" \" with \"\\t\".", - "NOTE: file.mk:3: Variable values should be aligned with tabs, not spaces.", - "AUTOFIX: file.mk:3: Replacing \" \" with \"\\t\".", - "NOTE: file.mk:5: This variable value should be aligned with tabs, not spaces, to column 9.", - "AUTOFIX: file.mk:5: Replacing \" \" with \"\\t\".", - "NOTE: file.mk:7: Variable values should be aligned with tabs, not spaces.", - "AUTOFIX: file.mk:7: Replacing \" \\t\" with \"\\t\".", - "NOTE: file.mk:9: Variable values should be aligned with tabs, not spaces.", - "AUTOFIX: file.mk:9: Replacing \" \\t\" with \"\\t\".", - "NOTE: file.mk:11: Variable values should be aligned with tabs, not spaces.", - "AUTOFIX: file.mk:11: Replacing \" \\t\" with \"\\t\\t\".") +func (s *Suite) Test_VaralignBlock_Process__var_ssst8_value(c *check.C) { + vt := NewVaralignTester(s, c) + vt.Input( + "VAR= \tvalue") + vt.Diagnostics( + "NOTE: Makefile:1: Variable values should be aligned with tabs, not spaces.") + vt.Autofixes( + "AUTOFIX: Makefile:1: Replacing \" \\t\" with \"\\t\".") + vt.Fixed( + "VAR= value") + vt.Run() +} + +// Since the variable is visually aligned at column 17, the alignment +// is kept at that depth, although a smaller indentation were possible. +func (s *Suite) Test_VaralignBlock_Process__var_sssst16_value(c *check.C) { + vt := NewVaralignTester(s, c) + vt.Input( + "VAR= \tvalue") + vt.Diagnostics( + "NOTE: Makefile:1: Variable values should be aligned with tabs, not spaces.") + vt.Autofixes( + "AUTOFIX: Makefile:1: Replacing \" \\t\" with \"\\t\\t\".") + vt.Fixed( + "VAR= value") + vt.Run() } // When the lines of a paragraph are inconsistently aligned, @@ -2370,26 +2454,28 @@ func (s *Suite) Test_VaralignBlock_Process__reduce_indentation(c *check.C) { // name and the value. Even if it is the longest line, and even if the value would start // exactly at a tab stop. func (s *Suite) Test_VaralignBlock_Process__longest_line_no_space(c *check.C) { - t := s.Init(c) - - t.SetUpCommandLine("-Wspace") - mklines := t.NewMkLines("file.mk", - "SUBST_CLASSES+= aaaaaaaa", - "SUBST_STAGE.aaaaaaaa= pre-configure", - "SUBST_FILES.aaaaaaaa= *.pl", - "SUBST_FILTER_CMD.aaaaaa=cat") - - var varalign VaralignBlock - for _, mkline := range mklines.mklines { - varalign.Process(mkline) - } - varalign.Finish() - - t.CheckOutputLines( - "NOTE: file.mk:1: This variable value should be aligned with tabs, not spaces, to column 33.", - "NOTE: file.mk:2: This variable value should be aligned with tabs, not spaces, to column 33.", - "NOTE: file.mk:3: This variable value should be aligned with tabs, not spaces, to column 33.", - "NOTE: file.mk:4: This variable value should be aligned to column 33.") + vt := NewVaralignTester(s, c) + vt.Input( + "SUBST_CLASSES+= 123456", + "SUBST_STAGE.123456= pre-configure", + "SUBST_FILES.123456= *.pl", + "SUBST_FILTER_CMD.123456=cat") + vt.Diagnostics( + "NOTE: Makefile:1: This variable value should be aligned with tabs, not spaces, to column 33.", + "NOTE: Makefile:2: This variable value should be aligned with tabs, not spaces, to column 33.", + "NOTE: Makefile:3: This variable value should be aligned with tabs, not spaces, to column 33.", + "NOTE: Makefile:4: This variable value should be aligned to column 33.") + vt.Autofixes( + "AUTOFIX: Makefile:1: Replacing \" \" with \"\\t\\t\\t\".", + "AUTOFIX: Makefile:2: Replacing \" \" with \"\\t\\t\".", + "AUTOFIX: Makefile:3: Replacing \" \" with \"\\t\\t\".", + "AUTOFIX: Makefile:4: Replacing \"\" with \"\\t\".") + vt.Fixed( + "SUBST_CLASSES+= 123456", + "SUBST_STAGE.123456= pre-configure", + "SUBST_FILES.123456= *.pl", + "SUBST_FILTER_CMD.123456= cat") + vt.Run() } func (s *Suite) Test_VaralignBlock_Process__only_spaces(c *check.C) { @@ -2414,6 +2500,23 @@ func (s *Suite) Test_VaralignBlock_Process__only_spaces(c *check.C) { "NOTE: file.mk:4: This variable value should be aligned with tabs, not spaces, to column 33.") } +func (s *Suite) Test_VaralignBlock_processVarassign__comment_with_continuation(c *check.C) { + vt := NewVaralignTester(s, c) + vt.Input( + "VAR.param= # comment \\", + "#\tthe comment continues") + vt.Diagnostics( + "NOTE: Makefile:1: This variable value should be aligned with tabs, not spaces, to column 17.", + "NOTE: Makefile:2: This continuation line should be indented with \"\\t\\t\".") + vt.Autofixes( + "AUTOFIX: Makefile:1: Replacing \" \" with \"\\t\".", + "AUTOFIX: Makefile:2: Replacing \"\\t\" with \"\\t\\t\".") + vt.Fixed( + "VAR.param= # comment \\", + "# the comment continues") + vt.Run() +} + func (s *Suite) Test_VaralignBlock_realignMultiEmptyInitial(c *check.C) { t := s.Init(c) @@ -2426,7 +2529,7 @@ func (s *Suite) Test_VaralignBlock_realignMultiEmptyInitial(c *check.C) { mklines.Check() t.CheckOutputLines( - "NOTE: filename.mk:3: This outlier variable value should be aligned with a single space.") + "NOTE: filename.mk:3: The continuation backslash should be preceded by a single space or tab.") } func (s *Suite) Test_VaralignBlock_realignMultiEmptyInitial__spaces(c *check.C) { @@ -2437,15 +2540,15 @@ func (s *Suite) Test_VaralignBlock_realignMultiEmptyInitial__spaces(c *check.C) // This line is necessary to trigger the realignment; see VaralignBlock.Finish. "VAR= value") vt.Internals( - "04 08", + "04 08 08", " 08", "04 05") vt.Diagnostics( - "NOTE: ~/Makefile:1: Variable values should be aligned with tabs, not spaces.", - "NOTE: ~/Makefile:3: This variable value should be aligned with tabs, not spaces, to column 9.") + "NOTE: Makefile:1: Variable values should be aligned with tabs, not spaces.", + "NOTE: Makefile:3: This variable value should be aligned with tabs, not spaces, to column 9.") vt.Autofixes( - "AUTOFIX: ~/Makefile:1: Replacing \" \" with \"\\t\".", - "AUTOFIX: ~/Makefile:3: Replacing \" \" with \"\\t\".") + "AUTOFIX: Makefile:1: Replacing \" \" with \"\\t\".", + "AUTOFIX: Makefile:3: Replacing \" \" with \"\\t\".") vt.Fixed( "VAR= \\", " value", @@ -2459,14 +2562,14 @@ func (s *Suite) Test_VaralignBlock_realignMultiInitial__spaces(c *check.C) { "VAR= value1 \\", " value2") vt.Internals( - "04 08", + "04 08 15", " 08") vt.Diagnostics( - "NOTE: ~/Makefile:1: Variable values should be aligned with tabs, not spaces.", - "NOTE: ~/Makefile:2: This continuation line should be indented with \"\\t\".") + "NOTE: Makefile:1: Variable values should be aligned with tabs, not spaces.", + "NOTE: Makefile:2: This continuation line should be indented with \"\\t\".") vt.Autofixes( - "AUTOFIX: ~/Makefile:1: Replacing \" \" with \"\\t\".", - "AUTOFIX: ~/Makefile:2: Replacing \" \" with \"\\t\".") + "AUTOFIX: Makefile:1: Replacing \" \" with \"\\t\".", + "AUTOFIX: Makefile:2: Replacing \" \" with \"\\t\".") vt.Fixed( "VAR= value1 \\", " value2") @@ -2489,27 +2592,27 @@ func (s *Suite) Test_VaralignBlock_realignMultiEmptyFollow(c *check.C) { "\\", "# comment") vt.Internals( - "04 05", - " 08", - " 10", - " 06", - " 00", - " 00", + "04 05 05", + " 08 15", + " 10 17", + " 06 13", + " 00 07", + " 00 00", " 00") vt.Diagnostics( - "NOTE: ~/Makefile:2: This continuation line should be indented with \"\\t\".", - "NOTE: ~/Makefile:3: This continuation line should be indented with \"\\t \".", - "NOTE: ~/Makefile:4: This continuation line should be indented with \"\\t\".", - "NOTE: ~/Makefile:5: This continuation line should be indented with \"\\t\".", - "NOTE: ~/Makefile:6: This continuation line should be indented with \"\\t\".", - "NOTE: ~/Makefile:7: This continuation line should be indented with \"\\t\".") + "NOTE: Makefile:2: This continuation line should be indented with \"\\t\".", + "NOTE: Makefile:3: This continuation line should be indented with \"\\t \".", + "NOTE: Makefile:4: This continuation line should be indented with \"\\t\".", + "NOTE: Makefile:5: This continuation line should be indented with \"\\t\".", + "NOTE: Makefile:6: This continuation line should be indented with \"\\t\".", + "NOTE: Makefile:7: This continuation line should be indented with \"\\t\".") vt.Autofixes( - "AUTOFIX: ~/Makefile:2: Replacing \" \" with \"\\t\".", - "AUTOFIX: ~/Makefile:3: Replacing \" \" with \"\\t \".", - "AUTOFIX: ~/Makefile:4: Replacing \" \" with \"\\t\".", - "AUTOFIX: ~/Makefile:5: Replacing \"\" with \"\\t\".", - "AUTOFIX: ~/Makefile:6: Replacing \"\" with \"\\t\".", - "AUTOFIX: ~/Makefile:7: Replacing \"\" with \"\\t\".") + "AUTOFIX: Makefile:2: Replacing \" \" with \"\\t\".", + "AUTOFIX: Makefile:3: Replacing \" \" with \"\\t \".", + "AUTOFIX: Makefile:4: Replacing \" \" with \"\\t\".", + "AUTOFIX: Makefile:5: Replacing \"\" with \"\\t\".", + "AUTOFIX: Makefile:6: Replacing \"\" with \"\\t\".", + "AUTOFIX: Makefile:7: Replacing \"\" with \"\\t\".") vt.Fixed( "VAR= \\", " value1 \\", @@ -2521,22 +2624,220 @@ func (s *Suite) Test_VaralignBlock_realignMultiEmptyFollow(c *check.C) { vt.Run() } -func (s *Suite) Test_VaralignBlock_split(c *check.C) { +// It's ok to have all backslashes in the same column, even if that column +// is not 73. +func (s *Suite) Test_VaralignBlock__continuation_backslashes_aligned(c *check.C) { + vt := NewVaralignTester(s, c) + vt.Input( + "VAR=\tvalue value value\t\\", + "\tvalue\t\t\t\\", + "\tvalue\t\t\t\\", + "\tvalue") + vt.Internals( + "04 08 32", + " 08 32", + " 08 32", + " 08") + vt.Diagnostics( + nil...) + vt.Autofixes( + nil...) + vt.Fixed( + "VAR= value value value \\", + " value \\", + " value \\", + " value") + vt.Run() +} + +// The first line is indented with a single tab. This looks strange but +// pkglint considers it acceptable since there is a simple rule saying +// "a single tab is always ok". Any rule that would replace this simple +// rule would have to be similarly simple and intuitive. +func (s *Suite) Test_VaralignBlock__continuation_backslashes_aligned_except_initial(c *check.C) { + vt := NewVaralignTester(s, c) + vt.Input( + "VAR=\tvalue value value\t\\", + "\tvalue\t\t\t\t\\", + "\tvalue\t\t\t\t\\", + "\tvalue") + vt.Internals( + "04 08 32", + " 08 40", + " 08 40", + " 08") + vt.Diagnostics( + nil...) + vt.Autofixes( + nil...) + // TODO: The backslashes should be aligned by a _simple_ rule. + vt.Fixed( + "VAR= value value value \\", + " value \\", + " value \\", + " value") + vt.Run() +} + +// Lines whose continuation backslash is indented by a single space are +// usually those that stick out further than column 73. These are not +// touched by the realignment. +func (s *Suite) Test_VaralignBlock__continuation_backslashes_one_sticks_out(c *check.C) { + vt := NewVaralignTester(s, c) + vt.Input( + "VAR=\tvalue\t\\", + "\tvalue value value \\", + "\tvalue\t\\", + "\tvalue") + vt.Internals( + "04 08 16", + " 08 26", + " 08 16", + " 08") + vt.Diagnostics( + nil...) + vt.Autofixes( + nil...) + vt.Fixed( + "VAR= value \\", + " value value value \\", + " value \\", + " value") + vt.Run() +} + +func (s *Suite) Test_VaralignBlock__initial_value_tab80(c *check.C) { + vt := NewVaralignTester(s, c) + vt.Input( + "VVVVVVVVVVVVVVVVVVV=\tvalue\t\t\t\t\t\t\t\\", + "\t\t\tvalue\t\t\t\t\t\t\\", + "\t\t\tvalue\t\t\t\t\t\t\\", + "\t\t\tvalue") + vt.Internals( + "20 24 80", + " 24 72", + " 24 72", + " 24") + vt.Diagnostics( + "NOTE: Makefile:1: The continuation backslash should be preceded " + + "by a single space or tab, or be in column 73, not 81.") + vt.Autofixes( + "AUTOFIX: Makefile:1: Replacing \"\\t\\t\\t\\t\\t\\t\\t\" with \"\\t\\t\\t\\t\\t\\t\".") + vt.Fixed( + "VVVVVVVVVVVVVVVVVVV= value \\", + " value \\", + " value \\", + " value") + vt.Run() +} + +func (s *Suite) Test_VaralignBlock__long_lines(c *check.C) { + vt := NewVaralignTester(s, c) + vt.Input( + "VAR=\t\t\t\t\t\tvalue\t\t \\", + "\tvalue \t \\", + "\tvalue") + vt.Internals( + "04 48 65", + " 08 17", + " 08") + vt.Diagnostics( + "NOTE: Makefile:1: The continuation backslash should be preceded "+ + "by a single space or tab, or be in column 57, not 66.", + "NOTE: Makefile:2: This continuation line should be indented with \"\\t\\t\\t\\t\\t\\t\".", + "NOTE: Makefile:3: This continuation line should be indented with \"\\t\\t\\t\\t\\t\\t\".") + vt.Autofixes( + "AUTOFIX: Makefile:1: Replacing \"\\t\\t \" with \"\\t\".", + "AUTOFIX: Makefile:2: Replacing \"\\t\" with \"\\t\\t\\t\\t\\t\\t\".", + "AUTOFIX: Makefile:3: Replacing \"\\t\" with \"\\t\\t\\t\\t\\t\\t\".") + vt.Fixed( + "VAR= value \\", + // FIXME: The backslash should be aligned properly. + " value \\", + " value") + vt.Run() +} + +// A practical chaotic test case, derived from wip/compat32_mit-krb5/Makefile. +// It made pkglint before 2019-09-03 panic. +func (s *Suite) Test_VaralignBlock__long_lines_2(c *check.C) { + vt := NewVaralignTester(s, c) + vt.Input( + "INSTALLATION_DIRS=\t_____________________________________________________________________ \\"+ + "\t\t\t\t __________________________________________________________\t\t \\", + "\t\t\t__________________________________________________________\t\t \t\\", + "\t\t\t__________________________________________________________\t\t \t\\", + "\t\t\t_________________________") + vt.Internals( + "18 24 201", + " 24 104", + " 24 104", + " 24") + vt.Diagnostics( + "NOTE: Makefile:1: The continuation backslash should be preceded by a single space or tab.") + vt.Autofixes( + "AUTOFIX: Makefile:1: Replacing \"\\t\\t \" with \" \".") + vt.Fixed( + "INSTALLATION_DIRS= _____________________________________________________________________ \\"+ + " __________________________________________________________ \\", + " __________________________________________________________ \\", + " __________________________________________________________ \\", + " _________________________") + vt.Run() +} + +// I've never seen an intentionally continued comment in practice, +// but pkglint needs to be able to handle this situation anyway. +func (s *Suite) Test_varalignParts_spaceBeforeContinuation__continued_comment(c *check.C) { + vt := NewVaralignTester(s, c) + vt.Input( + "VAR=\tvalue # comment \\", + "\tstill comment \\", + "\tand still") + vt.Internals( + "04 08 24", + " 08 22", + " 08") + vt.Diagnostics( + nil...) + vt.Autofixes( + nil...) + vt.Fixed( + "VAR= value # comment \\", + " still comment \\", + " and still") + vt.Run() +} + +func (s *Suite) Test_VaralignSplitter_split(c *check.C) { t := s.Init(c) - test := func(textnl string, initial bool, expected varalignSplitResult) { - actual := (&VaralignBlock{}).split(textnl, initial) + test := func(rawText string, initial bool, expected varalignParts) { + actual := NewVaralignSplitter().split(rawText, initial) t.CheckEquals(actual, expected) t.CheckEquals( actual.leadingComment+actual.varnameOp+ actual.spaceBeforeValue+actual.value+actual.spaceAfterValue+ actual.trailingComment+actual.spaceAfterComment+actual.continuation, - textnl) + rawText) } - test("VAR=value\n", true, - varalignSplitResult{ + // This case is prevented from occurring in practice by the guard + // code in VaralignBlock.processVarassign, see INCLUSION_GUARD_MK. + test("VAR=", true, + varalignParts{ + leadingComment: "", + varnameOp: "VAR=", + spaceBeforeValue: "", + value: "", + spaceAfterValue: "", + trailingComment: "", + spaceAfterComment: "", + continuation: ""}) + + test("VAR=value", true, + varalignParts{ leadingComment: "", varnameOp: "VAR=", spaceBeforeValue: "", @@ -2544,11 +2845,10 @@ func (s *Suite) Test_VaralignBlock_split(c *check.C) { spaceAfterValue: "", trailingComment: "", spaceAfterComment: "", - continuation: "\n", - }) + continuation: ""}) - test("#VAR=value\n", true, - varalignSplitResult{ + test("#VAR=value", true, + varalignParts{ leadingComment: "#", varnameOp: "VAR=", spaceBeforeValue: "", @@ -2556,11 +2856,10 @@ func (s *Suite) Test_VaralignBlock_split(c *check.C) { spaceAfterValue: "", trailingComment: "", spaceAfterComment: "", - continuation: "\n", - }) + continuation: ""}) - test("#VAR = value # comment \\\n", true, - varalignSplitResult{ + test("#VAR = value # comment \\", true, + varalignParts{ leadingComment: "#", varnameOp: "VAR =", spaceBeforeValue: " ", @@ -2568,11 +2867,10 @@ func (s *Suite) Test_VaralignBlock_split(c *check.C) { spaceAfterValue: " ", trailingComment: "# comment", spaceAfterComment: " ", - continuation: "\\\n", - }) + continuation: "\\"}) - test("VAR=value \\\n", true, - varalignSplitResult{ + test("VAR=value \\", true, + varalignParts{ leadingComment: "", varnameOp: "VAR=", spaceBeforeValue: "", @@ -2580,11 +2878,10 @@ func (s *Suite) Test_VaralignBlock_split(c *check.C) { spaceAfterValue: " ", trailingComment: "", spaceAfterComment: "", - continuation: "\\\n", - }) + continuation: "\\"}) - test("VAR=value # comment \\\n", true, - varalignSplitResult{ + test("VAR=value # comment \\", true, + varalignParts{ leadingComment: "", varnameOp: "VAR=", spaceBeforeValue: "", @@ -2592,11 +2889,10 @@ func (s *Suite) Test_VaralignBlock_split(c *check.C) { spaceAfterValue: " ", trailingComment: "# comment", spaceAfterComment: " ", - continuation: "\\\n", - }) + continuation: "\\"}) - test("VAR=value # comment \\\\\n", true, - varalignSplitResult{ + test("VAR=value # comment \\\\", true, + varalignParts{ leadingComment: "", varnameOp: "VAR=", spaceBeforeValue: "", @@ -2604,11 +2900,10 @@ func (s *Suite) Test_VaralignBlock_split(c *check.C) { spaceAfterValue: " ", trailingComment: "# comment \\\\", spaceAfterComment: "", - continuation: "\n", - }) + continuation: ""}) - test("VAR=\\# a [#] b # comment \\\\\n", true, - varalignSplitResult{ + test("VAR=\\# a [#] b # comment \\\\", true, + varalignParts{ leadingComment: "", varnameOp: "VAR=", spaceBeforeValue: "", @@ -2616,11 +2911,10 @@ func (s *Suite) Test_VaralignBlock_split(c *check.C) { spaceAfterValue: " ", trailingComment: "# comment \\\\", spaceAfterComment: "", - continuation: "\n", - }) + continuation: ""}) - test("VAR.${param:[#]}=\tvalue\n", true, - varalignSplitResult{ + test("VAR.${param:[#]}=\tvalue", true, + varalignParts{ leadingComment: "", varnameOp: "VAR.${param:[#]}=", spaceBeforeValue: "\t", @@ -2628,11 +2922,10 @@ func (s *Suite) Test_VaralignBlock_split(c *check.C) { spaceAfterValue: "", trailingComment: "", spaceAfterComment: "", - continuation: "\n", - }) + continuation: ""}) test("VAR=value", true, - varalignSplitResult{ + varalignParts{ leadingComment: "", varnameOp: "VAR=", spaceBeforeValue: "", @@ -2640,13 +2933,12 @@ func (s *Suite) Test_VaralignBlock_split(c *check.C) { spaceAfterValue: "", trailingComment: "", spaceAfterComment: "", - continuation: "", - }) + continuation: ""}) // Since this is a follow-up line, the text ends up in the variable // value, and varnameOp is necessarily empty. test("VAR=value", false, - varalignSplitResult{ + varalignParts{ leadingComment: "", varnameOp: "", spaceBeforeValue: "", @@ -2654,13 +2946,12 @@ func (s *Suite) Test_VaralignBlock_split(c *check.C) { spaceAfterValue: "", trailingComment: "", spaceAfterComment: "", - continuation: "", - }) + continuation: ""}) // In some edge cases the variable name is indented with ordinary spaces. // This must not lead to a panic. test(" VAR=value", true, - varalignSplitResult{ + varalignParts{ leadingComment: " ", varnameOp: "VAR=", spaceBeforeValue: "", @@ -2668,13 +2959,12 @@ func (s *Suite) Test_VaralignBlock_split(c *check.C) { spaceAfterValue: "", trailingComment: "", spaceAfterComment: "", - continuation: "", - }) + continuation: ""}) // And in really edgy cases, the leading space may even be followed by tabs. // This should not happen in practice since it is really confusing. test(" \t VAR=value", true, - varalignSplitResult{ + varalignParts{ leadingComment: " \t ", varnameOp: "VAR=", spaceBeforeValue: "", @@ -2682,11 +2972,10 @@ func (s *Suite) Test_VaralignBlock_split(c *check.C) { spaceAfterValue: "", trailingComment: "", spaceAfterComment: "", - continuation: "", - }) + continuation: ""}) test(" value", false, - varalignSplitResult{ + varalignParts{ leadingComment: "", varnameOp: "", spaceBeforeValue: " ", @@ -2694,14 +2983,13 @@ func (s *Suite) Test_VaralignBlock_split(c *check.C) { spaceAfterValue: "", trailingComment: "", spaceAfterComment: "", - continuation: "", - }) + continuation: ""}) // In practice it doesn't really happen that the last line of a file // ends in a backslash and at the same time it doesn't have the usual // newline ending. test(" value \\", false, - varalignSplitResult{ + varalignParts{ leadingComment: "", varnameOp: "", spaceBeforeValue: " ", @@ -2709,8 +2997,7 @@ func (s *Suite) Test_VaralignBlock_split(c *check.C) { spaceAfterValue: " ", trailingComment: "", spaceAfterComment: "", - continuation: "\\", - }) + continuation: "\\"}) // A follow-up line may start with a comment character. There are // two possible interpretations: @@ -2727,7 +3014,7 @@ func (s *Suite) Test_VaralignBlock_split(c *check.C) { // Any other character makes it a leading comment. test("#\tcomment", false, - varalignSplitResult{ + varalignParts{ leadingComment: "#", varnameOp: "", spaceBeforeValue: "\t", @@ -2735,11 +3022,10 @@ func (s *Suite) Test_VaralignBlock_split(c *check.C) { spaceAfterValue: "", trailingComment: "", spaceAfterComment: "", - continuation: "", - }) + continuation: ""}) test("#\tcomment \\", false, - varalignSplitResult{ + varalignParts{ leadingComment: "#", varnameOp: "", spaceBeforeValue: "\t", @@ -2747,11 +3033,10 @@ func (s *Suite) Test_VaralignBlock_split(c *check.C) { spaceAfterValue: " ", trailingComment: "", spaceAfterComment: "", - continuation: "\\", - }) + continuation: "\\"}) test("# comment", false, - varalignSplitResult{ + varalignParts{ leadingComment: "", varnameOp: "", spaceBeforeValue: "", @@ -2759,11 +3044,10 @@ func (s *Suite) Test_VaralignBlock_split(c *check.C) { spaceAfterValue: "", trailingComment: "# comment", spaceAfterComment: "", - continuation: "", - }) + continuation: ""}) test("# comment \\", false, - varalignSplitResult{ + varalignParts{ leadingComment: "", varnameOp: "", spaceBeforeValue: "", @@ -2771,8 +3055,7 @@ func (s *Suite) Test_VaralignBlock_split(c *check.C) { spaceAfterValue: "", trailingComment: "# comment", spaceAfterComment: " ", - continuation: "\\", - }) + continuation: "\\"}) // Commented variable assignments are only valid if they // directly follow the comment sign. @@ -2780,35 +3063,38 @@ func (s *Suite) Test_VaralignBlock_split(c *check.C) { // It is a programming error if such a line is ever added to // the VaralignBlock. t.ExpectAssert( - func() { test("# VAR= value", true, varalignSplitResult{}) }) + func() { test("# VAR= value", true, varalignParts{}) }) + + t.ExpectAssert( + func() { test("VAR=\tvalue\n", true, varalignParts{}) }) } // This test runs canonicalInitial directly since as of August 2019 // that function is only used in a single place, and from this place // varnameOpSpaceWidth is always bigger than width. -func (s *Suite) Test_varalignLine_canonicalInitial(c *check.C) { +func (s *Suite) Test_varalignParts_canonicalInitial(c *check.C) { t := s.Init(c) var v varalignLine - v.parts.varnameOp = "LONG.123456789=" - v.parts.spaceBeforeValue = " " + v.varnameOp = "LONG.123456789=" + v.spaceBeforeValue = " " t.CheckEquals(v.canonicalInitial(16), false) - v.parts.varnameOp = "LONG.1234567890=" + v.varnameOp = "LONG.1234567890=" t.CheckEquals(v.canonicalInitial(16), true) - v.parts.spaceBeforeValue = "" + v.spaceBeforeValue = "" t.CheckEquals(v.canonicalInitial(16), false) } -func (s *Suite) Test_varalignLine_canonicalFollow(c *check.C) { +func (s *Suite) Test_varalignParts_canonicalFollow(c *check.C) { t := s.Init(c) test := func(comment, space string, expected bool) { l := varalignLine{ - parts: varalignSplitResult{ + varalignParts: varalignParts{ leadingComment: comment, spaceBeforeValue: space}} @@ -2842,3 +3128,13 @@ func (s *Suite) Test_varalignLine_canonicalFollow(c *check.C) { test("#", " ", false) test("#", "\t", true) } + +// This constellation doesn't occur in practice because the code in +// VaralignBlock.processVarassign skips it, see INCLUSION_GUARD_MK. +func (s *Suite) Test_varalignParts_isEmptyContinuation__edge_case(c *check.C) { + t := s.Init(c) + + parts := NewVaralignSplitter().split("VAR=", true) + + t.CheckEquals(parts.isEmptyContinuation(), false) +} diff --git a/pkgtools/pkglint/files/vardefs.go b/pkgtools/pkglint/files/vardefs.go index 0f04f34758e..a7e71f923e4 100644 --- a/pkgtools/pkglint/files/vardefs.go +++ b/pkgtools/pkglint/files/vardefs.go @@ -311,11 +311,7 @@ func (reg *VarTypeRegistry) infralist(varname string, basicType *BasicType) { // compilerLanguages reads the available languages that are typically // bundled in a single compiler framework, such as GCC or Clang. func (reg *VarTypeRegistry) compilerLanguages(src *Pkgsrc) *BasicType { - options := NotEmpty - if !G.Testing { - options = NotEmpty | MustSucceed - } - mklines := LoadMk(src.File("mk/compiler.mk"), options) + mklines := src.LoadMkInfra("mk/compiler.mk", NotEmpty|MustSucceed) languages := make(map[string]bool) if mklines != nil { @@ -1141,6 +1137,7 @@ func (reg *VarTypeRegistry) Init(src *Pkgsrc) { reg.pkglistbl3rat("INCOMPAT_CURSES", BtMachinePlatformPattern) reg.sys("INFO_DIR", BtPathname) // relative to PREFIX reg.pkg("INFO_FILES", BtYes) + reg.sys("INFO_MSG", BtShellCommand) reg.sys("INSTALL", BtShellCommand) reg.pkglist("INSTALLATION_DIRS", BtPrefixPathname) reg.pkg("INSTALLATION_DIRS_FROM_PLIST", BtYes) @@ -1233,11 +1230,7 @@ func (reg *VarTypeRegistry) Init(src *Pkgsrc) { reg.pkglist("MASTER_SITES", BtFetchURL) for _, filename := range []string{"mk/fetch/sites.mk", "mk/fetch/fetch.mk"} { - loadOptions := NotEmpty | MustSucceed - if G.Testing { - loadOptions = NotEmpty - } - sitesMk := LoadMk(src.File(filename), loadOptions) + sitesMk := src.LoadMkInfra(filename, NotEmpty|MustSucceed) if sitesMk != nil { sitesMk.ForEach(func(mkline *MkLine) { if mkline.IsVarassign() && hasPrefix(mkline.Varname(), "MASTER_SITE_") { @@ -1682,6 +1675,7 @@ func (reg *VarTypeRegistry) Init(src *Pkgsrc) { reg.infralist("_SYS_VARS.*", BtVariableName) reg.infralist("_DEF_VARS.*", BtVariableName) reg.infralist("_USE_VARS.*", BtVariableName) + reg.infralist("_IGN_VARS.*", BtVariableName) reg.infralist("_SORTED_VARS.*", BtVariableNamePattern) reg.infralist("_LISTED_VARS.*", BtVariableNamePattern) } diff --git a/pkgtools/pkglint/files/vargroups.go b/pkgtools/pkglint/files/vargroups.go new file mode 100644 index 00000000000..8fc17242d0b --- /dev/null +++ b/pkgtools/pkglint/files/vargroups.go @@ -0,0 +1,246 @@ +package pkglint + +import "strings" + +// VargroupsChecker checks that the _VARGROUPS section of an infrastructure +// file matches the rest of the file content: +// +// - All variables that are used in the file should also be declared as used. +// +// - All variables that are declared to be used should actually be used. +// +// See mk/misc/show.mk, keyword _VARGROUPS. +type VargroupsChecker struct { + mklines *MkLines + skip bool + + registered map[string]*MkLine + userVars map[string]*MkLine + pkgVars map[string]*MkLine + sysVars map[string]*MkLine + defVars map[string]*MkLine + useVars map[string]*MkLine + ignVars map[string]*MkLine + sortedVars map[string]*MkLine + listedVars map[string]*MkLine + + undefinedVars map[string]*MkLine + unusedVars map[string]*MkLine +} + +func NewVargroupsChecker(mklines *MkLines) *VargroupsChecker { + ck := VargroupsChecker{mklines: mklines} + ck.init() + return &ck +} + +func (ck *VargroupsChecker) init() { + mklines := ck.mklines + scope := mklines.vars + if !scope.Defined("_VARGROUPS") { + ck.skip = true + return + } + + ck.registered = make(map[string]*MkLine) + ck.userVars = make(map[string]*MkLine) + ck.pkgVars = make(map[string]*MkLine) + ck.sysVars = make(map[string]*MkLine) + ck.defVars = make(map[string]*MkLine) + ck.useVars = make(map[string]*MkLine) + ck.ignVars = make(map[string]*MkLine) + ck.sortedVars = make(map[string]*MkLine) + ck.listedVars = make(map[string]*MkLine) + + group := "" + + checkGroupName := func(mkline *MkLine) { + varname := mkline.Varname() + if varnameParam(varname) != group { + mkline.Warnf("Expected %s.%s, but found %q.", + varnameBase(varname), group, varnameParam(varname)) + } + } + + appendTo := func(vars map[string]*MkLine, mkline *MkLine, public bool) { + checkGroupName(mkline) + + for _, varname := range mkline.ValueFields(mkline.Value()) { + if containsVarRef(varname) { + continue + } + + if public && hasPrefix(varname, "_") { + mkline.Warnf("%s should list only variables that start with a letter, not %q.", + mkline.Varname(), varname) + } + + if ck.registered[varname] != nil { + mkline.Warnf("Duplicate variable name %s, already appeared in %s.", + varname, mkline.RefTo(ck.registered[varname])) + } else { + ck.registered[varname] = mkline + } + + vars[varname] = mkline + } + } + + appendToStyle := func(vars map[string]*MkLine, mkline *MkLine) { + checkGroupName(mkline) + + for _, varname := range mkline.ValueFields(mkline.Value()) { + if !containsVarRef(varname) { + vars[varname] = mkline + } + } + } + + mklines.ForEach(func(mkline *MkLine) { + if mkline.IsVarassign() { + switch varnameCanon(mkline.Varname()) { + case "_VARGROUPS": + group = mkline.Value() + case "_USER_VARS.*": + appendTo(ck.userVars, mkline, true) + case "_PKG_VARS.*": + appendTo(ck.pkgVars, mkline, true) + case "_SYS_VARS.*": + appendTo(ck.sysVars, mkline, true) + case "_DEF_VARS.*": + appendTo(ck.defVars, mkline, false) + case "_USE_VARS.*": + appendTo(ck.useVars, mkline, false) + case "_IGN_VARS.*": + appendTo(ck.ignVars, mkline, false) + case "_SORTED_VARS.*": + appendToStyle(ck.sortedVars, mkline) + case "_LISTED_VARS.*": + appendToStyle(ck.listedVars, mkline) + } + } + }) + + ck.undefinedVars = copyStringMkLine(ck.defVars) + ck.unusedVars = copyStringMkLine(ck.useVars) +} + +// CheckVargroups checks that each variable that is used or defined +// somewhere in the file is also registered in the _VARGROUPS section, +// in order to make it discoverable by "bmake show-all". +// +// This check is intended mainly for infrastructure files and similar +// support files, such as lang/*/module.mk. +func (ck *VargroupsChecker) Check(mkline *MkLine) { + if ck.skip { + return + } + + ck.checkDef(mkline) + ck.checkUse(mkline) +} + +func (ck *VargroupsChecker) checkDef(mkline *MkLine) { + if !mkline.IsVarassignMaybeCommented() { + return + } + + varname := mkline.Varname() + delete(ck.undefinedVars, varname) + if ck.ignoreDef(varname) { + return + } + + if ck.mklines.once.FirstTimeSlice("_VARGROUPS", "def", varname) { + mkline.Warnf("Variable %s is defined but not mentioned in the _VARGROUPS section.", varname) + } +} + +func (ck *VargroupsChecker) ignoreDef(varname string) bool { + switch { + case containsVarRef(varname), + ck.registered[varname] != nil, + hasSuffix(varname, "_MK"), + ck.isVargroups(varname), + varname == "PKG_FAIL_REASON": + return true + } + + return false +} + +func (ck *VargroupsChecker) checkUse(mkline *MkLine) { + mkline.ForEachUsed(func(varUse *MkVarUse, _ VucTime) { ck.checkUseVar(mkline, varUse) }) +} + +func (ck *VargroupsChecker) checkUseVar(mkline *MkLine, varUse *MkVarUse) { + varname := varUse.varname + delete(ck.unusedVars, varname) + + if ck.ignoreUse(varname) { + return + } + + if ck.mklines.once.FirstTimeSlice("_VARGROUPS", "use", varname) { + mkline.Warnf("Variable %s is used but not mentioned in the _VARGROUPS section.", varname) + } +} + +func (ck *VargroupsChecker) ignoreUse(varname string) bool { + switch { + case containsVarRef(varname), + hasSuffix(varname, "_MK"), + varname == ".TARGET", + varname == "TOOLS_SHELL", + varname == "TOUCH_FLAGS", + varname == strings.ToLower(varname), + G.Pkgsrc.Tools.ExistsVar(varname), + ck.isShellCommand(varname), + ck.registered[varname] != nil, + ck.isVargroups(varname): + return true + } + + return false +} + +func (ck *VargroupsChecker) isShellCommand(varname string) bool { + vartype := G.Pkgsrc.VariableType(ck.mklines, varname) + if vartype != nil && vartype.basicType == BtShellCommand { + return true + } + return false +} + +func (ck *VargroupsChecker) isVargroups(varname string) bool { + if !hasPrefix(varname, "_") { + return false + } + + switch varnameCanon(varname) { + case "_VARGROUPS", + "_USER_VARS.*", + "_PKG_VARS.*", + "_SYS_VARS.*", + "_DEF_VARS.*", + "_USE_VARS.*", + "_IGN_VARS.*", + "_LISTED_VARS.*", + "_SORTED_VARS.*": + return true + } + return false +} + +func (ck *VargroupsChecker) Finish(mkline *MkLine) { + if ck.skip { + return + } + + forEachStringMkLine(ck.undefinedVars, func(varname string, mkline *MkLine) { + mkline.Warnf("The variable %s is not actually defined in this file.", varname) + }) + forEachStringMkLine(ck.unusedVars, func(varname string, mkline *MkLine) { + mkline.Warnf("The variable %s is not actually used in this file.", varname) + }) +} diff --git a/pkgtools/pkglint/files/vargroups_test.go b/pkgtools/pkglint/files/vargroups_test.go new file mode 100644 index 00000000000..6978fe3d973 --- /dev/null +++ b/pkgtools/pkglint/files/vargroups_test.go @@ -0,0 +1,142 @@ +package pkglint + +import "gopkg.in/check.v1" + +func (s *Suite) Test_VargroupsChecker__typo_in_group_name(c *check.C) { + t := s.Init(c) + + t.SetUpVartypes() + + mklines := t.NewMkLines("Makefile", + MkCvsID, + "", + "_VARGROUPS+=\t\tgroup", + "_DEF_VARS.typo=\t\t# none", + "_LISTED_VARS.typo=\t# none") + + mklines.Check() + + t.CheckOutputLines( + "WARN: Makefile:4: Expected _DEF_VARS.group, but found \"typo\".", + "WARN: Makefile:5: Expected _LISTED_VARS.group, but found \"typo\".") +} + +func (s *Suite) Test_VargroupsChecker__duplicate_variable_name(c *check.C) { + t := s.Init(c) + + t.SetUpVartypes() + + mklines := t.NewMkLines("Makefile", + MkCvsID, + "", + "_VARGROUPS+=\t\tgroup", + "_USER_VARS.group=\tVAR", + "_PKG_VARS.group=\tVAR", + "_SYS_VARS.group=\tVAR", + "_DEF_VARS.group=\tVAR", + "_USE_VARS.group=\tVAR", + "_IGN_VARS.group=\tVAR", + "_SORTED_VARS.group=\tVAR", + "_LISTED_VARS.group=\tVAR", + "", + "VAR=\t${VAR}") + + mklines.Check() + + t.CheckOutputLines( + "WARN: Makefile:5: Duplicate variable name VAR, already appeared in line 4.", + "WARN: Makefile:6: Duplicate variable name VAR, already appeared in line 4.", + "WARN: Makefile:7: Duplicate variable name VAR, already appeared in line 4.", + "WARN: Makefile:8: Duplicate variable name VAR, already appeared in line 4.", + "WARN: Makefile:9: Duplicate variable name VAR, already appeared in line 4.") +} + +func (s *Suite) Test_VargroupsChecker__variable_reference(c *check.C) { + t := s.Init(c) + + t.SetUpVartypes() + + mklines := t.NewMkLines("Makefile", + MkCvsID, + "", + "_VARGROUPS+=\t\tgroup", + "_USER_VARS.group=\t${:Uparam:@param@VAR.${param}@}", + "", + "VAR.param=\tvalue") + + mklines.Check() + + t.CheckOutputLines( + "WARN: Makefile:6: VAR.param is defined but not used.", + // FIXME: Hmmm, that's going to be complicated to get right. + "WARN: Makefile:6: Variable VAR.param is defined but not mentioned in the _VARGROUPS section.") +} + +func (s *Suite) Test_VargroupsChecker__public_underscore(c *check.C) { + t := s.Init(c) + + t.SetUpVartypes() + + mklines := t.NewMkLines("Makefile", + MkCvsID, + "", + "_VARGROUPS+=\t\tgroup", + "_USER_VARS.group=\t_VARGROUPS") + + mklines.Check() + + t.CheckOutputLines( + "WARN: Makefile:4: _USER_VARS.group should list only variables that " + + "start with a letter, not \"_VARGROUPS\".") +} + +func (s *Suite) Test_VargroupsChecker__declared_but_undefined(c *check.C) { + t := s.Init(c) + + t.SetUpVartypes() + + mklines := t.NewMkLines("Makefile", + MkCvsID, + "", + "# UNDECLARED", // Documented to avoid a warning about being defined but unused. + "#\tDocumentation", + "#\tDocumentation", + "", + "_VARGROUPS+=\t\tgroup", + "_DEF_VARS.group=\tUNDEFINED", + "", + "UNDECLARED=\tvalue", + "UNDECLARED=\tvalue") + + mklines.Check() + + t.CheckOutputLines( + "WARN: Makefile:10: Variable UNDECLARED is defined but not mentioned in the _VARGROUPS section.", + "WARN: Makefile:8: The variable UNDEFINED is not actually defined in this file.") +} + +func (s *Suite) Test_VargroupsChecker__declared_but_unused(c *check.C) { + t := s.Init(c) + + t.SetUpVartypes() + + mklines := t.NewMkLines("Makefile", + MkCvsID, + "", + "# UNDECLARED", // Documented to avoid a "used but not defined" warning. + "#\tDocumentation", + "#\tDocumentation", + "", + "_VARGROUPS+=\t\tgroup", + "_USE_VARS.group=\tUNUSED", + "", + "target: .PHONY", + "\t: ${UNDECLARED}", + "\t: ${UNDECLARED}") + + mklines.Check() + + t.CheckOutputLines( + "WARN: Makefile:11: Variable UNDECLARED is used but not mentioned in the _VARGROUPS section.", + "WARN: Makefile:8: The variable UNUSED is not actually used in this file.") +} |