Skip to content

Commit 2ca2107

Browse files
authored
Merge pull request #463 from diffplug/feature/no-eager-config
Dont break config avoidance for other tasks
2 parents 9713eda + e97a580 commit 2ca2107

File tree

9 files changed

+228
-94
lines changed

9 files changed

+228
-94
lines changed

plugin-gradle/CHANGES.md

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,9 @@
22

33
### Version 3.25.0-SNAPSHOT - TBD ([javadoc](https://diffplug.github.io/spotless/javadoc/snapshot/), [snapshot](https://oss.sonatype.org/content/repositories/snapshots/com/diffplug/spotless/spotless-plugin-gradle/))
44

5+
* Spotless no longer breaks configuration avoidance for other tasks (specifically the `check` task and all of its dependees) ([#463](https://github.com/diffplug/spotless/pull/463)).
6+
* Important change: **Formerly, Spotless did not create its tasks until the `afterEvaluate` phase. Spotless now creates them as soon as the plugin is applied**, and it creates the format-specific tasks as soon as the formats are defined. There is no performance degradation associated with this change, and it makes configuring Spotless easier.
7+
58
### Version 3.24.3 - September 23rd 2019 ([javadoc](https://diffplug.github.io/spotless/javadoc/spotless-plugin-gradle/3.24.3/), [jcenter](https://bintray.com/diffplug/opensource/spotless-plugin-gradle/3.24.3))
69

710
* Update jgit from `5.3.2.201906051522-r` to `5.5.0.201909110433-r`. ([#445](https://github.com/diffplug/spotless/pull/445))

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

Lines changed: 78 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -26,15 +26,38 @@
2626
import org.gradle.api.Action;
2727
import org.gradle.api.GradleException;
2828
import org.gradle.api.Project;
29+
import org.gradle.api.Task;
30+
import org.gradle.api.execution.TaskExecutionGraph;
31+
import org.gradle.api.plugins.BasePlugin;
2932

3033
import com.diffplug.common.base.Errors;
3134
import com.diffplug.spotless.LineEnding;
3235

36+
import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
37+
import groovy.lang.Closure;
38+
3339
public class SpotlessExtension {
3440
final Project project;
41+
final Task rootCheckTask, rootApplyTask;
42+
43+
static final String EXTENSION = "spotless";
44+
static final String CHECK = "Check";
45+
static final String APPLY = "Apply";
46+
47+
private static final String TASK_GROUP = "Verification";
48+
private static final String CHECK_DESCRIPTION = "Checks that sourcecode satisfies formatting steps.";
49+
private static final String APPLY_DESCRIPTION = "Applies code formatting steps to sourcecode in-place.";
50+
51+
private static final String FILES_PROPERTY = "spotlessFiles";
3552

3653
public SpotlessExtension(Project project) {
3754
this.project = requireNonNull(project);
55+
rootCheckTask = project.task(EXTENSION + CHECK);
56+
rootCheckTask.setGroup(TASK_GROUP);
57+
rootCheckTask.setDescription(CHECK_DESCRIPTION);
58+
rootApplyTask = project.task(EXTENSION + APPLY);
59+
rootApplyTask.setGroup(TASK_GROUP);
60+
rootApplyTask.setDescription(APPLY_DESCRIPTION);
3861
}
3962

4063
/** Line endings (if any). */
@@ -201,14 +224,66 @@ private <T extends FormatExtension> T maybeCreate(String name, Class<T> clazz) {
201224
} else {
202225
try {
203226
Constructor<T> constructor = clazz.getConstructor(SpotlessExtension.class);
204-
T newlyCreated = constructor.newInstance(this);
205-
formats.put(name, newlyCreated);
206-
return newlyCreated;
227+
T formatExtension = constructor.newInstance(this);
228+
formats.put(name, formatExtension);
229+
createFormatTask(name, formatExtension);
230+
return formatExtension;
207231
} catch (NoSuchMethodException e) {
208232
throw new GradleException("Must have a constructor " + clazz.getSimpleName() + "(SpotlessExtension root)", e);
209233
} catch (Exception e) {
210234
throw Errors.asRuntime(e);
211235
}
212236
}
213237
}
238+
239+
@SuppressWarnings("rawtypes")
240+
private void createFormatTask(String name, FormatExtension formatExtension) {
241+
// create the SpotlessTask
242+
String taskName = EXTENSION + SpotlessPlugin.capitalize(name);
243+
SpotlessTask spotlessTask = project.getTasks().create(taskName, SpotlessTask.class);
244+
project.afterEvaluate(unused -> formatExtension.setupTask(spotlessTask));
245+
246+
// clean removes the SpotlessCache, so we have to run after clean
247+
Task clean = project.getTasks().getByName(BasePlugin.CLEAN_TASK_NAME);
248+
spotlessTask.mustRunAfter(clean);
249+
250+
// create the check and apply control tasks
251+
Task checkTask = project.getTasks().create(taskName + CHECK);
252+
Task applyTask = project.getTasks().create(taskName + APPLY);
253+
254+
checkTask.dependsOn(spotlessTask);
255+
applyTask.dependsOn(spotlessTask);
256+
// when the task graph is ready, we'll configure the spotlessTask appropriately
257+
project.getGradle().getTaskGraph().whenReady(new Closure(null) {
258+
private static final long serialVersionUID = 1L;
259+
260+
// called by gradle
261+
@SuppressFBWarnings("UMAC_UNCALLABLE_METHOD_OF_ANONYMOUS_CLASS")
262+
public Object doCall(TaskExecutionGraph graph) {
263+
if (graph.hasTask(checkTask)) {
264+
spotlessTask.setCheck();
265+
}
266+
if (graph.hasTask(applyTask)) {
267+
spotlessTask.setApply();
268+
}
269+
return Closure.DONE;
270+
}
271+
});
272+
273+
// set the filePatterns property
274+
project.afterEvaluate(unused -> {
275+
String filePatterns;
276+
if (project.hasProperty(FILES_PROPERTY) && project.property(FILES_PROPERTY) instanceof String) {
277+
filePatterns = (String) project.property(FILES_PROPERTY);
278+
} else {
279+
// needs to be non-null since it is an @Input property of the task
280+
filePatterns = "";
281+
}
282+
spotlessTask.setFilePatterns(filePatterns);
283+
});
284+
285+
// the root tasks depend on the control tasks
286+
rootCheckTask.dependsOn(checkTask);
287+
rootApplyTask.dependsOn(applyTask);
288+
}
214289
}

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

Lines changed: 18 additions & 85 deletions
Original file line numberDiff line numberDiff line change
@@ -18,112 +18,45 @@
1818
import org.gradle.api.Plugin;
1919
import org.gradle.api.Project;
2020
import org.gradle.api.Task;
21-
import org.gradle.api.execution.TaskExecutionGraph;
2221
import org.gradle.api.plugins.BasePlugin;
23-
import org.gradle.api.plugins.JavaBasePlugin;
22+
import org.gradle.util.GradleVersion;
2423

2524
import com.diffplug.spotless.SpotlessCache;
2625

27-
import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
28-
import groovy.lang.Closure;
29-
3026
public class SpotlessPlugin implements Plugin<Project> {
3127
SpotlessExtension spotlessExtension;
3228

33-
static final String EXTENSION = "spotless";
34-
static final String CHECK = "Check";
35-
static final String APPLY = "Apply";
36-
37-
private static final String TASK_GROUP = "Verification";
38-
private static final String CHECK_DESCRIPTION = "Checks that sourcecode satisfies formatting steps.";
39-
private static final String APPLY_DESCRIPTION = "Applies code formatting steps to sourcecode in-place.";
40-
private static final String FILES_PROPERTY = "spotlessFiles";
41-
4229
@Override
4330
public void apply(Project project) {
4431
// make sure there's a `clean` task
4532
project.getPlugins().apply(BasePlugin.class);
4633

4734
// setup the extension
48-
spotlessExtension = project.getExtensions().create(EXTENSION, SpotlessExtension.class, project);
35+
spotlessExtension = project.getExtensions().create(SpotlessExtension.EXTENSION, SpotlessExtension.class, project);
36+
37+
// clear spotless' cache when the user does a clean
38+
Task clean = project.getTasks().getByName(BasePlugin.CLEAN_TASK_NAME);
39+
clean.doLast(unused -> SpotlessCache.clear());
4940

50-
// after the project has been evaluated, configure the check and format tasks per source set
51-
project.afterEvaluate(this::createTasks);
41+
project.afterEvaluate(unused -> {
42+
// Add our check task as a dependency on the global check task
43+
// getTasks() returns a "live" collection, so this works even if the
44+
// task doesn't exist at the time this call is made
45+
if (spotlessExtension.enforceCheck) {
46+
if (GradleVersion.current().compareTo(SpotlessPluginLegacy.CONFIG_AVOIDANCE_INTRODUCED) >= 0) {
47+
SpotlessPluginConfigAvoidance.enforceCheck(spotlessExtension, project);
48+
} else {
49+
SpotlessPluginLegacy.enforceCheck(spotlessExtension, project);
50+
}
51+
}
52+
});
5253
}
5354

5455
/** The extension for this plugin. */
5556
public SpotlessExtension getExtension() {
5657
return spotlessExtension;
5758
}
5859

59-
@SuppressWarnings("rawtypes")
60-
void createTasks(Project project) {
61-
Task rootCheckTask = project.task(EXTENSION + CHECK);
62-
rootCheckTask.setGroup(TASK_GROUP);
63-
rootCheckTask.setDescription(CHECK_DESCRIPTION);
64-
Task rootApplyTask = project.task(EXTENSION + APPLY);
65-
rootApplyTask.setGroup(TASK_GROUP);
66-
rootApplyTask.setDescription(APPLY_DESCRIPTION);
67-
String filePatterns;
68-
if (project.hasProperty(FILES_PROPERTY) && project.property(FILES_PROPERTY) instanceof String) {
69-
filePatterns = (String) project.property(FILES_PROPERTY);
70-
} else {
71-
// needs to be non-null since it is an @Input property of the task
72-
filePatterns = "";
73-
}
74-
75-
spotlessExtension.formats.forEach((key, value) -> {
76-
// create the task that does the work
77-
String taskName = EXTENSION + capitalize(key);
78-
SpotlessTask spotlessTask = project.getTasks().create(taskName, SpotlessTask.class);
79-
value.setupTask(spotlessTask);
80-
spotlessTask.setFilePatterns(filePatterns);
81-
82-
// create the check and apply control tasks
83-
Task checkTask = project.getTasks().create(taskName + CHECK);
84-
Task applyTask = project.getTasks().create(taskName + APPLY);
85-
// the root tasks depend on them
86-
rootCheckTask.dependsOn(checkTask);
87-
rootApplyTask.dependsOn(applyTask);
88-
// and they depend on the work task
89-
checkTask.dependsOn(spotlessTask);
90-
applyTask.dependsOn(spotlessTask);
91-
92-
// when the task graph is ready, we'll configure the spotlessTask appropriately
93-
project.getGradle().getTaskGraph().whenReady(new Closure(null) {
94-
private static final long serialVersionUID = 1L;
95-
96-
// called by gradle
97-
@SuppressFBWarnings("UMAC_UNCALLABLE_METHOD_OF_ANONYMOUS_CLASS")
98-
public Object doCall(TaskExecutionGraph graph) {
99-
if (graph.hasTask(checkTask)) {
100-
spotlessTask.setCheck();
101-
}
102-
if (graph.hasTask(applyTask)) {
103-
spotlessTask.setApply();
104-
}
105-
return Closure.DONE;
106-
}
107-
});
108-
});
109-
110-
// Add our check task as a dependency on the global check task
111-
// getTasks() returns a "live" collection, so this works even if the
112-
// task doesn't exist at the time this call is made
113-
if (spotlessExtension.enforceCheck) {
114-
project.getTasks()
115-
.matching(task -> task.getName().equals(JavaBasePlugin.CHECK_TASK_NAME))
116-
.all(task -> task.dependsOn(rootCheckTask));
117-
}
118-
119-
// clear spotless' cache when the user does a clean, but only after any spotless tasks
120-
Task clean = project.getTasks().getByName(BasePlugin.CLEAN_TASK_NAME);
121-
clean.doLast(unused -> SpotlessCache.clear());
122-
project.getTasks()
123-
.withType(SpotlessTask.class)
124-
.all(task -> task.mustRunAfter(clean));
125-
}
126-
12760
static String capitalize(String input) {
12861
return Character.toUpperCase(input.charAt(0)) + input.substring(1);
12962
}
Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
/*
2+
* Copyright 2016 DiffPlug
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
package com.diffplug.gradle.spotless;
17+
18+
import org.gradle.api.Project;
19+
import org.gradle.api.Task;
20+
import org.gradle.api.plugins.JavaBasePlugin;
21+
import org.gradle.api.tasks.TaskProvider;
22+
23+
class SpotlessPluginConfigAvoidance {
24+
static void enforceCheck(SpotlessExtension extension, Project project) {
25+
TaskProvider<Task> check = project.getTasks().named(JavaBasePlugin.CHECK_TASK_NAME);
26+
check.configure(task -> task.dependsOn(extension.rootCheckTask));
27+
}
28+
}
Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
/*
2+
* Copyright 2016 DiffPlug
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
package com.diffplug.gradle.spotless;
17+
18+
import org.gradle.api.Project;
19+
import org.gradle.api.Task;
20+
import org.gradle.api.plugins.JavaBasePlugin;
21+
import org.gradle.util.GradleVersion;
22+
23+
class SpotlessPluginLegacy {
24+
static final GradleVersion CONFIG_AVOIDANCE_INTRODUCED = GradleVersion.version("4.9");
25+
26+
static void enforceCheck(SpotlessExtension extension, Project project) {
27+
Task check = project.getTasks().getByName(JavaBasePlugin.CHECK_TASK_NAME);
28+
check.dependsOn(extension.rootCheckTask);
29+
}
30+
}

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

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -171,8 +171,8 @@ public void setApply() {
171171
/** Returns the name of this format. */
172172
String formatName() {
173173
String name = getName();
174-
if (name.startsWith(SpotlessPlugin.EXTENSION)) {
175-
return name.substring(SpotlessPlugin.EXTENSION.length()).toLowerCase(Locale.ROOT);
174+
if (name.startsWith(SpotlessExtension.EXTENSION)) {
175+
return name.substring(SpotlessExtension.EXTENSION.length()).toLowerCase(Locale.ROOT);
176176
} else {
177177
return name;
178178
}
@@ -181,10 +181,10 @@ String formatName() {
181181
@TaskAction
182182
public void performAction(IncrementalTaskInputs inputs) throws Exception {
183183
if (target == null) {
184-
throw new GradleException("You must specify 'Iterable<File> toFormat'");
184+
throw new GradleException("You must specify 'Iterable<File> target'");
185185
}
186186
if (!check && !apply) {
187-
throw new GradleException("Don't call " + getName() + " directly, call " + getName() + SpotlessPlugin.CHECK + " or " + getName() + SpotlessPlugin.APPLY);
187+
throw new GradleException("Don't call " + getName() + " directly, call " + getName() + SpotlessExtension.CHECK + " or " + getName() + SpotlessExtension.APPLY);
188188
}
189189

190190
Predicate<File> shouldInclude;
Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,64 @@
1+
/*
2+
* Copyright 2016 DiffPlug
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
package com.diffplug.gradle.spotless;
17+
18+
import java.io.IOException;
19+
20+
import org.assertj.core.api.Assertions;
21+
import org.gradle.testkit.runner.GradleRunner;
22+
import org.junit.Test;
23+
24+
public class ConfigAvoidanceTest extends GradleIntegrationTest {
25+
protected final GradleRunner gradleRunnerConfigAvoidance() throws IOException {
26+
return gradleRunner().withGradleVersion(SpotlessPluginLegacy.CONFIG_AVOIDANCE_INTRODUCED.getVersion());
27+
}
28+
29+
@Test
30+
public void noConfigOnHelp() throws IOException {
31+
setFile("build.gradle").toLines(
32+
"buildscript { repositories { mavenCentral() } }",
33+
"plugins {",
34+
" id 'com.diffplug.gradle.spotless'",
35+
"}",
36+
"apply plugin: 'java'",
37+
"spotless {",
38+
" java {",
39+
" googleJavaFormat('1.2')",
40+
" }",
41+
"}",
42+
"",
43+
"class ConfigureCanary extends DefaultTask {",
44+
" ConfigureCanary() {",
45+
" println('Canary was configured')",
46+
" }",
47+
"",
48+
" @TaskAction",
49+
" def action() {",
50+
" println('Canary ran')",
51+
" }",
52+
"}",
53+
"def canary = tasks.register('canary', ConfigureCanary) {}",
54+
"tasks.named('check').configure {",
55+
" dependsOn(canary)",
56+
"}");
57+
setFile("src/main/java/test.java").toResource("java/googlejavaformat/JavaCodeUnformatted.test");
58+
59+
String help_4_9 = gradleRunnerConfigAvoidance().withArguments("help").build().getOutput();
60+
Assertions.assertThat(help_4_9).doesNotContain("Canary was configured");
61+
String check_4_9 = gradleRunnerConfigAvoidance().withArguments("check").buildAndFail().getOutput();
62+
Assertions.assertThat(check_4_9).contains("Canary was configured", "Canary ran", "Execution failed for task ':spotlessJava'");
63+
}
64+
}

0 commit comments

Comments
 (0)