Move the computation of JS native load specs to the back-end.

This allows to remove `SJSPlatform.perRunInfo`.

We still check that they are valid during `PrepJSInterop`.

Author: sjrd

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

@@ -464,7 +464,7 @@ class JSCodeGen()(using genCtx: Context) {
     val superClass =
       if (sym.is(Trait)) None
       else Some(encodeClassNameIdent(sym.superClass))
-    val jsNativeLoadSpec = sjsPlatform.perRunInfo.jsNativeLoadSpecOfOption(sym)
+    val jsNativeLoadSpec = computeJSNativeLoadSpecOfClass(sym)
 
     js.ClassDef(
         classIdent,
@@ -3497,6 +3497,84 @@ class JSCodeGen()(using genCtx: Context) {
     }
   }
 
+  private def computeJSNativeLoadSpecOfClass(sym: Symbol): Option[js.JSNativeLoadSpec] = {
+    if (sym.is(Trait) || sym.hasAnnotation(jsdefn.JSGlobalScopeAnnot)) {
+      None
+    } else {
+      atPhase(picklerPhase.next) {
+        if (sym.owner.isStaticOwner)
+          Some(computeJSNativeLoadSpecOfInPhase(sym))
+        else
+          None
+      }
+    }
+  }
+
+  private def computeJSNativeLoadSpecOfInPhase(sym: Symbol)(using Context): js.JSNativeLoadSpec = {
+    import js.JSNativeLoadSpec._
+
+    val symOwner = sym.owner
+
+    // Marks a code path as unexpected because it should have been reported as an error in `PrepJSInterop`.
+    def unexpected(msg: String): Nothing =
+      throw new FatalError(i"$msg for ${sym.fullName} at ${sym.srcPos}")
+
+    if (symOwner.hasAnnotation(jsdefn.JSNativeAnnot)) {
+      val jsName = sym.jsName match {
+        case JSName.Literal(jsName) => jsName
+        case JSName.Computed(_)     => unexpected("could not read the simple JS name as a string literal")
+      }
+
+      if (symOwner.hasAnnotation(jsdefn.JSGlobalScopeAnnot)) {
+        Global(jsName, Nil)
+      } else {
+        val ownerLoadSpec = computeJSNativeLoadSpecOfInPhase(symOwner)
+        ownerLoadSpec match {
+          case Global(globalRef, path) =>
+            Global(globalRef, path :+ jsName)
+          case Import(module, path) =>
+            Import(module, path :+ jsName)
+          case ImportWithGlobalFallback(Import(module, modulePath), Global(globalRef, globalPath)) =>
+            ImportWithGlobalFallback(
+                Import(module, modulePath :+ jsName),
+                Global(globalRef, globalPath :+ jsName))
+        }
+      }
+    } else {
+      def parsePath(pathName: String): List[String] =
+        pathName.split('.').toList
+
+      def parseGlobalPath(pathName: String): Global = {
+        val globalRef :: path = parsePath(pathName)
+        Global(globalRef, path)
+      }
+
+      val annot = sym.annotations.find { annot =>
+        annot.symbol == jsdefn.JSGlobalAnnot || annot.symbol == jsdefn.JSImportAnnot
+      }.getOrElse {
+        unexpected("could not find the JS native load spec annotation")
+      }
+
+      if (annot.symbol == jsdefn.JSGlobalAnnot) {
+        val pathName = annot.argumentConstantString(0).getOrElse {
+          sym.defaultJSName
+        }
+        parseGlobalPath(pathName)
+      } else { // annot.symbol == jsdefn.JSImportAnnot
+        val module = annot.argumentConstantString(0).getOrElse {
+          unexpected("could not read the module argument as a string literal")
+        }
+        val path = annot.argumentConstantString(1).fold[List[String]](Nil)(parsePath)
+        val importSpec = Import(module, path)
+        annot.argumentConstantString(2).fold[js.JSNativeLoadSpec] {
+          importSpec
+        } { globalPathName =>
+          ImportWithGlobalFallback(importSpec, parseGlobalPath(globalPathName))
+        }
+      }
+    }
+  }
+
   private def isMethodStaticInIR(sym: Symbol): Boolean =
     sym.is(JavaStatic)
 

compiler/src/dotty/tools/dotc/config/SJSPlatform.scala

@@ -28,24 +28,4 @@ class SJSPlatform()(using Context) extends JavaPlatform {
 
   override def shouldReceiveJavaSerializationMethods(sym: ClassSymbol)(using Context): Boolean =
     !sym.isSubClass(jsDefinitions.JSAnyClass)
-
-  object perRunInfo {
-    private val jsNativeLoadSpecs = new MutableSymbolMap[JSNativeLoadSpec]
-
-    /** Clears all the info at the beginning of a run. */
-    def clear(): Unit =
-      jsNativeLoadSpecs.clear()
-
-    /** Stores the JS native load spec of a symbol for the current compilation run. */
-    def storeJSNativeLoadSpec(sym: Symbol, spec: JSNativeLoadSpec): Unit =
-      jsNativeLoadSpecs(sym) = spec
-
-    /** Gets the JS native load spec of a symbol in the current compilation run. */
-    def jsNativeLoadSpecOf(sym: Symbol): JSNativeLoadSpec =
-      jsNativeLoadSpecs(sym)
-
-    /** Gets the JS native load spec of a symbol in the current compilation run, if it has one. */
-    def jsNativeLoadSpecOfOption(sym: Symbol): Option[JSNativeLoadSpec] =
-      jsNativeLoadSpecs.get(sym)
-  }
 }

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

@@ -69,11 +69,6 @@ class PrepJSInterop extends MacroTransform with IdentityDenotTransformer { thisP
 
   override def changesMembers: Boolean = true // the phase adds export forwarders
 
-  override def runOn(units: List[CompilationUnit])(using Context): List[CompilationUnit] = {
-    sjsPlatform.perRunInfo.clear()
-    super.runOn(units)
-  }
-
   protected def newTransformer(using Context): Transformer =
     new ScalaJSPrepJSInteropTransformer
 
@@ -544,29 +539,17 @@ class PrepJSInterop extends MacroTransform with IdentityDenotTransformer { thisP
               report.error(i"Traits may not have an @${annotSym.name} annotation.", annot.tree)
           }
         } else {
-          /* Compute the loading spec now, before `flatten` destroys the name.
-           * We store it in a global map.
-           */
-          val optLoadSpec = checkAndComputeJSNativeLoadSpecOf(treePos, sym)
-          for (loadSpec <- optLoadSpec)
-            sjsPlatform.perRunInfo.storeJSNativeLoadSpec(sym, loadSpec)
+          checkJSNativeLoadSpecOf(treePos, sym)
         }
       }
     }
 
