Hi!
I have pretty simple code that makes SonarLint report false-positive on city.getCountry():
if (address.getCity() != null) {
final City city = address.getCity();
// ...
keywords.addAll( getKeywords( city.getCountry() ) );
// ...
}
The warning is: A “NullPointerException” could be thrown; “city” is nullable here.
The idea is to avoid creating City instance if none is available, so first goes the check, and if city exists, an instance is created which is obviously not null because of previous check.
Hello, welcome to the community! And thank you for reporting this behavior.
The problem is, the analyzer has no guarantee that the 2 subsequent calls to getCity() will return the same thing. Even assuming that address is a POJO and getCity a simple getter method, the internal state of address could change between the 2 calls - the static analyzer cannot prove that the second call does not return null.
One way to write it with statically correct null checks would be:
final City city = address.getCity();
if (city != null) {
// ...
keywords.addAll( getKeywords( city.getCountry() ) );
// ...
}
Note that there is no instance creation here, only an assignment to a variable in the local scope that points to the instance returned by getCity.
As this code snippet is a part of indexing engine for a whole lot of data, the idea was to have only one null check instead of assignment and null check. But it can go this way too, I agree.