Fix #4372: Generalize js.Function's to allow user-defined ones.

Previously, there was a fixed set of built-in JS function types,
`js.FunctionN` and `js.ThisFunctionN`, that represented JS function
values. The only way to create JS function values was to have a
lambda expression used as a SAM of these types.

In this commit, we generalize these to any SAM-able non-native JS
trait whose single abstract method is named `apply` and whose
super-*class* is `js.Function`. This allows the user to define
custom JS function types for signatures that are not made available
by the built-in types. A typical example is signatures with rest
parameters (i.e., varargs).

To do this, we make the following changes:

* We allow declaring an *abstract* `apply` method in a non-native
  JS trait if that makes that trait a SAM type with that method and
  if its superclass is `js.Function`.
  Even though we cannot in general implement `apply` methods, it is
  only the *concrete* ones that are problematic.
* We apply the JS `Closure` treatment in the back-end to any
  anonymous function class that is the result of desugaring a
  lambda for such a SAM type.
* In that treatment, we make a `function` function (as opposed to
  an arrow function) and use a `this` parameter if the anonymous
  function class inherits from js.ThisFunction.
* We add logic to handle repeated parameters in the treatment as
  well.

In addition, we remove the `@js.native` annotation from all the
"built-in" `js.FunctionN` and `js.ThisFunctionN` types. They lose
their prilileged status in the spec and in the compiler, to become
regular definitions that fit the above spec.

Author: sjrd

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

