[K2 Serialization] Store serializable properties in metadata extension

This solution is temporary (see KT-64694 for details) ^KT-64312 Fixed
2024-01-03 14:01:38 +02:00
parent fb3963dead
commit c2cbbecfe9
11 changed files with 116 additions and 5 deletions
@@ -14,6 +14,7 @@ dependencies {
    compileOnly(project(":compiler:ir.tree"))
    compileOnly(project(":compiler:fir:fir2ir"))
    compileOnly(project(":compiler:fir:tree"))
+    compileOnly(project(":compiler:fir:fir-deserialization"))
    compileOnly(project(":js:js.frontend"))
    compileOnly(project(":js:js.translator"))
    compileOnly(project(":kotlin-util-klib-metadata"))
@@ -8,6 +8,7 @@ package org.jetbrains.kotlinx.serialization.compiler.backend.ir
 import org.jetbrains.kotlin.descriptors.DescriptorVisibilities
 import org.jetbrains.kotlin.fir.declarations.impl.FirDefaultPropertyGetter
 import org.jetbrains.kotlin.fir.declarations.impl.FirPrimaryConstructor
+import org.jetbrains.kotlin.fir.deserialization.registeredInSerializationPluginMetadataExtension
 import org.jetbrains.kotlin.fir.lazy.Fir2IrLazyProperty
 import org.jetbrains.kotlin.ir.ObsoleteDescriptorBasedAPI
 import org.jetbrains.kotlin.ir.declarations.IrClass
@@ -57,11 +58,12 @@ fun IrProperty.analyzeIfFromAnotherModule(): Pair<Boolean, Boolean> {
        val hasDefault = descriptor.declaresDefaultValue()
        hasDefault to (descriptor.backingField != null || hasDefault)
    } else if (this is Fir2IrLazyProperty) {
-        // Fir2IrLazyProperty after K2 correctly deserializes information about backing field.
-        // However, nor Fir2IrLazyProp nor deserialized FirProperty do not store default value (initializer expression) for property,
+        // Deserialized properties don't contain information about backing field, so we should extract this information from the
+        // attribute, which is set if the property was mentioned in SerializationPluginMetadataExtensions.
+        // Also, deserialized properties do not store default value (initializer expression) for property,
        // so we either should find corresponding constructor parameter and check its default, or rely on less strict check for default getter.
        // Comments are copied from PropertyDescriptor.declaresDefaultValue() as it has similar logic.
-        val hasBackingField = backingField != null
+        val hasBackingField = fir.symbol.registeredInSerializationPluginMetadataExtension
        val matchingPrimaryConstructorParam = containingClass?.declarations?.filterIsInstance<FirPrimaryConstructor>()
            ?.singleOrNull()?.valueParameters?.find { it.name == this.name }
        if (matchingPrimaryConstructorParam != null) {
@@ -122,10 +124,14 @@ internal fun serializablePropertiesForIrBackend(
        .map {
            val isConstructorParameterWithDefault = primaryParamsAsProps[it] ?: false
            val (isPropertyFromAnotherModuleDeclaresDefaultValue, isPropertyWithBackingFieldFromAnotherModule) = it.analyzeIfFromAnotherModule()
+            val hasBackingField = when (it.origin) {
+                IrDeclarationOrigin.IR_EXTERNAL_DECLARATION_STUB -> isPropertyWithBackingFieldFromAnotherModule
+                else -> it.backingField != null
+            }
            IrSerializableProperty(
                it,
                isConstructorParameterWithDefault,
-                it.backingField != null || isPropertyWithBackingFieldFromAnotherModule,
+                hasBackingField,
                it.backingField?.initializer.let { init -> init != null && !init.expression.isInitializePropertyFromParameter() } || isConstructorParameterWithDefault
                        || isPropertyFromAnotherModuleDeclaresDefaultValue,
                typeReplacement?.get(it) ?: it.getter!!.returnType as IrSimpleType
@@ -8,6 +8,7 @@ plugins {
 dependencies {
    compileOnly(project(":compiler:fir:cones"))
    compileOnly(project(":compiler:fir:tree"))
+    compileOnly(project(":compiler:fir:fir-serialization"))
    compileOnly(project(":compiler:fir:resolve"))
    compileOnly(project(":compiler:fir:plugin-utils"))
    compileOnly(project(":compiler:fir:entrypoint"))
@@ -6,7 +6,9 @@
 package org.jetbrains.kotlinx.serialization.compiler.fir

 import org.jetbrains.kotlin.fir.FirSession
+import org.jetbrains.kotlin.fir.declarations.FirDeclarationOrigin
 import org.jetbrains.kotlin.fir.declarations.utils.hasBackingField
+import org.jetbrains.kotlin.fir.deserialization.registeredInSerializationPluginMetadataExtension
 import org.jetbrains.kotlin.fir.resolve.defaultType
 import org.jetbrains.kotlin.fir.symbols.impl.FirPropertySymbol
 import org.jetbrains.kotlin.fir.types.ConeKotlinType
@@ -28,7 +30,14 @@ class FirSerializableProperty(

    override val optional: Boolean = !propertySymbol.getSerialRequired(session) && declaresDefaultValue

-    override val transient: Boolean = propertySymbol.hasSerialTransient(session) || !propertySymbol.hasBackingField
+    override val transient: Boolean = run {
+        if (propertySymbol.hasSerialTransient(session)) return@run true
+        val hasBackingField = when (propertySymbol.origin) {
+            FirDeclarationOrigin.Library -> propertySymbol.registeredInSerializationPluginMetadataExtension
+            else -> propertySymbol.hasBackingField
+        }
+        !hasBackingField
+    }

    val serializableWith: ConeKotlinType? = propertySymbol.getSerializableWith(session)
        ?: analyzeSpecialSerializers(session, propertySymbol.resolvedAnnotationsWithArguments)?.defaultType()
@@ -5,6 +5,7 @@

 package org.jetbrains.kotlinx.serialization.compiler.fir

+import org.jetbrains.kotlin.fir.extensions.FirExtensionApiInternals
 import org.jetbrains.kotlin.fir.extensions.FirExtensionRegistrar
 import org.jetbrains.kotlinx.serialization.compiler.fir.checkers.FirSerializationCheckersComponent
 import org.jetbrains.kotlinx.serialization.compiler.fir.services.ContextualSerializersProvider
@@ -17,6 +18,8 @@ class FirSerializationExtensionRegistrar : FirExtensionRegistrar() {
        +::SerializationFirResolveExtension
        +::SerializationFirSupertypesExtension
        +::FirSerializationCheckersComponent
+        @OptIn(FirExtensionApiInternals::class)
+        +::FirSerializationMetadataSerializerPlugin

        // services
        +::DependencySerializationInfoProvider
@@ -0,0 +1,33 @@
+/*
+ * Copyright 2010-2024 JetBrains s.r.o. and Kotlin Programming Language contributors.
+ * Use of this source code is governed by the Apache 2.0 license that can be found in the license/LICENSE.txt file.
+ */
+
+package org.jetbrains.kotlinx.serialization.compiler.fir
+
+import org.jetbrains.kotlin.descriptors.Modality
+import org.jetbrains.kotlin.fir.FirSession
+import org.jetbrains.kotlin.fir.declarations.utils.modality
+import org.jetbrains.kotlin.fir.extensions.FirExtensionApiInternals
+import org.jetbrains.kotlin.fir.serialization.FirElementAwareStringTable
+import org.jetbrains.kotlin.fir.serialization.FirMetadataSerializerPlugin
+import org.jetbrains.kotlin.fir.symbols.impl.FirRegularClassSymbol
+import org.jetbrains.kotlin.metadata.SerializationPluginMetadataExtensions
+import org.jetbrains.kotlinx.serialization.compiler.fir.services.serializablePropertiesProvider
+
+@OptIn(FirExtensionApiInternals::class)
+class FirSerializationMetadataSerializerPlugin(session: FirSession) : FirMetadataSerializerPlugin(session) {
+    override fun registerProtoExtensions(
+        symbol: FirRegularClassSymbol,
+        stringTable: FirElementAwareStringTable,
+        protoRegistrar: ProtoRegistrar,
+    ) {
+        if (!symbol.needSaveProgramOrder) return
+        val properties = session.serializablePropertiesProvider.getSerializablePropertiesForClass(symbol)
+        val propertyNames = properties.serializableProperties.map { stringTable.getStringIndex(it.propertySymbol.name.asString()) }
+        protoRegistrar.setExtension(SerializationPluginMetadataExtensions.propertiesNamesInProgramOrder, propertyNames)
+    }
+
+    private val FirRegularClassSymbol.needSaveProgramOrder: Boolean
+        get() = (modality == Modality.OPEN || modality == Modality.ABSTRACT) && with(session) { isInternalSerializable }
+}
@@ -0,0 +1,34 @@
+// ISSUE: KT-64312
+// WITH_STDLIB
+// MODULE: lib
+import kotlinx.serialization.*
+import kotlinx.serialization.json.*
+
+@Serializable
+abstract class Base {
+    @SerialName("properties")
+    protected var propertiesInternal: String? = null
+
+    val properties: String // should not be serialized because it does not have a backing field
+        get() = propertiesInternal?.reversed().orEmpty()
+}
+
+// MODULE: main(lib)
+import kotlinx.serialization.*
+import kotlinx.serialization.json.*
+
+@Serializable
+class Derived: Base() {
+    fun initialize() {
+        propertiesInternal = "KO"
+    }
+}
+
+fun box(): String {
+    val expected = Derived().also { it.initialize() }
+    val json = Json.encodeToString(Derived.serializer(), expected)
+    if (json != """{"properties":"KO"}""") return "Fail: $json"
+    val actual = Json.decodeFromString(Derived.serializer(), json)
+    if (expected.properties != actual.properties) return "Fail: $actual"
+    return actual.properties
+}
@@ -25,6 +25,12 @@ public class SerializationFirJsBoxTestGenerated extends AbstractSerializationFir
        KtTestUtil.assertAllTestsPresentByMetadataWithExcluded(this.getClass(), new File("plugins/kotlinx-serialization/testData/boxIr"), Pattern.compile("^(.+)\\.kt$"), null, TargetBackend.JS_IR, true);
    }

+    @Test
+    @TestMetadata("clashBetweenSerializableAndNonSerializableProperty.kt")
+    public void testClashBetweenSerializableAndNonSerializableProperty() throws Exception {
+        runTest("plugins/kotlinx-serialization/testData/boxIr/clashBetweenSerializableAndNonSerializableProperty.kt");
+    }
+
    @Test
    @TestMetadata("constValInSerialName.kt")
    public void testConstValInSerialName() throws Exception {
@@ -45,6 +45,12 @@ public class SerializationFirLightTreeBlackBoxTestGenerated extends AbstractSeri
            runTest("plugins/kotlinx-serialization/testData/boxIr/caching.kt");
        }

+        @Test
+        @TestMetadata("clashBetweenSerializableAndNonSerializableProperty.kt")
+        public void testClashBetweenSerializableAndNonSerializableProperty() throws Exception {
+            runTest("plugins/kotlinx-serialization/testData/boxIr/clashBetweenSerializableAndNonSerializableProperty.kt");
+        }
+
        @Test
        @TestMetadata("classSerializerAsObject.kt")
        public void testClassSerializerAsObject() throws Exception {
@@ -43,6 +43,12 @@ public class SerializationIrBoxTestGenerated extends AbstractSerializationIrBoxT
        runTest("plugins/kotlinx-serialization/testData/boxIr/caching.kt");
    }

+    @Test
+    @TestMetadata("clashBetweenSerializableAndNonSerializableProperty.kt")
+    public void testClashBetweenSerializableAndNonSerializableProperty() throws Exception {
+        runTest("plugins/kotlinx-serialization/testData/boxIr/clashBetweenSerializableAndNonSerializableProperty.kt");
+    }
+
    @Test
    @TestMetadata("classSerializerAsObject.kt")
    public void testClassSerializerAsObject() throws Exception {
@@ -25,6 +25,12 @@ public class SerializationIrJsBoxTestGenerated extends AbstractSerializationIrJs
        KtTestUtil.assertAllTestsPresentByMetadataWithExcluded(this.getClass(), new File("plugins/kotlinx-serialization/testData/boxIr"), Pattern.compile("^(.+)\\.kt$"), null, TargetBackend.JS_IR, true);
    }

+    @Test
+    @TestMetadata("clashBetweenSerializableAndNonSerializableProperty.kt")
+    public void testClashBetweenSerializableAndNonSerializableProperty() throws Exception {
+        runTest("plugins/kotlinx-serialization/testData/boxIr/clashBetweenSerializableAndNonSerializableProperty.kt");
+    }
+
    @Test
    @TestMetadata("constValInSerialName.kt")
    public void testConstValInSerialName() throws Exception {