Handle java source/target defined at extension level

[Arthurm1]

This will take into account the java plugin source/target compatibility settings.
e.g. using...

java {
  targetCompatibility = JavaVersion.VERSION_1_8
  sourceCompatibility = JavaVersion.VERSION_1_9
}

instead of

tasks.withType(JavaCompile) {
  sourceCompatibility = JavaVersion.VERSION_1_8
  targetCompatibility = JavaVersion.VERSION_1_9
}

I'm not sure if this is enough to fix redhat-developer/vscode-java#3721. The source/target compatibility in JVMBuildTarget will have changed but the javac options will still contain --release 8. Hopefully that will be ok for the eclipse settings.

I note that here it says to configure the compileJava task with source/target and not the java plugin when using Jabel

I've also extended the POJO constructor copying used in DefaultSourceSets to the language extensions. It's not needed for normal running but it's a pain when debugging as all the variables/watches just show the proxy classes.

[jdneo]

I've also extended the POJO constructor copying used in DefaultSourceSets to the language extensions. It's not needed for normal running but it's a pain when debugging as all the variables/watches just show the proxy classes.

There is an issue about this: #175. I also did some investigation and turns out the copy constructor is not needed for running.

I'm thinking that, for debugging purpose, maybe override toString() is a better approach than using the copy constructor?

---

Update:

The copy constructor is required. If we remove all the copy constructor, here:

build-server-for-gradle/model/src/main/java/com/microsoft/java/bs/gradle/model/actions/GetSourceSetsAction.java

Line 174 in 395b977

if (sourceSet instanceof DefaultGradleSourceSet) {

The sourceset is a proxy wrapperred type, and it will encounter some problem

[Arthurm1]

The copy constructor is required. If we remove all the copy constructor, here:

Shall I leave my changes to DefaultJavaExtension/DefaultScalaExtension then or do you want them gone?

[jdneo]

Shall I leave my changes to DefaultJavaExtension/DefaultScalaExtension

I think you can leave as it is. Until we find a way to get rid of the copy constructor entirely

[jdneo]

@Arthurm1

I think we should use javaCompile.getSourceCompatibility() and javaCompile.getTargetCompatibility().

AFAIK, there are three different kind of ways to set the source/target level.

java {
  targetCompatibility = JavaVersion.VERSION_1_8
  sourceCompatibility = JavaVersion.VERSION_1_9
}

tasks.withType(JavaCompile) {
  sourceCompatibility = JavaVersion.VERSION_1_8
  targetCompatibility = JavaVersion.VERSION_1_9
}

sourceCompatibility = JavaVersion.VERSION_1_8
targetCompatibility = JavaVersion.VERSION_1_9

The current approach cannot get the version in case 2. While using the JavaCompile instance can cover all three cases.

Considering JavaCompile is official API which no need to use reflection, I prefer to use it directly.

[jdneo]

Hi @Arthurm1, would you like to update the PR?

Or do you have any concern about javaCompile.getSourceCompatibility() and javaCompile.getTargetCompatibility()

[Arthurm1]

@jdneo Will look at it this weekend and probably change the PR

[jdneo]

@Arthurm1 Thank you.

My thought is that we can just simply get the source/target level from JavaCompile, no need to inspecting from compile args. (Correct me if this is working for any cases)

[Arthurm1]

@jdneo I've changed it so that the source/target compatibility is populated purely from the JavaCompile task. JavacOptions are still passed back for BSP but they are ignored for compatibility settings.

This seems to reflect more how people use the Gradle settings, i.e. set source/target compatibility in JavaCompile for IDE usage and the JavacOptions for jar and release.

[jdneo]

Thank you @Arthurm1