fix(gazelle): delete invalid py_library and use correct NonEmptyAttrs for py_* (bazel-contrib#1887)

Before gazelle would leave generated py_library targets aroundeven when
no files in a directory exist because X. With this change gazelle
correctly handles this case.

Author: hunshcn

CHANGELOG.md

@@ -25,6 +25,12 @@ A brief description of the categories of changes:
 
 ### Fixed
 
+* (gazelle) Remove `visibility` from `NonEmptyAttr`.
+  Now empty(have no `deps/main/srcs/imports` attr) `py_library/test/binary` rules will
+  be automatically deleted correctly. For example, if `python_generation_mode`
+  is set to package, when `__init__.py` is deleted, the `py_library` generated
+  for this package before will be deleted automatically.
+
 ### Added
 
 ## [0.32.2] - 2024-05-14

gazelle/python/generate.go

@@ -270,11 +270,6 @@ func (py *Python) GenerateRules(args language.GenerateArgs) language.GenerateRes
 			}
 		}
 
-		// If we're doing per-file generation, srcs could be empty at this point, meaning we shouldn't make a py_library.
-		if srcs.Empty() {
-			return
-		}
-
 		// Check if a target with the same name we are generating already
 		// exists, and if it is of a different kind from the one we are
 		// generating. If so, we have to throw an error since Gazelle won't
@@ -295,8 +290,12 @@ func (py *Python) GenerateRules(args language.GenerateArgs) language.GenerateRes
 			generateImportsAttribute().
 			build()
 
-		result.Gen = append(result.Gen, pyLibrary)
-		result.Imports = append(result.Imports, pyLibrary.PrivateAttr(config.GazelleImportsKey))
+		if pyLibrary.IsEmpty(py.Kinds()[pyLibrary.Kind()]) {
+			result.Empty = append(result.Gen, pyLibrary)
+		} else {
+			result.Gen = append(result.Gen, pyLibrary)
+			result.Imports = append(result.Imports, pyLibrary.PrivateAttr(config.GazelleImportsKey))
+		}
 	}
 	if cfg.PerFileGeneration() {
 		hasInit, nonEmptyInit := hasLibraryEntrypointFile(args.Dir)
@@ -311,7 +310,7 @@ func (py *Python) GenerateRules(args language.GenerateArgs) language.GenerateRes
 			}
 			appendPyLibrary(srcs, pyLibraryTargetName)
 		})
-	} else if !pyLibraryFilenames.Empty() {
+	} else {
 		appendPyLibrary(pyLibraryFilenames, cfg.RenderLibraryName(packageName))
 	}
 
@@ -411,7 +410,7 @@ func (py *Python) GenerateRules(args language.GenerateArgs) language.GenerateRes
 			// the file exists on disk.
 			pyTestFilenames.Add(pyTestEntrypointFilename)
 		}
-		if (hasPyTestEntryPointTarget || !pyTestFilenames.Empty()) {
+		if hasPyTestEntryPointTarget || !pyTestFilenames.Empty() {
 			pyTestTargetName := cfg.RenderTestName(packageName)
 			pyTestTarget := newPyTestTargetBuilder(pyTestFilenames, pyTestTargetName)
 

gazelle/python/kinds.go

@@ -34,11 +34,10 @@ var pyKinds = map[string]rule.KindInfo{
 	pyBinaryKind: {
 		MatchAny: true,
 		NonEmptyAttrs: map[string]bool{
-			"deps":       true,
-			"main":       true,
-			"srcs":       true,
-			"imports":    true,
-			"visibility": true,
+			"deps":    true,
+			"main":    true,
+			"srcs":    true,
+			"imports": true,
 		},
 		SubstituteAttrs: map[string]bool{},
 		MergeableAttrs: map[string]bool{
@@ -52,10 +51,9 @@ var pyKinds = map[string]rule.KindInfo{
 		MatchAny:   false,
 		MatchAttrs: []string{"srcs"},
 		NonEmptyAttrs: map[string]bool{
-			"deps":       true,
-			"srcs":       true,
-			"imports":    true,
-			"visibility": true,
+			"deps":    true,
+			"srcs":    true,
+			"imports": true,
 		},
 		SubstituteAttrs: map[string]bool{},
 		MergeableAttrs: map[string]bool{
@@ -68,11 +66,10 @@ var pyKinds = map[string]rule.KindInfo{
 	pyTestKind: {
 		MatchAny: false,
 		NonEmptyAttrs: map[string]bool{
-			"deps":       true,
-			"main":       true,
-			"srcs":       true,
-			"imports":    true,
-			"visibility": true,
+			"deps":    true,
+			"main":    true,
+			"srcs":    true,
+			"imports": true,
 		},
 		SubstituteAttrs: map[string]bool{},
 		MergeableAttrs: map[string]bool{

gazelle/python/testdata/remove_invalid_library/BUILD.in

@@ -0,0 +1,16 @@
+load("@rules_python//python:defs.bzl", "py_library")
+
+py_library(
+    name = "remove_invalid_library",
+    srcs = ["__init__.py"],
+    visibility = ["//:__subpackages__"],
+)
+
+py_library(
+    name = "deps_with_no_srcs_library",
+    deps = [
+        "//:remove_invalid_library",
+        "@pypi//bar",
+        "@pypi//foo",
+    ],
+)

gazelle/python/testdata/remove_invalid_library/BUILD.out

@@ -0,0 +1,10 @@
+load("@rules_python//python:defs.bzl", "py_library")
+
+py_library(
+    name = "deps_with_no_srcs_library",
+    deps = [
+        "//:remove_invalid_library",
+        "@pypi//bar",
+        "@pypi//foo",
+    ],
+)

gazelle/python/testdata/remove_invalid_library/README.md

@@ -0,0 +1,3 @@
+# Remove invalid
+
+This test case asserts that `py_library` should be deleted if invalid.

gazelle/python/testdata/remove_invalid_library/WORKSPACE

@@ -0,0 +1 @@
+# This is a Bazel workspace for the Gazelle test data.

gazelle/python/testdata/remove_invalid_library/test.yaml

@@ -0,0 +1,15 @@
+# Copyright 2023 The Bazel Authors. All rights reserved.
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+---