Skip to content

Commit 9fa06c7

Browse files
authored
Fix msgspec field ordering (#3292)
* Fix msgspec field ordering * Address msgspec test failures * Address msgspec review comments * Update msgspec keyword-only expectations * Cover msgspec test helpers * Avoid unused msgspec Meta import * Use e2e golden tests for msgspec fixes
1 parent 963368a commit 9fa06c7

14 files changed

Lines changed: 234 additions & 25 deletions

src/datamodel_code_generator/model/msgspec.py

Lines changed: 14 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -69,11 +69,10 @@ def __str__(self) -> str:
6969
from datamodel_code_generator.reference import Reference
7070

7171

72-
def _has_field_assignment(field: DataModelFieldBase) -> bool:
73-
return (
74-
bool(field.field)
75-
or field.use_default_with_required
76-
or not (field.required or (field.represented_default == "None" and field.strip_default_none))
72+
def has_field_assignment(field: DataModelFieldBase) -> bool:
73+
"""Return whether a msgspec field renders with a default assignment."""
74+
return field.use_default_with_required or not (
75+
field.required or (field.represented_default == "None" and field.strip_default_none)
7776
)
7877

7978

@@ -118,6 +117,7 @@ class Struct(DataModel):
118117
BASE_CLASS_ALIAS: ClassVar[str] = "_Struct"
119118
DEFAULT_IMPORTS: ClassVar[tuple[Import, ...]] = ()
120119
SUPPORTS_DISCRIMINATOR: ClassVar[bool] = True
120+
SUPPORTS_KW_ONLY: ClassVar[bool] = True
121121
CONFIG_MAPPING: ClassVar[dict[tuple[str, Any], tuple[str, Any] | None]] = {
122122
("allow_mutation", False): ("frozen", True),
123123
("extra_fields", "forbid"): ("forbid_unknown_fields", True),
@@ -149,7 +149,7 @@ def __init__( # noqa: PLR0913
149149
"""Initialize msgspec Struct with fields sorted by field assignment requirement."""
150150
super().__init__(
151151
reference=reference,
152-
fields=sorted(fields, key=_has_field_assignment),
152+
fields=sorted(fields, key=has_field_assignment),
153153
decorators=decorators,
154154
base_classes=base_classes,
155155
custom_base_class=custom_base_class,
@@ -265,8 +265,8 @@ class DataModelField(DataModelFieldBase):
265265
"lt",
266266
"le",
267267
"multiple_of",
268-
# 'min_items', # not supported by msgspec
269-
# 'max_items', # not supported by msgspec
268+
"min_items",
269+
"max_items",
270270
"min_length",
271271
"max_length",
272272
"pattern",
@@ -398,6 +398,11 @@ def _get_meta_string(self) -> str | None:
398398
},
399399
}
400400

401+
if (min_items := data.pop("min_items", None)) is not None:
402+
data["min_length"] = min_items
403+
if (max_items := data.pop("max_items", None)) is not None:
404+
data["max_length"] = max_items
405+
401406
meta_arguments = sorted(f"{k}={v!r}" for k, v in data.items() if v is not None)
402407
return f"Meta({', '.join(meta_arguments)})" if meta_arguments else None
403408

@@ -448,7 +453,7 @@ def needs_annotated_import(self) -> bool:
448453
@property
449454
def needs_meta_import(self) -> bool:
450455
"""Check if this field requires the Meta import."""
451-
return self._get_meta_string() is not None
456+
return self.use_annotated and self._get_meta_string() is not None
452457

453458
def _get_default_as_struct_model(self) -> str | None:
454459
"""Convert default value to Struct model using msgspec convert."""

src/datamodel_code_generator/model/template/msgspec.jinja2

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ class {{ class_name }}:
2727
{%- else %}
2828
{{ field.name }}: {{ field.type_hint }}
2929
{%- endif %}
30-
{%- if not field.field and (not field.required or field.use_default_with_required or field.data_type.is_optional or field.nullable)
30+
{%- if not field.field and (not field.required or field.use_default_with_required)
3131
%} = {{ field.represented_default }}
3232
{%- endif -%}
3333
{%- endif %}

src/datamodel_code_generator/parser/base.py

Lines changed: 29 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -2480,12 +2480,17 @@ def __fix_dataclass_field_ordering(self, models: list[DataModel]) -> None:
24802480
if (inherited := self.__get_dataclass_inherited_info(model)) is None:
24812481
continue
24822482
inherited_names, has_default = inherited
2483-
if not has_default or not any(self.__is_new_required_field(f, inherited_names) for f in model.fields):
2483+
field_has_assignment = Parser._get_field_assignment_checker(model)
2484+
if not has_default or not any(
2485+
self.__is_new_required_field(f, inherited_names, field_has_assignment) for f in model.fields
2486+
):
24842487
continue
24852488

2486-
if self.target_python_version.has_kw_only_dataclass:
2489+
if isinstance(model, msgspec_model.Struct):
2490+
model.add_base_class_kwarg("kw_only", "True")
2491+
elif self.target_python_version.has_kw_only_dataclass:
24872492
for field in model.fields:
2488-
if self.__is_new_required_field(field, inherited_names):
2493+
if self.__is_new_required_field(field, inherited_names, field_has_assignment):
24892494
field.extras["kw_only"] = True
24902495
else: # pragma: no cover
24912496
warn(
@@ -2496,16 +2501,20 @@ def __fix_dataclass_field_ordering(self, models: list[DataModel]) -> None:
24962501
category=UserWarning,
24972502
stacklevel=2,
24982503
)
2499-
self.generation_store.set_fields(model, sorted(model.fields, key=dataclass_model.has_field_assignment))
2504+
self.generation_store.set_fields(model, sorted(model.fields, key=field_has_assignment))
25002505

25012506
@classmethod
25022507
def __get_dataclass_inherited_info(cls, model: DataModel) -> tuple[set[str], bool] | None:
25032508
"""Get inherited field names and whether any has default. Returns None if not applicable."""
25042509
if not model.SUPPORTS_KW_ONLY:
25052510
return None
2506-
if not model.base_classes or model.dataclass_arguments.get("kw_only"):
2511+
base_class_kw_only = None
2512+
if isinstance(model, msgspec_model.Struct):
2513+
base_class_kw_only = model.extra_template_data.get("base_class_kwargs", {}).get("kw_only")
2514+
if not model.base_classes or model.dataclass_arguments.get("kw_only") or base_class_kw_only in {True, "True"}:
25072515
return None
25082516

2517+
field_has_assignment = cls._get_field_assignment_checker(model)
25092518
inherited_names: set[str] = set()
25102519
has_default = False
25112520
for base in model.base_classes:
@@ -2515,23 +2524,30 @@ def __get_dataclass_inherited_info(cls, model: DataModel) -> tuple[set[str], boo
25152524
if not f.name or f.extras.get("init") is False:
25162525
continue # pragma: no cover
25172526
inherited_names.add(f.name)
2518-
if dataclass_model.has_field_assignment(f):
2527+
if field_has_assignment(f):
25192528
has_default = True
25202529

25212530
for f in model.fields:
25222531
if f.name not in inherited_names or f.extras.get("init") is False:
25232532
continue
2524-
if dataclass_model.has_field_assignment(f): # pragma: no branch
2533+
if field_has_assignment(f): # pragma: no branch
25252534
has_default = True
25262535
return (inherited_names, has_default) if inherited_names else None
25272536

2528-
def __is_new_required_field(self, field: DataModelFieldBase, inherited: set[str]) -> bool: # noqa: PLR6301
2537+
@staticmethod
2538+
def _get_field_assignment_checker(model: DataModel) -> Callable[[DataModelFieldBase], bool]:
2539+
if isinstance(model, msgspec_model.Struct):
2540+
return msgspec_model.has_field_assignment
2541+
return dataclass_model.has_field_assignment
2542+
2543+
def __is_new_required_field( # noqa: PLR6301
2544+
self,
2545+
field: DataModelFieldBase,
2546+
inherited: set[str],
2547+
field_has_assignment: Callable[[DataModelFieldBase], bool],
2548+
) -> bool:
25292549
"""Check if field is a new required init field."""
2530-
return (
2531-
field.name not in inherited
2532-
and field.extras.get("init") is not False
2533-
and not dataclass_model.has_field_assignment(field)
2534-
)
2550+
return field.name not in inherited and field.extras.get("init") is not False and not field_has_assignment(field)
25352551

25362552
def __remove_overridden_models(self, models: list[DataModel]) -> list[DataModel]:
25372553
"""Remove models that are being overridden by custom types (model-level only).
Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
# generated by datamodel-codegen:
2+
# filename: msgspec_array_length_constraints.json
3+
# timestamp: 2019-07-26T00:00:00+00:00
4+
5+
from __future__ import annotations
6+
7+
from typing import Annotated
8+
9+
from msgspec import UNSET, Meta, Struct, UnsetType
10+
11+
12+
class Model(Struct):
13+
items: Annotated[list[str], Meta(max_length=5, min_length=1)] | UnsetType = UNSET
Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
# generated by datamodel-codegen:
2+
# filename: msgspec_array_length_constraints.json
3+
# timestamp: 2019-07-26T00:00:00+00:00
4+
5+
from __future__ import annotations
6+
7+
from msgspec import UNSET, Struct, UnsetType
8+
9+
10+
class Model(Struct):
11+
items: list[str] | UnsetType = UNSET
Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
# generated by datamodel-codegen:
2+
# filename: msgspec_inherited_optional_default.json
3+
# timestamp: 2019-07-26T00:00:00+00:00
4+
5+
from __future__ import annotations
6+
7+
from msgspec import UNSET, Struct, UnsetType
8+
9+
10+
class Base(Struct):
11+
kind: str | UnsetType = 'base'
12+
note: str | UnsetType = UNSET
13+
14+
15+
class Child(Base, kw_only=True):
16+
id: int
17+
18+
19+
class Model(Struct):
20+
child: Child | UnsetType = UNSET
Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
# generated by datamodel-codegen:
2+
# filename: msgspec_required_alias_field.json
3+
# timestamp: 2019-07-26T00:00:00+00:00
4+
5+
from __future__ import annotations
6+
7+
from msgspec import UNSET, Struct, UnsetType, field
8+
9+
10+
class Record(Struct):
11+
req_id: int = field(name='req-id')
12+
opt: str | UnsetType = UNSET
Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
# generated by datamodel-codegen:
2+
# filename: msgspec_required_nullable_field.json
3+
# timestamp: 2019-07-26T00:00:00+00:00
4+
5+
from __future__ import annotations
6+
7+
from msgspec import Struct
8+
9+
10+
class Record(Struct):
11+
label: str | None
12+
count: int

tests/data/expected/main/openapi/msgspec_use_annotated_with_field_constraints.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ class Pet(Struct):
1515
tag: Annotated[str, Meta(max_length=64)] | UnsetType = UNSET
1616

1717

18-
Pets: TypeAlias = list[Pet]
18+
Pets: TypeAlias = Annotated[list[Pet], Meta(max_length=10, min_length=1)]
1919

2020

2121
UID: TypeAlias = Annotated[int, Meta(ge=0)]
@@ -32,7 +32,7 @@ class User(Struct):
3232
name: Annotated[str, Meta(max_length=256)]
3333
uid: UID
3434
tag: Annotated[str, Meta(max_length=64)] | UnsetType = UNSET
35-
phones: list[Phone] | UnsetType = UNSET
35+
phones: Annotated[list[Phone], Meta(max_length=10)] | UnsetType = UNSET
3636
fax: list[FaxItem] | UnsetType = UNSET
3737
height: Annotated[int | float, Meta(ge=1.0, le=300.0)] | UnsetType = UNSET
3838
weight: Annotated[float | int, Meta(ge=1.0, le=1000.0)] | UnsetType = UNSET
Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
{
2+
"type": "object",
3+
"properties": {
4+
"items": {
5+
"type": "array",
6+
"items": {"type": "string"},
7+
"minItems": 1,
8+
"maxItems": 5
9+
}
10+
}
11+
}

0 commit comments

Comments
 (0)