Skip to content

Commit 6a1ac1b

Browse files
authored
Kotlin: remap builtins on Java-origin method/field types (#7428)
* Kotlin: remap builtins on method parameter, return, and variable types `KotlinTypeMapping` applied `remapKotlinBuiltin` to supertypes, interfaces, generic bounds, and annotations in #7364, but not to method parameter types, return types, receiver types, or field types. A Kotlin `IllegalArgumentException("msg", cause)` therefore produced a constructor whose parameter types were `kotlin.String` / `kotlin.Throwable`, so a `MethodMatcher` written against the Java FQNs (`java.lang.String` / `java.lang.Throwable`) failed to match on Kotlin sources even though the bytecode signature is identical. Wire `remapKotlinBuiltin` through both `methodDeclarationType` and `methodInvocationType` (parameter types, return type, receiver) and through `variableType` (field/property type). * Scope Kotlin builtin remap to Java-origin classes Following review on #7428, only remap kotlin.* parameter/return/field types when the declaring class is a FirJavaClass (third-party Java libraries on the classpath). Kotlin-declared signatures — `kotlin.io.ConsoleKt.println(kotlin.Any)`, user properties of type `String`, etc. — keep their author-intended Kotlin builtin names. The motivating case (#7427) is Jackson's JsonGenerationException, which is FirJavaClass; the test now uses that directly. JDK classes go through Kotlin's classfile loader as something other than FirJavaClass and are out of scope here — recipes that target JDK signatures over Kotlin code need a Kotlin-aware MethodMatcher. * Use reduce() to assign found in new test
1 parent 0b1667c commit 6a1ac1b

2 files changed

Lines changed: 85 additions & 8 deletions

File tree

rewrite-kotlin/src/main/kotlin/org/openrewrite/kotlin/KotlinTypeMapping.kt

Lines changed: 47 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ import org.jetbrains.kotlin.fir.declarations.utils.modality
3333
import org.jetbrains.kotlin.fir.declarations.utils.visibility
3434
import org.jetbrains.kotlin.fir.expressions.*
3535
import org.jetbrains.kotlin.fir.expressions.impl.FirResolvedArgumentList
36+
import org.jetbrains.kotlin.fir.java.declarations.FirJavaClass
3637
import org.jetbrains.kotlin.fir.java.declarations.FirJavaField
3738
import org.jetbrains.kotlin.fir.references.FirErrorNamedReference
3839
import org.jetbrains.kotlin.fir.references.FirResolvedNamedReference
@@ -702,7 +703,15 @@ class KotlinTypeMapping(
702703
parentType = parentType.type
703704
}
704705
val resolvedDeclaringType = TypeUtils.asFullyQualified(parentType)
705-
var returnType = type(function.returnTypeRef)
706+
// Only remap kotlin.* builtins to JVM FQNs for methods declared on Java-origin
707+
// classes (third-party Java libraries loaded as FirJavaClass). The Kotlin compiler's
708+
// FIR renders a Java method's `String` parameter as `kotlin.String` even though the
709+
// bytecode signature is `java.lang.String`; remapping puts the parser's output back
710+
// in line with the JVM signature so MethodMatcher patterns written against Java
711+
// FQNs match. Kotlin-declared methods are left alone — `kotlin.Any` there is the
712+
// author's intent, and Kotlin-aware matching belongs in KotlinTypeUtils.
713+
val javaOrigin = parent is FirJavaClass
714+
var returnType = if (javaOrigin) remapKotlinBuiltin(type(function.returnTypeRef)) else type(function.returnTypeRef)
706715
// Java parser uses the raw Class for a constructor's returnType, not the
707716
// class's parameterized form. Kotlin's FIR renders a constructor's
708717
// `returnTypeRef` as the class instantiated with its own type parameters
@@ -719,13 +728,14 @@ class KotlinTypeMapping(
719728
else -> null
720729
}
721730
if (function.receiverParameter != null) {
722-
parameterTypes!!.add(type(function.receiverParameter!!.typeRef))
731+
val rt = type(function.receiverParameter!!.typeRef)
732+
parameterTypes!!.add(if (javaOrigin) remapKotlinBuiltin(rt)!! else rt)
723733
}
724734
if (function.valueParameters.isNotEmpty()) {
725735
for (p in function.valueParameters) {
726736
val t = type(p.returnTypeRef, function)
727737
if (t != null) {
728-
parameterTypes!!.add(t)
738+
parameterTypes!!.add(if (javaOrigin) remapKotlinBuiltin(t)!! else t)
729739
}
730740
}
731741
}
@@ -971,10 +981,20 @@ class KotlinTypeMapping(
971981
if (declaringType == null) {
972982
declaringType = TypeUtils.asFullyQualified(type(firFile))
973983
}
974-
val returnType = type(function.resolvedType)
984+
// See methodDeclarationType: only remap for Java-origin methods so Kotlin-declared
985+
// signatures (e.g. `kotlin.io.ConsoleKt.println(kotlin.Any)`) are preserved.
986+
// The discriminator is the containing class FIR — Java-origin classes (third-party
987+
// Java libraries on the classpath) are loaded as FirJavaClass even when the
988+
// synthesized member FIR is FirConstructorImpl with an Enhancement origin. JDK
989+
// classes are special-cased by Kotlin's classfile loader and aren't FirJavaClass —
990+
// recipes targeting JDK signatures need a Kotlin-aware matcher instead.
991+
val javaOrigin = (sym as? FirCallableSymbol<*>)
992+
?.containingClassLookupTag()?.toRegularClassSymbol(firSession)?.fir is FirJavaClass
993+
val returnType = if (javaOrigin) remapKotlinBuiltin(type(function.resolvedType)) else type(function.resolvedType)
975994

976995
if (function.toResolvedCallableSymbol()?.receiverParameterSymbol != null) {
977-
paramTypes!!.add(type(function.toResolvedCallableSymbol()?.receiverParameterSymbol!!.fir.typeRef))
996+
val rt = type(function.toResolvedCallableSymbol()?.receiverParameterSymbol!!.fir.typeRef)
997+
paramTypes!!.add(if (javaOrigin) remapKotlinBuiltin(rt)!! else rt)
978998
}
979999
// Build a mapping from parameter name to its corresponding argument expression
9801000
val paramToArg: Map<String, FirExpression>? =
@@ -991,12 +1011,13 @@ class KotlinTypeMapping(
9911011
if (t is GenericTypeVariable) {
9921012
val arg = paramToArg?.get(p.name.asString())
9931013
if (arg != null) {
994-
paramTypes.add(type(arg.resolvedType, function)!!)
1014+
val argType = type(arg.resolvedType, function)!!
1015+
paramTypes.add(if (javaOrigin) remapKotlinBuiltin(argType)!! else argType)
9951016
} else {
9961017
paramTypes.add(t)
9971018
}
9981019
} else {
999-
paramTypes.add(t)
1020+
paramTypes.add(if (javaOrigin) remapKotlinBuiltin(t)!! else t)
10001021
}
10011022
}
10021023
method.unsafeSet(
@@ -1112,6 +1133,18 @@ class KotlinTypeMapping(
11121133
return type.fqName.asString()
11131134
}
11141135

1136+
/**
1137+
* Convenience overload: apply [remapKotlinBuiltin] to any [JavaType], returning the
1138+
* input unchanged when it is not a [FullyQualified]. Callers use this on method
1139+
* parameter, receiver, return, and field types so Kotlin builtins (kotlin.String /
1140+
* kotlin.Throwable / ...) surface as the JVM FQN the Java parser would produce.
1141+
*/
1142+
private fun remapKotlinBuiltin(t: JavaType?): JavaType? {
1143+
if (t == null) return null
1144+
val fq = TypeUtils.asFullyQualified(t)
1145+
return if (fq != null) remapKotlinBuiltin(fq) else t
1146+
}
1147+
11151148
private fun remapKotlinBuiltin(fq: FullyQualified): FullyQualified {
11161149
// For a Parameterized (e.g. kotlin.Enum<Foo>) remap the underlying raw type but
11171150
// keep the parameterization so we don't lose type argument information.
@@ -1277,7 +1310,13 @@ class KotlinTypeMapping(
12771310
declaringType = declaringType.type
12781311
}
12791312

1280-
val typeRef = type(variable.returnTypeRef)
1313+
// See methodDeclarationType: only remap for Java-origin variables (e.g. fields read
1314+
// from a Java class file) so Kotlin properties keep their author-intended types.
1315+
val typeRef = if (variable is FirJavaField) {
1316+
remapKotlinBuiltin(type(variable.returnTypeRef))
1317+
} else {
1318+
type(variable.returnTypeRef)
1319+
}
12811320
vt.unsafeSet(declaringType!!, typeRef, annotations)
12821321
return vt
12831322
}

rewrite-kotlin/src/test/java/org/openrewrite/kotlin/KotlinTypeMappingTest.java

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -547,6 +547,44 @@ public J.MethodInvocation visitMethodInvocation(J.MethodInvocation method, Atomi
547547
);
548548
}
549549

550+
@Issue("https://github.com/openrewrite/rewrite/issues/7427")
551+
@Test
552+
void constructorParameterTypesRemapKotlinBuiltinsForJavaOrigin() {
553+
// A Kotlin call into a Java-origin constructor taking `String` / `Throwable`
554+
// must surface parameter types as the JVM `java.lang.String` /
555+
// `java.lang.Throwable`, not the Kotlin builtins, so `MethodMatcher` patterns
556+
// written against the Java FQN match. Jackson's JsonGenerationException is the
557+
// motivating case from #7427. Kotlin-declared signatures are intentionally
558+
// left as-is — see #7428 review discussion.
559+
rewriteRun(
560+
kotlin(
561+
"""
562+
import com.fasterxml.jackson.core.JsonGenerationException
563+
import com.fasterxml.jackson.core.JsonGenerator
564+
565+
fun example(gen: JsonGenerator, cause: Throwable) {
566+
throw JsonGenerationException("message", cause, gen)
567+
}
568+
""",
569+
spec -> spec.afterRecipe(cu -> {
570+
var matcher = new MethodMatcher("com.fasterxml.jackson.core.JsonGenerationException <constructor>(String, Throwable, com.fasterxml.jackson.core.JsonGenerator)");
571+
AtomicBoolean found = new KotlinIsoVisitor<AtomicBoolean>() {
572+
@Override
573+
public J.NewClass visitNewClass(J.NewClass newClass, AtomicBoolean found) {
574+
var paramTypes = newClass.getConstructorType().getParameterTypes();
575+
assertThat(paramTypes.get(0).toString()).isEqualTo("java.lang.String");
576+
assertThat(paramTypes.get(1).toString()).isEqualTo("java.lang.Throwable");
577+
assertThat(matcher.matches(newClass)).isTrue();
578+
found.set(true);
579+
return super.visitNewClass(newClass, found);
580+
}
581+
}.reduce(cu, new AtomicBoolean());
582+
assertThat(found.get()).isTrue();
583+
})
584+
)
585+
);
586+
}
587+
550588
@Test
551589
void whenExpression() {
552590
rewriteRun(

0 commit comments

Comments
 (0)