SI-5984 improves error reporting in JavaMirrors

Factors out error raising code and introduces a special exception class
for Scala reflection errors.

Also adds membership sanity checks to reflectXXX. Previously
reflectField, reflectMethod, reflectClass and reflectModule
in InstanceMirror didn't check that the symbols being passed to them
actually correspond to some member of the related class.

12 files changed, 80 insertions, 28 deletions

diff --git a/src/library/scala/reflect/package.scala b/src/library/scala/reflect/package.scala
index 9f9d4089c4..d97f2ec633 100644
--- a/src/library/scala/reflect/package.scala
+++ b/src/library/scala/reflect/package.scala
@@ -65,3 +65,6 @@ package object reflect {
   @deprecated("Use `@scala.beans.ScalaBeanInfo` instead", "2.10.0")
   type ScalaBeanInfo = scala.beans.ScalaBeanInfo
 }
+
+/** An exception that indicates an error during Scala reflection */
+case class ScalaReflectionException(msg: String) extends Exception(msg)
diff --git a/src/reflect/scala/reflect/runtime/JavaMirrors.scala b/src/reflect/scala/reflect/runtime/JavaMirrors.scala
index 1c5ea9caba..e7bebd624c 100644
--- a/src/reflect/scala/reflect/runtime/JavaMirrors.scala
+++ b/src/reflect/scala/reflect/runtime/JavaMirrors.scala
@@ -107,15 +107,28 @@ trait JavaMirrors extends internal.SymbolTable with api.JavaUniverse { self: Sym
 
 // ----------- Implementations of mirror operations and classes  -------------------
 
+    private def ErrorInnerClass(wannabe: Symbol) = throw new ScalaReflectionException(s"$wannabe is an inner class, use reflectClass on an InstanceMirror to obtain its ClassMirror")
+    private def ErrorInnerModule(wannabe: Symbol) = throw new ScalaReflectionException(s"$wannabe is an inner module, use reflectModule on an InstanceMirror to obtain its ModuleMirror")
+    private def ErrorStaticClass(wannabe: Symbol) = throw new ScalaReflectionException(s"$wannabe is a static class, use reflectClass on a RuntimeMirror to obtain its ClassMirror")
+    private def ErrorStaticModule(wannabe: Symbol) = throw new ScalaReflectionException(s"$wannabe is a static module, use reflectModule on a RuntimeMirror to obtain its ModuleMirror")
+    private def ErrorNotMember(wannabe: Symbol, owner: Symbol) = throw new ScalaReflectionException(s"expected a member of $owner, you provided ${wannabe.kind} ${wannabe.fullName}")
+    private def ErrorNotField(wannabe: Symbol) = throw new ScalaReflectionException(s"expected a field or an accessor method symbol, you provided $wannabe}")
+    private def ErrorNonExistentField(wannabe: Symbol) = throw new ScalaReflectionException(s"""
+      |Scala field ${wannabe.name} isn't represented as a Java field, neither it has a Java accessor method
+      |note that private parameters of class constructors don't get mapped onto fields and/or accessors,
+      |unless they are used outside of their declaring constructors.
+    """.trim.stripMargin)
+    private def ErrorSetImmutableField(wannabe: Symbol) = throw new ScalaReflectionException(s"cannot set an immutable field ${wannabe.name}")
+
     def reflect(obj: Any): InstanceMirror = new JavaInstanceMirror(obj.asInstanceOf[AnyRef])
 
     def reflectClass(cls: ClassSymbol): ClassMirror = {
-      if (!cls.isStatic) throw new Error("this is an inner class, use reflectClass on an InstanceMirror to obtain its ClassMirror")
+      if (!cls.isStatic) ErrorInnerClass(cls)
       new JavaClassMirror(null, cls)
     }
 
     def reflectModule(mod: ModuleSymbol): ModuleMirror = {
-      if (!mod.isStatic) throw new Error("this is an inner module, use reflectModule on an InstanceMirror to obtain its ModuleMirror")
+      if (!mod.isStatic) ErrorInnerModule(mod)
       new JavaModuleMirror(null, mod)
     }
 
@@ -127,13 +140,16 @@ trait JavaMirrors extends internal.SymbolTable with api.JavaUniverse { self: Sym
 
     def moduleSymbol(rtcls: RuntimeClass): ModuleSymbol = classToScala(rtcls).companionModule.asModuleSymbol
 
+    private def checkMemberOf(wannabe: Symbol, owner: Symbol) =
+      if (!owner.info.member(wannabe.name).alternatives.contains(wannabe)) ErrorNotMember(wannabe, owner)
+
     private class JavaInstanceMirror(obj: AnyRef)
             extends InstanceMirror {
       def instance = obj
       def symbol = wholemirror.classSymbol(obj.getClass)
       def reflectField(field: TermSymbol): FieldMirror = {
-        // [Eugene+++] check whether `field` represents a member of a `symbol`
-        if ((field.isMethod && !field.isAccessor) || field.isModule) throw new Error(s"expected a field or accessor method symbol, you provided a ${field.kind} symbol")
+        checkMemberOf(field, symbol)
+        if ((field.isMethod && !field.isAccessor) || field.isModule) ErrorNotField(field)
         val name =
           if (field.isGetter) nme.getterToLocal(field.name)
           else if (field.isSetter) nme.getterToLocal(nme.setterToGetter(field.name))
@@ -141,27 +157,22 @@ trait JavaMirrors extends internal.SymbolTable with api.JavaUniverse { self: Sym
         val field1 = (field.owner.info decl name).asTermSymbol
         try fieldToJava(field1)
         catch {
-          case _: NoSuchFieldException =>
-            throw new Error(s"""
-              |this Scala field isn't represented as a Java field, neither it has a Java accessor method
-              |note that private parameters of class constructors don't get mapped onto fields and/or accessors,
-              |unless they are used outside of their declaring constructors.
-            """.trim.stripMargin)
+          case _: NoSuchFieldException => ErrorNonExistentField(field1)
         }
         new JavaFieldMirror(obj, field1)
       }
       def reflectMethod(method: MethodSymbol): MethodMirror = {
-        // [Eugene+++] check whether `method` represents a member of a `symbol`
+        checkMemberOf(method, symbol)
         new JavaMethodMirror(obj, method)
       }
       def reflectClass(cls: ClassSymbol): ClassMirror = {
-        // [Eugene+++] check whether `cls` represents a member of a `symbol`
-        if (cls.isStatic) throw new Error("this is a static class, use reflectClass on a RuntimeMirror to obtain its ClassMirror")
+        if (cls.isStatic) ErrorStaticClass(cls)
+        checkMemberOf(cls, symbol)
         new JavaClassMirror(instance, cls)
       }
       def reflectModule(mod: ModuleSymbol): ModuleMirror = {
-        // [Eugene+++] check whether `mod` represents a member of a `symbol`
-        if (mod.isStatic) throw new Error("this is a static module, use reflectModule on a RuntimeMirror to obtain its ModuleMirror")
+        if (mod.isStatic) ErrorStaticModule(mod)
+        checkMemberOf(mod, symbol)
         new JavaModuleMirror(instance, mod)
       }
     }
@@ -175,7 +186,7 @@ trait JavaMirrors extends internal.SymbolTable with api.JavaUniverse { self: Sym
       }
       def get = jfield.get(receiver)
       def set(value: Any) = {
-        if (!symbol.isMutable) throw new Error("cannot set an immutable field")
+        if (!symbol.isMutable) ErrorSetImmutableField(symbol)
         jfield.set(receiver, value)
       }
     }
@@ -193,7 +204,7 @@ trait JavaMirrors extends internal.SymbolTable with api.JavaUniverse { self: Sym
             case nme.length => jArray.getLength(receiver)
             case nme.apply => jArray.get(receiver, args(0).asInstanceOf[Int])
             case nme.update => jArray.set(receiver, args(0).asInstanceOf[Int], args(1))
-            case _ => throw new Error(s"unexpected array method $symbol")
+            case _ => assert(false, s"unexpected array method: $symbol")
           }
         else
           jmeth.invoke(receiver, args.asInstanceOf[Seq[AnyRef]]: _*)
diff --git a/test/files/run/reflection-constructormirror-inner-badpath.check b/test/files/run/reflection-constructormirror-inner-badpath.check
index 28b936eca1..2fb0610ad6 100644
--- a/test/files/run/reflection-constructormirror-inner-badpath.check
+++ b/test/files/run/reflection-constructormirror-inner-badpath.check
@@ -1,2 +1,2 @@
-this is an inner class, use reflectClass on an InstanceMirror to obtain its ClassMirror

+class R is an inner class, use reflectClass on an InstanceMirror to obtain its ClassMirror

 ()

diff --git a/test/files/run/reflection-constructormirror-nested-badpath.check b/test/files/run/reflection-constructormirror-nested-badpath.check
index 9ceb603dc2..acd21df9c0 100644
--- a/test/files/run/reflection-constructormirror-nested-badpath.check
+++ b/test/files/run/reflection-constructormirror-nested-badpath.check
@@ -1,2 +1,2 @@
-this is a static class, use reflectClass on a RuntimeMirror to obtain its ClassMirror

+class R is a static class, use reflectClass on a RuntimeMirror to obtain its ClassMirror

 ()

diff --git a/test/files/run/reflection-constructormirror-toplevel-badpath.check b/test/files/run/reflection-constructormirror-toplevel-badpath.check
index 9ceb603dc2..acd21df9c0 100644
--- a/test/files/run/reflection-constructormirror-toplevel-badpath.check
+++ b/test/files/run/reflection-constructormirror-toplevel-badpath.check
@@ -1,2 +1,2 @@
-this is a static class, use reflectClass on a RuntimeMirror to obtain its ClassMirror

+class R is a static class, use reflectClass on a RuntimeMirror to obtain its ClassMirror

 ()

diff --git a/test/files/run/reflection-fieldmirror-ctorparam.check b/test/files/run/reflection-fieldmirror-ctorparam.check
index 7ae2cec81e..31f6491b14 100644
--- a/test/files/run/reflection-fieldmirror-ctorparam.check
+++ b/test/files/run/reflection-fieldmirror-ctorparam.check
@@ -1,3 +1,3 @@
-class java.lang.Error: this Scala field isn't represented as a Java field, neither it has a Java accessor method
+class scala.ScalaReflectionException: Scala field x isn't represented as a Java field, neither it has a Java accessor method
 note that private parameters of class constructors don't get mapped onto fields and/or accessors,
 unless they are used outside of their declaring constructors.

diff --git a/test/files/run/reflection-fieldmirror-getsetval.check b/test/files/run/reflection-fieldmirror-getsetval.check
index 707bbcccce..e1927f68d0 100644
--- a/test/files/run/reflection-fieldmirror-getsetval.check
+++ b/test/files/run/reflection-fieldmirror-getsetval.check
@@ -1,2 +1,2 @@
 42

-cannot set an immutable field

+cannot set an immutable field x 

diff --git a/test/files/run/reflection-modulemirror-inner-badpath.check b/test/files/run/reflection-modulemirror-inner-badpath.check
index d3fe43336e..1e990ec900 100644
--- a/test/files/run/reflection-modulemirror-inner-badpath.check
+++ b/test/files/run/reflection-modulemirror-inner-badpath.check
@@ -1,2 +1,2 @@
-this is an inner module, use reflectModule on an InstanceMirror to obtain its ModuleMirror
-()
+object R is an inner module, use reflectModule on an InstanceMirror to obtain its ModuleMirror

+()

diff --git a/test/files/run/reflection-modulemirror-nested-badpath.check b/test/files/run/reflection-modulemirror-nested-badpath.check
index 16a5b10390..f7980b9986 100644
--- a/test/files/run/reflection-modulemirror-nested-badpath.check
+++ b/test/files/run/reflection-modulemirror-nested-badpath.check
@@ -1,2 +1,2 @@
-this is a static module, use reflectModule on a RuntimeMirror to obtain its ModuleMirror
-()
+object R is a static module, use reflectModule on a RuntimeMirror to obtain its ModuleMirror

+()

diff --git a/test/files/run/reflection-modulemirror-toplevel-badpath.check b/test/files/run/reflection-modulemirror-toplevel-badpath.check
index 16a5b10390..f7980b9986 100644
--- a/test/files/run/reflection-modulemirror-toplevel-badpath.check
+++ b/test/files/run/reflection-modulemirror-toplevel-badpath.check
@@ -1,2 +1,2 @@
-this is a static module, use reflectModule on a RuntimeMirror to obtain its ModuleMirror
-()
+object R is a static module, use reflectModule on a RuntimeMirror to obtain its ModuleMirror

+()

diff --git a/test/files/run/reflection-sanitychecks.check b/test/files/run/reflection-sanitychecks.check
new file mode 100644
index 0000000000..d977e0ed66
--- /dev/null
+++ b/test/files/run/reflection-sanitychecks.check
@@ -0,0 +1,8 @@
+field: 1

+method: 2

+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

+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

diff --git a/test/files/run/reflection-sanitychecks.scala b/test/files/run/reflection-sanitychecks.scala
new file mode 100644
index 0000000000..a6a24088a4
--- /dev/null
+++ b/test/files/run/reflection-sanitychecks.scala
@@ -0,0 +1,30 @@
+class C {
+  val foo = 1
+  def bar = 2
+  class C { override def toString = "CC" }
+  object O { override def toString = "CO" }
+}
+
+class D {
+  val foo = 3
+  def bar = 4
+  class C { override def toString = "DC" }
+  object O { override def toString = "DO" }
+}
+
+object Test extends App {
+  import scala.reflect.runtime.universe._
+  import scala.reflect.runtime.{currentMirror => cm}
+  val im = cm.reflect(new C)
+
+  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")).asTermSymbol).get))
+    println("method: " + failsafe(im.reflectMethod(tpe.member(newTermName("bar")).asMethodSymbol)()))
+    println("class: " + failsafe(im.reflectClass(tpe.member(newTypeName("C")).asClassSymbol).reflectConstructor(typeOf[C].member(newTypeName("C")).asClassSymbol.typeSignature.member(newTermName("<init>")).asMethodSymbol)()))
+    println("object: " + failsafe(im.reflectModule(tpe.member(newTermName("O")).asModuleSymbol).instance))
+  }
+
+  test(typeOf[C])
+  test(typeOf[D])
+}
\ No newline at end of file