disneystreaming / smithy4s

https://disneystreaming.github.io/smithy4s/
Other
345 stars 69 forks source link

Should transitive mixins be taken into account in `@adt`? #1312

Open kubukoz opened 9 months ago

kubukoz commented 9 months ago

If you use @adt and all of the members apply a mixin directly, you get a sealed trait that also extends that mixin:

$version: "2"

namespace input

use smithy4s.meta#adt

@adt
union Items {
  s1: Item1
  s2: Item2
}

@mixin
structure HasStuff {
  stuff: String
}

structure Item1 with [HasStuff] {}
structure Item2 with [HasStuff] {}

generates an Items that starts with:

sealed trait Items extends HasStuff with scala.Product with scala.Serializable {

However, if you insert a mixin between ≥1 of the members and the desired mixin:

$version: "2"

namespace input

use smithy4s.meta#adt

@adt
union Items {
  s1: Item1
  s2: Item2
}

@mixin
structure HasStuff {
  stuff: String
}

+ @mixin
+ structure HasHasStuff with [HasStuff] {}

- structure Item1 with [HasStuff] {}
+ structure Item1 with [HasHasStuff] {}
structure Item2 with [HasStuff] {}

then you don't see HasStuff in the trait definition:

sealed trait Items extends scala.Product with scala.Serializable {

Should this be supported by "flattening" the mixin dependencies, or should the models be updated to avoid transitive mixins if we want to use @adt as designed?

kubukoz commented 9 months ago

Worth pointing out: the subtypes of Items all inherit HasStuff anyway, it's just that Item1 does so indirectly:

trait HasHasStuff extends HasStuff {
  def stuff: Option[String]
}

//...
case class Item1(stuff: Option[String] = None) extends Items with HasHasStuff