Skip to content

Commit f128063

Browse files
Fix #17344: Make implicit references to this above dynamic imports explicit. (#17357)
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.
2 parents bae3a33 + 7c96eed commit f128063

File tree

3 files changed

+73
-1
lines changed

3 files changed

+73
-1
lines changed

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

Lines changed: 32 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -93,6 +93,24 @@ class PrepJSInterop extends MacroTransform with IdentityDenotTransformer { thisP
9393
}
9494
}
9595

96+
private var dynamicImportEnclosingClasses: Set[Symbol] = Set.empty
97+
98+
private def enterDynamicImportEnclosingClass[A](cls: Symbol)(body: => A): A = {
99+
val saved = dynamicImportEnclosingClasses
100+
dynamicImportEnclosingClasses = saved + cls
101+
try
102+
body
103+
finally
104+
dynamicImportEnclosingClasses = saved
105+
}
106+
107+
private def hasImplicitThisPrefixToDynamicImportEnclosingClass(tpe: Type)(using Context): Boolean =
108+
tpe match
109+
case tpe: ThisType => dynamicImportEnclosingClasses.contains(tpe.cls)
110+
case TermRef(prefix, _) => hasImplicitThisPrefixToDynamicImportEnclosingClass(prefix)
111+
case _ => false
112+
end hasImplicitThisPrefixToDynamicImportEnclosingClass
113+
96114
/** DefDefs in class templates that export methods to JavaScript */
97115
private val exporters = mutable.Map.empty[Symbol, mutable.ListBuffer[Tree]]
98116

@@ -297,10 +315,15 @@ class PrepJSInterop extends MacroTransform with IdentityDenotTransformer { thisP
297315

298316
assert(currentOwner.isTerm, s"unexpected owner: $currentOwner at ${tree.sourcePos}")
299317

318+
val enclosingClass = currentOwner.enclosingClass
319+
300320
// new DynamicImportThunk { def apply(): Any = body }
301321
val dynamicImportThunkAnonClass = AnonClass(currentOwner, List(jsdefn.DynamicImportThunkType), span) { cls =>
302322
val applySym = newSymbol(cls, nme.apply, Method, MethodType(Nil, Nil, defn.AnyType), coord = span).entered
303-
val newBody = transform(body).changeOwnerAfter(currentOwner, applySym, thisPhase)
323+
val transformedBody = enterDynamicImportEnclosingClass(enclosingClass) {
324+
transform(body)
325+
}
326+
val newBody = transformedBody.changeOwnerAfter(currentOwner, applySym, thisPhase)
304327
val applyDefDef = DefDef(applySym, newBody)
305328
List(applyDefDef)
306329
}
@@ -310,6 +333,14 @@ class PrepJSInterop extends MacroTransform with IdentityDenotTransformer { thisP
310333
.appliedToTypeTree(tpeArg)
311334
.appliedTo(dynamicImportThunkAnonClass)
312335

336+
// #17344 Make `ThisType`-based references to enclosing classes of `js.dynamicImport` explicit
337+
case tree: Ident if hasImplicitThisPrefixToDynamicImportEnclosingClass(tree.tpe) =>
338+
def rec(tpe: Type): Tree = (tpe: @unchecked) match // exhaustive because of the `if ... =>`
339+
case tpe: ThisType => This(tpe.cls)
340+
case tpe @ TermRef(prefix, _) => rec(prefix).select(tpe.symbol)
341+
342+
rec(tree.tpe).withSpan(tree.span)
343+
313344
// Compile-time errors and warnings for js.Dynamic.literal
314345
case Apply(Apply(fun, nameArgs), args)
315346
if fun.symbol == jsdefn.JSDynamicLiteral_applyDynamic ||

project/Build.scala

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1219,6 +1219,18 @@ object Build {
12191219
org.scalajs.jsenv.Input.Script(f) +: (Test / jsEnvInput).value
12201220
},
12211221

1222+
Test / unmanagedSourceDirectories ++= {
1223+
val linkerConfig = scalaJSStage.value match {
1224+
case FastOptStage => (Test / fastLinkJS / scalaJSLinkerConfig).value
1225+
case FullOptStage => (Test / fullLinkJS / scalaJSLinkerConfig).value
1226+
}
1227+
1228+
if (linkerConfig.moduleKind != ModuleKind.NoModule && !linkerConfig.closureCompiler)
1229+
Seq(baseDirectory.value / "test-require-multi-modules")
1230+
else
1231+
Nil
1232+
},
1233+
12221234
(Compile / managedSources) ++= {
12231235
val dir = fetchScalaJSSource.value
12241236
(
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
package org.scalajs.testsuite.jsinterop
2+
3+
import org.junit.Assert.*
4+
import org.junit.Test
5+
6+
import org.scalajs.junit.async._
7+
8+
import scala.scalajs.js
9+
import scala.scalajs.js.annotation.*
10+
11+
class SJSDynamicImportTestScala3 {
12+
import scala.concurrent.ExecutionContext.Implicits.global
13+
14+
@Test def implicitThisReferenceInDynamicImport_Issue17344(): AsyncResult = await {
15+
class Foo() {
16+
def foo(): Int = 1
17+
}
18+
class Bar(foo: Foo) {
19+
def bar(): js.Promise[Int] = js.dynamicImport(foo.foo())
20+
}
21+
22+
val bar = new Bar(new Foo())
23+
val promise = bar.bar()
24+
25+
promise.toFuture.map { r =>
26+
assertEquals(1, r)
27+
}
28+
}
29+
}

0 commit comments

Comments
 (0)