Fix variance loophole for private vars

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

@@ -993,6 +993,7 @@ class Definitions {
   // Annotation classes
   @tu lazy val AllowConversionsAnnot: ClassSymbol = requiredClass("scala.annotation.allowConversions")
   @tu lazy val AnnotationDefaultAnnot: ClassSymbol = requiredClass("scala.annotation.internal.AnnotationDefault")
+  @tu lazy val AssignedNonLocallyAnnot: ClassSymbol = requiredClass("scala.annotation.internal.AssignedNonLocally")
   @tu lazy val BeanPropertyAnnot: ClassSymbol = requiredClass("scala.beans.BeanProperty")
   @tu lazy val BooleanBeanPropertyAnnot: ClassSymbol = requiredClass("scala.beans.BooleanBeanProperty")
   @tu lazy val BodyAnnot: ClassSymbol = requiredClass("scala.annotation.internal.Body")

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

@@ -864,6 +864,17 @@ object SymDenotations {
     final def isNullableClassAfterErasure(using Context): Boolean =
       isClass && !isValueClass && !is(ModuleClass) && symbol != defn.NothingClass
 
+    /** Is `pre` the same as C.this, where C is exactly the owner of this symbol,
+     *  or, if this symbol is protected, a subclass of the owner?
+     */
+    def isAccessPrivilegedThisType(pre: Type)(using Context): Boolean = pre match
+      case pre: ThisType =>
+        (pre.cls eq owner) || this.is(Protected) && pre.cls.derivesFrom(owner)
+      case pre: TermRef =>
+        pre.symbol.moduleClass == owner
+      case _ =>
+        false
+
     /** Is this definition accessible as a member of tree with type `pre`?
      *  @param pre          The type of the tree from which the selection is made
      *  @param superAccess  Access is via super
@@ -888,18 +899,6 @@ object SymDenotations {
         (linked ne NoSymbol) && accessWithin(linked)
       }
 
-      /** Is `pre` the same as C.thisThis, where C is exactly the owner of this symbol,
-       *  or, if this symbol is protected, a subclass of the owner?
-       */
-      def isCorrectThisType(pre: Type): Boolean = pre match {
-        case pre: ThisType =>
-          (pre.cls eq owner) || this.is(Protected) && pre.cls.derivesFrom(owner)
-        case pre: TermRef =>
-          pre.symbol.moduleClass == owner
-        case _ =>
-          false
-      }
-
       /** Is protected access to target symbol permitted? */
       def isProtectedAccessOK: Boolean =
         inline def fail(str: String): false =
@@ -933,7 +932,7 @@ object SymDenotations {
         || boundary.isRoot
         || (accessWithin(boundary) || accessWithinLinked(boundary)) &&
              (  !this.is(Local)
-             || isCorrectThisType(pre)
+             || isAccessPrivilegedThisType(pre)
              || canBeLocal(name, flags)
                 && {
                   resetFlag(Local)

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

@@ -1103,6 +1103,18 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer
             // allow assignments from the primary constructor to class fields
           ctx.owner.name.is(TraitSetterName) || ctx.owner.isStaticConstructor
 
+        /** Mark private variables that are assigned with a prefix other than
+         *  the `this` type of their owner with a `annotation.internal.AssignedNonLocally`
+         *  annotation. The annotation influences the variance check for these
+         *  variables, which is done at PostTyper. It will be removed after the
+         *  variance check.
+         */
+        def rememberNonLocalAssignToPrivate(sym: Symbol) = lhs1 match
+          case Select(qual, _)
+          if sym.is(Private, butNot = Local) && !sym.isAccessPrivilegedThisType(qual.tpe) =>
+            sym.addAnnotation(Annotation(defn.AssignedNonLocallyAnnot, lhs1.span))
+          case _ =>
+
         lhsCore match
           case Apply(fn, _) if fn.symbol.is(ExtensionMethod) =>
             def toSetter(fn: Tree): untpd.Tree = fn match
@@ -1136,15 +1148,16 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer
           case _ => lhsCore.tpe match {
             case ref: TermRef =>
               val lhsVal = lhsCore.denot.suchThat(!_.is(Method))
-              if (canAssign(lhsVal.symbol)) {
-                // lhsBounds: (T .. Any) as seen from lhs prefix, where T is the type of lhsVal.symbol
+              val lhsSym = lhsVal.symbol
+              if canAssign(lhsSym) then
+                rememberNonLocalAssignToPrivate(lhsSym)
+                // lhsBounds: (T .. Any) as seen from lhs prefix, where T is the type of lhsSym
                 // This ensures we do the as-seen-from on T with variance -1. Test case neg/i2928.scala
                 val lhsBounds =
-                  TypeBounds.lower(lhsVal.symbol.info).asSeenFrom(ref.prefix, lhsVal.symbol.owner)
+                  TypeBounds.lower(lhsSym.info).asSeenFrom(ref.prefix, lhsSym.owner)
                 assignType(cpy.Assign(tree)(lhs1, typed(tree.rhs, lhsBounds.loBound)))
                   .computeAssignNullable()
-              }
-              else {
+              else
                 val pre = ref.prefix
                 val setterName = ref.name.setterName
                 val setter = pre.member(setterName)
@@ -1157,7 +1170,6 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer
                   case _ =>
                     reassignmentToVal
                 }
-              }
             case TryDynamicCallType =>
               typedDynamicAssign(tree, pt)
             case tpe =>

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

@@ -148,12 +148,20 @@ class VarianceChecker(using Context) {
       case _ =>
         apply(None, info)
 
-    def validateDefinition(base: Symbol): Option[VarianceError] = {
-      val saved = this.base
+    def validateDefinition(base: Symbol): Option[VarianceError] =
+      val savedBase = this.base
       this.base = base
+      val savedVariance = variance
+      def isLocal =
+        base.isAllOf(PrivateLocal)
+        || base.is(Private) && !base.hasAnnotation(defn.AssignedNonLocallyAnnot)
+      if base.is(Mutable, butNot = Method) && !isLocal then
+        base.removeAnnotation(defn.AssignedNonLocallyAnnot)
+        variance = 0
       try checkInfo(base.info)
-      finally this.base = saved
-    }
+      finally
+        this.base = savedBase
+        this.variance = savedVariance
   }
 
   private object Traverser extends TreeTraverser {

library/src/scala/annotation/internal/AssignedNonLocally.scala

@@ -0,0 +1,7 @@
+package scala.annotation
+package internal
+
+/** An annotation to indicate that a private `var` was assigned with a prefix
+ *  other than the `this` type of its owner.
+ */
+class AssignedNonLocally() extends Annotation

tests/neg/i18588.check

@@ -0,0 +1,8 @@
+-- Error: tests/neg/i18588.scala:7:14 ----------------------------------------------------------------------------------
+7 |  private var cached: A = value // error
+  |  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+  |  covariant type A occurs in invariant position in type A of variable cached
+-- Error: tests/neg/i18588.scala:17:14 ---------------------------------------------------------------------------------
+17 |  private var cached: A = value // error
+   |  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |  covariant type A occurs in invariant position in type A of variable cached

tests/neg/i18588.scala

@@ -0,0 +1,42 @@
+class ROBox[+A](value: A) {
+  private var cached: A = value
+  def get: A = ROBox[A](value).cached
+}
+
+class Box[+A](value: A) {
+  private var cached: A = value // error
+  def get: A = cached
+
+  def put[AA >: A](value: AA): Unit = {
+    val box: Box[AA] = this
+    box.cached = value
+  }
+}
+
+class BoxWithCompanion[+A](value: A) {
+  private var cached: A = value // error
+  def get: A = cached
+}
+
+class BoxValid[+A](value: A, orig: A) {
+  private var cached: A = value // ok
+  def get: A = cached
+
+  def reset(): Unit =
+    cached = orig // ok: mutated through this prefix
+}
+
+trait Animal
+object Dog extends Animal
+object Cat extends Animal
+
+val dogBox: Box[Dog.type] = new Box(Dog)
+val _ = dogBox.put(Cat)
+val dog: Dog.type = dogBox.get
+
+
+object BoxWithCompanion {
+  def put[A](box: BoxWithCompanion[A], value: A): Unit = {
+    box.cached = value
+  }
+}