Ensure that parameter names read from bytecode aren't obliterated by …

…generic signatures.

c78d771 added code to parse the `MethodParameters` attribute from
Java classfiles. However, if `javac` emits a `Signature` attribute
after the `MethodParameters` attribute, the method info (previously
parsed from the descriptor) is overwritten with the generic info,
which doesn't keep the parameter symbols from the description-based
info.

Therefore, collect names in a buffer until all attributes are parsed,
then attach them to the parameter symbols in the final info.

Also, use the Java reflection `getParameters` method to populate
these parameter symbols in runtime reflection.

Fixes scala/bug#t10699.

src/compiler/scala/tools/nsc/Global.scala

@@ -287,7 +287,7 @@ class Global(var currentSettings: Settings, var reporter: Reporter)
       body
   }
 
-  override protected def isDeveloper = settings.developer || super.isDeveloper
+  override def isDeveloper = settings.developer || super.isDeveloper
 
   /** This is for WARNINGS which should reach the ears of scala developers
    *  whenever they occur, but are not useful for normal users. They should

src/compiler/scala/tools/nsc/symtab/classfile/ClassfileParser.scala

@@ -17,6 +17,7 @@ import scala.annotation.switch
 import scala.reflect.internal.JavaAccFlags
 import scala.reflect.internal.pickling.{ByteCodecs, PickleBuffer}
 import scala.reflect.io.NoAbstractFile
+import scala.reflect.internal.util.Collections._
 import scala.tools.nsc.util.ClassPath
 import scala.tools.nsc.io.AbstractFile
 import scala.util.control.NonFatal
@@ -802,6 +803,7 @@ abstract class ClassfileParser {
   } // sigToType
 
   def parseAttributes(sym: Symbol, symtype: Type, removedOuterParameter: Boolean = false) {
+    var paramNames: ListBuffer[Name] = null // null means we didn't find any
     def convertTo(c: Constant, pt: Type): Constant = {
       if (pt.typeSymbol == BooleanClass && c.tag == IntTag)
         Constant(c.value != 0)
@@ -843,18 +845,16 @@ abstract class ClassfileParser {
               in.skip(4)
               i += 1
             }
-            var remainingParams = sym.paramss.head // Java only has exactly one parameter list
+            paramNames = new ListBuffer()
             while (i < paramCount) {
-              val name = pool.getName(u2)
+              val rawname = pool.getName(u2)
               val access = u2
-              if (remainingParams.nonEmpty) {
-                val param = remainingParams.head
-                remainingParams = remainingParams.tail
-                if ((access & ACC_SYNTHETIC) != ACC_SYNTHETIC) { // name not synthetic
-                  param.name = name.encode
-                  param.resetFlag(SYNTHETIC)
-                }
-              }
+
+              val name =
+                if ((access & ACC_SYNTHETIC) == 0) rawname.encode
+                else nme.NO_NAME
+
+              paramNames += name
               i += 1
             }
           }
@@ -1088,8 +1088,27 @@ abstract class ClassfileParser {
       scalaSigAnnot
     }
 
+    def addParamNames(): Unit =
+      if ((paramNames ne null) && sym.hasRawInfo && sym.isMethod) {
+        val params = sym.rawInfo.params
+        (paramNames zip params).foreach {
+          case (nme.NO_NAME, _) => // param was ACC_SYNTHETIC; ignore
+          case (name, param) =>
+            param.resetFlag(SYNTHETIC)
+            param.name = name
+        }
+        if (isDeveloper && !sameLength(paramNames.toList, params)) {
+          // there's not anything we can do, but it's slightly worrisome
+          devWarning(
+            sm"""MethodParameters length mismatch while parsing $sym:
+                |  rawInfo.params: ${sym.rawInfo.params}
+                |  MethodParameters: ${paramNames.toList}""")
+        }
+      }
+
     // begin parseAttributes
     for (i <- 0 until u2) parseAttribute()
+    addParamNames()
   }
 
   /** Apply `@native`/`@transient`/`@volatile` annotations to `sym`,

src/reflect/scala/reflect/internal/SymbolTable.scala

@@ -71,7 +71,7 @@ abstract class SymbolTable extends macros.Universe
 
   def shouldLogAtThisPhase = false
   def isPastTyper = false
-  protected def isDeveloper: Boolean = settings.debug
+  def isDeveloper: Boolean = settings.debug
 
   @deprecated("use devWarning if this is really a warning; otherwise use log", "2.11.0")
   def debugwarn(msg: => String): Unit = devWarning(msg)

src/reflect/scala/reflect/internal/Types.scala

@@ -4069,19 +4069,6 @@ trait Types
   /** Are `tps1` and `tps2` lists of pairwise equivalent types? */
   def isSameTypes(tps1: List[Type], tps2: List[Type]): Boolean = (tps1 corresponds tps2)(_ =:= _)
 
