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

Author: ennru

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}
+           |"""
+  }
+
 }

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)

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")
+    }
 }

tests/repl/overrides.check

@@ -0,0 +1,17 @@
+scala> class B { override def foo(i: Int): Unit = {}; }
+-- [E037] Reference Error: <console> -------------------------------------------
+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: <console> -------------------------------------------
+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