From 46122253b5bbd1a14edff0cdc8c83a52fc6c8468 Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Sat, 6 Nov 2021 12:40:28 -0700 Subject: [PATCH 01/24] First cut at replacing our task-specific fields with SpotlessTaskService. --- .../gradle/spotless/FormatExtension.java | 5 +- .../gradle/spotless/SpotlessApply.java | 19 ++++-- .../gradle/spotless/SpotlessCheck.java | 30 ++++++--- .../gradle/spotless/SpotlessExtension.java | 3 + .../spotless/SpotlessExtensionImpl.java | 16 ++++- .../gradle/spotless/SpotlessTask.java | 10 +-- .../gradle/spotless/SpotlessTaskImpl.java | 8 ++- .../gradle/spotless/SpotlessTaskService.java | 65 +++++++++++++++++++ 8 files changed, 126 insertions(+), 30 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 92b9e20f3a..0f533909cf 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 @@ -758,7 +758,8 @@ 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); + spotlessTask.getTaskService().set(spotless.getTaskService()); setupTask(spotlessTask); // enforce the clean ordering Task clean = spotless.project.getTasks().getByName(BasePlugin.CLEAN_TASK_NAME); @@ -766,7 +767,7 @@ public SpotlessApply createIndependentApplyTask(String taskName) { // create the apply task SpotlessApply applyTask = spotless.project.getTasks().create(taskName, SpotlessApply.class); applyTask.setSpotlessOutDirectory(spotlessTask.getOutputDirectory()); - applyTask.linkSource(spotlessTask); + applyTask.getTaskService().set(spotless.getTaskService()); applyTask.dependsOn(spotlessTask); return applyTask; 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 f96f2b1131..3c09ae19ed 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 @@ -24,16 +24,20 @@ import org.gradle.api.file.ConfigurableFileTree; import org.gradle.api.file.FileVisitDetails; import org.gradle.api.file.FileVisitor; +import org.gradle.api.provider.Property; import org.gradle.api.tasks.Internal; import org.gradle.api.tasks.TaskAction; -public class SpotlessApply extends DefaultTask { - private SpotlessTask source; +import com.diffplug.common.base.Preconditions; - /** 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; +public abstract class SpotlessApply extends DefaultTask { + @Internal + abstract Property getTaskService(); + + private String getSourceTaskPath() { + String path = getPath(); + Preconditions.checkArgument(path.endsWith(SpotlessExtension.APPLY)); + return path.substring(0, path.length() - SpotlessExtension.APPLY.length()); } private File spotlessOutDirectory; @@ -49,9 +53,10 @@ public void setSpotlessOutDirectory(File spotlessOutDirectory) { @TaskAction public void performAction() { + getTaskService().get().registerApply(this); ConfigurableFileTree files = getProject().fileTree(spotlessOutDirectory); if (files.isEmpty()) { - getState().setDidWork(source.getDidWork()); + getState().setDidWork(getTaskService().get().getSourceDidWork(getSourceTaskPath())); } else { files.visit(new FileVisitor() { @Override 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 3fb3e0e8c3..19b23fd7d8 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 @@ -29,16 +29,30 @@ import org.gradle.api.file.ConfigurableFileTree; import org.gradle.api.file.FileVisitDetails; import org.gradle.api.file.FileVisitor; +import org.gradle.api.provider.Property; import org.gradle.api.tasks.Internal; import org.gradle.api.tasks.TaskAction; +import com.diffplug.common.base.Preconditions; import com.diffplug.spotless.FileSignature; import com.diffplug.spotless.Formatter; import com.diffplug.spotless.ThrowingEx; import com.diffplug.spotless.extra.integration.DiffMessageFormatter; -public class SpotlessCheck extends DefaultTask { - SpotlessTask source; +public abstract class SpotlessCheck extends DefaultTask { + @Internal + abstract Property getTaskService(); + + private String getSourceTaskPath() { + String path = getPath(); + Preconditions.checkArgument(path.endsWith(SpotlessExtension.CHECK)); + return path.substring(0, path.length() - SpotlessExtension.CHECK.length()); + } + + private String getApplyTaskPath() { + return getSourceTaskPath() + SpotlessExtension.APPLY; + } + private File spotlessOutDirectory; @Internal @@ -50,20 +64,20 @@ public void setSpotlessOutDirectory(File spotlessOutDirectory) { this.spotlessOutDirectory = spotlessOutDirectory; } - public void performActionTest() throws Exception { + public void performActionTest() { performAction(true); } @TaskAction - public void performAction() throws Exception { + public void performAction() { performAction(false); } 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)) { + getState().setDidWork(getTaskService().get().getApplyDidWork(getApplyTaskPath())); + } else if (!isTest && getTaskService().get().applyWasInGraph(getApplyTaskPath())) { // if our matching apply has already run, then we don't need to do anything getState().setDidWork(false); } else { @@ -106,8 +120,8 @@ public void visitFile(FileVisitDetails fileVisitDetails) { } }); if (!problemFiles.isEmpty()) { - try (Formatter formatter = source.buildFormatter()) { - Collections.sort(problemFiles); + Collections.sort(problemFiles); + try (Formatter formatter = getTaskService().get().buildFormatter(getSourceTaskPath())) { throw formatViolationsFor(formatter, problemFiles); } } diff --git a/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessExtension.java b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessExtension.java index 7959bdc71d..d89acdd061 100644 --- a/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessExtension.java +++ b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessExtension.java @@ -27,6 +27,7 @@ import org.gradle.api.Action; import org.gradle.api.GradleException; import org.gradle.api.Project; +import org.gradle.api.provider.Provider; import com.diffplug.spotless.LineEnding; @@ -46,6 +47,8 @@ protected SpotlessExtension(Project project) { this.project = requireNonNull(project); } + abstract Provider getTaskService(); + abstract RegisterDependenciesTask getRegisterDependenciesTask(); /** Line endings (if any). */ 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..61eb121e36 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 @@ -1,5 +1,5 @@ /* - * Copyright 2016-2020 DiffPlug + * Copyright 2016-2021 DiffPlug * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -19,11 +19,13 @@ import org.gradle.api.Project; import org.gradle.api.plugins.BasePlugin; import org.gradle.api.plugins.JavaBasePlugin; +import org.gradle.api.provider.Provider; import org.gradle.api.tasks.TaskContainer; import org.gradle.api.tasks.TaskProvider; public class SpotlessExtensionImpl extends SpotlessExtension { private final TaskProvider registerDependenciesTask; + private final Provider taskService; public SpotlessExtensionImpl(Project project) { super(project); @@ -52,6 +54,13 @@ public SpotlessExtensionImpl(Project project) { .configure(task -> task.dependsOn(rootCheckTask)); } }); + + taskService = project.getGradle().getSharedServices().registerIfAbsent("SpotlessTaskService", SpotlessTaskService.class, spec -> {}); + } + + @Override + Provider getTaskService() { + return taskService; } final TaskProvider rootCheckTask, rootApplyTask, rootDiagnoseTask; @@ -69,6 +78,7 @@ protected void createFormatTasks(String name, FormatExtension formatExtension) { // create the SpotlessTask String taskName = EXTENSION + SpotlessPlugin.capitalize(name); TaskProvider spotlessTask = tasks.register(taskName, SpotlessTaskImpl.class, task -> { + task.getTaskService().set(taskService); task.setEnabled(!isIdeHook); // clean removes the SpotlessCache, so we have to run after clean task.mustRunAfter(cleanTask); @@ -87,10 +97,10 @@ protected void createFormatTasks(String name, FormatExtension formatExtension) { // create the check and apply control tasks TaskProvider applyTask = tasks.register(taskName + APPLY, SpotlessApply.class, task -> { + task.getTaskService().set(taskService); task.setEnabled(!isIdeHook); task.dependsOn(spotlessTask); task.setSpotlessOutDirectory(spotlessTask.get().getOutputDirectory()); - task.linkSource(spotlessTask.get()); }); rootApplyTask.configure(task -> { task.dependsOn(applyTask); @@ -102,10 +112,10 @@ protected void createFormatTasks(String name, FormatExtension formatExtension) { }); TaskProvider checkTask = tasks.register(taskName + CHECK, SpotlessCheck.class, task -> { + task.getTaskService().set(taskService); task.setEnabled(!isIdeHook); task.dependsOn(spotlessTask); task.setSpotlessOutDirectory(spotlessTask.get().getOutputDirectory()); - task.source = spotlessTask.get(); // if the user runs both, make sure that apply happens first, task.mustRunAfter(applyTask); 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..9a08aa9b35 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 @@ -1,5 +1,5 @@ /* - * Copyright 2020 DiffPlug + * Copyright 2020-2021 DiffPlug * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -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"; 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 b5351c701b..f3008217d5 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 @@ -23,7 +23,9 @@ import java.util.Comparator; import org.gradle.api.GradleException; +import org.gradle.api.provider.Property; 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,9 +36,13 @@ import com.diffplug.spotless.PaddedCell; @CacheableTask -public class SpotlessTaskImpl extends SpotlessTask { +public abstract class SpotlessTaskImpl extends SpotlessTask { + @Internal + abstract Property getTaskService(); + @TaskAction public void performAction(InputChanges inputs) throws Exception { + getTaskService().get().registerSource(this); if (target == null) { throw new GradleException("You must specify 'Iterable target'"); } 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..f83efcba52 --- /dev/null +++ b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessTaskService.java @@ -0,0 +1,65 @@ +/* + * Copyright 2021 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.services.BuildService; +import org.gradle.api.services.BuildServiceParameters; + +import com.diffplug.spotless.Formatter; + +public abstract class SpotlessTaskService implements BuildService { + private Map apply = new HashMap<>(); + private Map source = new HashMap<>(); + + public void registerSource(SpotlessTask task) { + source.put(task.getPath(), task); + } + + public void registerApply(SpotlessApply task) { + apply.put(task.getPath(), task); + } + + public boolean getSourceDidWork(String sourceTaskPath) { + SpotlessTask sourceTask = source.get(sourceTaskPath); + if (sourceTask != null) { + return sourceTask.getDidWork(); + } else { + return false; + } + } + + public boolean getApplyDidWork(String applyTaskPath) { + SpotlessApply applyTask = apply.get(applyTaskPath); + if (applyTask != null) { + return applyTask.getDidWork(); + } else { + return false; + } + } + + public boolean applyWasInGraph(String applyTaskPath) { + return apply.containsKey(applyTaskPath); + } + + public Formatter buildFormatter(String sourceTaskPath) { + SpotlessTask task = Objects.requireNonNull(source.get(sourceTaskPath)); + return task.buildFormatter(); + } +} From 23ed7c404fd10d8c8f92002470053ec5f4e8d0c3 Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Sat, 6 Nov 2021 12:55:01 -0700 Subject: [PATCH 02/24] Refactor check and apply to be subclass of SpotlessTaskService.ClientTask. --- .../gradle/spotless/SpotlessApply.java | 17 +--- .../gradle/spotless/SpotlessCheck.java | 24 +----- .../gradle/spotless/SpotlessTaskService.java | 80 +++++++++++++------ 3 files changed, 63 insertions(+), 58 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 3c09ae19ed..903c151187 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,26 +20,13 @@ 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.provider.Property; import org.gradle.api.tasks.Internal; import org.gradle.api.tasks.TaskAction; -import com.diffplug.common.base.Preconditions; - -public abstract class SpotlessApply extends DefaultTask { - @Internal - abstract Property getTaskService(); - - private String getSourceTaskPath() { - String path = getPath(); - Preconditions.checkArgument(path.endsWith(SpotlessExtension.APPLY)); - return path.substring(0, path.length() - SpotlessExtension.APPLY.length()); - } - +public abstract class SpotlessApply extends SpotlessTaskService.ClientTask { private File spotlessOutDirectory; @Internal @@ -56,7 +43,7 @@ public void performAction() { getTaskService().get().registerApply(this); ConfigurableFileTree files = getProject().fileTree(spotlessOutDirectory); if (files.isEmpty()) { - getState().setDidWork(getTaskService().get().getSourceDidWork(getSourceTaskPath())); + getState().setDidWork(getSourceDidWork()); } else { files.visit(new FileVisitor() { @Override 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 19b23fd7d8..e0a12d2f71 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,35 +24,19 @@ 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.provider.Property; import org.gradle.api.tasks.Internal; import org.gradle.api.tasks.TaskAction; -import com.diffplug.common.base.Preconditions; import com.diffplug.spotless.FileSignature; import com.diffplug.spotless.Formatter; import com.diffplug.spotless.ThrowingEx; import com.diffplug.spotless.extra.integration.DiffMessageFormatter; -public abstract class SpotlessCheck extends DefaultTask { - @Internal - abstract Property getTaskService(); - - private String getSourceTaskPath() { - String path = getPath(); - Preconditions.checkArgument(path.endsWith(SpotlessExtension.CHECK)); - return path.substring(0, path.length() - SpotlessExtension.CHECK.length()); - } - - private String getApplyTaskPath() { - return getSourceTaskPath() + SpotlessExtension.APPLY; - } - +public abstract class SpotlessCheck extends SpotlessTaskService.ClientTask { private File spotlessOutDirectory; @Internal @@ -76,8 +60,8 @@ public void performAction() { private void performAction(boolean isTest) { ConfigurableFileTree files = getProject().fileTree(spotlessOutDirectory); if (files.isEmpty()) { - getState().setDidWork(getTaskService().get().getApplyDidWork(getApplyTaskPath())); - } else if (!isTest && getTaskService().get().applyWasInGraph(getApplyTaskPath())) { + getState().setDidWork(getApplyDidWork()); + } else if (!isTest && applyWasInGraph()) { // if our matching apply has already run, then we don't need to do anything getState().setDidWork(false); } else { @@ -121,7 +105,7 @@ public void visitFile(FileVisitDetails fileVisitDetails) { }); if (!problemFiles.isEmpty()) { Collections.sort(problemFiles); - try (Formatter formatter = getTaskService().get().buildFormatter(getSourceTaskPath())) { + try (Formatter formatter = buildFormatter()) { throw formatViolationsFor(formatter, problemFiles); } } 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 f83efcba52..04bd379ba6 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 @@ -15,51 +15,85 @@ */ package com.diffplug.gradle.spotless; +import java.util.Collections; import java.util.HashMap; import java.util.Map; import java.util.Objects; +import org.gradle.api.DefaultTask; +import org.gradle.api.provider.Property; import org.gradle.api.services.BuildService; import org.gradle.api.services.BuildServiceParameters; +import org.gradle.api.tasks.Internal; +import com.diffplug.common.base.Preconditions; +import com.diffplug.common.base.Unhandled; import com.diffplug.spotless.Formatter; +/** + * Allows the check and apply tasks to coordinate + * with each other (and the source task) to reduce + * duplicated work (e.g. no need for check to run if + * apply already did). + */ public abstract class SpotlessTaskService implements BuildService { - private Map apply = new HashMap<>(); - private Map source = new HashMap<>(); + private Map apply = Collections.synchronizedMap(new HashMap<>()); + private Map source = Collections.synchronizedMap(new HashMap<>()); public void registerSource(SpotlessTask task) { source.put(task.getPath(), task); } public void registerApply(SpotlessApply task) { - apply.put(task.getPath(), task); + apply.put(task.getSourceTaskPath(), task); } - public boolean getSourceDidWork(String sourceTaskPath) { - SpotlessTask sourceTask = source.get(sourceTaskPath); - if (sourceTask != null) { - return sourceTask.getDidWork(); - } else { - return false; + public static abstract class ClientTask extends DefaultTask { + @Internal + abstract Property getTaskService(); + + String getSourceTaskPath() { + String path = getPath(); + if (this instanceof SpotlessApply) { + Preconditions.checkArgument(path.endsWith(SpotlessExtension.APPLY)); + return path.substring(0, path.length() - SpotlessExtension.APPLY.length()); + } else if (this instanceof SpotlessCheck) { + Preconditions.checkArgument(path.endsWith(SpotlessExtension.CHECK)); + return path.substring(0, path.length() - SpotlessExtension.CHECK.length()); + } else { + throw Unhandled.classException(this); + } } - } - public boolean getApplyDidWork(String applyTaskPath) { - SpotlessApply applyTask = apply.get(applyTaskPath); - if (applyTask != null) { - return applyTask.getDidWork(); - } else { - return false; + private SpotlessTaskService service() { + return getTaskService().get(); } - } - public boolean applyWasInGraph(String applyTaskPath) { - return apply.containsKey(applyTaskPath); - } + public boolean getSourceDidWork() { + SpotlessTask sourceTask = service().source.get(getSourceTaskPath()); + if (sourceTask != null) { + return sourceTask.getDidWork(); + } else { + return false; + } + } - public Formatter buildFormatter(String sourceTaskPath) { - SpotlessTask task = Objects.requireNonNull(source.get(sourceTaskPath)); - return task.buildFormatter(); + public boolean getApplyDidWork() { + SpotlessApply applyTask = service().apply.get(getSourceTaskPath()); + if (applyTask != null) { + return applyTask.getDidWork(); + } else { + return false; + } + } + + public boolean applyWasInGraph() { + return service().apply.containsKey(getSourceTaskPath()); + } + + public Formatter buildFormatter() { + SpotlessTask task = Objects.requireNonNull(service().source.get(getSourceTaskPath())); + return task.buildFormatter(); + } } } From c6f8e84907faf27b54c2ce2677664d37528503a7 Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Sat, 6 Nov 2021 13:01:13 -0700 Subject: [PATCH 03/24] Fix FormatExtension::createIndependentApplyTask to work with the string-mapping in SpotlessTaskService. --- plugin-gradle/CHANGES.md | 1 + .../com/diffplug/gradle/spotless/FormatExtension.java | 6 +++++- .../diffplug/gradle/spotless/SpotlessTaskService.java | 9 +++++++-- 3 files changed, 13 insertions(+), 3 deletions(-) diff --git a/plugin-gradle/CHANGES.md b/plugin-gradle/CHANGES.md index 7d4587fe47..f3c72a325e 100644 --- a/plugin-gradle/CHANGES.md +++ b/plugin-gradle/CHANGES.md @@ -5,6 +5,7 @@ We adhere to the [keepachangelog](https://keepachangelog.com/en/1.0.0/) format ( ## [Unreleased] ### Changed * **BREAKING** Previously, many projects required `buildscript { repositories { mavenCentral() }}` at the top of their root project, because Spotless resolved its dependencies using the buildscript repositories. Spotless now resolves its dependencies from the normal project repositories of the root project, which means that you can remove the `buildscript {}` block, but you still need `repositories { mavenCentral() }` (or similar) in the root project. +* **BREAKING** `createIndepentApplyTask(String taskName)` now requires that `taskName` does not end with `Apply` * Bump minimum required Gradle from `6.1` to `6.1.1`. ## [5.17.1] - 2021-10-26 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 0f533909cf..3db4b1a750 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 @@ -40,6 +40,7 @@ import org.gradle.api.file.FileCollection; import org.gradle.api.plugins.BasePlugin; +import com.diffplug.common.base.Preconditions; import com.diffplug.spotless.FormatExceptionPolicyStrict; import com.diffplug.spotless.FormatterFunc; import com.diffplug.spotless.FormatterStep; @@ -754,11 +755,14 @@ protected Project getProject() { * * The returned task will not be hooked up to the global {@code spotlessApply}, and there will be no corresponding {@code check} task. * + * The task name must not end with `Apply`. + * * NOTE: does not respect the rarely-used {@code spotlessFiles} property. */ public SpotlessApply createIndependentApplyTask(String taskName) { + Preconditions.checkArgument(!taskName.endsWith(SpotlessExtension.APPLY), "Task name must not end with " + SpotlessExtension.APPLY); // create and setup the task - SpotlessTaskImpl spotlessTask = spotless.project.getTasks().create(taskName + "Helper", SpotlessTaskImpl.class); + SpotlessTaskImpl spotlessTask = spotless.project.getTasks().create(taskName + SpotlessTaskService.INDEPENDENT_HELPER, SpotlessTaskImpl.class); spotlessTask.getTaskService().set(spotless.getTaskService()); setupTask(spotlessTask); // enforce the clean ordering 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 04bd379ba6..310d6dd209 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 @@ -48,6 +48,8 @@ public void registerApply(SpotlessApply task) { apply.put(task.getSourceTaskPath(), task); } + static String INDEPENDENT_HELPER = "Helper"; + public static abstract class ClientTask extends DefaultTask { @Internal abstract Property getTaskService(); @@ -55,8 +57,11 @@ public static abstract class ClientTask extends DefaultTask { String getSourceTaskPath() { String path = getPath(); if (this instanceof SpotlessApply) { - Preconditions.checkArgument(path.endsWith(SpotlessExtension.APPLY)); - return path.substring(0, path.length() - SpotlessExtension.APPLY.length()); + if (path.endsWith(SpotlessExtension.APPLY)) { + return path.substring(0, path.length() - SpotlessExtension.APPLY.length()); + } else { + return path + INDEPENDENT_HELPER; + } } else if (this instanceof SpotlessCheck) { Preconditions.checkArgument(path.endsWith(SpotlessExtension.CHECK)); return path.substring(0, path.length() - SpotlessExtension.CHECK.length()); From e9ad708951bfef3503163e1509e617639a88fb45 Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Sat, 6 Nov 2021 13:36:03 -0700 Subject: [PATCH 04/24] Better names for SpotlessTaskService. --- .../gradle/spotless/SpotlessApply.java | 2 +- .../gradle/spotless/SpotlessCheck.java | 4 ++-- .../gradle/spotless/SpotlessTaskImpl.java | 2 +- .../gradle/spotless/SpotlessTaskService.java | 18 ++++++++++-------- 4 files changed, 14 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 903c151187..84cb5ad932 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 @@ -40,7 +40,7 @@ public void setSpotlessOutDirectory(File spotlessOutDirectory) { @TaskAction public void performAction() { - getTaskService().get().registerApply(this); + getTaskService().get().registerApplyAlreadyRan(this); ConfigurableFileTree files = getProject().fileTree(spotlessOutDirectory); if (files.isEmpty()) { getState().setDidWork(getSourceDidWork()); 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 e0a12d2f71..2ea10e7617 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 @@ -61,7 +61,7 @@ private void performAction(boolean isTest) { ConfigurableFileTree files = getProject().fileTree(spotlessOutDirectory); if (files.isEmpty()) { getState().setDidWork(getApplyDidWork()); - } else if (!isTest && applyWasInGraph()) { + } else if (!isTest && applyHasRun()) { // if our matching apply has already run, then we don't need to do anything getState().setDidWork(false); } else { @@ -105,7 +105,7 @@ public void visitFile(FileVisitDetails fileVisitDetails) { }); if (!problemFiles.isEmpty()) { Collections.sort(problemFiles); - try (Formatter formatter = buildFormatter()) { + try (Formatter formatter = buildFormatterForCheck()) { throw formatViolationsFor(formatter, problemFiles); } } 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 f3008217d5..1700029023 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 @@ -42,7 +42,7 @@ public abstract class SpotlessTaskImpl extends SpotlessTask { @TaskAction public void performAction(InputChanges inputs) throws Exception { - getTaskService().get().registerSource(this); + getTaskService().get().registerSourceAlreadyRan(this); if (target == null) { throw new GradleException("You must specify 'Iterable target'"); } 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 310d6dd209..952e964ff8 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 @@ -18,7 +18,6 @@ import java.util.Collections; import java.util.HashMap; import java.util.Map; -import java.util.Objects; import org.gradle.api.DefaultTask; import org.gradle.api.provider.Property; @@ -37,14 +36,14 @@ * apply already did). */ public abstract class SpotlessTaskService implements BuildService { - private Map apply = Collections.synchronizedMap(new HashMap<>()); - private Map source = Collections.synchronizedMap(new HashMap<>()); + private final Map apply = Collections.synchronizedMap(new HashMap<>()); + private final Map source = Collections.synchronizedMap(new HashMap<>()); - public void registerSource(SpotlessTask task) { + public void registerSourceAlreadyRan(SpotlessTask task) { source.put(task.getPath(), task); } - public void registerApply(SpotlessApply task) { + public void registerApplyAlreadyRan(SpotlessApply task) { apply.put(task.getSourceTaskPath(), task); } @@ -92,12 +91,15 @@ public boolean getApplyDidWork() { } } - public boolean applyWasInGraph() { + public boolean applyHasRun() { return service().apply.containsKey(getSourceTaskPath()); } - public Formatter buildFormatter() { - SpotlessTask task = Objects.requireNonNull(service().source.get(getSourceTaskPath())); + public Formatter buildFormatterForCheck() { + SpotlessTask task = service().source.get(getSourceTaskPath()); + if (task == null) { + throw new IllegalStateException(getPath() + " is running but " + getSourceTaskPath() + " was up-to-date and didn't run"); + } return task.buildFormatter(); } } From 3c762bb83e301371b15f026598ca9349d7f2ddec Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Sat, 6 Nov 2021 13:58:21 -0700 Subject: [PATCH 05/24] Fix tests which needed to set the SpotlessTaskService manually. --- .../extra/integration/DiffMessageFormatter.java | 5 ++--- .../gradle/spotless/DiffMessageFormatterTest.java | 13 +++++++++++-- .../diffplug/gradle/spotless/FormatTaskTest.java | 7 +++++++ .../gradle/spotless/PaddedCellTaskTest.java | 12 ++++++++++-- 4 files changed, 30 insertions(+), 7 deletions(-) diff --git a/lib-extra/src/main/java/com/diffplug/spotless/extra/integration/DiffMessageFormatter.java b/lib-extra/src/main/java/com/diffplug/spotless/extra/integration/DiffMessageFormatter.java index 8db5ebeb24..1551d3113a 100644 --- a/lib-extra/src/main/java/com/diffplug/spotless/extra/integration/DiffMessageFormatter.java +++ b/lib-extra/src/main/java/com/diffplug/spotless/extra/integration/DiffMessageFormatter.java @@ -1,5 +1,5 @@ /* - * Copyright 2016-2020 DiffPlug + * Copyright 2016-2021 DiffPlug * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -164,8 +164,7 @@ private void addIntendedLine(String indent, String line) { private static String diff(Builder builder, File file) throws IOException { String raw = new String(Files.readAllBytes(file.toPath()), builder.formatter.getEncoding()); String rawUnix = LineEnding.toUnix(raw); - String formattedUnix; - formattedUnix = PaddedCell.check(builder.formatter, file, rawUnix).canonical(); + String formattedUnix = PaddedCell.check(builder.formatter, file, rawUnix).canonical(); if (rawUnix.equals(formattedUnix)) { // the formatting is fine, so it's a line-ending issue 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 d64bf91501..87c93d0bda 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 @@ -25,6 +25,7 @@ import org.assertj.core.api.Assertions; import org.gradle.api.Project; +import org.gradle.api.services.BuildServiceParameters; import org.junit.jupiter.api.Test; import com.diffplug.common.base.StringPrinter; @@ -39,6 +40,13 @@ class DiffMessageFormatterTest extends ResourceHarness { private class Bundle { Project project = TestProvisioner.gradleProject(rootFolder()); + SpotlessTaskService taskService = new SpotlessTaskService() { + @Override + public BuildServiceParameters.None getParameters() { + return null; + } + }; + File file; SpotlessTaskImpl task; SpotlessCheck check; @@ -52,6 +60,7 @@ private class Bundle { private SpotlessTaskImpl createFormatTask(String name) { SpotlessTaskImpl task = project.getTasks().create("spotless" + SpotlessPlugin.capitalize(name), SpotlessTaskImpl.class); + task.getTaskService().set(taskService); task.setLineEndingsPolicy(LineEnding.UNIX.createPolicy()); task.setTarget(Collections.singletonList(file)); return task; @@ -59,14 +68,14 @@ private SpotlessTaskImpl createFormatTask(String name) { private SpotlessCheck createCheckTask(String name, SpotlessTask source) { SpotlessCheck task = project.getTasks().create("spotless" + SpotlessPlugin.capitalize(name) + "Check", SpotlessCheck.class); - task.source = source; + task.getTaskService().set(taskService); task.setSpotlessOutDirectory(source.getOutputDirectory()); return task; } private SpotlessApply createApplyTask(String name, SpotlessTask source) { SpotlessApply task = project.getTasks().create("spotless" + SpotlessPlugin.capitalize(name) + "Apply", SpotlessApply.class); - task.linkSource(this.task); + task.getTaskService().set(taskService); task.setSpotlessOutDirectory(source.getOutputDirectory()); return task; } diff --git a/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/FormatTaskTest.java b/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/FormatTaskTest.java index 91ce4de704..e3e7e01114 100644 --- a/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/FormatTaskTest.java +++ b/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/FormatTaskTest.java @@ -19,6 +19,7 @@ import java.util.Collections; import org.gradle.api.Project; +import org.gradle.api.services.BuildServiceParameters; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; @@ -35,6 +36,12 @@ void createTask() { Project project = TestProvisioner.gradleProject(rootFolder()); spotlessTask = project.getTasks().create("spotlessTaskUnderTest", SpotlessTaskImpl.class); spotlessTask.setLineEndingsPolicy(LineEnding.UNIX.createPolicy()); + spotlessTask.getTaskService().set(new SpotlessTaskService() { + @Override + public BuildServiceParameters.None getParameters() { + return null; + } + }); } @Test 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 f03018c697..35c95facb1 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 @@ -23,6 +23,7 @@ import java.util.Collections; import org.gradle.api.Project; +import org.gradle.api.services.BuildServiceParameters; import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.Test; @@ -38,6 +39,12 @@ class PaddedCellTaskTest extends ResourceHarness { private class Bundle { String name; Project project = TestProvisioner.gradleProject(rootFolder()); + SpotlessTaskService taskService = new SpotlessTaskService() { + @Override + public BuildServiceParameters.None getParameters() { + return null; + } + }; File file; File outputFile; SpotlessTaskImpl task; @@ -56,6 +63,7 @@ private class Bundle { private SpotlessTaskImpl createFormatTask(String name, FormatterStep step) { SpotlessTaskImpl task = project.getTasks().create("spotless" + SpotlessPlugin.capitalize(name), SpotlessTaskImpl.class); + task.getTaskService().set(taskService); task.addStep(step); task.setLineEndingsPolicy(LineEnding.UNIX.createPolicy()); task.setTarget(Collections.singletonList(file)); @@ -64,14 +72,14 @@ private SpotlessTaskImpl createFormatTask(String name, FormatterStep step) { private SpotlessCheck createCheckTask(String name, SpotlessTask source) { SpotlessCheck task = project.getTasks().create("spotless" + SpotlessPlugin.capitalize(name) + "Check", SpotlessCheck.class); - task.source = source; + task.getTaskService().set(taskService); task.setSpotlessOutDirectory(source.getOutputDirectory()); return task; } private SpotlessApply createApplyTask(String name, SpotlessTask source) { SpotlessApply task = project.getTasks().create("spotless" + SpotlessPlugin.capitalize(name) + "Apply", SpotlessApply.class); - task.linkSource(source); + task.getTaskService().set(taskService); task.setSpotlessOutDirectory(source.getOutputDirectory()); return task; } From e393951b4191c0b7cec7d2881a51d6304580a3cf Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Sat, 6 Nov 2021 14:21:10 -0700 Subject: [PATCH 06/24] SpotlessTaskService.Client now includes `Property getSpotlessOutDirectory()` --- .../diffplug/gradle/spotless/FormatExtension.java | 2 +- .../diffplug/gradle/spotless/SpotlessApply.java | 14 +------------- .../diffplug/gradle/spotless/SpotlessCheck.java | 14 +------------- .../gradle/spotless/SpotlessExtensionImpl.java | 4 ++-- .../gradle/spotless/SpotlessTaskService.java | 4 ++++ .../gradle/spotless/DiffMessageFormatterTest.java | 4 ++-- .../gradle/spotless/PaddedCellTaskTest.java | 4 ++-- 7 files changed, 13 insertions(+), 33 deletions(-) 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 3db4b1a750..18ca7f0374 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 @@ -770,7 +770,7 @@ public SpotlessApply createIndependentApplyTask(String taskName) { spotlessTask.mustRunAfter(clean); // create the apply task SpotlessApply applyTask = spotless.project.getTasks().create(taskName, SpotlessApply.class); - applyTask.setSpotlessOutDirectory(spotlessTask.getOutputDirectory()); + applyTask.getSpotlessOutDirectory().set(spotlessTask.getOutputDirectory()); applyTask.getTaskService().set(spotless.getTaskService()); applyTask.dependsOn(spotlessTask); 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 84cb5ad932..4357b21d00 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 @@ -23,25 +23,13 @@ import org.gradle.api.file.ConfigurableFileTree; import org.gradle.api.file.FileVisitDetails; import org.gradle.api.file.FileVisitor; -import org.gradle.api.tasks.Internal; import org.gradle.api.tasks.TaskAction; public abstract class SpotlessApply extends SpotlessTaskService.ClientTask { - private File spotlessOutDirectory; - - @Internal - public File getSpotlessOutDirectory() { - return spotlessOutDirectory; - } - - public void setSpotlessOutDirectory(File spotlessOutDirectory) { - this.spotlessOutDirectory = spotlessOutDirectory; - } - @TaskAction public void performAction() { getTaskService().get().registerApplyAlreadyRan(this); - ConfigurableFileTree files = getProject().fileTree(spotlessOutDirectory); + ConfigurableFileTree files = getProject().fileTree(getSpotlessOutDirectory().get()); if (files.isEmpty()) { getState().setDidWork(getSourceDidWork()); } else { 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 2ea10e7617..5c86e182b3 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 @@ -28,7 +28,6 @@ import org.gradle.api.file.ConfigurableFileTree; import org.gradle.api.file.FileVisitDetails; import org.gradle.api.file.FileVisitor; -import org.gradle.api.tasks.Internal; import org.gradle.api.tasks.TaskAction; import com.diffplug.spotless.FileSignature; @@ -37,17 +36,6 @@ import com.diffplug.spotless.extra.integration.DiffMessageFormatter; public abstract class SpotlessCheck extends SpotlessTaskService.ClientTask { - private File spotlessOutDirectory; - - @Internal - public File getSpotlessOutDirectory() { - return spotlessOutDirectory; - } - - public void setSpotlessOutDirectory(File spotlessOutDirectory) { - this.spotlessOutDirectory = spotlessOutDirectory; - } - public void performActionTest() { performAction(true); } @@ -58,7 +46,7 @@ public void performAction() { } private void performAction(boolean isTest) { - ConfigurableFileTree files = getProject().fileTree(spotlessOutDirectory); + ConfigurableFileTree files = getProject().fileTree(getSpotlessOutDirectory().get()); if (files.isEmpty()) { getState().setDidWork(getApplyDidWork()); } else if (!isTest && applyHasRun()) { 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 61eb121e36..a587cb90c8 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 @@ -97,10 +97,10 @@ protected void createFormatTasks(String name, FormatExtension formatExtension) { // create the check and apply control tasks TaskProvider applyTask = tasks.register(taskName + APPLY, SpotlessApply.class, task -> { + task.getSpotlessOutDirectory().set(spotlessTask.get().getOutputDirectory()); task.getTaskService().set(taskService); task.setEnabled(!isIdeHook); task.dependsOn(spotlessTask); - task.setSpotlessOutDirectory(spotlessTask.get().getOutputDirectory()); }); rootApplyTask.configure(task -> { task.dependsOn(applyTask); @@ -112,10 +112,10 @@ protected void createFormatTasks(String name, FormatExtension formatExtension) { }); TaskProvider checkTask = tasks.register(taskName + CHECK, SpotlessCheck.class, task -> { + task.getSpotlessOutDirectory().set(spotlessTask.get().getOutputDirectory()); task.getTaskService().set(taskService); task.setEnabled(!isIdeHook); task.dependsOn(spotlessTask); - task.setSpotlessOutDirectory(spotlessTask.get().getOutputDirectory()); // if the user runs both, make sure that apply happens first, task.mustRunAfter(applyTask); 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 952e964ff8..da7841a203 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 @@ -15,6 +15,7 @@ */ package com.diffplug.gradle.spotless; +import java.io.File; import java.util.Collections; import java.util.HashMap; import java.util.Map; @@ -50,6 +51,9 @@ public void registerApplyAlreadyRan(SpotlessApply task) { static String INDEPENDENT_HELPER = "Helper"; public static abstract class ClientTask extends DefaultTask { + @Internal + abstract Property getSpotlessOutDirectory(); + @Internal abstract Property getTaskService(); 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 87c93d0bda..e577a6de60 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 @@ -68,15 +68,15 @@ private SpotlessTaskImpl createFormatTask(String name) { private SpotlessCheck createCheckTask(String name, SpotlessTask source) { SpotlessCheck task = project.getTasks().create("spotless" + SpotlessPlugin.capitalize(name) + "Check", SpotlessCheck.class); + task.getSpotlessOutDirectory().set(source.getOutputDirectory()); task.getTaskService().set(taskService); - task.setSpotlessOutDirectory(source.getOutputDirectory()); return task; } private SpotlessApply createApplyTask(String name, SpotlessTask source) { SpotlessApply task = project.getTasks().create("spotless" + SpotlessPlugin.capitalize(name) + "Apply", SpotlessApply.class); + task.getSpotlessOutDirectory().set(source.getOutputDirectory()); task.getTaskService().set(taskService); - task.setSpotlessOutDirectory(source.getOutputDirectory()); 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 35c95facb1..1fabf57d83 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 @@ -73,14 +73,14 @@ private SpotlessTaskImpl createFormatTask(String name, FormatterStep step) { private SpotlessCheck createCheckTask(String name, SpotlessTask source) { SpotlessCheck task = project.getTasks().create("spotless" + SpotlessPlugin.capitalize(name) + "Check", SpotlessCheck.class); task.getTaskService().set(taskService); - task.setSpotlessOutDirectory(source.getOutputDirectory()); + task.getSpotlessOutDirectory().set(source.getOutputDirectory()); return task; } private SpotlessApply createApplyTask(String name, SpotlessTask source) { SpotlessApply task = project.getTasks().create("spotless" + SpotlessPlugin.capitalize(name) + "Apply", SpotlessApply.class); task.getTaskService().set(taskService); - task.setSpotlessOutDirectory(source.getOutputDirectory()); + task.getSpotlessOutDirectory().set(source.getOutputDirectory()); return task; } From 3bb79c887094be1d93363ec0f2eb5d672b6e70db Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Sat, 6 Nov 2021 15:01:37 -0700 Subject: [PATCH 07/24] Oops, spotlessCheck cares about getSourceDidWork(), not apply. --- .../java/com/diffplug/gradle/spotless/SpotlessCheck.java | 2 +- .../diffplug/gradle/spotless/SpotlessTaskService.java | 9 --------- 2 files changed, 1 insertion(+), 10 deletions(-) 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 5c86e182b3..c28010a1cd 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 @@ -48,7 +48,7 @@ public void performAction() { private void performAction(boolean isTest) { ConfigurableFileTree files = getProject().fileTree(getSpotlessOutDirectory().get()); if (files.isEmpty()) { - getState().setDidWork(getApplyDidWork()); + getState().setDidWork(getSourceDidWork()); } else if (!isTest && applyHasRun()) { // if our matching apply has already run, then we don't need to do anything getState().setDidWork(false); 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 da7841a203..ba0231c70e 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 @@ -86,15 +86,6 @@ public boolean getSourceDidWork() { } } - public boolean getApplyDidWork() { - SpotlessApply applyTask = service().apply.get(getSourceTaskPath()); - if (applyTask != null) { - return applyTask.getDidWork(); - } else { - return false; - } - } - public boolean applyHasRun() { return service().apply.containsKey(getSourceTaskPath()); } From bcd40d56ce76195e0cbe72bfccec930ac021b438 Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Sat, 6 Nov 2021 15:07:35 -0700 Subject: [PATCH 08/24] Make RegisterDependenciesTask more focused. --- .../gradle/spotless/RegisterDependenciesTaskTest.java | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/RegisterDependenciesTaskTest.java b/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/RegisterDependenciesTaskTest.java index 1b24201b12..3f5fa30c9b 100644 --- a/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/RegisterDependenciesTaskTest.java +++ b/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/RegisterDependenciesTaskTest.java @@ -45,8 +45,6 @@ void registerDependencies() throws IOException { "> Task :spotlessInternalRegisterDependencies\n") .contains("> Task :sub:spotlessJava\n" + "> Task :sub:spotlessJavaCheck\n" + - "> Task :sub:spotlessCheck\n" + - "\n" + - "BUILD SUCCESSFUL"); + "> Task :sub:spotlessCheck\n"); } } From 84101b9bcd2784c1d527703a632333f743d3b708 Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Sat, 6 Nov 2021 15:29:47 -0700 Subject: [PATCH 09/24] FileSignature now supports `.equals` and `.hashCode` without serialization. --- .../com/diffplug/spotless/FileSignature.java | 29 +++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/lib/src/main/java/com/diffplug/spotless/FileSignature.java b/lib/src/main/java/com/diffplug/spotless/FileSignature.java index 59893f26e6..894d0c2b87 100644 --- a/lib/src/main/java/com/diffplug/spotless/FileSignature.java +++ b/lib/src/main/java/com/diffplug/spotless/FileSignature.java @@ -174,6 +174,20 @@ synchronized Sig sign(File fileInput) throws IOException { } } + @Override + public boolean equals(Object other) { + if (other instanceof FileSignature) { + return Arrays.equals(signatures, ((FileSignature) other).signatures); + } else { + return false; + } + } + + @Override + public int hashCode() { + return Arrays.hashCode(signatures); + } + @SuppressFBWarnings("SE_TRANSIENT_FIELD_NOT_RESTORED") private static final class Sig implements Serializable { private static final long serialVersionUID = 6727302747168655222L; @@ -193,6 +207,21 @@ private static final class Sig implements Serializable { this.hash = hash; this.lastModified = lastModified; } + + @Override + public boolean equals(Object other) { + if (other instanceof Sig) { + Sig o = (Sig) other; + return name.equals(o.name) && size == o.size && Arrays.equals(hash, o.hash); + } else { + return false; + } + } + + @Override + public int hashCode() { + return Arrays.hashCode(hash) | name.hashCode(); + } } /** Asserts that child is a subpath of root. and returns the subpath. */ From dbad65917a4e51faee2d7e2a173bc54ba65ed8e2 Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Sat, 6 Nov 2021 15:32:25 -0700 Subject: [PATCH 10/24] SpotlessTaskSerivce now caches error messages from run to run. --- .../gradle/spotless/SerializableMisc.java | 4 +- .../gradle/spotless/SpotlessCheck.java | 23 +++------ .../gradle/spotless/SpotlessTaskService.java | 51 +++++++++++++++++-- .../GradleIncrementalResolutionTest.java | 36 ++++++++++--- 4 files changed, 85 insertions(+), 29 deletions(-) diff --git a/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SerializableMisc.java b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SerializableMisc.java index 81db8adacc..c4ffbc4837 100644 --- a/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SerializableMisc.java +++ b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SerializableMisc.java @@ -1,5 +1,5 @@ /* - * Copyright 2016 DiffPlug + * Copyright 2016-2021 DiffPlug * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -39,7 +39,7 @@ static void toFile(Serializable obj, File file) { } } - static T fromFile(Class clazz, File file) { + static T fromFile(Class clazz, File file) { try (InputStream input = Files.asByteSource(file).openBufferedStream()) { return fromStream(clazz, input); } catch (IOException e) { 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 c28010a1cd..18fa778e00 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 @@ -31,21 +31,19 @@ import org.gradle.api.tasks.TaskAction; import com.diffplug.spotless.FileSignature; -import com.diffplug.spotless.Formatter; import com.diffplug.spotless.ThrowingEx; -import com.diffplug.spotless.extra.integration.DiffMessageFormatter; public abstract class SpotlessCheck extends SpotlessTaskService.ClientTask { - public void performActionTest() { + public void performActionTest() throws IOException { performAction(true); } @TaskAction - public void performAction() { + public void performAction() throws IOException { performAction(false); } - private void performAction(boolean isTest) { + private void performAction(boolean isTest) throws IOException { ConfigurableFileTree files = getProject().fileTree(getSpotlessOutDirectory().get()); if (files.isEmpty()) { getState().setDidWork(getSourceDidWork()); @@ -93,22 +91,13 @@ public void visitFile(FileVisitDetails fileVisitDetails) { }); if (!problemFiles.isEmpty()) { Collections.sort(problemFiles); - try (Formatter formatter = buildFormatterForCheck()) { - throw formatViolationsFor(formatter, problemFiles); - } + String errMsg = errorMsgForCheck(problemFiles, "Run '" + calculateGradleCommand() + " " + getTaskPathPrefix() + "spotlessApply' to fix these violations."); + throw new GradleException(errMsg); + } } } - /** 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.") - .formatter(formatter) - .problemFiles(problemFiles) - .getMessage()); - } - private String getTaskPathPrefix() { return getProject().getPath().equals(":") ? ":" 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 ba0231c70e..fd3f168ed4 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 @@ -16,8 +16,11 @@ package com.diffplug.gradle.spotless; import java.io.File; +import java.io.IOException; +import java.io.Serializable; import java.util.Collections; import java.util.HashMap; +import java.util.List; import java.util.Map; import org.gradle.api.DefaultTask; @@ -28,7 +31,10 @@ import com.diffplug.common.base.Preconditions; import com.diffplug.common.base.Unhandled; +import com.diffplug.common.collect.Maps; +import com.diffplug.spotless.FileSignature; import com.diffplug.spotless.Formatter; +import com.diffplug.spotless.extra.integration.DiffMessageFormatter; /** * Allows the check and apply tasks to coordinate @@ -41,6 +47,8 @@ public abstract class SpotlessTaskService implements BuildService source = Collections.synchronizedMap(new HashMap<>()); public void registerSourceAlreadyRan(SpotlessTask task) { + // if there's a registered source, we can wipe the cache + cachePut(task.getOutputDirectory(), null, null); source.put(task.getPath(), task); } @@ -90,12 +98,49 @@ public boolean applyHasRun() { return service().apply.containsKey(getSourceTaskPath()); } - public Formatter buildFormatterForCheck() { + String errorMsgForCheck(List problemFiles, String runToFix) throws IOException { SpotlessTask task = service().source.get(getSourceTaskPath()); - if (task == null) { + ((SpotlessCheck) this).getSpotlessOutDirectory(); + if (task != null) { + try (Formatter formatter = task.buildFormatter()) { + String msg = DiffMessageFormatter.builder() + .runToFix(runToFix) + .formatter(formatter) + .problemFiles(problemFiles) + .getMessage(); + cachePut(FileSignature.signAsList(problemFiles), msg); + return msg; + } + } else { + return cacheGet(FileSignature.signAsList(problemFiles)); + } + } + + private void cachePut(FileSignature key, String msg) { + SpotlessTaskService.cachePut(getSpotlessOutDirectory().get(), key, msg); + } + + private String cacheGet(FileSignature key) { + Map.Entry cached = SerializableMisc.fromFile(Map.Entry.class, getCacheFile(getSpotlessOutDirectory().get())); + if (cached != null && cached.getKey().equals(key)) { + return cached.getValue(); + } else { throw new IllegalStateException(getPath() + " is running but " + getSourceTaskPath() + " was up-to-date and didn't run"); } - return task.buildFormatter(); } } + + private static void cachePut(File spotlessOut, FileSignature key, String msg) { + File cacheFile = getCacheFile(spotlessOut); + if (key == null) { + cacheFile.delete(); + } else { + SerializableMisc.toFile((Serializable) Maps.immutableEntry(key, msg), cacheFile); + } + } + + private static File getCacheFile(File spotlessOut) { + File parent = spotlessOut.getParentFile(); + return new File(parent, spotlessOut.getName() + "-errorMsg"); + } } diff --git a/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/GradleIncrementalResolutionTest.java b/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/GradleIncrementalResolutionTest.java index 1de2ef2b4a..9c804feafb 100644 --- a/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/GradleIncrementalResolutionTest.java +++ b/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/GradleIncrementalResolutionTest.java @@ -24,6 +24,9 @@ import java.util.SortedSet; import java.util.TreeSet; +import org.assertj.core.api.AbstractStringAssert; +import org.assertj.core.api.Assertions; +import org.gradle.testkit.runner.BuildResult; import org.junit.jupiter.api.Test; import com.diffplug.common.base.Errors; @@ -53,10 +56,18 @@ void failureDoesntTriggerAll() throws IOException { writeState("aBc"); assertState("aBc"); // check will run against all three the first time. - // Subsequent runs will only run the formatter on the bad file (in order to generate the failure message) checkRanAgainst("abc"); - checkRanAgainst("b"); - checkRanAgainst("b"); + // Subsequent runs will use the cached error message + checkRanAgainstNoneButError().contains("Caused by: org.gradle.api.GradleException: The following files had format violations:\n" + + " b.md\n" + + " @@ -1 +1 @@\n" + + " -B\n" + + " +b"); + checkRanAgainstNoneButError().contains("Caused by: org.gradle.api.GradleException: The following files had format violations:\n" + + " b.md\n" + + " @@ -1 +1 @@\n" + + " -B\n" + + " +b"); // apply will simply copy outputs the first time: no formatters executed applyRanAgainst(""); @@ -69,8 +80,12 @@ void failureDoesntTriggerAll() throws IOException { writeState("Abc"); // then check runs against just the changed file checkRanAgainst("a"); - // even after failing - checkRanAgainst("a"); + // even after failing once the error is still there + checkRanAgainstNoneButError().contains("> The following files had format violations:\n" + + " a.md\n" + + " @@ -1 +1 @@\n" + + " -A\n" + + " +a"); // and so does apply applyRanAgainst(); applyRanAgainst("a"); @@ -112,12 +127,18 @@ private void checkRanAgainst(String... ranAgainst) throws IOException { taskRanAgainst("spotlessCheck", ranAgainst); } - private void taskRanAgainst(String task, String... ranAgainst) throws IOException { + private AbstractStringAssert checkRanAgainstNoneButError() throws IOException { + String console = taskRanAgainst("spotlessCheck"); + return Assertions.assertThat(console); + } + + private String taskRanAgainst(String task, String... ranAgainst) throws IOException { pauseForFilesystem(); String console = StringPrinter.buildString(Errors.rethrow().wrap(printer -> { boolean expectFailure = task.equals("spotlessCheck") && !isClean(); if (expectFailure) { - gradleRunner().withArguments(task).forwardStdOutput(printer.toWriter()).buildAndFail(); + BuildResult b = gradleRunner().withArguments(task, "--stacktrace").forwardStdOutput(printer.toWriter()).forwardStdError(printer.toWriter()).buildAndFail(); + // System.err.println(b.getOutput()); } else { gradleRunner().withArguments(task).forwardStdOutput(printer.toWriter()).build(); } @@ -130,6 +151,7 @@ private void taskRanAgainst(String task, String... ranAgainst) throws IOExceptio } } assertEquals(concat(Arrays.asList(ranAgainst)), concat(added)); + return console; } private String concat(Iterable iterable) { From 93e31026483742a73267ac0204330dce323e2982 Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Sat, 6 Nov 2021 16:28:48 -0700 Subject: [PATCH 11/24] Fix validatePlugins errors. --- .../gradle/spotless/SpotlessApply.java | 2 +- .../gradle/spotless/SpotlessCheck.java | 2 +- .../gradle/spotless/SpotlessTaskService.java | 18 +++++++++--------- 3 files changed, 11 insertions(+), 11 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 4357b21d00..4c4c7a7ed4 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 @@ -31,7 +31,7 @@ public void performAction() { getTaskService().get().registerApplyAlreadyRan(this); ConfigurableFileTree files = getProject().fileTree(getSpotlessOutDirectory().get()); if (files.isEmpty()) { - getState().setDidWork(getSourceDidWork()); + getState().setDidWork(sourceDidWork()); } else { files.visit(new FileVisitor() { @Override 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 18fa778e00..d57f47764c 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 @@ -46,7 +46,7 @@ public void performAction() throws IOException { private void performAction(boolean isTest) throws IOException { ConfigurableFileTree files = getProject().fileTree(getSpotlessOutDirectory().get()); if (files.isEmpty()) { - getState().setDidWork(getSourceDidWork()); + getState().setDidWork(sourceDidWork()); } else if (!isTest && applyHasRun()) { // if our matching apply has already run, then we don't need to do anything getState().setDidWork(false); 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 fd3f168ed4..2a8cd6003a 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 @@ -53,7 +53,7 @@ public void registerSourceAlreadyRan(SpotlessTask task) { } public void registerApplyAlreadyRan(SpotlessApply task) { - apply.put(task.getSourceTaskPath(), task); + apply.put(task.sourceTaskPath(), task); } static String INDEPENDENT_HELPER = "Helper"; @@ -65,7 +65,7 @@ public static abstract class ClientTask extends DefaultTask { @Internal abstract Property getTaskService(); - String getSourceTaskPath() { + String sourceTaskPath() { String path = getPath(); if (this instanceof SpotlessApply) { if (path.endsWith(SpotlessExtension.APPLY)) { @@ -85,8 +85,8 @@ private SpotlessTaskService service() { return getTaskService().get(); } - public boolean getSourceDidWork() { - SpotlessTask sourceTask = service().source.get(getSourceTaskPath()); + protected boolean sourceDidWork() { + SpotlessTask sourceTask = service().source.get(sourceTaskPath()); if (sourceTask != null) { return sourceTask.getDidWork(); } else { @@ -94,12 +94,12 @@ public boolean getSourceDidWork() { } } - public boolean applyHasRun() { - return service().apply.containsKey(getSourceTaskPath()); + protected boolean applyHasRun() { + return service().apply.containsKey(sourceTaskPath()); } - String errorMsgForCheck(List problemFiles, String runToFix) throws IOException { - SpotlessTask task = service().source.get(getSourceTaskPath()); + protected String errorMsgForCheck(List problemFiles, String runToFix) throws IOException { + SpotlessTask task = service().source.get(sourceTaskPath()); ((SpotlessCheck) this).getSpotlessOutDirectory(); if (task != null) { try (Formatter formatter = task.buildFormatter()) { @@ -125,7 +125,7 @@ private String cacheGet(FileSignature key) { if (cached != null && cached.getKey().equals(key)) { return cached.getValue(); } else { - throw new IllegalStateException(getPath() + " is running but " + getSourceTaskPath() + " was up-to-date and didn't run"); + throw new IllegalStateException(getPath() + " is running but " + sourceTaskPath() + " was up-to-date and didn't run"); } } } From 13f70f185d080b0579ddbdf125647c77cd903e5e Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Sat, 6 Nov 2021 15:39:33 -0700 Subject: [PATCH 12/24] Refactor DiffMessageFormatter to prepare for formatting from a folder of clean files. --- .../integration/DiffMessageFormatter.java | 43 +++++++++++++++++-- 1 file changed, 39 insertions(+), 4 deletions(-) diff --git a/lib-extra/src/main/java/com/diffplug/spotless/extra/integration/DiffMessageFormatter.java b/lib-extra/src/main/java/com/diffplug/spotless/extra/integration/DiffMessageFormatter.java index 1551d3113a..9a9ea9954c 100644 --- a/lib-extra/src/main/java/com/diffplug/spotless/extra/integration/DiffMessageFormatter.java +++ b/lib-extra/src/main/java/com/diffplug/spotless/extra/integration/DiffMessageFormatter.java @@ -18,8 +18,10 @@ import java.io.ByteArrayOutputStream; import java.io.File; import java.io.IOException; +import java.nio.charset.Charset; import java.nio.charset.StandardCharsets; import java.nio.file.Files; +import java.nio.file.Path; import java.util.List; import java.util.ListIterator; import java.util.Objects; @@ -44,11 +46,44 @@ public static Builder builder() { return new Builder(); } + interface CleanProvider { + + Path getRootDir(); + + Charset getEncoding(); + + String getFormatted(File file, String rawUnix); + } + + private static class CleanProviderFormatter implements CleanProvider { + private final Formatter formatter; + + CleanProviderFormatter(Formatter formatter) { + this.formatter = Objects.requireNonNull(formatter); + } + + @Override + public Path getRootDir() { + return formatter.getRootDir(); + } + + @Override + public Charset getEncoding() { + return formatter.getEncoding(); + } + + @Override + public String getFormatted(File file, String rawUnix) { + String unix = PaddedCell.check(formatter, file, rawUnix).canonical(); + return formatter.computeLineEndings(unix, file); + } + } + public static class Builder { private Builder() {} private String runToFix; - private Formatter formatter; + private CleanProvider formatter; private List problemFiles; /** "Run 'gradlew spotlessApply' to fix these violations." */ @@ -58,7 +93,7 @@ public Builder runToFix(String runToFix) { } public Builder formatter(Formatter formatter) { - this.formatter = Objects.requireNonNull(formatter); + this.formatter = new CleanProviderFormatter(formatter); return this; } @@ -164,11 +199,11 @@ private void addIntendedLine(String indent, String line) { private static String diff(Builder builder, File file) throws IOException { String raw = new String(Files.readAllBytes(file.toPath()), builder.formatter.getEncoding()); String rawUnix = LineEnding.toUnix(raw); - String formattedUnix = PaddedCell.check(builder.formatter, file, rawUnix).canonical(); + String formatted = builder.formatter.getFormatted(file, rawUnix); + String formattedUnix = LineEnding.toUnix(formatted); if (rawUnix.equals(formattedUnix)) { // the formatting is fine, so it's a line-ending issue - String formatted = builder.formatter.computeLineEndings(formattedUnix, file); return diffWhitespaceLineEndings(raw, formatted, false, true); } else { return diffWhitespaceLineEndings(rawUnix, formattedUnix, true, false); From a3cad7b1b755625b0e5cb8934dfbd8d0045fc5b5 Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Sat, 6 Nov 2021 15:49:46 -0700 Subject: [PATCH 13/24] Add a DiffMessageFormatter which works on a folder of clean files. --- .../integration/DiffMessageFormatter.java | 35 +++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/lib-extra/src/main/java/com/diffplug/spotless/extra/integration/DiffMessageFormatter.java b/lib-extra/src/main/java/com/diffplug/spotless/extra/integration/DiffMessageFormatter.java index 9a9ea9954c..b5d5656198 100644 --- a/lib-extra/src/main/java/com/diffplug/spotless/extra/integration/DiffMessageFormatter.java +++ b/lib-extra/src/main/java/com/diffplug/spotless/extra/integration/DiffMessageFormatter.java @@ -79,6 +79,36 @@ public String getFormatted(File file, String rawUnix) { } } + private static class CleanProviderFolder implements CleanProvider { + private final Path rootDir; + private final Path cleanDir; + private final Charset encoding; + + CleanProviderFolder(Path rootDir, Path cleanDir, String encoding) { + this.rootDir = rootDir; + this.cleanDir = cleanDir; + this.encoding = Charset.forName(encoding); + } + + @Override + public Path getRootDir() { + return rootDir; + } + + @Override + public Charset getEncoding() { + return encoding; + } + + @Override + public String getFormatted(File file, String rawUnix) { + Path relative = rootDir.relativize(file.toPath()); + Path clean = cleanDir.resolve(rootDir.relativize(file.toPath())); + byte[] content = Errors.rethrow().get(() -> Files.readAllBytes(clean)); + return new String(content, encoding); + } + } + public static class Builder { private Builder() {} @@ -97,6 +127,11 @@ public Builder formatter(Formatter formatter) { return this; } + public Builder formatterFolder(Path rootDir, Path cleanDir, String encoding) { + this.formatter = new CleanProviderFolder(rootDir, cleanDir, encoding); + return this; + } + public Builder problemFiles(List problemFiles) { this.problemFiles = Objects.requireNonNull(problemFiles); Preconditions.checkArgument(!problemFiles.isEmpty(), "cannot be empty"); From 36947e6898b80c23444440a1682bdf50e0052365 Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Sat, 6 Nov 2021 16:43:22 -0700 Subject: [PATCH 14/24] Refactor SpotlessCheck to use that formatter so that we can ditch the clumsy cache. --- .../gradle/spotless/SpotlessCheck.java | 13 +++-- .../gradle/spotless/SpotlessTaskService.java | 54 ------------------- 2 files changed, 10 insertions(+), 57 deletions(-) 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 d57f47764c..eaf034e14b 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 @@ -18,6 +18,7 @@ import java.io.ByteArrayOutputStream; import java.io.File; import java.io.IOException; +import java.nio.charset.StandardCharsets; import java.nio.file.Files; import java.util.ArrayList; import java.util.Arrays; @@ -32,6 +33,7 @@ import com.diffplug.spotless.FileSignature; import com.diffplug.spotless.ThrowingEx; +import com.diffplug.spotless.extra.integration.DiffMessageFormatter; public abstract class SpotlessCheck extends SpotlessTaskService.ClientTask { public void performActionTest() throws IOException { @@ -91,9 +93,14 @@ public void visitFile(FileVisitDetails fileVisitDetails) { }); if (!problemFiles.isEmpty()) { Collections.sort(problemFiles); - String errMsg = errorMsgForCheck(problemFiles, "Run '" + calculateGradleCommand() + " " + getTaskPathPrefix() + "spotlessApply' to fix these violations."); - throw new GradleException(errMsg); - + throw new GradleException(DiffMessageFormatter.builder() + .runToFix("Run '" + calculateGradleCommand() + " " + getTaskPathPrefix() + "spotlessApply' to fix these violations.") + .formatterFolder( + getProject().getRootDir().toPath(), + getSpotlessOutDirectory().get().toPath(), + StandardCharsets.UTF_8.name()) + .problemFiles(problemFiles) + .getMessage()); } } } 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 2a8cd6003a..839d704ed3 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 @@ -16,11 +16,8 @@ package com.diffplug.gradle.spotless; import java.io.File; -import java.io.IOException; -import java.io.Serializable; import java.util.Collections; import java.util.HashMap; -import java.util.List; import java.util.Map; import org.gradle.api.DefaultTask; @@ -31,10 +28,6 @@ import com.diffplug.common.base.Preconditions; import com.diffplug.common.base.Unhandled; -import com.diffplug.common.collect.Maps; -import com.diffplug.spotless.FileSignature; -import com.diffplug.spotless.Formatter; -import com.diffplug.spotless.extra.integration.DiffMessageFormatter; /** * Allows the check and apply tasks to coordinate @@ -47,8 +40,6 @@ public abstract class SpotlessTaskService implements BuildService source = Collections.synchronizedMap(new HashMap<>()); public void registerSourceAlreadyRan(SpotlessTask task) { - // if there's a registered source, we can wipe the cache - cachePut(task.getOutputDirectory(), null, null); source.put(task.getPath(), task); } @@ -97,50 +88,5 @@ protected boolean sourceDidWork() { protected boolean applyHasRun() { return service().apply.containsKey(sourceTaskPath()); } - - protected String errorMsgForCheck(List problemFiles, String runToFix) throws IOException { - SpotlessTask task = service().source.get(sourceTaskPath()); - ((SpotlessCheck) this).getSpotlessOutDirectory(); - if (task != null) { - try (Formatter formatter = task.buildFormatter()) { - String msg = DiffMessageFormatter.builder() - .runToFix(runToFix) - .formatter(formatter) - .problemFiles(problemFiles) - .getMessage(); - cachePut(FileSignature.signAsList(problemFiles), msg); - return msg; - } - } else { - return cacheGet(FileSignature.signAsList(problemFiles)); - } - } - - private void cachePut(FileSignature key, String msg) { - SpotlessTaskService.cachePut(getSpotlessOutDirectory().get(), key, msg); - } - - private String cacheGet(FileSignature key) { - Map.Entry cached = SerializableMisc.fromFile(Map.Entry.class, getCacheFile(getSpotlessOutDirectory().get())); - if (cached != null && cached.getKey().equals(key)) { - return cached.getValue(); - } else { - throw new IllegalStateException(getPath() + " is running but " + sourceTaskPath() + " was up-to-date and didn't run"); - } - } - } - - private static void cachePut(File spotlessOut, FileSignature key, String msg) { - File cacheFile = getCacheFile(spotlessOut); - if (key == null) { - cacheFile.delete(); - } else { - SerializableMisc.toFile((Serializable) Maps.immutableEntry(key, msg), cacheFile); - } - } - - private static File getCacheFile(File spotlessOut) { - File parent = spotlessOut.getParentFile(); - return new File(parent, spotlessOut.getName() + "-errorMsg"); } } From 88e6298c35adf8f010f6ca26c1c15182d298cd94 Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Sat, 6 Nov 2021 16:51:24 -0700 Subject: [PATCH 15/24] SpotlessCheck now pipes the actual encoding to the error message rather than just assuming UTF-8. --- .../java/com/diffplug/gradle/spotless/SpotlessCheck.java | 8 ++++++-- .../diffplug/gradle/spotless/SpotlessExtensionImpl.java | 4 +++- .../java/com/diffplug/gradle/spotless/SpotlessTask.java | 2 +- .../gradle/spotless/DiffMessageFormatterTest.java | 2 ++ .../com/diffplug/gradle/spotless/PaddedCellTaskTest.java | 2 ++ 5 files changed, 14 insertions(+), 4 deletions(-) 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 eaf034e14b..eeac49d194 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 @@ -18,7 +18,6 @@ import java.io.ByteArrayOutputStream; import java.io.File; import java.io.IOException; -import java.nio.charset.StandardCharsets; import java.nio.file.Files; import java.util.ArrayList; import java.util.Arrays; @@ -29,6 +28,8 @@ import org.gradle.api.file.ConfigurableFileTree; import org.gradle.api.file.FileVisitDetails; import org.gradle.api.file.FileVisitor; +import org.gradle.api.provider.Property; +import org.gradle.api.tasks.Internal; import org.gradle.api.tasks.TaskAction; import com.diffplug.spotless.FileSignature; @@ -36,6 +37,9 @@ import com.diffplug.spotless.extra.integration.DiffMessageFormatter; public abstract class SpotlessCheck extends SpotlessTaskService.ClientTask { + @Internal + public abstract Property getEncoding(); + public void performActionTest() throws IOException { performAction(true); } @@ -98,7 +102,7 @@ public void visitFile(FileVisitDetails fileVisitDetails) { .formatterFolder( getProject().getRootDir().toPath(), getSpotlessOutDirectory().get().toPath(), - StandardCharsets.UTF_8.name()) + getEncoding().get()) .problemFiles(problemFiles) .getMessage()); } 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 a587cb90c8..cf364a093c 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,8 +112,10 @@ protected void createFormatTasks(String name, FormatExtension formatExtension) { }); TaskProvider checkTask = tasks.register(taskName + CHECK, SpotlessCheck.class, task -> { - task.getSpotlessOutDirectory().set(spotlessTask.get().getOutputDirectory()); + SpotlessTask source = spotlessTask.get(); + task.getSpotlessOutDirectory().set(source.getOutputDirectory()); task.getTaskService().set(taskService); + task.getEncoding().set(source.getEncoding()); task.setEnabled(!isIdeHook); task.dependsOn(spotlessTask); 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 9a08aa9b35..ae80ce9dbd 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 @@ -42,7 +42,7 @@ import com.diffplug.spotless.FormatterStep; import com.diffplug.spotless.LineEnding; -public class SpotlessTask extends DefaultTask { +public abstract class SpotlessTask extends DefaultTask { // set by SpotlessExtension, but possibly overridden by FormatExtension protected String encoding = "UTF-8"; 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 e577a6de60..0df95c7b82 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 @@ -17,6 +17,7 @@ import java.io.File; import java.io.IOException; +import java.nio.charset.StandardCharsets; import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; @@ -70,6 +71,7 @@ private SpotlessCheck createCheckTask(String name, SpotlessTask source) { SpotlessCheck task = project.getTasks().create("spotless" + SpotlessPlugin.capitalize(name) + "Check", SpotlessCheck.class); task.getSpotlessOutDirectory().set(source.getOutputDirectory()); task.getTaskService().set(taskService); + task.getEncoding().set(StandardCharsets.UTF_8.name()); 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 1fabf57d83..45904d8c83 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 @@ -19,6 +19,7 @@ import java.io.File; import java.io.IOException; +import java.nio.charset.StandardCharsets; import java.util.Arrays; import java.util.Collections; @@ -74,6 +75,7 @@ private SpotlessCheck createCheckTask(String name, SpotlessTask source) { SpotlessCheck task = project.getTasks().create("spotless" + SpotlessPlugin.capitalize(name) + "Check", SpotlessCheck.class); task.getTaskService().set(taskService); task.getSpotlessOutDirectory().set(source.getOutputDirectory()); + task.getEncoding().set(StandardCharsets.UTF_8.name()); return task; } From d059972d1f58798541131e4d67ce373c79c94cae Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Sat, 6 Nov 2021 17:08:44 -0700 Subject: [PATCH 16/24] Replace `getProject()` invocations with ObjectFactory and FileSystemOperations where it was easy to do so. --- .../diffplug/gradle/spotless/SpotlessApply.java | 2 +- .../diffplug/gradle/spotless/SpotlessCheck.java | 2 +- .../diffplug/gradle/spotless/SpotlessTaskImpl.java | 14 ++++++++------ .../gradle/spotless/SpotlessTaskService.java | 8 +++++++- 4 files changed, 17 insertions(+), 9 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 4c4c7a7ed4..5c09a70357 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 @@ -29,7 +29,7 @@ public abstract class SpotlessApply extends SpotlessTaskService.ClientTask { @TaskAction public void performAction() { getTaskService().get().registerApplyAlreadyRan(this); - ConfigurableFileTree files = getProject().fileTree(getSpotlessOutDirectory().get()); + ConfigurableFileTree files = getConfigCacheWorkaround().fileTree().from(getSpotlessOutDirectory().get()); if (files.isEmpty()) { getState().setDidWork(sourceDidWork()); } else { 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 eeac49d194..3fa22efd3c 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 @@ -50,7 +50,7 @@ public void performAction() throws IOException { } private void performAction(boolean isTest) throws IOException { - ConfigurableFileTree files = getProject().fileTree(getSpotlessOutDirectory().get()); + ConfigurableFileTree files = getConfigCacheWorkaround().fileTree().from(getSpotlessOutDirectory().get()); if (files.isEmpty()) { getState().setDidWork(sourceDidWork()); } else if (!isTest && applyHasRun()) { 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 1700029023..a80233ef8a 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 @@ -20,9 +20,11 @@ import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.StandardCopyOption; -import java.util.Comparator; + +import javax.inject.Inject; import org.gradle.api.GradleException; +import org.gradle.api.file.FileSystemOperations; import org.gradle.api.provider.Property; import org.gradle.api.tasks.CacheableTask; import org.gradle.api.tasks.Internal; @@ -40,6 +42,9 @@ public abstract class SpotlessTaskImpl extends SpotlessTask { @Internal abstract Property getTaskService(); + @Inject + protected abstract FileSystemOperations getFs(); + @TaskAction public void performAction(InputChanges inputs) throws Exception { getTaskService().get().registerSourceAlreadyRan(this); @@ -49,7 +54,7 @@ public void performAction(InputChanges inputs) throws Exception { if (!inputs.isIncremental()) { getLogger().info("Not incremental: removing prior outputs"); - getProject().delete(outputDirectory); + getFs().delete(d -> d.delete(outputDirectory)); Files.createDirectories(outputDirectory.toPath()); } @@ -96,10 +101,7 @@ private void processInputFile(Formatter formatter, File input) throws IOExceptio private void deletePreviousResult(File input) throws IOException { File output = getOutputFile(input); if (output.isDirectory()) { - Files.walk(output.toPath()) - .sorted(Comparator.reverseOrder()) - .map(Path::toFile) - .forEach(File::delete); + getFs().delete(d -> d.delete(output)); } else { Files.deleteIfExists(output.toPath()); } 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 839d704ed3..ad9bd62bfd 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,7 +20,10 @@ import java.util.HashMap; import java.util.Map; +import javax.inject.Inject; + import org.gradle.api.DefaultTask; +import org.gradle.api.model.ObjectFactory; import org.gradle.api.provider.Property; import org.gradle.api.services.BuildService; import org.gradle.api.services.BuildServiceParameters; @@ -49,13 +52,16 @@ public void registerApplyAlreadyRan(SpotlessApply task) { static String INDEPENDENT_HELPER = "Helper"; - public static abstract class ClientTask extends DefaultTask { + static abstract class ClientTask extends DefaultTask { @Internal abstract Property getSpotlessOutDirectory(); @Internal abstract Property getTaskService(); + @Inject + protected abstract ObjectFactory getConfigCacheWorkaround(); + String sourceTaskPath() { String path = getPath(); if (this instanceof SpotlessApply) { From e0b850eeb55903c06e0813e6f2b474a452be9732 Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Sat, 6 Nov 2021 17:21:02 -0700 Subject: [PATCH 17/24] Use SpotlessTaskService.init to consolidate configuration-avoidance fixups. --- .../gradle/spotless/FormatExtension.java | 3 +- .../spotless/SpotlessExtensionImpl.java | 8 ++---- .../gradle/spotless/SpotlessTaskService.java | 5 ++++ .../spotless/DiffMessageFormatterTest.java | 10 +++---- .../gradle/spotless/PaddedCellTaskTest.java | 28 +++++++++---------- 5 files changed, 26 insertions(+), 28 deletions(-) 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 18ca7f0374..0891d50b85 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 @@ -770,8 +770,7 @@ public SpotlessApply createIndependentApplyTask(String taskName) { spotlessTask.mustRunAfter(clean); // create the apply task SpotlessApply applyTask = spotless.project.getTasks().create(taskName, SpotlessApply.class); - applyTask.getSpotlessOutDirectory().set(spotlessTask.getOutputDirectory()); - applyTask.getTaskService().set(spotless.getTaskService()); + applyTask.init(spotlessTask); applyTask.dependsOn(spotlessTask); return applyTask; 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 cf364a093c..fc646d0a68 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 @@ -97,8 +97,7 @@ protected void createFormatTasks(String name, FormatExtension formatExtension) { // create the check and apply control tasks TaskProvider applyTask = tasks.register(taskName + APPLY, SpotlessApply.class, task -> { - task.getSpotlessOutDirectory().set(spotlessTask.get().getOutputDirectory()); - task.getTaskService().set(taskService); + task.init(spotlessTask.get()); task.setEnabled(!isIdeHook); task.dependsOn(spotlessTask); }); @@ -112,9 +111,8 @@ protected void createFormatTasks(String name, FormatExtension formatExtension) { }); TaskProvider checkTask = tasks.register(taskName + CHECK, SpotlessCheck.class, task -> { - SpotlessTask source = spotlessTask.get(); - task.getSpotlessOutDirectory().set(source.getOutputDirectory()); - task.getTaskService().set(taskService); + SpotlessTaskImpl source = spotlessTask.get(); + task.init(spotlessTask.get()); task.getEncoding().set(source.getEncoding()); task.setEnabled(!isIdeHook); task.dependsOn(spotlessTask); 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 ad9bd62bfd..7ea954fbb3 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 @@ -62,6 +62,11 @@ static abstract class ClientTask extends DefaultTask { @Inject protected abstract ObjectFactory getConfigCacheWorkaround(); + void init(SpotlessTaskImpl impl) { + getSpotlessOutDirectory().set(impl.getOutputDirectory()); + getTaskService().set(impl.getTaskService()); + } + String sourceTaskPath() { String path = getPath(); if (this instanceof SpotlessApply) { 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 0df95c7b82..cd574601c0 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 @@ -67,18 +67,16 @@ 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.getSpotlessOutDirectory().set(source.getOutputDirectory()); - task.getTaskService().set(taskService); + task.init(source); task.getEncoding().set(StandardCharsets.UTF_8.name()); 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.getSpotlessOutDirectory().set(source.getOutputDirectory()); - task.getTaskService().set(taskService); + task.init(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 45904d8c83..6a4cf56084 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 @@ -48,7 +48,7 @@ public BuildServiceParameters.None getParameters() { }; File file; File outputFile; - SpotlessTaskImpl task; + SpotlessTaskImpl source; SpotlessCheck check; SpotlessApply apply; @@ -56,10 +56,10 @@ public BuildServiceParameters.None getParameters() { this.name = name; file = setFile("src/test." + name).toContent("CCC"); FormatterStep step = FormatterStep.createNeverUpToDate(name, function); - task = createFormatTask(name, step); - check = createCheckTask(name, task); - apply = createApplyTask(name, task); - outputFile = new File(task.getOutputDirectory() + "/src", file.getName()); + source = createFormatTask(name, step); + check = createCheckTask(name, source); + apply = createApplyTask(name, source); + outputFile = new File(source.getOutputDirectory() + "/src", file.getName()); } private SpotlessTaskImpl createFormatTask(String name, FormatterStep step) { @@ -71,18 +71,16 @@ 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.getTaskService().set(taskService); - task.getSpotlessOutDirectory().set(source.getOutputDirectory()); + task.init(source); task.getEncoding().set(StandardCharsets.UTF_8.name()); 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.getTaskService().set(taskService); - task.getSpotlessOutDirectory().set(source.getOutputDirectory()); + task.init(source); return task; } @@ -97,21 +95,21 @@ String checkFailureMsg() { void diagnose() throws IOException { SpotlessDiagnoseTask diagnose = project.getTasks().create("spotless" + SpotlessPlugin.capitalize(name) + "Diagnose", SpotlessDiagnoseTask.class); - diagnose.source = task; + diagnose.source = source; diagnose.performAction(); } void format() throws Exception { - Tasks.execute(task); + Tasks.execute(source); } void apply() throws Exception { - Tasks.execute(task); + Tasks.execute(source); apply.performAction(); } void check() throws Exception { - Tasks.execute(task); + Tasks.execute(source); check.performActionTest(); } } From ff9bb5f463a42ffbad4eeb7dc3687e7dde199c76 Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Sat, 6 Nov 2021 18:01:53 -0700 Subject: [PATCH 18/24] Add SpotlessTaskImpl.init to simplify that task as well. --- .../diffplug/gradle/spotless/FormatExtension.java | 2 +- .../gradle/spotless/SpotlessExtensionImpl.java | 2 +- .../diffplug/gradle/spotless/SpotlessTaskImpl.java | 14 ++++++++++++-- .../gradle/spotless/DiffMessageFormatterTest.java | 7 ++++--- .../diffplug/gradle/spotless/FormatTaskTest.java | 4 ++-- .../gradle/spotless/GradleIntegrationHarness.java | 5 +++++ .../gradle/spotless/PaddedCellTaskTest.java | 7 ++++--- 7 files changed, 29 insertions(+), 12 deletions(-) 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 0891d50b85..5c7b20b82f 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 @@ -763,7 +763,7 @@ public SpotlessApply createIndependentApplyTask(String taskName) { Preconditions.checkArgument(!taskName.endsWith(SpotlessExtension.APPLY), "Task name must not end with " + SpotlessExtension.APPLY); // create and setup the task SpotlessTaskImpl spotlessTask = spotless.project.getTasks().create(taskName + SpotlessTaskService.INDEPENDENT_HELPER, SpotlessTaskImpl.class); - spotlessTask.getTaskService().set(spotless.getTaskService()); + spotlessTask.init(spotless.getTaskService()); setupTask(spotlessTask); // enforce the clean ordering Task clean = spotless.project.getTasks().getByName(BasePlugin.CLEAN_TASK_NAME); 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 fc646d0a68..42b2e37fa6 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 @@ -78,7 +78,7 @@ protected void createFormatTasks(String name, FormatExtension formatExtension) { // create the SpotlessTask String taskName = EXTENSION + SpotlessPlugin.capitalize(name); TaskProvider spotlessTask = tasks.register(taskName, SpotlessTaskImpl.class, task -> { - task.getTaskService().set(taskService); + task.init(taskService); task.setEnabled(!isIdeHook); // clean removes the SpotlessCache, so we have to run after clean task.mustRunAfter(cleanTask); 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 a80233ef8a..838d9f7983 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 @@ -24,8 +24,10 @@ import javax.inject.Inject; 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.Internal; import org.gradle.api.tasks.TaskAction; @@ -40,14 +42,22 @@ @CacheableTask public abstract class SpotlessTaskImpl extends SpotlessTask { @Internal - abstract Property getTaskService(); + abstract Property getTakService(); + + @Internal + abstract DirectoryProperty getProjectDir(); + + void init(Provider service) { + getTakService().set(service); + getProjectDir().set(getProject().getProjectDir()); + } @Inject protected abstract FileSystemOperations getFs(); @TaskAction public void performAction(InputChanges inputs) throws Exception { - getTaskService().get().registerSourceAlreadyRan(this); + getTakService().get().registerSourceAlreadyRan(this); if (target == null) { throw new GradleException("You must specify 'Iterable target'"); } 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 cd574601c0..e5898a1ca2 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 @@ -26,6 +26,7 @@ import org.assertj.core.api.Assertions; import org.gradle.api.Project; +import org.gradle.api.provider.Provider; import org.gradle.api.services.BuildServiceParameters; import org.junit.jupiter.api.Test; @@ -41,12 +42,12 @@ class DiffMessageFormatterTest extends ResourceHarness { private class Bundle { Project project = TestProvisioner.gradleProject(rootFolder()); - SpotlessTaskService taskService = new SpotlessTaskService() { + Provider taskService = GradleIntegrationHarness.providerOf(new SpotlessTaskService() { @Override public BuildServiceParameters.None getParameters() { return null; } - }; + }); File file; SpotlessTaskImpl task; @@ -61,7 +62,7 @@ public BuildServiceParameters.None getParameters() { private SpotlessTaskImpl createFormatTask(String name) { SpotlessTaskImpl task = project.getTasks().create("spotless" + SpotlessPlugin.capitalize(name), SpotlessTaskImpl.class); - task.getTaskService().set(taskService); + task.init(taskService); task.setLineEndingsPolicy(LineEnding.UNIX.createPolicy()); task.setTarget(Collections.singletonList(file)); return task; diff --git a/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/FormatTaskTest.java b/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/FormatTaskTest.java index e3e7e01114..8985cd7a14 100644 --- a/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/FormatTaskTest.java +++ b/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/FormatTaskTest.java @@ -36,12 +36,12 @@ void createTask() { Project project = TestProvisioner.gradleProject(rootFolder()); spotlessTask = project.getTasks().create("spotlessTaskUnderTest", SpotlessTaskImpl.class); spotlessTask.setLineEndingsPolicy(LineEnding.UNIX.createPolicy()); - spotlessTask.getTaskService().set(new SpotlessTaskService() { + spotlessTask.init(GradleIntegrationHarness.providerOf(new SpotlessTaskService() { @Override public BuildServiceParameters.None getParameters() { return null; } - }); + })); } @Test diff --git a/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/GradleIntegrationHarness.java b/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/GradleIntegrationHarness.java index 2d92f27eff..89f7f25ffe 100644 --- a/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/GradleIntegrationHarness.java +++ b/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/GradleIntegrationHarness.java @@ -24,6 +24,7 @@ import java.util.function.Predicate; import java.util.stream.Collectors; +import org.gradle.api.provider.Provider; import org.gradle.testkit.runner.BuildResult; import org.gradle.testkit.runner.BuildTask; import org.gradle.testkit.runner.GradleRunner; @@ -59,6 +60,10 @@ public enum GradleVersionSupport { } } + public static Provider providerOf(T value) { + return org.gradle.api.internal.provider.Providers.of(value); + } + /** * Each test gets its own temp folder, and we create a gradle * build there and run it. 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 6a4cf56084..1114acb580 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 @@ -24,6 +24,7 @@ import java.util.Collections; import org.gradle.api.Project; +import org.gradle.api.provider.Provider; import org.gradle.api.services.BuildServiceParameters; import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.Test; @@ -40,12 +41,12 @@ class PaddedCellTaskTest extends ResourceHarness { private class Bundle { String name; Project project = TestProvisioner.gradleProject(rootFolder()); - SpotlessTaskService taskService = new SpotlessTaskService() { + Provider taskService = GradleIntegrationHarness.providerOf(new SpotlessTaskService() { @Override public BuildServiceParameters.None getParameters() { return null; } - }; + }); File file; File outputFile; SpotlessTaskImpl source; @@ -64,7 +65,7 @@ public BuildServiceParameters.None getParameters() { private SpotlessTaskImpl createFormatTask(String name, FormatterStep step) { SpotlessTaskImpl task = project.getTasks().create("spotless" + SpotlessPlugin.capitalize(name), SpotlessTaskImpl.class); - task.getTaskService().set(taskService); + task.init(taskService); task.addStep(step); task.setLineEndingsPolicy(LineEnding.UNIX.createPolicy()); task.setTarget(Collections.singletonList(file)); From cf4bdd1bae2a8d722c7a23a67cbd4c44d4fd131b Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Sat, 6 Nov 2021 18:10:56 -0700 Subject: [PATCH 19/24] We should definitely remove unused imports before yelling about internal deps. --- gradle/spotless.gradle | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gradle/spotless.gradle b/gradle/spotless.gradle index 1c59e64afa..96037b21a3 100644 --- a/gradle/spotless.gradle +++ b/gradle/spotless.gradle @@ -16,13 +16,13 @@ spotless { // the rootProject doesn't have any java java { ratchetFrom 'origin/main' - custom 'noInternalDeps', noInternalDepsClosure bumpThisNumberIfACustomStepChanges(1) licenseHeaderFile rootProject.file('gradle/spotless.license') importOrderFile rootProject.file('gradle/spotless.importorder') eclipse().configFile rootProject.file('gradle/spotless.eclipseformat.xml') trimTrailingWhitespace() removeUnusedImports() + custom 'noInternalDeps', noInternalDepsClosure } } groovyGradle { From 998f188a2cbf989613be36ab1230ea50db48aeae Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Sat, 6 Nov 2021 18:15:09 -0700 Subject: [PATCH 20/24] Convert GitRatchetGradle to be File-based, which allows us to remove all `getProject()` except IdeHook. --- .../gradle/spotless/GitRatchetGradle.java | 11 +++++------ .../com/diffplug/gradle/spotless/IdeHook.java | 6 +++--- .../gradle/spotless/SpotlessApply.java | 2 +- .../gradle/spotless/SpotlessCheck.java | 18 +++++++++++++----- .../diffplug/gradle/spotless/SpotlessTask.java | 11 ++++++++--- .../gradle/spotless/SpotlessTaskImpl.java | 11 ++++++----- .../gradle/spotless/SpotlessTaskService.java | 7 ++++++- 7 files changed, 42 insertions(+), 24 deletions(-) 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 5a7f402b89..7517018d66 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,7 +19,6 @@ import javax.annotation.Nullable; -import org.gradle.api.Project; import org.gradle.api.services.BuildService; import org.gradle.api.services.BuildServiceParameters; import org.gradle.tooling.events.FinishEvent; @@ -28,15 +27,15 @@ import com.diffplug.spotless.extra.GitRatchet; /** Gradle implementation of GitRatchet. */ -public abstract class GitRatchetGradle extends GitRatchet implements BuildService, OperationCompletionListener { +public abstract class GitRatchetGradle extends GitRatchet implements BuildService, OperationCompletionListener { @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(); } @Override 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..02f60c21e6 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 @@ -1,5 +1,5 @@ /* - * Copyright 2016-2020 DiffPlug + * Copyright 2016-2021 DiffPlug * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -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().get().getAsFile(), 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 5c09a70357..a85195fe9f 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 @@ -42,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(getProjectDir().get().getAsFile(), 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 3fa22efd3c..42c91c4c96 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 @@ -67,7 +67,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(getProjectDir().get().getAsFile(), path); try { // read the file on disk byte[] userFile = Files.readAllBytes(originalSource.toPath()); @@ -100,7 +100,7 @@ public void visitFile(FileVisitDetails fileVisitDetails) { throw new GradleException(DiffMessageFormatter.builder() .runToFix("Run '" + calculateGradleCommand() + " " + getTaskPathPrefix() + "spotlessApply' to fix these violations.") .formatterFolder( - getProject().getRootDir().toPath(), + getProjectDir().get().getAsFile().toPath(), getSpotlessOutDirectory().get().toPath(), getEncoding().get()) .problemFiles(problemFiles) @@ -109,10 +109,18 @@ public void visitFile(FileVisitDetails fileVisitDetails) { } } + @Internal + abstract Property getProjectPath(); + + @Override + void init(SpotlessTaskImpl impl) { + super.init(impl); + getProjectPath().set(getProject().getPath()); + } + private String getTaskPathPrefix() { - return getProject().getPath().equals(":") - ? ":" - : getProject().getPath() + ":"; + String path = getProjectPath().get(); + return path.equals(":") ? ":" : path + ":"; } private static String calculateGradleCommand() { 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 ae80ce9dbd..8c997153ec 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 @@ -27,6 +27,7 @@ import org.eclipse.jgit.lib.ObjectId; import org.gradle.api.DefaultTask; +import org.gradle.api.file.DirectoryProperty; import org.gradle.api.file.FileCollection; import org.gradle.api.tasks.Input; import org.gradle.api.tasks.InputFiles; @@ -80,10 +81,14 @@ 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); + File projectDir = getProjectDir().get().getAsFile(); + rootTreeSha = gitRatchet.rootTreeShaOf(projectDir, ratchetFrom); + subtreeSha = gitRatchet.subtreeShaOf(projectDir, rootTreeSha); } + @Internal + abstract DirectoryProperty getProjectDir(); + @Internal GitRatchetGradle getRatchet() { return ratchet; @@ -163,7 +168,7 @@ Formatter buildFormatter() { return Formatter.builder() .lineEndingsPolicy(lineEndingsPolicy) .encoding(Charset.forName(encoding)) - .rootDir(getProject().getRootDir().toPath()) + .rootDir(getProjectDir().get().getAsFile().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 838d9f7983..957dbdf0c4 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 @@ -86,7 +86,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(getProjectDir().get().getAsFile(), rootTreeSha, input)) { dirtyState = PaddedCell.isClean(); } else { dirtyState = PaddedCell.calculateDirtyState(formatter, input); @@ -118,12 +118,13 @@ private void deletePreviousResult(File input) throws IOException { } private File getOutputFile(File input) { - String outputFileName = FormatExtension.relativize(getProject().getProjectDir(), input); + File projectDir = getProjectDir().get().getAsFile(); + 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(" target: " + input.getAbsolutePath()); + printer.println("Spotless error! All target files must be within the project root."); + printer.println(" project dir: " + projectDir.getAbsolutePath()); + printer.println(" target: " + input.getAbsolutePath()); })); } return new File(outputDirectory, outputFileName); 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 7ea954fbb3..88afbebb0b 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 @@ -23,6 +23,7 @@ import javax.inject.Inject; import org.gradle.api.DefaultTask; +import org.gradle.api.file.DirectoryProperty; import org.gradle.api.model.ObjectFactory; import org.gradle.api.provider.Property; import org.gradle.api.services.BuildService; @@ -59,12 +60,16 @@ static abstract class ClientTask extends DefaultTask { @Internal abstract Property getTaskService(); + @Internal + abstract DirectoryProperty getProjectDir(); + @Inject protected abstract ObjectFactory getConfigCacheWorkaround(); void init(SpotlessTaskImpl impl) { getSpotlessOutDirectory().set(impl.getOutputDirectory()); - getTaskService().set(impl.getTaskService()); + getTaskService().set(impl.getTakService()); + getProjectDir().set(impl.getProjectDir()); } String sourceTaskPath() { From c8d13b18c60ca0f3b3b5715dc2f424891a902a5a Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Sat, 6 Nov 2021 19:36:47 -0700 Subject: [PATCH 21/24] MIscellaneous cleanup. --- .../java/com/diffplug/gradle/spotless/SpotlessCheck.java | 1 + .../diffplug/gradle/spotless/SpotlessExtensionImpl.java | 5 ++--- .../com/diffplug/gradle/spotless/SpotlessTaskImpl.java | 8 ++++---- .../com/diffplug/gradle/spotless/SpotlessTaskService.java | 2 +- .../gradle/spotless/DiffMessageFormatterTest.java | 2 -- .../gradle/spotless/GradleIncrementalResolutionTest.java | 4 +--- .../com/diffplug/gradle/spotless/PaddedCellTaskTest.java | 2 -- 7 files changed, 9 insertions(+), 15 deletions(-) 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 42c91c4c96..55336a00ad 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 @@ -116,6 +116,7 @@ public void visitFile(FileVisitDetails fileVisitDetails) { void init(SpotlessTaskImpl impl) { super.init(impl); getProjectPath().set(getProject().getPath()); + getEncoding().set(impl.getEncoding()); } private String getTaskPathPrefix() { 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 42b2e37fa6..1a2a66422a 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,10 +112,9 @@ protected void createFormatTasks(String name, FormatExtension formatExtension) { TaskProvider checkTask = tasks.register(taskName + CHECK, SpotlessCheck.class, task -> { SpotlessTaskImpl source = spotlessTask.get(); - task.init(spotlessTask.get()); - task.getEncoding().set(source.getEncoding()); + task.init(source); task.setEnabled(!isIdeHook); - task.dependsOn(spotlessTask); + task.dependsOn(source); // if the user runs both, make sure that apply happens first, task.mustRunAfter(applyTask); 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 957dbdf0c4..2fc37fd741 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 @@ -42,13 +42,13 @@ @CacheableTask public abstract class SpotlessTaskImpl extends SpotlessTask { @Internal - abstract Property getTakService(); + abstract Property getTaskService(); @Internal abstract DirectoryProperty getProjectDir(); void init(Provider service) { - getTakService().set(service); + getTaskService().set(service); getProjectDir().set(getProject().getProjectDir()); } @@ -57,7 +57,7 @@ void init(Provider service) { @TaskAction public void performAction(InputChanges inputs) throws Exception { - getTakService().get().registerSourceAlreadyRan(this); + getTaskService().get().registerSourceAlreadyRan(this); if (target == null) { throw new GradleException("You must specify 'Iterable target'"); } @@ -122,7 +122,7 @@ private File getOutputFile(File 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."); + printer.println("Spotless error! All target files must be within the project dir."); printer.println(" project dir: " + projectDir.getAbsolutePath()); printer.println(" target: " + input.getAbsolutePath()); })); 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 88afbebb0b..a08e93bd12 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 @@ -68,7 +68,7 @@ static abstract class ClientTask extends DefaultTask { void init(SpotlessTaskImpl impl) { getSpotlessOutDirectory().set(impl.getOutputDirectory()); - getTaskService().set(impl.getTakService()); + getTaskService().set(impl.getTaskService()); getProjectDir().set(impl.getProjectDir()); } 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 e5898a1ca2..c46d398bf8 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 @@ -17,7 +17,6 @@ import java.io.File; import java.io.IOException; -import java.nio.charset.StandardCharsets; import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; @@ -71,7 +70,6 @@ private SpotlessTaskImpl createFormatTask(String name) { private SpotlessCheck createCheckTask(String name, SpotlessTaskImpl source) { SpotlessCheck task = project.getTasks().create("spotless" + SpotlessPlugin.capitalize(name) + "Check", SpotlessCheck.class); task.init(source); - task.getEncoding().set(StandardCharsets.UTF_8.name()); return task; } diff --git a/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/GradleIncrementalResolutionTest.java b/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/GradleIncrementalResolutionTest.java index 9c804feafb..865c7c0387 100644 --- a/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/GradleIncrementalResolutionTest.java +++ b/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/GradleIncrementalResolutionTest.java @@ -26,7 +26,6 @@ import org.assertj.core.api.AbstractStringAssert; import org.assertj.core.api.Assertions; -import org.gradle.testkit.runner.BuildResult; import org.junit.jupiter.api.Test; import com.diffplug.common.base.Errors; @@ -137,8 +136,7 @@ private String taskRanAgainst(String task, String... ranAgainst) throws IOExcept String console = StringPrinter.buildString(Errors.rethrow().wrap(printer -> { boolean expectFailure = task.equals("spotlessCheck") && !isClean(); if (expectFailure) { - BuildResult b = gradleRunner().withArguments(task, "--stacktrace").forwardStdOutput(printer.toWriter()).forwardStdError(printer.toWriter()).buildAndFail(); - // System.err.println(b.getOutput()); + gradleRunner().withArguments(task).forwardStdOutput(printer.toWriter()).forwardStdError(printer.toWriter()).buildAndFail(); } else { gradleRunner().withArguments(task).forwardStdOutput(printer.toWriter()).build(); } 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 1114acb580..bc7a2fdc96 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 @@ -19,7 +19,6 @@ import java.io.File; import java.io.IOException; -import java.nio.charset.StandardCharsets; import java.util.Arrays; import java.util.Collections; @@ -75,7 +74,6 @@ private SpotlessTaskImpl createFormatTask(String name, FormatterStep step) { private SpotlessCheck createCheckTask(String name, SpotlessTaskImpl source) { SpotlessCheck task = project.getTasks().create("spotless" + SpotlessPlugin.capitalize(name) + "Check", SpotlessCheck.class); task.init(source); - task.getEncoding().set(StandardCharsets.UTF_8.name()); return task; } From 4af94ec79d6871f39682d5bfadacb835143db218 Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Sat, 6 Nov 2021 19:40:20 -0700 Subject: [PATCH 22/24] Another small fix. --- .../GradleIncrementalResolutionTest.java | 21 +++++++++---------- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/GradleIncrementalResolutionTest.java b/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/GradleIncrementalResolutionTest.java index 865c7c0387..6235c246bd 100644 --- a/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/GradleIncrementalResolutionTest.java +++ b/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/GradleIncrementalResolutionTest.java @@ -57,17 +57,16 @@ void failureDoesntTriggerAll() throws IOException { // check will run against all three the first time. checkRanAgainst("abc"); // Subsequent runs will use the cached error message - checkRanAgainstNoneButError().contains("Caused by: org.gradle.api.GradleException: The following files had format violations:\n" + - " b.md\n" + - " @@ -1 +1 @@\n" + - " -B\n" + - " +b"); - checkRanAgainstNoneButError().contains("Caused by: org.gradle.api.GradleException: The following files had format violations:\n" + - " b.md\n" + - " @@ -1 +1 @@\n" + - " -B\n" + - " +b"); - + checkRanAgainstNoneButError().contains("> The following files had format violations:\n" + + " b.md\n" + + " @@ -1 +1 @@\n" + + " -B\n" + + " +b"); + checkRanAgainstNoneButError().contains("> The following files had format violations:\n" + + " b.md\n" + + " @@ -1 +1 @@\n" + + " -B\n" + + " +b"); // apply will simply copy outputs the first time: no formatters executed applyRanAgainst(""); // the second time, it will only run on the file that was changed by apply From 959688146d054d6c44c363ea2d460ef0888712c7 Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Sat, 6 Nov 2021 20:25:36 -0700 Subject: [PATCH 23/24] (Unneeded change) Revert "FileSignature now supports `.equals` and `.hashCode` without serialization." This reverts commit 84101b9bcd2784c1d527703a632333f743d3b708. --- .../com/diffplug/spotless/FileSignature.java | 29 ------------------- 1 file changed, 29 deletions(-) diff --git a/lib/src/main/java/com/diffplug/spotless/FileSignature.java b/lib/src/main/java/com/diffplug/spotless/FileSignature.java index 894d0c2b87..59893f26e6 100644 --- a/lib/src/main/java/com/diffplug/spotless/FileSignature.java +++ b/lib/src/main/java/com/diffplug/spotless/FileSignature.java @@ -174,20 +174,6 @@ synchronized Sig sign(File fileInput) throws IOException { } } - @Override - public boolean equals(Object other) { - if (other instanceof FileSignature) { - return Arrays.equals(signatures, ((FileSignature) other).signatures); - } else { - return false; - } - } - - @Override - public int hashCode() { - return Arrays.hashCode(signatures); - } - @SuppressFBWarnings("SE_TRANSIENT_FIELD_NOT_RESTORED") private static final class Sig implements Serializable { private static final long serialVersionUID = 6727302747168655222L; @@ -207,21 +193,6 @@ private static final class Sig implements Serializable { this.hash = hash; this.lastModified = lastModified; } - - @Override - public boolean equals(Object other) { - if (other instanceof Sig) { - Sig o = (Sig) other; - return name.equals(o.name) && size == o.size && Arrays.equals(hash, o.hash); - } else { - return false; - } - } - - @Override - public int hashCode() { - return Arrays.hashCode(hash) | name.hashCode(); - } } /** Asserts that child is a subpath of root. and returns the subpath. */ From eb8db284371d358267bd3f10f7094af5c68dd76a Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Mon, 8 Nov 2021 19:08:49 -0800 Subject: [PATCH 24/24] Update the lib changelog. --- CHANGES.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGES.md b/CHANGES.md index 40d40e1934..7773f2052c 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -10,6 +10,8 @@ This document is intended for Spotless developers. We adhere to the [keepachangelog](https://keepachangelog.com/en/1.0.0/) format (starting after version `1.27.0`). ## [Unreleased] +### Added +* `DiffMessageFormatter` can now generate messages based on a folder of cleaned files, as an alternative to a `Formatter` ([#982](https://github.com/diffplug/spotless/pull/982)). ## [2.19.2] - 2021-10-26 ### Changed