Skip to content

Commit dca262b

Browse files
tefratefra
authored
Fix elements in same choice different branches incorrectly merged as list (#1206)
When an element appears in multiple branches of the same xs:choice at different nesting levels, they should be treated as mutually exclusive (only one branch is selected at runtime), not additive.
1 parent b5e42df commit dca262b

File tree

4 files changed

+136
-17
lines changed

4 files changed

+136
-17
lines changed

‎tests/codegen/handlers/test_merge_attributes.py‎

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -108,3 +108,34 @@ def test_process_elements_in_different_choice_groups(self) -> None:
108108
self.assertEqual("configOption", target.attrs[0].name)
109109
self.assertEqual(0, target.attrs[0].restrictions.min_occurs)
110110
self.assertEqual(1, target.attrs[0].restrictions.max_occurs)
111+
112+
def test_process_elements_in_same_choice_different_branches(self) -> None:
113+
"""Test that elements in the same choice but different branches use max.
114+
115+
This tests the case where an element appears in multiple branches of the
116+
same choice. Since only one branch is selected at runtime, they are
117+
mutually exclusive and max_occurs should use max(), not sum().
118+
"""
119+
choice_id = 12345
120+
121+
attr1 = AttrFactory.element(
122+
name="b",
123+
index=10,
124+
restrictions=Restrictions(min_occurs=0, max_occurs=1, choice=choice_id),
125+
)
126+
attr2 = AttrFactory.element(
127+
name="b",
128+
index=11,
129+
restrictions=Restrictions(min_occurs=1, max_occurs=1, choice=choice_id),
130+
)
131+
132+
target = ClassFactory.create(attrs=[attr1, attr2])
133+
134+
self.processor.process(target)
135+
136+
# Should merge into one attr with max_occurs=1 (not 2)
137+
# because both are in different branches of the same choice
138+
self.assertEqual(1, len(target.attrs))
139+
self.assertEqual("b", target.attrs[0].name)
140+
self.assertEqual(0, target.attrs[0].restrictions.min_occurs)
141+
self.assertEqual(1, target.attrs[0].restrictions.max_occurs)

‎tests/codegen/handlers/test_update_attributes_effective_choice.py‎

Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -176,3 +176,82 @@ def test_process_elements_in_different_choice_blocks(self) -> None:
176176
["configOption", "optionA", "configOption", "optionB"],
177177
[x.name for x in target.attrs],
178178
)
179+
180+
def test_process_elements_in_same_choice_different_branches(self) -> None:
181+
"""Test that elements in same choice but different branches are not merged.
182+
183+
This tests the case where an element appears in multiple branches of the
184+
same choice, but at different nesting levels (different path lengths).
185+
Example XSD pattern:
186+
<xs:choice>
187+
<xs:sequence>
188+
<xs:element name="a"/>
189+
<xs:element name="b" minOccurs="0"/>
190+
</xs:sequence>
191+
<xs:element name="b"/>
192+
</xs:choice>
193+
The two 'b' elements are mutually exclusive (different branches of the
194+
same choice) and should not be grouped as repeating elements.
195+
"""
196+
choice_id = 12345
197+
outer_seq_id = 11111
198+
inner_seq_id = 22222
199+
200+
target = ClassFactory.create(
201+
attrs=[
202+
AttrFactory.element(
203+
name="a",
204+
namespace="ns",
205+
restrictions=Restrictions(
206+
min_occurs=1,
207+
max_occurs=1,
208+
choice=choice_id,
209+
sequence=outer_seq_id,
210+
path=[
211+
("s", outer_seq_id, 0, 1),
212+
("c", choice_id, 1, 1),
213+
("s", inner_seq_id, 1, 1),
214+
],
215+
),
216+
),
217+
AttrFactory.element(
218+
name="b",
219+
namespace="ns",
220+
restrictions=Restrictions(
221+
min_occurs=0,
222+
max_occurs=1,
223+
choice=choice_id,
224+
sequence=outer_seq_id,
225+
path=[
226+
("s", outer_seq_id, 0, 1),
227+
("c", choice_id, 1, 1),
228+
("s", inner_seq_id, 1, 1),
229+
],
230+
),
231+
),
232+
AttrFactory.element(
233+
name="b",
234+
namespace="ns",
235+
restrictions=Restrictions(
236+
min_occurs=1,
237+
max_occurs=1,
238+
choice=choice_id,
239+
sequence=outer_seq_id,
240+
path=[
241+
("s", outer_seq_id, 0, 1),
242+
("c", choice_id, 1, 1),
243+
],
244+
),
245+
),
246+
]
247+
)
248+
249+
self.processor.process(target)
250+
251+
# Should not merge 'b' elements since they have different path lengths
252+
# (different nesting levels within the same choice)
253+
self.assertEqual(3, len(target.attrs))
254+
self.assertEqual(["a", "b", "b"], [x.name for x in target.attrs])
255+
# Both 'b' elements should retain their original max_occurs=1
256+
self.assertEqual(1, target.attrs[1].restrictions.max_occurs)
257+
self.assertEqual(1, target.attrs[2].restrictions.max_occurs)

‎xsdata/codegen/handlers/merge_attributes.py‎

Lines changed: 11 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -65,21 +65,20 @@ def merge_duplicate_attrs(cls, target: Class):
6565

6666
e_res.min_occurs = min(min_occurs, attr_min_occurs)
6767

68-
# If attrs are in different choice groups AND have different
69-
# indices AND have the same path depth, they're in sibling
70-
# choice blocks (mutually exclusive) -> use max
71-
# If attrs have same index, they're substitutions for same
72-
# element -> use +
73-
# If attrs have different path depths, one is nested deeper
74-
# -> use +
75-
# Otherwise, use + (default behavior for sequences)
76-
if (
68+
# Determine if attrs are mutually exclusive (max) or additive (sum)
69+
is_mutually_exclusive = (
7770
e_res.choice is not None
7871
and a_res.choice is not None
79-
and e_res.choice != a_res.choice
8072
and existing.index != attr.index
81-
and len(e_res.path) == len(a_res.path)
82-
):
73+
and (
74+
# Same choice, different branches
75+
e_res.choice == a_res.choice
76+
# Different sibling choices at same depth
77+
or len(e_res.path) == len(a_res.path)
78+
)
79+
)
80+
81+
if is_mutually_exclusive:
8382
e_res.max_occurs = max(max_occurs, attr_max_occurs)
8483
else:
8584
e_res.max_occurs = max_occurs + attr_max_occurs

‎xsdata/codegen/handlers/update_attributes_effective_choice.py‎

Lines changed: 15 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -149,11 +149,21 @@ def group_repeating_attrs(cls, target: Class) -> list[list[int]]:
149149
# If they have different choice IDs (and both are not None),
150150
# they're in different choice blocks and shouldn't be merged
151151
choice_ids = {target.attrs[i].restrictions.choice for i in indices}
152-
# Allow grouping if:
153-
# 1. All have the same choice ID, OR
154-
# 2. At least one has choice=None (not in a choice block)
155-
if len(choice_ids) == 1 or None in choice_ids:
156-
# Group them
152+
153+
# Also check if all occurrences have the same path length
154+
# If they have different path lengths within the same choice,
155+
# they're in different branches (mutually exclusive)
156+
path_lengths = {len(target.attrs[i].restrictions.path) for i in indices}
157+
158+
if None in choice_ids:
159+
# At least one element is outside a choice, so they can co-exist
160+
# (e.g., one in outer sequence, one in inner choice branch)
161+
result.append(list(range(indices[0], indices[-1] + 1)))
162+
elif len(choice_ids) == 1 and len(path_lengths) == 1:
163+
# All elements in the same choice at the same nesting level
164+
# This is a repeating pattern, group them
157165
result.append(list(range(indices[0], indices[-1] + 1)))
166+
# else: different choices or same choice but different nesting levels
167+
# = mutually exclusive branches, don't group
158168

159169
return result

0 commit comments

Comments
 (0)