buildozer: only remove visibility matching explicit default_visibility

Only remove visibility attributes if default_visibility is explicitly
defined in a package() declaration. Support unordered string list
comparisons and identifier matching.

edit/fix.go

diff --git a/edit/fix.go b/edit/fix.go
index 0643caa..e2fc221 100644
--- a/edit/fix.go
+++ b/edit/fix.go
@@ -20,6 +20,7 @@
 
 import (
 	"regexp"
+	"slices"
 	"sort"
 	"strings"
 
@@ -100,60 +101,38 @@
 	return fixed
 }
 
-// removeVisibility removes useless visibility attributes.
+// removeVisibility removes useless visibility attributes (repeating default_visibility).
 func removeVisibility(f *build.File, r *build.Rule, pkg string) bool {
-	// Do not remove visibility from macros or loaded rules, as they may have custom
-	// default visibility logic (e.g. falling back to a non-private default if visibility
-	// is omitted).
-	if isMacroOrLoadedRule(f, r) {
+	pkgDecl := ExistingPackageDeclaration(f)
+	if pkgDecl == nil || pkgDecl.Attr("default_visibility") == nil {
+		// If default visibility is not explicit we do not replace other attrs.
 		return false
 	}
 
-	// If no default_visibility is given, it is implicitly private.
-	defaultVisibility := []string{"//visibility:private"}
-	if pkgDecl := ExistingPackageDeclaration(f); pkgDecl != nil {
-		if pkgDecl.Attr("default_visibility") != nil {
-			defaultVisibility = pkgDecl.AttrStrings("default_visibility")
+	defaultVis := pkgDecl.Attr("default_visibility")
+	vis := r.Attr("visibility")
+
+	// Check equality if both are string lists.
+	if dvs := build.Strings(defaultVis); dvs != nil {
+		if vs := build.Strings(vis); vs != nil {
+			if slices.Equal(slices.Sorted(slices.Values(dvs)), slices.Sorted(slices.Values(vs))) {
+				r.DelAttr("visibility")
+				return true
+			}
 		}
 	}
-
-	visibility := r.AttrStrings("visibility")
-	if len(visibility) == 0 || len(visibility) != len(defaultVisibility) {
-		return false
-	}
-	sort.Strings(defaultVisibility)
-	sort.Strings(visibility)
-	for i, vis := range visibility {
-		if vis != defaultVisibility[i] {
-			return false
-		}
-	}
-	r.DelAttr("visibility")
-	return true
-}
-
-// isMacroOrLoadedRule reports whether the rule is a macro or was loaded from a .bzl file.
-func isMacroOrLoadedRule(f *build.File, r *build.Rule) bool {
-	if f == nil || r == nil {
-		return false
-	}
-	kind := r.Kind()
-	if strings.Contains(kind, ".") {
-		return true
-	}
-	for _, stmt := range f.Stmt {
-		if load, ok := stmt.(*build.LoadStmt); ok {
-			for _, to := range load.To {
-				if to.Name == kind {
-					return true
-				}
+	// Check Name equality if both are idents.
+	if ai, aok := defaultVis.(*build.Ident); aok {
+		if bi, bok := vis.(*build.Ident); bok {
+			if ai.Name == bi.Name {
+				r.DelAttr("visibility")
+				return true
 			}
 		}
 	}
 	return false
 }
 
-
 // removeTestOnly removes the useless testonly attributes.
 func removeTestOnly(f *build.File, r *build.Rule, pkg string) bool {
 	pkgDecl := ExistingPackageDeclaration(f)

edit/fix_test.go

diff --git a/edit/fix_test.go b/edit/fix_test.go
index ccce057..0cebe72 100644
--- a/edit/fix_test.go
+++ b/edit/fix_test.go
@@ -112,74 +112,79 @@
 `,
 		},
 		{
-			name: "remove redundant visibility for native rule",
+			name: "preserve visibility when default_visibility is not set",
 			input: `cc_library(
     name = "native_lib",
     visibility = ["//visibility:private"],
 )
 `,
-			want: `cc_library(name = "native_lib")
-`,
-		},
-		{
-			name: "preserve private visibility for dotted macro",
-			input: `load("//foo:bar.bzl", "rules")
-
-rules.my_library(
-    name = "macro_lib",
-    visibility = ["//visibility:private"],
-)
-`,
-			want: `load("//foo:bar.bzl", "rules")
-
-rules.my_library(
-    name = "macro_lib",
+			want: `cc_library(
+    name = "native_lib",
     visibility = ["//visibility:private"],
 )
 `,
 		},
 		{
-			name: "preserve private visibility for loaded macro",
-			input: `load("//foo:bar.bzl", "my_macro")
-
-my_macro(
-    name = "macro_lib",
-    visibility = ["//visibility:private"],
-)
-`,
-			want: `load("//foo:bar.bzl", "my_macro")
-
-my_macro(
-    name = "macro_lib",
-    visibility = ["//visibility:private"],
-)
-`,
-		},
-		{
-			name: "package default visibility removes for native rule but preserves for loaded macro",
-			input: `load("//foo:bar.bzl", "my_macro")
-
-package(default_visibility = ["//visibility:public"])
+			name: "remove redundant visibility with different slice order",
+			input: `package(default_visibility = [
+    "//foo",
+    "//bar",
+])
 
 cc_library(
     name = "native_lib",
-    visibility = ["//visibility:public"],
-)
-
-my_macro(
-    name = "macro_lib",
-    visibility = ["//visibility:public"],
+    visibility = [
+        "//bar",
+        "//foo",
+    ],
 )
 `,
-			want: `load("//foo:bar.bzl", "my_macro")
+			want: `package(default_visibility = [
+    "//bar",
+    "//foo",
+])
 
-package(default_visibility = ["//visibility:public"])
+cc_library(name = "native_lib")
+`,
+		},
+		{
+			name: "remove redundant visibility matching ident",
+			input: `package(default_visibility = PUBLIC)
+
+cc_library(
+    name = "native_lib",
+    visibility = PUBLIC,
+)
+
+cc_library(
+    name = "other_lib",
+    visibility = PRIVATE,
+)
+`,
+			want: `package(default_visibility = PUBLIC)
 
 cc_library(name = "native_lib")
 
-my_macro(
-    name = "macro_lib",
-    visibility = ["//visibility:public"],
+cc_library(
+    name = "other_lib",
+    visibility = PRIVATE,
+)
+`,
+		},
+		{
+			name: "preserve non-matching visibility",
+			input: `package(default_visibility = ["//visibility:public"])
+
+cc_library(
+    name = "native_lib",
+    visibility = ["//visibility:private"],
+)
+`,
+			want: `package(default_visibility = ["//visibility:public"])
+
+cc_library(
+    name = "native_lib",
+    visibility = ["//visibility:private"],
 )
 `,
 		},