From 7c96eed5fc8bd726480e005281e1a02608c5c0fb Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?S=C3=A9bastien=20Doeraene?= <sjrdoeraene@gmail.com>
Date: Wed, 26 Apr 2023 16:00:49 +0200
Subject: [PATCH] Fix #17344: Make implicit references to this above dynamic
 imports explicit.

Implicit references to the `this` of an outer class are made
explicit by the typer, and they need to be for `explicitOuter` to
do its job correctly.

When we desugar a `js.dynamicImport`, we move the code inside a
synthetic inner class. If it contains implicit references to an
enclosing class, we must make them explicit at that point.
---
 .../dotc/transform/sjs/PrepJSInterop.scala    | 33 ++++++++++++++++++-
 project/Build.scala                           | 12 +++++++
 .../SJSDynamicImportTestScala3.scala          | 29 ++++++++++++++++
 3 files changed, 73 insertions(+), 1 deletion(-)
 create mode 100644 tests/sjs-junit/test-require-multi-modules/org/scalajs/testsuite/jsinterop/SJSDynamicImportTestScala3.scala

diff --git a/compiler/src/dotty/tools/dotc/transform/sjs/PrepJSInterop.scala b/compiler/src/dotty/tools/dotc/transform/sjs/PrepJSInterop.scala
index 8a430991e378..a2f9a0fb45a3 100644
--- a/compiler/src/dotty/tools/dotc/transform/sjs/PrepJSInterop.scala
+++ b/compiler/src/dotty/tools/dotc/transform/sjs/PrepJSInterop.scala
@@ -93,6 +93,24 @@ class PrepJSInterop extends MacroTransform with IdentityDenotTransformer { thisP
       }
     }
 
+    private var dynamicImportEnclosingClasses: Set[Symbol] = Set.empty
+
+    private def enterDynamicImportEnclosingClass[A](cls: Symbol)(body: => A): A = {
+      val saved = dynamicImportEnclosingClasses
+      dynamicImportEnclosingClasses = saved + cls
+      try
+        body
+      finally
+        dynamicImportEnclosingClasses = saved
+    }
+
+    private def hasImplicitThisPrefixToDynamicImportEnclosingClass(tpe: Type)(using Context): Boolean =
+      tpe match
+        case tpe: ThisType      => dynamicImportEnclosingClasses.contains(tpe.cls)
+        case TermRef(prefix, _) => hasImplicitThisPrefixToDynamicImportEnclosingClass(prefix)
+        case _                  => false
+    end hasImplicitThisPrefixToDynamicImportEnclosingClass
+
     /** DefDefs in class templates that export methods to JavaScript */
     private val exporters = mutable.Map.empty[Symbol, mutable.ListBuffer[Tree]]
 
@@ -297,10 +315,15 @@ class PrepJSInterop extends MacroTransform with IdentityDenotTransformer { thisP
 
           assert(currentOwner.isTerm, s"unexpected owner: $currentOwner at ${tree.sourcePos}")
 
+          val enclosingClass = currentOwner.enclosingClass
+
           // new DynamicImportThunk { def apply(): Any = body }
           val dynamicImportThunkAnonClass = AnonClass(currentOwner, List(jsdefn.DynamicImportThunkType), span) { cls =>
             val applySym = newSymbol(cls, nme.apply, Method, MethodType(Nil, Nil, defn.AnyType), coord = span).entered
-            val newBody = transform(body).changeOwnerAfter(currentOwner, applySym, thisPhase)
+            val transformedBody = enterDynamicImportEnclosingClass(enclosingClass) {
+              transform(body)
+            }
+            val newBody = transformedBody.changeOwnerAfter(currentOwner, applySym, thisPhase)
             val applyDefDef = DefDef(applySym, newBody)
             List(applyDefDef)
           }
@@ -310,6 +333,14 @@ class PrepJSInterop extends MacroTransform with IdentityDenotTransformer { thisP
             .appliedToTypeTree(tpeArg)
             .appliedTo(dynamicImportThunkAnonClass)
 
+        // #17344 Make `ThisType`-based references to enclosing classes of `js.dynamicImport` explicit
+        case tree: Ident if hasImplicitThisPrefixToDynamicImportEnclosingClass(tree.tpe) =>
+          def rec(tpe: Type): Tree = (tpe: @unchecked) match // exhaustive because of the `if ... =>`
+            case tpe: ThisType            => This(tpe.cls)
+            case tpe @ TermRef(prefix, _) => rec(prefix).select(tpe.symbol)
+
+          rec(tree.tpe).withSpan(tree.span)
+
         // Compile-time errors and warnings for js.Dynamic.literal
         case Apply(Apply(fun, nameArgs), args)
             if fun.symbol == jsdefn.JSDynamicLiteral_applyDynamic ||
diff --git a/project/Build.scala b/project/Build.scala
index 97b4d31f5e02..a1245dfd8460 100644
--- a/project/Build.scala
+++ b/project/Build.scala
@@ -1219,6 +1219,18 @@ object Build {
         org.scalajs.jsenv.Input.Script(f) +: (Test / jsEnvInput).value
       },
 
+      Test / unmanagedSourceDirectories ++= {
+        val linkerConfig = scalaJSStage.value match {
+          case FastOptStage => (Test / fastLinkJS / scalaJSLinkerConfig).value
+          case FullOptStage => (Test / fullLinkJS / scalaJSLinkerConfig).value
+        }
+
+        if (linkerConfig.moduleKind != ModuleKind.NoModule && !linkerConfig.closureCompiler)
+          Seq(baseDirectory.value / "test-require-multi-modules")
+        else
+          Nil
+      },
+
       (Compile / managedSources) ++= {
         val dir = fetchScalaJSSource.value
         (
diff --git a/tests/sjs-junit/test-require-multi-modules/org/scalajs/testsuite/jsinterop/SJSDynamicImportTestScala3.scala b/tests/sjs-junit/test-require-multi-modules/org/scalajs/testsuite/jsinterop/SJSDynamicImportTestScala3.scala
new file mode 100644
index 000000000000..b7c9060fcce1
--- /dev/null
+++ b/tests/sjs-junit/test-require-multi-modules/org/scalajs/testsuite/jsinterop/SJSDynamicImportTestScala3.scala
@@ -0,0 +1,29 @@
+package org.scalajs.testsuite.jsinterop
+
+import org.junit.Assert.*
+import org.junit.Test
+
+import org.scalajs.junit.async._
+
+import scala.scalajs.js
+import scala.scalajs.js.annotation.*
+
+class SJSDynamicImportTestScala3 {
+  import scala.concurrent.ExecutionContext.Implicits.global
+
+  @Test def implicitThisReferenceInDynamicImport_Issue17344(): AsyncResult = await {
+    class Foo() {
+      def foo(): Int = 1
+    }
+    class Bar(foo: Foo) {
+      def bar(): js.Promise[Int] = js.dynamicImport(foo.foo())
+    }
+
+    val bar = new Bar(new Foo())
+    val promise = bar.bar()
+
+    promise.toFuture.map { r =>
+      assertEquals(1, r)
+    }
+  }
+}