com-lihaoyi / upickle

uPickle: a simple, fast, dependency-free JSON & Binary (MessagePack) serialization library for Scala
https://com-lihaoyi.github.io/upickle
MIT License
722 stars 165 forks source link

Make uPickle derivation in Scala 3 call `apply` method instead of `new` #607

Closed lihaoyi closed 4 months ago

lihaoyi commented 4 months ago

fixes https://github.com/com-lihaoyi/upickle/issues/552

Bumps minimum Scala 3 version to 3.4.2 because 3.3.3 crashes with weird errors

bishabosha commented 4 months ago

I think I have the right solution for matching the signatures - and rejects when there isn't a single match (seems to work well in the repl)

def apply(typeApply: Option[List[TypeRepr]]) = {
    val tpe = TypeRepr.of[T]
    val companion: Symbol = tpe.classSymbol.get.companionModule
    val constructorSym = tpe.typeSymbol.primaryConstructor
    val constructorParamSymss = constructorSym.paramSymss

    val (tparams0, params0) = constructorParamSymss.flatten.partition(_.isType)
    val constructorTpe = tpe.memberType(constructorSym).widen

    val companionRef = companion.termRef

    val sub: (tpe: TypeRepr, tparams: List[Symbol]) => TypeRepr =
      typeApply match
        case Some(tps) => (tpe, tparams) => tpe.substituteTypes(tparams, tps)
        case None => (tpe, tparams) => tpe

    val apps = for
      app <- companion.methodMember("apply")
      info = companionRef.memberType(app).widen
      (tps, ps) = app.paramSymss.flatten.partition(_.isType)
      if tps.length == tparams0.length
      if ps.length == params0.length
      if tps.corresponds(tparams0)((t0, t1) =>
        sub(info.memberType(t0), tps) =:= sub(constructorTpe.memberType(t1), tparams0)
      )
      if ps.corresponds(params0)((p0, p1) =>
        sub(info.memberType(p0), tps) =:= sub(constructorTpe.memberType(p1), tparams0)
      )
    yield app

    val matching = apps match
      case app :: Nil => app
      case _ => report.errorAndAbort(s"Cannot find a uniquely matching apply method in companion ${companionRef.show}")

    val lhs = Select(Ref(companion), matching)

    val rhs = params0.zipWithIndex.map {
      case (sym0, i) =>
        val lhs = '{$params(${ Expr(i) })}
        sub(constructorTpe.memberType(sym0), tparams0) match {
          case AnnotatedType(AppliedType(base, Seq(arg)), x)
            if x.tpe =:= defn.RepeatedAnnot.typeRef =>
            arg.asType match {
              case '[t] =>
                Typed(
                  lhs.asTerm,
                  TypeTree.of(using AppliedType(defn.RepeatedParamClass.typeRef, List(arg)).asType)
                )
            }
          case tpe =>
            tpe.asType match {
              case '[t] => '{ $lhs.asInstanceOf[t] }.asTerm
            }
        }

    }

    typeApply match{
      case None => lhs.appliedToArgs(rhs).asExprOf[T]
      case Some(args) => lhs.appliedToTypes(args).appliedToArgs(rhs).asExprOf[T]
    }
lihaoyi commented 4 months ago

Thanks @bishabosha ! I refactored the code a bit and updated the PR

lihaoyi commented 4 months ago

I don't think this quite follows the exact behavior of Scala 2. In particular, in Scala 2 the argument lists do not even need to match AFAIK; you can have an extra implicit parameter list the gets filled in by typechecking, or extra parameters with default values. But it will have to do for now, and we can refine the implementation further in future

DamianReeves commented 3 months ago

Is it possible to add a chore to revert the minimum Scala 3 version back down to Scala 3.3.4 once that becomes available with the backported changes?

It seems if a library depends on upickle 4.x it is forced to update to using Scala 3.4.x and it seems the intention of Scala Center is for library authors to avoid this, though the guidance does address needing new language features.

As upickle is a part of the toolkit and is used in many places in the ecosystem, it seems forcing the 3.4.2 here for the entirety of the upickle 4.x version will either limit users from upgrading or force the library authors using upickle to upgrade and thus produce a ripple effect.

I admit I may be dead wrong in my analysis here as the Scala LTS versioning strategy is still confusing to me. Also it seems we are around 2 years removed from the announcement of LTS, is a new LTS version perhaps on the horizon that means this is no big deal? 🤷🏾

lihaoyi commented 3 months ago

@DamianReeves yes, that's fine. We can add back support for 3.3.4 or 3.3.5 or whatever version has our bug fix backported. I just didn't want to hold up uPickle 4 for the folks on 2.12/2.13/3.4, but once we can compile on 3.3 we can publish a version supporting it

nafg commented 1 month ago

Did 3.3.4 fix the errors?

nafg commented 1 month ago

I guess it did https://github.com/com-lihaoyi/upickle/pull/635