Make sure Getters doesn't change signatures

Getters will replace `val x: Int`, by `def x: Int`, before this commit,
this lead to the signature of `x` changing from `Signature.NotAMethod`
to `Signature(Nil, scala.Int)` but we need signatures to be stable
pre-erasure to disambiguate overloads. This commit fixes this by
changing ExprType#signature to always return `NotAMethod`, this should
not break anything since we do not allow overloads which only differ in
their result type anyway.

This commit also adds tests to make sure that the invariants related to
signatures are not violated, currently there is one class of failure
related to enums which is fixed in the next commit.

Author: smarter

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

@@ -141,7 +141,7 @@ object Signature {
   export MatchDegree._
 
   /** The signature of everything that's not a method, i.e. that has
-   *  a type different from PolyType, MethodType, or ExprType.
+   *  a type different from PolyType or MethodType.
    */
   val NotAMethod: Signature = Signature(List(), EmptyTypeName)
 

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

@@ -3075,7 +3075,7 @@ object Types {
     }
   }
 
-  trait MethodicType extends SignatureCachingType {
+  trait MethodicType extends TermType {
     protected def resultSignature(implicit ctx: Context): Signature = try resultType match {
       case rtp: MethodicType => rtp.signature
       case tp =>
@@ -3095,7 +3095,7 @@ object Types {
     override def resultType(implicit ctx: Context): Type = resType
     override def underlying(implicit ctx: Context): Type = resType
 
-    def computeSignature(implicit ctx: Context): Signature = resultSignature
+    override def signature(implicit ctx: Context): Signature = Signature.NotAMethod
 
     def derivedExprType(resType: Type)(implicit ctx: Context): ExprType =
       if (resType eq this.resType) this else ExprType(resType)
@@ -3201,7 +3201,7 @@ object Types {
     final override def equals(that: Any): Boolean = equals(that, null)
   }
 
-  abstract class MethodOrPoly extends UncachedGroundType with LambdaType with MethodicType {
+  abstract class MethodOrPoly extends UncachedGroundType with LambdaType with MethodicType with SignatureCachingType {
     final override def hashCode: Int = System.identityHashCode(this)
 
     final override def equals(that: Any): Boolean = equals(that, null)

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

@@ -87,6 +87,19 @@ class TreeChecker extends Phase with SymTransformer {
 
     checkCompanion(symd)
 
+    // Signatures are used to disambiguate overloads and need to stay stable
+    // until erasure, see the comment above `Compiler#phases`.
+    if (ctx.phaseId <= ctx.erasurePhase.id) {
+      val cur = symd.info
+      val initial = symd.initial.info
+      assert(cur.signature == initial.signature,
+        i"""Signature of $symd changed at phase ${ctx.phase}
+           |Initial info: ${initial}
+           |Initial sig : ${initial.signature}
+           |Current info: ${cur}
+           |Current sig : ${cur.signature}""")
+    }
+
     symd
   }
 

compiler/test/dotty/tools/SignatureTest.scala

@@ -0,0 +1,38 @@
+package dotty.tools
+
+import vulpix.TestConfiguration
+
+import org.junit.Test
+
+import dotc.ast.Trees._
+import dotc.core.Decorators._
+import dotc.core.Contexts._
+import dotc.core.Types._
+
+import java.io.File
+import java.nio.file._
+
+class SignatureTest:
+  @Test def signatureCaching: Unit =
+    inCompilerContext(TestConfiguration.basicClasspath, separateRun = true, "case class Foo(value: Unit)") {
+      val (ref, refSig) = ctx.atPhase(ctx.erasurePhase.next) {
+        val cls = ctx.requiredClass("Foo")
+        val ref = cls.requiredMethod("value").termRef
+        (ref, ref.signature)
+      }
+      ctx.atPhase(ctx.typerPhase) {
+        // NamedType#signature is always computed before erasure, which ensures
+        // that it stays stable and therefore can be cached as long as
+        // signatures are guaranteed to be stable before erasure, see the
+        // comment above `Compiler#phases`.
+        assert(refSig == ref.signature,
+          s"""The signature of a type should never change but the signature of $ref was:
+             |${ref.signature} at typer, whereas it was:
+             |${refSig} after erasure""".stripMargin)
+        assert(ref.signature == ref.denot.signature,
+          s"""Before erasure, the signature of a TypeRef should be the signature of its denotation,
+             |but the cached signature of $ref was:
+             |${ref.signature}, whereas its denotation signature at typer was:
+             |${ref.denot.signature}""".stripMargin)
+      }
+    }

compiler/test/dotty/tools/compilerSupport.scala

@@ -1,7 +1,7 @@
 package dotty.tools
 
 import javax.tools._
-import java.io.File
+import java.io._
 import java.nio.file._
 import java.net.URI
 import scala.jdk.CollectionConverters._
@@ -10,14 +10,22 @@ import core._
 import core.Contexts._
 import dotc.core.Comments.{ContextDoc, ContextDocstrings}
 
-/** Initialize a compiler context with the given classpath and
- *  pass it to `op`.
+/** Initialize a compiler context with the given `classpath`, compile all
+ *  `scalaSources`, then run `op`.
+ *
+ *  If `separateRun` is true , then `op` will be run in a new run different from the
+ *  one used to compile the sources, this makes it easier to test for potential
+ *  issues involving retrieving symbols defined in a previous run.
  */
-def inCompilerContext[T](classpath: String)(op: Context ?=> T): T =
+def inCompilerContext[T](classpath: String, separateRun: Boolean = true, scalaSources: String*)(op: Context ?=> T): T =
   val compiler = Compiler()
-  val run = compiler.newRun(initCtx(classpath))
-  run.compileUnits(Nil) // Initialize phases
-  op(using run.runContext)
+  val rootCtx = initCtx(classpath)
+  val firstRun = compiler.newRun(rootCtx)
+  firstRun.compileFromStrings(scalaSources.toList)
+  val opRun = if separateRun
+    then compiler.newRun(rootCtx)
+    else firstRun
+  op(using opRun.runContext)
 
 private def initCtx(classpath: String): Context =
   val ctx0 = (new ContextBase).initialCtx.fresh