Adds tests and refactors existing tests for clarity.

As currently implemented, this bugpattern checker only handles an extremely narrow issue set and likely gives developers much more confidence in Optional safety than is actually provided. If we paste the same two files into the Checker Framework's live demo for the Optional checker, it passes on all test cases except that it still has an (understandably) false-positive for the OptionalNotPresentNegativeCases.getWhenTestedSafe_equals.

I would recommend we highlight the shortcomings of this checker very clearly.

PiperOrigin-RevId: 477896538

core/src/test/java/com/google/errorprone/bugpatterns/testdata/OptionalNotPresentNegativeCases.java

@@ -18,49 +18,53 @@
 import java.util.Optional;
 import java.util.function.Predicate;
 
+/** Includes true-negative cases and false-positive cases. */
 public class OptionalNotPresentNegativeCases {
 
   // Test this doesn't trigger NullPointerException
   private final Predicate<Optional<?>> asField = o -> !o.isPresent();
 
-  public void testBasic(Optional<String> optional) {
-    if (optional.get() != null) {
-      String str = optional.get();
+  // False-positive
+  public String getWhenTestedSafe_referenceEquality(Optional<String> optional) {
+    if (!optional.isPresent()) {
+      if (optional == Optional.of("OK")) { // always false
+        // BUG: Diagnostic contains: Optional
+        return optional.get();
+      }
     }
+    return "";
   }
 
-  public void testNested(Optional<String> optional) {
-    if (7 == 7) {
-      String str = optional.get();
-      if (!optional.isPresent()) {
-        System.out.println("test");
+  // False-positive
+  public String getWhenTestedSafe_equals(Optional<String> optional) {
+    if (!optional.isPresent()) {
+      if (optional.equals(Optional.of("OK"))) { // always false
+        // BUG: Diagnostic contains: Optional
+        return optional.get();
       }
     }
+    return "";
   }
 
-  public void testOptional(Optional<String> optional) {
+  public String getWhenPresent_blockReassigned(Optional<String> optional) {
     if (!optional.isPresent()) {
       optional = Optional.of("value");
-      String str = optional.get();
+      return optional.get();
     }
+    return "";
   }
 
-  public void afterIf(Optional<String> optional) {
+  public String getWhenPresent_localReassigned(Optional<String> optional) {
     if (!optional.isPresent()) {
       optional = Optional.of("value");
     }
-    String str = optional.get();
-  }
-
-  public void doubleChecked(Optional<String> optional) {
-    if (!optional.isPresent() || 7 == 7) {
-      String foo = optional.isPresent() ? optional.get() : "";
-    }
+    return optional.get();
   }
 
-  public void checkMultipleInIf(Optional<String> optional) {
-    if (!optional.isPresent() || 7 == 7) {
-      String str = !optional.isPresent() ? optional.get() : "";
+  public String getWhenPresent_nestedCheck(Optional<String> optional) {
+    if (!optional.isPresent() || true) {
+      return optional.isPresent() ? optional.get() : "";
     }
+    return "";
   }
 }

core/src/test/java/com/google/errorprone/bugpatterns/testdata/OptionalNotPresentPositiveCases.java

@@ -17,45 +17,97 @@
 
 import java.util.Optional;
 
+/** Includes true-positive and false-negative cases. */
 public class OptionalNotPresentPositiveCases {
 
-  public void test(Optional<String> testStr) {
+  // False-negative
+  public String getWhenUnknown(Optional<String> optional) {
+    return optional.get();
+  }
+
+  // False-negative
+  public String getWhenUnknown_testNull(Optional<String> optional) {
+    if (optional.get() != null) {
+      return optional.get();
+    }
+    return "";
+  }
+
+  // False-negative
+  public String getWhenAbsent_testAndNestUnrelated(Optional<String> optional) {
+    if (true) {
+      String str = optional.get();
+      if (!optional.isPresent()) {
+        return "";
+      }
+      return str;
+    }
+    return "";
+  }
+
+  public String getWhenAbsent(Optional<String> testStr) {
     if (!testStr.isPresent()) {
       // BUG: Diagnostic contains: Optional
-      String str = testStr.get();
+      return testStr.get();
     }
+    return "";
   }
 
-  public void testMultipleStatements(Optional<String> optional) {
+  public String getWhenAbsent_multipleStatements(Optional<String> optional) {
     if (!optional.isPresent()) {
       String test = "test";
       // BUG: Diagnostic contains: Optional
-      String str = optional.get();
+      return test + optional.get();
     }
+    return "";
   }
 
-  public void testNestedIf(Optional<String> optional) {
-    if (!optional.isPresent()) {
-      if (optional == Optional.of("")) {
-        // BUG: Diagnostic contains: Optional
-        String str = optional.get();
-      }
+  // False-negative
+  public String getWhenAbsent_nestedCheck(Optional<String> optional) {
+    if (!optional.isPresent() || true) {
+      return !optional.isPresent() ? optional.get() : "";
     }
+    return "";
   }
 
-  public void testAnd(Optional<String> optional) {
-    if (!optional.isPresent() && 7 == 7) {
+  public String getWhenAbsent_compoundIf_false(Optional<String> optional) {
+    if (!optional.isPresent() && true) {
       // BUG: Diagnostic contains: Optional
-      String str = optional.get();
+      return optional.get();
     }
+    return "";
   }
 
-  public String checkedInElse(Optional<String> optional) {
+  // False-negative
+  public String getWhenAbsent_compoundIf_true(Optional<String> optional) {
+    if (!optional.isPresent() || true) {
+      return optional.get();
+    }
+    return "";
+  }
+
+  public String getWhenAbsent_elseClause(Optional<String> optional) {
     if (optional.isPresent()) {
       return optional.get();
     } else {
       // BUG: Diagnostic contains: Optional
       return optional.get();
     }
   }
+
+  // False-negative
+  public String getWhenAbsent_localReassigned(Optional<String> optional) {
+    if (!optional.isPresent()) {
+      optional = Optional.empty();
+    }
+    return optional.get();
+  }
+
+  // False-negative
+  public String getWhenAbsent_methodScoped(Optional<String> optional) {
+    if (optional.isPresent()) {
+      return "";
+    }
+    return optional.get();
+  }
 }