Only rename method type params shadowing enclosing class type params (#24684)

Previously, method type parameters were always renamed if they had the
same name as any class type parameter, regardless of whether the class
type parameter was actually used in the method signature.

As a result, we rename methods's type params for unnecessary names like:
#24671 (Note that renaming
actually doesn't cause binary incompatibility, and this change is just
for make generic signature a bit clear and silence false-positive MIMA
reporting).

For example, in an example below, the Scala compiler rename the generic
signature of `bar` to something like `bar[T1](x: T1): T1` because `T` is
used by `Foo[T]`. However, this is unnessary rename because none of T in
method signature refer the `T` of `Foo[T]`.

```scala
class Foo[T]:
  def bar[T](x: T): T = ???
```

This commit makes the renaming conditional:
Method type parameters are only renamed when
- (1) A class type parameter is referenced in the method signature, and
- (2) That class type parameter is shadowed by a method type parameter

Author: tanishiking

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

@@ -49,24 +49,21 @@ object GenericSignatures {
     val builder = new StringBuilder(64)
     val isTraitSignature = sym0.enclosingClass.is(Trait)
 
-    // Collect class-level type parameter names to avoid conflicts with method-level type parameters
-    val usedNames = collection.mutable.Set.empty[String]
-    if(sym0.is(Method)) {
-      sym0.enclosingClass.typeParams.foreach { tp =>
-        usedNames += sanitizeName(tp.name)
-      }
-    }
+    // Track class type parameter names that are shadowed by method type parameters
+    // Used to trigger renaming of method type parameters to avoid conflicts
+    val shadowedClassTypeParamNames = collection.mutable.Set.empty[String]
     val methodTypeParamRenaming = collection.mutable.Map.empty[String, String]
+
     def freshTypeParamName(sanitizedName: String): String = {
-      if !usedNames.contains(sanitizedName) then sanitizedName
+      if !shadowedClassTypeParamNames.contains(sanitizedName) then sanitizedName
       else {
         var i = 1
         var newName = sanitizedName + i
-        while usedNames.contains(newName) do
+        while shadowedClassTypeParamNames.contains(newName) do
           i += 1
           newName = sanitizedName + i
         methodTypeParamRenaming(sanitizedName) = newName
-        usedNames += newName
+        shadowedClassTypeParamNames += newName
         newName
       }
     }
@@ -347,7 +344,22 @@ object GenericSignatures {
 
         case mtd: MethodOrPoly =>
           val (tparams, vparams, rte) = collectMethodParams(mtd)
-          if (toplevel && !sym0.isConstructor) polyParamSig(tparams)
+          if (toplevel && !sym0.isConstructor) {
+            if (sym0.is(Method)) {
+              val (usedMethodTypeParamNames, usedClassTypeParams) = collectUsedTypeParams(vparams :+ rte, sym0)
+              val methodTypeParamNames = tparams.map(tp => sanitizeName(tp.paramName.lastPart)).toSet
+              // Only add class type parameters to shadowedClassTypeParamNames if they are:
+              // 1. Referenced in the method signature, AND
+              // 2. Shadowed by a method type parameter with the same name
+              // This will trigger renaming of the method type parameter
+              usedClassTypeParams.foreach { classTypeParam =>
+                val classTypeParamName = sanitizeName(classTypeParam.name)
+                if methodTypeParamNames.contains(classTypeParamName) then
+                  shadowedClassTypeParamNames += classTypeParamName
+              }
+            }
+            polyParamSig(tparams)
+          }
           builder.append('(')
           for vparam <- vparams do jsig1(vparam)
           builder.append(')')
@@ -528,4 +540,27 @@ object GenericSignatures {
     val rte = recur(mtd)
     (tparams.toList, vparams.toList, rte)
   end collectMethodParams
+
+  /** Collect type parameters that are actually used in the given types. */
+  private def collectUsedTypeParams(types: List[Type], initialSymbol: Symbol)(using Context): (Set[Name], Set[Symbol]) =
+    assert(initialSymbol.is(Method))
+    def isTypeParameterInMethSig(sym: Symbol, initialSymbol: Symbol)(using Context) =
+      !sym.maybeOwner.isTypeParam && // check if it's not higher order type param
+        sym.isTypeParam && sym.owner == initialSymbol
+
+    val usedMethodTypeParamNames = collection.mutable.Set.empty[Name]
+    val usedClassTypeParams = collection.mutable.Set.empty[Symbol]
+
+    def collect(tp: Type): Unit = tp.foreachPart:
+      case ref @ TypeParamRef(_: PolyType, _) =>
+        usedMethodTypeParamNames += ref.paramName
+      case tp: TypeRef =>
+        val sym = tp.typeSymbol
+        if sym.isTypeParam && sym.isContainedIn(initialSymbol.topLevelClass) then
+          usedClassTypeParams += sym
+      case _ =>
+
+    types.foreach(collect)
+    (usedMethodTypeParamNames.toSet, usedClassTypeParams.toSet)
+  end collectUsedTypeParams
 }

compiler/test/dotc/pos-test-pickling.excludelist

@@ -34,6 +34,7 @@ i11982a.scala
 i17255
 i17735.scala
 i24134
+i24134-nested
 
 # Tree is huge and blows stack for printing Text
 i7034.scala

tests/generic-java-signatures/nestedClassTypeParams.check

@@ -0,0 +1,4 @@
+noShadow: public scala.Tuple3<T, U, V> Outer$Middle$Inner.noShadow(T,U,V)
+shadowOuter: public <T1> T Outer$Middle$Inner.shadowOuter()
+shadowMiddle: public <U1> U Outer$Middle$Inner.shadowMiddle()
+shadowInner: public <V1> V Outer$Middle$Inner.shadowInner()

tests/generic-java-signatures/nestedClassTypeParams.scala

@@ -0,0 +1,27 @@
+class Outer[T]:
+  def outerValue: T = ???
+
+  class Middle[U]:
+    def middleValue: U = ???
+
+    class Inner[V]:
+      def innerValue: V = ???
+      def noShadow(x: T, y: U, z: V): (T, U, V) = (x, y, z)
+      def shadowOuter[T] = outerValue
+      def shadowMiddle[U] = middleValue
+      def shadowInner[V] = innerValue
+
+@main def Test(): Unit =
+  val innerMethods = classOf[Outer[_]].getDeclaredClasses()
+    .find(_.getName.contains("Middle")).get.getDeclaredClasses()
+    .find(_.getName.contains("Inner")).get.getDeclaredMethods()
+
+  printMethodSig(innerMethods, "noShadow")
+  printMethodSig(innerMethods, "shadowOuter")
+  printMethodSig(innerMethods, "shadowMiddle")
+  printMethodSig(innerMethods, "shadowInner")
+
+def printMethodSig(methods: Array[java.lang.reflect.Method], name: String): Unit =
+  methods.find(_.getName.endsWith(name)).foreach { m =>
+    println(s"$name: ${m.toGenericString}")
+  }

tests/generic-java-signatures/shadowedTypeParam.check

@@ -0,0 +1,9 @@
+bar: public <T> T Foo.bar(T)
+baz: public <T> T Foo.baz(T,Foo<T>)
+qux: public <U> U Foo.qux(U,Foo<T>)
+quux: public <T,U> scala.Tuple2<T, U> Foo.quux(T,U,Foo<T>)
+copy: public <T> Bar<T> Bar.copy(T)
+copy$default$1: public <T1> T Bar.copy$default$1()
+m: public <T1> T C.m()
+compose: public <A1> scala.Function1<A1, B> JavaPartialFunction.compose(scala.Function1<A1, A>)
+compose: public <R> scala.PartialFunction<R, B> JavaPartialFunction.compose(scala.PartialFunction<R, A>)

tests/generic-java-signatures/shadowedTypeParam.scala

@@ -0,0 +1,53 @@
+// All of type params shouldn't rename
+class Foo[T]:
+  def bar[T](x: T): T = x
+  def baz[T](x: T, y: Foo[T]): T = x
+  def qux[U](x: U, y: Foo[T]): U = x
+  def quux[T, U](x: T, y: U, z: Foo[T]): (T, U) = (x, y)
+
+// https://github.com/scala/scala3/issues/24671
+final case class Bar[+T](t: T)
+
+// `m: public <T1> T C.m()` rather than `m: public <T> T C.m()`
+// that was wrong and could be crashed with
+// `val c = C[String]; String x = c.<Object>m();`.
+abstract class C[T]:
+  def x: T
+  def m[T] = x
+
+// https://github.com/scala/scala3/issues/24134
+// The mixin forwarders for compose in Function1 method  has signature
+// `def compose[A](g: A => T1): A => R`
+// Where the JavaPartialFunction[A, B] has type parameter A (name clash),
+// The type parameter A in method should be renamed to avoid name duplication.
+abstract class JavaPartialFunction[A, B] extends PartialFunction[A, B]
+
+@main def Test(): Unit =
+  val fooMethods = classOf[Foo[_]].getDeclaredMethods()
+  printMethodSig(fooMethods, "bar")
+  printMethodSig(fooMethods, "baz")
+  printMethodSig(fooMethods, "qux")
+  printMethodSig(fooMethods, "quux")
+
+  val barMethods = classOf[Bar[_]].getDeclaredMethods()
+  printMethodSig(barMethods, "copy")
+  // copy$default$1 have `<T1> T Bar.copy$default$1` rather than `<T> T Bar.copy$default$1`
+  // as reported in https://github.com/scala/scala3/issues/24671
+  // The type parameter rename occurs because the return type T refers the enclosing class's type param T.
+  printMethodSig(barMethods, "copy$default$1")
+
+  val cMethods = classOf[C[_]].getDeclaredMethods()
+  printMethodSig(cMethods, "m")
+
+  val jpfMethods = classOf[JavaPartialFunction[_, _]].getDeclaredMethods()
+  printMethodSigs(jpfMethods, "compose")
+
+def printMethodSig(methods: Array[java.lang.reflect.Method], name: String): Unit =
+  methods.find(_.getName.endsWith(name)).foreach { m =>
+    println(s"$name: ${m.toGenericString}")
+  }
+
+def printMethodSigs(methods: Array[java.lang.reflect.Method], name: String): Unit =
+  methods.filter(_.getName == name).sortBy(_.toGenericString).foreach { m =>
+    println(s"$name: ${m.toGenericString}")
+  }

tests/pos/i24134-nested/Container.scala

@@ -0,0 +1,2 @@
+class Container[A]:
+  abstract class JavaPartialFunction[B] extends PartialFunction[A, B]

tests/pos/i24134-nested/Main.java

@@ -0,0 +1,15 @@
+public class Main {
+    public static void main(String[] args) {
+        Container<String> container = new Container<>();
+        Container<String>.JavaPartialFunction<Integer> pf = container.new JavaPartialFunction<Integer>() {
+            @Override
+            public boolean isDefinedAt(String x) {
+                return x != null && !x.isEmpty();
+            }
+            @Override
+            public Integer apply(String x) {
+                return x.length();
+            }
+        };
+    }
+}