Merge pull request #11439 from dotty-staging/override-fix-2

Fix check for unimplemented members in a concrete class

Author: smarter

community-build/community-projects/scala-stm

@@ -541,11 +541,11 @@ object RefChecks {
         || mbr.is(JavaDefined) && hasJavaErasedOverriding(mbr)
 
       def isImplemented(mbr: Symbol) =
-        val mbrType = clazz.thisType.memberInfo(mbr)
+        val mbrDenot = mbr.asSeenFrom(clazz.thisType)
         def isConcrete(sym: Symbol) = sym.exists && !sym.isOneOf(NotConcrete)
         clazz.nonPrivateMembersNamed(mbr.name)
           .filterWithPredicate(
-            impl => isConcrete(impl.symbol) && mbrType.matchesLoosely(impl.info))
+            impl => isConcrete(impl.symbol) && mbrDenot.matchesLoosely(impl))
           .exists
 
       /** The term symbols in this class and its baseclasses that are
@@ -602,8 +602,10 @@ object RefChecks {
         }
 
         for (member <- missing) {
+          def showDclAndLocation(sym: Symbol) =
+            s"${sym.showDcl} in ${sym.owner.showLocated}"
           def undefined(msg: String) =
-            abstractClassError(false, s"${member.showDcl} is not defined $msg")
+            abstractClassError(false, s"${showDclAndLocation(member)} is not defined $msg")
           val underlying = member.underlyingSymbol
 
           // Give a specific error message for abstract vars based on why it fails:
@@ -641,13 +643,13 @@ object RefChecks {
                     val abstractSym = pa.typeSymbol
                     val concreteSym = pc.typeSymbol
                     def subclassMsg(c1: Symbol, c2: Symbol) =
-                      s": ${c1.showLocated} is a subclass of ${c2.showLocated}, but method parameter types must match exactly."
+                      s"${c1.showLocated} is a subclass of ${c2.showLocated}, but method parameter types must match exactly."
                     val addendum =
                       if (abstractSym == concreteSym)
                         (pa.typeConstructor, pc.typeConstructor) match {
                           case (TypeRef(pre1, _), TypeRef(pre2, _)) =>
-                            if (pre1 =:= pre2) ": their type parameters differ"
-                            else ": their prefixes (i.e. enclosing instances) differ"
+                            if (pre1 =:= pre2) "their type parameters differ"
+                            else "their prefixes (i.e. enclosing instances) differ"
                           case _ =>
                             ""
                         }
@@ -657,18 +659,22 @@ object RefChecks {
                         subclassMsg(concreteSym, abstractSym)
                       else ""
 
-                    undefined(s"\n(Note that ${pa.show} does not match ${pc.show}$addendum)")
+                    undefined(s"""
+                                 |(Note that
+                                 | parameter ${pa.show} in ${showDclAndLocation(underlying)} does not match
+                                 | parameter ${pc.show} in ${showDclAndLocation(concrete.symbol)}
+                                 | $addendum)""".stripMargin)
                   case xs =>
                     undefined(
                       if concrete.symbol.is(AbsOverride) then
-                        s"\n(The class implements ${concrete.showDcl} but that definition still needs an implementation)"
+                        s"\n(The class implements ${showDclAndLocation(concrete.symbol)} but that definition still needs an implementation)"
                       else
-                        s"\n(The class implements a member with a different type: ${concrete.showDcl})")
+                        s"\n(The class implements a member with a different type: ${showDclAndLocation(concrete.symbol)})")
                 }
               case Nil =>
                 undefined("")
               case concretes =>
-                undefined(s"\n(The class implements members with different types: ${concretes.map(_.showDcl)}%\n  %)")
+                undefined(s"\n(The class implements members with different types: ${concretes.map(c => showDclAndLocation(c.symbol))}%\n  %)")
             }
           }
           else undefined("")

compiler/src/dotty/tools/dotc/typer/RefChecks.scala

@@ -7,7 +7,7 @@ import javax.management.{Notification, NotificationEmitter, NotificationListener
 class Foo {
 
   def bar(): Unit = {
-    val listener = new NotificationListener() { // error: object creation impossible
+    val listener = new NotificationListener() {
       override def handleNotification(n: Notification|Null, emitter: Object): Unit = { // error: method handleNotification overrides nothing
       }
     }
@@ -17,7 +17,7 @@ class Foo {
       }
     }
 
-    val listener3 = new NotificationListener() { // error: object creation impossible
+    val listener3 = new NotificationListener() {
       override def handleNotification(n: Notification, emitter: Object|Null): Unit = { // error: method handleNotification overrides nothing
       }
     }

tests/explicit-nulls/neg/override-java-object-arg.scala

@@ -4,7 +4,7 @@ import javax.management.{Notification, NotificationEmitter, NotificationListener
 class Foo {
 
   def bar(): Unit = {
-    val listener4 = new NotificationListener() { // error: duplicate symbol error
+    val listener4 = new NotificationListener() {
       def handleNotification(n: Notification|Null, emitter: Object): Unit = { // error
       }
     }

tests/explicit-nulls/neg/override-java-object-arg2.scala

@@ -1,7 +1,7 @@
 -- Error: tests/neg/abstract-givens.scala:11:8 -------------------------------------------------------------------------
 11 |  given s[T](using T): Seq[T] with // error
    |        ^
-   |        instance cannot be created, since def iterator: => Iterator[A] is not defined 
+   |instance cannot be created, since def iterator: => Iterator[A] in trait IterableOnce in package scala.collection is not defined 
 -- Error: tests/neg/abstract-givens.scala:8:8 --------------------------------------------------------------------------
 8 |  given y(using Int): String = summon[Int].toString * 22 // error
   |        ^

tests/neg/abstract-givens.check

@@ -0,0 +1,8 @@
+-- Error: tests/neg/i10666.scala:8:6 -----------------------------------------------------------------------------------
+8 |class Bar extends Foo { // error
+  |      ^
+  |      class Bar needs to be abstract, since def foo: [T <: B](tx: T): Unit in trait Foo is not defined 
+  |      (Note that
+  |       parameter T in def foo: [T <: B](tx: T): Unit in trait Foo does not match
+  |       parameter T in def foo: [T <: A](tx: T): Unit in class Bar
+  |       class B is a subclass of class A, but method parameter types must match exactly.)

tests/neg/i10666.check

@@ -0,0 +1,17 @@
+class A
+class B extends A
+
+trait Foo {
+  def foo[T <: B](tx: T): Unit
+}
+
+class Bar extends Foo { // error
+  def foo[T <: A](tx: T): Unit = {}
+}
+
+object Test {
+  def main(args: Array[String]): Unit = {
+    val f: Foo = new Bar
+    f.foo(new B)
+  }
+}

tests/neg/i10666.scala

@@ -1,12 +1,12 @@
 object Test extends App {
   def foo[S <: String]: String => Int =
-    new (String => Int) { def apply(s: S): Int = 0 } // error // error
+    new (String => Int) { def apply(s: S): Int = 0 } // error
 
   trait Fn[A, B] {
     def apply(x: A): B
   }
 
-  class C[S <: String] extends Fn[String, Int] {  // error
+  class C[S <: String] extends Fn[String, Int] {
     def apply(s: S): Int = 0 // error
   }
 

tests/neg/i7597.scala

@@ -1,5 +1,5 @@
 -- Error: tests/neg/i9329.scala:8:6 ------------------------------------------------------------------------------------
 8 |class GrandSon extends Son // error
   |      ^
-  |    class GrandSon needs to be abstract, since def name: => String is not defined 
-  |    (The class implements abstract override def name: => String but that definition still needs an implementation)
+  |class GrandSon needs to be abstract, since def name: => String in trait Parent is not defined 
+  |(The class implements abstract override def name: => String in trait Son but that definition still needs an implementation)

tests/neg/i9329.check

@@ -0,0 +1,4 @@
+public abstract class A<T> {
+    public abstract void foo(T value);
+    public void foo(T value, Object obj) { return; }
+}

tests/pos-java-interop/i10599/A.java

@@ -0,0 +1,7 @@
+abstract class Base[M[_], T] extends A[M[T]] {
+  override def foo(value: M[T]): Unit = ???
+}
+
+class ArrayTest[T] extends Base[Array, T]
+
+class ListTest[T] extends Base[List, T]