Adding support for using plain Writes/Format as custom implicits.

[ncreep]

When using custom formats with the the nested format it's now possible to use any Writes/Format.

The flat format still requires an OWrites/OFormat type.

Partially addresses #76.

This change required duplicating the content of the withTypeTag object, so that now we have withTypeTag.flat. Hopefully, this change won't be noticed by most users (as using withTypeTag with non-default arguments is probably rare).

[julienrf]

Hey @ncreep thank you for the PR! I think your solution is rather elegant, implementation-wise. However, I’m a bit worried by all the things we need to take care of to correctly provide custom formats that work as expected, and by the complexity the PR adds to the current design (which is already quite complex, IMHO).

[ncreep]

Seeing how I'm responsible for introducing the weird behavior with user-provided formats, I felt responsible to try and do something about it.

I can see your point about the complexity this adds, but the edge cases with mismatched/ignored formats are rather unpleasant.
The question is whether the current behavior as described in #76 is acceptable.

The alternative solution that I can see, was the one I suggested in #76, lifting the restriction on TypeTagOWrites to work with any Writes. That would require zero added complexity, but will introduce an edge case for the flat formats.

Or, worst case for my purposes, is to completely remove support for custom formats this way, and revert to the behavior before I started breaking things...

[julienrf]

The alternative solution that I can see, was the one I suggested in #76, lifting the restriction on TypeTagOWrites to work with any Writes. That would require zero added complexity, but will introduce an edge case for the flat formats.

If I understand correctly, we would not be able to detect, at compile-time, that someone tries to use a Write with flat, so the best we could do in that case would be to fail at run-time. Am I right?

I prefer what we have in this PR, if that’s the case.

Or, worst case for my purposes, is to completely remove support for custom formats this way ,and revert to the behavior before I started breaking things...

I think it’s a valuable feature, I wouldn’t drop it like that. I just wanted to take some time to brainstorm a little bit more about possible alternative solutions…

[ncreep]

I'm not aware of a technique to detect the Write/flat problem at compile-time (detecting what's being provided in the regular scope vs. generated is too deep for me, though it's probably possible with more type-tagging and added complexity). So I can only think of runtime behavior. But it doesn't have to be failure. As mentioned in my original issue, it's possible to fall back to a synthetic wrapper in that case. So if you have:

sealed trait Foo
case class Bar(x: Int) extends Foo

object Bar { implicit val format: Format[Bar] = Json.valueFormat }
object Foo { implicit val format: Format[Foo] = derived.flat[Foo]() }

Then the resulting JSON for Bar(42) would be something like:

{ "type": "Bar", "value": 42 }

Where the value field was auto-generated, never defined by the user, and doesn't really match the idea of being flat.
That may be a surprising edge case, but I would personally prefer that over the mismatched and failing Reads/Writes.

[julienrf]

That may be a surprising edge case, but I would personally prefer that over the mismatched and failing Reads/Writes.

Interesting, I didn’t think about that idea. I think that could be a good compromise that would reduce the complexity in play-json-derived-codecs, and offer a reasonable behavior (it needs to be properly documented, though). Would you be interested in changing to that?

[ncreep]

Yes, I will try to come up with a new pull request in the coming days.

Thanks

[ncreep]

Opened another pull request (#81)