close
Skip to content
Merged
Next Next commit
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.
  • Loading branch information
simschla committed Apr 7, 2025
commit da3b75b4cc8e50960d3dbbf2601d1ba4ab9613f1
Original file line number Diff line number Diff line change
@@ -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.
Expand Down Expand Up @@ -68,13 +68,14 @@ public static Map<String, String> defaultDevDependenciesWithEslint(String versio
return Collections.singletonMap("eslint", version);
}

public static FormatterStep create(Map<String, String> devDependencies, Provisioner provisioner, File projectDir, File buildDir, File cacheDir, NpmPathResolver npmPathResolver, EslintConfig eslintConfig) {
public static FormatterStep create(String formatName, Map<String, String> 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);
}

Expand Down
Original file line number Diff line number Diff line change
@@ -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.
Expand Down Expand Up @@ -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);
}

Expand Down
Original file line number Diff line number Diff line change
@@ -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.
Expand Down Expand Up @@ -51,12 +51,13 @@ public static final Map<String, String> defaultDevDependenciesWithPrettier(Strin
return Collections.singletonMap("prettier", version);
}

public static FormatterStep create(Map<String, String> devDependencies, Provisioner provisioner, File projectDir, File buildDir, File cacheDir, NpmPathResolver npmPathResolver, PrettierConfig prettierConfig) {
public static FormatterStep create(String formatName, Map<String, String> 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);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -798,7 +798,7 @@ public PrettierConfig config(final Map<String, Object> 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())),
Expand Down
Original file line number Diff line number Diff line change
@@ -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.
Expand Down Expand Up @@ -106,7 +106,7 @@ public JavascriptEslintConfig(Map<String, String> 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())),
Expand Down
Original file line number Diff line number Diff line change
@@ -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.
Expand Down Expand Up @@ -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())),
Expand Down
Original file line number Diff line number Diff line change
@@ -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.
Expand All @@ -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;

Expand Down Expand Up @@ -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");
Expand Down Expand Up @@ -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();
Expand All @@ -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");
Expand All @@ -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");
}

Expand Down Expand Up @@ -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");
}

Expand Down Expand Up @@ -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");
}

Expand All @@ -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}")
Expand Down Expand Up @@ -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");
}

Expand Down Expand Up @@ -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");
}
Expand Down Expand Up @@ -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}")
Expand All @@ -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}")
Expand All @@ -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}")
Expand All @@ -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<Path> pathStream = Files.walk(buildFolder.toPath())) {
List<Path> nodeModulesDirs = pathStream
.sorted()
.filter(Files::isDirectory)
.filter(path -> path.getFileName().toString().contains("spotless-prettier"))
.collect(Collectors.toList());
assertThat(nodeModulesDirs).hasSize(3);
}
}
}
Original file line number Diff line number Diff line change
@@ -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.
Expand Down Expand Up @@ -83,12 +83,12 @@ public final Set<String> excludes() {
return excludes == null ? emptySet() : Sets.newHashSet(excludes);
}

public final Formatter newFormatter(Supplier<Iterable<File>> filesToFormat, FormatterConfig config) {
public final Formatter newFormatter(Supplier<Iterable<File>> 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<FormatterStepFactory> factories = gatherStepFactories(config.getGlobalStepFactories(), stepFactories);

List<FormatterStep> formatterSteps = factories.stream()
Expand Down Expand Up @@ -174,8 +174,8 @@ Optional<String> 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<FormatterStepFactory> gatherStepFactories(List<FormatterStepFactory> allGlobal, List<FormatterStepFactory> allConfigured) {
Expand Down
Original file line number Diff line number Diff line change
@@ -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.
Expand Down Expand Up @@ -28,14 +28,16 @@ public class FormatterStepConfig {
private final Provisioner provisioner;
private final FileLocator fileLocator;
private final Optional<String> spotlessSetLicenseHeaderYearsFromGitHistory;
private final String name;

public FormatterStepConfig(Charset encoding, String licenseHeaderDelimiter, Optional<String> ratchetFrom, Provisioner provisioner, FileLocator fileLocator, Optional<String> spotlessSetLicenseHeaderYearsFromGitHistory) {
public FormatterStepConfig(Charset encoding, String licenseHeaderDelimiter, Optional<String> ratchetFrom, Provisioner provisioner, FileLocator fileLocator, Optional<String> spotlessSetLicenseHeaderYearsFromGitHistory, String name) {
this.encoding = encoding;
this.licenseHeaderDelimiter = licenseHeaderDelimiter;
this.ratchetFrom = ratchetFrom;
this.provisioner = provisioner;
this.fileLocator = fileLocator;
this.spotlessSetLicenseHeaderYearsFromGitHistory = spotlessSetLicenseHeaderYearsFromGitHistory;
this.name = name;
Comment thread
simschla marked this conversation as resolved.
Outdated
}

public Charset getEncoding() {
Expand All @@ -61,4 +63,8 @@ public FileLocator getFileLocator() {
public Optional<String> spotlessSetLicenseHeaderYearsFromGitHistory() {
return spotlessSetLicenseHeaderYearsFromGitHistory;
}

public String getName() {
return name;
}
}
Loading