Merge pull request #4729 from dotty-staging/fix-4709

Fix #4709: report if local implicits need type annotation

Author: Blaisorblade

compiler/src/dotty/tools/dotc/ast/Trees.scala

@@ -373,6 +373,7 @@ object Trees {
 
   /** A ValDef or DefDef tree */
   trait ValOrDefDef[-T >: Untyped] extends MemberDef[T] with WithLazyField[Tree[T]] {
+    def name: TermName
     def tpt: Tree[T]
     def unforcedRhs: LazyTree = unforced
     def rhs(implicit ctx: Context): Tree[T] = forceIfLazy

compiler/src/dotty/tools/dotc/core/TypeErrors.scala

@@ -98,28 +98,52 @@ object handleRecursive {
   }
 }
 
+/**
+ * This TypeError signals that completing denot encountered a cycle: it asked for denot.info (or similar),
+ * so it requires knowing denot already.
+ * @param denot
+ */
 class CyclicReference private (val denot: SymDenotation) extends TypeError {
+  var inImplicitSearch: Boolean = false
 
-  override def toMessage(implicit ctx: Context) = {
+  override def toMessage(implicit ctx: Context): Message = {
+    val cycleSym = denot.symbol
 
-    def errorMsg(cx: Context): Message =
+    // cycleSym.flags would try completing denot and would fail, but here we can use flagsUNSAFE to detect flags
+    // set by the parser.
+    val unsafeFlags = cycleSym.flagsUNSAFE
+    val isMethod = unsafeFlags.is(Method)
+    val isVal = !isMethod && cycleSym.isTerm
+
+    /* This CyclicReference might have arisen from asking for `m`'s type while trying to infer it.
+     * To try to diagnose this, walk the context chain searching for context in
+     * Mode.InferringReturnType for the innermost member without type
+     * annotations (!tree.tpt.typeOpt.exists).
+     */
+    def errorMsg(cx: Context): Message = {
       if (cx.mode is Mode.InferringReturnType) {
         cx.tree match {
-          case tree: untpd.DefDef if !tree.tpt.typeOpt.exists =>
-            OverloadedOrRecursiveMethodNeedsResultType(tree.name)
-          case tree: untpd.ValDef if !tree.tpt.typeOpt.exists =>
-            RecursiveValueNeedsResultType(tree.name)
+          case tree: untpd.ValOrDefDef if !tree.tpt.typeOpt.exists =>
+            if (inImplicitSearch)
+              TermMemberNeedsResultTypeForImplicitSearch(cycleSym)
+            else if (isMethod)
+              OverloadedOrRecursiveMethodNeedsResultType(cycleSym)
+            else if (isVal)
+              RecursiveValueNeedsResultType(cycleSym)
+            else
+              errorMsg(cx.outer)
           case _ =>
             errorMsg(cx.outer)
         }
       }
-      else CyclicReferenceInvolving(denot)
+      // Give up and give generic errors.
+      else if (cycleSym.is(Implicit, butNot = Method) && cycleSym.owner.isTerm)
+        CyclicReferenceInvolvingImplicit(cycleSym)
+      else
+        CyclicReferenceInvolving(denot)
+    }
 
-    val cycleSym = denot.symbol
-    if (cycleSym.is(Implicit, butNot = Method) && cycleSym.owner.isTerm)
-      CyclicReferenceInvolvingImplicit(cycleSym)
-    else
-      errorMsg(ctx)
+    errorMsg(ctx)
   }
 }
 

compiler/src/dotty/tools/dotc/reporting/diagnostic/ErrorMessageID.java

@@ -131,6 +131,7 @@ public enum ErrorMessageID {
     MatchCaseOnlyNullWarningID,
     ImportRenamedTwiceID,
     TypeTestAlwaysSucceedsID,
+    TermMemberNeedsNeedsResultTypeForImplicitSearchID
     ;
 
     public int errorNumber() {

compiler/src/dotty/tools/dotc/reporting/diagnostic/messages.scala

@@ -1260,46 +1260,50 @@ object messages {
            |""".stripMargin
   }
 
-  case class OverloadedOrRecursiveMethodNeedsResultType(method: Names.TermName)(implicit ctx: Context)
+  case class OverloadedOrRecursiveMethodNeedsResultType(cycleSym: Symbol)(implicit ctx: Context)
   extends Message(OverloadedOrRecursiveMethodNeedsResultTypeID) {
-    val kind = "Syntax"
-    val msg = hl"""overloaded or recursive method ${method} needs return type"""
+    val kind = "Cyclic"
+    val msg = hl"""overloaded or recursive $cycleSym needs return type"""
     val explanation =
-      hl"""Case 1: ${method} is overloaded
-          |If there are multiple methods named `${method}` and at least one definition of
+      hl"""Case 1: $cycleSym is overloaded
+          |If there are multiple methods named `$cycleSym` and at least one definition of
           |it calls another, you need to specify the calling method's return type.
           |
-          |Case 2: ${method} is recursive
-          |If `${method}` calls itself on any path, you need to specify its return type.
+          |Case 2: $cycleSym is recursive
+          |If `$cycleSym` calls itself on any path (even through mutual recursion), you need to specify the return type
+          |of `$cycleSym` or of a definition it's mutually recursive with.
           |""".stripMargin
   }
 
-  case class RecursiveValueNeedsResultType(value: Names.TermName)(implicit ctx: Context)
+  case class RecursiveValueNeedsResultType(cycleSym: Symbol)(implicit ctx: Context)
   extends Message(RecursiveValueNeedsResultTypeID) {
-    val kind = "Syntax"
-    val msg = hl"""recursive value ${value} needs type"""
+    val kind = "Cyclic"
+    val msg = hl"""recursive $cycleSym needs type"""
     val explanation =
-      hl"""The definition of `${value}` is recursive and you need to specify its type.
+      hl"""The definition of `$cycleSym` 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 kind = "Cyclic"
     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.
+           |To avoid this error, try giving `${denot.name}` an explicit type.
            |""".stripMargin
   }
 
   case class CyclicReferenceInvolvingImplicit(cycleSym: Symbol)(implicit ctx: Context)
   extends Message(CyclicReferenceInvolvingImplicitID) {
-    val kind = "Syntax"
+    val kind = "Cyclic"
     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.
+      hl"""|$cycleSym is declared as part of a cycle which makes it impossible for the
+           |compiler to decide upon ${cycleSym.name}'s type.
+           |This might happen when the right hand-side of $cycleSym's definition involves an implicit search.
+           |To avoid this error, try giving `${cycleSym.name}` an explicit type.
            |""".stripMargin
   }
 
@@ -2105,4 +2109,15 @@ object messages {
     }
     val explanation = ""
   }
+
+  // Relative of CyclicReferenceInvolvingImplicit and RecursiveValueNeedsResultType
+  case class TermMemberNeedsResultTypeForImplicitSearch(cycleSym: Symbol)(implicit ctx: Context)
+    extends Message(TermMemberNeedsNeedsResultTypeForImplicitSearchID) {
+    val kind = "Cyclic"
+    val msg = hl"""$cycleSym needs result type because its right-hand side attempts implicit search"""
+    val explanation =
+      hl"""|The right hand-side of $cycleSym's definition requires an implicit search at the highlighted position.
+           |To avoid this error, give `$cycleSym` an explicit type.
+           |""".stripMargin
+  }
 }

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

@@ -847,7 +847,15 @@ trait Implicits { self: Typer =>
       else i"type error: ${argument.tpe} does not conform to $pt${err.whyNoMatchStr(argument.tpe, pt)}")
     trace(s"search implicit ${pt.show}, arg = ${argument.show}: ${argument.tpe.show}", implicits, show = true) {
       assert(!pt.isInstanceOf[ExprType])
-      val result = new ImplicitSearch(pt, argument, pos).bestImplicit(contextual = true)
+      val result =
+        try {
+          new ImplicitSearch(pt, argument, pos).bestImplicit(contextual = true)
+        } catch {
+          case ce: CyclicReference =>
+            ce.inImplicitSearch = true
+            throw ce
+        }
+
       result match {
         case result: SearchSuccess =>
           result.tstate.commit()

compiler/test/dotty/tools/dotc/reporting/ErrorMessagesTests.scala

@@ -207,8 +207,8 @@ class ErrorMessagesTests extends ErrorMessagesTest {
       implicit val ctx: Context = ictx
 
       assertMessageCount(1, messages)
-      val OverloadedOrRecursiveMethodNeedsResultType(tree) :: Nil = messages
-      assertEquals("foo", tree.show)
+      val OverloadedOrRecursiveMethodNeedsResultType(cycleSym) :: Nil = messages
+      assertEquals("foo", cycleSym.name.show)
     }
 
   @Test def recursiveMethodNeedsReturnType =
@@ -223,8 +223,8 @@ class ErrorMessagesTests extends ErrorMessagesTest {
       implicit val ctx: Context = ictx
 
       assertMessageCount(1, messages)
-      val OverloadedOrRecursiveMethodNeedsResultType(tree) :: Nil = messages
-      assertEquals("i", tree.show)
+      val OverloadedOrRecursiveMethodNeedsResultType(cycleSym) :: Nil = messages
+      assertEquals("i", cycleSym.name.show)
     }
 
   @Test def recursiveValueNeedsReturnType =
@@ -239,10 +239,27 @@ class ErrorMessagesTests extends ErrorMessagesTest {
       implicit val ctx: Context = ictx
 
       assertMessageCount(1, messages)
-      val RecursiveValueNeedsResultType(tree) :: Nil = messages
-      assertEquals("i", tree.show)
+      val RecursiveValueNeedsResultType(cycleSym) :: Nil = messages
+      assertEquals("i", cycleSym.name.show)
     }
 
+  @Test def recursiveValueNeedsReturnType2 =
+    checkMessagesAfter(FrontEnd.name) {
+      """
+        |class Scope() {
+        |  lazy val i = j + 5
+        |  lazy val j = i
+        |}
+      """.stripMargin
+    }
+      .expect { (ictx, messages) =>
+        implicit val ctx: Context = ictx
+
+        assertMessageCount(1, messages)
+        val RecursiveValueNeedsResultType(cycleSym) :: Nil = messages
+        assertEquals("i", cycleSym.name.show)
+      }
+
   @Test def cyclicReferenceInvolving =
     checkMessagesAfter(FrontEnd.name) {
       """
@@ -260,7 +277,85 @@ class ErrorMessagesTests extends ErrorMessagesTest {
       assertEquals("value x", denot.show)
     }
 
-  @Test def cyclicReferenceInvolvingImplicit =
+  @Test def cyclicReferenceInvolving2 =
+    checkMessagesAfter(FrontEnd.name) {
+      """
+        |class A {
+        |  implicit val x: T = ???
+        |  type T <: x.type // error: cyclic reference involving value x
+        |}
+      """.stripMargin
+    }
+      .expect { (ictx, messages) =>
+        implicit val ctx: Context = ictx
+
+        assertMessageCount(1, messages)
+        val CyclicReferenceInvolving(denot) :: Nil = messages
+        assertEquals("value x", denot.show)
+      }
+
+  @Test def mutualRecursionre_i2001 =
+    checkMessagesAfter(FrontEnd.name) {
+      """
+        |class A {
+        |  def odd(x: Int) = if (x == 0) false else !even(x-1)
+        |  def even(x: Int) = if (x == 0) true else !odd(x-1) // error: overloaded or recursive method needs result type
+        |}
+      """.stripMargin
+    }
+      .expect { (ictx, messages) =>
+        implicit val ctx: Context = ictx
+
+        assertMessageCount(1, messages)
+        val OverloadedOrRecursiveMethodNeedsResultType(cycleSym) :: Nil = messages
+        assertEquals("odd", cycleSym.name.show)
+      }
+
+  @Test def mutualRecursion_i2001a =
+    checkMessagesAfter(FrontEnd.name) {
+      """
+        |class A {
+        |  def odd(x: Int) = if (x == 0) false else !even(x-1)
+        |  def even(x: Int) = {
+        |    def foo = {
+        |      if (x == 0) true else !odd(x-1) // error: overloaded or recursive method needs result type
+        |    }
+        |    false
+        |  }
+        |}
+      """.stripMargin
+    }
+      .expect { (ictx, messages) =>
+        implicit val ctx: Context = ictx
+
+        assertMessageCount(1, messages)
+        val OverloadedOrRecursiveMethodNeedsResultType(cycleSym) :: Nil = messages
+        assertEquals("odd", cycleSym.name.show)
+      }
+
+  @Test def mutualRecursion_i2001b =
+    checkMessagesAfter(FrontEnd.name) {
+      """
+        |class A {
+        |  def odd(x: Int) = if (x == 0) false else !even(x-1)
+        |  def even(x: Int) = {
+        |    val foo = {
+        |      if (x == 0) true else !odd(x-1) // error: overloaded or recursive method needs result type
+        |    }
+        |    false
+        |  }
+        |}
+      """.stripMargin
+    }
+      .expect { (ictx, messages) =>
+        implicit val ctx: Context = ictx
+
+        assertMessageCount(1, messages)
+        val OverloadedOrRecursiveMethodNeedsResultType(cycleSym) :: Nil = messages
+        assertEquals("odd", cycleSym.name.show)
+      }
+
+  @Test def termMemberNeedsResultTypeForImplicitSearch =
     checkMessagesAfter(FrontEnd.name) {
       """
         |object implicitDefs {
@@ -276,10 +371,53 @@ class ErrorMessagesTests extends ErrorMessagesTest {
       implicit val ctx: Context = ictx
 
       assertMessageCount(1, messages)
-      val CyclicReferenceInvolvingImplicit(tree) :: Nil = messages
-      assertEquals("x", tree.name.show)
+      val TermMemberNeedsResultTypeForImplicitSearch(cycleSym) :: Nil = messages
+      assertEquals("x", cycleSym.name.show)
     }
 
+  @Test def implicitSearchForcesImplicitRetType_i4709 =
+    checkMessagesAfter(FrontEnd.name) {
+      """
+        |import scala.language.implicitConversions
+        |
+        |class Context
+        |class ContextBase { def settings = 1 }
+        |
+        |class Test {
+        |  implicit def toBase(ctx: Context): ContextBase = ???
+        |
+        |  def test(ctx0: Context) = {
+        |    implicit val ctx = { ctx0.settings; ??? }
+        |  }
+        |}
+      """.stripMargin
+    }
+    .expect{ (ictx, messages) =>
+      implicit val ctx: Context = ictx
+
+      assertMessageCount(1, messages)
+      val TermMemberNeedsResultTypeForImplicitSearch(cycleSym) :: Nil = messages
+      assertEquals("ctx", cycleSym.name.show)
+    }
+
+  @Test def implicitSearchForcesNonImplicitRetTypeOnExplicitImport_i3253 =
+    checkMessagesAfter(FrontEnd.name) {
+      """
+        |import Test.test
+        |
+        |object Test {
+        |  def test = "  " * 10
+        |}
+      """.stripMargin
+    }
+      .expect{ (ictx, messages) =>
+        implicit val ctx: Context = ictx
+
+        assertMessageCount(1, messages)
+        val TermMemberNeedsResultTypeForImplicitSearch(cycleSym) :: Nil = messages
+        assertEquals("test", cycleSym.name.show)
+      }
+
   @Test def superQualMustBeParent =
     checkMessagesAfter(FrontEnd.name) {
       """
@@ -334,7 +472,7 @@ class ErrorMessagesTests extends ErrorMessagesTest {
       assertEquals(namedImport, prevPrec)
     }
 
-  @Test def methodDoesNotTakePrameters =
+  @Test def methodDoesNotTakeParameters =
     checkMessagesAfter(FrontEnd.name) {
       """
         |object Scope {
@@ -353,7 +491,7 @@ class ErrorMessagesTests extends ErrorMessagesTest {
       assertEquals("method foo", msg.methodSymbol.show)
     }
 
-  @Test def methodDoesNotTakeMorePrameters =
+  @Test def methodDoesNotTakeMoreParameters =
     checkMessagesAfter(FrontEnd.name) {
       """
         |object Scope{