Skip to content

Commit e954ac3

Browse files
sjrdsjrd
committed
Fix #14488: Scala.js: Add compiler support for scala.Enumeration.
This is the same logic that is used in the Scala.js compiler plugin for Scala 2. We catch ValDefs of the forms val SomeField = Value val SomeOtherField = Value(5) and rewrite them as val SomeField = Value("SomeField") val SomeOtherField = Value(5, "SomeOtherField") For calls to `Value` and `new Val` without name that we cannot rewrite, we emit warnings.
1 parent 0c4c967 commit e954ac3

File tree

6 files changed

+244
-8
lines changed

6 files changed

+244
-8
lines changed

‎community-build/src/scala/dotty/communitybuild/projects.scala

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -697,7 +697,8 @@ object projects:
697697

698698
lazy val playJson = SbtCommunityProject(
699699
project = "play-json",
700-
sbtTestCommand = "test",
700+
// EnumSpec.scala actively tests that `scala.Enumeration` is buggy in Scala.js
701+
sbtTestCommand = """set `play-json`.js / Test / unmanagedSources / excludeFilter := HiddenFileFilter || "EnumSpec.scala"; test""",
701702
sbtPublishCommand = "publishLocal",
702703
dependencies = List(scalatest, scalatestplusScalacheck),
703704
)

‎compiler/src/dotty/tools/backend/sjs/JSDefinitions.scala

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import scala.language.unsafeNulls
55
import scala.annotation.threadUnsafe
66

77
import dotty.tools.dotc.core._
8+
import Names._
89
import Types._
910
import Contexts._
1011
import Symbols._
@@ -257,6 +258,45 @@ final class JSDefinitions()(using Context) {
257258
allRefClassesCache
258259
}
259260

261+
/** Definitions related to scala.Enumeration. */
262+
object scalaEnumeration {
263+
val nmeValue = termName("Value")
264+
val nmeVal = termName("Val")
265+
val hasNext = termName("hasNext")
266+
val next = termName("next")
267+
268+
@threadUnsafe lazy val EnumerationClass = requiredClass("scala.Enumeration")
269+
@threadUnsafe lazy val Enumeration_Value_NoArg = EnumerationClass.requiredValue(nmeValue)
270+
@threadUnsafe lazy val Enumeration_Value_IntArg = EnumerationClass.requiredMethod(nmeValue, List(defn.IntType))
271+
@threadUnsafe lazy val Enumeration_Value_StringArg = EnumerationClass.requiredMethod(nmeValue, List(defn.StringType))
272+
@threadUnsafe lazy val Enumeration_Value_IntStringArg = EnumerationClass.requiredMethod(nmeValue, List(defn.IntType, defn.StringType))
273+
@threadUnsafe lazy val Enumeration_nextName = EnumerationClass.requiredMethod(termName("nextName"))
274+
275+
@threadUnsafe lazy val EnumerationValClass = EnumerationClass.requiredClass("Val")
276+
@threadUnsafe lazy val Enumeration_Val_NoArg = EnumerationValClass.requiredMethod(nme.CONSTRUCTOR, Nil)
277+
@threadUnsafe lazy val Enumeration_Val_IntArg = EnumerationValClass.requiredMethod(nme.CONSTRUCTOR, List(defn.IntType))
278+
@threadUnsafe lazy val Enumeration_Val_StringArg = EnumerationValClass.requiredMethod(nme.CONSTRUCTOR, List(defn.StringType))
279+
@threadUnsafe lazy val Enumeration_Val_IntStringArg = EnumerationValClass.requiredMethod(nme.CONSTRUCTOR, List(defn.IntType, defn.StringType))
280+
281+
def isValueMethod(sym: Symbol)(using Context): Boolean =
282+
sym.name == nmeValue && sym.owner == EnumerationClass
283+
284+
def isValueMethodNoName(sym: Symbol)(using Context): Boolean =
285+
isValueMethod(sym) && (sym == Enumeration_Value_NoArg || sym == Enumeration_Value_IntArg)
286+
287+
def isValueMethodName(sym: Symbol)(using Context): Boolean =
288+
isValueMethod(sym) && (sym == Enumeration_Value_StringArg || sym == Enumeration_Value_IntStringArg)
289+
290+
def isValCtor(sym: Symbol)(using Context): Boolean =
291+
sym.isClassConstructor && sym.owner == EnumerationValClass
292+
293+
def isValCtorNoName(sym: Symbol)(using Context): Boolean =
294+
isValCtor(sym) && (sym == Enumeration_Val_NoArg || sym == Enumeration_Val_IntArg)
295+
296+
def isValCtorName(sym: Symbol)(using Context): Boolean =
297+
isValCtor(sym) && (sym == Enumeration_Val_StringArg || sym == Enumeration_Val_IntStringArg)
298+
}
299+
260300
/** Definitions related to the treatment of JUnit bootstrappers. */
261301
object junit {
262302
@threadUnsafe lazy val TestAnnotType: TypeRef = requiredClassRef("org.junit.Test")

‎compiler/src/dotty/tools/dotc/transform/sjs/PrepJSInterop.scala

Lines changed: 105 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -163,7 +163,7 @@ class PrepJSInterop extends MacroTransform with IdentityDenotTransformer { thisP
163163
else if (enclosingOwner is OwnerKind.JSType)
164164
transformValOrDefDefInJSType(tree)
165165
else
166-
super.transform(tree) // There is nothing special to do for a Scala val or def
166+
transformScalaValOrDefDef(tree)
167167
}
168168
}
169169

