From 3df9336d42d885109b1eee9449b8f4284c0a2598 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?S=C3=A9bastien=20Doeraene?= <sjrdoeraene@gmail.com>
Date: Mon, 27 Nov 2023 18:20:20 +0100
Subject: [PATCH] Fix #397: Use a fake owner for orphan symbols in Scala 2
 pickles.

We had a long-standing issue with "orphan" `TYPEsym`s and `VALsym`s
in Scala 2 pickles. We were replacing them by "no symbol"
themselves, because we did not know what to do with them.

Issue #397 finally provided a good setup to diagnose it. It turns
out that those orphan symbols can be found as term and type params
of method types and poly types, in particular, Scala 2's encoding
of type lambdas, which can be produced by kind-projector.

Since we now have a good reason to have those orphan symbols, we
bite the bullet and allocate one global symbol to "host" all those
symbols coming from Scala 2.
---
 .../main/scala/tastyquery/Definitions.scala   | 10 ++++++++++
 .../tastyquery/reader/ReaderContext.scala     |  2 ++
 .../reader/pickles/PickleReader.scala         | 20 +++++++------------
 3 files changed, 19 insertions(+), 13 deletions(-)

diff --git a/tasty-query/shared/src/main/scala/tastyquery/Definitions.scala b/tasty-query/shared/src/main/scala/tastyquery/Definitions.scala
index 8a8b4396..f62168d2 100644
--- a/tasty-query/shared/src/main/scala/tastyquery/Definitions.scala
+++ b/tasty-query/shared/src/main/scala/tastyquery/Definitions.scala
@@ -134,6 +134,16 @@ final class Definitions private[tastyquery] (ctx: Context, rootPackage: PackageS
 
   val NothingAnyBounds = AbstractTypeBounds(SyntacticNothingType, AnyClass.topLevelRef)
 
+  // See `case noRef: NoExternalSymbolRef` in PickleReader
+  private[tastyquery] val scala2FakeOwner: TermSymbol =
+    TermSymbol
+      .createNotDeclaration(termName("<nosymbol>"), scalaPackage)
+      .withFlags(Synthetic, None)
+      .withDeclaredType(AnyType)
+      .setAnnotations(Nil)
+      .checkCompleted()
+  end scala2FakeOwner
+
   private def createSpecialTypeAlias(
     name: TypeName,
     owner: DeclaringSymbol,
diff --git a/tasty-query/shared/src/main/scala/tastyquery/reader/ReaderContext.scala b/tasty-query/shared/src/main/scala/tastyquery/reader/ReaderContext.scala
index 4bf204fa..2b2b3875 100644
--- a/tasty-query/shared/src/main/scala/tastyquery/reader/ReaderContext.scala
+++ b/tasty-query/shared/src/main/scala/tastyquery/reader/ReaderContext.scala
@@ -43,6 +43,8 @@ private[reader] final class ReaderContext(underlying: Context):
 
   def uninitializedMethodTermRef: TermRef = underlying.defn.uninitializedMethodTermRef
 
+  def scala2FakeOwner: TermSymbol = underlying.defn.scala2FakeOwner
+
   def findPackageFromRootOrCreate(fullyQualifiedName: PackageFullName): PackageSymbol =
     underlying.findPackageFromRootOrCreate(fullyQualifiedName)
 
diff --git a/tasty-query/shared/src/main/scala/tastyquery/reader/pickles/PickleReader.scala b/tasty-query/shared/src/main/scala/tastyquery/reader/pickles/PickleReader.scala
index 713a2507..ad50dec1 100644
--- a/tasty-query/shared/src/main/scala/tastyquery/reader/pickles/PickleReader.scala
+++ b/tasty-query/shared/src/main/scala/tastyquery/reader/pickles/PickleReader.scala
@@ -203,17 +203,13 @@ private[pickles] class PickleReader {
       case sym: Symbol =>
         sym
       case noRef: NoExternalSymbolRef if tag == TYPEsym || tag == VALsym =>
-        /* For some reason, Scala 2 pickles `TYPEsym` and `VALsym` symbols whose owner references `NONEsym`.
-         * However, they do not appear to be referenced *themselves* from anywhere. They
-         * only appear in the table, and hence get read from the loop in `Unpickler.run`.
-         * We ignore these entries here by replacing them with `NONEsym`. If they end up
-         * being actually referenced somewhere, then that somewhere will then crash with
-         * an unexpected reference to `NONEsym`, which would provide us with more context
-         * to perhaps solve this at a deeper level.
-         * If someone wants to investigate, there is a case of `TYPEsym` in `scala.collection.Iterator`.
-         * There is a case of `VALsym` somewhere in sbt, triggered when tasty-mima-checking sbt-tasty-mima itself.
+        /* Sometimes, Scala 2 pickles `TYPEsym` and `VALsym` symbols whose owner references `NONEsym`.
+         * This seems to happen in METHODtpe and POLYtpe that are not for declared methods
+         * (but for type lambdas, or in annotations), for their parameter symbols.
+         * Our system does not have a notion of "no symbol". Instead, we use one fake
+         * owner to host these "orphan" symbols.
          */
-        return NoExternalSymbolRef.instance
+        rctx.scala2FakeOwner
       case external =>
         errorBadSignature(
           s"expected local symbol reference but found $external for owner of ${name1.toDebugString} with tag $tag"
@@ -727,9 +723,7 @@ private[pickles] class PickleReader {
         // create PolyType
         //   - PT => register at index
         val restpe = readTypeMappableRef()
-        val typeParams = pkl.until(end, () => readMaybeExternalSymbolRef()).collect { case typeParam: TypeParamSymbol =>
-          typeParam
-        }
+        val typeParams = pkl.until(end, () => readLocalSymbolRef().asInstanceOf[TypeParamSymbol])
         if typeParams.nonEmpty then TempPolyType(typeParams, restpe)
         else restpe
       case EXISTENTIALtpe =>