From 118b23061b197ba3a6ef08804c33fde561fb8b7a Mon Sep 17 00:00:00 2001 From: Valentin Kipyatkov Date: Wed, 4 Jun 2014 15:12:37 +0400 Subject: [PATCH] Java to Kotlin converter: fixed overriding of Object methods --- j2k/src/org/jetbrains/jet/j2k/Converter.kt | 126 ++++++++++++------ j2k/src/org/jetbrains/jet/j2k/MethodUtils.kt | 65 --------- .../org/jetbrains/jet/j2k/ast/Constructors.kt | 2 +- j2k/src/org/jetbrains/jet/j2k/ast/Types.kt | 37 +++-- .../jet/j2k/visitors/ExpressionVisitor.kt | 2 +- ...ressionVisitorForDirectObjectInheritors.kt | 47 ------- .../jetbrains/jet/j2k/visitors/TypeVisitor.kt | 12 +- .../function/extendsBaseWhichExtendsObject.kt | 12 +- .../testData/ast/function/overrideObject.java | 46 +++---- .../testData/ast/function/overrideObject.kt | 26 ++-- j2k/tests/testData/ast/issues/kt-696.kt | 12 +- j2k/tests/testData/ast/issues/kt-836.kt | 4 +- 12 files changed, 169 insertions(+), 222 deletions(-) delete mode 100644 j2k/src/org/jetbrains/jet/j2k/MethodUtils.kt delete mode 100644 j2k/src/org/jetbrains/jet/j2k/visitors/ExpressionVisitorForDirectObjectInheritors.kt diff --git a/j2k/src/org/jetbrains/jet/j2k/Converter.kt b/j2k/src/org/jetbrains/jet/j2k/Converter.kt index 1d95f94608f..750d9ea6dc0 100644 --- a/j2k/src/org/jetbrains/jet/j2k/Converter.kt +++ b/j2k/src/org/jetbrains/jet/j2k/Converter.kt @@ -226,57 +226,96 @@ public class Converter(val project: Project, val settings: ConverterSettings) { } private fun convertMethod(method: PsiMethod, membersToRemove: MutableSet): Function { - if (directlyOverridesMethodFromObject(method)) { - dispatcher.expressionVisitor = ExpressionVisitorForDirectObjectInheritors(this) - } - else { - dispatcher.expressionVisitor = ExpressionVisitor(this) - } + methodReturnType = method.getReturnType() + val returnType = convertType(method.getReturnType(), method.isAnnotatedAsNotNull()) - try { - methodReturnType = method.getReturnType() - val returnType = convertType(method.getReturnType(), method.isAnnotatedAsNotNull()) + val modifiers = convertModifiers(method) - val modifiers = convertModifiers(method) + val comments = getComments(method) - val containingClass = method.getContainingClass() - val isEffectivelyFinal = method.hasModifierProperty(PsiModifier.FINAL) || - containingClass != null && (containingClass.hasModifierProperty(PsiModifier.FINAL) || containingClass.isEnum()) - - if (isOverride(method)) { - modifiers.add(Modifier.OVERRIDE) - } - - if (settings.openByDefault && - !modifiers.contains(Modifier.ABSTRACT) && - !isEffectivelyFinal && - !modifiers.contains(Modifier.PRIVATE)) { - modifiers.add(Modifier.OPEN) - } - - val comments = getComments(method) - - if (method.isConstructor()) { - if (method.isPrimaryConstructor()) { - return convertPrimaryConstructor(method, modifiers, comments, membersToRemove) - } - else { - val params = convertParameterList(method.getParameterList()) - return SecondaryConstructor(this, comments, modifiers, params, convertBlock(method.getBody())) - } + if (method.isConstructor()) { + if (method.isPrimaryConstructor()) { + return convertPrimaryConstructor(method, modifiers, comments, membersToRemove) } else { val params = convertParameterList(method.getParameterList()) - val typeParameterList = convertTypeParameterList(method.getTypeParameterList()) - val block = convertBlock(method.getBody()) - return Function(this, Identifier(method.getName()), comments, modifiers, returnType, typeParameterList, params, block, containingClass?.isInterface() ?: false) + return SecondaryConstructor(this, comments, modifiers, params, convertBlock(method.getBody())) } } - finally { - dispatcher.expressionVisitor = ExpressionVisitor(this) + else { + val isOverride = isOverride(method) + if (isOverride) { + modifiers.add(Modifier.OVERRIDE) + } + + val containingClass = method.getContainingClass() + + if (settings.openByDefault) { + val isEffectivelyFinal = method.hasModifierProperty(PsiModifier.FINAL) || + containingClass != null && (containingClass.hasModifierProperty(PsiModifier.FINAL) || containingClass.isEnum()) + if (!isEffectivelyFinal && !modifiers.contains(Modifier.ABSTRACT) && !modifiers.contains(Modifier.PRIVATE)) { + modifiers.add(Modifier.OPEN) + } + } + + var params = convertParameterList(method.getParameterList()) + + // if we override equals from Object, change parameter type to nullable + if (isOverride && method.getName() == "equals") { + val superSignatures = method.getHierarchicalMethodSignature().getSuperSignatures() + val overridesMethodFromObject = superSignatures.any { + it.getMethod().getContainingClass()?.getQualifiedName() == JAVA_LANG_OBJECT + } + if (overridesMethodFromObject) { + val correctedParameter = Parameter(Identifier("other"), + ClassType(Identifier("Any"), listOf(), Nullability.Nullable, this), + Parameter.VarValModifier.None, + listOf()) + params = ParameterList(listOf(correctedParameter)) + } + } + + val typeParameterList = convertTypeParameterList(method.getTypeParameterList()) + val block = convertBlock(method.getBody()) + return Function(this, Identifier(method.getName()), comments, modifiers, returnType, typeParameterList, params, block, containingClass?.isInterface() ?: false) } } + /** + * Overrides of methods from Object should not be marked as overrides in Kotlin unless the class itself has java ancestors + */ + private fun isOverride(method: PsiMethod): Boolean { + val superSignatures = method.getHierarchicalMethodSignature().getSuperSignatures() + + val overridesMethodNotFromObject = superSignatures.any { + it.getMethod().getContainingClass()?.getQualifiedName() != JAVA_LANG_OBJECT + } + if (overridesMethodNotFromObject) return true + + val overridesMethodFromObject = superSignatures.any { + it.getMethod().getContainingClass()?.getQualifiedName() == JAVA_LANG_OBJECT + } + if (overridesMethodFromObject) { + when(method.getName()) { + "equals", "hashCode", "toString" -> return true // these methods from java.lang.Object exist in kotlin.Any + + else -> { + val containing = method.getContainingClass() + if (containing != null) { + val hasOtherJavaSuperclasses = containing.getSuperTypes().any { + val canonicalText = it.getCanonicalText() + //TODO: correctly check for kotlin class + canonicalText != JAVA_LANG_OBJECT && !getClassIdentifiers().contains(canonicalText) + } + if (hasOtherJavaSuperclasses) return true + } + } + } + } + + return false + } + private fun convertPrimaryConstructor(constructor: PsiMethod, modifiers: Set, comments: MemberComments, @@ -300,7 +339,12 @@ public class Converter(val project: Project, val settings: ConverterSettings) { } } dispatcher.expressionVisitor = ExpressionVisitor(this, usageReplacementMap) - Block(convertStatements(body.getStatements().filter{ !statementsToRemove.contains(it) }), false) + try { + Block(convertStatements(body.getStatements().filter{ !statementsToRemove.contains(it) }), false) + } + finally { + dispatcher.expressionVisitor = ExpressionVisitor(this, mapOf()) + } } else { Block.Empty diff --git a/j2k/src/org/jetbrains/jet/j2k/MethodUtils.kt b/j2k/src/org/jetbrains/jet/j2k/MethodUtils.kt deleted file mode 100644 index f5c0f303b3f..00000000000 --- a/j2k/src/org/jetbrains/jet/j2k/MethodUtils.kt +++ /dev/null @@ -1,65 +0,0 @@ -/* - * Copyright 2010-2013 JetBrains s.r.o. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package org.jetbrains.jet.j2k - -import com.intellij.psi.PsiMethod -import com.intellij.psi.PsiClass -import com.intellij.psi.PsiModifier -import com.intellij.psi.CommonClassNames.JAVA_LANG_OBJECT - -/* -* Overrides of methods from object should not be marked as overrides in Kotlin unless the class itself has java ancestors -* */ -fun Converter.isOverride(method: PsiMethod): Boolean { - val superSignatures = method.getHierarchicalMethodSignature().getSuperSignatures() - val overridesMethodNotFromObject = superSignatures any { - it.getMethod().getContainingClass()?.getQualifiedName() != JAVA_LANG_OBJECT - } - - if (overridesMethodNotFromObject) { - return true - } - - val overridesMethodFromObject = superSignatures any { - it.getMethod().getContainingClass()?.getQualifiedName() == JAVA_LANG_OBJECT - } - - if (overridesMethodFromObject) { - val containing = method.getContainingClass() - if (containing != null) { - val hasOtherJavaSuperclasses = containing.getSuperTypes() any { - val canonicalText = it.getCanonicalText() - //TODO: correctly check for kotlin class - canonicalText != JAVA_LANG_OBJECT && !getClassIdentifiers().contains(canonicalText) - } - if (hasOtherJavaSuperclasses) { - return true - } - } - } - - return false -} - -fun directlyOverridesMethodFromObject(method: PsiMethod): Boolean { - var superSignatures = method.getHierarchicalMethodSignature().getSuperSignatures() - if (superSignatures.size() == 1) { - val qualifiedName = superSignatures.single().getMethod().getContainingClass()?.getQualifiedName() - return qualifiedName == JAVA_LANG_OBJECT - } - return false -} \ No newline at end of file diff --git a/j2k/src/org/jetbrains/jet/j2k/ast/Constructors.kt b/j2k/src/org/jetbrains/jet/j2k/ast/Constructors.kt index cb1404f028a..a4912571c36 100644 --- a/j2k/src/org/jetbrains/jet/j2k/ast/Constructors.kt +++ b/j2k/src/org/jetbrains/jet/j2k/ast/Constructors.kt @@ -60,7 +60,7 @@ class SecondaryConstructor(converter: Converter, val typeParameters = ArrayList() typeParameters.addAll(containingClass.typeParameterList.parameters) return Function(converter, Identifier("init"), MemberComments.Empty, modifiers, - ClassType(containingClass.name, typeParameters, false, converter), + ClassType(containingClass.name, typeParameters, Nullability.NotNull, converter), TypeParameterList(typeParameters), parameterList, block, false) } } diff --git a/j2k/src/org/jetbrains/jet/j2k/ast/Types.kt b/j2k/src/org/jetbrains/jet/j2k/ast/Types.kt index 9a6395b30b5..ae367f0a495 100644 --- a/j2k/src/org/jetbrains/jet/j2k/ast/Types.kt +++ b/j2k/src/org/jetbrains/jet/j2k/ast/Types.kt @@ -22,8 +22,18 @@ import java.util.ArrayList fun Type.isPrimitive(): Boolean = this is PrimitiveType fun Type.isUnit(): Boolean = this == Type.Unit -abstract class MayBeNullableType(isNullable: Boolean, val converter: Converter) : Type { - override val isNullable: Boolean = !converter.settings.forceNotNullTypes && isNullable +enum class Nullability { + Nullable + NotNull + Default +} + +abstract class MayBeNullableType(nullability: Nullability, val converter: Converter) : Type { + override val isNullable: Boolean = when (nullability) { + Nullability.Nullable -> true + Nullability.NotNull -> false + Nullability.Default -> !converter.settings.forceNotNullTypes + } } trait NotNullType : Type { @@ -39,6 +49,11 @@ trait Type : Element { return this } + open fun toNullableType(): Type { + if (!isNullable) throw UnsupportedOperationException("toNullableType must be defined") + return this + } + protected fun isNullableStr(): String? { return if (isNullable) "?" else "" } @@ -56,8 +71,8 @@ trait Type : Element { override fun hashCode(): Int = toKotlin().hashCode() } -open class ClassType(val `type`: Identifier, val parameters: List, isNullable: Boolean, - converter: Converter) : MayBeNullableType(isNullable, converter) { +open class ClassType(val `type`: Identifier, val parameters: List, nullability: Nullability, converter: Converter) + : MayBeNullableType(nullability, converter) { override fun toKotlin(): String { // TODO change to map() when KT-2051 is fixed @@ -73,14 +88,13 @@ open class ClassType(val `type`: Identifier, val parameters: List, isNu } - override fun toNotNullType(): Type = ClassType(`type`, parameters, false, converter) + override fun toNotNullType(): Type = ClassType(`type`, parameters, Nullability.NotNull, converter) + override fun toNullableType(): Type = ClassType(`type`, parameters, Nullability.Nullable, converter) } -class ArrayType( - val elementType: Type, - isNullable: Boolean, - converter: Converter -) : MayBeNullableType(isNullable, converter) { +class ArrayType(val elementType: Type, nullability: Nullability, converter: Converter) + : MayBeNullableType(nullability, converter) { + override fun toKotlin(): String { if (elementType is PrimitiveType) { return elementType.toKotlin() + "Array" + isNullableStr() @@ -89,7 +103,8 @@ class ArrayType( return "Array<" + elementType.toKotlin() + ">" + isNullableStr() } - override fun toNotNullType(): Type = ArrayType(elementType, false, converter) + override fun toNotNullType(): Type = ArrayType(elementType, Nullability.NotNull, converter) + override fun toNullableType(): Type = ArrayType(elementType, Nullability.Nullable, converter) } open class InProjectionType(val bound: Type) : NotNullType { diff --git a/j2k/src/org/jetbrains/jet/j2k/visitors/ExpressionVisitor.kt b/j2k/src/org/jetbrains/jet/j2k/visitors/ExpressionVisitor.kt index 610a30518b3..bf42627e294 100644 --- a/j2k/src/org/jetbrains/jet/j2k/visitors/ExpressionVisitor.kt +++ b/j2k/src/org/jetbrains/jet/j2k/visitors/ExpressionVisitor.kt @@ -41,7 +41,7 @@ open class ExpressionVisitor(protected val converter: Converter, override fun visitArrayInitializerExpression(expression: PsiArrayInitializerExpression) { val expressionType = converter.convertType(expression.getType()) - assert(expressionType is ArrayType) { "Array initializer must have array type" } + assert(expressionType is ArrayType, "Array initializer must have array type") result = ArrayInitializerExpression(expressionType as ArrayType, converter.convertExpressions(expression.getInitializers())) } diff --git a/j2k/src/org/jetbrains/jet/j2k/visitors/ExpressionVisitorForDirectObjectInheritors.kt b/j2k/src/org/jetbrains/jet/j2k/visitors/ExpressionVisitorForDirectObjectInheritors.kt deleted file mode 100644 index c7ada820449..00000000000 --- a/j2k/src/org/jetbrains/jet/j2k/visitors/ExpressionVisitorForDirectObjectInheritors.kt +++ /dev/null @@ -1,47 +0,0 @@ -/* - * Copyright 2010-2013 JetBrains s.r.o. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package org.jetbrains.jet.j2k.visitors - -import com.intellij.psi.* -import org.jetbrains.jet.j2k.Converter -import org.jetbrains.jet.j2k.ast.DummyStringExpression -import org.jetbrains.jet.j2k.ast.Identifier -import com.intellij.psi.CommonClassNames.JAVA_LANG_OBJECT -import org.jetbrains.jet.j2k.ast.MethodCallExpression - -open class ExpressionVisitorForDirectObjectInheritors(converter: Converter, usageReplacementMap: Map = mapOf()) -: ExpressionVisitor(converter, usageReplacementMap) { - override fun visitMethodCallExpression(expression: PsiMethodCallExpression) { - val methodExpression = expression.getMethodExpression() - if (isSuperMethodInvocation(methodExpression, "hashCode")) { - result = MethodCallExpression.build(Identifier("System", false), "identityHashCode", listOf(Identifier("this"))) - } - else if (isSuperMethodInvocation(methodExpression, "equals")) { - result = MethodCallExpression.build(Identifier("this", false), "identityEquals", converter.convertArguments(expression)) - } - else if (isSuperMethodInvocation(methodExpression, "toString")) { - result = DummyStringExpression("getJavaClass<${getClassName(methodExpression)}>.getName() + '@' + Integer.toHexString(hashCode())") - } - else { - convertMethodCallExpression(expression) - } - } - - private fun isSuperMethodInvocation(expression: PsiReferenceExpression, methodName: String): Boolean - = expression.getReferenceName() == methodName - && (expression.getQualifierExpression() as? PsiSuperExpression)?.getType()?.getCanonicalText() == JAVA_LANG_OBJECT -} diff --git a/j2k/src/org/jetbrains/jet/j2k/visitors/TypeVisitor.kt b/j2k/src/org/jetbrains/jet/j2k/visitors/TypeVisitor.kt index 69332b51b77..3ab3a7fc326 100644 --- a/j2k/src/org/jetbrains/jet/j2k/visitors/TypeVisitor.kt +++ b/j2k/src/org/jetbrains/jet/j2k/visitors/TypeVisitor.kt @@ -42,7 +42,7 @@ open class TypeVisitor(private val converter: Converter) : PsiTypeVisitor( } override fun visitArrayType(arrayType: PsiArrayType): Type { - return ArrayType(converter.convertType(arrayType.getComponentType()), true, converter) + return ArrayType(converter.convertType(arrayType.getComponentType()), Nullability.Default, converter) } override fun visitClassType(classType: PsiClassType): Type { @@ -53,18 +53,18 @@ open class TypeVisitor(private val converter: Converter) : PsiTypeVisitor( if (resolvedClassTypeParams.size() == 1) { if ((resolvedClassTypeParams.single() as ClassType).`type`.name == "Any") { starParamList.add(StarProjectionType()) - return ClassType(identifier, starParamList, true, converter) + return ClassType(identifier, starParamList, Nullability.Default, converter) } else { - return ClassType(identifier, resolvedClassTypeParams, true, converter) + return ClassType(identifier, resolvedClassTypeParams, Nullability.Default, converter) } } else { - return ClassType(identifier, resolvedClassTypeParams, true, converter) + return ClassType(identifier, resolvedClassTypeParams, Nullability.Default, converter) } } else { - return ClassType(identifier, converter.convertTypes(classType.getParameters()), true, converter) + return ClassType(identifier, converter.convertTypes(classType.getParameters()), Nullability.Default, converter) } } @@ -98,7 +98,7 @@ open class TypeVisitor(private val converter: Converter) : PsiTypeVisitor( val boundType = if (superTypes.size > 0) ClassType(Identifier(getClassTypeName(superTypes[0])), converter.convertTypes(superTypes[0].getParameters()), - true, + Nullability.Default, converter) else StarProjectionType() diff --git a/j2k/tests/testData/ast/function/extendsBaseWhichExtendsObject.kt b/j2k/tests/testData/ast/function/extendsBaseWhichExtendsObject.kt index 5fdb3b33eea..d5089fa31bc 100644 --- a/j2k/tests/testData/ast/function/extendsBaseWhichExtendsObject.kt +++ b/j2k/tests/testData/ast/function/extendsBaseWhichExtendsObject.kt @@ -23,20 +23,20 @@ class Test() : Base() { } class Base() { - public fun hashCode(): Int { - return System.identityHashCode(this) + override fun hashCode(): Int { + return super.hashCode() } - public fun equals(o: Any): Boolean { - return this.identityEquals(o) + override fun equals(other: Any?): Boolean { + return super.equals(o) } protected fun clone(): Any { return super.clone() } - public fun toString(): String { - return getJavaClass.getName() + '@' + Integer.toHexString(hashCode()) + override fun toString(): String { + return super.toString() } protected fun finalize() { diff --git a/j2k/tests/testData/ast/function/overrideObject.java b/j2k/tests/testData/ast/function/overrideObject.java index f3e75c843ee..a0ca0d5002a 100644 --- a/j2k/tests/testData/ast/function/overrideObject.java +++ b/j2k/tests/testData/ast/function/overrideObject.java @@ -1,29 +1,29 @@ //file -package test; +class X { + @Override + public int hashCode() { + return super.hashCode(); + } -class Test { - @Override - public int hashCode() { - return super.hashCode(); - } + @Override + public boolean equals(Object o) { + return super.equals(o); + } - @Override - public boolean equals(Object o) { - return super.equals(o); - } + @Override + public String toString() { + return super.toString(); + } - @Override - protected Object clone() throws CloneNotSupportedException { - return super.clone(); - } + @Override + protected Object clone() throws CloneNotSupportedException { + return super.clone(); + } +} - @Override - public String toString() { - return super.toString(); - } - - @Override - protected void finalize() throws Throwable { - super.finalize(); - } +class Y extends Thread { + @Override + protected Object clone() throws CloneNotSupportedException { + return super.clone(); + } } \ No newline at end of file diff --git a/j2k/tests/testData/ast/function/overrideObject.kt b/j2k/tests/testData/ast/function/overrideObject.kt index 65bf9dcdc49..8d8a5919c85 100644 --- a/j2k/tests/testData/ast/function/overrideObject.kt +++ b/j2k/tests/testData/ast/function/overrideObject.kt @@ -1,23 +1,23 @@ -package test - -class Test() { - public fun hashCode(): Int { - return System.identityHashCode(this) +class X() { + override fun hashCode(): Int { + return super.hashCode() } - public fun equals(o: Any): Boolean { - return this.identityEquals(o) + override fun equals(other: Any?): Boolean { + return super.equals(o) + } + + override fun toString(): String { + return super.toString() } protected fun clone(): Any { return super.clone() } +} - public fun toString(): String { - return getJavaClass.getName() + '@' + Integer.toHexString(hashCode()) - } - - protected fun finalize() { - super.finalize() +class Y() : Thread() { + override fun clone(): Any { + return super.clone() } } \ No newline at end of file diff --git a/j2k/tests/testData/ast/issues/kt-696.kt b/j2k/tests/testData/ast/issues/kt-696.kt index e1b0319f277..405b1c9e5d3 100644 --- a/j2k/tests/testData/ast/issues/kt-696.kt +++ b/j2k/tests/testData/ast/issues/kt-696.kt @@ -1,16 +1,16 @@ package test class Base() { - public fun hashCode(): Int { - return System.identityHashCode(this) + override fun hashCode(): Int { + return super.hashCode() } - public fun equals(o: Any): Boolean { - return this.identityEquals(o) + override fun equals(other: Any?): Boolean { + return super.equals(o) } - public fun toString(): String { - return getJavaClass.getName() + '@' + Integer.toHexString(hashCode()) + override fun toString(): String { + return super.toString() } } diff --git a/j2k/tests/testData/ast/issues/kt-836.kt b/j2k/tests/testData/ast/issues/kt-836.kt index f570a57bb23..e1799b31b15 100644 --- a/j2k/tests/testData/ast/issues/kt-836.kt +++ b/j2k/tests/testData/ast/issues/kt-836.kt @@ -13,7 +13,7 @@ public class Language(protected var code: String) : Serializable { class Base() { fun test() { } - fun toString(): String { + override fun toString(): String { return "BASE" } } @@ -24,4 +24,4 @@ class Child() : Base() { override fun toString(): String { return "Child" } -} \ No newline at end of file +}