Retain parameter annotations in the generation of mixin phase forwarding methods -- PR choice 2

compiler/src/dotty/tools/dotc/ast/tpd.scala

@@ -253,15 +253,14 @@ object tpd extends Trees.Instance[Type] with TypedTreeInfo {
 
   def DefDef(sym: TermSymbol, rhs: Tree = EmptyTree)(using Context): DefDef =
     ta.assignType(DefDef(sym, Function.const(rhs)), sym)
-
+    
   /** A DefDef with given method symbol `sym`.
    *  @rhsFn  A function from parameter references
    *          to the method's right-hand side.
    *  Parameter symbols are taken from the `rawParamss` field of `sym`, or
    *  are freshly generated if `rawParamss` is empty.
    */
   def DefDef(sym: TermSymbol, rhsFn: List[List[Tree]] => Tree)(using Context): DefDef =
-
     // Map method type `tp` with remaining parameters stored in rawParamss to
     // final result type and all (given or synthesized) parameters
     def recur(tp: Type, remaining: List[List[Symbol]]): (Type, List[List[Symbol]]) = tp match
@@ -275,36 +274,14 @@ object tpd extends Trees.Instance[Type] with TypedTreeInfo {
         val (rtp, paramss) = recur(tp.instantiate(tparams.map(_.typeRef)), remaining1)
         (rtp, tparams :: paramss)
       case tp: MethodType =>
-        val isParamDependent = tp.isParamDependent
-        val previousParamRefs: ListBuffer[TermRef] =
-          // It is ok to assign `null` here.
-          // If `isParamDependent == false`, the value of `previousParamRefs` is not used.
-          if isParamDependent then mutable.ListBuffer[TermRef]() else (null: ListBuffer[TermRef] | Null).uncheckedNN
-
-        def valueParam(name: TermName, origInfo: Type, isErased: Boolean): TermSymbol =
-          val maybeImplicit =
-            if tp.isContextualMethod then Given
-            else if tp.isImplicitMethod then Implicit
-            else EmptyFlags
-          val maybeErased = if isErased then Erased else EmptyFlags
-
-          def makeSym(info: Type) = newSymbol(sym, name, TermParam | maybeImplicit | maybeErased, info, coord = sym.coord)
-
-          if isParamDependent then
-            val sym = makeSym(origInfo.substParams(tp, previousParamRefs.toList))
-            previousParamRefs += sym.termRef
-            sym
-          else makeSym(origInfo)
-        end valueParam
-
         val (vparams: List[TermSymbol], remaining1) =
           if tp.paramNames.isEmpty then (Nil, remaining)
           else remaining match
             case vparams :: remaining1 =>
               assert(vparams.hasSameLengthAs(tp.paramNames) && vparams.head.isTerm)
               (vparams.asInstanceOf[List[TermSymbol]], remaining1)
             case nil =>
-              (tp.paramNames.lazyZip(tp.paramInfos).lazyZip(tp.paramErasureStatuses).map(valueParam), Nil)
+              (newMethodValueParams(sym, tp), Nil)
         val (rtp, paramss) = recur(tp.instantiate(vparams.map(_.termRef)), remaining1)
         (rtp, vparams :: paramss)
       case _ =>

compiler/src/dotty/tools/dotc/core/Symbols.scala

@@ -19,16 +19,21 @@ import DenotTransformers.*
 import StdNames.*
 import NameOps.*
 import NameKinds.LazyImplicitName
-import ast.*, tpd.*
+import ast.*
+import tpd.*
 import Constants.Constant
 import Variances.Variance
 import reporting.Message
+
 import collection.mutable
 import io.AbstractFile
-import util.{SourceFile, NoSource, Property, SourcePosition, SrcPos, EqHashMap}
+import util.{EqHashMap, NoSource, Property, SourceFile, SourcePosition, SrcPos}
+
 import scala.annotation.internal.sharable
 import config.Printers.typr
 import dotty.tools.dotc.classpath.FileUtils.isScalaBinary
+import dotty.tools.dotc.core.Names.TermName
+import dotty.tools.dotc.core.Types.Type
 
 import scala.compiletime.uninitialized
 import dotty.tools.tasty.TastyVersion
@@ -895,6 +900,28 @@ object Symbols extends SymUtils {
     tparams
   }
 
+  /** Create new method value parameter symbols for a Method.
+   */
+  def newMethodValueParams(meth: TermSymbol, methodType: MethodType)(using Context): List[TermSymbol] =
+    def makeParamSym(name: TermName, info: Type, isErased: Boolean) =
+      val maybeImplicit =
+        if methodType.isContextualMethod then Given
+        else if methodType.isImplicitMethod then Implicit
+        else EmptyFlags
+      val maybeErased = if isErased then Erased else EmptyFlags
+      newSymbol(meth, name, TermParam | maybeImplicit | maybeErased, info, coord = meth.coord)
+
+    def makeParamSymOverPreviousParams: (TermName, Type, Boolean) => TermSymbol =
+      val previousParamRefs: mutable.ListBuffer[TermRef] = mutable.ListBuffer[TermRef]()
+      (name: TermName, tpe: Type, isErased: Boolean) =>
+        val sym = makeParamSym(name, tpe.substParams(methodType, previousParamRefs.toList), isErased)
+        previousParamRefs += sym.termRef
+        sym
+
+    val newValueParam = if methodType.isParamDependent then makeParamSymOverPreviousParams else makeParamSym
+    methodType.paramNames.lazyZip(methodType.paramInfos).lazyZip(methodType.paramErasureStatuses).map(newValueParam)
+  end newMethodValueParams  
+
   /** Create a new skolem symbol. This is not the same as SkolemType, even though the
    *  motivation (create a singleton referencing to a type) is similar.
    */

compiler/src/dotty/tools/dotc/transform/Mixin.scala

@@ -16,6 +16,7 @@ import StdNames.*
 import Names.*
 import NameKinds.*
 import NameOps.*
+import Annotations.*
 import Phases.erasurePhase
 import ast.Trees.*
 
@@ -315,7 +316,27 @@ class Mixin extends MiniPhase with SymTransformer { thisPhase =>
       for (meth <- mixin.info.decls.toList if needsMixinForwarder(meth))
       yield {
         util.Stats.record("mixin forwarders")
-        transformFollowing(DefDef(mkMixinForwarderSym(meth.asTerm), forwarderRhsFn(meth)))
+        val sym = meth.asTerm
+        val forwarderSym = mkMixinForwarderSym(sym)
+        //Whereas method annotations are set directly in mkMixForwarderSym from the original method symbol
+        //in the case of parameter annotations, we cant rely on DefDef of the forwarderSym directly as this
+        //will not get the parameter annotations.
+        //Hence we replicate a portion of DefDef to create the forwarderSyms method parameters and then set
+        //the annotations on these parameters and then rely on a different path thru DefDef to finish the logic.
+        lazy val annotationss: Map[TermName, List[Annotation]] = (for
+          rawParams <- meth.asTerm.rawParamss
+          if rawParams.nonEmpty && !rawParams.head.isType
+          p <- rawParams
+        yield (p.termRef.name, p.annotations)).toMap
+        if sym.info.isInstanceOf[MethodType] && annotationss.values.exists(_.nonEmpty) then
+          val annotss = annotationss.withDefaultValue(Nil)
+          val tp: MethodType = forwarderSym.info.asInstanceOf[MethodType]
+          val vparams = newMethodValueParams(forwarderSym, tp)
+          vparams.lazyZip(tp.paramNames.map(annotss)).foreach: (vpms, annots) =>
+            vpms.addAnnotations(annots)
+          forwarderSym.setParamss(vparams :: Nil)
+        end if
+        transformFollowing(DefDef(forwarderSym, forwarderRhsFn(meth)))
       }
 
     def mkMixinForwarderSym(target: TermSymbol): TermSymbol =

tests/run/i22991/Bar.scala

@@ -0,0 +1,9 @@
+class Blah extends scala.annotation.StaticAnnotation
+
+trait Barly {
+  def bar[T](a: String, @Foo v: Int)(@Foo b: T, @Blah w: Int) = ()
+}
+
+class Bar extends Barly{
+  def bar2(@Foo v: Int) = ()
+}

tests/run/i22991/Foo.java

@@ -0,0 +1,6 @@
+import java.lang.annotation.*;
+
+@Retention(RetentionPolicy.RUNTIME)
+public @interface Foo {
+}
+

tests/run/i22991/Test.scala

@@ -0,0 +1,17 @@
+
+//Test java runtime reflection access to @Runtime annotations on method parameters.
+object Test extends App:
+  val method: java.lang.reflect.Method = classOf[Bar].getMethod("bar", classOf[String], classOf[Int], classOf[Object], classOf[Int])
+  val annots: Array[Array[java.lang.annotation.Annotation]] = method.getParameterAnnotations()
+  assert(annots.length == 4)
+  assert(annots(0).length == 0)
+  assert(annots(1).length == 1)
+  assert(annots(1)(0).isInstanceOf[Foo])
+  assert(annots(2).length == 1)
+  assert(annots(2)(0).isInstanceOf[Foo])
+  assert(annots(3).length == 0)
+
+  val method2: java.lang.reflect.Method = classOf[Bar].getMethod("bar2", classOf[Int])
+  val annots2: Array[Array[java.lang.annotation.Annotation]] = method2.getParameterAnnotations()
+  assert(annots2.length == 1)
+  assert(annots2(0)(0).isInstanceOf[Foo])