Update XML writing (#599)

* write xml to file normally and test

* remove filepath check

Author: clavedeluna

src/codemodder/codemods/xml_transformer.py

@@ -1,7 +1,6 @@
-import mmap
 from dataclasses import dataclass, field
 from tempfile import TemporaryFile
-from xml.sax import SAXParseException, handler
+from xml.sax import handler
 from xml.sax.handler import LexicalHandler
 from xml.sax.saxutils import XMLGenerator
 from xml.sax.xmlreader import AttributesImpl, Locator
@@ -13,6 +12,7 @@
 from codemodder.context import CodemodExecutionContext
 from codemodder.diff import create_diff
 from codemodder.file_context import FileContext
+from codemodder.logging import logger
 from codemodder.result import Result
 
 
@@ -171,12 +171,7 @@ def apply(
         file_context: FileContext,
         results: list[Result] | None,
     ) -> ChangeSet | None:
-        if file_context.file_path.suffix.lower() not in (".config", ".xml"):
-            return None
-
-        changes = []
         with TemporaryFile("w+") as output_file:
-
             # this will fail fast for files that are not XML
             try:
                 transformer_instance = self.xml_transformer(
@@ -187,42 +182,33 @@ def apply(
                 parser.setProperty(
                     handler.property_lexical_handler, transformer_instance
                 )
-                parser.parse(file_context.file_path)
+                parser.parse(file_path := file_context.file_path)
                 changes = transformer_instance.changes
                 output_file.seek(0)
+            except Exception:
+                file_context.add_failure(
+                    file_path, reason := "Failed to parse XML file"
+                )
+                logger.exception("%s %s", reason, file_path)
+                return None
 
-            except SAXParseException:
+            if not changes:
                 return None
 
-            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(),
-                    )
-
-            # 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")
-                    # copy contents of result into original file
-                    # the snippet below preserves the original file metadata and accounts for large files.
-                    output_file.seek(0)
-                    output_mmap = mmap.mmap(output_file.fileno(), 0)
-
-                    original.truncate()
-                    original_mmap = mmap.mmap(original.fileno(), 0)
-                    original_mmap.resize(len(output_mmap))
-                    original_mmap[:] = output_mmap
-                    original_mmap.flush()
+            new_lines = output_file.readlines()
+            with open(file_path, "r") as original:
+                # TODO there's a failure potential here for very large files
+                diff = create_diff(
+                    original.readlines(),
+                    new_lines,
+                )
+
+            if not context.dry_run:
+                with open(file_path, "w+") as original:
+                    original.writelines(new_lines)
 
             return ChangeSet(
-                path=str(file_context.file_path.relative_to(context.directory)),
+                path=str(file_path.relative_to(context.directory)),
                 diff=diff,
                 changes=changes,
             )

tests/codemods/test_xml_transformer.py

@@ -0,0 +1,69 @@
+from pathlib import Path
+
+from codemodder.codemods.xml_transformer import (
+    ElementAttributeXMLTransformer,
+    XMLTransformerPipeline,
+)
+from codemodder.context import CodemodExecutionContext
+from codemodder.file_context import FileContext
+
+
+def test_transformer_failure(mocker, caplog):
+    mocker.patch(
+        "defusedxml.expatreader.DefusedExpatParser.parse",
+        side_effect=Exception,
+    )
+    file_context = FileContext(
+        "home",
+        Path("test.xml"),
+    )
+    execution_context = CodemodExecutionContext(
+        directory=mocker.MagicMock(),
+        dry_run=True,
+        verbose=False,
+        registry=mocker.MagicMock(),
+        repo_manager=mocker.MagicMock(),
+        path_include=[],
+        path_exclude=[],
+    )
+    transformer = mocker.MagicMock(spec=ElementAttributeXMLTransformer)
+    pipeline = XMLTransformerPipeline(transformer)
+
+    changeset = pipeline.apply(
+        context=execution_context,
+        file_context=file_context,
+        results=None,
+    )
+    assert changeset is None
+    assert "Failed to parse XML file" in caplog.text
+
+
+def test_transformer(mocker):
+    mocker.patch(
+        "defusedxml.expatreader.DefusedExpatParser.parse",
+    )
+    mocker.patch("builtins.open")
+    mocker.patch("codemodder.codemods.xml_transformer.create_diff", return_value="diff")
+    file_context = FileContext(
+        parent_dir := Path("home"),
+        parent_dir / Path("test.xml"),
+    )
+    execution_context = CodemodExecutionContext(
+        directory=parent_dir,
+        dry_run=True,
+        verbose=False,
+        registry=mocker.MagicMock(),
+        repo_manager=mocker.MagicMock(),
+        path_include=[],
+        path_exclude=[],
+    )
+    transformer = mocker.MagicMock(spec=ElementAttributeXMLTransformer)
+    transformer.changes = ["change1", "change2"]
+    pipeline = XMLTransformerPipeline(transformer)
+
+    changeset = pipeline.apply(
+        context=execution_context,
+        file_context=file_context,
+        results=None,
+    )
+    assert changeset is not None