Skip to content

Commit

Permalink
Fix scala#14488: Scala.js: Add compiler support for scala.Enumeration.
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
sjrd committed Jul 28, 2022
1 parent a2d9635 commit b419bca
Show file tree
Hide file tree
Showing 6 changed files with 244 additions and 8 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -694,7 +694,8 @@ object projects:

lazy val playJson = SbtCommunityProject(
project = "play-json",
sbtTestCommand = "test",
// EnumSpec.scala actively tests that `scala.Enumeration` is buggy in Scala.js
sbtTestCommand = """set `play-json`.js / Test / unmanagedSources / excludeFilter := HiddenFileFilter || "EnumSpec.scala"; test""",
sbtPublishCommand = "publishLocal",
dependencies = List(scalatest, scalatestplusScalacheck),
)
Expand Down
40 changes: 40 additions & 0 deletions compiler/src/dotty/tools/backend/sjs/JSDefinitions.scala
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import scala.language.unsafeNulls
import scala.annotation.threadUnsafe

import dotty.tools.dotc.core._
import Names._
import Types._
import Contexts._
import Symbols._
Expand Down Expand Up @@ -257,6 +258,45 @@ final class JSDefinitions()(using Context) {
allRefClassesCache
}

/** Definitions related to scala.Enumeration. */
object scalaEnumeration {
val nmeValue = termName("Value")
val nmeVal = termName("Val")
val hasNext = termName("hasNext")
val next = termName("next")

@threadUnsafe lazy val EnumerationClass = requiredClass("scala.Enumeration")
@threadUnsafe lazy val Enumeration_Value_NoArg = EnumerationClass.requiredValue(nmeValue)
@threadUnsafe lazy val Enumeration_Value_IntArg = EnumerationClass.requiredMethod(nmeValue, List(defn.IntType))
@threadUnsafe lazy val Enumeration_Value_StringArg = EnumerationClass.requiredMethod(nmeValue, List(defn.StringType))
@threadUnsafe lazy val Enumeration_Value_IntStringArg = EnumerationClass.requiredMethod(nmeValue, List(defn.IntType, defn.StringType))
@threadUnsafe lazy val Enumeration_nextName = EnumerationClass.requiredMethod(termName("nextName"))

@threadUnsafe lazy val EnumerationValClass = EnumerationClass.requiredClass("Val")
@threadUnsafe lazy val Enumeration_Val_NoArg = EnumerationValClass.requiredMethod(nme.CONSTRUCTOR, Nil)
@threadUnsafe lazy val Enumeration_Val_IntArg = EnumerationValClass.requiredMethod(nme.CONSTRUCTOR, List(defn.IntType))
@threadUnsafe lazy val Enumeration_Val_StringArg = EnumerationValClass.requiredMethod(nme.CONSTRUCTOR, List(defn.StringType))
@threadUnsafe lazy val Enumeration_Val_IntStringArg = EnumerationValClass.requiredMethod(nme.CONSTRUCTOR, List(defn.IntType, defn.StringType))

def isValueMethod(sym: Symbol)(using Context): Boolean =
sym.name == nmeValue && sym.owner == EnumerationClass

def isValueMethodNoName(sym: Symbol)(using Context): Boolean =
isValueMethod(sym) && (sym == Enumeration_Value_NoArg || sym == Enumeration_Value_IntArg)

def isValueMethodName(sym: Symbol)(using Context): Boolean =
isValueMethod(sym) && (sym == Enumeration_Value_StringArg || sym == Enumeration_Value_IntStringArg)

def isValCtor(sym: Symbol)(using Context): Boolean =
sym.isClassConstructor && sym.owner == EnumerationValClass

def isValCtorNoName(sym: Symbol)(using Context): Boolean =
isValCtor(sym) && (sym == Enumeration_Val_NoArg || sym == Enumeration_Val_IntArg)

def isValCtorName(sym: Symbol)(using Context): Boolean =
isValCtor(sym) && (sym == Enumeration_Val_StringArg || sym == Enumeration_Val_IntStringArg)
}

