Implement access checking for enum cases

Previously, enum cases moved from the enum to its companion
object could "accidentally" refer to definitions defined
in the object, but inaccessible from the enum. We now
check that no such accesses occur.

Author: odersky

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

@@ -68,8 +68,8 @@ object DesugarEnums {
 
   /** Add implied flags to an enum class or an enum case */
   def addEnumFlags(cdef: TypeDef)(implicit ctx: Context) =
-    if (cdef.mods.hasMod[Mod.Enum]) cdef.withFlags(cdef.mods.flags | Abstract | Sealed)
-    else if (isEnumCase(cdef)) cdef.withFlags(cdef.mods.flags | Final)
+    if (cdef.mods.hasMod[Mod.Enum]) cdef.withMods(cdef.mods.withFlags(cdef.mods.flags | Abstract | Sealed))
+    else if (isEnumCase(cdef)) cdef.withMods(cdef.mods.withFlags(cdef.mods.flags | Final))
     else cdef
 
   private def valuesDot(name: String) = Select(Ident(nme.DOLLAR_VALUES), name.toTermName)

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

@@ -28,6 +28,7 @@ import Decorators._
 import Uniques._
 import ErrorReporting.{err, errorType}
 import config.Printers.typr
+import NameKinds.DefaultGetterName
 
 import collection.mutable
 import SymDenotations.NoCompleter
@@ -741,9 +742,100 @@ trait Checking {
     tp.foreachPart(check, stopAtStatic = true)
     tp
   }
+
+  /** Check that all non-synthetic references of the form `<ident>` or
+   *  `this.<ident>` in `tree` that refer to a member of `badOwner` are
+   *  `allowed`.
+   */
+  def checkRefsLegal(tree: tpd.Tree, badOwner: Symbol, allowed: (Name, Symbol) => Boolean, where: String)(implicit ctx: Context): Unit = {
+    tree.foreachSubTree { tree =>
+      tree match {
+        case Ident(_) | Select(This(_), _) if tree.pos.isSourceDerived =>
+          val sym = tree.symbol
+          if (sym.maybeOwner == badOwner && !allowed(tree.asInstanceOf[RefTree].name, sym))
+            ctx.error(i"illegal reference to $sym from $where: $tree // ${tree.toString}", tree.pos)
+        case _ =>
+      }
+    }
+  }
+
+  /** Check that all case classes that extend `scala.Enum` are `enum` cases */
+  def checkEnum(cdef: untpd.TypeDef, cls: Symbol)(implicit ctx: Context): Unit = {
+    import untpd.modsDeco
+    def isEnumAnonCls =
+      cls.isAnonymousClass &&
+      cls.owner.isTerm &&
+      (cls.owner.flagsUNSAFE.is(Case) || cls.owner.name == nme.DOLLAR_NEW)
+    if (!cdef.mods.hasMod[untpd.Mod.EnumCase] && !isEnumAnonCls)
+      ctx.error(em"normal case $cls in ${cls.owner} cannot extend an enum", cdef.pos)
+  }
+
+  /** Check that all references coming from enum cases in an enum companion object
+   *  are legal.
+   *  @param  cdef     the enum companion object class
+   *  @param  enumCtx  the context immediately enclosing the corresponding enum
+   */
+  private def checkEnumCaseRefsLegal(cdef: TypeDef, enumCtx: Context)(implicit ctx: Context): Unit = {
+    def check(tree: Tree) = {
+      // allow access to `sym` if a typedIdent just outside the enclosing enum
+      // would have produced the same symbol without errors
+      def allowAccess(name: Name, sym: Symbol): Boolean = {
+        val testCtx = enumCtx.fresh.setNewTyperState()
+        val ref = ctx.typer.typedIdent(untpd.Ident(name), WildcardType)(testCtx)
+        ref.symbol == sym && !testCtx.reporter.hasErrors
+      }
+      checkRefsLegal(tree, cdef.symbol, allowAccess, "enum case")
+    }
+    cdef.rhs match {
+      case impl: Template =>
+        for (stat <- impl.body)
+          if (stat.symbol.is(Case))
+            stat match {
+              case TypeDef(_, Template(DefDef(_, tparams, vparamss, _, _), parents, _, _)) =>
+                tparams.foreach(check)
+                vparamss.foreach(_.foreach(check))
+                parents.foreach(check)
+              case vdef: ValDef =>
+                vdef.rhs match {
+                  case Block((clsDef @ TypeDef(_, impl: Template)) :: Nil, _)
+                  if clsDef.symbol.isAnonymousClass =>
+                    impl.parents.foreach(check)
+                  case _ =>
+                }
+              case _ =>
+            }
+          else if (stat.symbol.is(Module) && stat.symbol.linkedClass.is(Case))
+            stat match {
+              case TypeDef(_, impl: Template) =>
+                for ((defaultGetter @
+                      DefDef(DefaultGetterName(nme.CONSTRUCTOR, _), _, _, _, _)) <- impl.body)
+                  check(defaultGetter.rhs)
+              case _ =>
+            }
+      case _ =>
+    }
+  }
+
+  /** Check all enum cases in all enum companions in `stats` for legal accesses.
+   *  @param  enumContexts  a map from`enum` symbols to the contexts enclosing their definitions
+   */
+  def checkEnumCompanions(stats: List[Tree], enumContexts: collection.Map[Symbol, Context])(implicit ctx: Context): List[Tree] = {
+    for (stat @ TypeDef(_, _) <- stats)
+      if (stat.symbol.is(Module))
+        for (enumContext <- enumContexts.get(stat.symbol.linkedClass))
+          checkEnumCaseRefsLegal(stat, enumContext)
+    stats
+  }
+}
+
+trait ReChecking extends Checking {
+  import tpd._
+  override def checkEnum(cdef: untpd.TypeDef, cls: Symbol)(implicit ctx: Context): Unit = ()
+  override def checkRefsLegal(tree: tpd.Tree, badOwner: Symbol, allowed: (Name, Symbol) => Boolean, where: String)(implicit ctx: Context): Unit = ()
+  override def checkEnumCompanions(stats: List[Tree], enumContexts: collection.Map[Symbol, Context])(implicit ctx: Context): List[Tree] = stats
 }
 
