After upgrade to 1.1.0, scala-reflect is required as a dependency on JVM

[sideeffffect]

We upgraded to portable-scala-reflect 1.1.0 in this PR: zio/zio#4619 . If scala-reflect would not be newly added as a dependency, the compilation failed with

[error] /home/ondra/Projects/zio/core-tests/jvm/src/test/scala/zio/system/SystemSpec.scala:8:22: object io is not a member of package reflect
[error] import scala.reflect.io.File
[error]                      ^

But the dependency is added only as "org.scala-lang" % "scala-reflect" % scalaVersion.value % Test. That caused issues for users of ZIO who wanted to run tests via the ZIO's sbt runner:

java.lang.NoClassDefFoundError: scala/reflect/api/TypeCreator
    at zio.test.sbt.BaseTestTask.specInstance$lzycompute(BaseTestTask.scala:18)

That was solved by adding scala-reflect as a regular Compile dependency in this PR: zio/zio#4645

/cc @sjrd

[sjrd]

Thanks for the issue. Now I see things more clearly. So apparently the compilation error happened on some code where you directly use APIs from scala-reflect: scala.reflect.io is only part of that dependency.

With portable-scala-reflect v1.0.0, you happened to receive that artifact as a transitive dependency of portable-scala-reflect. However, that was not intentional, and it was fixed in d4f5323c6ed62da36d9326d7ecc9b9faa6cc8178. Now that dependency doesn't leak to users of portable-scala-reflect anymore. But for you, you were actually depending on that dependency for reasons unrelated to portable-scala-reflect. To me it seems legit that you needed to add that dependency. (Or use java.io.File instead of scala.reflect.io.File; perhaps even that was not intentional, and was just the result of an IDE's auto-import feature?)

[sideeffffect]

However, that was not intentional, and it was fixed in d4f5323.

:+1: great, that would explain why the dependency "suddenly appeared" in compile time for us. But even if I change scala.reflect.io.File to java.io.File and then the compilation succeeds, the runtime failure is still there:

java.lang.NoClassDefFoundError: scala/reflect/api/TypeCreator
    at zio.test.sbt.BaseTestTask.specInstance$lzycompute(BaseTestTask.scala:18)
    at zio.test.sbt.BaseTestTask.specInstance(BaseTestTask.scala:15)
    at zio.test.sbt.BaseTestTask.execute(BaseTestTask.scala:42)

https://app.circleci.com/pipelines/github/zio/zio/10026/workflows/b328a36a-42b0-4ed4-a0e4-b8d68d948701/jobs/103118

Happens in this code: https://github.com/zio/zio/blob/master/test-sbt/shared/src/main/scala/zio/test/sbt/BaseTestTask.scala#L15-L23 Even though we're using Reflect.lookupLoadableModuleClass from org.portablescala.reflect.

That seems like an error in portable-scala-reflect, but I'm not sure :thinking:

[sjrd]

Hum, that is really weird. The stack trace suggests that you're not entering any frame from portable-scala-reflect, so it somehow suggests that the error is not there, although I also don't see anything that would prompt a TypeCreator in your code. I would have blamed the macros of portable-scala-reflect, but you're not using any; you're using a perfectly regular def defined at https://github.com/portable-scala/portable-scala-reflect/blob/2e2fb55e5406028b50b133e238420bfeacbd6dd2/jvm/src/main/scala/org/portablescala/reflect/Reflect.scala#L86-L89

Could it be that this is a scalac codegen issue, somehow? I'm not sure. It all looks very weird to me, and I don't see anything to blame either in portable-scala-reflect nor zio.

[sideeffffect]

The class that lookupLoadableModuleClass tries to instantiate is AbstractRunnableSpec. Could this be happening because AbstractRunnableSpec depends on the scala.reflect.api.TypeCreator class? (For the record, AbstractRunnableSpec does have the org.portablescala.reflect.annotation.EnableReflectiveInstantiation annotation.)

[sideeffffect]

For the record, the Scala 2.x part of zio-test uses scala.reflect.macros.{TypecheckException, blackbox} and scala.reflect.macros.whitebox.Context, but what's striking is that even tests compiled with Dotty (that shouldn't have to do anything with scala-reflect) fail with java.lang.NoClassDefFoundError: scala/reflect/api/TypeCreator https://app.circleci.com/pipelines/github/zio/zio/10027/workflows/42612819-9cc2-41f5-9eaa-70a6909efcf4/jobs/103135

[sideeffffect]

