From 0b3243391ece609d958c8cb5f896f6b4aabb63f7 Mon Sep 17 00:00:00 2001 From: cpovirk Date: Tue, 11 Jan 2022 14:42:02 -0800 Subject: [PATCH] Assorted improvements to nullness annotations. I don't think any of the Guava changes are user-visible. Details: - Many of these fix bugs that will be noticed by the forthcoming version of our hacky internal nullness checker. - Some of these fix bugs that even the forthcoming version won't notice but that I happened to see. - Some changes add `@Nullable` to return types of methods that always throw an exception. There's no strict need for this, but we've mostly done it otherwise, so I figured I'd be consistent (and quiet `ReturnMissingNullable`, at least until I quiet it for all such methods with https://github.com/google/error-prone/issues/2910). - The `NullnessCasts` change is to discourage `ReturnMissingNullable` from adding a `@Nullable` annotation where we don't want it. (But we'll probably never run `ReturnMissingNullable` in the "aggressive" mode over this code, anyway, so there's not likely to be a need for the suppression.) - The `@ParametricNulless` changes evidently aren't necessary for anyone right now, but they could theoretically be necessary for j2objc users until j2objc further enhances its support for nullness annotations. - The `AbstractFuture` change removes a suppression that would be necessary under the Checker Framework (which would consider the supermethod's return type to be non-nullable) but isn't necessary for us (because we consider the supermethod's return type to have unspecified nullness). RELNOTES=n/a PiperOrigin-RevId: 421129392 --- .../com/google/common/base/Equivalence.java | 38 ++++++++++++++++++- .../common/base/FunctionalEquivalence.java | 5 ++- .../google/common/collect/Collections2.java | 3 +- .../google/common/collect/ImmutableMap.java | 4 +- .../common/collect/ImmutableMultiset.java | 3 +- .../src/com/google/common/collect/Maps.java | 15 +++++++- .../google/common/collect/NullnessCasts.java | 2 +- .../collect/RegularImmutableSortedSet.java | 2 +- .../google/common/collect/TopKSelector.java | 8 +++- .../util/concurrent/AbstractFuture.java | 1 - .../common/util/concurrent/NullnessCasts.java | 3 +- .../com/google/common/base/Equivalence.java | 38 ++++++++++++++++++- .../common/base/FunctionalEquivalence.java | 5 ++- .../com/google/common/cache/LocalCache.java | 6 +-- .../com/google/common/collect/HashBiMap.java | 12 +++++- .../common/collect/ImmutableMultiset.java | 2 +- guava/src/com/google/common/collect/Maps.java | 25 ++++++++++-- .../google/common/collect/NullnessCasts.java | 2 +- .../collect/RegularImmutableSortedSet.java | 3 +- .../google/common/collect/Synchronized.java | 3 +- .../google/common/collect/TopKSelector.java | 8 +++- .../google/common/collect/TreeRangeMap.java | 3 +- .../util/concurrent/AbstractFuture.java | 1 - .../common/util/concurrent/NullnessCasts.java | 3 +- 24 files changed, 162 insertions(+), 33 deletions(-) diff --git a/android/guava/src/com/google/common/base/Equivalence.java b/android/guava/src/com/google/common/base/Equivalence.java index 0ce901feb290..1bf7f98998f2 100644 --- a/android/guava/src/com/google/common/base/Equivalence.java +++ b/android/guava/src/com/google/common/base/Equivalence.java @@ -153,7 +153,43 @@ public final Equivalence onResultOf(Function Wrapper wrap(@ParametricNullness S reference) { - return new Wrapper(this, reference); + /* + * I'm pretty sure that this warning "makes sense" but doesn't indicate a real problem. + * + * Why it "makes sense": If we pass a `@Nullable Foo`, then we should also pass an + * `Equivalence`. And there's no such thing because Equivalence doesn't + * permit nullable type arguments. + * + * Why there's no real problem: Every Equivalence can handle null. + * + * We could work around this by giving Wrapper 2 type parameters. In the terms of this method, + * that would be both the T parameter (from the class) and the S parameter (from this method). + * However, such a change would be source-incompatible. (Plus, there's no reason for the S + * parameter from the user's perspective, so it would be a wart.) + * + * We could probably also work around this by making Wrapper non-final and putting the + * implementation into a subclass with those 2 type parameters. But we like `final`, if only to + * deter users from using mocking frameworks to construct instances. (And could also complicate + * serialization, which is discussed more in the next paragraph.) + * + * We could probably also work around this by having Wrapper accept an instance of a new + * WrapperGuts class, which would then be the class that would declare the 2 type parameters. + * But that would break deserialization of previously serialized Wrapper instances. And while we + * specifically say not to rely on serialization across Guava versions, users sometimes do. So + * we'd rather not break them without a good enough reason. + * + * (We could work around the serialization problem by writing custom serialization code. But + * even that helps only the case of serializing with an old version and deserializing with a + * new, not vice versa -- unless we introduce WrapperGuts and the logic to handle it today, wait + * until "everyone" has picked up a version of Guava with that code, and *then* change to use + * WrapperGuts.) + * + * Anyway, a suppression isn't really a big deal. But I have tried to do some due diligence on + * avoiding it :) + */ + @SuppressWarnings("nullness") + Wrapper w = new Wrapper<>(this, reference); + return w; } /** diff --git a/android/guava/src/com/google/common/base/FunctionalEquivalence.java b/android/guava/src/com/google/common/base/FunctionalEquivalence.java index 8bb67be2885f..4383f4f36c1a 100644 --- a/android/guava/src/com/google/common/base/FunctionalEquivalence.java +++ b/android/guava/src/com/google/common/base/FunctionalEquivalence.java @@ -20,6 +20,7 @@ import com.google.common.annotations.GwtCompatible; import java.io.Serializable; import javax.annotation.CheckForNull; +import org.checkerframework.checker.nullness.qual.Nullable; /** * Equivalence applied on functional result. @@ -34,11 +35,11 @@ final class FunctionalEquivalence extends Equivalence implements Serial private static final long serialVersionUID = 0; - private final Function function; + private final Function function; private final Equivalence resultEquivalence; FunctionalEquivalence( - Function function, Equivalence resultEquivalence) { + Function function, Equivalence resultEquivalence) { this.function = checkNotNull(function); this.resultEquivalence = checkNotNull(resultEquivalence); } diff --git a/android/guava/src/com/google/common/collect/Collections2.java b/android/guava/src/com/google/common/collect/Collections2.java index 1e651b50a0f9..8d799d5932b8 100644 --- a/android/guava/src/com/google/common/collect/Collections2.java +++ b/android/guava/src/com/google/common/collect/Collections2.java @@ -695,7 +695,8 @@ private static boolean isPermutation(List first, List second) { return true; } - private static ObjectCountHashMap counts(Collection collection) { + private static ObjectCountHashMap counts( + Collection collection) { ObjectCountHashMap map = new ObjectCountHashMap<>(); for (E e : collection) { map.put(e, map.get(e) + 1); diff --git a/android/guava/src/com/google/common/collect/ImmutableMap.java b/android/guava/src/com/google/common/collect/ImmutableMap.java index fe384c9e6fc2..9195115d7870 100644 --- a/android/guava/src/com/google/common/collect/ImmutableMap.java +++ b/android/guava/src/com/google/common/collect/ImmutableMap.java @@ -599,7 +599,9 @@ public ImmutableMap buildKeepingLast() { } static void sortEntries( - @Nullable Object[] alternatingKeysAndValues, int size, Comparator valueComparator) { + @Nullable Object[] alternatingKeysAndValues, + int size, + Comparator valueComparator) { @SuppressWarnings({"rawtypes", "unchecked"}) Entry[] entries = new Entry[size]; for (int i = 0; i < size; i++) { diff --git a/android/guava/src/com/google/common/collect/ImmutableMultiset.java b/android/guava/src/com/google/common/collect/ImmutableMultiset.java index 6d15ab8c6057..ab281d4b3b3f 100644 --- a/android/guava/src/com/google/common/collect/ImmutableMultiset.java +++ b/android/guava/src/com/google/common/collect/ImmutableMultiset.java @@ -31,6 +31,7 @@ import java.util.Iterator; import java.util.Set; import javax.annotation.CheckForNull; +import org.checkerframework.checker.nullness.qual.Nullable; /** * A {@link Multiset} whose contents will never change, with many other important properties @@ -282,7 +283,7 @@ public final boolean setCount(E element, int oldCount, int newCount) { @GwtIncompatible // not present in emulated superclass @Override - int copyIntoArray(Object[] dst, int offset) { + int copyIntoArray(@Nullable Object[] dst, int offset) { for (Multiset.Entry entry : entrySet()) { Arrays.fill(dst, offset, offset + entry.getCount(), entry.getElement()); offset += entry.getCount(); diff --git a/android/guava/src/com/google/common/collect/Maps.java b/android/guava/src/com/google/common/collect/Maps.java index a8db83df5849..76e69abb79c0 100644 --- a/android/guava/src/com/google/common/collect/Maps.java +++ b/android/guava/src/com/google/common/collect/Maps.java @@ -528,7 +528,20 @@ SortedMapDifference difference( onlyOnRight.putAll(right); // will whittle it down SortedMap onBoth = Maps.newTreeMap(comparator); SortedMap> differences = Maps.newTreeMap(comparator); - doDifference(left, right, Equivalence.equals(), onlyOnLeft, onlyOnRight, onBoth, differences); + + /* + * V is a possibly nullable type, but we decided to declare Equivalence with a type parameter + * that is restricted to non-nullable types. Still, this code is safe: We made that decision + * about Equivalence not because Equivalence is null-hostile but because *every* Equivalence can + * handle null inputs -- and thus it would be meaningless for the type system to distinguish + * between "an Equivalence for nullable Foo" and "an Equivalence for non-nullable Foo." + * + * (And the unchecked cast is safe because Equivalence is contravariant.) + */ + @SuppressWarnings({"nullness", "unchecked"}) + Equivalence equalsEquivalence = (Equivalence) Equivalence.equals(); + + doDifference(left, right, equalsEquivalence, onlyOnLeft, onlyOnRight, onBoth, differences); return new SortedMapDifferenceImpl<>(onlyOnLeft, onlyOnRight, onBoth, differences); } diff --git a/android/guava/src/com/google/common/collect/NullnessCasts.java b/android/guava/src/com/google/common/collect/NullnessCasts.java index 6ceeda75ffbc..4f894dbd3139 100644 --- a/android/guava/src/com/google/common/collect/NullnessCasts.java +++ b/android/guava/src/com/google/common/collect/NullnessCasts.java @@ -57,7 +57,7 @@ final class NullnessCasts { } /** Returns {@code null} as any type, even one that does not include {@code null}. */ - @SuppressWarnings({"nullness", "TypeParameterUnusedInFormals"}) + @SuppressWarnings({"nullness", "TypeParameterUnusedInFormals", "ReturnMissingNullable"}) // The warnings are legitimate. Each time we use this method, we document why. @ParametricNullness static T unsafeNull() { diff --git a/android/guava/src/com/google/common/collect/RegularImmutableSortedSet.java b/android/guava/src/com/google/common/collect/RegularImmutableSortedSet.java index dd987988b87d..ef0fbb37857e 100644 --- a/android/guava/src/com/google/common/collect/RegularImmutableSortedSet.java +++ b/android/guava/src/com/google/common/collect/RegularImmutableSortedSet.java @@ -155,7 +155,7 @@ boolean isPartialView() { } @Override - int copyIntoArray(Object[] dst, int offset) { + int copyIntoArray(@Nullable Object[] dst, int offset) { return elements.copyIntoArray(dst, offset); } diff --git a/android/guava/src/com/google/common/collect/TopKSelector.java b/android/guava/src/com/google/common/collect/TopKSelector.java index f8cca0deaff9..236daf3f2062 100644 --- a/android/guava/src/com/google/common/collect/TopKSelector.java +++ b/android/guava/src/com/google/common/collect/TopKSelector.java @@ -184,8 +184,10 @@ private void trim() { } iterations++; if (iterations >= maxIterations) { + @SuppressWarnings("nullness") // safe because we pass sort() a range that contains real Ts + T[] nonNullBuffer = (T[]) buffer; // We've already taken O(k log k), let's make sure we don't take longer than O(k log k). - Arrays.sort(buffer, left, right + 1, comparator); + Arrays.sort(nonNullBuffer, left, right + 1, comparator); break; } } @@ -263,7 +265,9 @@ public void offerAll(Iterator elements) { * this {@code TopKSelector}. This method returns in O(k log k) time. */ public List topK() { - Arrays.sort(buffer, 0, bufferSize, comparator); + @SuppressWarnings("nullness") // safe because we pass sort() a range that contains real Ts + T[] nonNullBuffer = (T[]) buffer; + Arrays.sort(nonNullBuffer, 0, bufferSize, comparator); if (bufferSize > k) { Arrays.fill(buffer, k, buffer.length, null); bufferSize = k; diff --git a/android/guava/src/com/google/common/util/concurrent/AbstractFuture.java b/android/guava/src/com/google/common/util/concurrent/AbstractFuture.java index 40ddba8e3eee..e03df9094327 100644 --- a/android/guava/src/com/google/common/util/concurrent/AbstractFuture.java +++ b/android/guava/src/com/google/common/util/concurrent/AbstractFuture.java @@ -1084,7 +1084,6 @@ protected void afterDone() {} * method returns @Nullable, too. However, we're not sure if we want to make any changes to that * class, since it's in a separate artifact that we planned to release only a single version of. */ - @SuppressWarnings("nullness") @CheckForNull protected final Throwable tryInternalFastPathGetFailure() { if (this instanceof Trusted) { diff --git a/android/guava/src/com/google/common/util/concurrent/NullnessCasts.java b/android/guava/src/com/google/common/util/concurrent/NullnessCasts.java index 0a0d719ef20b..a3a914e8970a 100644 --- a/android/guava/src/com/google/common/util/concurrent/NullnessCasts.java +++ b/android/guava/src/com/google/common/util/concurrent/NullnessCasts.java @@ -65,7 +65,8 @@ final class NullnessCasts { * return to a caller, the code needs to a way to return {@code null} from a method that returns * "plain {@code T}." This API provides that. */ - @SuppressWarnings("nullness") + @SuppressWarnings({"nullness", "TypeParameterUnusedInFormals", "ReturnMissingNullable"}) + // The warnings are legitimate. Each time we use this method, we document why. @ParametricNullness static T uncheckedNull() { return null; diff --git a/guava/src/com/google/common/base/Equivalence.java b/guava/src/com/google/common/base/Equivalence.java index fbcd21237ee8..4e2e89886a2a 100644 --- a/guava/src/com/google/common/base/Equivalence.java +++ b/guava/src/com/google/common/base/Equivalence.java @@ -165,7 +165,43 @@ public final Equivalence onResultOf(Function Wrapper wrap(@ParametricNullness S reference) { - return new Wrapper(this, reference); + /* + * I'm pretty sure that this warning "makes sense" but doesn't indicate a real problem. + * + * Why it "makes sense": If we pass a `@Nullable Foo`, then we should also pass an + * `Equivalence`. And there's no such thing because Equivalence doesn't + * permit nullable type arguments. + * + * Why there's no real problem: Every Equivalence can handle null. + * + * We could work around this by giving Wrapper 2 type parameters. In the terms of this method, + * that would be both the T parameter (from the class) and the S parameter (from this method). + * However, such a change would be source-incompatible. (Plus, there's no reason for the S + * parameter from the user's perspective, so it would be a wart.) + * + * We could probably also work around this by making Wrapper non-final and putting the + * implementation into a subclass with those 2 type parameters. But we like `final`, if only to + * deter users from using mocking frameworks to construct instances. (And could also complicate + * serialization, which is discussed more in the next paragraph.) + * + * We could probably also work around this by having Wrapper accept an instance of a new + * WrapperGuts class, which would then be the class that would declare the 2 type parameters. + * But that would break deserialization of previously serialized Wrapper instances. And while we + * specifically say not to rely on serialization across Guava versions, users sometimes do. So + * we'd rather not break them without a good enough reason. + * + * (We could work around the serialization problem by writing custom serialization code. But + * even that helps only the case of serializing with an old version and deserializing with a + * new, not vice versa -- unless we introduce WrapperGuts and the logic to handle it today, wait + * until "everyone" has picked up a version of Guava with that code, and *then* change to use + * WrapperGuts.) + * + * Anyway, a suppression isn't really a big deal. But I have tried to do some due diligence on + * avoiding it :) + */ + @SuppressWarnings("nullness") + Wrapper w = new Wrapper<>(this, reference); + return w; } /** diff --git a/guava/src/com/google/common/base/FunctionalEquivalence.java b/guava/src/com/google/common/base/FunctionalEquivalence.java index 8bb67be2885f..4383f4f36c1a 100644 --- a/guava/src/com/google/common/base/FunctionalEquivalence.java +++ b/guava/src/com/google/common/base/FunctionalEquivalence.java @@ -20,6 +20,7 @@ import com.google.common.annotations.GwtCompatible; import java.io.Serializable; import javax.annotation.CheckForNull; +import org.checkerframework.checker.nullness.qual.Nullable; /** * Equivalence applied on functional result. @@ -34,11 +35,11 @@ final class FunctionalEquivalence extends Equivalence implements Serial private static final long serialVersionUID = 0; - private final Function function; + private final Function function; private final Equivalence resultEquivalence; FunctionalEquivalence( - Function function, Equivalence resultEquivalence) { + Function function, Equivalence resultEquivalence) { this.function = checkNotNull(function); this.resultEquivalence = checkNotNull(resultEquivalence); } diff --git a/guava/src/com/google/common/cache/LocalCache.java b/guava/src/com/google/common/cache/LocalCache.java index 5b5312136f67..f2cbee3dbfdb 100644 --- a/guava/src/com/google/common/cache/LocalCache.java +++ b/guava/src/com/google/common/cache/LocalCache.java @@ -2189,7 +2189,7 @@ V waitForLoadingValue(ReferenceEntry e, K key, ValueReference valueR } } - V compute(K key, int hash, BiFunction function) { + V compute(K key, int hash, BiFunction function) { ReferenceEntry e; ValueReference valueReference = null; ComputingValueReference computingValueReference = null; @@ -3558,7 +3558,7 @@ public V apply(V newValue) { } } - public V compute(K key, BiFunction function) { + public V compute(K key, BiFunction function) { stopwatch.start(); V previousValue; try { @@ -4205,7 +4205,7 @@ public V putIfAbsent(K key, V value) { } @Override - public V compute(K key, BiFunction function) { + public V compute(K key, BiFunction function) { checkNotNull(key); checkNotNull(function); int hash = hash(key); diff --git a/guava/src/com/google/common/collect/HashBiMap.java b/guava/src/com/google/common/collect/HashBiMap.java index 150e207ce5a8..b0a5dd50edfd 100644 --- a/guava/src/com/google/common/collect/HashBiMap.java +++ b/guava/src/com/google/common/collect/HashBiMap.java @@ -489,6 +489,7 @@ private final class KeySet extends Maps.KeySet { public Iterator iterator() { return new Itr() { @Override + @ParametricNullness K output(BiEntry entry) { return entry.key; } @@ -530,17 +531,20 @@ class MapEntry extends AbstractMapEntry { } @Override + @ParametricNullness public K getKey() { return delegate.key; } @Override + @ParametricNullness public V getValue() { return delegate.value; } @Override - public V setValue(V value) { + @ParametricNullness + public V setValue(@ParametricNullness V value) { V oldValue = delegate.value; int valueHash = smearedHash(value); if (valueHash == delegate.valueHash && Objects.equal(value, oldValue)) { @@ -675,6 +679,7 @@ public boolean remove(@CheckForNull Object o) { public Iterator iterator() { return new Itr() { @Override + @ParametricNullness V output(BiEntry entry) { return entry.value; } @@ -703,17 +708,20 @@ class InverseEntry extends AbstractMapEntry { } @Override + @ParametricNullness public V getKey() { return delegate.value; } @Override + @ParametricNullness public K getValue() { return delegate.key; } @Override - public K setValue(K key) { + @ParametricNullness + public K setValue(@ParametricNullness K key) { K oldKey = delegate.key; int keyHash = smearedHash(key); if (keyHash == delegate.keyHash && Objects.equal(key, oldKey)) { diff --git a/guava/src/com/google/common/collect/ImmutableMultiset.java b/guava/src/com/google/common/collect/ImmutableMultiset.java index e7f4b184446e..b05ab3fe0108 100644 --- a/guava/src/com/google/common/collect/ImmutableMultiset.java +++ b/guava/src/com/google/common/collect/ImmutableMultiset.java @@ -325,7 +325,7 @@ public final boolean setCount(E element, int oldCount, int newCount) { @GwtIncompatible // not present in emulated superclass @Override - int copyIntoArray(Object[] dst, int offset) { + int copyIntoArray(@Nullable Object[] dst, int offset) { for (Multiset.Entry entry : entrySet()) { Arrays.fill(dst, offset, offset + entry.getCount(), entry.getElement()); offset += entry.getCount(); diff --git a/guava/src/com/google/common/collect/Maps.java b/guava/src/com/google/common/collect/Maps.java index d2860f2dbe19..6265f5dcee2f 100644 --- a/guava/src/com/google/common/collect/Maps.java +++ b/guava/src/com/google/common/collect/Maps.java @@ -575,7 +575,20 @@ SortedMapDifference difference( onlyOnRight.putAll(right); // will whittle it down SortedMap onBoth = Maps.newTreeMap(comparator); SortedMap> differences = Maps.newTreeMap(comparator); - doDifference(left, right, Equivalence.equals(), onlyOnLeft, onlyOnRight, onBoth, differences); + + /* + * V is a possibly nullable type, but we decided to declare Equivalence with a type parameter + * that is restricted to non-nullable types. Still, this code is safe: We made that decision + * about Equivalence not because Equivalence is null-hostile but because *every* Equivalence can + * handle null inputs -- and thus it would be meaningless for the type system to distinguish + * between "an Equivalence for nullable Foo" and "an Equivalence for non-nullable Foo." + * + * (And the unchecked cast is safe because Equivalence is contravariant.) + */ + @SuppressWarnings({"nullness", "unchecked"}) + Equivalence equalsEquivalence = (Equivalence) Equivalence.equals(); + + doDifference(left, right, equalsEquivalence, onlyOnLeft, onlyOnRight, onBoth, differences); return new SortedMapDifferenceImpl<>(onlyOnLeft, onlyOnRight, onBoth, differences); } @@ -1710,6 +1723,7 @@ public void replaceAll(BiFunction function) { } @Override + @CheckForNull public V putIfAbsent(K key, V value) { throw new UnsupportedOperationException(); } @@ -1725,6 +1739,7 @@ public boolean replace(K key, V oldValue, V newValue) { } @Override + @CheckForNull public V replace(K key, V value) { throw new UnsupportedOperationException(); } @@ -1742,7 +1757,8 @@ public V computeIfPresent( } @Override - public V compute(K key, BiFunction remappingFunction) { + public V compute( + K key, BiFunction remappingFunction) { throw new UnsupportedOperationException(); } @@ -3612,6 +3628,7 @@ public void replaceAll(BiFunction function) { } @Override + @CheckForNull public V putIfAbsent(K key, V value) { throw new UnsupportedOperationException(); } @@ -3627,6 +3644,7 @@ public boolean replace(K key, V oldValue, V newValue) { } @Override + @CheckForNull public V replace(K key, V value) { throw new UnsupportedOperationException(); } @@ -3644,7 +3662,8 @@ public V computeIfPresent( } @Override - public V compute(K key, BiFunction remappingFunction) { + public V compute( + K key, BiFunction remappingFunction) { throw new UnsupportedOperationException(); } diff --git a/guava/src/com/google/common/collect/NullnessCasts.java b/guava/src/com/google/common/collect/NullnessCasts.java index 6ceeda75ffbc..4f894dbd3139 100644 --- a/guava/src/com/google/common/collect/NullnessCasts.java +++ b/guava/src/com/google/common/collect/NullnessCasts.java @@ -57,7 +57,7 @@ final class NullnessCasts { } /** Returns {@code null} as any type, even one that does not include {@code null}. */ - @SuppressWarnings({"nullness", "TypeParameterUnusedInFormals"}) + @SuppressWarnings({"nullness", "TypeParameterUnusedInFormals", "ReturnMissingNullable"}) // The warnings are legitimate. Each time we use this method, we document why. @ParametricNullness static T unsafeNull() { diff --git a/guava/src/com/google/common/collect/RegularImmutableSortedSet.java b/guava/src/com/google/common/collect/RegularImmutableSortedSet.java index fcb683f56205..572e0acbf51f 100644 --- a/guava/src/com/google/common/collect/RegularImmutableSortedSet.java +++ b/guava/src/com/google/common/collect/RegularImmutableSortedSet.java @@ -29,6 +29,7 @@ import java.util.Spliterator; import java.util.function.Consumer; import javax.annotation.CheckForNull; +import org.checkerframework.checker.nullness.qual.Nullable; /** * An immutable sorted set with one or more elements. TODO(jlevy): Consider separate class for a @@ -164,7 +165,7 @@ boolean isPartialView() { } @Override - int copyIntoArray(Object[] dst, int offset) { + int copyIntoArray(@Nullable Object[] dst, int offset) { return elements.copyIntoArray(dst, offset); } diff --git a/guava/src/com/google/common/collect/Synchronized.java b/guava/src/com/google/common/collect/Synchronized.java index 25e28709dbba..29fe61a89b8a 100644 --- a/guava/src/com/google/common/collect/Synchronized.java +++ b/guava/src/com/google/common/collect/Synchronized.java @@ -1195,7 +1195,8 @@ public V computeIfPresent( } @Override - public V compute(K key, BiFunction remappingFunction) { + public V compute( + K key, BiFunction remappingFunction) { synchronized (mutex) { return delegate().compute(key, remappingFunction); } diff --git a/guava/src/com/google/common/collect/TopKSelector.java b/guava/src/com/google/common/collect/TopKSelector.java index 8411fcdea0bf..4bbb84c7c3c8 100644 --- a/guava/src/com/google/common/collect/TopKSelector.java +++ b/guava/src/com/google/common/collect/TopKSelector.java @@ -185,8 +185,10 @@ private void trim() { } iterations++; if (iterations >= maxIterations) { + @SuppressWarnings("nullness") // safe because we pass sort() a range that contains real Ts + T[] nonNullBuffer = (T[]) buffer; // We've already taken O(k log k), let's make sure we don't take longer than O(k log k). - Arrays.sort(buffer, left, right + 1, comparator); + Arrays.sort(nonNullBuffer, left, right + 1, comparator); break; } } @@ -271,7 +273,9 @@ public void offerAll(Iterator elements) { * this {@code TopKSelector}. This method returns in O(k log k) time. */ public List topK() { - Arrays.sort(buffer, 0, bufferSize, comparator); + @SuppressWarnings("nullness") // safe because we pass sort() a range that contains real Ts + T[] nonNullBuffer = (T[]) buffer; + Arrays.sort(nonNullBuffer, 0, bufferSize, comparator); if (bufferSize > k) { Arrays.fill(buffer, k, buffer.length, null); bufferSize = k; diff --git a/guava/src/com/google/common/collect/TreeRangeMap.java b/guava/src/com/google/common/collect/TreeRangeMap.java index a9200896b3d5..60e7bf3df874 100644 --- a/guava/src/com/google/common/collect/TreeRangeMap.java +++ b/guava/src/com/google/common/collect/TreeRangeMap.java @@ -436,7 +436,8 @@ public void remove(Range> range) { public void merge( Range> range, @CheckForNull Object value, - BiFunction remappingFunction) { + BiFunction + remappingFunction) { checkNotNull(range); throw new IllegalArgumentException( "Cannot merge range " + range + " into an empty subRangeMap"); diff --git a/guava/src/com/google/common/util/concurrent/AbstractFuture.java b/guava/src/com/google/common/util/concurrent/AbstractFuture.java index b1ec76f6ed13..b6758d207637 100644 --- a/guava/src/com/google/common/util/concurrent/AbstractFuture.java +++ b/guava/src/com/google/common/util/concurrent/AbstractFuture.java @@ -1084,7 +1084,6 @@ protected void afterDone() {} * method returns @Nullable, too. However, we're not sure if we want to make any changes to that * class, since it's in a separate artifact that we planned to release only a single version of. */ - @SuppressWarnings("nullness") @CheckForNull protected final Throwable tryInternalFastPathGetFailure() { if (this instanceof Trusted) { diff --git a/guava/src/com/google/common/util/concurrent/NullnessCasts.java b/guava/src/com/google/common/util/concurrent/NullnessCasts.java index 0a0d719ef20b..a3a914e8970a 100644 --- a/guava/src/com/google/common/util/concurrent/NullnessCasts.java +++ b/guava/src/com/google/common/util/concurrent/NullnessCasts.java @@ -65,7 +65,8 @@ final class NullnessCasts { * return to a caller, the code needs to a way to return {@code null} from a method that returns * "plain {@code T}." This API provides that. */ - @SuppressWarnings("nullness") + @SuppressWarnings({"nullness", "TypeParameterUnusedInFormals", "ReturnMissingNullable"}) + // The warnings are legitimate. Each time we use this method, we document why. @ParametricNullness static T uncheckedNull() { return null;