Skip to content

Commit

Permalink
Enable having aspect parameters of type boolean
Browse files Browse the repository at this point in the history 
This CL enables using `attr.bool` as a type of aspects parameters.

PiperOrigin-RevId: 417803129
  • Loading branch information
@mai93 @copybara-github
mai93 authored and copybara-github committed Dec 22, 2021
1 parent 72d5255 commit 30fd508
Show file tree
Hide file tree
Showing 5 changed files with 507 additions and 38 deletions.
15 changes: 8 additions & 7 deletions ...main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkRuleClassFunctions.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -639,9 +639,12 @@ public StarlarkAspect aspect(
} 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()) {
Expand Down Expand Up @@ -799,14 +802,12 @@ private static void validateRulePropagatedAspects(RuleClass ruleClass) throws Ev
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);
}
}
Expand Down
3 changes: 2 additions & 1 deletion src/main/java/com/google/devtools/build/lib/packages/AspectDefinition.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -460,7 +460,8 @@ public Builder add(Attribute attribute) {
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(),
Expand Down
79 changes: 56 additions & 23 deletions src/main/java/com/google/devtools/build/lib/packages/StarlarkDefinedAspect.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -51,6 +52,12 @@ public final class StarlarkDefinedAspect implements StarlarkExportable, Starlark

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,
Expand Down Expand Up @@ -147,7 +154,8 @@ public AspectDefinition getDefinition(AspectParameters aspectParams) {
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).",
Expand Down Expand Up @@ -182,11 +190,15 @@ public AspectDefinition getDefinition(AspectParameters aspectParams) {

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);
}
Expand All @@ -197,7 +209,7 @@ private static Attribute addAttrValue(Attribute attr, String attrValue) {
// 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());
Expand Down Expand Up @@ -232,8 +244,11 @@ public Function<Rule, AspectParameters> getDefaultParametersExtractor() {
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);
Expand Down Expand Up @@ -264,9 +279,11 @@ public AspectParameters extractTopLevelParameters(ImmutableMap<String, String> p
}

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);
Expand All @@ -275,36 +292,52 @@ public AspectParameters extractTopLevelParameters(ImmutableMap<String, String> p
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;
}
Expand Down
Loading

0 comments on commit 30fd508

Please sign in to comment.