From 3629a9db3055d74a4bf36b912b999f8c0e3b68ce Mon Sep 17 00:00:00 2001 From: "Sergey.Shanshin" Date: Tue, 4 Jul 2023 18:31:35 +0000 Subject: [PATCH] [KxSerialization] Added inspection on abstract custom serializer Serialization requires an instance of the serializer, which cannot be obtained with the passed interface, abstract or sealed class. Therefore, the specifying of such classes in `Serializable` annotation must be prohibited. Fixes https://github.com/Kotlin/kotlinx.serialization/issues/2173 Relates #KT-58036 Merge-request: KT-MR-10753 Merged-by: Sergey Shanshin --- .../diagnostic/SerializationErrors.java | 1 + .../SerializationPluginDeclarationChecker.kt | 29 +++++++++++++++ .../SerializationPluginErrorsRendering.kt | 6 +++ .../compiler/resolve/KSerializationUtil.kt | 3 ++ .../compiler/fir/SerializationFirUtils.kt | 5 +++ .../fir/checkers/FirSerializationErrors.kt | 1 + .../FirSerializationPluginClassChecker.kt | 23 ++++++++++++ .../KtDefaultErrorMessagesSerialization.kt | 8 +++- .../diagnostics/abstractCustomSerializer.kt | 37 +++++++++++++++++++ ...lizationFirPsiDiagnosticTestGenerated.java | 6 +++ ...lizationPluginDiagnosticTestGenerated.java | 6 +++ 11 files changed, 123 insertions(+), 2 deletions(-) create mode 100644 plugins/kotlinx-serialization/testData/diagnostics/abstractCustomSerializer.kt diff --git a/plugins/kotlinx-serialization/kotlinx-serialization.k1/src/org/jetbrains/kotlinx/serialization/compiler/diagnostic/SerializationErrors.java b/plugins/kotlinx-serialization/kotlinx-serialization.k1/src/org/jetbrains/kotlinx/serialization/compiler/diagnostic/SerializationErrors.java index 3519881a44c..657508fba87 100644 --- a/plugins/kotlinx-serialization/kotlinx-serialization.k1/src/org/jetbrains/kotlinx/serialization/compiler/diagnostic/SerializationErrors.java +++ b/plugins/kotlinx-serialization/kotlinx-serialization.k1/src/org/jetbrains/kotlinx/serialization/compiler/diagnostic/SerializationErrors.java @@ -36,6 +36,7 @@ public interface SerializationErrors { DiagnosticFactory3 DUPLICATE_SERIAL_NAME_ENUM = DiagnosticFactory3.create(ERROR); DiagnosticFactory1 SERIALIZER_NOT_FOUND = DiagnosticFactory1.create(ERROR); DiagnosticFactory2 SERIALIZER_NULLABILITY_INCOMPATIBLE = DiagnosticFactory2.create(ERROR); + DiagnosticFactory2 ABSTRACT_SERIALIZER_TYPE = DiagnosticFactory2.create(ERROR); DiagnosticFactory3 SERIALIZER_TYPE_INCOMPATIBLE = DiagnosticFactory3.create(WARNING); DiagnosticFactory1 LOCAL_SERIALIZER_USAGE = DiagnosticFactory1.create(ERROR); DiagnosticFactory0 TRANSIENT_MISSING_INITIALIZER = DiagnosticFactory0.create(ERROR); diff --git a/plugins/kotlinx-serialization/kotlinx-serialization.k1/src/org/jetbrains/kotlinx/serialization/compiler/diagnostic/SerializationPluginDeclarationChecker.kt b/plugins/kotlinx-serialization/kotlinx-serialization.k1/src/org/jetbrains/kotlinx/serialization/compiler/diagnostic/SerializationPluginDeclarationChecker.kt index 52b231990f6..883bbd816de 100644 --- a/plugins/kotlinx-serialization/kotlinx-serialization.k1/src/org/jetbrains/kotlinx/serialization/compiler/diagnostic/SerializationPluginDeclarationChecker.kt +++ b/plugins/kotlinx-serialization/kotlinx-serialization.k1/src/org/jetbrains/kotlinx/serialization/compiler/diagnostic/SerializationPluginDeclarationChecker.kt @@ -325,6 +325,7 @@ open class SerializationPluginDeclarationChecker : DeclarationChecker { val annotationPsi = descriptor.findSerializableOrMetaAnnotationDeclaration() checkCustomSerializerMatch(descriptor.module, descriptor.defaultType, descriptor, annotationPsi, trace, declaration) checkCustomSerializerIsNotLocal(descriptor.module, descriptor, trace, declaration) + checkCustomSerializerNotAbstract(descriptor.module, descriptor.defaultType, descriptor, annotationPsi, trace, declaration) } private val ClassDescriptor.isAnonymousObjectOrContained: Boolean @@ -440,6 +441,14 @@ open class SerializationPluginDeclarationChecker : DeclarationChecker { if (serializer != null) { val element = ktType?.typeElement checkCustomSerializerMatch(it.module, it.type, it.descriptor, element, trace, propertyPsi) + checkCustomSerializerNotAbstract( + it.module, + it.type, + it.descriptor, + it.descriptor.findSerializableOrMetaAnnotationDeclaration(), + trace, + propertyPsi + ) checkCustomSerializerIsNotLocal(it.module, it.descriptor, trace, propertyPsi) checkSerializerNullability(it.type, serializer.defaultType, element, trace, propertyPsi) generatorContextForAnalysis.checkTypeArguments(it.module, it.type, element, trace, propertyPsi) @@ -519,6 +528,26 @@ open class SerializationPluginDeclarationChecker : DeclarationChecker { } } + private fun checkCustomSerializerNotAbstract( + module: ModuleDescriptor, + classType: KotlinType, + descriptor: Annotated, + element: KtElement?, + trace: BindingTrace, + fallbackElement: PsiElement + ) { + val serializerType = descriptor.annotations.serializableWith(module) ?: return + if (serializerType.toClassDescriptor?.isAbstractOrSealedOrInterface == true) { + trace.report( + SerializationErrors.ABSTRACT_SERIALIZER_TYPE.on( + element ?: fallbackElement, + classType, + serializerType + ) + ) + } + } + private fun checkCustomSerializerMatch( module: ModuleDescriptor, classType: KotlinType, diff --git a/plugins/kotlinx-serialization/kotlinx-serialization.k1/src/org/jetbrains/kotlinx/serialization/compiler/diagnostic/SerializationPluginErrorsRendering.kt b/plugins/kotlinx-serialization/kotlinx-serialization.k1/src/org/jetbrains/kotlinx/serialization/compiler/diagnostic/SerializationPluginErrorsRendering.kt index 620976d5de5..f0fca2453ec 100644 --- a/plugins/kotlinx-serialization/kotlinx-serialization.k1/src/org/jetbrains/kotlinx/serialization/compiler/diagnostic/SerializationPluginErrorsRendering.kt +++ b/plugins/kotlinx-serialization/kotlinx-serialization.k1/src/org/jetbrains/kotlinx/serialization/compiler/diagnostic/SerializationPluginErrorsRendering.kt @@ -107,6 +107,12 @@ object SerializationPluginErrorsRendering : DefaultErrorMessages.Extension { Renderers.RENDER_TYPE, Renderers.RENDER_TYPE ) + MAP.put( + SerializationErrors.ABSTRACT_SERIALIZER_TYPE, + "Custom serializer ''{1}'' on serializable type ''{0}'' can not be instantiated. It is not allowed to specify the interface, abstract or sealed class as a custom serializer.", + Renderers.RENDER_TYPE, + Renderers.RENDER_TYPE + ) MAP.put( SerializationErrors.LOCAL_SERIALIZER_USAGE, "Class ''{0}'' can't be used as a serializer since it is local", diff --git a/plugins/kotlinx-serialization/kotlinx-serialization.k1/src/org/jetbrains/kotlinx/serialization/compiler/resolve/KSerializationUtil.kt b/plugins/kotlinx-serialization/kotlinx-serialization.k1/src/org/jetbrains/kotlinx/serialization/compiler/resolve/KSerializationUtil.kt index b3f9c4c4fc7..d4116221acc 100644 --- a/plugins/kotlinx-serialization/kotlinx-serialization.k1/src/org/jetbrains/kotlinx/serialization/compiler/resolve/KSerializationUtil.kt +++ b/plugins/kotlinx-serialization/kotlinx-serialization.k1/src/org/jetbrains/kotlinx/serialization/compiler/resolve/KSerializationUtil.kt @@ -124,6 +124,9 @@ val ClassDescriptor.isInternallySerializableObject: Boolean val ClassDescriptor.isSealedSerializableInterface: Boolean get() = kind == ClassKind.INTERFACE && modality == Modality.SEALED && hasSerializableOrMetaAnnotation +val ClassDescriptor.isAbstractOrSealedOrInterface: Boolean + get() = kind == ClassKind.INTERFACE || modality == Modality.SEALED || modality == Modality.ABSTRACT + val ClassDescriptor.isInternalSerializable: Boolean //todo normal checking get() { if (kind != ClassKind.CLASS) return false diff --git a/plugins/kotlinx-serialization/kotlinx-serialization.k2/src/org/jetbrains/kotlinx/serialization/compiler/fir/SerializationFirUtils.kt b/plugins/kotlinx-serialization/kotlinx-serialization.k2/src/org/jetbrains/kotlinx/serialization/compiler/fir/SerializationFirUtils.kt index 21d5635ddab..031c20da406 100644 --- a/plugins/kotlinx-serialization/kotlinx-serialization.k2/src/org/jetbrains/kotlinx/serialization/compiler/fir/SerializationFirUtils.kt +++ b/plugins/kotlinx-serialization/kotlinx-serialization.k2/src/org/jetbrains/kotlinx/serialization/compiler/fir/SerializationFirUtils.kt @@ -222,6 +222,11 @@ context(FirSession) val ConeKotlinType.isGeneratedSerializableObject: Boolean get() = toRegularClassSymbol(this@FirSession)?.let { it.classKind.isObject && it.hasSerializableOrMetaAnnotationWithoutArgs } ?: false +context(FirSession) +val ConeKotlinType.isAbstractOrSealedOrInterface: Boolean + get() = toRegularClassSymbol(this@FirSession)?.let { it.classKind.isInterface || it.rawStatus.modality == Modality.ABSTRACT || it.rawStatus.modality == Modality.SEALED } + ?: false + context(FirExtension) fun FirAnnotationContainer.excludeFromJsExport() { diff --git a/plugins/kotlinx-serialization/kotlinx-serialization.k2/src/org/jetbrains/kotlinx/serialization/compiler/fir/checkers/FirSerializationErrors.kt b/plugins/kotlinx-serialization/kotlinx-serialization.k2/src/org/jetbrains/kotlinx/serialization/compiler/fir/checkers/FirSerializationErrors.kt index 4ed8de7feb0..76f02762508 100644 --- a/plugins/kotlinx-serialization/kotlinx-serialization.k2/src/org/jetbrains/kotlinx/serialization/compiler/fir/checkers/FirSerializationErrors.kt +++ b/plugins/kotlinx-serialization/kotlinx-serialization.k2/src/org/jetbrains/kotlinx/serialization/compiler/fir/checkers/FirSerializationErrors.kt @@ -34,6 +34,7 @@ object FirSerializationErrors { val SERIALIZER_NOT_FOUND by error1() val SERIALIZER_NULLABILITY_INCOMPATIBLE by error2() val SERIALIZER_TYPE_INCOMPATIBLE by warning3() + val ABSTRACT_SERIALIZER_TYPE by error2() val LOCAL_SERIALIZER_USAGE by error1() val GENERIC_ARRAY_ELEMENT_NOT_SUPPORTED by error0() val TRANSIENT_MISSING_INITIALIZER by error0() diff --git a/plugins/kotlinx-serialization/kotlinx-serialization.k2/src/org/jetbrains/kotlinx/serialization/compiler/fir/checkers/FirSerializationPluginClassChecker.kt b/plugins/kotlinx-serialization/kotlinx-serialization.k2/src/org/jetbrains/kotlinx/serialization/compiler/fir/checkers/FirSerializationPluginClassChecker.kt index 60623a13d23..6065b42b37e 100644 --- a/plugins/kotlinx-serialization/kotlinx-serialization.k2/src/org/jetbrains/kotlinx/serialization/compiler/fir/checkers/FirSerializationPluginClassChecker.kt +++ b/plugins/kotlinx-serialization/kotlinx-serialization.k2/src/org/jetbrains/kotlinx/serialization/compiler/fir/checkers/FirSerializationPluginClassChecker.kt @@ -363,6 +363,7 @@ object FirSerializationPluginClassChecker : FirClassChecker() { val serializerType = classSymbol.getSerializableWith(session)?.fullyExpandedType(session) ?: return checkCustomSerializerMatch(classSymbol, source = null, classSymbol.defaultType(), serializerType, reporter) checkCustomSerializerIsNotLocal(source = null, classSymbol, serializerType, reporter) + checkCustomSerializerNotAbstract(classSymbol, source = null, serializerType, reporter) } context(CheckerContext) @@ -471,6 +472,12 @@ object FirSerializationPluginClassChecker : FirClassChecker() { customSerializerType, reporter ) + checkCustomSerializerNotAbstract( + classSymbol, + source = propertySymbol.serializableAnnotation(needArguments = false, session)?.source, + customSerializerType, + reporter + ) checkCustomSerializerIsNotLocal( source = propertySymbol.serializableAnnotation(needArguments = false, session)?.source, classSymbol, @@ -566,6 +573,22 @@ object FirSerializationPluginClassChecker : FirClassChecker() { ) } } + context(CheckerContext) + private fun checkCustomSerializerNotAbstract( + containingClassSymbol: FirClassSymbol<*>, + source: KtSourceElement?, + serializerType: ConeKotlinType, + reporter: DiagnosticReporter + ) { + if (with(session) { serializerType.isAbstractOrSealedOrInterface }) { + reporter.reportOn( + source ?: containingClassSymbol.serializableOrMetaAnnotationSource, + FirSerializationErrors.ABSTRACT_SERIALIZER_TYPE, + containingClassSymbol.defaultType(), + serializerType + ) + } + } context(CheckerContext) private fun checkCustomSerializerIsNotLocal( diff --git a/plugins/kotlinx-serialization/kotlinx-serialization.k2/src/org/jetbrains/kotlinx/serialization/compiler/fir/checkers/KtDefaultErrorMessagesSerialization.kt b/plugins/kotlinx-serialization/kotlinx-serialization.k2/src/org/jetbrains/kotlinx/serialization/compiler/fir/checkers/KtDefaultErrorMessagesSerialization.kt index f12a517ea2e..d9fe2af4aac 100644 --- a/plugins/kotlinx-serialization/kotlinx-serialization.k2/src/org/jetbrains/kotlinx/serialization/compiler/fir/checkers/KtDefaultErrorMessagesSerialization.kt +++ b/plugins/kotlinx-serialization/kotlinx-serialization.k2/src/org/jetbrains/kotlinx/serialization/compiler/fir/checkers/KtDefaultErrorMessagesSerialization.kt @@ -5,9 +5,7 @@ package org.jetbrains.kotlinx.serialization.compiler.fir.checkers -import org.jetbrains.kotlin.diagnostics.KtDiagnostic import org.jetbrains.kotlin.diagnostics.KtDiagnosticFactoryToRendererMap -import org.jetbrains.kotlin.diagnostics.KtDiagnosticRenderer import org.jetbrains.kotlin.diagnostics.rendering.BaseDiagnosticRendererFactory import org.jetbrains.kotlin.diagnostics.rendering.CommonRenderers import org.jetbrains.kotlin.fir.analysis.diagnostics.FirDiagnosticRenderers @@ -106,6 +104,12 @@ object KtDefaultErrorMessagesSerialization : BaseDiagnosticRendererFactory() { FirDiagnosticRenderers.RENDER_TYPE, FirDiagnosticRenderers.RENDER_TYPE ) + put( + FirSerializationErrors.ABSTRACT_SERIALIZER_TYPE, + "Custom serializer ''{1}'' on serializable type ''{0}'' can not be instantiated. It is not allowed to specify the interface, abstract or sealed class as a custom serializer.", + FirDiagnosticRenderers.RENDER_TYPE, + FirDiagnosticRenderers.RENDER_TYPE + ) put( FirSerializationErrors.LOCAL_SERIALIZER_USAGE, "Class ''{0}'' can't be used as a serializer since it is local", diff --git a/plugins/kotlinx-serialization/testData/diagnostics/abstractCustomSerializer.kt b/plugins/kotlinx-serialization/testData/diagnostics/abstractCustomSerializer.kt new file mode 100644 index 00000000000..b3b6333bc1e --- /dev/null +++ b/plugins/kotlinx-serialization/testData/diagnostics/abstractCustomSerializer.kt @@ -0,0 +1,37 @@ +// FIR_DISABLE_LAZY_RESOLVE_CHECKS +// FIR_IDENTICAL +// SKIP_TXT +// WITH_STDLIB + +// FILE: test.kt +import kotlinx.serialization.* + + +interface InterfaceSerializer: KSerializer + +@Serializable(InterfaceSerializer::class) +class WithInterfaceSerializer(val i: Int) + + +abstract class AbstractSerializer: KSerializer + +@Serializable(AbstractSerializer::class) +class WithAbstract(val i: Int) + + +sealed class SealedSerializer: KSerializer + +@Serializable(SealedSerializer::class) +class WithSealed(val i: Int) + +@Serializable +class Holder ( + @Serializable(InterfaceSerializer::class) +val withInterface: WithInterfaceSerializer, + +@Serializable(AbstractSerializer::class) +val withAbstract: WithAbstract, + +@Serializable(SealedSerializer::class) +val withSealed: WithSealed +) diff --git a/plugins/kotlinx-serialization/tests-gen/org/jetbrains/kotlinx/serialization/runners/SerializationFirPsiDiagnosticTestGenerated.java b/plugins/kotlinx-serialization/tests-gen/org/jetbrains/kotlinx/serialization/runners/SerializationFirPsiDiagnosticTestGenerated.java index d95ddc5b6c2..fa3c76f39ce 100644 --- a/plugins/kotlinx-serialization/tests-gen/org/jetbrains/kotlinx/serialization/runners/SerializationFirPsiDiagnosticTestGenerated.java +++ b/plugins/kotlinx-serialization/tests-gen/org/jetbrains/kotlinx/serialization/runners/SerializationFirPsiDiagnosticTestGenerated.java @@ -21,6 +21,12 @@ public class SerializationFirPsiDiagnosticTestGenerated extends AbstractSerializ @TestMetadata("plugins/kotlinx-serialization/testData/diagnostics") @TestDataPath("$PROJECT_ROOT") public class Diagnostics { + @Test + @TestMetadata("abstractCustomSerializer.kt") + public void testAbstractCustomSerializer() throws Exception { + runTest("plugins/kotlinx-serialization/testData/diagnostics/abstractCustomSerializer.kt"); + } + @Test public void testAllFilesPresentInDiagnostics() throws Exception { KtTestUtil.assertAllTestsPresentByMetadataWithExcluded(this.getClass(), new File("plugins/kotlinx-serialization/testData/diagnostics"), Pattern.compile("^(.+)\\.kt$"), Pattern.compile("^(.+)\\.fir\\.kts?$"), true); diff --git a/plugins/kotlinx-serialization/tests-gen/org/jetbrains/kotlinx/serialization/runners/SerializationPluginDiagnosticTestGenerated.java b/plugins/kotlinx-serialization/tests-gen/org/jetbrains/kotlinx/serialization/runners/SerializationPluginDiagnosticTestGenerated.java index bebbc52d8e9..b6f2fb9ccd4 100644 --- a/plugins/kotlinx-serialization/tests-gen/org/jetbrains/kotlinx/serialization/runners/SerializationPluginDiagnosticTestGenerated.java +++ b/plugins/kotlinx-serialization/tests-gen/org/jetbrains/kotlinx/serialization/runners/SerializationPluginDiagnosticTestGenerated.java @@ -19,6 +19,12 @@ import java.util.regex.Pattern; @TestMetadata("plugins/kotlinx-serialization/testData/diagnostics") @TestDataPath("$PROJECT_ROOT") public class SerializationPluginDiagnosticTestGenerated extends AbstractSerializationPluginDiagnosticTest { + @Test + @TestMetadata("abstractCustomSerializer.kt") + public void testAbstractCustomSerializer() throws Exception { + runTest("plugins/kotlinx-serialization/testData/diagnostics/abstractCustomSerializer.kt"); + } + @Test public void testAllFilesPresentInDiagnostics() throws Exception { KtTestUtil.assertAllTestsPresentByMetadataWithExcluded(this.getClass(), new File("plugins/kotlinx-serialization/testData/diagnostics"), Pattern.compile("^(.+)\\.kt$"), Pattern.compile("^(.+)\\.fir\\.kts?$"), true);