Break module extension groups after a `use_repo` (#1339)
This allows users to separate tags and associated `use_repo` statements into separate groups.
Also use unified diffs in the integration tests since ordinary diffs make it very hard to reason about whitespace.
Before:
```
go_deps = use_extension("//:extensions.bzl", "go_deps")
go_deps.from_file(go_mod = "//:go.mod")
use_repo(
go_deps,
"com_github_bazelbuild_buildtools",
)
go_deps.module(name = "foo")
use_repo(go_deps, "foo")
```
After:
```
go_deps = use_extension("//:extensions.bzl", "go_deps")
go_deps.from_file(go_mod = "//:go.mod")
use_repo(
go_deps,
"com_github_bazelbuild_buildtools",
)
go_deps.module(name = "foo")
use_repo(go_deps, "foo")
```diff --git a/build/print.go b/build/print.go
index 091971f..8e6cc96 100644
--- a/build/print.go
+++ b/build/print.go
@@ -387,56 +387,64 @@
}
func useSameModuleExtensionProxy(x, y Expr) bool {
- extX := usedModuleExtensionProxy(x)
+ extX, isUseRepoX := usedModuleExtensionProxy(x)
if extX == "" {
return false
}
- extY := usedModuleExtensionProxy(y)
- return extX == extY
+ extY, isUseRepoY := usedModuleExtensionProxy(y)
+ // Switching from a use_repo to a non-use_repo statement should break the
+ // sequence of statements.
+ //
+ // foo_deps.module(path = "github.com/foo/bar")
+ // use_repo(foo_deps, "com_github_foo_bar")
+ //
+ // foo_deps.module(path = "github.com/foo/bar2")
+ // use_repo(foo_deps, "com_github_foo_bar2")
+ return extX == extY && (!isUseRepoX || isUseRepoY)
}
-func usedModuleExtensionProxy(x Expr) string {
+func usedModuleExtensionProxy(x Expr) (name string, isUseRepo bool) {
if call, ok := x.(*CallExpr); ok {
if callee, isIdent := call.X.(*Ident); isIdent && callee.Name == "use_repo" {
// Handles:
// use_repo(foo_deps, "com_github_foo_bar")
if len(call.List) < 1 {
- return ""
+ return "", true
}
proxy, isIdent := call.List[0].(*Ident)
if !isIdent {
- return ""
+ return "", true
}
- return proxy.Name
+ return proxy.Name, true
} else if dot, isDot := call.X.(*DotExpr); isDot {
// Handles:
// foo_deps.module(path = "github.com/foo/bar")
extension, isIdent := dot.X.(*Ident)
if !isIdent {
- return ""
+ return "", false
}
- return extension.Name
+ return extension.Name, false
} else {
- return ""
+ return "", false
}
} else if assign, ok := x.(*AssignExpr); ok {
// Handles:
// foo_deps = use_extension("//:foo.bzl", "foo_deps")
assignee, isIdent := assign.LHS.(*Ident)
if !isIdent {
- return ""
+ return "", false
}
call, isCall := assign.RHS.(*CallExpr)
if !isCall {
- return ""
+ return "", false
}
callee, isIdent := call.X.(*Ident)
if !isIdent || callee.Name != "use_extension" {
- return ""
+ return "", false
}
- return assignee.Name
+ return assignee.Name, false
} else {
- return ""
+ return "", false
}
}
diff --git a/buildifier/integration_test.sh b/buildifier/integration_test.sh
index e22e8b4..293e8bb 100755
--- a/buildifier/integration_test.sh
+++ b/buildifier/integration_test.sh
@@ -91,6 +91,8 @@
b = "b",
a = "c",
)
+go_deps.module(name = "foo")
+use_repo(go_deps, "foo")
bazel_dep(name="foo",version="1.0")
git_override(module_name="foo",remote="foo.git",commit="1234567890")
bazel_dep(name="bar",version="1.0")
@@ -204,6 +206,9 @@
c = "a",
)
+go_deps.module(name = "foo")
+use_repo(go_deps, "foo")
+
bazel_dep(name = "foo", version = "1.0")
git_override(
module_name = "foo",
@@ -349,19 +354,19 @@
}
EOF
-diff test_dir/BUILD golden/BUILD.golden
-diff test_dir/test.bzl golden/test.bzl.golden
-diff test_dir/subdir/test.bzl golden/test.bzl.golden
-diff test_dir/test.bzl.BUILD.out golden/BUILD.golden
-diff test_dir/subdir/build golden/build
-diff test_dir/foo.bar golden/foo.bar
-diff test.bzl golden/test.bzl.golden
-diff test2.bzl golden/test.bzl.golden
-diff stdout golden/test.bzl.golden
-diff test_dir/test.bzl.out golden/test.bzl.golden
-diff test_dir/.git/git.bzl golden/git.bzl
-diff test_dir/MODULE.bazel golden/MODULE.bazel.golden
-diff test_dir/.buildifier.example.json golden/.buildifier.example.json
+diff -u test_dir/BUILD golden/BUILD.golden
+diff -u test_dir/test.bzl golden/test.bzl.golden
+diff -u test_dir/subdir/test.bzl golden/test.bzl.golden
+diff -u test_dir/test.bzl.BUILD.out golden/BUILD.golden
+diff -u test_dir/subdir/build golden/build
+diff -u test_dir/foo.bar golden/foo.bar
+diff -u test.bzl golden/test.bzl.golden
+diff -u test2.bzl golden/test.bzl.golden
+diff -u stdout golden/test.bzl.golden
+diff -u test_dir/test.bzl.out golden/test.bzl.golden
+diff -u test_dir/.git/git.bzl golden/git.bzl
+diff -u test_dir/MODULE.bazel golden/MODULE.bazel.golden
+diff -u test_dir/.buildifier.example.json golden/.buildifier.example.json
# Test run on a directory without -r
"$buildifier" test_dir || ret=$?
@@ -444,30 +449,30 @@
if [[ $ret -ne 4 ]]; then
die "$1: warn: Expected buildifier to exit with 4, actual: $ret"
fi
- diff test_dir/error golden/error_golden || die "$1: wrong console output for --mode=check --lint=warn"
- diff test_dir/to_fix.bzl test_dir/to_fix.bzl || die "$1: --mode=check --lint=warn shouldn't modify files"
+ diff -u test_dir/error golden/error_golden || die "$1: wrong console output for --mode=check --lint=warn"
+ diff -u test_dir/to_fix.bzl test_dir/to_fix.bzl || die "$1: --mode=check --lint=warn shouldn't modify files"
# --lint=warn
$buildifier --lint=warn $2 test_dir/to_fix_tmp.bzl 2> test_dir/error || ret=$?
if [[ $ret -ne 4 ]]; then
die "$1: warn: Expected buildifier to exit with 4, actual: $ret"
fi
- diff test_dir/error golden/error_golden || die "$1: wrong console output for --lint=warn"
+ diff -u test_dir/error golden/error_golden || die "$1: wrong console output for --lint=warn"
# --lint=warn with --path
$buildifier --lint=warn --path=another_test_dir/to_fix_tmp.bzl $2 test_dir/to_fix_tmp.bzl 2> test_dir/error || ret=$?
if [[ $ret -ne 4 ]]; then
die "$1: warn: Expected buildifier to exit with 4, actual: $ret"
fi
- diff test_dir/error golden/error_golden_another || die "$1: wrong console output for --lint=warn and --path"
+ diff -u test_dir/error golden/error_golden_another || die "$1: wrong console output for --lint=warn and --path"
# --lint=fix
$buildifier --lint=fix $2 -v test_dir/to_fix_tmp.bzl 2> test_dir/fix_report || ret=$?
if [[ $ret -ne 4 ]]; then
die "$1: fix: Expected buildifier to exit with 4, actual: $ret"
fi
- diff test_dir/to_fix_tmp.bzl $3 || die "$1: wrong file output for --lint=fix"
- diff test_dir/fix_report golden/fix_report_golden || die "$1: wrong console output for --lint=fix"
+ diff -u test_dir/to_fix_tmp.bzl $3 || die "$1: wrong file output for --lint=fix"
+ diff -u test_dir/fix_report golden/fix_report_golden || die "$1: wrong console output for --lint=fix"
}
test_lint "default" "" "test_dir/fixed_golden.bzl" "$error_bzl"$'\n'"$error_docstring"$'\n'"$error_integer"$'\n'"$error_cfg" 2
@@ -629,16 +634,16 @@
cd test_dir/json
$buildifier --mode=check --format=json --lint=warn --warnings=-module-docstring -v to_fix.bzl to_fix_2.bzl to_fix_3.bzl to_fix_4.bzl > json_report
-diff json_report ../../golden/json_report_golden || die "$1: wrong console output for --mode=check --format=json --lint=warn with many files"
+diff -u json_report ../../golden/json_report_golden || die "$1: wrong console output for --mode=check --format=json --lint=warn with many files"
$buildifier --mode=check --format=json --lint=warn --warnings=-module-docstring -v to_fix_4.bzl > json_report
-diff json_report ../../golden/json_report_small_golden || die "$1: wrong console output for --mode=check --format=json --lint=warn with a single file"
+diff -u json_report ../../golden/json_report_small_golden || die "$1: wrong console output for --mode=check --format=json --lint=warn with a single file"
$buildifier --mode=check --format=json --lint=warn --warnings=-module-docstring -v < to_fix_4.bzl > json_report
-diff json_report ../../golden/json_report_stdin_golden || die "$1: wrong console output for --mode=check --format=json --lint=warn with stdin"
+diff -u json_report ../../golden/json_report_stdin_golden || die "$1: wrong console output for --mode=check --format=json --lint=warn with stdin"
$buildifier --mode=check --format=json --lint=warn --warnings=-module-docstring -v to_fix_4.bzl foo.bar > json_report
-diff json_report ../../golden/json_report_invalid_file_golden || die "$1: wrong console output for --mode=check --format=json --lint=warn with an invalid file"
+diff -u json_report ../../golden/json_report_invalid_file_golden || die "$1: wrong console output for --mode=check --format=json --lint=warn with an invalid file"
cd ../..
@@ -679,4 +684,4 @@
EOF
$buildifier --lint=warn --warnings=deprecated-function BUILD 2> report || ret=$?
-diff report_golden report || die "$1: wrong console output for multifile warnings (WORKSPACE exists)"
+diff -u report_golden report || die "$1: wrong console output for multifile warnings (WORKSPACE exists)"
