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:

> run -Ycheck:all tests/run/enum-constrs.scala
[...]
checking tests/run/enum-constrs.scala after phase MegaPhase{completeJavaEnums, elimRepeated, expandSAMs, protectedAccessors, extmethods, cacheAliasImplicits, byNameClosures, hoistSuperArgs, classOf, refchecks}
exception occurred while compiling tests/run/enum-constrs.scala
java.lang.AssertionError: assertion failed: Signature of constructor Color changed at phase Ycheck
Initial info: (): Color
Initial sig : Signature(List(),Color)
Current info: (_$name: String, _$ordinal: Int): Color
Current sig : Signature(List(java.lang.String, scala.Int),Color) while compiling tests/run/enum-constrs.scala

The problem is that the phase CompleteJavaEnums changes the signature of
enum constructors and runs before erasure, thus violating the invariant
specified in the comment above Compiler#phases. I tried simply moving
CompleteJavaEnums after Erasure, but that currently fails with:

-- Error: tests/run/enum-constrs.scala:2:7 -------------------------------------
2 |  case Red, Green, Blue
  |       ^
  |wrong number of arguments at getClass for (_$name: String, _$ordinal: Int): Color: ($anon.super.<init> : (_$name: String, _$ordinal: Int): Color), expected: 2, found: 0

So, some more work is required here.

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

@@ -3088,7 +3088,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
+    def computeSignature(implicit ctx: Context): Signature = Signature.NotAMethod
 
     def derivedExprType(resType: Type)(implicit ctx: Context): ExprType =
       if (resType eq this.resType) this else ExprType(resType)

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,34 @@ import core._
 import core.Contexts._
 import dotc.core.Comments.{ContextDoc, ContextDocstrings}
 
-/** Initialize a compiler context with the given classpath and
- *  pass it to `op`.
+// TODO: refactor, copy-pasted from Run#compileFromStrings
+private def sourceFile(source: String, isJava: Boolean): util.SourceFile = {
+  val uuid = java.util.UUID.randomUUID().toString
+  val ext = if (isJava) ".java" else ".scala"
+  val virtualFile = new io.VirtualFile(s"compileFromString-$uuid.$ext")
+  val writer = new BufferedWriter(new OutputStreamWriter(virtualFile.output, "UTF-8")) // buffering is still advised by javadoc
+  writer.write(source)
+  writer.close()
+  new util.SourceFile(virtualFile, scala.io.Codec.UTF8)
+}
+
+/** 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.compileUnits(scalaSources.toList.map(s =>
+    CompilationUnit(sourceFile(s, isJava = false))(using firstRun.runContext)))
+  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