Skip to content

Conversation

@ChiragAgg5k
Copy link
Member

@ChiragAgg5k ChiragAgg5k commented Nov 7, 2025

Screenshot 2025-11-07 at 2 26 53 PM

Summary by CodeRabbit

  • Bug Fixes

    • Namespaced generated enum types with their collection names across all languages to prevent enum name collisions between collections.
  • Improvements

    • Type generation now scopes enum and related types to the parent collection, improving clarity and correctness of generated models and serialization code.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 7, 2025

Walkthrough

This pull request updates type-generation templates for C#, Dart, Java, Kotlin, PHP, Swift, and TypeScript to thread collection context into type resolution. The getType signature in each language template now accepts a collectionName parameter and all getType call sites pass collection.name. String-like ENUM types are renamed to include the collection name as a prefix (e.g., CollectionNameEnumName), and enum references across declarations, property types, constructors, mapping/parsing, and (de)serialization logic are updated accordingly. Additional changes include C# error handling for missing related collections and PHP enum case value formatting changed to upper-snake-case.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

  • Broad but consistent changes across seven language templates applying the same pattern.
  • Multiple touch points per file (enum declarations, property/parameter types, mapping/parsing/serialization code) require verification.

Areas requiring extra attention:

  • C#: new error handling paths for missing related collections and enum parsing changes.
  • PHP: enum case value formatting changed to upper-snake-case; verify enum case generation and usage.
  • Consistency of getType(attribute, collections, collection.name) call sites across all templates and correct propagation of the collection name into enum/type names.
  • All mapping/deserialization and serialization code paths (From/ToMap, fromJson/toJson equivalents) to ensure enum type references use the new collection-prefixed names.

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately reflects the main change: prefixing enum type names with collection/table names across all language templates to avoid naming conflicts.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix-prefix-table-name

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between c9100c2 and 123771d.

📒 Files selected for processing (1)
  • templates/cli/lib/type-generation/languages/csharp.js.twig (5 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • templates/cli/lib/type-generation/languages/csharp.js.twig
⏰ 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). (19)
  • GitHub Check: build (8.3, Go112)
  • GitHub Check: build (8.3, Python39)
  • GitHub Check: build (8.3, Python311)
  • GitHub Check: build (8.3, KotlinJava8)
  • GitHub Check: build (8.3, Swift56)
  • GitHub Check: build (8.3, PHP83)
  • GitHub Check: build (8.3, Node16)
  • GitHub Check: build (8.3, Python310)
  • GitHub Check: build (8.3, CLINode18)
  • GitHub Check: build (8.3, FlutterBeta)
  • GitHub Check: build (8.3, Android14Java17)
  • GitHub Check: build (8.3, FlutterStable)
  • GitHub Check: build (8.3, DotNet90)
  • GitHub Check: build (8.3, CLINode16)
  • GitHub Check: swift (server)
  • GitHub Check: build (8.3, Android5Java17)
  • GitHub Check: apple (client)
  • GitHub Check: web (client)
  • GitHub Check: android (client)

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.

Copy link
Contributor

@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

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between cd71267 and c9100c2.

📒 Files selected for processing (7)
  • templates/cli/lib/type-generation/languages/csharp.js.twig (4 hunks)
  • templates/cli/lib/type-generation/languages/dart.js.twig (4 hunks)
  • templates/cli/lib/type-generation/languages/java.js.twig (4 hunks)
  • templates/cli/lib/type-generation/languages/kotlin.js.twig (3 hunks)
  • templates/cli/lib/type-generation/languages/php.js.twig (4 hunks)
  • templates/cli/lib/type-generation/languages/swift.js.twig (6 hunks)
  • templates/cli/lib/type-generation/languages/typescript.js.twig (4 hunks)
