Skip to content

Conversation

MintsInc
Copy link
Member

@MintsInc MintsInc commented Oct 8, 2025

Context

When we have a oneOf containing two complex types with the same base type (List<String> and List<Double> for example), it causes compilation errors in Java:

  1. Constructor collision: Multiple constructors with the same erasure signature (e.g., both List<String> and List<Double> erase to List)
  2. Getter collision: Multiple getter methods with the same name (e.g., getList() for both types)

This issue will surface in PR #3057 which introduces CustomAttributeValuesUnion with oneOf: String, List<String>, Double, List<Double>.

Changes

Implemented collision detection and resolution for oneOf types with the same type erasure. The logic has been placed in the Python generator (rather than the Jinja2 template) for better separation of concerns and maintainability.

  1. Added prepare_oneof_methods() function in formatter.py:

    • Pre-computes method information for all oneOf types
    • Detects type erasure collisions using a two-pass approach:
      • First pass: Count occurrences of each unparameterized type
      • Second pass: Generate appropriate method names based on collision detection
    • Returns structured data containing method names and type information
  2. Updated modelOneOf.j2 template:

    • Calls the new prepare_oneof_methods() function
    • Iterates over pre-computed data to generate methods
    • Keeps template logic simple and focused on presentation
  3. Method generation strategy:

    • No collision (e.g., String, Double): Generate regular constructors + simple getters (getString(), getDouble())
    • Collision detected (e.g., List<String>, List<Double>): Generate static factory methods + descriptive getters
      • Factory methods: fromStringList(List<String>), fromDoubleList(List<Double>)
      • Getters: getStringList(), getDoubleList()

Test

I've tested that on the changes from the problematic PR and I got this result:

// Regular constructors (no collision)
public CustomAttributeValuesUnion(String o) { ... }
public CustomAttributeValuesUnion(Double o) { ... }

// Factory methods (collision detected)
public static CustomAttributeValuesUnion fromStringList(List<String> o) { ... }
public static CustomAttributeValuesUnion fromDoubleList(List<Double> o) { ... }

// Getters with unique names
public String getString() { ... }
public List<String> getStringList() { ... }
public Double getDouble() { ... }
public List<Double> getDoubleList() { ... }

Copy link
Member Author

MintsInc commented Oct 8, 2025

This stack of pull requests is managed by Graphite. Learn more about stacking.

@MintsInc MintsInc marked this pull request as ready for review October 8, 2025 12:19
@MintsInc MintsInc requested review from a team as code owners October 8, 2025 12:19
@MintsInc MintsInc added the changelog/no-changelog Changes don't appear in changelog label Oct 8, 2025
@MintsInc MintsInc force-pushed the ulysse.mavrocordatos/fix-oneof-methods-name branch from 8ce0b15 to 4552755 Compare October 8, 2025 12:22
@amaskara-dd
Copy link
Contributor

amaskara-dd commented Oct 8, 2025

This is a lot of logic to place in the template — would it be possible to move more of this into the generator itself? perhaps in the formatter

@MintsInc MintsInc force-pushed the ulysse.mavrocordatos/fix-oneof-methods-name branch from 4552755 to 2004f15 Compare October 9, 2025 09:04
When there is a oneof with at least two similar type, ie List<string> and List<int> this cause some conflict.
@MintsInc MintsInc force-pushed the ulysse.mavrocordatos/fix-oneof-methods-name branch from 2004f15 to a3383fb Compare October 10, 2025 07:47
@MintsInc MintsInc merged commit d9ac7ea into master Oct 10, 2025
14 checks passed
@MintsInc MintsInc deleted the ulysse.mavrocordatos/fix-oneof-methods-name branch October 10, 2025 08:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changelog/no-changelog Changes don't appear in changelog
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants