Skip to content

Commit ea71030

Browse files
authored
fix gradle custom step with closure and configuration cache (#2376 fixes #2351)
2 parents a52fa2f + 7a985fb commit ea71030

File tree

8 files changed

+139
-14
lines changed

8 files changed

+139
-14
lines changed

lib-extra/src/main/java/com/diffplug/spotless/extra/GitAttributesLineEndings.java

+3-1
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2016-2023 DiffPlug
2+
* Copyright 2016-2025 DiffPlug
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -193,6 +193,8 @@ static class RuntimeInit {
193193
/////////////////////////////////
194194
// USER AND SYSTEM-WIDE VALUES //
195195
/////////////////////////////////
196+
FS.DETECTED.setGitSystemConfig(new File("no-global-git-config-for-spotless")); // this fixes a problem
197+
// that was only occurring on Java 11. If we remove support for Java 11, we could probably remove it.
196198
systemConfig = SystemReader.getInstance().openSystemConfig(null, FS.DETECTED);
197199
Errors.log().run(systemConfig::load);
198200
userConfig = SystemReader.getInstance().openUserConfig(systemConfig, FS.DETECTED);

lib/src/main/java/com/diffplug/spotless/ConfigurationCacheHackList.java

+21-3
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,8 @@
2222
import java.util.List;
2323
import java.util.Objects;
2424

25+
import com.diffplug.spotless.yaml.SerializeToByteArrayHack;
26+
2527
import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
2628

2729
/**
@@ -51,27 +53,43 @@
5153
* to make Spotless work with all of Gradle's cache systems at once.
5254
*/
5355
public class ConfigurationCacheHackList implements java.io.Serializable {
54-
private static final long serialVersionUID = 1L;
56+
private static final long serialVersionUID = 6914178791997323870L;
57+
5558
private boolean optimizeForEquality;
5659
private ArrayList<Object> backingList = new ArrayList<>();
5760

61+
private boolean shouldWeSerializeToByteArrayFirst() {
62+
return backingList.stream().anyMatch(step -> step instanceof SerializeToByteArrayHack);
63+
}
64+
5865
private void writeObject(java.io.ObjectOutputStream out) throws IOException {
66+
boolean serializeToByteArrayFirst = shouldWeSerializeToByteArrayFirst();
67+
out.writeBoolean(serializeToByteArrayFirst);
5968
out.writeBoolean(optimizeForEquality);
6069
out.writeInt(backingList.size());
6170
for (Object obj : backingList) {
6271
// if write out the list on its own, we'll get java's non-deterministic object-graph serialization
6372
// by writing each object to raw bytes independently, we avoid this
64-
out.writeObject(LazyForwardingEquality.toBytes((Serializable) obj));
73+
if (serializeToByteArrayFirst) {
74+
out.writeObject(LazyForwardingEquality.toBytes((Serializable) obj));
75+
} else {
76+
out.writeObject(obj);
77+
}
6578
}
6679
}
6780

6881
@SuppressFBWarnings("MC_OVERRIDABLE_METHOD_CALL_IN_READ_OBJECT")
6982
private void readObject(java.io.ObjectInputStream in) throws IOException, ClassNotFoundException {
83+
boolean serializeToByteArrayFirst = in.readBoolean();
7084
optimizeForEquality = in.readBoolean();
7185
backingList = new ArrayList<>();
7286
int size = in.readInt();
7387
for (int i = 0; i < size; i++) {
74-
backingList.add(LazyForwardingEquality.fromBytes((byte[]) in.readObject()));
88+
if (serializeToByteArrayFirst) {
89+
backingList.add(LazyForwardingEquality.fromBytes((byte[]) in.readObject()));
90+
} else {
91+
backingList.add(in.readObject());
92+
}
7593
}
7694
}
7795

Original file line numberDiff line numberDiff line change
@@ -0,0 +1,57 @@
1+
/*
2+
* Copyright 2025 DiffPlug
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
package com.diffplug.spotless.yaml;
17+
18+
import java.io.File;
19+
20+
import javax.annotation.Nullable;
21+
22+
import com.diffplug.spotless.FormatterStep;
23+
24+
/**
25+
* This step is a flag which marks that `ConfigurationCacheHackList` should
26+
* serialize each item individually into `byte[]` array, rather than using normal
27+
* serialization.
28+
*
29+
* The reason to use this is if you are using `toggleOffOn` *and* two kinds of
30+
* google-java-format (e.g. one for format and the other for imports), then
31+
* problems with Java's handling of object graphs will cause your up-to-date checks
32+
* to always fail. `CombinedJavaFormatStepTest` recreates this situation. By adding
33+
* this step, it will trigger this workaround which fixes the up-to-dateness bug.
34+
*
35+
* But, turning it on will break all `custom` steps that use Groovy closures. So
36+
* by default you get regular serialization. If you're using `toggleOffOn` and having
37+
* problems with up-to-dateness, then adding this step can be a workaround.
38+
*/
39+
public class SerializeToByteArrayHack implements FormatterStep {
40+
private static final long serialVersionUID = 8071047581828362545L;
41+
42+
@Override
43+
public String getName() {
44+
return "hack to force serializing objects to byte array";
45+
}
46+
47+
@Nullable
48+
@Override
49+
public String format(String rawUnix, File file) throws Exception {
50+
return null;
51+
}
52+
53+
@Override
54+
public void close() throws Exception {
55+
56+
}
57+
}

plugin-gradle/CHANGES.md

+4
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,10 @@ We adhere to the [keepachangelog](https://keepachangelog.com/en/1.0.0/) format (
1212
* Bump minimum `eclipse-cdt` version to `11.0` (removed support for `10.7`). ([#2373](https://github.com/diffplug/spotless/pull/2373))
1313
### Fixed
1414
* `toggleOffOn` now works with the configuration cache. ([#2378](https://github.com/diffplug/spotless/pull/2378) fixes [#2317](https://github.com/diffplug/spotless/issues/2317))
15+
* Using `custom` with a Groovy closure now works with and without configuration cache. ([#2376](https://github.com/diffplug/spotless/pull/2376))
16+
* Minimum required Gradle version for this to work has bumped from `8.0` to `8.4`.
17+
* The global git system config is now ignored for line-ending purposes.
18+
* Added `SerializeToByteArrayHack` as a flag for a limitation at the intersection of `toggleOffOn` and `custom`.
1519
* You can now use `removeUnusedImports` and `googleJavaFormat` at the same time again. (fixes [#2159](https://github.com/diffplug/spotless/issues/2159))
1620
* The default list of type annotations used by `formatAnnotations` now includes Jakarta Validation's `Valid` and constraints validations (fixes [#2334](https://github.com/diffplug/spotless/issues/2334))
1721
* `indentWith[Spaces|Tabs]` has been deprecated in favor of `leadingTabsToSpaces` and `leadingSpacesToTabs`. ([#2350](https://github.com/diffplug/spotless/pull/2350) fixes [#794](https://github.com/diffplug/spotless/issues/794))

plugin-gradle/src/main/java/com/diffplug/gradle/spotless/FormatExtension.java

+11-4
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2016-2024 DiffPlug
2+
* Copyright 2016-2025 DiffPlug
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -457,12 +457,11 @@ protected Integer calculateState() throws Exception {
457457
*/
458458
public void custom(String name, Closure<String> formatter) {
459459
requireNonNull(formatter, "formatter");
460-
Closure<String> dehydrated = formatter.dehydrate();
461-
custom(name, new ClosureFormatterFunc(dehydrated));
460+
custom(name, new ClosureFormatterFunc(formatter));
462461
}
463462

464463
static class ClosureFormatterFunc implements FormatterFunc, Serializable {
465-
private final Closure<String> closure;
464+
private Closure<String> closure;
466465

467466
ClosureFormatterFunc(Closure<String> closure) {
468467
this.closure = closure;
@@ -472,6 +471,14 @@ static class ClosureFormatterFunc implements FormatterFunc, Serializable {
472471
public String apply(String unixNewlines) {
473472
return closure.call(unixNewlines);
474473
}
474+
475+
private void writeObject(java.io.ObjectOutputStream stream) throws java.io.IOException {
476+
stream.writeObject(closure.dehydrate());
477+
}
478+
479+
private void readObject(java.io.ObjectInputStream stream) throws java.io.IOException, ClassNotFoundException {
480+
this.closure = (Closure<String>) stream.readObject();
481+
}
475482
}
476483

477484
/**

plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessPlugin.java

+2-2
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2016-2024 DiffPlug
2+
* Copyright 2016-2025 DiffPlug
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -29,7 +29,7 @@ public class SpotlessPlugin implements Plugin<Project> {
2929
static final String SPOTLESS_MODERN = "spotlessModern";
3030
static final String VER_GRADLE_min = "6.1.1";
3131
static final String VER_GRADLE_javaPluginExtension = "7.1";
32-
static final String VER_GRADLE_minVersionForCustom = "8.0";
32+
static final String VER_GRADLE_minVersionForCustom = "8.4";
3333
private static final int MINIMUM_JRE = 11;
3434

3535
@Override

plugin-gradle/src/test/java/com/diffplug/gradle/spotless/BumpThisNumberIfACustomStepChangesTest.java

+38-3
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2016-2024 DiffPlug
2+
* Copyright 2016-2025 DiffPlug
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -17,9 +17,39 @@
1717

1818
import java.io.IOException;
1919

20+
import org.gradle.testkit.runner.GradleRunner;
2021
import org.junit.jupiter.api.Test;
2122

22-
class BumpThisNumberIfACustomStepChangesTest extends GradleIntegrationHarness {
23+
abstract class BumpThisNumberIfACustomStepChangesTest extends GradleIntegrationHarness {
24+
private boolean useConfigCache;
25+
26+
BumpThisNumberIfACustomStepChangesTest(boolean useConfigCache) {
27+
this.useConfigCache = useConfigCache;
28+
}
29+
30+
static class WithConfigCache extends BumpThisNumberIfACustomStepChangesTest {
31+
WithConfigCache() {
32+
super(true);
33+
}
34+
}
35+
36+
static class WithoutConfigCache extends BumpThisNumberIfACustomStepChangesTest {
37+
WithoutConfigCache() {
38+
super(false);
39+
}
40+
}
41+
42+
@Override
43+
public GradleRunner gradleRunner() throws IOException {
44+
if (useConfigCache) {
45+
setFile("gradle.properties").toLines("org.gradle.unsafe.configuration-cache=true",
46+
"org.gradle.configuration-cache=true");
47+
return super.gradleRunner().withGradleVersion(GradleVersionSupport.CUSTOM_STEPS.version);
48+
} else {
49+
return super.gradleRunner();
50+
}
51+
}
52+
2353
private void writeBuildFile(String toInsert) throws IOException {
2454
setFile("build.gradle").toLines(
2555
"plugins {",
@@ -50,7 +80,12 @@ void customRuleNeverUpToDate() throws IOException {
5080
writeContentWithBadFormatting();
5181
applyIsUpToDate(false);
5282
checkIsUpToDate(false);
53-
checkIsUpToDate(false);
83+
if (useConfigCache) {
84+
// if the config cache is in-effect, then it's okay for custom rules to become "up-to-date"
85+
checkIsUpToDate(true);
86+
} else {
87+
checkIsUpToDate(false);
88+
}
5489
}
5590

5691
@Test

testlib/src/test/java/com/diffplug/spotless/combined/CombinedJavaFormatStepTest.java

+3-1
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2023-2024 DiffPlug
2+
* Copyright 2023-2025 DiffPlug
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -31,6 +31,7 @@
3131
import com.diffplug.spotless.java.GoogleJavaFormatStep;
3232
import com.diffplug.spotless.java.ImportOrderStep;
3333
import com.diffplug.spotless.java.RemoveUnusedImportsStep;
34+
import com.diffplug.spotless.yaml.SerializeToByteArrayHack;
3435

3536
public class CombinedJavaFormatStepTest extends ResourceHarness {
3637

@@ -45,6 +46,7 @@ void checkIssue1679() {
4546
FenceStep toggleOffOnPair = FenceStep.named(FenceStep.defaultToggleName()).openClose("formatting:off", "formatting:on");
4647
try (StepHarness formatter = StepHarness.forSteps(
4748
toggleOffOnPair.preserveWithin(List.of(
49+
new SerializeToByteArrayHack(),
4850
gjf,
4951
indentWithSpaces,
5052
importOrder,

0 commit comments

Comments
 (0)