Skip to content

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

Merged
koxudaxi merged 10 commits intomainfrom
feature/field-type-collision-strategy
Dec 22, 2025
Merged

Add --field-type-collision-strategy option#2733
koxudaxi merged 10 commits intomainfrom
feature/field-type-collision-strategy

Conversation

@koxudaxi
Copy link
Copy Markdown
Owner

@koxudaxi koxudaxi commented Dec 22, 2025

Fixes: #2364

Summary by CodeRabbit

  • New Features

    • Added a new CLI option to choose how field/type name collisions are handled (rename-field or rename-type) for Pydantic v2.
  • Documentation

    • CLI reference and quick-reference updated with full option docs, examples, and index entries.
  • Tests

    • Added tests validating the rename-type collision strategy for single- and multi-model schemas.

✏️ 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 12 minutes and 4 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 63cfb52 and 943fb8f.

📒 Files selected for processing (4)
  • docs/cli-reference/field-customization.md
  • src/datamodel_code_generator/parser/base.py
  • tests/data/expected/main/jsonschema/field_type_collision_rename_type.py
  • tests/data/expected/main/jsonschema/field_type_collision_rename_type_multi_model.py

Walkthrough

Introduces FieldTypeCollisionStrategy enum to handle field-type name collisions in Pydantic V2 models. Threads new field_type_collision_strategy parameter through CLI, generate() entry point, and all parser implementations. Updates field collision resolution logic to rename types instead of fields when RenameType strategy is selected. Includes documentation, CLI bindings, and tests.

Changes

Cohort / File(s) Change Summary
Documentation
docs/cli-reference/field-customization.md, docs/cli-reference/index.md, docs/cli-reference/quick-reference.md
Added --field-type-collision-strategy CLI option documentation, usage, examples, and updated option counts/indices
Core API
src/datamodel_code_generator/__init__.py
Added FieldTypeCollisionStrategy enum ("rename-field", "rename-type"); extended generate() signature with field_type_collision_strategy; exported enum in __all__; adjusted Error to accept and store a message
CLI Configuration
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 and CLI argument; registered metadata for the new option
Parser Base Logic
src/datamodel_code_generator/parser/base.py
Added constructor parameter field_type_collision_strategy; updated __change_field_name() to branch on RenameType and rename the colliding type (append underscore) instead of renaming the field when appropriate
Parser Subclasses
src/datamodel_code_generator/parser/graphql.py, src/datamodel_code_generator/parser/jsonschema.py, src/datamodel_code_generator/parser/openapi.py
Threaded new field_type_collision_strategy parameter through each parser __init__ and forwarded it to the base parser
Test Fixtures
tests/data/jsonschema/field_type_collision_rename_type.json, tests/data/jsonschema/field_type_collision_rename_type_multi_model.json
Added JSON Schema fixtures to exercise field/type collision scenarios
Test Expected Output
tests/data/expected/main/jsonschema/field_type_collision_rename_type.py, tests/data/expected/main/jsonschema/field_type_collision_rename_type_multi_model.py
Added expected generated code demonstrating renamed enum types (e.g., Fruit_) under RenameType strategy
Tests
tests/main/jsonschema/test_main_jsonschema.py
Added two tests verifying rename-type strategy output for single and multi-model JSON Schema inputs

Sequence Diagram(s)

sequenceDiagram
    participant User as CLI/User
    participant Config as Config (cli args)
    participant Generate as generate()
    participant Parser as Parser (JsonSchema/OpenAPI/GraphQL)
    participant Codegen as Code Generator / Emitter

    User->>Config: provide --field-type-collision-strategy
    Config->>Generate: call generate(..., field_type_collision_strategy)
    Generate->>Parser: instantiate Parser(..., field_type_collision_strategy)
    Parser->>Parser: parse schemas, detect field/type name collision
    alt strategy == RenameType
        Parser->>Parser: rename type (append underscore)
    else strategy == RenameField or None
        Parser->>Parser: rename field (append suffix/alias)
    end
    Parser->>Codegen: emit models with resolved names
    Codegen->>User: write generated files
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

  • Verify the new enum values and their string representations match CLI choices and docs.
  • Confirm field_type_collision_strategy is consistently threaded from CLI → Config → generate() → all parser subclasses → base parser.
  • Review parser/base.py::__change_field_name for correctness: collision detection, colliding_reference handling, and interactions with aliasing and existing renaming behavior.
  • Check tests and expected outputs for alignment with collision strategy behavior and ensure no accidental duplication in docs.

Possibly related PRs

Suggested labels

safe-to-fix

Suggested reviewers

  • ilovelinux

Poem

🐰 Hopped through code with gentle paws,
Found names that clashed and gave a pause.
Rename the type or let the field be,
I nudge an underscore — tidy as can be! 🥕

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 'Add --field-type-collision-strategy option' accurately describes the main change—introduction of a new CLI option for handling field-type name collision strategies.
Linked Issues check ✅ Passed The PR adds the --field-type-collision-strategy option with 'rename-type' and 'rename-field' choices to resolve field/type collisions, directly addressing issue #2364's request for a configurable strategy.
Out of Scope Changes check ✅ Passed All changes are scoped to implementing the field-type collision strategy feature, including documentation, CLI option, parser logic, and tests. No unrelated or out-of-scope code modifications detected.
Docstring Coverage ✅ Passed Docstring coverage is 84.62% 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.

🤖 Generated by GitHub Actions
@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 (355f2b2) to head (943fb8f).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #2733   +/-   ##
=======================================
  Coverage   99.36%   99.36%           
=======================================
  Files          83       83           
  Lines       12068    12087   +19     
  Branches     1456     1457    +1     
=======================================
+ Hits        11991    12010   +19     
  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 #2733 will not alter performance

Comparing feature/field-type-collision-strategy (943fb8f) with main (355f2b2)

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.

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: 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/base.py (1)

1843-1877: Rename‑type strategy mutates types without cross‑module filtering, breaking imports

The rename_type collision strategy in __change_field_name unconditionally mutates source.class_name when a field name collides with a referenced type's short name, but it does not check whether the colliding type lives in the same module as the field.

Risk: When a field in module A collides with a type from module B:

  • Imports for module A are generated before __change_field_name runs
  • The rename-type logic then mutates the type's class name in module B to add a _ suffix
  • Module A's imports reference the old name, but module B now defines the suffixed name
  • This causes NameError at runtime

The codebase already uses source.module_path != model.module_path checks in __apply_discriminator_type for exactly this purpose, but __change_field_name omits this guard.

Additional concern: Appending "_" without validation against existing types can silently create duplicate class names if the schema already defines a type with that suffix.

Recommended fix:

  • Guard the colliding_reference assignment with a module-path check: only set it when source.module_path == model.module_path
  • For cross-module collisions, fall back to field renaming (the else branch), which doesn't mutate types
  • Consider using ModelResolver to generate unique names instead of hard-coding the suffix

Tests needed:

  • Cross-module collision scenario: type and field in different modules with --field-type-collision-strategy rename-type
  • Existing _-suffixed type: verify no silent name collisions when schema already defines Fruit_
🧹 Nitpick comments (6)
docs/cli-reference/field-customization.md (1)

15-15: Update table description to match improved documentation.

The table entry uses the same test-focused language. After updating the main documentation section, this should be updated to match.

Suggested description for the table:

