[Scala3] Add Definition/Instance API#5277
Conversation
| /** Determine whether underlying proto is of type provided. | ||
| * | ||
| * @note IMPORTANT: this function requires summoning a TypeTag[B], which will fail if B is an inner class. | ||
| * @note IMPORTANT: this function requires summoning a ClassTag[B], which will fail if B is an inner class. |
There was a problem hiding this comment.
Is this statement still true? I asked Opus 4.7 to review and it said ClassTags can be summoned for inner classes, can we add a test for this and delete the note?
There's also a chance that we could capture the ClassTag for A in Scala 3 that might be able to make this whole thing work a lot better and just use <:<. We can save that for a follow on PR though.
There was a problem hiding this comment.
Another alternative is to use the new TypeTest, the following works:
case class Box[A](a: A) {
def isA[B](using tt: TypeTest[A, B]): Boolean = tt.unapply(a).isDefined
}
Box(List(1, 2, 3)).isA[Seq[Int]] // trueThat has same limitation about type parameterized types though. Maybe we should consider izumi-reflect?
There was a problem hiding this comment.
Ok I confirmed that ClassTags work with inner classes, updated comment. In general I agree the isA code can be refactored, we can discuss this for follow up work?
There was a problem hiding this comment.
Yeah I think refactoring is a future work thing.
| case TypeBounds(lo, hi) if lo =:= hi => lo | ||
| case TypeBounds(_, hi) => hi |
There was a problem hiding this comment.
Doesn't the 2nd line also implement the first line?
2 participants
Tested locally with (a working subset of) InstanceSpec and DefinitionSpec - will move all tests once other unsupported APIs are ported
Contributor Checklist
docs/src?Type of Improvement
Desired Merge Strategy
Release Notes
Reviewer Checklist (only modified by reviewer)
3.6.x,5.x, or6.xdepending on impact, API modification or big change:7.0)?Enable auto-merge (squash)and clean up the commit message.Create a merge commit.