Fix #7597: Refine checks whether a deferred term member is implemented

The check for a concrete class used to be simply that its `abstractTermMembers`
are empty. However, i7597.scala shows that this is not enough. The problem is
that abstractTermMembers is based on signatures. It will not include a member
as long as there is a concrete member with the same signature.

But as #7597 shows it's possible for a concrete member to have the same signature
as an abstract one but still not have a matching type.

Author: odersky

compiler/src/dotty/tools/dotc/core/SymDenotations.scala

@@ -2377,7 +2377,7 @@ object SymDenotations {
     /** is the cache valid in current run at given phase? */
     def isValidAt(phase: Phase)(implicit ctx: Context): Boolean
 
-    /** Render invalid this cache and all cache that depend on it */
+    /** Render invalid this cache and all caches that depend on it */
     def invalidate(): Unit
   }
 

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

@@ -6,7 +6,7 @@ import core._
 import Contexts._, Types._, Symbols._, Names._, Decorators._, ProtoTypes._
 import Flags._
 import NameKinds.FlatName
-import config.Printers.implicits
+import config.Printers.{implicits, implicitsDetailed}
 import util.Spans.Span
 import ast.{untpd, tpd}
 import Implicits.{hasExtMethod, Candidate}
@@ -94,7 +94,7 @@ trait ImportSuggestions:
     def rootsIn(ref: TermRef)(using Context): List[TermRef] =
       if seen.contains(ref) then Nil
       else
-        implicits.println(i"search for suggestions in ${ref.symbol.fullName}")
+        implicitsDetailed.println(i"search for suggestions in ${ref.symbol.fullName}")
         seen += ref
         ref :: rootsStrictlyIn(ref)
 

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

