typelevel / cats

Lightweight, modular, and extensible library for functional programming.
https://typelevel.org/cats/
Other
5.25k stars 1.21k forks source link

Rethinking type class boilerplate generation #3192

Closed travisbrown closed 4 years ago

travisbrown commented 4 years ago

This issue is a follow-up to #2553 that focuses on the Simulacrum question and gives a concrete proposal for a replacement.

The problem is that Simulacrum 1 is based on Macro Paradise's macro annotations, which means it has a couple of disadvantages:

I've been experimenting with reworking Simulacrum's code generation as a set of Scalafix rules. This Scalafix-based approach resolves both of the macro-related issues:

This second part is also a disadvantage: in my current proof-of-concept you end up with a lot (like 2.2k lines) of boilerplate being added to the Cats source. It "reserves" the last part of the companion object of every type class for rule-managed code, and if contributors accidentally edit this part of these files, their changes will be overwritten.

I've tried a few ways of generating at least some of the syntax method boilerplate as managed source, in order to minimize the additions to checked-in code, but I don't think it's possible to do this in a way that makes sense without breaking binary compatibility. This might be a viable solution for Cats 3, though, and it would be pretty straightforward to change my proof-of-concept to make this configurable (i.e. your build indicates whether the generated code should go behind a banner in src/main/scala or in src_managed).

The alternative seems to be to implement the current Simulacrum functionality either as a Dotty compiler plugin or directly in the compiler.

If we do decide to go the Scalafix route, there's also a question of when. In theory it could happen at any time, since whether the code generation is done by a macro or a Scalafix rule shouldn't make any difference to the published artifacts.

Simulacrum isn't the only thing blocking cross-building on Dotty, though—the use of kind-projector is also a problem. My proof-of-concept also includes rules for rewriting kind-projector's syntax, but only for the sake of experimenting with Dotty cross-building (the result is an awful mess and not something I'd ever want to merge).

So personally I think we should switch out Simulacrum 1 with a Scalafix-based approach sooner rather than later, but we're still going to have to come up with a Dotty-friendly way to write type lambdas and polymorphic function value definitions before we can start cross-building.

smarter commented 4 years ago

The alternative seems to be to implement the current Simulacrum functionality either as a Dotty compiler plugin or directly in the compiler.

If there was no need to cross-compile with Scala 2, and Dotty features could be freely used, would any part of simulacrum still be needed ? If not, maybe the cats typeclasses could be written using Dotty features and somehow Scala 2 code would be generated from that.

My proof-of-concept also includes rules for rewriting kind-projector's syntax, but only for the sake of experimenting with Dotty cross-building (the result is an awful mess and not something I'd ever want to merge).

So dotty has a -Ykind-projector flag for this purpose, but it's not actually implemented yet :) https://github.com/lampepfl/dotty/blob/3a7dfd2c14e1bc146f79faef8fcad42efadcdaeb/compiler/src/dotty/tools/dotc/parsing/Parsers.scala#L1520-L1521 It probably wouldn't be too much work to fix this and support most of the kind projector syntax.

travisbrown commented 4 years ago

@smarter

would any part of simulacrum still be needed ?

@mpilquist's Spotted Leopards is an experiment in this direction. I'm not personally sold on this encoding of type classes (yet?)—I'd prefer to keep type class operations and syntax methods distinct.

There's also the apply summoner methods, etc., but I guess we could do those as a one-time thing.

So dotty has a -Ykind-projector flag for this purpose, but it's not actually implemented yet

My understanding though is that even when -Ykind-projector is implemented, it will only support the * syntax? Cats uses kind-projector's λ[α => F[G[α]]] a lot, so we'd still either need to make Dotty support that syntax or kind-project support Dotty's [α] =>> F[G[α]].

There's also the matter of kind-projector's value-level λ[ZipStream ~> Stream](_.value) syntax. Personally I think using this in Cats was a mistake, and that writing out new (Stream ~> ZipStream) is almost always much clearer. I guess I could put together a PR proposing that.

smarter commented 4 years ago

My understanding though is that even when -Ykind-projector is implemented, it will only support the * syntax? Cats uses kind-projector's λ[α => F[G[α]]] a lot, so we'd still either need to make Dotty support that syntax or kind-project support Dotty's [α] =>> F[G[α]].

I think it's reasonable to extend the scope of Ykind-projector to also support the lambda syntax, it's just a bit more work.

There's also the matter of kind-projector's value-level λZipStream ~> Stream syntax.

Eventually dotty should have polymorphic SAM which should help, meanwhile I agree that writing out the new looks fine too.

travisbrown commented 4 years ago

I think it's reasonable to extend the scope of Ykind-projector to also support the lambda syntax, it's just a bit more work.

@smarter That's great to hear! I don't want to sound impatient, but do you happen to have any sense of the timeline for work on -Ykind-projector? (I'd be happy to try to help out.)

smarter commented 4 years ago

No one is actively working on it, I'd like to do it but probably won't get to it in the short term, so if you or anyone else is interested, please have a look :) Since it's mostly parsing stuff it should be doable without any deep knowledge of dotty internals, and I'm more than happy to help along (I'm always on https://gitter.im/lampepfl/dotty)

smarter commented 4 years ago

As a first hint, you can get dotty to output the parser AST by running it with -Xprint:parser, furthermore you can see the raw AST instead of the pretty-printed trees by adding -Yplain-printer (and Thicket(List()) is the same thing as EmptyTree)

nafg commented 4 years ago

FWIW the problem of generated members is relevant for monocle @Lenses too. There's no way to do that as generated sources, and I can't imagine the scalafix approach being very pleasant.

I think we really need to convince the Dotty team that being able to generate synthetic members is a really important feature.

travisbrown commented 4 years ago

@nafg You definitely could use generated sources for something like Monocle lenses (or derived type class instances, etc.). I'm experimenting with a couple of approaches for Circe codecs:

I'm still not sure how these will feel in practice, but macro-based derivation definitely isn't pain-free either.

travisbrown commented 4 years ago

@smarter I'm looking at -Ykind-projector now and have a basic implementation working for *. What would be the best place to discuss the scope of the option (e.g. whether it should support -*, λ[`-α` => ...], etc.)?

smarter commented 4 years ago

@travisbrown I think it depends on how much complexity it brings, I asked Martin and he's OK with lambda syntax stuff like λ[`-α` => ...] so I suggest doing the minimum needed for cats.

smarter commented 4 years ago

(this could be discussed further on https://github.com/lampepfl/dotty/issues/7139)

ceedubs commented 4 years ago

@travisbrown boilerplate code generally feels bad, but I think that there are a couple of potential advantages to dropping the macro-based solution that you've only started to hint at with "The generated code is right there in front of you".

  1. Source navigation. I've seen coworkers get tripped up with a scenario like:
    • What's this bimap method that's being called on Either?
    • Click on bimap in IntelliJ. No source found.
    • Shrug. No way to know.
  2. ScalaDoc discoverability. From what I recall, some Simulacrum generated methods end up in the ScalaDocs, but not all of them. I never understood why this was the case, and maybe it no longer is. But even if they are, using Scalafix may open up options to actually give them useful ScalaDocs?
LukaJCB commented 4 years ago

I think we've delayed this decision long enough and @travisbrown has found a solution that should work in a backwards compatible way. I suggest we start implementing this if there are no objections within the next ~10 days?

travisbrown commented 4 years ago

I opened a PR (#3424) that splits this out from the Dotty cross-build (which has some open questions like which Dotty version we should support, since we're still missing e.g. the scalatestplus dependency for 0.24).

travisbrown commented 4 years ago

Done in #3424.