diff --git a/plugins/android-extensions/android-extensions-compiler/src/org/jetbrains/kotlin/android/parcel/ParcelableDeclarationChecker.kt b/plugins/android-extensions/android-extensions-compiler/src/org/jetbrains/kotlin/android/parcel/ParcelableDeclarationChecker.kt index 73aa7ebe4a5..e77bf43fbfe 100644 --- a/plugins/android-extensions/android-extensions-compiler/src/org/jetbrains/kotlin/android/parcel/ParcelableDeclarationChecker.kt +++ b/plugins/android-extensions/android-extensions-compiler/src/org/jetbrains/kotlin/android/parcel/ParcelableDeclarationChecker.kt @@ -20,19 +20,19 @@ import org.jetbrains.kotlin.android.synthetic.diagnostic.ErrorsAndroid import org.jetbrains.kotlin.descriptors.ClassDescriptor import org.jetbrains.kotlin.descriptors.DeclarationDescriptor import org.jetbrains.kotlin.descriptors.PropertyDescriptor +import org.jetbrains.kotlin.descriptors.SimpleFunctionDescriptor import org.jetbrains.kotlin.diagnostics.DiagnosticSink import org.jetbrains.kotlin.lexer.KtTokens import org.jetbrains.kotlin.name.FqName -import org.jetbrains.kotlin.psi.KtClass -import org.jetbrains.kotlin.psi.KtClassOrObject -import org.jetbrains.kotlin.psi.KtDeclaration -import org.jetbrains.kotlin.psi.KtProperty +import org.jetbrains.kotlin.psi.* import org.jetbrains.kotlin.resolve.BindingContext import org.jetbrains.kotlin.resolve.checkers.SimpleDeclarationChecker import org.jetbrains.kotlin.resolve.descriptorUtil.fqNameSafe +import org.jetbrains.kotlin.resolve.jvm.annotations.findJvmFieldAnnotation import org.jetbrains.kotlin.types.TypeUtils private val ANDROID_PARCELABLE_CLASS_FQNAME = FqName("android.os.Parcelable") +internal val ANDROID_PARCEL_CLASS_FQNAME = FqName("android.os.Parcel") class ParcelableDeclarationChecker : SimpleDeclarationChecker { private companion object { @@ -47,6 +47,13 @@ class ParcelableDeclarationChecker : SimpleDeclarationChecker { ) { when (descriptor) { is ClassDescriptor -> checkParcelableClass(descriptor, declaration, diagnosticHolder) + is SimpleFunctionDescriptor -> { + val containingClass = descriptor.containingDeclaration as? ClassDescriptor + val ktFunction = declaration as? KtFunction + if (containingClass != null && ktFunction != null) { + checkParcelableClassMethod(descriptor, containingClass, ktFunction, diagnosticHolder) + } + } is PropertyDescriptor -> { val containingClass = descriptor.containingDeclaration as? ClassDescriptor val ktProperty = declaration as? KtProperty @@ -57,19 +64,41 @@ class ParcelableDeclarationChecker : SimpleDeclarationChecker { } } + private fun checkParcelableClassMethod( + method: SimpleFunctionDescriptor, + containingClass: ClassDescriptor, + declaration: KtFunction, + diagnosticHolder: DiagnosticSink + ) { + if (!containingClass.isMagicParcelable) return + + if (method.isWriteToParcel()) { + val reportElement = declaration.modifierList?.getModifier(KtTokens.OVERRIDE_KEYWORD) ?: declaration.nameIdentifier ?: declaration + diagnosticHolder.report(ErrorsAndroid.OVERRIDING_WRITE_TO_PARCEL_IS_FORBIDDEN.on(reportElement)) + } + } + private fun checkParcelableClassProperty( property: PropertyDescriptor, containingClass: ClassDescriptor, declaration: KtProperty, diagnosticHolder: DiagnosticSink ) { - if (!containingClass.isMagicParcelable) return + if (containingClass.isMagicParcelable) { + // Do not report on calculated properties + if (declaration.getter?.hasBody() == true) return - // Do not report on calculated properties - if (declaration.getter?.hasBody() == true) return + if (!property.annotations.hasAnnotation(TRANSIENT_FQNAME)) { + diagnosticHolder.report(ErrorsAndroid.PROPERTY_WONT_BE_SERIALIZED.on(declaration.nameIdentifier ?: declaration)) + } + } - if (!property.annotations.hasAnnotation(TRANSIENT_FQNAME)) { - diagnosticHolder.report(ErrorsAndroid.PROPERTY_WONT_BE_SERIALIZED.on(declaration.nameIdentifier ?: declaration)) + // @JvmName is not applicable to property so we can check just the descriptor name + if (property.name.asString() == "CREATOR" && property.findJvmFieldAnnotation() != null && containingClass.isCompanionObject) { + val outerClass = containingClass.containingDeclaration as? ClassDescriptor + if (outerClass != null && outerClass.isMagicParcelable) { + diagnosticHolder.report(ErrorsAndroid.CREATOR_DEFINITION_IS_FORBIDDEN.on(declaration.nameIdentifier ?: declaration)) + } } } diff --git a/plugins/android-extensions/android-extensions-compiler/src/org/jetbrains/kotlin/android/parcel/ParcelableResolveExtension.kt b/plugins/android-extensions/android-extensions-compiler/src/org/jetbrains/kotlin/android/parcel/ParcelableResolveExtension.kt index e5f9dea17dd..77b70a1ff4d 100644 --- a/plugins/android-extensions/android-extensions-compiler/src/org/jetbrains/kotlin/android/parcel/ParcelableResolveExtension.kt +++ b/plugins/android-extensions/android-extensions-compiler/src/org/jetbrains/kotlin/android/parcel/ParcelableResolveExtension.kt @@ -27,6 +27,7 @@ import org.jetbrains.kotlin.name.ClassId import org.jetbrains.kotlin.name.FqName import org.jetbrains.kotlin.name.Name import org.jetbrains.kotlin.resolve.descriptorUtil.builtIns +import org.jetbrains.kotlin.resolve.descriptorUtil.fqNameSafe import org.jetbrains.kotlin.resolve.descriptorUtil.module import org.jetbrains.kotlin.resolve.extensions.SyntheticResolveExtension import org.jetbrains.kotlin.types.KotlinType @@ -81,7 +82,7 @@ class ParcelableResolveExtension : SyntheticResolveExtension { ) { if (name.asString() == DESCRIBE_CONTENTS.methodName && clazz.isMagicParcelable && result.none { it.isDescribeContents() }) { result += createMethod(clazz, DESCRIBE_CONTENTS, clazz.builtIns.intType) - } else if (name.asString() == WRITE_TO_PARCEL.methodName && clazz.isMagicParcelable) { + } else if (name.asString() == WRITE_TO_PARCEL.methodName && clazz.isMagicParcelable && result.none { it.isWriteToParcel() }) { val builtIns = clazz.builtIns val parcelClassType = resolveParcelClassType(clazz.module) result += createMethod(clazz, WRITE_TO_PARCEL, builtIns.unitType, "parcel" to parcelClassType, "flags" to builtIns.intType) @@ -89,10 +90,21 @@ class ParcelableResolveExtension : SyntheticResolveExtension { } private fun SimpleFunctionDescriptor.isDescribeContents(): Boolean { - return typeParameters.isEmpty() && valueParameters.isEmpty() && returnType?.let { type -> KotlinBuiltIns.isInt(type) } == true + return typeParameters.isEmpty() + && valueParameters.isEmpty() + && returnType?.let { type -> KotlinBuiltIns.isInt(type) } == true } } +internal fun SimpleFunctionDescriptor.isWriteToParcel(): Boolean { + return typeParameters.isEmpty() + && valueParameters.size == 2 + && valueParameters[0].type.isParcel() + && KotlinBuiltIns.isInt(valueParameters[1].type) +} + +private fun KotlinType.isParcel() = constructor.declarationDescriptor?.fqNameSafe == ANDROID_PARCEL_CLASS_FQNAME + interface ParcelableSyntheticComponent { val componentKind: ComponentKind diff --git a/plugins/android-extensions/android-extensions-compiler/src/org/jetbrains/kotlin/android/synthetic/diagnostic/DefaultErrorMessagesAndroid.kt b/plugins/android-extensions/android-extensions-compiler/src/org/jetbrains/kotlin/android/synthetic/diagnostic/DefaultErrorMessagesAndroid.kt index f98ca6d256b..62b2c244199 100644 --- a/plugins/android-extensions/android-extensions-compiler/src/org/jetbrains/kotlin/android/synthetic/diagnostic/DefaultErrorMessagesAndroid.kt +++ b/plugins/android-extensions/android-extensions-compiler/src/org/jetbrains/kotlin/android/synthetic/diagnostic/DefaultErrorMessagesAndroid.kt @@ -63,6 +63,12 @@ class DefaultErrorMessagesAndroid : DefaultErrorMessages.Extension { MAP.put(ErrorsAndroid.PROPERTY_WONT_BE_SERIALIZED, "Property would not be serialized into a 'Parcel'. Add '@Transient' annotation to it") + + MAP.put(ErrorsAndroid.OVERRIDING_WRITE_TO_PARCEL_IS_FORBIDDEN, + "Overriding 'writeToParcel' is forbidden. Use 'Parceler' companion object instead.") + + MAP.put(ErrorsAndroid.CREATOR_DEFINITION_IS_FORBIDDEN, + "'CREATOR' definition is forbidden. Use 'Parceler' companion object instead.") } } diff --git a/plugins/android-extensions/android-extensions-compiler/src/org/jetbrains/kotlin/android/synthetic/diagnostic/ErrorsAndroid.java b/plugins/android-extensions/android-extensions-compiler/src/org/jetbrains/kotlin/android/synthetic/diagnostic/ErrorsAndroid.java index db0bfc64b3b..a0298204da8 100644 --- a/plugins/android-extensions/android-extensions-compiler/src/org/jetbrains/kotlin/android/synthetic/diagnostic/ErrorsAndroid.java +++ b/plugins/android-extensions/android-extensions-compiler/src/org/jetbrains/kotlin/android/synthetic/diagnostic/ErrorsAndroid.java @@ -39,6 +39,8 @@ public interface ErrorsAndroid { DiagnosticFactory0 PARCELABLE_SHOULD_HAVE_PRIMARY_CONSTRUCTOR = DiagnosticFactory0.create(ERROR); DiagnosticFactory0 PARCELABLE_CONSTRUCTOR_PARAMETER_SHOULD_BE_VAL_OR_VAR = DiagnosticFactory0.create(ERROR); DiagnosticFactory0 PROPERTY_WONT_BE_SERIALIZED = DiagnosticFactory0.create(WARNING); + DiagnosticFactory0 OVERRIDING_WRITE_TO_PARCEL_IS_FORBIDDEN = DiagnosticFactory0.create(ERROR); + DiagnosticFactory0 CREATOR_DEFINITION_IS_FORBIDDEN = DiagnosticFactory0.create(ERROR); @SuppressWarnings("UnusedDeclaration") Object _initializer = new Object() { diff --git a/plugins/android-extensions/android-extensions-idea/testData/android/parcel/checker/customCreator.kt b/plugins/android-extensions/android-extensions-idea/testData/android/parcel/checker/customCreator.kt new file mode 100644 index 00000000000..7c1278337ed --- /dev/null +++ b/plugins/android-extensions/android-extensions-idea/testData/android/parcel/checker/customCreator.kt @@ -0,0 +1,16 @@ +package test + +import kotlinx.android.parcel.MagicParcel +import android.os.Parcelable +import android.os.Parcel + +@MagicParcel +class A(val a: String) : Parcelable { + companion object { + @JvmField + val CREATOR = object : Parcelable.Creator { + override fun createFromParcel(source: Parcel): A = A("") + override fun newArray(size: Int) = arrayOfNulls(size) + } + } +} \ No newline at end of file diff --git a/plugins/android-extensions/android-extensions-idea/testData/android/parcel/checker/customWriteToParcel.kt b/plugins/android-extensions/android-extensions-idea/testData/android/parcel/checker/customWriteToParcel.kt new file mode 100644 index 00000000000..9be5f4029f9 --- /dev/null +++ b/plugins/android-extensions/android-extensions-idea/testData/android/parcel/checker/customWriteToParcel.kt @@ -0,0 +1,21 @@ +package test + +import kotlinx.android.parcel.MagicParcel +import android.os.Parcelable +import android.os.Parcel + +@MagicParcel +class A(val a: String) : Parcelable { + override fun writeToParcel(p: Parcel?, flags: Int) {} + override fun describeContents() = 0 +} + +@MagicParcel +class B(val a: String) : Parcelable { + override fun writeToParcel(p: Parcel?, flags: Int) {} +} + +@MagicParcel +class C(val a: String) : Parcelable { + override fun writeToParcel(p: Parcel, flags: Int) {} +} \ No newline at end of file diff --git a/plugins/android-extensions/android-extensions-idea/tests/org/jetbrains/kotlin/android/ParcelCheckerTestGenerated.java b/plugins/android-extensions/android-extensions-idea/tests/org/jetbrains/kotlin/android/ParcelCheckerTestGenerated.java index 5a3252a5375..7959229c54b 100644 --- a/plugins/android-extensions/android-extensions-idea/tests/org/jetbrains/kotlin/android/ParcelCheckerTestGenerated.java +++ b/plugins/android-extensions/android-extensions-idea/tests/org/jetbrains/kotlin/android/ParcelCheckerTestGenerated.java @@ -42,6 +42,18 @@ public class ParcelCheckerTestGenerated extends AbstractParcelCheckerTest { doTest(fileName); } + @TestMetadata("customCreator.kt") + public void testCustomCreator() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("plugins/android-extensions/android-extensions-idea/testData/android/parcel/checker/customCreator.kt"); + doTest(fileName); + } + + @TestMetadata("customWriteToParcel.kt") + public void testCustomWriteToParcel() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("plugins/android-extensions/android-extensions-idea/testData/android/parcel/checker/customWriteToParcel.kt"); + doTest(fileName); + } + @TestMetadata("modality.kt") public void testModality() throws Exception { String fileName = KotlinTestUtils.navigationMetadata("plugins/android-extensions/android-extensions-idea/testData/android/parcel/checker/modality.kt");