Skip to content

fix: Handle class name prefix correctly in GraphQL parser#2926

Merged
koxudaxi merged 4 commits intokoxudaxi:mainfrom
siminn-arnorgj:fix-graphql-classprefix
Jan 5, 2026
Merged

fix: Handle class name prefix correctly in GraphQL parser#2926
koxudaxi merged 4 commits intokoxudaxi:mainfrom
siminn-arnorgj:fix-graphql-classprefix

Conversation

@siminn-arnorgj
Copy link
Copy Markdown
Contributor

@siminn-arnorgj siminn-arnorgj commented Jan 5, 2026

Summary

  • When looking up the python type from extra_template_data, use the original name of the reference.
  • Use the correct name when parsing unions.
  • Add regression tests for both issues.

Fixes: #2923 #2925

Summary by CodeRabbit

  • New Features

    • Added --class-name-prefix option for GraphQL code generation, allowing customizable prefixes on generated Python classes and scalar types.
  • Bug Fixes

    • Improved union type field naming resolution in GraphQL schema parsing.
    • Enhanced scalar type name resolution for consistent reference handling.

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

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Jan 5, 2026

📝 Walkthrough

Walkthrough

This PR fixes a bug where GraphQL scalar type mappings fail when using the --class-name-prefix option. The issue stems from incorrect reference name lookups during scalar type resolution and union field name derivation. The fix updates these lookups to use original_name from references instead of the prefixed names.

Changes

Cohort / File(s) Summary
Core Logic Fixes
src/datamodel_code_generator/model/scalar.py, src/datamodel_code_generator/parser/graphql.py
Updated scalar type lookup to use reference.original_name instead of scalar_name and reference.name. In GraphQL union parsing, field names now resolve via self.references[type_.name].name to use prefixed reference names correctly.
GraphQL Test Expected Outputs
tests/data/expected/main/graphql/simple_star_wars_class_name_prefix.py, tests/data/expected/parser/graphql/union_with_prefix.py
New expected output files demonstrating correct GraphQL code generation with prefixed class names (e.g., FooString, FooInt, FooBoolean), including scalar aliases and Pydantic models with proper typename field handling.
Test Cases
tests/main/graphql/test_main_graphql.py, tests/parser/test_graphql.py
Added test functions test_main_graphql_class_name_prefix() and test_graphql_union_with_prefix() to exercise the --class-name-prefix option with GraphQL inputs and verify correct output generation.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Suggested reviewers

  • koxudaxi

Poem

