CheckOptionalEmptiness disregards initial value, fallback value
Nobody has claimed this yet.
- Dominant language
- Java
- Stars
- 4.1k
- Forks
- 370
- Avg merge
- 1d 13h
- Merged PRs (30d)
- 68
Description
From the history of CheckOptionalEmptiness I'm not entirely certain which exact issues it is intended to protect against but I ran into a surprise with j.u.Optional::or that led me to map out a number of false positive warnings. These seem to revolve around
- the initial value passed to an
Optionalfactory is ignored - fallback providers are not recognized
Both cases fail with [NullAway] Invoking get() on possibly empty Optional o
Some of these look like #557 but I don't see other related issues.
All of my cases for posterity:
Object value;
Optional<Object> o;
o = Optional.empty();
if (o.isPresent()) {
value = o.get(); // true negative
} else {
value = o.get(); // true positive
}
if (o.isEmpty()) {
value = o.get(); // true positive
} else {
value = o.get(); // true negative
}
o = Optional.of(new Object());
value = o.get(); // false positive
o = Optional.ofNullable(new Object()); // huh, no ErrorProne lint!
value = o.get(); // false positive
try {
o = Optional.of(new Object());
} catch (Exception e) {
throw new RuntimeException();
}
value = o.get(); // false positive
try {
o = Optional.of(new Object());
if (o.isEmpty()) {
throw new RuntimeException();
}
value = o.get(); // true negative
} catch (Exception e) { }
o = o.isPresent() ? o : Optional.of(new Object());
value = o.get(); // false positive
o = o.isEmpty() ? Optional.of(new Object()) : o;
value = o.get(); // false positive
o = o.or(() -> Optional.of(new Object()));
value = o.get(); // false positive
value = o.orElse(new Object()); // true negative
value = o.orElseGet(() -> new Object()); // true negative
value = o.orElseGet(Object::new); // true negative
value = o.orElseThrow(); // true negative
value = o.orElseThrow(RuntimeException::new); // true negative
using JDK 17 and
<build>
<plugins>
<plugin>
<artifactId>maven-compiler-plugin</artifactId>
<configuration>
<fork>true</fork>
<annotationProcessorPaths>
<path>
<groupId>com.google.errorprone</groupId>
<artifactId>error_prone_core</artifactId>
<version>2.24.1</version>
</path>
<path>
<groupId>com.uber.nullaway</groupId>
<artifactId>nullaway</artifactId>
<version>0.10.19</version>
</path>
</annotationProcessorPaths>
<compilerArgs>
<arg>-XDcompilePolicy=simple</arg>
<arg>
-Xplugin:ErrorProne \
-Xep:NullAway:ERROR \
-XepOpt:NullAway:AnnotatedPackages=foo \
-XepOpt:NullAway:CheckOptionalEmptiness=true \
</arg>
<arg>-J--add-exports=jdk.compiler/com.sun.tools.javac.api=ALL-UNNAMED</arg>
<arg>-J--add-exports=jdk.compiler/com.sun.tools.javac.file=ALL-UNNAMED</arg>
<arg>-J--add-exports=jdk.compiler/com.sun.tools.javac.main=ALL-UNNAMED</arg>
<arg>-J--add-exports=jdk.compiler/com.sun.tools.javac.model=ALL-UNNAMED</arg>
<arg>-J--add-exports=jdk.compiler/com.sun.tools.javac.parser=ALL-UNNAMED</arg>
<arg>-J--add-exports=jdk.compiler/com.sun.tools.javac.processing=ALL-UNNAMED</arg>
<arg>-J--add-exports=jdk.compiler/com.sun.tools.javac.tree=ALL-UNNAMED</arg>
<arg>-J--add-exports=jdk.compiler/com.sun.tools.javac.util=ALL-UNNAMED</arg>
<arg>-J--add-opens=jdk.compiler/com.sun.tools.javac.code=ALL-UNNAMED</arg>
<arg>-J--add-opens=jdk.compiler/com.sun.tools.javac.comp=ALL-UNNAMED</arg>
</compilerArgs>
</configuration>
</plugin>
</plugins>
</build>
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start by locating the CheckOptionalEmptiness implementation and its existing Optional analysis tests. Reproduce the Java 17 cases for Optional.of, conditional assignments, and Optional.or, then add coverage for the reported false positives while preserving the existing true-positive checks. Done means valid non-empty flows no longer produce warnings and genuinely unsafe get() calls still do.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- java
- Domain
- tooling
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Quiet
- Clarity
- Mostly clear
- Newbie friendliness
- 45/100