fix(gazelle) Delete python targets with invalid srcs (#3046)
When running Gazelle, it generated the following target:
```
py_binary(
name = "remove_py_binary",
srcs = ["__main__.py"],
main = "__main__.py",
visibility = ["//visibility:public"],
)
```
After `__main__.py` was deleted and the change committed, re-running
Gazelle did not remove the file from the srcs list.
This change introduces logic to check whether all entries in a Python
target’s srcs attribute correspond to valid files. If none of them
exist, the target is added to result.Empty to signal that it should be
cleaned up. This cleanup behavior applies to when python_generation mode
is package or file, as all `srcs` are expected to reside directly within
the current directory.
---------
Co-authored-by: yushan <yushan@uber.com>
Co-authored-by: Douglas Thor <dougthor42@users.noreply.github.com>diff --git a/CHANGELOG.md b/CHANGELOG.md
index 23e05f0..4576926 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -38,7 +38,8 @@
{#v0-0-0-fixed}
### Fixed
-* Nothing fixed.
+* (gazelle) Remove {obj}`py_binary` targets with invalid `srcs`. This includes files
+ that are not generated or regular files.
{#v0-0-0-added}
### Added
diff --git a/gazelle/python/generate.go b/gazelle/python/generate.go
index a180ec5..cbceea4 100644
--- a/gazelle/python/generate.go
+++ b/gazelle/python/generate.go
@@ -231,9 +231,14 @@
}
collisionErrors := singlylinkedlist.New()
+ // Create a validFilesMap of mainModules to validate if python macros have valid srcs.
+ validFilesMap := make(map[string]struct{})
appendPyLibrary := func(srcs *treeset.Set, pyLibraryTargetName string) {
allDeps, mainModules, annotations, err := parser.parse(srcs)
+ for name := range mainModules {
+ validFilesMap[name] = struct{}{}
+ }
if err != nil {
log.Fatalf("ERROR: %v\n", err)
}
@@ -363,6 +368,7 @@
setAnnotations(*annotations).
generateImportsAttribute()
+
pyBinary := pyBinaryTarget.build()
result.Gen = append(result.Gen, pyBinary)
@@ -490,7 +496,8 @@
result.Gen = append(result.Gen, pyTest)
result.Imports = append(result.Imports, pyTest.PrivateAttr(config.GazelleImportsKey))
}
-
+ emptyRules := py.getRulesWithInvalidSrcs(args, validFilesMap)
+ result.Empty = append(result.Empty, emptyRules...)
if !collisionErrors.Empty() {
it := collisionErrors.Iterator()
for it.Next() {
@@ -502,6 +509,42 @@
return result
}
+// getRulesWithInvalidSrcs checks existing Python rules in the BUILD file and return the rules with invalid source files.
+// Invalid source files are files that do not exist or not a target.
+func (py *Python) getRulesWithInvalidSrcs(args language.GenerateArgs, validFilesMap map[string]struct{}) (invalidRules []*rule.Rule) {
+ if args.File == nil {
+ return
+ }
+ for _, file := range args.GenFiles {
+ validFilesMap[file] = struct{}{}
+ }
+
+ isTarget := func(src string) bool {
+ return strings.HasPrefix(src, "@") || strings.HasPrefix(src, "//") || strings.HasPrefix(src, ":")
+ }
+ for _, existingRule := range args.File.Rules {
+ actualPyBinaryKind := GetActualKindName(pyBinaryKind, args)
+ if existingRule.Kind() != actualPyBinaryKind {
+ continue
+ }
+ var hasValidSrcs bool
+ for _, src := range existingRule.AttrStrings("srcs") {
+ if isTarget(src) {
+ hasValidSrcs = true
+ break
+ }
+ if _, ok := validFilesMap[src]; ok {
+ hasValidSrcs = true
+ break
+ }
+ }
+ if !hasValidSrcs {
+ invalidRules = append(invalidRules, newTargetBuilder(pyBinaryKind, existingRule.Name(), "", "", nil, false).build())
+ }
+ }
+ return invalidRules
+}
+
// isBazelPackage determines if the directory is a Bazel package by probing for
// the existence of a known BUILD file name.
func isBazelPackage(dir string) bool {
diff --git a/gazelle/python/kinds.go b/gazelle/python/kinds.go
index a4ce572..4fe8090 100644
--- a/gazelle/python/kinds.go
+++ b/gazelle/python/kinds.go
@@ -46,6 +46,7 @@
SubstituteAttrs: map[string]bool{},
MergeableAttrs: map[string]bool{
"srcs": true,
+ "imports": true,
},
ResolveAttrs: map[string]bool{
"deps": true,
diff --git a/gazelle/python/testdata/remove_invalid_binary/BUILD.in b/gazelle/python/testdata/remove_invalid_binary/BUILD.in
new file mode 100644
index 0000000..87d3571
--- /dev/null
+++ b/gazelle/python/testdata/remove_invalid_binary/BUILD.in
@@ -0,0 +1,18 @@
+load("@rules_python//python:defs.bzl", "py_binary", "py_library")
+
+py_library(
+ name = "keep_library",
+ deps = ["//keep_binary:foo"],
+)
+py_binary(
+ name = "remove_invalid_binary",
+ srcs = ["__main__.py"],
+ data = ["testdata/test.txt"],
+ visibility = ["//:__subpackages__"],
+)
+
+py_binary(
+ name = "another_removed_binary",
+ srcs = ["foo.py"], # eg a now-deleted file that used to have `if __name__` block
+ imports = ["."],
+)
diff --git a/gazelle/python/testdata/remove_invalid_binary/BUILD.out b/gazelle/python/testdata/remove_invalid_binary/BUILD.out
new file mode 100644
index 0000000..069188f
--- /dev/null
+++ b/gazelle/python/testdata/remove_invalid_binary/BUILD.out
@@ -0,0 +1,6 @@
+load("@rules_python//python:defs.bzl", "py_library")
+
+py_library(
+ name = "keep_library",
+ deps = ["//keep_binary:foo"],
+)
diff --git a/gazelle/python/testdata/remove_invalid_binary/README.md b/gazelle/python/testdata/remove_invalid_binary/README.md
new file mode 100644
index 0000000..be8a894
--- /dev/null
+++ b/gazelle/python/testdata/remove_invalid_binary/README.md
@@ -0,0 +1,3 @@
+# Remove invalid binary
+
+This test case asserts that `py_binary` should be deleted if invalid (no source files).
diff --git a/gazelle/python/testdata/remove_invalid_binary/WORKSPACE b/gazelle/python/testdata/remove_invalid_binary/WORKSPACE
new file mode 100644
index 0000000..e69de29
--- /dev/null
+++ b/gazelle/python/testdata/remove_invalid_binary/WORKSPACE
diff --git a/gazelle/python/testdata/remove_invalid_binary/keep_binary/BUILD.in b/gazelle/python/testdata/remove_invalid_binary/keep_binary/BUILD.in
new file mode 100644
index 0000000..0036c67
--- /dev/null
+++ b/gazelle/python/testdata/remove_invalid_binary/keep_binary/BUILD.in
@@ -0,0 +1,13 @@
+load("@rules_python//python:defs.bzl", "py_binary", "py_library")
+
+py_binary(
+ name = "foo",
+ srcs = ["foo.py"],
+ visibility = ["//:__subpackages__"],
+)
+
+py_library(
+ name = "keep_binary",
+ srcs = ["foo.py"],
+ visibility = ["//:__subpackages__"],
+)
diff --git a/gazelle/python/testdata/remove_invalid_binary/keep_binary/BUILD.out b/gazelle/python/testdata/remove_invalid_binary/keep_binary/BUILD.out
new file mode 100644
index 0000000..0036c67
--- /dev/null
+++ b/gazelle/python/testdata/remove_invalid_binary/keep_binary/BUILD.out
@@ -0,0 +1,13 @@
+load("@rules_python//python:defs.bzl", "py_binary", "py_library")
+
+py_binary(
+ name = "foo",
+ srcs = ["foo.py"],
+ visibility = ["//:__subpackages__"],
+)
+
+py_library(
+ name = "keep_binary",
+ srcs = ["foo.py"],
+ visibility = ["//:__subpackages__"],
+)
diff --git a/gazelle/python/testdata/remove_invalid_binary/keep_binary/foo.py b/gazelle/python/testdata/remove_invalid_binary/keep_binary/foo.py
new file mode 100644
index 0000000..d3b51ee
--- /dev/null
+++ b/gazelle/python/testdata/remove_invalid_binary/keep_binary/foo.py
@@ -0,0 +1,2 @@
+if __name__ == "__main__":
+ print("foo")
diff --git a/gazelle/python/testdata/remove_invalid_binary/test.yaml b/gazelle/python/testdata/remove_invalid_binary/test.yaml
new file mode 100644
index 0000000..e69de29
--- /dev/null
+++ b/gazelle/python/testdata/remove_invalid_binary/test.yaml
diff --git a/gazelle/python/testdata/respect_kind_mapping/BUILD.in b/gazelle/python/testdata/respect_kind_mapping/BUILD.in
index 6a06737..3c6ec07 100644
--- a/gazelle/python/testdata/respect_kind_mapping/BUILD.in
+++ b/gazelle/python/testdata/respect_kind_mapping/BUILD.in
@@ -1,6 +1,7 @@
load("@rules_python//python:defs.bzl", "py_library")
# gazelle:map_kind py_test my_test :mytest.bzl
+# gazelle:map_kind py_binary my_bin :mytest.bzl
py_library(
name = "respect_kind_mapping",
@@ -13,3 +14,8 @@
main = "__test__.py",
deps = [":respect_kind_mapping"],
)
+
+my_bin(
+ name = "my_bin_gets_removed",
+ srcs = ["__main__.py"],
+)
diff --git a/gazelle/python/testdata/respect_kind_mapping/BUILD.out b/gazelle/python/testdata/respect_kind_mapping/BUILD.out
index fa06e2a..eb6894f 100644
--- a/gazelle/python/testdata/respect_kind_mapping/BUILD.out
+++ b/gazelle/python/testdata/respect_kind_mapping/BUILD.out
@@ -2,6 +2,7 @@
load(":mytest.bzl", "my_test")
# gazelle:map_kind py_test my_test :mytest.bzl
+# gazelle:map_kind py_binary my_bin :mytest.bzl
py_library(
name = "respect_kind_mapping",
