Skip to content

Commit

Permalink
various type attribution fixes.
Browse files Browse the repository at this point in the history
  • Loading branch information
@traceyyoshima
traceyyoshima committed Oct 29, 2023
1 parent 519fa0c commit 11c8e5f
Show file tree
Hide file tree
Showing 5 changed files with 59 additions and 43 deletions.
52 changes: 29 additions & 23 deletions src/main/java/org/openrewrite/kotlin/internal/KotlinTreeParserVisitor.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

Expand Down Expand Up @@ -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())));
Expand All @@ -828,7 +831,7 @@ public J visitParameter(KtParameter parameter, ExecutionContext data) {
name,
emptyList(),
initializer,
name.getFieldType()
vt
);

vars.add(padRight(namedVariable, Space.EMPTY));
Expand Down Expand Up @@ -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<JRightPadded<Expression>> parameters = new ArrayList<>(expression.getTypeArguments().size());
for (KtTypeProjection ktTypeProjection : expression.getTypeArguments()) {
Expand Down Expand Up @@ -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 {
Expand All @@ -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(),
"<destruct>",
variableType(multiDeclaration),
null
),
createIdentifier("<destruct>", Space.SINGLE_SPACE, vt),
emptyList(),
paddedInitializer,
null
vt
);

J.VariableDeclarations variableDeclarations = new J.VariableDeclarations(
Expand Down Expand Up @@ -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());
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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));
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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
);
}
Expand Down Expand Up @@ -3562,7 +3568,7 @@ private J.VariableDeclarations mapDestructuringDeclaration(KtDestructuringDeclar
name,
emptyList(),
null,
variableType(ktDestructuringDeclaration)
variableType(ktDestructuringDeclarationEntry)
);
variables.add(padRight(namedVariable, suffix(ktDestructuringDeclarationEntry)));
}
Expand Down
8 changes: 7 additions & 1 deletion src/main/kotlin/org/openrewrite/kotlin/KotlinTypeMapping.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 -> {
Expand All @@ -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,
Expand Down
20 changes: 13 additions & 7 deletions src/main/kotlin/org/openrewrite/kotlin/KotlinTypeSignatureBuilder.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -82,7 +83,7 @@ class KotlinTypeSignatureBuilder(private val firSession: FirSession, private val
}

is FirFile -> {
return convertFileNameToFqn(type.name)
return convertFileNameToFqn(type)
}

is FirJavaTypeRef -> {
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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()
}

/**
Expand Down
7 changes: 4 additions & 3 deletions src/main/kotlin/org/openrewrite/kotlin/internal/PsiElementAssociations.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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<*>? {
Expand All @@ -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
}

Expand All @@ -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
Expand Down
15 changes: 6 additions & 9 deletions src/test/java/org/openrewrite/kotlin/KotlinTypeMappingTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand All @@ -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("<get-field>");
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=<get-field>,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("<set-field>");
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=<set-field>,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()
Expand All @@ -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()
Expand Down

0 comments on commit 11c8e5f

Please sign in to comment.