Skip to content

Commit

Permalink
Make IDE hook work with configuration cache, take 2 (#2308, revises #…
Browse files Browse the repository at this point in the history
…2278, fixes #1082)
  • Loading branch information
nedtwigg authored Oct 24, 2024
2 parents 7feb013 + f80e923 commit 236f480
Show file tree
Hide file tree
Showing 5 changed files with 135 additions and 37 deletions.
37 changes: 37 additions & 0 deletions lib/src/main/java/com/diffplug/spotless/TestingOnly.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
/*
* Copyright 2024 DiffPlug
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package com.diffplug.spotless;

import java.util.Locale;

/** These FormatterStep are meant to be used for testing only. */
public class TestingOnly {
public static FormatterStep lowercase() {
return FormatterStep.create("lowercase", "lowercaseStateUnused", unused -> TestingOnly::lowercase);
}

private static String lowercase(String raw) {
return raw.toLowerCase(Locale.ROOT);
}

public static FormatterStep diverge() {
return FormatterStep.create("diverge", "divergeStateUnused", unused -> TestingOnly::diverge);
}

private static String diverge(String raw) {
return raw + " ";
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -19,12 +19,34 @@
import java.io.IOException;
import java.nio.file.Files;

import javax.annotation.Nullable;

import org.gradle.api.Project;

import com.diffplug.common.base.Errors;
import com.diffplug.common.io.ByteStreams;
import com.diffplug.spotless.DirtyState;
import com.diffplug.spotless.Formatter;
import com.diffplug.spotless.NoLambda;

class IdeHook {
static class State extends NoLambda.EqualityBasedOnSerialization {
final @Nullable String path;
final boolean useStdIn;
final boolean useStdOut;

State(Project project) {
path = (String) project.findProperty(PROPERTY);
if (path != null) {
useStdIn = project.hasProperty(USE_STD_IN);
useStdOut = project.hasProperty(USE_STD_OUT);
} else {
useStdIn = false;
useStdOut = false;
}
}
}

final static String PROPERTY = "spotlessIdeHook";
final static String USE_STD_IN = "spotlessIdeHookUseStdIn";
final static String USE_STD_OUT = "spotlessIdeHookUseStdOut";
Expand All @@ -33,9 +55,8 @@ private static void dumpIsClean() {
System.err.println("IS CLEAN");
}

static void performHook(SpotlessTaskImpl spotlessTask) {
String path = (String) spotlessTask.getProject().property(PROPERTY);
File file = new File(path);
static void performHook(SpotlessTaskImpl spotlessTask, IdeHook.State state) {
File file = new File(state.path);
if (!file.isAbsolute()) {
System.err.println("Argument passed to " + PROPERTY + " must be an absolute path");
return;
Expand All @@ -50,7 +71,7 @@ static void performHook(SpotlessTaskImpl spotlessTask) {
}
}
byte[] bytes;
if (spotlessTask.getProject().hasProperty(USE_STD_IN)) {
if (state.useStdIn) {
bytes = ByteStreams.toByteArray(System.in);
} else {
bytes = Files.readAllBytes(file.toPath());
Expand All @@ -63,7 +84,7 @@ static void performHook(SpotlessTaskImpl spotlessTask) {
System.err.println("Run 'spotlessDiagnose' for details https://github.com/diffplug/spotless/blob/main/PADDEDCELL.md");
} else {
System.err.println("IS DIRTY");
if (spotlessTask.getProject().hasProperty(USE_STD_OUT)) {
if (state.useStdOut) {
dirty.writeCanonicalTo(System.out);
} else {
dirty.writeCanonicalTo(file);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2016-2022 DiffPlug
* Copyright 2016-2024 DiffPlug
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -48,15 +48,15 @@ public SpotlessExtensionImpl(Project project) {

@Override
protected void createFormatTasks(String name, FormatExtension formatExtension) {
boolean isIdeHook = project.hasProperty(IdeHook.PROPERTY);
IdeHook.State ideHook = new IdeHook.State(project);
TaskContainer tasks = project.getTasks();

// create the SpotlessTask
String taskName = EXTENSION + SpotlessPlugin.capitalize(name);
TaskProvider<SpotlessTaskImpl> spotlessTask = tasks.register(taskName, SpotlessTaskImpl.class, task -> {
task.init(getRegisterDependenciesTask().getTaskService());
task.setGroup(TASK_GROUP);
task.setEnabled(!isIdeHook);
task.getIdeHookState().set(ideHook);
// clean removes the SpotlessCache, so we have to run after clean
task.mustRunAfter(BasePlugin.CLEAN_TASK_NAME);
});
Expand All @@ -75,23 +75,18 @@ protected void createFormatTasks(String name, FormatExtension formatExtension) {
TaskProvider<SpotlessApply> applyTask = tasks.register(taskName + APPLY, SpotlessApply.class, task -> {
task.init(spotlessTask.get());
task.setGroup(TASK_GROUP);
task.setEnabled(!isIdeHook);
task.setEnabled(ideHook.path == null);
task.dependsOn(spotlessTask);
});
rootApplyTask.configure(task -> {
task.dependsOn(applyTask);

if (isIdeHook) {
// the rootApplyTask is no longer just a marker task, now it does a bit of work itself
task.doLast(unused -> IdeHook.performHook(spotlessTask.get()));
}
task.dependsOn(ideHook.path == null ? applyTask : spotlessTask);
});

TaskProvider<SpotlessCheck> checkTask = tasks.register(taskName + CHECK, SpotlessCheck.class, task -> {
SpotlessTaskImpl source = spotlessTask.get();
task.setGroup(TASK_GROUP);
task.init(source);
task.setEnabled(!isIdeHook);
task.setEnabled(ideHook.path == null);
task.dependsOn(source);

// if the user runs both, make sure that apply happens first,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,12 @@
import org.gradle.api.GradleException;
import org.gradle.api.file.DirectoryProperty;
import org.gradle.api.file.FileSystemOperations;
import org.gradle.api.provider.Property;
import org.gradle.api.provider.Provider;
import org.gradle.api.tasks.CacheableTask;
import org.gradle.api.tasks.Input;
import org.gradle.api.tasks.Internal;
import org.gradle.api.tasks.Optional;
import org.gradle.api.tasks.TaskAction;
import org.gradle.work.ChangeType;
import org.gradle.work.FileChange;
Expand All @@ -46,6 +49,10 @@

@CacheableTask
public abstract class SpotlessTaskImpl extends SpotlessTask {
@Input
@Optional
abstract Property<IdeHook.State> getIdeHookState();

@Internal
abstract DirectoryProperty getProjectDir();

Expand All @@ -69,6 +76,12 @@ Provider<SpotlessTaskService> getTaskServiceProvider() {

@TaskAction
public void performAction(InputChanges inputs) throws Exception {
IdeHook.State ideHook = getIdeHookState().getOrNull();
if (ideHook != null && ideHook.path != null) {
IdeHook.performHook(this, ideHook);
return;
}

SpotlessTaskService taskService = getTaskService().get();
taskService.registerSourceAlreadyRan(this);
if (target == null) {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2016-2021 DiffPlug
* Copyright 2016-2024 DiffPlug
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand All @@ -19,14 +19,19 @@
import java.io.IOException;
import java.io.Writer;
import java.nio.charset.StandardCharsets;
import java.util.stream.Stream;

import org.assertj.core.api.Assertions;
import org.gradle.testkit.runner.GradleRunner;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.TestInstance;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.MethodSource;

import com.diffplug.common.base.StringPrinter;
import com.diffplug.common.io.Files;

@TestInstance(TestInstance.Lifecycle.PER_CLASS)
class IdeHookTest extends GradleIntegrationHarness {
private String output, error;
private File dirty, clean, diverge, outofbounds;
Expand All @@ -40,11 +45,11 @@ void before() throws IOException {
"spotless {",
" format 'misc', {",
" target 'DIRTY.md', 'CLEAN.md'",
" custom 'lowercase', { str -> str.toLowerCase(Locale.ROOT) }",
" addStep com.diffplug.spotless.TestingOnly.lowercase()",
" }",
" format 'diverge', {",
" target 'DIVERGE.md'",
" custom 'diverge', { str -> str + ' ' }",
" addStep com.diffplug.spotless.TestingOnly.diverge()",
" }",
"}");
dirty = new File(rootFolder(), "DIRTY.md");
Expand All @@ -57,12 +62,16 @@ void before() throws IOException {
Files.write("ABC".getBytes(StandardCharsets.UTF_8), outofbounds);
}

private void runWith(String... arguments) throws IOException {
private static Stream<Boolean> configurationCacheProvider() {
return Stream.of(false, true);
}

private void runWith(boolean configurationCache, String... arguments) throws IOException {
StringBuilder output = new StringBuilder();
StringBuilder error = new StringBuilder();
try (Writer outputWriter = new StringPrinter(output::append).toWriter();
Writer errorWriter = new StringPrinter(error::append).toWriter();) {
gradleRunner()
gradleRunner(configurationCache)
.withArguments(arguments)
.forwardStdOutput(outputWriter)
.forwardStdError(errorWriter)
Expand All @@ -72,37 +81,60 @@ private void runWith(String... arguments) throws IOException {
this.error = error.toString();
}

@Test
void dirty() throws IOException {
runWith("spotlessApply", "--quiet", "-PspotlessIdeHook=" + dirty.getAbsolutePath(), "-PspotlessIdeHookUseStdOut");
protected GradleRunner gradleRunner(boolean configurationCache) throws IOException {
if (configurationCache) {
setFile("gradle.properties").toContent("org.gradle.unsafe.configuration-cache=true");
setFile("settings.gradle").toContent("enableFeaturePreview(\"STABLE_CONFIGURATION_CACHE\")");
return super.gradleRunner().withGradleVersion(GradleVersionSupport.STABLE_CONFIGURATION_CACHE.version);
} else {
File gradleProps = new File(rootFolder(), "gradle.properties");
if (gradleProps.exists()) {
gradleProps.delete();
}
File settingsGradle = new File(rootFolder(), "settings.gradle");
if (settingsGradle.exists()) {
settingsGradle.delete();
}
return super.gradleRunner();
}
}

@ParameterizedTest
@MethodSource("configurationCacheProvider")
void dirty(boolean configurationCache) throws IOException {
runWith(configurationCache, "spotlessApply", "--quiet", "-PspotlessIdeHook=" + dirty.getAbsolutePath(), "-PspotlessIdeHookUseStdOut");
Assertions.assertThat(output).isEqualTo("abc");
Assertions.assertThat(error).startsWith("IS DIRTY");
}

@Test
void clean() throws IOException {
runWith("spotlessApply", "--quiet", "-PspotlessIdeHook=" + clean.getAbsolutePath(), "-PspotlessIdeHookUseStdOut");
@ParameterizedTest
@MethodSource("configurationCacheProvider")
void clean(boolean configurationCache) throws IOException {
runWith(configurationCache, "spotlessApply", "--quiet", "-PspotlessIdeHook=" + clean.getAbsolutePath(), "-PspotlessIdeHookUseStdOut");
Assertions.assertThat(output).isEmpty();
Assertions.assertThat(error).startsWith("IS CLEAN");
}

@Test
void diverge() throws IOException {
runWith("spotlessApply", "--quiet", "-PspotlessIdeHook=" + diverge.getAbsolutePath(), "-PspotlessIdeHookUseStdOut");
@ParameterizedTest
@MethodSource("configurationCacheProvider")
void diverge(boolean configurationCache) throws IOException {
runWith(configurationCache, "spotlessApply", "--quiet", "-PspotlessIdeHook=" + diverge.getAbsolutePath(), "-PspotlessIdeHookUseStdOut");
Assertions.assertThat(output).isEmpty();
Assertions.assertThat(error).startsWith("DID NOT CONVERGE");
}

@Test
void outofbounds() throws IOException {
runWith("spotlessApply", "--quiet", "-PspotlessIdeHook=" + outofbounds.getAbsolutePath(), "-PspotlessIdeHookUseStdOut");
@ParameterizedTest
@MethodSource("configurationCacheProvider")
void outofbounds(boolean configurationCache) throws IOException {
runWith(configurationCache, "spotlessApply", "--quiet", "-PspotlessIdeHook=" + outofbounds.getAbsolutePath(), "-PspotlessIdeHookUseStdOut");
Assertions.assertThat(output).isEmpty();
Assertions.assertThat(error).isEmpty();
}

@Test
void notAbsolute() throws IOException {
runWith("spotlessApply", "--quiet", "-PspotlessIdeHook=build.gradle", "-PspotlessIdeHookUseStdOut");
@ParameterizedTest
@MethodSource("configurationCacheProvider")
void notAbsolute(boolean configurationCache) throws IOException {
runWith(configurationCache, "spotlessApply", "--quiet", "-PspotlessIdeHook=build.gradle", "-PspotlessIdeHookUseStdOut");
Assertions.assertThat(output).isEmpty();
Assertions.assertThat(error).contains("Argument passed to spotlessIdeHook must be an absolute path");
}
Expand Down

0 comments on commit 236f480

Please sign in to comment.