From c73329c35fffa44bdb4eee771ed9a916eae6cf4d Mon Sep 17 00:00:00 2001 From: Alex Abashev Date: Sat, 7 Nov 2026 11:17:32 +0300 Subject: [PATCH 1/3] Run formatDiff without the project when it executes formatDiff called getProject() when it ran, for the project directory and for the root project's JavaFormatExtension, which loads the formatter. With the configuration cache on, Gradle refuses that, and the task failed on every run (#193). The task now gets the project directory and the formatter classpath as properties when it is configured. Loading the formatter moves from JavaFormatExtension to FormatterServiceLoader, which keeps it loaded for as long as its jars stay the same: a build that reuses its cache entry creates no extension to remember it in. One formatter at a time, so a daemon that sees a new formatter version drops the old one. serviceLoad() keeps working for build scripts that call it. --- .../gradle/PalantirJavaFormatPlugin.java | 34 ++++- .../gradle/FormatterServiceLoader.java | 130 ++++++++++++++++++ .../gradle/JavaFormatExtension.java | 80 +---------- 3 files changed, 159 insertions(+), 85 deletions(-) create mode 100644 gradle-open-java-format/src/main/java/com/palantir/javaformat/gradle/FormatterServiceLoader.java diff --git a/gradle-open-java-format/src/main/groovy/com/palantir/javaformat/gradle/PalantirJavaFormatPlugin.java b/gradle-open-java-format/src/main/groovy/com/palantir/javaformat/gradle/PalantirJavaFormatPlugin.java index e36f2416c..22dffc61c 100644 --- a/gradle-open-java-format/src/main/groovy/com/palantir/javaformat/gradle/PalantirJavaFormatPlugin.java +++ b/gradle-open-java-format/src/main/groovy/com/palantir/javaformat/gradle/PalantirJavaFormatPlugin.java @@ -17,18 +17,22 @@ package com.palantir.javaformat.gradle; import com.palantir.javaformat.bootstrap.NativeImageFormatterService; -import com.palantir.javaformat.java.FormatterService; import java.io.File; import java.io.IOException; import org.gradle.api.DefaultTask; import org.gradle.api.Plugin; import org.gradle.api.Project; +import org.gradle.api.artifacts.Configuration; +import org.gradle.api.file.ConfigurableFileCollection; +import org.gradle.api.file.DirectoryProperty; import org.gradle.api.file.FileCollection; import org.gradle.api.file.RegularFileProperty; import org.gradle.api.logging.Logger; import org.gradle.api.logging.Logging; import org.gradle.api.provider.Provider; +import org.gradle.api.tasks.Classpath; import org.gradle.api.tasks.InputFile; +import org.gradle.api.tasks.Internal; import org.gradle.api.tasks.Nested; import org.gradle.api.tasks.PathSensitive; import org.gradle.api.tasks.PathSensitivity; @@ -53,13 +57,20 @@ public void apply(Project project) { // TODO(dfox): in the future we may want to offer a simple 'format' task so people don't need to use // spotless to try out our formatter project.getTasks().register("formatDiff", FormatDiffTask.class, task -> { + task.getProjectDirectory().set(project.getLayout().getProjectDirectory()); if (getNativeImageSupport().isNativeImageConfigured()) { task.getNativeImage().fileProvider(getNativeImplConfiguration(project)); + } else { + task.getFormatterClasspath().from(getImplConfiguration(project)); } }); }); } + private static Provider getImplConfiguration(Project project) { + return project.getRootProject().getConfigurations().named(PalantirJavaFormatProviderPlugin.CONFIGURATION_NAME); + } + private static Provider getNativeImplConfiguration(Project project) { return (project.getRootProject() .getConfigurations() @@ -78,6 +89,17 @@ public abstract static class FormatDiffTask extends DefaultTask { @PathSensitive(PathSensitivity.NAME_ONLY) abstract RegularFileProperty getNativeImage(); + /** The jars of the Java-based formatter, which runs when the native image is not configured. */ + @Classpath + abstract ConfigurableFileCollection getFormatterClasspath(); + + /** + * The project whose changed lines are formatted. The task reads it here rather than from {@code getProject()}, + * which a task must not call when it runs with the configuration cache. + */ + @Internal + abstract DirectoryProperty getProjectDirectory(); + public FormatDiffTask() { setDescription("Format only chunks of files that appear in git diff"); setGroup("Formatting"); @@ -85,18 +107,18 @@ public FormatDiffTask() { @TaskAction public final void formatDiff() throws IOException, InterruptedException { + var projectDir = getProjectDirectory().get().getAsFile().toPath(); if (getNativeImage().isPresent()) { log.info("Using the native-image formatter"); FormatDiff.formatDiff( - getProject().getProjectDir().toPath(), + projectDir, new NativeImageFormatterService( getNativeImage().get().getAsFile().toPath())); } else { log.info("Using the Java-based formatter"); - JavaFormatExtension extension = - getProject().getRootProject().getExtensions().getByType(JavaFormatExtension.class); - FormatterService formatterService = extension.serviceLoad(); - FormatDiff.formatDiff(getProject().getProjectDir().toPath(), formatterService); + FormatDiff.formatDiff( + projectDir, + FormatterServiceLoader.load(getFormatterClasspath().getFiles())); } } } diff --git a/gradle-open-java-format/src/main/java/com/palantir/javaformat/gradle/FormatterServiceLoader.java b/gradle-open-java-format/src/main/java/com/palantir/javaformat/gradle/FormatterServiceLoader.java new file mode 100644 index 000000000..52229d198 --- /dev/null +++ b/gradle-open-java-format/src/main/java/com/palantir/javaformat/gradle/FormatterServiceLoader.java @@ -0,0 +1,130 @@ +/* + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.palantir.javaformat.gradle; + +import com.google.common.collect.Iterables; +import com.palantir.javaformat.java.FormatterService; +import java.io.File; +import java.io.UncheckedIOException; +import java.net.MalformedURLException; +import java.net.URL; +import java.net.URLClassLoader; +import java.util.Collection; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.ServiceLoader; +import java.util.stream.Collectors; +import org.gradle.api.GradleException; + +/** + * Loads the Java-based formatter from its jars, in a class loader of its own, so that the formatter's dependencies do + * not meet the build's. + * + *

