amnaredo / test

0 stars 0 forks source link

java.util.UUID reader and writer cannot be implemented due to assertion #111

Open amnaredo opened 3 years ago

amnaredo commented 3 years ago

I'm attempting to create implicit converters for the UUID class, as such:

object Helper {
  implicit val uuid2Writer = upickle.Writer[UUID] {
    case t: UUID => Js.Str(t.toString)
  }
  implicit val uuid2Reader = upickle.Reader[UUID] {
    case Js.Str(s) => UUID.fromString(s)
  }
}

It appears to compile up to the point where it fails an assertion, specifically Macros.scala:49

assert(!tpe.typeSymbol.fullName.startsWith("scala."))

As this is a java.util class, it fails the assertion, but what is the purpose of having this here? If this assertion were removed, would my implementation work as intended?

ID: 67 Original Author: k3d3

amnaredo commented 3 years ago

Are you sure this assertion fails because of your implicits as it specifically checks for scala. and not java.? The problem may be that you did not import the implicits with import Helper._ and uPickle tries to encode UUID's internal structure, which most likely contains java. types.

You could remove it, although I would advise against it. When refactoring the code, I came across this assertion a few times. The reason why the assertion exists is that uPickle implements all native types in Implicits and the macros should be only executed for user-defined types.

Your two implicits look reasonable and I don't see any reason why they should not be part of uPickle itself. So you could add them directly to the Implicits trait.

Original Author: tindzk

amnaredo commented 3 years ago

You are correct. Importing Helper appears to work.

That said, is it actually possible to extend the existing Implicits trait without modifying upickle itself?

Original Author: k3d3

amnaredo commented 3 years ago

No, right now this is not possible, which is one of the reasons why it would make sense to structure uPickle using the Cake approach internally (see #59).

Let's wait for Li to comment on this issue. I guess he wouldn't mind if your implicits become part of the upstream code.

Original Author: tindzk

amnaredo commented 3 years ago

Haven't had time to look at it, but in principle it seems reasonable now that java.util.UUID works in Scala.js out of the box

On Wed, Feb 11, 2015 at 11:00 AM, Tim Nieradzik notifications@github.com wrote:

No, right now this is not possible, which is one of the reasons why it would make sense to structure uPickle using the Cake approach internally (see #59 https://github.com/lihaoyi/upickle/issues/59).

Let's wait for Li to comment on this issue. I guess he wouldn't mind if your implicits become part of the upstream code.

— Reply to this email directly or view it on GitHub https://github.com/lihaoyi/upickle/issues/67#issuecomment-73941522.

Original Author: lihaoyi