Add support to allow multiple prettier instances for different target…

… source sets (#1565 fixes #1162)

CHANGES.md

@@ -12,7 +12,9 @@ We adhere to the [keepachangelog](https://keepachangelog.com/en/1.0.0/) format (
 ## [Unreleased]
 ### Added
 * CleanThat Java Refactorer. ([#1560](https://github.com/diffplug/spotless/pull/1560))
+* Introduce `LazyArgLogger` to allow for lazy evaluation of log messages in slf4j logging. ([#1565](https://github.com/diffplug/spotless/pull/1565))
 ### Fixed
+* Allow multiple instances of the same npm-based formatter to be used by separating their `node_modules` directories. ([#1565](https://github.com/diffplug/spotless/pull/1565))
 * `ktfmt` default style uses correct continuation indent. ([#1562](https://github.com/diffplug/spotless/pull/1562))
 
 ## [2.34.1] - 2023-02-05

lib/src/main/java/com/diffplug/spotless/LazyArgLogger.java

@@ -0,0 +1,40 @@
+/*
+ * Copyright 2023 DiffPlug
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package com.diffplug.spotless;
+
+import java.util.function.Supplier;
+
+/**
+ * This is a utility class to allow for lazy evaluation of arguments to be passed to a logger
+ * and thus avoid unnecessary computation of the arguments if the log level is not enabled.
+ */
+public final class LazyArgLogger {
+
+	private final Supplier<Object> argSupplier;
+
+	private LazyArgLogger(Supplier<Object> argSupplier) {
+		this.argSupplier = argSupplier;
+	}
+
+	public static LazyArgLogger lazy(Supplier<Object> argSupplier) {
+		return new LazyArgLogger(argSupplier);
+	}
+
+	@Override
+	public String toString() {
+		return String.valueOf(argSupplier.get());
+	}
+}

lib/src/main/java/com/diffplug/spotless/npm/EslintFormatterStep.java

@@ -15,6 +15,7 @@
  */
 package com.diffplug.spotless.npm;
 
+import static com.diffplug.spotless.LazyArgLogger.lazy;
 import static java.util.Objects.requireNonNull;
 
 import java.io.File;
@@ -115,7 +116,7 @@ protected void prepareNodeServerLayout() throws IOException {
 				// If any config files are provided, we need to make sure they are at the same location as the node modules
 				// as eslint will try to resolve plugin/config names relatively to the config file location and some
 				// eslint configs contain relative paths to additional config files (such as tsconfig.json e.g.)
-				FormattedPrinter.SYSOUT.print("Copying config file <%s> to <%s> and using the copy", origEslintConfig.getEslintConfigPath(), nodeServerLayout.nodeModulesDir());
+				logger.info("Copying config file <{}> to <{}> and using the copy", origEslintConfig.getEslintConfigPath(), nodeServerLayout.nodeModulesDir());
 				File configFileCopy = NpmResourceHelper.copyFileToDir(origEslintConfig.getEslintConfigPath(), nodeServerLayout.nodeModulesDir());
 				this.eslintConfigInUse = this.origEslintConfig.withEslintConfigPath(configFileCopy).verify();
 			}
@@ -125,7 +126,7 @@ protected void prepareNodeServerLayout() throws IOException {
 		@Nonnull
 		public FormatterFunc createFormatterFunc() {
 			try {
-				FormattedPrinter.SYSOUT.print("creating formatter function (starting server)");
+				logger.info("Creating formatter function (starting server)");
 				ServerProcessInfo eslintRestServer = npmRunServer();
 				EslintRestService restService = new EslintRestService(eslintRestServer.getBaseUrl());
 				return Closeable.ofDangerous(() -> endServer(restService, eslintRestServer), new EslintFilePathPassingFormatterFunc(locations.projectDir(), nodeServerLayout.nodeModulesDir(), eslintConfigInUse, restService));
@@ -135,7 +136,7 @@ public FormatterFunc createFormatterFunc() {
 		}
 
 		private void endServer(BaseNpmRestService restService, ServerProcessInfo restServer) throws Exception {
-			FormattedPrinter.SYSOUT.print("Closing formatting function (ending server).");
+			logger.info("Closing formatting function (ending server).");
 			try {
 				restService.shutdown();
 			} catch (Throwable t) {
@@ -161,7 +162,7 @@ public EslintFilePathPassingFormatterFunc(File projectDir, File nodeModulesDir,
 
 		@Override
 		public String applyWithFile(String unix, File file) throws Exception {
-			FormattedPrinter.SYSOUT.print("formatting String '" + unix.substring(0, Math.min(50, unix.length())) + "[...]' in file '" + file + "'");
+			logger.info("formatting String '{}[...]' in file '{}'", lazy(() -> unix.substring(0, Math.min(50, unix.length()))), file);
 
 			Map<FormatOption, Object> eslintCallOptions = new HashMap<>();
 			setConfigToCallOptions(eslintCallOptions);

lib/src/main/java/com/diffplug/spotless/npm/NodeServerLayout.java

@@ -18,25 +18,44 @@
 import java.io.File;
 import java.nio.file.Files;
 import java.nio.file.Path;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
 import java.util.stream.Stream;
 
 import com.diffplug.spotless.ThrowingEx;
 
 class NodeServerLayout {
 
+	private static final Pattern PACKAGE_JSON_NAME_PATTERN = Pattern.compile("\"name\"\\s*:\\s*\"([^\"]+)\"");
+
 	private final File nodeModulesDir;
 	private final File packageJsonFile;
+
+	private final File packageLockJsonFile;
+
 	private final File serveJsFile;
 	private final File npmrcFile;
 
-	NodeServerLayout(File buildDir, String stepName) {
-		this.nodeModulesDir = new File(buildDir, "spotless-node-modules-" + stepName);
+	NodeServerLayout(File buildDir, String packageJsonContent) {
+		this.nodeModulesDir = new File(buildDir, nodeModulesDirName(packageJsonContent));
 		this.packageJsonFile = new File(nodeModulesDir, "package.json");
+		this.packageLockJsonFile = new File(nodeModulesDir, "package-lock.json");
 		this.serveJsFile = new File(nodeModulesDir, "serve.js");
 		this.npmrcFile = new File(nodeModulesDir, ".npmrc");
 	}
 
+	private static String nodeModulesDirName(String packageJsonContent) {
+		String md5Hash = NpmResourceHelper.md5(packageJsonContent);
+		Matcher matcher = PACKAGE_JSON_NAME_PATTERN.matcher(packageJsonContent);
+		if (!matcher.find()) {
+			throw new IllegalArgumentException("package.json must contain a name property");
+		}
+		String packageName = matcher.group(1);
+		return String.format("%s-node-modules-%s", packageName, md5Hash);
+	}
+
 	File nodeModulesDir() {
+
 		return nodeModulesDir;
 	}
 
@@ -52,17 +71,16 @@ public File npmrcFile() {
 		return npmrcFile;
 	}
 
-	static File getBuildDirFromNodeModulesDir(File nodeModulesDir) {
-		return nodeModulesDir.getParentFile();
-	}
-
 	public boolean isLayoutPrepared() {
 		if (!nodeModulesDir().isDirectory()) {
 			return false;
 		}
 		if (!packageJsonFile().isFile()) {
 			return false;
 		}
+		if (!packageLockJsonFile.isFile()) {
+			return false;
+		}
 		if (!serveJsFile().isFile()) {
 			return false;
 		}
@@ -82,4 +100,14 @@ public boolean isNodeModulesPrepared() {
 			}
 		});
 	}
+
+	@Override
+	public String toString() {
+		return String.format(
+				"NodeServerLayout[nodeModulesDir=%s, packageJsonFile=%s, serveJsFile=%s, npmrcFile=%s]",
+				this.nodeModulesDir,
+				this.packageJsonFile,
+				this.serveJsFile,
+				this.npmrcFile);
+	}
 }

lib/src/main/java/com/diffplug/spotless/npm/NpmFormatterStepStateBase.java

@@ -56,10 +56,13 @@ protected NpmFormatterStepStateBase(String stepName, NpmConfig npmConfig, NpmFor
 		this.stepName = requireNonNull(stepName);
 		this.npmConfig = requireNonNull(npmConfig);
 		this.locations = locations;
-		this.nodeServerLayout = new NodeServerLayout(locations.buildDir(), stepName);
+		this.nodeServerLayout = new NodeServerLayout(locations.buildDir(), npmConfig.getPackageJsonContent());
 	}
 
 	protected void prepareNodeServerLayout() throws IOException {
+		final long started = System.currentTimeMillis();
+		// maybe introduce trace logger?
+		logger.info("Preparing {} for npm step {}.", this.nodeServerLayout, getClass().getName());
 		NpmResourceHelper.assertDirectoryExists(nodeServerLayout.nodeModulesDir());
 		NpmResourceHelper.writeUtf8StringToFile(nodeServerLayout.packageJsonFile(),
 				this.npmConfig.getPackageJsonContent());
@@ -70,12 +73,14 @@ protected void prepareNodeServerLayout() throws IOException {
 		} else {
 			NpmResourceHelper.deleteFileIfExists(nodeServerLayout.npmrcFile());
 		}
+		logger.info("Prepared {} for npm step {} in {} ms.", this.nodeServerLayout, getClass().getName(), System.currentTimeMillis() - started);
 	}
 
 	protected void prepareNodeServer() throws IOException {
-		FormattedPrinter.SYSOUT.print("running npm install");
+		final long started = System.currentTimeMillis();
+		logger.info("running npm install in {} for npm step {}", this.nodeServerLayout.nodeModulesDir(), getClass().getName());
 		runNpmInstall(nodeServerLayout.nodeModulesDir());
-		FormattedPrinter.SYSOUT.print("npm install finished");
+		logger.info("npm install finished in {} ms in {} for npm step {}", System.currentTimeMillis() - started, this.nodeServerLayout.nodeModulesDir(), getClass().getName());
 	}
 
 	private void runNpmInstall(File npmProjectDir) throws IOException {

lib/src/main/java/com/diffplug/spotless/npm/NpmResourceHelper.java

@@ -21,6 +21,7 @@
 import java.nio.file.Path;
 import java.nio.file.Paths;
 import java.nio.file.StandardCopyOption;
+import java.security.MessageDigest;
 import java.time.Duration;
 import java.util.Arrays;
 import java.util.Objects;
@@ -122,4 +123,20 @@ static File copyFileToDirAtSubpath(File file, File targetDir, String relativePat
 			throw ThrowingEx.asRuntime(e);
 		}
 	}
+
+	static String md5(File file) {
+		return md5(readUtf8StringFromFile(file));
+	}
+
+	static String md5(String fileContent) {
+		MessageDigest md = ThrowingEx.get(() -> MessageDigest.getInstance("MD5"));
+		md.update(fileContent.getBytes(StandardCharsets.UTF_8));
+		byte[] digest = md.digest();
+		// convert byte array digest to hex string
+		StringBuilder sb = new StringBuilder();
+		for (byte b : digest) {
+			sb.append(String.format("%02x", b & 0xff));
+		}
+		return sb.toString();
+	}
 }

lib/src/main/java/com/diffplug/spotless/npm/PrettierFormatterStep.java

@@ -15,6 +15,7 @@
  */
 package com.diffplug.spotless.npm;
 
+import static com.diffplug.spotless.LazyArgLogger.lazy;
 import static java.util.Objects.requireNonNull;
 
 import java.io.File;
@@ -86,7 +87,7 @@ private static class State extends NpmFormatterStepStateBase implements Serializ
 		@Nonnull
 		public FormatterFunc createFormatterFunc() {
 			try {
-				FormattedPrinter.SYSOUT.print("creating formatter function (starting server)");
+				logger.info("creating formatter function (starting server)");
 				ServerProcessInfo prettierRestServer = npmRunServer();
 				PrettierRestService restService = new PrettierRestService(prettierRestServer.getBaseUrl());
 				String prettierConfigOptions = restService.resolveConfig(this.prettierConfig.getPrettierConfigPath(), this.prettierConfig.getOptions());
@@ -97,7 +98,7 @@ public FormatterFunc createFormatterFunc() {
 		}
 
 		private void endServer(PrettierRestService restService, ServerProcessInfo restServer) throws Exception {
-			FormattedPrinter.SYSOUT.print("Closing formatting function (ending server).");
+			logger.info("Closing formatting function (ending server).");
 			try {
 				restService.shutdown();
 			} catch (Throwable t) {
@@ -119,7 +120,7 @@ public PrettierFilePathPassingFormatterFunc(String prettierConfigOptions, Pretti
 
 		@Override
 		public String applyWithFile(String unix, File file) throws Exception {
-			FormattedPrinter.SYSOUT.print("formatting String '" + unix.substring(0, Math.min(50, unix.length())) + "[...]' in file '" + file + "'");
+			logger.info("formatting String '{}[...]' in file '{}'", lazy(() -> unix.substring(0, Math.min(50, unix.length()))), file);
 
 			final String prettierConfigOptionsWithFilepath = assertFilepathInConfigOptions(file);
 			try {

lib/src/main/resources/com/diffplug/spotless/npm/eslint-package.json

@@ -1,5 +1,5 @@
 {
-	"name": "spotless-eslint-formatter-step",
+	"name": "spotless-eslint",
 	"version": "2.0.0",
 	"description": "Spotless formatter step for running eslint as a rest service.",
 	"repository": "https://github.com/diffplug/spotless",

lib/src/main/resources/com/diffplug/spotless/npm/prettier-package.json

@@ -1,5 +1,5 @@
 {
-	"name": "spotless-prettier-formatter-step",
+	"name": "spotless-prettier",
 	"version": "2.0.0",
 	"description": "Spotless formatter step for running prettier as a rest service.",
 	"repository": "https://github.com/diffplug/spotless",

lib/src/main/resources/com/diffplug/spotless/npm/tsfmt-package.json

@@ -1,5 +1,5 @@
 {
-	"name": "spotless-tsfmt-formatter-step",
+	"name": "spotless-tsfmt",
 	"version": "2.0.0",
 	"description": "Spotless formatter step for running tsfmt as a rest service.",
 	"repository": "https://github.com/diffplug/spotless",

plugin-gradle/CHANGES.md

@@ -6,6 +6,8 @@ We adhere to the [keepachangelog](https://keepachangelog.com/en/1.0.0/) format (
 ### Added
 * CleanThat Java Refactorer. ([#1560](https://github.com/diffplug/spotless/pull/1560))
 ### Fixed
+* Allow multiple instances of the same npm-based formatter to be used simultaneously. E.g. use prettier for typescript
+  *and* Java (using the community prettier-plugin-java) without messing up their respective `node_module` dependencies. ([#1565](https://github.com/diffplug/spotless/pull/1565))
 * `ktfmt` default style uses correct continuation indent. ([#1562](https://github.com/diffplug/spotless/pull/1562))
 
 ## [6.14.1] - 2023-02-05

plugin-gradle/src/test/java/com/diffplug/gradle/spotless/PrettierIntegrationTest.java

@@ -183,6 +183,50 @@ void usePhpCommunityPlugin() throws IOException {
 		assertFile("php-example.php").sameAsResource("npm/prettier/plugins/php.clean");
 	}
 
+	/**
+	 * This test is to ensure that we can have multiple prettier instances in one spotless config.
+	 *
+	 * @see <a href="https://github.com/diffplug/spotless/issues/1162">Issue #1162 on github</a>
+	 */
+	@Test
+	void usePhpAndJavaCommunityPlugin() throws IOException {
+		setFile("build.gradle").toLines(
+				"plugins {",
+				"    id 'com.diffplug.spotless'",
+				"}",
+				"repositories { mavenCentral() }",
+				"def prettierConfigPhp = [:]",
+				"prettierConfigPhp['tabWidth'] = 3",
+				"prettierConfigPhp['parser'] = 'php'",
+				"def prettierPackagesPhp = [:]",
+				"prettierPackagesPhp['prettier'] = '2.0.5'",
+				"prettierPackagesPhp['@prettier/plugin-php'] = '0.14.2'",
+				"def prettierConfigJava = [:]",
+				"prettierConfigJava['tabWidth'] = 4",
+				"prettierConfigJava['parser'] = 'java'",
+				"def prettierPackagesJava = [:]",
+				"prettierPackagesJava['prettier'] = '2.0.5'",
+				"prettierPackagesJava['prettier-plugin-java'] = '0.8.0'",
+				"spotless {",
+				"    format 'php', {",
+				"        target 'php-example.php'",
+				"        prettier(prettierPackagesPhp).config(prettierConfigPhp)",
+				"    }",
+				"    java {",
+				"        target 'JavaTest.java'",
+				"        prettier(prettierPackagesJava).config(prettierConfigJava)",
+				"    }",
+				"}");
+		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");
+		final BuildResult spotlessApply2 = gradleRunner().forwardOutput().withArguments("--stacktrace", "--info", "spotlessApply").build();
+		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");
+	}
+
 	@Test
 	void autodetectNpmrcFileConfig() throws IOException {
 		setFile(".npmrc").toLines(