From ee5a853faa4ab6b5b02e30679668090ad7cdc41a Mon Sep 17 00:00:00 2001 From: Valentin Kipyatkov Date: Wed, 23 Sep 2015 14:41:35 +0300 Subject: [PATCH] Generate accessor with delegation to super when it's necessary --- j2k/src/org/jetbrains/kotlin/j2k/Converter.kt | 59 ++++++++++++++----- .../jetbrains/kotlin/j2k/propertyDetection.kt | 48 +++++++++++---- .../dropAccessors/Overrides.java | 6 ++ .../fileOrElement/dropAccessors/Overrides.kt | 24 ++++++-- .../dropAccessors/OverridesOfExternalCode.kt | 24 ++++++-- 5 files changed, 122 insertions(+), 39 deletions(-) diff --git a/j2k/src/org/jetbrains/kotlin/j2k/Converter.kt b/j2k/src/org/jetbrains/kotlin/j2k/Converter.kt index 94910f6b59f..67d5c7f350d 100644 --- a/j2k/src/org/jetbrains/kotlin/j2k/Converter.kt +++ b/j2k/src/org/jetbrains/kotlin/j2k/Converter.kt @@ -340,24 +340,52 @@ class Converter private constructor( //TODO: doc-comments - val getter = getMethod - ?.check { propertyInfo.needExplicitGetter } //TODO: what if annotations are not empty? - ?.let { - val method = convertMethod(it, null, null, null, classKind)!! - PropertyAccessor(AccessorKind.GETTER, method.annotations, Modifiers.Empty, method.parameterList, method.body) - .assignPrototype(it, CommentsAndSpacesInheritance.NO_SPACES) - } + var getter: PropertyAccessor? = null + if (propertyInfo.needExplicitGetter) { + if (getMethod != null) { + val method = convertMethod(getMethod, null, null, null, classKind)!! + getter = PropertyAccessor(AccessorKind.GETTER, method.annotations, Modifiers.Empty, method.parameterList, method.body) + getter.assignPrototype(getMethod, CommentsAndSpacesInheritance.NO_SPACES) + } + else if (propertyInfo.isOverride) { //TODO: expression body! + val superExpression = SuperExpression(Identifier.Empty).assignNoPrototype() + val superAccess = QualifiedExpression(superExpression, propertyInfo.identifier).assignNoPrototype() + val returnStatement = ReturnStatement(superAccess).assignNoPrototype() + val body = Block(listOf(returnStatement), LBrace().assignNoPrototype(), RBrace().assignNoPrototype()).assignNoPrototype() + val parameterList = ParameterList(emptyList()).assignNoPrototype() + getter = PropertyAccessor(AccessorKind.GETTER, Annotations.Empty, Modifiers.Empty, parameterList, deferredElement { body }) + getter.assignNoPrototype() + } + //TODO: what else? + } var setter: PropertyAccessor? = null if (propertyInfo.needExplicitSetter) { - val method = setMethod?.let { convertMethod(it, null, null, null, classKind)!! } val accessorModifiers = Modifiers(propertyInfo.specialSetterAccess.singletonOrEmptyList()).assignNoPrototype() - setter = PropertyAccessor( - AccessorKind.SETTER, - method?.annotations ?: Annotations.Empty, - accessorModifiers, - method?.parameterList?.check { propertyInfo.needSetterBody }, - method?.body?.check { propertyInfo.needSetterBody }).assignPrototype(setMethod, CommentsAndSpacesInheritance.NO_SPACES) + if (setMethod != null) { + val method = setMethod.let { convertMethod(it, null, null, null, classKind)!! } + setter = PropertyAccessor( + AccessorKind.SETTER, + method.annotations, + accessorModifiers, + method.parameterList?.check { method.body != null }, + method.body) + setter.assignPrototype(setMethod, CommentsAndSpacesInheritance.NO_SPACES) + } + else if (propertyInfo.isOverride) { //TODO: expression body! + val superExpression = SuperExpression(Identifier.Empty).assignNoPrototype() + val superAccess = QualifiedExpression(superExpression, propertyInfo.identifier).assignNoPrototype() + val valueIdentifier = Identifier("value", false).assignNoPrototype() + val assignment = AssignmentExpression(superAccess, valueIdentifier, "=").assignNoPrototype() + val body = Block(listOf(assignment), LBrace().assignNoPrototype(), RBrace().assignNoPrototype()).assignNoPrototype() + val parameter = FunctionParameter(valueIdentifier, propertyType, FunctionParameter.VarValModifier.None, Annotations.Empty, Modifiers.Empty).assignNoPrototype() + val parameterList = ParameterList(listOf(parameter)).assignNoPrototype() + setter = PropertyAccessor(AccessorKind.SETTER, Annotations.Empty, accessorModifiers, parameterList, deferredElement { body }) + setter.assignNoPrototype() + } + else { + setter = PropertyAccessor(AccessorKind.SETTER, Annotations.Empty, accessorModifiers, null, null).assignNoPrototype() + } } val needInitializer = field != null && shouldGenerateDefaultInitializer(referenceSearcher, field) @@ -648,8 +676,7 @@ class Converter private constructor( val modifiers = ArrayList() - //TODO: what if one is abstract and another is not? - if (getterModifiers.contains(Modifier.ABSTRACT) || setterModifiers.contains(Modifier.ABSTRACT)) { + if (propertyInfo.isAbstract) { modifiers.add(Modifier.ABSTRACT) } diff --git a/j2k/src/org/jetbrains/kotlin/j2k/propertyDetection.kt b/j2k/src/org/jetbrains/kotlin/j2k/propertyDetection.kt index 2136db7da76..d36d9fe0679 100644 --- a/j2k/src/org/jetbrains/kotlin/j2k/propertyDetection.kt +++ b/j2k/src/org/jetbrains/kotlin/j2k/propertyDetection.kt @@ -36,30 +36,44 @@ class PropertyInfo( val field: PsiField?, val getMethod: PsiMethod?, val setMethod: PsiMethod?, - val needGetterBody: Boolean, - val needSetterBody: Boolean, + val isGetMethodBodyFieldAccess: Boolean, + val isSetMethodBodyFieldAccess: Boolean, val specialSetterAccess: Modifier?, - val isOverride: Boolean + val isOverride: Boolean, + val isAbstract: Boolean //TODO: modifiers here ) { init { assert(field != null || getMethod != null || setMethod != null) - if (needGetterBody) { - assert(getMethod != null && getMethod.body != null) + if (isGetMethodBodyFieldAccess) { + assert(field != null && getMethod != null) } - if (needSetterBody) { - assert(setMethod != null && setMethod.body != null) + if (isSetMethodBodyFieldAccess) { + assert(field != null && setMethod != null) } } val name: String get() = identifier.name - val needExplicitGetter: Boolean get() = needGetterBody - val needExplicitSetter: Boolean get() = needSetterBody || specialSetterAccess != null + //TODO: what if annotations are not empty? + val needExplicitGetter: Boolean + get() { + if (getMethod != null && getMethod.body != null && !isGetMethodBodyFieldAccess) return true + return isOverride && this.field == null && !isAbstract + } + + //TODO: what if annotations are not empty? + val needExplicitSetter: Boolean + get() { + if (!isVar) return false + if (specialSetterAccess != null) return true + if (setMethod != null && setMethod.body != null && !isSetMethodBodyFieldAccess) return true + return isOverride && this.field == null && !isAbstract + } companion object { fun fromFieldWithNoAccessors(field: PsiField, isVar: Boolean) - = PropertyInfo(field.declarationIdentifier(), isVar, field.type, field, null, null, false, false, null, false) + = PropertyInfo(field.declarationIdentifier(), isVar, field.type, field, null, null, false, false, null, false, false) } } @@ -148,16 +162,24 @@ private class PropertyDetector( val type = field?.type ?: getterInfo?.method?.returnType ?: setterInfo!!.method.parameterList.parameters.single()?.type!! + val isOverride = getterInfo?.superProperty != null || setterInfo?.superProperty != null + + //TODO: what if one is abstract and another is not? + val isGetterAbstract = getterInfo?.method?.hasModifierProperty(PsiModifier.ABSTRACT) ?: true + val isSetterAbstract = setterInfo?.method?.hasModifierProperty(PsiModifier.ABSTRACT) ?: true + val isAbstract = field == null && isGetterAbstract && isSetterAbstract + val propertyInfo = PropertyInfo(Identifier(propertyName).assignNoPrototype(), isVar, type, field, getterInfo?.method, setterInfo?.method, - getterInfo != null && getterInfo.method.body != null && (field == null || getterInfo.field != field), - setterInfo != null && setterInfo.method.body != null && (field == null || setterInfo.field != field), + field != null && getterInfo?.field == field, + field != null && setterInfo?.field == field, specialSetterAccess, - getterInfo?.superProperty != null || setterInfo?.superProperty != null) + isOverride, + isAbstract) if (field != null) { memberToPropertyInfo[field] = propertyInfo diff --git a/j2k/testData/fileOrElement/dropAccessors/Overrides.java b/j2k/testData/fileOrElement/dropAccessors/Overrides.java index 5d447629074..89870c83432 100644 --- a/j2k/testData/fileOrElement/dropAccessors/Overrides.java +++ b/j2k/testData/fileOrElement/dropAccessors/Overrides.java @@ -15,6 +15,12 @@ interface I { void setSomething6(int value); } +interface I1 extends I { + void setSomething1(int value); + + int getSomething6(); +} + class B { public String getFromB1() { return ""; diff --git a/j2k/testData/fileOrElement/dropAccessors/Overrides.kt b/j2k/testData/fileOrElement/dropAccessors/Overrides.kt index e7c6a9443a5..6c8395865c3 100644 --- a/j2k/testData/fileOrElement/dropAccessors/Overrides.kt +++ b/j2k/testData/fileOrElement/dropAccessors/Overrides.kt @@ -1,7 +1,5 @@ -// ERROR: Property must be initialized -// ERROR: Property must be initialized -// ERROR: Property must be initialized -// ERROR: Property must be initialized +// ERROR: Abstract member cannot be accessed directly +// ERROR: Abstract member cannot be accessed directly internal interface I { val something1: Int @@ -16,6 +14,12 @@ internal interface I { fun setSomething6(value: Int) } +internal interface I1 : I { + fun setSomething1(value: Int) + + val something6: Int +} + internal open class B { open val fromB1: String get() { @@ -66,8 +70,14 @@ internal abstract class C(override val something1: Int) : B(), I { get() { return 0 } + set(value: Int) { + super.something4 = value + } override var something5: Int + get() { + return super.something5 + } set(value: Int) { } @@ -97,8 +107,14 @@ internal abstract class C(override val something1: Int) : B(), I { get() { return super.fromB3 } + set(value: String) { + super.fromB3 = value + } override var fromB4: String + get() { + return super.fromB4 + } set(value: String) { super.fromB4 = value } diff --git a/j2k/testData/fileOrElement/dropAccessors/OverridesOfExternalCode.kt b/j2k/testData/fileOrElement/dropAccessors/OverridesOfExternalCode.kt index 5917179870c..a840b7d7eff 100644 --- a/j2k/testData/fileOrElement/dropAccessors/OverridesOfExternalCode.kt +++ b/j2k/testData/fileOrElement/dropAccessors/OverridesOfExternalCode.kt @@ -1,9 +1,3 @@ -// ERROR: Property must be initialized -// ERROR: Property must be initialized -// ERROR: Property must be initialized -// ERROR: Property must be initialized -// ERROR: Property must be initialized -// ERROR: Property must be initialized import kotlinApi.KotlinClassWithProperties import javaApi.JavaClassWithProperties import javaApi.JavaClassDerivedFromKotlinClassWithProperties @@ -21,8 +15,14 @@ internal open class A : KotlinClassWithProperties() { get() { return super.someVar2 } + set(value: String) { + super.someVar2 = value + } override var someVar3: String + get() { + return super.someVar3 + } set(s: String) { super.someVar3 = s } @@ -31,6 +31,9 @@ internal open class A : KotlinClassWithProperties() { get() { return super.someVar4 } + set(value: String) { + super.someVar4 = value + } override val someVal: String get() { @@ -85,6 +88,9 @@ internal class C : A() { get() { return super.someVar1 } + set(value: String) { + super.someVar1 = value + } } internal class D : JavaClassDerivedFromKotlinClassWithProperties() { @@ -92,8 +98,14 @@ internal class D : JavaClassDerivedFromKotlinClassWithProperties() { get() { return "a" } + set(value: String) { + super.someVar1 = value + } override var someVar2: String + get() { + return super.someVar2 + } set(value: String) { }