mirrors now support overriden fields and methods

Previously `checkMemberOf` was blocking base fields and methods
that are overriden in receiver.getClass. Now this is fixed.

The fix also uncovered an issue with field mirrors. Currently
their `get` and `set` methods don't respect overriding and always
return field values from a base class.

After discussing this on a reflection meeting, we decided that this
behavior is desirable and that for overriding people should use
reflectMethod and then apply on getters/setters. See the discussion at:
https://github.com/scala/scala/pull/1054.

4 files changed, 76 insertions, 23 deletions

diff --git a/src/reflect/scala/reflect/api/Mirrors.scala b/src/reflect/scala/reflect/api/Mirrors.scala
index 41acd73492..8f69ab526b 100644
--- a/src/reflect/scala/reflect/api/Mirrors.scala
+++ b/src/reflect/scala/reflect/api/Mirrors.scala
@@ -33,6 +33,14 @@ trait Mirrors { self: Universe =>
     /** Reflects against a field symbol and returns a mirror
      *  that can be used to get and, if appropriate, set the value of the field.
      *
+     *  FieldMirrors are the only way to get at private[this] vals and vars and
+     *  might be useful to inspect the data of underlying Java fields.
+     *  For all other uses, it's better to go through the fields accessor.
+     *
+     *  In particular, there should be no need to ever access a field mirror
+     *  when reflecting on just the public members of a class or trait.
+     *  Note also that only accessor MethodMirrors, but not FieldMirrors will accurately reflect overriding behavior.
+     *
      *  To get a field symbol by the name of the field you would like to reflect,
      *  use `<this mirror>.symbol.typeSignature.member(newTermName(<name of the field>)).asTerm.accessed`.
      *  For further information about member lookup refer to `Symbol.typeSignature`.
@@ -107,6 +115,10 @@ trait Mirrors { self: Universe =>
      *  Scala reflection uses reflection capabilities of the underlying platform,
      *  so `FieldMirror.get` might throw platform-specific exceptions associated
      *  with getting a field or invoking a getter method of the field.
+     *
+     *  If `symbol` represents a field of a base class with respect to the class of the receiver,
+     *  and this base field is overriden in the class of the receiver, then this method will retrieve
+     *  the value of the base field. To achieve overriding behavior, use reflectMethod on an accessor.
      */
     def get: Any
 
@@ -117,6 +129,10 @@ trait Mirrors { self: Universe =>
      *  Scala reflection uses reflection capabilities of the underlying platform,
      *  so `FieldMirror.get` might throw platform-specific exceptions associated
      *  with setting a field or invoking a setter method of the field.
+     *
+     *  If `symbol` represents a field of a base class with respect to the class of the receiver,
+     *  and this base field is overriden in the class of the receiver, then this method will set
+     *  the value of the base field. To achieve overriding behavior, use reflectMethod on an accessor.
      */
     def set(value: Any): Unit
   }
diff --git a/src/reflect/scala/reflect/runtime/JavaMirrors.scala b/src/reflect/scala/reflect/runtime/JavaMirrors.scala
index f9407d5b1b..d671225c37 100644
--- a/src/reflect/scala/reflect/runtime/JavaMirrors.scala
+++ b/src/reflect/scala/reflect/runtime/JavaMirrors.scala
@@ -161,7 +161,11 @@ trait JavaMirrors extends internal.SymbolTable with api.JavaUniverse { self: Sym
       } else if (wannabe.owner == AnyValClass) {
         if (!owner.isPrimitiveValueClass && !owner.isDerivedValueClass) ErrorNotMember(wannabe, owner)
       } else {
-        if (!owner.info.member(wannabe.name).alternatives.contains(wannabe)) ErrorNotMember(wannabe, owner)
+        def isMemberOf(wannabe: Symbol, owner: ClassSymbol): Boolean = {
+          val isNonShadowedMember = owner.info.member(wannabe.name).alternatives.contains(wannabe)
+          isNonShadowedMember || owner.info.baseClasses.tail.exists(base => isMemberOf(wannabe, base.asClass))
+        }
+        if (!isMemberOf(wannabe, owner)) ErrorNotMember(wannabe, owner)
       }
     }
 
diff --git a/test/files/run/reflection-sanitychecks.check b/test/files/run/reflection-sanitychecks.check
index 4881285bc0..a1df486b51 100644
--- a/test/files/run/reflection-sanitychecks.check
+++ b/test/files/run/reflection-sanitychecks.check
@@ -1,12 +1,30 @@
-field: 1

-method: 2

-constructor #1: scala.ScalaReflectionException: expected a constructor of class C, you provided method bar

-constructor #2: an instance of class C

-class: CC

-object: java.lang.Error: inner and nested modules are not supported yet

-field: scala.ScalaReflectionException: expected a member of class C, you provided value D.foo

-method: scala.ScalaReflectionException: expected a member of class C, you provided method D.bar

