Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add possibility to separate gen from combine #279

Closed
wants to merge 1 commit into from

Conversation

oyvindberg
Copy link

This solves two related issues with magnolia:

auto/semiauto

Some people prefer automatic derivation, while others prefer semiauto. As it stands now the type class author has to decide which one to offer (based on the existence of implicit of the gen method which is implemented by the magnolia macro)

This choice should be made by users instead. The mechanism introduced in this PR enables a type class author to offer both

Bundling more than one type class derivation behind one import

As it stands now, it's not really possible to offer more than one type class derivation through a wildcard import, because they ultimately need to be implemented in an object, where the combine methods and so on would clash.

Workarounds

If you look around you'll find that many projects have workarounds by writing their own macros around the magnolia macro. For instance:

Proposed solution

Add an annotation to gen where we can specify the object which has the combine method and the rest of the machinery.
For now this annotation is called @proxy, but I'm sure there are better names.

Example, given two non-interesting type classes called ToString1 and ToString2:

  object auto {
    @proxy(ToString1)
    implicit def deriveToString1[A]: ToString1[A] = macro Magnolia.gen[A]

    @proxy(ToString2)
    implicit def deriveToString2[A]: ToString2[A] = macro Magnolia.gen[A]
  }

  object semiauto {
    @proxy(ToString1)
    def deriveToString1[A]: ToString1[A] = macro Magnolia.gen[A]

    @proxy(ToString2)
    def deriveToString2[A]: ToString2[A] = macro Magnolia.gen[A]
  }

…ass derivation

Make it possible to export both semiauto and auto derivations, as well as more than one auto derivation through one import
@joroKr21
Copy link
Contributor

We have a lot of requests to customise the derivation but keep reaching for annotations when we don't know what to do.
I suggest a way to configure the macro:

  trait Config {
    type Proxy <: Singleton { type Typeclass[A] }
    type Ignore <: annotation.Annotation
    val readOnly: Boolean
    val minFields: Int
    val maxFields: Int
    val minCases: Int
    val maxCases: Int
  }

  def genWith[T: c.WeakTypeTag, C <: Config with Singleton: c.WeakTypeTag](c: whitebox.Context): c.Tree = {
    import c.internal._
    import c.universe._

    weakTypeOf[C].decls.toList.map(x => x -> x.typeSignature).foreach(println)
    Magnolia.gen[T](c)
  }

  object CsvConfig extends Magnolia.Config {
    type Proxy = Csv.type
    type Ignore = transient
    final val readOnly = true
    final val minFields = 0
    final val maxFields = -1
    final val minCases = 0
    final val maxCases = -1
  }

  implicit def deriveCsv[A]: Csv[A] = macro Magnolia.genWith[A, CsvConfig.type]

/*
(constructor CsvConfig,()magnolia.examples.Csv.CsvConfig.type)
(type Proxy,magnolia.examples.Csv.type)
(type Ignore,transient)
(value readOnly,=> Boolean(true))
(value readOnly,Boolean(true))
(value minFields,=> Int(0))
(value minFields,Int(0))
(value maxFields,=> Int(-1))
(value maxFields,Int(-1))
(value minCases,=> Int(0))
(value minCases,Int(0))
(value maxCases,=> Int(-1))
(value maxCases,Int(-1))
*/

@oyvindberg
Copy link
Author

Hey there @joroKr21 - sorry about the delay.

I'll preface by saying I would accept any encoding for this, including the one you proposed.

That said, I don't particularly think annotations are a bad fit for this. Most people have negative associations with annotations because how people use them at runtime - but as a compile-time thing I think they are completely fine! Especially for this particular usage, to pass static information to a macro. They are easy to use and easy to document.

I'm a bit skeptical towards the Config because you are immediately exposed to all options, even if they don't apply. The options themselves, however, are only valid by themselves in more narrow contexts. For instance:

  • @debug and @proxy apply to the macro expansion itself
  • @maxFields and @minFields belong with the implementation of combine (if any), and should absolutely not be configurable at macro-invocation time.
  • @maxCases and @minCases I imagine would belong with the implementation of dispatch (if any).

I don't exactly know where @ignore would fit in, probably along with @debug/@proxy as something supplied by the user?

@joroKr21
Copy link
Contributor

I may be traumatised by Spring when it comes to configuration with annotations 😃

I'm a bit skeptical towards the Config because you are immediately exposed to all options, even if they don't apply.

I actually like that because you can see all configuration options at a glance and you have autocomplete etc.
Note that we can provide default configuration so you wouldn't necessarily have to specify all options.

The options themselves, however, are only valid by themselves in more narrow contexts.

That's a valid point. Still I'm not sure which is better or worse.
Another thing we could go for is a @configure annotation with all options.

@oyvindberg
Copy link
Author

I may be traumatised by Spring when it comes to configuration with annotations smiley

Aren't we all :)

The Config trait actually has two further problems I think:

  • Backwards/forwards compatibility if we add more parameters, adding a member to a trait is not backwards compatible.
  • Your code seems to rely on lifting the values to the type level (via final), right? At that point you really want type maxCases <: Int, or if you account for optionality type maxCases <: Option[Int] = None with the override override type maxClasses = Some[1]?

If it's the discoverability we want we could scope all the annotations inside a objectmaybe?

Another thing we could go for is a @configure annotation with all options.

I like this better, but there will still be backwards compatibility issues when adding a parameter, unless we provide an overloaded constructor.

@plokhotnyuk
Copy link

@oyvindberg Am I understand right that if Config is used only in compile time then it is more about the source compatibility (no binary compatibility issues through transitive dependencies)? Another question about compile time annotations: will they be supported by Scala 3?

@oyvindberg
Copy link
Author

Am I understand right that if Config is used only in compile time then it is more about the source compatibility (no binary compatibility issues through transitive dependencies)?

@plokhotnyuk As long as it doesn't cross any sbt project boundary (where an old version of magnolia might have been evicted in favour of a newever version in a downstream project) I suppose that's true. I'm just wary I suppose after seeing #266 .

Another question about compile time annotations: will they be supported by Scala 3?

I would be incredibly surprised if they didn't, but let's find out!

@plokhotnyuk
Copy link

@oyvindberg
Copy link
Author

So I did some hacking on a scala 3 magnolia here one afternoon, and so far my impression is that compile-time annotations work just fine, and the @proxy pattern suggested in this PR seems to work.

In fact, if we want to cross-compile code which uses magnolia between Scala 2 and 3 I think we'll have to do something in that direction, to separate combine/dispatch from gen, since we don't want the macro keyword to appear in Scala 3 code.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants