From 16a2aec29626a5aab01d8b37e8fdcbfa78d16fa2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Steven=20Sch=C3=A4fer?= Date: Fri, 3 Dec 2021 15:22:01 +0100 Subject: [PATCH] Parcelize: Skip constructor checks when used with custom Parceler See https://issuetracker.google.com/177850558 --- .../parcelize/ParcelizeDeclarationChecker.kt | 8 +++- .../kotlin/parcelize/ParcelizeNames.kt | 1 + .../diagnostics/FirParcelizeClassChecker.kt | 8 ++++ .../FirParcelizeConstructorChecker.kt | 2 +- .../FirParcelizePropertyChecker.kt | 3 +- .../ir/ParcelizeIrTransformerBase.kt | 4 +- .../box/constructorWithoutValOrVar.kt | 35 ++++++++++++++++ .../testData/box/customParcelerChecks.kt | 42 +++++++++++++++++++ .../runners/ParcelizeBoxTestGenerated.java | 12 ++++++ .../runners/ParcelizeFirBoxTestGenerated.java | 12 ++++++ .../runners/ParcelizeIrBoxTestGenerated.java | 12 ++++++ 11 files changed, 134 insertions(+), 5 deletions(-) create mode 100644 plugins/parcelize/parcelize-compiler/testData/box/constructorWithoutValOrVar.kt create mode 100644 plugins/parcelize/parcelize-compiler/testData/box/customParcelerChecks.kt diff --git a/plugins/parcelize/parcelize-compiler/src/org/jetbrains/kotlin/parcelize/ParcelizeDeclarationChecker.kt b/plugins/parcelize/parcelize-compiler/src/org/jetbrains/kotlin/parcelize/ParcelizeDeclarationChecker.kt index 60fa6a402a5..7204dabc3e2 100644 --- a/plugins/parcelize/parcelize-compiler/src/org/jetbrains/kotlin/parcelize/ParcelizeDeclarationChecker.kt +++ b/plugins/parcelize/parcelize-compiler/src/org/jetbrains/kotlin/parcelize/ParcelizeDeclarationChecker.kt @@ -99,6 +99,7 @@ open class ParcelizeDeclarationChecker : DeclarationChecker { if (containingClass.isParcelize && (declaration.hasDelegate() || bindingContext[BindingContext.BACKING_FIELD_REQUIRED, property] == true) && !hasIgnoredOnParcel() + && !containingClass.hasCustomParceler() ) { val reportElement = declaration.nameIdentifier ?: declaration diagnosticHolder.report(ErrorsParcelize.PROPERTY_WONT_BE_SERIALIZED.on(reportElement)) @@ -192,6 +193,11 @@ open class ParcelizeDeclarationChecker : DeclarationChecker { } } + // The constructor checks are irrelevant for custom parcelers + if (descriptor.hasCustomParceler()) { + return + } + val primaryConstructor = declaration.primaryConstructor if (primaryConstructor == null && declaration.secondaryConstructors.isNotEmpty()) { val reportElement = declaration.nameIdentifier ?: declaration @@ -234,7 +240,7 @@ open class ParcelizeDeclarationChecker : DeclarationChecker { val type = descriptor.type - if (!type.isError && !containerClass.hasCustomParceler()) { + if (!type.isError) { val asmType = typeMapper.mapType(type, mode = TypeMappingMode.CLASS_DECLARATION) try { diff --git a/plugins/parcelize/parcelize-compiler/src/org/jetbrains/kotlin/parcelize/ParcelizeNames.kt b/plugins/parcelize/parcelize-compiler/src/org/jetbrains/kotlin/parcelize/ParcelizeNames.kt index 4eb52df6e0b..ba887ce6346 100644 --- a/plugins/parcelize/parcelize-compiler/src/org/jetbrains/kotlin/parcelize/ParcelizeNames.kt +++ b/plugins/parcelize/parcelize-compiler/src/org/jetbrains/kotlin/parcelize/ParcelizeNames.kt @@ -32,6 +32,7 @@ object ParcelizeNames { val TYPE_PARCELER_CLASS_IDS = createClassIds("TypeParceler") val WRITE_WITH_CLASS_IDS = createClassIds("WriteWith") val IGNORED_ON_PARCEL_CLASS_IDS = createClassIds("IgnoredOnParcel") + val PARCELER_CLASS_IDS = createClassIds("Parceler") val PARCELIZE_CLASS_CLASS_IDS = createClassIds("Parcelize") val RAW_VALUE_ANNOTATION_CLASS_IDS = createClassIds("RawValue") diff --git a/plugins/parcelize/parcelize-compiler/src/org/jetbrains/kotlin/parcelize/fir/diagnostics/FirParcelizeClassChecker.kt b/plugins/parcelize/parcelize-compiler/src/org/jetbrains/kotlin/parcelize/fir/diagnostics/FirParcelizeClassChecker.kt index a36629b4b20..36249763fdb 100644 --- a/plugins/parcelize/parcelize-compiler/src/org/jetbrains/kotlin/parcelize/fir/diagnostics/FirParcelizeClassChecker.kt +++ b/plugins/parcelize/parcelize-compiler/src/org/jetbrains/kotlin/parcelize/fir/diagnostics/FirParcelizeClassChecker.kt @@ -32,6 +32,7 @@ import org.jetbrains.kotlin.name.SpecialNames import org.jetbrains.kotlin.parcelize.ParcelizeNames.CREATOR_NAME import org.jetbrains.kotlin.parcelize.ParcelizeNames.OLD_PARCELER_ID import org.jetbrains.kotlin.parcelize.ParcelizeNames.PARCELABLE_ID +import org.jetbrains.kotlin.parcelize.ParcelizeNames.PARCELER_CLASS_IDS import org.jetbrains.kotlin.parcelize.ParcelizeNames.PARCELIZE_CLASS_CLASS_IDS import kotlin.contracts.ExperimentalContracts import kotlin.contracts.contract @@ -131,3 +132,10 @@ fun FirClassSymbol<*>?.isParcelize(session: FirSession): Boolean { symbol.annotations.any { it.classId in PARCELIZE_CLASS_CLASS_IDS } } } + +fun FirRegularClass.hasCustomParceler(session: FirSession): Boolean { + val companion = companionObjectSymbol ?: return false + return lookupSuperTypes(companion, lookupInterfaces = true, deep = true, useSiteSession = session).any { + it.classId in PARCELER_CLASS_IDS + } +} diff --git a/plugins/parcelize/parcelize-compiler/src/org/jetbrains/kotlin/parcelize/fir/diagnostics/FirParcelizeConstructorChecker.kt b/plugins/parcelize/parcelize-compiler/src/org/jetbrains/kotlin/parcelize/fir/diagnostics/FirParcelizeConstructorChecker.kt index ca1a5da8485..3a04133463f 100644 --- a/plugins/parcelize/parcelize-compiler/src/org/jetbrains/kotlin/parcelize/fir/diagnostics/FirParcelizeConstructorChecker.kt +++ b/plugins/parcelize/parcelize-compiler/src/org/jetbrains/kotlin/parcelize/fir/diagnostics/FirParcelizeConstructorChecker.kt @@ -23,7 +23,7 @@ object FirParcelizeConstructorChecker : FirConstructorChecker() { if (source.kind == KtFakeSourceElementKind.ImplicitConstructor) return val containingClass = context.containingDeclarations.last() as? FirRegularClass ?: return val containingClassSymbol = containingClass.symbol - if (!containingClassSymbol.isParcelize(context.session)) return + if (!containingClassSymbol.isParcelize(context.session) || containingClass.hasCustomParceler(context.session)) return if (declaration.valueParameters.isEmpty()) { reporter.reportOn(containingClass.source, KtErrorsParcelize.PARCELABLE_PRIMARY_CONSTRUCTOR_IS_EMPTY, context) diff --git a/plugins/parcelize/parcelize-compiler/src/org/jetbrains/kotlin/parcelize/fir/diagnostics/FirParcelizePropertyChecker.kt b/plugins/parcelize/parcelize-compiler/src/org/jetbrains/kotlin/parcelize/fir/diagnostics/FirParcelizePropertyChecker.kt index 46e670211cd..6dc7c10620d 100644 --- a/plugins/parcelize/parcelize-compiler/src/org/jetbrains/kotlin/parcelize/fir/diagnostics/FirParcelizePropertyChecker.kt +++ b/plugins/parcelize/parcelize-compiler/src/org/jetbrains/kotlin/parcelize/fir/diagnostics/FirParcelizePropertyChecker.kt @@ -38,7 +38,8 @@ object FirParcelizePropertyChecker : FirPropertyChecker() { if ( !fromPrimaryConstructor && (declaration.hasBackingField || declaration.delegate != null) && - !declaration.hasIgnoredOnParcel() + !declaration.hasIgnoredOnParcel() && + !containingClassSymbol.hasCustomParceler(context.session) ) { reporter.reportOn(declaration.source, KtErrorsParcelize.PROPERTY_WONT_BE_SERIALIZED, context) } diff --git a/plugins/parcelize/parcelize-compiler/src/org/jetbrains/kotlin/parcelize/ir/ParcelizeIrTransformerBase.kt b/plugins/parcelize/parcelize-compiler/src/org/jetbrains/kotlin/parcelize/ir/ParcelizeIrTransformerBase.kt index e4bc74459c4..232decd7cc5 100644 --- a/plugins/parcelize/parcelize-compiler/src/org/jetbrains/kotlin/parcelize/ir/ParcelizeIrTransformerBase.kt +++ b/plugins/parcelize/parcelize-compiler/src/org/jetbrains/kotlin/parcelize/ir/ParcelizeIrTransformerBase.kt @@ -184,8 +184,8 @@ abstract class ParcelizeIrTransformerBase( val topLevelScope = getParcelerScope() return constructor.valueParameters.map { parameter -> - val property = properties.first { it.name == parameter.name } - if (property.hasAnyAnnotation(IGNORED_ON_PARCEL_FQ_NAMES)) { + val property = properties.firstOrNull { it.name == parameter.name } + if (property == null || property.hasAnyAnnotation(IGNORED_ON_PARCEL_FQ_NAMES)) { null } else { val localScope = property.getParcelerScope(topLevelScope) diff --git a/plugins/parcelize/parcelize-compiler/testData/box/constructorWithoutValOrVar.kt b/plugins/parcelize/parcelize-compiler/testData/box/constructorWithoutValOrVar.kt new file mode 100644 index 00000000000..03499b0cf33 --- /dev/null +++ b/plugins/parcelize/parcelize-compiler/testData/box/constructorWithoutValOrVar.kt @@ -0,0 +1,35 @@ +// WITH_STDLIB + +@file:JvmName("TestKt") +package test + +import kotlinx.parcelize.* +import android.os.Parcel +import android.os.Parcelable + +@Parcelize +class A(_a: String) : Parcelable { + var a: String = _a + private set + + companion object : Parceler { + override fun A.write(parcel: Parcel, flags: Int) { + parcel.writeString(a) + } + + override fun create(parcel: Parcel) = A(parcel.readString()) + } +} + +fun box() = parcelTest { parcel -> + val test = A("OK") + test.writeToParcel(parcel, 0) + + val bytes = parcel.marshall() + parcel.unmarshall(bytes, 0, bytes.size) + parcel.setDataPosition(0) + + val test2 = parcelableCreator().createFromParcel(parcel) + + assert(test.a == test2.a) +} diff --git a/plugins/parcelize/parcelize-compiler/testData/box/customParcelerChecks.kt b/plugins/parcelize/parcelize-compiler/testData/box/customParcelerChecks.kt new file mode 100644 index 00000000000..14ed537c916 --- /dev/null +++ b/plugins/parcelize/parcelize-compiler/testData/box/customParcelerChecks.kt @@ -0,0 +1,42 @@ +// WITH_STDLIB + +@file:JvmName("TestKt") +package test + +import kotlinx.parcelize.* +import android.os.Parcel +import android.os.Parcelable + +class T(val value: String) + +// There's no warnings on empty constructors, secondary constructors, or +// non-parcelable types if there is a custom parceler. +@Parcelize +class A() : Parcelable { + var a: T = T("Fail") + + constructor(value: String) : this() { + a = T(value) + } + + companion object : Parceler { + override fun A.write(parcel: Parcel, flags: Int) { + parcel.writeString(a.value) + } + + override fun create(parcel: Parcel) = A(parcel.readString()) + } +} + +fun box() = parcelTest { parcel -> + val test = A("OK") + test.writeToParcel(parcel, 0) + + val bytes = parcel.marshall() + parcel.unmarshall(bytes, 0, bytes.size) + parcel.setDataPosition(0) + + val test2 = parcelableCreator().createFromParcel(parcel) + + assert(test.a.value == test2.a.value) +} diff --git a/plugins/parcelize/parcelize-compiler/tests-gen/org/jetbrains/kotlin/parcelize/test/runners/ParcelizeBoxTestGenerated.java b/plugins/parcelize/parcelize-compiler/tests-gen/org/jetbrains/kotlin/parcelize/test/runners/ParcelizeBoxTestGenerated.java index a8d4c17e07f..02b6581ecc0 100644 --- a/plugins/parcelize/parcelize-compiler/tests-gen/org/jetbrains/kotlin/parcelize/test/runners/ParcelizeBoxTestGenerated.java +++ b/plugins/parcelize/parcelize-compiler/tests-gen/org/jetbrains/kotlin/parcelize/test/runners/ParcelizeBoxTestGenerated.java @@ -73,6 +73,12 @@ public class ParcelizeBoxTestGenerated extends AbstractParcelizeBoxTest { runTest("plugins/parcelize/parcelize-compiler/testData/box/charSequence.kt"); } + @Test + @TestMetadata("constructorWithoutValOrVar.kt") + public void testConstructorWithoutValOrVar() throws Exception { + runTest("plugins/parcelize/parcelize-compiler/testData/box/constructorWithoutValOrVar.kt"); + } + @Test @TestMetadata("customNewArray.kt") public void testCustomNewArray() throws Exception { @@ -85,6 +91,12 @@ public class ParcelizeBoxTestGenerated extends AbstractParcelizeBoxTest { runTest("plugins/parcelize/parcelize-compiler/testData/box/customParcelable.kt"); } + @Test + @TestMetadata("customParcelerChecks.kt") + public void testCustomParcelerChecks() throws Exception { + runTest("plugins/parcelize/parcelize-compiler/testData/box/customParcelerChecks.kt"); + } + @Test @TestMetadata("customParcelerScoping.kt") public void testCustomParcelerScoping() throws Exception { diff --git a/plugins/parcelize/parcelize-compiler/tests-gen/org/jetbrains/kotlin/parcelize/test/runners/ParcelizeFirBoxTestGenerated.java b/plugins/parcelize/parcelize-compiler/tests-gen/org/jetbrains/kotlin/parcelize/test/runners/ParcelizeFirBoxTestGenerated.java index 4d4981770e1..4abb690de68 100644 --- a/plugins/parcelize/parcelize-compiler/tests-gen/org/jetbrains/kotlin/parcelize/test/runners/ParcelizeFirBoxTestGenerated.java +++ b/plugins/parcelize/parcelize-compiler/tests-gen/org/jetbrains/kotlin/parcelize/test/runners/ParcelizeFirBoxTestGenerated.java @@ -73,6 +73,12 @@ public class ParcelizeFirBoxTestGenerated extends AbstractParcelizeFirBoxTest { runTest("plugins/parcelize/parcelize-compiler/testData/box/charSequence.kt"); } + @Test + @TestMetadata("constructorWithoutValOrVar.kt") + public void testConstructorWithoutValOrVar() throws Exception { + runTest("plugins/parcelize/parcelize-compiler/testData/box/constructorWithoutValOrVar.kt"); + } + @Test @TestMetadata("customNewArray.kt") public void testCustomNewArray() throws Exception { @@ -85,6 +91,12 @@ public class ParcelizeFirBoxTestGenerated extends AbstractParcelizeFirBoxTest { runTest("plugins/parcelize/parcelize-compiler/testData/box/customParcelable.kt"); } + @Test + @TestMetadata("customParcelerChecks.kt") + public void testCustomParcelerChecks() throws Exception { + runTest("plugins/parcelize/parcelize-compiler/testData/box/customParcelerChecks.kt"); + } + @Test @TestMetadata("customParcelerScoping.kt") public void testCustomParcelerScoping() throws Exception { diff --git a/plugins/parcelize/parcelize-compiler/tests-gen/org/jetbrains/kotlin/parcelize/test/runners/ParcelizeIrBoxTestGenerated.java b/plugins/parcelize/parcelize-compiler/tests-gen/org/jetbrains/kotlin/parcelize/test/runners/ParcelizeIrBoxTestGenerated.java index 1444fb1be2a..3170a43523e 100644 --- a/plugins/parcelize/parcelize-compiler/tests-gen/org/jetbrains/kotlin/parcelize/test/runners/ParcelizeIrBoxTestGenerated.java +++ b/plugins/parcelize/parcelize-compiler/tests-gen/org/jetbrains/kotlin/parcelize/test/runners/ParcelizeIrBoxTestGenerated.java @@ -73,6 +73,12 @@ public class ParcelizeIrBoxTestGenerated extends AbstractParcelizeIrBoxTest { runTest("plugins/parcelize/parcelize-compiler/testData/box/charSequence.kt"); } + @Test + @TestMetadata("constructorWithoutValOrVar.kt") + public void testConstructorWithoutValOrVar() throws Exception { + runTest("plugins/parcelize/parcelize-compiler/testData/box/constructorWithoutValOrVar.kt"); + } + @Test @TestMetadata("customNewArray.kt") public void testCustomNewArray() throws Exception { @@ -85,6 +91,12 @@ public class ParcelizeIrBoxTestGenerated extends AbstractParcelizeIrBoxTest { runTest("plugins/parcelize/parcelize-compiler/testData/box/customParcelable.kt"); } + @Test + @TestMetadata("customParcelerChecks.kt") + public void testCustomParcelerChecks() throws Exception { + runTest("plugins/parcelize/parcelize-compiler/testData/box/customParcelerChecks.kt"); + } + @Test @TestMetadata("customParcelerScoping.kt") public void testCustomParcelerScoping() throws Exception {