pkgtools/pkglint: update to 19.3.6

Changes since 19.3.5:

Improved indentation and alignment of multi-line variable assignments.

Improved indentation of multi-line shell commands.

Added warning for adding unquoted words to PKG_FAIL_REASON, which is a
list of messages, one per line.

Lines that start with tabs followed by a # are not shell commands, they
are comments. Bmake treats them in the same way.

Change the type of BROKEN to be a list of messages, instead of a single
message. This allows at least a bit of formatting in the error messages.

12 files changed, 295 insertions, 132 deletions

diff --git a/pkgtools/pkglint/Makefile b/pkgtools/pkglint/Makefile
index be6f812be13..e5e2ce8c49f 100644
--- a/pkgtools/pkglint/Makefile
+++ b/pkgtools/pkglint/Makefile
@@ -1,6 +1,6 @@
-# $NetBSD: Makefile,v 1.605 2019/11/02 16:37:48 rillig Exp $
+# $NetBSD: Makefile,v 1.606 2019/11/04 18:44:21 rillig Exp $
 
-PKGNAME=	pkglint-19.3.5
+PKGNAME=	pkglint-19.3.6
 CATEGORIES=	pkgtools
 DISTNAME=	tools
 MASTER_SITES=	${MASTER_SITE_GITHUB:=golang/}
