TypeConversions produces an incorrect TypeLiteral for Scala value classes

[cacoco]

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 through net.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 the InjectorExtensions.

We're using scalaguice 4.2.9, guice 4.2.3, Scala 2.12.10 on JDK 8.

A simple example:

Welcome to Scala 2.12.10 (JDK 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] = java.lang.Long

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):

Welcome to Scala 2.12.10 (JDK 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

Why is this potentially bad? A somewhat more involved example:

Welcome to Scala 2.12.10 (JDK 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 com.twitter.util.Future
import com.twitter.util.Future

scala> type UserIdFunction = (UserId) => Future[Boolean]
defined type alias UserIdFunction

scala> import javax.inject.Singleton
import javax.inject.Singleton

scala> import com.google.inject.{AbstractModule, Guice, Provides}
import com.google.inject.{AbstractModule, Guice, Provides}

scala> import net.codingwell.scalaguice._
import net.codingwell.scalaguice._

scala> val injector = Guice.createInjector(
     |   new AbstractModule with ScalaModule {
     |     @Provides
     |     @Singleton
     |     def provideUserIdFunction: UserIdFunction = (uid: UserId) => {
     |       if (uid.id > 0) Future.value(true) else Future.value(false)
     |     }
     |   }
     | )
injector: com.google.inject.Injector = Injector{bindings=[InstanceBinding{key=Key[type=com.google.inject.Stage, annotation=[none]], source=[unknown source], instance=DEVELOPMENT}, ProviderInstanceBinding{key=Key[type=com.google.inject.Injector, annotation=[none]], source=[unknown source], scope=Scopes.NO_SCOPE, provider=Provider<Injector>}, ProviderInstanceBinding{key=Key[type=java.util.logging.Logger, annotation=[none]], source=[unknown source], scope=Scopes.NO_SCOPE, provider=Provider<Logger>}, ProviderInstanceBinding{key=Key[type=scala.Function1<UserId, com.twitter.util.Future<java.lang.Object>>, annotation=[none]], source=public scala.Function1 $anon$1.provideUserIdFunction(), scope=Scopes.SINGLETON, provider=@Provides $anon$1.provideUserIdFunction(<console...

scala> import net.codingwell.scalaguice.InjectorExtensions._
import net.codingwell.scalaguice.InjectorExtensions._

scala> injector.instance[UserIdFunction]
com.google.inject.ConfigurationException: Guice configuration errors:

1) No implementation for scala.Function1<java.lang.Long, com.twitter.util.Future<java.lang.Boolean>> was bound.
  while locating scala.Function1<java.lang.Long, com.twitter.util.Future<java.lang.Boolean>>

1 error
  at com.google.inject.internal.InjectorImpl.getProvider(InjectorImpl.java:1120)
  at com.google.inject.internal.InjectorImpl.getInstance(InjectorImpl.java:1126)
  at net.codingwell.scalaguice.InjectorExtensions$ScalaInjector$.instance$extension0(InjectorExtensions.scala:27)
  ... 34 elided

scala>

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 on scala.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.

[cacoco]

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 #88.

[tsuckow]

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 ac3437e

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.

[tsuckow]

I hate type things.

I bet it was:
c817b09

Which I snuck in. I was trying to avoid doing a try catch, but i probably have to

[cacoco]

Actually yeah I just bisected, its this change: c817b09

[tsuckow]

Nothing is ever easy

[tsuckow]

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.

[cacoco]

@tsuckow tangentially: have you seen: scala/scala#8816 (and https://github.com/scala/scala/releases/tag/v2.13.3, retronym/monad-ui#1)?

[tsuckow]

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.

[tsuckow]

4.2.10 should be synced to maven central in the next few hours.

[cacoco]

Sounds, good. Thanks!

[tsuckow]

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.