Add debugging helper to Environment.Extension to say why something's unequal
RELNOTES: None
PiperOrigin-RevId: 184649483
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 038961f..cafa8a0 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 @@
/**
* Cached hash code for the transitive content of this {@code Extension} and its dependencies.
+ *
+ * <p>Note that "content" refers to the AST content, not the evaluated bindings.
*/
private final String transitiveContentHashCode;
@@ -389,6 +394,61 @@
&& bindings.equals(other.getBindings());
}
+ /**
+ * Throws {@link IllegalStateException} if this {@link Extension} is not equal to {@code obj}.
+ *
+ * <p>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<String, Object> otherBindings = other.getBindings();
+
+ Set<String> names = bindings.keySet();
+ Set<String> 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<String> 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 1254fef..e6c0bd1 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 @@
+ "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);
+ }
}
