From 84aedee06d206271cb5c21e752e73f946cea88a7 Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Fri, 16 Oct 2020 12:06:17 -0700 Subject: [PATCH 1/6] Add a test to see if the spotless tasks can config cache (hint: nope). --- .../spotless/ConfigurationCacheTest.java | 20 +++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/ConfigurationCacheTest.java b/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/ConfigurationCacheTest.java index 2907f16dba..1bdf3a732c 100644 --- a/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/ConfigurationCacheTest.java +++ b/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/ConfigurationCacheTest.java @@ -26,6 +26,7 @@ public class ConfigurationCacheTest extends GradleIntegrationHarness { protected void runTasks(String... tasks) throws IOException { setFile("gradle.properties").toContent("org.gradle.unsafe.configuration-cache=true"); List args = new ArrayList<>(); + args.add("--stacktrace"); args.addAll(Arrays.asList(tasks)); gradleRunner() .withGradleVersion(GradleVersionSupport.CONFIGURATION_CACHE.version) @@ -66,4 +67,23 @@ public void helpConfiguresIfTasksAreCreated() throws IOException { "tasks.named('spotlessJavaApply').get()"); runTasks("help"); } + + @Test + public void spotlessConfigures() throws IOException { + setFile("build.gradle").toLines( + "buildscript { repositories { mavenCentral() } }", + "plugins {", + " id 'com.diffplug.spotless'", + "}", + "spotless {", + " format 'misc', {", + " target '*.txt'", + " custom 'lowercase', { str -> str.toLowerCase() }", + " bumpThisNumberIfACustomStepChanges(1)", + " }", + "}"); + setFile("test.txt").toContent("ABC"); + runTasks("spotlessApply"); + assertFile("test.txt").hasContent("abc"); + } } From 5c70c0f3be53e378de33bfe9c79d4e4aadef6526 Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Thu, 15 Oct 2020 15:28:51 -0700 Subject: [PATCH 2/6] If apply is going to run, there's no need for check to run. Moved the place where this is handled to be config-cache friendly. --- .../java/com/diffplug/gradle/spotless/SpotlessApply.java | 1 - .../java/com/diffplug/gradle/spotless/SpotlessCheck.java | 3 --- .../diffplug/gradle/spotless/SpotlessExtensionImpl.java | 7 +++++++ .../java/com/diffplug/gradle/spotless/SpotlessTask.java | 8 -------- 4 files changed, 7 insertions(+), 12 deletions(-) diff --git a/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessApply.java b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessApply.java index a815d5c33c..1926649178 100644 --- a/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessApply.java +++ b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessApply.java @@ -35,7 +35,6 @@ public class SpotlessApply extends DefaultTask { /** Bidirectional link between Apply and Spotless allows check to know if Apply ran or not. */ void linkSource(SpotlessTask source) { this.source = source; - source.applyTask = this; } private File spotlessOutDirectory; diff --git a/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessCheck.java b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessCheck.java index 0be634f603..b72ff4ed60 100644 --- a/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessCheck.java +++ b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessCheck.java @@ -66,9 +66,6 @@ private void performAction(boolean isTest) { ConfigurableFileTree files = getProject().fileTree(spotlessOutDirectory); if (files.isEmpty()) { getState().setDidWork(source.getDidWork()); - } else if (!isTest && getProject().getGradle().getTaskGraph().hasTask(source.applyTask)) { - // if our matching apply has already run, then we don't need to do anything - getState().setDidWork(false); } else { List problemFiles = new ArrayList<>(); files.visit(new FileVisitor() { diff --git a/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessExtensionImpl.java b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessExtensionImpl.java index a542b5a96d..ee28490b78 100644 --- a/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessExtensionImpl.java +++ b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessExtensionImpl.java @@ -112,6 +112,13 @@ protected void createFormatTasks(String name, FormatExtension formatExtension) { }); rootCheckTask.configure(task -> task.dependsOn(checkTask)); + // no need to run check if apply is going to run + project.getGradle().getTaskGraph().whenReady(graph -> { + if (graph.hasTask(taskName + APPLY) && graph.hasTask(taskName + CHECK)) { + checkTask.get().setEnabled(false); + } + }); + // create the diagnose task TaskProvider diagnoseTask = tasks.register(taskName + DIAGNOSE, SpotlessDiagnoseTask.class, task -> { task.source = spotlessTask.get(); diff --git a/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessTask.java b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessTask.java index bca839547a..ddcb0cb2ff 100644 --- a/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessTask.java +++ b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessTask.java @@ -43,14 +43,6 @@ import com.diffplug.spotless.LineEnding; public class SpotlessTask extends DefaultTask { - SpotlessApply applyTask; - - /** Internal use only, allows coordination between check and apply when they are in the same build */ - @Internal - SpotlessApply getApplyTask() { - return applyTask; - } - // set by SpotlessExtension, but possibly overridden by FormatExtension protected String encoding = "UTF-8"; From 3e20f1bbf772b4ff97ac8bc2ebc145261681c6ee Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Fri, 16 Oct 2020 11:34:12 -0700 Subject: [PATCH 3/6] Our main problem is that `spotlessFooApply` and `spotlessFooCheck` need to reference the "worker" task `spotlessFoo` to do various things. We create a `SpotlessTaskService implements BuildService` to serve this role, which allows configuration cache to complete without errors. --- .../gradle/spotless/FormatExtension.java | 5 +- .../gradle/spotless/GitRatchetGradle.java | 11 ++-- .../com/diffplug/gradle/spotless/IdeHook.java | 4 +- .../gradle/spotless/SpotlessApply.java | 32 ++-------- .../gradle/spotless/SpotlessCheck.java | 35 +++-------- .../gradle/spotless/SpotlessDiagnoseTask.java | 18 ++---- .../spotless/SpotlessExtensionImpl.java | 8 +-- .../gradle/spotless/SpotlessTask.java | 16 +---- .../gradle/spotless/SpotlessTaskImpl.java | 61 +++++++++++++++++-- .../gradle/spotless/SpotlessTaskService.java | 59 ++++++++++++++++++ .../spotless/DiffMessageFormatterTest.java | 10 ++- .../gradle/spotless/PaddedCellTaskTest.java | 12 ++-- 12 files changed, 154 insertions(+), 117 deletions(-) create mode 100644 plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessTaskService.java diff --git a/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/FormatExtension.java b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/FormatExtension.java index 1f4f90352d..483a0930c7 100644 --- a/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/FormatExtension.java +++ b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/FormatExtension.java @@ -747,15 +747,14 @@ protected Project getProject() { */ public SpotlessApply createIndependentApplyTask(String taskName) { // create and setup the task - SpotlessTask spotlessTask = spotless.project.getTasks().create(taskName + "Helper", SpotlessTaskImpl.class); + SpotlessTaskImpl spotlessTask = spotless.project.getTasks().create(taskName + "Helper", SpotlessTaskImpl.class); setupTask(spotlessTask); // enforce the clean ordering Task clean = spotless.project.getTasks().getByName(BasePlugin.CLEAN_TASK_NAME); spotlessTask.mustRunAfter(clean); // create the apply task SpotlessApply applyTask = spotless.project.getTasks().create(taskName, SpotlessApply.class); - applyTask.setSpotlessOutDirectory(spotlessTask.getOutputDirectory()); - applyTask.linkSource(spotlessTask); + applyTask.link(spotlessTask); applyTask.dependsOn(spotlessTask); return applyTask; diff --git a/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/GitRatchetGradle.java b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/GitRatchetGradle.java index 7aee242a8f..8b8dc69341 100644 --- a/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/GitRatchetGradle.java +++ b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/GitRatchetGradle.java @@ -19,21 +19,20 @@ import javax.annotation.Nullable; -import org.gradle.api.Project; import org.gradle.api.services.BuildService; import org.gradle.api.services.BuildServiceParameters; import com.diffplug.spotless.extra.GitRatchet; /** Gradle implementation of GitRatchet. */ -public abstract class GitRatchetGradle extends GitRatchet implements BuildService { +public abstract class GitRatchetGradle extends GitRatchet implements BuildService { @Override - protected File getDir(Project project) { - return project.getProjectDir(); + protected File getDir(File project) { + return project; } @Override - protected @Nullable Project getParent(Project project) { - return project.getParent(); + protected @Nullable File getParent(File project) { + return project.getParentFile(); } } diff --git a/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/IdeHook.java b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/IdeHook.java index 9f2e213a17..415361b3d5 100644 --- a/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/IdeHook.java +++ b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/IdeHook.java @@ -33,7 +33,7 @@ private static void dumpIsClean() { System.err.println("IS CLEAN"); } - static void performHook(SpotlessTask spotlessTask) { + static void performHook(SpotlessTaskImpl spotlessTask) { String path = (String) spotlessTask.getProject().property(PROPERTY); File file = new File(path); if (!file.isAbsolute()) { @@ -43,7 +43,7 @@ static void performHook(SpotlessTask spotlessTask) { if (spotlessTask.getTarget().contains(file)) { try (Formatter formatter = spotlessTask.buildFormatter()) { if (spotlessTask.ratchet != null) { - if (spotlessTask.ratchet.isClean(spotlessTask.getProject(), spotlessTask.rootTreeSha, file)) { + if (spotlessTask.ratchet.isClean(spotlessTask.getProjectDir(), spotlessTask.rootTreeSha, file)) { dumpIsClean(); return; } diff --git a/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessApply.java b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessApply.java index 1926649178..9027787aef 100644 --- a/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessApply.java +++ b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessApply.java @@ -20,40 +20,18 @@ import java.nio.file.Files; import java.nio.file.StandardCopyOption; -import org.gradle.api.DefaultTask; import org.gradle.api.file.ConfigurableFileTree; import org.gradle.api.file.FileVisitDetails; import org.gradle.api.file.FileVisitor; -import org.gradle.api.tasks.InputDirectory; -import org.gradle.api.tasks.PathSensitive; -import org.gradle.api.tasks.PathSensitivity; import org.gradle.api.tasks.TaskAction; -public class SpotlessApply extends DefaultTask { - private SpotlessTask source; - - /** Bidirectional link between Apply and Spotless allows check to know if Apply ran or not. */ - void linkSource(SpotlessTask source) { - this.source = source; - } - - private File spotlessOutDirectory; - - @PathSensitive(PathSensitivity.RELATIVE) - @InputDirectory - public File getSpotlessOutDirectory() { - return spotlessOutDirectory; - } - - public void setSpotlessOutDirectory(File spotlessOutDirectory) { - this.spotlessOutDirectory = spotlessOutDirectory; - } - +public abstract class SpotlessApply extends SpotlessTaskService.DependentTask { @TaskAction public void performAction() { - ConfigurableFileTree files = getProject().fileTree(spotlessOutDirectory); + SpotlessTaskImpl task = source(); + ConfigurableFileTree files = task.outputFiles(); if (files.isEmpty()) { - getState().setDidWork(source.getDidWork()); + getState().setDidWork(task.getDidWork()); } else { files.visit(new FileVisitor() { @Override @@ -64,7 +42,7 @@ public void visitDir(FileVisitDetails fileVisitDetails) { @Override public void visitFile(FileVisitDetails fileVisitDetails) { String path = fileVisitDetails.getPath(); - File originalSource = new File(getProject().getProjectDir(), path); + File originalSource = new File(task.getProjectDir(), path); try { getLogger().debug("Copying " + fileVisitDetails.getFile() + " to " + originalSource); Files.copy(fileVisitDetails.getFile().toPath(), originalSource.toPath(), StandardCopyOption.REPLACE_EXISTING, StandardCopyOption.COPY_ATTRIBUTES); diff --git a/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessCheck.java b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessCheck.java index b72ff4ed60..e04180e9bb 100644 --- a/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessCheck.java +++ b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessCheck.java @@ -24,14 +24,10 @@ import java.util.Collections; import java.util.List; -import org.gradle.api.DefaultTask; import org.gradle.api.GradleException; import org.gradle.api.file.ConfigurableFileTree; import org.gradle.api.file.FileVisitDetails; import org.gradle.api.file.FileVisitor; -import org.gradle.api.tasks.InputDirectory; -import org.gradle.api.tasks.PathSensitive; -import org.gradle.api.tasks.PathSensitivity; import org.gradle.api.tasks.TaskAction; import com.diffplug.spotless.FileSignature; @@ -39,19 +35,7 @@ import com.diffplug.spotless.ThrowingEx; import com.diffplug.spotless.extra.integration.DiffMessageFormatter; -public class SpotlessCheck extends DefaultTask { - SpotlessTask source; - private File spotlessOutDirectory; - - @PathSensitive(PathSensitivity.RELATIVE) - @InputDirectory - public File getSpotlessOutDirectory() { - return spotlessOutDirectory; - } - - public void setSpotlessOutDirectory(File spotlessOutDirectory) { - this.spotlessOutDirectory = spotlessOutDirectory; - } +public abstract class SpotlessCheck extends SpotlessTaskService.DependentTask { public void performActionTest() throws Exception { performAction(true); @@ -63,9 +47,10 @@ public void performAction() throws Exception { } private void performAction(boolean isTest) { - ConfigurableFileTree files = getProject().fileTree(spotlessOutDirectory); + SpotlessTaskImpl task = source(); + ConfigurableFileTree files = task.outputFiles(); if (files.isEmpty()) { - getState().setDidWork(source.getDidWork()); + getState().setDidWork(task.getDidWork()); } else { List problemFiles = new ArrayList<>(); files.visit(new FileVisitor() { @@ -77,7 +62,7 @@ public void visitDir(FileVisitDetails fileVisitDetails) { @Override public void visitFile(FileVisitDetails fileVisitDetails) { String path = fileVisitDetails.getPath(); - File originalSource = new File(getProject().getProjectDir(), path); + File originalSource = new File(task.getProjectDir(), path); try { // read the file on disk byte[] userFile = Files.readAllBytes(originalSource.toPath()); @@ -106,7 +91,7 @@ public void visitFile(FileVisitDetails fileVisitDetails) { } }); if (!problemFiles.isEmpty()) { - try (Formatter formatter = source.buildFormatter()) { + try (Formatter formatter = task.buildFormatter()) { Collections.sort(problemFiles); throw formatViolationsFor(formatter, problemFiles); } @@ -117,18 +102,12 @@ public void visitFile(FileVisitDetails fileVisitDetails) { /** Returns an exception which indicates problem files nicely. */ private GradleException formatViolationsFor(Formatter formatter, List problemFiles) { return new GradleException(DiffMessageFormatter.builder() - .runToFix("Run '" + calculateGradleCommand() + " " + getTaskPathPrefix() + "spotlessApply' to fix these violations.") + .runToFix("Run '" + calculateGradleCommand() + " " + SpotlessTaskImpl.getProjectPath(this) + "spotlessApply' to fix these violations.") .formatter(formatter) .problemFiles(problemFiles) .getMessage()); } - private String getTaskPathPrefix() { - return getProject().getPath().equals(":") - ? ":" - : getProject().getPath() + ":"; - } - private static String calculateGradleCommand() { return FileSignature.machineIsWin() ? "gradlew.bat" : "./gradlew"; } diff --git a/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessDiagnoseTask.java b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessDiagnoseTask.java index f20957e650..1f86cbe1db 100644 --- a/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessDiagnoseTask.java +++ b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessDiagnoseTask.java @@ -1,5 +1,5 @@ /* - * Copyright 2016 DiffPlug + * Copyright 2016-2020 DiffPlug * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -22,8 +22,6 @@ import java.nio.file.Paths; import java.util.Locale; -import org.gradle.api.DefaultTask; -import org.gradle.api.tasks.Internal; import org.gradle.api.tasks.TaskAction; import com.diffplug.spotless.Formatter; @@ -31,22 +29,16 @@ import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; -public class SpotlessDiagnoseTask extends DefaultTask { - SpotlessTask source; - - @Internal - public SpotlessTask getSource() { - return source; - } +public abstract class SpotlessDiagnoseTask extends SpotlessTaskService.DependentTask { @TaskAction @SuppressFBWarnings("NP_NULL_ON_SOME_PATH_FROM_RETURN_VALUE") public void performAction() throws IOException { Path srcRoot = getProject().getProjectDir().toPath(); - Path diagnoseRoot = getProject().getBuildDir().toPath().resolve("spotless-diagnose-" + source.formatName()); + Path diagnoseRoot = getProject().getBuildDir().toPath().resolve("spotless-diagnose-" + source().formatName()); getProject().delete(diagnoseRoot.toFile()); - try (Formatter formatter = source.buildFormatter()) { - for (File file : source.target) { + try (Formatter formatter = source().buildFormatter()) { + for (File file : source().target) { getLogger().debug("Running padded cell check on " + file); PaddedCell padded = PaddedCell.check(formatter, file); if (!padded.misbehaved()) { diff --git a/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessExtensionImpl.java b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessExtensionImpl.java index ee28490b78..ac2d8bee00 100644 --- a/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessExtensionImpl.java +++ b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessExtensionImpl.java @@ -89,8 +89,7 @@ protected void createFormatTasks(String name, FormatExtension formatExtension) { TaskProvider applyTask = tasks.register(taskName + APPLY, SpotlessApply.class, task -> { task.setEnabled(!isIdeHook); task.dependsOn(spotlessTask); - task.setSpotlessOutDirectory(spotlessTask.get().getOutputDirectory()); - task.linkSource(spotlessTask.get()); + task.link(spotlessTask.get()); }); rootApplyTask.configure(task -> { task.dependsOn(applyTask); @@ -104,8 +103,7 @@ protected void createFormatTasks(String name, FormatExtension formatExtension) { TaskProvider checkTask = tasks.register(taskName + CHECK, SpotlessCheck.class, task -> { task.setEnabled(!isIdeHook); task.dependsOn(spotlessTask); - task.setSpotlessOutDirectory(spotlessTask.get().getOutputDirectory()); - task.source = spotlessTask.get(); + task.link(spotlessTask.get()); // if the user runs both, make sure that apply happens first, task.mustRunAfter(applyTask); @@ -121,7 +119,7 @@ protected void createFormatTasks(String name, FormatExtension formatExtension) { // create the diagnose task TaskProvider diagnoseTask = tasks.register(taskName + DIAGNOSE, SpotlessDiagnoseTask.class, task -> { - task.source = spotlessTask.get(); + task.link(spotlessTask.get()); task.mustRunAfter(cleanTask); }); rootDiagnoseTask.configure(task -> task.dependsOn(diagnoseTask)); diff --git a/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessTask.java b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessTask.java index ddcb0cb2ff..3294534265 100644 --- a/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessTask.java +++ b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessTask.java @@ -16,7 +16,6 @@ package com.diffplug.gradle.spotless; import java.io.File; -import java.nio.charset.Charset; import java.util.ArrayList; import java.util.Collections; import java.util.List; @@ -38,7 +37,6 @@ import com.diffplug.spotless.FormatExceptionPolicy; import com.diffplug.spotless.FormatExceptionPolicyStrict; -import com.diffplug.spotless.Formatter; import com.diffplug.spotless.FormatterStep; import com.diffplug.spotless.LineEnding; @@ -80,8 +78,8 @@ public void setLineEndingsPolicy(LineEnding.Policy lineEndingsPolicy) { public void setupRatchet(GitRatchetGradle gitRatchet, String ratchetFrom) { ratchet = gitRatchet; - rootTreeSha = gitRatchet.rootTreeShaOf(getProject(), ratchetFrom); - subtreeSha = gitRatchet.subtreeShaOf(getProject(), rootTreeSha); + rootTreeSha = gitRatchet.rootTreeShaOf(getProject().getProjectDir(), ratchetFrom); + subtreeSha = gitRatchet.subtreeShaOf(getProject().getProjectDir(), rootTreeSha); } @Internal @@ -158,14 +156,4 @@ String formatName() { return name; } } - - Formatter buildFormatter() { - return Formatter.builder() - .lineEndingsPolicy(lineEndingsPolicy) - .encoding(Charset.forName(encoding)) - .rootDir(getProject().getRootDir().toPath()) - .steps(steps) - .exceptionPolicy(exceptionPolicy) - .build(); - } } diff --git a/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessTaskImpl.java b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessTaskImpl.java index 23c0679558..882b0096eb 100644 --- a/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessTaskImpl.java +++ b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessTaskImpl.java @@ -17,13 +17,19 @@ import java.io.File; import java.io.IOException; +import java.nio.charset.Charset; import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.StandardCopyOption; import java.util.Comparator; import org.gradle.api.GradleException; +import org.gradle.api.Task; +import org.gradle.api.file.ConfigurableFileTree; +import org.gradle.api.file.FileSystemOperations; +import org.gradle.api.model.ObjectFactory; import org.gradle.api.tasks.CacheableTask; +import org.gradle.api.tasks.Internal; import org.gradle.api.tasks.TaskAction; import org.gradle.work.ChangeType; import org.gradle.work.FileChange; @@ -34,7 +40,44 @@ import com.diffplug.spotless.PaddedCell; @CacheableTask -public class SpotlessTaskImpl extends SpotlessTask { +public abstract class SpotlessTaskImpl extends SpotlessTask { + SpotlessTaskService taskService; + final File projectDir; + + @Internal + public File getProjectDir() { + return projectDir; + } + + @Internal + public SpotlessTaskService getTaskService() { + return taskService; + } + + Formatter buildFormatter() { + return Formatter.builder() + .lineEndingsPolicy(lineEndingsPolicy) + .encoding(Charset.forName(encoding)) + .rootDir(getProjectDir().toPath()) + .steps(steps) + .exceptionPolicy(exceptionPolicy) + .build(); + } + + private final FileSystemOperations fileSystemOperations; + private final ObjectFactory objectFactory; + + @javax.inject.Inject + public SpotlessTaskImpl(FileSystemOperations fileSystemOperations, ObjectFactory objectFactory) { + this.fileSystemOperations = fileSystemOperations; + this.objectFactory = objectFactory; + this.projectDir = getProject().getProjectDir(); + } + + ConfigurableFileTree outputFiles() { + return objectFactory.fileTree().from(getOutputDirectory()); + } + @TaskAction public void performAction(InputChanges inputs) throws Exception { if (target == null) { @@ -43,7 +86,7 @@ public void performAction(InputChanges inputs) throws Exception { if (!inputs.isIncremental()) { getLogger().info("Not incremental: removing prior outputs"); - getProject().delete(outputDirectory); + fileSystemOperations.delete(spec -> spec.delete(outputDirectory)); Files.createDirectories(outputDirectory.toPath()); } @@ -65,7 +108,7 @@ private void processInputFile(Formatter formatter, File input) throws IOExceptio File output = getOutputFile(input); getLogger().debug("Applying format to " + input + " and writing to " + output); PaddedCell.DirtyState dirtyState; - if (ratchet != null && ratchet.isClean(getProject(), rootTreeSha, input)) { + if (ratchet != null && ratchet.isClean(projectDir, rootTreeSha, input)) { dirtyState = PaddedCell.isClean(); } else { dirtyState = PaddedCell.calculateDirtyState(formatter, input); @@ -100,14 +143,20 @@ private void deletePreviousResult(File input) throws IOException { } private File getOutputFile(File input) { - String outputFileName = FormatExtension.relativize(getProject().getProjectDir(), input); + String outputFileName = FormatExtension.relativize(projectDir, input); if (outputFileName == null) { throw new IllegalArgumentException(StringPrinter.buildString(printer -> { - printer.println("Spotless error! All target files must be within the project root. In project " + getProject().getPath()); - printer.println(" root dir: " + getProject().getProjectDir().getAbsolutePath()); + printer.println("Spotless error! All target files must be within the project root. In project " + getProjectPath(this)); + printer.println(" root dir: " + projectDir.getAbsolutePath()); printer.println(" target: " + input.getAbsolutePath()); })); } return new File(outputDirectory, outputFileName); } + + static String getProjectPath(Task task) { + String taskPath = task.getPath(); + int lastColon = taskPath.lastIndexOf(':'); + return lastColon == -1 ? ":" : taskPath.substring(0, lastColon + 1); + } } diff --git a/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessTaskService.java b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessTaskService.java new file mode 100644 index 0000000000..58ccd5fefc --- /dev/null +++ b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessTaskService.java @@ -0,0 +1,59 @@ +/* + * Copyright 2020 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.gradle.spotless; + +import java.util.HashMap; +import java.util.Map; +import java.util.Objects; + +import org.gradle.api.DefaultTask; +import org.gradle.api.services.BuildService; +import org.gradle.api.services.BuildServiceParameters; +import org.gradle.api.tasks.Input; +import org.gradle.api.tasks.Internal; + +/** A service which allows looking up a SpotlessTaskImpl from its path. */ +public abstract class SpotlessTaskService implements BuildService { + private Map taskPathToTask = new HashMap<>(); + + public static abstract class DependentTask extends DefaultTask { + private String sourceTaskPath; + private transient SpotlessTaskService taskService; + + @Input + public final String getSourceTaskPath() { + return sourceTaskPath; + } + + @Internal + public SpotlessTaskService getTaskService() { + return taskService; + } + + protected final SpotlessTaskImpl source() { + return Objects.requireNonNull(taskService.taskPathToTask.get(sourceTaskPath), sourceTaskPath); + } + + public void link(SpotlessTaskImpl task) { + sourceTaskPath = task.getPath(); + taskService = task.getProject().getGradle().getSharedServices() + .registerIfAbsent("SpotlessTaskService", SpotlessTaskService.class, unused -> {}).get(); + if (taskService.taskPathToTask.put(sourceTaskPath, task) == null) { + task.taskService = taskService; + } + } + } +} diff --git a/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/DiffMessageFormatterTest.java b/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/DiffMessageFormatterTest.java index 95544eedc1..bad505cd32 100644 --- a/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/DiffMessageFormatterTest.java +++ b/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/DiffMessageFormatterTest.java @@ -57,17 +57,15 @@ private SpotlessTaskImpl createFormatTask(String name) { return task; } - private SpotlessCheck createCheckTask(String name, SpotlessTask source) { + private SpotlessCheck createCheckTask(String name, SpotlessTaskImpl source) { SpotlessCheck task = project.getTasks().create("spotless" + SpotlessPlugin.capitalize(name) + "Check", SpotlessCheck.class); - task.source = source; - task.setSpotlessOutDirectory(source.getOutputDirectory()); + task.link(source); return task; } - private SpotlessApply createApplyTask(String name, SpotlessTask source) { + private SpotlessApply createApplyTask(String name, SpotlessTaskImpl source) { SpotlessApply task = project.getTasks().create("spotless" + SpotlessPlugin.capitalize(name) + "Apply", SpotlessApply.class); - task.linkSource(this.task); - task.setSpotlessOutDirectory(source.getOutputDirectory()); + task.link(source); return task; } diff --git a/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/PaddedCellTaskTest.java b/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/PaddedCellTaskTest.java index 27dd125d81..36910b1360 100644 --- a/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/PaddedCellTaskTest.java +++ b/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/PaddedCellTaskTest.java @@ -62,17 +62,15 @@ private SpotlessTaskImpl createFormatTask(String name, FormatterStep step) { return task; } - private SpotlessCheck createCheckTask(String name, SpotlessTask source) { + private SpotlessCheck createCheckTask(String name, SpotlessTaskImpl source) { SpotlessCheck task = project.getTasks().create("spotless" + SpotlessPlugin.capitalize(name) + "Check", SpotlessCheck.class); - task.source = source; - task.setSpotlessOutDirectory(source.getOutputDirectory()); + task.link(source); return task; } - private SpotlessApply createApplyTask(String name, SpotlessTask source) { + private SpotlessApply createApplyTask(String name, SpotlessTaskImpl source) { SpotlessApply task = project.getTasks().create("spotless" + SpotlessPlugin.capitalize(name) + "Apply", SpotlessApply.class); - task.linkSource(source); - task.setSpotlessOutDirectory(source.getOutputDirectory()); + task.link(source); return task; } @@ -87,7 +85,7 @@ String checkFailureMsg() { void diagnose() throws IOException { SpotlessDiagnoseTask diagnose = project.getTasks().create("spotless" + SpotlessPlugin.capitalize(name) + "Diagnose", SpotlessDiagnoseTask.class); - diagnose.source = task; + diagnose.link(task); diagnose.performAction(); } From d0b0711140b1bef5ee87fe25e77abd43fe4ded17 Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Fri, 16 Oct 2020 12:28:21 -0700 Subject: [PATCH 4/6] We can cache configuration, but can we execute from that cache? hint: no --- .../com/diffplug/gradle/spotless/ConfigurationCacheTest.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/ConfigurationCacheTest.java b/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/ConfigurationCacheTest.java index 1bdf3a732c..96242cbe61 100644 --- a/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/ConfigurationCacheTest.java +++ b/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/ConfigurationCacheTest.java @@ -69,7 +69,7 @@ public void helpConfiguresIfTasksAreCreated() throws IOException { } @Test - public void spotlessConfigures() throws IOException { + public void spotlessWorksForSimpleStep() throws IOException { setFile("build.gradle").toLines( "buildscript { repositories { mavenCentral() } }", "plugins {", @@ -85,5 +85,6 @@ public void spotlessConfigures() throws IOException { setFile("test.txt").toContent("ABC"); runTasks("spotlessApply"); assertFile("test.txt").hasContent("abc"); + runTasks("spotlessApply"); } } From a8f4798f1ec65e4dd395adbc181a00ba20b4cc2c Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Fri, 16 Oct 2020 14:31:48 -0700 Subject: [PATCH 5/6] A "private static SpotlessTaskService", which allows us to grab the SpotlessTask from a previous build (the last build which actually configured) --- .../gradle/spotless/SpotlessTaskImpl.java | 35 +++++++++++------- .../gradle/spotless/SpotlessTaskService.java | 36 ++++++++++--------- .../spotless/ConfigurationCacheTest.java | 4 +++ 3 files changed, 45 insertions(+), 30 deletions(-) diff --git a/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessTaskImpl.java b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessTaskImpl.java index 882b0096eb..9804e7e3be 100644 --- a/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessTaskImpl.java +++ b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessTaskImpl.java @@ -41,7 +41,6 @@ @CacheableTask public abstract class SpotlessTaskImpl extends SpotlessTask { - SpotlessTaskService taskService; final File projectDir; @Internal @@ -49,19 +48,28 @@ public File getProjectDir() { return projectDir; } - @Internal - public SpotlessTaskService getTaskService() { - return taskService; - } - Formatter buildFormatter() { - return Formatter.builder() - .lineEndingsPolicy(lineEndingsPolicy) - .encoding(Charset.forName(encoding)) - .rootDir(getProjectDir().toPath()) - .steps(steps) - .exceptionPolicy(exceptionPolicy) - .build(); + // + SpotlessTaskImpl original = SpotlessTaskService.instance().get(getPath()); + if (original == this) { + // a SpotlessTask is registered with the SpotlessTaskService **only** if configuration ran + // so if we're in this block, it means that we were configured + return Formatter.builder() + .lineEndingsPolicy(lineEndingsPolicy) + .encoding(Charset.forName(encoding)) + .rootDir(getProjectDir().toPath()) + .steps(steps) + .exceptionPolicy(exceptionPolicy) + .build(); + } else { + // if we're in this block, it means that configuration did not run, and this + // task was deserialized from disk. All of our fields are ".equals" to their + // originals, but their transient fields are missing, so we can't actually run + // them. Luckily, we saved the task from the original, just so that we could restore + // its formatter, whose transient fields are fully populated. + return original.buildFormatter(); + } + // } private final FileSystemOperations fileSystemOperations; @@ -72,6 +80,7 @@ public SpotlessTaskImpl(FileSystemOperations fileSystemOperations, ObjectFactory this.fileSystemOperations = fileSystemOperations; this.objectFactory = objectFactory; this.projectDir = getProject().getProjectDir(); + SpotlessTaskService.instance().put(this); } ConfigurableFileTree outputFiles() { diff --git a/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessTaskService.java b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessTaskService.java index 58ccd5fefc..492b31dd3b 100644 --- a/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessTaskService.java +++ b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessTaskService.java @@ -20,40 +20,42 @@ import java.util.Objects; import org.gradle.api.DefaultTask; -import org.gradle.api.services.BuildService; -import org.gradle.api.services.BuildServiceParameters; import org.gradle.api.tasks.Input; -import org.gradle.api.tasks.Internal; /** A service which allows looking up a SpotlessTaskImpl from its path. */ -public abstract class SpotlessTaskService implements BuildService { - private Map taskPathToTask = new HashMap<>(); +class SpotlessTaskService { + private static final SpotlessTaskService instance = new SpotlessTaskService(); + + static SpotlessTaskService instance() { + return instance; + } + + private SpotlessTaskService() {} + + private Map map = new HashMap<>(); + + synchronized void put(SpotlessTaskImpl task) { + map.put(task.getPath(), task); + } + + synchronized SpotlessTaskImpl get(String taskPath) { + return Objects.requireNonNull(map.get(taskPath), taskPath); + } public static abstract class DependentTask extends DefaultTask { private String sourceTaskPath; - private transient SpotlessTaskService taskService; @Input public final String getSourceTaskPath() { return sourceTaskPath; } - @Internal - public SpotlessTaskService getTaskService() { - return taskService; - } - protected final SpotlessTaskImpl source() { - return Objects.requireNonNull(taskService.taskPathToTask.get(sourceTaskPath), sourceTaskPath); + return instance().get(sourceTaskPath); } public void link(SpotlessTaskImpl task) { sourceTaskPath = task.getPath(); - taskService = task.getProject().getGradle().getSharedServices() - .registerIfAbsent("SpotlessTaskService", SpotlessTaskService.class, unused -> {}).get(); - if (taskService.taskPathToTask.put(sourceTaskPath, task) == null) { - task.taskService = taskService; - } } } } diff --git a/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/ConfigurationCacheTest.java b/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/ConfigurationCacheTest.java index 96242cbe61..882ff55946 100644 --- a/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/ConfigurationCacheTest.java +++ b/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/ConfigurationCacheTest.java @@ -86,5 +86,9 @@ public void spotlessWorksForSimpleStep() throws IOException { runTasks("spotlessApply"); assertFile("test.txt").hasContent("abc"); runTasks("spotlessApply"); + runTasks("spotlessApply"); + setFile("test.txt").toContent("ABC"); + runTasks("spotlessApply"); + assertFile("test.txt").hasContent("abc"); } } From 3986fe6c047baae784b5e9d37008267c61cff028 Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Fri, 16 Oct 2020 14:46:15 -0700 Subject: [PATCH 6/6] But this classloader solution isn't perfect, because Gradle throws deserialization error: ``` Caused by: java.lang.InstantiationError: [B at org.gradle.instantexecution.serialization.beans.BeanPropertyReader.newBean(BeanPropertyReader.kt:58) at org.gradle.instantexecution.serialization.beans.BeanStateReader$DefaultImpls.newBeanWithId(BeanStateReader.kt:25) at org.gradle.instantexecution.serialization.beans.BeanPropertyReader.newBeanWithId(BeanPropertyReader.kt:37) ``` --- .../spotless/ConfigurationCacheTest.java | 25 +++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/ConfigurationCacheTest.java b/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/ConfigurationCacheTest.java index 882ff55946..1da42aad60 100644 --- a/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/ConfigurationCacheTest.java +++ b/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/ConfigurationCacheTest.java @@ -91,4 +91,29 @@ public void spotlessWorksForSimpleStep() throws IOException { runTasks("spotlessApply"); assertFile("test.txt").hasContent("abc"); } + + @Test + public void spotlessWorksForComplexStep() throws IOException { + setFile("build.gradle").toLines( + "buildscript { repositories { mavenCentral() } }", + "plugins {", + " id 'com.diffplug.spotless'", + "}", + "apply plugin: 'java'", + "spotless {", + " java {", + " target 'test.java'", + " googleJavaFormat('1.2')", + " }", + "}"); + setFile("test.java").toResource("java/googlejavaformat/JavaCodeUnformatted.test"); + runTasks("spotlessApply"); + assertFile("test.java").sameAsResource("java/googlejavaformat/JavaCodeFormatted.test"); + runTasks("spotlessApply"); + runTasks("spotlessApply"); + + setFile("test.java").toResource("java/googlejavaformat/JavaCodeUnformatted.test"); + runTasks("spotlessApply"); + assertFile("test.java").sameAsResource("java/googlejavaformat/JavaCodeFormatted.test"); + } }