diff options
author | rillig <rillig@pkgsrc.org> | 2016-01-24 02:03:28 +0000 |
---|---|---|
committer | rillig <rillig@pkgsrc.org> | 2016-01-24 02:03:28 +0000 |
commit | 0266c6a3eb8f13ec76f8898b54fbbb07d29a3765 (patch) | |
tree | e8b82cbcfa5652b0a34e58c6360d6b08a81dc2f2 /pkgtools | |
parent | 76261a694b8285a897a28fe10960b0e43c0f1aa3 (diff) | |
download | pkgsrc-0266c6a3eb8f13ec76f8898b54fbbb07d29a3765.tar.gz |
Updated pkglint to 5.3.2
Changes since 5.3.1:
Alignment of variable values is no longer checked by single line, but by
the complete block (e.g. SUBST_*). Pkglint now checks that all variables
belonging to a block are indented consistently, so that their values are
aligned nicely.
Since pkglint does not report warnings, but only notes, and since it can
fix them automatically, the burden on the package developers will be very
low. Especially, since these notes are only printed when pkglint is called
with the -Wspace or -Wall options.
Also, pkglint supports running its unit tests now.
Diffstat (limited to 'pkgtools')
-rw-r--r-- | pkgtools/pkglint/Makefile | 18 | ||||
-rw-r--r-- | pkgtools/pkglint/files/check_test.go | 8 | ||||
-rw-r--r-- | pkgtools/pkglint/files/globaldata.go | 6 | ||||
-rw-r--r-- | pkgtools/pkglint/files/mkline.go | 24 | ||||
-rw-r--r-- | pkgtools/pkglint/files/mkline_test.go | 210 | ||||
-rw-r--r-- | pkgtools/pkglint/files/mklines.go | 138 | ||||
-rw-r--r-- | pkgtools/pkglint/files/pkglint.go | 5 | ||||
-rw-r--r-- | pkgtools/pkglint/files/pkglint_test.go | 28 |
8 files changed, 358 insertions, 79 deletions
diff --git a/pkgtools/pkglint/Makefile b/pkgtools/pkglint/Makefile index ec3c51b40e7..9f6c62d8936 100644 --- a/pkgtools/pkglint/Makefile +++ b/pkgtools/pkglint/Makefile @@ -1,7 +1,6 @@ -# $NetBSD: Makefile,v 1.477 2016/01/18 15:33:44 fhajny Exp $ +# $NetBSD: Makefile,v 1.478 2016/01/24 02:03:28 rillig Exp $ -PKGNAME= pkglint-5.3.1 -PKGREVISION= 1 +PKGNAME= pkglint-5.3.2 DISTFILES= # none CATEGORIES= pkgtools @@ -14,6 +13,7 @@ CONFLICTS+= pkglint4-[0-9]* WRKSRC= ${WRKDIR}/netbsd.org/pkglint NO_CHECKSUM= yes USE_LANGUAGES= # none +USE_TOOLS+= pax AUTO_MKDIRS= yes GO_SRCPATH= netbsd.org/pkglint @@ -24,17 +24,14 @@ SUBST_SED.pkglint+= -e s\|@VERSION@\|${PKGNAME:S/pkglint-//}\|g SUBST_SED.pkglint+= -e s\|@BMAKE@\|${MAKE:Q}\|g do-extract: - mkdir -p ${WRKDIR}/pkglint/plist-clash - cd ${FILESDIR} && ${PAX} -rw *.go */*.go pkglint.[01] ${WRKDIR}/pkglint - -do-test: - cd ${WRKSRC} && go test + ${RUN} mkdir -p ${WRKDIR}/pkglint/plist-clash + ${RUN} cd ${FILESDIR} && ${PAX} -rw *.go */*.go pkglint.[01] ${WRKDIR}/pkglint do-install: do-install-man .include "../../mk/bsd.prefs.mk" -do-install-man: +do-install-man: .PHONY .if !empty(MANINSTALL:Mcatinstall) . if defined(CATMAN_SECTION_SUFFIX) && !empty(CATMAN_SECTION_SUFFIX:M[Yy][Ee][Ss]) ${INSTALL_MAN} ${WRKSRC}/pkglint.0 ${DESTDIR}${PREFIX}/${PKGMANDIR}/cat1/pkglint.1 @@ -47,4 +44,7 @@ do-install-man: .endif .include "../../lang/go/go-package.mk" +.if !empty(PKGSRC_RUN_TEST:M[yY][eE][sS]) +.include "../../devel/go-check/buildlink3.mk" +.endif .include "../../mk/bsd.pkg.mk" diff --git a/pkgtools/pkglint/files/check_test.go b/pkgtools/pkglint/files/check_test.go index 8e63d260c2b..cee87f982e2 100644 --- a/pkgtools/pkglint/files/check_test.go +++ b/pkgtools/pkglint/files/check_test.go @@ -117,6 +117,14 @@ func (s *Suite) CreateTmpFile(c *check.C, relFname, content string) (absFname st return } +func (s *Suite) CreateTmpFileLines(c *check.C, relFname string, rawTexts ...string) (absFname string) { + text := "" + for _, rawText := range rawTexts { + text += rawText + "\n" + } + return s.CreateTmpFile(c, relFname, text) +} + func (s *Suite) LoadTmpFile(c *check.C, relFname string) string { bytes, err := ioutil.ReadFile(s.tmpdir + "/" + relFname) c.Assert(err, check.IsNil) diff --git a/pkgtools/pkglint/files/globaldata.go b/pkgtools/pkglint/files/globaldata.go index adb55f0c447..d86f67e8952 100644 --- a/pkgtools/pkglint/files/globaldata.go +++ b/pkgtools/pkglint/files/globaldata.go @@ -75,7 +75,7 @@ func (gd *GlobalData) loadDistSites() { names := make(map[string]bool) url2name := make(map[string]string) for _, line := range lines { - if m, varname, _, urls, _ := MatchVarassign(line.Text); m { + if m, varname, _, _, urls, _ := MatchVarassign(line.Text); m { if hasPrefix(varname, "MASTER_SITE_") && varname != "MASTER_SITE_BACKUP" { names[varname] = true for _, url := range splitOnSpace(urls) { @@ -139,7 +139,7 @@ func (gd *GlobalData) loadTools() { fname := G.globalData.Pkgsrcdir + "/mk/tools/" + basename lines := LoadExistingLines(fname, true) for _, line := range lines { - if m, varname, _, value, _ := MatchVarassign(line.Text); m { + if m, varname, _, _, value, _ := MatchVarassign(line.Text); m { if varname == "TOOLS_CREATE" && (value == "[" || matches(value, `^?[-\w.]+$`)) { tools[value] = true } else if m, toolname := match1(varname, `^(?:_TOOLS_VARNAME)\.([-\w.]+|\[)$`); m { @@ -169,7 +169,7 @@ func (gd *GlobalData) loadTools() { for _, line := range lines { text := line.Text - if m, varname, _, value, _ := MatchVarassign(text); m { + if m, varname, _, _, value, _ := MatchVarassign(text); m { if varname == "USE_TOOLS" { if G.opts.DebugTools { line.Debugf("[condDepth=%d] %s", condDepth, value) diff --git a/pkgtools/pkglint/files/mkline.go b/pkgtools/pkglint/files/mkline.go index 04a5d4d7d25..599fbb0d539 100644 --- a/pkgtools/pkglint/files/mkline.go +++ b/pkgtools/pkglint/files/mkline.go @@ -15,6 +15,7 @@ type MkLine struct { xtype uint8 xmustexist bool xop MkOperator + xvalign string xs1 string xs2 string xs3 string @@ -44,7 +45,7 @@ func NewMkLine(line *Line) (mkline *MkLine) { "white-space.") } - if m, varname, op, value, comment := MatchVarassign(text); m { + if m, varname, op, valueAlign, value, comment := MatchVarassign(text); m { value = strings.Replace(value, "\\#", "#", -1) varparam := varnameParam(varname) @@ -53,6 +54,7 @@ func NewMkLine(line *Line) (mkline *MkLine) { mkline.xs2 = varnameCanon(varname) mkline.xs3 = varparam mkline.xop = NewMkOperator(op) + mkline.xvalign = valueAlign mkline.xvalue = value mkline.xcomment = comment mkline.Tokenize(value) @@ -122,6 +124,7 @@ func (mkline *MkLine) Varname() string { return mkline.xs1 } func (mkline *MkLine) Varcanon() string { return mkline.xs2 } func (mkline *MkLine) Varparam() string { return mkline.xs3 } func (mkline *MkLine) Op() MkOperator { return mkline.xop } +func (mkline *MkLine) ValueAlign() string { return mkline.xvalign } func (mkline *MkLine) Value() string { return mkline.xvalue } func (mkline *MkLine) Comment() string { return mkline.xcomment } func (mkline *MkLine) IsShellcmd() bool { return mkline.xtype == 2 } @@ -753,25 +756,6 @@ func (mkline *MkLine) withoutMakeVariables(value string, qModifierAllowed bool) } } -func (mkline *MkLine) CheckVaralign() { - if !G.opts.WarnSpace { - return - } - - if m, prefix, align := match2(mkline.Line.Text, `^( *[-*+A-Z_a-z0-9.${}\[]+\s*[!:?]?=)(\s*)`); m { - if align != " " && strings.Trim(align, "\t") != "" { - alignedWidth := tabLength(prefix + align) - tabs := "" - for tabLength(prefix+tabs) < alignedWidth { - tabs += "\t" - } - if !mkline.Line.AutofixReplace(prefix+align, prefix+tabs) { - mkline.Note0("Alignment of variable values should be done with tabs, not spaces.") - } - } - } -} - func (mkline *MkLine) CheckText(text string) { if G.opts.DebugTrace { defer tracecall1(text)() diff --git a/pkgtools/pkglint/files/mkline_test.go b/pkgtools/pkglint/files/mkline_test.go index afe599571ae..fbf9b1e47b8 100644 --- a/pkgtools/pkglint/files/mkline_test.go +++ b/pkgtools/pkglint/files/mkline_test.go @@ -32,49 +32,208 @@ func (s *Suite) TestChecklineMkVartype(c *check.C) { mkline.CheckVartype("DISTNAME", opAssign, "gcc-${GCC_VERSION}", "") } -func (s *Suite) TestChecklineMkVaralign(c *check.C) { +func (s *Suite) TestMkLine_CheckVaralign_Autofix(c *check.C) { s.UseCommandLine(c, "-Wspace", "-f") lines := s.NewLines("file.mk", "VAR= value", // Indentation 7, fixed to 8. + "", // "VAR= value", // Indentation 8, fixed to 8. - "VAR= value", // Indentation 9, fixed to 16. + "", // + "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", // Mixed indentation 16, fixed to 8. + "", // "VAR=\tvalue") // Already aligned with tabs only, left unchanged. + varalign := new(VaralignBlock) for _, line := range lines { - NewMkLine(line).CheckVaralign() + varalign.Check(NewMkLine(line)) } + varalign.Finish() c.Check(lines[0].changed, equals, true) c.Check(lines[0].rawLines()[0].String(), equals, "1:VAR=\tvalue\n") - c.Check(lines[1].changed, equals, true) - c.Check(lines[1].rawLines()[0].String(), equals, "2:VAR=\tvalue\n") c.Check(lines[2].changed, equals, true) - c.Check(lines[2].rawLines()[0].String(), equals, "3:VAR=\t\tvalue\n") - c.Check(lines[3].changed, equals, true) - c.Check(lines[3].rawLines()[0].String(), equals, "4:VAR=\tvalue\n") + c.Check(lines[2].rawLines()[0].String(), equals, "3:VAR=\tvalue\n") c.Check(lines[4].changed, equals, true) c.Check(lines[4].rawLines()[0].String(), equals, "5:VAR=\tvalue\n") - c.Check(lines[5].changed, equals, true) - c.Check(lines[5].rawLines()[0].String(), equals, "6:VAR=\t\tvalue\n") - c.Check(lines[6].changed, equals, false) + c.Check(lines[6].changed, equals, true) c.Check(lines[6].rawLines()[0].String(), equals, "7:VAR=\tvalue\n") + c.Check(lines[8].changed, equals, true) + c.Check(lines[8].rawLines()[0].String(), equals, "9:VAR=\tvalue\n") + c.Check(lines[10].changed, equals, true) + c.Check(lines[10].rawLines()[0].String(), equals, "11:VAR=\tvalue\n") + c.Check(lines[12].changed, equals, false) + c.Check(lines[12].rawLines()[0].String(), equals, "13:VAR=\tvalue\n") c.Check(s.Output(), equals, ""+ - "NOTE: file.mk:1: Alignment of variable values should be done with tabs, not spaces.\n"+ + "NOTE: file.mk:1: This variable value should be aligned with tabs, not spaces, to column 9.\n"+ "AUTOFIX: file.mk:1: Replacing \"VAR= \" with \"VAR=\\t\".\n"+ - "NOTE: file.mk:2: Alignment of variable values should be done with tabs, not spaces.\n"+ - "AUTOFIX: file.mk:2: Replacing \"VAR= \" with \"VAR=\\t\".\n"+ - "NOTE: file.mk:3: Alignment of variable values should be done with tabs, not spaces.\n"+ - "AUTOFIX: file.mk:3: Replacing \"VAR= \" with \"VAR=\\t\\t\".\n"+ - "NOTE: file.mk:4: Alignment of variable values should be done with tabs, not spaces.\n"+ - "AUTOFIX: file.mk:4: Replacing \"VAR= \\t\" with \"VAR=\\t\".\n"+ - "NOTE: file.mk:5: Alignment of variable values should be done with tabs, not spaces.\n"+ - "AUTOFIX: file.mk:5: Replacing \"VAR= \\t\" with \"VAR=\\t\".\n"+ - "NOTE: file.mk:6: Alignment of variable values should be done with tabs, not spaces.\n"+ - "AUTOFIX: file.mk:6: Replacing \"VAR= \\t\" with \"VAR=\\t\\t\".\n") - c.Check(tabLength("VAR= \t"), equals, 16) + "NOTE: file.mk:3: Variable values should be aligned with tabs, not spaces.\n"+ + "AUTOFIX: file.mk:3: Replacing \"VAR= \" with \"VAR=\\t\".\n"+ + "NOTE: file.mk:5: This variable value should be aligned with tabs, not spaces, to column 9.\n"+ + "AUTOFIX: file.mk:5: Replacing \"VAR= \" with \"VAR=\\t\".\n"+ + "NOTE: file.mk:7: Variable values should be aligned with tabs, not spaces.\n"+ + "AUTOFIX: file.mk:7: Replacing \"VAR= \\t\" with \"VAR=\\t\".\n"+ + "NOTE: file.mk:9: Variable values should be aligned with tabs, not spaces.\n"+ + "AUTOFIX: file.mk:9: Replacing \"VAR= \\t\" with \"VAR=\\t\".\n"+ + "NOTE: file.mk:11: This variable value should be aligned with tabs, not spaces, to column 9.\n"+ + "AUTOFIX: file.mk:11: Replacing \"VAR= \\t\" with \"VAR=\\t\".\n") +} + +func (s *Suite) TestMkLine_CheckVaralign_ReduceIndentation(c *check.C) { + s.UseCommandLine(c, "-Wspace") + mklines := s.NewMkLines("file.mk", + "VAR= \tvalue", + "VAR= \tvalue", + "VAR=\t\t\t\tvalue", + "", + "VAR=\t\t\tneedlessly", // Nothing to be fixed here, since it looks good. + "VAR=\t\t\tdeep", + "VAR=\t\t\tindentation") + + varalign := new(VaralignBlock) + for _, mkline := range mklines.mklines { + varalign.Check(mkline) + } + varalign.Finish() + + c.Check(s.Output(), equals, ""+ + "NOTE: file.mk:1: Variable values should be aligned with tabs, not spaces.\n"+ + "NOTE: file.mk:2: This variable value should be aligned with tabs, not spaces, to column 9.\n"+ + "NOTE: file.mk:3: This variable value should be aligned to column 9.\n") +} + +func (s *Suite) TestMkLine_CheckVaralign_LongestLineEmptyAlignment(c *check.C) { + s.UseCommandLine(c, "-Wspace") + mklines := s.NewMkLines("file.mk", + "SUBST_CLASSES+= aaaaaaaa", + "SUBST_STAGE.aaaaaaaa= pre-configure", + "SUBST_FILES.aaaaaaaa= *.pl", + "SUBST_FILTER_CMD.aaaaaaaa=cat") + + varalign := new(VaralignBlock) + for _, mkline := range mklines.mklines { + varalign.Check(mkline) + } + varalign.Finish() + + c.Check(s.Output(), equals, ""+ + "NOTE: file.mk:1: This variable value should be aligned with tabs, not spaces, to column 33.\n"+ + "NOTE: file.mk:2: This variable value should be aligned with tabs, not spaces, to column 33.\n"+ + "NOTE: file.mk:3: This variable value should be aligned with tabs, not spaces, to column 33.\n"+ + "NOTE: file.mk:4: This variable value should be aligned to column 33.\n") +} + +func (s *Suite) TestMkLine_CheckVaralign_OnlySpaces(c *check.C) { + s.UseCommandLine(c, "-Wspace") + mklines := s.NewMkLines("file.mk", + "SUBST_CLASSES+= aaaaaaaa", + "SUBST_STAGE.aaaaaaaa= pre-configure", + "SUBST_FILES.aaaaaaaa= *.pl", + "SUBST_FILTER_CMD.aaaaaaaa= cat") + + varalign := new(VaralignBlock) + for _, mkline := range mklines.mklines { + varalign.Check(mkline) + } + varalign.Finish() + + c.Check(s.Output(), equals, ""+ + "NOTE: file.mk:1: This variable value should be aligned with tabs, not spaces, to column 33.\n"+ + "NOTE: file.mk:2: This variable value should be aligned with tabs, not spaces, to column 33.\n"+ + "NOTE: file.mk:3: This variable value should be aligned with tabs, not spaces, to column 33.\n"+ + "NOTE: file.mk:4: This variable value should be aligned with tabs, not spaces, to column 33.\n") +} + +func (s *Suite) TestMkLine_CheckVaralign_Advanced(c *check.C) { + s.UseCommandLine(c, "-Wspace") + fname := s.CreateTmpFileLines(c, "Makefile", + "# $"+"NetBSD$", + "", + "VAR= \\", // In continuation lines, indenting with spaces is ok + "\tvalue", + "", + "VAR= indented with one space", // Exactly one space is ok in general + "VAR= indented with two spaces", // Two spaces are uncommon + "", + "BLOCK=\tindented with tab", + "BLOCK_LONGVAR= indented with space", // This is ok, to prevent the block from being indented further + "", + "BLOCK=\tshort", + "BLOCK_LONGVAR=\tlong", + "", + "GRP_A= avalue", // The values in a block should be aligned + "GRP_AA= value", + "GRP_AAA= value", + "GRP_AAAA= value", + "", + "VAR=\t${VAR}${BLOCK}${BLOCK_LONGVAR} # suppress warnings about unused variables", + "VAR=\t${GRP_A}${GRP_AA}${GRP_AAA}${GRP_AAAA}") + mklines := NewMkLines(LoadExistingLines(fname, true)) + + mklines.Check() + + c.Check(s.OutputCleanTmpdir(), equals, ""+ + "NOTE: ~/Makefile:6: This variable value should be aligned with tabs, not spaces, to column 9.\n"+ + "NOTE: ~/Makefile:7: This variable value should be aligned with tabs, not spaces, to column 9.\n"+ + "NOTE: ~/Makefile:12: This variable value should be aligned to column 17.\n"+ + "NOTE: ~/Makefile:15: This variable value should be aligned with tabs, not spaces, to column 17.\n"+ + "NOTE: ~/Makefile:16: This variable value should be aligned with tabs, not spaces, to column 17.\n"+ + "NOTE: ~/Makefile:17: This variable value should be aligned with tabs, not spaces, to column 17.\n"+ + "NOTE: ~/Makefile:18: This variable value should be aligned with tabs, not spaces, to column 17.\n") + + s.UseCommandLine(c, "-Wspace", "--autofix") + + mklines.Check() + + c.Check(s.OutputCleanTmpdir(), equals, ""+ + "AUTOFIX: ~/Makefile:6: Replacing \"VAR= \" with \"VAR=\\t\".\n"+ + "AUTOFIX: ~/Makefile:7: Replacing \"VAR= \" with \"VAR=\\t\".\n"+ + "AUTOFIX: ~/Makefile:12: Replacing \"BLOCK=\\t\" with \"BLOCK=\\t\\t\".\n"+ + "AUTOFIX: ~/Makefile:15: Replacing \"GRP_A= \" with \"GRP_A=\\t\\t\".\n"+ + "AUTOFIX: ~/Makefile:16: Replacing \"GRP_AA= \" with \"GRP_AA=\\t\\t\".\n"+ + "AUTOFIX: ~/Makefile:17: Replacing \"GRP_AAA= \" with \"GRP_AAA=\\t\".\n"+ + "AUTOFIX: ~/Makefile:18: Replacing \"GRP_AAAA= \" with \"GRP_AAAA=\\t\".\n"+ + "AUTOFIX: ~/Makefile: Has been auto-fixed. Please re-run pkglint.\n") + c.Check(s.LoadTmpFile(c, "Makefile"), equals, ""+ + "# $NetBSD: mkline_test.go,v 1.7 2016/01/24 02:03:28 rillig Exp $\n"+ + "\n"+ + "VAR= \\\n"+ + "\tvalue\n"+ + "\n"+ + "VAR=\tindented with one space\n"+ + "VAR=\tindented with two spaces\n"+ + "\n"+ + "BLOCK=\tindented with tab\n"+ + "BLOCK_LONGVAR= indented with space\n"+ + "\n"+ + "BLOCK=\t\tshort\n"+ + "BLOCK_LONGVAR=\tlong\n"+ + "\n"+ + "GRP_A=\t\tavalue\n"+ + "GRP_AA=\t\tvalue\n"+ + "GRP_AAA=\tvalue\n"+ + "GRP_AAAA=\tvalue\n"+ + "\n"+ + "VAR=\t${VAR}${BLOCK}${BLOCK_LONGVAR} # suppress warnings about unused variables\n"+ + "VAR=\t${GRP_A}${GRP_AA}${GRP_AAA}${GRP_AAAA}\n") +} + +func (s *Suite) TestMkLine_CheckVaralign_Misc(c *check.C) { + s.UseCommandLine(c, "-Wspace") + mklines := s.NewMkLines("Makefile", + "# $"+"NetBSD$", + "", + "VAR= space", + "VAR=\ttab ${VAR}") + + mklines.Check() + + c.Check(s.Output(), equals, "NOTE: Makefile:3: Variable values should be aligned with tabs, not spaces.\n") } func (s *Suite) TestMkLine_fields(c *check.C) { @@ -285,7 +444,8 @@ func (s *Suite) TestMkLine_CheckVarusePermissions(c *check.C) { "WARN: options.mk:3: PKGBASE should not be evaluated at load time.\n"+ "WARN: options.mk:4: The variable PYPKGPREFIX may not be set in this file; it would be ok in pyversion.mk.\n"+ "WARN: options.mk:4: \"${PKGBASE}\" is not valid for PYPKGPREFIX. Use one of { py27 py33 py34 } instead.\n"+ - "WARN: options.mk:4: PKGBASE should not be evaluated indirectly at load time.\n") + "WARN: options.mk:4: PKGBASE should not be evaluated indirectly at load time.\n"+ + "NOTE: options.mk:4: This variable value should be aligned to column 17.\n") } func (s *Suite) TestMkLine_WarnVaruseLocalbase(c *check.C) { diff --git a/pkgtools/pkglint/files/mklines.go b/pkgtools/pkglint/files/mklines.go index 4390b313c97..0e0146e08f9 100644 --- a/pkgtools/pkglint/files/mklines.go +++ b/pkgtools/pkglint/files/mklines.go @@ -84,14 +84,10 @@ func (mklines *MkLines) Check() { defer tracecall1(mklines.lines[0].Fname)() } - allowedTargets := make(map[string]bool) - substcontext := new(SubstContext) - G.Mk = mklines defer func() { G.Mk = nil }() - mklines.DetermineUsedVariables() - + allowedTargets := make(map[string]bool) prefixes := splitOnSpace("pre do post") actions := splitOnSpace("fetch extract patch tools wrapper configure build test install package clean") for _, prefix := range prefixes { @@ -102,15 +98,19 @@ func (mklines *MkLines) Check() { // In the first pass, all additions to BUILD_DEFS and USE_TOOLS // are collected to make the order of the definitions irrelevant. + mklines.DetermineUsedVariables() mklines.determineDefinedVariables() // In the second pass, the actual checks are done. mklines.lines[0].CheckRcsid(`#\s+`, "# ") + substcontext := new(SubstContext) + varalign := new(VaralignBlock) for _, mkline := range mklines.mklines { mkline.Line.CheckTrailingWhitespace() mkline.Line.CheckValidCharacters(`[\t -~]`) + varalign.Check(mkline) switch { case mkline.IsEmpty(): @@ -118,7 +118,6 @@ func (mklines *MkLines) Check() { case mkline.IsVarassign(): mklines.target = "" - mkline.CheckVaralign() mkline.CheckVarassign() substcontext.Varassign(mkline) @@ -140,6 +139,7 @@ func (mklines *MkLines) Check() { } lastMkline := mklines.mklines[len(mklines.mklines)-1] substcontext.Finish(lastMkline) + varalign.Finish() ChecklinesTrailingEmptyLines(mklines.lines) @@ -379,3 +379,129 @@ func (mklines *MkLines) checklineInclude(mkline *MkLine) { mkline.Line.Error2("%s must not be included directly. Include \"%s/buildlink3.mk\" instead.", includefile, dir) } } + +type VaralignBlock struct { + info []struct { + mkline *MkLine + prefix string + align string + } + skip bool + differ bool + maxPrefixWidth int + maxSpaceWidth int + maxTabWidth int +} + +func (va *VaralignBlock) Check(mkline *MkLine) { + if !G.opts.WarnSpace || mkline.Line.IsMultiline() || mkline.IsComment() || mkline.IsCond() { + return + } + if mkline.IsEmpty() { + va.Finish() + return + } + if !mkline.IsVarassign() { + va.skip = true + return + } + + valueAlign := mkline.ValueAlign() + prefix := strings.TrimRight(valueAlign, " \t") + align := valueAlign[len(prefix):] + + va.info = append(va.info, struct { + mkline *MkLine + prefix string + align string + }{mkline, prefix, align}) + + alignedWidth := tabLength(valueAlign) + if contains(align, " ") { + if va.maxSpaceWidth != 0 && alignedWidth != va.maxSpaceWidth { + va.differ = true + } + if alignedWidth > va.maxSpaceWidth { + va.maxSpaceWidth = alignedWidth + } + } else { + if va.maxTabWidth != 0 && alignedWidth != va.maxTabWidth { + va.differ = true + } + if alignedWidth > va.maxTabWidth { + va.maxTabWidth = alignedWidth + } + } + + va.maxPrefixWidth = imax(va.maxPrefixWidth, tabLength(prefix)) +} + +func (va *VaralignBlock) Finish() { + if !va.skip { + for _, info := range va.info { + if !info.mkline.Line.IsMultiline() { + va.fixalign(info.mkline, info.prefix, info.align) + } + } + } + *va = VaralignBlock{} +} + +func (va *VaralignBlock) fixalign(mkline *MkLine, prefix, oldalign string) { + if mkline.Value() == "" && mkline.Comment() == "" { + return + } + + hasSpace := contains(oldalign, " ") + if hasSpace && + va.maxTabWidth != 0 && + va.maxSpaceWidth > va.maxTabWidth && + tabLength(prefix+oldalign) == va.maxSpaceWidth { + return + } + + goodWidth := va.maxTabWidth + if goodWidth == 0 && va.differ { + goodWidth = va.maxSpaceWidth + } + minWidth := va.maxPrefixWidth + 1 + if goodWidth == 0 || minWidth < goodWidth && va.differ { + goodWidth = minWidth + } + goodWidth = (goodWidth + 7) & -8 + + newalign := "" + for tabLength(prefix+newalign) < goodWidth { + newalign += "\t" + } + if newalign == oldalign { + return + } + + if !mkline.Line.AutofixReplace(prefix+oldalign, prefix+newalign) { + wrongColumn := tabLength(prefix+oldalign) != tabLength(prefix+newalign) + switch { + case hasSpace && wrongColumn: + mkline.Line.Notef("This variable value should be aligned with tabs, not spaces, to column %d.", goodWidth+1) + case hasSpace: + mkline.Line.Notef("Variable values should be aligned with tabs, not spaces.") + case wrongColumn: + mkline.Line.Notef("This variable value should be aligned to column %d.", goodWidth+1) + } + if wrongColumn { + Explain( + "Normally, all variable values in a block should start at the same", + "column. There are some exceptions to this rule:", + "", + "Definitions for long variable names may be indented with a single", + "space instead of tabs, but only if they appear in a block that is", + "otherwise indented using tabs.", + "", + "Variable definitions that span multiple lines are not checked for", + "alignment at all.", + "", + "When the block contains something else than variable definitions,", + "it is not checked at all.") + } + } +} diff --git a/pkgtools/pkglint/files/pkglint.go b/pkgtools/pkglint/files/pkglint.go index 12e6091d474..c9d76126ec5 100644 --- a/pkgtools/pkglint/files/pkglint.go +++ b/pkgtools/pkglint/files/pkglint.go @@ -303,7 +303,7 @@ func ChecklinesTrailingEmptyLines(lines []*Line) { } } -func MatchVarassign(text string) (m bool, varname, op, value, comment string) { +func MatchVarassign(text string) (m bool, varname, op, valueAlign, value, comment string) { i, n := 0, len(text) for i < n && text[i] == ' ' { @@ -338,7 +338,7 @@ func MatchVarassign(text string) (m bool, varname, op, value, comment string) { opStart := i if i < n { - if b := text[i]; b&0xE0 == 0x20 && (uint(0x84000802)>>(b&0x1F))&1 != 0 { + if b := text[i]; b == '!' || b == '+' || b == ':' || b == '?' { i++ } } @@ -377,6 +377,7 @@ func MatchVarassign(text string) (m bool, varname, op, value, comment string) { m = true varname = text[varnameStart:varnameEnd] op = text[opStart:opEnd] + valueAlign = text[0:valueStart] value = strings.TrimSpace(string(valuebuf[:j])) comment = text[commentStart:commentEnd] return diff --git a/pkgtools/pkglint/files/pkglint_test.go b/pkgtools/pkglint/files/pkglint_test.go index 675b4e49666..8df1aa903cd 100644 --- a/pkgtools/pkglint/files/pkglint_test.go +++ b/pkgtools/pkglint/files/pkglint_test.go @@ -85,34 +85,34 @@ func (s *Suite) TestChecklineRcsid(c *check.C) { } func (s *Suite) TestMatchVarassign(c *check.C) { - checkVarassign := func(text string, ck check.Checker, varname, op, value, comment string) { + checkVarassign := func(text string, ck check.Checker, varname, op, align, value, comment string) { type va struct { - varname, op, value, comment string + varname, op, align, value, comment string } - expected := va{varname, op, value, comment} - am, avarname, aop, avalue, acomment := MatchVarassign(text) + expected := va{varname, op, align, value, comment} + am, avarname, aop, aalign, avalue, acomment := MatchVarassign(text) if !am { c.Errorf("Text %q doesn’t match variable assignment", text) return } - actual := va{avarname, aop, avalue, acomment} + actual := va{avarname, aop, aalign, avalue, acomment} c.Check(actual, ck, expected) } checkNotVarassign := func(text string) { - m, _, _, _, _ := MatchVarassign(text) + m, _, _, _, _, _ := MatchVarassign(text) if m { c.Errorf("Text %q matches variable assignment, but shouldn’t.", text) } } - checkVarassign("C++=c11", equals, "C+", "+=", "c11", "") - checkVarassign("V=v", equals, "V", "=", "v", "") - checkVarassign("VAR=#comment", equals, "VAR", "=", "", "#comment") - checkVarassign("VAR=\\#comment", equals, "VAR", "=", "#comment", "") - checkVarassign("VAR=\\\\\\##comment", equals, "VAR", "=", "\\\\#", "#comment") - checkVarassign("VAR=\\", equals, "VAR", "=", "\\", "") - checkVarassign("VAR += value", equals, "VAR", "+=", "value", "") - checkVarassign(" VAR=value", equals, "VAR", "=", "value", "") + checkVarassign("C++=c11", equals, "C+", "+=", "C++=", "c11", "") + checkVarassign("V=v", equals, "V", "=", "V=", "v", "") + checkVarassign("VAR=#comment", equals, "VAR", "=", "VAR=", "", "#comment") + checkVarassign("VAR=\\#comment", equals, "VAR", "=", "VAR=", "#comment", "") + checkVarassign("VAR=\\\\\\##comment", equals, "VAR", "=", "VAR=", "\\\\#", "#comment") + checkVarassign("VAR=\\", equals, "VAR", "=", "VAR=", "\\", "") + checkVarassign("VAR += value", equals, "VAR", "+=", "VAR += ", "value", "") + checkVarassign(" VAR=value", equals, "VAR", "=", " VAR=", "value", "") checkNotVarassign("\tVAR=value") checkNotVarassign("?=value") checkNotVarassign("<=value") |