Repository: bazelbuild/buildtools
Pull request #429 (2018-11-14 18:48:24)

[buildifier] Warn and fix loads from the same file.
Loads like
```
load(":f.bzl", "s1")
load(":f.bzl", "s2")
```
can be simplified as
```
load(":f.bzl", "s1", "s2")
```
which is more concise and less error prone.

vladmos: Please also add the warning description to the WARNINGS.md file.
ttsugriy: > Please also add the warning description to the WARNINGS.md file.

@vladmos, I actually had a warning description initially, but I removed it because I somehow got under impression that those were for buildozer fix instead of buildifier, since there is only one reference to buildifier and a lot of reference including command examples for buildozer :) Now that you've removed the confusion for me, I readded the warning description.

Patch:
diff --git a/WARNINGS.md b/WARNINGS.md
index d5fa9ad..0c21d67 100644
--- a/WARNINGS.md
+++ b/WARNINGS.md
@@ -141,6 +141,34 @@ or at the beginning of a rule.
 
 --------------------------------------------------------------------------------
 
+## <a name="same-origin-load"></a>Same label is used for multiple loads
+
+  * Category_name: `same-origin-load`
+  * Automatic fix: yes
+
+### Background
+
+[load](https://docs.bazel.build/versions/master/skylark/concepts.html#loading-an-extension)
+is used to import definitions in a BUILD file. If the same label is used for loading
+symbols more the ones, all such loads can be merged into a single one.
+
+### How to fix it
+
+Merge all loads into a single one. For example,
+
+```
+load(":f.bzl", "s1")
+load(":f.bzl", "s2")
+```
+
+can be written more compactly as
+
+```
+load(":f.bzl", "s1", "s2")
+```
+
+--------------------------------------------------------------------------------
+
 ## <a name="unused-variable"></a>Variable is unused
 
   * Category_name: `unused-variable`
@@ -399,9 +427,9 @@ copy the data to it. There are several ways to avoid it, for example, instead of
 you can use one of the following:
 
   * Use [Skylib](https://github.com/bazelbuild/bazel-skylib):
-  
+
     load("@bazel_skylib//lib/dicts.bzl", "dicts")
-    
+
     d = dicts.add(d1, d2, d3)
 
   * The same if you don't want to use Skylib:
diff --git a/warn/warn.go b/warn/warn.go
index e035455..08b6a04 100644
--- a/warn/warn.go
+++ b/warn/warn.go
@@ -169,6 +169,36 @@ func unusedLoadWarning(f *build.File, fix bool) []*Finding {
 	return findings
 }
 
+func sameOriginLoadWarning(f *build.File, fix bool) []*Finding {
+	findings := []*Finding{}
+	loaded := make(map[string]*build.LoadStmt)
+	for stmtIndex := 0; stmtIndex < len(f.Stmt); stmtIndex++ {
+		load, ok := f.Stmt[stmtIndex].(*build.LoadStmt)
+		if !ok {
+			continue
+		}
+
+		previousLoad := loaded[load.Module.Value]
+		if previousLoad == nil {
+			loaded[load.Module.Value] = load
+			continue
+		}
+
+		if fix {
+			previousLoad.To = append(previousLoad.To, load.To...)
+			previousLoad.From = append(previousLoad.From, load.From...)
+			f.Stmt = append(f.Stmt[:stmtIndex], f.Stmt[stmtIndex+1:]...)
+			stmtIndex--
+		} else {
+			start, end := load.Module.Span()
+			findings = append(findings,
+				makeFinding(f, start, end, "same-origin-load",
+					"There is already a load from \""+load.Module.Value+"\". Please merge all loads from the same origin into a single one.", true, nil))
+		}
+	}
+	return findings
+}
+
 func redefinedVariableWarning(f *build.File, fix bool) []*Finding {
 	findings := []*Finding{}
 	definedSymbols := make(map[string]bool)
@@ -717,6 +747,7 @@ var FileWarningMap = map[string]func(f *build.File, fix bool) []*Finding{
 	"package-on-top":     packageOnTopWarning,
 	"redefined-variable": redefinedVariableWarning,
 	"repository-name":    repositoryNameWarning,
+	"same-origin-load":   sameOriginLoadWarning,
 	"string-iteration":   stringIterationWarning,
 	"unused-variable":    unusedVariableWarning,
 }


Test patch:
diff --git a/warn/warn_test.go b/warn/warn_test.go
index 7524186..ea79f63 100644
--- a/warn/warn_test.go
+++ b/warn/warn_test.go
@@ -235,6 +235,54 @@ x = "unused"`,
 		false)
 }
 
+func TestWarnSameOriginLoad(t *testing.T) {
+	category := "same-origin-load"
+
+	checkFindingsAndFix(t, category, `
+	load(
+		":f.bzl",
+		"s1"
+	)
+	load(":t.bzl", "s3")
+	load(
+		":f.bzl",
+		"s2"
+	)`, `
+	load(
+		":f.bzl",
+		"s1",
+		"s2"
+	)
+	load(":t.bzl", "s3")`,
+		[]string{":7: There is already a load from \":f.bzl\". Please merge all loads from the same origin into a single one."},
+		false,
+	)
+
+	checkFindingsAndFix(t, category, `
+	load(
+		":f.bzl",
+		"s1"
+	)
+	load(
+		":f.bzl",
+		"s2"
+	)
+	load(
+		":f.bzl",
+		"s3"
+	)`, `
+	load(
+		":f.bzl",
+		"s1",
+		"s2",
+		"s3"
+	)`,
+		[]string{":6: There is already a load from \":f.bzl\". Please merge all loads from the same origin into a single one.",
+			":10: There is already a load from \":f.bzl\". Please merge all loads from the same origin into a single one."},
+		false,
+	)
+}
+
 func TestWarnUnusedVariables(t *testing.T) {
 	checkFindings(t, "unused-variable", `
 load(":f.bzl", "x")