fix: skip redundant isinstance dispatch for homogeneous model unions

When a property is a union of schemas that are all object/model types
(e.g. `oneOf: [ModelA, ModelB]`), `union_property.py.jinja`'s `transform`
macro generated a per-member `isinstance` dispatch in `to_dict()`, even
though every branch does the exact same thing (`.to_dict()`). The check
never affects the outcome, since every generated model class converts
identically.

Detect when every union member is model-shaped and skip the per-member
dispatch:

- required: unconditional `dest = source.to_dict()`, no isinstance at all
- optional: `else: dest = source.to_dict()` after the existing Unset guard
- optional request bodies (`skip_unset=True`, no Unset guard emitted
  yet): guard with `if not isinstance(source, Unset)` instead of the
  previous single-member `isinstance(source, ModelX)` check

Mixed unions (model + primitive/enum/None/Any members) are unaffected
and keep their existing per-member dispatch, since those branches
genuinely differ in behavior.

Discussion: #1486

Author: AndersBennedsgaard

end_to_end_tests/golden-records/my-test-api-client/my_test_api_client/api/default/post_types_unions_duplicate_types.py

@@ -20,7 +20,7 @@ def _get_kwargs(
         "url": "/types/unions/duplicate-types",
     }
 
-    if isinstance(body, AModel):
+    if not isinstance(body, Unset):
         _kwargs["json"] = body.to_dict()
 
     headers["Content-Type"] = "application/json"

end_to_end_tests/golden-records/my-test-api-client/my_test_api_client/models/a_model.py

@@ -179,8 +179,6 @@ def to_dict(self) -> dict[str, Any]:
         not_required_one_of_models: dict[str, Any] | Unset
         if isinstance(self.not_required_one_of_models, Unset):
             not_required_one_of_models = UNSET
-        elif isinstance(self.not_required_one_of_models, FreeFormModel):
-            not_required_one_of_models = self.not_required_one_of_models.to_dict()
         else:
             not_required_one_of_models = self.not_required_one_of_models.to_dict()
 

end_to_end_tests/golden-records/my-test-api-client/my_test_api_client/models/extended.py

@@ -182,8 +182,6 @@ def to_dict(self) -> dict[str, Any]:
         not_required_one_of_models: dict[str, Any] | Unset
         if isinstance(self.not_required_one_of_models, Unset):
             not_required_one_of_models = UNSET
-        elif isinstance(self.not_required_one_of_models, FreeFormModel):
-            not_required_one_of_models = self.not_required_one_of_models.to_dict()
         else:
             not_required_one_of_models = self.not_required_one_of_models.to_dict()
 

end_to_end_tests/golden-records/my-test-api-client/my_test_api_client/models/model_with_union_property_inlined.py

@@ -25,15 +25,9 @@ class ModelWithUnionPropertyInlined:
     fruit: ModelWithUnionPropertyInlinedApples | ModelWithUnionPropertyInlinedBananas | Unset = UNSET
 
     def to_dict(self) -> dict[str, Any]:
-        from ..models.model_with_union_property_inlined_apples import (
-            ModelWithUnionPropertyInlinedApples,  # noqa: PLC0415
-        )
-
         fruit: dict[str, Any] | Unset
         if isinstance(self.fruit, Unset):
             fruit = UNSET
-        elif isinstance(self.fruit, ModelWithUnionPropertyInlinedApples):
-            fruit = self.fruit.to_dict()
         else:
             fruit = self.fruit.to_dict()
 

openapi_python_client/templates/property_templates/union_property.py.jinja

