Fix deep hierarchy type inheritance in allOf property overrides#2843
Fix deep hierarchy type inheritance in allOf property overrides#2843
Conversation
📝 WalkthroughWalkthroughThe PR fixes allOf class hierarchy generation by enhancing Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested labels
Poem
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
|
📚 Docs Preview: https://pr-2843.datamodel-code-generator.pages.dev |
CodSpeed Performance ReportMerging #2843 will not alter performanceComparing
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2843 +/- ##
=======================================
Coverage 99.49% 99.49%
=======================================
Files 90 90
Lines 14353 14376 +23
Branches 1717 1723 +6
=======================================
+ Hits 14281 14304 +23
Misses 37 37
Partials 35 35
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:
|
49d2677 to
4ba0ba7
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/datamodel_code_generator/parser/jsonschema.py (1)
1778-1778: Remove unused noqa directive.The
# noqa: PLR0912directive is no longer needed as the function no longer triggers the "too many branches" rule.🔎 Proposed fix
- def _get_inherited_field_type( # noqa: PLR0912 + def _get_inherited_field_type(
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (4)
tests/data/jsonschema/all_of_deep_hierarchy_property_override.jsonis excluded by!tests/data/**/*.jsonand included by nonetests/data/jsonschema/all_of_hierarchy_inline_allof.jsonis excluded by!tests/data/**/*.jsonand included by nonetests/data/jsonschema/all_of_hierarchy_property_not_in_ancestor.jsonis excluded by!tests/data/**/*.jsonand included by nonetests/data/jsonschema/all_of_very_deep_hierarchy_property_override.jsonis excluded by!tests/data/**/*.jsonand included by none
📒 Files selected for processing (6)
src/datamodel_code_generator/parser/jsonschema.pytests/data/expected/main/jsonschema/all_of_deep_hierarchy_property_override.pytests/data/expected/main/jsonschema/all_of_hierarchy_inline_allof.pytests/data/expected/main/jsonschema/all_of_hierarchy_property_not_in_ancestor.pytests/data/expected/main/jsonschema/all_of_very_deep_hierarchy_property_override.pytests/main/jsonschema/test_main_jsonschema.py
🧰 Additional context used
🧬 Code graph analysis (5)
tests/main/jsonschema/test_main_jsonschema.py (2)
tests/main/conftest.py (2)
output_file(98-100)run_main_and_assert(244-408)src/datamodel_code_generator/__init__.py (1)
chdir(245-255)
src/datamodel_code_generator/parser/jsonschema.py (1)
src/datamodel_code_generator/reference.py (3)
Reference(141-200)get(965-967)add_ref(787-810)
tests/data/expected/main/jsonschema/all_of_hierarchy_property_not_in_ancestor.py (2)
src/datamodel_code_generator/model/base.py (1)
name(826-828)tests/data/expected/main/jsonschema/all_of_hierarchy_inline_allof.py (2)
Parent(12-14)Child(17-18)
tests/data/expected/main/jsonschema/all_of_deep_hierarchy_property_override.py (2)
tests/data/expected/main/jsonschema/all_of_very_deep_hierarchy_property_override.py (3)
Entity(14-15)Thing(18-20)Person(23-25)src/datamodel_code_generator/model/base.py (1)
name(826-828)
tests/data/expected/main/jsonschema/all_of_very_deep_hierarchy_property_override.py (2)
tests/data/expected/main/jsonschema/all_of_deep_hierarchy_property_override.py (3)
Entity(10-11)Thing(14-16)Person(19-20)src/datamodel_code_generator/model/base.py (1)
name(826-828)
🪛 Ruff (0.14.10)
src/datamodel_code_generator/parser/jsonschema.py
1778-1778: Unused noqa directive (non-enabled: PLR0912)
Remove unused noqa directive
(RUF100)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (10)
- GitHub Check: py312-isort6 on Ubuntu
- GitHub Check: 3.10 on Ubuntu
- GitHub Check: py312-isort7 on Ubuntu
- GitHub Check: 3.10 on Windows
- GitHub Check: 3.13 on Windows
- GitHub Check: 3.12 on Windows
- GitHub Check: 3.14 on Windows
- GitHub Check: 3.11 on Windows
- GitHub Check: Analyze (python)
- GitHub Check: benchmarks
🔇 Additional comments (6)
tests/data/expected/main/jsonschema/all_of_deep_hierarchy_property_override.py (1)
1-20: Expected model correctly encodes deep allOf hierarchy and property overrideEntity → Thing → Person inheritance and the
typeoverrides look consistent with the intended fix: Person preserves thestrbase type while adding optionality and a default, and constraints onThing.nameare carried as expected. No changes needed.tests/main/jsonschema/test_main_jsonschema.py (1)
1426-1471: New allOf deep-hierarchy tests align with existing patterns and cover the regression wellThe four added tests follow the established
chdir(JSON_SCHEMA_DATA_PATH) + run_main_and_assertpattern, target the correct JSON inputs, and assert against the corresponding expected Python artifacts. The docstrings clearly document the deep/very-deep, missing-ancestor, and inline-allOf scenarios, giving good coverage for the fixed inheritance logic. No issues found.src/datamodel_code_generator/parser/jsonschema.py (1)
1778-1820: LGTM! The enhanced_get_inherited_field_typemethod properly handles deep hierarchies.The implementation correctly:
- Adds cycle detection via the
visitedfrozenset to prevent infinite loops- Recursively traverses grandparent references through
allOfconstraints- Constructs proper ref paths for both fragment-based (
#/...) and plain pathsThis addresses the regression from v0.38.0 where deep class hierarchies with
allOfstopped producing proper inheritance.tests/data/expected/main/jsonschema/all_of_hierarchy_inline_allof.py (1)
1-18: LGTM! Test fixture correctly validates inline allOf hierarchy generation.The generated models demonstrate proper inheritance where
ChildextendsParentand overrides thestatusfield with a different default value, which is the expected behavior for the inline allOf scenario.tests/data/expected/main/jsonschema/all_of_hierarchy_property_not_in_ancestor.py (1)
1-21: LGTM! Test fixture validates property inheritance when property is not in root ancestor.This correctly tests the scenario where a property (
category) is introduced at an intermediate level (Parent) and overridden at the child level, whileGrandparenthas a different property (name). The inheritance chain is properly maintained.tests/data/expected/main/jsonschema/all_of_very_deep_hierarchy_property_override.py (1)
1-30: LGTM! Test fixture validates very deep hierarchy with property overrides.This 5-level inheritance chain (
Base→Entity→Thing→Person→SpecificPerson) effectively validates the recursive traversal enhancement. Thetypefield is progressively overridden with different defaults at each level, andSpecificPersoncorrectly overrides theidfield from the rootBaseclass—demonstrating the fix works across the full inheritance depth.
Breaking Change AnalysisResult: No breaking changes detected Reasoning: This PR is a bug fix that improves the handling of deep inheritance hierarchies in allOf property overrides. The changes are:
This analysis was performed by Claude Code Action |
|
🎉 Released in 0.51.0 This PR is now available in the latest release. See the release notes for details. |
Fixes: #2645
Summary by CodeRabbit
New Features
Tests
✏️ Tip: You can customize this high-level summary in your review settings.