SonarJava: "Call "Optional#isPresent()" before accessing the value." is incorrect for ternary operator

1,552 views
Skip to first unread message

stanisla...@outlook.com

unread,
Oct 31, 2017, 8:57:21 AM10/31/17
to SonarQube
Hello SonarJava plugin folks.

SonarJava reported for this code:

public Boolean getEnabled() {
    return conf.isPresent() ? conf.get().getEnabled() : enabled;
}

(note code simplified)

this issue:

squid:S3655 Call "Optional#isPresent()" before accessing the value.
Issue:
https://github.com/SonarSource/sonar-java/blob/master/java-checks/src/main/resources/org/sonar/l10n/java/rules/squid/S3655_java.json
Unit test for the issue:
https://github.com/SonarSource/sonar-java/blob/master/java-frontend/src/test/files/se/OptionalGetBeforeIsPresentCheck.java#L56

I see some problems with this reported issue:
1. The title is misleading, "Optional#isPresent()" is actually called. I assume the intent is to suggest "Optional#orElse" instead, ternary operator is not mentioned though.
2. In this particular case "Optional#orElse" can't be used anyway, because there is no orElse conf which could be used instead, so I think that the the issue is incorrect

Version 4.10.0.10260 but I think it's for the latest one as well, I haven't updated though, just checked code in master.

Thanks,
Stan

simon schrottner

unread,
Oct 31, 2017, 9:57:27 AM10/31/17
to SonarQube
hi...

i have to add on the side, that i think this usage of ifPresent - get are an antipattern!

use

return conf.map(conf::getEnabled).orElse(enabled)

and there is a orelse to use :D

br
simon

Michael Gumowski

unread,
Nov 1, 2017, 3:54:09 AM11/1/17
to simon schrottner, SonarQube
Hello Stan,

This rule is based on our Symbolic Execution (SE) engine. Which means that improvement of the engine (independent from the rule) can still improve the behavior of the rule itself, even the code is let unchanged. I can not reproduce the issue with the latest release version (4.14.0.11784) and the following code snippet:

import java.util.Optional;

class A {

  Optional<A> conf;
  boolean enabled;

  public Boolean getEnabled() {
    return conf.isPresent() ? conf.get().getEnabled() : enabled;
  }
}

Please check with latest version of the SonarJava analyzer. It's most likely that the engine has been improved between your version and the actual one.
If the code is not equivalent to your case, please provide a self contained reproducer (including all dependencies and defining all the fields).

Cheers,
Michael

--
You received this message because you are subscribed to the Google Groups "SonarQube" group.
To unsubscribe from this group and stop receiving emails from it, send an email to sonarqube+...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/sonarqube/fe4bb387-729b-4278-93f8-34a9c3759ca9%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
--
Michael Gumowski | SonarSource
Software Developer, Language Team
https://www.sonarsource.com

stanisla...@outlook.com

unread,
Nov 1, 2017, 11:20:44 AM11/1/17
to SonarQube
Thanks Simon for the suggestion, thanks Michael for the info. We've upgraded both the plugin and the server today, I'll check if the behavior has changed!
Reply all
Reply to author
Forward
0 new messages