Parcelize: Support @IgnoreOnParcel primary constructor parameters...

...with default values. See https://issuetracker.google.com/177850560
2021-12-03 14:18:55 +01:00
parent 00289d3514
commit 58c687e42d
15 changed files with 172 additions and 26 deletions
@@ -86,7 +86,9 @@ open class ParcelizeAnnotationChecker : CallChecker {

    private fun checkIgnoredOnParcelUsage(annotationEntry: KtAnnotationEntry, context: CallCheckerContext, element: KtModifierListOwner) {
        if (element is KtParameter && PsiTreeUtil.getParentOfType(element, KtDeclaration::class.java) is KtPrimaryConstructor) {
-            context.trace.report(ErrorsParcelize.INAPPLICABLE_IGNORED_ON_PARCEL_CONSTRUCTOR_PROPERTY.on(annotationEntry))
+            if (!element.hasDefaultValue()) {
+                context.trace.report(ErrorsParcelize.INAPPLICABLE_IGNORED_ON_PARCEL_CONSTRUCTOR_PROPERTY.on(annotationEntry))
+            }
        } else if (element !is KtProperty || PsiTreeUtil.getParentOfType(element, KtDeclaration::class.java) !is KtClassOrObject) {
            context.trace.report(ErrorsParcelize.INAPPLICABLE_IGNORED_ON_PARCEL.on(annotationEntry))
        }
@@ -226,6 +226,12 @@ open class ParcelizeDeclarationChecker : DeclarationChecker {
        }

        val descriptor = typeMapper.bindingContext[BindingContext.PRIMARY_CONSTRUCTOR_PARAMETER, parameter] ?: return
+
+        // Don't check parameters which won't be serialized
+        if (descriptor.annotations.any { it.fqName in IGNORED_ON_PARCEL_FQ_NAMES }) {
+            return
+        }
+
        val type = descriptor.type

        if (!type.isError && !containerClass.hasCustomParceler()) {
@@ -132,7 +132,7 @@ object DefaultErrorMessagesParcelize : DefaultErrorMessages.Extension {

        MAP.put(
            ErrorsParcelize.INAPPLICABLE_IGNORED_ON_PARCEL_CONSTRUCTOR_PROPERTY,
-            "'@IgnoredOnParcel' is inapplicable to properties declared in the primary constructor"
+            "'@IgnoredOnParcel' is inapplicable to properties without default value declared in the primary constructor"
        )

        MAP.put(
@@ -13,7 +13,8 @@ import org.jetbrains.kotlin.fir.analysis.checkers.context.CheckerContext
 import org.jetbrains.kotlin.fir.analysis.checkers.declaration.FirConstructorChecker
 import org.jetbrains.kotlin.fir.declarations.FirConstructor
 import org.jetbrains.kotlin.fir.declarations.FirRegularClass
-import org.jetbrains.kotlin.parcelize.fir.diagnostics.KtErrorsParcelize.PARCELABLE_CONSTRUCTOR_PARAMETER_SHOULD_BE_VAL_OR_VAR
+import org.jetbrains.kotlin.fir.expressions.classId
+import org.jetbrains.kotlin.parcelize.ParcelizeNames

 object FirParcelizeConstructorChecker : FirConstructorChecker() {
    override fun check(declaration: FirConstructor, context: CheckerContext, reporter: DiagnosticReporter) {
@@ -29,7 +30,23 @@ object FirParcelizeConstructorChecker : FirConstructorChecker() {
        } else {
            for (valueParameter in declaration.valueParameters) {
                if (valueParameter.source?.hasValOrVar() != true) {
-                    reporter.reportOn(valueParameter.source, PARCELABLE_CONSTRUCTOR_PARAMETER_SHOULD_BE_VAL_OR_VAR, context)
+                    reporter.reportOn(
+                        valueParameter.source,
+                        KtErrorsParcelize.PARCELABLE_CONSTRUCTOR_PARAMETER_SHOULD_BE_VAL_OR_VAR,
+                        context
+                    )
+                }
+                if (valueParameter.defaultValue == null) {
+                    val illegalAnnotation = valueParameter.annotations.firstOrNull {
+                        it.classId in ParcelizeNames.IGNORED_ON_PARCEL_CLASS_IDS
+                    }
+                    if (illegalAnnotation != null) {
+                        reporter.reportOn(
+                            illegalAnnotation.source,
+                            KtErrorsParcelize.INAPPLICABLE_IGNORED_ON_PARCEL_CONSTRUCTOR_PROPERTY,
+                            context
+                        )
+                    }
                }
            }
        }
@@ -44,7 +44,6 @@ object FirParcelizePropertyChecker : FirPropertyChecker() {
            }
            if (fromPrimaryConstructor) {
                checkParcelableClassProperty(declaration, containingClassSymbol, context, reporter)
-                checkIgnoredOnParcelUsage(declaration, context, reporter)
            }
        }

@@ -56,11 +55,6 @@ object FirParcelizePropertyChecker : FirPropertyChecker() {
        }
    }

-    private fun checkIgnoredOnParcelUsage(property: FirProperty, context: CheckerContext, reporter: DiagnosticReporter) {
-        val illegalAnnotation = property.annotations.firstOrNull { it.classId in IGNORED_ON_PARCEL_CLASS_IDS } ?: return
-        reporter.reportOn(illegalAnnotation.source, KtErrorsParcelize.INAPPLICABLE_IGNORED_ON_PARCEL_CONSTRUCTOR_PROPERTY, context)
-    }
-
    @Suppress("UNUSED_PARAMETER")
    private fun checkParcelableClassProperty(
        property: FirProperty,
@@ -159,7 +159,7 @@ object KtDefaultErrorMessagesParcelize {

        map.put(
            INAPPLICABLE_IGNORED_ON_PARCEL_CONSTRUCTOR_PROPERTY,
-            "'@IgnoredOnParcel' is inapplicable to properties declared in the primary constructor"
+            "'@IgnoredOnParcel' is inapplicable to properties without default value declared in the primary constructor"
        )

        map.put(
@@ -22,6 +22,7 @@ import org.jetbrains.kotlin.ir.visitors.IrElementVisitorVoid
 import org.jetbrains.kotlin.name.Name
 import org.jetbrains.kotlin.parcelize.ParcelizeNames.CREATE_FROM_PARCEL_NAME
 import org.jetbrains.kotlin.parcelize.ParcelizeNames.CREATOR_NAME
+import org.jetbrains.kotlin.parcelize.ParcelizeNames.IGNORED_ON_PARCEL_FQ_NAMES
 import org.jetbrains.kotlin.parcelize.ParcelizeNames.NEW_ARRAY_NAME
 import org.jetbrains.kotlin.parcelize.serializers.ParcelizeExtensionBase

@@ -34,8 +35,8 @@ abstract class ParcelizeIrTransformerBase(
    protected val deferredOperations = mutableListOf<() -> Unit>()
    protected fun defer(block: () -> Unit) = deferredOperations.add(block)

-    protected fun IrSimpleFunction.generateDescribeContentsBody(parcelableProperties: List<ParcelableProperty>) {
-        val flags = if (parcelableProperties.any { it.field.type.containsFileDescriptors }) 1 else 0
+    protected fun IrSimpleFunction.generateDescribeContentsBody(parcelableProperties: List<ParcelableProperty?>) {
+        val flags = if (parcelableProperties.any { it != null && it.field.type.containsFileDescriptors }) 1 else 0
        body = context.createIrBuilder(symbol).run {
            irExprBody(irInt(flags))
        }
@@ -44,7 +45,7 @@ abstract class ParcelizeIrTransformerBase(
    protected fun IrSimpleFunction.generateWriteToParcelBody(
        irClass: IrClass,
        parcelerObject: IrClass?,
-        parcelableProperties: List<ParcelableProperty>,
+        parcelableProperties: List<ParcelableProperty?>,
        receiverParameter: IrValueParameter,
        parcelParameter: IrValueParameter,
        flagsParameter: IrValueParameter
@@ -57,12 +58,14 @@ abstract class ParcelizeIrTransformerBase(

                    parcelableProperties.isNotEmpty() ->
                        for (property in parcelableProperties) {
-                            +writeParcelWith(
-                                property.parceler,
-                                parcelParameter,
-                                flagsParameter,
-                                irGetField(irGet(receiverParameter), property.field)
-                            )
+                            if (property != null) {
+                                +writeParcelWith(
+                                    property.parceler,
+                                    parcelParameter,
+                                    flagsParameter,
+                                    irGetField(irGet(receiverParameter), property.field)
+                                )
+                            }
                        }

                    else ->
@@ -77,7 +80,7 @@ abstract class ParcelizeIrTransformerBase(
        }
    }

-    protected fun generateCreator(declaration: IrClass, parcelerObject: IrClass?, parcelableProperties: List<ParcelableProperty>) {
+    protected fun generateCreator(declaration: IrClass, parcelerObject: IrClass?, parcelableProperties: List<ParcelableProperty?>) {
        // Since the `CREATOR` object cannot refer to the type parameters of the parcelable class we use a star projected type
        val declarationType = declaration.symbol.starProjectedType
        val creatorType = androidSymbols.androidOsParcelableCreator.typeWith(declarationType)
@@ -136,7 +139,9 @@ abstract class ParcelizeIrTransformerBase(
                                    parcelableProperties.isNotEmpty() ->
                                        irCall(declaration.primaryConstructor!!).apply {
                                            for ((index, property) in parcelableProperties.withIndex()) {
-                                                putValueArgument(index, readParcelWith(property.parceler, parcelParameter))
+                                                if (property != null) {
+                                                    putValueArgument(index, readParcelWith(property.parceler, parcelParameter))
+                                                }
                                            }
                                        }

@@ -171,7 +176,7 @@ abstract class ParcelizeIrTransformerBase(

    private val serializerFactory = IrParcelSerializerFactory(androidSymbols)

-    protected val IrClass.parcelableProperties: List<ParcelableProperty>
+    protected val IrClass.parcelableProperties: List<ParcelableProperty?>
        get() {
            if (kind != ClassKind.CLASS) return emptyList()

@@ -180,9 +185,13 @@ abstract class ParcelizeIrTransformerBase(

            return constructor.valueParameters.map { parameter ->
                val property = properties.first { it.name == parameter.name }
-                val localScope = property.getParcelerScope(topLevelScope)
-                ParcelableProperty(property.backingField!!) {
-                    serializerFactory.get(parameter.type, parcelizeType = defaultType, scope = localScope)
+                if (property.hasAnyAnnotation(IGNORED_ON_PARCEL_FQ_NAMES)) {
+                    null
+                } else {
+                    val localScope = property.getParcelerScope(topLevelScope)
+                    ParcelableProperty(property.backingField!!) {
+                        serializerFactory.get(parameter.type, parcelizeType = defaultType, scope = localScope)
+                    }
                }
            }
        }
@@ -0,0 +1,51 @@
+// WITH_STDLIB
+// IGNORE_BACKEND: JVM
+
+@file:JvmName("TestKt")
+package test
+
+import kotlinx.parcelize.*
+import android.os.Parcel
+import android.os.Parcelable
+
+class T(val value: Int)
+
+@Parcelize
+class A(
+    val x: String,
+    @IgnoredOnParcel
+    val i1: String = "default",
+    @IgnoredOnParcel
+    val i2: T = T(10),
+    val d: String = "default",
+) : Parcelable {
+    @IgnoredOnParcel
+    val a: String = x
+}
+
+@Parcelize
+object B : Parcelable {
+    @IgnoredOnParcel
+    val x: T = T(2)
+}
+
+fun box() = parcelTest { parcel ->
+    val a1 = A("X", "i1", T(0), "d")
+
+    a1.writeToParcel(parcel, 0)
+    B.writeToParcel(parcel, 0)
+
+    val bytes = parcel.marshall()
+    parcel.unmarshall(bytes, 0, bytes.size)
+    parcel.setDataPosition(0)
+
+    val a2 = parcelableCreator<A>().createFromParcel(parcel)
+    val b = parcelableCreator<B>().createFromParcel(parcel)
+
+    assert(a1.x == a2.x)
+    assert(a2.i1 == "default")
+    assert(a2.i2.value == 10)
+    assert(a1.d == a2.d)
+    assert(a1.a == a2.a)
+    assert(b == B)
+}
@@ -0,0 +1,9 @@
+// FIR_IDENTICAL
+package test
+
+import android.os.Parcelable
+import kotlinx.parcelize.Parcelize
+import kotlinx.parcelize.IgnoredOnParcel
+
+@Parcelize
+class A(@IgnoredOnParcel val x: String = "OK") : Parcelable
@@ -0,0 +1,16 @@
+// FIR_IDENTICAL
+package test
+
+import android.os.Parcelable
+import kotlinx.parcelize.Parcelize
+import kotlinx.parcelize.IgnoredOnParcel
+
+class T(val x: Int)
+
+@Parcelize
+class A(
+    // T is not parcelable, but we don't need it to be since it is not being serialized.
+    @IgnoredOnParcel val x: T = T(0)
+) : Parcelable {
+    @IgnoredOnParcel val y: T = x
+}
@@ -66,6 +66,18 @@ public class FirParcelizeDiagnosticTestGenerated extends AbstractFirParcelizeDia
        runTest("plugins/parcelize/parcelize-compiler/testData/diagnostics/emptyPrimaryConstructor.kt");
    }

+    @Test
+    @TestMetadata("ignoredOnParcelDefaultValues.kt")
+    public void testIgnoredOnParcelDefaultValues() throws Exception {
+        runTest("plugins/parcelize/parcelize-compiler/testData/diagnostics/ignoredOnParcelDefaultValues.kt");
+    }
+
+    @Test
+    @TestMetadata("ignoredOnParcelUnsupportedType.kt")
+    public void testIgnoredOnParcelUnsupportedType() throws Exception {
+        runTest("plugins/parcelize/parcelize-compiler/testData/diagnostics/ignoredOnParcelUnsupportedType.kt");
+    }
+
    @Test
    @TestMetadata("kt20062.kt")
    public void testKt20062() throws Exception {
@@ -151,6 +151,12 @@ public class ParcelizeBoxTestGenerated extends AbstractParcelizeBoxTest {
        runTest("plugins/parcelize/parcelize-compiler/testData/box/generics.kt");
    }

+    @Test
+    @TestMetadata("ignoredOnParcel.kt")
+    public void testIgnoredOnParcel() throws Exception {
+        runTest("plugins/parcelize/parcelize-compiler/testData/box/ignoredOnParcel.kt");
+    }
+
    @Test
    @TestMetadata("intArray.kt")
    public void testIntArray() throws Exception {
@@ -66,6 +66,18 @@ public class ParcelizeDiagnosticTestGenerated extends AbstractParcelizeDiagnosti
        runTest("plugins/parcelize/parcelize-compiler/testData/diagnostics/emptyPrimaryConstructor.kt");
    }

+    @Test
+    @TestMetadata("ignoredOnParcelDefaultValues.kt")
+    public void testIgnoredOnParcelDefaultValues() throws Exception {
+        runTest("plugins/parcelize/parcelize-compiler/testData/diagnostics/ignoredOnParcelDefaultValues.kt");
+    }
+
+    @Test
+    @TestMetadata("ignoredOnParcelUnsupportedType.kt")
+    public void testIgnoredOnParcelUnsupportedType() throws Exception {
+        runTest("plugins/parcelize/parcelize-compiler/testData/diagnostics/ignoredOnParcelUnsupportedType.kt");
+    }
+
    @Test
    @TestMetadata("kt20062.kt")
    public void testKt20062() throws Exception {
@@ -151,6 +151,12 @@ public class ParcelizeFirBoxTestGenerated extends AbstractParcelizeFirBoxTest {
        runTest("plugins/parcelize/parcelize-compiler/testData/box/generics.kt");
    }

+    @Test
+    @TestMetadata("ignoredOnParcel.kt")
+    public void testIgnoredOnParcel() throws Exception {
+        runTest("plugins/parcelize/parcelize-compiler/testData/box/ignoredOnParcel.kt");
+    }
+
    @Test
    @TestMetadata("intArray.kt")
    public void testIntArray() throws Exception {
@@ -151,6 +151,12 @@ public class ParcelizeIrBoxTestGenerated extends AbstractParcelizeIrBoxTest {
        runTest("plugins/parcelize/parcelize-compiler/testData/box/generics.kt");
    }

+    @Test
+    @TestMetadata("ignoredOnParcel.kt")
+    public void testIgnoredOnParcel() throws Exception {
+        runTest("plugins/parcelize/parcelize-compiler/testData/box/ignoredOnParcel.kt");
+    }
+
    @Test
    @TestMetadata("intArray.kt")
    public void testIntArray() throws Exception {