A new rule suggestion:
- Calling any of the methods
orElse(other),orElseGet(supplier),orElseThrow(),orElseThrow(supplier),or(supplier)on an optional that is known to be always present is redundant and most likely not what the programmer intended. - Suggested fix: Either make sure that the optional might be empty, or remove the redundant call.
Snippets of Noncompliant Code
Foo foo = Optional.of(someFoo)
.orElse(defaultFoo); // Noncompliant -- defaultFoo will never be returned
Note that Optional.of(someFoo) never returns an empty optional; however, Optional.ofNullable(someFoo) does if someFoo is null. So the programmer probably meant:
Foo foo = Optional.ofNullable(someFoo)
.orElse(defaultFoo); // Compliant
Another example
Bar bar = Optional.of(someBar)
.orElseThrow(FancyException::new); // Noncompliant -- a FancyException will never be thrown
Suggested fix:
Bar bar = Optional.ofNullable(someBar)
.orElseThrow(FancyException::new); // Compliant
External references and/or language specifications:
-
Java 8 Optional: https://docs.oracle.com/javase/8/docs/api/java/util/Optional.html#of-T-
-
Java 11 Optional: https://docs.oracle.com/en/java/javase/11/docs/api/java.base/java/util/Optional.html#or(java.util.function.Supplier)
-
type : Bug
-
tags: java, optional, pitfall
Limitation of scope
It might be difficult to analyze the “presentness” of optionals in the general case, hence this rule could be limited to detecting only the examples above, i.e. mistakenly using Optional#of() instead of Optional#ofNullable().
I assume that these violations can be detected quite easily.
An extended version of this rule should also detect cases like the following:
Optional<Foo> opt = findAny("myfoo");
assertTrue(opt.isPresent()); // guarantees that opt is present
Foo foo1 = opt.orElseThrow(); // Noncompliant -- will never throw
Foo foo2 = opt.orElse(otherFoo); // Noncompliant -- will never be otherFoo
Foo foo3 = opt.get(); // Compliant