Piotr Krzemiński
10/18/2024, 6:45 AMcopy
and the change related to it that is going to be made in Kotlin 2.1.0 (ref). We do have some thoughts, but I wanted to consult it with wider group of people before we decide on something.
In the library github-workflows-kt, we generate certain data classes on the server side, and the users consume them via Kotlin Scripting. Here's a simple example of such generated class: SimpleActionWithRequiredStringInputs.kt. Notice what is done with the constructors: there are two, and the primary constructor is made private. It's because we want to enforce using named arguments when instantiating the class. To achieve it, we use a kind of hack with vararg pleaseUseNamedArguments: Unit
as the first arg, and it's done in the secondary constructor because Primary constructor vararg parameters are prohibited for data classes.
- that's why we also make the primary one private.
Now, the whole problem revolves around the fact that our users may already depend on the existence of the public copy
method. It's going to become private starting Kotlin 2.1.0. We want to keep providing this API for the users. These ideas came to our minds, among others:
1. implement copy
ourselves
◦ won't work since there would be conflicting overloads
2. ensure that named arguments are used in a different way, e.g. in runtime via reflection (performance-wise, it's OK), then we could make the primary constructor public
◦ is it even possible? named/positional arguments distinction exists probably only in compile time
3. make the classes regular (non-data
) and implement everything that data classes provide on our own, with the ultimate control over visibility
◦ well, it would probably work, but it's complex and I'm trying to see if there's another way 😄
So basically, the problem boils down to this: data classes (or data-like classes) with enforcing named arguments upon creation, working in Kotlin 2.1.0+.
Our discussion so far: https://kotlinlang.slack.com/archives/C02UUATR7RC/p1724584851609139
Any novel take on this problem is greatly appreciated!
CC @Vampireephemient
10/18/2024, 7:34 AMPiotr Krzemiński
10/18/2024, 7:36 AMephemient
10/18/2024, 7:38 AMephemient
10/18/2024, 7:38 AMdata class
to begin withPiotr Krzemiński
10/18/2024, 7:43 AMPiotr Krzemiński
10/18/2024, 7:48 AMephemient
10/18/2024, 7:58 AM@ExposedCopyVisibility
Piotr Krzemiński
10/18/2024, 7:59 AMPiotr Krzemiński
10/18/2024, 8:00 AM@ExposedCopyVisibility
does not solve it.
> It only ensures binary compatibility for already compiled code that uses the method according to its documentation
> but will not allow the consumer code to compile.
+
> As a quick fix the annotation is fine of course. I was under the impression you meant it as final solution. But from 2.1 on it will turn into an unsuppressible error.ephemient
10/18/2024, 8:04 AMcopy
legal or illegal)Vampire
10/18/2024, 11:53 AMillegal usages are `@ConsistentCopyVisibility data class private constructor(...)`'s `copy`; that's the upcoming default. the current behavior isThat's not what I read. There are two things. There is the declaration of@ExposedCopyVisibility
copy
done by the data class mechanism.
There is the call to the copy
function from consumer code.
This call in consumer code is, what is called "illegal *usage*".
Further down it also says, that as soon as the illegal usages are fixed the annotation should be dropped again.
Besides that, the intention of generating the methods we want was not only due to the copy
visibility.
The classes should never have been data classes (yes I know that I suggested it back then, mainly to get the copy
function), you can for example read more at https://kotlinlang.org/docs/api-guidelines-backward-compatibility.html.
The data classes undermine several points of why we do the named-argument forcing. For example for copy
you can use non-named arguments and thus any change in inputs except adding new in the end will be a source-breaking change and using decomposition also is a positional operation unless the feature for named decomposition is added to Kotlin, so if used has the same problem, which also is the reason I did not generate the componentX
methods in the PR.
I don't think there is any real alternative to generating the code to achieve the original goal, because any way that uses data classes includes the bad stuff we actually do not want.Piotr Krzemiński
10/18/2024, 11:57 AMI don't think there is any real alternative to generating the code to achieve the original goal, because any way that uses data classes includes the bad stuff we actually do not want.yeah, ok - I was trying to clarify whether we're in a rush preparing the library (the bindings server) for newer versions of Kotlin, or it's basically an independent change to move away from data classes. For me it looks like it's the latter
Vampire
10/18/2024, 11:59 AMVampire
10/18/2024, 12:00 PMPiotr Krzemiński
10/18/2024, 12:21 PMVampire
10/18/2024, 12:40 PM