Fix detection of generic type projections as 'weak' types on Scala 2 …

…when obscured by type alias, also fix #189
zio · Apr 25, 2023 · d95d887 · d95d887
1 parent d89cc3b
commit d95d887
Show file tree

Hide file tree

Showing 14 changed files with 175 additions and 111 deletions.
diff --git a/...mi/reflect/test/LightTypeTagTestJvm.scala → ...ct/test/AllPartsStrongTestScala2Jvm.scala b/...mi/reflect/test/LightTypeTagTestJvm.scala → ...ct/test/AllPartsStrongTestScala2Jvm.scala
@@ -19,9 +19,10 @@
 package izumi.reflect.test
 
 import izumi.reflect.ReflectionUtil
+import izumi.reflect.test.DiscoveryModel.{DiscoverableService, DiscoverableServiceImpl, DiscoveryNodeProvider, GetDiscoveryNode}
 import org.scalatest.wordspec.AnyWordSpec
 
-class LightTypeTagTestJvm extends AnyWordSpec {
+class AllPartsStrongTestScala2Jvm extends AnyWordSpec {
 
   type FP1[+T] = List[T]
   type Ap1[+F[+_], +T] = F[T]
@@ -53,7 +54,6 @@ class LightTypeTagTestJvm extends AnyWordSpec {
       .asInstanceOf[scala.reflect.runtime.universe.RefinedTypeApi].decl(scala.reflect.runtime.universe.TypeName("l"))
       .asType.typeSignature
       .typeConstructor
-    println(tpe)
     val res1 = ReflectionUtil.allPartsStrong(tpe)
     assert(res1)
   }
@@ -63,4 +63,15 @@ class LightTypeTagTestJvm extends AnyWordSpec {
     assert(res1)
   }
 
+  "allPartsStrong for TC#DiscoveryNode type projection" in {
+    def test1[TC <: DiscoverableService]: Boolean = {
+      ReflectionUtil.allPartsStrong(scala.reflect.runtime.universe.weakTypeOf[DiscoveryNodeProvider[GetDiscoveryNode[TC]]])
+    }
+    def test2[TC <: DiscoverableService]: Boolean = {
+      ReflectionUtil.allPartsStrong(scala.reflect.runtime.universe.weakTypeOf[DiscoveryNodeProvider[TC#DiscoveryNode]])
+    }
+    assert(!test1[DiscoverableServiceImpl])
+    assert(!test2[DiscoverableServiceImpl])
+  }
+
 }
diff --git a/izumi-reflect/izumi-reflect/src/main/scala-2/izumi/reflect/ReflectionUtil.scala b/izumi-reflect/izumi-reflect/src/main/scala-2/izumi/reflect/ReflectionUtil.scala
@@ -45,43 +45,42 @@ private[reflect] object ReflectionUtil {
   }
 
   def allPartsStrong(tpe: Universe#Type): Boolean = {
-    val selfStrong = isSelfStrong(tpe)
-    def prefixStrong = {
-      tpe match {
-        case t: Universe#TypeRefApi =>
-          allPartsStrong(t.pre.dealias)
-        case _ =>
-          true
-      }
-    }
-    val dealiased = tpe.dealias
-    def typeCtorStrong = {
-      val resType = dealiased.finalResultType
-      val ctor = resType.typeConstructor
-      (resType == dealiased) || (ctor == resType) || (ctor == tpe) ||
-      tpe.typeParams.contains(ctor.typeSymbol) || allPartsStrong(ctor)
-    }
-    def argsStrong = {
-      dealiased.finalResultType.typeArgs.forall {
-        arg =>
-          tpe.typeParams.contains(arg.typeSymbol) ||
-          allPartsStrong(arg)
-      }
-    }
-    def intersectionStructStrong = {
-      tpe match {
-        case t: Universe#RefinedTypeApi =>
-          t.parents.forall(allPartsStrong) &&
-          t.decls.toSeq.forall((s: Universe#Symbol) => s.isTerm || allPartsStrong(s.asType.typeSignature.dealias))
-        case _ =>
-          true
-      }
-    }
+    allPartsStrong(Set.empty, tpe)
+  }
+
+  def allPartsStrong(outerTypeParams: Set[Universe#Symbol], tpeRaw: Universe#Type): Boolean = {
+    val dealiased = tpeRaw.dealias
+    val selfStrong = isSelfStrong(outerTypeParams, dealiased) || outerTypeParams.contains(dealiased.typeSymbol)
+
+    dealiased match {
+      case t: Universe#RefinedTypeApi =>
+        t.parents.forall(allPartsStrong(outerTypeParams, _)) &&
+        t.decls.toSeq.forall((s: Universe#Symbol) => s.isTerm || allPartsStrong(outerTypeParams, s.asType.typeSignature.dealias))
+      case _ =>
+        val resType = dealiased.finalResultType
+        if (dealiased.takesTypeArgs && !dealiased.typeParams.forall(outerTypeParams.contains)) {
+          allPartsStrong(outerTypeParams ++ dealiased.typeParams, resType)
+        } else {
 
-    selfStrong && prefixStrong && typeCtorStrong && argsStrong && intersectionStructStrong
+          def typeCtorStrong: Boolean = {
+            val ctor = resType.typeConstructor
+            (resType == dealiased) || (ctor == dealiased) || (ctor == tpeRaw) ||
+            outerTypeParams.contains(ctor.typeSymbol) || allPartsStrong(outerTypeParams, ctor)
+          }
+
+          def argsStrong: Boolean = {
+            resType.typeArgs.forall {
+              arg =>
+                outerTypeParams.contains(arg.typeSymbol) || allPartsStrong(outerTypeParams, arg)
+            }
+          }
+
+          selfStrong && /*prefixStrong &&*/ typeCtorStrong && argsStrong
+        }
+    }
   }
 
-  def isSelfStrong(tpe: Universe#Type): Boolean = {
+  def isSelfStrong(outerTypeParams: Set[Universe#Symbol], tpe: Universe#Type): Boolean = {
     // FIXME: strengthening check to capture `IntersectionBlockingIO` test case causes StackOverflow during implicit search
 //    def intersectionMembersStrong = {
 //      tpe match {
@@ -91,15 +90,24 @@ private[reflect] object ReflectionUtil {
 //      }
 //    }
 
-    !(tpe.typeSymbol.isParameter || (
+    def prefixStrong: Boolean = {
+      tpe match {
+        case t: Universe#TypeRefApi =>
+          allPartsStrong(outerTypeParams, t.pre.dealias)
+        case _ =>
+          true
+      }
+    }
+
+    (prefixStrong && !(tpe.typeSymbol.isParameter || (
       // we regard abstract types like T in trait X { type T; Tag[this.T] } - when we are _inside_ the definition template
       // as 'type parameters' too. So that you could define `implicit def tagForT: Tag[this.T]` and the tag would be resolved
       // to this implicit correctly, instead of generating a useless `X::this.type::T` tag.
       tpe.isInstanceOf[Universe#TypeRefApi] &&
       tpe.asInstanceOf[Universe#TypeRefApi].pre.isInstanceOf[Universe#ThisTypeApi] &&
       tpe.typeSymbol.isAbstract && !tpe.typeSymbol.isClass && isNotDealiasedFurther(tpe)
-    )) /*&& intersectionMembersStrong*/ ||
-    tpe.typeParams.exists {
+    ))) /*&& intersectionMembersStrong*/ ||
+    tpe.typeParams.exists { // is identity
       t =>
         t == tpe.typeSymbol ||
         t.typeSignature == tpe.typeSymbol.typeSignature ||

diff --git a/izumi-reflect/izumi-reflect/src/main/scala-2/izumi/reflect/TagMacro.scala b/izumi-reflect/izumi-reflect/src/main/scala-2/izumi/reflect/TagMacro.scala
@@ -282,7 +282,7 @@ class TagMacro(val c: blackbox.Context) {
           // skip resolution for types in methods/vals (that would need a new runtime constructor, `methodTag`, like `refinedTag` for the case & dealing with method type parameters may be non-trivial)
           // see: "progression test: can't handle parameters in defs/vals in structural types"
           symbol.isTerm ||
-          ReflectionUtil.isSelfStrong(symbol.info)
+          ReflectionUtil.isSelfStrong(Set.empty, symbol.info)
       }
 
     val strongDeclsTpe = internal.refinedType(intersection, originalRefinement.typeSymbol.owner, internal.newScopeWith(strongDecls.toSeq: _*))
@@ -354,7 +354,7 @@ class TagMacro(val c: blackbox.Context) {
 
   @inline
   private[this] final def getCtorKindIfCtorIsTypeParameter(tpe: Type): Option[Kind] = {
-    if (!ReflectionUtil.isSelfStrong(tpe)) Some(kindOf(tpe))
+    if (!ReflectionUtil.isSelfStrong(Set.empty, tpe)) Some(kindOf(tpe))
     else None
   }
 

diff --git a/izumi-reflect/izumi-reflect/src/main/scala-2/izumi/reflect/macrortti/LightTypeTagImpl.scala b/izumi-reflect/izumi-reflect/src/main/scala-2/izumi/reflect/macrortti/LightTypeTagImpl.scala
@@ -91,7 +91,6 @@ final class LightTypeTagImpl[U <: Universe with Singleton](val u: U, withCache:
     logger.log(s"Initial mainTpe=$tpe:${tpe.getClass} beforeDealias=$tpe0:${tpe0.getClass}")
 
     val lttRef = makeRef(tpe)
-//    println(s"$tpe0 => $tpe => $lttRef")
 
     val allReferenceComponents = allTypeReferences(tpe)
 
@@ -243,14 +242,17 @@ final class LightTypeTagImpl[U <: Universe with Singleton](val u: U, withCache:
 
       IzAssert(
         assertion = {
-          if (componentsOfPolyTypeResultType.maybeUnbrokenType.nonEmpty) {
-            componentsOfPolyTypeResultType.intersectionComponents.exists(_.etaExpand.typeParams.nonEmpty)
-          } else true
+          if (componentsOfPolyTypeResultType.maybeUnbrokenType.isEmpty) {
+            !componentsOfPolyTypeResultType.intersectionComponents.exists(_.takesTypeArgs)
+          } else {
+            true
+          }
         },
         clue = {
           s"""Unexpected intersection contains a PolyType:
              |tpeRaw0 = $tpeRaw0
              |components = ${componentsOfPolyTypeResultType.intersectionComponents.niceList(prefix = "*")}
+             |takesTypeArgs = ${componentsOfPolyTypeResultType.intersectionComponents.map(_.takesTypeArgs).niceList(prefix = "*")}
              |etaExpand = ${componentsOfPolyTypeResultType.intersectionComponents.map(_.etaExpand).niceList(prefix = "+")}
              |tparams = ${componentsOfPolyTypeResultType.intersectionComponents.map(_.etaExpand.typeParams).niceList(prefix = "-")}
              |""".stripMargin
@@ -260,7 +262,7 @@ final class LightTypeTagImpl[U <: Universe with Singleton](val u: U, withCache:
       componentsOfPolyTypeResultType.intersectionComponents.toSeq.flatMap {
         component =>
           val componentAsPolyType = component.etaExpand
-          val tparams = component.etaExpand.typeParams
+          val tparams = componentAsPolyType.typeParams
 
           if (tparams.isEmpty) {
             Seq.empty

diff --git a/...eflect/izumi-reflect/src/main/scala-3/izumi/reflect/dottyreflection/FullDbInspector.scala b/...eflect/izumi-reflect/src/main/scala-3/izumi/reflect/dottyreflection/FullDbInspector.scala
@@ -86,7 +86,8 @@ abstract class FullDbInspector(protected val shift: Int) extends InspectorBase {
           processSymbol(typeRef, selfRef)
 
         case paramRef: ParamRef =>
-          processSymbol(paramRef, selfRef)
+          // do not process type parameters for bases db
+          Nil
 
         case termRef: TermRef =>
           extractBase(termRef, selfRef, false)

diff --git a/...izumi-reflect/src/main/scala-3/izumi/reflect/dottyreflection/InheritanceDbInspector.scala b/...izumi-reflect/src/main/scala-3/izumi/reflect/dottyreflection/InheritanceDbInspector.scala
@@ -23,7 +23,7 @@ abstract class InheritanceDbInspector(protected val shift: Int) extends Inspecto
     val tpe0 = TypeRepr.of[T].dealias
 
     val allReferenceComponents = allTypeReferences(tpe0).filter {
-      case _: ParamRef => false
+      case _: ParamRef => false // do not process type parameters for inheritance db
       case _ => true
     }
 

diff --git a/izumi-reflect/izumi-reflect/src/test/scala-2/izumi/reflect/test/DiscoveryModel.scala b/izumi-reflect/izumi-reflect/src/test/scala-2/izumi/reflect/test/DiscoveryModel.scala
@@ -0,0 +1,17 @@
+package izumi.reflect.test
+
+object DiscoveryModel {
+
+  trait NodeId
+  trait DiscoverableService {
+    type DiscoveryNode <: NodeId
+  }
+  trait DiscoveryNodeProvider[Id <: NodeId]
+  trait DiscoverableServiceImpl extends DiscoverableService {
+    override type DiscoveryNode = NodeIdImpl
+  }
+  class NodeIdImpl extends NodeId
+
+  type GetDiscoveryNode[T <: DiscoverableService] = T#DiscoveryNode
+
+}
diff --git a/izumi-reflect/izumi-reflect/src/test/scala-2/izumi/reflect/test/TagProgressionTest.scala b/izumi-reflect/izumi-reflect/src/test/scala-2/izumi/reflect/test/TagProgressionTest.scala
@@ -6,25 +6,6 @@ class TagProgressionTest extends SharedTagProgressionTest {
 
   "[progression] Tag (Scala 2)" should {
 
-    "progression test: we may accidentally materialize tags for type parameters that are prefixes of type projections (Scala 2 specific, generic type projection)" in {
-      class Path {
-        type Child
-      }
-      val path = new Path
-
-      // A has no tag and the definition of `getTag` should not compile at all. It's a bug that it compiles
-      def getTag[A <: Path]: Tag[A#Child] = Tag[A#Child]
-
-      val directChildTag = Tag[Path#Child].tag // Path::Child
-      val indirectChildTag = getTag[path.type].tag // A|<Nothing..Path>::Child
-
-      assertDifferent(indirectChildTag, directChildTag)
-      assertNotChild(directChildTag, indirectChildTag)
-      assertNotChild(indirectChildTag, directChildTag)
-
-      assert(indirectChildTag.toString == "A|<Nothing..Path>::Child")
-    }
-
     "progression test: combine intersection path-dependent intersection types with inner tags doesn't work yet (Scala 2 specific, generic type projection)" in {
       trait PDT {
         type T

diff --git a/izumi-reflect/izumi-reflect/src/test/scala-2/izumi/reflect/test/TagTest.scala b/izumi-reflect/izumi-reflect/src/test/scala-2/izumi/reflect/test/TagTest.scala
@@ -20,6 +20,7 @@ package izumi.reflect.test
 
 import izumi.reflect._
 import izumi.reflect.macrortti._
+import org.scalatest.exceptions.TestFailedException
 
 class TagTest extends SharedTagTest {
 
@@ -67,6 +68,37 @@ class TagTest extends SharedTagTest {
       assertCompiles("HKTag.hktagFromTagMacro[({ type l[F[_]] = { type Arg[C] = F[C] } })#l[Option]]")
     }
 
+    "we no longer accidentally materialize tags for type parameters that are prefixes of type projections (Scala 2 specific, generic type projection)" in {
+      class Path {
+        type Child
+      }
+      val path = new Path
+
+      // A has no tag and the definition of `getTag` should not compile at all. It's a bug that it compiles
+      val t = intercept[TestFailedException](
+        assertCompiles(
+          """
+          def getTag[A <: Path]: Tag[A#Child] = Tag[A#Child]
+          """
+        )
+      )
+      assert(
+        t.getMessage.contains("could not find implicit value") ||
+        t.getMessage.contains("no implicit argument of type") /*Dotty*/
+      )
+
+      def getTag[A <: Path](implicit t: Tag[A#Child]): Tag[A#Child] = Tag[A#Child]
+
+      val directChildTag = Tag[Path#Child].tag // Path::Child
+      val indirectChildTag = getTag[path.type].tag // A|<Nothing..Path>::Child
+
+      assertDifferent(indirectChildTag, directChildTag)
+      assertNotChild(directChildTag, indirectChildTag)
+      assertNotChild(indirectChildTag, directChildTag)
+
+      assertRepr(indirectChildTag, "path::Child")
+    }
+
   }
 
 }
diff --git a/izumi-reflect/izumi-reflect/src/test/scala-3/izumi/reflect/test/DiscoveryModel.scala b/izumi-reflect/izumi-reflect/src/test/scala-3/izumi/reflect/test/DiscoveryModel.scala
@@ -0,0 +1,20 @@
+package izumi.reflect.test
+
+object DiscoveryModel {
+
+  trait NodeId
+  trait DiscoverableService {
+    type DiscoveryNode <: NodeId
+  }
+  trait DiscoveryNodeProvider[Id <: NodeId]
+  trait DiscoverableServiceImpl extends DiscoverableService {
+    override type DiscoveryNode = NodeIdImpl
+  }
+  class NodeIdImpl extends NodeId
+
+  type GetDiscoveryNode[T <: DiscoverableService] <: NodeId = T match {
+    case GetDiscoveryNodePattern[t] => t
+  }
+  type GetDiscoveryNodePattern[Id <: NodeId] = DiscoverableService { type DiscoveryNode = Id }
+
+}
diff --git a/...flect/izumi-reflect/src/test/scala/izumi/reflect/test/CurrentDottySupportExtentTest.scala b/...flect/izumi-reflect/src/test/scala/izumi/reflect/test/CurrentDottySupportExtentTest.scala
@@ -65,9 +65,6 @@ trait CurrentDottySupportExtentTest extends TagAssertions {
     assertChild(tupleTag1, tupleTag0)
     assertChild(tupleTag3, tupleTag2)
 
-//      println(`LTT[_]`[List].debug())
-//      println(`LTT[_]`[List[B]].debug())
-
     assertChild(LTT[List[B]], LTT[Seq[A]])
     assertChild(`LTT[_]`[List].combine(LTT[B]), `LTT[_]`[Seq].combine(LTT[A]))
   }

diff --git a/izumi-reflect/izumi-reflect/src/test/scala/izumi/reflect/test/SharedLightTypeTagTest.scala b/izumi-reflect/izumi-reflect/src/test/scala/izumi/reflect/test/SharedLightTypeTagTest.scala
@@ -1,8 +1,7 @@
 package izumi.reflect.test
 
-import izumi.reflect.macrortti.LightTypeTagRef.{AbstractReference, AppliedNamedReference, AppliedReference, Boundaries, FullReference, NameReference, SymName, TypeParam}
+import izumi.reflect.macrortti.LightTypeTagRef.{AbstractReference, AppliedNamedReference, Boundaries, FullReference, NameReference, TypeParam}
 import izumi.reflect.macrortti._
-import izumi.reflect.test.TestModel.W3
 
 import scala.collection.immutable.ListSet
 import scala.collection.{BitSet, immutable, mutable}
@@ -511,11 +510,6 @@ abstract class SharedLightTypeTagTest extends TagAssertions {
       val alias = LTT[T3[Int, Boolean]]
       val direct = LTT[W1 with W4[Boolean] with W5[Int]]
 
-      println(debugCtor)
-      println(debugCombined)
-      println(alias.debug("alias"))
-      println(direct.debug("direct"))
-
       assert(!debugCtor.contains("<refinement>"))
       assert(!debugCtor.contains("<none>"))
       assert(!debugCtor.contains("- T"))
@@ -584,11 +578,6 @@ abstract class SharedLightTypeTagTest extends TagAssertions {
       val w3 = `LTT[_]`[W3]
       val w4 = `LTT[_]`[W4]
 
-      println(t1.debug("T1[_]"))
-      println(t2.debug("T2[_]"))
-      println(w3.debug("W3[_]"))
-      println(w4.debug("W4[_]"))
-
       assertChild(t1, w3)
       assertChild(t1, LTT[W1])
       assertChild(w4, w3)