azalea/kotlin-fork
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Parcelable: Correctly handle writeToParcel() overriding, report errors on custom writeToParcel() and CREATOR

Browse Source
This commit is contained in:
Yan Zhulanow
2017-06-29 21:00:05 +03:00
parent 96c9bcd820
commit aa5f9ee3ec
7 changed files with 109 additions and 11 deletions
Download Patch File Download Diff File Expand all files Collapse all files
plugins/android-extensions/android-extensions-compiler/src/org/jetbrains/kotlin/android/parcel/ParcelableDeclarationChecker.kt
+38 -9
View File
@@ -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))
}
}
}
plugins/android-extensions/android-extensions-compiler/src/org/jetbrains/kotlin/android/parcel/ParcelableResolveExtension.kt
+14 -2
View File
@@ -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
plugins/android-extensions/android-extensions-compiler/src/org/jetbrains/kotlin/android/synthetic/diagnostic/DefaultErrorMessagesAndroid.kt
+6
View File
@@ -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.")
}
}
plugins/android-extensions/android-extensions-compiler/src/org/jetbrains/kotlin/android/synthetic/diagnostic/ErrorsAndroid.java
+2
View File
@@ -39,6 +39,8 @@ public interface ErrorsAndroid {
DiagnosticFactory0<PsiElement> PARCELABLE_SHOULD_HAVE_PRIMARY_CONSTRUCTOR = DiagnosticFactory0.create(ERROR);
DiagnosticFactory0<PsiElement> PARCELABLE_CONSTRUCTOR_PARAMETER_SHOULD_BE_VAL_OR_VAR = DiagnosticFactory0.create(ERROR);
DiagnosticFactory0<PsiElement> PROPERTY_WONT_BE_SERIALIZED = DiagnosticFactory0.create(WARNING);
DiagnosticFactory0<PsiElement> OVERRIDING_WRITE_TO_PARCEL_IS_FORBIDDEN = DiagnosticFactory0.create(ERROR);
DiagnosticFactory0<PsiElement> CREATOR_DEFINITION_IS_FORBIDDEN = DiagnosticFactory0.create(ERROR);
@SuppressWarnings("UnusedDeclaration")
Object _initializer = new Object() {
plugins/android-extensions/android-extensions-idea/testData/android/parcel/checker/customCreator.kt
Vendored
+16
View File
@@ -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 <error descr="[CREATOR_DEFINITION_IS_FORBIDDEN] 'CREATOR' definition is forbidden. Use 'Parceler' nested object instead.">CREATOR</error> = object : Parcelable.Creator<A> {
override fun createFromParcel(source: Parcel): A = A("")
override fun newArray(size: Int) = arrayOfNulls<A>(size)
}
}
}
plugins/android-extensions/android-extensions-idea/testData/android/parcel/checker/customWriteToParcel.kt
Vendored
+21
View File
@@ -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 {
<error descr="[OVERRIDING_WRITE_TO_PARCEL_IS_FORBIDDEN] Overriding 'writeToParcel' is forbidden. Use 'Parceler' nested object instead.">override</error> fun writeToParcel(p: Parcel?, flags: Int) {}
override fun describeContents() = 0
}
@MagicParcel
class B(val a: String) : Parcelable {
<error descr="[OVERRIDING_WRITE_TO_PARCEL_IS_FORBIDDEN] Overriding 'writeToParcel' is forbidden. Use 'Parceler' nested object instead.">override</error> fun writeToParcel(p: Parcel?, flags: Int) {}
}
@MagicParcel
class C(val a: String) : Parcelable {
<error descr="[OVERRIDING_WRITE_TO_PARCEL_IS_FORBIDDEN] Overriding 'writeToParcel' is forbidden. Use 'Parceler' nested object instead.">override</error> fun writeToParcel(p: Parcel, flags: Int) {}
}
plugins/android-extensions/android-extensions-idea/tests/org/jetbrains/kotlin/android/ParcelCheckerTestGenerated.java
+12
View File
@@ -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");