diff --git a/pkgtools/pkglint/files/mklinechecker.go b/pkgtools/pkglint/files/mklinechecker.go
index bf86857e790..ef95ef53894 100644
--- a/pkgtools/pkglint/files/mklinechecker.go
+++ b/pkgtools/pkglint/files/mklinechecker.go
@@ -66,6 +66,9 @@ func (ck MkLineChecker) checkShellCommand() {
 
 	shellCommand := mkline.ShellCommand()
 	if G.Opts.WarnSpace && hasPrefix(mkline.Text, "\t\t") {
+		lexer := textproc.NewLexer(mkline.raw[0].textnl)
+		tabs := lexer.NextBytesFunc(func(b byte) bool { return b == '\t' })
+
 		fix := mkline.Autofix()
 		fix.Notef("Shell programs should be indented with a single tab.")
 		fix.Explain(
@@ -73,7 +76,12 @@ func (ck MkLineChecker) checkShellCommand() {
 			"Since every line of shell commands starts with a completely new shell environment,",
 			"there is no need to indent some of the commands,",
 			"or to use more horizontal space than necessary.")
-		fix.ReplaceRegex(`^\t\t+`, "\t", 1)
+
+		for i, raw := range mkline.Line.raw {
+			if hasPrefix(raw.textnl, tabs) {
+				fix.ReplaceAt(i, 0, tabs, "\t")
+			}
+		}
 		fix.Apply()
 	}
 
@@ -1422,6 +1430,13 @@ func (ck MkLineChecker) checkVartype(varname string, op MkOperator, value, comme
 
 	default:
 		words := mkline.ValueFields(value)
+		if len(words) > 1 && vartype.OnePerLine() {
+			mkline.Warnf("%s should only get one item per line.", varname)
+			mkline.Explain(
+				"Use the += operator to append each of the items.",
+				"",
+				"Or, enclose the words in quotes to group them.")
+		}
 		for _, word := range words {
 			ck.CheckVartypeBasic(varname, vartype.basicType, op, word, comment, vartype.Guessed())
 		}
diff --git a/pkgtools/pkglint/files/mklinechecker_test.go b/pkgtools/pkglint/files/mklinechecker_test.go
index cc61459ac06..d400f484a29 100644
--- a/pkgtools/pkglint/files/mklinechecker_test.go
+++ b/pkgtools/pkglint/files/mklinechecker_test.go
@@ -25,6 +25,38 @@ func (s *Suite) Test_MkLineChecker_checkEmptyContinuation(c *check.C) {
 		"WARN: ~/filename.mk:3: This line looks empty but continues the previous line.")
 }
 
+func (s *Suite) Test_MkLineChecker_checkShellCommand__indentation(c *check.C) {
+	t := s.Init(c)
+
+	t.SetUpCommandLine("-Wall", "--autofix")
+	mklines := t.SetUpFileMkLines("filename.mk",
+		MkCvsID,
+		"",
+		"do-install:",
+		"\t\techo 'unnecessarily indented'",
+		"\t\tfor var in 1 2 3; do \\",
+		"\t\t\techo \"$$var\"; \\",
+		"\t                echo \"spaces\"; \\",
+		"\t\tdone")
+
+	mklines.Check()
+
+	t.CheckOutputLines(
+		"AUTOFIX: ~/filename.mk:4: Replacing \"\\t\\t\" with \"\\t\".",
+		"AUTOFIX: ~/filename.mk:5: Replacing \"\\t\\t\" with \"\\t\".",
+		"AUTOFIX: ~/filename.mk:6: Replacing \"\\t\\t\" with \"\\t\".",
+		"AUTOFIX: ~/filename.mk:8: Replacing \"\\t\\t\" with \"\\t\".")
+	t.CheckFileLinesDetab("filename.mk",
+		MkCvsID,
+		"",
+		"do-install:",
+		"        echo 'unnecessarily indented'",
+		"        for var in 1 2 3; do \\",
+		"                echo \"$$var\"; \\",
+		"                        echo \"spaces\"; \\", // not changed
+		"        done")
+}
+
 func (s *Suite) Test_MkLineChecker_checkVarassignLeft(c *check.C) {
 	t := s.Init(c)
 
@@ -724,6 +756,23 @@ func (s *Suite) Test_MkLineChecker_checkVartype__no_tracing(c *check.C) {
 		"WARN: filename.mk:3: CUR_DIR is defined but not used.")
 }
 
+func (s *Suite) Test_MkLineChecker_checkVartype__one_per_line(c *check.C) {
+	t := s.Init(c)
+
+	t.SetUpVartypes()
+	mklines := t.NewMkLines("filename.mk",
+		MkCvsID,
+		"PKG_FAIL_REASON+=\tSeveral words are wrong.",
+		"PKG_FAIL_REASON+=\t\"Properly quoted\"",
+		"PKG_FAIL_REASON+=\t# none")
+	t.DisableTracing()
+
+	mklines.Check()
+
+	t.CheckOutputLines(
+		"WARN: filename.mk:2: PKG_FAIL_REASON should only get one item per line.")
+}
+
 // Pkglint once interpreted all lists as consisting of shell tokens,
 // splitting this URL at the ampersand.
 func (s *Suite) Test_MkLineChecker_checkVarassign__URL_with_shell_special_characters(c *check.C) {
@@ -2535,7 +2584,8 @@ func (s *Suite) Test_MkLineChecker_CheckVaruse__build_defs(c *check.C) {
 	mklines.Check()
 
 	t.CheckOutputLines(
-		"WARN: ~/options.mk:2: The user-defined variable VARBASE is used but not added to BUILD_DEFS.")
+		"WARN: ~/options.mk:2: The user-defined variable VARBASE is used but not added to BUILD_DEFS.",
+		"WARN: ~/options.mk:3: PKG_FAIL_REASON should only get one item per line.")
 }
 
 // The LOCALBASE variable may be defined and used in the infrastructure.
diff --git a/pkgtools/pkglint/files/mkshparser.go b/pkgtools/pkglint/files/mkshparser.go
index 301154e64c2..98547f5822a 100644
--- a/pkgtools/pkglint/files/mkshparser.go
+++ b/pkgtools/pkglint/files/mkshparser.go
@@ -80,6 +80,10 @@ func (lex *ShellLexer) Lex(lval *shyySymType) (ttype int) {
 
 	if trace.Tracing {
 		defer func() {
+			if ttype == 0 {
+				trace.Stepf("lex EOF because of a comment")
+				return
+			}
 			tname := shyyTokname(shyyTok2[ttype-shyyPrivate])
 			switch ttype {
 			case tkWORD, tkASSIGNMENT_WORD:
@@ -238,6 +242,10 @@ func (lex *ShellLexer) Lex(lval *shyySymType) (ttype int) {
 		ttype = tkASSIGNMENT_WORD
 		p := NewShTokenizer(dummyLine, token, false) // Just for converting the string to a ShToken
 		lval.Word = p.ShToken()
+	case hasPrefix(token, "#"):
+		// This doesn't work for multiline shell programs.
+		// Since pkglint only processes single lines, that's ok.
+		return 0
 	default:
 		ttype = tkWORD
 		p := NewShTokenizer(dummyLine, token, false) // Just for converting the string to a ShToken
diff --git a/pkgtools/pkglint/files/options_test.go b/pkgtools/pkglint/files/options_test.go
index 978888d1de8..662ee576802 100755
--- a/pkgtools/pkglint/files/options_test.go
+++ b/pkgtools/pkglint/files/options_test.go
@@ -103,6 +103,92 @@ func (s *Suite) Test_CheckLinesOptionsMk__prefs(c *check.C) {
 	t.CheckOutputEmpty()
 }
 
+func (s *Suite) Test_CheckLinesOptionsMk__variable_order(c *check.C) {
+	t := s.Init(c)
+
+	t.SetUpOption("option", "")
+	t.SetUpPackage("category/package",
+		".include \"options.mk\"")
+	t.CreateFileLines("mk/bsd.options.mk",
+		MkCvsID)
+	mklines := t.SetUpFileMkLines("category/package/options.mk",
+		MkCvsID,
+		"",
+		"PKG_SUPPORTED_OPTIONS=\toption",
+		"PKG_OPTIONS_VAR=\tPKG_OPTIONS.package",
+		"",
+		".include \"../../mk/bsd.options.mk\"",
+		"",
+		".if ${PKG_OPTIONS:Moption}",
+		".endif")
+	t.Chdir("category/package")
+	t.FinishSetUp()
+
+	CheckLinesOptionsMk(mklines)
+
+	t.CheckOutputLines(
+		"WARN: ~/category/package/options.mk:3: " +
+			"Expected definition of PKG_OPTIONS_VAR.")
+}
+
+func (s *Suite) Test_CheckLinesOptionsMk__empty(c *check.C) {
+	t := s.Init(c)
+
+	t.SetUpPackage("category/package",
+		".include \"options.mk\"")
+	mklines := t.SetUpFileMkLines("category/package/options.mk",
+		MkCvsID)
+	t.Chdir("category/package")
+	t.FinishSetUp()
+
+	CheckLinesOptionsMk(mklines)
+
+	t.CheckOutputLines(
+		"ERROR: ~/category/package/options.mk: "+
+			"Each options.mk file must define PKG_OPTIONS_VAR.",
+		"ERROR: ~/category/package/options.mk: "+
+			"Each options.mk file must .include \"../../mk/bsd.options.mk\".")
+}
+
+func (s *Suite) Test_CheckLinesOptionsMk__conditionals(c *check.C) {
+	t := s.Init(c)
+
+	t.SetUpOption("option", "")
+	t.SetUpPackage("category/package",
+		".include \"../../mk/bsd.options.mk\"")
+	t.CreateFileLines("mk/bsd.options.mk",
+		MkCvsID)
+	t.Chdir("category/package")
+	mklines := t.SetUpFileMkLines("options.mk",
+		MkCvsID,
+		"",
+		"PKG_OPTIONS_VAR=\tPKG_OPTIONS.package",
+		"PKG_SUPPORTED_OPTIONS=\toption",
+		"",
+		".include \"../../mk/bsd.options.mk\"",
+		"",
+		".if ${PKG_OPTIONS}", // typo: should be ${PKG_OPTIONS:Moption}
+		".endif",
+		"",
+		".if ${PKG_OPTIONS:Nnegative}", // :N instead of :M, is ignored
+		".endif",
+		"",
+		".if ${PKG_OPTIONS:Ncodec-*}",
+		".endif",
+		"",
+		".if ${PKG_OPTIONS:tl}", // doesn't make sense, just for branch coverage
+		".endif")
+	t.FinishSetUp()
+
+	CheckLinesOptionsMk(mklines)
+
+	t.CheckOutputLines(
+		// This warning comes from VarTypeCheck.PkgOption
+		"WARN: options.mk:11: Unknown option \"negative\".",
+		"WARN: options.mk:4: "+
+			"Option \"option\" should be handled below in an .if block.")
+}
+
 func (s *Suite) Test_CheckLinesOptionsMk(c *check.C) {
 	t := s.Init(c)
 
@@ -170,79 +256,6 @@ func (s *Suite) Test_CheckLinesOptionsMk(c *check.C) {
 	//  disables possible wrong warnings, but a few too many.
 }
 
-// This test is provided for code coverage. Similarities to existing files are purely coincidental.
-func (s *Suite) Test_CheckLinesOptionsMk__edge_cases(c *check.C) {
-	t := s.Init(c)
-
-	t.SetUpCommandLine("-Wall,no-space")
-	t.SetUpVartypes()
-	t.SetUpOption("option1", "Description for option1")
-	t.CreateFileLines("mk/compiler.mk",
-		MkCvsID)
-	t.CreateFileLines("mk/bsd.options.mk",
-		MkCvsID)
-	t.DisableTracing()
-
-	mklines := t.SetUpFileMkLines("category/package/options.mk",
-		MkCvsID)
-
-	CheckLinesOptionsMk(mklines)
-
-	t.CheckOutputLines(
-		"ERROR: ~/category/package/options.mk: Each options.mk file must define PKG_OPTIONS_VAR.",
-		"ERROR: ~/category/package/options.mk: Each options.mk file must .include \"../../mk/bsd.options.mk\".")
-
-	mklines = t.SetUpFileMkLines("category/package/options.mk",
-		MkCvsID,
-		"PKG_SUPPORTED_OPTIONS=\toption1")
-
-	CheckLinesOptionsMk(mklines)
-
-	t.CheckOutputLines(
-		"WARN: ~/category/package/options.mk:2: "+
-			"Expected definition of PKG_OPTIONS_VAR.",
-		"ERROR: ~/category/package/options.mk: "+
-			"Each options.mk file must define PKG_OPTIONS_VAR.",
-		"ERROR: ~/category/package/options.mk: "+
-			"Each options.mk file must .include \"../../mk/bsd.options.mk\".",
-		"WARN: ~/category/package/options.mk:2: "+
-			"Option \"option1\" should be handled below in an .if block.")
-
-	mklines = t.SetUpFileMkLines("category/package/options.mk",
-		MkCvsID,
-		"PKG_OPTIONS_VAR=\tPKG_OPTIONS.pkgbase",
-		"PKG_SUPPORTED_OPTIONS=\toption1",
-		".include \"../../mk/compiler.mk\"")
-
-	CheckLinesOptionsMk(mklines)
-
-	t.CheckOutputLines(
-		"ERROR: ~/category/package/options.mk: "+
-			"Each options.mk file must .include \"../../mk/bsd.options.mk\".",
-		"WARN: ~/category/package/options.mk:3: "+
-			"Option \"option1\" should be handled below in an .if block.")
-
-	mklines = t.SetUpFileMkLines("category/package/options.mk",
-		MkCvsID,
-		"PKG_OPTIONS_VAR=\tPKG_OPTIONS.pkgbase",
-		"PKG_SUPPORTED_OPTIONS=\toption1",
-		".include \"../../mk/bsd.options.mk\"",
-		"",
-		".if !empty(PKG_OPTIONS:O:u:Moption1) "+
-			"|| !empty(PKG_OPTIONS:Noption1) "+
-			"|| !empty(PKG_OPTIONS:O) "+
-			"|| !empty(X11_TYPE) "+
-			"|| !empty(PKG_OPTIONS:M${X11_TYPE})",
-		".endif")
-
-	CheckLinesOptionsMk(mklines)
-
-	// Although technically this option is handled by the :Noption1 modifier,
-	// this is so unusual that the warning is justified.
-	t.CheckOutputLines(
-		"WARN: ~/category/package/options.mk:3: Option \"option1\" should be handled below in an .if block.")
-}
-
 // If there is no .include line after the declaration of the package-settable
 // variables, the whole analysis stops.
 //
diff --git a/pkgtools/pkglint/files/shell.go b/pkgtools/pkglint/files/shell.go
index 1b707118e0f..97407de0749 100644
--- a/pkgtools/pkglint/files/shell.go
+++ b/pkgtools/pkglint/files/shell.go
@@ -591,9 +591,6 @@ func (scc *SimpleCommandChecker) handleComment() bool {
 		defer trace.Call0()()
 	}
 
-	// FIXME: Research and explain how pkglint can ever interpret
-	//  a shell comment as a simple command. That just doesn't fit.
-
 	shellword := scc.strcmd.Name
 	if trace.Tracing {
 		defer trace.Call1(shellword)()
@@ -603,36 +600,26 @@ func (scc *SimpleCommandChecker) handleComment() bool {
 		return false
 	}
 
-	semicolon := contains(shellword, ";")
-	multiline := scc.mkline.IsMultiline()
-
-	if semicolon {
-		scc.mkline.Warnf("A shell comment should not contain semicolons.")
-		// TODO: Explain.
-		// TODO: Check whether the existing warnings are useful.
-	}
-
-	if multiline {
-		scc.mkline.Warnf("A shell comment does not stop at the end of line.")
+	if !scc.mkline.IsMultiline() {
+		return false
 	}
 
-	if semicolon || multiline {
-		scc.Explain(
-			"When a shell command is split into multiple lines that are",
-			"continued with a backslash, they will nevertheless be converted to",
-			"a single line before the shell sees them.",
-			"",
-			"This means that even if it looks as if the comment only spanned",
-			"one line in the Makefile, in fact it spans until the end of the whole",
-			"shell command.",
-			"",
-			"To insert a comment into shell code, you can write it like this:",
-			"",
-			"\t"+"${SHCOMMENT} \"The following command might fail; this is ok.\"",
-			"",
-			"Note that any special characters in the comment are still",
-			"interpreted by the shell.")
-	}
+	scc.mkline.Warnf("A shell comment does not stop at the end of line.")
+	scc.Explain(
+		"When a shell command is split into multiple lines that are",
+		"continued with a backslash, they will nevertheless be converted to",
+		"a single line before the shell sees them.",
+		"",
+		"This means that even if it looks as if the comment only spanned",
+		"one line in the Makefile, in fact it spans until the end of the whole",
+		"shell command.",
+		"",
+		"To insert a comment into shell code, you can write it like this:",
+		"",
+		"\t"+"${SHCOMMENT} \"The following command might fail; this is ok.\"",
+		"",
+		"Note that any special characters in the comment are still",
+		"interpreted by the shell.")
 	return true
 }
 
diff --git a/pkgtools/pkglint/files/shell_test.go b/pkgtools/pkglint/files/shell_test.go
index 8a0e0f73f45..7f6f93079bc 100644
--- a/pkgtools/pkglint/files/shell_test.go
+++ b/pkgtools/pkglint/files/shell_test.go
@@ -833,8 +833,8 @@ func (s *Suite) Test_ShellLineChecker__shell_comment_with_line_continuation(c *c
 
 	mklines.Check()
 
-	t.CheckOutputLines(
-		"WARN: ~/Makefile:3--4: A shell comment does not stop at the end of line.")
+	// TODO: "WARN: ~/Makefile:3--4: A shell comment does not stop at the end of line."
+	t.CheckOutputEmpty()
 }
 
 func (s *Suite) Test_ShellLineChecker_checkWordQuoting(c *check.C) {
@@ -1264,19 +1264,6 @@ func (s *Suite) Test_SimpleCommandChecker_handleCommandVariable__from_package(c
 	t.CheckOutputEmpty()
 }
 
-func (s *Suite) Test_SimpleCommandChecker_handleComment(c *check.C) {
-	t := s.Init(c)
-
-	mklines := t.NewMkLines("file.mk",
-		MkCvsID,
-		"\t# comment; continuation")
-
-	mklines.Check()
-
-	t.CheckOutputLines(
-		"WARN: file.mk:2: A shell comment should not contain semicolons.")
-}
-
 // This test ensures that the command line options to INSTALL_*_DIR are properly
 // parsed and do not lead to "can only handle one directory at a time" warnings.
 func (s *Suite) Test_SimpleCommandChecker_checkInstallMulti(c *check.C) {
diff --git a/pkgtools/pkglint/files/varalignblock.go b/pkgtools/pkglint/files/varalignblock.go
index 78c5c2a4c0e..6eb502965fa 100644
--- a/pkgtools/pkglint/files/varalignblock.go
+++ b/pkgtools/pkglint/files/varalignblock.go
@@ -87,6 +87,9 @@ type varalignLine struct {
 	// of the first value found.
 	multiEmpty bool
 
+	// Whether the line is so long that it may use a single tab as indentation.
+	long bool
+
 	varalignParts
 }
 
@@ -133,7 +136,7 @@ func (va *VaralignBlock) processVarassign(mkline *MkLine) {
 	follow := false
 	for i, raw := range mkline.raw {
 		parts := NewVaralignSplitter().split(strings.TrimSuffix(raw.textnl, "\n"), i == 0)
-		info := varalignLine{mkline, i, follow, parts}
+		info := varalignLine{mkline, i, follow, false, parts}
 
 		if i == 0 && info.isEmptyContinuation() {
 			follow = true
@@ -158,6 +161,7 @@ func (va *VaralignBlock) Finish() {
 	}
 
 	newWidth := va.optimalWidth(infos)
+	va.adjustLong(newWidth, infos)
 	rightMargin := 0
 
 	// When the indentation of the initial line of a multiline is
@@ -179,7 +183,7 @@ func (va *VaralignBlock) Finish() {
 			rightMargin = va.rightMargin(infos[restIndex:])
 		}
 
-		va.checkContinuationIndentation(info, newWidth, rightMargin)
+		va.checkRightMargin(info, newWidth, rightMargin)
 
 		if newWidth > 0 || info.rawIndex > 0 {
 			va.realign(info, newWidth, &indentDiffSet, &indentDiff)
@@ -287,7 +291,20 @@ func (*VaralignBlock) optimalWidth(infos []*varalignLine) int {
 	return (minVarnameOpWidth & -8) + 8
 }
 
-func (va *VaralignBlock) checkContinuationIndentation(info *varalignLine, newWidth int, rightMargin int) {
+func (va *VaralignBlock) adjustLong(newWidth int, infos []*varalignLine) {
+	long := false
+	for _, info := range infos {
+		if info.rawIndex == 0 {
+			long = false
+		}
+		if !info.multiEmpty && info.spaceBeforeValue == "\t" && info.varnameOpSpaceWidth() != newWidth && info.widthAlignedAt(newWidth) > 72 {
+			long = true
+		}
+		info.long = long
+	}
+}
+
+func (va *VaralignBlock) checkRightMargin(info *varalignLine, newWidth int, rightMargin int) {
 	if !info.isContinuation() {
 		return
 	}
@@ -352,7 +369,7 @@ func (*VaralignBlock) realignMultiEmptyInitial(info *varalignLine, newWidth int)
 	}
 
 	if newSpace == " " {
-		return // This case is handled by checkContinuationIndentation.
+		return // This case is handled by checkRightMargin.
 	}
 
 	hasSpace := strings.IndexByte(oldSpace, ' ') != -1
@@ -435,7 +452,7 @@ func (va *VaralignBlock) realignMultiFollow(info *varalignLine, newWidth int, in
 	if tabWidth(newSpace) < newWidth {
 		newSpace = indent(newWidth)
 	}
-	if newSpace == oldSpace || (oldSpace == "\t" && info.widthAlignedAt(newWidth) > 72) {
+	if newSpace == oldSpace || info.long {
 		return
 	}
 
diff --git a/pkgtools/pkglint/files/varalignblock_test.go b/pkgtools/pkglint/files/varalignblock_test.go
index b0bbf0d0fb4..43f25a1a4de 100644
--- a/pkgtools/pkglint/files/varalignblock_test.go
+++ b/pkgtools/pkglint/files/varalignblock_test.go
@@ -118,6 +118,13 @@ func (vt *VaralignTester) checkTestName() {
 		name  string
 		width int
 	}
+	descriptorsString := func(ds []descriptor) string {
+		var strs []string
+		for _, d := range ds {
+			strs = append(strs, sprintf("%s%d", d.name, d.width))
+		}
+		return strings.Join(strs, "_")
+	}
 
 	var actual []descriptor
 	width := 0
@@ -195,6 +202,7 @@ func (vt *VaralignTester) checkTestName() {
 		expected = append(expected, descriptor{name, width})
 	}
 
+	vt.tester.CheckDeepEquals(descriptorsString(actual), descriptorsString(expected))
 	vt.tester.CheckDeepEquals(actual, expected)
 }
 
@@ -2002,6 +2010,38 @@ func (s *Suite) Test_VaralignBlock__lead_var_tab8_value_lead_var_tab16_value(c *
 	vt.Run()
 }
 
+// Before 19.3.6, pkglint would indent the last line in column 16.
+//
+// The value in the first line starts in column 16, which means that all
+// follow-up lines should also start in column 16 or further to the right.
+// Line 2 though is already quite long, and since its right margin is in
+// column 72, it may keep its lower-than-usual indentation of 8.
+// Line 3 is not that long, therefore the rule from line 2 doesn't apply
+// here, and it needs to be indented to column 16.
+//
+// Since the above result would look inconsistent, all follow-up lines
+// after a long line may be indented in column 8 as well.
+func (s *Suite) Test_VaralignBlock__var_tab_value63_space_cont_tab8_value71_space_cont_tab8_value(c *check.C) {
+	vt := NewVaralignTester(s, c)
+	vt.Input(
+		"PROGFILES=\t67890 234567890 234567890 234567890 234567890 2 \\",
+		"\t890 234567890 234567890 234567890 234567890 234567890 234567890 \\",
+		"\tvalue")
+	vt.Internals(
+		"10 16 64",
+		"   08 72",
+		"   08")
+	vt.Diagnostics(
+		nil...)
+	vt.Autofixes(
+		nil...)
+	vt.Fixed(
+		"PROGFILES=      67890 234567890 234567890 234567890 234567890 2 \\",
+		"        890 234567890 234567890 234567890 234567890 234567890 234567890 \\",
+		"        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) {
@@ -2215,6 +2255,24 @@ func (s *Suite) Test_VaralignBlock__mixed_indentation(c *check.C) {
 	vt.Run()
 }
 
+func (s *Suite) Test_VaralignBlock__long_line_followed_by_short_line_with_small_indentation(c *check.C) {
+	vt := NewVaralignTester(s, c)
+	vt.Input(
+		"VAR.567890123456+=\t----30 -------40 -------50 -------60 -------70 234567 \\",
+		"\t\t--20 -------30")
+	vt.Internals(
+		"18 24 78",
+		"   16")
+	vt.Diagnostics(
+		"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(
+		"VAR.567890123456+=      ----30 -------40 -------50 -------60 -------70 234567 \\",
+		"                        --20 -------30")
+	vt.Run()
+}
+
 // Ensure that the end-of-line comment is properly aligned
 // to the variable values.
 //
diff --git a/pkgtools/pkglint/files/vardefs.go b/pkgtools/pkglint/files/vardefs.go
index 69858caad6b..42ded03a3a0 100644
--- a/pkgtools/pkglint/files/vardefs.go
+++ b/pkgtools/pkglint/files/vardefs.go
@@ -156,6 +156,15 @@ func (reg *VarTypeRegistry) pkglistrat(varname string, basicType *BasicType) {
 		"Makefile, Makefile.*, *.mk: default, set, append, use")
 }
 
+// Like pkglist, but only one value per line should be given.
+// Typical example: PKG_FAIL_REASON.
+func (reg *VarTypeRegistry) pkglistone(varname string, basicType *BasicType) {
+	reg.acllist(varname, basicType,
+		List|PackageSettable|OnePerLine,
+		"buildlink3.mk, builtin.mk: none",
+		"Makefile, Makefile.*, *.mk: default, set, append, use")
+}
+
 // A package-defined load-time list may be used or defined or appended to in
 // all Makefiles except buildlink3.mk and builtin.mk. Simple assignment
 // (instead of appending) is also allowed. If this leads to an unconditional
@@ -816,8 +825,7 @@ func (reg *VarTypeRegistry) Init(src *Pkgsrc) {
 	reg.sys("BINOWN", BtUserGroupName)
 	reg.pkglist("BOOTSTRAP_DEPENDS", BtDependencyWithPath)
 	reg.pkg("BOOTSTRAP_PKG", BtYesNo)
-	// BROKEN should better be a list of messages instead of a simple string.
-	reg.pkgappend("BROKEN", BtMessage)
+	reg.pkglistone("BROKEN", BtShellWord)
 	reg.pkg("BROKEN_GETTEXT_DETECTION", BtYesNo)
 	reg.pkglistrat("BROKEN_EXCEPT_ON_PLATFORM", BtMachinePlatformPattern)
 	reg.pkglistrat("BROKEN_ON_PLATFORM", BtMachinePlatformPattern)
@@ -1391,7 +1399,7 @@ func (reg *VarTypeRegistry) Init(src *Pkgsrc) {
 	reg.usrlist("PKG_DEFAULT_OPTIONS", BtOption)
 	reg.sys("PKG_DELETE", BtShellCommand)
 	reg.pkglist("PKG_DESTDIR_SUPPORT", enum("destdir user-destdir"))
-	reg.pkglist("PKG_FAIL_REASON", BtShellWord)
+	reg.pkglistone("PKG_FAIL_REASON", BtShellWord)
 	reg.sysload("PKG_FORMAT", BtIdentifier)
 	reg.pkg("PKG_GECOS.*", BtMessage)
 	reg.pkg("PKG_GID.*", BtInteger)
@@ -1440,7 +1448,7 @@ func (reg *VarTypeRegistry) Init(src *Pkgsrc) {
 	reg.pkg("PKG_SHELL", BtPathname)
 	reg.pkg("PKG_SHELL.*", BtPathname)
 	reg.sys("PKG_SHLIBTOOL", BtPathname)
-	reg.pkglist("PKG_SKIP_REASON", BtShellWord)
+	reg.pkglistone("PKG_SKIP_REASON", BtShellWord)
 	// The special exception for buildlink3.mk is only here because
 	// of textproc/xmlcatmgr.
 	reg.acl("PKG_SYSCONFDIR*", BtPathname,
diff --git a/pkgtools/pkglint/files/vartype.go b/pkgtools/pkglint/files/vartype.go
index bce33dbcd9c..563cab95134 100644
--- a/pkgtools/pkglint/files/vartype.go
+++ b/pkgtools/pkglint/files/vartype.go
@@ -38,6 +38,8 @@ const (
 	// describing why they are set. Typical examples are NOT_FOR_* variables.
 	NeedsRationale
 
+	OnePerLine
+
 	NoVartypeOptions = 0
 )
 
@@ -101,6 +103,7 @@ func (vt *Vartype) UserSettable() bool        { return vt.options&UserSettable !
 func (vt *Vartype) SystemProvided() bool      { return vt.options&SystemProvided != 0 }
 func (vt *Vartype) CommandLineProvided() bool { return vt.options&CommandLineProvided != 0 }
 func (vt *Vartype) NeedsRationale() bool      { return vt.options&NeedsRationale != 0 }
+func (vt *Vartype) OnePerLine() bool          { return vt.options&OnePerLine != 0 }
 
 func (vt *Vartype) EffectivePermissions(basename string) ACLPermissions {
 	for _, aclEntry := range vt.aclEntries {
diff --git a/pkgtools/pkglint/files/vartypecheck_test.go b/pkgtools/pkglint/files/vartypecheck_test.go
index fed4d208ae8..bf36d9d212c 100644
--- a/pkgtools/pkglint/files/vartypecheck_test.go
+++ b/pkgtools/pkglint/files/vartypecheck_test.go
@@ -1457,6 +1457,23 @@ func (s *Suite) Test_VartypeCheck_ShellCommands(c *check.C) {
 		"WARN: filename.mk:1: This shell command list should end with a semicolon.")
 }
 
+func (s *Suite) Test_VartypeCheck_ShellWord(c *check.C) {
+	t := s.Init(c)
+	t.SetUpVartypes()
+	vt := NewVartypeCheckTester(t, BtShellWord)
+
+	vt.Varname("PKG_FAIL_REASON")
+	vt.Values(
+		"The package does not work here.",
+		"\"Properly quoted reason.\"")
+
+	// At this level, there can be no warning for line 1 since each word
+	// is analyzed on its own.
+	//
+	// See Test_MkLineChecker_checkVartype__one_per_line.
+	vt.OutputEmpty()
+}
+
 func (s *Suite) Test_VartypeCheck_Stage(c *check.C) {
 	vt := NewVartypeCheckTester(s.Init(c), BtStage)