Fixes for type attribution and TypeUtils#isAssignableTo() (#4427)

* Fixes for type attribution and `TypeUtils#isAssignableTo()` Try to fix #4423 by correcting the type attribution for some generics and the corresponding type assignability check. Fixes: #4423 * Better type attribution for `Type.CapturedType` * Replicate code changes to Java 8, 11 and 21 --------- Co-authored-by: Tim te Beek <[email protected]>
openrewrite · Aug 21, 2024 · a8f51ac · a8f51ac
1 parent 4404c55
commit a8f51ac
Show file tree

Hide file tree

Showing 10 changed files with 104 additions and 14 deletions.
diff --git a/rewrite-java-11/src/main/java/org/openrewrite/java/isolated/ReloadableJava11TypeMapping.java b/rewrite-java-11/src/main/java/org/openrewrite/java/isolated/ReloadableJava11TypeMapping.java
@@ -196,7 +196,12 @@ private JavaType.GenericTypeVariable generic(Type.WildcardType wildcard, String
     }
 
     private JavaType generic(Type.TypeVar type, String signature) {
-        String name = type.tsym.name.toString();
+        String name;
+        if (type instanceof Type.CapturedType && ((Type.CapturedType) type).wildcard.kind == BoundKind.UNBOUND) {
+            name = "?";
+        } else {
+            name = type.tsym.name.toString();
+        }
         JavaType.GenericTypeVariable gtv = new JavaType.GenericTypeVariable(null,
                 name, INVARIANT, null);
         typeCache.put(signature, gtv);

diff --git a/...-11/src/main/java/org/openrewrite/java/isolated/ReloadableJava11TypeSignatureBuilder.java b/...-11/src/main/java/org/openrewrite/java/isolated/ReloadableJava11TypeSignatureBuilder.java
@@ -15,6 +15,7 @@
  */
 package org.openrewrite.java.isolated;
 
+import com.sun.tools.javac.code.BoundKind;
 import com.sun.tools.javac.code.Symbol;
 import com.sun.tools.javac.code.Type;
 import com.sun.tools.javac.code.TypeTag;
@@ -51,7 +52,7 @@ private String signature(@Nullable Type type) {
                 return ((Type.ClassType) type).typarams_field != null && ((Type.ClassType) type).typarams_field.length() > 0 ? parameterizedSignature(type) : classSignature(type);
             }
         } else if (type instanceof Type.CapturedType) { // CapturedType must be evaluated before TypeVar
-            return signature(((Type.CapturedType) type).wildcard);
+            return genericSignature(type);
         } else if (type instanceof Type.TypeVar) {
             return genericSignature(type);
         } else if (type instanceof Type.JCPrimitiveType) {
@@ -140,7 +141,12 @@ public String classSignature(Object type) {
     @Override
     public String genericSignature(Object type) {
         Type.TypeVar generic = (Type.TypeVar) type;
-        String name = generic.tsym.name.toString();
+        String name;
+        if (generic instanceof Type.CapturedType && ((Type.CapturedType) generic).wildcard.kind == BoundKind.UNBOUND) {
+            name = "?";
+        } else {
+            name = generic.tsym.name.toString();
+        }
 
         if (typeVariableNameStack == null) {
             typeVariableNameStack = new HashSet<>();

diff --git a/rewrite-java-17/src/main/java/org/openrewrite/java/isolated/ReloadableJava17TypeMapping.java b/rewrite-java-17/src/main/java/org/openrewrite/java/isolated/ReloadableJava17TypeMapping.java
@@ -193,7 +193,12 @@ private JavaType.GenericTypeVariable generic(Type.WildcardType wildcard, String
     }
 
     private JavaType generic(Type.TypeVar type, String signature) {
-        String name = type.tsym.name.toString();
+        String name;
+        if (type instanceof Type.CapturedType && ((Type.CapturedType) type).wildcard.kind == BoundKind.UNBOUND) {
+            name = "?";
+        } else {
+            name = type.tsym.name.toString();
+        }
         JavaType.GenericTypeVariable gtv = new JavaType.GenericTypeVariable(null,
                 name, INVARIANT, null);
         typeCache.put(signature, gtv);

diff --git a/...-17/src/main/java/org/openrewrite/java/isolated/ReloadableJava17TypeSignatureBuilder.java b/...-17/src/main/java/org/openrewrite/java/isolated/ReloadableJava17TypeSignatureBuilder.java
@@ -15,6 +15,7 @@
  */
 package org.openrewrite.java.isolated;
 
+import com.sun.tools.javac.code.BoundKind;
 import com.sun.tools.javac.code.Symbol;
 import com.sun.tools.javac.code.Type;
 import com.sun.tools.javac.code.TypeTag;
@@ -51,7 +52,7 @@ private String signature(@Nullable Type type) {
                 return ((Type.ClassType) type).typarams_field != null && ((Type.ClassType) type).typarams_field.length() > 0 ? parameterizedSignature(type) : classSignature(type);
             }
         } else if (type instanceof Type.CapturedType) { // CapturedType must be evaluated before TypeVar
-            return signature(((Type.CapturedType) type).wildcard);
+            return genericSignature(type);
         } else if (type instanceof Type.TypeVar) {
             return genericSignature(type);
         } else if (type instanceof Type.JCPrimitiveType) {
@@ -140,7 +141,12 @@ public String classSignature(Object type) {
     @Override
     public String genericSignature(Object type) {
         Type.TypeVar generic = (Type.TypeVar) type;
-        String name = generic.tsym.name.toString();
+        String name;
+        if (generic instanceof Type.CapturedType && ((Type.CapturedType) generic).wildcard.kind == BoundKind.UNBOUND) {
+            name = "?";
+        } else {
+            name = generic.tsym.name.toString();
+        }
 
         if (typeVariableNameStack == null) {
             typeVariableNameStack = new HashSet<>();

diff --git a/rewrite-java-21/src/main/java/org/openrewrite/java/isolated/ReloadableJava21TypeMapping.java b/rewrite-java-21/src/main/java/org/openrewrite/java/isolated/ReloadableJava21TypeMapping.java
@@ -47,7 +47,7 @@ class ReloadableJava21TypeMapping implements JavaTypeMapping<Tree> {
 
     public JavaType type(com.sun.tools.javac.code.@Nullable Type type) {
         if (type == null || type instanceof Type.ErrorType || type instanceof Type.PackageType || type instanceof Type.UnknownType ||
-                type instanceof NullType) {
+            type instanceof NullType) {
             return JavaType.Class.Unknown.getInstance();
         }
 
@@ -211,7 +211,12 @@ private JavaType.GenericTypeVariable generic(Type.WildcardType wildcard, String
     }
 
     private JavaType generic(Type.TypeVar type, String signature) {
-        String name = type.tsym.name.toString();
+        String name;
+        if (type instanceof Type.CapturedType && ((Type.CapturedType) type).wildcard.kind == BoundKind.UNBOUND) {
+            name = "?";
+        } else {
+            name = type.tsym.name.toString();
+        }
         JavaType.GenericTypeVariable gtv = new JavaType.GenericTypeVariable(null,
                 name, INVARIANT, null);
         typeCache.put(signature, gtv);

diff --git a/...-21/src/main/java/org/openrewrite/java/isolated/ReloadableJava21TypeSignatureBuilder.java b/...-21/src/main/java/org/openrewrite/java/isolated/ReloadableJava21TypeSignatureBuilder.java
@@ -15,6 +15,7 @@
  */
 package org.openrewrite.java.isolated;
 
+import com.sun.tools.javac.code.BoundKind;
 import com.sun.tools.javac.code.Symbol;
 import com.sun.tools.javac.code.Type;
 import com.sun.tools.javac.code.TypeTag;
@@ -51,7 +52,7 @@ private String signature(@Nullable Type type) {
                 return ((Type.ClassType) type).typarams_field != null && ((Type.ClassType) type).typarams_field.length() > 0 ? parameterizedSignature(type) : classSignature(type);
             }
         } else if (type instanceof Type.CapturedType) { // CapturedType must be evaluated before TypeVar
-            return signature(((Type.CapturedType) type).wildcard);
+            return genericSignature(type);
         } else if (type instanceof Type.TypeVar) {
             return genericSignature(type);
         } else if (type instanceof Type.JCPrimitiveType) {
@@ -140,7 +141,12 @@ public String classSignature(Object type) {
     @Override
     public String genericSignature(Object type) {
         Type.TypeVar generic = (Type.TypeVar) type;
-        String name = generic.tsym.name.toString();
+        String name;
+        if (generic instanceof Type.CapturedType && ((Type.CapturedType) generic).wildcard.kind == BoundKind.UNBOUND) {
+            name = "?";
+        } else {
+            name = generic.tsym.name.toString();
+        }
 
         if (typeVariableNameStack == null) {
             typeVariableNameStack = new HashSet<>();

diff --git a/rewrite-java-8/src/main/java/org/openrewrite/java/ReloadableJava8TypeMapping.java b/rewrite-java-8/src/main/java/org/openrewrite/java/ReloadableJava8TypeMapping.java
@@ -197,7 +197,12 @@ private JavaType.GenericTypeVariable generic(Type.WildcardType wildcard, String
     }
 
     private JavaType generic(Type.TypeVar type, String signature) {
-        String name = type.tsym.name.toString();
+        String name;
+        if (type instanceof Type.CapturedType && ((Type.CapturedType) type).wildcard.kind == BoundKind.UNBOUND) {
+            name = "?";
+        } else {
+            name = type.tsym.name.toString();
+        }
         JavaType.GenericTypeVariable gtv = new JavaType.GenericTypeVariable(null,
                 name, INVARIANT, null);
         typeCache.put(signature, gtv);

diff --git a/rewrite-java-8/src/main/java/org/openrewrite/java/ReloadableJava8TypeSignatureBuilder.java b/rewrite-java-8/src/main/java/org/openrewrite/java/ReloadableJava8TypeSignatureBuilder.java
@@ -15,6 +15,7 @@
  */
 package org.openrewrite.java;
 
+import com.sun.tools.javac.code.BoundKind;
 import com.sun.tools.javac.code.Symbol;
 import com.sun.tools.javac.code.Type;
 import com.sun.tools.javac.code.TypeTag;
@@ -49,8 +50,8 @@ private String signature(@Nullable Type type) {
             } catch (Symbol.CompletionFailure ignored) {
                 return ((Type.ClassType) type).typarams_field != null && ((Type.ClassType) type).typarams_field.length() > 0 ? parameterizedSignature(type) : classSignature(type);
             }
-        } else if (type instanceof Type.CapturedType) {  // CapturedType must be evaluated before TypeVar
-            return signature(((Type.CapturedType) type).wildcard);
+        } else if (type instanceof Type.CapturedType) { // CapturedType must be evaluated before TypeVar
+            return genericSignature(type);
         } else if (type instanceof Type.TypeVar) {
             return genericSignature(type);
         } else if (type instanceof Type.JCPrimitiveType) {
@@ -139,7 +140,12 @@ public String classSignature(Object type) {
     @Override
     public String genericSignature(Object type) {
         Type.TypeVar generic = (Type.TypeVar) type;
-        String name = generic.tsym.name.toString();
+        String name;
+        if (generic instanceof Type.CapturedType && ((Type.CapturedType) generic).wildcard.kind == BoundKind.UNBOUND) {
+            name = "?";
+        } else {
+            name = generic.tsym.name.toString();
+        }
 
         if (typeVariableNameStack == null) {
             typeVariableNameStack = new HashSet<>();

diff --git a/rewrite-java-test/src/test/java/org/openrewrite/java/JavaTemplateMatchTest.java b/rewrite-java-test/src/test/java/org/openrewrite/java/JavaTemplateMatchTest.java
@@ -18,6 +18,7 @@
 import org.junit.jupiter.api.Test;
 import org.openrewrite.DocumentExample;
 import org.openrewrite.ExecutionContext;
+import org.openrewrite.Issue;
 import org.openrewrite.java.tree.Expression;
 import org.openrewrite.java.tree.J;
 import org.openrewrite.marker.SearchResult;
@@ -28,6 +29,48 @@
 
 class JavaTemplateMatchTest implements RewriteTest {
 
+    @Test
+    @Issue("https://github.com/openrewrite/rewrite-templating/pull/91")
+    void shouldMatchAbstractStringAssertIsEqualToEmptyString() {
+        rewriteRun(
+          spec -> spec
+            .parser(JavaParser.fromJavaVersion().classpath("assertj-core"))
+            .recipe(toRecipe(() -> new JavaIsoVisitor<>() {
+                // Mimics what we saw in rewrite-templating
+                final JavaTemplate before = JavaTemplate
+                  .builder("#{stringAssert:any(org.assertj.core.api.AbstractStringAssert<?>)}.isEqualTo(\"\");")
+                  .javaParser(JavaParser.fromJavaVersion().classpath(JavaParser.runtimeClasspath()))
+                  .build();
+
+                @Override
+                public J.MethodInvocation visitMethodInvocation(J.MethodInvocation method, ExecutionContext ctx) {
+                    J.MethodInvocation mi = super.visitMethodInvocation(method, ctx);
+                    return before.matches(getCursor()) ? SearchResult.found(mi) : mi;
+                }
+            })),
+          //language=java
+          java(
+            """
+              import static org.assertj.core.api.Assertions.assertThat;
+              class Foo {
+                  void test() {
+                      assertThat("foo").isEqualTo("");
+                  }
+              }
+              """,
+            """
+              import static org.assertj.core.api.Assertions.assertThat;
+              class Foo {
+                  void test() {
+                      /*~~>*/assertThat("foo").isEqualTo("");
+                  }
+              }
+              """
+          )
+        );
+
+    }
+
     @DocumentExample
     @SuppressWarnings({"ConstantValue", "ConstantConditions"})
     @Test

diff --git a/rewrite-java/src/main/java/org/openrewrite/java/tree/TypeUtils.java b/rewrite-java/src/main/java/org/openrewrite/java/tree/TypeUtils.java
@@ -227,6 +227,9 @@ public static boolean isAssignableTo(@Nullable JavaType to, @Nullable JavaType f
                 }
                 return !(from instanceof JavaType.GenericTypeVariable) && isAssignableTo(toFq.getFullyQualifiedName(), from);
             } else if (to instanceof JavaType.GenericTypeVariable) {
+                if (from instanceof JavaType.GenericTypeVariable && isOfType(to, from)) {
+                    return true;
+                }
                 JavaType.GenericTypeVariable toGeneric = (JavaType.GenericTypeVariable) to;
                 List<JavaType> toBounds = toGeneric.getBounds();
                 if (!toGeneric.getName().equals("?")) {