From 30ac9e2fdda6fddadaf42284cc45fef4272c51c2 Mon Sep 17 00:00:00 2001 From: bbarker Date: Mon, 3 Jul 2017 08:19:40 -0400 Subject: [PATCH 1/3] Partial fix for #1731 by fixing error message for overloaded method without return type --- .../src/dotty/tools/dotc/core/Contexts.scala | 2 +- .../dotc/reporting/diagnostic/messages.scala | 24 +++++++++++----- .../tools/dotc/typer/ErrorReporting.scala | 2 +- .../dotty/tools/dotc/typer/ProtoTypes.scala | 2 +- .../src/dotty/tools/dotc/typer/Typer.scala | 10 +++++-- .../dotc/reporting/ErrorMessagesTests.scala | 28 +++++++++++++++---- 6 files changed, 49 insertions(+), 19 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/core/Contexts.scala b/compiler/src/dotty/tools/dotc/core/Contexts.scala index 2e7979a98c25..567647e269c5 100644 --- a/compiler/src/dotty/tools/dotc/core/Contexts.scala +++ b/compiler/src/dotty/tools/dotc/core/Contexts.scala @@ -126,7 +126,7 @@ object Contexts { protected def sstate_=(sstate: SettingsState) = _sstate = sstate def sstate: SettingsState = _sstate - /** The current tree */ + /** The current compilation unit */ private[this] var _compilationUnit: CompilationUnit = _ protected def compilationUnit_=(compilationUnit: CompilationUnit) = _compilationUnit = compilationUnit def compilationUnit: CompilationUnit = _compilationUnit diff --git a/compiler/src/dotty/tools/dotc/reporting/diagnostic/messages.scala b/compiler/src/dotty/tools/dotc/reporting/diagnostic/messages.scala index c338cf5534c1..da40f283244b 100644 --- a/compiler/src/dotty/tools/dotc/reporting/diagnostic/messages.scala +++ b/compiler/src/dotty/tools/dotc/reporting/diagnostic/messages.scala @@ -1209,20 +1209,29 @@ object messages { |""".stripMargin } - case class OverloadedOrRecursiveMethodNeedsResultType(tree: Names.TermName)(implicit ctx: Context) + case class OverloadedOrRecursiveMethodNeedsResultType private (termName: String)(implicit ctx: Context) extends Message(OverloadedOrRecursiveMethodNeedsResultTypeID) { val kind = "Syntax" - val msg = hl"""overloaded or recursive method ${tree} needs return type""" + val msg = hl"""overloaded or recursive method $termName 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 + hl"""Case 1: $termName is overloaded + |If there are multiple methods named `$termName` 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. + |Case 2: $termName is recursive + |If `$termName` calls itself on any path, you need to specify its return type. |""".stripMargin } + object OverloadedOrRecursiveMethodNeedsResultType { + @specialized def apply[T >: Trees.Untyped](tree: NameTree[T])(implicit ctx: Context) + : OverloadedOrRecursiveMethodNeedsResultType = + OverloadedOrRecursiveMethodNeedsResultType(tree.name.toString)(ctx) + def apply(symbol: Symbol)(implicit ctx: Context) + : OverloadedOrRecursiveMethodNeedsResultType = + OverloadedOrRecursiveMethodNeedsResultType(symbol.name.toString)(ctx) + } + case class RecursiveValueNeedsResultType(tree: Names.TermName)(implicit ctx: Context) extends Message(RecursiveValueNeedsResultTypeID) { val kind = "Syntax" @@ -1320,7 +1329,8 @@ object messages { |""" } - case class MethodDoesNotTakeParameters(tree: tpd.Tree, methPartType: Types.Type)(err: typer.ErrorReporting.Errors)(implicit ctx: Context) + case class MethodDoesNotTakeParameters(tree: tpd.Tree, methPartType: Types.Type) + (err: typer.ErrorReporting.Errors)(implicit ctx: Context) extends Message(MethodDoesNotTakeParametersId) { private val more = tree match { case Apply(_, _) => " more" diff --git a/compiler/src/dotty/tools/dotc/typer/ErrorReporting.scala b/compiler/src/dotty/tools/dotc/typer/ErrorReporting.scala index d0ab39a988c4..209f79d9c424 100644 --- a/compiler/src/dotty/tools/dotc/typer/ErrorReporting.scala +++ b/compiler/src/dotty/tools/dotc/typer/ErrorReporting.scala @@ -32,7 +32,7 @@ object ErrorReporting { if (cx.mode is Mode.InferringReturnType) { cx.tree match { case tree: untpd.DefDef if !tree.tpt.typeOpt.exists => - OverloadedOrRecursiveMethodNeedsResultType(tree.name) + OverloadedOrRecursiveMethodNeedsResultType(tree) case tree: untpd.ValDef if !tree.tpt.typeOpt.exists => RecursiveValueNeedsResultType(tree.name) case _ => diff --git a/compiler/src/dotty/tools/dotc/typer/ProtoTypes.scala b/compiler/src/dotty/tools/dotc/typer/ProtoTypes.scala index e0399ffec964..967770be037a 100644 --- a/compiler/src/dotty/tools/dotc/typer/ProtoTypes.scala +++ b/compiler/src/dotty/tools/dotc/typer/ProtoTypes.scala @@ -48,7 +48,7 @@ object ProtoTypes { private def disregardProto(pt: Type)(implicit ctx: Context): Boolean = pt.dealias match { case _: OrType => true - case pt => pt.isRef(defn.UnitClass) + case ptd => ptd.isRef(defn.UnitClass) } /** Check that the result type of the current method diff --git a/compiler/src/dotty/tools/dotc/typer/Typer.scala b/compiler/src/dotty/tools/dotc/typer/Typer.scala index b16c520d5f37..780c5d71ad55 100644 --- a/compiler/src/dotty/tools/dotc/typer/Typer.scala +++ b/compiler/src/dotty/tools/dotc/typer/Typer.scala @@ -1872,8 +1872,6 @@ class Typer extends Namer with TypeAssigner with Applications with Implicits wit */ def adaptInterpolated(tree: Tree, pt: Type, original: untpd.Tree)(implicit ctx: Context): Tree = { - assert(pt.exists) - def methodStr = err.refStr(methPart(tree).tpe) def missingArgs(mt: MethodType) = { @@ -1929,7 +1927,13 @@ class Typer extends Namer with TypeAssigner with Applications with Implicits wit else tree case _ => tryInsertApplyOrImplicit(tree, pt) { - errorTree(tree, MethodDoesNotTakeParameters(tree, methPart(tree).tpe)(err)) + pt.resType match { + case IgnoredProto(WildcardType(optBounds)) + if (optBounds == NoType) && (pt.args.size == tree.productArity) => + errorTree(tree, OverloadedOrRecursiveMethodNeedsResultType(tree.symbol)) + case resType => + errorTree(tree, MethodDoesNotTakeParameters(tree, methPart(tree).tpe)(err)) + } } } diff --git a/compiler/test/dotty/tools/dotc/reporting/ErrorMessagesTests.scala b/compiler/test/dotty/tools/dotc/reporting/ErrorMessagesTests.scala index b798eff3dcef..e8bf71230138 100644 --- a/compiler/test/dotty/tools/dotc/reporting/ErrorMessagesTests.scala +++ b/compiler/test/dotty/tools/dotc/reporting/ErrorMessagesTests.scala @@ -200,7 +200,7 @@ class ErrorMessagesTests extends ErrorMessagesTest { assertTrue("expected trait", isTrait) } - @Test def overloadedMethodNeedsReturnType = + @Test def overloadedMethodNeedsReturnType = { checkMessagesAfter("frontend") { """ |class Scope() { @@ -214,9 +214,25 @@ class ErrorMessagesTests extends ErrorMessagesTest { val defn = ictx.definitions assertMessageCount(1, messages) - val OverloadedOrRecursiveMethodNeedsResultType(tree) :: Nil = messages - assertEquals("foo", tree.show) - } + val OverloadedOrRecursiveMethodNeedsResultType(treeName) :: Nil = messages + assertEquals("foo", treeName) + } + +//The following test is disabled (may be running in an unsupported phase) +// +// checkMessagesAfter("frontend") { +// """ +// |case class Foo[T](x: T) +// |object Foo { def apply[T]() = Foo(null.asInstanceOf[T]) } +// """.stripMargin +// }.expect { (ictx, messages) => +// implicit val ctx: Context = ictx +// +// assertMessageCount(1, messages) +// val OverloadedOrRecursiveMethodNeedsResultType(treeName2) :: Nil = messages +// assertEquals("Foo", treeName2) +// } + } @Test def recursiveMethodNeedsReturnType = checkMessagesAfter("frontend") { @@ -231,8 +247,8 @@ class ErrorMessagesTests extends ErrorMessagesTest { val defn = ictx.definitions assertMessageCount(1, messages) - val OverloadedOrRecursiveMethodNeedsResultType(tree) :: Nil = messages - assertEquals("i", tree.show) + val OverloadedOrRecursiveMethodNeedsResultType(treeName) :: Nil = messages + assertEquals("i", treeName) } @Test def recursiveValueNeedsReturnType = From 6c334e15b8cfc8cfb382f1e0ecd0bfccc45afc92 Mon Sep 17 00:00:00 2001 From: bbarker Date: Mon, 3 Jul 2017 09:14:20 -0400 Subject: [PATCH 2/3] fixes for review in PR #2823 --- .../dotc/reporting/diagnostic/messages.scala | 6 ++--- .../dotty/tools/dotc/typer/ProtoTypes.scala | 2 +- .../src/dotty/tools/dotc/typer/Typer.scala | 2 ++ .../dotc/reporting/ErrorMessagesTests.scala | 27 +++++++++---------- 4 files changed, 19 insertions(+), 18 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/reporting/diagnostic/messages.scala b/compiler/src/dotty/tools/dotc/reporting/diagnostic/messages.scala index da40f283244b..8b719299dd99 100644 --- a/compiler/src/dotty/tools/dotc/reporting/diagnostic/messages.scala +++ b/compiler/src/dotty/tools/dotc/reporting/diagnostic/messages.scala @@ -1224,12 +1224,12 @@ object messages { } object OverloadedOrRecursiveMethodNeedsResultType { - @specialized def apply[T >: Trees.Untyped](tree: NameTree[T])(implicit ctx: Context) + def apply[T >: Trees.Untyped](tree: NameTree[T])(implicit ctx: Context) : OverloadedOrRecursiveMethodNeedsResultType = - OverloadedOrRecursiveMethodNeedsResultType(tree.name.toString)(ctx) + OverloadedOrRecursiveMethodNeedsResultType(tree.show)(ctx) def apply(symbol: Symbol)(implicit ctx: Context) : OverloadedOrRecursiveMethodNeedsResultType = - OverloadedOrRecursiveMethodNeedsResultType(symbol.name.toString)(ctx) + OverloadedOrRecursiveMethodNeedsResultType(symbol.show)(ctx) } case class RecursiveValueNeedsResultType(tree: Names.TermName)(implicit ctx: Context) diff --git a/compiler/src/dotty/tools/dotc/typer/ProtoTypes.scala b/compiler/src/dotty/tools/dotc/typer/ProtoTypes.scala index 967770be037a..e0399ffec964 100644 --- a/compiler/src/dotty/tools/dotc/typer/ProtoTypes.scala +++ b/compiler/src/dotty/tools/dotc/typer/ProtoTypes.scala @@ -48,7 +48,7 @@ object ProtoTypes { private def disregardProto(pt: Type)(implicit ctx: Context): Boolean = pt.dealias match { case _: OrType => true - case ptd => ptd.isRef(defn.UnitClass) + case pt => pt.isRef(defn.UnitClass) } /** Check that the result type of the current method diff --git a/compiler/src/dotty/tools/dotc/typer/Typer.scala b/compiler/src/dotty/tools/dotc/typer/Typer.scala index 780c5d71ad55..c91ba23a802f 100644 --- a/compiler/src/dotty/tools/dotc/typer/Typer.scala +++ b/compiler/src/dotty/tools/dotc/typer/Typer.scala @@ -1872,6 +1872,8 @@ class Typer extends Namer with TypeAssigner with Applications with Implicits wit */ def adaptInterpolated(tree: Tree, pt: Type, original: untpd.Tree)(implicit ctx: Context): Tree = { + assert(pt.exists) + def methodStr = err.refStr(methPart(tree).tpe) def missingArgs(mt: MethodType) = { diff --git a/compiler/test/dotty/tools/dotc/reporting/ErrorMessagesTests.scala b/compiler/test/dotty/tools/dotc/reporting/ErrorMessagesTests.scala index e8bf71230138..48c13407657d 100644 --- a/compiler/test/dotty/tools/dotc/reporting/ErrorMessagesTests.scala +++ b/compiler/test/dotty/tools/dotc/reporting/ErrorMessagesTests.scala @@ -218,20 +218,19 @@ class ErrorMessagesTests extends ErrorMessagesTest { assertEquals("foo", treeName) } -//The following test is disabled (may be running in an unsupported phase) -// -// checkMessagesAfter("frontend") { -// """ -// |case class Foo[T](x: T) -// |object Foo { def apply[T]() = Foo(null.asInstanceOf[T]) } -// """.stripMargin -// }.expect { (ictx, messages) => -// implicit val ctx: Context = ictx -// -// assertMessageCount(1, messages) -// val OverloadedOrRecursiveMethodNeedsResultType(treeName2) :: Nil = messages -// assertEquals("Foo", treeName2) -// } + + checkMessagesAfter("frontend") { + """ + |case class Foo[T](x: T) + |object Foo { def apply[T]() = Foo(null.asInstanceOf[T]) } + """.stripMargin + }.expect { (ictx, messages) => + implicit val ctx: Context = ictx + + assertMessageCount(1, messages) + val OverloadedOrRecursiveMethodNeedsResultType(treeName2) :: Nil = messages + assertEquals("Foo", treeName2) + } } @Test def recursiveMethodNeedsReturnType = From 0d8d1cab66742aac8ec48c9997a27cc3a457835b Mon Sep 17 00:00:00 2001 From: bbarker Date: Mon, 3 Jul 2017 15:41:56 +0000 Subject: [PATCH 3/3] fixing 'show' usage to only show name --- .../src/dotty/tools/dotc/reporting/diagnostic/messages.scala | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/reporting/diagnostic/messages.scala b/compiler/src/dotty/tools/dotc/reporting/diagnostic/messages.scala index 8b719299dd99..84c6b8a2cf71 100644 --- a/compiler/src/dotty/tools/dotc/reporting/diagnostic/messages.scala +++ b/compiler/src/dotty/tools/dotc/reporting/diagnostic/messages.scala @@ -1226,10 +1226,10 @@ object messages { object OverloadedOrRecursiveMethodNeedsResultType { def apply[T >: Trees.Untyped](tree: NameTree[T])(implicit ctx: Context) : OverloadedOrRecursiveMethodNeedsResultType = - OverloadedOrRecursiveMethodNeedsResultType(tree.show)(ctx) + OverloadedOrRecursiveMethodNeedsResultType(tree.name.show)(ctx) def apply(symbol: Symbol)(implicit ctx: Context) : OverloadedOrRecursiveMethodNeedsResultType = - OverloadedOrRecursiveMethodNeedsResultType(symbol.show)(ctx) + OverloadedOrRecursiveMethodNeedsResultType(symbol.name.show)(ctx) } case class RecursiveValueNeedsResultType(tree: Names.TermName)(implicit ctx: Context)