Move analysis_test into testing.analysis_test (#16702)

* Move analysis_test into testing.analysis_test

Exporting it on a toplevel makes it impossible to override its name without a workaround.

A temporary workaround is provided that should be in place until this is released in Bazel 3.6.

RELNOTES[INC]: analysis_test moved into testing.analysis_test

PiperOrigin-RevId: 471806624
Change-Id: I13bd1f5b38e5fb538d4c046952b9db7b82b3f31a

* Passing useToolchainResolution to make it compatible with Bazel 5.x.x

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

@@ -100,7 +100,7 @@
 import com.google.errorprone.annotations.FormatMethod;
 import java.util.Collection;
 import java.util.Map;
-import java.util.regex.Pattern;
+import java.util.Objects;
 import javax.annotation.Nullable;
 import net.starlark.java.eval.Debug;
 import net.starlark.java.eval.Dict;
@@ -112,7 +112,6 @@
 import net.starlark.java.eval.StarlarkCallable;
 import net.starlark.java.eval.StarlarkFunction;
 import net.starlark.java.eval.StarlarkInt;
-import net.starlark.java.eval.StarlarkList;
 import net.starlark.java.eval.StarlarkThread;
 import net.starlark.java.eval.Tuple;
 import net.starlark.java.syntax.Identifier;
@@ -140,7 +139,6 @@ public Label load(String from) throws Exception {
                   }
                 }
               });
-  private static final Pattern RULE_NAME_PATTERN = Pattern.compile("[A-Za-z_][A-Za-z0-9_]*");
 
   // TODO(bazel-team): Remove the code duplication (BaseRuleClasses and this class).
   /** Parent rule class for non-executable non-test Starlark rules. */
