Skip to content

Commit fe5dbf7

Browse files
committed
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 Kotlin/kotlinx.serialization#830
1 parent bc432ec commit fe5dbf7

File tree

8 files changed

+140
-4
lines changed

8 files changed

+140
-4
lines changed

plugins/kotlin-serialization/kotlin-serialization-compiler/src/org/jetbrains/kotlinx/serialization/compiler/diagnostic/SerializationErrors.java

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ public interface SerializationErrors {
2525
DiagnosticFactory1<KtAnnotationEntry, String> DUPLICATE_SERIAL_NAME = DiagnosticFactory1.create(ERROR);
2626
DiagnosticFactory1<PsiElement, KotlinType> SERIALIZER_NOT_FOUND = DiagnosticFactory1.create(ERROR);
2727
DiagnosticFactory2<PsiElement, KotlinType, KotlinType> SERIALIZER_NULLABILITY_INCOMPATIBLE = DiagnosticFactory2.create(ERROR);
28+
DiagnosticFactory3<PsiElement, KotlinType, KotlinType, KotlinType> SERIALIZER_TYPE_INCOMPATIBLE = DiagnosticFactory3.create(WARNING);
2829
DiagnosticFactory0<PsiElement> TRANSIENT_MISSING_INITIALIZER = DiagnosticFactory0.create(ERROR);
2930

3031
DiagnosticFactory0<PsiElement> TRANSIENT_IS_REDUNDANT = DiagnosticFactory0.create(WARNING);

plugins/kotlin-serialization/kotlin-serialization-compiler/src/org/jetbrains/kotlinx/serialization/compiler/diagnostic/SerializationPluginDeclarationChecker.kt

Lines changed: 27 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import com.intellij.psi.PsiElement
99
import org.jetbrains.kotlin.builtins.KotlinBuiltIns
1010
import org.jetbrains.kotlin.config.KotlinCompilerVersion
1111
import org.jetbrains.kotlin.descriptors.*
12+
import org.jetbrains.kotlin.descriptors.annotations.Annotated
1213
import org.jetbrains.kotlin.diagnostics.DiagnosticFactory0
1314
import org.jetbrains.kotlin.js.resolve.diagnostics.findPsi
1415
import org.jetbrains.kotlin.psi.*
@@ -218,6 +219,7 @@ open class SerializationPluginDeclarationChecker : DeclarationChecker {
218219
val ktType = (propertyPsi as? KtCallableDeclaration)?.typeReference
219220
if (serializer != null) {
220221
val element = ktType?.typeElement
222+
checkCustomSerializerMatch(it.module, it.type, it.descriptor, element, trace, propertyPsi)
221223
checkSerializerNullability(it.type, serializer.defaultType, element, trace, propertyPsi)
222224
generatorContextForAnalysis.checkTypeArguments(it.module, it.type, element, trace, propertyPsi)
223225
} else {
@@ -260,13 +262,36 @@ open class SerializationPluginDeclarationChecker : DeclarationChecker {
260262
}
261263
val serializer = findTypeSerializerOrContextUnchecked(module, type)
262264
if (serializer != null) {
265+
checkCustomSerializerMatch(module, type, type, element, trace, fallbackElement)
263266
checkSerializerNullability(type, serializer.defaultType, element, trace, fallbackElement)
264267
checkTypeArguments(module, type, element, trace, fallbackElement)
265268
} else {
266269
trace.report(SerializationErrors.SERIALIZER_NOT_FOUND.on(element ?: fallbackElement, type))
267270
}
268271
}
269272

273+
private fun checkCustomSerializerMatch(
274+
module: ModuleDescriptor,
275+
classType: KotlinType,
276+
descriptor: Annotated,
277+
element: KtElement?,
278+
trace: BindingTrace,
279+
fallbackElement: PsiElement
280+
) {
281+
val serializerType = descriptor.annotations.serializableWith(module) ?: return
282+
val serializerForType = serializerType.supertypes().find { isKSerializer(it) }?.arguments?.first()?.type ?: return
283+
// Compare constructors because we do not care about generic arguments and nullability
284+
if (classType.constructor != serializerForType.constructor)
285+
trace.report(
286+
SerializationErrors.SERIALIZER_TYPE_INCOMPATIBLE.on(
287+
element ?: fallbackElement,
288+
classType,
289+
serializerType,
290+
serializerForType
291+
)
292+
)
293+
}
294+
270295
private fun checkSerializerNullability(
271296
classType: KotlinType,
272297
serializerType: KotlinType,
@@ -277,7 +302,8 @@ open class SerializationPluginDeclarationChecker : DeclarationChecker {
277302
// @Serializable annotation has proper signature so this error would be caught in type checker
278303
val castedToKSerial = serializerType.supertypes().find { isKSerializer(it) } ?: return
279304

280-
if (!classType.isMarkedNullable && castedToKSerial.arguments.first().type.isMarkedNullable)
305+
val serializerForType = castedToKSerial.arguments.first().type
306+
if (!classType.isMarkedNullable && serializerForType.isMarkedNullable)
281307
trace.report(
282308
SerializationErrors.SERIALIZER_NULLABILITY_INCOMPATIBLE.on(element ?: fallbackElement, serializerType, classType),
283309
)

plugins/kotlin-serialization/kotlin-serialization-compiler/src/org/jetbrains/kotlinx/serialization/compiler/diagnostic/SerializationPluginErrorsRendering.kt

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -54,8 +54,15 @@ object SerializationPluginErrorsRendering : DefaultErrorMessages.Extension {
5454
MAP.put(
5555
SerializationErrors.SERIALIZER_NULLABILITY_INCOMPATIBLE,
5656
"Type ''{1}'' is non-nullable and therefore can not be serialized with serializer for nullable type ''{0}''",
57-
Renderers.RENDER_TYPE_WITH_ANNOTATIONS,
58-
Renderers.RENDER_TYPE_WITH_ANNOTATIONS
57+
Renderers.RENDER_TYPE,
58+
Renderers.RENDER_TYPE
59+
)
60+
MAP.put(
61+
SerializationErrors.SERIALIZER_TYPE_INCOMPATIBLE,
62+
"Class ''{1}'', which is serializer for type ''{2}'', is applied here to type ''{0}''. This may lead to errors or incorrect behavior.",
63+
Renderers.RENDER_TYPE,
64+
Renderers.RENDER_TYPE,
65+
Renderers.RENDER_TYPE
5966
)
6067
MAP.put(
6168
SerializationErrors.TRANSIENT_MISSING_INITIALIZER,

plugins/kotlin-serialization/kotlin-serialization-compiler/test/org/jetbrains/kotlinx/serialization/SerializationPluginDiagnosticTestGenerated.java

Lines changed: 10 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

plugins/kotlin-serialization/kotlin-serialization-compiler/testData/diagnostics/SerializableEnums.kt

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,4 +18,7 @@ object EnumSerializer: KSerializer<ExplicitlyMarkedEnumCustom> {
1818
override val descriptor = TODO()
1919
override fun serialize(encoder: Encoder, value: ExplicitlyMarkedEnumCustom) = TODO()
2020
override fun deserialize(decoder: Decoder): ExplicitlyMarkedEnumCustom = TODO()
21-
}
21+
}
22+
23+
@Serializable
24+
data class EnumUsage(val s: SimpleEnum, val m: MarkedNameEnum, val e: ExplicitlyMarkedEnum)

plugins/kotlin-serialization/kotlin-serialization-compiler/testData/diagnostics/SerializableEnums.txt

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,42 @@ public object EnumSerializer : kotlinx.serialization.KSerializer<ExplicitlyMarke
1111
public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String
1212
}
1313

14+
@kotlinx.serialization.Serializable public final data class EnumUsage {
15+
public constructor EnumUsage(/*0*/ s: SimpleEnum, /*1*/ m: MarkedNameEnum, /*2*/ e: ExplicitlyMarkedEnum)
16+
@kotlin.Deprecated(level = DeprecationLevel.HIDDEN, message = "This synthesized declaration should not be used directly", replaceWith = kotlin.ReplaceWith(expression = "", imports = {})) public /*synthesized*/ constructor EnumUsage(/*0*/ seen1: kotlin.Int, /*1*/ s: SimpleEnum?, /*2*/ m: MarkedNameEnum?, /*3*/ e: ExplicitlyMarkedEnum?, /*4*/ serializationConstructorMarker: kotlinx.serialization.internal.SerializationConstructorMarker?)
17+
public final val e: ExplicitlyMarkedEnum
18+
public final val m: MarkedNameEnum
19+
public final val s: SimpleEnum
20+
public final operator /*synthesized*/ fun component1(): SimpleEnum
21+
public final operator /*synthesized*/ fun component2(): MarkedNameEnum
22+
public final operator /*synthesized*/ fun component3(): ExplicitlyMarkedEnum
23+
public final /*synthesized*/ fun copy(/*0*/ s: SimpleEnum = ..., /*1*/ m: MarkedNameEnum = ..., /*2*/ e: ExplicitlyMarkedEnum = ...): EnumUsage
24+
public open override /*1*/ /*synthesized*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean
25+
public open override /*1*/ /*synthesized*/ fun hashCode(): kotlin.Int
26+
public open override /*1*/ /*synthesized*/ fun toString(): kotlin.String
27+
28+
@kotlin.Deprecated(level = DeprecationLevel.HIDDEN, message = "This synthesized declaration should not be used directly", replaceWith = kotlin.ReplaceWith(expression = "", imports = {})) public object `$serializer` : kotlinx.serialization.internal.GeneratedSerializer<EnumUsage> {
29+
private constructor `$serializer`()
30+
public open override /*1*/ /*synthesized*/ val descriptor: kotlinx.serialization.descriptors.SerialDescriptor
31+
public open override /*1*/ /*synthesized*/ fun childSerializers(): kotlin.Array<kotlinx.serialization.KSerializer<*>>
32+
public open override /*1*/ /*synthesized*/ fun deserialize(/*0*/ decoder: kotlinx.serialization.encoding.Decoder): EnumUsage
33+
public open override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean
34+
public open override /*1*/ /*fake_override*/ fun hashCode(): kotlin.Int
35+
@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
36+
public open override /*1*/ /*synthesized*/ fun serialize(/*0*/ encoder: kotlinx.serialization.encoding.Encoder, /*1*/ value: EnumUsage): kotlin.Unit
37+
public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String
38+
public open override /*1*/ /*fake_override*/ fun typeParametersSerializers(): kotlin.Array<kotlinx.serialization.KSerializer<*>>
39+
}
40+
41+
public companion object Companion {
42+
private constructor Companion()
43+
public open override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean
44+
public open override /*1*/ /*fake_override*/ fun hashCode(): kotlin.Int
45+
public final /*synthesized*/ fun serializer(): kotlinx.serialization.KSerializer<EnumUsage>
46+
public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String
47+
}
48+
}
49+
1450
@kotlinx.serialization.Serializable public final enum class ExplicitlyMarkedEnum : kotlin.Enum<ExplicitlyMarkedEnum> {
1551
@kotlinx.serialization.SerialName(value = "a") enum entry A
1652

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
// WITH_RUNTIME
2+
// SKIP_TXT
3+
4+
import kotlinx.serialization.*
5+
6+
class Bar
7+
@Serializer(forClass = Bar::class)
8+
object BarSerializer: KSerializer<Bar>
9+
10+
class Baz
11+
@Serializer(forClass = Baz::class)
12+
object BazSerializer: KSerializer<Baz>
13+
14+
@Serializable
15+
class Foo1(@Polymorphic val i: Baz)
16+
17+
@Serializable
18+
class Foo2(val li: MutableList<@Serializable(with = BazSerializer::class) Baz>)
Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
// WITH_RUNTIME
2+
// SKIP_TXT
3+
4+
import kotlinx.serialization.*
5+
6+
class Bar
7+
@Serializer(forClass = Bar::class)
8+
object BarSerializer: KSerializer<Bar>
9+
10+
class Baz
11+
@Serializer(forClass = Baz::class)
12+
object BazSerializer: KSerializer<Baz>
13+
@Serializer(forClass = Baz::class)
14+
object NullableBazSerializer: KSerializer<Baz?>
15+
16+
@Serializable
17+
class Foo(@Serializable(with = BazSerializer::class) val i: <!SERIALIZER_TYPE_INCOMPATIBLE!>Bar<!>)
18+
19+
@Serializable
20+
class Foo2(val li: List<@Serializable(with = BazSerializer::class) <!SERIALIZER_TYPE_INCOMPATIBLE!>Bar<!>>)
21+
22+
@Serializable
23+
class Foo3(@Serializable(with = BazSerializer::class) val i: Baz)
24+
25+
@Serializable
26+
class Foo4(val li: List<@Serializable(with = BazSerializer::class) Baz>)
27+
28+
@Serializable
29+
class Foo5(@Serializable(with = BazSerializer::class) val i: <!SERIALIZER_TYPE_INCOMPATIBLE!>Bar?<!>)
30+
31+
@Serializable
32+
class Foo6(@Serializable(with = NullableBazSerializer::class) val i: <!SERIALIZER_NULLABILITY_INCOMPATIBLE, SERIALIZER_TYPE_INCOMPATIBLE!>Bar<!>)
33+
34+
@Serializable
35+
class Foo7(@Serializable(with = NullableBazSerializer::class) val i: <!SERIALIZER_TYPE_INCOMPATIBLE!>Bar?<!>)

0 commit comments

Comments
 (0)