Fix crash that happens when an aspect has the same implicit attribute as the rule that defines it.
RELNOTES: None.
PiperOrigin-RevId: 318456188
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 dba23a8..ffb9794 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
@@ -172,11 +172,12 @@
String value = aspectParams.getOnlyValueOfAttribute(attr.getName());
Preconditions.checkState(!Attribute.isImplicit(attr.getName()));
Preconditions.checkState(attr.getType() == Type.STRING);
- Preconditions.checkArgument(aspectParams.getAttribute(attr.getName()).size() == 1,
- String.format("Aspect %s parameter %s has %d values (must have exactly 1).",
- getName(),
- attr.getName(),
- aspectParams.getAttribute(attr.getName()).size()));
+ Preconditions.checkArgument(
+ aspectParams.getAttribute(attr.getName()).size() == 1,
+ "Aspect %s parameter %s has %s values (must have exactly 1).",
+ getName(),
+ attr.getName(),
+ aspectParams.getAttribute(attr.getName()).size());
attr = attr.cloneBuilder(Type.STRING).value(value).build(attr.getName());
}
builder.add(attr);
@@ -206,29 +207,35 @@
AttributeMap ruleAttrs = RawAttributeMapper.of(rule);
AspectParameters.Builder builder = new AspectParameters.Builder();
for (Attribute aspectAttr : attributes) {
- if (!Attribute.isImplicit(aspectAttr.getName())) {
- String param = aspectAttr.getName();
- Attribute ruleAttr = ruleAttrs.getAttributeDefinition(param);
- if (paramAttributes.contains(aspectAttr.getName())) {
- // These are preconditions because if they are false, RuleFunction.call() should
- // already have generated an error.
- Preconditions.checkArgument(
- ruleAttr != null,
- String.format(
- "Cannot apply aspect %s to %s that does not define attribute '%s'.",
- getName(), rule.getTargetKind(), param));
- Preconditions.checkArgument(
- ruleAttr.getType() == Type.STRING,
- String.format(
- "Cannot apply aspect %s to %s with non-string attribute '%s'.",
- getName(), rule.getTargetKind(), param));
- }
- if (ruleAttr != null && ruleAttr.getType() == aspectAttr.getType()) {
- // If the attribute has a select() (which aspect attributes don't yet support), the
- // error gets reported in RuleClass.checkAspectAllowedValues.
- if (!ruleAttrs.isConfigurable(param)) {
- builder.addAttribute(param, (String) ruleAttrs.get(param, ruleAttr.getType()));
- }
+ String param = aspectAttr.getName();
+ if (Attribute.isImplicit(param) || Attribute.isLateBound(param)) {
+ // These attributes are the private matters of the aspect
+ continue;
+ }
+
+ Attribute ruleAttr = ruleAttrs.getAttributeDefinition(param);
+ if (paramAttributes.contains(aspectAttr.getName())) {
+ // These are preconditions because if they are false, RuleFunction.call() should
+ // already have generated an error.
+ Preconditions.checkArgument(
+ ruleAttr != null,
+ "Cannot apply aspect %s to %s that does not define attribute '%s'.",
+ getName(),
+ rule.getTargetKind(),
+ param);
+ Preconditions.checkArgument(
+ ruleAttr.getType() == Type.STRING,
+ "Cannot apply aspect %s to %s with non-string attribute '%s'.",
+ getName(),
+ rule.getTargetKind(),
+ param);
+ }
+
+ if (ruleAttr != null && ruleAttr.getType() == aspectAttr.getType()) {
+ // If the attribute has a select() (which aspect attributes don't yet support), the
+ // error gets reported in RuleClass.checkAspectAllowedValues.
+ if (!ruleAttrs.isConfigurable(param)) {
+ builder.addAttribute(param, (String) ruleAttrs.get(param, ruleAttr.getType()));
}
}
}
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/AspectTest.java b/src/test/java/com/google/devtools/build/lib/analysis/AspectTest.java
index 8760a00..6c68ae7 100644
--- a/src/test/java/com/google/devtools/build/lib/analysis/AspectTest.java
+++ b/src/test/java/com/google/devtools/build/lib/analysis/AspectTest.java
@@ -895,4 +895,30 @@
aspect.getProvider(AspectApplyingToFiles.Provider.class);
assertThat(provider.getLabel()).isEqualTo(Label.parseAbsoluteUnchecked("//a:x_deploy.jar"));
}
+
+ @Test
+ public void sameConfiguredAttributeOnAspectAndRule() throws Exception {
+ scratch.file(
+ "a/a.bzl",
+ "def _a_impl(t, ctx):",
+ " return [DefaultInfo()]",
+ "def _r_impl(ctx):",
+ " return [DefaultInfo()]",
+ "a = aspect(",
+ " implementation = _a_impl,",
+ " attrs = {'_f': attr.label(",
+ " default = configuration_field(",
+ " fragment = 'cpp', name = 'cc_toolchain'))})",
+ "r = rule(",
+ " implementation = _r_impl,",
+ " attrs = {'_f': attr.label(",
+ " default = configuration_field(",
+ " fragment = 'cpp', name = 'cc_toolchain')),",
+ " 'dep': attr.label(aspects=[a])})");
+
+ scratch.file("a/BUILD", "load(':a.bzl', 'r')", "r(name='r')");
+
+ setRulesAndAspectsAvailableInTests(ImmutableList.of(), ImmutableList.of());
+ getConfiguredTarget("//a:r");
+ }
}
