Skip to content

Commit 30fd508

Browse files
mai93copybara-github
mai93
authored and
copybara-github
committed
Enable having aspect parameters of type boolean
This CL enables using `attr.bool` as a type of aspects parameters. PiperOrigin-RevId: 417803129
1 parent 72d5255 commit 30fd508

5 files changed

Lines changed: 507 additions & 38 deletions

File tree

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

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -639,9 +639,12 @@ public StarlarkAspect aspect(
639639
} else if (attribute.getType() == Type.INTEGER) {
640640
// isValueSet() is always true for attr.int as default value is 0 by default.
641641
hasDefault = !Objects.equals(attribute.getDefaultValue(null), StarlarkInt.of(0));
642+
} else if (attribute.getType() == Type.BOOLEAN) {
643+
hasDefault = !Objects.equals(attribute.getDefaultValue(null), false);
642644
} else {
643645
throw Starlark.errorf(
644-
"Aspect parameter attribute '%s' must have type 'int' or 'string'.", nativeName);
646+
"Aspect parameter attribute '%s' must have type 'bool', 'int' or 'string'.",
647+
nativeName);
645648
}
646649

647650
if (hasDefault && attribute.checkAllowedValues()) {
@@ -799,14 +802,12 @@ private static void validateRulePropagatedAspects(RuleClass ruleClass) throws Ev
799802
String aspectAttrName = aspectAttribute.getPublicName();
800803
Type<?> aspectAttrType = aspectAttribute.getType();
801804

802-
// When propagated from a rule, explicit aspect attributes must be of type int or string
803-
// and they must have `values` restriction.
805+
// When propagated from a rule, explicit aspect attributes must be of type boolean, int
806+
// or string. Integer and string attributes must have the `values` restriction.
804807
if (!aspectAttribute.isImplicit() && !aspectAttribute.isLateBound()) {
805-
if ((aspectAttrType != Type.STRING && aspectAttrType != Type.INTEGER)
806-
|| !aspectAttribute.checkAllowedValues()) {
808+
if (aspectAttrType != Type.BOOLEAN && !aspectAttribute.checkAllowedValues()) {
807809
throw Starlark.errorf(
808-
"Aspect %s: Aspect parameter attribute '%s' must have type 'int' or 'string'"
809-
+ " and use the 'values' restriction.",
810+
"Aspect %s: Aspect parameter attribute '%s' must use the 'values' restriction.",
810811
aspect.getName(), aspectAttrName);
811812
}
812813
}

‎src/main/java/com/google/devtools/build/lib/packages/AspectDefinition.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -460,7 +460,8 @@ public Builder add(Attribute attribute) {
460460
attribute.isImplicit()
461461
|| attribute.isLateBound()
462462
|| (attribute.getType() == Type.STRING && attribute.checkAllowedValues())
463-
|| (attribute.getType() == Type.INTEGER && attribute.checkAllowedValues()),
463+
|| (attribute.getType() == Type.INTEGER && attribute.checkAllowedValues())
464+
|| attribute.getType() == Type.BOOLEAN,
464465
"%s: Invalid attribute '%s' (%s)",
465466
aspectClass.getName(),
466467
attribute.getName(),

‎src/main/java/com/google/devtools/build/lib/packages/StarlarkDefinedAspect.java‎

Lines changed: 56 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414

1515
package com.google.devtools.build.lib.packages;
1616

17+
import com.google.common.base.Ascii;
1718
import com.google.common.base.Function;
1819
import com.google.common.base.Preconditions;
1920
import com.google.common.collect.ImmutableList;
@@ -51,6 +52,12 @@ public final class StarlarkDefinedAspect implements StarlarkExportable, Starlark
5152

5253
private StarlarkAspectClass aspectClass;
5354

55+
private static final ImmutableSet<String> TRUE_REPS =
56+
ImmutableSet.of("true", "1", "yes", "t", "y");
57+
58+
private static final ImmutableSet<String> FALSE_REPS =
59+
ImmutableSet.of("false", "0", "no", "f", "n");
60+
5461
public StarlarkDefinedAspect(
5562
StarlarkCallable implementation,
5663
ImmutableList<String> attributeAspects,
@@ -147,7 +154,8 @@ public AspectDefinition getDefinition(AspectParameters aspectParams) {
147154
String attrName = attr.getName();
148155
String attrValue = aspectParams.getOnlyValueOfAttribute(attrName);
149156
Preconditions.checkState(!Attribute.isImplicit(attrName));
150-
Preconditions.checkState(attrType == Type.STRING || attrType == Type.INTEGER);
157+
Preconditions.checkState(
158+
attrType == Type.STRING || attrType == Type.INTEGER || attrType == Type.BOOLEAN);
151159
Preconditions.checkArgument(
152160
aspectParams.getAttribute(attrName).size() == 1,
153161
"Aspect %s parameter %s has %s values (must have exactly 1).",
@@ -182,11 +190,15 @@ public AspectDefinition getDefinition(AspectParameters aspectParams) {
182190

183191
private static Attribute addAttrValue(Attribute attr, String attrValue) {
184192
Attribute.Builder<?> attrBuilder;
193+
Type<?> attrType = attr.getType();
185194
Object castedValue = attrValue;
186195

187-
if (attr.getType() == Type.INTEGER) {
196+
if (attrType == Type.INTEGER) {
188197
castedValue = StarlarkInt.parse(attrValue, /*base=*/ 0);
189198
attrBuilder = attr.cloneBuilder(Type.INTEGER).value((StarlarkInt) castedValue);
199+
} else if (attrType == Type.BOOLEAN) {
200+
castedValue = Boolean.parseBoolean(attrValue);
201+
attrBuilder = attr.cloneBuilder(Type.BOOLEAN).value((Boolean) castedValue);
190202
} else {
191203
attrBuilder = attr.cloneBuilder(Type.STRING).value((String) castedValue);
192204
}
@@ -197,7 +209,7 @@ private static Attribute addAttrValue(Attribute attr, String attrValue) {
197209
// values in all aspects string attributes for both native and starlark aspects.
198210
// Therefore, allowedValues list is added here with only the current value of the attribute.
199211
return attrBuilder
200-
.allowedValues(new Attribute.AllowedValueSet(attr.getType().cast(castedValue)))
212+
.allowedValues(new Attribute.AllowedValueSet(attrType.cast(castedValue)))
201213
.build(attr.getName());
202214
} else {
203215
return attrBuilder.build(attr.getName());
@@ -232,8 +244,11 @@ public Function<Rule, AspectParameters> getDefaultParametersExtractor() {
232244
rule.getTargetKind(),
233245
param);
234246
Preconditions.checkArgument(
235-
ruleAttr.getType() == Type.STRING || ruleAttr.getType() == Type.INTEGER,
236-
"Cannot apply aspect %s to %s since attribute '%s' is not string and not integer.",
247+
ruleAttr.getType() == Type.STRING
248+
|| ruleAttr.getType() == Type.INTEGER
249+
|| ruleAttr.getType() == Type.BOOLEAN,
250+
"Cannot apply aspect %s to %s since attribute '%s' is not boolean, integer, nor"
251+
+ " string.",
237252
getName(),
238253
rule.getTargetKind(),
239254
param);
@@ -264,9 +279,11 @@ public AspectParameters extractTopLevelParameters(ImmutableMap<String, String> p
264279
}
265280

266281
Preconditions.checkArgument(
267-
parameterType == Type.STRING || parameterType == Type.INTEGER,
268-
"Aspect %s: Cannot pass value of attribute '%s' of type %s, only 'int' and 'string'"
269-
+ " attributes are allowed.",
282+
parameterType == Type.STRING
283+
|| parameterType == Type.INTEGER
284+
|| parameterType == Type.BOOLEAN,
285+
"Aspect %s: Cannot pass value of attribute '%s' of type %s, only 'boolean', 'int' and"
286+
+ " 'string' attributes are allowed.",
270287
getName(),
271288
parameterName,
272289
parameterType);
@@ -275,36 +292,52 @@ public AspectParameters extractTopLevelParameters(ImmutableMap<String, String> p
275292
parametersValues.getOrDefault(
276293
parameterName, parameterType.cast(aspectParameter.getDefaultValue(null)).toString());
277294

278-
// Validate integer values for integer attributes
279295
Object castedParameterValue = parameterValue;
296+
// Validate integer and boolean parameters values
280297
if (parameterType == Type.INTEGER) {
281-
try {
282-
castedParameterValue = StarlarkInt.parse(parameterValue, /*base=*/ 0);
283-
} catch (NumberFormatException e) {
284-
throw new EvalException(
285-
String.format(
286-
"%s: expected value of type 'int' for attribute '%s' but got '%s'",
287-
getName(), parameterName, parameterValue),
288-
e);
289-
}
298+
castedParameterValue = parseIntParameter(parameterName, parameterValue);
299+
} else if (parameterType == Type.BOOLEAN) {
300+
castedParameterValue = parseBooleanParameter(parameterName, parameterValue);
290301
}
291302

292303
if (aspectParameter.checkAllowedValues()) {
293304
PredicateWithMessage<Object> allowedValues = aspectParameter.getAllowedValues();
294-
if (parameterType == Type.INTEGER) {
295-
castedParameterValue = StarlarkInt.parse(parameterValue, /*base=*/ 0);
296-
}
297305
if (!allowedValues.apply(castedParameterValue)) {
298306
throw Starlark.errorf(
299307
"%s: invalid value in '%s' attribute: %s",
300-
getName(), parameterName, allowedValues.getErrorReason(parameterValue));
308+
getName(), parameterName, allowedValues.getErrorReason(castedParameterValue));
301309
}
302310
}
303-
builder.addAttribute(parameterName, parameterValue);
311+
builder.addAttribute(parameterName, castedParameterValue.toString());
304312
}
305313
return builder.build();
306314
}
307315

316+
private StarlarkInt parseIntParameter(String name, String value) throws EvalException {
317+
try {
318+
return StarlarkInt.parse(value, /*base=*/ 0);
319+
} catch (NumberFormatException e) {
320+
throw new EvalException(
321+
String.format(
322+
"%s: expected value of type 'int' for attribute '%s' but got '%s'",
323+
getName(), name, value),
324+
e);
325+
}
326+
}
327+
328+
private Boolean parseBooleanParameter(String name, String value) throws EvalException {
329+
value = Ascii.toLowerCase(value);
330+
if (TRUE_REPS.contains(value)) {
331+
return true;
332+
}
333+
if (FALSE_REPS.contains(value)) {
334+
return false;
335+
}
336+
throw Starlark.errorf(
337+
"%s: expected value of type 'bool' for attribute '%s' but got '%s'",
338+
getName(), name, value);
339+
}
340+
308341
public ImmutableList<Label> getRequiredToolchains() {
309342
return requiredToolchains;
310343
}

0 commit comments

Comments
 (0)