@@ -40,6 +40,8 @@ def _parse_{{ property.python_name }}(data: object) -> {{ property.get_type_stri
 {% endmacro %}
 
 {% macro transform(property, source, destination, declare_type=True, skip_unset=False) %}
+{% set inner_templates = property.inner_properties | map(attribute="template") | unique | list %}
+{% set homogeneous_model_union = property.inner_properties and inner_templates == ["model_property.py.jinja"] %}
 {% set ns = namespace(contains_properties_without_transform = false, contains_modified_properties = not property.required, has_if = false) %}
 {% if declare_type %}{{ destination }}: {{ property.get_type_string(json=True) | as_unembedded_code }}{% endif %}
 
@@ -48,6 +50,17 @@ if isinstance({{ source }}, Unset):
     {{ destination }} = UNSET
     {% set ns.has_if = true %}
 {% endif %}
+{% if homogeneous_model_union %}
+{% if property.required %}
+{{ destination }} = {{ source }}.to_dict()
+{% elif ns.has_if %}
+else:
+    {{ destination }} = {{ source }}.to_dict()
+{% else %}
+if not isinstance({{ source }}, Unset):
+    {{ destination }} = {{ source }}.to_dict()
+{% endif %}
+{% else %}
 {% for inner_property in property.inner_properties %}
     {% import "property_templates/" + inner_property.template as inner_template %}
     {% if not inner_template.transform %}
@@ -72,6 +85,7 @@ else:
 {%- elif ns.contains_properties_without_transform %}
 {{ destination }} = {{ source }}
 {%- endif %}
+{% endif %}
 {% endmacro %}
 
 

tests/test_templates/test_property_templates/test_union_property/test_union_property.py

@@ -0,0 +1,92 @@
+"""Tests for union_property.py.jinja's `transform` macro.
+
+See issue: redundant `isinstance` dispatch when every union member is a model.
+"""
+
+import pytest
+from jinja2 import Environment, PackageLoader
+
+from openapi_python_client import TEMPLATE_FILTERS
+
+
+@pytest.fixture
+def jinja_env() -> Environment:
+    """A jinja2 environment matching the one used for real code generation.
+
+    Needs the `loopcontrols` extension (for `{% continue %}`) which the shared `env` fixture in
+    `tests/test_templates/conftest.py` does not enable.
+    """
+    env = Environment(
+        loader=PackageLoader("openapi_python_client"),
+        trim_blocks=True,
+        lstrip_blocks=True,
+        extensions=["jinja2.ext.loopcontrols"],
+    )
+    env.filters.update(TEMPLATE_FILTERS)
+    return env
+
+
+def test_transform_homogeneous_model_union_skips_isinstance_dispatch(
+    jinja_env: Environment, union_property_factory, model_property_factory
+) -> None:
+    """When every union member is a model, `.to_dict()` is called unconditionally (no per-member isinstance
+    dispatch), since every branch's body is identical regardless of which member actually matched.
+    """
+    model_a = model_property_factory(name="model_a", required=True)
+    model_b = model_property_factory(name="model_b", required=True)
+    union = union_property_factory(name="both_models", required=False, inner_properties=[model_a, model_b])
+
+    template = jinja_env.get_template("property_templates/union_property.py.jinja")
+    result = template.module.transform(union, "self.both_models", "both_models")
+
+    assert "isinstance(self.both_models, Unset)" in result
+    assert "isinstance(self.both_models, MyClass)" not in result
+    assert result.count(".to_dict()") == 1
+
+
+def test_transform_mixed_union_keeps_isinstance_dispatch(
+    jinja_env: Environment, union_property_factory, model_property_factory, string_property_factory
+) -> None:
+    """Mixed unions (model + non-model members) must keep the existing per-member isinstance dispatch,
+    since those branches genuinely differ in behavior.
+    """
+    model_a = model_property_factory(name="model_a", required=True)
+    string_b = string_property_factory(name="string_b", required=True)
+    union = union_property_factory(name="mixed", required=False, inner_properties=[model_a, string_b])
+
+    template = jinja_env.get_template("property_templates/union_property.py.jinja")
+    result = template.module.transform(union, "self.mixed", "mixed")
+
+    assert "isinstance(self.mixed, MyClass)" in result
+    assert result.count(".to_dict()") == 1
+
+
+def test_transform_required_homogeneous_model_union_has_no_guard(
+    jinja_env: Environment, union_property_factory, model_property_factory
+) -> None:
+    """A required homogeneous-model union can never be Unset, so `.to_dict()` is called with no guard at all."""
+    model_a = model_property_factory(name="model_a", required=True)
+    model_b = model_property_factory(name="model_b", required=True)
+    union = union_property_factory(name="both_models", required=True, inner_properties=[model_a, model_b])
+
+    template = jinja_env.get_template("property_templates/union_property.py.jinja")
+    result = template.module.transform(union, "self.both_models", "both_models")
+
+    assert "isinstance" not in result
+    assert result.strip() == "both_models: dict[str, Any]\nboth_models = self.both_models.to_dict()"
+
+
+def test_transform_skip_unset_homogeneous_model_union_still_guards_unset(
+    jinja_env: Environment, union_property_factory, model_property_factory
+) -> None:
+    """Request bodies render `transform` with `skip_unset=True` (the `UNSET` default is assigned elsewhere), but an
+    optional homogeneous-model union can still *be* `Unset` at runtime, so `.to_dict()` must still be guarded --
+    unconditionally calling it here would raise `AttributeError`/fail type-checking (`Unset` has no `to_dict`).
+    """
+    model_a = model_property_factory(name="model_a", required=True)
+    union = union_property_factory(name="body", required=False, inner_properties=[model_a])
+
+    template = jinja_env.get_template("property_templates/union_property.py.jinja")
+    result = template.module.transform(union, "body", '_kwargs["json"]', skip_unset=True, declare_type=False)
+
+    assert result.strip() == 'if not isinstance(body, Unset):\n    _kwargs["json"] = body.to_dict()'