Prohibit duplicate addition of aspect to an attribute and improve diagnostics.
--
MOS_MIGRATED_REVID=127808009
diff --git a/src/main/java/com/google/devtools/build/lib/packages/Attribute.java b/src/main/java/com/google/devtools/build/lib/packages/Attribute.java
index c605964..0655f9f 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/Attribute.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/Attribute.java
@@ -25,6 +25,7 @@
import com.google.common.collect.Sets;
import com.google.devtools.build.lib.analysis.TransitiveInfoProvider;
import com.google.devtools.build.lib.cmdline.Label;
+import com.google.devtools.build.lib.events.Location;
import com.google.devtools.build.lib.skylarkinterface.SkylarkModule;
import com.google.devtools.build.lib.skylarkinterface.SkylarkModuleCategory;
import com.google.devtools.build.lib.syntax.ClassObject;
@@ -37,16 +38,14 @@
import com.google.devtools.build.lib.util.FileTypeSet;
import com.google.devtools.build.lib.util.Preconditions;
import com.google.devtools.build.lib.util.StringUtil;
-
import java.util.Arrays;
import java.util.Collection;
import java.util.EnumSet;
import java.util.HashMap;
-import java.util.LinkedHashSet;
+import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;
import java.util.Set;
-
import javax.annotation.concurrent.Immutable;
/**
@@ -415,7 +414,6 @@
* already undocumented based on its name cannot be marked as undocumented.
*/
public static class Builder <TYPE> {
-
private String name;
private final Type<TYPE> type;
private Transition configTransition = ConfigurationTransition.NONE;
@@ -435,7 +433,7 @@
ImmutableList.<ImmutableSet<String>>of();
private ImmutableList<Class<? extends TransitiveInfoProvider>> mandatoryNativeProviders =
ImmutableList.of();
- private Set<RuleAspect> aspects = new LinkedHashSet<>();
+ private HashMap<String, RuleAspect<?>> aspects = new LinkedHashMap<>();
/**
* Creates an attribute builder with given name and type. This attribute is optional, uses
@@ -890,7 +888,12 @@
*/
public Builder<TYPE> aspect(
NativeAspectClass aspect, Function<Rule, AspectParameters> evaluator) {
- this.aspects.add(new NativeRuleAspect(aspect, evaluator));
+ NativeRuleAspect nativeRuleAspect = new NativeRuleAspect(aspect, evaluator);
+ RuleAspect<?> oldAspect = this.aspects.put(nativeRuleAspect.getName(), nativeRuleAspect);
+ if (oldAspect != null) {
+ throw new AssertionError(
+ String.format("Aspect %s has already been added", oldAspect.getName()));
+ }
return this;
}
@@ -909,13 +912,25 @@
return this.aspect(aspect, noParameters);
}
- public Builder<TYPE> aspect(SkylarkAspect skylarkAspect) {
- this.aspects.add(new SkylarkRuleAspect(skylarkAspect));
+ public Builder<TYPE> aspect(
+ SkylarkAspect skylarkAspect, Location location) throws EvalException {
+ SkylarkRuleAspect skylarkRuleAspect = new SkylarkRuleAspect(skylarkAspect);
+ RuleAspect<?> oldAspect = this.aspects.put(skylarkAspect.getName(), skylarkRuleAspect);
+ if (oldAspect != null) {
+ throw new EvalException(location,
+ String.format("Aspect %s added more than once", skylarkAspect.getName()));
+ }
return this;
}
public Builder<TYPE> aspect(final Aspect aspect) {
- this.aspects.add(new PredefinedRuleAspect(aspect));
+ PredefinedRuleAspect predefinedRuleAspect = new PredefinedRuleAspect(aspect);
+ RuleAspect<?> oldAspect =
+ this.aspects.put(predefinedRuleAspect.getName(), predefinedRuleAspect);
+ if (oldAspect != null) {
+ throw new AssertionError(
+ String.format("Aspect %s has already been added", oldAspect.getName()));
+ }
return this;
}
@@ -1002,7 +1017,7 @@
allowedValues,
mandatoryProvidersList,
mandatoryNativeProviders,
- ImmutableSet.copyOf(aspects));
+ ImmutableList.copyOf(aspects.values()));
}
}
@@ -1289,7 +1304,7 @@
private final ImmutableList<Class<? extends TransitiveInfoProvider>> mandatoryNativeProviders;
- private final ImmutableSet<RuleAspect> aspects;
+ private final ImmutableList<RuleAspect<?>> aspects;
/**
* Constructs a rule attribute with the specified name, type and default
@@ -1322,7 +1337,7 @@
PredicateWithMessage<Object> allowedValues,
ImmutableList<ImmutableSet<String>> mandatoryProvidersList,
ImmutableList<Class<? extends TransitiveInfoProvider>> mandatoryNativeProviders,
- ImmutableSet<RuleAspect> aspects) {
+ ImmutableList<RuleAspect<?>> aspects) {
Preconditions.checkNotNull(configTransition);
Preconditions.checkArgument(
(configTransition == ConfigurationTransition.NONE && configurator == null)
@@ -1695,7 +1710,10 @@
builder.value = defaultValue;
builder.valueSet = false;
builder.allowedValues = allowedValues;
- builder.aspects = new LinkedHashSet<>(aspects);
+ builder.aspects = new LinkedHashMap<>();
+ for (RuleAspect<?> aspect : aspects) {
+ builder.aspects.put(aspect.getName(), aspect);
+ }
return builder;
}