add ability to use npm and node executables installed by node gradle plugin (#1522)

Author: nedtwigg

CHANGES.md

@@ -23,6 +23,8 @@ We adhere to the [keepachangelog](https://keepachangelog.com/en/1.0.0/) format (
 * ** POTENTIALLY BREAKING** Removed support for KtLint 0.3x and 0.45.2 ([#1475](https://github.com/diffplug/spotless/pull/1475))
   * `KtLint` does not maintain a stable API - before this PR, we supported every breaking change in the API since 2019.
   * From now on, we will support no more than 2 breaking changes at a time.
+* NpmFormatterStepStateBase delays `npm install` call until the formatter is first used. This enables better integration
+  with `gradle-node-plugin`. ([#1522](https://github.com/diffplug/spotless/pull/1522))
 
 ## [2.32.0] - 2023-01-13
 ### Added

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

@@ -39,6 +39,8 @@
 import com.diffplug.spotless.ThrowingEx;
 import com.diffplug.spotless.npm.EslintRestService.FormatOption;
 
+import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
+
 public class EslintFormatterStep {
 
 	private static final Logger logger = LoggerFactory.getLogger(EslintFormatterStep.class);
@@ -81,7 +83,10 @@ public static FormatterStep create(Map<String, String> devDependencies, Provisio
 	private static class State extends NpmFormatterStepStateBase implements Serializable {
 
 		private static final long serialVersionUID = -539537027004745812L;
-		private final EslintConfig eslintConfig;
+		private final EslintConfig origEslintConfig;
+
+		@SuppressFBWarnings("SE_TRANSIENT_FIELD_NOT_RESTORED")
+		private transient EslintConfig eslintConfigInUse;
 
 		State(String stepName, Map<String, String> devDependencies, File projectDir, File buildDir, NpmPathResolver npmPathResolver, EslintConfig eslintConfig) throws IOException {
 			super(stepName,
@@ -97,21 +102,23 @@ private static class State extends NpmFormatterStepStateBase implements Serializ
 					new NpmFormatterStepLocations(
 							projectDir,
 							buildDir,
-							npmPathResolver.resolveNpmExecutable(),
-							npmPathResolver.resolveNodeExecutable()));
-			this.eslintConfig = localCopyFiles(requireNonNull(eslintConfig));
+							npmPathResolver::resolveNpmExecutable,
+							npmPathResolver::resolveNodeExecutable));
+			this.origEslintConfig = requireNonNull(eslintConfig.verify());
+			this.eslintConfigInUse = eslintConfig;
 		}
 
-		private EslintConfig localCopyFiles(EslintConfig orig) {
-			if (orig.getEslintConfigPath() == null) {
-				return orig.verify();
+		@Override
+		protected void prepareNodeServerLayout() throws IOException {
+			super.prepareNodeServerLayout();
+			if (origEslintConfig.getEslintConfigPath() != null) {
+				// 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());
+				File configFileCopy = NpmResourceHelper.copyFileToDir(origEslintConfig.getEslintConfigPath(), nodeServerLayout.nodeModulesDir());
+				this.eslintConfigInUse = this.origEslintConfig.withEslintConfigPath(configFileCopy).verify();
 			}
-			// 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", orig.getEslintConfigPath(), nodeModulesDir);
-			File configFileCopy = NpmResourceHelper.copyFileToDir(orig.getEslintConfigPath(), nodeModulesDir);
-			return orig.withEslintConfigPath(configFileCopy).verify();
 		}
 
 		@Override
@@ -121,7 +128,7 @@ public FormatterFunc createFormatterFunc() {
 				FormattedPrinter.SYSOUT.print("creating formatter function (starting server)");
 				ServerProcessInfo eslintRestServer = npmRunServer();
 				EslintRestService restService = new EslintRestService(eslintRestServer.getBaseUrl());
-				return Closeable.ofDangerous(() -> endServer(restService, eslintRestServer), new EslintFilePathPassingFormatterFunc(locations.projectDir(), nodeModulesDir, eslintConfig, restService));
+				return Closeable.ofDangerous(() -> endServer(restService, eslintRestServer), new EslintFilePathPassingFormatterFunc(locations.projectDir(), nodeServerLayout.nodeModulesDir(), eslintConfigInUse, restService));
 			} catch (IOException e) {
 				throw ThrowingEx.asRuntime(e);
 			}

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

@@ -1,5 +1,5 @@
 /*
- * Copyright 2020-2021 DiffPlug
+ * Copyright 2020-2023 DiffPlug
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -16,6 +16,11 @@
 package com.diffplug.spotless.npm;
 
 import java.io.File;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.util.stream.Stream;
+
+import com.diffplug.spotless.ThrowingEx;
 
 class NodeServerLayout {
 
@@ -50,4 +55,31 @@ public File npmrcFile() {
 	static File getBuildDirFromNodeModulesDir(File nodeModulesDir) {
 		return nodeModulesDir.getParentFile();
 	}
+
+	public boolean isLayoutPrepared() {
+		if (!nodeModulesDir().isDirectory()) {
+			return false;
+		}
+		if (!packageJsonFile().isFile()) {
+			return false;
+		}
+		if (!serveJsFile().isFile()) {
+			return false;
+		}
+		// npmrc is optional, so must not be checked here
+		return true;
+	}
+
+	public boolean isNodeModulesPrepared() {
+		Path nodeModulesInstallDirPath = new File(nodeModulesDir(), "node_modules").toPath();
+		if (!Files.isDirectory(nodeModulesInstallDirPath)) {
+			return false;
+		}
+		// check if it is NOT empty
+		return ThrowingEx.get(() -> {
+			try (Stream<Path> entries = Files.list(nodeModulesInstallDirPath)) {
+				return entries.findFirst().isPresent();
+			}
+		});
+	}
 }

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

@@ -19,6 +19,7 @@
 
 import java.io.File;
 import java.io.Serializable;
+import java.util.function.Supplier;
 
 import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
 
@@ -32,12 +33,12 @@ class NpmFormatterStepLocations implements Serializable {
 	private final transient File buildDir;
 
 	@SuppressFBWarnings("SE_TRANSIENT_FIELD_NOT_RESTORED")
-	private final transient File npmExecutable;
+	private final transient Supplier<File> npmExecutable;
 
 	@SuppressFBWarnings("SE_TRANSIENT_FIELD_NOT_RESTORED")
-	private final transient File nodeExecutable;
+	private final transient Supplier<File> nodeExecutable;
 
-	public NpmFormatterStepLocations(File projectDir, File buildDir, File npmExecutable, File nodeExecutable) {
+	public NpmFormatterStepLocations(File projectDir, File buildDir, Supplier<File> npmExecutable, Supplier<File> nodeExecutable) {
 		this.projectDir = requireNonNull(projectDir);
 		this.buildDir = requireNonNull(buildDir);
 		this.npmExecutable = requireNonNull(npmExecutable);
@@ -53,10 +54,10 @@ public File buildDir() {
 	}
 
 	public File npmExecutable() {
-		return npmExecutable;
+		return npmExecutable.get();
 	}
 
 	public File nodeExecutable() {
-		return nodeExecutable;
+		return nodeExecutable.get();
 	}
 }

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

@@ -31,8 +31,9 @@
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
-import com.diffplug.spotless.FileSignature;
 import com.diffplug.spotless.FormatterFunc;
+import com.diffplug.spotless.ProcessRunner.LongRunningProcess;
+import com.diffplug.spotless.ThrowingEx;
 
 import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
 
@@ -42,11 +43,8 @@ abstract class NpmFormatterStepStateBase implements Serializable {
 
 	private static final long serialVersionUID = 1460749955865959948L;
 
-	@SuppressWarnings("unused")
-	private final FileSignature packageJsonSignature;
-
 	@SuppressFBWarnings("SE_TRANSIENT_FIELD_NOT_RESTORED")
-	public final transient File nodeModulesDir;
+	protected final transient NodeServerLayout nodeServerLayout;
 
 	public final NpmFormatterStepLocations locations;
 
@@ -58,45 +56,61 @@ protected NpmFormatterStepStateBase(String stepName, NpmConfig npmConfig, NpmFor
 		this.stepName = requireNonNull(stepName);
 		this.npmConfig = requireNonNull(npmConfig);
 		this.locations = locations;
-		NodeServerLayout layout = prepareNodeServer(locations.buildDir());
-		this.nodeModulesDir = layout.nodeModulesDir();
-		this.packageJsonSignature = FileSignature.signAsList(layout.packageJsonFile());
+		this.nodeServerLayout = new NodeServerLayout(locations.buildDir(), stepName);
 	}
 
-	private NodeServerLayout prepareNodeServer(File buildDir) throws IOException {
-		NodeServerLayout layout = new NodeServerLayout(buildDir, stepName);
-		NpmResourceHelper.assertDirectoryExists(layout.nodeModulesDir());
-		NpmResourceHelper.writeUtf8StringToFile(layout.packageJsonFile(),
+	protected void prepareNodeServerLayout() throws IOException {
+		NpmResourceHelper.assertDirectoryExists(nodeServerLayout.nodeModulesDir());
+		NpmResourceHelper.writeUtf8StringToFile(nodeServerLayout.packageJsonFile(),
 				this.npmConfig.getPackageJsonContent());
 		NpmResourceHelper
-				.writeUtf8StringToFile(layout.serveJsFile(), this.npmConfig.getServeScriptContent());
+				.writeUtf8StringToFile(nodeServerLayout.serveJsFile(), this.npmConfig.getServeScriptContent());
 		if (this.npmConfig.getNpmrcContent() != null) {
-			NpmResourceHelper.writeUtf8StringToFile(layout.npmrcFile(), this.npmConfig.getNpmrcContent());
+			NpmResourceHelper.writeUtf8StringToFile(nodeServerLayout.npmrcFile(), this.npmConfig.getNpmrcContent());
 		} else {
-			NpmResourceHelper.deleteFileIfExists(layout.npmrcFile());
+			NpmResourceHelper.deleteFileIfExists(nodeServerLayout.npmrcFile());
 		}
+	}
+
+	protected void prepareNodeServer() throws IOException {
 		FormattedPrinter.SYSOUT.print("running npm install");
-		runNpmInstall(layout.nodeModulesDir());
+		runNpmInstall(nodeServerLayout.nodeModulesDir());
 		FormattedPrinter.SYSOUT.print("npm install finished");
-		return layout;
 	}
 
 	private void runNpmInstall(File npmProjectDir) throws IOException {
 		new NpmProcess(npmProjectDir, this.locations.npmExecutable(), this.locations.nodeExecutable()).install();
 	}
 
-	protected ServerProcessInfo npmRunServer() throws ServerStartException, IOException {
-		if (!this.nodeModulesDir.exists()) {
-			prepareNodeServer(NodeServerLayout.getBuildDirFromNodeModulesDir(this.nodeModulesDir));
+	protected void assertNodeServerDirReady() throws IOException {
+		if (needsPrepareNodeServerLayout()) {
+			// reinstall if missing
+			prepareNodeServerLayout();
+		}
+		if (needsPrepareNodeServer()) {
+			// run npm install if node_modules is missing
+			prepareNodeServer();
 		}
+	}
 
+	protected boolean needsPrepareNodeServer() {
+		return !this.nodeServerLayout.isNodeModulesPrepared();
+	}
+
+	protected boolean needsPrepareNodeServerLayout() {
+		return !this.nodeServerLayout.isLayoutPrepared();
+	}
+
+	protected ServerProcessInfo npmRunServer() throws ServerStartException, IOException {
+		assertNodeServerDirReady();
+		LongRunningProcess server = null;
 		try {
 			// 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.nodeModulesDir, "server.port");
+			final File serverPortFile = new File(this.nodeServerLayout.nodeModulesDir(), "server.port");
 			NpmResourceHelper.deleteFileIfExists(serverPortFile);
 			// start the http server in node
-			Process server = new NpmProcess(this.nodeModulesDir, this.locations.npmExecutable(), this.locations.nodeExecutable()).start();
+			server = new NpmProcess(this.nodeServerLayout.nodeModulesDir(), this.locations.npmExecutable(), this.locations.nodeExecutable()).start();
 
 			// await the readiness of the http server - wait for at most 60 seconds
 			try {
@@ -117,7 +131,7 @@ protected ServerProcessInfo npmRunServer() throws ServerStartException, IOExcept
 			String serverPort = NpmResourceHelper.readUtf8StringFromFile(serverPortFile).trim();
 			return new ServerProcessInfo(server, serverPort, serverPortFile);
 		} catch (IOException | TimeoutException e) {
-			throw new ServerStartException(e);
+			throw new ServerStartException("Starting server failed." + (server != null ? "\n\nProcess result:\n" + ThrowingEx.get(server::result) : ""), e);
 		}
 	}
 
@@ -186,7 +200,7 @@ public void close() throws Exception {
 	protected static class ServerStartException extends RuntimeException {
 		private static final long serialVersionUID = -8803977379866483002L;
 
-		public ServerStartException(Throwable cause) {
+		public ServerStartException(String message, Throwable cause) {
 			super(cause);
 		}
 	}

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

@@ -77,8 +77,8 @@ private static class State extends NpmFormatterStepStateBase implements Serializ
 					new NpmFormatterStepLocations(
 							projectDir,
 							buildDir,
-							npmPathResolver.resolveNpmExecutable(),
-							npmPathResolver.resolveNodeExecutable()));
+							npmPathResolver::resolveNpmExecutable,
+							npmPathResolver::resolveNodeExecutable));
 			this.prettierConfig = requireNonNull(prettierConfig);
 		}
 

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

@@ -83,8 +83,8 @@ public State(String stepName, Map<String, String> versions, File projectDir, Fil
 					new NpmFormatterStepLocations(
 							projectDir,
 							buildDir,
-							npmPathResolver.resolveNpmExecutable(),
-							npmPathResolver.resolveNodeExecutable()));
+							npmPathResolver::resolveNpmExecutable,
+							npmPathResolver::resolveNodeExecutable));
 			this.buildDir = requireNonNull(buildDir);
 			this.configFile = configFile;
 			this.inlineTsFmtSettings = inlineTsFmtSettings == null ? new TreeMap<>() : new TreeMap<>(inlineTsFmtSettings);

plugin-gradle/CHANGES.md

@@ -15,6 +15,9 @@ We adhere to the [keepachangelog](https://keepachangelog.com/en/1.0.0/) format (
 * **POTENTIALLY BREAKING** Removed support for KtLint 0.3x and 0.45.2 ([#1475](https://github.com/diffplug/spotless/pull/1475))
   * `KtLint` does not maintain a stable API - before this PR, we supported every breaking change in the API since 2019.
   * From now on, we will support no more than 2 breaking changes at a time.
+* `npm`-based formatters `ESLint`, `prettier` and `tsfmt` delay their `npm install` call until the formatters are first
+  used. For gradle this effectively moves the `npm install` call out of the configuration phase and as such enables
+  better integration with `gradle-node-plugin`. ([#1522](https://github.com/diffplug/spotless/pull/1522))
 
 ## [6.13.0] - 2023-01-14
 ### Added

plugin-gradle/README.md

@@ -907,6 +907,13 @@ spotless {
 If you provide both `npmExecutable` and `nodeExecutable`, spotless will use these paths. If you specify only one of the
 two, spotless will assume the other one is in the same directory.
 
+If you use the `gradle-node-plugin` ([github](https://github.com/node-gradle/gradle-node-plugin)), it is possible to use the
+node- and npm-binaries dynamically installed by this plugin. See
+[this](https://github.com/diffplug/spotless/blob/main/plugin-gradle/src/test/resources/com/diffplug/gradle/spotless/NpmTestsWithoutNpmInstallationTest_gradle_node_plugin_example_1.gradle)
+or [this](https://github.com/diffplug/spotless/blob/main/plugin-gradle/src/test/resources/com/diffplug/gradle/spotless/NpmTestsWithoutNpmInstallationTest_gradle_node_plugin_example_2.gradle) example.
+
+```gradle
+
 ### `.npmrc` detection
 
 Spotless picks up npm configuration stored in a `.npmrc` file either in the project directory or in your user home.

plugin-gradle/src/main/java/com/diffplug/gradle/spotless/TypescriptExtension.java

@@ -166,7 +166,7 @@ protected FormatterStep createStep() {
 
 		private void fixParserToTypescript() {
 			if (this.prettierConfig == null) {
-				this.prettierConfig = Collections.singletonMap("parser", "typescript");
+				this.prettierConfig = new TreeMap<>(Collections.singletonMap("parser", "typescript"));
 			} else {
 				final Object replaced = this.prettierConfig.put("parser", "typescript");
 				if (replaced != null) {