Skip to content

Add --field-type-collision-strategy option#2735

Merged
koxudaxi merged 1 commit intomainfrom
feature/field-type-collision-strategy-v2
Dec 22, 2025
Merged

Add --field-type-collision-strategy option#2735
koxudaxi merged 1 commit intomainfrom
feature/field-type-collision-strategy-v2

Conversation

@koxudaxi
Copy link
Copy Markdown
Owner

@koxudaxi koxudaxi commented Dec 22, 2025

Fixes: #2364

Summary by CodeRabbit

Release Notes

  • New Features
    • Introduced --field-type-collision-strategy command-line option for handling field and type name collisions. Choose between RenameField (renames the field) or RenameType (renames the type) strategies.
    • Collision handling support now available across JSON Schema, GraphQL, and OpenAPI parsers.
    • Compatible with Pydantic v2 model generation.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Dec 22, 2025

Warning

Rate limit exceeded

@koxudaxi has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 11 minutes and 44 seconds before requesting another review.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

📥 Commits

Reviewing files that changed from the base of the PR and between 60a1ce5 and c410609.

📒 Files selected for processing (16)
  • docs/cli-reference/field-customization.md
  • docs/cli-reference/index.md
  • docs/cli-reference/quick-reference.md
  • src/datamodel_code_generator/__init__.py
  • src/datamodel_code_generator/__main__.py
  • src/datamodel_code_generator/arguments.py
  • src/datamodel_code_generator/cli_options.py
  • src/datamodel_code_generator/parser/base.py
  • src/datamodel_code_generator/parser/graphql.py
  • src/datamodel_code_generator/parser/jsonschema.py
  • src/datamodel_code_generator/parser/openapi.py
  • tests/data/expected/main/jsonschema/field_has_same_name_rename_type.py
  • tests/data/expected/main/jsonschema/field_type_collision_rename_type_double.py
  • tests/data/jsonschema/field_type_collision_complex.json
  • tests/data/jsonschema/field_type_collision_rename_type_double.json
  • tests/main/jsonschema/test_main_jsonschema.py

Walkthrough

This PR introduces a new FieldTypeCollisionStrategy enum with RenameField and RenameType options to control how field/type name collisions are resolved in generated Pydantic v2 models. The configuration is threaded through the CLI, config, and parser layer, with collision handling logic implemented in the base parser's field-name resolution method.

Changes

Cohort / File(s) Summary
Core API & Enum Definition
src/datamodel_code_generator/__init__.py
Added FieldTypeCollisionStrategy enum with RenameField and RenameType members; extended generate() function signature with field_type_collision_strategy: FieldTypeCollisionStrategy | None parameter and threaded it through parser instantiation calls.
CLI & Config Wiring
src/datamodel_code_generator/__main__.py
src/datamodel_code_generator/arguments.py
src/datamodel_code_generator/cli_options.py
Added field_type_collision_strategy config field to Config model; exposed --field-type-collision-strategy CLI argument in field customization group; added metadata entry for new CLI option.
Parser Layer Threading
src/datamodel_code_generator/parser/base.py
src/datamodel_code_generator/parser/graphql.py
src/datamodel_code_generator/parser/jsonschema.py
src/datamodel_code_generator/parser/openapi.py
Added field_type_collision_strategy parameter to all parser __init__ signatures and propagated to superclass; implemented collision-handling logic in base.Parser.__change_field_name() to rename type (via colliding_reference) when RenameType strategy is active, otherwise preserve field-renaming behavior.
Test Data & Cases
tests/data/jsonschema/field_type_collision_rename_type_double.json
tests/data/jsonschema/field_type_collision_complex.json
tests/data/expected/main/jsonschema/field_has_same_name_rename_type.py
tests/data/expected/main/jsonschema/field_type_collision_rename_type_double.py
tests/main/jsonschema/test_main_jsonschema.py
Added JSON Schema test inputs and Pydantic model outputs for rename-type collision scenarios; added three test cases validating the new strategy with existing and edge-case input schemas.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

  • Key areas requiring attention:
    • The collision-handling refactor in base.Parser.__change_field_name() introduces conditional logic with type-rename behavior and reference tracking; verify the conditional branches handle both RenameField (default) and RenameType paths correctly.
    • Ensure colliding_reference tracking and all_class_names updates are consistent across the collision path.
    • Validate that the new parameter threading through all parser subclasses (graphql, jsonschema, openapi) properly invokes the parent constructor without breaking initialization order.
    • Cross-check test expectations against generated output for edge cases (nested collisions, pre-existing _1 suffixes in schema).

