Closed cacoco closed 4 years ago
Yep, tested this case against scala-guice 4.2.8 (guice 4.2.3, Scala 2.12.10 and JDK8) and it appears to work:
Welcome to Scala 2.12.10 (TwitterJDK 64-Bit Server VM, Java 1.8.0_222).
Type in expressions for evaluation. Or try :help.
scala> case class UserId(id: Long) extends AnyVal
defined class UserId
scala> import net.codingwell.scalaguice._
import net.codingwell.scalaguice._
scala> typeLiteral[UserId]
res0: com.google.inject.TypeLiteral[UserId] = UserId
So looks tied to changes in https://github.com/codingwell/scala-guice/pull/88.
As far as I can remember, it was because some types weren't mapped right even with Manifests.
typeLiteral[List[_]] shouldEqual new TypeLiteral[List[_]] {}
Also, your issue with AnyVal might be a dup of #49 which predates TypeTags
But you are more than welcome to try and revert https://github.com/codingwell/scala-guice/commit/ac3437e346c25790329d3bf785b93007291657da
As far as Dotty and scala.reflect. Who knows. While it seems like Scala and metals has made great strides recently I'm having a hard time selling Scala to my coworkers and I'll probably next year be proposing moving to C#9. The java ecosystem is a tire fire and the lack of async await frankly makes even rust look like a better alternative.
I hate type things.
I bet it was: https://github.com/codingwell/scala-guice/commit/c817b0974031b254cc9a4de0d598566b7e12484f
Which I snuck in. I was trying to avoid doing a try catch, but i probably have to
Actually yeah I just bisected, its this change: https://github.com/codingwell/scala-guice/commit/c817b0974031b254cc9a4de0d598566b7e12484f
Nothing is ever easy
Made a new test and fixed it with a try catch. I hate having normal flow in the catch block but it is what it is. I'll see about buttoning up the outstanding pr and make a new version.
@tsuckow tangentially: have you seen: https://github.com/scala/scala/pull/8816 (and https://github.com/scala/scala/releases/tag/v2.13.3, https://github.com/retronym/monad-ui/pull/1)?
I had not seen that / scala-async for that matter. I should think about using those on my scala project, would save me a mountain of boiler plate.
4.2.10 should be synced to maven central in the next few hours.
Sounds, good. Thanks!
Offtopic: Thanks for pointing out scala-async. It's been a life saver. I don't do much scala work these days so I've fallen quite behind.
Problem
The
TypeConversions#scalaTypeToJavaType
ends up creating a TypeLiteral of the wrapped type of a value class instead of the type of the value class. Guice however will bind to the value class type as the TypeLiteral of the Key. Thus, any code path that goes throughnet.codingwell.scalaguice.typeLiteral[T]
will end up with an incongruous TypeLiteral. This is true both when binding and then trying to@Inject
a type, or when looking up an already bound type from the injector via theInjectorExtensions
.We're using scalaguice 4.2.9, guice 4.2.3, Scala 2.12.10 on JDK 8.
A simple example:
This is different than the result before the switch from Manifests to TypeTags (using scalaguice 4.2.0 and guice 4.2.1 with Scala 2.12.10 and JDK8):
Why is this potentially bad? A somewhat more involved example:
Guice binds with a Key with type
scala.Function1<UserId, com.twitter.util.Future<java.lang.Object>>
but we end up looking for a Key based onscala.Function1<java.lang.Long, com.twitter.util.Future<java.lang.Boolean>>
. Looking through the codebase, I don't see tests for this case though it seems like a regression. Maybe this is related to the fix for Mixins, we're going to test against the version before that fix.We're still attempting to upgrade from scalaguice 4.2.0 to a recent version for the Finatra framework but are hitting various type conversion issues from our extensive usage internally. Any help would be appreciated.