Merge pull request #4895 from gzm0/no-override-abstract

sjrd · web-flow · commit 1972c0ed462c · 2023-08-19T10:50:12.000+02:00
Analyzer: Do not override abstract methods with missing methods
diff --git a/linker/shared/src/main/scala/org/scalajs/linker/analyzer/Analyzer.scala b/linker/shared/src/main/scala/org/scalajs/linker/analyzer/Analyzer.scala
@@ -657,10 +657,14 @@ final class Analyzer(config: CommonPhaseConfig, initial: Boolean,
     }
 
     private def createNonExistentPublicMethod(methodName: MethodName): MethodInfo = {
-      val syntheticData = makeSyntheticMethodInfo(methodName, MemberNamespace.Public)
-      val m = new MethodInfo(this, syntheticData, nonExistent = true)
-      publicMethodInfos += methodName -> m
-      m
+      /* Use getOrElseUpdate to avoid overriding an abstract method:
+       * When being called from lookupMethod, it is possible that an abstract
+       * method exists.
+       */
+      publicMethodInfos.getOrElseUpdate(methodName, {
+        val syntheticData = makeSyntheticMethodInfo(methodName, MemberNamespace.Public)
+        new MethodInfo(this, syntheticData, nonExistent = true)
+      })
     }
 
     def tryLookupMethod(methodName: MethodName): Option[MethodInfo] = {
@@ -959,6 +963,7 @@ final class Analyzer(config: CommonPhaseConfig, initial: Boolean,
 
     def tryLookupStaticLikeMethod(namespace: MemberNamespace,
         methodName: MethodName): Option[MethodInfo] = {
+      assert(namespace != MemberNamespace.Public)
       methodInfos(namespace).get(methodName)
     }
 
@@ -1246,10 +1251,7 @@ final class Analyzer(config: CommonPhaseConfig, initial: Boolean,
       s"$owner.${methodName.simpleName.nameString}"
 
     def reachStatic()(implicit from: From): Unit = {
-      assert(!isAbstract,
-          s"Trying to reach statically the abstract method $this")
-
-      checkExistent()
+      checkConcrete()
 
       calledFrom ::= from
       if (!isReachable) {
@@ -1272,14 +1274,12 @@ final class Analyzer(config: CommonPhaseConfig, initial: Boolean,
     def reach(inClass: ClassInfo)(implicit from: From): Unit = {
       assert(!namespace.isStatic,
           s"Trying to dynamically reach the static method $this")
-      assert(!isAbstract,
-          s"Trying to dynamically reach the abstract method $this")
       assert(owner.isAnyClass,
           s"Trying to dynamically reach the non-class method $this")
       assert(!namespace.isConstructor,
           s"Trying to dynamically reach the constructor $this")
 
-      checkExistent()
+      checkConcrete()
 
       calledFrom ::= from
       instantiatedSubclasses ::= inClass
@@ -1296,6 +1296,11 @@ final class Analyzer(config: CommonPhaseConfig, initial: Boolean,
         _errors += MissingMethod(this, from)
     }
 
+    private def checkConcrete()(implicit from: From) = {
+      if (nonExistent || isAbstract)
+        _errors += MissingMethod(this, from)
+    }
+
     private[this] def doReach(): Unit = {
       followReachabilityInfo(data.reachabilityInfo, owner.staticDependencies,
           owner.externalDependencies, owner.dynamicDependencies)(FromMethod(this))
diff --git a/linker/shared/src/test/scala/org/scalajs/linker/AnalyzerTest.scala b/linker/shared/src/test/scala/org/scalajs/linker/AnalyzerTest.scala
@@ -364,6 +364,49 @@ class AnalyzerTest {
     }
   }
 
+  @Test
+  def callAbstractMethod(): AsyncResult = await {
+    val fooMethodName = m("foo", Nil, IntRef)
+
+    val classDefs = Seq(
+        classDef("A", superClass = Some(ObjectClass),
+            methods = List(
+                trivialCtor("A"),
+                MethodDef(EMF, fooMethodName, NON, Nil, IntType, None)(EOH, UNV)
+            )
+        )
+    )
+
+    val analysis = computeAnalysis(classDefs,
+        reqsFactory.instantiateClass("A", NoArgConstructorName) ++
+        reqsFactory.callMethod("A", fooMethodName))
+
+    assertContainsError("MissingMethod(A.foo;I)", analysis) {
+      case MissingMethod(MethInfo("A", "foo;I"), FromDispatch(ClsInfo("A"),`fooMethodName`)) => true
+    }
+  }
+
+  @Test
+  def staticCallAbstractMethod(): AsyncResult = await {
+    val fooMethodName = m("foo", Nil, IntRef)
+
+    val classDefs = Seq(
+        classDef("A", superClass = Some(ObjectClass),
+            methods = List(
+                trivialCtor("A"),
+                MethodDef(EMF, fooMethodName, NON, Nil, IntType, None)(EOH, UNV)
+            )
+        )
+    )
+
+    val analysis = computeAnalysis(classDefs,
+        reqsFactory.callMethodStatically("A", fooMethodName))
+
+    assertContainsError("MissingMethod(A.foo;I)", analysis) {
+      case MissingMethod(MethInfo("A", "foo;I"), `fromUnitTest`) => true
+    }
+  }
+
   @Test
   def missingJSNativeMember(): AsyncResult = await {
     val mainName = m("main", Nil, V)
