Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

"Incomplete" annotations #116

Open
J-N-K opened this issue Nov 7, 2020 · 5 comments
Open

"Incomplete" annotations #116

J-N-K opened this issue Nov 7, 2020 · 5 comments

Comments

@J-N-K
Copy link
Member

J-N-K commented Nov 7, 2020

E.g. Map.computeIfAbsent(K key, Function<? super K,? extends V> mappingFunction):

The return value is either the @NonNull value already present in the map or the result of mappingFunction, which is allowed to return null. Then the return-value of computeIsAbsent is also null. Our current annotation is

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

Which is in general correct. However the eclipse compiler is not able to infer @NonNull if the mappingFunction has a @NonNullreturn value, e.g. Map.computeIfAbsent(key, k -> new Foo());.

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

is not working, because then return Map.computeIfAbsent(key, k -> null); passes, even if the return-value of the method is annotated @NonNull.

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

is not working either, because then the compiler requires the mappingFunction to have a @NonNull return value.

Another problem is that

Stream<@NonNull T> = Stream<@Nullable T>.filter(Objects::nonNull);

is not working, because the compiler is not able to detect that all null-objects will be filtered by .filter(Objects::nonNull).

Has anyone any idea how to solve that?

@agentgt
Copy link

agentgt commented Apr 11, 2021

I'm still learning Eclipse's NP analysis but I think eclipse not inferring that:

Stream<@NonNull T> = Stream<@Nullable T>.filter(Objects::nonNull);

is correct logic. This is because nonNull could be the inverse logic. How does Eclipse know?

Anyway assuming you want to convert a stream to @Nonnull the following works:

List<@Nullable String> list = List.of();
Stream<@Nonnull String> s = list.stream().filter(Objects::nonNull).map(Objects::requireNonNull);

@J-N-K
Copy link
Member Author

J-N-K commented Apr 13, 2021

How could that be inverse logic?

@agentgt
Copy link

agentgt commented Apr 18, 2021

Sorry for the late reply.

I'm still figuring out how the inference is done for Eclipse NP but from a purely type semantics and inverse logic I meant you could write:

Stream<@NonNull T> = Stream<@Nullable T>.filter(!! Objects::nonNull ); // Where "!" is some not like predicate function

So how would eclipse know the above is NonNull given we "not" it twice. Even Kotlin or Scala would have problems with this.

requireNonNull on the other hand actually converts the type from @Nullable T -> @NonNull T.

That being said I don't understand how Eclipse will infer some object is nonNull if I do this:

Some object = .... some @Nullable function;
requireNonNull(object); // normally one would do object = requireNonNull(object)
object.stuff(); //this should still be a warning but for some reason Eclipse infers now that object is NonNull without the assignment.

That is from above in my mind object is still @Nullable. So obviously Eclipse is doing more than just treating NonNull as a type which makes since to handle if(object != null) you need that. But still how does it know requireNonNull now guarantees that object isn't null?

@agentgt
Copy link

agentgt commented Apr 18, 2021

I guess Eclipse does additional null analysis on attached source libraries such that

requireNonNull and nonNull are equivalent to inlining the conditional logic (ie if o != null ... else/return).

It seems to get this analysis you need it be an actual source library and not say a maven dependency. For example I can't write a requireNonNull in another maven module and expect it to work/infer like the JDK ones. Maybe the M2E eclipse addon fixes this (I thought I installed it but maybe not).

@agentgt
Copy link

agentgt commented May 3, 2021

@J-N-K Actually you are right about:

Stream<@NonNull T> = Stream<@Nullable T>.filter(Objects::nonNull);

I couldn't understand how the Objects.* methods could have complete null analysis. My understanding was just type based (ie annotation attached to types).

However ostensible there is an internal whitelist of methods that the JDT will then assume the input parameter will be @NonNull sort of like a flow type. https://bugs.eclipse.org/bugs/show_bug.cgi?id=553272

I looked at the EEA format to see if you could define such methods as the ones in Object.* and it doesn't appear so which is strange because those methods are still defined in the EEA augments library.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

2 participants