[Avoid throwing](#207) for null and other "weird" inputs.

cpovirk · Google Java Core Libraries · commit 974ef195b1b0 · 2025-06-18T19:35:46.000-07:00
This includes making `isInstanceOf(int.class)` work like `isInstanceOf(Integer.class)`. This is particularly important for Kotlin callers: `isInstanceOf(Int::class.java)` behaves like `isInstanceOf(int.class)`. By changing this behavior, this CL revises what I originally did back in cl/478924507.

The rest of the changes implement the usual approach of failing instead of throwing an exception.

Plus, simplify `Subject.integralValue` now that b/419535511 is fixed.

RELNOTES=Made Kotlin's `isInstanceOf(Int::class.java)` (and Java's `isInstanceOf(int.class)`) a valid way to check for `Int`/`Integer` instances.
PiperOrigin-RevId: 773153958
diff --git a/core/src/main/java/com/google/common/truth/IntStreamSubject.java b/core/src/main/java/com/google/common/truth/IntStreamSubject.java
@@ -15,8 +15,8 @@
  */
 package com.google.common.truth;
 
-import static com.google.common.base.Preconditions.checkNotNull;
 import static com.google.common.base.Suppliers.memoize;
+import static com.google.common.truth.Fact.simpleFact;
 import static java.util.stream.Collectors.toCollection;
 
 import com.google.common.base.Supplier;