@@ -64,6 +64,8 @@ trait CompatComponent {
   def isImplClass(sym: Symbol): Boolean =
     scalaUsesImplClasses && sym.hasFlag(Flags.IMPLCLASS)
 
+  lazy val isScala211: Boolean = scalaUsesImplClasses
+
   implicit final class StdTermNamesCompat(self: global.nme.type) {
     def IMPL_CLASS_SUFFIX: String = noImplClasses()
 

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

@@ -127,7 +127,7 @@ abstract class ExplicitLocalJS[G <: Global with Singleton](val global: G)
 
   import global._
   import jsAddons._
-  import jsInterop.jsclassAccessorFor
+  import jsInterop.{jsclassAccessorFor, JSCallingConvention}
   import definitions._
   import rootMirror._
   import jsDefinitions._
@@ -171,8 +171,12 @@ abstract class ExplicitLocalJS[G <: Global with Singleton](val global: G)
 
   /** Is the given clazz a local JS class or object? */
   private def isLocalJSClassOrObject(clazz: Symbol): Boolean = {
-    def isJSLambda =
-      clazz.isAnonymousClass && AllJSFunctionClasses.exists(clazz.isSubClass(_))
+    def isJSLambda: Boolean = {
+      // See GenJSCode.isJSFunctionDef
+      clazz.isAnonymousClass &&
+      clazz.superClass == JSFunctionClass &&
+      clazz.info.decl(nme.apply).filter(JSCallingConvention.isCall(_)).exists
+    }
 
     clazz.isLocalToBlock &&
     !clazz.isTrait && clazz.hasAnnotation(JSTypeAnnot) &&

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

@@ -5796,6 +5796,8 @@ abstract class GenJSCode[G <: Global with Singleton](val global: G)
               if (name == "apply" || (ddsym.isSpecialized && name.startsWith("apply$"))) {
                 if ((applyDef eq null) || ddsym.isSpecialized)
                   applyDef = dd
+              } else if (ddsym.hasAnnotation(JSOptionalAnnotation)) {
+                // Ignore (this is useful for default parameters in custom JS function types)
               } else {
                 // Found a method we cannot encode in the rewriting
                 fail(s"Found a non-apply method $ddsym in $cd")
@@ -5847,15 +5849,31 @@ abstract class GenJSCode[G <: Global with Singleton](val global: G)
 
         // Fourth step: patch the body to unbox parameters and box result
 
+        val hasRepeatedParam = {
+          sym.superClass == JSFunctionClass && // Scala functions are known not to have repeated params
+          enteringUncurry {
+            applyDef.symbol.paramss.flatten.lastOption.exists(isRepeated(_))
+          }
+        }
+
         val js.MethodDef(_, _, _, params, _, body) = applyMethod
-        val (patchedParams, patchedBody) =
-          patchFunBodyWithBoxes(applyDef.symbol, params, body.get)
+        val (patchedParams, patchedBody) = {
+          patchFunBodyWithBoxes(applyDef.symbol, params, body.get,
+              useParamsBeforeLambdaLift = false,
+              hasRepeatedParam = hasRepeatedParam)
+        }
 
         // Fifth step: build the js.Closure
 
-        val isThisFunction = JSThisFunctionClasses.exists(sym isSubClass _)
-        assert(!isThisFunction || patchedParams.nonEmpty,
-            s"Empty param list in ThisFunction: $cd")
+        val isThisFunction = sym.isSubClass(JSThisFunctionClass) && {
+          val ok = patchedParams.headOption.exists(!_.rest)
+          if (!ok) {
+            reporter.error(pos,
+                "The SAM or apply method for a js.ThisFunction must have a " +
+                "leading non-varargs parameter")
+          }
+          ok
+        }
 
         val capturedArgs =
           if (hasUnusedOuterCtorParam) initialCapturedArgs.tail
@@ -5973,7 +5991,7 @@ abstract class GenJSCode[G <: Global with Singleton](val global: G)
 
       val (patchedFormalArgs, patchedBody) = {
         patchFunBodyWithBoxes(target, formalArgs, body,
-            useParamsBeforeLambdaLift = true)
+            useParamsBeforeLambdaLift = true, hasRepeatedParam = false)
       }
 
       val closure = js.Closure(
@@ -6121,7 +6139,7 @@ abstract class GenJSCode[G <: Global with Singleton](val global: G)
 
     private def patchFunBodyWithBoxes(methodSym: Symbol,
         params: List[js.ParamDef], body: js.Tree,
-        useParamsBeforeLambdaLift: Boolean = false)(
+        useParamsBeforeLambdaLift: Boolean, hasRepeatedParam: Boolean)(
         implicit pos: Position): (List[js.ParamDef], js.Tree) = {
       val methodType = enteringPhase(currentRun.posterasurePhase)(methodSym.tpe)
 
@@ -6147,18 +6165,26 @@ abstract class GenJSCode[G <: Global with Singleton](val global: G)
           methodSym.tpe.params
       }
 
+      val theRepeatedParamOrNull =
+        if (!hasRepeatedParam) null
+        else params.last
+
       val (patchedParams, paramsLocal) = (for {
         (param, paramSym) <- params zip paramSyms
       } yield {
-        val paramTpe = paramTpes.getOrElse(paramSym.name, paramSym.tpe)
+        val isRepeated = param eq theRepeatedParamOrNull
+        def paramTpe = paramTpes.getOrElse(paramSym.name, paramSym.tpe)
         val paramNameIdent = param.name
         val origName = param.originalName
         val newNameIdent = freshLocalIdent(paramNameIdent.name)(paramNameIdent.pos)
         val newOrigName = origName.orElse(paramNameIdent.name)
         val patchedParam = js.ParamDef(newNameIdent, newOrigName, jstpe.AnyType,
-            mutable = false, rest = param.rest)(param.pos)
+            mutable = false, rest = isRepeated)(param.pos)
+        val paramLocalRhs =
+          if (isRepeated) genJSArrayToVarArgs(patchedParam.ref)
+          else fromAny(patchedParam.ref, paramTpe)
         val paramLocal = js.VarDef(paramNameIdent, origName, param.ptpe,
-            mutable = false, fromAny(patchedParam.ref, paramTpe))
+            mutable = false, paramLocalRhs)
         (patchedParam, paramLocal)
       }).unzip
 
@@ -6532,16 +6558,16 @@ abstract class GenJSCode[G <: Global with Singleton](val global: G)
      * non-lambda, concrete, non-native JS type, cannot implement a method named
      * `apply`.
      *
-     * All JS function classes have an abstract member named `apply`. Therefore,
-     * a class is a JS lambda iff it is concrete, a non-native JS type and
-     * inherits from a JS function class.
+     * Therefore, a class is a JS lambda iff it is anonymous, its direct
+     * super class is `js.Function`, and it contains an implementation of an
+     * `apply` method.
      *
-     * To avoid having to an isSubClass check on all concrete non-native JS
-     * classes, we short-circuit check that the class is an anonymous class
-     * (a necessary, but not sufficient condition for a JS lambda).
+     * Note that being anonymous implies being concrete and non-native, so we
+     * do not have to test that.
      */
-    !isJSNativeClass(sym) && !sym.isAbstract && sym.isAnonymousClass &&
-    AllJSFunctionClasses.exists(sym.isSubClass(_))
+    sym.isAnonymousClass &&
+    sym.superClass == JSFunctionClass &&
+    sym.info.decl(nme.apply).filter(JSCallingConvention.isCall(_)).exists
   }
 
   private def isJSCtorDefaultParam(sym: Symbol) = {

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

@@ -51,16 +51,13 @@ trait JSDefinitions {
     lazy val JSAnyClass       = getRequiredClass("scala.scalajs.js.Any")
     lazy val JSDynamicClass   = getRequiredClass("scala.scalajs.js.Dynamic")
     lazy val JSObjectClass    = getRequiredClass("scala.scalajs.js.Object")
+    lazy val JSFunctionClass = getRequiredClass("scala.scalajs.js.Function")
     lazy val JSThisFunctionClass = getRequiredClass("scala.scalajs.js.ThisFunction")
 
     lazy val UnionClass = getRequiredClass("scala.scalajs.js.$bar")
 
     lazy val JSArrayClass = getRequiredClass("scala.scalajs.js.Array")
 
-    lazy val JSFunctionClasses     = (0 to 22) map (n => getRequiredClass("scala.scalajs.js.Function"+n))
-    lazy val JSThisFunctionClasses = (0 to 21) map (n => getRequiredClass("scala.scalajs.js.ThisFunction"+n))
-    lazy val AllJSFunctionClasses  = JSFunctionClasses ++ JSThisFunctionClasses
-
     lazy val JavaScriptExceptionClass = getClassIfDefined("scala.scalajs.js.JavaScriptException")
 
     lazy val JSNameAnnotation          = getRequiredClass("scala.scalajs.js.annotation.JSName")

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

@@ -199,6 +199,13 @@ trait JSGlobalAddons extends JSDefinitions
           }
         }
       }