@@ -169,7 +169,7 @@ object RefChecks {
    *  TODO This still needs to be cleaned up; the current version is a straight port of what was there
    *       before, but it looks too complicated and method bodies are far too large.
    */
-  private def checkAllOverrides(clazz: Symbol)(implicit ctx: Context): Unit = {
+  private def checkAllOverrides(clazz: ClassSymbol)(implicit ctx: Context): Unit = {
     val self = clazz.thisType
     val upwardsSelf = upwardsThisType(clazz)
     var hasErrors = false
@@ -470,33 +470,52 @@ object RefChecks {
             }
         }
 
-      def ignoreDeferred(member: SingleDenotation) =
-        member.isType || {
-          val mbr = member.symbol
-          mbr.isSuperAccessor || // not yet synthesized
-          ShortcutImplicits.isImplicitShortcut(mbr) || // only synthesized when referenced, see Note in ShortcutImplicits
-          mbr.is(JavaDefined) && hasJavaErasedOverriding(mbr)
-        }
+       def ignoreDeferred(mbr: Symbol) =
+        mbr.isType
+        || mbr.isSuperAccessor // not yet synthesized
+        || ShortcutImplicits.isImplicitShortcut(mbr) // only synthesized when referenced, see Note in ShortcutImplicits
+        || mbr.is(JavaDefined) && hasJavaErasedOverriding(mbr)
+
+      def isImplemented(mbr: Symbol) =
+        val mbrType = clazz.thisType.memberInfo(mbr)
+        def (sym: Symbol).isConcrete = sym.exists && !sym.is(Deferred)
+        clazz.nonPrivateMembersNamed(mbr.name)
+          .filterWithPredicate(
+            impl => impl.symbol.isConcrete && mbrType.matchesLoosely(impl.info))
+          .exists
+
+      /** The term symbols in this class and its baseclasses that are
+       *  abstract in this class. We can't use memberNames for that since
+       *  a concrete member might have the same signature as an abstract
+       *  member in a base class, yet might not override it.
+       */
+      def missingTermSymbols: List[Symbol] =
+        val buf = new mutable.ListBuffer[Symbol]
+        for bc <- clazz.baseClasses
+            sym <- bc.info.decls.toList
+            if sym.is(DeferredTerm) && !isImplemented(sym) && !ignoreDeferred(sym)
+        do buf += sym
+        buf.toList
 
       // 2. Check that only abstract classes have deferred members
       def checkNoAbstractMembers(): Unit = {
         // Avoid spurious duplicates: first gather any missing members.
-        val missing = clazz.thisType.abstractTermMembers.filterNot(ignoreDeferred)
+        val missing = missingTermSymbols
         // Group missing members by the name of the underlying symbol,
         // to consolidate getters and setters.
-        val grouped = missing.groupBy(_.symbol.underlyingSymbol.name)
+        val grouped = missing.groupBy(_.underlyingSymbol.name)
 
         val missingMethods = grouped.toList flatMap {
           case (name, syms) =>
-            val withoutSetters = syms filterNot (_.symbol.isSetter)
+            val withoutSetters = syms filterNot (_.isSetter)
             if (withoutSetters.nonEmpty) withoutSetters else syms
         }
 
         def stubImplementations: List[String] = {
           // Grouping missing methods by the declaring class
-          val regrouped = missingMethods.groupBy(_.symbol.owner).toList
-          def membersStrings(members: List[SingleDenotation]) =
-            members.sortBy(_.symbol.name.toString).map(_.showDcl + " = ???")
+          val regrouped = missingMethods.groupBy(_.owner).toList
+          def membersStrings(members: List[Symbol]) =
+            members.sortBy(_.name.toString).map(_.showDcl + " = ???")
 
           if (regrouped.tail.isEmpty)
             membersStrings(regrouped.head._2)
@@ -520,22 +539,21 @@ object RefChecks {
         }
 
         for (member <- missing) {
-          val memberSym = member.symbol
           def undefined(msg: String) =
             abstractClassError(false, s"${member.showDcl} is not defined $msg")
-          val underlying = memberSym.underlyingSymbol
+          val underlying = member.underlyingSymbol
 
           // Give a specific error message for abstract vars based on why it fails:
           // It could be unimplemented, have only one accessor, or be uninitialized.
           if (underlying.is(Mutable)) {
             val isMultiple = grouped.getOrElse(underlying.name(ctx), Nil).size > 1
 
             // If both getter and setter are missing, squelch the setter error.
-            if (memberSym.isSetter && isMultiple) ()
+            if (member.isSetter && isMultiple) ()
             else undefined(
-              if (memberSym.isSetter) "\n(Note that an abstract var requires a setter in addition to the getter)"
-              else if (memberSym.isGetter && !isMultiple) "\n(Note that an abstract var requires a getter in addition to the setter)"
-              else err.abstractVarMessage(memberSym))
+              if (member.isSetter) "\n(Note that an abstract var requires a setter in addition to the getter)"
+              else if (member.isGetter && !isMultiple) "\n(Note that an abstract var requires a getter in addition to the setter)"
+              else err.abstractVarMessage(member))
           }
           else if (underlying.is(Method)) {
             // If there is a concrete method whose name matches the unimplemented
@@ -961,7 +979,7 @@ class RefChecks extends MiniPhase { thisPhase =>
   }
 
   override def transformTemplate(tree: Template)(implicit ctx: Context): Tree = try {
-    val cls = ctx.owner
+    val cls = ctx.owner.asClass
     checkOverloadedRestrictions(cls)
     checkParents(cls)
     if (cls.is(Trait)) tree.parents.foreach(checkParentPrefix(cls, _))

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

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

tests/neg-custom-args/erased/erased-case-class.scala

@@ -1 +1 @@
-case class Foo1(erased x: Int) // error // error
+case class Foo1(erased x: Int) // error

tests/neg/i7597.scala

@@ -0,0 +1,14 @@
+object Test extends App {
+  def foo[S <: String]: String => Int =
+    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
+    def apply(s: S): Int = 0
+  }
+
+  foo("")
+}