Skip to content

Commit da5d6e9

Browse files
cpovirkGoogle Java Core Libraries
cpovirk
authored and
Google Java Core Libraries
committed
[Fail instead of throwing](#207) for null maps and multimaps (and negative sizes).
This CL includes a larger change to the behavior of at least one edge-case assertion: ``` assertThat(nullMap).containsAtLeastEntriesIn(empty); ``` Prior to this CL, that assertion would succeed. Now, it will fail. Additionally, this CL fixes yet another parameter name that I missed. This time, it's the one for `MultimapSubject.isEqualTo`. As a minor implementation point, this CL renames "`getCastSubject`" to "`castActual`," eliminating what I hope is one of the final usages of "subject" to refer to the value under test. This is just in implementation code, though, not in Javadoc or public parameter names. RELNOTES=Changed assertions like `assertThat(nullMap).isEmpty()` to fail with a useful failure message instead of throwing `NullPointerException`, and similarly for other "bogus" values, such as negative sizes. PiperOrigin-RevId: 773487350
1 parent e132196 commit da5d6e9

File tree

5 files changed

+493
-150
lines changed

5 files changed

+493
-150
lines changed

‎core/src/main/java/com/google/common/truth/MapSubject.java‎

Lines changed: 122 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,6 @@
2929
import static java.util.Collections.singletonList;
3030

3131
import com.google.common.collect.ImmutableList;
32-
import com.google.common.collect.ImmutableMap;
3332
import com.google.common.collect.LinkedHashMultiset;
3433
import com.google.common.collect.Maps;
3534
import com.google.common.collect.Multiset;
@@ -78,46 +77,70 @@ public final void isEqualTo(@Nullable Object expected) {
7877
return;
7978
}
8079

81-
containsEntriesInAnyOrder((Map<?, ?>) expected, /* allowUnexpected= */ false);
80+
containsEntriesInAnyOrder(actual, (Map<?, ?>) expected, /* allowUnexpected= */ false);
8281
}
8382

