Gazelle: Delete stale py_library and py_test targets (#3817)
## Summary
Fix the issue #3375
Right now the `py_library` and `py_test` targets with missing srcs
wouldn't be cleaned up by Gazelle in file mode.
This can be reproduced by a unit test added in commit
ad0e48ce0849ef9a0e89fb75a51345afb1947eb6
## Testing
In commit ad0e48ce0849ef9a0e89fb75a51345afb1947eb6 , without the fix,
`bazel test //python/...` failed in the newly added tests. The stale
`py_library ` and `py_test ` cannot be removed.
In latest HEAD d32fb7bb70a996e2d1215944a17941c5c4a708ed, `bazel test
//python/...` can pass
```
INFO: From Testing //python:python_test_remove_invalid_per_file:
==================== Test output for //python:python_test_remove_invalid_per_file:
--- FAIL: TestGazelleBinary (0.02s)
--- FAIL: TestGazelleBinary/remove_invalid_per_file (0.08s)
python_test.go:186: remove_invalid_per_file/BUILD diff (-want,+got):
(
"""
... // 8 identical lines
)
+ py_library(
+ name = "deleted_lib",
+ srcs = ["deleted.py"],
+ visibility = ["//:__subpackages__"],
+ )
+
+ py_test(
+ name = "bar_test",
+ srcs = ["bar_test.py"],
+ )
+
py_test(
- name = "bar_test",
- srcs = ["bar_test.py"],
+ name = "deleted_test",
+ srcs = ["deleted_test.py"],
)
"""
)
```
---------
Co-authored-by: Tao Wang <watao@microsoft.com>diff --git a/CHANGELOG.md b/CHANGELOG.md
index 43e05b6..978464f 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -74,6 +74,11 @@
{#v0-0-0-fixed}
### Fixed
+* (gazelle) `py_library` and `py_test` targets with missing source files can now be
+ removed by Gazelle ([#3375](https://github.com/bazel-contrib/rules_python/issues/3375)).
+ However `map_kind` and `alias_kind` will not be removed unless people are running a
+ gazelle version that includes
+ [bazel-gazelle#2362](https://github.com/bazel-contrib/bazel-gazelle/pull/2362)
* (bootstrap) Fixed a potential race condition with symlink creation during
startup.
* (gazelle) Fixed handling of auto-included `__init__.py` files when generating `py_binary`
diff --git a/gazelle/python/generate.go b/gazelle/python/generate.go
index 6fa6252..90e06a1 100644
--- a/gazelle/python/generate.go
+++ b/gazelle/python/generate.go
@@ -593,26 +593,53 @@
validFilesMap[file] = struct{}{}
}
+ // allFilesMap extends validFilesMap with all regular files on disk.
+ // py_binary uses validFilesMap (main modules + generated files), while py_library
+ // and py_test use allFilesMap since any file is a valid src for them.
+ allFilesMap := make(map[string]struct{}, len(validFilesMap)+len(args.RegularFiles))
+ for file := range validFilesMap {
+ allFilesMap[file] = struct{}{}
+ }
+ for _, file := range args.RegularFiles {
+ allFilesMap[file] = struct{}{}
+ }
+
isTarget := func(src string) bool {
return strings.HasPrefix(src, "@") || strings.HasPrefix(src, "//") || strings.HasPrefix(src, ":")
}
for _, existingRule := range args.File.Rules {
- if !kindMatches(args.Config, existingRule, pyBinaryKind) {
+ var matchedKind string
+ var filesMap map[string]struct{}
+ if kindMatches(args.Config, existingRule, pyBinaryKind) {
+ matchedKind = pyBinaryKind
+ filesMap = validFilesMap
+ } else if kindMatches(args.Config, existingRule, pyLibraryKind) {
+ matchedKind = pyLibraryKind
+ filesMap = allFilesMap
+ } else if kindMatches(args.Config, existingRule, pyTestKind) {
+ matchedKind = pyTestKind
+ filesMap = allFilesMap
+ } else {
+ continue
+ }
+
+ srcs := existingRule.AttrStrings("srcs")
+ if len(srcs) == 0 {
continue
}
var hasValidSrcs bool
- for _, src := range existingRule.AttrStrings("srcs") {
+ for _, src := range srcs {
if isTarget(src) {
hasValidSrcs = true
break
}
- if _, ok := validFilesMap[src]; ok {
+ if _, ok := filesMap[src]; ok {
hasValidSrcs = true
break
}
}
if !hasValidSrcs {
- invalidRules = append(invalidRules, newTargetBuilder(pyBinaryKind, existingRule.Name(), "", "", nil, false).build())
+ invalidRules = append(invalidRules, newTargetBuilder(matchedKind, existingRule.Name(), "", "", nil, false).build())
}
}
return invalidRules
diff --git a/gazelle/python/testdata/remove_invalid_library/BUILD.in b/gazelle/python/testdata/remove_invalid_library_package_mode/BUILD.in
similarity index 100%
rename from gazelle/python/testdata/remove_invalid_library/BUILD.in
rename to gazelle/python/testdata/remove_invalid_library_package_mode/BUILD.in
diff --git a/gazelle/python/testdata/remove_invalid_library/BUILD.out b/gazelle/python/testdata/remove_invalid_library_package_mode/BUILD.out
similarity index 100%
rename from gazelle/python/testdata/remove_invalid_library/BUILD.out
rename to gazelle/python/testdata/remove_invalid_library_package_mode/BUILD.out
diff --git a/gazelle/python/testdata/remove_invalid_library/README.md b/gazelle/python/testdata/remove_invalid_library_package_mode/README.md
similarity index 100%
rename from gazelle/python/testdata/remove_invalid_library/README.md
rename to gazelle/python/testdata/remove_invalid_library_package_mode/README.md
diff --git a/gazelle/python/testdata/remove_invalid_library/WORKSPACE b/gazelle/python/testdata/remove_invalid_library_package_mode/WORKSPACE
similarity index 100%
copy from gazelle/python/testdata/remove_invalid_library/WORKSPACE
copy to gazelle/python/testdata/remove_invalid_library_package_mode/WORKSPACE
diff --git a/gazelle/python/testdata/remove_invalid_library/my_test.py b/gazelle/python/testdata/remove_invalid_library_package_mode/my_test.py
similarity index 100%
rename from gazelle/python/testdata/remove_invalid_library/my_test.py
rename to gazelle/python/testdata/remove_invalid_library_package_mode/my_test.py
diff --git a/gazelle/python/testdata/remove_invalid_library/others/BUILD.in b/gazelle/python/testdata/remove_invalid_library_package_mode/others/BUILD.in
similarity index 100%
rename from gazelle/python/testdata/remove_invalid_library/others/BUILD.in
rename to gazelle/python/testdata/remove_invalid_library_package_mode/others/BUILD.in
diff --git a/gazelle/python/testdata/remove_invalid_library/others/BUILD.out b/gazelle/python/testdata/remove_invalid_library_package_mode/others/BUILD.out
similarity index 100%
rename from gazelle/python/testdata/remove_invalid_library/others/BUILD.out
rename to gazelle/python/testdata/remove_invalid_library_package_mode/others/BUILD.out
diff --git a/gazelle/python/testdata/remove_invalid_library/test.yaml b/gazelle/python/testdata/remove_invalid_library_package_mode/test.yaml
similarity index 100%
rename from gazelle/python/testdata/remove_invalid_library/test.yaml
rename to gazelle/python/testdata/remove_invalid_library_package_mode/test.yaml
diff --git a/gazelle/python/testdata/remove_invalid_per_file/BUILD.in b/gazelle/python/testdata/remove_invalid_per_file/BUILD.in
new file mode 100644
index 0000000..d3b15a7
--- /dev/null
+++ b/gazelle/python/testdata/remove_invalid_per_file/BUILD.in
@@ -0,0 +1,45 @@
+load("@rules_python//python:defs.bzl", "py_binary", "py_library", "py_test")
+
+# gazelle:python_generation_mode file
+
+# Valid py_library — bar.py exists on disk, should be kept.
+py_library(
+ name = "bar",
+ srcs = ["bar.py"],
+ visibility = ["//:__subpackages__"],
+)
+
+# Stale py_library — deleted.py does not exist, should be removed.
+py_library(
+ name = "deleted_lib",
+ srcs = ["deleted.py"],
+ visibility = ["//:__subpackages__"],
+)
+
+# Valid py_binary — __main__.py exists on disk, should be kept.
+py_binary(
+ name = "remove_invalid_per_file_bin",
+ srcs = ["__main__.py"],
+ visibility = ["//:__subpackages__"],
+)
+
+# Stale py_binary — deleted_bin.py does not exist, should be removed.
+py_binary(
+ name = "deleted_bin",
+ srcs = ["deleted_bin.py"],
+ visibility = ["//:__subpackages__"],
+)
+
+# Valid py_test — bar_test.py exists on disk, should be kept.
+py_test(
+ name = "bar_test",
+ srcs = ["bar_test.py"],
+ visibility = ["//:__subpackages__"],
+)
+
+# Stale py_test — deleted_test.py does not exist, should be removed.
+py_test(
+ name = "deleted_test",
+ srcs = ["deleted_test.py"],
+ visibility = ["//:__subpackages__"],
+)
diff --git a/gazelle/python/testdata/remove_invalid_per_file/BUILD.out b/gazelle/python/testdata/remove_invalid_per_file/BUILD.out
new file mode 100644
index 0000000..376c14c
--- /dev/null
+++ b/gazelle/python/testdata/remove_invalid_per_file/BUILD.out
@@ -0,0 +1,24 @@
+load("@rules_python//python:defs.bzl", "py_binary", "py_library", "py_test")
+
+# gazelle:python_generation_mode file
+
+# Valid py_library — bar.py exists on disk, should be kept.
+py_library(
+ name = "bar",
+ srcs = ["bar.py"],
+ visibility = ["//:__subpackages__"],
+)
+
+# Valid py_test — bar_test.py exists on disk, should be kept.
+py_test(
+ name = "bar_test",
+ srcs = ["bar_test.py"],
+ visibility = ["//:__subpackages__"],
+)
+
+py_binary(
+ name = "remove_invalid_per_file_bin",
+ srcs = ["__main__.py"],
+ main = "__main__.py",
+ visibility = ["//:__subpackages__"],
+)
diff --git a/gazelle/python/testdata/remove_invalid_library/WORKSPACE b/gazelle/python/testdata/remove_invalid_per_file/WORKSPACE
similarity index 100%
rename from gazelle/python/testdata/remove_invalid_library/WORKSPACE
rename to gazelle/python/testdata/remove_invalid_per_file/WORKSPACE
diff --git a/gazelle/python/testdata/remove_invalid_library/my_test.py b/gazelle/python/testdata/remove_invalid_per_file/__main__.py
similarity index 100%
copy from gazelle/python/testdata/remove_invalid_library/my_test.py
copy to gazelle/python/testdata/remove_invalid_per_file/__main__.py
diff --git a/gazelle/python/testdata/remove_invalid_per_file/alias_kind/BUILD.in b/gazelle/python/testdata/remove_invalid_per_file/alias_kind/BUILD.in
new file mode 100644
index 0000000..e56ea16
--- /dev/null
+++ b/gazelle/python/testdata/remove_invalid_per_file/alias_kind/BUILD.in
@@ -0,0 +1,37 @@
+load(":mylib.bzl", "my_py_library", "my_py_test")
+
+# gazelle:python_generation_mode file
+# gazelle:alias_kind my_py_library py_library
+# gazelle:alias_kind my_py_test py_test
+
+# Valid aliased py_library — bar.py exists on disk, should be kept.
+my_py_library(
+ name = "bar",
+ srcs = ["bar.py"],
+ visibility = ["//:__subpackages__"],
+)
+
+# Stale aliased py_library — deleted.py does not exist, should be removed.
+# TODO: Known limitation: not fully deleted until gazelle is bumped to include
+# https://github.com/bazel-contrib/bazel-gazelle/pull/2362
+my_py_library(
+ name = "deleted_lib",
+ srcs = ["deleted.py"],
+ visibility = ["//:__subpackages__"],
+)
+
+# Valid aliased py_test — bar_test.py exists on disk, should be kept.
+my_py_test(
+ name = "bar_test",
+ srcs = ["bar_test.py"],
+ visibility = ["//:__subpackages__"],
+)
+
+# Stale aliased py_test — deleted_test.py does not exist, should be removed.
+# TODO: Known limitation: not fully deleted until gazelle is bumped to include
+# https://github.com/bazel-contrib/bazel-gazelle/pull/2362
+my_py_test(
+ name = "deleted_test",
+ srcs = ["deleted_test.py"],
+ visibility = ["//:__subpackages__"],
+)
diff --git a/gazelle/python/testdata/remove_invalid_per_file/alias_kind/BUILD.out b/gazelle/python/testdata/remove_invalid_per_file/alias_kind/BUILD.out
new file mode 100644
index 0000000..19d6c01
--- /dev/null
+++ b/gazelle/python/testdata/remove_invalid_per_file/alias_kind/BUILD.out
@@ -0,0 +1,35 @@
+load(":mylib.bzl", "my_py_library", "my_py_test")
+
+# gazelle:python_generation_mode file
+# gazelle:alias_kind my_py_library py_library
+# gazelle:alias_kind my_py_test py_test
+
+# Valid aliased py_library — bar.py exists on disk, should be kept.
+my_py_library(
+ name = "bar",
+ srcs = ["bar.py"],
+ visibility = ["//:__subpackages__"],
+)
+
+# Stale aliased py_library — deleted.py does not exist, should be removed.
+# TODO: Known limitation: not fully deleted until gazelle is bumped to include
+# https://github.com/bazel-contrib/bazel-gazelle/pull/2362
+my_py_library(
+ name = "deleted_lib",
+ visibility = ["//:__subpackages__"],
+)
+
+# Valid aliased py_test — bar_test.py exists on disk, should be kept.
+my_py_test(
+ name = "bar_test",
+ srcs = ["bar_test.py"],
+ visibility = ["//:__subpackages__"],
+)
+
+# Stale aliased py_test — deleted_test.py does not exist, should be removed.
+# TODO: Known limitation: not fully deleted until gazelle is bumped to include
+# https://github.com/bazel-contrib/bazel-gazelle/pull/2362
+my_py_test(
+ name = "deleted_test",
+ visibility = ["//:__subpackages__"],
+)
diff --git a/gazelle/python/testdata/remove_invalid_library/my_test.py b/gazelle/python/testdata/remove_invalid_per_file/alias_kind/bar.py
similarity index 100%
copy from gazelle/python/testdata/remove_invalid_library/my_test.py
copy to gazelle/python/testdata/remove_invalid_per_file/alias_kind/bar.py
diff --git a/gazelle/python/testdata/remove_invalid_library/my_test.py b/gazelle/python/testdata/remove_invalid_per_file/alias_kind/bar_test.py
similarity index 100%
copy from gazelle/python/testdata/remove_invalid_library/my_test.py
copy to gazelle/python/testdata/remove_invalid_per_file/alias_kind/bar_test.py
diff --git a/gazelle/python/testdata/remove_invalid_library/my_test.py b/gazelle/python/testdata/remove_invalid_per_file/bar.py
similarity index 100%
copy from gazelle/python/testdata/remove_invalid_library/my_test.py
copy to gazelle/python/testdata/remove_invalid_per_file/bar.py
diff --git a/gazelle/python/testdata/remove_invalid_library/my_test.py b/gazelle/python/testdata/remove_invalid_per_file/bar_test.py
similarity index 100%
copy from gazelle/python/testdata/remove_invalid_library/my_test.py
copy to gazelle/python/testdata/remove_invalid_per_file/bar_test.py
diff --git a/gazelle/python/testdata/remove_invalid_per_file/map_kind/BUILD.in b/gazelle/python/testdata/remove_invalid_per_file/map_kind/BUILD.in
new file mode 100644
index 0000000..00b9a60
--- /dev/null
+++ b/gazelle/python/testdata/remove_invalid_per_file/map_kind/BUILD.in
@@ -0,0 +1,37 @@
+load(":mylib.bzl", "my_py_binary", "my_py_test")
+
+# gazelle:python_generation_mode file
+# gazelle:map_kind py_binary my_py_binary :mylib.bzl
+# gazelle:map_kind py_test my_py_test :mylib.bzl
+
+# Valid py_library — bar.py exists on disk, should be kept.
+py_library(
+ name = "bar",
+ srcs = ["bar.py"],
+ visibility = ["//:__subpackages__"],
+)
+
+# Stale py_library — deleted.py does not exist, should be removed.
+py_library(
+ name = "deleted_lib",
+ srcs = ["deleted.py"],
+ visibility = ["//:__subpackages__"],
+)
+
+# Stale mapped py_binary — deleted_bin.py does not exist, should be removed.
+my_py_binary(
+ name = "deleted_bin",
+ srcs = ["deleted_bin.py"],
+)
+
+# Valid mapped py_test — bar_test.py exists on disk, should be kept.
+my_py_test(
+ name = "bar_test",
+ srcs = ["bar_test.py"],
+)
+
+# Stale mapped py_test — deleted_test.py does not exist, should be removed.
+my_py_test(
+ name = "deleted_test",
+ srcs = ["deleted_test.py"],
+)
diff --git a/gazelle/python/testdata/remove_invalid_per_file/map_kind/BUILD.out b/gazelle/python/testdata/remove_invalid_per_file/map_kind/BUILD.out
new file mode 100644
index 0000000..84ca905
--- /dev/null
+++ b/gazelle/python/testdata/remove_invalid_per_file/map_kind/BUILD.out
@@ -0,0 +1,19 @@
+load("@rules_python//python:defs.bzl", "py_library")
+load(":mylib.bzl", "my_py_test")
+
+# gazelle:python_generation_mode file
+# gazelle:map_kind py_binary my_py_binary :mylib.bzl
+# gazelle:map_kind py_test my_py_test :mylib.bzl
+
+# Valid py_library — bar.py exists on disk, should be kept.
+py_library(
+ name = "bar",
+ srcs = ["bar.py"],
+ visibility = ["//:__subpackages__"],
+)
+
+# Valid mapped py_test — bar_test.py exists on disk, should be kept.
+my_py_test(
+ name = "bar_test",
+ srcs = ["bar_test.py"],
+)
diff --git a/gazelle/python/testdata/remove_invalid_library/my_test.py b/gazelle/python/testdata/remove_invalid_per_file/map_kind/bar.py
similarity index 100%
copy from gazelle/python/testdata/remove_invalid_library/my_test.py
copy to gazelle/python/testdata/remove_invalid_per_file/map_kind/bar.py
diff --git a/gazelle/python/testdata/remove_invalid_library/my_test.py b/gazelle/python/testdata/remove_invalid_per_file/map_kind/bar_test.py
similarity index 100%
copy from gazelle/python/testdata/remove_invalid_library/my_test.py
copy to gazelle/python/testdata/remove_invalid_per_file/map_kind/bar_test.py
diff --git a/gazelle/python/testdata/remove_invalid_per_file/test.yaml b/gazelle/python/testdata/remove_invalid_per_file/test.yaml
new file mode 100644
index 0000000..ed97d53
--- /dev/null
+++ b/gazelle/python/testdata/remove_invalid_per_file/test.yaml
@@ -0,0 +1 @@
+---
