Skip to content

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

Merged
merged 4 commits into from
Jun 16, 2025
Merged

Serialization decoupling #354

merged 4 commits into from
Jun 16, 2025

Conversation

Mr3zee
Copy link
Collaborator

@Mr3zee Mr3zee commented Jun 13, 2025

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 by RpcTypeKrpc class. Compiler plugin extensions are needed to resolve this, https://youtrack.jetbrains.com/issue/KRPC-178/General-Approach-for-custom-Service-Descriptors

@Mr3zee Mr3zee requested a review from e5l June 13, 2025 15:31
@Mr3zee Mr3zee self-assigned this Jun 13, 2025
@Mr3zee Mr3zee added the feature New feature or request label Jun 13, 2025
@Mr3zee Mr3zee force-pushed the serilization-decoupling branch from 108cc32 to daf9e25 Compare June 13, 2025 15:31
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) }!!
Copy link
Member

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

Copy link
Collaborator Author

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

@e5l e5l requested a review from Copilot June 16, 2025 07:42
Copy link

@Copilot Copilot AI left a 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 ContextualTransient 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 only var properties marked with @Transient, but transient properties should be excluded. Change propertySymbol.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) {

@Mr3zee Mr3zee force-pushed the serilization-decoupling branch from b604652 to 9218007 Compare June 16, 2025 09:09
@Mr3zee Mr3zee merged commit 0700fb7 into main Jun 16, 2025
9 checks passed
@Mr3zee Mr3zee deleted the serilization-decoupling branch June 16, 2025 09:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants