Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Renaming classes assigns wrong references #967

Merged
merged 1 commit into from
Mar 5, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 11 additions & 13 deletions tests/codegen/handlers/test_rename_duplicate_classes.py
Original file line number Diff line number Diff line change
Expand Up @@ -114,22 +114,20 @@ def test_merge_classes(self, mock_update_class_references):
self.container.extend([first, second, third, fourth, fifth])
self.processor.run()

replacements = {
id(second): id(first),
id(third): id(first),
}
search = {first.ref, second.ref}
replace = third.ref

mock_update_class_references.assert_has_calls(
[
mock.call(first, replacements),
mock.call(fourth, replacements),
mock.call(fifth, replacements),
mock.call(first, search, replace),
mock.call(fourth, search, replace),
mock.call(fifth, search, replace),
]
)
self.assertEqual([first, fourth, fifth], list(self.container))

def test_update_class_references(self):
replacements = {1: 2, 3: 4, 5: 6, 7: 8}
replacements = {1, 2, 3, 4}
target = ClassFactory.create(
attrs=AttrFactory.list(3),
extensions=ExtensionFactory.list(2),
Expand All @@ -138,12 +136,12 @@ def test_update_class_references(self):
target.attrs[1].choices = AttrFactory.list(2)

target.attrs[0].types[0].reference = 1
target.attrs[1].choices[0].types[0].reference = 3
target.extensions[1].type.reference = 5
target.inner[0].attrs[0].types[0].reference = 7
target.attrs[1].choices[0].types[0].reference = 2
target.extensions[1].type.reference = 3
target.inner[0].attrs[0].types[0].reference = 4

self.processor.update_class_references(target, replacements)
self.assertEqual([6, 2, 4, 8], list(target.references))
self.processor.update_class_references(target, replacements, 5)
self.assertEqual([5, 5, 5, 5], list(target.references))

@mock.patch.object(RenameDuplicateClasses, "rename_class")
def test_rename_classes(self, mock_rename_class):
Expand Down
13 changes: 8 additions & 5 deletions xsdata/codegen/container.py
Original file line number Diff line number Diff line change
Expand Up @@ -241,13 +241,16 @@ def add(self, item: Class):
"""
self.data.setdefault(item.qname, []).append(item)

def remove(self, item: Class):
"""Remove class instance from to the container.
def remove(self, *items: Class):
"""Safely remove classes from the container.

Args:
item: The class instances to remove
items: The classes to remove
"""
self.data[item.qname].remove(item)
for item in items:
self.data[item.qname] = [
c for c in self.data[item.qname] if c.ref != item.ref
]

def reset(self, item: Class, qname: str):
"""Update the given class qualified name.
Expand All @@ -256,7 +259,7 @@ def reset(self, item: Class, qname: str):
item: The target class instance to update
qname: The new qualified name of the class
"""
self.data[qname].remove(item)
self.data[qname] = [c for c in self.data[qname] if c.ref != item.ref]
self.add(item)

def set(self, items: List[Class]):
Expand Down
27 changes: 12 additions & 15 deletions xsdata/codegen/handlers/rename_duplicate_classes.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
from typing import Dict, List
from typing import List, Set

from xsdata.codegen.mixins import ContainerHandlerInterface
from xsdata.codegen.models import (
Expand Down Expand Up @@ -54,16 +54,13 @@ def merge_classes(self, classes: List[Class]):
Args:
classes: A list of duplicate classes
"""
keep = classes[0]
new = keep.ref

replacements = {}
for item in classes[1:]:
replacements[item.ref] = new
self.container.remove(item)
keep = classes.pop()
replace = keep.ref
self.container.remove(*classes)
search = {item.ref for item in classes}

for item in self.container:
self.update_class_references(item, replacements)
self.update_class_references(item, search, replace)

def rename_classes(self, classes: List[Class], use_name: bool):
"""Rename all the classes in the list.
Expand Down Expand Up @@ -127,18 +124,18 @@ def next_qname(self, namespace: str, name: str, use_name: bool) -> str:
if cmp not in reserved:
return qname

def update_class_references(self, target: Class, replacements: Dict[int, int]):
def update_class_references(self, target: Class, search: Set[int], replace: int):
"""Go through all class types and update all references.

Args:
target: The target class instance to update
replacements: A mapping of old-new class references
search: A set of class references to find
replace: The new class reference to replace
"""

def update_maybe(attr_type: AttrType):
exists = replacements.get(attr_type.reference)
if exists:
attr_type.reference = exists
if attr_type.reference in search:
attr_type.reference = replace

for attr in target.attrs:
for tp in attr.types:
Expand All @@ -152,7 +149,7 @@ def update_maybe(attr_type: AttrType):
update_maybe(ext.type)

for inner in target.inner:
self.update_class_references(inner, replacements)
self.update_class_references(inner, search, replace)

def rename_class_dependencies(self, target: Class, reference: int, replace: str):
"""Search and replace the old qualified class name in all classes.
Expand Down
6 changes: 3 additions & 3 deletions xsdata/codegen/mixins.py
Original file line number Diff line number Diff line change
Expand Up @@ -77,11 +77,11 @@ def add(self, item: Class):
"""

@abc.abstractmethod
def remove(self, item: Class):
"""Remove class instance from the container.
def remove(self, *items: Class):
"""Safely remove classes from the container.

Args:
item: The class instances to remove
items: The classes to remove
"""

@abc.abstractmethod
Expand Down
Loading