Merge pull request #7850 from dotty-staging/fix-#5495

Fix #5495: Disallow lazy enums

Author: odersky

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

@@ -70,8 +70,8 @@ object DesugarEnums {
 
   /** Add implied flags to an enum class or an enum case */
   def addEnumFlags(cdef: TypeDef)(implicit ctx: Context): TypeDef =
-    if (cdef.mods.isEnumClass) cdef.withMods(cdef.mods.withFlags(cdef.mods.flags | Abstract | Sealed))
-    else if (isEnumCase(cdef)) cdef.withMods(cdef.mods.withFlags(cdef.mods.flags | Final))
+    if (cdef.mods.isEnumClass) cdef.withMods(cdef.mods.withAddedFlags(Abstract | Sealed, cdef.span))
+    else if (isEnumCase(cdef)) cdef.withMods(cdef.mods.withAddedFlags(Final, cdef.span))
     else cdef
 
   private def valuesDot(name: PreName)(implicit src: SourceFile) =
@@ -288,7 +288,7 @@ object DesugarEnums {
       val toStringDef = toStringMethLit(name.toString)
       val impl1 = cpy.Template(impl)(body = List(ordinalDef, toStringDef) ++ registerCall)
         .withAttachment(ExtendsSingletonMirror, ())
-      val vdef = ValDef(name, TypeTree(), New(impl1)).withMods(mods | EnumValue)
+      val vdef = ValDef(name, TypeTree(), New(impl1)).withMods(mods.withAddedFlags(EnumValue, span))
       flatTree(scaffolding ::: vdef :: Nil).withSpan(span)
     }
   }
@@ -304,7 +304,7 @@ object DesugarEnums {
     else {
       val (tag, scaffolding) = nextOrdinal(CaseKind.Simple)
       val creator = Apply(Ident(nme.DOLLAR_NEW), List(Literal(Constant(tag)), Literal(Constant(name.toString))))
-      val vdef = ValDef(name, enumClassRef, creator).withMods(mods | EnumValue)
+      val vdef = ValDef(name, enumClassRef, creator).withMods(mods.withAddedFlags(EnumValue, span))
       flatTree(scaffolding ::: vdef :: Nil).withSpan(span)
     }
 }

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

@@ -6,9 +6,11 @@ import core._
 import Types._, Contexts._, Constants._, Names._, Flags._
 import Symbols._, StdNames._, Trees._
 import util.{Property, SourceFile, NoSource}
+import util.Spans.Span
 import language.higherKinds
 import annotation.constructorOnly
 import annotation.internal.sharable
+import Decorators._
 
 object untpd extends Trees.Instance[Untyped] with UntypedTreeInfo {
 
@@ -233,6 +235,25 @@ object untpd extends Trees.Instance[Untyped] with UntypedTreeInfo {
       if (mods.exists(_ eq mod)) this
       else withMods(mods :+ mod)
 
+    private def compatible(flags1: FlagSet, flags2: FlagSet): Boolean =
+      flags1.isEmpty || flags2.isEmpty
+      || flags1.isTermFlags && flags2.isTermFlags
+      || flags1.isTypeFlags && flags2.isTypeFlags
+
+    /** Add `flags` to thos modifier set, checking that there are no type/term conflicts.
+     *  If there are conflicts, issue an error and return the modifiers consisting of
+     *  the added flags only. The reason to do it this way is that the added flags usually
+     *  describe the core of a construct whereas the existing set are the modifiers
+     *  given in the source.
+     */
+    def withAddedFlags(flags: FlagSet, span: Span)(given ctx: Context): Modifiers =
+      if this.flags.isAllOf(flags) then this
+      else if compatible(this.flags, flags) then this | flags
+      else
+        val what = if flags.isTermFlags then "values" else "types"
+        ctx.error(em"${(flags & ModifierFlags).flagsString} $what cannot be ${this.flags.flagsString}", ctx.source.atSpan(span))
+        Modifiers(flags)
+
     /** Modifiers with given list of Mods. It is checked that
      *  all modifiers are already accounted for in `flags` and `privateWithin`.
      */

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

@@ -339,7 +339,7 @@ object Flags {
   val (_, DefaultMethod @ _, _) = newFlags(38, "<defaultmethod>")
 
   /** Symbol is an enum class or enum case (if used with case) */
-  val (Enum @ _, _, _) = newFlags(40, "<enum>")
+  val (Enum @ _, _, _) = newFlags(40, "enum")
 
   /** An export forwarder */
   val (Exported @ _, _, _) = newFlags(41, "exported")

compiler/src/dotty/tools/dotc/parsing/Parsers.scala

@@ -2631,23 +2631,8 @@ object Parsers {
       addMod(mods, mod)
     }
 
-    private def compatible(flags1: FlagSet, flags2: FlagSet): Boolean = (
-         flags1.isEmpty
-      || flags2.isEmpty
-      || flags1.isTermFlags && flags2.isTermFlags
-      || flags1.isTypeFlags && flags2.isTypeFlags
-    )
-
-    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))
-        Modifiers(flag)
-      }
-    }
+    def addFlag(mods: Modifiers, flag: FlagSet): Modifiers =
+      mods.withAddedFlags(flag, Span(in.offset))
 
     /** Always add the syntactic `mod`, but check and conditionally add semantic `mod.flags`
      */
