scala / scala-dev

Scala 2 team issues. Not for user-facing bugs or directly actionable user-facing improvements. For build/test/infra and for longer-term planning and idea tracking. Our bug tracker is at https://github.com/scala/bug/issues
Apache License 2.0
130 stars 14 forks source link

Running compiler JUnit tests in IntelliJ through BSP fails #786

Closed lrytz closed 3 years ago

lrytz commented 3 years ago

Running compiler JUnit tests (e.g. scala.tools.nsc.backend.jvmBytecodeTest) in IntelliJ after importing scala/scala 2.13.x through BSP (Import Project from Existing Sources) fails:

java.lang.NoSuchMethodError: 'boolean scala.tools.nsc.symtab.SymbolTable.openPackageModule$default$2()'

    at scala.tools.nsc.symtab.SymbolLoaders$PackageLoader.doComplete(SymbolLoaders.scala:312)
    at scala.tools.nsc.symtab.SymbolLoaders$SymbolLoader.$anonfun$complete$2(SymbolLoaders.scala:249)
    at scala.runtime.java8.JFunction0$mcV$sp.apply(JFunction0$mcV$sp.scala:18)
    at scala.reflect.internal.SymbolTable.informingProgress(SymbolTable.scala:85)
    at scala.tools.nsc.symtab.SymbolLoaders$SymbolLoader.complete(SymbolLoaders.scala:247)
    at scala.reflect.internal.Symbols$Symbol.completeInfo(Symbols.scala:1561)
    at scala.reflect.internal.Symbols$Symbol.info(Symbols.scala:1533)
    at scala.reflect.internal.Mirrors$RootsBase.init(Mirrors.scala:262)
    at scala.tools.nsc.Global.rootMirror$lzycompute(Global.scala:75)
    at scala.tools.nsc.Global.rootMirror(Global.scala:73)
    at scala.tools.nsc.Global.rootMirror(Global.scala:45)
    at scala.reflect.internal.Definitions$DefinitionsClass.ObjectClass$lzycompute(Definitions.scala:287)
    at scala.reflect.internal.Definitions$DefinitionsClass.ObjectClass(Definitions.scala:287)
    at scala.reflect.internal.Definitions$DefinitionsClass.init(Definitions.scala:1643)
    at scala.tools.nsc.Global$Run.<init>(Global.scala:1226)
    at scala.tools.testkit.Compiler.newRun(BytecodeTesting.scala:75)
    at scala.tools.testkit.Compiler.compileToBytes(BytecodeTesting.scala:89)
    at scala.tools.testkit.Compiler.compileClasses(BytecodeTesting.scala:96)
    at scala.tools.nsc.backend.jvm.BytecodeTest.$anonfun$t10812$1(BytecodeTest.scala:25)
    at scala.tools.nsc.backend.jvm.BytecodeTest.$anonfun$t10812$1$adapted(BytecodeTest.scala:24)
    at scala.collection.immutable.List.foreach(List.scala:333)
    at scala.tools.nsc.backend.jvm.BytecodeTest.t10812(BytecodeTest.scala:24)

It works when using the sbt intellij generated scala.ipr to open the project.

I'm happy to take a look; maybe someone has an idea how to debug this?

dwijnand commented 3 years ago

Intelligent ideas only, please.

dwijnand commented 3 years ago

https://github.com/scala/scala/commit/8a15b1c0ce77402d330b692e828e3f8f681b8e92 is where I added that default argument.

dwijnand commented 3 years ago

So presumably BSP has instructed that the scala-compiler to use while running the tests is probably STARR rather than the just-built compiler.

lrytz commented 3 years ago

Turns out both the STARR and the just-built compiler are on the classpath (used a breakpoint and "evaluate expression" of System.getProperty("java.class.path")).

The reason is that in the IntelliJ module config, the "Scala SDK" module dependency (STARR) adds the STARR to the run-time classpath ("Standard library" in the screenshot below):

image

Making the "Standard library" empty in the Scala SDK dependency fixes things (doing it only for the "junit" module is not enough, it needs to be done for all modules "junit" depends on).

