Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

===================================================================
RCS file: /ftp/cvs/cvsroot/pkgsrc/pkgtools/pkglint/files/Attic/mkvarusechecker.go,v
rcsdiff: /ftp/cvs/cvsroot/pkgsrc/pkgtools/pkglint/files/Attic/mkvarusechecker.go,v: warning: Unknown phrases like `commitid ...;' are present.
retrieving revision 1.2
retrieving revision 1.7
diff -u -p -r1.2 -r1.7
--- pkgsrc/pkgtools/pkglint/files/Attic/mkvarusechecker.go	2019/12/08 22:03:38	1.2
+++ pkgsrc/pkgtools/pkglint/files/Attic/mkvarusechecker.go	2020/03/07 23:35:35	1.7
@@ -25,10 +25,12 @@ func (ck *MkVarUseChecker) Check(vuc *Va
 	ck.checkUndefined()
 	ck.checkPermissions(vuc)
 
-	ck.checkVarname()
+	ck.checkVarname(vuc.time)
 	ck.checkModifiers()
+	ck.checkAssignable(vuc)
 	ck.checkQuoting(vuc)
 
+	ck.checkToolsPlatform()
 	ck.checkBuildDefs()
 	ck.checkDeprecated()
 
@@ -42,15 +44,15 @@ func (ck *MkVarUseChecker) checkUndefine
 	varname := varuse.varname
 
 	switch {
-	case !G.Opts.WarnExtra,
+	case !G.WarnExtra,
 		// Well-known variables are probably defined by the infrastructure.
 		vartype != nil && !vartype.IsGuessed(),
 		// TODO: At load time, check ck.MkLines.loadVars instead of allVars.
 		ck.MkLines.allVars.IsDefinedSimilar(varname),
-		ck.MkLines.forVars[varname],
+		ck.MkLines.checkAllData.forVars[varname],
 		ck.MkLines.allVars.Mentioned(varname) != nil,
-		G.Pkg != nil && G.Pkg.vars.IsDefinedSimilar(varname),
-		containsVarRef(varname),
+		ck.MkLines.pkg != nil && ck.MkLines.pkg.vars.IsDefinedSimilar(varname),
+		containsVarUse(varname),
 		G.Pkgsrc.vartypes.IsDefinedCanon(varname),
 		varname == "":
 		return
@@ -130,7 +132,7 @@ func (ck *MkVarUseChecker) checkModifier
 	fix.Apply()
 }
 
-func (ck *MkVarUseChecker) checkVarname() {
+func (ck *MkVarUseChecker) checkVarname(time VucTime) {
 	varname := ck.use.varname
 	if varname == "@" {
 		ck.MkLine.Warnf("Please use %q instead of %q.", "${.TARGET}", "$@")
@@ -139,10 +141,10 @@ func (ck *MkVarUseChecker) checkVarname(
 			"of the same name.")
 	}
 
-	if varname == "LOCALBASE" && !G.Infrastructure {
+	if varname == "LOCALBASE" && !G.Infrastructure && time == VucRunTime {
 		fix := ck.MkLine.Autofix()
 		fix.Warnf("Please use PREFIX instead of LOCALBASE.")
-		fix.ReplaceRegex(`\$\{LOCALBASE\b`, "${PREFIX", 1)
+		fix.ReplaceAfter("${", "LOCALBASE", "PREFIX")
 		fix.Apply()
 	}
 }
@@ -153,7 +155,7 @@ func (ck *MkVarUseChecker) checkVarname(
 //
 // See checkVarassignLeftPermissions.
 func (ck *MkVarUseChecker) checkPermissions(vuc *VarUseContext) {
-	if !G.Opts.WarnPerm {
+	if !G.WarnPerm {
 		return
 	}
 	if G.Infrastructure {
@@ -200,7 +202,9 @@ func (ck *MkVarUseChecker) checkPermissi
 
 	effPerms := vartype.EffectivePermissions(basename)
 	if effPerms.Contains(aclpUseLoadtime) {
-		ck.checkUseAtLoadTime(vuc.time)
+		if vuc.time == VucLoadTime {
+			ck.checkUseAtLoadTime()
+		}
 
 		// Since the variable may be used at load time, it probably
 		// may be used at run time as well. If it weren't, that would
@@ -285,34 +289,32 @@ func (ck *MkVarUseChecker) warnPermissio
 	}
 	alternativeFiles := vartype.AlternativeFiles(needed)
 
-	loadTimeExplanation := func() []string {
-		return []string{
-			"Many variables, especially lists of something, get their values incrementally.",
-			"Therefore it is generally unsafe to rely on their",
-			"value until it is clear that it will never change again.",
-			"This point is reached when the whole package Makefile is loaded and",
-			"execution of the shell commands starts; in some cases earlier.",
-			"",
-			"Additionally, when using the \":=\" operator, each $$ is replaced",
-			"with a single $, so variables that have references to shell",
-			"variables or regular expressions are modified in a subtle way."}
-	}
+	loadTimeExplanation := []string{
+		"Many variables, especially lists of something, get their values incrementally.",
+		"Therefore it is generally unsafe to rely on their",
+		"value until it is clear that it will never change again.",
+		"This point is reached when the whole package Makefile is loaded and",
+		"execution of the shell commands starts; in some cases earlier.",
+		"",
+		"Additionally, when using the \":=\" operator, each $$ is replaced",
+		"with a single $, so variables that have references to shell",
+		"variables or regular expressions are modified in a subtle way."}
 
 	switch {
 	case alternativeFiles == "" && directly:
 		mkline.Warnf("%s should not be used at load time in any file.", varname)
-		ck.explainPermissions(varname, vartype, loadTimeExplanation()...)
+		ck.explainPermissions(varname, vartype, loadTimeExplanation...)
 
 	case alternativeFiles == "":
 		mkline.Warnf("%s should not be used in any file.", varname)
-		ck.explainPermissions(varname, vartype, loadTimeExplanation()...)
+		ck.explainPermissions(varname, vartype, loadTimeExplanation...)
 
 	case directly:
 		mkline.Warnf(
 			"%s should not be used at load time in this file; "+
 				"it would be ok in %s.",
 			varname, alternativeFiles)
-		ck.explainPermissions(varname, vartype, loadTimeExplanation()...)
+		ck.explainPermissions(varname, vartype, loadTimeExplanation...)
 
 	default:
 		mkline.Warnf(
@@ -364,14 +366,11 @@ func (ck *MkVarUseChecker) explainPermis
 	ck.MkLine.Explain(expl...)
 }
 
-func (ck *MkVarUseChecker) checkUseAtLoadTime(time VucTime) {
-	if time != VucLoadTime {
-		return
-	}
+func (ck *MkVarUseChecker) checkUseAtLoadTime() {
 	if ck.vartype.IsAlwaysInScope() || ck.MkLines.Tools.SeenPrefs {
 		return
 	}
-	if G.Pkg != nil && G.Pkg.seenPrefs {
+	if ck.MkLines.pkg != nil && ck.MkLines.pkg.seenPrefs {
 		return
 	}
 	mkline := ck.MkLine
@@ -380,12 +379,10 @@ func (ck *MkVarUseChecker) checkUseAtLoa
 		return
 	}
 
-	if ck.vartype.IsPackageSettable() &&
-		basename != "Makefile" && basename != "options.mk" {
-
-		// For package-settable variables, the explanation doesn't
-		// make sense since it talks about completely different
-		// types of variables.
+	if ck.vartype.IsPackageSettable() {
+		// For package-settable variables, the explanation below
+		// doesn't make sense since including bsd.prefs.mk won't
+		// define any package-settable variables.
 		return
 	}
 
@@ -419,8 +416,6 @@ func (ck *MkVarUseChecker) warnToolLoadT
 	//  to skip the shell and execute the commands via execve, which
 	//  means that even echo is not a shell-builtin anymore.
 
-	// TODO: Replace "parse time" with "load time" everywhere.
-
 	if tool.Validity == AfterPrefsMk {
 		ck.MkLine.Warnf("To use the tool ${%s} at load time, bsd.prefs.mk has to be included before.", varname)
 		return
@@ -452,10 +447,39 @@ func (ck *MkVarUseChecker) warnToolLoadT
 		"except in the package Makefile itself.")
 }
 
+func (ck *MkVarUseChecker) checkAssignable(vuc *VarUseContext) {
+	leftType := vuc.vartype
+	if leftType == nil || leftType.basicType != BtPathname {
+		return
+	}
+	rightType := G.Pkgsrc.VariableType(ck.MkLines, ck.use.varname)
+	if rightType == nil || rightType.basicType != BtShellCommand {
+		return
+	}
+
+	mkline := ck.MkLine
+	if mkline.Varcanon() == "PKG_SHELL.*" {
+		switch ck.use.varname {
+		case "SH", "BASH", "TOOLS_PLATFORM.sh":
+			return
+		}
+	}
+
+	mkline.Warnf(
+		"Incompatible types: %s (type %q) cannot be assigned to type %q.",
+		ck.use.varname, rightType.basicType.name, leftType.basicType.name)
+	mkline.Explain(
+		"Shell commands often start with a pathname.",
+		"They could also start with a list of environment variable",
+		"definitions, since that is accepted by the shell.",
+		"They can also contain addition command line arguments",
+		"that are not filenames at all.")
+}
+
 // checkVarUseWords checks whether a variable use of the form ${VAR}
 // or ${VAR:modifiers} is allowed in a certain context.
 func (ck *MkVarUseChecker) checkQuoting(vuc *VarUseContext) {
-	if !G.Opts.WarnQuoting || vuc.quoting == VucQuotUnknown {
+	if !G.WarnQuoting || vuc.quoting == VucQuotUnknown {
 		return
 	}
 
@@ -475,7 +499,7 @@ func (ck *MkVarUseChecker) checkQuoting(
 	// since the GNU configure scripts cannot handle these space characters.
 	//
 	// When doing checks outside a package, the :M* modifier is needed for safety.
-	needMstar := (G.Pkg == nil || G.Pkg.vars.IsDefined("GNU_CONFIGURE")) &&
+	needMstar := (ck.MkLines.pkg == nil || ck.MkLines.pkg.vars.IsDefined("GNU_CONFIGURE")) &&
 		matches(varUse.varname, `^(?:.*_)?(?:CFLAGS|CPPFLAGS|CXXFLAGS|FFLAGS|LDFLAGS|LIBS)$`)
 
 	mkline := ck.MkLine
@@ -504,11 +528,11 @@ func (ck *MkVarUseChecker) checkQuotingQ
 	if correctMod == mod+":Q" && vuc.IsWordPart && !vartype.IsShell() {
 
 		isSingleWordConstant := func() bool {
-			if G.Pkg == nil {
+			if ck.MkLines.pkg == nil {
 				return false
 			}
 
-			varinfo := G.Pkg.redundant.vars[varname]
+			varinfo := ck.MkLines.pkg.redundant.vars[varname]
 			if varinfo == nil || !varinfo.vari.IsConstant() {
 				return false
 			}
@@ -645,6 +669,40 @@ func (ck *MkVarUseChecker) warnRedundant
 	fix.Apply()
 }
 
+func (ck *MkVarUseChecker) checkToolsPlatform() {
+	if ck.MkLine.IsDirective() {
+		return
+	}
+
+	varname := ck.use.varname
+	if varnameCanon(varname) != "TOOLS_PLATFORM.*" {
+		return
+	}
+
+	indentation := ck.MkLines.indentation
+	switch {
+	case indentation.DependsOn("OPSYS"),
+		indentation.DependsOn("MACHINE_PLATFORM"),
+		indentation.DependsOn(varname):
+		// TODO: Only return if the conditional is on the correct OPSYS.
+		return
+	}
+
+	toolName := varnameParam(varname)
+	tool := G.Pkgsrc.Tools.ByName(toolName)
+	if tool == nil {
+		return
+	}
+
+	if len(tool.undefinedOn) > 0 {
+		ck.MkLine.Warnf("%s is undefined on %s.",
+			varname, joinCambridge("and", tool.undefinedOn...))
+	} else if len(tool.conditionalOn) > 0 {
+		ck.MkLine.Warnf("%s may be undefined on %s.",
+			varname, joinCambridge("and", tool.conditionalOn...))
+	}
+}
+
 func (ck *MkVarUseChecker) checkBuildDefs() {
 	varname := ck.use.varname