@@ -3345,22 +3330,29 @@ object Parsers {
       }
     }
 
+    private def checkAccessOnly(mods: Modifiers, where: String): Modifiers =
+      val mods1 = mods & (AccessFlags | Enum)
+      if mods1 ne mods then
+        syntaxError(s"Only access modifiers are allowed on enum $where")
+      mods1
+
     /**  EnumDef ::=  id ClassConstr InheritClauses [‘with’] EnumBody
      */
     def enumDef(start: Offset, mods: Modifiers): TypeDef = atSpan(start, nameStart) {
+      val mods1 = checkAccessOnly(mods, "definitions")
       val modulName = ident()
       in.endMarkerScope(modulName) {
         val clsName = modulName.toTypeName
         val constr = classConstr()
         val templ = template(constr, isEnum = true)
-        finalizeDef(TypeDef(clsName, templ), mods, start)
+        finalizeDef(TypeDef(clsName, templ), mods1, start)
       }
     }
 
     /** EnumCase = `case' (id ClassConstr [`extends' ConstrApps] | ids)
      */
     def enumCase(start: Offset, mods: Modifiers): DefTree = {
-      val mods1 = mods | EnumCase
+      val mods1 = checkAccessOnly(mods, "cases") | EnumCase
       accept(CASE)
 
       atSpan(start, nameStart) {

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

@@ -91,7 +91,7 @@ enum ErrorMessageID extends java.lang.Enum[ErrorMessageID] {
     ExpectedTopLevelDefID,
     AnonymousFunctionMissingParamTypeID,
     SuperCallsNotAllowedInlineableID,
-    ModifiersNotAllowedID,
+    UNUSED1,  // not used anymore, but left so that error numbers stay the same
     WildcardOnTypeArgumentNotAllowedOnNewID,
     FunctionTypeNeedsNonEmptyParameterListID,
     WrongNumberOfParametersID,

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

@@ -1695,27 +1695,6 @@ object messages {
     val explanation: String = "Method inlining prohibits calling superclass methods, as it may lead to confusion about which super is being called."
   }
 
-  case class ModifiersNotAllowed(flags: FlagSet, printableType: Option[String])(implicit ctx: Context)
-    extends Message(ModifiersNotAllowedID) {
-    val kind: String = "Syntax"
-    val msg: String = em"Modifier(s) `${flags.flagsString}` not allowed for ${printableType.getOrElse("combination")}"
-    val explanation: String = {
-      val first = "sealed def y: Int = 1"
-      val second = "sealed lazy class z"
-      em"""You tried to use a modifier that is inapplicable for the type of item under modification
-         |
-         |  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
-         |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
-        """
-    }
-  }
-
   case class WrongNumberOfParameters(expected: Int)(implicit ctx: Context)
     extends Message(WrongNumberOfParametersID) {
     val kind: String = "Syntax"

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

@@ -1005,24 +1005,6 @@ class ErrorMessagesTests extends ErrorMessagesTest {
       assertEquals("method bar", symbol.show)
     }
 
-  @Test def modifiersNotAllowed =
-    verifyModifiersNotAllowed("lazy trait T", "lazy", Some("trait"))
-
-  @Test def modifiersOtherThanTraitMethodVariable =
-    verifyModifiersNotAllowed("sealed lazy class x", "sealed")
-
-  private def verifyModifiersNotAllowed(code: String, modifierAssertion: String,
-                                        typeAssertion: Option[String] = None) = {
-    checkMessagesAfter(RefChecks.name)(code)
-      .expect { (ictx, messages) =>
-        implicit val ctx: Context = ictx
-        assertMessageCount(1, messages)
-        val ModifiersNotAllowed(flags, sort) :: Nil = messages
-        assertEquals(modifierAssertion, flags.flagsString)
-        assertEquals(typeAssertion, sort)
-      }
-  }
-
   @Test def wildcardOnTypeArgumentNotAllowedOnNew =
     checkMessagesAfter(RefChecks.name) {
       """

tests/neg/i5495.scala

@@ -0,0 +1,4 @@
+lazy enum LazyList[+A] {  // error: sealed abstract types cannot be lazy enum
+  case :: (head: A, tail: LazyList[A])
+  case Nil
+}

tests/neg/i5525.scala

@@ -0,0 +1,37 @@
+abstract enum Foo1 {} // error: only access modifiers allowed
+final    enum Foo2 {} // error: only access modifiers allowed
+sealed   enum Foo3 {} // error: only access modifiers allowed
+implicit enum Foo4 {} // error: only access modifiers allowed
+lazy     enum Foo5 {} // error: only access modifiers allowed
+erased   enum Foo6 {} // error: only access modifiers allowed
+override enum Foo7 {} // error: only access modifiers allowed
+inline   enum Foo8 {} // error: only access modifiers allowed
+opaque   enum Foo9 {} // error: only access modifiers allowed
+
+enum Foo10 {
+  abstract  case C1()  // error: only access modifiers allowed
+  final     case C2()  // error: only access modifiers allowed
+  sealed    case C3()  // error: only access modifiers allowed
+  implicit  case C4()  // error: only access modifiers allowed
+  lazy      case C5()  // error: only access modifiers allowed
+  erased    case C6()  // error: only access modifiers allowed
+  override  case C7()  // error: only access modifiers allowed
+  private   case C8()  // ok
+  protected case C9()  // ok
+}
+
+enum Foo11 {
+  abstract  case C1  // error: only access modifiers allowed
+  final     case C2  // error: only access modifiers allowed
+  sealed    case C3  // error: only access modifiers allowed
+  implicit  case C4  // error: only access modifiers allowed
+  lazy      case C5  // error: only access modifiers allowed
+  erased    case C6  // error: only access modifiers allowed
+  override  case C7  // error: only access modifiers allowed
+  private   case C8  // ok
+  protected case C9  // ok
+}
+
+enum Foo12 {  // error: enums must contain at least one case
+  inline    case C10() // error // error // error (inline treated as ident here)
+}

tests/neg/i6795-b.check

@@ -1,6 +1,4 @@
--- [E083] Syntax Error: tests/neg/i6795-b.scala:1:11 -------------------------------------------------------------------
+-- Error: tests/neg/i6795-b.scala:1:11 ---------------------------------------------------------------------------------
 1 |sealed def y: Int = 1 // error
   |           ^
-  |           Modifier(s) `sealed` not allowed for method
-
-longer explanation available when compiling with `-explain`
+  |            values cannot be sealed