top-level function declaration type fixes. #339
Conversation
| @@ -58,15 +58,17 @@ import org.openrewrite.kotlin.KotlinTypeSignatureBuilder.Companion.convertFileNa | |||
| import org.openrewrite.kotlin.KotlinTypeSignatureBuilder.Companion.convertKotlinFqToJavaFq | |||
|
|
|||
| @Incubating(since = "0.0") | |||
| class KotlinTypeMapping(typeCache: JavaTypeCache, firSession: FirSession) : JavaTypeMapping<Any> { | |||
| class KotlinTypeMapping(typeCache: JavaTypeCache, firSession: FirSession, firFileSymbol: FirFileSymbol) : JavaTypeMapping<Any> { | |||
There was a problem hiding this comment.
The ownerStack will be wrong without access to the file when a SimpleFunction is on the top of the stack for the parameters in a top-level method declaration.
| @@ -546,7 +550,7 @@ class KotlinTypeMapping(typeCache: JavaTypeCache, firSession: FirSession) : Java | |||
| } | |||
| } | |||
| if (resolvedDeclaringType == null) { | |||
| return null | |||
| resolvedDeclaringType = TypeUtils.asFullyQualified(type(firFileSymbol.fir)) | |||
There was a problem hiding this comment.
@knutwannheden I needed to figure out where we should fall back to the FirFileSymbol. This "could" technically set the incorrect owner if we don't assign the owner appropriately, but it also isolates incorrect types to each file.
Curious, what do you think about falling back onto the FirFileSymbol by default?
There was a problem hiding this comment.
Curious, what do you think about falling back onto the FirFileSymbol by default?
I think that sounds reasonable. I wanted to do something similar myself, but first wanted to discuss that with you. So instead I added this owner stack, which is easy enough to remove again.
| @@ -882,6 +886,8 @@ class KotlinTypeMapping(typeCache: JavaTypeCache, firSession: FirSession) : Java | |||
| // TODO: fix type attribution for anonymous objects. Currently, returns JavaType.Unknown. | |||
| if (symbol.dispatchReceiverType != null) { | |||
| resolvedOwner = type(symbol.dispatchReceiverType) | |||
| } else if (ownerFallBack is FirFunctionSymbol<*>) { | |||
| resolvedOwner = methodDeclarationType(ownerFallBack.fir, null, firFileSymbol) | |||
There was a problem hiding this comment.
The signature is set correctly to the method declaration signature. This change fixes the JavaType returned by the signature.
Added FirFileSymbol as field to SignatureBuilder and TypeMapping. Fixed owner of top-level method declarations of method parameters. Fixed owner type of method parameters.
traceyyoshima
force-pushed
the
top-level-method-type-fixes
branch
from
c9d5124 to
adacc44
Compare
| public J.MethodInvocation visitMethodInvocation(J.MethodInvocation method, AtomicBoolean atomicBoolean) { | ||
| if (methodMatcher.matches(method)) { | ||
| assertThat(method.getMethodType().toString()) | ||
| .isEqualTo("kotlin.collections.CollectionsKt{name=listOf,return=kotlin.collections.List<kotlin.Pair<kotlin.String, Generic{kotlin.Comparable<Generic{*}> & java.io.Serializable}>>,parameters=[kotlin.Array<Generic{? extends Generic{T}}>]}"); |
There was a problem hiding this comment.
Just so you know -- In Java, a generic type intersection contains the bounded generic name like T, V, U. So the type becomes Generic{<name> extends <typeA> & <typeB>}.
In Kotlin, the ConeIntersectionType contains a generic name it and appears as it(<typeA>, <typeB>). So, I left the name out. It's better if we add the name of the Generic type variable. So, I'll check on that tomorrow.
Changes:
?