Reproduced this in SonarLint for Eclipse 10.8.0.82289.
There are cases where the rule “S3655: Optional value should only be accessed after calling isPresent()” produces a false-positive when we could logically reason that the accessed value is present at that point.
Take the following simplified example:
private String test()
{
Optional<String> opt = getOptString(); // Get an Optional from somewhere.
if ( opt.map( String::toLowerCase ).filter( s -> s.equals( "test" ) ).isPresent() )
{
return opt.get(); // FP: Sonar thinks that opt has not been checked here yet.
}
return "";
}
The problem is basically the call to Optional.map()
in the condition. This call can at most change an already present value to become absent (by mapping to null). But if the value would already be absent, then it could not become present via that call (because the mapping function would not be executed in the first place). Thus we can reason that if the result of Optional.map()
is present, the original value should also be considered present and therefore a false-positive in this rule.
Note that if we only have the Optional.filter()
call without the Optional.map()
like this
if ( opt.filter( s -> s.equals( "test" ) ).isPresent() )
{
return opt.get(); // No FP.
}
then Sonar is already correctly considering opt
as having been checked. Logically speaking, this is effectively the same as Optional.map( s -> s.equals( "test" ) ? s : null )
. So the only thing map()
does differently than filter()
is that it could also return something else than s
, but that does not change the fact that s
was present in this chain, implying that the same reasoning could be used.