Bugfix/create server resources on function creation

CHANGES.md

@@ -10,7 +10,10 @@ This document is intended for Spotless developers.
 We adhere to the [keepachangelog](https://keepachangelog.com/en/1.0.0/) format (starting after version `1.27.0`).
 
 ## [Unreleased]
-* Bump default ktfmt from 0.15 to 0.16, and remove duplicated logic for the --dropbox-style option ([#642](https://github.com/diffplug/spotless/pull/648))
+### Fixed
+* `FormatterFunc.Closeable` had a "use after free" bug which caused errors in the npm-based formatters (e.g. prettier) if `spotlessCheck` was called on dirty files. ([#651](https://github.com/diffplug/spotless/issues/651))
+### Changed
+* Bump default ktfmt from `0.15` to `0.16`, and remove duplicated logic for the `--dropbox-style` option ([#642](https://github.com/diffplug/spotless/pull/648))
 
 ## [2.2.0] - 2020-07-13
 ### Added

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

@@ -81,6 +81,7 @@ protected String format(State state, String rawUnix, File file) throws Exception
 		void cleanupFormatterFunc() {
 			if (formatter instanceof FormatterFunc.Closeable) {
 				((FormatterFunc.Closeable) formatter).close();
+				formatter = null;
 			}
 		}
 	}

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

@@ -0,0 +1,42 @@
+/*
+ * Copyright 2020 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 static java.util.Objects.requireNonNull;
+
+import java.io.PrintStream;
+import java.time.LocalDateTime;
+
+public enum FormattedPrinter {
+	SYSOUT(System.out);
+
+	private static final boolean enabled = false;
+
+	private final PrintStream printStream;
+
+	FormattedPrinter(PrintStream printStream) {
+		this.printStream = requireNonNull(printStream);
+	}
+
+	public void print(String msg, Object... paramsForStringFormat) {
+		if (!enabled) {
+			return;
+		}
+		String formatted = String.format(msg, paramsForStringFormat);
+		String prefixed = String.format("[%s] %s", LocalDateTime.now().toString(), formatted);
+		this.printStream.println(prefixed);
+	}
+}

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

@@ -32,7 +32,6 @@
 
 import javax.annotation.Nullable;
 
-import com.diffplug.spotless.FileSignature;
 import com.diffplug.spotless.FormatterFunc;
 
 import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
@@ -43,11 +42,7 @@ 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;
+	private final File buildDir;
 
 	@SuppressFBWarnings("SE_TRANSIENT_FIELD_NOT_RESTORED")
 	private final transient File npmExecutable;
@@ -60,21 +55,20 @@ protected NpmFormatterStepStateBase(String stepName, NpmConfig npmConfig, File b
 			@Nullable File npm) throws IOException {
 		this.stepName = requireNonNull(stepName);
 		this.npmConfig = requireNonNull(npmConfig);
+		this.buildDir = buildDir;
 		this.npmExecutable = resolveNpm(npm);
-
-		NodeServerLayout layout = prepareNodeServer(buildDir);
-		this.nodeModulesDir = layout.nodeModulesDir();
-		this.packageJsonSignature = FileSignature.signAsList(layout.packageJsonFile());
 	}
 
-	private NodeServerLayout prepareNodeServer(File buildDir) throws IOException {
+	private NodeServerLayout prepareNodeServer() throws IOException {
 		NodeServerLayout layout = new NodeServerLayout(buildDir, stepName);
 		NpmResourceHelper.assertDirectoryExists(layout.nodeModulesDir());
 		NpmResourceHelper.writeUtf8StringToFile(layout.packageJsonFile(),
 				this.npmConfig.getPackageJsonContent());
 		NpmResourceHelper
 				.writeUtf8StringToFile(layout.serveJsFile(), this.npmConfig.getServeScriptContent());
+		FormattedPrinter.SYSOUT.print("running npm install");
 		runNpmInstall(layout.nodeModulesDir());
+		FormattedPrinter.SYSOUT.print("npm install finished");
 		return layout;
 	}
 
@@ -84,12 +78,14 @@ private void runNpmInstall(File npmProjectDir) throws IOException {
 
 	protected ServerProcessInfo npmRunServer() throws ServerStartException {
 		try {
+			FormattedPrinter.SYSOUT.print("preparing node server");
+			final NodeServerLayout nodeServerLayout = prepareNodeServer();
 			// 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(nodeServerLayout.nodeModulesDir(), "server.port");
 			NpmResourceHelper.deleteFileIfExists(serverPortFile);
 			// start the http server in node
-			Process server = new NpmProcess(this.nodeModulesDir, this.npmExecutable).start();
+			Process server = new NpmProcess(nodeServerLayout.nodeModulesDir(), this.npmExecutable).start();
 
 			// await the readiness of the http server - wait for at most 60 seconds
 			try {

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

@@ -80,6 +80,7 @@ private static class State extends NpmFormatterStepStateBase implements Serializ
 		@Nonnull
 		public FormatterFunc createFormatterFunc() {
 			try {
+				FormattedPrinter.SYSOUT.print("creating formatter function (starting server)");
 				ServerProcessInfo prettierRestServer = npmRunServer();
 				PrettierRestService restService = new PrettierRestService(prettierRestServer.getBaseUrl());
 				String prettierConfigOptions = restService.resolveConfig(this.prettierConfig.getPrettierConfigPath(), this.prettierConfig.getOptions());
@@ -90,6 +91,7 @@ public FormatterFunc createFormatterFunc() {
 		}
 
 		private void endServer(PrettierRestService restService, ServerProcessInfo restServer) throws Exception {
+			FormattedPrinter.SYSOUT.print("Closing formatting function (ending server).");
 			try {
 				restService.shutdown();
 			} catch (Throwable t) {
@@ -111,6 +113,8 @@ public PrettierFilePathPassingFormatterFunc(String prettierConfigOptions, Pretti
 
 		@Override
 		public String applyWithFile(String unix, File file) throws Exception {
+			FormattedPrinter.SYSOUT.print("formatting String '" + unix.substring(0, 50) + "[...]' in file '" + file + "'");
+
 			final String prettierConfigOptionsWithFilepath = assertFilepathInConfigOptions(file);
 			return restService.format(unix, prettierConfigOptionsWithFilepath);
 		}

plugin-gradle/CHANGES.md

@@ -3,8 +3,11 @@
 We adhere to the [keepachangelog](https://keepachangelog.com/en/1.0.0/) format (starting after version `3.27.0`).
 
 ## [Unreleased]
+### Fixed
 * Depending on the file system, executing `gradle spotlessApply` might change permission on the changed files from `644` to `755`; fixes ([#656](https://github.com/diffplug/spotless/pull/656))
-* Bump default ktfmt from 0.15 to 0.16, and remove duplicated logic for the --dropbox-style option ([#642](https://github.com/diffplug/spotless/pull/648))
+* When using the `prettier` or `tsfmt` steps, if any files were dirty then `spotlessCheck` would fail with `java.net.ConnectException: Connection refused` rather than the proper error message ([#651](https://github.com/diffplug/spotless/issues/651)).
+### Changed
+* Bump default ktfmt from `0.15` to `0.16`, and remove duplicated logic for the `--dropbox-style` option ([#642](https://github.com/diffplug/spotless/pull/648))
 
 ## [5.1.0] - 2020-07-13
 ### Added

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

@@ -48,6 +48,30 @@ public void useInlineConfig() throws IOException {
 		assertFile("test.ts").sameAsResource("npm/prettier/config/typescript.configfile.clean");
 	}
 
+	@Test
+	public void verifyCleanSpotlessCheckWorks() throws IOException {
+		setFile("build.gradle").toLines(
+				"buildscript { repositories { mavenCentral() } }",
+				"plugins {",
+				"    id 'com.diffplug.spotless'",
+				"}",
+				"def prettierConfig = [:]",
+				"prettierConfig['printWidth'] = 50",
+				"prettierConfig['parser'] = 'typescript'",
+				"spotless {",
+				"    format 'mytypescript', {",
+				"        target 'test.ts'",
+				"        prettier().config(prettierConfig)",
+				"    }",
+				"}");
+		setFile("test.ts").toResource("npm/prettier/config/typescript.dirty");
+		BuildResult spotlessCheckFailsGracefully = gradleRunner().withArguments("--stacktrace", "clean", "spotlessCheck").buildAndFail();
+		Assertions.assertThat(spotlessCheckFailsGracefully.getOutput()).contains("> The following files had format violations:");
+
+		gradleRunner().withArguments("--stacktrace", "clean", "spotlessApply").build();
+		gradleRunner().withArguments("--stacktrace", "clean", "spotlessCheck").build();
+	}
+
 	@Test
 	public void useFileConfig() throws IOException {
 		setFile(".prettierrc.yml").toResource("npm/prettier/config/.prettierrc.yml");