FP on squid:S3655: Optional value should only be accessed after calling isPresent()

java

(Greg D Thomas) #1

I think I’ve found a false positive on the rule in the title. The following code triggers it;

final Optional<String> optionalString = Optional.of("not-a-number");
try {
    optionalString.ifPresent(value->Long.parseLong(value)
} catch(final NumberFormatException ignored) {
    System.out.println("Supplied value is not valid:" + optionalString.get());
}

I can see how this may be hard to fix, but as the exception can only be raised if the Optional is present, calling .get() in the catch block is valid.


(Alexandre Frigout) #2

Hello,

Have you tried to analyse your project with the last version of SonarJava ?
I did so and could not trigger the rule with your code (i checked that the rule is active in my QP).

Greetings,
Alex.


(Greg D Thomas) #3

Hmm, my apologies.

The FP was originally triggered on sonarcloud.io - I can’t check the version there as I don’t have the rights; instead I ran it against our local Sonar installation (SonarJava 5.4 build 14284 installed) , and that doesn’t seem to report it as a problem, so it does sound like it was a False-False-Positive :wink:

Greg


(Nicolas Bontoux) #4

Hi Greg,

SonarCloud :sonarcloud: is always running the latest and greatest features we’ve developed here. If you catch a false-positive in SonarCloud, then it likely needs to further looked into (independently of past behaviour). Provided you’ve executed the analysis as recommended of course, for example making sure that code is built and bytecode available to analysis (make sure there’s no related warning in your logs).

If you’ve got a minimal reproducer, then an ideal course of action would be to analyse it in a public project in SonarCloud, so that we can access/review the code/issue directly in SonarCloud.


(Greg D Thomas) #5

Well, the original is still present in a public project at https://sonarcloud.io/project/issues?id=com.bt.openlink%3Abt-openlink&open=AWXseVKDOlZ5QoPzqX_j&resolutions=FALSE-POSITIVE

There are three in that file, all along the same lines.

Greg


(Greg D Thomas) #6

Ah, apologies again. My reproduction was wrong, taking a look at the original issue refreshed my mind. A proper reproducer is as follow:

        final Optional<String> possibleLong = Optional.empty();
        try {
            possibleLong.map(Long::valueOf).ifPresent(number -> System.out.println("The value is " + number));
        } catch (final NumberFormatException ignored) {
            System.out.println(possibleLong.get() + " is not a valid number");
        }

(Aside; FWIW, IntelliJ does the “right thing” and does not report the call to possibleLong.get() as being possibly invalid)

Greg