jspecify / jspecify/jdk

Consider `@NonNull` in `Map.computeIfAbsent`

Open
#102 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
Java
Stars
7
Forks
3
Avg merge
15h 17m
Merged PRs (30d)
8

Description

computeIfAbsent is annoying: The reason that almost everyone uses it is to insert a non-null value into the map if no value is yet present for that key. But very occasionally, people pass in a function that returns null, which is a perfectly valid thing to do according to the documentation and which is handled properly by implementations. But the natural result of that is that the return type of the method would @Nullable V, which is annoying for everyone else, requiring the kinds of suppressions and !!s that give people warnings blindness and give nullness checking a bad name.

Given that, https://github.com/jspecify/jdk/commit/a65745b7062d1e7969a51eb2e3821c81364ae7bc (later commented upon in https://github.com/jspecify/jdk/pull/32) annotated computeIfAbsent to require a function that returns plain V—and thus to have a return type of plain V[*]. This follows Kotlin's behavior [edit: discussed on YouTrack and again later] (and following Kotlin's behavior may be important for some users for pragmatic reasons), and I had to migrate only one Kotlin caller (who was invoking the method on a Java Map subtype) to use compute instead (which required only a second function parameter and an associated if (existing != null) return existing).

I still feel reasonably good about that, if not completely good. We could talk again about whether to support nullable values.

(Yes, I think @PolyNull would be nice here, as in the Checker Framework, and one option would be to use @Nullable in the signature of computeIfAbsent but program special logic into checkers to give the method @PolyNull-like behavior, as they'll likely already want to do for methods like Optional.orElse.)

This issue is not about that. (Maybe I should preemptively open one now that is about that... :)) What this issue is about is whether to more aggressively use @NonNull. We could do so by making the V type in the function @NonNull and by making the return type @NonNull. This could theoretically maybe someday help someone who wants to use computeIfAbsent on a map with a nullable value type but still wants to get a non-null V out.

I'm actually filing this issue mostly to discuss a couple reasons not to do that:

  • As noted above, our current signature matches Kotlin's built-in treatment of the Map method.
  • An explicitly non-null type would close off escape hatches for getting a null through computeIfAbsent:
    • It might encourage tools to insert runtime null checks more aggressively.
    • It prevents you from casting your Map<Foo, Bar> to a Map<Foo, @Nullable Bar> and then being able to pass a null through without any problem.

(I also was mistakenly thinking that there was a separate opportunity to add @NonNull elsewhere in the signature. But I was thinking of cases like computeIfPresent, which receive a non-null "existing value" parameter, which obviously doesn't make sense for computeIfAbsent.)

[*] Observant readers may have noted that I made similar changes for compute and computeIfPresent and that merge was already in a similar boat. But I subsequently changed those methods to use @Nullable types after all (in https://github.com/jspecify/jdk/pull/14 and https://github.com/jspecify/jdk/pull/32).

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with the java.util.Map.computeIfAbsent signature and the issue's discussion of its function and return types. Compare the current annotation choices with the linked JSpecify and Kotlin behavior, then determine whether a more explicit @NonNull treatment is warranted; done means reaching and documenting a clear annotation decision.

Written by the indexing model from the issue text.

Assessment

Tech stack
java
Domain
api
Issue type
Feature
Difficulty
5/5
Estimated time
Over a week
Activity status
Stale
Clarity
Needs clarification
Newbie friendliness
25/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.