diff --git a/src/main/java/com/google/devtools/build/lib/syntax/Environment.java b/src/main/java/com/google/devtools/build/lib/syntax/Environment.java index 038961f9464d00..cafa8a0abc86fa 100644 --- a/src/main/java/com/google/devtools/build/lib/syntax/Environment.java +++ b/src/main/java/com/google/devtools/build/lib/syntax/Environment.java @@ -14,8 +14,10 @@ package com.google.devtools.build.lib.syntax; +import com.google.common.base.Joiner; import com.google.common.base.Preconditions; import com.google.common.collect.ImmutableMap; +import com.google.common.collect.Sets; import com.google.devtools.build.lib.cmdline.Label; import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable; import com.google.devtools.build.lib.events.Event; @@ -27,6 +29,7 @@ import com.google.devtools.build.lib.util.Fingerprint; import com.google.devtools.build.lib.util.Pair; import com.google.devtools.build.lib.util.SpellChecker; +import java.util.ArrayList; import java.util.Collections; import java.util.HashMap; import java.util.HashSet; @@ -351,6 +354,8 @@ public static final class Extension { /** * Cached hash code for the transitive content of this {@code Extension} and its dependencies. + * + *

Note that "content" refers to the AST content, not the evaluated bindings. */ private final String transitiveContentHashCode; @@ -389,6 +394,61 @@ public boolean equals(Object obj) { && bindings.equals(other.getBindings()); } + /** + * Throws {@link IllegalStateException} if this {@link Extension} is not equal to {@code obj}. + * + *

The exception explains the reason for the inequality, including all unequal bindings. + */ + public void checkStateEquals(Object obj) { + if (this == obj) { + return; + } + if (!(obj instanceof Extension)) { + throw new IllegalStateException(String.format( + "Expected an equal Extension, but got a %s instead of an Extension", + obj == null ? "null" : obj.getClass().getName())); + } + Extension other = (Extension) obj; + ImmutableMap otherBindings = other.getBindings(); + + Set names = bindings.keySet(); + Set otherNames = otherBindings.keySet(); + if (!names.equals(otherNames)) { + throw new IllegalStateException(String.format( + "Expected Extensions to be equal, but they don't define the same bindings: " + + "in this one but not given one: [%s]; in given one but not this one: [%s]", + Joiner.on(", ").join(Sets.difference(names, otherNames)), + Joiner.on(", ").join(Sets.difference(otherNames, names)))); + } + + ArrayList badEntries = new ArrayList<>(); + for (String name : names) { + Object value = bindings.get(name); + Object otherValue = otherBindings.get(name); + if (!value.equals(otherValue)) { + badEntries.add(String.format( + "%s: this one has %s (class %s), but given one has %s (class %s)", + name, + Printer.repr(value), + value.getClass().getName(), + Printer.repr(otherValue), + otherValue.getClass().getName())); + } + } + if (!badEntries.isEmpty()) { + throw new IllegalStateException( + "Expected Extensions to be equal, but the following bindings are unequal: " + + Joiner.on("; ").join(badEntries)); + } + + if (!transitiveContentHashCode.equals(other.getTransitiveContentHashCode())) { + throw new IllegalStateException(String.format( + "Expected Extensions to be equal, but transitive content hashes don't match: %s != %s", + transitiveContentHashCode, + other.getTransitiveContentHashCode())); + } + } + @Override public int hashCode() { return Objects.hash(bindings, transitiveContentHashCode); diff --git a/src/test/java/com/google/devtools/build/lib/syntax/EnvironmentTest.java b/src/test/java/com/google/devtools/build/lib/syntax/EnvironmentTest.java index 1254fef4bbe9cd..e6c0bd11e8298d 100644 --- a/src/test/java/com/google/devtools/build/lib/syntax/EnvironmentTest.java +++ b/src/test/java/com/google/devtools/build/lib/syntax/EnvironmentTest.java @@ -18,7 +18,9 @@ import static com.google.devtools.build.lib.testutil.MoreAsserts.expectThrows; import static org.junit.Assert.fail; +import com.google.common.collect.ImmutableMap; import com.google.common.collect.Sets; +import com.google.devtools.build.lib.syntax.Environment.Extension; import com.google.devtools.build.lib.syntax.util.EvaluationTestCase; import org.junit.Test; import org.junit.runner.RunWith; @@ -270,4 +272,55 @@ public void testReadOnly() throws Exception { + "The variable is defined in the global scope."); } } + + @Test + public void testExtensionEqualityDebugging_RhsIsNull() { + assertCheckStateFailsWithMessage( + new Extension(ImmutableMap.of(), "abc"), + null, + "got a null"); + } + + @Test + public void testExtensionEqualityDebugging_RhsHasBadType() { + assertCheckStateFailsWithMessage( + new Extension(ImmutableMap.of(), "abc"), + 5, + "got a java.lang.Integer"); + } + + @Test + public void testExtensionEqualityDebugging_DifferentBindings() { + assertCheckStateFailsWithMessage( + new Extension(ImmutableMap.of("w", 1, "x", 2, "y", 3), "abc"), + new Extension(ImmutableMap.of("y", 3, "z", 4), "abc"), + "in this one but not given one: [w, x]; in given one but not this one: [z]"); + } + + @Test + public void testExtensionEqualityDebugging_DifferentValues() { + assertCheckStateFailsWithMessage( + new Extension(ImmutableMap.of("x", 1, "y", "foo", "z", true), "abc"), + new Extension(ImmutableMap.of("x", 2.0, "y", "foo", "z", false), "abc"), + "bindings are unequal: x: this one has 1 (class java.lang.Integer), but given one has 2.0 " + + "(class java.lang.Double); z: this one has True (class java.lang.Boolean), but given " + + "one has False (class java.lang.Boolean)"); + } + + @Test + public void testExtensionEqualityDebugging_DifferentHashes() { + assertCheckStateFailsWithMessage( + new Extension(ImmutableMap.of(), "abc"), + new Extension(ImmutableMap.of(), "xyz"), + "transitive content hashes don't match: abc != xyz"); + } + + private static void assertCheckStateFailsWithMessage( + Extension left, Object right, String substring) { + IllegalStateException expected = + expectThrows( + IllegalStateException.class, + () -> left.checkStateEquals(right)); + assertThat(expected).hasMessageThat().contains(substring); + } }