| [`--field-type-collision-strategy`](#field-type-collision-strategy) | Configure how to resolve naming conflicts between field names and generated type names. |
src/datamodel_code_generator/__main__.py (1)

474-474: Remove unused noqa directive.

Static analysis correctly identifies that the UP045 code in the noqa comment is not needed, as it refers to a rule that isn't being violated here.

🔎 Proposed fix
-    field_type_collision_strategy: Optional[FieldTypeCollisionStrategy] = None  # noqa: UP045
+    field_type_collision_strategy: Optional[FieldTypeCollisionStrategy] = None
docs/cli-reference/index.md (1)

77-77: Update description after fixing field-customization.md.

This entry links to the field-customization page and should be updated once the documentation there is improved (as noted in the review comment for field-customization.md).

docs/cli-reference/quick-reference.md (1)

59-59: Update descriptions after fixing field-customization.md.

Both entries (table at line 59 and alphabetical index at line 208) use the test-focused description "Test field-type collision with rename-type strategy." These should be updated to match the improved user-facing documentation suggested in the review comment for field-customization.md.

Also applies to: 208-208

src/datamodel_code_generator/arguments.py (1)

598-605: Keep help text and core default for collision strategy in sync

The help string states that 'rename-field' is the default strategy, while the CLI argument itself defaults to None (with the actual default presumably applied in core logic). That’s fine, but it does create a small coupling: if the core’s default ever changes, this help text will also need updating to avoid drift.

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

3575-3612: Rename‑type field/type collision tests are well‑targeted

Both tests correctly exercise --field-type-collision-strategy rename-type for single‑ and multi‑model JSON Schema inputs against the new golden files, matching the intended behavior (renaming the enum type while preserving the field name). The cli_doc metadata also looks consistent; just ensure that golden_output="field_type_collision_rename_type.py" matches the path conventions your CLI docs generator expects.

📜 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 0906812 and a223a37.

📒 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_type_collision_rename_type.py
  • tests/data/expected/main/jsonschema/field_type_collision_rename_type_multi_model.py
  • tests/data/jsonschema/field_type_collision_rename_type.json
  • tests/data/jsonschema/field_type_collision_rename_type_multi_model.json
  • tests/main/jsonschema/test_main_jsonschema.py
🧰 Additional context used
🧬 Code graph analysis (7)
src/datamodel_code_generator/parser/graphql.py (1)
src/datamodel_code_generator/__init__.py (1)
  • FieldTypeCollisionStrategy (312-320)
src/datamodel_code_generator/parser/jsonschema.py (1)
src/datamodel_code_generator/__init__.py (1)
  • FieldTypeCollisionStrategy (312-320)
tests/data/expected/main/jsonschema/field_type_collision_rename_type.py (2)
src/datamodel_code_generator/model/enum.py (1)
  • Enum (39-121)
tests/data/expected/main/jsonschema/field_type_collision_rename_type_multi_model.py (1)
  • Fruit_ (12-14)
tests/data/expected/main/jsonschema/field_type_collision_rename_type_multi_model.py (2)
src/datamodel_code_generator/model/enum.py (1)
  • Enum (39-121)
tests/data/expected/main/jsonschema/field_type_collision_rename_type.py (1)
  • Fruit_ (12-14)
src/datamodel_code_generator/parser/openapi.py (1)
src/datamodel_code_generator/__init__.py (1)
  • FieldTypeCollisionStrategy (312-320)
src/datamodel_code_generator/__main__.py (1)
src/datamodel_code_generator/__init__.py (1)
  • FieldTypeCollisionStrategy (312-320)
src/datamodel_code_generator/arguments.py (1)
src/datamodel_code_generator/__init__.py (1)
  • FieldTypeCollisionStrategy (312-320)
🪛 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). (12)
  • GitHub Check: py312-black22 on Ubuntu
  • GitHub Check: 3.10 on Ubuntu
  • GitHub Check: 3.11 on Windows
  • GitHub Check: 3.14 on Windows
  • GitHub Check: py312-isort7 on Ubuntu
  • GitHub Check: 3.12 on Windows
  • GitHub Check: py312-pydantic1 on Ubuntu
  • GitHub Check: py312-black24 on Ubuntu
  • GitHub Check: 3.13 on Windows
  • GitHub Check: 3.10 on Windows
  • GitHub Check: Analyze (python)
  • GitHub Check: benchmarks
🔇 Additional comments (22)
tests/data/jsonschema/field_type_collision_rename_type.json (1)

1-12: LGTM! Well-designed test case.

This JSON schema effectively tests the field-type collision scenario by creating a property named "Fruit" with enum values that would generate a type also named "Fruit", triggering the collision resolution logic.

src/datamodel_code_generator/cli_options.py (1)

137-139: LGTM! Consistent with existing option metadata.

The CLI option metadata entry follows the established pattern and is correctly categorized under OptionCategory.FIELD.

src/datamodel_code_generator/__main__.py (2)

31-31: LGTM! Import added correctly.

The FieldTypeCollisionStrategy import is properly added alongside other similar enums and configuration types.


781-781: LGTM! Parameter correctly threaded through to generation.

The field_type_collision_strategy is properly passed from the config to the generate() function, following the same pattern as other configuration options.

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

1-27: LGTM! Comprehensive multi-model test case.

This schema effectively tests the collision resolution strategy across multiple models (Parent and Child) that both reference the same enum definition, ensuring the rename-type strategy works correctly when the same type is shared.

docs/cli-reference/index.md (1)

13-13: LGTM! Count correctly updated.

The Field Customization option count is properly incremented from 20 to 21 to reflect the new --field-type-collision-strategy option.

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

12-18: LGTM! Expected output correctly demonstrates rename-type strategy.

The generated code correctly shows:

  • The enum type renamed to Fruit_ (with underscore suffix) to avoid collision
  • The field keeps its original name Fruit (no alias needed)

This demonstrates that the rename-type strategy successfully resolves the naming conflict by renaming the generated type rather than the field, which aligns with the PR's objective to address issue #2364.

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

1-23: Expected model layout matches rename‑type semantics

Enum Fruit_ reused by both Child.Fruit and Parent.Fruit with optional Fruit_ | None = None matches the intended rename‑type strategy and is consistent with existing fixtures. No changes needed.

