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 enum companion reflection to scala.reflect #14136

Closed
wants to merge 6 commits into from

Conversation

soronpo
Copy link
Contributor

@soronpo soronpo commented Dec 19, 2021

As discussed in https://contributors.scala-lang.org/t/missing-dedicated-class-for-enum-companions/5118, I add ability to generalize over enum companion objects.
This PR adds the following to scala.reflect:

/** A base trait of all Scala enum companion definitions */
@annotation.transparentTrait trait EnumCompanion[E <: Enum] extends AnyRef

/** A base trait of all Scala singleton enum companion definitions */
@annotation.transparentTrait trait SingletonEnumCompanion[E <: Enum] extends EnumCompanion[E]:
  def values : Array[E]
  def valueOf(name : String) : E
  • If the enum is an enum of singletons, then SingletonEnumCompanion will be extended by the constructed compiler. Otherwise, EnumCompanion is extended.
  • If the enum has type arguments, then the extended companion fills them with wildcards.
  • See tests/run/enum-reflect-companion.scala for covered examples.

Note:
Currently the implementation is lacking where the user defines a custom companion object.
This object is not "fixed" to extend the companion reflection trait. It makes sense to do so, IMO.
Any help how to complete this will be appreciated.

@sjrd
Copy link
Member

sjrd commented Dec 19, 2021

I don't think we should do this for java.lang.Enums. Otherwise we lose the ability to define enums exactly like Java, that don't depend on the Scala standard library.

@soronpo
Copy link
Contributor Author

soronpo commented Dec 19, 2021

I don't think we should do this for java.lang.Enums. Otherwise we lose the ability to define enums exactly like Java, that don't depend on the Scala standard library.

So if I understand you correctly, if the user extends java.lang.Enum, then the behavior before this PR should continue unchanged, right?

@sjrd
Copy link
Member

sjrd commented Dec 19, 2021

Yes, exactly.

@bishabosha
Copy link
Member

I don't think we should do this for java.lang.Enums. Otherwise we lose the ability to define enums exactly like Java, that don't depend on the Scala standard library.

currently the java enums still extend scala.reflect.Enum

@sjrd
Copy link
Member

sjrd commented Dec 22, 2021

It occurred to me now that such a change is fundamentally incompatible with using the proposed -scala-release flag. If there is any enum in the codebase, it will now reference a trait added in a newer version of the library, and that would not be allowed when emitting TASTy compatible with older compilers.

@bishabosha
Copy link
Member

bishabosha commented Dec 22, 2021

It occurred to me now that such a change is fundamentally incompatible with using the proposed -scala-release flag. If there is any enum in the codebase, it will now reference a trait added in a newer version of the library, and that would not be allowed when emitting TASTy compatible with older compilers.

My suggestion in this case is to redesign this PR so that we do not extend the companion, but create a factory in Synthetics so we can use it as a type class with implicit search - If you want to you can cache enum companions also - like we do for ClassTag

@smarter
Copy link
Member

smarter commented Dec 22, 2021

If there is any enum in the codebase, it will now reference a trait added in a newer version of the library

Not necessarily if I understand this PR correctly: under -scala-release 3.1 and 3.0 enum desugaring should simply not extend the newly added traits.

@sjrd
Copy link
Member

sjrd commented Dec 22, 2021

If there is any enum in the codebase, it will now reference a trait added in a newer version of the library

Not necessarily if I understand this PR correctly: under -scala-release 3.1 and 3.0 enum desugaring should simply not extend the newly added traits.

That is not compatible with the design goal that -scala-release should not affect semantics.

@smarter
Copy link
Member

smarter commented Dec 22, 2021

I think it's fine for -scala-release to affect semantics when it's a situation like here where we're just turning off a new behavior to emit something compatible with an older compiler, if we don't allow -scala-release to do that sort of things we're likely to be stuck very quickly indeed.

@sjrd
Copy link
Member

sjrd commented Dec 22, 2021