-  /** True if two lists have the same length.  Since calling length on linear sequences
-   *  is O(n), it is an inadvisable way to test length equality.
-   */
-  final def sameLength(xs1: List[_], xs2: List[_]) = compareLengths(xs1, xs2) == 0
-  @tailrec final def compareLengths(xs1: List[_], xs2: List[_]): Int =
-    if (xs1.isEmpty) { if (xs2.isEmpty) 0 else -1 }
-    else if (xs2.isEmpty) 1
-    else compareLengths(xs1.tail, xs2.tail)
-
-  /** Again avoiding calling length, but the lengthCompare interface is clunky.
-   */
-  final def hasLength(xs: List[_], len: Int) = xs.lengthCompare(len) == 0
-
   private var _basetypeRecursions: Int = 0
   def basetypeRecursions = _basetypeRecursions
   def basetypeRecursions_=(value: Int) = _basetypeRecursions = value

src/reflect/scala/reflect/internal/util/Collections.scala

@@ -298,6 +298,19 @@ trait Collections {
   } catch {
     case _: IllegalArgumentException => None
   }
+
+  /** True if two lists have the same length.  Since calling length on linear sequences
+    *  is O(n), it is an inadvisable way to test length equality.
+    */
+  final def sameLength(xs1: List[_], xs2: List[_]) = compareLengths(xs1, xs2) == 0
+  @tailrec final def compareLengths(xs1: List[_], xs2: List[_]): Int =
+    if (xs1.isEmpty) { if (xs2.isEmpty) 0 else -1 }
+    else if (xs2.isEmpty) 1
+    else compareLengths(xs1.tail, xs2.tail)
+
+  /** Again avoiding calling length, but the lengthCompare interface is clunky.
+    */
+  final def hasLength(xs: List[_], len: Int) = xs.lengthCompare(len) == 0
 }
 
 object Collections extends Collections

src/reflect/scala/reflect/runtime/JavaMirrors.scala

@@ -11,7 +11,7 @@ import java.lang.{Class => jClass, Package => jPackage}
 import java.lang.reflect.{
   Method => jMethod, Constructor => jConstructor, Field => jField,
   Member => jMember, Type => jType, TypeVariable => jTypeVariable,
-  Modifier => jModifier, GenericDeclaration, GenericArrayType,
+  Parameter => jParameter, GenericDeclaration, GenericArrayType,
   ParameterizedType, WildcardType, AnnotatedElement }
 import java.lang.annotation.{Annotation => jAnnotation}
 import java.io.IOException
@@ -1143,8 +1143,8 @@ private[scala] trait JavaMirrors extends internal.SymbolTable with api.JavaUnive
       field
     }
 