Possibly related PRs

Poem

🐰 A clever collision, we solve it with care—
Rename the type or the field, take your pick there!
Through layers we thread a new strategy's dance,
Where Pydantic v2 models get their second chance! ✨

Pre-merge checks and finishing touches

✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: adding a new CLI option for handling field-type collisions.
Linked Issues check ✅ Passed The PR implements the core requirement from issue #2364 by introducing a collision-handling strategy option (RenameField vs RenameType) allowing users to choose between renaming fields or types when collisions occur.
Out of Scope Changes check ✅ Passed All changes are directly related to implementing the field-type-collision-strategy feature, including CLI option, parser updates, configuration plumbing, and test coverage.
Docstring Coverage ✅ Passed Docstring coverage is 85.71% which is sufficient. The required threshold is 80.00%.

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@codecov
Copy link
Copy Markdown

codecov Bot commented Dec 22, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.36%. Comparing base (e8ff0f8) to head (c410609).
⚠️ Report is 2 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #2735   +/-   ##
=======================================
  Coverage   99.36%   99.36%           
=======================================
  Files          83       83           
  Lines       12068    12094   +26     
  Branches     1456     1458    +2     
=======================================
+ Hits        11991    12017   +26     
  Misses         45       45           
  Partials       32       32           
Flag Coverage Δ
unittests 99.36% <100.00%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@codspeed-hq
Copy link
Copy Markdown

codspeed-hq Bot commented Dec 22, 2025

CodSpeed Performance Report

Merging #2735 will not alter performance

Comparing feature/field-type-collision-strategy-v2 (c410609) with main (e8ff0f8)

Summary

✅ 70 untouched
⏩ 10 skipped1

Footnotes

  1. 10 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports.

@koxudaxi koxudaxi force-pushed the feature/field-type-collision-strategy-v2 branch from 39d8c72 to 60a1ce5 Compare December 22, 2025 13:24
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (3)
src/datamodel_code_generator/__main__.py (1)

24-40: Config wiring is correct; drop unused # noqa: UP045 to satisfy Ruff

Importing FieldTypeCollisionStrategy, adding it to Config, and forwarding config.field_type_collision_strategy into generate() are all consistent with the rest of the configuration pipeline. The only problem is the new # noqa: UP045 on the config field, which Ruff flags as an unused noqa (rule not enabled).

You can fix this by removing the noqa directive:

Proposed diff to remove unused noqa
-    field_type_collision_strategy: Optional[FieldTypeCollisionStrategy] = None  # noqa: UP045
+    field_type_collision_strategy: Optional[FieldTypeCollisionStrategy] = None

Also applies to: 474-475, 781-783

src/datamodel_code_generator/parser/base.py (2)

859-859: Fix variable name typo.

The variable is named filed_name but should be field_name for consistency and clarity.

🔎 Proposed fix
-                filed_name = field.name
+                field_name = field.name

Note: This typo is also used on lines 867 and 877, which should be updated accordingly.


870-880: Add runtime type check before casting to DataModel.

Line 876 casts colliding_reference.source to DataModel without verifying the actual type. Reference sources can be Enum, TypeAlias, or other model types. If the source is not a DataModel, line 879 (source.class_name = new_class_name) could fail or produce unexpected behavior.

Additionally, line 877 uses the typo variable filed_name (should be field_name).

🔎 Proposed fix
                if colliding_reference is not None:
                    resolver = ModelResolver(
                        exclude_names=all_class_names.copy(),
                        snake_case_field=self.snake_case_field,
                        remove_suffix_number=True,
                    )
-                   source = cast("DataModel", colliding_reference.source)
-                   resolver.exclude_names.add(cast("str", filed_name))
-                   new_class_name = resolver.add(["type"], cast("str", source.class_name)).name
-                   source.class_name = new_class_name
-                   all_class_names.add(new_class_name)
+                   source = colliding_reference.source
+                   if isinstance(source, DataModel):
+                       resolver.exclude_names.add(cast("str", field_name))
+                       new_class_name = resolver.add(["type"], cast("str", source.class_name)).name
+                       source.class_name = new_class_name
+                       all_class_names.add(new_class_name)
🧹 Nitpick comments (1)
src/datamodel_code_generator/__init__.py (1)

