Scala/Java mixed projects fail to compile using processorpath bloop + metals about bloop HOT 7 CLOSED

Hi @gersonsosa

Not sure if you were saying that you'd like to look at or fix this yourself. If so, the code responsible for adding the semanticdb plugin is here...

Currently it adds the semanticdb plugin to the classpath and adds various javac options. I guess it could test to see if -processorpath exists and then classpath can be left unchanged and -processorpath could be altered to include the semanticdb plugin path.

I note here that it's recommended to use the classpath unless you're using annotation processors so I don't know if there would be an issue moving to -processorpath in all cases, maybe a performance hit?

It would also need to be changed in Metals here...

https://github.com/scalameta/metals/blob/cce28f192c1625a0b77c09fef63ea63cd0b83bc0/metals/src/main/scala/scala/meta/internal/metals/JavaInteractiveSemanticdb.scala#L59-L72

and possibly here but I don't know this area...

https://github.com/scalameta/metals/blob/cce28f192c1625a0b77c09fef63ea63cd0b83bc0/sbt-metals/src/main/scala/scala/meta/metals/MetalsPlugin.scala#L109-L130

from bloop.

It looks correct what @Arthurm1 wrote. Let us know if you are interesting in helping out!

If you can't, it would be awesome if you created a small repro that we can take a look like. So probably a small project that uses a preprocessor.

from bloop.

What I would like some guidance is if I can include a sample project for example here as a test in the code or it would be best to just test the parts I change independently and where should I put those tests

I think using a sample project is perfectly fine, we already have some tests that use it. For example in the BspProtocolSuite. We have a method that loads up those test project (loadBspBuildFromResources)

from bloop.

@gersonsosa I think you're right. It looks like the on-the-fly generation ignores the java options.

I'm looking to try to change how this code works so I'd recommend not bothering to change Metals.

from bloop.

@Arthurm1 @tgodzik thanks for your response, sure I’m in for helping out, while playing with the code it seems that with the changes in bloop

1. here (by altering the processorpath string including semanticdb path)

   bloop/frontend/src/main/scala/bloop/data/Project.scala

   Lines 477 to 504 in 7a7a913

   	def enableJavaSemanticdbOptions(options: List[String]): List[String] = {
   	if (hasJavaSemanticDBEnabledInCompilerOptions(options)) options
   	else {
   	val semanticdbOptions =
   	s"-Xplugin:semanticdb -sourceroot:${workspaceDir} -targetroot:javac-classes-directory" :: options
   	if (project.javaVersionAtLeast("17", logger))
   	List(
   	"-J--add-exports",
   	"-Jjdk.compiler/com.sun.tools.javac.api=ALL-UNNAMED",
   	"-J--add-exports",
   	"-Jjdk.compiler/com.sun.tools.javac.code=ALL-UNNAMED",
   	"-J--add-exports",
   	"-Jjdk.compiler/com.sun.tools.javac.model=ALL-UNNAMED",
   	"-J--add-exports",
   	"-Jjdk.compiler/com.sun.tools.javac.tree=ALL-UNNAMED",
   	"-J--add-exports",
   	"-Jjdk.compiler/com.sun.tools.javac.util=ALL-UNNAMED"
   	) ::: semanticdbOptions
   	else semanticdbOptions
   	}
   	}
   	def enableJavaSemanticdbClasspath(
   	pluginPath: AbsolutePath,
   	classpath: List[AbsolutePath]
   	): List[AbsolutePath] = {
   	if (classpath.contains(pluginPath)) classpath else pluginPath :: classpath
   	}

2. also looking at whether it’s necessary to include the classpath if the processorpath is there, but this change seems optional

   bloop/frontend/src/main/scala/bloop/data/Project.scala

   Lines 530 to 541 in 7a7a913

   	javaSemanticDBPlugin match {
   	case None =>
   	scalaProjectWithRangePositions
   	case Some(pluginPath) =>
   	val javacOptionsWithSemanticDB = enableJavaSemanticdbOptions(javacOptions)
   	val classpathWithSemanticDB =
   	enableJavaSemanticdbClasspath(pluginPath, scalaProjectWithRangePositions.rawClasspath)
   	scalaProjectWithRangePositions.copy(
   	javacOptions = javacOptionsWithSemanticDB,
   	rawClasspath = classpathWithSemanticDB
   	)
   	}

things run smooth once again.

What I would like some guidance is if I can include a sample project for example here as a test in the code or it would be best to just test the parts I change independently and where should I put those tests

additionally I haven’t checked metals code but I can give a try after having a PR for bloop.

from bloop.

@tgodzik @Arthurm1 I finally got some time to work on this, I made a small PR to fix this, I included small project in the test resources

I'll now look into metals now, I'm not sure why we have to change it there though

from bloop.

To elaborate on why I'm not sure I have to change metals usages of javac

I think the parts where metals compile java sources using javac do not rely on the build tool to generate the classpath of the command, therefore they will never have the -processorpath option when executing (please correct me if I'm wrong)

1. for on the fly generation https://github.com/scalameta/metals/blob/8283260dc275a82dea0261ad3aaec632892014b2/metals/src/main/scala/scala/meta/internal/metals/JavaInteractiveSemanticdb.scala#L63
2. for the project settings https://github.com/scalameta/metals/blob/8283260dc275a82dea0261ad3aaec632892014b2/sbt-metals/src/main/scala/scala/meta/metals/MetalsPlugin.scala#L67

I added a small test to a branch here it seems to work as expected https://github.com/gersonsosa/metals/tree/fix-java-options-processorpath

from bloop.