Kotlin: remap builtins on Java-origin method/field types#7428
Kotlin: remap builtins on Java-origin method/field types#7428
Conversation
`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).
| } | ||
| """, | ||
| spec -> spec.afterRecipe(cu -> { | ||
| var matcher = new MethodMatcher("java.lang.IllegalArgumentException <constructor>(String, Throwable)"); |
There was a problem hiding this comment.
The problem we saw was specifically with MethodMatcher not being KotlinTypeUtils aware, and hence seeing Kotlin types here instead of their Java equivalents.
| public J.MethodInvocation visitMethodInvocation(J.MethodInvocation method, AtomicBoolean atomicBoolean) { | ||
| if ("println".equals(method.getSimpleName())) { | ||
| assertThat(method.getMethodType().toString()).isEqualTo("kotlin.io.ConsoleKt{name=println,return=void,parameters=[kotlin.Any]}"); | ||
| assertThat(method.getMethodType().toString()).isEqualTo("kotlin.io.ConsoleKt{name=println,return=void,parameters=[java.lang.Object]}"); |
There was a problem hiding this comment.
This is where I'm a bit doubtful; do we really want to map all Kotlin types to their Java equivalents, even for packages like kotlin.io ?
There was a problem hiding this comment.
I suppose not all Kotlin types have a direct Java equivalent. E.g. Nothing?
So I guess the answer would be no? For consistency.
There was a problem hiding this comment.
Scoped to Java-origin classes (FirJavaClass) in 97295cd. The motivating Jackson case still works; Kotlin-declared signatures like kotlin.io.ConsoleKt.println(kotlin.Any) are now preserved. JDK classes go through Kotlin's classfile loader as a different FIR type, so they're out of scope here — that'd need a Kotlin-aware MethodMatcher.
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.
Summary
KotlinTypeMappingappliedremapKotlinBuiltinto supertypes, interfaces, generic bounds, and annotations in #7364, but not to method parameter/return/receiver types or field types — so a Kotlin call to a JacksonJsonGenerationException("msg", cause, gen)constructor produced parameter typeskotlin.String/kotlin.Throwable, and aMethodMatcherwritten against the JVM FQNs failed to match. Per review, the remap is scoped to methods/fields whose declaring class isFirJavaClass(third-party Java libraries on the classpath); Kotlin-declared signatures likekotlin.io.ConsoleKt.println(kotlin.Any)and user properties keep their author-intended Kotlin builtin names. JDK classes go through Kotlin's classfile loader as a different FIR type and are out of scope here — recipes targeting JDK signatures over Kotlin code need a Kotlin-awareMethodMatcher.Test plan
./gradlew :rewrite-kotlin:testpassesconstructorParameterTypesRemapKotlinBuiltinsForJavaOrigintest asserts Jackson'sJsonGenerationException(String, Throwable, JsonGenerator)surfaces JVM FQNs and a Java-FQNMethodMatchermatchesprintln,methodInvocationWithDefaults) unchanged — they still expectkotlin.*parameter types