From 8bdc91f7a5fc3efd93b6be255ec1bfb83787c69b Mon Sep 17 00:00:00 2001 From: Enno Date: Sun, 12 Feb 2017 17:54:33 +0100 Subject: Change 'overrides nothing' to report via Message (see #1965) (#1968) * Change 'overrides nothing' to report via Message, split into two different messages * Change 'overrides nothing' to report via Message, split into two different messages --- .../tools/dotc/reporting/diagnostic/messages.scala | 32 ++++++++++++++++ .../src/dotty/tools/dotc/typer/RefChecks.scala | 12 +++--- .../tools/dotc/reporting/ErrorMessagesTests.scala | 43 ++++++++++++++++++++++ tests/repl/overrides.check | 17 +++++++++ 4 files changed, 98 insertions(+), 6 deletions(-) create mode 100644 tests/repl/overrides.check diff --git a/compiler/src/dotty/tools/dotc/reporting/diagnostic/messages.scala b/compiler/src/dotty/tools/dotc/reporting/diagnostic/messages.scala index 89cd2cd8f..c25c49597 100644 --- a/compiler/src/dotty/tools/dotc/reporting/diagnostic/messages.scala +++ b/compiler/src/dotty/tools/dotc/reporting/diagnostic/messages.scala @@ -1002,4 +1002,36 @@ object messages { |${typeCode} |""" } + + case class OverridesNothing(member: Symbol)(implicit ctx: Context) + extends Message(37) { + val kind = "Reference" + val msg = hl"""${member} overrides nothing""" + + val explanation = + hl"""|There must be a field or method with the name `${member.name}` in a super + |class of `${member.owner}` to override it. Did you misspell it? + |Are you extending the right classes? + |""" + } + + case class OverridesNothingButNameExists(member: Symbol, existing: List[Denotations.SingleDenotation])(implicit ctx: Context) + extends Message(38) { + val kind = "Reference" + val msg = hl"""${member} has a different signature than the overridden declaration""" + + val existingDecl = existing.map(_.showDcl).mkString(" \n") + + val explanation = + hl"""|There must be a non-final field or method with the name `${member.name}` and the + |same parameter list in a super class of `${member.owner}` to override it. + | + | ${member.showDcl} + | + |The super classes of `${member.owner}` contain the following members + |named `${member.name}`: + | ${existingDecl} + |""" + } + } diff --git a/compiler/src/dotty/tools/dotc/typer/RefChecks.scala b/compiler/src/dotty/tools/dotc/typer/RefChecks.scala index e113399c5..ada53047a 100644 --- a/compiler/src/dotty/tools/dotc/typer/RefChecks.scala +++ b/compiler/src/dotty/tools/dotc/typer/RefChecks.scala @@ -21,6 +21,9 @@ import DenotTransformers._ object RefChecks { import tpd._ + import reporting.diagnostic.Message + import reporting.diagnostic.messages._ + private def isDefaultGetter(name: Name): Boolean = name.isTermName && name.asTermName.defaultGetterIndex >= 0 @@ -622,15 +625,12 @@ object RefChecks { if (member.isAnyOverride && !(clazz.thisType.baseClasses exists (hasMatchingSym(_, member)))) { // for (bc <- clazz.info.baseClasses.tail) Console.println("" + bc + " has " + bc.info.decl(member.name) + ":" + bc.info.decl(member.name).tpe);//DEBUG - val nonMatching = clazz.info.member(member.name).altsWith(alt => alt.owner != clazz && !alt.is(Final)) - def issueError(suffix: String) = - ctx.error(i"$member overrides nothing$suffix", member.pos) + val nonMatching = clazz.info.member(member.name).altsWith(alt => alt.owner != clazz) nonMatching match { case Nil => - issueError("") + ctx.error(OverridesNothing(member), member.pos) case ms => - val superSigs = ms.map(_.showDcl).mkString("\n") - issueError(s".\nNote: the super classes of ${member.owner} contain the following, non final members named ${member.name}:\n${superSigs}") + ctx.error(OverridesNothingButNameExists(member, ms), member.pos) } member.resetFlag(Override) member.resetFlag(AbsOverride) diff --git a/compiler/test/dotty/tools/dotc/reporting/ErrorMessagesTests.scala b/compiler/test/dotty/tools/dotc/reporting/ErrorMessagesTests.scala index f43d559d4..be641fe15 100644 --- a/compiler/test/dotty/tools/dotc/reporting/ErrorMessagesTests.scala +++ b/compiler/test/dotty/tools/dotc/reporting/ErrorMessagesTests.scala @@ -42,4 +42,47 @@ class ErrorMessagesTests extends ErrorMessagesTest { // is a type reference to `java.lang.String` assert(expected.dealias =:= defn.StringType, s"expected was: $expected") } + + @Test def overridesNothing = + checkMessagesAfter("refchecks") { + """ + |object Foo { + | override def bar: Unit = {} + |} + """.stripMargin + } + .expect { (ictx, messages) => + implicit val ctx: Context = ictx + val defn = ictx.definitions + + assertMessageCount(1, messages) + val OverridesNothing(member) :: Nil = messages + assertEquals("bar", member.name.show) + } + + @Test def overridesNothingDifferentSignature = + checkMessagesAfter("refchecks") { + """ + |class Bar { + | def bar(s: String): Unit = {} + | def bar(s: Int): Unit = {} + | final def bar(s: Long): Unit = {} + |} + |object Foo extends Bar { + | override def bar: Unit = {} + |} + """.stripMargin + } + .expect { (ictx, messages) => + implicit val ctx: Context = ictx + val defn = ictx.definitions + + assertMessageCount(1, messages) + val OverridesNothingButNameExists(member, sameName) :: Nil = messages + // check expected context data + assertEquals("bar", member.name.show) + assertEquals(3, sameName.size) + assert(sameName.forall(_.symbol.name.show == "bar"), + "at least one method had an unexpected name") + } } diff --git a/tests/repl/overrides.check b/tests/repl/overrides.check new file mode 100644 index 000000000..2424c80ce --- /dev/null +++ b/tests/repl/overrides.check @@ -0,0 +1,17 @@ +scala> class B { override def foo(i: Int): Unit = {}; } +-- [E037] Reference Error: ------------------------------------------- +4 |class B { override def foo(i: Int): Unit = {}; } + | ^ + | method foo overrides nothing + +longer explanation available when compiling with `-explain` +scala> class A { def foo: Unit = {}; } +defined class A +scala> class B extends A { override def foo(i: Int): Unit = {}; } +-- [E038] Reference Error: ------------------------------------------- +5 |class B extends A { override def foo(i: Int): Unit = {}; } + | ^ + | method foo has a different signature than the overridden declaration + +longer explanation available when compiling with `-explain` +scala> :quit -- cgit v1.2.3