Skip to content

Fix #17344: Make implicit references to this above dynamic imports explicit. #17357

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 32 additions & 1 deletion compiler/src/dotty/tools/dotc/transform/sjs/PrepJSInterop.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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]]

Expand Down Expand Up @@ -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)
}
Expand All @@ -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 ||
Expand Down
12 changes: 12 additions & 0 deletions project/Build.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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
(
Expand Down
Original file line number Diff line number Diff line change
@@ -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)
}
}
}