com-lihaoyi / upickle

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

Implicit conversion can easily lead to lost data #512

Open szeiger opened 10 months ago

szeiger commented 10 months ago

After refactoring some code from putting data into an ujson.Obj to directly updating the underlying LinkedHashMap we were left with code like this:

    val j = mutable.LinkedHashMap[String, ujson.Value]()
    j.value.put("n", ujson.Str(name))
    ujson.Obj(j)

It compiles without warnings but the n field never makes it into the object. The unnecessary .value call which got the underlying map from the previously used Obj now triggers an implicit conversion that copies the map:

  implicit def JsonableDict[T](items: TraversableOnce[(String, T)])
                              (implicit f: T => Value): Obj = Obj.from(items.map(x => (x._1, f(x._2))))

The new entry is only written into the copy and immediately discarded.

I'm not sure what the right solution is. Clearly .value is necessary. The implicit conversion could be removed but it may make the API harder to use in some cases.