Support the SBT extension in BSP import #6553

retronym · 2021-06-18T09:06:23Z

This enables code assist in the .sbt and project/*.scala files in IntelliJ (pending merge of the related PR to intelli-scala).

Other IDEs that support BSP import could also take advantage of this; scala-metals has a ticket to add support: scalameta/metals-feature-requests#143

To disable this feature (e.g. if using an IDE version that does not yet support the sbt extension in the BSP protocl, set ThisBuild / bspSbtEnabled := false.

Fixes #5953

Requires: JetBrains/intellij-scala#593

main/src/main/scala/sbt/Keys.scala

main/src/main/scala/sbt/internal/server/BuildServerProtocol.scala

adpi2 · 2021-06-18T13:13:26Z

Looks very promising so far. Thank you @retronym for taking care of this!

retronym · 2021-06-22T23:24:58Z

I should get back to this later today, thanks for the review comments so far.

The corresponding change to IntelliJ is just about ready to land: JetBrains/intellij-scala#593

retronym · 2021-06-23T22:02:02Z

This one is ready for another round of review. I'll try to address attaching SBT source JARs to the project in a followup PR.

main/src/main/scala/sbt/internal/server/BuildServerProtocol.scala

This enables code assist in the .sbt and project/*.scala files in IntelliJ and any other IDEs that implement this extension.

- Restore old type of `bspWorkspace` key for backwards compat. Instead, introduce `bspFullWorkspace` that includes the SBT targets - Log a warning if the client requests, e.g. `scalaMainClasses` for a SBT target - Refactor the code that creates the SBT build targets so it doesn't depend on `sbtFullWorkspace`. - Add a setting `bspSbtEnabled` to let the user opt-opt of SBT target export (e.g. to compensate for a client that does not yet support them)

eed3si9n

LGTM, but I yield the detail review to Adrien.

eed3si9n

LGTM, but I yield the detail review to Adrien.

adpi2 · 2021-06-28T08:45:28Z

main/src/main/scala/sbt/EvaluateTask.scala

+  // Not a constructor parameter to avoid a binary incompatibility
+  private[this] var _buildSources: BuildSources = BuildSources.Empty
+  private[sbt] def setBuildSourcesData(data: BuildSources): PluginData = {
+    _buildSources = data; this
+  }
+  private[sbt] def buildSourcesData: BuildSources = _buildSources


@eed3si9n Is it really necessary to preserve the binary compatibility of PluginData? How bad would it be to break it in sbt 1.6?

I am asking this because I also want to add more parameters in it for #6566.

The breakage is just for the constructor, copy, and apply factory methods, not for consumers of the data. I think it is highly unlikely to break existing plugins.

Is it really necessary to preserve the binary compatibility of PluginData? How bad would it be to break it in sbt 1.6?

We maintain binary compatibility for the purpose of maintaining plugin compatibility, so I think PluginData would be ok to break, but ultimately we the only way to quantify this would be to release a milestone and test it using Community Build. If none of the existing builds break, then we should be ok.

adpi2

The 2 comments below should be addressed before merging. Otherwise I would suggest to update the test here:

sbt/server-test/src/test/scala/testpkg/BuildServerTest.scala

Lines 24 to 35 in e4231ac

    
           test("workspace/buildTargets") { _ => 
        
             svr.sendJsonRpc( 
        
               """{ "jsonrpc": "2.0", "id": "16", "method": "workspace/buildTargets", "params": {} }""" 
        
             ) 
        
             assert(processing("workspace/buildTargets")) 
        
             assert { 
        
               svr.waitForString(10.seconds) { s => 
        
                 (s contains """"id":"16"""") && 
        
                 (s contains """"displayName":"util"""") 
        
               } 
        
             } 
        
           }

and to add a new test for the sources of the build.sbt target.

main/src/main/scala/sbt/internal/Load.scala

main/src/main/scala/sbt/internal/server/BuildServerProtocol.scala

retronym · 2021-06-29T06:42:20Z

Update: I'm currently trying to extend the BSP test. Having some difficulty there as one of my new tests for sources (sbt build) works in isolation but not after the exising test for normal sources. Haven't figured out the crosstalk yet.

- Pass source dirs and current list of files - Align display name and URI

adpi2

I just pushed a small fix for the new buildTarget/resources request. Otherwise it looks good to me as it is.
I tried in Metals and most of the things (goto definitions and code completions) are already working out-of-the-box. That's cool!

retronym · 2021-07-01T04:59:16Z

@eed3si9n Are you happy with this one now, in particular deeming PluginData creation/copying as sbt-internal and excluded from MiMa?

retronym · 2021-07-01T05:03:33Z

There was a scalacheck test failure in one of the test runs, not related to this PR, but maybe worth looking into:

[info] - sbt.ParseKey.An unspecified project axis resolves to the current project: Falsified after 4341 passed tests
[info] > StructureKeyMask(Env:
[info]     Tasks:
[info]     aa (delegates: )
[info] Build file:///aaaaaaa/#a :
[info]     Project a
[info]       Delegates:
[info]         
[info]       Configurations:
[info]         A
[info]     Project b
[info]       Delegates:
[info]         
[info]       Configurations:
[info]         A
[info]     Project aa
[info]       Delegates:
[info]         
[info]       Configurations:
[info]         A
[info] current: ProjectRef(file:///aaaaaaa/#a,a)
[info] Settings:
[info] 	Global / <key>
[info] 	   aa
[info] All keys:
[info] 	aa,ScopedKey(Scope(Select(BuildRef(file:///aaaaaaa/#a)), Select(ConfigKey(A)), Select(aa), Zero),aa),ScopeMask(false,false,true,false))
[info] > === Not Equal ===
[info] > --- lhs ---
[info] > Select(ConfigKey(A))
[info] > --- rhs ---
[info] > Zero
[info] > === Not Equal ===
[info] > --- lhs ---
[info] > Select(ProjectRef(file:///aaaaaaa/#a,aa))
[info] > --- rhs ---
[info] > Select(ProjectRef(file:///aaaaaaa/#a,a))
[info] > parsed subproject: Select(ProjectRef(file:///aaaaaaa/#a,aa))
[info] > current subproject: ProjectRef(file:///aaaaaaa/#a,a)
[info] > Key: {file:///aaaaaaa/#a} / A / aa / aa
[info] > Mask: ScopeMask(false,false,true,false)
[info] > Key string: 'aa / aa'
[info] > Parsed: Right(ProjectRef(uri("file:///aaaaaaa/#a"), "aa") / aa)
[info] > Structure: Env:
[info]     Tasks:
[info]     aa (delegates: )
[info] Build file:///aaaaaaa/#a :
[info]     Project a
[info]       Delegates:
[info]         
[info]       Configurations:
[info]         A
[info]     Project b
[info]       Delegates:
[info]         
[info]       Configurations:
[info]         A
[info]     Project aa
[info]       Delegates:
[info]         
[info]       Configurations:
[info]         A
[info] current: ProjectRef(file:///aaaaaaa/#a,a)
[info] Settings:
[info] 	Global / <key>
[info] 	   aa
[info] All keys:
[info] 	aa

eed3si9n · 2021-07-01T05:11:43Z

@retronym I'm guessing it's a known ambiguity when we have subproject named aa and a task key aa, aa / aa can be interpreted as "aa task in aa project", not "aa task scoped to aa task in current subproject", and thus "An unspecified project axis resolves to the current project" gets falsified. I probably should try to filter that out of the test.

I'm fine with PluginData. I guess the question is if we should call this a 1.6.0 feature or 1.5.x. We can publish 1.6.0-M1, run Community Build, and then back port it to 1.5.x if it passes?

retronym · 2021-07-02T00:07:14Z

I did a GitHub code search for "plugindata language:scala", filtered out files with package sbt, and didn't spot any usages of the changed methods.

$ gh api --paginate -X GET search/code -f q='language:scala plugindata' -q ".items[] | .url" > /tmp/urls.txt
$ for url in $(cat /tmp/urls.txt); do echo $url; code=$(gh api $url  -q ".content" | base64 --decode); echo $code | grep 'package sbt' || echo $code; done | tee /tmp/code.txt

Not a perfect method, but I think it is likely safe enough for 1.5.x. @adpi2 mentioned that he also has a followup change to add extra data to PluginData.

eed3si9n · 2021-07-05T16:51:00Z

ok. We can merge it now, and see.

retronym force-pushed the topic/bsp-sbt-target branch from 14b2eee to 256adb8 Compare June 18, 2021 09:08

retronym mentioned this pull request Jun 18, 2021

SCL-18372 Support SBT extension in BSP import JetBrains/intellij-scala#593

Merged

adpi2 reviewed Jun 18, 2021

View reviewed changes

main/src/main/scala/sbt/Keys.scala Outdated Show resolved Hide resolved

adpi2 reviewed Jun 18, 2021

View reviewed changes

main/src/main/scala/sbt/Keys.scala Outdated Show resolved Hide resolved

adpi2 reviewed Jun 18, 2021

View reviewed changes

main/src/main/scala/sbt/internal/server/BuildServerProtocol.scala Show resolved Hide resolved

adpi2 reviewed Jun 18, 2021

View reviewed changes

main/src/main/scala/sbt/internal/server/BuildServerProtocol.scala Outdated Show resolved Hide resolved

retronym force-pushed the topic/bsp-sbt-target branch 3 times, most recently from ff2db6f to 05ba7d7 Compare June 23, 2021 04:17

retronym mentioned this pull request Jun 23, 2021

Support diagnostics in sbt scalameta/metals-feature-requests#143

Open

retronym marked this pull request as ready for review June 23, 2021 22:02

eed3si9n reviewed Jun 24, 2021

View reviewed changes

main/src/main/scala/sbt/internal/server/BuildServerProtocol.scala Outdated Show resolved Hide resolved

adpi2 reviewed Jun 24, 2021

View reviewed changes

main/src/main/scala/sbt/internal/server/BuildServerProtocol.scala Outdated Show resolved Hide resolved

retronym added 3 commits June 28, 2021 11:02

Support the SBT extension in BSP import

45e8e2f

This enables code assist in the .sbt and project/*.scala files in IntelliJ and any other IDEs that implement this extension.

Add scalacOptions to the SBT targets

111cc5f

retronym force-pushed the topic/bsp-sbt-target branch 6 times, most recently from 89ae692 to 1dd9cf1 Compare June 28, 2021 01:27

eed3si9n approved these changes Jun 28, 2021

View reviewed changes

eed3si9n previously approved these changes Jun 28, 2021

View reviewed changes

adpi2 reviewed Jun 28, 2021

View reviewed changes

adpi2 requested changes Jun 28, 2021

View reviewed changes

main/src/main/scala/sbt/internal/Load.scala Outdated Show resolved Hide resolved

main/src/main/scala/sbt/internal/server/BuildServerProtocol.scala Outdated Show resolved Hide resolved

Record build sources in PluginData for BSP

0bd736b

retronym dismissed eed3si9n’s stale review via 0bd736b June 28, 2021 13:27

retronym force-pushed the topic/bsp-sbt-target branch from 1dd9cf1 to 0bd736b Compare June 28, 2021 13:27

adpi2 mentioned this pull request Jun 28, 2021

[BSP] Send diagnostics and meaningful error message when reloading fails #6566

Merged

retronym and others added 4 commits June 30, 2021 11:19

Refine and test SBT BSP target

fe04647

- Pass source dirs and current list of files - Align display name and URI

Exclude create/copy methods of PluginData from MiMa's consideration

df5fb17

Fix compilation error in test

3d92827

[BSP] Ignore sbt target in resources request

a825c11

adpi2 approved these changes Jun 30, 2021

View reviewed changes

eed3si9n merged commit 1497686 into sbt:develop Jul 5, 2021

ckipp01 mentioned this pull request Jul 15, 2021

[BSP] - empty.max exception thrown with sbt 1.5.5 when you try to compile your build file. #6598

Closed

adpi2 mentioned this pull request Aug 16, 2023

[BSP] Implement buildTarget/javacOptions #7352

Merged

Support the SBT extension in BSP import #6553

Support the SBT extension in BSP import #6553

retronym commented Jun 18, 2021 •

edited

adpi2 commented Jun 18, 2021

retronym commented Jun 22, 2021 •

edited

retronym commented Jun 23, 2021

eed3si9n left a comment

eed3si9n left a comment

adpi2 Jun 28, 2021

retronym Jun 28, 2021

eed3si9n Jun 28, 2021

adpi2 left a comment

retronym commented Jun 29, 2021

adpi2 left a comment

retronym commented Jul 1, 2021

retronym commented Jul 1, 2021

eed3si9n commented Jul 1, 2021

retronym commented Jul 2, 2021

eed3si9n commented Jul 5, 2021

	test("workspace/buildTargets") { _ =>
	svr.sendJsonRpc(
	"""{ "jsonrpc": "2.0", "id": "16", "method": "workspace/buildTargets", "params": {} }"""
	)
	assert(processing("workspace/buildTargets"))
	assert {
	svr.waitForString(10.seconds) { s =>
	(s contains """"id":"16"""") &&
	(s contains """"displayName":"util"""")
	}
	}
	}

Support the SBT extension in BSP import #6553

Support the SBT extension in BSP import #6553

Conversation

retronym commented Jun 18, 2021 • edited

adpi2 commented Jun 18, 2021

retronym commented Jun 22, 2021 • edited

retronym commented Jun 23, 2021

eed3si9n left a comment

Choose a reason for hiding this comment

eed3si9n left a comment

Choose a reason for hiding this comment

adpi2 Jun 28, 2021

Choose a reason for hiding this comment

retronym Jun 28, 2021

Choose a reason for hiding this comment

eed3si9n Jun 28, 2021

Choose a reason for hiding this comment

adpi2 left a comment

Choose a reason for hiding this comment

retronym commented Jun 29, 2021

adpi2 left a comment

Choose a reason for hiding this comment

retronym commented Jul 1, 2021

retronym commented Jul 1, 2021

eed3si9n commented Jul 1, 2021

retronym commented Jul 2, 2021

eed3si9n commented Jul 5, 2021

retronym commented Jun 18, 2021 •

edited

retronym commented Jun 22, 2021 •

edited