@@ -308,6 +306,51 @@ public StarlarkRuleFunction rule(
       Object name,
       StarlarkThread thread)
       throws EvalException {
+    return createRule(
+        implementation,
+        test,
+        attrs,
+        implicitOutputs,
+        executable,
+        outputToGenfiles,
+        fragments,
+        hostFragments,
+        starlarkTestable,
+        toolchains,
+        useToolchainTransition,
+        providesArg,
+        execCompatibleWith,
+        analysisTest,
+        buildSetting,
+        cfg,
+        execGroups,
+        compileOneFiletype,
+        name,
+        thread);
+  }
+
+  public static StarlarkRuleFunction createRule(
+      StarlarkFunction implementation,
+      boolean test,
+      Object attrs,
+      Object implicitOutputs,
+      boolean executable,
+      boolean outputToGenfiles,
+      Sequence<?> fragments,
+      Sequence<?> hostFragments,
+      boolean starlarkTestable,
+      Sequence<?> toolchains,
+      boolean useToolchainTransition,
+      Sequence<?> providesArg,
+      Sequence<?> execCompatibleWith,
+      Object analysisTest,
+      Object buildSetting,
+      Object cfg,
+      Object execGroups,
+      Object compileOneFiletype,
+      Object name,
+      StarlarkThread thread)
+      throws EvalException {
     BazelStarlarkContext bazelContext = BazelStarlarkContext.from(thread);
     bazelContext.checkLoadingOrWorkspacePhase("rule");
     // analysis_test=true implies test=true.
@@ -510,81 +553,6 @@ public StarlarkRuleFunction rule(
     return starlarkRuleFunction;
   }
 
-  @Override
-  public void analysisTest(
-      String name,
-      StarlarkFunction implementation,
-      Object attrs,
-      Sequence<?> fragments,
-      Sequence<?> toolchains,
-      Object attrValuesApi,
-      StarlarkThread thread)
-      throws EvalException, InterruptedException {
-    if (!RULE_NAME_PATTERN.matcher(name).matches()) {
-      throw Starlark.errorf("'name' is limited to Starlark identifiers, got %s", name);
-    }
-    Dict<String, Object> attrValues =
-        Dict.cast(attrValuesApi, String.class, Object.class, "attr_values");
-    if (attrValues.containsKey("name")) {
-      throw Starlark.errorf("'name' cannot be set or overridden in 'attr_values'");
-    }
-
-    StarlarkRuleFunction starlarkRuleFunction =
-        rule(
-            implementation,
-            /*test=*/ true,
-            attrs,
-            /*implicitOutputs=*/ Starlark.NONE,
-            /*executable=*/ false,
-            /*outputToGenfiles=*/ false,
-            /*fragments=*/ fragments,
-            /*hostFragments=*/ StarlarkList.empty(),
-            /*starlarkTestable=*/ false,
-            /*toolchains=*/ toolchains,
-            /*useToolchainTransition=*/ false,
-            /*doc=*/ "",
-            /*providesArg=*/ StarlarkList.empty(),
-            /*execCompatibleWith=*/ StarlarkList.empty(),
-            /*analysisTest=*/ Boolean.TRUE,
-            /*buildSetting=*/ Starlark.NONE,
-            /*cfg=*/ Starlark.NONE,
-            /*execGroups=*/ Starlark.NONE,
-            /*compileOneFiletype=*/ Starlark.NONE,
-            /*name=*/ Starlark.NONE,
-            thread);
-
-    // Export the rule
-    // 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.
-    PackageContext pkgContext = thread.getThreadLocal(PackageContext.class);
-    StoredEventHandler handler = new StoredEventHandler();
-    starlarkRuleFunction.export(
-        handler, pkgContext.getLabel(), name + "_test"); // export in BUILD thread
-    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());
-    }
-
-    // Instantiate the target
-    Dict.Builder<String, Object> args = Dict.builder();
-    args.put("name", name);
-    args.putAll(attrValues);
-    starlarkRuleFunction.call(thread, Tuple.of(), args.buildImmutable());
-  }
-
   /**
    * Returns the module (file) of the outermost enclosing Starlark function on the call stack or
    * null if none of the active calls are functions defined in Starlark.
@@ -1002,6 +970,7 @@ private void errorf(EventHandler handler, String format, Object... args) {
       handler.handle(Event.error(definitionLocation, String.format(format, args)));
     }
 
+    @Override
     public RuleClass getRuleClass() {
       Preconditions.checkState(ruleClass != null && builder == null);
       return ruleClass;

src/main/java/com/google/devtools/build/lib/rules/BUILD

@@ -131,6 +131,7 @@ java_library(
         "//src/main/java/com/google/devtools/build/lib/analysis:transitive_info_collection",
         "//src/main/java/com/google/devtools/build/lib/analysis:transitive_info_provider",
         "//src/main/java/com/google/devtools/build/lib/concurrent",
+        "//src/main/java/com/google/devtools/build/lib/events",
         "//src/main/java/com/google/devtools/build/lib/packages",
         "//src/main/java/com/google/devtools/build/lib/starlarkbuildapi/test",
         "//src/main/java/com/google/devtools/build/lib/util",

src/main/java/com/google/devtools/build/lib/rules/config/ConfigGlobalLibrary.java

@@ -68,6 +68,7 @@ public ConfigurationTransitionApi transition(
         moduleContext.repoMapping());
   }
 
+  // TODO(b/237422931): move into testing module
   @Override
   public ConfigurationTransitionApi analysisTestTransition(
       Dict<?, ?> changedSettings, // <String, String> expected

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

@@ -14,15 +14,26 @@
 package com.google.devtools.build.lib.rules.test;
 
 import com.google.devtools.build.lib.analysis.RunEnvironmentInfo;
+import com.google.devtools.build.lib.analysis.starlark.StarlarkRuleClassFunctions;
+import com.google.devtools.build.lib.analysis.starlark.StarlarkRuleClassFunctions.StarlarkRuleFunction;
 import com.google.devtools.build.lib.analysis.test.ExecutionInfo;
+import com.google.devtools.build.lib.events.EventKind;
+import com.google.devtools.build.lib.events.StoredEventHandler;
+import com.google.devtools.build.lib.packages.PackageFactory.PackageContext;
 import com.google.devtools.build.lib.starlarkbuildapi.test.TestingModuleApi;
+import java.util.regex.Pattern;
 import net.starlark.java.eval.Dict;
 import net.starlark.java.eval.EvalException;
 import net.starlark.java.eval.Sequence;
+import net.starlark.java.eval.Starlark;
+import net.starlark.java.eval.StarlarkFunction;
 import net.starlark.java.eval.StarlarkList;
+import net.starlark.java.eval.StarlarkThread;
+import net.starlark.java.eval.Tuple;
 
 /** A class that exposes testing infrastructure to Starlark. */
 public class StarlarkTestingModule implements TestingModuleApi {
+  private static final Pattern RULE_NAME_PATTERN = Pattern.compile("[A-Za-z_][A-Za-z0-9_]*");
 
   @Override
   public ExecutionInfo executionInfo(Dict<?, ?> requirements /* <String, String> */)
@@ -41,4 +52,78 @@ public RunEnvironmentInfo testEnvironment(
             Sequence.cast(inheritedEnvironment, String.class, "inherited_environment")),
         /* shouldErrorOnNonExecutableRule */ false);
   }
