Macro generated `BSONWriter` and `BSONReader` fails to compile when writer/reader for `Option` is in scope
arainko opened this issue · 2 comments
ReactiveMongo Version (0.10.5, 0.11.6 / etc)
ReactiveMongo 1.0.10
Scala version
2.13.8, 2.11.7
Operating System (Ubuntu 15.10 / MacOS 10.10 / Windows 10)
Ubuntu 21.04
JDK (Oracle 1.8.0_72, OpenJDK 1.8.x, Azul Zing)
openjdk version "11.0.10" 2021-01-19 LTS
Expected Behavior
- I define a custom reader/writer for
Option
. - The defined reader and writer are resolved and properly used when deriving for a case class using the provided macros.
Actual Behavior
Given a case class like this:
final case class Person(name: String, age: Option[Int])
When a a custom Option
reader or writer is in scope when calling Macros.writer
or Macros.reader
(and their other flavors Macros.readerOpts
etc.) the generated code fails to type check.
Analyzing the output of Macros.writerOpts[Person, MacroOptions.Verbose]
for the age
field:
// Writer
Person.unapply(macroVal) match {
...
tuple$macro$1._2.foreach(((age$macro$9: Int) => (Person.this.optionAsNullWriter[Int](reactivemongo.api.bson.`package`.BSONIntegerHandler).writeTry(age$macro$9): _root_.scala.util.Try[_root_.reactivemongo.api.bson.BSONValue]) match {
case _root_.scala.util.Success((bson$macro$8 @ _)) => {
ok$macro$2.$plus$eq(_root_.reactivemongo.api.bson.BSONElement(macroCfg$macro$6.fieldNaming("age"), bson$macro$8));
()
}
case _root_.scala.util.Failure((cause$macro$4 @ _)) => {
err$macro$3.$plus$eq(_root_.reactivemongo.api.bson.exceptions.HandlerException("age", cause$macro$4));
()
}
}));
...
}
We can see that the macro calls foreach
on the age
field (which is of type Option[Int]
) and then tries to apply the resolved BSONWriter[Option[Int]]
inside foreach
(where it is of type Int
) hence the type mismatch; found : Int required: Option[Int]
compilation failure.
I think I managed to pinpoint the issue to this call for writer
where the macro actually takes notice of a BSONWriter
for Option
but it is not acted upon (the call is forwarded to the same case as the one without explicit BSONWriter
for `Option).
Following up with the output for Macros.readerOpts[Person, MacroOptions.Verbose]
for the age
field:
// Reader
{
...
macroDoc.getAsUnflattenedTry(macroCfg$macro$6.fieldNaming("age"))(Person.this.optionAsNullReader[Int](reactivemongo.api.bson.`package`.BSONIntegerHandler)) match {
case _root_.scala.util.Success((read$macro$7 @ _)) => {
age$macro$3.take(read$macro$7);
()
}
case _root_.scala.util.Failure((cause$macro$4 @ _)) => err$macro$1.$plus$eq(_root_.reactivemongo.api.bson.exceptions.HandlerException("age", cause$macro$4))
};
...
}
}
Here we can see the macro calls getAsUnflattenedTry
which has a result type Try[Option[T]]
, please note it calls it on a Option[Int]
which results in the type Try[Option[Option[Int]]
hence the second type error type mismatch; found : Option[Option[Int] required: Option[Int]
. I think the line responsible for this is located here.
Reproducible Test Case
package repro
import reactivemongo.api.bson.Macros
import scala.util.Success
import reactivemongo.api.bson._
final case class Person(name: String, age: Option[Int])
object Person {
implicit def optionAsNullWriter[A](implicit writer: BSONWriter[A]): BSONWriter[Option[A]] =
BSONWriter.from(_.map(writer.writeTry).getOrElse(Success(BSONNull)))
implicit def optionAsNullReader[A](implicit reader: BSONReader[A]): BSONReader[Option[A]] =
BSONReader.from {
case BSONNull => Success(None)
case value => reader.readTry(value).map(Some.apply)
}
implicit val writer: BSONDocumentWriter[Person] = Macros.writer[Person] // type mismatch; found : Int required: Option[Int]
implicit val reader: BSONDocumentReader[Person] = Macros.reader[Person] // type mismatch; found : Option[Option[Int] required: Option[Int]
}
You need to indicate the Scala version.
Anyway, defining handler for such purpose doesn't really make sense, rather use @NoneAsNull
You need to indicate the Scala version.
Yeah, sorry about that! I've edited the issue. The issue seems to be present on Scala 2.13 and 2.11.
However, while it compiles on Scala 2.12 and 3.1.1 they both ignore the instances for Option
if there are any in scope.
While this is understandable for 3.x.x (the MacroImpl
for this version seems to go straight for a Reader/Writer
for A
when it encounters an Option[A]
), I don't really get the behavior for 2.12. It seems to share the same MacroImpl
as 2.13 and 2.11 but its behavior differs from these two (I'm probably not familiar with the codebase enough).
Anyway, defining handler for such purpose doesn't really make sense, rather use @NoneAsNull
While generally I do agree, there are some cases where defining such handlers makes sense eg.:
- en masse
NoneAsNull
(from what I've tried, the annotation can only be applied on a per field basis, not for a case class as a whole), - you can't apply the annotation if you don't own the type (of course you should pretty much always have a separate model for DB access but as with all things in life - it's not always the case)
That being said, I wouldn't want to burden any of the maintainers with additional work so I'd like to volunteer to fix this myself if we were to reach an agreement on the semantics of this change (and of course if you want to support such a feature going forward!) 😄