-
Notifications
You must be signed in to change notification settings - Fork 30
Serialization decoupling #354
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
108cc32
to
daf9e25
Compare
override fun merge(key: K, value: V, remappingFunction: (V, V) -> V): V? { | ||
return map.merge(key, value) { old, new -> remappingFunction(old, new) } | ||
override fun merge(key: K, value: V, remappingFunction: (V, V) -> V): V { | ||
return map.merge(key, value) { old, new -> remappingFunction(old, new) }!! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please add meaningful error message
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There can't be null, as this map doesn't hold nulls, NPE is unreachable
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR decouples kotlinx.serialization
from the compiler plugin core by moving serialization logic to the protocol side and generating RPC payloads as parameter arrays.
- Removed all direct serialization-plugin dependencies and diagnostics
- Introduced
FirSerializablePropertiesProvider
as a session component - Simplified RPC service generator and strict-mode checker to use the new provider
Reviewed Changes
Copilot reviewed 74 out of 74 changed files in this pull request and generated no comments.
Show a summary per file
File | Description |
---|---|
compiler-plugin-k2/.../RpcDiagnosticRendererFactory.kt | Removed the missing-serialization diagnostic mapping |
compiler-plugin-k2/.../FirRpcDiagnostics.kt | Dropped the MISSING_SERIALIZATION_MODULE diagnostic |
compiler-plugin-k2/.../SerializableProperties.kt | Added FirSerializablePropertiesProvider for custom serialization logic |
compiler-plugin-k2/.../FirRpcStrictModeClassChecker.kt | Switched strict-mode checker to use new properties provider |
compiler-plugin-k2/.../FirRpcCheckers.kt | Simplified checker registration, no longer toggled by serialization presence |
compiler-plugin-k2/.../FirRpcAnnotationChecker.kt | Removed the serialization-module-missing check |
compiler-plugin-k2/.../FirVersionSpecificApi.kt | Introduced toRegularClassSymbolVS extension |
compiler-plugin-k2/.../FirRpcUtils.kt | Added doesMatchesClassId helper |
compiler-plugin-k2/.../FirRpcServiceGenerator.kt | Stripped out nested class generation via the serialization plugin |
compiler-plugin-k2/.../FirRpcExtensionRegistrar.kt | Always registers service generator and new session component |
compiler-plugin-k2/.../FirRpcAdditionalCheckers.kt | Removed the serializationIsPresent flag |
compiler-plugin-k2/.../FirGenerationKeys.kt | Eliminated RpcGeneratedRpcMethodClassKey |
compiler-plugin-k2/build.gradle.kts | Removed serialization.plugin dependency |
compiler-plugin-common/.../Names.kt | Updated Contextual → Transient ClassId and removed name helpers |
compiler-plugin-backend/.../VersionSpecificApiImpl.kt | Added VS API overrides for constructor parameters and nullability |
compiler-plugin-backend/.../extension/RpcIrContext.kt | Enhanced IR context with default rpc-type symbols and annotation utilities |
compiler-plugin-backend/.../VersionSpecificApi.kt | Declared new IR API members for use by VS implementations |
Comments suppressed due to low confidence (3)
compiler-plugin-k2/src/main/core/kotlinx/rpc/codegen/checkers/SerializableProperties.kt:64
- The logic for
var
properties is inverted: it currently includes onlyvar
properties marked with@Transient
, but transient properties should be excluded. ChangepropertySymbol.hasSerialTransient(session)
to!propertySymbol.hasSerialTransient(session)
.
else -> (propertySymbol.isVar && propertySymbol.hasSerialTransient(session))
compiler-plugin-k2/src/main/core/kotlinx/rpc/codegen/checkers/SerializableProperties.kt:41
- New
SerializablePropertiesProvider
logic covers complex scenarios (primary-constructor properties, inheritance, transient flags). Consider adding unit tests to verify each code path (val vs var, transient, private, superclass inheritance).
fun getSerializablePropertiesForClass(classSymbol: FirClassSymbol<*>): List<FirPropertySymbol> {
compiler-plugin-k2/src/main/core/kotlinx/rpc/codegen/checkers/SerializableProperties.kt:37
- [nitpick] Add KDoc to
FirSerializablePropertiesProvider
explaining its responsibility, caching behavior, and how it determines serializable properties (including handling of inheritance and@Transient
).
internal class FirSerializablePropertiesProvider(session: FirSession) : FirExtensionSessionComponent(session) {
b604652
to
9218007
Compare
Subsystem
Compiler Plugin, kRPC
Problem Description
kotlinx.serialization
was nailed down to our compiler plugin and core, which not always a proper coupling (like for gRPC it is unnecessary).Solution
Move serialization responsibility to protocol side, switch generated objects to an array of parameters.
Note: Very are still coupled in
core
byRpcTypeKrpc
class. Compiler plugin extensions are needed to resolve this, https://youtrack.jetbrains.com/issue/KRPC-178/General-Approach-for-custom-Service-Descriptors