Support org.apache.commons.lang3.StringUtils.isNotEmpty

[ilopezv]

Add method support for org.apache.commons.lang3.StringUtils.isNotEmpty

Following code rise a exception with all versions of nullaway including 0.10.24

[75,103] [NullAway] dereferenced expression this.properties.username() is @Nullable
ERROR    (see http://t.uber.com/nullaway )
ERROR   Did you mean '@SuppressWarnings("NullAway") @Override'?
[76,40] [NullAway] dereferenced expression this.properties.password() is @Nullable
ERROR   (see http://t.uber.com/nullaway )
ERROR   Did you mean '@SuppressWarnings("NullAway") @Override'?

Also I tried to use the following config but it not worked

-XepOpt:NullAway:CheckOptionalEmptiness=true \
-XepOpt:NullAway:KnownInitializers=org.apache.commons.lang3.StringUtils.isNotEmpty

same happens with org.springframework.util.StringUtils

[msridhar]

The issue is that you call this.properties.username() (and password()) inside a lambda body. NullAway doesn't know when the lambda function will be invoked, and it does not assume that username() and password() will remain non-null until that point. If you store this.properties.username() and password() in local variables and do both the isNotEmpty() checks and the uses with those locals, everything will work (since the value of the local variables cannot change).

[ilopezv]

I know that the issue is closed, but it is not a similar case?
There is no lambda and nullpointer checks have been made at line 43.

KnownInitializers has been configured with org.apache.commons.collections4.isNotEmpty

-XepOpt:NullAway:KnownInitializers=org.springframework.util.Assert.notNull,org.apache.commons.collections4.isNotEmpty \
-XepOpt:NullAway:CheckOptionalEmptiness=true \ 

And the error still appears at line 45

[ERROR] /C:/santalucia/workspace/ams-spring-data/ams-spring-data-rest/src/main/java/com/santalucia/arq/ams/data/rest/config/repository/AmsRepositoryRestConfigurer.java:[45,52] [NullAway] dereferenced expression properties.exposeIdsFor() is @Nullable
[ERROR]     (see http://t.uber.com/nullaway )
[ERROR]   Did you mean '@SuppressWarnings("NullAway") @Override'?

[msridhar]

This is a different case. I don't think the KnownInitializers flag is being used correctly; that flag is for (most likely inherited) methods that behave like constructors almost and run before other methods in the class:

https://github.com/uber/NullAway/wiki/Configuration#known-initializers

For this case, we seem to be missing a model for the relevant CollectionUtils.isNotEmpty() method. That would be added around here:

NullAway/nullaway/src/main/java/com/uber/nullaway/handlers/LibraryModelsHandler.java

Line 735 in e3a3c76

.put(methodRef("spark.utils.CollectionUtils", "isNotEmpty(java.util.Collection<?>)"), 0)

A PR with a fix would be welcome!

[pochopsp]

Hello @msridhar . Would it just be sufficient to add a
.put(methodRef("org.apache.commons.collections4.CollectionUtils", "isNotEmpty(java.util.Collection<?>)"), 0)
?
P.S. I noticed that apache had a previous version with a different package name org.apache.commons.collections.CollectionUtils . Do we need to support this too ?

[msridhar]

@pochopsp something like that should be sufficient, but we should write a test to be sure. And yeah I'm fine with supporting org.apache.commons.collections.CollectionUtils also.