The formatter stays loaded for as long as its jars stay the same, across the tasks and builds of one daemon. The + * tasks have nowhere else to keep it: a build that reuses its configuration cache entry creates no project and no + * {@link JavaFormatExtension}, and the Spotless step comes back from the cache with nothing that does not serialize. + */ +public final class FormatterServiceLoader { + /** + * The javac packages the formatter reads. A plain Gradle JVM exports none of them, and the formatter then fails + * on its first file with an IllegalAccessError that names a class and a module and nothing the user can change. + * The check below fails before the first file instead, with the two settings that make it work. + */ + private static final List JAVAC_PACKAGES = List.of( + "com.sun.tools.javac.api", + "com.sun.tools.javac.file", + "com.sun.tools.javac.parser", + "com.sun.tools.javac.tree", + "com.sun.tools.javac.util"); + + private static final String DOCS = "https://openjavaformat.dev/get-started/gradle/#choose-how-the-formatter-runs"; + + /** + * The formatter loaded last, by the jars it came from. One at a time: a build has one formatter classpath, and a + * daemon that outlives a change of formatter version should not keep the old one. + */ + private static final Map, FormatterService> LOADED = new HashMap<>(); + + private FormatterServiceLoader() {} + + /** The formatter in these jars: the one loaded last if they have not changed since, a new one otherwise. */ + public static synchronized FormatterService load(Collection jars) { + var key = jars.stream().map(Jar::of).toList(); + if (!LOADED.containsKey(key)) { + LOADED.clear(); + } + return LOADED.computeIfAbsent(key, _key -> loadUncached(jars)); + } + + @SuppressWarnings("for-rollout:NullAway") + private static FormatterService loadUncached(Collection jars) { + URL[] jarUris = jars.stream() + .map(file -> { + try { + return file.toURI().toURL(); + } catch (MalformedURLException e) { + throw new UncheckedIOException("Unable to convert URI to URL: " + file, e); + } + }) + .toArray(URL[]::new); + + ClassLoader classLoader = new URLClassLoader(jarUris, FormatterService.class.getClassLoader()); + checkJavacIsExportedTo(classLoader); + return Iterables.getOnlyElement(ServiceLoader.load(FormatterService.class, classLoader)); + } + + /** + * Fails with the fix spelt out when this JVM does not export javac's internals to the formatter's class loader: + * either the native binary, which runs outside the JVM, or the {@code --add-exports} flags on the Gradle JVM. + */ + private static void checkJavacIsExportedTo(ClassLoader formatterClassLoader) { + Module jdkCompiler = ModuleLayer.boot() + .findModule("jdk.compiler") + .orElseThrow(() -> new GradleException("open-java-format needs the module jdk.compiler, which this" + + " Gradle JVM does not have: run Gradle on a JDK, not a JRE. See " + DOCS)); + Module formatter = formatterClassLoader.getUnnamedModule(); + List notExported = JAVAC_PACKAGES.stream() + .filter(javacPackage -> !jdkCompiler.isExported(javacPackage, formatter)) + .toList(); + if (notExported.isEmpty()) { + return; + } + String addExports = JAVAC_PACKAGES.stream() + .map(javacPackage -> "--add-exports jdk.compiler/" + javacPackage + "=ALL-UNNAMED") + .collect(Collectors.joining(" ")); + throw new GradleException(""" + open-java-format cannot run inside this Gradle JVM: module jdk.compiler does not export %s to it. \ + Set one of these in gradle.properties: + + openjavaformat.native.formatter=true + runs the formatter as a native binary, outside the Gradle JVM (Linux with glibc, macOS, \ + Windows on x86-64) + + org.gradle.jvmargs=%s + opens javac's packages to the Gradle JVM; if the file already sets org.gradle.jvmargs, add \ + the flags to that line + + See %s\ + """.formatted(String.join(", ", notExported), addExports, DOCS)); + } + + /** A jar as it was when the formatter was loaded from it: rebuilt in place, it changes size or time. */ + private record Jar(File file, long length, long lastModified) { + static Jar of(File file) { + return new Jar(file, file.length(), file.lastModified()); + } + } +} diff --git a/gradle-open-java-format/src/main/java/com/palantir/javaformat/gradle/JavaFormatExtension.java b/gradle-open-java-format/src/main/java/com/palantir/javaformat/gradle/JavaFormatExtension.java index 5554e5b0e..f8b8a4795 100644 --- a/gradle-open-java-format/src/main/java/com/palantir/javaformat/gradle/JavaFormatExtension.java +++ b/gradle-open-java-format/src/main/java/com/palantir/javaformat/gradle/JavaFormatExtension.java @@ -17,96 +17,18 @@ package com.palantir.javaformat.gradle; import com.google.common.base.Suppliers; -import com.google.common.collect.ImmutableList; -import com.google.common.collect.Iterables; import com.palantir.javaformat.java.FormatterService; -import java.io.UncheckedIOException; -import java.net.MalformedURLException; -import java.net.URL; -import java.net.URLClassLoader; -import java.util.List; -import java.util.ServiceLoader; import java.util.function.Supplier; -import java.util.stream.Collectors; -import org.gradle.api.GradleException; import org.gradle.api.artifacts.Configuration; public class JavaFormatExtension { - /** - * The javac packages the formatter reads. A plain Gradle JVM exports none of them, and the formatter then fails - * on its first file with an IllegalAccessError that names a class and a module and nothing the user can change. - * The check below fails before the first file instead, with the two settings that make it work. - */ - private static final ImmutableList JAVAC_PACKAGES = ImmutableList.of( - "com.sun.tools.javac.api", - "com.sun.tools.javac.file", - "com.sun.tools.javac.parser", - "com.sun.tools.javac.tree", - "com.sun.tools.javac.util"); - - private static final String DOCS = "https://openjavaformat.dev/get-started/gradle/#choose-how-the-formatter-runs"; - - private final Configuration configuration; private final Supplier memoizedService; public JavaFormatExtension(Configuration configuration) { - this.configuration = configuration; - this.memoizedService = Suppliers.memoize(this::serviceLoadInternal); + this.memoizedService = Suppliers.memoize(() -> FormatterServiceLoader.load(configuration.getFiles())); } public FormatterService serviceLoad() { return memoizedService.get(); } - - @SuppressWarnings("for-rollout:NullAway") - private FormatterService serviceLoadInternal() { - URL[] jarUris = configuration.getFiles().stream() - .map(file -> { - try { - return file.toURI().toURL(); - } catch (MalformedURLException e) { - throw new UncheckedIOException("Unable to convert URI to URL: " + file, e); - } - }) - .toArray(URL[]::new); - - ClassLoader classLoader = new URLClassLoader(jarUris, FormatterService.class.getClassLoader()); - checkJavacIsExportedTo(classLoader); - return Iterables.getOnlyElement(ServiceLoader.load(FormatterService.class, classLoader)); - } - - /** - * Fails with the fix spelt out when this JVM does not export javac's internals to the formatter's class loader: - * either the native binary, which runs outside the JVM, or the {@code --add-exports} flags on the Gradle JVM. - */ - private static void checkJavacIsExportedTo(ClassLoader formatterClassLoader) { - Module jdkCompiler = ModuleLayer.boot() - .findModule("jdk.compiler") - .orElseThrow(() -> new GradleException("open-java-format needs the module jdk.compiler, which this" - + " Gradle JVM does not have: run Gradle on a JDK, not a JRE. See " + DOCS)); - Module formatter = formatterClassLoader.getUnnamedModule(); - List notExported = JAVAC_PACKAGES.stream() - .filter(javacPackage -> !jdkCompiler.isExported(javacPackage, formatter)) - .collect(Collectors.toList()); - if (notExported.isEmpty()) { - return; - } - String addExports = JAVAC_PACKAGES.stream() - .map(javacPackage -> "--add-exports jdk.compiler/" + javacPackage + "=ALL-UNNAMED") - .collect(Collectors.joining(" ")); - throw new GradleException(""" - open-java-format cannot run inside this Gradle JVM: module jdk.compiler does not export %s to it. \ - Set one of these in gradle.properties: - - openjavaformat.native.formatter=true - runs the formatter as a native binary, outside the Gradle JVM (Linux with glibc, macOS, \ - Windows on x86-64) - - org.gradle.jvmargs=%s - opens javac's packages to the Gradle JVM; if the file already sets org.gradle.jvmargs, add \ - the flags to that line - - See %s\ - """.formatted(String.join(", ", notExported), addExports, DOCS)); - } } From 47da25323eb7e4ce3b263f689c2ec7b07ee1d479 Mon Sep 17 00:00:00 2001 From: Alex Abashev Date: Sat, 7 Nov 2026 11:17:38 +0300 Subject: [PATCH 2/3] Keep the Spotless step's jars through the configuration cache The step kept its jars and the loaded formatter in transient fields. The configuration cache serializes the step, and the one it read back had neither: every file got a lint error, a NullPointerException because "this.jarsSupplier" is null (#193). The step now uses Spotless' round-trip API, as Spotless' own steps do: the cache keeps the jar files, the up-to-date check compares their signature, and the formatter is loaded from the jars on first use. create(Configuration, Supplier) stays, deprecated. The cache entry was not reused either: "an input to unknown location has changed". That came from targetExclude with patterns, which Spotless matches against a file tree of the whole project directory, and every build changes that tree; Spotless alone does the same with those patterns. The excludes are now the source sets' Java files in a generated directory, by the same patterns, which is what Spotless formats by default. A target set by hand to files outside the source sets no longer has those directories left out. --- .../PalantirJavaFormatSpotlessPlugin.java | 16 ++-- .../javaformat/gradle/SpotlessInterop.java | 81 +++++++++++++---- .../spotless/PalantirJavaFormatStep.java | 88 ++++++------------- 3 files changed, 99 insertions(+), 86 deletions(-) diff --git a/gradle-open-java-format/src/main/java/com/palantir/javaformat/gradle/PalantirJavaFormatSpotlessPlugin.java b/gradle-open-java-format/src/main/java/com/palantir/javaformat/gradle/PalantirJavaFormatSpotlessPlugin.java index 0d08d6f32..3b183eb24 100644 --- a/gradle-open-java-format/src/main/java/com/palantir/javaformat/gradle/PalantirJavaFormatSpotlessPlugin.java +++ b/gradle-open-java-format/src/main/java/com/palantir/javaformat/gradle/PalantirJavaFormatSpotlessPlugin.java @@ -17,10 +17,9 @@ import com.diffplug.gradle.spotless.SpotlessExtension; import com.google.common.collect.ImmutableList; -import com.palantir.javaformat.java.FormatterService; -import java.util.function.Supplier; import org.gradle.api.Plugin; import org.gradle.api.Project; +import org.gradle.api.plugins.JavaPluginExtension; public abstract class PalantirJavaFormatSpotlessPlugin implements Plugin { // The spotless gradle plugin got renamed to 'com.diffplug.spotless' at version 5.0.0 @@ -32,16 +31,19 @@ public void apply(Project project) { Project rootProject = project.getRootProject(); rootProject.getPluginManager().apply(PalantirJavaFormatProviderPlugin.class); - Supplier memoizedService = - rootProject.getExtensions().getByType(JavaFormatExtension.class)::serviceLoad; - project.getPluginManager().withPlugin("java", _javaPlugin -> { SPOTLESS_PLUGINS.forEach(spotlessPluginId -> project.getPluginManager() .withPlugin(spotlessPluginId, _spotlessPlugin -> { // Not before this point: Gradle's decoration of SpotlessInterop loads Spotless classes, which // are missing from a project that never applies Spotless. - SpotlessInterop spotlessInterop = - rootProject.getObjects().newInstance(SpotlessInterop.class, memoizedService); + SpotlessInterop spotlessInterop = rootProject + .getObjects() + .newInstance( + SpotlessInterop.class, + project.getProjectDir(), + project.getExtensions() + .getByType(JavaPluginExtension.class) + .getSourceSets()); SpotlessExtension spotlessExtension = project.getExtensions().getByType(SpotlessExtension.class); spotlessExtension.java(spotlessInterop); diff --git a/gradle-open-java-format/src/main/java/com/palantir/javaformat/gradle/SpotlessInterop.java b/gradle-open-java-format/src/main/java/com/palantir/javaformat/gradle/SpotlessInterop.java index 5957bd66f..7c89a6e7e 100644 --- a/gradle-open-java-format/src/main/java/com/palantir/javaformat/gradle/SpotlessInterop.java +++ b/gradle-open-java-format/src/main/java/com/palantir/javaformat/gradle/SpotlessInterop.java @@ -19,15 +19,21 @@ import com.diffplug.spotless.FormatterStep; import com.palantir.javaformat.gradle.spotless.NativePalantirJavaFormatStep; import com.palantir.javaformat.gradle.spotless.PalantirJavaFormatStep; -import com.palantir.javaformat.java.FormatterService; -import java.util.function.Supplier; +import java.io.File; +import java.util.List; +import java.util.concurrent.Callable; +import java.util.regex.Pattern; import javax.inject.Inject; import org.gradle.api.Action; import org.gradle.api.JavaVersion; import org.gradle.api.artifacts.ConfigurationContainer; +import org.gradle.api.file.FileCollection; import org.gradle.api.logging.Logger; import org.gradle.api.logging.Logging; +import org.gradle.api.model.ObjectFactory; +import org.gradle.api.specs.Spec; import org.gradle.api.tasks.Nested; +import org.gradle.api.tasks.SourceSetContainer; /** * Class that exists only to encapsulate accessing spotless classes, so that Gradle can generate a decorated class for @@ -36,7 +42,19 @@ public abstract class SpotlessInterop implements Action { private static final Logger logger = Logging.getLogger(SpotlessInterop.class); - private final Supplier formatterService; + // Generated source directories, which are not formatted: a file is in one when its path in the project directory + // matches **/build/generated*/**, **/src/generated*/**, **/generated_*src/**, **/generated_*Src/** or + // **/groovy-dsl-plugins/**. + // Note: We cannot simply exclude **/build/**/* because some repos might contain a build directory + // within the src sourceset that should be formatted. + // build/groovy-dsl-plugins contains Java wrapper classes for + // https://docs.gradle.org/9.3.1/userguide/implementing_gradle_plugins_convention.html + // and don't need to be formatted. + private static final Pattern GENERATED = Pattern.compile( + "(.*/)?(build/generated[^/]*|src/generated[^/]*|generated_[^/]*src|generated_[^/]*Src|groovy-dsl-plugins)/.+"); + + private final File projectDir; + private final SourceSetContainer sourceSets; @Nested protected abstract NativeImageSupport getNativeImageSupport(); @@ -45,28 +63,33 @@ public abstract class SpotlessInterop implements Action { protected abstract ConfigurationContainer getConfigurations(); @Inject - public SpotlessInterop(Supplier formatterService) { - this.formatterService = formatterService; + protected abstract ObjectFactory getObjects(); + + @Inject + public SpotlessInterop(File projectDir, SourceSetContainer sourceSets) { + this.projectDir = projectDir; + this.sourceSets = sourceSets; } @Override public void execute(JavaExtension java) { - // Exclude generated source directories - java.targetExclude( - // Note: We cannot simply exclude **/build/**/* because some repos might contain a build directory - // within the src sourceset that should be formatted. - "**/build/generated*/**", - "**/src/generated*/**", - "**/generated_*src/**", - "**/generated_*Src/**", - // build/groovy-dsl-plugins contains Java wrapper classes for - // https://docs.gradle.org/9.3.1/userguide/implementing_gradle_plugins_convention.html - // and don't need to be formatted. - "**/groovy-dsl-plugins/**"); + java.targetExclude(generatedSources()); // This is configuration cache safe as happening afterEvaluate java.addStep(spotlessJavaFormatStep()); } + /** + * The Java files of the source sets that are in a generated directory. Not the patterns themselves: Spotless + * matches a pattern against a file tree of the whole project directory, which every build changes, and the + * configuration cache entry is then never reused (#193). The source sets' files are what Spotless formats by + * default; a callable, so that source sets added after the plugin count too. + */ + private FileCollection generatedSources() { + return getObjects().fileCollection().from((Callable>) () -> sourceSets.stream() + .map(sourceSet -> sourceSet.getAllJava().filter(new InGeneratedDirectory(projectDir))) + .toList()); + } + private FormatterStep spotlessJavaFormatStep() { if (getNativeImageSupport().isNativeImageConfigured() && JavaVersion.current().compareTo(JavaVersion.VERSION_21) < 0) { @@ -76,6 +99,28 @@ private FormatterStep spotlessJavaFormatStep() { } logger.info("Using the Java-based formatter {}", JavaVersion.current()); return PalantirJavaFormatStep.create( - getConfigurations().getByName(PalantirJavaFormatProviderPlugin.CONFIGURATION_NAME), formatterService); + getConfigurations().getByName(PalantirJavaFormatProviderPlugin.CONFIGURATION_NAME)); + } + + /** + * Whether a file of the project is in a generated directory, see {@link #GENERATED}. A class, not a record: the + * configuration cache stores it with the Spotless task's target, and could not create it again as a record. + */ + private static final class InGeneratedDirectory implements Spec { + private final File projectDir; + + InGeneratedDirectory(File projectDir) { + this.projectDir = projectDir; + } + + @Override + public boolean isSatisfiedBy(File file) { + var project = projectDir.toPath(); + var path = file.toPath(); + return path.startsWith(project) + && GENERATED + .matcher(project.relativize(path).toString().replace(File.separatorChar, '/')) + .matches(); + } } } diff --git a/gradle-open-java-format/src/main/java/com/palantir/javaformat/gradle/spotless/PalantirJavaFormatStep.java b/gradle-open-java-format/src/main/java/com/palantir/javaformat/gradle/spotless/PalantirJavaFormatStep.java index 5ef44909e..27f0a95a3 100644 --- a/gradle-open-java-format/src/main/java/com/palantir/javaformat/gradle/spotless/PalantirJavaFormatStep.java +++ b/gradle-open-java-format/src/main/java/com/palantir/javaformat/gradle/spotless/PalantirJavaFormatStep.java @@ -18,11 +18,9 @@ import com.diffplug.spotless.FileSignature; import com.diffplug.spotless.FormatterFunc; import com.diffplug.spotless.FormatterStep; +import com.google.errorprone.annotations.InlineMe; +import com.palantir.javaformat.gradle.FormatterServiceLoader; import com.palantir.javaformat.java.FormatterService; -import java.io.File; -import java.io.IOException; -import java.io.Serializable; -import java.io.UncheckedIOException; import java.util.function.Supplier; import org.gradle.api.artifacts.Configuration; @@ -35,68 +33,36 @@ private PalantirJavaFormatStep() {} private static final String NAME = "open-java-format"; /** Creates a step which formats everything - code, import order, and unused imports. */ - public static FormatterStep create(Configuration palantirJavaFormat, Supplier memoizedService) { + public static FormatterStep create(Configuration palantirJavaFormat) { ensureImplementationNotDirectlyLoadable(); + // Spotless keeps a step in two forms, and neither can hold the configuration or a loaded formatter, which do + // not serialize. The configuration cache stores the jars, resolved when the entry is written at the earliest. + // The up-to-date check compares their signature, so that another formatter version makes the task run again; + // Spotless takes it when the task runs, or with the configuration cache when the entry is written. The + // formatter is loaded from the jars on first use. return FormatterStep.createLazy( - NAME, () -> new State(palantirJavaFormat::getFiles, memoizedService), State::createFormat); + NAME, + () -> FileSignature.promise(palantirJavaFormat.getFiles()), + FileSignature.Promised::get, + PalantirJavaFormatStep::createFormat); } - static final class State implements Serializable { - private static final long serialVersionUID = 1L; - - // Kept for state serialization purposes. - @SuppressWarnings("unused") - private final String stepName = NAME; - - // Spotless' `FormatterStepImpl` implements Java's `Serializable` interface in a weird way: - // It serializes this `State` class[1]. - // - // Gradle understands Java's `Serializable`, and uses it to invalidate `@Input`s to tasks. - // - // Since FormatterStepImpl is an input to `SpotlessTask`[2], anything serialized as part of this `State` - // is used for up-to-date checking for the `SpotlessTask` - // - // [1] - // https://github.com/diffplug/spotless/blob/52654ef8c4a6191d983b10a2370d53b1ca023f7d/lib/src/main/java/com/diffplug/spotless/LazyForwardingEquality.java#L68 - // [2] - // https://github.com/diffplug/spotless/blob/f32701212bf8d327c67d10c35316cb80dcdf577b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessTask.java#L163 - @SuppressWarnings({"unused", "FieldCanBeLocal"}) - private FileSignature jarsSignature; - - private final transient Supplier> jarsSupplier; - - // Transient as this is not serializable. - private final transient Supplier memoizedFormatter; - - /** - * Build a cacheable state for spotless from the given jars, that uses the given {@link FormatterService}. - * - * @param jarsSupplier Supplies the jars that contain the open-java-format implementation. This is only used for caching and - * up-to-dateness purposes. - */ - @SuppressWarnings("for-rollout:NullAway") - State(Supplier> jarsSupplier, Supplier memoizedFormatter) { - this.jarsSupplier = jarsSupplier; - this.memoizedFormatter = memoizedFormatter; - } - - @SuppressWarnings("NullableProblems") - FormatterFunc createFormat() { - return input -> { - try { - // Only resolve the jars and compute the signature at execution time! - Iterable jars = jarsSupplier.get(); - - // Not a performance issue, as Spotless caches this - // https://github.com/diffplug/spotless/blob/228eb10af382b19e130d8d9479f7a95238cb4358/lib/src/main/java/com/diffplug/spotless/FileSignature.java#L138-L143 - this.jarsSignature = FileSignature.signAsSet(jars); + /** + * Creates the same step as {@link #create(Configuration)}. + * + * @deprecated the step loads the formatter from the configuration's jars, and ignores {@code memoizedService}. + */ + @Deprecated + @InlineMe( + replacement = "PalantirJavaFormatStep.create(palantirJavaFormat)", + imports = "com.palantir.javaformat.gradle.spotless.PalantirJavaFormatStep") + public static FormatterStep create(Configuration palantirJavaFormat, Supplier memoizedService) { + return create(palantirJavaFormat); + } - return memoizedFormatter.get().formatSourceReflowStringsAndFixImports(input); - } catch (IOException e) { - throw new UncheckedIOException(e); - } - }; - } + private static FormatterFunc createFormat(FileSignature jars) { + FormatterService formatter = FormatterServiceLoader.load(jars.files()); + return formatter::formatSourceReflowStringsAndFixImports; } private static void ensureImplementationNotDirectlyLoadable() { From 3b720d5552e45e628abbf1ade9d256731b08250a Mon Sep 17 00:00:00 2001 From: Alex Abashev Date: Sat, 7 Nov 2026 11:17:39 +0300 Subject: [PATCH 3/3] Test the plugin with and without the configuration cache ConfigurationCacheTest runs formatDiff, spotlessApply, spotlessCheck and the IntelliJ tasks twice with the cache on: the first build stores the entry, the second reuses it, and both do the work. Until now the only test with the cache on ran classes, which creates the Spotless step but never formats with it. GradleTestProject turns the cache on by default, and the TestKit test classes run both ways through @ParameterizedClass, in the same test task, so CI runs them unchanged. The builds also run without file system watching: a test changes a file faster than the events arrive, and the next build took a deleted output for one still there. Tests on JDK 21: 68 in :gradle-open-java-format on Gradle 9.3.1 and on 9.8.0, 83 with -PnativeImage=true; ./gradlew test passes. --- .../gradle/AppliesWithoutSpotlessTest.java | 9 + .../gradle/ConfigurationCacheTest.java | 193 ++++++++++++++++++ .../FormatDiffWithoutJavacExportsTest.java | 21 +- .../PalantirJavaFormatIdeaPluginTest.java | 22 +- .../gradle/PalantirJavaFormatPluginTest.java | 73 +++---- .../PalantirJavaFormatSpotlessPluginTest.java | 9 + .../gradle/RequiredIdeaPluginTest.java | 12 +- .../gradle/SpotlessExcludesTest.java | 65 +++--- .../gradle/SupportsCurrentSpotlessTest.java | 20 +- .../gradle/testing/GradleTestProject.java | 14 +- 10 files changed, 338 insertions(+), 100 deletions(-) create mode 100644 gradle-open-java-format/src/test/java/com/palantir/javaformat/gradle/ConfigurationCacheTest.java diff --git a/gradle-open-java-format/src/test/java/com/palantir/javaformat/gradle/AppliesWithoutSpotlessTest.java b/gradle-open-java-format/src/test/java/com/palantir/javaformat/gradle/AppliesWithoutSpotlessTest.java index 155fa9e97..74cd21dbe 100644 --- a/gradle-open-java-format/src/test/java/com/palantir/javaformat/gradle/AppliesWithoutSpotlessTest.java +++ b/gradle-open-java-format/src/test/java/com/palantir/javaformat/gradle/AppliesWithoutSpotlessTest.java @@ -23,19 +23,28 @@ import org.gradle.testkit.runner.BuildResult; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.io.TempDir; +import org.junit.jupiter.params.Parameter; +import org.junit.jupiter.params.ParameterizedClass; +import org.junit.jupiter.params.provider.ValueSource; /** * Most projects that apply the formatter never apply Spotless, so there are no Spotless classes on their buildscript * classpath. The Spotless integration has to stay dormant there instead of failing the whole plugin. */ +@ParameterizedClass(name = "configuration cache {0}") +@ValueSource(booleans = {true, false}) final class AppliesWithoutSpotlessTest { + @Parameter + private boolean configurationCache; + @TempDir private Path projectDir; @Test void applies_without_spotless_on_the_classpath() { GradleTestProject project = new GradleTestProject(projectDir) + .configurationCache(configurationCache) .plugins("java", "dev.openjavaformat.java-format") .withoutSpotlessOnClasspath(); diff --git a/gradle-open-java-format/src/test/java/com/palantir/javaformat/gradle/ConfigurationCacheTest.java b/gradle-open-java-format/src/test/java/com/palantir/javaformat/gradle/ConfigurationCacheTest.java new file mode 100644 index 000000000..6bae506f2 --- /dev/null +++ b/gradle-open-java-format/src/test/java/com/palantir/javaformat/gradle/ConfigurationCacheTest.java @@ -0,0 +1,193 @@ +/* + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.palantir.javaformat.gradle; + +import static org.assertj.core.api.Assertions.assertThat; + +import com.palantir.javaformat.gradle.testing.GradleTestProject; +import java.io.File; +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.List; +import java.util.jar.JarEntry; +import java.util.jar.JarOutputStream; +import org.gradle.testkit.runner.BuildResult; +import org.gradle.testkit.runner.BuildTask; +import org.gradle.testkit.runner.TaskOutcome; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; + +/** + * Runs each task twice with the configuration cache, as a build that turns the cache on does: the first run stores the + * entry, and already runs its tasks from it, the second run reuses it. formatDiff used to call Task.project when it + * ran, and the Spotless step came back from the cache without its jars and its formatter (#193). + */ +final class ConfigurationCacheTest { + + /** ./gradlew writeImplClasspath generates this file. */ + private static final Path CLASSPATH_FILE = Path.of("build/impl.classpath"); + + private static final String STORED = "Configuration cache entry stored."; + private static final String REUSED = "Configuration cache entry reused."; + + private static final String MAIN_JAVA = "src/main/java/Main.java"; + private static final String UNFORMATTED = "class Main {\n int x;\n}\n"; + private static final String FORMATTED = "class Main {\n int x;\n}\n"; + + @TempDir + private Path projectDir; + + @Test + void formatDiff_formats_from_a_reused_entry() throws IOException, InterruptedException { + var project = project("java", "dev.openjavaformat.java-format"); + git(project, "init"); + git(project, "config", "user.name", "Foo"); + git(project, "config", "user.email", "foo@bar.com"); + // The repository this runs in may sign commits; a throwaway one has no key to sign with. + git(project, "config", "commit.gpgsign", "false"); + project.writeFile(MAIN_JAVA, "class Main {}\n"); + git(project, "add", "."); + git(project, "commit", "-m", "Commit"); + + for (var entry : List.of(STORED, REUSED)) { + project.writeFile(MAIN_JAVA, UNFORMATTED); + + assertThat(project.succeeds("formatDiff").getOutput()).contains(entry); + assertThat(project.readFile(MAIN_JAVA)).isEqualTo(FORMATTED); + } + } + + @Test + void spotlessApply_formats_from_a_reused_entry() throws IOException { + var project = project("java", "dev.openjavaformat.java-format", "com.diffplug.spotless"); + + for (var entry : List.of(STORED, REUSED)) { + project.writeFile(MAIN_JAVA, UNFORMATTED); + + assertThat(project.succeeds("spotlessApply").getOutput()).contains(entry); + // Spotless writes the platform's line endings, CRLF on Windows. + assertThat(project.readFile(MAIN_JAVA)).isEqualToNormalizingNewlines(FORMATTED); + } + } + + @Test + void spotlessCheck_reports_the_violation_from_a_reused_entry() throws IOException { + var project = project("java", "dev.openjavaformat.java-format", "com.diffplug.spotless") + .writeFile(MAIN_JAVA, UNFORMATTED); + + for (var entry : List.of(STORED, REUSED)) { + // A formatter that fails reports a lint error instead, which is how the lost formatter showed. + assertThat(project.fails("spotlessCheck").getOutput()) + .contains(entry) + .contains("to fix all violations") + .doesNotContain("lint error"); + } + } + + @Test + void spotlessJava_is_up_to_date_until_the_formatter_jars_change() throws IOException { + var jars = Files.readString(CLASSPATH_FILE).trim(); + var project = new GradleTestProject(projectDir) + .configurationCache(true) + .plugins("java", "dev.openjavaformat.java-format", "com.diffplug.spotless") + .withJavacInternalExports() + .writeFile("formatter.classpath", jars) + .buildGradle(""" + dependencies { + palantirJavaFormat files(file("formatter.classpath").text.split(File.pathSeparator)) + } + """) + .writeFile(MAIN_JAVA, FORMATTED); + + assertThat(spotlessJava(project.succeeds("spotlessCheck"))).isEqualTo(TaskOutcome.SUCCESS); + var unchanged = project.succeeds("spotlessCheck"); + assertThat(unchanged.getOutput()).contains(REUSED); + assertThat(spotlessJava(unchanged)).isEqualTo(TaskOutcome.UP_TO_DATE); + + // Spotless signs the jars by name, size and content, as another formatter version changes them. + var extraJar = projectDir.resolve("extra.jar"); + try (var jar = new JarOutputStream(Files.newOutputStream(extraJar))) { + jar.putNextEntry(new JarEntry("extra.txt")); + } + project.writeFile("formatter.classpath", jars + File.pathSeparator + extraJar); + + assertThat(spotlessJava(project.succeeds("spotlessCheck"))).isEqualTo(TaskOutcome.SUCCESS); + } + + @Test + void idea_tasks_run_from_a_reused_entry() { + var project = project("java", "dev.openjavaformat.java-format", "idea"); + + for (var entry : List.of(STORED, REUSED)) { + project.file(".idea/open-java-format.xml").toFile().delete(); + + // Not `idea`: Gradle's own idea tasks are not compatible with the configuration cache, and running + // them turns it off. The plugin appends its tasks to every build, so `help` runs them too. + var result = project.succeeds("help"); + + assertThat(result.getOutput()).contains(entry); + assertThat(result.task(":updateOpenJavaFormatXml")) + .isNotNull() + .extracting(BuildTask::getOutcome) + .isEqualTo(TaskOutcome.SUCCESS); + assertThat(project.file(".idea/open-java-format.xml")).exists(); + } + } + + @Test + void required_idea_plugin_is_listed_from_a_reused_entry() { + var project = project("java", "dev.openjavaformat.java-format"); + + for (var entry : List.of(STORED, REUSED)) { + project.file(".idea/externalDependencies.xml").toFile().delete(); + + assertThat(project.succeeds("help", "-Didea.active=true").getOutput()) + .contains(entry); + assertThat(project.file(".idea/externalDependencies.xml")).exists(); + } + } + + private GradleTestProject project(String... plugins) { + // Forward slashes: the path goes into a Groovy string, where a Windows backslash would start an escape. + return new GradleTestProject(projectDir) + .configurationCache(true) + .plugins(plugins) + .withJavacInternalExports() + .buildGradle(""" + dependencies { + palantirJavaFormat files(file("%s").text.split(File.pathSeparator)) + } + """, CLASSPATH_FILE.toAbsolutePath().toString().replace('\\', '/')); + } + + private static TaskOutcome spotlessJava(BuildResult result) { + var task = result.task(":spotlessJava"); + assertThat(task).isNotNull(); + return task.getOutcome(); + } + + private static void git(GradleTestProject project, String... args) throws IOException, InterruptedException { + var command = new String[args.length + 1]; + command[0] = "git"; + System.arraycopy(args, 0, command, 1, args.length); + var process = + new ProcessBuilder(command).directory(project.path().toFile()).start(); + var exitCode = process.waitFor(); + if (exitCode != 0) { + throw new RuntimeException("git " + String.join(" ", args) + " failed with exit code " + exitCode); + } + } +} diff --git a/gradle-open-java-format/src/test/java/com/palantir/javaformat/gradle/FormatDiffWithoutJavacExportsTest.java b/gradle-open-java-format/src/test/java/com/palantir/javaformat/gradle/FormatDiffWithoutJavacExportsTest.java index e89dcd9cb..06eabede1 100644 --- a/gradle-open-java-format/src/test/java/com/palantir/javaformat/gradle/FormatDiffWithoutJavacExportsTest.java +++ b/gradle-open-java-format/src/test/java/com/palantir/javaformat/gradle/FormatDiffWithoutJavacExportsTest.java @@ -24,12 +24,17 @@ import org.gradle.testkit.runner.BuildResult; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.io.TempDir; +import org.junit.jupiter.params.Parameter; +import org.junit.jupiter.params.ParameterizedClass; +import org.junit.jupiter.params.provider.ValueSource; /** * A build without the JVM flags used to fail on the first file with a raw IllegalAccessError from the formatter's * internals (#19). It fails before the first file now, naming the two settings that make the formatter run. This * class runs in the default test task; PalantirJavaFormatPluginTest does not, because it needs the native binary. */ +@ParameterizedClass(name = "configuration cache {0}") +@ValueSource(booleans = {true, false}) final class FormatDiffWithoutJavacExportsTest { /** ./gradlew writeImplClasspath generates this file. Forward slashes: the path goes into a Groovy string. */ @@ -38,6 +43,9 @@ final class FormatDiffWithoutJavacExportsTest { private static final String MAIN_JAVA = "src/main/java/Main.java"; + @Parameter + private boolean configurationCache; + @TempDir private Path projectDir; @@ -45,14 +53,13 @@ final class FormatDiffWithoutJavacExportsTest { void formatDiff_names_both_settings_when_the_gradle_jvm_does_not_export_javac() throws IOException, InterruptedException { GradleTestProject project = new GradleTestProject(projectDir) + .configurationCache(configurationCache) .plugins("java", "dev.openjavaformat.java-format") - .buildGradle( - """ - dependencies { - palantirJavaFormat files(file("%s").text.split(File.pathSeparator)) - } - """, - CLASSPATH_FILE); + .buildGradle(""" + dependencies { + palantirJavaFormat files(file("%s").text.split(File.pathSeparator)) + } + """, CLASSPATH_FILE); git(project, "init"); git(project, "config", "user.name", "Foo"); diff --git a/gradle-open-java-format/src/test/java/com/palantir/javaformat/gradle/PalantirJavaFormatIdeaPluginTest.java b/gradle-open-java-format/src/test/java/com/palantir/javaformat/gradle/PalantirJavaFormatIdeaPluginTest.java index fc83eceac..941f09587 100644 --- a/gradle-open-java-format/src/test/java/com/palantir/javaformat/gradle/PalantirJavaFormatIdeaPluginTest.java +++ b/gradle-open-java-format/src/test/java/com/palantir/javaformat/gradle/PalantirJavaFormatIdeaPluginTest.java @@ -32,9 +32,13 @@ import org.gradle.testkit.runner.BuildTask; import org.gradle.testkit.runner.TaskOutcome; import org.junit.jupiter.api.io.TempDir; +import org.junit.jupiter.params.Parameter; +import org.junit.jupiter.params.ParameterizedClass; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.ValueSource; +@ParameterizedClass(name = "configuration cache {0}") +@ValueSource(booleans = {true, false}) final class PalantirJavaFormatIdeaPluginTest { // Forward slashes: the path goes into a Groovy string, where a Windows backslash would start an escape. @@ -46,6 +50,9 @@ final class PalantirJavaFormatIdeaPluginTest { private static final ObjectMapper XML_MAPPER = new XmlMapper(); + @Parameter + private boolean configurationCache; + @TempDir private Path projectDir; @@ -53,16 +60,15 @@ final class PalantirJavaFormatIdeaPluginTest { @ValueSource(strings = {"", "openjavaformat.native.formatter=true"}) void idea_configures_xml_files(String extraGradleProperties) throws IOException { GradleTestProject project = new GradleTestProject(projectDir) + .configurationCache(configurationCache) .plugins("dev.openjavaformat.java-format-idea", "idea") .gradleProperties(extraGradleProperties) - .buildGradle( - """ - dependencies { - palantirJavaFormat project.files() // no need to store the real thing in here - %s - } - """, - extraGradleProperties.isBlank() ? "" : NATIVE_CONFIG); + .buildGradle(""" + dependencies { + palantirJavaFormat project.files() // no need to store the real thing in here + %s + } + """, extraGradleProperties.isBlank() ? "" : NATIVE_CONFIG); BuildResult result = project.succeeds("idea"); diff --git a/gradle-open-java-format/src/test/java/com/palantir/javaformat/gradle/PalantirJavaFormatPluginTest.java b/gradle-open-java-format/src/test/java/com/palantir/javaformat/gradle/PalantirJavaFormatPluginTest.java index ff07638cf..a8e4f8eda 100644 --- a/gradle-open-java-format/src/test/java/com/palantir/javaformat/gradle/PalantirJavaFormatPluginTest.java +++ b/gradle-open-java-format/src/test/java/com/palantir/javaformat/gradle/PalantirJavaFormatPluginTest.java @@ -23,9 +23,14 @@ import java.nio.file.Path; import org.gradle.testkit.runner.BuildResult; import org.junit.jupiter.api.io.TempDir; +import org.junit.jupiter.params.Parameter; +import org.junit.jupiter.params.ParameterizedClass; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.CsvSource; +import org.junit.jupiter.params.provider.ValueSource; +@ParameterizedClass(name = "configuration cache {0}") +@ValueSource(booleans = {true, false}) final class PalantirJavaFormatPluginTest { /** ./gradlew writeImplClasspath generates this file. */ @@ -41,6 +46,9 @@ final class PalantirJavaFormatPluginTest { private static final String MAIN_JAVA = "src/main/java/Main.java"; + @Parameter + private boolean configurationCache; + @TempDir private Path projectDir; @@ -56,17 +64,16 @@ void formatDiff_updates_only_lines_changed_in_git_diff(String extraGradlePropert boolean nativeFormatter = extraGradleProperties != null && !extraGradleProperties.isBlank(); GradleTestProject project = new GradleTestProject(projectDir) + .configurationCache(configurationCache) .plugins("java", "dev.openjavaformat.java-format", "idea") .withJavacInternalExports() .gradleProperties(nativeFormatter ? extraGradleProperties : "") - .buildGradle( - """ - dependencies { - palantirJavaFormat files(file("%s").text.split(File.pathSeparator)) - %s - } - """, - CLASSPATH_FILE, nativeFormatter ? NATIVE_CONFIG : ""); + .buildGradle(""" + dependencies { + palantirJavaFormat files(file("%s").text.split(File.pathSeparator)) + %s + } + """, CLASSPATH_FILE, nativeFormatter ? NATIVE_CONFIG : ""); executeGitCommand(project, "git", "init"); executeGitCommand(project, "git", "config", "user.name", "Foo"); @@ -74,46 +81,40 @@ palantirJavaFormat files(file("%s").text.split(File.pathSeparator)) // The repository this runs in may sign commits; a throwaway one has no key to sign with. executeGitCommand(project, "git", "config", "commit.gpgsign", "false"); - project.writeFile( - MAIN_JAVA, - """ - class Main { - public static void crazyExistingFormatting ( String... args) { + project.writeFile(MAIN_JAVA, """ + class Main { + public static void crazyExistingFormatting ( String... args) { - } } - """); + } + """); executeGitCommand(project, "git", "add", "."); executeGitCommand(project, "git", "commit", "-m", "Commit"); - project.writeFile( - MAIN_JAVA, - """ - class Main { - public static void crazyExistingFormatting ( String... args) { - System.out.println("Reformat me please"); - // some comments - System.out.println("Reformat me again please"); - } + project.writeFile(MAIN_JAVA, """ + class Main { + public static void crazyExistingFormatting ( String... args) { + System.out.println("Reformat me please"); + // some comments + System.out.println("Reformat me again please"); } - """); + } + """); BuildResult result = project.succeeds("formatDiff", "--info"); assertThat(result.getOutput()).contains(expectedOutput); - assertThat(project.readFile(MAIN_JAVA)) - .isEqualTo( - """ - class Main { - public static void crazyExistingFormatting ( String... args) { - System.out.println("Reformat me please"); - // some comments - System.out.println("Reformat me again please"); - } - } - """); + assertThat(project.readFile(MAIN_JAVA)).isEqualTo(""" + class Main { + public static void crazyExistingFormatting ( String... args) { + System.out.println("Reformat me please"); + // some comments + System.out.println("Reformat me again please"); + } + } + """); } private void executeGitCommand(GradleTestProject project, String... command) diff --git a/gradle-open-java-format/src/test/java/com/palantir/javaformat/gradle/PalantirJavaFormatSpotlessPluginTest.java b/gradle-open-java-format/src/test/java/com/palantir/javaformat/gradle/PalantirJavaFormatSpotlessPluginTest.java index bea1d4ad1..ee43932c1 100644 --- a/gradle-open-java-format/src/test/java/com/palantir/javaformat/gradle/PalantirJavaFormatSpotlessPluginTest.java +++ b/gradle-open-java-format/src/test/java/com/palantir/javaformat/gradle/PalantirJavaFormatSpotlessPluginTest.java @@ -22,9 +22,14 @@ import java.nio.file.Path; import org.gradle.testkit.runner.BuildResult; import org.junit.jupiter.api.io.TempDir; +import org.junit.jupiter.params.Parameter; +import org.junit.jupiter.params.ParameterizedClass; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.CsvSource; +import org.junit.jupiter.params.provider.ValueSource; +@ParameterizedClass(name = "configuration cache {0}") +@ValueSource(booleans = {true, false}) final class PalantirJavaFormatSpotlessPluginTest { /** ./gradlew writeImplClasspath generates this file. */ @@ -40,6 +45,9 @@ final class PalantirJavaFormatSpotlessPluginTest { private static final String MAIN_JAVA = "src/main/java/Main.java"; + @Parameter + private boolean configurationCache; + @TempDir private Path projectDir; @@ -61,6 +69,7 @@ void formats_with_spotless_when_spotless_is_applied( String extraDependencies = extraGradleProperties == null ? "" : NATIVE_CONFIG; GradleTestProject project = new GradleTestProject(projectDir) + .configurationCache(configurationCache) // The spotless plugin dependency is already brought in by open-java-format .plugins("java", "dev.openjavaformat.java-format", "com.diffplug.spotless") .withJavacInternalExports() diff --git a/gradle-open-java-format/src/test/java/com/palantir/javaformat/gradle/RequiredIdeaPluginTest.java b/gradle-open-java-format/src/test/java/com/palantir/javaformat/gradle/RequiredIdeaPluginTest.java index 9b36cf412..cbb112daa 100644 --- a/gradle-open-java-format/src/test/java/com/palantir/javaformat/gradle/RequiredIdeaPluginTest.java +++ b/gradle-open-java-format/src/test/java/com/palantir/javaformat/gradle/RequiredIdeaPluginTest.java @@ -25,16 +25,26 @@ import org.gradle.testkit.runner.TaskOutcome; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.io.TempDir; +import org.junit.jupiter.params.Parameter; +import org.junit.jupiter.params.ParameterizedClass; +import org.junit.jupiter.params.provider.ValueSource; // Apart from PalantirJavaFormatIdeaPluginTest, which needs the native image and so runs in CI's native jobs only. +@ParameterizedClass(name = "configuration cache {0}") +@ValueSource(booleans = {true, false}) final class RequiredIdeaPluginTest { + @Parameter + private boolean configurationCache; + @TempDir private Path projectDir; @Test void lists_the_idea_plugin_as_required_when_intellij_syncs() { - GradleTestProject project = new GradleTestProject(projectDir).plugins("dev.openjavaformat.java-format-idea"); + GradleTestProject project = new GradleTestProject(projectDir) + .configurationCache(configurationCache) + .plugins("dev.openjavaformat.java-format-idea"); project.succeeds("help"); diff --git a/gradle-open-java-format/src/test/java/com/palantir/javaformat/gradle/SpotlessExcludesTest.java b/gradle-open-java-format/src/test/java/com/palantir/javaformat/gradle/SpotlessExcludesTest.java index 18369abd1..7487f5a26 100644 --- a/gradle-open-java-format/src/test/java/com/palantir/javaformat/gradle/SpotlessExcludesTest.java +++ b/gradle-open-java-format/src/test/java/com/palantir/javaformat/gradle/SpotlessExcludesTest.java @@ -24,21 +24,27 @@ import org.gradle.testkit.runner.TaskOutcome; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.io.TempDir; +import org.junit.jupiter.params.Parameter; +import org.junit.jupiter.params.ParameterizedClass; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.ValueSource; +@ParameterizedClass(name = "configuration cache {0}") +@ValueSource(booleans = {true, false}) final class SpotlessExcludesTest { // Forward slashes: the path goes into a Groovy string, where a Windows backslash would start an escape. private static final String CLASSPATH_FILE = new File("build/impl.classpath").getAbsolutePath().replace('\\', '/'); - private static final String SOURCE_FILE = - """ - package test; - import java.lang.Void; - public class Test { Void test() { return null; } } - """; + private static final String SOURCE_FILE = """ + package test; + import java.lang.Void; + public class Test { Void test() { return null; } } + """; + + @Parameter + private boolean configurationCache; @TempDir private Path projectDir; @@ -48,15 +54,14 @@ public class Test { Void test() { return null; } } @BeforeEach void setup() { project = new GradleTestProject(projectDir) + .configurationCache(configurationCache) .plugins("java", "dev.openjavaformat.java-format", "com.diffplug.spotless") .withJavacInternalExports() - .buildGradle( - """ - dependencies { - palantirJavaFormat files(file("%s").text.split(File.pathSeparator)) - } - """, - CLASSPATH_FILE); + .buildGradle(""" + dependencies { + palantirJavaFormat files(file("%s").text.split(File.pathSeparator)) + } + """, CLASSPATH_FILE); } @ParameterizedTest @@ -70,16 +75,13 @@ palantirJavaFormat files(file("%s").text.split(File.pathSeparator)) "build/groovy-dsl-plugins/output" }) void format_ignores_excluded_directories(String srcDir) { - project.buildGradle( - """ - sourceSets { - main { - java { srcDir '%s' } - } - } - """, - srcDir) - .writeFile(srcDir + "/test/Test.java", SOURCE_FILE); + project.buildGradle(""" + sourceSets { + main { + java { srcDir '%s' } + } + } + """, srcDir).writeFile(srcDir + "/test/Test.java", SOURCE_FILE); BuildResult result = project.succeeds("spotlessJavaCheck"); @@ -94,16 +96,13 @@ void format_ignores_excluded_directories(String srcDir) { "src/main/java/test/generatedNamespace", }) void format_checks_non_generated_files(String srcDir) { - project.buildGradle( - """ - sourceSets { - main { - java { srcDir '%s' } - } - } - """, - srcDir) - .writeFile(srcDir + "/Test.java", SOURCE_FILE); + project.buildGradle(""" + sourceSets { + main { + java { srcDir '%s' } + } + } + """, srcDir).writeFile(srcDir + "/Test.java", SOURCE_FILE); BuildResult result = project.fails("spotlessJavaCheck"); diff --git a/gradle-open-java-format/src/test/java/com/palantir/javaformat/gradle/SupportsCurrentSpotlessTest.java b/gradle-open-java-format/src/test/java/com/palantir/javaformat/gradle/SupportsCurrentSpotlessTest.java index f85d4369f..e70b03166 100644 --- a/gradle-open-java-format/src/test/java/com/palantir/javaformat/gradle/SupportsCurrentSpotlessTest.java +++ b/gradle-open-java-format/src/test/java/com/palantir/javaformat/gradle/SupportsCurrentSpotlessTest.java @@ -43,20 +43,18 @@ final class SupportsCurrentSpotlessTest { @Test void palantirjavaformatplugin_works_with_current_spotless() { new GradleTestProject(projectDir) - .withConfigurationCache() + .configurationCache(true) .plugins("java", "dev.openjavaformat.java-format", "com.diffplug.spotless") .withJavacInternalExports() - .buildGradle( - """ - dependencies { - palantirJavaFormat files(file("%s").text.split(File.pathSeparator)) - } + .buildGradle(""" + dependencies { + palantirJavaFormat files(file("%s").text.split(File.pathSeparator)) + } - // Forces realization of the spotlessJava task, creating the spotless steps. Any - // configuration resolved eagerly in them fails the configuration cache here. - project.getTasks().getByName("spotlessJava") - """, - CLASSPATH_FILE) + // Forces realization of the spotlessJava task, creating the spotless steps. Any + // configuration resolved eagerly in them fails the configuration cache here. + project.getTasks().getByName("spotlessJava") + """, CLASSPATH_FILE) .succeeds("classes", "--info"); } } diff --git a/gradle-open-java-format/src/test/java/com/palantir/javaformat/gradle/testing/GradleTestProject.java b/gradle-open-java-format/src/test/java/com/palantir/javaformat/gradle/testing/GradleTestProject.java index 59e4a8093..f7c8fe50a 100644 --- a/gradle-open-java-format/src/test/java/com/palantir/javaformat/gradle/testing/GradleTestProject.java +++ b/gradle-open-java-format/src/test/java/com/palantir/javaformat/gradle/testing/GradleTestProject.java @@ -59,7 +59,7 @@ public final class GradleTestProject { private final Set plugins = new LinkedHashSet<>(); private final StringBuilder buildGradle = new StringBuilder(); private final StringBuilder gradleProperties = new StringBuilder(); - private boolean configurationCache; + private boolean configurationCache = true; private boolean spotlessOnClasspath = true; public GradleTestProject(Path projectDir) { @@ -100,9 +100,12 @@ public GradleTestProject withJavacInternalExports() { + "--add-exports jdk.compiler/com.sun.tools.javac.util=ALL-UNNAMED"); } - /** Builds run with the configuration cache off unless a test opts in, which is what the plugin's own tests need. */ - public GradleTestProject withConfigurationCache() { - this.configurationCache = true; + /** + * Whether the builds run with the configuration cache, on unless a test turns it off. The TestKit test classes run + * both ways: a plugin has to work with the cache and without it, and the two run its tasks differently. + */ + public GradleTestProject configurationCache(boolean enabled) { + this.configurationCache = enabled; return this; } @@ -152,6 +155,9 @@ private GradleRunner runner(String... arguments) { List allArguments = new ArrayList<>(Arrays.asList(arguments)); allArguments.add("--stacktrace"); allArguments.add(configurationCache ? "--configuration-cache" : "--no-configuration-cache"); + // Every build looks at the files afresh. A test changes files between two builds faster than file system + // watching reports it, and the second build then took a deleted output for one still there. + allArguments.add("--no-watch-fs"); GradleRunner runner = GradleRunner.create() .withProjectDir(projectDir.toFile()) .withGradleVersion(GRADLE_VERSION)