src/datamodel_code_generator/arguments.py (1)

17-30: CLI wiring for --field-type-collision-strategy looks correct

Importing FieldTypeCollisionStrategy and using choices=[s.value for s in FieldTypeCollisionStrategy] integrates the new option cleanly into the existing field customization group while remaining backward compatible (default=None). No functional issues spotted.

Also applies to: 598-605

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

20-35: OpenAPI parser correctly threads field/type collision strategy to base class

Importing FieldTypeCollisionStrategy, adding field_type_collision_strategy as the last __init__ parameter, and forwarding it via super().__init__(..., field_type_collision_strategy=field_type_collision_strategy) cleanly exposes the new configuration without breaking existing call sites.

Also applies to: 181-282, 284-379

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

16-28: GraphQL parser API stays backward compatible while exposing new strategy

The new field_type_collision_strategy parameter is appended at the end of GraphQLParser.__init__ and forwarded to the base Parser, matching the pattern used in other parsers. This provides consistent configurability without impacting existing usages.

Also applies to: 98-197, 199-295

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

24-37: Importing FieldTypeCollisionStrategy here is appropriate

Wiring the enum in at the parser layer via this import matches its usage in the base Parser; no issues from a dependency or layering perspective.


512-609: JSONSchemaParser: new field_type_collision_strategy arg is correctly threaded

The new parameter is optional, at the end of the signature, and passed through to Parser.__init__, so it’s backwards compatible for existing callers and keeps behavior unchanged when left as None.

Please double‑check that every other Parser subclass (OpenAPIParser, GraphQLParser, etc.) also accepts and forwards field_type_collision_strategy so that generate(..., field_type_collision_strategy=...) never hits an unexpected‑keyword error.


612-707: Super call propagation of field_type_collision_strategy looks correct

Passing field_type_collision_strategy=field_type_collision_strategy into super().__init__ is consistent with the base signature and is the only wiring needed here; no additional logic is required in this class.

src/datamodel_code_generator/__init__.py (4)

312-321: FieldTypeCollisionStrategy enum definition is clear and well‑scoped

The enum values ("rename-field", "rename-type") and docstring clearly document the behavior; exposing this at the top level is appropriate for both CLI and library users.


393-500: generate() parameter addition is API‑compatible

Adding field_type_collision_strategy: FieldTypeCollisionStrategy | None = None at the end of generate()’s parameter list preserves all existing call patterns and keeps default behavior unchanged when None.

If any external integrations call generate() with **kwargs built dynamically, ensure they either tolerate or explicitly set this new key when running against older versions for backwards‑compat.


646-745: Correctly propagating field_type_collision_strategy into parser construction

Passing field_type_collision_strategy=field_type_collision_strategy into parser_class(...) ensures the strategy flows through to all parsers without impacting users who don’t opt in.


882-898: Re‑exporting FieldTypeCollisionStrategy via __all__ is appropriate

Adding FieldTypeCollisionStrategy to __all__ makes the new enum part of the public API surface, which aligns with its use in CLI and library code.

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

25-35: Base parser import of FieldTypeCollisionStrategy is fine

Importing FieldTypeCollisionStrategy alongside the other configuration enums keeps configuration concerns centralized in this module.


677-775: Parser.init: new strategy parameter is wired but unused beyond storage

Accepting field_type_collision_strategy here and storing it on self is the right place for parser‑wide configuration; there’s no extra logic required at construction time.


929-930: Storing field_type_collision_strategy on the instance is appropriate

This keeps the strategy available for downstream methods like __change_field_name without affecting existing behavior when left as None.


2727-2730: Ordering: field/type renaming after imports is subtle

__change_field_name(models) is invoked after __change_from_import(...) within _process_single_module. This ordering is fine for the existing field‑rename behavior, but it’s exactly why the rename‑type strategy can desync type names from already‑computed imports in cross‑module scenarios (see earlier comment on __change_field_name).

If you keep rename‑type, please ensure either:

  • Type renames happen before import computation, or
  • They’re constrained to same‑module types so imports remain valid.

Comment thread docs/cli-reference/field-customization.md
@koxudaxi koxudaxi merged commit c416d2c into main Dec 22, 2025
38 checks passed
@koxudaxi koxudaxi deleted the feature/field-type-collision-strategy branch December 22, 2025 12:14
@koxudaxi koxudaxi restored the feature/field-type-collision-strategy branch December 22, 2025 12:14
koxudaxi added a commit that referenced this pull request Dec 22, 2025
koxudaxi added a commit that referenced this pull request Dec 22, 2025
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