Rename to AllowedSymbolLoadLocations

Author: styurin

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)
@@ -96,7 +97,6 @@ Warning categories supported by buildifier's linter:
   * [`repository-name`](#repository-name)
   * [`return-value`](#return-value)
   * [`rule-impl-return`](#rule-impl-return)
-  * [`rule-load-location`](#rule-load-location)
   * [`same-origin-load`](#same-origin-load)
   * [`skylark-comment`](#skylark-comment)
   * [`skylark-docstring`](#skylark-docstring)
@@ -129,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`
@@ -1341,26 +1362,6 @@ consider using
 [providers](https://docs.bazel.build/versions/main/skylark/rules.html#providers)
 or lists of providers instead.
 
---------------------------------------------------------------------------------
-
-## <a name="rule-load-location"></a>Rule must be loaded from a specific location
-
-  * Category name: `rule-load-location`
-  * Automatic fix: no
-  * [Suppress the warning](#suppress): `# buildifier: disable=rule-load-location`
-
-Warns when a rule is loaded from a location other than the expected one.
-Expected locations are specified in the tables file:
-
-```json
-{
-  "RuleLoadLocation": {
-    "genrule": "//tools/bazel:genrule.bzl"
-  }
-}
-```
-
-
 --------------------------------------------------------------------------------
 
 ## <a name="same-origin-load"></a>Same label is used for multiple loads

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",
@@ -134,7 +135,6 @@ func ExampleExample() {
 	//     "repository-name",
 	//     "return-value",
 	//     "rule-impl-return",
-	//     "rule-load-location",
 	//     "skylark-comment",
 	//     "skylark-docstring",
 	//     "string-iteration",
@@ -264,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",
@@ -355,7 +356,6 @@ func TestValidate(t *testing.T) {
 			"repository-name",
 			"return-value",
 			"rule-impl-return",
-			"rule-load-location",
 			"skylark-comment",
 			"skylark-docstring",
 			"string-iteration",
@@ -366,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",
@@ -457,7 +458,6 @@ func TestValidate(t *testing.T) {
 			"repository-name",
 			"return-value",
 			"rule-impl-return",
-			"rule-load-location",
 			"skylark-comment",
 			"skylark-docstring",
 			"string-iteration",
@@ -468,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",
@@ -559,7 +560,6 @@ func TestValidate(t *testing.T) {
 			"repository-name",
 			"return-value",
 			"rule-impl-return",
-			"rule-load-location",
 			"skylark-comment",
 			"skylark-docstring",
 			"string-iteration",
@@ -570,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",
@@ -661,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",
@@ -349,7 +350,6 @@ cat > golden/.buildifier.example.json <<EOF
     "repository-name",
     "return-value",
     "rule-impl-return",
-    "rule-load-location",
     "skylark-comment",
     "skylark-docstring",
     "string-iteration",
@@ -692,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,7 +31,7 @@ type Definitions struct {
 	NamePriority                    map[string]int
 	StripLabelLeadingSlashes        bool
 	ShortenAbsoluteLabelsToRelative bool
-	RuleLoadLocation                map[string]string
+	AllowedSymbolLoadLocations      map[string][]string
 }
 
 // ParseJSONDefinitions reads and parses JSON table definitions from file.
@@ -56,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, definitions.RuleLoadLocation)
+		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, definitions.RuleLoadLocation)
+		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,14 +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,
-		RuleLoadLocation:         map[string]string{"genrule": "//tools/bazel:genrule.bzl"},
+		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,11 +265,11 @@ var IsModuleOverride = map[string]bool{
 	"single_version_override":   true,
 }
 
-// RuleLoadLocation contains custom locations for loading rules.
-var RuleLoadLocation = map[string]string{}
+// 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, ruleLoadLocation map[string]string) {
+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
@@ -279,11 +279,19 @@ func OverrideTables(labelArg, denylist, listArg, sortableListArg, sortDenylist,
 	NamePriority = namePriority
 	StripLabelLeadingSlashes = stripLabelLeadingSlashes
 	ShortenAbsoluteLabelsToRelative = shortenAbsoluteLabelsToRelative
-	RuleLoadLocation = ruleLoadLocation
+
+	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, ruleLoadLocation map[string]string) {
+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
 	}
@@ -307,7 +315,12 @@ func MergeTables(labelArg, denylist, listArg, sortableListArg, sortDenylist, sor
 	}
 	StripLabelLeadingSlashes = stripLabelLeadingSlashes || StripLabelLeadingSlashes
 	ShortenAbsoluteLabelsToRelative = shortenAbsoluteLabelsToRelative || ShortenAbsoluteLabelsToRelative
-	for k, v := range ruleLoadLocation {
-		RuleLoadLocation[k] = v
+
+	for k, v := range symbolLoadLocation {
+		locations := map[string]bool{}
+		for _, l := range v {
+			locations[l] = true
+		}
+		AllowedSymbolLoadLocations[k] = locations
 	}
 }

tables/testdata/simple_tables.json

@@ -20,7 +20,9 @@
     "name": -1
   },
   "StripLabelLeadingSlashes": true,
-  "RuleLoadLocation": {
-    "genrule": "//tools/bazel:genrule.bzl"
+  "AllowedSymbolLoadLocations": {
+    "genrule": [
+      "//tools/bazel:genrule.bzl"
+    ]
   }
 }

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"
@@ -983,21 +1000,6 @@ warnings: {
     "or lists of providers instead."
 }
 
-warnings: {
-  name: "rule-load-location"
-  header: "Rule must be loaded from a specific location"
-  description:
-      "Warns when a rule is loaded from a location other than the expected one.\n"
-      "Expected locations are specified in the tables file:\n\n"
-  "```json\n"
-  "{\n"
-  "  \"RuleLoadLocation\": {\n"
-  "    \"genrule\": \"//tools/bazel:genrule.bzl\"\n"
-  "  }\n"
-  "}\n"
-  "```\n"
-}
-
 warnings: {
   name: "same-origin-load"
   header: "Same label is used for multiple loads"