Skip to content

Commit

Permalink
Fix/suppress a few nullness mismatches.
Browse files Browse the repository at this point in the history
We could still stand to give actual thought to the `util.concurrent` mismatches:
- #3566
- #3568

Those two mismatches will be detected when we begin using a checker that contains [an annotated copy of `ThreadFactory`](jspecify/jdk@24191c6).

The mismatch in `MutableClassToInstanceMap` is currently not detected. That's a bug. But for some reason, it _is_ detected when we use type-use anntotations.

(I included _additional_ edits to `MutableClassToInstanceMap` and `ImmutableClassToInstanceMap`—specifically, in their `cast` methods. Those changes aren't necessary to the main change here. I had just started to change them to be consistent with the principle we'd discussed in cl/526184065, which is to use a non-null bound for a type parameter if all its usages would otherwise be projected. And then I realized that the second type parameter was serving no purpose, so I simplified further.)

RELNOTES=n/a
PiperOrigin-RevId: 531012514
  • Loading branch information
cpovirk authored and Google Java Core Libraries committed May 10, 2023
1 parent 667a9d4 commit cba0b0a
Show file tree
Hide file tree
Showing 8 changed files with 22 additions and 10 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ public <T extends B> Builder<B> putAll(Map<? extends Class<? extends T>, ? exten
return this;
}

private static <B, T extends B> T cast(Class<T> type, B value) {
private static <T> T cast(Class<T> type, Object value) {
return Primitives.wrap(type).cast(value);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,8 @@ private MutableClassToInstanceMap(Map<Class<? extends @NonNull B>, B> delegate)
@Override
@ParametricNullness
public B setValue(@ParametricNullness B value) {
return super.setValue(cast(getKey(), value));
cast(getKey(), value);
return super.setValue(value);
}
};
}
Expand Down Expand Up @@ -148,7 +149,8 @@ public Object[] toArray() {
@CanIgnoreReturnValue
@CheckForNull
public B put(Class<? extends @NonNull B> key, @ParametricNullness B value) {
return super.put(key, cast(key, value));
cast(key, value);
return super.put(key, value);
}

@Override
Expand All @@ -175,7 +177,7 @@ public <T extends B> T putInstance(Class<@NonNull T> type, @ParametricNullness T

@CanIgnoreReturnValue
@CheckForNull
private static <B, T extends B> T cast(Class<@NonNull T> type, @CheckForNull B value) {
private static <T> T cast(Class<T> type, @CheckForNull Object value) {
return Primitives.wrap(type).cast(value);
}

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

import static com.google.common.base.Preconditions.checkArgument;
import static com.google.common.base.Preconditions.checkNotNull;
import static java.util.Objects.requireNonNull;

import com.google.common.annotations.Beta;
import com.google.common.annotations.GwtCompatible;
Expand Down Expand Up @@ -891,7 +892,8 @@ private static boolean isAppEngineWithApiClasses() {
static Thread newThread(String name, Runnable runnable) {
checkNotNull(name);
checkNotNull(runnable);
Thread result = platformThreadFactory().newThread(runnable);
// TODO(b/139726489): Confirm that null is impossible here.
Thread result = requireNonNull(platformThreadFactory().newThread(runnable));
try {
result.setName(name);
} catch (SecurityException e) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -171,6 +171,8 @@ private static ThreadFactory doBuild(ThreadFactoryBuilder builder) {
@Override
public Thread newThread(Runnable runnable) {
Thread thread = backingThreadFactory.newThread(runnable);
// TODO(b/139735208): Figure out what to do when the factory returns null.
requireNonNull(thread);
if (nameFormat != null) {
// requireNonNull is safe because we create `count` if (and only if) we have a nameFormat.
thread.setName(format(nameFormat, requireNonNull(count).getAndIncrement()));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ public <T extends B> Builder<B> putAll(Map<? extends Class<? extends T>, ? exten
return this;
}

private static <B, T extends B> T cast(Class<T> type, B value) {
private static <T> T cast(Class<T> type, Object value) {
return Primitives.wrap(type).cast(value);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,8 @@ private MutableClassToInstanceMap(Map<Class<? extends @NonNull B>, B> delegate)
@Override
@ParametricNullness
public B setValue(@ParametricNullness B value) {
return super.setValue(cast(getKey(), value));
cast(getKey(), value);
return super.setValue(value);
}
};
}
Expand Down Expand Up @@ -155,7 +156,8 @@ public Object[] toArray() {
@CanIgnoreReturnValue
@CheckForNull
public B put(Class<? extends @NonNull B> key, @ParametricNullness B value) {
return super.put(key, cast(key, value));
cast(key, value);
return super.put(key, value);
}

@Override
Expand All @@ -182,7 +184,7 @@ public <T extends B> T putInstance(Class<@NonNull T> type, @ParametricNullness T

@CanIgnoreReturnValue
@CheckForNull
private static <B, T extends B> T cast(Class<@NonNull T> type, @CheckForNull B value) {
private static <T> T cast(Class<T> type, @CheckForNull Object value) {
return Primitives.wrap(type).cast(value);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
import static com.google.common.base.Preconditions.checkArgument;
import static com.google.common.base.Preconditions.checkNotNull;
import static com.google.common.util.concurrent.Internal.toNanosSaturated;
import static java.util.Objects.requireNonNull;

import com.google.common.annotations.Beta;
import com.google.common.annotations.GwtCompatible;
Expand Down Expand Up @@ -972,7 +973,8 @@ private static boolean isAppEngineWithApiClasses() {
static Thread newThread(String name, Runnable runnable) {
checkNotNull(name);
checkNotNull(runnable);
Thread result = platformThreadFactory().newThread(runnable);
// TODO(b/139726489): Confirm that null is impossible here.
Thread result = requireNonNull(platformThreadFactory().newThread(runnable));
try {
result.setName(name);
} catch (SecurityException e) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -171,6 +171,8 @@ private static ThreadFactory doBuild(ThreadFactoryBuilder builder) {
@Override
public Thread newThread(Runnable runnable) {
Thread thread = backingThreadFactory.newThread(runnable);
// TODO(b/139735208): Figure out what to do when the factory returns null.
requireNonNull(thread);
if (nameFormat != null) {
// requireNonNull is safe because we create `count` if (and only if) we have a nameFormat.
thread.setName(format(nameFormat, requireNonNull(count).getAndIncrement()));
Expand Down

0 comments on commit cba0b0a

Please sign in to comment.