/** Definitions related to the treatment of JUnit bootstrappers. */
object junit {
@threadUnsafe lazy val TestAnnotType: TypeRef = requiredClassRef("org.junit.Test")
Expand Down
111 changes: 105 additions & 6 deletions compiler/src/dotty/tools/dotc/transform/sjs/PrepJSInterop.scala
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,7 @@ class PrepJSInterop extends MacroTransform with IdentityDenotTransformer { thisP
else if (enclosingOwner is OwnerKind.JSType)
transformValOrDefDefInJSType(tree)
else
super.transform(tree) // There is nothing special to do for a Scala val or def
transformScalaValOrDefDef(tree)
}
}

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

val kind =
if (sym.is(Module)) OwnerKind.ScalaMod
else OwnerKind.ScalaClass
val kind = if (sym.isSubClass(jsdefn.scalaEnumeration.EnumerationClass)) {
if (sym.is(Module)) OwnerKind.EnumMod
else if (sym == jsdefn.scalaEnumeration.EnumerationClass) OwnerKind.EnumImpl
else OwnerKind.EnumClass
} else {
if (sym.is(Module)) OwnerKind.NonEnumScalaMod
else OwnerKind.NonEnumScalaClass
}
enterOwner(kind) {
super.transform(tree)
}
Expand Down Expand Up @@ -322,6 +327,38 @@ class PrepJSInterop extends MacroTransform with IdentityDenotTransformer { thisP

super.transform(tree)

// Warnings for scala.Enumeration.Value that could not be transformed
case _:Ident | _:Select | _:Apply if jsdefn.scalaEnumeration.isValueMethodNoName(tree.symbol) =>
report.warning(
"Could not transform call to scala.Enumeration.Value.\n" +
"The resulting program is unlikely to function properly as this operation requires reflection.",
tree)
super.transform(tree)

// Warnings for scala.Enumeration.Value with a `null` name
case Apply(_, args) if jsdefn.scalaEnumeration.isValueMethodName(tree.symbol) && isNullLiteral(args.last) =>
report.warning(
"Passing null as name to scala.Enumeration.Value requires reflection at run-time.\n" +
"The resulting program is unlikely to function properly.",
tree)
super.transform(tree)

// Warnings for scala.Enumeration.Val without name
case _: Apply if jsdefn.scalaEnumeration.isValCtorNoName(tree.symbol) =>
report.warning(
"Calls to the non-string constructors of scala.Enumeration.Val require reflection at run-time.\n" +
"The resulting program is unlikely to function properly.",
tree)
super.transform(tree)

// Warnings for scala.Enumeration.Val with a `null` name
case Apply(_, args) if jsdefn.scalaEnumeration.isValCtorName(tree.symbol) && isNullLiteral(args.last) =>
report.warning(
"Passing null as name to a constructor of scala.Enumeration.Val requires reflection at run-time.\n" +
"The resulting program is unlikely to function properly.",
tree)
super.transform(tree)

case _: Export =>
if enclosingOwner is OwnerKind.JSNative then
report.error("Native JS traits, classes and objects cannot contain exported definitions.", tree)
Expand All @@ -335,6 +372,10 @@ class PrepJSInterop extends MacroTransform with IdentityDenotTransformer { thisP
}
}

private def isNullLiteral(tree: Tree): Boolean = tree match
case Literal(Constant(null)) => true
case _ => false

private def validateJSConstructorOf(tree: Tree, tpeArg: Tree)(using Context): Unit = {
val tpe = checkClassType(tpeArg.tpe, tpeArg.srcPos, traitReq = false, stablePrefixReq = false)

Expand Down Expand Up @@ -625,6 +666,50 @@ class PrepJSInterop extends MacroTransform with IdentityDenotTransformer { thisP
}
}

