docs.scala-lang
docs.scala-lang copied to clipboard
Improve the "Backwards compatible case classes" guide with Scala 2 and 3 specifics
Continuation of
- https://github.com/scala/docs.scala-lang/pull/2662
- https://github.com/scala/docs.scala-lang/pull/2760
For Scala 3
fromProductis necessary: https://contributors.scala-lang.org/t/can-we-make-adding-a-parameter-with-a-default-value-binary-compatible/6132/8?u=sideeffffect- touching
unapplyshouldn't be necessary according to https://contributors.scala-lang.org/t/can-we-make-adding-a-parameter-with-a-default-value-binary-compatible/6132?u=sideeffffect
For Scala 2
copyneeds to be marked private manually (or maybe not? https://github.com/scala/docs.scala-lang/pull/2760#issuecomment-1533836924 )
/cc @julienrf
Not to remove the need for your improvement, but:
- (Scala 2) you have to add the compiler option
-Xsource:3- (Scala 2) define the
copymethod with all the current fields manually and set it asprivate- (Scala 2) define a private
unapplymethod in the companion object (note that by doing that the case class loses the ability to be used as an extractor in match expressions)- (Scala 3) define a custom
fromProductmethod in the companion object
makes it painfully obvious to me that this is not a viable strategy. At least not if you have Scala 2 in the mix. If you get any of those things wrong (by omitting it or even by writing the wrong signature), you end up with generated stuff anyway. And then you cannot get rid of it later.
TBH, I think we should not recommend this for Scala 2 at all. Developers will inevitably get it wrong. I would get it wrong.
painfully obvious to me that this is not a viable strategy
Maybe that has been my plan all along :smiling_imp:
But seriously now. I think there is a value for providing guidelines even for Scala 2. There are many eager volunteers maintaining popular libraries who would love to prevent their users from suffering from binary incompatibilities. These libraries are cross-compiled for both Scala 2 and 3. And having a guideline for both could help them. What would be the point not having this in the guide? It's complicated, yes, but it's not targeted at the ordinary Scala developer audience.
Anyway, I'm glad we now have a seed of an initiative to fix this pesky problem once and for all: https://contributors.scala-lang.org/t/can-we-make-adding-a-parameter-with-a-default-value-binary-compatible/6132
Continuing from https://github.com/scala/docs.scala-lang/pull/2788#discussion_r1185751629:
- what is the current state of mirrors (without defining
fromProduct)- demonstrate in a project example that defining
fromProductis enough to get type class derivation working on case classes with a private primary constructor, and that it works as expected when fields are added.
I am currently still skeptical that case classes can be evolved backwards-compatibly when you take into account derivation.
e.g. consider evolving case class Foo(bar: String) to case class Foo(bar: String, baz: Int). For source- and binary-compatibility we can add a constructor with a default value def apply(bar: String) = Foo(bar, 0) and follow all the other changes described in the guide.
Now, suppose someone downstream is using the mirror to derive a JSON decoder for Foo. Previously, that decoder would accept something like { "bar": "..." } but when they compile against the new API it will start expecting something like { "bar": "...", "baz": 42 } and raise a decoding error otherwise. So this would not be a compatible change and the program would break for previously valid input, even though the authors of Foo were careful to evolve it compatibly.
Specifically, mirror does not have a way to indicate which members were "added later" and what their default values are. It may be possible to emulate this in ad-hoc ways using Option or default parameters but I do not feel so sure about that.
Now, suppose someone downstream is using the mirror to derive a JSON decoder for
Foo. Previously, that decoder would accept something like{ "bar": "..." }but when they compile against the new API it will start expecting something like{ "bar": "...", "baz": 42 }and raise a decoding error otherwise.
The question is, whether defining custom fromProduct could help with that. I think it could.
But in any case, this is not a problem with binary compatibility, just to be clear. All the classes and methods from the old version are still present in the new.
Now, suppose someone downstream is using the mirror to derive a JSON decoder for
Foo. Previously, that decoder would accept something like{ "bar": "..." }but when they compile against the new API it will start expecting something like{ "bar": "...", "baz": 42 }and raise a decoding error otherwise. So this would not be a compatible change and the program would break for previously valid input, even though the authors ofFoowere careful to evolve it compatibly.
Not all decoders will be able to handle backwards compatibility in all possible cases. But many decoders can handle backwards compatibility in common cases.
"Add a new param on the left with a default value" is perhaps the most common one, and many JSON decoders already handle that in a backwards compatible manner by instantiating the default during de-serialization if the new field is not received in the input. uPickle goes the extra step to avoid emitting the key-value pair during serialization if the current value is equal to the default, to provide backwards compatibility during serialization as well, but that's a design choice and other libraries do it differently.
Being backwards compatible is certainly doable, within limits, and that's something that is up to the thing being derived to deal with
Here is a scenario that breaks mirrors and unapply (in Scala 3):
- in v1.1.0, define
case class Foo private (x: Int, s: Option[String] = None) - in v1.2.0, replace
swithy: Option[Int]:case class Foo private (x: Int, y: Option[Int] = None). It is possible to make that change in a backward binary-compatible way by still definingdef s: Option[String] = Nonein the class. - Now, consider a program that depends on v1.1.0. By summoning a
Mirror[Foo]it becomes possible to convert instances to a tuple of type(Int, Option[String])and vice versa. In that program, if we pattern match on a value of typeFoowith a “constructor pattern”, iecase Foo(x, s) => …,shas typeOption[String]. - Last, suppose that our program is now linked with the classfiles of v1.2.0. The mirror instance lives in the new classfiles, which means that the value it will accept or return will have different type than the type the program expects. Furthermore, the code that uses pattern matching will break at run-time because the type of the second field will be
Option[Int]instead of the expectedOption[String].
The conclusion is that Mirrors provide an equivalent to both apply and unapply, which both lead to binary incompatibilities.
@julienrf this is a great example where case classes can be evolved in a generally incompatible way that even MiMa wouldn't detect. (Maybe https://github.com/scalacenter/tasty-mima would be of help here?)
But I think it's intuitive even to Scala beginners that removing fields from case class constructor is problematic.
Anyway, in this PR we're trying to come up with a guide how to evolve case classes without breaking, not how to break them :joy: (there are thousand other ways that are even simpler, but I see how this example is interesting because of MiMa).
I think @lihaoyi has a great point here:
Being backwards compatible is certainly doable, within limits, and that's something that is up to the thing being derived to deal with
There's only so much you as a case class author can do. Then you have to draw a line beyond which it's the case class user's (the one doing the deriving and/or the Type Class author) responsibility.
Then you have to draw a line beyond which it's the case class user's (the one doing the deriving and/or the Type Class author) responsibility.
Yes. I believe it's the case class author's responsibility to evolve it compatibly, including providing default values for fields added after-the-fact that can be used by authors of typeclass derivation mechanisms. Unfortunately the Mirror API offers no way to indicate default values, so currently this can only be done in ad-hoc ways.
@lihaoyi you've mentioned that
many JSON decoders already handle that in a backwards compatible manner by instantiating the default during de-serialization if the new field is not received in the input
Could you tell us more about this? Are you doing this with uPickle on Scala 3? Are you using Mirror for that? Have you came across the issue @armanbilge has described? If yes, how did you work around it?
Hi all, what is the status of this PR?
Thank you for pinging us @bishabosha. There are still at least two discussions that need to be resolved before we can move forward with this PR. @sideeffffect, would you be interested in investigating the points raised in those discussions?