That is a very slippery slope. What qualifies as "turning off new behavior" and what doesn't? Turning this off may influence implicit resolution somewhere else. It definitely affects semantics. The only "turning off" that we can tolerate is to reject an otherwise valid program. Any amount of alteration of the semantics is going to create problems down the line.

@smarter
Copy link
Member

smarter commented Dec 22, 2021

To me it's just the price users of -scala-release will need to be willing to pay to use this feature, of course we'll need to carefully document that so they're aware of what they're getting into.

@prolativ
Copy link
Contributor

In general the assumption was that -scala-release should not change the way how implicits are resolved (although it might make the compilation fail if the resolved implicit instance is not available in the older release). I think in this particular case the solution suggested by @bishabosha should solve the problem. And in general maybe we should put more effort into thinking how to extend the standard library in a way that would cause as little problems with forward compatibility as possible. Of course in some special cases it might turn out that some code will be impossible to compile e.g. with -scala-release 3.0 and one would need to target a newer release but I think in most situations with a clever design of the stdlib (and other libraries that would like to be binary forward compatible) this should work even though it might require rewriting some part of a user's code

@soronpo
Copy link
Contributor Author

soronpo commented Dec 22, 2021

It occurred to me now that such a change is fundamentally incompatible with using the proposed -scala-release flag. If there is any enum in the codebase, it will now reference a trait added in a newer version of the library, and that would not be allowed when emitting TASTy compatible with older compilers.

My suggestion in this case is to redesign this PR so that we do not extend the companion, but create a factory in Synthetics so we can use it as a type class with implicit search - If you want to you can cache enum companions also - like we do for ClassTag

Can you please scribble an example API of how a user will apply this? I don't mind changing the implementation, once we have an agreement of what it shall be.

@bishabosha
Copy link
Member

bishabosha commented Dec 22, 2021

Can you please scribble an example API of how a user will apply this? I don't mind changing the implementation, once we have an agreement of what it shall be.

sure, I meant something like

def names[E: SingletonEnumCompanion]: IndexedSeq[String] =
  summon[SingletonEnumCompanion[E]].values.toIndexedSeq.map(_.toString)

the compiler would then create an implicit argument magically, like we do for ClassTag, Typeable, Mirror etc.

@Lasering
Copy link

Lasering commented Feb 10, 2022

Consider including the following methods to SingletonEnumCompanion:

def fromOrdinal(ordinal: Int): E // The compiler is already generating it.
def valueOfOpt(name: String): Option[E] =
  scala.util.control.Exception.catching(classOf[IllegalArgumentException]).opt(valueOf(name))

def next(elem: E): E = fromOrdinal((elem.ordinal + 1) % values.length)
def previous(elem: E): E = fromOrdinal((elem.ordinal - 1) % values.length)

@Lasering
Copy link

Lasering commented May 4, 2022

Also it would be nice for the companion object to be a case object, or at least define a clean toString.

@Lasering
Copy link

Lasering commented May 9, 2022

transparent trait EnumObject[E <: Enum]

transparent trait SingletonEnumObject[E <: Enum] extends EnumObject[E] with Ordered[E] derives CanEqual:
  def compare(that: E): Int = that.ordinal
  def values: Array[E]
  def fromOrdinal(ordinal: Int): E
  def valueOf(name: String): E
  def valueOfOpt(name: String): Option[E] = catching(classOf[IllegalArgumentException]).opt(valueOf(name))
  def next(elem: E): E = fromOrdinal((elem.ordinal + 1) % values.length)
  def previous(elem: E): E = fromOrdinal((elem.ordinal - 1) % values.length)

Aditionally the object that will extend SingletonEnumObject should be a case object (mainly for toString), or alternatively toString should be filled by the compiler to a clean value (without the $).

@bishabosha
Copy link
Member

bishabosha commented May 9, 2022

