OI_OPTIONAL_ISSUES_USES_IMMEDIATE_EXECUTION: False Positive on final static initialized fields

[aepfli]

OI_OPTIONAL_ISSUES_USES_IMMEDIATE_EXECUTION is producing false positives when using static methods which are returning statically initialized fields, like Collections.emptyList().

List list = Optional.of(null).orElse(Collections.emptyList());

This will cause an issue. But Collections.emptyList() is returning Collections.EMPTY_LIST which is a final static field with an empty list. hence that this is producing a false positive.

This false positive applies at least for:

• https://docs.oracle.com/javase/7/docs/api/java/util/Collections.html#emptyList()
• https://docs.oracle.com/javase/7/docs/api/java/util/Collections.html#emptySet()
• https://docs.oracle.com/javase/7/docs/api/java/util/Collections.html#emptyMap()
• https://google.github.io/guava/releases/21.0/api/docs/com/google/common/collect/ImmutableList.html#of--
• https://google.github.io/guava/releases/21.0/api/docs/com/google/common/collect/ImmutableSet.html#of--
• https://google.github.io/guava/releases/21.0/api/docs/com/google/common/collect/ImmutableMap.html#of--

---

With a little guidance, i might be able to fix this issue and provide a pull request. if this is easily achievable.

[mebigfatguy]

There are probably a bunch of methods that are so non-impactful in nature that reporting this violation is pointless. The problem is how to you determine which methods do you consider basically no-ops. The simple solution is to just have a list of commonly known simple methods to ignore, which i've done in the past for other things. Certainly would be fine to do here as well, but it's a hack, at it's basis.

[ThrawnCA]

@aepfli You could just use Collections::emptyList, right?

[aepfli]

@ThrawnCA no we cant, because this is not lambda but an object -> we could use .orElseGet(Collections::emptyList) as a work arround.

Assigning a raw type to a generic one is not type safe, and will generate a warning. The old EMPTY_... fields of the Collections class return raw types, whereas the newer empty...() methods return generic ones. https://rules.sonarsource.com/java/RSPEC-1596

[aepfli]

i am not that familiar with spotbugs and its capabilities

i would love to try to detect if a method just returns a final static field - and if so do not report the issue. as a first step. if you think that is suitable @mebigfatguy. else i will go for the whitelisting, but as you are said already, there are most likely a lot of those methods, and white-listing will cause more maintainance, in the end to keep it up to date (find all usages). so i would rather for a deterministic approach :D

[mebigfatguy]

FindBugs is pass-based. so you can run over the code base multiple times collecting info. In an earlier pass you could determine what methods just return a static constant, and build a set up of those methods, then in the later pass, where this detector runs, you can look at that set to see if the method in question is one of them.

[zman0900]

Also applies when constants are used directly:

int x = Optional.ofNullable(getSomeInteger()).orElse(42);

[mebigfatguy]

yeah, definitely a stupid report. it's reporting the Integer.valueOf(42) call