-    private def setMethType(meth: Symbol, tparams: List[Symbol], paramtpes: List[Type], restpe: Type) = {
-      meth setInfo GenPolyType(tparams, MethodType(meth.owner.newSyntheticValueParams(paramtpes map objToAny), restpe))
+    private def setMethType(meth: Symbol, tparams: List[Symbol], params: List[Symbol], restpe: Type) = {
+      meth setInfo GenPolyType(tparams, MethodType(params, restpe))
     }
 
     /**
@@ -1161,9 +1161,9 @@ private[scala] trait JavaMirrors extends internal.SymbolTable with api.JavaUnive
       val meth = clazz.newMethod(newTermName(jmeth.getName), NoPosition, jmeth.scalaFlags)
       methodCache enter (jmeth, meth)
       val tparams = jmeth.getTypeParameters.toList map createTypeParameter
-      val paramtpes = jmeth.getGenericParameterTypes.toList map typeToScala
+      val params = jparamsAsScala(meth, jmeth.getParameters.toList)
       val resulttpe = typeToScala(jmeth.getGenericReturnType)
-      setMethType(meth, tparams, paramtpes, resulttpe)
+      setMethType(meth, tparams, params, resulttpe)
       propagatePackageBoundary(jmeth.javaFlags, meth)
       copyAnnotations(meth, jmeth)
       if (jmeth.javaFlags.isVarargs) meth modifyInfo arrayToRepeated
@@ -1187,16 +1187,30 @@ private[scala] trait JavaMirrors extends internal.SymbolTable with api.JavaUnive
       val constr = clazz.newConstructor(NoPosition, jconstr.scalaFlags)
       constructorCache enter (jconstr, constr)
       val tparams = jconstr.getTypeParameters.toList map createTypeParameter
-      val paramtpes = jconstr.getGenericParameterTypes.toList map typeToScala
-      setMethType(constr, tparams, paramtpes, clazz.tpe_*)
-      constr setInfo GenPolyType(tparams, MethodType(clazz.newSyntheticValueParams(paramtpes), clazz.tpe))
+      val params = jparamsAsScala(constr, jconstr.getParameters.toList)
+      setMethType(constr, tparams, params, clazz.tpe)
       propagatePackageBoundary(jconstr.javaFlags, constr)
       copyAnnotations(constr, jconstr)
       if (jconstr.javaFlags.isVarargs) constr modifyInfo arrayToRepeated
       markAllCompleted(constr)
       constr
     }
 
+    /** Transform Java parameters `params` into a list of value parameters
+      * for `meth`.
+      */
+    private def jparamsAsScala(meth: MethodSymbol, params: List[jParameter]): List[Symbol] = {
+      params.zipWithIndex.map {
+        case (param, ix) =>
+          val name =
+            if (param.isNamePresent) TermName(param.getName)
+            else nme.syntheticParamName(ix + 1)
+          meth.owner.newValueParameter(name, meth.pos)
+            .setInfo(objToAny(typeToScala(param.getParameterizedType)))
+            .setFlag(if (param.isNamePresent) 0 else SYNTHETIC)
+      }
+    }
+
 // -------------------- Scala to Java  -----------------------------------
 
     /** The Java class corresponding to given Scala class.

test/files/run/reflect-java-param-names/J_1.java

@@ -0,0 +1,8 @@
+/*
+ * javac: -parameters
+ */
+public class J_1<T> {
+    public J_1(int i, int j) {}
+    public <J extends T> void inst(int i, J j) {}
+    public static <J> void statik(int i, J j) {}
+}

test/files/run/reflect-java-param-names/Test_2.scala

@@ -0,0 +1,16 @@
+object Test extends App {
+  import reflect.runtime.universe._
+
+  val j_1 = symbolOf[J_1[_]]
+  val constr = j_1.info.decl(termNames.CONSTRUCTOR)
+  val inst   = j_1.info.decl(TermName("inst"))
+  val statik = j_1.companion.info.decl(TermName("statik"))
+
+  def check(info: Type) {
+    assert(info.paramLists.head.map(_.name) == List(TermName("i"), TermName("j")), info)
+  }
+
+  check(constr.info)
+  check(inst.info)
+  check(statik.info)
+}

test/files/run/t10699/A_1.java

@@ -0,0 +1,7 @@
+/*
+ * javac: -parameters
+ */
+public class A_1 {
+    public <T> T identity_inst(T t, T other) { return t; }
+    public static <T> T identity_static(T t, T other) { return t; }
+}

test/files/run/t10699/Test_2.scala

@@ -0,0 +1,7 @@
+object Test extends App {
+  val a_1 = new A_1
+  val t     = "t"
+  val other = "other"
+  assert(a_1.identity_inst(other = other, t = t) == t)
+  assert(A_1.identity_static(other = other, t = t) == t)
+}