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

Use BSP for IntelliJ import rather than our custom intellij tasks? #778

Open
4 of 6 tasks
retronym opened this issue Jun 17, 2021 · 9 comments
Open
4 of 6 tasks

Use BSP for IntelliJ import rather than our custom intellij tasks? #778

retronym opened this issue Jun 17, 2021 · 9 comments

Comments

@retronym
Copy link
Member

retronym commented Jun 17, 2021

Remaining issues:


IntelliJ could never correctly import our project as a standard SBT project. IIRC that was due to an assumption that scala-library was a third party dependency, whereas in our project we have it as an internal sub-project.

Instead, we have some custom SBT tasks that fill in template IntelliJ config from ./src/intellij/*.SAMPLE and combine it with result of the update task to fill in the classpath.

SBT 1.4.x introduced support for the Build Server Protocol (BSP), which is an alternative means to import the project structure into IntelliJ (or Metals, VSCode, ...).

We should try to migrate to this as the standard way to use our project in an IDE of the users choice.

Status

  • Checkout 2.13.x
  • Run sbt once to generate ./bsp/sbt.json
  • In Intellij, "File, New Project from Existing Sources, $CODE/scala, BSP"

The import was successful for me. In the SBT shell I left running, I saw a notification that a new client has connected.

Here's what I saw in the Build->Sync output in IntelliJ:

Processing workspace/buildTargets
Processing buildTarget/sources
compiling 533 Scala sources and 33 Java sources to /Users/jz/code/scala-worktrees/scala-bsp-intellij/build/quick/classes/library ...
/Users/jz/code/scala-worktrees/scala-bsp-intellij/src/library/scala/reflect/package.scala:61:27: @nowarn annotation does not suppress any warnings
    if (!m.isAccessible: @nowarn("cat=deprecation")) {
                          ^
one warning found
compiling 160 Scala sources and 3 Java sources to /Users/jz/code/scala-worktrees/scala-bsp-intellij/build/quick/classes/reflect ...
/Users/jz/code/scala-worktrees/scala-bsp-intellij/src/reflect/scala/reflect/internal/util/AbstractFileClassLoader.scala:114:43: @nowarn annotation does not suppress any warnings
    case null => super.getPackage(name): @nowarn("cat=deprecation")
                                          ^
one warning found
compiling 326 Scala sources and 5 Java sources to /Users/jz/code/scala-worktrees/scala-bsp-intellij/build/quick/classes/compiler ...
/Users/jz/code/scala-worktrees/scala-bsp-intellij/src/compiler/scala/tools/nsc/ast/TreeBrowsers.scala:218:76: @nowarn annotation does not suppress any warnings
      val menuKey = Toolkit.getDefaultToolkit().getMenuShortcutKeyMask(): @nowarn("cat=deprecation") // deprecated since JDK 10, replacement only available in 10+
                                                                           ^
one warning found
compiling 61 Scala sources and 1 Java source to /Users/jz/code/scala-worktrees/scala-bsp-intellij/test/benchmarks/target/scala-2.13/classes ...
41 deprecations (since 2.13.0)
1 deprecation (since 2.13.2)
10 deprecations (since 2.13.3)
52 deprecations in total; re-run with -deprecation for details
four warnings found
Processing buildTarget/dependencySources
Updating 
Resolved  dependencies
Updating 
Resolved  dependencies
Fetching artifacts of 
Fetched artifacts of 
Updating 
Resolved  dependencies
Fetching artifacts of 
Fetched artifacts of 
Fetching artifacts of 
Fetched artifacts of 
Fetching artifacts of 
Fetched artifacts of 
Fetching artifacts of 
Fetched artifacts of 
Fetching artifacts of 
Fetched artifacts of 
Fetching artifacts of 
Fetched artifacts of 
Processing buildTarget/scalacOptions

Notes:

  • Why does it need to compile the sources? Does this mean that we can't re-import the project structure if there is an error in the sources somewhere?
  • What's with the missing project names after "Fetched artifacts of "?
  • Per-project scala compiler options look to be correctly propagated, including -srcpath on the library project
  • There appear to be extra intra-project dependencies on the "Test" scope. This is harmless but it would be preferable if BSP could more accurately reflect the true dependencies.
  • Clicking Run on a JUnit tests Just Works!
  • We no longer get a sub-project for the SBT build definition itself, so lose all code-assist in there.
@lrytz
Copy link
Member

lrytz commented Jun 17, 2021

I've been using IntelliJ (including JUnit tests) in this mode for a few months and it's overall working well.

I had to start sbt in a terminal before doing an import / build refresh in IntelliJ, at least in the past, maybe that's fixed now.

I think the main drawback is the long delay until the IDE is ready when the build changes (common when switching branches - not switching betweeen 2.12 and 2.13, but even switching between 2.13 branches). And losing code-assist in the build itself.

@Jasper-M
Copy link

Jasper-M commented Jun 17, 2021

Sorry if this message is redundant, but if you can use bloop instead of the sbt server you should still get code-assist in the build.

@retronym
Copy link
Member Author

Sorry if this message is redundant, but if you can use bloop instead of the sbt server you should still get code-assist in the build.

I'm working on adding to SBT's BSP server too. WIP: retronym/sbt#3

@retronym
Copy link
Member Author

retronym commented Jun 17, 2021

I think the main drawback is the long delay until the IDE is ready when the build changes

I think the problem here is that evaluating bench / Jmh / bspBuildTargetSourcesItem actually runs JMH source code generator. This is what triggers compilation of library. IntelliJ would be perfectly happy knowing that bench/target/scala-2.13/src_managed will be a root directory for generated sources, but the BSP server instead eagerly generates the sources and provides the full list of them.

I guess it does this so that builds the directly override the behaviour of managedSources are correctly represented, rather than relying on the convention that they should only return files within one of managedSourceDirectories. I guess for many types of generated sources the user wants them eagerly generated so they can code against the APIs. I feel like this should be a separate BSP operation though that isn't on the critical path to getting the project loaded.

We could probably override:

    bspBuildTargetSourcesItem := {
      val id = bspTargetIdentifier.value
      val dirs = unmanagedSourceDirectories.value
      val managed = managedSources.value
      val items = (dirs.toVector map { dir =>
        SourceItem(dir.toURI, SourceItemKind.Directory, generated = false)
      }) ++
        (managed.toVector map { x =>
          SourceItem(x.toURI, SourceItemKind.File, generated = true)
        })
      SourcesItem(id, items)
    }

In the scala build to be:

val dirs = unmanagedSourceDirectories.value ++ managedSourceDirectories.value
val items = (dirs.toVector map { dir =>
    SourceItem(dir.toURI, SourceItemKind.Directory, generated = false)
  })
  SourcesItem(id, items)

Or, we might be able to tag the bench / Jmh scope with the no-ide tag so it isn't imported at all. We should still be able to edit the code.

@retronym
Copy link
Member Author

retronym commented Jun 28, 2021

I've just noticed another problem: the "Refresh All BSP Targets" action doesn't pick up changes to the build definitions that occurred during the lifetime of the BSP server process: https://youtrack.jetbrains.com/issue/SCL-19230 [fixed]

@lrytz
Copy link
Member

lrytz commented Sep 2, 2021

On current 2.13.x, importing / refreshing the BSP build still trigger compilation of the compiler (and library, reflect). I managed to narrow it down to the buildTarget/resources call, which runs bspBuildTargetResources file:/Users/luc/scala/scala13/#repl/Compile [... a lot more ...] in sbt. The contender is bspBuildTargetResources file:/Users/luc/scala/scala13/#scaladoc/Compile, it can be reproduced by just calling scaladoc/resources in sbt, which triggers compiler/compile.

I'm out of time for today, but it should hopefully be an easy fix.

@lrytz
Copy link
Member

lrytz commented Sep 2, 2021

scala/scala#9749

@lrytz
Copy link
Member

lrytz commented Sep 3, 2021

updated the issue description with a list of remaining issues

@jastice
Copy link

jastice commented Apr 7, 2022

https://youtrack.jetbrains.com/issue/SCL-19300 has been fixed in the meantime

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