Skip to content

feat: allow to derive enumeration codec for nested sealed hierarchies #2158

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 1 commit into
base: series/0.14.x
Choose a base branch
from
Open

feat: allow to derive enumeration codec for nested sealed hierarchies #2158

wants to merge 1 commit into from

Conversation

kpodsiad
Copy link

Hey folks! Current logic for deriving enum codec lacks ability to derive it for nested hierarchies. What do you think about adding it? In case answer is yes, here's the PR.

There is one caveat though, Mirror.Of[A].ordinal doesn't work as expected for nested hierarchies, that's why I needed to use summonTypeTestRecursively (credits to @Odomontois and https://github.com/evolution-gaming/derivation). Here's scala-cli snippet for quick showcase what's the problem

//> using scala "3.3.0"

import scala.deriving.Mirror

sealed trait Animal
object Animal:
  sealed trait Mammal extends Animal
  case object Dog extends Mammal
  case object Cat extends Mammal
  case object Bird extends Animal

object Main extends App {
  val mirror = summon[Mirror.Of[Animal]]
  assert(mirror.ordinal(Animal.Dog) == 0)
  assert(mirror.ordinal(Animal.Cat) == 0) // <<< HERE
  assert(mirror.ordinal(Animal.Bird) == 1)
}

@kpodsiad
Copy link
Author

@zarthross @zmccoy what do you folks think about it?

@MartinHH
Copy link
Contributor

Seems like their are several issues related to (scala 3) derivation for such nested "sum" hierarchies - see #2110 / #2156 and previously #2096 (all of them seemingly related to how those nested sums are "mirrored").

I guess ideally, one would take a step back and try to look at the bigger picture and try to find a better approach for all cases of scala 3 derivation for sums than fixing them one by one.

@kpodsiad
Copy link
Author

Thanks for letting me know @MartinHH. To some point I wasn't even aware of this until 2days ago we hit such errors ourselves :/ I can take a look at them and try to tackle nested sum problem, as soon at company I'm working at we might have more advanced cases. We're in the process of cross-compilation between Scala 2 & 3 and so far circe is one the biggest obstacle, I'll try to come up with some actionable actions.

@MartinHH
Copy link
Contributor

MartinHH commented Jul 2, 2023

I had a second (and third) look myself and came to the conclusion that the use cases and implementations of ConfiguredDe/EnCoder and ConfiguredEnumDe/Encoder are so different that one might as well go ahead and solve the "nesteds sum" problem in two different ways (i.e. keep following the route that was chosen for ConfiguredDe/EnCoder in #2096 and go ahead with the approach of this PR for ConfiguredEnumDe/Encoder). But that's just my opinion and in the end, it's up to the maintainers of this project...

@hamnis
Copy link
Collaborator

hamnis commented May 28, 2024

needs conflict fixing

This was referenced Apr 8, 2025
Merged
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@zarthross zarthross Awaiting requested review from zarthross zarthross is a code owner

@zmccoy zmccoy Awaiting requested review from zmccoy

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@kpodsiad @MartinHH @hamnis