From da3b75b4cc8e50960d3dbbf2601d1ba4ab9613f1 Mon Sep 17 00:00:00 2001 From: Simon Gamma Date: Mon, 7 Apr 2025 20:16:56 +0200 Subject: [PATCH 01/11] fix: separate node_modules dir per npm based formatter without this change, using the exact same npm-based formatter (e.g. prettier) in multiple formatters, we could end up using the same directory to store their node_modules inside. This could lead - especially when using parallel builds, to a lot of issues: - overwriting each others node_modules mid-flight - overwriting package.json mid-flight - starting multiple npm-based servers on the same directory (overwriting the port-file thus leading to cross-access between formatter steps and their corresponding node server). By applying this fix, each formatter will have its own separate node_modules directory. --- .../spotless/npm/EslintFormatterStep.java | 9 +- .../npm/NpmFormatterStepStateBase.java | 4 +- .../spotless/npm/PrettierFormatterStep.java | 9 +- .../gradle/spotless/FormatExtension.java | 2 +- .../gradle/spotless/JavascriptExtension.java | 4 +- .../gradle/spotless/TypescriptExtension.java | 4 +- .../spotless/PrettierIntegrationTest.java | 91 +++++++++++++++---- .../spotless/maven/FormatterFactory.java | 10 +- .../spotless/maven/FormatterStepConfig.java | 10 +- .../spotless/maven/FormattersHolder.java | 13 ++- .../spotless/maven/generic/Prettier.java | 4 +- .../maven/javascript/AbstractEslint.java | 4 +- .../prettier/PrettierFormatStepTest.java | 36 +++++++- .../spotless/npm/EslintFormatterStepTest.java | 5 +- .../npm/PrettierFormatterStepTest.java | 10 +- 15 files changed, 163 insertions(+), 52 deletions(-) diff --git a/lib/src/main/java/com/diffplug/spotless/npm/EslintFormatterStep.java b/lib/src/main/java/com/diffplug/spotless/npm/EslintFormatterStep.java index 70beaf91ca..d32f4e58b9 100644 --- a/lib/src/main/java/com/diffplug/spotless/npm/EslintFormatterStep.java +++ b/lib/src/main/java/com/diffplug/spotless/npm/EslintFormatterStep.java @@ -1,5 +1,5 @@ /* - * Copyright 2016-2024 DiffPlug + * Copyright 2016-2025 DiffPlug * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -68,13 +68,14 @@ public static Map defaultDevDependenciesWithEslint(String versio return Collections.singletonMap("eslint", version); } - public static FormatterStep create(Map devDependencies, Provisioner provisioner, File projectDir, File buildDir, File cacheDir, NpmPathResolver npmPathResolver, EslintConfig eslintConfig) { + public static FormatterStep create(String formatName, Map devDependencies, Provisioner provisioner, File projectDir, File buildDir, File cacheDir, NpmPathResolver npmPathResolver, EslintConfig eslintConfig) { requireNonNull(devDependencies); requireNonNull(provisioner); requireNonNull(projectDir); requireNonNull(buildDir); - return FormatterStep.createLazy(NAME, - () -> new State(NAME, devDependencies, projectDir, buildDir, cacheDir, npmPathResolver, eslintConfig), + final String prefixedName = String.format("%s-%s", formatName, NAME); + return FormatterStep.createLazy(prefixedName, + () -> new State(prefixedName, devDependencies, projectDir, buildDir, cacheDir, npmPathResolver, eslintConfig), State::createFormatterFunc); } diff --git a/lib/src/main/java/com/diffplug/spotless/npm/NpmFormatterStepStateBase.java b/lib/src/main/java/com/diffplug/spotless/npm/NpmFormatterStepStateBase.java index 3ec06e0434..dd798e6e55 100644 --- a/lib/src/main/java/com/diffplug/spotless/npm/NpmFormatterStepStateBase.java +++ b/lib/src/main/java/com/diffplug/spotless/npm/NpmFormatterStepStateBase.java @@ -1,5 +1,5 @@ /* - * Copyright 2016-2024 DiffPlug + * Copyright 2016-2025 DiffPlug * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -69,7 +69,7 @@ public static class Runtime { Runtime(NpmFormatterStepStateBase parent) { this.parent = parent; - this.nodeServerLayout = new NodeServerLayout(parent.locations.buildDir(), parent.npmConfig.getPackageJsonContent()); + this.nodeServerLayout = new NodeServerLayout(new File(parent.locations.buildDir(), parent.stepName), parent.npmConfig.getPackageJsonContent()); this.nodeServeApp = new NodeServeApp(nodeServerLayout, parent.npmConfig, parent.locations); } diff --git a/lib/src/main/java/com/diffplug/spotless/npm/PrettierFormatterStep.java b/lib/src/main/java/com/diffplug/spotless/npm/PrettierFormatterStep.java index 27a1002df5..0f81934e24 100644 --- a/lib/src/main/java/com/diffplug/spotless/npm/PrettierFormatterStep.java +++ b/lib/src/main/java/com/diffplug/spotless/npm/PrettierFormatterStep.java @@ -1,5 +1,5 @@ /* - * Copyright 2016-2024 DiffPlug + * Copyright 2016-2025 DiffPlug * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -51,12 +51,13 @@ public static final Map defaultDevDependenciesWithPrettier(Strin return Collections.singletonMap("prettier", version); } - public static FormatterStep create(Map devDependencies, Provisioner provisioner, File projectDir, File buildDir, File cacheDir, NpmPathResolver npmPathResolver, PrettierConfig prettierConfig) { + public static FormatterStep create(String formatName, Map devDependencies, Provisioner provisioner, File projectDir, File buildDir, File cacheDir, NpmPathResolver npmPathResolver, PrettierConfig prettierConfig) { requireNonNull(devDependencies); requireNonNull(provisioner); requireNonNull(buildDir); - return FormatterStep.createLazy(NAME, - () -> new State(NAME, devDependencies, projectDir, buildDir, cacheDir, npmPathResolver, prettierConfig), + final String prefixedName = String.format("%s-%s", formatName, NAME); + return FormatterStep.createLazy(prefixedName, + () -> new State(prefixedName, devDependencies, projectDir, buildDir, cacheDir, npmPathResolver, prettierConfig), State::createFormatterFunc); } diff --git a/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/FormatExtension.java b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/FormatExtension.java index 69150724c1..712e6fe00a 100644 --- a/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/FormatExtension.java +++ b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/FormatExtension.java @@ -798,7 +798,7 @@ public PrettierConfig config(final Map prettierConfig) { @Override protected FormatterStep createStep() { final Project project = getProject(); - return PrettierFormatterStep.create(devDependencies, provisioner(), project.getProjectDir(), + return PrettierFormatterStep.create(formatName(), devDependencies, provisioner(), project.getProjectDir(), project.getLayout().getBuildDirectory().getAsFile().get(), npmModulesCacheOrNull(), new NpmPathResolver(npmFileOrNull(), nodeFileOrNull(), npmrcFileOrNull(), Arrays.asList(project.getProjectDir(), project.getRootDir())), diff --git a/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/JavascriptExtension.java b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/JavascriptExtension.java index 5532045bc2..7968f86e28 100644 --- a/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/JavascriptExtension.java +++ b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/JavascriptExtension.java @@ -1,5 +1,5 @@ /* - * Copyright 2016-2024 DiffPlug + * Copyright 2016-2025 DiffPlug * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -106,7 +106,7 @@ public JavascriptEslintConfig(Map devDependencies) { public FormatterStep createStep() { final Project project = getProject(); - return EslintFormatterStep.create(devDependencies, provisioner(), project.getProjectDir(), + return EslintFormatterStep.create(NAME, devDependencies, provisioner(), project.getProjectDir(), project.getLayout().getBuildDirectory().getAsFile().get(), npmModulesCacheOrNull(), new NpmPathResolver(npmFileOrNull(), nodeFileOrNull(), npmrcFileOrNull(), Arrays.asList(project.getProjectDir(), project.getRootDir())), diff --git a/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/TypescriptExtension.java b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/TypescriptExtension.java index 39e050d51e..9794a3c349 100644 --- a/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/TypescriptExtension.java +++ b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/TypescriptExtension.java @@ -1,5 +1,5 @@ /* - * Copyright 2016-2024 DiffPlug + * Copyright 2016-2025 DiffPlug * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -214,7 +214,7 @@ public TypescriptEslintConfig tsconfigFile(Object path) { public FormatterStep createStep() { final Project project = getProject(); - return EslintFormatterStep.create(devDependencies, provisioner(), project.getProjectDir(), + return EslintFormatterStep.create(NAME, devDependencies, provisioner(), project.getProjectDir(), project.getLayout().getBuildDirectory().getAsFile().get(), npmModulesCacheOrNull(), new NpmPathResolver(npmFileOrNull(), nodeFileOrNull(), npmrcFileOrNull(), Arrays.asList(project.getProjectDir(), project.getRootDir())), diff --git a/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/PrettierIntegrationTest.java b/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/PrettierIntegrationTest.java index b6547204b8..6c85949861 100644 --- a/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/PrettierIntegrationTest.java +++ b/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/PrettierIntegrationTest.java @@ -1,5 +1,5 @@ /* - * Copyright 2016-2024 DiffPlug + * Copyright 2016-2025 DiffPlug * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -15,10 +15,18 @@ */ package com.diffplug.gradle.spotless; +import static org.assertj.core.api.Assertions.assertThat; + +import java.io.File; import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.List; +import java.util.stream.Collectors; +import java.util.stream.Stream; -import org.assertj.core.api.Assertions; import org.gradle.testkit.runner.BuildResult; +import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.ValueSource; @@ -51,7 +59,7 @@ void useInlineConfig(String prettierVersion) throws IOException { "}"); setFile("test.ts").toResource("npm/prettier/config/typescript.dirty"); final BuildResult spotlessApply = gradleRunner().withArguments("--stacktrace", "spotlessApply").build(); - Assertions.assertThat(spotlessApply.getOutput()).contains("BUILD SUCCESSFUL"); + assertThat(spotlessApply.getOutput()).contains("BUILD SUCCESSFUL"); switch (prettierVersion) { case PRETTIER_VERSION_2: assertFile("test.ts").sameAsResource("npm/prettier/config/typescript.configfile_prettier_2.clean"); @@ -81,7 +89,7 @@ void verifyCleanSpotlessCheckWorks(String prettierVersion) throws IOException { "}"); setFile("test.ts").toResource("npm/prettier/config/typescript.dirty"); final BuildResult spotlessCheckFailsGracefully = gradleRunner().withArguments("--stacktrace", "spotlessCheck").buildAndFail(); - Assertions.assertThat(spotlessCheckFailsGracefully.getOutput()).contains("> The following files had format violations:"); + assertThat(spotlessCheckFailsGracefully.getOutput()).contains("> The following files had format violations:"); gradleRunner().withArguments("--stacktrace", "spotlessApply").build(); gradleRunner().withArguments("--stacktrace", "spotlessCheck").build(); @@ -104,7 +112,7 @@ void useFileConfig(String prettierVersion) throws IOException { "}"); setFile("test.ts").toResource("npm/prettier/config/typescript.dirty"); final BuildResult spotlessApply = gradleRunner().withArguments("--stacktrace", "spotlessApply").build(); - Assertions.assertThat(spotlessApply.getOutput()).contains("BUILD SUCCESSFUL"); + assertThat(spotlessApply.getOutput()).contains("BUILD SUCCESSFUL"); switch (prettierVersion) { case PRETTIER_VERSION_2: assertFile("test.ts").sameAsResource("npm/prettier/config/typescript.configfile_prettier_2.clean"); @@ -131,7 +139,7 @@ void chooseParserBasedOnFilename(String prettierVersion) throws IOException { "}"); setFile("dirty.json").toResource("npm/prettier/filename/dirty.json"); final BuildResult spotlessApply = gradleRunner().withArguments("--stacktrace", "spotlessApply").build(); - Assertions.assertThat(spotlessApply.getOutput()).contains("BUILD SUCCESSFUL"); + assertThat(spotlessApply.getOutput()).contains("BUILD SUCCESSFUL"); assertFile("dirty.json").sameAsResource("npm/prettier/filename/clean.json"); } @@ -169,7 +177,7 @@ void useJavaCommunityPlugin(String prettierVersion) throws IOException { "}"); setFile("JavaTest.java").toResource("npm/prettier/plugins/java-test.dirty"); final BuildResult spotlessApply = gradleRunner().withArguments("--stacktrace", "spotlessApply").build(); - Assertions.assertThat(spotlessApply.getOutput()).contains("BUILD SUCCESSFUL"); + assertThat(spotlessApply.getOutput()).contains("BUILD SUCCESSFUL"); assertFile("JavaTest.java").sameAsResource("npm/prettier/plugins/java-test.clean"); } @@ -202,7 +210,7 @@ void useJavaCommunityPluginFileConfig(String prettierVersion) throws IOException "}"); setFile("JavaTest.java").toResource("npm/prettier/plugins/java-test.dirty"); final BuildResult spotlessApply = gradleRunner().withArguments("--stacktrace", "spotlessApply").build(); - Assertions.assertThat(spotlessApply.getOutput()).contains("BUILD SUCCESSFUL"); + assertThat(spotlessApply.getOutput()).contains("BUILD SUCCESSFUL"); assertFile("JavaTest.java").sameAsResource("npm/prettier/plugins/java-test.clean"); } @@ -226,8 +234,8 @@ void suggestsMissingJavaCommunityPlugin(String prettierVersion) throws IOExcepti "}"); setFile("JavaTest.java").toResource("npm/prettier/plugins/java-test.dirty"); final BuildResult spotlessApply = gradleRunner().withArguments("--stacktrace", "spotlessApply").buildAndFail(); - Assertions.assertThat(spotlessApply.getOutput()).contains("Could not infer a parser"); - Assertions.assertThat(spotlessApply.getOutput()).contains("prettier-plugin-java"); + assertThat(spotlessApply.getOutput()).contains("Could not infer a parser"); + assertThat(spotlessApply.getOutput()).contains("prettier-plugin-java"); } @ParameterizedTest(name = "{index}: usePhpCommunityPlugin with prettier {0}") @@ -264,7 +272,7 @@ void usePhpCommunityPlugin(String prettierVersion) throws IOException { "}"); setFile("php-example.php").toResource("npm/prettier/plugins/php.dirty"); final BuildResult spotlessApply = gradleRunner().withArguments("--stacktrace", "spotlessApply").build(); - Assertions.assertThat(spotlessApply.getOutput()).contains("BUILD SUCCESSFUL"); + assertThat(spotlessApply.getOutput()).contains("BUILD SUCCESSFUL"); assertFile("php-example.php").sameAsResource("npm/prettier/plugins/php.clean"); } @@ -324,9 +332,9 @@ void usePhpAndJavaCommunityPlugin(String prettierVersion) throws IOException { setFile("php-example.php").toResource("npm/prettier/plugins/php.dirty"); setFile("JavaTest.java").toResource("npm/prettier/plugins/java-test.dirty"); final BuildResult spotlessApply = gradleRunner().forwardOutput().withArguments("--stacktrace", "--info", "spotlessApply").build(); - Assertions.assertThat(spotlessApply.getOutput()).contains("BUILD SUCCESSFUL"); + assertThat(spotlessApply.getOutput()).contains("BUILD SUCCESSFUL"); final BuildResult spotlessApply2 = gradleRunner().forwardOutput().withArguments("--stacktrace", "--info", "spotlessApply").build(); - Assertions.assertThat(spotlessApply2.getOutput()).contains("BUILD SUCCESSFUL"); + assertThat(spotlessApply2.getOutput()).contains("BUILD SUCCESSFUL"); assertFile("php-example.php").sameAsResource("npm/prettier/plugins/php.clean"); assertFile("JavaTest.java").sameAsResource("npm/prettier/plugins/java-test.clean"); } @@ -355,7 +363,7 @@ void autodetectNpmrcFileConfig(String prettierVersion) throws IOException { "}"); setFile("test.ts").toResource("npm/prettier/config/typescript.dirty"); final BuildResult spotlessApply = gradleRunner().withArguments("--stacktrace", "spotlessApply").buildAndFail(); - Assertions.assertThat(spotlessApply.getOutput()).containsPattern("Running npm command.*npm install.* failed with exit code: 1"); + assertThat(spotlessApply.getOutput()).containsPattern("Running npm command.*npm install.* failed with exit code: 1"); } @ParameterizedTest(name = "{index}: verifyCleanAndSpotlessWorks with prettier {0}") @@ -377,9 +385,9 @@ void verifyCleanAndSpotlessWorks(String prettierVersion) throws IOException { "}"); setFile("test.ts").toResource("npm/prettier/config/typescript.dirty"); final BuildResult spotlessApply = gradleRunner().withArguments("--stacktrace", "clean", "spotlessApply").build(); - Assertions.assertThat(spotlessApply.getOutput()).contains("BUILD SUCCESSFUL"); + assertThat(spotlessApply.getOutput()).contains("BUILD SUCCESSFUL"); final BuildResult spotlessApply2 = gradleRunner().withArguments("--stacktrace", "clean", "spotlessApply").build(); - Assertions.assertThat(spotlessApply2.getOutput()).contains("BUILD SUCCESSFUL"); + assertThat(spotlessApply2.getOutput()).contains("BUILD SUCCESSFUL"); } @ParameterizedTest(name = "{index}: verifyCleanAndSpotlessWithNpmInstallCacheWorks with prettier {0}") @@ -401,9 +409,9 @@ void verifyCleanAndSpotlessWithNpmInstallCacheWorks(String prettierVersion) thro "}"); setFile("test.ts").toResource("npm/prettier/config/typescript.dirty"); final BuildResult spotlessApply = gradleRunner().withArguments("--stacktrace", "clean", "spotlessApply").build(); - Assertions.assertThat(spotlessApply.getOutput()).contains("BUILD SUCCESSFUL"); + assertThat(spotlessApply.getOutput()).contains("BUILD SUCCESSFUL"); final BuildResult spotlessApply2 = gradleRunner().withArguments("--stacktrace", "clean", "spotlessApply").build(); - Assertions.assertThat(spotlessApply2.getOutput()).contains("BUILD SUCCESSFUL"); + assertThat(spotlessApply2.getOutput()).contains("BUILD SUCCESSFUL"); } @ParameterizedTest(name = "{index}: autodetectNpmrcFileConfig with prettier {0}") @@ -430,6 +438,51 @@ void pickupNpmrcFileConfig(String prettierVersion) throws IOException { "}"); setFile("test.ts").toResource("npm/prettier/config/typescript.dirty"); final BuildResult spotlessApply = gradleRunner().withArguments("--stacktrace", "spotlessApply").buildAndFail(); - Assertions.assertThat(spotlessApply.getOutput()).containsPattern("Running npm command.*npm install.* failed with exit code: 1"); + assertThat(spotlessApply.getOutput()).containsPattern("Running npm command.*npm install.* failed with exit code: 1"); + } + + @Test + void multiplePrettierSetupsDoNotIntersectOnNpmDir() throws IOException { + setFile("build.gradle").toLines( + "plugins {", + " id 'com.diffplug.spotless'", + "}", + "repositories { mavenCentral() }", + "def prettierConfig = [:]", + "prettierConfig['printWidth'] = 120", + "spotless {", + " format 'mytypescript', {", + " target 'test.ts'", + " prettier().config(prettierConfig)", + " }", + " format 'json', {", + " target 'test.json'", + " prettier().config(prettierConfig)", + " }", + " javascript {", + " target 'test.js'", + " prettier().config(prettierConfig)", + " }", + "}"); + + setFile("test.ts").toResource("npm/prettier/config/typescript.dirty"); + setFile("test.json").toResource("npm/prettier/filetypes/json/json.dirty"); + setFile("test.js").toResource("npm/prettier/filetypes/javascript-es5/javascript-es5.dirty"); + + final BuildResult spotlessApply = gradleRunner().withArguments("--stacktrace", "spotlessApply").build(); + assertThat(spotlessApply.getOutput()).contains("BUILD SUCCESSFUL"); + + File buildFolder = new File(rootFolder(), "build"); + assertThat(buildFolder).isNotEmptyDirectory(); + + // verify it contains 3 folders containing "spotless-prettier" in it (recursively) - one for each format + try (Stream pathStream = Files.walk(buildFolder.toPath())) { + List nodeModulesDirs = pathStream + .sorted() + .filter(Files::isDirectory) + .filter(path -> path.getFileName().toString().contains("spotless-prettier")) + .collect(Collectors.toList()); + assertThat(nodeModulesDirs).hasSize(3); + } } } 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 456c31b9a3..541d7385eb 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 @@ -1,5 +1,5 @@ /* - * Copyright 2016-2024 DiffPlug + * Copyright 2016-2025 DiffPlug * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -83,12 +83,12 @@ public final Set excludes() { return excludes == null ? emptySet() : Sets.newHashSet(excludes); } - public final Formatter newFormatter(Supplier> filesToFormat, FormatterConfig config) { + public final Formatter newFormatter(Supplier> filesToFormat, FormatterConfig config, int formatterIndex) { Charset formatterEncoding = encoding(config); LineEnding formatterLineEndings = lineEndings(config); LineEnding.Policy formatterLineEndingPolicy = formatterLineEndings.createPolicy(config.getFileLocator().getBaseDir(), filesToFormat); - FormatterStepConfig stepConfig = stepConfig(formatterEncoding, config); + FormatterStepConfig stepConfig = stepConfig(formatterEncoding, config, formatterIndex); List factories = gatherStepFactories(config.getGlobalStepFactories(), stepFactories); List formatterSteps = factories.stream() @@ -174,8 +174,8 @@ Optional ratchetFrom(FormatterConfig config) { } } - private FormatterStepConfig stepConfig(Charset encoding, FormatterConfig config) { - return new FormatterStepConfig(encoding, licenseHeaderDelimiter(), ratchetFrom(config), config.getProvisioner(), config.getFileLocator(), config.getSpotlessSetLicenseHeaderYearsFromGitHistory()); + private FormatterStepConfig stepConfig(Charset encoding, FormatterConfig config, int formatterIndex) { + return new FormatterStepConfig(encoding, licenseHeaderDelimiter(), ratchetFrom(config), config.getProvisioner(), config.getFileLocator(), config.getSpotlessSetLicenseHeaderYearsFromGitHistory(), String.format("%s-%d", "formatter", formatterIndex)); } private static List gatherStepFactories(List allGlobal, List allConfigured) { diff --git a/plugin-maven/src/main/java/com/diffplug/spotless/maven/FormatterStepConfig.java b/plugin-maven/src/main/java/com/diffplug/spotless/maven/FormatterStepConfig.java index a1f2e52b41..ecaad55446 100644 --- a/plugin-maven/src/main/java/com/diffplug/spotless/maven/FormatterStepConfig.java +++ b/plugin-maven/src/main/java/com/diffplug/spotless/maven/FormatterStepConfig.java @@ -1,5 +1,5 @@ /* - * Copyright 2016-2020 DiffPlug + * Copyright 2016-2025 DiffPlug * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -28,14 +28,16 @@ public class FormatterStepConfig { private final Provisioner provisioner; private final FileLocator fileLocator; private final Optional spotlessSetLicenseHeaderYearsFromGitHistory; + private final String name; - public FormatterStepConfig(Charset encoding, String licenseHeaderDelimiter, Optional ratchetFrom, Provisioner provisioner, FileLocator fileLocator, Optional spotlessSetLicenseHeaderYearsFromGitHistory) { + public FormatterStepConfig(Charset encoding, String licenseHeaderDelimiter, Optional ratchetFrom, Provisioner provisioner, FileLocator fileLocator, Optional spotlessSetLicenseHeaderYearsFromGitHistory, String name) { this.encoding = encoding; this.licenseHeaderDelimiter = licenseHeaderDelimiter; this.ratchetFrom = ratchetFrom; this.provisioner = provisioner; this.fileLocator = fileLocator; this.spotlessSetLicenseHeaderYearsFromGitHistory = spotlessSetLicenseHeaderYearsFromGitHistory; + this.name = name; } public Charset getEncoding() { @@ -61,4 +63,8 @@ public FileLocator getFileLocator() { public Optional spotlessSetLicenseHeaderYearsFromGitHistory() { return spotlessSetLicenseHeaderYearsFromGitHistory; } + + public String getName() { + return name; + } } 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 9b279c8845..dea60739db 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 @@ -1,5 +1,5 @@ /* - * Copyright 2021-2024 DiffPlug + * Copyright 2021-2025 DiffPlug * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -16,11 +16,14 @@ package com.diffplug.spotless.maven; import java.io.File; +import java.util.ArrayList; import java.util.Collection; import java.util.LinkedHashMap; +import java.util.List; import java.util.Map; import java.util.Map.Entry; import java.util.function.Supplier; +import java.util.stream.Collectors; import com.diffplug.spotless.Formatter; @@ -40,12 +43,16 @@ public String nameFor(FormatterFactory factory) { static FormattersHolder create(Map>> formatterFactoryToFiles, FormatterConfig config) { Map openFormatters = new LinkedHashMap<>(); try { - for (Entry>> entry : formatterFactoryToFiles.entrySet()) { + List>>> formatterEntries = new ArrayList<>(formatterFactoryToFiles.entrySet()); + for (int formatterIndex = 0; formatterIndex < formatterEntries.size(); formatterIndex++) { + Entry>> entry = formatterEntries.get(formatterIndex); FormatterFactory formatterFactory = entry.getKey(); Supplier> files = entry.getValue(); - Formatter formatter = formatterFactory.newFormatter(files, config); + Formatter formatter = formatterFactory.newFormatter(files, config, formatterIndex); openFormatters.put(formatterFactory, formatter); } + + System.out.println("Created formatters: " + openFormatters.keySet().stream().map(f -> f.includes()).collect(Collectors.toList())); } catch (RuntimeException openError) { try { close(openFormatters.values()); diff --git a/plugin-maven/src/main/java/com/diffplug/spotless/maven/generic/Prettier.java b/plugin-maven/src/main/java/com/diffplug/spotless/maven/generic/Prettier.java index c6b3e46e3c..f747f8764a 100644 --- a/plugin-maven/src/main/java/com/diffplug/spotless/maven/generic/Prettier.java +++ b/plugin-maven/src/main/java/com/diffplug/spotless/maven/generic/Prettier.java @@ -1,5 +1,5 @@ /* - * Copyright 2016-2023 DiffPlug + * Copyright 2016-2025 DiffPlug * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -103,7 +103,7 @@ public FormatterStep newFormatterStep(FormatterStepConfig stepConfig) { File cacheDir = cacheDir(stepConfig); PrettierConfig prettierConfig = new PrettierConfig(configFileHandler, configInline); NpmPathResolver npmPathResolver = npmPathResolver(stepConfig); - return PrettierFormatterStep.create(devDependencies, stepConfig.getProvisioner(), baseDir, buildDir, cacheDir, npmPathResolver, prettierConfig); + return PrettierFormatterStep.create(stepConfig.getName(), devDependencies, stepConfig.getProvisioner(), baseDir, buildDir, cacheDir, npmPathResolver, prettierConfig); } private static IllegalArgumentException onlyOneConfig() { diff --git a/plugin-maven/src/main/java/com/diffplug/spotless/maven/javascript/AbstractEslint.java b/plugin-maven/src/main/java/com/diffplug/spotless/maven/javascript/AbstractEslint.java index ad113de833..2b85cdcebc 100644 --- a/plugin-maven/src/main/java/com/diffplug/spotless/maven/javascript/AbstractEslint.java +++ b/plugin-maven/src/main/java/com/diffplug/spotless/maven/javascript/AbstractEslint.java @@ -1,5 +1,5 @@ /* - * Copyright 2016-2023 DiffPlug + * Copyright 2016-2025 DiffPlug * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -69,7 +69,7 @@ public FormatterStep newFormatterStep(FormatterStepConfig stepConfig) { File baseDir = baseDir(stepConfig); File cacheDir = cacheDir(stepConfig); NpmPathResolver npmPathResolver = npmPathResolver(stepConfig); - return EslintFormatterStep.create(devDependencies, stepConfig.getProvisioner(), baseDir, buildDir, cacheDir, npmPathResolver, eslintConfig(stepConfig)); + return EslintFormatterStep.create(stepConfig.getName(), devDependencies, stepConfig.getProvisioner(), baseDir, buildDir, cacheDir, npmPathResolver, eslintConfig(stepConfig)); } private static IllegalArgumentException onlyOneConfig() { diff --git a/plugin-maven/src/test/java/com/diffplug/spotless/maven/prettier/PrettierFormatStepTest.java b/plugin-maven/src/test/java/com/diffplug/spotless/maven/prettier/PrettierFormatStepTest.java index abba35e72c..f5f5bddccd 100644 --- a/plugin-maven/src/test/java/com/diffplug/spotless/maven/prettier/PrettierFormatStepTest.java +++ b/plugin-maven/src/test/java/com/diffplug/spotless/maven/prettier/PrettierFormatStepTest.java @@ -1,5 +1,5 @@ /* - * Copyright 2016-2023 DiffPlug + * Copyright 2016-2025 DiffPlug * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -17,7 +17,13 @@ import static org.assertj.core.api.Assertions.assertThat; +import java.io.File; import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.List; +import java.util.stream.Collectors; +import java.util.stream.Stream; import org.junit.jupiter.api.Test; @@ -158,6 +164,34 @@ void multiple_prettier_configs() throws Exception { } + /** + * This test is to ensure that we can have multiple equivalent prettier instances in one spotless config. + */ + @Test + void multiple_equivalent_prettier_configs_do_not_intersect() throws Exception { + writePom( + formats( + groupWithSteps("myjson", including("test.json"), + "", + ""), + groupWithSteps("myts", including("test.ts"), + "", + ""))); + + setFile("test.ts").toResource("npm/prettier/config/typescript.dirty"); + setFile("test.json").toResource("npm/prettier/filetypes/json/json.dirty"); + mavenRunner().withArguments("spotless:apply").runNoError(); + + // check that each prettier instance has its own node_modules directory + File targetDir = new File(rootFolder(), "target"); + try (Stream files = Files.walk(targetDir.toPath())) { + List prettierFolders = files.filter(Files::isDirectory) + .filter(path -> path.getFileName().toString().contains("spotless-prettier")) + .collect(Collectors.toList()); + assertThat(prettierFolders).hasSize(2); // one for each prettier instance + } + } + @Test void custom_plugin() throws Exception { writePomWithFormatSteps( diff --git a/testlib/src/test/java/com/diffplug/spotless/npm/EslintFormatterStepTest.java b/testlib/src/test/java/com/diffplug/spotless/npm/EslintFormatterStepTest.java index da00e37d35..6b8debedfb 100644 --- a/testlib/src/test/java/com/diffplug/spotless/npm/EslintFormatterStepTest.java +++ b/testlib/src/test/java/com/diffplug/spotless/npm/EslintFormatterStepTest.java @@ -1,5 +1,5 @@ /* - * Copyright 2016-2024 DiffPlug + * Copyright 2016-2025 DiffPlug * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -58,6 +58,7 @@ void formattingUsingRulesetsFile(String ruleSetName) throws Exception { final String cleanFile = filedir + "javascript-es6.clean"; final FormatterStep formatterStep = EslintFormatterStep.create( + "ESLINT_TEST", devDependenciesForRuleset.get(ruleSetName), TestProvisioner.mavenCentral(), projectDir(), @@ -100,6 +101,7 @@ void formattingUsingRulesetsFile(String ruleSetName) throws Exception { final String cleanFile = filedir + "typescript.clean"; final FormatterStep formatterStep = EslintFormatterStep.create( + "ESLINT_TEST", devDependenciesForRuleset.get(ruleSetName), TestProvisioner.mavenCentral(), projectDir(), @@ -157,6 +159,7 @@ void formattingUsingInlineXoConfig() throws Exception { final String cleanFile = filedir + "typescript.clean"; final FormatterStep formatterStep = EslintFormatterStep.create( + "ESLINT_TEST", EslintStyleGuide.TS_XO_TYPESCRIPT.mergedWith(EslintFormatterStep.defaultDevDependenciesForTypescript()), TestProvisioner.mavenCentral(), projectDir(), diff --git a/testlib/src/test/java/com/diffplug/spotless/npm/PrettierFormatterStepTest.java b/testlib/src/test/java/com/diffplug/spotless/npm/PrettierFormatterStepTest.java index 54ce09e7e8..915d757d33 100644 --- a/testlib/src/test/java/com/diffplug/spotless/npm/PrettierFormatterStepTest.java +++ b/testlib/src/test/java/com/diffplug/spotless/npm/PrettierFormatterStepTest.java @@ -1,5 +1,5 @@ /* - * Copyright 2016-2024 DiffPlug + * Copyright 2016-2025 DiffPlug * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -63,6 +63,7 @@ private void runTestUsingPrettier(String fileType, Map dependenc final String cleanFile = filedir + fileType + ".clean"; final FormatterStep formatterStep = PrettierFormatterStep.create( + "PRETTIER_TEST", dependencies, TestProvisioner.mavenCentral(), projectDir(), @@ -90,6 +91,7 @@ void parserInferenceBasedOnExplicitFilepathIsWorking(String prettierVersion) thr final String cleanFile = filedir + "json.clean"; final FormatterStep formatterStep = PrettierFormatterStep.create( + "PRETTIER_TEST", ImmutableMap.of("prettier", prettierVersion), TestProvisioner.mavenCentral(), projectDir(), @@ -112,6 +114,7 @@ void parserInferenceBasedOnFilenameIsWorking(String prettierVersion) throws Exce final String cleanFile = filedir + "clean.json"; final FormatterStep formatterStep = PrettierFormatterStep.create( + "PRETTIER_TEST", ImmutableMap.of("prettier", prettierVersion), TestProvisioner.mavenCentral(), projectDir(), @@ -128,6 +131,7 @@ void parserInferenceBasedOnFilenameIsWorking(String prettierVersion) throws Exce @Test void verifyPrettierErrorMessageIsRelayed() throws Exception { FormatterStep formatterStep = PrettierFormatterStep.create( + "PRETTIER_TEST", PrettierFormatterStep.defaultDevDependenciesWithPrettier("2.8.8"), TestProvisioner.mavenCentral(), projectDir(), @@ -137,7 +141,7 @@ void verifyPrettierErrorMessageIsRelayed() throws Exception { new PrettierConfig(null, ImmutableMap.of("parser", "postcss"))); try (StepHarnessWithFile stepHarness = StepHarnessWithFile.forStep(this, formatterStep)) { stepHarness.expectLintsOfResource("npm/prettier/filetypes/scss/scss.dirty") - .toBe("LINE_UNDEFINED prettier-format(com.diffplug.spotless.npm.SimpleRestClient$SimpleRestResponseException) Unexpected response status code at /prettier/format [HTTP 500] -- (Error while formatting: Error: Couldn't resolve parser \"postcss\") (...)"); + .toBe("LINE_UNDEFINED PRETTIER_TEST-prettier-format(com.diffplug.spotless.npm.SimpleRestClient$SimpleRestResponseException) Unexpected response status code at /prettier/format [HTTP 500] -- (Error while formatting: Error: Couldn't resolve parser \"postcss\") (...)"); } } } @@ -154,6 +158,7 @@ void runFormatTest(String prettierVersion, PrettierConfig config, String cleanFi final String cleanFile = FILEDIR + "typescript." + cleanFileNameSuffix + ".clean"; final FormatterStep formatterStep = PrettierFormatterStep.create( + "PRETTIER_TEST", ImmutableMap.of("prettier", prettierVersion), TestProvisioner.mavenCentral(), projectDir(), @@ -210,6 +215,7 @@ protected void setupTest(API api) { @Override protected FormatterStep create() { return PrettierFormatterStep.create( + "PRETTIER_TEST", ImmutableMap.of("prettier", prettierVersion), TestProvisioner.mavenCentral(), projectDir(), From 4589ca9dcd1d56792296dd6794dd76db59fcd735 Mon Sep 17 00:00:00 2001 From: Simon Gamma Date: Mon, 7 Apr 2025 20:58:51 +0200 Subject: [PATCH 02/11] docs: add node_modules separation to CHANGELOG --- CHANGES.md | 1 + plugin-gradle/CHANGES.md | 1 + plugin-maven/CHANGES.md | 1 + 3 files changed, 3 insertions(+) diff --git a/CHANGES.md b/CHANGES.md index 7d16300cce..8e2b6ac202 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -13,6 +13,7 @@ We adhere to the [keepachangelog](https://keepachangelog.com/en/1.0.0/) format ( ### Changed * Use palantir-java-format 2.57.0 on Java 21. ([#2447](https://github.com/diffplug/spotless/pull/2447)) * Re-try `npm install` with `--prefer-online` after `ERESOLVE` error. ([#2448](https://github.com/diffplug/spotless/pull/2448)) +* **BREAKING** Assert that each npm-based step uses its own `node_modules` directory, even when configured exactly the same, to prevent race conditions when running in parallel. For this, the API of npm-based formatter steps has been changed. ([#TODO](https://github.com/diffplug/spotless/pull/TODO)) ## [3.1.0] - 2025-02-20 ### Added diff --git a/plugin-gradle/CHANGES.md b/plugin-gradle/CHANGES.md index 5d2cb19742..733d5cf3dc 100644 --- a/plugin-gradle/CHANGES.md +++ b/plugin-gradle/CHANGES.md @@ -7,6 +7,7 @@ We adhere to the [keepachangelog](https://keepachangelog.com/en/1.0.0/) format ( * Use palantir-java-format 2.57.0 on Java 21. ([#2447](https://github.com/diffplug/spotless/pull/2447)) * Re-try `npm install` with `--prefer-online` after `ERESOLVE` error. ([#2448](https://github.com/diffplug/spotless/pull/2448)) * Apply Gradle's strict plugin types validation to the Spotless plugin. ([#2454](https://github.com/diffplug/spotless/pull/2454)) +* Assert that each npm-based step uses its own `node_modules` directory, even when configured exactly the same to prevent race conditions when running in parallel. ([#TODO](https://github.com/diffplug/spotless/pull/TODO)) ## [7.0.2] - 2025-01-14 ### Fixed diff --git a/plugin-maven/CHANGES.md b/plugin-maven/CHANGES.md index 483258c36a..f46eebf703 100644 --- a/plugin-maven/CHANGES.md +++ b/plugin-maven/CHANGES.md @@ -6,6 +6,7 @@ We adhere to the [keepachangelog](https://keepachangelog.com/en/1.0.0/) format ( ### Changed * Use palantir-java-format 2.57.0 on Java 21. ([#2447](https://github.com/diffplug/spotless/pull/2447)) * Re-try `npm install` with `--prefer-online` after `ERESOLVE` error. ([#2448](https://github.com/diffplug/spotless/pull/2448)) +* Assert that each npm-based step uses its own `node_modules` directory, even when configured exactly the same to prevent race conditions when running in parallel. ([#TODO](https://github.com/diffplug/spotless/pull/TODO)) ## [2.44.3] - 2025-02-20 * Support for `clang-format` ([#2406](https://github.com/diffplug/spotless/pull/2406)) From 215793d2709c546e76f985fd4987506bb9410006 Mon Sep 17 00:00:00 2001 From: Simon Gamma Date: Mon, 7 Apr 2025 22:33:34 +0200 Subject: [PATCH 03/11] docs: apply correct pr number Refs: #2462 --- CHANGES.md | 2 +- plugin-gradle/CHANGES.md | 2 +- plugin-maven/CHANGES.md | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index 8e2b6ac202..fe1e97c286 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -13,7 +13,7 @@ We adhere to the [keepachangelog](https://keepachangelog.com/en/1.0.0/) format ( ### Changed * Use palantir-java-format 2.57.0 on Java 21. ([#2447](https://github.com/diffplug/spotless/pull/2447)) * Re-try `npm install` with `--prefer-online` after `ERESOLVE` error. ([#2448](https://github.com/diffplug/spotless/pull/2448)) -* **BREAKING** Assert that each npm-based step uses its own `node_modules` directory, even when configured exactly the same, to prevent race conditions when running in parallel. For this, the API of npm-based formatter steps has been changed. ([#TODO](https://github.com/diffplug/spotless/pull/TODO)) +* **BREAKING** Assert that each npm-based step uses its own `node_modules` directory, even when configured exactly the same, to prevent race conditions when running in parallel. For this, the API of npm-based formatter steps has been changed. ([#2462](https://github.com/diffplug/spotless/pull/2462)) ## [3.1.0] - 2025-02-20 ### Added diff --git a/plugin-gradle/CHANGES.md b/plugin-gradle/CHANGES.md index 733d5cf3dc..c402a1b09b 100644 --- a/plugin-gradle/CHANGES.md +++ b/plugin-gradle/CHANGES.md @@ -7,7 +7,7 @@ We adhere to the [keepachangelog](https://keepachangelog.com/en/1.0.0/) format ( * Use palantir-java-format 2.57.0 on Java 21. ([#2447](https://github.com/diffplug/spotless/pull/2447)) * Re-try `npm install` with `--prefer-online` after `ERESOLVE` error. ([#2448](https://github.com/diffplug/spotless/pull/2448)) * Apply Gradle's strict plugin types validation to the Spotless plugin. ([#2454](https://github.com/diffplug/spotless/pull/2454)) -* Assert that each npm-based step uses its own `node_modules` directory, even when configured exactly the same to prevent race conditions when running in parallel. ([#TODO](https://github.com/diffplug/spotless/pull/TODO)) +* Assert that each npm-based step uses its own `node_modules` directory, even when configured exactly the same to prevent race conditions when running in parallel. ([#2462](https://github.com/diffplug/spotless/pull/2462)) ## [7.0.2] - 2025-01-14 ### Fixed diff --git a/plugin-maven/CHANGES.md b/plugin-maven/CHANGES.md index f46eebf703..f9ea744bae 100644 --- a/plugin-maven/CHANGES.md +++ b/plugin-maven/CHANGES.md @@ -6,7 +6,7 @@ We adhere to the [keepachangelog](https://keepachangelog.com/en/1.0.0/) format ( ### Changed * Use palantir-java-format 2.57.0 on Java 21. ([#2447](https://github.com/diffplug/spotless/pull/2447)) * Re-try `npm install` with `--prefer-online` after `ERESOLVE` error. ([#2448](https://github.com/diffplug/spotless/pull/2448)) -* Assert that each npm-based step uses its own `node_modules` directory, even when configured exactly the same to prevent race conditions when running in parallel. ([#TODO](https://github.com/diffplug/spotless/pull/TODO)) +* Assert that each npm-based step uses its own `node_modules` directory, even when configured exactly the same to prevent race conditions when running in parallel. ([#2462](https://github.com/diffplug/spotless/pull/2462)) ## [2.44.3] - 2025-02-20 * Support for `clang-format` ([#2406](https://github.com/diffplug/spotless/pull/2406)) From fe34e9cd07287486b9d544f7c18e11299e6cef67 Mon Sep 17 00:00:00 2001 From: Simon Gamma Date: Mon, 7 Apr 2025 22:41:40 +0200 Subject: [PATCH 04/11] chore: add input validation for formatName Refs: #2462 --- .../main/java/com/diffplug/spotless/npm/EslintFormatterStep.java | 1 + .../java/com/diffplug/spotless/npm/PrettierFormatterStep.java | 1 + 2 files changed, 2 insertions(+) diff --git a/lib/src/main/java/com/diffplug/spotless/npm/EslintFormatterStep.java b/lib/src/main/java/com/diffplug/spotless/npm/EslintFormatterStep.java index d32f4e58b9..2f0f016476 100644 --- a/lib/src/main/java/com/diffplug/spotless/npm/EslintFormatterStep.java +++ b/lib/src/main/java/com/diffplug/spotless/npm/EslintFormatterStep.java @@ -69,6 +69,7 @@ public static Map defaultDevDependenciesWithEslint(String versio } public static FormatterStep create(String formatName, Map devDependencies, Provisioner provisioner, File projectDir, File buildDir, File cacheDir, NpmPathResolver npmPathResolver, EslintConfig eslintConfig) { + requireNonNull(formatName); requireNonNull(devDependencies); requireNonNull(provisioner); requireNonNull(projectDir); diff --git a/lib/src/main/java/com/diffplug/spotless/npm/PrettierFormatterStep.java b/lib/src/main/java/com/diffplug/spotless/npm/PrettierFormatterStep.java index 0f81934e24..6a79116435 100644 --- a/lib/src/main/java/com/diffplug/spotless/npm/PrettierFormatterStep.java +++ b/lib/src/main/java/com/diffplug/spotless/npm/PrettierFormatterStep.java @@ -52,6 +52,7 @@ public static final Map defaultDevDependenciesWithPrettier(Strin } public static FormatterStep create(String formatName, Map devDependencies, Provisioner provisioner, File projectDir, File buildDir, File cacheDir, NpmPathResolver npmPathResolver, PrettierConfig prettierConfig) { + requireNonNull(formatName); requireNonNull(devDependencies); requireNonNull(provisioner); requireNonNull(buildDir); From 0f500122616c2876245caf3dd9ac1b1e4f3a1784 Mon Sep 17 00:00:00 2001 From: Simon Gamma Date: Tue, 8 Apr 2025 16:09:29 +0200 Subject: [PATCH 05/11] Revert "chore: add input validation for formatName" This reverts commit fe34e9cd07287486b9d544f7c18e11299e6cef67. --- .../main/java/com/diffplug/spotless/npm/EslintFormatterStep.java | 1 - .../java/com/diffplug/spotless/npm/PrettierFormatterStep.java | 1 - 2 files changed, 2 deletions(-) diff --git a/lib/src/main/java/com/diffplug/spotless/npm/EslintFormatterStep.java b/lib/src/main/java/com/diffplug/spotless/npm/EslintFormatterStep.java index 2f0f016476..d32f4e58b9 100644 --- a/lib/src/main/java/com/diffplug/spotless/npm/EslintFormatterStep.java +++ b/lib/src/main/java/com/diffplug/spotless/npm/EslintFormatterStep.java @@ -69,7 +69,6 @@ public static Map defaultDevDependenciesWithEslint(String versio } public static FormatterStep create(String formatName, Map devDependencies, Provisioner provisioner, File projectDir, File buildDir, File cacheDir, NpmPathResolver npmPathResolver, EslintConfig eslintConfig) { - requireNonNull(formatName); requireNonNull(devDependencies); requireNonNull(provisioner); requireNonNull(projectDir); diff --git a/lib/src/main/java/com/diffplug/spotless/npm/PrettierFormatterStep.java b/lib/src/main/java/com/diffplug/spotless/npm/PrettierFormatterStep.java index 6a79116435..0f81934e24 100644 --- a/lib/src/main/java/com/diffplug/spotless/npm/PrettierFormatterStep.java +++ b/lib/src/main/java/com/diffplug/spotless/npm/PrettierFormatterStep.java @@ -52,7 +52,6 @@ public static final Map defaultDevDependenciesWithPrettier(Strin } public static FormatterStep create(String formatName, Map devDependencies, Provisioner provisioner, File projectDir, File buildDir, File cacheDir, NpmPathResolver npmPathResolver, PrettierConfig prettierConfig) { - requireNonNull(formatName); requireNonNull(devDependencies); requireNonNull(provisioner); requireNonNull(buildDir); From e4b8f30136ec29d75bbe3dde5017b752b4e1cc7e Mon Sep 17 00:00:00 2001 From: Simon Gamma Date: Tue, 8 Apr 2025 16:09:33 +0200 Subject: [PATCH 06/11] Revert "fix: separate node_modules dir per npm based formatter" This reverts commit da3b75b4cc8e50960d3dbbf2601d1ba4ab9613f1. --- .../spotless/npm/EslintFormatterStep.java | 9 +- .../npm/NpmFormatterStepStateBase.java | 4 +- .../spotless/npm/PrettierFormatterStep.java | 9 +- .../gradle/spotless/FormatExtension.java | 2 +- .../gradle/spotless/JavascriptExtension.java | 4 +- .../gradle/spotless/TypescriptExtension.java | 4 +- .../spotless/PrettierIntegrationTest.java | 91 ++++--------------- .../spotless/maven/FormatterFactory.java | 10 +- .../spotless/maven/FormatterStepConfig.java | 10 +- .../spotless/maven/FormattersHolder.java | 13 +-- .../spotless/maven/generic/Prettier.java | 4 +- .../maven/javascript/AbstractEslint.java | 4 +- .../prettier/PrettierFormatStepTest.java | 36 +------- .../spotless/npm/EslintFormatterStepTest.java | 5 +- .../npm/PrettierFormatterStepTest.java | 10 +- 15 files changed, 52 insertions(+), 163 deletions(-) diff --git a/lib/src/main/java/com/diffplug/spotless/npm/EslintFormatterStep.java b/lib/src/main/java/com/diffplug/spotless/npm/EslintFormatterStep.java index d32f4e58b9..70beaf91ca 100644 --- a/lib/src/main/java/com/diffplug/spotless/npm/EslintFormatterStep.java +++ b/lib/src/main/java/com/diffplug/spotless/npm/EslintFormatterStep.java @@ -1,5 +1,5 @@ /* - * Copyright 2016-2025 DiffPlug + * Copyright 2016-2024 DiffPlug * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -68,14 +68,13 @@ public static Map defaultDevDependenciesWithEslint(String versio return Collections.singletonMap("eslint", version); } - public static FormatterStep create(String formatName, Map devDependencies, Provisioner provisioner, File projectDir, File buildDir, File cacheDir, NpmPathResolver npmPathResolver, EslintConfig eslintConfig) { + public static FormatterStep create(Map devDependencies, Provisioner provisioner, File projectDir, File buildDir, File cacheDir, NpmPathResolver npmPathResolver, EslintConfig eslintConfig) { requireNonNull(devDependencies); requireNonNull(provisioner); requireNonNull(projectDir); requireNonNull(buildDir); - final String prefixedName = String.format("%s-%s", formatName, NAME); - return FormatterStep.createLazy(prefixedName, - () -> new State(prefixedName, devDependencies, projectDir, buildDir, cacheDir, npmPathResolver, eslintConfig), + return FormatterStep.createLazy(NAME, + () -> new State(NAME, devDependencies, projectDir, buildDir, cacheDir, npmPathResolver, eslintConfig), State::createFormatterFunc); } diff --git a/lib/src/main/java/com/diffplug/spotless/npm/NpmFormatterStepStateBase.java b/lib/src/main/java/com/diffplug/spotless/npm/NpmFormatterStepStateBase.java index dd798e6e55..3ec06e0434 100644 --- a/lib/src/main/java/com/diffplug/spotless/npm/NpmFormatterStepStateBase.java +++ b/lib/src/main/java/com/diffplug/spotless/npm/NpmFormatterStepStateBase.java @@ -1,5 +1,5 @@ /* - * Copyright 2016-2025 DiffPlug + * Copyright 2016-2024 DiffPlug * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -69,7 +69,7 @@ public static class Runtime { Runtime(NpmFormatterStepStateBase parent) { this.parent = parent; - this.nodeServerLayout = new NodeServerLayout(new File(parent.locations.buildDir(), parent.stepName), parent.npmConfig.getPackageJsonContent()); + this.nodeServerLayout = new NodeServerLayout(parent.locations.buildDir(), parent.npmConfig.getPackageJsonContent()); this.nodeServeApp = new NodeServeApp(nodeServerLayout, parent.npmConfig, parent.locations); } diff --git a/lib/src/main/java/com/diffplug/spotless/npm/PrettierFormatterStep.java b/lib/src/main/java/com/diffplug/spotless/npm/PrettierFormatterStep.java index 0f81934e24..27a1002df5 100644 --- a/lib/src/main/java/com/diffplug/spotless/npm/PrettierFormatterStep.java +++ b/lib/src/main/java/com/diffplug/spotless/npm/PrettierFormatterStep.java @@ -1,5 +1,5 @@ /* - * Copyright 2016-2025 DiffPlug + * Copyright 2016-2024 DiffPlug * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -51,13 +51,12 @@ public static final Map defaultDevDependenciesWithPrettier(Strin return Collections.singletonMap("prettier", version); } - public static FormatterStep create(String formatName, Map devDependencies, Provisioner provisioner, File projectDir, File buildDir, File cacheDir, NpmPathResolver npmPathResolver, PrettierConfig prettierConfig) { + public static FormatterStep create(Map devDependencies, Provisioner provisioner, File projectDir, File buildDir, File cacheDir, NpmPathResolver npmPathResolver, PrettierConfig prettierConfig) { requireNonNull(devDependencies); requireNonNull(provisioner); requireNonNull(buildDir); - final String prefixedName = String.format("%s-%s", formatName, NAME); - return FormatterStep.createLazy(prefixedName, - () -> new State(prefixedName, devDependencies, projectDir, buildDir, cacheDir, npmPathResolver, prettierConfig), + return FormatterStep.createLazy(NAME, + () -> new State(NAME, devDependencies, projectDir, buildDir, cacheDir, npmPathResolver, prettierConfig), State::createFormatterFunc); } diff --git a/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/FormatExtension.java b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/FormatExtension.java index 712e6fe00a..69150724c1 100644 --- a/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/FormatExtension.java +++ b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/FormatExtension.java @@ -798,7 +798,7 @@ public PrettierConfig config(final Map prettierConfig) { @Override protected FormatterStep createStep() { final Project project = getProject(); - return PrettierFormatterStep.create(formatName(), devDependencies, provisioner(), project.getProjectDir(), + return PrettierFormatterStep.create(devDependencies, provisioner(), project.getProjectDir(), project.getLayout().getBuildDirectory().getAsFile().get(), npmModulesCacheOrNull(), new NpmPathResolver(npmFileOrNull(), nodeFileOrNull(), npmrcFileOrNull(), Arrays.asList(project.getProjectDir(), project.getRootDir())), diff --git a/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/JavascriptExtension.java b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/JavascriptExtension.java index 7968f86e28..5532045bc2 100644 --- a/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/JavascriptExtension.java +++ b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/JavascriptExtension.java @@ -1,5 +1,5 @@ /* - * Copyright 2016-2025 DiffPlug + * Copyright 2016-2024 DiffPlug * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -106,7 +106,7 @@ public JavascriptEslintConfig(Map devDependencies) { public FormatterStep createStep() { final Project project = getProject(); - return EslintFormatterStep.create(NAME, devDependencies, provisioner(), project.getProjectDir(), + return EslintFormatterStep.create(devDependencies, provisioner(), project.getProjectDir(), project.getLayout().getBuildDirectory().getAsFile().get(), npmModulesCacheOrNull(), new NpmPathResolver(npmFileOrNull(), nodeFileOrNull(), npmrcFileOrNull(), Arrays.asList(project.getProjectDir(), project.getRootDir())), diff --git a/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/TypescriptExtension.java b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/TypescriptExtension.java index 9794a3c349..39e050d51e 100644 --- a/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/TypescriptExtension.java +++ b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/TypescriptExtension.java @@ -1,5 +1,5 @@ /* - * Copyright 2016-2025 DiffPlug + * Copyright 2016-2024 DiffPlug * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -214,7 +214,7 @@ public TypescriptEslintConfig tsconfigFile(Object path) { public FormatterStep createStep() { final Project project = getProject(); - return EslintFormatterStep.create(NAME, devDependencies, provisioner(), project.getProjectDir(), + return EslintFormatterStep.create(devDependencies, provisioner(), project.getProjectDir(), project.getLayout().getBuildDirectory().getAsFile().get(), npmModulesCacheOrNull(), new NpmPathResolver(npmFileOrNull(), nodeFileOrNull(), npmrcFileOrNull(), Arrays.asList(project.getProjectDir(), project.getRootDir())), diff --git a/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/PrettierIntegrationTest.java b/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/PrettierIntegrationTest.java index 6c85949861..b6547204b8 100644 --- a/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/PrettierIntegrationTest.java +++ b/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/PrettierIntegrationTest.java @@ -1,5 +1,5 @@ /* - * Copyright 2016-2025 DiffPlug + * Copyright 2016-2024 DiffPlug * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -15,18 +15,10 @@ */ package com.diffplug.gradle.spotless; -import static org.assertj.core.api.Assertions.assertThat; - -import java.io.File; import java.io.IOException; -import java.nio.file.Files; -import java.nio.file.Path; -import java.util.List; -import java.util.stream.Collectors; -import java.util.stream.Stream; +import org.assertj.core.api.Assertions; import org.gradle.testkit.runner.BuildResult; -import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.ValueSource; @@ -59,7 +51,7 @@ void useInlineConfig(String prettierVersion) throws IOException { "}"); setFile("test.ts").toResource("npm/prettier/config/typescript.dirty"); final BuildResult spotlessApply = gradleRunner().withArguments("--stacktrace", "spotlessApply").build(); - assertThat(spotlessApply.getOutput()).contains("BUILD SUCCESSFUL"); + Assertions.assertThat(spotlessApply.getOutput()).contains("BUILD SUCCESSFUL"); switch (prettierVersion) { case PRETTIER_VERSION_2: assertFile("test.ts").sameAsResource("npm/prettier/config/typescript.configfile_prettier_2.clean"); @@ -89,7 +81,7 @@ void verifyCleanSpotlessCheckWorks(String prettierVersion) throws IOException { "}"); setFile("test.ts").toResource("npm/prettier/config/typescript.dirty"); final BuildResult spotlessCheckFailsGracefully = gradleRunner().withArguments("--stacktrace", "spotlessCheck").buildAndFail(); - assertThat(spotlessCheckFailsGracefully.getOutput()).contains("> The following files had format violations:"); + Assertions.assertThat(spotlessCheckFailsGracefully.getOutput()).contains("> The following files had format violations:"); gradleRunner().withArguments("--stacktrace", "spotlessApply").build(); gradleRunner().withArguments("--stacktrace", "spotlessCheck").build(); @@ -112,7 +104,7 @@ void useFileConfig(String prettierVersion) throws IOException { "}"); setFile("test.ts").toResource("npm/prettier/config/typescript.dirty"); final BuildResult spotlessApply = gradleRunner().withArguments("--stacktrace", "spotlessApply").build(); - assertThat(spotlessApply.getOutput()).contains("BUILD SUCCESSFUL"); + Assertions.assertThat(spotlessApply.getOutput()).contains("BUILD SUCCESSFUL"); switch (prettierVersion) { case PRETTIER_VERSION_2: assertFile("test.ts").sameAsResource("npm/prettier/config/typescript.configfile_prettier_2.clean"); @@ -139,7 +131,7 @@ void chooseParserBasedOnFilename(String prettierVersion) throws IOException { "}"); setFile("dirty.json").toResource("npm/prettier/filename/dirty.json"); final BuildResult spotlessApply = gradleRunner().withArguments("--stacktrace", "spotlessApply").build(); - assertThat(spotlessApply.getOutput()).contains("BUILD SUCCESSFUL"); + Assertions.assertThat(spotlessApply.getOutput()).contains("BUILD SUCCESSFUL"); assertFile("dirty.json").sameAsResource("npm/prettier/filename/clean.json"); } @@ -177,7 +169,7 @@ void useJavaCommunityPlugin(String prettierVersion) throws IOException { "}"); setFile("JavaTest.java").toResource("npm/prettier/plugins/java-test.dirty"); final BuildResult spotlessApply = gradleRunner().withArguments("--stacktrace", "spotlessApply").build(); - assertThat(spotlessApply.getOutput()).contains("BUILD SUCCESSFUL"); + Assertions.assertThat(spotlessApply.getOutput()).contains("BUILD SUCCESSFUL"); assertFile("JavaTest.java").sameAsResource("npm/prettier/plugins/java-test.clean"); } @@ -210,7 +202,7 @@ void useJavaCommunityPluginFileConfig(String prettierVersion) throws IOException "}"); setFile("JavaTest.java").toResource("npm/prettier/plugins/java-test.dirty"); final BuildResult spotlessApply = gradleRunner().withArguments("--stacktrace", "spotlessApply").build(); - assertThat(spotlessApply.getOutput()).contains("BUILD SUCCESSFUL"); + Assertions.assertThat(spotlessApply.getOutput()).contains("BUILD SUCCESSFUL"); assertFile("JavaTest.java").sameAsResource("npm/prettier/plugins/java-test.clean"); } @@ -234,8 +226,8 @@ void suggestsMissingJavaCommunityPlugin(String prettierVersion) throws IOExcepti "}"); setFile("JavaTest.java").toResource("npm/prettier/plugins/java-test.dirty"); final BuildResult spotlessApply = gradleRunner().withArguments("--stacktrace", "spotlessApply").buildAndFail(); - assertThat(spotlessApply.getOutput()).contains("Could not infer a parser"); - assertThat(spotlessApply.getOutput()).contains("prettier-plugin-java"); + Assertions.assertThat(spotlessApply.getOutput()).contains("Could not infer a parser"); + Assertions.assertThat(spotlessApply.getOutput()).contains("prettier-plugin-java"); } @ParameterizedTest(name = "{index}: usePhpCommunityPlugin with prettier {0}") @@ -272,7 +264,7 @@ void usePhpCommunityPlugin(String prettierVersion) throws IOException { "}"); setFile("php-example.php").toResource("npm/prettier/plugins/php.dirty"); final BuildResult spotlessApply = gradleRunner().withArguments("--stacktrace", "spotlessApply").build(); - assertThat(spotlessApply.getOutput()).contains("BUILD SUCCESSFUL"); + Assertions.assertThat(spotlessApply.getOutput()).contains("BUILD SUCCESSFUL"); assertFile("php-example.php").sameAsResource("npm/prettier/plugins/php.clean"); } @@ -332,9 +324,9 @@ void usePhpAndJavaCommunityPlugin(String prettierVersion) throws IOException { setFile("php-example.php").toResource("npm/prettier/plugins/php.dirty"); setFile("JavaTest.java").toResource("npm/prettier/plugins/java-test.dirty"); final BuildResult spotlessApply = gradleRunner().forwardOutput().withArguments("--stacktrace", "--info", "spotlessApply").build(); - assertThat(spotlessApply.getOutput()).contains("BUILD SUCCESSFUL"); + Assertions.assertThat(spotlessApply.getOutput()).contains("BUILD SUCCESSFUL"); final BuildResult spotlessApply2 = gradleRunner().forwardOutput().withArguments("--stacktrace", "--info", "spotlessApply").build(); - assertThat(spotlessApply2.getOutput()).contains("BUILD SUCCESSFUL"); + Assertions.assertThat(spotlessApply2.getOutput()).contains("BUILD SUCCESSFUL"); assertFile("php-example.php").sameAsResource("npm/prettier/plugins/php.clean"); assertFile("JavaTest.java").sameAsResource("npm/prettier/plugins/java-test.clean"); } @@ -363,7 +355,7 @@ void autodetectNpmrcFileConfig(String prettierVersion) throws IOException { "}"); setFile("test.ts").toResource("npm/prettier/config/typescript.dirty"); final BuildResult spotlessApply = gradleRunner().withArguments("--stacktrace", "spotlessApply").buildAndFail(); - assertThat(spotlessApply.getOutput()).containsPattern("Running npm command.*npm install.* failed with exit code: 1"); + Assertions.assertThat(spotlessApply.getOutput()).containsPattern("Running npm command.*npm install.* failed with exit code: 1"); } @ParameterizedTest(name = "{index}: verifyCleanAndSpotlessWorks with prettier {0}") @@ -385,9 +377,9 @@ void verifyCleanAndSpotlessWorks(String prettierVersion) throws IOException { "}"); setFile("test.ts").toResource("npm/prettier/config/typescript.dirty"); final BuildResult spotlessApply = gradleRunner().withArguments("--stacktrace", "clean", "spotlessApply").build(); - assertThat(spotlessApply.getOutput()).contains("BUILD SUCCESSFUL"); + Assertions.assertThat(spotlessApply.getOutput()).contains("BUILD SUCCESSFUL"); final BuildResult spotlessApply2 = gradleRunner().withArguments("--stacktrace", "clean", "spotlessApply").build(); - assertThat(spotlessApply2.getOutput()).contains("BUILD SUCCESSFUL"); + Assertions.assertThat(spotlessApply2.getOutput()).contains("BUILD SUCCESSFUL"); } @ParameterizedTest(name = "{index}: verifyCleanAndSpotlessWithNpmInstallCacheWorks with prettier {0}") @@ -409,9 +401,9 @@ void verifyCleanAndSpotlessWithNpmInstallCacheWorks(String prettierVersion) thro "}"); setFile("test.ts").toResource("npm/prettier/config/typescript.dirty"); final BuildResult spotlessApply = gradleRunner().withArguments("--stacktrace", "clean", "spotlessApply").build(); - assertThat(spotlessApply.getOutput()).contains("BUILD SUCCESSFUL"); + Assertions.assertThat(spotlessApply.getOutput()).contains("BUILD SUCCESSFUL"); final BuildResult spotlessApply2 = gradleRunner().withArguments("--stacktrace", "clean", "spotlessApply").build(); - assertThat(spotlessApply2.getOutput()).contains("BUILD SUCCESSFUL"); + Assertions.assertThat(spotlessApply2.getOutput()).contains("BUILD SUCCESSFUL"); } @ParameterizedTest(name = "{index}: autodetectNpmrcFileConfig with prettier {0}") @@ -438,51 +430,6 @@ void pickupNpmrcFileConfig(String prettierVersion) throws IOException { "}"); setFile("test.ts").toResource("npm/prettier/config/typescript.dirty"); final BuildResult spotlessApply = gradleRunner().withArguments("--stacktrace", "spotlessApply").buildAndFail(); - assertThat(spotlessApply.getOutput()).containsPattern("Running npm command.*npm install.* failed with exit code: 1"); - } - - @Test - void multiplePrettierSetupsDoNotIntersectOnNpmDir() throws IOException { - setFile("build.gradle").toLines( - "plugins {", - " id 'com.diffplug.spotless'", - "}", - "repositories { mavenCentral() }", - "def prettierConfig = [:]", - "prettierConfig['printWidth'] = 120", - "spotless {", - " format 'mytypescript', {", - " target 'test.ts'", - " prettier().config(prettierConfig)", - " }", - " format 'json', {", - " target 'test.json'", - " prettier().config(prettierConfig)", - " }", - " javascript {", - " target 'test.js'", - " prettier().config(prettierConfig)", - " }", - "}"); - - setFile("test.ts").toResource("npm/prettier/config/typescript.dirty"); - setFile("test.json").toResource("npm/prettier/filetypes/json/json.dirty"); - setFile("test.js").toResource("npm/prettier/filetypes/javascript-es5/javascript-es5.dirty"); - - final BuildResult spotlessApply = gradleRunner().withArguments("--stacktrace", "spotlessApply").build(); - assertThat(spotlessApply.getOutput()).contains("BUILD SUCCESSFUL"); - - File buildFolder = new File(rootFolder(), "build"); - assertThat(buildFolder).isNotEmptyDirectory(); - - // verify it contains 3 folders containing "spotless-prettier" in it (recursively) - one for each format - try (Stream pathStream = Files.walk(buildFolder.toPath())) { - List nodeModulesDirs = pathStream - .sorted() - .filter(Files::isDirectory) - .filter(path -> path.getFileName().toString().contains("spotless-prettier")) - .collect(Collectors.toList()); - assertThat(nodeModulesDirs).hasSize(3); - } + Assertions.assertThat(spotlessApply.getOutput()).containsPattern("Running npm command.*npm install.* failed with exit code: 1"); } } 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 541d7385eb..456c31b9a3 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 @@ -1,5 +1,5 @@ /* - * Copyright 2016-2025 DiffPlug + * Copyright 2016-2024 DiffPlug * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -83,12 +83,12 @@ public final Set excludes() { return excludes == null ? emptySet() : Sets.newHashSet(excludes); } - public final Formatter newFormatter(Supplier> filesToFormat, FormatterConfig config, int formatterIndex) { + 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); - FormatterStepConfig stepConfig = stepConfig(formatterEncoding, config, formatterIndex); + FormatterStepConfig stepConfig = stepConfig(formatterEncoding, config); List factories = gatherStepFactories(config.getGlobalStepFactories(), stepFactories); List formatterSteps = factories.stream() @@ -174,8 +174,8 @@ Optional ratchetFrom(FormatterConfig config) { } } - private FormatterStepConfig stepConfig(Charset encoding, FormatterConfig config, int formatterIndex) { - return new FormatterStepConfig(encoding, licenseHeaderDelimiter(), ratchetFrom(config), config.getProvisioner(), config.getFileLocator(), config.getSpotlessSetLicenseHeaderYearsFromGitHistory(), String.format("%s-%d", "formatter", formatterIndex)); + private FormatterStepConfig stepConfig(Charset encoding, FormatterConfig config) { + return new FormatterStepConfig(encoding, licenseHeaderDelimiter(), ratchetFrom(config), config.getProvisioner(), config.getFileLocator(), config.getSpotlessSetLicenseHeaderYearsFromGitHistory()); } private static List gatherStepFactories(List allGlobal, List allConfigured) { diff --git a/plugin-maven/src/main/java/com/diffplug/spotless/maven/FormatterStepConfig.java b/plugin-maven/src/main/java/com/diffplug/spotless/maven/FormatterStepConfig.java index ecaad55446..a1f2e52b41 100644 --- a/plugin-maven/src/main/java/com/diffplug/spotless/maven/FormatterStepConfig.java +++ b/plugin-maven/src/main/java/com/diffplug/spotless/maven/FormatterStepConfig.java @@ -1,5 +1,5 @@ /* - * Copyright 2016-2025 DiffPlug + * Copyright 2016-2020 DiffPlug * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -28,16 +28,14 @@ public class FormatterStepConfig { private final Provisioner provisioner; private final FileLocator fileLocator; private final Optional spotlessSetLicenseHeaderYearsFromGitHistory; - private final String name; - public FormatterStepConfig(Charset encoding, String licenseHeaderDelimiter, Optional ratchetFrom, Provisioner provisioner, FileLocator fileLocator, Optional spotlessSetLicenseHeaderYearsFromGitHistory, String name) { + public FormatterStepConfig(Charset encoding, String licenseHeaderDelimiter, Optional ratchetFrom, Provisioner provisioner, FileLocator fileLocator, Optional spotlessSetLicenseHeaderYearsFromGitHistory) { this.encoding = encoding; this.licenseHeaderDelimiter = licenseHeaderDelimiter; this.ratchetFrom = ratchetFrom; this.provisioner = provisioner; this.fileLocator = fileLocator; this.spotlessSetLicenseHeaderYearsFromGitHistory = spotlessSetLicenseHeaderYearsFromGitHistory; - this.name = name; } public Charset getEncoding() { @@ -63,8 +61,4 @@ public FileLocator getFileLocator() { public Optional spotlessSetLicenseHeaderYearsFromGitHistory() { return spotlessSetLicenseHeaderYearsFromGitHistory; } - - public String getName() { - return name; - } } 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 dea60739db..9b279c8845 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 @@ -1,5 +1,5 @@ /* - * Copyright 2021-2025 DiffPlug + * Copyright 2021-2024 DiffPlug * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -16,14 +16,11 @@ package com.diffplug.spotless.maven; import java.io.File; -import java.util.ArrayList; import java.util.Collection; import java.util.LinkedHashMap; -import java.util.List; import java.util.Map; import java.util.Map.Entry; import java.util.function.Supplier; -import java.util.stream.Collectors; import com.diffplug.spotless.Formatter; @@ -43,16 +40,12 @@ public String nameFor(FormatterFactory factory) { static FormattersHolder create(Map>> formatterFactoryToFiles, FormatterConfig config) { Map openFormatters = new LinkedHashMap<>(); try { - List>>> formatterEntries = new ArrayList<>(formatterFactoryToFiles.entrySet()); - for (int formatterIndex = 0; formatterIndex < formatterEntries.size(); formatterIndex++) { - Entry>> entry = formatterEntries.get(formatterIndex); + for (Entry>> entry : formatterFactoryToFiles.entrySet()) { FormatterFactory formatterFactory = entry.getKey(); Supplier> files = entry.getValue(); - Formatter formatter = formatterFactory.newFormatter(files, config, formatterIndex); + Formatter formatter = formatterFactory.newFormatter(files, config); openFormatters.put(formatterFactory, formatter); } - - System.out.println("Created formatters: " + openFormatters.keySet().stream().map(f -> f.includes()).collect(Collectors.toList())); } catch (RuntimeException openError) { try { close(openFormatters.values()); diff --git a/plugin-maven/src/main/java/com/diffplug/spotless/maven/generic/Prettier.java b/plugin-maven/src/main/java/com/diffplug/spotless/maven/generic/Prettier.java index f747f8764a..c6b3e46e3c 100644 --- a/plugin-maven/src/main/java/com/diffplug/spotless/maven/generic/Prettier.java +++ b/plugin-maven/src/main/java/com/diffplug/spotless/maven/generic/Prettier.java @@ -1,5 +1,5 @@ /* - * Copyright 2016-2025 DiffPlug + * Copyright 2016-2023 DiffPlug * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -103,7 +103,7 @@ public FormatterStep newFormatterStep(FormatterStepConfig stepConfig) { File cacheDir = cacheDir(stepConfig); PrettierConfig prettierConfig = new PrettierConfig(configFileHandler, configInline); NpmPathResolver npmPathResolver = npmPathResolver(stepConfig); - return PrettierFormatterStep.create(stepConfig.getName(), devDependencies, stepConfig.getProvisioner(), baseDir, buildDir, cacheDir, npmPathResolver, prettierConfig); + return PrettierFormatterStep.create(devDependencies, stepConfig.getProvisioner(), baseDir, buildDir, cacheDir, npmPathResolver, prettierConfig); } private static IllegalArgumentException onlyOneConfig() { diff --git a/plugin-maven/src/main/java/com/diffplug/spotless/maven/javascript/AbstractEslint.java b/plugin-maven/src/main/java/com/diffplug/spotless/maven/javascript/AbstractEslint.java index 2b85cdcebc..ad113de833 100644 --- a/plugin-maven/src/main/java/com/diffplug/spotless/maven/javascript/AbstractEslint.java +++ b/plugin-maven/src/main/java/com/diffplug/spotless/maven/javascript/AbstractEslint.java @@ -1,5 +1,5 @@ /* - * Copyright 2016-2025 DiffPlug + * Copyright 2016-2023 DiffPlug * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -69,7 +69,7 @@ public FormatterStep newFormatterStep(FormatterStepConfig stepConfig) { File baseDir = baseDir(stepConfig); File cacheDir = cacheDir(stepConfig); NpmPathResolver npmPathResolver = npmPathResolver(stepConfig); - return EslintFormatterStep.create(stepConfig.getName(), devDependencies, stepConfig.getProvisioner(), baseDir, buildDir, cacheDir, npmPathResolver, eslintConfig(stepConfig)); + return EslintFormatterStep.create(devDependencies, stepConfig.getProvisioner(), baseDir, buildDir, cacheDir, npmPathResolver, eslintConfig(stepConfig)); } private static IllegalArgumentException onlyOneConfig() { diff --git a/plugin-maven/src/test/java/com/diffplug/spotless/maven/prettier/PrettierFormatStepTest.java b/plugin-maven/src/test/java/com/diffplug/spotless/maven/prettier/PrettierFormatStepTest.java index f5f5bddccd..abba35e72c 100644 --- a/plugin-maven/src/test/java/com/diffplug/spotless/maven/prettier/PrettierFormatStepTest.java +++ b/plugin-maven/src/test/java/com/diffplug/spotless/maven/prettier/PrettierFormatStepTest.java @@ -1,5 +1,5 @@ /* - * Copyright 2016-2025 DiffPlug + * Copyright 2016-2023 DiffPlug * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -17,13 +17,7 @@ import static org.assertj.core.api.Assertions.assertThat; -import java.io.File; import java.io.IOException; -import java.nio.file.Files; -import java.nio.file.Path; -import java.util.List; -import java.util.stream.Collectors; -import java.util.stream.Stream; import org.junit.jupiter.api.Test; @@ -164,34 +158,6 @@ void multiple_prettier_configs() throws Exception { } - /** - * This test is to ensure that we can have multiple equivalent prettier instances in one spotless config. - */ - @Test - void multiple_equivalent_prettier_configs_do_not_intersect() throws Exception { - writePom( - formats( - groupWithSteps("myjson", including("test.json"), - "", - ""), - groupWithSteps("myts", including("test.ts"), - "", - ""))); - - setFile("test.ts").toResource("npm/prettier/config/typescript.dirty"); - setFile("test.json").toResource("npm/prettier/filetypes/json/json.dirty"); - mavenRunner().withArguments("spotless:apply").runNoError(); - - // check that each prettier instance has its own node_modules directory - File targetDir = new File(rootFolder(), "target"); - try (Stream files = Files.walk(targetDir.toPath())) { - List prettierFolders = files.filter(Files::isDirectory) - .filter(path -> path.getFileName().toString().contains("spotless-prettier")) - .collect(Collectors.toList()); - assertThat(prettierFolders).hasSize(2); // one for each prettier instance - } - } - @Test void custom_plugin() throws Exception { writePomWithFormatSteps( diff --git a/testlib/src/test/java/com/diffplug/spotless/npm/EslintFormatterStepTest.java b/testlib/src/test/java/com/diffplug/spotless/npm/EslintFormatterStepTest.java index 6b8debedfb..da00e37d35 100644 --- a/testlib/src/test/java/com/diffplug/spotless/npm/EslintFormatterStepTest.java +++ b/testlib/src/test/java/com/diffplug/spotless/npm/EslintFormatterStepTest.java @@ -1,5 +1,5 @@ /* - * Copyright 2016-2025 DiffPlug + * Copyright 2016-2024 DiffPlug * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -58,7 +58,6 @@ void formattingUsingRulesetsFile(String ruleSetName) throws Exception { final String cleanFile = filedir + "javascript-es6.clean"; final FormatterStep formatterStep = EslintFormatterStep.create( - "ESLINT_TEST", devDependenciesForRuleset.get(ruleSetName), TestProvisioner.mavenCentral(), projectDir(), @@ -101,7 +100,6 @@ void formattingUsingRulesetsFile(String ruleSetName) throws Exception { final String cleanFile = filedir + "typescript.clean"; final FormatterStep formatterStep = EslintFormatterStep.create( - "ESLINT_TEST", devDependenciesForRuleset.get(ruleSetName), TestProvisioner.mavenCentral(), projectDir(), @@ -159,7 +157,6 @@ void formattingUsingInlineXoConfig() throws Exception { final String cleanFile = filedir + "typescript.clean"; final FormatterStep formatterStep = EslintFormatterStep.create( - "ESLINT_TEST", EslintStyleGuide.TS_XO_TYPESCRIPT.mergedWith(EslintFormatterStep.defaultDevDependenciesForTypescript()), TestProvisioner.mavenCentral(), projectDir(), diff --git a/testlib/src/test/java/com/diffplug/spotless/npm/PrettierFormatterStepTest.java b/testlib/src/test/java/com/diffplug/spotless/npm/PrettierFormatterStepTest.java index 915d757d33..54ce09e7e8 100644 --- a/testlib/src/test/java/com/diffplug/spotless/npm/PrettierFormatterStepTest.java +++ b/testlib/src/test/java/com/diffplug/spotless/npm/PrettierFormatterStepTest.java @@ -1,5 +1,5 @@ /* - * Copyright 2016-2025 DiffPlug + * Copyright 2016-2024 DiffPlug * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -63,7 +63,6 @@ private void runTestUsingPrettier(String fileType, Map dependenc final String cleanFile = filedir + fileType + ".clean"; final FormatterStep formatterStep = PrettierFormatterStep.create( - "PRETTIER_TEST", dependencies, TestProvisioner.mavenCentral(), projectDir(), @@ -91,7 +90,6 @@ void parserInferenceBasedOnExplicitFilepathIsWorking(String prettierVersion) thr final String cleanFile = filedir + "json.clean"; final FormatterStep formatterStep = PrettierFormatterStep.create( - "PRETTIER_TEST", ImmutableMap.of("prettier", prettierVersion), TestProvisioner.mavenCentral(), projectDir(), @@ -114,7 +112,6 @@ void parserInferenceBasedOnFilenameIsWorking(String prettierVersion) throws Exce final String cleanFile = filedir + "clean.json"; final FormatterStep formatterStep = PrettierFormatterStep.create( - "PRETTIER_TEST", ImmutableMap.of("prettier", prettierVersion), TestProvisioner.mavenCentral(), projectDir(), @@ -131,7 +128,6 @@ void parserInferenceBasedOnFilenameIsWorking(String prettierVersion) throws Exce @Test void verifyPrettierErrorMessageIsRelayed() throws Exception { FormatterStep formatterStep = PrettierFormatterStep.create( - "PRETTIER_TEST", PrettierFormatterStep.defaultDevDependenciesWithPrettier("2.8.8"), TestProvisioner.mavenCentral(), projectDir(), @@ -141,7 +137,7 @@ void verifyPrettierErrorMessageIsRelayed() throws Exception { new PrettierConfig(null, ImmutableMap.of("parser", "postcss"))); try (StepHarnessWithFile stepHarness = StepHarnessWithFile.forStep(this, formatterStep)) { stepHarness.expectLintsOfResource("npm/prettier/filetypes/scss/scss.dirty") - .toBe("LINE_UNDEFINED PRETTIER_TEST-prettier-format(com.diffplug.spotless.npm.SimpleRestClient$SimpleRestResponseException) Unexpected response status code at /prettier/format [HTTP 500] -- (Error while formatting: Error: Couldn't resolve parser \"postcss\") (...)"); + .toBe("LINE_UNDEFINED prettier-format(com.diffplug.spotless.npm.SimpleRestClient$SimpleRestResponseException) Unexpected response status code at /prettier/format [HTTP 500] -- (Error while formatting: Error: Couldn't resolve parser \"postcss\") (...)"); } } } @@ -158,7 +154,6 @@ void runFormatTest(String prettierVersion, PrettierConfig config, String cleanFi final String cleanFile = FILEDIR + "typescript." + cleanFileNameSuffix + ".clean"; final FormatterStep formatterStep = PrettierFormatterStep.create( - "PRETTIER_TEST", ImmutableMap.of("prettier", prettierVersion), TestProvisioner.mavenCentral(), projectDir(), @@ -215,7 +210,6 @@ protected void setupTest(API api) { @Override protected FormatterStep create() { return PrettierFormatterStep.create( - "PRETTIER_TEST", ImmutableMap.of("prettier", prettierVersion), TestProvisioner.mavenCentral(), projectDir(), From 2466e47b6afed1a1bbd7389a4752062d4f692c24 Mon Sep 17 00:00:00 2001 From: Simon Gamma Date: Tue, 8 Apr 2025 16:37:38 +0200 Subject: [PATCH 07/11] fix: make sure node_modules dir is exclusively created Refs: #2462 --- .../spotless/npm/ExclusiveFolderAccess.java | 67 +++++++++++++++++++ .../npm/NpmFormatterStepStateBase.java | 21 +++--- 2 files changed, 79 insertions(+), 9 deletions(-) create mode 100644 lib/src/main/java/com/diffplug/spotless/npm/ExclusiveFolderAccess.java diff --git a/lib/src/main/java/com/diffplug/spotless/npm/ExclusiveFolderAccess.java b/lib/src/main/java/com/diffplug/spotless/npm/ExclusiveFolderAccess.java new file mode 100644 index 0000000000..f36fcbb8b4 --- /dev/null +++ b/lib/src/main/java/com/diffplug/spotless/npm/ExclusiveFolderAccess.java @@ -0,0 +1,67 @@ +/* + * Copyright 2025 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.npm; + +import java.io.File; +import java.util.Objects; +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.locks.Lock; +import java.util.concurrent.locks.ReentrantLock; + +import javax.annotation.Nonnull; + +import com.diffplug.spotless.ThrowingEx; + +interface ExclusiveFolderAccess { + + static ExclusiveFolderAccess forFolder(@Nonnull File folder) { + return forFolder(folder.getAbsolutePath()); + } + + static ExclusiveFolderAccess forFolder(@Nonnull String path) { + return new ExclusiveFolderAccessSharedMutex(Objects.requireNonNull(path)); + } + + void runExclusively(ThrowingEx.Runnable runnable); + + class ExclusiveFolderAccessSharedMutex implements ExclusiveFolderAccess { + + private static final ConcurrentHashMap mutexes = new ConcurrentHashMap<>(); + + private final String path; + + private ExclusiveFolderAccessSharedMutex(@Nonnull String path) { + this.path = Objects.requireNonNull(path); + } + + private Lock getMutex() { + return mutexes.computeIfAbsent(path, k -> new ReentrantLock()); + } + + @Override + public void runExclusively(ThrowingEx.Runnable runnable) { + final Lock lock = getMutex(); + try { + lock.lock(); + runnable.run(); + } catch (Exception e) { + throw ThrowingEx.asRuntime(e); + } finally { + lock.unlock(); + } + } + } +} diff --git a/lib/src/main/java/com/diffplug/spotless/npm/NpmFormatterStepStateBase.java b/lib/src/main/java/com/diffplug/spotless/npm/NpmFormatterStepStateBase.java index 3ec06e0434..eeb70b96c4 100644 --- a/lib/src/main/java/com/diffplug/spotless/npm/NpmFormatterStepStateBase.java +++ b/lib/src/main/java/com/diffplug/spotless/npm/NpmFormatterStepStateBase.java @@ -1,5 +1,5 @@ /* - * Copyright 2016-2024 DiffPlug + * Copyright 2016-2025 DiffPlug * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -87,14 +87,17 @@ protected void prepareNodeServer() throws IOException { } protected void assertNodeServerDirReady() throws IOException { - if (needsPrepareNodeServerLayout()) { - // reinstall if missing - prepareNodeServerLayout(); - } - if (needsPrepareNodeServer()) { - // run npm install if node_modules is missing - prepareNodeServer(); - } + ExclusiveFolderAccess.forFolder(nodeServerLayout.nodeModulesDir()) + .runExclusively(() -> { + if (needsPrepareNodeServerLayout()) { + // reinstall if missing + prepareNodeServerLayout(); + } + if (needsPrepareNodeServer()) { + // run npm install if node_modules is missing + prepareNodeServer(); + } + }); } protected boolean needsPrepareNodeServer() { From 1f50067b5068eb130b9145b573110e9fb811a806 Mon Sep 17 00:00:00 2001 From: Simon Gamma Date: Tue, 8 Apr 2025 17:46:59 +0200 Subject: [PATCH 08/11] feat: allow process to be started using instance_id to be able to launch multiple instances of npm in the same node_modules dir. Refs: #2462 --- .../NodeModulesCachingNpmProcessFactory.java | 7 +++-- .../diffplug/spotless/npm/NodeServeApp.java | 8 +++-- .../npm/NpmFormatterStepStateBase.java | 8 +++-- .../spotless/npm/NpmProcessFactory.java | 6 ++-- .../npm/StandardNpmProcessFactory.java | 16 ++++++---- .../com/diffplug/spotless/npm/common-serve.js | 30 +++++++++++++++---- 6 files changed, 54 insertions(+), 21 deletions(-) diff --git a/lib/src/main/java/com/diffplug/spotless/npm/NodeModulesCachingNpmProcessFactory.java b/lib/src/main/java/com/diffplug/spotless/npm/NodeModulesCachingNpmProcessFactory.java index 2f1addf2af..a821dd6b8e 100644 --- a/lib/src/main/java/com/diffplug/spotless/npm/NodeModulesCachingNpmProcessFactory.java +++ b/lib/src/main/java/com/diffplug/spotless/npm/NodeModulesCachingNpmProcessFactory.java @@ -1,5 +1,5 @@ /* - * Copyright 2023-2024 DiffPlug + * Copyright 2023-2025 DiffPlug * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -18,6 +18,7 @@ import java.io.File; import java.util.List; import java.util.Objects; +import java.util.UUID; import javax.annotation.Nonnull; @@ -65,8 +66,8 @@ public NpmProcess createNpmInstallProcess(NodeServerLayout nodeServerLayout, Npm } @Override - public NpmLongRunningProcess createNpmServeProcess(NodeServerLayout nodeServerLayout, NpmFormatterStepLocations formatterStepLocations) { - return StandardNpmProcessFactory.INSTANCE.createNpmServeProcess(nodeServerLayout, formatterStepLocations); + public NpmLongRunningProcess createNpmServeProcess(NodeServerLayout nodeServerLayout, NpmFormatterStepLocations formatterStepLocations, UUID nodeServerInstanceId) { + return StandardNpmProcessFactory.INSTANCE.createNpmServeProcess(nodeServerLayout, formatterStepLocations, nodeServerInstanceId); } private class CachingNmpInstall implements NpmProcess { diff --git a/lib/src/main/java/com/diffplug/spotless/npm/NodeServeApp.java b/lib/src/main/java/com/diffplug/spotless/npm/NodeServeApp.java index c9311a5589..3e2efd43e8 100644 --- a/lib/src/main/java/com/diffplug/spotless/npm/NodeServeApp.java +++ b/lib/src/main/java/com/diffplug/spotless/npm/NodeServeApp.java @@ -1,5 +1,5 @@ /* - * Copyright 2023 DiffPlug + * Copyright 2023-2025 DiffPlug * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -15,6 +15,8 @@ */ package com.diffplug.spotless.npm; +import java.util.UUID; + import javax.annotation.Nonnull; import org.slf4j.Logger; @@ -32,9 +34,9 @@ public NodeServeApp(@Nonnull NodeServerLayout nodeServerLayout, @Nonnull NpmConf super(nodeServerLayout, npmConfig, formatterStepLocations); } - ProcessRunner.LongRunningProcess startNpmServeProcess() { + ProcessRunner.LongRunningProcess startNpmServeProcess(UUID nodeServerInstanceId) { return timedLogger.withInfo("Starting npm based server in {} with {}.", this.nodeServerLayout.nodeModulesDir(), this.npmProcessFactory.describe()) - .call(() -> npmProcessFactory.createNpmServeProcess(nodeServerLayout, formatterStepLocations).start()); + .call(() -> npmProcessFactory.createNpmServeProcess(nodeServerLayout, formatterStepLocations, nodeServerInstanceId).start()); } } diff --git a/lib/src/main/java/com/diffplug/spotless/npm/NpmFormatterStepStateBase.java b/lib/src/main/java/com/diffplug/spotless/npm/NpmFormatterStepStateBase.java index eeb70b96c4..71eba3594b 100644 --- a/lib/src/main/java/com/diffplug/spotless/npm/NpmFormatterStepStateBase.java +++ b/lib/src/main/java/com/diffplug/spotless/npm/NpmFormatterStepStateBase.java @@ -25,6 +25,7 @@ import java.util.Iterator; import java.util.Map; import java.util.Map.Entry; +import java.util.UUID; import java.util.concurrent.TimeUnit; import java.util.concurrent.TimeoutException; @@ -112,12 +113,13 @@ protected ServerProcessInfo npmRunServer() throws ServerStartException, IOExcept assertNodeServerDirReady(); LongRunningProcess server = null; try { - // The npm process will output the randomly selected port of the http server process to 'server.port' file + final UUID nodeServerInstanceId = UUID.randomUUID(); + // The npm process will output the randomly selected port of the http server process to 'server-.port' file // so in order to be safe, remove such a file if it exists before starting. - final File serverPortFile = new File(this.nodeServerLayout.nodeModulesDir(), "server.port"); + final File serverPortFile = new File(this.nodeServerLayout.nodeModulesDir(), String.format("server-%s.port", nodeServerInstanceId)); NpmResourceHelper.deleteFileIfExists(serverPortFile); // start the http server in node - server = nodeServeApp.startNpmServeProcess(); + server = nodeServeApp.startNpmServeProcess(nodeServerInstanceId); // await the readiness of the http server - wait for at most 60 seconds try { diff --git a/lib/src/main/java/com/diffplug/spotless/npm/NpmProcessFactory.java b/lib/src/main/java/com/diffplug/spotless/npm/NpmProcessFactory.java index dbc9a807d5..4b3edee462 100644 --- a/lib/src/main/java/com/diffplug/spotless/npm/NpmProcessFactory.java +++ b/lib/src/main/java/com/diffplug/spotless/npm/NpmProcessFactory.java @@ -1,5 +1,5 @@ /* - * Copyright 2023 DiffPlug + * Copyright 2023-2025 DiffPlug * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -15,6 +15,8 @@ */ package com.diffplug.spotless.npm; +import java.util.UUID; + public interface NpmProcessFactory { enum OnlinePreferrence { @@ -33,7 +35,7 @@ public String option() { NpmProcess createNpmInstallProcess(NodeServerLayout nodeServerLayout, NpmFormatterStepLocations formatterStepLocations, OnlinePreferrence onlinePreferrence); - NpmLongRunningProcess createNpmServeProcess(NodeServerLayout nodeServerLayout, NpmFormatterStepLocations formatterStepLocations); + NpmLongRunningProcess createNpmServeProcess(NodeServerLayout nodeServerLayout, NpmFormatterStepLocations formatterStepLocations, UUID nodeServerInstanceId); default String describe() { return getClass().getSimpleName(); diff --git a/lib/src/main/java/com/diffplug/spotless/npm/StandardNpmProcessFactory.java b/lib/src/main/java/com/diffplug/spotless/npm/StandardNpmProcessFactory.java index 91cf80ad69..b9abd03029 100644 --- a/lib/src/main/java/com/diffplug/spotless/npm/StandardNpmProcessFactory.java +++ b/lib/src/main/java/com/diffplug/spotless/npm/StandardNpmProcessFactory.java @@ -1,5 +1,5 @@ /* - * Copyright 2023 DiffPlug + * Copyright 2023-2025 DiffPlug * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -19,6 +19,7 @@ import java.io.IOException; import java.util.List; import java.util.Map; +import java.util.UUID; import java.util.concurrent.ExecutionException; import com.diffplug.spotless.ProcessRunner; @@ -37,8 +38,8 @@ public NpmProcess createNpmInstallProcess(NodeServerLayout nodeServerLayout, Npm } @Override - public NpmLongRunningProcess createNpmServeProcess(NodeServerLayout nodeServerLayout, NpmFormatterStepLocations formatterStepLocations) { - return new NpmServe(nodeServerLayout.nodeModulesDir(), formatterStepLocations); + public NpmLongRunningProcess createNpmServeProcess(NodeServerLayout nodeServerLayout, NpmFormatterStepLocations formatterStepLocations, UUID nodeServerInstanceId) { + return new NpmServe(nodeServerLayout.nodeModulesDir(), formatterStepLocations, nodeServerInstanceId); } private static abstract class AbstractStandardNpmProcess { @@ -119,8 +120,11 @@ public ProcessRunner.Result waitFor() { private static class NpmServe extends AbstractStandardNpmProcess implements NpmLongRunningProcess { - public NpmServe(File workingDir, NpmFormatterStepLocations formatterStepLocations) { + private final UUID nodeServerInstanceId; + + public NpmServe(File workingDir, NpmFormatterStepLocations formatterStepLocations, UUID nodeServerInstanceId) { super(workingDir, formatterStepLocations); + this.nodeServerInstanceId = nodeServerInstanceId; } @Override @@ -128,7 +132,9 @@ protected List commandLine() { return List.of( npmExecutable(), "start", - "--scripts-prepend-node-path=true"); + "--scripts-prepend-node-path=true", + "--", + "--node-server-instance-id=" + nodeServerInstanceId); } @Override diff --git a/lib/src/main/resources/com/diffplug/spotless/npm/common-serve.js b/lib/src/main/resources/com/diffplug/spotless/npm/common-serve.js index c1c9d62757..5bfcb35612 100644 --- a/lib/src/main/resources/com/diffplug/spotless/npm/common-serve.js +++ b/lib/src/main/resources/com/diffplug/spotless/npm/common-serve.js @@ -4,7 +4,7 @@ const GracefulShutdownManager = require("@moebius/http-graceful-shutdown").Grace const express = require("express"); const app = express(); -app.use(express.json({ limit: "50mb" })); +app.use(express.json({limit: "50mb"})); const fs = require("fs"); @@ -14,13 +14,33 @@ function debugLog() { } } +function getInstanceId() { + const args = process.argv.slice(2); + + // Look for the --node-server-instance-id option + let instanceId; + + args.forEach(arg => { + if (arg.startsWith('--node-server-instance-id=')) { + instanceId = arg.split('=')[1]; + } + }); + + // throw if instanceId is not set + if (!instanceId) { + throw new Error("Missing --node-server-instance-id argument"); + } + return instanceId; +} + var listener = app.listen(0, "127.0.0.1", () => { - debugLog("Server running on port " + listener.address().port); - fs.writeFile("server.port.tmp", "" + listener.address().port, function(err) { + const instanceId = getInstanceId(); + debugLog("Server running on port " + listener.address().port + " for instance " + instanceId); + fs.writeFile("server.port.tmp", "" + listener.address().port, function (err) { if (err) { return console.log(err); } else { - fs.rename("server.port.tmp", "server.port", function(err) { + fs.rename("server.port.tmp", `server-${instanceId}.port`, function (err) { if (err) { return console.log(err); } @@ -32,7 +52,7 @@ const shutdownManager = new GracefulShutdownManager(listener); app.post("/shutdown", (req, res) => { res.status(200).send("Shutting down"); - setTimeout(function() { + setTimeout(function () { shutdownManager.terminate(() => debugLog("graceful shutdown finished.")); }, 200); }); From 01f4dedc2ddfd4f393b41ba7aa94436bb3952c0b Mon Sep 17 00:00:00 2001 From: Simon Gamma Date: Tue, 8 Apr 2025 22:33:46 +0200 Subject: [PATCH 09/11] chore: add more logging to server starting --- .../diffplug/spotless/npm/NpmFormatterStepStateBase.java | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/lib/src/main/java/com/diffplug/spotless/npm/NpmFormatterStepStateBase.java b/lib/src/main/java/com/diffplug/spotless/npm/NpmFormatterStepStateBase.java index 71eba3594b..6c4ddc0098 100644 --- a/lib/src/main/java/com/diffplug/spotless/npm/NpmFormatterStepStateBase.java +++ b/lib/src/main/java/com/diffplug/spotless/npm/NpmFormatterStepStateBase.java @@ -33,6 +33,7 @@ import org.slf4j.LoggerFactory; import com.diffplug.spotless.FormatterFunc; +import com.diffplug.spotless.ProcessRunner; import com.diffplug.spotless.ProcessRunner.LongRunningProcess; import com.diffplug.spotless.ThrowingEx; @@ -129,10 +130,12 @@ protected ServerProcessInfo npmRunServer() throws ServerStartException, IOExcept try { if (server.isAlive()) { server.destroyForcibly(); - server.waitFor(); + ProcessRunner.Result result = server.result(); + logger.info("Launching npm server process failed. Process result:\n{}", result); } } catch (Throwable t) { - // ignore + ProcessRunner.Result result = server != null ? ThrowingEx.get(server::result) : null; + logger.debug("Unable to forcibly end the server process. Process result:\n{}", result, t); } throw timeoutException; } From 65b4e0869d51ef89844f11c24c9f70ed79b23a8e Mon Sep 17 00:00:00 2001 From: Simon Gamma Date: Tue, 8 Apr 2025 22:42:29 +0200 Subject: [PATCH 10/11] docs: adapt CHANGES.md files to new approach before: make sure every formatter uses its own node_modules dir. now: actively support using same node_modules dir for multiple formatters Refs: #2462 --- CHANGES.md | 2 +- plugin-gradle/CHANGES.md | 2 +- plugin-maven/CHANGES.md | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index fe1e97c286..6b2092441d 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -13,7 +13,7 @@ We adhere to the [keepachangelog](https://keepachangelog.com/en/1.0.0/) format ( ### Changed * Use palantir-java-format 2.57.0 on Java 21. ([#2447](https://github.com/diffplug/spotless/pull/2447)) * Re-try `npm install` with `--prefer-online` after `ERESOLVE` error. ([#2448](https://github.com/diffplug/spotless/pull/2448)) -* **BREAKING** Assert that each npm-based step uses its own `node_modules` directory, even when configured exactly the same, to prevent race conditions when running in parallel. For this, the API of npm-based formatter steps has been changed. ([#2462](https://github.com/diffplug/spotless/pull/2462)) +* Allow multiple npm-based formatters having the same module dependencies, to share a `node_modules` dir without race conditions. [#2462](https://github.com/diffplug/spotless/pull/2462)) ## [3.1.0] - 2025-02-20 ### Added diff --git a/plugin-gradle/CHANGES.md b/plugin-gradle/CHANGES.md index c402a1b09b..25bfbdef92 100644 --- a/plugin-gradle/CHANGES.md +++ b/plugin-gradle/CHANGES.md @@ -7,7 +7,7 @@ We adhere to the [keepachangelog](https://keepachangelog.com/en/1.0.0/) format ( * Use palantir-java-format 2.57.0 on Java 21. ([#2447](https://github.com/diffplug/spotless/pull/2447)) * Re-try `npm install` with `--prefer-online` after `ERESOLVE` error. ([#2448](https://github.com/diffplug/spotless/pull/2448)) * Apply Gradle's strict plugin types validation to the Spotless plugin. ([#2454](https://github.com/diffplug/spotless/pull/2454)) -* Assert that each npm-based step uses its own `node_modules` directory, even when configured exactly the same to prevent race conditions when running in parallel. ([#2462](https://github.com/diffplug/spotless/pull/2462)) +* Allow multiple npm-based formatters having the same module dependencies, to share a `node_modules` dir without race conditions. [#2462](https://github.com/diffplug/spotless/pull/2462)) ## [7.0.2] - 2025-01-14 ### Fixed diff --git a/plugin-maven/CHANGES.md b/plugin-maven/CHANGES.md index f9ea744bae..38402fc5ad 100644 --- a/plugin-maven/CHANGES.md +++ b/plugin-maven/CHANGES.md @@ -6,7 +6,7 @@ We adhere to the [keepachangelog](https://keepachangelog.com/en/1.0.0/) format ( ### Changed * Use palantir-java-format 2.57.0 on Java 21. ([#2447](https://github.com/diffplug/spotless/pull/2447)) * Re-try `npm install` with `--prefer-online` after `ERESOLVE` error. ([#2448](https://github.com/diffplug/spotless/pull/2448)) -* Assert that each npm-based step uses its own `node_modules` directory, even when configured exactly the same to prevent race conditions when running in parallel. ([#2462](https://github.com/diffplug/spotless/pull/2462)) +* Allow multiple npm-based formatters having the same module dependencies, to share a `node_modules` dir without race conditions. [#2462](https://github.com/diffplug/spotless/pull/2462)) ## [2.44.3] - 2025-02-20 * Support for `clang-format` ([#2406](https://github.com/diffplug/spotless/pull/2406)) From 279372b1c35bb9cc0c1bf3ed545ec00c8d82c7b0 Mon Sep 17 00:00:00 2001 From: Simon Gamma Date: Wed, 9 Apr 2025 05:57:42 +0200 Subject: [PATCH 11/11] fix: spotbugs issue --- .../com/diffplug/spotless/npm/NpmFormatterStepStateBase.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/src/main/java/com/diffplug/spotless/npm/NpmFormatterStepStateBase.java b/lib/src/main/java/com/diffplug/spotless/npm/NpmFormatterStepStateBase.java index 6c4ddc0098..9b37533880 100644 --- a/lib/src/main/java/com/diffplug/spotless/npm/NpmFormatterStepStateBase.java +++ b/lib/src/main/java/com/diffplug/spotless/npm/NpmFormatterStepStateBase.java @@ -134,7 +134,7 @@ protected ServerProcessInfo npmRunServer() throws ServerStartException, IOExcept logger.info("Launching npm server process failed. Process result:\n{}", result); } } catch (Throwable t) { - ProcessRunner.Result result = server != null ? ThrowingEx.get(server::result) : null; + ProcessRunner.Result result = ThrowingEx.get(server::result); logger.debug("Unable to forcibly end the server process. Process result:\n{}", result, t); } throw timeoutException;