spray / spray-json

A lightweight, clean and simple JSON implementation in Scala
Apache License 2.0
969 stars 190 forks source link

Fixed Option sometimes creating Some(null)s instead of Nones #333

Closed porterdarby closed 3 years ago

porterdarby commented 3 years ago

By using Some(x.convertTo[T]), if the implicit conversion resulted in a null for any reason, the result would be a Some(null) instead of the expected None.

The CI build is failing because:

spray.json.StandardFormats#OptionFormat has a different result type in current version, where it is scala.Option rather than scala.Some

Which should be reasonable.

jrudolph commented 3 years ago

It seems this would break round-trips and composition, though. The OptionFormat doesn't make any assumptions about the inner type and I think it should stay that way. This change seems only useful if a JsonFormat converts a non-JsNull value into a null object. That seems like a pretty special case which would warrant a special format that deals with Option[ConcreteType] directly and also provides proper writing of that special value.

jrudolph commented 3 years ago

(and sorry for letting this lay around for so long)