From 11c8e5f1d5a3d3e467cbb662f07410b80e89bfc4 Mon Sep 17 00:00:00 2001 From: traceyyoshima Date: Sun, 29 Oct 2023 15:04:45 -0600 Subject: [PATCH] various type attribution fixes. --- .../internal/KotlinTreeParserVisitor.java | 52 +++++++++++-------- .../openrewrite/kotlin/KotlinTypeMapping.kt | 8 ++- .../kotlin/KotlinTypeSignatureBuilder.kt | 20 ++++--- .../kotlin/internal/PsiElementAssociations.kt | 7 +-- .../kotlin/KotlinTypeMappingTest.java | 15 +++--- 5 files changed, 59 insertions(+), 43 deletions(-) diff --git a/src/main/java/org/openrewrite/kotlin/internal/KotlinTreeParserVisitor.java b/src/main/java/org/openrewrite/kotlin/internal/KotlinTreeParserVisitor.java index f398dfdc7..14668d654 100644 --- a/src/main/java/org/openrewrite/kotlin/internal/KotlinTreeParserVisitor.java +++ b/src/main/java/org/openrewrite/kotlin/internal/KotlinTreeParserVisitor.java @@ -470,11 +470,13 @@ public J visitEnumEntry(KtEnumEntry enumEntry, ExecutionContext data) { if (!enumEntry.getAnnotationEntries().isEmpty()) { mapModifiers(enumEntry.getModifierList(), annotations, emptyList(), data); } - + // TODO: in java the EnumValue has a type of JavaType.Variable with a null fieldType. J.Identifier name = createIdentifier(enumEntry.getNameIdentifier(), type(enumEntry)); J.NewClass initializer = null; if (enumEntry.getInitializerList() != null) { + // TODO: this creates an empty init with no type attribution: enum class Foo { BAR() } + // Add constructor method type. initializer = (J.NewClass) enumEntry.getInitializerList().accept(this, data); } @@ -810,7 +812,8 @@ public J visitParameter(KtParameter parameter, ExecutionContext data) { .withPrefix(prefix(parameter)); } - J.Identifier name = createIdentifier(parameter.getNameIdentifier(), type(parameter)); + JavaType.Variable vt = variableType(parameter); + J.Identifier name = createIdentifier(parameter.getNameIdentifier(), vt); if (parameter.getTypeReference() != null) { markers = markers.addIfAbsent(new TypeReferencePrefix(randomId(), prefix(parameter.getColon()))); @@ -828,7 +831,7 @@ public J visitParameter(KtParameter parameter, ExecutionContext data) { name, emptyList(), initializer, - name.getFieldType() + vt ); vars.add(padRight(namedVariable, Space.EMPTY)); @@ -1839,6 +1842,8 @@ public J visitCallExpression(KtCallExpression expression, ExecutionContext data) PsiElementAssociations.ExpressionType type = psiElementAssociations.getCallType(expression); if (type == PsiElementAssociations.ExpressionType.CONSTRUCTOR) { TypeTree name = (J.Identifier) expression.getCalleeExpression().accept(this, data); + // FIXME. The PSI may does not require type arguments on parameterized types. Check the FIR. + // psiElementAssociations.primary(expression.getCalleeExpression()) => symbol may contain type params. if (!expression.getTypeArguments().isEmpty()) { List> parameters = new ArrayList<>(expression.getTypeArguments().size()); for (KtTypeProjection ktTypeProjection : expression.getTypeArguments()) { @@ -2184,7 +2189,7 @@ public J visitDestructuringDeclaration(KtDestructuringDeclaration multiDeclarati throw new UnsupportedOperationException(); } - J.Identifier nameVar = createIdentifier(entry.getNameIdentifier(), vt != null ? vt.getType() : null, vt); + J.Identifier nameVar = createIdentifier(entry.getNameIdentifier(), vt); if (!annotations.isEmpty()) { nameVar = nameVar.withAnnotations(annotations); } else { @@ -2208,22 +2213,15 @@ public J visitDestructuringDeclaration(KtDestructuringDeclaration multiDeclarati vars.add(padRight(namedVariable, suffix(entry))); } + JavaType.Variable vt = variableType(multiDeclaration); J.VariableDeclarations.NamedVariable emptyWithInitializer = new J.VariableDeclarations.NamedVariable( randomId(), Space.EMPTY, Markers.EMPTY, - new J.Identifier( - randomId(), - Space.SINGLE_SPACE, - Markers.EMPTY, - emptyList(), - "", - variableType(multiDeclaration), - null - ), + createIdentifier("", Space.SINGLE_SPACE, vt), emptyList(), paddedInitializer, - null + vt ); J.VariableDeclarations variableDeclarations = new J.VariableDeclarations( @@ -2323,13 +2321,14 @@ public J visitDotQualifiedExpression(KtDotQualifiedExpression expression, Execut // ); } else if (expression.getSelectorExpression() instanceof KtNameReferenceExpression) { // Maybe need to type check before creating a field access. + JavaType.Variable vt = variableType(expression.getSelectorExpression()); return new J.FieldAccess( randomId(), prefix(expression), Markers.EMPTY, expression.getReceiverExpression().accept(this, data).withPrefix(Space.EMPTY), - padLeft(suffix(expression.getReceiverExpression()), (J.Identifier) expression.getSelectorExpression().accept(this, data)), - type(expression.getSelectorExpression()) // FIXME: this will not result in a type. + padLeft(suffix(expression.getReceiverExpression()), createIdentifier(expression.getSelectorExpression(), vt)), + vt ); } else { throw new UnsupportedOperationException("Unsupported dot qualified selector: " + expression.getSelectorExpression().getClass()); @@ -2358,7 +2357,7 @@ public J visitImportDirective(KtImportDirective importDirective, ExecutionContex importDirective.getNode().findChildByType(KtTokens.WHITE_SPACE), importDirective.isAllUnder() ? importDirective.getNode().findChildByType(KtTokens.MUL) : importDirective.getNode().findChildByType(KtNodeTypes.DOT_QUALIFIED_EXPRESSION)); - J reference = TypeTree.build(text); + J reference = TypeTree.build(text); // FIXME: this creates a shallow class for a resolvable type. // J reference = importDirective.getImportedReference().accept(this, data); // if (importDirective.isAllUnder()) { // reference = new J.FieldAccess( @@ -2785,16 +2784,17 @@ public J visitProperty(KtProperty property, ExecutionContext data) { maybeBeforeSemicolon = prefix(property.getLastChild()); } + JavaType.Variable vt = variableType(property); J.VariableDeclarations.NamedVariable namedVariable = new J.VariableDeclarations.NamedVariable( randomId(), prefix(property.getNameIdentifier()), Markers.EMPTY, // TODO: fix NPE. - createIdentifier(property.getNameIdentifier(), type(property)).withPrefix(Space.EMPTY), + createIdentifier(property.getNameIdentifier(), vt).withPrefix(Space.EMPTY), emptyList(), initializer, - variableType(property) + vt ); variables.add(padRight(namedVariable, maybeBeforeSemicolon).withMarkers(rpMarker)); @@ -2914,7 +2914,13 @@ public J visitPropertyDelegate(KtPropertyDelegate delegate, ExecutionContext dat @Override public J visitSimpleNameExpression(KtSimpleNameExpression expression, ExecutionContext data) { // The correct type cannot consistently be associated to the expression due to the relationship between the PSI and FIR. - return createIdentifier(expression, null); + // The parent tree should use the associated FIR to fix mis-mapped types. + // I.E. MethodInvocations, MethodDeclarations, VariableNames should be set manually through `createIdentifier(psi, type)` + J.Identifier name = createIdentifier(expression, type(expression)); + if (name.getType() instanceof JavaType.Parameterized) { + name = name.withType(((JavaType.Parameterized) name.getType()).getType()); + } + return name; } @Override @@ -3309,7 +3315,7 @@ private JavaType.Method methodInvocationType(PsiElement psi) { return psiElementAssociations.getTypeMapping().methodInvocationType((FirFunctionCall) firElement, psiElementAssociations.getFile().getSymbol()); } if (firElement instanceof FirResolvedNamedReference) { - throw new UnsupportedOperationException("FIXME"); + return psiElementAssociations.getTypeMapping().methodDeclarationType(((FirFunction) ((FirResolvedNamedReference) firElement).getResolvedSymbol().getFir()), null, psiElementAssociations.getFile().getSymbol()); } return null; } @@ -3341,7 +3347,7 @@ private J.Identifier createIdentifier(String name, Space prefix, Markers.EMPTY, emptyList(), name, - (type instanceof JavaType.Parameterized) ? ((JavaType.Parameterized) type).getType() : type , + type, fieldType ); } @@ -3562,7 +3568,7 @@ private J.VariableDeclarations mapDestructuringDeclaration(KtDestructuringDeclar name, emptyList(), null, - variableType(ktDestructuringDeclaration) + variableType(ktDestructuringDeclarationEntry) ); variables.add(padRight(namedVariable, suffix(ktDestructuringDeclarationEntry))); } diff --git a/src/main/kotlin/org/openrewrite/kotlin/KotlinTypeMapping.kt b/src/main/kotlin/org/openrewrite/kotlin/KotlinTypeMapping.kt index 747ebebf8..fc83535fa 100644 --- a/src/main/kotlin/org/openrewrite/kotlin/KotlinTypeMapping.kt +++ b/src/main/kotlin/org/openrewrite/kotlin/KotlinTypeMapping.kt @@ -117,7 +117,7 @@ class KotlinTypeMapping(typeCache: JavaTypeCache, firSession: FirSession, firFil } is FirFile -> { - return JavaType.ShallowClass.build(convertFileNameToFqn(type.name)) + return fileType(signature) } is FirJavaTypeRef -> { @@ -140,6 +140,12 @@ class KotlinTypeMapping(typeCache: JavaTypeCache, firSession: FirSession, firFil } } + private fun fileType(signature: String): JavaType? { + val fileType = JavaType.ShallowClass.build(signature) + typeCache.put(signature, fileType) + return fileType + } + @OptIn(SymbolInternals::class) private fun resolveType( type: Any, diff --git a/src/main/kotlin/org/openrewrite/kotlin/KotlinTypeSignatureBuilder.kt b/src/main/kotlin/org/openrewrite/kotlin/KotlinTypeSignatureBuilder.kt index 3ef4e9711..11c7ef43b 100644 --- a/src/main/kotlin/org/openrewrite/kotlin/KotlinTypeSignatureBuilder.kt +++ b/src/main/kotlin/org/openrewrite/kotlin/KotlinTypeSignatureBuilder.kt @@ -24,6 +24,7 @@ import org.jetbrains.kotlin.fir.getOwnerLookupTag import org.jetbrains.kotlin.fir.java.declarations.FirJavaField import org.jetbrains.kotlin.fir.java.declarations.FirJavaMethod import org.jetbrains.kotlin.fir.java.declarations.FirJavaValueParameter +import org.jetbrains.kotlin.fir.packageFqName import org.jetbrains.kotlin.fir.references.FirResolvedNamedReference import org.jetbrains.kotlin.fir.resolve.inference.ConeTypeParameterBasedTypeVariable import org.jetbrains.kotlin.fir.resolve.toFirRegularClass @@ -82,7 +83,7 @@ class KotlinTypeSignatureBuilder(private val firSession: FirSession, private val } is FirFile -> { - return convertFileNameToFqn(type.name) + return convertFileNameToFqn(type) } is FirJavaTypeRef -> { @@ -575,11 +576,11 @@ class KotlinTypeSignatureBuilder(private val firSession: FirSession, private val } else if (ownerSymbol is FirFunctionSymbol<*>) { owner = methodDeclarationSignature(ownerSymbol, null) } else if (ownerSymbol is FirFileSymbol) { - owner = convertFileNameToFqn(ownerSymbol.fir.name) + owner = convertFileNameToFqn(ownerSymbol.fir) } else if (ownerSymbol != null) { owner = classSignature(ownerSymbol.fir) } else { - owner = convertFileNameToFqn(firFileSymbol.fir.name) + owner = convertFileNameToFqn(firFileSymbol.fir) } val typeSig = if (symbol.fir is FirJavaField || symbol.fir is FirEnumEntry) { @@ -647,7 +648,7 @@ class KotlinTypeSignatureBuilder(private val firSession: FirSession, private val } } if (owner == "{undefined}") { - owner = convertFileNameToFqn(firFileSymbol.fir.name) + owner = convertFileNameToFqn(firFileSymbol.fir) } var s = owner val namedReference = functionCall.calleeReference @@ -683,7 +684,7 @@ class KotlinTypeSignatureBuilder(private val firSession: FirSession, private val signature(ownerSymbol.fir) } else -> { - convertFileNameToFqn(firFileSymbol.fir.name) + convertFileNameToFqn(firFileSymbol.fir) } } s += if (symbol is FirConstructorSymbol) { @@ -766,8 +767,13 @@ class KotlinTypeSignatureBuilder(private val firSession: FirSession, private val return if (classId == null) "{undefined}" else convertKotlinFqToJavaFq(classId.toString()) } - fun convertFileNameToFqn(name: String): String { - return name.replace("/", ".").replace("\\", ".").replace(".kt", "Kt") + fun convertFileNameToFqn(type: FirFile): String { + val sb = StringBuilder() + if (type.packageFqName.asString().isNotEmpty()) { + sb.append(type.packageFqName.asString()).append(".") + } + sb.append(type.name.replace("/", ".").replace("\\", ".").replace(".kt", "Kt")) + return sb.toString() } /** diff --git a/src/main/kotlin/org/openrewrite/kotlin/internal/PsiElementAssociations.kt b/src/main/kotlin/org/openrewrite/kotlin/internal/PsiElementAssociations.kt index bd8f790e9..7a2acafc1 100644 --- a/src/main/kotlin/org/openrewrite/kotlin/internal/PsiElementAssociations.kt +++ b/src/main/kotlin/org/openrewrite/kotlin/internal/PsiElementAssociations.kt @@ -38,6 +38,7 @@ import org.jetbrains.kotlin.psi.KtArrayAccessExpression import org.jetbrains.kotlin.psi.KtBinaryExpression import org.jetbrains.kotlin.psi.KtDeclaration import org.jetbrains.kotlin.psi.KtExpression +import org.jetbrains.kotlin.psi.KtPackageDirective import org.jetbrains.kotlin.psi.KtPostfixExpression import org.jetbrains.kotlin.psi.KtPrefixExpression import org.jetbrains.kotlin.psi.KtWhenConditionInRange @@ -137,7 +138,7 @@ class PsiElementAssociations(val typeMapping: KotlinTypeMapping, val file: FirFi fun type(psiElement: PsiElement, ownerFallBack: FirBasedSymbol<*>?): JavaType? { val fir = primary(psiElement) - return if (fir != null) typeMapping.type(fir, ownerFallBack) else null + return if (fir != null) typeMapping.type(fir, ownerFallBack) else JavaType.Unknown.getInstance() } fun symbol(psi: KtDeclaration?): FirBasedSymbol<*>? { @@ -162,7 +163,7 @@ class PsiElementAssociations(val typeMapping: KotlinTypeMapping, val file: FirFi p = p.parent } - if (p == null) { + if (p == null || p is KtPackageDirective) { return null } @@ -174,7 +175,7 @@ class PsiElementAssociations(val typeMapping: KotlinTypeMapping, val file: FirFi return when (psi) { is KtPrefixExpression -> allFirInfos.first { it.fir is FirVariableAssignment }.fir is KtPostfixExpression -> allFirInfos.first { it.fir is FirResolvedTypeRef }.fir - is KtArrayAccessExpression -> allFirInfos.first { it.fir is FirResolvedNamedReference && it.fir.name.asString() == "get" }.fir + is KtArrayAccessExpression -> allFirInfos.firstOrNull { it.fir is FirResolvedNamedReference && (it.fir.name.asString() == "get" || it.fir.name.asString() == "set") }?.fir is KtWhenConditionInRange, is KtBinaryExpression -> allFirInfos.first { it.fir is FirFunctionCall }.fir else -> { allFirInfos[0].fir diff --git a/src/test/java/org/openrewrite/kotlin/KotlinTypeMappingTest.java b/src/test/java/org/openrewrite/kotlin/KotlinTypeMappingTest.java index 02f56742f..07ed6069f 100644 --- a/src/test/java/org/openrewrite/kotlin/KotlinTypeMappingTest.java +++ b/src/test/java/org/openrewrite/kotlin/KotlinTypeMappingTest.java @@ -109,7 +109,6 @@ void extendsKotlinAny() { assertThat(goatType.getSupertype().getFullyQualifiedName()).isEqualTo("kotlin.Any"); } - @ExpectedToFail("Enable when we switch to IR parser.") @Test void fieldType() { K.Property property = getProperty("field"); @@ -122,20 +121,19 @@ void fieldType() { assertThat(id.getType().toString()).isEqualTo("kotlin.Int"); JavaType.FullyQualified declaringType = property.getGetter().getMethodType().getDeclaringType(); - assertThat(declaringType.getFullyQualifiedName()).isEqualTo("org.openrewrite.kotlin.KotlinTypeGoat"); - assertThat(property.getGetter().getMethodType().getName()).isEqualTo(""); + assertThat(declaringType.getFullyQualifiedName()).isEqualTo("org.openrewrite.kotlin.KotlinTypeGoatKt"); + assertThat(property.getGetter().getMethodType().getName()).isEqualTo("accessor"); // FIXME assertThat(property.getGetter().getMethodType().getReturnType()).isEqualTo(id.getType()); assertThat(property.getGetter().getName().getType()).isEqualTo(property.getGetter().getMethodType()); - assertThat(property.getGetter().getMethodType().toString().substring(declaringType.toString().length())).isEqualTo("{name=,return=kotlin.Int,parameters=[]}"); + assertThat(property.getGetter().getMethodType().toString().substring(declaringType.toString().length())).isEqualTo("{name=accessor,return=kotlin.Int,parameters=[]}"); declaringType = property.getSetter().getMethodType().getDeclaringType(); - assertThat(declaringType.getFullyQualifiedName()).isEqualTo("org.openrewrite.kotlin.KotlinTypeGoat"); - assertThat(property.getSetter().getMethodType().getName()).isEqualTo(""); + assertThat(declaringType.getFullyQualifiedName()).isEqualTo("org.openrewrite.kotlin.KotlinTypeGoatKt"); + assertThat(property.getSetter().getMethodType().getName()).isEqualTo("accessor"); // FIXME assertThat(property.getSetter().getMethodType()).isEqualTo(property.getSetter().getName().getType()); - assertThat(property.getSetter().getMethodType().toString().substring(declaringType.toString().length())).isEqualTo("{name=,return=kotlin.Unit,parameters=[kotlin.Int]}"); + assertThat(property.getSetter().getMethodType().toString().substring(declaringType.toString().length())).isEqualTo("{name=accessor,return=kotlin.Unit,parameters=[kotlin.Int]}"); } - @ExpectedToFail("Enable when we switch to IR parser.") @Test void fileField() { J.VariableDeclarations.NamedVariable nv = cu.getStatements().stream() @@ -149,7 +147,6 @@ void fileField() { .isEqualTo("org.openrewrite.kotlin.KotlinTypeGoatKt{name=field,type=kotlin.Int}"); } - @ExpectedToFail("Signature is correct in IR based type mapping. FIR based type mapping is missing the package.") @Test void fileFunction() { J.MethodDeclaration md = cu.getStatements().stream()