From 6efa10afe558050e6fb14f741a55020895a57e49 Mon Sep 17 00:00:00 2001 From: Tomas Bjerre Date: Wed, 11 Dec 2024 08:58:33 +0100 Subject: [PATCH 1/3] feat: supply eclipse formatter settings as XML content --- CHANGES.md | 1 + .../spotless/extra/EquoBasedStepBuilder.java | 19 ++++-- .../spotless/FormatterProperties.java | 65 +++++++++++++++---- plugin-gradle/README.md | 18 ++++- .../gradle/spotless/BaseGroovyExtension.java | 7 ++ .../gradle/spotless/CppExtension.java | 7 ++ .../gradle/spotless/JavaExtension.java | 7 ++ .../gradle/spotless/JavaEclipseTest.java | 25 ++++++- .../spotless/FormatterPropertiesTest.java | 57 +++++++++++----- 9 files changed, 168 insertions(+), 38 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index 22b2b73e27..9465f8afe4 100644 --- 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 +* TODO ([#](https://github.com/diffplug/spotless/pull/X)) * 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)) 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 index df3824e9ca..267d6cc496 100644 --- a/lib-extra/src/main/java/com/diffplug/spotless/extra/EquoBasedStepBuilder.java +++ b/lib-extra/src/main/java/com/diffplug/spotless/extra/EquoBasedStepBuilder.java @@ -56,6 +56,7 @@ public abstract class EquoBasedStepBuilder { private String formatterVersion; private Iterable settingsFiles = new ArrayList<>(); private List settingProperties = new ArrayList<>(); + private List settingXml = new ArrayList<>(); private Map p2Mirrors = Map.of(); private File cacheDirectory; @@ -86,6 +87,10 @@ public void setPropertyPreferences(List propertyPreferences) { this.settingProperties = propertyPreferences; } + public void setXmlPreferences(List settingXml) { + this.settingXml = settingXml; + } + public void setP2Mirrors(Map p2Mirrors) { this.p2Mirrors = Map.copyOf(p2Mirrors); } @@ -119,7 +124,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, settingProperties, FileSignature.promise(settingsFiles), JarState.promise(() -> { + var roundtrippableState = new EquoStep(formatterVersion, settingProperties, settingXml, FileSignature.promise(settingsFiles), JarState.promise(() -> { P2QueryResult query; try { if (null != cacheDirectory) { @@ -174,23 +179,26 @@ static class EquoStep implements Serializable { private final JarState.Promised jarPromise; private final ImmutableMap stepProperties; private List settingProperties; + private List settingXml; EquoStep( String semanticVersion, List settingProperties, + List settingXml, FileSignature.Promised settingsPromise, JarState.Promised jarPromise, ImmutableMap stepProperties) { this.semanticVersion = semanticVersion; this.settingProperties = Optional.ofNullable(settingProperties).orElse(new ArrayList<>()); + this.settingXml = Optional.ofNullable(settingXml).orElse(new ArrayList<>()); this.settingsPromise = settingsPromise; this.jarPromise = jarPromise; this.stepProperties = stepProperties; } private State state() { - return new State(semanticVersion, jarPromise.get(), settingProperties, settingsPromise.get(), stepProperties); + return new State(semanticVersion, jarPromise.get(), settingProperties, settingXml, settingsPromise.get(), stepProperties); } } @@ -205,11 +213,13 @@ public static class State implements Serializable { final FileSignature settingsFiles; final ImmutableMap stepProperties; private List settingProperties; + private List settingXml; - public State(String semanticVersion, JarState jarState, List settingProperties, FileSignature settingsFiles, ImmutableMap stepProperties) { + public State(String semanticVersion, JarState jarState, List settingProperties, List settingXml, FileSignature settingsFiles, ImmutableMap stepProperties) { this.semanticVersion = semanticVersion; this.jarState = jarState; this.settingProperties = Optional.ofNullable(settingProperties).orElse(new ArrayList<>()); + this.settingXml = Optional.ofNullable(settingXml).orElse(new ArrayList<>()); this.settingsFiles = settingsFiles; this.stepProperties = stepProperties; } @@ -225,7 +235,8 @@ public String getSemanticVersion() { public Properties getPreferences() { FormatterProperties fromFiles = FormatterProperties.from(settingsFiles.files()); FormatterProperties fromPropertiesContent = FormatterProperties.fromPropertiesContent(settingProperties); - return FormatterProperties.merge(fromFiles.getProperties(), fromPropertiesContent.getProperties()).getProperties(); + FormatterProperties fromXmlContent = FormatterProperties.fromXmlContent(settingXml); + return FormatterProperties.merge(fromFiles.getProperties(), fromPropertiesContent.getProperties(), fromXmlContent.getProperties()).getProperties(); } public ImmutableMap getStepProperties() { diff --git a/lib/src/main/java/com/diffplug/spotless/FormatterProperties.java b/lib/src/main/java/com/diffplug/spotless/FormatterProperties.java index 5ea32f8f99..455318e38f 100644 --- a/lib/src/main/java/com/diffplug/spotless/FormatterProperties.java +++ b/lib/src/main/java/com/diffplug/spotless/FormatterProperties.java @@ -20,6 +20,7 @@ import java.io.ByteArrayInputStream; import java.io.File; import java.io.FileInputStream; +import java.io.FileNotFoundException; import java.io.IOException; import java.io.InputStream; import java.nio.charset.StandardCharsets; @@ -28,6 +29,7 @@ import java.util.List; import java.util.Objects; import java.util.Properties; +import java.util.function.Supplier; import java.util.stream.Collectors; import java.util.stream.IntStream; @@ -90,6 +92,25 @@ public static FormatterProperties fromPropertiesContent(Iterable content return properties; } + public static FormatterProperties fromXmlContent(final Iterable content) throws IllegalArgumentException { + final List nonNullElements = toNullHostileList(content); + final FormatterProperties properties = new FormatterProperties(); + nonNullElements.forEach(contentElement -> { + try { + final Properties newSettings = FileParser.XML.executeXmlContent(contentElement); + properties.properties.putAll(newSettings); + } catch (IOException | IllegalArgumentException exception) { + String message = String.format("Failed to add preferences from XML:%n%s%n", contentElement); + final String detailedMessage = exception.getMessage(); + if (null != detailedMessage) { + message += String.format(" %s", detailedMessage); + } + throw new IllegalArgumentException(message, exception); + } + }); + return properties; + } + public static FormatterProperties merge(Properties... properties) { FormatterProperties merged = new FormatterProperties(); List.of(properties).stream().forEach((source) -> merged.properties.putAll(source)); @@ -139,20 +160,40 @@ protected Properties execute(final File file) throws IOException, IllegalArgumen } return properties; } + + @Override + protected Properties executeXmlContent(String content) throws IOException, IllegalArgumentException { + throw new RuntimeException("Not implemented"); + } }, XML("xml") { @Override protected Properties execute(final File file) throws IOException, IllegalArgumentException { - Node rootNode = getRootNode(file); + return executeWithSupplier(() -> { + try { + return new FileInputStream(file); + } catch (FileNotFoundException e) { + throw new RuntimeException("File not found: " + file, e); + } + }); + } + + @Override + protected Properties executeXmlContent(String content) throws IOException, IllegalArgumentException { + return executeWithSupplier(() -> new ByteArrayInputStream(content.getBytes(StandardCharsets.UTF_8))); + } + + private Properties executeWithSupplier(Supplier isSupplier) throws IOException, IllegalArgumentException { + Node rootNode = getRootNode(isSupplier.get()); String nodeName = rootNode.getNodeName(); if (null == nodeName) { throw new IllegalArgumentException("XML document does not contain a root node."); } - return XmlParser.parse(file, rootNode); + return XmlParser.parse(isSupplier.get(), rootNode); } - private Node getRootNode(final File file) throws IOException, IllegalArgumentException { + private Node getRootNode(final InputStream is) throws IOException, IllegalArgumentException { try { DocumentBuilderFactory dbf = DocumentBuilderFactory.newInstance(); /* @@ -166,7 +207,7 @@ private Node getRootNode(final File file) throws IOException, IllegalArgumentExc */ dbf.setFeature(LOAD_EXTERNAL_DTD_PROP, false); DocumentBuilder db = dbf.newDocumentBuilder(); - return db.parse(file).getDocumentElement(); + return db.parse(is).getDocumentElement(); } catch (SAXException | ParserConfigurationException e) { throw new IllegalArgumentException("File has no valid XML syntax.", e); } @@ -186,6 +227,8 @@ private Node getRootNode(final File file) throws IOException, IllegalArgumentExc protected abstract Properties execute(File file) throws IOException, IllegalArgumentException; + protected abstract Properties executeXmlContent(String content) throws IOException, IllegalArgumentException; + public static Properties parse(final File file) throws IOException, IllegalArgumentException { String fileNameExtension = getFileNameExtension(file); for (FileParser parser : FileParser.values()) { @@ -211,19 +254,17 @@ private static String getFileNameExtension(File file) { private enum XmlParser { PROPERTIES("properties") { @Override - protected Properties execute(final File xmlFile, final Node rootNode) + protected Properties execute(final InputStream xmlFile, final Node rootNode) throws IOException, IllegalArgumentException { final Properties properties = new Properties(); - try (InputStream xmlInput = new FileInputStream(xmlFile)) { - properties.loadFromXML(xmlInput); - } + properties.loadFromXML(xmlFile); return properties; } }, PROFILES("profiles") { @Override - protected Properties execute(File file, Node rootNode) throws IOException, IllegalArgumentException { + protected Properties execute(InputStream file, Node rootNode) throws IOException, IllegalArgumentException { final Properties properties = new Properties(); Node firstProfile = getSingleProfile(rootNode); for (Object settingObj : getChildren(firstProfile, "setting")) { @@ -285,14 +326,14 @@ public String toString() { return this.rootNodeName; } - protected abstract Properties execute(File file, Node rootNode) throws IOException, IllegalArgumentException; + protected abstract Properties execute(InputStream is, Node rootNode) throws IOException, IllegalArgumentException; - public static Properties parse(final File file, final Node rootNode) + public static Properties parse(final InputStream is, final Node rootNode) throws IOException, IllegalArgumentException { String rootNodeName = rootNode.getNodeName(); for (XmlParser parser : XmlParser.values()) { if (parser.rootNodeName.equals(rootNodeName)) { - return parser.execute(file, rootNode); + return parser.execute(is, rootNode); } } String msg = String.format("The XML root node '%1$s' is not part of the supported root nodes [%2$s].", diff --git a/plugin-gradle/README.md b/plugin-gradle/README.md index 58db554e66..6ed02b0f2b 100644 --- a/plugin-gradle/README.md +++ b/plugin-gradle/README.md @@ -262,10 +262,14 @@ 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 + // Or supply the configuration properties as a string eclipse('4.26').configProperties(""" ... """) + // Or supply the configuration XML as a string + eclipse('4.26').configXml(""" + ... + """) // 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/']) @@ -422,10 +426,14 @@ 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 + // Or supply the configuration properties as a string greclipse('4.26').configProperties(""" ... """) + // Or supply the configuration XML as a string + greclipse('4.26').configXml(""" + ... + """) ``` 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. @@ -580,10 +588,14 @@ spotles { cpp { // version and configFile are both optional eclipseCdt('4.13.0').configFile('eclipse-cdt.xml') - // Or supply the configuration as a string + // Or supply the configuration properties as a string eclipseCdt('4.13.0').configProperties(""" ... """) + // Or supply the configuration XML as a string + eclipseCdt('4.13.0').configXml(""" + ... + """) } } ``` 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 index 83d678c46c..37712a470b 100644 --- a/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/BaseGroovyExtension.java +++ b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/BaseGroovyExtension.java @@ -80,6 +80,13 @@ public GrEclipseConfig configProperties(String... configs) { return this; } + public GrEclipseConfig configXml(String... configs) { + requireElementsNonNull(configs); + builder.setXmlPreferences(List.of(configs)); + extension.replaceStep(builder.build()); + return this; + } + public GrEclipseConfig withP2Mirrors(Map 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 index f2d6ee91bb..db7d9bee9a 100644 --- a/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/CppExtension.java +++ b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/CppExtension.java @@ -69,6 +69,13 @@ public EclipseConfig configProperties(String... configs) { return this; } + public EclipseConfig configXml(String... configs) { + requireElementsNonNull(configs); + builder.setXmlPreferences(List.of(configs)); + replaceStep(builder.build()); + return this; + } + public EclipseConfig withP2Mirrors(Map 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 index b7d3c84304..343092b4ad 100644 --- a/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/JavaExtension.java +++ b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/JavaExtension.java @@ -312,6 +312,13 @@ public EclipseConfig configProperties(String... configs) { return this; } + public EclipseConfig configXml(String... configs) { + requireElementsNonNull(configs); + builder.setXmlPreferences(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 index c102b7abe1..1ef433acd6 100644 --- a/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/JavaEclipseTest.java +++ b/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/JavaEclipseTest.java @@ -21,7 +21,7 @@ class JavaEclipseTest extends GradleIntegrationHarness { @Test - void settingsWithContentWithoutFile() throws IOException { + void settingsWithProprtiesContent() throws IOException { setFile("build.gradle").toLines( "plugins {", " id 'com.diffplug.spotless'", @@ -37,4 +37,27 @@ void settingsWithContentWithoutFile() throws IOException { gradleRunner().withArguments("spotlessApply").build(); } + + @Test + void settingsWithXmlContent() throws IOException { + setFile("build.gradle").toLines( + "plugins {", + " id 'com.diffplug.spotless'", + " id 'java'", + "}", + "repositories { mavenCentral() }", + "", + "spotless {", + " java { eclipse().configProperties(\"\"\"", + "", + "", + " ", + " ", + " ", + "", + "\"\"\") }", + "}"); + + 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 index 8333fcc41d..58ae1c913e 100644 --- a/testlib/src/test/java/com/diffplug/spotless/FormatterPropertiesTest.java +++ b/testlib/src/test/java/com/diffplug/spotless/FormatterPropertiesTest.java @@ -16,6 +16,7 @@ package com.diffplug.spotless; import static org.assertj.core.api.Assertions.assertThat; +import static org.junit.jupiter.api.Assertions.assertThrows; import java.io.File; import java.io.IOException; @@ -52,8 +53,12 @@ private List validPropertiesResources() { return List.of(VALID_SETTINGS_RESOURCES).stream().filter(it -> !it.endsWith(".xml")).collect(Collectors.toList()); } - private List invalidPropertiesResources() { - return List.of(INVALID_SETTINGS_RESOURCES).stream().filter(it -> !it.endsWith(".xml")).collect(Collectors.toList()); + private List validXmlResources() { + return List.of(VALID_SETTINGS_RESOURCES).stream().filter(it -> it.endsWith(".xml")).collect(Collectors.toList()); + } + + private List invalidXmlResources() { + return List.of(INVALID_SETTINGS_RESOURCES).stream().filter(it -> it.endsWith(".xml")).collect(Collectors.toList()); } private static final String[] VALID_VALUES = { @@ -86,6 +91,18 @@ void differentPropertyFileTypes_content_properties() throws IOException { } } + @Test + void differentPropertyFileTypes_content_xml() throws IOException { + for (String settingsResource : validXmlResources()) { + File settingsFile = createTestFile(settingsResource); + String content = Files.readString(settingsFile.toPath()); + FormatterProperties preferences = FormatterProperties.fromXmlContent(List.of(content)); + assertFor(preferences) + .containsSpecificValuesOf(settingsFile) + .containsCommonValueOf(settingsFile); + } + } + @Test void multiplePropertyFiles() throws IOException { LinkedList settingsFiles = new LinkedList<>(); @@ -116,6 +133,22 @@ void multiplePropertyFiles_content_properties() throws IOException { .containsCommonValueOf(settingsFiles.getLast()); } + @Test + void multiplePropertyFiles_content_xml() throws IOException { + LinkedList settingsFiles = new LinkedList<>(); + LinkedList content = new LinkedList<>(); + for (String settingsResource : validXmlResources()) { + File settingsFile = createTestFile(settingsResource); + content.add(Files.readString(settingsFile.toPath())); + settingsFiles.add(settingsFile); + } + FormatterProperties preferences = FormatterProperties.fromXmlContent(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) { @@ -136,22 +169,10 @@ 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(); + void invalidPropertyFiles_content_xml() throws IOException { + for (String settingsResource : invalidXmlResources()) { + IllegalArgumentException actual = assertThrows(IllegalArgumentException.class, () -> FormatterProperties.fromXmlContent(List.of(ResourceHarness.getTestResource(settingsResource)))); + assertThat(actual.getMessage()).startsWith("Failed to add preferences from XML:"); } } From eb7bf1a5d26b5f190946c603bdffb55ab02f9672 Mon Sep 17 00:00:00 2001 From: Tomas Bjerre Date: Wed, 11 Dec 2024 10:54:12 +0100 Subject: [PATCH 2/3] chore: changes --- CHANGES.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGES.md b/CHANGES.md index 9465f8afe4..2941d60e9d 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -11,7 +11,7 @@ We adhere to the [keepachangelog](https://keepachangelog.com/en/1.0.0/) format ( ## [Unreleased] ### Changed -* TODO ([#](https://github.com/diffplug/spotless/pull/X)) +* Allow setting Eclipse XML config from a string, not only from files ([#2361](https://github.com/diffplug/spotless/pull/2361)) * 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)) From 45420dcfb0ff07be6e598c5ecf48a7fbb325a833 Mon Sep 17 00:00:00 2001 From: Tomas Bjerre Date: Wed, 11 Dec 2024 11:01:52 +0100 Subject: [PATCH 3/3] chore: testing with configXml not properties --- .../test/java/com/diffplug/gradle/spotless/JavaEclipseTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 index 1ef433acd6..862f82010f 100644 --- a/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/JavaEclipseTest.java +++ b/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/JavaEclipseTest.java @@ -48,7 +48,7 @@ void settingsWithXmlContent() throws IOException { "repositories { mavenCentral() }", "", "spotless {", - " java { eclipse().configProperties(\"\"\"", + " java { eclipse().configXml(\"\"\"", "", "", " ",