summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorPaul Phillips <paulp@improving.org>2011-10-14 02:16:17 +0000
committerPaul Phillips <paulp@improving.org>2011-10-14 02:16:17 +0000
commitfcd0998f1e0f2307e9b0cbae6bf2c36234ca8d17 (patch)
tree3eb2fa53283a9f399cd3e11a15a391b9d0307ca3
parentbca8959a1ab162dadec51c0db7d062315f5e4d6e (diff)
downloadscala-fcd0998f1e0f2307e9b0cbae6bf2c36234ca8d17.tar.gz
scala-fcd0998f1e0f2307e9b0cbae6bf2c36234ca8d17.tar.bz2
scala-fcd0998f1e0f2307e9b0cbae6bf2c36234ca8d17.zip
Better error when abstract methods are missing.
When many methods are missing, print a list of signatures the way they need to be implemented, and throw in ??? stub implementations so it should be compilable code. If anyone would like this logic exposed more generally (for the IDE or whatever) just let me know. No review.
-rw-r--r--src/compiler/scala/reflect/internal/Symbols.scala15
-rw-r--r--src/compiler/scala/tools/nsc/typechecker/Infer.scala6
-rw-r--r--src/compiler/scala/tools/nsc/typechecker/Namers.scala26
-rw-r--r--src/compiler/scala/tools/nsc/typechecker/RefChecks.scala53
-rw-r--r--src/compiler/scala/tools/nsc/typechecker/TypeDiagnostics.scala26
-rw-r--r--test/files/neg/abstract-report.check24
-rw-r--r--test/files/neg/abstract-report.scala1
-rw-r--r--test/files/neg/abstract-report2.check99
-rw-r--r--test/files/neg/abstract-report2.scala11
-rw-r--r--test/files/neg/t2208.check2
-rw-r--r--test/files/neg/t2213.check26
-rw-r--r--test/files/neg/t856.check12
-rw-r--r--test/files/neg/tcpoly_ticket2101.check2
13 files changed, 250 insertions, 53 deletions
diff --git a/src/compiler/scala/reflect/internal/Symbols.scala b/src/compiler/scala/reflect/internal/Symbols.scala
index db17306768..4ded91c16c 100644
--- a/src/compiler/scala/reflect/internal/Symbols.scala
+++ b/src/compiler/scala/reflect/internal/Symbols.scala
@@ -1900,13 +1900,20 @@ trait Symbols extends api.Symbols { self: SymbolTable =>
else ExplicitFlags
def defaultFlagString = hasFlagsToString(defaultFlagMask)
-
- /** String representation of symbol's definition */
- def defString = compose(
+ private def defStringCompose(infoString: String) = compose(
defaultFlagString,
keyString,
- varianceString + nameString + signatureString
+ varianceString + nameString + infoString
)
+ /** String representation of symbol's definition. It uses the
+ * symbol's raw info to avoid forcing types.
+ */
+ def defString = defStringCompose(signatureString)
+
+ /** String representation of symbol's definition, using the supplied
+ * info rather than the symbol's.
+ */
+ def defStringSeenAs(info: Type) = defStringCompose(infoString(info))
/** Concatenate strings separated by spaces */
private def compose(ss: String*) = ss filter (_ != "") mkString " "
diff --git a/src/compiler/scala/tools/nsc/typechecker/Infer.scala b/src/compiler/scala/tools/nsc/typechecker/Infer.scala
index 354eb52913..f2a0091ee1 100644
--- a/src/compiler/scala/tools/nsc/typechecker/Infer.scala
+++ b/src/compiler/scala/tools/nsc/typechecker/Infer.scala
@@ -293,7 +293,7 @@ trait Infer {
else
ptBlock("because of an internal error (no accessible symbol)",
"sym.ownerChain" -> sym.ownerChain,
- "underlying(sym)" -> underlying(sym),
+ "underlyingSymbol(sym)" -> underlyingSymbol(sym),
"pre" -> pre,
"site" -> site,
"tree" -> tree,
@@ -312,7 +312,7 @@ trait Infer {
} catch {
case ex: MalformedType =>
if (settings.debug.value) ex.printStackTrace
- val sym2 = underlying(sym1)
+ val sym2 = underlyingSymbol(sym1)
val itype = pre.memberType(sym2)
new AccessError(tree, sym, pre,
"\n because its instance type "+itype+
@@ -1870,7 +1870,7 @@ trait Infer {
// @PP: It is improbable this logic shouldn't be in use elsewhere as well.
private def location = if (sym.isClassConstructor) context.enclClass.owner else pre.widen
def emit(): Tree = {
- val realsym = underlying(sym)
+ val realsym = underlyingSymbol(sym)
errorTree(tree, realsym.fullLocationString + " cannot be accessed in " + location + explanation)
}
}
diff --git a/src/compiler/scala/tools/nsc/typechecker/Namers.scala b/src/compiler/scala/tools/nsc/typechecker/Namers.scala
index b25c52b324..8dd23d14f3 100644
--- a/src/compiler/scala/tools/nsc/typechecker/Namers.scala
+++ b/src/compiler/scala/tools/nsc/typechecker/Namers.scala
@@ -1403,7 +1403,7 @@ trait Namers { self: Analyzer =>
!context.tree.isInstanceOf[ExistentialTypeTree] &&
(!sym.owner.isClass || sym.owner.isModuleClass || sym.owner.isAnonymousClass)) {
context.error(sym.pos,
- "only classes can have declared but undefined members" + varNotice(sym))
+ "only classes can have declared but undefined members" + abstractVarMessage(sym))
sym.resetFlag(DEFERRED)
}
}
@@ -1455,21 +1455,8 @@ trait Namers { self: Analyzer =>
}
}
- /** The symbol that which this accessor represents (possibly in part).
- * This is used for error messages, where we want to speak in terms
- * of the actual declaration or definition, not in terms of the generated setters
- * and getters */
- def underlying(member: Symbol): Symbol =
- if (member.hasAccessorFlag) {
- if (member.isDeferred) {
- val getter = if (member.isSetter) member.getter(member.owner) else member
- val result = getter.owner.newValue(getter.pos, getter.name.toTermName)
- .setInfo(getter.tpe.resultType)
- .setFlag(DEFERRED)
- if (getter.setter(member.owner) != NoSymbol) result.setFlag(MUTABLE)
- result
- } else member.accessed
- } else member
+ @deprecated("Use underlyingSymbol instead", "2.10.0")
+ def underlying(member: Symbol): Symbol = underlyingSymbol(member)
/**
* Finds the companion module of a class symbol. Calling .companionModule
@@ -1506,12 +1493,5 @@ trait Namers { self: Analyzer =>
if (sym.isTerm) companionClassOf(sym, context)
else if (sym.isClass) companionModuleOf(sym, context)
else NoSymbol
-
- /** An explanatory note to be added to error messages
- * when there's a problem with abstract var defs */
- def varNotice(sym: Symbol): String =
- if (underlying(sym).isVariable)
- "\n(Note that variables need to be initialized to be defined)"
- else ""
}
diff --git a/src/compiler/scala/tools/nsc/typechecker/RefChecks.scala b/src/compiler/scala/tools/nsc/typechecker/RefChecks.scala
index ca90bb8e6b..c9237627e7 100644
--- a/src/compiler/scala/tools/nsc/typechecker/RefChecks.scala
+++ b/src/compiler/scala/tools/nsc/typechecker/RefChecks.scala
@@ -247,7 +247,7 @@ abstract class RefChecks extends InfoTransform with reflect.internal.transform.R
def infoStringWithLocation(sym: Symbol) = infoString0(sym, true)
def infoString0(sym: Symbol, showLocation: Boolean) = {
- val sym1 = analyzer.underlying(sym)
+ val sym1 = analyzer.underlyingSymbol(sym)
sym1.toString() +
(if (showLocation)
sym1.locationString +
@@ -516,32 +516,63 @@ abstract class RefChecks extends InfoTransform with reflect.internal.transform.R
)
// 2. Check that only abstract classes have deferred members
- def checkNoAbstractMembers() = {
+ def checkNoAbstractMembers(): Unit = {
// Avoid spurious duplicates: first gather any missing members.
- def memberList = clazz.tpe.nonPrivateMembersAdmitting(VBRIDGE)
+ def memberList = clazz.info.nonPrivateMembersAdmitting(VBRIDGE)
val (missing, rest) = memberList partition (m => m.isDeferred && !ignoreDeferred(m))
- // Group missing members by the underlying symbol.
- val grouped = missing groupBy (analyzer underlying _ name)
+ // Group missing members by the name of the underlying symbol,
+ // to consolidate getters and setters.
+ val grouped = missing groupBy (sym => analyzer.underlyingSymbol(sym).name)
+ val missingMethods = grouped.toList map {
+ case (name, sym :: Nil) => sym
+ case (name, syms) => syms.sortBy(!_.isGetter).head
+ }
+
+ def stubImplementations: List[String] = {
+ // Grouping missing methods by the declaring class
+ val regrouped = missingMethods.groupBy(_.owner).toList
+ def membersStrings(members: List[Symbol]) =
+ members.sortBy("" + _.name) map (m => m.defStringSeenAs(clazz.tpe memberType m) + " = ???")
+
+ if (regrouped.tail.isEmpty)
+ membersStrings(regrouped.head._2)
+ else (regrouped.sortBy("" + _._1.name) flatMap {
+ case (owner, members) =>
+ ("// Members declared in " + owner.fullName) +: membersStrings(members) :+ ""
+ }).init
+ }
+
+ // If there are numerous missing methods, we presume they are aware of it and
+ // give them a nicely formatted set of method signatures for implementing.
+ if (missingMethods.size > 1) {
+ abstractClassError(false, "it has " + missingMethods.size + " unimplemented members.")
+ val preface =
+ """|/** As seen from %s, the missing signatures are as follows.
+ | * For convenience, these are usable as stub implementations.
+ | */
+ |""".stripMargin.format(clazz)
+ abstractErrors += stubImplementations.map(" " + _ + "\n").mkString(preface, "", "")
+ return
+ }
for (member <- missing) {
def undefined(msg: String) = abstractClassError(false, infoString(member) + " is not defined" + msg)
- val underlying = analyzer.underlying(member)
+ val underlying = analyzer.underlyingSymbol(member)
// 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.isVariable) {
+ val isMultiple = grouped.getOrElse(underlying.name, Nil).size > 1
+
// If both getter and setter are missing, squelch the setter error.
- val isMultiple = grouped(underlying.name).size > 1
- // TODO: messages shouldn't be spread over two files, and varNotice is not a clear name
if (member.isSetter && isMultiple) ()
else undefined(
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 analyzer.varNotice(member)
+ else analyzer.abstractVarMessage(member)
)
}
else if (underlying.isMethod) {
-
// If there is a concrete method whose name matches the unimplemented
// abstract method, and a cursory examination of the difference reveals
// something obvious to us, let's make it more obvious to them.
@@ -622,7 +653,7 @@ abstract class RefChecks extends InfoTransform with reflect.internal.transform.R
val impl = decl.matchingSymbol(clazz.thisType, admit = VBRIDGE)
if (impl == NoSymbol || (decl.owner isSubClass impl.owner)) {
abstractClassError(false, "there is a deferred declaration of "+infoString(decl)+
- " which is not implemented in a subclass"+analyzer.varNotice(decl))
+ " which is not implemented in a subclass"+analyzer.abstractVarMessage(decl))
}
}
}
diff --git a/src/compiler/scala/tools/nsc/typechecker/TypeDiagnostics.scala b/src/compiler/scala/tools/nsc/typechecker/TypeDiagnostics.scala
index 6c735a2d44..42f8188ac1 100644
--- a/src/compiler/scala/tools/nsc/typechecker/TypeDiagnostics.scala
+++ b/src/compiler/scala/tools/nsc/typechecker/TypeDiagnostics.scala
@@ -123,6 +123,14 @@ trait TypeDiagnostics {
else nameString + " is not a member of " + targetKindString + target + addendum
)
}
+
+ /** An explanatory note to be added to error messages
+ * when there's a problem with abstract var defs */
+ def abstractVarMessage(sym: Symbol): String =
+ if (underlyingSymbol(sym).isVariable)
+ "\n(Note that variables need to be initialized to be defined)"
+ else ""
+
def notAMemberError(pos: Position, qual: Tree, name: Name) =
context.error(pos, notAMemberMessage(pos, qual, name))
@@ -161,6 +169,24 @@ trait TypeDiagnostics {
"missing parameter type" + suffix
}
+ /** The symbol which the given accessor represents (possibly in part).
+ * This is used for error messages, where we want to speak in terms
+ * of the actual declaration or definition, not in terms of the generated setters
+ * and getters.
+ */
+ def underlyingSymbol(member: Symbol): Symbol =
+ if (!member.hasAccessorFlag) member
+ else if (!member.isDeferred) member.accessed
+ else {
+ val getter = if (member.isSetter) member.getter(member.owner) else member
+ val flags = if (getter.setter(member.owner) != NoSymbol) DEFERRED | MUTABLE else DEFERRED
+
+ ( getter.owner.newValue(getter.pos, getter.name.toTermName)
+ setInfo getter.tpe.resultType
+ setFlag flags
+ )
+ }
+
def treeSymTypeMsg(tree: Tree): String = {
val sym = tree.symbol
def hasParams = tree.tpe.paramSectionCount > 0
diff --git a/test/files/neg/abstract-report.check b/test/files/neg/abstract-report.check
new file mode 100644
index 0000000000..bd550f39f4
--- /dev/null
+++ b/test/files/neg/abstract-report.check
@@ -0,0 +1,24 @@
+abstract-report.scala:1: error: class Unimplemented needs to be abstract, since:
+it has 12 unimplemented members.
+/** As seen from class Unimplemented, the missing signatures are as follows.
+ * For convenience, these are usable as stub implementations.
+ */
+ // Members declared in scala.collection.GenTraversableOnce
+ def isTraversableAgain: Boolean = ???
+ def toIterator: Iterator[String] = ???
+ def toStream: Stream[String] = ???
+
+ // Members declared in scala.collection.TraversableOnce
+ def copyToArray[B >: String](xs: Array[B],start: Int,len: Int): Unit = ???
+ def exists(p: String => Boolean): Boolean = ???
+ def find(p: String => Boolean): Option[String] = ???
+ def forall(p: String => Boolean): Boolean = ???
+ def foreach[U](f: String => U): Unit = ???
+ def hasDefiniteSize: Boolean = ???
+ def isEmpty: Boolean = ???
+ def seq: scala.collection.TraversableOnce[String] = ???
+ def toTraversable: Traversable[String] = ???
+
+class Unimplemented extends TraversableOnce[String] { }
+ ^
+one error found
diff --git a/test/files/neg/abstract-report.scala b/test/files/neg/abstract-report.scala
new file mode 100644
index 0000000000..538e093547
--- /dev/null
+++ b/test/files/neg/abstract-report.scala
@@ -0,0 +1 @@
+class Unimplemented extends TraversableOnce[String] { } \ No newline at end of file
diff --git a/test/files/neg/abstract-report2.check b/test/files/neg/abstract-report2.check
new file mode 100644
index 0000000000..32d52e00f2
--- /dev/null
+++ b/test/files/neg/abstract-report2.check
@@ -0,0 +1,99 @@
+abstract-report2.scala:3: error: class Foo needs to be abstract, since:
+it has 12 unimplemented members.
+/** As seen from class Foo, the missing signatures are as follows.
+ * For convenience, these are usable as stub implementations.
+ */
+ def add(x$1: Int): Boolean = ???
+ def addAll(x$1: java.util.Collection[_ <: Int]): Boolean = ???
+ def clear(): Unit = ???
+ def contains(x$1: Any): Boolean = ???
+ def containsAll(x$1: java.util.Collection[_]): Boolean = ???
+ def isEmpty(): Boolean = ???
+ def iterator(): java.util.Iterator[Int] = ???
+ def remove(x$1: Any): Boolean = ???
+ def removeAll(x$1: java.util.Collection[_]): Boolean = ???
+ def retainAll(x$1: java.util.Collection[_]): Boolean = ???
+ def size(): Int = ???
+ def toArray[T](x$1: Array[T with Object]): Array[T with Object] = ???
+
+class Foo extends Collection[Int]
+ ^
+abstract-report2.scala:5: error: class Bar needs to be abstract, since:
+it has 12 unimplemented members.
+/** As seen from class Bar, the missing signatures are as follows.
+ * For convenience, these are usable as stub implementations.
+ */
+ def add(x$1: List[_ <: String]): Boolean = ???
+ def addAll(x$1: java.util.Collection[_ <: List[_ <: String]]): Boolean = ???
+ def clear(): Unit = ???
+ def contains(x$1: Any): Boolean = ???
+ def containsAll(x$1: java.util.Collection[_]): Boolean = ???
+ def isEmpty(): Boolean = ???
+ def iterator(): java.util.Iterator[List[_ <: String]] = ???
+ def remove(x$1: Any): Boolean = ???
+ def removeAll(x$1: java.util.Collection[_]): Boolean = ???
+ def retainAll(x$1: java.util.Collection[_]): Boolean = ???
+ def size(): Int = ???
+ def toArray[T](x$1: Array[T with Object]): Array[T with Object] = ???
+
+class Bar extends Collection[List[_ <: String]]
+ ^
+abstract-report2.scala:7: error: class Baz needs to be abstract, since:
+it has 12 unimplemented members.
+/** As seen from class Baz, the missing signatures are as follows.
+ * For convenience, these are usable as stub implementations.
+ */
+ def add(x$1: T): Boolean = ???
+ def addAll(x$1: java.util.Collection[_ <: T]): Boolean = ???
+ def clear(): Unit = ???
+ def contains(x$1: Any): Boolean = ???
+ def containsAll(x$1: java.util.Collection[_]): Boolean = ???
+ def isEmpty(): Boolean = ???
+ def iterator(): java.util.Iterator[T] = ???
+ def remove(x$1: Any): Boolean = ???
+ def removeAll(x$1: java.util.Collection[_]): Boolean = ???
+ def retainAll(x$1: java.util.Collection[_]): Boolean = ???
+ def size(): Int = ???
+ def toArray[T](x$1: Array[T with Object]): Array[T with Object] = ???
+
+class Baz[T] extends Collection[T]
+ ^
+abstract-report2.scala:11: error: class Dingus needs to be abstract, since:
+it has 23 unimplemented members.
+/** As seen from class Dingus, the missing signatures are as follows.
+ * For convenience, these are usable as stub implementations.
+ */
+ // Members declared in java.util.Collection
+ def add(x$1: String): Boolean = ???
+ def addAll(x$1: java.util.Collection[_ <: String]): Boolean = ???
+ def clear(): Unit = ???
+ def contains(x$1: Any): Boolean = ???
+ def containsAll(x$1: java.util.Collection[_]): Boolean = ???
+ def iterator(): java.util.Iterator[String] = ???
+ def remove(x$1: Any): Boolean = ???
+ def removeAll(x$1: java.util.Collection[_]): Boolean = ???
+ def retainAll(x$1: java.util.Collection[_]): Boolean = ???
+ def toArray[T](x$1: Array[T with Object]): Array[T with Object] = ???
+
+ // Members declared in scala.collection.GenTraversableOnce
+ def isTraversableAgain: Boolean = ???
+ def toIterator: Iterator[(Set[Int], String)] = ???
+ def toStream: Stream[(Set[Int], String)] = ???
+
+ // Members declared in scala.math.Ordering
+ def compare(x: List[Int],y: List[Int]): Int = ???
+
+ // Members declared in scala.collection.TraversableOnce
+ def copyToArray[B >: (Set[Int], String)](xs: Array[B],start: Int,len: Int): Unit = ???
+ def exists(p: ((Set[Int], String)) => Boolean): Boolean = ???
+ def find(p: ((Set[Int], String)) => Boolean): Option[(Set[Int], String)] = ???
+ def forall(p: ((Set[Int], String)) => Boolean): Boolean = ???
+ def foreach[U](f: ((Set[Int], String)) => U): Unit = ???
+ def hasDefiniteSize: Boolean = ???
+ def isEmpty: Boolean = ???
+ def seq: scala.collection.TraversableOnce[(Set[Int], String)] = ???
+ def toTraversable: Traversable[(Set[Int], String)] = ???
+
+class Dingus extends Bippy[String, Set[Int], List[Int]]
+ ^
+four errors found
diff --git a/test/files/neg/abstract-report2.scala b/test/files/neg/abstract-report2.scala
new file mode 100644
index 0000000000..b6327b0766
--- /dev/null
+++ b/test/files/neg/abstract-report2.scala
@@ -0,0 +1,11 @@
+import java.util.Collection
+
+class Foo extends Collection[Int]
+
+class Bar extends Collection[List[_ <: String]]
+
+class Baz[T] extends Collection[T]
+
+trait Bippy[T1, T2, T3] extends Collection[T1] with TraversableOnce[(T2, String)] with Ordering[T3]
+
+class Dingus extends Bippy[String, Set[Int], List[Int]] \ No newline at end of file
diff --git a/test/files/neg/t2208.check b/test/files/neg/t2208.check
index a97b20cba7..64bb3a77c8 100644
--- a/test/files/neg/t2208.check
+++ b/test/files/neg/t2208.check
@@ -1,4 +1,4 @@
t2208.scala:7: error: type arguments [Any] do not conform to type Alias's type parameter bounds [X <: Test.A]
class C extends Alias[Any] // not ok, normalisation should check bounds before expanding Alias
^
-one error found \ No newline at end of file
+one error found
diff --git a/test/files/neg/t2213.check b/test/files/neg/t2213.check
index f59503ee2a..9fb3bb2eb7 100644
--- a/test/files/neg/t2213.check
+++ b/test/files/neg/t2213.check
@@ -1,15 +1,25 @@
t2213.scala:9: error: class C needs to be abstract, since:
-value y in class A of type Int is not defined
-value x in class A of type Int is not defined
-method g in class A of type => Int is not defined
-method f in class A of type => Int is not defined
+it has 4 unimplemented members.
+/** As seen from class C, the missing signatures are as follows.
+ * For convenience, these are usable as stub implementations.
+ */
+ def f: Int = ???
+ def g: Int = ???
+ val x: Int = ???
+ val y: Int = ???
+
class C extends A {}
^
t2213.scala:11: error: object creation impossible, since:
-value y in class A of type Int is not defined
-value x in class A of type Int is not defined
-method g in class A of type => Int is not defined
-method f in class A of type => Int is not defined
+it has 4 unimplemented members.
+/** As seen from object Q, the missing signatures are as follows.
+ * For convenience, these are usable as stub implementations.
+ */
+ def f: Int = ???
+ def g: Int = ???
+ val x: Int = ???
+ val y: Int = ???
+
object Q extends A { }
^
two errors found
diff --git a/test/files/neg/t856.check b/test/files/neg/t856.check
index d0bbde6c58..02978e1622 100644
--- a/test/files/neg/t856.check
+++ b/test/files/neg/t856.check
@@ -1,6 +1,14 @@
t856.scala:3: error: class ComplexRect needs to be abstract, since:
-method _2 in trait Product2 of type => Double is not defined
-method canEqual in trait Equals of type (that: Any)Boolean is not defined
+it has 2 unimplemented members.
+/** As seen from class ComplexRect, the missing signatures are as follows.
+ * For convenience, these are usable as stub implementations.
+ */
+ // Members declared in scala.Equals
+ def canEqual(that: Any): Boolean = ???
+
+ // Members declared in scala.Product2
+ def _2: Double = ???
+
class ComplexRect(val _1:Double, _2:Double) extends Complex {
^
one error found
diff --git a/test/files/neg/tcpoly_ticket2101.check b/test/files/neg/tcpoly_ticket2101.check
index eac582e8ba..ad0fd8bda2 100644
--- a/test/files/neg/tcpoly_ticket2101.check
+++ b/test/files/neg/tcpoly_ticket2101.check
@@ -1,4 +1,4 @@
tcpoly_ticket2101.scala:2: error: type arguments [T2,X] do not conform to class T's type parameter bounds [A[Y] <: T[A,B],B]
class T2[X] extends T[T2, X] // ill-typed
^
-one error found \ No newline at end of file
+one error found