Override gradle's compiler version from compiler args.

[errael]

Fix #4704. This applies to both -source and -target. This information is used by editor/hints.

With this patch and the following build.gradle excerpt

tasks.withType(JavaCompile).each {
    it.options.compilerArgs << '--source' << '19'
    it.options.compilerArgs << '--target' << '11'
    it.options.compilerArgs << '-Xlint:deprecation' << '--enable-preview'
}

sourceCompatibility = 9
targetCompatibility = 9

There's

and the following has no errors in the editor.

        Object o2 = 3;
        return switch (o2) {
          case Integer i when i >= 0 -> 3; // positive integers
          case Integer i -> 4;             // negative integers
          case default -> 2;
        };

[mbien]

please label PRs before creating them. This will be important in future #4431, otherwise this PR would run no gradle jobs for example.

[errael]

please label PRs before creating them

I knew about the labeling and CI. I'll try to keep that in mind. I didn't think I was able to change/add labels.

[mbien]

please label PRs before creating them

I knew about the labeling and CI. I'll try to keep that in mind. I didn't think I was able to change/add labels.

oh my mistake - disregard what i said. I was sure you were already committer since you are so active (which is great!). I think you have to have at least committer role to be able to label things.

[lkishalmi]

Well, the implementation is Ok. The place is not so much. This is a kind of de-serialization code. So I would rather fix this at the source:

netbeans/extide/gradle/netbeans-gradle-tooling/src/main/java/org/netbeans/modules/gradle/tooling/NbProjectInfoBuilder.java

Line 357 in d27cbbc

model.getInfo().put(propBase + lang + "_compiler_args", new ArrayList<>(compilerArgs));

or right before the usage:

netbeans/java/gradle.java/src/org/netbeans/modules/gradle/java/api/GradleJavaSourceSet.java

Line 137 in d27cbbc

return sourcesCompatibility.getOrDefault(type, DEFAULT_SOURCE_COMPATIBILITY);

[errael]

I thought a builder would be good; immutable description. Looking at your two proposed locations, couldn't get breakpoints to work in NbProjectInfoBuilder, so that made up my mind.

[errael]

errael requested review from lkishalmi and removed request for sdedic 7 minutes ago

Not sure how I removed the request for @sdedic, not my intention. github bug?

[sdedic]

Nitpick: maybe pass the flag source/target explicitly rather than comparing Map instances ?

[errael]

The first implementation did that (the one that was in the wrong place). I'll make that change after any other suggested changes have time to roll in. Thanks for the comment.

[lkishalmi]

Well, I'd reduce the number of ifs: getCompilerArgs() never returns with null, so line 184 could go. The double if in line 188 and 190 could be combined...

[errael]

I remember now why I compared to map instance; because -source/-target are/might-be specific to java. I'll change the method name to fixJavaCompatibility. I'd like to take out the souceType argument, but then the check for JAVA would, would have to be in the caller.

[errael]

K

[mbien]

i am no heavy gradle user. But there is also the --release flag (JEP 247) which is in many ways better than using source+target since it enforces bytecode and API compatibility and avoids a bunch of nasty bugs (edit: one of them is described here, NB got hit by that too). Not sure if this is relevant here, e.g what if the build sets release not source - is this handled too?

[errael]

Yikes. Thanks for something real/detectable.

What to do if --release and one of the others? In any event, a compile will fail.

Arbitrarily use the value with --release. Unless want to consider error dialogs or somesuch, but then why stop there...

[mbien]

What to do if --release and one of the others? In any event, a compile will fail.

it replaces both, if you mix, javac should complain and fail as far as I know. It is similar to setting both source and target to the same value, but more powerful since javac will use ct.sym files which are basically method/class signature diffs and check that your code could actually run on the target JDK APIs.

The maven template uses the source/target tags too still, if we end up adding some kind of JDK or language level selector to the new maven project wizard we could let it use that too if java 9+ is selected.

[errael]

Any further suggestions, comments, questions?

[lkishalmi]

Thank you @errael !

[mbien]

looks good, thanks for making it aware of the --release flag too