Repository: openapi-generators/openapi-python-client (language: python)

Issue:
Fix: explicitly track first if-branch usage in union template
Needs tests 😅 

Fixes #603 
Client generation error when primitive types precede object types in a required oneOf / anyOf union
The first clause of the `to_dict` method uses `elif` instead of `if` when a model's required property is a `oneOf` or `anyOf` union with primitive-type members ordered before object-type members, resulting in a syntax error.

In the mean time, a workaround may be to sort primitive-type members below object-type members as long as each group's members has their ordering maintained among themselves.

Source of issue:
https://github.com/openapi-generators/openapi-python-client/blob/e1a8b24138e550073ef73517e79f05dde7a48454/openapi_python_client/templates/property_templates/union_property.py.jinja#L69-L72

Resulting error:
```
$ openapi-python-client generate --path schema-required-union-ordering.yaml --fail-on-warning
Generating schema-required-union-ordering-client
Error(s) encountered while generating, client was not created

black failed

reformatted schema_required_union_ordering_client/api/__init__.py
reformatted schema_required_union_ordering_client/__init__.py
error: cannot format schema_required_union_ordering_client/models/get_response_200.py: Cannot parse: 23:8:         elif isinstance(self.a, GetResponse200AType1):
reformatted schema_required_union_ordering_client/types.py
reformatted schema_required_union_ordering_client/models/get_response_200a_type_1.py
reformatted schema_required_union_ordering_client/client.py
reformatted schema_required_union_ordering_client/api/default/get.py

Oh no! 💥 💔 💥
6 files reformatted, 2 files left unchanged, 1 file failed to reformat.
```

Using an example `schema-required-union-ordering.yaml` file:
```yaml
openapi: 3.0.2
info:
  title: schema-required-union-ordering
  version: 1.0.0

paths:
  /:
    get:
      description: Test
      responses:
        '200':
          description: Test
          content:
            application/json:
              schema:
                type: object
                required:
                  - a
                properties:
                  a:
                    oneOf: # same error for `anyOf`
                    - type: string
                    - type: object
```

The problematic `schema_required_union_ordering_client/models/get_response_200.py` file:

```python
from typing import Any, BinaryIO, Dict, List, Optional, TextIO, Tuple, Type, TypeVar, Union, cast

import attr

from ..models.get_response_200a_type_1 import GetResponse200AType1
from ..types import UNSET, Unset

T = TypeVar("T", bound="GetResponse200")

@attr.s(auto_attribs=True)
class GetResponse200:
    """
    Attributes:
        a (Union[GetResponse200AType1, str]):
    """

    a: Union[GetResponse200AType1, str]
    additional_properties: Dict[str, Any] = attr.ib(init=False, factory=dict)


    def to_dict(self) -> Dict[str, Any]:
        
        elif isinstance(self.a, GetResponse200AType1):
            a = self.a.to_dict()

        else:
            a = self.a



        field_dict: Dict[str, Any] = {}
        field_dict.update(self.additional_properties)
        field_dict.update({
            "a": a,
        })

        return field_dict



    @classmethod
    def from_dict(cls: Type[T], src_dict: Dict[str, Any]) -> T:
        d = src_dict.copy()
        def _parse_a(data: object) -> Union[GetResponse200AType1, str]:
            try:
                if not isinstance(data, dict):
                    raise TypeError()
                a_type_1 = GetResponse200AType1.from_dict(data)



                return a_type_1
            except: # noqa: E722
                pass
            return cast(Union[GetResponse200AType1, str], data)

        a = _parse_a(d.pop("a"))


        get_response_200 = cls(
            a=a,
        )

        get_response_200.additional_properties = d
        return get_response_200

    @property
    def additional_keys(self) -> List[str]:
        return list(self.additional_properties.keys())

    def __getitem__(self, key: str) -> Any:
        return self.additional_properties[key]

    def __setitem__(self, key: str, value: Any) -> None:
        self.additional_properties[key] = value

    def __delitem__(self, key: str) -> None:
        del self.additional_properties[key]

    def __contains__(self, key: str) -> bool:
        return key in self.additional_properties

```

