Parcelable: Fixes after review

This commit is contained in:
Yan Zhulanow
2017-07-07 22:23:04 +03:00
parent d0e4b236a7
commit 38449caaed
12 changed files with 134 additions and 40 deletions
@@ -188,7 +188,7 @@ class ParcelableCodegenExtension : ExpressionCodegenExtension {
val asmType = codegen.typeMapper.mapType(type)
asmConstructorParameters.append(asmType.descriptor)
val serializer = ParcelSerializer.get(type, asmType, codegen.typeMapper, forceBoxed = false, strict = false)
val serializer = ParcelSerializer.get(type, asmType, codegen.typeMapper)
v.load(1, parcelAsmType)
serializer.readValue(v)
}
@@ -276,12 +276,12 @@ class ParcelableCodegenExtension : ExpressionCodegenExtension {
if (parcelerObject != null) {
val newArrayMethod = parcelerObject.unsubstitutedMemberScope
.getContributedFunctions(Name.identifier("newArray"), NoLookupLocation.WHEN_GET_ALL_DESCRIPTORS)
.filter {
.firstOrNull {
it.typeParameters.isEmpty()
&& it.kind == CallableMemberDescriptor.Kind.DECLARATION
&& (it.valueParameters.size == 1 && KotlinBuiltIns.isInt(it.valueParameters[0].type))
&& !((it.containingDeclaration as? ClassDescriptor)?.defaultType?.isParceler() ?: true)
}.firstOrNull()
}
if (newArrayMethod != null) {
val containerAsmType = codegen.typeMapper.mapType(parcelableClass.defaultType)
@@ -17,6 +17,7 @@
package org.jetbrains.kotlin.android.parcel
import org.jetbrains.kotlin.android.parcel.serializers.ParcelSerializer
import org.jetbrains.kotlin.android.parcel.serializers.isParcelable
import org.jetbrains.kotlin.android.synthetic.diagnostic.ErrorsAndroid
import org.jetbrains.kotlin.codegen.ClassBuilderMode
import org.jetbrains.kotlin.codegen.state.IncompatibleClassTracker
@@ -65,7 +66,7 @@ class ParcelableDeclarationChecker : SimpleDeclarationChecker {
val containingClass = descriptor.containingDeclaration as? ClassDescriptor
val ktProperty = declaration as? KtProperty
if (containingClass != null && ktProperty != null) {
checkParcelableClassProperty(descriptor, containingClass, ktProperty, diagnosticHolder)
checkParcelableClassProperty(descriptor, containingClass, ktProperty, diagnosticHolder, bindingContext)
}
}
}
@@ -79,9 +80,9 @@ class ParcelableDeclarationChecker : SimpleDeclarationChecker {
) {
if (!containingClass.isMagicParcelable) return
if (method.isWriteToParcel()) {
if (method.isWriteToParcel() && declaration.hasModifier(KtTokens.OVERRIDE_KEYWORD)) {
val reportElement = declaration.modifierList?.getModifier(KtTokens.OVERRIDE_KEYWORD) ?: declaration.nameIdentifier ?: declaration
diagnosticHolder.report(ErrorsAndroid.OVERRIDING_WRITE_TO_PARCEL_IS_FORBIDDEN.on(reportElement))
diagnosticHolder.report(ErrorsAndroid.OVERRIDING_WRITE_TO_PARCEL_IS_NOT_ALLOWED.on(reportElement))
}
}
@@ -89,22 +90,21 @@ class ParcelableDeclarationChecker : SimpleDeclarationChecker {
property: PropertyDescriptor,
containingClass: ClassDescriptor,
declaration: KtProperty,
diagnosticHolder: DiagnosticSink
diagnosticHolder: DiagnosticSink,
bindingContext: BindingContext
) {
if (containingClass.isMagicParcelable) {
// 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 (containingClass.isMagicParcelable
&& (declaration.hasDelegate() || bindingContext[BindingContext.BACKING_FIELD_REQUIRED, property] == true)
&& !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))
diagnosticHolder.report(ErrorsAndroid.CREATOR_DEFINITION_IS_NOT_ALLOWED.on(declaration.nameIdentifier ?: declaration))
}
}
}
@@ -117,12 +117,18 @@ class ParcelableDeclarationChecker : SimpleDeclarationChecker {
) {
if (!descriptor.isMagicParcelable) return
if (declaration !is KtClass || (declaration.isAnnotation() || declaration.isInterface() || declaration.isEnum())) {
if (declaration !is KtClass || (declaration.isAnnotation() || declaration.isInterface())) {
val reportElement = (declaration as? KtClassOrObject)?.nameIdentifier ?: declaration
diagnosticHolder.report(ErrorsAndroid.PARCELABLE_SHOULD_BE_CLASS.on(reportElement))
return
}
if (declaration.isEnum()) {
val reportElement = (declaration as? KtClass)?.nameIdentifier ?: declaration
diagnosticHolder.report(ErrorsAndroid.PARCELABLE_SHOULD_BE_CLASS.on(reportElement))
return
}
val sealedOrAbstract = declaration.modifierList?.let { it.getModifier(KtTokens.ABSTRACT_KEYWORD) ?: it.getModifier(KtTokens.SEALED_KEYWORD) }
if (sealedOrAbstract != null) {
diagnosticHolder.report(ErrorsAndroid.PARCELABLE_SHOULD_BE_INSTANTIABLE.on(sealedOrAbstract))
@@ -142,9 +148,21 @@ class ParcelableDeclarationChecker : SimpleDeclarationChecker {
diagnosticHolder.report(ErrorsAndroid.NO_PARCELABLE_SUPERTYPE.on(declaration.nameIdentifier ?: declaration))
}
for (supertypeEntry in declaration.superTypeListEntries) {
supertypeEntry as? KtDelegatedSuperTypeEntry ?: continue
val delegateExpression = supertypeEntry.delegateExpression ?: continue
val type = bindingContext[BindingContext.TYPE, supertypeEntry.typeReference] ?: continue
if (type.isParcelable()) {
val reportElement = supertypeEntry.byKeywordNode?.psi ?: delegateExpression
diagnosticHolder.report(ErrorsAndroid.PARCELABLE_DELEGATE_IS_NOT_ALLOWED.on(reportElement))
}
}
val primaryConstructor = declaration.primaryConstructor
if (primaryConstructor == null && declaration.secondaryConstructors.isNotEmpty()) {
diagnosticHolder.report(ErrorsAndroid.PARCELABLE_SHOULD_HAVE_PRIMARY_CONSTRUCTOR.on(declaration.nameIdentifier ?: declaration))
} else if (primaryConstructor != null && primaryConstructor.valueParameters.isEmpty()) {
diagnosticHolder.report(ErrorsAndroid.PARCELABLE_PRIMARY_CONSTRUCTOR_IS_EMPTY.on(declaration.nameIdentifier ?: declaration))
}
val typeMapper = KotlinTypeMapper(
@@ -225,7 +225,6 @@ interface ParcelSerializer {
private fun KotlinType.isException() = matchesFqNameWithSupertypes("java.lang.Exception")
private fun KotlinType.isIBinder() = matchesFqNameWithSupertypes("android.os.IBinder")
private fun KotlinType.isIInterface() = matchesFqNameWithSupertypes("android.os.IInterface")
private fun KotlinType.isParcelable() = matchesFqNameWithSupertypes("android.os.Parcelable")
private fun KotlinType.isCharSequence() = matchesFqName("kotlin.CharSequence") || matchesFqName("java.lang.CharSequence")
private fun KotlinType.isNamedObject(): Boolean {
@@ -253,17 +252,19 @@ interface ParcelSerializer {
"Ljava/lang/Double;" -> true
else -> false
}
private fun KotlinType.matchesFqNameWithSupertypes(fqName: String): Boolean {
if (this.matchesFqName(fqName)) {
return true
}
return TypeUtils.getAllSupertypes(this).any { it.matchesFqName(fqName) }
}
private fun KotlinType.matchesFqName(fqName: String): Boolean {
return this.constructor.declarationDescriptor?.fqNameSafe?.asString() == fqName
}
}
}
internal fun KotlinType.isParcelable() = matchesFqNameWithSupertypes("android.os.Parcelable")
private fun KotlinType.matchesFqName(fqName: String): Boolean {
return this.constructor.declarationDescriptor?.fqNameSafe?.asString() == fqName
}
private fun KotlinType.matchesFqNameWithSupertypes(fqName: String): Boolean {
if (this.matchesFqName(fqName)) {
return true
}
return TypeUtils.getAllSupertypes(this).any { it.matchesFqName(fqName) }
}
@@ -43,6 +43,12 @@ class DefaultErrorMessagesAndroid : DefaultErrorMessages.Extension {
MAP.put(ErrorsAndroid.PARCELABLE_SHOULD_BE_CLASS,
"'Parcelable' should be a class")
MAP.put(ErrorsAndroid.PARCELABLE_DELEGATE_IS_NOT_ALLOWED,
"Delegating 'Parcelable' is now allowed")
MAP.put(ErrorsAndroid.PARCELABLE_SHOULD_NOT_BE_ENUM_CLASS,
"'Parcelable' should not be a 'enum class'")
MAP.put(ErrorsAndroid.PARCELABLE_SHOULD_BE_INSTANTIABLE,
"'Parcelable' should not be a 'sealed' or 'abstract' class")
@@ -58,20 +64,23 @@ class DefaultErrorMessagesAndroid : DefaultErrorMessages.Extension {
MAP.put(ErrorsAndroid.PARCELABLE_SHOULD_HAVE_PRIMARY_CONSTRUCTOR,
"'Parcelable' should have a primary constructor")
MAP.put(ErrorsAndroid.PARCELABLE_PRIMARY_CONSTRUCTOR_IS_EMPTY,
"The primary constructor is empty, no data will be serialized to 'Parcel'")
MAP.put(ErrorsAndroid.PARCELABLE_CONSTRUCTOR_PARAMETER_SHOULD_BE_VAL_OR_VAR,
"'Parcelable' constructor parameter should be 'val' or 'var'")
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.OVERRIDING_WRITE_TO_PARCEL_IS_NOT_ALLOWED,
"Overriding 'writeToParcel' is not allowed. Use 'Parceler' companion object instead.")
MAP.put(ErrorsAndroid.CREATOR_DEFINITION_IS_FORBIDDEN,
"'CREATOR' definition is forbidden. Use 'Parceler' companion object instead.")
MAP.put(ErrorsAndroid.CREATOR_DEFINITION_IS_NOT_ALLOWED,
"'CREATOR' definition is not allowed. Use 'Parceler' companion object instead.")
MAP.put(ErrorsAndroid.PARCELABLE_TYPE_NOT_SUPPORTED,
"Type is not supported by 'Parcelable'. " +
"Type is not directly supported by 'MagicParcelable'. " +
"Annotate the parameter type with '@RawValue' if you want it to be serialized using 'writeValue()'")
}
}
@@ -32,15 +32,18 @@ public interface ErrorsAndroid {
DiagnosticFactory0<KtExpression> UNSAFE_CALL_ON_PARTIALLY_DEFINED_RESOURCE = DiagnosticFactory0.create(WARNING);
DiagnosticFactory0<PsiElement> PARCELABLE_SHOULD_BE_CLASS = DiagnosticFactory0.create(ERROR);
DiagnosticFactory0<PsiElement> PARCELABLE_DELEGATE_IS_NOT_ALLOWED = DiagnosticFactory0.create(ERROR);
DiagnosticFactory0<PsiElement> PARCELABLE_SHOULD_NOT_BE_ENUM_CLASS = DiagnosticFactory0.create(ERROR);
DiagnosticFactory0<PsiElement> PARCELABLE_SHOULD_BE_INSTANTIABLE = DiagnosticFactory0.create(ERROR);
DiagnosticFactory0<PsiElement> PARCELABLE_CANT_BE_INNER_CLASS = DiagnosticFactory0.create(ERROR);
DiagnosticFactory0<PsiElement> PARCELABLE_CANT_BE_LOCAL_CLASS = DiagnosticFactory0.create(ERROR);
DiagnosticFactory0<PsiElement> NO_PARCELABLE_SUPERTYPE = DiagnosticFactory0.create(ERROR);
DiagnosticFactory0<PsiElement> PARCELABLE_SHOULD_HAVE_PRIMARY_CONSTRUCTOR = DiagnosticFactory0.create(ERROR);
DiagnosticFactory0<PsiElement> PARCELABLE_PRIMARY_CONSTRUCTOR_IS_EMPTY = DiagnosticFactory0.create(WARNING);
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);
DiagnosticFactory0<PsiElement> OVERRIDING_WRITE_TO_PARCEL_IS_NOT_ALLOWED = DiagnosticFactory0.create(ERROR);
DiagnosticFactory0<PsiElement> CREATOR_DEFINITION_IS_NOT_ALLOWED = DiagnosticFactory0.create(ERROR);
DiagnosticFactory0<PsiElement> PARCELABLE_TYPE_NOT_SUPPORTED = DiagnosticFactory0.create(ERROR);
@SuppressWarnings("UnusedDeclaration")