diff --git a/CHANGELOG.md b/CHANGELOG.md index da59ecf8b5..966bb5141b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -69,6 +69,8 @@ END_UNRELEASED_TEMPLATE * (pypi) Wheels with BUILD.bazel (or other special Bazel files) no longer result in missing files at runtime ([#2782](https://github.com/bazel-contrib/rules_python/issues/2782)). +* (gazelle) Remove 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 c1edec4731..4adc3b4258 100644 --- a/gazelle/python/generate.go +++ b/gazelle/python/generate.go @@ -225,7 +225,6 @@ func (py *Python) GenerateRules(args language.GenerateArgs) language.GenerateRes var result language.GenerateResult result.Gen = make([]*rule.Rule, 0) - collisionErrors := singlylinkedlist.New() appendPyLibrary := func(srcs *treeset.Set, pyLibraryTargetName string) { @@ -473,7 +472,10 @@ func (py *Python) GenerateRules(args language.GenerateArgs) language.GenerateRes result.Gen = append(result.Gen, pyTest) result.Imports = append(result.Imports, pyTest.PrivateAttr(config.GazelleImportsKey)) } - + if !cfg.CoarseGrainedGeneration() { + emptyRules := py.getRulesWithInvalidSrcs(cfg, args) + result.Empty = append(result.Empty, emptyRules...) + } if !collisionErrors.Empty() { it := collisionErrors.Iterator() for it.Next() { @@ -485,6 +487,48 @@ 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(cfg *pythonconfig.Config, args language.GenerateArgs) (invalidRules []*rule.Rule) { + if args.File == nil { + return + } + filesMap := make(map[string]struct{}) + for _, file := range args.RegularFiles { + if cfg.IgnoresFile(filepath.Base(file)) { + continue + } + filesMap[file] = struct{}{} + } + for _, file := range args.GenFiles { + filesMap[file] = struct{}{} + } + + isTarget := func(src string) bool { + return strings.HasPrefix(src, "@") || strings.HasPrefix(src, "//") || strings.HasPrefix(src, ":") + } + for _, existingRule := range args.File.Rules { + if existingRule.Kind() != pyBinaryKind { + continue + } + hasValidSrcs := true + for _, src := range existingRule.AttrStrings("srcs") { + if isTarget(src) { + continue + } + if _, ok := filesMap[src]; ok { + continue + } + hasValidSrcs = false + break + } + if !hasValidSrcs { + invalidRules = append(invalidRules, newTargetBuilder(existingRule.Kind(), existingRule.Name(), args.Config.RepoRoot, args.Rel, nil).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/testdata/remove_invalid_binary/BUILD.in b/gazelle/python/testdata/remove_invalid_binary/BUILD.in new file mode 100644 index 0000000000..aca4df0a13 --- /dev/null +++ b/gazelle/python/testdata/remove_invalid_binary/BUILD.in @@ -0,0 +1,11 @@ +load("@rules_python//python:defs.bzl", "py_binary") + +py_library( + name = "keep_library", + deps = ["//keep_binary:foo"], +) +py_binary( + name = "remove_invalid_binary", + srcs = ["__main__.py"], + visibility = ["//:__subpackages__"], +) 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 0000000000..069188f5ca --- /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 0000000000..be8a894420 --- /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 0000000000..e69de29bb2 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 0000000000..0036c6797a --- /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 0000000000..0036c6797a --- /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 0000000000..d3b51eea8e --- /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 0000000000..e69de29bb2