-
-
Notifications
You must be signed in to change notification settings - Fork 87
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
Attempt to fix Scala 3 enum in snippet.sc, instead reproduce an issue with TypeRepr.of[A].memberType(subtype) #533
base: master
Are you sure you want to change the base?
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #533 +/- ##
==========================================
+ Coverage 87.72% 89.53% +1.81%
==========================================
Files 136 104 -32
Lines 5440 2589 -2851
Branches 438 243 -195
==========================================
- Hits 4772 2318 -2454
+ Misses 668 271 -397 ☔ View full report in Codecov by Sentry. |
….memberType(subtype)
998bc15
to
7e16b56
Compare
fromUntyped[A] { | ||
val isScala3EnumButNotJavaEnum = | ||
subtype.flags.is(Flags.Enum | Flags.StableRealizable) && !subtype.flags.is(Flags.JavaStatic) | ||
if isScala3EnumButNotJavaEnum then TypeRepr.of[A].memberType(subtype) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Recommended fix - unfortunately makes parameterless cases upcasted :(
} yield { | ||
import fixedFrom.Underlying as FixedFrom | ||
Ref(name).asExprOf[FixedFrom].asInstanceOf[Expr[From]] | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Naive attempt to apply TypeRepr.of[A].memberType(subtype)
fix once we already matched types. Does not work.
val fixedSym = | ||
if !sym.flags.is(Flags.JavaStatic) then TypeRepr.of[From].typeSymbol.children.find(_.name == sym.name).get | ||
else sym | ||
CaseDef(Bind(bindName, Ident(fixedSym.termRef)), None, body) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same as above.
.replaceAll("\\$\\d+", "") | ||
catch { | ||
case e: StackOverflowError => expr.toString | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For some reason, when working with cases that this PR attempts to fix, I sometimes get StackOverflow from expr.asTerm.show(using Printer.TreeAnsiCode)
:O
} | ||
|
||
new Snippet().result | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Poorly named test to check if fix worked without sbt publishLocal
-> scala-cli scripts/test-snippets.scala
trip.
I might consider some workaround where we'd fetch |
When Scala CLI
snippet.sc
is used macro generates assertion errors when Scala 3 enum is used:recommended way of calculating the type is
TypeRepr.of[A].memberType(subtypeSymbol)
the current way is based on
subtypeSymbol.typeRef
which cannot be used for types defined inand
enum
and eachcase
becomes a type defined in another class when we usesnippet.sc
(interestingly, the issue does not appear forcase object
s norcase class
es)however this recommended solution also upcasts parametherless cases, so that children of
are resolved to
List(Foo.Bar, Foo)
(which breaks macro in another way).Once this is solved, all Scala 3 snippets in documentation with enums - which required:
can have the workaround removed.
See:
typeMember
to select a type inherited from a base class scala/scala3#19825for the context (and why it's not so easy to "just fix in in the library").