 @@ -1,183 +1,416 @@
 package pkglint
 import "netbsd.org/pkglint/textproc"
 // SubstContext records the state of a block of variable assignments
-// that make up a SUBST class (see `mk/subst.mk`).
+// that make up a SUBST class.
 //
 // See mk/subst.mk.
 type SubstContext struct {
-	queuedIds []string
+	active *substBlock
 	id        string
 	doneIds   map[string]bool
-	foreignAllowed map[string]struct{}
+	scopes []*substScope
 	foreign        []*MkLine
 	conds []*substCond
 	once Once
+}
 func NewSubstContext() *SubstContext {
-	ctx := SubstContext{}
+	return &SubstContext{scopes: []*substScope{newSubstScope()}}
 	ctx.reset()
 	return &ctx
+}
 func (ctx *SubstContext) Process(mkline *MkLine) {
 	switch {
 	case mkline.IsEmpty():
-		ctx.finishClass(mkline)
+		ctx.emptyLine()
 	case mkline.IsVarassign():
 		ctx.varassign(mkline)
 	case mkline.IsDirective():
 		ctx.directive(mkline)
+	}
+}
 func (ctx *SubstContext) Finish(diag Diagnoser) {
-	ctx.finishClass(diag)
+	// Prevent panics on unbalanced conditionals.
-	ctx.finishFile(diag)
+	for len(ctx.scopes) > 1 {
 		ctx.leave(diag)
+	}
 	for _, scope := range ctx.scopes {
 		scope.finish(diag)
+	}
+}
 func (ctx *SubstContext) emptyLine() {
 	for _, scope := range ctx.scopes {
 		scope.emptyLine()
+	}
+}
 func (ctx *SubstContext) varassign(mkline *MkLine) {
 	varcanon := mkline.Varcanon()
 	if varcanon == "SUBST_CLASSES" || varcanon == "SUBST_CLASSES.*" {
 		ctx.varassignClasses(mkline)
 		return
+	}
 	if ctx.isForeign(mkline.Varcanon()) {
-		if ctx.isActive() {
+		if ctx.isActive() && !ctx.block().seenEmpty {
-			ctx.rememberForeign(mkline)
+			ctx.block().rememberForeign(mkline)
+		}
 		return
+	}
 	if !ctx.isActive() {
 		if !ctx.varassignOutsideBlock(mkline) {
 			return
+		}
+	}
-	if hasPrefix(mkline.Varname(), "SUBST_") && !ctx.isActiveId(mkline.Varparam()) {
+	if hasPrefix(mkline.Varname(), "SUBST_") && mkline.Varparam() != ctx.activeId() {
 		if !ctx.varassignDifferentClass(mkline) {
 			return
+		}
+	}
 	block := ctx.block()
 	switch varcanon {
 	case "SUBST_STAGE.*":
-		ctx.varassignStage(mkline)
+		block.varassignStage(mkline)
 	case "SUBST_MESSAGE.*":
-		ctx.varassignMessages(mkline)
+		block.varassignMessages(mkline)
 	case "SUBST_FILES.*":
-		ctx.varassignFiles(mkline)
+		block.varassignFiles(mkline)
 	case "SUBST_SED.*":
-		ctx.varassignSed(mkline)
+		block.varassignSed(mkline)
 	case "SUBST_VARS.*":
-		ctx.varassignVars(mkline)
+		block.varassignVars(mkline)
 	case "SUBST_FILTER_CMD.*":
-		ctx.varassignFilterCmd(mkline)
+		block.varassignFilterCmd(mkline)
+	}
+}
 func (ctx *SubstContext) varassignClasses(mkline *MkLine) {
-	classes := mkline.ValueFields(mkline.WithoutMakeVariables(mkline.Value()))
+	ids := mkline.ValueFields(mkline.WithoutMakeVariables(mkline.Value()))
-	if len(classes) == 0 {
+	if len(ids) == 0 {
 		return
+	}
-	if len(classes) > 1 {
+	if len(ids) > 1 {
 		mkline.Notef("Please add only one class at a time to SUBST_CLASSES.")
 		mkline.Explain(
 			"This way, each substitution class forms a block in the package Makefile,",
 			"and to delete this block, it is not necessary to look anywhere else.")
+	}
 	for _, class := range classes {
 		ctx.queue(class)
-	id := classes[0]
+	ctx.prepareSubstClasses(mkline)
-	if ctx.isActive() && !ctx.isActiveId(id) {
+	ctx.deactivate(mkline)
 		id := ctx.activeId() // since ctx.condEndif may reset it
-		for ctx.isConditional() {
+	for _, id := range ids {
-			// This will be confusing for the outer SUBST block,
+		if ctx.lookup(id) == nil {
-			// but since that block is assumed to be finished,
+			ctx.scopes[len(ctx.scopes)-1].define(id)
-			// this doesn't matter.
+		} else if mkline.Varparam() == "" {
-			ctx.condEndif(mkline)
+			mkline.Errorf("Duplicate SUBST class %q.", id)
 		complete := ctx.isComplete() // since ctx.finishClass will reset it
 		ctx.finishClass(mkline)
 		if !complete {
 			mkline.Warnf("Subst block %q should be finished before adding the next class to SUBST_CLASSES.", id)
+		}
+	}
+}
-	ctx.setActiveId(id)
+func (ctx *SubstContext) prepareSubstClasses(mkline *MkLine) {
 	for _, scope := range ctx.scopes {
-	return
+		scope.prepareSubstClasses(mkline)
+	}
+}
 // varassignOutsideBlock handles variable assignments of SUBST variables that
 // appear without a directly corresponding SUBST block.
 func (ctx *SubstContext) varassignOutsideBlock(mkline *MkLine) (continueWithNewId bool) {
-	varparam := mkline.Varparam()
+	id := mkline.Varparam()
-	if ctx.isListCanon(mkline.Varcanon()) && ctx.isDone(varparam) {
+	if id != "" && ctx.isListCanon(mkline.Varcanon()) && ctx.isDone(id) {
 		if mkline.Op() != opAssignAppend {
 			mkline.Warnf("Late additions to a SUBST variable should use the += operator.")
+		}
 		return
+	}
 	if containsWord(mkline.Rationale(), varparam) {
-		return
+	return ctx.activate(mkline, ctx.lookup(id) == nil)
+}
 func (ctx *SubstContext) varassignDifferentClass(mkline *MkLine) (ok bool) {
 	varname := mkline.Varname()
 	unknown := ctx.lookup(mkline.Varparam()) == nil
 	if unknown && ctx.isActive() && !ctx.block().isComplete() {
 		mkline.Warnf("Variable %q does not match SUBST class %q.",
 			varname, ctx.activeId())
 		if !ctx.block().seenEmpty {
 			return false
+		}
+	}
 	return ctx.activate(mkline, unknown)
+}
 func (ctx *SubstContext) directive(mkline *MkLine) {
 	dir := mkline.Directive()
 	switch dir {
 	case "if":
 		ctx.enter()
 	case "elif":
 		ctx.nextBranch(mkline, false)
 	case "else":
 		ctx.nextBranch(mkline, true)
 	case "endif":
 		ctx.leave(mkline)
+	}
+}
 func (ctx *SubstContext) enter() {
 	for _, scope := range ctx.scopes {
 		scope.enter()
+	}
 	ctx.scopes = append(ctx.scopes, newSubstScope())
+}
 func (ctx *SubstContext) nextBranch(diag Diagnoser, isElse bool) {
 	if ctx.isActive() && !ctx.block().isConditional() {
 		ctx.deactivate(diag)
+	}
 	for _, scope := range ctx.scopes {
 		scope.nextBranch(diag, isElse)
+	}
+}
 func (ctx *SubstContext) leave(diag Diagnoser) {
 	ctx.deactivate(diag)
 	for _, scope := range ctx.scopes {
 		scope.leave(diag)
+	}
-	if ctx.start(varparam) {
+	if len(ctx.scopes) > 1 {
 		ctx.scopes = ctx.scopes[:len(ctx.scopes)-1]
+	}
+}
 func (ctx *SubstContext) activate(mkline *MkLine, deactivate bool) bool {
 	id := mkline.Varparam()
 	if id == "" {
 		mkline.Errorf("Invalid SUBST class %q in variable name.", id)
 		return false
+	}
 	if deactivate {
 		ctx.deactivate(mkline)
+	}
 	if block := ctx.lookup(id); block != nil {
 		ctx.active = block
 		return true
+	}
-	if ctx.once.FirstTime(varparam) {
+	if ctx.once.FirstTime(id) && !containsWord(mkline.Rationale(), id) {
 		mkline.Warnf("Before defining %s, the SUBST class "+
 			"should be declared using \"SUBST_CLASSES+= %s\".",
-			mkline.Varname(), varparam)
+			mkline.Varname(), id)
+	}
-	return
+	return false
+}
-func (ctx *SubstContext) varassignDifferentClass(mkline *MkLine) (ok bool) {
+func (ctx *SubstContext) deactivate(diag Diagnoser) {
-	varname := mkline.Varname()
+	if !ctx.isActive() {
-	varparam := mkline.Varparam()
+		return
+	}
-	if !ctx.isComplete() {
+	block := ctx.block()
-		mkline.Warnf("Variable %q does not match SUBST class %q.", varname, ctx.activeId())
+	if !block.isConditional() {
 		block.finish(diag)
+	}
 	ctx.active = nil
+}
 func (*SubstContext) isForeign(varcanon string) bool {
 	switch varcanon {
 	case
 		"SUBST_STAGE.*",
 		"SUBST_MESSAGE.*",
 		"SUBST_FILES.*",
 		"SUBST_SED.*",
 		"SUBST_VARS.*",
 		"SUBST_FILTER_CMD.*":
 		return false
+	}
 	return true
+}
 func (*SubstContext) isListCanon(varcanon string) bool {
 	switch varcanon {
 	case
 		"SUBST_FILES.*",
 		"SUBST_SED.*",
 		"SUBST_VARS.*":
 		return true
+	}
 	return false
+}
-	ctx.finishClass(mkline)
+func (ctx *SubstContext) block() *substBlock {
 	assertNotNil(ctx.active)
 	return ctx.active
+}
-	ctx.start(varparam)
+func (ctx *SubstContext) lookup(id string) *substBlock {
-	return true
+	for i := len(ctx.scopes) - 1; i >= 0; i-- {
 		if def := ctx.scopes[i].def(id); def != nil {
 			return def
+		}
+	}
 	return nil
+}
 func (ctx *SubstContext) isDone(id string) bool {
 	for _, scope := range ctx.scopes {
 		if scope.isDone(id) {
 			return true
+		}
+	}
 	return false
+}
 func (ctx *SubstContext) isActive() bool { return ctx.active != nil }
 func (ctx *SubstContext) activeId() string {
 	assert(ctx.isActive())
 	return ctx.active.id
+}
 type substScope struct {
 	defs []*substBlock
+}
 func newSubstScope() *substScope {
 	return &substScope{nil}
+}
 func (s *substScope) def(id string) *substBlock {
 	for _, def := range s.defs {
 		if def.id == id {
 			return def
+		}
+	}
 	return nil
+}
 func (s *substScope) define(id string) {
 	assert(s.def(id) == nil)
 	s.defs = append(s.defs, newSubstBlock(id))
+}
 func (s *substScope) isDone(id string) bool {
 	def := s.def(id)
 	return def != nil && def.done
+}
-func (ctx *SubstContext) varassignStage(mkline *MkLine) {
+func (s *substScope) emptyLine() {
-	if ctx.isConditional() {
+	for _, block := range s.defs {
 		block.seenEmpty = true
+	}
+}
 // finish brings all blocks that are defined in the current scope
 // to an end.
 func (s *substScope) finish(diag Diagnoser) {
 	for _, block := range s.defs {
 		block.finish(diag)
+	}
+}
 func (s *substScope) prepareSubstClasses(diag Diagnoser) {
 	for _, block := range s.defs {
 		if block.hasStarted() && !block.isComplete() {
 			diag.Warnf("Subst block %q should be finished "+
 				"before adding the next class to SUBST_CLASSES.",
 				block.id)
+		}
+	}
+}
 func (s *substScope) enter() {
 	for _, block := range s.defs {
 		block.enter()
+	}
+}
 func (s *substScope) nextBranch(diag Diagnoser, isElse bool) {
 	for _, block := range s.defs {
 		if block.isConditional() {
 			block.nextBranch(isElse)
 		} else {
 			block.finish(diag)
+		}
+	}
+}
 func (s *substScope) leave(diag Diagnoser) {
 	for _, block := range s.defs {
 		if block.isConditional() {
 			block.leave()
 		} else {
 			block.finish(diag)
+		}
+	}
+}
 type substBlock struct {
 	id string
 	// Records which of the SUBST variables have been seen either
 	// directly or in a conditional branch.
 	conds []*substCond
 	// In the paragraph of a SUBST block, there should be only variables
 	// that actually belong to the SUBST block.
 	//
 	// In addition, variables that are mentioned in SUBST_VARS may also
 	// be defined there because they closely relate to the SUBST block.
 	foreignAllowed map[string]struct{}
 	foreign        []*MkLine
 	// Whether there has been an empty line between the SUBST_CLASSES
 	// line and the current line.
 	//
 	// Before the empty line, variables that don't obviously belong to
 	// this SUBST block generate warnings since they may be typos,
 	// such as for a different SUBST block.
 	seenEmpty bool
 	// Whether the SUBST_CLASSES has already gone out of scope.
 	//
 	// XXX: When it is out of scope, it should also be unreachable
 	//  by any pkglint code. There's something inconsistent here.
 	done bool
+}
 func newSubstBlock(id string) *substBlock {
 	assert(id != "")
 	return &substBlock{id: id, conds: []*substCond{newSubstCond()}}
+}
 func (b *substBlock) varassignStage(mkline *MkLine) {
 	if b.isConditional() {
 		mkline.Warnf("%s should not be defined conditionally.", mkline.Varname())
+	}
-	ctx.dupString(mkline, ssStage)
+	b.dupString(mkline, ssStage)
 	value := mkline.Value()
 	if value == "pre-patch" || value == "post-patch" {
 		fix := mkline.Autofix()
 		fix.Warnf("Substitutions should not happen in the patch phase.")
 		fix.Explain(
 			"Performing substitutions during post-patch breaks tools such as",
 			"mkpatches, making it very difficult to regenerate correct patches",
 			"after making changes, and often leading to substituted string",
 			"replacements being committed.",
 			"",
 			"Instead of pre-patch, use post-extract.",
 			"Instead of post-patch, use pre-configure.")
 @@ -186,128 +419,129 @@ func (ctx *SubstContext) varassignStage(
 		fix.Apply()
+	}
 	if G.Pkg != nil && (value == "pre-configure" || value == "post-configure") {
 		if noConfigureLine := G.Pkg.vars.FirstDefinition("NO_CONFIGURE"); noConfigureLine != nil {
 			mkline.Warnf("SUBST_STAGE %s has no effect when NO_CONFIGURE is set (in %s).",
 				value, mkline.RelMkLine(noConfigureLine))
 			mkline.Explain(
 				"To fix this properly, remove the definition of NO_CONFIGURE.")
+		}
+	}
+}
-func (ctx *SubstContext) varassignMessages(mkline *MkLine) {
+func (b *substBlock) varassignMessages(mkline *MkLine) {
 	varname := mkline.Varname()
-	if ctx.isConditional() {
+	if b.isConditional() {
 		mkline.Warnf("%s should not be defined conditionally.", varname)
+	}
-	ctx.dupString(mkline, ssMessage)
+	b.dupString(mkline, ssMessage)
+}
-func (ctx *SubstContext) varassignFiles(mkline *MkLine) {
+func (b *substBlock) varassignFiles(mkline *MkLine) {
-	ctx.dupList(mkline, ssFiles, ssNone)
+	b.dupList(mkline, ssFiles, ssNone)
+}
-func (ctx *SubstContext) varassignSed(mkline *MkLine) {
+func (b *substBlock) varassignSed(mkline *MkLine) {
-	ctx.dupList(mkline, ssSed, ssNone)
+	b.dupList(mkline, ssSed, ssNone)
-	ctx.seen().set(ssTransform)
+	b.addSeen(ssTransform)
-	ctx.suggestSubstVars(mkline)
+	b.suggestSubstVars(mkline)
+}
-func (ctx *SubstContext) varassignVars(mkline *MkLine) {
+func (b *substBlock) varassignVars(mkline *MkLine) {
-	ctx.dupList(mkline, ssVars, ssVarsAutofix)
+	b.dupList(mkline, ssVars, ssVarsAutofix)
-	ctx.seen().set(ssTransform)
+	b.addSeen(ssTransform)
 	for _, substVar := range mkline.Fields() {
-		ctx.allowVar(substVar)
+		b.allowVar(substVar)
+	}
+}
-func (ctx *SubstContext) varassignFilterCmd(mkline *MkLine) {
+func (b *substBlock) varassignFilterCmd(mkline *MkLine) {
-	ctx.dupString(mkline, ssFilterCmd)
+	b.dupString(mkline, ssFilterCmd)
-	ctx.seen().set(ssTransform)
+	b.addSeen(ssTransform)
+}
-func (ctx *SubstContext) dupList(mkline *MkLine, part substSeen, autofixPart substSeen) {
+func (b *substBlock) suggestSubstVars(mkline *MkLine) {
 	if ctx.seenInBranch(part) && mkline.Op() != opAssignAppend {
 		ctx.fixOperatorAppend(mkline, ctx.seenInBranch(autofixPart))
 	ctx.seen().set(part)
 func (ctx *SubstContext) dupString(mkline *MkLine, part substSeen) {
 	if ctx.seenInBranch(part) {
 		mkline.Warnf("Duplicate definition of %q.", mkline.Varname())
 	ctx.seen().set(part)
 func (ctx *SubstContext) fixOperatorAppend(mkline *MkLine, dueToAutofix bool) {
 	before := mkline.ValueAlign()
 	after := alignWith(mkline.Varname()+"+=", before)
 	fix := mkline.Autofix()
 	if dueToAutofix {
 		fix.Notef(SilentAutofixFormat)
 	} else {
 		fix.Warnf("All but the first assignment to %q should use the \"+=\" operator.",
 			mkline.Varname())
 	fix.Replace(before, after)
 	fix.Apply()
 func (ctx *SubstContext) suggestSubstVars(mkline *MkLine) {
 	tokens, _ := splitIntoShellTokens(mkline.Line, mkline.Value())
 	for _, token := range tokens {
-		varname := ctx.extractVarname(mkline.UnquoteShell(token, false))
+		varname := b.extractVarname(mkline.UnquoteShell(token, false))
 		if varname == "" {
 			continue
+		}
-		id := ctx.activeId()
+		id := b.id
 		varop := sprintf("SUBST_VARS.%s%s%s",
 			id,
 			condStr(hasSuffix(id, "+"), " ", ""),
-			condStr(ctx.seenInBranch(ssVars), "+=", "="))
+			condStr(b.hasSeen(ssVars), "+=", "="))
 		fix := mkline.Autofix()
 		fix.Notef("The substitution command %q can be replaced with \"%s %s\".",
 			token, varop, varname)
 		fix.Explain(
 			"Replacing @VAR@ with ${VAR} is such a typical pattern that pkgsrc has built-in support for it,",
 			"requiring only the variable name instead of the full sed command.")
 		if !mkline.HasComment() && len(tokens) == 2 && tokens[0] == "-e" {
 			fix.Replace(mkline.Text, alignWith(varop, mkline.ValueAlign())+varname)
+		}
 		fix.Apply()
 		// At this point the number of SUBST_SED assignments is one
 		// less than before. Therefore it is possible to adjust the
 		// assignment operators on them. It's probably not worth the
 		// effort, though.
-		ctx.seen().set(ssVars | ssVarsAutofix)
+		b.addSeen(ssVars)
 		b.addSeen(ssVarsAutofix)
+	}
+}
 func (b *substBlock) dupString(mkline *MkLine, part substSeen) {
 	if b.hasSeen(part) {
 		mkline.Warnf("Duplicate definition of %q.", mkline.Varname())
+	}
 	b.addSeen(part)
+}
 func (b *substBlock) dupList(mkline *MkLine, part substSeen, autofixPart substSeen) {
 	if b.hasSeenAnywhere(part) && mkline.Op() != opAssignAppend {
 		b.fixOperatorAppend(mkline, b.hasSeen(autofixPart))
+	}
 	b.addSeen(part)
+}
 func (b *substBlock) fixOperatorAppend(mkline *MkLine, dueToAutofix bool) {
 	before := mkline.ValueAlign()
 	after := alignWith(mkline.Varname()+"+=", before)
 	fix := mkline.Autofix()
 	if dueToAutofix {
 		fix.Notef(SilentAutofixFormat)
 	} else {
 		fix.Warnf("All but the first assignment to %q should use the \"+=\" operator.",
 			mkline.Varname())
+	}
 	fix.Replace(before, after)
 	fix.Apply()
+}
 // extractVarname extracts the variable name from a sed command of the form
 // s,@VARNAME@,${VARNAME}, and some related variants thereof.
-func (*SubstContext) extractVarname(token string) string {
+func (*substBlock) extractVarname(token string) string {
 	parser := NewMkLexer(token, nil)
 	lexer := parser.lexer
 	if !lexer.SkipByte('s') {
 		return ""
+	}
 	separator := lexer.NextByteSet(textproc.XPrint) // Really any character works
 	if separator == -1 {
 		return ""
+	}
 	if !lexer.SkipByte('@') {
 		return ""
 @@ -327,298 +561,209 @@ func (*SubstContext) extractVarname(toke
 	case "", ":Q":
 		break
 	default:
 		return ""
+	}
 	if !lexer.SkipByte(byte(separator)) {
 		return ""
+	}
 	return varname
+}
-func (*SubstContext) isForeign(varcanon string) bool {
+func (b *substBlock) isComplete() bool {
-	switch varcanon {
+	return b.allSeen().hasAll(ssStage | ssFiles | ssTransform)
 	case
 		"SUBST_STAGE.*",
 		"SUBST_MESSAGE.*",
 		"SUBST_FILES.*",
 		"SUBST_SED.*",
 		"SUBST_VARS.*",
 		"SUBST_FILTER_CMD.*":
 		return false
 	return true
+}
-func (*SubstContext) isListCanon(varcanon string) bool {
+func (b *substBlock) hasSeen(part substSeen) bool {
-	switch varcanon {
+	for _, cond := range b.conds {
-	case
+		if cond.hasSeen(part) {
-		"SUBST_FILES.*",
+			return true
 		"SUBST_SED.*",
 		"SUBST_VARS.*":
 		return true
+	}
 	return false
+}
-func (ctx *SubstContext) directive(mkline *MkLine) {
+func (b *substBlock) hasSeenAnywhere(part substSeen) bool {
-	dir := mkline.Directive()
+	for _, cond := range b.conds {
-	switch dir {
+		if cond.hasSeenAnywhere(part) {
-	case "if":
+			return true
 		ctx.condIf()
 	case "elif", "else":
 		ctx.condElse(mkline, dir)
 	case "endif":
 		ctx.condEndif(mkline)
+	}
 	return false
+}
-func (ctx *SubstContext) condIf() {
+func (b *substBlock) allSeen() substSeen {
-	top := substCond{total: ssAll}
+	all := ssNone
-	ctx.conds = append(ctx.conds, &top)
+	for _, cond := range b.conds {
 		all.addAll(cond.curr)
+	}
 	return all
+}
-func (ctx *SubstContext) condElse(mkline *MkLine, dir string) {
+func (b *substBlock) addSeen(part substSeen) {
-	top := ctx.cond()
+	b.conds[len(b.conds)-1].addSeen(part)
 	top.total.retain(top.curr)
 	if !ctx.isConditional() {
 		ctx.finishClass(mkline)
 	top.curr = ssNone
 	top.seenElse = dir == "else"
+}
-func (ctx *SubstContext) condEndif(diag Diagnoser) {
+func (b *substBlock) rememberForeign(mkline *MkLine) {
-	top := ctx.cond()
+	b.foreign = append(b.foreign, mkline)
 	top.total.retain(top.curr)
 	if !ctx.isConditional() {
 		ctx.finishClass(diag)
 	if !top.seenElse {
 		top.total = ssNone
 	if len(ctx.conds) > 1 {
 		ctx.conds = ctx.conds[:len(ctx.conds)-1]
 	ctx.seen().union(top.total)
+}
-func (ctx *SubstContext) finishClass(diag Diagnoser) {
+// isConditional returns whether the current line is at a deeper conditional
-	if !ctx.isActive() {
+// level than the corresponding SUBST_CLASSES line.
-		return
+func (b *substBlock) isConditional() bool { return len(b.conds) > 1 }
-	if ctx.seen().get(ssAll) {
+func (b *substBlock) allowVar(varname string) {
-		ctx.checkBlockComplete(diag)
+	if b.foreignAllowed == nil {
-		ctx.checkForeignVariables()
+		b.foreignAllowed = map[string]struct{}{}
 	} else {
 		ctx.markAsNotDone()
+	}
 	b.foreignAllowed[varname] = struct{}{}
 	ctx.reset()
+}
-func (ctx *SubstContext) checkBlockComplete(diag Diagnoser) {
+func (b *substBlock) enter() { b.conds = append(b.conds, newSubstCond()) }
 	id := ctx.activeId()
 	seen := ctx.seen()
 	if !seen.get(ssStage) {
 		diag.Warnf("Incomplete SUBST block: SUBST_STAGE.%s missing.", id)
 	if !seen.get(ssFiles) {
 		diag.Warnf("Incomplete SUBST block: SUBST_FILES.%s missing.", id)
 	if !seen.get(ssTransform) {
 		diag.Warnf("Incomplete SUBST block: SUBST_SED.%[1]s, SUBST_VARS.%[1]s or SUBST_FILTER_CMD.%[1]s missing.", id)
-func (ctx *SubstContext) checkForeignVariables() {
+func (b *substBlock) nextBranch(isElse bool) {
-	ctx.forEachForeignVar(func(mkline *MkLine) {
+	cond := b.conds[len(b.conds)-1]
-		mkline.Warnf("Foreign variable %q in SUBST block.", mkline.Varname())
+	cond.leaveBranch()
-	})
+	cond.enterBranch(isElse)
+}
-func (ctx *SubstContext) finishFile(diag Diagnoser) {
+func (b *substBlock) leave() {
-	for _, id := range ctx.queuedIds {
+	assert(b.isConditional())
 		if id != "" && !ctx.isDone(id) {
 			ctx.warnUndefinedBlock(diag, id)
-func (*SubstContext) warnUndefinedBlock(diag Diagnoser, id string) {
+	n := len(b.conds)
-	diag.Warnf("Missing SUBST block for %q.", id)
+	b.conds[n-2].leaveLevel(b.conds[n-1])
-	diag.Explain(
+	b.conds = b.conds[:n-1]
 		"After adding a SUBST class to SUBST_CLASSES,",
 		"the remaining SUBST variables should be defined in the same file.",
 		"",
 		"See mk/subst.mk for the comprehensive documentation.")
+}
-// In the paragraph of a SUBST block, there should be only variables
+func (b *substBlock) finish(diag Diagnoser) {
-// that actually belong to the SUBST block.
+	assert(len(b.conds) == 1)
 //
 // In addition, variables that are mentioned in SUBST_VARS may also
 // be defined there because they closely relate to the SUBST block.
-func (ctx *SubstContext) allowVar(varname string) {
+	if b.done {
-	if ctx.foreignAllowed == nil {
+		return
 		ctx.foreignAllowed = make(map[string]struct{})
+	}
-	ctx.foreignAllowed[varname] = struct{}{}
+	b.done = true
-func (ctx *SubstContext) rememberForeign(mkline *MkLine) {
+	if !b.hasStarted() {
-	ctx.foreign = append(ctx.foreign, mkline)
+		diag.Warnf("Missing SUBST block for %q.", b.id)
 		return
 // forEachForeignVar performs the given action for each variable that
 // is defined in the SUBST block and is not mentioned in SUBST_VARS.
 func (ctx *SubstContext) forEachForeignVar(action func(*MkLine)) {
 	for _, foreign := range ctx.foreign {
 		if _, ok := ctx.foreignAllowed[foreign.Varname()]; !ok {
 			action(foreign)
+	}
 func (ctx *SubstContext) reset() {
 	ctx.id = ""
 	ctx.foreignAllowed = nil
 	ctx.foreign = nil
 	ctx.conds = []*substCond{{seenElse: true}}
 func (ctx *SubstContext) isActive() bool { return ctx.id != "" }
 func (ctx *SubstContext) isActiveId(id string) bool { return ctx.id == id }
 func (ctx *SubstContext) activeId() string {
 	assert(ctx.isActive())
 	return ctx.id
 func (ctx *SubstContext) setActiveId(id string) {
 	ctx.id = id
 	ctx.cond().top = true
 	ctx.markAsDone(id)
-func (ctx *SubstContext) queue(id string) {
+	if !b.hasSeen(ssStage) {
-	ctx.queuedIds = append(ctx.queuedIds, id)
+		diag.Warnf("Incomplete SUBST block: SUBST_STAGE.%s missing.", b.id)
 func (ctx *SubstContext) start(id string) bool {
 	for i, queuedId := range ctx.queuedIds {
 		if queuedId == id {
 			ctx.queuedIds[i] = ""
 			ctx.setActiveId(id)
 			return true
+	}
-	return false
+	if !b.hasSeen(ssFiles) {
 		diag.Warnf("Incomplete SUBST block: SUBST_FILES.%s missing.", b.id)
+	}
-func (ctx *SubstContext) markAsDone(id string) {
+	if !b.hasSeen(ssTransform) {
-	if ctx.doneIds == nil {
+		diag.Warnf(
-		ctx.doneIds = map[string]bool{}
+			"Incomplete SUBST block: SUBST_SED.%[1]s, "+
 				"SUBST_VARS.%[1]s or SUBST_FILTER_CMD.%[1]s missing.",
 			b.id)
+	}
 	ctx.doneIds[id] = true
 func (ctx *SubstContext) markAsNotDone() {
 	ctx.doneIds[ctx.id] = false
 func (ctx *SubstContext) isDone(varparam string) bool {
 	return ctx.doneIds[varparam]
 func (ctx *SubstContext) isComplete() bool {
 	return ctx.seen().hasAll(ssStage | ssFiles | ssTransform)
 // isConditional returns whether the current line is at a deeper conditional
 // level than the assignment where the corresponding class ID is added to
 // SUBST_CLASSES.
 //
 // TODO: Adjust the implementation to this description.
 func (ctx *SubstContext) isConditional() bool {
 	return !ctx.cond().top
-// cond returns information about the parts of the SUBST block that
+	b.checkForeignVariables()
 // have already been seen in the current leaf branch of the conditionals.
 func (ctx *SubstContext) seen() *substSeen {
 	return &ctx.cond().curr
+}
-// cond returns information about the current branch of conditionals.
+func (b *substBlock) checkForeignVariables() {
-func (ctx *SubstContext) cond() *substCond {
+	for _, mkline := range b.foreign {
-	return ctx.conds[len(ctx.conds)-1]
+		if _, ok := b.foreignAllowed[mkline.Varname()]; !ok {
 			mkline.Warnf("Foreign variable %q in SUBST block.", mkline.Varname())
+		}
+	}
+}
-// Returns true if the given flag from substSeen has been seen
+func (b *substBlock) hasStarted() bool {
-// somewhere in the conditional path of the current line.
+	for _, cond := range b.conds {
-func (ctx *SubstContext) seenInBranch(part substSeen) bool {
+		if cond.currAny != ssNone {
 	for _, cond := range ctx.conds {
 		if cond.curr.get(part) {
 			return true
+		}
+	}
 	return false
+}
 type substCond struct {
 	// Tells whether a SUBST block has started at this conditional level.
 	// All variable assignments that belong to this class must happen at
 	// this conditional level or below it.
 	//
 	// TODO: For Test_SubstContext_Directive__conditional_complete,
 	//  this needs to be changed to the set of classes that have been
 	//  added to SUBST_CLASSES at this level.
 	top bool
 	// Collects the parts of the SUBST block that have been defined in all
 	// branches that have been parsed completely.
-	total substSeen
+	total    substSeen
 	totalAny substSeen
 	// Collects the parts of the SUBST block that are defined in the current
 	// branch of the conditional. At the end of the branch, they are merged
 	// into the total.
-	curr substSeen
+	curr    substSeen
 	currAny substSeen
 	// Marks whether the current conditional statement has
 	// an .else branch. If it doesn't, this means that all variables
 	// are potentially unset in that branch.
 	seenElse bool
+}
 func newSubstCond() *substCond { return &substCond{total: ssAll} }
 func (c *substCond) enterBranch(isElse bool) {
 	c.curr = ssNone
 	c.currAny = ssNone
 	c.seenElse = isElse
+}
 func (c *substCond) leaveBranch() {
 	c.total.retainAll(c.curr)
 	c.totalAny.addAll(c.currAny)
 	c.curr = ssNone
 	c.currAny = ssNone
+}
 func (c *substCond) leaveLevel(child *substCond) {
 	child.leaveBranch()
 	if child.seenElse {
 		c.curr.addAll(child.total)
+	}
 	c.currAny.addAll(child.totalAny)
+}
 func (c *substCond) hasSeen(part substSeen) bool { return c.curr.has(part) }
 func (c *substCond) hasSeenAnywhere(part substSeen) bool {
 	return c.currAny.has(part)
+}
 func (c *substCond) addSeen(part substSeen) {
 	c.curr.set(part)
 	c.currAny.set(part)
+}
 // substSeen contains all variables that depend on a particular SUBST
 // class ID. These variables can be set in conditional branches, and
 // pkglint keeps track whether they are set in all branches or only
 // in some of them.
 type substSeen uint8
 const (
 	ssStage substSeen = 1 << iota
 	ssMessage
 	ssFiles
 	ssSed
 	ssVars
 	ssVarsAutofix
 	ssFilterCmd
 	ssTransform
 	ssAll  substSeen = 1<<iota - 1
 	ssNone substSeen = 0
+)
-func (s *substSeen) set(part substSeen)          { *s |= part }
+func (s *substSeen) set(part substSeen) {
-func (s *substSeen) get(part substSeen) bool     { return *s&part != 0 }
+	assert(part&(part-1) == 0)
-func (s *substSeen) hasAll(other substSeen) bool { return *s&other == other }
+	*s |= part
 func (s *substSeen) union(other substSeen)       { *s |= other }
 func (s *substSeen) retain(other substSeen)      { *s &= other }
 func (s *substSeen) has(part substSeen) bool {
 	assert(part&(part-1) == 0)
 	return *s&part != 0
+}
 func (s substSeen) hasAll(other substSeen) bool { return s&other == other }
 func (s *substSeen) addAll(other substSeen)     { *s |= other }
 func (s *substSeen) retainAll(other substSeen)  { *s &= other }

 @@ -1,443 +1,381 @@
 package pkglint
 import "gopkg.in/check.v1"
 func (t *Tester) NewSubstAutofixTest(lines ...string) func(bool) {
 	return func(autofix bool) {
 		mklines := t.NewMkLines("filename.mk", lines...)
 		mklines.collectRationale()
 		ctx := NewSubstContext()
 		mklines.ForEach(ctx.Process)
 		ctx.Finish(mklines.EOFLine())
 		mklines.SaveAutofixChanges()
+	}
+}
 func (t *Tester) RunSubst(lines ...string) {
 	assert(lines[len(lines)-1] != "")
 	mklines := t.NewMkLines("filename.mk", lines...)
 	mklines.collectRationale()
 	ctx := NewSubstContext()
 	mklines.ForEach(ctx.Process)
 	ctx.Finish(mklines.EOFLine())
+}
 func (s *Suite) Test_SubstContext__OPSYSVARS(c *check.C) {
 	t := s.Init(c)
 	ctx := NewSubstContext()
 	// SUBST_CLASSES is added to OPSYSVARS in mk/bsd.pkg.mk.
 	ctx.varassign(t.NewMkLine("filename.mk", 11, "SUBST_CLASSES.SunOS+=prefix"))
 	ctx.varassign(t.NewMkLine("filename.mk", 12, "SUBST_CLASSES.NetBSD+=prefix"))
 	ctx.varassign(t.NewMkLine("filename.mk", 13, "SUBST_FILES.prefix=Makefile"))
 	ctx.varassign(t.NewMkLine("filename.mk", 14, "SUBST_SED.prefix=s,@PREFIX@,${PREFIX},g"))
 	ctx.varassign(t.NewMkLine("filename.mk", 15, "SUBST_STAGE.prefix=post-configure"))
 	t.CheckEquals(ctx.isComplete(), true)
 	ctx.Finish(t.NewMkLine("filename.mk", 15, ""))
 	t.CheckOutputLines(
 		"NOTE: filename.mk:14: The substitution command \"s,@PREFIX@,${PREFIX},g\" " +
 			"can be replaced with \"SUBST_VARS.prefix= PREFIX\".")
 func (s *Suite) Test_SubstContext__no_class(c *check.C) {
 	t := s.Init(c)
 	t.RunSubst(
 		"UNRELATED=anything",
 		"SUBST_FILES.repl+=Makefile.in",
 		"SUBST_SED.repl+=-e s,from,to,g")
 	t.CheckOutputLines(
 		"WARN: filename.mk:2: Before defining SUBST_FILES.repl, " +
 			"the SUBST class should be declared using \"SUBST_CLASSES+= repl\".")
 func (s *Suite) Test_SubstContext__multiple_classes_in_one_line(c *check.C) {
 	t := s.Init(c)
 	t.RunSubst(
 		"SUBST_CLASSES+=         one two",
 		"SUBST_STAGE.one=        post-configure",
 		"SUBST_FILES.one=        one.txt",
 		"SUBST_SED.one=          s,one,1,g",
 		"SUBST_STAGE.two=        post-configure",
 		"SUBST_FILES.two=        two.txt")
 	t.CheckOutputLines(
 		"NOTE: filename.mk:1: Please add only one class at a time to SUBST_CLASSES.",
 		"WARN: filename.mk:EOF: Incomplete SUBST block: SUBST_SED.two, SUBST_VARS.two or SUBST_FILTER_CMD.two missing.")
 func (s *Suite) Test_SubstContext__multiple_classes_in_one_line_multiple_blocks(c *check.C) {
 	t := s.Init(c)
 	t.RunSubst(
 		"SUBST_CLASSES+=         one two",
 		"SUBST_STAGE.one=        post-configure",
 		"SUBST_FILES.one=        one.txt",
 		"SUBST_SED.one=          s,one,1,g",
 		"",
 		"SUBST_STAGE.two=        post-configure",
 		"SUBST_FILES.two=        two.txt",
 		"",
 		"SUBST_STAGE.three=      post-configure",
 		"",
 		"SUBST_VARS.four=        PREFIX",
 		"",
 		"SUBST_VARS.three=       PREFIX")
 	t.CheckOutputLines(
 		"NOTE: filename.mk:1: Please add only one class at a time to SUBST_CLASSES.",
 		"WARN: filename.mk:8: Incomplete SUBST block: "+
 			"SUBST_SED.two, SUBST_VARS.two or SUBST_FILTER_CMD.two missing.",
 		"WARN: filename.mk:9: Before defining SUBST_STAGE.three, "+
 			"the SUBST class should be declared using \"SUBST_CLASSES+= three\".",
 		"WARN: filename.mk:11: Before defining SUBST_VARS.four, "+
 			"the SUBST class should be declared using \"SUBST_CLASSES+= four\".")
 func (s *Suite) Test_SubstContext__multiple_classes_in_one_block(c *check.C) {
 	t := s.Init(c)
 	t.RunSubst(
 		"SUBST_CLASSES+=         one",
 		"SUBST_STAGE.one=        post-configure",
 		"SUBST_STAGE.one=        post-configure",
 		"SUBST_FILES.one=        one.txt",
 		"SUBST_CLASSES+=         two", // The block "one" is not finished yet.
 		"SUBST_SED.one=          s,one,1,g",
 		"SUBST_STAGE.two=        post-configure",
 		"SUBST_FILES.two=        two.txt",
 		"SUBST_SED.two=          s,two,2,g")
 	t.CheckOutputLines(
 		"WARN: filename.mk:3: Duplicate definition of \"SUBST_STAGE.one\".",
 		"WARN: filename.mk:5: Incomplete SUBST block: SUBST_SED.one, SUBST_VARS.one or SUBST_FILTER_CMD.one missing.",
 		"WARN: filename.mk:5: Subst block \"one\" should be finished before adding the next class to SUBST_CLASSES.",
 		"WARN: filename.mk:6: Variable \"SUBST_SED.one\" does not match SUBST class \"two\".")
 // This is a strange example that probably won't occur in practice.
 //
 // Continuing a SUBST class in one of the branches and starting
 // a fresh one in the other seems far-fetched.
 func (s *Suite) Test_SubstContext__partially_continued_class_in_conditional(c *check.C) {
 	t := s.Init(c)
 	t.RunSubst(
 		"SUBST_CLASSES+=         outer",
 		"SUBST_STAGE.outer=      post-configure",
 		"SUBST_FILES.outer=      files",
 		"SUBST_VARS.outer=       OUTER.first",
 		".if ${:Ualways}",
 		"SUBST_VARS.outer+=      OUTER.second",
 		".else",
 		"SUBST_CLASSES+=         inner",
 		"SUBST_STAGE.inner=      post-configure",
 		"SUBST_FILES.inner=      files",
 		"SUBST_VARS.inner=       INNER",
 		".endif")
 	t.CheckOutputEmpty()
+}
-func (s *Suite) Test_SubstContext__files_missing(c *check.C) {
+func (s *Suite) Test_SubstContext__conditionals(c *check.C) {
 	t := s.Init(c)
 	t.RunSubst(
 		"SUBST_CLASSES+=         one",
 		"SUBST_STAGE.one=        pre-configure",
 		"SUBST_CLASSES+=         two",
 		"SUBST_STAGE.two=        pre-configure",
 		"SUBST_FILES.two=        two.txt",
 		"SUBST_SED.two=          s,two,2,g")
 	t.CheckOutputLines(
 		"WARN: filename.mk:3: Incomplete SUBST block: SUBST_FILES.one missing.",
 		"WARN: filename.mk:3: Incomplete SUBST block: "+
 			"SUBST_SED.one, SUBST_VARS.one or SUBST_FILTER_CMD.one missing.",
 		"WARN: filename.mk:3: Subst block \"one\" should be finished "+
 			"before adding the next class to SUBST_CLASSES.")
 func (s *Suite) Test_SubstContext__directives(c *check.C) {
 	t := s.Init(c)
 	t.RunSubst(
 		"SUBST_CLASSES+=         os",
 		"SUBST_STAGE.os=         post-configure",
 		"SUBST_MESSAGE.os=       Guessing operating system",
 		"SUBST_FILES.os=         guess-os.h",
 		".if ${OPSYS} == NetBSD",
 		"SUBST_FILTER_CMD.os=    ${SED} -e s,@OPSYS@,NetBSD,",
 		".elif ${OPSYS} == Darwin",
 		"SUBST_SED.os=           -e s,@OPSYS@,Darwin1,",
 		"SUBST_SED.os=           -e s,@OPSYS@,Darwin2,",
 		".elif ${OPSYS} == Linux",
 		"SUBST_SED.os=           -e s,@OPSYS@,Linux,",
 		".else",
 		"SUBST_VARS.os=           OPSYS",
 		".endif")
 	// All the other lines are correctly determined as being alternatives
 	// to each other. And since every branch contains some transformation
 	// (SED, VARS, FILTER_CMD), everything is fine.
 	t.CheckOutputLines(
 		"WARN: filename.mk:9: All but the first assignment " +
 			"to \"SUBST_SED.os\" should use the \"+=\" operator.")
+}
-func (s *Suite) Test_SubstContext__adjacent(c *check.C) {
+func (s *Suite) Test_SubstContext_varassign__no_class(c *check.C) {
 	t := s.Init(c)
 	t.RunSubst(
 		"SUBST_CLASSES+=\t1",
 		"SUBST_STAGE.1=\tpre-configure",
 		"SUBST_FILES.1=\tfile1",
 		"SUBST_SED.1=\t-e s,subst1,repl1,",
 		"SUBST_CLASSES+=\t2",
 		"SUBST_SED.1+=\t-e s,subst1b,repl1b,", // Misplaced
 		"SUBST_STAGE.2=\tpre-configure",
 		"SUBST_FILES.2=\tfile2",
 		"SUBST_SED.2=\t-e s,subst2,repl2,")
 	t.CheckOutputLines(
 		"WARN: filename.mk:6: Variable \"SUBST_SED.1\" does not match SUBST class \"2\".")
 func (s *Suite) Test_SubstContext__do_patch(c *check.C) {
 	t := s.Init(c)
 	t.RunSubst(
 		"SUBST_CLASSES+=\tos",
 		"SUBST_STAGE.os=\tdo-patch",
 		"SUBST_FILES.os=\tguess-os.h",
 		"SUBST_SED.os=\t-e s,@OPSYS@,Darwin,")
 	// No warning, since there is nothing to fix automatically.
 	// This case doesn't occur in practice anyway.
 	t.CheckOutputEmpty()
 // Variables mentioned in SUBST_VARS are not considered "foreign"
 // in the block and may be mixed with the other SUBST variables.
 func (s *Suite) Test_SubstContext__SUBST_VARS_defined_in_block(c *check.C) {
 	t := s.Init(c)
 	t.RunSubst(
-		"SUBST_CLASSES+=\tos",
+		"UNRELATED=anything",
-		"SUBST_STAGE.os=\tpre-configure",
+		"SUBST_FILES.repl+=Makefile.in",
-		"SUBST_FILES.os=\tguess-os.h",
+		"SUBST_SED.repl+=-e s,from,to,g")
 		"SUBST_VARS.os=\tTODAY1",
 		"TODAY1!=\tdate",
 		"TODAY2!=\tdate")
 	t.CheckOutputLines(
-		"WARN: filename.mk:6: Foreign variable \"TODAY2\" in SUBST block.")
+		"WARN: filename.mk:2: Before defining SUBST_FILES.repl, " +
 			"the SUBST class should be declared using \"SUBST_CLASSES+= repl\".")
+}
-// Variables mentioned in SUBST_VARS may appear in the same paragraph,
+func (s *Suite) Test_SubstContext_varassign__multiple_classes_in_one_line_multiple_blocks(c *check.C) {
 // or alternatively anywhere else in the file.
 func (s *Suite) Test_SubstContext__SUBST_VARS_in_next_paragraph(c *check.C) {
 	t := s.Init(c)
 	t.RunSubst(
-		"SUBST_CLASSES+=\tos",
+		"SUBST_CLASSES+=         one two",
-		"SUBST_STAGE.os=\tpre-configure",
+		"SUBST_STAGE.one=        post-configure",
-		"SUBST_FILES.os=\tguess-os.h",
+		"SUBST_FILES.one=        one.txt",
-		"SUBST_VARS.os=\tTODAY1",
+		"SUBST_SED.one=          s,one,1,g",
 		"",
-		"TODAY1!=\tdate",
+		"SUBST_STAGE.two=        post-configure",
-		"TODAY2!=\tdate")
+		"SUBST_FILES.two=        two.txt",
 		"",
 		"SUBST_STAGE.three=      post-configure",
 		"",
 		"SUBST_VARS.four=        PREFIX",
 		"",
 		"SUBST_VARS.three=       PREFIX")
-	t.CheckOutputEmpty()
+	t.CheckOutputLines(
 		"NOTE: filename.mk:1: Please add only one class at a time to SUBST_CLASSES.",
 		"WARN: filename.mk:9: Variable \"SUBST_STAGE.three\" "+
 			"does not match SUBST class \"two\".",
 		"WARN: filename.mk:9: Incomplete SUBST block: "+
 			"SUBST_SED.two, SUBST_VARS.two or SUBST_FILTER_CMD.two missing.",
 		"WARN: filename.mk:9: Before defining SUBST_STAGE.three, "+
 			"the SUBST class should be declared using \"SUBST_CLASSES+= three\".",
 		"WARN: filename.mk:11: Before defining SUBST_VARS.four, "+
 			"the SUBST class should be declared using \"SUBST_CLASSES+= four\".")
+}
-func (s *Suite) Test_SubstContext__multiple_SUBST_VARS(c *check.C) {
+func (s *Suite) Test_SubstContext_varassign__multiple_classes_in_one_block(c *check.C) {
 	t := s.Init(c)
 	t.RunSubst(
-		"SUBST_CLASSES+=\tos",
+		"SUBST_CLASSES+=         one",
-		"SUBST_STAGE.os=\tpre-configure",
+		"SUBST_STAGE.one=        post-configure",
-		"SUBST_FILES.os=\tguess-os.h",
+		"SUBST_STAGE.one=        post-configure",
-		"SUBST_VARS.os=\tPREFIX VARBASE")
+		"SUBST_FILES.one=        one.txt",
 		"SUBST_CLASSES+=         two", // The block "one" is not finished yet.
 		"SUBST_SED.one=          s,one,1,g",
 		"SUBST_STAGE.two=        post-configure",
 		"SUBST_FILES.two=        two.txt",
 		"SUBST_SED.two=          s,two,2,g")
-	t.CheckOutputEmpty()
+	t.CheckOutputLines(
 		"WARN: filename.mk:3: Duplicate definition of \"SUBST_STAGE.one\".",
 		"WARN: filename.mk:5: Subst block \"one\" should be finished "+
 			"before adding the next class to SUBST_CLASSES.",
 		"WARN: filename.mk:5: Incomplete SUBST block: SUBST_SED.one, SUBST_VARS.one or SUBST_FILTER_CMD.one missing.",
 		"WARN: filename.mk:6: Late additions to a SUBST variable should use the += operator.")
+}
-// As of May 2019, pkglint does not check the order of the variables in
+// As of December 2019, pkglint does not check the order of the variables in
 // a SUBST block. Enforcing this order, or at least suggesting it, would
 // make pkgsrc packages more uniform, which is a good idea, but not urgent.
-func (s *Suite) Test_SubstContext__unusual_variable_order(c *check.C) {
+func (s *Suite) Test_SubstContext_varassign__unusual_order(c *check.C) {
 	t := s.Init(c)
 	t.RunSubst(
 		"SUBST_CLASSES+=\t\tid",
 		"SUBST_SED.id=\t\t-e /deleteme/d",
 		"SUBST_FILES.id=\t\tfile",
 		"SUBST_MESSAGE.id=\tMessage",
 		"SUBST_STAGE.id=\t\tpre-configure")
 	t.CheckOutputEmpty()
+}
-func (s *Suite) Test_SubstContext__completely_conditional_then(c *check.C) {
+func (s *Suite) Test_SubstContext_varassign__blocks_in_separate_paragraphs(c *check.C) {
 	t := s.Init(c)
 	t.RunSubst(
 		".if ${OPSYS} == Linux",
 		"SUBST_CLASSES+=\tid",
 		"SUBST_STAGE.id=\tpre-configure",
 		"SUBST_SED.id=\t-e sahara",
 		".else",
 		".endif")
 	// The block already ends at the .else, not at the end of the file,
 	// since that is the scope where the SUBST id is defined.
 	t.CheckOutputLines(
 		"WARN: filename.mk:5: Incomplete SUBST block: SUBST_FILES.id missing.")
 func (s *Suite) Test_SubstContext__completely_conditional_else(c *check.C) {
 	t := s.Init(c)
 	t.RunSubst(
 		".if ${OPSYS} == Linux",
 		".else",
 		"SUBST_CLASSES+=\tid",
 		"SUBST_STAGE.id=\tpre-configure",
 		"SUBST_SED.id=\t-e sahara",
 		".endif")
 	// The block already ends at the .endif, not at the end of the file,
 	// since that is the scope where the SUBST id is defined.
 	t.CheckOutputLines(
 		"WARN: filename.mk:6: Incomplete SUBST block: SUBST_FILES.id missing.")
 func (s *Suite) Test_SubstContext__SUBST_CLASSES_in_separate_paragraph(c *check.C) {
 	t := s.Init(c)
 	t.RunSubst(
 		"SUBST_CLASSES+= 1 2 3 4",
 		"",
 		"SUBST_STAGE.1=  post-configure",
 		"SUBST_FILES.1=  files",
 		"SUBST_VARS.1=   VAR1",
 		"",
 		"SUBST_STAGE.2=  post-configure",
 		"SUBST_FILES.2=  files",
 		"SUBST_VARS.2=   VAR1",
 		"",
 		"SUBST_STAGE.3=  post-configure",
 		"SUBST_FILES.3=  files",
 		"SUBST_VARS.3=   VAR1")
 	t.CheckOutputLines(
 		"NOTE: filename.mk:1: Please add only one class at a time to SUBST_CLASSES.",
 		"WARN: filename.mk:EOF: Missing SUBST block for \"4\".")
+}
-func (s *Suite) Test_SubstContext__wrong_class(c *check.C) {
+func (s *Suite) Test_SubstContext_varassign__typo_in_id(c *check.C) {
 	t := s.Init(c)
 	t.RunSubst(
 		"SUBST_CLASSES+= 1 2",
 		"SUBST_STAGE.x=  post-configure",
 		"SUBST_FILES.x=  files",
 		"SUBST_VARS.x=   VAR1",
 		"SUBST_STAGE.2=  post-configure",
 		"SUBST_FILES.2=  files",
 		"SUBST_VARS.2=   VAR1")
 	t.CheckOutputLines(
 		"NOTE: filename.mk:1: Please add only one class at a time to SUBST_CLASSES.",
-		"WARN: filename.mk:2: Variable \"SUBST_STAGE.x\" does not match SUBST class \"1\".",
+		"WARN: filename.mk:2: Before defining SUBST_STAGE.x, "+
-		"WARN: filename.mk:3: Variable \"SUBST_FILES.x\" does not match SUBST class \"1\".",
+			"the SUBST class should be declared using \"SUBST_CLASSES+= x\".",
-		"WARN: filename.mk:4: Variable \"SUBST_VARS.x\" does not match SUBST class \"1\".",
+		"WARN: filename.mk:EOF: Missing SUBST block for \"1\".")
 		// XXX: This line could change to 2, since that is already in the queue.
 		"WARN: filename.mk:5: Variable \"SUBST_STAGE.2\" does not match SUBST class \"1\".",
 		"WARN: filename.mk:6: Variable \"SUBST_FILES.2\" does not match SUBST class \"1\".",
 		"WARN: filename.mk:7: Variable \"SUBST_VARS.2\" does not match SUBST class \"1\".",
 		"WARN: filename.mk:EOF: Missing SUBST block for \"1\".",
 		"WARN: filename.mk:EOF: Missing SUBST block for \"2\".")
 func (s *Suite) Test_SubstContext_varassign__late_addition(c *check.C) {
 	t := s.Init(c)
 	t.RunSubst(
 		"SUBST_CLASSES+=\tid",
 		"SUBST_STAGE.id=\tpost-configure",
 		"SUBST_FILES.id=\tfiles",
 		"SUBST_VARS.id=\tPREFIX",
 		"",
 		".if ${OPSYS} == NetBSD",
 		"SUBST_VARS.id=\tOPSYS",
 		".endif")
 	t.CheckOutputLines(
 		"WARN: filename.mk:7: Late additions to a SUBST variable " +
 			"should use the += operator.")
+}
 func (s *Suite) Test_SubstContext_varassign__late_addition_to_unknown_class(c *check.C) {
 	t := s.Init(c)
 	mklines := t.NewMkLines("filename.mk",
 		"SUBST_VARS.id=\tOPSYS",
 		"")
 	ctx := NewSubstContext()
 	mklines.collectRationale()
 	mklines.ForEach(ctx.Process)
 	t.CheckOutputLines(
 		"WARN: filename.mk:1: Before defining SUBST_VARS.id, " +
 			"the SUBST class should be declared using \"SUBST_CLASSES+= id\".")
+}
-func (s *Suite) Test_SubstContext_varassignClasses__none(c *check.C) {
+func (s *Suite) Test_SubstContext_varassign__rationale(c *check.C) {
 	t := s.Init(c)
 	t.RunSubst(
-		"SUBST_CLASSES+=\t# none")
+		"# Adjust setup.py",
 		"SUBST_CLASSES+=         setup",
 		"SUBST_STAGE.setup=      post-configure",
 		"SUBST_FILES.setup=      setup.py",
 		"SUBST_VARS.setup=       VAR")
 	// The rationale in line 1 is supposed to suppress warnings,
 	// not add new ones.
 	t.CheckOutputEmpty()
+}
-func (s *Suite) Test_SubstContext_varassignClasses__indirect(c *check.C) {
+func (s *Suite) Test_SubstContext_varassign__interleaved(c *check.C) {
 	t := s.Init(c)
-	t.SetUpVartypes()
+	t.RunSubst(
-	mklines := t.NewMkLines("filename.mk",
+		"SUBST_CLASSES+= 1 2 3",
-		MkCvsID,
+		"SUBST_STAGE.1=  post-configure",
-		"SUBST_CLASSES+=\t${VAR}")
+		"SUBST_STAGE.2=  post-configure",
 		"SUBST_STAGE.3=  post-configure",
-	mklines.Check()
+		"SUBST_FILES.1=  setup.py",
 		"SUBST_FILES.2=  setup.py",
 		"SUBST_FILES.3=  setup.py",
 		"SUBST_VARS.1=   VAR",
 		"SUBST_VARS.2=   VAR",
 		"SUBST_VARS.3=   VAR")
 	// The above does not follow the common pattern of defining
 	// each block on its own.
 	// It technically works but is not easy to read for humans.
 	t.CheckOutputLines(
 		"NOTE: filename.mk:1: " +
 			"Please add only one class at a time to SUBST_CLASSES.")
+}
 func (s *Suite) Test_SubstContext_varassignClasses__OPSYSVARS(c *check.C) {
 	t := s.Init(c)
 	ctx := NewSubstContext()
 	// SUBST_CLASSES is added to OPSYSVARS in mk/bsd.pkg.mk.
 	ctx.varassign(t.NewMkLine("filename.mk", 11, "SUBST_CLASSES.SunOS+=prefix"))
 	ctx.varassign(t.NewMkLine("filename.mk", 12, "SUBST_CLASSES.NetBSD+=prefix"))
 	ctx.varassign(t.NewMkLine("filename.mk", 13, "SUBST_FILES.prefix=Makefile"))
 	ctx.varassign(t.NewMkLine("filename.mk", 14, "SUBST_SED.prefix=s,@PREFIX@,${PREFIX},g"))
 	ctx.varassign(t.NewMkLine("filename.mk", 15, "SUBST_STAGE.prefix=post-configure"))
 	t.CheckEquals(ctx.block().isComplete(), true)
 	ctx.Finish(t.NewMkLine("filename.mk", 15, ""))
 	t.CheckOutputLines(
 		"NOTE: filename.mk:14: The substitution command \"s,@PREFIX@,${PREFIX},g\" " +
 			"can be replaced with \"SUBST_VARS.prefix= PREFIX\".")
+}
 func (s *Suite) Test_SubstContext_varassignClasses__duplicate_id(c *check.C) {
 	t := s.Init(c)
 	t.RunSubst(
 		"SUBST_CLASSES+= id",
 		"SUBST_CLASSES+= id")
 	t.CheckOutputLines(
 		"ERROR: filename.mk:2: Duplicate SUBST class \"id\".",
 		"WARN: filename.mk:EOF: Missing SUBST block for \"id\".")
+}
 func (s *Suite) Test_SubstContext_varassignClasses__multiple_classes_in_one_line(c *check.C) {
 	t := s.Init(c)
 	t.RunSubst(
 		"SUBST_CLASSES+=         one two",
 		"SUBST_STAGE.one=        post-configure",
 		"SUBST_FILES.one=        one.txt",
 		"SUBST_SED.one=          s,one,1,g",
 		"SUBST_STAGE.two=        post-configure",
 		"SUBST_FILES.two=        two.txt")
 	t.CheckOutputLines(
 		"NOTE: filename.mk:1: Please add only one class at a time to SUBST_CLASSES.",
 		"WARN: filename.mk:EOF: Incomplete SUBST block: SUBST_SED.two, SUBST_VARS.two or SUBST_FILTER_CMD.two missing.")
+}
 func (s *Suite) Test_SubstContext_varassignClasses__none(c *check.C) {
 	t := s.Init(c)
 	t.RunSubst(
 		"SUBST_CLASSES+=\t# none")
 	t.CheckOutputEmpty()
+}
 func (s *Suite) Test_SubstContext_varassignClasses__indirect(c *check.C) {
 	t := s.Init(c)
 	t.SetUpVartypes()
 	mklines := t.NewMkLines("filename.mk",
 		MkCvsID,
 		"SUBST_CLASSES+=\t${VAR}")
 	mklines.Check()
 	t.CheckOutputLines(
 		"ERROR: filename.mk:2: Identifiers for SUBST_CLASSES "+
 			"must not refer to other variables.",
 		"WARN: filename.mk:2: VAR is used but not defined.")
+}
 func (s *Suite) Test_SubstContext_varassignOutsideBlock__assign(c *check.C) {
 	t := s.Init(c)
 	t.RunSubst(
 		"SUBST_CLASSES+=\t1",
 		"SUBST_STAGE.1=\tpre-configure",
 		"SUBST_FILES.1=\tfile1",
 		"SUBST_SED.1=\t-e s,subst1,repl1,",
 		"SUBST_CLASSES+=\t2",
 		"SUBST_SED.1=\t-e s,subst1b,repl1b,", // Misplaced
 		"SUBST_STAGE.2=\tpre-configure",
 		"SUBST_FILES.2=\tfile2",
 		"SUBST_SED.2=\t-e s,subst2,repl2,")
 	t.CheckOutputLines(
 		"WARN: filename.mk:6: Late additions to a SUBST variable " +
 			"should use the += operator.")
+}
 func (s *Suite) Test_SubstContext_varassignOutsideBlock__append(c *check.C) {
 	t := s.Init(c)
 	t.RunSubst(
 		"SUBST_CLASSES+=\t1",
 		"SUBST_STAGE.1=\tpre-configure",
 		"SUBST_FILES.1=\tfile1",
 		"SUBST_SED.1=\t-e s,subst1,repl1,",
 		"SUBST_CLASSES+=\t2",
 		"SUBST_SED.1+=\t-e s,subst1b,repl1b,", // Misplaced
 		"SUBST_STAGE.2=\tpre-configure",
 		"SUBST_FILES.2=\tfile2",
 		"SUBST_SED.2=\t-e s,subst2,repl2,")
 	// The SUBST_SED.1 line is misplaced. It uses the += operator,
 	// which makes it unclear whether this is a typo or not.
 	t.CheckOutputEmpty()
+}
 // The rationale for the stray SUBST variables has to be specific.
 //
 // For example, in the following snippet from mail/dkim-milter/options.mk
 // revision 1.9, there is a comment, but that is not a rationale and also
 // not related to the SUBST_CLASS variable at all:
 //  ### IPv6 support.
 //  .if !empty(PKG_OPTIONS:Minet6)
 //  SUBST_SED.libs+=        -e 's|@INET6@||g'
 //  .endif
 func (s *Suite) Test_SubstContext_varassignOutsideBlock__rationale(c *check.C) {
 	t := s.Init(c)
 	mklines := t.NewMkLines("filename.mk",
 @@ -450,108 +388,540 @@ func (s *Suite) Test_SubstContext_varass
 		"# The two class is defined somewhere else.",
 		"SUBST_VARS.two=\tOPSYS",
 		"",
 		// The word "defined" doesn't match the subst class "def".
 		"# This subst class is defined somewhere else.",
 		"SUBST_VARS.def=\tOPSYS",
 		"",
 		"# Rationale that is completely irrelevant.",
 		"SUBST_SED.libs+=\t-e sahara",
 		"")
 	ctx := NewSubstContext()
 	mklines.collectRationale()
 	mklines.ForEach(ctx.Process)
 	t.CheckOutputLines(
 		"WARN: filename.mk:2: Before defining SUBST_VARS.one, "+
 			"the SUBST class should be declared using \"SUBST_CLASSES+= one\".",
 		// In filename.mk:5 there is a proper rationale, thus no warning.
 		"WARN: filename.mk:8: Before defining SUBST_VARS.def, "+
 			"the SUBST class should be declared using \"SUBST_CLASSES+= def\".",
 		"WARN: filename.mk:11: Before defining SUBST_SED.libs, "+
 			"the SUBST class should be declared using \"SUBST_CLASSES+= libs\".")
+}
 // Unbalanced conditionals must not lead to a panic.
 func (s *Suite) Test_SubstContext_directive__before_SUBST_CLASSES(c *check.C) {
 	t := s.Init(c)
 	t.RunSubst(
 		".if 0",
 		".endif",
 		"SUBST_CLASSES+=\tos",
 		".elif 0")
 	t.CheckOutputLines(
 		"WARN: filename.mk:4: Missing SUBST block for \"os\".")
+}
 func (s *Suite) Test_SubstContext_directive__conditional_blocks_complete(c *check.C) {
 	t := s.Init(c)
 	t.RunSubst(
 		".if ${OPSYS} == NetBSD",
 		"SUBST_CLASSES+= nb",
 		"SUBST_STAGE.nb= post-configure",
 		"SUBST_FILES.nb= guess-netbsd.h",
 		"SUBST_VARS.nb=  HAVE_NETBSD",
 		".else",
 		"SUBST_CLASSES+= os",
 		"SUBST_STAGE.os= post-configure",
 		"SUBST_FILES.os= guess-netbsd.h",
 		"SUBST_VARS.os=  HAVE_OTHER",
 		".endif")
 	t.CheckOutputEmpty()
+}
 func (s *Suite) Test_SubstContext_directive__conditional_blocks_incomplete(c *check.C) {
 	t := s.Init(c)
 	t.RunSubst(
 		".if ${OPSYS} == NetBSD",
 		"SUBST_CLASSES+= nb",
 		"SUBST_STAGE.nb= post-configure",
 		"SUBST_VARS.nb=  HAVE_NETBSD",
 		".else",
 		"SUBST_CLASSES+= os",
 		"SUBST_STAGE.os= post-configure",
 		"SUBST_FILES.os= guess-netbsd.h",
 		".endif")
 	t.CheckOutputLines(
 		"WARN: filename.mk:5: Incomplete SUBST block: SUBST_FILES.nb missing.",
 		"WARN: filename.mk:6: Subst block \"nb\" should be finished "+
 			"before adding the next class to SUBST_CLASSES.",
 		"WARN: filename.mk:9: Incomplete SUBST block: "+
 			"SUBST_SED.os, SUBST_VARS.os or SUBST_FILTER_CMD.os missing.")
+}
 func (s *Suite) Test_SubstContext_directive__conditional_complete(c *check.C) {
 	t := s.Init(c)
 	t.RunSubst(
 		"SUBST_CLASSES+= id",
 		".if ${OPSYS} == NetBSD",
 		"SUBST_STAGE.id=\t\tpost-configure",
 		"SUBST_MESSAGE.id=\tpost-configure",
 		"SUBST_FILES.id=\t\tguess-netbsd.h",
 		"SUBST_SED.id=\t\t-e s,from,to,",
 		"SUBST_VARS.id=\t\tHAVE_OTHER",
 		"SUBST_FILTER_CMD.id=\tHAVE_OTHER",
 		".else",
 		"SUBST_STAGE.id=\t\tpost-configure",
 		"SUBST_MESSAGE.id=\tpost-configure",
 		"SUBST_FILES.id=\t\tguess-netbsd.h",
 		"SUBST_SED.id=\t\t-e s,from,to,",
 		"SUBST_VARS.id=\t\tHAVE_OTHER",
 		"SUBST_FILTER_CMD.id=\tHAVE_OTHER",
 		".endif")
 	t.CheckOutputLines(
 		"WARN: filename.mk:3: SUBST_STAGE.id should not be defined conditionally.",
 		"WARN: filename.mk:4: SUBST_MESSAGE.id should not be defined conditionally.",
 		"WARN: filename.mk:10: SUBST_STAGE.id should not be defined conditionally.",
 		"WARN: filename.mk:11: SUBST_MESSAGE.id should not be defined conditionally.")
+}
 func (s *Suite) Test_SubstContext_directive__conditionally_overwritten_filter(c *check.C) {
 	t := s.Init(c)
 	t.RunSubst(
 		"SUBST_CLASSES+= id",
 		"SUBST_STAGE.id=\t\tpost-configure",
 		"SUBST_MESSAGE.id=\tpost-configure",
 		"SUBST_FILES.id=\t\tguess-netbsd.h",
 		"SUBST_FILTER_CMD.id=\tHAVE_OTHER",
 		".if ${OPSYS} == NetBSD",
 		"SUBST_FILTER_CMD.id=\tHAVE_OTHER",
 		".endif")
 	t.CheckOutputLines(
 		"WARN: filename.mk:7: Duplicate definition of \"SUBST_FILTER_CMD.id\".")
+}
 // Hopefully nobody will ever trigger this case in real pkgsrc.
 // It's plain confusing to a casual reader to nest a complete
 // SUBST block into another SUBST block.
 func (s *Suite) Test_SubstContext_directive__conditionally_nested_block(c *check.C) {
 	t := s.Init(c)
 	t.RunSubst(
 		"SUBST_CLASSES+=         outer",
 		"SUBST_STAGE.outer=      post-configure",
 		"SUBST_FILES.outer=      outer.txt",
 		".if ${OPSYS} == NetBSD",
 		"SUBST_CLASSES+=         inner",
 		"SUBST_STAGE.inner=      post-configure",
 		"SUBST_FILES.inner=      inner.txt",
 		"SUBST_VARS.inner=       INNER",
 		".endif",
 		"SUBST_VARS.outer=       OUTER")
 	t.CheckOutputLines(
 		"WARN: filename.mk:5: Subst block \"outer\" should be finished " +
 			"before adding the next class to SUBST_CLASSES.")
+}
 // It's completely valid to have several SUBST blocks in a single paragraph.
 // As soon as a SUBST_CLASSES line appears, pkglint assumes that all previous
 // SUBST blocks are finished. That's exactly the case here.
 func (s *Suite) Test_SubstContext_directive__conditionally_following_block(c *check.C) {
 	t := s.Init(c)
 	t.RunSubst(
 		"SUBST_CLASSES+=         outer",
 		"SUBST_STAGE.outer=      post-configure",
 		"SUBST_FILES.outer=      outer.txt",
 		"SUBST_VARS.outer=       OUTER",
 		".if ${OPSYS} == NetBSD",
 		"SUBST_CLASSES+=         middle",
 		"SUBST_STAGE.middle=     post-configure",
 		"SUBST_FILES.middle=     inner.txt",
 		"SUBST_VARS.middle=      INNER",
 		".  if ${MACHINE_ARCH} == amd64",
 		"SUBST_CLASSES+=         inner",
 		"SUBST_STAGE.inner=      post-configure",
 		"SUBST_FILES.inner=      inner.txt",
 		"SUBST_VARS.inner=       INNER",
 		".  endif",
 		".endif")
 	t.CheckOutputEmpty()
+}
 func (s *Suite) Test_SubstContext_directive__two_blocks_in_condition(c *check.C) {
 	t := s.Init(c)
 	t.RunSubst(
 		".if ${OPSYS} == NetBSD",
 		"SUBST_CLASSES+= a",
 		"SUBST_STAGE.a=  post-configure",
 		"SUBST_FILES.a=  outer.txt",
 		"SUBST_VARS.a=   OUTER",
 		"SUBST_CLASSES+= b",
 		"SUBST_STAGE.b=  post-configure",
 		"SUBST_FILES.b=  inner.txt",
 		"SUBST_VARS.b=   INNER",
 		".endif")
 	// Up to 2019-12-12, pkglint wrongly warned in filename.mk:6:
 	//  Subst block "a" should be finished before adding
 	//  the next class to SUBST_CLASSES.
 	// The warning was wrong since block "a" has all required fields set.
 	// The warning was caused by an inconsistent check whether the current
 	// block had any conditional variables.
 	t.CheckOutputEmpty()
+}
 func (s *Suite) Test_SubstContext_directive__nested_conditional_incomplete_block(c *check.C) {
 	t := s.Init(c)
 	t.RunSubst(
 		"SUBST_CLASSES+=         outer",
 		"SUBST_STAGE.outer=      post-configure",
 		"SUBST_FILES.outer=      outer.txt",
 		"SUBST_VARS.outer=       OUTER",
 		".if ${OPSYS} == NetBSD",
 		"SUBST_CLASSES+=         inner1",
 		"SUBST_STAGE.inner1=     post-configure",
 		"SUBST_VARS.inner1=      INNER",
 		"SUBST_CLASSES+=         inner2",
 		"SUBST_STAGE.inner2=     post-configure",
 		"SUBST_FILES.inner2=     inner.txt",
 		"SUBST_VARS.inner2=      INNER",
 		".endif")
 	t.CheckOutputLines(
 		"WARN: filename.mk:9: Subst block \"inner1\" should be finished "+
 			"before adding the next class to SUBST_CLASSES.",
 		"WARN: filename.mk:9: Incomplete SUBST block: SUBST_FILES.inner1 missing.")
+}
 func (s *Suite) Test_SubstContext_leave__details_in_then_branch(c *check.C) {
 	t := s.Init(c)
 	t.RunSubst(
 		"SUBST_CLASSES+=         os",
 		".if ${OPSYS} == NetBSD",
 		"SUBST_VARS.os=          OPSYS",
 		"SUBST_SED.os=           -e s,@OPSYS@,NetBSD,",
 		"SUBST_STAGE.os=         post-configure",
 		"SUBST_MESSAGE.os=       Guessing operating system",
 		"SUBST_FILES.os=         guess-os.h",
 		".endif")
 	t.CheckOutputLines(
 		"WARN: filename.mk:5: SUBST_STAGE.os should not be defined conditionally.",
 		"WARN: filename.mk:6: SUBST_MESSAGE.os should not be defined conditionally.",
 		"WARN: filename.mk:EOF: Incomplete SUBST block: SUBST_STAGE.os missing.",
 		"WARN: filename.mk:EOF: Incomplete SUBST block: SUBST_FILES.os missing.",
 		"WARN: filename.mk:EOF: Incomplete SUBST block: "+
 			"SUBST_SED.os, SUBST_VARS.os or SUBST_FILTER_CMD.os missing.")
+}
 func (s *Suite) Test_SubstContext_leave__details_in_else_branch(c *check.C) {
 	t := s.Init(c)
 	t.RunSubst(
 		"SUBST_CLASSES+=         os",
 		".if ${OPSYS} == NetBSD",
 		".else",
 		"SUBST_VARS.os=          OPSYS",
 		"SUBST_SED.os=           -e s,@OPSYS@,NetBSD,",
 		"SUBST_STAGE.os=         post-configure",
 		"SUBST_MESSAGE.os=       Guessing operating system",
 		"SUBST_FILES.os=         guess-os.h",
 		".endif")
 	t.CheckOutputLines(
 		"WARN: filename.mk:6: SUBST_STAGE.os should not be defined conditionally.",
 		"WARN: filename.mk:7: SUBST_MESSAGE.os should not be defined conditionally.",
 		"WARN: filename.mk:EOF: Incomplete SUBST block: SUBST_STAGE.os missing.",
 		"WARN: filename.mk:EOF: Incomplete SUBST block: SUBST_FILES.os missing.",
 		"WARN: filename.mk:EOF: Incomplete SUBST block: "+
 			"SUBST_SED.os, SUBST_VARS.os or SUBST_FILTER_CMD.os missing.")
+}
 func (s *Suite) Test_SubstContext_leave__empty_conditional_at_end(c *check.C) {
 	t := s.Init(c)
 	t.RunSubst(
 		"SUBST_CLASSES+=         os",
 		"SUBST_VARS.os=          OPSYS",
 		"SUBST_SED.os=           -e s,@OPSYS@,NetBSD,",
 		"SUBST_STAGE.os=         post-configure",
 		"SUBST_MESSAGE.os=       Guessing operating system",
 		"SUBST_FILES.os=         guess-os.h",
 		".if ${OPSYS} == NetBSD",
 		".else",
 		".endif")
 	t.CheckOutputEmpty()
+}
 func (s *Suite) Test_SubstContext_leave__missing_transformation_in_one_branch(c *check.C) {
 	t := s.Init(c)
 	t.RunSubst(
 		"SUBST_CLASSES+=         os",
 		"SUBST_STAGE.os=         post-configure",
 		"SUBST_MESSAGE.os=       Guessing operating system",
 		"SUBST_FILES.os=         guess-os.h",
 		".if ${OPSYS} == NetBSD",
 		"SUBST_FILES.os=         -e s,@OpSYS@,NetBSD,", // A simple typo, this should be SUBST_SED.
 		".elif ${OPSYS} == Darwin",
 		"SUBST_SED.os=           -e s,@OPSYS@,Darwin1,",
 		"SUBST_SED.os=           -e s,@OPSYS@,Darwin2,",
 		".else",
 		"SUBST_VARS.os=           OPSYS",
 		".endif")
 	t.CheckOutputLines(
 		"WARN: filename.mk:6: All but the first assignment "+
 			"to \"SUBST_FILES.os\" should use the \"+=\" operator.",
 		"WARN: filename.mk:9: All but the first assignment "+
 			"to \"SUBST_SED.os\" should use the \"+=\" operator.",
 		"WARN: filename.mk:EOF: Incomplete SUBST block: SUBST_SED.os, "+
 			"SUBST_VARS.os or SUBST_FILTER_CMD.os missing.")
+}
 func (s *Suite) Test_SubstContext_leave__nested_conditionals(c *check.C) {
 	t := s.Init(c)
 	t.RunSubst(
 		"SUBST_CLASSES+=         os",
 		"SUBST_STAGE.os=         post-configure",
 		"SUBST_MESSAGE.os=       Guessing operating system",
 		".if ${OPSYS} == NetBSD",
 		"SUBST_FILES.os=         guess-netbsd.h",
 		".  if ${ARCH} == i386",
 		"SUBST_FILTER_CMD.os=    ${SED} -e s,@OPSYS,NetBSD-i386,",
 		".  elif ${ARCH} == x86_64",
 		"SUBST_VARS.os=          OPSYS",
 		".  else",
 		"SUBST_SED.os=           -e s,@OPSYS,NetBSD-unknown",
 		".  endif",
 		".else",
 		// This branch omits SUBST_FILES.
 		"SUBST_SED.os=           -e s,@OPSYS@,unknown,",
 		".endif")
 	t.CheckOutputLines(
 		"WARN: filename.mk:EOF: Incomplete SUBST block: SUBST_FILES.os missing.")
+}
 // With every .if directive, a new scope is created, to properly
 // keep track of the conditional level at which the SUBST classes
 // are declared.
 //
 // The scopes are even created when there is no SUBST variable
 // anywhere close. The conditionals must be tracked for determining
 // the end of the scope for the SUBST_CLASSES IDs.
 func (s *Suite) Test_substScope__conditionals(c *check.C) {
 	t := s.Init(c)
 	ctx := NewSubstContext()
 	line := func(text string) {
 		mkline := t.NewMkLine("filename.mk", 123, text)
 		ctx.Process(mkline)
+	}
 	verifyScopes := func(n int) {
 		t.CheckEquals(len(ctx.scopes), n)
+	}
 	verifyScopes(1)
 	line(".if 1")
 	verifyScopes(2)
 	line(".  if 1")
 	verifyScopes(3)
 	line(".    if 1")
 	verifyScopes(4)
 	line(".    elif 1")
 	verifyScopes(4)
 	line(".    else")
 	verifyScopes(4)
 	line(".    endif")
 	verifyScopes(3)
 	line(".  endif")
 	verifyScopes(2)
 	line(".endif")
 	verifyScopes(1)
 	// An unbalanced .endif must not lead to a panic.
 	line(".endif")
 	verifyScopes(1)
 	ctx.Finish(NewLineEOF("filename.mk"))
+}
 func (s *Suite) Test_substScope_prepareSubstClasses(c *check.C) {
 	t := s.Init(c)
 	t.RunSubst(
 		"SUBST_CLASSES+= 1",
 		"SUBST_STAGE.1=  post-configure",
 		".if 0")
 	// There's no need to warn about unbalanced conditionals
 	// since that is already done by MkLines.Check.
 	t.CheckOutputLines(
-		"WARN: filename.mk:2: Before defining SUBST_VARS.one, "+
+		"WARN: filename.mk:EOF: Incomplete SUBST block: SUBST_FILES.1 missing.",
-			"the SUBST class should be declared using \"SUBST_CLASSES+= one\".",
+		"WARN: filename.mk:EOF: Incomplete SUBST block: "+
-		// In filename.mk:5 there is a proper rationale, thus no warning.
+			"SUBST_SED.1, SUBST_VARS.1 or SUBST_FILTER_CMD.1 missing.")
 		"WARN: filename.mk:8: Before defining SUBST_VARS.def, "+
 			"the SUBST class should be declared using \"SUBST_CLASSES+= def\".",
-		"WARN: filename.mk:11: Before defining SUBST_SED.libs, "+
+func (s *Suite) Test_substScope_prepareSubstClasses__nested(c *check.C) {
-			"the SUBST class should be declared using \"SUBST_CLASSES+= libs\".")
+	t := s.Init(c)
 	t.RunSubst(
 		"SUBST_CLASSES+= 1",
 		"SUBST_STAGE.1=  post-configure",
 		".if 0",
 		".if 0",
 		"SUBST_CLASSES+= 2")
 	t.CheckOutputLines(
 		"WARN: filename.mk:5: Subst block \"1\" should be finished "+
 			"before adding the next class to SUBST_CLASSES.",
 		"WARN: filename.mk:EOF: Missing SUBST block for \"2\".",
 		"WARN: filename.mk:EOF: Incomplete SUBST block: SUBST_FILES.1 missing.",
 		"WARN: filename.mk:EOF: Incomplete SUBST block: "+
 			"SUBST_SED.1, SUBST_VARS.1 or SUBST_FILTER_CMD.1 missing.")
+}
 func (s *Suite) Test_substBlock_varassignStage__do_patch(c *check.C) {
 	t := s.Init(c)
 	t.RunSubst(
 		"SUBST_CLASSES+=\tos",
 		"SUBST_STAGE.os=\tdo-patch",
 		"SUBST_FILES.os=\tguess-os.h",
 		"SUBST_SED.os=\t-e s,@OPSYS@,Darwin,")
 	// No warning, since there is nothing to fix automatically.
 	// This case doesn't occur in practice anyway.
 	t.CheckOutputEmpty()
+}
-func (s *Suite) Test_SubstContext_varassignStage__pre_patch(c *check.C) {
+func (s *Suite) Test_substBlock_varassignStage__pre_patch(c *check.C) {
 	t := s.Init(c)
 	t.Chdir(".")
 	doTest := t.NewSubstAutofixTest(
 		"SUBST_CLASSES+=\tos",
 		"SUBST_STAGE.os=\tpre-patch",
 		"SUBST_FILES.os=\tguess-os.h",
 		"SUBST_SED.os=\t-e s,@OPSYS@,Darwin,")
 	t.ExpectDiagnosticsAutofix(
 		doTest,
 		"WARN: filename.mk:2: Substitutions should not happen in the patch phase.",
 		"AUTOFIX: filename.mk:2: Replacing \"pre-patch\" with \"post-extract\".")
 	t.CheckFileLinesDetab("filename.mk",
 		"SUBST_CLASSES+= os",
 		"SUBST_STAGE.os= post-extract",
 		"SUBST_FILES.os= guess-os.h",
 		"SUBST_SED.os=   -e s,@OPSYS@,Darwin,")
+}
-func (s *Suite) Test_SubstContext_varassignStage__post_patch(c *check.C) {
+func (s *Suite) Test_substBlock_varassignStage__post_patch(c *check.C) {
 	t := s.Init(c)
 	t.Chdir(".")
 	doTest := t.NewSubstAutofixTest(
 		"SUBST_CLASSES+=\tos",
 		"SUBST_STAGE.os=\tpost-patch",
 		"SUBST_FILES.os=\tguess-os.h",
 		"SUBST_SED.os=\t-e s,@OPSYS@,Darwin,")
 	t.ExpectDiagnosticsAutofix(
 		doTest,
 		"WARN: filename.mk:2: Substitutions should not happen in the patch phase.",
 		"AUTOFIX: filename.mk:2: Replacing \"post-patch\" with \"pre-configure\".")
 	t.CheckFileLinesDetab("filename.mk",
 		"SUBST_CLASSES+= os",
 		"SUBST_STAGE.os= pre-configure",
 		"SUBST_FILES.os= guess-os.h",
 		"SUBST_SED.os=   -e s,@OPSYS@,Darwin,")
+}
 func (s *Suite) Test_substBlock_varassignStage__empty_class(c *check.C) {
 	t := s.Init(c)
 	t.Chdir(".")
 	t.RunSubst(
 		"SUBST_CLASSES+= id",
 		"",
 		"SUBST_STAGE.=   post-patch",
 		"SUBST_STAGE.id= post-configure",
 		"SUBST_FILES.id= files",
 		"SUBST_VARS.id=  VAR",
 		"SUBST_VARS.=    VAR")
 	t.CheckOutputLines(
 		"ERROR: filename.mk:3: Invalid SUBST class \"\" in variable name.",
 		"ERROR: filename.mk:7: Invalid SUBST class \"\" in variable name.")
+}
 // As of December 2019, pkglint does not use token positions internally.
 // Instead it only does simple string replacement when autofixing things.
 // To avoid damaging anything, replacements are only done if they are
 // unambiguous. This is not the case here, since line 4 contains the
 // string "pre-patch" twice.
-func (s *Suite) Test_SubstContext_varassignStage__ambiguous_replacement(c *check.C) {
+func (s *Suite) Test_substBlock_varassignStage__ambiguous_replacement(c *check.C) {
 	t := s.Init(c)
 	t.Chdir(".")
 	doTest := t.NewSubstAutofixTest(
 		"SUBST_CLASSES+=         pre-patch",
 		"SUBST_STAGE.pre-patch=  pre-patch",
 		"SUBST_FILES.pre-patch=  files",
 		"SUBST_VARS.pre-patch=   VARNAME")
 	t.ExpectDiagnosticsAutofix(
 		doTest,
 		"WARN: filename.mk:2: Substitutions should not happen in the patch phase.")
 	t.CheckEquals(t.File("filename.mk").IsFile(), false)
+}
-func (s *Suite) Test_SubstContext_varassignStage__with_NO_CONFIGURE(c *check.C) {
+func (s *Suite) Test_substBlock_varassignStage__with_NO_CONFIGURE(c *check.C) {
 	t := s.Init(c)
 	pkg := t.SetUpPackage("category/package",
 		"SUBST_CLASSES+=\t\tpre",
 		"SUBST_STAGE.pre=\tpre-configure",
 		"SUBST_FILES.pre=\tguess-os.h",
 		"SUBST_SED.pre=\t\t-e s,@OPSYS@,Darwin,",
 		"",
 		"SUBST_CLASSES+=\t\tpost",
 		"SUBST_STAGE.post=\tpost-configure",
 		"SUBST_FILES.post=\tguess-os.h",
 		"SUBST_SED.post=\t\t-e s,@OPSYS@,Darwin,",
 		"",
 @@ -562,83 +932,79 @@ func (s *Suite) Test_SubstContext_varass
 		"",
 		"NO_CONFIGURE=\tyes")
 	t.FinishSetUp()
 	G.Check(pkg)
 	t.CheckOutputLines(
 		"WARN: ~/category/package/Makefile:21: SUBST_STAGE pre-configure has no effect "+
 			"when NO_CONFIGURE is set (in line 35).",
 		"WARN: ~/category/package/Makefile:26: SUBST_STAGE post-configure has no effect "+
 			"when NO_CONFIGURE is set (in line 35).")
+}
-func (s *Suite) Test_SubstContext_varassignStage__without_NO_CONFIGURE(c *check.C) {
+func (s *Suite) Test_substBlock_varassignStage__without_NO_CONFIGURE(c *check.C) {
 	t := s.Init(c)
 	pkg := t.SetUpPackage("category/package",
 		"SUBST_CLASSES+=\t\tpre",
 		"SUBST_STAGE.pre=\tpre-configure",
 		"SUBST_FILES.pre=\tguess-os.h",
 		"SUBST_SED.pre=\t\t-e s,@OPSYS@,Darwin,")
 	t.FinishSetUp()
 	G.Check(pkg)
 	t.CheckOutputEmpty()
+}
 // Before 2019-12-12, pkglint wrongly warned about variables that were
 // not obviously SUBST variables, even if they were used later in SUBST_VARS.
-func (s *Suite) Test_SubstContext_varassignVars__var_before_SUBST_VARS(c *check.C) {
+func (s *Suite) Test_substBlock_varassignVars__var_before_SUBST_VARS(c *check.C) {
 	t := s.Init(c)
 	t.RunSubst(
 		"SUBST_CLASSES+= id",
 		"SUBST_STAGE.id= post-configure",
 		"SUBST_FILES.id= files",
 		"FOREIGN=        not mentioned in SUBST_VARS",
 		"VAR=            ok",
 		"SUBST_VARS.id=  VAR",
 		"",
 		// This second block makes sure that the list of foreign variables
 		// is properly reset at the end of a SUBST block.
 		// If it weren't, there would be additional warnings.
 		"SUBST_CLASSES+= 2",
 		"SUBST_STAGE.2=  post-configure",
 		"SUBST_FILES.2=  files",
 		"SUBST_VARS.2=   OTHER")
 	t.CheckOutputLines(
 		"WARN: filename.mk:4: Foreign variable \"FOREIGN\" in SUBST block.")
+}
-func (s *Suite) Test_SubstContext_dupList__conditional_before_unconditional(c *check.C) {
+func (s *Suite) Test_substBlock_varassignVars(c *check.C) {
 	t := s.Init(c)
 	t.RunSubst(
-		"SUBST_CLASSES+= os",
+		"SUBST_CLASSES+=\tos",
-		"SUBST_STAGE.os= post-configure",
+		"SUBST_STAGE.os=\tpre-configure",
-		".if 1",
+		"SUBST_FILES.os=\tguess-os.h",
-		"SUBST_FILES.os= conditional",
+		"SUBST_VARS.os=\tPREFIX VARBASE")
 		".endif",
 		"SUBST_FILES.os= unconditional",
 		"SUBST_VARS.os=  OPSYS")
 	// TODO: Warn that the conditional line is overwritten.
 	t.CheckOutputEmpty()
+}
-func (s *Suite) Test_SubstContext_suggestSubstVars(c *check.C) {
+func (s *Suite) Test_substBlock_suggestSubstVars(c *check.C) {
 	t := s.Init(c)
 	t.Chdir(".")
 	test := func(line string, diagnostics ...string) {
 		doTest := t.NewSubstAutofixTest(
 			"SUBST_CLASSES+=\t\ttest",
 			"SUBST_STAGE.test=\tpre-configure",
 			"SUBST_FILES.test=\tfilename",
 			line)
 		t.ExpectDiagnosticsAutofix(
 			doTest,
 @@ -742,27 +1108,27 @@ func (s *Suite) Test_SubstContext_sugges
 		"NOTE: filename.mk:4: The substitution command \"s,@SH@,${SH:Q},\" "+
 			"can be replaced with \"SUBST_VARS.test= SH\".")
 	// Just a note; not fixed because of the -n.
 	test(
 		"SUBST_SED.test+=\t-n s,@SH@,${SH:Q},",
 		"NOTE: filename.mk:4: The substitution command \"s,@SH@,${SH:Q},\" "+
 			"can be replaced with \"SUBST_VARS.test= SH\".")
+}
 // If the SUBST_CLASS identifier ends with a plus, the generated code must
 // use the correct assignment operator and be nicely formatted.
-func (s *Suite) Test_SubstContext_suggestSubstVars__plus(c *check.C) {
+func (s *Suite) Test_substBlock_suggestSubstVars__plus(c *check.C) {
 	t := s.Init(c)
 	t.Chdir(".")
 	doTest := t.NewSubstAutofixTest(
 		"SUBST_CLASSES+=\t\tgtk+",
 		"SUBST_STAGE.gtk+ =\tpre-configure",
 		"SUBST_FILES.gtk+ =\tfilename",
 		"SUBST_SED.gtk+ +=\t-e s,@SH@,${SH:Q},g",
 		"SUBST_SED.gtk+ +=\t-e s,@SH@,${SH:Q},g")
 	t.ExpectDiagnosticsAutofix(
 		doTest,
 @@ -776,27 +1142,27 @@ func (s *Suite) Test_SubstContext_sugges
 			"with \"SUBST_VARS.gtk+ +=\\tSH\".")
 	t.CheckFileLinesDetab("filename.mk",
 		"SUBST_CLASSES+=         gtk+",
 		"SUBST_STAGE.gtk+ =      pre-configure",
 		"SUBST_FILES.gtk+ =      filename",
 		"SUBST_VARS.gtk+ =       SH",
 		"SUBST_VARS.gtk+ +=      SH")
+}
 // The last of the SUBST_SED variables is 15 characters wide. When SUBST_SED
 // is replaced with SUBST_VARS, this becomes 16 characters and therefore
 // requires the whole paragraph to be indented by one more tab.
-func (s *Suite) Test_SubstContext_suggestSubstVars__autofix_realign_paragraph(c *check.C) {
+func (s *Suite) Test_substBlock_suggestSubstVars__autofix_realign_paragraph(c *check.C) {
 	t := s.Init(c)
 	t.Chdir(".")
 	doTest := t.NewSubstAutofixTest(
 		"SUBST_CLASSES+=\t\tpfx",
 		"SUBST_STAGE.pfx=\tpre-configure",
 		"SUBST_FILES.pfx=\tfilename",
 		"SUBST_SED.pfx=\t\t-e s,@PREFIX@,${PREFIX},g",
 		"SUBST_SED.pfx+=\t\t-e s,@PREFIX@,${PREFIX},g")
 	t.ExpectDiagnosticsAutofix(
 		doTest,
 @@ -808,27 +1174,27 @@ func (s *Suite) Test_SubstContext_sugges
 		"AUTOFIX: filename.mk:4: Replacing \"SUBST_SED.pfx=\\t\\t-e s,@PREFIX@,${PREFIX},g\" "+
 			"with \"SUBST_VARS.pfx=\\t\\tPREFIX\".",
 		"AUTOFIX: filename.mk:5: Replacing \"SUBST_SED.pfx+=\\t\\t-e s,@PREFIX@,${PREFIX},g\" "+
 			"with \"SUBST_VARS.pfx+=\\tPREFIX\".")
 	t.CheckFileLinesDetab("filename.mk",
 		"SUBST_CLASSES+=         pfx",
 		"SUBST_STAGE.pfx=        pre-configure",
 		"SUBST_FILES.pfx=        filename",
 		"SUBST_VARS.pfx=         PREFIX",
 		"SUBST_VARS.pfx+=        PREFIX")
+}
-func (s *Suite) Test_SubstContext_suggestSubstVars__autofix_plus_sed(c *check.C) {
+func (s *Suite) Test_substBlock_suggestSubstVars__autofix_plus_sed(c *check.C) {
 	t := s.Init(c)
 	t.Chdir(".")
 	doTest := t.NewSubstAutofixTest(
 		"SUBST_CLASSES+=\t\tpfx",
 		"SUBST_STAGE.pfx=\tpre-configure",
 		"SUBST_FILES.pfx=\tfilename",
 		"SUBST_SED.pfx=\t\t-e s,@PREFIX@,${PREFIX},g",
 		"SUBST_SED.pfx+=\t\t-e s,@PREFIX@,other,g")
 	t.ExpectDiagnosticsAutofix(
 		doTest,
 @@ -839,27 +1205,27 @@ func (s *Suite) Test_SubstContext_sugges
 			"Replacing \"SUBST_SED.pfx=\\t\\t-e s,@PREFIX@,${PREFIX},g\" "+
 			"with \"SUBST_VARS.pfx=\\t\\tPREFIX\".")
 	t.CheckFileLinesDetab("filename.mk",
 		"SUBST_CLASSES+=         pfx",
 		"SUBST_STAGE.pfx=        pre-configure",
 		"SUBST_FILES.pfx=        filename",
 		"SUBST_VARS.pfx=         PREFIX",
 		// Since the SUBST_SED that was previously here used the = operator,
 		// this += might be replaced with a simple =.
 		"SUBST_SED.pfx+=         -e s,@PREFIX@,other,g")
+}
-func (s *Suite) Test_SubstContext_suggestSubstVars__autofix_plus_vars(c *check.C) {
+func (s *Suite) Test_substBlock_suggestSubstVars__autofix_plus_vars(c *check.C) {
 	t := s.Init(c)
 	t.Chdir(".")
 	doTest := t.NewSubstAutofixTest(
 		"SUBST_CLASSES+=\tid",
 		"SUBST_STAGE.id=\tpre-configure",
 		"SUBST_FILES.id=\tfilename",
 		"SUBST_SED.id=\t-e s,@PREFIX@,${PREFIX},g",
 		"SUBST_VARS.id=\tPKGMANDIR")
 	t.ExpectDiagnosticsAutofix(
 		doTest,
 @@ -871,27 +1237,27 @@ func (s *Suite) Test_SubstContext_sugges
 			"with \"SUBST_VARS.id=\\tPREFIX\".",
 		"AUTOFIX: filename.mk:5: "+
 			"Replacing \"SUBST_VARS.id=\\t\" "+
 			"with \"SUBST_VARS.id+=\\t\".")
 	t.CheckFileLinesDetab("filename.mk",
 		"SUBST_CLASSES+= id",
 		"SUBST_STAGE.id= pre-configure",
 		"SUBST_FILES.id= filename",
 		"SUBST_VARS.id=  PREFIX",
 		"SUBST_VARS.id+= PKGMANDIR")
+}
-func (s *Suite) Test_SubstContext_suggestSubstVars__autofix_indentation(c *check.C) {
+func (s *Suite) Test_substBlock_suggestSubstVars__autofix_indentation(c *check.C) {
 	t := s.Init(c)
 	t.Chdir(".")
 	doTest := t.NewSubstAutofixTest(
 		"SUBST_CLASSES+=\t\t\tfix-paths",
 		"SUBST_STAGE.fix-paths=\t\tpre-configure",
 		"SUBST_MESSAGE.fix-paths=\tMessage",
 		"SUBST_FILES.fix-paths=\t\tfilename",
 		"SUBST_SED.fix-paths=\t\t-e s,@PREFIX@,${PREFIX},g")
 	t.ExpectDiagnosticsAutofix(
 		doTest,
 @@ -901,454 +1267,286 @@ func (s *Suite) Test_SubstContext_sugges
 			"can be replaced with \"SUBST_VARS.fix-paths= PREFIX\".",
 		"AUTOFIX: filename.mk:5: Replacing "+
 			"\"SUBST_SED.fix-paths=\\t\\t-e s,@PREFIX@,${PREFIX},g\" "+
 			"with \"SUBST_VARS.fix-paths=\\t\\tPREFIX\".")
 	t.CheckFileLinesDetab("filename.mk",
 		"SUBST_CLASSES+=                 fix-paths",
 		"SUBST_STAGE.fix-paths=          pre-configure",
 		"SUBST_MESSAGE.fix-paths=        Message",
 		"SUBST_FILES.fix-paths=          filename",
 		"SUBST_VARS.fix-paths=           PREFIX")
+}
-func (s *Suite) Test_SubstContext_suggestSubstVars__conditional(c *check.C) {
+func (s *Suite) Test_substBlock_suggestSubstVars__conditional(c *check.C) {
 	t := s.Init(c)
 	t.Chdir(".")
 	doTest := t.NewSubstAutofixTest(
 		"SUBST_CLASSES+= id",
 		"SUBST_STAGE.id= pre-configure",
 		"SUBST_FILES.id= files",
 		"SUBST_SED.id=   -e s,@VAR@,${VAR},",
 		".if 1",
 		"SUBST_SED.id+=  -e s,@VAR2@,${VAR2},",
 		".endif")
 	t.ExpectDiagnosticsAutofix(
 		doTest,
 		"NOTE: filename.mk:4: The substitution command \"s,@VAR@,${VAR},\" "+
 			"can be replaced with \"SUBST_VARS.id= VAR\".",
 		"NOTE: filename.mk:6: The substitution command \"s,@VAR2@,${VAR2},\" "+
 			"can be replaced with \"SUBST_VARS.id+= VAR2\".",
 		"AUTOFIX: filename.mk:4: Replacing \"SUBST_SED.id=   -e s,@VAR@,${VAR},\" "+
 			"with \"SUBST_VARS.id=\\tVAR\".",
 		"AUTOFIX: filename.mk:6: Replacing \"SUBST_SED.id+=  -e s,@VAR2@,${VAR2},\" "+
 			"with \"SUBST_VARS.id+=\\tVAR2\".")
 	t.CheckFileLinesDetab("filename.mk",
 		"SUBST_CLASSES+= id",
 		"SUBST_STAGE.id= pre-configure",
 		"SUBST_FILES.id= files",
 		"SUBST_VARS.id=  VAR",
 		".if 1",
 		"SUBST_VARS.id+= VAR2",
 		".endif")
 func (s *Suite) Test_SubstContext_extractVarname(c *check.C) {
 	t := s.Init(c)
 	test := func(input, expected string) {
 		t.CheckEquals((*SubstContext).extractVarname(nil, input), expected)
 	// A simple variable name.
 	test("s,@VAR@,${VAR},", "VAR")
 	// A parameterized variable name.
 	test("s,@VAR.param@,${VAR.param},", "VAR.param")
 	// Only substitution commands can be replaced with SUBST_VARS.
 	test("/pattern/d", "")
 	// An incomplete substitution command.
 	test("s", "")
 	// Wrong placeholder character, only @ works.
 	test("s,!VAR!,${VAR},", "")
 	// The placeholder must have exactly 1 @ on each side.
 	test("s,@@VAR@@,${VAR},", "")
 	// Malformed because the comma is the separator.
 	test("s,@VAR,VAR@,${VAR},", "")
 	// The replacement pattern is not a simple variable name enclosed in @.
 	test("s,@VAR!VAR@,${VAR},", "")
 	// The replacement may only contain the :Q modifier.
 	test("s,@VAR@,${VAR:Mpattern},", "")
 	// The :Q modifier is allowed in the replacement.
 	test("s,@VAR@,${VAR:Q},", "VAR")
 	// The replacement may contain the :Q modifier only once.
 	test("s,@VAR@,${VAR:Q:Q},", "")
 	// The replacement must be a plain variable expression, without prefix.
 	test("s,@VAR@,prefix${VAR},", "")
 	// The replacement must be a plain variable expression, without suffix.
 	test("s,@VAR@,${VAR}suffix,", "")
 func (s *Suite) Test_SubstContext_directive__before_SUBST_CLASSES(c *check.C) {
 	t := s.Init(c)
 	t.RunSubst(
 		".if 0",
 		".endif",
 		"SUBST_CLASSES+=\tos",
 		".elif 0") // Just for branch coverage.
 	t.CheckOutputLines(
 		"WARN: filename.mk:EOF: Missing SUBST block for \"os\".")
 func (s *Suite) Test_SubstContext_directive__conditional_blocks_complete(c *check.C) {
 	t := s.Init(c)
 	t.RunSubst(
 		".if ${OPSYS} == NetBSD",
 		"SUBST_CLASSES+= nb",
 		"SUBST_STAGE.nb= post-configure",
 		"SUBST_FILES.nb= guess-netbsd.h",
 		"SUBST_VARS.nb=  HAVE_NETBSD",
 		".else",
 		"SUBST_CLASSES+= os",
 		"SUBST_STAGE.os= post-configure",
 		"SUBST_FILES.os= guess-netbsd.h",
 		"SUBST_VARS.os=  HAVE_OTHER",
 		".endif")
 	t.CheckOutputEmpty()
 func (s *Suite) Test_SubstContext_directive__conditional_blocks_incomplete(c *check.C) {
 	t := s.Init(c)
 	t.RunSubst(
 		".if ${OPSYS} == NetBSD",
 		"SUBST_CLASSES+= nb",
 		"SUBST_STAGE.nb= post-configure",
 		"SUBST_VARS.nb=  HAVE_NETBSD",
 		".else",
 		"SUBST_CLASSES+= os",
 		"SUBST_STAGE.os= post-configure",
 		"SUBST_FILES.os= guess-netbsd.h",
 		".endif")
 	t.CheckOutputLines(
 		"WARN: filename.mk:5: Incomplete SUBST block: SUBST_FILES.nb missing.",
 		"WARN: filename.mk:9: Incomplete SUBST block: "+
 			"SUBST_SED.os, SUBST_VARS.os or SUBST_FILTER_CMD.os missing.")
 func (s *Suite) Test_SubstContext_directive__conditional_complete(c *check.C) {
 	t := s.Init(c)
 	t.RunSubst(
 		"SUBST_CLASSES+= id",
 		".if ${OPSYS} == NetBSD",
 		"SUBST_STAGE.id=\t\tpost-configure",
 		"SUBST_MESSAGE.id=\tpost-configure",
 		"SUBST_FILES.id=\t\tguess-netbsd.h",
 		"SUBST_SED.id=\t\t-e s,from,to,",
 		"SUBST_VARS.id=\t\tHAVE_OTHER",
 		"SUBST_FILTER_CMD.id=\tHAVE_OTHER",
 		".else",
 		"SUBST_STAGE.id=\t\tpost-configure",
 		"SUBST_MESSAGE.id=\tpost-configure",
 		"SUBST_FILES.id=\t\tguess-netbsd.h",
 		"SUBST_SED.id=\t\t-e s,from,to,",
 		"SUBST_VARS.id=\t\tHAVE_OTHER",
 		"SUBST_FILTER_CMD.id=\tHAVE_OTHER",
 		".endif")
 	t.CheckOutputLines(
 		"WARN: filename.mk:3: SUBST_STAGE.id should not be defined conditionally.",
 		"WARN: filename.mk:4: SUBST_MESSAGE.id should not be defined conditionally.",
 		"WARN: filename.mk:10: SUBST_STAGE.id should not be defined conditionally.",
 		"WARN: filename.mk:11: SUBST_MESSAGE.id should not be defined conditionally.")
 func (s *Suite) Test_SubstContext_directive__conditionally_overwritten_filter(c *check.C) {
 	t := s.Init(c)
 	t.RunSubst(
 		"SUBST_CLASSES+= id",
 		"SUBST_STAGE.id=\t\tpost-configure",
 		"SUBST_MESSAGE.id=\tpost-configure",
 		"SUBST_FILES.id=\t\tguess-netbsd.h",
 		"SUBST_FILTER_CMD.id=\tHAVE_OTHER",
 		".if ${OPSYS} == NetBSD",
 		"SUBST_FILTER_CMD.id=\tHAVE_OTHER",
 		".endif")
 	t.CheckOutputLines(
 		"WARN: filename.mk:7: Duplicate definition of \"SUBST_FILTER_CMD.id\".")
 // Hopefully nobody will ever trigger this case in real pkgsrc.
 // It's plain confusing to a casual reader to nest a complete
 // SUBST block into another SUBST block.
 // That's why pkglint doesn't cover this case correctly.
 func (s *Suite) Test_SubstContext_directive__conditionally_nested_block(c *check.C) {
 	t := s.Init(c)
 	t.RunSubst(
 		"SUBST_CLASSES+=         outer",
 		"SUBST_STAGE.outer=      post-configure",
 		"SUBST_FILES.outer=      outer.txt",
 		".if ${OPSYS} == NetBSD",
 		"SUBST_CLASSES+=         inner",
 		"SUBST_STAGE.inner=      post-configure",
 		"SUBST_FILES.inner=      inner.txt",
 		"SUBST_VARS.inner=       INNER",
 		".endif",
 		"SUBST_VARS.outer=       OUTER")
 	t.CheckOutputLines(
 		"WARN: filename.mk:5: Incomplete SUBST block: "+
 			"SUBST_SED.outer, SUBST_VARS.outer or SUBST_FILTER_CMD.outer missing.",
 		"WARN: filename.mk:5: Subst block \"outer\" should be finished "+
 			"before adding the next class to SUBST_CLASSES.",
 		"WARN: filename.mk:10: "+
 			"Late additions to a SUBST variable should use the += operator.")
 // It's completely valid to have several SUBST blocks in a single paragraph.
 // As soon as a SUBST_CLASSES line appears, pkglint assumes that all previous
 // SUBST blocks are finished. That's exactly the case here.
 func (s *Suite) Test_SubstContext_directive__conditionally_following_block(c *check.C) {
 	t := s.Init(c)
 	t.RunSubst(
 		"SUBST_CLASSES+=         outer",
 		"SUBST_STAGE.outer=      post-configure",
 		"SUBST_FILES.outer=      outer.txt",
 		"SUBST_VARS.outer=       OUTER",
 		".if ${OPSYS} == NetBSD",
 		"SUBST_CLASSES+=         middle",
 		"SUBST_STAGE.middle=     post-configure",
 		"SUBST_FILES.middle=     inner.txt",
 		"SUBST_VARS.middle=      INNER",
 		".  if ${MACHINE_ARCH} == amd64",
 		"SUBST_CLASSES+=         inner",
 		"SUBST_STAGE.inner=      post-configure",
 		"SUBST_FILES.inner=      inner.txt",
 		"SUBST_VARS.inner=       INNER",
 		".  endif",
 		".endif")
 	t.CheckOutputEmpty()
+}
-func (s *Suite) Test_SubstContext_directive__two_blocks_in_condition(c *check.C) {
+func (s *Suite) Test_substBlock_dupList__late_addition(c *check.C) {
 	t := s.Init(c)
 	t.RunSubst(
 		"SUBST_CLASSES+=\tid",
 		"SUBST_STAGE.id=\tpost-configure",
 		"SUBST_FILES.id=\tfiles",
 		"SUBST_VARS.id=\tPREFIX",
 		"",
 		".if ${OPSYS} == NetBSD",
-		"SUBST_CLASSES+= a",
+		"SUBST_VARS.id=\tOPSYS",
 		"SUBST_STAGE.a=  post-configure",
 		"SUBST_FILES.a=  outer.txt",
 		"SUBST_VARS.a=   OUTER",
 		"SUBST_CLASSES+= b",
 		"SUBST_STAGE.b=  post-configure",
 		"SUBST_FILES.b=  inner.txt",
 		"SUBST_VARS.b=   INNER",
 		".endif")
-	// Up to 2019-12-12, pkglint wrongly warned in filename.mk:6:
+	t.CheckOutputLines(
-	//  Subst block "a" should be finished before adding
+		"WARN: filename.mk:7: All but the first assignment " +
-	//  the next class to SUBST_CLASSES.
+			"to \"SUBST_VARS.id\" should use the \"+=\" operator.")
 	// The warning was wrong since block "a" has all required fields set.
 	// The warning was caused by an inconsistent check whether the current
 	// block had any conditional variables.
 	t.CheckOutputEmpty()
+}
-func (s *Suite) Test_SubstContext_directive__nested_conditional_incomplete_block(c *check.C) {
+func (s *Suite) Test_substBlock_dupList__conditional_before_unconditional(c *check.C) {
 	t := s.Init(c)
 	t.RunSubst(
-		"SUBST_CLASSES+=         outer",
+		"SUBST_CLASSES+= os",
-		"SUBST_STAGE.outer=      post-configure",
+		"SUBST_STAGE.os= post-configure",
-		"SUBST_FILES.outer=      outer.txt",
+		".if 1",
-		"SUBST_VARS.outer=       OUTER",
+		"SUBST_FILES.os= conditional",
-		".if ${OPSYS} == NetBSD",
+		".endif",
-		"SUBST_CLASSES+=         inner1",
+		"SUBST_FILES.os= unconditional",
-		"SUBST_STAGE.inner1=     post-configure",
+		"SUBST_VARS.os=  OPSYS")
 		"SUBST_VARS.inner1=      INNER",
 		"SUBST_CLASSES+=         inner2",
 		"SUBST_STAGE.inner2=     post-configure",
 		"SUBST_FILES.inner2=     inner.txt",
 		"SUBST_VARS.inner2=      INNER",
 		".endif")
 	t.CheckOutputLines(
-		"WARN: filename.mk:9: Incomplete SUBST block: SUBST_FILES.inner1 missing.",
+		"WARN: filename.mk:6: All but the first assignment " +
-		"WARN: filename.mk:9: Subst block \"inner1\" should be finished "+
+			"to \"SUBST_FILES.os\" should use the \"+=\" operator.")
 			"before adding the next class to SUBST_CLASSES.")
+}
-func (s *Suite) Test_SubstContext_finishClass__details_in_then_branch(c *check.C) {
+func (s *Suite) Test_substBlock_extractVarname(c *check.C) {
 	t := s.Init(c)
-	t.RunSubst(
+	test := func(input, expected string) {
-		"SUBST_CLASSES+=         os",
+		t.CheckEquals((*substBlock).extractVarname(nil, input), expected)
 		".if ${OPSYS} == NetBSD",
 		"SUBST_VARS.os=          OPSYS",
 		"SUBST_SED.os=           -e s,@OPSYS@,NetBSD,",
 		"SUBST_STAGE.os=         post-configure",
 		"SUBST_MESSAGE.os=       Guessing operating system",
 		"SUBST_FILES.os=         guess-os.h",
 		".endif")
-	t.CheckOutputLines(
+	// A simple variable name.
-		"WARN: filename.mk:5: SUBST_STAGE.os should not be defined conditionally.",
+	test("s,@VAR@,${VAR},", "VAR")
 		"WARN: filename.mk:6: SUBST_MESSAGE.os should not be defined conditionally.",
 		"WARN: filename.mk:EOF: Missing SUBST block for \"os\".")
-func (s *Suite) Test_SubstContext_finishClass__details_in_else_branch(c *check.C) {
+	// A parameterized variable name.
-	t := s.Init(c)
+	test("s,@VAR.param@,${VAR.param},", "VAR.param")
-	t.RunSubst(
+	// Only substitution commands can be replaced with SUBST_VARS.
-		"SUBST_CLASSES+=         os",
+	test("/pattern/d", "")
 		".if ${OPSYS} == NetBSD",
 		".else",
 		"SUBST_VARS.os=          OPSYS",
 		"SUBST_SED.os=           -e s,@OPSYS@,NetBSD,",
 		"SUBST_STAGE.os=         post-configure",
 		"SUBST_MESSAGE.os=       Guessing operating system",
 		"SUBST_FILES.os=         guess-os.h",
 		".endif")
-	t.CheckOutputLines(
+	// An incomplete substitution command.
-		"WARN: filename.mk:6: SUBST_STAGE.os should not be defined conditionally.",
+	test("s", "")
 		"WARN: filename.mk:7: SUBST_MESSAGE.os should not be defined conditionally.",
 		"WARN: filename.mk:EOF: Missing SUBST block for \"os\".")
-func (s *Suite) Test_SubstContext_finishClass__empty_conditional_at_end(c *check.C) {
+	// Wrong placeholder character, only @ works.
-	t := s.Init(c)
+	test("s,!VAR!,${VAR},", "")
-	t.RunSubst(
+	// The placeholder must have exactly 1 @ on each side.
-		"SUBST_CLASSES+=         os",
+	test("s,@@VAR@@,${VAR},", "")
 		"SUBST_VARS.os=          OPSYS",
 		"SUBST_SED.os=           -e s,@OPSYS@,NetBSD,",
 		"SUBST_STAGE.os=         post-configure",
 		"SUBST_MESSAGE.os=       Guessing operating system",
 		"SUBST_FILES.os=         guess-os.h",
 		".if ${OPSYS} == NetBSD",
 		".else",
 		".endif")
-	t.CheckOutputEmpty()
+	// Malformed because the comma is the separator.
 	test("s,@VAR,VAR@,${VAR},", "")
-func (s *Suite) Test_SubstContext_finishClass__missing_transformation_in_one_branch(c *check.C) {
+	// The replacement pattern is not a simple variable name enclosed in @.
-	t := s.Init(c)
+	test("s,@VAR!VAR@,${VAR},", "")
-	t.RunSubst(
+	// The replacement may only contain the :Q modifier.
-		"SUBST_CLASSES+=         os",
+	test("s,@VAR@,${VAR:Mpattern},", "")
 		"SUBST_STAGE.os=         post-configure",
 		"SUBST_MESSAGE.os=       Guessing operating system",
 		"SUBST_FILES.os=         guess-os.h",
 		".if ${OPSYS} == NetBSD",
 		"SUBST_FILES.os=         -e s,@OpSYS@,NetBSD,", // A simple typo, this should be SUBST_SED.
 		".elif ${OPSYS} == Darwin",
 		"SUBST_SED.os=           -e s,@OPSYS@,Darwin1,",
 		"SUBST_SED.os=           -e s,@OPSYS@,Darwin2,",
 		".else",
 		"SUBST_VARS.os=           OPSYS",
 		".endif")
-	t.CheckOutputLines(
+	// The :Q modifier is allowed in the replacement.
-		"WARN: filename.mk:6: All but the first assignment "+
+	test("s,@VAR@,${VAR:Q},", "VAR")
 			"to \"SUBST_FILES.os\" should use the \"+=\" operator.",
 		"WARN: filename.mk:9: All but the first assignment "+
 			"to \"SUBST_SED.os\" should use the \"+=\" operator.",
 		"WARN: filename.mk:EOF: Incomplete SUBST block: SUBST_SED.os, "+
 			"SUBST_VARS.os or SUBST_FILTER_CMD.os missing.")
-func (s *Suite) Test_SubstContext_finishClass__nested_conditionals(c *check.C) {
+	// The replacement may contain the :Q modifier only once.
-	t := s.Init(c)
+	test("s,@VAR@,${VAR:Q:Q},", "")
-	t.RunSubst(
+	// The replacement must be a plain variable expression, without prefix.
-		"SUBST_CLASSES+=         os",
+	test("s,@VAR@,prefix${VAR},", "")
 		"SUBST_STAGE.os=         post-configure",
 		"SUBST_MESSAGE.os=       Guessing operating system",
 		".if ${OPSYS} == NetBSD",
 		"SUBST_FILES.os=         guess-netbsd.h",
 		".  if ${ARCH} == i386",
 		"SUBST_FILTER_CMD.os=    ${SED} -e s,@OPSYS,NetBSD-i386,",
 		".  elif ${ARCH} == x86_64",
 		"SUBST_VARS.os=          OPSYS",
 		".  else",
 		"SUBST_SED.os=           -e s,@OPSYS,NetBSD-unknown",
 		".  endif",
 		".else",
 		// This branch omits SUBST_FILES.
 		"SUBST_SED.os=           -e s,@OPSYS@,unknown,",
 		".endif")
-	t.CheckOutputLines(
+	// The replacement must be a plain variable expression, without suffix.
-		"WARN: filename.mk:EOF: Incomplete SUBST block: SUBST_FILES.os missing.")
+	test("s,@VAR@,${VAR}suffix,", "")
+}
-func (s *Suite) Test_SubstContext_isComplete__incomplete(c *check.C) {
+func (s *Suite) Test_substBlock_isComplete__incomplete(c *check.C) {
 	t := s.Init(c)
 	ctx := NewSubstContext()
 	ctx.varassign(t.NewMkLine("filename.mk", 10, "PKGNAME=pkgname-1.0"))
-	t.CheckEquals(ctx.id, "")
+	t.CheckEquals(ctx.isActive(), false)
 	ctx.varassign(t.NewMkLine("filename.mk", 11, "SUBST_CLASSES+=interp"))
-	t.CheckEquals(ctx.id, "interp")
+	t.CheckEquals(ctx.isActive(), false)
 	ctx.varassign(t.NewMkLine("filename.mk", 12, "SUBST_FILES.interp=Makefile"))
-	t.CheckEquals(ctx.isComplete(), false)
+	t.CheckEquals(ctx.activeId(), "interp")
 	t.CheckEquals(ctx.block().isComplete(), false)
 	ctx.varassign(t.NewMkLine("filename.mk", 13, "SUBST_SED.interp=s,@PREFIX@,${PREFIX},g"))
-	t.CheckEquals(ctx.isComplete(), false)
+	t.CheckEquals(ctx.block().isComplete(), false)
 	ctx.Finish(t.NewMkLine("filename.mk", 14, ""))
 	t.CheckOutputLines(
 		"NOTE: filename.mk:13: The substitution command \"s,@PREFIX@,${PREFIX},g\" "+
 			"can be replaced with \"SUBST_VARS.interp= PREFIX\".",
 		"WARN: filename.mk:14: Incomplete SUBST block: SUBST_STAGE.interp missing.")
+}
-func (s *Suite) Test_SubstContext_isComplete__complete(c *check.C) {
+func (s *Suite) Test_substBlock_isComplete__complete(c *check.C) {
 	t := s.Init(c)
 	ctx := NewSubstContext()
 	ctx.varassign(t.NewMkLine("filename.mk", 10, "PKGNAME=pkgname-1.0"))
 	ctx.varassign(t.NewMkLine("filename.mk", 11, "SUBST_CLASSES+=p"))
 	ctx.varassign(t.NewMkLine("filename.mk", 12, "SUBST_FILES.p=Makefile"))
 	ctx.varassign(t.NewMkLine("filename.mk", 13, "SUBST_SED.p=s,@PREFIX@,${PREFIX},g"))
-	t.CheckEquals(ctx.isComplete(), false)
+	t.CheckEquals(ctx.block().isComplete(), false)
 	ctx.varassign(t.NewMkLine("filename.mk", 14, "SUBST_STAGE.p=post-configure"))
-	t.CheckEquals(ctx.isComplete(), true)
+	t.CheckEquals(ctx.block().isComplete(), true)
 	ctx.Finish(t.NewMkLine("filename.mk", 15, ""))
 	t.CheckOutputLines(
 		"NOTE: filename.mk:13: The substitution command \"s,@PREFIX@,${PREFIX},g\" " +
 			"can be replaced with \"SUBST_VARS.p= PREFIX\".")
+}
 func (s *Suite) Test_substBlock_finish__conditional_inside_then(c *check.C) {
 	t := s.Init(c)
 	t.RunSubst(
 		".if ${OPSYS} == Linux",
 		"SUBST_CLASSES+=\tid",
 		"SUBST_STAGE.id=\tpre-configure",
 		"SUBST_SED.id=\t-e sahara",
 		".else",
 		".endif")
 	// The block already ends at the .else, not at the end of the file,
 	// since that is the scope where the SUBST id is defined.
 	t.CheckOutputLines(
 		"WARN: filename.mk:5: Incomplete SUBST block: SUBST_FILES.id missing.")
+}
 func (s *Suite) Test_substBlock_finish__conditional_inside_else(c *check.C) {
 	t := s.Init(c)
 	t.RunSubst(
 		".if ${OPSYS} == Linux",
 		".else",
 		"SUBST_CLASSES+=\tid",
 		"SUBST_STAGE.id=\tpre-configure",
 		"SUBST_SED.id=\t-e sahara",
 		".endif")
 	// The block already ends at the .endif, not at the end of the file,
 	// since that is the scope where the SUBST id is defined.
 	t.CheckOutputLines(
 		"WARN: filename.mk:6: Incomplete SUBST block: SUBST_FILES.id missing.")
+}
 func (s *Suite) Test_substBlock_finish__files_missing(c *check.C) {
 	t := s.Init(c)
 	t.RunSubst(
 		"SUBST_CLASSES+=         one",
 		"SUBST_STAGE.one=        pre-configure",
 		"SUBST_CLASSES+=         two",
 		"SUBST_STAGE.two=        pre-configure",
 		"SUBST_FILES.two=        two.txt",
 		"SUBST_SED.two=          s,two,2,g")
 	t.CheckOutputLines(
 		"WARN: filename.mk:3: Subst block \"one\" should be finished "+
 			"before adding the next class to SUBST_CLASSES.",
 		"WARN: filename.mk:3: Incomplete SUBST block: SUBST_FILES.one missing.",
 		"WARN: filename.mk:3: Incomplete SUBST block: "+
 			"SUBST_SED.one, SUBST_VARS.one or SUBST_FILTER_CMD.one missing.")
+}
 // Variables mentioned in SUBST_VARS may appear in the same paragraph,
 // or alternatively anywhere else in the file.
 func (s *Suite) Test_substBlock_checkForeignVariables__in_next_paragraph(c *check.C) {
 	t := s.Init(c)
 	t.RunSubst(
 		"SUBST_CLASSES+=\tos",
 		"SUBST_STAGE.os=\tpre-configure",
 		"SUBST_FILES.os=\tguess-os.h",
 		"SUBST_VARS.os=\tTODAY1",
 		"",
 		"TODAY1!=\tdate",
 		"TODAY2!=\tdate")
 	t.CheckOutputEmpty()
+}
 func (s *Suite) Test_substBlock_checkForeignVariables__mixed_separate(c *check.C) {
 	t := s.Init(c)
 	t.RunSubst(
 		"SUBST_CLASSES+= 1",
 		"SUBST_STAGE.1=  post-configure",
 		"SUBST_FILES.1=  files",
 		"",
 		"SUBST_VARS.1=   VAR",
 		"USE_TOOLS+=     gmake")
 	// The USE_TOOLS is not in the SUBST block anymore since there is
 	// an empty line between SUBST_CLASSES and SUBST_VARS.
 	t.CheckOutputEmpty()
+}
 // Variables mentioned in SUBST_VARS are not considered "foreign"
 // in the block and may be mixed with the other SUBST variables.
 func (s *Suite) Test_substBlock_checkForeignVariables__in_block(c *check.C) {
 	t := s.Init(c)
 	t.RunSubst(
 		"SUBST_CLASSES+=\tos",
 		"SUBST_STAGE.os=\tpre-configure",
 		"SUBST_FILES.os=\tguess-os.h",
 		"SUBST_VARS.os=\tTODAY1",
 		"TODAY1!=\tdate",
 		"TODAY2!=\tdate")
 	t.CheckOutputLines(
 		"WARN: filename.mk:6: Foreign variable \"TODAY2\" in SUBST block.")
+}

 @@ -1,17 +1,16 @@
-# $NetBSD: Makefile,v 1.618 2019/12/13 07:44:02 bsiegert Exp $
+# $NetBSD: Makefile,v 1.619 2019/12/14 18:04:15 rillig Exp $
-PKGNAME=	pkglint-19.3.17
+PKGNAME=	pkglint-19.3.18
 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

 @@ -63,28 +63,27 @@ func (s *Suite) Test_MkCondChecker_Check
 		"ERROR: filename.mk:4: Use ${PKGSRC_COMPILER:Mmsvc} instead of the == operator.")
 	// PKG_LIBTOOL is only available after including bsd.pkg.mk,
 	// therefore the :U and the subsequent warning.
 	test(".if ${PKG_LIBTOOL:U:Mlibtool}",
 		"NOTE: filename.mk:4: PKG_LIBTOOL can be "+
 			"compared using the simpler \"${PKG_LIBTOOL:U} == libtool\" "+
 			"instead of matching against \":Mlibtool\".",
 		"WARN: filename.mk:4: PKG_LIBTOOL should not be used at load time in any file.")
 	test(".if ${MACHINE_PLATFORM:MUnknownOS-*-*} || ${MACHINE_ARCH:Mx86}",
 		"WARN: filename.mk:4: "+
 			"The pattern \"UnknownOS\" cannot match any of "+
-			"{ AIX BSDOS Bitrig Cygwin Darwin DragonFly FreeBSD FreeMiNT GNUkFreeBSD HPUX Haiku "+
+			"{ Cygwin DragonFly FreeBSD Linux NetBSD SunOS "+
 			"IRIX Interix Linux Minix MirBSD NetBSD OSF1 OpenBSD QNX SCO_SV SunOS UnixWare "+
 			"} for the operating system part of MACHINE_PLATFORM.",
 		"WARN: filename.mk:4: "+
 			"The pattern \"x86\" cannot match any of "+
 			"{ aarch64 aarch64eb alpha amd64 arc arm arm26 arm32 cobalt coldfire convex dreamcast earm "+
 			"earmeb earmhf earmhfeb earmv4 earmv4eb earmv5 earmv5eb earmv6 earmv6eb earmv6hf earmv6hfeb "+
 			"earmv7 earmv7eb earmv7hf earmv7hfeb evbarm hpcmips hpcsh hppa hppa64 i386 i586 i686 ia64 "+
 			"m68000 m68k m88k mips mips64 mips64eb mips64el mipseb mipsel mipsn32 mlrisc ns32k pc532 pmax "+
 			"powerpc powerpc64 rs6000 s390 sh3eb sh3el sparc sparc64 vax x86_64 "+
 			"} for MACHINE_ARCH.",
 		"NOTE: filename.mk:4: MACHINE_ARCH can be "+
 			"compared using the simpler \"${MACHINE_ARCH} == x86\" "+
 			"instead of matching against \":Mx86\".")

 @@ -508,27 +508,27 @@ func (reg *VarTypeRegistry) Init(src *Pk
 		"57 56 55 51 MARIADB55",
 		"MYSQL_VERSIONS_ACCEPTED")
 	pgsqlVersions := reg.enumFrom(src,
 		"mk/pgsql.buildlink3.mk",
 		"10 96 95 94 93",
 		"PGSQL_VERSIONS_ACCEPTED")
 	jvms := reg.enumFrom(src,
 		"mk/java-vm.mk",
 		"openjdk8 oracle-jdk8 openjdk7 sun-jdk7 jdk16 jdk15 kaffe",
 		"_PKG_JVMS.*")
-	platforms := reg.enumFromFiles(src,
+	operatingSystems := reg.enumFromFiles(src,
 		"mk/platform",
 		`(.*)\.mk$`, "$1",
 		"Cygwin DragonFly FreeBSD Linux NetBSD SunOS")
 	// TODO: Only mark those variables as user-settable that actually influence
 	//  the generated packages. For example, UPDATE_TARGET doesn't.
 	// Last synced with mk/defaults/mk.conf revision 1.300 (abbf617a26f3).
 	reg.usr("USE_CWRAPPERS", enum("yes no auto"))
 	reg.usr("ALLOW_VULNERABLE_PACKAGES", BtYes)
 	reg.usrlist("AUDIT_PACKAGES_FLAGS", BtShellWord)
 	reg.usrlist("MANINSTALL", enum("maninstall catinstall"))
 	reg.usr("MANZ", BtYes)
 @@ -1352,27 +1352,27 @@ func (reg *VarTypeRegistry) Init(src *Pk
 	reg.pkgload("NO_BUILD", BtYes)
 	reg.pkg("NO_CHECKSUM", BtYes)
 	reg.pkg("NO_CONFIGURE", BtYes)
 	reg.pkg("NO_EXPORT_CPP", BtYes)
 	reg.pkg("NO_EXTRACT", BtYes)
 	reg.pkg("NO_INSTALL_MANPAGES", BtYes) // only has an effect for Imake packages.
 	reg.pkg("NO_PKGTOOLS_REQD_CHECK", BtYes)
 	reg.pkg("NO_SRC_ON_CDROM", BtRestricted)
 	reg.pkg("NO_SRC_ON_FTP", BtRestricted)
 	reg.sysload("OBJECT_FMT", enum("COFF ECOFF ELF SOM XCOFF Mach-O PE a.out"))
 	reg.pkglistrat("ONLY_FOR_COMPILER", compilers)
 	reg.pkglistrat("ONLY_FOR_PLATFORM", BtMachinePlatformPattern)
 	reg.pkgrat("ONLY_FOR_UNPRIVILEGED", BtYesNo)
-	reg.sysload("OPSYS", platforms, DefinedIfInScope|NonemptyIfDefined)
+	reg.sysload("OPSYS", operatingSystems, DefinedIfInScope|NonemptyIfDefined)
 	reg.pkglistbl3("OPSYSVARS", BtVariableName)
 	reg.pkg("OSVERSION_SPECIFIC", BtYes)
 	reg.sysload("OS_VARIANT", BtIdentifierDirect, DefinedIfInScope)
 	reg.sysload("OS_VERSION", BtVersion)
 	reg.sysload("OSX_VERSION", BtVersion) // See mk/platform/Darwin.mk.
 	reg.pkg("OVERRIDE_DIRDEPTH*", BtInteger)
 	reg.pkg("OVERRIDE_GNU_CONFIG_SCRIPTS", BtYes)
 	reg.pkg("OWNER", BtMailAddress)
 	reg.pkglist("OWN_DIRS", BtPathPattern)
 	reg.pkglist("OWN_DIRS_PERMS", BtPerms)
 	reg.sys("PAMBASE", BtPathname)
 	reg.usr("PAM_DEFAULT", enum("linux-pam openpam solaris-pam"))
 	reg.pkgload("PATCHDIR", BtRelativePkgPath)

 @@ -457,53 +457,48 @@ var (
 	BtURL                    = &BasicType{"URL", (*VartypeCheck).URL}
 	BtUserGroupName          = &BasicType{"UserGroupName", (*VartypeCheck).UserGroupName}
 	BtVariableName           = &BasicType{"VariableName", (*VartypeCheck).VariableName}
 	BtVariableNamePattern    = &BasicType{"VariableNamePattern", (*VartypeCheck).VariableNamePattern}
 	BtVersion                = &BasicType{"Version", (*VartypeCheck).Version}
 	BtWrapperReorder         = &BasicType{"WrapperReorder", (*VartypeCheck).WrapperReorder}
 	BtWrapperTransform       = &BasicType{"WrapperTransform", (*VartypeCheck).WrapperTransform}
 	BtWrkdirSubdirectory     = &BasicType{"WrkdirSubdirectory", (*VartypeCheck).WrkdirSubdirectory}
 	BtWrksrcSubdirectory     = &BasicType{"WrksrcSubdirectory", (*VartypeCheck).WrksrcSubdirectory}
 	BtYes                    = &BasicType{"Yes", (*VartypeCheck).Yes}
 	BtYesNo                  = &BasicType{"YesNo", (*VartypeCheck).YesNo}
 	BtYesNoIndirectly        = &BasicType{"YesNoIndirectly", (*VartypeCheck).YesNoIndirectly}
 	BtMachineOpsys            = enumFromValues(machineOpsysValues)
 	BtMachineArch             = enumFromValues(machineArchValues)
 	BtMachineGnuArch          = enumFromValues(machineGnuArchValues)
 	BtEmulOpsys               = enumFromValues(emulOpsysValues)
 	BtEmulArch                = enumFromValues(machineArchValues) // Just a wild guess.
 	BtMachineGnuPlatformOpsys = BtEmulOpsys
 	btForLoop = &BasicType{".for loop", nil /* never called */}
+)
 // Necessary due to circular dependencies between the checkers.
 //
 // The Go compiler is stricter than absolutely necessary for this particular case.
 // The following methods are only referred to but not invoked during initialization.
 func init() {
 	BtShellCommand.checker = (*VartypeCheck).ShellCommand
 	BtShellCommands.checker = (*VartypeCheck).ShellCommands
 	BtShellWord.checker = (*VartypeCheck).ShellWord
+}
 // TODO: Move these values to VarTypeRegistry.Init and read them from the
 //  pkgsrc infrastructure files, as far as possible.
 const (
 	machineOpsysValues = "" + // See mk/platform
 		"AIX BSDOS Bitrig Cygwin Darwin DragonFly FreeBSD FreeMiNT GNUkFreeBSD " +
 		"HPUX Haiku IRIX Interix Linux Minix MirBSD NetBSD OSF1 OpenBSD QNX SCO_SV SunOS UnixWare"
 	// See mk/emulator/emulator-vars.mk.
 	emulOpsysValues = "" +
 		"bitrig bsdos cygwin darwin dragonfly freebsd " +
 		"haiku hpux interix irix linux mirbsd netbsd openbsd osf1 solaris sunos"
 	// Hardware architectures having the same name in bsd.own.mk and the GNU world.
 	// These are best-effort guesses, since they depend on the operating system.
 	archValues = "" +
 		"aarch64 alpha amd64 arc arm cobalt convex dreamcast i386 " +
 		"hpcmips hpcsh hppa hppa64 ia64 " +
 		"m68k m88k mips mips64 mips64el mipseb mipsel mipsn32 mlrisc " +
 		"ns32k pc532 pmax powerpc powerpc64 rs6000 s390 sparc sparc64 vax x86_64"

 @@ -789,28 +789,34 @@ func (cv *VartypeCheck) MachinePlatformP
 		return
+	}
 	const rePart = `(?:\[[^\]]+\]|[^-\[])+`
 	const rePair = `^(` + rePart + `)-(` + rePart + `)$`
 	const reTriple = `^(` + rePart + `)-(` + rePart + `)-(` + rePart + `)$`
 	pattern := cv.Value
 	if matches(pattern, rePair) && hasSuffix(pattern, "*") {
 		pattern += "-*"
+	}
 	if m, opsysPattern, versionPattern, archPattern := match3(pattern, reTriple); m {
 		// This is cheated, but the dynamically loaded enums are not
 		// provided in a type registry where they could be looked up
 		// by a name. The following line therefore assumes that OPSYS
 		// is an operating system name, which sounds like a safe bet.
 		btOpsys := G.Pkgsrc.vartypes.types["OPSYS"].basicType
 		opsysCv := cv.WithVarnameValueMatch("the operating system part of "+cv.Varname, opsysPattern)
-		BtMachineOpsys.checker(opsysCv)
+		btOpsys.checker(opsysCv)
 		versionCv := cv.WithVarnameValueMatch("the version part of "+cv.Varname, versionPattern)
 		versionCv.Version()
 		archCv := cv.WithVarnameValueMatch("the hardware architecture part of "+cv.Varname, archPattern)
 		BtMachineArch.checker(archCv)
 	} else {
 		cv.Warnf("%q is not a valid platform pattern.", cv.Value)
 		cv.Explain(
 			"A platform pattern has the form <OPSYS>-<OS_VERSION>-<MACHINE_ARCH>.",
 			"Each of these components may be a shell globbing expression.",
 			"",

 @@ -1166,43 +1166,41 @@ func (s *Suite) Test_VartypeCheck_Machin
 	vt.Values(
 		"linux-i386",
 		"nextbsd-5.0-8087",
 		"netbsd-7.0-l*",
 		"NetBSD-1.6.2-i386",
 		"FreeBSD*",
 		"FreeBSD-*",
 		"${LINUX}",
 		"NetBSD-[0-1]*-*")
 	vt.Output(
 		"WARN: filename.mk:1: \"linux-i386\" is not a valid platform pattern.",
 		"WARN: filename.mk:2: The pattern \"nextbsd\" cannot match any of "+
-			"{ AIX BSDOS Bitrig Cygwin Darwin DragonFly FreeBSD FreeMiNT GNUkFreeBSD HPUX Haiku "+
+			"{ Cygwin DragonFly FreeBSD Linux NetBSD SunOS "+
 			"IRIX Interix Linux Minix MirBSD NetBSD OSF1 OpenBSD QNX SCO_SV SunOS UnixWare "+
 			"} for the operating system part of MACHINE_PLATFORM.",
 		"WARN: filename.mk:2: The pattern \"8087\" cannot match any of "+
 			"{ aarch64 aarch64eb alpha amd64 arc arm arm26 arm32 "+
 			"cobalt coldfire convex dreamcast "+
 			"earm earmeb earmhf earmhfeb earmv4 earmv4eb "+
 			"earmv5 earmv5eb earmv6 earmv6eb earmv6hf "+
 			"earmv6hfeb earmv7 earmv7eb earmv7hf earmv7hfeb evbarm hpcmips hpcsh hppa hppa64 "+
 			"i386 i586 i686 ia64 m68000 m68k m88k "+
 			"mips mips64 mips64eb mips64el mipseb mipsel mipsn32 "+
 			"mlrisc ns32k pc532 pmax powerpc powerpc64 "+
 			"rs6000 s390 sh3eb sh3el sparc sparc64 vax x86_64 "+
 			"} for the hardware architecture part of MACHINE_PLATFORM.",
 		"WARN: filename.mk:3: The pattern \"netbsd\" cannot match any of "+
-			"{ AIX BSDOS Bitrig Cygwin Darwin DragonFly FreeBSD FreeMiNT GNUkFreeBSD HPUX Haiku "+
+			"{ Cygwin DragonFly FreeBSD Linux NetBSD SunOS "+
 			"IRIX Interix Linux Minix MirBSD NetBSD OSF1 OpenBSD QNX SCO_SV SunOS UnixWare "+
 			"} for the operating system part of MACHINE_PLATFORM.",
 		"WARN: filename.mk:3: The pattern \"l*\" cannot match any of "+
 			"{ aarch64 aarch64eb alpha amd64 arc arm arm26 arm32 "+
 			"cobalt coldfire convex dreamcast "+
 			"earm earmeb earmhf earmhfeb earmv4 earmv4eb "+
 			"earmv5 earmv5eb earmv6 earmv6eb earmv6hf "+
 			"earmv6hfeb earmv7 earmv7eb earmv7hf earmv7hfeb evbarm hpcmips hpcsh hppa hppa64 "+
 			"i386 i586 i686 ia64 m68000 m68k m88k "+
 			"mips mips64 mips64eb mips64el mipseb mipsel mipsn32 "+
 			"mlrisc ns32k pc532 pmax powerpc powerpc64 "+
 			"rs6000 s390 sh3eb sh3el sparc sparc64 vax x86_64 "+
 			"} for the hardware architecture part of MACHINE_PLATFORM.",
 		"WARN: filename.mk:5: \"FreeBSD*\" is not a valid platform pattern.",
 @@ -1217,43 +1215,41 @@ func (s *Suite) Test_VartypeCheck_Machin
 	vt.Values(
 		"linux-i386",
 		"nextbsd-5.0-8087",
 		"netbsd-7.0-l*",
 		"NetBSD-1.6.2-i386",
 		"FreeBSD*",
 		"FreeBSD-*",
 		"${LINUX}",
 		"NetBSD-[0-1]*-*")
 	vt.Output(
 		"WARN: filename.mk:1: \"linux-i386\" is not a valid platform pattern.",
 		"WARN: filename.mk:2: The pattern \"nextbsd\" cannot match any of "+
-			"{ AIX BSDOS Bitrig Cygwin Darwin DragonFly FreeBSD FreeMiNT GNUkFreeBSD HPUX Haiku "+
+			"{ Cygwin DragonFly FreeBSD Linux NetBSD SunOS "+
 			"IRIX Interix Linux Minix MirBSD NetBSD OSF1 OpenBSD QNX SCO_SV SunOS UnixWare "+
 			"} for the operating system part of ONLY_FOR_PLATFORM.",
 		"WARN: filename.mk:2: The pattern \"8087\" cannot match any of "+
 			"{ aarch64 aarch64eb alpha amd64 arc arm arm26 arm32 "+
 			"cobalt coldfire convex dreamcast "+
 			"earm earmeb earmhf earmhfeb earmv4 earmv4eb "+
 			"earmv5 earmv5eb earmv6 earmv6eb earmv6hf "+
 			"earmv6hfeb earmv7 earmv7eb earmv7hf earmv7hfeb evbarm hpcmips hpcsh hppa hppa64 "+
 			"i386 i586 i686 ia64 m68000 m68k m88k "+
 			"mips mips64 mips64eb mips64el mipseb mipsel mipsn32 "+
 			"mlrisc ns32k pc532 pmax powerpc powerpc64 "+
 			"rs6000 s390 sh3eb sh3el sparc sparc64 vax x86_64 "+
 			"} for the hardware architecture part of ONLY_FOR_PLATFORM.",
 		"WARN: filename.mk:3: The pattern \"netbsd\" cannot match any of "+
-			"{ AIX BSDOS Bitrig Cygwin Darwin DragonFly FreeBSD FreeMiNT GNUkFreeBSD HPUX Haiku "+
+			"{ Cygwin DragonFly FreeBSD Linux NetBSD SunOS "+
 			"IRIX Interix Linux Minix MirBSD NetBSD OSF1 OpenBSD QNX SCO_SV SunOS UnixWare "+
 			"} for the operating system part of ONLY_FOR_PLATFORM.",
 		"WARN: filename.mk:3: The pattern \"l*\" cannot match any of "+
 			"{ aarch64 aarch64eb alpha amd64 arc arm arm26 arm32 "+
 			"cobalt coldfire convex dreamcast "+
 			"earm earmeb earmhf earmhfeb earmv4 earmv4eb "+
 			"earmv5 earmv5eb earmv6 earmv6eb earmv6hf "+
 			"earmv6hfeb earmv7 earmv7eb earmv7hf earmv7hfeb evbarm hpcmips hpcsh hppa hppa64 "+
 			"i386 i586 i686 ia64 m68000 m68k m88k "+
 			"mips mips64 mips64eb mips64el mipseb mipsel mipsn32 "+
 			"mlrisc ns32k pc532 pmax powerpc powerpc64 "+
 			"rs6000 s390 sh3eb sh3el sparc sparc64 vax x86_64 "+
 			"} for the hardware architecture part of ONLY_FOR_PLATFORM.",
 		"WARN: filename.mk:5: \"FreeBSD*\" is not a valid platform pattern.",