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 <[email protected]>
Co-authored-by: Douglas Thor <[email protected]>

Author: 3 people

CHANGELOG.md

@@ -38,7 +38,8 @@ BEGIN_UNRELEASED_TEMPLATE
 
 {#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

gazelle/python/generate.go

@@ -231,9 +231,14 @@ func (py *Python) GenerateRules(args language.GenerateArgs) language.GenerateRes
 	}
 
 	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 @@ func (py *Python) GenerateRules(args language.GenerateArgs) language.GenerateRes
 			setAnnotations(*annotations).
 			generateImportsAttribute()
 
+
 		pyBinary := pyBinaryTarget.build()
 
 		result.Gen = append(result.Gen, pyBinary)
@@ -490,7 +496,8 @@ func (py *Python) GenerateRules(args language.GenerateArgs) language.GenerateRes
 		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 @@ func (py *Python) GenerateRules(args language.GenerateArgs) language.GenerateRes
 	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 {

gazelle/python/kinds.go

@@ -46,6 +46,7 @@ var pyKinds = map[string]rule.KindInfo{
 		SubstituteAttrs: map[string]bool{},
 		MergeableAttrs: map[string]bool{
 			"srcs": true,
+			"imports": true,
 		},
 		ResolveAttrs: map[string]bool{
 			"deps":     true,

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 = ["."],
+)

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"],
+)

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).

gazelle/python/testdata/remove_invalid_binary/WORKSPACE

@@ -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__"],
+)

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__"],
+)

gazelle/python/testdata/remove_invalid_binary/keep_binary/BUILD.out

@@ -0,0 +1,2 @@
+if __name__ == "__main__":
+    print("foo")