Add make-location lint for deprecated $(location) make variables

BUILD.bazel

@@ -1,9 +1,10 @@
 load("@bazel_gazelle//:def.bzl", "gazelle")
-load("//buildifier:def.bzl", "buildifier")
+load("//buildifier:def.bzl", "buildifier", "buildifier_test")
 
 exports_files([
     "LICENSE",
     "launcher.js",
+    "WORKSPACE",
 ])
 
 config_setting(
@@ -20,6 +21,7 @@ filegroup(
 test_suite(
     name = "tests",
     tests = [
+        ":make_location_lint",
         "//api_proto:api.gen.pb.go_checkshtest",
         "//build:build_test",
         "//build_proto:build.gen.pb.go_checkshtest",
@@ -45,3 +47,11 @@ gazelle(
 buildifier(
     name = "buildifier",
 )
+
+buildifier_test(
+    name = "make_location_lint",
+    lint_mode = "warn",
+    lint_warnings = ["make-location"],
+    no_sandbox = True,
+    workspace = "//:WORKSPACE",
+)

WARNINGS.md

@@ -38,6 +38,7 @@ Warning categories supported by buildifier's linter:
   * [`list-append`](#list-append)
   * [`load`](#load)
   * [`load-on-top`](#load-on-top)
+  * [`make-location`](#make-location)
   * [`module-docstring`](#module-docstring)
   * [`name-conventions`](#name-conventions)
   * [`native-android`](#native-android)
@@ -748,6 +749,36 @@ they can follow only comments and docstrings.
 
 --------------------------------------------------------------------------------
 
+## <a name="make-location"></a>The `$(location)` make variable is deprecated
+
+  * Category name: `make-location`
+  * Automatic fix: no
+  * [Suppress the warning](#suppress): `# buildifier: disable=make-location`
+
+The `$(location)` and `$(locations)` make variables are legacy synonyms for
+`$(execpath)` and `$(rootpath)` whose behavior depends on the attribute being
+expanded. Use `$(execpath ...)` when you need the execution path, or
+`$(rootpath ...)` when you need the runfiles path.
+
+Examples that trigger this warning:
+
+```python
+genrule(
+    name = "example",
+    srcs = [":input"],
+    outs = ["output"],
+    cmd = "cp $(location :input) $@",
+)
+```
+
+Instead, use an explicit make variable:
+
+```python
+cmd = "cp $(execpath :input) $@",
+```
+
+--------------------------------------------------------------------------------
+
 ## <a name="module-docstring"></a>The file has no module docstring
 
   * Category name: `module-docstring`

build/build_defs.bzl

@@ -144,15 +144,15 @@ eof
         srcs = [src + "_check.sh"],
         deps = ["@bazel_tools//tools/bash/runfiles"],
         data = [src, gen],
-        args = ["$(location " + src + ")", "$(location " + gen + ")"],
+        args = ["$(rootpath " + src + ")", "$(rootpath " + gen + ")"],
     )
 
     # magic copy rule used to update the checked-in version
     native.genrule(
         name = src + "_copysh",
         srcs = [gen],
         outs = [src + "copy.sh"],
-        cmd = "echo 'cp $${BUILD_WORKSPACE_DIRECTORY}/$(location " + gen +
+        cmd = "echo 'cp $${BUILD_WORKSPACE_DIRECTORY}/$(execpath " + gen +
               ") $${BUILD_WORKSPACE_DIRECTORY}/" + native.package_name() + "/" + src + "' > $@",
     )
     sh_binary(

buildifier/BUILD.bazel

@@ -93,7 +93,7 @@ sh_test(
     size = "small",
     srcs = ["integration_test.sh"],
     args = [
-        "$(location :buildifier)",
+        "$(rootpath :buildifier)",
     ],
     data = [
         ":buildifier",

buildifier/config/config_test.go

@@ -77,6 +77,7 @@ func ExampleExample() {
 	//     "keyword-positional-params",
 	//     "list-append",
 	//     "load",
+	//     "make-location",
 	//     "module-docstring",
 	//     "name-conventions",
 	//     "native-android",
@@ -298,6 +299,7 @@ func TestValidate(t *testing.T) {
 			"keyword-positional-params",
 			"list-append",
 			"load",
+			"make-location",
 			"module-docstring",
 			"name-conventions",
 			"native-android",
@@ -400,6 +402,7 @@ func TestValidate(t *testing.T) {
 			"keyword-positional-params",
 			"list-append",
 			"load",
+			"make-location",
 			"module-docstring",
 			"name-conventions",
 			"native-android",
@@ -502,6 +505,7 @@ func TestValidate(t *testing.T) {
 			"keyword-positional-params",
 			"list-append",
 			"load",
+			"make-location",
 			"module-docstring",
 			"name-conventions",
 			"native-android",
@@ -604,6 +608,7 @@ func TestValidate(t *testing.T) {
 			"keyword-positional-params",
 			"list-append",
 			"load",
+			"make-location",
 			"module-docstring",
 			"name-conventions",
 			"native-android",

buildifier/integration_test.sh

@@ -292,6 +292,7 @@ cat > golden/.buildifier.example.json <<EOF
     "keyword-positional-params",
     "list-append",
     "load",
+    "make-location",
     "module-docstring",
     "name-conventions",
     "native-android",

buildozer/BUILD.bazel

@@ -28,7 +28,7 @@ sh_test(
     size = "small",
     srcs = ["buildozer_test.sh"],
     args = [
-        "$(location :buildozer)",
+        "$(rootpath :buildozer)",
     ],
     data = [
         "test_common.sh",

warn/docs/warnings.textproto

@@ -501,6 +501,30 @@ warnings: {
   autofix: true
 }
 
+warnings: {
+  name: "make-location"
+  header: "The `$(location)` make variable is deprecated"
+  description:
+    "The `$(location)` and `$(locations)` make variables are legacy synonyms for\n"
+    "`$(execpath)` and `$(rootpath)` whose behavior depends on the attribute being\n"
+    "expanded. Use `$(execpath ...)` when you need the execution path, or\n"
+    "`$(rootpath ...)` when you need the runfiles path.\n\n"
+    "Examples that trigger this warning:\n\n"
+    "```python\n"
+    "genrule(\n"
+    "    name = \"example\",\n"
+    "    srcs = [\":input\"],\n"
+    "    outs = [\"output\"],\n"
+    "    cmd = \"cp $(location :input) $@\",\n"
+    ")\n"
+    "```\n\n"
+    "Instead, use an explicit make variable:\n\n"
+    "```python\n"
+    "cmd = \"cp $(execpath :input) $@\",\n"
+    "```"
+  autofix: false
+}
+
 warnings: {
   name: "module-docstring"
   header: "The file has no module docstring"

warn/warn.go

@@ -146,6 +146,7 @@ var FileWarningMap = map[string]func(f *build.File) []*LinterFinding{
 	"keyword-positional-params":     keywordPositionalParametersWarning,
 	"list-append":                   listAppendWarning,
 	"load":                          unusedLoadWarning,
+	"make-location":                 makeLocationVariableWarning,
 	"module-docstring":              moduleDocstringWarning,
 	"name-conventions":              nameConventionsWarning,
 	"native-build":                  nativeInBuildFilesWarning,

warn/warn_bazel.go

@@ -20,11 +20,15 @@ package warn
 
 import (
 	"fmt"
+	"regexp"
 	"strings"
 
 	"github.com/bazelbuild/buildtools/build"
 )
 
+// locationMakeVariableRe matches deprecated $(location) and $(locations) make variables.
+var locationMakeVariableRe = regexp.MustCompile(`\$\(locations?(?:\s[^)]*)?\)`)
+
 func constantGlobPatternWarning(patterns *build.ListExpr) []*LinterFinding {
 	findings := []*LinterFinding{}
 	for _, expr := range patterns.List {
@@ -256,6 +260,26 @@ func printWarning(f *build.File) []*LinterFinding {
 	return findings
 }
 
+func makeLocationVariableWarning(f *build.File) []*LinterFinding {
+	if f.Type != build.TypeBuild {
+		return nil
+	}
+
+	findings := []*LinterFinding{}
+	build.Walk(f, func(expr build.Expr, stack []build.Expr) {
+		stringExpr, ok := expr.(*build.StringExpr)
+		if !ok {
+			return
+		}
+		if locationMakeVariableRe.MatchString(stringExpr.Value) {
+			findings = append(findings,
+				makeLinterFinding(stringExpr,
+					`The "$(location)" and "$(locations)" make variables are deprecated. Use "$(execpath ...)" or "$(rootpath ...)" instead.`))
+		}
+	})
+	return findings
+}
+
 func externalPathWarning(f *build.File) []*LinterFinding {
 	if f.Type == build.TypeDefault {
 		// Only applicable to Bazel files

warn/warn_bazel_test.go

@@ -289,3 +289,41 @@ py_binary(
 		},
 		scopeBazel)
 }
+
+func TestMakeLocationVariable(t *testing.T) {
+	checkFindings(t, "make-location", `
+genrule(
+    name = "a",
+    srcs = [":foo"],
+    outs = ["out"],
+    cmd = "cp $(location :foo) $@",
+)
+
+genrule(
+    name = "b",
+    srcs = [":a", ":b"],
+    outs = ["out"],
+    cmd = "cat $(locations :a :b) > $@",
+)
+
+genrule(
+    name = "c",
+    srcs = [":foo"],
+    outs = ["out"],
+    cmd = "cp $$(location :foo) $$@",
+)
+
+cc_test(
+    name = "d",
+    args = ["--config=$(execpath :cfg)"],
+)
+
+load("location", "symbol")
+`,
+		[]string{
+			`:5: The "$(location)" and "$(locations)" make variables are deprecated. Use "$(execpath ...)" or "$(rootpath ...)" instead.`,
+			`:12: The "$(location)" and "$(locations)" make variables are deprecated. Use "$(execpath ...)" or "$(rootpath ...)" instead.`,
+			`:19: The "$(location)" and "$(locations)" make variables are deprecated. Use "$(execpath ...)" or "$(rootpath ...)" instead.`,
+		},
+		scopeBuild)
+}