diff --git a/CHANGES.md b/CHANGES.md index ad8f0e0bad..610fe61eef 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -10,6 +10,7 @@ This document is intended for Spotless developers. We adhere to the [keepachangelog](https://keepachangelog.com/en/1.0.0/) format (starting after version `1.27.0`). ## [Unreleased] +* Add batching to IDEA formatter ([#2662](https://github.com/diffplug/spotless/pull/2662)) ## [4.0.0] - 2025-09-24 ### Changes diff --git a/lib/src/main/java/com/diffplug/spotless/generic/IdeaStep.java b/lib/src/main/java/com/diffplug/spotless/generic/IdeaStep.java index 5766e93b05..8bc250cacb 100644 --- a/lib/src/main/java/com/diffplug/spotless/generic/IdeaStep.java +++ b/lib/src/main/java/com/diffplug/spotless/generic/IdeaStep.java @@ -22,7 +22,9 @@ import java.nio.charset.StandardCharsets; import java.nio.file.Files; import java.nio.file.Path; +import java.util.ArrayList; import java.util.HashMap; +import java.util.LinkedHashMap; import java.util.List; import java.util.Locale; import java.util.Map; @@ -30,8 +32,9 @@ import java.util.Properties; import java.util.TreeMap; import java.util.UUID; +import java.util.concurrent.locks.Lock; +import java.util.concurrent.locks.ReentrantLock; import java.util.regex.Pattern; -import java.util.stream.Collectors; import java.util.stream.Stream; import javax.annotation.CheckForNull; @@ -57,6 +60,10 @@ public final class IdeaStep { public static final String IDEA_CONFIG_PATH_PROPERTY = "idea.config.path"; public static final String IDEA_SYSTEM_PATH_PROPERTY = "idea.system.path"; + + /** Default batch size for formatting multiple files in a single IDEA invocation */ + public static final int DEFAULT_BATCH_SIZE = 100; + @Nonnull private final IdeaStepBuilder builder; @@ -87,6 +94,7 @@ public static final class IdeaStepBuilder { private String binaryPath = IDEA_EXECUTABLE_DEFAULT; @Nullable private String codeStyleSettingsPath; private final Map ideaProperties = new HashMap<>(); + private int batchSize = DEFAULT_BATCH_SIZE; @Nonnull private final File buildDir; @@ -118,18 +126,34 @@ public IdeaStepBuilder setIdeaProperties(@Nonnull Map ideaProper return this; } + /** + * Sets the batch size for formatting multiple files in a single IDEA invocation. + * Default is {@link #DEFAULT_BATCH_SIZE}. + * + * @param batchSize the maximum number of files to format in a single batch (must be >= 1) + * @return this builder + */ + public IdeaStepBuilder setBatchSize(int batchSize) { + if (batchSize < 1) { + throw new IllegalArgumentException("Batch size must be at least 1, got: " + batchSize); + } + this.batchSize = batchSize; + return this; + } + public FormatterStep build() { return create(this); } @Override public String toString() { - return "IdeaStepBuilder[useDefaults=%s, binaryPath=%s, codeStyleSettingsPath=%s, ideaProperties=%s, buildDir=%s]".formatted( + return "IdeaStepBuilder[useDefaults=%s, binaryPath=%s, codeStyleSettingsPath=%s, ideaProperties=%s, buildDir=%s, batchSize=%d]".formatted( this.useDefaults, this.binaryPath, this.codeStyleSettingsPath, this.ideaProperties, - this.buildDir); + this.buildDir, + this.batchSize); } } @@ -142,6 +166,7 @@ private static final class State implements Serializable { @Nullable private final String codeStyleSettingsPath; private final boolean withDefaults; private final TreeMap ideaProperties; + private final int batchSize; private State(@Nonnull IdeaStepBuilder builder) { LOGGER.debug("Creating {} state with configuration {}", NAME, builder); @@ -150,6 +175,7 @@ private State(@Nonnull IdeaStepBuilder builder) { this.codeStyleSettingsPath = builder.codeStyleSettingsPath; this.ideaProperties = new TreeMap<>(builder.ideaProperties); this.binaryPath = resolveFullBinaryPathAndCheckVersion(builder.binaryPath); + this.batchSize = builder.batchSize; } private static String resolveFullBinaryPathAndCheckVersion(String binaryPath) { @@ -211,22 +237,50 @@ private static boolean isMacOs() { return System.getProperty("os.name").toLowerCase(Locale.ROOT).contains("mac"); } + /** + * Represents a file to be formatted, tracking both the temporary file and the original file reference. + */ + private static class FileToFormat { + final File tempFile; + final File originalFile; + + FileToFormat(File tempFile, File originalFile) { + this.tempFile = tempFile; + this.originalFile = originalFile; + } + } + private String format(IdeaStepFormatterCleanupResources ideaStepFormatterCleanupResources, String unix, File file) throws Exception { - // since we cannot directly work with the file, we need to write the unix string to a temporary file - File tempFile = Files.createTempFile("spotless", file.getName()).toFile(); - try { - Files.write(tempFile.toPath(), unix.getBytes(StandardCharsets.UTF_8)); - List params = getParams(tempFile); - - Map env = createEnv(); - LOGGER.info("Launching IDEA formatter for orig file {} with params: {} and env: {}", file, params, env); - var result = ideaStepFormatterCleanupResources.runner.exec(null, env, null, params); - LOGGER.debug("command finished with exit code: {}", result.exitCode()); - LOGGER.debug("command finished with stdout: {}", - result.assertExitZero(StandardCharsets.UTF_8)); - return Files.readString(tempFile.toPath()); - } finally { - Files.delete(tempFile.toPath()); + // Delegate to the batch formatter in the cleanup resources + return ideaStepFormatterCleanupResources.formatFile(this, unix, file); + } + + /** + * Formats multiple files in a single IDEA invocation. + * + * @param ideaStepFormatterCleanupResources the cleanup resources containing the process runner + * @param filesToFormat the list of files to format + * @throws Exception if formatting fails + */ + private void formatBatch(IdeaStepFormatterCleanupResources ideaStepFormatterCleanupResources, List filesToFormat) throws Exception { + if (filesToFormat.isEmpty()) { + return; + } + + LOGGER.info("Formatting batch of {} files with IDEA", filesToFormat.size()); + + List params = getParamsForBatch(filesToFormat); + Map env = createEnv(); + + LOGGER.debug("Launching IDEA formatter with params: {} and env: {}", params, env); + var result = ideaStepFormatterCleanupResources.runner.exec(null, env, null, params); + LOGGER.debug("Batch command finished with exit code: {}", result.exitCode()); + LOGGER.debug("Batch command finished with stdout: {}", result.assertExitZero(StandardCharsets.UTF_8)); + + // Read back the formatted content for each file + for (FileToFormat fileToFormat : filesToFormat) { + String formatted = Files.readString(fileToFormat.tempFile.toPath()); + ideaStepFormatterCleanupResources.cacheFormattedResult(fileToFormat.originalFile, formatted); } } @@ -266,7 +320,10 @@ private File createIdeaPropertiesFile() { return ideaProps.toFile(); } - private List getParams(File file) { + /** + * Builds command-line parameters for formatting multiple files in a single invocation. + */ + private List getParamsForBatch(List filesToFormat) { /* https://www.jetbrains.com/help/idea/command-line-formatter.html */ var builder = Stream. builder(); builder.add(binaryPath); @@ -278,13 +335,18 @@ private List getParams(File file) { builder.add("-s"); builder.add(codeStyleSettingsPath); } - builder.add("-charset").add("UTF-8"); - builder.add(ThrowingEx.get(file::getCanonicalPath)); - return builder.build().collect(Collectors.toList()); + builder.add("-charset"); + builder.add("UTF-8"); + + // Add all file paths + for (FileToFormat fileToFormat : filesToFormat) { + builder.add(ThrowingEx.get(fileToFormat.tempFile::getCanonicalPath)); + } + return builder.build().toList(); } private FormatterFunc.Closeable toFunc() { - IdeaStepFormatterCleanupResources ideaStepFormatterCleanupResources = new IdeaStepFormatterCleanupResources(uniqueBuildFolder, new ProcessRunner()); + IdeaStepFormatterCleanupResources ideaStepFormatterCleanupResources = new IdeaStepFormatterCleanupResources(uniqueBuildFolder, new ProcessRunner(), batchSize); return FormatterFunc.Closeable.of(ideaStepFormatterCleanupResources, this::format); } } @@ -294,14 +356,116 @@ private static class IdeaStepFormatterCleanupResources implements AutoCloseable private final File uniqueBuildFolder; @Nonnull private final ProcessRunner runner; + private final int batchSize; + + // Batch processing state (transient - not serialized) + private final Map formattedCache = new LinkedHashMap<>(); + private final List pendingBatch = new ArrayList<>(); + private final Lock batchLock = new ReentrantLock(); + private State currentState; - public IdeaStepFormatterCleanupResources(@Nonnull File uniqueBuildFolder, @Nonnull ProcessRunner runner) { + public IdeaStepFormatterCleanupResources(@Nonnull File uniqueBuildFolder, @Nonnull ProcessRunner runner, int batchSize) { this.uniqueBuildFolder = uniqueBuildFolder; this.runner = runner; + this.batchSize = batchSize; + } + + /** + * Formats a single file, using batch processing for efficiency. + * Files are accumulated and formatted in batches to minimize IDEA process startups. + */ + String formatFile(State state, String unix, File file) throws Exception { + batchLock.lock(); + try { + // Store the state reference for batch processing + if (currentState == null) { + currentState = state; + } + + // Check if we already have the formatted result cached + if (formattedCache.containsKey(file)) { + String result = formattedCache.remove(file); + LOGGER.debug("Returning cached formatted result for file: {}", file); + return result; + } + + // Create a temporary file for this content + File tempFile = Files.createTempFile(uniqueBuildFolder.toPath(), "spotless", file.getName()).toFile(); + Files.write(tempFile.toPath(), unix.getBytes(StandardCharsets.UTF_8)); + + // Add to pending batch + pendingBatch.add(new State.FileToFormat(tempFile, file)); + LOGGER.debug("Added file {} to pending batch (size: {})", file, pendingBatch.size()); + + // If batch is full, process it + if (pendingBatch.size() >= batchSize) { + LOGGER.info("Batch size reached ({}/{}), processing batch", pendingBatch.size(), batchSize); + processPendingBatch(); + } + + // Check cache again after potential batch processing + if (formattedCache.containsKey(file)) { + return formattedCache.remove(file); + } + + // If still not in cache, we need to process immediately (shouldn't happen normally) + // This is a safety fallback + LOGGER.warn("File {} not found in cache after batch processing, forcing immediate format", file); + List singleFileBatch = new ArrayList<>(); + singleFileBatch.add(new State.FileToFormat(tempFile, file)); + currentState.formatBatch(this, singleFileBatch); + return formattedCache.remove(file); + + } finally { + batchLock.unlock(); + } + } + + /** + * Caches a formatted result for a file. + */ + void cacheFormattedResult(File originalFile, String formatted) { + formattedCache.put(originalFile, formatted); + } + + /** + * Processes all pending files in the current batch. + */ + private void processPendingBatch() throws Exception { + if (pendingBatch.isEmpty() || currentState == null) { + return; + } + + List batchToProcess = new ArrayList<>(pendingBatch); + pendingBatch.clear(); + + try { + currentState.formatBatch(this, batchToProcess); + } finally { + // Clean up temp files + for (State.FileToFormat fileToFormat : batchToProcess) { + try { + Files.deleteIfExists(fileToFormat.tempFile.toPath()); + } catch (IOException e) { + LOGGER.warn("Failed to delete temporary file: {}", fileToFormat.tempFile, e); + } + } + } } @Override public void close() throws Exception { + batchLock.lock(); + try { + // Process any remaining files in the batch + if (!pendingBatch.isEmpty()) { + LOGGER.info("Processing remaining {} files in batch on close", pendingBatch.size()); + processPendingBatch(); + } + } finally { + batchLock.unlock(); + } + // close the runner runner.close(); // delete the unique build folder diff --git a/plugin-gradle/CHANGES.md b/plugin-gradle/CHANGES.md index efe193a3b4..7da69d116e 100644 --- a/plugin-gradle/CHANGES.md +++ b/plugin-gradle/CHANGES.md @@ -3,6 +3,8 @@ We adhere to the [keepachangelog](https://keepachangelog.com/en/1.0.0/) format (starting after version `3.27.0`). ## [Unreleased] +### Added +* Add `batchSize` to `` formatter which determines the number of files to format in a single IDEA invocation (default=100) ([#2662](https://github.com/diffplug/spotless/pull/2662)) ## [8.0.0] - 2025-09-24 ### Changed diff --git a/plugin-gradle/README.md b/plugin-gradle/README.md index ac7b0f4ede..ae1e0c1608 100644 --- a/plugin-gradle/README.md +++ b/plugin-gradle/README.md @@ -1657,6 +1657,9 @@ spotless { // if idea is not on your path, you must specify the path to the executable idea().binaryPath('/path/to/idea') + + // to set the number of files per IDEA involcation (default: 100) + idea().batchSize(100) } } ``` @@ -1666,7 +1669,6 @@ See [here](../INTELLIJ_IDEA_SCREENSHOTS.md) for an explanation on how to extract ### Limitations - Currently, only IntelliJ IDEA is supported - none of the other jetbrains IDE. Consider opening a PR if you want to change this. -- Launching IntelliJ IDEA from the command line is pretty expensive and as of now, we do this for each file. If you want to change this, consider opening a PR. ## Generic steps diff --git a/plugin-maven/CHANGES.md b/plugin-maven/CHANGES.md index 1100a48c06..eab64c1178 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 +* Add `batchSize` to `` formatter which determines the number of files to format in a single IDEA invocation (default=100) ([#2662](https://github.com/diffplug/spotless/pull/2662)) ## [3.0.0] - 2025-09-24 ### Changes diff --git a/plugin-maven/README.md b/plugin-maven/README.md index 7de87eb5fa..97f33c1b26 100644 --- a/plugin-maven/README.md +++ b/plugin-maven/README.md @@ -1733,6 +1733,8 @@ Spotless provides access to IntelliJ IDEA's command line formatter. false /path/to/idea + + 100 @@ -1744,7 +1746,6 @@ See [here](../INTELLIJ_IDEA_SCREENSHOTS.md) for an explanation on how to extract ### Limitations - Currently, only IntelliJ IDEA is supported - none of the other jetbrains IDE. Consider opening a PR if you want to change this. -- Launching IntelliJ IDEA from the command line is pretty expensive and as of now, we do this for each file. If you want to change this, consider opening a PR. ## Generic steps diff --git a/plugin-maven/src/main/java/com/diffplug/spotless/maven/generic/Idea.java b/plugin-maven/src/main/java/com/diffplug/spotless/maven/generic/Idea.java index 2c3b5587a3..7b96ab4fed 100644 --- a/plugin-maven/src/main/java/com/diffplug/spotless/maven/generic/Idea.java +++ b/plugin-maven/src/main/java/com/diffplug/spotless/maven/generic/Idea.java @@ -33,12 +33,16 @@ public class Idea implements FormatterStepFactory { @Parameter private Boolean withDefaults = true; + @Parameter + private Integer batchSize = IdeaStep.DEFAULT_BATCH_SIZE; + @Override public FormatterStep newFormatterStep(FormatterStepConfig config) { return IdeaStep.newBuilder(config.getFileLocator().getBuildDir()) .setUseDefaults(withDefaults) .setCodeStyleSettingsPath(codeStyleSettingsPath) .setBinaryPath(binaryPath) + .setBatchSize(batchSize) .build(); } } diff --git a/testlib/src/test/java/com/diffplug/spotless/generic/IdeaStepTest.java b/testlib/src/test/java/com/diffplug/spotless/generic/IdeaStepTest.java index d7dbc8025a..1db67f13f9 100644 --- a/testlib/src/test/java/com/diffplug/spotless/generic/IdeaStepTest.java +++ b/testlib/src/test/java/com/diffplug/spotless/generic/IdeaStepTest.java @@ -104,6 +104,109 @@ void configureFile() throws Exception { "formatting was applied to clean file"); } + @Test + void batchFormattingSingleBatch() throws Exception { + // Test formatting multiple files in a single batch + String dirtyJava = ResourceHarness.getTestResource("java/idea/full.dirty.java"); + FormatterStep step = IdeaStep.newBuilder(buildDir()) + .setUseDefaults(true) + .setBatchSize(5) // Batch size larger than number of files + .build(); + + // Format 3 files - should all be in one batch + for (int i = 0; i < 3; i++) { + File dirtyFile = newFile("dirty" + i + ".java"); + Files.write(dirtyJava, dirtyFile, StandardCharsets.UTF_8); + var result = step.format(dirtyJava, dirtyFile); + Assertions.assertNotEquals(dirtyJava, result, + "file " + i + " was not formatted"); + } + } + + @Test + void batchFormattingMultipleBatches() throws Exception { + // Test formatting files across exactly 3 batches + String dirtyJava = ResourceHarness.getTestResource("java/idea/full.dirty.java"); + FormatterStep step = IdeaStep.newBuilder(buildDir()) + .setUseDefaults(true) + .setBatchSize(2) // Small batch size to force multiple batches + .build(); + + // Format 6 files - should be exactly 3 batches (2+2+2) + for (int i = 0; i < 6; i++) { + File dirtyFile = newFile("batch_dirty" + i + ".java"); + Files.write(dirtyJava, dirtyFile, StandardCharsets.UTF_8); + var result = step.format(dirtyJava, dirtyFile); + Assertions.assertNotEquals(dirtyJava, result, + "file " + i + " was not formatted in batch"); + } + } + + @Test + void batchFormattingPartialLastBatch() throws Exception { + // Test formatting files with a partial last batch + String dirtyJava = ResourceHarness.getTestResource("java/idea/full.dirty.java"); + FormatterStep step = IdeaStep.newBuilder(buildDir()) + .setUseDefaults(true) + .setBatchSize(2) // Small batch size + .build(); + + // Format 7 files - should be 4 batches (2+2+2+1) + for (int i = 0; i < 7; i++) { + File dirtyFile = newFile("partial_dirty" + i + ".java"); + Files.write(dirtyJava, dirtyFile, StandardCharsets.UTF_8); + var result = step.format(dirtyJava, dirtyFile); + Assertions.assertNotEquals(dirtyJava, result, + "file " + i + " was not formatted with partial batch"); + } + } + + @Test + void batchFormattingMixedCleanAndDirty() throws Exception { + // Test formatting a mix of clean and dirty files across multiple batches + String dirtyJava = ResourceHarness.getTestResource("java/idea/full.dirty.java"); + String cleanJava = ResourceHarness.getTestResource("java/idea/full.clean.java"); + FormatterStep step = IdeaStep.newBuilder(buildDir()) + .setUseDefaults(true) + .setBatchSize(3) + .build(); + + // Format 9 files (3 batches) - alternating dirty and clean + for (int i = 0; i < 9; i++) { + boolean isDirty = i % 2 == 0; + String content = isDirty ? dirtyJava : cleanJava; + File file = newFile("mixed" + i + ".java"); + Files.write(content, file, StandardCharsets.UTF_8); + var result = step.format(content, file); + + if (isDirty) { + Assertions.assertNotEquals(content, result, + "dirty file " + i + " was not formatted"); + } else { + Assertions.assertEquals(content, result, + "clean file " + i + " was incorrectly modified"); + } + } + } + + @Test + void batchSizeValidation() { + // Test that batch size must be at least 1 + Assertions.assertThrows(IllegalArgumentException.class, () -> { + IdeaStep.newBuilder(buildDir()) + .setUseDefaults(true) + .setBatchSize(0) + .build(); + }, "batch size of 0 should throw exception"); + + Assertions.assertThrows(IllegalArgumentException.class, () -> { + IdeaStep.newBuilder(buildDir()) + .setUseDefaults(true) + .setBatchSize(-1) + .build(); + }, "negative batch size should throw exception"); + } + private File buildDir; protected File buildDir() {