+
+      /** Tests whether the calling convention of the specified symbol is `Call`.
+       *
+       *  This helper is provided because we use this test in a few places.
+       */
+      def isCall(sym: Symbol): Boolean =
+        of(sym) == Call
     }
 
     private object JSUnaryOpMethodName {

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

@@ -313,16 +313,32 @@ abstract class PrepJSInterop[G <: Global with Singleton](val global: G)
     private def transformStatOrExpr(tree: Tree): Tree = {
       tree match {
         /* Anonymous function, need to check that it is not used as a SAM for a
-         * JS type, unless it is js.FunctionN or js.ThisFunctionN.
+         * JS type, unless it is a JS function type.
          * See #2921.
          */
         case tree: Function =>
+          // tpeSym is the type of the target SAM type (not the to-be-generated anonymous class)
           val tpeSym = tree.tpe.typeSymbol
-          if (isJSAny(tpeSym) && !AllJSFunctionClasses.contains(tpeSym)) {
-            reporter.error(tree.pos,
-                "Using an anonymous function as a SAM for the JavaScript " +
-                "type " + tpeSym.fullNameString + " is not allowed. " +
-                "Use an anonymous class instead.")
+          if (isJSAny(tpeSym)) {
+            def reportError(reasonAndExplanation: String): Unit = {
+              reporter.error(tree.pos,
+                  "Using an anonymous function as a SAM for the JavaScript " +
+                  s"type ${tpeSym.fullNameString} is not allowed because " +
+                  reasonAndExplanation)
+            }
+            if (!tpeSym.isTrait || tpeSym.superClass != JSFunctionClass) {
+              reportError(
+                  "it is not a trait extending js.Function. " +
+                  "Use an anonymous class instead.")
+            } else if (tpeSym.hasAnnotation(JSNativeAnnotation)) {
+              reportError(
+                  "it is a native JS type. " +
+                  "It is not possible to directly implement it.")
+            } else if (!JSCallingConvention.isCall(samOf(tree.tpe))) {
+              reportError(
+                  "its single abstract method is not named `apply`. " +
+                  "Use an anonymous class instead.")
+            }
           }
           super.transform(tree)
 
@@ -550,48 +566,11 @@ abstract class PrepJSInterop[G <: Global with Singleton](val global: G)
 
       sym.addAnnotation(JSTypeAnnot)
 
-      val isJSLambda = {
-        /* Under 2.11, sym.isAnonymousFunction does not properly recognize
-         * anonymous functions here (because they seem to not be marked as
-         * synthetic).
-         */
-        sym.name == tpnme.ANON_FUN_NAME &&
-        sym.info.member(nme.apply).isSynthetic &&
-        AllJSFunctionClasses.exists(sym.isSubClass(_))
-      }
-
-      if (isJSLambda)
-        transformJSLambdaImplDef(implDef)
-      else
-        transformNonLambdaJSImplDef(implDef)
-    }
-
-    /** Performs checks and rewrites specific to JS lambdas, i.e., anonymous
-     *  classes extending one of the JS function types.
-     *
-     *  JS lambdas are special because they are completely hijacked by the
-     *  back-end, so although at this phase they look like normal anonymous
-     *  classes, they do not behave like ones.
-     */
-    private def transformJSLambdaImplDef(implDef: ImplDef): Tree = {
-      /* For the purposes of checking inner members, a JS lambda acts as a JS
-       * native class owner.
-       *
-       * TODO This is probably not right, but historically it has always been
-       * that way. It should be revisited.
-       */
-      enterOwner(OwnerKind.JSNativeClass) {
-        super.transform(implDef)
-      }
-    }
-
-    /** Performs checks and rewrites for all JS classes, traits and objects
-     *  except JS lambdas.
-     */
-    private def transformNonLambdaJSImplDef(implDef: ImplDef): Tree = {
-      val sym = moduleToModuleClass(implDef.symbol)
       val isJSNative = sym.hasAnnotation(JSNativeAnnotation)
 
+      val isJSFunctionSAMInScala211 =
+        isScala211 && sym.name == tpnme.ANON_FUN_NAME && sym.superClass == JSFunctionClass
+
       // Forbid @EnableReflectiveInstantiation on JS types
       sym.getAnnotation(EnableReflectiveInstantiationAnnotation).foreach {
         annot =>
@@ -634,6 +613,12 @@ abstract class PrepJSInterop[G <: Global with Singleton](val global: G)
              * and similar constructs.
              * This causes the unsoundness filed as #1385.
              */
+            ()
+          case SerializableClass if isJSFunctionSAMInScala211 =>
+            /* Ignore the scala.Serializable trait that Scala 2.11 adds on all
+             * SAM classes when on a JS function SAM.
+             */
+            ()
           case parentSym =>
             /* This is a Scala class or trait other than AnyRef and Dynamic,
              * which is never valid.
@@ -644,6 +629,23 @@ abstract class PrepJSInterop[G <: Global with Singleton](val global: G)
         }
       }
 
+      // Require that the SAM of a JS function def be `apply` (2.11-only here)
+      if (isJSFunctionSAMInScala211) {
+        if (!sym.info.decl(nme.apply).filter(JSCallingConvention.isCall(_)).exists) {
+          val samType = sym.parentSymbols.find(_ != JSFunctionClass).getOrElse {
+            /* This shouldn't happen, but fall back on this symbol (which has a
+             * compiler-generated name) not to crash.
+             */
+            sym
+          }
+          reporter.error(implDef.pos,
+              "Using an anonymous function as a SAM for the JavaScript type " +
+              s"${samType.fullNameString} is not allowed because its single " +
+              "abstract method is not named `apply`. " +
+              "Use an anonymous class instead.")
+        }
+      }
+
       // Disallow bracket access / bracket call
       if (jsInterop.isJSBracketAccess(sym)) {
         reporter.error(implDef.pos,
@@ -982,10 +984,43 @@ abstract class PrepJSInterop[G <: Global with Singleton](val global: G)
           case JSCallingConvention.Property(_) => // checked above
           case JSCallingConvention.Method(_)   => // no checks needed
 
-          case JSCallingConvention.Call =>
-            reporter.error(sym.pos,
-                "A non-native JS class cannot declare a method " +
-                "named `apply` without `@JSName`")
+          case JSCallingConvention.Call if !sym.isDeferred =>
+            /* Concrete `def apply` methods are normally rejected because we
+             * cannot implement them in JavaScript. However, we do allow a
+             * synthetic `apply` method if it is in a SAM for a JS function
+             * type.
+             */
+            val isJSFunctionSAM = {
+              /* Under 2.11, sym.owner.isAnonymousFunction does not properly
+               * recognize anonymous functions here (because they seem to not
+               * be marked as synthetic).
+               */
+              sym.isSynthetic &&
+              sym.owner.name == tpnme.ANON_FUN_NAME &&
+              sym.owner.superClass == JSFunctionClass
+            }
+            if (!isJSFunctionSAM) {
+              reporter.error(sym.pos,
+                  "A non-native JS class cannot declare a concrete method " +
+                  "named `apply` without `@JSName`")
+            }
+
+          case JSCallingConvention.Call => // if sym.isDeferred
+            /* Allow an abstract `def apply` only if the owner is a plausible
+             * JS function SAM trait.
+             */
+            val owner = sym.owner
+            val isPlausibleJSFunctionType = {
+              owner.isTrait &&
+              owner.superClass == JSFunctionClass &&
+              samOf(owner.toTypeConstructor) == sym
+            }
+            if (!isPlausibleJSFunctionType) {
+              reporter.error(sym.pos,
+                  "A non-native JS type can only declare an abstract " +
+                  "method named `apply` without `@JSName` if it is the " +
+                  "SAM of a trait that extends js.Function")
+            }
 
           case JSCallingConvention.BracketAccess =>
             reporter.error(tree.pos,
@@ -1160,14 +1195,11 @@ abstract class PrepJSInterop[G <: Global with Singleton](val global: G)
       if (sym.isPrimaryConstructor || sym.isValueParameter ||
           sym.isParamWithDefault || sym.isAccessor ||
           sym.isParamAccessor || sym.isDeferred || sym.isSynthetic ||
-          AllJSFunctionClasses.contains(sym.owner) ||
           (enclosingOwner is OwnerKind.JSNonNative)) {
         /* Ignore (i.e. allow) primary ctor, parameters, default parameter
          * getters, accessors, param accessors, abstract members, synthetic
          * methods (to avoid double errors with case classes, e.g. generated
-         * copy method), js.Functions and js.ThisFunctions (they need abstract
-         * methods for SAM treatment), and any member of a non-native JS
-         * class/trait.
+         * copy method), and any member of a non-native JS class/trait.
          */
       } else if (jsPrimitives.isJavaScriptPrimitive(sym)) {
         // No check for primitives. We trust our own standard library.

compiler/src/test/scala/org/scalajs/nscplugin/test/JSOptionalTest.scala

@@ -81,6 +81,18 @@ class JSOptionalTest extends DirectTest with TestHelpers {
       |      def b(x: String = "foo"): Unit
       |                        ^
     """
+
+    // Also for custom JS function types (2.11 has more errors than expected here)
+    s"""
+    trait A extends js.Function {
+      def apply(x: js.UndefOr[Int] = 1): Int
+    }
+    """ containsErrors
+    """
+      |newSource1.scala:6: error: Members of non-native JS traits may not have default parameters unless their default is `js.undefined`.
+      |      def apply(x: js.UndefOr[Int] = 1): Int
+      |                                     ^
+    """
   }
 
   @Test