-constructor #1: scala.ScalaReflectionException: expected a constructor of class C, you provided method bar

-constructor #2: scala.ScalaReflectionException: expected a constructor of class C, you provided constructor D

-class: scala.ScalaReflectionException: expected a member of class C, you provided class D.C

-object: scala.ScalaReflectionException: expected a member of class C, you provided object D.O

+=========members of C in a mirror of D=========
+field #1: 11
+method #1: 22
+field #2: 13
+method #2: 14
+constructor #1: scala.ScalaReflectionException: expected a constructor of class D, you provided method bar
+constructor #2: scala.ScalaReflectionException: expected a constructor of class D, you provided constructor C
+class: CC
+object: java.lang.Error: inner and nested modules are not supported yet
+
+=========members of D in a mirror of D=========
+field #1: 21
+method #1: 22
+field #2: 13
+method #2: 14
+constructor #1: scala.ScalaReflectionException: expected a constructor of class D, you provided method bar
+constructor #2: an instance of class D
+class: CC
+object: java.lang.Error: inner and nested modules are not supported yet
+
+=========members of E in a mirror of D=========
+field #1: scala.ScalaReflectionException: expected a member of class D, you provided value E.foo
+method #1: scala.ScalaReflectionException: expected a member of class D, you provided method E.bar
+field #2: scala.ScalaReflectionException: expected a member of class D, you provided value E.quux
+method #2: scala.ScalaReflectionException: expected a member of class D, you provided method E.baz
+constructor #1: scala.ScalaReflectionException: expected a constructor of class D, you provided method bar
+constructor #2: scala.ScalaReflectionException: expected a constructor of class D, you provided constructor E
+class: scala.ScalaReflectionException: expected a member of class D, you provided class E.C
+object: scala.ScalaReflectionException: expected a member of class D, you provided object E.O
+
diff --git a/test/files/run/reflection-sanitychecks.scala b/test/files/run/reflection-sanitychecks.scala
index b0982fc2fc..f817f23731 100644
--- a/test/files/run/reflection-sanitychecks.scala
+++ b/test/files/run/reflection-sanitychecks.scala
@@ -1,34 +1,49 @@
 class C {
-  val foo = 1
-  def bar = 2
+  val foo = 11
+  def bar = 12
+  val quux = 13
+  def baz = 14
   class C { override def toString = "CC" }
   object O { override def toString = "CO" }
   override def toString = "an instance of class C"
 }
 
-class D {
-  val foo = 3
-  def bar = 4
-  class C { override def toString = "DC" }
-  object O { override def toString = "DO" }
+class D extends C {
+  override val foo = 21
+  override def bar = 22
   override def toString = "an instance of class D"
 }
 
+class E {
+  val foo = 31
+  def bar = 32
+  val quux = 33
+  def baz = 34
+  class C { override def toString = "EC" }
+  object O { override def toString = "EO" }
+  override def toString = "an instance of class E"
+}
+
 object Test extends App {
   import scala.reflect.runtime.universe._
   import scala.reflect.runtime.{currentMirror => cm}
-  val im = cm.reflect(new C)
+  val im = cm.reflect(new D)
 
   def test(tpe: Type): Unit = {
     def failsafe(action: => Any): Any = try action catch { case ex: Throwable => ex.toString }
-    println("field: " + failsafe(im.reflectField(tpe.member(newTermName("foo")).asTerm).get))
-    println("method: " + failsafe(im.reflectMethod(tpe.member(newTermName("bar")).asMethod)()))
+    println(s"=========members of ${tpe.typeSymbol.name} in a mirror of D=========")
+    println("field #1: " + failsafe(im.reflectField(tpe.member(newTermName("foo")).asTerm).get))
+    println("method #1: " + failsafe(im.reflectMethod(tpe.member(newTermName("bar")).asMethod)()))
+    println("field #2: " + failsafe(im.reflectField(tpe.member(newTermName("quux")).asTerm).get))
+    println("method #2: " + failsafe(im.reflectMethod(tpe.member(newTermName("baz")).asMethod)()))
     println("constructor #1: " + failsafe(cm.reflectClass(im.symbol).reflectConstructor(tpe.member(newTermName("bar")).asMethod)()))
     println("constructor #2: " + failsafe(cm.reflectClass(im.symbol).reflectConstructor(tpe.member(newTermName("<init>")).asMethod)()))
     println("class: " + failsafe(im.reflectClass(tpe.member(newTypeName("C")).asClass).reflectConstructor(typeOf[C].member(newTypeName("C")).asClass.typeSignature.member(newTermName("<init>")).asMethod)()))
     println("object: " + failsafe(im.reflectModule(tpe.member(newTermName("O")).asModule).instance))
+    println()
   }
 
   test(typeOf[C])
   test(typeOf[D])
+  test(typeOf[E])
 }
\ No newline at end of file