Warn about surprising shadowing.

It's hidden behind -Xlint and pretty specific, but makes me feel better
anyway. References SI-4762, no review.

Author: paulp

src/compiler/scala/reflect/internal/Symbols.scala

@@ -1923,6 +1923,7 @@ trait Symbols extends api.Symbols { self: SymbolTable =>
       else if (owner.isRefinementClass) ExplicitFlags & ~OVERRIDE
       else ExplicitFlags
 
+    def accessString = hasFlagsToString(PRIVATE | PROTECTED | LOCAL)
     def defaultFlagString = hasFlagsToString(defaultFlagMask)
     private def defStringCompose(infoString: String) = compose(
       defaultFlagString,

src/compiler/scala/tools/nsc/typechecker/SuperAccessors.scala

@@ -191,13 +191,33 @@ abstract class SuperAccessors extends transform.Transform with transform.TypingT
           mayNeedProtectedAccessor(sel, args, false)
 
         case sel @ Select(qual @ This(_), name) =>
-           // direct calls to aliases of param accessors to the superclass in order to avoid
-           // duplicating fields.
-           if (sym.isParamAccessor && sym.alias != NoSymbol) {
+          // warn if they are selecting a private[this] member which
+          // also exists in a superclass, because they may be surprised
+          // to find out that a constructor parameter will shadow a
+          // field. See SI-4762.
+          if (settings.lint.value) {
+            if (sym.isPrivateLocal && sym.paramss.isEmpty) {
+              qual.symbol.ancestors foreach { parent =>
+                parent.info.decls filterNot (x => x.isPrivate || x.hasLocalFlag) foreach { m2 =>
+                  if (sym.name == m2.name && m2.isGetter && m2.accessed.isMutable) {
+                    unit.warning(sel.pos,
+                        sym.accessString + " " + sym.fullLocationString + " shadows mutable " + m2.name
+                      + " inherited from " + m2.owner + ".  Changes to " + m2.name + " will not be visible within "
+                      + sym.owner + " - you may want to give them distinct names."
+                    )
+                  }
+                }
+              }
+            }
+          }
+
+          // direct calls to aliases of param accessors to the superclass in order to avoid
+          // duplicating fields.
+          if (sym.isParamAccessor && sym.alias != NoSymbol) {
             val result = localTyper.typed {
-                Select(
-                  Super(qual, tpnme.EMPTY/*qual.symbol.info.parents.head.symbol.name*/) setPos qual.pos,
-                  sym.alias) setPos tree.pos
+              Select(
+              Super(qual, tpnme.EMPTY/*qual.symbol.info.parents.head.symbol.name*/) setPos qual.pos,
+              sym.alias) setPos tree.pos
             }
             debuglog("alias replacement: " + tree + " ==> " + result);//debug
             localTyper.typed(gen.maybeMkAsInstanceOf(transformSuperSelect(result), sym.tpe, sym.alias.tpe, true))

test/files/neg/t4762.check

@@ -0,0 +1,7 @@
+t4762.scala:15: error: private[this] value x in class B shadows mutable x inherited from class A.  Changes to x will not be visible within class B - you may want to give them distinct names.
+    /* (99,99) */  (this.x, this.y),
+                         ^
+t4762.scala:48: error: private[this] value x in class Derived shadows mutable x inherited from class Base.  Changes to x will not be visible within class Derived - you may want to give them distinct names.
+  class Derived( x : Int ) extends Base( x ) { override def toString = x.toString }
+                                                                       ^
+two errors found

test/files/neg/t4762.flags

@@ -0,0 +1 @@
+-Xlint -Xfatal-warnings

test/files/neg/t4762.scala

@@ -0,0 +1,51 @@
+// https://issues.scala-lang.org/browse/SI-4762
+
+// In A, x and y are -1.
+class A(var x: Int) {
+  val y: Int = -1
+}
+
+// In B, x and y are 99 and private[this], implicitly so
+// in the case of x.
+class B(x: Int) extends A(-1) {
+  private[this] def y: Int = 99
+
+  // Three distinct results.
+  def f = List(
+    /* (99,99) */  (this.x, this.y),
+    /* (-1,99) */  ((this: B).x, (this: B).y),
+    /* (-1,-1) */  ((this: A).x, (this: A).y)
+  )
+
+  // The 99s tell us we are reading the private[this]
+  // data of a different instance.
+  def g(b: B) = List(
+    /* (-1,99) */  (b.x, b.y),
+    /* (-1,99) */  ((b: B).x, (b: B).y),
+    /* (-1,-1) */  ((b: A).x, (b: A).y)
+  )
+}
+
+object Test {
+  def f(x: A)  = /* -2 */  x.x + x.y
+  def g1(x: B) = /* -2 */  (x: A).x + (x: A).y
+  def g2(x: B) = (x: B).x + (x: B).y
+  // java.lang.IllegalAccessError: tried to access method B.y()I from class Test$
+
+  def main(args: Array[String]): Unit = {
+    val b = new B(99)
+    b.f foreach println
+    b.g(new B(99)) foreach println
+
+    println(f(b))
+    println(g1(b))
+    println(g2(b))
+  }
+}
+
+class bug4762 {
+  class Base( var x : Int ) { def increment() { x = x + 1 } }
+  class Derived( x : Int ) extends Base( x ) { override def toString = x.toString }
+
+  val derived = new Derived( 1 )
+}