rewrite-kotlin icon indicating copy to clipboard operation
rewrite-kotlin copied to clipboard

Method matcher should match Java primitive types in Kotlin

Open timtebeek opened this issue 1 year ago • 3 comments

What's changed?

Add a test to check that we can match Java primitive types in Kotlin

What's your motivation?

Saw that we do not match, as the method type uses kotlin.Int, not any Java int, which we then fail to match.

image

The declaring type does contain methods with the correct types, so perhaps we should look up the matching method as opposed to trusting the association look up. https://github.com/openrewrite/rewrite-kotlin/blob/181a00d35c7acba1056c3c2f76241a1f02cafc40/src/main/java/org/openrewrite/kotlin/internal/KotlinTreeParserVisitor.java#L3652-L3654

image

timtebeek avatar Jan 11 '25 22:01 timtebeek

I've briefly explored doing a method lookup in the declared type, but it's not pretty, and not sure if such an approach will fully work:

private JavaType.@Nullable Method methodInvocationType(PsiElement psi) {
    JavaType.Method psiMethodType = psiElementAssociations.methodInvocationType(psi);

    // Look for method in the declaring type to find the correct method parameter types, not the Kotlin equivalents
    JavaType.FullyQualified declaringType = psiMethodType.getDeclaringType();
    for (JavaType.Method m : declaringType.getMethods()) {
        if (m.getName().equals(psiMethodType.getName())) {
            List<JavaType> declParameterTypes = m.getParameterTypes();
            List<JavaType> psiParameterTypes = psiMethodType.getParameterTypes();
            if (declParameterTypes.size() == psiParameterTypes.size()) {
                for (int i = 0; i < declParameterTypes.size(); i++) {
                    JavaType declType = declParameterTypes.get(i);
                    JavaType psiType = psiParameterTypes.get(i);

                    // TODO compare types JavaType.Primitive.Int and JavaType.Class kotlin.Int, and similar friends
                }
            }
        }
    }
    return psiMethodType;
}

timtebeek avatar Jan 11 '25 22:01 timtebeek

Alternatively we can map Kotlin Primitives to JavaType.Primitive, instead of the Java.Class instances they are now; That would mean recipes written for Java that might use these primitives in their method patterns would match again. We could extend this to only do so for types coming from Java if needed / desired, but there does not appear to be an easy way to tell them apart from the conflicting signals coming back on the declared type (Java classes have super type kotlin.Any for instance). Let's agree to this approach before I update the failures in https://github.com/openrewrite/rewrite-kotlin/blob/f7417f68935829fcfe865accee4124843288216d/src/test/java/org/openrewrite/kotlin/KotlinTypeMappingTest.java#L48

timtebeek avatar Jan 12 '25 16:01 timtebeek

Any thoughts as to the kotlin.Int mapping to int here @knutwannheden ? I feel that would help recipe reuse across languages..

timtebeek avatar Jan 20 '25 16:01 timtebeek