@@ -186,9 +186,14 @@ class PrepJSInterop extends MacroTransform with IdentityDenotTransformer { thisP
186186
if (sym == jsdefn.PseudoUnionClass)
187187
sym.addAnnotation(jsdefn.JSTypeAnnot)
188188

189-
val kind =
190-
if (sym.is(Module)) OwnerKind.ScalaMod
191-
else OwnerKind.ScalaClass
189+
val kind = if (sym.isSubClass(jsdefn.scalaEnumeration.EnumerationClass)) {
190+
if (sym.is(Module)) OwnerKind.EnumMod
191+
else if (sym == jsdefn.scalaEnumeration.EnumerationClass) OwnerKind.EnumImpl
192+
else OwnerKind.EnumClass
193+
} else {
194+
if (sym.is(Module)) OwnerKind.NonEnumScalaMod
195+
else OwnerKind.NonEnumScalaClass
196+
}
192197
enterOwner(kind) {
193198
super.transform(tree)
194199
}
@@ -322,6 +327,38 @@ class PrepJSInterop extends MacroTransform with IdentityDenotTransformer { thisP
322327

323328
super.transform(tree)
324329

330+
// Warnings for scala.Enumeration.Value that could not be transformed
331+
case _:Ident | _:Select | _:Apply if jsdefn.scalaEnumeration.isValueMethodNoName(tree.symbol) =>
332+
report.warning(
333+
"Could not transform call to scala.Enumeration.Value.\n" +
334+
"The resulting program is unlikely to function properly as this operation requires reflection.",
335+
tree)
336+
super.transform(tree)
337+
338+
// Warnings for scala.Enumeration.Value with a `null` name
339+
case Apply(_, args) if jsdefn.scalaEnumeration.isValueMethodName(tree.symbol) && isNullLiteral(args.last) =>
340+
report.warning(
341+
"Passing null as name to scala.Enumeration.Value requires reflection at run-time.\n" +
342+
"The resulting program is unlikely to function properly.",
343+
tree)
344+
super.transform(tree)
345+
346+
// Warnings for scala.Enumeration.Val without name
347+
case _: Apply if jsdefn.scalaEnumeration.isValCtorNoName(tree.symbol) =>
348+
report.warning(
349+
"Calls to the non-string constructors of scala.Enumeration.Val require reflection at run-time.\n" +
350+
"The resulting program is unlikely to function properly.",
351+
tree)
352+
super.transform(tree)
353+
354+
// Warnings for scala.Enumeration.Val with a `null` name
355+
case Apply(_, args) if jsdefn.scalaEnumeration.isValCtorName(tree.symbol) && isNullLiteral(args.last) =>
356+
report.warning(
357+
"Passing null as name to a constructor of scala.Enumeration.Val requires reflection at run-time.\n" +
358+
"The resulting program is unlikely to function properly.",
359+
tree)
360+
super.transform(tree)
361+
325362
case _: Export =>
326363
if enclosingOwner is OwnerKind.JSNative then
327364
report.error("Native JS traits, classes and objects cannot contain exported definitions.", tree)
@@ -335,6 +372,10 @@ class PrepJSInterop extends MacroTransform with IdentityDenotTransformer { thisP
335372
}
336373
}
337374

375+
private def isNullLiteral(tree: Tree): Boolean = tree match
376+
case Literal(Constant(null)) => true
377+
case _ => false
378+
338379
private def validateJSConstructorOf(tree: Tree, tpeArg: Tree)(using Context): Unit = {
339380
val tpe = checkClassType(tpeArg.tpe, tpeArg.srcPos, traitReq = false, stablePrefixReq = false)
340381

@@ -625,6 +666,50 @@ class PrepJSInterop extends MacroTransform with IdentityDenotTransformer { thisP
625666
}
626667
}
627668

669+
/** Transforms a non-`@js.native` ValDef or DefDef in a Scala class. */
670+
private def transformScalaValOrDefDef(tree: ValOrDefDef)(using Context): Tree = {
671+
tree match {
672+
// Catch ValDefs in enumerations with simple calls to Value
673+
case vd: ValDef
674+
if (enclosingOwner is OwnerKind.Enum) && jsdefn.scalaEnumeration.isValueMethodNoName(vd.rhs.symbol) =>
675+
val enumDefn = jsdefn.scalaEnumeration
676+
677+
// Extract the Int argument if it is present
678+
val optIntArg = vd.rhs match {
679+
case _:Select | _:Ident => None
680+
case Apply(_, intArg :: Nil) => Some(intArg)
681+
}
682+
683+
val defaultName = vd.name.getterName.encode.toString
684+
685+
/* Construct the following tree
686+
*
687+
* if (nextName != null && nextName.hasNext)
688+
* nextName.next()
689+
* else
690+
* <defaultName>
691+
*/
692+
val thisClass = vd.symbol.owner.asClass
693+
val nextNameTree = This(thisClass).select(enumDefn.Enumeration_nextName)
694+
val nullCompTree = nextNameTree.select(nme.NE).appliedTo(Literal(Constant(null)))
695+
val hasNextTree = nextNameTree.select(enumDefn.hasNext)
696+
val condTree = nullCompTree.select(nme.ZAND).appliedTo(hasNextTree)
697+
val nameTree = If(condTree, nextNameTree.select(enumDefn.next).appliedToNone, Literal(Constant(defaultName)))
698+
699+
val newRhs = optIntArg match {
700+
case None =>
701+
This(thisClass).select(enumDefn.Enumeration_Value_StringArg).appliedTo(nameTree)
702+
case Some(intArg) =>
703+
This(thisClass).select(enumDefn.Enumeration_Value_IntStringArg).appliedTo(intArg, nameTree)
704+
}
705+
706+
cpy.ValDef(vd)(rhs = newRhs)
707+
708+
case _ =>
709+
super.transform(tree)
710+
}
711+
}
712+
628713
/** Verify a ValOrDefDef that is annotated with `@js.native`. */
629714
private def transformJSNativeValOrDefDef(tree: ValOrDefDef)(using Context): ValOrDefDef = {
630715
val sym = tree.symbol
@@ -1055,9 +1140,9 @@ object PrepJSInterop {
10551140
// Base kinds - those form a partition of all possible enclosing owners
10561141

10571142
/** A Scala class/trait. */
1058-
val ScalaClass = new OwnerKind(0x01)
1143+
val NonEnumScalaClass = new OwnerKind(0x01)
10591144
/** A Scala object. */
1060-
val ScalaMod = new OwnerKind(0x02)
1145+
val NonEnumScalaMod = new OwnerKind(0x02)
10611146
/** A native JS class/trait, which extends js.Any. */
10621147
val JSNativeClass = new OwnerKind(0x04)
10631148
/** A native JS object, which extends js.Any. */
@@ -1068,12 +1153,26 @@ object PrepJSInterop {
10681153
val JSTrait = new OwnerKind(0x20)
10691154
/** A non-native JS object. */
10701155
val JSMod = new OwnerKind(0x40)
1156+
/** A Scala class/trait that extends Enumeration. */
1157+
val EnumClass = new OwnerKind(0x80)
1158+
/** A Scala object that extends Enumeration. */
1159+
val EnumMod = new OwnerKind(0x100)
1160+
/** The Enumeration class itself. */
1161+
val EnumImpl = new OwnerKind(0x200)
10711162

10721163
// Compound kinds
10731164

1165+
/** A Scala class/trait, possibly Enumeration-related. */
1166+
val ScalaClass = NonEnumScalaClass | EnumClass | EnumImpl
1167+
/** A Scala object, possibly Enumeration-related. */
1168+
val ScalaMod = NonEnumScalaMod | EnumMod
1169+
10741170
/** A Scala class, trait or object, i.e., anything not extending js.Any. */
10751171
val ScalaType = ScalaClass | ScalaMod
10761172

1173+
/** A Scala class/trait/object extending Enumeration, but not Enumeration itself. */
1174+
val Enum = EnumClass | EnumMod
1175+
10771176
/** A native JS class/trait/object. */
10781177
val JSNative = JSNativeClass | JSNativeMod
10791178
/** A non-native JS class/trait/object. */

‎project/Build.scala

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1241,7 +1241,6 @@ object Build {
12411241
(
12421242
(dir / "shared/src/test/scala" ** (("*.scala": FileFilter)
12431243
-- "ReflectiveCallTest.scala" // uses many forms of structural calls that are not allowed in Scala 3 anymore
1244-
-- "EnumerationTest.scala" // scala.Enumeration support for Scala.js is not implemented in scalac (yet)
12451244
)).get
12461245

12471246
++ (dir / "shared/src/test/require-sam" ** "*.scala").get

‎tests/neg-scalajs/enumeration-warnings.check

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
-- Error: tests/neg-scalajs/enumeration-warnings.scala:6:4 -------------------------------------------------------------
2+
6 | Value // error
3+
| ^^^^^
4+
| Could not transform call to scala.Enumeration.Value.
5+
| The resulting program is unlikely to function properly as this operation requires reflection.
6+
-- Error: tests/neg-scalajs/enumeration-warnings.scala:10:9 ------------------------------------------------------------
7+
10 | Value(4) // error
8+
| ^^^^^^^^
9+
| Could not transform call to scala.Enumeration.Value.
10+
| The resulting program is unlikely to function properly as this operation requires reflection.
11+
-- Error: tests/neg-scalajs/enumeration-warnings.scala:15:15 -----------------------------------------------------------
12+
15 | val a = Value(null) // error
13+
| ^^^^^^^^^^^
14+
| Passing null as name to scala.Enumeration.Value requires reflection at run-time.
15+
| The resulting program is unlikely to function properly.
16+
-- Error: tests/neg-scalajs/enumeration-warnings.scala:16:15 -----------------------------------------------------------
17+
16 | val b = Value(10, null) // error
18+
| ^^^^^^^^^^^^^^^
19+
| Passing null as name to scala.Enumeration.Value requires reflection at run-time.
20+
| The resulting program is unlikely to function properly.
21+
-- Error: tests/neg-scalajs/enumeration-warnings.scala:20:10 -----------------------------------------------------------
22+
20 | val a = new Val // error
23+
| ^^^^^^^
24+
| Calls to the non-string constructors of scala.Enumeration.Val require reflection at run-time.
25+
| The resulting program is unlikely to function properly.
26+
-- Error: tests/neg-scalajs/enumeration-warnings.scala:21:10 -----------------------------------------------------------
27+
21 | val b = new Val(10) // error
28+
| ^^^^^^^^^^^
29+
| Calls to the non-string constructors of scala.Enumeration.Val require reflection at run-time.
30+
| The resulting program is unlikely to function properly.
31+
-- Error: tests/neg-scalajs/enumeration-warnings.scala:25:10 -----------------------------------------------------------
32+
25 | val a = new Val(null) // error
33+
| ^^^^^^^^^^^^^
34+
| Passing null as name to a constructor of scala.Enumeration.Val requires reflection at run-time.
35+
| The resulting program is unlikely to function properly.
36+
-- Error: tests/neg-scalajs/enumeration-warnings.scala:26:10 -----------------------------------------------------------
37+
26 | val b = new Val(10, null) // error
38+
| ^^^^^^^^^^^^^^^^^
39+
| Passing null as name to a constructor of scala.Enumeration.Val requires reflection at run-time.
40+
| The resulting program is unlikely to function properly.
41+
-- Error: tests/neg-scalajs/enumeration-warnings.scala:30:31 -----------------------------------------------------------
42+
30 | protected class Val1 extends Val // error
43+
| ^^^
44+
| Calls to the non-string constructors of scala.Enumeration.Val require reflection at run-time.
45+
| The resulting program is unlikely to function properly.
46+
-- Error: tests/neg-scalajs/enumeration-warnings.scala:31:31 -----------------------------------------------------------
47+
31 | protected class Val2 extends Val(1) // error
48+
| ^^^^^^
49+
| Calls to the non-string constructors of scala.Enumeration.Val require reflection at run-time.
50+
| The resulting program is unlikely to function properly.
51+
-- Error: tests/neg-scalajs/enumeration-warnings.scala:35:31 -----------------------------------------------------------
52+
35 | protected class Val1 extends Val(null) // error
53+
| ^^^^^^^^^
54+
| Passing null as name to a constructor of scala.Enumeration.Val requires reflection at run-time.
55+
| The resulting program is unlikely to function properly.
56+
-- Error: tests/neg-scalajs/enumeration-warnings.scala:36:31 -----------------------------------------------------------
57+
36 | protected class Val2 extends Val(1, null) // error
58+
| ^^^^^^^^^^^^
59+
| Passing null as name to a constructor of scala.Enumeration.Val requires reflection at run-time.
60+
| The resulting program is unlikely to function properly.

‎tests/neg-scalajs/enumeration-warnings.scala

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
// scalac: -Xfatal-warnings
2+
3+
class UnableToTransformValue extends Enumeration {
4+
val a = {
5+
println("oh, oh!")
6+
Value // error
7+
}
8+
val b = {
9+
println("oh, oh!")
10+
Value(4) // error
11+
}
12+
}
13+
14+
class ValueWithNullName extends Enumeration {
15+
val a = Value(null) // error
16+
val b = Value(10, null) // error
17+
}
18+
19+
class NewValWithNoName extends Enumeration {
20+
val a = new Val // error
21+
val b = new Val(10) // error
22+
}
23+
24+
class NewValWithNullName extends Enumeration {
25+
val a = new Val(null) // error
26+
val b = new Val(10, null) // error
27+
}
28+
29+
class ExtendsValWithNoName extends Enumeration {
30+
protected class Val1 extends Val // error
31+
protected class Val2 extends Val(1) // error
32+
}
33+
34+
class ExtendsValWithNullName extends Enumeration {
35+
protected class Val1 extends Val(null) // error
36+
protected class Val2 extends Val(1, null) // error
37+
}

0 commit comments

Comments
 (0)