+
+  @Override
+  public void analysisTest(
+      String name,
+      StarlarkFunction implementation,
+      Object attrs,
+      Sequence<?> fragments,
+      Sequence<?> toolchains,
+      Object attrValuesApi,
+      StarlarkThread thread)
+      throws EvalException, InterruptedException {
+    if (!RULE_NAME_PATTERN.matcher(name).matches()) {
+      throw Starlark.errorf("'name' is limited to Starlark identifiers, got %s", name);
+    }
+    Dict<String, Object> attrValues =
+        Dict.cast(attrValuesApi, String.class, Object.class, "attr_values");
+    if (attrValues.containsKey("name")) {
+      throw Starlark.errorf("'name' cannot be set or overridden in 'attr_values'");
+    }
+
+    StarlarkRuleFunction starlarkRuleFunction =
+        StarlarkRuleClassFunctions.createRule(
+            implementation,
+            /*test=*/ true,
+            attrs,
+            /*implicitOutputs=*/ Starlark.NONE,
+            /*executable=*/ false,
+            /*outputToGenfiles=*/ false,
+            /*fragments=*/ fragments,
+            /*hostFragments=*/ StarlarkList.empty(),
+            /*starlarkTestable=*/ false,
+            /*toolchains=*/ toolchains,
+            /*useToolchainTransition=*/ false,
+            /*providesArg=*/ StarlarkList.empty(),
+            /*execCompatibleWith=*/ StarlarkList.empty(),
+            /*analysisTest=*/ Boolean.TRUE,
+            /*buildSetting=*/ Starlark.NONE,
+            /*cfg=*/ Starlark.NONE,
+            /*execGroups=*/ Starlark.NONE,
+            /*compileOneFiletype=*/ Starlark.NONE,
+            /*name=*/ Starlark.NONE,
+            thread);
+
+    // Export the rule
+    // 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.
+    PackageContext pkgContext = thread.getThreadLocal(PackageContext.class);
+    StoredEventHandler handler = new StoredEventHandler();
+    starlarkRuleFunction.export(
+        handler, pkgContext.getLabel(), name + "_test"); // export in BUILD thread
+    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());
+    }
+
+    // Instantiate the target
+    Dict.Builder<String, Object> args = Dict.builder();
+    args.put("name", name);
+    args.putAll(attrValues);
+    starlarkRuleFunction.call(thread, Tuple.of(), args.buildImmutable());
+  }
 }

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

@@ -397,77 +397,6 @@ StarlarkCallable rule(
       StarlarkThread thread)
       throws EvalException;
 
-  @StarlarkMethod(
-      name = "analysis_test",
-      doc =
-          "Creates a new analysis test target. <p>The number of transitive dependencies of the test"
-              + " are limited. The limit is controlled by"
-              + " <code>--analysis_testing_deps_limit</code> flag.",
-      parameters = {
-        @Param(
-            name = "name",
-            named = true,
-            doc =
-                "Name of the target. It should be a Starlark identifier, matching pattern"
-                    + " '[A-Za-z_][A-Za-z0-9_]*'."),
-        @Param(
-            name = "implementation",
-            named = true,
-            doc =
-                "The Starlark function implementing this analysis test. It must have exactly one"
-                    + " parameter: <a href=\"ctx.html\">ctx</a>. The function is called during the"
-                    + " analysis phase. It can access the attributes declared by <code>attrs</code>"
-                    + " and populated via <code>attr_values</code>. The implementation function may"
-                    + " not register actions. Instead, it must register a pass/fail result"
-                    + " via providing <a"
-                    + " href='AnalysisTestResultInfo.html'>AnalysisTestResultInfo</a>."),
-        @Param(
-            name = "attrs",
-            allowedTypes = {
-              @ParamType(type = Dict.class),
-              @ParamType(type = NoneType.class),
-            },
-            named = true,
-            defaultValue = "None",
-            doc =
-                "Dictionary declaring the attributes. See the <a href=\"rule.html\">rule</a> call."
-                    + "Attributes are allowed to use configuration transitions defined using <a "
-                    + " href=\"#analysis_test_transition\">analysis_test_transition</a>."),
-        @Param(
-            name = "fragments",
-            allowedTypes = {@ParamType(type = Sequence.class, generic1 = String.class)},
-            named = true,
-            defaultValue = "[]",
-            doc =
-                "List of configuration fragments that are available to the implementation of the"
-                    + " analysis test."),
-        @Param(
-            name = TOOLCHAINS_PARAM,
-            allowedTypes = {@ParamType(type = Sequence.class, generic1 = Object.class)},
-            named = true,
-            defaultValue = "[]",
-            doc =
-                "The set of toolchains the test requires. See the <a href=\"#rule\">rule</a>"
-                    + " call."),
-        @Param(
-            name = "attr_values",
-            allowedTypes = {@ParamType(type = Dict.class, generic1 = String.class)},
-            named = true,
-            defaultValue = "{}",
-            doc = "Dictionary of attribute values to pass to the implementation."),
-      },
-      useStarlarkThread = true,
-      enableOnlyWithFlag = BuildLanguageOptions.EXPERIMENTAL_ANALYSIS_TEST_CALL)
-  void analysisTest(
-      String name,
-      StarlarkFunction implementation,
-      Object attrs,
-      Sequence<?> fragments,
-      Sequence<?> toolchains,
-      Object argsValue,
-      StarlarkThread thread)
-      throws EvalException, InterruptedException;
-
   @StarlarkMethod(
       name = "aspect",
       doc =

src/main/java/com/google/devtools/build/lib/starlarkbuildapi/test/BUILD

@@ -25,6 +25,7 @@ java_library(
         # TODO(b/80307387): Remove dependency on Label implementation.
         "//src/main/java/com/google/devtools/build/lib/cmdline",
         "//src/main/java/com/google/devtools/build/lib/collect/nestedset",
+        "//src/main/java/com/google/devtools/build/lib/packages/semantics",
         "//src/main/java/com/google/devtools/build/lib/starlarkbuildapi/core",
         "//src/main/java/com/google/devtools/build/lib/starlarkbuildapi/platform",
         "//src/main/java/net/starlark/java/annot",