```scala
transparent trait EnumObject[E <: Enum]

transparent trait SingletonEnumObject[E <: Enum] extends EnumObject[E] with Ordered[E] derives CanEqual:
  def compare(that: E): Int = that.ordinal
  def values: Array[E]
  def fromOrdinal(ordinal: Int): E
  def valueOf(name: String): E
  def valueOfOpt(name: String): Option[E] = catching(classOf[IllegalArgumentException]).opt(valueOf(name))
  def next(elem: E): E = fromOrdinal((elem.ordinal + 1) % values.length)
  def previous(elem: E): E = fromOrdinal((elem.ordinal - 1) % values.length)

Aditionally the object that will extend SingletonEnumObject should be a case object (mainly for toString), or alternatively toString should be filled by the compiler to a clean value (without the $).

with this you could also implement a Numeric, seeing as you are including modulo operations.

I would disagree with making the companion a case object, but we can improve toString

@Lasering
Copy link

I included next and previous because I've found them useful, however that is the only argument I have to suggest including them. And their ergonomics aren't that good for example:

enum SortMode:
  case Unsorted, Ascending, Descending
  
val currentSortMode = SortMode.Unsorted
val nextSortMode = SortMode.next(currentSortMode)
// However this would be much more ergonomic
val nextSortMode = currentSortMode.next()

I was kinda expecting those two methods to be rejected.

with this you could also implement a Numeric, seeing as you are including modulo operations.

I see why you are suggesting this, however I don't think it makes sense. I've used the modulo operations to implement the next and previous, however I'm not directly exposing the modulo operations. All the other methods provided by Numeric (plus, minus, times, negate, etc) don't strike me as something useful to have for singleton enums.

@mushtaq
Copy link

mushtaq commented Nov 11, 2022

This will be a very useful feature. Currently I can not find a way to abstract over valueOf method.

@ckipp01
Copy link
Member

ckipp01 commented May 10, 2023

Hey @soronpo, I wanted to check if this is something you plan on returning to? Or how should we move this forward?

@soronpo
Copy link
Contributor Author

soronpo commented May 10, 2023

My use-cases were eventually handled with meta-programming. In light of the (justified) pushback for the initial implementation, and the lack of time I'm now closing this. Anyone is welcome to pick it up. I will paste some relevant meta-programming magic that may help anyone approaching this PR:

// gets the case class from a companion object reference
trait CaseClass[Companion <: AnyRef, UB <: Product]:
  type CC <: UB

object CaseClass:
  type Aux[Comp <: AnyRef, UB <: Product, CC0 <: UB] = CaseClass[Comp, UB] { type CC = CC0 }
  transparent inline given [Comp <: AnyRef, UB <: Product]: CaseClass[Comp, UB] = ${
    macroImpl[Comp, UB]
  }
  def macroImpl[Comp <: AnyRef, UB <: Product](using
      Quotes,
      Type[Comp],
      Type[UB]
  ): Expr[CaseClass[Comp, UB]] =
    import quotes.reflect.*
    val clsTpe = TypeRepr.of[Comp].getCompanionClassTpe
    clsTpe.asType match
      case '[t & UB] =>
        type Case = t & UB
        '{
          new CaseClass[Comp, UB]:
            type CC = Case
        }
      case _ =>
        val msg =
          s"Type `${clsTpe.show}` is not a subtype of `${Type.show[UB]}`."
        '{
          compiletime.error(${ Expr(msg) })
          new CaseClass[Comp, UB]:
            type CC = UB
        }
    end match
  end macroImpl
end CaseClass

extension (using quotes: Quotes)(tpe: quotes.reflect.TypeRepr)
  // gets the class type from its companion object type
  def getCompanionClassTpe: quotes.reflect.TypeRepr =
    import quotes.reflect.*
    val compPrefix = tpe match
      case TermRef(pre, _) => pre
      case _               => report.errorAndAbort("Case class companion must be a term ref")
    val clsSym = tpe.typeSymbol.companionClass
    if !clsSym.paramSymss.forall(_.headOption.forall(_.isTerm)) then
      report.errorAndAbort("Case class with type parameters are not supported")
    compPrefix.select(clsSym)

@soronpo soronpo closed this May 10, 2023
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

8 participants