Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Support derivation to non-sealed trait #564

Closed

Conversation

jtjeferreira
Copy link

allow derivation to non-sealed trait, but require runtime that some runtime overrides exists to evaluate the rule

allow derivation to non-sealed trait, but require runtime that some runtime overrides exists to evaluate the rule
@jtjeferreira jtjeferreira force-pushed the non-sealed-2nd-attempt branch from c396c69 to 018272b Compare June 30, 2024 20:59
Comment on lines +19 to +20
case (SealedHierarchy(Enum(fromElements)), _) if !ctx.config.areOverridesEmpty =>
mapEachSealedElementToAnotherSealedElement(fromElements, List.empty)
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

my first approach was like this but without the if !ctx.config.areOverridesEmpty. Only 3 tests failed, mostly because error messages were different:

  • io.scalaland.chimney.PartialTransformerStdLibTypesSpec
  • io.scalaland.chimney.IssuesSpec
  • io.scalaland.chimney.PartialTransformerIntegrationsSpec

For example the test io.scalaland.chimney.IssuesSpec.fix issue #121 error message changed from

Chimney can't derive transformation from io.scalaland.chimney.IssuesSpec.Foo to io.scalaland.chimney.IssuesSpec.Bar

io.scalaland.chimney.IssuesSpec.Bar
  maybeString: scala.collection.immutable.Seq[java.lang.String] - can't derive transformation from maybeString: scala.Option[scala.collection.immutable.Set[java.lang.String]] in source type io.scalaland.chimney.IssuesSpec.Foo
  nested: io.scalaland.chimney.IssuesSpec.BarNested - can't derive transformation from nested: io.scalaland.chimney.IssuesSpec.FooNested in source type io.scalaland.chimney.IssuesSpec.Foo

scala.collection.immutable.Seq[java.lang.String]
  derivation from foo.maybeString: scala.Option[scala.collection.immutable.Set[java.lang.String]] to scala.collection.immutable.Seq[java.lang.String] is not supported in Chimney!

java.lang.String
  derivation from foo.nested.num: scala.Option[scala.Int] to java.lang.String is not supported in Chimney!

io.scalaland.chimney.IssuesSpec.BarNested
  num: java.lang.String - can't derive transformation from num: scala.Option[scala.Int] in source type io.scalaland.chimney.IssuesSpec.FooNested

Consult https://chimney.readthedocs.io for usage examples.

to

Chimney can't derive transformation from io.scalaland.chimney.IssuesSpec.Foo to io.scalaland.chimney.IssuesSpec.Bar

java.lang.String
  derivation from none: scala.None to java.lang.String is not supported in Chimney!

scala.collection.immutable.Seq[java.lang.String]
  derivation from none: scala.None to scala.collection.immutable.Seq[java.lang.String] is not supported in Chimney!

io.scalaland.chimney.IssuesSpec.Bar
  maybeString: scala.collection.immutable.Seq[java.lang.String] - can't derive transformation from maybeString: scala.Option[scala.collection.immutable.Set[java.lang.String]] in source type io.scalaland.chimney.IssuesSpec.Foo
  nested: io.scalaland.chimney.IssuesSpec.BarNested - can't derive transformation from nested: io.scalaland.chimney.IssuesSpec.FooNested in source type io.scalaland.chimney.IssuesSpec.Foo

scala.collection.immutable.Seq[java.lang.String]
  can't transform coproduct instance scala.None to scala.collection.immutable.Seq[java.lang.String]
  can't transform coproduct instance scala.Some[scala.collection.immutable.Set[java.lang.String]] to scala.collection.immutable.Seq[java.lang.String]

java.lang.String
  derivation from some: scala.Some[scala.Int] to java.lang.String is not supported in Chimney!

java.lang.String
  can't transform coproduct instance scala.None to java.lang.String
  can't transform coproduct instance scala.Some[scala.Int] to java.lang.String

io.scalaland.chimney.IssuesSpec.BarNested
  num: java.lang.String - can't derive transformation from num: scala.Option[scala.Int] in source type io.scalaland.chimney.IssuesSpec.FooNested

scala.collection.immutable.Seq[java.lang.String]
  derivation from some: scala.Some[scala.collection.immutable.Set[java.lang.String]] to scala.collection.immutable.Seq[java.lang.String] is not supported in Chimney!

Consult https://chimney.readthedocs.io for usage examples.

which would make it worse...

Since when the target is not a sealed trait, the user needs to configure the transformation, adding the !ctx.config.areOverridesEmpty will only trigger this rule if some runtime overrides exists. WDYT?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change removed case (_, SealedHierarchy(_)) => case while leaving its message for the case (SealedHierarchy(_), _) => case. I think having it as if-else in the old case (SealedHierarchy(_), _) => would be safer.

@@ -760,4 +760,14 @@ class IssuesSpec extends ChimneySpec {
import Issue498.*
(Foo.Sub1("test"): Foo).into[Bar].withFieldConst(_.b, 1).transform ==> Bar.Sub1("test", 1)
}

test("support non-sealed trait destination") {
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let me know if a better place for this test exists

Comment on lines +765 to +770
(colors1.Green: colors1.Color)
.into[String]
.withSealedSubtypeHandled[colors1.Red.type](_ => "red")
.withSealedSubtypeHandled[colors1.Green.type](_ => "green")
.withSealedSubtypeHandled[colors1.Blue.type](_ => "blue")
.transform ==> "green"
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi again @MateuszKubuszok,

While working on this test, I suddenly realised that this is not much different than

(colors1.Green: colors1.Color) match {
  case colors1.Red => "red"
  case colors1.Green => "green"
  case colors1.Blue => "blue"
}

or if I wanted to use Transformer, for the subtypes I can do that:

(colors1.Green: colors1.Color) match {
  case red:colors1.Red.type => red.into[Foo].transform
  case green:colors1.Green.type => green.into[Foo].transform
  case blue:colors1.Blue.Type => blue.into[Foo].transform
}

so I am happy to drop this PR, since we can achieve the same the same result and with the same effort, using regular pattern matching...

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed, if the only goal is to add matching of each input subtype and provide manually value manually, then it's basically

implicit val transformer: Transformer[From, To] = {
  case ... => ...
  case ... => ...
}

with extra steps. I think writing them explicitly would make it easier to read and possibly less boilerplate'y than with a builder DSL.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah, that would work. I will close this PR

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

Successfully merging this pull request may close these issues.

2 participants