@@ -175,18 +175,12 @@ public Ordered containsAtLeastElementsIn(@Nullable Iterable<?> expected) {
    */
   @CanIgnoreReturnValue
   public Ordered containsExactly(int @Nullable ... expected) {
-    /*
-     * We declare a parameter type that lets callers pass a nullable array, even though the
-     * assertion will fail if the array is ever actually null. This can be convenient if the
-     * expected value comes from a nullable source (e.g., a map lookup): Users would otherwise have
-     * to use {@code requireNonNull} or {@code !!} or similar, all to address a compile error
-     * warning about a runtime failure that might never happen—a runtime failure that Truth could
-     * produce a better exception message for, since it could make the message express that the
-     * caller is performing a containsExactly assertion.
-     *
-     * TODO(cpovirk): Actually produce such a better exception message.
-     */
-    checkNotNull(expected);
+    if (expected == null) {
+      failWithoutActual(
+          simpleFact("could not perform containment check because expected array is null"),
+          actualContents());
+      return ALREADY_FAILED;
+    }
     return checkThatContentsList().containsExactlyElementsIn(box(expected));
   }
 
@@ -274,6 +268,13 @@ private static Object[] box(int[] rest) {
     return IntStream.of(rest).boxed().toArray(Integer[]::new);
   }
 
+  private Fact actualContents() {
+    return actualValue("actual contents");
+  }
+
+  /** Ordered implementation that does nothing because an earlier check already caused a failure. */
+  private static final Ordered ALREADY_FAILED = () -> {};
+
   // TODO: b/246961366 - Do we want to override + deprecate isEqualTo/isNotEqualTo?
 
   // TODO(user): Do we want to support comparingElementsUsing() on StreamSubject?
diff --git a/core/src/main/java/com/google/common/truth/LongStreamSubject.java b/core/src/main/java/com/google/common/truth/LongStreamSubject.java
@@ -15,8 +15,8 @@
  */
 package com.google.common.truth;
 
-import static com.google.common.base.Preconditions.checkNotNull;
 import static com.google.common.base.Suppliers.memoize;
+import static com.google.common.truth.Fact.simpleFact;
 import static java.util.stream.Collectors.toCollection;
 
 import com.google.common.base.Supplier;
@@ -175,18 +175,12 @@ public Ordered containsAtLeastElementsIn(@Nullable Iterable<?> expected) {
    */
   @CanIgnoreReturnValue
   public Ordered containsExactly(long @Nullable ... expected) {
-    /*
-     * We declare a parameter type that lets callers pass a nullable array, even though the
-     * assertion will fail if the array is ever actually null. This can be convenient if the
-     * expected value comes from a nullable source (e.g., a map lookup): Users would otherwise have
-     * to use {@code requireNonNull} or {@code !!} or similar, all to address a compile error
-     * warning about a runtime failure that might never happen—a runtime failure that Truth could
-     * produce a better exception message for, since it could make the message express that the
-     * caller is performing a containsExactly assertion.
-     *
-     * TODO(cpovirk): Actually produce such a better exception message.
-     */
-    checkNotNull(expected);
+    if (expected == null) {
+      failWithoutActual(
+          simpleFact("could not perform containment check because expected array is null"),
+          actualContents());
+      return ALREADY_FAILED;
+    }
     return checkThatContentsList().containsExactlyElementsIn(box(expected));
   }
 
@@ -274,6 +268,13 @@ private static Object[] box(long[] rest) {
     return LongStream.of(rest).boxed().toArray(Long[]::new);
   }
 
+  private Fact actualContents() {
+    return actualValue("actual contents");
+  }
+
+  /** Ordered implementation that does nothing because an earlier check already caused a failure. */
+  private static final Ordered ALREADY_FAILED = () -> {};
+
   // TODO: b/246961366 - Do we want to override + deprecate isEqualTo/isNotEqualTo?
 
   // TODO(user): Do we want to support comparingElementsUsing() on StreamSubject?
diff --git a/core/src/main/java/com/google/common/truth/Subject.java b/core/src/main/java/com/google/common/truth/Subject.java
@@ -19,11 +19,12 @@
 import static com.google.common.base.CaseFormat.UPPER_CAMEL;
 import static com.google.common.base.CharMatcher.whitespace;
 import static com.google.common.base.MoreObjects.firstNonNull;
-import static com.google.common.base.Preconditions.checkArgument;
 import static com.google.common.truth.Fact.fact;
 import static com.google.common.truth.Fact.simpleFact;
+import static com.google.common.truth.Platform.classMetadataUnsupported;
 import static com.google.common.truth.Platform.doubleToString;
 import static com.google.common.truth.Platform.floatToString;
+import static com.google.common.truth.Platform.isInstanceOfType;
 import static com.google.common.truth.Platform.isKotlinRange;
 import static com.google.common.truth.Platform.kotlinRangeContains;
 import static com.google.common.truth.Platform.stringValueForFailure;
@@ -236,7 +237,7 @@ private static boolean isIntegralBoxedPrimitive(@Nullable Object o) {
 
   private static long integralValue(Object o) {
     if (o instanceof Character) {
-      return (long) ((Character) o).charValue();
+      return (Character) o;
     } else if (o instanceof Number) {
       return ((Number) o).longValue();
     } else {
@@ -283,16 +284,20 @@ public final void isNotSameInstanceAs(@Nullable Object other) {
   }
 
   /** Checks that the value under test is an instance of the given class. */
-  public void isInstanceOf(Class<?> clazz) {
+  public void isInstanceOf(@Nullable Class<?> clazz) {
     if (clazz == null) {
-      throw new NullPointerException("clazz");
+      failWithoutActual(
+          simpleFact("could not perform instanceof check because expected type was null"),
+          actualValue("value to check was"));
+      return;
     }
+    clazz = Primitives.wrap(clazz);
     if (actual == null) {
       failWithActual("expected instance of", clazz.getName());
       return;
     }
     if (!isInstanceOfType(actual, clazz)) {
-      if (Platform.classMetadataUnsupported()) {
+      if (classMetadataUnsupported()) {
         throw new UnsupportedOperationException(
             actualCustomStringRepresentation()
                 + ", an instance of "
@@ -309,11 +314,15 @@ public void isInstanceOf(Class<?> clazz) {
   }
 
   /** Checks that the value under test is not an instance of the given class. */
-  public void isNotInstanceOf(Class<?> clazz) {
+  public void isNotInstanceOf(@Nullable Class<?> clazz) {
     if (clazz == null) {
-      throw new NullPointerException("clazz");
+      failWithoutActual(
+          simpleFact("could not perform instanceof check because expected type was null"),
+          actualValue("value to check was"));
+      return;
     }
-    if (Platform.classMetadataUnsupported()) {
+    clazz = Primitives.wrap(clazz);
+    if (classMetadataUnsupported()) {
       throw new UnsupportedOperationException(
           "isNotInstanceOf is not supported under -XdisableClassMetadata");
     }
@@ -329,15 +338,6 @@ public void isNotInstanceOf(Class<?> clazz) {
     }
   }
 
-  private static boolean isInstanceOfType(Object instance, Class<?> clazz) {
-    checkArgument(
-        !clazz.isPrimitive(),
-        "Cannot check instanceof for primitive type %s. Pass the wrapper class %s instead.",
-        clazz.getSimpleName(),
-        Primitives.wrap(clazz).getSimpleName());
-    return Platform.isInstanceOfType(instance, clazz);
-  }
-
   /** Checks that the value under test is equal to any element in the given iterable. */
   public void isIn(@Nullable Iterable<?> iterable) {
     if (iterable == null) {
diff --git a/core/src/main/java/com/google/common/truth/TruthFailureSubject.java b/core/src/main/java/com/google/common/truth/TruthFailureSubject.java
@@ -17,8 +17,6 @@
 package com.google.common.truth;
 
 import static com.google.common.base.MoreObjects.firstNonNull;
-import static com.google.common.base.Preconditions.checkArgument;
-import static com.google.common.base.Preconditions.checkNotNull;
 import static com.google.common.truth.Fact.fact;
 import static com.google.common.truth.Fact.simpleFact;
 
@@ -98,7 +96,7 @@ private static ImmutableList<String> getFactKeys(ErrorWithFacts error) {
    * fail the test. To assert about such a failure, use {@linkplain #factValue(String, int) the
    * other overload} of {@code factValue}.
    */
-  public StringSubject factValue(String key) {
+  public StringSubject factValue(@Nullable String key) {
     return doFactValue(key, null);
   }
 
@@ -107,13 +105,24 @@ public StringSubject factValue(String key) {
    * name. Most Truth failures do not contain multiple facts with the same key, so most tests should
    * use {@linkplain #factValue(String) the other overload} of {@code factValue}.
    */
-  public StringSubject factValue(String key, int index) {
-    checkArgument(index >= 0, "index must be nonnegative: %s", index);
+  public StringSubject factValue(@Nullable String key, int index) {
+    if (index < 0) {
+      failWithoutActual(
+          simpleFact("could not perform fact-value check because requested index is null"),
+          fact("requested key", key),
+          actualValue("for assertion about Truth failure"));
+      return ignoreCheck().that("");
+    }
     return doFactValue(key, index);
   }
 
-  private StringSubject doFactValue(String key, @Nullable Integer index) {
-    checkNotNull(key);
+  private StringSubject doFactValue(@Nullable String key, @Nullable Integer index) {
+    if (key == null) {
+      failWithoutActual(
+          simpleFact("could not perform fact-value check because requested key is null"),
+          actualValue("for assertion about Truth failure"));
+      return ignoreCheck().that("");
+    }
     if (!(actual instanceof ErrorWithFacts)) {
       failWithActual(simpleFact("expected a failure thrown by Truth's failure API"));
       return ignoreCheck().that("");
diff --git a/core/src/test/java/com/google/common/truth/IntStreamSubjectTest.java b/core/src/test/java/com/google/common/truth/IntStreamSubjectTest.java
@@ -245,6 +245,17 @@ public void containsExactly_fails() {
     assertFailureValue(e, "expected", "[42]");
   }
 
+  @Test
+  public void testContainsExactly_nullExpected() {
+    AssertionError expected =
+        expectFailure(
+            whenTesting -> whenTesting.that(IntStream.of(42, 43)).containsExactly((int[]) null));
+    assertFailureKeys(
+        expected,
+        "could not perform containment check because expected array is null",
+        "actual contents");
+  }
+
   @Test
   public void containsExactly_inOrder() {
     assertThat(IntStream.of(42, 43)).containsExactly(42, 43).inOrder();
diff --git a/core/src/test/java/com/google/common/truth/LongStreamSubjectTest.java b/core/src/test/java/com/google/common/truth/LongStreamSubjectTest.java
@@ -265,6 +265,17 @@ public void containsExactly_fails() {
     assertFailureValue(e, "expected", "[42]");
   }
 
+  @Test
+  public void testContainsExactly_nullExpected() {
+    AssertionError expected =
+        expectFailure(
+            whenTesting -> whenTesting.that(LongStream.of(42, 43)).containsExactly((long[]) null));
+    assertFailureKeys(
+        expected,
+        "could not perform containment check because expected array is null",
+        "actual contents");
+  }
+
   @Test
   public void containsExactly_inOrder() {
     assertThat(LongStream.of(42, 43)).containsExactly(42, 43).inOrder();
diff --git a/core/src/test/java/com/google/common/truth/SubjectTest.java b/core/src/test/java/com/google/common/truth/SubjectTest.java
@@ -550,11 +550,19 @@ public void isInstanceOfInterfaceForNull() {
     expectFailure(whenTesting -> whenTesting.that((Object) null).isInstanceOf(CharSequence.class));
   }
 
-  // false positive; actually an intentional trivially *false* check
-  @SuppressWarnings("IsInstanceInteger")
+  @SuppressWarnings("IsInstanceInteger") // test is an intentional trivially true check
   @Test
   public void isInstanceOfPrimitiveType() {
-    assertThrows(IllegalArgumentException.class, () -> assertThat(1).isInstanceOf(int.class));
+    assertThat(1).isInstanceOf(int.class);
+  }
+
+  @Test
+  public void isInstanceOfNull() {
+    AssertionError e = expectFailure(whenTesting -> whenTesting.that(1).isInstanceOf(null));
+    assertFailureKeys(
+        e,
+        "could not perform instanceof check because expected type was null",
+        "value to check was");
   }
 
   @Test
@@ -601,7 +609,16 @@ public void isNotInstanceOfImplementedInterface() {
 
   @Test
   public void isNotInstanceOfPrimitiveType() {
-    assertThrows(IllegalArgumentException.class, () -> assertThat(1).isNotInstanceOf(int.class));
+    expectFailure(whenTesting -> whenTesting.that(1).isNotInstanceOf(int.class));
+  }
+
+  @Test
+  public void isNotInstanceOfNull() {
+    AssertionError e = expectFailure(whenTesting -> whenTesting.that(1).isNotInstanceOf(null));
+    assertFailureKeys(
+        e,
+        "could not perform instanceof check because expected type was null",
+        "value to check was");
   }
 
   @Test
diff --git a/core/src/test/java/com/google/common/truth/TruthFailureSubjectTest.java b/core/src/test/java/com/google/common/truth/TruthFailureSubjectTest.java
@@ -23,7 +23,6 @@
 import static com.google.common.truth.FailureAssertions.assertFailureValue;
 import static com.google.common.truth.TruthFailureSubject.HOW_TO_TEST_KEYS_WITHOUT_VALUES;
 import static com.google.common.truth.TruthFailureSubject.truthFailures;
-import static org.junit.Assert.assertThrows;
 
 import com.google.common.collect.ImmutableList;
 import com.google.common.truth.ExpectFailure.SimpleSubjectBuilderCallback;
@@ -114,6 +113,16 @@ public void factValueFailNoValue() {
     assertFailureValue(e, "for key", "foo");
   }
 
+  @Test
+  public void factValueFailNull() {
+    AssertionError e =
+        expectFailure(whenTesting -> whenTesting.that(failure(simpleFact("foo"))).factValue(null));
+    assertFailureKeys(
+        e,
+        "could not perform fact-value check because requested key is null",
+        "for assertion about Truth failure");
+  }
+
   // factValue(String, int)
 
   @Test
@@ -130,9 +139,15 @@ public void factValueIntMultipleKeys() {
 
   @Test
   public void factValueIntFailNegative() {
-    assertThrows(
-        IllegalArgumentException.class,
-        () -> assertThat(fact("foo", "the foo")).factValue("foo", -1));
+    AssertionError e =
+        expectFailure(
+            whenTesting -> whenTesting.that(failure(fact("foo", "the foo"))).factValue("foo", -1));
+    assertFailureKeys(
+        e,
+        "could not perform fact-value check because requested index is null",
+        "requested key",
+        "for assertion about Truth failure");
+    assertFailureValue(e, "requested key", "foo");
   }
 
   @Test
