UPickle doesn't work with overloaded apply methods

[amnaredo]

Consider:

package com.example

import upickle._

case class Container(value: String)

object Container {
  def apply(intValue: Int): Container = Container(intValue.toString)
}

object ContainerApp extends App {
  write(Container("V"))
}

Compiling this leads to an error:

[error] /Users/ramnivas/test-dev/upickle-issue/src/main/scala/com/example/OverloadedApply.scala:14: uPickle does not know how to write [com.example.Container]s; define an implicit Writer[com.example.Container] to teach it how
[error]   val cWritten = write(c)

If I remove the apply(Int) method, it compiles fine.

Even the following compiles fine (has an auxiliary constructor; requires new when using that constructor):

package com.example

import upickle._

case class Container(value: String) {
  def this(intValue: Int) = this(intValue.toString)
}

object ContainerApp extends App {
  write(new Container(5))
}

I think uPickle should consider only the primary apply method, much the same way it seems to consider only the primary constructor.

ID: 48 Original Author: ramnivas

[amnaredo]

In the general case, how would you define what the "primary" apply method is though? Perhaps the one that matches the primary constructor if there are multiple? But then you wouldn't be able to write a custom apply method to customize the writing of a case class, which probably is a nice thing to be able to do. Not sure what the best thing here is, though of course compilation-error isn't great either

Original Author: lihaoyi

[amnaredo]

Yah, I think the primary apply method from uPickle's point of view should the one that the Scala compiler generates for a case class i.e. the one that matches the primary constructor. Basically, adding an overloaded apply() methods shouldn't break pickling that was automatically available through uPickle.

I am not sure I understood what you mean by "But then you wouldn't be able to write a custom apply method..."

Original Author: ramnivas

[amnaredo]

I meant that us picking the non-default apply would give users the flexibility of using a secondary apply/unapply to customize the serialization behavior. Not that they can do that now (because it blows up) but it would be possible if we chose the user-written apply to generate the serialization code.

Of course, they can already override it with an implicit Writer and Reader in the companion object, so maybe it's worth standardizing on one-and-only-one way of customizing these things

Original Author: lihaoyi

[amnaredo]

Yah, for now primary-apply or implicit Reader+Writer seems easiest to explain.

Original Author: ramnivas

[amnaredo]

I just ran into this issue. My opinion is that upickle should only use the primary apply method and if you want to customize serialization you implement a reader/writer. I think it's confusing that other apply overloads would change how upickle works.

Original Author: zlangbert

[amnaredo]

Fixed in https://github.com/lihaoyi/upickle-pprint/commit/b8d897e8f589e2144d668d4b3978b371232eb591 to only ever use the primary apply which matches the primary constructor. Customization via custom apply/unapply is now broken and the unit tests have been removed.

Original Author: lihaoyi