Fix instability of self variable API representation

The reason for instability is a bit tricky so let's unpack what the
previous code checking if there's self type declared was doing. It would
check if `thisSym` of a class is equal to a symbol representing the class.
If that's true, we know that there's no self type. If it's false, then
`thisSym` represents either a self type or a self variable. The second
(type test) was supposed to check whether the type of `thisSym` is
different from a type of the class. However, it would always yield false
because TypeRef of `thisSym` was compared to ClassInfoType of a class.
So if you had a self variable the logic would see a self type (and that's
what API representation would give you).

Now the tricky bit: `thisSym` is not pickled when it's representing just
a self variable because self variable doesn't affect other classes
referring to a class. If you looked at a type after unpickling, the
symbol equality test would yield true and we would not see self type when
just a self variable was declared.

The fix is to check equality of type refs on both side of the type equality
check. This makes the pending test passing.

Also, I added another test that checks if self types are represented in
various combinations of declaring a self variable or/and self type.

Fixes #2504.

compile/interface/src/main/scala/xsbt/ExtractAPI.scala

@@ -578,7 +578,7 @@ class ExtractAPI[GlobalType <: CallbackGlobal](val global: GlobalType,
     // as that invariant is established on completing the class symbol (`mkClassLike` calls `s.initialize` before calling us).
     // Technically, we could even ignore a self type that's a supertype of the class's type,
     // as it does not contribute any information relevant outside of the class definition.
-    if ((s.thisSym eq s) || s.typeOfThis == s.info) Constants.emptyType else processType(in, s.typeOfThis)
+    if ((s.thisSym eq s) || (s.thisSym.tpeHK == s.tpeHK)) Constants.emptyType else processType(in, s.typeOfThis)
 
   def classLike(in: Symbol, c: Symbol): ClassLike = classLikeCache.getOrElseUpdate((in, c), mkClassLike(in, c))
   private def mkClassLike(in: Symbol, c: Symbol): ClassLike = {

compile/interface/src/test/scala/xsbt/ExtractAPISpecification.scala

@@ -1,7 +1,7 @@
 package xsbt
 
 import org.junit.runner.RunWith
-import xsbti.api.{ Definition, SourceAPI, ClassLike, Def }
+import xsbti.api._
 import xsbt.api.SameAPI
 import org.specs2.mutable.Specification
 import org.specs2.runner.JUnitRunner
@@ -40,11 +40,13 @@ class ExtractAPISpecification extends Specification {
   }
 
   /**
-    * Checks if representation of the inherited Namer class (with a declared self variable) in Global.Foo
-    * is stable between compiling from source and unpickling. We compare extracted APIs of Global when Global
-    * is compiled together with Namers or Namers is compiled first and then Global refers
-    * to Namers by unpickling types from class files.
-    */
+   * Checks if representation of the inherited Namer class (with a declared self variable) in Global.Foo
+   * is stable between compiling from source and unpickling. We compare extracted APIs of Global when Global
+   * is compiled together with Namers or Namers is compiled first and then Global refers
+   * to Namers by unpickling types from class files.
+   *
+   * See https://github.com/sbt/sbt/issues/2504
+   */
   "Self variable and no self type" in {
     def selectNamer(api: SourceAPI): ClassLike = {
       def selectClass(defs: Iterable[Definition], name: String): ClassLike = defs.collectFirst {
@@ -70,5 +72,33 @@ class ExtractAPISpecification extends Specification {
     val namerApi1 = selectNamer(src2Api1)
     val namerApi2 = selectNamer(src2Api2)
     SameAPI(namerApi1, namerApi2)
-  }.pendingUntilFixed("have unstable representation (#2504)")
+  }
+
+  /**
+   * Checks if self type is properly extracted in various cases of declaring a self type
+   * with our without a self variable.
+   */
+  "Self type" in {
+    def collectFirstClass(defs: Array[Definition]): ClassLike = defs.collectFirst {
+      case c: ClassLike => c
+    }.get
+    val srcX = "trait X"
+    val srcY = "trait Y"
+    val srcC1 = "class C1 { this: C1 => }"
+    val srcC2 = "class C2 { thisC: C2 => }"
+    val srcC3 = "class C3 { this: X => }"
+    val srcC4 = "class C4 { thisC: X => }"
+    val srcC5 = "class C5 extends AnyRef with X with Y { self: X with Y => }"
+    val srcC6 = "class C6 extends AnyRef with X { self: X with Y => }"
+    val compilerForTesting = new ScalaCompilerForUnitTesting
+    val apis = compilerForTesting.extractApisFromSrcs(reuseCompilerInstance = true)(
+      List(srcX, srcY, srcC1, srcC2, srcC3, srcC4, srcC5, srcC6)
+    ).map(x => collectFirstClass(x.definitions))
+    val emptyType = new EmptyType
+    def hasSelfType(c: ClassLike): Boolean =
+      c.selfType != emptyType
+    val (withSelfType, withoutSelfType) = apis.partition(hasSelfType)
+    withSelfType.map(_.name).toSet === Set("C3", "C4", "C5", "C6")
+    withoutSelfType.map(_.name).toSet === Set("X", "Y", "C1", "C2")
+  }
 }