More detail in error messages

Split error messages for recursive method and overloaded method needs type into two (but
did not solve the analysis which to show).
Make CyclicReference type error construct corresponding error message.

5 files changed, 71 insertions, 10 deletions

diff --git a/compiler/src/dotty/tools/dotc/core/Types.scala b/compiler/src/dotty/tools/dotc/core/Types.scala
index 200e94a1e..f49d65350 100644
--- a/compiler/src/dotty/tools/dotc/core/Types.scala
+++ b/compiler/src/dotty/tools/dotc/core/Types.scala
@@ -19,6 +19,7 @@ import util.Positions.{Position, NoPosition}
 import util.Stats._
 import util.{DotClass, SimpleMap}
 import reporting.diagnostic.Message
+import reporting.diagnostic.messages.CyclicReferenceInvolving
 import ast.tpd._
 import ast.TreeTypeMap
 import printing.Texts._
@@ -3856,7 +3857,7 @@ object Types {
 
   class CyclicReference private (val denot: SymDenotation)
     extends TypeError(s"cyclic reference involving $denot") {
-    def show(implicit ctx: Context) = s"cyclic reference involving ${denot.show}"
+    def toMessage(implicit ctx: Context) = CyclicReferenceInvolving(denot)
   }
 
   object CyclicReference {
diff --git a/compiler/src/dotty/tools/dotc/reporting/diagnostic/ErrorMessageID.java b/compiler/src/dotty/tools/dotc/reporting/diagnostic/ErrorMessageID.java
index 02869e752..f6d0de352 100644
--- a/compiler/src/dotty/tools/dotc/reporting/diagnostic/ErrorMessageID.java
+++ b/compiler/src/dotty/tools/dotc/reporting/diagnostic/ErrorMessageID.java
@@ -52,8 +52,10 @@ public enum ErrorMessageID {
     MixedLeftAndRightAssociativeOpsID,
     CantInstantiateAbstractClassOrTraitID,
     AnnotatedPrimaryConstructorRequiresModifierOrThisID,
-    OverloadedOrRecursiveMethodNeedsResultTypeID,
+    OverloadedMethodNeedsResultTypeID,
+    RecursiveMethodNeedsResultTypeID,
     RecursiveValueNeedsResultTypeID,
+    CyclicReferenceInvolvingID,
     CyclicReferenceInvolvingImplicitID,
     ;
 
diff --git a/compiler/src/dotty/tools/dotc/reporting/diagnostic/messages.scala b/compiler/src/dotty/tools/dotc/reporting/diagnostic/messages.scala
index 9d899923b..44ae98d82 100644
--- a/compiler/src/dotty/tools/dotc/reporting/diagnostic/messages.scala
+++ b/compiler/src/dotty/tools/dotc/reporting/diagnostic/messages.scala
@@ -18,6 +18,7 @@ import dotc.parsing.Tokens
 import printing.Highlighting._
 import printing.Formatting
 import ErrorMessageID._
+import dotty.tools.dotc.core.SymDenotations.SymDenotation
 
 object messages {
 
@@ -1161,16 +1162,25 @@ object messages {
            |""".stripMargin
   }
 
-  case class OverloadedOrRecursiveMethodNeedsResultType(tree: Names.TermName)(implicit ctx: Context)
-  extends Message(OverloadedOrRecursiveMethodNeedsResultTypeID) {
+  case class OverloadedMethodNeedsResultType(tree: Names.TermName)(implicit ctx: Context)
+  extends Message(OverloadedMethodNeedsResultTypeID) {
     val kind = "Syntax"
-    val msg = hl"""overloaded or recursive method ${tree} needs result type"""
+    val msg = hl"""overloaded method ${tree} needs result type"""
     val explanation =
       hl"""|${tree} is overloaded and at least one definition of it calls another.
            |You need to specify the calling method's return type.
          """.stripMargin
   }
 
+  case class RecursiveMethodNeedsResultType(tree: Names.TermName)(implicit ctx: Context)
+  extends Message(RecursiveMethodNeedsResultTypeID) {
+    val kind = "Syntax"
+    val msg = hl"""recursive method ${tree} needs result type"""
+    val explanation =
+      hl"""|The definition of `${tree.name}` is recursive and you need to specify its type.
+         """.stripMargin
+  }
+
   case class RecursiveValueNeedsResultType(tree: Names.TermName)(implicit ctx: Context)
   extends Message(RecursiveValueNeedsResultTypeID) {
     val kind = "Syntax"
@@ -1180,6 +1190,16 @@ object messages {
          """.stripMargin
   }
 
+  case class CyclicReferenceInvolving(denot: SymDenotation)(implicit ctx: Context)
+  extends Message(CyclicReferenceInvolvingID) {
+    val kind = "Syntax"
+    val msg = hl"""cyclic reference involving $denot"""
+    val explanation =
+      hl"""|$denot is declared as part of a cycle which makes it impossible for the
+           |compiler to decide upon ${denot.name}'s type.
+           |""".stripMargin
+  }
+
   case class CyclicReferenceInvolvingImplicit(cycleSym: Symbol)(implicit ctx: Context)
   extends Message(CyclicReferenceInvolvingImplicitID) {
     val kind = "Syntax"
diff --git a/compiler/src/dotty/tools/dotc/typer/ErrorReporting.scala b/compiler/src/dotty/tools/dotc/typer/ErrorReporting.scala
index f23b85c58..3a377bdc0 100644
--- a/compiler/src/dotty/tools/dotc/typer/ErrorReporting.scala
+++ b/compiler/src/dotty/tools/dotc/typer/ErrorReporting.scala
@@ -28,11 +28,14 @@ object ErrorReporting {
 
   def cyclicErrorMsg(ex: CyclicReference)(implicit ctx: Context) = {
     val cycleSym = ex.denot.symbol
-    def errorMsg(msg: String, cx: Context): Message =
+    def errorMsg(msg: Message, cx: Context): Message =
       if (cx.mode is Mode.InferringReturnType) {
         cx.tree match {
           case tree: untpd.DefDef if !tree.tpt.typeOpt.exists =>
-            OverloadedOrRecursiveMethodNeedsResultType(tree.name)
+            // TODO: analysis if tree is an overloaded method (or directly recursive)
+            val overloaded = true
+            if (overloaded) OverloadedMethodNeedsResultType(tree.name)
+            else RecursiveMethodNeedsResultType(tree.name)
           case tree: untpd.ValDef if !tree.tpt.typeOpt.exists =>
             RecursiveValueNeedsResultType(tree.name)
           case _ =>
@@ -43,7 +46,7 @@ object ErrorReporting {
     if (cycleSym.is(Implicit, butNot = Method) && cycleSym.owner.isTerm)
       CyclicReferenceInvolvingImplicit(cycleSym)
     else
-      errorMsg(ex.show, ctx)
+      errorMsg(ex.toMessage, ctx)
   }
 
   def wrongNumberOfTypeArgs(fntpe: Type, expectedArgs: List[TypeParamInfo], actual: List[untpd.Tree], pos: Position)(implicit ctx: Context) =
diff --git a/compiler/test/dotty/tools/dotc/reporting/ErrorMessagesTests.scala b/compiler/test/dotty/tools/dotc/reporting/ErrorMessagesTests.scala
index 257d40bb0..0c40ec477 100644
--- a/compiler/test/dotty/tools/dotc/reporting/ErrorMessagesTests.scala
+++ b/compiler/test/dotty/tools/dotc/reporting/ErrorMessagesTests.scala
@@ -6,7 +6,7 @@ import core.Contexts.Context
 import diagnostic.messages._
 import dotty.tools.dotc.parsing.Tokens
 import org.junit.Assert._
-import org.junit.Test
+import org.junit.{Ignore, Test}
 
 class ErrorMessagesTests extends ErrorMessagesTest {
   // In the case where there are no errors, we can do "expectNoErrors" in the
@@ -227,10 +227,27 @@ class ErrorMessagesTests extends ErrorMessagesTest {
       val defn = ictx.definitions
 
       assertMessageCount(1, messages)
-      val OverloadedOrRecursiveMethodNeedsResultType(tree) :: Nil = messages
+      val OverloadedMethodNeedsResultType(tree) :: Nil = messages
       assertEquals("foo", tree.show)
     }
 
+  @Test @Ignore def recursiveMethodNeedsReturnType =
+    checkMessagesAfter("frontend") {
+      """
+        |class Scope() {
+        |  def i = i + 5
+        |}
+      """.stripMargin
+    }
+    .expect { (ictx, messages) =>
+      implicit val ctx: Context = ictx
+      val defn = ictx.definitions
+
+      assertMessageCount(1, messages)
+      val RecursiveMethodNeedsResultType(tree) :: Nil = messages
+      assertEquals("i", tree.show)
+    }
+
   @Test def recursiveValueNeedsReturnType =
     checkMessagesAfter("frontend") {
       """
@@ -248,6 +265,24 @@ class ErrorMessagesTests extends ErrorMessagesTest {
       assertEquals("i", tree.show)
     }
 
+  @Test def cyclicReferenceInvolving =
+    checkMessagesAfter("frontend") {
+      """
+        |class A {
+        |  val x: T = ???
+        |  type T <: x.type // error: cyclic reference involving value x
+        |}
+      """.stripMargin
+    }
+    .expect { (ictx, messages) =>
+      implicit val ctx: Context = ictx
+      val defn = ictx.definitions
+
+      assertMessageCount(1, messages)
+      val CyclicReferenceInvolving(denot) :: Nil = messages
+      assertEquals("value x", denot.show)
+    }
+
   @Test def cyclicReferenceInvolvingImplicit =
     checkMessagesAfter("frontend") {
       """