prevent exports in JS non-native traits

Author: bishabosha

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

@@ -105,8 +105,6 @@ final class JSDefinitions()(using Context) {
   @threadUnsafe lazy val ExposedJSMemberAnnotType: TypeRef = requiredClassRef("scala.scalajs.js.annotation.internal.ExposedJSMember")
   def ExposedJSMemberAnnot(using Context) = ExposedJSMemberAnnotType.symbol.asClass
 
-  def JSAnnotInternalPackage(using Context) = JSTypeAnnot.owner.asClass
-
   @threadUnsafe lazy val JSImportNamespaceModuleRef = requiredModuleRef("scala.scalajs.js.annotation.JSImport.Namespace")
   def JSImportNamespaceModule(using Context) = JSImportNamespaceModuleRef.symbol
 

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

@@ -85,7 +85,7 @@ class PrepJSInterop extends MacroTransform with IdentityDenotTransformer { thisP
     private def anyEnclosingOwner: OwnerKind = allEnclosingOwners
 
     private def enterOwner[A](kind: OwnerKind)(body: => A): A = {
-      require(kind.isBaseKind, kind)
+      require(kind.withoutMods.isBaseKind, kind.withoutMods)
       val oldEnclosingOwner = enclosingOwner
       val oldAllEnclosingOwners = allEnclosingOwners
       enclosingOwner = kind
@@ -302,8 +302,10 @@ class PrepJSInterop extends MacroTransform with IdentityDenotTransformer { thisP
           super.transform(tree)
 
         case _: Export =>
-          if anyEnclosingOwner is OwnerKind.JSNative then
+          if enclosingOwner is OwnerKind.JSNative then
             report.error("Native JS traits, classes and objects cannot contain exported definitions.", tree)
+          else if enclosingOwner is OwnerKind.WithJSTrait then
+            report.error("Non-native JS traits cannot contain exported definitions.", tree)
 
           super.transform(tree)
 
@@ -465,6 +467,7 @@ class PrepJSInterop extends MacroTransform with IdentityDenotTransformer { thisP
       val kind = {
         if (!isJSNative) {
           if (sym.is(ModuleClass)) OwnerKind.JSMod
+          else if (sym.is(Trait)) OwnerKind.JSClass | OwnerKind.WithJSTrait
           else OwnerKind.JSClass
         } else {
           if (sym.is(ModuleClass)) OwnerKind.JSNativeMod
@@ -825,38 +828,38 @@ class PrepJSInterop extends MacroTransform with IdentityDenotTransformer { thisP
     /** Removes annotations from exported definitions (e.g. `export foo.bar`):
      *  - `js.native`
      *  - `js.annotation.*`
-     *  - `js.annotation.internal.*`
      */
     private def stripJSAnnotsOnExported(sym: Symbol)(using Context): Unit =
       if !sym.is(Exported) then return
 
       val JSNativeAnnot = jsdefn.JSNativeAnnot
       val JSAnnotPackage = jsdefn.JSAnnotPackage
-      val JSAnnotInternalPackage = jsdefn.JSAnnotInternalPackage
 
       extension (sym: Symbol) def isJSAnnot =
-        (sym eq JSNativeAnnot) || (sym.owner eq JSAnnotPackage) || (sym.owner eq JSAnnotInternalPackage)
+        (sym eq JSNativeAnnot) || (sym.owner eq JSAnnotPackage)
 
       val newAnnots = sym.annotations.filterConserve(!_.symbol.isJSAnnot)
       if newAnnots ne sym.annotations then
         sym.annotations = newAnnots
     end stripJSAnnotsOnExported
 
     private def checkRHSCallsJSNative(tree: ValOrDefDef, longKindStr: String)(using Context): Unit = {
-      if !tree.symbol.is(Exported) then
-        // Check that the rhs is exactly `= js.native`
-        tree.rhs match {
-          case sel: Select if sel.symbol == jsdefn.JSPackage_native =>
-            // ok
-          case _ =>
-            val pos = if (tree.rhs != EmptyTree) tree.rhs.srcPos else tree.srcPos
-            report.error(s"$longKindStr may only call js.native.", pos)
-        }
+      if tree.symbol.is(Exported) then
+        return // we already report an error that exports are not allowed here, this prevents extra errors.
+
+      // Check that the rhs is exactly `= js.native`
+      tree.rhs match {
+        case sel: Select if sel.symbol == jsdefn.JSPackage_native =>
+          // ok
+        case _ =>
+          val pos = if (tree.rhs != EmptyTree) tree.rhs.srcPos else tree.srcPos
+          report.error(s"$longKindStr may only call js.native.", pos)
+      }
 
-        // Check that the resul type was explicitly specified
-        // (This is stronger than Scala 2, which only warns, and only if it was inferred as Nothing.)
-        if (tree.tpt.span.isSynthetic)
-          report.error(i"The type of ${tree.name} must be explicitly specified because it is JS native.", tree)
+      // Check that the resul type was explicitly specified
+      // (This is stronger than Scala 2, which only warns, and only if it was inferred as Nothing.)
+      if (tree.tpt.span.isSynthetic)
+        report.error(i"The type of ${tree.name} must be explicitly specified because it is JS native.", tree)
     }
 
     private def checkJSNativeSpecificAnnotsOnNonJSNative(memberDef: MemberDef)(using Context): Unit = {
@@ -996,6 +999,9 @@ object PrepJSInterop {
     inline def isBaseKind: Boolean =
       Integer.lowestOneBit(baseKinds) == baseKinds && baseKinds != 0 // exactly 1 bit on
 
+    @inline def withoutMods: OwnerKind =
+      new OwnerKind(this.baseKinds & ~ModifierMask.baseKinds)
+
     // cannot be `inline` because it accesses the private constructor
     @inline def |(that: OwnerKind): OwnerKind =
       new OwnerKind(this.baseKinds | that.baseKinds)
@@ -1026,6 +1032,11 @@ object PrepJSInterop {
     /** A non-native JS object. */
     val JSMod = new OwnerKind(0x20)
 
+    // Modifiers - these add information to a base kind
+
+    /** A modifer for `JSClass` to signal a non-native JS trait */
+    val WithJSTrait = new OwnerKind(0x1000)
+
     // Compound kinds
 
     /** A Scala class, trait or object, i.e., anything not extending js.Any. */
@@ -1038,6 +1049,9 @@ object PrepJSInterop {
     /** A JS type, i.e., something extending js.Any. */
     val JSType = JSNative | JSNonNative
 
+    /** A mask of all OwnerKind modifiers */
+    private val ModifierMask = WithJSTrait
+
     /** Any kind of class/trait, i.e., a Scala or JS class/trait. */
     val AnyClass = ScalaClass | JSNativeClass | JSClass
   }

tests/neg-scalajs/js-native-exports.check

@@ -1,12 +1,16 @@
--- Error: tests/neg-scalajs/js-native-exports.scala:15:11 --------------------------------------------------------------
-15 |    export bag.{str, int} // error
-   |    ^^^^^^^^^^^^^^^^^^^^^
+-- Error: tests/neg-scalajs/js-native-exports.scala:17:11 --------------------------------------------------------------
+17 |    export bag.{str, int, bool, dbl} // error
+   |    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |    Native JS traits, classes and objects cannot contain exported definitions.
--- Error: tests/neg-scalajs/js-native-exports.scala:21:11 --------------------------------------------------------------
-21 |    export bag.{str, int} // error
-   |    ^^^^^^^^^^^^^^^^^^^^^
+-- Error: tests/neg-scalajs/js-native-exports.scala:23:11 --------------------------------------------------------------
+23 |    export bag.{str, int, bool, dbl} // error
+   |    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |    Native JS traits, classes and objects cannot contain exported definitions.
--- Error: tests/neg-scalajs/js-native-exports.scala:28:11 --------------------------------------------------------------
-28 |    export bag.{str, int} // error
-   |    ^^^^^^^^^^^^^^^^^^^^^
+-- Error: tests/neg-scalajs/js-native-exports.scala:30:11 --------------------------------------------------------------
+30 |    export bag.{str, int, bool, dbl} // error
+   |    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |    Native JS traits, classes and objects cannot contain exported definitions.
+-- Error: tests/neg-scalajs/js-native-exports.scala:35:11 --------------------------------------------------------------
+35 |    export bag.{str, int, bool, dbl} // error
+   |    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |    Non-native JS traits cannot contain exported definitions.

tests/neg-scalajs/js-native-exports.scala

@@ -6,26 +6,33 @@ object A {
   @js.native
   trait Bag extends js.Any {
     val str: String
-    val int: Int
+    def int: Int
+    def bool(): Boolean
+    def dbl(dbl: Double): Double
   }
 
   @js.native
   @JSGlobal("BagHolder_GlobalClass")
   final class BagHolder(val bag: Bag) extends js.Object {
-    export bag.{str, int} // error
+    export bag.{str, int, bool, dbl} // error
   }
 
   @js.native
   trait BagHolderTrait extends js.Any {
     val bag: Bag
-    export bag.{str, int} // error
+    export bag.{str, int, bool, dbl} // error
   }
 
   @js.native
   @JSGlobal("BagHolderModule_GlobalVar")
   object BagHolderModule extends js.Object {
     val bag: Bag = js.native
-    export bag.{str, int} // error
+    export bag.{str, int, bool, dbl} // error
+  }
+
+  trait NonNativeBagHolderTrait extends js.Any {
+    val bag: Bag
+    export bag.{str, int, bool, dbl} // error
   }
 
 }

tests/sjs-junit/test/org/scalajs/testsuite/jsinterop/ExportedJSNativeMembersScala3.scala

@@ -48,9 +48,25 @@ object ExportedJSNativeMembersScala3:
     @JSGlobal("quxxInstance_GlobalThatWillBeExported")
     val quxxInstance: QuxHolderHolderTrait = js.native
 
+    @js.native
+    @JSGlobal("addOne_GlobalThatWillBeExported")
+    def addOne(i: Int): Int = js.native
+
+  }
+
+  object B extends js.Object {
+    export A.FooModule            // trait  (native)
+    export A.Foo                  // val    (native)
+    export A.Bar                  // object (native)
+    export A.Baz                  // class  (native)
+    export A.QuxHolder            // class  (native)
+    export A.QuxHolderHolder      // class  (native)
+    export A.QuxHolderHolderTrait // trait  (native)
+    export A.quxxInstance         // val    (native)
+    export A.addOne               // def    (native)
   }
 
-  object B {
+  final class C extends js.Object {
     export A.FooModule            // trait  (native)
     export A.Foo                  // val    (native)
     export A.Bar                  // object (native)
@@ -59,6 +75,7 @@ object ExportedJSNativeMembersScala3:
     export A.QuxHolderHolder      // class  (native)
     export A.QuxHolderHolderTrait // trait  (native)
     export A.quxxInstance         // val    (native)
+    export A.addOne               // def    (native)
   }
 
 class ExportedJSNativeMembersScala3:
@@ -87,12 +104,20 @@ class ExportedJSNativeMembersScala3:
           new QuxHolder_GlobalThatWillBeExported("quxxInstance")
         )
       )
+      function addOne_GlobalThatWillBeExported(i) {
+        return i + 1;
+      }
     """)
+
+    val C = ExportedJSNativeMembersScala3.C()
+
     assertEquals("foo", A.Foo.foo)
     assertEquals("foo", B.Foo.foo)
+    assertEquals("foo", C.Foo.foo)
 
     assertEquals(23, A.Bar.bar)
     assertEquals(23, B.Bar.bar)
+    assertEquals(23, C.Bar.bar)
 
     val abaz = A.Baz("abaz1")
     assertEquals("abaz1", abaz.baz)
@@ -104,6 +129,11 @@ class ExportedJSNativeMembersScala3:
     bbaz.baz = "bbaz2"
     assertEquals("bbaz2", bbaz.baz)
 
+    val cbaz = C.Baz("cbaz1")
+    assertEquals("cbaz1", cbaz.baz)
+    cbaz.baz = "cbaz2"
+    assertEquals("cbaz2", cbaz.baz)
+
     val quxHolderHolderA = A.QuxHolderHolder(A.QuxHolder("quxHolderHolderA"))
     assertEquals("quxHolderHolderA", quxHolderHolderA.qux)
     assertEquals("quxHolderHolderA", quxHolderHolderA.quxHolder.qux)
@@ -112,10 +142,20 @@ class ExportedJSNativeMembersScala3:
     assertEquals("quxHolderHolderB", quxHolderHolderB.qux)
     assertEquals("quxHolderHolderB", quxHolderHolderB.quxHolder.qux)
 
+    val quxHolderHolderC = C.QuxHolderHolder(C.QuxHolder("quxHolderHolderC"))
+    assertEquals("quxHolderHolderC", quxHolderHolderC.qux)
+    assertEquals("quxHolderHolderC", quxHolderHolderC.quxHolder.qux)
+
     assertEquals("quxxInstance", A.quxxInstance.qux)
     assertEquals("quxxInstance", A.quxxInstance.quxHolder.qux)
     assertEquals("quxxInstance", B.quxxInstance.qux)
     assertEquals("quxxInstance", B.quxxInstance.quxHolder.qux)
+    assertEquals("quxxInstance", C.quxxInstance.qux)
+    assertEquals("quxxInstance", C.quxxInstance.quxHolder.qux)
+
+    assertEquals(2, A.addOne(1))
+    assertEquals(3, B.addOne(2))
+    assertEquals(4, C.addOne(3))
   }
 
 end ExportedJSNativeMembersScala3