softwaremill / macwire

Zero-cost, compile-time, type-safe dependency injection library.
https://softwaremill.com/open-source/
Apache License 2.0
1.27k stars 76 forks source link

"value x$2" when wiring a parametrised type #197

Open adamw opened 2 years ago

adamw commented 2 years ago

Using PrometheusMetrics[IO] as a dependency causes problems when compiling bootzooka:

[error] Error while emitting Dependencies.scala
[error] value x$2
[error] one error found

As a work-around, the dependency is created by hand: https://github.com/softwaremill/bootzooka/blob/tapir-update/backend/src/main/scala/com/softwaremill/bootzooka/Dependencies.scala#L36 (note that this is currently on a branch)

To reproduce, change the Dependencies class to:

object Dependencies {
  def wire(
      config: Config,
      sttpBackend: Resource[IO, SttpBackend[IO, Any]],
      xa: Resource[IO, Transactor[IO]],
      clock: Clock
  ): Resource[IO, Dependencies] = {
    def buildHttpApi(
        http: Http,
        userApi: UserApi,
        passwordResetApi: PasswordResetApi,
        versionApi: VersionApi,
        prometheusMetrics: PrometheusMetrics[IO],
        cfg: HttpConfig
    ) = {
      new HttpApi(
        http,
        userApi.endpoints concatNel passwordResetApi.endpoints,
        NonEmptyList.of(prometheusMetrics.metricsEndpoint, versionApi.versionEndpoint),
        prometheusMetrics,
        cfg
      )
    }

    autowire[Dependencies](
      config.api,
      config.user,
      config.passwordReset,
      config.email,
      DefaultIdGenerator,
      PrometheusMetrics.default[IO](registry = CollectorRegistry.defaultRegistry),
      clock,
      sttpBackend,
      xa,
      buildHttpApi _,
      new EmailService(_, _, _, _, _),
      EmailSender.create _,
      new ApiKeyAuthToken(_),
      new PasswordResetAuthToken(_)
    )
  }
}
mbore commented 2 years ago

Thanks for reporting the issue! It's pretty interesting, because when we change PrometheusMetrics.default application to

    autowire[Dependencies](
      config.api,
      config.user,
      config.passwordReset,
      config.email,
      DefaultIdGenerator,
      clock,
      sttpBackend,
      PrometheusMetrics.default[IO](
        namespace = "tapir",
        registry = new CollectorRegistry()
      ),
      xa,
      buildHttpApi _,
      new EmailService(_, _, _, _, _),
      EmailSender.create _,
      new ApiKeyAuthToken(_),
      new PasswordResetAuthToken(_)
    )

it works. It seems then that the problem is around primitive default parameters, but I have not managed to reproduce it in tests yet. It could be fixed with #198 but I'd like to first understand the origin of this bug.

For now, I can see two workarounds: 1) (recommended) assign the result of function application to a variable and then pass the variable to autowire 2) don't use default parameters

adamw commented 2 years ago

I doubt #198 would have any impact as here we are not creating an instance of PrometheusMetrics, so I think autowire never traverses inside that definition?

Interesting that it's the default parameters that are a problem ... do you have a small test case? :)

mbore commented 2 years ago

Right, I thought that we may use autowire to create instances of PrometheusMetrics unintentionally, but it seems it's not the case.

No, unfortunately, I don't have any small test case for it yet :(

mbore commented 2 years ago

I've managed to create a test case for this issue

case class A()
case class B(i: Int, s: String)

object B {
  //the order matters in this case - if we swap params it works
  def default(s: String = "defaultString", i: Int): B = new B(i, s)
}

// `a` param is necessary to reproduce the issue, but order, in this case, doesn't matter 
case class C(a: A, b: B)

object Test {
  val theC = autowire[C](
      B.default(i = 1)
  )
}

val theC = {
  import cats.effect.unsafe.implicits.global
  Test.theC.allocated.unsafeRunSync()._1
}

I think it's a quite specific issue that is easy to work around, so I'm not sure if it makes sense to work on a fix at this point

adamw commented 2 years ago

Hm interesting ;-) Well if there's no obvious fix, it's at least good that we have a test case now - who knows, maybe this will get fixed in some future Scala release :-)