Fix empty list default for GraphQL list fields#2955
Conversation
📝 WalkthroughWalkthroughThis PR fixes a bug where GraphQL list fields with empty list defaults Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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-2955.datamodel-code-generator.pages.dev |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/datamodel_code_generator/model/pydantic/base_model.py (2)
129-129: Remove the unusednoqadirective.The rules
PLR0911andPLR0912are not enabled, making this directive unnecessary.🧹 Proposed fix
- def _get_default_as_pydantic_model(self) -> str | None: # noqa: PLR0911, PLR0912 + def _get_default_as_pydantic_model(self) -> str | None:
132-163: Consider refactoring to eliminate code duplication.The new handling path (lines 132-145) and the existing loop logic (lines 151-163) contain nearly identical code for processing list types with BaseModel elements. The main difference is that the new code processes
self.data_typedirectly while the loop processes members ofself.data_type.data_types.While the current implementation correctly fixes the empty list default issue, consider extracting the shared logic into a helper method to improve maintainability.
♻️ Proposed refactoring approach
Extract the list processing logic into a helper method:
def _get_list_default_factory(self, data_type_child, default): """Helper to generate default_factory for list of BaseModel types.""" if not default: return STANDARD_LIST return ( f"lambda :[{data_type_child.alias or data_type_child.reference.source.class_name}." f"{self._PARSE_METHOD}(v) for v in {default!r}]" )Then use it in both locations:
# Lines 132-145 if self.data_type.is_list and len(self.data_type.data_types) == 1: data_type_child = self.data_type.data_types[0] if ( data_type_child.reference and isinstance(data_type_child.reference.source, BaseModelBase) and isinstance(self.default, list) ): return self._get_list_default_factory(data_type_child, self.default) # Lines 151-163 if data_type.is_list and len(data_type.data_types) == 1: data_type_child = data_type.data_types[0] if ( data_type_child.reference and isinstance(data_type_child.reference.source, BaseModelBase) and isinstance(self.default, list) ): return self._get_list_default_factory(data_type_child, self.default)
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
tests/data/graphql/empty_list_default.graphqlis excluded by!tests/data/**/*.graphqland included by none
📒 Files selected for processing (3)
src/datamodel_code_generator/model/pydantic/base_model.pytests/data/expected/main/graphql/empty_list_default.pytests/main/graphql/test_main_graphql.py
🧰 Additional context used
🧬 Code graph analysis (2)
tests/main/graphql/test_main_graphql.py (1)
tests/main/conftest.py (2)
output_file(99-101)run_main_and_assert(245-409)
src/datamodel_code_generator/model/pydantic/base_model.py (4)
src/datamodel_code_generator/model/_types.py (1)
WrappedDefault(10-18)src/datamodel_code_generator/parser/base.py (1)
data_type(1175-1177)src/datamodel_code_generator/reference.py (1)
reference(77-79)src/datamodel_code_generator/model/base.py (2)
class_name(860-862)class_name(865-869)
🪛 Ruff (0.14.10)
src/datamodel_code_generator/model/pydantic/base_model.py
129-129: Unused noqa directive (non-enabled: PLR0911, 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). (13)
- GitHub Check: 3.10 on Windows
- GitHub Check: 3.12 on Windows
- GitHub Check: 3.12 on macOS
- GitHub Check: 3.14 on macOS
- GitHub Check: 3.13 on Windows
- GitHub Check: 3.11 on macOS
- GitHub Check: 3.11 on Windows
- GitHub Check: 3.10 on macOS
- GitHub Check: benchmarks
- GitHub Check: 3.14 on Windows
- GitHub Check: build-deploy
- GitHub Check: docs
- GitHub Check: Analyze (python)
🔇 Additional comments (3)
src/datamodel_code_generator/model/pydantic/base_model.py (1)
140-141: LGTM - Correct fix for empty list defaults.Returning
STANDARD_LISTfor empty list defaults correctly resolves the issue wheredefault_factory=lambda: NestedClass.model_validate([])was causing validation errors. The generated code now properly usesField(default_factory=list).tests/main/graphql/test_main_graphql.py (1)
895-911: LGTM - Excellent test coverage for the fix.This test properly verifies that empty list defaults in GraphQL schemas generate
Field(default_factory=list)instead of the problematicField(default_factory=lambda: TagInput.model_validate([])). The test is well-documented with a clear reference to issue #2947.tests/data/expected/main/graphql/empty_list_default.py (1)
1-32: LGTM - Expected output correctly demonstrates the fix.The generated model at line 31 properly uses
Field(default_factory=list)for the empty list default, which is exactly the behavior this PR aims to achieve. This resolves the validation error that occurred when usinglambda: TagInput.model_validate([]).
Merging this PR will not alter performance
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2955 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 94 94
Lines 17723 17730 +7
Branches 2037 2039 +2
=========================================
+ Hits 17723 17730 +7
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:
|
Fixes: #2947
Summary by CodeRabbit
Bug Fixes
Tests
✏️ Tip: You can customize this high-level summary in your review settings.