java:S3655 false positive?

[Starnose (David Hicks)]

Code snippet -

if (resp.responseBody().isPresent()) {
    var responseBody = MAPPER.readValue(resp.responseBody().get(), ClientCreationResponse.class);

This is triggering java:S3655 :

Call “Optional#isPresent()” or “!Optional#isEmpty()” before accessing the value.Why is this an issue?

I’m calling “isPresent” right there in the preceding line! This has triggered in a few places in a recent changeset of mine. There are also two like this -

if (resp.responseBody().isEmpty()) {
    throw new CustomException("Exception message");
}
var responseObject = mapper.readValue(resp.responseBody().get(), ResponseObject.class);

Seems safe to me?

[Colin]

Hey there.

I believe a discussion of this rule (and this case) has taken place in this thread:

java:S3655 Call "Optional#isPresent()" before accessing the value

This is not a false positive. Your code is as follow: if (obj.getOptional().isPresent()) { obj.getOptional().get() // issue } There is no guarantee that the first getOptional() method execution returns the same object as the second getOptional() method execution. You have to store the value locally: Optional<String> value = obj.getOptional() if (value.isPresent()) { value.get() // no issue }

[Starnose (David Hicks)]

Thanks Colin,

I think this is a very borderline case to flag as a ‘bug’ category error. As we are happy the underlying code is not going to be switching out the return value of this accessor method, I’m marking this lot as “won’t fix”

I wonder if every instance of code like this triggers a bug?

if (obj.fieldAccessor() != null) {
  obj.fieldAccessor().doSomething()
}