🐰 With prefixes adorned, the types now align,
No more do the scalars break by design,
original_name guides the way,
GraphQL shines bright today! ✨

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 'fix: Handle class name prefix correctly in GraphQL parser' accurately summarizes the main change, which addresses the bug where class name prefixes broke GraphQL type mappings.
Linked Issues check ✅ Passed The PR successfully implements fixes for both linked issues: using original reference names for type mapping lookup [#2923] and correctly naming union fields via existing references [#2925], with comprehensive test coverage added.
Out of Scope Changes check ✅ Passed All changes are directly related to fixing the class name prefix handling in GraphQL parser and scalar type mappings, with no unrelated modifications present.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
✨ Finishing touches
  • 📝 Generate docstrings

📜 Recent 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 58e73ed and ff003e7.

📒 Files selected for processing (6)
  • src/datamodel_code_generator/model/scalar.py
  • src/datamodel_code_generator/parser/graphql.py
  • tests/data/expected/main/graphql/simple_star_wars_class_name_prefix.py
  • tests/data/expected/parser/graphql/union_with_prefix.py
  • tests/main/graphql/test_main_graphql.py
  • tests/parser/test_graphql.py
🧰 Additional context used
🧬 Code graph analysis (5)
tests/data/expected/parser/graphql/union_with_prefix.py (1)
src/datamodel_code_generator/model/type_alias.py (1)
  • TypeAlias (37-42)
src/datamodel_code_generator/model/scalar.py (2)
src/datamodel_code_generator/reference.py (2)
  • reference (77-79)
  • get (1019-1021)
src/datamodel_code_generator/__main__.py (1)
  • get (152-154)
src/datamodel_code_generator/parser/graphql.py (2)
src/datamodel_code_generator/model/base.py (1)
  • name (839-841)
src/datamodel_code_generator/parser/base.py (1)
  • data_type (1078-1080)
tests/data/expected/main/graphql/simple_star_wars_class_name_prefix.py (3)
src/datamodel_code_generator/model/type_alias.py (1)
  • TypeAlias (37-42)
tests/data/expected/parser/openapi/openapi_parser_parse_modular/bar.py (1)
  • Field (6-7)
src/datamodel_code_generator/model/base.py (1)
  • name (839-841)
tests/main/graphql/test_main_graphql.py (1)
tests/main/conftest.py (2)
  • output_file (99-101)
  • run_main_and_assert (245-409)
⏰ 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.10 on Windows
  • GitHub Check: 3.12 on Windows
  • GitHub Check: 3.10 on macOS
  • GitHub Check: 3.11 on Windows
  • GitHub Check: 3.14 on Windows
  • GitHub Check: 3.13 on Windows
  • GitHub Check: 3.12 on macOS
  • GitHub Check: Analyze (python)
  • GitHub Check: benchmarks
🔇 Additional comments (6)
src/datamodel_code_generator/model/scalar.py (1)

70-74: LGTM! Correct fix for scalar type mapping with class name prefixes.

The change correctly uses reference.original_name for lookups in extra_template_data and DEFAULT_GRAPHQL_SCALAR_TYPES while maintaining the prefixed scalar_name for the output key. This ensures that custom and default scalar mappings (e.g., Boolean → bool) are resolved using the original GraphQL type name, not the prefixed class name.

tests/parser/test_graphql.py (1)

58-67: LGTM! Good regression test for union with class name prefix.

The test properly validates that unions correctly reference prefixed class names when using --class-name-prefix. It reuses the existing union.graphql input file and compares against a new expected output file, following the established test patterns in this file.

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

480-485: LGTM! Correct fix for union member field names with class name prefix.

The change correctly resolves union member field names through self.references[type_.name].name, ensuring that when --class-name-prefix is used, union type aliases reference the prefixed class names (e.g., FooCar instead of Car). This is safe because unions are parsed last (per parse_order), guaranteeing all member types are already registered in self.references.

tests/main/graphql/test_main_graphql.py (1)

765-777: LGTM! Good regression test for class name prefix with GraphQL.

The test validates the end-to-end flow of GraphQL code generation with --class-name-prefix, ensuring scalar type mappings and model class names are correctly prefixed. This directly addresses the issue #2923 regression.

tests/data/expected/parser/graphql/union_with_prefix.py (1)

1-59: LGTM! Expected output correctly validates both bug fixes.

The file demonstrates:

  1. Scalar mappings are correct: FooBoolean: TypeAlias = bool, FooInt: TypeAlias = int, etc. — confirming the original_name lookup fix works.
  2. Union references are correct: FooResource: TypeAlias = Union['FooCar', 'FooEmployee'] — confirming union member names use the prefixed class names.

This expected output comprehensively validates the fixes for both issues #2923 (scalar mappings) and #2925 (union names).

tests/data/expected/main/graphql/simple_star_wars_class_name_prefix.py (1)

1-158: LGTM! Comprehensive expected output for class name prefix validation.

The file correctly demonstrates:

  1. Scalar type aliases with correct Python type mappings: FooBoolean → bool, FooID → str, FooInt → int, FooString → str
  2. Prefixed model classes with proper field type references using the prefixed scalar and model names
  3. Forward reference resolution via update_forward_refs() for circular model dependencies

This validates the complete end-to-end fix for issue #2923.


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.

@codspeed-hq
Copy link
Copy Markdown

codspeed-hq Bot commented Jan 5, 2026

CodSpeed Performance Report

Merging #2926 will not alter performance

Comparing siminn-arnorgj:fix-graphql-classprefix (ff003e7) with main (58e73ed)

⚠️ Unknown Walltime execution environment detected

Using the Walltime instrument on standard Hosted Runners will lead to inconsistent data.

For the most accurate results, we recommend using CodSpeed Macro Runners: bare-metal machines fine-tuned for performance measurement consistency.

Summary

✅ 11 untouched
⏩ 98 skipped1

Footnotes

  1. 98 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.

@codecov
Copy link
Copy Markdown

codecov Bot commented Jan 5, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 100.00%. Comparing base (58e73ed) to head (ff003e7).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff            @@
##              main     #2926   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           93        93           
  Lines        16913     16917    +4     
  Branches      1966      1966           
=========================================
+ Hits         16913     16917    +4     
Flag Coverage Δ
unittests 100.00% <100.00%> (ø)

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.

@koxudaxi koxudaxi merged commit 54c3ed9 into koxudaxi:main Jan 5, 2026
38 checks passed
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Jan 5, 2026

🎉 Released in 0.52.2

This PR is now available in the latest release. See the release notes for details.

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.

GraphQL type mappings break when using class prefixes

2 participants