Skip to content

Fix check for unimplemented members in a concrete class #11439

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Feb 23, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 16 additions & 10 deletions compiler/src/dotty/tools/dotc/typer/RefChecks.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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 _ =>
""
}
Expand All @@ -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("")
Expand Down
4 changes: 2 additions & 2 deletions tests/explicit-nulls/neg/override-java-object-arg.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
}
Expand All @@ -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
}
}
Expand Down
2 changes: 1 addition & 1 deletion tests/explicit-nulls/neg/override-java-object-arg2.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
}
Expand Down
2 changes: 1 addition & 1 deletion tests/neg/abstract-givens.check
Original file line number Diff line number Diff line change
@@ -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
| ^
Expand Down
8 changes: 8 additions & 0 deletions tests/neg/i10666.check
Original file line number Diff line number Diff line change
@@ -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.)
17 changes: 17 additions & 0 deletions tests/neg/i10666.scala
Original file line number Diff line number Diff line change
@@ -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)
}
}
4 changes: 2 additions & 2 deletions tests/neg/i7597.scala
Original file line number Diff line number Diff line change
@@ -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
}

Expand Down
4 changes: 2 additions & 2 deletions tests/neg/i9329.check
Original file line number Diff line number Diff line change
@@ -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)
4 changes: 4 additions & 0 deletions tests/pos-java-interop/i10599/A.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
public abstract class A<T> {
public abstract void foo(T value);
public void foo(T value, Object obj) { return; }
}
7 changes: 7 additions & 0 deletions tests/pos-java-interop/i10599/B.scala
Original file line number Diff line number Diff line change
@@ -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]