Merge pull request #10564 from dotty-staging/sjs-fix-static-methods-in-jsclass

Fix #10563: Handle static methods in non-native JS classes.

Author: sjrd

compiler/src/dotty/tools/backend/sjs/JSCodeGen.scala

@@ -1122,8 +1122,9 @@ class JSCodeGen()(using genCtx: Context) {
    *  type is Unit, then the body is emitted as a statement. Otherwise, it is
    *  emitted as an expression.
    *
-   *  Methods Scala.js-defined JS classes are compiled as static methods taking
-   *  an explicit parameter for their `this` value.
+   *  Instance methods in non-native JS classes are compiled as static methods
+   *  taking an explicit parameter for their `this` value. Static methods in
+   *  non-native JS classes are compiled as is, like methods in Scala classes.
    */
   private def genMethodDef(namespace: js.MemberNamespace, methodName: js.MethodIdent,
       originalName: OriginalName, paramsSyms: List[Symbol], resultIRType: jstpe.Type,
@@ -1137,13 +1138,11 @@ class JSCodeGen()(using genCtx: Context) {
       else genExpr(tree)
     }
 
-    if (!currentClassSym.isNonNativeJSClass) {
+    if (namespace.isStatic || !currentClassSym.isNonNativeJSClass) {
       val flags = js.MemberFlags.empty.withNamespace(namespace)
       js.MethodDef(flags, methodName, originalName, jsParams, resultIRType, Some(genBody()))(
             optimizerHints, None)
     } else {
-      assert(!namespace.isStatic, tree.span)
-
       val thisLocalIdent = freshLocalIdent("this")
       withScopedVars(
         thisLocalVarIdent := Some(thisLocalIdent)

compiler/src/dotty/tools/backend/sjs/JSEncoding.scala

@@ -19,6 +19,8 @@ import NameOps._
 import Names._
 import StdNames._
 
+import dotty.tools.dotc.transform.sjs.JSSymUtils._
+
 import org.scalajs.ir
 import org.scalajs.ir.{Trees => js, Types => jstpe}
 import org.scalajs.ir.Names.{LocalName, LabelName, FieldName, SimpleMethodName, MethodName, ClassName}
@@ -28,7 +30,6 @@ import org.scalajs.ir.UTF8String
 
 import ScopedVar.withScopedVars
 import JSDefinitions._
-import JSInterop._
 
 import dotty.tools.backend.jvm.DottyBackendInterface.symExtensions
 
@@ -199,7 +200,7 @@ object JSEncoding {
 
     val paramTypeRefs0 = tpe.firstParamTypes.map(paramOrResultTypeRef(_))
 
-    val hasExplicitThisParameter = isScalaJSDefinedJSClass(sym.owner)
+    val hasExplicitThisParameter = !sym.is(JavaStatic) && sym.owner.isNonNativeJSClass
     val paramTypeRefs =
       if (!hasExplicitThisParameter) paramTypeRefs0
       else encodeClassRef(sym.owner) :: paramTypeRefs0
@@ -243,7 +244,7 @@ object JSEncoding {
 
   def encodeClassType(sym: Symbol)(using Context): jstpe.Type = {
     if (sym == defn.ObjectClass) jstpe.AnyType
-    else if (isJSType(sym)) jstpe.AnyType
+    else if (sym.isJSType) jstpe.AnyType
     else {
       assert(sym != defn.ArrayClass,
           "encodeClassType() cannot be called with ArrayClass")
@@ -303,7 +304,7 @@ object JSEncoding {
 
       case typeRef: jstpe.ClassRef =>
         val sym = typeRefInternal._2
-        if (sym == defn.ObjectClass || isJSType(sym))
+        if (sym == defn.ObjectClass || sym.isJSType)
           jstpe.AnyType
         else if (sym == defn.NothingClass)
           jstpe.NothingType

compiler/src/dotty/tools/backend/sjs/JSInterop.scala

@@ -22,10 +22,6 @@ object JSInterop {
   def isJSType(sym: Symbol)(using Context): Boolean =
     sym.isJSType
 
-  /** Is this symbol a Scala.js-defined JS class, i.e., a non-native JS class? */
-  def isScalaJSDefinedJSClass(sym: Symbol)(using Context): Boolean =
-    sym.isNonNativeJSClass
-
   /** Should this symbol be translated into a JS getter?
    *
    *  This is true for any parameterless method, i.e., defined without `()`.

tests/sjs-junit/test/org/scalajs/testsuite/compiler/RegressionTestScala3.scala

@@ -3,12 +3,24 @@ package org.scalajs.testsuite.compiler
 import org.junit.Assert._
 import org.junit.Test
 
+import scala.scalajs.js
+
 class RegressionTestScala3 {
   import RegressionTestScala3._
 
   @Test def testRegressionDoubleDefinitionOfOuterPointerIssue10177(): Unit = {
     assertEquals(6, new OuterClassIssue10177().foo(5))
   }
+
+  @Test def testRegressionJSClassWithSyntheticStaticMethodsIssue10563(): Unit = {
+    val obj1 = new JSClassWithSyntheticStaticMethodsIssue10563(Some(3))
+    assertEquals(3, obj1.y)
+    assertEquals(8, obj1.foo(5))
+
+    val obj2 = new JSClassWithSyntheticStaticMethodsIssue10563(None)
+    assertEquals(-1, obj2.y)
+    assertEquals(4, obj2.foo(5))
+  }
 }
 
 object RegressionTestScala3 {
@@ -22,3 +34,10 @@ object RegressionTestScala3 {
     def foo(x: Int): Int = new ChildClass().concreteMethod(x)
   }
 }
+
+// This class needs to be at the top-level, not in an object, to reproduce the issue
+class JSClassWithSyntheticStaticMethodsIssue10563(x: Option[Int]) extends js.Object {
+  val y: Int = x.getOrElse(-1) // lambda in constructor
+
+  def foo(z: Int): Int = x.getOrElse(-1) + z // lambda in method
+}