Non-null type argument is expected

[ghost]

Hi

Lately I started working with intellij and its inspections mechanism seems to be too sensitive but I want to make sure I'm not missing something. I set the following
private Cache<String, AnObject> objectsCache = null; (Initialization comes later on)

It marks both String and AnObject with a warning:

Non-null type argument is expected

What am I missing?
Thanks

[ben-manes]

Oh I'm not sure ​(I use Eclipse). Maybe @vlsi can help explain what we should do.

In v3, we tried to better conform to the Checker Framework annotations for inspections (#337). The recommended change was use the signature Cache<K extends @NonNull Object, V extends @NonNull Object> to default the parameters and return types to being non-nullable, and then mark as null where needed (e.g. getIfPresent). Since all Java types can be nullable, this inspection should inform the analyzer whether a null is allowed as a parameter or return type. It sounds like IntelliJ is applying Kotlin's non-nullable type to Java code, which can't be expressed so then any Java type would be a warning. It seems like different understandings of the annotation and a warning that can't make sense for the Java language.

[ghost]

Thanks, I can suppress the warning but I usually prefer not to, unless it will really be something that can't be avoided otherwise.

[vlsi]

@amosss , do you have a reproducer?

[ghost]

mmm... I opened the project in intellij and I see that warning there where I define the Cache variable. Nothing special.

[vlsi]

Ok, AFAIK, IDEA does not support nullability annotations for generic type parameters yet.
A part of the story is that CheckerFramework is probably the only current implementation that suggests the way to annotate and verify generic types.

I guess IntelliJ team would follow up with https://github.com/jspecify/jspecify and support jspecify annotations and behavior once it is ready.

---

Cache<K extends @NonNull Object, V extends @NonNull Object>

For Checker Framework's default rules this declaration is the same as Cache<K extends Object, V extends Object>.
In practice, there are third-party issues when parsing bytecode with things like K extends @NonNull Object (javac might miscompile, or third-party libs might fail to parse).

That is why for now I refrain from K extends @NonNull Object and use K extends Object (see pgjdbc/pgjdbc#2010 ) even though it might be slightly less readable.

[ben-manes]

Oh, so the fix is to replace,

Cache<K extends @NonNull Object, V extends @NonNull Object>

with

Cache<K extends Object, V extends Object>

?

I hope jspecify makes progress because it seems kind of convoluted how analyzers interpret these annotations.

[vlsi]

Oh, so the fix is to replace,

It would probably do.
At least it should be sufficient for caffeine users even though caffeine is not fully annotated.

You might want replacing all instances of extends @NonNull with extends or extends /* @NonNull */

[ben-manes]

Thanks @vlsi!

@amosss can you try the snapshot / jitpack on the fix commit and let us know if it resolves the warnings?

[ghost]

Because it's a "living" project, I always work against releases. Is there a way I can try the snapshot by changing the maven dependency and then set it back after trying?

[ben-manes]

Yes, I didn't mean to imply that you would switch your project over beyond a quick check.

The CI will publish a snapshot build to Sonatype's repository under the version 3.0.2-SNAPSHOT. You can add that repository locally as https://oss.sonatype.org/content/repositories/snapshots. This isn't a stable version.

jitpack.io can build on-demand from a commit hash from a public repository. That remains stable as a decent means for hot patching if one has no other choice. We have it set up to skip tests when built that way to avoid timeouts.

You should be able to use either locally, see if the warnings go away, and then revert back to a release version.

[ghost]

So just to make sure, is it enough to add this jar: https://oss.sonatype.org/content/repositories/snapshots/com/github/ben-manes/caffeine/caffeine/3.0.2-SNAPSHOT/caffeine-3.0.2-20210410.225425-20.jar to my project to check the warnings disappear (and then revert back)?

[ben-manes]

Yep.

[ghost]

Works like a charm, I reverted to 3.0.1 until this is officially released, thanks!

[ghost]

Maybe related?
Unboxing of 'cache.getIfPresent(key)' may produce 'NullPointerException'

[vlsi]

   * @return the value to which the specified key is mapped, or {@code null} if this cache contains
   *         no mapping for the key

NPE is indeed possible if the key is not there.

[ghost]

Obviously :)
I do the following
if (cache.getIfPresent(key) == null || cache.getIfPresent(key) == 0)
Regarding the 2nd part, I get the mentioned warning. I guess this is not for you, but for intellij, my bad :)

[vlsi]

I do the following
if (cache.getIfPresent(key) == null || cache.getIfPresent(key) == 0)

Please do not do that. The second read can indeed cause NPE.
You want to query the cache just once and store the result into a local variable.

[ghost]

You got a point there...

[ben-manes]

Released in 3.0.2

[ghost]

Thanks!