Merge pull request #2049 from ennru/ennru_RecursiveNeedsType

Change "recursive/cyclic definitions needs type" errors to Message

Author: odersky

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._
@@ -3853,7 +3854,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

@@ -51,6 +51,10 @@ public enum ErrorMessageID {
     MixedLeftAndRightAssociativeOpsID,
     CantInstantiateAbstractClassOrTraitID,
     AnnotatedPrimaryConstructorRequiresModifierOrThisID,
+    OverloadedOrRecursiveMethodNeedsResultTypeID,
+    RecursiveValueNeedsResultTypeID,
+    CyclicReferenceInvolvingID,
+    CyclicReferenceInvolvingImplicitID,
     ;
 
     public int errorNumber() {

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 {
 
@@ -1134,7 +1135,7 @@ object messages {
   }
 
   case class AnnotatedPrimaryConstructorRequiresModifierOrThis(cls: Name)(implicit ctx: Context)
-    extends Message(AnnotatedPrimaryConstructorRequiresModifierOrThisID) {
+  extends Message(AnnotatedPrimaryConstructorRequiresModifierOrThisID) {
     val kind = "Syntax"
     val msg = hl"""${"private"}, ${"protected"}, or ${"this"} expected for annotated primary constructor"""
     val explanation =
@@ -1147,4 +1148,48 @@ object messages {
            |                           ^^^^
            |""".stripMargin
   }
+
+  case class OverloadedOrRecursiveMethodNeedsResultType(tree: Names.TermName)(implicit ctx: Context)
+  extends Message(OverloadedOrRecursiveMethodNeedsResultTypeID) {
+    val kind = "Syntax"
+    val msg = hl"""overloaded or recursive method ${tree} needs return type"""
+    val explanation =
+      hl"""Case 1: ${tree} is overloaded
+          |If there are multiple methods named `${tree.name}` and at least one definition of
+          |it calls another, you need to specify the calling method's return type.
+          |
+          |Case 2: ${tree} is recursive
+          |If `${tree.name}` calls itself on any path, you need to specify its return type.
+          |""".stripMargin
+  }
+
+  case class RecursiveValueNeedsResultType(tree: Names.TermName)(implicit ctx: Context)
+  extends Message(RecursiveValueNeedsResultTypeID) {
+    val kind = "Syntax"
+    val msg = hl"""recursive value ${tree.name} needs type"""
+    val explanation =
+      hl"""The definition of `${tree.name}` is recursive and you need to specify its type.
+          |""".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"
+    val msg = hl"""cyclic reference involving implicit $cycleSym"""
+    val explanation =
+      hl"""|This happens when the right hand-side of $cycleSym's definition involves an implicit search.
+           |To avoid this error, give `${cycleSym.name}` an explicit type.
+           |""".stripMargin
+  }
+
 }

compiler/src/dotty/tools/dotc/typer/ErrorReporting.scala

@@ -28,24 +28,22 @@ object ErrorReporting {
 
   def cyclicErrorMsg(ex: CyclicReference)(implicit ctx: Context) = {
     val cycleSym = ex.denot.symbol
-    def errorMsg(msg: String, cx: Context): String =
+    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 =>
-            em"overloaded or recursive method ${tree.name} needs result type"
+            OverloadedOrRecursiveMethodNeedsResultType(tree.name)
           case tree: untpd.ValDef if !tree.tpt.typeOpt.exists =>
-            em"recursive value ${tree.name} needs type"
+            RecursiveValueNeedsResultType(tree.name)
           case _ =>
             errorMsg(msg, cx.outer)
         }
       } else msg
 
     if (cycleSym.is(Implicit, butNot = Method) && cycleSym.owner.isTerm)
-      em"""cyclic reference involving implicit $cycleSym
-            |This happens when the right hand-side of $cycleSym's definition involves an implicit search.
-            |To avoid the error, give $cycleSym an explicit type."""
+      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
@@ -212,4 +212,97 @@ class ErrorMessagesTests extends ErrorMessagesTest {
       val AnnotatedPrimaryConstructorRequiresModifierOrThis(cls) :: Nil = messages
       assertEquals("AnotherClass", cls.show)
     }
+
+  @Test def overloadedMethodNeedsReturnType =
+    checkMessagesAfter("frontend") {
+      """
+        |class Scope() {
+        |  def foo(i: Int) = foo(i.toString)
+        |  def foo(s: String) = s
+        |}
+      """.stripMargin
+    }
+    .expect { (ictx, messages) =>
+      implicit val ctx: Context = ictx
+      val defn = ictx.definitions
+
+      assertMessageCount(1, messages)
+      val OverloadedOrRecursiveMethodNeedsResultType(tree) :: Nil = messages
+      assertEquals("foo", tree.show)
+    }
+
+  @Test 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 OverloadedOrRecursiveMethodNeedsResultType(tree) :: Nil = messages
+      assertEquals("i", tree.show)
+    }
+
+  @Test def recursiveValueNeedsReturnType =
+    checkMessagesAfter("frontend") {
+      """
+        |class Scope() {
+        |  lazy val i = i + 5
+        |}
+      """.stripMargin
+    }
+    .expect { (ictx, messages) =>
+      implicit val ctx: Context = ictx
+      val defn = ictx.definitions
+
+      assertMessageCount(1, messages)
+      val RecursiveValueNeedsResultType(tree) :: Nil = messages
+      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") {
+      """
+        |object implicitDefs {
+        |  def foo(implicit x: String) = 1
+        |  def bar() = {
+        |    implicit val x = foo
+        |    x
+        |  }
+        |}
+      """.stripMargin
+    }
+    .expect { (ictx, messages) =>
+      implicit val ctx: Context = ictx
+      val defn = ictx.definitions
+
+      assertMessageCount(1, messages)
+      val CyclicReferenceInvolvingImplicit(tree) :: Nil = messages
+      assertEquals("x", tree.name.show)
+    }
+
+
 }