Enabling IntelliJ's "BSP trace log" we see what IntelliJ gets from SBT (here for the "library" module):

    {
      "id": {
        "uri": "file:/Users/luc/scala/scala13/#library/Compile"
      },
      "displayName": "library",
      "baseDirectory": "file:/Users/luc/scala/scala13/src/library/",
      "tags": [
        "library"
      ],
      "languageIds": [
        "scala"
      ],
      "dependencies": [],
      "capabilities": {
        "canCompile": true,
        "canTest": true,
        "canRun": true,
        "canDebug": false
      },
      "dataKind": "scala",
      "data": {
        "scalaOrganization": "org.scala-lang",
        "scalaVersion": "2.13.6",
        "scalaBinaryVersion": "2.13",
        "platform": 1,
        "jars": [
          "file:/Users/luc/Library/Caches/Coursier/v1/https/repo1.maven.org/maven2/org/scala-lang/scala-library/2.13.6/scala-library-2.13.6.jar",
          "file:/Users/luc/Library/Caches/Coursier/v1/https/repo1.maven.org/maven2/org/scala-lang/scala-compiler/2.13.6/scala-compiler-2.13.6.jar",
          "file:/Users/luc/Library/Caches/Coursier/v1/https/repo1.maven.org/maven2/org/scala-lang/scala-reflect/2.13.6/scala-reflect-2.13.6.jar",
          "file:/Users/luc/Library/Caches/Coursier/v1/https/repo1.maven.org/maven2/org/jline/jline/3.19.0/jline-3.19.0.jar",
          "file:/Users/luc/Library/Caches/Coursier/v1/https/repo1.maven.org/maven2/net/java/dev/jna/jna/5.3.1/jna-5.3.1.jar"
        ]
      }
    },

For an ordinary Scala project, the scala library needs to be on the runtime classpath (IntelliJ puts also reflect / compiler, but that's not too problematic). But things are special here because we use STARR only to compile. We somehow need to get this bit across the wire.

lrytz commented 3 years ago

Maybe an additional tag in the build target? https://build-server-protocol.github.io/docs/specification.html#build-target

lrytz commented 3 years ago

Or extend the data for scala with an optional field (https://build-server-protocol.github.io/docs/extensions/scala.html)

lrytz commented 3 years ago

In the meantime:

$> brew install xmlstarlet

$> for f in $(find $(git rev-parse --show-toplevel)/.idea/modules -name '*.iml'); do xml ed --inplace -u '//module/component/orderEntry[@type="module-library"]/library[@type="Scala"]/CLASSES' -v '' $f; done

The changes are discarded when IntelliJ reloads the BSP project.

lrytz commented 3 years ago

@adpi2 do you have any thoughts about this issue?

adpi2 commented 3 years ago

Could this be a bug in IntelliJ?

The data field in the buildTarget response contains the scalaInstance which is needed for compiling.

In contrast there is a classpath field in the scalacOptions response. For ordinary Scala projects it contains the scala-library. For the Compile/library module it only contains the class directory, not the STARR library:

{
      "target": {
        "uri": "file:/home/piquerez/scala/scala/#library/Compile"
      },
      "options": [
        "-Xplugin:/home/piquerez/.cache/coursier/v1/https/repo1.maven.org/maven2/org/scalameta/semanticdb-scalac_2.13.6/4.4.20/semanticdb-scalac_2.13.6-4.4.20.jar",
        "-P:semanticdb:sourceroot:/home/piquerez/scala/scala",
        "-P:semanticdb:targetroot:/home/piquerez/scala/scala/target/library/meta",
        "-Yrangepos",
        "-P:semanticdb:synthetics:on",
        "-P:semanticdb:failures:warning",
        "-Wconf:cat\u003dunchecked\u0026msg\u003dThe outer reference in this type test cannot be checked at run time.:s",
        "-Wconf:cat\u003doptimizer:is",
        "-Wconf:cat\u003dunused-nowarn:s",
        "-deprecation",
        "-feature",
        "-sourcepath",
        "/home/piquerez/scala/scala/src/library",
        "-Xlint",
        "-feature"
      ],
      "classpath": [
        "file:/home/piquerez/scala/scala/build/quick/classes/library/"
      ],
      "classDirectory": "file:/home/piquerez/scala/scala/build/quick/classes/library/"
    },

For the Test/junit module it does not contain the STARR library either:

{
      "target": {
        "uri": "file:/home/piquerez/scala/scala/#junit/Test"
      },
      "options": [
        "-Xplugin:/home/piquerez/.cache/coursier/v1/https/repo1.maven.org/maven2/org/scalameta/semanticdb-scalac_2.13.6/4.4.20/semanticdb-scalac_2.13.6-4.4.20.jar",
        "-P:semanticdb:sourceroot:/home/piquerez/scala/scala",
        "-P:semanticdb:targetroot:/home/piquerez/scala/scala/target/junit/meta",
        "-Yrangepos",
        "-P:semanticdb:synthetics:on",
        "-P:semanticdb:failures:warning",
        "-Wconf:cat\u003dunchecked\u0026msg\u003dThe outer reference in this type test cannot be checked at run time.:s",
        "-Wconf:cat\u003doptimizer:is",
        "-Wconf:cat\u003dunused-nowarn:s",
        "-deprecation",
        "-feature",
        "-Xlint:-valpattern,_",
        "-Wconf:msg\u003dmatch may not be exhaustive:s",
        "-Wconf:cat\u003dlint-nullary-unit\u0026site\u003d.*Test:s",
        "-Ypatmat-exhaust-depth",
        "40",
        "-P:semanticdb:targetroot:/home/piquerez/scala/scala/target/junit/test-meta"
      ],
      "classpath": [
        "file:/home/piquerez/scala/scala/build/quick/classes/testkit/",
        "file:/home/piquerez/scala/scala/build/quick/classes/compiler/",
        "file:/home/piquerez/scala/scala/build/quick/classes/junit",
        "file:/home/piquerez/scala/scala/target/junit/test-classes/",
        "file:/home/piquerez/scala/scala/build/quick/classes/repl-frontend/",
        "file:/home/piquerez/scala/scala/build/quick/classes/library/",
        "file:/home/piquerez/scala/scala/build/quick/classes/interactive/",
        "file:/home/piquerez/scala/scala/build/quick/classes/scaladoc/",
        "file:/home/piquerez/scala/scala/build/quick/classes/repl/",
        "file:/home/piquerez/scala/scala/build/quick/classes/reflect/",
        "file:/home/piquerez/.cache/coursier/v1/https/repo1.maven.org/maven2/com/novocode/junit-interface/0.11/junit-interface-0.11.jar",
        "file:/home/piquerez/.cache/coursier/v1/https/repo1.maven.org/maven2/org/openjdk/jol/jol-core/0.13/jol-core-0.13.jar",
        "file:/home/piquerez/.cache/coursier/v1/https/repo1.maven.org/maven2/com/googlecode/java-diff-utils/diffutils/1.3.0/diffutils-1.3.0.jar",
        "file:/home/piquerez/.cache/coursier/v1/https/repo1.maven.org/maven2/junit/junit/4.13.2/junit-4.13.2.jar",
        "file:/home/piquerez/.cache/coursier/v1/https/repo1.maven.org/maven2/org/scala-lang/modules/scala-asm/9.2.0-scala-1/scala-asm-9.2.0-scala-1.jar",
        "file:/home/piquerez/.cache/coursier/v1/https/repo1.maven.org/maven2/org/jline/jline/3.20.0/jline-3.20.0.jar",
        "file:/home/piquerez/.cache/coursier/v1/https/repo1.maven.org/maven2/net/java/dev/jna/jna/5.8.0/jna-5.8.0.jar",
        "file:/home/piquerez/.cache/coursier/v1/https/repo1.maven.org/maven2/org/webjars/jquery/3.5.1/jquery-3.5.1.jar",
        "file:/home/piquerez/.cache/coursier/v1/https/repo1.maven.org/maven2/org/scala-sbt/test-interface/1.0/test-interface-1.0.jar",
        "file:/home/piquerez/.cache/coursier/v1/https/repo1.maven.org/maven2/org/hamcrest/hamcrest-core/1.3/hamcrest-core-1.3.jar"
      ],
      "classDirectory": "file:/home/piquerez/scala/scala/target/junit/test-classes/"
    },

IntelliJ should trust this classpath field as being the runtime classpath.

lrytz commented 3 years ago

@jastice what do you think?

When I import a simple sbt project as "sbt" project (not through BSP), the module has a single <library type="Scala"> dependency which sets the compiler-classpath (library/reflect/compiler jars) and adds the scala-library.jar to <CLASSES> / <SOURCES> ("Standard library" in the UI).

Importing the same project through bsp (not bloop) adds 3 dependencies to the module:

  1. a type="Scala" library which has the library/reflect/compiler jars in both the compiler-classpath and the <CLASSES> classpath (no <SOURCES>)
  2. a library with scala-library-2.13.6.jar (<CLASSES> and <SOURCES>)
  3. a thrid library called "BSP: [project-name] dependencies" with library/reflect/compiler jars on <SOURCES>

It seems 3. should be merged with 1. But I agree with @adpi2 that it seems better to completely remove the library/reflect/compiler jars from <CLASSES> / <SOURCES> of the Scala SDK library.

jastice commented 3 years ago

@lrytz Just for context what's happening here:

  1. The Scala plugin BSP client creates a specific library for the Scala SDK libraries obtained from data in ScalaBuildTarget from BSP.
  2. A heuristic creates project-level libraries out of the jars in the classpath of scalaOptions and tries to match sources and binaries by name. This fails when they don't share name and path.
  3. The unmatched jars/sources go into a module-level library.

We could merge/deduplicate scala libraries with the SDK libs, but would it help with this?

lrytz commented 3 years ago

@jastice thanks for the reply.

The problem for us is that the "Scala SDK" dependency created by the BSP client puts the Scala jars (compiler+reflect+library) into the "Standard library" section of the SDK, not only into the "Compiler classpath"

image

This is a. not entirely correct: the compiler should not be on the runtime classpath (unless added in sbt as libraryDependency) b. not necessary, because the scala-library.jar is also added to the module classpath as a separate dependency

Usually this doesn't matter, because the compiler's Scala library version matches the Scala library version at runtime, adding it twice is OK. But in our case these two are different.

From the bsp log:

[Trace - 09:09:56 PM] Received response 'workspace/buildTargets - (3)' in 75ms
...
    {
      "id": {
        "uri": "file:/Users/luc/tmp/sbtproj/#sbtproj/Compile"
      },
      "displayName": "sbtproj",
      "baseDirectory": "file:/Users/luc/tmp/sbtproj/",
      "tags": [
        "library"
      ],
      "languageIds": [
        "scala"
      ],
      "dependencies": [],
      "capabilities": {
        "canCompile": true,
        "canTest": true,
        "canRun": true,
        "canDebug": false
      },
      "dataKind": "scala",
      "data": {
        "scalaOrganization": "org.scala-lang",
        "scalaVersion": "2.13.6",
        "scalaBinaryVersion": "2.13",
        "platform": 1,
        "jars": [
          "file:/Users/luc/Library/Caches/Coursier/v1/https/repo1.maven.org/maven2/org/scala-lang/scala-library/2.13.6/scala-library-2.13.6.jar",
          "file:/Users/luc/Library/Caches/Coursier/v1/https/repo1.maven.org/maven2/org/scala-lang/scala-compiler/2.13.6/scala-compiler-2.13.6.jar",
          "file:/Users/luc/Library/Caches/Coursier/v1/https/repo1.maven.org/maven2/org/scala-lang/scala-reflect/2.13.6/scala-reflect-2.13.6.jar",
          "file:/Users/luc/Library/Caches/Coursier/v1/https/repo1.maven.org/maven2/org/jline/jline/3.19.0/jline-3.19.0.jar",
          "file:/Users/luc/Library/Caches/Coursier/v1/https/repo1.maven.org/maven2/net/java/dev/jna/jna/5.3.1/jna-5.3.1.jar"
        ]
      }
    },
...

the above jars are needed for running the compiler, but should not be on the runtime classpath

[Trace - 09:09:57 PM] Received response 'buildTarget/scalacOptions - (7)' in 222ms
...
    {
      "target": {
        "uri": "file:/Users/luc/tmp/sbtproj/#sbtproj/Compile"
      },
      "options": [],
      "classpath": [
        "file:/Users/luc/tmp/sbtproj/target/scala-2.13/classes",
        "file:/Users/luc/Library/Caches/Coursier/v1/https/repo1.maven.org/maven2/org/scala-lang/scala-library/2.13.6/scala-library-2.13.6.jar"
      ],
      "classDirectory": "file:/Users/luc/tmp/sbtproj/target/scala-2.13/classes"
    },
...

BSP exports the scala-library jar on module's classpath (and IntelliJ correctly adds that dependency to the module).

jastice commented 3 years ago

@lrytz Ah thanks for the clarification. That shouldn't be hard to fix. I'll appreciate if you open an issue on https://youtrack.jetbrains.com/issues/SCL or a PR if you get the chance :)

lrytz commented 3 years ago

I actually checked out the repo (first time) yesterday and looked around for a while, but didn't find my way through. But it seems I'm more lucky today 😊

lrytz commented 3 years ago

Fixed in latest nightly (2021.2.448)