Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make IDE hook work with configuration cache, take 2 #2308

Merged
merged 4 commits into from
Oct 24, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading