Merge pull request #15770 from dotty-staging/sjs-enumeration

Author: dwijnand

community-build/community-projects/play-json

@@ -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._
@@ -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")

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

@@ -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)
       }
     }
 
@@ -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)
       }
@@ -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)
@@ -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)
 
@@ -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
@@ -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. */
@@ -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. */

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

@@ -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

project/Build.scala

@@ -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.

tests/neg-scalajs/enumeration-warnings.check

@@ -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
+}

tests/neg-scalajs/enumeration-warnings.scala

@@ -1,5 +1,3 @@
-// scalajs: --skip --pending
-
 object Q extends Enumeration {
   val A = Value("A")
   val B = Value("B")

tests/run/t1505.scala

@@ -1,5 +1,3 @@
-// scalajs: --skip --pending
-
 object Test extends App {
 
   object Color extends Enumeration {

tests/run/t2111.scala

@@ -1,5 +1,3 @@
-// scalajs: --skip --pending
-
 object X extends Enumeration {
     val Y = Value
 }

tests/run/t3616.scala

@@ -1,5 +1,3 @@
-// scalajs: --skip --pending
-
 object t extends Enumeration { val a, b = Value }
 
 object Test extends App {

tests/run/t3687.scala

@@ -1,4 +1,4 @@
-// scalajs: --skip --pending
+// scalajs: --skip
 
 object Days extends Enumeration {
   type Day = DayValue

tests/run/t3719.scala

@@ -1,5 +1,3 @@
-// scalajs: --skip --pending
-
 object Test extends Enumeration {
   val foo = Value
   def bar = withName("foo")

tests/run/t4570.scala

@@ -1,5 +1,3 @@
-// scalajs: --skip --pending
-
 object L extends Enumeration {
   val One, Two, Three = Value
 }