/** Transforms a non-`@js.native` ValDef or DefDef in a Scala class. */
private def transformScalaValOrDefDef(tree: ValOrDefDef)(using Context): Tree = {
tree match {
// Catch ValDefs in enumerations with simple calls to Value
case vd: ValDef
if (enclosingOwner is OwnerKind.Enum) && jsdefn.scalaEnumeration.isValueMethodNoName(vd.rhs.symbol) =>
val enumDefn = jsdefn.scalaEnumeration

// Extract the Int argument if it is present
val optIntArg = vd.rhs match {
case _:Select | _:Ident => None
case Apply(_, intArg :: Nil) => Some(intArg)
}

val defaultName = vd.name.getterName.encode.toString

/* Construct the following tree
*
* if (nextName != null && nextName.hasNext)
* nextName.next()
* else
* <defaultName>
*/
val thisClass = vd.symbol.owner.asClass
val nextNameTree = This(thisClass).select(enumDefn.Enumeration_nextName)
val nullCompTree = nextNameTree.select(nme.NE).appliedTo(Literal(Constant(null)))
val hasNextTree = nextNameTree.select(enumDefn.hasNext)
val condTree = nullCompTree.select(nme.ZAND).appliedTo(hasNextTree)
val nameTree = If(condTree, nextNameTree.select(enumDefn.next).appliedToNone, Literal(Constant(defaultName)))

val newRhs = optIntArg match {
case None =>
This(thisClass).select(enumDefn.Enumeration_Value_StringArg).appliedTo(nameTree)
case Some(intArg) =>
This(thisClass).select(enumDefn.Enumeration_Value_IntStringArg).appliedTo(intArg, nameTree)
}

cpy.ValDef(vd)(rhs = newRhs)

case _ =>
super.transform(tree)
}
}

