-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Conversation
I don't think we should do this for |
So if I understand you correctly, if the user extends |
Yes, exactly. |
currently the java enums still extend |
It occurred to me now that such a change is fundamentally incompatible with using the proposed |
My suggestion in this case is to redesign this PR so that we do not extend the companion, but create a factory in |
Not necessarily if I understand this PR correctly: under |
That is not compatible with the design goal that |
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. |
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. |
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. |
In general the assumption was that |
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 |
Consider including the following methods to 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) |
Also it would be nice for the companion object to be a case object, or at least define a clean toString. |
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 |
with this you could also implement a I would disagree with making the companion a case object, but we can improve |
I included 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.
I see why you are suggesting this, however I don't think it makes sense. I've used the modulo operations to implement the |
This will be a very useful feature. Currently I can not find a way to abstract over valueOf method. |
Hey @soronpo, I wanted to check if this is something you plan on returning to? Or how should we move this forward? |
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) |
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
:SingletonEnumCompanion
will be extended by the constructed compiler. Otherwise,EnumCompanion
is extended.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.