Skip to content

Commit d2acb99

Browse files
authored
SpotlessApply uses by default the Java formatter (#1243)
SpotlessApply uses by default the Java formatter
1 parent ce7470f commit d2acb99

File tree

8 files changed

+159
-24
lines changed

8 files changed

+159
-24
lines changed

README.md

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -182,6 +182,25 @@ shortcut.
182182

183183
![Install plugin from disk](./docs/images/install_plugin_from_disk.png)
184184

185+
## Java 21 Support
186+
187+
In [1211](https://github.com/palantir/palantir-java-format/pull/1211) we shipped Java 21 support. In order to use the
188+
Java 21 formatting capabilities, ensure that either:
189+
190+
- the Gradle daemon and the Intellij Project SDK are set to Java 21
191+
- or that the gradle property `palantir.native.formatter=true`. This will run the formatter as a native image,
192+
- independent of the Gradle daemon/Intellij project JDK version.
193+
194+
### Native image formatter
195+
196+
[This comment](https://github.com/palantir/palantir-java-format/issues/952#issuecomment-2575750610) explains why we
197+
switched to a native image for the formatter. The startup time for the native image esp. in Intellij is >10x faster than
198+
spinning up a new JVM process that does the formatting.
199+
However, the throughput of running the native image for a large set of files (eg. running `./gradlew spotlessApply`) is
200+
considerably slower (eg. 30s using the Java implementation vs 1m20s using the native image implementation). Therefore,
201+
when running the formatter from `spotlessApply` we will default to using the Java implementation (if the Java version >= 21).
202+
203+
185204
## Future works
186205

187206
- [ ] preserve [NON-NLS markers][] - these are comments that are used when implementing NLS internationalisation, and need to stay on the same line with the strings they come after.

changelog/@unreleased/pr-1243.v2.yml

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
type: fix
2+
fix:
3+
description: SpotlessApply uses Java 21 version
4+
links:
5+
- https://github.com/palantir/palantir-java-format/pull/1243

gradle-palantir-java-format/src/main/groovy/com/palantir/javaformat/gradle/PalantirJavaFormatIdeaPlugin.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,7 @@ public void apply(Project rootProject) {
6868
}
6969

7070
private static Optional<Configuration> maybeGetNativeImplConfiguration(Project rootProject) {
71-
return NativeImageFormatProviderPlugin.shouldUseNativeImage(rootProject)
71+
return NativeImageFormatProviderPlugin.isNativeImageConfigured(rootProject)
7272
? Optional.of(rootProject
7373
.getConfigurations()
7474
.getByName(NativeImageFormatProviderPlugin.NATIVE_CONFIGURATION_NAME))

gradle-palantir-java-format/src/main/groovy/com/palantir/javaformat/gradle/PalantirJavaFormatPlugin.java

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ public void apply(Project project) {
4646
// TODO(dfox): in the future we may want to offer a simple 'format' task so people don't need to use
4747
// spotless to try out our formatter
4848
project.getTasks().register("formatDiff", FormatDiffTask.class, task -> {
49-
if (NativeImageFormatProviderPlugin.shouldUseNativeImage(project)) {
49+
if (NativeImageFormatProviderPlugin.isNativeImageConfigured(project)) {
5050
task.getNativeImage().fileProvider(getNativeImplConfiguration(project));
5151
}
5252
});
@@ -76,13 +76,13 @@ public FormatDiffTask() {
7676
@TaskAction
7777
public final void formatDiff() throws IOException, InterruptedException {
7878
if (getNativeImage().isPresent()) {
79-
log.info("Using the native-image to format");
79+
log.info("Using the native-image formatter");
8080
FormatDiff.formatDiff(
8181
getProject().getProjectDir().toPath(),
8282
new NativeImageFormatterService(
8383
getNativeImage().get().getAsFile().toPath()));
8484
} else {
85-
log.info("Using legacy java formatter");
85+
log.info("Using the Java-based formatter");
8686
JavaFormatExtension extension =
8787
getProject().getRootProject().getExtensions().getByType(JavaFormatExtension.class);
8888
FormatterService formatterService = extension.serviceLoad();

gradle-palantir-java-format/src/main/java/com/palantir/javaformat/gradle/NativeImageFormatProviderPlugin.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ public void apply(Project rootProject) {
3737
rootProject == rootProject.getRootProject(),
3838
"May only apply com.palantir.java-format-provider to the root project");
3939

40-
if (!shouldUseNativeImage(rootProject)) {
40+
if (!isNativeImageConfigured(rootProject)) {
4141
log.info("Skipping native image configuration as it is not supported on this platform");
4242
return;
4343
}
@@ -68,7 +68,7 @@ public void apply(Project rootProject) {
6868
});
6969
}
7070

71-
public static boolean shouldUseNativeImage(Project project) {
71+
public static boolean isNativeImageConfigured(Project project) {
7272
return isNativeImageSupported() && isNativeFlagEnabled(project);
7373
}
7474

gradle-palantir-java-format/src/main/java/com/palantir/javaformat/gradle/SpotlessInterop.java

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
import com.diffplug.spotless.FormatterStep;
2020
import com.palantir.javaformat.gradle.spotless.NativePalantirJavaFormatStep;
2121
import com.palantir.javaformat.gradle.spotless.PalantirJavaFormatStep;
22+
import org.gradle.api.JavaVersion;
2223
import org.gradle.api.Project;
2324
import org.gradle.api.logging.Logger;
2425
import org.gradle.api.logging.Logging;
@@ -38,13 +39,19 @@ static void addSpotlessJavaStep(Project project) {
3839
}
3940

4041
static FormatterStep addSpotlessJavaFormatStep(Project project) {
41-
if (NativeImageFormatProviderPlugin.shouldUseNativeImage(project)) {
42-
logger.info("Using the native-image palantir-java-formatter");
42+
43+
if (NativeImageFormatProviderPlugin.isNativeImageConfigured(project)
44+
// Native images have lower throughput than Java implementations. This logic gets called by the
45+
// Gradle spotlessApply step, which formats a full project.
46+
// If we are already running on java 21, then we can run the spotlessApply logic using the Java
47+
// formatter. Otherwise, we need to run the native-image.
48+
&& JavaVersion.current().compareTo(JavaVersion.VERSION_21) < 0) {
49+
logger.info("Using the native-image formatter");
4350
return NativePalantirJavaFormatStep.create(project.getRootProject()
4451
.getConfigurations()
4552
.getByName(NativeImageFormatProviderPlugin.NATIVE_CONFIGURATION_NAME));
4653
}
47-
logger.info("Using the legacy palantir-java-formatter");
54+
logger.info("Using the Java-based formatter {}", JavaVersion.current());
4855
return PalantirJavaFormatStep.create(
4956
project.getRootProject()
5057
.getConfigurations()

gradle-palantir-java-format/src/test/groovy/com/palantir/javaformat/gradle/PalantirJavaFormatPluginTest.groovy

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -93,8 +93,8 @@ class PalantirJavaFormatPluginTest extends IntegrationTestKitSpec {
9393
'''.stripIndent()
9494

9595
where:
96-
extraGradleProperties | expectedOutput
97-
"" | "Using legacy java formatter"
98-
"palantir.native.formatter=true" | "Using the native-image to format"
96+
extraGradleProperties | expectedOutput
97+
"" | "Using the Java-based formatter"
98+
"palantir.native.formatter=true" | "Using the native-image formatter"
9999
}
100100
}

gradle-palantir-java-format/src/test/groovy/com/palantir/javaformat/gradle/PalantirJavaFormatSpotlessPluginTest.groovy

Lines changed: 116 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -16,30 +16,69 @@
1616
package com.palantir.javaformat.gradle
1717

1818
import nebula.test.IntegrationTestKitSpec
19+
import nebula.test.functional.internal.classpath.ClasspathAddingInitScriptBuilder
1920
import spock.lang.Unroll
2021

22+
import java.nio.charset.StandardCharsets
23+
import java.nio.file.Path
24+
import java.util.concurrent.TimeUnit
25+
import java.util.stream.Collectors
26+
import java.util.stream.Stream
27+
2128
class PalantirJavaFormatSpotlessPluginTest extends IntegrationTestKitSpec {
2229
/** ./gradlew writeImplClasspath generates this file. */
2330
private static final CLASSPATH_FILE = new File("build/impl.classpath").absolutePath
2431
private static final NATIVE_IMAGE_FILE = new File("build/nativeImage.path")
2532
private static final NATIVE_CONFIG = String.format("palantirJavaFormatNative files(\"%s\")", NATIVE_IMAGE_FILE.text)
2633

27-
2834
@Unroll
29-
def "formats with spotless when spotless is applied"(String extraGradleProperties, String expectedOutput) {
35+
def "formats with spotless when spotless is applied"(String extraGradleProperties, String javaVersion, String expectedOutput) {
3036
def extraDependencies = extraGradleProperties.isEmpty() ? "" : NATIVE_CONFIG
37+
settingsFile << """
38+
buildscript {
39+
repositories {
40+
mavenCentral() { metadataSources { mavenPom(); ignoreGradleMetadataRedirection() } }
41+
gradlePluginPortal() { metadataSources { mavenPom(); ignoreGradleMetadataRedirection() } }
42+
}
43+
dependencies {
44+
classpath 'com.palantir.gradle.jdks:gradle-jdks-settings:0.62.0'
45+
}
46+
}
47+
apply plugin: 'com.palantir.jdks.settings'
48+
""".stripIndent(true)
49+
3150
buildFile << """
51+
buildscript {
52+
repositories {
53+
mavenCentral() { metadataSources { mavenPom(); ignoreGradleMetadataRedirection() } }
54+
gradlePluginPortal() { metadataSources { mavenPom(); ignoreGradleMetadataRedirection() } }
55+
}
56+
dependencies {
57+
classpath 'com.palantir.baseline:gradle-baseline-java:6.21.0'
58+
classpath 'com.palantir.gradle.jdks:gradle-jdks:0.62.0'
59+
classpath 'com.palantir.gradle.jdkslatest:gradle-jdks-latest:0.17.0'
60+
}
61+
}
62+
3263
// The 'com.diffplug.spotless:spotless-plugin-gradle' dependency is already added by palantir-java-format
3364
plugins {
3465
id 'java'
35-
id 'com.palantir.java-format'
66+
}
67+
68+
apply plugin: 'com.palantir.java-format'
69+
apply plugin: 'com.palantir.baseline-java-versions'
70+
apply plugin: 'com.palantir.jdks'
71+
apply plugin: 'com.palantir.jdks.latest'
72+
73+
javaVersions {
74+
libraryTarget = ${javaVersion}
3675
}
3776
38-
dependencies {
39-
palantirJavaFormat files(file("${CLASSPATH_FILE}").text.split(':'))
40-
EXTRA_CONFIGURATION
77+
jdks {
78+
daemonTarget = ${javaVersion}
4179
}
42-
""".replace("EXTRA_CONFIGURATION", extraDependencies).stripIndent()
80+
81+
""".stripIndent()
4382

4483
// Add jvm args to allow spotless and formatter gradle plugins to run with Java 16+
4584
file('gradle.properties') << """
@@ -48,27 +87,92 @@ class PalantirJavaFormatSpotlessPluginTest extends IntegrationTestKitSpec {
4887
--add-exports jdk.compiler/com.sun.tools.javac.parser=ALL-UNNAMED \
4988
--add-exports jdk.compiler/com.sun.tools.javac.tree=ALL-UNNAMED \
5089
--add-exports jdk.compiler/com.sun.tools.javac.util=ALL-UNNAMED
90+
palantir.jdk.setup.enabled=true
5191
""".stripIndent()
5292
file('gradle.properties') << extraGradleProperties
93+
runTasks('wrapper')
5394

5495
buildFile << """
5596
apply plugin: 'com.diffplug.spotless'
97+
98+
dependencies {
99+
palantirJavaFormat files(file("${CLASSPATH_FILE}").text.split(':'))
100+
${extraDependencies}
101+
}
56102
""".stripIndent()
57103

58104
file('src/main/java/Main.java').text = invalidJavaFile
59105

106+
60107
when:
61-
def result = runTasks('spotlessApply', '--info')
108+
def result = runGradlewTasks('spotlessApply', '--info')
62109

63110
then:
64-
result.output.contains(expectedOutput)
111+
result.standardOutput.contains(expectedOutput)
65112
file('src/main/java/Main.java').text == validJavaFile
66113

67114
where:
68-
extraGradleProperties | expectedOutput
69-
"" | "Using the legacy palantir-java-formatter"
70-
"palantir.native.formatter=true" | "Using the native-image"
115+
extraGradleProperties | javaVersion | expectedOutput
116+
"" | 21 | "Using the Java-based formatter"
117+
"palantir.native.formatter=true" | 21 | "Using the Java-based formatter"
118+
"palantir.native.formatter=true" | 17 | "Using the native-image formatter"
119+
120+
}
121+
122+
private static Iterable<File> getBuildPluginClasspathInjector() {
123+
return getPluginClasspathInjector(Path.of("../gradle-palantir-java-format/build/pluginUnderTestMetadata/plugin-under-test-metadata.properties"))
124+
}
125+
126+
private static Iterable<File> getPluginClasspathInjector(Path path) {
127+
File propertiesFile = path.toFile()
128+
Properties properties = new Properties()
129+
propertiesFile.withInputStream { inputStream ->
130+
properties.load(inputStream)
131+
}
132+
String classpath = properties.getProperty('implementation-classpath')
133+
return classpath.split(File.pathSeparator).collect { new File(it) }
134+
}
135+
136+
private GradlewExecutionResult runGradlewTasks(String... tasks) {
137+
ProcessBuilder processBuilder = getProcessBuilder(tasks)
138+
Process process = processBuilder.start()
139+
String output = readAllInput(process.getInputStream())
140+
process.waitFor(1, TimeUnit.MINUTES)
141+
GradlewExecutionResult result = new GradlewExecutionResult(process.exitValue(), output)
142+
assert result.success
143+
return result
144+
}
145+
146+
final class GradlewExecutionResult {
147+
148+
private final Boolean success
149+
private final String standardOutput
150+
private final Throwable failure
151+
152+
GradlewExecutionResult(int exitValue, String output) {
153+
this.success = exitValue == 0
154+
this.standardOutput = output
155+
this.failure = exitValue != 0 ? new RuntimeException(String.format("Build failed with exitCode %s", exitValue)) : null
156+
}
157+
}
158+
159+
static String readAllInput(InputStream inputStream) {
160+
try (Stream<String> lines =
161+
new BufferedReader(new InputStreamReader(inputStream, StandardCharsets.UTF_8)).lines()) {
162+
return lines.collect(Collectors.joining("\n"));
163+
}
164+
}
71165

166+
private ProcessBuilder getProcessBuilder(String... tasks) {
167+
File initScript = new File(projectDir, "init.gradle")
168+
ClasspathAddingInitScriptBuilder.build(initScript, getBuildPluginClasspathInjector().toList())
169+
List<String> arguments = ["./gradlew", "--init-script", initScript.toPath().toString()]
170+
Arrays.asList(tasks).forEach(arguments::add)
171+
ProcessBuilder processBuilder = new ProcessBuilder()
172+
.command(arguments)
173+
.directory(projectDir)
174+
.redirectErrorStream(true)
175+
return processBuilder
72176
}
73177

74178
def validJavaFile = '''\

0 commit comments

Comments
 (0)