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

@@ -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.JSTrait then
+            report.error("Non-native JS traits cannot contain exported definitions.", tree)
 
           super.transform(tree)
 
@@ -465,7 +467,8 @@ class PrepJSInterop extends MacroTransform with IdentityDenotTransformer { thisP
       val kind = {
         if (!isJSNative) {
           if (sym.is(ModuleClass)) OwnerKind.JSMod
-          else OwnerKind.JSClass
+          else if (sym.is(Trait)) OwnerKind.JSTrait
+          else OwnerKind.JSNonTraitClass
         } else {
           if (sym.is(ModuleClass)) OwnerKind.JSNativeMod
           else OwnerKind.JSNativeClass
@@ -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 = {
@@ -1021,10 +1024,12 @@ object PrepJSInterop {
     val JSNativeClass = new OwnerKind(0x04)
     /** A native JS object, which extends js.Any. */
     val JSNativeMod = new OwnerKind(0x08)
-    /** A non-native JS class/trait. */
-    val JSClass = new OwnerKind(0x10)
+    /** A non-native JS class (not a trait). */
+    val JSNonTraitClass = new OwnerKind(0x10)
+    /** A non-native JS trait. */
+    val JSTrait = new OwnerKind(0x20)
     /** A non-native JS object. */
-    val JSMod = new OwnerKind(0x20)
+    val JSMod = new OwnerKind(0x40)
 
     // Compound kinds
 
@@ -1034,12 +1039,12 @@ object PrepJSInterop {
     /** A native JS class/trait/object. */
     val JSNative = JSNativeClass | JSNativeMod
     /** A non-native JS class/trait/object. */
-    val JSNonNative = JSClass | JSMod
+    val JSNonNative = JSNonTraitClass | JSTrait | JSMod
     /** A JS type, i.e., something extending js.Any. */
     val JSType = JSNative | JSNonNative
 
     /** Any kind of class/trait, i.e., a Scala or JS class/trait. */
-    val AnyClass = ScalaClass | JSNativeClass | JSClass
+    val AnyClass = ScalaClass | JSNativeClass | JSNonTraitClass | JSTrait
   }
 
   /** Tests if the symbol extend `js.Any`.

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