diff --git a/src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkAttrModule.java b/src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkAttrModule.java index f4ef3a81931c18..90dd4a5bc2e2e9 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkAttrModule.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkAttrModule.java @@ -48,6 +48,7 @@ import com.google.devtools.build.lib.util.FileTypeSet; import java.util.List; import java.util.Map; +import java.util.Objects; import java.util.Optional; import javax.annotation.Nullable; import net.starlark.java.eval.Dict; @@ -790,6 +791,25 @@ public Attribute build(String name) { public void repr(Printer printer) { printer.append(""); } + + // Value equality semantics - same as for native Attribute. + @Override + public boolean equals(Object o) { + if (this == o) { + return true; + } + if (!(o instanceof Descriptor)) { + return false; + } + Descriptor that = (Descriptor) o; + return Objects.equals(name, that.name) + && Objects.equals(attributeFactory, that.attributeFactory); + } + + @Override + public int hashCode() { + return Objects.hash(name, attributeFactory); + } } // Returns an immutable map from a list of alternating name/value pairs, 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 6d4eeb61228a85..7703722a083638 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 @@ -265,6 +265,7 @@ public static class ImmutableAttributeFactory { private final PredicateWithMessage allowedValues; private final RequiredProviders requiredProviders; private final ImmutableList> aspects; + private final int hashCode; private ImmutableAttributeFactory( Type type, @@ -295,6 +296,22 @@ private ImmutableAttributeFactory( this.allowedValues = allowedValues; this.requiredProviders = requiredProviders; this.aspects = aspects; + this.hashCode = + Objects.hash( + type, + doc, + transitionFactory, + allowedRuleClassesForLabels, + allowedRuleClassesForLabelsWarning, + allowedFileTypesForLabels, + validityPredicate, + value, + valueSource, + valueSet, + propertyFlags, + allowedValues, + requiredProviders, + aspects); } public AttributeValueSource getValueSource() { @@ -344,6 +361,39 @@ public Attribute build(String name) { requiredProviders, aspects); } + + // Value equality semantics - same as for Attribute. + @Override + public boolean equals(Object o) { + if (this == o) { + return true; + } + if (!(o instanceof ImmutableAttributeFactory)) { + return false; + } + ImmutableAttributeFactory that = (ImmutableAttributeFactory) o; + return hashCode == that.hashCode + && Objects.equals(type, that.type) + && Objects.equals(doc, that.doc) + && Objects.equals(transitionFactory, that.transitionFactory) + && Objects.equals(allowedRuleClassesForLabels, that.allowedRuleClassesForLabels) + && Objects.equals( + allowedRuleClassesForLabelsWarning, that.allowedRuleClassesForLabelsWarning) + && Objects.equals(allowedFileTypesForLabels, that.allowedFileTypesForLabels) + && Objects.equals(validityPredicate, that.validityPredicate) + && Objects.equals(value, that.value) + && Objects.equals(valueSource, that.valueSource) + && valueSet == that.valueSet + && Objects.equals(propertyFlags, that.propertyFlags) + && Objects.equals(allowedValues, that.allowedValues) + && Objects.equals(requiredProviders, that.requiredProviders) + && Objects.equals(aspects, that.aspects); + } + + @Override + public int hashCode() { + return hashCode; + } } /** diff --git a/src/test/java/com/google/devtools/build/lib/packages/AttributeTest.java b/src/test/java/com/google/devtools/build/lib/packages/AttributeTest.java index 319fbbdaca5336..dfdd937dd22ace 100644 --- a/src/test/java/com/google/devtools/build/lib/packages/AttributeTest.java +++ b/src/test/java/com/google/devtools/build/lib/packages/AttributeTest.java @@ -17,6 +17,7 @@ import static com.google.devtools.build.lib.packages.Attribute.attr; import static com.google.devtools.build.lib.packages.BuildType.LABEL; import static com.google.devtools.build.lib.packages.BuildType.LABEL_LIST; +import static com.google.devtools.build.lib.packages.BuildType.NODEP_LABEL; import static com.google.devtools.build.lib.packages.Type.INTEGER; import static com.google.devtools.build.lib.packages.Type.STRING; import static com.google.devtools.build.lib.packages.Type.STRING_LIST; @@ -24,6 +25,8 @@ import com.google.common.base.Predicates; import com.google.common.collect.ImmutableMap; +import com.google.common.testing.EqualsTester; +import com.google.devtools.build.lib.analysis.DefaultInfo; import com.google.devtools.build.lib.analysis.config.BuildOptions; import com.google.devtools.build.lib.analysis.config.BuildOptionsView; import com.google.devtools.build.lib.analysis.config.ExecutionTransitionFactory; @@ -34,6 +37,7 @@ import com.google.devtools.build.lib.analysis.util.TestAspects; import com.google.devtools.build.lib.cmdline.Label; import com.google.devtools.build.lib.events.EventHandler; +import com.google.devtools.build.lib.packages.Attribute.AllowedValueSet; import com.google.devtools.build.lib.packages.RuleClass.Builder.RuleClassNamePredicate; import com.google.devtools.build.lib.testutil.FakeAttributeMapper; import com.google.devtools.build.lib.util.FileType; @@ -325,4 +329,77 @@ public void allowedRuleClassesAndAllowedRuleClassesWithWarningsCannotOverlap() { .build()); assertThat(e).hasMessageThat().contains("may not contain the same rule classes"); } + + @Test + public void factoryEquality() throws Exception { + new EqualsTester() + .addEqualityGroup(attr("foo", LABEL).buildPartial(), attr("foo", LABEL).buildPartial()) + .addEqualityGroup( + attr("foo", LABEL).value(Label.parseCanonicalUnchecked("//a:b")).buildPartial(), + attr("foo", LABEL).value(Label.parseCanonicalUnchecked("//a:b")).buildPartial()) + .addEqualityGroup( + attr("foo", NODEP_LABEL).value(Label.parseCanonicalUnchecked("//a:b")).buildPartial(), + attr("foo", NODEP_LABEL).value(Label.parseCanonicalUnchecked("//a:b")).buildPartial()) + .addEqualityGroup( + attr("foo", LABEL).value(Label.parseCanonicalUnchecked("//c:d")).buildPartial(), + attr("foo", LABEL).value(Label.parseCanonicalUnchecked("//c:d")).buildPartial()) + .addEqualityGroup( + attr("foo", LABEL) + .value(Label.parseCanonicalUnchecked("//a:b")) + .setDoc("My doc") + .buildPartial(), + attr("foo", LABEL) + .value(Label.parseCanonicalUnchecked("//a:b")) + .setDoc("My doc") + .buildPartial()) + .addEqualityGroup( + // PredicateWithMessage does not define any particular equality semantics + attr("foo", LABEL) + .value(Label.parseCanonicalUnchecked("//a:b")) + .allowedValues(new AllowedValueSet(Label.parseCanonical("//a:b"))) + .buildPartial()) + .addEqualityGroup( + attr("foo", LABEL) + .value(Label.parseCanonicalUnchecked("//a:b")) + .validityPredicate(Attribute.ANY_EDGE) + .buildPartial(), + attr("foo", LABEL) + .value(Label.parseCanonicalUnchecked("//a:b")) + .validityPredicate(Attribute.ANY_EDGE) + .buildPartial()) + .addEqualityGroup( + attr("foo", LABEL) + .value(Label.parseCanonicalUnchecked("//a:b")) + .allowedRuleClasses("java_binary") + .buildPartial(), + attr("foo", LABEL) + .value(Label.parseCanonicalUnchecked("//a:b")) + .allowedRuleClasses("java_binary") + .buildPartial()) + .addEqualityGroup( + attr("foo", LABEL) + .value(Label.parseCanonicalUnchecked("//a:b")) + .allowedFileTypes(FileTypeSet.ANY_FILE) + .buildPartial(), + attr("foo", LABEL) + .value(Label.parseCanonicalUnchecked("//a:b")) + .allowedFileTypes(FileTypeSet.ANY_FILE) + .buildPartial()) + .addEqualityGroup( + attr("foo", LABEL) + .value(Label.parseCanonicalUnchecked("//a:b")) + .mandatoryProviders(DefaultInfo.PROVIDER.id()) + .buildPartial(), + attr("foo", LABEL) + .value(Label.parseCanonicalUnchecked("//a:b")) + .mandatoryProviders(DefaultInfo.PROVIDER.id()) + .buildPartial()) + .addEqualityGroup( + // Aspects list builder does not define any particular equality semantics + attr("foo", LABEL) + .value(Label.parseCanonicalUnchecked("//a:b")) + .aspect(TestAspects.SIMPLE_ASPECT) + .buildPartial()) + .testEquals(); + } } diff --git a/src/test/java/com/google/devtools/build/lib/starlark/BUILD b/src/test/java/com/google/devtools/build/lib/starlark/BUILD index d0aa480040194f..a0f63877eb8346 100644 --- a/src/test/java/com/google/devtools/build/lib/starlark/BUILD +++ b/src/test/java/com/google/devtools/build/lib/starlark/BUILD @@ -85,6 +85,7 @@ java_test( "//src/test/java/com/google/devtools/build/lib/testutil", "//src/test/java/com/google/devtools/build/lib/testutil:JunitUtils", "//third_party:guava", + "//third_party:guava-testlib", "//third_party:jsr305", "//third_party:junit4", "//third_party:mockito", diff --git a/src/test/java/com/google/devtools/build/lib/starlark/StarlarkRuleClassFunctionsTest.java b/src/test/java/com/google/devtools/build/lib/starlark/StarlarkRuleClassFunctionsTest.java index e58342bcb74476..ebf17e281f139e 100644 --- a/src/test/java/com/google/devtools/build/lib/starlark/StarlarkRuleClassFunctionsTest.java +++ b/src/test/java/com/google/devtools/build/lib/starlark/StarlarkRuleClassFunctionsTest.java @@ -25,6 +25,7 @@ import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.common.collect.Iterables; +import com.google.common.testing.EqualsTester; import com.google.devtools.build.lib.analysis.ConfiguredRuleClassProvider; import com.google.devtools.build.lib.analysis.RuleContext; import com.google.devtools.build.lib.analysis.config.transitions.NoTransition; @@ -287,6 +288,33 @@ public void testAttrNameCannotStartWithDigit() throws Exception { ev.assertContainsError("attribute name `2_foo` is not a valid identifier"); } + @Test + public void testAttrEquality() throws Exception { + new EqualsTester() + .addEqualityGroup( + buildAttribute("foo", "attr.string_list(default = [])"), + buildAttribute("foo", "attr.string_list(default = [])")) + .addEqualityGroup( + buildAttribute("bar", "attr.string_list(default = [])"), + buildAttribute("bar", "attr.string_list(default = [])")) + .addEqualityGroup( + buildAttribute("bar", "attr.label_list(default = [])"), + buildAttribute("bar", "attr.label_list(default = [])")) + .addEqualityGroup( + buildAttribute("foo", "attr.string_list(default = ['hello'])"), + buildAttribute("foo", "attr.string_list(default = ['hello'])")) + .addEqualityGroup( + buildAttribute("foo", "attr.string_list(doc = 'Blah blah blah', default = [])"), + buildAttribute("foo", "attr.string_list(doc = 'Blah blah blah', default = [])")) + .addEqualityGroup( + buildAttribute("foo", "attr.string_list(mandatory = True, default = [])"), + buildAttribute("foo", "attr.string_list(mandatory = True, default = [])")) + .addEqualityGroup( + buildAttribute("foo", "attr.string_list(allow_empty = False, default = [])"), + buildAttribute("foo", "attr.string_list(allow_empty = False, default = [])")) + .testEquals(); + } + @Test public void testRuleClassTooManyAttributes() throws Exception { ev.setFailFast(false);