273-282: FieldTypeCollisionStrategy enum design matches existing patterns; consider optional export.

The enum (RenameField / RenameType with hyphenated values) is consistent with other strategy enums and the intended CLI surface. If you want it available via from datamodel_code_generator import *, consider adding FieldTypeCollisionStrategy to __all__ alongside the other public strategy enums; otherwise the current definition is fine.

Also applies to: 884-899

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between e8ff0f8 and 60a1ce5.

📒 Files selected for processing (13)
  • src/datamodel_code_generator/__init__.py
  • src/datamodel_code_generator/__main__.py
  • src/datamodel_code_generator/arguments.py
  • src/datamodel_code_generator/cli_options.py
  • src/datamodel_code_generator/parser/base.py
  • src/datamodel_code_generator/parser/graphql.py
  • src/datamodel_code_generator/parser/jsonschema.py
  • src/datamodel_code_generator/parser/openapi.py
  • tests/data/expected/main/jsonschema/field_has_same_name_rename_type.py
  • tests/data/expected/main/jsonschema/field_type_collision_rename_type_double.py
  • tests/data/jsonschema/field_type_collision_complex.json
  • tests/data/jsonschema/field_type_collision_rename_type_double.json
  • tests/main/jsonschema/test_main_jsonschema.py
🧰 Additional context used
🧬 Code graph analysis (9)
tests/data/expected/main/jsonschema/field_has_same_name_rename_type.py (1)
tests/data/expected/parser/openapi/openapi_parser_parse_modular/bar.py (1)
  • Field (6-7)
src/datamodel_code_generator/arguments.py (1)
src/datamodel_code_generator/__init__.py (1)
  • FieldTypeCollisionStrategy (273-281)
src/datamodel_code_generator/parser/openapi.py (1)
src/datamodel_code_generator/__init__.py (1)
  • FieldTypeCollisionStrategy (273-281)
src/datamodel_code_generator/parser/jsonschema.py (1)
src/datamodel_code_generator/__init__.py (1)
  • FieldTypeCollisionStrategy (273-281)
src/datamodel_code_generator/parser/base.py (6)
src/datamodel_code_generator/__init__.py (1)
  • FieldTypeCollisionStrategy (273-281)
src/datamodel_code_generator/model/base.py (6)
  • class_name (679-681)
  • class_name (684-688)
  • base_class (668-670)
  • field (333-335)
  • name (658-660)
  • all_data_types (706-710)
src/datamodel_code_generator/model/dataclass.py (1)
  • field (141-146)
src/datamodel_code_generator/model/pydantic/base_model.py (1)
  • field (90-104)
src/datamodel_code_generator/reference.py (3)
  • Reference (141-200)
  • reference (76-78)
  • add (775-829)
src/datamodel_code_generator/types.py (1)
  • all_data_types (424-430)
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/field_type_collision_rename_type_double.py (1)
src/datamodel_code_generator/model/enum.py (1)
  • Enum (39-121)
src/datamodel_code_generator/parser/graphql.py (1)
src/datamodel_code_generator/__init__.py (1)
  • FieldTypeCollisionStrategy (273-281)
src/datamodel_code_generator/__main__.py (1)
src/datamodel_code_generator/__init__.py (1)
  • FieldTypeCollisionStrategy (273-281)
🪛 Ruff (0.14.8)
src/datamodel_code_generator/__main__.py

474-474: Unused noqa directive (non-enabled: UP045)

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). (7)
  • GitHub Check: 3.13 on Windows
  • GitHub Check: 3.11 on Windows
  • GitHub Check: 3.10 on Windows
  • GitHub Check: 3.12 on Windows
  • GitHub Check: 3.14 on Windows
  • GitHub Check: Analyze (python)
  • GitHub Check: benchmarks
🔇 Additional comments (15)
tests/data/expected/main/jsonschema/field_has_same_name_rename_type.py (1)

1-15: Expected model for rename‑type strategy looks consistent

The generated classes reflect the rename‑type behavior correctly: the field TestObject keeps its original name while the referenced type is suffixed (TestObject_1). No issues spotted.

tests/data/expected/main/jsonschema/field_type_collision_rename_type_double.py (1)

1-22: Rename‑type double‑collision expectations are coherent

