buildTargetScalacOptions and buildTargetJavacOptions are returning empty

[SocksDevil]

After working on extending the BSP testkit for the extension methods, I decided to test the sample repo used here against those methods. It would seem both buildTargetScalacOptions and buildTargetJavacOptions are returning empty responses.

We should try and see what is going on there. Furthermore, after the testkit update is merged, we should take the chance to also make sure those requests are tested.

[SocksDevil]

After doing some digging, I've managed to find where the problem lies: BazelBspServer is only initialized with buildServer, neglecting java and scala specific servers. This is related to the separation of concerns refactor done in #24 .

@gerardd33 I think we need to create a new class that takes all the implementations and serves it in one API. Wdyt? cc @agluszak @abrams27 @magda-aug

[gerardd33]

After doing some digging, I've managed to find where the problem lies: BazelBspServer is only initialized with buildServer, neglecting java and scala specific servers. This is related to the separation of concerns refactor done in #24 .

@gerardd33 I think we need to create a new class that takes all the implementations and serves it in one API. Wdyt? cc @agluszak @abrams27 @magda-aug

I thought about it while doing this and to avoid this problem I made BazelBspServer store as attributes all three and they're initialised in startServer:

this.scalaBuildServer = new ScalaBuildServerImpl(scalaBuildServerService, serverRequestHelpers);
this.javaBuildServer = new JavaBuildServerImpl(javaBuildServerService, serverRequestHelpers);
this.buildServer = new BuildServerImpl(buildServerService, serverRequestHelpers);

But when I think about it now, I missed this spot – the problem seems to be here, right?:

new Launcher.Builder<BuildClient>()
...
.setLocalService(this.buildServer)
...

So this seems to be the place where we need this one hub for all the interfaces (I think @abrams27 asked about it once on the call and we all couldn't think of a reason why we shouldn't split it).

I agree, it looks like we should pass something that aggregates all of the endpoints here. I can try to fix it.

[SocksDevil]

But when I think about it now, I missed this spot – the problem seems to be here, right?:

new Launcher.Builder<BuildClient>()
...
.setLocalService(this.buildServer)
...

Yeah that's exactly it 😄

[gerardd33]

But when I think about it now, I missed this spot – the problem seems to be here, right?:

new Launcher.Builder<BuildClient>()
...
.setLocalService(this.buildServer)
...

Yeah that's exactly it

Okay, I'll do it later today.

[abrams27]

fixed