@adamgfraser are you familiar with the zio-test code? Do you have any idea why it would fail with Dotty because of missing scala-reflect (scala.reflect.api.TypeCreator in particular) even though, it shouldn't be using anything from scala-reflect (unlike Scala 2.x that uses scala.reflect.macros.{TypecheckException, blackbox} and scala.reflect.macros.whitebox.Context).

[adamgfraser]

@sideeffffect The main area where scala-reflect is used is in the SBT Test Runner to reflectively instantiate the tests so I would expect that it would fail without that.

[sideeffffect]

But zio-test-sbt now uses portable-scala-reflect's Reflect.lookupLoadableModuleClass. And that shouldn't be using scala.reflect.api.TypeCreator (or anything else from scala-reflect), am I saying that right, @sjrd ?

[adamgfraser]

Just looking at the error message above the NoClassDefFoundError occurred in zio.test.sbt.BaseTestTask which is exactly where the tests are instantiated. So I think it is somehow related to that.

[sjrd]

And that shouldn't be using scala.reflect.api.TypeCreator (or anything else from scala-reflect), am I saying that right, @sjrd ?

That's right. portable-scala-reflect uses Java reflection only on the JVM.

[sideeffffect]

And yet, when we try to instantiate some class (a subclass of AbstractRunnableSpec in this case in zio-test-sbt runner), it for some reason expects scala-reflect's TypeCreator to be present and fails because it's not possible. Now that looks like a bug. Maybe in portable-scala-reflect, but much more likely in ZIO (perhaps in zio-test?). @adamgfraser I have an experimentation PR where I'm playing with this, if you would like to have a look: https://github.com/zio/zio/pull/4659/files

[keynmol]

@sjrd we hit the same issue in Weaver, so if I'm reading the build and these comments correctly:

1. scala-reflect is needed on the jvm as compile-time dependency (we use portable-scala-reflect for suite instantiation)
   • it's marked as provided in the portable's build
2. scala js and scala native don't need/can't have scala-reflect dependency

So the configuration I added is following:

    libraryDependencies ++= {
      if (virtualAxes.value.contains(VirtualAxis.jvm)) {
        if (scalaVersion.value.startsWith("3."))
          Seq("org.scala-lang" % "scala-reflect" % scala213)
        else
          Seq("org.scala-lang" % "scala-reflect" % scalaVersion.value)
      } else Seq.empty
    }

It worked on the previous version because scala-reflect was pulled transitively by our dependency on Expecty

[sideeffffect]

@keynmol Just to be sure, you're also having issues with java.lang.NoClassDefFoundError: scala/reflect/api/TypeCreator? If yes, that would suggest that the root of the issue is not in how we use (portable-)scala-reflect in ZIO, but somewhere else (possibly in (portable-)scala-reflect).

But I can't see any TypeCreator error in your linked PR

In ZIO, we're using working around it with this https://github.com/zio/zio/blob/27091c813a578944946b96b3214d41e5a5293126/project/BuildHelper.scala#L292-L299

[keynmol]

@sideeffffect nope, we had exactly the same problem: https://cloud.drone.io/disneystreaming/weaver-test/679/1/2

The PR I linked is after me adding those settings https://github.com/portable-scala/portable-scala-reflect/issues/33#issuecomment-781222000

Which, now that I'm re-reading @sjrd's comment - it says Java reflection, not scala-reflect.

So you're saying that scala-reflect shouldn't be needed at all if using portable-scala-reflect? That smells like magic to me :D

[sideeffffect]

At this point, we need to have a minimal example that demonstrates (or not) that even when one uses portable-scala-reflect (which allegedly requires only Java's reflection), it still fails on missing scala-reflect (with java.lang.NoClassDefFoundError: scala/reflect/api/TypeCreator). If we have a proof like that, it should be helpful to portable-scala-reflect developers to fix this hypothetical issue. So who's volunteering? :grin:

[keynmol]

I'll need this anyway to verify we can safely upgrade, I'll do it after work today.

[keynmol]

Here's the project: https://github.com/keynmol/portable-scala-reflect-repro-1

And here's the error: https://github.com/keynmol/portable-scala-reflect-repro-1/runs/1929641170?check_suite_focus=true#step:5:24

the repro doesn't use scala reflect directly, but portable still fails.

I should've just used Scastie. Why do I keep doing this to myself :D

[keynmol]

@sideeffffect can you check my reproduction and see if I missed anything?

[sideeffffect]

Yep, that pretty much proves it. Thank you so much for the effort :bow: Hopefully that's now something that @sjrd or others can work with :+1: