NullAway icon indicating copy to clipboard operation
NullAway copied to clipboard

Support StreamEx and EntryStream

Open alxhill opened this issue 3 years ago • 2 comments

StreamEx is a library that adds utility methods to streams in Java. The EntryStream class is particularly useful for using Maps with streams. For example:

EntryStream.of(map)
    .filterKeys(key -> anotherMap.containsKey(key))
    .mapKeyValue((key, value) -> doSomething(value, anotherMap.get(key)))
    .toMap()

Currently, NullAway complains that anotherMap.get() is nullable in spite of the containsKey filtering.

Would this be suitable for a contribution? Any thoughts/guidance on how challenging it might be?

alxhill avatar Dec 08 '22 21:12 alxhill

@alxhill yes this would be suitable for a contribution! And I don't think it should be too difficult to implement. You'd want to add an appropriate method to com.uber.nullaway.handlers.StreamNullabilityPropagatorFactory (see the other code in that class):

https://github.com/uber/NullAway/blob/f6408c6f14dc41868cf5ff5921d9aff3172dbfc9/nullaway/src/main/java/com/uber/nullaway/handlers/StreamNullabilityPropagatorFactory.java#L30-L30

Then, you could add the handler to the handler list around here:

https://github.com/uber/NullAway/blob/f6408c6f14dc41868cf5ff5921d9aff3172dbfc9/nullaway/src/main/java/com/uber/nullaway/handlers/Handlers.java#L61-L61

And of course we'd want some tests as well :-)

My only vague concern is around adding overhead to NullAway, but my feeling is this shouldn't be too bad. If it ever became an issue, we could put support behind a flag that is off by default.

Let us know if you have any other questions.

msridhar avatar Dec 08 '22 21:12 msridhar

Thanks for the speedy response @msridhar! I noticed the case I'm describing above also seems not to work for standard java streams either - created the following issue to track as well: https://github.com/uber/NullAway/issues/697

alxhill avatar Dec 13 '22 17:12 alxhill