-trait NoChecking extends Checking {
+trait NoChecking extends ReChecking {
   import tpd._
   override def checkNonCyclic(sym: Symbol, info: TypeBounds, reportErrors: Boolean)(implicit ctx: Context): Type = info
   override def checkValue(tree: Tree, proto: Type)(implicit ctx: Context): tree.type = tree

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

@@ -22,7 +22,7 @@ import config.Printers.typr
  *
  *  Otherwise, everything is as in Typer.
  */
-class ReTyper extends Typer {
+class ReTyper extends Typer with ReChecking {
   import tpd._
 
   private def assertTyped(tree: untpd.Tree)(implicit ctx: Context): Unit =

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

@@ -1406,7 +1406,7 @@ class Typer extends Namer
     if (sym.isInlineMethod) Inliner.registerInlineInfo(sym, _ => rhs1)
 
     assignType(cpy.DefDef(ddef)(name, tparams1, vparamss1, tpt1, rhs1), sym)
-    //todo: make sure dependent method types do not depend on implicits or by-name params
+      //todo: make sure dependent method types do not depend on implicits or by-name params
   }
 
   def typedTypeDef(tdef: untpd.TypeDef, sym: Symbol)(implicit ctx: Context): Tree = track("typedTypeDef") {
@@ -1506,6 +1506,7 @@ class Typer extends Namer
       checkVariance(impl1)
       if (!cls.is(AbstractOrTrait) && !ctx.isAfterTyper)
         checkRealizableBounds(cls, cdef.namePos)
+      if (cls.is(Case) && cls.derivesFrom(defn.EnumClass)) checkEnum(cdef, cls)
       val cdef1 = assignType(cpy.TypeDef(cdef)(name, impl1), cls)
       if (ctx.phase.isTyper && cdef1.tpe.derivesFrom(defn.DynamicClass) && !ctx.dynamicsEnabled) {
         val isRequired = parents1.exists(_.tpe.isRef(defn.DynamicClass))
@@ -1813,6 +1814,8 @@ class Typer extends Namer
 
   def typedStats(stats: List[untpd.Tree], exprOwner: Symbol)(implicit ctx: Context): List[tpd.Tree] = {
     val buf = new mutable.ListBuffer[Tree]
+    val enumContexts = new mutable.HashMap[Symbol, Context]
+      // A map from `enum` symbols to the contexts enclosing their definitions
     @tailrec def traverse(stats: List[untpd.Tree])(implicit ctx: Context): List[Tree] = stats match {
       case (imp: untpd.Import) :: rest =>
         val imp1 = typed(imp)
@@ -1827,6 +1830,12 @@ class Typer extends Namer
               case mdef1: DefDef if Inliner.hasBodyToInline(mdef1.symbol) =>
                 buf ++= inlineExpansion(mdef1)
               case mdef1 =>
+                import untpd.modsDeco
+                mdef match {
+                  case mdef: untpd.TypeDef if mdef.mods.hasMod[untpd.Mod.Enum] =>
+                    enumContexts(mdef1.symbol) = ctx
+                  case _ =>
+                }
                 buf += mdef1
             }
             traverse(rest)
@@ -1846,7 +1855,7 @@ class Typer extends Namer
       val exprOwnerOpt = if (exprOwner == ctx.owner) None else Some(exprOwner)
       ctx.withProperty(ExprOwner, exprOwnerOpt)
     }
-    traverse(stats)(localCtx)
+    checkEnumCompanions(traverse(stats)(localCtx), enumContexts)
   }
 
   /** Given an inline method `mdef`, the method rewritten so that its body

tests/neg/enums.scala

@@ -17,6 +17,13 @@ enum E3[-T <: Ordered[T]] {
   case C // error: cannot determine type argument
 }
 
+enum E4 {
+  case C
+}
+
+case class C4() extends E4 // error: cannot extend enum
+case object O4 extends E4 // error: cannot extend enum
+
 enum Option[+T] {
   case Some(x: T)
   case None

tests/neg/enumsAccess.scala

@@ -0,0 +1,86 @@
+package enums
+
+object test1 {
+
+  enum E4 {
+    case C1(x: INT) // error: illegal reference
+    case C2(x: Int = defaultX) // error: illegal reference
+    case C3[T <: INT] // error: illegal reference
+  }
+
+  object E4 {
+    type INT = Integer
+    val defaultX = 2
+  }
+}
+
+object test2 {
+  import E5._
+  object E5 {
+    type INT = Integer
+    val defaultX = 2
+  }
+
+  enum E5 {
+    case C1(x: INT) // ok
+    case C2(x: Int = defaultX) // ok
+    case C3[T <: INT] // ok
+  }
+}
+
+object test3 {
+  object E5 {
+    type INT = Integer
+    val defaultX = 2
+  }
+
+  import E5._
+
+  enum E5 {
+    case C1(x: INT) // ok
+    case C2(x: Int = defaultX)// ok
+    case C3[T <: INT] // ok
+  }
+}
+
+object test4 {
+
+  enum E5 {
+    case C1(x: INT) // error: illegal reference
+    case C2(x: Int = defaultX) // error: illegal reference
+    case C3[T <: INT] // error: illegal reference
+  }
+
+  import E5._
+
+  object E5 {
+    type INT = Integer
+    val defaultX = 2
+  }
+}
+
+object test5 {
+  enum E5[T](x: T) {
+    case C3() extends E5[INT](defaultX)// error: illegal reference  // error: illegal reference
+    case C4 extends E5[INT](defaultX) // error: illegal reference  // error: illegal reference
+    case C5 extends E5[E5[_]](E5.this) // error: type mismatch
+  }
+
+  object E5 {
+    type INT = Integer
+    val defaultX = 2
+  }
+}
+
+object test6 {
+  import E5._
+  enum E5[T](x: T) {
+    case C3() extends E5[INT](defaultX) // ok
+    case C4 extends E5[INT](defaultX) // ok
+  }
+
+  object E5 {
+    type INT = Integer
+    val defaultX = 2
+  }
+}

tests/pos/enum-List-control.scala

@@ -1,11 +1,17 @@
 abstract sealed class List[T] extends Enum
 object List {
-  final case class Cons[T](x: T, xs: List[T]) extends List[T] {
+  final class Cons[T](x: T, xs: List[T]) extends List[T] {
     def enumTag = 0
   }
-  final case class Nil[T]() extends List[T] {
+  object Cons {
+    def apply[T](x: T, xs: List[T]): List[T] = new Cons(x, xs)
+  }
+  final class Nil[T]() extends List[T] {
     def enumTag = 1
   }
+  object Nil {
+    def apply[T](): List[T] = new Nil()
+  }
 }
 object Test {
   import List._

tests/run/enum-Color.scala

@@ -1,5 +1,6 @@
 enum Color {
   case Red, Green, Blue
+  class Color  // Just to throw a spanner in the works
 }
 
 object Test {

tests/run/generic/Enum.scala

@@ -0,0 +1,25 @@
+package generic
+
+trait Enum {
+  def enumTag: Int
+}
+
+object runtime {
+  class EnumValues[E <: Enum] {
+    private[this] var myMap: Map[Int, E] = Map()
+    private[this] var fromNameCache: Map[String, E] = null
+
+    def register(v: E) = {
+      require(!myMap.contains(v.enumTag))
+      myMap = myMap.updated(v.enumTag, v)
+      fromNameCache = null
+    }
+
+    def fromInt: Map[Int, E] = myMap
+    def fromName: Map[String, E] = {
+      if (fromNameCache == null) fromNameCache = myMap.values.map(v => v.toString -> v).toMap
+      fromNameCache
+    }
+    def values: Iterable[E] = myMap.values
+  }
+}