8483
/** Checks that the actual map is empty. */
8584
public final void isEmpty() {
86-
if (!checkNotNull(actual).isEmpty()) {
85+
if (actual == null) {
86+
failWithActual(simpleFact("expected an empty map"));
87+
} else if (!actual.isEmpty()) {
8788
failWithActual(simpleFact("expected to be empty"));
8889
}
8990
}
9091

9192
/** Checks that the actual map is not empty. */
9293
public final void isNotEmpty() {
93-
if (checkNotNull(actual).isEmpty()) {
94+
if (actual == null) {
95+
failWithActual(simpleFact("expected a nonempty map"));
96+
} else if (actual.isEmpty()) {
9497
failWithoutActual(simpleFact("expected not to be empty"));
9598
}
9699
}
97100

98101
/** Checks that the actual map has the given size. */
99102
public final void hasSize(int size) {
100-
checkArgument(size >= 0, "expected size (%s) must be >= 0", size);
101-
check("size()").that(checkNotNull(actual).size()).isEqualTo(size);
103+
if (actual == null) {
104+
failWithActual("expected a map with size", size);
105+
} else if (size < 0) {
106+
failWithoutActual(
107+
simpleFact("expected a map with a negative size, but that is impossible"),
108+
fact("expected size", size),
109+
fact("actual size", actual.size()),
110+
actualContents());
111+
} else {
112+
check("size()").that(actual.size()).isEqualTo(size);
113+
}
102114
}
103115

104116
/** Checks that the actual map contains the given key. */
105117
public final void containsKey(@Nullable Object key) {
106-
check("keySet()").that(checkNotNull(actual).keySet()).contains(key);
118+
if (actual == null) {
119+
failWithActual("expected a map that contains key", key);
120+
return;
121+
}
122+
check("keySet()").that(actual.keySet()).contains(key);
107123
}
108124

109125
/** Checks that the actual map does not contain the given key. */
110126
public final void doesNotContainKey(@Nullable Object key) {
111-
check("keySet()").that(checkNotNull(actual).keySet()).doesNotContain(key);
127+
if (actual == null) {
128+
failWithActual("expected a map that does not contain key", key);
129+
return;
130+
}
131+
check("keySet()").that(actual.keySet()).doesNotContain(key);
112132
}
113133

114134
/** Checks that the actual map contains the given entry. */
115135
public final void containsEntry(@Nullable Object key, @Nullable Object value) {
116136
Entry<?, ?> entry = immutableEntry(key, value);
117-
checkNotNull(actual);
137+
if (actual == null) {
138+
failWithActual("expected a map that contains entry", entry);
139+
return;
140+
}
118141
if (!actual.entrySet().contains(entry)) {
119-
List<@Nullable Object> keyList = singletonList(key);
120-
List<@Nullable Object> valueList = singletonList(value);
142+
List<?> keyList = singletonList(key);
143+
List<?> valueList = singletonList(value);
121144
if (actual.containsKey(key)) {
122145
Object actualValue = actual.get(key);
123146
if (Objects.equals(actualValue, value)) {
@@ -185,15 +208,22 @@ public final void containsEntry(@Nullable Object key, @Nullable Object value) {
185208

186209
/** Checks that the actual map does not contain the given entry. */
187210
public final void doesNotContainEntry(@Nullable Object key, @Nullable Object value) {
188-
checkNoNeedToDisplayBothValues("entrySet()")
189-
.that(checkNotNull(actual).entrySet())
190-
.doesNotContain(immutableEntry(key, value));
211+
Entry<?, ?> entry = immutableEntry(key, value);
212+
if (actual == null) {
213+
failWithActual("expected a map that does not contain entry", entry);
214+
return;
215+
}
216+
checkNoNeedToDisplayBothValues("entrySet()").that(actual.entrySet()).doesNotContain(entry);
191217
}
192218

193219
/** Checks that the actual map is empty. */
194220
@CanIgnoreReturnValue
195221
public final Ordered containsExactly() {
196-
return containsExactlyEntriesIn(ImmutableMap.of());
222+
isEmpty();
223+
// If the actual value was empty, then it is vacuously in order, so we'd return IN_ORDER.
224+
// If the actual value not empty (or was null), then we'd return ALREADY_FAILED.
225+
// Luckily, the two are equivalent, so it doesn't matter which we pick.
226+
return IN_ORDER;
197227
}
198228

199229
/**
@@ -243,36 +273,49 @@ public final Ordered containsAtLeast(
243273

244274
/** Checks that the actual map contains exactly the given set of entries in the given map. */
245275
@CanIgnoreReturnValue
246-
public final Ordered containsExactlyEntriesIn(Map<?, ?> expected) {
247-
if (expected.isEmpty()) {
248-
if (checkNotNull(actual).isEmpty()) {
249-
return IN_ORDER;
250-
} else {
251-
isEmpty(); // fails
252-
return ALREADY_FAILED;
253-
}
276+
public final Ordered containsExactlyEntriesIn(@Nullable Map<?, ?> expected) {
277+
if (expected == null) {
278+
failWithoutActual(
279+
simpleFact("could not perform containment check because expected map was null"),
280+
actualContents());
281+
return ALREADY_FAILED;
282+
} else if (actual == null) {
283+
failWithActual("expected a map that contains exactly", expected);
284+
return ALREADY_FAILED;
285+
} else if (expected.isEmpty()) {
286+
return containsExactly();
254287
}
255-
return containsEntriesInAnyOrder(expected, /* allowUnexpected= */ false)
288+
return containsEntriesInAnyOrder(actual, expected, /* allowUnexpected= */ false)
256289
? MapInOrder.create(
257-
this, expected, /* allowUnexpected= */ false, /* correspondence= */ null)
290+
this, actual, expected, /* allowUnexpected= */ false, /* correspondence= */ null)
258291
: ALREADY_FAILED;
259292
}
260293

261294
/** Checks that the actual map contains at least the given set of entries in the given map. */
262295
@CanIgnoreReturnValue
263-
public final Ordered containsAtLeastEntriesIn(Map<?, ?> expected) {
264-
if (expected.isEmpty()) {
296+
public final Ordered containsAtLeastEntriesIn(@Nullable Map<?, ?> expected) {
297+
if (expected == null) {
298+
failWithoutActual(
299+
simpleFact("could not perform containment check because expected map was null"),
300+
actualContents());
301+
return ALREADY_FAILED;
302+
} else if (actual == null) {
303+
failWithActual("expected a map that contains at least", expected);
304+
return ALREADY_FAILED;
305+
} else if (expected.isEmpty()) {
265306
return IN_ORDER;
266307
}
267-
return containsEntriesInAnyOrder(expected, /* allowUnexpected= */ true)
268-
? MapInOrder.create(this, expected, /* allowUnexpected= */ true, /* correspondence= */ null)
308+
return containsEntriesInAnyOrder(actual, expected, /* allowUnexpected= */ true)
309+
? MapInOrder.create(
310+
this, actual, expected, /* allowUnexpected= */ true, /* correspondence= */ null)
269311
: ALREADY_FAILED;
270312
}
271313

272314
@CanIgnoreReturnValue
273-
private boolean containsEntriesInAnyOrder(Map<?, ?> expected, boolean allowUnexpected) {
315+
private boolean containsEntriesInAnyOrder(
316+
Map<?, ?> actual, Map<?, ?> expected, boolean allowUnexpected) {
274317
MapDifference<@Nullable Object, @Nullable Object, @Nullable Object> diff =
275-
MapDifference.create(checkNotNull(actual), expected, allowUnexpected, Objects::equals);
318+
MapDifference.create(actual, expected, allowUnexpected, Objects::equals);
276319
if (diff.isEmpty()) {
277320
return true;
278321
}
@@ -438,16 +481,19 @@ private static String maybeAddType(@Nullable Object o, boolean includeTypes) {
438481

439482
private static final class MapInOrder implements Ordered {
440483
private final MapSubject subject;
484+
private final Map<?, ?> actual;
441485
private final Map<?, ?> expectedMap;
442486
private final boolean allowUnexpected;
443487
private final @Nullable Correspondence<?, ?> correspondence;
444488

445489
private MapInOrder(
446490
MapSubject subject,
491+
Map<?, ?> actual,
447492
Map<?, ?> expectedMap,
448493
boolean allowUnexpected,
449494
@Nullable Correspondence<?, ?> correspondence) {
450495
this.subject = subject;
496+
this.actual = actual;
451497
this.expectedMap = expectedMap;
452498
this.allowUnexpected = allowUnexpected;
453499
this.correspondence = correspondence;
@@ -462,11 +508,10 @@ private MapInOrder(
462508
@Override
463509
public void inOrder() {
464510
// We're using the fact that Sets.intersection keeps the order of the first set.
465-
checkNotNull(subject.actual);
466511
List<?> expectedKeyOrder =
467-
new ArrayList<>(Sets.intersection(expectedMap.keySet(), subject.actual.keySet()));
512+
new ArrayList<>(Sets.intersection(expectedMap.keySet(), actual.keySet()));
468513
List<?> actualKeyOrder =
469-
new ArrayList<>(Sets.intersection(subject.actual.keySet(), expectedMap.keySet()));
514+
new ArrayList<>(Sets.intersection(actual.keySet(), expectedMap.keySet()));
470515
if (!actualKeyOrder.equals(expectedKeyOrder)) {
471516
ImmutableList.Builder<Fact> facts =
472517
factsBuilder()
@@ -492,10 +537,11 @@ private void failWithActual(Iterable<Fact> facts) {
492537

493538
static MapInOrder create(
494539
MapSubject subject,
540+
Map<?, ?> actual,
495541
Map<?, ?> expectedMap,
496542
boolean allowUnexpected,
497543
@Nullable Correspondence<?, ?> correspondence) {
498-
return new MapInOrder(subject, expectedMap, allowUnexpected, correspondence);
544+
return new MapInOrder(subject, actual, expectedMap, allowUnexpected, correspondence);
499545
}
500546
}
501547

@@ -590,9 +636,13 @@ private UsingCorrespondence(
590636
*/
591637
@SuppressWarnings("UnnecessaryCast") // needed by nullness checker
592638
public void containsEntry(@Nullable Object key, E value) {
593-
if (checkNotNull(actual).containsKey(key)) {
639+
if (actual == null) {
640+
failWithActual("expected a map that contains entry", immutableEntry(key, value));
641+
return;
642+
}
643+
if (actual.containsKey(key)) {
594644
// Found matching key.
595-
A actualValue = getCastSubject().get(key);
645+
A actualValue = castActual(actual).get(key);
596646
Correspondence.ExceptionStore exceptions = Correspondence.ExceptionStore.forMapValues();
597647
if (correspondence.safeCompare((A) actualValue, value, exceptions)) {
598648
// The expected key had the expected value. There's no need to check exceptions here,
@@ -627,7 +677,7 @@ public void containsEntry(@Nullable Object key, E value) {
627677
// Did not find matching key. Look for the matching value with a different key.
628678
Set<@Nullable Object> keys = new LinkedHashSet<>();
629679
Correspondence.ExceptionStore exceptions = Correspondence.ExceptionStore.forMapValues();
630-
for (Map.Entry<?, A> actualEntry : getCastSubject().entrySet()) {
680+
for (Map.Entry<?, A> actualEntry : castActual(actual).entrySet()) {
631681
if (correspondence.safeCompare(actualEntry.getValue(), value, exceptions)) {
632682
keys.add(actualEntry.getKey());
633683
}
@@ -665,9 +715,13 @@ public void containsEntry(@Nullable Object key, E value) {
665715
*/
666716
@SuppressWarnings("UnnecessaryCast") // needed by nullness checker
667717
public void doesNotContainEntry(@Nullable Object key, E value) {
668-
if (checkNotNull(actual).containsKey(key)) {
718+
if (actual == null) {
719+
failWithActual("expected a map that does not contain entry", immutableEntry(key, value));
720+
return;
721+
}
722+
if (actual.containsKey(key)) {
669723
// Found matching key. Fail if the value matches, too.
670-
A actualValue = getCastSubject().get(key);
724+
A actualValue = castActual(actual).get(key);
671725
Correspondence.ExceptionStore exceptions = Correspondence.ExceptionStore.forMapValues();
672726
if (correspondence.safeCompare((A) actualValue, value, exceptions)) {
673727
// The matching key had a matching value. There's no need to check exceptions here,
@@ -741,36 +795,47 @@ public Ordered containsAtLeast(@Nullable Object k0, @Nullable E v0, @Nullable Ob
741795
* correspond to the values of the given map.
742796
*/
743797
@CanIgnoreReturnValue
744-
public Ordered containsExactlyEntriesIn(Map<?, ? extends E> expected) {
745-
if (expected.isEmpty()) {
746-
if (checkNotNull(actual).isEmpty()) {
747-
return IN_ORDER;
748-
} else {
749-
subject.isEmpty(); // fails
750-
return ALREADY_FAILED;
751-
}
798+
public Ordered containsExactlyEntriesIn(@Nullable Map<?, ? extends E> expected) {
799+
if (expected == null) {
800+
failWithoutActual(
801+
simpleFact("could not perform containment check because expected map was null"),
802+
actualContents());
803+
return ALREADY_FAILED;
804+
} else if (expected.isEmpty()) {
805+
return subject.containsExactly();
806+
} else if (actual == null) {
807+
failWithActual("expected a map that contains exactly", expected);
808+
return ALREADY_FAILED;
752809
}
753-
return internalContainsEntriesIn(expected, /* allowUnexpected= */ false);
810+
return internalContainsEntriesIn(actual, expected, /* allowUnexpected= */ false);
754811
}
755812

756813
/**
757814
* Checks that the actual map contains at least the keys in the given map, mapping to values
758815
* that correspond to the values of the given map.
759816
*/
760817
@CanIgnoreReturnValue
761-
public Ordered containsAtLeastEntriesIn(Map<?, ? extends E> expected) {
762-
if (expected.isEmpty()) {
818+
public Ordered containsAtLeastEntriesIn(@Nullable Map<?, ? extends E> expected) {
819+
if (expected == null) {
820+
failWithoutActual(
821+
simpleFact("could not perform containment check because expected map was null"),
822+
actualContents());
823+
return ALREADY_FAILED;
824+
} else if (expected.isEmpty()) {
763825
return IN_ORDER;
826+
} else if (actual == null) {
827+
failWithActual("expected a map that contains at least", expected);
828+
return ALREADY_FAILED;
764829
}
765-
return internalContainsEntriesIn(expected, /* allowUnexpected= */ true);
830+
return internalContainsEntriesIn(actual, expected, /* allowUnexpected= */ true);
766831
}
767832

768833
private <K extends @Nullable Object, V extends E> Ordered internalContainsEntriesIn(
769-
Map<K, V> expected, boolean allowUnexpected) {
834+
Map<?, ?> actual, Map<K, V> expected, boolean allowUnexpected) {
770835
Correspondence.ExceptionStore exceptions = Correspondence.ExceptionStore.forMapValues();
771836
MapDifference<@Nullable Object, A, V> diff =
772837
MapDifference.create(
773-
getCastSubject(),
838+
castActual(actual),
774839
expected,
775840
allowUnexpected,
776841
(actualValue, expectedValue) ->
@@ -779,7 +844,7 @@ public Ordered containsAtLeastEntriesIn(Map<?, ? extends E> expected) {
779844
// The maps correspond exactly. There's no need to check exceptions here, because if
780845
// Correspondence.compare() threw then safeCompare() would return false and the diff would
781846
// record that we had the wrong value for that key.
782-
return MapInOrder.create(subject, expected, allowUnexpected, correspondence);
847+
return MapInOrder.create(subject, actual, expected, allowUnexpected, correspondence);
783848
}
784849
failWithoutActual(
785850
factsBuilder()
@@ -798,8 +863,8 @@ private <V extends E> Differ<A, V> differ(Correspondence.ExceptionStore exceptio
798863
}
799864

800865
@SuppressWarnings("unchecked") // throwing ClassCastException is the correct behaviour
801-
private Map<?, A> getCastSubject() {
802-
return (Map<?, A>) checkNotNull(actual);
866+
private Map<?, A> castActual(Map<?, ?> actual) {
867+
return (Map<?, A>) actual;
803868
}
804869

805870
private String actualCustomStringRepresentationForPackageMembersToCall() {

0 commit comments

Comments
 (0)