Merge pull request #279 from diffplug/eclipse_classloader_cache_extension

SpotlessCache extension for Eclipse formatters

Author: nedtwigg

lib-extra/src/main/java/com/diffplug/spotless/extra/EclipseBasedStepBuilder.java

@@ -41,6 +41,7 @@
  */
 public class EclipseBasedStepBuilder {
 	private final String formatterName;
+	private final String formatterStepExt;
 	private final ThrowingEx.Function<State, FormatterFunc> stateToFormatter;
 	private final Provisioner jarProvisioner;
 
@@ -63,14 +64,20 @@ public class EclipseBasedStepBuilder {
 
 	/** Initialize valid default configuration, taking latest version */
 	public EclipseBasedStepBuilder(String formatterName, Provisioner jarProvisioner, ThrowingEx.Function<State, FormatterFunc> stateToFormatter) {
+		this(formatterName, "", jarProvisioner, stateToFormatter);
+	}
+
+	/** Initialize valid default configuration, taking latest version */
+	public EclipseBasedStepBuilder(String formatterName, String formatterStepExt, Provisioner jarProvisioner, ThrowingEx.Function<State, FormatterFunc> stateToFormatter) {
 		this.formatterName = Objects.requireNonNull(formatterName, "formatterName");
+		this.formatterStepExt = Objects.requireNonNull(formatterStepExt, "formatterStepExt");
 		this.jarProvisioner = Objects.requireNonNull(jarProvisioner, "jarProvisioner");
 		this.stateToFormatter = Objects.requireNonNull(stateToFormatter, "stateToFormatter");
 	}
 
 	/** Returns the FormatterStep (whose state will be calculated lazily). */
 	public FormatterStep build() {
-		return FormatterStep.createLazy(formatterName, this::get, stateToFormatter);
+		return FormatterStep.createLazy(formatterName + formatterStepExt, this::get, stateToFormatter);
 	}
 
 	/** Set dependencies for the corresponding Eclipse version */
@@ -123,6 +130,7 @@ EclipseBasedStepBuilder.State get() throws IOException {
 		 * Hence a lazy construction is not required.
 		 */
 		return new State(
+				formatterStepExt,
 				jarProvisioner,
 				dependencies,
 				settingsFiles);
@@ -137,12 +145,16 @@ public static class State implements Serializable {
 		private static final long serialVersionUID = 1L;
 
 		private final JarState jarState;
+		//The formatterStepExt assures that different class loaders are used for different step types
+		@SuppressWarnings("unused")
+		private final String formatterStepExt;
 		private final FileSignature settingsFiles;
 
 		/** State constructor expects that all passed items are not modified afterwards */
-		protected State(Provisioner jarProvisioner, List<String> dependencies, Iterable<File> settingsFiles) throws IOException {
+		protected State(String formatterStepExt, Provisioner jarProvisioner, List<String> dependencies, Iterable<File> settingsFiles) throws IOException {
 			this.jarState = JarState.from(dependencies, jarProvisioner);
 			this.settingsFiles = FileSignature.signAsList(settingsFiles);
+			this.formatterStepExt = formatterStepExt;
 		}
 
 		/** Get formatter preferences */
@@ -158,10 +170,18 @@ public Optional<String> getMavenCoordinate(String prefix) {
 					.filter(coordinate -> coordinate.startsWith(prefix)).findFirst();
 		}
 
-		/** Load class based on the given configuration of JAR provider and Maven coordinates. */
+		/**
+		 * Load class based on the given configuration of JAR provider and Maven coordinates.
+		 * Different class loader instances are provided in the following scenarios:
+		 * <ol>
+		 * <li>The JARs ({@link #jarState}) have changes (this should only occur during development)</li>
+		 * <li>Different configurations ({@link #settingsFiles}) are used for different sub-projects</li>
+		 * <li>The same Eclipse step implementation provides different formatter types ({@link #formatterStepExt})</li>
+		 * </ol>
+		 */
 		public Class<?> loadClass(String name) {
 			try {
-				return jarState.getClassLoader().loadClass(name);
+				return jarState.getClassLoader(this).loadClass(name);
 			} catch (ClassNotFoundException e) {
 				throw Errors.asRuntime(e);
 			}

lib-extra/src/main/java/com/diffplug/spotless/extra/wtp/WtpEclipseFormatterStep.java

@@ -0,0 +1,80 @@
+/*
+ * Copyright 2016 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.extra.wtp;
+
+import java.lang.reflect.InvocationTargetException;
+import java.lang.reflect.Method;
+import java.util.Properties;
+
+import com.diffplug.spotless.FormatterFunc;
+import com.diffplug.spotless.Provisioner;
+import com.diffplug.spotless.extra.EclipseBasedStepBuilder;
+
+/** Formatter step which calls out to the Groovy-Eclipse formatter. */
+public final class WtpEclipseFormatterStep {
+	// prevent direct instantiation
+	private WtpEclipseFormatterStep() {}
+
+	private static final String NAME = "eclipse wtp formatters";
+	private static final String FORMATTER_PACKAGE = "com.diffplug.spotless.extra.eclipse.wtp.";
+	private static final String DEFAULT_VERSION = "4.7.3a";
+	private static final String FORMATTER_METHOD = "format";
+
+	public static String defaultVersion() {
+		return DEFAULT_VERSION;
+	}
+
+	/** Provides default configuration for CSSformatter */
+	public static EclipseBasedStepBuilder createCssBuilder(Provisioner provisioner) {
+		return new EclipseBasedStepBuilder(NAME, " - css", provisioner, state -> apply("EclipseCssFormatterStepImpl", state));
+	}
+
+	/** Provides default configuration for HTML formatter */
+	public static EclipseBasedStepBuilder createHtmlBuilder(Provisioner provisioner) {
+		return new EclipseBasedStepBuilder(NAME, " - html", provisioner, state -> apply("EclipseHtmlFormatterStepImpl", state));
+	}
+
+	/** Provides default configuration for Java Script formatter */
+	public static EclipseBasedStepBuilder createJsBuilder(Provisioner provisioner) {
+		return new EclipseBasedStepBuilder(NAME, " - js", provisioner, state -> apply("EclipseJsFormatterStepImpl", state));
+	}
+
+	/** Provides default configuration for JSON formatter */
+	public static EclipseBasedStepBuilder createJsonBuilder(Provisioner provisioner) {
+		return new EclipseBasedStepBuilder(NAME, " - json", provisioner, state -> apply("EclipseJsonFormatterStepImpl", state));
+	}
+
+	/** Provides default configuration for XML formatter */
+	public static EclipseBasedStepBuilder createXmlBuilder(Provisioner provisioner) {
+		return new EclipseBasedStepBuilder(NAME, " - xml", provisioner, state -> apply("EclipseXmlFormatterStepImpl", state));
+	}
+
+	private static FormatterFunc apply(String className, EclipseBasedStepBuilder.State state) throws Exception {
+		Class<?> formatterClazz = state.loadClass(FORMATTER_PACKAGE + className);
+		Object formatter = formatterClazz.getConstructor(Properties.class).newInstance(state.getPreferences());
+		Method method = formatterClazz.getMethod(FORMATTER_METHOD, String.class);
+		return input -> {
+			try {
+				return (String) method.invoke(formatter, input);
+			} catch (InvocationTargetException exceptionWrapper) {
+				Throwable throwable = exceptionWrapper.getTargetException();
+				Exception exception = (throwable instanceof Exception) ? (Exception) throwable : null;
+				throw (null == exception) ? exceptionWrapper : exception;
+			}
+		};
+	}
+
+}

lib-extra/src/main/java/com/diffplug/spotless/extra/wtp/package-info.java

@@ -0,0 +1,7 @@
+@ParametersAreNonnullByDefault
+@ReturnValuesAreNonnullByDefault
+package com.diffplug.spotless.extra.wtp;
+
+import javax.annotation.ParametersAreNonnullByDefault;
+
+import com.diffplug.spotless.annotations.ReturnValuesAreNonnullByDefault;

lib-extra/src/main/resources/com/diffplug/spotless/extra/eclipse_wtp_formatters/v4.7.3a.lockfile

@@ -0,0 +1,27 @@
+# Spotless formatter based on Eclipse-WTP version 3.9.5 (see https://www.eclipse.org/webtools/)
+com.diffplug.spotless:spotless-eclipse-wtp:3.9.5
+com.diffplug.spotless:spotless-eclipse-base:3.0.0
+com.google.code.findbugs:annotations:3.0.0
+com.google.code.findbugs:jsr305:3.0.0
+com.ibm.icu:icu4j:61.1
+org.eclipse.emf:org.eclipse.emf.common:2.12.0
+org.eclipse.emf:org.eclipse.emf.ecore:2.12.0
+org.eclipse.platform:org.eclipse.core.commands:3.9.100
+org.eclipse.platform:org.eclipse.core.contenttype:3.7.0
+org.eclipse.platform:org.eclipse.core.filebuffers:3.6.200
+org.eclipse.platform:org.eclipse.core.filesystem:1.7.100
+org.eclipse.platform:org.eclipse.core.jobs:3.10.0
+org.eclipse.platform:org.eclipse.core.resources:3.13.0
+org.eclipse.platform:org.eclipse.core.runtime:3.14.0
+org.eclipse.platform:org.eclipse.equinox.app:1.3.500
+org.eclipse.platform:org.eclipse.equinox.common:3.10.0
+org.eclipse.platform:org.eclipse.equinox.preferences:3.7.100
+org.eclipse.platform:org.eclipse.equinox.registry:3.8.0
+#Spotless currently loads all transitive dependencies.
+#jface requires platform specific JARs (not used by formatter), which are not hosted via M2.
+#org.eclipse.platform:org.eclipse.jface.text:3.13.0
+#org.eclipse.platform:org.eclipse.jface:3.14.0
+org.eclipse.platform:org.eclipse.osgi.services:3.7.0
+org.eclipse.platform:org.eclipse.osgi:3.13.0
+org.eclipse.platform:org.eclipse.text:3.6.300
+org.eclipse.xsd:org.eclipse.xsd:2.12.0

lib-extra/src/test/java/com/diffplug/spotless/extra/wtp/EclipseWtpFormatterStepTest.java

@@ -0,0 +1,141 @@
+/*
+ * Copyright 2016 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.extra.wtp;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+import java.io.File;
+import java.io.FileOutputStream;
+import java.io.IOException;
+import java.io.OutputStream;
+import java.util.Arrays;
+import java.util.Properties;
+import java.util.function.Consumer;
+import java.util.function.Function;
+import java.util.stream.Collectors;
+
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+import org.junit.runners.Parameterized.Parameter;
+import org.junit.runners.Parameterized.Parameters;
+
+import com.diffplug.spotless.FormatterStep;
+import com.diffplug.spotless.Provisioner;
+import com.diffplug.spotless.TestProvisioner;
+import com.diffplug.spotless.extra.EclipseBasedStepBuilder;
+import com.diffplug.spotless.extra.eclipse.EclipseCommonTests;
+
+@RunWith(value = Parameterized.class)
+public class EclipseWtpFormatterStepTest extends EclipseCommonTests {
+
+	private enum WTP {
+		// @formatter:off
+		CSS(	"body {\na: v;   b:   \nv;\n}  \n",
+				"body {\n\ta: v;\n\tb: v;\n}",
+				WtpEclipseFormatterStep::createCssBuilder),
+		HTML(	"<!DOCTYPE html> <html>\t<head> <meta   charset=\"UTF-8\"></head>\n</html>  ",
+				"<!DOCTYPE html>\n<html>\n<head>\n<meta charset=\"UTF-8\">\n</head>\n</html>\n",
+				WtpEclipseFormatterStep::createHtmlBuilder),
+		JS(		"function f(  )   {\na.b(1,\n2);}",
+				"function f() {\n    a.b(1, 2);\n}",
+				WtpEclipseFormatterStep::createJsBuilder),
+		JSON(	"{\"a\": \"b\",	\"c\":   { \"d\": \"e\",\"f\": \"g\"}}",
+				"{\n\t\"a\": \"b\",\n\t\"c\": {\n\t\t\"d\": \"e\",\n\t\t\"f\": \"g\"\n\t}\n}",
+				WtpEclipseFormatterStep::createJsonBuilder),
+		XML(	"<a><b>   c</b></a>", "<a>\n\t<b> c</b>\n</a>",
+				WtpEclipseFormatterStep::createXmlBuilder);
+		// @formatter:on
+
+		public final String input;
+		public final String expectation;
+		public final Function<Provisioner, EclipseBasedStepBuilder> builderMethod;
+
+		private WTP(String input, final String expectation, Function<Provisioner, EclipseBasedStepBuilder> builderMethod) {
+			this.input = input;
+			this.expectation = expectation;
+			this.builderMethod = builderMethod;
+		}
+	}
+
+	@Parameters(name = "{0}")
+	public static Iterable<WTP> data() {
+		//TODO: XML is excluded. How to provide base location will be addressed by separate PR.
+		return Arrays.asList(WTP.values()).stream().filter(e -> e != WTP.XML).collect(Collectors.toList());
+	}
+
+	@Parameter(0)
+	public WTP wtp;
+
+	@Override
+	protected String[] getSupportedVersions() {
+		return new String[]{"4.7.3a"};
+	}
+
+	@Override
+	protected String getTestInput(String version) {
+		return wtp.input;
+	}
+
+	@Override
+	protected String getTestExpectation(String version) {
+		return wtp.expectation;
+	}
+
+	@Override
+	protected FormatterStep createStep(String version) {
+		EclipseBasedStepBuilder builder = wtp.builderMethod.apply(TestProvisioner.mavenCentral());
+		builder.setVersion(version);
+		return builder.build();
+	}
+
+	/**
+	 * Check that configuration change is supported by all WTP formatters.
+	 * Some of the formatters only support static workspace configuration.
+	 * Hence separated class loaders are required for different configurations.
+	 */
+	@Test
+	public void multipleConfigurations() throws Exception {
+		FormatterStep tabFormatter = createStepForDefaultVersion(config -> {
+			config.setProperty("indentationChar", "tab");
+			config.setProperty("indentationSize", "1");
+		});
+		FormatterStep spaceFormatter = createStepForDefaultVersion(config -> {
+			config.setProperty("indentationChar", "space");
+			config.setProperty("indentationSize", "5");
+		});
+
+		assertThat(formatWith(tabFormatter)).as("Tab formatting output unexpected").isEqualTo(wtp.expectation); //This is the default configuration
+		assertThat(formatWith(spaceFormatter)).as("Space formatting output unexpected").isEqualTo(wtp.expectation.replace("\t", "     "));
+	}
+
+	private String formatWith(FormatterStep formatter) throws Exception {
+		File baseLocation = File.createTempFile("EclipseWtpFormatterStepTest-", ".xml"); //Only required for relative path lookup
+		return formatter.format(wtp.input, baseLocation);
+	}
+
+	private FormatterStep createStepForDefaultVersion(Consumer<Properties> config) throws IOException {
+		Properties configProps = new Properties();
+		config.accept(configProps);
+		File tempFile = File.createTempFile("EclipseWtpFormatterStepTest-", ".properties");
+		OutputStream tempOut = new FileOutputStream(tempFile);
+		configProps.store(tempOut, "test properties");
+		EclipseBasedStepBuilder builder = wtp.builderMethod.apply(TestProvisioner.mavenCentral());
+		builder.setVersion(WtpEclipseFormatterStep.defaultVersion());
+		builder.setPreferences(Arrays.asList(tempFile));
+		return builder.build();
+	}
+}

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

@@ -92,6 +92,15 @@ public ClassLoader getClassLoader() {
 		return SpotlessCache.instance().classloader(this);
 	}
 
+	/**
+	 * Returns a classloader containing only the jars in this JarState.
+	 *
+	 * The lifetime of the underlying cacheloader is controlled by {@link SpotlessCache}.
+	 */
+	public ClassLoader getClassLoader(Serializable key) {
+		return SpotlessCache.instance().classloader(key, this);
+	}
+
 	/** Returns unmodifiable view on sorted Maven coordinates */
 	public Set<String> getMavenCoordinates() {
 		return Collections.unmodifiableSet(mavenCoordinates);

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

@@ -59,9 +59,14 @@ public final int hashCode() {
 
 	@SuppressFBWarnings("DP_CREATE_CLASSLOADER_INSIDE_DO_PRIVILEGED")
 	synchronized ClassLoader classloader(JarState state) {
-		SerializedKey key = new SerializedKey(state);
+		return classloader(state, state);
+	}
+
+	@SuppressFBWarnings("DP_CREATE_CLASSLOADER_INSIDE_DO_PRIVILEGED")
+	synchronized ClassLoader classloader(Serializable key, JarState state) {
+		SerializedKey serializedKey = new SerializedKey(key);
 		return cache
-				.computeIfAbsent(key, k -> new URLClassLoader(state.jarUrls(), null));
+				.computeIfAbsent(serializedKey, k -> new URLClassLoader(state.jarUrls(), null));
 	}
 
 	static SpotlessCache instance() {