From 0988ffa829559e905dd03e688364c4301d9cdd0c Mon Sep 17 00:00:00 2001 From: Jacob Juric Date: Sun, 10 Sep 2017 01:08:35 +1000 Subject: [PATCH 1/3] Convert "method has return statement; needs result type" to new error format with MissingReturnTypeWithReturnStatement --- .../reporting/diagnostic/ErrorMessageID.java | 1 + .../dotc/reporting/diagnostic/messages.scala | 15 +++++++++++++++ compiler/src/dotty/tools/dotc/typer/Typer.scala | 2 +- .../dotc/reporting/ErrorMessagesTests.scala | 17 +++++++++++++++++ 4 files changed, 34 insertions(+), 1 deletion(-) diff --git a/compiler/src/dotty/tools/dotc/reporting/diagnostic/ErrorMessageID.java b/compiler/src/dotty/tools/dotc/reporting/diagnostic/ErrorMessageID.java index 37c6aee6540b..b4dbd031bea9 100644 --- a/compiler/src/dotty/tools/dotc/reporting/diagnostic/ErrorMessageID.java +++ b/compiler/src/dotty/tools/dotc/reporting/diagnostic/ErrorMessageID.java @@ -96,6 +96,7 @@ public enum ErrorMessageID { WrongNumberOfParametersID, DuplicatePrivateProtectedQualifierID, ExpectedStartOfTopLevelDefinitionID, + MissingReturnTypeWithReturnStatementID, ; public int errorNumber() { diff --git a/compiler/src/dotty/tools/dotc/reporting/diagnostic/messages.scala b/compiler/src/dotty/tools/dotc/reporting/diagnostic/messages.scala index 848832dd8c0c..3768610a5026 100644 --- a/compiler/src/dotty/tools/dotc/reporting/diagnostic/messages.scala +++ b/compiler/src/dotty/tools/dotc/reporting/diagnostic/messages.scala @@ -598,6 +598,21 @@ object messages { |}""" } + case class MissingReturnTypeWithReturnStatement(method: Symbol)(implicit ctx: Context) + extends Message(MissingReturnTypeWithReturnStatementID) { + val kind = "Syntax" + val msg = hl"$method has a return statement; it needs a result type" + val explanation = + hl"""|If a method contains a ${"return"} statement, it must have a return + |type explicitly given to it. For example: + | + |${"def bad() = { return \"fail\" }"} + | + |This is illegal because bad does not have a return type given + |to it. Giving bad a return type of ${"String"} will resolve + |this error.""" + } + case class YieldOrDoExpectedInForComprehension()(implicit ctx: Context) extends Message(YieldOrDoExpectedInForComprehensionID) { val kind = "Syntax" diff --git a/compiler/src/dotty/tools/dotc/typer/Typer.scala b/compiler/src/dotty/tools/dotc/typer/Typer.scala index bd5ff738ff2d..a9055e331ac0 100644 --- a/compiler/src/dotty/tools/dotc/typer/Typer.scala +++ b/compiler/src/dotty/tools/dotc/typer/Typer.scala @@ -987,7 +987,7 @@ class Typer extends Namer with TypeAssigner with Applications with Implicits wit if (owner.isInlineMethod) (EmptyTree, errorType(em"no explicit return allowed from inline $owner", tree.pos)) else if (!owner.isCompleted) - (EmptyTree, errorType(em"$owner has return statement; needs result type", tree.pos)) + (EmptyTree, errorType(MissingReturnTypeWithReturnStatement(owner), tree.pos)) else { val from = Ident(TermRef(NoPrefix, owner.asTerm)) val proto = returnProto(owner, cx.scope) diff --git a/compiler/test/dotty/tools/dotc/reporting/ErrorMessagesTests.scala b/compiler/test/dotty/tools/dotc/reporting/ErrorMessagesTests.scala index 7749f80ecaf0..5f058f7f0b9c 100644 --- a/compiler/test/dotty/tools/dotc/reporting/ErrorMessagesTests.scala +++ b/compiler/test/dotty/tools/dotc/reporting/ErrorMessagesTests.scala @@ -955,4 +955,21 @@ class ErrorMessagesTests extends ErrorMessagesTest { assertEquals(ExpectedStartOfTopLevelDefinition(), err) } + + @Test def missingReturnTypeWithReturnStatement = + checkMessagesAfter("frontend") { + """class BadFunction { + | def bad() = { return "fail" } + |} + """.stripMargin + }.expect { (ictx, messages) => + implicit val ctx: Context = ictx + val defn = ictx.definitions + + assertMessageCount(1, messages) + + val MissingReturnTypeWithReturnStatement(method) :: Nil = messages + + assertEquals(method.name.toString, "bad") + } } From 5cb057191aaf572eccfb504461de76a298733008 Mon Sep 17 00:00:00 2001 From: Jacob Juric Date: Thu, 14 Sep 2017 17:36:28 +1000 Subject: [PATCH 2/3] Corrected additions as per review comments --- .../tools/dotc/reporting/diagnostic/messages.scala | 10 +++------- .../tools/dotc/reporting/ErrorMessagesTests.scala | 6 +----- 2 files changed, 4 insertions(+), 12 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/reporting/diagnostic/messages.scala b/compiler/src/dotty/tools/dotc/reporting/diagnostic/messages.scala index 3768610a5026..6bd4069fc2a9 100644 --- a/compiler/src/dotty/tools/dotc/reporting/diagnostic/messages.scala +++ b/compiler/src/dotty/tools/dotc/reporting/diagnostic/messages.scala @@ -603,14 +603,10 @@ object messages { val kind = "Syntax" val msg = hl"$method has a return statement; it needs a result type" val explanation = - hl"""|If a method contains a ${"return"} statement, it must have a return - |type explicitly given to it. For example: + hl"""|If a method contains a ${"return"} statement, it must have an + |explicit return type. For example: | - |${"def bad() = { return \"fail\" }"} - | - |This is illegal because bad does not have a return type given - |to it. Giving bad a return type of ${"String"} will resolve - |this error.""" + |${"def good: Int /* explicit return type */ = return 1"}""" } case class YieldOrDoExpectedInForComprehension()(implicit ctx: Context) diff --git a/compiler/test/dotty/tools/dotc/reporting/ErrorMessagesTests.scala b/compiler/test/dotty/tools/dotc/reporting/ErrorMessagesTests.scala index 5f058f7f0b9c..3ea605cc57fa 100644 --- a/compiler/test/dotty/tools/dotc/reporting/ErrorMessagesTests.scala +++ b/compiler/test/dotty/tools/dotc/reporting/ErrorMessagesTests.scala @@ -963,13 +963,9 @@ class ErrorMessagesTests extends ErrorMessagesTest { |} """.stripMargin }.expect { (ictx, messages) => - implicit val ctx: Context = ictx - val defn = ictx.definitions - assertMessageCount(1, messages) val MissingReturnTypeWithReturnStatement(method) :: Nil = messages - - assertEquals(method.name.toString, "bad") + assertEquals(method.show, "bad") } } From 14772a8e0225db139feb4a5024864aa2a5fc6d96 Mon Sep 17 00:00:00 2001 From: Jacob Juric Date: Thu, 14 Sep 2017 17:45:42 +1000 Subject: [PATCH 3/3] Corrected additions as per 2nd review comments --- .../test/dotty/tools/dotc/reporting/ErrorMessagesTests.scala | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/compiler/test/dotty/tools/dotc/reporting/ErrorMessagesTests.scala b/compiler/test/dotty/tools/dotc/reporting/ErrorMessagesTests.scala index 3ea605cc57fa..aa473912ed7d 100644 --- a/compiler/test/dotty/tools/dotc/reporting/ErrorMessagesTests.scala +++ b/compiler/test/dotty/tools/dotc/reporting/ErrorMessagesTests.scala @@ -963,9 +963,11 @@ class ErrorMessagesTests extends ErrorMessagesTest { |} """.stripMargin }.expect { (ictx, messages) => + implicit val ctx: Context = ictx + assertMessageCount(1, messages) val MissingReturnTypeWithReturnStatement(method) :: Nil = messages - assertEquals(method.show, "bad") + assertEquals(method.name.show, "bad") } }