Remove go_tool_library references.

This is required only for the built-in bazel nogo functionality.
Since we roll these targets manually via the wrappers, we don't need
to use go_tool_library. The inconsistent use of these targets leads
to conflicting instantiations of go_default_library and go_tool_library,
which both contain the same output files.

PiperOrigin-RevId: 355184975

9 files changed, 86 insertions, 35 deletions

diff --git a/WORKSPACE b/WORKSPACE
index efa55248f..494541a25 100644
--- a/WORKSPACE
+++ b/WORKSPACE
@@ -8,7 +8,7 @@ load("@bazel_tools//tools/build_defs/repo:git.bzl", "git_repository")
 http_file(
     name = "google_root_pem",
     urls = [
-        "https://pki.goog/roots.pem"
+        "https://pki.goog/roots.pem",
     ],
 )
 
@@ -36,9 +36,11 @@ http_archive(
     name = "io_bazel_rules_go",
     patch_args = ["-p1"],
     patches = [
+        # Ensure we don't destroy the facts visibility.
+        "//tools:rules_go_visibility.patch",
         # Newer versions of the rules_go rules will automatically strip test
         # binaries of symbols, which we don't want.
-        "//tools:rules_go.patch",
+        "//tools:rules_go_symbols.patch",
     ],
     sha256 = "8e9434015ff8f3d6962cb8f016230ea7acc1ac402b760a8d66ff54dc11673ca6",
     urls = [
@@ -51,9 +53,13 @@ http_archive(
     name = "bazel_gazelle",
     patch_args = ["-p1"],
     patches = [
+        # Fix permissions for facts for go_library, not just tool library.
+        # This is actually a no-op with the hacky patch above, but should
+        # slightly future proof this mechanism.
+        "//tools:bazel_gazelle_generate.patch",
         # False positive output complaining about Go logrus versions spam the
         # logs. Strip this message in this case. Does not affect control flow.
-        "//tools:bazel_gazelle.patch",
+        "//tools:bazel_gazelle_noise.patch",
     ],
     sha256 = "b85f48fa105c4403326e9525ad2b2cc437babaa6e15a3fc0b1dbab0ab064bc7c",
     urls = [
diff --git a/tools/bazel_gazelle_generate.patch b/tools/bazel_gazelle_generate.patch
new file mode 100644
index 000000000..fd1e1bda6
--- /dev/null
+++ b/tools/bazel_gazelle_generate.patch
@@ -0,0 +1,15 @@
+diff --git a/language/go/generate.go b/language/go/generate.go
+index 2892948..feb4ad6 100644
+--- a/language/go/generate.go
++++ b/language/go/generate.go
+@@ -691,6 +691,10 @@ func (g *generator) setImportAttrs(r *rule.Rule, importPath string) {
+ }
+ 
+ func (g *generator) commonVisibility(importPath string) []string {
++	if importPath == "golang.org/x/tools/go/analysis/internal/facts" {
++		// Imported by nogo main. We add a visibility exception.
++		return []string{"//visibility:public"}
++	}
+ 	// If the Bazel package name (rel) contains "internal", add visibility for
+ 	// subpackages of the parent.
+ 	// If the import path contains "internal" but rel does not, this is
diff --git a/tools/bazel_gazelle.patch b/tools/bazel_gazelle_noise.patch
index e35f38933..e35f38933 100644
--- a/tools/bazel_gazelle.patch
+++ b/tools/bazel_gazelle_noise.patch
diff --git a/tools/checkescape/BUILD b/tools/checkescape/BUILD
index 8956be621..940538b9e 100644
--- a/tools/checkescape/BUILD
+++ b/tools/checkescape/BUILD
@@ -8,8 +8,8 @@ go_library(
     nogo = False,
     visibility = ["//tools/nogo:__subpackages__"],
     deps = [
-        "@org_golang_x_tools//go/analysis:go_tool_library",
-        "@org_golang_x_tools//go/analysis/passes/buildssa:go_tool_library",
-        "@org_golang_x_tools//go/ssa:go_tool_library",
+        "@org_golang_x_tools//go/analysis:go_default_library",
+        "@org_golang_x_tools//go/analysis/passes/buildssa:go_default_library",
+        "@org_golang_x_tools//go/ssa:go_default_library",
     ],
 )
diff --git a/tools/checkunsafe/BUILD b/tools/checkunsafe/BUILD
index 0c264151b..0bb07b415 100644
--- a/tools/checkunsafe/BUILD
+++ b/tools/checkunsafe/BUILD
@@ -8,6 +8,6 @@ go_library(
     nogo = False,
     visibility = ["//tools/nogo:__subpackages__"],
     deps = [
-        "@org_golang_x_tools//go/analysis:go_tool_library",
+        "@org_golang_x_tools//go/analysis:go_default_library",
     ],
 )
diff --git a/tools/nogo/BUILD b/tools/nogo/BUILD
index 566e0889e..7976c7521 100644
--- a/tools/nogo/BUILD
+++ b/tools/nogo/BUILD
@@ -38,34 +38,34 @@ go_library(
         "//tools/checkunsafe",
         "@co_honnef_go_tools//staticcheck:go_default_library",
         "@co_honnef_go_tools//stylecheck:go_default_library",
-        "@org_golang_x_tools//go/analysis:go_tool_library",
-        "@org_golang_x_tools//go/analysis/internal/facts:go_tool_library",
-        "@org_golang_x_tools//go/analysis/passes/asmdecl:go_tool_library",
-        "@org_golang_x_tools//go/analysis/passes/assign:go_tool_library",
-        "@org_golang_x_tools//go/analysis/passes/atomic:go_tool_library",
-        "@org_golang_x_tools//go/analysis/passes/bools:go_tool_library",
-        "@org_golang_x_tools//go/analysis/passes/buildtag:go_tool_library",
-        "@org_golang_x_tools//go/analysis/passes/cgocall:go_tool_library",
-        "@org_golang_x_tools//go/analysis/passes/composite:go_tool_library",
-        "@org_golang_x_tools//go/analysis/passes/copylock:go_tool_library",
-        "@org_golang_x_tools//go/analysis/passes/errorsas:go_tool_library",
-        "@org_golang_x_tools//go/analysis/passes/httpresponse:go_tool_library",
-        "@org_golang_x_tools//go/analysis/passes/loopclosure:go_tool_library",
-        "@org_golang_x_tools//go/analysis/passes/lostcancel:go_tool_library",
-        "@org_golang_x_tools//go/analysis/passes/nilfunc:go_tool_library",
-        "@org_golang_x_tools//go/analysis/passes/nilness:go_tool_library",
-        "@org_golang_x_tools//go/analysis/passes/printf:go_tool_library",
-        "@org_golang_x_tools//go/analysis/passes/shadow:go_tool_library",
-        "@org_golang_x_tools//go/analysis/passes/shift:go_tool_library",
-        "@org_golang_x_tools//go/analysis/passes/stdmethods:go_tool_library",
-        "@org_golang_x_tools//go/analysis/passes/stringintconv:go_tool_library",
-        "@org_golang_x_tools//go/analysis/passes/structtag:go_tool_library",
-        "@org_golang_x_tools//go/analysis/passes/tests:go_tool_library",
-        "@org_golang_x_tools//go/analysis/passes/unmarshal:go_tool_library",
-        "@org_golang_x_tools//go/analysis/passes/unreachable:go_tool_library",
-        "@org_golang_x_tools//go/analysis/passes/unsafeptr:go_tool_library",
-        "@org_golang_x_tools//go/analysis/passes/unusedresult:go_tool_library",
-        "@org_golang_x_tools//go/gcexportdata:go_tool_library",
+        "@org_golang_x_tools//go/analysis:go_default_library",
+        "@org_golang_x_tools//go/analysis/internal/facts:go_default_library",
+        "@org_golang_x_tools//go/analysis/passes/asmdecl:go_default_library",
+        "@org_golang_x_tools//go/analysis/passes/assign:go_default_library",
+        "@org_golang_x_tools//go/analysis/passes/atomic:go_default_library",
+        "@org_golang_x_tools//go/analysis/passes/bools:go_default_library",
+        "@org_golang_x_tools//go/analysis/passes/buildtag:go_default_library",
+        "@org_golang_x_tools//go/analysis/passes/cgocall:go_default_library",
+        "@org_golang_x_tools//go/analysis/passes/composite:go_default_library",
+        "@org_golang_x_tools//go/analysis/passes/copylock:go_default_library",
+        "@org_golang_x_tools//go/analysis/passes/errorsas:go_default_library",
+        "@org_golang_x_tools//go/analysis/passes/httpresponse:go_default_library",
+        "@org_golang_x_tools//go/analysis/passes/loopclosure:go_default_library",
+        "@org_golang_x_tools//go/analysis/passes/lostcancel:go_default_library",
+        "@org_golang_x_tools//go/analysis/passes/nilfunc:go_default_library",
+        "@org_golang_x_tools//go/analysis/passes/nilness:go_default_library",
+        "@org_golang_x_tools//go/analysis/passes/printf:go_default_library",
+        "@org_golang_x_tools//go/analysis/passes/shadow:go_default_library",
+        "@org_golang_x_tools//go/analysis/passes/shift:go_default_library",
+        "@org_golang_x_tools//go/analysis/passes/stdmethods:go_default_library",
+        "@org_golang_x_tools//go/analysis/passes/stringintconv:go_default_library",
+        "@org_golang_x_tools//go/analysis/passes/structtag:go_default_library",
+        "@org_golang_x_tools//go/analysis/passes/tests:go_default_library",
+        "@org_golang_x_tools//go/analysis/passes/unmarshal:go_default_library",
+        "@org_golang_x_tools//go/analysis/passes/unreachable:go_default_library",
+        "@org_golang_x_tools//go/analysis/passes/unsafeptr:go_default_library",
+        "@org_golang_x_tools//go/analysis/passes/unusedresult:go_default_library",
+        "@org_golang_x_tools//go/gcexportdata:go_default_library",
     ],
 )
 
diff --git a/tools/nogo/defs.bzl b/tools/nogo/defs.bzl
index 161ea972e..0c48a7a5a 100644
--- a/tools/nogo/defs.bzl
+++ b/tools/nogo/defs.bzl
@@ -188,6 +188,14 @@ def _nogo_aspect_impl(target, ctx):
     # All work is done in the shadow properties for go rules. For a proto
     # library, we simply skip the analysis portion but still need to return a
     # valid NogoInfo to reference the generated binary.
+    #
+    # Note that we almost exclusively use go_library, not go_tool_library.
+    # This is because nogo is manually annotated, so the go_tool_library kind
+    # is not needed to avoid dependency loops. Unfortunately, bazel coverdata
+    # is exported *only* as a go_tool_library. This does not cause a problem,
+    # since there is guaranteed to be no conflict. However for consistency,
+    # we should not introduce new go_tool_library dependencies unless strictly
+    # necessary.
     if ctx.rule.kind in ("go_library", "go_tool_library", "go_binary", "go_test"):
         srcs = ctx.rule.files.srcs
         deps = ctx.rule.attr.deps
diff --git a/tools/rules_go.patch b/tools/rules_go_symbols.patch
index 5e1e87084..5e1e87084 100644
--- a/tools/rules_go.patch
+++ b/tools/rules_go_symbols.patch
diff --git a/tools/rules_go_visibility.patch b/tools/rules_go_visibility.patch
new file mode 100644
index 000000000..e5bb2e3d5
--- /dev/null
+++ b/tools/rules_go_visibility.patch
@@ -0,0 +1,22 @@
+diff --git a/third_party/org_golang_x_tools-gazelle.patch b/third_party/org_golang_x_tools-gazelle.patch
+index 7bdacff5..2fe9ce93 100644
+--- a/third_party/org_golang_x_tools-gazelle.patch
++++ b/third_party/org_golang_x_tools-gazelle.patch
+@@ -2054,7 +2054,7 @@ diff -urN b/go/analysis/internal/facts/BUILD.bazel c/go/analysis/internal/facts/
+ +        "imports.go",
+ +    ],
+ +    importpath = "golang.org/x/tools/go/analysis/internal/facts",
+-+    visibility = ["//go/analysis:__subpackages__"],
+++    visibility = ["//visibility:public"],
+ +    deps = [
+ +        "//go/analysis",
+ +        "//go/types/objectpath",
+@@ -2078,7 +2078,7 @@ diff -urN b/go/analysis/internal/facts/BUILD.bazel c/go/analysis/internal/facts/
+ +alias(
+ +    name = "go_default_library",
+ +    actual = ":facts",
+-+    visibility = ["//go/analysis:__subpackages__"],
+++    visibility = ["//visibility:public"],
+ +)
+ +
+ +go_test(