 @@ -1,17 +1,16 @@
-# $NetBSD: Makefile,v 1.629 2020/02/02 14:19:09 bsiegert Exp $
+# $NetBSD: Makefile,v 1.630 2020/02/05 04:09:00 rillig Exp $
-PKGNAME=	pkglint-19.4.6
+PKGNAME=	pkglint-19.4.7
 PKGREVISION=	1
 CATEGORIES=	pkgtools
 DISTNAME=	tools
 MASTER_SITES=	${MASTER_SITE_GITHUB:=golang/}
 GITHUB_PROJECT=	tools
 GITHUB_TAG=	92d8274bd7b8a4c65f24bafe401a029e58392704
 MAINTAINER=	rillig@NetBSD.org
 HOMEPAGE=	https://github.com/rillig/pkglint
 COMMENT=	Verifier for NetBSD packages
 LICENSE=	2-clause-bsd
 CONFLICTS+=	pkglint4-[0-9]*
 USE_TOOLS+=		pax

 @@ -30,30 +30,35 @@ import (
 //  - https://sf.net/projects/$project/
 //  - http://sf.net/projects/$project/
 //  - https://sf.net/p/$project/
 //  - http://sf.net/p/$project/
 //
 // TODO: implement complete homepage migration for SourceForge.
 // TODO: allow to suppress the automatic migration for SourceForge,
 //  even if it is not about https vs. http.
 type HomepageChecker struct {
 	Value      string
 	ValueNoVar string
 	MkLine     *MkLine
 	MkLines    *MkLines
 	// Can be mocked for the tests.
 	isReachable func(url string) YesNoUnknown
+}
 func NewHomepageChecker(value string, valueNoVar string, mkline *MkLine, mklines *MkLines) *HomepageChecker {
-	return &HomepageChecker{value, valueNoVar, mkline, mklines}
+	ck := HomepageChecker{value, valueNoVar, mkline, mklines, nil}
 	ck.isReachable = ck.isReachableOnline
 	return &ck
+}
 func (ck *HomepageChecker) Check() {
 	ck.checkBasedOnMasterSites()
 	ck.checkFtp()
 	ck.checkHttp()
 	ck.checkBadUrls()
 	ck.checkReachable()
+}
 func (ck *HomepageChecker) checkBasedOnMasterSites() {
 	m, wrong, sitename, subdir := match3(ck.Value, `^(\$\{(MASTER_SITE\w+)(?::=([\w\-/]+))?\})`)
 	if !m {
 @@ -109,133 +114,127 @@ func (ck *HomepageChecker) checkFtp() {
 	mkline.Explain(
 		"This homepage URL has probably been generated by url2pkg",
 		"and not been reviewed by the package author.",
 		"",
 		"In most cases there exists a more welcoming URL,",
 		"which is usually served via HTTP.")
+}
 func (ck *HomepageChecker) checkHttp() {
 	if ck.MkLine.HasRationale("http", "https") {
 		return
+	}
-	shouldAutofix, from, to := ck.toHttps(ck.Value)
+	migrate, from, to := ck.migrate(ck.Value)
-	if from == "" {
+	if !migrate {
 		return
+	}
 	fix := ck.MkLine.Autofix()
 	fix.Warnf("HOMEPAGE should migrate from %s to %s.", from, to)
-	if shouldAutofix {
+	fix.Replace(from, to)
-		fix.Replace(from, to)
+	if from == "http" && to == "https" {
 		fix.Explain(
 			"To provide secure communication by default,",
 			"the HOMEPAGE URL should use the https protocol if available.",
 			"",
 			"If the HOMEPAGE really does not support https,",
 			"add a comment near the HOMEPAGE variable stating this clearly.")
+	}
 	fix.Explain(
 		"To provide secure communication by default,",
 		"the HOMEPAGE URL should use the https protocol if available.",
 		"",
 		"If the HOMEPAGE really does not support https,",
 		"add a comment near the HOMEPAGE variable stating this clearly.")
 	fix.Apply()
+}
-// toHttps checks whether the homepage should be migrated from http to https
+// migrate checks whether the homepage should be migrated from http to https
 // and which part of the homepage URL needs to be modified for that.
 //
 // If for some reason the https URL should not be reachable but the
 // corresponding http URL is, the homepage is changed back to http.
-func (ck *HomepageChecker) toHttps(url string) (bool, string, string) {
+func (ck *HomepageChecker) migrate(url string) (bool, string, string) {
-	m, scheme, host, port := match3(url, `(https?)://([A-Za-z0-9-.]+)(:[0-9]+)?`)
+	m, scheme, host := match2(url, `(https?)://([A-Za-z0-9-.]+)(?:/|$)?`)
-	if !m {
+	if !m || containsVarRefLong(url) {
 		return false, "", ""
+	}
-	if ck.hasAnySuffix(host,
+	if scheme == "http" && ck.hasAnySuffix(host,
-		"www.gnustep.org",           // 2020-01-18
+		"www.gnustep.org",           // no https as of 2020-01-18
-		"aspell.net",                // 2020-01-18
+		"aspell.net",                // no https as of 2020-01-18
 		"downloads.sourceforge.net", // gets another warning already
-		".dl.sourceforge.net",       // gets another warning already
+		"dl.sourceforge.net",        // gets another warning already
 	) {
 		return false, "", ""
+	}
 	if scheme == "http" && ck.hasAnySuffix(host,
 		"apache.org",
 		"archive.org",
 		"ctan.org",
 		"freedesktop.org",
 		"github.com",
 		"github.io",
 		"gnome.org",
 		"gnu.org",
 		"kde.org",
 		"kldp.net",
 		"linuxfoundation.org",
 		"NetBSD.org",
 		"nongnu.org",
 		"tryton.org",
 		"tug.org") {
-		return port == "", "http", "https"
+		return true, "http", "https"
+	}
-	if scheme == "http" && host == "sf.net" {
+	if host == "sf.net" {
-		return port == "", "http://sf.net", "https://sourceforge.net"
+		// sf.net redirects to sourceforge.net
 		return true, scheme + "://sf.net", "https://sourceforge.net"
+	}
 	from := scheme
 	to := "https"
 	toReachable := unknown
 	// SourceForge projects use either http://project.sourceforge.net or
 	// https://project.sourceforge.io (not net).
-	if m, project := match1(host, `^([\w-]+)\.(?:sf|sourceforge)\.net$`); m {
+	if m, project, domain := match2(host, `^([\w-]+)\.((?:sf|sourceforge)\.net)$`); m {
 		if scheme == "http" {
 			from = scheme + "://" + host
 			// See https://sourceforge.net/p/forge/documentation/Custom%20VHOSTs
 			from = "http://" + host
 			to = "https://" + project + ".sourceforge.io"
 		} else {
-			from = "sourceforge.net"
+			from = domain
 			to = "sourceforge.io"
 			// Roll back wrong https SourceForge homepages generated by:
 			// https://mail-index.netbsd.org/pkgsrc-changes/2020/01/18/msg205146.html
-			if port == "" && G.Opts.Network {
+			_, migrated := replaceOnce(url, from, to)
-				_, migrated := replaceOnce(url, from, to)
+			if ck.isReachable(migrated) == no {
-				if ck.isReachable(migrated) == no {
+				_, httpOnly := replaceOnce(url, "https://", "http://")
-					ok, httpOnly := replaceOnce(url, "https://", "http://")
+				if ck.isReachable(httpOnly) == yes && ck.isReachable(url) == no {
-					if ok && ck.isReachable(httpOnly) == yes && ck.isReachable(url) == no {
+					return true, "https", "http"
 						from = "https"
 						to = "http"
 						toReachable = yes
+				}
+			}
+		}
+	}
 	if from == to {
 		return false, "", ""
+	}
-	shouldAutofix := toReachable == yes
+	_, migrated := replaceOnce(url, from, to)
-	if port == "" && G.Opts.Network && toReachable == unknown {
+	migrate := ck.isReachable(migrated)
-		_, migrated := replaceOnce(url, from, to)
+	if migrate == yes {
-		if ck.isReachable(migrated) == yes {
+		return true, from, to
 			shouldAutofix = true
 		} else {
 			return false, "", ""
+	}
 	return shouldAutofix, from, to
 	return false, "", ""
+}
 func (ck *HomepageChecker) checkBadUrls() {
 	m, host := match1(ck.Value, `https?://([A-Za-z0-9-.]+)`)
 	if !m {
 		return
+	}
 	if !ck.hasAnySuffix(host,
 		".dl.sourceforge.net",
 		"downloads.sourceforge.net") {
 		return
+	}
 @@ -282,27 +281,27 @@ func (ck *HomepageChecker) checkReachabl
 	location, err := response.Location()
 	if err == nil {
 		mkline.Warnf("Homepage %q redirects to %q.", url, location.String())
 		return
+	}
 	if response.StatusCode != 200 {
 		mkline.Warnf("Homepage %q returns HTTP status %q.", url, response.Status)
 		return
+	}
+}
-func (*HomepageChecker) isReachable(url string) YesNoUnknown {
+func (*HomepageChecker) isReachableOnline(url string) YesNoUnknown {
 	switch {
 	case !G.Opts.Network,
 		containsVarRefLong(url),
 		!matches(url, `^https?://[A-Za-z0-9-.]+(?::[0-9]+)?/[!-~]*$`):
 		return unknown
+	}
 	var client http.Client
 	client.Timeout = 3 * time.Second
 	client.CheckRedirect = func(req *http.Request, via []*http.Request) error {
 		return http.ErrUseLastResponse
+	}

 @@ -122,88 +122,163 @@ func (s *Suite) Test_HomepageChecker_che
 	vt.Varname("HOMEPAGE")
 	vt.Values(
 		"http://www.gnustep.org/",
 		"http://www.pkgsrc.org/",
 		"http://project.sourceforge.net/",
 		"http://sf.net/p/project/",
 		"http://sourceforge.net/p/project/",
 		"http://example.org/ # doesn't support https",
 		"http://example.org/ # only supports http",
 		"http://asf.net/")
 	vt.Output(
-		"WARN: filename.mk:2: HOMEPAGE should migrate from http to https.",
+		"WARN: filename.mk:4: HOMEPAGE should migrate " +
-		"WARN: filename.mk:3: HOMEPAGE should migrate "+
+			"from http://sf.net to https://sourceforge.net.")
 			"from http://project.sourceforge.net "+
 			"to https://project.sourceforge.io.",
 		"WARN: filename.mk:4: HOMEPAGE should migrate "+
 			"from http://sf.net to https://sourceforge.net.",
 		"WARN: filename.mk:5: HOMEPAGE should migrate from http to https.",
 		"WARN: filename.mk:8: HOMEPAGE should migrate from http to https.")
 	t.SetUpCommandLine("--autofix")
 	vt.Values(
 		"http://www.gnustep.org/",
 		"http://www.pkgsrc.org/",
 		"http://project.sourceforge.net/",
 		"http://sf.net/p/project/",
 		"http://sourceforge.net/p/project/",
 		"http://example.org/ # doesn't support https",
 		"http://example.org/ # only supports http",
 		"http://kde.org/",
 		"http://asf.net/")
 	// www.gnustep.org does not support https at all.
 	// www.pkgsrc.org is not in the (short) list of known https domains,
 	// therefore pkglint does not dare to change it automatically.
 	vt.Output(
 		"AUTOFIX: filename.mk:14: Replacing \"http://sf.net\" "+
 			"with \"https://sourceforge.net\".",
 		"AUTOFIX: filename.mk:18: Replacing \"http\" with \"https\".")
+}
-func (s *Suite) Test_HomepageChecker_toHttps(c *check.C) {
+func (s *Suite) Test_HomepageChecker_migrate(c *check.C) {
 	t := s.Init(c)
-	test := func(url string, shouldAutofix bool, from, to string) {
+	reachable := map[string]YesNoUnknown{}
-		toHttps := (*HomepageChecker).toHttps
+	used := map[string]bool{}
 		actualShouldAutofix, actualFrom, actualTo := toHttps(nil, url)
 	isReachable := func(url string) YesNoUnknown {
 		if r, ok := reachable[url]; ok {
 			used[url] = true
 			return r
+		}
 		panic(url)
+	}
 	test := func(url string, migrate bool, from, to string) {
 		ck := NewHomepageChecker(url, url, nil, nil)
 		ck.isReachable = isReachable
 		actualMigrate, actualFrom, actualTo := ck.migrate(url)
 		t.CheckDeepEquals(
-			[]interface{}{actualShouldAutofix, actualFrom, actualTo},
+			[]interface{}{actualMigrate, actualFrom, actualTo},
-			[]interface{}{shouldAutofix, from, to})
+			[]interface{}{migrate, from, to})
 		for key, _ := range reachable {
 			assertf(used[key], "Reachability of %q was not used.", key)
 			delete(reachable, key)
+		}
+	}
-	test("http://localhost/", false, "http", "https")
+	reachable["https://localhost/"] = unknown
 	test(
 		"http://localhost/",
 		false,
 		"",
 		"")
 	reachable["https://localhost/"] = yes
 	test(
 		"http://localhost/",
 		true,
 		"http",
 		"https")
 	reachable["https://project.sourceforge.io/"] = unknown
 	test(
 		"http://project.sourceforge.net/",
 		false,
 		"",
 		"")
 	reachable["https://project.sourceforge.io/"] = yes
 	test(
 		"http://project.sourceforge.net/",
 		true,
 		"http://project.sourceforge.net",
 		"https://project.sourceforge.io")
 	// To clean up the wrong autofix from 2020-01-18:
 	// https://mail-index.netbsd.org/pkgsrc-changes/2020/01/18/msg205146.html
 	reachable["https://project.sourceforge.io/"] = yes
 	test(
 		"https://project.sourceforge.net/",
-		false,
+		true,
 		"sourceforge.net",
 		"sourceforge.io")
 	// To clean up the wrong autofix from 2020-01-18:
 	// https://mail-index.netbsd.org/pkgsrc-changes/2020/01/18/msg205146.html
 	//
 	// If neither of the https URLs is reachable, the homepage
 	// is rolled back to the http URL.
 	reachable["https://project.sourceforge.io/"] = no
 	reachable["http://project.sourceforge.net/"] = yes
 	reachable["https://project.sourceforge.net/"] = no
 	test(
 		"https://project.sourceforge.net/",
 		true,
 		"https",
 		"http")
 	// To clean up the wrong autofix from 2020-01-18:
 	// https://mail-index.netbsd.org/pkgsrc-changes/2020/01/18/msg205146.html
 	//
 	// If the https URL of sourceforge.net is reachable,
 	// it is preferred over the http URL,
 	// even though it is not mentioned in the SourceForge documentation.
 	reachable["https://project.sourceforge.io/"] = no
 	reachable["http://project.sourceforge.net/"] = yes
 	reachable["https://project.sourceforge.net/"] = yes
 	test(
 		"https://project.sourceforge.net/",
 		false,
 		"",
 		"")
 	// To clean up the wrong autofix from 2020-01-18:
 	// https://mail-index.netbsd.org/pkgsrc-changes/2020/01/18/msg205146.html
 	reachable["https://project.sourceforge.io/"] = no
 	reachable["http://project.sourceforge.net/"] = no
 	test(
 		"https://project.sourceforge.net/",
 		false,
 		"",
 		"")
 	// Since the URL contains a variable, it cannot be resolved.
 	// Therefore it is skipped without any HTTP request being sent.
 	test(
 		"http://godoc.org/${GO_SRCPATH}",
 		false,
-		"http",
+		"",
-		"https")
+		"")
+}
 func (s *Suite) Test_HomepageChecker_checkBadUrls(c *check.C) {
 	t := s.Init(c)
 	vt := NewVartypeCheckTester(t, BtHomepage)
 	vt.Varname("HOMEPAGE")
 	vt.Values(
 		"http://garr.dl.sourceforge.net/project/name/dir/subdir/",
 		"https://downloads.sourceforge.net/project/name/dir/subdir/")
 	vt.Output(
 		"WARN: filename.mk:1: A direct download URL is not a user-friendly homepage.",
 @@ -249,80 +324,77 @@ func (s *Suite) Test_HomepageChecker_che
 		<-shutdown
 	}()
 	vt.Varname("HOMEPAGE")
 	vt.Values(
 		"http://localhost:28780/status/200",
 		"http://localhost:28780/status/301?location=/redirect301",
 		"http://localhost:28780/status/302?location=/redirect302",
 		"http://localhost:28780/status/307?location=/redirect307",
 		"http://localhost:28780/status/404",
 		"http://localhost:28780/status/500")
 	vt.Output(
 		"WARN: filename.mk:1: HOMEPAGE should migrate from http to https.",
 		"WARN: filename.mk:2: HOMEPAGE should migrate from http to https.",
 		"WARN: filename.mk:2: Homepage "+
 			"\"http://localhost:28780/status/301?location=/redirect301\" "+
 			"redirects to \"http://localhost:28780/redirect301\".",
 		"WARN: filename.mk:3: HOMEPAGE should migrate from http to https.",
 		"WARN: filename.mk:3: Homepage "+
 			"\"http://localhost:28780/status/302?location=/redirect302\" "+
 			"redirects to \"http://localhost:28780/redirect302\".",
 		"WARN: filename.mk:4: HOMEPAGE should migrate from http to https.",
 		"WARN: filename.mk:4: Homepage "+
 			"\"http://localhost:28780/status/307?location=/redirect307\" "+
 			"redirects to \"http://localhost:28780/redirect307\".",
 		"WARN: filename.mk:5: HOMEPAGE should migrate from http to https.",
 		"WARN: filename.mk:5: Homepage \"http://localhost:28780/status/404\" "+
 			"returns HTTP status \"404 Not Found\".",
 		"WARN: filename.mk:6: HOMEPAGE should migrate from http to https.",
 		"WARN: filename.mk:6: Homepage \"http://localhost:28780/status/500\" "+
 			"returns HTTP status \"500 Internal Server Error\".")
 	vt.Values(
 		"http://localhost:28780/timeout")
 	vt.Output(
-		"WARN: filename.mk:11: HOMEPAGE should migrate from http to https.",
+		"WARN: filename.mk:11: Homepage \"http://localhost:28780/timeout\" " +
 		"WARN: filename.mk:11: Homepage \"http://localhost:28780/timeout\" "+
 			"cannot be checked: timeout")
 	vt.Values(
 		"http://localhost:28780/%invalid")
 	vt.Output(
 		"WARN: filename.mk:21: HOMEPAGE should migrate from http to https.",
 		"ERROR: filename.mk:21: Invalid URL \"http://localhost:28780/%invalid\".")
 	vt.Values(
 		"http://localhost:28781/")
 	// The "unknown network error" is for compatibility with Go < 1.13.
 	t.CheckOutputMatches(
-		"WARN: filename.mk:31: HOMEPAGE should migrate from http to https.",
+		`^WARN: filename\.mk:31: Homepage "http://localhost:28781/" ` +
 		`^WARN: filename\.mk:31: Homepage "http://localhost:28781/" `+
 			`cannot be checked: (connection refused|unknown network error:.*)$`)
 	vt.Values(
 		"https://no-such-name.example.org/")
 	// The "unknown network error" is for compatibility with Go < 1.13.
 	t.CheckOutputMatches(
 		`^WARN: filename\.mk:41: Homepage "https://no-such-name.example.org/" ` +
 			`cannot be checked: (name not found|unknown network error:.*)$`)
 	vt.Values(
 		"https://!!!invalid/")
 	t.CheckOutputLines(
 		"WARN: filename.mk:51: \"https://!!!invalid/\" is not a valid URL.")
+}
-func (s *Suite) Test_HomepageChecker_isReachable(c *check.C) {
+func (s *Suite) Test_HomepageChecker_isReachableOnline(c *check.C) {
 	t := s.Init(c)
 	t.SetUpCommandLine("--network")
 	mux := http.NewServeMux()
 	mux.HandleFunc("/status/", func(writer http.ResponseWriter, request *http.Request) {
 		location := request.URL.Query().Get("location")
 		if location != "" {
 			writer.Header().Set("Location", location)
+		}
 		status, err := strconv.Atoi(request.URL.Path[len("/status/"):])
 		assertNil(err, "")
 @@ -342,27 +414,28 @@ func (s *Suite) Test_HomepageChecker_isR
 	go func() {
 		err := srv.Serve(listener)
 		assertf(err == http.ErrServerClosed, "%s", err)
 		shutdown <- true
 	}()
 	defer func() {
 		err := srv.Shutdown(context.Background())
 		assertNil(err, "")
 		<-shutdown
 	}()
 	test := func(url string, reachable YesNoUnknown) {
-		actual := (*HomepageChecker).isReachable(nil, url)
+		ck := NewHomepageChecker(url, url, nil, nil)
 		actual := ck.isReachableOnline(url)
 		t.CheckEquals(actual, reachable)
+	}
 	test("http://localhost:28780/status/200", yes)
 	test("http://localhost:28780/status/301?location=/", no)
 	test("http://localhost:28780/status/404", no)
 	test("http://localhost:28780/status/500", no)
 	test("http://localhost:28780/timeout", no)
 	test("http://localhost:28780/ok/${VAR}", unknown)
 	test("http://localhost:28780/ invalid", unknown)
 	test("http://localhost:28780/%invalid", no)
 	test("http://localhost:28781/", no)

 @@ -48,26 +48,29 @@ type LoggerOpts struct {
 	ShowSource,
 	GccOutput,
 	Quiet bool
 	Only []string
+}
 type LogLevel struct {
 	TraditionalName string
 	GccName         string
+}
 var (
 	// FIXME: By definition there cannot be fatal diagnostics.
 	//  Having these was a misconception from the beginning,
 	//  and they must be re-classified as fatal technical errors.
 	Fatal           = &LogLevel{"FATAL", "fatal"}
 	Error           = &LogLevel{"ERROR", "error"}
 	Warn            = &LogLevel{"WARN", "warning"}
 	Note            = &LogLevel{"NOTE", "note"}
 	AutofixLogLevel = &LogLevel{"AUTOFIX", "autofix"}
+)
 // Explain outputs an explanation for the preceding diagnostic
 // if the --explain option is given. Otherwise it just records
 // that an explanation is available.
 func (l *Logger) Explain(explanation ...string) {
 	if l.suppressExpl {
 		return

 @@ -1435,30 +1435,27 @@ func (s *Suite) Test_RedundantScope__is_
 		"PKG_OPTIONS=\t# empty",
 		"PKG_OPTIONS=\toverwritten")
 	t.CreateFileLines("options.mk",
 		"OUTSIDE:=\t# empty",
 		"OUTSIDE=\t# empty",
 		"OUTSIDE=\toverwritten",
 		".include \"mk/bsd.options.mk\"")
 	test := func(diagnostics ...string) {
 		mklines := t.LoadMkInclude("options.mk")
 		scope := NewRedundantScope()
 		scope.IsRelevant = func(mkline *MkLine) bool {
 			// See checkfilePackageMakefile.
-			if !G.Infrastructure && !G.Opts.CheckGlobal {
+			return G.Opts.CheckGlobal || !G.Pkgsrc.IsInfra(mkline.Filename)
 				return !G.Pkgsrc.IsInfra(mkline.Filename)
 			return true
+		}
 		scope.Check(mklines)
 		// No note about the redundant variable assignment in bsd.options.mk
 		// because it is part of the infrastructure, which is filtered out.
 		t.CheckOutput(diagnostics)
+	}
 	test(
 		"NOTE: ~/options.mk:2: "+
 			"Definition of OUTSIDE is redundant because of line 1.",
 		"WARN: ~/options.mk:2: "+

 @@ -645,26 +645,29 @@ func (info *varalignLine) alignContinuat
+	}
 	if column == 72 || column == rightMarginColumn || column <= valueColumn {
 		return
+	}
 	newSpace := " "
 	fix := info.fixer.Autofix()
 	if oldSpace == "" || rightMarginColumn == 0 {
 		fix.Notef("The continuation backslash should be preceded by a single space or tab.")
 	} else if info.isTooLongFor(valueColumn) {
 		fix.Notef("The continuation backslash should be preceded by a single space.")
 	} else {
 		newSpace = alignmentToWidths(info.uptoValueWidth(), rightMarginColumn)
 		if newSpace == "" {
 			newSpace = " "
+		}
 		fix.Notef(
 			"The continuation backslash should be in column %d, not %d.",
 			rightMarginColumn+1, column+1)
+	}
 	index := info.continuationIndex() - len(oldSpace)
 	fix.ReplaceAt(info.rawIndex, index, oldSpace, newSpace)
 	info.setSpaceBeforeContinuation(newSpace)
 	fix.Apply()
+}
 func (info *varalignLine) replaceSpaceBeforeValue(fix *Autofix, newSpace string) {
 	index := info.spaceBeforeValueIndex()
 	fix.ReplaceAt(info.rawIndex, index, info.spaceBeforeValue, newSpace)
 @@ -842,19 +845,22 @@ func (p *varalignParts) isCanonicalIniti
+	}
 	return strings.TrimLeft(space, "\t") == ""
+}
 // isCanonicalFollow returns whether the space before the value has its
 // canonical form, which is at least one tab, followed by up to 7 spaces.
 func (p *varalignParts) isCanonicalFollow() bool {
 	column := p.valueColumn()
 	return column >= 8 && p.spaceBeforeValue == indent(column)
+}
 func (p *varalignParts) isTooLongFor(valueColumn int) bool {
 	// FIXME: As of 2020-01-27, this method is called from
 	//  Test_VaralignBlock__right_margin with valueColumn == 0,
 	//  which doesn't make sense. It should at least be 8.
 	column := tabWidthAppend(valueColumn, p.value)
 	if p.isContinuation() {
 		column += 2
+	}
 	return column > 73
+}

 @@ -208,27 +208,27 @@ func (vt *VaralignTester) checkTestName(
 		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(descriptorsString(actual), descriptorsString(expected))
-	vt.tester.CheckDeepEquals(actual, expected)
+	vt.tester.CheckDeepEquals(expected, actual)
+}
 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 instead of 21.")
 	vt.Autofixes(
 		"AUTOFIX: Makefile:1: Replacing \"\" with \"\\t\".")
 	vt.Fixed(
 @@ -2053,26 +2053,49 @@ func (s *Suite) Test_VaralignBlock__var_
 			"indented with \"\\t\\t\".",
 		"NOTE: Makefile:3: This continuation line should be "+
 			"indented with \"\\t\\t\".")
 	vt.Autofixes(
 		"AUTOFIX: Makefile:2: Replacing \"\\t\" with \"\\t\\t\".",
 		"AUTOFIX: Makefile:3: Replacing \"\\t\" with \"\\t\\t\".")
 	vt.Fixed(
 		"PROGFILES=      67890 234567890 234567890 234567890 234567890 2 \\",
 		"                890 234567890 234567890 234567890 234567890 234567890 234567890 \\",
 		"                value")
 	vt.Run()
+}
 // Up to 2020-01-27, pkglint removed all spaces before the backslash,
 // which was against the rule of having at least one space.
 func (s *Suite) Test_VaralignBlock__right_margin(c *check.C) {
 	vt := NewVaralignTester(s, c)
 	vt.Input(
 		"CONFIGURE_ARGS+=\t\\",
 		"\t.....................................................................79\t\\",
 		"\t............................................................70 \t\t\\",
 		"\t........................................................66")
 	vt.Diagnostics(
 		"NOTE: Makefile:2: The continuation backslash should be in column 73, not 81.",
 		"NOTE: Makefile:3: The continuation backslash should be in column 73, not 81.")
 	vt.Autofixes(
 		"AUTOFIX: Makefile:2: Replacing \"\\t\" with \" \".",
 		"AUTOFIX: Makefile:3: Replacing \" \\t\\t\" with \"\\t\".")
 	vt.Fixed(
 		"CONFIGURE_ARGS+=        \\",
 		"        .....................................................................79 \\",
 		"        ............................................................70  \\",
 		"        ........................................................66")
 	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) {
 	vt := NewVaralignTester(s, c)
 	vt.Input(
 		"MESSAGE_SUBST+=\t\tRUBY_DISTNAME=${RUBY_DISTNAME}",
 		"PLIST_SUBST+=\t\tRUBY_SHLIBVER=${RUBY_SHLIBVER:Q} \\",
 		"\t\t\tRUBY_SHLIBMAJOR=${RUBY_SHLIBMAJOR:Q} \\",
 		"\t\t\tRUBY_NOSHLIBMAJOR=${RUBY_NOSHLIBMAJOR} \\",
 		"\t\t\tRUBY_NAME=${RUBY_NAME:Q}")
 	vt.Internals(
 		"15 24",
 		"13 24 57",

 @@ -907,27 +907,26 @@ func (s *Suite) Test_VartypeCheck_GccReq
+}
 func (s *Suite) Test_VartypeCheck_Homepage(c *check.C) {
 	t := s.Init(c)
 	vt := NewVartypeCheckTester(t, BtHomepage)
 	vt.Varname("HOMEPAGE")
 	vt.Values(
 		"http://www.pkgsrc.org/",
 		"https://www.pkgsrc.org/",
 		"${MASTER_SITES}")
 	vt.Output(
 		"WARN: filename.mk:1: HOMEPAGE should migrate from http to https.",
 		"WARN: filename.mk:3: HOMEPAGE should not be defined in terms of MASTER_SITEs.")
 	// For more tests, see HomepageChecker.
+}
 func (s *Suite) Test_VartypeCheck_IdentifierDirect(c *check.C) {
 	t := s.Init(c)
 	vt := NewVartypeCheckTester(t, BtIdentifierDirect)
 	vt.Varname("PKGBASE")
 	vt.Values(
 		"${OTHER_VAR}",
 		"identifiers cannot contain spaces",

 @@ -75,26 +75,42 @@ func (s *Suite) Test_newVersion(c *check
 		&version{[]int{1, 0, 0, -3, 3}, 0})
 	c.Check(newVersion("1_0alpha3"), check.DeepEquals,
 		&version{[]int{1, 0, 0, -3, 3}, 0})
 	c.Check(newVersion("2.5beta"), check.DeepEquals,
 		&version{[]int{2, 0, 5, -2}, 0})
 	c.Check(newVersion("20151110"), check.DeepEquals,
 		&version{[]int{20151110}, 0})
 	c.Check(newVersion("0"), check.DeepEquals,
 		&version{[]int{0}, 0})
 	c.Check(newVersion("nb1"), check.DeepEquals,
 		&version{nil, 1})
 	c.Check(newVersion("1.0.1a"), check.DeepEquals,
 		&version{[]int{1, 0, 0, 0, 1, 1}, 0})
 	c.Check(newVersion("1.1.1dnb2"), check.DeepEquals,
 		&version{[]int{1, 0, 1, 0, 1, 4}, 2})
 	c.Check(newVersion("1.0.1z"), check.DeepEquals,
 		&version{[]int{1, 0, 0, 0, 1, 26}, 0})
 	c.Check(newVersion("0pre20160620"), check.DeepEquals,
 		&version{[]int{0, -1, 20160620}, 0})
 	c.Check(newVersion("3.5.DEV1710"), check.DeepEquals,
 		&version{[]int{3, 0, 5, 0, 4, 5, 22, 1710}, 0})
 	// In the following edge case, the "nb" and "beta" overlap.
 	// All the digits after the "nb" (which in this case are none at all)
 	// end up in the nb version part, and parsing continues with the next
 	// letter.
 	//
 	// Luckily this will not happen in practice since most version numbers
 	// are completely numeric, and those that aren't might have suffixes
 	// like "alpha", "beta", "public beta", "GA" (general availability),
 	// "final", "snapshot". The word "nonbeta" is purely hypothetical, and
 	// I didn't find any other word that would contain "nbeta". Commit
 	// hashes are also safe since their hex encoding cannot contain "n".
 	c.Check(newVersion("1.0nonbeta"), check.DeepEquals,
 		&version{[]int{1, 0, 0, 14, 15, 5, 20, 1}, 0})
+}
 func (s *Suite) Test__qa(c *check.C) {
 	ck := intqa.NewQAChecker(c.Errorf)
 	ck.Configure("*", "*", "*", -intqa.EMissingTest)
 	ck.Check()
+}

	@@ -134,26 +134,29 @@ func (line *Line) RelLocation(other Loca		@@ -134,26 +134,29 @@ func (line *Line) RelLocation(other Loca
134	func (line *Line) Rel(other CurrPath) RelPath {	134	func (line *Line) Rel(other CurrPath) RelPath {
135	return G.Pkgsrc.Relpath(line.Filename.DirNoClean(), other)	135	return G.Pkgsrc.Relpath(line.Filename.DirNoClean(), other)
136	}	136	}
137		137
138	func (line *Line) IsMultiline() bool {	138	func (line *Line) IsMultiline() bool {
139	return line.firstLine > 0 && line.firstLine != line.lastLine	139	return line.firstLine > 0 && line.firstLine != line.lastLine
140	}	140	}
141		141
142	func (line *Line) IsCvsID(prefixRe regex.Pattern) (found bool, expanded bool) {	142	func (line *Line) IsCvsID(prefixRe regex.Pattern) (found bool, expanded bool) {
143	m, exp := match1(line.Text, `^`+prefixRe+`\$`+`NetBSD(:[^\$]+)?\$$`)	143	m, exp := match1(line.Text, `^`+prefixRe+`\$`+`NetBSD(:[^\$]+)?\$$`)
144	return m, exp != ""	144	return m, exp != ""
145	}	145	}
146		146
		147	// FIXME: By definition there cannot be fatal diagnostics.
		148	// Having these was a misconception from the beginning,
		149	// and they must be re-classified as fatal technical errors.
147	func (line *Line) Fatalf(format string, args ...interface{}) {	150	func (line *Line) Fatalf(format string, args ...interface{}) {
148	if trace.Tracing {	151	if trace.Tracing {
149	trace.Stepf("Fatalf: %q, %v", format, args)	152	trace.Stepf("Fatalf: %q, %v", format, args)
150	}	153	}
151	G.Logger.Diag(line, Fatal, format, args...)	154	G.Logger.Diag(line, Fatal, format, args...)
152	}	155	}
153		156
154	func (line *Line) Errorf(format string, args ...interface{}) {	157	func (line *Line) Errorf(format string, args ...interface{}) {
155	G.Logger.Diag(line, Error, format, args...)	158	G.Logger.Diag(line, Error, format, args...)
156	}	159	}
157		160
158	func (line *Line) Warnf(format string, args ...interface{}) {	161	func (line *Line) Warnf(format string, args ...interface{}) {
159	G.Logger.Diag(line, Warn, format, args...)	162	G.Logger.Diag(line, Warn, format, args...)