uber / NullAway

A tool to help eliminate NullPointerExceptions (NPEs) in your Java code with low build-time overhead
MIT License
3.6k stars 288 forks source link

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

Open ilopezv opened 4 months ago

ilopezv commented 4 months ago

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

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

[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 commented 4 months ago

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 commented 4 months ago

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.

image

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 commented 4 months ago

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:

https://github.com/uber/NullAway/blob/e3a3c7672d51ec2e90c0e60c30af477ccf6f7757/nullaway/src/main/java/com/uber/nullaway/handlers/LibraryModelsHandler.java#L735-L735

A PR with a fix would be welcome!