Patched an issue with formatting empty elements

plugins/codemodder-plugin-maven/src/main/java/io/codemodder/plugins/maven/operator/CommandChain.java

@@ -16,18 +16,7 @@ class CommandChain {
   private static final Logger LOGGER = LoggerFactory.getLogger(CommandChain.class);
 
   /** Internal ArrayList of the Commands */
-  private List<Command> commandList;
-
-  private static List<Command> COMMON_COMMANDS =
-      List.of(
-          // Validation commands
-          CheckDependencyPresent.getInstance(),
-          CheckParentPackaging.getInstance(),
-          // Format commands
-          new FormatCommand(),
-          new DiscardFormatCommand(),
-          // Multipom command
-          new CompositeDependencyManagement());
+  private final List<Command> commandList;
 
   private CommandChain(List<Command> commands) {
     this.commandList = commands;
@@ -93,7 +82,17 @@ public boolean execute(ProjectModel c)
    * @return A pre-configured Chain for modifying a POM.
    */
   public static CommandChain modifyDependency() {
-    final List<Command> modifyCommands = new ArrayList<>(COMMON_COMMANDS);
+    final List<Command> modifyCommands =
+        new ArrayList<>(
+            List.of(
+                // Validation commands
+                CheckDependencyPresent.getInstance(),
+                CheckParentPackaging.getInstance(),
+                // Format commands
+                new FormatCommand(),
+                new DiscardFormatCommand(),
+                // Multipom command
+                new CompositeDependencyManagement()));
     modifyCommands.addAll(
         List.of(
             SimpleUpgrade.getInstance(),
@@ -109,7 +108,17 @@ public static CommandChain modifyDependency() {
    * @return A pre-configured Chain.
    */
   public static CommandChain insertDependency() {
-    final List<Command> insertCommands = new ArrayList<>(COMMON_COMMANDS);
+    final List<Command> insertCommands =
+        new ArrayList<>(
+            List.of(
+                // Validation commands
+                CheckDependencyPresent.getInstance(),
+                CheckParentPackaging.getInstance(),
+                // Format commands
+                new FormatCommand(),
+                new DiscardFormatCommand(),
+                // Multipom command
+                new CompositeDependencyManagement()));
     insertCommands.add(new SimpleInsert(true));
     return new CommandChain(insertCommands);
   }
@@ -120,7 +129,17 @@ public static CommandChain insertDependency() {
    * @return A pre-configured Chain.
    */
   public static CommandChain updateDependency() {
-    final List<Command> insertCommands = new ArrayList<>(COMMON_COMMANDS);
+    final List<Command> insertCommands =
+        new ArrayList<>(
+            List.of(
+                // Validation commands
+                CheckDependencyPresent.getInstance(),
+                CheckParentPackaging.getInstance(),
+                // Format commands
+                new FormatCommand(),
+                new DiscardFormatCommand(),
+                // Multipom command
+                new CompositeDependencyManagement()));
     insertCommands.addAll(
         List.of(SimpleUpgrade.getInstance(), SimpleDependencyManagement.getInstance()));
 
@@ -169,7 +188,8 @@ public static CommandChain createForDependencyQuery(QueryType queryType) {
     return filterByQueryType(
         AVAILABLE_DEPENDENCY_QUERY_COMMANDS,
         queryType,
-        Arrays.asList(CheckLocalRepositoryDirCommand.CheckParentDirCommand.getInstance()),
+        Collections.singletonList(
+            CheckLocalRepositoryDirCommand.CheckParentDirCommand.getInstance()),
         it -> it == queryType);
   }
 
@@ -196,7 +216,7 @@ public static CommandChain createForVersionQuery(QueryType queryType) {
    * and report issues creating
    */
   static final List<Pair<QueryType, String>> AVAILABLE_DEPENDENCY_QUERY_COMMANDS =
-      new ArrayList<>(Arrays.asList(new Pair<>(QueryType.SAFE, "QueryByParsing")));
+      new ArrayList<>(List.of(new Pair<>(QueryType.SAFE, "QueryByParsing")));
 
   /** List of Commands for Version Query */
   private static final List<Pair<QueryType, String>> AVAILABLE_QUERY_VERSION_COMMANDS =

plugins/codemodder-plugin-maven/src/main/java/io/codemodder/plugins/maven/operator/FormatCommand.java

@@ -9,6 +9,7 @@
 import java.net.URISyntaxException;
 import java.nio.charset.Charset;
 import java.util.*;
+import java.util.function.UnaryOperator;
 import java.util.regex.Pattern;
 import java.util.stream.Collectors;
 import java.util.stream.Stream;
@@ -137,22 +138,22 @@ private BitSet elementBitSet(byte[] doc) throws XMLStreamException {
    * A Slight variation on writeAsUnicode from stax which writes as a regex string so we could
    * rewrite its output
    */
-  private String writeAsRegex(StartElement element) {
+  private String writeAsRegex(final StartElement element, final List<Attribute> orderedAttributes) {
     StringWriter writer = new StringWriter();
 
     writer.write("<");
     writer.write(Pattern.quote(element.getName().getLocalPart()));
 
-    Iterator<?> attrIter = element.getAttributes();
-    while (attrIter.hasNext()) {
-      Attribute attr = (Attribute) attrIter.next();
-
+    for (var attr : orderedAttributes) {
       writer.write("\\s+");
 
       writer.write(Pattern.quote(attr.getName().getLocalPart()));
-      writer.write("=[\\\"\']");
+      writer.write("\\s*");
+      writer.write("=");
+      writer.write("\\s*");
+      writer.write("[\\\"']");
       writer.write(Pattern.quote(attr.getValue()));
-      writer.write("[\\\"\']");
+      writer.write("[\\\"']");
     }
     writer.write("\\s*\\/>");
 
@@ -335,7 +336,25 @@ private void parseXmlAndCharset(POMDocument pomFile) throws XMLStreamException,
               new IntRange(
                   realElementStart, realElementStart + 1 + trimmedOriginalContent.length());
 
-          String contentRe = writeAsRegex(getLastStartElement(prevEvents));
+          var element = getLastStartElement(prevEvents);
+
+          // order the attributes by the original ordering
+          // attributes names are unique, we can just order them by the index of the name
+
+          // Remove attribute contents, just in case some they contain the name of an attribute
+          // TODO should we trim the element name beforehand?
+          String contentRemoved = untrimmedOriginalContent.replaceAll("[\\\"'].*[\\\"']", "");
+
+          var it = element.getAttributes();
+          var orderedAttributes =
+              Stream.iterate(it, Iterator::hasNext, UnaryOperator.identity())
+                  .map(Iterator::next)
+                  .map(a -> new Pair<>(a, contentRemoved.indexOf(a.getName().getLocalPart())))
+                  .sorted(Comparator.comparing(p -> p.getSecond()))
+                  .map(p -> p.getFirst())
+                  .collect(Collectors.toList());
+
+          String contentRe = writeAsRegex(element, orderedAttributes);
 
           Regex modifiedContentRE = new Regex(contentRe);
 
@@ -494,6 +513,8 @@ private byte[] serializePomFile(POMDocument pom) throws XMLStreamException {
     // Let's find out the original empty elements from the original pom and store into a stack
     List<MatchData> elementsToReplace = getElementsToReplace(originalElementMap, pom);
 
+    // DOM parsers don't guarantee attribute ordering, extract the original ordering for the regex
+
     // Lets to the replacements backwards on the existing, current pom
     Map<Integer, MatchData> emptyElements = getEmptyElements(targetElementMap, xmlRepresentation);
 

plugins/codemodder-plugin-maven/src/test/java/io/codemodder/plugins/maven/operator/AbstractTestBase.java

@@ -59,7 +59,7 @@ protected ProjectModel performAndAssertPomOperation(
       String testName, ProjectModel context, final OperationType operationType) throws Exception {
 
     String resultFile = "pom-" + testName + "-result.xml";
-    URL resource = AbstractTestBase.class.getClass().getResource(resultFile);
+    URL resource = AbstractTestBase.class.getResource(resultFile);
 
     if (resource != null) {
       Document outcome = new SAXReader().read(resource);

plugins/codemodder-plugin-maven/src/test/java/io/codemodder/plugins/maven/operator/POMOperatorTest.java

@@ -581,6 +581,19 @@ void modify_adds_dependency_to_file_with_tabs()
             "\n\t\t<dependency>\n\t\t\t<groupId>org.dom4j</groupId>\n\t\t\t<artifactId>dom4j</artifactId>\n\t\t</dependency>\n");
   }
 
+  @Test
+  void modify_adds_dependency_to_pom_with_empty_elements_with_multiple_attributes()
+      throws Exception {
+    Dependency dependencyToUpgrade =
+        new Dependency("io.github.pixee", "java-security-toolkit", "1.0.2", null, null, null);
+
+    performAndAssertModifyPomOperation(
+        "trimmed-roller",
+        ProjectModelFactory.load(POMOperatorTest.class.getResource("pom-trimmed-roller.xml"))
+            .withDependency(dependencyToUpgrade)
+            .withUseProperties(true));
+  }
+
   /**
    * Tests a scenario with an empty element from a customer's POM file and validates the resultant
    * POM.

plugins/codemodder-plugin-maven/src/test/resources/io/codemodder/plugins/maven/operator/pom-trimmed-roller-result.xml

@@ -0,0 +1,113 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+Licensed to the Apache Software Foundation (ASF) under one or more
+contributor license agreements.  See the NOTICE file distributed with
+this work for additional information regarding copyright ownership.
+The ASF licenses this file to You 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.
+-->
+<project xmlns="http://maven.apache.org/POM/4.0.0"
+         xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+         xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
+
+    <dependencies>
+        <dependency>
+            <groupId>org.instancio</groupId>
+            <artifactId>instancio-junit</artifactId>
+            <version>5.0.1</version>
+            <scope>test</scope>
+        </dependency>
+        <dependency>
+            <groupId>io.github.pixee</groupId>
+            <artifactId>java-security-toolkit</artifactId>
+        </dependency>
+    </dependencies>
+
+    <build>
+
+        <finalName>roller</finalName>
+
+        <plugins>
+
+            <plugin>
+                <artifactId>maven-antrun-plugin</artifactId>
+                <version>3.1.0</version>
+                <dependencies>
+                    <dependency>
+                        <groupId>ant-contrib</groupId>
+                        <artifactId>ant-contrib</artifactId>
+                        <version>${maven-antrun.version}</version>
+                        <exclusions>
+                            <exclusion>
+                                <groupId>ant</groupId>
+                                <artifactId>ant</artifactId>
+                            </exclusion>
+                        </exclusions>
+                    </dependency>
+                    <!-- last velocity version which had the Ant TexenTask -->
+                    <dependency>
+                        <groupId>org.apache.velocity</groupId>
+                        <artifactId>velocity</artifactId>
+                        <version>1.7</version>
+                    </dependency>
+                </dependencies>
+
+                <executions>
+                    <execution>
+                        <id>gen-db-scripts</id>
+                        <phase>generate-resources</phase>
+                        <goals>
+                            <goal>run</goal>
+                        </goals>
+                        <configuration>
+                            <target>
+                                <!-- Workaround for git distribution that doesn't keep empty directories.
+                                     For detail check [ROL-2086] -->
+                                <taskdef resource="net/sf/antcontrib/antlib.xml" />
+                                <property file="${basedir}/src/main/resources/sql/dbscripts.properties" />
+                                <for list="${databases}" param="database" delimiter=" ">
+                                    <sequential>
+                                        <mkdir dir="${basedir}/target/classes/dbscripts/@{database}" />
+                                    </sequential>
+                                </for>
+
+                                <taskdef name="texen"
+                                         classname="org.apache.velocity.texen.ant.TexenTask"
+                                         classpathref="maven.plugin.classpath"/>
+                                <texen
+                                    controlTemplate  ="control.vm"
+                                    contextProperties="${basedir}/src/main/resources/sql/dbscripts.properties"
+                                    templatePath     ="${basedir}/src/main/resources/sql"
+                                    outputDirectory  ="${basedir}/target/classes/dbscripts"
+                                    outputFile       ="README.txt"/>
+                            </target>
+                        </configuration>
+                    </execution>
+                </executions>
+            </plugin>
+
+        </plugins>
+
+    </build>
+    <dependencyManagement>
+        <dependencies>
+            <dependency>
+                <groupId>io.github.pixee</groupId>
+                <artifactId>java-security-toolkit</artifactId>
+                <version>${versions.java-security-toolkit}</version>
+            </dependency>
+        </dependencies>
+    </dependencyManagement>
+    <properties>
+        <versions.java-security-toolkit>1.0.2</versions.java-security-toolkit>
+    </properties>
+</project>