Make Predicate<String> obtainable from RuleClassNamePredicate
* Extend RuleClassNamePredicate to be both inclusive or exclusive
* Attribute uses RuleClassNamePredicate instead of Predicate<RuleClass>
PiperOrigin-RevId: 177656647
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 dcd7fb3..03f56dc 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
@@ -67,9 +67,9 @@
@Immutable
public final class Attribute implements Comparable<Attribute> {
- public static final Predicate<RuleClass> ANY_RULE = Predicates.alwaysTrue();
+ public static final RuleClassNamePredicate ANY_RULE = RuleClassNamePredicate.unspecified();
- public static final Predicate<RuleClass> NO_RULE = Predicates.alwaysFalse();
+ public static final RuleClassNamePredicate NO_RULE = RuleClassNamePredicate.only();
/**
* Wraps the information necessary to construct an Aspect.
@@ -406,8 +406,8 @@
private final String name;
private final Type<TYPE> type;
private Transition configTransition = ConfigurationTransition.NONE;
- private Predicate<RuleClass> allowedRuleClassesForLabels = Predicates.alwaysTrue();
- private Predicate<RuleClass> allowedRuleClassesForLabelsWarning = Predicates.alwaysFalse();
+ private RuleClassNamePredicate allowedRuleClassesForLabels = ANY_RULE;
+ private RuleClassNamePredicate allowedRuleClassesForLabelsWarning = NO_RULE;
private SplitTransitionProvider splitTransitionProvider;
private FileTypeSet allowedFileTypesForLabels;
private ValidityPredicate validityPredicate = ANY_EDGE;
@@ -769,7 +769,7 @@
*/
public Builder<TYPE> allowedRuleClasses(Iterable<String> allowedRuleClasses) {
return allowedRuleClasses(
- new RuleClassNamePredicate(allowedRuleClasses));
+ RuleClassNamePredicate.only(allowedRuleClasses));
}
/**
@@ -787,7 +787,7 @@
* <p>This only works on a per-target basis, not on a per-file basis; with other words, it
* works for 'deps' attributes, but not 'srcs' attributes.
*/
- public Builder<TYPE> allowedRuleClasses(Predicate<RuleClass> allowedRuleClasses) {
+ public Builder<TYPE> allowedRuleClasses(RuleClassNamePredicate allowedRuleClasses) {
Preconditions.checkState(type.getLabelClass() == LabelClass.DEPENDENCY,
"must be a label-valued type");
propertyFlags.add(PropertyFlag.STRICT_LABEL_CHECKING);
@@ -870,7 +870,7 @@
*/
public Builder<TYPE> allowedRuleClassesWithWarning(Collection<String> allowedRuleClasses) {
return allowedRuleClassesWithWarning(
- new RuleClassNamePredicate(allowedRuleClasses));
+ RuleClassNamePredicate.only(allowedRuleClasses));
}
/**
@@ -888,7 +888,7 @@
* <p>This only works on a per-target basis, not on a per-file basis; with other words, it
* works for 'deps' attributes, but not 'srcs' attributes.
*/
- public Builder<TYPE> allowedRuleClassesWithWarning(Predicate<RuleClass> allowedRuleClasses) {
+ public Builder<TYPE> allowedRuleClassesWithWarning(RuleClassNamePredicate allowedRuleClasses) {
Preconditions.checkState(type.getLabelClass() == LabelClass.DEPENDENCY,
"must be a label-valued type");
propertyFlags.add(PropertyFlag.STRICT_LABEL_CHECKING);
@@ -1102,14 +1102,12 @@
}
}
- if (allowedRuleClassesForLabels instanceof RuleClassNamePredicate
- && allowedRuleClassesForLabelsWarning instanceof RuleClassNamePredicate) {
- Preconditions.checkState(
- !((RuleClassNamePredicate) allowedRuleClassesForLabels)
- .intersects((RuleClassNamePredicate) allowedRuleClassesForLabelsWarning),
- "allowedRuleClasses and allowedRuleClassesWithWarning may not contain "
- + "the same rule classes");
- }
+ Preconditions.checkState(
+ !allowedRuleClassesForLabels.consideredOverlapping(allowedRuleClassesForLabelsWarning),
+ "allowedRuleClasses %s and allowedRuleClassesWithWarning %s "
+ + "may not contain the same rule classes",
+ allowedRuleClassesForLabels,
+ allowedRuleClassesForLabelsWarning);
return new Attribute(
name,
@@ -1791,13 +1789,13 @@
* For label or label-list attributes, this predicate returns which rule
* classes are allowed for the targets in the attribute.
*/
- private final Predicate<RuleClass> allowedRuleClassesForLabels;
+ private final RuleClassNamePredicate allowedRuleClassesForLabels;
/**
* For label or label-list attributes, this predicate returns which rule
* classes are allowed for the targets in the attribute with warning.
*/
- private final Predicate<RuleClass> allowedRuleClassesForLabelsWarning;
+ private final RuleClassNamePredicate allowedRuleClassesForLabelsWarning;
/**
* For label or label-list attributes, this predicate returns which file
@@ -1840,8 +1838,8 @@
Object defaultValue,
Transition configTransition,
SplitTransitionProvider splitTransitionProvider,
- Predicate<RuleClass> allowedRuleClassesForLabels,
- Predicate<RuleClass> allowedRuleClassesForLabelsWarning,
+ RuleClassNamePredicate allowedRuleClassesForLabels,
+ RuleClassNamePredicate allowedRuleClassesForLabelsWarning,
FileTypeSet allowedFileTypesForLabels,
ValidityPredicate validityPredicate,
Predicate<AttributeMap> condition,
@@ -2051,12 +2049,26 @@
}
/**
+ * Returns a predicate that evaluates to true for rule classes that are allowed labels in this
+ * attribute. If this is not a label or label-list attribute, the returned predicate always
+ * evaluates to true.
+ *
+ * <p>NOTE: This may return Predicates.<RuleClass>alwaysTrue() as a sentinel meaning "do the right
+ * thing", rather than actually allowing all rule classes in that attribute. Others parts of bazel
+ * code check for that specific instance.
+ */
+ public Predicate<RuleClass> getAllowedRuleClassesPredicate() {
+ return allowedRuleClassesForLabels.asPredicateOfRuleClass();
+ }
+
+ /**
* Returns a predicate that evaluates to true for rule classes that are
* allowed labels in this attribute. If this is not a label or label-list
* attribute, the returned predicate always evaluates to true.
*/
- public Predicate<RuleClass> getAllowedRuleClassesPredicate() {
- return allowedRuleClassesForLabels;
+ // TODO(b/69917891) Remove these methods once checkbuilddeps no longer depends on them.
+ public Predicate<String> getAllowedRuleClassNamesPredicate() {
+ return allowedRuleClassesForLabels.asPredicateOfRuleClassName();
}
/**
@@ -2065,7 +2077,7 @@
* attribute, the returned predicate always evaluates to true.
*/
public Predicate<RuleClass> getAllowedRuleClassesWarningPredicate() {
- return allowedRuleClassesForLabelsWarning;
+ return allowedRuleClassesForLabelsWarning.asPredicateOfRuleClass();
}
public RequiredProviders getRequiredProviders() {
diff --git a/src/main/java/com/google/devtools/build/lib/packages/RuleClass.java b/src/main/java/com/google/devtools/build/lib/packages/RuleClass.java
index 743625d..273745b 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/RuleClass.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/RuleClass.java
@@ -56,6 +56,7 @@
import com.google.devtools.build.lib.util.StringUtil;
import com.google.devtools.build.lib.vfs.PathFragment;
import java.util.ArrayList;
+import java.util.Arrays;
import java.util.BitSet;
import java.util.Collection;
import java.util.Collections;
@@ -380,47 +381,135 @@
public abstract void checkAttributes(Map<String, Attribute> attributes);
}
- /**
- * A predicate that filters rule classes based on their names.
- */
- public static class RuleClassNamePredicate implements Predicate<RuleClass> {
+ /** A predicate that filters rule classes based on their names. */
+ public static class RuleClassNamePredicate {
- private final Set<String> ruleClasses;
+ private final Predicate<String> ruleClassNamePredicate;
+ private final Predicate<RuleClass> ruleClassPredicate;
+ // if non-null, used ONLY for checking overlap
+ @Nullable private final Set<?> overlappable;
- public RuleClassNamePredicate(Iterable<String> ruleClasses) {
- this.ruleClasses = ImmutableSet.copyOf(ruleClasses);
+ private RuleClassNamePredicate(
+ Predicate<String> ruleClassNamePredicate, @Nullable Set<?> overlappable) {
+ this(
+ ruleClassNamePredicate,
+ new DescribedPredicate<>(
+ Predicates.compose(ruleClassNamePredicate, RuleClass::getName),
+ ruleClassNamePredicate.toString()),
+ overlappable);
}
- public RuleClassNamePredicate(String... ruleClasses) {
- this.ruleClasses = ImmutableSet.copyOf(ruleClasses);
+ private RuleClassNamePredicate(
+ Predicate<String> ruleClassNamePredicate,
+ Predicate<RuleClass> ruleClassPredicate,
+ @Nullable Set<?> overlappable) {
+ this.ruleClassNamePredicate = ruleClassNamePredicate;
+ this.ruleClassPredicate = ruleClassPredicate;
+ this.overlappable = overlappable;
}
- @Override
- public boolean apply(RuleClass ruleClass) {
- return ruleClasses.contains(ruleClass.getName());
+ public static RuleClassNamePredicate only(Iterable<String> ruleClassNamesAsIterable) {
+ ImmutableSet<String> ruleClassNames = ImmutableSet.copyOf(ruleClassNamesAsIterable);
+ return new RuleClassNamePredicate(
+ new DescribedPredicate<>(
+ Predicates.in(ruleClassNames), StringUtil.joinEnglishList(ruleClassNames)),
+ ruleClassNames);
+ }
+
+ public static RuleClassNamePredicate only(String... ruleClasses) {
+ return only(Arrays.asList(ruleClasses));
+ }
+
+ public static RuleClassNamePredicate allExcept(String... ruleClasses) {
+ ImmutableSet<String> ruleClassNames = ImmutableSet.copyOf(ruleClasses);
+ Preconditions.checkState(!ruleClassNames.isEmpty(), "Use unspecified() instead");
+ Predicate<String> containing = only(ruleClassNames).asPredicateOfRuleClassName();
+ return new RuleClassNamePredicate(
+ new DescribedPredicate<>(
+ Predicates.not(containing), "all but " + containing.toString()),
+ null);
+ }
+
+ /**
+ * This is a special sentinel value which represents a "default" {@link
+ * RuleClassNamePredicate} which is unspecified. Note that a call to its {@link
+ * RuleClassNamePredicate#asPredicateOfRuleClass} produces {@code
+ * Predicates.<RuleClass>alwaysTrue()}, which is a sentinel value for other parts of bazel.
+ */
+ public static RuleClassNamePredicate unspecified() {
+ return new RuleClassNamePredicate(Predicates.alwaysTrue(), Predicates.alwaysTrue(), null);
+ }
+
+ public final Predicate<String> asPredicateOfRuleClassName() {
+ return ruleClassNamePredicate;
+ }
+
+ public final Predicate<RuleClass> asPredicateOfRuleClass() {
+ return ruleClassPredicate;
+ }
+
+ /**
+ * Determines whether two {@code RuleClassNamePredicate}s should be considered incompatible as
+ * rule class predicate and rule class warning predicate.
+ *
+ * <p>Specifically, if both list sets of explicit rule class names to permit, those two sets
+ * must be disjoint, so the restriction only applies when both predicates have been created by
+ * {@link #only}.
+ */
+ boolean consideredOverlapping(RuleClassNamePredicate that) {
+ return this.overlappable != null
+ && that.overlappable != null
+ && !Collections.disjoint(this.overlappable, that.overlappable);
}
@Override
public int hashCode() {
- return ruleClasses.hashCode();
+ return ruleClassNamePredicate.hashCode();
}
@Override
- public boolean equals(Object o) {
- return (o instanceof RuleClassNamePredicate)
- && ruleClasses.equals(((RuleClassNamePredicate) o).ruleClasses);
- }
-
- /**
- * Returns true if this and the other predicate have common rule class entries.
- */
- public boolean intersects(RuleClassNamePredicate other) {
- return !Collections.disjoint(ruleClasses, other.ruleClasses);
+ public boolean equals(Object obj) {
+ // NOTE: Specifically not checking equality of ruleClassPredicate.
+ // By construction, if the name predicates are equals, the rule class predicates are, too.
+ return obj instanceof RuleClassNamePredicate
+ && ruleClassNamePredicate.equals(((RuleClassNamePredicate) obj).ruleClassNamePredicate);
}
@Override
public String toString() {
- return ruleClasses.isEmpty() ? "nothing" : StringUtil.joinEnglishList(ruleClasses);
+ return ruleClassNamePredicate.toString();
+ }
+
+ /** A pass-through predicate, except that an explicit {@link #toString()} is provided. */
+ private static class DescribedPredicate<T> implements Predicate<T> {
+ private final Predicate<T> delegate; // the actual predicate
+ private final String description;
+
+ private DescribedPredicate(Predicate<T> delegate, String description) {
+ this.delegate = delegate;
+ this.description = description;
+ }
+
+ @Override
+ public boolean apply(T input) {
+ return delegate.apply(input);
+ }
+
+ @Override
+ public int hashCode() {
+ return delegate.hashCode();
+ }
+
+ @Override
+ public boolean equals(Object obj) {
+ return obj instanceof DescribedPredicate
+ && delegate.equals(((DescribedPredicate<?>) obj).delegate);
+ }
+
+ @Override
+ public String toString() {
+ return description;
+ }
}
}