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.

Author: ennru

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 {

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,
     ;
 

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"

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) =

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") {
       """