Received: by mail.netbsd.org (Postfix, from userid 605) id 1211E84D85; Sun, 8 Sep 2019 22:47:58 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by mail.netbsd.org (Postfix) with ESMTP id 85AE084D4D for ; Sun, 8 Sep 2019 22:47:57 +0000 (UTC) X-Virus-Scanned: amavisd-new at netbsd.org Received: from mail.netbsd.org ([127.0.0.1]) by localhost (mail.netbsd.org [127.0.0.1]) (amavisd-new, port 10025) with ESMTP id vMcTd293kcus for ; Sun, 8 Sep 2019 22:47:48 +0000 (UTC) Received: from cvs.NetBSD.org (ivanova.netbsd.org [199.233.217.197]) by mail.netbsd.org (Postfix) with ESMTP id 2874884CD4 for ; Sun, 8 Sep 2019 22:47:48 +0000 (UTC) Received: by cvs.NetBSD.org (Postfix, from userid 500) id 210D5FBF4; Sun, 8 Sep 2019 22:47:48 +0000 (UTC) Content-Transfer-Encoding: 7bit Content-Type: multipart/mixed; boundary="_----------=_1567982868138080" MIME-Version: 1.0 Date: Sun, 8 Sep 2019 22:47:48 +0000 From: "Roland Illig" Subject: CVS commit: pkgsrc/pkgtools/pkglint To: pkgsrc-changes@NetBSD.org Reply-To: rillig@netbsd.org X-Mailer: log_accum Message-Id: <20190908224748.210D5FBF4@cvs.NetBSD.org> Sender: pkgsrc-changes-owner@NetBSD.org List-Id: pkgsrc-changes.NetBSD.org Precedence: bulk List-Unsubscribe: This is a multi-part message in MIME format. --_----------=_1567982868138080 Content-Disposition: inline Content-Transfer-Encoding: 8bit Content-Type: text/plain; charset="US-ASCII" Module Name: pkgsrc Committed By: rillig Date: Sun Sep 8 22:47:47 UTC 2019 Modified Files: pkgsrc/pkgtools/pkglint: Makefile PLIST pkgsrc/pkgtools/pkglint/files: autofix.go autofix_test.go category_test.go lines.go lines_test.go logging.go logging_test.go mkline_test.go mklinechecker.go mklinechecker_test.go mklines.go mkparser.go mkparser_test.go package.go package_test.go pkglint.go pkglint_test.go pkgsrc.go shell.go tools.go util.go util_test.go varalignblock.go varalignblock_test.go vardefs.go Added Files: pkgsrc/pkgtools/pkglint/files: vargroups.go vargroups_test.go Log Message: 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. To generate a diff of this commit: cvs rdiff -u -r1.595 -r1.596 pkgsrc/pkgtools/pkglint/Makefile cvs rdiff -u -r1.13 -r1.14 pkgsrc/pkgtools/pkglint/PLIST cvs rdiff -u -r1.27 -r1.28 pkgsrc/pkgtools/pkglint/files/autofix.go cvs rdiff -u -r1.28 -r1.29 pkgsrc/pkgtools/pkglint/files/autofix_test.go cvs rdiff -u -r1.24 -r1.25 pkgsrc/pkgtools/pkglint/files/category_test.go cvs rdiff -u -r1.7 -r1.8 pkgsrc/pkgtools/pkglint/files/lines.go cvs rdiff -u -r1.10 -r1.11 pkgsrc/pkgtools/pkglint/files/lines_test.go cvs rdiff -u -r1.25 -r1.26 pkgsrc/pkgtools/pkglint/files/logging.go cvs rdiff -u -r1.18 -r1.19 pkgsrc/pkgtools/pkglint/files/logging_test.go \ pkgsrc/pkgtools/pkglint/files/tools.go cvs rdiff -u -r1.65 -r1.66 pkgsrc/pkgtools/pkglint/files/mkline_test.go cvs rdiff -u -r1.44 -r1.45 pkgsrc/pkgtools/pkglint/files/mklinechecker.go \ pkgsrc/pkgtools/pkglint/files/shell.go cvs rdiff -u -r1.40 -r1.41 \ pkgsrc/pkgtools/pkglint/files/mklinechecker_test.go cvs rdiff -u -r1.54 -r1.55 pkgsrc/pkgtools/pkglint/files/mklines.go cvs rdiff -u -r1.32 -r1.33 pkgsrc/pkgtools/pkglint/files/mkparser.go \ pkgsrc/pkgtools/pkglint/files/util_test.go cvs rdiff -u -r1.31 -r1.32 pkgsrc/pkgtools/pkglint/files/mkparser_test.go cvs rdiff -u -r1.61 -r1.62 pkgsrc/pkgtools/pkglint/files/package.go cvs rdiff -u -r1.52 -r1.53 pkgsrc/pkgtools/pkglint/files/package_test.go cvs rdiff -u -r1.59 -r1.60 pkgsrc/pkgtools/pkglint/files/pkglint.go cvs rdiff -u -r1.46 -r1.47 pkgsrc/pkgtools/pkglint/files/pkglint_test.go cvs rdiff -u -r1.34 -r1.35 pkgsrc/pkgtools/pkglint/files/pkgsrc.go cvs rdiff -u -r1.50 -r1.51 pkgsrc/pkgtools/pkglint/files/util.go cvs rdiff -u -r1.4 -r1.5 pkgsrc/pkgtools/pkglint/files/varalignblock.go \ pkgsrc/pkgtools/pkglint/files/varalignblock_test.go cvs rdiff -u -r1.70 -r1.71 pkgsrc/pkgtools/pkglint/files/vardefs.go cvs rdiff -u -r0 -r1.1 pkgsrc/pkgtools/pkglint/files/vargroups.go \ pkgsrc/pkgtools/pkglint/files/vargroups_test.go Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files. --_----------=_1567982868138080 Content-Disposition: inline Content-Length: 195141 Content-Transfer-Encoding: binary Content-Type: text/x-diff; charset=us-ascii Modified files: Index: pkgsrc/pkgtools/pkglint/Makefile diff -u pkgsrc/pkgtools/pkglint/Makefile:1.595 pkgsrc/pkgtools/pkglint/Makefile:1.596 --- pkgsrc/pkgtools/pkglint/Makefile:1.595 Sun Aug 25 21:47:11 2019 +++ pkgsrc/pkgtools/pkglint/Makefile Sun Sep 8 22:47:47 2019 @@ -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/} Index: pkgsrc/pkgtools/pkglint/PLIST diff -u pkgsrc/pkgtools/pkglint/PLIST:1.13 pkgsrc/pkgtools/pkglint/PLIST:1.14 --- pkgsrc/pkgtools/pkglint/PLIST:1.13 Sun Jul 14 21:25:47 2019 +++ pkgsrc/pkgtools/pkglint/PLIST Sun Sep 8 22:47:47 2019 @@ -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/varalignblo 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 Index: pkgsrc/pkgtools/pkglint/files/autofix.go diff -u pkgsrc/pkgtools/pkglint/files/autofix.go:1.27 pkgsrc/pkgtools/pkglint/files/autofix.go:1.28 --- pkgsrc/pkgtools/pkglint/files/autofix.go:1.27 Sun Jul 14 21:25:47 2019 +++ pkgsrc/pkgtools/pkglint/files/autofix.go Sun Sep 8 22:47:47 2019 @@ -127,7 +127,7 @@ func (fix *Autofix) ReplaceAfter(prefix, // ReplaceAt replaces the text "from" with "to", a single time. // But only if the text at the given position is indeed "from". -func (fix *Autofix) ReplaceAt(rawIndex int, 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 i } 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() Index: pkgsrc/pkgtools/pkglint/files/autofix_test.go diff -u pkgsrc/pkgtools/pkglint/files/autofix_test.go:1.28 pkgsrc/pkgtools/pkglint/files/autofix_test.go:1.29 --- pkgsrc/pkgtools/pkglint/files/autofix_test.go:1.28 Thu Aug 1 22:38:49 2019 +++ pkgsrc/pkgtools/pkglint/files/autofix_test.go Sun Sep 8 22:47:47 2019 @@ -966,8 +966,6 @@ func (s *Suite) Test_Autofix_Apply__expl t.CheckOutputLines( ">\ttext", "WARN: README.txt:123: A warning with autofix.", - "", - ">\ttext", "NOTE: README.txt:123: A note without fix.") } Index: pkgsrc/pkgtools/pkglint/files/category_test.go diff -u pkgsrc/pkgtools/pkglint/files/category_test.go:1.24 pkgsrc/pkgtools/pkglint/files/category_test.go:1.25 --- pkgsrc/pkgtools/pkglint/files/category_test.go:1.24 Sun Jul 14 21:25:47 2019 +++ pkgsrc/pkgtools/pkglint/files/category_test.go Sun Sep 8 22:47:47 2019 @@ -343,3 +343,28 @@ func (s *Suite) Test_CheckdirCategory__n 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.") +} Index: pkgsrc/pkgtools/pkglint/files/lines.go diff -u pkgsrc/pkgtools/pkglint/files/lines.go:1.7 pkgsrc/pkgtools/pkglint/files/lines.go:1.8 --- pkgsrc/pkgtools/pkglint/files/lines.go:1.7 Sun Jun 30 20:56:19 2019 +++ pkgsrc/pkgtools/pkglint/files/lines.go Sun Sep 8 22:47:47 2019 @@ -42,7 +42,11 @@ func (ls *Lines) CheckCvsID(index int, p 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( Index: pkgsrc/pkgtools/pkglint/files/lines_test.go diff -u pkgsrc/pkgtools/pkglint/files/lines_test.go:1.10 pkgsrc/pkgtools/pkglint/files/lines_test.go:1.11 --- pkgsrc/pkgtools/pkglint/files/lines_test.go:1.10 Sun Jun 30 20:56:19 2019 +++ pkgsrc/pkgtools/pkglint/files/lines_test.go Sun Sep 8 22:47:47 2019 @@ -30,6 +30,7 @@ func (s *Suite) Test_Lines_CheckCvsID(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", Index: pkgsrc/pkgtools/pkglint/files/logging.go diff -u pkgsrc/pkgtools/pkglint/files/logging.go:1.25 pkgsrc/pkgtools/pkglint/files/logging.go:1.26 --- pkgsrc/pkgtools/pkglint/files/logging.go:1.25 Sun Jul 14 21:25:47 2019 +++ pkgsrc/pkgtools/pkglint/files/logging.go Sun Sep 8 22:47:47 2019 @@ -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 ... 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 } 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 st } // 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 } Index: pkgsrc/pkgtools/pkglint/files/logging_test.go diff -u pkgsrc/pkgtools/pkglint/files/logging_test.go:1.18 pkgsrc/pkgtools/pkglint/files/logging_test.go:1.19 --- pkgsrc/pkgtools/pkglint/files/logging_test.go:1.18 Sun Jul 14 21:25:47 2019 +++ pkgsrc/pkgtools/pkglint/files/logging_test.go Sun Sep 8 22:47:47 2019 @@ -206,9 +206,107 @@ func (s *Suite) Test_Logger__show_source "", ">\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_separa "+\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_Flu 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(), Index: pkgsrc/pkgtools/pkglint/files/tools.go diff -u pkgsrc/pkgtools/pkglint/files/tools.go:1.18 pkgsrc/pkgtools/pkglint/files/tools.go:1.19 --- pkgsrc/pkgtools/pkglint/files/tools.go:1.18 Sun Jul 14 21:25:47 2019 +++ pkgsrc/pkgtools/pkglint/files/tools.go Sun Sep 8 22:47:47 2019 @@ -415,6 +415,8 @@ func (tr *Tools) adjustValidity(tool *To } } +func (tr *Tools) ExistsVar(varname string) bool { return tr.byVarname[varname] != nil } + type Validity uint8 const ( Index: pkgsrc/pkgtools/pkglint/files/mkline_test.go diff -u pkgsrc/pkgtools/pkglint/files/mkline_test.go:1.65 pkgsrc/pkgtools/pkglint/files/mkline_test.go:1.66 --- pkgsrc/pkgtools/pkglint/files/mkline_test.go:1.65 Sun Aug 25 21:44:37 2019 +++ pkgsrc/pkgtools/pkglint/files/mkline_test.go Sun Sep 8 22:47:47 2019 @@ -337,7 +337,11 @@ func (s *Suite) Test_MkLine__aligned(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 * // 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 * // 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 * // 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 * // 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 * // line is not indented properly. test( "GENERATE_PLIST+= \\", - "\t/pattern/ {\\", + "\t/pattern/ { \\", "\t action(); \\", "}", false) Index: pkgsrc/pkgtools/pkglint/files/mklinechecker.go diff -u pkgsrc/pkgtools/pkglint/files/mklinechecker.go:1.44 pkgsrc/pkgtools/pkglint/files/mklinechecker.go:1.45 --- pkgsrc/pkgtools/pkglint/files/mklinechecker.go:1.44 Wed Aug 21 16:45:17 2019 +++ pkgsrc/pkgtools/pkglint/files/mklinechecker.go Sun Sep 8 22:47:47 2019 @@ -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) checkVarUseBuild 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) simplifyConditio 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() } } Index: pkgsrc/pkgtools/pkglint/files/shell.go diff -u pkgsrc/pkgtools/pkglint/files/shell.go:1.44 pkgsrc/pkgtools/pkglint/files/shell.go:1.45 --- pkgsrc/pkgtools/pkglint/files/shell.go:1.44 Sun Jul 14 21:25:47 2019 +++ pkgsrc/pkgtools/pkglint/files/shell.go Sun Sep 8 22:47:47 2019 @@ -422,6 +422,7 @@ func (ck *ShellLineChecker) checkHiddenA "${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}": Index: pkgsrc/pkgtools/pkglint/files/mklinechecker_test.go diff -u pkgsrc/pkgtools/pkglint/files/mklinechecker_test.go:1.40 pkgsrc/pkgtools/pkglint/files/mklinechecker_test.go:1.41 --- pkgsrc/pkgtools/pkglint/files/mklinechecker_test.go:1.40 Wed Aug 21 16:45:17 2019 +++ pkgsrc/pkgtools/pkglint/files/mklinechecker_test.go Sun Sep 8 22:47:47 2019 @@ -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) Index: pkgsrc/pkgtools/pkglint/files/mklines.go diff -u pkgsrc/pkgtools/pkglint/files/mklines.go:1.54 pkgsrc/pkgtools/pkglint/files/mklines.go:1.55 --- pkgsrc/pkgtools/pkglint/files/mklines.go:1.54 Sun Aug 25 21:44:37 2019 +++ pkgsrc/pkgtools/pkglint/files/mklines.go Sun Sep 8 22:47:47 2019 @@ -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 { Index: pkgsrc/pkgtools/pkglint/files/mkparser.go diff -u pkgsrc/pkgtools/pkglint/files/mkparser.go:1.32 pkgsrc/pkgtools/pkglint/files/mkparser.go:1.33 --- pkgsrc/pkgtools/pkglint/files/mkparser.go:1.32 Sun Jul 14 21:25:47 2019 +++ pkgsrc/pkgtools/pkglint/files/mkparser.go Sun Sep 8 22:47:47 2019 @@ -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() *MkCo 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}}} } } Index: pkgsrc/pkgtools/pkglint/files/util_test.go diff -u pkgsrc/pkgtools/pkglint/files/util_test.go:1.32 pkgsrc/pkgtools/pkglint/files/util_test.go:1.33 --- pkgsrc/pkgtools/pkglint/files/util_test.go:1.32 Sun Jul 14 21:25:47 2019 +++ pkgsrc/pkgtools/pkglint/files/util_test.go Sun Sep 8 22:47:47 2019 @@ -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) Index: pkgsrc/pkgtools/pkglint/files/mkparser_test.go diff -u pkgsrc/pkgtools/pkglint/files/mkparser_test.go:1.31 pkgsrc/pkgtools/pkglint/files/mkparser_test.go:1.32 --- pkgsrc/pkgtools/pkglint/files/mkparser_test.go:1.31 Sun Jul 14 21:25:47 2019 +++ pkgsrc/pkgtools/pkglint/files/mkparser_test.go Sun Sep 8 22:47:47 2019 @@ -548,7 +548,7 @@ func (s *Suite) Test_MkParser_MkCond(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 * "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) Index: pkgsrc/pkgtools/pkglint/files/package.go diff -u pkgsrc/pkgtools/pkglint/files/package.go:1.61 pkgsrc/pkgtools/pkglint/files/package.go:1.62 --- pkgsrc/pkgtools/pkglint/files/package.go:1.61 Sun Aug 25 21:44:37 2019 +++ pkgsrc/pkgtools/pkglint/files/package.go Sun Sep 8 22:47:47 2019 @@ -695,7 +695,10 @@ func (pkg *Package) checkfilePackageMake 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}.") } Index: pkgsrc/pkgtools/pkglint/files/package_test.go diff -u pkgsrc/pkgtools/pkglint/files/package_test.go:1.52 pkgsrc/pkgtools/pkglint/files/package_test.go:1.53 --- pkgsrc/pkgtools/pkglint/files/package_test.go:1.52 Sun Aug 25 21:44:37 2019 +++ pkgsrc/pkgtools/pkglint/files/package_test.go Sun Sep 8 22:47:47 2019 @@ -719,6 +719,7 @@ func (s *Suite) Test_Package_determineEf 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_determineEf 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_determineEf 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_determineEf 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 // 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() +} Index: pkgsrc/pkgtools/pkglint/files/pkglint.go diff -u pkgsrc/pkgtools/pkglint/files/pkglint.go:1.59 pkgsrc/pkgtools/pkglint/files/pkglint.go:1.60 --- pkgsrc/pkgtools/pkglint/files/pkglint.go:1.59 Sun Aug 25 21:44:37 2019 +++ pkgsrc/pkgtools/pkglint/files/pkglint.go Sun Sep 8 22:47:47 2019 @@ -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 Index: pkgsrc/pkgtools/pkglint/files/pkglint_test.go diff -u pkgsrc/pkgtools/pkglint/files/pkglint_test.go:1.46 pkgsrc/pkgtools/pkglint/files/pkglint_test.go:1.47 --- pkgsrc/pkgtools/pkglint/files/pkglint_test.go:1.46 Sun Jul 14 21:25:47 2019 +++ pkgsrc/pkgtools/pkglint/files/pkglint_test.go Sun Sep 8 22:47:47 2019 @@ -726,7 +726,7 @@ func (s *Suite) Test_Pkglint_Tool__looku 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") Index: pkgsrc/pkgtools/pkglint/files/pkgsrc.go diff -u pkgsrc/pkgtools/pkglint/files/pkgsrc.go:1.34 pkgsrc/pkgtools/pkglint/files/pkgsrc.go:1.35 --- pkgsrc/pkgtools/pkglint/files/pkgsrc.go:1.34 Fri Aug 16 21:00:17 2019 +++ pkgsrc/pkgtools/pkglint/files/pkgsrc.go Sun Sep 8 22:47:47 2019 @@ -819,6 +819,15 @@ func (src *Pkgsrc) LoadMk(filename strin 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. Index: pkgsrc/pkgtools/pkglint/files/util.go diff -u pkgsrc/pkgtools/pkglint/files/util.go:1.50 pkgsrc/pkgtools/pkglint/files/util.go:1.51 --- pkgsrc/pkgtools/pkglint/files/util.go:1.50 Thu Aug 1 22:38:49 2019 +++ pkgsrc/pkgtools/pkglint/files/util.go Sun Sep 8 22:47:47 2019 @@ -149,6 +149,25 @@ func keysJoined(m map[string]bool) strin 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) []strin 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 []st s.Add(element) } } + +func (s *StringSet) Contains(needle string) bool { + _, found := s.seen[needle] + return found +} Index: pkgsrc/pkgtools/pkglint/files/varalignblock.go diff -u pkgsrc/pkgtools/pkglint/files/varalignblock.go:1.4 pkgsrc/pkgtools/pkglint/files/varalignblock.go:1.5 --- pkgsrc/pkgtools/pkglint/files/varalignblock.go:1.4 Sun Aug 25 21:44:37 2019 +++ pkgsrc/pkgtools/pkglint/files/varalignblock.go Sun Sep 8 22:47:47 2019 @@ -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) processVarassig 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) processVarassig } } -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() + sort.Ints(columns) - newline := len(rest) - for newline > 0 && rest[newline-1] == '\n' { - newline-- + for i := len(columns) - 2; i >= 0; i-- { + if columns[i] == columns[i+1] { + return columns[i] } - - backslash := newline - for backslash > 0 && rest[backslash-1] == '\\' { - backslash-- - } - - if (newline-backslash)%2 == 1 { - continuation := rest[backslash:] - commentSpace := rest[:backslash] - comment := rtrimHspace(commentSpace) - space := commentSpace[len(comment):] - return comment, space, continuation - } - - 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 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) realignMultiEmptyI 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) realignMultiEmptyI 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) realignMultiEmp 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) realignMultiIni 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) realignMultiFol } 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) explainWrongCol "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 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 } -func (l *varalignLine) varnameOpSpaceWidth() int { - return tabWidth(l.parts.leadingComment + l.parts.varnameOp + l.parts.spaceBeforeValue) +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( // 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() 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) } Index: pkgsrc/pkgtools/pkglint/files/varalignblock_test.go diff -u pkgsrc/pkgtools/pkglint/files/varalignblock_test.go:1.4 pkgsrc/pkgtools/pkglint/files/varalignblock_test.go:1.5 --- pkgsrc/pkgtools/pkglint/files/varalignblock_test.go:1.4 Sun Aug 25 21:44:37 2019 +++ pkgsrc/pkgtools/pkglint/files/varalignblock_test.go Sun Sep 8 22:47:47 2019 @@ -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 .. // 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 bo 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 bo } } -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_ 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_ 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_ 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_ 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_ 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_ 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_ 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_ 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_ 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_ 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_ 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_ // 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__seve "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.") - 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\".") + "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\".") vt.Fixed( "GRP_A= value", "GRP_AA= value", @@ -815,18 +862,21 @@ func (s *Suite) Test_VaralignBlock__seve 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__cont // 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__shor "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__shor // 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__shor "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__shor // 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__spac "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__empt "\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__empt 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__empt "\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__empt 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__empt "\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__empt 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__empt "\tminimum indentation") vt.Internals( "14 16", - "21 24", + "21 24 24", " 08") vt.Diagnostics( nil...) @@ -1044,7 +1083,7 @@ func (s *Suite) Test_VaralignBlock__empt // 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__outl "\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__outl 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__cont "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.") - 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\".") + "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\".") vt.Fixed( "DISTFILES+= value", "DISTFILES+= \\", @@ -1115,7 +1154,7 @@ func (s *Suite) Test_VaralignBlock__cont // 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__cont "\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__cont // 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__outl "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__outl // 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__alig // 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__shel "\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__shel 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__esca // 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__cont 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\".") - 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\".") + "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\".") vt.Fixed( "WRKSRC= ${WRKDIR}", "DISTFILES= distfile-1.0.0.tar.gz", @@ -1265,7 +1307,7 @@ func (s *Suite) Test_VaralignBlock__cont // 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__cont 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__cont // 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__cont 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__cont // 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__cont 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\".") - 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\".") + "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\".") vt.Fixed( "WRKSRC= ${WRKDIR}", "DISTFILES= distfile-1.0.0.tar.gz", @@ -1387,19 +1429,19 @@ func (s *Suite) Test_VaralignBlock__cont 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\".") - 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\".") + "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\".") vt.Fixed( "WRKSRC= ${WRKDIR}", "DISTFILES= distfile-1.0.0.tar.gz", @@ -1410,9 +1452,6 @@ func (s *Suite) Test_VaralignBlock__cont } 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__foll "\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\".") - vt.Autofixes( - "AUTOFIX: ~/Makefile:2: Replacing \" \" with \"\\t\".", - "AUTOFIX: ~/Makefile:3: Replacing \" \" with \"\\t\".", - "AUTOFIX: ~/Makefile:4: Replacing \" \" 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\".") vt.Fixed( "DISTFILES= \\", " one space \\", @@ -1450,8 +1489,9 @@ func (s *Suite) Test_VaralignBlock__foll // 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_ // 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__mult ".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.") - 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\".") + "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\\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__mult 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.") - 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\".") + "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\".") 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__sing // 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 "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.") - vt.Autofixes( - "AUTOFIX: ~/Makefile:1: Replacing \" \" with \"\\t\".", - "AUTOFIX: ~/Makefile:2: Replacing \" \" with \"\\t\".", - "AUTOFIX: ~/Makefile:3: Replacing \" \" with \"\\t\".") + "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\".") vt.Fixed( "REPLACE_PYTHON+= *.py", "REPLACE_PYTHON+= lib/*.py", @@ -1581,7 +1606,7 @@ func (s *Suite) Test_VaralignBlock__only // // 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__mixe "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__mixe } // 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__outl "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__outl } // 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__outl "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__outl // 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__outl "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__outl // 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__outl "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__outl // 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__outl } // 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__outl "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__outl } // 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__outl "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__outl "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__outl // 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__outl "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__outl "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__sing "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__outl "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__outl // 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 // 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__tabb 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__tabb // // 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__inde 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__inde 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__inde 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_ "\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_ 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__cont "", // 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__real 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.") - 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\".") + "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\".") vt.Fixed( "SHORT= value", "#DISTFILES= distfile-1.0.0.tar.gz", @@ -2083,15 +2122,15 @@ func (s *Suite) Test_VaralignBlock__real "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__real "#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__mixe " \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__foll "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__stai "\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__stai 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__comm "\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.") - vt.Autofixes( - "AUTOFIX: ~/Makefile:1: Replacing \" \" with \"\\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\".", + "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: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__empt 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_Proce // 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_Proce "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_reali 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_reali // 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_reali "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_reali "\\", "# 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\".") - 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\".") + "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\".") vt.Fixed( "VAR= \\", " value1 \\", @@ -2521,22 +2624,220 @@ func (s *Suite) Test_VaralignBlock_reali 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 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 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 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 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 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 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 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 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 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 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 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 spaceAfterValue: "", trailingComment: "", spaceAfterComment: "", - continuation: "", - }) + continuation: ""}) test(" value", false, - varalignSplitResult{ + varalignParts{ leadingComment: "", varnameOp: "", spaceBeforeValue: " ", @@ -2694,14 +2983,13 @@ func (s *Suite) Test_VaralignBlock_split 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 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 // 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 spaceAfterValue: "", trailingComment: "", spaceAfterComment: "", - continuation: "", - }) + continuation: ""}) test("#\tcomment \\", false, - varalignSplitResult{ + varalignParts{ leadingComment: "#", varnameOp: "", spaceBeforeValue: "\t", @@ -2747,11 +3033,10 @@ func (s *Suite) Test_VaralignBlock_split spaceAfterValue: " ", trailingComment: "", spaceAfterComment: "", - continuation: "\\", - }) + continuation: "\\"}) test("# comment", false, - varalignSplitResult{ + varalignParts{ leadingComment: "", varnameOp: "", spaceBeforeValue: "", @@ -2759,11 +3044,10 @@ func (s *Suite) Test_VaralignBlock_split spaceAfterValue: "", trailingComment: "# comment", spaceAfterComment: "", - continuation: "", - }) + continuation: ""}) test("# comment \\", false, - varalignSplitResult{ + varalignParts{ leadingComment: "", varnameOp: "", spaceBeforeValue: "", @@ -2771,8 +3055,7 @@ func (s *Suite) Test_VaralignBlock_split 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 // 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_canoni 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) +} Index: pkgsrc/pkgtools/pkglint/files/vardefs.go diff -u pkgsrc/pkgtools/pkglint/files/vardefs.go:1.70 pkgsrc/pkgtools/pkglint/files/vardefs.go:1.71 --- pkgsrc/pkgtools/pkglint/files/vardefs.go:1.70 Wed Aug 21 16:45:17 2019 +++ pkgsrc/pkgtools/pkglint/files/vardefs.go Sun Sep 8 22:47:47 2019 @@ -311,11 +311,7 @@ func (reg *VarTypeRegistry) infralist(va // 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 *Pk 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 *Pk 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 *Pk 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) } Added files: Index: pkgsrc/pkgtools/pkglint/files/vargroups.go diff -u /dev/null pkgsrc/pkgtools/pkglint/files/vargroups.go:1.1 --- /dev/null Sun Sep 8 22:47:48 2019 +++ pkgsrc/pkgtools/pkglint/files/vargroups.go Sun Sep 8 22:47:47 2019 @@ -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) + }) +} Index: pkgsrc/pkgtools/pkglint/files/vargroups_test.go diff -u /dev/null pkgsrc/pkgtools/pkglint/files/vargroups_test.go:1.1 --- /dev/null Sun Sep 8 22:47:48 2019 +++ pkgsrc/pkgtools/pkglint/files/vargroups_test.go Sun Sep 8 22:47:47 2019 @@ -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.") +} --_----------=_1567982868138080--