Recognize orElseThrow() in some checks that handle get()

PiperOrigin-RevId: 390462777
google · Aug 12, 2021 · ba12995 · ba12995
1 parent 10ae8a1
commit ba12995
Show file tree

Hide file tree

Showing 4 changed files with 54 additions and 2 deletions.
diff --git a/core/src/main/java/com/google/errorprone/bugpatterns/OptionalNotPresent.java b/core/src/main/java/com/google/errorprone/bugpatterns/OptionalNotPresent.java
@@ -38,6 +38,7 @@
 import com.sun.source.util.TreeScanner;
 import java.util.Iterator;
 import javax.annotation.Nullable;
+import javax.lang.model.element.Name;
 
 /** @author mariasam@google.com (Maria Sam) */
 @BugPattern(
@@ -132,7 +133,8 @@ public Void visitMethodInvocation(MethodInvocationTree tree, ExpressionTree opti
       if (receiver != null && ASTHelpers.sameVariable(receiver, optionalVar)) {
         ExpressionTree treeIdent = tree.getMethodSelect();
         if (treeIdent instanceof MemberSelectTree) {
-          if (((MemberSelectTree) treeIdent).getIdentifier().contentEquals("get")) {
+          Name identifier = ((MemberSelectTree) treeIdent).getIdentifier();
+          if (identifier.contentEquals("get") || identifier.contentEquals("orElseThrow")) {
             hasGet = true;
           }
         }

diff --git a/core/src/main/java/com/google/errorprone/bugpatterns/UnnecessaryOptionalGet.java b/core/src/main/java/com/google/errorprone/bugpatterns/UnnecessaryOptionalGet.java
@@ -61,7 +61,7 @@ public final class UnnecessaryOptionalGet extends BugChecker
       anyOf(
           instanceMethod()
               .onExactClass("java.util.Optional")
-              .namedAnyOf("get", "orElse", "orElseGet", "orElseThrow"),
+              .namedAnyOf("get", "orElseThrow", "orElse", "orElseGet", "orElseThrow"),
           instanceMethod()
               .onExactClass("java.util.OptionalLong")
               .namedAnyOf("getAsLong", "orElse", "orElseGet", "orElseThrow"),

diff --git a/core/src/test/java/com/google/errorprone/bugpatterns/OptionalNotPresentTest.java b/core/src/test/java/com/google/errorprone/bugpatterns/OptionalNotPresentTest.java
@@ -79,4 +79,23 @@ public void isEmpty() {
             "}")
         .doTest();
   }
+
+  @Test
+  public void orElseThrow() {
+    assumeTrue(RuntimeVersion.isAtLeast11());
+    compilationTestHelper
+        .addSourceLines(
+            "Test.java",
+            "import java.util.Optional;",
+            "class Test {",
+            "  int g(Optional<Integer> o) {",
+            "    // BUG: Diagnostic contains:",
+            "    if (o.isEmpty()) {",
+            "      return o.orElseThrow();",
+            "    }",
+            "    return -1;",
+            "  }",
+            "}")
+        .doTest();
+  }
 }
diff --git a/core/src/test/java/com/google/errorprone/bugpatterns/UnnecessaryOptionalGetTest.java b/core/src/test/java/com/google/errorprone/bugpatterns/UnnecessaryOptionalGetTest.java
@@ -16,7 +16,10 @@
 
 package com.google.errorprone.bugpatterns;
 
+import static org.junit.Assume.assumeTrue;
+
 import com.google.errorprone.BugCheckerRefactoringTestHelper;
+import com.google.errorprone.util.RuntimeVersion;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.junit.runners.JUnit4;
@@ -29,6 +32,7 @@ public final class UnnecessaryOptionalGetTest {
 
   @Test
   public void genericOptionalVars_sameVarGet_replacesWithLambdaArg() {
+    assumeTrue(RuntimeVersion.isAtLeast11());
     refactoringTestHelper
         .addInputLines(
             "Test.java",
@@ -40,6 +44,7 @@ public void genericOptionalVars_sameVarGet_replacesWithLambdaArg() {
             "    op.map(x -> Long.parseLong(op.get()));",
             "    op.filter(x -> op.get().isEmpty());",
             "    op.flatMap(x -> Optional.of(op.get()));",
+            "    op.flatMap(x -> Optional.of(op.orElseThrow()));",
             "  }",
             "}")
         .addOutputLines(
@@ -52,6 +57,7 @@ public void genericOptionalVars_sameVarGet_replacesWithLambdaArg() {
             "    op.map(x -> Long.parseLong(x));",
             "    op.filter(x -> x.isEmpty());",
             "    op.flatMap(x -> Optional.of(x));",
+            "    op.flatMap(x -> Optional.of(x));",
             "  }",
             "}")
         .doTest();
@@ -280,4 +286,29 @@ public void differentReceivers() {
         .expectUnchanged()
         .doTest();
   }
+
+  @Test
+  public void orElseThrow() {
+    assumeTrue(RuntimeVersion.isAtLeast11());
+    refactoringTestHelper
+        .addInputLines(
+            "Test.java",
+            "import java.util.Optional;",
+            "public class Test {",
+            "  private void home() {",
+            "    Optional<String> op = Optional.of(\"hello\");",
+            "    op.flatMap(x -> Optional.of(op.orElseThrow()));",
+            "  }",
+            "}")
+        .addOutputLines(
+            "Test.java",
+            "import java.util.Optional;",
+            "public class Test {",
+            "  private void home() {",
+            "    Optional<String> op = Optional.of(\"hello\");",
+            "    op.flatMap(x -> Optional.of(x));",
+            "  }",
+            "}")
+        .doTest();
+  }
 }