/** Verify a ValOrDefDef that is annotated with `@js.native`. */
private def transformJSNativeValOrDefDef(tree: ValOrDefDef)(using Context): ValOrDefDef = {
val sym = tree.symbol
Expand Down Expand Up @@ -1055,9 +1140,9 @@ object PrepJSInterop {
// Base kinds - those form a partition of all possible enclosing owners

/** A Scala class/trait. */
val ScalaClass = new OwnerKind(0x01)
val NonEnumScalaClass = new OwnerKind(0x01)
/** A Scala object. */
val ScalaMod = new OwnerKind(0x02)
val NonEnumScalaMod = new OwnerKind(0x02)
/** A native JS class/trait, which extends js.Any. */
val JSNativeClass = new OwnerKind(0x04)
/** A native JS object, which extends js.Any. */
Expand All @@ -1068,12 +1153,26 @@ object PrepJSInterop {
val JSTrait = new OwnerKind(0x20)
/** A non-native JS object. */
val JSMod = new OwnerKind(0x40)
/** A Scala class/trait that extends Enumeration. */
val EnumClass = new OwnerKind(0x80)
/** A Scala object that extends Enumeration. */
val EnumMod = new OwnerKind(0x100)
/** The Enumeration class itself. */
val EnumImpl = new OwnerKind(0x200)

// Compound kinds

/** A Scala class/trait, possibly Enumeration-related. */
val ScalaClass = NonEnumScalaClass | EnumClass | EnumImpl
/** A Scala object, possibly Enumeration-related. */
val ScalaMod = NonEnumScalaMod | EnumMod

/** A Scala class, trait or object, i.e., anything not extending js.Any. */
val ScalaType = ScalaClass | ScalaMod

/** A Scala class/trait/object extending Enumeration, but not Enumeration itself. */
val Enum = EnumClass | EnumMod

/** A native JS class/trait/object. */
val JSNative = JSNativeClass | JSNativeMod
/** A non-native JS class/trait/object. */
Expand Down
1 change: 0 additions & 1 deletion project/Build.scala
Original file line number Diff line number Diff line change
Expand Up @@ -1241,7 +1241,6 @@ object Build {
(
(dir / "shared/src/test/scala" ** (("*.scala": FileFilter)
-- "ReflectiveCallTest.scala" // uses many forms of structural calls that are not allowed in Scala 3 anymore
-- "EnumerationTest.scala" // scala.Enumeration support for Scala.js is not implemented in scalac (yet)
)).get

++ (dir / "shared/src/test/require-sam" ** "*.scala").get
Expand Down
60 changes: 60 additions & 0 deletions tests/neg-scalajs/enumeration-warnings.check
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
-- Error: tests/neg-scalajs/enumeration-warnings.scala:6:4 -------------------------------------------------------------
6 | Value // error
| ^^^^^
| Could not transform call to scala.Enumeration.Value.
| The resulting program is unlikely to function properly as this operation requires reflection.
-- Error: tests/neg-scalajs/enumeration-warnings.scala:10:9 ------------------------------------------------------------
10 | Value(4) // error
| ^^^^^^^^
| Could not transform call to scala.Enumeration.Value.
| The resulting program is unlikely to function properly as this operation requires reflection.
-- Error: tests/neg-scalajs/enumeration-warnings.scala:15:15 -----------------------------------------------------------
15 | val a = Value(null) // error
| ^^^^^^^^^^^
| Passing null as name to scala.Enumeration.Value requires reflection at run-time.
| The resulting program is unlikely to function properly.
-- Error: tests/neg-scalajs/enumeration-warnings.scala:16:15 -----------------------------------------------------------
16 | val b = Value(10, null) // error
| ^^^^^^^^^^^^^^^
| Passing null as name to scala.Enumeration.Value requires reflection at run-time.
| The resulting program is unlikely to function properly.
-- Error: tests/neg-scalajs/enumeration-warnings.scala:20:10 -----------------------------------------------------------
20 | val a = new Val // error
| ^^^^^^^
| Calls to the non-string constructors of scala.Enumeration.Val require reflection at run-time.
| The resulting program is unlikely to function properly.
-- Error: tests/neg-scalajs/enumeration-warnings.scala:21:10 -----------------------------------------------------------
21 | val b = new Val(10) // error
| ^^^^^^^^^^^
| Calls to the non-string constructors of scala.Enumeration.Val require reflection at run-time.
| The resulting program is unlikely to function properly.
-- Error: tests/neg-scalajs/enumeration-warnings.scala:25:10 -----------------------------------------------------------
25 | val a = new Val(null) // error
| ^^^^^^^^^^^^^
| Passing null as name to a constructor of scala.Enumeration.Val requires reflection at run-time.
| The resulting program is unlikely to function properly.
-- Error: tests/neg-scalajs/enumeration-warnings.scala:26:10 -----------------------------------------------------------
26 | val b = new Val(10, null) // error
| ^^^^^^^^^^^^^^^^^
| Passing null as name to a constructor of scala.Enumeration.Val requires reflection at run-time.
| The resulting program is unlikely to function properly.
-- Error: tests/neg-scalajs/enumeration-warnings.scala:30:31 -----------------------------------------------------------
30 | protected class Val1 extends Val // error
| ^^^
| Calls to the non-string constructors of scala.Enumeration.Val require reflection at run-time.
| The resulting program is unlikely to function properly.
-- Error: tests/neg-scalajs/enumeration-warnings.scala:31:31 -----------------------------------------------------------
31 | protected class Val2 extends Val(1) // error
| ^^^^^^
| Calls to the non-string constructors of scala.Enumeration.Val require reflection at run-time.
| The resulting program is unlikely to function properly.
-- Error: tests/neg-scalajs/enumeration-warnings.scala:35:31 -----------------------------------------------------------
35 | protected class Val1 extends Val(null) // error
| ^^^^^^^^^
| Passing null as name to a constructor of scala.Enumeration.Val requires reflection at run-time.
| The resulting program is unlikely to function properly.
-- Error: tests/neg-scalajs/enumeration-warnings.scala:36:31 -----------------------------------------------------------
36 | protected class Val2 extends Val(1, null) // error
| ^^^^^^^^^^^^
| Passing null as name to a constructor of scala.Enumeration.Val requires reflection at run-time.
| The resulting program is unlikely to function properly.
37 changes: 37 additions & 0 deletions tests/neg-scalajs/enumeration-warnings.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
// scalac: -Xfatal-warnings

class UnableToTransformValue extends Enumeration {
val a = {
println("oh, oh!")
Value // error
}
val b = {
println("oh, oh!")
Value(4) // error
}
}

class ValueWithNullName extends Enumeration {
val a = Value(null) // error
val b = Value(10, null) // error
}

class NewValWithNoName extends Enumeration {
val a = new Val // error
val b = new Val(10) // error
}

class NewValWithNullName extends Enumeration {
val a = new Val(null) // error
val b = new Val(10, null) // error
}

class ExtendsValWithNoName extends Enumeration {
protected class Val1 extends Val // error
protected class Val2 extends Val(1) // error
}

class ExtendsValWithNullName extends Enumeration {
protected class Val1 extends Val(null) // error
protected class Val2 extends Val(1, null) // error
}

0 comments on commit b419bca

Please sign in to comment.