From fe5dbf75fa981726a3046803537dc69216e9e5bb Mon Sep 17 00:00:00 2001 From: Leonid Startsev Date: Tue, 8 Sep 2020 19:45:19 +0300 Subject: [PATCH] Add diagnostic to check whether provided custom serializer matches type of the property. Check for serializer type mismatch only when custom serializer is present Otherwise, there are too many false positives on e.g. PolymorphicSerializer #KT-36329 Fixed Fixes https://github.com/Kotlin/kotlinx.serialization/issues/830 --- .../diagnostic/SerializationErrors.java | 1 + .../SerializationPluginDeclarationChecker.kt | 28 ++++++++++++++- .../SerializationPluginErrorsRendering.kt | 11 ++++-- ...lizationPluginDiagnosticTestGenerated.java | 10 ++++++ .../testData/diagnostics/SerializableEnums.kt | 5 ++- .../diagnostics/SerializableEnums.txt | 36 +++++++++++++++++++ .../SerializerTypeCompatibleForSpecials.kt | 18 ++++++++++ .../diagnostics/SerializerTypeIncompatible.kt | 35 ++++++++++++++++++ 8 files changed, 140 insertions(+), 4 deletions(-) create mode 100644 plugins/kotlin-serialization/kotlin-serialization-compiler/testData/diagnostics/SerializerTypeCompatibleForSpecials.kt create mode 100644 plugins/kotlin-serialization/kotlin-serialization-compiler/testData/diagnostics/SerializerTypeIncompatible.kt diff --git a/plugins/kotlin-serialization/kotlin-serialization-compiler/src/org/jetbrains/kotlinx/serialization/compiler/diagnostic/SerializationErrors.java b/plugins/kotlin-serialization/kotlin-serialization-compiler/src/org/jetbrains/kotlinx/serialization/compiler/diagnostic/SerializationErrors.java index 63730db6d83..54af6738761 100644 --- a/plugins/kotlin-serialization/kotlin-serialization-compiler/src/org/jetbrains/kotlinx/serialization/compiler/diagnostic/SerializationErrors.java +++ b/plugins/kotlin-serialization/kotlin-serialization-compiler/src/org/jetbrains/kotlinx/serialization/compiler/diagnostic/SerializationErrors.java @@ -25,6 +25,7 @@ public interface SerializationErrors { DiagnosticFactory1 DUPLICATE_SERIAL_NAME = DiagnosticFactory1.create(ERROR); DiagnosticFactory1 SERIALIZER_NOT_FOUND = DiagnosticFactory1.create(ERROR); DiagnosticFactory2 SERIALIZER_NULLABILITY_INCOMPATIBLE = DiagnosticFactory2.create(ERROR); + DiagnosticFactory3 SERIALIZER_TYPE_INCOMPATIBLE = DiagnosticFactory3.create(WARNING); DiagnosticFactory0 TRANSIENT_MISSING_INITIALIZER = DiagnosticFactory0.create(ERROR); DiagnosticFactory0 TRANSIENT_IS_REDUNDANT = DiagnosticFactory0.create(WARNING); diff --git a/plugins/kotlin-serialization/kotlin-serialization-compiler/src/org/jetbrains/kotlinx/serialization/compiler/diagnostic/SerializationPluginDeclarationChecker.kt b/plugins/kotlin-serialization/kotlin-serialization-compiler/src/org/jetbrains/kotlinx/serialization/compiler/diagnostic/SerializationPluginDeclarationChecker.kt index 08b201f3ed8..40a2b4b9d33 100644 --- a/plugins/kotlin-serialization/kotlin-serialization-compiler/src/org/jetbrains/kotlinx/serialization/compiler/diagnostic/SerializationPluginDeclarationChecker.kt +++ b/plugins/kotlin-serialization/kotlin-serialization-compiler/src/org/jetbrains/kotlinx/serialization/compiler/diagnostic/SerializationPluginDeclarationChecker.kt @@ -9,6 +9,7 @@ import com.intellij.psi.PsiElement import org.jetbrains.kotlin.builtins.KotlinBuiltIns import org.jetbrains.kotlin.config.KotlinCompilerVersion import org.jetbrains.kotlin.descriptors.* +import org.jetbrains.kotlin.descriptors.annotations.Annotated import org.jetbrains.kotlin.diagnostics.DiagnosticFactory0 import org.jetbrains.kotlin.js.resolve.diagnostics.findPsi import org.jetbrains.kotlin.psi.* @@ -218,6 +219,7 @@ open class SerializationPluginDeclarationChecker : DeclarationChecker { val ktType = (propertyPsi as? KtCallableDeclaration)?.typeReference if (serializer != null) { val element = ktType?.typeElement + checkCustomSerializerMatch(it.module, it.type, it.descriptor, element, trace, propertyPsi) checkSerializerNullability(it.type, serializer.defaultType, element, trace, propertyPsi) generatorContextForAnalysis.checkTypeArguments(it.module, it.type, element, trace, propertyPsi) } else { @@ -260,6 +262,7 @@ open class SerializationPluginDeclarationChecker : DeclarationChecker { } val serializer = findTypeSerializerOrContextUnchecked(module, type) if (serializer != null) { + checkCustomSerializerMatch(module, type, type, element, trace, fallbackElement) checkSerializerNullability(type, serializer.defaultType, element, trace, fallbackElement) checkTypeArguments(module, type, element, trace, fallbackElement) } else { @@ -267,6 +270,28 @@ open class SerializationPluginDeclarationChecker : DeclarationChecker { } } + private fun checkCustomSerializerMatch( + module: ModuleDescriptor, + classType: KotlinType, + descriptor: Annotated, + element: KtElement?, + trace: BindingTrace, + fallbackElement: PsiElement + ) { + val serializerType = descriptor.annotations.serializableWith(module) ?: return + val serializerForType = serializerType.supertypes().find { isKSerializer(it) }?.arguments?.first()?.type ?: return + // Compare constructors because we do not care about generic arguments and nullability + if (classType.constructor != serializerForType.constructor) + trace.report( + SerializationErrors.SERIALIZER_TYPE_INCOMPATIBLE.on( + element ?: fallbackElement, + classType, + serializerType, + serializerForType + ) + ) + } + private fun checkSerializerNullability( classType: KotlinType, serializerType: KotlinType, @@ -277,7 +302,8 @@ open class SerializationPluginDeclarationChecker : DeclarationChecker { // @Serializable annotation has proper signature so this error would be caught in type checker val castedToKSerial = serializerType.supertypes().find { isKSerializer(it) } ?: return - if (!classType.isMarkedNullable && castedToKSerial.arguments.first().type.isMarkedNullable) + val serializerForType = castedToKSerial.arguments.first().type + if (!classType.isMarkedNullable && serializerForType.isMarkedNullable) trace.report( SerializationErrors.SERIALIZER_NULLABILITY_INCOMPATIBLE.on(element ?: fallbackElement, serializerType, classType), ) diff --git a/plugins/kotlin-serialization/kotlin-serialization-compiler/src/org/jetbrains/kotlinx/serialization/compiler/diagnostic/SerializationPluginErrorsRendering.kt b/plugins/kotlin-serialization/kotlin-serialization-compiler/src/org/jetbrains/kotlinx/serialization/compiler/diagnostic/SerializationPluginErrorsRendering.kt index a636eebbe02..185a9e8ff24 100644 --- a/plugins/kotlin-serialization/kotlin-serialization-compiler/src/org/jetbrains/kotlinx/serialization/compiler/diagnostic/SerializationPluginErrorsRendering.kt +++ b/plugins/kotlin-serialization/kotlin-serialization-compiler/src/org/jetbrains/kotlinx/serialization/compiler/diagnostic/SerializationPluginErrorsRendering.kt @@ -54,8 +54,15 @@ object SerializationPluginErrorsRendering : DefaultErrorMessages.Extension { MAP.put( SerializationErrors.SERIALIZER_NULLABILITY_INCOMPATIBLE, "Type ''{1}'' is non-nullable and therefore can not be serialized with serializer for nullable type ''{0}''", - Renderers.RENDER_TYPE_WITH_ANNOTATIONS, - Renderers.RENDER_TYPE_WITH_ANNOTATIONS + Renderers.RENDER_TYPE, + Renderers.RENDER_TYPE + ) + MAP.put( + SerializationErrors.SERIALIZER_TYPE_INCOMPATIBLE, + "Class ''{1}'', which is serializer for type ''{2}'', is applied here to type ''{0}''. This may lead to errors or incorrect behavior.", + Renderers.RENDER_TYPE, + Renderers.RENDER_TYPE, + Renderers.RENDER_TYPE ) MAP.put( SerializationErrors.TRANSIENT_MISSING_INITIALIZER, diff --git a/plugins/kotlin-serialization/kotlin-serialization-compiler/test/org/jetbrains/kotlinx/serialization/SerializationPluginDiagnosticTestGenerated.java b/plugins/kotlin-serialization/kotlin-serialization-compiler/test/org/jetbrains/kotlinx/serialization/SerializationPluginDiagnosticTestGenerated.java index 9d4ccb39ac5..525821a41ac 100644 --- a/plugins/kotlin-serialization/kotlin-serialization-compiler/test/org/jetbrains/kotlinx/serialization/SerializationPluginDiagnosticTestGenerated.java +++ b/plugins/kotlin-serialization/kotlin-serialization-compiler/test/org/jetbrains/kotlinx/serialization/SerializationPluginDiagnosticTestGenerated.java @@ -78,6 +78,16 @@ public class SerializationPluginDiagnosticTestGenerated extends AbstractSerializ runTest("plugins/kotlin-serialization/kotlin-serialization-compiler/testData/diagnostics/SerializableIgnored.kt"); } + @TestMetadata("SerializerTypeCompatibleForSpecials.kt") + public void testSerializerTypeCompatibleForSpecials() throws Exception { + runTest("plugins/kotlin-serialization/kotlin-serialization-compiler/testData/diagnostics/SerializerTypeCompatibleForSpecials.kt"); + } + + @TestMetadata("SerializerTypeIncompatible.kt") + public void testSerializerTypeIncompatible() throws Exception { + runTest("plugins/kotlin-serialization/kotlin-serialization-compiler/testData/diagnostics/SerializerTypeIncompatible.kt"); + } + @TestMetadata("Transients.kt") public void testTransients() throws Exception { runTest("plugins/kotlin-serialization/kotlin-serialization-compiler/testData/diagnostics/Transients.kt"); diff --git a/plugins/kotlin-serialization/kotlin-serialization-compiler/testData/diagnostics/SerializableEnums.kt b/plugins/kotlin-serialization/kotlin-serialization-compiler/testData/diagnostics/SerializableEnums.kt index d5e4fe4158f..c36d606989e 100644 --- a/plugins/kotlin-serialization/kotlin-serialization-compiler/testData/diagnostics/SerializableEnums.kt +++ b/plugins/kotlin-serialization/kotlin-serialization-compiler/testData/diagnostics/SerializableEnums.kt @@ -18,4 +18,7 @@ object EnumSerializer: KSerializer { override val descriptor = TODO() override fun serialize(encoder: Encoder, value: ExplicitlyMarkedEnumCustom) = TODO() override fun deserialize(decoder: Decoder): ExplicitlyMarkedEnumCustom = TODO() -} \ No newline at end of file +} + +@Serializable +data class EnumUsage(val s: SimpleEnum, val m: MarkedNameEnum, val e: ExplicitlyMarkedEnum) \ No newline at end of file diff --git a/plugins/kotlin-serialization/kotlin-serialization-compiler/testData/diagnostics/SerializableEnums.txt b/plugins/kotlin-serialization/kotlin-serialization-compiler/testData/diagnostics/SerializableEnums.txt index abbe0f712ec..80246518bdc 100644 --- a/plugins/kotlin-serialization/kotlin-serialization-compiler/testData/diagnostics/SerializableEnums.txt +++ b/plugins/kotlin-serialization/kotlin-serialization-compiler/testData/diagnostics/SerializableEnums.txt @@ -11,6 +11,42 @@ public object EnumSerializer : kotlinx.serialization.KSerializer { + private constructor `$serializer`() + public open override /*1*/ /*synthesized*/ val descriptor: kotlinx.serialization.descriptors.SerialDescriptor + public open override /*1*/ /*synthesized*/ fun childSerializers(): kotlin.Array> + public open override /*1*/ /*synthesized*/ fun deserialize(/*0*/ decoder: kotlinx.serialization.encoding.Decoder): EnumUsage + public open override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean + public open override /*1*/ /*fake_override*/ fun hashCode(): kotlin.Int + @kotlin.Deprecated(level = DeprecationLevel.ERROR, message = "Patch function is deprecated for removal since this functionality is no longer supported by serializer.Some formats may provide implementation-specific patching in their Decoders.") public open override /*1*/ /*fake_override*/ fun patch(/*0*/ decoder: kotlinx.serialization.encoding.Decoder, /*1*/ old: EnumUsage): EnumUsage + public open override /*1*/ /*synthesized*/ fun serialize(/*0*/ encoder: kotlinx.serialization.encoding.Encoder, /*1*/ value: EnumUsage): kotlin.Unit + public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String + public open override /*1*/ /*fake_override*/ fun typeParametersSerializers(): kotlin.Array> + } + + public companion object Companion { + private constructor Companion() + public open override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean + public open override /*1*/ /*fake_override*/ fun hashCode(): kotlin.Int + public final /*synthesized*/ fun serializer(): kotlinx.serialization.KSerializer + public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String + } +} + @kotlinx.serialization.Serializable public final enum class ExplicitlyMarkedEnum : kotlin.Enum { @kotlinx.serialization.SerialName(value = "a") enum entry A diff --git a/plugins/kotlin-serialization/kotlin-serialization-compiler/testData/diagnostics/SerializerTypeCompatibleForSpecials.kt b/plugins/kotlin-serialization/kotlin-serialization-compiler/testData/diagnostics/SerializerTypeCompatibleForSpecials.kt new file mode 100644 index 00000000000..ae6e6425f7d --- /dev/null +++ b/plugins/kotlin-serialization/kotlin-serialization-compiler/testData/diagnostics/SerializerTypeCompatibleForSpecials.kt @@ -0,0 +1,18 @@ +// WITH_RUNTIME +// SKIP_TXT + +import kotlinx.serialization.* + +class Bar +@Serializer(forClass = Bar::class) +object BarSerializer: KSerializer + +class Baz +@Serializer(forClass = Baz::class) +object BazSerializer: KSerializer + +@Serializable +class Foo1(@Polymorphic val i: Baz) + +@Serializable +class Foo2(val li: MutableList<@Serializable(with = BazSerializer::class) Baz>) \ No newline at end of file diff --git a/plugins/kotlin-serialization/kotlin-serialization-compiler/testData/diagnostics/SerializerTypeIncompatible.kt b/plugins/kotlin-serialization/kotlin-serialization-compiler/testData/diagnostics/SerializerTypeIncompatible.kt new file mode 100644 index 00000000000..5767ef717ab --- /dev/null +++ b/plugins/kotlin-serialization/kotlin-serialization-compiler/testData/diagnostics/SerializerTypeIncompatible.kt @@ -0,0 +1,35 @@ +// WITH_RUNTIME +// SKIP_TXT + +import kotlinx.serialization.* + +class Bar +@Serializer(forClass = Bar::class) +object BarSerializer: KSerializer + +class Baz +@Serializer(forClass = Baz::class) +object BazSerializer: KSerializer +@Serializer(forClass = Baz::class) +object NullableBazSerializer: KSerializer + +@Serializable +class Foo(@Serializable(with = BazSerializer::class) val i: Bar) + +@Serializable +class Foo2(val li: List<@Serializable(with = BazSerializer::class) Bar>) + +@Serializable +class Foo3(@Serializable(with = BazSerializer::class) val i: Baz) + +@Serializable +class Foo4(val li: List<@Serializable(with = BazSerializer::class) Baz>) + +@Serializable +class Foo5(@Serializable(with = BazSerializer::class) val i: Bar?) + +@Serializable +class Foo6(@Serializable(with = NullableBazSerializer::class) val i: Bar) + +@Serializable +class Foo7(@Serializable(with = NullableBazSerializer::class) val i: Bar?) \ No newline at end of file