From c41a5d8bf0a00b3a922f685361536af7e1f0c95b Mon Sep 17 00:00:00 2001 From: Kostiantyn Liutovych Date: Fri, 17 Sep 2021 00:31:10 +0200 Subject: [PATCH 01/21] Up-to-date index for Maven plugin POC of a component that allows the Maven plugin to skip already formatted and unchanged files. The code is not production-ready and probably does not work. It is only to illustrate the approach and gather feedback. A new `UpToDateChecker` maintains an index file in the build directory (`/target`). The index file has the following structure: ``` ... ``` `UpToDateChecker` loads or creates an index file at creation time into an in-memory map. Method `#isUpToDate(File)` checks the file's current last modified time against the last modified time from the in-memory map. Method `#setUpToDate(File)` updates or adds the file and its last modified time to the in-memory map. Method `#close()` writes the complete in-memory map into the index file. Spotless apply goal uses `UpToDateChecker` to determine if a file needs to be formatted. It also notifies the checker if the file has was formatted. --- plugin-maven/build.gradle | 2 + .../spotless/maven/AbstractSpotlessMojo.java | 16 ++ .../spotless/maven/SpotlessApplyMojo.java | 23 ++- .../spotless/maven/SpotlessCheckMojo.java | 21 ++- .../spotless/maven/incremental/FileIndex.java | 144 ++++++++++++++++++ .../maven/incremental/IndexBasedChecker.java | 79 ++++++++++ .../maven/incremental/NoopChecker.java | 36 +++++ .../maven/incremental/PluginFingerprint.java | 96 ++++++++++++ .../maven/incremental/UpToDateChecker.java | 38 +++++ .../maven/MavenIntegrationHarness.java | 14 +- .../incremental/PluginFingerprintTest.java | 127 +++++++++++++++ .../src/test/resources/pom-build.xml.mustache | 6 + 12 files changed, 583 insertions(+), 19 deletions(-) create mode 100644 plugin-maven/src/main/java/com/diffplug/spotless/maven/incremental/FileIndex.java create mode 100644 plugin-maven/src/main/java/com/diffplug/spotless/maven/incremental/IndexBasedChecker.java create mode 100644 plugin-maven/src/main/java/com/diffplug/spotless/maven/incremental/NoopChecker.java create mode 100644 plugin-maven/src/main/java/com/diffplug/spotless/maven/incremental/PluginFingerprint.java create mode 100644 plugin-maven/src/main/java/com/diffplug/spotless/maven/incremental/UpToDateChecker.java create mode 100644 plugin-maven/src/test/java/com/diffplug/spotless/maven/incremental/PluginFingerprintTest.java diff --git a/plugin-maven/build.gradle b/plugin-maven/build.gradle index f5257e065c..90bc3cbe7c 100644 --- a/plugin-maven/build.gradle +++ b/plugin-maven/build.gradle @@ -79,6 +79,7 @@ dependencies { compileOnly "org.apache.maven:maven-plugin-api:${VER_MAVEN_API}" compileOnly "org.apache.maven.plugin-tools:maven-plugin-annotations:${VER_MAVEN_API}" + compileOnly "org.apache.maven:maven-core:${VER_MAVEN_API}" compileOnly "org.eclipse.aether:aether-api:${VER_ECLIPSE_AETHER}" compileOnly "org.eclipse.aether:aether-util:${VER_ECLIPSE_AETHER}" @@ -95,6 +96,7 @@ dependencies { testImplementation "org.apache.maven:maven-plugin-api:${VER_MAVEN_API}" testImplementation "org.eclipse.aether:aether-api:${VER_ECLIPSE_AETHER}" testImplementation "org.codehaus.plexus:plexus-resources:${VER_PLEXUS_RESOURCES}" + testImplementation "org.apache.maven:maven-core:${VER_MAVEN_API}" } task cleanMavenProjectDir(type: Delete) { delete MAVEN_PROJECT_DIR } diff --git a/plugin-maven/src/main/java/com/diffplug/spotless/maven/AbstractSpotlessMojo.java b/plugin-maven/src/main/java/com/diffplug/spotless/maven/AbstractSpotlessMojo.java index 3a98ee240b..30e341d4bf 100644 --- a/plugin-maven/src/main/java/com/diffplug/spotless/maven/AbstractSpotlessMojo.java +++ b/plugin-maven/src/main/java/com/diffplug/spotless/maven/AbstractSpotlessMojo.java @@ -37,6 +37,7 @@ import org.apache.maven.plugin.MojoExecutionException; import org.apache.maven.plugins.annotations.Component; import org.apache.maven.plugins.annotations.Parameter; +import org.apache.maven.project.MavenProject; import org.codehaus.plexus.resource.ResourceManager; import org.codehaus.plexus.resource.loader.FileResourceLoader; import org.codehaus.plexus.util.FileUtils; @@ -54,6 +55,7 @@ import com.diffplug.spotless.maven.generic.Format; import com.diffplug.spotless.maven.generic.LicenseHeader; import com.diffplug.spotless.maven.groovy.Groovy; +import com.diffplug.spotless.maven.incremental.UpToDateChecker; import com.diffplug.spotless.maven.java.Java; import com.diffplug.spotless.maven.kotlin.Kotlin; import com.diffplug.spotless.maven.pom.Pom; @@ -84,6 +86,9 @@ public abstract class AbstractSpotlessMojo extends AbstractMojo { @Parameter(property = "spotless.check.skip", defaultValue = "false") private boolean checkSkip; + @Parameter(defaultValue = "${project}", required = true, readonly = true) + private MavenProject project; + @Parameter(defaultValue = "${repositorySystemSession}", required = true, readonly = true) private RepositorySystemSession repositorySystemSession; @@ -147,6 +152,10 @@ public abstract class AbstractSpotlessMojo extends AbstractMojo { @Parameter(property = LicenseHeaderStep.spotlessSetLicenseHeaderYearsFromGitHistory) private String setLicenseHeaderYearsFromGitHistory; + // todo: check naming convensions for plugin properties + @Parameter(property = "spotless.upToDateCheckingEnabled", defaultValue = "false") + private boolean upToDateCheckingEnabled; + protected abstract void process(Iterable files, Formatter formatter) throws MojoExecutionException; @Override @@ -157,6 +166,13 @@ public final void execute() throws MojoExecutionException { } } + final UpToDateChecker createUpToDateChecker() { + if (upToDateCheckingEnabled) { + return UpToDateChecker.forProject(project, getLog()); + } + return UpToDateChecker.noop(); + } + private boolean shouldSkip() { switch (goal) { case GOAL_CHECK: diff --git a/plugin-maven/src/main/java/com/diffplug/spotless/maven/SpotlessApplyMojo.java b/plugin-maven/src/main/java/com/diffplug/spotless/maven/SpotlessApplyMojo.java index b7c0c31080..739216aa5a 100644 --- a/plugin-maven/src/main/java/com/diffplug/spotless/maven/SpotlessApplyMojo.java +++ b/plugin-maven/src/main/java/com/diffplug/spotless/maven/SpotlessApplyMojo.java @@ -23,6 +23,7 @@ import com.diffplug.spotless.Formatter; import com.diffplug.spotless.PaddedCell; +import com.diffplug.spotless.maven.incremental.UpToDateChecker; /** * Performs formatting of all source files according to configured formatters. @@ -32,14 +33,22 @@ public class SpotlessApplyMojo extends AbstractSpotlessMojo { @Override protected void process(Iterable files, Formatter formatter) throws MojoExecutionException { - for (File file : files) { - try { - PaddedCell.DirtyState dirtyState = PaddedCell.calculateDirtyState(formatter, file); - if (!dirtyState.isClean() && !dirtyState.didNotConverge()) { - dirtyState.writeCanonicalTo(file); + try (UpToDateChecker upToDateChecker = createUpToDateChecker()) { + for (File file : files) { + if (upToDateChecker.isUpToDate(file)) { + continue; } - } catch (IOException e) { - throw new MojoExecutionException("Unable to format file " + file, e); + + try { + PaddedCell.DirtyState dirtyState = PaddedCell.calculateDirtyState(formatter, file); + if (!dirtyState.isClean() && !dirtyState.didNotConverge()) { + dirtyState.writeCanonicalTo(file); + } + } catch (IOException e) { + throw new MojoExecutionException("Unable to format file " + file, e); + } + + upToDateChecker.setUpToDate(file); } } } diff --git a/plugin-maven/src/main/java/com/diffplug/spotless/maven/SpotlessCheckMojo.java b/plugin-maven/src/main/java/com/diffplug/spotless/maven/SpotlessCheckMojo.java index 440a43f3bb..2556d9106f 100644 --- a/plugin-maven/src/main/java/com/diffplug/spotless/maven/SpotlessCheckMojo.java +++ b/plugin-maven/src/main/java/com/diffplug/spotless/maven/SpotlessCheckMojo.java @@ -27,6 +27,7 @@ import com.diffplug.spotless.Formatter; import com.diffplug.spotless.PaddedCell; import com.diffplug.spotless.extra.integration.DiffMessageFormatter; +import com.diffplug.spotless.maven.incremental.UpToDateChecker; /** * Performs code formatting analysis and prints all violations to the console. @@ -38,14 +39,20 @@ public class SpotlessCheckMojo extends AbstractSpotlessMojo { @Override protected void process(Iterable files, Formatter formatter) throws MojoExecutionException { List problemFiles = new ArrayList<>(); - for (File file : files) { - try { - PaddedCell.DirtyState dirtyState = PaddedCell.calculateDirtyState(formatter, file); - if (!dirtyState.isClean() && !dirtyState.didNotConverge()) { - problemFiles.add(file); + try (UpToDateChecker upToDateChecker = createUpToDateChecker()) { + for (File file : files) { + if (upToDateChecker.isUpToDate(file)) { + continue; + } + + try { + PaddedCell.DirtyState dirtyState = PaddedCell.calculateDirtyState(formatter, file); + if (!dirtyState.isClean() && !dirtyState.didNotConverge()) { + problemFiles.add(file); + } + } catch (IOException e) { + throw new MojoExecutionException("Unable to format file " + file, e); } - } catch (IOException e) { - throw new MojoExecutionException("Unable to format file " + file, e); } } diff --git a/plugin-maven/src/main/java/com/diffplug/spotless/maven/incremental/FileIndex.java b/plugin-maven/src/main/java/com/diffplug/spotless/maven/incremental/FileIndex.java new file mode 100644 index 0000000000..d7db521f84 --- /dev/null +++ b/plugin-maven/src/main/java/com/diffplug/spotless/maven/incremental/FileIndex.java @@ -0,0 +1,144 @@ +/* + * 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.spotless.maven.incremental; + +import static java.nio.charset.StandardCharsets.UTF_8; +import static java.nio.file.Files.newBufferedReader; +import static java.nio.file.Files.newBufferedWriter; +import static java.nio.file.StandardOpenOption.CREATE; +import static java.nio.file.StandardOpenOption.TRUNCATE_EXISTING; + +import java.io.BufferedReader; +import java.io.IOException; +import java.io.PrintWriter; +import java.io.UncheckedIOException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.Paths; +import java.time.Instant; +import java.util.Map; +import java.util.Map.Entry; +import java.util.Optional; +import java.util.TreeMap; + +import org.apache.maven.plugin.logging.Log; + +// todo: use TrieMap for fileToLastModifiedTime with String keys +// todo: FileTime -> Instant can be a lossy conversion +// todo: add unit tests +class FileIndex { + + private static final String INDEX_FILE_NAME = "spotless-index"; + private static final String SEPARATOR = " "; + + private final PluginFingerprint pluginFingerprint; + private final Path baseDir; + private final Map fileToLastModifiedTime; + private boolean updated; + + private FileIndex(PluginFingerprint pluginFingerprint, Path baseDir, Map fileToLastModifiedTime) { + this.pluginFingerprint = pluginFingerprint; + this.baseDir = baseDir; + this.fileToLastModifiedTime = fileToLastModifiedTime; + } + + static FileIndex read(Path baseDir, PluginFingerprint pluginFingerprint, Log log) { + Path indexFile = indexFile(baseDir); + if (Files.notExists(indexFile)) { + log.info("Index file does not exist. Fallback to an empty index"); + return emptyIndex(baseDir, pluginFingerprint); + } + + try (BufferedReader reader = newBufferedReader(indexFile, UTF_8)) { + PluginFingerprint actualPluginFingerprint = PluginFingerprint.from(reader.readLine()); + if (!pluginFingerprint.equals(actualPluginFingerprint)) { + log.info("Fingerprint mismatch in the index file. Fallback to an empty index"); + return emptyIndex(baseDir, pluginFingerprint); + } else { + Map fileToLastModifiedTime = readFileToLastModifiedTime(reader, baseDir, log); + return new FileIndex(pluginFingerprint, baseDir, fileToLastModifiedTime); + } + } catch (IOException e) { + log.warn("Error reading the index file. Fallback to an empty index", e); + return emptyIndex(baseDir, pluginFingerprint); + } + } + + Optional getLastModifiedTime(Path file) { + if (!file.startsWith(baseDir)) { + return Optional.empty(); + } + Path relativeFile = baseDir.relativize(file); + return Optional.ofNullable(fileToLastModifiedTime.get(relativeFile)); + } + + void setLastModifiedTime(Path file, Instant time) { + Path relativeFile = baseDir.relativize(file); + fileToLastModifiedTime.put(relativeFile, time); + updated = true; + } + + void write() { + if (!updated) { + return; + } + + Path indexFile = indexFile(baseDir); + try (PrintWriter writer = new PrintWriter(newBufferedWriter(indexFile, UTF_8, CREATE, TRUNCATE_EXISTING))) { + writer.println(pluginFingerprint.value()); + + for (Entry entry : fileToLastModifiedTime.entrySet()) { + writer.print(entry.getKey() + SEPARATOR + entry.getValue()); + } + } catch (IOException e) { + throw new UncheckedIOException("Unable to write the index", e); + } + } + + private static Map readFileToLastModifiedTime(BufferedReader reader, Path baseDir, Log log) throws IOException { + Map fileToLastModifiedTime = new TreeMap<>(); + String line; + while ((line = reader.readLine()) != null) { + int separatorIndex = line.lastIndexOf(SEPARATOR); + if (separatorIndex == -1) { + throw new IOException("Incorrect index file. No separator found in '" + line + "'"); + } + + Path relativeFile = Paths.get(line.substring(0, separatorIndex)); + Path absoluteFile = baseDir.resolve(relativeFile); + if (Files.notExists(absoluteFile)) { + log.info("File stored in the index does not exist: " + relativeFile); + } else { + Instant lastModifiedTime; + try { + lastModifiedTime = Instant.parse(line.substring(separatorIndex)); + } catch (Exception e) { + throw new IOException("Incorrect index file. Unable to parse last modified time from '" + line + "'", e); + } + fileToLastModifiedTime.put(relativeFile, lastModifiedTime); + } + } + return fileToLastModifiedTime; + } + + private static Path indexFile(Path baseDir) { + return baseDir.resolve(INDEX_FILE_NAME); + } + + private static FileIndex emptyIndex(Path baseDir, PluginFingerprint pluginFingerprint) { + return new FileIndex(pluginFingerprint, baseDir, new TreeMap<>()); + } +} diff --git a/plugin-maven/src/main/java/com/diffplug/spotless/maven/incremental/IndexBasedChecker.java b/plugin-maven/src/main/java/com/diffplug/spotless/maven/incremental/IndexBasedChecker.java new file mode 100644 index 0000000000..cb5655be75 --- /dev/null +++ b/plugin-maven/src/main/java/com/diffplug/spotless/maven/incremental/IndexBasedChecker.java @@ -0,0 +1,79 @@ +/* + * 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.spotless.maven.incremental; + +import java.io.File; +import java.io.IOException; +import java.io.UncheckedIOException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.time.Instant; +import java.util.Optional; + +import org.apache.maven.plugin.logging.Log; +import org.apache.maven.project.MavenProject; + +// todo: add unit tests +class IndexBasedChecker implements UpToDateChecker { + + private final FileIndex index; + + private IndexBasedChecker(FileIndex index) { + this.index = index; + } + + static IndexBasedChecker create(MavenProject project, Log log) { + PluginFingerprint pluginFingerprint = PluginFingerprint.from(project); + // todo: does this produce the correct dir? + Path buildDir = project.getBasedir().toPath().resolve(project.getBuild().getDirectory()); + FileIndex fileIndex = FileIndex.read(buildDir, pluginFingerprint, log); + return new IndexBasedChecker(fileIndex); + } + + @Override + public boolean isUpToDate(File file) { + Path path = file.toPath(); + + Optional storedLastModifiedTimeOptional = index.getLastModifiedTime(path); + if (!storedLastModifiedTimeOptional.isPresent()) { + return false; + } + + Instant currentLastModifiedTime = lastModifiedTime(path); + Instant storedLastModifiedTime = storedLastModifiedTimeOptional.get(); + return currentLastModifiedTime.isAfter(storedLastModifiedTime); + } + + @Override + public void setUpToDate(File file) { + Path path = file.toPath(); + Instant currentLastModifiedTime = lastModifiedTime(path); + index.setLastModifiedTime(path, currentLastModifiedTime); + } + + @Override + public void close() { + index.write(); + } + + private static Instant lastModifiedTime(Path path) { + try { + return Files.getLastModifiedTime(path).toInstant(); + } catch (IOException e) { + throw new UncheckedIOException("Unable to get last modified date for " + path, e); + } + } +} diff --git a/plugin-maven/src/main/java/com/diffplug/spotless/maven/incremental/NoopChecker.java b/plugin-maven/src/main/java/com/diffplug/spotless/maven/incremental/NoopChecker.java new file mode 100644 index 0000000000..11f1bfc6d4 --- /dev/null +++ b/plugin-maven/src/main/java/com/diffplug/spotless/maven/incremental/NoopChecker.java @@ -0,0 +1,36 @@ +/* + * 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.spotless.maven.incremental; + +import java.io.File; + +class NoopChecker implements UpToDateChecker { + + static final NoopChecker INSTANCE = new NoopChecker(); + + private NoopChecker() {} + + @Override + public boolean isUpToDate(File file) { + return false; + } + + @Override + public void setUpToDate(File file) {} + + @Override + public void close() {} +} diff --git a/plugin-maven/src/main/java/com/diffplug/spotless/maven/incremental/PluginFingerprint.java b/plugin-maven/src/main/java/com/diffplug/spotless/maven/incremental/PluginFingerprint.java new file mode 100644 index 0000000000..39366e3e48 --- /dev/null +++ b/plugin-maven/src/main/java/com/diffplug/spotless/maven/incremental/PluginFingerprint.java @@ -0,0 +1,96 @@ +/* + * 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.spotless.maven.incremental; + +import java.io.ByteArrayOutputStream; +import java.io.IOException; +import java.io.ObjectOutputStream; +import java.io.UncheckedIOException; +import java.security.MessageDigest; +import java.security.NoSuchAlgorithmException; +import java.util.Base64; +import java.util.Objects; + +import org.apache.maven.model.Plugin; +import org.apache.maven.project.MavenProject; + +class PluginFingerprint { + + private static final String SPOTLESS_PLUGIN_KEY = "com.diffplug.spotless:spotless-maven-plugin"; + + private final String value; + + private PluginFingerprint(String value) { + this.value = value; + } + + static PluginFingerprint from(MavenProject project) { + Plugin spotlessPlugin = project.getPlugin(SPOTLESS_PLUGIN_KEY); + byte[] serialized = serialize(spotlessPlugin); + byte[] digest = digest(serialized); + String value = Base64.getEncoder().encodeToString(digest); + return new PluginFingerprint(value); + } + + static PluginFingerprint from(String value) { + return new PluginFingerprint(value); + } + + String value() { + return value; + } + + @Override + public boolean equals(Object o) { + if (this == o) { + return true; + } + if (o == null || getClass() != o.getClass()) { + return false; + } + PluginFingerprint that = (PluginFingerprint) o; + return value.equals(that.value); + } + + @Override + public int hashCode() { + return Objects.hash(value); + } + + @Override + public String toString() { + return "PluginFingerprint[" + value + "]"; + } + + private static byte[] digest(byte[] bytes) { + try { + return MessageDigest.getInstance("SHA-256").digest(bytes); + } catch (NoSuchAlgorithmException e) { + throw new IllegalStateException("SHA-256 digest algorithm not available", e); + } + } + + private static byte[] serialize(Plugin plugin) { + try (ByteArrayOutputStream byteStream = new ByteArrayOutputStream(); + ObjectOutputStream objectStream = new ObjectOutputStream(byteStream)) { + objectStream.writeObject(plugin); + objectStream.flush(); + return byteStream.toByteArray(); + } catch (IOException e) { + throw new UncheckedIOException("Unable to serialize plugin " + plugin, e); + } + } +} diff --git a/plugin-maven/src/main/java/com/diffplug/spotless/maven/incremental/UpToDateChecker.java b/plugin-maven/src/main/java/com/diffplug/spotless/maven/incremental/UpToDateChecker.java new file mode 100644 index 0000000000..cc865fe50a --- /dev/null +++ b/plugin-maven/src/main/java/com/diffplug/spotless/maven/incremental/UpToDateChecker.java @@ -0,0 +1,38 @@ +/* + * 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.spotless.maven.incremental; + +import java.io.File; + +import org.apache.maven.plugin.logging.Log; +import org.apache.maven.project.MavenProject; + +public interface UpToDateChecker extends AutoCloseable { + + boolean isUpToDate(File file); + + void setUpToDate(File file); + + void close(); + + static UpToDateChecker noop() { + return NoopChecker.INSTANCE; + } + + static UpToDateChecker forProject(MavenProject project, Log log) { + return IndexBasedChecker.create(project, log); + } +} diff --git a/plugin-maven/src/test/java/com/diffplug/spotless/maven/MavenIntegrationHarness.java b/plugin-maven/src/test/java/com/diffplug/spotless/maven/MavenIntegrationHarness.java index fa5224bb8d..b213b67731 100644 --- a/plugin-maven/src/test/java/com/diffplug/spotless/maven/MavenIntegrationHarness.java +++ b/plugin-maven/src/test/java/com/diffplug/spotless/maven/MavenIntegrationHarness.java @@ -157,8 +157,12 @@ protected MultiModuleProjectCreator multiModuleProject() { return new MultiModuleProjectCreator(); } - private String createPomXmlContent(String[] executions, String[] configuration) throws IOException { - Map params = buildPomXmlParams(executions, configuration, null); + protected String createPomXmlContent(String[] executions, String[] configuration) throws IOException { + return createPomXmlContent(null, executions, configuration); + } + + protected String createPomXmlContent(String pluginVersion, String[] executions, String[] configuration) throws IOException { + Map params = buildPomXmlParams(pluginVersion, executions, configuration, null); return createPomXmlContent("/pom-test.xml.mustache", params); } @@ -172,9 +176,9 @@ private String createPomXmlContent(String pomTemplate, Map param } } - private static Map buildPomXmlParams(String[] executions, String[] configuration, String[] modules) { + private static Map buildPomXmlParams(String pluginVersion, String[] executions, String[] configuration, String[] modules) { Map params = new HashMap<>(); - params.put(SPOTLESS_MAVEN_PLUGIN_VERSION, getSystemProperty(SPOTLESS_MAVEN_PLUGIN_VERSION)); + params.put(SPOTLESS_MAVEN_PLUGIN_VERSION, pluginVersion == null ? getSystemProperty(SPOTLESS_MAVEN_PLUGIN_VERSION) : pluginVersion); if (configuration != null) { params.put(CONFIGURATION, String.join("\n", configuration)); @@ -265,7 +269,7 @@ private void createRootPom() throws IOException { modulesList.addAll(subProjects.keySet()); String[] modules = modulesList.toArray(new String[0]); - Map rootPomParams = buildPomXmlParams(null, configuration, modules); + Map rootPomParams = buildPomXmlParams(null, null, configuration, modules); setFile("pom.xml").toContent(createPomXmlContent("/multi-module/pom-parent.xml.mustache", rootPomParams)); } diff --git a/plugin-maven/src/test/java/com/diffplug/spotless/maven/incremental/PluginFingerprintTest.java b/plugin-maven/src/test/java/com/diffplug/spotless/maven/incremental/PluginFingerprintTest.java new file mode 100644 index 0000000000..ba7bf52471 --- /dev/null +++ b/plugin-maven/src/test/java/com/diffplug/spotless/maven/incremental/PluginFingerprintTest.java @@ -0,0 +1,127 @@ +/* + * 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.spotless.maven.incremental; + +import static java.nio.charset.StandardCharsets.UTF_8; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotEquals; + +import java.io.ByteArrayInputStream; + +import org.apache.maven.model.Model; +import org.apache.maven.model.io.xpp3.MavenXpp3Reader; +import org.apache.maven.project.MavenProject; +import org.codehaus.plexus.util.ReaderFactory; +import org.codehaus.plexus.util.xml.XmlStreamReader; +import org.junit.jupiter.api.Test; + +import com.diffplug.spotless.maven.MavenIntegrationHarness; + +class PluginFingerprintTest extends MavenIntegrationHarness { + + private static final String VERSION_1 = "1.0.0"; + private static final String VERSION_2 = "2.0.0"; + + private static final String[] EXECUTION_1 = { + "", + " check", + " ", + " check", + " ", + "" + }; + private static final String[] EXECUTION_2 = {}; + + private static final String[] CONFIGURATION_1 = { + "", + " 1.2", + "" + }; + private static final String[] CONFIGURATION_2 = { + "", + " 1.8", + " true", + "" + }; + + @Test + void sameFingerprint() throws Exception { + String xml1 = createPomXmlContent(VERSION_1, EXECUTION_1, CONFIGURATION_1); + String xml2 = createPomXmlContent(VERSION_1, EXECUTION_1, CONFIGURATION_1); + + MavenProject project1 = mavenProject(xml1); + MavenProject project2 = mavenProject(xml2); + + PluginFingerprint fingerprint1 = PluginFingerprint.from(project1); + PluginFingerprint fingerprint2 = PluginFingerprint.from(project2); + + assertEquals(fingerprint1, fingerprint2); + } + + @Test + void differentFingerprintForDifferentPluginVersion() throws Exception { + String xml1 = createPomXmlContent(VERSION_1, EXECUTION_1, CONFIGURATION_1); + String xml2 = createPomXmlContent(VERSION_2, EXECUTION_1, CONFIGURATION_1); + + MavenProject project1 = mavenProject(xml1); + MavenProject project2 = mavenProject(xml2); + + PluginFingerprint fingerprint1 = PluginFingerprint.from(project1); + PluginFingerprint fingerprint2 = PluginFingerprint.from(project2); + + assertNotEquals(fingerprint1, fingerprint2); + } + + @Test + void differentFingerprintForDifferentExecution() throws Exception { + String xml1 = createPomXmlContent(VERSION_2, EXECUTION_1, CONFIGURATION_1); + String xml2 = createPomXmlContent(VERSION_2, EXECUTION_2, CONFIGURATION_1); + + MavenProject project1 = mavenProject(xml1); + MavenProject project2 = mavenProject(xml2); + + PluginFingerprint fingerprint1 = PluginFingerprint.from(project1); + PluginFingerprint fingerprint2 = PluginFingerprint.from(project2); + + assertNotEquals(fingerprint1, fingerprint2); + } + + @Test + void differentFingerprintForDifferentConfiguration() throws Exception { + String xml1 = createPomXmlContent(VERSION_1, EXECUTION_2, CONFIGURATION_2); + String xml2 = createPomXmlContent(VERSION_1, EXECUTION_2, CONFIGURATION_1); + + MavenProject project1 = mavenProject(xml1); + MavenProject project2 = mavenProject(xml2); + + PluginFingerprint fingerprint1 = PluginFingerprint.from(project1); + PluginFingerprint fingerprint2 = PluginFingerprint.from(project2); + + assertNotEquals(fingerprint1, fingerprint2); + } + + private static MavenProject mavenProject(String xml) throws Exception { + return new MavenProject(readPom(xml)); + } + + private static Model readPom(String xml) throws Exception { + byte[] bytes = xml.getBytes(UTF_8); + try (XmlStreamReader xmlReader = ReaderFactory.newXmlReader(new ByteArrayInputStream(bytes))) { + MavenXpp3Reader pomReader = new MavenXpp3Reader(); + return pomReader.read(xmlReader); + } + } +} diff --git a/plugin-maven/src/test/resources/pom-build.xml.mustache b/plugin-maven/src/test/resources/pom-build.xml.mustache index 3b0c1c2778..9e40094cfe 100644 --- a/plugin-maven/src/test/resources/pom-build.xml.mustache +++ b/plugin-maven/src/test/resources/pom-build.xml.mustache @@ -26,6 +26,12 @@ + + org.apache.maven + maven-core + ${maven.api.version} + provided + org.apache.maven maven-plugin-api From ae4b7230b0e4af145e0d4c92a3acd8ce3077640e Mon Sep 17 00:00:00 2001 From: Kostiantyn Liutovych Date: Tue, 2 Nov 2021 11:59:08 +0100 Subject: [PATCH 02/21] Create a single up-to-date index per Maven mojo execution Instead of creating/opening an index for every source file. --- .../spotless/maven/AbstractSpotlessMojo.java | 26 +++++++++-------- .../spotless/maven/SpotlessApplyMojo.java | 28 +++++++++---------- .../spotless/maven/SpotlessCheckMojo.java | 24 ++++++++-------- 3 files changed, 38 insertions(+), 40 deletions(-) diff --git a/plugin-maven/src/main/java/com/diffplug/spotless/maven/AbstractSpotlessMojo.java b/plugin-maven/src/main/java/com/diffplug/spotless/maven/AbstractSpotlessMojo.java index 30e341d4bf..0a8ab6e650 100644 --- a/plugin-maven/src/main/java/com/diffplug/spotless/maven/AbstractSpotlessMojo.java +++ b/plugin-maven/src/main/java/com/diffplug/spotless/maven/AbstractSpotlessMojo.java @@ -156,21 +156,16 @@ public abstract class AbstractSpotlessMojo extends AbstractMojo { @Parameter(property = "spotless.upToDateCheckingEnabled", defaultValue = "false") private boolean upToDateCheckingEnabled; - protected abstract void process(Iterable files, Formatter formatter) throws MojoExecutionException; + protected abstract void process(Iterable files, Formatter formatter, UpToDateChecker upToDateChecker) throws MojoExecutionException; @Override public final void execute() throws MojoExecutionException { List formatterFactories = getFormatterFactories(); - for (FormatterFactory formatterFactory : formatterFactories) { - execute(formatterFactory); - } - } - - final UpToDateChecker createUpToDateChecker() { - if (upToDateCheckingEnabled) { - return UpToDateChecker.forProject(project, getLog()); + try (UpToDateChecker upToDateChecker = createUpToDateChecker()) { + for (FormatterFactory formatterFactory : formatterFactories) { + execute(formatterFactory, upToDateChecker); + } } - return UpToDateChecker.noop(); } private boolean shouldSkip() { @@ -185,7 +180,7 @@ private boolean shouldSkip() { return false; } - private void execute(FormatterFactory formatterFactory) throws MojoExecutionException { + private void execute(FormatterFactory formatterFactory, UpToDateChecker upToDateChecker) throws MojoExecutionException { if (shouldSkip()) { getLog().info(String.format("Spotless %s skipped", goal)); return; @@ -195,7 +190,7 @@ private void execute(FormatterFactory formatterFactory) throws MojoExecutionExce List files = collectFiles(formatterFactory, config); try (Formatter formatter = formatterFactory.newFormatter(files, config)) { - process(files, formatter); + process(files, formatter, upToDateChecker); } } @@ -316,4 +311,11 @@ private List getFormatterStepFactories() { .filter(Objects::nonNull) .collect(toList()); } + + private UpToDateChecker createUpToDateChecker() { + if (upToDateCheckingEnabled) { + return UpToDateChecker.forProject(project, getLog()); + } + return UpToDateChecker.noop(); + } } diff --git a/plugin-maven/src/main/java/com/diffplug/spotless/maven/SpotlessApplyMojo.java b/plugin-maven/src/main/java/com/diffplug/spotless/maven/SpotlessApplyMojo.java index 739216aa5a..cff01b90a0 100644 --- a/plugin-maven/src/main/java/com/diffplug/spotless/maven/SpotlessApplyMojo.java +++ b/plugin-maven/src/main/java/com/diffplug/spotless/maven/SpotlessApplyMojo.java @@ -32,24 +32,22 @@ public class SpotlessApplyMojo extends AbstractSpotlessMojo { @Override - protected void process(Iterable files, Formatter formatter) throws MojoExecutionException { - try (UpToDateChecker upToDateChecker = createUpToDateChecker()) { - for (File file : files) { - if (upToDateChecker.isUpToDate(file)) { - continue; - } + protected void process(Iterable files, Formatter formatter, UpToDateChecker upToDateChecker) throws MojoExecutionException { + for (File file : files) { + if (upToDateChecker.isUpToDate(file)) { + continue; + } - try { - PaddedCell.DirtyState dirtyState = PaddedCell.calculateDirtyState(formatter, file); - if (!dirtyState.isClean() && !dirtyState.didNotConverge()) { - dirtyState.writeCanonicalTo(file); - } - } catch (IOException e) { - throw new MojoExecutionException("Unable to format file " + file, e); + try { + PaddedCell.DirtyState dirtyState = PaddedCell.calculateDirtyState(formatter, file); + if (!dirtyState.isClean() && !dirtyState.didNotConverge()) { + dirtyState.writeCanonicalTo(file); } - - upToDateChecker.setUpToDate(file); + } catch (IOException e) { + throw new MojoExecutionException("Unable to format file " + file, e); } + + upToDateChecker.setUpToDate(file); } } } diff --git a/plugin-maven/src/main/java/com/diffplug/spotless/maven/SpotlessCheckMojo.java b/plugin-maven/src/main/java/com/diffplug/spotless/maven/SpotlessCheckMojo.java index 2556d9106f..33c2ad9ff6 100644 --- a/plugin-maven/src/main/java/com/diffplug/spotless/maven/SpotlessCheckMojo.java +++ b/plugin-maven/src/main/java/com/diffplug/spotless/maven/SpotlessCheckMojo.java @@ -37,22 +37,20 @@ public class SpotlessCheckMojo extends AbstractSpotlessMojo { @Override - protected void process(Iterable files, Formatter formatter) throws MojoExecutionException { + protected void process(Iterable files, Formatter formatter, UpToDateChecker upToDateChecker) throws MojoExecutionException { List problemFiles = new ArrayList<>(); - try (UpToDateChecker upToDateChecker = createUpToDateChecker()) { - for (File file : files) { - if (upToDateChecker.isUpToDate(file)) { - continue; - } + for (File file : files) { + if (upToDateChecker.isUpToDate(file)) { + continue; + } - try { - PaddedCell.DirtyState dirtyState = PaddedCell.calculateDirtyState(formatter, file); - if (!dirtyState.isClean() && !dirtyState.didNotConverge()) { - problemFiles.add(file); - } - } catch (IOException e) { - throw new MojoExecutionException("Unable to format file " + file, e); + try { + PaddedCell.DirtyState dirtyState = PaddedCell.calculateDirtyState(formatter, file); + if (!dirtyState.isClean() && !dirtyState.didNotConverge()) { + problemFiles.add(file); } + } catch (IOException e) { + throw new MojoExecutionException("Unable to format file " + file, e); } } From e7f8ac9b18cedce5306570598763c8b8f46b0379 Mon Sep 17 00:00:00 2001 From: Kostiantyn Liutovych Date: Sun, 7 Nov 2021 23:39:26 +0100 Subject: [PATCH 03/21] Make Maven file index care about formatters Include the list of serialized formatters in PluginFingerprint. It makes the file index sensitive to changes in configuration files referenced by the POM (e.g. `eclipseformat.xml` and `.gitattributes`). To achieve this, Maven plugin has to instantiate all formatters upfront. It builds a map of formatters and files that need formatting. The formatters are then used to calculate a PluginFingerprint. --- .../spotless/maven/AbstractSpotlessMojo.java | 41 +++++---- .../spotless/maven/FormattersHolder.java | 91 +++++++++++++++++++ .../maven/incremental/IndexBasedChecker.java | 6 +- .../incremental/ObjectDigestOutputStream.java | 57 ++++++++++++ .../maven/incremental/PluginFingerprint.java | 33 +++---- .../maven/incremental/UpToDateChecker.java | 6 +- .../incremental/PluginFingerprintTest.java | 82 +++++++++++++++-- 7 files changed, 264 insertions(+), 52 deletions(-) create mode 100644 plugin-maven/src/main/java/com/diffplug/spotless/maven/FormattersHolder.java create mode 100644 plugin-maven/src/main/java/com/diffplug/spotless/maven/incremental/ObjectDigestOutputStream.java diff --git a/plugin-maven/src/main/java/com/diffplug/spotless/maven/AbstractSpotlessMojo.java b/plugin-maven/src/main/java/com/diffplug/spotless/maven/AbstractSpotlessMojo.java index 0a8ab6e650..6e88ab6024 100644 --- a/plugin-maven/src/main/java/com/diffplug/spotless/maven/AbstractSpotlessMojo.java +++ b/plugin-maven/src/main/java/com/diffplug/spotless/maven/AbstractSpotlessMojo.java @@ -22,8 +22,11 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; +import java.util.HashMap; import java.util.HashSet; import java.util.List; +import java.util.Map; +import java.util.Map.Entry; import java.util.Objects; import java.util.Optional; import java.util.Set; @@ -160,10 +163,24 @@ public abstract class AbstractSpotlessMojo extends AbstractMojo { @Override public final void execute() throws MojoExecutionException { + if (shouldSkip()) { + getLog().info(String.format("Spotless %s skipped", goal)); + return; + } + List formatterFactories = getFormatterFactories(); - try (UpToDateChecker upToDateChecker = createUpToDateChecker()) { - for (FormatterFactory formatterFactory : formatterFactories) { - execute(formatterFactory, upToDateChecker); + FormatterConfig config = getFormatterConfig(); + + Map> formatterFactoryToFiles = new HashMap<>(); + for (FormatterFactory formatterFactory : formatterFactories) { + List files = collectFiles(formatterFactory, config); + formatterFactoryToFiles.put(formatterFactory, files); + } + + try (FormattersHolder formattersHolder = FormattersHolder.create(formatterFactoryToFiles, config); + UpToDateChecker upToDateChecker = createUpToDateChecker(formattersHolder.getFormatters())) { + for (Entry> entry : formattersHolder.getFormattersWithFiles().entrySet()) { + process(entry.getValue(), entry.getKey(), upToDateChecker); } } } @@ -180,20 +197,6 @@ private boolean shouldSkip() { return false; } - private void execute(FormatterFactory formatterFactory, UpToDateChecker upToDateChecker) throws MojoExecutionException { - if (shouldSkip()) { - getLog().info(String.format("Spotless %s skipped", goal)); - return; - } - - FormatterConfig config = getFormatterConfig(); - List files = collectFiles(formatterFactory, config); - - try (Formatter formatter = formatterFactory.newFormatter(files, config)) { - process(files, formatter, upToDateChecker); - } - } - private List collectFiles(FormatterFactory formatterFactory, FormatterConfig config) throws MojoExecutionException { Optional ratchetFrom = formatterFactory.ratchetFrom(config); try { @@ -312,9 +315,9 @@ private List getFormatterStepFactories() { .collect(toList()); } - private UpToDateChecker createUpToDateChecker() { + private UpToDateChecker createUpToDateChecker(Iterable formatters) { if (upToDateCheckingEnabled) { - return UpToDateChecker.forProject(project, getLog()); + return UpToDateChecker.forProject(project, formatters, getLog()); } return UpToDateChecker.noop(); } diff --git a/plugin-maven/src/main/java/com/diffplug/spotless/maven/FormattersHolder.java b/plugin-maven/src/main/java/com/diffplug/spotless/maven/FormattersHolder.java new file mode 100644 index 0000000000..7fa15150cc --- /dev/null +++ b/plugin-maven/src/main/java/com/diffplug/spotless/maven/FormattersHolder.java @@ -0,0 +1,91 @@ +/* + * 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.spotless.maven; + +import java.io.File; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.Map.Entry; +import java.util.Set; + +import com.diffplug.spotless.Formatter; + +class FormattersHolder implements AutoCloseable { + + private final Map> formatterToFiles; + + private FormattersHolder(Map> formatterToFiles) { + this.formatterToFiles = formatterToFiles; + } + + static FormattersHolder create(Map> formatterFactoryToFiles, FormatterConfig config) { + Map> formatterToFiles = new HashMap<>(); + try { + for (Entry> entry : formatterFactoryToFiles.entrySet()) { + FormatterFactory formatterFactory = entry.getKey(); + List files = entry.getValue(); + + Formatter formatter = formatterFactory.newFormatter(files, config); + formatterToFiles.put(formatter, files); + } + } catch (RuntimeException openError) { + try { + close(formatterToFiles.keySet()); + } catch (Exception closeError) { + openError.addSuppressed(closeError); + } + throw openError; + } + + return new FormattersHolder(formatterToFiles); + } + + Iterable getFormatters() { + return formatterToFiles.keySet(); + } + + Map> getFormattersWithFiles() { + return formatterToFiles; + } + + @Override + public void close() { + try { + close(formatterToFiles.keySet()); + } catch (Exception e) { + throw new RuntimeException("Unable to close formatters", e); + } + } + + private static void close(Set formatters) throws Exception { + Exception error = null; + for (Formatter formatter : formatters) { + try { + formatter.close(); + } catch (Exception e) { + if (error == null) { + error = e; + } else { + error.addSuppressed(e); + } + } + } + if (error != null) { + throw error; + } + } +} diff --git a/plugin-maven/src/main/java/com/diffplug/spotless/maven/incremental/IndexBasedChecker.java b/plugin-maven/src/main/java/com/diffplug/spotless/maven/incremental/IndexBasedChecker.java index cb5655be75..16af303b1f 100644 --- a/plugin-maven/src/main/java/com/diffplug/spotless/maven/incremental/IndexBasedChecker.java +++ b/plugin-maven/src/main/java/com/diffplug/spotless/maven/incremental/IndexBasedChecker.java @@ -26,6 +26,8 @@ import org.apache.maven.plugin.logging.Log; import org.apache.maven.project.MavenProject; +import com.diffplug.spotless.Formatter; + // todo: add unit tests class IndexBasedChecker implements UpToDateChecker { @@ -35,8 +37,8 @@ private IndexBasedChecker(FileIndex index) { this.index = index; } - static IndexBasedChecker create(MavenProject project, Log log) { - PluginFingerprint pluginFingerprint = PluginFingerprint.from(project); + static IndexBasedChecker create(MavenProject project, Iterable formatters, Log log) { + PluginFingerprint pluginFingerprint = PluginFingerprint.from(project, formatters); // todo: does this produce the correct dir? Path buildDir = project.getBasedir().toPath().resolve(project.getBuild().getDirectory()); FileIndex fileIndex = FileIndex.read(buildDir, pluginFingerprint, log); diff --git a/plugin-maven/src/main/java/com/diffplug/spotless/maven/incremental/ObjectDigestOutputStream.java b/plugin-maven/src/main/java/com/diffplug/spotless/maven/incremental/ObjectDigestOutputStream.java new file mode 100644 index 0000000000..8ff2c03c9f --- /dev/null +++ b/plugin-maven/src/main/java/com/diffplug/spotless/maven/incremental/ObjectDigestOutputStream.java @@ -0,0 +1,57 @@ +/* + * 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.spotless.maven.incremental; + +import java.io.IOException; +import java.io.ObjectOutputStream; +import java.io.OutputStream; +import java.security.DigestOutputStream; +import java.security.MessageDigest; +import java.security.NoSuchAlgorithmException; + +class ObjectDigestOutputStream extends ObjectOutputStream { + + private final MessageDigest messageDigest; + + private ObjectDigestOutputStream(DigestOutputStream out) throws IOException { + super(out); + messageDigest = out.getMessageDigest(); + } + + static ObjectDigestOutputStream create() throws IOException { + return new ObjectDigestOutputStream(createDigestOutputStream()); + } + + byte[] digest() { + return messageDigest.digest(); + } + + private static DigestOutputStream createDigestOutputStream() { + OutputStream nullOutputStream = new OutputStream() { + @Override + public void write(int b) {} + }; + + MessageDigest result; + try { + result = MessageDigest.getInstance("SHA-256"); + } catch (NoSuchAlgorithmException e) { + throw new IllegalStateException("SHA-256 digest algorithm not available", e); + } + + return new DigestOutputStream(nullOutputStream, result); + } +} diff --git a/plugin-maven/src/main/java/com/diffplug/spotless/maven/incremental/PluginFingerprint.java b/plugin-maven/src/main/java/com/diffplug/spotless/maven/incremental/PluginFingerprint.java index 39366e3e48..c5fc431c39 100644 --- a/plugin-maven/src/main/java/com/diffplug/spotless/maven/incremental/PluginFingerprint.java +++ b/plugin-maven/src/main/java/com/diffplug/spotless/maven/incremental/PluginFingerprint.java @@ -15,18 +15,16 @@ */ package com.diffplug.spotless.maven.incremental; -import java.io.ByteArrayOutputStream; import java.io.IOException; -import java.io.ObjectOutputStream; import java.io.UncheckedIOException; -import java.security.MessageDigest; -import java.security.NoSuchAlgorithmException; import java.util.Base64; import java.util.Objects; import org.apache.maven.model.Plugin; import org.apache.maven.project.MavenProject; +import com.diffplug.spotless.Formatter; + class PluginFingerprint { private static final String SPOTLESS_PLUGIN_KEY = "com.diffplug.spotless:spotless-maven-plugin"; @@ -37,10 +35,9 @@ private PluginFingerprint(String value) { this.value = value; } - static PluginFingerprint from(MavenProject project) { + static PluginFingerprint from(MavenProject project, Iterable formatters) { Plugin spotlessPlugin = project.getPlugin(SPOTLESS_PLUGIN_KEY); - byte[] serialized = serialize(spotlessPlugin); - byte[] digest = digest(serialized); + byte[] digest = digest(spotlessPlugin, formatters); String value = Base64.getEncoder().encodeToString(digest); return new PluginFingerprint(value); } @@ -75,20 +72,14 @@ public String toString() { return "PluginFingerprint[" + value + "]"; } - private static byte[] digest(byte[] bytes) { - try { - return MessageDigest.getInstance("SHA-256").digest(bytes); - } catch (NoSuchAlgorithmException e) { - throw new IllegalStateException("SHA-256 digest algorithm not available", e); - } - } - - private static byte[] serialize(Plugin plugin) { - try (ByteArrayOutputStream byteStream = new ByteArrayOutputStream(); - ObjectOutputStream objectStream = new ObjectOutputStream(byteStream)) { - objectStream.writeObject(plugin); - objectStream.flush(); - return byteStream.toByteArray(); + private static byte[] digest(Plugin plugin, Iterable formatters) { + try (ObjectDigestOutputStream out = ObjectDigestOutputStream.create()) { + out.writeObject(plugin); + for (Formatter formatter : formatters) { + out.writeObject(formatter); + } + out.flush(); + return out.digest(); } catch (IOException e) { throw new UncheckedIOException("Unable to serialize plugin " + plugin, e); } diff --git a/plugin-maven/src/main/java/com/diffplug/spotless/maven/incremental/UpToDateChecker.java b/plugin-maven/src/main/java/com/diffplug/spotless/maven/incremental/UpToDateChecker.java index cc865fe50a..aa9536186f 100644 --- a/plugin-maven/src/main/java/com/diffplug/spotless/maven/incremental/UpToDateChecker.java +++ b/plugin-maven/src/main/java/com/diffplug/spotless/maven/incremental/UpToDateChecker.java @@ -20,6 +20,8 @@ import org.apache.maven.plugin.logging.Log; import org.apache.maven.project.MavenProject; +import com.diffplug.spotless.Formatter; + public interface UpToDateChecker extends AutoCloseable { boolean isUpToDate(File file); @@ -32,7 +34,7 @@ static UpToDateChecker noop() { return NoopChecker.INSTANCE; } - static UpToDateChecker forProject(MavenProject project, Log log) { - return IndexBasedChecker.create(project, log); + static UpToDateChecker forProject(MavenProject project, Iterable formatters, Log log) { + return IndexBasedChecker.create(project, formatters, log); } } diff --git a/plugin-maven/src/test/java/com/diffplug/spotless/maven/incremental/PluginFingerprintTest.java b/plugin-maven/src/test/java/com/diffplug/spotless/maven/incremental/PluginFingerprintTest.java index ba7bf52471..2a7675026f 100644 --- a/plugin-maven/src/test/java/com/diffplug/spotless/maven/incremental/PluginFingerprintTest.java +++ b/plugin-maven/src/test/java/com/diffplug/spotless/maven/incremental/PluginFingerprintTest.java @@ -16,10 +16,14 @@ package com.diffplug.spotless.maven.incremental; import static java.nio.charset.StandardCharsets.UTF_8; +import static java.util.Collections.singletonList; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertNotEquals; import java.io.ByteArrayInputStream; +import java.nio.file.Paths; +import java.util.Arrays; +import java.util.List; import org.apache.maven.model.Model; import org.apache.maven.model.io.xpp3.MavenXpp3Reader; @@ -28,6 +32,10 @@ import org.codehaus.plexus.util.xml.XmlStreamReader; import org.junit.jupiter.api.Test; +import com.diffplug.spotless.FormatExceptionPolicyStrict; +import com.diffplug.spotless.Formatter; +import com.diffplug.spotless.FormatterStep; +import com.diffplug.spotless.LineEnding; import com.diffplug.spotless.maven.MavenIntegrationHarness; class PluginFingerprintTest extends MavenIntegrationHarness { @@ -57,6 +65,8 @@ class PluginFingerprintTest extends MavenIntegrationHarness { "" }; + private static final List FORMATTERS = singletonList(formatter(formatterStep("default"))); + @Test void sameFingerprint() throws Exception { String xml1 = createPomXmlContent(VERSION_1, EXECUTION_1, CONFIGURATION_1); @@ -65,8 +75,8 @@ void sameFingerprint() throws Exception { MavenProject project1 = mavenProject(xml1); MavenProject project2 = mavenProject(xml2); - PluginFingerprint fingerprint1 = PluginFingerprint.from(project1); - PluginFingerprint fingerprint2 = PluginFingerprint.from(project2); + PluginFingerprint fingerprint1 = PluginFingerprint.from(project1, FORMATTERS); + PluginFingerprint fingerprint2 = PluginFingerprint.from(project2, FORMATTERS); assertEquals(fingerprint1, fingerprint2); } @@ -79,8 +89,8 @@ void differentFingerprintForDifferentPluginVersion() throws Exception { MavenProject project1 = mavenProject(xml1); MavenProject project2 = mavenProject(xml2); - PluginFingerprint fingerprint1 = PluginFingerprint.from(project1); - PluginFingerprint fingerprint2 = PluginFingerprint.from(project2); + PluginFingerprint fingerprint1 = PluginFingerprint.from(project1, FORMATTERS); + PluginFingerprint fingerprint2 = PluginFingerprint.from(project2, FORMATTERS); assertNotEquals(fingerprint1, fingerprint2); } @@ -93,8 +103,8 @@ void differentFingerprintForDifferentExecution() throws Exception { MavenProject project1 = mavenProject(xml1); MavenProject project2 = mavenProject(xml2); - PluginFingerprint fingerprint1 = PluginFingerprint.from(project1); - PluginFingerprint fingerprint2 = PluginFingerprint.from(project2); + PluginFingerprint fingerprint1 = PluginFingerprint.from(project1, FORMATTERS); + PluginFingerprint fingerprint2 = PluginFingerprint.from(project2, FORMATTERS); assertNotEquals(fingerprint1, fingerprint2); } @@ -107,8 +117,46 @@ void differentFingerprintForDifferentConfiguration() throws Exception { MavenProject project1 = mavenProject(xml1); MavenProject project2 = mavenProject(xml2); - PluginFingerprint fingerprint1 = PluginFingerprint.from(project1); - PluginFingerprint fingerprint2 = PluginFingerprint.from(project2); + PluginFingerprint fingerprint1 = PluginFingerprint.from(project1, FORMATTERS); + PluginFingerprint fingerprint2 = PluginFingerprint.from(project2, FORMATTERS); + + assertNotEquals(fingerprint1, fingerprint2); + } + + @Test + void differentFingerprintForFormattersWithDifferentSteps() throws Exception { + String xml1 = createPomXmlContent(VERSION_1, EXECUTION_1, CONFIGURATION_1); + String xml2 = createPomXmlContent(VERSION_1, EXECUTION_1, CONFIGURATION_1); + + MavenProject project1 = mavenProject(xml1); + MavenProject project2 = mavenProject(xml2); + + FormatterStep step1 = formatterStep("step1"); + FormatterStep step2 = formatterStep("step2"); + FormatterStep step3 = formatterStep("step3"); + List formatters1 = singletonList(formatter(step1, step2)); + List formatters2 = singletonList(formatter(step2, step3)); + + PluginFingerprint fingerprint1 = PluginFingerprint.from(project1, formatters1); + PluginFingerprint fingerprint2 = PluginFingerprint.from(project2, formatters2); + + assertNotEquals(fingerprint1, fingerprint2); + } + + @Test + void differentFingerprintForFormattersWithDifferentLineEndings() throws Exception { + String xml1 = createPomXmlContent(VERSION_1, EXECUTION_1, CONFIGURATION_1); + String xml2 = createPomXmlContent(VERSION_1, EXECUTION_1, CONFIGURATION_1); + + MavenProject project1 = mavenProject(xml1); + MavenProject project2 = mavenProject(xml2); + + FormatterStep step = formatterStep("step"); + List formatters1 = singletonList(formatter(LineEnding.UNIX, step)); + List formatters2 = singletonList(formatter(LineEnding.WINDOWS, step)); + + PluginFingerprint fingerprint1 = PluginFingerprint.from(project1, formatters1); + PluginFingerprint fingerprint2 = PluginFingerprint.from(project2, formatters2); assertNotEquals(fingerprint1, fingerprint2); } @@ -124,4 +172,22 @@ private static Model readPom(String xml) throws Exception { return pomReader.read(xmlReader); } } + + private static FormatterStep formatterStep(String name) { + return FormatterStep.createNeverUpToDate(name, input -> input); + } + + private static Formatter formatter(FormatterStep... steps) { + return formatter(LineEnding.UNIX, steps); + } + + private static Formatter formatter(LineEnding lineEnding, FormatterStep... steps) { + return Formatter.builder() + .rootDir(Paths.get("")) + .lineEndingsPolicy(lineEnding.createPolicy()) + .encoding(UTF_8) + .steps(Arrays.asList(steps)) + .exceptionPolicy(new FormatExceptionPolicyStrict()) + .build(); + } } From f0e2d3eac35e16ef787d0009150b0eca19a9bc1e Mon Sep 17 00:00:00 2001 From: Kostiantyn Liutovych Date: Mon, 15 Nov 2021 15:53:55 +0100 Subject: [PATCH 04/21] Lazily collect unformatted files for each formatter --- .../spotless/maven/AbstractSpotlessMojo.java | 30 +++++++++-------- .../spotless/maven/FormatterFactory.java | 5 +-- .../spotless/maven/FormattersHolder.java | 16 ++++----- .../spotless/maven/PluginException.java | 33 +++++++++++++++++++ 4 files changed, 61 insertions(+), 23 deletions(-) create mode 100644 plugin-maven/src/main/java/com/diffplug/spotless/maven/PluginException.java diff --git a/plugin-maven/src/main/java/com/diffplug/spotless/maven/AbstractSpotlessMojo.java b/plugin-maven/src/main/java/com/diffplug/spotless/maven/AbstractSpotlessMojo.java index 6e88ab6024..5a82f15fb3 100644 --- a/plugin-maven/src/main/java/com/diffplug/spotless/maven/AbstractSpotlessMojo.java +++ b/plugin-maven/src/main/java/com/diffplug/spotless/maven/AbstractSpotlessMojo.java @@ -31,6 +31,7 @@ import java.util.Optional; import java.util.Set; import java.util.function.Predicate; +import java.util.function.Supplier; import java.util.regex.Pattern; import java.util.stream.Collectors; import java.util.stream.Stream; @@ -171,17 +172,21 @@ public final void execute() throws MojoExecutionException { List formatterFactories = getFormatterFactories(); FormatterConfig config = getFormatterConfig(); - Map> formatterFactoryToFiles = new HashMap<>(); + Map>> formatterFactoryToFiles = new HashMap<>(); for (FormatterFactory formatterFactory : formatterFactories) { - List files = collectFiles(formatterFactory, config); - formatterFactoryToFiles.put(formatterFactory, files); + Supplier> filesToFormat = () -> collectFiles(formatterFactory, config); + formatterFactoryToFiles.put(formatterFactory, filesToFormat); } try (FormattersHolder formattersHolder = FormattersHolder.create(formatterFactoryToFiles, config); UpToDateChecker upToDateChecker = createUpToDateChecker(formattersHolder.getFormatters())) { - for (Entry> entry : formattersHolder.getFormattersWithFiles().entrySet()) { - process(entry.getValue(), entry.getKey(), upToDateChecker); + for (Entry>> entry : formattersHolder.getFormattersWithFiles().entrySet()) { + Formatter formatter = entry.getKey(); + Iterable files = entry.getValue().get(); + process(files, formatter, upToDateChecker); } + } catch (PluginException e) { + throw e.asMojoExecutionException(); } } @@ -197,7 +202,7 @@ private boolean shouldSkip() { return false; } - private List collectFiles(FormatterFactory formatterFactory, FormatterConfig config) throws MojoExecutionException { + private List collectFiles(FormatterFactory formatterFactory, FormatterConfig config) { Optional ratchetFrom = formatterFactory.ratchetFrom(config); try { final List files; @@ -222,11 +227,11 @@ private List collectFiles(FormatterFactory formatterFactory, FormatterConf .filter(shouldInclude) .collect(toList()); } catch (IOException e) { - throw new MojoExecutionException("Unable to scan file tree rooted at " + baseDir, e); + throw new PluginException("Unable to scan file tree rooted at " + baseDir, e); } } - private List collectFilesFromGit(FormatterFactory formatterFactory, String ratchetFrom) throws MojoExecutionException { + private List collectFilesFromGit(FormatterFactory formatterFactory, String ratchetFrom) { MatchPatterns includePatterns = MatchPatterns.from( withNormalizedFileSeparators(getIncludes(formatterFactory))); MatchPatterns excludePatterns = MatchPatterns.from( @@ -237,7 +242,7 @@ private List collectFilesFromGit(FormatterFactory formatterFactory, String dirtyFiles = GitRatchetMaven .instance().getDirtyFiles(baseDir, ratchetFrom); } catch (IOException e) { - throw new MojoExecutionException("Unable to scan file tree rooted at " + baseDir, e); + throw new PluginException("Unable to scan file tree rooted at " + baseDir, e); } List result = new ArrayList<>(); @@ -251,8 +256,7 @@ private List collectFilesFromGit(FormatterFactory formatterFactory, String return result; } - private List collectFilesFromFormatterFactory(FormatterFactory formatterFactory) - throws MojoExecutionException, IOException { + private List collectFilesFromFormatterFactory(FormatterFactory formatterFactory) throws IOException { String includesString = String.join(",", getIncludes(formatterFactory)); String excludesString = String.join(",", getExcludes(formatterFactory)); @@ -270,11 +274,11 @@ private static String withTrailingSeparator(String path) { return path.endsWith(File.separator) ? path : path + File.separator; } - private Set getIncludes(FormatterFactory formatterFactory) throws MojoExecutionException { + private Set getIncludes(FormatterFactory formatterFactory) { Set configuredIncludes = formatterFactory.includes(); Set includes = configuredIncludes.isEmpty() ? formatterFactory.defaultIncludes() : configuredIncludes; if (includes.isEmpty()) { - throw new MojoExecutionException("You must specify some files to include, such as 'src/**'"); + throw new PluginException("You must specify some files to include, such as 'src/**'"); } return includes; } diff --git a/plugin-maven/src/main/java/com/diffplug/spotless/maven/FormatterFactory.java b/plugin-maven/src/main/java/com/diffplug/spotless/maven/FormatterFactory.java index e15c987ffa..c06822486c 100644 --- a/plugin-maven/src/main/java/com/diffplug/spotless/maven/FormatterFactory.java +++ b/plugin-maven/src/main/java/com/diffplug/spotless/maven/FormatterFactory.java @@ -24,6 +24,7 @@ import java.util.Objects; import java.util.Optional; import java.util.Set; +import java.util.function.Supplier; import java.util.stream.Collectors; import org.apache.maven.plugins.annotations.Parameter; @@ -71,10 +72,10 @@ public final Set excludes() { return excludes == null ? emptySet() : Sets.newHashSet(excludes); } - public final Formatter newFormatter(List filesToFormat, FormatterConfig config) { + public final Formatter newFormatter(Supplier> filesToFormat, FormatterConfig config) { Charset formatterEncoding = encoding(config); LineEnding formatterLineEndings = lineEndings(config); - LineEnding.Policy formatterLineEndingPolicy = formatterLineEndings.createPolicy(config.getFileLocator().getBaseDir(), () -> filesToFormat); + LineEnding.Policy formatterLineEndingPolicy = formatterLineEndings.createPolicy(config.getFileLocator().getBaseDir(), filesToFormat); FormatterStepConfig stepConfig = stepConfig(formatterEncoding, config); List factories = gatherStepFactories(config.getGlobalStepFactories(), stepFactories); diff --git a/plugin-maven/src/main/java/com/diffplug/spotless/maven/FormattersHolder.java b/plugin-maven/src/main/java/com/diffplug/spotless/maven/FormattersHolder.java index 7fa15150cc..1c226423f6 100644 --- a/plugin-maven/src/main/java/com/diffplug/spotless/maven/FormattersHolder.java +++ b/plugin-maven/src/main/java/com/diffplug/spotless/maven/FormattersHolder.java @@ -17,27 +17,27 @@ import java.io.File; import java.util.HashMap; -import java.util.List; import java.util.Map; import java.util.Map.Entry; import java.util.Set; +import java.util.function.Supplier; import com.diffplug.spotless.Formatter; class FormattersHolder implements AutoCloseable { - private final Map> formatterToFiles; + private final Map>> formatterToFiles; - private FormattersHolder(Map> formatterToFiles) { + FormattersHolder(Map>> formatterToFiles) { this.formatterToFiles = formatterToFiles; } - static FormattersHolder create(Map> formatterFactoryToFiles, FormatterConfig config) { - Map> formatterToFiles = new HashMap<>(); + static FormattersHolder create(Map>> formatterFactoryToFiles, FormatterConfig config) { + Map>> formatterToFiles = new HashMap<>(); try { - for (Entry> entry : formatterFactoryToFiles.entrySet()) { + for (Entry>> entry : formatterFactoryToFiles.entrySet()) { FormatterFactory formatterFactory = entry.getKey(); - List files = entry.getValue(); + Supplier> files = entry.getValue(); Formatter formatter = formatterFactory.newFormatter(files, config); formatterToFiles.put(formatter, files); @@ -58,7 +58,7 @@ Iterable getFormatters() { return formatterToFiles.keySet(); } - Map> getFormattersWithFiles() { + Map>> getFormattersWithFiles() { return formatterToFiles; } diff --git a/plugin-maven/src/main/java/com/diffplug/spotless/maven/PluginException.java b/plugin-maven/src/main/java/com/diffplug/spotless/maven/PluginException.java new file mode 100644 index 0000000000..b7594939c7 --- /dev/null +++ b/plugin-maven/src/main/java/com/diffplug/spotless/maven/PluginException.java @@ -0,0 +1,33 @@ +/* + * 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.spotless.maven; + +import org.apache.maven.plugin.MojoExecutionException; + +class PluginException extends RuntimeException { + + PluginException(String message) { + super(message); + } + + PluginException(String message, Throwable cause) { + super(message, cause); + } + + MojoExecutionException asMojoExecutionException() { + return new MojoExecutionException(getMessage(), getCause()); + } +} From c463a588b51cd8b01182b56051c3a85b1c087025 Mon Sep 17 00:00:00 2001 From: Kostiantyn Liutovych Date: Mon, 15 Nov 2021 21:41:00 +0100 Subject: [PATCH 05/21] Add remote debugging for maven plugin tests It is useful for local development only. --- .../spotless/maven/MavenIntegrationHarness.java | 10 ++++++++++ .../java/com/diffplug/spotless/maven/MavenRunner.java | 11 +++++++++++ 2 files changed, 21 insertions(+) diff --git a/plugin-maven/src/test/java/com/diffplug/spotless/maven/MavenIntegrationHarness.java b/plugin-maven/src/test/java/com/diffplug/spotless/maven/MavenIntegrationHarness.java index b213b67731..a5dad0b904 100644 --- a/plugin-maven/src/test/java/com/diffplug/spotless/maven/MavenIntegrationHarness.java +++ b/plugin-maven/src/test/java/com/diffplug/spotless/maven/MavenIntegrationHarness.java @@ -59,6 +59,7 @@ public class MavenIntegrationHarness extends ResourceHarness { private static final String MODULES = "modules"; private static final String MODULE_NAME = "name"; private static final String CHILD_ID = "childId"; + private static final int REMOTE_DEBUG_PORT = 5005; private final MustacheFactory mustacheFactory = new DefaultMustacheFactory(); @@ -153,6 +154,15 @@ protected MavenRunner mavenRunner() throws IOException { .withLocalRepository(new File(getSystemProperty(LOCAL_MAVEN_REPOSITORY_DIR))); } + /** + * Useful for local development. Allows debugging the Spotless Maven Plugin remotely. + * Effectively translates into running {@code mvnDebug} on port 5005. The forked JVM will be + * suspended until the debugger connects. + */ + protected MavenRunner mavenRunnerWithRemoteDebug() throws IOException { + return mavenRunner().withRemoteDebug(REMOTE_DEBUG_PORT); + } + protected MultiModuleProjectCreator multiModuleProject() { return new MultiModuleProjectCreator(); } diff --git a/plugin-maven/src/test/java/com/diffplug/spotless/maven/MavenRunner.java b/plugin-maven/src/test/java/com/diffplug/spotless/maven/MavenRunner.java index aebf3265fc..a86e000187 100644 --- a/plugin-maven/src/test/java/com/diffplug/spotless/maven/MavenRunner.java +++ b/plugin-maven/src/test/java/com/diffplug/spotless/maven/MavenRunner.java @@ -23,12 +23,15 @@ import java.io.InputStream; import java.nio.charset.Charset; import java.util.Arrays; +import java.util.HashMap; import java.util.List; +import java.util.Map; import java.util.Objects; import com.diffplug.common.base.Throwables; import com.diffplug.common.io.ByteStreams; import com.diffplug.spotless.FileSignature; +import com.diffplug.spotless.Jvm; /** * Harness for running a maven build, same idea as the @@ -44,6 +47,7 @@ private MavenRunner() {} private File projectDir; private String[] args; private File localRepositoryDir; + private Map environment = new HashMap<>(); public MavenRunner withProjectDir(File projectDir) { this.projectDir = Objects.requireNonNull(projectDir); @@ -60,6 +64,12 @@ public MavenRunner withLocalRepository(File localRepositoryDir) { return this; } + public MavenRunner withRemoteDebug(int port) { + String address = (Jvm.version() < 9 ? "" : "*:") + port; + environment.put("MAVEN_OPTS", "-agentlib:jdwp=transport=dt_socket,server=y,suspend=y,address=" + address); + return this; + } + private Result run() throws IOException, InterruptedException { Objects.requireNonNull(projectDir, "Need to call withProjectDir() first"); Objects.requireNonNull(args, "Need to call withArguments() first"); @@ -68,6 +78,7 @@ private Result run() throws IOException, InterruptedException { List cmds = getPlatformCmds("-e -Dmaven.repo.local=" + localRepositoryDir + ' ' + argsString); ProcessBuilder builder = new ProcessBuilder(cmds); builder.directory(projectDir); + builder.environment().putAll(environment); Process process = builder.start(); // slurp and return the stdout, stderr, and exitValue Slurper output = new Slurper(process.getInputStream()); From b1d8524db671b51cb3b4d176adf8dd17c40000de Mon Sep 17 00:00:00 2001 From: Kostiantyn Liutovych Date: Mon, 15 Nov 2021 21:46:09 +0100 Subject: [PATCH 06/21] Improve up-to-date checking XML config Add a dedicated POM XML element for up-to-date checking: ``` com.diffplug.spotless spotless-maven-plugin ${spotless.version} true ``` It makes the configuration more flexible and allows adding more options related to up-to-date checking in the future. --- .../spotless/maven/AbstractSpotlessMojo.java | 9 ++-- .../maven/incremental/UpToDateChecking.java | 28 +++++++++++ .../incremental/UpToDateCheckingTest.java | 49 +++++++++++++++++++ 3 files changed, 82 insertions(+), 4 deletions(-) create mode 100644 plugin-maven/src/main/java/com/diffplug/spotless/maven/incremental/UpToDateChecking.java create mode 100644 plugin-maven/src/test/java/com/diffplug/spotless/maven/incremental/UpToDateCheckingTest.java diff --git a/plugin-maven/src/main/java/com/diffplug/spotless/maven/AbstractSpotlessMojo.java b/plugin-maven/src/main/java/com/diffplug/spotless/maven/AbstractSpotlessMojo.java index 5a82f15fb3..078725b8af 100644 --- a/plugin-maven/src/main/java/com/diffplug/spotless/maven/AbstractSpotlessMojo.java +++ b/plugin-maven/src/main/java/com/diffplug/spotless/maven/AbstractSpotlessMojo.java @@ -60,6 +60,7 @@ import com.diffplug.spotless.maven.generic.LicenseHeader; import com.diffplug.spotless.maven.groovy.Groovy; import com.diffplug.spotless.maven.incremental.UpToDateChecker; +import com.diffplug.spotless.maven.incremental.UpToDateChecking; import com.diffplug.spotless.maven.java.Java; import com.diffplug.spotless.maven.kotlin.Kotlin; import com.diffplug.spotless.maven.pom.Pom; @@ -156,9 +157,8 @@ public abstract class AbstractSpotlessMojo extends AbstractMojo { @Parameter(property = LicenseHeaderStep.spotlessSetLicenseHeaderYearsFromGitHistory) private String setLicenseHeaderYearsFromGitHistory; - // todo: check naming convensions for plugin properties - @Parameter(property = "spotless.upToDateCheckingEnabled", defaultValue = "false") - private boolean upToDateCheckingEnabled; + @Parameter + private UpToDateChecking upToDateChecking; protected abstract void process(Iterable files, Formatter formatter, UpToDateChecker upToDateChecker) throws MojoExecutionException; @@ -320,7 +320,8 @@ private List getFormatterStepFactories() { } private UpToDateChecker createUpToDateChecker(Iterable formatters) { - if (upToDateCheckingEnabled) { + if (upToDateChecking.isEnabled()) { + getLog().info("Up-to-date checking enabled"); return UpToDateChecker.forProject(project, formatters, getLog()); } return UpToDateChecker.noop(); diff --git a/plugin-maven/src/main/java/com/diffplug/spotless/maven/incremental/UpToDateChecking.java b/plugin-maven/src/main/java/com/diffplug/spotless/maven/incremental/UpToDateChecking.java new file mode 100644 index 0000000000..098d24fb81 --- /dev/null +++ b/plugin-maven/src/main/java/com/diffplug/spotless/maven/incremental/UpToDateChecking.java @@ -0,0 +1,28 @@ +/* + * 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.spotless.maven.incremental; + +import org.apache.maven.plugins.annotations.Parameter; + +public class UpToDateChecking { + + @Parameter + private boolean enabled; + + public boolean isEnabled() { + return enabled; + } +} diff --git a/plugin-maven/src/test/java/com/diffplug/spotless/maven/incremental/UpToDateCheckingTest.java b/plugin-maven/src/test/java/com/diffplug/spotless/maven/incremental/UpToDateCheckingTest.java new file mode 100644 index 0000000000..0b8b0ad252 --- /dev/null +++ b/plugin-maven/src/test/java/com/diffplug/spotless/maven/incremental/UpToDateCheckingTest.java @@ -0,0 +1,49 @@ +/* + * 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.spotless.maven.incremental; + +import static org.assertj.core.api.Assertions.assertThat; + +import org.junit.jupiter.api.Test; + +import com.diffplug.spotless.maven.MavenIntegrationHarness; +import com.diffplug.spotless.maven.MavenRunner.Result; + +class UpToDateCheckingTest extends MavenIntegrationHarness { + + @Test + void enableUpToDateChecking() throws Exception { + writePom( + "", + " ", + "", + "", + " true", + ""); + + String output = runTest("java/googlejavaformat/JavaCodeFormatted18.test"); + + assertThat(output).contains("Up-to-date checking enabled"); + } + + private String runTest(String targetResource) throws Exception { + String path = "src/main/java/test.java"; + setFile(path).toResource("java/googlejavaformat/JavaCodeUnformatted.test"); + Result result = mavenRunner().withArguments("spotless:apply").runNoError(); + assertFile(path).sameAsResource(targetResource); + return result.output(); + } +} From 839b4bdf3c67e9c2eb124dcb07fec6a764972355 Mon Sep 17 00:00:00 2001 From: Kostiantyn Liutovych Date: Mon, 15 Nov 2021 21:55:09 +0100 Subject: [PATCH 07/21] Simplify FileIndex construction Introduce `FileIndexConfig` that abstracts away some directory manipulations and file naming. Fix two issues: * ensure the parent dir for the index file exists before writing the file itself * relativize files against the project dir, not against the target dir --- .../spotless/maven/incremental/FileIndex.java | 53 ++++++++++--------- .../maven/incremental/FileIndexConfig.java | 46 ++++++++++++++++ .../maven/incremental/IndexBasedChecker.java | 5 +- 3 files changed, 76 insertions(+), 28 deletions(-) create mode 100644 plugin-maven/src/main/java/com/diffplug/spotless/maven/incremental/FileIndexConfig.java diff --git a/plugin-maven/src/main/java/com/diffplug/spotless/maven/incremental/FileIndex.java b/plugin-maven/src/main/java/com/diffplug/spotless/maven/incremental/FileIndex.java index d7db521f84..3211b1ea97 100644 --- a/plugin-maven/src/main/java/com/diffplug/spotless/maven/incremental/FileIndex.java +++ b/plugin-maven/src/main/java/com/diffplug/spotless/maven/incremental/FileIndex.java @@ -29,6 +29,7 @@ import java.nio.file.Path; import java.nio.file.Paths; import java.time.Instant; +import java.time.format.DateTimeParseException; import java.util.Map; import java.util.Map.Entry; import java.util.Optional; @@ -41,52 +42,54 @@ // todo: add unit tests class FileIndex { - private static final String INDEX_FILE_NAME = "spotless-index"; private static final String SEPARATOR = " "; + private final Path indexFile; private final PluginFingerprint pluginFingerprint; - private final Path baseDir; private final Map fileToLastModifiedTime; + private final Path projectDir; + private boolean updated; - private FileIndex(PluginFingerprint pluginFingerprint, Path baseDir, Map fileToLastModifiedTime) { + private FileIndex(Path indexFile, PluginFingerprint pluginFingerprint, Map fileToLastModifiedTime, Path projectDir) { + this.indexFile = indexFile; this.pluginFingerprint = pluginFingerprint; - this.baseDir = baseDir; this.fileToLastModifiedTime = fileToLastModifiedTime; + this.projectDir = projectDir; } - static FileIndex read(Path baseDir, PluginFingerprint pluginFingerprint, Log log) { - Path indexFile = indexFile(baseDir); + static FileIndex read(FileIndexConfig config, Log log) { + Path indexFile = config.getIndexFile(); if (Files.notExists(indexFile)) { log.info("Index file does not exist. Fallback to an empty index"); - return emptyIndex(baseDir, pluginFingerprint); + return emptyIndex(config); } try (BufferedReader reader = newBufferedReader(indexFile, UTF_8)) { PluginFingerprint actualPluginFingerprint = PluginFingerprint.from(reader.readLine()); - if (!pluginFingerprint.equals(actualPluginFingerprint)) { + if (!config.getPluginFingerprint().equals(actualPluginFingerprint)) { log.info("Fingerprint mismatch in the index file. Fallback to an empty index"); - return emptyIndex(baseDir, pluginFingerprint); + return emptyIndex(config); } else { - Map fileToLastModifiedTime = readFileToLastModifiedTime(reader, baseDir, log); - return new FileIndex(pluginFingerprint, baseDir, fileToLastModifiedTime); + Map fileToLastModifiedTime = readFileToLastModifiedTime(reader, config.getProjectDir(), log); + return new FileIndex(indexFile, config.getPluginFingerprint(), fileToLastModifiedTime, config.getProjectDir()); } } catch (IOException e) { log.warn("Error reading the index file. Fallback to an empty index", e); - return emptyIndex(baseDir, pluginFingerprint); + return emptyIndex(config); } } Optional getLastModifiedTime(Path file) { - if (!file.startsWith(baseDir)) { + if (!file.startsWith(projectDir)) { return Optional.empty(); } - Path relativeFile = baseDir.relativize(file); + Path relativeFile = projectDir.relativize(file); return Optional.ofNullable(fileToLastModifiedTime.get(relativeFile)); } void setLastModifiedTime(Path file, Instant time) { - Path relativeFile = baseDir.relativize(file); + Path relativeFile = projectDir.relativize(file); fileToLastModifiedTime.put(relativeFile, time); updated = true; } @@ -96,7 +99,11 @@ void write() { return; } - Path indexFile = indexFile(baseDir); + try { + Files.createDirectories(indexFile.getParent()); + } catch (IOException e) { + throw new UncheckedIOException("Unable to create parent directory for the index file: " + indexFile, e); + } try (PrintWriter writer = new PrintWriter(newBufferedWriter(indexFile, UTF_8, CREATE, TRUNCATE_EXISTING))) { writer.println(pluginFingerprint.value()); @@ -108,7 +115,7 @@ void write() { } } - private static Map readFileToLastModifiedTime(BufferedReader reader, Path baseDir, Log log) throws IOException { + private static Map readFileToLastModifiedTime(BufferedReader reader, Path projectDir, Log log) throws IOException { Map fileToLastModifiedTime = new TreeMap<>(); String line; while ((line = reader.readLine()) != null) { @@ -118,14 +125,14 @@ private static Map readFileToLastModifiedTime(BufferedReader read } Path relativeFile = Paths.get(line.substring(0, separatorIndex)); - Path absoluteFile = baseDir.resolve(relativeFile); + Path absoluteFile = projectDir.resolve(relativeFile); if (Files.notExists(absoluteFile)) { log.info("File stored in the index does not exist: " + relativeFile); } else { Instant lastModifiedTime; try { lastModifiedTime = Instant.parse(line.substring(separatorIndex)); - } catch (Exception e) { + } catch (DateTimeParseException e) { throw new IOException("Incorrect index file. Unable to parse last modified time from '" + line + "'", e); } fileToLastModifiedTime.put(relativeFile, lastModifiedTime); @@ -134,11 +141,7 @@ private static Map readFileToLastModifiedTime(BufferedReader read return fileToLastModifiedTime; } - private static Path indexFile(Path baseDir) { - return baseDir.resolve(INDEX_FILE_NAME); - } - - private static FileIndex emptyIndex(Path baseDir, PluginFingerprint pluginFingerprint) { - return new FileIndex(pluginFingerprint, baseDir, new TreeMap<>()); + private static FileIndex emptyIndex(FileIndexConfig config) { + return new FileIndex(config.getIndexFile(), config.getPluginFingerprint(), new TreeMap<>(), config.getProjectDir()); } } diff --git a/plugin-maven/src/main/java/com/diffplug/spotless/maven/incremental/FileIndexConfig.java b/plugin-maven/src/main/java/com/diffplug/spotless/maven/incremental/FileIndexConfig.java new file mode 100644 index 0000000000..226feec6e1 --- /dev/null +++ b/plugin-maven/src/main/java/com/diffplug/spotless/maven/incremental/FileIndexConfig.java @@ -0,0 +1,46 @@ +/* + * 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.spotless.maven.incremental; + +import java.nio.file.Path; + +import org.apache.maven.project.MavenProject; + +class FileIndexConfig { + + private static final String INDEX_FILE_NAME = "spotless-index"; + + private final MavenProject project; + private final PluginFingerprint pluginFingerprint; + + FileIndexConfig(MavenProject project, PluginFingerprint pluginFingerprint) { + this.project = project; + this.pluginFingerprint = pluginFingerprint; + } + + Path getProjectDir() { + return project.getBasedir().toPath(); + } + + Path getIndexFile() { + Path targetDir = getProjectDir().resolve(project.getBuild().getDirectory()); + return targetDir.resolve(INDEX_FILE_NAME); + } + + PluginFingerprint getPluginFingerprint() { + return pluginFingerprint; + } +} diff --git a/plugin-maven/src/main/java/com/diffplug/spotless/maven/incremental/IndexBasedChecker.java b/plugin-maven/src/main/java/com/diffplug/spotless/maven/incremental/IndexBasedChecker.java index 16af303b1f..25b675b14a 100644 --- a/plugin-maven/src/main/java/com/diffplug/spotless/maven/incremental/IndexBasedChecker.java +++ b/plugin-maven/src/main/java/com/diffplug/spotless/maven/incremental/IndexBasedChecker.java @@ -39,9 +39,8 @@ private IndexBasedChecker(FileIndex index) { static IndexBasedChecker create(MavenProject project, Iterable formatters, Log log) { PluginFingerprint pluginFingerprint = PluginFingerprint.from(project, formatters); - // todo: does this produce the correct dir? - Path buildDir = project.getBasedir().toPath().resolve(project.getBuild().getDirectory()); - FileIndex fileIndex = FileIndex.read(buildDir, pluginFingerprint, log); + FileIndexConfig indexConfig = new FileIndexConfig(project, pluginFingerprint); + FileIndex fileIndex = FileIndex.read(indexConfig, log); return new IndexBasedChecker(fileIndex); } From d26308eb2c3cae8f0a859f5e4fbf62e3683c4b7b Mon Sep 17 00:00:00 2001 From: Kostiantyn Liutovych Date: Mon, 15 Nov 2021 21:59:19 +0100 Subject: [PATCH 08/21] Improve local variable naming --- .../com/diffplug/spotless/maven/incremental/FileIndex.java | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/plugin-maven/src/main/java/com/diffplug/spotless/maven/incremental/FileIndex.java b/plugin-maven/src/main/java/com/diffplug/spotless/maven/incremental/FileIndex.java index 3211b1ea97..08293cb752 100644 --- a/plugin-maven/src/main/java/com/diffplug/spotless/maven/incremental/FileIndex.java +++ b/plugin-maven/src/main/java/com/diffplug/spotless/maven/incremental/FileIndex.java @@ -66,13 +66,14 @@ static FileIndex read(FileIndexConfig config, Log log) { } try (BufferedReader reader = newBufferedReader(indexFile, UTF_8)) { - PluginFingerprint actualPluginFingerprint = PluginFingerprint.from(reader.readLine()); - if (!config.getPluginFingerprint().equals(actualPluginFingerprint)) { + PluginFingerprint computedFingerprint = config.getPluginFingerprint(); + PluginFingerprint storedFingerprint = PluginFingerprint.from(reader.readLine()); + if (!computedFingerprint.equals(storedFingerprint)) { log.info("Fingerprint mismatch in the index file. Fallback to an empty index"); return emptyIndex(config); } else { Map fileToLastModifiedTime = readFileToLastModifiedTime(reader, config.getProjectDir(), log); - return new FileIndex(indexFile, config.getPluginFingerprint(), fileToLastModifiedTime, config.getProjectDir()); + return new FileIndex(indexFile, computedFingerprint, fileToLastModifiedTime, config.getProjectDir()); } } catch (IOException e) { log.warn("Error reading the index file. Fallback to an empty index", e); From 6200d7a204f9e6eb1cb362189452de88e0568d76 Mon Sep 17 00:00:00 2001 From: Kostiantyn Liutovych Date: Mon, 15 Nov 2021 22:01:05 +0100 Subject: [PATCH 09/21] Extract a method to improve code readability --- .../spotless/maven/incremental/FileIndex.java | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/plugin-maven/src/main/java/com/diffplug/spotless/maven/incremental/FileIndex.java b/plugin-maven/src/main/java/com/diffplug/spotless/maven/incremental/FileIndex.java index 08293cb752..9fb6770ba0 100644 --- a/plugin-maven/src/main/java/com/diffplug/spotless/maven/incremental/FileIndex.java +++ b/plugin-maven/src/main/java/com/diffplug/spotless/maven/incremental/FileIndex.java @@ -100,11 +100,7 @@ void write() { return; } - try { - Files.createDirectories(indexFile.getParent()); - } catch (IOException e) { - throw new UncheckedIOException("Unable to create parent directory for the index file: " + indexFile, e); - } + ensureParentDirExists(); try (PrintWriter writer = new PrintWriter(newBufferedWriter(indexFile, UTF_8, CREATE, TRUNCATE_EXISTING))) { writer.println(pluginFingerprint.value()); @@ -116,6 +112,14 @@ void write() { } } + private void ensureParentDirExists() { + try { + Files.createDirectories(indexFile.getParent()); + } catch (IOException e) { + throw new UncheckedIOException("Unable to create parent directory for the index file: " + indexFile, e); + } + } + private static Map readFileToLastModifiedTime(BufferedReader reader, Path projectDir, Log log) throws IOException { Map fileToLastModifiedTime = new TreeMap<>(); String line; From 1119f1be145a0de5784f08be167b1db25108d30b Mon Sep 17 00:00:00 2001 From: Kostiantyn Liutovych Date: Tue, 16 Nov 2021 10:20:28 +0100 Subject: [PATCH 10/21] Fix NPE on the up-to-date checking mojo parameter --- .../java/com/diffplug/spotless/maven/AbstractSpotlessMojo.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugin-maven/src/main/java/com/diffplug/spotless/maven/AbstractSpotlessMojo.java b/plugin-maven/src/main/java/com/diffplug/spotless/maven/AbstractSpotlessMojo.java index 078725b8af..3173a09963 100644 --- a/plugin-maven/src/main/java/com/diffplug/spotless/maven/AbstractSpotlessMojo.java +++ b/plugin-maven/src/main/java/com/diffplug/spotless/maven/AbstractSpotlessMojo.java @@ -320,7 +320,7 @@ private List getFormatterStepFactories() { } private UpToDateChecker createUpToDateChecker(Iterable formatters) { - if (upToDateChecking.isEnabled()) { + if (upToDateChecking != null && upToDateChecking.isEnabled()) { getLog().info("Up-to-date checking enabled"); return UpToDateChecker.forProject(project, formatters, getLog()); } From 736615795745f61fae904b33e258366d70ad9582 Mon Sep 17 00:00:00 2001 From: Kostiantyn Liutovych Date: Tue, 16 Nov 2021 16:51:10 +0100 Subject: [PATCH 11/21] Delete file index when up-to-date checking is off --- .../spotless/maven/AbstractSpotlessMojo.java | 2 +- .../spotless/maven/SpotlessCheckMojo.java | 1 + .../spotless/maven/incremental/FileIndex.java | 13 ++ .../maven/incremental/FileIndexConfig.java | 4 + .../maven/incremental/NoopChecker.java | 11 +- .../maven/incremental/PluginFingerprint.java | 4 + .../maven/incremental/UpToDateChecker.java | 4 +- .../incremental/FileIndexConfigTest.java | 66 ++++++++++ .../maven/incremental/NoopCheckerTest.java | 121 ++++++++++++++++++ .../incremental/PluginFingerprintTest.java | 22 ++-- 10 files changed, 235 insertions(+), 13 deletions(-) create mode 100644 plugin-maven/src/test/java/com/diffplug/spotless/maven/incremental/FileIndexConfigTest.java create mode 100644 plugin-maven/src/test/java/com/diffplug/spotless/maven/incremental/NoopCheckerTest.java diff --git a/plugin-maven/src/main/java/com/diffplug/spotless/maven/AbstractSpotlessMojo.java b/plugin-maven/src/main/java/com/diffplug/spotless/maven/AbstractSpotlessMojo.java index 3173a09963..6f6636a946 100644 --- a/plugin-maven/src/main/java/com/diffplug/spotless/maven/AbstractSpotlessMojo.java +++ b/plugin-maven/src/main/java/com/diffplug/spotless/maven/AbstractSpotlessMojo.java @@ -324,6 +324,6 @@ private UpToDateChecker createUpToDateChecker(Iterable formatters) { getLog().info("Up-to-date checking enabled"); return UpToDateChecker.forProject(project, formatters, getLog()); } - return UpToDateChecker.noop(); + return UpToDateChecker.noop(project, getLog()); } } diff --git a/plugin-maven/src/main/java/com/diffplug/spotless/maven/SpotlessCheckMojo.java b/plugin-maven/src/main/java/com/diffplug/spotless/maven/SpotlessCheckMojo.java index 33c2ad9ff6..941ab42a3d 100644 --- a/plugin-maven/src/main/java/com/diffplug/spotless/maven/SpotlessCheckMojo.java +++ b/plugin-maven/src/main/java/com/diffplug/spotless/maven/SpotlessCheckMojo.java @@ -49,6 +49,7 @@ protected void process(Iterable files, Formatter formatter, UpToDateChecke if (!dirtyState.isClean() && !dirtyState.didNotConverge()) { problemFiles.add(file); } + // todo: else - mark file as up-to-date? } catch (IOException e) { throw new MojoExecutionException("Unable to format file " + file, e); } diff --git a/plugin-maven/src/main/java/com/diffplug/spotless/maven/incremental/FileIndex.java b/plugin-maven/src/main/java/com/diffplug/spotless/maven/incremental/FileIndex.java index 9fb6770ba0..9815f5a4d2 100644 --- a/plugin-maven/src/main/java/com/diffplug/spotless/maven/incremental/FileIndex.java +++ b/plugin-maven/src/main/java/com/diffplug/spotless/maven/incremental/FileIndex.java @@ -81,6 +81,19 @@ static FileIndex read(FileIndexConfig config, Log log) { } } + static void delete(FileIndexConfig config, Log log) { + Path indexFile = config.getIndexFile(); + boolean deleted = false; + try { + deleted = Files.deleteIfExists(indexFile); + } catch (IOException e) { + log.warn("Unable to delete the index file: " + indexFile, e); + } + if (deleted) { + log.info("Deleted the index file: " + indexFile); + } + } + Optional getLastModifiedTime(Path file) { if (!file.startsWith(projectDir)) { return Optional.empty(); diff --git a/plugin-maven/src/main/java/com/diffplug/spotless/maven/incremental/FileIndexConfig.java b/plugin-maven/src/main/java/com/diffplug/spotless/maven/incremental/FileIndexConfig.java index 226feec6e1..5ea1dcb8b5 100644 --- a/plugin-maven/src/main/java/com/diffplug/spotless/maven/incremental/FileIndexConfig.java +++ b/plugin-maven/src/main/java/com/diffplug/spotless/maven/incremental/FileIndexConfig.java @@ -26,6 +26,10 @@ class FileIndexConfig { private final MavenProject project; private final PluginFingerprint pluginFingerprint; + FileIndexConfig(MavenProject project) { + this(project, PluginFingerprint.empty()); + } + FileIndexConfig(MavenProject project, PluginFingerprint pluginFingerprint) { this.project = project; this.pluginFingerprint = pluginFingerprint; diff --git a/plugin-maven/src/main/java/com/diffplug/spotless/maven/incremental/NoopChecker.java b/plugin-maven/src/main/java/com/diffplug/spotless/maven/incremental/NoopChecker.java index 11f1bfc6d4..a46ac7bbc9 100644 --- a/plugin-maven/src/main/java/com/diffplug/spotless/maven/incremental/NoopChecker.java +++ b/plugin-maven/src/main/java/com/diffplug/spotless/maven/incremental/NoopChecker.java @@ -17,12 +17,19 @@ import java.io.File; -class NoopChecker implements UpToDateChecker { +import org.apache.maven.plugin.logging.Log; +import org.apache.maven.project.MavenProject; - static final NoopChecker INSTANCE = new NoopChecker(); +class NoopChecker implements UpToDateChecker { private NoopChecker() {} + static NoopChecker create(MavenProject project, Log log) { + FileIndexConfig indexConfig = new FileIndexConfig(project); + FileIndex.delete(indexConfig, log); + return new NoopChecker(); + } + @Override public boolean isUpToDate(File file) { return false; diff --git a/plugin-maven/src/main/java/com/diffplug/spotless/maven/incremental/PluginFingerprint.java b/plugin-maven/src/main/java/com/diffplug/spotless/maven/incremental/PluginFingerprint.java index c5fc431c39..48dfb1914d 100644 --- a/plugin-maven/src/main/java/com/diffplug/spotless/maven/incremental/PluginFingerprint.java +++ b/plugin-maven/src/main/java/com/diffplug/spotless/maven/incremental/PluginFingerprint.java @@ -46,6 +46,10 @@ static PluginFingerprint from(String value) { return new PluginFingerprint(value); } + static PluginFingerprint empty() { + return new PluginFingerprint(""); + } + String value() { return value; } diff --git a/plugin-maven/src/main/java/com/diffplug/spotless/maven/incremental/UpToDateChecker.java b/plugin-maven/src/main/java/com/diffplug/spotless/maven/incremental/UpToDateChecker.java index aa9536186f..80e17f37a1 100644 --- a/plugin-maven/src/main/java/com/diffplug/spotless/maven/incremental/UpToDateChecker.java +++ b/plugin-maven/src/main/java/com/diffplug/spotless/maven/incremental/UpToDateChecker.java @@ -30,8 +30,8 @@ public interface UpToDateChecker extends AutoCloseable { void close(); - static UpToDateChecker noop() { - return NoopChecker.INSTANCE; + static UpToDateChecker noop(MavenProject project, Log log) { + return NoopChecker.create(project, log); } static UpToDateChecker forProject(MavenProject project, Iterable formatters, Log log) { diff --git a/plugin-maven/src/test/java/com/diffplug/spotless/maven/incremental/FileIndexConfigTest.java b/plugin-maven/src/test/java/com/diffplug/spotless/maven/incremental/FileIndexConfigTest.java new file mode 100644 index 0000000000..17b3a338ba --- /dev/null +++ b/plugin-maven/src/test/java/com/diffplug/spotless/maven/incremental/FileIndexConfigTest.java @@ -0,0 +1,66 @@ +/* + * 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.spotless.maven.incremental; + +import static org.assertj.core.api.Assertions.assertThat; + +import java.io.File; +import java.nio.file.Paths; + +import org.apache.maven.model.Build; +import org.apache.maven.project.MavenProject; +import org.junit.jupiter.api.Test; + +class FileIndexConfigTest { + + @Test + void returnsCorrectProjectDir() { + FileIndexConfig config = new FileIndexConfig(mavenProject(), PluginFingerprint.from("foo")); + + assertThat(config.getProjectDir()).isEqualTo(Paths.get("projectDir")); + } + + @Test + void returnsCorrectIndexFile() { + FileIndexConfig config = new FileIndexConfig(mavenProject(), PluginFingerprint.from("foo")); + + assertThat(config.getIndexFile()) + .isEqualTo(Paths.get("projectDir", "target", "spotless-index")); + } + + @Test + void returnsCorrectPluginFingerprint() { + FileIndexConfig config = new FileIndexConfig(mavenProject(), PluginFingerprint.from("foo")); + + assertThat(config.getPluginFingerprint()).isEqualTo(PluginFingerprint.from("foo")); + } + + @Test + void returnsEmptyPluginFingerprint() { + FileIndexConfig config = new FileIndexConfig(mavenProject()); + + assertThat(config.getPluginFingerprint()).isEqualTo(PluginFingerprint.from("")); + } + + private static MavenProject mavenProject() { + MavenProject project = new MavenProject(); + project.setFile(new File("projectDir", "pom.xml")); + Build build = new Build(); + build.setDirectory("target"); + project.setBuild(build); + return project; + } +} diff --git a/plugin-maven/src/test/java/com/diffplug/spotless/maven/incremental/NoopCheckerTest.java b/plugin-maven/src/test/java/com/diffplug/spotless/maven/incremental/NoopCheckerTest.java new file mode 100644 index 0000000000..bdf6ccedfa --- /dev/null +++ b/plugin-maven/src/test/java/com/diffplug/spotless/maven/incremental/NoopCheckerTest.java @@ -0,0 +1,121 @@ +/* + * 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.spotless.maven.incremental; + +import static java.nio.charset.StandardCharsets.UTF_8; +import static java.nio.file.StandardOpenOption.CREATE_NEW; +import static java.util.Collections.singletonList; +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyNoInteractions; +import static org.mockito.Mockito.withSettings; + +import java.io.File; +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.Paths; + +import org.apache.maven.model.Build; +import org.apache.maven.plugin.logging.Log; +import org.apache.maven.project.MavenProject; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +import com.diffplug.spotless.FormatExceptionPolicyStrict; +import com.diffplug.spotless.Formatter; +import com.diffplug.spotless.FormatterStep; +import com.diffplug.spotless.LineEnding; +import com.diffplug.spotless.ResourceHarness; + +class NoopCheckerTest extends ResourceHarness { + + private MavenProject project; + private Path indexFile; + private File existingSourceFile; + private File nonExistingSourceFile; + + @BeforeEach + void beforeEach() throws Exception { + project = buildMavenProject(); + indexFile = new FileIndexConfig(project).getIndexFile(); + existingSourceFile = new File(project.getBasedir(), "existing.txt"); + Files.write(existingSourceFile.toPath(), "foo".getBytes(UTF_8), CREATE_NEW); + nonExistingSourceFile = new File(project.getBasedir(), "non-existing.txt"); + } + + @Test + void deletesExistingIndexFileWhenCreated() { + Log log = mock(Log.class); + try (UpToDateChecker realChecker = UpToDateChecker.forProject(project, singletonList(dummyFormatter()), log)) { + realChecker.setUpToDate(existingSourceFile); + } + assertThat(indexFile).exists(); + + try (UpToDateChecker noopChecker = UpToDateChecker.noop(project, log)) { + assertThat(noopChecker).isNotNull(); + } + assertThat(indexFile).doesNotExist(); + verify(log).info("Deleted the index file: " + indexFile); + } + + @Test + void doesNothingWhenIndexFileDoesNotExist() { + assertThat(indexFile).doesNotExist(); + + Log log = mock(Log.class); + try (UpToDateChecker noopChecker = UpToDateChecker.noop(project, log)) { + assertThat(noopChecker).isNotNull(); + } + assertThat(indexFile).doesNotExist(); + verifyNoInteractions(log); + } + + @Test + void neverUpToDate() { + try (UpToDateChecker noopChecker = UpToDateChecker.noop(project, mock(Log.class))) { + assertThat(noopChecker.isUpToDate(existingSourceFile)).isFalse(); + assertThat(noopChecker.isUpToDate(nonExistingSourceFile)).isFalse(); + } + } + + private MavenProject buildMavenProject() throws IOException { + File projectDir = newFolder("project"); + File targetDir = new File(projectDir, "target"); + File pomFile = new File(projectDir, "pom.xml"); + + assertThat(targetDir.mkdir()).isTrue(); + assertThat(pomFile.createNewFile()).isTrue(); + + MavenProject project = new MavenProject(); + project.setFile(pomFile); + Build build = new Build(); + build.setDirectory(targetDir.getName()); + project.setBuild(build); + return project; + } + + private static Formatter dummyFormatter() { + return Formatter.builder() + .rootDir(Paths.get("")) + .lineEndingsPolicy(LineEnding.UNIX.createPolicy()) + .encoding(UTF_8) + .steps(singletonList(mock(FormatterStep.class, withSettings().serializable()))) + .exceptionPolicy(new FormatExceptionPolicyStrict()) + .build(); + } +} diff --git a/plugin-maven/src/test/java/com/diffplug/spotless/maven/incremental/PluginFingerprintTest.java b/plugin-maven/src/test/java/com/diffplug/spotless/maven/incremental/PluginFingerprintTest.java index 2a7675026f..f0e8a0e0f0 100644 --- a/plugin-maven/src/test/java/com/diffplug/spotless/maven/incremental/PluginFingerprintTest.java +++ b/plugin-maven/src/test/java/com/diffplug/spotless/maven/incremental/PluginFingerprintTest.java @@ -17,8 +17,7 @@ import static java.nio.charset.StandardCharsets.UTF_8; import static java.util.Collections.singletonList; -import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.junit.jupiter.api.Assertions.assertNotEquals; +import static org.assertj.core.api.Assertions.assertThat; import java.io.ByteArrayInputStream; import java.nio.file.Paths; @@ -78,7 +77,7 @@ void sameFingerprint() throws Exception { PluginFingerprint fingerprint1 = PluginFingerprint.from(project1, FORMATTERS); PluginFingerprint fingerprint2 = PluginFingerprint.from(project2, FORMATTERS); - assertEquals(fingerprint1, fingerprint2); + assertThat(fingerprint1).isEqualTo(fingerprint2); } @Test @@ -92,7 +91,7 @@ void differentFingerprintForDifferentPluginVersion() throws Exception { PluginFingerprint fingerprint1 = PluginFingerprint.from(project1, FORMATTERS); PluginFingerprint fingerprint2 = PluginFingerprint.from(project2, FORMATTERS); - assertNotEquals(fingerprint1, fingerprint2); + assertThat(fingerprint1).isNotEqualTo(fingerprint2); } @Test @@ -106,7 +105,7 @@ void differentFingerprintForDifferentExecution() throws Exception { PluginFingerprint fingerprint1 = PluginFingerprint.from(project1, FORMATTERS); PluginFingerprint fingerprint2 = PluginFingerprint.from(project2, FORMATTERS); - assertNotEquals(fingerprint1, fingerprint2); + assertThat(fingerprint1).isNotEqualTo(fingerprint2); } @Test @@ -120,7 +119,7 @@ void differentFingerprintForDifferentConfiguration() throws Exception { PluginFingerprint fingerprint1 = PluginFingerprint.from(project1, FORMATTERS); PluginFingerprint fingerprint2 = PluginFingerprint.from(project2, FORMATTERS); - assertNotEquals(fingerprint1, fingerprint2); + assertThat(fingerprint1).isNotEqualTo(fingerprint2); } @Test @@ -140,7 +139,7 @@ void differentFingerprintForFormattersWithDifferentSteps() throws Exception { PluginFingerprint fingerprint1 = PluginFingerprint.from(project1, formatters1); PluginFingerprint fingerprint2 = PluginFingerprint.from(project2, formatters2); - assertNotEquals(fingerprint1, fingerprint2); + assertThat(fingerprint1).isNotEqualTo(fingerprint2); } @Test @@ -158,7 +157,14 @@ void differentFingerprintForFormattersWithDifferentLineEndings() throws Exceptio PluginFingerprint fingerprint1 = PluginFingerprint.from(project1, formatters1); PluginFingerprint fingerprint2 = PluginFingerprint.from(project2, formatters2); - assertNotEquals(fingerprint1, fingerprint2); + assertThat(fingerprint1).isNotEqualTo(fingerprint2); + } + + @Test + void emptyFingerprint() { + PluginFingerprint fingerprint = PluginFingerprint.empty(); + + assertThat(fingerprint.value()).isEmpty(); } private static MavenProject mavenProject(String xml) throws Exception { From 60e8048773ad0a380637402bb0d246a3160718e8 Mon Sep 17 00:00:00 2001 From: Kostiantyn Liutovych Date: Mon, 13 Dec 2021 17:09:49 +0100 Subject: [PATCH 12/21] Unit tests for FileIndex --- .../spotless/maven/incremental/FileIndex.java | 75 +++-- .../maven/incremental/FileIndexTest.java | 313 ++++++++++++++++++ 2 files changed, 365 insertions(+), 23 deletions(-) create mode 100644 plugin-maven/src/test/java/com/diffplug/spotless/maven/incremental/FileIndexTest.java diff --git a/plugin-maven/src/main/java/com/diffplug/spotless/maven/incremental/FileIndex.java b/plugin-maven/src/main/java/com/diffplug/spotless/maven/incremental/FileIndex.java index 9815f5a4d2..0ee45c6c56 100644 --- a/plugin-maven/src/main/java/com/diffplug/spotless/maven/incremental/FileIndex.java +++ b/plugin-maven/src/main/java/com/diffplug/spotless/maven/incremental/FileIndex.java @@ -37,9 +37,9 @@ import org.apache.maven.plugin.logging.Log; -// todo: use TrieMap for fileToLastModifiedTime with String keys +import com.diffplug.common.annotations.VisibleForTesting; + // todo: FileTime -> Instant can be a lossy conversion -// todo: add unit tests class FileIndex { private static final String SEPARATOR = " "; @@ -49,35 +49,42 @@ class FileIndex { private final Map fileToLastModifiedTime; private final Path projectDir; - private boolean updated; + private boolean modified; - private FileIndex(Path indexFile, PluginFingerprint pluginFingerprint, Map fileToLastModifiedTime, Path projectDir) { + private FileIndex(Path indexFile, PluginFingerprint pluginFingerprint, Map fileToLastModifiedTime, Path projectDir, boolean needsRewrite) { this.indexFile = indexFile; this.pluginFingerprint = pluginFingerprint; this.fileToLastModifiedTime = fileToLastModifiedTime; this.projectDir = projectDir; + this.modified = needsRewrite; } static FileIndex read(FileIndexConfig config, Log log) { Path indexFile = config.getIndexFile(); if (Files.notExists(indexFile)) { log.info("Index file does not exist. Fallback to an empty index"); - return emptyIndex(config); + return emptyIndexFallback(config); } try (BufferedReader reader = newBufferedReader(indexFile, UTF_8)) { + String firstLine = reader.readLine(); + if (firstLine == null) { + log.info("Index file is empty. Fallback to an empty index"); + return emptyIndexFallback(config); + } + PluginFingerprint computedFingerprint = config.getPluginFingerprint(); - PluginFingerprint storedFingerprint = PluginFingerprint.from(reader.readLine()); + PluginFingerprint storedFingerprint = PluginFingerprint.from(firstLine); if (!computedFingerprint.equals(storedFingerprint)) { log.info("Fingerprint mismatch in the index file. Fallback to an empty index"); - return emptyIndex(config); + return emptyIndexFallback(config); } else { - Map fileToLastModifiedTime = readFileToLastModifiedTime(reader, config.getProjectDir(), log); - return new FileIndex(indexFile, computedFingerprint, fileToLastModifiedTime, config.getProjectDir()); + Content content = readIndexContent(reader, config.getProjectDir(), log); + return new FileIndex(indexFile, computedFingerprint, content.fileToLastModifiedTime, config.getProjectDir(), content.needsRewrite); } } catch (IOException e) { log.warn("Error reading the index file. Fallback to an empty index", e); - return emptyIndex(config); + return emptyIndexFallback(config); } } @@ -105,11 +112,16 @@ Optional getLastModifiedTime(Path file) { void setLastModifiedTime(Path file, Instant time) { Path relativeFile = projectDir.relativize(file); fileToLastModifiedTime.put(relativeFile, time); - updated = true; + modified = true; + } + + @VisibleForTesting + int size() { + return fileToLastModifiedTime.size(); } void write() { - if (!updated) { + if (!modified) { return; } @@ -118,7 +130,7 @@ void write() { writer.println(pluginFingerprint.value()); for (Entry entry : fileToLastModifiedTime.entrySet()) { - writer.print(entry.getKey() + SEPARATOR + entry.getValue()); + writer.println(entry.getKey() + SEPARATOR + entry.getValue()); } } catch (IOException e) { throw new UncheckedIOException("Unable to write the index", e); @@ -133,8 +145,10 @@ private void ensureParentDirExists() { } } - private static Map readFileToLastModifiedTime(BufferedReader reader, Path projectDir, Log log) throws IOException { + private static Content readIndexContent(BufferedReader reader, Path projectDir, Log log) throws IOException { Map fileToLastModifiedTime = new TreeMap<>(); + boolean needsRewrite = false; + String line; while ((line = reader.readLine()) != null) { int separatorIndex = line.lastIndexOf(SEPARATOR); @@ -146,20 +160,35 @@ private static Map readFileToLastModifiedTime(BufferedReader read Path absoluteFile = projectDir.resolve(relativeFile); if (Files.notExists(absoluteFile)) { log.info("File stored in the index does not exist: " + relativeFile); + needsRewrite = true; } else { - Instant lastModifiedTime; - try { - lastModifiedTime = Instant.parse(line.substring(separatorIndex)); - } catch (DateTimeParseException e) { - throw new IOException("Incorrect index file. Unable to parse last modified time from '" + line + "'", e); - } + Instant lastModifiedTime = parseLastModifiedTime(line, separatorIndex); fileToLastModifiedTime.put(relativeFile, lastModifiedTime); } } - return fileToLastModifiedTime; + + return new Content(fileToLastModifiedTime, needsRewrite); } - private static FileIndex emptyIndex(FileIndexConfig config) { - return new FileIndex(config.getIndexFile(), config.getPluginFingerprint(), new TreeMap<>(), config.getProjectDir()); + private static Instant parseLastModifiedTime(String line, int separatorIndex) throws IOException { + try { + return Instant.parse(line.substring(separatorIndex + 1)); + } catch (DateTimeParseException e) { + throw new IOException("Incorrect index file. Unable to parse last modified time from '" + line + "'", e); + } + } + + private static FileIndex emptyIndexFallback(FileIndexConfig config) { + return new FileIndex(config.getIndexFile(), config.getPluginFingerprint(), new TreeMap<>(), config.getProjectDir(), true); + } + + private static class Content { + final Map fileToLastModifiedTime; + final boolean needsRewrite; + + Content(Map fileToLastModifiedTime, boolean needsRewrite) { + this.fileToLastModifiedTime = fileToLastModifiedTime; + this.needsRewrite = needsRewrite; + } } } diff --git a/plugin-maven/src/test/java/com/diffplug/spotless/maven/incremental/FileIndexTest.java b/plugin-maven/src/test/java/com/diffplug/spotless/maven/incremental/FileIndexTest.java new file mode 100644 index 0000000000..ef0efd3444 --- /dev/null +++ b/plugin-maven/src/test/java/com/diffplug/spotless/maven/incremental/FileIndexTest.java @@ -0,0 +1,313 @@ +/* + * 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.spotless.maven.incremental; + +import static java.nio.charset.StandardCharsets.UTF_8; +import static java.nio.file.StandardOpenOption.APPEND; +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; +import static org.mockito.ArgumentMatchers.contains; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyNoInteractions; +import static org.mockito.Mockito.when; + +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.Paths; +import java.nio.file.attribute.FileTime; +import java.time.Instant; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.List; +import java.util.Optional; + +import org.apache.maven.plugin.logging.Log; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; +import org.mockito.ArgumentCaptor; + +class FileIndexTest { + + private final FileIndexConfig config = mock(FileIndexConfig.class); + private final Log log = mock(Log.class); + + private Path tempDir; + + @BeforeEach + void beforeEach(@TempDir Path tempDir) throws Exception { + this.tempDir = tempDir; + + Path projectDir = tempDir.resolve("my-project"); + Files.createDirectory(projectDir); + when(config.getProjectDir()).thenReturn(projectDir); + + Path indexFile = projectDir.resolve("target").resolve("spotless-index"); + when(config.getIndexFile()).thenReturn(indexFile); + + when(config.getPluginFingerprint()).thenReturn(PluginFingerprint.from("foo")); + } + + @Test + void readFallsBackToEmptyIndexWhenIndexFileDoesNotExist() { + FileIndex index = FileIndex.read(config, log); + + assertThat(index.size()).isZero(); + verify(log).info("Index file does not exist. Fallback to an empty index"); + } + + @Test + void readFallsBackToEmptyIndexWhenIndexFileIsEmpty() throws Exception { + writeIndexFile(); + + FileIndex index = FileIndex.read(config, log); + + assertThat(index.size()).isZero(); + verify(log).info("Index file is empty. Fallback to an empty index"); + } + + @Test + void readFallsBackToEmptyIndexOnFingerprintMismatch() throws Exception { + createSourceFilesAndWriteIndexFile("bar", "source1.txt", "source2.txt"); + + FileIndex index = FileIndex.read(config, log); + + assertThat(index.size()).isZero(); + verify(log).info("Fingerprint mismatch in the index file. Fallback to an empty index"); + } + + @Test + void readFallsBackToEmptyIndexWhenIncorrectSeparator() throws Exception { + createSourceFile("source.txt"); + writeIndexFile(config.getPluginFingerprint().value(), "source.txt|" + Instant.now()); + + FileIndex index = FileIndex.read(config, log); + + assertThat(index.size()).isZero(); + ArgumentCaptor errorCaptor = ArgumentCaptor.forClass(Throwable.class); + verify(log).warn(eq("Error reading the index file. Fallback to an empty index"), errorCaptor.capture()); + assertThat(errorCaptor.getValue()).hasMessageContaining("Incorrect index file. No separator found"); + } + + @Test + void readFallsBackToEmptyIndexWhenUnparseableTimestamp() throws Exception { + createSourceFile("source.txt"); + writeIndexFile(config.getPluginFingerprint().value(), "source.txt 12345-bad-instant"); + + FileIndex index = FileIndex.read(config, log); + + assertThat(index.size()).isZero(); + ArgumentCaptor errorCaptor = ArgumentCaptor.forClass(Throwable.class); + verify(log).warn(eq("Error reading the index file. Fallback to an empty index"), errorCaptor.capture()); + assertThat(errorCaptor.getValue()).hasMessageContaining("Incorrect index file. Unable to parse last modified time"); + } + + @Test + void readEmptyIndex() throws Exception { + writeIndexFile("foo"); + + FileIndex index = FileIndex.read(config, log); + + assertThat(index.size()).isZero(); + verifyNoInteractions(log); + } + + @Test + void readIndexWithSingleEntry() throws Exception { + Path sourceFile = createSourceFilesAndWriteIndexFile("foo", "source.txt").get(0); + + FileIndex index = FileIndex.read(config, log); + + assertThat(index.size()).isOne(); + assertThat(index.getLastModifiedTime(sourceFile)).contains(Files.getLastModifiedTime(sourceFile).toInstant()); + verifyNoInteractions(log); + } + + @Test + void readIndexWithMultipleEntries() throws Exception { + List sourceFiles = createSourceFilesAndWriteIndexFile("foo", "source1.txt", "source2.txt", "source3.txt"); + + FileIndex index = FileIndex.read(config, log); + + assertThat(index.size()).isEqualTo(3); + for (Path sourceFile : sourceFiles) { + assertThat(index.getLastModifiedTime(sourceFile)).contains(Files.getLastModifiedTime(sourceFile).toInstant()); + } + verifyNoInteractions(log); + } + + @Test + void writeIndexWithoutUpdatesDoesNotUpdateTheFile() throws Exception { + createSourceFilesAndWriteIndexFile("foo", "source.txt"); + FileTime modifiedTimeBeforeRead = Files.getLastModifiedTime(config.getIndexFile()); + + FileIndex index = FileIndex.read(config, log); + FileTime modifiedTimeAfterRead = Files.getLastModifiedTime(config.getIndexFile()); + + index.write(); + FileTime modifiedTimeAfterWrite = Files.getLastModifiedTime(config.getIndexFile()); + + assertThat(modifiedTimeAfterRead).isEqualTo(modifiedTimeBeforeRead); + assertThat(modifiedTimeAfterWrite).isEqualTo(modifiedTimeAfterRead); + } + + @Test + void writeIndexContainingUpdates() throws Exception { + createSourceFilesAndWriteIndexFile("foo", "source1.txt", "source2.txt"); + Path sourceFile3 = createSourceFile("source3.txt"); + Path sourceFile4 = createSourceFile("source4.txt"); + Instant modifiedTime3 = Instant.now(); + Instant modifiedTime4 = Instant.now().plusSeconds(42); + + FileIndex index1 = FileIndex.read(config, log); + index1.setLastModifiedTime(sourceFile3, modifiedTime3); + index1.setLastModifiedTime(sourceFile4, modifiedTime4); + index1.write(); + + FileIndex index2 = FileIndex.read(config, log); + assertThat(index2.getLastModifiedTime(sourceFile3)).contains(modifiedTime3); + assertThat(index2.getLastModifiedTime(sourceFile4)).contains(modifiedTime4); + } + + @Test + void writeIndexWhenParentDirDoesNotExist() throws Exception { + assertThat(config.getIndexFile().getParent()).doesNotExist(); + FileIndex index1 = FileIndex.read(config, log); + Path sourceFile = createSourceFile("source.txt"); + Instant modifiedTime = Instant.now(); + index1.setLastModifiedTime(sourceFile, modifiedTime); + + index1.write(); + + assertThat(config.getIndexFile().getParent()).exists(); + FileIndex index2 = FileIndex.read(config, log); + assertThat(index2.getLastModifiedTime(sourceFile)).contains(modifiedTime); + } + + @Test + void deleteNonExistingIndex() { + FileIndex.delete(config, log); + + verifyNoInteractions(log); + } + + @Test + void deleteExistingIndex() throws Exception { + createSourceFilesAndWriteIndexFile("foo", "source1.txt", "source2.txt"); + assertThat(config.getIndexFile()).exists(); + + FileIndex.delete(config, log); + + verify(log).info(contains("Deleted the index file")); + assertThat(config.getIndexFile()).doesNotExist(); + } + + @Test + void getLastModifiedTimeReturnsEmptyOptionalForNonProjectFile() throws Exception { + createSourceFilesAndWriteIndexFile("foo", "source.txt"); + Path nonProjectDir = tempDir.resolve("some-other-project"); + Files.createDirectory(nonProjectDir); + Path nonProjectFile = Files.createFile(nonProjectDir.resolve("some-other-source.txt")); + + FileIndex index = FileIndex.read(config, log); + + assertThat(index.getLastModifiedTime(nonProjectFile)).isEmpty(); + } + + @Test + void getLastModifiedTimeReturnsEmptyOptionalForUnknownFile() throws Exception { + createSourceFilesAndWriteIndexFile("foo", "source.txt"); + Path unknownSourceFile = createSourceFile("unknown-source.txt"); + + FileIndex index = FileIndex.read(config, log); + + assertThat(index.getLastModifiedTime(unknownSourceFile)).isEmpty(); + } + + @Test + void setLastModifiedTimeThrowsForNonProjectFile() throws Exception { + FileIndex index = FileIndex.read(config, log); + Path nonProjectFile = Paths.get("non-project-file"); + + assertThatThrownBy(() -> index.setLastModifiedTime(nonProjectFile, Instant.now())).isInstanceOf(IllegalArgumentException.class); + } + + @Test + void setLastModifiedTimeUpdatesModifiedTime() throws Exception { + Path sourceFile = createSourceFilesAndWriteIndexFile("foo", "source.txt").get(0); + FileIndex index = FileIndex.read(config, log); + + Optional oldTimeOptional = index.getLastModifiedTime(sourceFile); + assertThat(oldTimeOptional).isPresent(); + + Instant newTime = Instant.now().plusSeconds(42); + assertThat(oldTimeOptional.get()).isNotEqualTo(newTime); + + index.setLastModifiedTime(sourceFile, newTime); + assertThat(index.getLastModifiedTime(sourceFile)).contains(newTime); + } + + @Test + void rewritesIndexFileThatReferencesNonExistingFile() throws Exception { + List sourceFiles = createSourceFilesAndWriteIndexFile("foo", "source1.txt", "source2.txt"); + Path nonExistingSourceFile = config.getProjectDir().resolve("non-existing-source.txt"); + FileIndex index1 = FileIndex.read(config, log); + assertThat(index1.size()).isEqualTo(2); + + index1.setLastModifiedTime(nonExistingSourceFile, Instant.now()); + assertThat(index1.size()).isEqualTo(3); + index1.write(); + + FileIndex index2 = FileIndex.read(config, log); + verify(log).info("File stored in the index does not exist: " + nonExistingSourceFile.getFileName()); + assertThat(index2.size()).isEqualTo(2); + index2.write(); + + FileIndex index3 = FileIndex.read(config, log); + assertThat(index3.size()).isEqualTo(2); + } + + private List createSourceFilesAndWriteIndexFile(String fingerprint, String... files) + throws IOException { + List lines = new ArrayList<>(); + lines.add(fingerprint); + + List sourceFiles = new ArrayList<>(); + for (String file : files) { + Path path = createSourceFile(file); + lines.add(file + " " + Files.getLastModifiedTime(path).toInstant()); + sourceFiles.add(path); + } + + writeIndexFile(lines.toArray(new String[0])); + return sourceFiles; + } + + private void writeIndexFile(String... lines) throws IOException { + Files.createDirectory(config.getIndexFile().getParent()); + Files.createFile(config.getIndexFile()); + Files.write(config.getIndexFile(), Arrays.asList(lines), UTF_8, APPEND); + } + + private Path createSourceFile(String name) throws IOException { + Path file = config.getProjectDir().resolve(name); + Files.createFile(file); + return file; + } +} From cf4aefbfcfbb55238cabe4ab05d373542647b152 Mon Sep 17 00:00:00 2001 From: Kostiantyn Liutovych Date: Mon, 13 Dec 2021 17:46:04 +0100 Subject: [PATCH 13/21] Unit tests for IndexBasedChecker --- .../maven/incremental/IndexBasedChecker.java | 11 +- .../maven/incremental/FileIndexHarness.java | 83 +++++++++++++++ .../maven/incremental/FileIndexTest.java | 81 +++----------- .../incremental/IndexBasedCheckerTest.java | 100 ++++++++++++++++++ 4 files changed, 200 insertions(+), 75 deletions(-) create mode 100644 plugin-maven/src/test/java/com/diffplug/spotless/maven/incremental/FileIndexHarness.java create mode 100644 plugin-maven/src/test/java/com/diffplug/spotless/maven/incremental/IndexBasedCheckerTest.java diff --git a/plugin-maven/src/main/java/com/diffplug/spotless/maven/incremental/IndexBasedChecker.java b/plugin-maven/src/main/java/com/diffplug/spotless/maven/incremental/IndexBasedChecker.java index 25b675b14a..c4d7821c53 100644 --- a/plugin-maven/src/main/java/com/diffplug/spotless/maven/incremental/IndexBasedChecker.java +++ b/plugin-maven/src/main/java/com/diffplug/spotless/maven/incremental/IndexBasedChecker.java @@ -26,14 +26,15 @@ import org.apache.maven.plugin.logging.Log; import org.apache.maven.project.MavenProject; +import com.diffplug.common.annotations.VisibleForTesting; import com.diffplug.spotless.Formatter; -// todo: add unit tests class IndexBasedChecker implements UpToDateChecker { private final FileIndex index; - private IndexBasedChecker(FileIndex index) { + @VisibleForTesting + IndexBasedChecker(FileIndex index) { this.index = index; } @@ -53,16 +54,14 @@ public boolean isUpToDate(File file) { return false; } - Instant currentLastModifiedTime = lastModifiedTime(path); Instant storedLastModifiedTime = storedLastModifiedTimeOptional.get(); - return currentLastModifiedTime.isAfter(storedLastModifiedTime); + return storedLastModifiedTime.equals(lastModifiedTime(path)); } @Override public void setUpToDate(File file) { Path path = file.toPath(); - Instant currentLastModifiedTime = lastModifiedTime(path); - index.setLastModifiedTime(path, currentLastModifiedTime); + index.setLastModifiedTime(path, lastModifiedTime(path)); } @Override diff --git a/plugin-maven/src/test/java/com/diffplug/spotless/maven/incremental/FileIndexHarness.java b/plugin-maven/src/test/java/com/diffplug/spotless/maven/incremental/FileIndexHarness.java new file mode 100644 index 0000000000..8260038efa --- /dev/null +++ b/plugin-maven/src/test/java/com/diffplug/spotless/maven/incremental/FileIndexHarness.java @@ -0,0 +1,83 @@ +/* + * 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.spotless.maven.incremental; + +import static java.nio.charset.StandardCharsets.UTF_8; +import static java.nio.file.StandardOpenOption.APPEND; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.List; + +import org.apache.maven.plugin.logging.Log; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.io.TempDir; + +abstract class FileIndexHarness { + + protected static final PluginFingerprint FINGERPRINT = PluginFingerprint.from("foo"); + + protected final FileIndexConfig config = mock(FileIndexConfig.class); + protected final Log log = mock(Log.class); + + protected Path tempDir; + + @BeforeEach + void beforeEach(@TempDir Path tempDir) throws Exception { + this.tempDir = tempDir; + + Path projectDir = tempDir.resolve("my-project"); + Files.createDirectory(projectDir); + when(config.getProjectDir()).thenReturn(projectDir); + + Path indexFile = projectDir.resolve("target").resolve("spotless-index"); + when(config.getIndexFile()).thenReturn(indexFile); + + when(config.getPluginFingerprint()).thenReturn(FINGERPRINT); + } + + protected List createSourceFilesAndWriteIndexFile(PluginFingerprint fingerprint, String... files) throws IOException { + List lines = new ArrayList<>(); + lines.add(fingerprint.value()); + + List sourceFiles = new ArrayList<>(); + for (String file : files) { + Path path = createSourceFile(file); + lines.add(file + " " + Files.getLastModifiedTime(path).toInstant()); + sourceFiles.add(path); + } + + writeIndexFile(lines.toArray(new String[0])); + return sourceFiles; + } + + protected void writeIndexFile(String... lines) throws IOException { + Files.createDirectory(config.getIndexFile().getParent()); + Files.createFile(config.getIndexFile()); + Files.write(config.getIndexFile(), Arrays.asList(lines), UTF_8, APPEND); + } + + protected Path createSourceFile(String name) throws IOException { + Path file = config.getProjectDir().resolve(name); + Files.createFile(file); + return file; + } +} diff --git a/plugin-maven/src/test/java/com/diffplug/spotless/maven/incremental/FileIndexTest.java b/plugin-maven/src/test/java/com/diffplug/spotless/maven/incremental/FileIndexTest.java index ef0efd3444..0126fddf31 100644 --- a/plugin-maven/src/test/java/com/diffplug/spotless/maven/incremental/FileIndexTest.java +++ b/plugin-maven/src/test/java/com/diffplug/spotless/maven/incremental/FileIndexTest.java @@ -15,54 +15,25 @@ */ package com.diffplug.spotless.maven.incremental; -import static java.nio.charset.StandardCharsets.UTF_8; -import static java.nio.file.StandardOpenOption.APPEND; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatThrownBy; import static org.mockito.ArgumentMatchers.contains; import static org.mockito.ArgumentMatchers.eq; -import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.verifyNoInteractions; -import static org.mockito.Mockito.when; -import java.io.IOException; import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.Paths; import java.nio.file.attribute.FileTime; import java.time.Instant; -import java.util.ArrayList; -import java.util.Arrays; import java.util.List; import java.util.Optional; -import org.apache.maven.plugin.logging.Log; -import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; -import org.junit.jupiter.api.io.TempDir; import org.mockito.ArgumentCaptor; -class FileIndexTest { - - private final FileIndexConfig config = mock(FileIndexConfig.class); - private final Log log = mock(Log.class); - - private Path tempDir; - - @BeforeEach - void beforeEach(@TempDir Path tempDir) throws Exception { - this.tempDir = tempDir; - - Path projectDir = tempDir.resolve("my-project"); - Files.createDirectory(projectDir); - when(config.getProjectDir()).thenReturn(projectDir); - - Path indexFile = projectDir.resolve("target").resolve("spotless-index"); - when(config.getIndexFile()).thenReturn(indexFile); - - when(config.getPluginFingerprint()).thenReturn(PluginFingerprint.from("foo")); - } +class FileIndexTest extends FileIndexHarness { @Test void readFallsBackToEmptyIndexWhenIndexFileDoesNotExist() { @@ -84,7 +55,7 @@ void readFallsBackToEmptyIndexWhenIndexFileIsEmpty() throws Exception { @Test void readFallsBackToEmptyIndexOnFingerprintMismatch() throws Exception { - createSourceFilesAndWriteIndexFile("bar", "source1.txt", "source2.txt"); + createSourceFilesAndWriteIndexFile(PluginFingerprint.from("wrong"), "source1.txt", "source2.txt"); FileIndex index = FileIndex.read(config, log); @@ -130,7 +101,7 @@ void readEmptyIndex() throws Exception { @Test void readIndexWithSingleEntry() throws Exception { - Path sourceFile = createSourceFilesAndWriteIndexFile("foo", "source.txt").get(0); + Path sourceFile = createSourceFilesAndWriteIndexFile(FINGERPRINT, "source.txt").get(0); FileIndex index = FileIndex.read(config, log); @@ -141,7 +112,7 @@ void readIndexWithSingleEntry() throws Exception { @Test void readIndexWithMultipleEntries() throws Exception { - List sourceFiles = createSourceFilesAndWriteIndexFile("foo", "source1.txt", "source2.txt", "source3.txt"); + List sourceFiles = createSourceFilesAndWriteIndexFile(FINGERPRINT, "source1.txt", "source2.txt", "source3.txt"); FileIndex index = FileIndex.read(config, log); @@ -154,7 +125,7 @@ void readIndexWithMultipleEntries() throws Exception { @Test void writeIndexWithoutUpdatesDoesNotUpdateTheFile() throws Exception { - createSourceFilesAndWriteIndexFile("foo", "source.txt"); + createSourceFilesAndWriteIndexFile(FINGERPRINT, "source.txt"); FileTime modifiedTimeBeforeRead = Files.getLastModifiedTime(config.getIndexFile()); FileIndex index = FileIndex.read(config, log); @@ -169,7 +140,7 @@ void writeIndexWithoutUpdatesDoesNotUpdateTheFile() throws Exception { @Test void writeIndexContainingUpdates() throws Exception { - createSourceFilesAndWriteIndexFile("foo", "source1.txt", "source2.txt"); + createSourceFilesAndWriteIndexFile(FINGERPRINT, "source1.txt", "source2.txt"); Path sourceFile3 = createSourceFile("source3.txt"); Path sourceFile4 = createSourceFile("source4.txt"); Instant modifiedTime3 = Instant.now(); @@ -209,7 +180,7 @@ void deleteNonExistingIndex() { @Test void deleteExistingIndex() throws Exception { - createSourceFilesAndWriteIndexFile("foo", "source1.txt", "source2.txt"); + createSourceFilesAndWriteIndexFile(FINGERPRINT, "source1.txt", "source2.txt"); assertThat(config.getIndexFile()).exists(); FileIndex.delete(config, log); @@ -220,7 +191,7 @@ void deleteExistingIndex() throws Exception { @Test void getLastModifiedTimeReturnsEmptyOptionalForNonProjectFile() throws Exception { - createSourceFilesAndWriteIndexFile("foo", "source.txt"); + createSourceFilesAndWriteIndexFile(FINGERPRINT, "source.txt"); Path nonProjectDir = tempDir.resolve("some-other-project"); Files.createDirectory(nonProjectDir); Path nonProjectFile = Files.createFile(nonProjectDir.resolve("some-other-source.txt")); @@ -232,7 +203,7 @@ void getLastModifiedTimeReturnsEmptyOptionalForNonProjectFile() throws Exception @Test void getLastModifiedTimeReturnsEmptyOptionalForUnknownFile() throws Exception { - createSourceFilesAndWriteIndexFile("foo", "source.txt"); + createSourceFilesAndWriteIndexFile(FINGERPRINT, "source.txt"); Path unknownSourceFile = createSourceFile("unknown-source.txt"); FileIndex index = FileIndex.read(config, log); @@ -241,7 +212,7 @@ void getLastModifiedTimeReturnsEmptyOptionalForUnknownFile() throws Exception { } @Test - void setLastModifiedTimeThrowsForNonProjectFile() throws Exception { + void setLastModifiedTimeThrowsForNonProjectFile() { FileIndex index = FileIndex.read(config, log); Path nonProjectFile = Paths.get("non-project-file"); @@ -250,7 +221,7 @@ void setLastModifiedTimeThrowsForNonProjectFile() throws Exception { @Test void setLastModifiedTimeUpdatesModifiedTime() throws Exception { - Path sourceFile = createSourceFilesAndWriteIndexFile("foo", "source.txt").get(0); + Path sourceFile = createSourceFilesAndWriteIndexFile(FINGERPRINT, "source.txt").get(0); FileIndex index = FileIndex.read(config, log); Optional oldTimeOptional = index.getLastModifiedTime(sourceFile); @@ -265,7 +236,7 @@ void setLastModifiedTimeUpdatesModifiedTime() throws Exception { @Test void rewritesIndexFileThatReferencesNonExistingFile() throws Exception { - List sourceFiles = createSourceFilesAndWriteIndexFile("foo", "source1.txt", "source2.txt"); + createSourceFilesAndWriteIndexFile(FINGERPRINT, "source1.txt", "source2.txt"); Path nonExistingSourceFile = config.getProjectDir().resolve("non-existing-source.txt"); FileIndex index1 = FileIndex.read(config, log); assertThat(index1.size()).isEqualTo(2); @@ -282,32 +253,4 @@ void rewritesIndexFileThatReferencesNonExistingFile() throws Exception { FileIndex index3 = FileIndex.read(config, log); assertThat(index3.size()).isEqualTo(2); } - - private List createSourceFilesAndWriteIndexFile(String fingerprint, String... files) - throws IOException { - List lines = new ArrayList<>(); - lines.add(fingerprint); - - List sourceFiles = new ArrayList<>(); - for (String file : files) { - Path path = createSourceFile(file); - lines.add(file + " " + Files.getLastModifiedTime(path).toInstant()); - sourceFiles.add(path); - } - - writeIndexFile(lines.toArray(new String[0])); - return sourceFiles; - } - - private void writeIndexFile(String... lines) throws IOException { - Files.createDirectory(config.getIndexFile().getParent()); - Files.createFile(config.getIndexFile()); - Files.write(config.getIndexFile(), Arrays.asList(lines), UTF_8, APPEND); - } - - private Path createSourceFile(String name) throws IOException { - Path file = config.getProjectDir().resolve(name); - Files.createFile(file); - return file; - } } diff --git a/plugin-maven/src/test/java/com/diffplug/spotless/maven/incremental/IndexBasedCheckerTest.java b/plugin-maven/src/test/java/com/diffplug/spotless/maven/incremental/IndexBasedCheckerTest.java new file mode 100644 index 0000000000..330c4ab216 --- /dev/null +++ b/plugin-maven/src/test/java/com/diffplug/spotless/maven/incremental/IndexBasedCheckerTest.java @@ -0,0 +1,100 @@ +/* + * 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.spotless.maven.incremental; + +import static org.assertj.core.api.Assertions.assertThat; + +import java.io.File; +import java.nio.file.Files; +import java.nio.file.Path; +import java.time.Instant; + +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +class IndexBasedCheckerTest extends FileIndexHarness { + + private FileIndex index; + private IndexBasedChecker checker; + + @BeforeEach + void beforeEach() { + index = FileIndex.read(config, log); + checker = new IndexBasedChecker(index); + } + + @Test + void isUpToDateReturnsFalseForUnknownFile() { + File unknownFile = new File("unknown-file.txt"); + assertThat(checker.isUpToDate(unknownFile)).isFalse(); + } + + @Test + void isUpToDateReturnsTrueWhenOnDiskFileIsSameAsInTheIndex() throws Exception { + Path sourceFile = createSourceFile("source.txt"); + Instant modifiedTime = Files.getLastModifiedTime(sourceFile).toInstant(); + index.setLastModifiedTime(sourceFile, modifiedTime); + + assertThat(checker.isUpToDate(sourceFile.toFile())).isTrue(); + } + + @Test + void isUpToDateReturnsFalseWhenOnDiskFileIsNewerThanInTheIndex() throws Exception { + Path sourceFile = createSourceFile("source.txt"); + Instant modifiedTime = Files.getLastModifiedTime(sourceFile).toInstant().minusSeconds(42); + index.setLastModifiedTime(sourceFile, modifiedTime); + + assertThat(checker.isUpToDate(sourceFile.toFile())).isFalse(); + } + + /** + * This test checks a bit of a weird case when file's last modified time in the index is greater + * than file's last modified time on-disk. This should not happen because of how the index is + * used. To be on the safe side, we consider the file to be out of date if this ever happens. + */ + @Test + void isUpToDateReturnsFalseWhenOnDiskFileIsOlderThanInTheIndex() throws Exception { + Path sourceFile = createSourceFile("source.txt"); + Instant modifiedTime = Files.getLastModifiedTime(sourceFile).toInstant().plusSeconds(42); + index.setLastModifiedTime(sourceFile, modifiedTime); + + assertThat(checker.isUpToDate(sourceFile.toFile())).isFalse(); + } + + @Test + void setUpToDateUpdatesTheIndex() throws Exception { + Path sourceFile = createSourceFile("source.txt"); + assertThat(index.getLastModifiedTime(sourceFile)).isEmpty(); + assertThat(checker.isUpToDate(sourceFile.toFile())).isFalse(); + + checker.setUpToDate(sourceFile.toFile()); + + assertThat(index.getLastModifiedTime(sourceFile)).contains(Files.getLastModifiedTime(sourceFile).toInstant()); + assertThat(checker.isUpToDate(sourceFile.toFile())).isTrue(); + } + + @Test + void closeWritesTheIndex() throws Exception { + Path sourceFile = createSourceFile("source.txt"); + assertThat(index.getLastModifiedTime(sourceFile)).isEmpty(); + + checker.setUpToDate(sourceFile.toFile()); + checker.close(); + + FileIndex newIndex = FileIndex.read(config, log); + assertThat(newIndex.getLastModifiedTime(sourceFile)).contains(Files.getLastModifiedTime(sourceFile).toInstant()); + } +} From 0f35c40829879b808665b0a88e8599aed2e61415 Mon Sep 17 00:00:00 2001 From: Kostiantyn Liutovych Date: Mon, 13 Dec 2021 18:51:19 +0100 Subject: [PATCH 14/21] Remove Optionals from FileIndex Instead, use `@Nullable` to simplify the API. This requires adding a new dependency to the POM when building the Maven plugin. --- gradle.properties | 1 + gradle/java-setup.gradle | 2 +- plugin-maven/build.gradle | 1 + .../spotless/maven/incremental/FileIndex.java | 10 ++++---- .../maven/incremental/IndexBasedChecker.java | 12 +++------- .../maven/incremental/FileIndexTest.java | 23 +++++++++---------- .../incremental/IndexBasedCheckerTest.java | 14 +++++------ .../src/test/resources/pom-build.xml.mustache | 7 ++++++ 8 files changed, 37 insertions(+), 33 deletions(-) diff --git a/gradle.properties b/gradle.properties index 4d4530ea8d..c50c63f277 100644 --- a/gradle.properties +++ b/gradle.properties @@ -16,6 +16,7 @@ artifactIdGradle=spotless-plugin-gradle # Build requirements VER_JAVA=1.8 VER_SPOTBUGS=4.5.0 +VER_JSR_305=3.0.2 # Dependencies provided by Spotless plugin VER_SLF4J=[1.6,2.0[ diff --git a/gradle/java-setup.gradle b/gradle/java-setup.gradle index 2df8805844..5d9d756414 100644 --- a/gradle/java-setup.gradle +++ b/gradle/java-setup.gradle @@ -35,5 +35,5 @@ tasks.named('spotbugsMain') { dependencies { compileOnly 'net.jcip:jcip-annotations:1.0' compileOnly "com.github.spotbugs:spotbugs-annotations:${VER_SPOTBUGS}" - compileOnly 'com.google.code.findbugs:jsr305:3.0.2' + compileOnly "com.google.code.findbugs:jsr305:${VER_JSR_305}" } diff --git a/plugin-maven/build.gradle b/plugin-maven/build.gradle index 90bc3cbe7c..ac062dd497 100644 --- a/plugin-maven/build.gradle +++ b/plugin-maven/build.gradle @@ -161,6 +161,7 @@ task createPomXml(dependsOn: installLocalDependencies) { mavenApiVersion : VER_MAVEN_API, eclipseAetherVersion : VER_ECLIPSE_AETHER, spotlessLibVersion : libVersion, + jsr305Version : VER_JSR_305, additionalDependencies : additionalDependencies ] diff --git a/plugin-maven/src/main/java/com/diffplug/spotless/maven/incremental/FileIndex.java b/plugin-maven/src/main/java/com/diffplug/spotless/maven/incremental/FileIndex.java index 0ee45c6c56..a6ec7a8867 100644 --- a/plugin-maven/src/main/java/com/diffplug/spotless/maven/incremental/FileIndex.java +++ b/plugin-maven/src/main/java/com/diffplug/spotless/maven/incremental/FileIndex.java @@ -32,9 +32,10 @@ import java.time.format.DateTimeParseException; import java.util.Map; import java.util.Map.Entry; -import java.util.Optional; import java.util.TreeMap; +import javax.annotation.Nullable; + import org.apache.maven.plugin.logging.Log; import com.diffplug.common.annotations.VisibleForTesting; @@ -101,12 +102,13 @@ static void delete(FileIndexConfig config, Log log) { } } - Optional getLastModifiedTime(Path file) { + @Nullable + Instant getLastModifiedTime(Path file) { if (!file.startsWith(projectDir)) { - return Optional.empty(); + return null; } Path relativeFile = projectDir.relativize(file); - return Optional.ofNullable(fileToLastModifiedTime.get(relativeFile)); + return fileToLastModifiedTime.get(relativeFile); } void setLastModifiedTime(Path file, Instant time) { diff --git a/plugin-maven/src/main/java/com/diffplug/spotless/maven/incremental/IndexBasedChecker.java b/plugin-maven/src/main/java/com/diffplug/spotless/maven/incremental/IndexBasedChecker.java index c4d7821c53..c1b3affafc 100644 --- a/plugin-maven/src/main/java/com/diffplug/spotless/maven/incremental/IndexBasedChecker.java +++ b/plugin-maven/src/main/java/com/diffplug/spotless/maven/incremental/IndexBasedChecker.java @@ -21,7 +21,7 @@ import java.nio.file.Files; import java.nio.file.Path; import java.time.Instant; -import java.util.Optional; +import java.util.Objects; import org.apache.maven.plugin.logging.Log; import org.apache.maven.project.MavenProject; @@ -48,14 +48,8 @@ static IndexBasedChecker create(MavenProject project, Iterable format @Override public boolean isUpToDate(File file) { Path path = file.toPath(); - - Optional storedLastModifiedTimeOptional = index.getLastModifiedTime(path); - if (!storedLastModifiedTimeOptional.isPresent()) { - return false; - } - - Instant storedLastModifiedTime = storedLastModifiedTimeOptional.get(); - return storedLastModifiedTime.equals(lastModifiedTime(path)); + Instant storedLastModifiedTime = index.getLastModifiedTime(path); + return Objects.equals(storedLastModifiedTime, lastModifiedTime(path)); } @Override diff --git a/plugin-maven/src/test/java/com/diffplug/spotless/maven/incremental/FileIndexTest.java b/plugin-maven/src/test/java/com/diffplug/spotless/maven/incremental/FileIndexTest.java index 0126fddf31..c12d00d218 100644 --- a/plugin-maven/src/test/java/com/diffplug/spotless/maven/incremental/FileIndexTest.java +++ b/plugin-maven/src/test/java/com/diffplug/spotless/maven/incremental/FileIndexTest.java @@ -28,7 +28,6 @@ import java.nio.file.attribute.FileTime; import java.time.Instant; import java.util.List; -import java.util.Optional; import org.junit.jupiter.api.Test; import org.mockito.ArgumentCaptor; @@ -106,7 +105,7 @@ void readIndexWithSingleEntry() throws Exception { FileIndex index = FileIndex.read(config, log); assertThat(index.size()).isOne(); - assertThat(index.getLastModifiedTime(sourceFile)).contains(Files.getLastModifiedTime(sourceFile).toInstant()); + assertThat(index.getLastModifiedTime(sourceFile)).isEqualTo(Files.getLastModifiedTime(sourceFile).toInstant()); verifyNoInteractions(log); } @@ -118,7 +117,7 @@ void readIndexWithMultipleEntries() throws Exception { assertThat(index.size()).isEqualTo(3); for (Path sourceFile : sourceFiles) { - assertThat(index.getLastModifiedTime(sourceFile)).contains(Files.getLastModifiedTime(sourceFile).toInstant()); + assertThat(index.getLastModifiedTime(sourceFile)).isEqualTo(Files.getLastModifiedTime(sourceFile).toInstant()); } verifyNoInteractions(log); } @@ -152,8 +151,8 @@ void writeIndexContainingUpdates() throws Exception { index1.write(); FileIndex index2 = FileIndex.read(config, log); - assertThat(index2.getLastModifiedTime(sourceFile3)).contains(modifiedTime3); - assertThat(index2.getLastModifiedTime(sourceFile4)).contains(modifiedTime4); + assertThat(index2.getLastModifiedTime(sourceFile3)).isEqualTo(modifiedTime3); + assertThat(index2.getLastModifiedTime(sourceFile4)).isEqualTo(modifiedTime4); } @Test @@ -168,7 +167,7 @@ void writeIndexWhenParentDirDoesNotExist() throws Exception { assertThat(config.getIndexFile().getParent()).exists(); FileIndex index2 = FileIndex.read(config, log); - assertThat(index2.getLastModifiedTime(sourceFile)).contains(modifiedTime); + assertThat(index2.getLastModifiedTime(sourceFile)).isEqualTo(modifiedTime); } @Test @@ -198,7 +197,7 @@ void getLastModifiedTimeReturnsEmptyOptionalForNonProjectFile() throws Exception FileIndex index = FileIndex.read(config, log); - assertThat(index.getLastModifiedTime(nonProjectFile)).isEmpty(); + assertThat(index.getLastModifiedTime(nonProjectFile)).isNull(); } @Test @@ -208,7 +207,7 @@ void getLastModifiedTimeReturnsEmptyOptionalForUnknownFile() throws Exception { FileIndex index = FileIndex.read(config, log); - assertThat(index.getLastModifiedTime(unknownSourceFile)).isEmpty(); + assertThat(index.getLastModifiedTime(unknownSourceFile)).isNull(); } @Test @@ -224,14 +223,14 @@ void setLastModifiedTimeUpdatesModifiedTime() throws Exception { Path sourceFile = createSourceFilesAndWriteIndexFile(FINGERPRINT, "source.txt").get(0); FileIndex index = FileIndex.read(config, log); - Optional oldTimeOptional = index.getLastModifiedTime(sourceFile); - assertThat(oldTimeOptional).isPresent(); + Instant oldTime = index.getLastModifiedTime(sourceFile); + assertThat(oldTime).isNotNull(); Instant newTime = Instant.now().plusSeconds(42); - assertThat(oldTimeOptional.get()).isNotEqualTo(newTime); + assertThat(oldTime).isNotEqualTo(newTime); index.setLastModifiedTime(sourceFile, newTime); - assertThat(index.getLastModifiedTime(sourceFile)).contains(newTime); + assertThat(index.getLastModifiedTime(sourceFile)).isEqualTo(newTime); } @Test diff --git a/plugin-maven/src/test/java/com/diffplug/spotless/maven/incremental/IndexBasedCheckerTest.java b/plugin-maven/src/test/java/com/diffplug/spotless/maven/incremental/IndexBasedCheckerTest.java index 330c4ab216..e6a62ba1cf 100644 --- a/plugin-maven/src/test/java/com/diffplug/spotless/maven/incremental/IndexBasedCheckerTest.java +++ b/plugin-maven/src/test/java/com/diffplug/spotless/maven/incremental/IndexBasedCheckerTest.java @@ -37,9 +37,9 @@ void beforeEach() { } @Test - void isUpToDateReturnsFalseForUnknownFile() { - File unknownFile = new File("unknown-file.txt"); - assertThat(checker.isUpToDate(unknownFile)).isFalse(); + void isUpToDateReturnsFalseForUnknownFile() throws Exception { + Path sourceFile = createSourceFile("source.txt"); + assertThat(checker.isUpToDate(sourceFile.toFile())).isFalse(); } @Test @@ -77,24 +77,24 @@ void isUpToDateReturnsFalseWhenOnDiskFileIsOlderThanInTheIndex() throws Exceptio @Test void setUpToDateUpdatesTheIndex() throws Exception { Path sourceFile = createSourceFile("source.txt"); - assertThat(index.getLastModifiedTime(sourceFile)).isEmpty(); + assertThat(index.getLastModifiedTime(sourceFile)).isNull(); assertThat(checker.isUpToDate(sourceFile.toFile())).isFalse(); checker.setUpToDate(sourceFile.toFile()); - assertThat(index.getLastModifiedTime(sourceFile)).contains(Files.getLastModifiedTime(sourceFile).toInstant()); + assertThat(index.getLastModifiedTime(sourceFile)).isEqualTo(Files.getLastModifiedTime(sourceFile).toInstant()); assertThat(checker.isUpToDate(sourceFile.toFile())).isTrue(); } @Test void closeWritesTheIndex() throws Exception { Path sourceFile = createSourceFile("source.txt"); - assertThat(index.getLastModifiedTime(sourceFile)).isEmpty(); + assertThat(index.getLastModifiedTime(sourceFile)).isNull(); checker.setUpToDate(sourceFile.toFile()); checker.close(); FileIndex newIndex = FileIndex.read(config, log); - assertThat(newIndex.getLastModifiedTime(sourceFile)).contains(Files.getLastModifiedTime(sourceFile).toInstant()); + assertThat(newIndex.getLastModifiedTime(sourceFile)).isEqualTo(Files.getLastModifiedTime(sourceFile).toInstant()); } } diff --git a/plugin-maven/src/test/resources/pom-build.xml.mustache b/plugin-maven/src/test/resources/pom-build.xml.mustache index 9e40094cfe..3c1d1a7d5b 100644 --- a/plugin-maven/src/test/resources/pom-build.xml.mustache +++ b/plugin-maven/src/test/resources/pom-build.xml.mustache @@ -23,6 +23,7 @@ {{mavenApiVersion}} {{eclipseAetherVersion}} {{spotlessLibVersion}} + {{jsr305Version}} @@ -56,6 +57,12 @@ ${eclipse.aether.version} provided + + com.google.code.findbugs + jsr305 + ${jsr305.version} + provided + com.diffplug.spotless From bfd884b89abc753a645dbb842cc58abc66664bff Mon Sep 17 00:00:00 2001 From: Kostiantyn Liutovych Date: Tue, 14 Dec 2021 14:46:15 +0100 Subject: [PATCH 15/21] Use Paths for index and up-to-date checker APIs Instead of using a mix of File and Path. Path has more suitable API for: * Relativizing file paths * Getting last modified time. `File#lastModified()` may return zero. It is also not very clear about overflows --- .../spotless/maven/SpotlessApplyMojo.java | 4 ++-- .../spotless/maven/SpotlessCheckMojo.java | 2 +- .../spotless/maven/incremental/FileIndex.java | 1 - .../maven/incremental/IndexBasedChecker.java | 18 +++++++++--------- .../maven/incremental/NoopChecker.java | 6 +++--- .../maven/incremental/UpToDateChecker.java | 6 +++--- .../incremental/IndexBasedCheckerTest.java | 17 ++++++++--------- .../maven/incremental/NoopCheckerTest.java | 10 +++++----- 8 files changed, 31 insertions(+), 33 deletions(-) diff --git a/plugin-maven/src/main/java/com/diffplug/spotless/maven/SpotlessApplyMojo.java b/plugin-maven/src/main/java/com/diffplug/spotless/maven/SpotlessApplyMojo.java index cff01b90a0..7995a75b8c 100644 --- a/plugin-maven/src/main/java/com/diffplug/spotless/maven/SpotlessApplyMojo.java +++ b/plugin-maven/src/main/java/com/diffplug/spotless/maven/SpotlessApplyMojo.java @@ -34,7 +34,7 @@ public class SpotlessApplyMojo extends AbstractSpotlessMojo { @Override protected void process(Iterable files, Formatter formatter, UpToDateChecker upToDateChecker) throws MojoExecutionException { for (File file : files) { - if (upToDateChecker.isUpToDate(file)) { + if (upToDateChecker.isUpToDate(file.toPath())) { continue; } @@ -47,7 +47,7 @@ protected void process(Iterable files, Formatter formatter, UpToDateChecke throw new MojoExecutionException("Unable to format file " + file, e); } - upToDateChecker.setUpToDate(file); + upToDateChecker.setUpToDate(file.toPath()); } } } diff --git a/plugin-maven/src/main/java/com/diffplug/spotless/maven/SpotlessCheckMojo.java b/plugin-maven/src/main/java/com/diffplug/spotless/maven/SpotlessCheckMojo.java index 941ab42a3d..aae7912a40 100644 --- a/plugin-maven/src/main/java/com/diffplug/spotless/maven/SpotlessCheckMojo.java +++ b/plugin-maven/src/main/java/com/diffplug/spotless/maven/SpotlessCheckMojo.java @@ -40,7 +40,7 @@ public class SpotlessCheckMojo extends AbstractSpotlessMojo { protected void process(Iterable files, Formatter formatter, UpToDateChecker upToDateChecker) throws MojoExecutionException { List problemFiles = new ArrayList<>(); for (File file : files) { - if (upToDateChecker.isUpToDate(file)) { + if (upToDateChecker.isUpToDate(file.toPath())) { continue; } diff --git a/plugin-maven/src/main/java/com/diffplug/spotless/maven/incremental/FileIndex.java b/plugin-maven/src/main/java/com/diffplug/spotless/maven/incremental/FileIndex.java index a6ec7a8867..d6a2e56e86 100644 --- a/plugin-maven/src/main/java/com/diffplug/spotless/maven/incremental/FileIndex.java +++ b/plugin-maven/src/main/java/com/diffplug/spotless/maven/incremental/FileIndex.java @@ -40,7 +40,6 @@ import com.diffplug.common.annotations.VisibleForTesting; -// todo: FileTime -> Instant can be a lossy conversion class FileIndex { private static final String SEPARATOR = " "; diff --git a/plugin-maven/src/main/java/com/diffplug/spotless/maven/incremental/IndexBasedChecker.java b/plugin-maven/src/main/java/com/diffplug/spotless/maven/incremental/IndexBasedChecker.java index c1b3affafc..0722b8a296 100644 --- a/plugin-maven/src/main/java/com/diffplug/spotless/maven/incremental/IndexBasedChecker.java +++ b/plugin-maven/src/main/java/com/diffplug/spotless/maven/incremental/IndexBasedChecker.java @@ -15,11 +15,11 @@ */ package com.diffplug.spotless.maven.incremental; -import java.io.File; import java.io.IOException; import java.io.UncheckedIOException; import java.nio.file.Files; import java.nio.file.Path; +import java.nio.file.attribute.FileTime; import java.time.Instant; import java.util.Objects; @@ -46,16 +46,14 @@ static IndexBasedChecker create(MavenProject project, Iterable format } @Override - public boolean isUpToDate(File file) { - Path path = file.toPath(); - Instant storedLastModifiedTime = index.getLastModifiedTime(path); - return Objects.equals(storedLastModifiedTime, lastModifiedTime(path)); + public boolean isUpToDate(Path file) { + Instant storedLastModifiedTime = index.getLastModifiedTime(file); + return Objects.equals(storedLastModifiedTime, lastModifiedTime(file)); } @Override - public void setUpToDate(File file) { - Path path = file.toPath(); - index.setLastModifiedTime(path, lastModifiedTime(path)); + public void setUpToDate(Path file) { + index.setLastModifiedTime(file, lastModifiedTime(file)); } @Override @@ -63,9 +61,11 @@ public void close() { index.write(); } + // todo: FileTime -> Instant can be a lossy conversion private static Instant lastModifiedTime(Path path) { try { - return Files.getLastModifiedTime(path).toInstant(); + FileTime lastModifiedTime = Files.getLastModifiedTime(path); + return lastModifiedTime.toInstant(); } catch (IOException e) { throw new UncheckedIOException("Unable to get last modified date for " + path, e); } diff --git a/plugin-maven/src/main/java/com/diffplug/spotless/maven/incremental/NoopChecker.java b/plugin-maven/src/main/java/com/diffplug/spotless/maven/incremental/NoopChecker.java index a46ac7bbc9..0814938f5f 100644 --- a/plugin-maven/src/main/java/com/diffplug/spotless/maven/incremental/NoopChecker.java +++ b/plugin-maven/src/main/java/com/diffplug/spotless/maven/incremental/NoopChecker.java @@ -15,7 +15,7 @@ */ package com.diffplug.spotless.maven.incremental; -import java.io.File; +import java.nio.file.Path; import org.apache.maven.plugin.logging.Log; import org.apache.maven.project.MavenProject; @@ -31,12 +31,12 @@ static NoopChecker create(MavenProject project, Log log) { } @Override - public boolean isUpToDate(File file) { + public boolean isUpToDate(Path file) { return false; } @Override - public void setUpToDate(File file) {} + public void setUpToDate(Path file) {} @Override public void close() {} diff --git a/plugin-maven/src/main/java/com/diffplug/spotless/maven/incremental/UpToDateChecker.java b/plugin-maven/src/main/java/com/diffplug/spotless/maven/incremental/UpToDateChecker.java index 80e17f37a1..0851679707 100644 --- a/plugin-maven/src/main/java/com/diffplug/spotless/maven/incremental/UpToDateChecker.java +++ b/plugin-maven/src/main/java/com/diffplug/spotless/maven/incremental/UpToDateChecker.java @@ -15,7 +15,7 @@ */ package com.diffplug.spotless.maven.incremental; -import java.io.File; +import java.nio.file.Path; import org.apache.maven.plugin.logging.Log; import org.apache.maven.project.MavenProject; @@ -24,9 +24,9 @@ public interface UpToDateChecker extends AutoCloseable { - boolean isUpToDate(File file); + boolean isUpToDate(Path file); - void setUpToDate(File file); + void setUpToDate(Path file); void close(); diff --git a/plugin-maven/src/test/java/com/diffplug/spotless/maven/incremental/IndexBasedCheckerTest.java b/plugin-maven/src/test/java/com/diffplug/spotless/maven/incremental/IndexBasedCheckerTest.java index e6a62ba1cf..1f4e06bb39 100644 --- a/plugin-maven/src/test/java/com/diffplug/spotless/maven/incremental/IndexBasedCheckerTest.java +++ b/plugin-maven/src/test/java/com/diffplug/spotless/maven/incremental/IndexBasedCheckerTest.java @@ -17,7 +17,6 @@ import static org.assertj.core.api.Assertions.assertThat; -import java.io.File; import java.nio.file.Files; import java.nio.file.Path; import java.time.Instant; @@ -39,7 +38,7 @@ void beforeEach() { @Test void isUpToDateReturnsFalseForUnknownFile() throws Exception { Path sourceFile = createSourceFile("source.txt"); - assertThat(checker.isUpToDate(sourceFile.toFile())).isFalse(); + assertThat(checker.isUpToDate(sourceFile)).isFalse(); } @Test @@ -48,7 +47,7 @@ void isUpToDateReturnsTrueWhenOnDiskFileIsSameAsInTheIndex() throws Exception { Instant modifiedTime = Files.getLastModifiedTime(sourceFile).toInstant(); index.setLastModifiedTime(sourceFile, modifiedTime); - assertThat(checker.isUpToDate(sourceFile.toFile())).isTrue(); + assertThat(checker.isUpToDate(sourceFile)).isTrue(); } @Test @@ -57,7 +56,7 @@ void isUpToDateReturnsFalseWhenOnDiskFileIsNewerThanInTheIndex() throws Exceptio Instant modifiedTime = Files.getLastModifiedTime(sourceFile).toInstant().minusSeconds(42); index.setLastModifiedTime(sourceFile, modifiedTime); - assertThat(checker.isUpToDate(sourceFile.toFile())).isFalse(); + assertThat(checker.isUpToDate(sourceFile)).isFalse(); } /** @@ -71,19 +70,19 @@ void isUpToDateReturnsFalseWhenOnDiskFileIsOlderThanInTheIndex() throws Exceptio Instant modifiedTime = Files.getLastModifiedTime(sourceFile).toInstant().plusSeconds(42); index.setLastModifiedTime(sourceFile, modifiedTime); - assertThat(checker.isUpToDate(sourceFile.toFile())).isFalse(); + assertThat(checker.isUpToDate(sourceFile)).isFalse(); } @Test void setUpToDateUpdatesTheIndex() throws Exception { Path sourceFile = createSourceFile("source.txt"); assertThat(index.getLastModifiedTime(sourceFile)).isNull(); - assertThat(checker.isUpToDate(sourceFile.toFile())).isFalse(); + assertThat(checker.isUpToDate(sourceFile)).isFalse(); - checker.setUpToDate(sourceFile.toFile()); + checker.setUpToDate(sourceFile); assertThat(index.getLastModifiedTime(sourceFile)).isEqualTo(Files.getLastModifiedTime(sourceFile).toInstant()); - assertThat(checker.isUpToDate(sourceFile.toFile())).isTrue(); + assertThat(checker.isUpToDate(sourceFile)).isTrue(); } @Test @@ -91,7 +90,7 @@ void closeWritesTheIndex() throws Exception { Path sourceFile = createSourceFile("source.txt"); assertThat(index.getLastModifiedTime(sourceFile)).isNull(); - checker.setUpToDate(sourceFile.toFile()); + checker.setUpToDate(sourceFile); checker.close(); FileIndex newIndex = FileIndex.read(config, log); diff --git a/plugin-maven/src/test/java/com/diffplug/spotless/maven/incremental/NoopCheckerTest.java b/plugin-maven/src/test/java/com/diffplug/spotless/maven/incremental/NoopCheckerTest.java index bdf6ccedfa..ae719273e1 100644 --- a/plugin-maven/src/test/java/com/diffplug/spotless/maven/incremental/NoopCheckerTest.java +++ b/plugin-maven/src/test/java/com/diffplug/spotless/maven/incremental/NoopCheckerTest.java @@ -46,16 +46,16 @@ class NoopCheckerTest extends ResourceHarness { private MavenProject project; private Path indexFile; - private File existingSourceFile; - private File nonExistingSourceFile; + private Path existingSourceFile; + private Path nonExistingSourceFile; @BeforeEach void beforeEach() throws Exception { project = buildMavenProject(); indexFile = new FileIndexConfig(project).getIndexFile(); - existingSourceFile = new File(project.getBasedir(), "existing.txt"); - Files.write(existingSourceFile.toPath(), "foo".getBytes(UTF_8), CREATE_NEW); - nonExistingSourceFile = new File(project.getBasedir(), "non-existing.txt"); + existingSourceFile = project.getBasedir().toPath().resolve("existing.txt"); + Files.write(existingSourceFile, "foo".getBytes(UTF_8), CREATE_NEW); + nonExistingSourceFile = project.getBasedir().toPath().resolve("non-existing.txt"); } @Test From 0fa03f4dbf9cbf147cdea1f65b3f96bea3a3aed7 Mon Sep 17 00:00:00 2001 From: Kostiantyn Liutovych Date: Tue, 14 Dec 2021 15:19:29 +0100 Subject: [PATCH 16/21] Handle overflow when converting FileTime to Instant This conversion is needed because FileIndex stores human-readable timestamps in the index file. There's a way to convert FileTime to String but no way to convert String to FileTime when reading an index from a file. Thus, we convert to Instant that has well-defined `#toString()` and `#parse()` methods. Conversion from FileTime to Instant can be lossy because FileTime can represent timestamps further in the past/future than Instant. Such timestamps are saturated to Instant.MIN and Instant.MAX. It is not safe to record such timestamps in the index file because they are lossy. IndexBasedChecker will not record overflown Instants in the index. It will also log a warning for every file with a small/large last modified time that overflows an Instant. --- .../maven/incremental/IndexBasedChecker.java | 22 +++++++++++++------ .../incremental/IndexBasedCheckerTest.java | 2 +- 2 files changed, 16 insertions(+), 8 deletions(-) diff --git a/plugin-maven/src/main/java/com/diffplug/spotless/maven/incremental/IndexBasedChecker.java b/plugin-maven/src/main/java/com/diffplug/spotless/maven/incremental/IndexBasedChecker.java index 0722b8a296..e58c479f2e 100644 --- a/plugin-maven/src/main/java/com/diffplug/spotless/maven/incremental/IndexBasedChecker.java +++ b/plugin-maven/src/main/java/com/diffplug/spotless/maven/incremental/IndexBasedChecker.java @@ -19,7 +19,6 @@ import java.io.UncheckedIOException; import java.nio.file.Files; import java.nio.file.Path; -import java.nio.file.attribute.FileTime; import java.time.Instant; import java.util.Objects; @@ -32,17 +31,19 @@ class IndexBasedChecker implements UpToDateChecker { private final FileIndex index; + private final Log log; @VisibleForTesting - IndexBasedChecker(FileIndex index) { + IndexBasedChecker(FileIndex index, Log log) { this.index = index; + this.log = log; } static IndexBasedChecker create(MavenProject project, Iterable formatters, Log log) { PluginFingerprint pluginFingerprint = PluginFingerprint.from(project, formatters); FileIndexConfig indexConfig = new FileIndexConfig(project, pluginFingerprint); FileIndex fileIndex = FileIndex.read(indexConfig, log); - return new IndexBasedChecker(fileIndex); + return new IndexBasedChecker(fileIndex, log); } @Override @@ -53,7 +54,16 @@ public boolean isUpToDate(Path file) { @Override public void setUpToDate(Path file) { - index.setLastModifiedTime(file, lastModifiedTime(file)); + Instant lastModified = lastModifiedTime(file); + if (Instant.MIN.equals(lastModified) || Instant.MAX.equals(lastModified)) { + // FileTime can store timestamps further in the past/future than Instant. + // Such timestamps are saturated to Instant.MIN/Instant.MAX. + // Do not store such timestamps in the index because they are imprecise. + log.warn("File " + file + " has an approximated last modified time of " + lastModified + ". " + + "It will not be recorded in the up-to-date index."); + return; + } + index.setLastModifiedTime(file, lastModified); } @Override @@ -61,11 +71,9 @@ public void close() { index.write(); } - // todo: FileTime -> Instant can be a lossy conversion private static Instant lastModifiedTime(Path path) { try { - FileTime lastModifiedTime = Files.getLastModifiedTime(path); - return lastModifiedTime.toInstant(); + return Files.getLastModifiedTime(path).toInstant(); } catch (IOException e) { throw new UncheckedIOException("Unable to get last modified date for " + path, e); } diff --git a/plugin-maven/src/test/java/com/diffplug/spotless/maven/incremental/IndexBasedCheckerTest.java b/plugin-maven/src/test/java/com/diffplug/spotless/maven/incremental/IndexBasedCheckerTest.java index 1f4e06bb39..7137a32e1e 100644 --- a/plugin-maven/src/test/java/com/diffplug/spotless/maven/incremental/IndexBasedCheckerTest.java +++ b/plugin-maven/src/test/java/com/diffplug/spotless/maven/incremental/IndexBasedCheckerTest.java @@ -32,7 +32,7 @@ class IndexBasedCheckerTest extends FileIndexHarness { @BeforeEach void beforeEach() { index = FileIndex.read(config, log); - checker = new IndexBasedChecker(index); + checker = new IndexBasedChecker(index, log); } @Test From 17904287f1de26a74c87485f2c8b9d85fb7b8d44 Mon Sep 17 00:00:00 2001 From: Kostiantyn Liutovych Date: Tue, 14 Dec 2021 15:28:19 +0100 Subject: [PATCH 17/21] Record clean files in the index for spotless:check --- .../java/com/diffplug/spotless/maven/SpotlessCheckMojo.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/plugin-maven/src/main/java/com/diffplug/spotless/maven/SpotlessCheckMojo.java b/plugin-maven/src/main/java/com/diffplug/spotless/maven/SpotlessCheckMojo.java index aae7912a40..539ac3bdd6 100644 --- a/plugin-maven/src/main/java/com/diffplug/spotless/maven/SpotlessCheckMojo.java +++ b/plugin-maven/src/main/java/com/diffplug/spotless/maven/SpotlessCheckMojo.java @@ -48,8 +48,9 @@ protected void process(Iterable files, Formatter formatter, UpToDateChecke PaddedCell.DirtyState dirtyState = PaddedCell.calculateDirtyState(formatter, file); if (!dirtyState.isClean() && !dirtyState.didNotConverge()) { problemFiles.add(file); + } else { + upToDateChecker.setUpToDate(file.toPath()); } - // todo: else - mark file as up-to-date? } catch (IOException e) { throw new MojoExecutionException("Unable to format file " + file, e); } From 2728a06528a4dfc7e1e963ef36f143dfa8bd26d0 Mon Sep 17 00:00:00 2001 From: Kostiantyn Liutovych Date: Tue, 14 Dec 2021 17:08:56 +0100 Subject: [PATCH 18/21] Add integration tests for up-to-date checking The added tests assert on debug logging output of the apply and check goals. --- .../spotless/maven/SpotlessApplyMojo.java | 3 + .../spotless/maven/SpotlessCheckMojo.java | 3 + .../incremental/UpToDateCheckingTest.java | 175 +++++++++++++++++- 3 files changed, 171 insertions(+), 10 deletions(-) diff --git a/plugin-maven/src/main/java/com/diffplug/spotless/maven/SpotlessApplyMojo.java b/plugin-maven/src/main/java/com/diffplug/spotless/maven/SpotlessApplyMojo.java index 7995a75b8c..6ea67240ee 100644 --- a/plugin-maven/src/main/java/com/diffplug/spotless/maven/SpotlessApplyMojo.java +++ b/plugin-maven/src/main/java/com/diffplug/spotless/maven/SpotlessApplyMojo.java @@ -35,6 +35,9 @@ public class SpotlessApplyMojo extends AbstractSpotlessMojo { protected void process(Iterable files, Formatter formatter, UpToDateChecker upToDateChecker) throws MojoExecutionException { for (File file : files) { if (upToDateChecker.isUpToDate(file.toPath())) { + if (getLog().isDebugEnabled()) { + getLog().debug("Spotless will not format an up-to-date file: " + file); + } continue; } diff --git a/plugin-maven/src/main/java/com/diffplug/spotless/maven/SpotlessCheckMojo.java b/plugin-maven/src/main/java/com/diffplug/spotless/maven/SpotlessCheckMojo.java index 539ac3bdd6..ad3230f583 100644 --- a/plugin-maven/src/main/java/com/diffplug/spotless/maven/SpotlessCheckMojo.java +++ b/plugin-maven/src/main/java/com/diffplug/spotless/maven/SpotlessCheckMojo.java @@ -41,6 +41,9 @@ protected void process(Iterable files, Formatter formatter, UpToDateChecke List problemFiles = new ArrayList<>(); for (File file : files) { if (upToDateChecker.isUpToDate(file.toPath())) { + if (getLog().isDebugEnabled()) { + getLog().debug("Spotless will not check an up-to-date file: " + file); + } continue; } diff --git a/plugin-maven/src/test/java/com/diffplug/spotless/maven/incremental/UpToDateCheckingTest.java b/plugin-maven/src/test/java/com/diffplug/spotless/maven/incremental/UpToDateCheckingTest.java index 0b8b0ad252..ffcd87c1fe 100644 --- a/plugin-maven/src/test/java/com/diffplug/spotless/maven/incremental/UpToDateCheckingTest.java +++ b/plugin-maven/src/test/java/com/diffplug/spotless/maven/incremental/UpToDateCheckingTest.java @@ -17,33 +17,188 @@ import static org.assertj.core.api.Assertions.assertThat; +import java.io.File; +import java.io.IOException; +import java.util.ArrayList; +import java.util.List; + import org.junit.jupiter.api.Test; import com.diffplug.spotless.maven.MavenIntegrationHarness; -import com.diffplug.spotless.maven.MavenRunner.Result; +import com.diffplug.spotless.maven.MavenRunner; class UpToDateCheckingTest extends MavenIntegrationHarness { + @Test + void upToDateCheckingDisabledByDefault() throws Exception { + writePom( + "", + " ", + ""); + + List files = writeUnformattedFiles(1); + String output = runSpotlessApply(); + + assertThat(output).doesNotContain("Up-to-date checking enabled"); + assertFormatted(files); + } + @Test void enableUpToDateChecking() throws Exception { + writePomWithUpToDateCheckingEnabled(true); + + List files = writeUnformattedFiles(1); + String output = runSpotlessApply(); + + assertThat(output).contains("Up-to-date checking enabled"); + assertFormatted(files); + } + + @Test + void disableUpToDateChecking() throws Exception { + writePomWithUpToDateCheckingEnabled(false); + + List files = writeUnformattedFiles(1); + String output = runSpotlessApply(); + + assertThat(output).doesNotContain("Up-to-date checking enabled"); + assertFormatted(files); + } + + @Test + void spotlessApplyRecordsCorrectlyFormattedFiles() throws Exception { + writePomWithUpToDateCheckingEnabled(true); + List files = writeFormattedFiles(5); + + String applyOutput1 = runSpotlessApply(); + assertSpotlessApplyDidNotSkipAnyFiles(applyOutput1); + assertFormatted(files); + + String applyOutput2 = runSpotlessApply(); + assertSpotlessApplySkipped(files, applyOutput2); + + String checkOutput = runSpotlessCheck(); + assertSpotlessCheckSkipped(files, checkOutput); + } + + @Test + void spotlessApplyRecordsUnformattedFiles() throws Exception { + writePomWithUpToDateCheckingEnabled(true); + List files = writeUnformattedFiles(4); + + String applyOutput1 = runSpotlessApply(); + assertSpotlessApplyDidNotSkipAnyFiles(applyOutput1); + assertFormatted(files); + + String applyOutput2 = runSpotlessApply(); + assertSpotlessApplySkipped(files, applyOutput2); + + String checkOutput = runSpotlessCheck(); + assertSpotlessCheckSkipped(files, checkOutput); + } + + @Test + void spotlessCheckRecordsCorrectlyFormattedFiles() throws Exception { + writePomWithUpToDateCheckingEnabled(true); + List files = writeFormattedFiles(7); + + String checkOutput1 = runSpotlessCheck(); + assertSpotlessCheckDidNotSkipAnyFiles(checkOutput1); + + String checkOutput2 = runSpotlessCheck(); + assertSpotlessCheckSkipped(files, checkOutput2); + + String applyOutput = runSpotlessApply(); + assertSpotlessApplySkipped(files, applyOutput); + } + + @Test + void spotlessCheckRecordsUnformattedFiles() throws Exception { + writePomWithUpToDateCheckingEnabled(true); + List files = writeUnformattedFiles(6); + + String checkOutput1 = runSpotlessCheckOnUnformattedFiles(); + assertSpotlessCheckDidNotSkipAnyFiles(checkOutput1); + + String checkOutput2 = runSpotlessCheckOnUnformattedFiles(); + assertSpotlessCheckDidNotSkipAnyFiles(checkOutput2); + + String applyOutput = runSpotlessApply(); + assertSpotlessApplyDidNotSkipAnyFiles(applyOutput); + assertFormatted(files); + + String checkOutput3 = runSpotlessCheck(); + assertSpotlessCheckSkipped(files, checkOutput3); + } + + private void writePomWithUpToDateCheckingEnabled(boolean enabled) throws IOException { writePom( "", " ", "", "", - " true", + " " + enabled + "", ""); + } - String output = runTest("java/googlejavaformat/JavaCodeFormatted18.test"); + private List writeFormattedFiles(int count) throws IOException { + return writeFiles("java/googlejavaformat/JavaCodeFormatted18.test", "formatted", count); + } - assertThat(output).contains("Up-to-date checking enabled"); + private List writeUnformattedFiles(int count) throws IOException { + return writeFiles("java/googlejavaformat/JavaCodeUnformatted.test", "unformatted", count); + } + + private List writeFiles(String resource, String suffix, int count) throws IOException { + List result = new ArrayList<>(count); + for (int i = 0; i < count; i++) { + String path = "src/main/java/test_" + suffix + "_" + i + ".java"; + File file = setFile(path).toResource(resource); + result.add(file); + } + return result; + } + + private String runSpotlessApply() throws Exception { + return mavenRunnerForGoal("apply").runNoError().output(); + } + + private String runSpotlessCheck() throws Exception { + return mavenRunnerForGoal("check").runNoError().output(); + } + + private String runSpotlessCheckOnUnformattedFiles() throws Exception { + return mavenRunnerForGoal("check").runHasError().output(); + } + + private MavenRunner mavenRunnerForGoal(String goal) throws IOException { + // -X enables debug logging + return mavenRunner().withArguments("-X", "spotless:" + goal); + } + + private void assertFormatted(List files) throws IOException { + for (File file : files) { + assertFile(file).sameAsResource("java/googlejavaformat/JavaCodeFormatted18.test"); + } + } + + private void assertSpotlessApplyDidNotSkipAnyFiles(String applyOutput) { + assertThat(applyOutput).doesNotContain("Spotless will not format"); + } + + private void assertSpotlessCheckDidNotSkipAnyFiles(String checkOutput) { + assertThat(checkOutput).doesNotContain("Spotless will not format"); + } + + private void assertSpotlessApplySkipped(List files, String applyOutput) { + for (File file : files) { + assertThat(applyOutput).contains("Spotless will not format an up-to-date file: " + file); + } } - private String runTest(String targetResource) throws Exception { - String path = "src/main/java/test.java"; - setFile(path).toResource("java/googlejavaformat/JavaCodeUnformatted.test"); - Result result = mavenRunner().withArguments("spotless:apply").runNoError(); - assertFile(path).sameAsResource(targetResource); - return result.output(); + private void assertSpotlessCheckSkipped(List files, String checkOutput) { + for (File file : files) { + assertThat(checkOutput).contains("Spotless will not check an up-to-date file: " + file); + } } } From 2b0d591f26c5bd4bc593bddd63779a54935fe2fc Mon Sep 17 00:00:00 2001 From: Kostiantyn Liutovych Date: Thu, 16 Dec 2021 11:04:42 +0100 Subject: [PATCH 19/21] Fix a Spotbugs warning Path#getParent() can return null. This should not happen for the maven plugin up-to-date index. Thus, the fix is to check for null and throw an IllegalStateException. --- .../spotless/maven/incremental/FileIndex.java | 6 +++++- .../spotless/maven/incremental/FileIndexTest.java | 11 +++++++++++ 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/plugin-maven/src/main/java/com/diffplug/spotless/maven/incremental/FileIndex.java b/plugin-maven/src/main/java/com/diffplug/spotless/maven/incremental/FileIndex.java index d6a2e56e86..728f8a57bc 100644 --- a/plugin-maven/src/main/java/com/diffplug/spotless/maven/incremental/FileIndex.java +++ b/plugin-maven/src/main/java/com/diffplug/spotless/maven/incremental/FileIndex.java @@ -139,8 +139,12 @@ void write() { } private void ensureParentDirExists() { + Path parentDir = indexFile.getParent(); + if (parentDir == null) { + throw new IllegalStateException("Index file does not have a parent dir: " + indexFile); + } try { - Files.createDirectories(indexFile.getParent()); + Files.createDirectories(parentDir); } catch (IOException e) { throw new UncheckedIOException("Unable to create parent directory for the index file: " + indexFile, e); } diff --git a/plugin-maven/src/test/java/com/diffplug/spotless/maven/incremental/FileIndexTest.java b/plugin-maven/src/test/java/com/diffplug/spotless/maven/incremental/FileIndexTest.java index c12d00d218..ce9c0077e8 100644 --- a/plugin-maven/src/test/java/com/diffplug/spotless/maven/incremental/FileIndexTest.java +++ b/plugin-maven/src/test/java/com/diffplug/spotless/maven/incremental/FileIndexTest.java @@ -21,6 +21,7 @@ import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.verifyNoInteractions; +import static org.mockito.Mockito.when; import java.nio.file.Files; import java.nio.file.Path; @@ -252,4 +253,14 @@ void rewritesIndexFileThatReferencesNonExistingFile() throws Exception { FileIndex index3 = FileIndex.read(config, log); assertThat(index3.size()).isEqualTo(2); } + + @Test + void writeFailsWhenIndexFilesDoesNotHaveParentDir() { + when(config.getIndexFile()).thenReturn(Paths.get("file-without-parent")); + FileIndex index = FileIndex.read(config, log); + assertThat(index.size()).isZero(); + + assertThatThrownBy(index::write).isInstanceOf(IllegalStateException.class) + .hasMessageContaining("Index file does not have a parent dir"); + } } From ffc564f894e792104053ed6c7d780b7ea9fa9faf Mon Sep 17 00:00:00 2001 From: Kostiantyn Liutovych Date: Wed, 22 Dec 2021 19:10:09 +0100 Subject: [PATCH 20/21] Docs about up-to-date checking for Maven plugin --- README.md | 4 ++-- plugin-maven/README.md | 41 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 43 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index bdf428eb46..2c01d70459 100644 --- a/README.md +++ b/README.md @@ -42,7 +42,7 @@ output = [ '| Toggle with [`spotless:off` and `spotless:on`](plugin-gradle/#spotlessoff-and-spotlesson) | {{yes}} | {{yes}} | {{no}} | {{no}} |', '| [Ratchet from](https://github.com/diffplug/spotless/tree/main/plugin-gradle#ratchet) `origin/main` or other git ref | {{yes}} | {{yes}} | {{no}} | {{no}} |', '| Define [line endings using git](https://github.com/diffplug/spotless/tree/main/plugin-gradle#line-endings-and-encodings-invisible-stuff) | {{yes}} | {{yes}} | {{yes}} | {{no}} |', -'| Fast incremental format and up-to-date check | {{yes}} | {{no}} | {{no}} | {{no}} |', +'| Fast incremental format and up-to-date check | {{yes}} | {{yes}} | {{no}} | {{no}} |', '| Fast format on fresh checkout using buildcache | {{yes}} | {{no}} | {{no}} | {{no}} |', lib('generic.EndWithNewlineStep') +'{{yes}} | {{yes}} | {{no}} | {{no}} |', lib('generic.IndentStep') +'{{yes}} | {{yes}} | {{no}} | {{no}} |', @@ -81,7 +81,7 @@ extra('wtp.EclipseWtpFormatterStep') +'{{yes}} | {{yes}} | Toggle with [`spotless:off` and `spotless:on`](plugin-gradle/#spotlessoff-and-spotlesson) | :+1: | :+1: | :white_large_square: | :white_large_square: | | [Ratchet from](https://github.com/diffplug/spotless/tree/main/plugin-gradle#ratchet) `origin/main` or other git ref | :+1: | :+1: | :white_large_square: | :white_large_square: | | Define [line endings using git](https://github.com/diffplug/spotless/tree/main/plugin-gradle#line-endings-and-encodings-invisible-stuff) | :+1: | :+1: | :+1: | :white_large_square: | -| Fast incremental format and up-to-date check | :+1: | :white_large_square: | :white_large_square: | :white_large_square: | +| Fast incremental format and up-to-date check | :+1: | :+1: | :white_large_square: | :white_large_square: | | Fast format on fresh checkout using buildcache | :+1: | :white_large_square: | :white_large_square: | :white_large_square: | | [`generic.EndWithNewlineStep`](lib/src/main/java/com/diffplug/spotless/generic/EndWithNewlineStep.java) | :+1: | :+1: | :white_large_square: | :white_large_square: | | [`generic.IndentStep`](lib/src/main/java/com/diffplug/spotless/generic/IndentStep.java) | :+1: | :+1: | :white_large_square: | :white_large_square: | diff --git a/plugin-maven/README.md b/plugin-maven/README.md index a4ff8d6006..4b57e19ee8 100644 --- a/plugin-maven/README.md +++ b/plugin-maven/README.md @@ -900,6 +900,47 @@ If your project has not been rigorous with copyright headers, and you'd like to +## Incremental up-to-date checking and formatting + +**This feature is turned off by default.** + +Execution of `spotless:check` and `spotless:apply` for large projects can take time. +By default, Spotless Maven plugin needs to read and format each source file. +Repeated executions of `spotless:check` or `spotless:apply` are completely independent. + +If your project has many source files managed by Spotless and formatting takes a long time, you can +enable incremental up-to-date checking with the following configuration: + +```xml + + + true + + + +``` + +With up-to-date checking enabled, Spotless creates an index file in the `target` directory. +The index file contains source file paths and corresponding last modified timestamps. +It allows Spotless to skip already formatted files that have not changed. + +**Note:** the index file is located in the `target` directory. Executing `mvn clean` will delete +the index file, and Spotless will need to check/format all the source files. + +Spotless will remove the index file when up-to-date checking is explicitly turned off with the +following configuration: + +```xml + + + false + + + +``` + +Consider using this configuration if you experience issues with up-to-date checking. + ## How can I enforce formatting gradually? (aka "ratchet") If your project is not currently enforcing formatting, then it can be a noisy transition. Having a giant commit where every single file gets changed makes the history harder to read. To address this, you can use the `ratchet` feature: From a50a58f9c4f1b03fb4e1c0cd913e9db2f4ee7ead Mon Sep 17 00:00:00 2001 From: Kostiantyn Liutovych Date: Wed, 22 Dec 2021 19:10:15 +0100 Subject: [PATCH 21/21] Changelog about up-to-date checking for Maven plugin --- plugin-maven/CHANGES.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/plugin-maven/CHANGES.md b/plugin-maven/CHANGES.md index 51ba44cee0..d5f1ff5b74 100644 --- a/plugin-maven/CHANGES.md +++ b/plugin-maven/CHANGES.md @@ -3,6 +3,8 @@ We adhere to the [keepachangelog](https://keepachangelog.com/en/1.0.0/) format (starting after version `1.27.0`). ## [Unreleased] +### Added +* Incremental up-to-date checking ([#935](https://github.com/diffplug/spotless/pull/935)). ## [2.17.7] - 2021-12-16 ### Fixed