Add support for predeclared dependencies. (#1039)

.circleci/config.yml

@@ -108,14 +108,14 @@ jobs:
       - *restore_cache_wrapper
       - *restore_cache_deps
       - run:
-          name: gradlew npmTest
-          command: export SPOTLESS_EXCLUDE_MAVEN=true && ./gradlew npmTest --build-cache
+          name: gradlew testNpm
+          command: export SPOTLESS_EXCLUDE_MAVEN=true && ./gradlew testNpm --build-cache
       - store_test_results:
-          path: testlib/build/test-results/NpmTest
+          path: testlib/build/test-results/testNpm
       - store_test_results:
-          path: plugin-maven/build/test-results/NpmTest
+          path: plugin-maven/build/test-results/testNpm
       - store_test_results:
-          path: plugin-gradle/build/test-results/NpmTest
+          path: plugin-gradle/build/test-results/testNpm
       - run:
           name: gradlew test
           command: export SPOTLESS_EXCLUDE_MAVEN=true && ./gradlew test --build-cache
@@ -141,12 +141,12 @@ jobs:
       - store_test_results:
           path: plugin-gradle/build/test-results/test
       - run:
-          name: gradlew npmTest
-          command: gradlew npmTest --build-cache -PSPOTLESS_EXCLUDE_MAVEN=true
+          name: gradlew testNpm
+          command: gradlew testNpm --build-cache -PSPOTLESS_EXCLUDE_MAVEN=true
       - store_test_results:
-          path: testlib/build/test-results/NpmTest
+          path: testlib/build/test-results/testNpm
       - store_test_results:
-          path: plugin-gradle/build/test-results/NpmTest
+          path: plugin-gradle/build/test-results/testNpm
   changelog_print:
     << : *env_gradle
     steps:

gradle/special-tests.gradle

@@ -1,5 +1,5 @@
 apply plugin: 'org.gradle.test-retry'
-
+apply plugin: 'com.adarshr.test-logger'
 def special = [
 	'Npm',
 	'Black',
@@ -21,7 +21,7 @@ tasks.named('test') {
 }
 
 special.forEach { tag ->
-	tasks.register("${tag}Test", Test) {
+	tasks.register("test${tag}", Test) {
 		useJUnitPlatform { includeTags tag }
 	}
 }

plugin-gradle/CHANGES.md

@@ -3,6 +3,23 @@
 We adhere to the [keepachangelog](https://keepachangelog.com/en/1.0.0/) format (starting after version `3.27.0`).
 
 ## [Unreleased]
+### Added
+* You can now predeclare formatter dependencies in the root project.
+  * specify one of:
+  * `spotless { predeclareDeps() }` to resolve all deps from the root project, which will show up in dependency reports.
+  * `spotless { predeclareDepsFromBuildscript() }` to resolve all deps from `buildscript { repositories {`, which will not show up in dependency reports ([see #1027](https://github.com/diffplug/spotless/issues/1027)).
+  * and then below that you have a block where you simply declare each formatter which you are using, e.g.
+  * ```
+    spotless {
+      ...
+      predeclareDepsFromBuildscript()
+    }
+    spotlessPredeclare {
+      java { eclipse() }
+      kotlin { ktfmt('0.28') }
+    }
+    ```
+  * By default, Spotless resolves all dependencies per-project, and the predeclaration above is unnecessary in the vast majority of cases.
 
 ## [6.0.5] - 2021-12-16
 ### Fixed
@@ -42,7 +59,7 @@ We adhere to the [keepachangelog](https://keepachangelog.com/en/1.0.0/) format (
   * To make this daemon-restriction obsolete, please see and upvote [#987](https://github.com/diffplug/spotless/issues/987).
 ### 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 each project with a `spotless {...}` block. This means that you can remove the `buildscript {}` block, but you still need a `repositories { mavenCentral() }` (or similar) in each project which is using Spotless. ([#980](https://github.com/diffplug/spotless/pull/980), [#983](https://github.com/diffplug/spotless/pull/983))
-  * If you prefer the old behavior, we are open to adding that back as a new feature, see [#984 predeclared dependencies](https://github.com/diffplug/spotless/issues/984).
+  * If you prefer the old behavior, it is available via [`predeclareDepsFromBuildscript()` starting in `6.1.0`](../README.md#dependency-resolution-modes).
 * **BREAKING** `createIndepentApplyTask(String taskName)` now requires that `taskName` does not end with `Apply`
 * Bump minimum required Gradle from `6.1` to `6.1.1`.
 * Bump default formatter versions ([#989](https://github.com/diffplug/spotless/pull/989))

plugin-gradle/README.md

@@ -86,6 +86,7 @@ Spotless supports all of Gradle's built-in performance features (incremental bui
   - [Multiple (or custom) language-specific blocks](#multiple-or-custom-language-specific-blocks)
   - [Inception (languages within languages within...)](#inception-languages-within-languages-within)
   - [Disabling warnings and error messages](#disabling-warnings-and-error-messages)
+  - [Dependency resolution modes](#dependency-resolution-modes)
   - [How do I preview what `spotlessApply` will do?](#how-do-i-preview-what-spotlessapply-will-do)
   - [Example configurations (from real-world projects)](#example-configurations-from-real-world-projects)
 
@@ -910,6 +911,26 @@ spotless {
     ignoreErrorForPath('path/to/file.java') // ignore errors by all steps on this specific file
 ```
 
+<a name="dependency-resolution-modes"></a>
+## Dependency resolution modes
+
+By default, Spotless resolves dependencies on a per-project basis. For very large parallel builds, this can sometimes cause problems. As an alternative, Spotless can be configured to resolve all dependencies in the root project like so:
+
+```gradle
+spotless {
+  ...
+  predeclareDeps()
+}
+spotlessPredeclare {
+  java { eclipse() }
+  kotlin { ktfmt('0.28') }
+}
+```
+
+Alternatively, you can also use `predeclareDepsFromBuildscript()` to resolve the dependencies from the buildscript repositories rather than the project repositories.
+
+If you use this feature, you will get an error if you use a formatter in a subproject which is not declared in the `spotlessPredeclare` block.
+
 <a name="preview"></a>
 
 ## How do I preview what `spotlessApply` will do?

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

@@ -73,7 +73,7 @@ public FormatExtension(SpotlessExtension spotless) {
 	}
 
 	protected final Provisioner provisioner() {
-		return spotless.getRegisterDependenciesTask().getTaskService().get().provisionerFor(spotless.project);
+		return spotless.getRegisterDependenciesTask().getTaskService().get().provisionerFor(spotless);
 	}
 
 	private String formatName() {
@@ -757,16 +757,6 @@ protected void setupTask(SpotlessTask task) {
 		} else {
 			steps = this.steps;
 		}
-		// <IMPORTANT>
-		// By calling .hashCode, we are triggering all steps to evaluate their state,
-		// which triggers dependency resolution. It's important to do that here, because
-		// otherwise it won't happen until Gradle starts checking for task up-to-date-ness.
-		// For a large parallel build, the task up-to-dateness might get called on a different
-		// thread than the thread where task configuration happens, which will trigger a
-		// java.util.ConcurrentModificationException
-		// See https://github.com/diffplug/spotless/issues/1015 for details.
-		steps.hashCode();
-		// </IMPORTANT>
 		task.setSteps(steps);
 		task.setLineEndingsPolicy(getLineEndings().createPolicy(getProject().getProjectDir(), () -> totalTarget));
 		spotless.getRegisterDependenciesTask().hookSubprojectTask(task);

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

@@ -19,61 +19,102 @@
 import java.util.Collection;
 import java.util.HashMap;
 import java.util.Map;
-import java.util.Objects;
 import java.util.Set;
 import java.util.logging.Level;
 import java.util.logging.Logger;
 
+import org.gradle.api.GradleException;
 import org.gradle.api.Project;
 import org.gradle.api.artifacts.Configuration;
+import org.gradle.api.artifacts.ConfigurationContainer;
+import org.gradle.api.artifacts.dsl.DependencyHandler;
 import org.gradle.api.attributes.Bundling;
+import org.gradle.api.initialization.dsl.ScriptHandler;
 
+import com.diffplug.common.base.Unhandled;
 import com.diffplug.common.collect.ImmutableList;
 import com.diffplug.spotless.Provisioner;
 
 /** Should be package-private. */
 class GradleProvisioner {
 	private GradleProvisioner() {}
 
-	static Provisioner newDedupingProvisioner(Project project) {
-		return new DedupingProvisioner(project);
+	enum Policy {
+		INDEPENDENT, ROOT_PROJECT, ROOT_BUILDSCRIPT;
+
+		public DedupingProvisioner dedupingProvisioner(Project project) {
+			switch (this) {
+			case ROOT_PROJECT:
+				return new DedupingProvisioner(forProject(project));
+			case ROOT_BUILDSCRIPT:
+				return new DedupingProvisioner(forRootProjectBuildscript(project));
+			case INDEPENDENT:
+			default:
+				throw Unhandled.enumException(this);
+			}
+		}
 	}
 
 	static class DedupingProvisioner implements Provisioner {
-		private final Project project;
+		private final Provisioner provisioner;
 		private final Map<Request, Set<File>> cache = new HashMap<>();
 
-		DedupingProvisioner(Project project) {
-			this.project = project;
+		DedupingProvisioner(Provisioner provisioner) {
+			this.provisioner = provisioner;
 		}
 
 		@Override
 		public Set<File> provisionWithTransitives(boolean withTransitives, Collection<String> mavenCoordinates) {
 			Request req = new Request(withTransitives, mavenCoordinates);
-			Set<File> result = cache.get(req);
+			Set<File> result;
+			synchronized (cache) {
+				result = cache.get(req);
+			}
 			if (result != null) {
 				return result;
 			} else {
-				result = cache.get(req);
-				if (result != null) {
-					return result;
-				} else {
-					result = forProject(project).provisionWithTransitives(req.withTransitives, req.mavenCoords);
-					cache.put(req, result);
+				synchronized (cache) {
+					result = cache.get(req);
+					if (result == null) {
+						result = provisioner.provisionWithTransitives(req.withTransitives, req.mavenCoords);
+						cache.put(req, result);
+					}
 					return result;
 				}
 			}
 		}
+
+		/** A child Provisioner which retries cached elements only. */
+		final Provisioner cachedOnly = (withTransitives, mavenCoordinates) -> {
+			Request req = new Request(withTransitives, mavenCoordinates);
+			Set<File> result;
+			synchronized (cache) {
+				result = cache.get(req);
+			}
+			if (result != null) {
+				return result;
+			}
+			throw new GradleException("Add a step with " + req.mavenCoords + " into the `spotlessPredeclare` block in the root project.");
+		};
+	}
+
+	static Provisioner forProject(Project project) {
+		return forConfigurationContainer(project, project.getConfigurations(), project.getDependencies());
+	}
+
+	static Provisioner forRootProjectBuildscript(Project project) {
+		Project rootProject = project.getRootProject();
+		ScriptHandler buildscript = rootProject.getBuildscript();
+		return forConfigurationContainer(rootProject, buildscript.getConfigurations(), buildscript.getDependencies());
 	}
 
-	private static Provisioner forProject(Project project) {
-		Objects.requireNonNull(project);
+	private static Provisioner forConfigurationContainer(Project project, ConfigurationContainer configurations, DependencyHandler dependencies) {
 		return (withTransitives, mavenCoords) -> {
 			try {
-				Configuration config = project.getConfigurations().create("spotless"
+				Configuration config = configurations.create("spotless"
 						+ new Request(withTransitives, mavenCoords).hashCode());
 				mavenCoords.stream()
-						.map(project.getDependencies()::create)
+						.map(dependencies::create)
 						.forEach(config.getDependencies()::add);
 				config.setDescription(mavenCoords.toString());
 				config.setTransitive(withTransitives);

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

@@ -15,16 +15,26 @@
  */
 package com.diffplug.gradle.spotless;
 
+import java.io.File;
+import java.io.IOException;
+import java.nio.charset.StandardCharsets;
+import java.util.ArrayList;
+import java.util.List;
+
 import javax.inject.Inject;
 
 import org.gradle.api.DefaultTask;
 import org.gradle.api.provider.Property;
 import org.gradle.api.services.BuildServiceRegistry;
+import org.gradle.api.tasks.Input;
 import org.gradle.api.tasks.Internal;
+import org.gradle.api.tasks.OutputFile;
 import org.gradle.api.tasks.TaskAction;
 import org.gradle.build.event.BuildEventsListenerRegistry;
 
 import com.diffplug.common.base.Preconditions;
+import com.diffplug.common.io.Files;
+import com.diffplug.spotless.FormatterStep;
 
 /**
  * NOT AN END-USER TASK, DO NOT USE FOR ANYTHING!
@@ -40,11 +50,10 @@ public abstract class RegisterDependenciesTask extends DefaultTask {
 	static final String TASK_NAME = "spotlessInternalRegisterDependencies";
 
 	void hookSubprojectTask(SpotlessTask task) {
-		// TODO: in the future, we might use this hook to implement #984
-		// spotlessSetup {
-		//    java { googleJavaFormat('1.2') }
-		//    ...etc
-		// }
+		// this ensures that if a user is using predeclared dependencies,
+		// those predeclared deps will be resolved before they are needed
+		// by the child tasks
+		//
 		// it's also needed to make sure that jvmLocalCache gets set
 		// in the SpotlessTaskService before any spotless tasks run
 		task.dependsOn(this);
@@ -56,11 +65,27 @@ void setup() {
 		BuildServiceRegistry buildServices = getProject().getGradle().getSharedServices();
 		getTaskService().set(buildServices.registerIfAbsent("SpotlessTaskService" + compositeBuildSuffix, SpotlessTaskService.class, spec -> {}));
 		getBuildEventsListenerRegistry().onTaskCompletion(getTaskService());
+		unitOutput = new File(getProject().getBuildDir(), "tmp/spotless-register-dependencies");
+	}
+
+	List<FormatterStep> steps = new ArrayList<>();
+
+	@Input
+	public List<FormatterStep> getSteps() {
+		return steps;
+	}
+
+	File unitOutput;
+
+	@OutputFile
+	public File getUnitOutput() {
+		return unitOutput;
 	}
 
 	@TaskAction
-	public void trivialFunction() {
-		// nothing to do :)
+	public void trivialFunction() throws IOException {
+		Files.createParentDirs(unitOutput);
+		Files.write(Integer.toString(1), unitOutput, StandardCharsets.UTF_8);
 	}
 
 	@Internal