Fix ClassCastException in ReplaceOptionalIsPresentWithIfPresent

Fixes #430
openrewrite · Jan 7, 2025 · 9d252a1 · 9d252a1
1 parent bc9aeb2
commit 9d252a1
Show file tree

Hide file tree

Showing 2 changed files with 48 additions and 3 deletions.
diff --git a/src/main/java/org/openrewrite/staticanalysis/ReplaceOptionalIsPresentWithIfPresent.java b/src/main/java/org/openrewrite/staticanalysis/ReplaceOptionalIsPresentWithIfPresent.java
@@ -20,6 +20,7 @@
 import org.openrewrite.*;
 import org.openrewrite.java.*;
 import org.openrewrite.java.search.UsesMethod;
+import org.openrewrite.java.tree.Expression;
 import org.openrewrite.java.tree.J;
 import org.openrewrite.java.tree.JavaType;
 import org.openrewrite.java.tree.Statement;
@@ -86,13 +87,13 @@ public J visitIf(J.If _if, ExecutionContext ctx) {
                 return _if;
             }
 
-            J.Identifier optionalVariable =
-                    (J.Identifier) ((J.MethodInvocation) _if.getIfCondition().getTree()).getSelect();
-            if (optionalVariable == null || !isStatementLambdaConvertible(_if.getThenPart())) {
+            Expression select = ((J.MethodInvocation) _if.getIfCondition().getTree()).getSelect();
+            if (!(select instanceof J.Identifier) || !isStatementLambdaConvertible(_if.getThenPart())) {
                 return _if;
             }
 
             /* replace if block with Optional#ifPresent and lambda expression */
+            J.Identifier optionalVariable = (J.Identifier) select;
             String methodSelector = optionalVariable.getSimpleName();
 
             Cursor nameScope = getCursor();

diff --git a/src/test/java/org/openrewrite/staticanalysis/ReplaceOptionalIsPresentWithIfPresentTest.java b/src/test/java/org/openrewrite/staticanalysis/ReplaceOptionalIsPresentWithIfPresentTest.java
@@ -37,6 +37,7 @@ public void defaults(RecipeSpec spec) {
     @Test
     void doNothingIfIsPresentNotFound() {
         rewriteRun(
+          //language=java
           java(
             """
               import java.util.Optional;
@@ -56,6 +57,7 @@ void method() {
     @Test
     void doNothingIfPresentPartOfElseIf() {
         rewriteRun(
+          //language=java
           java(
             """
               import java.util.Optional;
@@ -81,6 +83,7 @@ void method() {
     @Test
     void doNothingIfElsePartPresent() {
         rewriteRun(
+          //language=java
           java(
             """
               import java.util.Optional;
@@ -105,6 +108,7 @@ void method() {
     @Test
     void doNothingIfContainsReturn() {
         rewriteRun(
+          //language=java
           java(
             """
               import java.util.Optional;
@@ -125,6 +129,7 @@ Integer method(Optional<Integer> o) {
     @Test
     void ignoreReturnInsideLambda() {
         rewriteRun(
+          //language=java
           java(
             """
               import java.util.Optional;
@@ -163,6 +168,7 @@ int method(Optional<Integer> o) {
     @Test
     void allowFieldAccess() {
         rewriteRun(
+          //language=java
           java(
             """
               import java.util.Optional;
@@ -195,6 +201,7 @@ int method(Optional<Integer> o) {
     @Test
     void ignoreReturnInsideAnonymousSubclass() {
         rewriteRun(
+          //language=java
           java(
             """
               import java.util.Optional;
@@ -239,6 +246,7 @@ public Integer get() {
     @Test
     void doNothingIfContainsThrow() {
         rewriteRun(
+          //language=java
           java(
             """
               import java.util.Optional;
@@ -258,6 +266,7 @@ Integer method(Optional<Integer> o) {
     @Test
     void doNothingIfLocalVariableAssignedInsideIfBlock() {
         rewriteRun(
+          //language=java
           java(
             """
               import java.util.Optional;
@@ -282,6 +291,7 @@ void method() {
     @Test
     void doNothingIfNonEffectivelyFinalVariableAccessed() {
         rewriteRun(
+          //language=java
           java(
             """
               import java.util.Optional;
@@ -301,6 +311,7 @@ void method() {
               }
               """
           ),
+          //language=java
           java(
             """
               import java.util.Optional;
@@ -327,6 +338,7 @@ void method() {
     @Test
     void nestedOptionalsUnlessHandledCorrectly() {
         rewriteRun(
+          //language=java
           java(
             """
               import java.util.Optional;
@@ -365,6 +377,7 @@ Integer method(Optional<Integer> a, Optional<Integer> b, Optional<Integer> c) {
     @Test
     void replace() {
         rewriteRun(
+          //language=java
           java(
             """
               import java.util.Optional;
@@ -402,6 +415,7 @@ void method() {
     @Test
     void replace2() {
         rewriteRun(
+          //language=java
           java(
             """
               import java.util.Optional;
@@ -448,6 +462,7 @@ void method() {
     @Test
     void replaceIfStaticVariableAccessedORAssigned() {
         rewriteRun(
+          //language=java
           java(
             """
               import java.util.Optional;
@@ -484,6 +499,7 @@ void method() {
               }
               """
           ),
+          //language=java
           java(
             """
               import java.util.Optional;
@@ -524,6 +540,7 @@ void method() {
     @Test
     void replaceIfInstanceVariableAssignedORAccessed() {
         rewriteRun(
+          //language=java
           java(
             """
               import java.util.Optional;
@@ -560,6 +577,7 @@ void method() {
               }
               """
           ),
+          //language=java
           java(
             """
               import java.util.Optional;
@@ -600,6 +618,7 @@ void method() {
     @Test
     void replaceNestedIf() {
         rewriteRun(
+          //language=java
           java(
             """
               import java.util.Optional;
@@ -642,6 +661,7 @@ void method() {
     @Test
     void replaceAndHandleDifferentOptionalsPresent() {
         rewriteRun(
+          //language=java
           java(
             """
               import java.util.Optional;
@@ -678,4 +698,28 @@ void method() {
           )
         );
     }
+
+    @Test
+    void doNothingIfIsPresentOnMethodInvocation() {
+        rewriteRun(
+          //language=java
+          java(
+            """
+              import java.util.Optional;
+
+              public class Foo {
+                  public static void main(String[] args) {
+                      if (next().isPresent()) {
+                          System.out.println("Message: " + next().get());
+                      }
+                  }
+                  private static Optional<String> next() {
+                      // not guaranteed to return the same value every time, so best not to change above code
+                      return Optional.of("foo");
+                  }
+              }
+              """
+          )
+        );
+    }
 }