From 81341a8acebf19160e7add46561f73df08f8c5f8 Mon Sep 17 00:00:00 2001
From: Knut Wannheden <knut@moderne.io>
Date: Tue, 5 Nov 2024 08:55:34 +0100
Subject: [PATCH] `ReplaceClassIsInstanceWithInstanceof`: Fix another corner
 case

---
 .../ReplaceClassIsInstanceWithInstanceof.java | 27 +++++++++++--------
 ...laceClassIsInstanceWithInstanceofTest.java | 17 ++++++++++++
 2 files changed, 33 insertions(+), 11 deletions(-)

diff --git a/src/main/java/org/openrewrite/staticanalysis/ReplaceClassIsInstanceWithInstanceof.java b/src/main/java/org/openrewrite/staticanalysis/ReplaceClassIsInstanceWithInstanceof.java
index 716416ab..49f95d8a 100644
--- a/src/main/java/org/openrewrite/staticanalysis/ReplaceClassIsInstanceWithInstanceof.java
+++ b/src/main/java/org/openrewrite/staticanalysis/ReplaceClassIsInstanceWithInstanceof.java
@@ -17,20 +17,28 @@
 
 import org.jspecify.annotations.Nullable;
 import org.openrewrite.ExecutionContext;
+import org.openrewrite.Preconditions;
 import org.openrewrite.Recipe;
+import org.openrewrite.TreeVisitor;
 import org.openrewrite.java.JavaTemplate;
 import org.openrewrite.java.JavaVisitor;
 import org.openrewrite.java.MethodMatcher;
-import org.openrewrite.java.tree.*;
+import org.openrewrite.java.search.UsesMethod;
+import org.openrewrite.java.tree.Expression;
+import org.openrewrite.java.tree.J;
 import org.openrewrite.java.tree.J.FieldAccess;
 import org.openrewrite.java.tree.J.Identifier;
 import org.openrewrite.java.tree.J.MethodInvocation;
+import org.openrewrite.java.tree.JavaCoordinates;
+import org.openrewrite.java.tree.JavaType;
 
 import java.util.Collections;
 import java.util.Set;
 
 public class ReplaceClassIsInstanceWithInstanceof extends Recipe {
 
+    private static final MethodMatcher ISINSTANCE_MATCHER = new MethodMatcher("java.lang.Class isInstance(..)");
+
     @Override
     public String getDisplayName() {
         return "Replace `A.class.isInstance(a)` with `a instanceof A`";
@@ -47,25 +55,22 @@ public Set<String> getTags() {
     }
 
     @Override
-    public JavaVisitor<ExecutionContext> getVisitor() {
+    public TreeVisitor<?, ExecutionContext> getVisitor() {
         // use JavaVisitor instead of JavaIsoVisitor because we changed the type of LST
-        return new JavaVisitor<ExecutionContext>() {
-
-            private final MethodMatcher matcher = new MethodMatcher("java.lang.Class isInstance(java.lang.Object)");
+        return Preconditions.check(new UsesMethod<>(ISINSTANCE_MATCHER), new JavaVisitor<ExecutionContext>() {
 
             @Override
             public J visitMethodInvocation(MethodInvocation method, ExecutionContext ctx) {
                 // make sure we find the right method and the left part is something like "SomeClass.class"
-                if (matcher.matches(method) && isObjectClass(method.getSelect())) {
+                if (ISINSTANCE_MATCHER.matches(method) && isObjectClass(method.getSelect())) {
                     // for code like "A.class.isInstance(a)", select is "String.class", name is "isInstance", argument is "a"
                     Expression objectExpression = method.getArguments().get(0);
                     FieldAccess fieldAccessPart = (FieldAccess) method.getSelect();
-                    String className = fieldAccessPart.getTarget().toString();
                     // upcast to type J, so J.MethodInvocation can be replaced by J.InstanceOf
                     JavaCoordinates coordinates = method.getCoordinates().replace();
-                    J.InstanceOf instanceOf = JavaTemplate.builder("#{any()} instanceof #{}")
+                    J.InstanceOf instanceOf = JavaTemplate.builder("#{any()} instanceof Object")
                             .build()
-                            .apply(getCursor(), coordinates, new Object[]{objectExpression, className});
+                            .apply(getCursor(), coordinates, objectExpression);
                     instanceOf = instanceOf.withClazz(fieldAccessPart.getTarget().withPrefix(instanceOf.getClazz().getPrefix()));
                     return maybeAutoFormat(method, instanceOf, ctx);
                 }
@@ -75,13 +80,13 @@ public J visitMethodInvocation(MethodInvocation method, ExecutionContext ctx) {
             private boolean isObjectClass(@Nullable Expression expression) {
                 if (expression instanceof J.FieldAccess) {
                     J.FieldAccess fieldAccess = (J.FieldAccess) expression;
-                    if (fieldAccess.getTarget() instanceof Identifier) {
+                    if (fieldAccess.getName().getSimpleName().equals("class") && fieldAccess.getTarget() instanceof Identifier) {
                         Identifier identifier = (Identifier) fieldAccess.getTarget();
                         return identifier.getType() instanceof JavaType.Class;
                     }
                 }
                 return false;
             }
-        };
+        });
     }
 }
diff --git a/src/test/java/org/openrewrite/staticanalysis/ReplaceClassIsInstanceWithInstanceofTest.java b/src/test/java/org/openrewrite/staticanalysis/ReplaceClassIsInstanceWithInstanceofTest.java
index 221caf5b..292bc3ff 100644
--- a/src/test/java/org/openrewrite/staticanalysis/ReplaceClassIsInstanceWithInstanceofTest.java
+++ b/src/test/java/org/openrewrite/staticanalysis/ReplaceClassIsInstanceWithInstanceofTest.java
@@ -265,4 +265,21 @@ boolean foo(Object obj) {
         );
     }
 
+    @Test
+    void typeVariable() {
+        rewriteRun(
+          //language=java
+          java(
+            """
+              class A {
+                  Class<?> clazz;
+                  boolean foo(Object obj) {
+                      return this.clazz.isInstance(obj);
+                  }
+              }
+              """
+          )
+        );
+    }
+
 }