Fix #3989: Check that members of concrete classes are type-correct

This fixes the second half of #3989. Some tests had to be updated or
re-classified because they were unsound before.

Author: odersky

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

@@ -594,12 +594,61 @@ object RefChecks {
           checkNoAbstractDecls(bc.asClass.superClass)
       }
 
+      // Check that every term member of this concrete class has a symbol that matches the member's type
+      // Member types are computed by intersecting the types of all members that have the same name
+      // and signature. But a member selection will pick one particular implementation, according to
+      // the rules of overriding and linearization. This method checks that the implementation has indeed
+      // a type that subsumes the full member type.
+      def checkMemberTypesOK() = {
+
+        // First compute all member names we need to check in `membersToCheck`.
+        // We do not check
+        //  - types
+        //  - synthetic members or bridges
+        //  - members in other concrete classes, since these have been checked before
+        //    (this is done for efficiency)
+        //  - members in a prefix of inherited parents that all come from Java or Scala2
+        //    (this is done to avoid false negatives since Scala2's rules for checking are different)
+        val membersToCheck = mutable.Set[Name]()
+        val seenClasses = mutable.Set[Symbol]()
+        def addDecls(cls: Symbol): Unit =
+          if (!seenClasses.contains(cls)) {
+            seenClasses.+=(cls)
+            for (mbr <- cls.info.decls)
+              if (mbr.isTerm && !mbr.is(Synthetic | Bridge) && mbr.memberCanMatchInheritedSymbols &&
+                  !membersToCheck.contains(mbr.name))
+                membersToCheck.+=(mbr.name)
+            cls.info.parents.map(_.classSymbol)
+              .filter(_.is(AbstractOrTrait))
+              .dropWhile(_.is(JavaDefined | Scala2x))
+              .foreach(addDecls)
+          }
+        addDecls(clazz)
+
+        // For each member, check that the type of its symbol, as seen from `self`
+        // can override the info of this member
+        for (name <- membersToCheck) {
+          for (mbrd <- self.member(name).alternatives) {
+            val mbr = mbrd.symbol
+            val mbrType = mbr.info.asSeenFrom(self, mbr.owner)
+            if (!mbrType.overrides(mbrd.info, matchLoosely = true))
+              ctx.errorOrMigrationWarning(
+                em"""${mbr.showLocated} is not a legal implementation of `$name' in $clazz
+                    |  its type             $mbrType
+                    |  does not conform to  ${mbrd.info}""",
+                (if (mbr.owner == clazz) mbr else clazz).pos)
+          }
+        }
+      }
+
       checkNoAbstractMembers()
       if (abstractErrors.isEmpty)
         checkNoAbstractDecls(clazz)
 
       if (abstractErrors.nonEmpty)
         ctx.error(abstractErrorMessage, clazz.pos)
+
+      checkMemberTypesOK()
     } else if (clazz.is(Trait) && !(clazz derivesFrom defn.AnyValClass)) {
       // For non-AnyVal classes, prevent abstract methods in interfaces that override
       // final members in Object; see #4431

tests/neg/i1240b.scala

@@ -9,4 +9,4 @@ abstract class A[X] extends T[X] {
 trait U[X] extends T[X] {
   abstract override def foo(x: X): X = super.foo(x)
 }
-object Test extends A[String] with U[String] // error: accidental override
+object Test extends A[String] with U[String] // error: accidental override // error: merge error

tests/neg/i3989.scala

@@ -0,0 +1,7 @@
+object Test extends App {
+  trait A[+X] { def get: X }
+  case class B[X](x: X) extends A[X] { def get: X = x }
+  class C[X](x: Any) extends B[Any](x) with A[X] // error: not a legal implementation of `get'
+  def g(a: A[Int]): Int = a.get
+  g(new C[Int]("foo"))
+}

tests/pos/t4731.scala renamed to tests/neg/t4731.scala

@@ -1,14 +1,16 @@
 import java.util.Comparator
 
-trait Trait1[T] { def foo(arg: Comparator[T]): Unit }
+trait Trait1[T] { def foo(arg: Comparator[T]): String }
 
 trait Trait2[T] extends Trait1[T] { def foo(arg: Comparator[String]): Int = 0 }
 
-class Class1 extends Trait2[String] { }
+class Class1 extends Trait2[String] { } // error: not a legal implementation of `foo'
 
 object Test {
   def main(args: Array[String]): Unit = {
     val c = new Class1
     c.foo(Ordering[String])
+    val t: Trait1[String] = c
+    val x: String = t.foo(Ordering[String])
   }
 }

tests/pos-special/strawman-collections/CollectionStrawMan6.scala

@@ -415,7 +415,7 @@ object CollectionStrawMan6 extends LowPriority {
   /** Concrete collection type: List */
   sealed trait List[+A]
   extends LinearSeq[A]
-     with SeqLike[A, List]
+     with LinearSeqLike[A, List]
      with Buildable[A, List[A]] {
 
     def fromIterable[B](c: Iterable[B]): List[B] = List.fromIterable(c)
@@ -604,7 +604,7 @@ object CollectionStrawMan6 extends LowPriority {
   }
 
   class LazyList[+A](expr: => LazyList.Evaluated[A])
-  extends LinearSeq[A] with SeqLike[A, LazyList] {
+  extends LinearSeq[A] with LinearSeqLike[A, LazyList] {
     private[this] var evaluated = false
     private[this] var result: LazyList.Evaluated[A] = _
 

tests/run/colltest6/CollectionStrawMan6_1.scala

@@ -416,7 +416,7 @@ object CollectionStrawMan6 extends LowPriority {
   /** Concrete collection type: List */
   sealed trait List[+A]
   extends LinearSeq[A]
-     with SeqLike[A, List]
+     with LinearSeqLike[A, List]
      with Buildable[A, List[A]] {
 
     def fromIterable[B](c: Iterable[B]): List[B] = List.fromIterable(c)
@@ -605,7 +605,7 @@ object CollectionStrawMan6 extends LowPriority {
   }
 
   class LazyList[+A](expr: => LazyList.Evaluated[A])
-  extends LinearSeq[A] with SeqLike[A, LazyList] {
+  extends LinearSeq[A] with LinearSeqLike[A, LazyList] {
     private[this] var evaluated = false
     private[this] var result: LazyList.Evaluated[A] = _