From 1145f89575c7d8da3ae18505e70574486504ccde Mon Sep 17 00:00:00 2001 From: johnregan Date: Sat, 24 Jun 2017 19:20:39 +0100 Subject: [PATCH 1/3] Combines errors and simplifies logic. Adds more detail to explanation --- .../dotty/tools/dotc/parsing/Parsers.scala | 17 +++++-------- .../dotc/reporting/diagnostic/messages.scala | 25 ++++++++++++++++--- .../dotc/reporting/ErrorMessagesTests.scala | 14 ++++++++++- 3 files changed, 40 insertions(+), 16 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/parsing/Parsers.scala b/compiler/src/dotty/tools/dotc/parsing/Parsers.scala index 9a9678c4c0c9..593858684bf8 100644 --- a/compiler/src/dotty/tools/dotc/parsing/Parsers.scala +++ b/compiler/src/dotty/tools/dotc/parsing/Parsers.scala @@ -1680,21 +1680,16 @@ object Parsers { ) def addFlag(mods: Modifiers, flag: FlagSet): Modifiers = { - def incompatible(kind: String) = { - syntaxError(ModifiersNotAllowed(mods.flags, kind)) - Modifiers(flag) - } if (compatible(mods.flags, flag)) mods | flag - else flag match { - case Trait => incompatible("trait") - case Method => incompatible("method") - case Mutable => incompatible("variable") - case _ => - syntaxError(s"illegal modifier combination: ${mods.flags} and $flag") - mods + else { + syntaxError(ModifiersNotAllowed(mods.flags, getPrintableTypeFromFlagSet(flag))) + Modifiers(flag) } } + private def getPrintableTypeFromFlagSet(flag: FlagSet) = + Map(Trait -> "trait", Method -> "method", Mutable -> "variable").get(flag) + /** Always add the syntactic `mod`, but check and conditionally add semantic `mod.flags` */ def addMod(mods: Modifiers, mod: Mod): Modifiers = diff --git a/compiler/src/dotty/tools/dotc/reporting/diagnostic/messages.scala b/compiler/src/dotty/tools/dotc/reporting/diagnostic/messages.scala index 923befcf2f09..349c1e450fc3 100644 --- a/compiler/src/dotty/tools/dotc/reporting/diagnostic/messages.scala +++ b/compiler/src/dotty/tools/dotc/reporting/diagnostic/messages.scala @@ -1649,18 +1649,35 @@ object messages { val explanation = "Method inlining prohibits calling superclass methods, as it may lead to confusion about which super is being called." } - case class ModifiersNotAllowed(flags: FlagSet, sort: String)(implicit ctx: Context) + case class ModifiersNotAllowed(flags: FlagSet, printableType: Option[String])(implicit ctx: Context) extends Message(ModifiersNotAllowedID) { val kind = "Syntax" - val msg = s"modifier(s) `$flags' not allowed for $sort" + val msg = s"modifier(s) `$flags' not allowed for ${printableType.getOrElse("combination")}" val explanation = { - val code = "sealed def y: Int = 1" + val first = "sealed def y: Int = 1" + val second = "sealed lazy class z" hl"""You tried to use a modifier that is inapplicable for the type of item under modification + | + | Here is a list of modifiers and applicable item types + | + | + | Modifier ::= LocalModifier + | AccessModifier + | `override' traits, methods + | LocalModifier ::= `abstract' classes, traits + | `final' classes, traits, methods + | `sealed' classes, traits + | `implicit' variables, methods, classes + | `lazy' values + | AccessModifier ::= (`private' | `protected') [AccessQualifier] methods, classes + | AccessQualifier ::= `[' (id | `this') `]' | | |Consider the following example: - |$code + |$first |In this instance, the modifier 'sealed' is not applicable to the item type 'def' (method) + |$second + |In this instance, the modifier combination is not supported """ } } diff --git a/compiler/test/dotty/tools/dotc/reporting/ErrorMessagesTests.scala b/compiler/test/dotty/tools/dotc/reporting/ErrorMessagesTests.scala index 2c07738ca334..64cab3acf4d8 100644 --- a/compiler/test/dotty/tools/dotc/reporting/ErrorMessagesTests.scala +++ b/compiler/test/dotty/tools/dotc/reporting/ErrorMessagesTests.scala @@ -4,6 +4,8 @@ package reporting import core.Contexts.Context import diagnostic.messages._ +import dotty.tools.dotc.core.Flags +import dotty.tools.dotc.core.Flags.FlagSet import dotty.tools.dotc.core.Types.WildcardType import dotty.tools.dotc.parsing.Tokens import org.junit.Assert._ @@ -847,7 +849,17 @@ class ErrorMessagesTests extends ErrorMessagesTest { assertMessageCount(1, messages) val ModifiersNotAllowed(flags, sort) :: Nil = messages assertEquals("lazy", flags.toString) - assertEquals("trait", sort) + assertEquals(Some("trait"), sort) + } + + @Test def modifiersOtherThanTraitMethodVariable = + checkMessagesAfter("refchecks")("""sealed lazy class x""") + .expect { (ictx, messages) => + implicit val ctx: Context = ictx + assertMessageCount(1, messages) + val ModifiersNotAllowed(flags, sort) :: Nil = messages + assertEquals("sealed", flags.toString) + assertEquals(None, sort) } @Test def wildcardOnTypeArgumentNotAllowedOnNew = From d178ff393c935acce3dd5b4b0012b96490242b5b Mon Sep 17 00:00:00 2001 From: johnregan Date: Sat, 24 Jun 2017 19:47:56 +0100 Subject: [PATCH 2/3] refactors to DRY prinicpal --- .../dotc/reporting/ErrorMessagesTests.scala | 20 +++++++++---------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/compiler/test/dotty/tools/dotc/reporting/ErrorMessagesTests.scala b/compiler/test/dotty/tools/dotc/reporting/ErrorMessagesTests.scala index 64cab3acf4d8..a36f34a1fa80 100644 --- a/compiler/test/dotty/tools/dotc/reporting/ErrorMessagesTests.scala +++ b/compiler/test/dotty/tools/dotc/reporting/ErrorMessagesTests.scala @@ -843,24 +843,22 @@ class ErrorMessagesTests extends ErrorMessagesTest { } @Test def modifiersNotAllowed = - checkMessagesAfter("refchecks")("""lazy trait T""") - .expect { (ictx, messages) => - implicit val ctx: Context = ictx - assertMessageCount(1, messages) - val ModifiersNotAllowed(flags, sort) :: Nil = messages - assertEquals("lazy", flags.toString) - assertEquals(Some("trait"), sort) - } + verifyModifiersNotAllowed("lazy trait T", "lazy", Some("trait")) @Test def modifiersOtherThanTraitMethodVariable = - checkMessagesAfter("refchecks")("""sealed lazy class x""") + verifyModifiersNotAllowed("sealed lazy class x", "sealed") + + private def verifyModifiersNotAllowed(code: String, modifierAssertion: String, + typeAssertion: Option[String] = None) = { + checkMessagesAfter("refchecks")(code) .expect { (ictx, messages) => implicit val ctx: Context = ictx assertMessageCount(1, messages) val ModifiersNotAllowed(flags, sort) :: Nil = messages - assertEquals("sealed", flags.toString) - assertEquals(None, sort) + assertEquals(modifierAssertion, flags.toString) + assertEquals(typeAssertion, sort) } + } @Test def wildcardOnTypeArgumentNotAllowedOnNew = checkMessagesAfter("refchecks") { From 8a187c5f0fbc179ae11f3866f18096d76aa367b8 Mon Sep 17 00:00:00 2001 From: johnregan Date: Mon, 26 Jun 2017 20:44:33 +0100 Subject: [PATCH 3/3] move method to local scope, replace paraphrasing with url to documentation --- .../src/dotty/tools/dotc/parsing/Parsers.scala | 8 ++++---- .../dotc/reporting/diagnostic/messages.scala | 16 ++-------------- 2 files changed, 6 insertions(+), 18 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/parsing/Parsers.scala b/compiler/src/dotty/tools/dotc/parsing/Parsers.scala index 593858684bf8..b73080465231 100644 --- a/compiler/src/dotty/tools/dotc/parsing/Parsers.scala +++ b/compiler/src/dotty/tools/dotc/parsing/Parsers.scala @@ -1680,16 +1680,16 @@ object Parsers { ) def addFlag(mods: Modifiers, flag: FlagSet): Modifiers = { + def getPrintableTypeFromFlagSet = + Map(Trait -> "trait", Method -> "method", Mutable -> "variable").get(flag) + if (compatible(mods.flags, flag)) mods | flag else { - syntaxError(ModifiersNotAllowed(mods.flags, getPrintableTypeFromFlagSet(flag))) + syntaxError(ModifiersNotAllowed(mods.flags, getPrintableTypeFromFlagSet)) Modifiers(flag) } } - private def getPrintableTypeFromFlagSet(flag: FlagSet) = - Map(Trait -> "trait", Method -> "method", Mutable -> "variable").get(flag) - /** Always add the syntactic `mod`, but check and conditionally add semantic `mod.flags` */ def addMod(mods: Modifiers, mod: Mod): Modifiers = diff --git a/compiler/src/dotty/tools/dotc/reporting/diagnostic/messages.scala b/compiler/src/dotty/tools/dotc/reporting/diagnostic/messages.scala index 349c1e450fc3..2d6efd8e0f50 100644 --- a/compiler/src/dotty/tools/dotc/reporting/diagnostic/messages.scala +++ b/compiler/src/dotty/tools/dotc/reporting/diagnostic/messages.scala @@ -1658,20 +1658,8 @@ object messages { val second = "sealed lazy class z" hl"""You tried to use a modifier that is inapplicable for the type of item under modification | - | Here is a list of modifiers and applicable item types - | - | - | Modifier ::= LocalModifier - | AccessModifier - | `override' traits, methods - | LocalModifier ::= `abstract' classes, traits - | `final' classes, traits, methods - | `sealed' classes, traits - | `implicit' variables, methods, classes - | `lazy' values - | AccessModifier ::= (`private' | `protected') [AccessQualifier] methods, classes - | AccessQualifier ::= `[' (id | `this') `]' - | + | Please see the official Scala Language Specification section on modifiers: + | https://www.scala-lang.org/files/archive/spec/2.11/05-classes-and-objects.html#modifiers | |Consider the following example: |$first