Skip to content

Commit

Permalink
Annotate the rest of the main package (basically just the Java 8 subj…
Browse files Browse the repository at this point in the history
…ects) for nullness.

And move the `@NullMarked` annotation from individual classes up to the package.

Motivation:
- The annotating is about potentially making things nicer for callers (or any future J2KT use?).
- Moving `@NullMarked` to the package is about saving Truth users from [a warning when running Error Prone with `--release 8`](#1320): When `@NullMarked` appears on a _class_, Error Prone processes the entire `NullMarked` class in order to check whether `NullMarked` is `@Inherited`. This leads to `warning: unknown enum constant ElementType.MODULE` because `NullMarked` has `@Target(MODULE, ...)` but `MODULE` isn't available until Java 9. We'll also be fixing this on the Error Prone side, but we might as well work around it on the Truth side, too—and annotate the rest of Truth while we're at it.

(In principle, I should now add `@NullUnmarked` to all of Truth's test classes, since they haven't been annotated. But doing so would have little to no effect in practice unless maybe IntelliJ recognizes `@NullUnmarked` (probably not now?) or we improve our nullness offerings in Error Prone. I'm planning to not bother, but let me know if I'm being overly lazy.)

Note that this CL applies `@Nullable` to some assertion methods' parameters even though those assertions would always fail if the callers passed `null`. This follows a principle that we'd applied (albeit incompletely) in cl/516515683, which showed that such changes avoided producing build errors in existing, working code. The principle is the same as that discussed for `EqualsTester` in cl/578260904.

Fixes #1320

RELNOTES=Annotated the rest of the main package for nullness, and moved the `@NullMarked` annotation from individual classes up to the package to avoid [a warning under `--release 8`](#1320).
PiperOrigin-RevId: 651780766
  • Loading branch information
cpovirk authored and Google Java Core Libraries committed Jul 12, 2024
1 parent 8ac91a6 commit 400ad8c
Show file tree
Hide file tree
Showing 62 changed files with 58 additions and 133 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -20,15 +20,13 @@
import static com.google.common.truth.Fact.simpleFact;

import java.lang.reflect.Array;
import org.jspecify.annotations.NullMarked;
import org.jspecify.annotations.Nullable;

/**
* A common supertype for Array subjects, abstracting some common display and error infrastructure.
*
* @author Christian Gruber ([email protected])
*/
@NullMarked
abstract class AbstractArraySubject extends Subject {
private final @Nullable Object actual;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@
import java.io.InputStream;
import java.util.ArrayList;
import java.util.Map.Entry;
import org.jspecify.annotations.NullMarked;
import org.jspecify.annotations.Nullable;
import org.objectweb.asm.ClassReader;
import org.objectweb.asm.ClassVisitor;
Expand Down Expand Up @@ -70,7 +69,6 @@
*/
@GwtIncompatible
@J2ktIncompatible
@NullMarked
final class ActualValueInference {
/** <b>Call {@link Platform#inferDescription} rather than calling this directly.</b> */
static @Nullable String describeActualValue(String className, String methodName, int lineNumber) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,15 +19,13 @@
import static com.google.common.truth.Fact.makeMessage;

import com.google.common.collect.ImmutableList;
import org.jspecify.annotations.NullMarked;
import org.jspecify.annotations.Nullable;

/**
* An {@link AssertionError} composed of structured {@link Fact} instances and other string
* messages.
*/
@SuppressWarnings("OverrideThrowableToString") // We intentionally hide the class name.
@NullMarked
final class AssertionErrorWithFacts extends AssertionError implements ErrorWithFacts {
private final ImmutableList<Fact> facts;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,15 +20,13 @@
import static com.google.common.truth.Fact.simpleFact;

import java.math.BigDecimal;
import org.jspecify.annotations.NullMarked;
import org.jspecify.annotations.Nullable;

/**
* Propositions for {@link BigDecimal} typed subjects.
*
* @author Kurt Alfred Kluever
*/
@NullMarked
public final class BigDecimalSubject extends ComparableSubject<BigDecimal> {
private final @Nullable BigDecimal actual;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,15 +17,13 @@

import static com.google.common.truth.Fact.simpleFact;

import org.jspecify.annotations.NullMarked;
import org.jspecify.annotations.Nullable;

/**
* Propositions for boolean subjects.
*
* @author Christian Gruber ([email protected])
*/
@NullMarked
public final class BooleanSubject extends Subject {
private final @Nullable Boolean actual;

Expand Down
2 changes: 0 additions & 2 deletions core/src/main/java/com/google/common/truth/ClassSubject.java
Original file line number Diff line number Diff line change
Expand Up @@ -18,15 +18,13 @@
import static com.google.common.base.Preconditions.checkNotNull;

import com.google.common.annotations.GwtIncompatible;
import org.jspecify.annotations.NullMarked;
import org.jspecify.annotations.Nullable;

/**
* Propositions for {@link Class} subjects.
*
* @author Kurt Alfred Kluever
*/
@NullMarked
@GwtIncompatible("reflection")
@J2ktIncompatible
public final class ClassSubject extends Subject {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@
import static com.google.common.base.Preconditions.checkNotNull;

import com.google.common.collect.Range;
import org.jspecify.annotations.NullMarked;
import org.jspecify.annotations.Nullable;

/**
Expand All @@ -27,7 +26,6 @@
* @author Kurt Alfred Kluever
* @param <T> the type of the object being tested by this {@code ComparableSubject}
*/
@NullMarked
// TODO(b/136040841): Consider further tightening this to the proper `extends Comparable<? super T>`
public abstract class ComparableSubject<T extends Comparable<?>> extends Subject {
/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,14 +21,12 @@

import com.google.common.collect.ImmutableList;
import com.google.common.truth.Platform.PlatformComparisonFailure;
import org.jspecify.annotations.NullMarked;
import org.jspecify.annotations.Nullable;

/**
* An {@link AssertionError} (usually a JUnit {@code ComparisonFailure}, but not under GWT) composed
* of structured {@link Fact} instances and other string messages.
*/
@NullMarked
final class ComparisonFailureWithFacts extends PlatformComparisonFailure implements ErrorWithFacts {
private final ImmutableList<Fact> facts;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@

import com.google.common.annotations.VisibleForTesting;
import com.google.common.collect.ImmutableList;
import org.jspecify.annotations.NullMarked;
import org.jspecify.annotations.Nullable;

/**
Expand All @@ -43,7 +42,6 @@
* different implementation under GWT/j2cl, where {@code ComparisonFailure} is also unavailable but
* we can't just recover from that at runtime.
*/
@NullMarked
final class ComparisonFailures {
static ImmutableList<Fact> makeComparisonFailureFacts(
ImmutableList<Fact> headFacts,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@
import com.google.common.collect.ImmutableList;
import java.util.Arrays;
import java.util.List;
import org.jspecify.annotations.NullMarked;
import org.jspecify.annotations.Nullable;

/**
Expand Down Expand Up @@ -66,7 +65,6 @@
*
* @author Pete Gillin
*/
@NullMarked
public abstract class Correspondence<A extends @Nullable Object, E extends @Nullable Object> {

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,6 @@

import static com.google.common.base.Preconditions.checkNotNull;

import org.jspecify.annotations.NullMarked;

/**
* In a fluent assertion chain, exposes one or more "custom" {@code that} methods, which accept a
* value under test and return a {@link Subject}.
Expand All @@ -36,7 +34,6 @@
* extensions</a>. It explains the cases in which {@code CustomSubjectBuilder} is necessary, and it
* links to further instructions.
*/
@NullMarked
public abstract class CustomSubjectBuilder {
/**
* In a fluent assertion chain, the argument to the "custom" overload of {@link
Expand Down
2 changes: 0 additions & 2 deletions core/src/main/java/com/google/common/truth/DiffUtils.java
Original file line number Diff line number Diff line change
Expand Up @@ -23,15 +23,13 @@
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import org.jspecify.annotations.NullMarked;

/**
* A custom implementation of the diff algorithm based on the solution described at
* https://en.wikipedia.org/wiki/Longest_common_subsequence_problem
*
* @author Yun Peng ([email protected])
*/
@NullMarked
final class DiffUtils {
// A list of unique strings appeared in compared texts.
// The index of each string is its incremental Id.
Expand Down
2 changes: 0 additions & 2 deletions core/src/main/java/com/google/common/truth/DoubleSubject.java
Original file line number Diff line number Diff line change
Expand Up @@ -26,15 +26,13 @@
import static java.lang.Double.NaN;
import static java.lang.Double.doubleToLongBits;

import org.jspecify.annotations.NullMarked;
import org.jspecify.annotations.Nullable;

/**
* Propositions for {@link Double} subjects.
*
* @author Kurt Alfred Kluever
*/
@NullMarked
public final class DoubleSubject extends ComparableSubject<Double> {
private static final long NEG_ZERO_BITS = doubleToLongBits(-0.0);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,13 +17,11 @@
package com.google.common.truth;

import com.google.common.collect.ImmutableList;
import org.jspecify.annotations.NullMarked;

/**
* Supertype of Truth's {@link AssertionError} subclasses that are created from a list of {@link
* Fact} instances.
*/
@NullMarked
interface ErrorWithFacts {
ImmutableList<Fact> facts();
}
2 changes: 0 additions & 2 deletions core/src/main/java/com/google/common/truth/Expect.java
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@
import com.google.errorprone.annotations.concurrent.GuardedBy;
import java.util.ArrayList;
import java.util.List;
import org.jspecify.annotations.NullMarked;
import org.jspecify.annotations.Nullable;
import org.junit.internal.AssumptionViolatedException;
import org.junit.rules.ErrorCollector;
Expand Down Expand Up @@ -84,7 +83,6 @@
*/
@GwtIncompatible("JUnit4")
@J2ktIncompatible
@NullMarked
public final class Expect extends StandardSubjectBuilder implements TestRule {

private static final class ExpectationGatherer implements FailureStrategy {
Expand Down
2 changes: 0 additions & 2 deletions core/src/main/java/com/google/common/truth/ExpectFailure.java
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@
import com.google.common.annotations.GwtIncompatible;
import com.google.common.truth.Truth.SimpleAssertionError;
import com.google.errorprone.annotations.CanIgnoreReturnValue;
import org.jspecify.annotations.NullMarked;
import org.jspecify.annotations.Nullable;
import org.junit.runner.Description;
import org.junit.runners.model.Statement;
Expand Down Expand Up @@ -67,7 +66,6 @@
* also checks that the assertion you're testing uses the supplied {@link FailureStrategy} and calls
* {@link FailureStrategy#fail} only once.
*/
@NullMarked
public final class ExpectFailure implements Platform.JUnitTestRule {
private boolean inRuleContext = false;
private boolean failureExpected = false;
Expand Down
2 changes: 0 additions & 2 deletions core/src/main/java/com/google/common/truth/Fact.java
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@

import com.google.common.collect.ImmutableList;
import java.io.Serializable;
import org.jspecify.annotations.NullMarked;
import org.jspecify.annotations.Nullable;

/**
Expand All @@ -35,7 +34,6 @@
* <p>If you are writing a custom {@code Subject}, see <a
* href="https://truth.dev/failure_messages">our tips on writing failure messages</a>.
*/
@NullMarked
public final class Fact implements Serializable {
/**
* Creates a fact with the given key and value, which will be printed in a format like "key:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@

import com.google.common.base.Function;
import com.google.common.collect.ImmutableList;
import org.jspecify.annotations.NullMarked;
import org.jspecify.annotations.Nullable;

/**
Expand All @@ -52,7 +51,6 @@
* using their {@link CustomSubjectBuilder#metadata()} method to get an instance to pass to the
* constructor.)
*/
@NullMarked
public final class FailureMetadata {
static FailureMetadata forFailureStrategy(FailureStrategy failureStrategy) {
return new FailureMetadata(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
*/
package com.google.common.truth;

import org.jspecify.annotations.NullMarked;

/**
* Defines what to do when a check fails.
Expand Down Expand Up @@ -54,7 +53,6 @@
* StandardSubjectBuilder#forCustomFailureStrategy
* StandardSubjectBuilder.forCustomFailureStrategy(STRATEGY)}.
*/
@NullMarked
public interface FailureStrategy {
/**
* Handles a failure. The parameter is an {@code AssertionError} or subclass thereof, and it
Expand Down
2 changes: 0 additions & 2 deletions core/src/main/java/com/google/common/truth/FloatSubject.java
Original file line number Diff line number Diff line change
Expand Up @@ -26,15 +26,13 @@
import static java.lang.Float.NaN;
import static java.lang.Float.floatToIntBits;

import org.jspecify.annotations.NullMarked;
import org.jspecify.annotations.Nullable;

/**
* Propositions for {@link Float} subjects.
*
* @author Kurt Alfred Kluever
*/
@NullMarked
public final class FloatSubject extends ComparableSubject<Float> {
private static final int NEG_ZERO_BITS = floatToIntBits(-0.0f);

Expand Down
2 changes: 0 additions & 2 deletions core/src/main/java/com/google/common/truth/GraphMatching.java
Original file line number Diff line number Diff line change
Expand Up @@ -27,15 +27,13 @@
import java.util.HashMap;
import java.util.Map;
import java.util.Queue;
import org.jspecify.annotations.NullMarked;

/**
* Helper routines related to <a href="https://en.wikipedia.org/wiki/Matching_(graph_theory)">graph
* matchings</a>.
*
* @author Pete Gillin
*/
@NullMarked
final class GraphMatching {

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@
import static com.google.common.truth.Fact.simpleFact;

import com.google.common.base.Optional;
import org.jspecify.annotations.NullMarked;
import org.jspecify.annotations.Nullable;

/**
Expand All @@ -29,7 +28,6 @@
*
* @author Christian Gruber
*/
@NullMarked
public final class GuavaOptionalSubject extends Subject {
@SuppressWarnings("NullableOptional") // Truth always accepts nulls, no matter the type
private final @Nullable Optional<?> actual;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,9 @@
import static java.lang.annotation.ElementType.TYPE;

import java.lang.annotation.Target;
import org.jspecify.annotations.NullMarked;

/**
* Disables Animal Sniffer's checking of compatibility with older versions of Java/Android.
*/
@Target({METHOD, CONSTRUCTOR, TYPE})
@NullMarked
@interface IgnoreJRERequirement {}
Loading

0 comments on commit 400ad8c

Please sign in to comment.