Repository: tefra/xsdata
Pull request #1127 (2025-03-09 15:30:34)

Split rename and merge logic for duplicate classes
## 📒 Description

Extract merge duplicate classes handler from the rename duplicate handler, because at the moment it's missing a lot of valid cases.


Resolves #xxxx

## 🔗 What I've Done

> Write a description of the steps taken to resolve the issue

## 💬 Comments

> A place to write any comments to the reviewer.

## 🛫 Checklist

- [ ] Updated docs
- [ ] Added unit-tests
- [ ] [Sample tests](https://github.com/tefra/xsdata-samples) pass
- [ ] [W3C tests](https://github.com/tefra/xsdata-w3c-tests) pass


sonarqubecloud[bot]: ## [![Quality Gate Passed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/qg-passed-20px.png 'Quality Gate Passed')](https://sonarcloud.io/dashboard?id=tefra_xsdata&pullRequest=1127) **Quality Gate passed**  
Issues  
![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [0 New issues](https://sonarcloud.io/project/issues?id=tefra_xsdata&pullRequest=1127&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)  
![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/accepted-16px.png '') [0 Accepted issues](https://sonarcloud.io/project/issues?id=tefra_xsdata&pullRequest=1127&issueStatuses=ACCEPTED)

Measures  
![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=tefra_xsdata&pullRequest=1127&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)  
![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=tefra_xsdata&pullRequest=1127&metric=new_coverage&view=list)  
![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=tefra_xsdata&pullRequest=1127&metric=new_duplicated_lines_density&view=list)  
  
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=tefra_xsdata&pullRequest=1127)


codecov[bot]: ## [Codecov](https://app.codecov.io/gh/tefra/xsdata/pull/1127?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Christodoulos+Tsoulloftas) Report
All modified and coverable lines are covered by tests :white_check_mark:
> Project coverage is 99.94%. Comparing base [(`bb0f69c`)](https://app.codecov.io/gh/tefra/xsdata/commit/bb0f69c46a331c80905e4615496a4b581fff2f1b?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Christodoulos+Tsoulloftas) to head [(`2317ca7`)](https://app.codecov.io/gh/tefra/xsdata/commit/2317ca7e948f3513a52b5ee6641e591c58e473a0?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Christodoulos+Tsoulloftas).

<details><summary>Additional details and impacted files</summary>


```diff
@@            Coverage Diff             @@
##             main    #1127      +/-   ##
==========================================
+ Coverage   99.87%   99.94%   +0.07%     
==========================================
  Files         115      116       +1     
  Lines        9321     9356      +35     
  Branches     1419     1428       +9     
==========================================
+ Hits         9309     9351      +42     
+ Misses          8        5       -3     
+ Partials        4        0       -4     
```

</details>

[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/tefra/xsdata/pull/1127?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Christodoulos+Tsoulloftas).   
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Christodoulos+Tsoulloftas).

<details><summary>🚀 New features to boost your workflow: </summary>

- ❄ [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
</details>

Patch:
diff --git a/xsdata/codegen/container.py b/xsdata/codegen/container.py
index 02eff708..c14dee28 100644
--- a/xsdata/codegen/container.py
+++ b/xsdata/codegen/container.py
@@ -13,6 +13,7 @@ from xsdata.codegen.handlers import (
     FlattenAttributeGroups,
     FlattenClassExtensions,
     MergeAttributes,
+    MergeDuplicateClasses,
     ProcessAttributeTypes,
     ProcessMixedContentClass,
     RenameDuplicateAttributes,
@@ -225,6 +226,7 @@ class ClassContainer(ContainerInterface):
     def designate_classes(self) -> None:
         """Designate the final class names, packages and modules."""
         designators = [
+            MergeDuplicateClasses(self),
             RenameDuplicateClasses(self),
             ValidateReferences(self),
             DesignateClassPackages(self),
diff --git a/xsdata/codegen/handlers/__init__.py b/xsdata/codegen/handlers/__init__.py
index eae3879b..5e295a4d 100644
--- a/xsdata/codegen/handlers/__init__.py
+++ b/xsdata/codegen/handlers/__init__.py
@@ -9,6 +9,7 @@ from .filter_classes import FilterClasses
 from .flatten_attribute_groups import FlattenAttributeGroups
 from .flatten_class_extensions import FlattenClassExtensions
 from .merge_attributes import MergeAttributes
+from .merge_duplicate_classes import MergeDuplicateClasses
 from .process_attributes_types import ProcessAttributeTypes
 from .process_mixed_content_class import ProcessMixedContentClass
 from .rename_duplicate_attributes import RenameDuplicateAttributes
@@ -35,6 +36,7 @@ __all__ = [
     "FlattenAttributeGroups",
     "FlattenClassExtensions",
     "MergeAttributes",
+    "MergeDuplicateClasses",
     "ProcessAttributeTypes",
     "ProcessMixedContentClass",
     "RenameDuplicateAttributes",
diff --git a/xsdata/codegen/handlers/merge_duplicate_classes.py b/xsdata/codegen/handlers/merge_duplicate_classes.py
new file mode 100644
index 00000000..c7e49266
--- /dev/null
+++ b/xsdata/codegen/handlers/merge_duplicate_classes.py
@@ -0,0 +1,73 @@
+from xsdata.codegen.mixins import ContainerHandlerInterface, ContainerInterface
+from xsdata.codegen.models import (
+    Class,
+)
+
+
+class MergeDuplicateClasses(ContainerHandlerInterface):
+    """Remove duplicate classes.
+
+    Args:
+        container: The class container instance
+
+    Attributes:
+        merges: The merge instructions applied at the end
+    """
+
+    __slots__ = ("merges",)
+
+    def __init__(self, container: ContainerInterface):
+        """Initialize the rename duplicate class handler."""
+        super().__init__(container)
+
+        self.merges: dict[int, int] = {}
+
+    def run(self) -> None:
+        """Detect and merge duplicate classes conflicts."""
+        for classes in self.group_duplicate_classes():
+            if len(classes) > 1:
+                self.merge_classes(classes)
+
+        if self.merges:
+            for target in self.container:
+                self.update_references(target)
+
+    def group_duplicate_classes(self) -> list[list[Class]]:
+        """Group duplicate classes.
+
+        This is very slow, because our gen models are unhashable.
+        """
+        groups: list[list[Class]] = []
+        for cls in self.container:
+            found = False
+            for group in groups:
+                if cls == group[0]:
+                    group.append(cls)
+                    found = True
+                    break
+            if not found:
+                groups.append([cls])
+        return groups
+
+    def merge_classes(self, classes: list[Class]) -> None:
+        """Remove the duplicate classes and schedule updates.
+
+        Args:
+            classes: A list of duplicate classes
+        """
+        keep = classes.pop()
+        replace = keep.ref
+        self.container.remove(*classes)
+
+        for item in classes:
+            self.merges[item.ref] = replace
+
+    def update_references(self, target: Class) -> None:
+        """Search and update the target class for merged references.
+
+        Args:
+            target: The target class instance to inspect and update
+        """
+        for tp in target.types():
+            if tp.reference in self.merges:
+                tp.reference = self.merges[tp.reference]
diff --git a/xsdata/codegen/handlers/rename_duplicate_classes.py b/xsdata/codegen/handlers/rename_duplicate_classes.py
index e1f2355b..e16fb219 100644
--- a/xsdata/codegen/handlers/rename_duplicate_classes.py
+++ b/xsdata/codegen/handlers/rename_duplicate_classes.py
@@ -20,8 +20,7 @@ class RenameDuplicateClasses(ContainerHandlerInterface):
 
     Attributes:
         use_names: Whether names or qualified names should be used to group classes
-        merges: The renamed instructions applied at the end
-        merges: The merge instructions applied at the end
+        renames: The renamed instructions applied at the end
         reserved: The reserved class names or qualified names
     """
 
@@ -42,12 +41,7 @@ class RenameDuplicateClasses(ContainerHandlerInterface):
         groups = collections.group_by(self.container, lambda x: text.alnum(getter(x)))
 
         for classes in groups.values():
-            if len(classes) < 2:
-                continue
-
-            if all(x == classes[0] for x in classes):
-                self.merge_classes(classes)
-            else:
+            if len(classes) > 1:
                 self.rename_classes(classes)
 
         if self.renames or self.merges:
@@ -120,7 +114,7 @@ class RenameDuplicateClasses(ContainerHandlerInterface):
         self.rename_class(target, new_qname)
 
     def rename_class(self, target: Class, new_qname: str) -> None:
-        """Update the class qualified name and update references.
+        """Update the class qualified name and schedule updates.
 
         Args:
             target: The target class to update
@@ -162,7 +156,7 @@ class RenameDuplicateClasses(ContainerHandlerInterface):
         return self.reserved
 
     def update_references(self, target: Class) -> None:
-        """Search and update the target class for renamed and merged references.
+        """Search and update the target class for renamed references.
 
         Args:
             target: The target class instance to inspect and update
@@ -178,6 +172,3 @@ class RenameDuplicateClasses(ContainerHandlerInterface):
                 ):
                     members = text.suffix(parent.default, "::")
                     parent.default = f"@enum@{tp.qname}::{members}"
-
-            elif tp.reference in self.merges:
-                tp.reference = self.merges[tp.reference]


Test patch:
diff --git a/tests/codegen/handlers/test_merge_duplicate_classes.py b/tests/codegen/handlers/test_merge_duplicate_classes.py
new file mode 100644
index 00000000..c545445a
--- /dev/null
+++ b/tests/codegen/handlers/test_merge_duplicate_classes.py
@@ -0,0 +1,46 @@
+from xsdata.codegen.container import ClassContainer
+from xsdata.codegen.handlers import MergeDuplicateClasses
+from xsdata.models.config import GeneratorConfig
+from xsdata.utils.testing import (
+    AttrFactory,
+    ClassFactory,
+    FactoryTestCase,
+)
+
+
+class MergeDuplicateClassesTests(FactoryTestCase):
+    def setUp(self) -> None:
+        super().setUp()
+
+        self.container = ClassContainer(config=GeneratorConfig())
+        self.processor = MergeDuplicateClasses(container=self.container)
+
+    def test_run_merges_same_classes(self) -> None:
+        first = ClassFactory.create()
+        second = first.clone()
+        third = first.clone()
+        fourth = ClassFactory.create()
+        fifth = ClassFactory.create()
+
+        self.container.extend([first, second, third, fourth, fifth])
+        self.processor.run()
+
+        self.assertEqual([first, fourth, fifth], list(self.container))
+        self.assertEqual(
+            {first.ref: third.ref, second.ref: third.ref}, self.processor.merges
+        )
+
+    def test_update_references(self) -> None:
+        target = ClassFactory.create()
+        target.attrs.append(AttrFactory.reference(qname="foo", reference=1))
+        target.attrs.append(
+            AttrFactory.reference(
+                qname="thug",
+                reference=2,
+            )
+        )
+        self.processor.merges = {2: 3}
+        self.processor.update_references(target)
+
+        self.assertEqual(1, target.attrs[0].types[0].reference)
+        self.assertEqual(3, target.attrs[1].types[0].reference)
diff --git a/tests/codegen/handlers/test_rename_duplicate_classes.py b/tests/codegen/handlers/test_rename_duplicate_classes.py
index 35d7262a..c1b8d1f6 100644
--- a/tests/codegen/handlers/test_rename_duplicate_classes.py
+++ b/tests/codegen/handlers/test_rename_duplicate_classes.py
@@ -3,7 +3,7 @@ from unittest import mock
 from xsdata.codegen.container import ClassContainer
 from xsdata.codegen.handlers import RenameDuplicateClasses
 from xsdata.models.config import GeneratorConfig
-from xsdata.models.enums import DataType, Tag
+from xsdata.models.enums import Tag
 from xsdata.utils.testing import (
     AttrFactory,
     ClassFactory,
@@ -41,21 +41,6 @@ class RenameDuplicateClassesTests(FactoryTestCase):
         self.assertEqual("_b_1", classes[3].qname)
         self.assertEqual("b_2", classes[4].qname)
 
-    def test_run_merges_same_classes(self) -> None:
-        first = ClassFactory.create()
-        second = first.clone()
-        third = first.clone()
-        fourth = ClassFactory.create()
-        fifth = ClassFactory.create()
-
-        self.container.extend([first, second, third, fourth, fifth])
-        self.processor.run()
-
-        self.assertEqual([first, fourth, fifth], list(self.container))
-        self.assertEqual(
-            {first.ref: third.ref, second.ref: third.ref}, self.processor.merges
-        )
-
     @mock.patch.object(RenameDuplicateClasses, "add_numeric_suffix")
     def test_rename_classes(self, mock_add_numeric_suffix) -> None:
         classes = [
@@ -155,15 +140,11 @@ class RenameDuplicateClassesTests(FactoryTestCase):
                 reference=3,
             )
         )
-        target.attrs.append(AttrFactory.native(DataType.STRING))
 
         self.processor.renames = {1: "bar", 2: "foo"}
-        self.processor.merges = {3: 4}
         self.processor.update_references(target)
 
         self.assertEqual("bar", target.attrs[0].types[0].qname)
         self.assertEqual("foo", target.attrs[1].types[0].qname)
         self.assertEqual("@enum@foo::member", target.attrs[1].default)
         self.assertEqual("thug", target.attrs[2].types[0].qname)
-        self.assertEqual(4, target.attrs[2].types[0].reference)
-        self.assertEqual(0, target.attrs[3].types[0].reference)