Automated rollback of changelist 391720461.

*** Reason for rollback ***

rule.name was deprecated

PiperOrigin-RevId: 539850239
Change-Id: I9a447fefea2200f6e99a40e7c43fc299080e0f24

src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkRuleClassFunctions.java

@@ -55,8 +55,6 @@
 import com.google.devtools.build.lib.cmdline.RepositoryName;
 import com.google.devtools.build.lib.events.Event;
 import com.google.devtools.build.lib.events.EventHandler;
-import com.google.devtools.build.lib.events.EventKind;
-import com.google.devtools.build.lib.events.StoredEventHandler;
 import com.google.devtools.build.lib.packages.AllowlistChecker;
 import com.google.devtools.build.lib.packages.AspectsListBuilder.AspectDetails;
 import com.google.devtools.build.lib.packages.Attribute;
@@ -294,7 +292,6 @@ public StarlarkRuleFunction rule(
       Object buildSetting,
       Object cfg,
       Object execGroups,
-      Object name,
       StarlarkThread thread)
       throws EvalException {
     return createRule(
@@ -314,7 +311,6 @@ public StarlarkRuleFunction rule(
         buildSetting,
         cfg,
         execGroups,
-        name,
         thread);
   }
 
@@ -335,7 +331,6 @@ public static StarlarkRuleFunction createRule(
       Object buildSetting,
       Object cfg,
       Object execGroups,
-      Object name,
       StarlarkThread thread)
       throws EvalException {
     BazelStarlarkContext bazelContext = BazelStarlarkContext.from(thread);
@@ -487,41 +482,12 @@ public static StarlarkRuleFunction createRule(
       builder.addExecutionPlatformConstraints(parseExecCompatibleWith(execCompatibleWith, thread));
     }
 
-    StarlarkRuleFunction starlarkRuleFunction =
-        new StarlarkRuleFunction(
-            builder,
-            type,
-            attributes,
-            thread.getCallerLocation(),
-            Starlark.toJavaOptional(doc, String.class));
-    // If a name= parameter is supplied (and we're currently initializing a .bzl module), export the
-    // rule immediately under that name; otherwise the rule will be exported by the postAssignHook
-    // set up in BzlLoadFunction.
-    //
-    // Because exporting can raise multiple errors, we need to accumulate them here into a single
-    // EvalException. This is a code smell because any non-ERROR events will be lost, and any
-    // location
-    // information in the events will be overwritten by the location of this rule's definition.
-    // However, this is currently fine because StarlarkRuleFunction#export only creates events that
-    // are ERRORs and that have the rule definition as their location.
-    // TODO(brandjon): Instead of accumulating events here, consider registering the rule in the
-    // BazelStarlarkContext, and exporting such rules after module evaluation in
-    // BzlLoadFunction#execAndExport.
-    if (name != Starlark.NONE && bzlModule != null) {
-      StoredEventHandler handler = new StoredEventHandler();
-      starlarkRuleFunction.export(handler, bzlModule.label(), (String) name);
-      if (handler.hasErrors()) {
-        StringBuilder errors =
-            handler.getEvents().stream()
-                .filter(e -> e.getKind() == EventKind.ERROR)
-                .reduce(
-                    new StringBuilder(),
-                    (sb, ev) -> sb.append("\n").append(ev.getMessage()),
-                    StringBuilder::append);
-        throw Starlark.errorf("Errors in exporting %s: %s", name, errors.toString());
-      }
-    }
-    return starlarkRuleFunction;
+    return new StarlarkRuleFunction(
+        builder,
+        type,
+        attributes,
+        thread.getCallerLocation(),
+        Starlark.toJavaOptional(doc, String.class));
   }
 
   /**
@@ -855,9 +821,6 @@ private static void validateRulePropagatedAspects(RuleClass ruleClass) throws Ev
     }
 
     /** Export a RuleFunction from a Starlark file with a given name. */
-    // To avoid losing event information in the case where the rule was defined with an explicit
-    // name= arg, all events should be created using errorf(). See the comment in rule() above for
-    // details.
     @Override
     public void export(EventHandler handler, Label starlarkLabel, String ruleClassName) {
       checkState(ruleClass == null && builder != null);

src/main/java/com/google/devtools/build/lib/rules/test/StarlarkTestingModule.java

@@ -89,7 +89,6 @@ public void analysisTest(
             /* buildSetting= */ Starlark.NONE,
             /* cfg= */ Starlark.NONE,
             /* execGroups= */ Starlark.NONE,
-            /* name= */ Starlark.NONE,
             thread);
 
     // Export the rule

src/main/java/com/google/devtools/build/lib/starlarkbuildapi/StarlarkRuleFunctionsApi.java

@@ -441,27 +441,6 @@ Object provider(Object doc, Object fields, Object init, StarlarkThread thread)
                     + " allows rules to run actions on multiple execution platforms within a"
                     + " single target. See <a href='${link exec-groups}'>execution groups"
                     + " documentation</a> for more info."),
-        @Param(
-            name = "name",
-            named = true,
-            defaultValue = "None",
-            positional = false,
-            allowedTypes = {@ParamType(type = String.class), @ParamType(type = NoneType.class)},
-            disableWithFlag = BuildLanguageOptions.INCOMPATIBLE_REMOVE_RULE_NAME_PARAMETER,
-            valueWhenDisabled = "None",
-            doc =
-                "Deprecated: do not use.<p>The name of this rule, as understood by Bazel and"
-                    + " reported in contexts such as logging,"
-                    + " <code>native.existing_rule(...)[kind]</code>, and <code>bazel query</code>."
-                    + " Usually this is the same as the Starlark identifier that gets bound to this"
-                    + " rule; for instance a rule called <code>foo_library</code> would typically"
-                    + " be declared as <code>foo_library = rule(...)</code> and instantiated in a"
-                    + " BUILD file as <code>foo_library(...)</code>.<p>If this parameter is"
-                    + " omitted, the rule's name is set to the name of the first Starlark global"
-                    + " variable to be bound to this rule within its declaring .bzl module. Thus,"
-                    + " <code>foo_library = rule(...)</code> need not specify this parameter if the"
-                    + " name is <code>foo_library</code>.<p>Specifying an explicit name for a rule"
-                    + " does not change where you are allowed to instantiate the rule."),
       },
       useStarlarkThread = true)
   StarlarkCallable rule(
@@ -483,7 +462,6 @@ StarlarkCallable rule(
       Object buildSetting,
       Object cfg,
       Object execGroups,
-      Object name,
       StarlarkThread thread)
       throws EvalException;
 

src/main/java/com/google/devtools/build/skydoc/SkydocMain.java

@@ -307,14 +307,9 @@ public Module eval(
 
     for (Entry<String, Object> envEntry : sortedBindings.entrySet()) {
       if (ruleFunctions.containsKey(envEntry.getValue())) {
-        RuleInfo ruleInfo = ruleFunctions.get(envEntry.getValue()).getRuleInfo().build();
-        // Use symbol name as the rule name only if not already set in the call to rule().
-        if ("".equals(ruleInfo.getRuleName())) {
-          // We make a copy so that additional exports are not affected by setting the rule name on
-          // this builder
-          ruleInfo = ruleInfo.toBuilder().setRuleName(envEntry.getKey()).build();
-        }
-        ruleInfoMap.put(ruleInfo.getRuleName(), ruleInfo);
+        RuleInfo.Builder ruleInfoBuild = ruleFunctions.get(envEntry.getValue()).getRuleInfo();
+        RuleInfo ruleInfo = ruleInfoBuild.setRuleName(envEntry.getKey()).build();
+        ruleInfoMap.put(envEntry.getKey(), ruleInfo);
       }
       if (providerInfos.containsKey(envEntry.getValue())) {
         ProviderInfo.Builder providerInfoBuild =

src/main/java/com/google/devtools/build/skydoc/fakebuildapi/FakeStarlarkRuleFunctionsApi.java

@@ -148,7 +148,6 @@ public StarlarkCallable rule(
       Object buildSetting,
       Object cfg,
       Object execGroups,
-      Object name,
       StarlarkThread thread)
       throws EvalException {
     ImmutableMap.Builder<String, FakeDescriptor> attrsMapBuilder = ImmutableMap.builder();
@@ -166,10 +165,10 @@ public StarlarkCallable rule(
 
     RuleDefinitionIdentifier functionIdentifier = new RuleDefinitionIdentifier();
 
-    // Only the Builder is passed to RuleInfoWrapper as the rule name may not be available yet.
+    // Only the Builder is passed to RuleInfoWrapper as the rule name is not yet available.
     RuleInfo.Builder ruleInfo = RuleInfo.newBuilder().addAllAttribute(attrInfos);
     Starlark.toJavaOptional(doc, String.class).ifPresent(ruleInfo::setDocString);
-    Starlark.toJavaOptional(name, String.class).ifPresent(ruleInfo::setRuleName);
+
     Location loc = thread.getCallerLocation();
     ruleInfoList.add(new RuleInfoWrapper(functionIdentifier, loc, ruleInfo));
 

src/test/java/com/google/devtools/build/lib/starlark/StarlarkRuleClassFunctionsTest.java

@@ -992,73 +992,6 @@ public void testExportAliasedName() throws Exception {
     assertThat(fooName).isEqualTo("d");
   }
 
-  @Test
-  public void testExportWithSpecifiedName() throws Exception {
-    setBuildLanguageOptions("--noincompatible_remove_rule_name_parameter");
-    evalAndExport(
-        ev, //
-        "def _impl(ctx): pass",
-        "a = rule(implementation = _impl, name = 'r')",
-        "z = a");
-
-    String aName = ((StarlarkRuleFunction) ev.lookup("a")).getRuleClass().getName();
-    assertThat(aName).isEqualTo("r");
-    String zName = ((StarlarkRuleFunction) ev.lookup("z")).getRuleClass().getName();
-    assertThat(zName).isEqualTo("r");
-  }
-
-  @Test
-  public void testExportWithSpecifiedNameFailure() throws Exception {
-    setBuildLanguageOptions("--noincompatible_remove_rule_name_parameter");
-    ev.setFailFast(false);
-
-    evalAndExport(
-        ev, //
-        "def _impl(ctx): pass",
-        "rule(implementation = _impl, name = '1a')");
-
-    ev.assertContainsError("Invalid rule name: 1a");
-  }
-
-  @Test
-  public void testExportWithNonStringNameFailsCleanly() throws Exception {
-    setBuildLanguageOptions("--noincompatible_remove_rule_name_parameter");
-    ev.setFailFast(false);
-
-    evalAndExport(
-        ev, //
-        "def _impl(ctx): pass",
-        "rule(implementation = _impl, name = {'not_a_string': True})");
-
-    ev.assertContainsError("got value of type 'dict', want 'string or NoneType'");
-  }
-
-  @Test
-  public void testExportWithMultipleErrors() throws Exception {
-    setBuildLanguageOptions("--noincompatible_remove_rule_name_parameter");
-    ev.setFailFast(false);
-
-    evalAndExport(
-        ev,
-        "def _impl(ctx): pass",
-        "rule(",
-        "  implementation = _impl,",
-        "  attrs = {",
-        "    'name' : attr.string(),",
-        "    'tags' : attr.string_list(),",
-        "  },",
-        "  name = '1a',",
-        ")");
-
-    ev.assertContainsError(
-        "Error in rule: Errors in exporting 1a: \n"
-            + "cannot add attribute: There is already a built-in attribute 'name' which cannot be"
-            + " overridden.\n"
-            + "cannot add attribute: There is already a built-in attribute 'tags' which cannot be"
-            + " overridden.\n"
-            + "Invalid rule name: 1a");
-  }
-
   @Test
   public void testOutputToGenfiles() throws Exception {
     evalAndExport(ev, "def impl(ctx): pass", "r1 = rule(impl, output_to_genfiles=True)");

src/test/java/com/google/devtools/build/skydoc/SkydocTest.java

@@ -233,64 +233,6 @@ public void testRuleWithMultipleExports() throws Exception {
     assertThat(ruleInfoMap.buildOrThrow().keySet()).containsExactly("rule_one", "rule_two");
   }
 
-  @Test
-  public void testRuleExportedWithSpecifiedName() throws Exception {
-    scratchRunfile(
-        "io_bazel/test/test.bzl",
-        "def rule_impl(ctx):",
-        "  return []",
-        "",
-        "rule_one = rule(",
-        "    doc = 'Rule one',",
-        "    implementation = rule_impl,",
-        "    name = 'rule_one_exported_name',",
-        ")");
-
-    ImmutableMap.Builder<String, RuleInfo> ruleInfoMap = ImmutableMap.builder();
-
-    Module unused =
-        skydocMain.eval(
-            StarlarkSemantics.builder()
-                .setBool(BuildLanguageOptions.INCOMPATIBLE_REMOVE_RULE_NAME_PARAMETER, false)
-                .build(),
-            Label.parseCanonicalUnchecked("//test:test.bzl"),
-            ruleInfoMap,
-            ImmutableMap.builder(),
-            ImmutableMap.builder(),
-            ImmutableMap.builder());
-
-    assertThat(ruleInfoMap.buildOrThrow().keySet()).containsExactly("rule_one_exported_name");
-  }
-
-  @Test
-  public void testUnassignedRuleNotDocumented() throws Exception {
-    scratchRunfile(
-        "io_bazel/test/test.bzl",
-        "def rule_impl(ctx):",
-        "  return []",
-        "",
-        "rule(",
-        "    doc = 'Undocumented rule',",
-        "    implementation = rule_impl,",
-        "    name = 'rule_exported_name',",
-        ")");
-
-    ImmutableMap.Builder<String, RuleInfo> ruleInfoMap = ImmutableMap.builder();
-
-    Module unused =
-        skydocMain.eval(
-            StarlarkSemantics.builder()
-                .setBool(BuildLanguageOptions.INCOMPATIBLE_REMOVE_RULE_NAME_PARAMETER, false)
-                .build(),
-            Label.parseCanonicalUnchecked("//test:test.bzl"),
-            ruleInfoMap,
-            ImmutableMap.builder(),
-            ImmutableMap.builder(),
-            ImmutableMap.builder());
-
-    assertThat(ruleInfoMap.buildOrThrow().keySet()).isEmpty();
-  }
-
   @Test
   public void testRulesAcrossMultipleFiles() throws Exception {
     scratchRunfile("io_bazel/lib/rule_impl.bzl", "def rule_impl(ctx):", "  return []");