Enable having aspect parameters of type `boolean`
This CL enables using `attr.bool` as a type of aspects parameters.
PiperOrigin-RevId: 417803129
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 5fbb8b2..3eb5c85 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
@@ -639,9 +639,12 @@
} else if (attribute.getType() == Type.INTEGER) {
// isValueSet() is always true for attr.int as default value is 0 by default.
hasDefault = !Objects.equals(attribute.getDefaultValue(null), StarlarkInt.of(0));
+ } else if (attribute.getType() == Type.BOOLEAN) {
+ hasDefault = !Objects.equals(attribute.getDefaultValue(null), false);
} else {
throw Starlark.errorf(
- "Aspect parameter attribute '%s' must have type 'int' or 'string'.", nativeName);
+ "Aspect parameter attribute '%s' must have type 'bool', 'int' or 'string'.",
+ nativeName);
}
if (hasDefault && attribute.checkAllowedValues()) {
@@ -799,14 +802,12 @@
String aspectAttrName = aspectAttribute.getPublicName();
Type<?> aspectAttrType = aspectAttribute.getType();
- // When propagated from a rule, explicit aspect attributes must be of type int or string
- // and they must have `values` restriction.
+ // When propagated from a rule, explicit aspect attributes must be of type boolean, int
+ // or string. Integer and string attributes must have the `values` restriction.
if (!aspectAttribute.isImplicit() && !aspectAttribute.isLateBound()) {
- if ((aspectAttrType != Type.STRING && aspectAttrType != Type.INTEGER)
- || !aspectAttribute.checkAllowedValues()) {
+ if (aspectAttrType != Type.BOOLEAN && !aspectAttribute.checkAllowedValues()) {
throw Starlark.errorf(
- "Aspect %s: Aspect parameter attribute '%s' must have type 'int' or 'string'"
- + " and use the 'values' restriction.",
+ "Aspect %s: Aspect parameter attribute '%s' must use the 'values' restriction.",
aspect.getName(), aspectAttrName);
}
}
diff --git a/src/main/java/com/google/devtools/build/lib/packages/AspectDefinition.java b/src/main/java/com/google/devtools/build/lib/packages/AspectDefinition.java
index 0f8772c..556b498 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/AspectDefinition.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/AspectDefinition.java
@@ -460,7 +460,8 @@
attribute.isImplicit()
|| attribute.isLateBound()
|| (attribute.getType() == Type.STRING && attribute.checkAllowedValues())
- || (attribute.getType() == Type.INTEGER && attribute.checkAllowedValues()),
+ || (attribute.getType() == Type.INTEGER && attribute.checkAllowedValues())
+ || attribute.getType() == Type.BOOLEAN,
"%s: Invalid attribute '%s' (%s)",
aspectClass.getName(),
attribute.getName(),
diff --git a/src/main/java/com/google/devtools/build/lib/packages/StarlarkDefinedAspect.java b/src/main/java/com/google/devtools/build/lib/packages/StarlarkDefinedAspect.java
index 6a66447..14f3e3c 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/StarlarkDefinedAspect.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/StarlarkDefinedAspect.java
@@ -14,6 +14,7 @@
package com.google.devtools.build.lib.packages;
+import com.google.common.base.Ascii;
import com.google.common.base.Function;
import com.google.common.base.Preconditions;
import com.google.common.collect.ImmutableList;
@@ -51,6 +52,12 @@
private StarlarkAspectClass aspectClass;
+ private static final ImmutableSet<String> TRUE_REPS =
+ ImmutableSet.of("true", "1", "yes", "t", "y");
+
+ private static final ImmutableSet<String> FALSE_REPS =
+ ImmutableSet.of("false", "0", "no", "f", "n");
+
public StarlarkDefinedAspect(
StarlarkCallable implementation,
ImmutableList<String> attributeAspects,
@@ -147,7 +154,8 @@
String attrName = attr.getName();
String attrValue = aspectParams.getOnlyValueOfAttribute(attrName);
Preconditions.checkState(!Attribute.isImplicit(attrName));
- Preconditions.checkState(attrType == Type.STRING || attrType == Type.INTEGER);
+ Preconditions.checkState(
+ attrType == Type.STRING || attrType == Type.INTEGER || attrType == Type.BOOLEAN);
Preconditions.checkArgument(
aspectParams.getAttribute(attrName).size() == 1,
"Aspect %s parameter %s has %s values (must have exactly 1).",
@@ -182,11 +190,15 @@
private static Attribute addAttrValue(Attribute attr, String attrValue) {
Attribute.Builder<?> attrBuilder;
+ Type<?> attrType = attr.getType();
Object castedValue = attrValue;
- if (attr.getType() == Type.INTEGER) {
+ if (attrType == Type.INTEGER) {
castedValue = StarlarkInt.parse(attrValue, /*base=*/ 0);
attrBuilder = attr.cloneBuilder(Type.INTEGER).value((StarlarkInt) castedValue);
+ } else if (attrType == Type.BOOLEAN) {
+ castedValue = Boolean.parseBoolean(attrValue);
+ attrBuilder = attr.cloneBuilder(Type.BOOLEAN).value((Boolean) castedValue);
} else {
attrBuilder = attr.cloneBuilder(Type.STRING).value((String) castedValue);
}
@@ -197,7 +209,7 @@
// values in all aspects string attributes for both native and starlark aspects.
// Therefore, allowedValues list is added here with only the current value of the attribute.
return attrBuilder
- .allowedValues(new Attribute.AllowedValueSet(attr.getType().cast(castedValue)))
+ .allowedValues(new Attribute.AllowedValueSet(attrType.cast(castedValue)))
.build(attr.getName());
} else {
return attrBuilder.build(attr.getName());
@@ -232,8 +244,11 @@
rule.getTargetKind(),
param);
Preconditions.checkArgument(
- ruleAttr.getType() == Type.STRING || ruleAttr.getType() == Type.INTEGER,
- "Cannot apply aspect %s to %s since attribute '%s' is not string and not integer.",
+ ruleAttr.getType() == Type.STRING
+ || ruleAttr.getType() == Type.INTEGER
+ || ruleAttr.getType() == Type.BOOLEAN,
+ "Cannot apply aspect %s to %s since attribute '%s' is not boolean, integer, nor"
+ + " string.",
getName(),
rule.getTargetKind(),
param);
@@ -264,9 +279,11 @@
}
Preconditions.checkArgument(
- parameterType == Type.STRING || parameterType == Type.INTEGER,
- "Aspect %s: Cannot pass value of attribute '%s' of type %s, only 'int' and 'string'"
- + " attributes are allowed.",
+ parameterType == Type.STRING
+ || parameterType == Type.INTEGER
+ || parameterType == Type.BOOLEAN,
+ "Aspect %s: Cannot pass value of attribute '%s' of type %s, only 'boolean', 'int' and"
+ + " 'string' attributes are allowed.",
getName(),
parameterName,
parameterType);
@@ -275,36 +292,52 @@
parametersValues.getOrDefault(
parameterName, parameterType.cast(aspectParameter.getDefaultValue(null)).toString());
- // Validate integer values for integer attributes
Object castedParameterValue = parameterValue;
+ // Validate integer and boolean parameters values
if (parameterType == Type.INTEGER) {
- try {
- castedParameterValue = StarlarkInt.parse(parameterValue, /*base=*/ 0);
- } catch (NumberFormatException e) {
- throw new EvalException(
- String.format(
- "%s: expected value of type 'int' for attribute '%s' but got '%s'",
- getName(), parameterName, parameterValue),
- e);
- }
+ castedParameterValue = parseIntParameter(parameterName, parameterValue);
+ } else if (parameterType == Type.BOOLEAN) {
+ castedParameterValue = parseBooleanParameter(parameterName, parameterValue);
}
if (aspectParameter.checkAllowedValues()) {
PredicateWithMessage<Object> allowedValues = aspectParameter.getAllowedValues();
- if (parameterType == Type.INTEGER) {
- castedParameterValue = StarlarkInt.parse(parameterValue, /*base=*/ 0);
- }
if (!allowedValues.apply(castedParameterValue)) {
throw Starlark.errorf(
"%s: invalid value in '%s' attribute: %s",
- getName(), parameterName, allowedValues.getErrorReason(parameterValue));
+ getName(), parameterName, allowedValues.getErrorReason(castedParameterValue));
}
}
- builder.addAttribute(parameterName, parameterValue);
+ builder.addAttribute(parameterName, castedParameterValue.toString());
}
return builder.build();
}
+ private StarlarkInt parseIntParameter(String name, String value) throws EvalException {
+ try {
+ return StarlarkInt.parse(value, /*base=*/ 0);
+ } catch (NumberFormatException e) {
+ throw new EvalException(
+ String.format(
+ "%s: expected value of type 'int' for attribute '%s' but got '%s'",
+ getName(), name, value),
+ e);
+ }
+ }
+
+ private Boolean parseBooleanParameter(String name, String value) throws EvalException {
+ value = Ascii.toLowerCase(value);
+ if (TRUE_REPS.contains(value)) {
+ return true;
+ }
+ if (FALSE_REPS.contains(value)) {
+ return false;
+ }
+ throw Starlark.errorf(
+ "%s: expected value of type 'bool' for attribute '%s' but got '%s'",
+ getName(), name, value);
+ }
+
public ImmutableList<Label> getRequiredToolchains() {
return requiredToolchains;
}
diff --git a/src/test/java/com/google/devtools/build/lib/starlark/StarlarkDefinedAspectsTest.java b/src/test/java/com/google/devtools/build/lib/starlark/StarlarkDefinedAspectsTest.java
index 8800035..0ff29ab 100644
--- a/src/test/java/com/google/devtools/build/lib/starlark/StarlarkDefinedAspectsTest.java
+++ b/src/test/java/com/google/devtools/build/lib/starlark/StarlarkDefinedAspectsTest.java
@@ -7299,8 +7299,8 @@
assertThrows(TargetParsingException.class, () -> update("//test:main_target"));
}
assertContainsEvent(
- "Aspect //test:defs.bzl%my_aspect: Aspect parameter attribute 'param' must have type"
- + " 'int' or 'string' and use the 'values' restriction.");
+ "Aspect //test:defs.bzl%my_aspect: Aspect parameter attribute 'param' must use the 'values'"
+ + " restriction.");
}
/**
@@ -7334,8 +7334,8 @@
assertThrows(TargetParsingException.class, () -> update("//test:main_target"));
}
assertContainsEvent(
- "Aspect //test:defs.bzl%required_aspect: Aspect parameter attribute 'p1' must have type"
- + " 'int' or 'string' and use the 'values' restriction.");
+ "Aspect //test:defs.bzl%required_aspect: Aspect parameter attribute 'p1' must use the"
+ + " 'values' restriction.");
}
@Test
@@ -7502,8 +7502,8 @@
}
assertContainsEvent(
- "Aspect //test:aspect.bzl%MyAspectUncovered: Aspect parameter attribute 'my_attr' must have"
- + " type 'int' or 'string' and use the 'values' restriction.");
+ "Aspect //test:aspect.bzl%MyAspectUncovered: Aspect parameter attribute 'my_attr' must use"
+ + " the 'values' restriction.");
}
@Test
@@ -7841,6 +7841,440 @@
}
@Test
+ public void booleanAspectParameter_mandatoryAttrNotCoveredByRule() throws Exception {
+ scratch.file(
+ "test/aspect.bzl",
+ "def _impl(target, ctx):",
+ " return struct()",
+ "def _rule_impl(ctx):",
+ " return struct()",
+ "MyAspectUncovered = aspect(",
+ " implementation=_impl,",
+ " attrs = { 'my_attr' : attr.bool(default = True, mandatory = True) },",
+ ")",
+ "my_rule = rule(",
+ " implementation=_rule_impl,",
+ " attrs = { 'deps' : attr.label_list(aspects=[MyAspectUncovered]) },",
+ ")");
+ scratch.file("test/BUILD", "load('//test:aspect.bzl', 'my_rule')", "my_rule(name ='main')");
+ reporter.removeHandler(failFastHandler);
+
+ // This call succeeds if "--keep_going" was passed, which it does in the WithKeepGoing test
+ // suite. Otherwise, it fails and throws a TargetParsingException.
+ if (keepGoing()) {
+ AnalysisResult analysisResult = update(ImmutableList.of(), "//test:main");
+ assertThat(analysisResult.hasError()).isTrue();
+ } else {
+ assertThrows(TargetParsingException.class, () -> update(ImmutableList.of(), "//test:main"));
+ }
+
+ assertContainsEvent(
+ "Aspect //test:aspect.bzl%MyAspectUncovered requires rule my_rule to specify attribute "
+ + "'my_attr' with type boolean.");
+ }
+
+ @Test
+ public void booleanAspectParameter_mandatoryAttrWithWrongTypeInRule() throws Exception {
+ scratch.file(
+ "test/aspect.bzl",
+ "def _impl(target, ctx):",
+ " return struct()",
+ "def _rule_impl(ctx):",
+ " return struct()",
+ "MyAspectUncovered = aspect(",
+ " implementation=_impl,",
+ " attrs = { 'my_attr' : attr.bool(default = True, mandatory = True) },",
+ ")",
+ "my_rule = rule(",
+ " implementation=_rule_impl,",
+ " attrs = { 'deps' : attr.label_list(aspects=[MyAspectUncovered]),",
+ " 'my_attr': attr.string() },",
+ ")");
+ scratch.file("test/BUILD", "load('//test:aspect.bzl', 'my_rule')", "my_rule(name ='main')");
+ reporter.removeHandler(failFastHandler);
+
+ // This call succeeds if "--keep_going" was passed, which it does in the WithKeepGoing test
+ // suite. Otherwise, it fails and throws a TargetParsingException.
+ if (keepGoing()) {
+ AnalysisResult analysisResult = update(ImmutableList.of(), "//test:main");
+ assertThat(analysisResult.hasError()).isTrue();
+ } else {
+ assertThrows(TargetParsingException.class, () -> update(ImmutableList.of(), "//test:main"));
+ }
+
+ assertContainsEvent(
+ "Aspect //test:aspect.bzl%MyAspectUncovered requires rule my_rule to specify attribute "
+ + "'my_attr' with type boolean.");
+ }
+
+ @Test
+ public void booleanAspectParameter_attrWithoutDefaultNotCoveredByRule() throws Exception {
+ scratch.file(
+ "test/aspect.bzl",
+ "def _impl(target, ctx):",
+ " return struct()",
+ "def _rule_impl(ctx):",
+ " return struct()",
+ "MyAspectUncovered = aspect(",
+ " implementation=_impl,",
+ " attrs = { 'my_attr' : attr.bool() },",
+ ")",
+ "my_rule = rule(",
+ " implementation=_rule_impl,",
+ " attrs = { 'deps' : attr.label_list(aspects=[MyAspectUncovered]) },",
+ ")");
+ scratch.file("test/BUILD", "load('//test:aspect.bzl', 'my_rule')", "my_rule(name ='main')");
+ reporter.removeHandler(failFastHandler);
+
+ // This call succeeds if "--keep_going" was passed, which it does in the WithKeepGoing test
+ // suite. Otherwise, it fails and throws a TargetParsingException.
+ if (keepGoing()) {
+ AnalysisResult analysisResult = update(ImmutableList.of(), "//test:main");
+ assertThat(analysisResult.hasError()).isTrue();
+ } else {
+ assertThrows(TargetParsingException.class, () -> update(ImmutableList.of(), "//test:main"));
+ }
+
+ assertContainsEvent(
+ "Aspect //test:aspect.bzl%MyAspectUncovered requires rule my_rule to specify attribute "
+ + "'my_attr' with type boolean.");
+ }
+
+ @Test
+ public void booleanAspectParameter_attrWithoutDefaultWrongTypeInRule() throws Exception {
+ scratch.file(
+ "test/aspect.bzl",
+ "def _impl(target, ctx):",
+ " return struct()",
+ "def _rule_impl(ctx):",
+ " return struct()",
+ "MyAspectUncovered = aspect(",
+ " implementation=_impl,",
+ " attrs = { 'my_attr' : attr.bool() },",
+ ")",
+ "my_rule = rule(",
+ " implementation=_rule_impl,",
+ " attrs = { 'deps' : attr.label_list(aspects=[MyAspectUncovered]),",
+ " 'my_attr': attr.string() },",
+ ")");
+ scratch.file("test/BUILD", "load('//test:aspect.bzl', 'my_rule')", "my_rule(name ='main')");
+ reporter.removeHandler(failFastHandler);
+
+ // This call succeeds if "--keep_going" was passed, which it does in the WithKeepGoing test
+ // suite. Otherwise, it fails and throws a TargetParsingException.
+ if (keepGoing()) {
+ AnalysisResult analysisResult = update(ImmutableList.of(), "//test:main");
+ assertThat(analysisResult.hasError()).isTrue();
+ } else {
+ assertThrows(TargetParsingException.class, () -> update(ImmutableList.of(), "//test:main"));
+ }
+
+ assertContainsEvent(
+ "Aspect //test:aspect.bzl%MyAspectUncovered requires rule my_rule to specify attribute "
+ + "'my_attr' with type boolean.");
+ }
+
+ @Test
+ public void aspectBooleanParameter_withDefaultValue() throws Exception {
+ scratch.file(
+ "test/aspect.bzl",
+ "def _aspect_impl(target, ctx):",
+ " result = ['my_aspect on target {}, my_attr = {}'.",
+ " format(target.label, ctx.attr.my_attr)]",
+ " if ctx.rule.attr.dep:",
+ " result += ctx.rule.attr.dep.my_aspect_result",
+ " return struct(my_aspect_result = result)",
+ "",
+ "def _rule_impl(ctx):",
+ " if ctx.attr.dep:",
+ " return struct(my_rule_result = ctx.attr.dep.my_aspect_result)",
+ " pass",
+ "",
+ "MyAspect = aspect(",
+ " implementation = _aspect_impl,",
+ " attrs = { 'my_attr' : attr.bool(default = True) },",
+ ")",
+ "",
+ "my_rule = rule(",
+ " implementation=_rule_impl,",
+ " attrs = { 'dep' : attr.label(aspects=[MyAspect]) }",
+ ")");
+ scratch.file(
+ "test/BUILD",
+ "load('//test:aspect.bzl', 'my_rule')",
+ "my_rule(name = 'main_target',",
+ " dep = ':dep_target',",
+ ")",
+ "my_rule(name = 'dep_target')");
+
+ AnalysisResult analysisResult = update(ImmutableList.of(), "//test:main_target");
+
+ ConfiguredTarget configuredTarget =
+ Iterables.getOnlyElement(analysisResult.getTargetsToBuild());
+ StarlarkList<?> ruleResult = (StarlarkList) configuredTarget.get("my_rule_result");
+ assertThat(Starlark.toIterable(ruleResult))
+ .containsExactly("my_aspect on target //test:dep_target, my_attr = True");
+ }
+
+ @Test
+ public void aspectBooleanParameter_valueOverwrittenByRuleDefault() throws Exception {
+ scratch.file(
+ "test/aspect.bzl",
+ "def _aspect_impl(target, ctx):",
+ " result = ['my_aspect on target {}, my_attr = {}'.",
+ " format(target.label, ctx.attr.my_attr)]",
+ " if ctx.rule.attr.dep:",
+ " result += ctx.rule.attr.dep.my_aspect_result",
+ " return struct(my_aspect_result = result)",
+ "",
+ "def _rule_impl(ctx):",
+ " if ctx.attr.dep:",
+ " return struct(my_rule_result = ctx.attr.dep.my_aspect_result)",
+ " pass",
+ "",
+ "MyAspect = aspect(",
+ " implementation = _aspect_impl,",
+ " attrs = { 'my_attr' : attr.bool(default = True) },",
+ ")",
+ "",
+ "my_rule = rule(",
+ " implementation=_rule_impl,",
+ " attrs = { 'dep' : attr.label(aspects=[MyAspect]),",
+ " 'my_attr': attr.bool(default = False) }",
+ ")");
+ scratch.file(
+ "test/BUILD",
+ "load('//test:aspect.bzl', 'my_rule')",
+ "my_rule(name = 'main_target',",
+ " dep = ':dep_target',",
+ ")",
+ "my_rule(name = 'dep_target')");
+
+ AnalysisResult analysisResult = update(ImmutableList.of(), "//test:main_target");
+
+ ConfiguredTarget configuredTarget =
+ Iterables.getOnlyElement(analysisResult.getTargetsToBuild());
+ StarlarkList<?> ruleResult = (StarlarkList) configuredTarget.get("my_rule_result");
+ assertThat(Starlark.toIterable(ruleResult))
+ .containsExactly("my_aspect on target //test:dep_target, my_attr = False");
+ }
+
+ @Test
+ public void aspectBooleanParameter_valueOverwrittenByTargetValue() throws Exception {
+ scratch.file(
+ "test/aspect.bzl",
+ "def _aspect_impl(target, ctx):",
+ " result = ['my_aspect on target {}, my_attr = {}'.",
+ " format(target.label, ctx.attr.my_attr)]",
+ " if ctx.rule.attr.dep:",
+ " result += ctx.rule.attr.dep.my_aspect_result",
+ " return struct(my_aspect_result = result)",
+ "",
+ "def _rule_impl(ctx):",
+ " if ctx.attr.dep:",
+ " return struct(my_rule_result = ctx.attr.dep.my_aspect_result)",
+ " pass",
+ "",
+ "MyAspect = aspect(",
+ " implementation = _aspect_impl,",
+ " attrs = { 'my_attr' : attr.bool(default = True) },",
+ ")",
+ "",
+ "my_rule = rule(",
+ " implementation=_rule_impl,",
+ " attrs = { 'dep' : attr.label(aspects=[MyAspect]),",
+ " 'my_attr': attr.bool(default = True) }",
+ ")");
+ scratch.file(
+ "test/BUILD",
+ "load('//test:aspect.bzl', 'my_rule')",
+ "my_rule(name = 'main_target',",
+ " dep = ':dep_target',",
+ " my_attr = False,",
+ ")",
+ "my_rule(name = 'dep_target')");
+
+ AnalysisResult analysisResult = update(ImmutableList.of(), "//test:main_target");
+
+ ConfiguredTarget configuredTarget =
+ Iterables.getOnlyElement(analysisResult.getTargetsToBuild());
+ StarlarkList<?> ruleResult = (StarlarkList) configuredTarget.get("my_rule_result");
+ assertThat(Starlark.toIterable(ruleResult))
+ .containsExactly("my_aspect on target //test:dep_target, my_attr = False");
+ }
+
+ @Test
+ public void testTopLevelAspectsWithBooleanParameter() throws Exception {
+ scratch.file(
+ "test/defs.bzl",
+ "def _aspect_a_impl(target, ctx):",
+ " result = ['aspect_a on target {}, p = {}'.",
+ " format(target.label, ctx.attr.p)]",
+ " if ctx.rule.attr.dep:",
+ " result += ctx.rule.attr.dep.aspect_a_result",
+ " return struct(aspect_a_result = result)",
+ "",
+ "aspect_a = aspect(",
+ " implementation = _aspect_a_impl,",
+ " attr_aspects = ['dep'],",
+ " attrs = { 'p' : attr.bool() },",
+ ")",
+ "",
+ "def _my_rule_impl(ctx):",
+ " pass",
+ "my_rule = rule(",
+ " implementation = _my_rule_impl,",
+ " attrs = { 'dep' : attr.label() },",
+ ")");
+ scratch.file(
+ "test/BUILD",
+ "load('//test:defs.bzl', 'my_rule')",
+ "my_rule(",
+ " name = 'main_target',",
+ " dep = ':dep_target_1',",
+ ")",
+ "my_rule(",
+ " name = 'dep_target_1',",
+ " dep = ':dep_target_2',",
+ ")",
+ "my_rule(",
+ " name = 'dep_target_2',",
+ ")");
+ useConfiguration("--experimental_allow_top_level_aspects_parameters");
+
+ AnalysisResult analysisResult =
+ update(
+ ImmutableList.of("//test:defs.bzl%aspect_a"),
+ ImmutableMap.of("p", "y"),
+ "//test:main_target");
+
+ ImmutableMap<AspectKey, ConfiguredAspect> configuredAspects = analysisResult.getAspectsMap();
+ ConfiguredAspect aspectA = getConfiguredAspect(configuredAspects, "aspect_a");
+ assertThat(aspectA).isNotNull();
+ StarlarkList<?> aspectAResult = (StarlarkList) aspectA.get("aspect_a_result");
+ assertThat(Starlark.toIterable(aspectAResult))
+ .containsExactly(
+ "aspect_a on target //test:main_target, p = True",
+ "aspect_a on target //test:dep_target_1, p = True",
+ "aspect_a on target //test:dep_target_2, p = True");
+ }
+
+ @Test
+ public void testTopLevelAspectsWithBooleanParameter_useDefaultValue() throws Exception {
+ scratch.file(
+ "test/defs.bzl",
+ "def _aspect_a_impl(target, ctx):",
+ " result = ['aspect_a on target {}, p = {}'.",
+ " format(target.label, ctx.attr.p)]",
+ " if ctx.rule.attr.dep:",
+ " result += ctx.rule.attr.dep.aspect_a_result",
+ " return struct(aspect_a_result = result)",
+ "",
+ "aspect_a = aspect(",
+ " implementation = _aspect_a_impl,",
+ " attr_aspects = ['dep'],",
+ " attrs = { 'p' : attr.bool(default = False) },",
+ ")",
+ "",
+ "def _my_rule_impl(ctx):",
+ " pass",
+ "my_rule = rule(",
+ " implementation = _my_rule_impl,",
+ " attrs = { 'dep' : attr.label() },",
+ ")");
+ scratch.file(
+ "test/BUILD",
+ "load('//test:defs.bzl', 'my_rule')",
+ "my_rule(",
+ " name = 'main_target',",
+ " dep = ':dep_target_1',",
+ ")",
+ "my_rule(",
+ " name = 'dep_target_1',",
+ " dep = ':dep_target_2',",
+ ")",
+ "my_rule(",
+ " name = 'dep_target_2',",
+ ")");
+ useConfiguration("--experimental_allow_top_level_aspects_parameters");
+
+ AnalysisResult analysisResult =
+ update(ImmutableList.of("//test:defs.bzl%aspect_a"), "//test:main_target");
+
+ ImmutableMap<AspectKey, ConfiguredAspect> configuredAspects = analysisResult.getAspectsMap();
+ ConfiguredAspect aspectA = getConfiguredAspect(configuredAspects, "aspect_a");
+ assertThat(aspectA).isNotNull();
+ StarlarkList<?> aspectAResult = (StarlarkList) aspectA.get("aspect_a_result");
+ assertThat(Starlark.toIterable(aspectAResult))
+ .containsExactly(
+ "aspect_a on target //test:main_target, p = False",
+ "aspect_a on target //test:dep_target_1, p = False",
+ "aspect_a on target //test:dep_target_2, p = False");
+ }
+
+ @Test
+ public void testTopLevelAspectsWithBooleanParameter_invalidValue() throws Exception {
+ scratch.file(
+ "test/defs.bzl",
+ "def _aspect_a_impl(target, ctx):",
+ " result = ['aspect_a on target {}, p = {}'.",
+ " format(target.label, ctx.attr.p)]",
+ " if ctx.rule.attr.dep:",
+ " result += ctx.rule.attr.dep.aspect_a_result",
+ " return struct(aspect_a_result = result)",
+ "",
+ "aspect_a = aspect(",
+ " implementation = _aspect_a_impl,",
+ " attr_aspects = ['dep'],",
+ " attrs = { 'p' : attr.bool() },",
+ ")",
+ "",
+ "def _my_rule_impl(ctx):",
+ " pass",
+ "my_rule = rule(",
+ " implementation = _my_rule_impl,",
+ " attrs = { 'dep' : attr.label() },",
+ ")");
+ scratch.file(
+ "test/BUILD",
+ "load('//test:defs.bzl', 'my_rule')",
+ "my_rule(",
+ " name = 'main_target',",
+ " dep = ':dep_target_1',",
+ ")",
+ "my_rule(",
+ " name = 'dep_target_1',",
+ " dep = ':dep_target_2',",
+ ")",
+ "my_rule(",
+ " name = 'dep_target_2',",
+ ")");
+ useConfiguration("--experimental_allow_top_level_aspects_parameters");
+ reporter.removeHandler(failFastHandler);
+
+ // This call succeeds if "--keep_going" was passed, which it does in the WithKeepGoing test
+ // suite. Otherwise, it fails and throws a ViewCreationFailedException.
+ if (keepGoing()) {
+ AnalysisResult analysisResult =
+ update(
+ ImmutableList.of("//test:defs.bzl%aspect_a"),
+ ImmutableMap.of("p", "x"),
+ "//test:main_target");
+ assertThat(analysisResult.hasError()).isTrue();
+ } else {
+ assertThrows(
+ ViewCreationFailedException.class,
+ () ->
+ update(
+ ImmutableList.of("//test:defs.bzl%aspect_a"),
+ ImmutableMap.of("p", "x"),
+ "//test:main_target"));
+ }
+ assertContainsEvent(
+ "//test:defs.bzl%aspect_a: expected value of type 'bool' for attribute 'p' but got 'x'");
+ }
+
+ @Test
public void testRuleAspectWithMandatoryParameterNotProvided() throws Exception {
scratch.file(
"test/defs.bzl",
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 3a6120d..5336000 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
@@ -615,7 +615,7 @@
@Test
public void testAspectParameterBadType() throws Exception {
ev.checkEvalErrorContains(
- "Aspect parameter attribute 'param' must have type 'int' or 'string'.",
+ "Aspect parameter attribute 'param' must have type 'bool', 'int' or 'string'.",
"def _impl(target, ctx):",
" pass",
"my_aspect = aspect(_impl,",
