Skip to content

Commit 491176f

Browse files
gzm0gzm0
committed
Fix #4465: Properly call default param getters for nested JS ctors
1 parent a73746b commit 491176f

File tree

3 files changed

+127
-15
lines changed

3 files changed

+127
-15
lines changed

‎compiler/src/main/scala/org/scalajs/nscplugin/GenJSCode.scala

Lines changed: 15 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1584,8 +1584,15 @@ abstract class GenJSCode[G <: Global with Singleton](val global: G)
15841584
ctorArgs, afterThisCall.result())
15851585
}
15861586

1587-
private def genJSClassCtorDispatch(sym: Symbol, allParams: List[Symbol],
1587+
private def genJSClassCtorDispatch(ctorSym: Symbol, allParamSyms: List[Symbol],
15881588
overloadNum: Int): (Exported, List[js.ParamDef]) = {
1589+
implicit val pos = ctorSym.pos
1590+
1591+
val allParamsAndInfos = for {
1592+
(paramSym, info) <- allParamSyms.zip(jsParamInfos(ctorSym))
1593+
} yield {
1594+
genVarRef(paramSym) -> info
1595+
}
15891596

15901597
/* `allParams` are the parameters as seen from *inside* the constructor
15911598
* body. the symbols returned in jsParamInfos are the parameters as seen
@@ -1595,7 +1602,7 @@ abstract class GenJSCode[G <: Global with Singleton](val global: G)
15951602
* identifiers (the ones generated by the trees in the constructor body).
15961603
*/
15971604
val (captureParamsAndInfos, normalParamsAndInfos) =
1598-
allParams.zip(jsParamInfos(sym)).partition(_._2.capture)
1605+
allParamsAndInfos.partition(_._2.capture)
15991606

16001607
/* We use the *outer* param symbol to get different names than the *inner*
16011608
* symbols. This is necessary so that we can forward captures properly
@@ -1606,23 +1613,22 @@ abstract class GenJSCode[G <: Global with Singleton](val global: G)
16061613

16071614
val normalInfos = normalParamsAndInfos.map(_._2).toIndexedSeq
16081615

1609-
val jsExport = new Exported(sym, normalInfos) {
1616+
val jsExport = new Exported(ctorSym, normalInfos) {
16101617
def genBody(formalArgsRegistry: FormalArgsRegistry): js.Tree = {
1611-
implicit val pos = sym.pos
1612-
16131618
val captureAssigns = for {
16141619
(param, info) <- captureParamsAndInfos
16151620
} yield {
1616-
js.Assign(genVarRef(param), genVarRef(info.sym))
1621+
js.Assign(param, genVarRef(info.sym))
16171622
}
16181623

16191624
val paramAssigns = for {
16201625
((param, info), i) <- normalParamsAndInfos.zipWithIndex
16211626
} yield {
1622-
val rhs = genScalaArg(sym, i, formalArgsRegistry, info, static = true)(
1623-
prevArgsCount => allParams.take(prevArgsCount).map(genVarRef(_)))
1627+
val rhs = genScalaArg(sym, i, formalArgsRegistry, info, static = true,
1628+
captures = captureParamsAndInfos.map(_._1))(
1629+
prevArgsCount => normalParamsAndInfos.take(prevArgsCount).map(_._1))
16241630

1625-
js.Assign(genVarRef(param), rhs)
1631+
js.Assign(param, rhs)
16261632
}
16271633

16281634
js.Block(captureAssigns ::: paramAssigns, js.IntLiteral(overloadNum))

‎compiler/src/main/scala/org/scalajs/nscplugin/GenJSExports.scala

Lines changed: 31 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -649,7 +649,7 @@ trait GenJSExports[G <: Global with Singleton] extends SubComponent {
649649
val varDefs = new mutable.ListBuffer[js.VarDef]
650650

651651
for ((param, i) <- jsParamInfos(sym).zipWithIndex) {
652-
val rhs = genScalaArg(sym, i, formalArgsRegistry, param, static)(
652+
val rhs = genScalaArg(sym, i, formalArgsRegistry, param, static, captures = Nil)(
653653
prevArgsCount => varDefs.take(prevArgsCount).toList.map(_.ref))
654654

655655
varDefs += js.VarDef(freshLocalIdent("prep" + i), NoOriginalName,
@@ -668,7 +668,8 @@ trait GenJSExports[G <: Global with Singleton] extends SubComponent {
668668
*/
669669
def genScalaArg(methodSym: Symbol, paramIndex: Int,
670670
formalArgsRegistry: FormalArgsRegistry, param: JSParamInfo,
671-
static: Boolean)(previousArgsValues: Int => List[js.Tree])(
671+
static: Boolean, captures: List[js.Tree])(
672+
previousArgsValues: Int => List[js.Tree])(
672673
implicit pos: Position): js.Tree = {
673674

674675
if (param.repeated) {
@@ -681,7 +682,7 @@ trait GenJSExports[G <: Global with Singleton] extends SubComponent {
681682
if (param.hasDefault) {
682683
// If argument is undefined and there is a default getter, call it
683684
val default = genCallDefaultGetter(methodSym, paramIndex,
684-
param.sym.pos, static)(previousArgsValues)
685+
param.sym.pos, static, captures)(previousArgsValues)
685686
js.If(js.BinaryOp(js.BinaryOp.===, jsArg, js.Undefined()),
686687
default, unboxedArg)(unboxedArg.tpe)
687688
} else {
@@ -692,7 +693,7 @@ trait GenJSExports[G <: Global with Singleton] extends SubComponent {
692693
}
693694

694695
private def genCallDefaultGetter(sym: Symbol, paramIndex: Int,
695-
paramPos: Position, static: Boolean)(
696+
paramPos: Position, static: Boolean, captures: List[js.Tree])(
696697
previousArgsValues: Int => List[js.Tree])(
697698
implicit pos: Position): js.Tree = {
698699

@@ -701,6 +702,10 @@ trait GenJSExports[G <: Global with Singleton] extends SubComponent {
701702
/* Get the companion module class.
702703
* For inner classes the sym.owner.companionModule can be broken,
703704
* therefore companionModule is fetched at uncurryPhase.
705+
*
706+
* #4465: If owner is a nested class, the linked class is *not* a
707+
* module value, but another class. In this case we need to call the
708+
* module accessor on the enclosing class to retrieve this.
704709
*/
705710
val companionModule = enteringPhase(currentRun.namerPhase) {
706711
sym.owner.companionModule
@@ -719,8 +724,28 @@ trait GenJSExports[G <: Global with Singleton] extends SubComponent {
719724
s"found overloaded default getter $defaultGetter")
720725

721726
val trgTree = {
722-
if (sym.isClassConstructor || static) genLoadModule(trgSym)
723-
else js.This()(encodeClassType(trgSym))
727+
if (sym.isClassConstructor || static) {
728+
if (!trgSym.isLifted) {
729+
assert(captures.isEmpty, "expected empty captures")
730+
genLoadModule(trgSym)
731+
} else {
732+
assert(captures.size == 1, "expected exactly one capture")
733+
734+
// Find the module accessor.
735+
val outer = trgSym.originalOwner
736+
val name = enteringPhase(currentRun.typerPhase)(trgSym.unexpandedName)
737+
738+
val modAccessor = outer.info.members.lookupModule(name)
739+
val receiver = captures.head
740+
if (isJSType(outer)) {
741+
genApplyJSClassMethod(receiver, modAccessor, Nil)
742+
} else {
743+
genApplyMethodMaybeStatically(receiver, modAccessor, Nil)
744+
}
745+
}
746+
} else {
747+
js.This()(encodeClassType(trgSym))
748+
}
724749
}
725750

726751
// Pass previous arguments to defaultGetter

‎test-suite/js/src/test/scala/org/scalajs/testsuite/jsinterop/NestedJSClassTest.scala

Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -425,6 +425,50 @@ class NestedJSClassTest {
425425
assertEquals("InnerJSClass(5) of issue 4086", obj.toString())
426426
}
427427

428+
@Test def defaultCtorParamsInnerJSClassScalaContainer_Issue4465(): Unit = {
429+
val container = new ScalaClassContainer("container")
430+
431+
val inner = new container.InnerJSClassDefaultParams_Issue4465()()
432+
assertEquals("container inner inner foo", inner.foo())
433+
434+
assertEquals(1, container.moduleSideEffect)
435+
436+
// Check that we do not create two companion modules.
437+
new container.InnerJSClassDefaultParams_Issue4465()()
438+
assertEquals(1, container.moduleSideEffect)
439+
}
440+
441+
@Test def defaultCtorParamsInnerJSClassTraitContainer_Issue4465(): Unit = {
442+
val container = new ScalaTraitContainerSubclass("container")
443+
444+
val inner = new container.InnerJSClassDefaultParams_Issue4465()()
445+
assertEquals("container inner inner foo", inner.foo())
446+
447+
assertEquals(1, container.moduleSideEffect)
448+
449+
// Check that we do not create two companion modules.
450+
new container.InnerJSClassDefaultParams_Issue4465()()
451+
assertEquals(1, container.moduleSideEffect)
452+
}
453+
454+
@Test def defaultCtorParamsInnerJSClassJSContainer_Issue4465(): Unit = {
455+
val container = new JSClassContainer("container")
456+
457+
// Typed
458+
val inner = new container.InnerJSClassDefaultParams_Issue4465()()
459+
assertEquals("container inner inner foo", inner.foo())
460+
461+
assertEquals(1, container.moduleSideEffect)
462+
463+
// Dynamic
464+
val dynContainer = container.asInstanceOf[js.Dynamic]
465+
val dynInner = js.Dynamic.newInstance(dynContainer.InnerJSClassDefaultParams_Issue4465)()
466+
assertEquals("container inner inner foo", dynInner.foo())
467+
468+
// Check that we do not create two companion modules.
469+
assertEquals(1, container.moduleSideEffect)
470+
}
471+
428472
@Test def doublyNestedInnerObject_Issue4114(): Unit = {
429473
val outer1 = new DoublyNestedInnerObject_Issue4114().asInstanceOf[js.Dynamic]
430474
val outer2 = new DoublyNestedInnerObject_Issue4114().asInstanceOf[js.Dynamic]
@@ -640,6 +684,18 @@ object NestedJSClassTest {
640684

641685
js.constructorOf[LocalJSClass]
642686
}
687+
688+
var moduleSideEffect = 0
689+
690+
class InnerJSClassDefaultParams_Issue4465(withDefault: String = "inner")(
691+
dependentDefault: String = withDefault) extends js.Object {
692+
def foo(methodDefault: String = "foo"): String =
693+
s"$xxx $withDefault $dependentDefault $methodDefault"
694+
}
695+
696+
object InnerJSClassDefaultParams_Issue4465 {
697+
moduleSideEffect += 1
698+
}
643699
}
644700

645701
trait ScalaTraitContainer {
@@ -663,6 +719,18 @@ object NestedJSClassTest {
663719

664720
js.constructorOf[LocalJSClass]
665721
}
722+
723+
var moduleSideEffect = 0
724+
725+
class InnerJSClassDefaultParams_Issue4465(withDefault: String = "inner")(
726+
dependentDefault: String = withDefault) extends js.Object {
727+
def foo(methodDefault: String = "foo"): String =
728+
s"$xxx $withDefault $dependentDefault $methodDefault"
729+
}
730+
731+
object InnerJSClassDefaultParams_Issue4465 {
732+
moduleSideEffect += 1
733+
}
666734
}
667735

668736
class ScalaTraitContainerSubclass(val xxx: String) extends ScalaTraitContainer
@@ -758,6 +826,19 @@ object NestedJSClassTest {
758826

759827
// Not visible from JS, but can be instantiated from Scala.js code
760828
class InnerScalaClass(val zzz: Int)
829+
830+
var moduleSideEffect = 0
831+
832+
class InnerJSClassDefaultParams_Issue4465(withDefault: String = "inner")(
833+
dependentDefault: String = withDefault) extends js.Object {
834+
def foo(methodDefault: String = "foo"): String =
835+
s"$xxx $withDefault $dependentDefault $methodDefault"
836+
}
837+
838+
@JSName("InnerJSClassDefaultParamsOtherName_Issue4465")
839+
object InnerJSClassDefaultParams_Issue4465 {
840+
moduleSideEffect += 1
841+
}
761842
}
762843

763844
class DoublyNestedInnerObject_Issue4114 extends js.Object {

0 commit comments

Comments
 (0)