⏰ 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). (20)
  • GitHub Check: build (8.3, Ruby31)
  • GitHub Check: build (8.3, AppleSwift56)
  • GitHub Check: build (8.3, WebChromium)
  • GitHub Check: build (8.3, Python311)
  • GitHub Check: dotnet (server)
  • GitHub Check: build (8.3, Python310)
  • GitHub Check: build (8.3, Ruby27)
  • GitHub Check: swift (server)
  • GitHub Check: kotlin (server)
  • GitHub Check: build (8.3, PHP80)
  • GitHub Check: react-native (client)
  • GitHub Check: android (client)
  • GitHub Check: web (client)
  • GitHub Check: build (8.3, FlutterStable)
  • GitHub Check: build (8.3, FlutterBeta)
  • GitHub Check: apple (client)
  • GitHub Check: build (8.3, Android5Java17)
  • GitHub Check: build (8.3, CLINode18)
  • GitHub Check: build (8.3, Android14Java17)
  • GitHub Check: build (8.3, DartStable)
🔇 Additional comments (15)
templates/cli/lib/type-generation/languages/kotlin.js.twig (1)

6-75: Enum prefix propagation looks clean. The extra collectionName parameter flows through to each call site, so Kotlin enums and property types now emit collection-scoped identifiers without leaving any stragglers.

templates/cli/lib/type-generation/languages/swift.js.twig (1)

6-147: Swift template updates are consistent. Prefixed enum identifiers feed through getType, the enum declaration, and both decode/from-map paths, so collisions across collections are avoided.

templates/cli/lib/type-generation/languages/typescript.js.twig (1)

9-95: TypeScript typing is scoped correctly. Passing collection.name into getType ensures both enum declarations and property types share the new collection-prefixed identifier, solving the previous collision issue.

templates/cli/lib/type-generation/languages/csharp.js.twig (1)

6-99: C# enum handling aligns with the new scheme. The prefixed names flow from getType through property signatures and Enum.Parse, so generated models won’t clash across collections anymore.

templates/cli/lib/type-generation/languages/dart.js.twig (1)

43-177: Dart enum renaming is thorough. Prefixed enums are emitted and consumed consistently in fields, constructors, and map conversions, so name clashes across collections are resolved end-to-end.

templates/cli/lib/type-generation/languages/java.js.twig (5)

6-15: LGTM! Enum type generation now includes collection context.

The method signature correctly adds the collectionName parameter, and enum types are properly prefixed with the collection name. This change prevents naming conflicts when multiple collections have attributes with the same enum name.

Note: This is a breaking change. Existing generated code that references enum types by their attribute name alone will need to be updated to use the new prefixed names (e.g., AttributeNameCollectionNameAttributeName).


64-68: LGTM! Enum declaration consistent with type generation.

The enum declaration correctly includes the collection name prefix, matching the type generation logic in the getType method.


73-73: LGTM! Field declarations updated correctly.

The private field declarations correctly pass collection.name to the getType method.


79-87: LGTM! Constructor parameters updated correctly.

Constructor parameter types correctly use getType(attribute, collections, collection.name), ensuring consistency with field declarations.


90-96: LGTM! Getter and setter methods updated correctly.

Both getter return types and setter parameter types correctly pass collection.name to the getType method, ensuring type consistency across the generated class.

templates/cli/lib/type-generation/languages/php.js.twig (5)

6-19: LGTM! Enum type generation now includes collection context.

The method signature correctly adds the collectionName parameter, and enum types are properly prefixed with the collection name, consistent with the Java implementation.

Note: This is a breaking change. Existing generated code that references enum types by their attribute name alone will need to be updated to use the new prefixed names.


63-68: LGTM! Enum declaration consistent with type generation.

The enum declaration correctly includes the collection name prefix and uses PHP 8.1+ backed enum syntax with appropriate case naming conventions.


73-73: LGTM! Property declarations updated correctly.

The private property declarations correctly pass collection.name to the getType method.


76-88: LGTM! Constructor parameters updated correctly.

Constructor parameter types correctly use getType(attribute, collections, collection.name) with appropriate handling of nullable types for PHP's parameter declaration syntax.


91-97: LGTM! Getter and setter methods updated correctly.

Both getter return types and setter parameter types correctly pass collection.name to the getType method, ensuring type consistency across the generated class.

@ChiragAgg5k ChiragAgg5k merged commit 39ee7e0 into master Nov 10, 2025
52 checks passed
@ChiragAgg5k ChiragAgg5k deleted the fix-prefix-table-name branch November 10, 2025 04:32
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.

3 participants