Add support for prefixItems to emit tuples#2537
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2537 +/- ##
=======================================
Coverage 99.33% 99.34%
=======================================
Files 81 81
Lines 11480 11535 +55
Branches 1367 1387 +20
=======================================
+ Hits 11404 11459 +55
Misses 45 45
Partials 31 31
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
CodSpeed Performance ReportMerging #2537 will not alter performanceComparing Summary
Footnotes
|
|
I have merged main branch into this branch. and I have applid new test assertion helper function to the test. |
|
Thank you! Sorry I haven't gotten time to address the feedback yet, I should in a week or so, but feel free to make any changes as well. |
There was a problem hiding this comment.
Pull request overview
This pull request adds support for generating Python tuple type annotations from JSON Schema's prefixItems keyword. When a JSON Schema array has prefixItems defined with matching minItems and maxItems constraints (equal to the number of prefix items) and no additional items specification, the code generator now emits precise Tuple[...] types instead of generic List[...] types.
Key Changes
- Added
prefixItemsfield toJsonSchemaObjectmodel for parsing JSON Schema 2020-12 tuple validation syntax - Implemented tuple detection logic in array field parsing when prefix items count matches min/max items constraints
- Extended
DataTypeclass withis_tupleflag and tuple-specific type hint generation - Added
IMPORT_TUPLEconstant and integrated tuple import handling for non-standard collections
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
src/datamodel_code_generator/parser/jsonschema.py |
Added prefixItems field to schema object model; implemented tuple detection logic in parse_array_fields; extended parse_ref and parse_id to handle prefixItems references |
src/datamodel_code_generator/types.py |
Added is_tuple flag to DataType; implemented tuple type hint generation with support for both Tuple and tuple (standard collections); added tuple import handling |
src/datamodel_code_generator/imports.py |
Added IMPORT_TUPLE constant for importing typing.Tuple |
tests/main/jsonschema/test_main_jsonschema.py |
Added parametrized test covering both pydantic and msgspec output models for prefix items |
tests/data/jsonschema/prefix_items.json |
Added test input schema demonstrating tuple validation with prefixItems, minItems, and maxItems |
tests/data/expected/main/jsonschema/prefix_items.py |
Added expected pydantic output with Tuple[Span, str] annotation |
tests/data/expected/main/jsonschema/prefix_items_msgspec.py |
Added expected msgspec output with Tuple[Span, str] annotation |
docs/jsonschema.md |
Added documentation section explaining tuple validation with prefixItems including example schema and generated code |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
# Conflicts: # docs/jsonschema.md
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughAdds JSON Schema prefixItems support: parser records Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Parser as JSON Schema Parser
participant Types as Type System
participant Generator as Code Generator
User->>Parser: Provide schema with `prefixItems`, `minItems`, `maxItems`, `items`
Parser->>Parser: Parse schema, record `prefixItems`, `minItems`, `maxItems`, `items`
alt fixed-length tuple detected (min==max==len(prefixItems) and items is None/false)
Parser->>Types: Create DataType (is_tuple = true) with element types from prefixItems
Parser->>Parser: Suppress minItems/maxItems in field constraints
else
Parser->>Types: Create DataType for list/array, retain items and constraints
end
Types->>Types: Build type_hint (Tuple[...] or list[...]) and resolve imports (typing.Tuple if needed)
Types->>Generator: Emit type hint and import statements
Generator->>User: Output generated model code
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (5)
🔇 Additional comments (6)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
@ilovelinux Can you review the PR again? |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
src/datamodel_code_generator/parser/jsonschema.py (4)
312-312: Remove unusednoqadirective.Static analysis indicates
N815andUP045rules are not enabled, making thisnoqacomment unnecessary.🔎 Apply this diff:
- prefixItems: Optional[list[JsonSchemaObject]] = None # noqa: N815, UP045 + prefixItems: Optional[list[JsonSchemaObject]] = None
2378-2382: Mutating input object has potential side effects.Setting
obj.minItems = obj.maxItems = Nonedirectly modifies theJsonSchemaObjectinstance. While this achieves the goal of excluding these constraints from output (viaobj.dict()at line 2415), mutating input parameters can cause unexpected behavior if the same object is referenced elsewhere.Consider creating a modified constraints dict instead of mutating the source object.
🔎 Alternative approach (defer constraints filtering):
Instead of mutating
obj, filter out the constraints when building the field:elif obj.prefixItems is not None and obj.minItems == obj.maxItems == len(obj.prefixItems): - # Set these to None so that it won't output max item constraints - obj.minItems = obj.maxItems = None items = obj.prefixItems is_tuple = True + # Mark for later: exclude minItems/maxItems from constraints + exclude_item_constraints = True +else: + exclude_item_constraints = FalseThen at line 2415:
constraints = obj.dict() if exclude_item_constraints: constraints.pop('minItems', None) constraints.pop('maxItems', None)
2355-2355: Remove unusednoqadirective.The
PLR0912rule is not enabled, making thisnoqacomment unnecessary.🔎 Apply this diff:
- def parse_array_fields( # noqa: PLR0912 + def parse_array_fields(
2941-2941: Remove unusednoqadirective.The
PLR0912rule is not enabled, making thisnoqacomment unnecessary.🔎 Apply this diff:
- def _add_id_callback(self, obj: JsonSchemaObject, path: list[str]) -> None: # noqa: PLR0912 + def _add_id_callback(self, obj: JsonSchemaObject, path: list[str]) -> None:
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
docs/jsonschema.md(1 hunks)src/datamodel_code_generator/imports.py(1 hunks)src/datamodel_code_generator/parser/jsonschema.py(7 hunks)src/datamodel_code_generator/types.py(5 hunks)tests/data/expected/main/jsonschema/prefix_items.py(1 hunks)tests/data/expected/main/jsonschema/prefix_items_msgspec.py(1 hunks)tests/data/expected/main/jsonschema/prefix_items_no_tuple.py(1 hunks)tests/data/jsonschema/prefix_items.json(1 hunks)tests/data/jsonschema/prefix_items_no_tuple.json(1 hunks)tests/main/jsonschema/test_main_jsonschema.py(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
src/datamodel_code_generator/types.py (4)
src/datamodel_code_generator/model/base.py (3)
imports(238-255)imports(583-588)type_hint(217-235)src/datamodel_code_generator/model/enum.py (1)
imports(118-120)src/datamodel_code_generator/model/typed_dict.py (2)
imports(150-155)type_hint(132-137)src/datamodel_code_generator/model/type_alias.py (1)
imports(28-34)
src/datamodel_code_generator/parser/jsonschema.py (2)
src/datamodel_code_generator/model/base.py (1)
path(664-666)src/datamodel_code_generator/reference.py (1)
add_id(633-635)
tests/data/expected/main/jsonschema/prefix_items.py (1)
tests/data/expected/main/jsonschema/prefix_items_msgspec.py (2)
Span(12-13)Defaults(16-17)
🪛 Ruff (0.14.8)
src/datamodel_code_generator/parser/jsonschema.py
312-312: Unused noqa directive (non-enabled: N815, UP045)
Remove unused noqa directive
(RUF100)
2355-2355: Unused noqa directive (non-enabled: PLR0912)
Remove unused noqa directive
(RUF100)
2941-2941: Unused noqa directive (non-enabled: PLR0912)
Remove unused noqa directive
(RUF100)
🔇 Additional comments (15)
src/datamodel_code_generator/imports.py (1)
164-164: LGTM!The
IMPORT_TUPLEconstant follows the established pattern for typing imports and is correctly placed alongside related constants.tests/data/jsonschema/prefix_items.json (1)
1-23: LGTM!This test fixture correctly exercises the tuple generation conditions:
prefixItemspresent with two items,minItemsandmaxItemsboth equal to 2, and noitemsspecified.tests/data/jsonschema/prefix_items_no_tuple.json (1)
1-23: LGTM!This test fixture correctly validates the fallback behavior when
minItems(1) andmaxItems(3) don't match theprefixItemslength (2), ensuring aListtype is generated instead ofTuple.tests/data/expected/main/jsonschema/prefix_items.py (1)
1-17: LGTM!The expected output correctly demonstrates tuple generation with
Tuple[Span, str]matching theprefixItemsschema definition.tests/data/expected/main/jsonschema/prefix_items_no_tuple.py (1)
12-17: LGTM!The expected output correctly falls back to
List[Any]with min/max length constraints when the tuple conditions aren't satisfied. TheSpanclass is still generated from$defsas expected, even though it's not referenced in the fallback type.tests/data/expected/main/jsonschema/prefix_items_msgspec.py (1)
1-17: LGTM!The msgspec variant correctly generates
Tuple[Span, str]withmsgspec.Structbase class, demonstrating that tuple support works across different output model types.tests/main/jsonschema/test_main_jsonschema.py (2)
2886-2915: LGTM!The test is well-structured with appropriate parameterization for both
pydantic_v2.BaseModelandmsgspec.Structoutput models. The@freeze_timedecorator ensures reproducible timestamps, and the black version skip maintains compatibility.
2918-2932: LGTM!Good coverage of the fallback behavior when
minItems != maxItems, ensuring the generator correctly producesList[Any]instead of a tuple type.src/datamodel_code_generator/types.py (3)
79-83: LGTM on constant placement.The constants are correctly organized:
TUPLEwith the typing module generics (lines 73-79) andSTANDARD_TUPLEwith the builtin type names (lines 80-83).
464-487: Tuple import handling is correct.The
IMPORT_TUPLEis added only whenuse_standard_collections=False, which aligns with usingtyping.Tuple. Whenuse_standard_collections=True, the builtintupletype is used (no import required). This follows the same pattern as other container types.
533-537: Tuple type hint generation looks good.The implementation correctly:
- Selects between
tupleandTuplebased onuse_standard_collections- Builds inner types from
data_typesfor heterogeneous tuple elements- Handles the empty tuple case with
tuple[()]src/datamodel_code_generator/parser/jsonschema.py (4)
384-386: LGTM on is_array update.The inclusion of
prefixItemsin the array detection is correct per JSON Schema semantics.
2397-2402: LGTM on DataType construction.The mutual exclusivity of
is_tupleandis_listis correctly enforced.
2916-2918: LGTM on prefixItems traversal.The traversal correctly handles
prefixItemsfor reference resolution, consistent with the existingitemshandling pattern.
2951-2953: LGTM on prefixItems ID resolution.The
_add_id_callbackcorrectly processesprefixItemsfor ID registration, matching the pattern used for other schema structures.
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/datamodel_code_generator/parser/jsonschema.py (1)
2388-2417: Critical: Tuple detection incorrectly handles schemas withitemsset.The condition at line 393 determines whether to emit a tuple type, but it doesn't verify that
itemsis absent. According to JSON Schema 2020-12:
prefixItemsdefines schemas for the first N elementsitemsdefines the schema for any additional elements beyondprefixItemsIf both
prefixItemsanditemsare present, the array can have more elements thanprefixItems, making it not a fixed-length tuple.Example that would be incorrectly handled:
{ "prefixItems": [{"type": "string"}, {"type": "number"}], "items": {"type": "boolean"}, "minItems": 2, "maxItems": 3 }This allows arrays like
["a", 1]or["a", 1, true], which is not a tuple. The current code would incorrectly emitTuple[str, float].🔎 Proposed fix
- elif obj.prefixItems is not None and obj.minItems == obj.maxItems == len(obj.prefixItems): + elif obj.prefixItems is not None and obj.minItems == obj.maxItems == len(obj.prefixItems) and obj.items in (None, False): # Set these to None so that it won't output max item constraints obj.minItems = obj.maxItems = None items = obj.prefixItems is_tuple = TrueThis ensures tuples are only emitted when:
prefixItemsis specified- Array length is fixed (
minItems == maxItems == len(prefixItems))- No additional items are allowed (
itemsisNoneorFalse)
♻️ Duplicate comments (2)
src/datamodel_code_generator/types.py (2)
49-49: LGTM: Tuple constants are correctly positioned.The
TUPLEconstant is appropriately placed with other typing module constants (afterLIST), andSTANDARD_TUPLEis grouped with other standard/builtin type constants. The organization is consistent with the existing pattern.Also applies to: 77-77, 81-81
461-484: LGTM: Import logic correctly handles all branches.The tuple import handling is correct across all configuration combinations:
- When
use_standard_collections=True: Uses builtintuple, no import needed- When
use_standard_collections=False: UsesTuplefrom typing, import added at lines 475 and 483The implementation properly accounts for both
use_generic_containersettings.
🧹 Nitpick comments (1)
src/datamodel_code_generator/parser/jsonschema.py (1)
313-313: Optional: Remove unusednoqadirectives.Static analysis indicates the
noqadirectives at these lines suppress rules (N815, PLR0912, UP045) that are not enabled in your configuration. While these directives are defensive and don't cause issues, removing them would clean up the code.Suggested cleanup
Line 313:
- prefixItems: Optional[list[JsonSchemaObject]] = None # noqa: N815, UP045 + prefixItems: Optional[list[JsonSchemaObject]] = None # noqa: N815Line 2370:
- def parse_array_fields( # noqa: PLR0912 + def parse_array_fields(Line 2956:
- def _add_id_callback(self, obj: JsonSchemaObject, path: list[str]) -> None: # noqa: PLR0912 + def _add_id_callback(self, obj: JsonSchemaObject, path: list[str]) -> None:Note: The N815 suppression for
prefixItemsshould be kept since the field name uses camelCase to match the JSON Schema specification.Also applies to: 2370-2370, 2956-2956
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
docs/jsonschema.md(1 hunks)src/datamodel_code_generator/imports.py(1 hunks)src/datamodel_code_generator/parser/jsonschema.py(7 hunks)src/datamodel_code_generator/types.py(5 hunks)tests/data/expected/main/jsonschema/prefix_items.py(1 hunks)tests/data/expected/main/jsonschema/prefix_items_msgspec.py(1 hunks)tests/data/expected/main/jsonschema/prefix_items_no_tuple.py(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (5)
- docs/jsonschema.md
- src/datamodel_code_generator/imports.py
- tests/data/expected/main/jsonschema/prefix_items_no_tuple.py
- tests/data/expected/main/jsonschema/prefix_items.py
- tests/data/expected/main/jsonschema/prefix_items_msgspec.py
🧰 Additional context used
🧬 Code graph analysis (1)
src/datamodel_code_generator/parser/jsonschema.py (2)
src/datamodel_code_generator/model/base.py (1)
path(689-691)src/datamodel_code_generator/reference.py (1)
add_id(632-634)
🪛 Ruff (0.14.8)
src/datamodel_code_generator/parser/jsonschema.py
313-313: Unused noqa directive (non-enabled: N815, UP045)
Remove unused noqa directive
(RUF100)
2370-2370: Unused noqa directive (non-enabled: PLR0912)
Remove unused noqa directive
(RUF100)
2956-2956: Unused noqa directive (non-enabled: PLR0912)
Remove unused noqa directive
(RUF100)
🔇 Additional comments (4)
src/datamodel_code_generator/types.py (1)
530-534: LGTM: Tuple type hint generation is well implemented.The tuple type hint logic correctly:
- Selects between
tuple(builtin) andTuple(typing) based onuse_standard_collections- Extracts inner element types from nested
data_types- Formats as
tuple[type1, type2, ...]for heterogeneous tuples- Handles empty tuples with
tuple[()]src/datamodel_code_generator/parser/jsonschema.py (3)
387-387: LGTM: Correctly extends array detection to includeprefixItems.Arrays can be defined solely through
prefixItemsin JSON Schema 2020-12 without anitemsfield, so this check is necessary and correct.
2931-2933: LGTM: Traversal correctly processesprefixItems.The recursive traversal of
prefixItemsmatches the existing pattern foritemsand ensures all nested schema objects are processed by the callback.
2960-2981: LGTM: ID callback correctly handlesprefixItems.The
_add_id_callbackmethod now processesprefixItemsfor ID registration, consistent with how it handlesitems,additionalProperties, and other schema collections.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
src/datamodel_code_generator/parser/jsonschema.py (3)
313-313: Remove unusednoqadirective.The
noqa: N815, UP045directive is flagged as unused by static analysis. These rules are not being violated here, so the directive should be removed.🔎 Proposed fix
- prefixItems: Optional[list[JsonSchemaObject]] = None # noqa: N815, UP045 + prefixItems: Optional[list[JsonSchemaObject]] = None
2370-2370: Remove unusednoqadirective.The
noqa: PLR0912directive is flagged as unused. It should be removed.🔎 Proposed fix
- def parse_array_fields( # noqa: PLR0912 + def parse_array_fields(
2960-2960: Remove unusednoqadirective.The
noqa: PLR0912directive is flagged as unused. It should be removed.🔎 Proposed fix
- def _add_id_callback(self, obj: JsonSchemaObject, path: list[str]) -> None: # noqa: PLR0912 + def _add_id_callback(self, obj: JsonSchemaObject, path: list[str]) -> None:
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/datamodel_code_generator/parser/jsonschema.py(7 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/datamodel_code_generator/parser/jsonschema.py (2)
src/datamodel_code_generator/model/base.py (1)
path(689-691)src/datamodel_code_generator/reference.py (1)
add_id(632-634)
🪛 Ruff (0.14.8)
src/datamodel_code_generator/parser/jsonschema.py
313-313: Unused noqa directive (non-enabled: N815, UP045)
Remove unused noqa directive
(RUF100)
2370-2370: Unused noqa directive (non-enabled: PLR0912)
Remove unused noqa directive
(RUF100)
2960-2960: Unused noqa directive (non-enabled: PLR0912)
Remove unused noqa directive
(RUF100)
🔇 Additional comments (4)
src/datamodel_code_generator/parser/jsonschema.py (4)
387-387: LGTM! Correct array detection enhancement.The
is_arrayproperty now correctly identifies arrays that useprefixItems, which is essential for proper tuple type generation.
2935-2937: LGTM! Correct traversal ofprefixItems.The traversal logic properly iterates through
prefixItemsand recursively processes each schema object, consistent with how other collection fields are handled.
2970-2972: LGTM! Correct ID propagation forprefixItems.The ID parsing logic for
prefixItemsis correctly implemented, following the same pattern asitemshandling.
388-403: Incorrect line number references in review comment.The review references lines 388-403 but the actual prefixItems handling is located around line 2393-2403. Additionally, the behavior described is intentional and already tested: when
prefixItemsconditions don't match a fixed-length tuple (minItems == maxItems == len(prefixItems)), the code correctly generatesList[Any]with field constraints. A test case for this scenario already exists inprefix_items_no_tuple.json.Likely an incorrect or invalid review comment.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/datamodel_code_generator/parser/jsonschema.py (1)
2393-2403: Address previous review feedback: Add test for prefixItems with mismatched item counts.A previous reviewer requested a test case for when
prefixItemsis set butminItems != maxItems. Currently, when this condition occurs, the code falls through toitems = [](line 2403), which results inTypes.anybeing used (line 2414).Please verify this is the intended behavior and add a test case to document it.
Based on learnings from previous review by ilovelinux.
🧹 Nitpick comments (3)
src/datamodel_code_generator/parser/jsonschema.py (3)
313-313: Remove unusednoqadirective.The static analysis tool correctly identified that the
noqadirective forN815andUP045is unnecessary since these checks are not enabled in the project configuration.🔎 Proposed fix
- prefixItems: Optional[list[JsonSchemaObject]] = None # noqa: N815, UP045 + prefixItems: Optional[list[JsonSchemaObject]] = None # noqa: UP045
2370-2370: Remove unusednoqadirective.The
noqadirective forPLR0912is unnecessary as this check is not enabled.🔎 Proposed fix
- def parse_array_fields( # noqa: PLR0912 + def parse_array_fields(
2960-2960: Remove unusednoqadirective.The
noqadirective forPLR0912is unnecessary as this check is not enabled.🔎 Proposed fix
- def _add_id_callback(self, obj: JsonSchemaObject, path: list[str]) -> None: # noqa: PLR0912 + def _add_id_callback(self, obj: JsonSchemaObject, path: list[str]) -> None:
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/datamodel_code_generator/parser/jsonschema.py(7 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/datamodel_code_generator/parser/jsonschema.py (2)
src/datamodel_code_generator/model/base.py (1)
path(689-691)src/datamodel_code_generator/reference.py (1)
add_id(632-634)
🪛 Ruff (0.14.8)
src/datamodel_code_generator/parser/jsonschema.py
313-313: Unused noqa directive (non-enabled: N815, UP045)
Remove unused noqa directive
(RUF100)
2370-2370: Unused noqa directive (non-enabled: PLR0912)
Remove unused noqa directive
(RUF100)
2960-2960: Unused noqa directive (non-enabled: PLR0912)
Remove unused noqa directive
(RUF100)
🔇 Additional comments (3)
src/datamodel_code_generator/parser/jsonschema.py (3)
387-387: LGTM! Correctly identifies prefixItems schemas as arrays.The addition of
self.prefixItems is not Noneto theis_arraycheck properly handles JSON Schema tuple validation whereprefixItemsdefines the array structure without requiringitemsto be set.
2935-2937: LGTM! Correctly traverses prefixItems.The prefixItems traversal follows the same pattern as the items list traversal and properly propagates the
include_one_ofparameter through the recursive calls.
2962-2985: LGTM! Comprehensive ID propagation through all schema constructs.The expanded
_add_id_callbacknow correctly handles ID propagation throughprefixItems,items(both object and list forms),additionalProperties,patternProperties,anyOf,allOf, andproperties. The prefixItems handling at lines 2970-2972 correctly mirrors the items list handling pattern.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
src/datamodel_code_generator/parser/jsonschema.py (3)
313-313: Remove unused noqa directives.The noqa directives for N815 and UP045 are flagged as unused by Ruff, indicating these rules aren't enabled in your configuration.
🔎 Suggested cleanup
- prefixItems: Optional[list[JsonSchemaObject]] = None # noqa: N815, UP045 + prefixItems: Optional[list[JsonSchemaObject]] = None # noqa: N815
1018-1027: Consider extracting duplicate tuple detection logic.The same condition for detecting fixed-length tuples appears in both
get_object_field(lines 1020-1025) andparse_array_fields(lines 2404-2408). While both locations need this check, extracting it to a helper method would improve maintainability and ensure consistency.🔎 Suggested refactor
Add a helper method to the
JsonSchemaParserclass:def _is_fixed_length_tuple(self, field: JsonSchemaObject) -> bool: """Check if field represents a fixed-length tuple via prefixItems.""" return ( field.prefixItems is not None and field.minItems == field.maxItems == len(field.prefixItems) and field.items in {None, False} )Then use it in both locations:
def get_object_field(...): constraints = field.dict() if self.is_constraints_field(field) else None - # Suppress minItems/maxItems for fixed-length tuples - if ( - constraints - and field.prefixItems is not None - and field.minItems == field.maxItems == len(field.prefixItems) - and field.items in {None, False} - ): + if constraints and self._is_fixed_length_tuple(field): constraints.pop("minItems", None) constraints.pop("maxItems", None)def parse_array_fields(...): - elif ( - obj.prefixItems is not None - and obj.minItems == obj.maxItems == len(obj.prefixItems) - and obj.items in {None, False} - ): + elif self._is_fixed_length_tuple(obj): suppress_item_constraints = True items = obj.prefixItems is_tuple = TrueAlso applies to: 2398-2414
2975-3000: Remove unused noqa directive and consider method complexity.The expansion of
_add_id_callbackto handleprefixItemsand other properties is correct, but static analysis flags an unusedPLR0912(too-many-branches) directive on line 2975.🔎 Suggested cleanup
- def _add_id_callback(self, obj: JsonSchemaObject, path: list[str]) -> None: # noqa: PLR0912 + def _add_id_callback(self, obj: JsonSchemaObject, path: list[str]) -> None: """Add $id to model resolver."""If complexity warnings appear after removing the directive, consider extracting property traversal into a separate helper method.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/datamodel_code_generator/parser/jsonschema.py(9 hunks)
🧰 Additional context used
🪛 Ruff (0.14.8)
src/datamodel_code_generator/parser/jsonschema.py
313-313: Unused noqa directive (non-enabled: N815, UP045)
Remove unused noqa directive
(RUF100)
2380-2380: Unused noqa directive (non-enabled: PLR0912)
Remove unused noqa directive
(RUF100)
2975-2975: Unused noqa directive (non-enabled: PLR0912)
Remove unused noqa directive
(RUF100)
🔇 Additional comments (3)
src/datamodel_code_generator/parser/jsonschema.py (3)
387-387: LGTM! Correct array type detection.Including
prefixItems is not Nonein the array check correctly identifies schemas using prefixItems as array types, aligning with JSON Schema specifications.
2950-2952: LGTM! Correct traversal of prefixItems.The traversal logic correctly handles
prefixItemssimilarly to howitemsis processed, ensuring all nested schemas are visited by the callback.
2404-2414: This behavior is already tested and documented. The code correctly handles two cases:
- Fixed-length tuples (minItems == maxItems == len(prefixItems)): Generates
tuple[...]using prefixItems schemas.- Variable-length arrays (minItems != maxItems or != len(prefixItems)): Falls back to
list[Any]with min_length/max_length constraints preserved via Field.The test
test_main_jsonschema_prefix_items_no_tupleexplicitly validates this fallback behavior with the test inputprefix_items_no_tuple.json(minItems: 1, maxItems: 3) and confirms the expected output islist[Any] = Field(..., max_length=3, min_length=1). The design choice to lose prefixItems schema information in variable-length cases is intentional, trading schema specificity for code simplicity outside the scope of fixed-length tuple support.
- Replace Optional[X] with X | None - Replace List[X] with list[X] - Replace Tuple[X] with tuple[X] - Update Pydantic v1 examples to v2 syntax (RootModel instead of __root__)
|
Thanks for fixing this! I ended up just taking some time off last month and am now coming back to things. |
Closes #1546 by adding support for emitting tuples from
prefixItems, if the min and max are set to the number of prefix items and no items are specified.Summary by CodeRabbit
New Features
Documentation
Tests
✏️ Tip: You can customize this high-level summary in your review settings.