diff --git a/src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkRuleClassFunctions.java b/src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkRuleClassFunctions.java index 96b7ef0ced38e7..1c90805a6ac4c9 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkRuleClassFunctions.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkRuleClassFunctions.java @@ -100,6 +100,7 @@ import com.google.errorprone.annotations.FormatMethod; import java.util.Collection; import java.util.Map; +import java.util.regex.Pattern; import javax.annotation.Nullable; import net.starlark.java.eval.Debug; import net.starlark.java.eval.Dict; @@ -111,6 +112,7 @@ 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; @@ -138,6 +140,7 @@ 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. */ @@ -282,7 +285,7 @@ public Provider provider(String doc, Object fields, StarlarkThread thread) throw // TODO(bazel-team): implement attribute copy and other rule properties @Override - public StarlarkCallable rule( + public StarlarkRuleFunction rule( StarlarkFunction implementation, Boolean test, Object attrs, @@ -507,6 +510,81 @@ public StarlarkCallable 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 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 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. diff --git a/src/main/java/com/google/devtools/build/lib/packages/semantics/BuildLanguageOptions.java b/src/main/java/com/google/devtools/build/lib/packages/semantics/BuildLanguageOptions.java index dba3e3827b4fdc..a0f111d679d6e7 100644 --- a/src/main/java/com/google/devtools/build/lib/packages/semantics/BuildLanguageOptions.java +++ b/src/main/java/com/google/devtools/build/lib/packages/semantics/BuildLanguageOptions.java @@ -297,6 +297,18 @@ public class BuildLanguageOptions extends OptionsBase implements Serializable { + " and 1 cpu.") public boolean experimentalActionResourceSet; + + @Option( + name = "experimental_analysis_test_call", + defaultValue = "true", + documentationCategory = OptionDocumentationCategory.STARLARK_SEMANTICS, + effectTags = {OptionEffectTag.LOADING_AND_ANALYSIS, OptionEffectTag.BUILD_FILE_SEMANTICS}, + metadataTags = { + OptionMetadataTag.EXPERIMENTAL, + }, + help = "If set to true, analysis_test native call is available.") + public boolean experimentalAnalysisTestCall; + @Option( name = "incompatible_struct_has_no_methods", defaultValue = "false", @@ -586,6 +598,7 @@ public StarlarkSemantics toStarlarkSemantics() { .setBool( INCOMPATIBLE_EXISTING_RULES_IMMUTABLE_VIEW, incompatibleExistingRulesImmutableView) .setBool(EXPERIMENTAL_ACTION_RESOURCE_SET, experimentalActionResourceSet) + .setBool(EXPERIMENTAL_ANALYSIS_TEST_CALL, experimentalAnalysisTestCall) .setBool(EXPERIMENTAL_GOOGLE_LEGACY_API, experimentalGoogleLegacyApi) .setBool(EXPERIMENTAL_NINJA_ACTIONS, experimentalNinjaActions) .setBool(EXPERIMENTAL_PLATFORMS_API, experimentalPlatformsApi) @@ -662,6 +675,7 @@ public StarlarkSemantics toStarlarkSemantics() { public static final String EXPERIMENTAL_SIBLING_REPOSITORY_LAYOUT = "-experimental_sibling_repository_layout"; public static final String EXPERIMENTAL_ACTION_RESOURCE_SET = "+experimental_action_resource_set"; + public static final String EXPERIMENTAL_ANALYSIS_TEST_CALL = "+experimental_analysis_test_call"; public static final String INCOMPATIBLE_ALLOW_TAGS_PROPAGATION = "-incompatible_allow_tags_propagation"; public static final String INCOMPATIBLE_ALWAYS_CHECK_DEPSET_ELEMENTS = diff --git a/src/main/java/com/google/devtools/build/lib/starlarkbuildapi/StarlarkRuleFunctionsApi.java b/src/main/java/com/google/devtools/build/lib/starlarkbuildapi/StarlarkRuleFunctionsApi.java index 7365caa4ac62a8..facd72ce53865d 100644 --- a/src/main/java/com/google/devtools/build/lib/starlarkbuildapi/StarlarkRuleFunctionsApi.java +++ b/src/main/java/com/google/devtools/build/lib/starlarkbuildapi/StarlarkRuleFunctionsApi.java @@ -359,18 +359,18 @@ public interface StarlarkRuleFunctionsApi { positional = false, allowedTypes = {@ParamType(type = String.class), @ParamType(type = NoneType.class)}, doc = - "The name of this rule, as understood by Bazel and reported in contexts such as" - + " logging, native.existing_rule(...)[kind], and bazel" - + " query. Usually this is the same as the Starlark identifier that gets" - + " bound to this rule; for instance a rule called foo_library" - + " would typically be declared as foo_library = rule(...) and" - + " instantiated in a BUILD file as foo_library(...).

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, foo_library = rule(...) need not specify this" - + " parameter if the name is foo_library.

