Skip to content
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

Generate declarations with internal/package-private visibility #2534

Closed
lukellmann opened this issue Dec 20, 2023 · 5 comments
Closed

Generate declarations with internal/package-private visibility #2534

lukellmann opened this issue Dec 20, 2023 · 5 comments
Labels

Comments

@lukellmann
Copy link
Contributor

lukellmann commented Dec 20, 2023

What is your use-case and why do you need this feature?

Some classes and methods generated by the compiler plugin seem to have public JVM visibility, as can be seen with a tool like Binary compatibility validator.

This can lead to huge changes in files tracking public api in libraries using kotlinx.serialization when the generation logic changes. An example for this are a couple commits in the Kord library:

  • public static final synthetic field descriptor Lkotlinx/serialization/descriptors/SerialDescriptor; disappeared in generated serializer classes with Kotlin 1.8.0
  • write$Self methods in serializable classes became synthetic with Kotlin 1.8.20
  • synthetic constructors taking kotlinx.serialization.internal.SerializationConstructorMarker and write$Self methods disappeared in serializable classes with Kotlin 1.9.20
  • synthetic constructors and write$Self methods are back (fixed by KT-64124), generated serializer classes changed from final to synthetic and some methods in generated serializer classes are now final with K2

Describe the solution you'd like

To avoid changes in the generation logic of the serialization compiler plugin causing changes in the public api of libraries using it, generated declarations should have lower visibility than public, e.g. internal (or even package-private to avoid exposing them to Java clients). These lower visibilities are usually ignored by tools like BCV.

@lukellmann
Copy link
Contributor Author

might be related to https://youtrack.jetbrains.com/issue/KT-64124

@sandwwraith
Copy link
Member

All changes you've described here are, in fact, connected with your original request and made to make generated declarations less intrusive for tools.

synthetic modifier to be excluded from code coverage tools — #961
Lowering visibility from public to internal when possible — #2209
Regarding re-appearance of constructors and writeSelf — it is indeed https://youtrack.jetbrains.com/issue/KT-64124, and it's already fixed, as you can see. The rest of K2 changes are dictated by new plugin API, not kotlinx.serialization. But I think they're better that way, because of the new synthetic modifier on $serializer — that helps with code coverage tools.

For the rest of the declarations: it is by design that they are public if the serializable class is public. See explanation here: #2108 (comment)

@lukellmann
Copy link
Contributor Author

lukellmann commented Dec 21, 2023

All changes you've described here are, in fact, connected with your original request and made to make generated declarations less intrusive for tools.

That's nice to hear, probably should have dug a bit deeper before opening this issue.

For the rest of the declarations: it is by design that they are public if the serializable class is public. See explanation here: #2108 (comment)

For this class I understand that the companion object and the serializer function in it are part of the public API:

package org.example

@Serializable
public class Data(public val data: String)
public final class org/example/Data {
	public static final field Companion Lorg/example/Data$Companion;
	public fun <init> (Ljava/lang/String;)V
	public final fun getData ()Ljava/lang/String;
}

public final class org/example/Data$Companion {
	public final fun serializer ()Lkotlinx/serialization/KSerializer;
}

But why is the generated serializer also public? It can be obtained with Data.serializer(), so shouldn't it be possible to make it internal? Or am I missing something?

public final class org/example/Data$$serializer : kotlinx/serialization/internal/GeneratedSerializer {
	public static final field INSTANCE Lorg/example/Data$$serializer;
	public fun childSerializers ()[Lkotlinx/serialization/KSerializer;
	public fun deserialize (Lkotlinx/serialization/encoding/Decoder;)Lorg/example/Data;
	public synthetic fun deserialize (Lkotlinx/serialization/encoding/Decoder;)Ljava/lang/Object;
	public fun getDescriptor ()Lkotlinx/serialization/descriptors/SerialDescriptor;
	public fun serialize (Lkotlinx/serialization/encoding/Encoder;Lorg/example/Data;)V
	public synthetic fun serialize (Lkotlinx/serialization/encoding/Encoder;Ljava/lang/Object;)V
	public fun typeParametersSerializers ()[Lkotlinx/serialization/KSerializer;
}

If you write a custom serializer, it doesn't have to be public:

@OptIn(ExperimentalSerializationApi::class)
@Serializer(forClass = Data::class) // or some other way to write a custom serializer
private object DataSerializer

@Serializable(with = DataSerializer::class)
public class Data(public val data: String)

@sandwwraith
Copy link
Member

But why is the generated serializer also public? It can be obtained with Data.serializer(), so shouldn't it be possible to make it internal?

Yes and no. Some older versions of the plugin reference serializer directly instead of using serializer(), and for some cases, it is accessed directly in actual versions, too.

If you write a custom serializer, it doesn't have to be public

Because private object in compiled bytecode really has internal/public visibility (private Java classes obviously can't be referenced from other classes, so doing so would make the object inaccessible).
You can get errors in such code when e.g. using it together with JPMS: #2495

@lukellmann
Copy link
Contributor Author

Yes and no. Some older versions of the plugin reference serializer directly instead of using serializer(), and for some cases, it is accessed directly in actual versions, too.

I see, so this seems like this can't be changed in the short term.

Because private object in compiled bytecode really has internal/public visibility [...]

It's technically package-private, as can be seen by javap ./path/to/DataSerializer.class, but that doesn't change the situation.

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants