Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Running compiler JUnit tests in IntelliJ through BSP fails #786

Closed
lrytz opened this issue Sep 1, 2021 · 15 comments
Closed

Running compiler JUnit tests in IntelliJ through BSP fails #786

lrytz opened this issue Sep 1, 2021 · 15 comments

Comments

@lrytz
Copy link
Member

lrytz commented Sep 1, 2021

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
Copy link
Member

dwijnand commented Sep 1, 2021

Intelligent ideas only, please.

@dwijnand
Copy link
Member

dwijnand commented Sep 1, 2021

scala/scala@8a15b1c is where I added that default argument.

@dwijnand
Copy link
Member

dwijnand commented Sep 1, 2021

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
Copy link
Member Author

lrytz commented Sep 2, 2021

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
Copy link
Member Author

lrytz commented Sep 2, 2021

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

@lrytz
Copy link
Member Author

lrytz commented Sep 2, 2021

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

@lrytz
Copy link
Member Author

lrytz commented Sep 2, 2021

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
Copy link
Member Author

lrytz commented Sep 2, 2021

@adpi2 do you have any thoughts about this issue?

@adpi2
Copy link
Member

adpi2 commented Sep 6, 2021

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
Copy link
Member Author

lrytz commented Sep 7, 2021

@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
Copy link

jastice commented Sep 7, 2021

@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
Copy link
Member Author

lrytz commented Sep 7, 2021

@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
Copy link

jastice commented Sep 7, 2021

@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
Copy link
Member Author

lrytz commented Sep 8, 2021

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 added a commit to lrytz/intellij-scala that referenced this issue Sep 8, 2021
When importing a project through bsp, the compiler classpath
(scala-library, scala-reflect, scala-compiler) obtained through
bsp should only be used for the "Compiler classpath" section of the
Scala SDK.

Before this change, the classpath was also added to the module's
`<CLASSES>` ("Standard library" section of the Scala SDK in the UI).
This is
  - slightly incorrect, because it adds the scala-compiler jar to the
    module's classpath
  - not needed, because the scala-library is added to the module's
    classpath as a separate library

scala/scala-dev#786 (comment)
@lrytz
Copy link
Member Author

lrytz commented Sep 30, 2021

Fixed in latest nightly (2021.2.448)

@lrytz lrytz closed this as completed Sep 30, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants