Add a buildifier warning to enforce symbol load location

WARNINGS.md

@@ -2,6 +2,7 @@
 
 Warning categories supported by buildifier's linter:
 
+  * [`allowed-symbol-load-locations`](#allowed-symbol-load-locations)
   * [`attr-applicable_licenses`](#attr-applicable_licenses)
   * [`attr-cfg`](#attr-cfg)
   * [`attr-license`](#attr-license)
@@ -128,6 +129,27 @@ if debug:
 
 --------------------------------------------------------------------------------
 
+## <a name="allowed-symbol-load-locations"></a>Symbol must be loaded from a specific location
+
+  * Category name: `allowed-symbol-load-locations`
+  * Automatic fix: no
+  * [Suppress the warning](#suppress): `# buildifier: disable=allowed-symbol-load-locations`
+
+Warns when a symbol is loaded from a location other than the expected ones.
+Expected locations are specified in the tables file:
+
+```json
+{
+  "AllowedSymbolLoadLocations": {
+    "genrule": [
+      "//tools/bazel:genrule.bzl"
+    ]
+  }
+}
+```
+
+--------------------------------------------------------------------------------
+
 ## <a name="attr-applicable_licenses"></a>Do not use `applicable_licenses` as an attribute name.
 
   * Category name: `attr-applicable_licenses`

buildifier/config/config_test.go

@@ -43,6 +43,7 @@ func ExampleExample() {
 	//   "mode": "fix",
 	//   "lint": "fix",
 	//   "warningsList": [
+	//     "allowed-symbol-load-locations",
 	//     "attr-applicable_licenses",
 	//     "attr-cfg",
 	//     "attr-license",
@@ -263,6 +264,7 @@ func TestValidate(t *testing.T) {
 		"type auto":             {options: "--type=auto"},
 		"type error":            {options: "--type=foo", wantErr: fmt.Errorf("unrecognized input type foo; valid types are build, bzl, workspace, default, module, auto")},
 		"warnings all": {options: "--warnings=all", wantWarnings: []string{
+			"allowed-symbol-load-locations",
 			"attr-applicable_licenses",
 			"attr-cfg",
 			"attr-license",
@@ -364,6 +366,7 @@ func TestValidate(t *testing.T) {
 			"unused-variable",
 		}},
 		"warnings default": {options: "--warnings=default", wantWarnings: []string{
+			"allowed-symbol-load-locations",
 			"attr-applicable_licenses",
 			"attr-cfg",
 			"attr-license",
@@ -465,6 +468,7 @@ func TestValidate(t *testing.T) {
 			"unused-variable",
 		}},
 		"warnings plus/minus": {options: "--warnings=+unsorted-dict-items,-print,-deprecated-function", wantWarnings: []string{
+			"allowed-symbol-load-locations",
 			"attr-applicable_licenses",
 			"attr-cfg",
 			"attr-license",
@@ -556,7 +560,6 @@ func TestValidate(t *testing.T) {
 			"repository-name",
 			"return-value",
 			"rule-impl-return",
-
 			"skylark-comment",
 			"skylark-docstring",
 			"string-iteration",
@@ -567,6 +570,7 @@ func TestValidate(t *testing.T) {
 			"unused-variable",
 		}},
 		"warnings no duplicates": {options: "--warnings=+unused-variable", wantWarnings: []string{
+			"allowed-symbol-load-locations",
 			"attr-applicable_licenses",
 			"attr-cfg",
 			"attr-license",
@@ -658,7 +662,6 @@ func TestValidate(t *testing.T) {
 			"repository-name",
 			"return-value",
 			"rule-impl-return",
-
 			"skylark-comment",
 			"skylark-docstring",
 			"string-iteration",

buildifier/integration_test.sh

@@ -258,6 +258,7 @@ cat > golden/.buildifier.example.json <<EOF
   "mode": "fix",
   "lint": "fix",
   "warningsList": [
+    "allowed-symbol-load-locations",
     "attr-applicable_licenses",
     "attr-cfg",
     "attr-license",
@@ -691,3 +692,36 @@ EOF
 
 $buildifier --lint=warn --warnings=deprecated-function BUILD 2> report || ret=$?
 diff -u report_golden report || die "$1: wrong console output for multifile warnings (WORKSPACE exists)"
+
+cd ..
+
+# Test allowed symbol load locations
+
+mkdir test_dir/allowed_locations
+cd test_dir/allowed_locations
+
+cat > BUILD <<EOF
+load(":f.bzl", "s1", "s2")
+load(":a.bzl", "s3")
+load(":a.bzl", "s4")
+EOF
+
+cat > buildifier.tables <<EOF
+{
+  "AllowedSymbolLoadLocations": {
+    "s1": [":z.bzl"],
+    "s3": [":y.bzl", ":x.bzl"],
+    "s4": [":a.bzl"]
+  }
+}
+EOF
+
+cat > report_golden <<EOF
+BUILD:1: allowed-symbol-load-locations: Symbol "s1" must be loaded from :z.bzl. (https://github.com/bazelbuild/buildtools/blob/main/WARNINGS.md#allowed-symbol-load-locations)
+BUILD:2: allowed-symbol-load-locations: Symbol "s3" must be loaded from one of the allowed locations: :x.bzl, :y.bzl. (https://github.com/bazelbuild/buildtools/blob/main/WARNINGS.md#allowed-symbol-load-locations)
+EOF
+
+$buildifier --lint=warn --warnings=allowed-symbol-load-locations -tables=buildifier.tables BUILD 2> report || true
+diff -u report_golden report || die "$1: wrong console output for allowed symbol load locations"
+
+cd ../..

tables/jsonparser.go

@@ -31,6 +31,7 @@ type Definitions struct {
 	NamePriority                    map[string]int
 	StripLabelLeadingSlashes        bool
 	ShortenAbsoluteLabelsToRelative bool
+	AllowedSymbolLoadLocations      map[string][]string
 }
 
 // ParseJSONDefinitions reads and parses JSON table definitions from file.
@@ -55,9 +56,9 @@ func ParseAndUpdateJSONDefinitions(file string, merge bool) error {
 	}
 
 	if merge {
-		MergeTables(definitions.IsLabelArg, definitions.LabelDenylist, definitions.IsListArg, definitions.IsSortableListArg, definitions.SortableDenylist, definitions.SortableAllowlist, definitions.NamePriority, definitions.StripLabelLeadingSlashes, definitions.ShortenAbsoluteLabelsToRelative)
+		MergeTables(definitions.IsLabelArg, definitions.LabelDenylist, definitions.IsListArg, definitions.IsSortableListArg, definitions.SortableDenylist, definitions.SortableAllowlist, definitions.NamePriority, definitions.StripLabelLeadingSlashes, definitions.ShortenAbsoluteLabelsToRelative, definitions.AllowedSymbolLoadLocations)
 	} else {
-		OverrideTables(definitions.IsLabelArg, definitions.LabelDenylist, definitions.IsListArg, definitions.IsSortableListArg, definitions.SortableDenylist, definitions.SortableAllowlist, definitions.NamePriority, definitions.StripLabelLeadingSlashes, definitions.ShortenAbsoluteLabelsToRelative)
+		OverrideTables(definitions.IsLabelArg, definitions.LabelDenylist, definitions.IsListArg, definitions.IsSortableListArg, definitions.SortableDenylist, definitions.SortableAllowlist, definitions.NamePriority, definitions.StripLabelLeadingSlashes, definitions.ShortenAbsoluteLabelsToRelative, definitions.AllowedSymbolLoadLocations)
 	}
 	return nil
 }

tables/jsonparser_test.go

@@ -30,13 +30,14 @@ func TestParseJSONDefinitions(t *testing.T) {
 	}
 
 	expected := Definitions{
-		IsLabelArg:               map[string]bool{"srcs": true},
-		LabelDenylist:            map[string]bool{},
-		IsSortableListArg:        map[string]bool{"srcs": true, "visibility": true},
-		SortableDenylist:         map[string]bool{"genrule.srcs": true},
-		SortableAllowlist:        map[string]bool{},
-		NamePriority:             map[string]int{"name": -1},
-		StripLabelLeadingSlashes: true,
+		IsLabelArg:                 map[string]bool{"srcs": true},
+		LabelDenylist:              map[string]bool{},
+		IsSortableListArg:          map[string]bool{"srcs": true, "visibility": true},
+		SortableDenylist:           map[string]bool{"genrule.srcs": true},
+		SortableAllowlist:          map[string]bool{},
+		NamePriority:               map[string]int{"name": -1},
+		StripLabelLeadingSlashes:   true,
+		AllowedSymbolLoadLocations: map[string][]string{"genrule": {"//tools/bazel:genrule.bzl"}},
 	}
 	if !reflect.DeepEqual(expected, definitions) {
 		t.Errorf("ParseJSONDefinitions(simple_tables.json) = %v; want %v", definitions, expected)

tables/tables.go

@@ -265,8 +265,11 @@ var IsModuleOverride = map[string]bool{
 	"single_version_override":   true,
 }
 
+// AllowedSymbolLoadLocations contains locations for loading rules that are allowed to be used.
+var AllowedSymbolLoadLocations = map[string]map[string]bool{}
+
 // OverrideTables allows a user of the build package to override the special-case rules. The user-provided tables replace the built-in tables.
-func OverrideTables(labelArg, denylist, listArg, sortableListArg, sortDenylist, sortAllowlist map[string]bool, namePriority map[string]int, stripLabelLeadingSlashes, shortenAbsoluteLabelsToRelative bool) {
+func OverrideTables(labelArg, denylist, listArg, sortableListArg, sortDenylist, sortAllowlist map[string]bool, namePriority map[string]int, stripLabelLeadingSlashes, shortenAbsoluteLabelsToRelative bool, symbolLoadLocation map[string][]string) {
 	IsLabelArg = labelArg
 	LabelDenylist = denylist
 	IsListArg = listArg
@@ -276,10 +279,19 @@ func OverrideTables(labelArg, denylist, listArg, sortableListArg, sortDenylist,
 	NamePriority = namePriority
 	StripLabelLeadingSlashes = stripLabelLeadingSlashes
 	ShortenAbsoluteLabelsToRelative = shortenAbsoluteLabelsToRelative
+
+	AllowedSymbolLoadLocations = map[string]map[string]bool{}
+	for k, v := range symbolLoadLocation {
+		locations := map[string]bool{}
+		for _, l := range v {
+			locations[l] = true
+		}
+		AllowedSymbolLoadLocations[k] = locations
+	}
 }
 
 // MergeTables allows a user of the build package to override the special-case rules. The user-provided tables are merged into the built-in tables.
-func MergeTables(labelArg, denylist, listArg, sortableListArg, sortDenylist, sortAllowlist map[string]bool, namePriority map[string]int, stripLabelLeadingSlashes, shortenAbsoluteLabelsToRelative bool) {
+func MergeTables(labelArg, denylist, listArg, sortableListArg, sortDenylist, sortAllowlist map[string]bool, namePriority map[string]int, stripLabelLeadingSlashes, shortenAbsoluteLabelsToRelative bool, symbolLoadLocation map[string][]string) {
 	for k, v := range labelArg {
 		IsLabelArg[k] = v
 	}
@@ -303,4 +315,12 @@ func MergeTables(labelArg, denylist, listArg, sortableListArg, sortDenylist, sor
 	}
 	StripLabelLeadingSlashes = stripLabelLeadingSlashes || StripLabelLeadingSlashes
 	ShortenAbsoluteLabelsToRelative = shortenAbsoluteLabelsToRelative || ShortenAbsoluteLabelsToRelative
+
+	for k, v := range symbolLoadLocation {
+		locations := map[string]bool{}
+		for _, l := range v {
+			locations[l] = true
+		}
+		AllowedSymbolLoadLocations[k] = locations
+	}
 }

tables/testdata/simple_tables.json

@@ -19,5 +19,10 @@
   "NamePriority": {
     "name": -1
   },
-  "StripLabelLeadingSlashes": true
+  "StripLabelLeadingSlashes": true,
+  "AllowedSymbolLoadLocations": {
+    "genrule": [
+      "//tools/bazel:genrule.bzl"
+    ]
+  }
 }

warn/BUILD.bazel

@@ -13,6 +13,7 @@ go_library(
         "warn_cosmetic.go",
         "warn_deprecated.go",
         "warn_docstring.go",
+        "warn_load.go",
         "warn_macro.go",
         "warn_naming.go",
         "warn_operation.go",
@@ -42,6 +43,7 @@ go_test(
         "warn_cosmetic_test.go",
         "warn_deprecated_test.go",
         "warn_docstring_test.go",
+        "warn_load_test.go",
         "warn_macro_test.go",
         "warn_naming_test.go",
         "warn_operation_test.go",

warn/docs/warnings.textproto

@@ -2,6 +2,23 @@
 # proto-message: Warnings
 # After modifying this file, run `bazel build //warn/docs:warnings_docs && cp bazel-bin/warn/docs/WARNINGS.md .`
 
+warnings: {
+  name: "allowed-symbol-load-locations"
+  header: "Symbol must be loaded from a specific location"
+  description:
+      "Warns when a symbol is loaded from a location other than the expected ones.\n"
+          "Expected locations are specified in the tables file:\n\n"
+          "```json\n"
+          "{\n"
+          "  \"AllowedSymbolLoadLocations\": {\n"
+          "    \"genrule\": [\n"
+          "      \"//tools/bazel:genrule.bzl\"\n"
+          "    ]\n"
+          "  }\n"
+          "}\n"
+          "```"
+}
+
 warnings: {
   name: "attr-cfg"
   header: "`cfg = \"data\"` for attr definitions has no effect"

warn/warn.go

@@ -116,57 +116,58 @@ var RuleWarningMap = map[string]func(call *build.CallExpr, pkg string) *LinterFi
 
 // FileWarningMap lists the warnings that run on the whole file.
 var FileWarningMap = map[string]func(f *build.File) []*LinterFinding{
-	"attr-applicable_licenses":  attrApplicableLicensesWarning,
-	"attr-cfg":                  attrConfigurationWarning,
-	"attr-license":              attrLicenseWarning,
-	"attr-licenses":             attrLicensesWarning,
-	"attr-non-empty":            attrNonEmptyWarning,
-	"attr-output-default":       attrOutputDefaultWarning,
-	"attr-single-file":          attrSingleFileWarning,
-	"build-args-kwargs":         argsKwargsInBuildFilesWarning,
-	"canonical-repository":      canonicalRepositoryWarning,
-	"confusing-name":            confusingNameWarning,
-	"constant-glob":             constantGlobWarning,
-	"ctx-actions":               ctxActionsWarning,
-	"ctx-args":                  contextArgsAPIWarning,
-	"depset-items":              depsetItemsWarning,
-	"depset-iteration":          depsetIterationWarning,
-	"depset-union":              depsetUnionWarning,
-	"dict-method-named-arg":     dictMethodNamedArgWarning,
-	"dict-concatenation":        dictionaryConcatenationWarning,
-	"duplicated-name":           duplicatedNameWarning,
-	"external-path":             externalPathWarning,
-	"filetype":                  fileTypeWarning,
-	"function-docstring":        functionDocstringWarning,
-	"function-docstring-header": functionDocstringHeaderWarning,
-	"function-docstring-args":   functionDocstringArgsWarning,
-	"function-docstring-return": functionDocstringReturnWarning,
-	"integer-division":          integerDivisionWarning,
-	"keyword-positional-params": keywordPositionalParametersWarning,
-	"list-append":               listAppendWarning,
-	"load":                      unusedLoadWarning,
-	"module-docstring":          moduleDocstringWarning,
-	"name-conventions":          nameConventionsWarning,
-	"native-build":              nativeInBuildFilesWarning,
-	"native-package":            nativePackageWarning,
-	"no-effect":                 noEffectWarning,
-	"output-group":              outputGroupWarning,
-	"overly-nested-depset":      overlyNestedDepsetWarning,
-	"package-name":              packageNameWarning,
-	"package-on-top":            packageOnTopWarning,
-	"print":                     printWarning,
-	"provider-params":           providerParamsWarning,
-	"redefined-variable":        redefinedVariableWarning,
-	"repository-name":           repositoryNameWarning,
-	"rule-impl-return":          ruleImplReturnWarning,
-	"return-value":              missingReturnValueWarning,
-	"skylark-comment":           skylarkCommentWarning,
-	"skylark-docstring":         skylarkDocstringWarning,
-	"string-iteration":          stringIterationWarning,
-	"uninitialized":             uninitializedVariableWarning,
-	"unreachable":               unreachableStatementWarning,
-	"unsorted-dict-items":       unsortedDictItemsWarning,
-	"unused-variable":           unusedVariableWarning,
+	"allowed-symbol-load-locations": symbolLoadLocationWarning,
+	"attr-applicable_licenses":      attrApplicableLicensesWarning,
+	"attr-cfg":                      attrConfigurationWarning,
+	"attr-license":                  attrLicenseWarning,
+	"attr-licenses":                 attrLicensesWarning,
+	"attr-non-empty":                attrNonEmptyWarning,
+	"attr-output-default":           attrOutputDefaultWarning,
+	"attr-single-file":              attrSingleFileWarning,
+	"build-args-kwargs":             argsKwargsInBuildFilesWarning,
+	"canonical-repository":          canonicalRepositoryWarning,
+	"confusing-name":                confusingNameWarning,
+	"constant-glob":                 constantGlobWarning,
+	"ctx-actions":                   ctxActionsWarning,
+	"ctx-args":                      contextArgsAPIWarning,
+	"depset-items":                  depsetItemsWarning,
+	"depset-iteration":              depsetIterationWarning,
+	"depset-union":                  depsetUnionWarning,
+	"dict-method-named-arg":         dictMethodNamedArgWarning,
+	"dict-concatenation":            dictionaryConcatenationWarning,
+	"duplicated-name":               duplicatedNameWarning,
+	"external-path":                 externalPathWarning,
+	"filetype":                      fileTypeWarning,
+	"function-docstring":            functionDocstringWarning,
+	"function-docstring-header":     functionDocstringHeaderWarning,
+	"function-docstring-args":       functionDocstringArgsWarning,
+	"function-docstring-return":     functionDocstringReturnWarning,
+	"integer-division":              integerDivisionWarning,
+	"keyword-positional-params":     keywordPositionalParametersWarning,
+	"list-append":                   listAppendWarning,
+	"load":                          unusedLoadWarning,
+	"module-docstring":              moduleDocstringWarning,
+	"name-conventions":              nameConventionsWarning,
+	"native-build":                  nativeInBuildFilesWarning,
+	"native-package":                nativePackageWarning,
+	"no-effect":                     noEffectWarning,
+	"output-group":                  outputGroupWarning,
+	"overly-nested-depset":          overlyNestedDepsetWarning,
+	"package-name":                  packageNameWarning,
+	"package-on-top":                packageOnTopWarning,
+	"print":                         printWarning,
+	"provider-params":               providerParamsWarning,
+	"redefined-variable":            redefinedVariableWarning,
+	"repository-name":               repositoryNameWarning,
+	"rule-impl-return":              ruleImplReturnWarning,
+	"return-value":                  missingReturnValueWarning,
+	"skylark-comment":               skylarkCommentWarning,
+	"skylark-docstring":             skylarkDocstringWarning,
+	"string-iteration":              stringIterationWarning,
+	"uninitialized":                 uninitializedVariableWarning,
+	"unreachable":                   unreachableStatementWarning,
+	"unsorted-dict-items":           unsortedDictItemsWarning,
+	"unused-variable":               unusedVariableWarning,
 }
 
 // MultiFileWarningMap lists the warnings that run on the whole file, but may use other files.