Make IDE hook work with configuration cache, take 2 (#2308, revises #2278, fixes #1082)

Author: nedtwigg

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

@@ -0,0 +1,37 @@
+/*
+ * Copyright 2024 DiffPlug
+ *
+ * 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.diffplug.spotless;
+
+import java.util.Locale;
+
+/** These FormatterStep are meant to be used for testing only. */
+public class TestingOnly {
+	public static FormatterStep lowercase() {
+		return FormatterStep.create("lowercase", "lowercaseStateUnused", unused -> TestingOnly::lowercase);
+	}
+
+	private static String lowercase(String raw) {
+		return raw.toLowerCase(Locale.ROOT);
+	}
+
+	public static FormatterStep diverge() {
+		return FormatterStep.create("diverge", "divergeStateUnused", unused -> TestingOnly::diverge);
+	}
+
+	private static String diverge(String raw) {
+		return raw + " ";
+	}
+}

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

@@ -19,12 +19,34 @@
 import java.io.IOException;
 import java.nio.file.Files;
 
+import javax.annotation.Nullable;
+
+import org.gradle.api.Project;
+
 import com.diffplug.common.base.Errors;
 import com.diffplug.common.io.ByteStreams;
 import com.diffplug.spotless.DirtyState;
 import com.diffplug.spotless.Formatter;
+import com.diffplug.spotless.NoLambda;
 
 class IdeHook {
+	static class State extends NoLambda.EqualityBasedOnSerialization {
+		final @Nullable String path;
+		final boolean useStdIn;
+		final boolean useStdOut;
+
+		State(Project project) {
+			path = (String) project.findProperty(PROPERTY);
+			if (path != null) {
+				useStdIn = project.hasProperty(USE_STD_IN);
+				useStdOut = project.hasProperty(USE_STD_OUT);
+			} else {
+				useStdIn = false;
+				useStdOut = false;
+			}
+		}
+	}
+
 	final static String PROPERTY = "spotlessIdeHook";
 	final static String USE_STD_IN = "spotlessIdeHookUseStdIn";
 	final static String USE_STD_OUT = "spotlessIdeHookUseStdOut";
@@ -33,9 +55,8 @@ private static void dumpIsClean() {
 		System.err.println("IS CLEAN");
 	}
 
-	static void performHook(SpotlessTaskImpl spotlessTask) {
-		String path = (String) spotlessTask.getProject().property(PROPERTY);
-		File file = new File(path);
+	static void performHook(SpotlessTaskImpl spotlessTask, IdeHook.State state) {
+		File file = new File(state.path);
 		if (!file.isAbsolute()) {
 			System.err.println("Argument passed to " + PROPERTY + " must be an absolute path");
 			return;
@@ -50,7 +71,7 @@ static void performHook(SpotlessTaskImpl spotlessTask) {
 					}
 				}
 				byte[] bytes;
-				if (spotlessTask.getProject().hasProperty(USE_STD_IN)) {
+				if (state.useStdIn) {
 					bytes = ByteStreams.toByteArray(System.in);
 				} else {
 					bytes = Files.readAllBytes(file.toPath());
@@ -63,7 +84,7 @@ static void performHook(SpotlessTaskImpl spotlessTask) {
 					System.err.println("Run 'spotlessDiagnose' for details https://github.com/diffplug/spotless/blob/main/PADDEDCELL.md");
 				} else {
 					System.err.println("IS DIRTY");
-					if (spotlessTask.getProject().hasProperty(USE_STD_OUT)) {
+					if (state.useStdOut) {
 						dirty.writeCanonicalTo(System.out);
 					} else {
 						dirty.writeCanonicalTo(file);

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

@@ -1,5 +1,5 @@
 /*
- * Copyright 2016-2022 DiffPlug
+ * Copyright 2016-2024 DiffPlug
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -48,15 +48,15 @@ public SpotlessExtensionImpl(Project project) {
 
 	@Override
 	protected void createFormatTasks(String name, FormatExtension formatExtension) {
-		boolean isIdeHook = project.hasProperty(IdeHook.PROPERTY);
+		IdeHook.State ideHook = new IdeHook.State(project);
 		TaskContainer tasks = project.getTasks();
 
 		// create the SpotlessTask
 		String taskName = EXTENSION + SpotlessPlugin.capitalize(name);
 		TaskProvider<SpotlessTaskImpl> spotlessTask = tasks.register(taskName, SpotlessTaskImpl.class, task -> {
 			task.init(getRegisterDependenciesTask().getTaskService());
 			task.setGroup(TASK_GROUP);
-			task.setEnabled(!isIdeHook);
+			task.getIdeHookState().set(ideHook);
 			// clean removes the SpotlessCache, so we have to run after clean
 			task.mustRunAfter(BasePlugin.CLEAN_TASK_NAME);
 		});
@@ -75,23 +75,18 @@ protected void createFormatTasks(String name, FormatExtension formatExtension) {
 		TaskProvider<SpotlessApply> applyTask = tasks.register(taskName + APPLY, SpotlessApply.class, task -> {
 			task.init(spotlessTask.get());
 			task.setGroup(TASK_GROUP);
-			task.setEnabled(!isIdeHook);
+			task.setEnabled(ideHook.path == null);
 			task.dependsOn(spotlessTask);
 		});
 		rootApplyTask.configure(task -> {
-			task.dependsOn(applyTask);
-
-			if (isIdeHook) {
-				// the rootApplyTask is no longer just a marker task, now it does a bit of work itself
-				task.doLast(unused -> IdeHook.performHook(spotlessTask.get()));
-			}
+			task.dependsOn(ideHook.path == null ? applyTask : spotlessTask);
 		});
 
 		TaskProvider<SpotlessCheck> checkTask = tasks.register(taskName + CHECK, SpotlessCheck.class, task -> {
 			SpotlessTaskImpl source = spotlessTask.get();
 			task.setGroup(TASK_GROUP);
 			task.init(source);
-			task.setEnabled(!isIdeHook);
+			task.setEnabled(ideHook.path == null);
 			task.dependsOn(source);
 
 			// if the user runs both, make sure that apply happens first,

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

@@ -29,9 +29,12 @@
 import org.gradle.api.GradleException;
 import org.gradle.api.file.DirectoryProperty;
 import org.gradle.api.file.FileSystemOperations;
+import org.gradle.api.provider.Property;
 import org.gradle.api.provider.Provider;
 import org.gradle.api.tasks.CacheableTask;
+import org.gradle.api.tasks.Input;
 import org.gradle.api.tasks.Internal;
+import org.gradle.api.tasks.Optional;
 import org.gradle.api.tasks.TaskAction;
 import org.gradle.work.ChangeType;
 import org.gradle.work.FileChange;
@@ -46,6 +49,10 @@
 
 @CacheableTask
 public abstract class SpotlessTaskImpl extends SpotlessTask {
+	@Input
+	@Optional
+	abstract Property<IdeHook.State> getIdeHookState();
+
 	@Internal
 	abstract DirectoryProperty getProjectDir();
 
@@ -69,6 +76,12 @@ Provider<SpotlessTaskService> getTaskServiceProvider() {
 
 	@TaskAction
 	public void performAction(InputChanges inputs) throws Exception {
+		IdeHook.State ideHook = getIdeHookState().getOrNull();
+		if (ideHook != null && ideHook.path != null) {
+			IdeHook.performHook(this, ideHook);
+			return;
+		}
+
 		SpotlessTaskService taskService = getTaskService().get();
 		taskService.registerSourceAlreadyRan(this);
 		if (target == null) {

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

@@ -1,5 +1,5 @@
 /*
- * Copyright 2016-2021 DiffPlug
+ * Copyright 2016-2024 DiffPlug
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -19,14 +19,19 @@
 import java.io.IOException;
 import java.io.Writer;
 import java.nio.charset.StandardCharsets;
+import java.util.stream.Stream;
 
 import org.assertj.core.api.Assertions;
+import org.gradle.testkit.runner.GradleRunner;
 import org.junit.jupiter.api.BeforeEach;
-import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.TestInstance;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.MethodSource;
 
 import com.diffplug.common.base.StringPrinter;
 import com.diffplug.common.io.Files;
 
+@TestInstance(TestInstance.Lifecycle.PER_CLASS)
 class IdeHookTest extends GradleIntegrationHarness {
 	private String output, error;
 	private File dirty, clean, diverge, outofbounds;
@@ -40,11 +45,11 @@ void before() throws IOException {
 				"spotless {",
 				"  format 'misc', {",
 				"    target 'DIRTY.md', 'CLEAN.md'",
-				"    custom 'lowercase', { str -> str.toLowerCase(Locale.ROOT) }",
+				"    addStep com.diffplug.spotless.TestingOnly.lowercase()",
 				"  }",
 				"  format 'diverge', {",
 				"    target 'DIVERGE.md'",
-				"    custom 'diverge', { str -> str + ' ' }",
+				"    addStep com.diffplug.spotless.TestingOnly.diverge()",
 				"  }",
 				"}");
 		dirty = new File(rootFolder(), "DIRTY.md");
@@ -57,12 +62,16 @@ void before() throws IOException {
 		Files.write("ABC".getBytes(StandardCharsets.UTF_8), outofbounds);
 	}
 
-	private void runWith(String... arguments) throws IOException {
+	private static Stream<Boolean> configurationCacheProvider() {
+		return Stream.of(false, true);
+	}
+
+	private void runWith(boolean configurationCache, String... arguments) throws IOException {
 		StringBuilder output = new StringBuilder();
 		StringBuilder error = new StringBuilder();
 		try (Writer outputWriter = new StringPrinter(output::append).toWriter();
 				Writer errorWriter = new StringPrinter(error::append).toWriter();) {
-			gradleRunner()
+			gradleRunner(configurationCache)
 					.withArguments(arguments)
 					.forwardStdOutput(outputWriter)
 					.forwardStdError(errorWriter)
@@ -72,37 +81,60 @@ private void runWith(String... arguments) throws IOException {
 		this.error = error.toString();
 	}
 
-	@Test
-	void dirty() throws IOException {
-		runWith("spotlessApply", "--quiet", "-PspotlessIdeHook=" + dirty.getAbsolutePath(), "-PspotlessIdeHookUseStdOut");
+	protected GradleRunner gradleRunner(boolean configurationCache) throws IOException {
+		if (configurationCache) {
+			setFile("gradle.properties").toContent("org.gradle.unsafe.configuration-cache=true");
+			setFile("settings.gradle").toContent("enableFeaturePreview(\"STABLE_CONFIGURATION_CACHE\")");
+			return super.gradleRunner().withGradleVersion(GradleVersionSupport.STABLE_CONFIGURATION_CACHE.version);
+		} else {
+			File gradleProps = new File(rootFolder(), "gradle.properties");
+			if (gradleProps.exists()) {
+				gradleProps.delete();
+			}
+			File settingsGradle = new File(rootFolder(), "settings.gradle");
+			if (settingsGradle.exists()) {
+				settingsGradle.delete();
+			}
+			return super.gradleRunner();
+		}
+	}
+
+	@ParameterizedTest
+	@MethodSource("configurationCacheProvider")
+	void dirty(boolean configurationCache) throws IOException {
+		runWith(configurationCache, "spotlessApply", "--quiet", "-PspotlessIdeHook=" + dirty.getAbsolutePath(), "-PspotlessIdeHookUseStdOut");
 		Assertions.assertThat(output).isEqualTo("abc");
 		Assertions.assertThat(error).startsWith("IS DIRTY");
 	}
 
-	@Test
-	void clean() throws IOException {
-		runWith("spotlessApply", "--quiet", "-PspotlessIdeHook=" + clean.getAbsolutePath(), "-PspotlessIdeHookUseStdOut");
+	@ParameterizedTest
+	@MethodSource("configurationCacheProvider")
+	void clean(boolean configurationCache) throws IOException {
+		runWith(configurationCache, "spotlessApply", "--quiet", "-PspotlessIdeHook=" + clean.getAbsolutePath(), "-PspotlessIdeHookUseStdOut");
 		Assertions.assertThat(output).isEmpty();
 		Assertions.assertThat(error).startsWith("IS CLEAN");
 	}
 
-	@Test
-	void diverge() throws IOException {
-		runWith("spotlessApply", "--quiet", "-PspotlessIdeHook=" + diverge.getAbsolutePath(), "-PspotlessIdeHookUseStdOut");
+	@ParameterizedTest
+	@MethodSource("configurationCacheProvider")
+	void diverge(boolean configurationCache) throws IOException {
+		runWith(configurationCache, "spotlessApply", "--quiet", "-PspotlessIdeHook=" + diverge.getAbsolutePath(), "-PspotlessIdeHookUseStdOut");
 		Assertions.assertThat(output).isEmpty();
 		Assertions.assertThat(error).startsWith("DID NOT CONVERGE");
 	}
 
-	@Test
-	void outofbounds() throws IOException {
-		runWith("spotlessApply", "--quiet", "-PspotlessIdeHook=" + outofbounds.getAbsolutePath(), "-PspotlessIdeHookUseStdOut");
+	@ParameterizedTest
+	@MethodSource("configurationCacheProvider")
+	void outofbounds(boolean configurationCache) throws IOException {
+		runWith(configurationCache, "spotlessApply", "--quiet", "-PspotlessIdeHook=" + outofbounds.getAbsolutePath(), "-PspotlessIdeHookUseStdOut");
 		Assertions.assertThat(output).isEmpty();
 		Assertions.assertThat(error).isEmpty();
 	}
 
-	@Test
-	void notAbsolute() throws IOException {
-		runWith("spotlessApply", "--quiet", "-PspotlessIdeHook=build.gradle", "-PspotlessIdeHookUseStdOut");
+	@ParameterizedTest
+	@MethodSource("configurationCacheProvider")
+	void notAbsolute(boolean configurationCache) throws IOException {
+		runWith(configurationCache, "spotlessApply", "--quiet", "-PspotlessIdeHook=build.gradle", "-PspotlessIdeHookUseStdOut");
 		Assertions.assertThat(output).isEmpty();
 		Assertions.assertThat(error).contains("Argument passed to spotlessIdeHook must be an absolute path");
 	}