Merge pull request #657 from simschla/bugfix/create-server-resources-on-function-creation

Bugfix/create server resources on function creation

Author: nedtwigg

Diff for: 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

Diff for: 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;
 			}
 		}
 	}

Diff for: 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;
+
+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);
+	}
+}

Diff for: lib/src/main/java/com/diffplug/spotless/npm/NpmFormatterStepStateBase.java

@@ -74,7 +74,9 @@ private NodeServerLayout prepareNodeServer(File buildDir) throws IOException {
 				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;
 	}
 

Diff for: 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);
 		}

Diff for: 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

Diff for: 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");