Skip to content

Commit

Permalink
Annotate the parameter of List.sort as @Nullable.
Browse files Browse the repository at this point in the history
This has always struck me as a weird feature, but people use it, and it
"works" in the sense of "does not cause NPE" (though it may cause
CCE...). I see both calls to `sort(null)` and calls like
`sort(priorityQueue.comparator())` (which _might_ be `null`;
[example](https://github.com/google/guava/blob/e82e2a2c07c68108f318958ee0355cc835c97743/guava-testlib/src/com/google/common/collect/testing/testers/SortedSetNavigationTester.java#L57)).
And while I prefer a world in which methods like `comparator()` never
return `null`, as we arranged for [in
`SortedMultiset`](https://guava.dev/SortedMultiset#comparator()), there
are apparently [downsides](google/guava#6187)
to using a `Comparator` that implements natural order rather than using
`null`.

Is any of that convincing? :) This is definitely a case in which I can
see how eisop would want to stay on its own path. My main motivation for
doing this now is that I hear that the current signature is causing us
trouble during Java->Kotlin transpilation. I can get more details.
  • Loading branch information
cpovirk committed Jun 7, 2023
1 parent f9395fb commit 37ee6dc
Showing 1 changed file with 1 addition and 1 deletion.
2 changes: 1 addition & 1 deletion src/java.base/share/classes/java/util/List.java
Original file line number Diff line number Diff line change
Expand Up @@ -514,7 +514,7 @@ default void replaceAll(UnaryOperator<E> operator) {
* @since 1.8
*/
@SuppressWarnings({"unchecked", "rawtypes"})
default void sort(Comparator<? super E> c) {
default void sort(@Nullable Comparator<? super E> c) {
Object[] a = this.toArray();
Arrays.sort(a, (Comparator) c);
ListIterator<E> i = this.listIterator();
Expand Down

0 comments on commit 37ee6dc

Please sign in to comment.