fix: cleanup code

json2xml/dicttoxml.py

@@ -8,12 +8,16 @@
 from typing import Any, Union
 
 from defusedxml.minidom import parseString
+from xml.parsers.expat import ExpatError
 
 # Create a safe random number generator
 
 # Set up logging
 LOG = logging.getLogger("dicttoxml")
 
+# Module-level set for true uniqueness tracking
+_used_ids: set[str] = set()
+
 
 def make_id(element: str, start: int = 100000, end: int = 999999) -> str:
     """
@@ -41,16 +45,11 @@ def get_unique_id(element: str) -> str:
     Returns:
         str: The unique ID.
     """
-    ids: list[str] = []  # initialize list of unique ids
     this_id = make_id(element)
-    dup = True
-    while dup:
-        if this_id not in ids:
-            dup = False
-            ids.append(this_id)
-        else:
-            this_id = make_id(element)
-    return ids[-1]
+    while this_id in _used_ids:
+        this_id = make_id(element)
+    _used_ids.add(this_id)
+    return this_id
 
 
 ELEMENT = Union[
@@ -77,23 +76,22 @@ def get_xml_type(val: ELEMENT) -> str:
     Returns:
         str: The XML type.
     """
-    if val is not None:
-        if type(val).__name__ in ("str", "unicode"):
-            return "str"
-        if type(val).__name__ in ("int", "long"):
-            return "int"
-        if type(val).__name__ == "float":
-            return "float"
-        if type(val).__name__ == "bool":
-            return "bool"
-        if isinstance(val, numbers.Number):
-            return "number"
-        if isinstance(val, dict):
-            return "dict"
-        if isinstance(val, Sequence):
-            return "list"
-    else:
+    if val is None:
         return "null"
+    if isinstance(val, bool):  # Check bool before int (bool is subclass of int)
+        return "bool"
+    if isinstance(val, int):
+        return "int"
+    if isinstance(val, float):
+        return "float"
+    if isinstance(val, str):
+        return "str"
+    if isinstance(val, numbers.Number):
+        return "number"
+    if isinstance(val, dict):
+        return "dict"
+    if isinstance(val, Sequence):
+        return "list"
     return type(val).__name__
 
 
@@ -102,19 +100,19 @@ def escape_xml(s: str | int | float | numbers.Number) -> str:
     Escape a string for use in XML.
 
     Args:
-        s (str | numbers.Number): The string to escape.
+        s (str | int | float | numbers.Number): The string to escape.
 
     Returns:
         str: The escaped string.
     """
+    s_str = str(s)  # Convert to string once
     if isinstance(s, str):
-        s = str(s)  # avoid UnicodeDecodeError
-        s = s.replace("&", "&")
-        s = s.replace('"', """)
-        s = s.replace("'", "'")
-        s = s.replace("<", "&lt;")
-        s = s.replace(">", "&gt;")
-    return str(s)
+        s_str = s_str.replace("&", "&amp;")
+        s_str = s_str.replace('"', "&quot;")
+        s_str = s_str.replace("'", "&apos;")
+        s_str = s_str.replace("<", "&lt;")
+        s_str = s_str.replace(">", "&gt;")
+    return s_str
 
 
 def make_attrstring(attr: dict[str, Any]) -> str:
@@ -145,37 +143,39 @@ def key_is_valid_xml(key: str) -> bool:
     try:
         parseString(test_xml)
         return True
-    except Exception:  # minidom does not implement exceptions well
+    except (ExpatError, ValueError) as e:
+        LOG.debug(f"Invalid XML name '{key}': {e}")
         return False
 
 
-def make_valid_xml_name(key: str, attr: dict[str, Any]) -> tuple[str, dict[str, Any]]:
+def make_valid_xml_name(key: str | int, attr: dict[str, Any]) -> tuple[str, dict[str, Any]]:
     """Tests an XML name and fixes it if invalid"""
-    key = escape_xml(key)
+    key_str = str(key)  # Ensure we're working with strings
+    key_str = escape_xml(key_str)
     # nothing happens at escape_xml if attr is not a string, we don't
     # need to pass it to the method at all.
     # attr = escape_xml(attr)
 
     # pass through if key is already valid
-    if key_is_valid_xml(key):
-        return key, attr
+    if key_is_valid_xml(key_str):
+        return key_str, attr
 
     # prepend a lowercase n if the key is numeric
-    if isinstance(key, int) or key.isdigit():
-        return f"n{key}", attr
+    if key_str.isdigit():
+        return f"n{key_str}", attr
 
     # replace spaces with underscores if that fixes the problem
-    if key_is_valid_xml(key.replace(" ", "_")):
-        return key.replace(" ", "_"), attr
+    if key_is_valid_xml(key_str.replace(" ", "_")):
+        return key_str.replace(" ", "_"), attr
 
     # allow namespace prefixes + ignore @flat in key
-    if key_is_valid_xml(key.replace(":", "").replace("@flat", "")):
-        return key, attr
+    if key_is_valid_xml(key_str.replace(":", "").replace("@flat", "")):
+        return key_str, attr
 
     # key is still invalid - move it into a name attribute
-    attr["name"] = key
-    key = "key"
-    return key, attr
+    attr["name"] = key_str
+    key_str = "key"
+    return key_str, attr
 
 
 def wrap_cdata(s: str | int | float | numbers.Number) -> str:
@@ -188,6 +188,25 @@ def default_item_func(parent: str) -> str:
     return "item"
 
 
+def _build_namespace_string(xml_namespaces: dict[str, Any]) -> str:
+    """Build XML namespace string from namespace dictionary."""
+    parts = []
+
+    for prefix, value in xml_namespaces.items():
+        if prefix == 'xsi' and isinstance(value, dict):
+            for schema_att, ns in value.items():
+                if schema_att == 'schemaInstance':
+                    parts.append(f'xmlns:{prefix}="{ns}"')
+                elif schema_att == 'schemaLocation':
+                    parts.append(f'xsi:{schema_att}="{ns}"')
+        elif prefix == 'xmlns':
+            parts.append(f'xmlns="{value}"')
+        else:
+            parts.append(f'xmlns:{prefix}="{value}"')
+
+    return ' ' + ' '.join(parts) if parts else ''
+
+
 def convert(
     obj: ELEMENT,
     ids: Any,
@@ -262,7 +281,6 @@ def dict2xml_str(
     parse dict2xml
     """
     ids: list[str] = []  # initialize list of unique ids
-    ", ".join(str(key) for key in item)
     subtree = ""  # Initialize subtree with default empty string
 
     if attr_type:
@@ -562,7 +580,7 @@ def dicttoxml(
     item_wrap: bool = True,
     item_func: Callable[[str], str] = default_item_func,
     cdata: bool = False,
-    xml_namespaces: dict[str, Any] = {},
+    xml_namespaces: dict[str, Any] | None = None,
     list_headers: bool = False
 ) -> bytes:
     """
@@ -681,26 +699,11 @@ def dicttoxml(
         <list a="b" c="d"><item>4</item><item>5</item><item>6</item></list>
 
     """
+    if xml_namespaces is None:
+        xml_namespaces = {}
+
     output = []
-    namespace_str = ""
-    for prefix in xml_namespaces:
-        if prefix == 'xsi':
-            for schema_att in xml_namespaces[prefix]:
-                if schema_att == 'schemaInstance':
-                    ns = xml_namespaces[prefix]['schemaInstance']
-                    namespace_str += f' xmlns:{prefix}="{ns}"'
-                elif schema_att == 'schemaLocation':
-                    ns = xml_namespaces[prefix][schema_att]
-                    namespace_str += f' xsi:{schema_att}="{ns}"'
-
-        elif prefix == 'xmlns':
-            # xmns needs no prefix
-            ns = xml_namespaces[prefix]
-            namespace_str += f' xmlns="{ns}"'
-
-        else:
-            ns = xml_namespaces[prefix]
-            namespace_str += f' xmlns:{prefix}="{ns}"'
+    namespace_str = _build_namespace_string(xml_namespaces)
     if root:
         output.append('<?xml version="1.0" encoding="UTF-8" ?>')
         output_elem = convert(

pyproject.toml

@@ -63,7 +63,6 @@ lint.ignore = [
     "E501",  # line too long
     "F403",  # 'from module import *' used; unable to detect undefined names
     "E701",  # multiple statements on one line (colon)
-    "F401",  # module imported but unused
 ]
 line-length = 119
 lint.select = [

tests/test_dict2xml.py

@@ -1,18 +1,10 @@
 import datetime
-import numbers
 from typing import TYPE_CHECKING, Any
 
 import pytest
 
 from json2xml import dicttoxml
 
-if TYPE_CHECKING:
-    from _pytest.capture import CaptureFixture
-    from _pytest.fixtures import FixtureRequest
-    from _pytest.logging import LogCaptureFixture
-    from _pytest.monkeypatch import MonkeyPatch
-    from pytest_mock.plugin import MockerFixture
-
 
 class TestDict2xml:
     """Test class for dicttoxml functionality."""
@@ -1143,3 +1135,59 @@
         empty_attrs: dict[str, Any] = {}
         result = make_attrstring(empty_attrs)
         assert result == ""
+
+    def test_get_unique_id_collision_coverage(self) -> None:
+        """Test get_unique_id to cover line 50 - the collision case."""
+        import json2xml.dicttoxml as module
+
+        # Clear the global _used_ids set to start fresh
+        original_used_ids = module._used_ids.copy()
+        module._used_ids.clear()
+
+        # Mock make_id to return the same ID twice, then a different one
+        original_make_id = module.make_id
+        call_count = 0
+
+        def mock_make_id(element: str, start: int = 100000, end: int = 999999) -> str:
+            nonlocal call_count
+            call_count += 1
+            if call_count == 1:
+                return "test_123456"  # First call - will be added to _used_ids
+            elif call_count == 2:
+                return "test_123456"  # Second call - will collide, triggering line 50
+            else:
+                return "test_789012"  # Third call - unique
+
+        module.make_id = mock_make_id
+
+        try:
+            # First call - adds "test_123456" to _used_ids
+            result1 = dicttoxml.get_unique_id("test")
+            assert result1 == "test_123456"
+
+            # Reset call count for second test
+            call_count = 0
+
+            # Second call - should trigger collision and regenerate
+            result2 = dicttoxml.get_unique_id("test")
+            assert result2 == "test_789012"
+            assert call_count == 3  # Should have called make_id 3 times
+        finally:
+            module.make_id = original_make_id
+            module._used_ids.clear()
+            module._used_ids.update(original_used_ids)
+
+    def test_get_xml_type_numbers_number_coverage(self) -> None:
+        """Test get_xml_type to cover line 90 - numbers.Number that's not int/float."""
+        import decimal
+        import fractions
+
+        # Test with Decimal (numbers.Number but not int/float)
+        decimal_val = decimal.Decimal('3.14159')
+        result = dicttoxml.get_xml_type(decimal_val)
+        assert result == "number"
+
+        # Test with Fraction (numbers.Number but not int/float)
+        fraction_val = fractions.Fraction(22, 7)
+        result = dicttoxml.get_xml_type(fraction_val)
+        assert result == "number"

tests/test_json2xml.py

@@ -2,7 +2,6 @@
 
 """Tests for `json2xml` package."""
 
-import json
 from pyexpat import ExpatError
 
 import pytest

tests/test_utils.py

@@ -16,13 +16,6 @@
     readfromurl,
 )
 
-if TYPE_CHECKING:
-    from _pytest.capture import CaptureFixture
-    from _pytest.fixtures import FixtureRequest
-    from _pytest.logging import LogCaptureFixture
-    from _pytest.monkeypatch import MonkeyPatch
-    from pytest_mock.plugin import MockerFixture
-
 
 class TestExceptions:
     """Test custom exception classes."""