diff --git a/j2k/src/org/jetbrains/jet/j2k/Converter.kt b/j2k/src/org/jetbrains/jet/j2k/Converter.kt index 3ba4c87e7f9..977415ce42c 100644 --- a/j2k/src/org/jetbrains/jet/j2k/Converter.kt +++ b/j2k/src/org/jetbrains/jet/j2k/Converter.kt @@ -330,23 +330,6 @@ public class Converter private(val project: Project, val settings: ConverterSett } 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, settings), - Parameter.VarValModifier.None, - params.parameters.single().annotations, - Modifiers.Empty) - params = ParameterList(listOf(correctedParameter)) - } - } - val typeParameterList = convertTypeParameterList(method.getTypeParameterList()) val block = convertBlock(method.getBody()) return Function(this, method.declarationIdentifier(), annotations, modifiers, returnType, typeParameterList, params, block, containingClass?.isInterface() ?: false) diff --git a/j2k/src/org/jetbrains/jet/j2k/TypeConverter.kt b/j2k/src/org/jetbrains/jet/j2k/TypeConverter.kt index e7ed3cba81a..c92d79ba8c3 100644 --- a/j2k/src/org/jetbrains/jet/j2k/TypeConverter.kt +++ b/j2k/src/org/jetbrains/jet/j2k/TypeConverter.kt @@ -25,6 +25,7 @@ import java.util.HashSet import org.jetbrains.jet.j2k.ast.Import import org.jetbrains.jet.j2k.ast.ImportList import org.jetbrains.jet.j2k.ast.assignPrototype +import com.intellij.psi.CommonClassNames.JAVA_LANG_OBJECT class TypeConverter(val settings: ConverterSettings, val conversionScope: ConversionScope) { private val nullabilityCache = HashMap() @@ -72,6 +73,19 @@ class TypeConverter(val settings: ConverterSettings, val conversionScope: Conver var nullability = variable.nullabilityFromAnnotations() + if (nullability == Nullability.Default && variable is PsiParameter) { + val scope = variable.getDeclarationScope() + if (scope is PsiMethod) { + val paramIndex = scope.getParameterList().getParameters().indexOf(variable) + assert(paramIndex >= 0) + val superSignatures = scope.getHierarchicalMethodSignature().getSuperSignatures() + nullability = superSignatures.map { signature -> + val params = signature.getMethod().getParameterList().getParameters() + if (paramIndex < params.size) variableNullability(params[paramIndex]) else Nullability.Default + }.firstOrNull { it != Nullability.Default } ?: Nullability.Default + } + } + if (nullability == Nullability.Default) { val initializer = variable.getInitializer() if (initializer != null) { @@ -85,7 +99,17 @@ class TypeConverter(val settings: ConverterSettings, val conversionScope: Conver } } - if (!conversionScope.contains(variable)) return nullability // do not analyze usages of fields out of our conversion scope + if (!conversionScope.contains(variable)) { // do not analyze usages out of our conversion scope + if (variable is PsiParameter) { + // Object.equals corresponds to Any.equals which has nullable parameter: + val scope = variable.getDeclarationScope() + if (scope is PsiMethod && scope.getName() == "equals" && scope.getContainingClass()?.getQualifiedName() == JAVA_LANG_OBJECT) { + return Nullability.Nullable + } + } + + return nullability + } if (nullability == Nullability.Default) { val scope = searchScope(variable) @@ -133,6 +157,11 @@ class TypeConverter(val settings: ConverterSettings, val conversionScope: Conver private fun methodNullabilityNoCache(method: PsiMethod): Nullability { var nullability = method.nullabilityFromAnnotations() + if (nullability == Nullability.Default) { + val superSignatures = method.getHierarchicalMethodSignature().getSuperSignatures() + nullability = superSignatures.map { methodNullability(it.getMethod()) }.firstOrNull { it != Nullability.Default } ?: Nullability.Default + } + if (!conversionScope.contains(method)) return nullability // do not analyze body and usages of methods out of our conversion scope if (nullability == Nullability.Default) { diff --git a/j2k/tests/test/org/jetbrains/jet/j2k/test/JavaToKotlinConverterTestGenerated.java b/j2k/tests/test/org/jetbrains/jet/j2k/test/JavaToKotlinConverterTestGenerated.java index 831cd2a68a7..2c51401dedd 100644 --- a/j2k/tests/test/org/jetbrains/jet/j2k/test/JavaToKotlinConverterTestGenerated.java +++ b/j2k/tests/test/org/jetbrains/jet/j2k/test/JavaToKotlinConverterTestGenerated.java @@ -1326,6 +1326,11 @@ public class JavaToKotlinConverterTestGenerated extends AbstractJavaToKotlinConv doTest("j2k/tests/testData/ast/function/overrideObject2.java"); } + @TestMetadata("overrideObject3.java") + public void testOverrideObject3() throws Exception { + doTest("j2k/tests/testData/ast/function/overrideObject3.java"); + } + @TestMetadata("ownGenericParam.java") public void testOwnGenericParam() throws Exception { doTest("j2k/tests/testData/ast/function/ownGenericParam.java"); @@ -2004,6 +2009,11 @@ public class JavaToKotlinConverterTestGenerated extends AbstractJavaToKotlinConv doTest("j2k/tests/testData/ast/nullability/FieldInitializedWithNull.java"); } + @TestMetadata("IndirectOverride.java") + public void testIndirectOverride() throws Exception { + doTest("j2k/tests/testData/ast/nullability/IndirectOverride.java"); + } + @TestMetadata("MethodInvokedWithNullArg.java") public void testMethodInvokedWithNullArg() throws Exception { doTest("j2k/tests/testData/ast/nullability/MethodInvokedWithNullArg.java"); @@ -2069,6 +2079,16 @@ public class JavaToKotlinConverterTestGenerated extends AbstractJavaToKotlinConv doTest("j2k/tests/testData/ast/nullability/NullableVariableDotAccess.java"); } + @TestMetadata("OverrideWithInheritanceLoop.java") + public void testOverrideWithInheritanceLoop() throws Exception { + doTest("j2k/tests/testData/ast/nullability/OverrideWithInheritanceLoop.java"); + } + + @TestMetadata("Overrides.java") + public void testOverrides() throws Exception { + doTest("j2k/tests/testData/ast/nullability/Overrides.java"); + } + @TestMetadata("ParameterComparedWithNull.java") public void testParameterComparedWithNull() throws Exception { doTest("j2k/tests/testData/ast/nullability/ParameterComparedWithNull.java"); diff --git a/j2k/tests/testData/JavaApi.java b/j2k/tests/testData/JavaApi.java index f0d51257d23..1da536be2ec 100644 --- a/j2k/tests/testData/JavaApi.java +++ b/j2k/tests/testData/JavaApi.java @@ -1,5 +1,10 @@ package javaApi; +import org.jetbrains.annotations.Nullable; + +import java.lang.Override; +import java.lang.String; + public @interface Anon1 { String[] value(); String[] stringArray(); @@ -43,3 +48,11 @@ public @interface Anon8 { public enum E { A, B, C } + +class Base { + public @Nullable String foo(@Nullable String s) { return s; } +} + +class Derived extends Base { + public String foo(String s) { return s; } +} diff --git a/j2k/tests/testData/ast/function/extendsBaseWhichExtendsObject.kt b/j2k/tests/testData/ast/function/extendsBaseWhichExtendsObject.kt index e7f0ebe62dc..57d95afb66e 100644 --- a/j2k/tests/testData/ast/function/extendsBaseWhichExtendsObject.kt +++ b/j2k/tests/testData/ast/function/extendsBaseWhichExtendsObject.kt @@ -5,7 +5,7 @@ class Test() : Base() { return super.hashCode() } - override fun equals(o: Any): Boolean { + override fun equals(o: Any?): Boolean { return super.equals(o) } @@ -29,7 +29,7 @@ class Base() { return super.hashCode() } - override fun equals(other: Any?): Boolean { + override fun equals(o: Any?): Boolean { return super.equals(o) } diff --git a/j2k/tests/testData/ast/function/overrideObject.kt b/j2k/tests/testData/ast/function/overrideObject.kt index 2bebbe28944..fe55ec8bf2c 100644 --- a/j2k/tests/testData/ast/function/overrideObject.kt +++ b/j2k/tests/testData/ast/function/overrideObject.kt @@ -3,7 +3,7 @@ class X() { return super.hashCode() } - override fun equals(other: Any?): Boolean { + override fun equals(o: Any?): Boolean { return super.equals(o) } diff --git a/j2k/tests/testData/ast/function/overrideObject2.kt b/j2k/tests/testData/ast/function/overrideObject2.kt index 40f8b234ba3..9646e9e2740 100644 --- a/j2k/tests/testData/ast/function/overrideObject2.kt +++ b/j2k/tests/testData/ast/function/overrideObject2.kt @@ -5,7 +5,7 @@ class X() : Base() { return super.hashCode() } - override fun equals(other: Any?): Boolean { + override fun equals(o: Any?): Boolean { return super.equals(o) } diff --git a/j2k/tests/testData/ast/function/overrideObject3.java.todo b/j2k/tests/testData/ast/function/overrideObject3.java similarity index 100% rename from j2k/tests/testData/ast/function/overrideObject3.java.todo rename to j2k/tests/testData/ast/function/overrideObject3.java diff --git a/j2k/tests/testData/ast/function/overrideObject3.kt b/j2k/tests/testData/ast/function/overrideObject3.kt index 94e655b81d6..de83777788d 100644 --- a/j2k/tests/testData/ast/function/overrideObject3.kt +++ b/j2k/tests/testData/ast/function/overrideObject3.kt @@ -1,5 +1,5 @@ class Base() { - override fun equals(other: Any?): Boolean { + override fun equals(o: Any?): Boolean { return super.equals(o) } } diff --git a/j2k/tests/testData/ast/issues/kt-696.kt b/j2k/tests/testData/ast/issues/kt-696.kt index 405b1c9e5d3..d9aefd9c3ac 100644 --- a/j2k/tests/testData/ast/issues/kt-696.kt +++ b/j2k/tests/testData/ast/issues/kt-696.kt @@ -5,7 +5,7 @@ class Base() { return super.hashCode() } - override fun equals(other: Any?): Boolean { + override fun equals(o: Any?): Boolean { return super.equals(o) } @@ -19,7 +19,7 @@ class Child() : Base() { return super.hashCode() } - override fun equals(o: Any): Boolean { + override fun equals(o: Any?): Boolean { return super.equals(o) } diff --git a/j2k/tests/testData/ast/nullability/IndirectOverride.java b/j2k/tests/testData/ast/nullability/IndirectOverride.java new file mode 100644 index 00000000000..6548af24619 --- /dev/null +++ b/j2k/tests/testData/ast/nullability/IndirectOverride.java @@ -0,0 +1,4 @@ +//file +class C extends javaApi.Derived { + public String foo(String s) { return s; } +} diff --git a/j2k/tests/testData/ast/nullability/IndirectOverride.kt b/j2k/tests/testData/ast/nullability/IndirectOverride.kt new file mode 100644 index 00000000000..d65becf9df5 --- /dev/null +++ b/j2k/tests/testData/ast/nullability/IndirectOverride.kt @@ -0,0 +1,5 @@ +class C() : javaApi.Derived() { + override fun foo(s: String?): String? { + return s + } +} \ No newline at end of file diff --git a/j2k/tests/testData/ast/nullability/OverrideWithInheritanceLoop.java b/j2k/tests/testData/ast/nullability/OverrideWithInheritanceLoop.java new file mode 100644 index 00000000000..0d9ca83749c --- /dev/null +++ b/j2k/tests/testData/ast/nullability/OverrideWithInheritanceLoop.java @@ -0,0 +1,8 @@ +//file +class A extends B { + public void foo(String s) {} +} + +class B extends A { + public void foo(String s) {} +} diff --git a/j2k/tests/testData/ast/nullability/OverrideWithInheritanceLoop.kt b/j2k/tests/testData/ast/nullability/OverrideWithInheritanceLoop.kt new file mode 100644 index 00000000000..b4577d0e5d0 --- /dev/null +++ b/j2k/tests/testData/ast/nullability/OverrideWithInheritanceLoop.kt @@ -0,0 +1,9 @@ +class A() : B() { + public fun foo(s: String) { + } +} + +class B() : A() { + public fun foo(s: String) { + } +} diff --git a/j2k/tests/testData/ast/nullability/Overrides.java b/j2k/tests/testData/ast/nullability/Overrides.java new file mode 100644 index 00000000000..93a02f5450b --- /dev/null +++ b/j2k/tests/testData/ast/nullability/Overrides.java @@ -0,0 +1,26 @@ +//file +import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; + +class Base { + @Nullable + public String foo(@Nullable String s) { return ""; } + + public String bar(String s) { + return s != null ? s + 1 : null; + } + + public String zoo(Object o){ return ""; } +} + +interface I { + @Nullable String zoo(@Nullable Object o); +} + +class C extends Base implements I { + public String foo(String s) { return ""; } + + public String bar(String s) { return ""; } + + public String zoo(Object o) { return ""; } +} diff --git a/j2k/tests/testData/ast/nullability/Overrides.kt b/j2k/tests/testData/ast/nullability/Overrides.kt new file mode 100644 index 00000000000..0158b9f0f68 --- /dev/null +++ b/j2k/tests/testData/ast/nullability/Overrides.kt @@ -0,0 +1,31 @@ +class Base() { + public fun foo(s: String?): String? { + return "" + } + + public fun bar(s: String?): String? { + return if (s != null) s + 1 else null + } + + public fun zoo(o: Any): String { + return "" + } +} + +trait I { + public fun zoo(o: Any?): String? +} + +class C() : Base(), I { + override fun foo(s: String?): String? { + return "" + } + + override fun bar(s: String?): String? { + return "" + } + + override fun zoo(o: Any?): String? { + return "" + } +}