Add deprecated decorators for dataclass output#3044
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughPropagates JSON Schema "deprecated" metadata into model template data and applies a class-level Changes
Sequence DiagramsequenceDiagram
participant Parser as JSONSchemaParser
participant Metadata as SchemaMetadata
participant Factory as ModelFactory
participant DataClass as DataClass.__init__
participant Model as DataModel
Parser->>Metadata: _set_schema_metadata(path, obj)
Metadata->>Metadata: set_deprecated(path, obj)
Metadata-->>Parser: record deprecated in extra_template_data
Parser->>Factory: create model / reference
Factory->>DataClass: instantiate DataClass(with template data)
DataClass->>Model: _set_deprecated_decorator()
Model->>Model: inspect extra_template_data["deprecated"] and decorators
alt deprecated and no existing decorator
Model->>Model: append `@deprecated("ClassName is deprecated.")`
Model->>Model: add `typing_extensions.deprecated` to _additional_imports
end
Model-->>Factory: model ready
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
Merging this PR will not alter performance
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3044 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 86 86
Lines 18045 18071 +26
Branches 2098 2101 +3
=========================================
+ Hits 18045 18071 +26
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:
|
![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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/datamodel_code_generator/parser/jsonschema.py (1)
3623-3777:⚠️ Potential issue | 🟡 MinorMove
_set_schema_metadata()above the non-nullable enum early return.Line 3774 returns before metadata is recorded, so ordinary enums never get the new
deprecatedflag inextra_template_data. Only the nullable-wrapper enum path hits the new metadata flow.💡 Proposed fix
reference = self.model_resolver.add( path, name, class_name=True, singular_name=singular_name, singular_name_suffix="Enum", loaded=True, model_type="enum", ) + self._set_schema_metadata(reference.path, obj) + self.set_schema_extensions(reference.path, obj) if not nullable: return create_enum(reference) - - self._set_schema_metadata(reference.path, obj) - self.set_schema_extensions(reference.path, obj) enum_reference = self.model_resolver.add( [*path, "Enum"], f"{reference.name}Enum", class_name=True,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/datamodel_code_generator/parser/jsonschema.py` around lines 3623 - 3777, The parse_enum path returns early for non-nullable enums before recording schema metadata, so move the metadata calls above that return: inside parse_enum, call self._set_schema_metadata(reference.path, obj) and self.set_schema_extensions(reference.path, obj) immediately after creating the Reference (the reference variable produced by self.model_resolver.add) and before the line that currently does "return create_enum(reference)"; this ensures both the non-nullable branch and the nullable-wrapper branch record deprecated/extra_template_data consistently (references: parse_enum, create_enum, _set_schema_metadata, set_schema_extensions, model_resolver.add).
🧹 Nitpick comments (1)
tests/model/pydantic_v2/test_dataclass.py (1)
136-151: LGTM!The test correctly verifies the deprecated decorator functionality for Pydantic v2 dataclasses, mirroring the test in the standard dataclass module.
Consider adding a test for existing decorator preservation.
The standard dataclass tests include
test_dataclass_preserves_existing_deprecated_decoratorto verify no decorator duplication. For consistency and complete coverage of the Pydantic v2 dataclass path, consider adding a similar test here.,
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/model/pydantic_v2/test_dataclass.py` around lines 136 - 151, Add a new test (e.g., test_data_class_preserves_existing_deprecated_decorator) alongside test_data_class_with_deprecated_decorator that constructs a DataClass (using DataModelFieldBase, DataType, Reference) but simulates an already-present deprecated decorator in the template data (e.g., include a pre-existing decorator string in extra_template_data for "test_model"); render the DataClass and assert the deprecated decorator appears only once in rendered output and that the typing_extensions import for "deprecated" is not duplicated (check data_class.imports) to ensure existing decorators are preserved rather than duplicated.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/datamodel_code_generator/model/base.py`:
- Around line 742-750: The code mutates a caller-owned list because
self.decorators aliases the input from DataModel.__init__, so
_set_deprecated_decorator should not append in-place; instead create a new list
and reassign it (e.g., set self.decorators = list(self.decorators) + [f"..."] or
construct a new list and assign) when adding the deprecated decorator in
_set_deprecated_decorator, leaving the original caller list unchanged; keep
adding the import to self._additional_imports as before.
---
Outside diff comments:
In `@src/datamodel_code_generator/parser/jsonschema.py`:
- Around line 3623-3777: The parse_enum path returns early for non-nullable
enums before recording schema metadata, so move the metadata calls above that
return: inside parse_enum, call self._set_schema_metadata(reference.path, obj)
and self.set_schema_extensions(reference.path, obj) immediately after creating
the Reference (the reference variable produced by self.model_resolver.add) and
before the line that currently does "return create_enum(reference)"; this
ensures both the non-nullable branch and the nullable-wrapper branch record
deprecated/extra_template_data consistently (references: parse_enum,
create_enum, _set_schema_metadata, set_schema_extensions, model_resolver.add).
---
Nitpick comments:
In `@tests/model/pydantic_v2/test_dataclass.py`:
- Around line 136-151: Add a new test (e.g.,
test_data_class_preserves_existing_deprecated_decorator) alongside
test_data_class_with_deprecated_decorator that constructs a DataClass (using
DataModelFieldBase, DataType, Reference) but simulates an already-present
deprecated decorator in the template data (e.g., include a pre-existing
decorator string in extra_template_data for "test_model"); render the DataClass
and assert the deprecated decorator appears only once in rendered output and
that the typing_extensions import for "deprecated" is not duplicated (check
data_class.imports) to ensure existing decorators are preserved rather than
duplicated.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 9b77c985-ec73-45be-bc39-f6dd579abab0
⛔ Files ignored due to path filters (1)
tests/data/jsonschema/deprecated_dataclass.jsonis excluded by!tests/data/**/*.jsonand included by none
📒 Files selected for processing (8)
src/datamodel_code_generator/model/base.pysrc/datamodel_code_generator/model/dataclass.pysrc/datamodel_code_generator/model/pydantic_v2/dataclass.pysrc/datamodel_code_generator/parser/jsonschema.pytests/data/expected/main/jsonschema/deprecated_dataclass.pytests/main/jsonschema/test_main_jsonschema.pytests/model/dataclass/test_param.pytests/model/pydantic_v2/test_dataclass.py
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/datamodel_code_generator/model/base.py`:
- Around line 742-750: The _set_deprecated_decorator method currently returns
early if a "@deprecated" decorator already exists, which prevents adding the
required import; update _set_deprecated_decorator so that when
extra_template_data.get("deprecated") is true you always append
Import.from_full_path("typing_extensions.deprecated") to
self._additional_imports (regardless of the early-return path), and only skip
adding the decorator string to self.decorators if one already exists; reference
_set_deprecated_decorator, self.decorators, self._additional_imports,
Import.from_full_path("typing_extensions.deprecated") and self.class_name to
locate the change.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 6ba1f0f2-d67f-4263-bb84-55b067386ecb
📒 Files selected for processing (4)
src/datamodel_code_generator/model/base.pysrc/datamodel_code_generator/parser/jsonschema.pytests/data/expected/main/jsonschema/deprecated_pydantic_v2_dataclass.pytests/main/jsonschema/test_main_jsonschema.py
🚧 Files skipped from review as they are similar to previous changes (1)
- tests/main/jsonschema/test_main_jsonschema.py
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@tests/main/jsonschema/test_main_jsonschema.py`:
- Around line 3978-3992: Add a second test alongside
test_main_dataclass_deprecated_model_preserves_existing_decorator that verifies
the generator will add the deprecated decorator and import when a different
pre-existing decorator is present: copy the existing test pattern using
run_main_and_assert with input_path "deprecated_dataclass.json", keep
"--output-model-type" and use "--class-decorators" to pass a non-deprecated
decorator (e.g., "@some_decorator"), and assert against a new expected output
file that contains both the original decorator and the added `@deprecated`
import/annotation; name the new test to indicate it covers the non-@deprecated
existing-decorator case and use the same assert_file_content helper.
- Around line 3995-4004: Add a new test that mirrors the "already has
decorators" regression case for the pydantic_v2.dataclass backend: create a test
function alongside test_main_pydantic_v2_dataclass_deprecated_model (e.g.,
test_main_pydantic_v2_dataclass_deprecated_model_with_existing_decorators) that
calls run_main_and_assert with input_path=JSON_SCHEMA_DATA_PATH /
"deprecated_dataclass_with_decorators.json" (or the equivalent
existing-decorated input), output_path=output_file,
input_file_type="jsonschema", assert_func=assert_file_content, expected_file set
to the decorated-expected fixture (matching the pattern used for the other
dataclass backend), and
extra_args=["--output-model-type","pydantic_v2.dataclass"]; ensure the test uses
the same decorated fixture names and assertion helper as the existing-decorator
case so decorator/import handling is covered for pydantic_v2.dataclass.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 23c46f5e-f353-4cbd-b6a4-6ce60fdec939
📒 Files selected for processing (2)
src/datamodel_code_generator/model/base.pytests/main/jsonschema/test_main_jsonschema.py
Breaking Change AnalysisResult: No breaking changes detected Reasoning: This PR is an additive feature that implements JSON Schema This analysis was performed by Claude Code Action |
|
🎉 Released in 0.55.0 This PR is now available in the latest release. See the release notes for details. |
1 participant
Fixes: #2543
Summary by CodeRabbit
New Features
Tests