Specifying an explicit" - + " name for a rule does not change where you are allowed to instantiate the" - + " rule."), + "Deprecated: do not use.

The name of this rule, as understood by Bazel and" + + " reported in contexts such as logging," + + " native.existing_rule(...)[kind], and bazel query." + + " Usually this is the same as the Starlark identifier that gets bound to this" + + " rule; for instance a rule called foo_library would typically" + + " be declared as foo_library = rule(...) and instantiated in a" + + " BUILD file as foo_library(...).

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," + + " foo_library = rule(...) need not specify this parameter if the" + + " name is foo_library.

Specifying an explicit name for a rule" + + " does not change where you are allowed to instantiate the rule."), }, useStarlarkThread = true) StarlarkCallable rule( @@ -397,6 +397,77 @@ StarlarkCallable rule( StarlarkThread thread) throws EvalException; + @StarlarkMethod( + name = "analysis_test", + doc = + "Creates a new analysis test target.

The number of transitive dependencies of the test" + + " are limited. The limit is controlled by" + + " --analysis_testing_deps_limit 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: ctx. The function is called during the" + + " analysis phase. It can access the attributes declared by attrs" + + " and populated via attr_values. The implementation function may" + + " not register actions. Instead, it must register a pass/fail result" + + " via providing AnalysisTestResultInfo."), + @Param( + name = "attrs", + allowedTypes = { + @ParamType(type = Dict.class), + @ParamType(type = NoneType.class), + }, + named = true, + defaultValue = "None", + doc = + "Dictionary declaring the attributes. See the rule call." + + "Attributes are allowed to use configuration transitions defined using analysis_test_transition."), + @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 rule" + + " 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 = diff --git a/src/main/java/com/google/devtools/build/skydoc/fakebuildapi/FakeStarlarkRuleFunctionsApi.java b/src/main/java/com/google/devtools/build/skydoc/fakebuildapi/FakeStarlarkRuleFunctionsApi.java index ac4413a667f9eb..8c7d4f71e547d0 100644 --- a/src/main/java/com/google/devtools/build/skydoc/fakebuildapi/FakeStarlarkRuleFunctionsApi.java +++ b/src/main/java/com/google/devtools/build/skydoc/fakebuildapi/FakeStarlarkRuleFunctionsApi.java @@ -166,6 +166,17 @@ public StarlarkCallable rule( return functionIdentifier; } + @Override + public void analysisTest( + String name, + StarlarkFunction implementation, + Object attrs, + Sequence fragments, + Sequence toolchains, + Object argsValue, + StarlarkThread thread) + throws EvalException, InterruptedException {} + @Override public Label label(String labelString, StarlarkThread thread) throws EvalException { try { diff --git a/src/test/java/com/google/devtools/build/lib/starlark/StarlarkRuleClassFunctionsTest.java b/src/test/java/com/google/devtools/build/lib/starlark/StarlarkRuleClassFunctionsTest.java index 482ebb9039b971..636ce2b230a251 100644 --- a/src/test/java/com/google/devtools/build/lib/starlark/StarlarkRuleClassFunctionsTest.java +++ b/src/test/java/com/google/devtools/build/lib/starlark/StarlarkRuleClassFunctionsTest.java @@ -2391,4 +2391,190 @@ public void testAttrWithAspectRequiringAspects_requiredNativeAspect_getsParamsFr .getDefaultValueUnchecked()) .isEqualTo("v1"); } + + @Test + public void testAnalysisTest() throws Exception { + scratch.file( + "p/b.bzl", + "def impl(ctx): ", + " return [AnalysisTestResultInfo(", + " success = True,", + " message = ''", + " )]", + "def my_test_macro(name):", + " analysis_test(name = name, implementation = impl)"); + scratch.file( + "p/BUILD", // + "load(':b.bzl','my_test_macro')", + "my_test_macro(name = 'my_test_target')"); + + getConfiguredTarget("//p:my_test_target"); + + assertNoEvents(); + } + + @Test + public void testAnalysisTestAttrs() throws Exception { + scratch.file( + "p/b.bzl", + "def impl(ctx): ", + " ctx.attr.target_under_test", + " return [AnalysisTestResultInfo(", + " success = True,", + " message = ''", + " )]", + "def my_test_macro(name):", + " native.filegroup(name = 'my_subject', srcs = [])", + " analysis_test(name = name,", + " implementation = impl,", + " attrs = {'target_under_test': attr.label_list()},", + " attr_values = {'target_under_test': [':my_subject']},", + " )"); + scratch.file( + "p/BUILD", // + "load(':b.bzl','my_test_macro')", + "my_test_macro(name = 'my_test_target')"); + + getConfiguredTarget("//p:my_test_target"); + + assertNoEvents(); + } + + /** Tests two analysis_test calls with same name. */ + @Test + public void testAnalysisTestDuplicateName() throws Exception { + scratch.file( + "p/a.bzl", + "def impl(ctx): ", + " return [AnalysisTestResultInfo(", + " success = True,", + " message = ''", + " )]", + "def my_test_macro1(name):", + " analysis_test(name = name, implementation = impl)"); + scratch.file( + "p/b.bzl", + "def impl(ctx): ", + " return [AnalysisTestResultInfo(", + " success = True,", + " message = ''", + " )]", + "def my_test_macro2(name):", + " analysis_test(name = name, implementation = impl)"); + scratch.file( + "p/BUILD", // + "load(':a.bzl','my_test_macro1')", + "load(':b.bzl','my_test_macro2')", + "my_test_macro1(name = 'my_test_target')", + "my_test_macro2(name = 'my_test_target')"); + + reporter.removeHandler(failFastHandler); + reporter.addHandler(ev.getEventCollector()); + getConfiguredTarget("//p:my_test_target"); + + ev.assertContainsError( + "Error in analysis_test: my_test_target_test rule 'my_test_target' in package 'p' conflicts" + + " with existing my_test_target_test rule"); + } + + /** + * Tests analysis_test call with a name that is not Starlark identifier (but still a good target + * name). + */ + @Test + public void testAnalysisTestBadName() throws Exception { + scratch.file( + "p/b.bzl", + "def impl(ctx): ", + " return [AnalysisTestResultInfo(", + " success = True,", + " message = ''", + " )]", + "def my_test_macro(name):", + " analysis_test(name = name, implementation = impl)"); + scratch.file( + "p/BUILD", // + "load(':b.bzl','my_test_macro')", + "my_test_macro(name = 'my+test+target')"); + + reporter.removeHandler(failFastHandler); + reporter.addHandler(ev.getEventCollector()); + getConfiguredTarget("//p:my+test+target"); + + ev.assertContainsError( + "Error in analysis_test: 'name' is limited to Starlark identifiers, got my+test+target"); + } + + @Test + public void testAnalysisTestBadArgs() throws Exception { + scratch.file( + "p/b.bzl", + "def impl(ctx): ", + " return [AnalysisTestResultInfo(", + " success = True,", + " message = ''", + " )]", + "def my_test_macro(name):", + " analysis_test(name = name, implementation = impl, attr_values = {'notthere': []})"); + scratch.file( + "p/BUILD", // + "load(':b.bzl','my_test_macro')", + "my_test_macro(name = 'my_test_target')"); + + reporter.removeHandler(failFastHandler); + reporter.addHandler(ev.getEventCollector()); + getConfiguredTarget("//p:my_test_target"); + + ev.assertContainsError("no such attribute 'notthere' in 'my_test_target_test' rule"); + } + + @Test + public void testAnalysisTestErrorOnExport() throws Exception { + scratch.file( + "p/b.bzl", + "def impl(ctx): ", + " return [AnalysisTestResultInfo(", + " success = True,", + " message = ''", + " )]", + "def my_test_macro(name):", + " analysis_test(name = name, implementation = impl, attrs = {'name': attr.string()})"); + scratch.file( + "p/BUILD", // + "load(':b.bzl','my_test_macro')", + "my_test_macro(name = 'my_test_target')"); + + reporter.removeHandler(failFastHandler); + reporter.addHandler(ev.getEventCollector()); + getConfiguredTarget("//p:my_test_target"); + + ev.assertContainsError( + "Error in analysis_test: Errors in exporting my_test_target: \n" + + "cannot add attribute: There is already a built-in attribute 'name' which cannot be" + + " overridden."); + } + + @Test + public void testAnalysisTestErrorOverridingName() throws Exception { + scratch.file( + "p/b.bzl", + "def impl(ctx): ", + " return [AnalysisTestResultInfo(", + " success = True,", + " message = ''", + " )]", + "def my_test_macro(name):", + " analysis_test(name = name, implementation = impl, attr_values = {'name': 'override'})"); + scratch.file( + "p/BUILD", // + "load(':b.bzl','my_test_macro')", + "my_test_macro(name = 'my_test_target')"); + + reporter.removeHandler(failFastHandler); + reporter.addHandler(ev.getEventCollector()); + getConfiguredTarget("//p:override"); + + ev.assertContainsError( + "Error in analysis_test: 'name' cannot be set or overridden in 'attr_values'"); + } }