Skip to content

Commit 47da253

Browse files
committed
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.
1 parent c73329c commit 47da253

3 files changed

Lines changed: 99 additions & 86 deletions

File tree

‎gradle-open-java-format/src/main/java/com/palantir/javaformat/gradle/PalantirJavaFormatSpotlessPlugin.java‎

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -17,10 +17,9 @@
1717

1818
import com.diffplug.gradle.spotless.SpotlessExtension;
1919
import com.google.common.collect.ImmutableList;
20-
import com.palantir.javaformat.java.FormatterService;
21-
import java.util.function.Supplier;
2220
import org.gradle.api.Plugin;
2321
import org.gradle.api.Project;
22+
import org.gradle.api.plugins.JavaPluginExtension;
2423

2524
public abstract class PalantirJavaFormatSpotlessPlugin implements Plugin<Project> {
2625
// The spotless gradle plugin got renamed to 'com.diffplug.spotless' at version 5.0.0
@@ -32,16 +31,19 @@ public void apply(Project project) {
3231
Project rootProject = project.getRootProject();
3332
rootProject.getPluginManager().apply(PalantirJavaFormatProviderPlugin.class);
3433

35-
Supplier<FormatterService> memoizedService =
36-
rootProject.getExtensions().getByType(JavaFormatExtension.class)::serviceLoad;
37-
3834
project.getPluginManager().withPlugin("java", _javaPlugin -> {
3935
SPOTLESS_PLUGINS.forEach(spotlessPluginId -> project.getPluginManager()
4036
.withPlugin(spotlessPluginId, _spotlessPlugin -> {
4137
// Not before this point: Gradle's decoration of SpotlessInterop loads Spotless classes, which
4238
// are missing from a project that never applies Spotless.
43-
SpotlessInterop spotlessInterop =
44-
rootProject.getObjects().newInstance(SpotlessInterop.class, memoizedService);
39+
SpotlessInterop spotlessInterop = rootProject
40+
.getObjects()
41+
.newInstance(
42+
SpotlessInterop.class,
43+
project.getProjectDir(),
44+
project.getExtensions()
45+
.getByType(JavaPluginExtension.class)
46+
.getSourceSets());
4547
SpotlessExtension spotlessExtension =
4648
project.getExtensions().getByType(SpotlessExtension.class);
4749
spotlessExtension.java(spotlessInterop);

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

Lines changed: 63 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -19,15 +19,21 @@
1919
import com.diffplug.spotless.FormatterStep;
2020
import com.palantir.javaformat.gradle.spotless.NativePalantirJavaFormatStep;
2121
import com.palantir.javaformat.gradle.spotless.PalantirJavaFormatStep;
22-
import com.palantir.javaformat.java.FormatterService;
23-
import java.util.function.Supplier;
22+
import java.io.File;
23+
import java.util.List;
24+
import java.util.concurrent.Callable;
25+
import java.util.regex.Pattern;
2426
import javax.inject.Inject;
2527
import org.gradle.api.Action;
2628
import org.gradle.api.JavaVersion;
2729
import org.gradle.api.artifacts.ConfigurationContainer;
30+
import org.gradle.api.file.FileCollection;
2831
import org.gradle.api.logging.Logger;
2932
import org.gradle.api.logging.Logging;
33+
import org.gradle.api.model.ObjectFactory;
34+
import org.gradle.api.specs.Spec;
3035
import org.gradle.api.tasks.Nested;
36+
import org.gradle.api.tasks.SourceSetContainer;
3137

3238
/**
3339
* Class that exists only to encapsulate accessing spotless classes, so that Gradle can generate a decorated class for
@@ -36,7 +42,19 @@
3642
public abstract class SpotlessInterop implements Action<JavaExtension> {
3743
private static final Logger logger = Logging.getLogger(SpotlessInterop.class);
3844

39-
private final Supplier<FormatterService> formatterService;
45+
// Generated source directories, which are not formatted: a file is in one when its path in the project directory
46+
// matches **/build/generated*/**, **/src/generated*/**, **/generated_*src/**, **/generated_*Src/** or
47+
// **/groovy-dsl-plugins/**.
48+
// Note: We cannot simply exclude **/build/**/* because some repos might contain a build directory
49+
// within the src sourceset that should be formatted.
50+
// build/groovy-dsl-plugins contains Java wrapper classes for
51+
// https://docs.gradle.org/9.3.1/userguide/implementing_gradle_plugins_convention.html
52+
// and don't need to be formatted.
53+
private static final Pattern GENERATED = Pattern.compile(
54+
"(.*/)?(build/generated[^/]*|src/generated[^/]*|generated_[^/]*src|generated_[^/]*Src|groovy-dsl-plugins)/.+");
55+
56+
private final File projectDir;
57+
private final SourceSetContainer sourceSets;
4058

4159
@Nested
4260
protected abstract NativeImageSupport getNativeImageSupport();
@@ -45,28 +63,33 @@ public abstract class SpotlessInterop implements Action<JavaExtension> {
4563
protected abstract ConfigurationContainer getConfigurations();
4664

4765
@Inject
48-
public SpotlessInterop(Supplier<FormatterService> formatterService) {
49-
this.formatterService = formatterService;
66+
protected abstract ObjectFactory getObjects();
67+
68+
@Inject
69+
public SpotlessInterop(File projectDir, SourceSetContainer sourceSets) {
70+
this.projectDir = projectDir;
71+
this.sourceSets = sourceSets;
5072
}
5173

5274
@Override
5375
public void execute(JavaExtension java) {
54-
// Exclude generated source directories
55-
java.targetExclude(
56-
// Note: We cannot simply exclude **/build/**/* because some repos might contain a build directory
57-
// within the src sourceset that should be formatted.
58-
"**/build/generated*/**",
59-
"**/src/generated*/**",
60-
"**/generated_*src/**",
61-
"**/generated_*Src/**",
62-
// build/groovy-dsl-plugins contains Java wrapper classes for
63-
// https://docs.gradle.org/9.3.1/userguide/implementing_gradle_plugins_convention.html
64-
// and don't need to be formatted.
65-
"**/groovy-dsl-plugins/**");
76+
java.targetExclude(generatedSources());
6677
// This is configuration cache safe as happening afterEvaluate
6778
java.addStep(spotlessJavaFormatStep());
6879
}
6980

81+
/**
82+
* The Java files of the source sets that are in a generated directory. Not the patterns themselves: Spotless
83+
* matches a pattern against a file tree of the whole project directory, which every build changes, and the
84+
* configuration cache entry is then never reused (#193). The source sets' files are what Spotless formats by
85+
* default; a callable, so that source sets added after the plugin count too.
86+
*/
87+
private FileCollection generatedSources() {
88+
return getObjects().fileCollection().from((Callable<List<FileCollection>>) () -> sourceSets.stream()
89+
.map(sourceSet -> sourceSet.getAllJava().filter(new InGeneratedDirectory(projectDir)))
90+
.toList());
91+
}
92+
7093
private FormatterStep spotlessJavaFormatStep() {
7194
if (getNativeImageSupport().isNativeImageConfigured()
7295
&& JavaVersion.current().compareTo(JavaVersion.VERSION_21) < 0) {
@@ -76,6 +99,28 @@ private FormatterStep spotlessJavaFormatStep() {
7699
}
77100
logger.info("Using the Java-based formatter {}", JavaVersion.current());
78101
return PalantirJavaFormatStep.create(
79-
getConfigurations().getByName(PalantirJavaFormatProviderPlugin.CONFIGURATION_NAME), formatterService);
102+
getConfigurations().getByName(PalantirJavaFormatProviderPlugin.CONFIGURATION_NAME));
103+
}
104+
105+
/**
106+
* Whether a file of the project is in a generated directory, see {@link #GENERATED}. A class, not a record: the
107+
* configuration cache stores it with the Spotless task's target, and could not create it again as a record.
108+
*/
109+
private static final class InGeneratedDirectory implements Spec<File> {
110+
private final File projectDir;
111+
112+
InGeneratedDirectory(File projectDir) {
113+
this.projectDir = projectDir;
114+
}
115+
116+
@Override
117+
public boolean isSatisfiedBy(File file) {
118+
var project = projectDir.toPath();
119+
var path = file.toPath();
120+
return path.startsWith(project)
121+
&& GENERATED
122+
.matcher(project.relativize(path).toString().replace(File.separatorChar, '/'))
123+
.matches();
124+
}
80125
}
81126
}

‎gradle-open-java-format/src/main/java/com/palantir/javaformat/gradle/spotless/PalantirJavaFormatStep.java‎

Lines changed: 27 additions & 61 deletions
Original file line numberDiff line numberDiff line change
@@ -18,11 +18,9 @@
1818
import com.diffplug.spotless.FileSignature;
1919
import com.diffplug.spotless.FormatterFunc;
2020
import com.diffplug.spotless.FormatterStep;
21+
import com.google.errorprone.annotations.InlineMe;
22+
import com.palantir.javaformat.gradle.FormatterServiceLoader;
2123
import com.palantir.javaformat.java.FormatterService;
22-
import java.io.File;
23-
import java.io.IOException;
24-
import java.io.Serializable;
25-
import java.io.UncheckedIOException;
2624
import java.util.function.Supplier;
2725
import org.gradle.api.artifacts.Configuration;
2826

@@ -35,68 +33,36 @@ private PalantirJavaFormatStep() {}
3533
private static final String NAME = "open-java-format";
3634

3735
/** Creates a step which formats everything - code, import order, and unused imports. */
38-
public static FormatterStep create(Configuration palantirJavaFormat, Supplier<FormatterService> memoizedService) {
36+
public static FormatterStep create(Configuration palantirJavaFormat) {
3937
ensureImplementationNotDirectlyLoadable();
38+
// Spotless keeps a step in two forms, and neither can hold the configuration or a loaded formatter, which do
39+
// not serialize. The configuration cache stores the jars, resolved when the entry is written at the earliest.
40+
// The up-to-date check compares their signature, so that another formatter version makes the task run again;
41+
// Spotless takes it when the task runs, or with the configuration cache when the entry is written. The
42+
// formatter is loaded from the jars on first use.
4043
return FormatterStep.createLazy(
41-
NAME, () -> new State(palantirJavaFormat::getFiles, memoizedService), State::createFormat);
44+
NAME,
45+
() -> FileSignature.promise(palantirJavaFormat.getFiles()),
46+
FileSignature.Promised::get,
47+
PalantirJavaFormatStep::createFormat);
4248
}
4349

44-
static final class State implements Serializable {
45-
private static final long serialVersionUID = 1L;
46-
47-
// Kept for state serialization purposes.
48-
@SuppressWarnings("unused")
49-
private final String stepName = NAME;
50-
51-
// Spotless' `FormatterStepImpl` implements Java's `Serializable` interface in a weird way:
52-
// It serializes this `State` class[1].
53-
//
54-
// Gradle understands Java's `Serializable`, and uses it to invalidate `@Input`s to tasks.
55-
//
56-
// Since FormatterStepImpl is an input to `SpotlessTask`[2], anything serialized as part of this `State`
57-
// is used for up-to-date checking for the `SpotlessTask`
58-
//
59-
// [1]
60-
// https://github.com/diffplug/spotless/blob/52654ef8c4a6191d983b10a2370d53b1ca023f7d/lib/src/main/java/com/diffplug/spotless/LazyForwardingEquality.java#L68
61-
// [2]
62-
// https://github.com/diffplug/spotless/blob/f32701212bf8d327c67d10c35316cb80dcdf577b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessTask.java#L163
63-
@SuppressWarnings({"unused", "FieldCanBeLocal"})
64-
private FileSignature jarsSignature;
65-
66-
private final transient Supplier<Iterable<File>> jarsSupplier;
67-
68-
// Transient as this is not serializable.
69-
private final transient Supplier<FormatterService> memoizedFormatter;
70-
71-
/**
72-
* Build a cacheable state for spotless from the given jars, that uses the given {@link FormatterService}.
73-
*
74-
* @param jarsSupplier Supplies the jars that contain the open-java-format implementation. This is only used for caching and
75-
* up-to-dateness purposes.
76-
*/
77-
@SuppressWarnings("for-rollout:NullAway")
78-
State(Supplier<Iterable<File>> jarsSupplier, Supplier<FormatterService> memoizedFormatter) {
79-
this.jarsSupplier = jarsSupplier;
80-
this.memoizedFormatter = memoizedFormatter;
81-
}
82-
83-
@SuppressWarnings("NullableProblems")
84-
FormatterFunc createFormat() {
85-
return input -> {
86-
try {
87-
// Only resolve the jars and compute the signature at execution time!
88-
Iterable<File> jars = jarsSupplier.get();
89-
90-
// Not a performance issue, as Spotless caches this
91-
// https://github.com/diffplug/spotless/blob/228eb10af382b19e130d8d9479f7a95238cb4358/lib/src/main/java/com/diffplug/spotless/FileSignature.java#L138-L143
92-
this.jarsSignature = FileSignature.signAsSet(jars);
50+
/**
51+
* Creates the same step as {@link #create(Configuration)}.
52+
*
53+
* @deprecated the step loads the formatter from the configuration's jars, and ignores {@code memoizedService}.
54+
*/
55+
@Deprecated
56+
@InlineMe(
57+
replacement = "PalantirJavaFormatStep.create(palantirJavaFormat)",
58+
imports = "com.palantir.javaformat.gradle.spotless.PalantirJavaFormatStep")
59+
public static FormatterStep create(Configuration palantirJavaFormat, Supplier<FormatterService> memoizedService) {
60+
return create(palantirJavaFormat);
61+
}
9362

94-
return memoizedFormatter.get().formatSourceReflowStringsAndFixImports(input);
95-
} catch (IOException e) {
96-
throw new UncheckedIOException(e);
97-
}
98-
};
99-
}
63+
private static FormatterFunc createFormat(FileSignature jars) {
64+
FormatterService formatter = FormatterServiceLoader.load(jars.files());
65+
return formatter::formatSourceReflowStringsAndFixImports;
10066
}
10167

10268
private static void ensureImplementationNotDirectlyLoadable() {

0 commit comments

Comments
 (0)