**Versions**
- Python: `3.7.12`
- openapi-python-client: `0.11.1`



Discussion / hints:

We hit this as well, and are stuck on 0.9.2 as a result. All versions newer than 0.9.2 exhibit this behavior. A fix would be great!

Fix patch:
diff --git a/openapi_python_client/templates/model.py.jinja b/openapi_python_client/templates/model.py.jinja
index 07f929d66..dc033c41a 100644
--- a/openapi_python_client/templates/model.py.jinja
+++ b/openapi_python_client/templates/model.py.jinja
@@ -85,7 +85,7 @@ field_dict: Dict[str, Any] = {}
 {% endif %}
 {% if prop_template and prop_template.transform %}
 for prop_name, prop in self.additional_properties.items():
-    {{ prop_template.transform(model.additional_properties, "prop", "field_dict[prop_name]", multipart=multipart) | indent(4) }}
+    {{ prop_template.transform(model.additional_properties, "prop", "field_dict[prop_name]", multipart=multipart, declare_type=false) | indent(4) }}
 {% elif multipart %}
 field_dict.update({
     key: (None, str(value).encode(), "text/plain")
diff --git a/openapi_python_client/templates/property_templates/union_property.py.jinja b/openapi_python_client/templates/property_templates/union_property.py.jinja
index 8a7d506d6..039709369 100644
--- a/openapi_python_client/templates/property_templates/union_property.py.jinja
+++ b/openapi_python_client/templates/property_templates/union_property.py.jinja
@@ -40,24 +40,24 @@ def _parse_{{ property.python_name }}(data: object) -> {{ property.get_type_stri
 {% endmacro %}
 
 {% macro transform(property, source, destination, declare_type=True, multipart=False) %}
-{% if not property.required or property.nullable %}
-{{ destination }}{% if declare_type %}: {{ property.get_type_string(json=True) }}{% endif %}
+{% 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) }}{% endif %}
 
 {% if not property.required %}
 if isinstance({{ source }}, Unset):
     {{ destination }} = UNSET
-{% endif %}
+    {% set ns.has_if = true %}
 {% endif %}
 {% if property.nullable %}
-    {% if property.required %}
-if {{ source }} is None:
-    {% else %}{# There's an if UNSET statement before this #}
+    {% if ns.has_if %}
 elif {{ source }} is None:
+    {% else %}
+if {{ source }} is None:
+        {% set ns.has_if = true %}
     {% endif %}
     {{ destination }} = None
 {% endif %}
 
-{% set ns = namespace(contains_properties_without_transform = false, contains_modified_properties = not property.required) %}
 {% for inner_property in property.inner_properties %}
     {% import "property_templates/" + inner_property.template as inner_template %}
     {% if not inner_template.transform %}
@@ -66,10 +66,12 @@ elif {{ source }} is None:
     {% else %}
         {% set ns.contains_modified_properties = true %}
     {% endif %}
-    {% if loop.first and property.required and not property.nullable %}{# No if UNSET or if None statement before this #}
+    {% if not ns.has_if %}
 if isinstance({{ source }}, {{ inner_property.get_instance_type_string() }}):
+        {% set ns.has_if = true %}
     {% elif not loop.last or ns.contains_properties_without_transform %}
 elif isinstance({{ source }}, {{ inner_property.get_instance_type_string() }}):
+    {% set has_first_clause = true %}
     {% else %}
 else:
     {% endif %}


Test patch:
diff --git a/end_to_end_tests/custom-templates-golden-record/my_test_api_client/api/__init__.py b/end_to_end_tests/custom-templates-golden-record/my_test_api_client/api/__init__.py
index 3295d210c..cd12c2407 100644
--- a/end_to_end_tests/custom-templates-golden-record/my_test_api_client/api/__init__.py
+++ b/end_to_end_tests/custom-templates-golden-record/my_test_api_client/api/__init__.py
@@ -5,6 +5,7 @@
 from .default import DefaultEndpoints
 from .location import LocationEndpoints
 from .parameters import ParametersEndpoints
+from .responses import ResponsesEndpoints
 from .tag1 import Tag1Endpoints
 from .tests import TestsEndpoints
 from .true_ import True_Endpoints
@@ -15,6 +16,10 @@ class MyTestApiClientApi:
     def tests(cls) -> Type[TestsEndpoints]:
         return TestsEndpoints
 
+    @classmethod
+    def responses(cls) -> Type[ResponsesEndpoints]:
+        return ResponsesEndpoints
+
     @classmethod
     def default(cls) -> Type[DefaultEndpoints]:
         return DefaultEndpoints
diff --git a/end_to_end_tests/custom-templates-golden-record/my_test_api_client/api/responses/__init__.py b/end_to_end_tests/custom-templates-golden-record/my_test_api_client/api/responses/__init__.py
new file mode 100644
index 000000000..c2e39c16a
--- /dev/null
+++ b/end_to_end_tests/custom-templates-golden-record/my_test_api_client/api/responses/__init__.py
@@ -0,0 +1,14 @@
+""" Contains methods for accessing the API Endpoints """
+
+import types
+
+from . import post_responses_unions_simple_before_complex
+
+
+class ResponsesEndpoints:
+    @classmethod
+    def post_responses_unions_simple_before_complex(cls) -> types.ModuleType:
+        """
+        Regression test for #603
+        """
+        return post_responses_unions_simple_before_complex
diff --git a/end_to_end_tests/golden-record/my_test_api_client/api/responses/__init__.py b/end_to_end_tests/golden-record/my_test_api_client/api/responses/__init__.py
new file mode 100644
index 000000000..e69de29bb
diff --git a/end_to_end_tests/golden-record/my_test_api_client/api/responses/post_responses_unions_simple_before_complex.py b/end_to_end_tests/golden-record/my_test_api_client/api/responses/post_responses_unions_simple_before_complex.py
new file mode 100644
index 000000000..9dd058470
--- /dev/null
+++ b/end_to_end_tests/golden-record/my_test_api_client/api/responses/post_responses_unions_simple_before_complex.py
@@ -0,0 +1,118 @@
+from typing import Any, Dict, Optional
+
+import httpx
+
+from ...client import Client
+from ...models.post_responses_unions_simple_before_complex_response_200 import (
+    PostResponsesUnionsSimpleBeforeComplexResponse200,
+)
+from ...types import Response
+
+
+def _get_kwargs(
+    *,
+    client: Client,
+) -> Dict[str, Any]:
+    url = "{}/responses/unions/simple_before_complex".format(client.base_url)
+
+    headers: Dict[str, str] = client.get_headers()
+    cookies: Dict[str, Any] = client.get_cookies()
+
+    return {
+        "method": "post",
+        "url": url,
+        "headers": headers,
+        "cookies": cookies,
+        "timeout": client.get_timeout(),
+    }
+
+
+def _parse_response(*, response: httpx.Response) -> Optional[PostResponsesUnionsSimpleBeforeComplexResponse200]:
+    if response.status_code == 200:
+        response_200 = PostResponsesUnionsSimpleBeforeComplexResponse200.from_dict(response.json())
+
+        return response_200
+    return None
+
+
+def _build_response(*, response: httpx.Response) -> Response[PostResponsesUnionsSimpleBeforeComplexResponse200]:
+    return Response(
+        status_code=response.status_code,
+        content=response.content,
+        headers=response.headers,
+        parsed=_parse_response(response=response),
+    )
+
+
+def sync_detailed(
+    *,
+    client: Client,
+) -> Response[PostResponsesUnionsSimpleBeforeComplexResponse200]:
+    """Regression test for #603
+
+    Returns:
+        Response[PostResponsesUnionsSimpleBeforeComplexResponse200]
+    """
+
+    kwargs = _get_kwargs(
+        client=client,
+    )
+
+    response = httpx.request(
+        verify=client.verify_ssl,
+        **kwargs,
+    )
+
+    return _build_response(response=response)
+
+
+def sync(
+    *,
+    client: Client,
+) -> Optional[PostResponsesUnionsSimpleBeforeComplexResponse200]:
+    """Regression test for #603
+
+    Returns:
+        Response[PostResponsesUnionsSimpleBeforeComplexResponse200]
+    """
+
+    return sync_detailed(
+        client=client,
+    ).parsed
+
+
+async def asyncio_detailed(
+    *,
+    client: Client,
+) -> Response[PostResponsesUnionsSimpleBeforeComplexResponse200]:
+    """Regression test for #603
+
+    Returns:
+        Response[PostResponsesUnionsSimpleBeforeComplexResponse200]
+    """
+
+    kwargs = _get_kwargs(
+        client=client,
+    )
+
+    async with httpx.AsyncClient(verify=client.verify_ssl) as _client:
+        response = await _client.request(**kwargs)
+
+    return _build_response(response=response)
+
+
+async def asyncio(
+    *,
+    client: Client,
+) -> Optional[PostResponsesUnionsSimpleBeforeComplexResponse200]:
+    """Regression test for #603
+
+    Returns:
+        Response[PostResponsesUnionsSimpleBeforeComplexResponse200]
+    """
+
+    return (
+        await asyncio_detailed(
+            client=client,
+        )
+    ).parsed
diff --git a/end_to_end_tests/golden-record/my_test_api_client/api/tests/defaults_tests_defaults_post.py b/end_to_end_tests/golden-record/my_test_api_client/api/tests/defaults_tests_defaults_post.py
index 6bcd59c8c..6c30b939d 100644
--- a/end_to_end_tests/golden-record/my_test_api_client/api/tests/defaults_tests_defaults_post.py
+++ b/end_to_end_tests/golden-record/my_test_api_client/api/tests/defaults_tests_defaults_post.py
@@ -51,6 +51,8 @@ def _get_kwargs(
 
     params["list_prop"] = json_list_prop
 
+    json_union_prop: Union[float, str]
+
     json_union_prop = union_prop
 
     params["union_prop"] = json_union_prop
diff --git a/end_to_end_tests/golden-record/my_test_api_client/api/tests/get_user_list.py b/end_to_end_tests/golden-record/my_test_api_client/api/tests/get_user_list.py
index 29dd706e8..e4ed22231 100644
--- a/end_to_end_tests/golden-record/my_test_api_client/api/tests/get_user_list.py
+++ b/end_to_end_tests/golden-record/my_test_api_client/api/tests/get_user_list.py
@@ -47,6 +47,8 @@ def _get_kwargs(
 
     params["an_enum_value_with_only_null"] = json_an_enum_value_with_only_null
 
+    json_some_date: str
+
     if isinstance(some_date, datetime.date):
         json_some_date = some_date.isoformat()
     else:
diff --git a/end_to_end_tests/golden-record/my_test_api_client/models/__init__.py b/end_to_end_tests/golden-record/my_test_api_client/models/__init__.py
index 85e5243ec..f34a171f6 100644
--- a/end_to_end_tests/golden-record/my_test_api_client/models/__init__.py
+++ b/end_to_end_tests/golden-record/my_test_api_client/models/__init__.py
@@ -40,6 +40,10 @@
 from .model_with_union_property_inlined_fruit_type_0 import ModelWithUnionPropertyInlinedFruitType0
 from .model_with_union_property_inlined_fruit_type_1 import ModelWithUnionPropertyInlinedFruitType1
 from .none import None_
+from .post_responses_unions_simple_before_complex_response_200 import PostResponsesUnionsSimpleBeforeComplexResponse200
+from .post_responses_unions_simple_before_complex_response_200a_type_1 import (
+    PostResponsesUnionsSimpleBeforeComplexResponse200AType1,
+)
 from .test_inline_objects_json_body import TestInlineObjectsJsonBody
 from .test_inline_objects_response_200 import TestInlineObjectsResponse200
 from .validation_error import ValidationError
diff --git a/end_to_end_tests/golden-record/my_test_api_client/models/a_model.py b/end_to_end_tests/golden-record/my_test_api_client/models/a_model.py
index d52001229..0cf302e56 100644
--- a/end_to_end_tests/golden-record/my_test_api_client/models/a_model.py
+++ b/end_to_end_tests/golden-record/my_test_api_client/models/a_model.py
@@ -71,6 +71,8 @@ def to_dict(self) -> Dict[str, Any]:
 
         an_allof_enum_with_overridden_default = self.an_allof_enum_with_overridden_default.value
 
+        a_camel_date_time: str
+
         if isinstance(self.a_camel_date_time, datetime.datetime):
             a_camel_date_time = self.a_camel_date_time.isoformat()
 
@@ -79,6 +81,7 @@ def to_dict(self) -> Dict[str, Any]:
 
         a_date = self.a_date.isoformat()
         required_not_nullable = self.required_not_nullable
+        one_of_models: Union[Any, Dict[str, Any]]
 
         if isinstance(self.one_of_models, FreeFormModel):
             one_of_models = self.one_of_models.to_dict()
diff --git a/end_to_end_tests/golden-record/my_test_api_client/models/post_responses_unions_simple_before_complex_response_200.py b/end_to_end_tests/golden-record/my_test_api_client/models/post_responses_unions_simple_before_complex_response_200.py
new file mode 100644
index 000000000..6ce78fcd5
--- /dev/null
+++ b/end_to_end_tests/golden-record/my_test_api_client/models/post_responses_unions_simple_before_complex_response_200.py
@@ -0,0 +1,79 @@
+from typing import Any, Dict, List, Type, TypeVar, Union, cast
+
+import attr
+
+from ..models.post_responses_unions_simple_before_complex_response_200a_type_1 import (
+    PostResponsesUnionsSimpleBeforeComplexResponse200AType1,
+)
+
+T = TypeVar("T", bound="PostResponsesUnionsSimpleBeforeComplexResponse200")
+
+
+@attr.s(auto_attribs=True)
+class PostResponsesUnionsSimpleBeforeComplexResponse200:
+    """
+    Attributes:
+        a (Union[PostResponsesUnionsSimpleBeforeComplexResponse200AType1, str]):
+    """
+
+    a: Union[PostResponsesUnionsSimpleBeforeComplexResponse200AType1, str]
+    additional_properties: Dict[str, Any] = attr.ib(init=False, factory=dict)
+
+    def to_dict(self) -> Dict[str, Any]:
+        a: Union[Dict[str, Any], str]
+
+        if isinstance(self.a, PostResponsesUnionsSimpleBeforeComplexResponse200AType1):
+            a = self.a.to_dict()
+
+        else:
+            a = self.a
+
+        field_dict: Dict[str, Any] = {}
+        field_dict.update(self.additional_properties)
+        field_dict.update(
+            {
+                "a": a,
+            }
+        )
+
+        return field_dict
+
+    @classmethod
+    def from_dict(cls: Type[T], src_dict: Dict[str, Any]) -> T:
+        d = src_dict.copy()
+
+        def _parse_a(data: object) -> Union[PostResponsesUnionsSimpleBeforeComplexResponse200AType1, str]:
+            try:
+                if not isinstance(data, dict):
+                    raise TypeError()
+                a_type_1 = PostResponsesUnionsSimpleBeforeComplexResponse200AType1.from_dict(data)
+
+                return a_type_1
+            except:  # noqa: E722
+                pass
+            return cast(Union[PostResponsesUnionsSimpleBeforeComplexResponse200AType1, str], data)
+
+        a = _parse_a(d.pop("a"))
+
+        post_responses_unions_simple_before_complex_response_200 = cls(
+            a=a,
+        )
+
+        post_responses_unions_simple_before_complex_response_200.additional_properties = d
+        return post_responses_unions_simple_before_complex_response_200
+
+    @property
+    def additional_keys(self) -> List[str]:
+        return list(self.additional_properties.keys())
+
+    def __getitem__(self, key: str) -> Any:
+        return self.additional_properties[key]
+
+    def __setitem__(self, key: str, value: Any) -> None:
+        self.additional_properties[key] = value
+
+    def __delitem__(self, key: str) -> None:
+        del self.additional_properties[key]
+
+    def __contains__(self, key: str) -> bool:
+        return key in self.additional_properties
diff --git a/end_to_end_tests/golden-record/my_test_api_client/models/post_responses_unions_simple_before_complex_response_200a_type_1.py b/end_to_end_tests/golden-record/my_test_api_client/models/post_responses_unions_simple_before_complex_response_200a_type_1.py
new file mode 100644
index 000000000..6b9ae2484
--- /dev/null
+++ b/end_to_end_tests/golden-record/my_test_api_client/models/post_responses_unions_simple_before_complex_response_200a_type_1.py
@@ -0,0 +1,44 @@
+from typing import Any, Dict, List, Type, TypeVar
+
+import attr
+
+T = TypeVar("T", bound="PostResponsesUnionsSimpleBeforeComplexResponse200AType1")
+
+
+@attr.s(auto_attribs=True)
+class PostResponsesUnionsSimpleBeforeComplexResponse200AType1:
+    """ """
+
+    additional_properties: Dict[str, Any] = attr.ib(init=False, factory=dict)
+
+    def to_dict(self) -> Dict[str, Any]:
+
+        field_dict: Dict[str, Any] = {}
+        field_dict.update(self.additional_properties)
+        field_dict.update({})
+
+        return field_dict
+
+    @classmethod
+    def from_dict(cls: Type[T], src_dict: Dict[str, Any]) -> T:
+        d = src_dict.copy()
+        post_responses_unions_simple_before_complex_response_200a_type_1 = cls()
+
+        post_responses_unions_simple_before_complex_response_200a_type_1.additional_properties = d
+        return post_responses_unions_simple_before_complex_response_200a_type_1
+
+    @property
+    def additional_keys(self) -> List[str]:
+        return list(self.additional_properties.keys())
+
+    def __getitem__(self, key: str) -> Any:
+        return self.additional_properties[key]
+
+    def __setitem__(self, key: str, value: Any) -> None:
+        self.additional_properties[key] = value
+
+    def __delitem__(self, key: str) -> None:
+        del self.additional_properties[key]
+
+    def __contains__(self, key: str) -> bool:
+        return key in self.additional_properties
diff --git a/end_to_end_tests/openapi.json b/end_to_end_tests/openapi.json
index b958e530b..dc74e033c 100644
--- a/end_to_end_tests/openapi.json
+++ b/end_to_end_tests/openapi.json
@@ -713,6 +713,33 @@
         }
       }
     },
+    "/responses/unions/simple_before_complex": {
+      "post": {
+        "tags": ["responses"],
+        "description": "Regression test for #603",
+        "responses": {
+          "200": {
+            "description": "A union with simple types before complex ones.",
+            "content": {
+              "application/json": {
+                "schema": {
+                  "type": "object",
+                  "required": ["a"],
+                  "properties": {
+                    "a": {
+                      "oneOf": [
+                        {"type": "string"},
+                        {"type": "object"}
+                      ]
+                    }
+                  }
+                }
+              }
+            }
+          }
+        }
+      }
+    },
     "/auth/token_with_cookie": {
       "get": {
         "tags": [