Add more Optional methods to ReturnValueIgnored

google · Apr 13, 2021 · 900d17d · 900d17d
1 parent 2f5fbbe
commit 900d17d
Show file tree

Hide file tree

Showing 3 changed files with 41 additions and 38 deletions.
diff --git a/core/src/main/java/com/google/errorprone/bugpatterns/ReturnValueIgnored.java b/core/src/main/java/com/google/errorprone/bugpatterns/ReturnValueIgnored.java
@@ -204,8 +204,16 @@ private static boolean functionalMethod(ExpressionTree tree, VisitorState state)
   private static final Matcher<ExpressionTree> OPTIONAL_METHODS =
       anyOf(
           staticMethod().onClass("java.util.Optional"),
+          instanceMethod().onExactClass("java.util.Optional").named("filter"),
+          instanceMethod().onExactClass("java.util.Optional").named("flatMap"),
+          instanceMethod().onExactClass("java.util.Optional").named("get"),
+          instanceMethod().onExactClass("java.util.Optional").named("isEmpty"),
           instanceMethod().onExactClass("java.util.Optional").named("isPresent"),
-          instanceMethod().onExactClass("java.util.Optional").named("isEmpty"));
+          instanceMethod().onExactClass("java.util.Optional").named("map"),
+          instanceMethod().onExactClass("java.util.Optional").named("or"),
+          instanceMethod().onExactClass("java.util.Optional").named("orElse"),
+          instanceMethod().onExactClass("java.util.Optional").named("orElseGet"),
+          instanceMethod().onExactClass("java.util.Optional").named("orElseThrow"));
 
   /**
    * The return values of {@link java.util.concurrent.TimeUnit} methods should always be checked.

diff --git a/core/src/test/java/com/google/errorprone/bugpatterns/ReturnValueIgnoredTest.java b/core/src/test/java/com/google/errorprone/bugpatterns/ReturnValueIgnoredTest.java
@@ -140,43 +140,6 @@ public void javaTime() {
         .doTest();
   }
 
-  @Test
-  public void optionalStaticMethods() {
-    compilationHelper
-        .addSourceLines(
-            "Test.java",
-            "import java.util.Optional;",
-            "class Test {",
-            "  void optional() {",
-            "    // BUG: Diagnostic contains: ReturnValueIgnored",
-            "    Optional.empty();",
-            "    // BUG: Diagnostic contains: ReturnValueIgnored",
-            "    Optional.of(42);",
-            "    // BUG: Diagnostic contains: ReturnValueIgnored",
-            "    Optional.ofNullable(null);",
-            "  }",
-            "}")
-        .doTest();
-  }
-
-  @Test
-  public void optionalInstanceMethods() {
-    compilationHelper
-        .addSourceLines(
-            "Test.java",
-            "import java.util.Optional;",
-            "class Test {",
-            "  void optional() {",
-            "    Optional<Integer> optional = Optional.of(42);",
-            "    // BUG: Diagnostic contains: ReturnValueIgnored",
-            "    optional.isPresent();",
-            "    optional.filter(v -> v > 40);",
-            "    optional.map(v -> Integer.toString(v));",
-            "  }",
-            "}")
-        .doTest();
-  }
-
   @Test
   public void timeUnitApis() {
     compilationHelper

diff --git a/...test/java/com/google/errorprone/bugpatterns/testdata/ReturnValueIgnoredPositiveCases.java b/...test/java/com/google/errorprone/bugpatterns/testdata/ReturnValueIgnoredPositiveCases.java
@@ -23,6 +23,7 @@
 import java.nio.file.Paths;
 import java.util.Arrays;
 import java.util.Locale;
+import java.util.Optional;
 
 /** @author alexeagle@google.com (Alex Eagle) */
 public class ReturnValueIgnoredPositiveCases {
@@ -160,4 +161,35 @@ public class ReturnValueIgnoredPositiveCases {
     // BUG: Diagnostic contains: Return value of this method must be used
     Arrays.toString(objects);
   }
+
+  Optional<String> o = Optional.of("foo");
+
+  { // Optional methods
+    // BUG: Diagnostic contains: Return value of this method must be used
+    Optional.empty();
+    // BUG: Diagnostic contains: Return value of this method must be used
+    Optional.of("foo");
+    // BUG: Diagnostic contains: Return value of this method must be used
+    Optional.ofNullable("foo");
+    // BUG: Diagnostic contains: Return value of this method must be used
+    o.filter(v -> true);
+    // BUG: Diagnostic contains: Return value of this method must be used
+    o.flatMap(v -> Optional.of(v));
+    // BUG: Diagnostic contains: Return value of this method must be used
+    o.get();
+    // BUG: Diagnostic contains: Return value of this method must be used
+    o.isEmpty();
+    // BUG: Diagnostic contains: Return value of this method must be used
+    o.isPresent();
+    // BUG: Diagnostic contains: Return value of this method must be used
+    o.map(v -> v);
+    // BUG: Diagnostic contains: Return value of this method must be used
+    o.or(() -> Optional.of("bar"));
+    // BUG: Diagnostic contains: Return value of this method must be used
+    o.orElse("bar");
+    // BUG: Diagnostic contains: Return value of this method must be used
+    o.orElseGet(() -> "bar");
+    // BUG: Diagnostic contains: Return value of this method must be used
+    o.orElseThrow(() -> new RuntimeException());
+  }
 }