Skip to content

Workaround to IDEA-301084 #2368

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
bulldozer-bot merged 3 commits into from
Sep 1, 2022
Merged

Workaround to IDEA-301084 #2368

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
c829724
Workaround to IDEA-301084
j-baker Sep 1, 2022
ecc0a90
Add generated changelog entries
svc-changelog Sep 1, 2022
8b66787
make clear that this is a hack
j-baker Sep 1, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions changelog/@unreleased/pr-2368.v2.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
type: fix
fix:
description: Workaround to IDEA-301084
links:
- https://github.com/palantir/gradle-baseline/pull/2368
17 changes: 17 additions & 0 deletions ...-java/src/main/groovy/com/palantir/baseline/plugins/javaversions/BaselineJavaVersion.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
import org.gradle.api.tasks.JavaExec;
import org.gradle.api.tasks.TaskAction;
import org.gradle.api.tasks.TaskProvider;
import org.gradle.api.tasks.compile.CompileOptions;
import org.gradle.api.tasks.compile.GroovyCompile;
import org.gradle.api.tasks.compile.JavaCompile;
import org.gradle.api.tasks.javadoc.Javadoc;
Expand Down Expand Up @@ -100,6 +101,7 @@ public void execute(JavaCompile javaCompileTask) {
.getOptions()
.getCompilerArgumentProviders()
.add(new EnablePreviewArgumentProvider(target));
hackTryToMakePreviewFeaturesWorkInIntellij(target, javaCompileTask.getOptions());

// Set sourceCompatibility to opt out of '-release', allowing opens/exports to be used.
javaCompileTask.doFirst(new Action<Task>() {
Expand Down Expand Up @@ -267,6 +269,21 @@ public final void checkJavaVersions() {
}
}

/**
* <a href="https://github.com/JetBrains/intellij-community/pull/2135">Further details available here.</a>
* This is not guaranteed to work - there's a race that depends on plugin application vs when the javaVersions
* closer is applied. However, based on the root project being configured before subprojects, this will likely
* catch practical occurrences.
* An artifact of this is that compile tasks will get _two_ --enable-preview flags set, although this is
* semantically meaningless.
*/
private static void hackTryToMakePreviewFeaturesWorkInIntellij(
Provider<ChosenJavaVersion> provider, CompileOptions compileOptions) {
if (provider.get().enablePreview()) {
Copy link
Contributor Author

@j-baker j-baker Sep 1, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is eager and not lazy, obviously. @iamdanfox would this be a problem?

Copy link
Contributor

@iamdanfox iamdanfox Sep 1, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The downside of this 'eager' approach is:

If the first line of your build.gradle says apply plugin: 'com.palantir.baseline-java-version' and then your second line says: javaVersions { runtime = '19_PREVIEW' }, then we will already have configured this compileOptions task and not added the --enable-preview compiler arg.

Copy link
Contributor

@CRogers CRogers Sep 1, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah this should be lazy. You can use getCompilerArgumentProviders().add to give an argument provider that can return a list of size 0 or 1 to be lazy.

Copy link
Contributor

@iamdanfox iamdanfox Sep 1, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@CRogers the compilerArgumentProviders approach is what's already on develop, but sadly when you use IntelliJ's native integration, they literally check javaCompileTask.options.compilerArgs.contains("--enable-preview") which does not invoke my beloved command line argument provider.

James has a fix up here: JetBrains/intellij-community#2135

Copy link
Contributor

@CRogers CRogers Sep 1, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah I probably should have read that first 🤦🏻

Copy link
Contributor Author

@j-baker j-baker Sep 1, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok folks. I've updated this PR to actually use both methods (lazy and eager). Dan and I think that this will most likely solve the problem in all practical cases because in any case where code is in subprojects, the javaVersions closure will have been set on the root and so evaluated before the java library declaration, avoiding the problem.

However, just in case laziness is an issue (e.g. you have only a root project) this will maintain the fact that --enable-preview is set, with the only downside being it's most likely set twice. This should be a no-op in all cases, should guarantee no semantic changes from present except avoiding the IntelliJ bug.

compileOptions.getCompilerArgs().add(EnablePreviewArgumentProvider.FLAG);
}
}

private static class EnablePreviewArgumentProvider implements CommandLineArgumentProvider {

public static final String FLAG = "--enable-preview";
Expand Down