Hello @incident-recipient,
What you are facing is a known issue, and we have no ways to avoid it except by, like you said, storing the result of the optional call to a variable. We however have this ticket to try to make it explicit in the rule message and description: SONARJAVA-3881
The “FPs” you report exist because we consider that two distinct calls to methods are potentially returning different values. On our side, we don’t consider them as FP.
Concretely, in the following code, we would consider that o1 and o2 could be different. It’s not necessarily true, but it’s not necessarily false as well.
Object o1 = foo();
Object o2 = foo();
We actually have no easy way to know what’s the internal behavior of the foo() method (is it related to the state of its owning object? is it the same object being returned systematically? is there any other means to change the returned value without calling foo()?). Considering that its two distinct values guarantee that we will not recommend wrong fixes and raises FPs for many rules. However, this model still causes noise for some rules, and in particular in S3655 with methods returning the same Optionals.
The following code, similar to yours, will raise an issue that could be considered as FP, but only if myOptional() returns always the same optional (which goes with other concerns as soon as you want to store it as a field):
if (myOptional().isPresent()
&& "hello".equals(myOptional().get())) // FP call isPresent() before calling get()
While Optionals are immutable, I still don’t manage to see it as a False Positive. We can not guarantee that it’s the same optional which will be returned between two method invocations. When no arguments are provided, we could expect that the method would act as a getter… but then it raises again some other problems. How to differentiate the following situation with a static-analysis engine?
boolean foo(MyObject a, MyObject b) {
return a.myOptional().isPresent() && b.myOptional.get() > 42;
// ^ 'a' is not the same as 'b' ^
}
A way to reduce the noise could be to consider that consecutive calls without arguments to a method returning an Optional actually return the same Optional… but this approximation could also lead to False Negatives… and ultimately bugs in your code. A risk that we don’t want to take.
I would therefore recommend marking your issues as “Won’t Fix” if you disagree with them.
Note that If you are doing this operation in many many places, I would even rather opt for another approach and a dedicated consumer:
import java.util.Optional;
import java.util.function.Consumer;
import java.util.function.Supplier;
public class Utils {
static <T> void consume(Supplier<Optional<T>> supplier, Consumer<T> consumer) {
supplier.get().ifPresent(value -> consumer.accept(value));
}
}
And your code would be:
Utils.consume(feature::getLongDescription, pickleJarBuilder::withLongDescription);
Cheers,
Michael