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

Change in implicit resolution? #187

Closed
BenFradet opened this issue Jul 23, 2024 · 7 comments · Fixed by #191
Closed

Change in implicit resolution? #187

BenFradet opened this issue Jul 23, 2024 · 7 comments · Fixed by #191

Comments

@BenFradet
Copy link

BenFradet commented Jul 23, 2024

Hello 👋 ,

We're trying to upgrade a codebase from 0.2.2 to 0.2.3.

Our issues boil down to something like the following:

object Common:
  given Transformer.Fallible[Option, A, B] = ???
import Common.given
object Specialized:
  given Transformer.Fallible[Option, ADTA, ADTB] = _ match
    case V1A(v1) => v1.fallibleTo[V1B]
    // so on and so forth

This is failing with :

Couldn't build a transformation plan between A and scala.Option[B] @ V1.field

whereas it was working nicely with 0.2.2

Thank you,

@arainko
Copy link
Owner

arainko commented Jul 23, 2024

Heyo! Thanks for submitting a report, could you provide the exact definitions of ADTA and ADTB and the fallible transformer definition in Common or provide a more complete minimization? I'd like to drill in on the exact cause of the problem

@BenFradet
Copy link
Author

something like this:

import cats.syntax.apply.*
import io.github.arainko.ducktape.*

opaque type UnsignedLong = Long
extension (long: UnsignedLong) def value: Long = long
object UnsignedLong:
  def apply(l: Long): Either[String, UnsignedLong] =
    Either.cond(l >= 0, l, "Long was < 0, but must be >= 0")

final case class ClockL(epoch: Long, counter: Long)
final case class ClockUL(epoch: UnsignedLong, counter: UnsignedLong)

object Common:
  given Mode.FailFast.Option with {}
  given Transformer.Fallible[Option, Long, UnsignedLong] = l => UnsignedLong(l).toOption
  given Transformer.Fallible[Option, ClockL, ClockUL] = l =>
    (l.epoch.fallibleTo[UnsignedLong], l.counter.fallibleTo[UnsignedLong])
      .mapN(ClockUL.apply)

sealed trait A
final case class AA(w: AAWrapper) extends A
final case class AAWrapper(c: Option[ClockL])
sealed trait B
final case class BB(c: Option[ClockUL]) extends B

object Specialized:
  import Common.given
  given Transformer.Fallible[Option, A, B] =
    case AA(w) => w.fallibleTo[BB]

@arainko
Copy link
Owner

arainko commented Jul 24, 2024

Thanks! I'll try to get to the bottom of it asap

@arainko
Copy link
Owner

arainko commented Jul 27, 2024

Sorry, but I can't seem to reproduce the issue, my lightly edited minimization without the usage of cats seems to compile and work just fine:

class Issue187Suite extends DucktapeSuite {
  test("example works") {
    import Issue187Suite.*

    println(Issue187Suite.Specialized.t.transform(AA(AAWrapper(Some(ClockL(123, 123))))))
  }
}

object Issue187Suite {

  opaque type UnsignedLong = Long
  extension (long: UnsignedLong) def value: Long = long
  object UnsignedLong:
    def apply(l: Long): Either[String, UnsignedLong] =
      Either.cond(l >= 0, l, "Long was < 0, but must be >= 0")

  final case class ClockL(epoch: Long, counter: Long)
  final case class ClockUL(epoch: UnsignedLong, counter: UnsignedLong)

  object Common:
    given Mode.FailFast.Option with {}
    given Transformer.Fallible[Option, Long, UnsignedLong] = l => UnsignedLong(l).toOption
    given Transformer.Fallible[Option, ClockL, ClockUL] = l =>
      l.epoch.fallibleTo[UnsignedLong].zip(l.counter.fallibleTo[UnsignedLong]).map(ClockUL.apply)

  sealed trait A
  final case class AA(w: AAWrapper) extends A
  final case class AAWrapper(c: Option[ClockL])
  sealed trait B
  final case class BB(c: Option[ClockUL]) extends B

  object Specialized:
    import Common.given
    given t: Transformer.Fallible[Option, A, B] =
      case AA(w) => w.fallibleTo[BB]
}

the result is: Some(BB(Some(ClockUL(123,123))))

@arainko
Copy link
Owner

arainko commented Jul 27, 2024

I think I managed to capture the culprit with a transformation like this:

given Transformer.Fallible[Option, Int, String] = a => Some(a.toString)

case class Source(int: Int)
case class Dest(int: Option[String])

Mode.FailFast.option.locally {
   Source(1).fallibleTo[Dest]
}

and it indeed does work in 0.2.2 but breaks in 0.2.3 - it's definitely due to how F-unwrapping takes precedence to the special-cased transformations on options.

@arainko
Copy link
Owner

arainko commented Jul 28, 2024

I've published 0.2.3-16-ba40550-SNAPSHOT with the fix, it'd be extra-reassuring if you could test this release on your codebase before I pull the trigger on 0.2.4 ❤️

@necosta
Copy link

necosta commented Jul 29, 2024

Hi @arainko , thank you very much for the fix. I published from local 0.2.3-15-ff6b08c-SNAPSHOT (branch origin/issue-187) and the issue we had on v0.2.3 no longer occurs in this branch.

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