Repository: bazelbuild/buildtools
Pull request #1191 (2023-08-30 13:07:38)

Compare load paths as paths instead of just strings
Paths like "abc/def" should precede "abc-def" (although `-` is smaller than `/` in ASCII).


Patch:
diff --git a/buildifier/npm/BUILD.bazel b/buildifier/npm/BUILD.bazel
index 7e04d5f..dbaf48c 100644
--- a/buildifier/npm/BUILD.bazel
+++ b/buildifier/npm/BUILD.bazel
@@ -1,5 +1,5 @@
-load("@build_bazel_rules_nodejs//:index.bzl", "nodejs_test", "pkg_npm")
 load("@bazel_skylib//rules:copy_file.bzl", "copy_file")
+load("@build_bazel_rules_nodejs//:index.bzl", "nodejs_test", "pkg_npm")
 
 copy_file(
     name = "copy_LICENSE",
diff --git a/buildozer/npm/BUILD.bazel b/buildozer/npm/BUILD.bazel
index ca21350..5e83b85 100644
--- a/buildozer/npm/BUILD.bazel
+++ b/buildozer/npm/BUILD.bazel
@@ -1,5 +1,5 @@
-load("@build_bazel_rules_nodejs//:index.bzl", "nodejs_test", "pkg_npm")
 load("@bazel_skylib//rules:copy_file.bzl", "copy_file")
+load("@build_bazel_rules_nodejs//:index.bzl", "nodejs_test", "pkg_npm")
 load("@buildozer_npm_deps//typescript:index.bzl", "tsc")
 
 copy_file(
diff --git a/warn/warn_cosmetic.go b/warn/warn_cosmetic.go
index 1a99a78..7ee5f49 100644
--- a/warn/warn_cosmetic.go
+++ b/warn/warn_cosmetic.go
@@ -120,8 +120,8 @@ func packageOnTopWarning(f *build.File) []*LinterFinding {
 			}
 			return []*LinterFinding{makeLinterFinding(rule.Call,
 				"Package declaration should be at the top of the file, after the load() statements, "+
-					"but before any call to a rule or a macro. "+
-					"package_group() and licenses() may be called before package().")}
+						"but before any call to a rule or a macro. "+
+						"package_group() and licenses() may be called before package().")}
 		}
 		seenRule = true
 	}
@@ -129,7 +129,7 @@ func packageOnTopWarning(f *build.File) []*LinterFinding {
 }
 
 func loadOnTopWarning(f *build.File) []*LinterFinding {
-	if f.Type == build.TypeWorkspace || f.Type == build.TypeModule{
+	if f.Type == build.TypeWorkspace || f.Type == build.TypeModule {
 		// Not applicable to WORKSPACE or MODULE files
 		return nil
 	}
@@ -191,6 +191,28 @@ func loadOnTopWarning(f *build.File) []*LinterFinding {
 	return findings
 }
 
+// comparePaths compares two strings as if they were paths (the path delimiter,
+// '/', should be treated as smallest symbol).
+func comparePaths(path1, path2 string) bool {
+	if path1 == path2 {
+		return false
+	}
+
+	chunks1 := strings.Split(path1, "/")
+	chunks2 := strings.Split(path2, "/")
+
+	for i, chunk1 := range chunks1 {
+		if i >= len(chunks2) {
+			return false
+		}
+		chunk2 := chunks2[i]
+		if chunk1 != chunk2 {
+			return chunk1 < chunk2
+		}
+	}
+	return true
+}
+
 // compareLoadLabels compares two module names
 // If one label has explicit repository path (starts with @), it goes first
 // If the packages are different, labels are sorted by package name (empty package goes first)
@@ -221,7 +243,7 @@ func compareLoadLabels(load1Label, load2Label string) bool {
 
 	// in case both packages are the same, use file names to break ties
 	if package1 == package2 {
-		return filename1 < filename2
+		return comparePaths(filename1, filename2)
 	}
 
 	// in case one of the packages is empty, the empty one goes first
@@ -230,7 +252,7 @@ func compareLoadLabels(load1Label, load2Label string) bool {
 	}
 
 	// both packages are non-empty and not equal, so compare them
-	return package1 < package2
+	return comparePaths(package1, package2)
 }
 
 // outOfOrderLoadWarning only sorts consequent chunks of load statements. If applied together with


Test patch:
diff --git a/warn/warn_cosmetic_test.go b/warn/warn_cosmetic_test.go
index 3242856..43adf82 100644
--- a/warn/warn_cosmetic_test.go
+++ b/warn/warn_cosmetic_test.go
@@ -277,12 +277,18 @@ load(":a.bzl", "a")`,
 		[]string{}, scopeEverywhere)
 
 	checkFindingsAndFix(t, "out-of-order-load", `
-load("//foo:xyz.bzl", "xyz")
-load("//foo/bar:mno.bzl", "mno")
+load("//foo-bar:xyz.bzl", "xyz")
+load("//foo/bar:baz/mno.bzl", "mno")
+load("//foo/bar-baz:mno/prs.bzl", "prs")
+load("//foo/bar-baz:mno-prs.bzl", "prs")
 `, `
-load("//foo:xyz.bzl", "xyz")
-load("//foo/bar:mno.bzl", "mno")`,
-		[]string{}, scopeEverywhere)
+load("//foo/bar:baz/mno.bzl", "mno")
+load("//foo/bar-baz:mno/prs.bzl", "prs")
+load("//foo/bar-baz:mno-prs.bzl", "prs")
+load("//foo-bar:xyz.bzl", "xyz")`,
+		[]string{
+			"2: Load statement is out of its lexicographical order.",
+		}, scopeEverywhere)
 
 	checkFindingsAndFix(t, "out-of-order-load", `
 load("//foo:xyz.bzl", "xyz")