ReactiveMongo/ReactiveMongo-BSON

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

  1. I define a custom reader/writer for Option.
  2. 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!) 😄