-    private def checkAndComputeJSNativeLoadSpecOf(pos: SrcPos, sym: Symbol)(
-        using Context): Option[JSNativeLoadSpec] = {
+    private def checkJSNativeLoadSpecOf(pos: SrcPos, sym: Symbol)(using Context): Unit = {
       import JSNativeLoadSpec._
 
-      def makeGlobalRefNativeLoadSpec(globalRef: String,
-          path: List[String]): Global = {
-        val validatedGlobalRef = if (!JSGlobalRef.isValidJSGlobalRefName(globalRef)) {
+      def checkGlobalRefName(globalRef: String): Unit = {
+        if (!JSGlobalRef.isValidJSGlobalRefName(globalRef))
           report.error(s"The name of a JS global variable must be a valid JS identifier (got '$globalRef')", pos)
-          "erroneous"
-        } else {
-          globalRef
-        }
-        Global(validatedGlobalRef, path)
       }
 
       if (enclosingOwner is OwnerKind.JSNative) {
@@ -593,86 +576,56 @@ class PrepJSInterop extends MacroTransform with IdentityDenotTransformer { thisP
             }
           }
 
-          val jsName = sym.jsName match {
-            case JSName.Literal(jsName) => jsName
-            case JSName.Computed(_)     => "<erroneous>" // compile error above
-          }
-
-          val ownerLoadSpec = sjsPlatform.perRunInfo.jsNativeLoadSpecOfOption(sym.owner)
-          val loadSpec = ownerLoadSpec match {
-            case None =>
-              // The owner is a JSGlobalScope
-              makeGlobalRefNativeLoadSpec(jsName, Nil)
-            case Some(Global(globalRef, path)) =>
-              Global(globalRef, path :+ jsName)
-            case Some(Import(module, path)) =>
-              Import(module, path :+ jsName)
-            case Some(ImportWithGlobalFallback(
-                Import(module, modulePath), Global(globalRef, globalPath))) =>
-              ImportWithGlobalFallback(
-                  Import(module, modulePath :+ jsName),
-                  Global(globalRef, globalPath :+ jsName))
+          if (sym.owner.hasAnnotation(jsdefn.JSGlobalScopeAnnot)) {
+            val jsName = sym.jsName match {
+              case JSName.Literal(jsName) =>
+                checkGlobalRefName(jsName)
+              case JSName.Computed(_) =>
+                () // compile error above
+            }
           }
-          Some(loadSpec)
-        } else {
-          None
         }
       } else {
-        def parsePath(pathName: String): List[String] =
-          pathName.split('.').toList
-
-        def parseGlobalPath(pathName: String): Global = {
-          val globalRef :: path = parsePath(pathName)
-          makeGlobalRefNativeLoadSpec(globalRef, path)
+        def firstElementOfPath(pathName: String): String = {
+          val dotIndex = pathName.indexOf('.')
+          if (dotIndex < 0) pathName
+          else pathName.substring(0, dotIndex)
         }
 
+        def checkGlobalRefPath(pathName: String): Unit =
+          checkGlobalRefName(firstElementOfPath(pathName))
+
         checkAndGetJSNativeLoadingSpecAnnotOf(pos, sym) match {
           case Some(annot) if annot.symbol == jsdefn.JSGlobalScopeAnnot =>
             if (!sym.is(Module)) {
               report.error(
                   "@JSGlobalScope can only be used on native JS objects (with @js.native).",
                   annot.tree)
             }
-            None
 
           case Some(annot) if annot.symbol == jsdefn.JSGlobalAnnot =>
             if (shouldCheckLiterals)
               checkJSGlobalLiteral(annot)
             val pathName = annot.argumentConstantString(0).getOrElse {
-              val needsExplicitJSName = {
-                (enclosingOwner is OwnerKind.ScalaMod) &&
-                !sym.owner.isPackageObject
-              }
-
-              if (needsExplicitJSName) {
+              if ((enclosingOwner is OwnerKind.ScalaMod) && !sym.owner.isPackageObject) {
                 report.error(
                     "Native JS members inside non-native objects must have an explicit name in @JSGlobal",
                     annot.tree)
               }
               sym.defaultJSName
             }
-            Some(parseGlobalPath(pathName))
+            checkGlobalRefPath(pathName)
 
           case Some(annot) if annot.symbol == jsdefn.JSImportAnnot =>
             if (shouldCheckLiterals)
               checkJSImportLiteral(annot)
-            val module = annot.argumentConstantString(0).getOrElse {
-              "" // an error is reported by checkJSImportLiteral in this case
-            }
-            val path = annot.argumentConstantString(1).fold[List[String]](Nil)(parsePath)
-            val importSpec = Import(module, path)
-            val loadSpec = annot.argumentConstantString(2).fold[JSNativeLoadSpec] {
-              importSpec
-            } { globalPathName =>
-              ImportWithGlobalFallback(importSpec, parseGlobalPath(globalPathName))
+            annot.argumentConstantString(2).foreach { globalPathName =>
+              checkGlobalRefPath(globalPathName)
             }
-            Some(loadSpec)
 
           case _ =>
-            /* We already emitted an error. Invent something not to cause
-             * cascading errors.
-             */
-            Some(JSNativeLoadSpec.Global("erroneous", Nil))
+            // We already emitted an error in checkAndGetJSNativeLoadingSpecAnnotOf
+            ()
         }
       }
     }

project/Build.scala

@@ -1049,7 +1049,7 @@ object Build {
             -- "ExportsTest.scala" // JS exports
             -- "IterableTest.scala" // non-native JS classes
             -- "JSExportStaticTest.scala" // JS exports
-            -- "JSNativeInPackage.scala" // tests fail (wrong load spec for JS globals)
+            -- "JSNativeInPackage.scala" // #9785 tests fail due to js.typeOf(globalVar) being incorrect
             -- "JSOptionalTest.scala" // non-native JS classes
             -- "JSSymbolTest.scala" // non-native JS classes
             -- "MiscInteropTest.scala" // non-native JS classes