Add --use-generic-base-class option for DRY model config#2726
Conversation
WalkthroughAdds a new CLI flag Changes
sequenceDiagram
autonumber
participant CLI as CLI / User
participant Main as __main__ / generate()
participant Parser as Parser (JsonSchema/OpenAPI/GraphQL)
participant Processor as Model Processor
participant Factory as DataModel.create_base_class_model
participant Renderer as Code Renderer
CLI->>Main: invoke with --use-generic-base-class
Main->>Parser: instantiate Parser(..., use_generic_base_class=True)
Parser->>Processor: build per-module models and collect config
Processor->>Factory: request shared base-class creation with collected config
Factory-->>Processor: return base-class model (or None)
Processor->>Processor: insert base model, update base_classes & imports
Processor->>Renderer: render updated modules
Renderer-->>CLI: write generated files
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Possibly related PRs
Suggested labels
Suggested reviewers
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 |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2726 +/- ##
==========================================
- Coverage 99.35% 99.35% -0.01%
==========================================
Files 83 83
Lines 11865 12004 +139
Branches 1433 1452 +19
==========================================
+ Hits 11789 11927 +138
Misses 45 45
- Partials 31 32 +1
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 #2726 will not alter performanceComparing Summary
Footnotes
|
![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 (6)
tests/data/expected/main/jsonschema/use_generic_base_class.py (2)
23-27: Consider optimizing redundant config declarations.Bar explicitly sets
extra='forbid'even though it's already defined in ProjectBaseModel. While this provides clarity, the generator could optimize by omitting model_config when it matches the parent's configuration exactly.Compare with Baz (lines 30-31), which correctly inherits the base config without redundant declaration.
34-40: Same redundant config pattern as Bar.Test also explicitly declares
extra='forbid'when it matches the ProjectBaseModel default. This is the same optimization opportunity noted for Bar.src/datamodel_code_generator/parser/base.py (2)
648-648: Remove unusednoqadirective.Per the static analysis hint, the
noqadirective forPLR0912,PLR0913,PLR0915is no longer needed on this line.🔎 Proposed fix
- def __init__( # noqa: PLR0912, PLR0913, PLR0915 + def __init__( # noqa: PLR0913
2042-2059: Consider settingfile_pathon the generatedProjectBaseModel.The
ProjectBaseModelis created without afile_path, while the generated models in the same module have theirfile_pathset. This inconsistency might cause issues with import resolution or source tracking.🔎 Proposed fix
project_base_model = self.data_model_type( reference=project_base_ref, fields=[], custom_template_dir=self.custom_template_dir, extra_template_data=project_extra_data, keyword_only=self.keyword_only, treat_dot_as_module=self.treat_dot_as_module, + path=models[0].file_path if models and models[0].file_path else None, )docs/cli-reference/quick-reference.md (1)
100-100: Clarify description; avoid “Test …” phrasing for user docsBoth quick-reference entries describe the option as “Test --use-generic-base-class…”, which reads like an internal test note rather than user-facing help. Consider rephrasing to match the CLI help text, e.g., that it “generates a shared base class with model configuration (e.g., extra='forbid') instead of repeating the configuration in each model, keeping code DRY.”
Also applies to: 264-264
docs/cli-reference/model-customization.md (1)
31-31: Rephrase--use-generic-base-classdocs to be descriptive, not test-orientedBoth the options table row and the section intro describe this as “Test --use-generic-base-class…”, which reads like an internal test case rather than feature documentation. It would be clearer to:
- Describe what the option does (e.g., “Generate a shared base model (like
ProjectBaseModel) that centralizes commonmodel_configso individual models don’t repeat it.”).- Optionally note that per-model overrides (like
extra='allow'for specific schemas) still generate their ownmodel_config, as shown in the example.Aligning the wording with the CLI help in
arguments.pywill make the behavior easier to understand.Also applies to: 5168-5171
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (21)
docs/cli-reference/index.md(2 hunks)docs/cli-reference/model-customization.md(2 hunks)docs/cli-reference/quick-reference.md(2 hunks)src/datamodel_code_generator/__init__.py(2 hunks)src/datamodel_code_generator/__main__.py(2 hunks)src/datamodel_code_generator/arguments.py(1 hunks)src/datamodel_code_generator/cli_options.py(1 hunks)src/datamodel_code_generator/parser/base.py(5 hunks)src/datamodel_code_generator/parser/graphql.py(2 hunks)src/datamodel_code_generator/parser/jsonschema.py(2 hunks)src/datamodel_code_generator/parser/openapi.py(2 hunks)tests/data/expected/main/jsonschema/use_generic_base_class.py(1 hunks)tests/data/expected/main/jsonschema/use_generic_base_class_allow_extra.py(1 hunks)tests/data/expected/main/jsonschema/use_generic_base_class_attr_docstrings.py(1 hunks)tests/data/expected/main/jsonschema/use_generic_base_class_dataclass.py(1 hunks)tests/data/expected/main/jsonschema/use_generic_base_class_enum_only.py(1 hunks)tests/data/expected/main/jsonschema/use_generic_base_class_frozen.py(1 hunks)tests/data/expected/main/jsonschema/use_generic_base_class_populate_by_name.py(1 hunks)tests/data/jsonschema/use_generic_base_class_enum_only.json(1 hunks)tests/data/jsonschema/use_generic_base_class_simple.json(1 hunks)tests/main/jsonschema/test_main_jsonschema.py(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (9)
tests/data/expected/main/jsonschema/use_generic_base_class_dataclass.py (4)
tests/data/expected/main/jsonschema/use_generic_base_class_allow_extra.py (1)
Simple(16-17)tests/data/expected/main/jsonschema/use_generic_base_class_attr_docstrings.py (1)
Simple(16-17)tests/data/expected/main/jsonschema/use_generic_base_class_frozen.py (1)
Simple(16-17)tests/data/expected/main/jsonschema/use_generic_base_class_populate_by_name.py (1)
Simple(16-17)
tests/data/expected/main/jsonschema/use_generic_base_class_frozen.py (5)
src/datamodel_code_generator/model/pydantic_v2/__init__.py (1)
ConfigDict(26-41)tests/data/expected/main/jsonschema/use_generic_base_class_allow_extra.py (2)
ProjectBaseModel(10-13)Simple(16-17)tests/data/expected/main/jsonschema/use_generic_base_class_attr_docstrings.py (2)
ProjectBaseModel(10-13)Simple(16-17)tests/data/expected/main/jsonschema/use_generic_base_class_populate_by_name.py (2)
ProjectBaseModel(10-13)Simple(16-17)tests/data/expected/main/jsonschema/use_generic_base_class_dataclass.py (1)
Simple(11-12)
tests/data/expected/main/jsonschema/use_generic_base_class_populate_by_name.py (1)
src/datamodel_code_generator/model/pydantic_v2/__init__.py (1)
ConfigDict(26-41)
tests/data/expected/main/jsonschema/use_generic_base_class_attr_docstrings.py (1)
src/datamodel_code_generator/model/pydantic_v2/__init__.py (1)
ConfigDict(26-41)
tests/data/expected/main/jsonschema/use_generic_base_class_enum_only.py (1)
src/datamodel_code_generator/model/enum.py (1)
Enum(39-120)
tests/data/expected/main/jsonschema/use_generic_base_class_allow_extra.py (1)
src/datamodel_code_generator/model/pydantic_v2/__init__.py (1)
ConfigDict(26-41)
tests/main/jsonschema/test_main_jsonschema.py (2)
tests/main/conftest.py (2)
output_file(98-100)run_main_and_assert(244-408)tests/test_main_kr.py (1)
output_file(44-46)
tests/data/expected/main/jsonschema/use_generic_base_class.py (1)
src/datamodel_code_generator/model/pydantic_v2/__init__.py (1)
ConfigDict(26-41)
src/datamodel_code_generator/parser/base.py (2)
src/datamodel_code_generator/model/base.py (4)
DataModel(468-737)path(716-718)name(651-653)BaseClassDataType(461-462)src/datamodel_code_generator/reference.py (3)
ModelResolver(492-940)Reference(141-200)reference(76-78)
🪛 Ruff (0.14.8)
src/datamodel_code_generator/parser/base.py
648-648: Unused noqa directive (non-enabled: PLR0912, PLR0913, PLR0915)
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). (11)
- GitHub Check: py312-isort5 on Ubuntu
- GitHub Check: 3.11 on macOS
- GitHub Check: 3.11 on Windows
- GitHub Check: 3.10 on Windows
- GitHub Check: 3.10 on macOS
- GitHub Check: 3.12 on Windows
- GitHub Check: 3.12 on macOS
- GitHub Check: 3.14 on Windows
- GitHub Check: 3.13 on Windows
- GitHub Check: Analyze (python)
- GitHub Check: benchmarks
🔇 Additional comments (20)
tests/data/expected/main/jsonschema/use_generic_base_class_dataclass.py (1)
10-12: Correct handling for dataclass output.The absence of a ProjectBaseModel is appropriate since dataclasses don't support Pydantic's ConfigDict. The feature correctly skips base class generation for this output model type.
tests/data/expected/main/jsonschema/use_generic_base_class_populate_by_name.py (1)
10-17: Well-structured base class pattern.The ProjectBaseModel correctly centralizes the
populate_by_nameconfiguration, and the Simple model inherits it without repeating the config. This demonstrates the DRY principle that the feature aims to achieve.tests/data/expected/main/jsonschema/use_generic_base_class_frozen.py (1)
10-17: LGTM!The frozen configuration is correctly centralized in ProjectBaseModel, demonstrating proper use of the generic base class pattern for immutable models.
tests/data/expected/main/jsonschema/use_generic_base_class_attr_docstrings.py (1)
10-17: LGTM!The
use_attribute_docstringsconfiguration is properly centralized in the ProjectBaseModel, allowing all derived models to inherit this behavior.tests/data/expected/main/jsonschema/use_generic_base_class.py (1)
10-20: LGTM!The ProjectBaseModel correctly establishes a shared
extra='forbid'configuration, and Foo properly overrides it withextra='allow'to customize its behavior.tests/data/expected/main/jsonschema/use_generic_base_class_enum_only.py (1)
13-14: Verify necessity of placeholder Model for enum-only schemas.When the schema contains only enum definitions (no actual object models), the generator creates a generic
Model(RootModel[Any])placeholder. Consider whether this is necessary or if the generator should skip model creation entirely for enum-only schemas.#!/bin/bash # Check if this placeholder model generation is pre-existing behavior or new to this PR # by looking for similar patterns in other test files echo "=== Searching for other enum-only test cases ===" fd -e json . tests/data/jsonschema | xargs grep -l '"enum"' | head -5 echo -e "\n=== Checking expected outputs for enum-only patterns ===" rg -l "class Model\(RootModel\[Any\]\)" tests/data/expected/src/datamodel_code_generator/parser/base.py (2)
820-851: LGTM!The conditional routing of configuration options to either
generic_base_class_configorextra_template_data[ALL_MODEL]is implemented correctly and follows the existing pattern.
2665-2666: LGTM!The invocation of
__apply_generic_base_classis correctly placed after model processing and before import collection, ensuring theProjectBaseModelis included in the final output.src/datamodel_code_generator/__init__.py (1)
406-406: LGTM!The
use_generic_base_classparameter is correctly added to thegenerate()function signature and properly passed through to the parser instantiation.Also applies to: 654-654
tests/data/expected/main/jsonschema/use_generic_base_class_allow_extra.py (1)
1-17: LGTM!The expected output correctly demonstrates the
--use-generic-base-classfeature:ProjectBaseModelcentralizes themodel_config, and derived classes inherit it without repetition.src/datamodel_code_generator/cli_options.py (1)
103-103: LGTM!The CLI option metadata for
--use-generic-base-classis correctly added under the Model Customization category, following the established pattern.docs/cli-reference/index.md (1)
14-14: LGTM!The documentation index is correctly updated with the incremented option count and the new
--use-generic-base-classentry in proper alphabetical order.Also applies to: 170-170
src/datamodel_code_generator/arguments.py (1)
272-278: New CLI flag wiring looks consistentThe
--use-generic-base-classoption is added in the right group with clear help text, uses the standardstore_true+default=Nonepattern, and matches the Config field name. This should integrate cleanly with existing arg parsing and pyproject overrides.src/datamodel_code_generator/parser/openapi.py (1)
180-279: OpenAPI parser correctly forwardsuse_generic_base_classAdding
use_generic_base_classas a keyword-only parameter and passing it through tosuper().__init__keeps OpenAPI generation in sync with the base JsonSchema parser’s configuration, without breaking existing call sites.Also applies to: 282-376
src/datamodel_code_generator/__main__.py (1)
394-394: Config and generator wiring foruse_generic_base_classlooks correctThe new
Config.use_generic_base_classfield and its propagation intogenerate()viarun_generate_from_configfollow the existing pattern for boolean options. With argparse usingdefault=None, CLI usage will override pyproject config when set, and otherwise respect the config value.Also applies to: 700-701
tests/main/jsonschema/test_main_jsonschema.py (1)
3715-3841: Excellent test coverage for the new --use-generic-base-class feature!The test suite comprehensively covers:
- Basic usage with
--extra-fields forbid- Interaction with other flags (populate-by-field-name, allow-extra-fields, faux-immutability, attribute-docstrings)
- Edge cases (dataclasses where the feature has no effect, enum-only schemas)
All tests follow the established patterns in the file and include clear docstrings. The first test includes the
@pytest.mark.cli_docdecorator for documentation generation, which is appropriate for a new CLI option.src/datamodel_code_generator/parser/jsonschema.py (2)
534-534: LGTM: Parameter addition is correct.The
use_generic_base_classparameter is properly typed and has a sensible default value (False) that maintains backward compatibility.
630-630: LGTM: Parameter properly forwarded to parent class.The parameter is correctly passed to the parent class
super().__init__()call, following the same pattern as other configuration parameters.src/datamodel_code_generator/parser/graphql.py (2)
122-122: LGTM: Parameter addition is correct and consistent.The
use_generic_base_classparameter is properly typed with a backward-compatible default value (False), matching the implementation inJsonSchemaParser.
217-217: LGTM: Parameter properly forwarded to parent class.The parameter forwarding is correct and consistent with the pattern used in
JsonSchemaParser, ensuring uniform behavior across parser types.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/datamodel_code_generator/parser/base.py (2)
648-648: Remove unusednoqadirective.Static analysis indicates the
noqadirective forPLR0912,PLR0913,PLR0915is no longer needed on this line.🔎 Proposed fix
- def __init__( # noqa: PLR0912, PLR0913, PLR0915 + def __init__(
2025-2025: Consider making the base class name configurable in the future.The
ProjectBaseModelname is currently hardcoded. While acceptable for the initial implementation, users may want to customize this name to match their project conventions.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/datamodel_code_generator/parser/base.py(5 hunks)tests/data/expected/main/jsonschema/use_generic_base_class_with_inheritance.py(1 hunks)tests/data/jsonschema/use_generic_base_class_with_inheritance.json(1 hunks)tests/main/jsonschema/test_main_jsonschema.py(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
tests/data/expected/main/jsonschema/use_generic_base_class_with_inheritance.py (2)
src/datamodel_code_generator/model/pydantic_v2/__init__.py (1)
ConfigDict(26-41)src/datamodel_code_generator/model/base.py (1)
name(651-653)
🪛 Ruff (0.14.8)
src/datamodel_code_generator/parser/base.py
648-648: Unused noqa directive (non-enabled: PLR0912, PLR0913, PLR0915)
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-isort7 on Ubuntu
- GitHub Check: 3.10 on Ubuntu
- GitHub Check: py312-black22 on Ubuntu
- GitHub Check: py312-isort6 on Ubuntu
- GitHub Check: 3.10 on Windows
- GitHub Check: 3.12 on Windows
- GitHub Check: 3.11 on Windows
- GitHub Check: 3.14 on Windows
- GitHub Check: Analyze (python)
- GitHub Check: benchmarks
🔇 Additional comments (7)
tests/data/expected/main/jsonschema/use_generic_base_class_with_inheritance.py (1)
12-27: LGTM! Expected output correctly demonstrates the inheritance preservation.The test fixture properly validates the key behavior of
--use-generic-base-class:
ProjectBaseModelis generated with the sharedmodel_configAnimalinherits only fromProjectBaseModel(no schema-defined parent)Dogcorrectly inherits from bothProjectBaseModelandAnimal, preserving the schema-defined inheritance from theallOfwhile prepending the generic baseModel(RootModel[Any])is correctly excluded from the transformationsrc/datamodel_code_generator/parser/base.py (4)
820-851: LGTM! Config routing logic is clean and consistent.The conditional routing pattern correctly separates per-model configuration from the shared base class configuration when
use_generic_base_classis enabled.
2010-2069: Well-implemented inheritance preservation logic.The implementation correctly addresses the schema inheritance concern:
- Line 2065 properly detects schema-defined inheritance by checking
any(bc.reference for bc in model.base_classes)- Lines 2066-2067 use
insert(0, ...)to prependProjectBaseModelwhile preserving existing base classes- Lines 2068-2069 only replace when there's no schema inheritance to preserve
This ensures models like
Dog(which inherits fromAnimalviaallOf) correctly becomeDog(ProjectBaseModel, Animal)rather than losing theAnimalparent.
2042-2059: LGTM! ProjectBaseModel creation is properly configured.The synthetic reference path
#/ProjectBaseModeland model instantiation with the collected configuration options are correctly implemented.
2669-2670: LGTM! Method invocation is correctly positioned.Calling
__apply_generic_base_classafter model processing but before import finalization ensures theProjectBaseModelis properly integrated into the module structure.tests/data/jsonschema/use_generic_base_class_with_inheritance.json (1)
1-28: LGTM! Well-designed test fixture for inheritance scenario.The JSON Schema correctly uses
allOfwith$refto establish theDog extends Animalinheritance relationship. This provides good coverage for validating that--use-generic-base-classpreserves schema-defined inheritance while prepending the shared base class.tests/main/jsonschema/test_main_jsonschema.py (1)
3715-3858: LGTM! Comprehensive test coverage for the new feature.The test suite thoroughly validates the
--use-generic-base-classfeature across multiple scenarios:
- Basic functionality with various Pydantic v2 config options (forbid, populate by name, allow extra, frozen)
- Edge cases like enum-only schemas and dataclass compatibility
- Integration with existing features like attribute docstrings and schema inheritance
The tests follow established patterns in the file and have clear, descriptive docstrings.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
src/datamodel_code_generator/parser/base.py (1)
2063-2073: The existing review comment appears to be incorrect.The logic here correctly handles three cases:
- Model inherits from another base model (line 2068-2069): Do nothing, because the parent will have
ProjectBaseModelin its hierarchy, giving transitive inheritance.- Model has schema-defined parents (line 2070-2071): Use
insert(0, project_base_dt)to prependProjectBaseModel, preserving existing base classes.- Model has no parents (line 2072-2073): Set
ProjectBaseModelas the sole base.The
insert(0, ...)at line 2071 means schema-defined inheritance (e.g., viaallOf) is augmented, not replaced. Only models with no parents get replacement (line 2073), which is correct.The test file confirms this:
Doginherits fromAnimal, which inherits fromProjectBaseModel. When processingDog,has_base_model_parentis True (Animal is in the set), so we hit thepassclause and preserveDog's inheritance fromAnimal.
🧹 Nitpick comments (1)
src/datamodel_code_generator/parser/base.py (1)
648-648: Optional cleanup: Remove unused noqa directive.Ruff reports that the codes in this
noqadirective (PLR0912,PLR0913,PLR0915) are not enabled in your linter configuration, making the directive unnecessary.🔎 Proposed fix
- def __init__( # noqa: PLR0912, PLR0913, PLR0915 + def __init__( self, source: str | Path | list[Path] | ParseResult,
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/datamodel_code_generator/parser/base.py(5 hunks)tests/data/expected/main/jsonschema/use_generic_base_class_with_inheritance.py(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
tests/data/expected/main/jsonschema/use_generic_base_class_with_inheritance.py (2)
src/datamodel_code_generator/model/pydantic_v2/__init__.py (1)
ConfigDict(26-41)src/datamodel_code_generator/model/base.py (1)
name(651-653)
🪛 Ruff (0.14.8)
src/datamodel_code_generator/parser/base.py
648-648: Unused noqa directive (non-enabled: PLR0912, PLR0913, PLR0915)
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: CodeQL
- GitHub Check: benchmarks
- GitHub Check: py312-pydantic1 on Ubuntu
- GitHub Check: py312-isort7 on Ubuntu
- GitHub Check: 3.11 on Ubuntu
- GitHub Check: py312-isort6 on Ubuntu
- GitHub Check: 3.10 on Windows
- GitHub Check: 3.11 on Windows
- GitHub Check: 3.12 on Windows
- GitHub Check: Analyze (python)
🔇 Additional comments (3)
tests/data/expected/main/jsonschema/use_generic_base_class_with_inheritance.py (1)
22-27: LGTM! The inheritance hierarchy is correct.The inheritance chain
Dog -> Animal -> ProjectBaseModel -> BaseModelis valid and won't cause MRO issues. The GitHub Advanced Security warning appears to be a false positive—there's no multiple inheritance or diamond problem here that could break method resolution order.The design correctly demonstrates the feature:
AnimalgetsProjectBaseModelas its base (which carries the shared config), andDoginherits fromAnimal, transitively receiving the config without needing to referenceProjectBaseModeldirectly.src/datamodel_code_generator/parser/base.py (2)
648-671: LGTM! Clean feature flag integration.The constructor changes properly introduce the
use_generic_base_classflag with backward-compatible defaults, and the config routing logic (lines 823-851) consistently channels model configuration to either the sharedgeneric_base_class_configor per-modelextra_template_databased on the flag.Also applies to: 820-851
2673-2673: LGTM! Correct placement in the processing pipeline.The call to
__apply_generic_base_classis correctly positioned after all model processing (deduplication, sorting, inheritance resolution) but before final code generation. This ensures the shared base class is available when imports and final output are rendered.
koxudaxi
force-pushed
the
fix/use-generic-base-class
branch
3 times, most recently
from
1910851 to
76d207b
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
src/datamodel_code_generator/model/base.py (1)
712-726: Clean template method pattern for base class factory.The method correctly returns
Noneby default, allowing subclasses to override with their implementation. TheARG003noqa directives are appropriate since these parameters are intentionally unused in the base class.However, the
FBT001andFBT002noqa directives on lines 718-719 are unused per Ruff since these are keyword-only parameters (after*in signature would make them positional-only restricted, but here they have defaults making them keyword arguments).🔎 Remove unused noqa directives
@classmethod def create_base_class_model( cls, config: dict[str, Any], # noqa: ARG003 reference: Reference, # noqa: ARG003 custom_template_dir: Path | None = None, # noqa: ARG003 - keyword_only: bool = False, # noqa: ARG003, FBT001, FBT002 - treat_dot_as_module: bool = False, # noqa: ARG003, FBT001, FBT002 + keyword_only: bool = False, # noqa: ARG003 + treat_dot_as_module: bool = False, # noqa: ARG003 ) -> tuple[DataModel, str] | None:src/datamodel_code_generator/model/pydantic_v2/base_model.py (1)
276-313: Well-implemented generic base class factory.The implementation correctly:
- Creates a
BaseModelwith configuration fromALL_MODELtemplate data- Aliases
pydantic.BaseModelas_BaseModelto avoid name collision with the generated class- Filters existing imports and adds the aliased import
Minor: The
FBT001/FBT002noqa directives on lines 282-283 are unused per Ruff.🔎 Remove unused noqa directives
@classmethod def create_base_class_model( cls, config: dict[str, Any], reference: Reference, custom_template_dir: Path | None = None, - keyword_only: bool = False, # noqa: FBT001, FBT002 - treat_dot_as_module: bool = False, # noqa: FBT001, FBT002 + keyword_only: bool = False, + treat_dot_as_module: bool = False, ) -> tuple[BaseModel, str] | None:src/datamodel_code_generator/parser/base.py (3)
2060-2061: Consider adding a clarifying comment for thepassstatement.The
passon line 2061 is intentional (transitive inheritance), but a brief comment would improve readability for future maintainers.🔎 Add clarifying comment
if has_target_model_parent: - pass + pass # Base class will be inherited through the parent model elif parent_refs:
2055-2065: Inheritance logic correctly preserves schema-defined parents.The three-case logic is sound:
- Model already inherits from another target model → skip (base class inherited transitively)
- Model has other parent refs → prepend base class (preserves schema inheritance)
- Model has no parents → set base class as sole parent
One minor note:
base_class_dtis shared across all target models. Currently this is safe sinceBaseClassDataTypewith onlytypeset has no parent relationships that would cross-contaminate models. However, future changes that introduce parent tracking on base class types could make this problematic. Creating a new instance per model would be more defensive.
2046-2047: Consider logging a warning when--use-generic-base-classis used with unsupported model types.The feature currently has no effect for dataclasses and other non-Pydantic v2 model types, but users receive no feedback about this. While the silent return is intentional design (as evidenced by tests like
test_main_use_generic_base_class_dataclass), adding a warning would improve user experience. The codebase already uses thewarnings.warn()pattern in similar scenarios (e.g.,openapi.py,jsonschema.py), so adding one here would be consistent.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (17)
docs/cli-reference/model-customization.md(2 hunks)docs/cli-reference/quick-reference.md(2 hunks)src/datamodel_code_generator/arguments.py(1 hunks)src/datamodel_code_generator/model/base.py(2 hunks)src/datamodel_code_generator/model/pydantic_v2/base_model.py(2 hunks)src/datamodel_code_generator/parser/base.py(6 hunks)tests/data/expected/main/jsonschema/use_generic_base_class.py(1 hunks)tests/data/expected/main/jsonschema/use_generic_base_class_allow_extra.py(1 hunks)tests/data/expected/main/jsonschema/use_generic_base_class_attr_docstrings.py(1 hunks)tests/data/expected/main/jsonschema/use_generic_base_class_frozen.py(1 hunks)tests/data/expected/main/jsonschema/use_generic_base_class_module_split/__init__.py(1 hunks)tests/data/expected/main/jsonschema/use_generic_base_class_module_split/animal.py(1 hunks)tests/data/expected/main/jsonschema/use_generic_base_class_module_split/dog.py(1 hunks)tests/data/expected/main/jsonschema/use_generic_base_class_module_split/model.py(1 hunks)tests/data/expected/main/jsonschema/use_generic_base_class_populate_by_name.py(1 hunks)tests/data/expected/main/jsonschema/use_generic_base_class_with_inheritance.py(1 hunks)tests/main/jsonschema/test_main_jsonschema.py(2 hunks)
✅ Files skipped from review due to trivial changes (1)
- tests/data/expected/main/jsonschema/use_generic_base_class_module_split/init.py
🚧 Files skipped from review as they are similar to previous changes (7)
- docs/cli-reference/quick-reference.md
- docs/cli-reference/model-customization.md
- tests/data/expected/main/jsonschema/use_generic_base_class_with_inheritance.py
- tests/data/expected/main/jsonschema/use_generic_base_class_attr_docstrings.py
- tests/data/expected/main/jsonschema/use_generic_base_class_frozen.py
- src/datamodel_code_generator/arguments.py
- tests/data/expected/main/jsonschema/use_generic_base_class.py
🧰 Additional context used
🧬 Code graph analysis (2)
src/datamodel_code_generator/model/pydantic_v2/base_model.py (2)
src/datamodel_code_generator/model/base.py (3)
imports(283-302)imports(637-642)BaseClassDataType(463-464)src/datamodel_code_generator/imports.py (1)
Import(20-38)
tests/data/expected/main/jsonschema/use_generic_base_class_populate_by_name.py (2)
src/datamodel_code_generator/model/pydantic_v2/base_model.py (1)
BaseModel(170-314)src/datamodel_code_generator/model/pydantic_v2/__init__.py (1)
ConfigDict(26-41)
🪛 GitHub Check: CodeQL
src/datamodel_code_generator/model/base.py
[notice] 52-52: Unused global variable
The global variable 'GENERIC_BASE_CLASS_PATH' is not used.
[notice] 53-53: Unused global variable
The global variable 'GENERIC_BASE_CLASS_NAME' is not used.
🪛 Ruff (0.14.8)
src/datamodel_code_generator/model/pydantic_v2/base_model.py
282-282: Unused noqa directive (non-enabled: FBT001, FBT002)
Remove unused noqa directive
(RUF100)
283-283: Unused noqa directive (non-enabled: FBT001, FBT002)
Remove unused noqa directive
(RUF100)
src/datamodel_code_generator/model/base.py
718-718: Unused noqa directive (non-enabled: FBT001, FBT002)
Remove unused noqa directive
(RUF100)
719-719: Unused noqa directive (non-enabled: FBT001, FBT002)
Remove unused noqa directive
(RUF100)
src/datamodel_code_generator/parser/base.py
650-650: Unused noqa directive (non-enabled: PLR0912, PLR0913, PLR0915)
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). (9)
- GitHub Check: 3.13 on Windows
- GitHub Check: py312-black23 on Ubuntu
- GitHub Check: py312-isort5 on Ubuntu
- GitHub Check: 3.10 on Windows
- GitHub Check: 3.14 on Windows
- GitHub Check: 3.11 on Windows
- GitHub Check: 3.12 on Windows
- GitHub Check: Analyze (python)
- GitHub Check: benchmarks
🔇 Additional comments (9)
tests/data/expected/main/jsonschema/use_generic_base_class_populate_by_name.py (1)
1-18: LGTM! Test expectation correctly demonstrates the DRY pattern.This expected output file correctly demonstrates the new
--use-generic-base-classfeature:
- Proper aliasing (line 7): Pydantic's
BaseModelis imported as_BaseModelto avoid naming conflicts with the custom base class.- Centralized configuration (lines 11-14): The custom
BaseModeldefinesmodel_configonce withpopulate_by_name=True.- DRY inheritance (lines 17-18): The
Simplemodel inherits from the customBaseModeland automatically receives the configuration without repeating themodel_configblock.This achieves the objective from issue #2501 of eliminating repeated
model_configblocks across generated models.tests/data/expected/main/jsonschema/use_generic_base_class_allow_extra.py (4)
1-4: LGTM! Metadata header is correct.The generated file header correctly documents the generator, input schema filename, and timestamp.
5-8: LGTM! Import structure correctly supports Pydantic v2.The imports properly alias
_BaseModelto avoid collision with the customBaseModelclass, and use Pydantic v2'sConfigDictpattern.
17-18: LGTM! Demonstrates the DRY benefit of the feature.The
Simpleclass correctly inherits from the customBaseModel, gaining the sharedmodel_configwithout repetition. This validates the PR objective of eliminating redundant configuration blocks.
11-14: LGTM! Correctly implements shared base class with DRY pattern.The
BaseModelclass correctly uses Pydantic v2'smodel_config = ConfigDict(extra='allow')pattern, which allows additional fields on model instances. By defining this configuration once in the shared base class, all inheriting models avoid repeating the configuration.src/datamodel_code_generator/model/base.py (1)
52-53: LGTM! Constants for generic base class feature.These constants are properly imported and used in
src/datamodel_code_generator/parser/base.py. The CodeQL warning about unused globals is a false positive at the file level since these are exported for use elsewhere.src/datamodel_code_generator/parser/base.py (1)
822-853: Config routing for generic base class is correct.The conditional logic properly routes configuration options to either
generic_base_class_config(when the feature is enabled) orextra_template_data[ALL_MODEL](legacy behavior). This ensures backward compatibility while enabling the DRY pattern.tests/main/jsonschema/test_main_jsonschema.py (1)
3716-3878: LGTM! Comprehensive test coverage for the new feature.The nine new test functions provide thorough coverage of the
--use-generic-base-classfeature across different scenarios:
- Basic usage with extra='forbid'
- Interaction with other flags (populate_by_name, allow_extra, frozen, attribute_docstrings)
- Different output types (dataclass)
- Edge cases (enum-only schemas, inheritance, module-split)
The tests are well-structured, properly documented with cli_doc markers, and follow the existing test patterns consistently.
tests/data/expected/main/jsonschema/use_generic_base_class_module_split/dog.py (1)
13-16: This appears to be intentional design for module-split mode, not a violation of DRY principlesThe duplicate
BaseModeldefinitions indog.pyandanimal.pyare expected in module-split mode. The test docstring confirms this is intentional: "Test --use-generic-base-class with module split mode (cross-module inheritance)." When modules are split, each module defines its own base class to remain self-contained. Unlike the non-split version which uses a singleBaseModel, this module-split pattern trades DRY compliance for module independence.The alternative of importing
BaseModelfromanimal.pyindog.pywould reduce duplication but would create a cross-module dependency that conflicts with the intended module-split design goal.
koxudaxi
force-pushed
the
fix/use-generic-base-class
branch
from
76d207b to
4983a03
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (2)
src/datamodel_code_generator/model/base.py (1)
52-53: Unused constants flagged by static analysis.
GENERIC_BASE_CLASS_PATHandGENERIC_BASE_CLASS_NAMEare defined but not used in this file. If these are intended for use elsewhere, ensure they're imported and used appropriately. Otherwise, consider removing them.#!/bin/bash # Check if these constants are used anywhere in the codebase echo "=== Searching for GENERIC_BASE_CLASS_PATH usage ===" rg -n "GENERIC_BASE_CLASS_PATH" --type py echo "" echo "=== Searching for GENERIC_BASE_CLASS_NAME usage ===" rg -n "GENERIC_BASE_CLASS_NAME" --type pysrc/datamodel_code_generator/model/msgspec.py (1)
23-23: Cyclic import already flagged by static analysis.This cyclic import pattern exists and has been previously noted. The import is required for the
BaseClassDataTypeusage in the newcreate_base_class_modelmethod.
🧹 Nitpick comments (6)
src/datamodel_code_generator/model/base.py (1)
713-727: LGTM with optional cleanup.The base implementation correctly provides a default
Nonereturn, allowing model-specific subclasses to override with their implementations. The method signature appropriately matches the concrete implementations inpydantic_v2/base_model.pyandmsgspec.py.Optional: Remove unused noqa directives
Ruff indicates the
FBT001, FBT002noqa directives on lines 719-720 are unused (likely the rules aren't enabled). You could simplify to justARG003:- keyword_only: bool = False, # noqa: ARG003, FBT001, FBT002 - treat_dot_as_module: bool = False, # noqa: ARG003, FBT001, FBT002 + keyword_only: bool = False, # noqa: ARG003 + treat_dot_as_module: bool = False, # noqa: ARG003src/datamodel_code_generator/model/msgspec.py (2)
163-164: Remove unusednoqadirectives.Static analysis indicates the
FBT001andFBT002rules are not enabled, making thesenoqadirectives unnecessary.🔎 Proposed fix
- keyword_only: bool = False, # noqa: FBT001, FBT002 - treat_dot_as_module: bool = False, # noqa: FBT001, FBT002 + keyword_only: bool = False, + treat_dot_as_module: bool = False,
165-203: Return type includesNonebut the method never returnsNone.The return type is declared as
tuple[Struct, str] | None, but the method always returns(base_model, base_class_name). Either simplify the return type totuple[Struct, str], or add documentation explaining whenNonewould be returned (e.g., for future extensibility or consistency with a protocol).🔎 Proposed simplification
If
Noneis not expected, simplify the return type:) -> tuple[Struct, str] | None: + ) -> tuple[Struct, str]:Alternatively, if the
| Noneis intentional for interface consistency with other implementations, consider adding a docstring note explaining this.src/datamodel_code_generator/model/pydantic_v2/base_model.py (2)
282-283: Remove unusednoqadirectives.Static analysis indicates the
FBT001andFBT002rules are not enabled, making thesenoqadirectives unnecessary.🔎 Proposed fix
- keyword_only: bool = False, # noqa: FBT001, FBT002 - treat_dot_as_module: bool = False, # noqa: FBT001, FBT002 + keyword_only: bool = False, + treat_dot_as_module: bool = False,
284-284: Return type includesNonebut the method never returnsNone.Same as in
msgspec.py: the return type istuple[BaseModel, str] | None, but the method always returns a tuple. Consider simplifying totuple[BaseModel, str]unlessNoneis intentionally reserved for future use or protocol conformance.src/datamodel_code_generator/parser/base.py (1)
650-650: Consider removing the unusednoqadirective.The static analysis tool indicates that the suppression codes
PLR0912,PLR0913,PLR0915are not currently enabled in your linter configuration.🔎 Cleanup suggestion
- def __init__( # noqa: PLR0912, PLR0913, PLR0915 + def __init__(
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
docs/cli-reference/model-customization.mddocs/cli-reference/quick-reference.mdsrc/datamodel_code_generator/arguments.pysrc/datamodel_code_generator/model/base.pysrc/datamodel_code_generator/model/enum.pysrc/datamodel_code_generator/model/msgspec.pysrc/datamodel_code_generator/model/pydantic_v2/base_model.pysrc/datamodel_code_generator/model/type_alias.pysrc/datamodel_code_generator/parser/base.pytests/data/expected/main/jsonschema/use_generic_base_class_msgspec.pytests/data/expected/main/jsonschema/use_generic_base_class_msgspec_forbid.pytests/main/jsonschema/test_main_jsonschema.py
🚧 Files skipped from review as they are similar to previous changes (2)
- docs/cli-reference/quick-reference.md
- src/datamodel_code_generator/arguments.py
🧰 Additional context used
🧬 Code graph analysis (2)
src/datamodel_code_generator/model/base.py (3)
src/datamodel_code_generator/model/pydantic_v2/base_model.py (1)
create_base_class_model(277-314)src/datamodel_code_generator/model/msgspec.py (1)
create_base_class_model(158-203)src/datamodel_code_generator/reference.py (2)
reference(76-78)Reference(141-200)
tests/main/jsonschema/test_main_jsonschema.py (1)
tests/main/conftest.py (2)
output_file(98-100)output_dir(104-106)
🪛 GitHub Check: CodeQL
src/datamodel_code_generator/model/base.py
[notice] 52-52: Unused global variable
The global variable 'GENERIC_BASE_CLASS_PATH' is not used.
[notice] 53-53: Unused global variable
The global variable 'GENERIC_BASE_CLASS_NAME' is not used.
src/datamodel_code_generator/model/msgspec.py
[notice] 23-23: Cyclic import
Import of module datamodel_code_generator.model.base begins an import cycle.
🪛 Ruff (0.14.8)
src/datamodel_code_generator/model/pydantic_v2/base_model.py
282-282: Unused noqa directive (non-enabled: FBT001, FBT002)
Remove unused noqa directive
(RUF100)
283-283: Unused noqa directive (non-enabled: FBT001, FBT002)
Remove unused noqa directive
(RUF100)
src/datamodel_code_generator/parser/base.py
650-650: Unused noqa directive (non-enabled: PLR0912, PLR0913, PLR0915)
Remove unused noqa directive
(RUF100)
src/datamodel_code_generator/model/base.py
719-719: Unused noqa directive (non-enabled: FBT001, FBT002)
Remove unused noqa directive
(RUF100)
720-720: Unused noqa directive (non-enabled: FBT001, FBT002)
Remove unused noqa directive
(RUF100)
src/datamodel_code_generator/model/msgspec.py
163-163: Unused noqa directive (non-enabled: FBT001, FBT002)
Remove unused noqa directive
(RUF100)
164-164: Unused noqa directive (non-enabled: FBT001, FBT002)
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: 3.12 on macOS
- GitHub Check: py312-black23 on Ubuntu
- GitHub Check: py312-isort5 on Ubuntu
- GitHub Check: 3.12 on Windows
- GitHub Check: 3.11 on Windows
- GitHub Check: py312-isort7 on Ubuntu
- GitHub Check: 3.10 on Windows
- GitHub Check: 3.13 on Windows
- GitHub Check: Analyze (python)
- GitHub Check: benchmarks
🔇 Additional comments (10)
src/datamodel_code_generator/model/enum.py (1)
45-45: LGTM! The flag correctly indicates Enum doesn't support generic base classes.Setting
SUPPORTS_GENERIC_BASE_CLASS = Falseis correct because Enum types have a fixed inheritance hierarchy (must inherit fromenum.Enumor its variants) and cannot participate in the generic base class pattern. The attribute is consistently defined across DataModel subclasses—the base class defaults toTrue, while special types like Enum and TypeAlias that cannot use the generic pattern override it toFalse. The parser properly checks this flag to filter which models receive the generic base class treatment.src/datamodel_code_generator/model/type_alias.py (1)
24-25: LGTM!The addition of
SUPPORTS_GENERIC_BASE_CLASS: ClassVar[bool] = Falsecorrectly indicates that type aliases don't support the generic base class feature, which makes sense since they're aliases rather than model classes.tests/data/expected/main/jsonschema/use_generic_base_class_msgspec.py (1)
1-19: LGTM!The expected output correctly demonstrates the generic base class pattern for msgspec: a shared
Structbase class withfrozen=Trueconfiguration, andSimpleinheriting from it without repeating the configuration.tests/data/expected/main/jsonschema/use_generic_base_class_msgspec_forbid.py (1)
1-19: LGTM!The expected output correctly demonstrates the generic base class pattern for msgspec with both
forbid_unknown_fields=Trueandfrozen=Trueconfigurations consolidated in the sharedStructbase class.src/datamodel_code_generator/model/base.py (1)
479-480: LGTM!The addition of
SUPPORTS_GENERIC_BASE_CLASS: ClassVar[bool] = Trueas default forDataModelprovides a clean extension point. Model types that don't support generic base classes (likeTypeAliasBase) can override this toFalse.tests/main/jsonschema/test_main_jsonschema.py (3)
30-30: LGTM: New import for test path constant.The
EXPECTED_MAIN_PATHimport is correctly added to support the module split test at line 3877.
3716-3737: Well-structured CLI documentation test for the new feature.The test properly uses the
@pytest.mark.cli_docdecorator for documentation and follows the established pattern for testing CLI options withrun_main_and_assert.
3862-3878: Good coverage for module split mode with generic base class.This test verifies the cross-module inheritance scenario, which is a critical edge case for the DRY base class feature when models are split across multiple files.
src/datamodel_code_generator/model/pydantic_v2/base_model.py (2)
10-10: LGTM: Import reorganization for new functionality.Moving
defaultdictto runtime imports and addingImport,ALL_MODEL,UNDEFINED, andBaseClassDataTypesupports the newcreate_base_class_modelmethod.Also applies to: 16-17
306-312: Well-implemented import aliasing to avoid name conflicts.The logic correctly:
- Filters out existing
pydantic.BaseModelimports- Adds the aliased import as
_BaseModelThis ensures the generated base class inherits from the aliased import without circular naming issues.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
src/datamodel_code_generator/parser/base.py (1)
2049-2057: Cross-module inheritance may create redundant base classes.When a model in one module inherits from a model in another module and both modules apply the generic base class, the child ends up with duplicate inheritance. For example, if
Dog(module2) inherits fromAnimal(module1):
- module1 generates:
BaseModel,Animal(BaseModel)- module2 generates:
BaseModel,Dog(BaseModel, Animal)Result:
Doginherits from bothmodule2.BaseModelandAnimal(which already inherits frommodule1.BaseModel), creating redundant and potentially confusing inheritance.Root cause: Line 2051 checks
has_target_model_parent = any(ref.source in target_model_set ...)which only considers parents within the current module. It does not check whether any ancestor from another module already has the generic base class applied.Suggested fix: Before adding the generic base at line 2055, check whether any ancestor (transitively, across modules) already inherits from a model with a name matching the generic base class (e.g., "BaseModel" or "Struct"). If so, skip adding it to avoid duplication.
Note: This issue was previously flagged in past reviews; confirming it remains unresolved in the current implementation.
🧹 Nitpick comments (4)
src/datamodel_code_generator/model/msgspec.py (1)
163-164: Remove unusednoqadirectives.Ruff reports that
FBT001andFBT002are not enabled in your configuration, making these suppression comments unnecessary.🔎 Cleanup suggestion
- def create_base_class_model( - cls, - config: dict[str, Any], - reference: Reference, - custom_template_dir: Path | None = None, - keyword_only: bool = False, # noqa: FBT001, FBT002 - treat_dot_as_module: bool = False, # noqa: FBT001, FBT002 - ) -> Struct | None: + def create_base_class_model( + cls, + config: dict[str, Any], + reference: Reference, + custom_template_dir: Path | None = None, + keyword_only: bool = False, + treat_dot_as_module: bool = False, + ) -> Struct | None:src/datamodel_code_generator/model/base.py (1)
719-720: Remove unusednoqadirectives.The
FBT001andFBT002suppressions are unnecessary since these rules are not enabled.🔎 Cleanup suggestion
@classmethod def create_base_class_model( cls, config: dict[str, Any], # noqa: ARG003 reference: Reference, # noqa: ARG003 custom_template_dir: Path | None = None, # noqa: ARG003 - keyword_only: bool = False, # noqa: ARG003, FBT001, FBT002 - treat_dot_as_module: bool = False, # noqa: ARG003, FBT001, FBT002 + keyword_only: bool = False, # noqa: ARG003 + treat_dot_as_module: bool = False, # noqa: ARG003 ) -> DataModel | None:src/datamodel_code_generator/model/pydantic_v2/base_model.py (1)
282-283: Remove unusednoqadirectives.The
FBT001andFBT002suppressions are not needed.🔎 Cleanup suggestion
@classmethod def create_base_class_model( cls, config: dict[str, Any], reference: Reference, custom_template_dir: Path | None = None, - keyword_only: bool = False, # noqa: FBT001, FBT002 - treat_dot_as_module: bool = False, # noqa: FBT001, FBT002 + keyword_only: bool = False, + treat_dot_as_module: bool = False, ) -> BaseModel | None:src/datamodel_code_generator/parser/base.py (1)
650-650: Remove unusednoqadirective.Ruff indicates the
PLR0912,PLR0913, andPLR0915suppressions are not necessary.🔎 Cleanup suggestion
- def __init__( # noqa: PLR0912, PLR0913, PLR0915 + def __init__( self, source: str | Path | list[Path] | ParseResult,
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/datamodel_code_generator/model/base.pysrc/datamodel_code_generator/model/msgspec.pysrc/datamodel_code_generator/model/pydantic_v2/base_model.pysrc/datamodel_code_generator/parser/base.py
🧰 Additional context used
🧬 Code graph analysis (3)
src/datamodel_code_generator/parser/base.py (2)
src/datamodel_code_generator/model/msgspec.py (1)
create_base_class_model(158-206)src/datamodel_code_generator/model/pydantic_v2/base_model.py (1)
create_base_class_model(277-317)
src/datamodel_code_generator/model/msgspec.py (5)
src/datamodel_code_generator/model/base.py (4)
BaseClassDataType(463-464)create_base_class_model(714-727)path(735-737)name(654-656)src/datamodel_code_generator/model/pydantic_v2/base_model.py (1)
create_base_class_model(277-317)src/datamodel_code_generator/reference.py (3)
reference(76-78)Reference(141-200)get(831-833)src/datamodel_code_generator/model/typed_dict.py (1)
key(125-129)src/datamodel_code_generator/imports.py (2)
append(74-89)Import(20-38)
src/datamodel_code_generator/model/base.py (3)
src/datamodel_code_generator/model/msgspec.py (1)
create_base_class_model(158-206)src/datamodel_code_generator/model/pydantic_v2/base_model.py (1)
create_base_class_model(277-317)src/datamodel_code_generator/reference.py (2)
reference(76-78)Reference(141-200)
🪛 GitHub Check: CodeQL
src/datamodel_code_generator/model/msgspec.py
[notice] 23-23: Cyclic import
Import of module datamodel_code_generator.model.base begins an import cycle.
src/datamodel_code_generator/model/base.py
[notice] 52-52: Unused global variable
The global variable 'GENERIC_BASE_CLASS_PATH' is not used.
[notice] 53-53: Unused global variable
The global variable 'GENERIC_BASE_CLASS_NAME' is not used.
🪛 Ruff (0.14.8)
src/datamodel_code_generator/model/pydantic_v2/base_model.py
282-282: Unused noqa directive (non-enabled: FBT001, FBT002)
Remove unused noqa directive
(RUF100)
283-283: Unused noqa directive (non-enabled: FBT001, FBT002)
Remove unused noqa directive
(RUF100)
src/datamodel_code_generator/parser/base.py
650-650: Unused noqa directive (non-enabled: PLR0912, PLR0913, PLR0915)
Remove unused noqa directive
(RUF100)
src/datamodel_code_generator/model/msgspec.py
163-163: Unused noqa directive (non-enabled: FBT001, FBT002)
Remove unused noqa directive
(RUF100)
164-164: Unused noqa directive (non-enabled: FBT001, FBT002)
Remove unused noqa directive
(RUF100)
src/datamodel_code_generator/model/base.py
719-719: Unused noqa directive (non-enabled: FBT001, FBT002)
Remove unused noqa directive
(RUF100)
720-720: Unused noqa directive (non-enabled: FBT001, FBT002)
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). (2)
- GitHub Check: benchmarks
- GitHub Check: 3.10 on Windows
🔇 Additional comments (7)
src/datamodel_code_generator/model/msgspec.py (2)
157-206: LGTM – Msgspec base class factory is well-structured.The method correctly creates a generic Struct base class with aliased imports to avoid naming conflicts, and the config mapping appropriately translates Pydantic-style options to msgspec equivalents while ignoring unsupported options.
23-23: Remove the cyclic import concern—no import cycle exists.The import at line 23 is unidirectional:
msgspec.pyimports frombase.py, whilebase.pycontains no imports of msgspec or other model submodules. This dependency structure is clean and poses no initialization risk.Likely an incorrect or invalid review comment.
src/datamodel_code_generator/model/base.py (2)
52-53: False positive: Constants are used in parser/base.py.Static analysis flags these as unused because it doesn't track cross-module imports. Lines 57-58 in
src/datamodel_code_generator/parser/base.pyimport both constants.
479-480: LGTM – Clean extension mechanism for generic base class support.Converting
IS_ALIASto aClassVarand addingSUPPORTS_GENERIC_BASE_CLASSprovides a clear, type-safe way for subclasses to declare capabilities. The defaultcreate_base_class_modelreturningNoneallows model types to opt in gracefully.Also applies to: 713-727
src/datamodel_code_generator/model/pydantic_v2/base_model.py (1)
276-317: LGTM – Pydantic v2 base class generation is solid.The method correctly creates a ConfigDict-based BaseModel with aliased imports, and the import-filtering logic at lines 310-314 prevents conflicts between the aliased
_BaseModeland any existingBaseModelimports.src/datamodel_code_generator/parser/base.py (2)
822-853: LGTM – Config routing logic correctly isolates generic base class settings.The conditional checks cleanly divert shared configuration options into
generic_base_class_configwhen the feature is enabled, preventing them from being applied per-model viaALL_MODELscope.
2012-2057: Approve with caveat: Core generic base class application is sound, but cross-module inheritance needs refinement.The method correctly creates a shared base class model, inserts it at the start of each module's model list, and updates target models to inherit from it. The logic at lines 2052-2057 appropriately skips adding the base when a parent in the same module already has it. However, the cross-module inheritance concern noted above should be addressed to avoid redundant base classes in multi-module scenarios.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
src/datamodel_code_generator/parser/base.py (3)
650-650: Remove unused noqa directive codes.The
noqadirective includes codesPLR0912,PLR0913, andPLR0915that are not actually triggered by linters, as indicated by static analysis.🔎 Proposed fix
- def __init__( # noqa: PLR0912, PLR0913, PLR0915 + def __init__(
825-854: Consider extracting the repeated conditional routing pattern.The same pattern appears five times for different config keys. While functional, this could be refactored into a helper method for better maintainability.
🔎 Proposed refactor
+ def _route_config(self, key: str, value: Any) -> None: + """Route configuration to generic base class config or per-model template data.""" + if self.use_generic_base_class: + self.generic_base_class_config[key] = value + else: + self.extra_template_data[ALL_MODEL][key] = value + if allow_population_by_field_name: - if use_generic_base_class: - self.generic_base_class_config["allow_population_by_field_name"] = True - else: - self.extra_template_data[ALL_MODEL]["allow_population_by_field_name"] = True + self._route_config("allow_population_by_field_name", True) if allow_extra_fields: - if use_generic_base_class: - self.generic_base_class_config["allow_extra_fields"] = True - else: - self.extra_template_data[ALL_MODEL]["allow_extra_fields"] = True + self._route_config("allow_extra_fields", True) if extra_fields: - if use_generic_base_class: - self.generic_base_class_config["extra_fields"] = extra_fields - else: - self.extra_template_data[ALL_MODEL]["extra_fields"] = extra_fields + self._route_config("extra_fields", extra_fields) if enable_faux_immutability: - if use_generic_base_class: - self.generic_base_class_config["allow_mutation"] = False - else: - self.extra_template_data[ALL_MODEL]["allow_mutation"] = False + self._route_config("allow_mutation", False) if use_attribute_docstrings: - if use_generic_base_class: - self.generic_base_class_config["use_attribute_docstrings"] = True - else: - self.extra_template_data[ALL_MODEL]["use_attribute_docstrings"] = True + self._route_config("use_attribute_docstrings", True)
2012-2012: Remove unused noqa directive.Static analysis indicates
PLR0912is not triggered.🔎 Proposed fix
- def __apply_generic_base_class( # noqa: PLR0912 + def __apply_generic_base_class(tests/main/jsonschema/test_main_jsonschema.py (1)
3716-3935: Consider adding unit tests for base class generation logic.While the integration tests provide excellent end-to-end validation, consider supplementing them with unit tests that directly test the base class generation logic. This would provide:
- Faster test execution
- More granular coverage of edge cases
- Easier debugging when tests fail
For example, unit tests could verify:
- Correct
model_configgeneration for different options- Proper inheritance chain setup
- Base class naming and import handling
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
src/datamodel_code_generator/parser/base.pytests/data/expected/main/jsonschema/use_generic_base_class_deep_inheritance/__init__.pytests/data/expected/main/jsonschema/use_generic_base_class_deep_inheritance/animal.pytests/data/expected/main/jsonschema/use_generic_base_class_deep_inheritance/dog.pytests/data/expected/main/jsonschema/use_generic_base_class_deep_inheritance/mammal.pytests/data/expected/main/jsonschema/use_generic_base_class_deep_inheritance/model.pytests/data/expected/main/jsonschema/use_generic_base_class_module_split/dog.pytests/data/jsonschema/use_generic_base_class_deep_inheritance.jsontests/main/jsonschema/test_main_jsonschema.py
✅ Files skipped from review due to trivial changes (1)
- tests/data/expected/main/jsonschema/use_generic_base_class_deep_inheritance/init.py
🧰 Additional context used
🧬 Code graph analysis (5)
tests/data/expected/main/jsonschema/use_generic_base_class_deep_inheritance/animal.py (3)
src/datamodel_code_generator/reference.py (1)
_BaseModel(81-138)src/datamodel_code_generator/model/pydantic_v2/__init__.py (1)
ConfigDict(26-41)src/datamodel_code_generator/model/base.py (1)
name(654-656)
tests/data/expected/main/jsonschema/use_generic_base_class_deep_inheritance/dog.py (2)
tests/data/expected/main/jsonschema/use_generic_base_class_deep_inheritance/mammal.py (1)
Mammal(10-11)tests/data/expected/main/jsonschema/use_generic_base_class_module_split/dog.py (1)
Dog(10-11)
src/datamodel_code_generator/parser/base.py (3)
src/datamodel_code_generator/model/base.py (5)
DataModel(470-756)path(735-737)name(654-656)create_base_class_model(714-727)BaseClassDataType(463-464)src/datamodel_code_generator/model/msgspec.py (1)
create_base_class_model(158-206)src/datamodel_code_generator/model/pydantic_v2/base_model.py (1)
create_base_class_model(277-317)
tests/data/expected/main/jsonschema/use_generic_base_class_module_split/dog.py (2)
tests/data/expected/main/jsonschema/use_generic_base_class_module_split/animal.py (1)
Animal(17-18)tests/data/expected/main/jsonschema/use_generic_base_class_with_inheritance.py (2)
Animal(23-24)Dog(27-28)
tests/data/expected/main/jsonschema/use_generic_base_class_deep_inheritance/mammal.py (1)
tests/data/expected/main/jsonschema/use_generic_base_class_deep_inheritance/animal.py (1)
Animal(17-18)
🪛 Ruff (0.14.8)
src/datamodel_code_generator/parser/base.py
650-650: Unused noqa directive (non-enabled: PLR0912, PLR0913, PLR0915)
Remove unused noqa directive
(RUF100)
2012-2012: 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: 3.10 on Windows
- GitHub Check: 3.10 on Ubuntu
- GitHub Check: 3.11 on Windows
- GitHub Check: py312-isort7 on Ubuntu
- GitHub Check: 3.12 on Ubuntu
- GitHub Check: 3.12 on Windows
- GitHub Check: 3.13 on Windows
- GitHub Check: 3.14 on Ubuntu
- GitHub Check: benchmarks
- GitHub Check: Analyze (python)
🔇 Additional comments (14)
tests/data/expected/main/jsonschema/use_generic_base_class_module_split/dog.py (3)
5-7: LGTM: Import structure is correct.The
from __future__ import annotationsenables modern union type syntax, and the relative import from.animalcorrectly reflects the module split layout.
10-11: LGTM: Inheritance pattern correctly demonstrates the feature.The
Dogclass correctly inherits fromAnimal(which inherits fromBaseModelwithmodel_config), achieving the DRY objective of the--use-generic-base-classfeature by avoiding repeated configuration blocks.
1-3: No action needed. The generation metadata is correct.The test configuration confirms that
use_generic_base_class_with_inheritance.jsonis the correct input schema file for this test case. The output directory is nameduse_generic_base_class_module_splitto reflect the test mode (module split with--module-split-mode single), not the input filename. The generation header accurately identifies the source schema file.Likely an incorrect or invalid review comment.
tests/data/jsonschema/use_generic_base_class_deep_inheritance.json (1)
1-43: LGTM! Clean test schema for deep inheritance.The JSON schema correctly establishes a three-level inheritance chain (Animal → Mammal → Dog) using
allOfreferences, which effectively tests the generic base class feature with deep inheritance scenarios.src/datamodel_code_generator/parser/base.py (4)
57-58: LGTM! Necessary imports for the generic base class feature.
673-673: LGTM! Parameter and state initialization are correct.The new
use_generic_base_classparameter andgeneric_base_class_configdictionary are properly initialized and follow existing patterns in the codebase.Also applies to: 822-823
2075-2086: LGTM! Base class inheritance logic correctly handles schema-defined inheritance.The logic properly distinguishes between three cases:
- Models with a
target_model_parentinherit the base class through their parent chain (no action needed)- Models with schema-defined parents get the base class prepended (line 2083)
- Models without parents get the base class as their only base (line 2085)
This preserves schema-defined inheritance relationships while adding the generic base class, addressing the concern from previous reviews about replacing existing base classes.
2685-2685: LGTM! Correct placement of generic base class application.The method is invoked after all per-module processing but before rendering, ensuring the generic base class transformation is applied to the final model structures.
tests/data/expected/main/jsonschema/use_generic_base_class_deep_inheritance/mammal.py (1)
1-11: LGTM! Generated code correctly implements inheritance.The
Mammalclass properly inherits fromAnimal(imported from the sibling module) and defines its ownwarm_bloodedfield, demonstrating the deep inheritance chain.tests/data/expected/main/jsonschema/use_generic_base_class_deep_inheritance/model.py (1)
1-13: LGTM! Standard RootModel implementation.The generated
RootModelcorrectly uses Pydantic'sRootModel[Any]pattern for schema-level models.tests/data/expected/main/jsonschema/use_generic_base_class_deep_inheritance/dog.py (1)
1-11: LGTM! Deep inheritance chain correctly completed.The
Dogclass properly inherits fromMammal, which completes the three-level inheritance chain (Dog → Mammal → Animal → BaseModel), demonstrating that the generic base class feature works correctly with deep inheritance.tests/data/expected/main/jsonschema/use_generic_base_class_deep_inheritance/animal.py (1)
11-18: LGTM! Excellent demonstration of the DRY pattern.The generic
BaseModelis defined once in the root module withmodel_config = ConfigDict(extra='forbid'), andAnimalinherits from it. This allows the entire inheritance chain (Animal → Mammal → Dog) to share the configuration without repetition, which is the core benefit of the--use-generic-base-classfeature.tests/main/jsonschema/test_main_jsonschema.py (2)
30-30: LGTM! Import addition supports new test cases.The
EXPECTED_MAIN_PATHimport is correctly added and used in the module split test scenarios (lines 3877, 3896).
3716-3935: Comprehensive test coverage for the new feature.The 12 new test functions provide excellent coverage of the
--use-generic-base-classfeature across different scenarios:
- Various model types (Pydantic v2, dataclasses, msgspec)
- Different configuration options (populate_by_name, allow_extra, frozen, etc.)
- Edge cases (enum_only, with_inheritance, deep_inheritance, module_split)
The tests are well-structured, follow existing patterns, and include clear docstrings. The main test correctly uses the
@pytest.mark.cli_docdecorator for documentation generation.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
tests/data/expected/main/jsonschema/use_generic_base_class_multi_root/model.py (1)
11-14: Duplicate BaseModel definition (see animal.py comment).This is another instance of the duplicated
BaseModelclass. The same issue noted inanimal.pyapplies here: three modules define identicalBaseModelclasses, which contradicts the DRY principle that--use-generic-base-classaims to enforce.tests/data/expected/main/jsonschema/use_generic_base_class_multi_root/vehicle.py (1)
11-14: Duplicate BaseModel definition (see animal.py comment).This is the third instance of the duplicated
BaseModelclass across the multi-root test suite. As noted inanimal.py, having separateBaseModeldefinitions in each root module undermines the feature's goal of centralizing shared configuration.
🧹 Nitpick comments (1)
src/datamodel_code_generator/parser/base.py (1)
2074-2084: Refactor to avoid duplicate BaseModel definitions across modules.When multiple root modules exist (e.g., Animal/Dog in one module, Vehicle/Car in another), lines 2074-2084 create a separate
BaseModelclass for each root module beyond the first. This results in identical class definitions being repeated across modules, as evidenced by the test expectations whereanimal.py,model.py, andvehicle.pyeach define their ownBaseModel(_BaseModel)withextra='forbid'.This duplication contradicts the DRY principle and the stated goal of
--use-generic-base-classto eliminate repeatedmodel_configblocks. Consider one of these approaches:
- Shared module approach: Create a single
BaseModelin a shared module (e.g.,_base.py) and have all root modules import from it.- Single root constraint: Document that
--use-generic-base-classonly supports single-root schemas and validate/error when multiple roots are detected.- Primary root strategy: Insert
BaseModelonly in the first root module and ensure all other modules can import it via relative imports.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
src/datamodel_code_generator/parser/base.pytests/data/expected/main/jsonschema/use_generic_base_class_multi_root/__init__.pytests/data/expected/main/jsonschema/use_generic_base_class_multi_root/animal.pytests/data/expected/main/jsonschema/use_generic_base_class_multi_root/car.pytests/data/expected/main/jsonschema/use_generic_base_class_multi_root/dog.pytests/data/expected/main/jsonschema/use_generic_base_class_multi_root/model.pytests/data/expected/main/jsonschema/use_generic_base_class_multi_root/vehicle.pytests/data/jsonschema/use_generic_base_class_multi_root.jsontests/main/jsonschema/test_main_jsonschema.py
✅ Files skipped from review due to trivial changes (1)
- tests/data/expected/main/jsonschema/use_generic_base_class_multi_root/init.py
🧰 Additional context used
🧬 Code graph analysis (6)
tests/data/expected/main/jsonschema/use_generic_base_class_multi_root/dog.py (1)
tests/data/expected/main/jsonschema/use_generic_base_class_multi_root/animal.py (1)
Animal(17-18)
tests/data/expected/main/jsonschema/use_generic_base_class_multi_root/vehicle.py (4)
tests/data/expected/main/jsonschema/use_generic_base_class_multi_root/animal.py (1)
BaseModel(11-14)tests/data/expected/main/jsonschema/use_generic_base_class_multi_root/model.py (1)
BaseModel(11-14)src/datamodel_code_generator/reference.py (1)
_BaseModel(81-138)src/datamodel_code_generator/model/pydantic_v2/__init__.py (1)
ConfigDict(26-41)
tests/data/expected/main/jsonschema/use_generic_base_class_multi_root/model.py (4)
tests/data/expected/main/jsonschema/use_generic_base_class_multi_root/animal.py (1)
BaseModel(11-14)tests/data/expected/main/jsonschema/use_generic_base_class_multi_root/vehicle.py (1)
BaseModel(11-14)src/datamodel_code_generator/reference.py (1)
_BaseModel(81-138)src/datamodel_code_generator/model/pydantic_v2/__init__.py (1)
ConfigDict(26-41)
tests/data/expected/main/jsonschema/use_generic_base_class_multi_root/car.py (1)
tests/data/expected/main/jsonschema/use_generic_base_class_multi_root/vehicle.py (1)
Vehicle(17-18)
tests/data/expected/main/jsonschema/use_generic_base_class_multi_root/animal.py (5)
tests/data/expected/main/jsonschema/use_generic_base_class_multi_root/model.py (1)
BaseModel(11-14)tests/data/expected/main/jsonschema/use_generic_base_class_multi_root/vehicle.py (1)
BaseModel(11-14)src/datamodel_code_generator/reference.py (1)
_BaseModel(81-138)src/datamodel_code_generator/model/pydantic_v2/__init__.py (1)
ConfigDict(26-41)src/datamodel_code_generator/model/base.py (1)
name(654-656)
tests/main/jsonschema/test_main_jsonschema.py (2)
tests/main/conftest.py (2)
output_file(98-100)run_main_and_assert(244-408)tests/test_main_kr.py (1)
output_file(44-46)
🪛 Ruff (0.14.8)
src/datamodel_code_generator/parser/base.py
650-650: Unused noqa directive (non-enabled: PLR0912, PLR0913, PLR0915)
Remove unused noqa directive
(RUF100)
2012-2012: 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: benchmarks
- GitHub Check: Analyze (python)
- GitHub Check: 3.13 on macOS
- GitHub Check: 3.10 on macOS
- GitHub Check: 3.11 on Windows
- GitHub Check: 3.10 on Windows
- GitHub Check: 3.12 on macOS
- GitHub Check: 3.12 on Windows
- GitHub Check: 3.13 on Windows
- GitHub Check: 3.14 on Windows
🔇 Additional comments (7)
tests/data/jsonschema/use_generic_base_class_multi_root.json (1)
1-43: LGTM! Multi-root test schema is well-structured.This schema effectively defines two independent inheritance hierarchies (Animal→Dog and Vehicle→Car) to test the generic base class feature with multiple root models. The structure appropriately exercises the code paths in the parser that handle multi-root scenarios.
tests/data/expected/main/jsonschema/use_generic_base_class_multi_root/dog.py (1)
1-11: LGTM! Generated model correctly inherits from Animal.The
Dogclass properly extendsAnimalwith thebreedfield and uses relative imports from the sibling module. The inheritance structure matches the schema definition.tests/data/expected/main/jsonschema/use_generic_base_class_multi_root/car.py (1)
1-11: LGTM! Generated model correctly inherits from Vehicle.The
Carclass properly extendsVehiclewith thebrandfield, mirroring the Animal→Dog inheritance pattern in the parallel hierarchy.src/datamodel_code_generator/parser/base.py (2)
650-650: Remove unusednoqadirective.Static analysis reports that the
PLR0912,PLR0913, andPLR0915codes in thenoqacomment are not enabled and therefore unnecessary. Cleaning up unused suppressions improves code clarity.🔎 Proposed fix
- def __init__( # noqa: PLR0912, PLR0913, PLR0915 + def __init__(⛔ Skipped due to learnings
Learnt from: koxudaxi Repo: koxudaxi/datamodel-code-generator PR: 2681 File: tests/cli_doc/test_cli_doc_coverage.py:82-82 Timestamp: 2025-12-18T13:43:06.494Z Learning: In datamodel-code-generator project, Ruff preview mode is enabled via `lint.preview = true` in pyproject.toml. This enables preview rules like PLR6301 (no-self-use), so `noqa: PLR6301` directives are necessary and should not be removed even if RUF100 suggests they are unused.
2012-2012: Remove unusednoqadirective.Static analysis reports that the
PLR0912code in thenoqacomment is not enabled and therefore unnecessary.🔎 Proposed fix
- def __apply_generic_base_class( # noqa: PLR0912 + def __apply_generic_base_class(⛔ Skipped due to learnings
Learnt from: koxudaxi Repo: koxudaxi/datamodel-code-generator PR: 2681 File: tests/cli_doc/test_cli_doc_coverage.py:82-82 Timestamp: 2025-12-18T13:43:06.494Z Learning: In datamodel-code-generator project, Ruff preview mode is enabled via `lint.preview = true` in pyproject.toml. This enables preview rules like PLR6301 (no-self-use), so `noqa: PLR6301` directives are necessary and should not be removed even if RUF100 suggests they are unused.tests/main/jsonschema/test_main_jsonschema.py (2)
30-30: LGTM! Import is properly used.The
EXPECTED_MAIN_PATHimport is correctly added and used by the new--use-generic-base-classtests that generate module-split outputs.
3716-3954: Excellent comprehensive test coverage for the new feature!The test suite for
--use-generic-base-classis well-structured and thorough:✅ Comprehensive scenarios covered:
- Basic usage with different config options (forbid, allow, frozen, populate_by_name)
- Edge cases (enum-only, dataclass no-op)
- Complex inheritance (simple, deep, multi-root)
- Module split mode (cross-module base class)
- Multiple model types (Pydantic v2, msgspec)
✅ Good test organization:
- Clear, descriptive docstrings
- Consistent test patterns
- CLI documentation annotation on the primary test
- Proper use of expected output files
The test coverage thoroughly validates the feature works correctly across different configurations and edge cases.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/datamodel_code_generator/parser/base.py (1)
2012-2012: Consider removing unusednoqadirective.Static analysis indicates the
noqadirective forPLR0912(too many branches) may be unused on this line.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
src/datamodel_code_generator/parser/base.pytests/data/expected/main/jsonschema/use_generic_base_class_circular/__init__.pytests/data/expected/main/jsonschema/use_generic_base_class_circular/_internal.pytests/data/expected/main/jsonschema/use_generic_base_class_circular/friend.pytests/data/expected/main/jsonschema/use_generic_base_class_circular/model.pytests/data/expected/main/jsonschema/use_generic_base_class_circular/person.pytests/data/jsonschema/use_generic_base_class_circular.jsontests/main/jsonschema/test_main_jsonschema.py
✅ Files skipped from review due to trivial changes (1)
- tests/data/expected/main/jsonschema/use_generic_base_class_circular/init.py
🧰 Additional context used
🧬 Code graph analysis (3)
tests/data/expected/main/jsonschema/use_generic_base_class_circular/person.py (1)
tests/data/expected/main/jsonschema/use_generic_base_class_circular/_internal.py (1)
Person(22-24)
src/datamodel_code_generator/parser/base.py (3)
src/datamodel_code_generator/model/base.py (5)
DataModel(470-756)path(735-737)name(654-656)create_base_class_model(714-727)BaseClassDataType(463-464)src/datamodel_code_generator/model/pydantic_v2/base_model.py (1)
create_base_class_model(277-317)src/datamodel_code_generator/model/msgspec.py (1)
create_base_class_model(158-206)
tests/data/expected/main/jsonschema/use_generic_base_class_circular/friend.py (1)
tests/data/expected/main/jsonschema/use_generic_base_class_circular/_internal.py (1)
Friend(17-19)
🪛 Ruff (0.14.8)
src/datamodel_code_generator/parser/base.py
650-650: Unused noqa directive (non-enabled: PLR0912, PLR0913, PLR0915)
Remove unused noqa directive
(RUF100)
2012-2012: 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). (8)
- GitHub Check: 3.10 on Windows
- GitHub Check: 3.11 on Windows
- GitHub Check: 3.13 on macOS
- GitHub Check: 3.12 on Windows
- GitHub Check: 3.13 on Windows
- GitHub Check: 3.14 on Windows
- GitHub Check: Analyze (python)
- GitHub Check: benchmarks
🔇 Additional comments (20)
src/datamodel_code_generator/parser/base.py (7)
55-65: LGTM!The new imports
GENERIC_BASE_CLASS_NAMEandGENERIC_BASE_CLASS_PATHare correctly added and used in the__apply_generic_base_classmethod.
822-853: LGTM!The config routing logic correctly diverts model configuration options to
generic_base_class_configwhenuse_generic_base_classis enabled. The pattern is consistent across all five options, and the fallback to per-model template data (extra_template_data[ALL_MODEL]) preserves backward compatibility.
2074-2084: Multi-root module handling addresses past review concern.This code correctly creates separate base class models for each independent root module, resolving the cross-module inheritance issue flagged in previous reviews. Each module now gets its own
BaseModeldefinition with properfile_pathassignment.
2696-2697: LGTM!The invocation of
__apply_generic_base_classis correctly placed after all other model processing steps (sorting, discriminator handling, etc.) and before import collection. This ensures the generic base class is applied to the final model state.
650-650: Keep thenoqadirective — it prevents rule violations that would otherwise occur.The rules PLR0912 (too-many-statements), PLR0913 (too-many-arguments), and PLR0915 (too-many-branches) are actively enforced for this source file (no per-file ignores configured), so the
noqadirective is necessary and should not be removed.
2086-2096: Sharing a singleBaseClassDataTypeinstance is intentional and safe.The same
base_class_dtinstance is reused across target models for memory efficiency. While the sharedreferenceobject does have itschildrenlist mutated when each model adds itself during initialization, this is the intended design for tracking parent-child relationships between models. Thetypeandreferencefields of theBaseClassDataTypeinstance itself are never reassigned, so creating separate instances per model would be unnecessary overhead without providing additional safety.
2088-2096: Untested code path for models with non-target base classes.Line 2093-2094 (
elif parent_refs:) handles models that have existing base classes but none are target models. The# pragma: no coverindicates this branch lacks test coverage.Consider adding a test case with a schema where a model inherits from a non-target type combined with
--use-generic-base-classto ensure this path works correctly.tests/main/jsonschema/test_main_jsonschema.py (6)
30-30: LGTM!The import of
EXPECTED_MAIN_PATHis correctly added to support the new module-split test expectations.
3716-3737: LGTM!Well-structured test with appropriate CLI documentation marker. The test covers the basic
--use-generic-base-classfunctionality with Pydantic v2.
3740-3805: LGTM!Good test coverage for the various model configuration options (
--allow-population-by-field-name,--allow-extra-fields,--enable-faux-immutability,--use-attribute-docstrings) combined with--use-generic-base-class. These tests ensure the feature works correctly with different configuration combinations.
3808-3841: LGTM!These tests appropriately verify edge cases: dataclass output (where the feature should have no effect) and enum-only schemas (where no ProjectBaseModel should be generated).
3844-3935: LGTM!Comprehensive test coverage for inheritance scenarios: simple inheritance, module-split mode, deep inheritance chains, multiple independent inheritance trees, and circular references. The tests use
expected_directoryappropriately for module-split outputs.
3938-3972: LGTM!Good coverage for msgspec.Struct support with the
--use-generic-base-classfeature, testing both the basic case and the--extra-fields forbidvariant.tests/data/jsonschema/use_generic_base_class_circular.json (1)
1-21: LGTM!Well-designed test schema that properly establishes circular references between
PersonandFrienddefinitions. This is an effective test case for verifying the--use-generic-base-classfeature handles circular dependencies correctly.tests/data/expected/main/jsonschema/use_generic_base_class_circular/friend.py (1)
1-7: LGTM!Correct expected output structure for the Friend model. The pattern of importing from
._internaland re-exporting via__all__is the standard approach for handling circular references in module-split mode.tests/data/expected/main/jsonschema/use_generic_base_class_circular/person.py (1)
1-7: LGTM!Consistent with the
friend.pystructure, correctly re-exportingPersonfrom the internal module.tests/data/expected/main/jsonschema/use_generic_base_class_circular/model.py (1)
1-18: LGTM!This expected output correctly demonstrates the
--use-generic-base-classfeature. The sharedBaseModelclass definesmodel_config = ConfigDict(extra='forbid')once, andModelinherits from it without repeating the configuration. This achieves the DRY principle as described in the PR objectives.tests/data/expected/main/jsonschema/use_generic_base_class_circular/_internal.py (3)
1-8: LGTM!The imports are well-structured. Using
BaseModel as _BaseModelcorrectly avoids name collision with the customBaseModelclass defined below, andfrom __future__ import annotationsproperly enables forward references for the circular dependency betweenFriendandPerson.
11-14: LGTM!This is the core of the
--use-generic-base-classfeature. The customBaseModelwith sharedmodel_configeliminates the need to repeat configuration in every generated model, directly addressing the DRY objective from issue #2501.
17-24: LGTM!The circular reference between
FriendandPersonis correctly handled viafrom __future__ import annotations. Both classes properly inherit from the customBaseModel, ensuring they share theextra='forbid'configuration without repetition.
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
src/datamodel_code_generator/parser/base.py (2)
2012-2100: Consider refactoring to reduce complexity.This method has 22 local variables (exceeding the limit of 15) and handles multiple responsibilities:
- Collecting target models across modules
- Finding root modules
- Creating the base class model
- Applying the base class to target models
- Managing imports
While the logic appears correct, consider extracting helper methods to improve maintainability:
_collect_target_models_by_module()_find_root_modules()_apply_base_class_to_models()_update_imports_for_base_class()
2098-2098: Simplify complex import path construction.The nested ternary expression is difficult to read. Consider refactoring for clarity:
🔎 Proposed refactor
- from_ = f"{from_}{first_root_module[-1].replace('.py', '')}" if from_.endswith(".") else f"{from_}.{first_root_module[-1].replace('.py', '')}" + module_name = first_root_module[-1].replace('.py', '') + from_ = f"{from_}{module_name}" if from_.endswith(".") else f"{from_}.{module_name}"
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/datamodel_code_generator/parser/base.pytests/data/expected/main/jsonschema/use_generic_base_class_circular/model.pytests/data/expected/main/jsonschema/use_generic_base_class_multi_root/animal.pytests/data/expected/main/jsonschema/use_generic_base_class_multi_root/model.py
🚧 Files skipped from review as they are similar to previous changes (1)
- tests/data/expected/main/jsonschema/use_generic_base_class_multi_root/model.py
🧰 Additional context used
🧬 Code graph analysis (2)
tests/data/expected/main/jsonschema/use_generic_base_class_circular/model.py (2)
tests/data/expected/main/jsonschema/use_generic_base_class_circular/_internal.py (1)
BaseModel(11-14)tests/data/expected/main/jsonschema/use_generic_base_class_multi_root/model.py (1)
Model(10-11)
tests/data/expected/main/jsonschema/use_generic_base_class_multi_root/animal.py (2)
tests/data/expected/main/jsonschema/use_generic_base_class_multi_root/vehicle.py (1)
BaseModel(11-14)src/datamodel_code_generator/model/base.py (1)
name(654-656)
🪛 GitHub Actions: Lint
src/datamodel_code_generator/parser/base.py
[error] 2012-2012: ruff: PLR0914 Too many local variables (22/15).
[error] 2084-2084: SLF001 Private member accessed: _additional_imports.
[error] 2084-2084: SLF001 Private member accessed: _additional_imports.
[error] 1-1: ruff-format modified 1 file. Run 'prek run --all-files' to apply formatting locally.
🪛 Ruff (0.14.8)
src/datamodel_code_generator/parser/base.py
650-650: Unused noqa directive (non-enabled: PLR0912, PLR0913, PLR0915)
Remove unused noqa directive
(RUF100)
2012-2012: Unused noqa directive (non-enabled: PLR0912, PLR0915)
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). (11)
- GitHub Check: 3.13 on Windows
- GitHub Check: 3.10 on Windows
- GitHub Check: py312-isort6 on Ubuntu
- GitHub Check: 3.11 on Windows
- GitHub Check: 3.14 on macOS
- GitHub Check: 3.11 on macOS
- GitHub Check: 3.12 on Windows
- GitHub Check: 3.12 on macOS
- GitHub Check: 3.14 on Windows
- GitHub Check: benchmarks
- GitHub Check: Analyze (python)
🔇 Additional comments (7)
tests/data/expected/main/jsonschema/use_generic_base_class_circular/model.py (1)
1-11: LGTM! Test fixture correctly demonstrates the generic base class feature.This test fixture appropriately shows the
--use-generic-base-classfeature in action: theModelclass inherits from the sharedBaseModel(defined in_internal.pywith themodel_config) rather than repeating the configuration. The emptyModelclass is a valid minimal test case for verifying base class inheritance works correctly in circular reference scenarios.tests/data/expected/main/jsonschema/use_generic_base_class_multi_root/animal.py (2)
7-7: Past concern resolved: shared BaseModel correctly imported.The import of
BaseModelfrom.vehicleaddresses the duplicate definition issue flagged in the previous review. By reusing a single shared base class, the generated code now follows the DRY principle and eliminates type incompatibility across modules.
10-11: LGTM!The
Animalmodel correctly inherits from the sharedBaseModeland demonstrates the--use-generic-base-classfeature working as intended. The model_config (extra='forbid') is inherited from the base class without duplication.src/datamodel_code_generator/parser/base.py (4)
57-58: LGTM! Clean imports for the generic base class feature.The new constants are properly imported and used throughout the implementation.
673-673: Well-structured feature flag with clean conditional config initialization.The implementation properly separates generic base class configuration from per-model configuration based on the
use_generic_base_classflag, maintaining backward compatibility.Also applies to: 822-853
2699-2699: LGTM! Correctly positioned in the processing flow.The
__apply_generic_base_classcall is properly placed after per-module model processing and before final import cleanup and rendering.
1-1: Verify code formatting with ruff-format.The project uses ruff-format configured to run against the entire repository with
prek run --all-files. Before merging, ensure the code passes formatting checks by running this command locally.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (3)
src/datamodel_code_generator/parser/base.py (3)
650-650: Remove unused noqa directive.The
noqadirective forPLR0912,PLR0913,PLR0915is flagged as unused by Ruff, indicating these rules are not currently enabled. Clean up by removing the unnecessary directive.🔎 Proposed fix
- def __init__( # noqa: PLR0012, PLR0913, PLR0915 + def __init__(
2012-2012: Update or remove noqa directive.Static analysis indicates the
noqadirective forPLR0912,PLR0914,PLR0915is unused because these rules are not enabled. However, a past review comment indicatedPLR0914(too many local variables) was an actual issue. Either remove the directive if the rules are truly disabled, or verify the correct rule codes and update accordingly.
2084-2086: Avoid accessing private member_additional_imports.Lines 2084-2085 directly access
model._additional_imports, violating encapsulation. A past review suggested adding a public method to the DataModel class (e.g.,remove_additional_import) to handle this operation.🔎 Previously suggested approach
Add a public method to the DataModel class:
# In DataModel class def remove_additional_import(self, import_obj: Import) -> None: """Remove an import from additional imports.""" self._additional_imports = [i for i in self._additional_imports if i != import_obj]Then use it here:
if original_import: model.remove_additional_import(original_import)
🧹 Nitpick comments (2)
src/datamodel_code_generator/parser/base.py (2)
822-854: Consider extracting repetitive conditional config logic into a helper method.The pattern of checking
use_generic_base_classand conditionally populating eithergeneric_base_class_configorextra_template_data[ALL_MODEL]is repeated five times. While clear, extracting this into a helper method would reduce repetition and make future additions easier.🔎 Example refactor
def _set_model_config(self, key: str, value: Any) -> None: """Set config in generic base class or per-model template data.""" if self.use_generic_base_class: self.generic_base_class_config[key] = value else: self.extra_template_data[ALL_MODEL][key] = value # Then use it: if allow_population_by_field_name: self._set_model_config("allow_population_by_field_name", True) if allow_extra_fields: self._set_model_config("allow_extra_fields", True) # ... etc
2012-2103: Feature implementation is functionally sound but has minor cleanup opportunities.The
__apply_generic_base_classmethod successfully implements the DRY base class pattern described in PR #2501. The logic correctly:
- Identifies target models and root modules
- Creates a shared base class with common configuration
- Preserves schema-defined inheritance while adding the generic base class
- Handles cross-module scenarios
However, consider these improvements:
- Extract helper methods to reduce complexity (22 local variables)
- Address the private member access violation
- Clean up unused noqa directives
The core functionality appears correct based on the comprehensive past reviews that addressed inheritance edge cases, cross-module scenarios, and multiple inheritance trees.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/datamodel_code_generator/parser/base.py
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-12-18T13:43:06.494Z
Learnt from: koxudaxi
Repo: koxudaxi/datamodel-code-generator PR: 2681
File: tests/cli_doc/test_cli_doc_coverage.py:82-82
Timestamp: 2025-12-18T13:43:06.494Z
Learning: In datamodel-code-generator project, Ruff preview mode is enabled via `lint.preview = true` in pyproject.toml. This enables preview rules like PLR6301 (no-self-use), so `noqa: PLR6301` directives are necessary and should not be removed even if RUF100 suggests they are unused.
Applied to files:
src/datamodel_code_generator/parser/base.py
🧬 Code graph analysis (1)
src/datamodel_code_generator/parser/base.py (6)
src/datamodel_code_generator/model/base.py (7)
DataModel(470-756)imports(283-302)imports(638-643)path(735-737)name(654-656)create_base_class_model(714-727)BaseClassDataType(463-464)src/datamodel_code_generator/imports.py (3)
append(74-89)Import(20-38)from_full_path(35-38)src/datamodel_code_generator/model/enum.py (1)
imports(119-121)src/datamodel_code_generator/types.py (1)
imports(450-495)src/datamodel_code_generator/model/typed_dict.py (1)
imports(150-155)src/datamodel_code_generator/model/pydantic_v2/base_model.py (1)
create_base_class_model(277-317)
🪛 Ruff (0.14.8)
src/datamodel_code_generator/parser/base.py
650-650: Unused noqa directive (non-enabled: PLR0912, PLR0913, PLR0915)
Remove unused noqa directive
(RUF100)
2012-2012: Unused noqa directive (non-enabled: PLR0912, PLR0914, PLR0915)
Remove unused noqa directive
(RUF100)
2084-2084: Unused noqa directive (non-enabled: SLF001)
Remove unused noqa directive
(RUF100)
2085-2085: Unused noqa directive (non-enabled: SLF001)
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). (8)
- GitHub Check: benchmarks
- GitHub Check: py312-black23 on Ubuntu
- GitHub Check: 3.14 on Windows
- GitHub Check: 3.11 on Windows
- GitHub Check: 3.10 on Windows
- GitHub Check: 3.12 on macOS
- GitHub Check: 3.13 on Windows
- GitHub Check: 3.12 on Windows
🔇 Additional comments (3)
src/datamodel_code_generator/parser/base.py (3)
57-58: LGTM! Imports are properly added for the generic base class feature.The imports of
GENERIC_BASE_CLASS_NAMEandGENERIC_BASE_CLASS_PATHare necessary for the new functionality and are correctly sourced from the model.base module.
2086-2093: Verify base class inheritance logic handles all edge cases correctly.The logic for assigning base classes:
- If the model has a target_model_parent → skip (inherits transitively)
- Else if the model has other parent_refs →
insert(0, base_class_dt)to prepend- Else →
base_classes = [base_class_dt]to replaceThis appears correct for preserving schema-defined inheritance while adding the generic base class. However, ensure this is tested with:
- Models with multiple base classes (some target, some not)
- Diamond inheritance patterns
- Cross-module inheritance scenarios
Past review comments indicate these were addressed, but verification with comprehensive tests is important given the complexity.
2702-2702: LGTM! Method placement is appropriate.Calling
__apply_generic_base_classat this point in the pipeline ensures all individual model processing is complete before applying the shared base class transformation. This is the correct integration point.
koxudaxi
force-pushed
the
fix/use-generic-base-class
branch
from
88a8c48 to
0bf316c
Compare
This option creates a shared base class (BaseModel/Struct) with common configuration instead of repeating model_config in every generated model. Features: - Supports pydantic v2 BaseModel and msgspec Struct - Works with module split mode (single BaseModel shared across modules) - Preserves schema inheritance relationships - Handles circular imports via _internal.py pattern
koxudaxi
force-pushed
the
fix/use-generic-base-class
branch
from
0bf316c to
40d67fe
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (3)
src/datamodel_code_generator/parser/base.py (3)
650-650: Remove unusednoqadirective.The
noqacomment references rules (PLR0912,PLR0913,PLR0915) that are not triggering warnings per static analysis. Remove the directive to reduce noise.🔎 Proposed fix
- def __init__( # noqa: PLR0912, PLR0913, PLR0915 + def __init__(
2012-2012: Updatenoqadirective to suppress the correct rule.Static analysis indicates the current
noqacomment references rules (PLR0912,PLR0914,PLR0915) that are not triggering. IfPLR0914(too many local variables) is the actual concern based on previous review, update accordingly; otherwise, remove the directive.🔎 Proposed fix
- def __apply_generic_base_class( # noqa: PLR0912, PLR0914, PLR0915 + def __apply_generic_base_class(
2084-2085: Private member access violates encapsulation.Lines 2084-2085 directly access and mutate
model._additional_imports. The static analysis hints indicate thenoqa: SLF001directives are unused, suggesting the linter rule isn't active, but the encapsulation concern remains valid. Consider adding a public method toDataModelfor removing additional imports.🔎 Proposed approach
Add a public method to the
DataModelclass:def remove_additional_import(self, import_to_remove: Import) -> None: """Remove an import from additional imports.""" self._additional_imports = [i for i in self._additional_imports if i != import_to_remove]Then replace lines 2084-2085 with:
if original_import: model.remove_additional_import(original_import)
🧹 Nitpick comments (5)
src/datamodel_code_generator/model/base.py (1)
713-727: Template method pattern is appropriate; minor cleanup available.The base implementation correctly returns
None, allowing concrete model types (pydantic_v2.BaseModel,msgspec.Struct) to override with their specific implementations. The docstring clearly explains the contract.Per static analysis, the
FBT001andFBT002noqa directives on lines 719-720 are unused since these rules are not enabled. Consider removing them for cleaner code:🔎 Proposed cleanup
@classmethod def create_base_class_model( cls, config: dict[str, Any], # noqa: ARG003 reference: Reference, # noqa: ARG003 - custom_template_dir: Path | None = None, # noqa: ARG003 - keyword_only: bool = False, # noqa: ARG003, FBT001, FBT002 - treat_dot_as_module: bool = False, # noqa: ARG003, FBT001, FBT002 + custom_template_dir: Path | None = None, # noqa: ARG003 + keyword_only: bool = False, # noqa: ARG003 + treat_dot_as_module: bool = False, # noqa: ARG003 ) -> DataModel | None:src/datamodel_code_generator/model/pydantic_v2/base_model.py (1)
284-286: Remove unusednoqadirectives.Static analysis indicates these
noqadirectives are not needed as FBT001/FBT002 rules are not enabled.Proposed fix
- keyword_only: bool = False, # noqa: FBT001, FBT002 - treat_dot_as_module: bool = False, # noqa: FBT001, FBT002 + keyword_only: bool = False, + treat_dot_as_module: bool = False,src/datamodel_code_generator/model/msgspec.py (2)
175-177: Remove unusednoqadirectives.Same as the Pydantic v2 implementation, static analysis indicates these directives are unnecessary.
Proposed fix
- keyword_only: bool = False, # noqa: FBT001, FBT002 - treat_dot_as_module: bool = False, # noqa: FBT001, FBT002 + keyword_only: bool = False, + treat_dot_as_module: bool = False,
116-124: CONFIG_MAPPING silently ignores unsupported options.Several configuration options (
extra_fields=allow,extra_fields=ignore,allow_extra_fields,allow_population_by_field_name,use_attribute_docstrings) map toNoneand are silently skipped. This is valid since msgspec doesn't support these features, but users may not realize their configuration has no effect.Consider documenting this limitation in CLI help or emitting a warning when these options are used with
msgspec.Struct.src/datamodel_code_generator/parser/base.py (1)
2088-2089: Clarify the no-op branch.The
passstatement at line 2089 is a no-op for models that already have a target model parent. Consider adding a brief comment to clarify the intent, or restructure usingelifto make the logic flow clearer.🔎 Proposed fix
if has_target_model_parent: - pass + # Model already inherits from a target model that will have the base class; + # no need to add base_class_dt directly to this model. + pass elif parent_refs: # pragma: no cover
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (54)
docs/cli-reference/index.mddocs/cli-reference/model-customization.mddocs/cli-reference/quick-reference.mdsrc/datamodel_code_generator/__init__.pysrc/datamodel_code_generator/__main__.pysrc/datamodel_code_generator/arguments.pysrc/datamodel_code_generator/cli_options.pysrc/datamodel_code_generator/model/base.pysrc/datamodel_code_generator/model/enum.pysrc/datamodel_code_generator/model/msgspec.pysrc/datamodel_code_generator/model/pydantic_v2/base_model.pysrc/datamodel_code_generator/model/pydantic_v2/imports.pysrc/datamodel_code_generator/model/type_alias.pysrc/datamodel_code_generator/parser/base.pysrc/datamodel_code_generator/parser/graphql.pysrc/datamodel_code_generator/parser/jsonschema.pysrc/datamodel_code_generator/parser/openapi.pytests/data/expected/main/jsonschema/use_generic_base_class.pytests/data/expected/main/jsonschema/use_generic_base_class_allow_extra.pytests/data/expected/main/jsonschema/use_generic_base_class_attr_docstrings.pytests/data/expected/main/jsonschema/use_generic_base_class_circular/__init__.pytests/data/expected/main/jsonschema/use_generic_base_class_circular/_internal.pytests/data/expected/main/jsonschema/use_generic_base_class_circular/friend.pytests/data/expected/main/jsonschema/use_generic_base_class_circular/model.pytests/data/expected/main/jsonschema/use_generic_base_class_circular/person.pytests/data/expected/main/jsonschema/use_generic_base_class_dataclass.pytests/data/expected/main/jsonschema/use_generic_base_class_deep_inheritance/__init__.pytests/data/expected/main/jsonschema/use_generic_base_class_deep_inheritance/animal.pytests/data/expected/main/jsonschema/use_generic_base_class_deep_inheritance/dog.pytests/data/expected/main/jsonschema/use_generic_base_class_deep_inheritance/mammal.pytests/data/expected/main/jsonschema/use_generic_base_class_deep_inheritance/model.pytests/data/expected/main/jsonschema/use_generic_base_class_enum_only.pytests/data/expected/main/jsonschema/use_generic_base_class_frozen.pytests/data/expected/main/jsonschema/use_generic_base_class_module_split/__init__.pytests/data/expected/main/jsonschema/use_generic_base_class_module_split/animal.pytests/data/expected/main/jsonschema/use_generic_base_class_module_split/dog.pytests/data/expected/main/jsonschema/use_generic_base_class_module_split/model.pytests/data/expected/main/jsonschema/use_generic_base_class_msgspec.pytests/data/expected/main/jsonschema/use_generic_base_class_msgspec_forbid.pytests/data/expected/main/jsonschema/use_generic_base_class_multi_root/__init__.pytests/data/expected/main/jsonschema/use_generic_base_class_multi_root/animal.pytests/data/expected/main/jsonschema/use_generic_base_class_multi_root/car.pytests/data/expected/main/jsonschema/use_generic_base_class_multi_root/dog.pytests/data/expected/main/jsonschema/use_generic_base_class_multi_root/model.pytests/data/expected/main/jsonschema/use_generic_base_class_multi_root/vehicle.pytests/data/expected/main/jsonschema/use_generic_base_class_populate_by_name.pytests/data/expected/main/jsonschema/use_generic_base_class_with_inheritance.pytests/data/jsonschema/use_generic_base_class_circular.jsontests/data/jsonschema/use_generic_base_class_deep_inheritance.jsontests/data/jsonschema/use_generic_base_class_enum_only.jsontests/data/jsonschema/use_generic_base_class_multi_root.jsontests/data/jsonschema/use_generic_base_class_simple.jsontests/data/jsonschema/use_generic_base_class_with_inheritance.jsontests/main/jsonschema/test_main_jsonschema.py
✅ Files skipped from review due to trivial changes (3)
- tests/data/expected/main/jsonschema/use_generic_base_class_circular/init.py
- tests/data/expected/main/jsonschema/use_generic_base_class_module_split/init.py
- tests/data/expected/main/jsonschema/use_generic_base_class_deep_inheritance/init.py
🚧 Files skipped from review as they are similar to previous changes (27)
- tests/data/expected/main/jsonschema/use_generic_base_class_multi_root/init.py
- tests/data/jsonschema/use_generic_base_class_enum_only.json
- docs/cli-reference/quick-reference.md
- src/datamodel_code_generator/main.py
- tests/data/jsonschema/use_generic_base_class_circular.json
- tests/data/jsonschema/use_generic_base_class_simple.json
- tests/data/expected/main/jsonschema/use_generic_base_class_module_split/model.py
- tests/data/jsonschema/use_generic_base_class_multi_root.json
- tests/data/expected/main/jsonschema/use_generic_base_class_multi_root/model.py
- src/datamodel_code_generator/cli_options.py
- tests/data/expected/main/jsonschema/use_generic_base_class_msgspec.py
- tests/data/expected/main/jsonschema/use_generic_base_class_enum_only.py
- src/datamodel_code_generator/parser/graphql.py
- tests/data/expected/main/jsonschema/use_generic_base_class_deep_inheritance/model.py
- tests/data/expected/main/jsonschema/use_generic_base_class_circular/_internal.py
- tests/data/expected/main/jsonschema/use_generic_base_class_multi_root/animal.py
- src/datamodel_code_generator/init.py
- docs/cli-reference/model-customization.md
- src/datamodel_code_generator/parser/jsonschema.py
- tests/data/expected/main/jsonschema/use_generic_base_class_circular/person.py
- tests/data/jsonschema/use_generic_base_class_deep_inheritance.json
- tests/data/expected/main/jsonschema/use_generic_base_class_dataclass.py
- tests/data/expected/main/jsonschema/use_generic_base_class_circular/model.py
- tests/data/expected/main/jsonschema/use_generic_base_class_multi_root/car.py
- tests/data/expected/main/jsonschema/use_generic_base_class_module_split/dog.py
- tests/data/expected/main/jsonschema/use_generic_base_class_attr_docstrings.py
- tests/data/expected/main/jsonschema/use_generic_base_class.py
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-12-18T13:43:06.494Z
Learnt from: koxudaxi
Repo: koxudaxi/datamodel-code-generator PR: 2681
File: tests/cli_doc/test_cli_doc_coverage.py:82-82
Timestamp: 2025-12-18T13:43:06.494Z
Learning: In datamodel-code-generator project, Ruff preview mode is enabled via `lint.preview = true` in pyproject.toml. This enables preview rules like PLR6301 (no-self-use), so `noqa: PLR6301` directives are necessary and should not be removed even if RUF100 suggests they are unused.
Applied to files:
src/datamodel_code_generator/parser/base.py
🧬 Code graph analysis (12)
tests/data/expected/main/jsonschema/use_generic_base_class_module_split/animal.py (5)
src/datamodel_code_generator/model/pydantic_v2/base_model.py (1)
BaseModel(170-318)tests/data/expected/main/jsonschema/use_generic_base_class.py (1)
BaseModel(11-14)tests/data/expected/main/jsonschema/use_generic_base_class_with_inheritance.py (2)
BaseModel(13-16)Animal(23-24)src/datamodel_code_generator/reference.py (1)
_BaseModel(81-138)src/datamodel_code_generator/model/pydantic_v2/__init__.py (1)
ConfigDict(26-41)
tests/data/expected/main/jsonschema/use_generic_base_class_populate_by_name.py (2)
src/datamodel_code_generator/model/pydantic_v2/base_model.py (1)
BaseModel(170-318)src/datamodel_code_generator/model/pydantic_v2/__init__.py (1)
ConfigDict(26-41)
tests/data/expected/main/jsonschema/use_generic_base_class_circular/friend.py (1)
tests/data/expected/main/jsonschema/use_generic_base_class_circular/_internal.py (1)
Friend(17-19)
src/datamodel_code_generator/model/base.py (3)
src/datamodel_code_generator/model/msgspec.py (1)
create_base_class_model(170-207)src/datamodel_code_generator/model/pydantic_v2/base_model.py (1)
create_base_class_model(279-318)src/datamodel_code_generator/reference.py (2)
reference(76-78)Reference(141-200)
tests/data/expected/main/jsonschema/use_generic_base_class_multi_root/dog.py (2)
tests/data/expected/main/jsonschema/use_generic_base_class_multi_root/animal.py (1)
Animal(10-11)tests/data/expected/main/jsonschema/use_generic_base_class_module_split/dog.py (1)
Dog(10-11)
src/datamodel_code_generator/model/pydantic_v2/imports.py (1)
src/datamodel_code_generator/imports.py (2)
Import(20-38)from_full_path(35-38)
tests/data/expected/main/jsonschema/use_generic_base_class_deep_inheritance/mammal.py (4)
tests/data/expected/main/jsonschema/use_generic_base_class_deep_inheritance/animal.py (1)
Animal(17-18)tests/data/expected/main/jsonschema/use_generic_base_class_module_split/animal.py (1)
Animal(17-18)tests/data/expected/main/jsonschema/use_generic_base_class_multi_root/animal.py (1)
Animal(10-11)tests/data/expected/main/jsonschema/use_generic_base_class_with_inheritance.py (1)
Animal(23-24)
tests/data/expected/main/jsonschema/use_generic_base_class_frozen.py (4)
src/datamodel_code_generator/model/pydantic_v2/base_model.py (1)
BaseModel(170-318)tests/data/expected/main/jsonschema/use_generic_base_class.py (1)
BaseModel(11-14)src/datamodel_code_generator/reference.py (1)
_BaseModel(81-138)src/datamodel_code_generator/model/pydantic_v2/__init__.py (1)
ConfigDict(26-41)
src/datamodel_code_generator/model/msgspec.py (3)
src/datamodel_code_generator/model/base.py (6)
BaseClassDataType(463-464)imports(283-302)imports(638-643)create_base_class_model(714-727)path(735-737)name(654-656)src/datamodel_code_generator/imports.py (2)
Import(20-38)append(74-89)src/datamodel_code_generator/model/pydantic_v2/base_model.py (1)
create_base_class_model(279-318)
tests/data/expected/main/jsonschema/use_generic_base_class_deep_inheritance/dog.py (4)
tests/data/expected/main/jsonschema/use_generic_base_class_deep_inheritance/mammal.py (1)
Mammal(10-11)tests/data/expected/main/jsonschema/use_generic_base_class_module_split/dog.py (1)
Dog(10-11)tests/data/expected/main/jsonschema/use_generic_base_class_multi_root/dog.py (1)
Dog(10-11)tests/data/expected/main/jsonschema/use_generic_base_class_with_inheritance.py (1)
Dog(27-28)
tests/data/expected/main/jsonschema/use_generic_base_class_deep_inheritance/animal.py (2)
tests/data/expected/main/jsonschema/use_generic_base_class.py (1)
BaseModel(11-14)src/datamodel_code_generator/model/pydantic_v2/__init__.py (1)
ConfigDict(26-41)
src/datamodel_code_generator/parser/base.py (4)
src/datamodel_code_generator/model/base.py (5)
imports(283-302)imports(638-643)path(735-737)create_base_class_model(714-727)BaseClassDataType(463-464)src/datamodel_code_generator/imports.py (3)
append(74-89)Import(20-38)from_full_path(35-38)src/datamodel_code_generator/model/msgspec.py (1)
create_base_class_model(170-207)src/datamodel_code_generator/model/pydantic_v2/base_model.py (1)
create_base_class_model(279-318)
🪛 Ruff (0.14.8)
src/datamodel_code_generator/model/base.py
719-719: Unused noqa directive (non-enabled: FBT001, FBT002)
Remove unused noqa directive
(RUF100)
720-720: Unused noqa directive (non-enabled: FBT001, FBT002)
Remove unused noqa directive
(RUF100)
src/datamodel_code_generator/model/pydantic_v2/base_model.py
284-284: Unused noqa directive (non-enabled: FBT001, FBT002)
Remove unused noqa directive
(RUF100)
285-285: Unused noqa directive (non-enabled: FBT001, FBT002)
Remove unused noqa directive
(RUF100)
src/datamodel_code_generator/model/msgspec.py
175-175: Unused noqa directive (non-enabled: FBT001, FBT002)
Remove unused noqa directive
(RUF100)
176-176: Unused noqa directive (non-enabled: FBT001, FBT002)
Remove unused noqa directive
(RUF100)
src/datamodel_code_generator/parser/base.py
650-650: Unused noqa directive (non-enabled: PLR0912, PLR0913, PLR0915)
Remove unused noqa directive
(RUF100)
2012-2012: Unused noqa directive (non-enabled: PLR0912, PLR0914, PLR0915)
Remove unused noqa directive
(RUF100)
2084-2084: Unused noqa directive (non-enabled: SLF001)
Remove unused noqa directive
(RUF100)
2085-2085: Unused noqa directive (non-enabled: SLF001)
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: 3.10 on Ubuntu
- GitHub Check: 3.13 on Windows
- GitHub Check: 3.10 on Windows
- GitHub Check: 3.11 on Windows
- GitHub Check: 3.14 on Windows
- GitHub Check: 3.13 on Ubuntu
- GitHub Check: 3.14 on Ubuntu
- GitHub Check: 3.12 on Windows
- GitHub Check: Analyze (python)
- GitHub Check: benchmarks
🔇 Additional comments (35)
tests/data/expected/main/jsonschema/use_generic_base_class_deep_inheritance/animal.py (1)
1-18: LGTM! Correctly implements the DRY base class pattern.This expected output file correctly demonstrates the core feature: a shared
BaseModelclass that definesmodel_configonce (lines 11-14), withAnimalinheriting from it (line 17). This achieves the PR objective of eliminating repeatedmodel_configblocks across multiple models.tests/data/expected/main/jsonschema/use_generic_base_class_deep_inheritance/dog.py (1)
1-11: LGTM! Correct deep inheritance structure.This file correctly extends the inheritance chain to
Dog(Mammal)(line 10), demonstrating that the feature works with multi-level inheritance. The absence ofmodel_confighere confirms that models properly inherit the shared configuration from the base class hierarchy.tests/data/expected/main/jsonschema/use_generic_base_class_deep_inheritance/mammal.py (1)
1-11: LGTM! Correctly demonstrates intermediate inheritance.The
Mammal(Animal)class (line 10) correctly forms the middle layer of the inheritance chain, demonstrating that the feature works properly with multi-level hierarchies. The sharedmodel_configfromBaseModelpropagates throughAnimaltoMammalwithout repetition.tests/data/expected/main/jsonschema/use_generic_base_class_circular/friend.py (1)
1-7: LGTM! Clean re-export pattern for circular references.This expected output correctly demonstrates the modular structure generated when
--use-generic-base-classis used with circular references. The re-export pattern (importing from_internaland declaring__all__) is a standard Python approach for resolving circular imports while maintaining a clean public API.tests/data/expected/main/jsonschema/use_generic_base_class_multi_root/dog.py (1)
1-11: LGTM! Expected test output correctly demonstrates the DRY base-class feature.This test file properly demonstrates the
--use-generic-base-classfeature:
- Dog inherits from Animal (which inherits from the shared BaseModel with
model_config)- No repeated
model_configblock in Dog (the DRY benefit)- Correct multi-level inheritance chain for the multi-root structure
tests/data/expected/main/jsonschema/use_generic_base_class_frozen.py (3)
1-8: LGTM! Correct import structure for the DRY base class pattern.The import aliasing (
BaseModel as _BaseModel) correctly enables the definition of a customBaseModelthat wraps the Pydantic base with shared configuration.
11-14: LGTM! Correctly implements the shared base class with frozen config.The custom
BaseModelsuccessfully consolidates thefrozen=Trueconfiguration in one place, which will be inherited by all models. This achieves the DRY principle that is the core objective of the--use-generic-base-classfeature.
17-18: LGTM! Model correctly inherits shared configuration.The
Simplemodel inherits from the customBaseModeland correctly omits themodel_configdefinition, demonstrating that the shared configuration is properly inherited. This is exactly the DRY pattern the feature aims to achieve.src/datamodel_code_generator/model/enum.py (1)
45-45: LGTM!Correctly marking
Enumas not supporting the generic base class feature. Enums have their own distinct inheritance hierarchy (enum.Enum,enum.StrEnum,enum.IntEnum) and shouldn't participate in the pydantic/msgspec base class pattern.docs/cli-reference/index.md (1)
14-14: LGTM!Documentation is correctly updated with the new
--use-generic-base-classoption. The option count increment and the alphabetically-placed link are accurate.Also applies to: 170-170
src/datamodel_code_generator/model/pydantic_v2/imports.py (1)
10-10: LGTM!The new
IMPORT_BASE_MODELconstant follows the established pattern and is needed for aliasingpydantic.BaseModelas_BaseModelwhen generating the generic base class.src/datamodel_code_generator/model/type_alias.py (1)
24-25: LGTM!Both changes are appropriate:
IS_ALIAScorrectly annotated asClassVar[bool](was previously justboolfor a class-level constant)SUPPORTS_GENERIC_BASE_CLASS = Falsecorrectly marks that type aliases don't participate in the generic base class feature since they aren't class-based modelstests/data/expected/main/jsonschema/use_generic_base_class_multi_root/vehicle.py (1)
1-18: LGTM!The test fixture correctly demonstrates the DRY pattern introduced by the
--use-generic-base-classfeature:
BaseModelis aliased from_BaseModelto avoid naming collisionmodel_configwithextra='forbid'is defined once in the sharedBaseModelVehicleinherits the configuration without repeating ittests/data/expected/main/jsonschema/use_generic_base_class_populate_by_name.py (1)
2-2: Inconsistent source filename in generated header.The header comment indicates
filename: use_generic_base_class_simple.json, but this expected output file is for thepopulate_by_namescenario. The source JSON file should likely be named differently (e.g.,use_generic_base_class_populate_by_name.json) or the test is reusing the same input file for multiple scenarios.If this is intentional reuse, consider if the header should reflect the actual test case name for clarity. Otherwise, verify the test input file naming.
tests/data/expected/main/jsonschema/use_generic_base_class_msgspec_forbid.py (2)
2-2: Same source filename discrepancy.Similar to the
populate_by_nametest, this file referencesuse_generic_base_class_simple.jsonin the header while being themsgspec_forbidexpected output. Please verify test input consistency.
14-19: LGTM on the msgspec generic base class pattern.The msgspec implementation correctly demonstrates the DRY pattern:
Structaliased from_Struct- Base
Structclass appliesforbid_unknown_fields=Trueandfrozen=Trueas class parametersSimpleinherits configuration cleanly without repetitionThis properly mirrors the pydantic approach adapted for msgspec's class-parameter-based configuration style.
tests/data/expected/main/jsonschema/use_generic_base_class_allow_extra.py (1)
1-17: LGTM!The expected output correctly demonstrates the DRY pattern:
BaseModeldefinesmodel_configonce withextra='allow', andSimpleinherits from it without repeating the configuration. This aligns with the PR objective of avoiding duplicatemodel_configblocks.src/datamodel_code_generator/arguments.py (1)
272-278: LGTM!The new
--use-generic-base-classoption follows the established pattern for boolean CLI flags in this file, with appropriate placement in the model customization group and clear help text explaining the DRY benefit.tests/data/jsonschema/use_generic_base_class_with_inheritance.json (1)
1-28: LGTM!This test schema appropriately covers the inheritance scenario using
allOf, which is essential for verifying that the generic base class feature correctly handles models with existing inheritance relationships.src/datamodel_code_generator/parser/openapi.py (1)
202-202: LGTM!The
use_generic_base_classparameter is correctly added to the__init__signature and properly forwarded to the parent class constructor at line 302. This follows the established pattern for propagating configuration options through the parser hierarchy.src/datamodel_code_generator/model/base.py (2)
52-53: Constants support the generic base class feature.These constants are used by the parser module (
src/datamodel_code_generator/parser/base.py) to wire the generic base-class flow into the model generation process. The SAST warnings about unused global variables appear to be false positives as the imports happen in other modules.
479-480: LGTM!The
SUPPORTS_GENERIC_BASE_CLASSclass variable provides a clean opt-in/opt-out mechanism for model types, and typingIS_ALIASasClassVar[bool]correctly reflects its nature as a class-level constant.tests/data/expected/main/jsonschema/use_generic_base_class_with_inheritance.py (1)
1-27: LGTM! Inheritance chain is correctly structured.The expected output demonstrates the proper inheritance hierarchy:
BaseModelinherits from_BaseModel(aliased pydantic BaseModel) with sharedmodel_configAnimalinherits from the project'sBaseModelDoginherits fromAnimal(single inheritance, avoiding MRO issues)This correctly implements the DRY pattern for
model_configwhile preserving schema-defined inheritance relationships.src/datamodel_code_generator/model/pydantic_v2/base_model.py (1)
278-317: Well-structured base class factory method.The implementation correctly:
- Creates a shared base model inheriting from the aliased
_BaseModel- Propagates configuration through
extra_template_data- Manages imports by removing duplicates and adding the aliased import
src/datamodel_code_generator/model/msgspec.py (1)
169-207: Consistent implementation with Pydantic v2 pattern.The
create_base_class_modelmethod follows the same structure as the Pydantic v2 implementation while correctly adapting to msgspec's base class kwargs pattern for configuration.tests/main/jsonschema/test_main_jsonschema.py (3)
3716-3737: Comprehensive CLI documentation and test coverage for--use-generic-base-class.The primary test case includes proper
@pytest.mark.cli_docdecoration for documentation generation, with clear inputs and expected outputs. The test correctly validates the DRY pattern formodel_configwith the--extra-fields forbidoption.
3862-3878: Module split test correctly validates cross-module base class inheritance.This test ensures the generic base class is properly imported across module boundaries when using
--module-split-mode single, which is critical for the feature's usability in larger codebases.
3938-3972: msgspec coverage validates feature works across model types.These tests ensure the
--use-generic-base-classfeature works correctly withmsgspec.Struct, including the limited configuration mapping (frozen via--enable-faux-immutabilityand forbid unknown fields via--extra-fields forbid).src/datamodel_code_generator/parser/base.py (3)
55-65: LGTM!The new imports for
GENERIC_BASE_CLASS_NAMEandGENERIC_BASE_CLASS_PATHare correctly added alongside other imports fromdatamodel_code_generator.model.base.
822-853: LGTM!The initialization of
use_generic_base_classandgeneric_base_class_config, along with the conditional routing of configuration options, is correctly implemented. The pattern consistently directs settings to either the shared base class configuration or per-model template data based on the flag.
2702-2703: LGTM!The invocation of
__apply_generic_base_class(processed_models)is correctly positioned after all model processing steps and before the final rendering phase. This ensures the generic base class can be properly applied across all processed modules.tests/data/expected/main/jsonschema/use_generic_base_class_module_split/animal.py (4)
5-8: LGTM!The imports are correct for Pydantic v2. The aliasing of
BaseModelas_BaseModelfollows the expected pattern for the generic base class feature, allowing the customBaseModelto be defined without naming conflicts.
11-14: LGTM!The
BaseModelclass correctly implements the DRY pattern by definingmodel_configonce in a shared base class. This achieves the PR objective of avoiding repeatedmodel_configblocks across generated models. The structure matches the expected output from related test files.
17-18: LGTM!The
Animalclass correctly inherits from the customBaseModel, demonstrating the benefit of the generic base class feature—no need to repeatmodel_configin this model. The type annotation uses modern Python 3.10+ union syntax (str | None), which is appropriate for the target codebase.
1-4: No issue identified. The filename metadata in the header comment correctly referencesuse_generic_base_class_with_inheritance.json, which is the actual input JSON file. Theuse_generic_base_class_module_splitdirectory is an output variant generated from this same JSON input file with module-split output enabled. The tool's generated header always shows the source input filename regardless of the output structure.
2 participants
Fixes: #2501
Summary by CodeRabbit
New Features
Documentation
Tests
✏️ Tip: You can customize this high-level summary in your review settings.