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

Does Magnolia 1 for Scala 2 support getting the original index of a subtype in a sealed trait? #514

Open
ThijsBroersen opened this issue Feb 22, 2024 · 7 comments

Comments

@ThijsBroersen
Copy link

In Magnolia 1 for Scala 3 the index of the subtype is retrieved from the Mirror.SumOf.
In Magnolia 1 for Scala 2 the index of the subtype is determined after doing a .sortBy(_.fullName) using .zipWithIndex. ->

val genericSubtypes = knownSubclassesOf(classType.get).toList.sortBy(_.fullName)

Is the conclusion correct?

Is it possible to preserve the original index in the Scala 2 implementation?

@adamw
Copy link
Member

adamw commented Feb 28, 2024

Hm what would be the "original index" in case of Scala 2 implementation?

I'm not sure if such information is available ... plus, the hierarchies might be quite complex, so there might even not be a single good answer (there are some weird hierarchies in the tests I think)

@ThijsBroersen
Copy link
Author

Are you saying knownSubclassesOf(classType.get).toList does not reflect the exact order in which the subtypes are written in the source code?

@adamw
Copy link
Member

adamw commented Feb 28, 2024

Hm ... this sounds reasonable. Though I'm not 100% sure how Symbol.knownDirectSubclasses works. Maybe it would be doable :) We'd just have to account for the fact that some children might appear twice (as they might implement multiple traits)

@ThijsBroersen
Copy link
Author

Changing the behaviour will probably cause issues/pain with library users.

The main reason I raises this issue was because of Avro4s where the order of sealed trait and enums depend on Magnolia. The Avro-spec encodes enums to integers based the order of which they appear in the spec. So a change in the order will result in different avro encodings and wrongly decoding of old messages. Although IMHO deriving these types of specs is risky, it would be nice of it is possible to derive specs by the exact order in the Scale code.

The best approach would probably be to add a function which allows a user to get the subtypes as close to the order of the source code as possible.

@adamw
Copy link
Member

adamw commented Feb 28, 2024

Ah I see the problem. Yes, that would be a breaking change then. Not sure if adding another function is viable - it would have to be another top-level derive macro I guess?

@ThijsBroersen
Copy link
Author

What about adding a field to Subtype?

@adamw
Copy link
Member

adamw commented Feb 28, 2024

Good idea :) Since it's a trait, we could probably add a new method in a binary-compatible way. Then we'd need to keep the old constructors and delegate to the new ones. Seems doable.

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

No branches or pull requests

2 participants