Java to Kotlin converter: fixed overriding of Object methods

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<PsiMember>): Function {
-        if (directlyOverridesMethodFromObject(method)) {
+        methodReturnType = method.getReturnType()
-            dispatcher.expressionVisitor = ExpressionVisitorForDirectObjectInheritors(this)
+        val returnType = convertType(method.getReturnType(), method.isAnnotatedAsNotNull())
-        }
-        else {
-            dispatcher.expressionVisitor = ExpressionVisitor(this)
-        }
 
-        try {
+        val modifiers = convertModifiers(method)
-            methodReturnType = method.getReturnType()
-            val returnType = convertType(method.getReturnType(), method.isAnnotatedAsNotNull())
 
-            val modifiers = convertModifiers(method)
+        val comments = getComments(method)
 
-            val containingClass = method.getContainingClass()
+        if (method.isConstructor()) {
-            val isEffectivelyFinal = method.hasModifierProperty(PsiModifier.FINAL) ||
+            if (method.isPrimaryConstructor()) {
-                    containingClass != null && (containingClass.hasModifierProperty(PsiModifier.FINAL) || containingClass.isEnum())
+                return convertPrimaryConstructor(method, modifiers, comments, membersToRemove)
-
-            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()))
-                }
             }
             else {
                 val params = convertParameterList(method.getParameterList())
-                val typeParameterList = convertTypeParameterList(method.getTypeParameterList())
+                return SecondaryConstructor(this, comments, modifiers, params, convertBlock(method.getBody()))
-                val block = convertBlock(method.getBody())
-                return Function(this, Identifier(method.getName()), comments, modifiers, returnType, typeParameterList, params, block, containingClass?.isInterface() ?: false)
             }
         }
-        finally {
+        else {
-            dispatcher.expressionVisitor = ExpressionVisitor(this)
+            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<Modifier>,
                                           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

j2k/src/org/jetbrains/jet/j2k/MethodUtils.kt

@@ -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
-}

j2k/src/org/jetbrains/jet/j2k/ast/Constructors.kt

@@ -60,7 +60,7 @@ class SecondaryConstructor(converter: Converter,
         val typeParameters = ArrayList<TypeParameter>()
         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)
     }
 }

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 {
+enum class Nullability {
-    override val isNullable: Boolean = !converter.settings.forceNotNullTypes && isNullable
+    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<Element>, isNullable: Boolean,
+open class ClassType(val `type`: Identifier, val parameters: List<Element>, nullability: Nullability, converter: Converter)
-                     converter: Converter) : MayBeNullableType(isNullable, 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<Element>, 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(
+class ArrayType(val elementType: Type, nullability: Nullability, converter: Converter)
-        val elementType: Type,
+  : MayBeNullableType(nullability, converter) {
-        isNullable: Boolean,
+
-        converter: Converter
-) : MayBeNullableType(isNullable, 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 {

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()))
     }

j2k/src/org/jetbrains/jet/j2k/visitors/ExpressionVisitorForDirectObjectInheritors.kt

@@ -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<PsiVariable, String> = 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
-}

j2k/src/org/jetbrains/jet/j2k/visitors/TypeVisitor.kt

@@ -42,7 +42,7 @@ open class TypeVisitor(private val converter: Converter) : PsiTypeVisitor<Type>(
     }
 
     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<Type>(
             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<Type>(
                     val boundType = if (superTypes.size > 0)
                         ClassType(Identifier(getClassTypeName(superTypes[0])),
                                   converter.convertTypes(superTypes[0].getParameters()),
-                                  true,
+                                  Nullability.Default,
                                   converter)
                     else
                         StarProjectionType()

j2k/tests/testData/ast/function/extendsBaseWhichExtendsObject.kt

@@ -23,20 +23,20 @@ class Test() : Base() {
 }
 
 class Base() {
-    public fun hashCode(): Int {
+    override fun hashCode(): Int {
-        return System.identityHashCode(this)
+        return super.hashCode()
     }
 
-    public fun equals(o: Any): Boolean {
+    override fun equals(other: Any?): Boolean {
-        return this.identityEquals(o)
+        return super.equals(o)
     }
 
     protected fun clone(): Any {
         return super.clone()
     }
 
-    public fun toString(): String {
+    override fun toString(): String {
-        return getJavaClass<Base>.getName() + '@' + Integer.toHexString(hashCode())
+        return super.toString()
     }
 
     protected fun finalize() {

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
-  @Override
+    public boolean equals(Object o) {
-  public int hashCode() {
+        return super.equals(o);
-    return super.hashCode();
+    }
-  }
 
-  @Override
+    @Override
-  public boolean equals(Object o) {
+    public String toString() {
-    return super.equals(o);
+        return super.toString();
-  }
+    }
 
-  @Override
+    @Override
-  protected Object clone() throws CloneNotSupportedException {
+    protected Object clone() throws CloneNotSupportedException {
-    return super.clone();
+        return super.clone();
-  }
+    }
+}
 
-  @Override
+class Y extends Thread {
-  public String toString() {
+    @Override
-    return super.toString();
+    protected Object clone() throws CloneNotSupportedException {
-  }
+        return super.clone();
-
+    }
-  @Override
-  protected void finalize() throws Throwable {
-    super.finalize();
-  }
 }

j2k/tests/testData/ast/function/overrideObject.kt

@@ -1,23 +1,23 @@
-package test
+class X() {
-
+    override fun hashCode(): Int {
-class Test() {
+        return super.hashCode()
-    public fun hashCode(): Int {
-        return System.identityHashCode(this)
     }
 
-    public fun equals(o: Any): Boolean {
+    override fun equals(other: Any?): Boolean {
-        return this.identityEquals(o)
+        return super.equals(o)
+    }
+
+    override fun toString(): String {
+        return super.toString()
     }
 
     protected fun clone(): Any {
         return super.clone()
     }
+}
 
-    public fun toString(): String {
+class Y() : Thread() {
-        return getJavaClass<Test>.getName() + '@' + Integer.toHexString(hashCode())
+    override fun clone(): Any {
-    }
+        return super.clone()
-
-    protected fun finalize() {
-        super.finalize()
     }
 }

j2k/tests/testData/ast/issues/kt-696.kt

@@ -1,16 +1,16 @@
 package test
 
 class Base() {
-    public fun hashCode(): Int {
+    override fun hashCode(): Int {
-        return System.identityHashCode(this)
+        return super.hashCode()
     }
 
-    public fun equals(o: Any): Boolean {
+    override fun equals(other: Any?): Boolean {
-        return this.identityEquals(o)
+        return super.equals(o)
     }
 
-    public fun toString(): String {
+    override fun toString(): String {
-        return getJavaClass<Base>.getName() + '@' + Integer.toHexString(hashCode())
+        return super.toString()
     }
 }
 

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"
     }
-}
+}