rework union type logic to preserve original type name for class when possible

Author: eli-bl

.changeset/nullable_enum_fix.md

@@ -0,0 +1,7 @@
+---
+default: patch
+---
+
+# Fix class generation for some union types
+
+Fixed issue #1120, where certain combinations of types-- such as a `oneOf` between a model or an enum and null, or the OpenAPI 3.0 equivalent of using `nullable: true`-- could cause unnecessary suffixes like "Type0" to be added to the class name, and/or could cause extra copies of the class to be generated.

.changeset/union_fixes.md

@@ -53,7 +53,7 @@
 )
 from .model_with_additional_properties_refed import ModelWithAdditionalPropertiesRefed
 from .model_with_any_json_properties import ModelWithAnyJsonProperties
-from .model_with_any_json_properties_additional_property_type_0 import ModelWithAnyJsonPropertiesAdditionalPropertyType0
+from .model_with_any_json_properties_additional_property import ModelWithAnyJsonPropertiesAdditionalProperty
 from .model_with_backslash_in_description import ModelWithBackslashInDescription
 from .model_with_circular_ref_a import ModelWithCircularRefA
 from .model_with_circular_ref_b import ModelWithCircularRefB
@@ -81,8 +81,8 @@
 from .post_naming_property_conflict_with_import_body import PostNamingPropertyConflictWithImportBody
 from .post_naming_property_conflict_with_import_response_200 import PostNamingPropertyConflictWithImportResponse200
 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 .post_responses_unions_simple_before_complex_response_200a import (
+    PostResponsesUnionsSimpleBeforeComplexResponse200A,
 )
 from .test_inline_objects_body import TestInlineObjectsBody
 from .test_inline_objects_response_200 import TestInlineObjectsResponse200
@@ -134,7 +134,7 @@
     "ModelWithAdditionalPropertiesInlinedAdditionalProperty",
     "ModelWithAdditionalPropertiesRefed",
     "ModelWithAnyJsonProperties",
-    "ModelWithAnyJsonPropertiesAdditionalPropertyType0",
+    "ModelWithAnyJsonPropertiesAdditionalProperty",
     "ModelWithBackslashInDescription",
     "ModelWithCircularRefA",
     "ModelWithCircularRefB",
@@ -162,7 +162,7 @@
     "PostNamingPropertyConflictWithImportBody",
     "PostNamingPropertyConflictWithImportResponse200",
     "PostResponsesUnionsSimpleBeforeComplexResponse200",
-    "PostResponsesUnionsSimpleBeforeComplexResponse200AType1",
+    "PostResponsesUnionsSimpleBeforeComplexResponse200A",
     "TestInlineObjectsBody",
     "TestInlineObjectsResponse200",
     "ValidationError",

end_to_end_tests/golden-record/my_test_api_client/models/__init__.py

@@ -353,9 +353,9 @@ def _parse_nullable_model(data: object) -> Union["ModelWithUnionProperty", None]
             try:
                 if not isinstance(data, dict):
                     raise TypeError()
-                nullable_model_type_1 = ModelWithUnionProperty.from_dict(data)
+                nullable_model = ModelWithUnionProperty.from_dict(data)
 
-                return nullable_model_type_1
+                return nullable_model
             except:  # noqa: E722
                 pass
             return cast(Union["ModelWithUnionProperty", None], data)
