Some fixes for XMLTransformer and Pipeline (#585)

Author: andrecsilva

src/codemodder/codemods/xml_transformer.py

@@ -3,7 +3,7 @@
 from xml.sax import SAXParseException, handler
 from xml.sax.handler import LexicalHandler
 from xml.sax.saxutils import XMLGenerator
-from xml.sax.xmlreader import Locator
+from xml.sax.xmlreader import AttributesImpl, Locator
 
 from defusedxml.sax import make_parser
 
@@ -107,9 +107,9 @@ def __init__(
         super().__init__(out, encoding, short_empty_elements, results)
 
     def startElement(self, name, attrs):
-        new_attrs = attrs
+        new_attrs: AttributesImpl = attrs
         if self.event_match_result() and name in self.name_attributes_map:
-            new_attrs = self.name_attributes_map[name]
+            new_attrs = AttributesImpl(attrs._attrs | self.name_attributes_map[name])
             self.add_change(self._my_locator.getLineNumber())
         super().startElement(name, new_attrs)
 
@@ -150,19 +150,18 @@ def apply(
                 return None
 
             diff = ""
-            with open(file_context.file_path, "r") as original:
-                # don't calculate diff if no changes were reported
-                # TODO there's a failure potential here for very large files
-                diff = (
-                    create_diff(
+            # don't calculate diff if no changes were reported
+            if changes:
+                with open(file_context.file_path, "r") as original:
+                    # TODO there's a failure potential here for very large files
+                    diff = create_diff(
                         original.readlines(),
                         output_file.readlines(),
                     )
-                    if changes
-                    else ""
-                )
 
-            if not context.dry_run:
+            # don't write anything if no changes were issued
+            # avoids simply formatting the file
+            if changes and not context.dry_run:
                 with open(file_context.file_path, "w+b") as original:
                     # mmap can't map empty files, write something first
                     original.write(b"a")

tests/test_xml_transformer.py

@@ -75,12 +75,22 @@ def test_change_single_attr(self):
         name_attr_map = {"element": {"attr": "true"}}
         self.run_and_assert(name_attr_map, input_code, expected_output)
 
-    def test_change_multiple_attr(self):
+    def test_add_new_attribute(self):
         input_code = """\
                 <?xml version="1.0" encoding="utf-8"?>
-                <element first="1" second="2"></element>"""
+                <element></element>"""
+        expected_output = """\
+                <?xml version="1.0" encoding="utf-8"?>
+                <element attr="true"></element>"""
+        name_attr_map = {"element": {"attr": "true"}}
+        self.run_and_assert(name_attr_map, input_code, expected_output)
+
+    def test_change_multiple_attr_and_preserve_existing(self):
+        input_code = """\
+                <?xml version="1.0" encoding="utf-8"?>
+                <element first="1" second="2" three="three"></element>"""
         expected_output = """\
                 <?xml version="1.0" encoding="utf-8"?>
-                <element first="one" second="two"></element>"""
+                <element first="one" second="two" three="three"></element>"""
         name_attr_map = {"element": {"first": "one", "second": "two"}}
         self.run_and_assert(name_attr_map, input_code, expected_output)