Map computeIfAbsent is incorrect

[agentgt]

@J-N-K I know you sort of mentioned this in #116 but I'm now fairly positive computeIfAbsent for map is wrong.

computeIfAbsent
 (TK;Ljava/util/function/Function<-TK;+TV;>;)TV;
 (TK;L1java/util/function/Function<-TK;+T0V;>;)T0V;

Should be

computeIfAbsent
 (TK;Ljava/util/function/Function<-TK;+TV;>;)TV;
 (TK;L1java/util/function/Function<-TK;+TV;>;)TV;

V is a free variable. It is not @Nullable like on get.

The Javadoc is:

     * @return the current (existing or computed) value associated with
     *         the specified key, or null if the computed value is null

For a Map<@NonNull String, @NonNull String> I think a reasonable assumption is the mappingFunction should return what type the map's key is and thanks to the generic V being @NonNull String Eclipse will enforce it. I'm not sure why you had problems with this but if Map is correctly annotated I get correct warnings like:

Map<@NonNull String, @NonNull String> m; ...
m.computeIfAbsent(key, k -> null /* warning here */);

Ditto for computeIfPresent EDIT I got confused with some other method on ConcurrentHashMap like removeIf.

These annotations are on ConcurrentHashMap as well.

[J-N-K]

Yes, but the computed value is allowed to be null (debatable if this makes sense, but that's what is in the JavaDoc). With (TK;L1java/util/function/Function<-TK;+TV;>;)TV; this not possible (at least when the map is @NonNull.

[agentgt]

I argue if you are using Map<@NonNull Key, @NonNull Value> you want computeIfAbsent to return @NonNull.
ComputeIfAbsent is almost always used in a caching.

Given that cache is used in performance areas it really is not desired to do another function call to either convert or check for null (e.g. requireNonNull).

I guess I could see a case for both uses but for folks that want it differently they could just make the map Map<@NonNull Key, @Nullable Value>.

Caffeine has its cache computeIfAbsent like:

  @PolyNull
  V get(K key, Function<? super K, ? extends @PolyNull V> mappingFunction);

We don't have the luxury of @PolyNull.

I guess it makes since to keep it @Nullable to be consistent. Besides the return null isn't about the containing type just that the mappingFunction declines to make a new value (perhaps to avoid caching everything or invalid input). It's kind of messed up because some maps can contain null.