feat: supply Eclipse config properties from string (#2343)

nedtwigg · web-flow · commit ca1a338ff439 · 2024-12-08T08:43:09.000-08:00
diff --git a/CHANGES.md b/CHANGES.md
@@ -11,6 +11,7 @@ We adhere to the [keepachangelog](https://keepachangelog.com/en/1.0.0/) format (
 
 ## [Unreleased]
 ### Changed
+* Allow setting Eclipse config from a string, not only from files ([#2337](https://github.com/diffplug/spotless/pull/2337))
 * Bump default `ktlint` version to latest `1.3.0` -> `1.4.0`. ([#2314](https://github.com/diffplug/spotless/pull/2314))
 * Add _Sort Members_ feature based on [Eclipse JDT](plugin-gradle/README.md#eclipse-jdt) implementation. ([#2312](https://github.com/diffplug/spotless/pull/2312))
 * Bump default `jackson` version to latest `2.18.0` -> `2.18.1`. ([#2319](https://github.com/diffplug/spotless/pull/2319))
diff --git a/lib-extra/src/main/java/com/diffplug/spotless/extra/EquoBasedStepBuilder.java b/lib-extra/src/main/java/com/diffplug/spotless/extra/EquoBasedStepBuilder.java
@@ -24,6 +24,7 @@
 import java.util.Collection;
 import java.util.List;
 import java.util.Map;
+import java.util.Optional;
 import java.util.Properties;
 
 import javax.annotation.Nullable;
@@ -54,6 +55,7 @@ public abstract class EquoBasedStepBuilder {
 	private final ImmutableMap.Builder<String, String> stepProperties;
 	private String formatterVersion;
 	private Iterable<File> settingsFiles = new ArrayList<>();
+	private List<String> settingProperties = new ArrayList<>();
 	private Map<String, String> p2Mirrors = Map.of();
 	private File cacheDirectory;
 
@@ -80,6 +82,10 @@ public void setPreferences(Iterable<File> settingsFiles) {
 		this.settingsFiles = settingsFiles;
 	}
 
+	public void setPropertyPreferences(List<String> propertyPreferences) {
+		this.settingProperties = propertyPreferences;
+	}
+
 	public void setP2Mirrors(Map<String, String> p2Mirrors) {
 		this.p2Mirrors = Map.copyOf(p2Mirrors);
 	}
@@ -113,7 +119,7 @@ protected void addPlatformRepo(P2Model model, String version) {
 
 	/** Returns the FormatterStep (whose state will be calculated lazily). */
 	public FormatterStep build() {
-		var roundtrippableState = new EquoStep(formatterVersion, FileSignature.promise(settingsFiles), JarState.promise(() -> {
+		var roundtrippableState = new EquoStep(formatterVersion, settingProperties, FileSignature.promise(settingsFiles), JarState.promise(() -> {
 			P2QueryResult query;
 			try {
 				if (null != cacheDirectory) {
@@ -167,21 +173,24 @@ static class EquoStep implements Serializable {
 		private final FileSignature.Promised settingsPromise;
 		private final JarState.Promised jarPromise;
 		private final ImmutableMap<String, String> stepProperties;
+		private List<String> settingProperties;
 
 		EquoStep(
 				String semanticVersion,
+				List<String> settingProperties,
 				FileSignature.Promised settingsPromise,
 				JarState.Promised jarPromise,
 				ImmutableMap<String, String> stepProperties) {
 
 			this.semanticVersion = semanticVersion;
+			this.settingProperties = Optional.ofNullable(settingProperties).orElse(new ArrayList<>());
 			this.settingsPromise = settingsPromise;
 			this.jarPromise = jarPromise;
 			this.stepProperties = stepProperties;
 		}
 
 		private State state() {
-			return new State(semanticVersion, jarPromise.get(), settingsPromise.get(), stepProperties);
+			return new State(semanticVersion, jarPromise.get(), settingProperties, settingsPromise.get(), stepProperties);
 		}
 	}
 
@@ -195,10 +204,12 @@ public static class State implements Serializable {
 		final JarState jarState;
 		final FileSignature settingsFiles;
 		final ImmutableMap<String, String> stepProperties;
+		private List<String> settingProperties;
 
-		public State(String semanticVersion, JarState jarState, FileSignature settingsFiles, ImmutableMap<String, String> stepProperties) {
+		public State(String semanticVersion, JarState jarState, List<String> settingProperties, FileSignature settingsFiles, ImmutableMap<String, String> stepProperties) {
 			this.semanticVersion = semanticVersion;
 			this.jarState = jarState;
+			this.settingProperties = Optional.ofNullable(settingProperties).orElse(new ArrayList<>());
 			this.settingsFiles = settingsFiles;
 			this.stepProperties = stepProperties;
 		}
@@ -212,7 +223,9 @@ public String getSemanticVersion() {
 		}
 
 		public Properties getPreferences() {
-			return FormatterProperties.from(settingsFiles.files()).getProperties();
+			FormatterProperties fromFiles = FormatterProperties.from(settingsFiles.files());
+			FormatterProperties fromPropertiesContent = FormatterProperties.fromPropertiesContent(settingProperties);
+			return FormatterProperties.merge(fromFiles.getProperties(), fromPropertiesContent.getProperties()).getProperties();
 		}
 
 		public ImmutableMap<String, String> getStepProperties() {
diff --git a/lib/src/main/java/com/diffplug/spotless/FormatterProperties.java b/lib/src/main/java/com/diffplug/spotless/FormatterProperties.java
@@ -1,5 +1,5 @@
 /*
- * Copyright 2016-2021 DiffPlug
+ * Copyright 2016-2024 DiffPlug
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -17,10 +17,12 @@
 
 import static com.diffplug.spotless.MoreIterables.toNullHostileList;
 
+import java.io.ByteArrayInputStream;
 import java.io.File;
 import java.io.FileInputStream;
 import java.io.IOException;
 import java.io.InputStream;
+import java.nio.charset.StandardCharsets;
 import java.util.Arrays;
 import java.util.LinkedList;
 import java.util.List;
@@ -75,6 +77,25 @@ public static FormatterProperties from(Iterable<File> files) throws IllegalArgum
 		return properties;
 	}
 
+	public static FormatterProperties fromPropertiesContent(Iterable<String> content) throws IllegalArgumentException {
+		List<String> nonNullElements = toNullHostileList(content);
+		FormatterProperties properties = new FormatterProperties();
+		nonNullElements.forEach(contentElement -> {
+			try (InputStream is = new ByteArrayInputStream(contentElement.getBytes(StandardCharsets.UTF_8))) {
+				properties.properties.load(is);
+			} catch (IOException e) {
+				throw new IllegalArgumentException("Unable to load properties: " + contentElement);
+			}
+		});
+		return properties;
+	}
+
+	public static FormatterProperties merge(Properties... properties) {
+		FormatterProperties merged = new FormatterProperties();
+		List.of(properties).stream().forEach((source) -> merged.properties.putAll(source));
+		return merged;
+	}
+
 	/**
 	 * Import settings from given file. New settings (with the same ID/key)
 	 * override existing once.
diff --git a/plugin-gradle/CHANGES.md b/plugin-gradle/CHANGES.md
@@ -4,6 +4,7 @@ We adhere to the [keepachangelog](https://keepachangelog.com/en/1.0.0/) format (
 
 ## [Unreleased]
 ### Changed
+* Allow setting Eclipse config from a string, not only from files ([#2337](https://github.com/diffplug/spotless/pull/2337))
 * Bump default `ktlint` version to latest `1.3.0` -> `1.4.0`. ([#2314](https://github.com/diffplug/spotless/pull/2314))
 * Bump default `jackson` version to latest `2.18.0` -> `2.18.1`. ([#2319](https://github.com/diffplug/spotless/pull/2319))
 * Bump default `ktfmt` version to latest `0.52` -> `0.53`. ([#2320](https://github.com/diffplug/spotless/pull/2320))
diff --git a/plugin-gradle/README.md b/plugin-gradle/README.md
@@ -262,6 +262,10 @@ spotless {
     eclipse()
     // optional: you can specify a specific version and/or config file
     eclipse('4.26').configFile('eclipse-prefs.xml')
+    // Or supply the configuration as a string
+    eclipse('4.26').configProperties("""
+    ...
+    """)
     // if the access to the p2 repositories is restricted, mirrors can be
     // specified using a URI prefix map as follows:
     eclipse().withP2Mirrors(['https://download.eclipse.org/eclipse/updates/4.29/':'https://some.internal.mirror/4-29-updates-p2/']) 
@@ -418,6 +422,10 @@ spotless {
     greclipse()
     // optional: you can specify a specific version or config file(s), version matches the Eclipse Platform
     greclipse('4.26').configFile('spotless.eclipseformat.xml', 'org.codehaus.groovy.eclipse.ui.prefs')
+    // Or supply the configuration as a string
+    greclipse('4.26').configProperties("""
+    ...
+    """)
 ```
 
 Groovy-Eclipse formatting errors/warnings lead per default to a build failure. This behavior can be changed by adding the property/key value `ignoreFormatterProblems=true` to a configuration file. In this scenario, files causing problems, will not be modified by this formatter step.
@@ -572,6 +580,10 @@ spotles {
   cpp {
     // version and configFile are both optional
     eclipseCdt('4.13.0').configFile('eclipse-cdt.xml')
+    // Or supply the configuration as a string
+    eclipseCdt('4.13.0').configProperties("""
+    ...
+    """)
   }
 }
 ```
diff --git a/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/BaseGroovyExtension.java b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/BaseGroovyExtension.java
@@ -17,6 +17,7 @@
 
 import static com.diffplug.gradle.spotless.PluginGradlePreconditions.requireElementsNonNull;
 
+import java.util.List;
 import java.util.Map;
 import java.util.Objects;
 
@@ -72,6 +73,13 @@ public GrEclipseConfig configFile(Object... configFiles) {
 			return this;
 		}
 
+		public GrEclipseConfig configProperties(String... configs) {
+			requireElementsNonNull(configs);
+			builder.setPropertyPreferences(List.of(configs));
+			extension.replaceStep(builder.build());
+			return this;
+		}
+
 		public GrEclipseConfig withP2Mirrors(Map<String, String> mirrors) {
 			builder.setP2Mirrors(mirrors);
 			extension.replaceStep(builder.build());
diff --git a/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/CppExtension.java b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/CppExtension.java
@@ -17,12 +17,14 @@
 
 import static com.diffplug.gradle.spotless.PluginGradlePreconditions.requireElementsNonNull;
 
+import java.util.List;
 import java.util.Map;
 
 import javax.inject.Inject;
 
 import org.gradle.api.Project;
 
+import com.diffplug.gradle.spotless.JavaExtension.EclipseConfig;
 import com.diffplug.spotless.cpp.CppDefaults;
 import com.diffplug.spotless.extra.EquoBasedStepBuilder;
 import com.diffplug.spotless.extra.cpp.EclipseCdtFormatterStep;
@@ -60,6 +62,13 @@ public EclipseConfig configFile(Object... configFiles) {
 			return this;
 		}
 
+		public EclipseConfig configProperties(String... configs) {
+			requireElementsNonNull(configs);
+			builder.setPropertyPreferences(List.of(configs));
+			replaceStep(builder.build());
+			return this;
+		}
+
 		public EclipseConfig withP2Mirrors(Map<String, String> mirrors) {
 			builder.setP2Mirrors(mirrors);
 			replaceStep(builder.build());
diff --git a/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/JavaExtension.java b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/JavaExtension.java
@@ -305,6 +305,13 @@ public EclipseConfig configFile(Object... configFiles) {
 			return this;
 		}
 
+		public EclipseConfig configProperties(String... configs) {
+			requireElementsNonNull(configs);
+			builder.setPropertyPreferences(List.of(configs));
+			replaceStep(builder.build());
+			return this;
+		}
+
 		public EclipseConfig sortMembersDoNotSortFields(boolean doNotSortFields) {
 			builder.sortMembersDoNotSortFields(doNotSortFields);
 			replaceStep(builder.build());
diff --git a/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/JavaEclipseTest.java b/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/JavaEclipseTest.java
@@ -0,0 +1,40 @@
+/*
+ * Copyright 2016-2024 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.gradle.spotless;
+
+import java.io.IOException;
+
+import org.junit.jupiter.api.Test;
+
+class JavaEclipseTest extends GradleIntegrationHarness {
+	@Test
+	void settingsWithContentWithoutFile() throws IOException {
+		setFile("build.gradle").toLines(
+				"plugins {",
+				"  id 'com.diffplug.spotless'",
+				"  id 'java'",
+				"}",
+				"repositories { mavenCentral() }",
+				"",
+				"spotless {",
+				"  java {  eclipse().configProperties(\"\"\"",
+				"valid_line_oriented.prefs.string=string",
+				"\"\"\")  }",
+				"}");
+
+		gradleRunner().withArguments("spotlessApply").build();
+	}
+}
diff --git a/testlib/src/test/java/com/diffplug/spotless/FormatterPropertiesTest.java b/testlib/src/test/java/com/diffplug/spotless/FormatterPropertiesTest.java
@@ -1,5 +1,5 @@
 /*
- * Copyright 2016-2021 DiffPlug
+ * Copyright 2016-2024 DiffPlug
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -19,9 +19,12 @@
 
 import java.io.File;
 import java.io.IOException;
+import java.nio.file.Files;
 import java.util.Collection;
 import java.util.LinkedList;
+import java.util.List;
 import java.util.Properties;
+import java.util.stream.Collectors;
 
 import org.assertj.core.api.AbstractAssert;
 import org.assertj.core.api.Assertions;
@@ -45,6 +48,14 @@ class FormatterPropertiesTest extends ResourceHarness {
 			RESOURCES_ROOT_DIR + "invalid_xml_properties.xml"
 	};
 
+	private List<String> validPropertiesResources() {
+		return List.of(VALID_SETTINGS_RESOURCES).stream().filter(it -> !it.endsWith(".xml")).collect(Collectors.toList());
+	}
+
+	private List<String> invalidPropertiesResources() {
+		return List.of(INVALID_SETTINGS_RESOURCES).stream().filter(it -> !it.endsWith(".xml")).collect(Collectors.toList());
+	}
+
 	private static final String[] VALID_VALUES = {
 			"string",
 			"true",
@@ -63,6 +74,18 @@ void differentPropertyFileTypes() throws IOException {
 		}
 	}
 
+	@Test
+	void differentPropertyFileTypes_content_properties() throws IOException {
+		for (String settingsResource : validPropertiesResources()) {
+			File settingsFile = createTestFile(settingsResource);
+			String content = Files.readString(settingsFile.toPath());
+			FormatterProperties preferences = FormatterProperties.fromPropertiesContent(List.of(content));
+			assertFor(preferences)
+					.containsSpecificValuesOf(settingsFile)
+					.containsCommonValueOf(settingsFile);
+		}
+	}
+
 	@Test
 	void multiplePropertyFiles() throws IOException {
 		LinkedList<File> settingsFiles = new LinkedList<>();
@@ -77,6 +100,22 @@ void multiplePropertyFiles() throws IOException {
 				.containsCommonValueOf(settingsFiles.getLast());
 	}
 
+	@Test
+	void multiplePropertyFiles_content_properties() throws IOException {
+		LinkedList<File> settingsFiles = new LinkedList<>();
+		LinkedList<String> content = new LinkedList<>();
+		for (String settingsResource : validPropertiesResources()) {
+			File settingsFile = createTestFile(settingsResource);
+			content.add(Files.readString(settingsFile.toPath()));
+			settingsFiles.add(settingsFile);
+		}
+		FormatterProperties preferences = FormatterProperties.fromPropertiesContent(content);
+		/* Settings are loaded / overridden in the sequence they are configured. */
+		assertFor(preferences)
+				.containsSpecificValuesOf(settingsFiles)
+				.containsCommonValueOf(settingsFiles.getLast());
+	}
+
 	@Test
 	void invalidPropertyFiles() throws IOException {
 		for (String settingsResource : INVALID_SETTINGS_RESOURCES) {
@@ -96,6 +135,26 @@ void invalidPropertyFiles() throws IOException {
 		}
 	}
 
+	@Test
+	void invalidPropertyFiles_content_properties() throws IOException {
+		for (String settingsResource : invalidPropertiesResources()) {
+			File settingsFile = createTestFile(settingsResource);
+			String content = Files.readString(settingsFile.toPath());
+			boolean exceptionCaught = false;
+			try {
+				FormatterProperties.fromPropertiesContent(List.of(content));
+			} catch (IllegalArgumentException ex) {
+				exceptionCaught = true;
+				assertThat(ex.getMessage())
+						.as("IllegalArgumentException does not contain absolute path of file '%s'", settingsFile.getName())
+						.contains(settingsFile.getAbsolutePath());
+			}
+			assertThat(exceptionCaught)
+					.as("No IllegalArgumentException thrown when parsing '%s'", settingsFile.getName())
+					.isTrue();
+		}
+	}
+
 	@Test
 	void nonExistingFile() throws IOException {
 		String filePath = FileSignature.pathUnixToNative("does/not/exist.properties");
