Fix scala-js#4684: Handle Unit parameters with default values in JS types.

Both at call site (for native and non-native JS types) and at
definition sites (for non-native JS types).

Author: sjrd

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

@@ -3857,8 +3857,14 @@ abstract class GenJSCode[G <: Global with Singleton](val global: G)
       val Block(stats, expr) = tree
 
       val genStatsAndExpr = if (!stats.exists(isCaseLabelDef(_))) {
-        // fast path
-        stats.map(genStat(_)) :+ genStatOrExpr(expr, isStat)
+        // #4684 Collapse { <undefined-param>; BoxedUnit } to <undefined-param>
+        val genStatsAndExpr0 = stats.map(genStat(_)) :+ genStatOrExpr(expr, isStat)
+        genStatsAndExpr0 match {
+          case (undefParam @ js.Transient(UndefinedParam)) :: js.Undefined() :: Nil =>
+            undefParam :: Nil
+          case _ =>
+            genStatsAndExpr0
+        }
       } else {
         genBlockWithCaseLabelDefs(stats :+ expr, isStat)
       }

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

@@ -780,7 +780,7 @@ trait GenJSExports[G <: Global with Singleton] extends SubComponent {
       // Pass previous arguments to defaultGetter
       val defaultGetterArgs = previousArgsValues(defaultGetter.tpe.params.size)
 
-      if (isJSType(trgSym)) {
+      val callGetter = if (isJSType(trgSym)) {
         if (isNonNativeJSClass(defaultGetter.owner)) {
           if (defaultGetter.hasAnnotation(JSOptionalAnnotation))
             js.Undefined()
@@ -802,6 +802,12 @@ trait GenJSExports[G <: Global with Singleton] extends SubComponent {
       } else {
         genApplyMethod(trgTree, defaultGetter, defaultGetterArgs)
       }
+
+      // #4684 If the getter returns void, we must "box" it by returning undefined
+      if (callGetter.tpe == jstpe.NoType)
+        js.Block(callGetter, js.Undefined())
+      else
+        callGetter
     }
 
     /** Generate the final forwarding call to the exported method. */

test-suite/js/src/test/scala/org/scalajs/testsuite/compiler/InteroperabilityTest.scala

@@ -543,6 +543,10 @@ class InteroperabilityTest {
 
     assertJSArrayEquals[Any](js.Array(js.undefined, 1, 2, 3, 4), obj.multi()(1, 2, 3, 4)())
     assertJSArrayEquals[Any](js.Array(2, 5), obj.multi(2)()(5))
+
+    // #4684 Default params with Unit type
+    assertJSArrayEquals[Any](js.Array(()), obj.unitParam(()))
+    assertJSArrayEquals[Any](js.Array((), ()), obj.unitParam((), ()))
   }
 
   @Test def defaultParametersForConstructors_Issue791(): Unit = {
@@ -793,6 +797,8 @@ object InteroperabilityTest {
     def named(x: Int = 1, y: Int = 1, z: Int = 1): js.Array[Any] = js.native
     @JSName("fun")
     def multi(x: Int = 1)(ys: Int*)(z: Int = 1): js.Array[Any] = js.native
+    @JSName("fun")
+    def unitParam(x: Unit, y: Unit = ()): js.Array[Any] = js.native
   }
 
   @js.native

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

@@ -1055,15 +1055,21 @@ class NonNativeJSTypeTest {
 
   @Test def defaultParameters(): Unit = {
     class DefaultParameters extends js.Object {
+      var sideEffectCounter: Int = 0
+
       def bar(x: Int, y: Int = 1): Int = x + y
       def dependent(x: Int)(y: Int = x + 1): Int = x + y
+      def unitParam(x: Unit, y: Unit = { sideEffectCounter += 1; () }): Int = sideEffectCounter
 
       def foobar(x: Int): Int = bar(x)
     }
 
     object DefaultParametersMod extends js.Object {
+      var sideEffectCounter: Int = 0
+
       def bar(x: Int, y: Int = 1): Int = x + y
       def dependent(x: Int)(y: Int = x + 1): Int = x + y
+      def unitParam(x: Unit, y: Unit = { sideEffectCounter += 1; () }): Int = sideEffectCounter
 
       def foobar(x: Int): Int = bar(x)
     }
@@ -1075,12 +1081,26 @@ class NonNativeJSTypeTest {
     assertEquals(9, foo.dependent(4)(5))
     assertEquals(17, foo.dependent(8)())
 
+    // #4684 Default params with Unit type
+    assertEquals(0, foo.sideEffectCounter)
+    assertEquals(1, foo.unitParam(()))
+    assertEquals(1, foo.sideEffectCounter)
+    assertEquals(2, foo.unitParam((), ())) // an actual undefined param counts as not provided
+    assertEquals(2, foo.sideEffectCounter)
+
     assertEquals(9, DefaultParametersMod.bar(4, 5))
     assertEquals(5, DefaultParametersMod.bar(4))
     assertEquals(4, DefaultParametersMod.foobar(3))
     assertEquals(9, DefaultParametersMod.dependent(4)(5))
     assertEquals(17, DefaultParametersMod.dependent(8)())
 
+    // #4684 Default params with Unit type
+    assertEquals(0, DefaultParametersMod.sideEffectCounter)
+    assertEquals(1, DefaultParametersMod.unitParam(()))
+    assertEquals(1, DefaultParametersMod.sideEffectCounter)
+    assertEquals(2, DefaultParametersMod.unitParam((), ())) // an actual undefined param counts as not provided
+    assertEquals(2, DefaultParametersMod.sideEffectCounter)
+
     def testDyn(dyn: js.Dynamic): Unit = {
       assertEquals(9, dyn.bar(4, 5))
       assertEquals(5, dyn.bar(4))