@@ -498,9 +498,9 @@ def _parse_not_required_nullable_model(data: object) -> Union["ModelWithUnionPro
             try:
                 if not isinstance(data, dict):
                     raise TypeError()
-                not_required_nullable_model_type_1 = ModelWithUnionProperty.from_dict(data)
+                not_required_nullable_model = ModelWithUnionProperty.from_dict(data)
 
-                return not_required_nullable_model_type_1
+                return not_required_nullable_model
             except:  # noqa: E722
                 pass
             return cast(Union["ModelWithUnionProperty", None, Unset], data)

end_to_end_tests/golden-record/my_test_api_client/models/a_model.py

@@ -361,9 +361,9 @@ def _parse_nullable_model(data: object) -> Union["ModelWithUnionProperty", None]
             try:
                 if not isinstance(data, dict):
                     raise TypeError()
-                nullable_model_type_1 = ModelWithUnionProperty.from_dict(data)
+                nullable_model = ModelWithUnionProperty.from_dict(data)
 
-                return nullable_model_type_1
+                return nullable_model
             except:  # noqa: E722
                 pass
             return cast(Union["ModelWithUnionProperty", None], data)
@@ -506,9 +506,9 @@ def _parse_not_required_nullable_model(data: object) -> Union["ModelWithUnionPro
             try:
                 if not isinstance(data, dict):
                     raise TypeError()
-                not_required_nullable_model_type_1 = ModelWithUnionProperty.from_dict(data)
+                not_required_nullable_model = ModelWithUnionProperty.from_dict(data)
 
-                return not_required_nullable_model_type_1
+                return not_required_nullable_model
             except:  # noqa: E722
                 pass
             return cast(Union["ModelWithUnionProperty", None, Unset], data)

end_to_end_tests/golden-record/my_test_api_client/models/extended.py

@@ -4,9 +4,7 @@
 from attrs import field as _attrs_field
 
 if TYPE_CHECKING:
-    from ..models.model_with_any_json_properties_additional_property_type_0 import (
-        ModelWithAnyJsonPropertiesAdditionalPropertyType0,
-    )
+    from ..models.model_with_any_json_properties_additional_property import ModelWithAnyJsonPropertiesAdditionalProperty
 
 
 T = TypeVar("T", bound="ModelWithAnyJsonProperties")
@@ -17,17 +15,17 @@ class ModelWithAnyJsonProperties:
     """ """
 
     additional_properties: Dict[
-        str, Union["ModelWithAnyJsonPropertiesAdditionalPropertyType0", List[str], bool, float, int, str]
+        str, Union["ModelWithAnyJsonPropertiesAdditionalProperty", List[str], bool, float, int, str]
     ] = _attrs_field(init=False, factory=dict)
 
     def to_dict(self) -> Dict[str, Any]:
-        from ..models.model_with_any_json_properties_additional_property_type_0 import (
-            ModelWithAnyJsonPropertiesAdditionalPropertyType0,
+        from ..models.model_with_any_json_properties_additional_property import (
+            ModelWithAnyJsonPropertiesAdditionalProperty,
         )
 
         field_dict: Dict[str, Any] = {}
         for prop_name, prop in self.additional_properties.items():
-            if isinstance(prop, ModelWithAnyJsonPropertiesAdditionalPropertyType0):
+            if isinstance(prop, ModelWithAnyJsonPropertiesAdditionalProperty):
                 field_dict[prop_name] = prop.to_dict()
             elif isinstance(prop, list):
                 field_dict[prop_name] = prop
@@ -39,8 +37,8 @@ def to_dict(self) -> Dict[str, Any]:
 
     @classmethod
     def from_dict(cls: Type[T], src_dict: Dict[str, Any]) -> T:
-        from ..models.model_with_any_json_properties_additional_property_type_0 import (
-            ModelWithAnyJsonPropertiesAdditionalPropertyType0,
+        from ..models.model_with_any_json_properties_additional_property import (
+            ModelWithAnyJsonPropertiesAdditionalProperty,
         )
 
         d = src_dict.copy()
@@ -51,13 +49,13 @@ def from_dict(cls: Type[T], src_dict: Dict[str, Any]) -> T:
 
             def _parse_additional_property(
                 data: object,
-            ) -> Union["ModelWithAnyJsonPropertiesAdditionalPropertyType0", List[str], bool, float, int, str]:
+            ) -> Union["ModelWithAnyJsonPropertiesAdditionalProperty", List[str], bool, float, int, str]:
                 try:
                     if not isinstance(data, dict):
                         raise TypeError()
-                    additional_property_type_0 = ModelWithAnyJsonPropertiesAdditionalPropertyType0.from_dict(data)
+                    additional_property = ModelWithAnyJsonPropertiesAdditionalProperty.from_dict(data)
 
-                    return additional_property_type_0
+                    return additional_property
                 except:  # noqa: E722
                     pass
                 try:
@@ -69,7 +67,7 @@ def _parse_additional_property(
                 except:  # noqa: E722
                     pass
                 return cast(
-                    Union["ModelWithAnyJsonPropertiesAdditionalPropertyType0", List[str], bool, float, int, str], data
+                    Union["ModelWithAnyJsonPropertiesAdditionalProperty", List[str], bool, float, int, str], data
                 )
 
             additional_property = _parse_additional_property(prop_dict)
@@ -85,13 +83,11 @@ def additional_keys(self) -> List[str]:
 
     def __getitem__(
         self, key: str
-    ) -> Union["ModelWithAnyJsonPropertiesAdditionalPropertyType0", List[str], bool, float, int, str]:
+    ) -> Union["ModelWithAnyJsonPropertiesAdditionalProperty", List[str], bool, float, int, str]:
         return self.additional_properties[key]
 
     def __setitem__(
-        self,
-        key: str,
-        value: Union["ModelWithAnyJsonPropertiesAdditionalPropertyType0", List[str], bool, float, int, str],
+        self, key: str, value: Union["ModelWithAnyJsonPropertiesAdditionalProperty", List[str], bool, float, int, str]
     ) -> None:
         self.additional_properties[key] = value
 

end_to_end_tests/golden-record/my_test_api_client/models/model_with_any_json_properties.py

@@ -3,11 +3,11 @@
 from attrs import define as _attrs_define
 from attrs import field as _attrs_field
 
-T = TypeVar("T", bound="ModelWithAnyJsonPropertiesAdditionalPropertyType0")
+T = TypeVar("T", bound="ModelWithAnyJsonPropertiesAdditionalProperty")
 
 
 @_attrs_define
-class ModelWithAnyJsonPropertiesAdditionalPropertyType0:
+class ModelWithAnyJsonPropertiesAdditionalProperty:
     """ """
 
     additional_properties: Dict[str, str] = _attrs_field(init=False, factory=dict)
@@ -21,10 +21,10 @@ def to_dict(self) -> Dict[str, Any]:
     @classmethod
     def from_dict(cls: Type[T], src_dict: Dict[str, Any]) -> T:
         d = src_dict.copy()
-        model_with_any_json_properties_additional_property_type_0 = cls()
+        model_with_any_json_properties_additional_property = cls()
 
-        model_with_any_json_properties_additional_property_type_0.additional_properties = d
-        return model_with_any_json_properties_additional_property_type_0
+        model_with_any_json_properties_additional_property.additional_properties = d
+        return model_with_any_json_properties_additional_property
 
     @property
     def additional_keys(self) -> List[str]:

end_to_end_tests/golden-record/my_test_api_client/models/model_with_any_json_properties_additional_property_type_0.py renamed to end_to_end_tests/golden-record/my_test_api_client/models/model_with_any_json_properties_additional_property.py

@@ -4,8 +4,8 @@
 from attrs import field as _attrs_field
 
 if TYPE_CHECKING:
-    from ..models.post_responses_unions_simple_before_complex_response_200a_type_1 import (
-        PostResponsesUnionsSimpleBeforeComplexResponse200AType1,
+    from ..models.post_responses_unions_simple_before_complex_response_200a import (
+        PostResponsesUnionsSimpleBeforeComplexResponse200A,
     )
 
 
@@ -16,19 +16,19 @@
 class PostResponsesUnionsSimpleBeforeComplexResponse200:
     """
     Attributes:
-        a (Union['PostResponsesUnionsSimpleBeforeComplexResponse200AType1', str]):
+        a (Union['PostResponsesUnionsSimpleBeforeComplexResponse200A', str]):
     """
 
-    a: Union["PostResponsesUnionsSimpleBeforeComplexResponse200AType1", str]
+    a: Union["PostResponsesUnionsSimpleBeforeComplexResponse200A", str]
     additional_properties: Dict[str, Any] = _attrs_field(init=False, factory=dict)
 
     def to_dict(self) -> Dict[str, Any]:
-        from ..models.post_responses_unions_simple_before_complex_response_200a_type_1 import (
-            PostResponsesUnionsSimpleBeforeComplexResponse200AType1,
+        from ..models.post_responses_unions_simple_before_complex_response_200a import (
+            PostResponsesUnionsSimpleBeforeComplexResponse200A,
         )
 
         a: Union[Dict[str, Any], str]
-        if isinstance(self.a, PostResponsesUnionsSimpleBeforeComplexResponse200AType1):
+        if isinstance(self.a, PostResponsesUnionsSimpleBeforeComplexResponse200A):
             a = self.a.to_dict()
         else:
             a = self.a
@@ -45,22 +45,22 @@ def to_dict(self) -> Dict[str, Any]:
 
     @classmethod
     def from_dict(cls: Type[T], src_dict: Dict[str, Any]) -> T:
-        from ..models.post_responses_unions_simple_before_complex_response_200a_type_1 import (
-            PostResponsesUnionsSimpleBeforeComplexResponse200AType1,
+        from ..models.post_responses_unions_simple_before_complex_response_200a import (
+            PostResponsesUnionsSimpleBeforeComplexResponse200A,
         )
 
         d = src_dict.copy()
 
-        def _parse_a(data: object) -> Union["PostResponsesUnionsSimpleBeforeComplexResponse200AType1", str]:
+        def _parse_a(data: object) -> Union["PostResponsesUnionsSimpleBeforeComplexResponse200A", str]:
             try:
                 if not isinstance(data, dict):
                     raise TypeError()
-                a_type_1 = PostResponsesUnionsSimpleBeforeComplexResponse200AType1.from_dict(data)
+                a = PostResponsesUnionsSimpleBeforeComplexResponse200A.from_dict(data)
 
-                return a_type_1
+                return a
             except:  # noqa: E722
                 pass
-            return cast(Union["PostResponsesUnionsSimpleBeforeComplexResponse200AType1", str], data)
+            return cast(Union["PostResponsesUnionsSimpleBeforeComplexResponse200A", str], data)
 
         a = _parse_a(d.pop("a"))
 

end_to_end_tests/golden-record/my_test_api_client/models/post_responses_unions_simple_before_complex_response_200.py

@@ -3,11 +3,11 @@
 from attrs import define as _attrs_define
 from attrs import field as _attrs_field
 
-T = TypeVar("T", bound="PostResponsesUnionsSimpleBeforeComplexResponse200AType1")
+T = TypeVar("T", bound="PostResponsesUnionsSimpleBeforeComplexResponse200A")
 
 
 @_attrs_define
-class PostResponsesUnionsSimpleBeforeComplexResponse200AType1:
+class PostResponsesUnionsSimpleBeforeComplexResponse200A:
     """ """
 
     additional_properties: Dict[str, Any] = _attrs_field(init=False, factory=dict)
@@ -21,10 +21,10 @@ def to_dict(self) -> Dict[str, Any]:
     @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 = 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
+        post_responses_unions_simple_before_complex_response_200a.additional_properties = d
+        return post_responses_unions_simple_before_complex_response_200a
 
     @property
     def additional_keys(self) -> List[str]:

end_to_end_tests/golden-record/my_test_api_client/models/post_responses_unions_simple_before_complex_response_200a_type_1.py renamed to end_to_end_tests/golden-record/my_test_api_client/models/post_responses_unions_simple_before_complex_response_200a.py

@@ -1,5 +1,8 @@
 from __future__ import annotations
 
+from openapi_python_client.parser.properties.has_named_class import HasNamedClass
+from openapi_python_client.schema.data_type import DataType
+
 __all__ = ["EnumProperty", "ValueType"]
 
 from typing import Any, ClassVar, List, Union, cast
@@ -19,7 +22,7 @@
 
 
 @define
-class EnumProperty(PropertyProtocol):
+class EnumProperty(PropertyProtocol, HasNamedClass):
     """A property that should use an enum"""
 
     name: str
@@ -42,7 +45,7 @@ class EnumProperty(PropertyProtocol):
     }
 
     @classmethod
-    def build(
+    def build(  # noqa: PLR0911
         cls,
         *,
         data: oai.Schema,
@@ -102,6 +105,21 @@ def build(
             Union[List[int], List[str]], unchecked_value_list
         )  # We checked this with all the value_types stuff
 
+        if allow_null:  # Only one of the values was None, that becomes a union
+            data.oneOf = [
+                oai.Schema(type=DataType.NULL),
+                data.model_copy(update={"enum": value_list, "default": data.default}),
+            ]
+            data.enum = None
+            return UnionProperty.build(
+                data=data,
+                name=name,
+                required=required,
+                schemas=schemas,
+                parent_name=parent_name,
+                config=config,
+            )
+
         class_name = data.title or name
         if parent_name:
             class_name = f"{utils.pascal_case(parent_name)}{utils.pascal_case(class_name)}"
@@ -135,38 +153,8 @@ def build(
             return checked_default, schemas
         prop = evolve(prop, default=checked_default)
 
-        # Now, if one of the values was None, wrap the type we just made in a union with None. We're
-        # constructing the UnionProperty directly instead of using UnionProperty.build() because we
-        # do *not* want the usual union behavior of creating ThingType1, ThingType2, etc.
-        returned_prop: EnumProperty | UnionProperty | PropertyError
-        if allow_null:
-            none_prop = NoneProperty.build(
-                name=name,
-                required=required,
-                default=None,
-                python_name=prop.python_name,
-                description=None,
-                example=None,
-            )
-            assert not isinstance(none_prop, PropertyError)
-            union_prop = UnionProperty(
-                name=name,
-                required=required,
-                default=checked_default,
-                python_name=prop.python_name,
-                description=data.description,
-                example=data.example,
-                inner_properties=[
-                    none_prop,
-                    prop,
-                ],
-            )
-            returned_prop = union_prop
-        else:
-            returned_prop = prop
-
         schemas = evolve(schemas, classes_by_name={**schemas.classes_by_name, class_info.name: prop})
-        return returned_prop, schemas
+        return prop, schemas
 
     def convert_value(self, value: Any) -> Value | PropertyError | None:
         if value is None or isinstance(value, Value):