The expected code cleanly disambiguates the enum and model names while keeping the Order.Status field bound to the enum. This matches the described rename‑type semantics without obvious edge‑case issues.

src/datamodel_code_generator/cli_options.py (1)

138-140: CLI docs metadata for --field-type-collision-strategy is wired correctly

The option is registered with the correct canonical name and placed in the FIELD category, staying in sync with arguments.py and existing doc tooling.

tests/data/jsonschema/field_type_collision_rename_type_double.json (1)

1-22: Schema covers the intended double‑collision scenario

The JSON Schema cleanly models the Status enum plus an additional Status_1 object, matching the expectations in the new golden file. No structural or validity problems seen.

src/datamodel_code_generator/arguments.py (1)

24-24: New CLI argument for field/type collision strategy is correctly defined

The import and field_options.add_argument wiring look consistent with the FieldTypeCollisionStrategy enum and existing option patterns (group placement, choices, default handling, and help text). No behavioral issues identified here.

Also applies to: 625-632

tests/data/jsonschema/field_type_collision_complex.json (1)

1-44: Complex collision schema is well‑formed and suitable for tests

The schema definition is valid and captures the layered enum/object relationships you need to stress the collision strategy logic. No changes needed.

tests/main/jsonschema/test_main_jsonschema.py (1)

3575-3633: New tests robustly cover the rename‑type collision strategy

The three added tests exercise:

  • Basic rename‑type behavior on field_has_same_name.json
  • The same scenario through the CLI‑docs harness
  • A schema with an existing _1 suffix to validate idempotent renaming

Arguments, expected files, and marks (cli_doc) are consistent with surrounding tests. No issues found.

src/datamodel_code_generator/parser/jsonschema.py (1)

24-37: JsonSchemaParser wiring for field_type_collision_strategy is clean and backward compatible.

Importing FieldTypeCollisionStrategy, adding it as a final keyword-only arg, and forwarding it to super().__init__ all look correct and consistent with the parser API. Existing call sites remain unaffected because everything after * is keyword-only.

Also applies to: 512-612, 613-709

src/datamodel_code_generator/parser/openapi.py (1)

20-35: OpenAPIParser correctly threads field_type_collision_strategy through to the base parser.

The added import, keyword-only parameter, and super().__init__ forwarding are consistent with JsonSchemaParser and keep the public constructor backward compatible while exposing the new collision strategy.

Also applies to: 181-283, 285-381

src/datamodel_code_generator/parser/graphql.py (1)

16-28: GraphQLParser integration with FieldTypeCollisionStrategy is consistent and safe.

The new enum import, added keyword-only field_type_collision_strategy argument, and forwarding to super().__init__ are aligned with the base Parser API and won’t break existing callers.

Also applies to: 98-199, 200-297

src/datamodel_code_generator/__init__.py (1)

393-500: generate() correctly exposes and forwards field_type_collision_strategy.

Adding field_type_collision_strategy as a keyword-only parameter to generate() and passing it through to parser_class(...) ensures the new collision strategy is configurable from the top-level API without impacting existing callers.

Also applies to: 645-747

src/datamodel_code_generator/parser/base.py (4)

31-31: LGTM: Field type collision strategy configuration properly threaded through Parser.

The import, parameter, and instance attribute correctly introduce the new collision strategy configuration to the parser.

Also applies to: 774-774, 931-931


850-853: LGTM: Collision strategy initialization is correct.

The logic correctly determines whether to rename types based on the strategy and builds the set of existing class names for collision detection.


881-890: LGTM: Field renaming logic correctly implements RenameField strategy.

The elif not rename_type condition ensures field renaming only occurs when NOT using the RenameType strategy. When RenameType is active, this block is skipped (either because a type was renamed or because there was no collision), which is the intended behavior.


860-869: The line numbers in this review comment are incorrect. The code snippet shown (containing allow_extra_fields and extra_fields configuration) is from lines 860-869, but the actual collision detection logic discussed in the comment is located at lines 1860-1880. The collision handling code at lines 1867-1868 does check colliding_reference is None to capture only the first collision, but this logic addresses field-to-type-name collisions within field_type_collision_strategy, which is a more specific scenario than multiple references with identical short names within a single field's data type. Correct the line number reference to lines 1860-1880 if the intent is to review this collision handling logic.

Likely an incorrect or invalid review comment.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Field name changing in Pydantic V2 model

1 participant