From 682a87c7025f4b58bb2fb1c75c608134ee00e559 Mon Sep 17 00:00:00 2001
From: Michael Mitchell <michaelm@openai.com>
Date: Mon, 26 Jan 2026 23:41:12 -0500
Subject: [PATCH] Allow repo rules to define previously reserved attributes

---
 .../build/lib/bazel/repository/RepoRule.java  | 16 +++----
 .../StarlarkRepositoryIntegrationTest.java    | 48 +++++++++++++++++++
 2 files changed, 55 insertions(+), 9 deletions(-)

diff --git a/src/main/java/com/google/devtools/build/lib/bazel/repository/RepoRule.java b/src/main/java/com/google/devtools/build/lib/bazel/repository/RepoRule.java
index 1c555401f3ec74..45bdc9b22bcbbe 100644
--- a/src/main/java/com/google/devtools/build/lib/bazel/repository/RepoRule.java
+++ b/src/main/java/com/google/devtools/build/lib/bazel/repository/RepoRule.java
@@ -64,12 +64,8 @@ public record RepoRule(
     boolean remotable,
     ImmutableSet<String> environ) {
 
-  /**
-   * A list of forbidden attribute names. These used to be present on all repo rules simply because
-   * they were built-in attributes for all rules.
-   */
-  private static final ImmutableSet<String> LEGACY_BUILTIN_ATTRIBUTES =
-      ImmutableSet.of("name", "tags", "deprecation", "visibility");
+  /** A list of reserved attribute names. */
+  private static final ImmutableSet<String> RESERVED_ATTRIBUTES = ImmutableSet.of("name");
 
   /** Supplies a {@link RepoRule} instance. */
   public interface Supplier {
@@ -98,7 +94,9 @@ public RepoSpec instantiate(
           AttributeUtils.typeCheckAttrValues(
               attributes,
               attributeIndices,
-              Maps.filterKeys(kwargs, k -> !LEGACY_BUILTIN_ATTRIBUTES.contains(k)),
+              Maps.filterKeys(
+                  kwargs,
+                  k -> !RESERVED_ATTRIBUTES.contains(k)),
               labelConverter,
               ExternalDeps.Code.EXTENSION_EVAL_ERROR,
               callStack,
@@ -107,7 +105,7 @@ public RepoSpec instantiate(
       var attrDict = Dict.<String, Object>builder();
       for (Map.Entry<String, Object> kwarg : kwargs.entrySet()) {
         // Only store explicitly-specified attributes.
-        if (!LEGACY_BUILTIN_ATTRIBUTES.contains(kwarg.getKey())
+        if (!RESERVED_ATTRIBUTES.contains(kwarg.getKey())
             && !Starlark.isNullOrNone(kwarg.getValue())) {
           attrDict.put(kwarg.getKey(), attrValues.get(attributeIndices.get(kwarg.getKey())));
         }
@@ -152,7 +150,7 @@ public final Builder addAttribute(Attribute attribute) {
     }
 
     public final boolean hasAttribute(String attrName) {
-      return attrNames.contains(attrName) || LEGACY_BUILTIN_ATTRIBUTES.contains(attrName);
+      return attrNames.contains(attrName) || RESERVED_ATTRIBUTES.contains(attrName);
     }
 
     public abstract Builder local(boolean value);
diff --git a/src/test/java/com/google/devtools/build/lib/bazel/repository/starlark/StarlarkRepositoryIntegrationTest.java b/src/test/java/com/google/devtools/build/lib/bazel/repository/starlark/StarlarkRepositoryIntegrationTest.java
index e55ff8d102b9db..d8306e3969573d 100644
--- a/src/test/java/com/google/devtools/build/lib/bazel/repository/starlark/StarlarkRepositoryIntegrationTest.java
+++ b/src/test/java/com/google/devtools/build/lib/bazel/repository/starlark/StarlarkRepositoryIntegrationTest.java
@@ -211,6 +211,54 @@ def _impl(ctx):
         .contains("There is already a built-in attribute 'name' " + "which cannot be overridden");
   }
 
+  @Test
+  public void testRepositoryRuleCanDefineVisibilityAttr() throws Exception {
+    scratch.file(
+        "def.bzl",
+        """
+        def _impl(ctx):
+            ctx.file("BUILD.bazel", "filegroup(name='x')")
+            _ = ctx.attr.visibility
+
+        repo = repository_rule(
+            implementation = _impl,
+            attrs = {"visibility": attr.string_list(default = [])},
+        )
+        """);
+    scratch.file(rootDirectory.getRelative("BUILD").getPathString());
+    scratch.overwriteFile(
+        rootDirectory.getRelative("MODULE.bazel").getPathString(),
+        "repo = use_repo_rule('//:def.bzl', 'repo')",
+        "repo(name='foo')");
+
+    invalidatePackages();
+    getConfiguredTargetAndData("@@+repo+foo//:x");
+  }
+
+  @Test
+  public void testRepositoryRuleAccessingUndefinedVisibilityAttrFails() throws Exception {
+    scratch.file(
+        "def.bzl",
+        """
+        def _impl(ctx):
+            _ = ctx.attr.visibility
+
+        repo = repository_rule(
+            implementation = _impl,
+        )
+        """);
+    scratch.file(rootDirectory.getRelative("BUILD").getPathString());
+    scratch.overwriteFile(
+        rootDirectory.getRelative("MODULE.bazel").getPathString(),
+        "repo = use_repo_rule('//:def.bzl', 'repo')",
+        "repo(name='foo')");
+
+    invalidatePackages();
+    AssertionError e =
+        assertThrows(AssertionError.class, () -> getConfiguredTarget("@@+repo+foo//:x"));
+    assertThat(e).hasMessageThat().contains("unknown attribute visibility");
+  }
+
   @Test
   public void testCallRepositoryRuleFromBuildFile() throws Exception {
     // Check that we get a proper error when calling a repository rule from a BUILD file.