refactor: Change null-only enums to simply be None

Author: dbanty

end_to_end_tests/golden-record/my_test_api_client/api/tests/get_user_list.py

@@ -7,7 +7,6 @@
 from ...models.a_model import AModel
 from ...models.an_enum import AnEnum
 from ...models.an_enum_with_null import AnEnumWithNull
-from ...models.an_enum_with_only_null import AnEnumWithOnlyNull
 from ...models.http_validation_error import HTTPValidationError
 from ...types import UNSET, Response
 
@@ -17,7 +16,7 @@ def _get_kwargs(
     client: Client,
     an_enum_value: List[AnEnum],
     an_enum_value_with_null: List[Optional[AnEnumWithNull]],
-    an_enum_value_with_only_null: List[AnEnumWithOnlyNull],
+    an_enum_value_with_only_null: List[None],
     some_date: Union[datetime.date, datetime.datetime],
 ) -> Dict[str, Any]:
     url = "{}/tests/".format(client.base_url)
@@ -39,11 +38,7 @@ def _get_kwargs(
 
         json_an_enum_value_with_null.append(an_enum_value_with_null_item)
 
-    json_an_enum_value_with_only_null = []
-    for an_enum_value_with_only_null_item_data in an_enum_value_with_only_null:
-        an_enum_value_with_only_null_item = an_enum_value_with_only_null_item_data.value
-
-        json_an_enum_value_with_only_null.append(an_enum_value_with_only_null_item)
+    json_an_enum_value_with_only_null = an_enum_value_with_only_null
 
     if isinstance(some_date, datetime.date):
         json_some_date = some_date.isoformat()
@@ -103,7 +98,7 @@ def sync_detailed(
     client: Client,
     an_enum_value: List[AnEnum],
     an_enum_value_with_null: List[Optional[AnEnumWithNull]],
-    an_enum_value_with_only_null: List[AnEnumWithOnlyNull],
+    an_enum_value_with_only_null: List[None],
     some_date: Union[datetime.date, datetime.datetime],
 ) -> Response[Union[HTTPValidationError, List[AModel]]]:
     kwargs = _get_kwargs(
@@ -126,7 +121,7 @@ def sync(
     client: Client,
     an_enum_value: List[AnEnum],
     an_enum_value_with_null: List[Optional[AnEnumWithNull]],
-    an_enum_value_with_only_null: List[AnEnumWithOnlyNull],
+    an_enum_value_with_only_null: List[None],
     some_date: Union[datetime.date, datetime.datetime],
 ) -> Optional[Union[HTTPValidationError, List[AModel]]]:
     """Get a list of things"""
@@ -145,7 +140,7 @@ async def asyncio_detailed(
     client: Client,
     an_enum_value: List[AnEnum],
     an_enum_value_with_null: List[Optional[AnEnumWithNull]],
-    an_enum_value_with_only_null: List[AnEnumWithOnlyNull],
+    an_enum_value_with_only_null: List[None],
     some_date: Union[datetime.date, datetime.datetime],
 ) -> Response[Union[HTTPValidationError, List[AModel]]]:
     kwargs = _get_kwargs(
@@ -167,7 +162,7 @@ async def asyncio(
     client: Client,
     an_enum_value: List[AnEnum],
     an_enum_value_with_null: List[Optional[AnEnumWithNull]],
-    an_enum_value_with_only_null: List[AnEnumWithOnlyNull],
+    an_enum_value_with_only_null: List[None],
     some_date: Union[datetime.date, datetime.datetime],
 ) -> Optional[Union[HTTPValidationError, List[AModel]]]:
     """Get a list of things"""

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

@@ -8,7 +8,6 @@
 from .an_all_of_enum import AnAllOfEnum
 from .an_enum import AnEnum
 from .an_enum_with_null import AnEnumWithNull
-from .an_enum_with_only_null import AnEnumWithOnlyNull
 from .an_int_enum import AnIntEnum
 from .another_all_of_sub_model import AnotherAllOfSubModel
 from .another_all_of_sub_model_type import AnotherAllOfSubModelType

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

@@ -7,4 +7,3 @@ class AnEnumWithNull(str, Enum):
 
     def __str__(self) -> str:
         return str(self.value)
- 

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

@@ -227,7 +227,6 @@ def _build_models(self) -> None:
         models_dir.mkdir()
         models_init = models_dir / "__init__.py"
         imports = []
-        NoneType = type(None)
 
         model_template = self.env.get_template("model.py.jinja")
         for model in self.openapi.models:
@@ -237,14 +236,11 @@ def _build_models(self) -> None:
 
         # Generate enums
         str_enum_template = self.env.get_template("str_enum.py.jinja")
-        null_enum_template = self.env.get_template("null_enum.py.jinja")
         int_enum_template = self.env.get_template("int_enum.py.jinja")
         for enum in self.openapi.enums:
             module_path = models_dir / f"{enum.class_info.module_name}.py"
             if enum.value_type is int:
                 module_path.write_text(int_enum_template.render(enum=enum), encoding=self.file_encoding)
-            elif enum.value_type is NoneType:
-                module_path.write_text(null_enum_template.render(enum=enum), encoding=self.file_encoding)
             else:
                 module_path.write_text(str_enum_template.render(enum=enum), encoding=self.file_encoding)
             imports.append(import_string_from_class(enum.class_info))

openapi_python_client/__init__.py

@@ -34,6 +34,14 @@ class AnyProperty(Property):
     template: ClassVar[Optional[str]] = "any_property.py.jinja"
 
 
+@attr.s(auto_attribs=True, frozen=True)
+class NoneProperty(Property):
+    """A property that can only be None"""
+
+    _type_string: ClassVar[str] = "None"
+    _json_type_string: ClassVar[str] = "None"
+
+
 @attr.s(auto_attribs=True, frozen=True)
 class StringProperty(Property):
     """A property of type str"""
@@ -299,7 +307,7 @@ def build_enum_property(
     enum: Union[List[Optional[str]], List[Optional[int]]],
     parent_name: Optional[str],
     config: Config,
-) -> Tuple[Union[EnumProperty, PropertyError], Schemas]:
+) -> Tuple[Union[EnumProperty, NoneProperty, PropertyError], Schemas]:
     """
     Create an EnumProperty from schema data.
 
@@ -316,11 +324,34 @@ def build_enum_property(
         A tuple containing either the created property or a PropertyError describing what went wrong AND update schemas.
     """
 
+    if len(enum) == 0:
+        return PropertyError(detail="No values provided for Enum", data=data), schemas
+
     class_name = data.title or name
     if parent_name:
         class_name = f"{utils.pascal_case(parent_name)}{utils.pascal_case(class_name)}"
     class_info = Class.from_string(string=class_name, config=config)
-    values = EnumProperty.values_from_list(enum)
+
+    # OpenAPI allows for null as an enum value, but it doesn't make sense with how enums are constructed in Python.
+    # So instead, if null is a possible value, make the property nullable.
+    # Mypy is not smart enough to know that the type is right though
+    value_list: Union[List[str], List[int]] = [value for value in enum if value is not None]  # type: ignore
+    if len(value_list) < len(enum):
+        data.nullable = True
+
+    # It's legal to have an enum that only contains null as a value, we don't bother constructing an enum in that case
+    if len(value_list) == 0:
+        return (
+            NoneProperty(
+                name=name,
+                required=required,
+                nullable=False,
+                default="None",
+                python_name=utils.PythonIdentifier(value=name, prefix=config.field_prefix),
+            ),
+            schemas,
+        )
+    values = EnumProperty.values_from_list(value_list)
 
     if class_info.name in schemas.classes_by_name:
         existing = schemas.classes_by_name[class_info.name]
@@ -332,19 +363,7 @@ def build_enum_property(
                 schemas,
             )
 
-    # Remove None from str / int list, if present, and mark property as nullable
-    # If list only has None, with no str or int, make special None enum instead
-    keys_to_remove = [key for key, value in values.items() if value is None]
-    if keys_to_remove and len(keys_to_remove) < len(values.items()):
-        data.nullable = True
-        for key in keys_to_remove:
-            values.pop(key)
-
-    for value in values.values():
-        value_type = type(value)
-        break
-    else:
-        return PropertyError(data=data, detail="No values provided for Enum"), schemas
+    value_type = type(next(iter(values.values())))
 
     prop = EnumProperty(
         name=name,

openapi_python_client/parser/properties/__init__.py

@@ -8,7 +8,7 @@
 from .property import Property
 from .schemas import Class
 
-ValueType = Union[str, int, None]
+ValueType = Union[str, int]
 
 
 @attr.s(auto_attribs=True, frozen=True)
@@ -41,7 +41,7 @@ def get_imports(self, *, prefix: str) -> Set[str]:
         return imports
 
     @staticmethod
-    def values_from_list(values: Union[List[Optional[str]], List[Optional[int]]]) -> Dict[str, ValueType]:
+    def values_from_list(values: Union[List[str], List[int]]) -> Dict[str, ValueType]:
         """Convert a list of values into dict of {name: value}, where value can sometimes be None"""
         output: Dict[str, ValueType] = {}
 
@@ -60,6 +60,5 @@ def values_from_list(values: Union[List[Optional[str]], List[Optional[int]]]) ->
             if key in output:
                 raise ValueError(f"Duplicate key {key} in Enum")
             sanitized_key = utils.snake_case(key).upper()
-            output[sanitized_key] = utils.remove_string_escapes(value) if isinstance(value, str) else value
-            # If value is the string "null", this becomes Python's None instead of a str, so must special-case it
+            output[sanitized_key] = utils.remove_string_escapes(value)
         return output

openapi_python_client/parser/properties/enum_property.py

@@ -6,7 +6,7 @@
 import openapi_python_client.schema as oai
 from openapi_python_client import Config
 from openapi_python_client.parser.errors import PropertyError, ValidationError
-from openapi_python_client.parser.properties import BooleanProperty, FloatProperty, IntProperty, Schemas
+from openapi_python_client.parser.properties import BooleanProperty, FloatProperty, IntProperty, NoneProperty, Schemas
 
 MODULE_NAME = "openapi_python_client.parser.properties"
 
@@ -222,8 +222,6 @@ class TestEnumProperty:
             (True, False, "{}"),
             (False, True, "Union[Unset, None, {}]"),
             (True, True, "Optional[{}]"),
-            (True, False, "Optional[{}]"),
-            (True, False, "{}"),
         ),
     )
     def test_get_type_string(self, mocker, enum_property_factory, required, nullable, expected):
@@ -275,37 +273,6 @@ def test_values_from_list_duplicate(self):
         with pytest.raises(ValueError):
             EnumProperty.values_from_list(data)
 
-    def test_values_from_list_with_null(self):
-        from openapi_python_client.parser.properties import EnumProperty
-
-        data = ["abc", "123", "a23", "1bc", 4, -3, "a Thing WIth spaces", "", "null"]
-
-        result = EnumProperty.values_from_list(data)
-
-        # None / null is removed from result, and result is now Optional[{}]
-        assert result == {
-            "ABC": "abc",
-            "VALUE_1": "123",
-            "A23": "a23",
-            "VALUE_3": "1bc",
-            "VALUE_4": 4,
-            "VALUE_NEGATIVE_3": -3,
-            "A_THING_WITH_SPACES": "a Thing WIth spaces",
-            "VALUE_7": "",
-        }
-
-    def test_values_from_list_with_only_null(self):
-        from openapi_python_client.parser.properties import EnumProperty
-
-        data = ["null"]
-
-        result = EnumProperty.values_from_list(data)
-
-        # None / null is not removed from result since it's the only value
-        assert result == {
-            "VALUE_0": None,
-        }
-
 
 class TestPropertyFromData:
     def test_property_from_data_str_enum(self, enum_property_factory):
@@ -342,7 +309,7 @@ def test_property_from_data_str_enum_with_null(self, enum_property_factory):
         from openapi_python_client.schema import Schema
 
         existing = enum_property_factory()
-        data = Schema(title="AnEnumWithNull", enum=["A", "B", "C", "null"], nullable=False, default="B")
+        data = Schema(title="AnEnum", enum=["A", "B", "C", None], nullable=False, default="B")
         name = "my_enum"
         required = True
 
@@ -356,6 +323,7 @@ def test_property_from_data_str_enum_with_null(self, enum_property_factory):
         assert prop == enum_property_factory(
             name=name,
             required=required,
+            nullable=True,
             values={"A": "A", "B": "B", "C": "C"},
             class_info=Class(name="ParentAnEnum", module_name="parent_an_enum"),
             value_type=str,
@@ -364,42 +332,30 @@ def test_property_from_data_str_enum_with_null(self, enum_property_factory):
         assert prop.nullable is True
         assert schemas != new_schemas, "Provided Schemas was mutated"
         assert new_schemas.classes_by_name == {
-            "AnEnumWithNull": existing,
+            "AnEnum": existing,
             "ParentAnEnum": prop,
         }
 
     def test_property_from_data_null_enum(self, enum_property_factory):
         from openapi_python_client.parser.properties import Class, Schemas, property_from_data
         from openapi_python_client.schema import Schema
 
-        existing = enum_property_factory()
-        data = Schema(title="AnEnumWithOnlyNull", enum=["null"], nullable=False, default=None)
+        data = Schema(title="AnEnumWithOnlyNull", enum=[None], nullable=False, default=None)
         name = "my_enum"
         required = True
 
-        schemas = Schemas(classes_by_name={"AnEnum": existing})
+        schemas = Schemas()
 
         prop, new_schemas = property_from_data(
             name=name, required=required, data=data, schemas=schemas, parent_name="parent", config=Config()
         )
 
-        assert prop == enum_property_factory(
-            name=name,
-            required=required,
-            values={"VALUE_0": None},
-            class_info=Class(name="ParentAnEnum", module_name="parent_an_enum"),
-            value_type=type(None),
-            default=None,
+        assert prop == NoneProperty(
+            name="my_enum", required=required, nullable=False, default="None", python_name="my_enum"
         )
-        assert prop.nullable is False
-        assert schemas != new_schemas, "Provided Schemas was mutated"
-        assert new_schemas.classes_by_name == {
-            "AnEnumWithOnlyNull": existing,
-            "ParentAnEnum": prop,
-        }
 
     def test_property_from_data_int_enum(self, enum_property_factory):
-        from openapi_python_client.parser.properties import Class, EnumProperty, Schemas, property_from_data
+        from openapi_python_client.parser.properties import Class, Schemas, property_from_data
         from openapi_python_client.schema import Schema
 
         name = "my_enum"
@@ -1017,14 +973,17 @@ def test_retries_failing_properties_while_making_progress(self, mocker):
         assert result.errors == [PropertyError()]
 
 
-def test_build_enum_property_conflict(mocker):
+def test_build_enum_property_conflict():
     from openapi_python_client.parser.properties import Schemas, build_enum_property
 
     data = oai.Schema()
-    schemas = Schemas(classes_by_name={"Existing": mocker.MagicMock()})
+    schemas = Schemas()
 
+    _, schemas = build_enum_property(
+        data=data, name="Existing", required=True, schemas=schemas, enum=["a"], parent_name=None, config=Config()
+    )
     err, schemas = build_enum_property(
-        data=data, name="Existing", required=True, schemas=schemas, enum=[], parent_name=None, config=Config()
+        data=data, name="Existing", required=True, schemas=schemas, enum=["a", "b"], parent_name=None, config=Config()
     )
 
     assert schemas == schemas