Document the pattern for evolving case classes in a compatible manner

[sideeffffect]

Documenting the patter coming from SIP-50 - Struct Classes scala/improvement-proposals#50 (comment)
English is not my first language, so I'd be very grateful for suggestions on how to improve the wording, etc.

Rendered:
https://github.com/sideeffffect/docs.scala-lang/blob/patch-1/_overviews/tutorials/binary-compatibility-for-library-authors.md#evolving-code-without-breaking-binary-compatibility

[sideeffffect]

Also, do we have some part(s) of the documentation where we discuss the concept of "binary compatibility"? It would be desirable from them to this newly created section.

[sjrd]

https://docs.scala-lang.org/overviews/core/binary-compatibility-for-library-authors.html

[bjornregnell]

I think it should be explained that the ability use pattern matching is lost if no unapply?

[sjrd]

IMO this whole section belongs the guide on compatibility for library authors. Application authors never need to care about this.

[kubukoz]

nitpick: PR title has a typo (patter -> pattern)

[kubukoz]

this isn't a binary compatible change, right? You're removing the method apply that takes 2 parameters.

[sideeffffect]

Nope, you're not removing anything. You're only adding new stuff. That's the point.

Unless I missed something 😅

[kubukoz]

ahh sorry, I assumed this was replacing the apply present before. Of course you're right

[sideeffffect]

Thanks for the feedback, I've reworked it.

[Philippus]

Maybe add a note on the "deprecated" annotation?

[Philippus]

"would break its users" sounds a bit strange, don't have an alternative yet though.

[bjornregnell]

"would brake usage" or "would break compatible usage" or similar perhaps?

[bjornregnell]

Well, usage of instances in patterns is possible e.g. typed patterns such as
case p: Person =>
so it is "only" constructor patterns and extractor patterns that are not possible, if I'm correct?

So case Person(n,a) => would not work as unapply is private, but other kind of patterns work. So I think this should be clarified as the reader might not know about all the unapply mechanics under the hood...

The terminology of all the 15, or so, different kinds of patterns is available here: https://scala-lang.org/files/archive/spec/2.13/08-pattern-matching.html

[bjornregnell]

Edit: I guess extractor patterns could be made to work if the library author provides custom extractors?

[bjornregnell]

Anyway, I think it would be good to explain the implications on pattern matching in more detail...

[sideeffffect]

@julienrf Thanks for you suggestions. I've applied them all. But I think this part is worth a discussion.

Shouldn't we recommend readers to also include the apply methods in the companion object? Because

Person("Bob", 21).withAddress(Some("Atlantic ocean"))

looks a bit clumsy. This looks much better

Person("Bob", 21, "Atlantic ocean")

I know, we save on some boilerplate in the class definition. But we're just pushing the boilerplate onto the users of the class. Is that the right choice to make?

Ideally we'd have our cake and eat it too, but I don't know if it's possible without a new language level feature...

[julienrf]

I don’t think the pattern I suggested is clumsy. It is similar to the “builder” pattern we often see around.

Anyway, there are probably several possible variations. The one I suggest here provides a public-facing API that (I think) is simpler because it contains fewer overloads of the apply methods.

[bjornregnell]

Well, documentation space is not scarce :) so I suggest to explain both possibilities here: withX or overloaded apply constructors. It's up to the library author. And if the library user is "refused" a more concise constructor then it's just a self-made extension away...

[sideeffffect]

There are parts of this pattern which are pretty repetitive and very regular:

• add a new constructor, private to the companion object, with the original fields (necessary, unless we want to deal with MiMa)
• add an apply factory method to the companion (necessary, unless we want to burden the class users with boilerplate)

Are these parts automatable using Macros? We would need the macro(s) to generate as many of these members as there are Options in the case class (well, only the Options that are on the right side of the case class constructor definition).

(Btw good things about this patten is that it seems to work with e.g. Circe for JSON codecs https://scastie.scala-lang.org/huP4tCstQBufYKFwXUFs4Q )

[sjrd]

Anything that needs to add members visible during typechecking cannot be done with macros.

[julienrf]

@sideeffffect do you want to rework some parts or should we merge it as it is? (I am fine with both)

[sideeffffect]

I thought we want to first finish the MiMa vs "private" primary constructors debate.

https://contributors.scala-lang.org/t/private-primary-constructor-vs-mima/6050/5

But I'm not against merging this documentation before. What do you guys think?

[julienrf]

That’s a good point.

@smarter, what do you think about Sébastien’s suggestion of emitting the constructors as “package private” constructors? If we can do that, then I guess we would not need to add the MiMa exception anymore.

[smarter]

That would also be a binary breaking change which I assume mima would flag unless we special-case it in mima.

[julienrf]

I assume MiMa already treats package private definitions like private definitions. If this is not the case, that should be fixed indeed. But then MiMa would not flag changes to the class constructors if they are emitted as package private. Of course, code compiled with a new version of the compiler (emitting private case class constructors as package private constructors at the bytecode level) would be binary incompatible with code compiled with a previous compiler, so library authors would have to add a MiMa exception for that. But at least, the emitted bytecode would be more correct because it would become impossible to call the constructor from Java code.

[julienrf]

I created scala/bug#12711 and scala/scala3#16651 to track the issue with private constructors that are still public.

We could add a note about that in the documentation, what do you think?

[julienrf]

I did another pass and made some final adjustments. I think the guide is ready to go now.

[bishabosha]

This doesn't mention defining a new apply method in the companion (and keeping the old one) after adding the address field

[bjornregnell]

Minor thing, fix word ordering to be "this makes the copy method private".

[julienrf]

Thank you for the review, @bishabosha and @bjornregnell, I’ve addressed your comments in a818702