Use type parameters' source information for kotlinx.serialization diagnostics

To be more consistent with K1, certain diagnostics should be reported
not on the whole properties' types, but on type arguments inside them.

Note that there is still a difference with K2 because K2 reports on a type argument
including its annotations, while K1 used KtTypeReference.typeElement.

IMO, K2 conveys the same or better meaning here, so I am willing to leave this
difference instead of providing PositioningStrategy.

#KT-53861 Fixed
This commit is contained in:
Leonid Startsev
2023-11-22 18:03:24 +01:00
committed by Space Team
parent a22a254c1e
commit f7d87f6d70
6 changed files with 61 additions and 38 deletions
@@ -11,10 +11,9 @@ import org.jetbrains.kotlin.descriptors.ClassKind
import org.jetbrains.kotlin.diagnostics.DiagnosticReporter
import org.jetbrains.kotlin.diagnostics.SourceElementPositioningStrategies
import org.jetbrains.kotlin.diagnostics.reportOn
import org.jetbrains.kotlin.fir.analysis.checkers.*
import org.jetbrains.kotlin.fir.analysis.checkers.context.CheckerContext
import org.jetbrains.kotlin.fir.analysis.checkers.declaration.FirClassChecker
import org.jetbrains.kotlin.fir.analysis.checkers.isSingleFieldValueClass
import org.jetbrains.kotlin.fir.analysis.checkers.toRegularClassSymbol
import org.jetbrains.kotlin.fir.declarations.*
import org.jetbrains.kotlin.fir.declarations.annotationPlatformSupport
import org.jetbrains.kotlin.fir.declarations.utils.*
@@ -489,7 +488,7 @@ object FirSerializationPluginClassChecker : FirClassChecker() {
)
checkSerializerNullability(propertyType, customSerializerType, source, reporter)
} else {
checkType(propertyType, source, reporter)
checkType(typeRef, source, reporter)
checkGenericArrayType(propertyType, source, reporter)
}
}
@@ -506,13 +505,15 @@ object FirSerializationPluginClassChecker : FirClassChecker() {
}
context(CheckerContext)
private fun checkTypeArguments(type: ConeKotlinType, source: KtSourceElement?, reporter: DiagnosticReporter) {
for (typeArgument in type.typeArguments) {
checkType(
typeArgument.type?.fullyExpandedType(session) ?: continue,
source,
reporter
)
private fun checkTypeArguments(
typeRef: FirTypeRef,
fallbackSource: KtSourceElement?,
reporter: DiagnosticReporter
) {
val argsRefs = extractArgumentsTypeRefAndSource(typeRef) ?: return
for (typeArgument in argsRefs) {
val argTypeRef = typeArgument.typeRef ?: continue
checkType(argTypeRef, typeArgument.source ?: fallbackSource, reporter)
}
}
@@ -526,11 +527,12 @@ object FirSerializationPluginClassChecker : FirClassChecker() {
get() = isSingleFieldValueClass(session) && !isPrimitiveOrNullablePrimitive
context(CheckerContext)
private fun checkType(type: ConeKotlinType, source: KtSourceElement?, reporter: DiagnosticReporter) {
private fun checkType(typeRef: FirTypeRef, typeSource: KtSourceElement?, reporter: DiagnosticReporter) {
val type = typeRef.coneType.fullyExpandedType(session)
if (type.lowerBoundIfFlexible().isTypeParameter) return // type parameters always have serializer stored in class' field
if (type.isUnsupportedInlineType && !canSupportInlineClasses()) {
reporter.reportOn(
source,
typeRef.source ?: typeSource,
FirSerializationErrors.INLINE_CLASSES_NOT_SUPPORTED,
RuntimeVersions.MINIMAL_VERSION_FOR_INLINE_CLASSES.toString(),
session.versionReader.runtimeVersions?.implementationVersion.toString()
@@ -541,15 +543,15 @@ object FirSerializationPluginClassChecker : FirClassChecker() {
if (serializer != null) {
val classSymbol = type.toRegularClassSymbol(session) ?: return
type.serializableWith?.fullyExpandedType(session)?.let { serializerType ->
checkCustomSerializerMatch(classSymbol, source, type, serializerType, reporter)
checkCustomSerializerIsNotLocal(source, classSymbol, serializerType, reporter)
checkSerializerNullability(type, serializerType, source, reporter)
checkCustomSerializerMatch(classSymbol, typeSource, type, serializerType, reporter)
checkCustomSerializerIsNotLocal(typeSource, classSymbol, serializerType, reporter)
checkSerializerNullability(type, serializerType, typeSource, reporter)
}
checkTypeArguments(type, source, reporter)
checkTypeArguments(typeRef, typeSource, reporter)
} else {
if (type.toRegularClassSymbol(session)?.isEnumClass != true) {
// enums are always serializable
reporter.reportOn(source, FirSerializationErrors.SERIALIZER_NOT_FOUND, type)
reporter.reportOn(typeSource, FirSerializationErrors.SERIALIZER_NOT_FOUND, type)
}
}
}
@@ -566,7 +568,6 @@ object FirSerializationPluginClassChecker : FirClassChecker() {
val declarationTypeClassId = declarationType.classId
if (declarationTypeClassId == null || declarationTypeClassId != serializerForType.classId) {
// TODO: report on specific type argument (KT-53861)
reporter.reportOn(
source ?: containingClassSymbol.serializableOrMetaAnnotationSource,
FirSerializationErrors.SERIALIZER_TYPE_INCOMPATIBLE,
@@ -576,6 +577,7 @@ object FirSerializationPluginClassChecker : FirClassChecker() {
)
}
}
context(CheckerContext)
private fun checkCustomSerializerNotAbstract(
containingClassSymbol: FirClassSymbol<*>,
@@ -1,18 +0,0 @@
// FIR_DISABLE_LAZY_RESOLVE_CHECKS
// WITH_STDLIB
// FIR_DIFFERENCE: KT-53861
// FILE: test.kt
import kotlinx.serialization.*
class NonSerializable
@Serializable
class Basic(val foo: <!SERIALIZER_NOT_FOUND("NonSerializable")!>NonSerializable<!>)
@Serializable
class Inside(val foo: <!SERIALIZER_NOT_FOUND("NonSerializable")!>List<NonSerializable><!>)
@Serializable
class WithImplicitType {
<!SERIALIZER_NOT_FOUND("NonSerializable")!>val foo = NonSerializable()<!>
}
@@ -1,6 +1,6 @@
// FIR_DISABLE_LAZY_RESOLVE_CHECKS
// WITH_STDLIB
// FIR_DIFFERENCE: KT-53861
// FIR_IDENTICAL
// FILE: test.kt
import kotlinx.serialization.*
@@ -12,6 +12,9 @@ class Basic(val foo: <!SERIALIZER_NOT_FOUND("NonSerializable")!>NonSerializable<
@Serializable
class Inside(val foo: List<<!SERIALIZER_NOT_FOUND("NonSerializable")!>NonSerializable<!>>)
@Serializable
class Inside2(val foo: List<List<<!SERIALIZER_NOT_FOUND("NonSerializable")!>NonSerializable<!>>>)
@Serializable
class WithImplicitType {
<!SERIALIZER_NOT_FOUND("NonSerializable")!>val foo = NonSerializable()<!>
@@ -60,6 +60,36 @@ package
}
}
@kotlinx.serialization.Serializable public final class Inside2 {
@kotlin.Deprecated(level = DeprecationLevel.HIDDEN, message = "This synthesized declaration should not be used directly", replaceWith = kotlin.ReplaceWith(expression = "", imports = {})) internal /*synthesized*/ constructor Inside2(/*0*/ seen1: kotlin.Int, /*1*/ foo: kotlin.collections.List<kotlin.collections.List<NonSerializable>>?, /*2*/ serializationConstructorMarker: kotlinx.serialization.internal.SerializationConstructorMarker?)
public constructor Inside2(/*0*/ foo: kotlin.collections.List<kotlin.collections.List<NonSerializable>>)
public final val foo: kotlin.collections.List<kotlin.collections.List<NonSerializable>>
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 open override /*1*/ /*fake_override*/ fun toString(): kotlin.String
@kotlin.jvm.JvmStatic internal final /*synthesized*/ fun `write$Self`(/*0*/ self: Inside2, /*1*/ output: kotlinx.serialization.encoding.CompositeEncoder, /*2*/ serialDesc: kotlinx.serialization.descriptors.SerialDescriptor): kotlin.Unit
@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<Inside2> {
private constructor `$serializer`()
public open override /*1*/ /*synthesized*/ val descriptor: kotlinx.serialization.descriptors.SerialDescriptor
public open override /*1*/ /*synthesized*/ fun childSerializers(): kotlin.Array<kotlinx.serialization.KSerializer<*>>
public open override /*1*/ /*synthesized*/ fun deserialize(/*0*/ decoder: kotlinx.serialization.encoding.Decoder): Inside2
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 open override /*1*/ /*synthesized*/ fun serialize(/*0*/ encoder: kotlinx.serialization.encoding.Encoder, /*1*/ value: Inside2): kotlin.Unit
public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String
public open override /*1*/ /*fake_override*/ fun typeParametersSerializers(): kotlin.Array<kotlinx.serialization.KSerializer<*>>
}
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<Inside2>
public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String
}
}
public final class NonSerializable {
public constructor NonSerializable()
public open override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean
@@ -21,7 +21,10 @@ class Biz(val i: Int)
class Foo(@Serializable(with = BazSerializer::class) val i: <!SERIALIZER_TYPE_INCOMPATIBLE!>Bar<!>)
@Serializable
class Foo2(val li: <!SERIALIZER_TYPE_INCOMPATIBLE!>List<@Serializable(with = BazSerializer::class) Bar><!>)
class Foo2(val li: List<<!SERIALIZER_TYPE_INCOMPATIBLE!>@Serializable(with = BazSerializer::class) Bar<!>>)
@Serializable
class Foo25(val i: <!SERIALIZER_TYPE_INCOMPATIBLE!>@Serializable(with = BazSerializer::class) Bar<!>)
@Serializable
class Foo3(@Serializable(with = BazSerializer::class) val i: Baz)
@@ -23,6 +23,9 @@ class Foo(@Serializable(with = BazSerializer::class) val i: <!SERIALIZER_TYPE_IN
@Serializable
class Foo2(val li: List<@Serializable(with = BazSerializer::class) <!SERIALIZER_TYPE_INCOMPATIBLE!>Bar<!>>)
@Serializable
class Foo25(val i: @Serializable(with = BazSerializer::class) <!SERIALIZER_TYPE_INCOMPATIBLE!>Bar<!>)
@Serializable
class Foo3(@Serializable(with = BazSerializer::class) val i: Baz)