SI-9437 Emit and support parameter names in class files

JEP 118 added a MethodParameters attribute to the class file spec which
holds the parameter names of methods when compiling Java code with
`javac -parameters`.

We emit parameter names by default now.

11 files changed, 194 insertions, 6 deletions

diff --git a/src/compiler/scala/tools/nsc/backend/jvm/BCodeHelpers.scala b/src/compiler/scala/tools/nsc/backend/jvm/BCodeHelpers.scala
index 1ec9ba8a95..1b5ece772c 100644
--- a/src/compiler/scala/tools/nsc/backend/jvm/BCodeHelpers.scala
+++ b/src/compiler/scala/tools/nsc/backend/jvm/BCodeHelpers.scala
@@ -841,6 +841,17 @@ abstract class BCodeHelpers extends BCodeIdiomatic with BytecodeWriters {
       }
     }
 
+    /*
+     * must-single-thread
+     */
+    def emitParamNames(jmethod: asm.MethodVisitor, params: List[Symbol]) = {
+      for (param <- params) {
+        var access = asm.Opcodes.ACC_FINAL
+        if (param.isArtifact)
+          access |= asm.Opcodes.ACC_SYNTHETIC
+        jmethod.visitParameter(param.name.decoded, access)
+      }
+    }
   } // end of trait BCAnnotGen
 
   trait BCJGenSigGen {
diff --git a/src/compiler/scala/tools/nsc/backend/jvm/BCodeSkelBuilder.scala b/src/compiler/scala/tools/nsc/backend/jvm/BCodeSkelBuilder.scala
index 7180c25d25..9dea21154f 100644
--- a/src/compiler/scala/tools/nsc/backend/jvm/BCodeSkelBuilder.scala
+++ b/src/compiler/scala/tools/nsc/backend/jvm/BCodeSkelBuilder.scala
@@ -512,7 +512,7 @@ abstract class BCodeSkelBuilder extends BCodeHelpers {
     /*
      * must-single-thread
      */
-    def initJMethod(flags: Int, paramAnnotations: List[List[AnnotationInfo]]) {
+    def initJMethod(flags: Int, params: List[Symbol]) {
 
       val jgensig = getGenericSignature(methSymbol, claszSymbol)
       addRemoteExceptionAnnot(isCZRemote, hasPublicBitSet(flags), methSymbol)
@@ -532,10 +532,9 @@ abstract class BCodeSkelBuilder extends BCodeHelpers {
         mkArray(thrownExceptions)
       ).asInstanceOf[asm.tree.MethodNode]
 
-      // TODO param names: (m.params map (p => javaName(p.sym)))
-
+      emitParamNames(mnode, params)
       emitAnnotations(mnode, others)
-      emitParamAnnotations(mnode, paramAnnotations)
+      emitParamAnnotations(mnode, params.map(_.annotations))
 
     } // end of method initJMethod
 
@@ -574,8 +573,7 @@ abstract class BCodeSkelBuilder extends BCodeHelpers {
         if (isNative)                asm.Opcodes.ACC_NATIVE     else 0  // native methods of objects are generated in mirror classes
       )
 
-      // TODO needed? for(ann <- m.symbol.annotations) { ann.symbol.initialize }
-      initJMethod(flags, params.map(p => p.symbol.annotations))
+      initJMethod(flags, params.map(_.symbol))
 
       /* Add method-local vars for LabelDef-params.
        *
diff --git a/src/compiler/scala/tools/nsc/symtab/classfile/ClassfileParser.scala b/src/compiler/scala/tools/nsc/symtab/classfile/ClassfileParser.scala
index 406411f21f..fffd48d145 100644
--- a/src/compiler/scala/tools/nsc/symtab/classfile/ClassfileParser.scala
+++ b/src/compiler/scala/tools/nsc/symtab/classfile/ClassfileParser.scala
@@ -816,6 +816,23 @@ abstract class ClassfileParser {
           val c1 = convertTo(c, symtype)
           if (c1 ne null) sym.setInfo(ConstantType(c1))
           else devWarning(s"failure to convert $c to $symtype")
+        case tpnme.MethodParametersATTR =>
+          def readParamNames(): Unit = {
+            import tools.asm.Opcodes.ACC_SYNTHETIC
+            val paramCount = u1
+            var i = 0
+            while (i < paramCount) {
+              val name = pool.getName(u2)
+              val access = u2
+              if ((access & ACC_SYNTHETIC) != ACC_SYNTHETIC) { // name not synthetic
+                val params = sym.paramss.head // Java only has exactly one parameter list
+                params(i).name = name.encode
+                params(i).resetFlag(SYNTHETIC)
+              }
+              i += 1
+            }
+          }
+          readParamNames()
         case tpnme.ScalaSignatureATTR =>
           if (!isScalaAnnot) {
             devWarning(s"symbol ${sym.fullName} has pickled signature in attribute")
diff --git a/src/reflect/scala/reflect/internal/StdNames.scala b/src/reflect/scala/reflect/internal/StdNames.scala
index d16d431f6e..80ed597fb7 100644
--- a/src/reflect/scala/reflect/internal/StdNames.scala
+++ b/src/reflect/scala/reflect/internal/StdNames.scala
@@ -291,6 +291,7 @@ trait StdNames {
     final val DeprecatedATTR: NameType             = "Deprecated"
     final val ExceptionsATTR: NameType             = "Exceptions"
     final val InnerClassesATTR: NameType           = "InnerClasses"
+    final val MethodParametersATTR: NameType       = "MethodParameters"
     final val RuntimeAnnotationATTR: NameType      = "RuntimeVisibleAnnotations"   // RetentionPolicy.RUNTIME
     final val ScalaATTR: NameType                  = "Scala"
     final val ScalaSignatureATTR: NameType         = "ScalaSig"
diff --git a/test/files/run/t9437a.check b/test/files/run/t9437a.check
new file mode 100644
index 0000000000..564213c587
--- /dev/null
+++ b/test/files/run/t9437a.check
@@ -0,0 +1,10 @@
+name: a; isNamePresent: true; isSynthetic: false
+name: _; isNamePresent: true; isSynthetic: false
+name: ***; isNamePresent: true; isSynthetic: false
+name: unary_!; isNamePresent: true; isSynthetic: false
+name: ABC; isNamePresent: true; isSynthetic: false
+name: a; isNamePresent: true; isSynthetic: false
+name: _; isNamePresent: true; isSynthetic: false
+name: ***; isNamePresent: true; isSynthetic: false
+name: unary_!; isNamePresent: true; isSynthetic: false
+name: ABC; isNamePresent: true; isSynthetic: false
diff --git a/test/files/run/t9437a/Test.scala b/test/files/run/t9437a/Test.scala
new file mode 100644
index 0000000000..a86c17b646
--- /dev/null
+++ b/test/files/run/t9437a/Test.scala
@@ -0,0 +1,20 @@
+class Foo(a: Int, `_`: String, *** : Long, `unary_!` : Float, ABC: Double) {
+  def bar(a: Int, `_`: String, *** : Long, `unary_!` : Float, ABC: Double) = null
+}
+
+object Test extends App {
+  val constrParams = classOf[Foo].getConstructors.head.getParameters
+  val methodParams = classOf[Foo].getDeclaredMethods.head.getParameters
+
+  def printParams(params: Array[java.lang.reflect.Parameter]) = {
+    params.foreach { param =>
+      println(s"name: ${param.getName}; isNamePresent: ${param.isNamePresent}; isSynthetic: ${param.isSynthetic}")
+    }
+  }
+
+  printParams(constrParams)
+  printParams(methodParams)
+
+  val foo = new Foo(a = 1, `_` = "2", *** = 3L, `unary_!` = 4.0f, ABC = 5.0)
+  foo.bar(a = 1, `_` = "2", *** = 3L, `unary_!` = 4.0f, ABC = 5.0)
+}
diff --git a/test/files/run/t9437b.check b/test/files/run/t9437b.check
new file mode 100644
index 0000000000..564213c587
--- /dev/null
+++ b/test/files/run/t9437b.check
@@ -0,0 +1,10 @@
+name: a; isNamePresent: true; isSynthetic: false
+name: _; isNamePresent: true; isSynthetic: false
+name: ***; isNamePresent: true; isSynthetic: false
+name: unary_!; isNamePresent: true; isSynthetic: false
+name: ABC; isNamePresent: true; isSynthetic: false
+name: a; isNamePresent: true; isSynthetic: false
+name: _; isNamePresent: true; isSynthetic: false
+name: ***; isNamePresent: true; isSynthetic: false
+name: unary_!; isNamePresent: true; isSynthetic: false
+name: ABC; isNamePresent: true; isSynthetic: false
diff --git a/test/files/run/t9437b/Foo_1.scala b/test/files/run/t9437b/Foo_1.scala
new file mode 100644
index 0000000000..ca6c9c6156
--- /dev/null
+++ b/test/files/run/t9437b/Foo_1.scala
@@ -0,0 +1,3 @@
+class Foo(a: Int, `_`: String, *** : Long, `unary_!` : Float, ABC: Double) {
+  def bar(a: Int, `_`: String, *** : Long, `unary_!` : Float, ABC: Double) = null
+}
diff --git a/test/files/run/t9437b/Test_2.scala b/test/files/run/t9437b/Test_2.scala
new file mode 100644
index 0000000000..521f525f1d
--- /dev/null
+++ b/test/files/run/t9437b/Test_2.scala
@@ -0,0 +1,16 @@
+object Test extends App {
+  val constrParams = classOf[Foo].getConstructors.head.getParameters
+  val methodParams = classOf[Foo].getDeclaredMethods.head.getParameters
+
+  def printParams(params: Array[java.lang.reflect.Parameter]) = {
+    params.foreach { param =>
+      println(s"name: ${param.getName}; isNamePresent: ${param.isNamePresent}; isSynthetic: ${param.isSynthetic}")
+    }
+  }
+
+  printParams(constrParams)
+  printParams(methodParams)
+
+  val foo = new Foo(a = 1, `_` = "2", *** = 3L, `unary_!` = 4.0f, ABC = 5.0)
+  foo.bar(a = 1, `_` = "2", *** = 3L, `unary_!` = 4.0f, ABC = 5.0)
+}
diff --git a/test/files/run/t9437c.check b/test/files/run/t9437c.check
new file mode 100644
index 0000000000..564213c587
--- /dev/null
+++ b/test/files/run/t9437c.check
@@ -0,0 +1,10 @@
+name: a; isNamePresent: true; isSynthetic: false
+name: _; isNamePresent: true; isSynthetic: false
+name: ***; isNamePresent: true; isSynthetic: false
+name: unary_!; isNamePresent: true; isSynthetic: false
+name: ABC; isNamePresent: true; isSynthetic: false
+name: a; isNamePresent: true; isSynthetic: false
+name: _; isNamePresent: true; isSynthetic: false
+name: ***; isNamePresent: true; isSynthetic: false
+name: unary_!; isNamePresent: true; isSynthetic: false
+name: ABC; isNamePresent: true; isSynthetic: false
diff --git a/test/files/run/t9437c/Test.scala b/test/files/run/t9437c/Test.scala
new file mode 100644
index 0000000000..4be233a258
--- /dev/null
+++ b/test/files/run/t9437c/Test.scala
@@ -0,0 +1,92 @@
+import java.io.{File, FileOutputStream}
+
+import scala.tools.nsc.settings.ScalaVersion
+import scala.tools.partest._
+import scala.tools.asm
+import asm.{AnnotationVisitor, ClassWriter, FieldVisitor, Handle, MethodVisitor, Opcodes}
+import Opcodes._
+
+// This test ensures that we can read JDK 8 (classfile format 52) files with
+// parameter names. To do that it first uses ASM to generate a class containing
+// these additional attributes. Then it runs a normal compile on Scala source
+// that uses the class with named arguments.
+// Any failure will be dumped to std out.
+object Test extends DirectTest {
+  override def extraSettings: String = "-usejavacp -d " + testOutput.path + " -cp " + testOutput.path
+
+  def generateCode(): Unit = {
+    val className =  "Foo"
+
+    val cw = new ClassWriter(0)
+    cw.visit(52, ACC_PUBLIC + ACC_SUPER, className, null, "java/lang/Object", null);
+
+    val mvC = cw.visitMethod(ACC_PUBLIC, "<init>", "(ILjava/lang/String;JFD)V", null, null);
+    mvC.visitParameter("a", ACC_FINAL);
+    mvC.visitParameter("_", ACC_FINAL);
+    mvC.visitParameter("***", ACC_FINAL);
+    mvC.visitParameter("unary_!", ACC_FINAL);
+    mvC.visitParameter("ABC", ACC_FINAL);
+    mvC.visitCode();
+    mvC.visitVarInsn(ALOAD, 0);
+    mvC.visitMethodInsn(INVOKESPECIAL, "java/lang/Object", "<init>", "()V", false);
+    mvC.visitInsn(RETURN);
+    mvC.visitMaxs(1, 8);
+    mvC.visitEnd();
+
+    val mvM = cw.visitMethod(ACC_PUBLIC, "bar", "(ILjava/lang/String;JFD)Lscala/runtime/Null$;", null, null);
+    mvM.visitParameter("a", ACC_FINAL);
+    mvM.visitParameter("_", ACC_FINAL);
+    mvM.visitParameter("***", ACC_FINAL);
+    mvM.visitParameter("unary_!", ACC_FINAL);
+    mvM.visitParameter("ABC", ACC_FINAL);
+    mvM.visitCode();
+    mvM.visitInsn(ACONST_NULL);
+    mvM.visitInsn(ARETURN);
+    mvM.visitMaxs(1, 8);
+    mvM.visitEnd();
+
+    cw.visitEnd();
+
+    val bytes = cw.toByteArray()
+
+    val fos = new FileOutputStream(new File(s"${testOutput.path}/$className.class"))
+    try
+      fos write bytes
+    finally
+      fos.close()
+
+  }
+
+  def code =
+"""
+class Driver {
+  val constrParams = classOf[Foo].getConstructors.head.getParameters
+  val methodParams = classOf[Foo].getDeclaredMethods.head.getParameters
+
+  def printParams(params: Array[java.lang.reflect.Parameter]) = {
+    params.foreach { param =>
+      println(s"name: ${param.getName}; isNamePresent: ${param.isNamePresent}; isSynthetic: ${param.isSynthetic}")
+    }
+  }
+
+  printParams(constrParams)
+  printParams(methodParams)
+
+  val foo = new Foo(a = 1, `_` = "2", *** = 3L, `unary_!` = 4.0f, ABC = 5.0)
+  foo.bar(a = 1, `_` = "2", *** = 3L, `unary_!` = 4.0f, ABC = 5.0)
+}
+"""
+
+  override def show(): Unit = {
+    // redirect err to out, for logging
+    val prevErr = System.err
+    System.setErr(System.out)
+    try {
+      generateCode()
+      compile()
+      Class.forName("Driver").newInstance()
+    }
+    finally
+      System.setErr(prevErr)
+  }
+}